Require document on internal stock transfer (bin-aware)

[gqcorneby]

📌 References

• Issue: #869ctkw25 CU-869ctkw25

Purpose

• Items in quarantine requires certification before being allowed to be moved out for distribution.
• Items in distribution that go expired will need some proof or documentation. To "tag" them as expired prior to disposal, they should be transferred to a dedicated bin and upload supporting documents.

📝 Implementation

• Custom backend service (CustomStockTransferDocumentService) under org.pih.warehouse.custom.stocktransferdocuments.*
• Custom frontend panel under src/js/custom/stockTransferDocuments/
• Custom API endpoint GET/POST /api/custom/stockTransfers/{id}/documents
• Two new activity codes: REQUIRE_TRANSFER_OUT_DOCUMENT (outbound) and REQUIRE_TRANSFER_IN_DOCUMENT (inbound)
• Bin-aware enforcement: checks order header origin/destination AND every orderItem's originBinLocation/destinationBinLocation
• Backend gate via StockTransferService.completeStockTransfer() delegation
• Frontend gate: fail-closed (Complete button disabled until panel confirms)
• Read-only Documents tab on stockTransfer/show.gsp via custom template
• updated inventory stock cards action to conditionally hide transfer stock and hide bins in selection that have corresponding activity codes
• 15 Spock unit tests + 8 integration tests + 7 Jest tests

✨ Description of Change

Description:

Adds document upload/download to stock transfers with optional enforcement. Admins can enable REQUIRE_TRANSFER_OUT_DOCUMENT or REQUIRE_TRANSFER_IN_DOCUMENT on a parent depot OR a specific bin (e.g., Quarantine). When enabled, the stock transfer cannot be completed until at least one document is attached.

Key design decisions:

• Bin-aware: the validator walks both order-header locations and every order item's bin locations, so the flag can be set on a Quarantine bin without enabling it depot-wide
• Fail-closed frontend: customCanComplete defaults to false; the panel must explicitly enable the Complete button after loading
• Upstream isolation: all new code under custom/ paths; upstream files have surgical, documented edits (see design.md upstream touch points)

📹 Screenshots/Screen capture

Inventory Actions

2026-04-16.12-10-58.mp4

Hide expiry bin because it requires documents to transfer in

Hide transfer stock option because it requires documents to transfer out

🔥 Notes to the tester

To run

cd docker && docker compose up -d db
./gradlew bootRun -Dserver.port=8081

1. Enable REQUIRE_TRANSFER_OUT_DOCUMENT on a bin location (e.g., Quarantine) via Admin > Locations > edit location > Supported Activities
2. Create a stock transfer picking items from that bin
3. On the Check page, verify the Supporting Documents section is expanded and shows a warning
4. Verify the Complete button is disabled
5. Upload a document via the dropzone, click Upload
6. Verify the Complete button becomes enabled
7. Complete the transfer — verify redirect to stockTransfer/show with a Documents tab showing the upload
8. Test without the activity code enabled — verify completion works normally (no regression)
9. Without the activity code, verify the Supporting Documents section is collapsed by default
10. Stock card — outbound block: go to the stock card for an item in the Quarantine bin. Verify the "Transfer Stock" action is not shown in the action menu
11. Stock card — outbound allowed: go to the stock card for an item in a bin without the flag. Verify "Transfer Stock" is shown
12. Enable REQUIRE_TRANSFER_IN_DOCUMENT on a bin (e.g., a Controlled bin). Open the stock card transfer dialog for any item — verify that bin is not listed in the
    destination bin dropdown
13. Verify bins without the flag still appear in the destination dropdown

[xurxodev]

thanks @gqcorneby

Disclaimer: I have only reviewed the code in this PR — I have not run it or tested it functionally. I don't have prior knowledge of OpenBoxes or Groovy, but I have identified common patterns from other technologies (MVC with services). I leaned on Claude to assist with this review, abstracting away the low-level technical aspects so I could focus on higher-level concerns.

1. Should fix

• File upload has no MIME/size guard, client or server. src/js/custom/stockTransferDocuments/components/StockTransferDocumentsPanel.jsx:176 uses <Dropzone> without accept or maxSize. grails-app/services/.../CustomStockTransferDocumentService.groovy:61-79 (uploadDocument) reads fileContents.bytes straight into a Document with no content-type allowlist, no size cap, and no filename sanitization. The repo's web/security.md explicitly requires both ("Validate MIME type and size on the client for UX, and always validate again server-side. react-dropzone is the standard — use its accept and maxSize props"). Add an allowlist (e.g. application/pdf,image/*) and a max size (e.g. 10 MB) on both sides; reject with 400 + i18n error on the server.

• Re-uploading after a partial failure duplicates already-uploaded files. StockTransferDocumentsPanel.jsx:95-115 runs uploads serially and on catch only sets uploadError, leaving the full pendingFiles array intact. If file 1/3 succeeds and file 2/3 fails, hitting "Upload" again resends file 1, creating a duplicate Document. Drop each file from pendingFiles as soon as its upload resolves, or track success/failure per file and only retry the failed ones.

• Fetch error is invisible to the user when the panel is collapsed. StockTransferDocumentsPanel.jsx:67-71, 143-157 — on fetch failure the panel reports canComplete=false (button disabled) but the Warning is rendered inside {!collapsed && ...} and the panel only auto-expands on documentRequired, not on fetchError. Users see a disabled "Complete" button with no explanation. Either auto-expand on fetchError, or render the warning above the collapsible body.

2. Recommendations non blocking

• CustomStockTransferDocumentController.upload swallows the service's IllegalArgumentException. grails-app/controllers/.../CustomStockTransferDocumentController.groovy:50-62 — if uploadDocument throws (no order, save fails), the controller has no try/catch and the user gets a default Grails 500 error page. Wrap in a try/catch and render a JSON errorMessage so the global apiClient interceptor can surface a useful toast.

[gqcorneby]

Thanks @xurxodev! Pushed updates to address the comments

File upload has no MIME/size guard, client or server.
Guards added.

• PDF / image / Word / Excel / CSV / allowlist (MIME + extension)
• sanitizeFilename() that strips path components, ISO control chars, and Windows-unsafe chars (<>:"|?*), then truncates to 255
• Guards implemented in frontend and backend

Re-uploading after a partial failure duplicates already-uploaded files

• per-file try/catch on the frontend upload chain (only failed files stay pending)
• server-side dedup by (filename, size) for the network-flake-but-server-persisted case

Fetch error is invisible to the user when the panel is collapsed

• auto-expand on fetchError

CustomStockTransferDocumentController.upload swallows the service's IllegalArgumentException

• added try/catch for IllegalArgumentException and general catch and render as JSON errorMessage