Retry action, basic frontend, cleanup handler

2025-04-02 13:45:10 -05:00 · 2024-11-07 17:31:28 -08:00 · 2024-11-07 17:31:28 -08:00 · 62b470f691
commit 62b470f691
parent a2e4977201
10 changed files with 168 additions and 38 deletions
--- a/src-ui/messages.xlf
+++ b/src-ui/messages.xlf
@ -1994,120 +1994,141 @@
          <context context-type="linenumber">72</context>
        </context-group>
      </trans-unit>
+      <trans-unit id="7934833136974560675" datatype="html">
+        <source>Retry</source>
+        <context-group purpose="location">
+          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.html</context>
+          <context context-type="linenumber">86</context>
+        </context-group>
+      </trans-unit>
      <trans-unit id="1536087519743707362" datatype="html">
        <source>Dismiss</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.html</context>
-          <context context-type="linenumber">85</context>
+          <context context-type="linenumber">90</context>
        </context-group>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
-          <context context-type="linenumber">68</context>
+          <context context-type="linenumber">71</context>
        </context-group>
      </trans-unit>
      <trans-unit id="2134950584701094962" datatype="html">
        <source>Open Document</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.html</context>
-          <context context-type="linenumber">90</context>
+          <context context-type="linenumber">95</context>
        </context-group>
      </trans-unit>
      <trans-unit id="428536141871853903" datatype="html">
        <source>{VAR_PLURAL, plural, =1 {One <x id="INTERPOLATION"/> task} other {<x id="INTERPOLATION_1"/> total <x id="INTERPOLATION"/> tasks}}</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.html</context>
-          <context context-type="linenumber">109</context>
+          <context context-type="linenumber">114</context>
        </context-group>
      </trans-unit>
      <trans-unit id="1943508481059904274" datatype="html">
        <source> (<x id="INTERPOLATION" equiv-text="{{selectedTasks.size}}"/> selected)</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.html</context>
-          <context context-type="linenumber">111</context>
+          <context context-type="linenumber">116</context>
        </context-group>
      </trans-unit>
      <trans-unit id="5639839509673911668" datatype="html">
        <source>Failed<x id="START_BLOCK_IF" equiv-text="@if (tasksService.failedFileTasks.length &gt; 0) {"/><x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span class=&quot;badge bg-danger ms-2&quot;&gt;"/><x id="INTERPOLATION" equiv-text="{{tasksService.failedFileTasks.length}}"/><x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span&gt;"/><x id="CLOSE_BLOCK_IF" equiv-text="}"/></source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.html</context>
-          <context context-type="linenumber">123,125</context>
+          <context context-type="linenumber">128,130</context>
        </context-group>
      </trans-unit>
      <trans-unit id="8210778930307085868" datatype="html">
        <source>Complete<x id="START_BLOCK_IF" equiv-text="@if (tasksService.completedFileTasks.length &gt; 0) {"/><x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span class=&quot;badge bg-secondary ms-2&quot;&gt;"/><x id="INTERPOLATION" equiv-text="{{tasksService.completedFileTasks.length}}"/><x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span&gt;"/><x id="CLOSE_BLOCK_IF" equiv-text="}"/></source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.html</context>
-          <context context-type="linenumber">131,133</context>
+          <context context-type="linenumber">136,138</context>
        </context-group>
      </trans-unit>
      <trans-unit id="3522801015717851360" datatype="html">
        <source>Started<x id="START_BLOCK_IF" equiv-text="@if (tasksService.startedFileTasks.length &gt; 0) {"/><x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span class=&quot;badge bg-secondary ms-2&quot;&gt;"/><x id="INTERPOLATION" equiv-text="{{tasksService.startedFileTasks.length}}"/><x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span&gt;"/><x id="CLOSE_BLOCK_IF" equiv-text="}"/></source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.html</context>
-          <context context-type="linenumber">139,141</context>
+          <context context-type="linenumber">144,146</context>
        </context-group>
      </trans-unit>
      <trans-unit id="2341807459308874922" datatype="html">
        <source>Queued<x id="START_BLOCK_IF" equiv-text="@if (tasksService.queuedFileTasks.length &gt; 0) {"/><x id="START_TAG_SPAN" ctype="x-span" equiv-text="&lt;span class=&quot;badge bg-secondary ms-2&quot;&gt;"/><x id="INTERPOLATION" equiv-text="{{tasksService.queuedFileTasks.length}}"/><x id="CLOSE_TAG_SPAN" ctype="x-span" equiv-text="&lt;/span&gt;"/><x id="CLOSE_BLOCK_IF" equiv-text="}"/></source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.html</context>
-          <context context-type="linenumber">147,149</context>
+          <context context-type="linenumber">152,154</context>
        </context-group>
      </trans-unit>
      <trans-unit id="5404910960991552159" datatype="html">
        <source>Dismiss selected</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
-          <context context-type="linenumber">31</context>
+          <context context-type="linenumber">33</context>
        </context-group>
      </trans-unit>
      <trans-unit id="8829078752502782653" datatype="html">
        <source>Dismiss all</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
-          <context context-type="linenumber">32</context>
+          <context context-type="linenumber">34</context>
        </context-group>
      </trans-unit>
      <trans-unit id="1323591410517879795" datatype="html">
        <source>Confirm Dismiss All</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
-          <context context-type="linenumber">65</context>
+          <context context-type="linenumber">68</context>
        </context-group>
      </trans-unit>
      <trans-unit id="4157200209636243740" datatype="html">
        <source>Dismiss all <x id="PH" equiv-text="tasks.size"/> tasks?</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
-          <context context-type="linenumber">66</context>
+          <context context-type="linenumber">69</context>
+        </context-group>
+      </trans-unit>
+      <trans-unit id="7611027432301841688" datatype="html">
+        <source>Retrying task...</source>
+        <context-group purpose="location">
+          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
+          <context context-type="linenumber">92</context>
+        </context-group>
+      </trans-unit>
+      <trans-unit id="5445438607105804721" datatype="html">
+        <source>Failed to retry task</source>
+        <context-group purpose="location">
+          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
+          <context context-type="linenumber">95</context>
        </context-group>
      </trans-unit>
      <trans-unit id="9011556615675272238" datatype="html">
        <source>queued</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
-          <context context-type="linenumber">135</context>
+          <context context-type="linenumber">149</context>
        </context-group>
      </trans-unit>
      <trans-unit id="6415892379431855826" datatype="html">
        <source>started</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
-          <context context-type="linenumber">137</context>
+          <context context-type="linenumber">151</context>
        </context-group>
      </trans-unit>
      <trans-unit id="7510279840486540181" datatype="html">
        <source>completed</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
-          <context context-type="linenumber">139</context>
+          <context context-type="linenumber">153</context>
        </context-group>
      </trans-unit>
      <trans-unit id="4083337005045748464" datatype="html">
        <source>failed</source>
        <context-group purpose="location">
          <context context-type="sourcefile">src/app/components/admin/tasks/tasks.component.ts</context>
-          <context context-type="linenumber">141</context>
+          <context context-type="linenumber">155</context>
        </context-group>
      </trans-unit>
      <trans-unit id="3418677553313974490" datatype="html">
--- 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 @@
          </td>
          <td scope="row">
            <div class="btn-group" role="group">
+              @if (task.status === PaperlessTaskStatus.Failed) {
+                <button class="btn btn-sm btn-outline-primary" (click)="retryTask(task); $event.stopPropagation();" *pngxIfPermissions="{ action: PermissionAction.Change, type: PermissionType.PaperlessTask }">
+                  <i-bs name="arrow-repeat"></i-bs>&nbsp;<ng-container i18n>Retry</ng-container>
+                </button>
+              }
              <button class="btn btn-sm btn-outline-secondary" (click)="dismissTask(task); $event.stopPropagation();" *pngxIfPermissions="{ action: PermissionAction.Change, type: PermissionType.PaperlessTask }">
                <i-bs name="check"></i-bs>&nbsp;<ng-container i18n>Dismiss</ng-container>
              </button>
--- 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')
+    )
+  })
 })
--- 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<number> = 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
  }
--- 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([])
+  })
 })
--- 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<any> {
+    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)
  }
--- 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")
--- 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():
--- 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)
--- 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,)