Skip to content

TJK: Disable user-initiated damaged stock adjustments#5

Open
gqcorneby wants to merge 11 commits into
release/est/tjk/0.9.7from
feature/disable-damaged-adjustments
Open

TJK: Disable user-initiated damaged stock adjustments#5
gqcorneby wants to merge 11 commits into
release/est/tjk/0.9.7from
feature/disable-damaged-adjustments

Conversation

@gqcorneby
Copy link
Copy Markdown

@gqcorneby gqcorneby commented May 21, 2026

📌 References

Purpose

  • Damaged stock currently leaves the warehouse via direct stock-adjustment flows that write straight to Transaction / TransactionEntry with no document attachment — there's no photo, no signed report, no vendor invoice. The drop in quantity is unauditable.
  • Tajikistan wants damaged goods to flow through the existing stock-transfer-documents workflow (transfer into a Damaged bin with proof attached, then write off downstream) instead of the direct adjustment paths.

NOTE: Implementation does not guard against direct API calls. Flag only applies to UI selectors createDamaged page

📝 Implementation

  • New config flag openboxes.custom.adjustments.damaged.enabledenabled by default, only literal false opts out. Tajikistan (docker/openboxes.yml) sets it to false;
  • New custom service CustomReasonCodeService under org.pih.warehouse.custom.damagedAdjustments — wraps ReasonCode.listInventoryAdjustmentReasonCodes() and filters DAMAGED out when the flag is disabled. Single isDamagedEnabled() getter centralizes the flag semantics.
  • New custom interceptor DamagedAdjustmentInterceptor — blocks GET/POST inventory/createDamaged with a 302 redirect to errors/handleForbidden when the flag is disabled.
  • Three surgical edits to upstream files route the existing UI through the custom service:
    • SelectTagLib.selectInventoryAdjustmentReasonCode / selectTransactionType → use the custom service.
    • ReasonCodeApiController ADJUST_INVENTORY branch → use the custom service.
    • _adjustStock.gsp → use <g:selectInventoryAdjustmentReasonCode> taglib (was inlining ReasonCode.listInventoryAdjustmentReasonCodes()).
    • _actions.gsp → wraps the "Create Damaged" menu link in <g:if>.
  • 2 Spock specs covering both flag states (unit + integration).

✨ Description of Change

Defence-in-depth gate on the four UI chokepoints where a user can record DAMAGED as an adjustment reason:

  1. Adjust Stock dialog (per-line reason code dropdown)
  2. Create Transaction wizard (selectTransactionType dropdown + reason code dropdown)
  3. Product → Actions menu (Create Damaged Transaction link)
  4. POST /inventory/createDamaged direct URL (interceptor 302)

Out of scope (intentional):

  • Programmatic creation of damaged transactions via services (e.g. a future "write-off from Damaged bin" flow). Only user-initiated paths are gated.
  • No DB migration — TransactionType for "Damaged" stays in the database, so programmatic flows and historical reports keep working.

📹 Screenshots/Screen capture

2026-05-21.16-55-13.mp4

🔥 Notes to the tester

To run with the flag disabled:

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

Verify each chokepoint (flag = false):

  1. Product Actions menu: open any product → Actions → no "mark as damaged" link.
  2. Direct URL: GET /openboxes/inventory/createDamaged?product.id=<id> redirects (302) to /openboxes/errors/handleForbidden. Server log emits Damaged adjustment blocked for user=<username> ....
  3. Adjust Stock dialog: from a stock card → Adjust Stock → Reason Code dropdown does not include "DAMAGED".
  4. Create Transaction wizard: Inventory → Create Transaction → Transaction Type dropdown does not include "Damaged".
  5. Reason Code API: GET /openboxes/api/reasonCodes?activityCode=ADJUST_INVENTORY response does not include DAMAGED.

Flip to enabled and re-verify (flag = true):

  • Remove the custom.adjustments.damaged.enabled: false block from docker/openboxes.yml, restart, repeat the five checks — DAMAGED reappears everywhere and createDamaged returns the form normally.

Regression checks:

  • Other reason codes (e.g. EXPIRED, STOLEN) and other transaction types still appear in the dropdowns with the flag disabled.
  • Existing historical "Damaged" transactions still render in transaction history (the TransactionType row was never deleted).

gqcorneby added 10 commits May 21, 2026 12:09
- Add literal block scalar (|) to 'Custom code isolation' and 'Configuration'
  sections so trailing prose lines don't break YAML parsing
