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 @@ -5,6 +5,7 @@
namespace Microsoft.Manufacturing.Subcontracting;

using Microsoft.Manufacturing.WorkCenter;
using Microsoft.Purchases.Document;

pageextension 99001506 "Subc. Work Center Card" extends "Work Center Card"
{
Expand Down Expand Up @@ -49,12 +50,24 @@ pageextension 99001506 "Subc. Work Center Card" extends "Work Center Card"
RunPageLink = "Work Center No." = field("No.");
ToolTip = 'View the Subcontracting WIP Entries that track work-in-progress quantities at this work center''s subcontracting location.';
}
action("Subcontractor Dispatch List")
Comment thread
ChethanT marked this conversation as resolved.
Comment thread
ChethanT marked this conversation as resolved.
Comment thread
ChethanT marked this conversation as resolved.

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{🟠\ High\ Severity\ —\ AppSource}$

The pageextension adds a new, unaffixed action "Subcontractor Dispatch List" to the base "Work Center Card" page. Members (fields/controls/actions) added to a standard/base object must carry the extension's reserved affix individually — the object-level affix on the pageextension itself is not sufficient. Without an affix on this action's name, it can collide with an identically-named action added by another extension, which AppSourceCop's AS0011 rule is designed to catch.

Knowledge:

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

{
ApplicationArea = Subcontracting;
Caption = 'Subcontractor - Dispatch List';
Enabled = IsSubcontractingWorkCenter;
Image = Print;
ToolTip = 'Print the dispatching list for the subcontractor assigned to this work center.';

trigger OnAction()
var
PurchaseHeader: Record "Purchase Header";
begin
PurchaseHeader.SetRange("Buy-from Vendor No.", Rec."Subcontractor No.");
Report.Run(Report::"Subc. Dispatching List", true, false, PurchaseHeader);
end;
Comment on lines +64 to +67

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 OnAction trigger in the "Subcontractor Dispatch List" action (SubcWorkCenterCard.PageExt.al) filters Purchase Header by Rec."Subcontractor No." and runs the report without first validating that a Subcontractor No. is present. The action's Enabled property gates normal UI invocation, but nothing in the trigger itself enforces the prerequisite, so any other caller (automation, a future promoted/context-menu entry point, or an Enabled-bypass) can run the report with an empty vendor filter. Add Rec.TestField("Subcontractor No.") before building the filter so the failure is explicit and self-documenting.

Suggested change
begin
PurchaseHeader.SetRange("Buy-from Vendor No.", Rec."Subcontractor No.");
Report.Run(Report::"Subc. Dispatching List", true, false, PurchaseHeader);
end;
begin
Rec.TestField("Subcontractor No.");
PurchaseHeader.SetRange("Buy-from Vendor No.", Rec."Subcontractor No.");
Report.Run(Report::"Subc. Dispatching List", true, false, PurchaseHeader);
end;

Knowledge:

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

}
}
}
modify("Subcontractor - Dispatch List")
{
Enabled = IsSubcontractingWorkCenter;
}
}

trigger OnOpenPage()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ codeunit 139990 "Subc. Subcontracting UI Test"
WorkCenterCard.GotoRecord(WorkCenter);

// [THEN] Subcontractor - Dispatch List action is not enabled
Assert.IsFalse(WorkCenterCard."Subcontractor - Dispatch List".Enabled(), SubcontractingActionsEnabledErr);
Assert.IsFalse(WorkCenterCard."Subcontractor Dispatch List".Enabled(), SubcontractingActionsEnabledErr);
Comment thread
ChethanT marked this conversation as resolved.

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

These tests now assert only the new page-extension action WorkCenterCard."Subcontractor Dispatch List". After this PR removed the extension's modify("Subcontractor - Dispatch List") { Enabled = IsSubcontractingWorkCenter; } block, there is no remaining test assertion covering the base page's original "Subcontractor - Dispatch List" action, so its (now default-enabled) behavior is no longer verified at all. Add or retain an assertion covering the base action's enabled/obsolete state, or confirm intentionally dropping its coverage is safe.

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

WorkCenterCard.Close();
end;

Expand All @@ -476,7 +476,7 @@ codeunit 139990 "Subc. Subcontracting UI Test"
WorkCenterCard.GotoRecord(WorkCenter);

// [THEN] Subcontractor - Dispatch List action is enabled
Assert.IsTrue(WorkCenterCard."Subcontractor - Dispatch List".Enabled(), SubcontractingActionsNotEnabledErr);
Assert.IsTrue(WorkCenterCard."Subcontractor Dispatch List".Enabled(), SubcontractingActionsNotEnabledErr);
WorkCenterCard.Close();
end;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ page 99000754 "Work Center Card"
action("Subcontractor - Dispatch List")
{
ApplicationArea = Manufacturing;
Caption = 'Subcontractor - Dispatch List';
Caption = 'Subcontractor - Dispatch List (Obsolete)';

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{🟠\ High\ Severity\ —\ Upgrade}$

Same issue as the W1 layer: the "Subcontractor - Dispatch List" action's Caption is renamed to 'Subcontractor - Dispatch List (Obsolete)' without setting ObsoleteState/ObsoleteReason/ObsoleteTag. The action remains fully callable and unconditionally enabled, so there is no compiler-enforced deprecation signal or migration path for consumers still referencing it.

Knowledge:

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

Image = "Report";
//The property 'PromotedCategory' can only be set if the property 'Promoted' is set to 'true'
//PromotedCategory = "Report";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ page 99000754 "Work Center Card"
action("Subcontractor - Dispatch List")
{
ApplicationArea = Manufacturing;
Caption = 'Subcontractor - Dispatch List';
Caption = 'Subcontractor - Dispatch List (Obsolete)';

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{🟠\ High\ Severity\ —\ Upgrade}$

The base page action "Subcontractor - Dispatch List" (Work Center Card) is being deprecated by only appending "(Obsolete)" to its Caption text, while a new differently-named action "Subcontractor Dispatch List" is introduced in the page extension to replace it. The old action is never entered into AL's formal obsoletion lifecycle: no ObsoleteState, ObsoleteReason, or ObsoleteTag is set. Consumers or extensions that customize, automate, or reference the old control by name (including the previous modify("Subcontractor - Dispatch List") pattern this PR removes) get no compiler warning and no structured migration guidance pointing them to the new action/report path. A caption-only rename is not a substitute for the supported obsolete lifecycle and does not prevent continued use of the deprecated control.

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

            action("Subcontractor - Dispatch List")
            {
                ApplicationArea = Manufacturing;
                Caption = 'Subcontractor - Dispatch List (Obsolete)';
                ObsoleteState = Pending;
                ObsoleteReason = 'Replaced by action "Subcontractor Dispatch List" on the Subcontracting page extension, which runs report "Subc. Dispatching List" filtered to the work center''s subcontractor.';
                ObsoleteTag = '29.0';
                Image = "Report";
                //The property 'PromotedCategory' can only be set if the property 'Promoted' is set to 'true'
                //PromotedCategory = "Report";
                RunObject = Report "Subcontractor - Dispatch List";
            }

Knowledge:

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

Image = "Report";
//The property 'PromotedCategory' can only be set if the property 'Promoted' is set to 'true'
//PromotedCategory = "Report";
Expand Down
Loading