Repository navigation
Bug 651509: [29.x] ICM 51000002083463: Close Income Statement duplicates or omits mixed LCY/USD amounts with Balance closing, with and without global dimensions - #12768
Conversation
Backport-Source: github:#11894 Backport-Target: BCApps:releases/29.x Backport-Path-Set: 619360cade099740fe14264a176158d22de189784300f6b8c31790e455439333
Good Sense Reviewer - Round 1Recommendation: AcceptWhat this PR doesThis change fixes incorrect income-statement closing totals when source-currency groups are interleaved in G/L entry order. It replaces positional row skipping with a dictionary that processes each complete filtered group once. The group key keeps the existing account, date, source-currency, business-unit, and selected global-dimension filters. Repeated records are skipped before journal aggregation, and the tracking dictionary is reset for each account. The W1, ES, and IT implementations are aligned, and the tests verify exact amounts, dimensions, journal balancing, posting, source-currency totals, and repeated closing behavior. Problem-solution fitFit: Strong The change directly removes the invalid assumption that entries in a filtered currency group are contiguous. The regression scenarios reproduce the incorrect totals before the fix and cover the affected closing configurations after the fix. SuggestionsNone. Risk assessment and necessityRisk: This affects financially sensitive year-end closing and retained-earnings amounts. The implementation is narrow, preserves the existing grouping criteria, is aligned across W1, ES, and IT, and has detailed regression coverage. Visiting each distinct filtered group may cost more than positional skipping, but the dictionary prevents repeated sum calculations and no realistic usability risk is evident. Necessity: The change is required because the previous logic can create incorrect closing and retained-earnings totals. Manual correction is not a safe workaround for a posting process.
|
Create an isolated source currency with a deterministic 0.25 exchange rate effective from the fiscal-year start, removing dependency on demo USD rates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77a301b0-00ce-45a1-b98d-b8e078f1adaa
Good Sense Reviewer - Round 2Recommendation: AcceptWhat this PR doesThe new commit makes the regression setup independent of a pre-existing USD record. Each test now creates a fresh foreign currency with an exchange rate and uses that generated code consistently. This keeps the intended 1:4 source-currency ratio, removes a country-data dependency, and leaves the production closing logic unchanged since round 1. The W1, CH, and IT test setup remains aligned, and the affected integration test suites pass. Status of previous suggestionsNo suggestions were raised in round 1. New observations (commits since round 1)None. Risk assessment and necessityRisk: The underlying change still affects financially sensitive year-end closing, but this round changes only test data setup and wording. It does not alter production behavior, public APIs, or the grouping rules reviewed in round 1. Necessity: The test update is needed so the mixed-currency regression scenarios have a valid exchange rate on every country test database instead of depending on how USD was already configured.
|
Align the localized request-page value queue and use the retained earnings account selected by the IT report. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c30597be-a58b-4f56-8e03-a2d16659ef63 (cherry picked from commit 1deed91)
Good Sense Reviewer - Round 3Recommendation: Request ChangesWhat this PR doesThe new commit aligns the Italian request-page queue with the localized report fields and uses one account for both the net-profit and net-loss closing lines. The queue order now matches the handler, so the earlier value shift is fixed. However, the new account lookup adds analyzer warnings that fail both Italian build variants, so this round is not ready to merge. Status of previous suggestionsNo suggestions were raised in round 2. New observations (commits since round 2)S1 (🔴 High): Remove new AA0210 warnings from IT helper Risk assessment and necessityRisk: The production closing logic is unchanged in this round. The test update currently fails the IT build warning gate, so the localized regression tests cannot provide merge-ready validation. Necessity: The queue alignment is needed for the localized request page and the financially sensitive closing tests. The account lookup must be changed so the test fix passes the repository's analyzer gate.
|
Use the account category totaling filter instead of filtering on fields without keys. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8716f85d-9b36-4a18-9a38-10f368725094
Good Sense Reviewer - Round 4Recommendation: AcceptWhat this PR doesThe new commit changes the Italian test helper that finds the retained-earnings account. It replaces direct filters on non-key fields with a scan of the account categories and uses the category's existing totaling filter to select the account by its primary key. The production closing logic is unchanged. The earlier analyzer warnings are removed from the new helper, while the test still selects an account assigned to the retained-earnings equity category and now reports a clear error when no such account exists. Status of previous suggestions
New observations (commits since round 3)None. Risk assessment and necessityRisk: This remains a financially sensitive year-end closing scenario, but this round changes only test setup. The lookup uses existing account-category behavior, does not change production code or public APIs, and fails clearly if the required setup is missing. Necessity: The change is needed so the localized regression tests can pass the analyzer gate without using non-key filters. It preserves coverage for the retained-earnings posting path.
|
Good Sense Reviewer - Round 5Recommendation: AcceptWhat this PR doesThis fixes year-end closing totals when local- and foreign-source-currency entries are interleaved. Each filtered group is summed once instead of skipping rows by position. The new commit explicitly loads the fields used after aggregation in the W1, ES, and IT reports. This avoids later partial-record loads after the amount fields hold calculated group totals. The currency test helper keeps the same exchange rate while adding posting-account setup. The Italian test supplies a balancing account in Details mode, matching the localized report's later assignment, while keeping Balance mode unchanged. Problem-solution fitFit: Strong The grouping change addresses duplicated or omitted currency groups. The latest changes preserve those totals through subsequent field reads and make the localized test setup follow the report's posting contract. Status of previous suggestions
New observations (commits since round 4)None. Risk assessment and necessityRisk: Year-end closing is financially sensitive. The new production changes use the same field-loading list in all three report implementations and do not change grouping filters, event signatures, or public procedures. Tests check exact mixed-currency closing totals, dimensions, balanced journals, and zero balances after posting; separate tests cover grouped and ungrouped local-source-currency totals. All 166 completed non-skipped checks pass, with two checks skipped. Local AL builds and tests were not run during this review. Necessity: Without the fix, interleaved currency groups can produce incorrect closing journals and leave income accounts incorrectly closed. Loading the aggregation and dimension fields before processing protects the calculated values, and the test setup changes support the existing posting scenarios.
|
4 participants
Backport of #11894 for AB#651509