Skip to content
Closed
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 @@ -1155,7 +1155,8 @@ table 5407 "Prod. Order Component"
TestField("Qty. on Transfer Order (Base)", 0);
end;
#endif
ConfirmDeletion();
if not CalledFromHeader then
CheckPickedQtyBeforeDeletion();
end;

ProdOrderWarehouseMgt.ProdComponentDelete(Rec);
Expand Down Expand Up @@ -1254,7 +1255,7 @@ table 5407 "Prod. Order Component"
#pragma warning restore AA0470
Text99000009: Label 'Automatic reservation is not possible.\Do you want to reserve items manually?';
#pragma warning restore AA0074
ConfirmDeleteQst: Label '%1 = %2 is greater than %3 = %4. If you delete the %5, the items will remain in the operation area until you put them away.\Any related item tracking information defined during the pick process will be deleted.\Do you still want to delete the %5?', Comment = '%1 = FieldCaption("Qty. Picked"), %2 = "Qty. Picked", %3 = Qty. Posted, %4 = ("Expected Quantity" - "Remaining Quantity"), %5 = TableCaption';
CannotDeleteWithPickedQtyErr: Label 'You cannot delete the production order because one or more components have a picked quantity that has not been consumed. Consume or return the picked quantity before deleting the production order.';

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\ —\ Error\ Handling}$

The replacement error for picked-quantity deletion is hard-coded to "You cannot delete the production order..." even though this path is deleting a Prod. Order Component record. That gives the user the wrong operation name in the error dialog. Use component-specific text here, or parameterize the helper so the message can reflect the record actually being deleted.

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

CannotAutoReserveErr: Label 'Quantity %1 in line %2 cannot be reserved automatically.', Comment = '%1 - quantity, %2 - line number';
#pragma warning disable AA0470
ProgressMsg: Label 'Reserving inventory...\#1##############\@2@@@@@';
Expand Down Expand Up @@ -2341,30 +2342,10 @@ table 5407 "Prod. Order Component"
end;
end;

local procedure ConfirmDeletion()
internal procedure CheckPickedQtyBeforeDeletion()

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}$

Replacing the previous confirm-based deletion check with a hard Error() is a user-visible breaking change. Before this change, users could still proceed with deletion after acknowledging the warning, and locations whose production warehouse handling was configured as No Warehouse Handling bypassed the check entirely. Preserve that compatibility path or stage the stricter behavior as an explicit migration, otherwise existing production-order, line, and component deletion flows will start failing immediately.

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

begin
if CalledFromHeader then
exit;

if ("Expected Quantity" - "Remaining Quantity") < "Qty. Picked" then begin
if "Location Code" <> '' then begin
GetLocation("Location Code");
if Location."Prod. Output Whse. Handling" = Location."Prod. Consump. Whse. Handling"::"No Warehouse Handling" then
exit;
end;

if not Confirm(
StrSubstNo(
ConfirmDeleteQst,
FieldCaption("Qty. Picked"),
"Qty. Picked",
'Qty. Posted',
("Expected Qty. (Base)" - "Remaining Qty. (Base)"),
TableCaption),
false)
then
Error('');
end;
if ("Expected Qty. (Base)" - "Remaining Qty. (Base)") < "Qty. Picked (Base)" then
Error(CannotDeleteWithPickedQtyErr);
end;

procedure SuspendDeletionCheck(Suspend: Boolean)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,9 @@ table 5407 "Prod. Order Component"
if not ItemLedgEntry.IsEmpty() then
Error(Text99000000, "Item No.", "Line No.");

ConfirmDeletion();

if not CalledFromHeader then
CheckPickedQtyBeforeDeletion();
end;

ProdOrderWarehouseMgt.ProdComponentDelete(Rec);
Expand Down Expand Up @@ -1121,7 +1123,7 @@ table 5407 "Prod. Order Component"
#pragma warning restore AA0470
Text99000009: Label 'Automatic reservation is not possible.\Do you want to reserve items manually?';
#pragma warning restore AA0074
ConfirmDeleteQst: Label '%1 = %2 is greater than %3 = %4. If you delete the %5, the items will remain in the operation area until you put them away.\Any related item tracking information defined during the pick process will be deleted.\Do you still want to delete the %5?', Comment = '%1 = FieldCaption("Qty. Picked"), %2 = "Qty. Picked", %3 = Qty. Posted, %4 = ("Expected Quantity" - "Remaining Quantity"), %5 = TableCaption';
CannotDeleteWithPickedQtyErr: Label 'You cannot delete the production order because one or more components have a picked quantity that has not been consumed. Consume or return the picked quantity before deleting the production order.';

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\ —\ Error\ Handling}$

