Skip to content

[Subscription Billing] Page 8002 "Extend Contract" — move key variables to protected var and add setters for PageExtension automation #9715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Patrik Müller (pamura1977) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Subscription Billing] Page 8002 "Extend Contract" — move key variables to protected var and add setters for PageExtension automation #9715

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 33 additions & 7 deletions src/Apps/W1/Subscription Billing/App/Customer Contracts/Pages/ExtendContract.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@ page 8002 "Extend Contract"
ValidateSubscriptionEntryNo();

CountTotalServiceCommitmentPackage();

if VariantCodeParam <> '' then

@github-actions github-actions Bot Aug 10, 2026

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

In OnOpenPage, VariantCode := VariantCodeParam (lines 306-307) is applied unconditionally whenever VariantCodeParam is non-empty, but it runs after ValidateItemNo(), which resets VariantCode to '' whenever ItemNo is empty/invalid (line 418-419). If a caller invokes SetItemParameters with an invalid or empty ItemNo but a non-empty VariantCode (e.g. NewItemNo not passed, or an item that doesn't exist), the page ends up displaying/holding a VariantCode with no corresponding ItemNo, undoing the reset ValidateItemNo() just performed. This inconsistent state can flow into ExtendContract -> InsertFromItemNoAndCustomerContract with a variant code but a blank item number. Recommend guarding the assignment on ItemNo being non-empty, mirroring ValidateItemNo's own condition.

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

        if (ItemNo <> '') and (VariantCodeParam <> '') then
            VariantCode := VariantCodeParam;

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

VariantCode := VariantCodeParam;

CurrPage.Update();
end;

Expand Down Expand Up @@ -607,6 +611,10 @@ page 8002 "Extend Contract"
ExtendCustomerContract := ExtendCustomerContractParam;
UsageDataSupplierNo := UsageDataSupplierNoParam;
SubscriptionEntryNo := SubscriptionEntryNoParam;
ExtendVendorContract := ExtendVendorContractParam;
VendorContractNo := VendorContractNoParam;
ItemNo := ItemNoParam;
QuantityDecimal := QuantityParam;
end;

local procedure FillTempServiceCommitmentPackage()
Expand Down Expand Up @@ -667,6 +675,19 @@ page 8002 "Extend Contract"
SubscriptionEntryNoParam := NewSubscriptionEntryNo;
end;

procedure SetVendorContractParameters(NewExtendVendorContract: Boolean; NewVendorContractNo: Code[20])

@github-actions github-actions Bot Aug 10, 2026

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

The two new parameter-setter procedures added in this diff, SetVendorContractParameters (line 678) and SetItemParameters (line 684), are declared as plain procedure (public), while the page's pre-existing, analogous parameter setters SetParameters (line 641) and SetUsageBasedParameters (line 672) are declared internal procedure. These setters exist solely to let sibling code in the same app pass values into this page before it opens (a page-internal wiring pattern), so widening two of the four to public unnecessarily enlarges the page's external API surface and is inconsistent with the established convention on this same object. Recommend declaring both new procedures as internal procedure to match the existing setters unless external (cross-extension) callers are an explicit, intended requirement.

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

@pamura1977 Patrik Müller (pamura1977) Aug 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

SetVendorContractParameters and SetItemParameters, the two new setter procedures added in this PR, need to stay public. A PageExtension from our app has to call them before OnOpenPage runs, in order to populate the protected var fields requested in issue #8856.

begin
ExtendVendorContractParam := NewExtendVendorContract;
VendorContractNoParam := NewVendorContractNo;
end;

procedure SetItemParameters(NewItemNo: Code[20]; NewVariantCode: Code[10]; NewQuantity: Decimal)
begin
ItemNoParam := NewItemNo;
VariantCodeParam := NewVariantCode;
QuantityParam := NewQuantity;
end;

[IntegrationEvent(false, false)]
local procedure OnBeforeExtendContract()
begin
Expand All @@ -691,11 +712,6 @@ page 8002 "Extend Contract"
ContractItemMgt: Codeunit "Sub. Contracts Item Management";
ExtendContractMgt: Codeunit "Extend Sub. Contract Mgt.";
ImportAndProcessUsageData: Codeunit "Import And Process Usage Data";
CustomerContractNo: Code[20];
VendorContractNo: Code[20];
UnitPrice: Decimal;
UnitCostLCY: Decimal;
ProvisionStartDate: Date;
ProvisionStartDateEmptyErr: Label 'Provision Start Date cannot be empty.';
NoOfSelectedPackagesLbl: Label '%1 of %2', Comment = '%1 = No. of selected service commitment packages, %2 = Total service commitment packages';
SelectedServiceCommitmentPackages: Integer;
Expand All @@ -705,9 +721,13 @@ page 8002 "Extend Contract"
CustomerContractNoParam: Code[20];
ProvisionStartDateParam: Date;
ExtendCustomerContractParam: Boolean;
ExtendVendorContractParam: Boolean;
VendorContractNoParam: Code[20];
ItemNoParam: Code[20];
VariantCodeParam: Code[10];
QuantityParam: Decimal;
UsageDataSupplierNo: Code[20];
UsageDataSupplierNoParam: Code[20];
SubscriptionDescription: Text[100];
SubscriptionEntryNo: Integer;
SubscriptionEntryNoParam: Integer;
SupplierReferenceEntryNo: Integer;
Expand All @@ -716,7 +736,6 @@ page 8002 "Extend Contract"
ItemMissingServCommPackageTxt: Label 'No Subscription Package is available for this item.';
AssignServCommPackageToItemTxt: Label 'In order to extend the contract properly, please make sure that at least one package is assigned.';
ItemNoEmptyErr: Label 'Item No. must be specified.';
ItemDescription: Text[100];

protected var
ItemNo: Code[20];
Expand All @@ -725,4 +744,11 @@ page 8002 "Extend Contract"
ExtendCustomerContract: Boolean;
ExtendVendorContract: Boolean;
SellToCustomerNo: Code[20];
CustomerContractNo: Code[20];
VendorContractNo: Code[20];
SubscriptionDescription: Text[100];
ItemDescription: Text[100];
UnitPrice: Decimal;
UnitCostLCY: Decimal;
ProvisionStartDate: Date;
}
Loading