Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1816,6 +1816,8 @@ codeunit 7307 "Whse.-Activity-Register"
if Location."Bin Mandatory" then
CheckBinRelatedFields(GlobalWhseActivLine);

CheckItemTrackingRequiredForPutAway(GlobalWhseActivLine);

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.

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

RegisterActivityLines now calls the new CheckItemTrackingRequiredForPutAway inside the warehouse-line repeat/until loop, and that helper performs a persistent Item Ledger Entry lookup (FindLast) for every qualifying Put-away line. This is an N+1 database-access pattern on a hot registration path; SetLoadFields reduces the payload per row but does not remove the per-line lookup. Consider preloading/caching the required Item Ledger Entry data by item/variant/location/serial key once before iterating the activity lines, rather than issuing one lookup per line.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.31.4


OnAfterCheckWhseActivLine(GlobalWhseActivLine);

if ((GlobalWhseActivLine."Activity Type" = GlobalWhseActivLine."Activity Type"::Pick) or
Expand Down Expand Up @@ -1857,6 +1859,33 @@ codeunit 7307 "Whse.-Activity-Register"
Cust.CheckBlockedCustOnDocs(Cust, GlobalWhseActivHeader."Source Document", false, false);
end;

local procedure CheckItemTrackingRequiredForPutAway(WhseActivLine: Record "Warehouse Activity Line")

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.

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

This change introduces a new runtime contract in published codeunit 7307 "Whse.-Activity-Register": registering a Put-away for a serial+lot tracked item now fails with a TestField error when the line's Lot No. is blank while the matching Item Ledger Entry carries a Lot No., where the same flow previously completed successfully (the new test RegisterPutAwayWithBlankLotForSerialAndLotItemIsBlocked demonstrates exactly this). No public AL signature changed, but existing extensions, integrations, or already-open Put-away activity lines created before this change carry no compatibility or migration path — on upgrade they will start failing registration with no accompanying data fix-up. Treat this as a breaking behavioral change: provide a migration/backfill for existing open lines, an opt-in/feature-flag gate, or at minimum a clear release-note callout, and document the new OnBeforeCheckItemTrackingRequiredForPutAway event as the supported way for subscribers to preserve legacy behavior if needed.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.31.4

var
ItemLedgerEntry: Record "Item Ledger Entry";
IsHandled: Boolean;
begin
IsHandled := false;
OnBeforeCheckItemTrackingRequiredForPutAway(WhseActivLine, IsHandled);
if IsHandled then
exit;

if WhseActivLine."Activity Type" <> WhseActivLine."Activity Type"::"Put-away" then
exit;
if WhseActivLine."Serial No." = '' then
exit;

ItemLedgerEntry.SetCurrentKey("Item No.", "Variant Code", "Location Code", "Serial No.");

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.

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

The new Item Ledger Entry lookup uses SetCurrentKey("Item No.", "Variant Code", "Location Code", "Serial No."), but the W1 Item Ledger Entry table defines no key with that leading field order (verified in ItemLedgerEntry.Table.al: the closest candidates are Key20 = "Serial No.", "Item No.", Open, "Variant Code", Positive, "Location Code", "Posting Date" and Key17 = "Item No.", Open, "Variant Code", Positive, "Lot No.", "Serial No.", "Package No."). Because the selected key does not align with the actual filters (Item No., Variant Code, Location Code, Serial No., Open) applied before FindLast, the engine falls back to a less selective scan, and — since ordering is not guaranteed to match intent — if more than one open entry ever satisfies the same Item/Variant/Location/Serial combination, FindLast can pick an entry other than the one the caller intends, comparing the Put-away line's Lot No. against the wrong ledger entry. Use an existing key that actually leads with the applied filters (for example Key20, which starts with Serial No./Item No./Open) so the lookup is both efficient and deterministic.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.31.4

ItemLedgerEntry.SetRange("Item No.", WhseActivLine."Item No.");
ItemLedgerEntry.SetRange("Variant Code", WhseActivLine."Variant Code");
ItemLedgerEntry.SetRange("Location Code", WhseActivLine."Location Code");
ItemLedgerEntry.SetRange("Serial No.", WhseActivLine."Serial No.");
ItemLedgerEntry.SetRange(Open, true);
ItemLedgerEntry.SetLoadFields("Lot No.");
if ItemLedgerEntry.FindLast() then
if ItemLedgerEntry."Lot No." <> '' then
WhseActivLine.TestField("Lot No.", ItemLedgerEntry."Lot No.");
end;

local procedure CheckBinRelatedFields(WhseActivLine: Record "Warehouse Activity Line")
var
IsHandled: Boolean;
Expand Down Expand Up @@ -2449,6 +2478,11 @@ codeunit 7307 "Whse.-Activity-Register"
begin
end;

[IntegrationEvent(false, false)]
local procedure OnBeforeCheckItemTrackingRequiredForPutAway(WarehouseActivityLine: Record "Warehouse Activity Line"; var IsHandled: Boolean)
begin
end;

[IntegrationEvent(false, false)]
local procedure OnBeforePostedWhseRcptLineModify(var PostedWhseReceiptLine: Record "Posted Whse. Receipt Line"; WarehouseActivityLine: Record "Warehouse Activity Line")
begin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4359,6 +4359,58 @@ codeunit 137152 "SCM Warehouse - Receiving"
Assert.RecordCount(WarehouseEntry, ExpectedLinesCount);
end;

[Test]
[HandlerFunctions('ItemTrackingPageHandler,EnterQuantityToCreatePageHandler')]
[Scope('OnPrem')]
procedure RegisterPutAwayWithBlankLotForSerialAndLotItemIsBlocked()

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.

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

The new test RegisterPutAwayWithBlankLotForSerialAndLotItemIsBlocked verifies only the failure path after clearing the Lot No. It does not add a companion success-path case that keeps the matching Lot No. and proves RegisterWarehouseActivity still succeeds, so an implementation that blocked every serial-and-lot Put-away (not just mismatched ones) would still satisfy this coverage. Add a positive-path test (or extend this one) that registers the Put-away with the unchanged, matching Lot No. and asserts success.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.31.4

var
Item: Record Item;
PurchaseHeader: Record "Purchase Header";
WarehouseReceiptLine: Record "Warehouse Receipt Line";
WarehouseActivityLine: Record "Warehouse Activity Line";
LotNo: Variant;
Quantity: Decimal;
ItemTrackingMode: Option "Assign Lot No.","Assign Lot And Serial","Assign Serial No.","Select Entries","Assign Multiple Lot No";
begin
// [FEATURE] [Item Tracking] [Put-away]
// [SCENARIO 642316] Registering a Put-away for a serial- and lot-tracked item must fail when the Lot No. is cleared on a line,
// because the item ledger entry being put away carries that Lot No. and the warehouse entry must match it,
// otherwise Item Ledger Entries and Warehouse Bin Content become permanently mismatched.

// [GIVEN] Item with both serial and lot tracking.
Initialize();
Quantity := LibraryRandom.RandInt(10);
CreateItemWithItemTrackingCode(Item, true, true, LibraryUtility.GetGlobalNoSeriesCode(), LibraryUtility.GetGlobalNoSeriesCode()); // Taking True for Serial and Lot.

// [GIVEN] Warehouse receipt from a purchase order is posted with serial and lot assigned, creating a Put-away activity.
CreateWarehouseReceiptFromPurchaseOrder(PurchaseHeader, LocationWhite.Code, Item."No.", Quantity, Item."Base Unit of Measure");
FindWarehouseReceiptLine(
WarehouseReceiptLine, WarehouseReceiptLine."Source Document"::"Purchase Order", PurchaseHeader."No.", LocationWhite.Code);
LibraryVariableStorage.Enqueue(ItemTrackingMode::"Assign Lot And Serial"); // Enqueue for ItemTrackingPageHandler.
WarehouseReceiptLine.OpenItemTrackingLines();
PostWarehouseReceiptFromPurchaseOrder(PurchaseHeader."No.", LocationWhite.Code);
LibraryVariableStorage.Dequeue(LotNo); // Drain the Lot No. enqueued by ItemTrackingPageHandler.

// [GIVEN] The required Lot No. is cleared on a Put-away line, simulating a user leaving it blank.
FindWarehouseActivityLine(
WarehouseActivityLine, WarehouseActivityLine."Source Document"::"Purchase Order", PurchaseHeader."No.",
WarehouseActivityLine."Activity Type"::"Put-away");
WarehouseActivityLine.SetRange("Action Type", WarehouseActivityLine."Action Type"::Place);
WarehouseActivityLine.FindFirst();
WarehouseActivityLine.Validate("Lot No.", '');
WarehouseActivityLine.Modify(true);

// [WHEN] Registering the Put-away activity.
asserterror RegisterWarehouseActivity(
WarehouseActivityLine, WarehouseActivityLine."Source Document"::"Purchase Order", PurchaseHeader."No.",
WarehouseActivityLine."Activity Type"::"Put-away");

// [THEN] Registration is blocked because the line's Lot No. must equal the posted item ledger entry's Lot No.,
// preventing the ledger/bin content mismatch.
Assert.ExpectedTestFieldError(WarehouseActivityLine.FieldCaption("Lot No."), Format(LotNo));
LibraryVariableStorage.AssertEmpty();
end;

local procedure Initialize()
var
LibraryERMCountryData: Codeunit "Library - ERM Country Data";
Expand Down
Loading