[Event Request] codeunit 367 "CheckManagement": add OnFinancialVoidCh…

[Explorer986]

What & why

Adds OnFinancialVoidCheckOnBeforePostBalanceAccount (an IsHandled event) to codeunit 367 "CheckManagement" so partners can replace the balance-account posting in FinancialVoidCheck — the exact extension point the requester needed but could not get from the too-early existing events. The large case CheckLedgEntry."Bal. Account Type" block was extracted verbatim into a new FinancialVoidPostBalanceAccount local procedure so the guard reads cleanly.

Linked work

Fixes AB#640242

How I validated this

• I read the full diff and it contains only changes I intended.
• I built the affected app(s) locally with no new analyzer warnings.
• I ran the change in Business Central and confirmed it behaves as expected.
• I added or updated tests for the new behavior, or explained below why none are needed.

What I tested and the outcome

• Diff review: the case body is moved unchanged into FinancialVoidPostBalanceAccount (with ConfirmFinancialVoid.GetVoidType()/GetVoidDate() passed in as VoidType/VoidDate params); default path (IsHandled := false) calls it, preserving behavior. AL language server reports no errors.

Risk & compatibility

Low-medium. Behavior is preserved when unsubscribed, but this is core financial-void posting, so it deserves the regression check above. The new IsHandled event lets a subscriber skip the standard bal.-account posting - document if the event is treated as public. No schema/data impact.

[github-actions]

Copilot PR Review

Iteration 4 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain	Findings	Knowledge-backed	Agent	Inline	Fallback
Events	1	1	0	1	0

Totals: 1 knowledge-backed · 0 agent findings.

Orchestrator pre-filter (2 file(s) excluded)

• layer-disabled (knowledge) : 2 file(s)

_{Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives.}

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR adds OnFinancialVoidCheckOnBeforePostBalanceAccount in codeunit 367 so an extension can replace the standard balance-account posting during FinancialVoidCheck. It also moves the existing balance-account case block into FinancialVoidPostBalanceAccount, and the default path still calls that block when IsHandled stays false.

The event is in the right part of the flow: after the void bank line is posted and before the balance-account entries are posted. The standard path keeps updating BalanceAmountLCY, closing the bank ledger entry, posting rounding, marking the check entries void, and committing as before. The blocker is the handled path: a subscriber can skip the standard posting, but the event does not pass all context needed to safely do an equivalent replacement.

Suggestions

S1 - Pass posting context to handled subscribers
Add the current GenJnlPostLine instance and the selected void type/date to OnFinancialVoidCheckOnBeforePostBalanceAccount. A subscriber that sets IsHandled := true must be able to post replacement lines through the same posting instance and match the same unapply decision. Without that context, the later rounding, mark-void, and commit steps can run after an incomplete replacement.

Risk assessment and necessity

Risk: This is core financial void posting in W1, NA, APAC, and RU. The unsubscribed path looks behavior-preserving, including BalanceAmountLCY feeding the rounding calculation. The risk is for subscribers of the new IsHandled event: the standard balance-account posting and unapply block can be skipped, while the procedure still closes ledger state, posts rounding if needed, marks check entries void, commits, and updates analysis views.

Necessity: The event request is valid because the existing early event forces extensions to replace too much of FinancialVoidCheck. The scope is close to the right extension point, and missing tests are not by itself a blocker for this event request. The event still needs the missing posting context before it is safe as a replacement seam in this posting-adjacent flow.

---

[AI-PR-REVIEW] version=1 system=github pr=9421 round=1 by=alexei-dobriansky at=2026-07-14T17:02:29Z lastSha=acf90107064ab1e2e47b0e25a8060adcc9fe0641 suggestions=S1

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Events} \quad \color{gray}{\texttt{\small Iteration\ 4}}$

OnFinancialVoidCheckOnBeforeCheckBalAccountType already shipped without an IsHandled parameter, and this change adds var IsHandled: Boolean directly to it, retrofitting an override seam onto an event that existing subscribers only ever knew as a plain notification.

The caller now wraps the standard Bal. Account Type posting logic in if not IsHandled then, so any subscriber that previously just observed this event can now unintentionally suppress the default posting (G/L, Customer, Vendor) if a future subscriber sets IsHandled without the author realizing the semantics changed. BCQuality guidance is to leave the existing event untouched and add a new dedicated OnBefore event carrying IsHandled instead.

Knowledge:

• microsoft/knowledge/events/do-not-add-ishandled-to-an-existing-event.md

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

[alexei-dobriansky]

Agentic PR Review - Round 2

Recommendation: Request Changes

What this PR does

The new commit removes the separate OnFinancialVoidCheckOnBeforePostBalanceAccount event and changes the existing OnFinancialVoidCheckOnBeforeCheckBalAccountType event instead. It adds var BalanceAmountLCY and var IsHandled, and the surrounding code now skips the standard balance-account case block when IsHandled is true.

This addresses only the rounding part of the previous concern. A handled subscriber still does not get the current GenJnlPostLine instance, and it still does not get the selected void type. The void date can be inferred from the journal line posting date, but the unapply decision cannot be matched safely without the void type.

Status of previous suggestions

ID	Title	Status	Author response
S1	Pass posting context to handled subscribers	Not addressed	The response says IsHandled and BalanceAmountLCY were added so a subscriber can bypass the case block and keep rounding balanced. The new event signature still does not pass GenJnlPostLine or the void type.

New observations (commits since round 1)

S2 - Do not add IsHandled to existing event
Keep OnFinancialVoidCheckOnBeforeCheckBalAccountType as the existing notification event and add a new dedicated handled event instead. Adding var IsHandled changes the contract and meaning of an event that subscribers may already use. This can break subscribers or let one subscriber unexpectedly skip the standard financial void posting for all others.

Risk assessment and necessity

Risk: This is still core financial void posting in W1, NA, APAC, and RU. The round-2 change also changes a public integration event signature and turns an existing notification event into an override seam. That is a compatibility risk on top of the original posting risk.

Necessity: The event request is still valid because extensions need a narrower point to replace balance-account posting. The replacement seam must be safe for posting and safe for existing subscribers. The current shape is not safe enough because it keeps the handled path under-contextualized and changes an existing event instead of adding a dedicated one.

---

[AI-PR-REVIEW] version=1 system=github pr=9421 round=2 by=alexei-dobriansky at=2026-07-15T17:04:30Z lastSha=f36f6719919d30399abc3815a6ad931aa0e8b807 suggestions=S1:notaddressed,S2:new parentRound=1