Skip to content

Refactor allocation strategy#6023

Open
jmiranda wants to merge 2 commits into
front-counter-path-bfrom
claude/review-pr-6014-19s0ss
Open

Refactor allocation strategy#6023
jmiranda wants to merge 2 commits into
front-counter-path-bfrom
claude/review-pr-6014-19s0ss

Conversation

@jmiranda

Copy link
Copy Markdown
Member

The flat AllocationStrategy enum conflated two independent concerns: which bins are eligible and in what zone order (WAREHOUSE_FIRST / DISPLAY_FIRST / WAREHOUSE_ONLY) versus which lot to take first (FEFO). As siblings in one enum they could not be combined, so "display first, FEFO within each zone" was inexpressible, and a caller could set the strategy to FEFO and silently get no zone ordering at all.

Split the two axes and compose them:

  • SourceStrategy (enum): zone eligibility + priority order, operating on the AvailableItemGroups split. Renamed "warehouse" -> "storage" to match the generic vocabulary already present in ActivityCode (ORDER_ALLOCATION_STRATEGY_STORAGE_FIRST) and drop the front-counter framing.
  • RotationRule (enum): item-level ordering as a Comparator (FEFO today; QUANTITY_ASC/DESC; FIFO/LIFO left as TODO once a received date is available).
  • AllocationStrategy becomes a curated preset pairing a SourceStrategy with a RotationRule and composing them (zone order, rotation-sorted within each zone). Stays a single field on the requisition and in the canonical message.

This removes the handler/abstract/resolver machinery (7 files) in favour of two enums plus a shared comparator holder, and defines the FEFO sort once in AvailableItemComparators instead of duplicating it.

Also:

  • Rename WAREHOUSE_FIRST -> STORAGE_FIRST at the two remaining call sites (StockMovementController, AutomaticBackorderReallocationJob).
  • Treat configured strategies as a preference: fall back to FEFO when none is configured so available stock is still allocated instead of failing with "Available: 0".

Claude-Session: https://claude.ai/code/session_01DG7d6mvv9EgyvspK166v4T

✨ Description of Change

Link to GitHub issue or Jira ticket:

Description:


📷 Screenshots & Recordings (optional)

@github-actions github-actions Bot added domain: backend Changes or discussions relating to the backend server flag: config change Hilights a pull request that contains a change to the app config labels Jul 14, 2026
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.26506% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 10.65%. Comparing base (fd776ca) to head (6129e5f).
⚠️ Report is 2 commits behind head on front-counter-path-b.

Files with missing lines Patch % Lines
.../pih/warehouse/allocation/AllocationService.groovy 22.22% 7 Missing ⚠️
...ih/warehouse/allocation/AssignedBinOrdering.groovy 64.28% 2 Missing and 3 partials ⚠️
...warehouse/allocation/SourceStrategyResolver.groovy 69.23% 3 Missing and 1 partial ⚠️
...rehouse/allocation/AvailableItemComparators.groovy 57.14% 1 Missing and 2 partials ⚠️
...h/warehouse/allocation/FefoRotationStrategy.groovy 71.42% 2 Missing ⚠️
...use/allocation/NaturalOrderRotationStrategy.groovy 60.00% 2 Missing ⚠️
...rehouse/allocation/RotationStrategyResolver.groovy 84.61% 1 Missing and 1 partial ⚠️
...warehouse/inventory/StockMovementController.groovy 0.00% 1 Missing ⚠️
...ouse/jobs/AutomaticBackorderReallocationJob.groovy 0.00% 1 Missing ⚠️
...warehouse/allocation/AbstractSourceStrategy.groovy 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##             front-counter-path-b    #6023      +/-   ##
==========================================================
+ Coverage                   10.41%   10.65%   +0.24%     
- Complexity                   1702     1762      +60     
==========================================================
  Files                         923      927       +4     
  Lines                       52229    52268      +39     
  Branches                    12365    12370       +5     
==========================================================
+ Hits                         5439     5571     +132     
+ Misses                      45921    45787     -134     
- Partials                      869      910      +41     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…otation + negative inventory

Keeps the existing handler/resolver structure; changes only what would otherwise
harden into a corner once data/messages exist.

Source axis naming (persisted + message contract):
- Rename enum AllocationStrategy -> AllocationSourceStrategy (it models the source
  axis, and "allocation strategy" collides with the supply-commitment sense used by
  NetSuite/Infor). Leaves room for a future ReplenishmentSourceStrategy.
- Values STORAGE_FIRST / DISPLAY_FIRST / STORAGE_ONLY (drop FEFO, which is rotation
  not source; rename WAREHOUSE_* to STORAGE_* to match ActivityCode vocabulary).
- Rename handler trio + Warehouse*Handler -> Storage*Handler and the two call sites.
- Requisition field allocationStrategy -> allocationSourceStrategy; column
  allocation_source_strategy (renamed in the not-yet-released migration).

Rotation as a separate axis, config-driven, not on the requisition:
- Add RotationRule identifier (NONE/FEFO/FIFO/LIFO) and apply it after the source
  ordering as a stable within-bin sort: ORDER BY <rotation>, <source bin priority>.
  NONE leaves source order intact; FEFO reuses one shared comparator
  (AvailableItemComparators, dedup target for sortAvailableItems). FIFO/LIFO are
  declared extension points that currently fall back to source order.
- Config: openboxes.order.allocation.source (default STORAGE_FIRST) and .rotation
  (default FEFO) replace the old .strategies list. resolveStrategies now always
  yields at least the configured source (no empty-strategies "Available: 0" throw).
- A comment at the seam marks where a RotationStrategy + resolver will go.

Remove allowNegativeInventory from the requisition (field, migration changeSet, DTO
plumbing): negative-inventory is a facility/location capability resolved from activity
codes at issuance time, not a per-order attribute. To be modeled by the issuance
consumer; the canonical message treats it as reserved.

Deferred to a follow-up ticket: rotation strategy pattern + FIFO/LIFO, per
facility/product rotation cascade, InventoryLevel.sortOrder bin ordering, configurable
source/rotation precedence, fill/best-available, provenance.

Not executed here: Gradle/Grails deps are blocked by the environment proxy; verified
by manual trace.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DG7d6mvv9EgyvspK166v4T
@jmiranda
jmiranda force-pushed the claude/review-pr-6014-19s0ss branch from 6129e5f to d584de8 Compare July 16, 2026 05:33
@github-actions github-actions Bot added the flag: schema change Hilights a pull request that contains a change to the database schema label Jul 16, 2026
@druchniewicz
druchniewicz force-pushed the front-counter-path-b branch from 60ca612 to bb294e7 Compare July 17, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: backend Changes or discussions relating to the backend server flag: config change Hilights a pull request that contains a change to the app config flag: schema change Hilights a pull request that contains a change to the database schema

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants