Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KseniaOreshkina wants to merge 13 commits into
base: main
Choose a base branch
from
+704 −14
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5fe173b
feat(Sustainability): Add Reverse Transaction for Sustainability Ledg…
KseniaOreshkina Jul 15, 2026
521e5b0
fix(Sustainability): move test codeunit into allocated 148180-148230 …
KseniaOreshkina Jul 15, 2026
d64cda4
fix(Sustainability): reverse on original posting date instead of Work…
KseniaOreshkina Jul 15, 2026
6de072b
feat(Sustainability): auto-reverse ledger entries on G/L transaction …
KseniaOreshkina Jul 15, 2026
d935eb5
refactor(Sustainability): use AutoIncrement entry no. and stamp User …
KseniaOreshkina Jul 15, 2026
4ba7e4d
fix(Sustainability): resolve clashing object IDs in reversal feature
KseniaOreshkina Jul 15, 2026
0300160
fix(Sustainability): restore clobbered GLPostingPreviewHandler signature
KseniaOreshkina Jul 15, 2026
00e64c9
fix(Sustainability): grant Modify on ledger entry and align reversal …
KseniaOreshkina Jul 16, 2026
4a42419
fix(Sustainability): grant indirect (not direct) Modify on ledger ent…
KseniaOreshkina Jul 16, 2026
39cb7b3
fix(Sustainability): make Insert on ledger entry indirect too
KseniaOreshkina Jul 16, 2026
1b4b8c0
test(Sustainability): handle confirm dialog in ReverseMultipleWithOne…
KseniaOreshkina Jul 16, 2026
364a843
test(Sustainability): make G/L reversal e2e test localization-neutral
KseniaOreshkina Jul 16, 2026
12a21c7
fix(MX DIOT): always initialize default company data
Copilot Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions src/Apps/NA/MX_DIOT/app/src/Install/DIOTInitialize.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ codeunit 27020 "DIOT - Initialize"

trigger OnInstallAppPerCompany()
begin
if InitializeDone() then
exit;

InitializeCompany();
end;

Expand All @@ -31,14 +28,6 @@ codeunit 27020 "DIOT - Initialize"
InsertDefaultDIOTCountryData();
end;

local procedure InitializeDone(): boolean
var
AppInfo: ModuleInfo;
begin
NavApp.GetCurrentModuleInfo(AppInfo);
exit(AppInfo.DataVersion() <> Version.Create('0.0.0.0'));
end;

local procedure ApplyEvaluationClassificationsForPrivacy()
var
Company: Record Company;
Expand Down
115 changes: 115 additions & 0 deletions src/Apps/W1/Sustainability/app/src/Ledger/SustEntryReverseMgt.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
namespace Microsoft.Sustainability.Ledger;

codeunit 6243 "Sust. Entry Reverse Mgt."
{
Permissions = tabledata "Sustainability Ledger Entry" = rimd;

var
AlreadyReversedErr: Label 'Entry No. %1 has already been reversed.', Comment = '%1 = Entry No.';
DocumentEntryErr: Label 'Entry No. %1 was posted from a document and cannot be reversed from here. Use a corrective document instead.', Comment = '%1 = Entry No.';
ConfirmReverseQst: Label 'Do you want to reverse the selected sustainability ledger entry?';
ConfirmReverseMultipleQst: Label 'Do you want to reverse %1 sustainability ledger entries?', Comment = '%1 = Count';

procedure ReverseEntry(var SustLedgEntry: Record "Sustainability Ledger Entry")

@github-actions github-actions Bot Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

@github-actions github-actions Bot Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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

var
NewSustLedgEntry: Record "Sustainability Ledger Entry";
begin
ValidateEntryForReversal(SustLedgEntry);

CreateReversalEntry(SustLedgEntry, NewSustLedgEntry);

@github-actions github-actions Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

UpdateOriginalEntry(SustLedgEntry, NewSustLedgEntry."Entry No.");
end;

procedure ReverseEntryFromGL(var SustLedgEntry: Record "Sustainability Ledger Entry")

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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

var
NewSustLedgEntry: Record "Sustainability Ledger Entry";
begin
if SustLedgEntry.Reversed then
exit;

CreateReversalEntry(SustLedgEntry, NewSustLedgEntry);
UpdateOriginalEntry(SustLedgEntry, NewSustLedgEntry."Entry No.");
end;

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

@github-actions github-actions Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

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)

@github-actions github-actions Bot Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

TempSustLedgEntry.Copy(SustLedgEntry);
TempSustLedgEntry.SetLoadFields("Entry No.", Reversed, "Journal Template Name");
if TempSustLedgEntry.FindSet() then

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

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;

local procedure ValidateEntryForReversal(SustLedgEntry: Record "Sustainability Ledger Entry")
begin
if SustLedgEntry.Reversed then
Error(AlreadyReversedErr, SustLedgEntry."Entry No.");

if SustLedgEntry."Journal Template Name" = '' then
Error(DocumentEntryErr, SustLedgEntry."Entry No.");
end;

local procedure CreateReversalEntry(OriginalEntry: Record "Sustainability Ledger Entry"; var NewEntry: Record "Sustainability Ledger Entry")
begin
NewEntry.Init();
NewEntry.TransferFields(OriginalEntry, false);
// AutoIncrement assigns the Entry No. on Insert (matches Sustainability Post Mgt and G/L reversal engine-assigned numbering).
NewEntry."Entry No." := 0;
// Post the reversal on the original entry's posting date so emissions net to zero within the same period (matches G/L Reverse).
NewEntry."Posting Date" := OriginalEntry."Posting Date";
NewEntry."Document No." := OriginalEntry."Document No.";
NewEntry.Validate("User ID", CopyStr(UserId(), 1, MaxStrLen(NewEntry."User ID")));

// Negate emission values
NewEntry."Emission CO2" := -OriginalEntry."Emission CO2";
NewEntry."Emission CH4" := -OriginalEntry."Emission CH4";
NewEntry."Emission N2O" := -OriginalEntry."Emission N2O";
NewEntry."CO2e Emission" := -OriginalEntry."CO2e Emission";
NewEntry."Carbon Fee" := -OriginalEntry."Carbon Fee";

// Negate water & waste values
NewEntry."Water Intensity" := -OriginalEntry."Water Intensity";
NewEntry."Discharged Into Water" := -OriginalEntry."Discharged Into Water";
NewEntry."Waste Intensity" := -OriginalEntry."Waste Intensity";
NewEntry."Energy Consumption" := -OriginalEntry."Energy Consumption";

// Set reversal tracking fields
NewEntry.Reversed := true;
NewEntry."Reversed Entry No." := OriginalEntry."Entry No.";
NewEntry."Reversed by Entry No." := 0;

NewEntry.Insert(true);
end;

local procedure UpdateOriginalEntry(var OriginalEntry: Record "Sustainability Ledger Entry"; ReversalEntryNo: Integer)
begin
OriginalEntry.Reversed := true;
OriginalEntry."Reversed by Entry No." := ReversalEntryNo;
OriginalEntry.Modify(true);
end;
}
39 changes: 39 additions & 0 deletions src/Apps/W1/Sustainability/app/src/Ledger/SustGLReverseSubscriber.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
namespace Microsoft.Sustainability.Ledger;

using Microsoft.Finance.GeneralLedger.Journal;
using Microsoft.Finance.GeneralLedger.Ledger;
using Microsoft.Finance.GeneralLedger.Posting;
using Microsoft.Finance.GeneralLedger.Reversal;

codeunit 6244 "Sust. GL Reverse Subscriber"
{
Permissions = tabledata "Sustainability Ledger Entry" = rimd;

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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


[EventSubscriber(ObjectType::Codeunit, Codeunit::"Gen. Jnl.-Post Reverse", 'OnReverseGLEntryOnAfterInsertGLEntry', '', false, false)]
local procedure ReverseSustLedgerEntriesOnReverseGLEntry(var GLEntry: Record "G/L Entry"; GenJnlLine: Record "Gen. Journal Line"; GLEntry2: Record "G/L Entry"; var GenJnlPostLine: Codeunit "Gen. Jnl.-Post Line")
var
SustLedgEntry: Record "Sustainability Ledger Entry";
SustLedgEntryToReverse: Record "Sustainability Ledger Entry";
SustEntryReverseMgt: Codeunit "Sust. Entry Reverse Mgt.";
EntryNos: List of [Integer];
EntryNo: Integer;
begin
// GLEntry2 is the original G/L entry being reversed. Sustainability Ledger Entries are linked to it
// through the shared Document No. and Posting Date (see Sust. Gen. Journal Subscriber posting logic).
// The event fires once per reversed G/L entry, so a document may be processed more than once;
// ReverseEntryFromGL skips already-reversed entries, making this idempotent.
SustLedgEntry.SetLoadFields("Entry No.");
SustLedgEntry.SetRange("Document No.", GLEntry2."Document No.");

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

@github-actions github-actions Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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

SustLedgEntry.SetRange("Posting Date", GLEntry2."Posting Date");
SustLedgEntry.SetRange(Reversed, false);
SustLedgEntry.SetFilter("Journal Template Name", '<>%1', '');
if SustLedgEntry.FindSet() then
repeat
EntryNos.Add(SustLedgEntry."Entry No.");
until SustLedgEntry.Next() = 0;

foreach EntryNo in EntryNos do

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

if SustLedgEntryToReverse.Get(EntryNo) then
SustEntryReverseMgt.ReverseEntryFromGL(SustLedgEntryToReverse);
end;
}
48 changes: 47 additions & 1 deletion src/Apps/W1/Sustainability/app/src/Ledger/SustainabilityLedgerEntries.Page.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,18 @@ page 6220 "Sustainability Ledger Entries"
{
ToolTip = 'Specifies the Energy Consumption.';
}
field(Reversed; Rec.Reversed)
{
ToolTip = 'Specifies whether this entry has been reversed.';
}
field("Reversed by Entry No."; Rec."Reversed by Entry No.")
{
ToolTip = 'Specifies the entry number that reversed this entry.';
}
field("Reversed Entry No."; Rec."Reversed Entry No.")
{
ToolTip = 'Specifies the original entry number that this entry reverses.';
}
field("Country/Region Code"; Rec."Country/Region Code")
{
ToolTip = 'Specifies the country/region code of the entry.';
Expand Down Expand Up @@ -260,6 +272,33 @@ page 6220 "Sustainability Ledger Entries"
}
}
}
area(processing)
{
action(ReverseTransaction)

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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 AndersLarsenMicrosoft Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

{
ApplicationArea = Basic, Suite;
Caption = 'Reverse Transaction';
ToolTip = 'Reverse the selected sustainability ledger entry by creating a new entry with negated emission values.';
Image = ReverseRegister;

trigger OnAction()
var
SustLedgEntry: Record "Sustainability Ledger Entry";
SustEntryReverseMgt: Codeunit "Sust. Entry Reverse Mgt.";
ReversedCount: Integer;
begin
CurrPage.SetSelectionFilter(SustLedgEntry);
ReversedCount := SustEntryReverseMgt.ReverseEntries(SustLedgEntry);
Comment on lines +290 to +291

@github-actions github-actions Bot Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

Comment on lines +290 to +291

@github-actions github-actions Bot Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

@AndersLarsenMicrosoft AndersLarsenMicrosoft Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate ! already reported !!!!!!!

if ReversedCount > 0 then begin
CurrPage.Update(false);
if ReversedCount = 1 then
Message(ReversalSuccessMsg)
else
Message(ReversalMultipleSuccessMsg, ReversedCount);
end;
end;
}
}
area(Promoted)
{
group(Category_Category4)
Expand All @@ -268,13 +307,20 @@ page 6220 "Sustainability Ledger Entries"
actionref(Dimensions_Promoted; Dimensions) { }
actionref(SetDimensionFilter_Promoted; SetDimensionFilter) { }
}
group(Category_Process)
{
Caption = 'Process';
actionref(ReverseTransaction_Promoted; ReverseTransaction) { }
}
}
}

var
DimensionSetIDFilter: Page "Dimension Set ID Filter";
Dim1Visible, Dim2Visible, Dim3Visible, Dim4Visible, Dim5Visible, Dim6Visible, Dim7Visible, Dim8Visible : Boolean;
DimensionCaptionLbl: Label '%1 %2', Locked = true;
ReversalSuccessMsg: Label 'The entry has been successfully reversed.';
ReversalMultipleSuccessMsg: Label '%1 entries have been successfully reversed.', Comment = '%1 = Count';

trigger OnOpenPage()
begin
Expand All @@ -287,4 +333,4 @@ page 6220 "Sustainability Ledger Entries"
begin
DimensionManagement.UseShortcutDims(Dim1Visible, Dim2Visible, Dim3Visible, Dim4Visible, Dim5Visible, Dim6Visible, Dim7Visible, Dim8Visible);
end;
}
}
22 changes: 21 additions & 1 deletion src/Apps/W1/Sustainability/app/src/Ledger/SustainabilityLedgerEntry.Table.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,26 @@ table 6216 "Sustainability Ledger Entry"
{
Caption = 'Correction';
}
field(5818; Reversed; Boolean)
{
Caption = 'Reversed';
DataClassification = SystemMetadata;
Editable = false;
}
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.";
}
Comment on lines +349 to +362

@github-actions github-actions Bot Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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

}

keys
Expand Down Expand Up @@ -370,4 +390,4 @@ table 6216 "Sustainability Ledger Entry"
begin
DimMgt.ShowDimensionSet("Dimension Set ID", StrSubstNo(EntryRecIDLbl, TableCaption(), "Entry No."));
end;
}
}
2 changes: 1 addition & 1 deletion src/Apps/W1/Sustainability/app/src/Permissions/SustainabilityEdit.permissionset.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ permissionset 6213 "Sustainability Edit"
tabledata "Sustainability Jnl. Template" = IMD,
tabledata "Sustainability Jnl. Batch" = IMD,
tabledata "Sustainability Jnl. Line" = IMD,
tabledata "Sustainability Ledger Entry" = I,

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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:

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

@github-actions github-actions Bot Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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 github-actions Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\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

tabledata "Sustainability Ledger Entry" = im,
tabledata "Sustainability Value Entry" = I;
}
74 changes: 74 additions & 0 deletions src/Apps/W1/Sustainability/test/src/SustGeneralJournalTest.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
namespace Microsoft.Test.Sustainability;

using Microsoft.Bank.BankAccount;
using Microsoft.Finance.GeneralLedger.Account;
using Microsoft.Finance.GeneralLedger.Journal;
using Microsoft.Finance.GeneralLedger.Ledger;
using Microsoft.Finance.GeneralLedger.Posting;
using Microsoft.Finance.GeneralLedger.Preview;
using Microsoft.Finance.GeneralLedger.Reversal;
using Microsoft.Foundation.Navigate;
using Microsoft.Purchases.Vendor;
using Microsoft.Sustainability.Account;
Expand Down Expand Up @@ -892,6 +895,77 @@ codeunit 148188 "Sust. General Journal Test"
true, true, true, '', false);
end;

[Test]
procedure VerifySustLedgerEntryIsReversedWhenGLTransactionIsReversed()
var
SustLedgerEntry: Record "Sustainability Ledger Entry";
ReversalSustLedgerEntry: Record "Sustainability Ledger Entry";
SustainabilityAccount: Record "Sustainability Account";
GenJournalTemplate: Record "Gen. Journal Template";
GenJournalBatch: Record "Gen. Journal Batch";
GenJournalLine: Record "Gen. Journal Line";
GLEntry: Record "G/L Entry";
ReversalEntry: Record "Reversal Entry";
GLAccount: Record "G/L Account";
BalGLAccount: Record "G/L Account";
EmissionCO2: Decimal;
OriginalEntryNo: Integer;
DocumentNo: Code[20];
CategoryCode: Code[20];
SubcategoryCode: Code[20];
AccountCode: Code[20];
begin
// [SCENARIO] When a G/L transaction that produced a Sustainability Ledger Entry is reversed, the sustainability entry is reversed automatically.
LibrarySustainability.CleanUpBeforeTesting();

// [GIVEN] A Sustainability Account and a General Journal Line posted with emissions.
CreateSustainabilityAccount(AccountCode, CategoryCode, SubcategoryCode, LibraryRandom.RandInt(10));
SustainabilityAccount.Get(AccountCode);
EmissionCO2 := LibraryRandom.RandIntInRange(10, 20);

// Post a plain G/L-to-G/L journal line with a blank document type so the reversal is
// localization-neutral (invoice documents cannot be reversed in some localizations, and
// bank/cash posting groups are not configured with a G/L account in every demo database).
LibraryERM.CreateGLAccount(GLAccount);
LibraryERM.CreateGLAccount(BalGLAccount);
LibraryERM.CreateGenJournalTemplate(GenJournalTemplate);
LibraryERM.CreateGenJournalBatch(GenJournalBatch, GenJournalTemplate.Name);
LibraryERM.CreateGeneralJnlLine(
GenJournalLine, GenJournalBatch."Journal Template Name", GenJournalBatch.Name,
GenJournalLine."Document Type"::" ", GenJournalLine."Account Type"::"G/L Account", GLAccount."No.",
LibraryRandom.RandIntInRange(100, 200));
GenJournalLine.Validate("Bal. Account Type", GenJournalLine."Bal. Account Type"::"G/L Account");
GenJournalLine.Validate("Bal. Account No.", BalGLAccount."No.");
GenJournalLine.Validate("Sust. Account No.", SustainabilityAccount."No.");
GenJournalLine.Validate("Total Emission CO2", EmissionCO2);
GenJournalLine.Modify(true);
DocumentNo := GenJournalLine."Document No.";
LibraryERM.PostGeneralJnlLine(GenJournalLine);

SustLedgerEntry.SetRange("Document No.", DocumentNo);
SustLedgerEntry.FindFirst();
OriginalEntryNo := SustLedgerEntry."Entry No.";

// [WHEN] The G/L transaction is reversed.
GLEntry.SetRange("Document No.", DocumentNo);
GLEntry.FindFirst();
ReversalEntry.SetHideDialog(true);
ReversalEntry.SetHideWarningDialogs();
ReversalEntry.ReverseTransaction(GLEntry."Transaction No.");

// [THEN] The original sustainability entry is marked as reversed.
SustLedgerEntry.Get(OriginalEntryNo);
Assert.IsTrue(SustLedgerEntry."Reversed", 'Original sustainability entry should be reversed after G/L reversal.');
Assert.AreNotEqual(0, SustLedgerEntry."Reversed by Entry No.", 'Reversed by Entry No. should be populated.');

// [THEN] A reversal sustainability entry exists with negated emission.
ReversalSustLedgerEntry.Get(SustLedgerEntry."Reversed by Entry No.");
Assert.AreEqual(-SustLedgerEntry."Emission CO2", ReversalSustLedgerEntry."Emission CO2",
'Reversal sustainability entry should have negated CO2.');
Assert.AreEqual(OriginalEntryNo, ReversalSustLedgerEntry."Reversed Entry No.",
'Reversal sustainability entry should reference the original entry.');
end;

[PageHandler]
[Scope('OnPrem')]
procedure GLPostingPreviewHandler(var GLPostingPreview: TestPage "G/L Posting Preview")
Expand Down
Loading