feat(documents): Add invoice support to document linking#378
Conversation
- Updated useDocumentLinks hook to accept entityType and entityId instead of hardcoded workItemId - Changed LinkedDocumentsSection to support work_item, household_item, and invoice entity types - Added entity-specific copy for empty states, picker subtitle, and unlink confirmations - Removed margin-top from LinkedDocumentsSection.section CSS (parent gap handles spacing) - Updated WorkItemDetailPage to pass entityType="work_item" and entityId - Added LinkedDocumentsSection to InvoiceDetailPage to display documents linked to invoices Fixes #358 Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
…nking for invoices Update useDocumentLinks.test.ts to match new 2-arg signature (entityType + entityId), add tests for invoice entity type, and fix waitFor conditions that relied on isLoading starting as true (it starts as false). Update LinkedDocumentsSection.test.tsx to match new props (entityType + entityId instead of workItemId), add invoice entity type tests and work_item backwards-compatibility tests, and add makeInvoiceLink helper fixture. Create InvoiceDetailPage.test.tsx covering loading/error states, successful render, and LinkedDocumentsSection integration (entityType=invoice, entityId=invoiceId). Fixes #358 Co-Authored-By: Claude qa-integration-tester (Sonnet 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.6) <noreply@anthropic.com>
- useDocumentLinks: include isLoading check inside waitFor to avoid race condition with async state updates - InvoiceDetailPage: use getAllByText for status badge since it renders in two places (page header + info list) - LinkedDocumentsSection: skip invoice unlink copy test pending fix for bug #379 (hardcoded "this work item" in unlink modal) Co-Authored-By: Claude qa-integration-tester (Sonnet 4.6) <noreply@anthropic.com>
The unlink confirmation dialog hardcoded "this work item" instead of using the entity-specific copy from entityCopy[entityType].unlinkBody. This caused invoice document unlink dialogs to incorrectly say "will be removed from this work item" instead of "this invoice". Also unskips the previously-skipped test for this scenario. Fixes #379 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
steilerDev
left a comment
There was a problem hiding this comment.
[qa-integration-tester] Unit test coverage complete for Story #358.
Test Files Updated/Created
client/src/hooks/useDocumentLinks.test.ts (modified)
- Updated all
useDocumentLinks('wi-abc')→useDocumentLinks('work_item', 'wi-abc')to match new 2-arg signature - Added 2 new tests for invoice entity type:
calls listDocumentLinks with invoice entityTypeandcalls createDocumentLink with invoice entityType - Fixed
waitForconditions to avoid race conditions withisLoadingstate
client/src/components/documents/LinkedDocumentsSection.test.tsx (modified)
- Updated all
<LinkedDocumentsSection workItemId="wi-abc" />→<LinkedDocumentsSection entityType="work_item" entityId="wi-abc" /> - Added
makeInvoiceLinktest helper - Added 4 new invoice entity type tests (picker subtitle, empty state, duplicate error)
- Added 2 work_item backwards-compatibility tests
- Skipped invoice unlink copy test pending fix for bug #379
client/src/pages/InvoiceDetailPage/InvoiceDetailPage.test.tsx (new)
- Loading state, 3 error states (404, 500, network)
- 6 successful render assertions (heading, vendor, amount, status badge, invoice details)
- 3 LinkedDocumentsSection integration tests (renders, entityType=invoice, entityId matches invoice ID)
Bug Found
Bug #379: LinkedDocumentsSection.tsx unlink modal body hardcodes "this work item" instead of using {copy.unlinkBody}, so invoice unlink dialogs incorrectly say "this work item" instead of "this invoice". One test is skipped pending this fix.
CI Status
All Quality Gates pass after fixing 3 test assertions:
waitForrace condition forisLoadingcheckgetByText('Pending')→getAllByText('Pending')(status badge renders in 2 places)- Skipped invoice unlink body test (blocked by bug #379)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
steilerDev
left a comment
There was a problem hiding this comment.
[security-engineer] Security review for PR #378 — Document Linking for Invoices (Story #358)
Verdict: No blocking findings. Approved from a security perspective.
Review Summary
This PR parameterizes useDocumentLinks and LinkedDocumentsSection to accept entityType: DocumentLinkEntityType and entityId: string, then integrates the component into InvoiceDetailPage. The change is additive and structurally identical to the existing work_item usage reviewed previously.
Checklist
- XSS: No
dangerouslySetInnerHTML,innerHTML, oreval. All user-supplied strings (doc.title,unlinkTarget.document?.title,hook.error) are rendered as React JSX text nodes and escaped by React. Thecopyobject (pickerSubtitle, unlinkBody, emptyBody) is a hard-codedas constmap keyed by a server-validated enum — no interpolation of user data into display copy. - Injection:
entityTypeis a TypeScript union ('work_item' | 'household_item' | 'invoice'). The server enforces anenumconstraint on both the POST body and GET querystring indocumentLinks.ts:24,37. No injection vector. - Authorization:
InvoiceDetailPageonly mountsLinkedDocumentsSectionafter successfully fetching an invoice by ID (line 82). TheuseDocumentLinkshook calls the existing document-links API which enforces authentication via the establishedrequireAuthpreHandler. No auth bypass introduced. - IDOR: The
entityIdpassed toLinkedDocumentsSectionisid!fromuseParams— the same ID used to load the invoice. The server's document-links service does not perform cross-entity ownership validation (this was a pre-existing informational finding from the original document linking audit, not introduced here). - Client-side storage: No sensitive data stored in localStorage/sessionStorage.
- New dependencies: None added.
- Error handling: Errors from
ApiClientErrorandNetworkErrorare mapped to generic user-facing strings — no internal detail leakage. -
id!non-null assertion (line 334 ofInvoiceDetailPage.tsx): The!is used on a route param that is guaranteed to be present by React Router's route definition. Consistent with existing usage patterns in the codebase (e.g.,WorkItemDetailPage). Not a security concern.
No new security issues identified. The entity type enum is enforced at both the TypeScript type level and the server's AJV schema validation, which is the correct defense-in-depth pattern.
steilerDev
left a comment
There was a problem hiding this comment.
[product-architect]
Architecture Review: PR #378 — Invoice Document Linking
Verdict: Approved (posted as comment — cannot approve own PR)
This is a clean, well-scoped frontend refactor that parameterizes the document linking subsystem to support multiple entity types. The changes are fully compliant with the established architecture.
What was verified
API Contract compliance: The useDocumentLinks hook now passes entityType and entityId to listDocumentLinks() and createDocumentLink(), which matches the Wiki API Contract for GET /api/document-links?entityType=...&entityId=... and POST /api/document-links. The supported entity types (work_item, household_item, invoice) match the contract exactly.
Shared type usage: The component and hook correctly import DocumentLinkEntityType from @cornerstone/shared, which is defined as 'work_item' | 'household_item' | 'invoice'. The as const satisfies Record<DocumentLinkEntityType, ...> pattern in the copy map ensures compile-time exhaustiveness — adding a new entity type to the union will produce a type error if copy is not provided. Good pattern.
Component architecture: The LinkedDocumentsSection props change from { workItemId: string } to { entityType: DocumentLinkEntityType; entityId: string } is the correct generalization. Entity-specific copy (subtitle, unlink body, empty state) is cleanly separated via a lookup map rather than conditionals.
CSS change: Removing margin-top from the section and relying on parent layout gap is the right approach for a reusable component.
Test coverage: All three test files are thorough:
useDocumentLinks.test.ts— tests bothwork_itemandinvoiceentity types for fetch, addLink, and error paths (15 tests)LinkedDocumentsSection.test.tsx— adds invoice-specific describe block covering subtitle, unlink text, empty state, and duplicate error copy (4 tests), plus backwards-compatibility block for work_item (2 tests)InvoiceDetailPage.test.tsx— new file with 10 tests covering loading, error states, successful render, and LinkedDocumentsSection integration (verifies correctentityTypeandentityIdprops)
No security concerns: No new API surface, no auth changes, no user input handling changes. The id! non-null assertion on line 334 of InvoiceDetailPage.tsx is acceptable — the route parameter is guaranteed by React Router's :id pattern, and the component only renders after successful invoice load.
Wiki status: No wiki updates needed — the API contract already documents entityType: 'invoice' as a supported value for document link endpoints. The backend endpoints were already in place; this PR only wires up the frontend.
steilerDev
left a comment
There was a problem hiding this comment.
[ux-designer]
Design compliance review for PR #378 — Document Linking for Invoices (Story #358).
Scope
This PR has minimal visual surface area: one CSS line removed, one component parameterized with entity-specific copy strings, and one new page integration. All visual rendering is delegated to the existing LinkedDocumentsSection component from Story 8.4.
Token Adherence
Pass. The only CSS change is the removal of margin-top: var(--spacing-8) from .section — this was already using a token and the removal is correct: spacing is now governed by the parent flex gap (--spacing-6) in InvoiceDetailPage.module.css's .content layout, consistent with how WorkItemDetailPage handles its section spacing.
No new hardcoded values were introduced in this PR.
Visual Consistency
Pass. LinkedDocumentsSection is rendered below the "Invoice Details" card section within the .content flex column, exactly mirroring the placement pattern used on WorkItemDetailPage. The component's visual appearance is identical across both entity types — same card shell, same card strip grid, same modal patterns — which is the correct behaviour.
Entity-specific copy strings (picker subtitle, unlink body, empty state body) are all plain text with no inline styles; they slot into existing styled <p> elements without any token deviations.
Dark Mode
Pass. No new color values were added. All colors in the component and page CSS continue to use semantic CSS custom properties that switch correctly in [data-theme="dark"].
Responsive Behaviour
Pass. No new layout rules were introduced. The InvoiceDetailPage .content flex column already handles mobile stacking (gap: var(--spacing-4) at max-width: 767px). The LinkedDocumentsSection responsive rules (single-column card strip at mobile, full-viewport modal sheet at < 768px) remain unchanged and apply identically for the invoice entity type.
Accessibility
Pass. No new interactive elements or ARIA structures were introduced. The existing aria-labelledby="documents-section-title" on the <section>, aria-live="polite" on the announcement region, and the full keyboard/focus management remain intact. The invoice-specific copy strings contain no ARIA label changes — they appear only in visible <p> text where plain prose is appropriate.
Informational Note — Pre-existing Issues
The following hardcoded font-size values exist in LinkedDocumentsSection.module.css and were present before this PR. They are out of scope for this review but should be addressed in a follow-up:
- Line 85:
font-size: 3rem(.emptyIcon) — no token equivalent exists at this size; acceptable as a decorative icon override, but worth documenting - Line 118:
font-size: 1.5rem(.notConfiguredIcon) — same rationale - Line 235:
font-size: 1.75rem(.modalClose) — no token equivalent - Line 259:
font-size: 0.9375rem(.modalText) — should usevar(--font-size-base)(1rem) orvar(--font-size-sm)(0.875rem)
Verdict
All changes in this PR are clean and compliant with the design system. The parameterization approach (compile-time entityCopy lookup keyed by DocumentLinkEntityType) is the correct way to vary copy without introducing new visual surface area.
Approved.
steilerDev
left a comment
There was a problem hiding this comment.
[product-owner] PR Review for Story #358 — Document Linking for Invoices
Verdict: APPROVED
All 10 acceptance criteria verified against the diff. This is a clean, well-scoped implementation that correctly parameterizes the existing LinkedDocumentsSection component for invoice entity support.
Acceptance Criteria Verification
| # | Criterion | Status | Evidence |
|---|---|---|---|
| 1 | Invoice detail/edit view displays "Documents" section | PASS | InvoiceDetailPage.tsx adds <LinkedDocumentsSection entityType="invoice" entityId={id!} /> after the Invoice Details section |
| 2 | Each linked document shows thumbnail, title, date, tags | PASS | Reuses LinkedDocumentCard from Story 8.4 — no changes needed |
| 3 | "Add Document" opens document browser modal (Story 8.3) | PASS | Existing DocumentBrowser in mode="modal" with invoice-specific subtitle: "Select a document from Paperless-ngx to link to this invoice." |
| 4 | Link created via API without full page reload | PASS | useDocumentLinks calls createDocumentLink({ entityType, entityId, paperlessDocumentId }) and refreshes via setFetchCount |
| 5 | "Unlink" with confirmation + "View in Paperless-ngx" | PASS | Unlink modal now uses {copy.unlinkBody} for entity-specific text (bug #379 fixed in commit ff50ea2f). "View in Paperless-ngx" from LinkedDocumentCard |
| 6 | Clicking thumbnail/title opens preview/detail | PASS | Handled by LinkedDocumentCard (Story 8.4, unchanged) |
| 7 | Empty state when no documents linked | PASS | Invoice-specific copy: "Link invoice PDFs, receipts, and related documents from Paperless-ngx to keep everything in one place." |
| 8 | Paperless-ngx not-configured state | PASS | Existing not-configured banner in LinkedDocumentsSection works for all entity types |
| 9 | Uses same reusable component as work items, parameterized by entity type | PASS | Props changed from workItemId to entityType + entityId. WorkItemDetailPage updated. entityCopy record provides entity-specific strings |
| 10 | Same reusable linked-documents component (restated) | PASS | Single component, parameterized — no code duplication |
Additional Checks
- Bug fix #379 included: The hardcoded "this work item" in the unlink confirmation modal is now parameterized via
{copy.unlinkBody}. Test unskipped and passing. - Test authorship: Production code by
frontend-developer (Haiku 4.5), tests byqa-integration-tester (Sonnet 4.6)— correct per CLAUDE.md. - Security review: Present (security-engineer, no blocking findings).
- QA review: Present (qa-integration-tester, 1 low finding — bug #379, now fixed).
- Scope discipline: PR stays within story scope. The
household_itementry inentityCopyis forward-looking prep for EPIC-04/Story 8.6 but is minimal and consistent with the parameterization pattern. No scope creep. - CSS change:
margin-top: var(--spacing-8)removed from.section— aligns with UX spec recommendation to let parent flex-gap control spacing. VerifiedWorkItemDetailPagewraps in.sectiondiv, so spacing is maintained. - No skipped tests: Confirmed no
it.skip,xit, orxdescriberemain in test files. - Backward compatibility:
WorkItemDetailPageupdated to passentityType="work_item" entityId={id!}— existing work item document linking continues to work.
CI Status
Quality Gates and Docker checks are still pending at time of review. Approval is conditional on CI passing.
Observations (non-blocking)
- The
household_itemcopy inentityCopyis proactive prep — acceptable since it is part of theDocumentLinkEntityTypeunion and costs nothing to include now. - The
InvoiceDetailPage.test.tsxmocksLinkedDocumentsSectionas a stub and verifies props viadata-*attributes — this is an appropriate integration test pattern.
|
🎉 This PR is included in version 1.10.0-beta.80 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.11.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
LinkedDocumentsSectionanduseDocumentLinkshook to support multiple entity types (work_item,invoice,household_item)LinkedDocumentsSectionto theInvoiceDetailPagewithentityType="invoice"WorkItemDetailPageto pass the new props formatmargin-topfrom section CSS (parent gap handles spacing)Fixes #358
Fixes #379
Test plan
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com