The new CannotDeleteWithPickedQtyErr text says the user cannot delete "the production order", but this label is raised from Prod. Order Component deletion and is also reused by the new Prod. Order Line deletion path. That makes the blocked operation read incorrectly for component and line deletes. Tailor the message to the actual delete target, or pass the caller caption into the shared check so each path reports the right entity.

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

CannotAutoReserveErr: Label 'Quantity %1 in line %2 cannot be reserved automatically.', Comment = '%1 - quantity, %2 - line number';
#pragma warning disable AA0470
ProgressMsg: Label 'Reserving inventory...\#1##############\@2@@@@@';
Expand Down Expand Up @@ -2205,30 +2207,10 @@ table 5407 "Prod. Order Component"
end;
end;

local procedure ConfirmDeletion()
internal procedure CheckPickedQtyBeforeDeletion()

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}$

Replacing the previous confirm-based deletion check with a hard Error() is a user-visible breaking change. Before this change, users could still proceed with deletion after acknowledging the warning, and locations whose production warehouse handling was configured as No Warehouse Handling bypassed the check entirely. Preserve that compatibility path or stage the stricter behavior as an explicit migration, otherwise existing production-order, line, and component deletion flows will start failing immediately.

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

begin
if CalledFromHeader then
exit;

if ("Expected Quantity" - "Remaining Quantity") < "Qty. Picked" then begin
if "Location Code" <> '' then begin
GetLocation("Location Code");
if Location."Prod. Output Whse. Handling" = Location."Prod. Consump. Whse. Handling"::"No Warehouse Handling" then
exit;
end;

if not Confirm(
StrSubstNo(
ConfirmDeleteQst,
FieldCaption("Qty. Picked"),
"Qty. Picked",
'Qty. Posted',
("Expected Qty. (Base)" - "Remaining Qty. (Base)"),
TableCaption),
false)
then
Error('');
end;
if ("Expected Qty. (Base)" - "Remaining Qty. (Base)") < "Qty. Picked (Base)" then
Error(CannotDeleteWithPickedQtyErr);
end;

procedure SuspendDeletionCheck(Suspend: Boolean)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,9 @@ table 5406 "Prod. Order Line"
Error(
Text99000000,
TableCaption, "Line No.", PurchLine.TableCaption());

if not CalledFromHeader then
CheckPickedQtyBeforeDeletion()
end;

ProdOrderLineReserve.DeleteLine(Rec);
Expand Down Expand Up @@ -1831,6 +1834,19 @@ table 5406 "Prod. Order Line"
CalledFromHeader := Suspend;
end;

local procedure CheckPickedQtyBeforeDeletion()
var
ProdOrderComponent: Record "Prod. Order Component";
begin
ProdOrderComponent.SetRange(Status, Status);
ProdOrderComponent.SetRange("Prod. Order No.", "Prod. Order No.");
ProdOrderComponent.SetRange("Prod. Order Line No.", "Line No.");
if ProdOrderComponent.FindSet() then
repeat
ProdOrderComponent.CheckPickedQtyBeforeDeletion();
until ProdOrderComponent.Next() = 0;
Comment on lines +1841 to +1847

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 production-order-line deletion scan loops over every component on the line even though only components with picked quantity can fail the new check. Filter "Qty. Picked (Base)" before FindSet() so the loop only evaluates rows that can actually block deletion.

Suggested change
ProdOrderComponent.SetRange(Status, Status);
ProdOrderComponent.SetRange("Prod. Order No.", "Prod. Order No.");
ProdOrderComponent.SetRange("Prod. Order Line No.", "Line No.");
if ProdOrderComponent.FindSet() then
repeat
ProdOrderComponent.CheckPickedQtyBeforeDeletion();
until ProdOrderComponent.Next() = 0;
ProdOrderComponent.SetRange(Status, Status);
ProdOrderComponent.SetRange("Prod. Order No.", "Prod. Order No.");
ProdOrderComponent.SetRange("Prod. Order Line No.", "Line No.");
ProdOrderComponent.SetFilter("Qty. Picked (Base)", '>%1', 0);
if ProdOrderComponent.FindSet() then
repeat
ProdOrderComponent.CheckPickedQtyBeforeDeletion();
until ProdOrderComponent.Next() = 0;

