[Main] Item tracking validation when lot numbers are assigned concerns the warehouse pick level rather than at the sales order levelInitial Commit

[Shikhverma]

Bug 646429: [master] [REPAIR] [ALL-E] Item tracking validation when lot numbers are assigned concerns the warehouse pick level rather than at the sales order level

Fixes AB#646429

[Shikhverma]

Copilot resolve the merge conflicts in this pull request

[Copilot]

Copilot resolve the merge conflicts in this pull request

Resolved by merging main into this branch (534f0c7). The merge completed cleanly with no manual conflict resolution needed — git's rename detection handled the upstream reorganization of the SCM test directories, and both files added in this PR are preserved with unchanged content in their new SCM-Reservation locations.

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Performance}$

AddSelectedTrackingToDataSet drives its main loop over TempEntrySummary with FindFirst() instead of FindSet() before calling Next() in a repeat/until. Even on a temporary table this pattern is the wrong API for multi-row iteration and should use FindSet().

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

        if TempEntrySummary.FindSet() then
            repeat
                TempTrackingSpecification.SetTrackingFilterFromEntrySummary(TempEntrySummary);
                OnAddSelectedTrackingToDataSetOnAfterSetTrackingFilterFromEntrySummary(TempTrackingSpecification, TempEntrySummary);
                if TempTrackingSpecification.FindFirst() then begin
                    OnAddSelectedTrackingToDataSetOnBeforeUpdateWithChange(TempEntrySummary, TempTrackingSpecification, ChangeType::Modify);
                    TempTrackingSpecification.Validate("Quantity (Base)",
                      TempTrackingSpecification."Quantity (Base)" + TempEntrySummary."Selected Quantity");
                    TempTrackingSpecification."Buffer Status" := TempTrackingSpecification."Buffer Status"::MODIFY;
                    TransferExpDateFromSummary(TempTrackingSpecification, TempEntrySummary);
                    TempTrackingSpecification.Modify();
                    UpdateTrackingDataSetWithChange(TempTrackingSpecification, true, CurrentSignFactor, ChangeType::Modify);
                end else begin
                    TempTrackingSpecification := TrackingSpecification2;
                    TempTrackingSpecification."Entry No." := LastEntryNo + 1;
                    LastEntryNo := TempTrackingSpecification."Entry No.";
                    TempTrackingSpecification.CopyTrackingFromEntrySummary(TempEntrySummary);
                    TempTrackingSpecification."Buffer Status" := TempTrackingSpecification."Buffer Status"::INSERT;
                    TransferExpDateFromSummary(TempTrackingSpecification, TempEntrySummary);
                    if TempTrackingSpecification.IsReclass() then
                        TempTrackingSpecification.CopyNewTrackingFromTrackingSpec(TempTrackingSpecification);
                    OnAddSelectedTrackingToDataSetOnAfterCopyNewTrackingFromTrackingSpec(TempTrackingSpecification, ChangeType);
                    TempTrackingSpecification.Validate("Quantity (Base)", TempEntrySummary."Selected Quantity");
                    OnBeforeTempTrackingSpecificationInsert(TempTrackingSpecification, TempEntrySummary);
                    TempTrackingSpecification.Insert();
                    UpdateTrackingDataSetWithChange(TempTrackingSpecification, true, CurrentSignFactor, ChangeType::Insert);
                end;
            until TempEntrySummary.Next() = 0;

Knowledge:

• microsoft/knowledge/performance/pair-findset-with-next-loop.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6}

[alexei-dobriansky]

Good Sense Reviewer - Round 11

Recommendation: Request Changes

What this PR does

The change updates item tracking availability so outstanding unregistered warehouse and inventory pick quantities reduce the available lot quantity. It also nets reservations that already exist for the pick source, so the same allocation is not counted twice.

The main code path is targeted: GetAvailableLotQty rebuilds lookup data, RetrieveLookupData now adds unregistered picks before the availability summaries are used, and the new helper groups split pick lines before subtracting source reservations. The new W1 tests cover full pick allocation, inventory pick allocation, split pick lines, and production component source pointers. The RU test file tries to mirror that coverage, but it no longer declares helper procedures that both old and new tests call.

Status of previous suggestions

ID	Title	Status	Author response
S1	Previous round finding	Addressed
S2	Previous round finding	Addressed
S3	Previous round finding	Addressed

New observations (commits since round 10)

S4 (🔴 High): RU test codeunit misses required helpers
Keep the helper procedures in the RU test codeunit, or copy the new W1 helper set there too. The RU file now calls local procedures that it does not declare, so the RU test app will not compile.

Risk assessment and necessity

Risk: The product change touches lot availability for warehouse pick and inventory pick flows, so a wrong result can affect inventory allocation. The current blocking risk is compile-time: the RU test codeunit calls helper procedures that are absent from that codeunit.

Necessity: The change is needed because an unregistered pick can already commit lot quantity before registration. Without this update, another demand can still see that committed lot as available.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=9982 round=11 by=alexei-dobriansky at=2026-09-11T10:24:47Z lastSha=5b49f9e9472231d7fd1c5dedf37eb9ad8f596f66 reviewKey=0e4a939311b0d41d1d1684115b59ac291092de0d0d18c62fb78d324748956834 suggestions=S1@7e12c54f:addressed,S2@e5f33065:addressed,S3@4f8b8f99:addressed,S4@0937cd9b:new parentRound=10

[alexei-dobriansky]

Good Sense Reviewer - Round 12

Recommendation: Accept

What this PR does

The change updates item tracking availability so outstanding unregistered warehouse and inventory pick quantities reduce the available lot quantity. It also nets reservations that already exist for the pick source, so the same allocation is not counted twice.

The product logic remains targeted, and the latest commit only completes the RU test helper set that the newly added RU tests call. The current W1 and RU tests cover the reported warehouse pick scenario, inventory picks, split pick take lines, and production component source pointers.

Status of previous suggestions

ID	Title	Status	Author response
S1	Inventory picks still look available	Addressed	Still addressed.
S2	Limit loaded warehouse activity fields	Addressed	Still addressed.
S3	Production component pick mapping lacks coverage	Addressed	Still addressed.
S4	RU test codeunit misses required helpers	Addressed	The RU test codeunit now declares the helper procedures used by the old and new tests.

New observations (commits since round 11)

None - changes only addressed prior suggestions.

Risk assessment and necessity

Risk: The product change touches lot availability for warehouse pick and inventory pick flows, so a wrong result can affect inventory allocation. The latest commit changes only RU test helpers and removes the compile-time risk raised in the previous round.

Necessity: The change is needed because an unregistered pick can already commit lot quantity before registration. Without this update, another demand can still see that committed lot as available.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=9982 round=12 by=alexei-dobriansky at=2026-09-11T12:58:02.4386932Z lastSha=2a3d43d56cc3385bd29895bce890ea33899ea05b reviewKey=047d1ba642d3d3146dc3c32171e22a6e8b2a3c06b5abecf43dcb91a847e76c0b suggestions=S1@7e12c54f:addressed,S2@e5f33065:addressed,S3@4f8b8f99:addressed,S4@0937cd9b:addressed parentRound=11

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Events}$

TransferUnregisteredPicksToTempRec raises a paired OnBefore/OnAfter event set, but the if IsHandled then exit; guard after OnBeforeTransferUnregisteredPicksToTempRec returns before OnAfterTransferUnregisteredPicksToTempRec can fire. Any subscriber that handles the OnBefore event now also suppresses every OnAfter subscriber, breaking the expected override-vs-after-notification separation between the two events.

Knowledge:

• microsoft/knowledge/events/preserve-onafter-execution-when-ishandled-skips-the-body.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6}

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Events}$

