Avoid a little redundancy here

.mypy-baseline.txt

@@ -97,7 +97,6 @@ src/documents/conditionals.py:0: error: Function is missing a type annotation fo
 src/documents/conditionals.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
 src/documents/conditionals.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
 src/documents/conditionals.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
-src/documents/conditionals.py:0: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
 src/documents/consumer.py:0: error: "ConsumerPluginMixin" has no attribute "input_doc"  [attr-defined]
 src/documents/consumer.py:0: error: "ConsumerPluginMixin" has no attribute "metadata"  [attr-defined]
 src/documents/consumer.py:0: error: "ConsumerPluginMixin" has no attribute "metadata"  [attr-defined]

docs/api.md

@@ -315,13 +315,13 @@ The following methods are supported:
             -   `"doc": OUTPUT_DOCUMENT_INDEX` Optional index of the output document for split operations.
     -   Optional `parameters`:
         -   `"delete_original": true` to delete the original documents after editing.
-        -   `"update_document": true` to update the existing document with the edited PDF.
+        -   `"update_document": true` to add the edited PDF as a new version of the selected/root document.
         -   `"include_metadata": true` to copy metadata from the original document to the edited document.
 -   `remove_password`
     -   Requires `parameters`:
         -   `"password": "PASSWORD_STRING"` The password to remove from the PDF documents.
     -   Optional `parameters`:
-        -   `"update_document": true` to replace the existing document with the password-less PDF.
+        -   `"update_document": true` to add the password-less PDF as a new version of the selected/root document.
         -   `"delete_original": true` to delete the original document after editing.
         -   `"include_metadata": true` to copy metadata from the original document to the new password-less document.
 -   `merge`

src-ui/src/app/components/document-detail/document-detail.component.html

@@ -24,89 +24,15 @@
     <i-bs width="1.2em" height="1.2em" name="trash"></i-bs><span class="d-none d-lg-inline ps-1" i18n>Delete</span>
   </button>
 
-  @if (document?.versions?.length > 0) {
-    <div class="btn-group" ngbDropdown autoClose="outside">
-      <button class="btn btn-sm btn-outline-secondary dropdown-toggle" ngbDropdownToggle>
-        <i-bs name="file-earmark-diff"></i-bs>
-        <span class="d-none d-lg-inline ps-1" i18n>Versions</span>
-      </button>
-      <div class="dropdown-menu shadow" ngbDropdownMenu>
-        <div class="px-3 py-2">
-          @if (versionUploadState === UploadState.Idle) {
-            <div class="input-group input-group-sm mb-2">
-              <span class="input-group-text" i18n>Label</span>
-              <input class="form-control" type="text" [(ngModel)]="newVersionLabel" i18n-placeholder placeholder="Optional" [disabled]="!userIsOwner || !userCanEdit" />
-            </div>
-            <input #versionFileInput type="file" class="visually-hidden" (change)="onVersionFileSelected($event)" />
-            <button class="btn btn-sm btn-outline-secondary w-100" (click)="versionFileInput.click()" [disabled]="!userIsOwner || !userCanEdit">
-              <i-bs name="file-earmark-plus"></i-bs><span class="ps-1" i18n>Add new version</span>
-            </button>
-          } @else {
-            @switch (versionUploadState) {
-              @case (UploadState.Uploading) {
-                <div class="small text-muted mt-1 d-flex align-items-center">
-                  <output class="spinner-border spinner-border-sm me-2" aria-hidden="true"></output>
-                  <span i18n>Uploading version...</span>
-                </div>
-              }
-              @case (UploadState.Processing) {
-                <div class="small text-muted mt-1 d-flex align-items-center">
-                  <output class="spinner-border spinner-border-sm me-2" aria-hidden="true"></output>
-                  <span i18n>Processing version...</span>
-                </div>
-              }
-              @case (UploadState.Failed) {
-                <div class="small text-danger mt-1 d-flex align-items-center justify-content-between">
-                  <span i18n>Version upload failed.</span>
-                  <button type="button" class="btn btn-link btn-sm p-0 ms-2" (click)="clearVersionUploadStatus()" i18n>Dismiss</button>
-                </div>
-                @if (versionUploadError) {
-                  <div class="small text-muted mt-1">{{ versionUploadError }}</div>
-                }
-              }
-            }
-          }
-        </div>
-        <div class="dropdown-divider"></div>
-        @for (version of document.versions; track version.id) {
-          <div class="dropdown-item">
-            <div class="d-flex align-items-center w-100 version-item">
-              <button type="button" class="btn btn-link link-underline link-underline-opacity-0 d-flex align-items-center flex-grow-1 small ps-0 text-start" (click)="selectVersion(version.id)">
-                <div class="badge bg-light text-lowercase text-muted">
-                  {{ version.checksum | slice:0:8 }}
-                </div>
-                <div class="d-flex flex-column small ms-2">
-                  <div>
-                    @if (version.version_label) {
-                      {{ version.version_label }}
-                    } @else {
-                      <span i18n>ID</span> #{{version.id}}
-                    }
-                  </div>
-                  <div class="text-muted">
-                    {{ version.added | customDate:'short' }}
-                  </div>
-                </div>
-              </button>
-              @if (selectedVersionId === version.id) { <span class="ms-2">✓</span> }
-              @if (!version.is_root) {
-                <pngx-confirm-button
-                  buttonClasses="btn-link btn-sm text-danger ms-2"
-                  iconName="trash"
-                  confirmMessage="Delete this version?"
-                  i18n-confirmMessage
-                  [disabled]="!userIsOwner || !userCanEdit"
-                  (confirm)="deleteVersion(version.id)"
-                >
-                  <span class="visually-hidden" i18n>Delete version</span>
-                </pngx-confirm-button>
-              }
-            </div>
-          </div>
-        }
-      </div>
-    </div>
-  }
+  <pngx-document-version-dropdown
+    [documentId]="documentId"
+    [versions]="document?.versions ?? []"
+    [selectedVersionId]="selectedVersionId"
+    [userIsOwner]="userIsOwner"
+    [userCanEdit]="userCanEdit"
+    (versionSelected)="onVersionSelected($event)"
+    (versionsUpdated)="onVersionsUpdated($event)"
+  />
 
   <div class="btn-group">
     <button (click)="download()" class="btn btn-sm btn-outline-primary" [disabled]="downloading">

src-ui/src/app/components/document-detail/document-detail.component.spec.ts

@@ -30,7 +30,7 @@ import {
 } from '@ng-bootstrap/ng-bootstrap'
 import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons'
 import { DeviceDetectorService } from 'ngx-device-detector'
-import { Subject, of, throwError } from 'rxjs'
+import { of, throwError } from 'rxjs'
 import { routes } from 'src/app/app-routing.module'
 import { Correspondent } from 'src/app/data/correspondent'
 import { CustomFieldDataType } from 'src/app/data/custom-field'
@@ -65,10 +65,6 @@ import { TagService } from 'src/app/services/rest/tag.service'
 import { UserService } from 'src/app/services/rest/user.service'
 import { SettingsService } from 'src/app/services/settings.service'
 import { ToastService } from 'src/app/services/toast.service'
-import {
-  UploadState,
-  WebsocketStatusService,
-} from 'src/app/services/websocket-status.service'
 import { environment } from 'src/environments/environment'
 import { ConfirmDialogComponent } from '../common/confirm-dialog/confirm-dialog.component'
 import { PasswordRemovalConfirmDialogComponent } from '../common/confirm-dialog/password-removal-confirm-dialog/password-removal-confirm-dialog.component'
@@ -131,24 +127,6 @@ const customFields = [
   },
 ]
 
-function createFileInput(file?: File) {
-  const input = document.createElement('input')
-  input.type = 'file'
-  const files = file
-    ? ({
-        0: file,
-        length: 1,
-        item: () => file,
-      } as unknown as FileList)
-    : ({
-        length: 0,
-        item: () => null,
-      } as unknown as FileList)
-  Object.defineProperty(input, 'files', { value: files })
-  input.value = ''
-  return input
-}
-
 describe('DocumentDetailComponent', () => {
   let component: DocumentDetailComponent
   let fixture: ComponentFixture<DocumentDetailComponent>
@@ -164,7 +142,6 @@ describe('DocumentDetailComponent', () => {
   let deviceDetectorService: DeviceDetectorService
   let httpTestingController: HttpTestingController
   let componentRouterService: ComponentRouterService
-  let websocketStatusService: WebsocketStatusService
 
   let currentUserCan = true
   let currentUserHasObjectPermissions = true
@@ -314,7 +291,6 @@ describe('DocumentDetailComponent', () => {
     fixture = TestBed.createComponent(DocumentDetailComponent)
     httpTestingController = TestBed.inject(HttpTestingController)
     componentRouterService = TestBed.inject(ComponentRouterService)
-    websocketStatusService = TestBed.inject(WebsocketStatusService)
     component = fixture.componentInstance
   })
 
@@ -1677,206 +1653,56 @@ describe('DocumentDetailComponent', () => {
     expect(component.previewText).toContain('An error occurred loading content')
   })
 
-  it('deleteVersion should update versions, fall back, and surface errors', () => {
+  it('selectVersion should show toast if version content retrieval fails', () => {
     initNormally()
     httpTestingController.expectOne(component.previewUrl).flush('preview')
 
-    component.document.versions = [
-      {
-        id: 3,
-        added: new Date(),
-        version_label: 'Original',
-        checksum: 'aaaa',
-        is_root: true,
-      },
-      {
-        id: 10,
-        added: new Date(),
-        version_label: 'Edited',
-        checksum: 'bbbb',
-        is_root: false,
-      },
-    ]
-    component.selectedVersionId = 10
+    jest.spyOn(documentService, 'getPreviewUrl').mockReturnValue('preview-ok')
+    jest.spyOn(documentService, 'getThumbUrl').mockReturnValue('thumb-ok')
+    jest
+      .spyOn(documentService, 'getMetadata')
+      .mockReturnValue(of({ has_archive_version: true } as any))
+    const contentError = new Error('content failed')
+    jest
+      .spyOn(documentService, 'get')
+      .mockReturnValue(throwError(() => contentError))
+    const toastSpy = jest.spyOn(toastService, 'showError')
 
-    const openDoc = { ...doc, versions: [] } as Document
-    jest.spyOn(openDocumentsService, 'getOpenDocument').mockReturnValue(openDoc)
-    const saveSpy = jest.spyOn(openDocumentsService, 'save')
-    const deleteSpy = jest.spyOn(documentService, 'deleteVersion')
-    const versionsSpy = jest.spyOn(documentService, 'getVersions')
-    const selectSpy = jest
-      .spyOn(component, 'selectVersion')
-      .mockImplementation(() => {})
-    const errorSpy = jest.spyOn(toastService, 'showError')
+    component.selectVersion(10)
+    httpTestingController.expectOne('preview-ok').flush('preview text')
 
-    deleteSpy.mockReturnValueOnce(of({ result: 'ok', current_version_id: 99 }))
-    versionsSpy.mockReturnValueOnce(
-      of({ id: doc.id, versions: [{ id: 99, is_root: false }] } as Document)
+    expect(toastSpy).toHaveBeenCalledWith(
+      'Error retrieving version content',
+      contentError
     )
-    component.deleteVersion(10)
-
-    expect(component.document.versions).toEqual([{ id: 99, is_root: false }])
-    expect(openDoc.versions).toEqual([{ id: 99, is_root: false }])
-    expect(saveSpy).toHaveBeenCalled()
-    expect(selectSpy).toHaveBeenCalledWith(99)
-
-    component.selectedVersionId = 3
-    deleteSpy.mockReturnValueOnce(
-      of({ result: 'ok', current_version_id: null })
-    )
-    versionsSpy.mockReturnValueOnce(
-      of({
-        id: doc.id,
-        versions: [
-          { id: 7, is_root: false },
-          { id: 9, is_root: false },
-        ],
-      } as Document)
-    )
-    component.deleteVersion(3)
-    expect(selectSpy).toHaveBeenCalledWith(component.documentId)
-
-    deleteSpy.mockReturnValueOnce(throwError(() => new Error('nope')))
-    component.deleteVersion(10)
-    expect(errorSpy).toHaveBeenCalled()
   })
 
-  it('onVersionFileSelected should cover upload flows and reset status', () => {
-    initNormally()
-    httpTestingController.expectOne(component.previewUrl).flush('preview')
-
-    const uploadSpy = jest.spyOn(documentService, 'uploadVersion')
-    const versionsSpy = jest.spyOn(documentService, 'getVersions')
-    const infoSpy = jest.spyOn(toastService, 'showInfo')
-    const errorSpy = jest.spyOn(toastService, 'showError')
-    const finishedSpy = jest.spyOn(
-      websocketStatusService,
-      'onDocumentConsumptionFinished'
-    )
-    const failedSpy = jest.spyOn(
-      websocketStatusService,
-      'onDocumentConsumptionFailed'
-    )
-    const selectSpy = jest
+  it('onVersionSelected should delegate to selectVersion', () => {
+    const selectVersionSpy = jest
       .spyOn(component, 'selectVersion')
       .mockImplementation(() => {})
-    const openDoc = { ...doc, versions: [] } as Document
-    jest.spyOn(openDocumentsService, 'getOpenDocument').mockReturnValue(openDoc)
-    const saveSpy = jest.spyOn(openDocumentsService, 'save')
 
-    component.onVersionFileSelected({ target: createFileInput() } as any)
-    expect(uploadSpy).not.toHaveBeenCalled()
+    component.onVersionSelected(42)
 
-    const fileMissing = new File(['data'], 'version.pdf', {
-      type: 'application/pdf',
-    })
-    component.newVersionLabel = '  label  '
-    uploadSpy.mockReturnValueOnce(of({}))
-    component.onVersionFileSelected({
-      target: createFileInput(fileMissing),
-    } as any)
-    expect(uploadSpy).toHaveBeenCalledWith(
-      component.documentId,
-      fileMissing,
-      'label'
-    )
-    expect(component.newVersionLabel).toBe('')
-    expect(component.versionUploadState).toBe(UploadState.Failed)
-    expect(component.versionUploadError).toBe('Missing task ID.')
-    expect(infoSpy).toHaveBeenCalled()
+    expect(selectVersionSpy).toHaveBeenCalledWith(42)
+  })
 
-    const finishedFail$ = new Subject<any>()
-    const failedFail$ = new Subject<any>()
-    finishedSpy.mockReturnValueOnce(finishedFail$ as any)
-    failedSpy.mockReturnValueOnce(failedFail$ as any)
-    uploadSpy.mockReturnValueOnce(of('task-1'))
-    component.onVersionFileSelected({
-      target: createFileInput(
-        new File(['data'], 'version.pdf', { type: 'application/pdf' })
-      ),
-    } as any)
-    expect(component.versionUploadState).toBe(UploadState.Processing)
-    failedFail$.next({ taskId: 'task-1', message: 'nope' })
-    expect(component.versionUploadState).toBe(UploadState.Failed)
-    expect(component.versionUploadError).toBe('nope')
-    expect(versionsSpy).not.toHaveBeenCalled()
-
-    const finishedOk$ = new Subject<any>()
-    const failedOk$ = new Subject<any>()
-    finishedSpy.mockReturnValueOnce(finishedOk$ as any)
-    failedSpy.mockReturnValueOnce(failedOk$ as any)
-    uploadSpy.mockReturnValueOnce(of({ task_id: 'task-2' }))
-    const versions = [
-      { id: 7, is_root: false },
-      { id: 12, is_root: false },
+  it('onVersionsUpdated should sync open document versions and save', () => {
+    component.documentId = doc.id
+    component.document = { ...doc, versions: [] } as Document
+    const updatedVersions = [
+      { id: doc.id, is_root: true },
+      { id: 10, is_root: false },
     ] as any
-    versionsSpy.mockReturnValueOnce(of({ id: doc.id, versions } as Document))
-    component.onVersionFileSelected({
-      target: createFileInput(
-        new File(['data'], 'version.pdf', { type: 'application/pdf' })
-      ),
-    } as any)
-    finishedOk$.next({ taskId: 'task-2' })
+    const openDoc = { ...doc, versions: [] } as Document
+    jest.spyOn(openDocumentsService, 'getOpenDocument').mockReturnValue(openDoc)
+    const saveSpy = jest.spyOn(openDocumentsService, 'save')
 
-    expect(component.document.versions).toEqual(versions)
-    expect(openDoc.versions).toEqual(versions)
+    component.onVersionsUpdated(updatedVersions)
+
+    expect(component.document.versions).toEqual(updatedVersions)
+    expect(openDoc.versions).toEqual(updatedVersions)
     expect(saveSpy).toHaveBeenCalled()
-    expect(selectSpy).toHaveBeenCalledWith(12)
-    expect(component.versionUploadState).toBe(UploadState.Idle)
-    expect(component.versionUploadError).toBeNull()
-
-    component.versionUploadState = UploadState.Failed
-    component.versionUploadError = 'boom'
-    component.clearVersionUploadStatus()
-    expect(component.versionUploadState).toBe(UploadState.Idle)
-    expect(component.versionUploadError).toBeNull()
-
-    uploadSpy.mockReturnValueOnce(throwError(() => new Error('upload blew up')))
-    component.onVersionFileSelected({
-      target: createFileInput(
-        new File(['data'], 'version.pdf', { type: 'application/pdf' })
-      ),
-    } as any)
-    expect(component.versionUploadState).toBe(UploadState.Failed)
-    expect(component.versionUploadError).toBe('upload blew up')
-    expect(errorSpy).toHaveBeenCalled()
-  })
-
-  it('should clear and isolate version upload state on document change', () => {
-    initNormally()
-    httpTestingController.expectOne(component.previewUrl).flush('preview')
-
-    component.versionUploadState = UploadState.Failed
-    component.versionUploadError = 'boom'
-    component.docChangeNotifier.next(999)
-    expect(component.versionUploadState).toBe(UploadState.Idle)
-    expect(component.versionUploadError).toBeNull()
-
-    const uploadSpy = jest.spyOn(documentService, 'uploadVersion')
-    const versionsSpy = jest.spyOn(documentService, 'getVersions')
-    const finished$ = new Subject<any>()
-    const failed$ = new Subject<any>()
-    jest
-      .spyOn(websocketStatusService, 'onDocumentConsumptionFinished')
-      .mockReturnValueOnce(finished$ as any)
-    jest
-      .spyOn(websocketStatusService, 'onDocumentConsumptionFailed')
-      .mockReturnValueOnce(failed$ as any)
-
-    uploadSpy.mockReturnValueOnce(of('task-stale'))
-    component.onVersionFileSelected({
-      target: createFileInput(
-        new File(['data'], 'version.pdf', { type: 'application/pdf' })
-      ),
-    } as any)
-    expect(component.versionUploadState).toBe(UploadState.Processing)
-
-    component.docChangeNotifier.next(1000)
-    failed$.next({ taskId: 'task-stale', message: 'stale-error' })
-
-    expect(component.versionUploadState).toBe(UploadState.Idle)
-    expect(component.versionUploadError).toBeNull()
-    expect(versionsSpy).not.toHaveBeenCalled()
   })
 
   it('createDisabled should return true if the user does not have permission to add the specified data type', () => {
@@ -2012,6 +1838,29 @@ describe('DocumentDetailComponent', () => {
       .error(new ProgressEvent('failed'))
   })
 
+  it('should omit version in download and print when no version is selected', () => {
+    initNormally()
+    component.document.versions = [] as any
+    ;(component as any).selectedVersionId = undefined
+
+    const getDownloadUrlSpy = jest
+      .spyOn(documentService, 'getDownloadUrl')
+      .mockReturnValueOnce('download-no-version')
+      .mockReturnValueOnce('print-no-version')
+
+    component.download()
+    expect(getDownloadUrlSpy).toHaveBeenNthCalledWith(1, doc.id, false, null)
+    httpTestingController
+      .expectOne('download-no-version')
+      .error(new ProgressEvent('failed'))
+
+    component.printDocument()
+    expect(getDownloadUrlSpy).toHaveBeenNthCalledWith(2, doc.id, false, null)
+    httpTestingController
+      .expectOne('print-no-version')
+      .error(new ProgressEvent('failed'))
+  })
+
   it('should download a file with the correct filename', () => {
     const mockBlob = new Blob(['test content'], { type: 'text/plain' })
     const mockResponse = new HttpResponse({

src-ui/src/app/components/document-detail/document-detail.component.ts

@@ -1,4 +1,4 @@
-import { AsyncPipe, NgTemplateOutlet, SlicePipe } from '@angular/common'
+import { AsyncPipe, NgTemplateOutlet } from '@angular/common'
 import { HttpClient, HttpResponse } from '@angular/common/http'
 import { Component, inject, OnDestroy, OnInit, ViewChild } from '@angular/core'
 import {
@@ -20,7 +20,7 @@ import {
 import { dirtyCheck, DirtyComponent } from '@ngneat/dirty-check-forms'
 import { NgxBootstrapIconsModule } from 'ngx-bootstrap-icons'
 import { DeviceDetectorService } from 'ngx-device-detector'
-import { BehaviorSubject, merge, Observable, of, Subject, timer } from 'rxjs'
+import { BehaviorSubject, Observable, of, Subject, timer } from 'rxjs'
 import {
   catchError,
   debounceTime,
@@ -29,7 +29,6 @@ import {
   first,
   map,
   switchMap,
-  take,
   takeUntil,
   tap,
 } from 'rxjs/operators'
@@ -37,7 +36,7 @@ import { Correspondent } from 'src/app/data/correspondent'
 import { CustomField, CustomFieldDataType } from 'src/app/data/custom-field'
 import { CustomFieldInstance } from 'src/app/data/custom-field-instance'
 import { DataType } from 'src/app/data/datatype'
-import { Document } from 'src/app/data/document'
+import { Document, DocumentVersionInfo } from 'src/app/data/document'
 import { DocumentMetadata } from 'src/app/data/document-metadata'
 import { DocumentNote } from 'src/app/data/document-note'
 import { DocumentSuggestions } from 'src/app/data/document-suggestions'
@@ -81,15 +80,10 @@ import { TagService } from 'src/app/services/rest/tag.service'
 import { UserService } from 'src/app/services/rest/user.service'
 import { SettingsService } from 'src/app/services/settings.service'
 import { ToastService } from 'src/app/services/toast.service'
-import {
-  UploadState,
-  WebsocketStatusService,
-} from 'src/app/services/websocket-status.service'
 import { getFilenameFromContentDisposition } from 'src/app/utils/http'
 import { ISODateAdapter } from 'src/app/utils/ngb-iso-date-adapter'
 import * as UTIF from 'utif'
 import { DocumentDetailFieldID } from '../admin/settings/settings.component'
-import { ConfirmButtonComponent } from '../common/confirm-button/confirm-button.component'
 import { ConfirmDialogComponent } from '../common/confirm-dialog/confirm-dialog.component'
 import { PasswordRemovalConfirmDialogComponent } from '../common/confirm-dialog/password-removal-confirm-dialog/password-removal-confirm-dialog.component'
 import { CustomFieldsDropdownComponent } from '../common/custom-fields-dropdown/custom-fields-dropdown.component'
@@ -126,6 +120,7 @@ import { SuggestionsDropdownComponent } from '../common/suggestions-dropdown/sug
 import { DocumentNotesComponent } from '../document-notes/document-notes.component'
 import { ComponentWithPermissions } from '../with-permissions/with-permissions.component'
 import { DocumentHistoryComponent } from './document-history/document-history.component'
+import { DocumentVersionDropdownComponent } from './document-version-dropdown/document-version-dropdown.component'
 import { MetadataCollapseComponent } from './metadata-collapse/metadata-collapse.component'
 
 enum DocumentDetailNavIDs {
@@ -183,8 +178,7 @@ enum ContentRenderType {
     TextAreaComponent,
     RouterModule,
     PngxPdfViewerComponent,
-    ConfirmButtonComponent,
-    SlicePipe,
+    DocumentVersionDropdownComponent,
   ],
 })
 export class DocumentDetailComponent
@@ -192,7 +186,6 @@ export class DocumentDetailComponent
   implements OnInit, OnDestroy, DirtyComponent
 {
   PdfRenderMode = PdfRenderMode
-  UploadState = UploadState
 
   documentsService = inject(DocumentService)
   private route = inject(ActivatedRoute)
@@ -215,7 +208,6 @@ export class DocumentDetailComponent
   private componentRouterService = inject(ComponentRouterService)
   private deviceDetectorService = inject(DeviceDetectorService)
   private savedViewService = inject(SavedViewService)
-  private readonly websocketStatusService = inject(WebsocketStatusService)
 
   @ViewChild('inputTitle')
   titleInput: TextComponent
@@ -247,10 +239,7 @@ export class DocumentDetailComponent
 
   // Versioning
   selectedVersionId: number
-  newVersionLabel: string = ''
   pdfSource: PdfSource
-  versionUploadState: UploadState = UploadState.Idle
-  versionUploadError: string | null = null
 
   correspondents: Correspondent[]
   documentTypes: DocumentType[]
@@ -297,7 +286,6 @@ export class DocumentDetailComponent
   public readonly DocumentDetailFieldID = DocumentDetailFieldID
 
   @ViewChild('nav') nav: NgbNav
-  @ViewChild('versionFileInput') versionFileInput
   @ViewChild('pdfPreview') set pdfPreview(element) {
     // this gets called when component added or removed from DOM
     if (
@@ -674,10 +662,6 @@ export class DocumentDetailComponent
         this.loadDocument(documentId)
       })
 
-    this.docChangeNotifier
-      .pipe(takeUntil(this.unsubscribeNotifier))
-      .subscribe(() => this.clearVersionUploadStatus())
-
     this.route.paramMap
       .pipe(takeUntil(this.unsubscribeNotifier))
       .subscribe((paramMap) => {
@@ -859,41 +843,17 @@ export class DocumentDetailComponent
       })
   }
 
-  deleteVersion(versionId: number) {
-    const wasSelected = this.selectedVersionId === versionId
-    this.documentsService
-      .deleteVersion(this.documentId, versionId)
-      .pipe(
-        switchMap((result) =>
-          this.documentsService
-            .getVersions(this.documentId)
-            .pipe(map((doc) => ({ doc, result })))
-        ),
-        first(),
-        takeUntil(this.unsubscribeNotifier)
-      )
-      .subscribe({
-        next: ({ doc, result }) => {
-          if (doc?.versions) {
-            this.document.versions = doc.versions
-            const openDoc = this.openDocumentService.getOpenDocument(
-              this.documentId
-            )
-            if (openDoc) {
-              openDoc.versions = doc.versions
-              this.openDocumentService.save()
-            }
-          }
+  onVersionSelected(versionId: number) {
+    this.selectVersion(versionId)
+  }
 
-          if (wasSelected) {
-            const fallbackId = result?.current_version_id ?? this.documentId
-            this.selectVersion(fallbackId)
-          }
-        },
-        error: (error) => {
-          this.toastService.showError($localize`Error deleting version`, error)
-        },
-      })
+  onVersionsUpdated(versions: DocumentVersionInfo[]) {
+    this.document.versions = versions
+    const openDoc = this.openDocumentService.getOpenDocument(this.documentId)
+    if (openDoc) {
+      openDoc.versions = versions
+      this.openDocumentService.save()
+    }
   }
 
   get customFieldFormFields(): FormArray {
@@ -1312,104 +1272,6 @@ export class DocumentDetailComponent
     })
   }
 
-  onVersionFileSelected(event: Event) {
-    const input = event.target as HTMLInputElement
-    if (!input?.files || input.files.length === 0) return
-    const uploadDocumentId = this.documentId
-    const file = input.files[0]
-    // Reset input to allow re-selection of the same file later
-    input.value = ''
-    const label = this.newVersionLabel?.trim()
-    this.versionUploadState = UploadState.Uploading
-    this.versionUploadError = null
-    this.documentsService
-      .uploadVersion(uploadDocumentId, file, label)
-      .pipe(
-        first(),
-        tap(() => {
-          this.toastService.showInfo(
-            $localize`Uploading new version. Processing will happen in the background.`
-          )
-          this.newVersionLabel = ''
-          this.versionUploadState = UploadState.Processing
-        }),
-        map((taskId) =>
-          typeof taskId === 'string'
-            ? taskId
-            : (taskId as { task_id?: string })?.task_id
-        ),
-        switchMap((taskId) => {
-          if (!taskId) {
-            this.versionUploadState = UploadState.Failed
-            this.versionUploadError = $localize`Missing task ID.`
-            return of(null)
-          }
-          return merge(
-            this.websocketStatusService.onDocumentConsumptionFinished().pipe(
-              filter((status) => status.taskId === taskId),
-              map(() => ({ state: 'success' as const }))
-            ),
-            this.websocketStatusService.onDocumentConsumptionFailed().pipe(
-              filter((status) => status.taskId === taskId),
-              map((status) => ({
-                state: 'failed' as const,
-                message: status.message,
-              }))
-            )
-          ).pipe(
-            takeUntil(merge(this.unsubscribeNotifier, this.docChangeNotifier)),
-            take(1)
-          )
-        }),
-        switchMap((result) => {
-          if (result?.state !== 'success') {
-            if (result?.state === 'failed') {
-              this.versionUploadState = UploadState.Failed
-              this.versionUploadError =
-                result.message || $localize`Upload failed.`
-            }
-            return of(null)
-          }
-          return this.documentsService.getVersions(uploadDocumentId)
-        }),
-        takeUntil(this.unsubscribeNotifier),
-        takeUntil(this.docChangeNotifier)
-      )
-      .subscribe({
-        next: (doc) => {
-          if (uploadDocumentId !== this.documentId) return
-          if (doc?.versions) {
-            this.document.versions = doc.versions
-            const openDoc = this.openDocumentService.getOpenDocument(
-              this.documentId
-            )
-            if (openDoc) {
-              openDoc.versions = doc.versions
-              this.openDocumentService.save()
-            }
-            this.selectVersion(
-              Math.max(...doc.versions.map((version) => version.id))
-            )
-            this.clearVersionUploadStatus()
-          }
-        },
-        error: (error) => {
-          if (uploadDocumentId !== this.documentId) return
-          this.versionUploadState = UploadState.Failed
-          this.versionUploadError = error?.message || $localize`Upload failed.`
-          this.toastService.showError(
-            $localize`Error uploading new version`,
-            error
-          )
-        },
-      })
-  }
-
-  clearVersionUploadStatus() {
-    this.versionUploadState = UploadState.Idle
-    this.versionUploadError = null
-  }
-
   private getSelectedNonLatestVersionId(): number | null {
     const versions = this.document?.versions ?? []
     if (!versions.length || !this.selectedVersionId) {

src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.html

@@ -0,0 +1,97 @@
+<div class="btn-group" ngbDropdown autoClose="outside">
+  <button class="btn btn-sm btn-outline-secondary dropdown-toggle" ngbDropdownToggle>
+    <i-bs name="file-earmark-diff"></i-bs>
+    <span class="d-none d-lg-inline ps-1" i18n>Versions</span>
+  </button>
+  <div class="dropdown-menu shadow" ngbDropdownMenu>
+    <div class="px-3 py-2">
+      @if (versionUploadState === UploadState.Idle) {
+        <div class="input-group input-group-sm mb-2">
+          <span class="input-group-text" i18n>Label</span>
+          <input
+            class="form-control"
+            type="text"
+            [(ngModel)]="newVersionLabel"
+            i18n-placeholder
+            placeholder="Optional"
+            [disabled]="!userIsOwner || !userCanEdit"
+          />
+        </div>
+        <input
+          #versionFileInput
+          type="file"
+          class="visually-hidden"
+          (change)="onVersionFileSelected($event)"
+        />
+        <button
+          class="btn btn-sm btn-outline-secondary w-100"
+          (click)="versionFileInput.click()"
+          [disabled]="!userIsOwner || !userCanEdit"
+        >
+          <i-bs name="file-earmark-plus"></i-bs><span class="ps-1" i18n>Add new version</span>
+        </button>
+      } @else {
+        @switch (versionUploadState) {
+          @case (UploadState.Uploading) {
+            <div class="small text-muted mt-1 d-flex align-items-center">
+              <output class="spinner-border spinner-border-sm me-2" aria-hidden="true"></output>
+              <span i18n>Uploading version...</span>
+            </div>
+          }
+          @case (UploadState.Processing) {
+            <div class="small text-muted mt-1 d-flex align-items-center">
+              <output class="spinner-border spinner-border-sm me-2" aria-hidden="true"></output>
+              <span i18n>Processing version...</span>
+            </div>
+          }
+          @case (UploadState.Failed) {
+            <div class="small text-danger mt-1 d-flex align-items-center justify-content-between">
+              <span i18n>Version upload failed.</span>
+              <button type="button" class="btn btn-link btn-sm p-0 ms-2" (click)="clearVersionUploadStatus()" i18n>Dismiss</button>
+            </div>
+            @if (versionUploadError) {
+              <div class="small text-muted mt-1">{{ versionUploadError }}</div>
+            }
+          }
+        }
+      }
+    </div>
+    <div class="dropdown-divider"></div>
+    @for (version of versions; track version.id) {
+      <div class="dropdown-item">
+        <div class="d-flex align-items-center w-100 version-item">
+          <button type="button" class="btn btn-link link-underline link-underline-opacity-0 d-flex align-items-center flex-grow-1 small ps-0 text-start" (click)="selectVersion(version.id)">
+            <div class="badge bg-light text-lowercase text-muted">
+              {{ version.checksum | slice:0:8 }}
+            </div>
+            <div class="d-flex flex-column small ms-2">
+              <div>
+                @if (version.version_label) {
+                  {{ version.version_label }}
+                } @else {
+                  <span i18n>ID</span> #{{version.id}}
+                }
+              </div>
+              <div class="text-muted">
+                {{ version.added | customDate:'short' }}
+              </div>
+            </div>
+          </button>
+          @if (selectedVersionId === version.id) { <span class="ms-2">✓</span> }
+          @if (!version.is_root) {
+            <pngx-confirm-button
+              buttonClasses="btn-link btn-sm text-danger ms-2"
+              iconName="trash"
+              confirmMessage="Delete this version?"
+              i18n-confirmMessage
+              [disabled]="!userIsOwner || !userCanEdit"
+              (confirm)="deleteVersion(version.id)"
+            >
+              <span class="visually-hidden" i18n>Delete version</span>
+            </pngx-confirm-button>
+          }
+        </div>
+      </div>
+    }
+  </div>
+</div>

src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.spec.ts

@@ -0,0 +1,226 @@
+import { DatePipe } from '@angular/common'
+import { SimpleChange } from '@angular/core'
+import { ComponentFixture, TestBed } from '@angular/core/testing'
+import { NgxBootstrapIconsModule, allIcons } from 'ngx-bootstrap-icons'
+import { Subject, of, throwError } from 'rxjs'
+import { DocumentVersionInfo } from 'src/app/data/document'
+import { DocumentService } from 'src/app/services/rest/document.service'
+import { SettingsService } from 'src/app/services/settings.service'
+import { ToastService } from 'src/app/services/toast.service'
+import {
+  UploadState,
+  WebsocketStatusService,
+} from 'src/app/services/websocket-status.service'
+import { DocumentVersionDropdownComponent } from './document-version-dropdown.component'
+
+describe('DocumentVersionDropdownComponent', () => {
+  let component: DocumentVersionDropdownComponent
+  let fixture: ComponentFixture<DocumentVersionDropdownComponent>
+  let documentService: jest.Mocked<
+    Pick<DocumentService, 'deleteVersion' | 'getVersions' | 'uploadVersion'>
+  >
+  let toastService: jest.Mocked<Pick<ToastService, 'showError' | 'showInfo'>>
+  let finished$: Subject<{ taskId: string }>
+  let failed$: Subject<{ taskId: string; message?: string }>
+
+  beforeEach(async () => {
+    finished$ = new Subject<{ taskId: string }>()
+    failed$ = new Subject<{ taskId: string; message?: string }>()
+    documentService = {
+      deleteVersion: jest.fn(),
+      getVersions: jest.fn(),
+      uploadVersion: jest.fn(),
+    }
+    toastService = {
+      showError: jest.fn(),
+      showInfo: jest.fn(),
+    }
+
+    await TestBed.configureTestingModule({
+      imports: [
+        DocumentVersionDropdownComponent,
+        NgxBootstrapIconsModule.pick(allIcons),
+      ],
+      providers: [
+        DatePipe,
+        {
+          provide: DocumentService,
+          useValue: documentService,
+        },
+        {
+          provide: SettingsService,
+          useValue: {
+            get: () => null,
+          },
+        },
+        {
+          provide: ToastService,
+          useValue: toastService,
+        },
+        {
+          provide: WebsocketStatusService,
+          useValue: {
+            onDocumentConsumptionFinished: () => finished$,
+            onDocumentConsumptionFailed: () => failed$,
+          },
+        },
+      ],
+    }).compileComponents()
+
+    fixture = TestBed.createComponent(DocumentVersionDropdownComponent)
+    component = fixture.componentInstance
+    component.documentId = 3
+    component.selectedVersionId = 3
+    component.versions = [
+      {
+        id: 3,
+        is_root: true,
+        checksum: 'aaaa',
+      },
+      {
+        id: 10,
+        is_root: false,
+        checksum: 'bbbb',
+      },
+    ]
+    fixture.detectChanges()
+  })
+
+  it('selectVersion should emit the selected id', () => {
+    const emitSpy = jest.spyOn(component.versionSelected, 'emit')
+    component.selectVersion(10)
+    expect(emitSpy).toHaveBeenCalledWith(10)
+  })
+
+  it('deleteVersion should refresh versions and select fallback when deleting current selection', () => {
+    const updatedVersions: DocumentVersionInfo[] = [
+      { id: 3, is_root: true, checksum: 'aaaa' },
+      { id: 20, is_root: false, checksum: 'cccc' },
+    ]
+    component.selectedVersionId = 10
+    documentService.deleteVersion.mockReturnValue(
+      of({ result: 'deleted', current_version_id: 3 })
+    )
+    documentService.getVersions.mockReturnValue(
+      of({ id: 3, versions: updatedVersions } as any)
+    )
+    const versionsEmitSpy = jest.spyOn(component.versionsUpdated, 'emit')
+    const selectedEmitSpy = jest.spyOn(component.versionSelected, 'emit')
+
+    component.deleteVersion(10)
+
+    expect(documentService.deleteVersion).toHaveBeenCalledWith(3, 10)
+    expect(documentService.getVersions).toHaveBeenCalledWith(3)
+    expect(versionsEmitSpy).toHaveBeenCalledWith(updatedVersions)
+    expect(selectedEmitSpy).toHaveBeenCalledWith(3)
+  })
+
+  it('deleteVersion should show an error toast on failure', () => {
+    const error = new Error('delete failed')
+    documentService.deleteVersion.mockReturnValue(throwError(() => error))
+
+    component.deleteVersion(10)
+
+    expect(toastService.showError).toHaveBeenCalledWith(
+      'Error deleting version',
+      error
+    )
+  })
+
+  it('onVersionFileSelected should upload and update versions after websocket success', () => {
+    const versions: DocumentVersionInfo[] = [
+      { id: 3, is_root: true, checksum: 'aaaa' },
+      { id: 20, is_root: false, checksum: 'cccc' },
+    ]
+    const file = new File(['test'], 'new-version.pdf', {
+      type: 'application/pdf',
+    })
+    const input = document.createElement('input')
+    Object.defineProperty(input, 'files', { value: [file] })
+    component.newVersionLabel = '  Updated scan  '
+    documentService.uploadVersion.mockReturnValue(
+      of({ task_id: 'task-1' } as any)
+    )
+    documentService.getVersions.mockReturnValue(of({ id: 3, versions } as any))
+    const versionsEmitSpy = jest.spyOn(component.versionsUpdated, 'emit')
+    const selectedEmitSpy = jest.spyOn(component.versionSelected, 'emit')
+
+    component.onVersionFileSelected({ target: input } as Event)
+    finished$.next({ taskId: 'task-1' })
+
+    expect(documentService.uploadVersion).toHaveBeenCalledWith(
+      3,
+      file,
+      'Updated scan'
+    )
+    expect(toastService.showInfo).toHaveBeenCalled()
+    expect(documentService.getVersions).toHaveBeenCalledWith(3)
+    expect(versionsEmitSpy).toHaveBeenCalledWith(versions)
+    expect(selectedEmitSpy).toHaveBeenCalledWith(20)
+    expect(component.newVersionLabel).toEqual('')
+    expect(component.versionUploadState).toEqual(UploadState.Idle)
+    expect(component.versionUploadError).toBeNull()
+  })
+
+  it('onVersionFileSelected should set failed state after websocket failure', () => {
+    const file = new File(['test'], 'new-version.pdf', {
+      type: 'application/pdf',
+    })
+    const input = document.createElement('input')
+    Object.defineProperty(input, 'files', { value: [file] })
+    documentService.uploadVersion.mockReturnValue(of('task-1'))
+
+    component.onVersionFileSelected({ target: input } as Event)
+    failed$.next({ taskId: 'task-1', message: 'processing failed' })
+
+    expect(component.versionUploadState).toEqual(UploadState.Failed)
+    expect(component.versionUploadError).toEqual('processing failed')
+    expect(documentService.getVersions).not.toHaveBeenCalled()
+  })
+
+  it('onVersionFileSelected should fail when backend response has no task id', () => {
+    const file = new File(['test'], 'new-version.pdf', {
+      type: 'application/pdf',
+    })
+    const input = document.createElement('input')
+    Object.defineProperty(input, 'files', { value: [file] })
+    documentService.uploadVersion.mockReturnValue(of({} as any))
+
+    component.onVersionFileSelected({ target: input } as Event)
+
+    expect(component.versionUploadState).toEqual(UploadState.Failed)
+    expect(component.versionUploadError).toEqual('Missing task ID.')
+    expect(documentService.getVersions).not.toHaveBeenCalled()
+  })
+
+  it('onVersionFileSelected should show error when upload request fails', () => {
+    const file = new File(['test'], 'new-version.pdf', {
+      type: 'application/pdf',
+    })
+    const input = document.createElement('input')
+    Object.defineProperty(input, 'files', { value: [file] })
+    const error = new Error('upload failed')
+    documentService.uploadVersion.mockReturnValue(throwError(() => error))
+
+    component.onVersionFileSelected({ target: input } as Event)
+
+    expect(component.versionUploadState).toEqual(UploadState.Failed)
+    expect(component.versionUploadError).toEqual('upload failed')
+    expect(toastService.showError).toHaveBeenCalledWith(
+      'Error uploading new version',
+      error
+    )
+  })
+
+  it('ngOnChanges should clear upload status on document switch', () => {
+    component.versionUploadState = UploadState.Failed
+    component.versionUploadError = 'something failed'
+
+    component.ngOnChanges({
+      documentId: new SimpleChange(3, 4, false),
+    })
+
+    expect(component.versionUploadState).toEqual(UploadState.Idle)
+    expect(component.versionUploadError).toBeNull()
+  })
+})

src-ui/src/app/components/document-detail/document-version-dropdown/document-version-dropdown.component.ts

@@ -0,0 +1,203 @@
+import { SlicePipe } from '@angular/common'
+import {
+  Component,
+  EventEmitter,
+  inject,
+  Input,
+  OnChanges,
+  OnDestroy,
+  Output,
+  SimpleChanges,
+} from '@angular/core'
+import { FormsModule } from '@angular/forms'
+import { NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap'
+import { NgxBootstrapIconsModule } from 'ngx-bootstrap-icons'
+import { merge, of, Subject } from 'rxjs'
+import {
+  filter,
+  first,
+  map,
+  switchMap,
+  take,
+  takeUntil,
+  tap,
+} from 'rxjs/operators'
+import { DocumentVersionInfo } from 'src/app/data/document'
+import { CustomDatePipe } from 'src/app/pipes/custom-date.pipe'
+import { DocumentService } from 'src/app/services/rest/document.service'
+import { ToastService } from 'src/app/services/toast.service'
+import {
+  UploadState,
+  WebsocketStatusService,
+} from 'src/app/services/websocket-status.service'
+import { ConfirmButtonComponent } from '../../common/confirm-button/confirm-button.component'
+
+@Component({
+  selector: 'pngx-document-version-dropdown',
+  templateUrl: './document-version-dropdown.component.html',
+  imports: [
+    FormsModule,
+    NgbDropdownModule,
+    NgxBootstrapIconsModule,
+    ConfirmButtonComponent,
+    SlicePipe,
+    CustomDatePipe,
+  ],
+})
+export class DocumentVersionDropdownComponent implements OnChanges, OnDestroy {
+  UploadState = UploadState
+
+  @Input() documentId: number
+  @Input() versions: DocumentVersionInfo[] = []
+  @Input() selectedVersionId: number
+  @Input() userCanEdit: boolean = false
+  @Input() userIsOwner: boolean = false
+
+  @Output() versionSelected = new EventEmitter<number>()
+  @Output() versionsUpdated = new EventEmitter<DocumentVersionInfo[]>()
+
+  newVersionLabel: string = ''
+  versionUploadState: UploadState = UploadState.Idle
+  versionUploadError: string | null = null
+
+  private readonly documentsService = inject(DocumentService)
+  private readonly toastService = inject(ToastService)
+  private readonly websocketStatusService = inject(WebsocketStatusService)
+  private readonly destroy$ = new Subject<void>()
+  private readonly documentChange$ = new Subject<void>()
+
+  ngOnChanges(changes: SimpleChanges): void {
+    if (changes.documentId && !changes.documentId.firstChange) {
+      this.documentChange$.next()
+      this.clearVersionUploadStatus()
+    }
+  }
+
+  ngOnDestroy(): void {
+    this.documentChange$.next()
+    this.documentChange$.complete()
+    this.destroy$.next()
+    this.destroy$.complete()
+  }
+
+  selectVersion(versionId: number): void {
+    this.versionSelected.emit(versionId)
+  }
+
+  deleteVersion(versionId: number): void {
+    const wasSelected = this.selectedVersionId === versionId
+    this.documentsService
+      .deleteVersion(this.documentId, versionId)
+      .pipe(
+        switchMap((result) =>
+          this.documentsService
+            .getVersions(this.documentId)
+            .pipe(map((doc) => ({ doc, result })))
+        ),
+        first(),
+        takeUntil(this.destroy$)
+      )
+      .subscribe({
+        next: ({ doc, result }) => {
+          if (doc?.versions) {
+            this.versionsUpdated.emit(doc.versions)
+          }
+
+          if (wasSelected || this.selectedVersionId === versionId) {
+            const fallbackId = result?.current_version_id ?? this.documentId
+            this.versionSelected.emit(fallbackId)
+          }
+        },
+        error: (error) => {
+          this.toastService.showError($localize`Error deleting version`, error)
+        },
+      })
+  }
+
+  onVersionFileSelected(event: Event): void {
+    const input = event.target as HTMLInputElement
+    if (!input?.files || input.files.length === 0) return
+    const uploadDocumentId = this.documentId
+    const file = input.files[0]
+    input.value = ''
+    const label = this.newVersionLabel?.trim()
+    this.versionUploadState = UploadState.Uploading
+    this.versionUploadError = null
+    this.documentsService
+      .uploadVersion(uploadDocumentId, file, label)
+      .pipe(
+        first(),
+        tap(() => {
+          this.toastService.showInfo(
+            $localize`Uploading new version. Processing will happen in the background.`
+          )
+          this.newVersionLabel = ''
+          this.versionUploadState = UploadState.Processing
+        }),
+        map((taskId) =>
+          typeof taskId === 'string'
+            ? taskId
+            : (taskId as { task_id?: string })?.task_id
+        ),
+        switchMap((taskId) => {
+          if (!taskId) {
+            this.versionUploadState = UploadState.Failed
+            this.versionUploadError = $localize`Missing task ID.`
+            return of(null)
+          }
+          return merge(
+            this.websocketStatusService.onDocumentConsumptionFinished().pipe(
+              filter((status) => status.taskId === taskId),
+              map(() => ({ state: 'success' as const }))
+            ),
+            this.websocketStatusService.onDocumentConsumptionFailed().pipe(
+              filter((status) => status.taskId === taskId),
+              map((status) => ({
+                state: 'failed' as const,
+                message: status.message,
+              }))
+            )
+          ).pipe(takeUntil(merge(this.destroy$, this.documentChange$)), take(1))
+        }),
+        switchMap((result) => {
+          if (result?.state !== 'success') {
+            if (result?.state === 'failed') {
+              this.versionUploadState = UploadState.Failed
+              this.versionUploadError =
+                result.message || $localize`Upload failed.`
+            }
+            return of(null)
+          }
+          return this.documentsService.getVersions(uploadDocumentId)
+        }),
+        takeUntil(this.destroy$),
+        takeUntil(this.documentChange$)
+      )
+      .subscribe({
+        next: (doc) => {
+          if (uploadDocumentId !== this.documentId) return
+          if (doc?.versions) {
+            this.versionsUpdated.emit(doc.versions)
+            this.versionSelected.emit(
+              Math.max(...doc.versions.map((version) => version.id))
+            )
+            this.clearVersionUploadStatus()
+          }
+        },
+        error: (error) => {
+          if (uploadDocumentId !== this.documentId) return
+          this.versionUploadState = UploadState.Failed
+          this.versionUploadError = error?.message || $localize`Upload failed.`
+          this.toastService.showError(
+            $localize`Error uploading new version`,
+            error
+          )
+        },
+      })
+  }
+
+  clearVersionUploadStatus(): void {
+    this.versionUploadState = UploadState.Idle
+    this.versionUploadError = null
+  }
+}

src/documents/bulk_edit.py

@@ -357,6 +357,7 @@ def delete(doc_ids: list[int]) -> Literal["OK"]:
         )
         version_ids = (
             Document.objects.filter(root_document_id__in=root_ids)
+            .exclude(id__in=doc_ids)
             .values_list("id", flat=True)
             .distinct()
         )

src/documents/conditionals.py

@@ -13,59 +13,7 @@ from documents.caching import CLASSIFIER_VERSION_KEY
 from documents.caching import get_thumbnail_modified_key
 from documents.classifier import DocumentClassifier
 from documents.models import Document
-
-
-def _resolve_effective_doc(pk: int, request) -> Document | None:
-    """
-    Resolve which Document row should be considered for caching keys:
-    - If a version is requested, use that version
-    - If pk is a root doc, use its newest child version if present, else the root.
-    - Else, pk is a version, use that version.
-    Returns None if resolution fails (treat as no-cache).
-    """
-    try:
-        request_doc = Document.objects.only("id", "root_document_id").get(pk=pk)
-    except Document.DoesNotExist:
-        return None
-
-    root_doc = (
-        request_doc
-        if request_doc.root_document_id is None
-        else Document.objects.only("id").get(id=request_doc.root_document_id)
-    )
-
-    version_param = (
-        request.query_params.get("version")
-        if hasattr(request, "query_params")
-        else None
-    )
-    if version_param:
-        try:
-            version_id = int(version_param)
-            candidate = Document.objects.only("id", "root_document_id").get(
-                id=version_id,
-            )
-            if (
-                candidate.id != root_doc.id
-                and candidate.root_document_id != root_doc.id
-            ):
-                return None
-            return candidate
-        except Exception:
-            return None
-
-    # Default behavior: if pk is a root doc, prefer its newest child version
-    if request_doc.root_document_id is None:
-        latest = (
-            Document.objects.filter(root_document=root_doc)
-            .only("id")
-            .order_by("id")
-            .last()
-        )
-        return latest or root_doc
-
-    # pk is already a version
-    return request_doc
+from documents.versioning import resolve_effective_document_by_pk
 
 
 def suggestions_etag(request, pk: int) -> str | None:
@@ -125,7 +73,7 @@ def metadata_etag(request, pk: int) -> str | None:
     Metadata is extracted from the original file, so use its checksum as the
     ETag
     """
-    doc = _resolve_effective_doc(pk, request)
+    doc = resolve_effective_document_by_pk(pk, request).document
     if doc is None:
         return None
     return doc.checksum
@@ -137,7 +85,7 @@ def metadata_last_modified(request, pk: int) -> datetime | None:
     not the modification of the original file, but of the database object, but might as well
     error on the side of more cautious
     """
-    doc = _resolve_effective_doc(pk, request)
+    doc = resolve_effective_document_by_pk(pk, request).document
     if doc is None:
         return None
     return doc.modified
@@ -147,7 +95,7 @@ def preview_etag(request, pk: int) -> str | None:
     """
     ETag for the document preview, using the original or archive checksum, depending on the request
     """
-    doc = _resolve_effective_doc(pk, request)
+    doc = resolve_effective_document_by_pk(pk, request).document
     if doc is None:
         return None
     use_original = (
@@ -163,7 +111,7 @@ def preview_last_modified(request, pk: int) -> datetime | None:
     Uses the documents modified time to set the Last-Modified header.  Not strictly
     speaking correct, but close enough and quick
     """
-    doc = _resolve_effective_doc(pk, request)
+    doc = resolve_effective_document_by_pk(pk, request).document
     if doc is None:
         return None
     return doc.modified
@@ -175,7 +123,7 @@ def thumbnail_last_modified(request: Any, pk: int) -> datetime | None:
     Cache should be (slightly?) faster than filesystem
     """
     try:
-        doc = _resolve_effective_doc(pk, request)
+        doc = resolve_effective_document_by_pk(pk, request).document
         if doc is None:
             return None
         if not doc.thumbnail_path.exists():
@@ -195,5 +143,5 @@ def thumbnail_last_modified(request: Any, pk: int) -> datetime | None:
         )
         cache.set(doc_key, last_modified, CACHE_50_MINUTES)
         return last_modified
-    except Document.DoesNotExist:  # pragma: no cover
+    except (Document.DoesNotExist, OSError):  # pragma: no cover
         return None

src/documents/consumer.py

@@ -183,6 +183,41 @@ class ConsumerPlugin(
 ):
     logging_name = LOGGING_NAME
 
+    def _clone_root_into_version(
+        self,
+        root_doc: Document,
+        *,
+        text: str | None,
+        page_count: int | None,
+        mime_type: str,
+    ) -> Document:
+        self.log.debug("Saving record for updated version to database")
+        version_doc = Document.objects.get(pk=root_doc.pk)
+        setattr(version_doc, "pk", None)
+        version_doc.root_document = root_doc
+        file_for_checksum = (
+            self.unmodified_original
+            if self.unmodified_original is not None
+            else self.working_copy
+        )
+        version_doc.checksum = hashlib.md5(
+            file_for_checksum.read_bytes(),
+        ).hexdigest()
+        version_doc.content = text or ""
+        version_doc.page_count = page_count
+        version_doc.mime_type = mime_type
+        version_doc.original_filename = self.filename
+        version_doc.storage_path = root_doc.storage_path
+        # Clear unique file path fields so they can be generated uniquely later
+        version_doc.filename = None
+        version_doc.archive_filename = None
+        version_doc.archive_checksum = None
+        if self.metadata.version_label is not None:
+            version_doc.version_label = self.metadata.version_label
+        version_doc.added = timezone.now()
+        version_doc.modified = timezone.now()
+        return version_doc
+
     def run_pre_consume_script(self) -> None:
         """
         If one is configured and exists, run the pre-consume script and
@@ -506,33 +541,12 @@ class ConsumerPlugin(
                     root_doc = Document.objects.get(
                         pk=self.input_doc.root_document_id,
                     )
-                    original_document = Document.objects.get(
-                        pk=self.input_doc.root_document_id,
+                    original_document = self._clone_root_into_version(
+                        root_doc,
+                        text=text,
+                        page_count=page_count,
+                        mime_type=mime_type,
                     )
-                    self.log.debug("Saving record for updated version to database")
-                    setattr(original_document, "pk", None)
-                    original_document.root_document = root_doc
-                    file_for_checksum = (
-                        self.unmodified_original
-                        if self.unmodified_original is not None
-                        else self.working_copy
-                    )
-                    original_document.checksum = hashlib.md5(
-                        file_for_checksum.read_bytes(),
-                    ).hexdigest()
-                    original_document.content = text or ""
-                    original_document.page_count = page_count
-                    original_document.mime_type = mime_type
-                    original_document.original_filename = self.filename
-                    original_document.storage_path = root_doc.storage_path
-                    # Clear unique file path fields so they can be generated uniquely later
-                    original_document.filename = None
-                    original_document.archive_filename = None
-                    original_document.archive_checksum = None
-                    if self.metadata.version_label is not None:
-                        original_document.version_label = self.metadata.version_label
-                    original_document.added = timezone.now()
-                    original_document.modified = timezone.now()
                     actor = None
 
                     # Save the new version, potentially creating an audit log entry for the version addition if enabled.

src/documents/index.py

@@ -158,7 +158,11 @@ def open_index_searcher() -> Searcher:
         searcher.close()
 
 
-def update_document(writer: AsyncWriter, doc: Document) -> None:
+def update_document(
+    writer: AsyncWriter,
+    doc: Document,
+    effective_content: str | None = None,
+) -> None:
     tags = ",".join([t.name for t in doc.tags.all()])
     tags_ids = ",".join([str(t.id) for t in doc.tags.all()])
     notes = ",".join([str(c.note) for c in Note.objects.filter(document=doc)])
@@ -185,20 +189,10 @@ def update_document(writer: AsyncWriter, doc: Document) -> None:
         only_with_perms_in=["view_document"],
     )
     viewer_ids: str = ",".join([str(u.id) for u in users_with_perms])
-    effective_content = doc.content
-    if doc.root_document_id is None:
-        latest_version = (
-            Document.objects.filter(root_document=doc)
-            .only("content")
-            .order_by("-id")
-            .first()
-        )
-        if latest_version is not None:
-            effective_content = latest_version.content
     writer.update_document(
         id=doc.pk,
         title=doc.title,
-        content=effective_content,
+        content=effective_content or doc.content,
         correspondent=doc.correspondent.name if doc.correspondent else None,
         correspondent_id=doc.correspondent.id if doc.correspondent else None,
         has_correspondent=doc.correspondent is not None,
@@ -241,9 +235,12 @@ def remove_document_by_id(writer: AsyncWriter, doc_id) -> None:
     writer.delete_by_term("id", doc_id)
 
 
-def add_or_update_document(document: Document) -> None:
+def add_or_update_document(
+    document: Document,
+    effective_content: str | None = None,
+) -> None:
     with open_index_writer() as writer:
-        update_document(writer, document)
+        update_document(writer, document, effective_content=effective_content)
 
 
 def remove_document_from_index(document: Document) -> None:

src/documents/signals/handlers.py

@@ -724,7 +724,10 @@ def add_to_index(sender, document, **kwargs) -> None:
     index.add_or_update_document(document)
     if document.root_document_id is not None and document.root_document is not None:
         # keep in sync when a new version is consumed.
-        index.add_or_update_document(document.root_document)
+        index.add_or_update_document(
+            document.root_document,
+            effective_content=document.content,
+        )
 
 
 def run_workflows_added(

src/documents/tests/test_api_document_versions.py

@@ -1,17 +1,21 @@
 from __future__ import annotations
 
 from typing import TYPE_CHECKING
+from unittest import TestCase
 from unittest import mock
 
 from auditlog.models import LogEntry  # type: ignore[import-untyped]
 from django.contrib.auth.models import Permission
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
+from django.core.exceptions import FieldError
 from django.core.files.uploadedfile import SimpleUploadedFile
 from rest_framework import status
 from rest_framework.test import APITestCase
 
 from documents.data_models import DocumentSource
+from documents.filters import EffectiveContentFilter
+from documents.filters import TitleContentFilter
 from documents.models import Document
 from documents.tests.utils import DirectoriesMixin
 
@@ -393,6 +397,28 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
         self.assertEqual(resp.status_code, status.HTTP_200_OK)
         self.assertTrue(metadata.called)
 
+    def test_metadata_version_param_errors(self) -> None:
+        root = self._create_pdf(title="root", checksum="root")
+
+        resp = self.client.get(
+            f"/api/documents/{root.id}/metadata/?version=not-a-number",
+        )
+        self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND)
+
+        resp = self.client.get(f"/api/documents/{root.id}/metadata/?version=9999")
+        self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND)
+
+        other_root = self._create_pdf(title="other", checksum="other")
+        other_version = self._create_pdf(
+            title="other-v1",
+            checksum="other-v1",
+            root_document=other_root,
+        )
+        resp = self.client.get(
+            f"/api/documents/{root.id}/metadata/?version={other_version.id}",
+        )
+        self.assertEqual(resp.status_code, status.HTTP_404_NOT_FOUND)
+
     def test_metadata_returns_403_when_user_lacks_permission(self) -> None:
         owner = User.objects.create_user(username="owner")
         other = User.objects.create_user(username="other")
@@ -439,6 +465,39 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
         self.assertEqual(overrides.version_label, "New Version")
         self.assertEqual(overrides.actor_id, self.user.id)
 
+    def test_update_version_with_version_pk_normalizes_to_root(self) -> None:
+        root = Document.objects.create(
+            title="root",
+            checksum="root",
+            mime_type="application/pdf",
+        )
+        version = Document.objects.create(
+            title="v1",
+            checksum="v1",
+            mime_type="application/pdf",
+            root_document=root,
+        )
+        upload = self._make_pdf_upload()
+
+        async_task = mock.Mock()
+        async_task.id = "task-123"
+
+        with mock.patch("documents.views.consume_file") as consume_mock:
+            consume_mock.delay.return_value = async_task
+            resp = self.client.post(
+                f"/api/documents/{version.id}/update_version/",
+                {"document": upload, "version_label": "  New Version  "},
+                format="multipart",
+            )
+
+        self.assertEqual(resp.status_code, status.HTTP_200_OK)
+        self.assertEqual(resp.data, "task-123")
+        consume_mock.delay.assert_called_once()
+        input_doc, overrides = consume_mock.delay.call_args[0]
+        self.assertEqual(input_doc.root_document_id, root.id)
+        self.assertEqual(overrides.version_label, "New Version")
+        self.assertEqual(overrides.actor_id, self.user.id)
+
     def test_update_version_returns_500_on_consume_failure(self) -> None:
         root = Document.objects.create(
             title="root",
@@ -613,3 +672,39 @@ class TestDocumentVersioningApi(DirectoriesMixin, APITestCase):
 
         self.assertEqual(resp.status_code, status.HTTP_200_OK)
         self.assertEqual(resp.data["content"], "v1-content")
+
+
+class TestVersionAwareFilters(TestCase):
+    def test_title_content_filter_falls_back_to_content(self) -> None:
+        queryset = mock.Mock()
+        fallback_queryset = mock.Mock()
+        queryset.filter.side_effect = [FieldError("missing field"), fallback_queryset]
+
+        result = TitleContentFilter().filter(queryset, " latest ")
+
+        self.assertIs(result, fallback_queryset)
+        self.assertEqual(queryset.filter.call_count, 2)
+
+    def test_effective_content_filter_falls_back_to_content_lookup(self) -> None:
+        queryset = mock.Mock()
+        fallback_queryset = mock.Mock()
+        queryset.filter.side_effect = [FieldError("missing field"), fallback_queryset]
+
+        result = EffectiveContentFilter(lookup_expr="icontains").filter(
+            queryset,
+            " latest ",
+        )
+
+        self.assertIs(result, fallback_queryset)
+        first_kwargs = queryset.filter.call_args_list[0].kwargs
+        second_kwargs = queryset.filter.call_args_list[1].kwargs
+        self.assertEqual(first_kwargs, {"effective_content__icontains": "latest"})
+        self.assertEqual(second_kwargs, {"content__icontains": "latest"})
+
+    def test_effective_content_filter_returns_input_for_empty_values(self) -> None:
+        queryset = mock.Mock()
+
+        result = EffectiveContentFilter(lookup_expr="icontains").filter(queryset, "   ")
+
+        self.assertIs(result, queryset)
+        queryset.filter.assert_not_called()

src/documents/tests/test_api_documents.py

@@ -1242,6 +1242,38 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase):
         self.assertIsNone(overrides.document_type_id)
         self.assertIsNone(overrides.tag_ids)
 
+    def test_document_filters_use_latest_version_content(self) -> None:
+        root = Document.objects.create(
+            title="versioned root",
+            checksum="root",
+            mime_type="application/pdf",
+            content="root-content",
+        )
+        version = Document.objects.create(
+            title="versioned root",
+            checksum="v1",
+            mime_type="application/pdf",
+            root_document=root,
+            content="latest-version-content",
+        )
+
+        response = self.client.get(
+            "/api/documents/?content__icontains=latest-version-content",
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        results = response.data["results"]
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0]["id"], root.id)
+        self.assertEqual(results[0]["content"], version.content)
+
+        response = self.client.get(
+            "/api/documents/?title_content=latest-version-content",
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        results = response.data["results"]
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0]["id"], root.id)
+
     def test_create_wrong_endpoint(self) -> None:
         response = self.client.post(
             "/api/documents/",

src/documents/tests/test_bulk_edit.py

@@ -381,6 +381,55 @@ class TestBulkEdit(DirectoriesMixin, TestCase):
             [self.doc3.id, self.doc4.id, self.doc5.id],
         )
 
+    def test_delete_root_document_deletes_all_versions(self) -> None:
+        version = Document.objects.create(
+            checksum="A-v1",
+            title="A version",
+            root_document=self.doc1,
+        )
+
+        bulk_edit.delete([self.doc1.id])
+
+        self.assertFalse(Document.objects.filter(id=self.doc1.id).exists())
+        self.assertFalse(Document.objects.filter(id=version.id).exists())
+
+    def test_delete_version_document_keeps_root(self) -> None:
+        version = Document.objects.create(
+            checksum="A-v1",
+            title="A version",
+            root_document=self.doc1,
+        )
+
+        bulk_edit.delete([version.id])
+
+        self.assertTrue(Document.objects.filter(id=self.doc1.id).exists())
+        self.assertFalse(Document.objects.filter(id=version.id).exists())
+
+    def test_get_root_and_current_doc_mapping(self) -> None:
+        version1 = Document.objects.create(
+            checksum="B-v1",
+            title="B version 1",
+            root_document=self.doc2,
+        )
+        version2 = Document.objects.create(
+            checksum="B-v2",
+            title="B version 2",
+            root_document=self.doc2,
+        )
+
+        root_ids_by_doc_id = bulk_edit._get_root_ids_by_doc_id(
+            [self.doc2.id, version1.id, version2.id],
+        )
+        self.assertEqual(root_ids_by_doc_id[self.doc2.id], self.doc2.id)
+        self.assertEqual(root_ids_by_doc_id[version1.id], self.doc2.id)
+        self.assertEqual(root_ids_by_doc_id[version2.id], self.doc2.id)
+
+        root_docs, current_docs = bulk_edit._get_root_and_current_docs_by_root_id(
+            {self.doc2.id},
+        )
+        self.assertEqual(root_docs[self.doc2.id].id, self.doc2.id)
+        self.assertEqual(current_docs[self.doc2.id].id, version2.id)
+
     @mock.patch("documents.tasks.bulk_update_documents.delay")
     def test_set_permissions(self, m) -> None:
         doc_ids = [self.doc1.id, self.doc2.id, self.doc3.id]

src/documents/tests/test_consumer.py

@@ -98,11 +98,13 @@ class FaultyGenericExceptionParser(_BaseTestParser):
         raise Exception("Generic exception.")
 
 
-def fake_magic_from_file(file, *, mime=False):
+def fake_magic_from_file(file, *, mime=False):  # NOSONAR
     if mime:
         filepath = Path(file)
         if filepath.name.startswith("invalid_pdf"):
             return "application/octet-stream"
+        if filepath.name.startswith("valid_pdf"):
+            return "application/pdf"
         if filepath.suffix == ".pdf":
             return "application/pdf"
         elif filepath.suffix == ".png":
@@ -747,6 +749,65 @@ class TestConsumer(
         self.assertTrue(version.original_filename.endswith("_v0.pdf"))
         self.assertTrue(bool(version.content))
 
+    @override_settings(AUDIT_LOG_ENABLED=True)
+    @mock.patch("documents.consumer.load_classifier")
+    def test_consume_version_with_missing_actor_and_filename_without_suffix(
+        self,
+        m: mock.Mock,
+    ) -> None:
+        m.return_value = MagicMock()
+
+        with self.get_consumer(self.get_test_file()) as consumer:
+            consumer.run()
+
+        root_doc = Document.objects.first()
+        self.assertIsNotNone(root_doc)
+        assert root_doc is not None
+
+        version_file = self.get_test_file2()
+        status = DummyProgressManager(version_file.name, None)
+        overrides = DocumentMetadataOverrides(
+            filename="valid_pdf_version-upload",
+            actor_id=999999,
+        )
+        doc = ConsumableDocument(
+            DocumentSource.ApiUpload,
+            original_file=version_file,
+            root_document_id=root_doc.pk,
+        )
+
+        preflight = ConsumerPreflightPlugin(
+            doc,
+            overrides,
+            status,  # type: ignore[arg-type]
+            self.dirs.scratch_dir,
+            "task-id",
+        )
+        preflight.setup()
+        preflight.run()
+
+        consumer = ConsumerPlugin(
+            doc,
+            overrides,
+            status,  # type: ignore[arg-type]
+            self.dirs.scratch_dir,
+            "task-id",
+        )
+        consumer.setup()
+        try:
+            self.assertEqual(consumer.filename, "valid_pdf_version-upload_v0")
+            consumer.run()
+        finally:
+            consumer.cleanup()
+
+        version = (
+            Document.objects.filter(root_document=root_doc).order_by("-id").first()
+        )
+        self.assertIsNotNone(version)
+        assert version is not None
+        self.assertEqual(version.original_filename, "valid_pdf_version-upload_v0")
+        self.assertTrue(bool(version.content))
+
     @mock.patch("documents.consumer.load_classifier")
     def testClassifyDocument(self, m) -> None:
         correspondent = Correspondent.objects.create(
@@ -1359,6 +1420,19 @@ class TestMetadataOverrides(TestCase):
         base.update(incoming)
         self.assertTrue(base.skip_asn_if_exists)
 
+    def test_update_actor_and_version_label(self) -> None:
+        base = DocumentMetadataOverrides(
+            actor_id=1,
+            version_label="root",
+        )
+        incoming = DocumentMetadataOverrides(
+            actor_id=2,
+            version_label="v2",
+        )
+        base.update(incoming)
+        self.assertEqual(base.actor_id, 2)
+        self.assertEqual(base.version_label, "v2")
+
 
 class TestBarcodeApplyDetectedASN(TestCase):
     """

src/documents/tests/test_task_signals.py

@@ -232,3 +232,7 @@ class TestTaskSignalHandler(DirectoriesMixin, TestCase):
         self.assertEqual(add.call_count, 2)
         self.assertEqual(add.call_args_list[0].args[0].id, version.id)
         self.assertEqual(add.call_args_list[1].args[0].id, root.id)
+        self.assertEqual(
+            add.call_args_list[1].kwargs,
+            {"effective_content": version.content},
+        )

src/documents/tests/test_version_conditionals.py

@@ -0,0 +1,91 @@
+from types import SimpleNamespace
+from unittest import mock
+
+from django.test import TestCase
+
+from documents.conditionals import metadata_etag
+from documents.conditionals import preview_etag
+from documents.conditionals import thumbnail_last_modified
+from documents.models import Document
+from documents.tests.utils import DirectoriesMixin
+from documents.versioning import resolve_effective_document_by_pk
+
+
+class TestConditionals(DirectoriesMixin, TestCase):
+    def test_metadata_etag_uses_latest_version_for_root_request(self) -> None:
+        root = Document.objects.create(
+            title="root",
+            checksum="root-checksum",
+            archive_checksum="root-archive",
+            mime_type="application/pdf",
+        )
+        latest = Document.objects.create(
+            title="v1",
+            checksum="version-checksum",
+            archive_checksum="version-archive",
+            mime_type="application/pdf",
+            root_document=root,
+        )
+        request = SimpleNamespace(query_params={})
+
+        self.assertEqual(metadata_etag(request, root.id), latest.checksum)
+        self.assertEqual(preview_etag(request, root.id), latest.archive_checksum)
+
+    def test_resolve_effective_doc_returns_none_for_invalid_or_unrelated_version(
+        self,
+    ) -> None:
+        root = Document.objects.create(
+            title="root",
+            checksum="root",
+            mime_type="application/pdf",
+        )
+        other_root = Document.objects.create(
+            title="other",
+            checksum="other",
+            mime_type="application/pdf",
+        )
+        other_version = Document.objects.create(
+            title="other-v1",
+            checksum="other-v1",
+            mime_type="application/pdf",
+            root_document=other_root,
+        )
+
+        invalid_request = SimpleNamespace(query_params={"version": "not-a-number"})
+        unrelated_request = SimpleNamespace(
+            query_params={"version": str(other_version.id)},
+        )
+
+        self.assertIsNone(
+            resolve_effective_document_by_pk(root.id, invalid_request).document,
+        )
+        self.assertIsNone(
+            resolve_effective_document_by_pk(root.id, unrelated_request).document,
+        )
+
+    def test_thumbnail_last_modified_uses_effective_document_for_cache_key(
+        self,
+    ) -> None:
+        root = Document.objects.create(
+            title="root",
+            checksum="root",
+            mime_type="application/pdf",
+        )
+        latest = Document.objects.create(
+            title="v2",
+            checksum="v2",
+            mime_type="application/pdf",
+            root_document=root,
+        )
+        latest.thumbnail_path.parent.mkdir(parents=True, exist_ok=True)
+        latest.thumbnail_path.write_bytes(b"thumb")
+
+        request = SimpleNamespace(query_params={})
+        with mock.patch(
+            "documents.conditionals.get_thumbnail_modified_key",
+            return_value="thumb-modified-key",
+        ) as get_thumb_key:
+            result = thumbnail_last_modified(request, root.id)
+
+        self.assertIsNotNone(result)
+        get_thumb_key.assert_called_once_with(latest.id)

src/documents/versioning.py

@@ -0,0 +1,120 @@
+from __future__ import annotations
+
+from dataclasses import dataclass
+from enum import Enum
+from typing import Any
+
+from documents.models import Document
+
+
+class VersionResolutionError(str, Enum):
+    INVALID = "invalid"
+    NOT_FOUND = "not_found"
+
+
+@dataclass(frozen=True)
+class VersionResolution:
+    document: Document | None
+    error: VersionResolutionError | None = None
+
+
+def _document_manager(*, include_deleted: bool) -> Any:
+    return Document.global_objects if include_deleted else Document.objects
+
+
+def get_request_version_param(request: Any) -> str | None:
+    if hasattr(request, "query_params"):
+        return request.query_params.get("version")
+    return None
+
+
+def get_root_document(doc: Document, *, include_deleted: bool = False) -> Document:
+    # Use root_document_id to avoid a query when this is already a root.
+    # If root_document isn't available, fall back to the document itself.
+    if doc.root_document_id is None:
+        return doc
+    if doc.root_document is not None:
+        return doc.root_document
+
+    manager = _document_manager(include_deleted=include_deleted)
+    root_doc = manager.only("id").filter(id=doc.root_document_id).first()
+    return root_doc or doc
+
+
+def get_latest_version_for_root(
+    root_doc: Document,
+    *,
+    include_deleted: bool = False,
+) -> Document:
+    manager = _document_manager(include_deleted=include_deleted)
+    latest = manager.filter(root_document=root_doc).order_by("-id").first()
+    return latest or root_doc
+
+
+def resolve_requested_version_for_root(
+    root_doc: Document,
+    request: Any,
+    *,
+    include_deleted: bool = False,
+) -> VersionResolution:
+    version_param = get_request_version_param(request)
+    if not version_param:
+        return VersionResolution(
+            document=get_latest_version_for_root(
+                root_doc,
+                include_deleted=include_deleted,
+            ),
+        )
+
+    try:
+        version_id = int(version_param)
+    except (TypeError, ValueError):
+        return VersionResolution(document=None, error=VersionResolutionError.INVALID)
+
+    manager = _document_manager(include_deleted=include_deleted)
+    candidate = manager.only("id", "root_document_id").filter(id=version_id).first()
+    if candidate is None:
+        return VersionResolution(document=None, error=VersionResolutionError.NOT_FOUND)
+    if candidate.id != root_doc.id and candidate.root_document_id != root_doc.id:
+        return VersionResolution(document=None, error=VersionResolutionError.NOT_FOUND)
+    return VersionResolution(document=candidate)
+
+
+def resolve_effective_document(
+    request_doc: Document,
+    request: Any,
+    *,
+    include_deleted: bool = False,
+) -> VersionResolution:
+    root_doc = get_root_document(request_doc, include_deleted=include_deleted)
+    if get_request_version_param(request) is not None:
+        return resolve_requested_version_for_root(
+            root_doc,
+            request,
+            include_deleted=include_deleted,
+        )
+    if request_doc.root_document_id is None:
+        return VersionResolution(
+            document=get_latest_version_for_root(
+                root_doc,
+                include_deleted=include_deleted,
+            ),
+        )
+    return VersionResolution(document=request_doc)
+
+
+def resolve_effective_document_by_pk(
+    pk: int,
+    request: Any,
+    *,
+    include_deleted: bool = False,
+) -> VersionResolution:
+    manager = _document_manager(include_deleted=include_deleted)
+    request_doc = manager.only("id", "root_document_id").filter(pk=pk).first()
+    if request_doc is None:
+        return VersionResolution(document=None, error=VersionResolutionError.NOT_FOUND)
+    return resolve_effective_document(
+        request_doc,
+        request,
+        include_deleted=include_deleted,
+    )

src/documents/views.py

@@ -206,6 +206,11 @@ from documents.tasks import sanity_check
 from documents.tasks import train_classifier
 from documents.tasks import update_document_parent_tags
 from documents.utils import get_boolean
+from documents.versioning import VersionResolutionError
+from documents.versioning import get_latest_version_for_root
+from documents.versioning import get_request_version_param
+from documents.versioning import get_root_document
+from documents.versioning import resolve_requested_version_for_root
 from paperless import version
 from paperless.celery import app as celery_app
 from paperless.config import AIConfig
@@ -834,14 +839,6 @@ class DocumentViewSet(
         )
         return super().get_serializer(*args, **kwargs)
 
-    @staticmethod
-    def _get_root_doc(doc: Document) -> Document:
-        # Use root_document_id to avoid a query when this is already a root.
-        # If root_document isn't available, fall back to the document itself.
-        if doc.root_document_id is None:
-            return doc
-        return doc.root_document or doc
-
     @extend_schema(
         operation_id="documents_root",
         responses=inline_serializer(
@@ -861,7 +858,7 @@ class DocumentViewSet(
         except Document.DoesNotExist:
             raise Http404
 
-        root_doc = self._get_root_doc(doc)
+        root_doc = get_root_document(doc)
         if request.user is not None and not has_perms_owner_aware(
             request.user,
             "view_document",
@@ -896,7 +893,7 @@ class DocumentViewSet(
         content_doc = (
             self._resolve_file_doc(root_doc, request)
             if "version" in request.query_params
-            else self._get_latest_doc_for_root(root_doc)
+            else get_latest_version_for_root(root_doc)
         )
         content_updated = "content" in request.data
         updated_content = request.data.get("content") if content_updated else None
@@ -967,31 +964,17 @@ class DocumentViewSet(
         )
 
     def _resolve_file_doc(self, root_doc: Document, request):
-        version_param = request.query_params.get("version")
-        if version_param:
-            try:
-                version_id = int(version_param)
-            except (TypeError, ValueError):
-                raise NotFound("Invalid version parameter")
-            try:
-                candidate = Document.global_objects.select_related("owner").get(
-                    id=version_id,
-                )
-            except Document.DoesNotExist:
-                raise Http404
-            if (
-                candidate.id != root_doc.id
-                and candidate.root_document_id != root_doc.id
-            ):
-                raise Http404
-            return candidate
-        latest = Document.objects.filter(root_document=root_doc).order_by("id").last()
-        return latest or root_doc
-
-    @staticmethod
-    def _get_latest_doc_for_root(root_doc: Document) -> Document:
-        latest = Document.objects.filter(root_document=root_doc).order_by("-id").first()
-        return latest or root_doc
+        version_requested = get_request_version_param(request) is not None
+        resolution = resolve_requested_version_for_root(
+            root_doc,
+            request,
+            include_deleted=version_requested,
+        )
+        if resolution.error == VersionResolutionError.INVALID:
+            raise NotFound("Invalid version parameter")
+        if resolution.document is None:
+            raise Http404
+        return resolution.document
 
     def _get_effective_file_doc(
         self,
@@ -999,29 +982,50 @@ class DocumentViewSet(
         root_doc: Document,
         request: Request,
     ) -> Document:
-        # If a version is explicitly requested, use it. Otherwise:
-        # - if pk is a root document: serve newest version
-        # - if pk is a version: serve that version
-        if "version" in request.query_params:
-            return self._resolve_file_doc(root_doc, request)
-        return (
-            self._resolve_file_doc(root_doc, request)
-            if request_doc.root_document_id is None
-            else request_doc
-        )
+        if (
+            request_doc.root_document_id is not None
+            and get_request_version_param(request) is None
+        ):
+            return request_doc
+        return self._resolve_file_doc(root_doc, request)
 
-    def file_response(self, pk, request, disposition):
-        request_doc = Document.global_objects.select_related(
-            "owner",
-            "root_document",
-        ).get(id=pk)
-        root_doc = self._get_root_doc(request_doc)
+    def _resolve_request_and_root_doc(
+        self,
+        pk,
+        request: Request,
+        *,
+        include_deleted: bool = False,
+    ) -> tuple[Document, Document] | HttpResponseForbidden:
+        manager = Document.global_objects if include_deleted else Document.objects
+        try:
+            request_doc = manager.select_related(
+                "owner",
+                "root_document",
+            ).get(id=pk)
+        except Document.DoesNotExist:
+            raise Http404
+
+        root_doc = get_root_document(
+            request_doc,
+            include_deleted=include_deleted,
+        )
         if request.user is not None and not has_perms_owner_aware(
             request.user,
             "view_document",
             root_doc,
         ):
             return HttpResponseForbidden("Insufficient permissions")
+        return request_doc, root_doc
+
+    def file_response(self, pk, request, disposition):
+        resolved = self._resolve_request_and_root_doc(
+            pk,
+            request,
+            include_deleted=True,
+        )
+        if isinstance(resolved, HttpResponseForbidden):
+            return resolved
+        request_doc, root_doc = resolved
         file_doc = self._get_effective_file_doc(request_doc, root_doc, request)
         return serve_file(
             doc=file_doc,
@@ -1060,20 +1064,10 @@ class DocumentViewSet(
         condition(etag_func=metadata_etag, last_modified_func=metadata_last_modified),
     )
     def metadata(self, request, pk=None):
-        try:
-            request_doc = Document.objects.select_related(
-                "owner",
-                "root_document",
-            ).get(pk=pk)
-            root_doc = self._get_root_doc(request_doc)
-            if request.user is not None and not has_perms_owner_aware(
-                request.user,
-                "view_document",
-                root_doc,
-            ):
-                return HttpResponseForbidden("Insufficient permissions")
-        except Document.DoesNotExist:
-            raise Http404
+        resolved = self._resolve_request_and_root_doc(pk, request)
+        if isinstance(resolved, HttpResponseForbidden):
+            return resolved
+        request_doc, root_doc = resolved
 
         # Choose the effective document (newest version by default,
         # or explicit via ?version=).
@@ -1246,19 +1240,12 @@ class DocumentViewSet(
         condition(etag_func=preview_etag, last_modified_func=preview_last_modified),
     )
     def preview(self, request, pk=None):
-        try:
-            request_doc = Document.objects.select_related(
-                "owner",
-                "root_document",
-            ).get(id=pk)
-            root_doc = self._get_root_doc(request_doc)
-            if request.user is not None and not has_perms_owner_aware(
-                request.user,
-                "view_document",
-                root_doc,
-            ):
-                return HttpResponseForbidden("Insufficient permissions")
+        resolved = self._resolve_request_and_root_doc(pk, request)
+        if isinstance(resolved, HttpResponseForbidden):
+            return resolved
+        request_doc, root_doc = resolved
 
+        try:
             file_doc = self._get_effective_file_doc(request_doc, root_doc, request)
 
             return serve_file(
@@ -1267,30 +1254,24 @@ class DocumentViewSet(
                 and file_doc.has_archive_version,
                 disposition="inline",
             )
-        except (FileNotFoundError, Document.DoesNotExist):
+        except FileNotFoundError:
             raise Http404
 
     @action(methods=["get"], detail=True, filter_backends=[])
     @method_decorator(cache_control(no_cache=True))
     @method_decorator(last_modified(thumbnail_last_modified))
     def thumb(self, request, pk=None):
+        resolved = self._resolve_request_and_root_doc(pk, request)
+        if isinstance(resolved, HttpResponseForbidden):
+            return resolved
+        request_doc, root_doc = resolved
+
         try:
-            request_doc = Document.objects.select_related(
-                "owner",
-                "root_document",
-            ).get(id=pk)
-            root_doc = self._get_root_doc(request_doc)
-            if request.user is not None and not has_perms_owner_aware(
-                request.user,
-                "view_document",
-                root_doc,
-            ):
-                return HttpResponseForbidden("Insufficient permissions")
             file_doc = self._get_effective_file_doc(request_doc, root_doc, request)
             handle = file_doc.thumbnail_file
 
             return HttpResponse(handle, content_type="image/webp")
-        except (FileNotFoundError, Document.DoesNotExist):
+        except FileNotFoundError:
             raise Http404
 
     @action(methods=["get"], detail=True)
@@ -1592,11 +1573,15 @@ class DocumentViewSet(
         serializer.is_valid(raise_exception=True)
 
         try:
-            doc = Document.objects.select_related("owner").get(pk=pk)
+            request_doc = Document.objects.select_related(
+                "owner",
+                "root_document",
+            ).get(pk=pk)
+            root_doc = get_root_document(request_doc)
             if request.user is not None and not has_perms_owner_aware(
                 request.user,
                 "change_document",
-                doc,
+                root_doc,
             ):
                 return HttpResponseForbidden("Insufficient permissions")
         except Document.DoesNotExist:
@@ -1621,7 +1606,7 @@ class DocumentViewSet(
             input_doc = ConsumableDocument(
                 source=DocumentSource.ApiUpload,
                 original_file=temp_file_path,
-                root_document_id=doc.pk,
+                root_document_id=root_doc.pk,
             )
 
             overrides = DocumentMetadataOverrides()
@@ -1635,7 +1620,7 @@ class DocumentViewSet(
                 overrides,
             )
             logger.debug(
-                f"Updated document {doc.id} with new version",
+                f"Updated document {root_doc.id} with new version",
             )
             return Response(async_task.id)
         except Exception as e:
@@ -1672,7 +1657,7 @@ class DocumentViewSet(
                 "owner",
                 "root_document",
             ).get(pk=pk)
-            root_doc = self._get_root_doc(root_doc)
+            root_doc = get_root_document(root_doc)
         except Document.DoesNotExist:
             raise Http404
 
@@ -1821,7 +1806,7 @@ class ChatStreamingView(GenericAPIView):
     ),
 )
 class UnifiedSearchViewSet(DocumentViewSet):
-    def __init__(self, *args, **kwargs) -> None:
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
         super().__init__(*args, **kwargs)
         self.searcher = None
 
@@ -2032,6 +2017,8 @@ class BulkEditView(PassUserMixin):
         "modify_custom_fields": "custom_fields",
         "set_permissions": None,
         "delete": "deleted_at",
+        # These operations create new documents/versions no longer altering
+        # fields on the selected document in place
         "rotate": None,
         "delete_pages": None,
         "split": None,