Knowledge:

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

end;

procedure GetRemainingPutAwayQty(): Decimal
begin
Rec.CalcFields("Put-away Qty. (Base)");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ table 5405 "Production Order"
end;

if Status = Status::Released then
ConfirmDeletion();
CheckPickedQtyBeforeDeletion();

if Status = Status::Finished then
DeleteFinishedProdOrderRelations()
Expand Down Expand Up @@ -868,7 +868,6 @@ table 5405 "Production Order"
Text010: Label 'You may have changed a dimension.\\Do you want to update the lines?';
Text011: Label 'You cannot change Finished Production Order dimensions.';
#pragma warning restore AA0074
ConfirmDeleteQst: Label 'The items have been picked. If you delete the Production Order, then the items will remain in the operation area until you put them away.\Related item tracking information that is defined during the pick will be deleted.\Are you sure that you want to delete the Production Order?';

protected var
HideValidationDialog: Boolean;
Expand Down Expand Up @@ -1647,20 +1646,16 @@ table 5405 "Production Order"
RemQtyBaseInvtItemProdOrdComp.Close();
end;

local procedure ConfirmDeletion()
local procedure CheckPickedQtyBeforeDeletion()
var
ProdOrderComponent: Record "Prod. Order Component";
Confirmed: Boolean;
begin
ProdOrderComponent.SetRange(Status, Rec.Status);

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 production-order deletion scan iterates every component for the order even though components with zero picked quantity can be excluded up front. Add a SetFilter("Qty. Picked (Base)", '>%1', 0) before FindSet() so rows that cannot trigger CheckPickedQtyBeforeDeletion() never cross the AL loop.

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

        ProdOrderComponent.SetRange(Status, Rec.Status);
        ProdOrderComponent.SetRange("Prod. Order No.", "No.");
        ProdOrderComponent.SetFilter("Qty. Picked (Base)", '>%1', 0);
        if ProdOrderComponent.FindSet() then
            repeat
                ProdOrderComponent.CheckPickedQtyBeforeDeletion();
            until ProdOrderComponent.Next() = 0;

Knowledge:

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

ProdOrderComponent.SetRange("Prod. Order No.", "No.");
if ProdOrderComponent.FindSet() then
repeat
if (ProdOrderComponent."Expected Quantity" - ProdOrderComponent."Remaining Quantity") < ProdOrderComponent."Qty. Picked" then begin
if not Confirm(ConfirmDeleteQst) then
Error('');
Confirmed := true;
end;
until (ProdOrderComponent.Next() = 0) or Confirmed;
ProdOrderComponent.CheckPickedQtyBeforeDeletion();
until ProdOrderComponent.Next() = 0;
end;

local procedure ValidateWarehousePutAwayLocation(ProductionOrder: Record "Production Order")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ codeunit 137298 "SCM Prod. Whse. Handling"
PickActivitiesCreatedMsg: Label 'Number of Invt. Pick activities created';
BinPickQtyIsWrongErr: Label 'Bin pick quantity is wrong.';
ShortageLineQtyIsWrongErr: Label 'Shortage line quantity is wrong.';
CannotDeleteWithPickedQtyErr: Label 'You cannot delete the production order because one or more components have a picked quantity that has not been consumed. Consume or return the picked quantity before deleting the production order.';
QtyPickedBaseShouldBePositiveErr: Label 'Qty. Picked (Base) should be positive after registering pick.';
ActConsumptionQtyShouldBeZeroErr: Label 'Act. Consumption (Qty) should be zero.';
ProdOrderShouldExistErr: Label 'Production Order should still exist after blocked deletion.';
ProdOrderComponentShouldExistErr: Label 'Production Order Component should still exist after blocked deletion.';
ProdOrderLineShouldExistErr: Label 'Production Order Line should still exist after blocked deletion.';

[Test]
[Scope('OnPrem')]
Expand Down Expand Up @@ -1698,6 +1704,131 @@ codeunit 137298 "SCM Prod. Whse. Handling"
LibraryVariableStorage.AssertEmpty();
end;

[Test]
procedure DeleteReleasedProdOrderBlockedWhenComponentHasPickedQty()
var
Location: Record Location;
ProductionOrder: Record "Production Order";
ProdOrderComponent: Record "Prod. Order Component";
begin
// [FEATURE] [AI test 0.4]
// [SCENARIO 647854] Deleting a released production order is blocked when a component has picked quantity not yet consumed.
Initialize();

// [GIVEN] Released production order "PO" with a fully registered warehouse pick and no consumption.
CreateProductionOrderWithRegisteredWarehousePick(ProductionOrder, Location);
FindPickedProdOrderComponent(ProdOrderComponent, ProductionOrder);
ProdOrderComponent.CalcFields("Act. Consumption (Qty)");
Assert.IsTrue(ProdOrderComponent."Qty. Picked (Base)" > 0, QtyPickedBaseShouldBePositiveErr);
Assert.AreEqual(0, ProdOrderComponent."Act. Consumption (Qty)", ActConsumptionQtyShouldBeZeroErr);
Commit();

// [WHEN] Delete the production order.
asserterror ProductionOrder.Delete(true);

// [THEN] Deletion is blocked and the production order still exists.
Assert.ExpectedError(CannotDeleteWithPickedQtyErr);
Assert.ExpectedErrorCode('Dialog');
Assert.IsTrue(
ProductionOrder.Get(ProductionOrder.Status, ProductionOrder."No."),
ProdOrderShouldExistErr);
end;

[Test]
procedure DeleteProdOrderComponentBlockedWhenComponentHasPickedQty()
var
Location: Record Location;
ProductionOrder: Record "Production Order";
ProdOrderComponent: Record "Prod. Order Component";
begin
// [FEATURE] [AI test 0.4]
// [SCENARIO 647854] Deleting a production order component is blocked when it has picked quantity not yet consumed.
Initialize();

// [GIVEN] Released production order "PO" with a fully registered warehouse pick.
CreateProductionOrderWithRegisteredWarehousePick(ProductionOrder, Location);
FindPickedProdOrderComponent(ProdOrderComponent, ProductionOrder);
Commit();

// [WHEN] Delete a picked production order component.
asserterror ProdOrderComponent.Delete(true);

// [THEN] Deletion is blocked and the component still exists.
Assert.ExpectedError(CannotDeleteWithPickedQtyErr);
Assert.ExpectedErrorCode('Dialog');
Assert.IsTrue(
ProdOrderComponent.Get(
ProdOrderComponent.Status, ProdOrderComponent."Prod. Order No.",
ProdOrderComponent."Prod. Order Line No.", ProdOrderComponent."Line No."),
ProdOrderComponentShouldExistErr);
end;

[Test]
procedure DeleteProdOrderLineBlockedWhenComponentHasPickedQty()
var
Location: Record Location;
ProductionOrder: Record "Production Order";
ProdOrderComponent: Record "Prod. Order Component";
ProdOrderLine: Record "Prod. Order Line";
begin
// [FEATURE] [AI test 0.4]
// [SCENARIO 647854] Deleting a production order line is blocked when a component has picked quantity not yet consumed.
Initialize();

// [GIVEN] Released production order "PO" with a fully registered warehouse pick.
CreateProductionOrderWithRegisteredWarehousePick(ProductionOrder, Location);
FindPickedProdOrderComponent(ProdOrderComponent, ProductionOrder);
ProdOrderLine.Get(
ProdOrderComponent.Status, ProdOrderComponent."Prod. Order No.", ProdOrderComponent."Prod. Order Line No.");
Commit();

// [WHEN] Delete the production order line that owns the picked component.
asserterror ProdOrderLine.Delete(true);

// [THEN] Deletion is blocked and the production order line still exists.
Assert.ExpectedError(CannotDeleteWithPickedQtyErr);
Assert.ExpectedErrorCode('Dialog');
Assert.IsTrue(
ProdOrderLine.Get(ProdOrderLine.Status, ProdOrderLine."Prod. Order No.", ProdOrderLine."Line No."),
ProdOrderLineShouldExistErr);
end;

