Skip to content

[Master] - [Expense Agent] Fix saving Expense Location after category change - #10984

Draft
v-rohangarg20 wants to merge 1 commit into
mainfrom
bugs/expense-agent-save-location
Draft

[Master] - [Expense Agent] Fix saving Expense Location after category change#10984
v-rohangarg20 wants to merge 1 commit into
mainfrom
bugs/expense-agent-save-location

Conversation

@v-rohangarg20

@v-rohangarg20 v-rohangarg20 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes AB#648711

Problem

Saving an Expense Location after changing an Expense Report Line from a Meals category to a Per-diem category could result in a stale-record error.

Fix

Removed the redundant page-level OnValidate trigger for Expense Location, which called CurrPage.Update(). The standard field validation now saves the selected location without an extra page refresh or second save.

Validation

  • AL diagnostics are clean.
  • The Expense Agent app builds successfully.
  • Verified the repro scenario by changing the category and then selecting and saving an Expense Location.

@v-rohangarg20
v-rohangarg20 requested a review from a team September 3, 2026 08:16
@v-rohangarg20
v-rohangarg20 requested a review from a team as a code owner September 3, 2026 08:16
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 Team: Finance GitHub request for Finance area labels Sep 3, 2026
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Could not find a linked ADO work item. Please link one by using the pattern 'AB#' followed by the relevant work item number. You may use the 'Fixes' keyword to automatically resolve the work item when the pull request is merged. E.g. 'Fixes AB#1234'

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Accept with Suggestions

What this PR does

This change removes the page-level Expense Location validation trigger from the Expense Report subpage. The table validation still saves the selected location and applies the rule, but the page no longer does an extra CurrPage.Update() that can try to save an outdated line after rule evaluation changed the record.

Problem-solution fit

Fit: Strong

The reported scenario is a stale page record when a line is changed to a per diem category and then an expense location is selected. Removing the redundant page refresh fits that scenario and leaves the existing field validation in the table as the single save path.

Suggestions

S1 (🟠 Moderate): Add a page regression test
Please add a test that uses the Expense Report page to change a line from a non-per diem category to a per diem category, set Expense Location, and then reread the line. This protects the page-save path, because table-level tests do not cover the stale page record behavior.

Risk assessment and necessity

Risk: The change is limited to ExpenseReportSubPage.Page.al, but it affects an editable field whose table trigger can update rule, amount, and per diem details. The main risk is a UI regression where related totals or rule state do not refresh immediately after the location is changed.

Necessity: The change is needed because an extra page refresh after field validation can create a second save of an outdated record. The scope is narrow and keeps the business validation in the table trigger.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10984 round=1 by=alexei-dobriansky at=2026-09-03T13:19:14Z lastSha=c3e919b30fce12cc9163fae1dd4495384aca753f reviewKey=e545ee9a8d3b579cb01346a7247b77a7d3fd26216d554d79d195fd96978b92a4 suggestions=S1@acbc19a5

@v-rohangarg20 v-rohangarg20 changed the title [Master] Expense Agent: Fix saving Expense Location after category change [Master] - [Expense Agent] Fix saving Expense Location after category change Sep 3, 2026
@github-actions github-actions Bot added this to the Version 30.0 milestone Sep 6, 2026
@v-rohangarg20 v-rohangarg20 reopened this Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

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

Removing the page field OnValidate trigger from "Expense Location" drops the explicit CurrPage.Update() refresh, even though the table's OnValidate for "Expense Location" (ExpenseReportLine.Table.al) still mutates sibling fields that are also shown on this same subpage: it auto-populates "Starting Date and Time" and "Ending Date and Time" when they are blank, and calls ApplyRule() which can change "Rule Violations"/"Applied Rule Id"-driven state. Both "Starting Date and Time" (line 204) and "Ending Date and Time" (line 215) are rendered as fields on this page. Without CurrPage.Update(), a user validating "Expense Location" will not see these dependent fields refresh immediately in the UI until the record is otherwise re-read (e.g. navigating away and back). Recommend restoring the trigger, or confirming/documenting that the platform already refreshes the row after Validate() in this scenario.

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

                field("Expense Location"; Rec."Expense Location")
                {
                    ApplicationArea = Basic, Suite;
                    ToolTip = 'Specifies where the expense occurred. Available when the expense requires per diem details.';
                    ShowMandatory = IsPerDiemCategory;
                    Editable = IsPerDiemCategory;

                    trigger OnValidate()
                    begin
                        CurrPage.Update();
                    end;
                }

Line mapping was unavailable, so this was posted as an issue comment.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants