Skip to content

APIV2: replace LoadFields with AddLoadFields #29955

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

Closed
jeffreybulanadi wants to merge 2 commits into from
Closed

APIV2: replace LoadFields with AddLoadFields #29955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1b23df
fix: Replace LoadFields with AddLoadFields in APIV2 pages
jeffreybulanadi Apr 21, 2026
124cb66
fix: use OnInit instead of OnOpenPage for AddLoadFields in BankAccounts
jeffreybulanadi Apr 29, 2026
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
6 changes: 5 additions & 1 deletion Apps/W1/APIV2/app/src/pages/APIV2BankAccounts.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ page 30051 "APIV2 - Bank Accounts"
CurrencyCodeDoesNotMatchACurrencyErr: Label 'The "currencyCode" does not match to a Currency.', Comment = 'currencyCode is a field name and should not be translated.';
CurrencyValuesDontMatchErr: Label 'The currency values do not match to a specific Currency.';

trigger OnInit()
begin
Rec.AddLoadFields("Currency Code");
Copy link
Copy Markdown
Contributor

@nikolakukrika nikolakukrika Apr 28, 2026

Choose a reason for hiding this comment

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

We had issues with this kind of approach in the past. API page does not behave 100% as the actual page. In some cases the platform would open and close the page. Many properties that would be set from the OnOpen page would be lost. I'm not sure if we still have this kind of behavior on e.g. delete, POST, get $batch and other operations that can be done.

Copy link
Copy Markdown
Contributor

@nikolakukrika nikolakukrika Apr 28, 2026

Choose a reason for hiding this comment

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

We would need an extensive testing before we uptake this change to address the risk.

Copy link
Copy Markdown
Author

@jeffreybulanadi jeffreybulanadi Apr 29, 2026

Choose a reason for hiding this comment

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

Good point. The BankAccounts page is different from the others in this PR because OnOpenPage was newly added just for AddLoadFields - there was no pre-existing OnOpenPage. This means that for API operations that may not trigger OnOpenPage (POST via OnInsertRecord, PATCH via OnModifyRecord, batch requests), AddLoadFields would not fire and LoadCurrencyInformation would fall back to a JIT read.

Switched OnOpenPage to OnInit for the BankAccounts page. OnInit fires when the page object is instantiated, before any trigger or data operation runs, making it the correct placement for AddLoadFields when there is no pre-existing OnOpenPage.

For the other pages in this PR (SalesInvoices, SalesCreditMemos, SalesQuotes, PurchaseInvoices, PurchaseCreditMemos), OnOpenPage already existed for permission checks. Adding AddLoadFields alongside those existing calls carries the same risk profile as the permission checks themselves, which are already relied on in production.

Regarding extensive testing - agreed. This change can be tested with API requests for GET (list and single), POST, PATCH, and DELETE on the Bank Accounts endpoint to confirm Currency Code is always correctly returned.

Copy link
Copy Markdown
Contributor

@nikolakukrika nikolakukrika May 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

We decided to skip this optimization for the time being to limit the risk.

end;

trigger OnAfterGetRecord()
begin
LoadCurrencyInformation();
Expand All @@ -127,7 +132,6 @@ page 30051 "APIV2 - Bank Accounts"
var
GraphMgtGeneralTools: Codeunit "Graph Mgt - General Tools";
begin
Rec.LoadFields("Currency Code");
CurrencyCodeTxt := GraphMgtGeneralTools.TranslateNAVCurrencyCodeToCurrencyCode(LCYCurrencyCode, Rec."Currency Code");
end;
}
2 changes: 1 addition & 1 deletion Apps/W1/APIV2/app/src/pages/APIV2PurchaseCreditMemos.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,7 @@ page 30083 "APIV2 - Purchase Credit Memos"
begin
CheckDataUpgrade();
SetPermissionsFilters();
Rec.AddLoadFields("Applies-to Doc. Type", "Currency Code");
end;

var
Expand Down Expand Up @@ -670,7 +671,6 @@ page 30083 "APIV2 - Purchase Credit Memos"

local procedure SetCalculatedFields()
begin
Rec.LoadFields("Applies-to Doc. Type", "Currency Code");
SetInvoiceId();
CurrencyCodeTxt := GraphMgtGeneralTools.TranslateNAVCurrencyCodeToCurrencyCode(LCYCurrencyCode, Rec."Currency Code");
end;
Expand Down
2 changes: 1 addition & 1 deletion Apps/W1/APIV2/app/src/pages/APIV2PurchaseInvoices.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,7 @@ page 30042 "APIV2 - Purchase Invoices"
trigger OnOpenPage()
begin
CheckPermissions();
Rec.AddLoadFields("Currency Code");
end;

var
Expand Down Expand Up @@ -622,7 +623,6 @@ page 30042 "APIV2 - Purchase Invoices"

local procedure SetCalculatedFields()
begin
Rec.LoadFields("Currency Code");
CurrencyCodeTxt := GraphMgtGeneralTools.TranslateNAVCurrencyCodeToCurrencyCode(LCYCurrencyCode, Rec."Currency Code");
end;

Expand Down
2 changes: 1 addition & 1 deletion Apps/W1/APIV2/app/src/pages/APIV2SalesCreditMemos.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,7 @@ page 30038 "APIV2 - Sales Credit Memos"
trigger OnOpenPage()
begin
SetPemissionsFilters();
Rec.AddLoadFields("Applies-to Doc. Type", "Currency Code");
end;

var
Expand Down Expand Up @@ -691,7 +692,6 @@ page 30038 "APIV2 - Sales Credit Memos"

local procedure SetCalculatedFields()
begin
Rec.LoadFields("Applies-to Doc. Type", "Currency Code");
SetInvoiceId();
CurrencyCodeTxt := GraphMgtGeneralTools.TranslateNAVCurrencyCodeToCurrencyCode(LCYCurrencyCode, Rec."Currency Code");
end;
Expand Down
2 changes: 1 addition & 1 deletion Apps/W1/APIV2/app/src/pages/APIV2SalesInvoices.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,7 @@ page 30012 "APIV2 - Sales Invoices"
trigger OnOpenPage()
begin
SetPermissionFilters();
Rec.AddLoadFields("No.", "Currency Code", "Amount Including VAT", Posted, Status);
end;

var
Expand Down Expand Up @@ -772,7 +773,6 @@ page 30012 "APIV2 - Sales Invoices"

local procedure SetCalculatedFields()
begin
Rec.LoadFields("No.", "Currency Code", "Amount Including VAT", Posted, Status);
GetRemainingAmount();
CurrencyCodeTxt := GraphMgtGeneralTools.TranslateNAVCurrencyCodeToCurrencyCode(LCYCurrencyCode, Rec."Currency Code");
end;
Expand Down
2 changes: 1 addition & 1 deletion Apps/W1/APIV2/app/src/pages/APIV2SalesQuotes.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,7 @@ page 30037 "APIV2 - Sales Quotes"
trigger OnOpenPage()
begin
CheckPermissions();
Rec.AddLoadFields("Currency Code");
end;

var
Expand Down Expand Up @@ -717,7 +718,6 @@ page 30037 "APIV2 - Sales Quotes"

local procedure SetCalculatedFields()
begin
Rec.LoadFields("Currency Code");
CurrencyCodeTxt := GraphMgtGeneralTools.TranslateNAVCurrencyCodeToCurrencyCode(LCYCurrencyCode, Rec."Currency Code");
end;

Expand Down
Loading