[Master]-Slice 425446: Payment Journal Batch Total

[ViditGupta1277]

Fixes AB#425446

Requirement:
Would be nice to have a Batch Total for Payment Journal. Currently, have to post a payment journal before knowing how much was selected for payment.

Balance and Total Balance is only related to balancing the transaction for each row and not useful for batch totals.

[github-actions]

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

The new OnCalcBatchTotalOnAfterCopyFilters publisher is added without the XML summary/param documentation used by neighboring GenJnlManagement events, which makes the subscriber contract less discoverable. Add matching event documentation in the ES/NL/W1 GenJnlManagement.Codeunit.al publishers.

_{Agent judgement — not directly backed by a BCQuality knowledge article.}

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

[github-actions]

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

The new OnBeforeUpdateBatchTotal publisher is added without the XML summary/param documentation that the surrounding Payment Journal events use, so subscribers lose the inline contract description for this new extensibility point. Add matching event documentation for each localized PaymentJournal.Page.al publisher.

_{Agent judgement — not directly backed by a BCQuality knowledge article.}

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

[github-actions]

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

Adding UpdateBalance() (which now also calls UpdateBatchTotal()) to the Amount, Amount (LCY), Debit Amount, and Credit Amount OnValidate triggers makes every amount edit recompute batch-wide aggregates through CalcBalance and CalcBatchTotal (CalcSums("Balance (LCY)"), Rec.Count(), and CalcSums("Amount (LCY)")). This pattern is duplicated across all localized PaymentJournal.Page.al copies in the PR, so large payment batches now pay several SQL aggregate round-trips on each line edit instead of one. Prefer updating the displayed totals from the old/new line delta, or defer the full batch refresh until the record changes instead of re-aggregating the whole batch on every field validation.

_{Agent judgement — not directly backed by a BCQuality knowledge article.}

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

[github-actions]

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

CalcBatchTotal adds a new public entry point on the shared GenJnlManagement library codeunit, but unlike the surrounding public procedures it has no XML documentation. Add a <summary> and <param> doc comments so extensions and callers can understand the method contract without reading the implementation.

Knowledge:

• microsoft/knowledge/style/xmldoc-for-public-library-procedures.md

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

[github-actions]

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

CalcBatchTotal adds a new public entry point on the shared GenJnlManagement library codeunit (added identically in ES, NL, and W1 layers), but unlike the surrounding public procedures it has no XML documentation. Add a <summary> and <param> doc comments so extensions and callers can understand the method contract without reading the implementation.

Knowledge:

• microsoft/knowledge/style/xmldoc-for-public-library-procedures.md

The same issue exists in these regional copies — apply the equivalent fix in each:

• src/Layers/ES/BaseApp/Finance/GeneralLedger/Journal/GenJnlManagement.Codeunit.al:620 (ES)

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

[github-actions]

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

Across the APAC/CH/CZ/ES/IT/W1 ERMGeneralJournalUT additions, the new Batch Total coverage only exercises the default interactive client path. Production code explicitly sets ShowBatchTotal := false for SOAP/OData/ODataV4/API client types, but none of these tests bind a Test Client Type Subscriber and assert that hidden-path contract. Add a client-type-driven unit test per layer that forces a non-UI client type, calls CalcBatchTotal, and verifies ShowBatchTotal = false and BatchTotal remains 0.

_{Agent judgement — not directly backed by a BCQuality knowledge article.}

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

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This Slice adds a Batch Total (LCY) field to Payment Journal pages across localized layers, backed by GenJnlManagement.CalcBatchTotal, and adds tests for totals, empty batches, batch changes, and amount edits. The feature goal is valid: users need a batch-level payment total before posting. The sum itself uses the current page filters and Amount (LCY), which matches the stated batch-total behavior. The main correctness risk is that the page recalculates the total during amount field validation by reading persisted lines, but the edited line has not been explicitly saved first.

Suggestions

S1 - Save before recalculating edited amounts
The new amount-field UpdateBalance() calls recalculate Batch Total from persisted Gen. Journal Line records. Save the current line first, or adjust the total with the Rec/xRec amount delta, so the edited amount is included before the user leaves the line.

