diff --git a/docs/api.md b/docs/api.md index 2e91d4f2b..bd5154ada 100644 --- a/docs/api.md +++ b/docs/api.md @@ -330,6 +330,64 @@ granted). You can pass the parameter `full_perms=true` to API calls to view the full permissions of objects in a format that mirrors the `set_permissions` parameter above. +## Bulk Editing + +The API supports various bulk-editing operations which are executed asynchronously. + +### Documents + +For bulk operations on documents, use the endpoint `/api/bulk_edit/` which accepts +a json payload of the format: + +```json +{ + "documents": [LIST_OF_DOCUMENT_IDS], + "method": METHOD, // see below + "parameters": args // see below +} +``` + +The following methods are supported: + +- `set_correspondent` + - Requires `parameters`: `{ "correspondent": CORRESPONDENT_ID }` +- `set_document_type` + - Requires `parameters`: `{ "document_type": DOCUMENT_TYPE_ID }` +- `set_storage_path` + - Requires `parameters`: `{ "storage_path": STORAGE_PATH_ID }` +- `add_tag` + - Requires `parameters`: `{ "tag": TAG_ID }` +- `remove_tag` + - Requires `parameters`: `{ "tag": TAG_ID }` +- `modify_tags` + - Requires `parameters`: `{ "add_tags": [LIST_OF_TAG_IDS] }` and / or `{ "remove_tags": [LIST_OF_TAG_IDS] }` +- `delete` + - No `parameters` required +- `redo_ocr` + - No `parameters` required +- `set_permissions` + - Requires `parameters`: + - `"permissions": PERMISSIONS_OBJ` (see format [above](#permissions)) and / or + - `"owner": OWNER_ID or null` + - `"merge": true or false` (defaults to false) + - The `merge` flag determines if the supplied permissions will overwrite all existing permissions (including + removing them) or be merged with existing permissions. + +### Objects + +Bulk editing for objects (tags, document types etc.) currently supports only updating permissions, using +the endpoint: `/api/bulk_edit_object_perms/` which requires a json payload of the format: + +```json +{ + "objects": [LIST_OF_OBJECT_IDS], + "object_type": "tags", "correspondents", "document_types" or "storage_paths" + "owner": OWNER_ID // optional + "permissions": { "view": { "users": [] ... }, "change": { ... } }, // (see 'set_permissions' format above) + "merge": true / false // defaults to false, see above +} +``` + ## API Versioning The REST API is versioned since Paperless-ngx 1.3.0. @@ -386,3 +444,13 @@ Initial API version. color to use for a specific tag, which is either black or white depending on the brightness of `Tag.color`. - Removed field `Tag.colour`. + +#### Version 3 + +- Permissions endpoints have been added. +- The format of the `/api/ui_settings/` has changed. + +#### Version 4 + +- Consumption templates were refactored to workflows and API endpoints + changed as such. diff --git a/src-ui/messages.xlf b/src-ui/messages.xlf index e7263e0f2..56866f512 100644 --- a/src-ui/messages.xlf +++ b/src-ui/messages.xlf @@ -620,7 +620,7 @@ src/app/components/common/permissions-dialog/permissions-dialog.component.html - 20 + 23 src/app/components/dashboard/dashboard.component.html @@ -2438,7 +2438,7 @@ src/app/components/common/permissions-dialog/permissions-dialog.component.html - 23 + 26 src/app/components/document-list/bulk-editor/bulk-editor.component.ts @@ -2505,7 +2505,7 @@ src/app/components/common/permissions-dialog/permissions-dialog.component.html - 22 + 25 src/app/components/common/profile-edit-dialog/profile-edit-dialog.component.html @@ -3923,6 +3923,13 @@ 15 + + Merge with existing permissions + + src/app/components/common/permissions-dialog/permissions-dialog.component.html + 14 + + Set permissions @@ -3937,11 +3944,18 @@ 33 - - Note that permissions set here will override any existing permissions + + Existing owner, user and group permissions will be merged with these settings. src/app/components/common/permissions-dialog/permissions-dialog.component.ts - 71 + 74 + + + + Any and all existing owner, user and group permissions will be replaced. + + src/app/components/common/permissions-dialog/permissions-dialog.component.ts + 75 @@ -6100,18 +6114,18 @@ Permissions updated src/app/components/manage/mail/mail.component.ts - 211 + 212 Error updating permissions src/app/components/manage/mail/mail.component.ts - 215 + 217 src/app/components/manage/management-list/management-list.component.ts - 300 + 301 @@ -6277,7 +6291,7 @@ Permissions updated successfully src/app/components/manage/management-list/management-list.component.ts - 293 + 294 diff --git a/src-ui/src/app/components/common/input/switch/switch.component.html b/src-ui/src/app/components/common/input/switch/switch.component.html index 489106fcb..5acef7a75 100644 --- a/src-ui/src/app/components/common/input/switch/switch.component.html +++ b/src-ui/src/app/components/common/input/switch/switch.component.html @@ -15,7 +15,7 @@ } } -
+
@if (horizontal) { diff --git a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.html b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.html index 37ab7efbd..fd88002ba 100644 --- a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.html +++ b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.html @@ -5,12 +5,15 @@
@@ -20,5 +23,5 @@ Loading... } - +
diff --git a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.spec.ts b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.spec.ts index 3f601d771..bc8ccdecb 100644 --- a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.spec.ts +++ b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.spec.ts @@ -11,6 +11,7 @@ import { NgSelectModule } from '@ng-select/ng-select' import { FormsModule, ReactiveFormsModule } from '@angular/forms' import { PermissionsUserComponent } from '../input/permissions/permissions-user/permissions-user.component' import { PermissionsGroupComponent } from '../input/permissions/permissions-group/permissions-group.component' +import { SwitchComponent } from '../input/switch/switch.component' const set_permissions = { owner: 10, @@ -37,6 +38,7 @@ describe('PermissionsDialogComponent', () => { PermissionsDialogComponent, SafeHtmlPipe, SelectComponent, + SwitchComponent, PermissionsFormComponent, PermissionsUserComponent, PermissionsGroupComponent, @@ -112,4 +114,23 @@ describe('PermissionsDialogComponent', () => { expect(component.title).toEqual(`Edit permissions for ${obj.name}`) expect(component.permissions).toEqual(set_permissions) }) + + it('should toggle hint based on object existence (if editing) or merge flag', () => { + component.form.get('merge').setValue(true) + expect(component.hint.includes('Existing')).toBeTruthy() + component.form.get('merge').setValue(false) + expect(component.hint.includes('will be replaced')).toBeTruthy() + component.object = {} + expect(component.hint).toBeNull() + }) + + it('should emit permissions and merge flag on confirm', () => { + const confirmSpy = jest.spyOn(component.confirmClicked, 'emit') + component.form.get('permissions_form').setValue(set_permissions) + component.confirm() + expect(confirmSpy).toHaveBeenCalledWith({ + permissions: set_permissions, + merge: true, + }) + }) }) diff --git a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.ts b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.ts index 9a514387c..afe1bebb3 100644 --- a/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.ts +++ b/src-ui/src/app/components/common/permissions-dialog/permissions-dialog.component.ts @@ -32,6 +32,7 @@ export class PermissionsDialogComponent { this.o = o this.title = $localize`Edit permissions for ` + o['name'] this.form.patchValue({ + merge: true, permissions_form: { owner: o.owner, set_permissions: o.permissions, @@ -43,8 +44,9 @@ export class PermissionsDialogComponent { return this.o } - form = new FormGroup({ + public form = new FormGroup({ permissions_form: new FormControl(), + merge: new FormControl(true), }) buttonsEnabled: boolean = true @@ -66,11 +68,21 @@ export class PermissionsDialogComponent { } } - @Input() - message = - $localize`Note that permissions set here will override any existing permissions` + get hint(): string { + if (this.object) return null + return this.form.get('merge').value + ? $localize`Existing owner, user and group permissions will be merged with these settings.` + : $localize`Any and all existing owner, user and group permissions will be replaced.` + } cancelClicked() { this.activeModal.close() } + + confirm() { + this.confirmClicked.emit({ + permissions: this.permissions, + merge: this.form.get('merge').value, + }) + } } diff --git a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts index 8edfcc7e1..42f8b6d1d 100644 --- a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts +++ b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.spec.ts @@ -41,6 +41,7 @@ import { PermissionsUserComponent } from '../../common/input/permissions/permiss import { NgSelectModule } from '@ng-select/ng-select' import { GroupService } from 'src/app/services/rest/group.service' import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons' +import { SwitchComponent } from '../../common/input/switch/switch.component' const selectionData: SelectionData = { selected_tags: [ @@ -81,6 +82,7 @@ describe('BulkEditorComponent', () => { SelectComponent, PermissionsGroupComponent, PermissionsUserComponent, + SwitchComponent, ], providers: [ PermissionsService, @@ -851,7 +853,18 @@ describe('BulkEditorComponent', () => { fixture.detectChanges() component.setPermissions() expect(modal).not.toBeUndefined() - modal.componentInstance.confirmClicked.next() + const perms = { + permissions: { + view_users: [], + change_users: [], + view_groups: [], + change_groups: [], + }, + } + modal.componentInstance.confirmClicked.emit({ + permissions: perms, + merge: true, + }) let req = httpTestingController.expectOne( `${environment.apiBaseUrl}documents/bulk_edit/` ) @@ -859,7 +872,10 @@ describe('BulkEditorComponent', () => { expect(req.request.body).toEqual({ documents: [3, 4], method: 'set_permissions', - parameters: undefined, + parameters: { + permissions: perms.permissions, + merge: true, + }, }) httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true` diff --git a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts index 1c7e0b9c6..49d4c070f 100644 --- a/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts +++ b/src-ui/src/app/components/document-list/bulk-editor/bulk-editor.component.ts @@ -540,9 +540,14 @@ export class BulkEditorComponent let modal = this.modalService.open(PermissionsDialogComponent, { backdrop: 'static', }) - modal.componentInstance.confirmClicked.subscribe((permissions) => { - modal.componentInstance.buttonsEnabled = false - this.executeBulkOperation(modal, 'set_permissions', permissions) - }) + modal.componentInstance.confirmClicked.subscribe( + ({ permissions, merge }) => { + modal.componentInstance.buttonsEnabled = false + this.executeBulkOperation(modal, 'set_permissions', { + ...permissions, + merge, + }) + } + ) } } diff --git a/src-ui/src/app/components/manage/mail/mail.component.spec.ts b/src-ui/src/app/components/manage/mail/mail.component.spec.ts index 5680d8faa..fcc5bcc6b 100644 --- a/src-ui/src/app/components/manage/mail/mail.component.spec.ts +++ b/src-ui/src/app/components/manage/mail/mail.component.spec.ts @@ -41,6 +41,7 @@ import { TagsComponent } from '../../common/input/tags/tags.component' import { FormsModule, ReactiveFormsModule } from '@angular/forms' import { EditDialogMode } from '../../common/edit-dialog/edit-dialog.component' import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons' +import { SwitchComponent } from '../../common/input/switch/switch.component' const mailAccounts = [ { id: 1, name: 'account1' }, @@ -82,6 +83,7 @@ describe('MailComponent', () => { PermissionsGroupComponent, PermissionsDialogComponent, PermissionsFormComponent, + SwitchComponent, ], providers: [CustomDatePipe, DatePipe, PermissionsGuard], imports: [ @@ -267,11 +269,11 @@ describe('MailComponent', () => { rulePatchSpy.mockReturnValueOnce( throwError(() => new Error('error saving perms')) ) - dialog.confirmClicked.emit(perms) + dialog.confirmClicked.emit({ permissions: perms, merge: true }) expect(rulePatchSpy).toHaveBeenCalled() expect(toastErrorSpy).toHaveBeenCalled() rulePatchSpy.mockReturnValueOnce(of(mailRules[0] as MailRule)) - dialog.confirmClicked.emit(perms) + dialog.confirmClicked.emit({ permissions: perms, merge: true }) expect(toastInfoSpy).toHaveBeenCalledWith('Permissions updated') modalService.dismissAll() @@ -299,8 +301,7 @@ describe('MailComponent', () => { expect(modal).not.toBeUndefined() let dialog = modal.componentInstance as PermissionsDialogComponent expect(dialog.object).toEqual(mailAccounts[0]) - dialog = modal.componentInstance as PermissionsDialogComponent - dialog.confirmClicked.emit(perms) + dialog.confirmClicked.emit({ permissions: perms, merge: true }) expect(accountPatchSpy).toHaveBeenCalled() }) }) diff --git a/src-ui/src/app/components/manage/mail/mail.component.ts b/src-ui/src/app/components/manage/mail/mail.component.ts index 9bb33c03b..d8820ed38 100644 --- a/src-ui/src/app/components/manage/mail/mail.component.ts +++ b/src-ui/src/app/components/manage/mail/mail.component.ts @@ -200,22 +200,27 @@ export class MailComponent const dialog: PermissionsDialogComponent = modal.componentInstance as PermissionsDialogComponent dialog.object = object - modal.componentInstance.confirmClicked.subscribe((permissions) => { - modal.componentInstance.buttonsEnabled = false - const service: AbstractPaperlessService = - 'account' in object ? this.mailRuleService : this.mailAccountService - object.owner = permissions['owner'] - object['set_permissions'] = permissions['set_permissions'] - service.patch(object).subscribe({ - next: () => { - this.toastService.showInfo($localize`Permissions updated`) - modal.close() - }, - error: (e) => { - this.toastService.showError($localize`Error updating permissions`, e) - }, - }) - }) + modal.componentInstance.confirmClicked.subscribe( + ({ permissions, merge }) => { + modal.componentInstance.buttonsEnabled = false + const service: AbstractPaperlessService = + 'account' in object ? this.mailRuleService : this.mailAccountService + object.owner = permissions['owner'] + object['set_permissions'] = permissions['set_permissions'] + service.patch(object).subscribe({ + next: () => { + this.toastService.showInfo($localize`Permissions updated`) + modal.close() + }, + error: (e) => { + this.toastService.showError( + $localize`Error updating permissions`, + e + ) + }, + }) + } + ) } userCanEdit(obj: ObjectWithPermissions): boolean { diff --git a/src-ui/src/app/components/manage/management-list/management-list.component.spec.ts b/src-ui/src/app/components/manage/management-list/management-list.component.spec.ts index 44a3452d7..6196a3c8a 100644 --- a/src-ui/src/app/components/manage/management-list/management-list.component.spec.ts +++ b/src-ui/src/app/components/manage/management-list/management-list.component.spec.ts @@ -264,13 +264,19 @@ describe('ManagementListComponent', () => { throwError(() => new Error('error setting permissions')) ) const errorToastSpy = jest.spyOn(toastService, 'showError') - modal.componentInstance.confirmClicked.emit() + modal.componentInstance.confirmClicked.emit({ + permissions: {}, + merge: true, + }) expect(bulkEditPermsSpy).toHaveBeenCalled() expect(errorToastSpy).toHaveBeenCalled() const successToastSpy = jest.spyOn(toastService, 'showInfo') bulkEditPermsSpy.mockReturnValueOnce(of('OK')) - modal.componentInstance.confirmClicked.emit() + modal.componentInstance.confirmClicked.emit({ + permissions: {}, + merge: true, + }) expect(bulkEditPermsSpy).toHaveBeenCalled() expect(successToastSpy).toHaveBeenCalled() }) diff --git a/src-ui/src/app/components/manage/management-list/management-list.component.ts b/src-ui/src/app/components/manage/management-list/management-list.component.ts index e20b5d4a7..a89b5e4f6 100644 --- a/src-ui/src/app/components/manage/management-list/management-list.component.ts +++ b/src-ui/src/app/components/manage/management-list/management-list.component.ts @@ -279,12 +279,13 @@ export abstract class ManagementListComponent backdrop: 'static', }) modal.componentInstance.confirmClicked.subscribe( - (permissions: { owner: number; set_permissions: PermissionsObject }) => { + ({ permissions, merge }) => { modal.componentInstance.buttonsEnabled = false this.service .bulk_update_permissions( Array.from(this.selectedObjects), - permissions + permissions, + merge ) .subscribe({ next: () => { diff --git a/src-ui/src/app/services/rest/abstract-name-filter-service.spec.ts b/src-ui/src/app/services/rest/abstract-name-filter-service.spec.ts index 70ae211e5..e09270701 100644 --- a/src-ui/src/app/services/rest/abstract-name-filter-service.spec.ts +++ b/src-ui/src/app/services/rest/abstract-name-filter-service.spec.ts @@ -53,10 +53,14 @@ export const commonAbstractNameFilterPaperlessServiceTests = ( }, } subscription = service - .bulk_update_permissions([1, 2], { - owner, - set_permissions: permissions, - }) + .bulk_update_permissions( + [1, 2], + { + owner, + set_permissions: permissions, + }, + true + ) .subscribe() const req = httpTestingController.expectOne( `${environment.apiBaseUrl}bulk_edit_object_perms/` diff --git a/src-ui/src/app/services/rest/abstract-name-filter-service.ts b/src-ui/src/app/services/rest/abstract-name-filter-service.ts index 5e0377cb9..b38994086 100644 --- a/src-ui/src/app/services/rest/abstract-name-filter-service.ts +++ b/src-ui/src/app/services/rest/abstract-name-filter-service.ts @@ -26,13 +26,15 @@ export abstract class AbstractNameFilterService< bulk_update_permissions( objects: Array, - permissions: { owner: number; set_permissions: PermissionsObject } + permissions: { owner: number; set_permissions: PermissionsObject }, + merge: boolean ): Observable { return this.http.post(`${this.baseUrl}bulk_edit_object_perms/`, { objects, object_type: this.resourceName, owner: permissions.owner, permissions: permissions.set_permissions, + merge, }) } } diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index ab0049eaa..ba001fd14 100644 --- a/src/documents/bulk_edit.py +++ b/src/documents/bulk_edit.py @@ -129,13 +129,17 @@ def redo_ocr(doc_ids): return "OK" -def set_permissions(doc_ids, set_permissions, owner=None): +def set_permissions(doc_ids, set_permissions, owner=None, merge=False): qs = Document.objects.filter(id__in=doc_ids) - qs.update(owner=owner) + if merge: + # If merging, only set owner for documents that don't have an owner + qs.filter(owner__isnull=True).update(owner=owner) + else: + qs.update(owner=owner) for doc in qs: - set_permissions_for_object(set_permissions, doc) + set_permissions_for_object(permissions=set_permissions, object=doc, merge=merge) affected_docs = [doc.id for doc in qs] diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 942d22fe5..3c65e11d9 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -916,6 +916,8 @@ class BulkEditSerializer(DocumentListSerializer, SetPermissionsMixin): ) if "owner" in parameters and parameters["owner"] is not None: self._validate_owner(parameters["owner"]) + if "merge" not in parameters: + parameters["merge"] = False def validate(self, attrs): method = attrs["method"] @@ -1258,6 +1260,12 @@ class BulkEditObjectPermissionsSerializer(serializers.Serializer, SetPermissions write_only=True, ) + merge = serializers.BooleanField( + default=False, + write_only=True, + required=False, + ) + def get_object_class(self, object_type): object_class = None if object_type == "tags": diff --git a/src/documents/test_bulk_edit.py b/src/documents/test_bulk_edit.py new file mode 100644 index 000000000..ad622ca24 --- /dev/null +++ b/src/documents/test_bulk_edit.py @@ -0,0 +1,110 @@ +from unittest import mock + +from django.contrib.auth.models import Group +from django.contrib.auth.models import User +from django.test import TestCase +from guardian.shortcuts import assign_perm +from guardian.shortcuts import get_groups_with_perms +from guardian.shortcuts import get_users_with_perms + +from documents.bulk_edit import set_permissions +from documents.models import Document +from documents.tests.utils import DirectoriesMixin + + +class TestBulkEditPermissions(DirectoriesMixin, TestCase): + def setUp(self): + super().setUp() + + self.doc1 = Document.objects.create(checksum="A", title="A") + self.doc2 = Document.objects.create(checksum="B", title="B") + self.doc3 = Document.objects.create(checksum="C", title="C") + + self.owner = User.objects.create(username="test_owner") + self.user1 = User.objects.create(username="user1") + self.user2 = User.objects.create(username="user2") + self.group1 = Group.objects.create(name="group1") + self.group2 = Group.objects.create(name="group2") + + @mock.patch("documents.tasks.bulk_update_documents.delay") + def test_set_permissions(self, m): + doc_ids = [self.doc1.id, self.doc2.id, self.doc3.id] + + assign_perm("view_document", self.group1, self.doc1) + + permissions = { + "view": { + "users": [self.user1.id, self.user2.id], + "groups": [self.group2.id], + }, + "change": { + "users": [self.user1.id], + "groups": [self.group2.id], + }, + } + + set_permissions( + doc_ids, + set_permissions=permissions, + owner=self.owner, + merge=False, + ) + m.assert_called_once() + + self.assertEqual(Document.objects.filter(owner=self.owner).count(), 3) + self.assertEqual(Document.objects.filter(id__in=doc_ids).count(), 3) + + users_with_perms = get_users_with_perms( + self.doc1, + ) + self.assertEqual(users_with_perms.count(), 2) + + # group1 should be replaced by group2 + groups_with_perms = get_groups_with_perms( + self.doc1, + ) + self.assertEqual(groups_with_perms.count(), 1) + + @mock.patch("documents.tasks.bulk_update_documents.delay") + def test_set_permissions_merge(self, m): + doc_ids = [self.doc1.id, self.doc2.id, self.doc3.id] + + self.doc1.owner = self.user1 + self.doc1.save() + + assign_perm("view_document", self.user1, self.doc1) + assign_perm("view_document", self.group1, self.doc1) + + permissions = { + "view": { + "users": [self.user2.id], + "groups": [self.group2.id], + }, + "change": { + "users": [self.user2.id], + "groups": [self.group2.id], + }, + } + set_permissions( + doc_ids, + set_permissions=permissions, + owner=self.owner, + merge=True, + ) + m.assert_called_once() + + # when merge is true owner doesn't get replaced if its not empty + self.assertEqual(Document.objects.filter(owner=self.owner).count(), 2) + self.assertEqual(Document.objects.filter(id__in=doc_ids).count(), 3) + + # merge of user1 which was pre-existing and user2 + users_with_perms = get_users_with_perms( + self.doc1, + ) + self.assertEqual(users_with_perms.count(), 2) + + # group1 should be merged by group2 + groups_with_perms = get_groups_with_perms( + self.doc1, + ) + self.assertEqual(groups_with_perms.count(), 2) diff --git a/src/documents/tests/test_api_bulk_edit.py b/src/documents/tests/test_api_bulk_edit.py index c2dc69a1e..4cf9909c0 100644 --- a/src/documents/tests/test_api_bulk_edit.py +++ b/src/documents/tests/test_api_bulk_edit.py @@ -765,6 +765,58 @@ class TestBulkEdit(DirectoriesMixin, APITestCase): self.assertCountEqual(args[0], [self.doc2.id, self.doc3.id]) self.assertEqual(len(kwargs["set_permissions"]["view"]["users"]), 2) + @mock.patch("documents.serialisers.bulk_edit.set_permissions") + def test_set_permissions_merge(self, m): + m.return_value = "OK" + user1 = User.objects.create(username="user1") + user2 = User.objects.create(username="user2") + permissions = { + "view": { + "users": [user1.id, user2.id], + "groups": None, + }, + "change": { + "users": [user1.id], + "groups": None, + }, + } + + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id, self.doc3.id], + "method": "set_permissions", + "parameters": {"set_permissions": permissions}, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + m.assert_called() + args, kwargs = m.call_args + self.assertEqual(kwargs["merge"], False) + + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [self.doc2.id, self.doc3.id], + "method": "set_permissions", + "parameters": {"set_permissions": permissions, "merge": True}, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + m.assert_called() + args, kwargs = m.call_args + self.assertEqual(kwargs["merge"], True) + @mock.patch("documents.serialisers.bulk_edit.set_permissions") def test_insufficient_permissions_ownership(self, m): """ diff --git a/src/documents/tests/test_api_permissions.py b/src/documents/tests/test_api_permissions.py index a0b22586b..851608f37 100644 --- a/src/documents/tests/test_api_permissions.py +++ b/src/documents/tests/test_api_permissions.py @@ -700,8 +700,8 @@ class TestBulkEditObjectPermissions(APITestCase): def setUp(self): super().setUp() - user = User.objects.create_superuser(username="temp_admin") - self.client.force_authenticate(user=user) + self.temp_admin = User.objects.create_superuser(username="temp_admin") + self.client.force_authenticate(user=self.temp_admin) self.t1 = Tag.objects.create(name="t1") self.t2 = Tag.objects.create(name="t2") @@ -822,6 +822,79 @@ class TestBulkEditObjectPermissions(APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(StoragePath.objects.get(pk=self.sp1.id).owner, self.user3) + def test_bulk_object_set_permissions_merge(self): + """ + GIVEN: + - Existing objects + WHEN: + - bulk_edit_object_perms API endpoint is called with merge=True or merge=False (default) + THEN: + - Permissions and / or owner are replaced or merged, depending on the merge flag + """ + permissions = { + "view": { + "users": [self.user1.id, self.user2.id], + "groups": [], + }, + "change": { + "users": [self.user1.id], + "groups": [], + }, + } + + assign_perm("view_tag", self.user3, self.t1) + self.t1.owner = self.user3 + self.t1.save() + + # merge=True + response = self.client.post( + "/api/bulk_edit_object_perms/", + json.dumps( + { + "objects": [self.t1.id, self.t2.id], + "object_type": "tags", + "owner": self.user1.id, + "permissions": permissions, + "merge": True, + }, + ), + content_type="application/json", + ) + + self.t1.refresh_from_db() + self.t2.refresh_from_db() + + self.assertEqual(response.status_code, status.HTTP_200_OK) + # user3 should still be owner of t1 since was set prior + self.assertEqual(self.t1.owner, self.user3) + # user1 should now be owner of t2 since it didn't have an owner + self.assertEqual(self.t2.owner, self.user1) + + # user1 should be added + self.assertIn(self.user1, get_users_with_perms(self.t1)) + # user3 should be preserved + self.assertIn(self.user3, get_users_with_perms(self.t1)) + + # merge=False (default) + response = self.client.post( + "/api/bulk_edit_object_perms/", + json.dumps( + { + "objects": [self.t1.id, self.t2.id], + "object_type": "tags", + "permissions": permissions, + "merge": False, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + # user1 should be added + self.assertIn(self.user1, get_users_with_perms(self.t1)) + # user3 should be removed + self.assertNotIn(self.user3, get_users_with_perms(self.t1)) + def test_bulk_edit_object_permissions_insufficient_perms(self): """ GIVEN: diff --git a/src/documents/views.py b/src/documents/views.py index 9d44ecbc4..11fb5b1f2 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -1385,6 +1385,7 @@ class BulkEditObjectPermissionsView(GenericAPIView, PassUserMixin): object_class = serializer.get_object_class(object_type) permissions = serializer.validated_data.get("permissions") owner = serializer.validated_data.get("owner") + merge = serializer.validated_data.get("merge") if not user.is_superuser: objs = object_class.objects.filter(pk__in=object_ids) @@ -1396,12 +1397,21 @@ class BulkEditObjectPermissionsView(GenericAPIView, PassUserMixin): try: qs = object_class.objects.filter(id__in=object_ids) - if "owner" in serializer.validated_data: - qs.update(owner=owner) + # if merge is true, we dont want to remove the owner + if "owner" in serializer.validated_data and ( + not merge or (merge and owner is not None) + ): + # if merge is true, we dont want to overwrite the owner + qs_owner_update = qs.filter(owner__isnull=True) if merge else qs + qs_owner_update.update(owner=owner) if "permissions" in serializer.validated_data: for obj in qs: - set_permissions_for_object(permissions, obj) + set_permissions_for_object( + permissions=permissions, + object=obj, + merge=merge, + ) return Response({"result": "OK"}) except Exception as e: