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 9636023429..909a7b820f 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 @@ -1175,18 +1175,17 @@ describe('BulkEditorComponent', () => { const notifySpy = jest .spyOn(trashService, 'notifyMovedToTrash') .mockImplementation(() => {}) - req.flush({ result: 'OK', task_id: 'queued-task' }) + req.flush(true) expect(req.request.body).toEqual({ - from_webui: true, documents: [3, 4], }) - expect(notifySpy).not.toHaveBeenCalled() - httpTestingController.expectNone( + expect(notifySpy).toHaveBeenCalledWith(expect.any(String), [3, 4]) + httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` - ) - httpTestingController.expectNone( + ) // list reload + httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=100000&fields=id` - ) + ) // listAllFilteredIds component.showConfirmationDialogs = false fixture.detectChanges() @@ -1197,12 +1196,12 @@ describe('BulkEditorComponent', () => { req = httpTestingController.expectOne( `${environment.apiBaseUrl}documents/delete/` ) - req.flush({ result: 'OK', task_id: 'queued-task' }) - // Acceptance does not claim deletion has completed. - expect(notifySpy).not.toHaveBeenCalled() - httpTestingController.expectNone( + req.flush(true) + // ids aren't known when all matching documents are selected, so no undo + expect(notifySpy).toHaveBeenLastCalledWith(expect.any(String), null) + httpTestingController.match( `${environment.apiBaseUrl}documents/?page=1&page_size=50&ordering=-created&truncate_content=true&include_selection_data=true` - ) + ) // list reload }) it('should not be accessible with insufficient global permissions', () => { 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 73c166be5f..647946768c 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 @@ -19,7 +19,15 @@ import { NgbModalRef, } from '@ng-bootstrap/ng-bootstrap' import { saveAs } from 'file-saver' -import { first, map, Observable, Subject, switchMap, takeUntil } from 'rxjs' +import { + first, + map, + Observable, + Subject, + switchMap, + takeUntil, + tap, +} from 'rxjs' import { BootstrapIconComponent } from 'src/app/components/common/bootstrap-icon/bootstrap-icon.component' import { ConfirmDialogComponent } from 'src/app/components/common/confirm-dialog/confirm-dialog.component' import { CustomField } from 'src/app/data/custom-field' @@ -49,6 +57,7 @@ import { StoragePathService } from 'src/app/services/rest/storage-path.service' import { TagService } from 'src/app/services/rest/tag.service' import { SettingsService } from 'src/app/services/settings.service' import { ToastService } from 'src/app/services/toast.service' +import { TrashService } from 'src/app/services/trash.service' import { flattenTags } from 'src/app/utils/flatten-tags' import { queryParamsFromFilterRules } from 'src/app/utils/query-params' import { MergeAsVersionsConfirmDialogComponent } from '../../common/confirm-dialog/merge-as-versions-confirm-dialog/merge-as-versions-confirm-dialog.component' @@ -95,6 +104,7 @@ export class BulkEditorComponent private correspondentService = inject(CorrespondentService) list = inject(DocumentListViewService) private documentService = inject(DocumentService) + private trashService = inject(TrashService) private modalService = inject(NgbModal) private openDocumentService = inject(OpenDocumentsService) private settings = inject(SettingsService) @@ -897,13 +907,23 @@ export class BulkEditorComponent } private deleteSelected(modal: NgbModalRef) { - this.documentService - .deleteDocuments(this.getSelectionQuery()) - .pipe(first()) - .subscribe({ - next: () => this.handleOperationQueued(modal), - error: (error) => this.handleOperationError(modal, error), - }) + const documents = this.list.allSelected + ? null + : Array.from(this.list.selected) + const count = this.getSelectionSize() + this.executeDocumentAction( + modal, + this.documentService + .deleteDocuments(this.getSelectionQuery()) + .pipe( + tap(() => + this.trashService.notifyMovedToTrash( + $localize`Moved ${count} document(s) to the trash`, + documents + ) + ) + ) + ) } downloadSelected() { diff --git a/src-ui/src/app/services/rest/document.service.spec.ts b/src-ui/src/app/services/rest/document.service.spec.ts index ec6ac248ff..eaa5d60f49 100644 --- a/src-ui/src/app/services/rest/document.service.spec.ts +++ b/src-ui/src/app/services/rest/document.service.spec.ts @@ -289,7 +289,6 @@ describe(`DocumentService`, () => { ) expect(req.request.method).toEqual('POST') expect(req.request.body).toEqual({ - from_webui: true, documents: ids, }) }) diff --git a/src-ui/src/app/services/rest/document.service.ts b/src-ui/src/app/services/rest/document.service.ts index a09109bfbe..ef7a171254 100644 --- a/src-ui/src/app/services/rest/document.service.ts +++ b/src-ui/src/app/services/rest/document.service.ts @@ -350,13 +350,9 @@ export class DocumentService extends AbstractPaperlessService { } deleteDocuments(selection: DocumentSelectionQuery) { - return this.http.post( - this.getResourceUrl(null, 'delete'), - { - ...selection, - from_webui: true, - } - ) + return this.http.post(this.getResourceUrl(null, 'delete'), { + ...selection, + }) } reprocessDocuments( diff --git a/src/documents/bulk_edit.py b/src/documents/bulk_edit.py index c66d87457b..26e857e323 100644 --- a/src/documents/bulk_edit.py +++ b/src/documents/bulk_edit.py @@ -432,7 +432,6 @@ def delete(doc_ids: list[int]) -> Literal["OK"]: "Detected a possible incompatible database column. See https://docs.paperless-ngx.com/troubleshooting/#convert-uuid-field", ) logger.error(f"Error deleting documents: {e!s}") - raise return "OK" diff --git a/src/documents/bulk_operations.py b/src/documents/bulk_operations.py index c6bea29097..eee7f8b077 100644 --- a/src/documents/bulk_operations.py +++ b/src/documents/bulk_operations.py @@ -1,4 +1,4 @@ -"""Queued document metadata edits and deletion.""" +"""Queued document metadata edits.""" from typing import Any @@ -24,7 +24,6 @@ MODIFIED_FIELDS = { "modify_tags": "tags", "modify_custom_fields": "custom_fields", "set_permissions": None, - "delete": None, } QUEUED_OPERATIONS = frozenset(MODIFIED_FIELDS) @@ -90,10 +89,7 @@ def execute_bulk_operation( if settings.AUDIT_LOG_ENABLED and modified_field else {} ) - if operation == "delete": - method(document_ids, **parameters) - else: - method(document_ids, **parameters, update_callback=affected_ids.extend) + method(document_ids, **parameters, update_callback=affected_ids.extend) if settings.AUDIT_LOG_ENABLED and modified_field: from auditlog.models import LogEntry diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 94743517b0..daef5b251a 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -1814,7 +1814,7 @@ class RemovePasswordDocumentsSerializer( class DeleteDocumentsSerializer(DocumentSelectionSerializer): - from_webui = serializers.BooleanField(required=False, default=False) + pass class ReprocessDocumentsSerializer(DocumentSelectionSerializer): diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 805b3886b9..407da76a68 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -1197,11 +1197,6 @@ def before_task_publish_handler( _, task_kwargs, _ = body task_id = headers["id"] - if ( - task_name == "documents.bulk_operations.execute_bulk_operation" - and task_kwargs.get("operation") == "delete" - ): - task_type = PaperlessTask.TaskType.BULK_DELETE input_data = _extract_input_data(task_type, task_kwargs) trigger_source = _determine_trigger_source(headers) owner_id = _extract_owner_id(task_type, task_kwargs) diff --git a/src/documents/tests/test_bulk_edit.py b/src/documents/tests/test_bulk_edit.py index 8e7d2e1546..b341f57444 100644 --- a/src/documents/tests/test_bulk_edit.py +++ b/src/documents/tests/test_bulk_edit.py @@ -698,10 +698,10 @@ class TestBulkEdit(DirectoriesMixin, TestCase): def test_delete_documents_old_uuid_field(self, m) -> None: m.side_effect = Exception("Data too long for column 'transaction_id' at row 1") doc_ids = [self.doc1.id, self.doc2.id, self.doc3.id] + bulk_edit.delete(doc_ids) with self.assertLogs(level="WARNING") as cm: - with self.assertRaisesRegex(Exception, "Data too long for column"): - bulk_edit.delete(doc_ids) - self.assertIn("possible incompatible database column", cm.output[0]) + bulk_edit.delete(doc_ids) + self.assertIn("possible incompatible database column", cm.output[0]) class TestPDFActions(DirectoriesMixin, TestCase): diff --git a/src/documents/tests/test_bulk_operations.py b/src/documents/tests/test_bulk_operations.py index d9cff036bf..07f78f6030 100644 --- a/src/documents/tests/test_bulk_operations.py +++ b/src/documents/tests/test_bulk_operations.py @@ -34,7 +34,6 @@ def bulk_setup(paperless_dirs): "endpoint,operation,parameters", [ ("bulk_edit", "set_correspondent", {"correspondent": None}), - ("delete", "delete", {}), ], ) def test_request_only_queues(bulk_setup, endpoint, operation, parameters): @@ -135,13 +134,17 @@ def test_tracked_outcome_records_initiator_and_result(bulk_setup, failed): }, body=( (), - {"operation": "delete", "document_ids": [doc.pk], "user_id": user.pk}, + { + "operation": "set_permissions", + "document_ids": [doc.pk], + "user_id": user.pk, + }, {}, ), ) task = PaperlessTask.objects.get(task_id=task_id) assert task.owner_id == user.pk - assert task.task_type == PaperlessTask.TaskType.BULK_DELETE + assert task.task_type == PaperlessTask.TaskType.BULK_UPDATE if failed: task_failure_handler(task_id=task_id, exception=ValueError("Could not update")) else: @@ -196,13 +199,17 @@ def test_tracked_outcome_notifies_only_initiator(bulk_setup, failed): }, body=( (), - {"operation": "delete", "document_ids": [doc.pk], "user_id": user.pk}, + { + "operation": "set_permissions", + "document_ids": [doc.pk], + "user_id": user.pk, + }, {}, ), ) task = PaperlessTask.objects.get(task_id=task_id) assert task.owner_id == user.pk - assert task.task_type == PaperlessTask.TaskType.BULK_DELETE + assert task.task_type == PaperlessTask.TaskType.BULK_UPDATE with mock.patch("documents.signals.handlers.async_to_sync") as send: if failed: task_failure_handler( @@ -276,3 +283,27 @@ async def test_completion_socket_is_private(anyio_backend): finally: await own.disconnect() await other.disconnect() + + +@pytest.mark.django_db +@pytest.mark.parametrize("endpoint", ["delete", "bulk_edit"]) +def test_trash_remains_synchronous(bulk_setup, endpoint): + _, client, doc, _ = bulk_setup + data = {"documents": [doc.pk]} + if endpoint == "bulk_edit": + data.update(method="delete", parameters={}) + with ( + mock.patch("documents.views.execute_bulk_operation.apply_async") as enqueue, + mock.patch("documents.search.get_backend"), + ): + response = client.post( + f"/api/documents/{endpoint}/", + data, + format="json", + HTTP_ACCEPT="application/json; version=9", + ) + assert response.status_code == 200 + assert response.data == {"result": "OK"} + assert not Document.objects.filter(pk=doc.pk).exists() + assert Document.deleted_objects.filter(pk=doc.pk).exists() + enqueue.assert_not_called() diff --git a/src/documents/views.py b/src/documents/views.py index 32012c2ee3..a614608680 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -3031,13 +3031,6 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin): return HttpResponseForbidden("Insufficient permissions") try: - if method.__name__ in QUEUED_OPERATIONS: - return self._queue_bulk_operation( - method, - documents, - parameters, - from_webui, - ) result = method(documents, **parameters) return Response({"result": result}) except Exception as e: @@ -3265,7 +3258,6 @@ class MergeDocumentsAsVersionsView(DocumentOperationPermissionMixin): name="DeleteDocumentsResult", fields={ "result": serializers.CharField(), - "task_id": serializers.CharField(required=False), }, ), },