S2 - Avoid extra full-batch sums per edit
Each amount edit now runs the existing balance refresh and another full-batch CalcSums("Amount (LCY)") across every localized Payment Journal page. Large payment batches can pay several aggregate queries per edit. Consider updating Batch Total from the edited-line delta, or refresh it once after the record is saved.

S3 - Document new public extension points
CalcBatchTotal and the new Batch Total events expose new integration points, but they do not have the XML summary and param comments used by nearby public procedures and events. Add those comments in W1, ES, NL, and the page copies so subscribers understand the filters, amount sign, and visibility contract.

Risk assessment and necessity

Risk: This is a financial UI surface, so a stale Batch Total can mislead users about the amount selected for payment before posting. The diff does not change posting or ledger-entry creation, but it adds a public helper and event publishers and it runs extra aggregate calculations on Payment Journal navigation and amount validation in all localized page copies.

Necessity: The change is justified by Slice 425446 and the Ideas-site request: Balance and Total Balance do not answer the batch-level payment-total question. The scope is mostly right because all Payment Journal page copies are covered and tests were added, but the edited-line refresh path should be fixed before merge.

---

[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10538 round=1 by=alexei-dobriansky at=2026-08-23T07:14:54Z lastSha=3ad85272d7b0e240ea0a4fb0419810ef39752787 reviewKey=29558c8ec93c737f8d43bebe7d56afb7dce690edd55dc6dc9caaecf91c7ebfc2 suggestions=S1@392321d0,S2@b721127e,S3@f9d4d535

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling}$

UpdateBatchTotalAfterAmountChange() forces an explicit CurrPage.SaveRecord() and then reruns the full-batch CalcBatchTotal from the OnValidate triggers of Amount, Amount (LCY), Debit Amount, and Credit Amount — even though the amount-field validation path already saves via CheckAmountMatchedToAppliedLines(). This second save re-enters OnModifyRecord (payment-file validation, approval status cleanup) purely to refresh BatchTotal, so save-time errors can surface from the new UI-refresh path, and every amount edit now pays an extra save plus a full-batch CalcSums — which scales poorly on large payment journals. Recalculate the total from the already-saved record and drop the redundant second save.

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

    local procedure UpdateBatchTotalAfterAmountChange()
    begin
        UpdateBatchTotal();
    end;

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

[github-actions]

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

CalcBatchTotal introduces a new public journal-total calculation seam, but unlike the adjacent CalcBalance flow it exposes no OnBefore.../OnAfter... integration events around the filter-copy / CalcSums work. That leaves extensions no supported way to adjust which lines contribute to Batch Total or react to the computed value without duplicating the procedure, so this new total becomes a hard extensibility wall.

Knowledge:

• microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.md

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

[github-actions]

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

The new UpdateBatchTotal / UpdateBatchTotalAfterAmountChange path on Payment Journal is wired into UpdateBalance() and multiple amount OnValidate triggers, but it adds no OnBefore.../OnAfter... publishers even though the existing sibling UpdateBalance routine already exposes them. Page extensions therefore cannot suppress, replace, or post-process the new batch-total refresh flow through supported events.

Knowledge:

• microsoft/knowledge/events/publish-thin-onbefore-onafter-integration-events.md

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

[github-actions]

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

UpdateBalance now unconditionally calls UpdateBatchTotal on every current-record refresh and OnNewRecord. That means each row navigation/new line adds another full filtered CalcSums over Gen. Journal Line on top of the existing CalcBalance work, and the same pattern is duplicated across all localized Payment Journal page copies in this PR. Compute Batch Total only when the batch/filter context changes, or fold it into the existing balance calculation instead of issuing a third aggregate query per refresh.

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

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

[github-actions]

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

The new CalcBatchTotal unit test only exercises the visible branch (ShowBatchTotal = true). GenJnlManagement.CalcBatchTotal now suppresses Batch Total for ClientType::SOAP, OData, ODataV4, and Api, but this codeunit never binds Test Client Type Subscriber to verify that ShowBatchTotal becomes false and BatchTotal stays 0 for those clients. Add a negative client-type test so the suppression branch cannot regress silently.

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