The three new IntegrationEvent publisher signatures use abbreviated record parameter names (TempGlobalReservEntry, WhseActivLine) instead of full, self-explanatory names such as TempGlobalReservationEntry and WarehouseActivityLine. Event parameter names are part of the subscriber contract, so these abbreviations make the new extension points harder to understand and inconsistent with naming guidance.

Knowledge:

• microsoft/knowledge/events/name-event-parameters-without-abbreviations.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Performance}$

LocationRequiresPick reads only Location."Require Pick" but does a full-row Get on the Location table. Add SetLoadFields("Require Pick") before Get so this new per-tracking-spec availability check does not materialize every Location column just to test one flag.

Knowledge:

• microsoft/knowledge/performance/use-setloadfields-for-partial-records.md

Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6}

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Testing}$

GetAvailableLotQtyNetsProductionComponentReservationOnce calls CreateAndPostLotStockForPick(), which invokes ItemJournalLine.OpenItemTrackingLines(false) and opens the Item Tracking Lines modal page, but unlike its sibling new tests in this file, it is missing a [HandlerFunctions(...)] attribute (e.g. ItemTrackingLinesLotSNQtyModalPageHandler). The test will fail at runtime on the unhandled modal page before reaching its assertion.

Knowledge:

• microsoft/knowledge/testing/ui-handlers-in-tests.md

The same issue exists in these regional copies — apply the equivalent fix in each:

• src/Layers/RU/Tests/SCM-Reservation/SCMItemTracking.Codeunit.al:0 (RU)
  Line mapping was unavailable, so this was posted as an issue comment.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6}

[alexei-dobriansky]

Good Sense Reviewer - Round 13

Recommendation: Accept

What this PR does

The product change keeps unregistered warehouse and inventory pick quantities from being shown as available for the same tracked lot. Since round 12, the only code change adds the missing modal page handler to the W1 and RU production component tests.

That latest change is correct. The production component tests call the helper that opens Item Tracking Lines, and both tests now declare the handler needed for that modal page. The product code is unchanged in this round, and the latest checks show the affected app builds passing.

Status of previous suggestions

ID	Title	Status	Author response
S1	Inventory picks still look available	Addressed	Still addressed.
S2	Limit loaded warehouse activity fields	Addressed	Still addressed.
S3	Production component pick mapping lacks coverage	Addressed	Still addressed.
S4	RU test codeunit misses required helpers	Addressed	The latest commit also adds the missing handler attribute to the RU production component test.

New observations (commits since round 12)

None - changes only addressed prior suggestions.

Risk assessment and necessity

Risk: The product change affects tracked-lot availability when unregistered picks exist, so the main functional risk remains inventory allocation. The latest commit only changes test attributes, and it reduces test runtime risk by handling the modal page opened during stock setup.

Necessity: The change is needed because an unregistered pick can already commit lot quantity before registration. Without this update, another demand can still see that committed lot as available.

---

[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=9982 round=13 by=alexei-dobriansky at=2026-09-13T18:05:17.334Z lastSha=fead9e24ae2a042ed283133a94b3035072c1984c reviewKey=25420c1bc234aacd684938f2aaf9dfa35595a68b74a640cea27f3069b168b6e7 suggestions=S1@7e12c54f:addressed,S2@e5f33065:addressed,S3@4f8b8f99:addressed,S4@0937cd9b:addressed parentRound=12

[Predrag Maricic (PredragMaricic)]

The final implementation correctly excludes outstanding warehouse and inventory picks from tracked-lot availability while avoiding double-counting reservations already represented by the pick source. Split Take lines are aggregated before reservation netting, and production-order components use the correct source-line and source-subline mapping.

The W1 and RU regression coverage exercises warehouse picks, inventory picks, existing source reservations, split Take lines, and production components.

[github-actions]

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.

---

🔄 How to trigger a new status check:

1. 📤 Push a new commit to the PR branch, or
2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.