Skip to content

Merge-ES-Excluded-from-Calculation-to-W1#9089

Open
Alexander-Ya wants to merge 5 commits into
mainfrom
bugs/Merge-ES-Excluded-from-Calculation-to-W1
Open

Merge-ES-Excluded-from-Calculation-to-W1#9089
Alexander-Ya wants to merge 5 commits into
mainfrom
bugs/Merge-ES-Excluded-from-Calculation-to-W1

Conversation

@Alexander-Ya

@Alexander-Ya Alexander-Ya commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

What & why

Linked work

Fixes AB#636627

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 (required — be specific: scenarios, commands, screenshots for UI changes)

Risk & compatibility

@Alexander-Ya Alexander-Ya requested a review from a team July 3, 2026 16:44
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Copilot PR Review

Iteration 5 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630

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

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

Findings produced by the Copilot CLI agent against BCQuality at 822cae1b2771ac25f665f73369f69093bd4fd630. Reply 👎 on any inline comment to flag false positives.

@github-actions

This comment was marked as resolved.

@Alexander-Ya Alexander-Ya added the Finance GitHub request for Finance area label Jul 5, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Jul 5, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR promotes the ES Excluded from calculation flag on detailed customer and vendor ledger entries into W1, then adds that flag to customer/vendor balance FlowFields and customer statement dataitems across the propagated layers. The direction is coherent: ES Cartera marks redrawal and rejection detailed entries as excluded, and the promoted filters keep those entries out of balances where ES already did so. The change is still not ready because propagation is incomplete, no W1 regression coverage was added for this financial calculation behavior, and one vendor-side cue remains inconsistent with the rest of the vendor balance changes.

Suggestions

S1 - Fix missing ES layer propagation
Verify App Changes is failing because src/Layers/ES/BaseApp/Finance/GeneralLedger/Setup/GeneralLedgerSetup.Table.al still needs to be integrated. Run the layer propagation and include the generated file so the PR can build and validate.

S2 - Add W1 regression coverage for balances
This promotes financial balance logic from ES to W1, but no test file changed and the PR gives no reason for skipping tests. Add W1 tests that create included and excluded detailed customer/vendor ledger entries and assert the affected balances and statement dataitems ignore the excluded entries.

S3 - Filter My Vendor balance too
table 9151 "My Vendor" still calculates Balance (LCY) from all detailed vendor ledger entries. Vendor redrawal and rejection entries can be marked as excluded, so the My Vendors cue can disagree with the Vendor card. Add the same "Excluded from calculation" = const(false) filter there, or document why that cue should include excluded entries.

Risk assessment and necessity

Risk: This changes core customer and vendor balance calculations for every W1-derived country layer. The default value is false, so countries that never set the flag should keep current balances, but any code that sets the flag now changes customer cards, vendor cards, ledger remaining amounts, general ledger setup due balances, currency due balances, and statements. The area is financially sensitive, and the current failed propagation check shows the layer set is not yet internally consistent.

Necessity: The change is necessary if ES Cartera behavior is being generalized: excluded redrawal/rejection detailed entries should not inflate normal balances. The scope is mostly right because ES-specific duplicate files are removed and W1 receives the common field and filters, but the PR must finish propagation, cover the new W1 behavior with tests, and fix the remaining vendor cue gap before merge.


[AI-PR-REVIEW] version=1 system=github pr=9089 round=1 by=alexei-dobriansky at=2026-07-06 lastSha=ca7ff6375861c500f9323f192034b247c85e6a58 suggestions=S1,S2,S3

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

@Alexander-Ya please review the suggestions

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

Labels

Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants