diff --git a/src-ui/src/app/components/admin/trash/trash.component.spec.ts b/src-ui/src/app/components/admin/trash/trash.component.spec.ts index 10e60e3e2..2f73c071a 100644 --- a/src-ui/src/app/components/admin/trash/trash.component.spec.ts +++ b/src-ui/src/app/components/admin/trash/trash.component.spec.ts @@ -1,22 +1,157 @@ import { ComponentFixture, TestBed } from '@angular/core/testing' import { TrashComponent } from './trash.component' +import { HttpClientTestingModule } from '@angular/common/http/testing' +import { PageHeaderComponent } from '../../common/page-header/page-header.component' +import { + NgbModal, + NgbPaginationModule, + NgbPopoverModule, +} from '@ng-bootstrap/ng-bootstrap' +import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons' +import { FormsModule, ReactiveFormsModule } from '@angular/forms' +import { TrashService } from 'src/app/services/trash.service' +import { of } from 'rxjs' +import { ConfirmDialogComponent } from '../../common/confirm-dialog/confirm-dialog.component' +import { By } from '@angular/platform-browser' + +const documentsInTrash = [ + { + id: 1, + name: 'test1', + created: new Date('2023-03-01T10:26:03.093116Z'), + deleted_at: new Date('2023-03-01T10:26:03.093116Z'), + }, + { + id: 2, + name: 'test2', + created: new Date('2023-03-01T10:26:03.093116Z'), + deleted_at: new Date('2023-03-01T10:26:03.093116Z'), + }, +] describe('TrashComponent', () => { let component: TrashComponent let fixture: ComponentFixture + let trashService: TrashService + let modalService: NgbModal beforeEach(async () => { await TestBed.configureTestingModule({ - declarations: [TrashComponent], + declarations: [ + TrashComponent, + PageHeaderComponent, + ConfirmDialogComponent, + ], + imports: [ + HttpClientTestingModule, + FormsModule, + ReactiveFormsModule, + NgbPopoverModule, + NgbPaginationModule, + NgxBootstrapIconsModule.pick(allIcons), + ], }).compileComponents() fixture = TestBed.createComponent(TrashComponent) + trashService = TestBed.inject(TrashService) + modalService = TestBed.inject(NgbModal) component = fixture.componentInstance fixture.detectChanges() }) - it('should create', () => { - expect(component).toBeTruthy() + it('should call correct service method on reload', () => { + const trashSpy = jest.spyOn(trashService, 'getTrash') + trashSpy.mockReturnValue(of(documentsInTrash)) + component.reload() + expect(trashSpy).toHaveBeenCalled() + expect(component.documentsInTrash).toEqual(documentsInTrash) + }) + + it('should support delete document', () => { + const trashSpy = jest.spyOn(trashService, 'emptyTrash') + let modal + modalService.activeInstances.subscribe((instances) => { + modal = instances[0] + }) + trashSpy.mockReturnValue(of('OK')) + component.delete(documentsInTrash[0]) + expect(modal).toBeDefined() + modal.componentInstance.confirmClicked.next() + expect(trashSpy).toHaveBeenCalled() + }) + + it('should support empty trash', () => { + const trashSpy = jest.spyOn(trashService, 'emptyTrash') + let modal + modalService.activeInstances.subscribe((instances) => { + modal = instances[instances.length - 1] + }) + trashSpy.mockReturnValue(of('OK')) + component.emptyTrash() + expect(modal).toBeDefined() + modal.componentInstance.confirmClicked.next() + expect(trashSpy).toHaveBeenCalled() + modal.close() + component.emptyTrash(new Set([1, 2])) + modal.componentInstance.confirmClicked.next() + expect(trashSpy).toHaveBeenCalledWith([1, 2]) + }) + + it('should support restore document', () => { + const restoreSpy = jest.spyOn(trashService, 'restoreDocuments') + const reloadSpy = jest.spyOn(component, 'reload') + restoreSpy.mockReturnValue(of('OK')) + component.restore(documentsInTrash[0]) + expect(restoreSpy).toHaveBeenCalledWith([documentsInTrash[0].id]) + expect(reloadSpy).toHaveBeenCalled() + }) + + it('should support restore all documents', () => { + const restoreSpy = jest.spyOn(trashService, 'restoreDocuments') + const reloadSpy = jest.spyOn(component, 'reload') + restoreSpy.mockReturnValue(of('OK')) + component.restoreAll() + expect(restoreSpy).toHaveBeenCalled() + expect(reloadSpy).toHaveBeenCalled() + component.restoreAll(new Set([1, 2])) + expect(restoreSpy).toHaveBeenCalledWith([1, 2]) + }) + + it('should support toggle all items in view', () => { + component.documentsInTrash = documentsInTrash + expect(component.selectedDocuments.size).toEqual(0) + const toggleAllSpy = jest.spyOn(component, 'toggleAll') + const checkButton = fixture.debugElement.queryAll( + By.css('input.form-check-input') + )[0] + checkButton.nativeElement.dispatchEvent(new Event('click')) + checkButton.nativeElement.checked = true + checkButton.nativeElement.dispatchEvent(new Event('click')) + expect(toggleAllSpy).toHaveBeenCalled() + expect(component.selectedDocuments.size).toEqual(documentsInTrash.length) + }) + + it('should support toggle item', () => { + component.selectedDocuments = new Set([1]) + component.toggleSelected(documentsInTrash[0]) + expect(component.selectedDocuments.size).toEqual(0) + component.toggleSelected(documentsInTrash[0]) + expect(component.selectedDocuments.size).toEqual(1) + }) + + it('should support clear selection', () => { + component.selectedDocuments = new Set([1]) + component.clearSelection() + expect(component.selectedDocuments.size).toEqual(0) + }) + + it('should correctly display days remaining', () => { + expect(component.getDaysRemaining(documentsInTrash[0])).toBeLessThan(0) + const tenDaysAgo = new Date() + tenDaysAgo.setDate(tenDaysAgo.getDate() - 10) + expect( + component.getDaysRemaining({ deleted_at: tenDaysAgo }) + ).toBeGreaterThan(0) // 10 days ago but depends on month }) }) diff --git a/src-ui/src/app/components/admin/trash/trash.component.ts b/src-ui/src/app/components/admin/trash/trash.component.ts index f5349434e..a56445d4b 100644 --- a/src-ui/src/app/components/admin/trash/trash.component.ts +++ b/src-ui/src/app/components/admin/trash/trash.component.ts @@ -79,7 +79,7 @@ export class TrashComponent implements OnDestroy { .pipe(takeUntil(this.unsubscribeNotifier)) .subscribe(() => { this.trashService - .emptyTrash(documents ? Array.from(documents) : []) + .emptyTrash(documents ? Array.from(documents) : null) .subscribe(() => { this.toastService.showInfo($localize`Document(s) deleted`) this.allToggled = false @@ -95,9 +95,9 @@ export class TrashComponent implements OnDestroy { }) } - restoreAll(objects: Set = null) { + restoreAll(documents: Set = null) { this.trashService - .restoreDocuments(objects ? Array.from(this.selectedDocuments) : []) + .restoreDocuments(documents ? Array.from(documents) : null) .subscribe(() => { this.toastService.showInfo($localize`Document(s) restored`) this.allToggled = false diff --git a/src-ui/src/app/services/trash.service.spec.ts b/src-ui/src/app/services/trash.service.spec.ts index 21813c785..8eeb76a76 100644 --- a/src-ui/src/app/services/trash.service.spec.ts +++ b/src-ui/src/app/services/trash.service.spec.ts @@ -1,16 +1,50 @@ import { TestBed } from '@angular/core/testing' import { TrashService } from './trash.service' +import { + HttpClientTestingModule, + HttpTestingController, +} from '@angular/common/http/testing' +import { environment } from 'src/environments/environment' describe('TrashServiceService', () => { let service: TrashService + let httpTestingController: HttpTestingController beforeEach(() => { - TestBed.configureTestingModule({}) + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + }) service = TestBed.inject(TrashService) + httpTestingController = TestBed.inject(HttpTestingController) }) - it('should be created', () => { - expect(service).toBeTruthy() + it('should call correct endpoint for getTrash', () => { + service.getTrash().subscribe() + const req = httpTestingController.expectOne( + `${environment.apiBaseUrl}trash/` + ) + expect(req.request.method).toEqual('GET') + }) + + it('should call correct endpoint for emptyTrash', () => { + service.emptyTrash().subscribe() + const req = httpTestingController.expectOne( + `${environment.apiBaseUrl}trash/` + ) + expect(req.request.method).toEqual('POST') + expect(req.request.body).toEqual({ action: 'empty', documents: [] }) + }) + + it('should call correct endpoint for restoreDocuments', () => { + service.restoreDocuments([1, 2, 3]).subscribe() + const req = httpTestingController.expectOne( + `${environment.apiBaseUrl}trash/` + ) + expect(req.request.method).toEqual('POST') + expect(req.request.body).toEqual({ + action: 'restore', + documents: [1, 2, 3], + }) }) }) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 42361cc20..ea4f17d53 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -1868,7 +1868,7 @@ class WorkflowSerializer(serializers.ModelSerializer): class TrashSerializer(SerializerWithPerms): documents = serializers.ListField( - required=True, + required=False, label="Documents", write_only=True, child=serializers.IntegerField(), diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 236a1705a..4bde27427 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -1,7 +1,6 @@ import logging import os import shutil -from datetime import timedelta from typing import Optional from celery import states @@ -302,12 +301,8 @@ def set_storage_path( document.save(update_fields=("storage_path",)) -@receiver(models.signals.post_delete, sender=Document) -def cleanup_document_deletion(sender, instance, force=False, **kwargs): - if not force: - now = timezone.localtime(timezone.now()) - if now - instance.deleted_at < timedelta(days=settings.EMPTY_TRASH_DELAY): - return +# see empty_trash in documents/tasks.py for signal handling +def cleanup_document_deletion(sender, instance, **kwargs): with FileLock(settings.MEDIA_LOCK): if settings.TRASH_DIR: # Find a non-conflicting filename in case a document with the same diff --git a/src/documents/tasks.py b/src/documents/tasks.py index ead3b7a35..80e81a458 100644 --- a/src/documents/tasks.py +++ b/src/documents/tasks.py @@ -11,8 +11,8 @@ import tqdm from celery import Task from celery import shared_task from django.conf import settings +from django.db import models from django.db import transaction -from django.db.models.signals import post_delete from django.db.models.signals import post_save from django.utils import timezone from filelock import FileLock @@ -44,6 +44,7 @@ from documents.plugins.base import StopConsumeTaskError from documents.plugins.helpers import ProgressStatusOptions from documents.sanity_checker import SanityCheckFailedException from documents.signals import document_updated +from documents.signals.handlers import cleanup_document_deletion if settings.AUDIT_LOG_ENABLED: import json @@ -307,10 +308,11 @@ def empty_trash(doc_ids=None): if doc_ids is not None else Document.deleted_objects.filter(deleted_at__gt=cutoff) ) + + # Temporarily connect the cleanup handler (hard_delete calls delete) + models.signals.post_delete.connect(cleanup_document_deletion, sender=Document) + for doc in documents: - doc.delete() - post_delete.send( - sender=Document, - instance=doc, - force=True, - ) + doc.hard_delete() + + models.signals.post_delete.disconnect(cleanup_document_deletion, sender=Document) diff --git a/src/documents/tests/test_api_uisettings.py b/src/documents/tests/test_api_uisettings.py index 0a52ea41c..65dee7d6d 100644 --- a/src/documents/tests/test_api_uisettings.py +++ b/src/documents/tests/test_api_uisettings.py @@ -40,6 +40,7 @@ class TestApiUiSettings(DirectoriesMixin, APITestCase): "app_title": None, "app_logo": None, "auditlog_enabled": True, + "trash_delay": 30, "update_checking": { "backend_setting": "default", }, diff --git a/src/documents/tests/test_document_model.py b/src/documents/tests/test_document_model.py index 68539e5dd..eca08f82a 100644 --- a/src/documents/tests/test_document_model.py +++ b/src/documents/tests/test_document_model.py @@ -10,6 +10,7 @@ from django.utils import timezone from documents.models import Correspondent from documents.models import Document +from documents.tasks import empty_trash class TestDocument(TestCase): @@ -43,10 +44,39 @@ class TestDocument(TestCase): with mock.patch("documents.signals.handlers.os.unlink") as mock_unlink: document.delete() + empty_trash([document.pk]) mock_unlink.assert_any_call(file_path) mock_unlink.assert_any_call(thumb_path) self.assertEqual(mock_unlink.call_count, 2) + def test_document_soft_delete(self): + document = Document.objects.create( + correspondent=Correspondent.objects.create(name="Test0"), + title="Title", + content="content", + checksum="checksum", + mime_type="application/pdf", + ) + + file_path = document.source_path + thumb_path = document.thumbnail_path + + Path(file_path).touch() + Path(thumb_path).touch() + + with mock.patch("documents.signals.handlers.os.unlink") as mock_unlink: + document.delete() + self.assertEqual(mock_unlink.call_count, 0) + + self.assertEqual(Document.objects.count(), 0) + + document.restore(strict=False) + self.assertEqual(Document.objects.count(), 1) + + document.delete() + empty_trash([document.pk]) + self.assertEqual(mock_unlink.call_count, 2) + def test_file_name(self): doc = Document( mime_type="application/pdf", diff --git a/src/documents/tests/test_file_handling.py b/src/documents/tests/test_file_handling.py index 2f085f2ce..44c4b1167 100644 --- a/src/documents/tests/test_file_handling.py +++ b/src/documents/tests/test_file_handling.py @@ -19,6 +19,7 @@ from documents.models import Correspondent from documents.models import Document from documents.models import DocumentType from documents.models import StoragePath +from documents.tasks import empty_trash from documents.tests.utils import DirectoriesMixin from documents.tests.utils import FileSystemAssertsMixin @@ -169,6 +170,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): # Ensure that filename is properly generated document.filename = generate_filename(document) + document.save() self.assertEqual(document.filename, "none/none.pdf") create_source_path_directory(document.source_path) @@ -176,6 +178,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): # Ensure file deletion after delete document.delete() + empty_trash([document.pk]) self.assertIsNotFile( os.path.join(settings.ORIGINALS_DIR, "none", "none.pdf"), ) @@ -185,7 +188,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): FILENAME_FORMAT="{correspondent}/{correspondent}", TRASH_DIR=tempfile.mkdtemp(), ) - def test_document_delete_trash(self): + def test_document_delete_trash_dir(self): document = Document() document.mime_type = "application/pdf" document.storage_type = Document.STORAGE_TYPE_UNENCRYPTED @@ -193,6 +196,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): # Ensure that filename is properly generated document.filename = generate_filename(document) + document.save() self.assertEqual(document.filename, "none/none.pdf") create_source_path_directory(document.source_path) @@ -201,6 +205,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): # Ensure file was moved to trash after delete self.assertIsNotFile(os.path.join(settings.TRASH_DIR, "none", "none.pdf")) document.delete() + empty_trash([document.pk]) self.assertIsNotFile( os.path.join(settings.ORIGINALS_DIR, "none", "none.pdf"), ) @@ -214,9 +219,11 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): document.storage_type = Document.STORAGE_TYPE_UNENCRYPTED document.save() document.filename = generate_filename(document) + document.save() create_source_path_directory(document.source_path) Path(document.source_path).touch() document.delete() + empty_trash([document.pk]) self.assertIsFile(os.path.join(settings.TRASH_DIR, "none_01.pdf")) @override_settings(FILENAME_FORMAT="{correspondent}/{correspondent}") @@ -227,6 +234,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): document.save() document.delete() + empty_trash([document.pk]) @override_settings(FILENAME_FORMAT="{correspondent}/{correspondent}") def test_directory_not_empty(self): @@ -436,6 +444,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): # Ensure that filename is properly generated document.filename = generate_filename(document) + document.save() self.assertEqual(document.filename, "none/none/none.pdf") create_source_path_directory(document.source_path) Path(document.source_path).touch() @@ -444,6 +453,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): self.assertIsDir(os.path.join(settings.ORIGINALS_DIR, "none/none")) document.delete() + empty_trash([document.pk]) self.assertIsNotFile( os.path.join(settings.ORIGINALS_DIR, "none/none/none.pdf"), @@ -550,6 +560,7 @@ class TestFileHandling(DirectoriesMixin, FileSystemAssertsMixin, TestCase): self.assertEqual(document2.filename, "qwe_01.pdf") document.delete() + empty_trash([document.pk]) self.assertIsNotFile(document.source_path) @@ -819,6 +830,7 @@ class TestFileHandlingWithArchive(DirectoriesMixin, FileSystemAssertsMixin, Test self.assertIsFile(doc.archive_path) doc.delete() + empty_trash([doc.pk]) self.assertIsNotFile(original) self.assertIsNotFile(archive) @@ -854,6 +866,7 @@ class TestFileHandlingWithArchive(DirectoriesMixin, FileSystemAssertsMixin, Test self.assertIsFile(doc2.source_path) doc2.delete() + empty_trash([doc2.pk]) self.assertIsFile(doc1.source_path) self.assertIsFile(doc1.archive_path) diff --git a/src/paperless/tests/test_settings.py b/src/paperless/tests/test_settings.py index 0051d40e7..511148523 100644 --- a/src/paperless/tests/test_settings.py +++ b/src/paperless/tests/test_settings.py @@ -156,6 +156,7 @@ class TestCeleryScheduleParsing(TestCase): CLASSIFIER_EXPIRE_TIME = 59.0 * 60.0 INDEX_EXPIRE_TIME = 23.0 * 60.0 * 60.0 SANITY_EXPIRE_TIME = ((7.0 * 24.0) - 1.0) * 60.0 * 60.0 + EMPTY_TRASH_EXPIRE_TIME = 23.0 * 60.0 * 60.0 def test_schedule_configuration_default(self): """ @@ -190,6 +191,11 @@ class TestCeleryScheduleParsing(TestCase): "schedule": crontab(minute=30, hour=0, day_of_week="sun"), "options": {"expires": self.SANITY_EXPIRE_TIME}, }, + "Empty trash": { + "task": "documents.tasks.empty_trash", + "schedule": crontab(minute=0, hour="1"), + "options": {"expires": self.EMPTY_TRASH_EXPIRE_TIME}, + }, }, schedule, ) @@ -232,6 +238,11 @@ class TestCeleryScheduleParsing(TestCase): "schedule": crontab(minute=30, hour=0, day_of_week="sun"), "options": {"expires": self.SANITY_EXPIRE_TIME}, }, + "Empty trash": { + "task": "documents.tasks.empty_trash", + "schedule": crontab(minute=0, hour="1"), + "options": {"expires": self.EMPTY_TRASH_EXPIRE_TIME}, + }, }, schedule, ) @@ -266,6 +277,11 @@ class TestCeleryScheduleParsing(TestCase): "schedule": crontab(minute=30, hour=0, day_of_week="sun"), "options": {"expires": self.SANITY_EXPIRE_TIME}, }, + "Empty trash": { + "task": "documents.tasks.empty_trash", + "schedule": crontab(minute=0, hour="1"), + "options": {"expires": self.EMPTY_TRASH_EXPIRE_TIME}, + }, }, schedule, ) @@ -286,6 +302,7 @@ class TestCeleryScheduleParsing(TestCase): "PAPERLESS_TRAIN_TASK_CRON": "disable", "PAPERLESS_SANITY_TASK_CRON": "disable", "PAPERLESS_INDEX_TASK_CRON": "disable", + "PAPERLESS_EMPTY_TRASH_TASK_CRON": "disable", }, ): schedule = _parse_beat_schedule()