TJK: Require document on internal stock transfer (bin-aware)

.claude/rules/upstream-entity-extension.md

@@ -69,6 +69,9 @@ Still zero edits to upstream `Container.groovy`.
 | Adding a Liquibase migration that does `addColumn(tableName: 'container', ...)` | Modifies the upstream schema directly. Merge risk if upstream adds a same-named column later. |
 | Groovy trait applied to upstream domain class via AST | Changes the upstream class's bytecode shape at runtime. |
 | Subclassing upstream domain class (`class CustomContainer extends Container`) | Creates an STI/table-per-hierarchy coupling that changes the upstream table's semantics. |
+| Adding new values to upstream enums (`RequisitionStatus`, `ShipmentStatusCode`, `LotStatusCode`, etc.) | Every consumer of the enum (frontend filters, status badges, switch statements, reports) must be aware of the new value. Upstream merges where they add their own values to the same enum cause guaranteed conflicts. **Express new state in a custom side-table** (e.g. `custom_approval_decision.status` enum), and have the chain *terminate* in an existing upstream status — never widen the upstream domain. |
+| Adding new values to `RoleType` enum | Same reason. Acceptable *only* when the value is genuinely needed for `findUsersByRoleTypes` recipient lookups (e.g. `ROLE_*_NOTIFICATION`); document the touch point in the OpenSpec `design.md` and accept the merge cost. Default answer: reuse an existing `Role`. |
+| Adding new values to `ActivityCode` for feature toggles | Same reason. Prefer external config (`openboxes-config.properties` keys) for boolean feature flags. Add to `ActivityCode` *only* when the toggle is genuinely per-location and the existing per-location infrastructure (admin UI, `Location.supports()`, location-type seeding) buys you something material. |
 
 ## When the rule does NOT apply
 
@@ -85,3 +88,13 @@ Still zero edits to upstream `Container.groovy`.
 ## Quick self-check before writing a migration
 
 If your custom Liquibase changeset contains `addColumn(tableName: '<upstream-table>', ...)`, stop. Replace it with `createTable(tableName: 'custom_<feature>', ...)` that has a `FOREIGN KEY ... REFERENCES <upstream-table>(id)` and a `UNIQUE` constraint on that FK column (for 1-to-1) or an index (for 1-to-many).
+
+## Quick self-check before naming a new domain concept
+
+Before introducing a new field, table column, or `Location.<something>` semantic, **grep upstream for the term you're about to use** and confirm it isn't already taken with different semantics. Real cases that have bitten this fork:
+
+- `Location.parentLocation` already exists upstream — but it expresses **physical containment** (bin → zone → warehouse), not **organizational hierarchy** (district → region → national). A custom feature that wanted "parent" for org hierarchy needed a separate side-table (`custom_location_admin_hierarchy.admin_parent_id`) to avoid colliding with upstream's `LocationService.findInternalLocation`, `getZones`, `getBinLocations`, and the picklist resolver, all of which read `parentLocation` as bin/zone.
+- `LotStatusCode` includes `ON_HOLD`, `QUARANTINED`, `EXPIRED`, etc. — **only `RECALLED` is referenced anywhere in the codebase**; the others are dead enum values. The actual "hold" mechanism is `ActivityCode.HOLD_STOCK` on a bin `Location`, not the lot status. A custom feature reaching for `LotStatusCode.ON_HOLD` would build on dead code.
+- `requisition.approvers` is a `Person` collection, not `User`. Custom approval logic that resolves users by role and compares against the approvers list needs a `User → Person` hop.
+
+**The check is simple: `grep -rn '<term>' grails-app/ src/main/`** before committing to a name. If the term shows up with surprising semantics, pick a different name (or build a side-table that owns the new semantic cleanly).

grails-app/controllers/org/pih/warehouse/UrlMappings.groovy

@@ -610,6 +610,15 @@ class UrlMappings {
             action = [DELETE: "removeAllItems"]
         }
 
+        // Custom: stock-transfer-document-upload
+        "/api/custom/stockTransfers/$id/documents"(parseRequest: true) {
+            controller = "customStockTransferDocument"
+            action = [GET: "list", POST: "upload"]
+        }
+        "/custom/stockTransferDocuments/refreshFilteredBinLocations"(controller: "customStockTransferDocument") {
+            action = [GET: "refreshFilteredBinLocations"]
+        }
+
         // Requirement API
 
         "/api/requirements"(parseRequest: true) {

grails-app/controllers/org/pih/warehouse/custom/stockTransferDocuments/CustomStockTransferDocumentController.groovy

@@ -0,0 +1,93 @@
+package org.pih.warehouse.custom.stockTransferDocuments
+
+import grails.converters.JSON
+import org.pih.warehouse.core.ActivityCode
+import org.pih.warehouse.core.Location
+import org.pih.warehouse.order.Order
+import org.springframework.context.i18n.LocaleContextHolder
+import org.springframework.web.multipart.MultipartFile
+import org.springframework.web.multipart.MultipartHttpServletRequest
+
+class CustomStockTransferDocumentController {
+
+    def customStockTransferDocumentService
+    def locationService
+    def messageSource
+
+    def list() {
+        Order order = Order.get(params.id)
+        if (!order) {
+            response.status = 404
+            render([errorMessage: "No stock transfer found for order ID ${params.id}"] as JSON)
+            return
+        }
+
+        render([
+            data: [
+                documentRequired: customStockTransferDocumentService.isDocumentRequired(order),
+                documents       : customStockTransferDocumentService.listDocuments(order),
+            ],
+        ] as JSON)
+    }
+
+    def refreshFilteredBinLocations() {
+        Location location = Location.get(params.id)
+        List<Location> bins = []
+        if (location?.hasBinLocationSupport()) {
+            bins = locationService.getBinLocations(location)
+                .findAll { !it.supports(ActivityCode.REQUIRE_TRANSFER_IN_DOCUMENT) }
+                .sort { it?.name?.toLowerCase() }
+        }
+
+        render g.select(
+            name: params.name ?: 'otherBinLocation.id',
+            'class': 'chzn-select-deselect',
+            noSelection: ['': g.message(code: 'default.label')],
+            from: bins,
+            optionKey: 'id',
+            optionValue: 'name',
+        )
+    }
+
+    def upload() {
+        MultipartFile fileContents = (request instanceof MultipartHttpServletRequest)
+            ? (request as MultipartHttpServletRequest).getFile("fileContents")
+            : null
+        if (!fileContents || fileContents.empty) {
+            response.status = 400
+            render([errorMessage: "fileContents is required"] as JSON)
+            return
+        }
+
+        try {
+            customStockTransferDocumentService.uploadDocument(params.id, fileContents)
+            render([data: "Document was uploaded successfully"] as JSON)
+        } catch (UploadValidationException ex) {
+            log.warn "custom_stock_transfer_document_upload_rejected orderId=${params.id}" +
+                    " code=${ex.messageCode} originalFilename=${fileContents.originalFilename}" +
+                    " contentType=${fileContents.contentType} size=${fileContents.size}"
+            response.status = 400
+            render([errorMessage: resolveMessage(ex)] as JSON)
+        } catch (IllegalArgumentException ex) {
+            log.warn "custom_stock_transfer_document_upload_bad_request orderId=${params.id} message=${ex.message}"
+            response.status = 404
+            render([errorMessage: ex.message] as JSON)
+        } catch (Exception ex) {
+            log.error "custom_stock_transfer_document_upload_failed orderId=${params.id}", ex
+            response.status = 500
+            render([errorMessage: 'Document upload failed. Please try again.'] as JSON)
+        }
+    }
+
+    private String resolveMessage(UploadValidationException ex) {
+        try {
+            return messageSource.getMessage(
+                    ex.messageCode,
+                    ex.messageArgs,
+                    ex.message,
+                    LocaleContextHolder.locale)
+        } catch (Exception ignore) {
+            return ex.message
+        }
+    }
+}

grails-app/i18n/messages.properties

@@ -760,6 +760,8 @@ enum.ActivityCode.ENABLE_FULFILLER_APPROVAL_NOTIFICATIONS=Enable fulfiller appro
 enum.ActivityCode.PACK_SHIPMENT=Pack shipment
 enum.ActivityCode.PARTIAL_RECEIVING=Partial receiving
 enum.ActivityCode.REQUIRE_ACCOUNTING=Require accounting
+enum.ActivityCode.REQUIRE_TRANSFER_OUT_DOCUMENT=Require document on outbound stock transfer
+enum.ActivityCode.REQUIRE_TRANSFER_IN_DOCUMENT=Require document on inbound stock transfer
 enum.ActivityCode.ENABLE_CENTRAL_PURCHASING=Enable central purchasing
 enum.ActivityCode.HOLD_STOCK=Hold stock
 enum.ActivityCode.SUBMIT_REQUEST=Submit request
@@ -4328,6 +4330,8 @@ react.locationsConfiguration.ActivityCode.ENABLE_NOTIFICATIONS=Enable Notificati
 react.locationsConfiguration.ActivityCode.PACK_SHIPMENT=Pack shipment
 react.locationsConfiguration.ActivityCode.PARTIAL_RECEIVING=Partial receiving
 react.locationsConfiguration.ActivityCode.REQUIRE_ACCOUNTING=Require accounting
+react.locationsConfiguration.ActivityCode.REQUIRE_TRANSFER_OUT_DOCUMENT=Require document on outbound stock transfer
+react.locationsConfiguration.ActivityCode.REQUIRE_TRANSFER_IN_DOCUMENT=Require document on inbound stock transfer
 react.locationsConfiguration.ActivityCode.ENABLE_CENTRAL_PURCHASING=Enable central purchasing
 react.locationsConfiguration.ActivityCode.HOLD_STOCK=Hold stock
 react.locationsConfiguration.ActivityCode.SUBMIT_REQUEST=Submit request
@@ -4937,3 +4941,23 @@ react.confirmExpirationDate.modal.productName.label=Product Name
 react.confirmExpirationDate.modal.lot.label=Lot
 react.confirmExpirationDate.modal.previousExpiry.label=Previous Expiry
 react.confirmExpirationDate.modal.newExpiry.label=New Expiry
+
+# Custom: stock-transfer-document-upload
+customStockTransferDocument.required.error=A document is required to complete this stock transfer
+customStockTransferDocument.noDocuments.label=No supporting documents
+react.custom.stockTransferDocuments.panel.title=Supporting documents
+react.custom.stockTransferDocuments.panel.empty=No documents attached yet
+react.custom.stockTransferDocuments.panel.dropzone=Drop files here or click to browse
+react.custom.stockTransferDocuments.panel.uploadButton=Upload
+react.custom.stockTransferDocuments.panel.removeButton=Remove
+react.custom.stockTransferDocuments.upload.success=Document was uploaded successfully
+react.custom.stockTransferDocuments.upload.error=Document upload failed
+react.custom.stockTransferDocuments.upload.partialError=Some documents failed to upload. The remaining files above can be retried.
+react.custom.stockTransferDocuments.fetch.error=Unable to load documents. Please refresh to try again.
+react.custom.stockTransferDocuments.required.warning=A document must be attached before this stock transfer can be completed
+customStockTransferDocument.upload.invalidType.error=Unsupported file type. Allowed: PDF, image, Word, Excel, CSV, ZIP.
+customStockTransferDocument.upload.tooLarge.error=File exceeds the maximum upload size of {0} bytes.
+customStockTransferDocument.upload.invalidFilename.error=File name is missing or invalid.
+react.custom.stockTransferDocuments.upload.invalidType.error=Unsupported file type. Allowed: PDF, image, Word, Excel, CSV, ZIP.
+react.custom.stockTransferDocuments.upload.tooLarge.error=File is too large.
+react.custom.stockTransferDocuments.upload.invalidFilename.error=File name is missing or invalid.

grails-app/services/org/pih/warehouse/custom/stockTransferDocuments/CustomStockTransferDocumentService.groovy

@@ -0,0 +1,149 @@
+package org.pih.warehouse.custom.stockTransferDocuments
+
+import grails.gorm.transactions.Transactional
+import grails.validation.ValidationException
+import org.pih.warehouse.core.ActivityCode
+import org.pih.warehouse.core.Constants
+import org.pih.warehouse.core.Document
+import org.pih.warehouse.core.DocumentType
+import org.pih.warehouse.core.Location
+import org.pih.warehouse.order.Order
+import org.pih.warehouse.order.OrderItem
+import org.springframework.web.multipart.MultipartFile
+
+@Transactional
+class CustomStockTransferDocumentService {
+
+    static final String NULL_ORDER_ERROR = 'Cannot validate stock transfer completion: order is null'
+    static final String DOCUMENT_REQUIRED_ERROR = 'A document is required to complete this stock transfer'
+    static final String DOCUMENT_REQUIRED_CODE = 'customStockTransferDocument.required.error'
+
+    def grailsApplication
+
+    List<Map> listDocuments(Order order) {
+        return order.documents?.collect { Document doc ->
+            [
+                id          : doc.id,
+                name        : doc.name ?: doc.filename,
+                filename    : doc.filename,
+                documentType: doc.documentType?.name,
+                contentType : doc.contentType,
+                size        : doc.size,
+                uri         : doc.link,
+            ]
+        } ?: []
+    }
+
+    Boolean isDocumentRequired(Order order) {
+        return originSideRequiresOutDocument(order) || destinationSideRequiresInDocument(order)
+    }
+
+    Boolean originSideRequiresOutDocument(Order order) {
+        if (locationSupports(order?.origin, ActivityCode.REQUIRE_TRANSFER_OUT_DOCUMENT)) {
+            return true
+        }
+        return order?.orderItems?.any { OrderItem item ->
+            locationSupports(item.originBinLocation, ActivityCode.REQUIRE_TRANSFER_OUT_DOCUMENT)
+        }
+    }
+
+    Boolean destinationSideRequiresInDocument(Order order) {
+        if (locationSupports(order?.destination, ActivityCode.REQUIRE_TRANSFER_IN_DOCUMENT)) {
+            return true
+        }
+        return order?.orderItems?.any { OrderItem item ->
+            locationSupports(item.destinationBinLocation, ActivityCode.REQUIRE_TRANSFER_IN_DOCUMENT)
+        }
+    }
+
+    private static Boolean locationSupports(Location location, ActivityCode activity) {
+        return Boolean.TRUE.equals(location?.supports(activity))
+    }
+
+    long getMaxUploadSizeBytes() {
+        Object configured = grailsApplication?.config?.openboxes?.custom?.stockTransferDocuments?.maxUploadSizeBytes
+        if (configured instanceof Number && ((Number) configured).longValue() > 0L) {
+            return ((Number) configured).longValue()
+        }
+        return UploadConstraints.DEFAULT_MAX_BYTES
+    }
+
+    Order uploadDocument(String orderId, MultipartFile fileContents) {
+        Order order = getOrderOrThrow(orderId)
+        if (!fileContents || fileContents.empty) {
+            throw new IllegalArgumentException("File contents are required")
+        }
+
+        long maxBytes = getMaxUploadSizeBytes()
+        if (fileContents.size > maxBytes) {
+            throw new UploadValidationException(
+                    UploadConstraints.TOO_LARGE_CODE,
+                    UploadConstraints.TOO_LARGE_DEFAULT,
+                    [maxBytes] as Object[])
+        }
+
+        String sanitizedName = UploadConstraints.sanitizeFilename(fileContents.originalFilename)
+        if (!sanitizedName) {
+            throw new UploadValidationException(
+                    UploadConstraints.INVALID_FILENAME_CODE,
+                    UploadConstraints.INVALID_FILENAME_DEFAULT)
+        }
+
+        boolean contentTypeOk = UploadConstraints.isContentTypeAllowed(fileContents.contentType)
+        boolean extensionOk = UploadConstraints.isExtensionAllowed(sanitizedName)
+        if (!contentTypeOk || !extensionOk) {
+            throw new UploadValidationException(
+                    UploadConstraints.INVALID_TYPE_CODE,
+                    UploadConstraints.INVALID_TYPE_DEFAULT)
+        }
+
+        // Idempotency: a flaky network can cancel the request browser-side after the server
+        // already persisted the file, leaving the client to retry and create a duplicate.
+        // Treat a re-upload of the same (filename, size) for the same order as a no-op so
+        // the retry returns 200 without inserting a second copy.
+        Document existing = order.documents?.find { Document doc ->
+            doc.filename == sanitizedName && doc.size == fileContents.size as int
+        }
+        if (existing) {
+            log.info "custom_stock_transfer_document_upload_dedup orderId=${order.id} documentId=${existing.id}" +
+                    " filename=${sanitizedName} size=${fileContents.size}"
+            return order
+        }
+
+        Document document = new Document()
+        document.fileContents = fileContents.bytes
+        document.contentType = fileContents.contentType
+        document.name = sanitizedName
+        document.filename = sanitizedName
+        document.documentType = DocumentType.get(Constants.DEFAULT_DOCUMENT_TYPE_ID)
+
+        order.addToDocuments(document)
+        order.save(failOnError: true)
+
+        log.info "custom_stock_transfer_document_uploaded orderId=${order.id} documentId=${document.id} size=${document.size}"
+        return order
+    }
+
+    void validateForCompletion(Order order) {
+        if (!order) {
+            throw new IllegalArgumentException(NULL_ORDER_ERROR)
+        }
+
+        if (isDocumentRequired(order) && !order.documents) {
+            log.warn "custom_stock_transfer_completion_blocked orderId=${order.id}" +
+                    " originLocationId=${order.origin?.id} destinationLocationId=${order.destination?.id}" +
+                    " originBinLocationIds=${order.orderItems*.originBinLocation*.id}" +
+                    " destinationBinLocationIds=${order.orderItems*.destinationBinLocation*.id}"
+            order.errors.reject(DOCUMENT_REQUIRED_CODE, DOCUMENT_REQUIRED_ERROR)
+            throw new ValidationException(DOCUMENT_REQUIRED_ERROR, order.errors)
+        }
+    }
+
+    private Order getOrderOrThrow(String orderId) {
+        Order order = Order.get(orderId)
+        if (!order) {
+            throw new IllegalArgumentException("No stock transfer order found for id ${orderId}")
+        }
+        return order
+    }
+}

grails-app/services/org/pih/warehouse/stockTransfer/StockTransferService.groovy

@@ -43,6 +43,7 @@ class StockTransferService {
     GrailsApplication grailsApplication
     def orderService
     def authService
+    def customStockTransferDocumentService
 
     /**
      * Gets paginated list of stock transfers (Orders with TRANSFER_ORDER type)
@@ -341,6 +342,7 @@ class StockTransferService {
     }
 
     Order completeStockTransfer(StockTransfer stockTransfer) {
+        customStockTransferDocumentService.validateForCompletion(Order.get(stockTransfer.id))
         validateStockTransfer(stockTransfer)
 
         // Save the stockTransfer as an order

grails-app/views/custom/stockTransferDocuments/_documentsList.gsp

@@ -0,0 +1,28 @@
+<g:if test="${orderInstance?.documents}">
+    <table class="table table-bordered table-condensed">
+        <thead>
+            <tr>
+                <th><warehouse:message code="document.name.label" default="Name"/></th>
+                <th><warehouse:message code="document.contentType.label" default="Type"/></th>
+                <th><warehouse:message code="default.dateCreated.label" default="Date"/></th>
+            </tr>
+        </thead>
+        <tbody>
+            <g:each var="document" in="${orderInstance.documents.sort { it.dateCreated }}">
+                <tr>
+                    <td>
+                        <a href="${createLink(controller: 'document', action: 'download', id: document.id)}"
+                           target="_blank">${document.name}</a>
+                    </td>
+                    <td>${document.contentType}</td>
+                    <td><format:date obj="${document.dateCreated}"/></td>
+                </tr>
+            </g:each>
+        </tbody>
+    </table>
+</g:if>
+<g:else>
+    <div class="empty-section center">
+        <warehouse:message code="customStockTransferDocument.noDocuments.label" default="No supporting documents"/>
+    </div>
+</g:else>