Fix E-Document purchase draft header Sub Total overwrite; notify on mismatch by ventselartur · Pull Request #9711 · microsoft/BCApps

ventselartur · 2026-07-24T11:22:46Z

Why

On the E-Document purchase draft page, editing a line recalculated the header "Sub Total" from the sum of the lines and silently overwrote the value that came in on the incoming document. This masked genuine discrepancies between the supplier-provided document total and the captured lines, so users could finalize a draft whose header total did not actually reflect the source document (bug 642364). The header total should be preserved, and the user should instead be warned when it diverges from the lines beyond a rounding tolerance.

Summary

Stopped overwriting the purchase draft header "Sub Total" from the lines — the incoming document total is now preserved.
Added a tolerance-based mismatch check (rounding-precision × line count) that compares the header Sub Total against the sum of the lines.
Added a "Sub Total Mismatch" notification type with Dismiss / Don't-show-again actions on the purchase draft subform.
Added a "Sub Total Mismatch Dismissed" flag on the E-Document Purchase Header so a dismissal persists and re-arms only when the user edits an amount.
Added telemetry logging for detected mismatches (with and without tolerance) including header/lines totals, tolerance, and difference.
Added comprehensive tests covering mismatch notification, dismissal persistence, re-arming on amount edits, and total invariants.

Fixes AB#642364

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1715353-5157-420a-8c46-89c1d255b62f

…smatch (642364) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1715353-5157-420a-8c46-89c1d255b62f

…notification (642364) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1715353-5157-420a-8c46-89c1d255b62f

…ce setup, cleanup) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1715353-5157-420a-8c46-89c1d255b62f

…lify recalc Add AL test AddingLineTriggersSubTotalMismatchNotification verifying the Sub Total Mismatch notification is shown when a new draft line makes the sum of the lines diverge from the header Sub Total. Tag existing tests with [AI test] and add Initialize/filter helpers. Simplify UpdateCalculatedAmounts to always re-check totals (drop UpdateParentRecord flag) and remove the redundant beyond-tolerance telemetry message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 00495e7a-6f16-4a80-b771-1977bec17731

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec60766f-5fac-4cc4-b894-2dd0de933ca4

…arm on amount edits

…atch

github-actions · 2026-07-24T11:32:18Z

$\textbf{🟠\ High\ Severity\ —\ Performance}$

UpdateCalculatedAmounts(false) is invoked unconditionally from the subform's OnAfterGetRecord, which fires once per visible line every time the user scrolls, sorts, or refreshes. It now always calls CheckSubTotalMatchesLines, which re-reads and re-sums every line of the document (an O(n) FindSet loop) and can Insert/Delete rows in the 'E-Document Notification' table and re-send all pending notifications for the document. For a draft with n lines this makes opening/scrolling the page O(n^2) and turns simple row navigation into repeated database writes, matching the 'Do not Modify inside OnAfterGetRecord' anti-pattern (DB writes belong behind an explicit user action, not per-row display). Previously this branch (UpdateParentRecord = false) exited immediately without touching the header or other lines.

Recommendation:

only recompute/notify from OnValidate triggers on Quantity/Unit Price/Total Discount (already wired via UpdateCalculatedAmounts(true)), and drop the per-row call from OnAfterGetRecord, or cache the last-checked totals so the mismatch check runs once per document load rather than once per row.

Knowledge:

microsoft/knowledge/performance/do-not-modify-in-onaftergetrecord.md

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

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4}

github-actions · 2026-07-24T11:32:19Z

+    var
+        EDocumentNotification: Record "E-Document Notification";
+        MyNotifications: Record "My Notifications";
+        SubTotalMismatchMsg: Label 'The document total does not match the sum of the lines. Review the amounts before finalizing the draft.';


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

The new SubTotalMismatchMsg Label in AddSubTotalMismatchNotification is declared inside the procedure's local var block instead of the codeunit's top-level var block, which hides it from object-level translation review and is fragile in XLIFF extraction across BC versions.

Recommendation:

move SubTotalMismatchMsg (and similarly-scoped new labels) to the codeunit-level var section, alongside the existing VendorMatchedByNameNotAddressMsg pattern this file already partly follows for other labels.

Knowledge:

microsoft/knowledge/style/labels-declared-at-object-scope.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4}

github-actions · 2026-07-24T11:32:21Z

+        CustomDimensions.Add('Difference', Format(Difference, 0, 9));
+
+        if Difference <> 0 then
+            Telemetry.LogMessage('', SubTotalMismatchNoToleranceTxt, Verbosity::Normal, DataClassification::SystemMetadata, TelemetryScope::All, CustomDimensions);


$\textbf{🟠\ High\ Severity\ —\ Telemetry}$

Telemetry.LogMessage is called with an empty string as the event ID ('', SubTotalMismatchNoToleranceTxt, ...). The event ID is the field telemetry consumers (Application Insights queries, KQL dashboards, alert rules) pivot on; an empty string is a placeholder that is unsearchable and indistinguishable from any other event that omits an ID.

Recommendation:

assign a real, registered, stable event ID (e.g. an 'EDOC-nnnn' style prefix already used elsewhere in this app) instead of ''.

Knowledge:

microsoft/knowledge/telemetry/telemetry-event-id-stable-unique.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4}

github-actions · 2026-07-24T11:32:22Z

+            Telemetry.LogMessage('', SubTotalMismatchNoToleranceTxt, Verbosity::Normal, DataClassification::SystemMetadata, TelemetryScope::All, CustomDimensions);
+
+        if Difference > Tolerance then begin
+            Telemetry.LogMessage('', SubTotalMismatchNotificationShownTxt, Verbosity::Normal, DataClassification::SystemMetadata, TelemetryScope::All, CustomDimensions);


$\textbf{🟠\ High\ Severity\ —\ Telemetry}$

Same issue as the neighboring call: Telemetry.LogMessage('', SubTotalMismatchNotificationShownTxt, ...) also uses an empty-string event ID, making this second new telemetry signal equally unsearchable and impossible to distinguish from other placeholder-ID events.

Knowledge:

microsoft/knowledge/telemetry/telemetry-event-id-stable-unique.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4}

github-actions · 2026-07-24T11:32:24Z

+        EDocument.DeleteAll();
+    end;
+
+    local procedure CreatePurchaseDraft(var EDocument: Record "E-Document"; var EDocumentPurchaseHeader: Record "E-Document Purchase Header"; HeaderSubTotal: Decimal)


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

The new test's local CreatePurchaseDraft/CreatePurchaseLine helpers hand-roll Record.Insert() on 'E-Document Purchase Header' and 'E-Document Purchase Line' instead of reusing the existing test-library fixtures in LibraryEDocument.Codeunit.al (InsertForEDocument via MockPurchaseDraftPrepared, and InsertPurchaseDraftLine), which already encode how these records should be created. Duplicating fixture-construction logic in the test file risks drifting from the library helpers as the schema evolves.

Recommendation:

extend LibraryEDocument with a parameterized header/line creation overload (accepting the custom Sub Total / amounts these tests need) rather than reimplementing insertion locally.

Knowledge:

microsoft/knowledge/testing/use-library-codeunits-for-test-fixtures.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4}

mynjj · 2026-07-24T11:37:40Z

+        Evaluate(EDocumentEntryNo, Notification.GetData(EDocumentNotification.FieldName("E-Document Entry No.")));
+        Evaluate(Id, Notification.GetData(EDocumentNotification.FieldName(ID)));
+        if EDocumentPurchaseHeader.Get(EDocumentEntryNo) then begin
+            EDocumentPurchaseHeader."Sub Total Mismatch Dismissed" := true;


maybe we can move this Sub Total Mismatch Dismissed to the EDocNotification table? then if it has it, it's just not launched? It feels a bit out of place in EDocPurchsaeHeader

mynjj · 2026-07-24T11:38:31Z

+            EDocumentPurchaseHeader."Sub Total Mismatch Dismissed" := false;
+            EDocumentPurchaseHeader.Modify();
+        end;
+        CheckSubTotalMatchesLines(EDocumentPurchaseHeader);


We could maybe avoid going down this path if we already know the notification is disabled?

mynjj · 2026-07-24T11:42:15Z

+        Tolerance: Decimal;
+        LineCount: Integer;
+    begin
+        RoundingPrecision := EDocumentImportHelper.GetCurrencyRoundingPrecision(EDocPurchaseHeader."Currency Code");


very unlikely, but just paranoid, this could be negative, so maybe an abs inside this proc wouldn't hurt

mynjj

.

ventselartur added 12 commits July 16, 2026 11:20

feat(edoc): add Sub Total Mismatch notification type and handlers

d4422bb

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1715353-5157-420a-8c46-89c1d255b62f

fix(edoc): stop overwriting purchase draft header total; notify on mi…

326f199

…smatch (642364) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1715353-5157-420a-8c46-89c1d255b62f

test(edoc): cover purchase draft header total invariant and mismatch …

e6e7785

…notification (642364) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1715353-5157-420a-8c46-89c1d255b62f

test(edoc): strengthen draft totals tests (precondition assert, servi…

4f7b286

…ce setup, cleanup) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c1715353-5157-420a-8c46-89c1d255b62f

test(edoc): expand purchase draft totals coverage

7d00b83

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ec60766f-5fac-4cc4-b894-2dd0de933ca4

add more tests

0efa0b7

feat(edoc): add Sub Total Mismatch Dismissed flag to purchase header

0711826

fix(edoc): suppress dismissed sub total mismatch notification and re-…

b84c6d0

…arm on amount edits

fix(edoc): persist dismissal on header when dismissing sub total mism…

51b9dff

…atch

test(edoc): cover sub total mismatch dismissal persistence and re-arm

06b453c

merge from main

67953b0

ventselartur requested review from a team July 24, 2026 11:22

ventselartur temporarily deployed to triage July 24, 2026 11:22 — with GitHub Actions Inactive

github-actions Bot added AL: Apps (W1) Add-on apps for W1 Integration GitHub request for Integration area labels Jul 24, 2026

github-actions Bot added this to the Version 29.0 milestone Jul 24, 2026

github-actions Bot reviewed Jul 24, 2026

View reviewed changes

mynjj reviewed Jul 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix E-Document purchase draft header Sub Total overwrite; notify on mismatch#9711

Fix E-Document purchase draft header Sub Total overwrite; notify on mismatch#9711
ventselartur wants to merge 12 commits into
mainfrom
bugs/642364-edoc-purchase-draft-header-totals

ventselartur commented Jul 24, 2026 •

edited by github-actions Bot

Loading

Uh oh!

github-actions Bot commented Jul 24, 2026

Uh oh!

github-actions Bot Jul 24, 2026

Uh oh!

github-actions Bot Jul 24, 2026

Uh oh!

github-actions Bot Jul 24, 2026

Uh oh!

github-actions Bot Jul 24, 2026

Uh oh!

mynjj Jul 24, 2026

Uh oh!

mynjj Jul 24, 2026

Uh oh!

mynjj Jul 24, 2026

Uh oh!

mynjj left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ventselartur commented Jul 24, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Summary

Uh oh!

github-actions Bot commented Jul 24, 2026

Uh oh!

github-actions Bot Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mynjj Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mynjj Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mynjj Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

mynjj left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ventselartur commented Jul 24, 2026 •

edited by github-actions Bot

Loading