Skip to content

[main] Bug 647499 Bin Replenishment with Pick by FEFO Creates Incorrect Inventory Movements - #10522

Open
MSNehaNawal wants to merge 4 commits into
mainfrom
bugs/Bug-647067-main-BinReplenishment_FEFOPick_IncorrectInvt
Open

[main] Bug 647499 Bin Replenishment with Pick by FEFO Creates Incorrect Inventory Movements#10522
MSNehaNawal wants to merge 4 commits into
mainfrom
bugs/Bug-647067-main-BinReplenishment_FEFOPick_IncorrectInvt

Conversation

@MSNehaNawal

@MSNehaNawal MSNehaNawal commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Bug 647499: [master][ALL-E] Bin Replenishment with Pick by FEFO Creates Incorrect Inventory Movements and Leaves Movement Worksheet Lines Unprocessed in Basic Warehouse Locations

Fixes AB#647499

Issue
Bin Replenishment with Pick by FEFO Creates Incorrect Inventory Movements and Leaves Movement Worksheet Lines Unprocessed in Basic Warehouse Locations

Root cause
Under Pick According to FEFO, Calculate Bin Replenishment leaves the worksheet line's From Bin blank (FEFO picks source bins at movement time). In codeunit 7322 this blank From Bin breaks two things:
1.Wrong quantity: CalcQtyAvailToPickOnBins counts a lot's stock across all bins including the destination bin. When the earliest FEFO lot already sits in the bin being replenished, that quantity gets reserved but can't be moved (can't move a bin onto itself), so it's dropped instead of rolling to the next lot — the movement comes up short.
2.Worksheet not cleared: UpdateHandledWhseActivityLineBuffer records the handled qty under the real take bin, but DeleteHandledWhseWorksheetLines matches it back to the worksheet line on From Bin (blank) — keys never match, so lines aren't removed and no confirmation shows.

Solution
CalcQtyAvailToPickOnBins: for a FEFO inventory movement, exclude the destination bin from the availability sum so FEFO moves on to the next movable lot and fulfills the full quantity.
UpdateHandledWhseActivityLineBuffer: when the source line is FEFO (blank From Bin), record the buffer under a blank From Bin so it reconciles with the worksheet line and the handled lines are deleted.

@github-actions github-actions Bot added the Team: SCM GitHub request for SCM area label Aug 21, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Aug 21, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

The fix targets Bin Replenishment with Pick by FEFO in basic warehouse locations (codeunit 7322). When FEFO leaves the worksheet line's From Bin blank, two problems occur: the availability sum counts the earliest lot already sitting in the destination bin (which cannot be moved onto itself), so the movement comes up short; and the handled-quantity buffer is recorded under the real take bin while the worksheet line matches on the blank From Bin, so lines are never cleared.

The two changes are aligned with those root causes: CalcQtyAvailToPickOnBins now excludes the destination bin from the availability sum, and UpdateHandledWhseActivityLineBuffer records the buffer under a blank From Bin for FEFO so it reconciles with the worksheet line. The logic reads correctly for the described scenario, but the destination-bin exclusion is broader than FEFO (see S2), and this data-integrity change ships with no automated test (see S1).

Suggestions

S1 - No automated test for inventory movement fix
This change alters how inventory movement quantities and worksheet cleanup are computed - a data-integrity path where a wrong result means incorrect stock movements. It needs a regression test. The warehouse test libraries already support this (location with Pick According to FEFO, bin replenishment, movement worksheet). Please add a test that reproduces the short movement and the unprocessed worksheet line, then passes with the fix.

S2 - Destination-bin exclusion is not limited to FEFO
In CalcQtyAvailToPickOnBins the new else branch applies to every inventory movement with a blank From Bin and a set destination bin (IsInvtMovement), not only FEFO. The buffer change in UpdateHandledWhseActivityLineBuffer guards on CurrLocation."Pick According to FEFO", but this one does not. Please confirm this wider scope is intended and does not change quantities for non-FEFO basic-warehouse movements, or add the same FEFO guard.

Risk assessment and necessity

Risk: Medium-high. Codeunit 7322 drives inventory pick and movement creation for all basic warehouse locations, so a change to availability summing and handled-line buffering has a wide blast radius. The destination-bin exclusion (S2) is not FEFO-gated, so non-FEFO movements can also be affected. Without a test, a regression here would surface as wrong on-hand quantities in the field.

Necessity: The scenario is valid and important - incorrect inventory movements and stuck worksheet lines are a real data-integrity defect. The fix is warranted; it just needs test coverage and a confirmed scope before it can merge safely.


[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10522 round=1 by=alexei-dobriansky at=2026-08-23T00:12:40Z lastSha=e131a5494030366989cfe94ff92971b32667b327 reviewKey=ebb2f337a40196c6854b4683eb060bb622827f5d281788dc4afc8ce77bcc20a1 suggestions=S1@b7ec42ab,S2@1ddae17f

@MSNehaNawal
MSNehaNawal marked this pull request as ready for review August 24, 2026 06:57
@MSNehaNawal
MSNehaNawal requested a review from a team August 24, 2026 06:57
@MSNehaNawal
MSNehaNawal requested a review from a team as a code owner August 24, 2026 06:57
@MSNehaNawal

Copy link
Copy Markdown
Contributor Author

Fixed S1 and S2

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 2

Recommendation: Accept

What this PR does

The fix targets Bin Replenishment with Pick by FEFO in basic warehouse locations (codeunit 7322). When FEFO leaves the worksheet line's From Bin blank, the availability sum counted the earliest lot already sitting in the destination bin (which cannot be moved onto itself), so the movement came up short; and the handled-quantity buffer was recorded under the real take bin while the worksheet line matched on the blank From Bin, so lines were never cleared.

Since round 1 both prior blockers are resolved. In CalcQtyAvailToPickOnBins the destination-bin exclusion is now gated on CurrLocation."Pick According to FEFO", so it no longer affects non-FEFO movements. UpdateHandledWhseActivityLineBuffer records the buffer under a blank From Bin only for a FEFO blank inventory movement. A new regression test reproduces the exact scenario (earliest lot split between a source bin and the destination bin) and asserts both that the full quantity is moved and that the worksheet lines are removed. The two changed flags (IsInvtMovement, IsBlankInvtMovement) are real codeunit globals and the exclusion only runs on the blank-From Bin path, so the scope is correct.

Status of previous suggestions
ID Title Status Author response
S1 No automated test for inventory movement fix Addressed Added CreateMovementFromWkshtFEFOWhenEarliestLotInDestinationBin in SCMMovement.Codeunit.al; asserts full quantity moved and worksheet lines cleared. "Fixed S1 and S2"
S2 Destination-bin exclusion is not limited to FEFO Addressed Added the CurrLocation."Pick According to FEFO" guard to the else branch in CalcQtyAvailToPickOnBins. "Fixed S1 and S2"
New observations (commits since round 1)

None - the changes only addressed the two prior suggestions, and no new issues were found.

Risk assessment and necessity

Risk: Low, and lower than round 1. The availability-sum exclusion and the handled-line buffer change are both now gated on Pick According to FEFO and only run on the blank-From Bin path, so non-FEFO basic-warehouse movements are unaffected. The new regression test exercises the fixed path end to end, which guards against a future regression in this data-integrity area.

Necessity: Unchanged from round 1. Incorrect inventory movements and stuck worksheet lines are a real data-integrity defect; the fix is warranted, correctly scoped, and now covered by a test.


[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10522 round=2 by=alexei-dobriansky at=2026-08-24T08:12:01Z lastSha=5c2b6b896e3c04d3c71a893a2c6c29c7bb8db35a reviewKey=46e5d2359589ea05210e3194c719bc8bf7e545abb2da6d81a9360e91ca4bd817 suggestions=S1@b7ec42ab:addressed,S2@1ddae17f:addressed parentRound=1

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a FEFO bin replenishment edge case in basic warehouse locations where a blank From Bin (FEFO-selected at movement time) could (1) miscalculate available quantity by including the destination bin and (2) prevent handled worksheet lines from being deleted due to key mismatches.

Changes:

  • Exclude the destination bin from FEFO inventory-movement availability calculations when the worksheet line’s From Bin is blank.
  • Reconcile handled-quantity buffering for FEFO (blank From Bin) movements by recording the buffer under a blank From Bin so deletion can match the worksheet line.
  • Add a regression test covering the “earliest lot already in destination bin” scenario and worksheet-line cleanup.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/Layers/W1/BaseApp/Warehouse/Activity/CreateInventoryPickMovement.Codeunit.al Adjusts FEFO movement bin-availability and handled-line buffering logic for blank From Bin scenarios.
src/Layers/W1/Tests/SCM-Warehouse/SCMMovement.Codeunit.al Adds a regression test and supporting helpers for FEFO replenishment when the earliest lot is already in the destination bin.
Suppressed comments (1)

src/Layers/W1/Tests/SCM-Warehouse/SCMMovement.Codeunit.al:1211

  • This comment refers to specific lot/bin labels ("L1", "L2", "F1", "F2"), but lots and bins are generated as GUIDs in the test. Using neutral wording avoids confusion when reading test output.
        // [GIVEN] Earliest lot "L1" is split between source bin "F1" (50) and the pick bin itself (50); later lot "L2" has 100 in "F2"

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Layers/W1/Tests/SCM-Warehouse/SCMMovement.Codeunit.al
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Aug 25, 2026
@MSNehaNawal
MSNehaNawal enabled auto-merge August 26, 2026 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team: SCM GitHub request for SCM area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants