From 62b470f691d632fb67ccd9bb805b9bd8a1bd998d Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Thu, 7 Nov 2024 17:31:28 -0800 Subject: [PATCH] Retry action, basic frontend, cleanup handler --- src-ui/messages.xlf | 55 +++++++++++++------ .../admin/tasks/tasks.component.html | 5 ++ .../admin/tasks/tasks.component.spec.ts | 20 +++++++ .../components/admin/tasks/tasks.component.ts | 16 +++++- src-ui/src/app/services/tasks.service.spec.ts | 25 +++++++++ src-ui/src/app/services/tasks.service.ts | 18 +++++- src/documents/signals/handlers.py | 14 +++++ src/documents/tasks.py | 10 ++-- src/documents/tests/test_tasks.py | 30 +++++----- src/documents/views.py | 13 +++++ 10 files changed, 168 insertions(+), 38 deletions(-) diff --git a/src-ui/messages.xlf b/src-ui/messages.xlf index fe002792b..fda84f41e 100644 --- a/src-ui/messages.xlf +++ b/src-ui/messages.xlf @@ -1994,120 +1994,141 @@ 72 + + Retry + + src/app/components/admin/tasks/tasks.component.html + 86 + + Dismiss src/app/components/admin/tasks/tasks.component.html - 85 + 90 src/app/components/admin/tasks/tasks.component.ts - 68 + 71 Open Document src/app/components/admin/tasks/tasks.component.html - 90 + 95 {VAR_PLURAL, plural, =1 {One task} other { total tasks}} src/app/components/admin/tasks/tasks.component.html - 109 + 114  ( selected) src/app/components/admin/tasks/tasks.component.html - 111 + 116 Failed src/app/components/admin/tasks/tasks.component.html - 123,125 + 128,130 Complete src/app/components/admin/tasks/tasks.component.html - 131,133 + 136,138 Started src/app/components/admin/tasks/tasks.component.html - 139,141 + 144,146 Queued src/app/components/admin/tasks/tasks.component.html - 147,149 + 152,154 Dismiss selected src/app/components/admin/tasks/tasks.component.ts - 31 + 33 Dismiss all src/app/components/admin/tasks/tasks.component.ts - 32 + 34 Confirm Dismiss All src/app/components/admin/tasks/tasks.component.ts - 65 + 68 Dismiss all tasks? src/app/components/admin/tasks/tasks.component.ts - 66 + 69 + + + + Retrying task... + + src/app/components/admin/tasks/tasks.component.ts + 92 + + + + Failed to retry task + + src/app/components/admin/tasks/tasks.component.ts + 95 queued src/app/components/admin/tasks/tasks.component.ts - 135 + 149 started src/app/components/admin/tasks/tasks.component.ts - 137 + 151 completed src/app/components/admin/tasks/tasks.component.ts - 139 + 153 failed src/app/components/admin/tasks/tasks.component.ts - 141 + 155 diff --git a/src-ui/src/app/components/admin/tasks/tasks.component.html b/src-ui/src/app/components/admin/tasks/tasks.component.html index 4178bb2c8..39f3aebbf 100644 --- a/src-ui/src/app/components/admin/tasks/tasks.component.html +++ b/src-ui/src/app/components/admin/tasks/tasks.component.html @@ -81,6 +81,11 @@
+ @if (task.status === PaperlessTaskStatus.Failed) { + + } diff --git a/src-ui/src/app/components/admin/tasks/tasks.component.spec.ts b/src-ui/src/app/components/admin/tasks/tasks.component.spec.ts index 1ad2d5311..435e95ac6 100644 --- a/src-ui/src/app/components/admin/tasks/tasks.component.spec.ts +++ b/src-ui/src/app/components/admin/tasks/tasks.component.spec.ts @@ -31,6 +31,8 @@ import { PermissionsGuard } from 'src/app/guards/permissions.guard' import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons' import { FormsModule } from '@angular/forms' import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http' +import { ToastService } from 'src/app/services/toast.service' +import { of, throwError } from 'rxjs' const tasks: PaperlessTask[] = [ { @@ -115,6 +117,7 @@ describe('TasksComponent', () => { let modalService: NgbModal let router: Router let httpTestingController: HttpTestingController + let toastService: ToastService let reloadSpy beforeEach(async () => { @@ -152,6 +155,7 @@ describe('TasksComponent', () => { httpTestingController = TestBed.inject(HttpTestingController) modalService = TestBed.inject(NgbModal) router = TestBed.inject(Router) + toastService = TestBed.inject(ToastService) fixture = TestBed.createComponent(TasksComponent) component = fixture.componentInstance jest.useFakeTimers() @@ -289,4 +293,20 @@ describe('TasksComponent', () => { jest.advanceTimersByTime(6000) expect(reloadSpy).toHaveBeenCalledTimes(2) }) + + it('should retry a task, show toast on error or success', () => { + const retrySpy = jest.spyOn(tasksService, 'retryTask') + const toastInfoSpy = jest.spyOn(toastService, 'showInfo') + const toastErrorSpy = jest.spyOn(toastService, 'showError') + retrySpy.mockReturnValueOnce(of({ task_id: '123' })) + component.retryTask(tasks[0]) + expect(retrySpy).toHaveBeenCalledWith(tasks[0]) + expect(toastInfoSpy).toHaveBeenCalledWith('Retrying task...') + retrySpy.mockReturnValueOnce(throwError(() => new Error('test'))) + component.retryTask(tasks[0]) + expect(toastErrorSpy).toHaveBeenCalledWith( + 'Failed to retry task', + new Error('test') + ) + }) }) diff --git a/src-ui/src/app/components/admin/tasks/tasks.component.ts b/src-ui/src/app/components/admin/tasks/tasks.component.ts index 7b01090d5..854c13142 100644 --- a/src-ui/src/app/components/admin/tasks/tasks.component.ts +++ b/src-ui/src/app/components/admin/tasks/tasks.component.ts @@ -2,10 +2,11 @@ import { Component, OnInit, OnDestroy } from '@angular/core' import { Router } from '@angular/router' import { NgbModal } from '@ng-bootstrap/ng-bootstrap' import { first } from 'rxjs' -import { PaperlessTask } from 'src/app/data/paperless-task' +import { PaperlessTask, PaperlessTaskStatus } from 'src/app/data/paperless-task' import { TasksService } from 'src/app/services/tasks.service' import { ConfirmDialogComponent } from '../../common/confirm-dialog/confirm-dialog.component' import { ComponentWithPermissions } from '../../with-permissions/with-permissions.component' +import { ToastService } from 'src/app/services/toast.service' @Component({ selector: 'pngx-tasks', @@ -16,6 +17,7 @@ export class TasksComponent extends ComponentWithPermissions implements OnInit, OnDestroy { + public PaperlessTaskStatus = PaperlessTaskStatus public activeTab: string public selectedTasks: Set = new Set() public togggleAll: boolean = false @@ -35,6 +37,7 @@ export class TasksComponent constructor( public tasksService: TasksService, private modalService: NgbModal, + private toastService: ToastService, private readonly router: Router ) { super() @@ -83,6 +86,17 @@ export class TasksComponent this.router.navigate(['documents', task.related_document]) } + retryTask(task: PaperlessTask) { + this.tasksService.retryTask(task).subscribe({ + next: () => { + this.toastService.showInfo($localize`Retrying task...`) + }, + error: (e) => { + this.toastService.showError($localize`Failed to retry task`, e) + }, + }) + } + expandTask(task: PaperlessTask) { this.expandedTask = this.expandedTask == task.id ? undefined : task.id } diff --git a/src-ui/src/app/services/tasks.service.spec.ts b/src-ui/src/app/services/tasks.service.spec.ts index 41a374831..60492bde0 100644 --- a/src-ui/src/app/services/tasks.service.spec.ts +++ b/src-ui/src/app/services/tasks.service.spec.ts @@ -118,4 +118,29 @@ describe('TasksService', () => { expect(tasksService.queuedFileTasks).toHaveLength(1) expect(tasksService.startedFileTasks).toHaveLength(1) }) + + it('should call retry task api endpoint', () => { + const task = { + id: 1, + type: PaperlessTaskType.File, + status: PaperlessTaskStatus.Failed, + acknowledged: false, + task_id: '1234', + task_file_name: 'file1.pdf', + date_created: new Date(), + } + + tasksService.retryTask(task).subscribe() + const reloadSpy = jest.spyOn(tasksService, 'reload') + const req = httpTestingController.expectOne( + `${environment.apiBaseUrl}tasks/${task.id}/retry/` + ) + expect(req.request.method).toEqual('POST') + expect(req.request.body).toEqual({ + task_id: task.id, + }) + req.flush({ task_id: 12345 }) + expect(reloadSpy).toHaveBeenCalled() + httpTestingController.expectOne(`${environment.apiBaseUrl}tasks/`).flush([]) + }) }) diff --git a/src-ui/src/app/services/tasks.service.ts b/src-ui/src/app/services/tasks.service.ts index e2c064e03..52c8b1a3e 100644 --- a/src-ui/src/app/services/tasks.service.ts +++ b/src-ui/src/app/services/tasks.service.ts @@ -1,7 +1,7 @@ import { HttpClient } from '@angular/common/http' import { Injectable } from '@angular/core' -import { Subject } from 'rxjs' -import { first, takeUntil } from 'rxjs/operators' +import { Observable, Subject } from 'rxjs' +import { first, takeUntil, tap } from 'rxjs/operators' import { PaperlessTask, PaperlessTaskStatus, @@ -73,6 +73,20 @@ export class TasksService { }) } + public retryTask(task: PaperlessTask): Observable { + return this.http + .post(`${this.baseUrl}tasks/${task.id}/retry/`, { + task_id: task.id, + }) + .pipe( + takeUntil(this.unsubscribeNotifer), + first(), + tap(() => { + this.reload() + }) + ) + } + public cancelPending(): void { this.unsubscribeNotifer.next(true) } diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 73aee2936..5cd1bae0b 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -1,6 +1,7 @@ import logging import os import shutil +from pathlib import Path from celery import states from celery.signals import before_task_publish @@ -520,6 +521,19 @@ def update_filename_and_move_files( ) +@receiver(models.signals.post_save, sender=PaperlessTask) +def cleanup_failed_documents(sender, instance: PaperlessTask, **kwargs): + if instance.status != states.FAILURE or not instance.acknowledged: + return + + if instance.task_file_name: + try: + Path(settings.CONSUMPTION_FAILED_DIR / instance.task_file_name).unlink() + logger.debug(f"Cleaned up failed file {instance.task_file_name}") + except FileNotFoundError: + logger.warning(f"Failed to clean up failed file {instance.task_file_name}") + + def set_log_entry(sender, document: Document, logging_group=None, **kwargs): ct = ContentType.objects.get(model="document") user = User.objects.get(username="consumer") diff --git a/src/documents/tasks.py b/src/documents/tasks.py index c2b7194e2..22bba619c 100644 --- a/src/documents/tasks.py +++ b/src/documents/tasks.py @@ -180,8 +180,8 @@ def retry_failed_file(task_id: str, clean: bool = False, skip_ocr: bool = False) if task: failed_file = settings.CONSUMPTION_FAILED_DIR / task.task_file_name if not failed_file.exists(): - logger.error(f"Failed file {failed_file} not found") - return + logger.error(f"File {failed_file} not found") + raise FileNotFoundError(f"File {failed_file} not found") working_copy = settings.SCRATCH_DIR / failed_file.name copy_file_with_basic_stats(failed_file, working_copy) @@ -204,15 +204,17 @@ def retry_failed_file(task_id: str, clean: bool = False, skip_ocr: bool = False) logger.debug("PDF cleaned successfully") except Exception as e: logger.error(f"Error while cleaning PDF: {e}") - return + raise e - consume_file( + task = consume_file.delay( ConsumableDocument( source=DocumentSource.ConsumeFolder, original_file=working_copy, ), ) + return task.id + @shared_task def sanity_check(): diff --git a/src/documents/tests/test_tasks.py b/src/documents/tests/test_tasks.py index 843dd38b4..aa4885179 100644 --- a/src/documents/tests/test_tasks.py +++ b/src/documents/tests/test_tasks.py @@ -7,7 +7,6 @@ from unittest import mock from django.conf import settings from django.test import TestCase -from django.test import override_settings from django.utils import timezone from documents import tasks @@ -203,9 +202,7 @@ class TestRetryConsumeTask( FileSystemAssertsMixin, TestCase, ): - @override_settings(CONSUMPTION_FAILED_DIR=Path(__file__).parent / "samples") - def test_retry_consume_clean(self): - test_file = self.SAMPLE_DIR / "corrupted.pdf" + def do_failed_task(self, test_file: Path) -> PaperlessTask: temp_copy = self.dirs.scratch_dir / test_file.name shutil.copy(test_file, temp_copy) @@ -246,14 +243,19 @@ class TestRetryConsumeTask( task = PaperlessTask.objects.first() # Ensure the file is moved to the failed dir self.assertIsFile(settings.CONSUMPTION_FAILED_DIR / task.task_file_name) + return task - with mock.patch("documents.tasks.ProgressManager", DummyProgressManager): - with self.assertLogs() as cm: - tasks.retry_failed_file(task_id=task.task_id, clean=True) - # on ci, the message is different because qpdf is not installed - msg = ( - "No such file or directory: 'qpdf'" - if "PAPERLESS_CI_TEST" in os.environ - else "New document id 1 created" - ) - self.assertIn(msg, cm.output[-1]) + @mock.patch("documents.tasks.consume_file.delay") + @mock.patch("documents.tasks.run_subprocess") + def test_retry_consume_clean(self, m_subprocess, m_consume_file): + task = self.do_failed_task(self.SAMPLE_DIR / "corrupted.pdf") + m_subprocess.return_value.returncode = 0 + task_id = tasks.retry_failed_file(task_id=task.task_id, clean=True) + self.assertIsNotNone(task_id) + m_consume_file.assert_called_once() + + def test_cleanup(self): + task = self.do_failed_task(self.SAMPLE_DIR / "corrupted.pdf") + task.acknowledged = True + task.save() # simulate the task being acknowledged + self.assertIsNotFile(settings.CONSUMPTION_FAILED_DIR / task.task_file_name) diff --git a/src/documents/views.py b/src/documents/views.py index 10b2d0cbd..1f0b3ad98 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -152,6 +152,7 @@ from documents.serialisers import WorkflowTriggerSerializer from documents.signals import document_updated from documents.tasks import consume_file from documents.tasks import empty_trash +from documents.tasks import retry_failed_file from documents.templating.filepath import validate_filepath_template_and_render from paperless import version from paperless.celery import app as celery_app @@ -1718,6 +1719,18 @@ class TasksViewSet(ReadOnlyModelViewSet): queryset = PaperlessTask.objects.filter(task_id=task_id) return queryset + @action(methods=["post"], detail=True) + def retry(self, request, pk=None): + task = self.get_object() + try: + new_task_id = retry_failed_file(task.task_id, True) + return Response({"task_id": new_task_id}) + except Exception as e: + logger.warning(f"An error occurred retrying task: {e!s}") + return HttpResponseBadRequest( + "Error retrying task, check logs for more detail.", + ) + class AcknowledgeTasksView(GenericAPIView): permission_classes = (IsAuthenticated,)