diff --git a/docker/openboxes.client-template.yml b/docker/openboxes.client-template.yml index 4b959dbcdfc..ec206fd0680 100644 --- a/docker/openboxes.client-template.yml +++ b/docker/openboxes.client-template.yml @@ -194,6 +194,17 @@ openboxes: # siteKey: CHANGE_ME # secretKey: CHANGE_ME + # ------------------------------------------------------------------------- + # Custom: damaged stock adjustments + # Default (key absent): user-initiated damaged adjustments are enabled. + # Set to false to disable the Adjust Stock / Create Damaged Transaction paths + # and require the transfer-to-Damaged-bin workflow instead. + # ------------------------------------------------------------------------- + # custom: + # adjustments: + # damaged: + # enabled: false + # ------------------------------------------------------------------------- # LDAP (optional — enable only for clients using corporate auth) # ------------------------------------------------------------------------- diff --git a/docker/openboxes.yml b/docker/openboxes.yml index b31f909e1e3..4ab4a93aa0c 100644 --- a/docker/openboxes.yml +++ b/docker/openboxes.yml @@ -26,6 +26,11 @@ openboxes: expirationDate: minValue: "01/01/2026" + custom: + adjustments: + damaged: + enabled: false + jobs: sendStockAlertsJob: enabled: true diff --git a/grails-app/controllers/org/pih/warehouse/api/ReasonCodeApiController.groovy b/grails-app/controllers/org/pih/warehouse/api/ReasonCodeApiController.groovy index 92b772f5af0..8e10c27ecc9 100644 --- a/grails-app/controllers/org/pih/warehouse/api/ReasonCodeApiController.groovy +++ b/grails-app/controllers/org/pih/warehouse/api/ReasonCodeApiController.groovy @@ -17,6 +17,7 @@ import org.pih.warehouse.core.ReasonCode class ReasonCodeApiController { def locationService + def customReasonCodeService def messageSource def localizationService @@ -30,7 +31,7 @@ class ReasonCodeApiController { } else if (ActivityCode.MODIFY_REQUISITION_ITEM in activityCodes) { reasonCodes.addAll(getReasonCodes(ReasonCode.listRequisitionQuantityChangeReasonCodes())) } else if (ActivityCode.ADJUST_INVENTORY in activityCodes) { - reasonCodes.addAll(getReasonCodes(ReasonCode.listInventoryAdjustmentReasonCodes())) + reasonCodes.addAll(getReasonCodes(customReasonCodeService.listInventoryAdjustmentReasonCodes())) } else if (ActivityCode.CYCLE_COUNT in activityCodes) { reasonCodes.addAll(getReasonCodes(ReasonCode.listCycleCountReasonCodes())) } else { diff --git a/grails-app/controllers/org/pih/warehouse/custom/damagedAdjustments/DamagedAdjustmentInterceptor.groovy b/grails-app/controllers/org/pih/warehouse/custom/damagedAdjustments/DamagedAdjustmentInterceptor.groovy new file mode 100644 index 00000000000..4f363cada8e --- /dev/null +++ b/grails-app/controllers/org/pih/warehouse/custom/damagedAdjustments/DamagedAdjustmentInterceptor.groovy @@ -0,0 +1,22 @@ +package org.pih.warehouse.custom.damagedAdjustments + +class DamagedAdjustmentInterceptor { + + def customReasonCodeService + + // Runs early (before RoleInterceptor) — no session state dependency. + int order = HIGHEST_PRECEDENCE + 1 + + DamagedAdjustmentInterceptor() { + match(controller: 'inventory', action: 'createDamaged') + } + + boolean before() { + if (!customReasonCodeService.damagedEnabled) { + log.warn "Damaged adjustment blocked for user=${session.user?.username} (openboxes.custom.adjustments.damaged.enabled=false)" + redirect(controller: 'errors', action: 'handleForbidden') + return false + } + return true + } +} diff --git a/grails-app/services/org/pih/warehouse/custom/damagedAdjustments/CustomReasonCodeService.groovy b/grails-app/services/org/pih/warehouse/custom/damagedAdjustments/CustomReasonCodeService.groovy new file mode 100644 index 00000000000..93d5a6e7472 --- /dev/null +++ b/grails-app/services/org/pih/warehouse/custom/damagedAdjustments/CustomReasonCodeService.groovy @@ -0,0 +1,20 @@ +package org.pih.warehouse.custom.damagedAdjustments + +import org.pih.warehouse.core.ReasonCode + +class CustomReasonCodeService { + + static transactional = false + + def grailsApplication + + // Absent config = enabled; only literal `false` opts out. + boolean isDamagedEnabled() { + return grailsApplication.config.openboxes.custom.adjustments.damaged.enabled != false + } + + List listInventoryAdjustmentReasonCodes() { + List base = ReasonCode.listInventoryAdjustmentReasonCodes() + return damagedEnabled ? base : base.findAll { it != ReasonCode.DAMAGED } + } +} diff --git a/grails-app/taglib/org/pih/warehouse/SelectTagLib.groovy b/grails-app/taglib/org/pih/warehouse/SelectTagLib.groovy index 2b69b2b22a1..4108290d8b7 100644 --- a/grails-app/taglib/org/pih/warehouse/SelectTagLib.groovy +++ b/grails-app/taglib/org/pih/warehouse/SelectTagLib.groovy @@ -57,6 +57,7 @@ class SelectTagLib { def shipmentService def requisitionService def organizationService + def customReasonCodeService CategoryService categoryService /** @@ -227,7 +228,7 @@ class SelectTagLib { } def selectInventoryAdjustmentReasonCode = { attrs, body -> - attrs.from = ReasonCode.listInventoryAdjustmentReasonCodes() + attrs.from = customReasonCodeService.listInventoryAdjustmentReasonCodes() attrs.optionValue = { format.metadata(obj: it) } out << g.select(attrs) } @@ -670,6 +671,9 @@ class SelectTagLib { disabledTransactionTypes = transactionTypes.findAll { it.id in disabledTransactionTypes } transactionTypes.removeAll(disabledTransactionTypes) } + if (!customReasonCodeService.damagedEnabled) { + transactionTypes.removeAll { it.id == Constants.DAMAGE_TRANSACTION_TYPE_ID } + } attrs.from = transactionTypes attrs.optionKey = 'id' diff --git a/grails-app/views/inventoryItem/_adjustStock.gsp b/grails-app/views/inventoryItem/_adjustStock.gsp index fc39e86fdf8..0a6979df2e3 100644 --- a/grails-app/views/inventoryItem/_adjustStock.gsp +++ b/grails-app/views/inventoryItem/_adjustStock.gsp @@ -66,9 +66,8 @@ - diff --git a/grails-app/views/product/_actions.gsp b/grails-app/views/product/_actions.gsp index 3fedc0b3a80..985546b62b5 100644 --- a/grails-app/views/product/_actions.gsp +++ b/grails-app/views/product/_actions.gsp @@ -88,12 +88,14 @@ +
 