local procedure CreateProductionOrderWithRegisteredWarehousePick(var ProductionOrder: Record "Production Order"; var Location: Record Location)
var
ParentItem: Record Item;
CompItem1: Record Item;
CompItem2: Record Item;
WarehouseActivityHeader: Record "Warehouse Activity Header";
WarehouseActivityLine: Record "Warehouse Activity Line";
begin
CreateProductionOrderWithLocationBinsAndTwoComponents(ProductionOrder, Location, ParentItem, CompItem1, CompItem2);
Location."Prod. Consump. Whse. Handling" := "Prod. Consump. Whse. Handling"::"Warehouse Pick (mandatory)";
Location.Modify(true);
LibraryManufacturing.RefreshProdOrder(ProductionOrder, false, true, true, true, false);

ProductionOrder.SetHideValidationDialog(true);
ProductionOrder.CreatePick(CopyStr(UserId(), 1, 50), 0, false, false, false);
FindWarehouseActivityLine(
WarehouseActivityLine, ProductionOrder."No.", WarehouseActivityLine."Activity Type"::Pick,
Location.Code, WarehouseActivityLine."Action Type"::Take);
WarehouseActivityHeader.Get(WarehouseActivityLine."Activity Type", WarehouseActivityLine."No.");
LibraryWarehouse.AutoFillQtyHandleWhseActivity(WarehouseActivityHeader);
LibraryWarehouse.RegisterWhseActivity(WarehouseActivityHeader);
end;

local procedure FindPickedProdOrderComponent(var ProdOrderComponent: Record "Prod. Order Component"; ProductionOrder: Record "Production Order")
begin
ProdOrderComponent.SetRange(Status, ProductionOrder.Status);
ProdOrderComponent.SetRange("Prod. Order No.", ProductionOrder."No.");
if ProdOrderComponent.FindSet() then
repeat
if ProdOrderComponent."Qty. Picked (Base)" > 0 then
exit;
until ProdOrderComponent.Next() = 0;

Error(QtyPickedBaseShouldBePositiveErr);
end;

local procedure Initialize()
var
LibraryERMCountryData: Codeunit "Library - ERM Country Data";
Expand Down Expand Up @@ -1802,7 +1933,10 @@ codeunit 137298 "SCM Prod. Whse. Handling"
CreateAndRefreshProductionOrder(ProductionOrder, "Production Order Status"::Released, "Prod. Order Source Type"::Item, ParentItem."No.", 1, Location.Code);
end;

local procedure CreateAndRefreshProductionOrder(var ProductionOrder: Record "Production Order"; ProdOrderStatus: Enum "Production Order Status"; SourceType: Enum "Prod. Order Source Type"; SourceNo: Code[20]; Quantity: Decimal; LocationCode: Code[10])
local procedure CreateAndRefreshProductionOrder(var ProductionOrder: Record "Production Order"; ProdOrderStatus: Enum "Production Order Status"; SourceType: Enum "Prod. Order Source Type";
SourceNo: Code[20];
Quantity: Decimal;
LocationCode: Code[10])
begin
LibraryManufacturing.CreateProductionOrder(ProductionOrder, ProdOrderStatus, SourceType, SourceNo, Quantity);
ProductionOrder.Validate("Location Code", LocationCode);
Expand Down Expand Up @@ -1857,7 +1991,8 @@ codeunit 137298 "SCM Prod. Whse. Handling"
ProdOrderLine.FindFirst();
end;

local procedure FindWarehouseActivityLine(var WarehouseActivityLine: Record "Warehouse Activity Line"; SourceNo: Code[20]; ActivityType: Enum "Warehouse Activity Type"; LocationCode: Code[10]; ActionType: Enum "Warehouse Action Type")
local procedure FindWarehouseActivityLine(var WarehouseActivityLine: Record "Warehouse Activity Line"; SourceNo: Code[20]; ActivityType: Enum "Warehouse Activity Type"; LocationCode: Code[10];
ActionType: Enum "Warehouse Action Type")
begin
WarehouseActivityLine.SetRange("Source No.", SourceNo);
WarehouseActivityLine.SetRange("Location Code", LocationCode);
Expand Down
Loading