[github-actions]

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

The new CalcBatchTotal unit test only exercises the visible branch (ShowBatchTotal = true). GenJnlManagement.CalcBatchTotal now suppresses Batch Total for ClientType::SOAP, OData, ODataV4, and Api, but this codeunit never binds Test Client Type Subscriber to verify that ShowBatchTotal becomes false and BatchTotal stays 0 for those clients. Add a negative client-type test so the suppression branch cannot regress silently.

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

[github-actions]

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

The new CalcBatchTotal unit test only exercises the visible branch (ShowBatchTotal = true). GenJnlManagement.CalcBatchTotal now suppresses Batch Total for ClientType::SOAP, OData, ODataV4, and Api, but this codeunit never binds Test Client Type Subscriber to verify that ShowBatchTotal becomes false and BatchTotal stays 0 for those clients. Add a negative client-type test so the suppression branch cannot regress silently.

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

[github-actions]

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

The new CalcBatchTotal unit test only exercises the visible branch (ShowBatchTotal = true). GenJnlManagement.CalcBatchTotal now suppresses Batch Total for ClientType::SOAP, OData, ODataV4, and Api, but this codeunit never binds Test Client Type Subscriber to verify that ShowBatchTotal becomes false and BatchTotal stays 0 for those clients. Add a negative client-type test so the suppression branch cannot regress silently.

The same issue exists in these regional copies — apply the equivalent fix in each:

• src/Layers/CZ/Tests/General Journal/ERMGeneralJournalUT.Codeunit.al:6432 (CZ)
• src/Layers/IT/Tests/General Journal/ERMGeneralJournalUT.Codeunit.al:6432 (IT)

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

[github-actions]

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

This PR adds a new public procedure CalcBatchTotal on codeunit 230 in W1/ES/NL, but within the change its only consumers are the in-box Payment Journal pages and new tests. Publishing the helper as a public API makes its current signature part of the app's upgrade contract for dependent extensions, so changing or removing it after release becomes a breaking change. If external consumption is not intended, narrow the surface before the release baseline is cut.

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

[alexei-dobriansky]

Agentic PR Review - Round 2

Recommendation: Accept with Suggestions

What this PR does

This round updates the Payment Journal Batch Total implementation after round 1. Amount validation now saves the current line and refreshes only Batch Total, so the edited amount is included without forcing the full balance refresh path. The new Batch Total filter hook events were also removed. The feature still matches the Slice goal: show the total selected for payment at batch level, with tests for totals, empty batches, batch switching, and amount edits.

Status of previous suggestions

ID	Title	Status	Author response
S1	Save before recalculating edited amounts	Addressed	The amount validation path now calls CurrPage.SaveRecord() before recalculating Batch Total.
S2	Avoid extra full-batch sums per edit	Addressed	The amount validation path no longer calls the full UpdateBalance() flow; it refreshes only Batch Total after save.
S3	Document new public extension points	Not addressed	No author reply found. The new event publishers were removed, but CalcBatchTotal is still a new public procedure without XML summary and parameter comments.

New observations (commits since round 1)

None - the new commit only addresses prior review feedback.

Risk assessment and necessity

Risk: This is a financial UI surface, so a wrong or stale Batch Total can mislead users before posting. The round-2 change lowers the main correctness risk by saving the edited line before recalculating. It does not change posting or ledger entries. One non-blocking maintainability issue remains: the new public CalcBatchTotal procedure should document its filter and visibility contract.

Necessity: The Slice remains justified by the Ideas-site request. Balance and Total Balance do not show the batch-level amount selected for payment, and the scope covers the Payment Journal page copies plus relevant tests.

---

[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10538 round=2 by=alexei-dobriansky at=2026-08-23T19:06:01Z lastSha=5101561d1f37efcc74a8e0040d6ba68dc92bde16 reviewKey=42c5d6b913f5616a33244e458d211df2a80cecb9755ca83e13e22dd133205fd4 suggestions=S1@392321d0:addressed,S2@b721127e:addressed,S3@f9d4d535:notaddressed parentRound=1