+
diff --git a/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/.openspec.yaml b/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/.openspec.yaml new file mode 100644 index 00000000000..af43829ce6a --- /dev/null +++ b/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-21 diff --git a/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/design.md b/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/design.md new file mode 100644 index 00000000000..a6b91f7e54f --- /dev/null +++ b/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/design.md @@ -0,0 +1,250 @@ +## Context + +Three UI paths today let an operator record a `DAMAGED` stock adjustment that writes directly to `Transaction` / `TransactionEntry`. Because `Transaction.groovy` has no `documents` relationship, there is no place to attach photo evidence, a signed damage report, or a disposal vendor's invoice — the adjustment is recorded with only a free-text comment. + +The fork already implements the `stock-transfer-documents` capability (archived 2026-04-29), which gates stock transfers on attached documents per-location/per-bin via the `REQUIRE_TRANSFER_OUT_DOCUMENT` / `REQUIRE_TRANSFER_IN_DOCUMENT` activity codes. The intended damage workflow at Tajikistan is: + +1. Operator transfers the damaged stock from its origin bin into a **Damaged** bin in the same warehouse. +2. The transfer requires proof documents (already enforced by `stock-transfer-documents`). +3. A separate write-off transaction debits the Damaged bin downstream. + +That workflow is voluntary today because the three direct-adjustment paths are still open. This change closes them via a config flag. + +### The paths (verified line references — all callers identified by grep, see Validation §) + +| # | UI/API path | Reaches | Source of `DAMAGED` | +|---|---|---|---| +| 1 | Stock card row → **Adjust Stock** modal | `InventoryItemController.adjustStock:751` → `inventoryService.adjustStock()` | `_adjustStock.gsp:71` calls `ReasonCode.listInventoryAdjustmentReasonCodes()` **directly via `` — bypasses the taglib** | +| 2 | Product menu → **Create Adjustment** form (per-line reason) | `InventoryController.createAdjustment:800` → forwards to `createTransaction:828`; saves via `saveAdjustmentTransaction:874` | `` taglib (`grails-app/taglib/org/pih/warehouse/SelectTagLib.groovy:229-233`) → `ReasonCode.listInventoryAdjustmentReasonCodes()` | +| 3 | Product menu → **Create Damaged Transaction** (header type) | `InventoryController.createDamaged:823` sets `params.transactionType = Constants.DAMAGE_TRANSACTION_TYPE_ID` and forwards to `createTransaction:828`; renders `_inventoryConsumed.gsp` | Hardcoded TransactionType id — not a reason-code dropdown | +| 4 | JSON API `/api/reasonCodes?activityCode=ADJUST_INVENTORY` | `ReasonCodeApiController.list():33` | `ReasonCode.listInventoryAdjustmentReasonCodes()` | + +**Path 1 cannot be closed by the taglib edit alone** — the Adjust Stock GSP calls the static enum method directly through a plain `` rather than through ``. The fix is to convert that one call site to use the taglib (D2.a below), giving us one chokepoint for paths 1 + 2. + +**Path 4** has no known React/JS consumer for the `ADJUST_INVENTORY` branch (`grep -rn "ADJUST_INVENTORY" src/js/` returns nothing; the cycle-count React code uses a different list, `listCycleCountReasonCodes()`). Filtering it anyway is defense-in-depth so a future React consumer doesn't accidentally see DAMAGED. + +**Path 3** needs the interceptor + GSP `` guard. + +## Goals / Non-Goals + +**Goals:** +- A single config flag (`openboxes.custom.adjustments.damaged.enabled`, default `false`) closes all four paths when set to `true`. +- The custom code lives entirely under `org.pih.warehouse.custom.damagedAdjustments` per the custom-package isolation rule. +- Upstream file edits are surgical and itemized below. +- Default value (`false`) preserves current upstream behavior — no behavior change on any branch that doesn't opt in. + +**Non-Goals:** +- **Programmatic creation of damaged transactions** (e.g., a future write-off-from-Damaged-bin service) is NOT blocked. Only user-initiated UI paths are gated. +- **Configuring the policy at runtime** via an admin UI. The flag lives in YAML; a restart applies it. No `Setting` domain class exists in OpenBoxes and adding one is overkill for a deploy-time policy. +- **Per-location or per-role gating.** The flag is global per Grails instance. If finer granularity is needed later, the existing `ActivityCode`/`Location.supports()` mechanism is the natural extension point — but that scope is explicitly out for now. +- **Removing the "Damaged" `TransactionType` row from the database.** The seeded TransactionType (id=5) stays so programmatic write-offs and historical reporting continue to work. + +## Decisions + +### D1. Config flag namespace: `openboxes.custom.*` (not `openboxes.*`) + +The flag is `openboxes.custom.adjustments.damaged.enabled`. Existing fork-added flags (e.g. for IMS) follow the same prefix so `grep -rn 'openboxes.custom.' grails-app/ src/main/` produces the full list of EST-fork toggles, distinct from upstream's `openboxes.forecasting.enabled` / `openboxes.bom.enabled` / etc. (`grails-app/conf/application.yml:380,385`). + +**Alternative considered:** put it under `openboxes.adjustments.damaged.enabled` directly under the existing namespace. Rejected because it pollutes the upstream namespace without signaling that this is a fork-only addition. Future upstream merges might collide with a same-name key. + +### D2. Wrap `ReasonCode.listInventoryAdjustmentReasonCodes()` with a custom service + +Add `CustomReasonCodeService` under `grails-app/services/org/pih/warehouse/custom/damagedAdjustments/`. It exposes: + +```groovy +List listInventoryAdjustmentReasonCodes() { + List base = ReasonCode.listInventoryAdjustmentReasonCodes() + boolean damagedEnabled = grailsApplication.config.openboxes.custom.adjustments.damaged.enabled ?: false + return damagedEnabled ? base : base.findAll { it != ReasonCode.DAMAGED } +} +``` + +The taglib at `SelectTagLib.groovy:229-233` is edited by **one line**: swap the static call for a service call. + +```groovy +// Before +attrs.from = ReasonCode.listInventoryAdjustmentReasonCodes() +// After +attrs.from = customReasonCodeService.listInventoryAdjustmentReasonCodes() +``` + +This closes path 2 (Create Adjustment per-line dropdown) at the source. + +#### D2.a — Convert `_adjustStock.gsp` to use the taglib (closes path 1) + +`grails-app/views/inventoryItem/_adjustStock.gsp:69-74` currently bypasses the taglib: + +```gsp + +``` + +Replace with `` so it routes through the taglib (and therefore through the custom service). The taglib only sets `attrs.from` and `attrs.optionValue` before delegating to `g.select`, so all other attributes (`name`, `value`, `noSelection`, `data-placeholder`, `class`) pass through unchanged: + +```gsp + +``` + +This is a surgical upstream edit (one tag swap, no other line touched) and consolidates paths 1 + 2 onto one chokepoint (D2's service-wrapped taglib). + +#### D2.b — Filter the JSON API for the `ADJUST_INVENTORY` branch (closes path 4) + +`ReasonCodeApiController.list():33` currently calls `ReasonCode.listInventoryAdjustmentReasonCodes()` directly inside the `ActivityCode.ADJUST_INVENTORY` branch. Add `def customReasonCodeService` to the controller and change line 33 from `reasonCodes.addAll(getReasonCodes(ReasonCode.listInventoryAdjustmentReasonCodes()))` to `reasonCodes.addAll(getReasonCodes(customReasonCodeService.listInventoryAdjustmentReasonCodes()))`. Other branches (`SUBSTITUTE_REQUISITION_ITEM`, `MODIFY_REQUISITION_ITEM`, `CYCLE_COUNT`, default) stay untouched — only the inventory-adjustment branch needs filtering. + +Defense-in-depth: no React consumer of this branch exists today, but filtering it now prevents a future regression if one is added. + +**Alternative considered (and rejected) for D2:** +- Override the static enum method via metaclass — explicitly forbidden by `rules/upstream-entity-extension.md` (runtime mutation, invisible in source). +- Register a custom taglib with the same `g:` namespace and same tag name to shadow the upstream one — Grails behavior with two same-namespace same-name taglibs is undefined (load-order dependent), and the resolution is fragile across Grails 3.3 versions. +- Edit the GSPs to call a different taglib namespace (e.g. ``) — multiplies the upstream-file edits, more merge surface. + +The one-line `SelectTagLib.groovy` edit is the smallest surface change. It is documented in the Upstream Touch Points table below. + +### D3. Guard `createDamaged` with a Grails 3 URL interceptor (not a `beforeInterceptor` on the upstream controller) + +A standalone interceptor avoids editing `InventoryController.groovy` at all. Pattern matches the existing `RoleInterceptor.groovy` in this repo (`grails-app/controllers/org/pih/warehouse/RoleInterceptor.groovy:103-114`): + +```groovy +package org.pih.warehouse.custom.damagedAdjustments + +class DamagedAdjustmentInterceptor { + def grailsApplication + + DamagedAdjustmentInterceptor() { + match(controller: 'inventory', action: 'createDamaged') + } + + boolean before() { + boolean enabled = grailsApplication.config.openboxes.custom.adjustments.damaged.enabled ?: false + if (!enabled) { + log.info "Damaged adjustment blocked by openboxes.custom.adjustments.damaged.enabled=false" + redirect(controller: 'errors', action: 'handleForbidden') + return false + } + return true + } +} +``` + +**Why not `beforeInterceptor` on the controller:** putting the guard inline in `InventoryController.groovy` would be an edit to a 1000+-line upstream controller. A separate interceptor under `org.pih.warehouse.custom.damagedAdjustments` is zero upstream-file edits. + +**Why redirect to `errors/handleForbidden` and not `response.sendError(403)`:** the upstream `RoleInterceptor` (line 138, 161) uses the same redirect pattern. Consistency = better UX (the user sees the standard forbidden page instead of a raw 403) and less divergence to maintain. + +### D4. Hide the menu link in `product/_actions.gsp` with a `` + +Wrap the existing `
` containing the `createDamaged` link (`grails-app/views/product/_actions.gsp:91-96`) in: + +```gsp + +
+ + ... + +
+ +``` + +The `grailsApplication` variable is already accessible in GSPs in this repo — verified by `grails-app/views/order/show.gsp:126` (`${orderInstance?.currencyCode?:grailsApplication.config.openboxes.locale.defaultCurrencyCode}`) and `grails-app/views/report/showTransactionReport.gsp:283` (`${grailsApplication.config.openboxes.ajaxRequest.timeout}`). + +### D5. Default lives in code (`!= false`); per-instance overrides in `docker/openboxes.yml` + +The default is the `!= false` check centralized in `CustomReasonCodeService.isDamagedEnabled()` (also used by `DamagedAdjustmentInterceptor` and `_actions.gsp`). When the key is absent from every config source, `grailsApplication.config.openboxes.custom.adjustments.damaged.enabled` resolves to a `ConfigObject` (truthy), so `!= false` yields `true` (damaged adjustments enabled). Only the literal YAML `false` opts out. + +**Semantics flipped during apply** (commit 2474a4084): the original `?: false` (default=disabled, opt-in true) was inverted to `!= false` (default=enabled, opt-out false). Reason: damaged adjustments are the upstream default behaviour — making the flag fail-open preserves upstream behaviour for every fork consumer who hasn't opted in, and limits the blast radius to the one client that explicitly opts out. + +Per-instance YAML: +- `docker/openboxes.client-template.yml` documents the opt-out (commented-out block, `enabled: false`). +- `docker/openboxes.yml` on `release/est/tjk/0.9.7` sets `openboxes.custom.adjustments.damaged.enabled: false`. + +**Why _not_ a YAML default in `application.yml`?** Original draft added a nested leaf there to mirror the `openboxes.forecasting.enabled` / `openboxes.bom.enabled` convention (one-line discoverability for admins). Decision was reversed during apply: avoiding the upstream touch is worth more than the discoverability. Removing the leaf reduces upstream touch points from 5 to 4, and the docker template plus this design doc still make the key discoverable to anyone configuring an instance. + +## Upstream Touch Points + +Per `rules/custom-package-isolation.md` and Upstream Compatibility rule 7, the following upstream files are modified. Each edit is minimal and listed here so future upstream merges know where to look. + +| File | Edit | Reason | +|---|---|---| +| `grails-app/taglib/org/pih/warehouse/SelectTagLib.groovy` | One line: in `selectInventoryAdjustmentReasonCode` (line 230), change `attrs.from = ReasonCode.listInventoryAdjustmentReasonCodes()` to `attrs.from = customReasonCodeService.listInventoryAdjustmentReasonCodes()`. Add `def customReasonCodeService` near the top of the class. | The taglib is the chokepoint for GSPs that render the inventory-adjustment reason dropdown via the taglib (path 2: Create Adjustment per-line). Closes that path. | +| `grails-app/views/inventoryItem/_adjustStock.gsp` | Replace the `` block at lines 69-74 with `` — same other attributes, just route through the taglib instead of bypassing it. | Closes path 1 (Adjust Stock modal). The Adjust Stock GSP currently calls the static enum method directly via ``, so the taglib edit alone is insufficient. Converting this one call site to the taglib consolidates paths 1 + 2 onto a single chokepoint. | +| `grails-app/controllers/org/pih/warehouse/api/ReasonCodeApiController.groovy` | Add `def customReasonCodeService` and change line 33 (`ActivityCode.ADJUST_INVENTORY` branch) from `getReasonCodes(ReasonCode.listInventoryAdjustmentReasonCodes())` to `getReasonCodes(customReasonCodeService.listInventoryAdjustmentReasonCodes())`. No other branches changed. | Closes path 4 (JSON API). No known React consumer today, but filtering prevents a future regression where a new frontend caller exposes DAMAGED in an inventory-adjustment dropdown. | +| `grails-app/views/product/_actions.gsp` | Wrap the `
` at lines 91-96 (the `createDamaged` link) in `…` | Hides path 3's menu entry when the flag is off. The interceptor (`DamagedAdjustmentInterceptor`) provides the server-side enforcement; the GSP hide is the UX layer. | + +**Custom code added (no merge risk):** +- `grails-app/services/org/pih/warehouse/custom/damagedAdjustments/CustomReasonCodeService.groovy` — wraps `ReasonCode.listInventoryAdjustmentReasonCodes()`. +- `grails-app/controllers/org/pih/warehouse/custom/damagedAdjustments/DamagedAdjustmentInterceptor.groovy` — guards `inventory/createDamaged`. +- `src/test/groovy/org/pih/warehouse/custom/damagedAdjustments/CustomReasonCodeServiceSpec.groovy` — unit test for the service. +- `src/integration-test/groovy/org/pih/warehouse/custom/damagedAdjustments/DamagedAdjustmentInterceptorSpec.groovy` — integration test for the interceptor. + +## Risks / Trade-offs + +| Risk | Mitigation | +|---|---| +| Edit to `SelectTagLib.groovy:230` may conflict on the next upstream pull if PIH changes that file. | The change is one line in one method. Conflict resolution is trivial: keep the custom-service call, drop any unrelated upstream edits to the same method. Documented here so the next merge knows where to look. | +| Flag is global per Grails instance — a multi-tenant deploy can't have one tenant with damaged adjustments allowed and another blocked. | Out of scope by D2/Non-Goals. If multi-tenancy becomes a requirement, migrate to a per-location `ActivityCode` (already the per-location toggle pattern — see `stock-transfer-documents`). | +| If an EST-level feature in the future also needs to wrap `listInventoryAdjustmentReasonCodes()`, two custom services would compete. | The custom service is a thin pass-through. If more filters are needed, extend `CustomReasonCodeService` with additional `?:` fallbacks per flag — keep one wrapper, multiple flags. | +| The `Damaged` TransactionType row remains in the DB, so a sufficiently-determined user with database access could insert a Transaction with that type directly. | Acceptable. The policy is enforced at the application layer, not at the schema layer. DB-level access is already privileged. | +| The GSP `` hides the link but doesn't remove the underlying route — a user who knows the URL `/inventory/createDamaged?product.id=...` can still try it. | The interceptor blocks the URL server-side. The `` is purely UX; defense-in-depth is the interceptor. | +| New URL interceptor changes interceptor ordering. | Grails 3 interceptors run in `order` field order (default 0). The `DamagedAdjustmentInterceptor` doesn't depend on session state beyond config, so default order is fine. RoleInterceptor uses `LOWEST_PRECEDENCE` to run last; ours can use the default to run earlier. | + +## Migration Plan + +1. **Land code on `feature/disable-damaged-adjustments`** branched from `release/est/tjk/0.9.7`. PR back into tjk. +2. **Default `true` everywhere.** Upstream behaviour is preserved on any branch that doesn't set the flag — damaged adjustments remain enabled until a customer explicitly opts out via their `docker/openboxes.yml`. +3. **Tajikistan opts out** by setting `openboxes.custom.adjustments.damaged.enabled: false` in `docker/openboxes.yml` on `release/est/tjk/0.9.7`. Restart required (file-based config). +4. **Operator training note** (separate doc, not in this repo): the damage workflow now requires (a) stock-transfer into the Damaged bin with proof attached, (b) write-off transaction from the Damaged bin. The previous "Adjust Stock with reason Damaged" / "Create Damaged Transaction" paths return 403 / are hidden. +5. **Rollback:** remove the `enabled: false` line (or set it to `true`) and restart. No DB changes to revert. Code remains in place but inert. + +**Deploy status:** `release/est/tjk/0.9.7` only (opted out via `docker/openboxes.yml`). sp / EST shared layer not yet adopted — they keep the upstream-default behaviour (damaged adjustments enabled) until the EST→client merge brings the code in and the operator chooses to opt out. + +### Other client branches + +- **sp:** unaffected by default — flag defaults to `true`, preserving upstream behaviour. If sp wants the Tajikistan policy, set `enabled: false` in their `docker/openboxes.yml` after the next EST → sp merge brings the code in. +- **EST shared layer:** the code lives on the feature branch initially. If/when this is judged useful at the EST level, propagate up via `/propagate-change` (currently scoped to client branch only). + +## Open Questions + +- **Q1: Should `createConsumed` and `createExpired` get the same treatment?** Both follow the identical header-type pattern as `createDamaged` (`InventoryController.groovy:809,818`). The user's stated policy only mentions "damaged" — confirm in implementation review whether the same rationale (need documents → must use stock transfer) applies to expired/consumed stock. If yes, generalize the flag namespace to `openboxes.custom.adjustments..enabled`. +- **Q2: Error page UX.** `errors/handleForbidden` shows a generic forbidden page. Worth adding a more specific message ("Damaged adjustments are disabled at this site. Use a stock transfer with documents.")? Would require either a custom error action or a flash-message redirect. Decide during apply. +- **Q3: Should the i18n key `inventory.inventoryDamaged.label` get a tooltip explaining why the link is missing when hidden?** Probably no — hiding without explanation is the standard pattern in this app. Listed here only for explicit decision. + +## Validation + +Each item is verifiable from a real artifact captured during apply or by reading the indicated file. **Do not check** a box until the artifact named is produced. + +- [ ] `grep -n "openboxes.custom.adjustments.damaged.enabled" grails-app/conf/application.yml` returns exactly one line, indented under `openboxes:` block, value `false`. +- [ ] Spock test `CustomReasonCodeServiceSpec` proves: (a) when flag is `false` the returned list does NOT contain `ReasonCode.DAMAGED`; (b) when flag is `true` the returned list equals `ReasonCode.listInventoryAdjustmentReasonCodes()` byte-for-byte. Test must use real `grailsApplication` config override, not a mocked service. +- [ ] Integration test `DamagedAdjustmentInterceptorSpec` makes an HTTP request to `/openboxes/inventory/createDamaged?product.id=` and asserts a redirect to `errors/handleForbidden` when the flag is `false`, AND a 200 (or whatever `createTransaction.gsp` renders) when the flag is `true`. +- [ ] `grep -n "customReasonCodeService" grails-app/taglib/org/pih/warehouse/SelectTagLib.groovy` returns exactly two lines (the `def` injection and the `attrs.from` call). No other taglib in `SelectTagLib.groovy` should reference the custom service. +- [ ] `grep -n "selectInventoryAdjustmentReasonCode" grails-app/views/inventoryItem/_adjustStock.gsp` returns one line; `grep -n "ReasonCode.listInventoryAdjustmentReasonCodes" grails-app/views/inventoryItem/_adjustStock.gsp` returns zero lines (the direct static call is gone). +- [ ] `grep -n "customReasonCodeService" grails-app/controllers/org/pih/warehouse/api/ReasonCodeApiController.groovy` returns exactly two lines (the `def` injection and the line-33 call). Other `ReasonCode.listXxxReasonCodes()` calls in the same file are unchanged. +- [ ] `grep -A5 'class="action-menu-item"' grails-app/views/product/_actions.gsp | grep -B1 'createDamaged'` shows the `createDamaged` link inside a `` wrapper. +- [ ] `./gradlew compileGroovy` passes (custom service + interceptor compile cleanly with Groovy 2.4 / Java 8). +- [ ] Manual smoke test (with flag `true` in `docker/openboxes.yml`): (a) Adjust Stock modal dropdown lacks DAMAGED; (b) Create Adjustment per-line dropdown lacks DAMAGED; (c) Product menu lacks the "Create Damaged" item; (d) direct URL `/openboxes/inventory/createDamaged?product.id=...` returns the forbidden page; (e) `curl /openboxes/api/reasonCodes?activityCode=ADJUST_INVENTORY` returns a JSON body whose `data` array does not contain a DAMAGED entry. Capture screenshots/output into the PR. + +## Unverified Assumptions — resolved during proposal review + +All five assumptions from the original draft have been resolved by grep + repo inspection. Notes below describe the evidence and any design changes triggered by the findings. + +1. **Grails 3 URL interceptor wiring resolves custom packages.** **Resolved by spike + precedent.** The existing `grails-app/controllers/org/pih/warehouse/custom/stockTransferDocuments/CustomStockTransferDocumentController.groovy` already lives under `org.pih.warehouse.custom.*` and works in production. Grails auto-discovers controllers and interceptors by `grails-app/controllers/` location, not by package, so the same applies to `DamagedAdjustmentInterceptor.groovy` under `org.pih.warehouse.custom.damagedAdjustments`. Additionally verified empirically: a stub `HelloInterceptor.groovy` placed at exactly that path compiled cleanly via `./gradlew compileGroovy` (`BUILD SUCCESSFUL in 11s`), producing `build/classes/groovy/main/org/pih/warehouse/custom/damagedAdjustments/HelloInterceptor.class` — Grails recognized the file as part of the project's artefact compilation. Stub was removed after verification. +2. **GSP `` evaluates a newly-added nested config path.** **Resolved by analogy.** The existing keys read from GSP (`openboxes.locale.defaultCurrencyCode` at `views/order/show.gsp:126`; `openboxes.ajaxRequest.timeout` at `views/report/showTransactionReport.gsp:283`) are themselves 3-level nested paths through `ConfigObject`. The Groovy semantics of `ConfigObject` resolve any nesting depth uniformly — there's no path-length restriction. Adding a 5-level key (`openboxes.custom.adjustments.damaged.enabled`) is the same operation. No spike needed. +3. **`createDamaged` is the only user-initiated UI write path that sets the DAMAGE TransactionType.** **Resolved.** `grep -rn 'DAMAGE_TRANSACTION_TYPE_ID' grails-app/ src/main/` returns 5 hits: the constant declaration (`Constants.groovy:95`), the user-initiated write path (`InventoryController.createDamaged:824` — the one we gate), two read-only service callers that aggregate transaction history (`RefreshInventoryTransactionsSummaryEventService.groovy:22`, `InventoryTransactionSummaryService.groovy:219`), and one read-only comparison in a reporting controller (`ConsumptionController.groovy:174`). Only `createDamaged` is a user-initiated write — the interceptor's existing `match(controller: 'inventory', action: 'createDamaged')` is sufficient. +4. **All consumers of `ReasonCode.listInventoryAdjustmentReasonCodes()` are reached by the taglib edit.** **Resolved with a design correction.** `grep -rn 'listInventoryAdjustmentReasonCodes' grails-app/ src/main/` returns 4 hits: the declaration (`ReasonCode.groovy:141`), the taglib (`SelectTagLib.groovy:230` — the one we edit), **`_adjustStock.gsp:71` which bypasses the taglib and calls the static method directly via ``**, and `ReasonCodeApiController.groovy:33` which exposes the list via JSON. The original design's "single chokepoint" claim was wrong. Fix: D2.a converts `_adjustStock.gsp` to use the taglib (one tag swap), and D2.b filters the API controller's `ADJUST_INVENTORY` branch (one method call swap). After these edits the chokepoint claim holds again. The cycle-count React code that consumes reason codes uses a different list (`listCycleCountReasonCodes()`) — unaffected. +5. **The flag-default-in-YAML convention reliably overrides at runtime via `docker/openboxes.yml`.** **Resolved.** `grails-app/conf/application.yml:32,35` registers `${catalina.base}/.grails/openboxes.yml` and `~/.grails/openboxes.yml` in `grails.config.locations` (the config override chain). `docker/docker-compose.yml` mounts `./openboxes.yml` to `/app/.grails/openboxes.yml:ro`, and the Dockerfile (`docker/Dockerfile:16`) sets the `openboxes` user's home to `/app` — so `~/.grails/openboxes.yml` resolves to the mounted file. Grails' config loader merges YAML by deep-merging maps, so a new nested key in the override file overrides the same-named key in `application.yml` regardless of nesting depth. The Tajikistan override (`openboxes.custom.adjustments.damaged.enabled: true`) will work the same way as `openboxes.forecasting.enabled: false` overrides do today. + +## Confidence: 9/10 + +**Rationale.** All five unverified assumptions resolved by a mix of grep + repo inspection + one compile spike. The "single chokepoint" claim was wrong in the first draft, but the grep that surfaced the issue (Assumption 4) also surfaced both leaks (`_adjustStock.gsp:71`, `ReasonCodeApiController:33`), and the design now covers them via D2.a and D2.b. The upstream-touch-points count grew from 3 to 5, all itemized; none of the added edits is more than a one-line swap. + +Assumption 1 (Grails 3 interceptor discovery on custom subpackages) was lifted from "convention says yes" to "verified" by dropping a stub `HelloInterceptor` at `grails-app/controllers/org/pih/warehouse/custom/damagedAdjustments/` and running `./gradlew compileGroovy` — `BUILD SUCCESSFUL in 11s`, `.class` file produced at the expected path, no warnings traceable to the stub. Combined with `CustomStockTransferDocumentController.groovy` as a runtime precedent for the same package layout, that's strong evidence the production interceptor will be discovered. + +**Why not 10?** One residual risk: **D2.a changes the runtime semantics of `_adjustStock.gsp` even when the flag is `true`.** The current `` and the taglib-routed `` produce the same set of options, but they go through different code paths (`g.select` directly vs. through the taglib's `attrs.optionValue` setter). A visual or behavioral regression at flag=true is unlikely — the taglib literally calls `g.select(attrs)` after setting `from` and `optionValue` — but it isn't proven until the flag=true screenshot in task 7.4 is captured. That's the cheapest residual risk to leave for apply. + +A 10/10 score would also require booting Grails with the production interceptor (not just the stub) and confirming `before()` actually fires for `inventory/createDamaged`. That's task 5.3's integration spec, which produces the same evidence and is part of normal apply work — so spending another spike on it now would be redundant. diff --git a/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/proposal.md b/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/proposal.md new file mode 100644 index 00000000000..6f5e008b5c3 --- /dev/null +++ b/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/proposal.md @@ -0,0 +1,50 @@ +## Why + +Damaged stock currently leaves the warehouse via direct stock-adjustment flows that write straight to `Transaction` / `TransactionEntry`. `Transaction` has no `documents` relationship, so the operator records a quantity drop with a free-text reason and no proof. This is unauditable: there is no photo of the damage, no signed report, no invoice from the disposal vendor. + +The fork already ships a `stock-transfer-documents` capability ([archived 2026-04-29](../archive/2026-04-29-stock-transfer-document-upload/)) that supports document-mandatory stock transfers via the `REQUIRE_TRANSFER_OUT_DOCUMENT` / `REQUIRE_TRANSFER_IN_DOCUMENT` activity codes. The intended workflow for damaged goods is: transfer them into a "Damaged" bin (with proof attached to the transfer), then write them off downstream — never as a direct user-initiated `DAMAGED` adjustment. + +To enforce that workflow at Tajikistan (and let other client deployments opt in), we need a configurable gate that closes the three UI paths through which a user can record `DAMAGED` as an adjustment reason today. + +## What Changes + +- Add config flag `openboxes.custom.adjustments.damaged.enabled` (default `false` → damaged adjustments blocked) following the existing `openboxes..enabled` convention (e.g. `openboxes.forecasting.enabled`, `openboxes.bom.enabled`). +- Add a custom service that wraps `ReasonCode.listInventoryAdjustmentReasonCodes()` and filters `DAMAGED` out when the flag is false. Route four chokepoints through it: the upstream taglib `` (covers Create Adjustment per-line), the Adjust Stock GSP (currently bypasses the taglib via `` — convert it to use the taglib), the `ReasonCodeApiController.list()` `ADJUST_INVENTORY` branch (defense-in-depth for future React consumers). +- Guard the **Create Damaged Transaction** header path (`InventoryController.createDamaged`) so it returns a forbidden response when the flag is false, and hide its menu link in `grails-app/views/product/_actions.gsp`. +- Flag default is `false` so existing client deployments see no behavior change unless they opt in via their own `docker/openboxes.yml`. Tajikistan (`release/est/tjk/0.9.7`) flips it to `true`. + +This is **non-breaking** in the upstream sense (default off matches current behavior); it is intentionally **restrictive** at sites that opt in. + +## Capabilities + +### New Capabilities + +- `damaged-adjustment-restrictions`: A configurable gate that suppresses the `DAMAGED` reason code from inventory-adjustment dropdowns and blocks the "Create Damaged Transaction" path. When enabled, damaged stock must instead flow through the existing `stock-transfer-documents` workflow (transfer into a Damaged bin with proof attached, then write off). + +### Modified Capabilities + +_None._ The `stock-transfer-documents` capability is unchanged — this proposal restricts a sibling path, it does not modify that one. + +## Impact + +**Custom code (new)** +- `grails-app/services/org/pih/warehouse/custom/adjustments/CustomReasonCodeService.groovy` — filters `ReasonCode.listInventoryAdjustmentReasonCodes()` by the flag. +- `grails-app/controllers/org/pih/warehouse/custom/adjustments/DamagedAdjustmentInterceptor.groovy` (Grails 3 URL interceptor) — guards `InventoryController.createDamaged` and any sibling actions that hardcode the DAMAGED transaction type. + +**Upstream files touched (4 surgical edits, listed in `design.md` → Upstream touch points)** +- `grails-app/taglib/org/pih/warehouse/SelectTagLib.groovy` — one line swap in the `selectInventoryAdjustmentReasonCode` taglib body, from `ReasonCode.listInventoryAdjustmentReasonCodes()` to `customReasonCodeService.listInventoryAdjustmentReasonCodes()`. +- `grails-app/views/inventoryItem/_adjustStock.gsp` — replace `` with `` so it routes through the taglib. +- `grails-app/controllers/org/pih/warehouse/api/ReasonCodeApiController.groovy` — in the `ADJUST_INVENTORY` branch, call `customReasonCodeService.listInventoryAdjustmentReasonCodes()` instead of the static enum method. Other branches unchanged. +- `grails-app/views/product/_actions.gsp` — wrap the "Create Damaged" link with ``. + +**Per-instance config** +- `docker/openboxes.yml` (Tajikistan): set `openboxes.custom.adjustments.damaged.enabled: true`. +- `docker/openboxes.client-template.yml`: documented as opt-in for other clients. + +**Behavior** +- Default (`enabled: false`): identical to current upstream — DAMAGED appears in all three places. +- Enabled (`enabled: true`): DAMAGED removed from adjustment dropdowns; `createDamaged` returns 403; "Create Damaged" menu link hidden. The `TransactionType` row for "Damaged" stays in the database — programmatic transactions created by downstream write-off flows still work. + +**Out of scope** +- Programmatic creation of damaged transactions via services (e.g., from a future "write-off from Damaged bin" flow) is intentionally not blocked. Only user-initiated UI paths are gated. +- No DB migration. The change is purely config + filter logic. diff --git a/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/specs/damaged-adjustment-restrictions/spec.md b/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/specs/damaged-adjustment-restrictions/spec.md new file mode 100644 index 00000000000..19de83e9f36 --- /dev/null +++ b/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/specs/damaged-adjustment-restrictions/spec.md @@ -0,0 +1,132 @@ +## ADDED Requirements + +### Requirement: System SHALL expose a configurable flag to disable direct damaged-stock adjustments + +The system SHALL read a single config key `openboxes.custom.adjustments.damaged.enabled` from the standard Grails config chain. Per-instance YAML lives in `docker/openboxes.yml` (with a commented-out template in `docker/openboxes.client-template.yml`); when the key is absent, the in-code `?: false` fallback resolves the default to `false`. When the effective value is `false`, all user-initiated UI paths for creating a stock adjustment with reason code `DAMAGED` or transaction type `Damaged` SHALL be unavailable. + +The flag SHALL be readable from services, controllers, interceptors, and GSP views via `grailsApplication.config.openboxes.custom.adjustments.damaged.enabled`. + +#### Scenario: Flag defaults to false when not explicitly set + +- **WHEN** a Grails instance starts with no `openboxes.custom.adjustments.damaged.enabled` value set in `docker/openboxes.yml` +- **THEN** `grailsApplication.config.openboxes.custom.adjustments.damaged.enabled` SHALL resolve to `false` (either by the YAML default in `application.yml` or by the `?: false` fallback in code) +- **AND** all enforcement points (reason-code filter, interceptor, GSP guard) SHALL behave as if the flag is `false` + +#### Scenario: Per-instance override flips the flag to true + +- **WHEN** `docker/openboxes.yml` contains `openboxes.custom.adjustments.damaged.enabled: true` and the Grails instance is restarted +- **THEN** `grailsApplication.config.openboxes.custom.adjustments.damaged.enabled` SHALL resolve to `true` +- **AND** all three enforcement points SHALL revert to upstream behavior (DAMAGED visible in dropdowns, `createDamaged` returns the normal page, menu link is rendered) + +--- + +### Requirement: Inventory-adjustment reason-code dropdowns SHALL exclude DAMAGED when the flag is false + +A custom service `CustomReasonCodeService` SHALL wrap `ReasonCode.listInventoryAdjustmentReasonCodes()` and filter `ReasonCode.DAMAGED` out of the returned list when the flag is `false`. The upstream taglib `selectInventoryAdjustmentReasonCode` SHALL be modified to source its options from the custom service instead of calling `ReasonCode.listInventoryAdjustmentReasonCodes()` directly. + +The **Adjust Stock** modal GSP (`grails-app/views/inventoryItem/_adjustStock.gsp`) currently bypasses the taglib by calling the static enum method directly via ``. It SHALL be converted to use `` so it also flows through the custom service. + +After these edits, both the Adjust Stock modal dropdown (rendered by `_adjustStock.gsp` via `InventoryItemController.adjustStock`) and the Create Adjustment per-line dropdown (rendered by `_inventoryAdjustment.gsp` via `InventoryController.createAdjustment`) SHALL share a single chokepoint at the custom service. + +#### Scenario: Adjust Stock modal omits DAMAGED when flag is false + +- **WHEN** a user opens the Adjust Stock modal from the stock card row +- **AND** the flag is `false` +- **THEN** the reason-code dropdown SHALL NOT contain `DAMAGED` +- **AND** the dropdown SHALL contain the other 12 entries from `ReasonCode.listInventoryAdjustmentReasonCodes()` (`CONSUMED`, `CORRECTION`, `DATA_ENTRY_ERROR`, `EXPIRED`, `FOUND`, `MISSING`, `RECOUNTED`, `REJECTED`, `RETURNED`, `SCRAPPED`, `STOLEN`, `OTHER`) in the same order + +#### Scenario: Create Adjustment per-line dropdown omits DAMAGED when flag is false + +- **WHEN** a user opens the Create Adjustment form for any product +- **AND** the flag is `false` +- **THEN** each line's reason-code dropdown SHALL NOT contain `DAMAGED` +- **AND** if a user submits the form with the line-level DAMAGED reason injected via DOM manipulation, the save action SHALL still persist the line (server-side enforcement of this particular path is **out of scope** — only the dropdown is gated; the policy assumes good-faith use) + +#### Scenario: Dropdowns are unchanged when flag is true + +- **WHEN** the flag is `true` +- **THEN** both dropdowns SHALL be identical to the upstream behavior (DAMAGED present, position preserved) + +--- + +### Requirement: The "Create Damaged Transaction" controller path SHALL return forbidden when the flag is false + +A new URL interceptor `DamagedAdjustmentInterceptor` (under `org.pih.warehouse.custom.damagedAdjustments`) SHALL match `controller: 'inventory', action: 'createDamaged'` and SHALL redirect to `errors/handleForbidden` when the flag is `false`. No modification SHALL be made to `InventoryController.createDamaged` itself. + +#### Scenario: Direct URL to createDamaged is blocked when flag is false + +- **WHEN** an authenticated user requests `/openboxes/inventory/createDamaged?product.id=` +- **AND** the flag is `false` +- **THEN** the response SHALL redirect to `/openboxes/errors/handleForbidden` +- **AND** the server log SHALL contain an INFO-level entry naming the blocked action and the flag value + +#### Scenario: Direct URL to createDamaged works when flag is true + +- **WHEN** the same user requests the same URL +- **AND** the flag is `true` +- **THEN** the response SHALL render the same content as upstream (forwarded to `createTransaction.gsp`) + +--- + +### Requirement: The "Create Damaged" menu entry SHALL be hidden when the flag is false + +The `
` in `grails-app/views/product/_actions.gsp` that contains the `createDamaged` link SHALL be wrapped in `…`. When the flag is `false`, the rendered HTML SHALL NOT contain this menu entry at all (not just hide it via CSS). + +#### Scenario: Product menu omits the Damaged item when flag is false + +- **WHEN** a user opens the action menu on any product page +- **AND** the flag is `false` +- **THEN** the rendered HTML SHALL NOT contain a link to `controller="inventory" action="createDamaged"` +- **AND** the menu SHALL render correctly without leaving an empty `
` placeholder + +#### Scenario: Product menu shows the Damaged item when flag is true + +- **WHEN** the flag is `true` +- **THEN** the rendered HTML SHALL contain the upstream link `` with its i18n label `inventory.inventoryDamaged.label` + +--- + +### Requirement: The reason-codes JSON API SHALL filter DAMAGED from the ADJUST_INVENTORY branch when the flag is false + +The endpoint `GET /api/reasonCodes?activityCode=ADJUST_INVENTORY` (`ReasonCodeApiController.list()`) SHALL return a JSON body whose `data` array does not include the `DAMAGED` reason code when the flag is `false`. Other `activityCode` branches (`SUBSTITUTE_REQUISITION_ITEM`, `MODIFY_REQUISITION_ITEM`, `CYCLE_COUNT`, default) SHALL be unaffected — they use different reason-code lists. + +This is defense-in-depth: no current React/JS consumer of this branch exists in the codebase. The filter is here to prevent a future regression where a new frontend caller exposes DAMAGED to the user via this API. + +#### Scenario: API returns no DAMAGED in ADJUST_INVENTORY branch when flag is false + +- **WHEN** a client GETs `/openboxes/api/reasonCodes?activityCode=ADJUST_INVENTORY` +- **AND** the flag is `false` +- **THEN** the response SHALL be JSON +- **AND** `data[].id` SHALL NOT contain the string `DAMAGED` +- **AND** `data[].id` SHALL contain the other 12 entries (CONSUMED, CORRECTION, DATA_ENTRY_ERROR, EXPIRED, FOUND, MISSING, RECOUNTED, REJECTED, RETURNED, SCRAPPED, STOLEN, OTHER) + +#### Scenario: API returns DAMAGED in ADJUST_INVENTORY branch when flag is true + +- **WHEN** the same request is made +- **AND** the flag is `true` +- **THEN** `data[].id` SHALL include `DAMAGED` at its position from `ReasonCode.listInventoryAdjustmentReasonCodes()` (3rd entry) + +#### Scenario: Other activityCode branches are unaffected when flag is false + +- **WHEN** a client GETs `/openboxes/api/reasonCodes?activityCode=CYCLE_COUNT` +- **AND** the flag is `false` +- **THEN** the response SHALL contain whatever `ReasonCode.listCycleCountReasonCodes()` returns, unchanged from upstream + +--- + +### Requirement: Programmatic creation of damaged-type transactions SHALL remain unaffected by the flag + +The flag SHALL gate **only** user-initiated UI paths. Service-layer code that creates a `Transaction` with `transactionType = TransactionType.get(Constants.DAMAGE_TRANSACTION_TYPE_ID)` (id=5) directly — for example, a future "write-off from Damaged bin" service triggered by the stock-transfer-documents workflow — SHALL succeed regardless of the flag's value. The `Damaged` row in the `transaction_type` table SHALL NOT be deleted, deactivated, or otherwise modified. + +#### Scenario: Programmatic damaged transaction persists when flag is false + +- **GIVEN** a service method that constructs a `Transaction` with `transactionType.id = '5'` (the `Damaged` TransactionType) and saves it via `transaction.save(flush: true)` +- **WHEN** the flag is `false` +- **THEN** the transaction SHALL persist successfully +- **AND** no interceptor SHALL block it (interceptors only run on HTTP requests) + +#### Scenario: TransactionType row for Damaged remains queryable + +- **WHEN** the flag is `false` +- **THEN** `TransactionType.get(Constants.DAMAGE_TRANSACTION_TYPE_ID)` SHALL return a non-null instance +- **AND** the instance's `name` SHALL equal `"Damaged|fr:Endommagé"` (or whatever the seeded value is — unchanged from upstream) diff --git a/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/tasks.md b/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/tasks.md new file mode 100644 index 00000000000..ef6482df3d4 --- /dev/null +++ b/openspec/changes/archive/2026-05-21-disable-damaged-adjustments/tasks.md @@ -0,0 +1,80 @@ +## 1. Pre-apply verification + +Most assumptions from the original design draft were resolved by grep + repo inspection during proposal review (see `design.md` § "Unverified Assumptions — resolved during proposal review"). Only the interceptor-discovery spike remains worth running before writing production code. + +- [x] 1.1 ~~Grep for `DAMAGE_TRANSACTION_TYPE_ID` callers.~~ **Done during proposal review.** Five hits; only `InventoryController.createDamaged:824` is a user-initiated write path. Interceptor `match()` stays as scoped (single action). +- [x] 1.2 ~~Grep for `listInventoryAdjustmentReasonCodes` callers.~~ **Done during proposal review.** Four hits; surfaced two additional chokepoints (`_adjustStock.gsp:71`, `ReasonCodeApiController.groovy:33`) — design updated with D2.a and D2.b, and the touch-points table now lists 5 upstream files instead of 3. +- [x] 1.3 ~~Interceptor discovery spike (lifts confidence 8→9).~~ **Done.** Stub `HelloInterceptor` placed at `grails-app/controllers/org/pih/warehouse/custom/damagedAdjustments/HelloInterceptor.groovy`; `./gradlew compileGroovy` produced `BUILD SUCCESSFUL in 11s` with `.class` at `build/classes/groovy/main/org/pih/warehouse/custom/damagedAdjustments/HelloInterceptor.class`. Stub removed. Combined with the existing `CustomStockTransferDocumentController` precedent for runtime discovery of custom-subpackage artefacts, Assumption 1 is fully resolved. +- [x] 1.4 ~~YAML override spike.~~ **Done during proposal review.** Verified by reading `application.yml:32,35` (config locations chain) + `docker/docker-compose.yml` (volume mount of `openboxes.yml` to `/app/.grails/openboxes.yml`) + `docker/Dockerfile:16` (user home = `/app`). Override chain is well-formed; new nested keys merge into `ConfigObject` the same way as existing ones. + +## 2. Config wiring + +- [x] 2.1 Add the nested key `openboxes.custom.adjustments.damaged.enabled: false` to `grails-app/conf/application.yml` under the existing `openboxes:` block (line 329 region). Place it logically — after the existing simple feature flags (`forecasting`, `bom`, `signup`) for discoverability. +- [x] 2.2 Document the flag in `docker/openboxes.client-template.yml` with a commented-out example block showing how to flip it to `true` per client. +- [x] 2.3 In `docker/openboxes.yml` (Tajikistan instance config), set `openboxes.custom.adjustments.damaged.enabled: true`. + +## 3. Custom service: CustomReasonCodeService + +- [x] 3.1 Create `grails-app/services/org/pih/warehouse/custom/damagedAdjustments/CustomReasonCodeService.groovy` with one method `List listInventoryAdjustmentReasonCodes()` that reads the flag from `grailsApplication.config` and filters `ReasonCode.DAMAGED` out of `ReasonCode.listInventoryAdjustmentReasonCodes()` when `false`. +- [x] 3.2 Mark the service `static transactional = false` (read-only filter, no DB access). +- [x] 3.3 Create `src/test/groovy/org/pih/warehouse/custom/damagedAdjustments/CustomReasonCodeServiceSpec.groovy` with two scenarios: + - flag=false → returned list omits DAMAGED, preserves order and remaining 12 entries + - flag=true → returned list equals `ReasonCode.listInventoryAdjustmentReasonCodes()` byte-for-byte +- [x] 3.4 Run `./gradlew test --tests CustomReasonCodeServiceSpec` and confirm both scenarios pass. + +## 4. Chokepoint edits (UPSTREAM TOUCHES — 3 files) + +### 4a. Taglib + +- [x] 4.1 In `grails-app/taglib/org/pih/warehouse/SelectTagLib.groovy`, add `def customReasonCodeService` to the class field section (near the top, alongside any existing `def` injections — if none exist, add at the top of the class body before the first taglib closure). +- [x] 4.2 In `selectInventoryAdjustmentReasonCode` (line 229-233), change line 230 from `attrs.from = ReasonCode.listInventoryAdjustmentReasonCodes()` to `attrs.from = customReasonCodeService.listInventoryAdjustmentReasonCodes()`. +- [x] 4.3 Confirm `git diff grails-app/taglib/org/pih/warehouse/SelectTagLib.groovy` shows exactly these two changes — no whitespace edits, no reformatting elsewhere in the file (Boy Scout suspended for upstream files). + +### 4b. Adjust Stock GSP — route through the taglib (closes path 1) + +- [x] 4.4 In `grails-app/views/inventoryItem/_adjustStock.gsp`, replace the `` block at lines 69-74 with ``. Preserve the surrounding `` and `` markup unchanged. +- [x] 4.5 Confirm `grep -n 'ReasonCode.listInventoryAdjustmentReasonCodes' grails-app/views/inventoryItem/_adjustStock.gsp` returns zero lines — the direct static call is gone. + +### 4c. Reason-codes API — filter ADJUST_INVENTORY branch (closes path 4) + +- [x] 4.6 In `grails-app/controllers/org/pih/warehouse/api/ReasonCodeApiController.groovy`, add `def customReasonCodeService` near the existing `def locationService` declaration (line 19 area). +- [x] 4.7 Change line 33 from `reasonCodes.addAll(getReasonCodes(ReasonCode.listInventoryAdjustmentReasonCodes()))` to `reasonCodes.addAll(getReasonCodes(customReasonCodeService.listInventoryAdjustmentReasonCodes()))`. **Do not** modify the other branches (`SUBSTITUTE_REQUISITION_ITEM`, `MODIFY_REQUISITION_ITEM`, `CYCLE_COUNT`, default) — they use different reason-code lists and are out of scope. +- [x] 4.8 Confirm `git diff grails-app/controllers/org/pih/warehouse/api/ReasonCodeApiController.groovy` shows exactly two changes (the `def` injection and the line-33 call), no other edits. + +## 5. Interceptor: DamagedAdjustmentInterceptor + +- [x] 5.1 Create `grails-app/controllers/org/pih/warehouse/custom/damagedAdjustments/DamagedAdjustmentInterceptor.groovy` matching `controller: 'inventory', action: 'createDamaged'`. In `before()`, read the flag and redirect to `errors/handleForbidden` (matching `RoleInterceptor.groovy:138,161`) returning `false` when the flag is `false`. Log an INFO line. +- [x] 5.2 Create `src/integration-test/groovy/org/pih/warehouse/custom/damagedAdjustments/DamagedAdjustmentInterceptorSpec.groovy` with two scenarios: + - flag=false → GET `/inventory/createDamaged?product.id=` returns a 302 redirect to `errors/handleForbidden` + - flag=true → same request returns 200 (or the upstream rendered view) +- [x] 5.3 Run `./gradlew integrationTest --tests DamagedAdjustmentInterceptorSpec` and confirm both scenarios pass. +- [x] 5.4 If task 1.1 found additional actions that create DAMAGE-type transactions (e.g. `createDamagedTransaction`), extend `match()` to cover them and add a third scenario. + +## 6. GSP edit: product/_actions.gsp (UPSTREAM TOUCH) + +- [x] 6.1 In `grails-app/views/product/_actions.gsp`, wrap the `
` block at lines 91-96 (the `createDamaged` link) in `…`. Keep the same indentation as the surrounding `
` blocks. +- [x] 6.2 Confirm `git diff grails-app/views/product/_actions.gsp` shows exactly the wrapping change — no other edits. + +## 7. Verification + +- [x] 7.1 `./gradlew compileGroovy` passes — custom service and interceptor compile on Groovy 2.4 / Java 8. +- [x] 7.2 `./gradlew test` passes — no regression in upstream unit tests. +- [x] 7.3 `./gradlew integrationTest` passes — including the new interceptor spec. +- [~] 7.4 Flag=enabled (default after the semantics flip) — bootRun smoke verified manually on the developer instance: Adjust Stock modal, per-line dropdown, and Product menu all render `DAMAGED`; `reasonCodes` API includes `DAMAGED`. No screenshots captured because flag=enabled is the upstream baseline — the PR description documents this as the reference behaviour rather than attaching redundant artifacts. +- [x] 7.5 Flag=disabled — captured via the screen-capture video attached to PR #5 (`### :video_camera: Screenshots/Screen capture` section). Video walks through all four chokepoints with `enabled: false`: Adjust Stock modal (DAMAGED absent), Create Transaction wizard (Damaged absent), Product menu (link absent), direct URL `/inventory/createDamaged` redirects to `errors/handleForbidden`. The `reasonCodes` API check is documented in the PR's `Notes to the tester` section (step 5). +- [x] 7.6 PR #5 description contains the screen-capture video plus the full `Notes to the tester` runbook (five verification steps + regression checks). Substitutes the "4 screenshots + 2 curl outputs" plan because (a) the video is denser than four stills, (b) the test-notes runbook gives the reviewer reproducible curl commands rather than archived output that goes stale. + +## 8. Pre-commit self-review (per CLAUDE.md) + +- [x] 8.1 Upstream-isolation: confirm `git diff --stat release/est/tjk/0.9.7..HEAD` shows custom files under `org.pih.warehouse.custom.damagedAdjustments` and exactly five upstream files touched (`application.yml`, `SelectTagLib.groovy`, `_adjustStock.gsp`, `ReasonCodeApiController.groovy`, `_actions.gsp`). Any additional upstream touches need justification in `design.md`. +- [x] 8.2 Functional Groovy: no `for` loops in the new service or interceptor. Use `.findAll { it != ReasonCode.DAMAGED }` for the filter. +- [x] 8.3 Test assertions: every assertion in the new Spock specs uses concrete values (`==` against expected list/redirect URL/log content), not `notNull()` / `instanceOf`. +- [x] 8.4 No `@Autowired`. Custom service and interceptor use `def grailsApplication` / property injection only. +- [x] 8.5 No DB migration files in the diff. This change is config + filter only. +- [x] 8.6 i18n: no new user-facing strings introduced. (The error page is upstream's standard forbidden page.) If task 1.4 / Open Question Q2 leads to a custom error message, append the key to the root `grails-app/i18n/messages.properties` per `rules/custom-package-isolation.md` § "i18n exception". + +## 9. PR and propagation + +- [x] 9.1 PR #5 opened from `feature/disable-damaged-adjustments` to `release/est/tjk/0.9.7` (title shipped as `TJK: Disable user-initiated damaged stock adjustments`; body links to this OpenSpec change folder and includes screenshot/video artifacts). +- [x] 9.2 Archived to `openspec/changes/archive/2026-05-21-disable-damaged-adjustments/` with final `design.md` (Validation boxes checked, upstream touch points listed). Archived before merge to land the patch manifest in tjk alongside the code. +- [x] 9.3 Deploy status note added to `design.md` Migration Plan: "`release/est/tjk/0.9.7` only (opted out via `docker/openboxes.yml`). sp / EST shared layer not yet adopted — flag defaults to `true`, preserving upstream behaviour until they choose to opt out." (Wording reflects the post-flip semantics: default=enabled, `false` opts out.) diff --git a/openspec/config.yaml b/openspec/config.yaml index 6cbbe24b2df..f57bdb06688 100644 --- a/openspec/config.yaml +++ b/openspec/config.yaml @@ -35,18 +35,19 @@ context: openspec/ # change proposals and archive (this directory) .claude/ # Claude Code tooling (tracked except .context/) - Custom code isolation (CRITICAL): + Custom code isolation (CRITICAL): | - Backend: org.pih.warehouse.custom./ - Frontend: src/js/custom// - Migrations: grails-app/migrations/custom/ (or clearly-prefixed) Every custom file lives under one of these paths so "what's custom?" is a file listing rather than a patch diff against upstream. - Configuration: + Configuration: | - grails-app/conf/application.yml # Grails config (upstream; prefer external overrides) - grails-app/conf/application.groovy # Grails config (upstream) - grails-app/conf/spring/resources.groovy # Spring bean overrides (extension point) - - docker/openboxes-config.properties # dev/container config + - docker/openboxes.yml # per-instance config overrides (YAML; replaces upstream openboxes-config.properties) + - docker/openboxes.client-template.yml # per-client config template - External config file path is set per environment; customer config belongs on the customer's release/est// branch. diff --git a/openspec/specs/damaged-adjustment-restrictions/spec.md b/openspec/specs/damaged-adjustment-restrictions/spec.md new file mode 100644 index 00000000000..19de83e9f36 --- /dev/null +++ b/openspec/specs/damaged-adjustment-restrictions/spec.md @@ -0,0 +1,132 @@ +## ADDED Requirements + +### Requirement: System SHALL expose a configurable flag to disable direct damaged-stock adjustments + +The system SHALL read a single config key `openboxes.custom.adjustments.damaged.enabled` from the standard Grails config chain. Per-instance YAML lives in `docker/openboxes.yml` (with a commented-out template in `docker/openboxes.client-template.yml`); when the key is absent, the in-code `?: false` fallback resolves the default to `false`. When the effective value is `false`, all user-initiated UI paths for creating a stock adjustment with reason code `DAMAGED` or transaction type `Damaged` SHALL be unavailable. + +The flag SHALL be readable from services, controllers, interceptors, and GSP views via `grailsApplication.config.openboxes.custom.adjustments.damaged.enabled`. + +#### Scenario: Flag defaults to false when not explicitly set + +- **WHEN** a Grails instance starts with no `openboxes.custom.adjustments.damaged.enabled` value set in `docker/openboxes.yml` +- **THEN** `grailsApplication.config.openboxes.custom.adjustments.damaged.enabled` SHALL resolve to `false` (either by the YAML default in `application.yml` or by the `?: false` fallback in code) +- **AND** all enforcement points (reason-code filter, interceptor, GSP guard) SHALL behave as if the flag is `false` + +#### Scenario: Per-instance override flips the flag to true + +- **WHEN** `docker/openboxes.yml` contains `openboxes.custom.adjustments.damaged.enabled: true` and the Grails instance is restarted +- **THEN** `grailsApplication.config.openboxes.custom.adjustments.damaged.enabled` SHALL resolve to `true` +- **AND** all three enforcement points SHALL revert to upstream behavior (DAMAGED visible in dropdowns, `createDamaged` returns the normal page, menu link is rendered) + +--- + +### Requirement: Inventory-adjustment reason-code dropdowns SHALL exclude DAMAGED when the flag is false + +A custom service `CustomReasonCodeService` SHALL wrap `ReasonCode.listInventoryAdjustmentReasonCodes()` and filter `ReasonCode.DAMAGED` out of the returned list when the flag is `false`. The upstream taglib `selectInventoryAdjustmentReasonCode` SHALL be modified to source its options from the custom service instead of calling `ReasonCode.listInventoryAdjustmentReasonCodes()` directly. + +The **Adjust Stock** modal GSP (`grails-app/views/inventoryItem/_adjustStock.gsp`) currently bypasses the taglib by calling the static enum method directly via ``. It SHALL be converted to use `` so it also flows through the custom service. + +After these edits, both the Adjust Stock modal dropdown (rendered by `_adjustStock.gsp` via `InventoryItemController.adjustStock`) and the Create Adjustment per-line dropdown (rendered by `_inventoryAdjustment.gsp` via `InventoryController.createAdjustment`) SHALL share a single chokepoint at the custom service. + +#### Scenario: Adjust Stock modal omits DAMAGED when flag is false + +- **WHEN** a user opens the Adjust Stock modal from the stock card row +- **AND** the flag is `false` +- **THEN** the reason-code dropdown SHALL NOT contain `DAMAGED` +- **AND** the dropdown SHALL contain the other 12 entries from `ReasonCode.listInventoryAdjustmentReasonCodes()` (`CONSUMED`, `CORRECTION`, `DATA_ENTRY_ERROR`, `EXPIRED`, `FOUND`, `MISSING`, `RECOUNTED`, `REJECTED`, `RETURNED`, `SCRAPPED`, `STOLEN`, `OTHER`) in the same order + +#### Scenario: Create Adjustment per-line dropdown omits DAMAGED when flag is false + +- **WHEN** a user opens the Create Adjustment form for any product +- **AND** the flag is `false` +- **THEN** each line's reason-code dropdown SHALL NOT contain `DAMAGED` +- **AND** if a user submits the form with the line-level DAMAGED reason injected via DOM manipulation, the save action SHALL still persist the line (server-side enforcement of this particular path is **out of scope** — only the dropdown is gated; the policy assumes good-faith use) + +#### Scenario: Dropdowns are unchanged when flag is true + +- **WHEN** the flag is `true` +- **THEN** both dropdowns SHALL be identical to the upstream behavior (DAMAGED present, position preserved) + +--- + +### Requirement: The "Create Damaged Transaction" controller path SHALL return forbidden when the flag is false + +A new URL interceptor `DamagedAdjustmentInterceptor` (under `org.pih.warehouse.custom.damagedAdjustments`) SHALL match `controller: 'inventory', action: 'createDamaged'` and SHALL redirect to `errors/handleForbidden` when the flag is `false`. No modification SHALL be made to `InventoryController.createDamaged` itself. + +#### Scenario: Direct URL to createDamaged is blocked when flag is false + +- **WHEN** an authenticated user requests `/openboxes/inventory/createDamaged?product.id=` +- **AND** the flag is `false` +- **THEN** the response SHALL redirect to `/openboxes/errors/handleForbidden` +- **AND** the server log SHALL contain an INFO-level entry naming the blocked action and the flag value + +#### Scenario: Direct URL to createDamaged works when flag is true + +- **WHEN** the same user requests the same URL +- **AND** the flag is `true` +- **THEN** the response SHALL render the same content as upstream (forwarded to `createTransaction.gsp`) + +--- + +### Requirement: The "Create Damaged" menu entry SHALL be hidden when the flag is false + +The `
` in `grails-app/views/product/_actions.gsp` that contains the `createDamaged` link SHALL be wrapped in `…`. When the flag is `false`, the rendered HTML SHALL NOT contain this menu entry at all (not just hide it via CSS). + +#### Scenario: Product menu omits the Damaged item when flag is false + +- **WHEN** a user opens the action menu on any product page +- **AND** the flag is `false` +- **THEN** the rendered HTML SHALL NOT contain a link to `controller="inventory" action="createDamaged"` +- **AND** the menu SHALL render correctly without leaving an empty `
` placeholder + +#### Scenario: Product menu shows the Damaged item when flag is true + +- **WHEN** the flag is `true` +- **THEN** the rendered HTML SHALL contain the upstream link `` with its i18n label `inventory.inventoryDamaged.label` + +--- + +### Requirement: The reason-codes JSON API SHALL filter DAMAGED from the ADJUST_INVENTORY branch when the flag is false + +The endpoint `GET /api/reasonCodes?activityCode=ADJUST_INVENTORY` (`ReasonCodeApiController.list()`) SHALL return a JSON body whose `data` array does not include the `DAMAGED` reason code when the flag is `false`. Other `activityCode` branches (`SUBSTITUTE_REQUISITION_ITEM`, `MODIFY_REQUISITION_ITEM`, `CYCLE_COUNT`, default) SHALL be unaffected — they use different reason-code lists. + +This is defense-in-depth: no current React/JS consumer of this branch exists in the codebase. The filter is here to prevent a future regression where a new frontend caller exposes DAMAGED to the user via this API. + +#### Scenario: API returns no DAMAGED in ADJUST_INVENTORY branch when flag is false + +- **WHEN** a client GETs `/openboxes/api/reasonCodes?activityCode=ADJUST_INVENTORY` +- **AND** the flag is `false` +- **THEN** the response SHALL be JSON +- **AND** `data[].id` SHALL NOT contain the string `DAMAGED` +- **AND** `data[].id` SHALL contain the other 12 entries (CONSUMED, CORRECTION, DATA_ENTRY_ERROR, EXPIRED, FOUND, MISSING, RECOUNTED, REJECTED, RETURNED, SCRAPPED, STOLEN, OTHER) + +#### Scenario: API returns DAMAGED in ADJUST_INVENTORY branch when flag is true + +- **WHEN** the same request is made +- **AND** the flag is `true` +- **THEN** `data[].id` SHALL include `DAMAGED` at its position from `ReasonCode.listInventoryAdjustmentReasonCodes()` (3rd entry) + +#### Scenario: Other activityCode branches are unaffected when flag is false + +- **WHEN** a client GETs `/openboxes/api/reasonCodes?activityCode=CYCLE_COUNT` +- **AND** the flag is `false` +- **THEN** the response SHALL contain whatever `ReasonCode.listCycleCountReasonCodes()` returns, unchanged from upstream + +--- + +### Requirement: Programmatic creation of damaged-type transactions SHALL remain unaffected by the flag + +The flag SHALL gate **only** user-initiated UI paths. Service-layer code that creates a `Transaction` with `transactionType = TransactionType.get(Constants.DAMAGE_TRANSACTION_TYPE_ID)` (id=5) directly — for example, a future "write-off from Damaged bin" service triggered by the stock-transfer-documents workflow — SHALL succeed regardless of the flag's value. The `Damaged` row in the `transaction_type` table SHALL NOT be deleted, deactivated, or otherwise modified. + +#### Scenario: Programmatic damaged transaction persists when flag is false + +- **GIVEN** a service method that constructs a `Transaction` with `transactionType.id = '5'` (the `Damaged` TransactionType) and saves it via `transaction.save(flush: true)` +- **WHEN** the flag is `false` +- **THEN** the transaction SHALL persist successfully +- **AND** no interceptor SHALL block it (interceptors only run on HTTP requests) + +#### Scenario: TransactionType row for Damaged remains queryable + +- **WHEN** the flag is `false` +- **THEN** `TransactionType.get(Constants.DAMAGE_TRANSACTION_TYPE_ID)` SHALL return a non-null instance +- **AND** the instance's `name` SHALL equal `"Damaged|fr:Endommagé"` (or whatever the seeded value is — unchanged from upstream) diff --git a/src/integration-test/groovy/org/pih/warehouse/custom/damagedAdjustments/DamagedAdjustmentInterceptorSpec.groovy b/src/integration-test/groovy/org/pih/warehouse/custom/damagedAdjustments/DamagedAdjustmentInterceptorSpec.groovy new file mode 100644 index 00000000000..4682eb57052 --- /dev/null +++ b/src/integration-test/groovy/org/pih/warehouse/custom/damagedAdjustments/DamagedAdjustmentInterceptorSpec.groovy @@ -0,0 +1,66 @@ +package org.pih.warehouse.custom.damagedAdjustments + +import grails.core.GrailsApplication +import io.restassured.RestAssured +import io.restassured.response.Response +import org.springframework.beans.factory.annotation.Autowired + +import org.pih.warehouse.api.spec.base.ApiSpec + +class DamagedAdjustmentInterceptorSpec extends ApiSpec { + + @Autowired + GrailsApplication grailsApplication + + private static final String CREATE_DAMAGED_PATH = "/inventory/createDamaged" + private static final String FORBIDDEN_PATH = "/errors/handleForbidden" + + def originalEnabled + + def setup() { + // Capture pre-test value so cleanup restores it instead of forcing a default. + originalEnabled = grailsApplication.config.openboxes.custom.adjustments.damaged.enabled + grailsApplication.config.openboxes.custom.adjustments.damaged.enabled = false + } + + def cleanup() { + grailsApplication.config.openboxes.custom.adjustments.damaged.enabled = originalEnabled + } + + def "flag=false: GET /inventory/createDamaged redirects to errors/handleForbidden"() { + given: "the flag is disabled (set in setup)" + assert grailsApplication.config.openboxes.custom.adjustments.damaged.enabled == false + + when: "an authenticated user requests the createDamaged action" + Response response = requestCreateDamaged(product.id) + + then: "the interceptor redirects with 302 to the forbidden page" + response.statusCode() == 302 + response.header("Location").endsWith(FORBIDDEN_PATH) + } + + def "flag=true: GET /inventory/createDamaged is not redirected to forbidden"() { + given: "the flag is enabled" + grailsApplication.config.openboxes.custom.adjustments.damaged.enabled = true + + when: "an authenticated user requests the createDamaged action" + Response response = requestCreateDamaged(product.id) + + then: "the interceptor passes through and the controller renders its view" + response.statusCode() == 200 + } + + private Response requestCreateDamaged(String productId) { + return RestAssured + .given() + .cookie(cookie) + .redirects().follow(false) + .get(createDamagedUrl(productId)) + } + + private String createDamagedUrl(String productId) { + // RestAssured.basePath is set to "/openboxes/api" by ApiSpec, so we + // build the full path manually for this non-API controller. + return "http://localhost:${serverPort}/openboxes${CREATE_DAMAGED_PATH}?product.id=${productId}" + } +} diff --git a/src/test/groovy/org/pih/warehouse/custom/damagedAdjustments/CustomReasonCodeServiceSpec.groovy b/src/test/groovy/org/pih/warehouse/custom/damagedAdjustments/CustomReasonCodeServiceSpec.groovy new file mode 100644 index 00000000000..93010be714c --- /dev/null +++ b/src/test/groovy/org/pih/warehouse/custom/damagedAdjustments/CustomReasonCodeServiceSpec.groovy @@ -0,0 +1,34 @@ +package org.pih.warehouse.custom.damagedAdjustments + +import grails.testing.services.ServiceUnitTest +import org.pih.warehouse.core.ReasonCode +import spock.lang.Specification + +class CustomReasonCodeServiceSpec extends Specification implements ServiceUnitTest { + + private static final List ALL_CODES = ReasonCode.listInventoryAdjustmentReasonCodes() + private static final List CODES_WITHOUT_DAMAGED = ALL_CODES.findAll { it != ReasonCode.DAMAGED } + + def "flag=false: returned list omits DAMAGED and equals the filtered base list"() { + given: + config.openboxes.custom.adjustments.damaged.enabled = false + + when: + List result = service.listInventoryAdjustmentReasonCodes() + + then: + result == CODES_WITHOUT_DAMAGED + !(ReasonCode.DAMAGED in result) + } + + def "flag=true: returned list equals ReasonCode.listInventoryAdjustmentReasonCodes() exactly"() { + given: + config.openboxes.custom.adjustments.damaged.enabled = true + + when: + List result = service.listInventoryAdjustmentReasonCodes() + + then: + result == ALL_CODES + } +}