Concurrent posting for General Ledger entries

[Alexander-Ya]

What & why

Linked work

Fixes AB#624164

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

[github-actions]

Copilot PR Review

Iteration 6 · 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
Agent	1	0	1	0	0
Breaking Changes	1	1	0	0	0

Totals: 1 knowledge-backed · 1 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.}

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 2}}$

This PR adds several new conditional code paths gated on GeneralLedgerSetup.UseConcurrentPosting() — sequence-based G/L Entry/VAT Entry numbering in InitNextEntryNo/StartPosting/ContinuePosting, the new GetNextGLEntryNoForPosting/GetNextVATEntryNoForPosting helpers, the PreviousEntryNo rollback path in UpdateGLEntryNo, and the new ValidateSequenceNo call for VAT Entry — none of which are exercised by any test file in this diff (no *.Test.al / *.Codeunit.al test file appears in the changed-file list).

Given the change affects core posting entry-numbering logic across every localization layer, the absence of any accompanying automated test for the new UseConcurrentPosting = true branch is a real coverage gap for a change this central to posting correctness. Impact would ordinarily warrant more than a minor flag, but this is capped per agent-finding rules since no BCQuality knowledge file backs the specific gap.

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

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 3}}$

CreateGeneralLedgerSetup.Codeunit.al now unconditionally sets "Use Concurrent Posting" := true for every Contoso demo company, in every localization, with the comment "to test concurrent posting for all countries".

The comment itself signals this is leftover test-enablement code rather than an intended product default; it silently changes G/L posting behavior for anyone who installs the Contoso demo dataset in any country. (Note: the statement also references GeneralledgerSetup, a different-cased spelling of the GeneralLedgerSetup variable; AL identifiers are case-insensitive so this is not a functional bug, but it reinforces that the line reads as an unreviewed, ad hoc addition.)

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

        GeneralLedgerSetup.Modify(true);

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

When GLSetup.UseConcurrentPosting() is enabled, G/L, VAT, and register numbers are taken from global sequences instead of FindLast()+1 under a table lock. This changes the previously contiguous, gap-free numbering behavior that extensions may infer from G/L Register "From Entry No."/"To Entry No." ranges and from sequential VAT-entry allocation: concurrent sessions can interleave sequence values, so range-based consumers of these ledgers can no longer assume gap-free or strictly-ordered numbering within a register. This is an intentional trade-off of the feature, but it is a behavior change worth calling out explicitly in release/upgrade notes for extension authors who filter by entry-number ranges. Severity is capped at minor per the agent-finding precision bar even though the practical impact for affected extensions could be larger; if this is confirmed to break real consumers it should be promoted to a documented, knowledge-backed breaking change.

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

The new "SIFT Bucket No." field on G/L Entry is populated from "Entry No." mod 5 in OnInsert(). Because "Entry No." increases monotonically across all registers, a single posting register's entries are spread across all five buckets rather than concentrated in one, which weakens the intended purpose of bucketing (reducing SIFT-key write contention per concurrent posting batch) since concurrently-posting transactions from different registers will still collide across the same small set of buckets in an uncoordinated way. Deriving the bucket from "G/L Register No." instead would align each register's entries to one bucket and better distribute concurrent posting load. This should be applied consistently to every localized copy of GLEntry.Table.al touched in this PR (W1, APAC, BE, CH, ES, FI, FR, IT, NA, NO, RU, DACH).

Suggested change

	Rec."SIFT Bucket No." := Rec."Entry No." mod 5;
	Rec."SIFT Bucket No." := Rec."G/L Register No." mod 5;

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

[github-actions]

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

The new G/L Transaction grants added to posting-only permission sets are direct/uppercase (Ri/Rimd) even though concurrent posting reaches that table only through Codeunit 12 (Gen. Jnl.-Post Line), which already mediates access to it. In files such as W1 GeneralLedgerJournalsPost.PermissionSet.al (tabledata "G/L Transaction" = Ri added), and the equivalent PayablesDocumentsPost, PayablesJournalsPost, RecievablesDocumentsPost, RecievablesJournalsPost, d365purchdocpost, d365salesdocpost permission sets and their CH/NA/RU clones, this grants end users direct table access to a system-of-record table outside the mediated posting path. These grants should be indirect (ri/rimd) so access is only available through the codeunit that enforces posting rules.

Knowledge:

• microsoft/knowledge/security/indirect-permissions-for-elevated-access.md

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

[github-actions]

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

UnappliedErr and TotalAmountErr each contain multiple placeholders but no Comment property, so translators cannot tell what %1..%4 refer to at localization time. Add a placeholder Comment for each ordinal.

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

        UnappliedErr: Label '%1 %2 field must be true after Unapply entries.', Comment = '%1 = Detailed ledger entry table caption, %2 = Unapplied field value.', Locked = true;
        TotalAmountErr: Label 'Total Amount must be %1 in %2 table for %3 field : %4.', Comment = '%1 = Expected total amount, %2 = Detailed ledger entry table caption, %3 = Field caption, %4 = Field value.', Locked = true;

Knowledge:

• microsoft/knowledge/style/label-comment-explains-placeholders.md

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

[github-actions]

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

New/modified test methods call Commit() without declaring [TransactionModel(TransactionModel::AutoCommit)] on the containing test codeunit — for example src/Layers/W1/Tests/ERM/ERMConcurrentGenJnlPosting.Codeunit.al (Commit() at line 51, codeunit 134080 has no TransactionModel attribute), and the same pattern added to GenJnlLineTransferCurrencyIsNotSupported/GenJnlLineCustRefund in src/Apps/DK/FIK/test/src/TestPmtExportValidationDKUT.Codeunit.al. Without the attribute, the test's outcome depends on the test runner's default transaction-model behavior rather than being an explicit, reliable contract, and can behave inconsistently across isolation levels.

Knowledge:

• microsoft/knowledge/testing/transactionmodel-attribute-governs-test-transactions.md

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

[github-actions]

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

The new reverse-posting tests wire ConfirmHandler/MessageHandler functions, but the handlers never verify which dialog fired: ConfirmHandler unconditionally returns true and MessageHandler ignores the message text. The test suite can therefore stay green even if the reversal flow raises an unexpected prompt or the wrong message. Drive these handlers from queued expectations set up by each calling test, and assert the message-handler queue is empty at the end of each test.

Knowledge:

• microsoft/knowledge/testing/ui-handlers-in-tests.md

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

[github-actions]

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

CreateEmployeePostingGroup hand-rolls an Employee Posting Group with Init/Insert even though Library - Human Resource already provides CreateEmployeePostingGroup. Using the library fixture keeps the record aligned with required setup fields and avoids drift if the table's mandatory fields change, while still allowing the payables account to be overridden afterward for this test.

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

LibraryHumanResource.CreateEmployeePostingGroup(EmployeePostingGroup);
EmployeePostingGroup.Validate("Payables Account", GLAccountNo);
EmployeePostingGroup.Modify(true);

Knowledge:

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

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

[github-actions]

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

The PR adds "SIFT Bucket No." to the shipped G/L Entry table, includes it in Key2, and only assigns a value in OnInsert(). Existing G/L Entry rows keep the datatype default 0 after upgrade, so only newly inserted rows get "Entry No." mod 5 while historical data is never redistributed across the new bucketed key. No upgrade codeunit or DataTransfer-based backfill accompanies this schema change in the diff; one should be added so historical rows populate the new field consistently before code relies on the new key shape.

Knowledge:

• microsoft/knowledge/upgrade/datatransfer-for-bulk-init.md

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