feat(Sustainability): Add Reverse Transaction for Sustainability Ledger Entries

[KseniaOreshkina]

Add the ability to reverse posted Sustainability Ledger Entries, matching the established G/L reversal pattern — both manually from the page and automatically when a related G/L transaction is reversed.

Changes

• Table 6216: Add Reversed, Reversed by Entry No., Reversed Entry No. fields
• Page 6220: Add Reverse Transaction action (promoted, modern actionref syntax)
• New Codeunit 6230 (Sust. Entry Reverse Mgt.): Core reversal logic with validation
• New Codeunit 6231 (Sust. GL Reverse Subscriber): Subscribes to Gen. Jnl.-Post Reverse.OnReverseGLEntryOnAfterInsertGLEntry to auto-reverse sustainability entries when their originating G/L transaction is reversed
• Tests: Reversal unit tests plus an end-to-end G/L-reversal test

Behavior

• Manual: Users select entries on the Sustainability Ledger Entries page, click Reverse Transaction, confirm, and entries are reversed with negated emissions
• Automatic (G/L): When a G/L transaction that produced journal-posted sustainability entries is reversed, the matching sustainability entries (linked by Document No. + Posting Date) are reversed automatically. Idempotent via the already-reversed guard, so repeated event firings per document are safe
• Blocks already-reversed entries and document-posted entries
• All emission fields negated: CO2, CH4, N2O, CO2e, Carbon Fee, Water, Waste, Energy
• The reversal entry is posted on the original entry's posting date (so emissions net to zero within the same period) and stamped with the current User ID
• Entry numbers are assigned via the table's AutoIncrement (consistent with Sustainability Post Mgt and the G/L reversal engine)

Quality

• Reviewed against BCQuality knowledge base (all High/Medium findings fixed)
• Uses SetLoadFields, indirect permissions, modern actionref syntax
• Test codeunit uses the Sustainability test app's allocated id range (148180–148230)

fixes AB#640652

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Accessibility} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

This batch action passes the result of CurrPage.SetSelectionFilter directly to the reversal codeunit.

When the user has not explicitly multi-selected rows, or uses Ctrl+A, MarkedOnly is false and the record variable can collapse to the current row, so "Reverse Transaction" can silently reverse only one visible entry instead of the intended list scope. After SetSelectionFilter, check MarkedOnly and call SustLedgEntry.Copy(Rec) when it is false to preserve the full page view.

Suggested change

	CurrPage.SetSelectionFilter(SustLedgEntry);
	ReversedCount := SustEntryReverseMgt.ReverseEntries(SustLedgEntry);
	CurrPage.SetSelectionFilter(SustLedgEntry);
	if not SustLedgEntry.MarkedOnly then
	SustLedgEntry.Copy(Rec);
	ReversedCount := SustEntryReverseMgt.ReverseEntries(SustLedgEntry);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

The new codeunit "Sust.

Entry Reverse Mgt." is public by default, and its ReverseEntry, ReverseEntryFromGL, and ReverseEntries procedures are also externally reachable. In this app the codeunit is only consumed by the internal page action and tests, so this exposes implementation-detail reversal logic as a supported API other extensions could bind to. Make the codeunit Access = Internal; and keep these procedures internal unless you intentionally want to support them as a stable external contract.

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

codeunit 6230 "Sust. Entry Reverse Mgt."
{
    Access = Internal;
    Permissions = tabledata "Sustainability Ledger Entry" = rimd;

    internal procedure ReverseEntry(var SustLedgEntry: Record "Sustainability Ledger Entry")
    internal procedure ReverseEntryFromGL(var SustLedgEntry: Record "Sustainability Ledger Entry")
    internal procedure ReverseEntries(var SustLedgEntry: Record "Sustainability Ledger Entry"): Integer

Knowledge:

• microsoft/knowledge/breaking-changes/choose-access-modifiers-deliberately.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

GetNextEntryNo() reads the last "Sustainability Ledger Entry" number with FindLast() and CreateReversalEntry then inserts that explicit "Entry No." without locking the table or letting the AutoIncrement key assign the value.

Two concurrent reversals or postings can therefore compute the same next number and fail (or silently collide) on duplicate primary-key insertion; this also adds an avoidable extra SQL lookup per row when reversing several entries in one batch. Either keep the AutoIncrement pattern already used for this table (leave "Entry No." at 0, insert, then read the assigned key back) or call LockTable() before computing and consuming a manual next entry number.

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

ReverseEntries is written as a batch validation pass, but each ValidateEntryForReversal failure is still raised with a plain Error, so the loop stops on the first invalid entry.

Users reversing multiple ledger entries will have to fix one failure at a time and rerun until the selection is clean instead of seeing the full set of invalid entries in one pass. Use ErrorBehavior::Collect on the orchestration path, run each entry validation in an isolated context, then GetCollectedErrors(true) and raise one final blocking error.

Knowledge:

• microsoft/knowledge/error-handling/collect-validation-errors-with-errorbehavior.md

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

[github-actions]

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

The new reversal workflow in codeunit "Sust.

Entry Reverse Mgt." is a core ledger operation but it exposes no OnBefore/OnAfter integration events around validation, reversal-entry creation, or original-entry update. That makes the reversal process a hard wall for extensions: partners must copy or replace the codeunit to customize reversal behavior. Add thin, empty publishers at the natural reversal boundaries and let the calling procedures own the logic.

Knowledge:

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

"Sustainability Read" is an assignable read-only role for tabledata "Sustainability Ledger Entry", but this new public codeunit adds Permissions = tabledata "Sustainability Ledger Entry" = rimd and the page action invokes it without any authorization gate.

That lets users who can open the ledger page reverse posted entries through this code path even though their role does not grant modify/delete rights directly. Restrict reversal to a dedicated role or add an explicit permission check/AccessByPermission gate before calling the reversal codeunit.

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

The confirm handlers hardcode Yes/No replies without enqueueing and verifying the expected prompt text, and the tests never drain shared expectations.

That means these reversal tests can stay green even if the wrong confirm dialog fires or the interaction count changes, so they are not proving the UI contract they claim to cover.

Knowledge:

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

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

[github-actions]

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

The fixture helpers manufacture "Sustainability Ledger Entry" records by picking their own "Entry No.", populating fields manually, and calling Insert(false) directly instead of going through a library helper or extending the existing "Library - Sustainability" codeunit.

That bypasses validation and posting setup, making the tests brittle as the ledger schema and posting prerequisites evolve.

Knowledge:

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

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

[github-actions]

Copilot PR Review

Iteration 13 · 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
Breaking Changes	2	2	0	2	0
Data Modeling	1	1	0	1	0
Error Handling	1	1	0	1	0
Performance	1	1	0	0	0
Security	2	2	0	0	0
Testing	2	2	0	2	0

Totals: 9 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.}

[github-actions]

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

GetNextEntryNo's if/then/else both branches terminate with exit, so the else is structural noise the reader has to mentally flatten.

Per BCQuality style guidance, drop the else and let the second exit fall through naturally.

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

if SustLedgEntry.FindLast() then
            exit(SustLedgEntry."Entry No." + 1);
        exit(1);

Knowledge:

• microsoft/knowledge/style/no-else-after-terminating-statement.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes} \quad \color{gray}{\texttt{\small Iteration\ 4}}$

The new reversal management codeunit exposes ReverseEntry, ReverseEntryFromGL, and ReverseEntries as public API by omitting access modifiers, even though the callers in this diff are only this app and its friend test app.

Mark these routines internal (or local where possible) so they do not become a supported external contract you must preserve indefinitely.

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

    internal procedure ReverseEntry(var SustLedgEntry: Record "Sustainability Ledger Entry")
    var
        NewSustLedgEntry: Record "Sustainability Ledger Entry";
        NextEntryNo: Integer;
    begin
        ValidateEntryForReversal(SustLedgEntry);

        NextEntryNo := GetNextEntryNo();

        CreateReversalEntry(SustLedgEntry, NewSustLedgEntry, NextEntryNo);
        UpdateOriginalEntry(SustLedgEntry, NextEntryNo);
    end;

    internal procedure ReverseEntryFromGL(var SustLedgEntry: Record "Sustainability Ledger Entry")
    var
        NewSustLedgEntry: Record "Sustainability Ledger Entry";
        NextEntryNo: Integer;
    begin
        if SustLedgEntry.Reversed then
            exit;

        NextEntryNo := GetNextEntryNo();

        CreateReversalEntry(SustLedgEntry, NewSustLedgEntry, NextEntryNo);
        UpdateOriginalEntry(SustLedgEntry, NextEntryNo);
    end;

    internal procedure ReverseEntries(var SustLedgEntry: Record "Sustainability Ledger Entry"): Integer
    var
        TempSustLedgEntry: Record "Sustainability Ledger Entry";
        EntryCount: Integer;
    begin
        EntryCount := SustLedgEntry.Count();

        if EntryCount = 0 then
            exit(0);

        if EntryCount = 1 then begin
            if not Confirm(ConfirmReverseQst) then
                exit(0);
        end else
            if not Confirm(ConfirmReverseMultipleQst, false, EntryCount) then
                exit(0);

        // Validate all entries first (all-or-nothing)
        TempSustLedgEntry.Copy(SustLedgEntry);
        TempSustLedgEntry.SetLoadFields("Entry No.", Reversed, "Journal Template Name");
        if TempSustLedgEntry.FindSet() then
            repeat
                ValidateEntryForReversal(TempSustLedgEntry);
            until TempSustLedgEntry.Next() = 0;

        // Reverse all entries
        if SustLedgEntry.FindSet(true) then
            repeat
                ReverseEntry(SustLedgEntry);
            until SustLedgEntry.Next() = 0;

        exit(EntryCount);
    end;

Knowledge:

• microsoft/knowledge/breaking-changes/choose-access-modifiers-deliberately.md

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

[github-actions]

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

The diff deletes the GLPostingPreviewHandler procedure declaration (and its [Scope('OnPrem')] attribute) but leaves the [PageHandler] attribute followed directly by begin.

Verified against the diff: this breaks compilation because the handler's name, parameters, and Scope are gone while [HandlerFunctions('GLPostingPreviewHandler')] on the calling test still expects a real procedure of that name. This would otherwise be a blocker (the test codeunit will not compile) but is capped at minor severity per agent-finding rules since no knowledge file backs it; treat it as build-breaking in practice.

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

    [PageHandler]
    procedure GLPostingPreviewHandler(var GLPostingPreview: TestPage "G/L Posting Preview")
    begin
        GLPostingPreview.Filter.SetFilter("Table ID", Format(Database::"Sustainability Ledger Entry"));
        GLPostingPreview."No. of Records".AssertEquals(2);
        GLPostingPreview.OK().Invoke();
    end;

_{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{🟠\ High\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 6}}$

ReverseMultipleWithOneAlreadyReversedThrowsError calls ReverseEntries on two records, which always prompts for confirmation before validation.

Because the test has no matching [HandlerFunctions('ConfirmYesHandler')] declaration, it will stop on unhandled UI instead of ever asserting the expected 'already reversed' error.

Suggested change

	[Test]
	procedure ReverseMultipleWithOneAlreadyReversedThrowsError()
	[Test]
	[HandlerFunctions('ConfirmYesHandler')]
	procedure ReverseMultipleWithOneAlreadyReversedThrowsError()

Knowledge:

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

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

[github-actions]

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

The [PageHandler] block at the end of SustGeneralJournalTest no longer has a procedure declaration, so the test codeunit does not compile and any test that names GLPostingPreviewHandler in [HandlerFunctions(...)] is broken.

Restore the handler signature around the existing body.

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

    [PageHandler]
    procedure GLPostingPreviewHandler(var GLPostingPreview: TestPage "G/L Posting Preview")
    begin

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

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ UI} \quad \color{gray}{\texttt{\small Iteration\ 7}}$

The 'Reverse Transaction' action calls CurrPage.SetSelectionFilter(SustLedgEntry) and passes the resulting record straight into SustEntryReverseMgt.ReverseEntries without checking MarkedOnly.

Per the cited guidance, SetSelectionFilter only marks the full explicit selection and sets MarkedOnly := true when the user has actively multi-selected rows; in other cases (cursor on a single row, or a Ctrl+A select-all that the platform does not report as an explicit mark) it silently narrows the filter to one row. Because ReverseEntries is a genuine multi-record batch action (it has dedicated 'reverse N entries' confirmation and success messages), a user who selects all rows with Ctrl+A instead of manually marking each one can have the action silently reverse only a single entry with no error, appearing to have succeeded.

Suggested change

	CurrPage.SetSelectionFilter(SustLedgEntry);
	ReversedCount := SustEntryReverseMgt.ReverseEntries(SustLedgEntry);
	CurrPage.SetSelectionFilter(SustLedgEntry);
	if not SustLedgEntry.MarkedOnly() then
	SustLedgEntry.Copy(Rec);
	ReversedCount := SustEntryReverseMgt.ReverseEntries(SustLedgEntry);

Knowledge:

• microsoft/knowledge/ui/set-selection-filter-list-scope.md

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

[AndersLarsenMicrosoft]

duplicate ! already reported !!!!!!!

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ UI} \quad \color{gray}{\texttt{\small Iteration\ 7}}$

The new Integer fields "Reversed by Entry No." and "Reversed Entry No." (fields 5819/5820 in Sustainability Ledger Entry) omit BlankZero = true.

The equivalent fields on G/L Entry ("Reversed by Entry No."/"Reversed Entry No.", fields 74/75), which this feature explicitly mirrors per its own comments, both set BlankZero = true. Without it, every unreversed Sustainability Ledger Entry will display a literal '0' in these Editable = false columns instead of a blank cell, which is a visible, user-facing inconsistency with the pattern this PR is deliberately copying. Add BlankZero = true to both fields.

Suggested change

	field(5819; "Reversed by Entry No."; Integer)
	{
	Caption = 'Reversed by Entry No.';
	DataClassification = SystemMetadata;
	Editable = false;
	TableRelation = "Sustainability Ledger Entry"."Entry No.";
	}
	field(5820; "Reversed Entry No."; Integer)
	{
	Caption = 'Reversed Entry No.';
	DataClassification = SystemMetadata;
	Editable = false;
	TableRelation = "Sustainability Ledger Entry"."Entry No.";
	}
	field(5819; "Reversed by Entry No."; Integer)
	{
	BlankZero = true;
	Caption = 'Reversed by Entry No.';
	DataClassification = SystemMetadata;
	Editable = false;
	TableRelation = "Sustainability Ledger Entry"."Entry No.";
	}
	field(5820; "Reversed Entry No."; Integer)
	{
	BlankZero = true;
	Caption = 'Reversed Entry No.';
	DataClassification = SystemMetadata;
	Editable = false;
	TableRelation = "Sustainability Ledger Entry"."Entry No.";
	}

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

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 8}}$

This reversal lookup filters "Sustainability Ledger Entry" by "Document No.", "Posting Date", Reversed, and "Journal Template Name", but the table changes do not add a key whose leading fields cover that pattern.

On a ledger table, each G/L reversal will read more rows than necessary; add a supporting key for this access path and select it with SetCurrentKey().

Knowledge:

• microsoft/knowledge/performance/setcurrentkey-aligns-key-with-filters.md

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Performance} \quad \color{gray}{\texttt{\small Iteration\ 8}}$

The subscriber first collects matching entry numbers, then does Get(...) for each one inside the foreach loop.

That turns the reversal into an N+1 access pattern against "Sustainability Ledger Entry"; iterate the filtered set directly, or otherwise avoid a per-entry Get before reversing.

Knowledge:

• microsoft/knowledge/performance/avoid-get-inside-loop-on-large-table.md

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 8}}$

"Sustainability Edit" now grants direct M access on "Sustainability Ledger Entry".

This reversal flow is already code-mediated by "Sust. Entry Reverse Mgt."; uppercase M lets assignees modify ledger entries through other UI or API surfaces and bypass that controlled path. Keep modify access indirect (m) or rely on the codeunit's own permissions instead.

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

        tabledata "Sustainability Ledger Entry" = Im,

Knowledge:

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

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

[github-actions]

$\textbf{🔴\ Critical\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 8}}$

The confirmation-path tests use handlers that hardcode Reply := true/false and never verify which confirm text was raised, and the tests do not use LibraryVariableStorage.Clear()/AssertEmpty() to prove the interaction happened exactly once.

That means these tests can stay green even if ReverseEntries raises the wrong confirm or leaks dialog expectations between runs.

Knowledge:

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 9}}$

The new Reverse Transaction action is exposed on a page that Sustainability Read users can execute, and its OnAction calls a codeunit with tabledata "Sustainability Ledger Entry" = rimd without any AccessByPermission or explicit authorization check.

In practice this lets read-only users invoke ledger reversals; gate the action or the privileged procedure on an edit-only permission before calling the reversal code. Because this is an agent finding, it is emitted as minor even though the underlying impact is authorization-sensitive.

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

[AndersLarsenMicrosoft]

not relevant as it will only take a transaction like on other entry pages

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Telemetry} \quad \color{gray}{\texttt{\small Iteration\ 9}}$

The new sustainability reversal flow commits a new ledger entry and marks the original entry as reversed in both ReverseEntry and ReverseEntryFromGL, but it never emits any FeatureTelemetry/Session.LogMessage signal.

In this app, successful sustainability posting paths already log usage and Used uptake when they create ledger/value entries, so leaving reversal uninstrumented makes this new financially significant workflow invisible in telemetry. Add a dedicated success telemetry event after the reversal completes, using a stable event ID and the same feature name/logger path the rest of the Sustainability app uses.

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 10}}$

Permissions = tabledata "Sustainability Ledger Entry" = rimd; grants Insert, Modify, and Delete on a codeunit that only reads the table (SetRange/FindSet to collect entry numbers) and delegates the actual write to Sust.

Entry Reverse Mgt., which already declares its own write permissions. The subscriber itself never inserts, modifies, or deletes this table, so the grant should be scoped to r.

Suggested change

	Permissions = tabledata "Sustainability Ledger Entry" = rimd;
	Permissions = tabledata "Sustainability Ledger Entry" = r;

Knowledge:

• microsoft/knowledge/security/inherent-permissions-minimal-grant.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 11}}$

ReverseEntry and ReverseEntries both route through ValidateEntryForReversal, which blocks reversing an entry whose "Journal Template Name" is blank (document-posted, per DocumentEntryErr: 'was posted from a document and cannot be reversed from here').

ReverseEntryFromGL is a third public entry point into the same reversal logic (CreateReversalEntry/UpdateOriginalEntry) but only checks SustLedgEntry.Reversed - it never checks "Journal Template Name". Today the only caller (Sust. GL Reverse Subscriber) happens to pre-filter with SetFilter("Journal Template Name", '<>%1', ''), but that guard lives in the caller, not in the shared procedure. Any other code that calls the public ReverseEntryFromGL directly with a document-posted entry would silently create an incorrect reversal that the two sibling entry points explicitly forbid. Consider moving the document-template check into ReverseEntryFromGL (or a shared validation step) so the business rule is enforced once, in the procedure that owns it, rather than relying on every caller to re-derive the same filter. If this were treated as high-impact, this would warrant a knowledge-backed rule rather than an agent-severity flag; it is kept at minor per the agent-finding severity cap.

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling} \quad \color{gray}{\texttt{\small Iteration\ 11}}$

ReverseEntries validates every selected entry in a loop but calls ValidateEntryForReversal, which raises a plain Error on the first invalid entry.

When multiple selected entries fail validation (for example several are already reversed or document-posted), the user only ever learns about the first failure, has to deselect that one entry, and re-run the action to discover the next failure. Marking the orchestrating validation loop with [ErrorBehavior(ErrorBehavior::Collect)] and reporting all collected failures at once would let the user fix every problem in one pass instead of one-at-a-time.

Knowledge:

• microsoft/knowledge/error-handling/collect-validation-errors-with-errorbehavior.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Style} \quad \color{gray}{\texttt{\small Iteration\ 12}}$

In 'Sustainability Edit' permission set, the 'Sustainability Ledger Entry' grant is written as lowercase 'im' while every other tabledata entry in the same Permissions block uses uppercase letters ('IMD', 'I').

AL permission tokens are case-insensitive so this compiles and behaves identically, but the inconsistent casing stands out against the file's own convention and against sibling permission sets in the app (SustainabilityAdmin.permissionset.al, SustainabilityRead.permissionset.al all use uppercase). Use 'IM' to match the surrounding style.

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

        tabledata "Sustainability Ledger Entry" = IM,

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes} \quad \color{gray}{\texttt{\small Iteration\ 13}}$

ReverseEntry is used only as an implementation helper inside Sust.

Entry Reverse Mgt. in this change, but it is declared without an access modifier, which publishes it as part of the app's supported surface. Make it local so the reversal implementation can evolve without creating an accidental external contract.

Suggested change

	CreateReversalEntry(SustLedgEntry, NewSustLedgEntry);
	local procedure ReverseEntry(var SustLedgEntry: Record "Sustainability Ledger Entry")

Knowledge:

• microsoft/knowledge/breaking-changes/choose-access-modifiers-deliberately.md

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes} \quad \color{gray}{\texttt{\small Iteration\ 13}}$

This changes the existing shipped permission set Sustainability Edit from direct insert (I) to indirect insert/modify (im) on Sustainability Ledger Entry, so consumers already assigned that permission set no longer receive the same rights.

Keep the shipped permission set unchanged and introduce a new or supplemental permission set for the reversal-specific permission change instead.

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling} \quad \color{gray}{\texttt{\small Iteration\ 13}}$

The G/L reversal subscriber identifies sustainability entries only by "Document No.", "Posting Date", and a nonblank "Journal Template Name".

That is not a transaction-unique relation, so reversing one G/L entry also reverses every sustainability entry that shares those values (also flagged from the events-subscriber angle by al-events-review, since the subscriber inherits the same weak key). Store a direct source G/L transaction or entry reference on "Sustainability Ledger Entry" and drive the reversal lookup from that keyed relation instead.

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Error\ Handling} \quad \color{gray}{\texttt{\small Iteration\ 13}}$

ReverseEntries validates the selected ledger entries by calling ValidateEntryForReversal inside a batch loop, but each validation failure still raises a plain Error.

That means the first invalid entry aborts the validation pass and the user must fix issues one at a time instead of seeing every bad entry in the selection. For this batch validation path, use ErrorBehavior::Collect, then retrieve and clear the collected errors and fail once with the aggregated result.

Knowledge:

• microsoft/knowledge/error-handling/collect-validation-errors-with-errorbehavior.md

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

[github-actions]

$\textbf{🔴\ Critical\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 13}}$

The confirmation tests use hardcoded [ConfirmHandler] implementations that just set Reply := true/false and never verify the expected confirm text or interaction count, so any confirm dialog can satisfy the scenario and prompt regressions go undetected.

Drive the handlers from Library - Variable Storage, assert the expected prompt with Assert.ExpectedConfirm, clear the queue in Initialize, and finish each handler-based test with AssertEmpty.

Knowledge:

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

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

[github-actions]

$\textbf{🟠\ High\ Severity\ —\ Testing} \quad \color{gray}{\texttt{\small Iteration\ 13}}$

The fixture helpers hand-roll Sustainability Ledger Entry records with FindLast, a manually assigned Entry No., direct field assignment, and Insert(false), which bypasses the normal posting and validation path and can keep the reversal tests green on ledger states the application never produces.

Build the prerequisite entries through existing posting or library helpers so numbering, relations, and validation run as they do in production.

Knowledge:

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

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