- Replace stale 'docker/openboxes-config.properties' reference with
  'docker/openboxes.yml' (+ client-template.yml) to match current docker/ layout
Add config-driven flag (openboxes.custom.adjustments.damaged.enabled,
default false) that, when enabled, closes four user-facing chokepoints
where a 'Damaged' inventory adjustment can be recorded directly:

- Adjust Stock modal dropdown
- Create Adjustment per-line dropdown
- Create Damaged Transaction header path (controller + menu link)
- /api/reasonCodes?activityCode=ADJUST_INVENTORY JSON branch

Intent: force damaged stock to flow through the existing
stock-transfer-documents capability (transfer into a Damaged bin
with proof attached) instead of a Transaction with no document
support.

5 upstream files touched (surgical), all itemized in design.md
Upstream Touch Points. Custom code under
org.pih.warehouse.custom.damagedAdjustments per
rules/custom-package-isolation.md. Confidence 8/10.
Stub HelloInterceptor placed at grails-app/controllers/org/pih/warehouse/
custom/damagedAdjustments/HelloInterceptor.groovy compiled cleanly via
./gradlew compileGroovy (BUILD SUCCESSFUL in 11s) and produced the
expected .class file in build/classes/. Stub removed.

Combined with CustomStockTransferDocumentController.groovy as a runtime
precedent for custom-subpackage artefact discovery, Assumption 1 (Grails
3 interceptor scanner picks up files under org.pih.warehouse.custom.*)
is fully resolved.

Remaining residual risk (D2.a render-parity for _adjustStock.gsp after
the taglib swap) is cheapest to verify during apply via screenshot
capture at flag=true.
…asonCodeService

Add CustomReasonCodeService under org.pih.warehouse.custom.damagedAdjustments that
wraps ReasonCode.listInventoryAdjustmentReasonCodes() and filters ReasonCode.DAMAGED
when openboxes.custom.adjustments.damaged.enabled is false.

Route the upstream taglib (SelectTagLib.selectInventoryAdjustmentReasonCode),
_adjustStock.gsp (converted from direct static call), and ReasonCodeApiController
(ADJUST_INVENTORY branch) through the service as the single chokepoint.
…terceptor

Add DamagedAdjustmentInterceptor under org.pih.warehouse.custom.damagedAdjustments
that matches inventory/createDamaged and redirects to errors/handleForbidden when
openboxes.custom.adjustments.damaged.enabled is false.

Hide the Create Damaged menu link in product/_actions.gsp with <g:if> so the
server-side block is matched by a UX-level guard.
Set openboxes.custom.adjustments.damaged.enabled: true in docker/openboxes.yml
(Tajikistan instance). Document the opt-in flag in openboxes.client-template.yml
for other clients.
Update design.md (D5 decision reversed: default in code not application.yml,
upstream touch points reduced 5→4), proposal.md, spec.md, and tasks.md
(all tasks through 8.6 checked off).
…s out

Replace '?: false' with '!= false' so an absent config key (default) resolves
to true (damaged adjustments enabled), matching upstream behavior. Only sites
that explicitly set enabled: false (e.g. Tajikistan) block the damaged paths.

Update docker/openboxes.yml to set enabled: false for Tajikistan and update
the client-template comment to reflect the new opt-out semantics.
…Type dropdown

When the flag is explicitly false, the Damaged transaction type (id=5)
was still visible in the g:selectTransactionType dropdown used by
editTransaction and _inventoryConsumed views.

Follows the same pattern already used for activity-code-gated types
in the same taglib.
…onCodeService

Add isDamagedEnabled() getter on CustomReasonCodeService so the
`!= false` semantics live in one place. Route the interceptor and
selectTransactionType taglib through it instead of reading config
directly.

Also:
- Interceptor logs a warning (was info) and includes the username.
- Restore multi-line attribute formatting in _adjustStock.gsp.
- DamagedAdjustmentInterceptorSpec captures the original flag in
  setup() and restores it in cleanup() instead of forcing `false`,
  so the spec no longer leaks state into later integration specs.
- Move openspec/changes/disable-damaged-adjustments to
  openspec/changes/archive/2026-05-21-disable-damaged-adjustments.
- Promote delta capability spec to openspec/specs/damaged-adjustment-restrictions.
- Update design.md D5 and Migration Plan to reflect the post-flip
  semantics (default enabled, false opts out) introduced in 2474a40.
- Record Deploy status (tjk only, opted out via docker/openboxes.yml).
- Close out section 9 tasks; section 7 validation tasks resolved with
  PR #5 video + tester runbook in lieu of separate stills/curl outputs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant