Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,14 @@ codeunit 6233 "Sust. Preview Post Instance"
if SustLedgEntry.IsTemporary() then
exit;

if NextSustLedgerPreviewEntryNo = 0 then

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

NextSustLedgerPreviewEntryNo uses 0 as its "not initialized" sentinel, but a counter that starts at -2000000000 can legitimately reach 0. On the next preview insert, this code resets the counter back to -2000000000 and reuses an existing temporary primary key, so a sufficiently large preview fails with a generic duplicate-key/runtime error instead of a controlled exhaustion check. Use a non-reachable sentinel or add an explicit guard when the negative preview range is exhausted.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.31.4

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The counter only reaches 0 after about 2 billion previews in one session, so it is not reachable in practice.

NextSustLedgerPreviewEntryNo := -2000000000;

TempSustLedgEntry := SustLedgEntry;
TempSustLedgEntry."Entry No." := NextSustLedgerPreviewEntryNo;
TempSustLedgEntry."Document No." := '***';
TempSustLedgEntry.Insert();
NextSustLedgerPreviewEntryNo += 1;
HasSustainabilityEntry := true;
end;

Expand Down Expand Up @@ -87,11 +92,14 @@ codeunit 6233 "Sust. Preview Post Instance"

TempSustValueEntry.Reset();
TempSustValueEntry.DeleteAll();

NextSustLedgerPreviewEntryNo := -2000000000;
Comment thread
AleksanderGladkov marked this conversation as resolved.
end;

var
TempSustLedgEntry: Record "Sustainability Ledger Entry" temporary;
TempSustValueEntry: Record "Sustainability Value Entry" temporary;
NextSustLedgerPreviewEntryNo: Integer;
HasSustainabilityEntry: Boolean;
HasSustainabilityValueEntry: Boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ codeunit 6228 "Sust. Preview Posting Handler"
EventSubscriberInstance = Manual;
SingleInstance = true;

[EventSubscriber(ObjectType::Codeunit, Codeunit::"Sustainability Post Mgt", 'OnInsertLedgerEntryOnBeforeInsert', '', false, false)]
local procedure OnInsertLedgerEntryOnBeforeInsert(var SustainabilityLedgerEntry: Record "Sustainability Ledger Entry"; var IsHandled: Boolean)
var
SustPreviewPostInstance: Codeunit "Sust. Preview Post Instance";
begin
if IsHandled then
exit;
SustPreviewPostInstance.InsertSustLedgEntry(SustainabilityLedgerEntry, true);
IsHandled := true;
end;

[EventSubscriber(ObjectType::Table, Database::"Sustainability Ledger Entry", 'OnAfterInsertEvent', '', false, false)]
local procedure OnInsertSustLedgEntry(var Rec: Record "Sustainability Ledger Entry"; RunTrigger: Boolean)
var
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ codeunit 6212 "Sustainability Post Mgt"
var
SustainabilityLedgerEntry: Record "Sustainability Ledger Entry";
FeatureTelemetry: Codeunit "Feature Telemetry";
IsHandled: Boolean;

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\ —\ Telemetry}$

FeatureTelemetry.LogUsage('0000PH5', ...) and LogUptake(..., "Used") are still called unconditionally near the top of InsertLedgerEntry, before the new OnInsertLedgerEntryOnBeforeInsert/IsHandled branch decides whether a real ledger entry is ever persisted. With the new preview-diversion path, a call that ends up fully handled (no physical Insert) still reports feature usage as if a real Sustainability Ledger Entry was created, inflating usage telemetry for preview-only invocations of this procedure and making usage counts unreliable for measuring real adoption.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.31.4

SustainabilityLedgerEntryAddedLbl: Label 'Sustainability Ledger Entry Added', Locked = true;
begin
SustainabilityLedgerEntry.Init();
Expand All @@ -42,7 +43,11 @@ codeunit 6212 "Sustainability Post Mgt"
UpdateCarbonFeeEmission(SustainabilityLedgerEntry);

OnBeforeInsertSustainabilityLedgerEntry(SustainabilityLedgerEntry, SustainabilityJnlLine);
SustainabilityLedgerEntry.Insert(true);

IsHandled := false;
Comment thread
AleksanderGladkov marked this conversation as resolved.
OnInsertLedgerEntryOnBeforeInsert(SustainabilityLedgerEntry, IsHandled);
if not IsHandled then
SustainabilityLedgerEntry.Insert(true);
end;

procedure InsertValueEntry(SustainabilityJnlLine: Record "Sustainability Jnl. Line"; ValueEntry: Record "Value Entry"; ItemLedgerEntry: Record "Item Ledger Entry")
Expand Down Expand Up @@ -520,4 +525,9 @@ codeunit 6212 "Sustainability Post Mgt"
local procedure OnBeforeInsertSustainabilityLedgerEntry(var SustainabilityLedgerEntry: Record "Sustainability Ledger Entry"; SustainabilityJnlLine: Record "Sustainability Jnl. Line")
begin
end;

[InternalEvent(false, false)]
local procedure OnInsertLedgerEntryOnBeforeInsert(var SustainabilityLedgerEntry: Record "Sustainability Ledger Entry"; var IsHandled: Boolean)
begin
end;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
namespace Microsoft.Test.Sustainability;

using Microsoft.Bank.BankAccount;
using Microsoft.Finance.GeneralLedger.Account;
using Microsoft.Finance.GeneralLedger.Journal;
using Microsoft.Finance.GeneralLedger.Posting;
using Microsoft.Finance.GeneralLedger.Preview;
Expand Down Expand Up @@ -867,6 +868,185 @@ codeunit 148188 "Sust. General Journal Test"
Navigate.Run();
end;

[Test]
[HandlerFunctions('GLPostingPreviewSingleEntryHandler')]
procedure VerifyPreviewPostingOfGenJournalDoesNotConsumeSustainabilityLedgerEntryNo()
var
SustainabilityLedgerEntry: Record "Sustainability Ledger Entry";
SustainabilityAccount: Record "Sustainability Account";
GenJournalTemplate: Record "Gen. Journal Template";
GenJournalBatch: Record "Gen. Journal Batch";
BaselineGenJournalLine: Record "Gen. Journal Line";
GenJournalLine: Record "Gen. Journal Line";
BankAccount: Record "Bank Account";
GLAccount: Record "G/L Account";
Vendor: Record Vendor;
GenJnlPost: Codeunit "Gen. Jnl.-Post";
CategoryCode: Code[20];
SubcategoryCode: Code[20];
AccountCode: Code[20];
Comment thread
AleksanderGladkov marked this conversation as resolved.
BaselineEntryNo: Integer;
Index: Integer;
EmissionCO2: Decimal;
EmissionCH4: Decimal;
EmissionN2O: Decimal;
begin
// [SCENARIO 640599] Preview Posting of a General Journal Line must not consume the Sustainability Ledger Entry identity.
LibrarySustainability.CleanUpBeforeTesting();

// [GIVEN] Create a Sustainability Account.
CreateSustainabilityAccount(AccountCode, CategoryCode, SubcategoryCode, LibraryRandom.RandInt(10));
SustainabilityAccount.Get(AccountCode);

// [GIVEN] Generate Emission.
EmissionCO2 := LibraryRandom.RandInt(20);
EmissionCH4 := LibraryRandom.RandInt(5);
EmissionN2O := LibraryRandom.RandInt(5);

// [GIVEN] Create a Bank Account whose posting group has a G/L account so the line can post.
LibraryERM.CreateGLAccount(GLAccount);
LibraryERM.CreateBankAccount(BankAccount, GLAccount);

// [GIVEN] Create a Vendor.
LibraryPurchase.CreateVendor(Vendor);

// [GIVEN] Create a Gen Journal Template.
LibraryERM.CreateGenJournalTemplate(GenJournalTemplate);

// [GIVEN] Create a Gen Journal Batch.
LibraryERM.CreateGenJournalBatch(GenJournalBatch, GenJournalTemplate.Name);

// [GIVEN] Post a baseline General Journal Line to observe the committed Sustainability Ledger Entry identity.
CreateGenJournalLineWithEmission(
BaselineGenJournalLine, GenJournalBatch, Vendor."No.", BankAccount."No.", SustainabilityAccount."No.",
EmissionCO2, EmissionCH4, EmissionN2O);
LibraryERM.PostGeneralJnlLine(BaselineGenJournalLine);

// [GIVEN] Record the committed baseline Entry No.
SustainabilityLedgerEntry.SetRange("Document No.", BaselineGenJournalLine."Document No.");
SustainabilityLedgerEntry.FindLast();
BaselineEntryNo := SustainabilityLedgerEntry."Entry No.";

// [GIVEN] Prepare a single General Journal Line with Sustainability emissions.
CreateGenJournalLineWithEmission(
GenJournalLine, GenJournalBatch, Vendor."No.", BankAccount."No.", SustainabilityAccount."No.",
EmissionCO2, EmissionCH4, EmissionN2O);

// [GIVEN] Save a transaction.
Commit();

// [WHEN] Preview the General Journal Line three times.
GenJournalLine.SetRange("Journal Template Name", GenJournalBatch."Journal Template Name");
GenJournalLine.SetRange("Journal Batch Name", GenJournalBatch.Name);
for Index := 1 to 3 do begin
asserterror GenJnlPost.Preview(GenJournalLine);
Assert.ExpectedError('');
Comment thread
AleksanderGladkov marked this conversation as resolved.
end;

// [WHEN] Post the General Journal Line.
LibraryERM.PostGeneralJnlLine(GenJournalLine);

// [THEN] The committed Sustainability Ledger Entry equals the baseline plus one, proving the three previews consumed no identity.
SustainabilityLedgerEntry.Reset();
SustainabilityLedgerEntry.SetRange("Document No.", GenJournalLine."Document No.");
SustainabilityLedgerEntry.FindLast();
Assert.AreEqual(
BaselineEntryNo + 1,
SustainabilityLedgerEntry."Entry No.",
StrSubstNo(ValueMustBeEqualErr, SustainabilityLedgerEntry.FieldCaption("Entry No."), BaselineEntryNo + 1, SustainabilityLedgerEntry.TableCaption()));
end;

[Test]
[HandlerFunctions('GLPostingPreviewResetKeyDrillDownHandler')]
procedure VerifyRepeatedGenJournalPreviewResetsNegativeTemporaryKeys()
var
SustainabilityLedgerEntry: Record "Sustainability Ledger Entry";
SustainabilityAccount: Record "Sustainability Account";
GenJournalTemplate: Record "Gen. Journal Template";
GenJournalBatch: Record "Gen. Journal Batch";
GenJournalLine: array[2] of Record "Gen. Journal Line";
BankAccount: Record "Bank Account";
GLAccount: Record "G/L Account";
Vendor: Record Vendor;
GenJnlPost: Codeunit "Gen. Jnl.-Post";
CategoryCode: Code[20];
SubcategoryCode: Code[20];
AccountCode: Code[20];
Comment thread
AleksanderGladkov marked this conversation as resolved.
Index: Integer;
EmissionCO2: Decimal;
EmissionCH4: Decimal;
EmissionN2O: Decimal;
begin
// [SCENARIO 640599] Every repeated General Journal preview reuses the same reset pair of negative temporary Entry No. values.
LibrarySustainability.CleanUpBeforeTesting();

// [GIVEN] Create a Sustainability Account.
CreateSustainabilityAccount(AccountCode, CategoryCode, SubcategoryCode, LibraryRandom.RandInt(10));
SustainabilityAccount.Get(AccountCode);

// [GIVEN] Generate Emission.
EmissionCO2 := LibraryRandom.RandInt(20);
EmissionCH4 := LibraryRandom.RandInt(5);
EmissionN2O := LibraryRandom.RandInt(5);

// [GIVEN] Create a Bank Account whose posting group has a G/L account so the line can post.
LibraryERM.CreateGLAccount(GLAccount);
LibraryERM.CreateBankAccount(BankAccount, GLAccount);

// [GIVEN] Create a Vendor.
LibraryPurchase.CreateVendor(Vendor);

// [GIVEN] Create a Gen Journal Template.
LibraryERM.CreateGenJournalTemplate(GenJournalTemplate);

// [GIVEN] Create a Gen Journal Batch.
LibraryERM.CreateGenJournalBatch(GenJournalBatch, GenJournalTemplate.Name);

// [GIVEN] Prepare two General Journal Lines each producing a preview Sustainability Ledger Entry.
CreateGenJournalLineWithEmission(
GenJournalLine[1], GenJournalBatch, Vendor."No.", BankAccount."No.", SustainabilityAccount."No.",
EmissionCO2, EmissionCH4, EmissionN2O);
CreateGenJournalLineWithEmission(
GenJournalLine[2], GenJournalBatch, Vendor."No.", BankAccount."No.", SustainabilityAccount."No.",
EmissionCO2, EmissionCH4, EmissionN2O);

// [GIVEN] Save a transaction.
Commit();

// [WHEN] Preview the General Journal Lines multiple times.
GenJournalLine[1].SetRange("Journal Template Name", GenJournalBatch."Journal Template Name");
GenJournalLine[1].SetRange("Journal Batch Name", GenJournalBatch.Name);
for Index := 1 to 2 do begin
// [THEN] The drilldown handler asserts the same reset key pair (-1999999999 then -2000000000) on every preview run.
asserterror GenJnlPost.Preview(GenJournalLine[1]);
Assert.ExpectedError('');
Comment thread
AleksanderGladkov marked this conversation as resolved.
end;

// [THEN] No physical preview Sustainability Ledger Entry persists in the real table.
SustainabilityLedgerEntry.Reset();
Assert.RecordIsEmpty(SustainabilityLedgerEntry);
end;

local procedure CreateGenJournalLineWithEmission(var GenJournalLine: Record "Gen. Journal Line"; GenJournalBatch: Record "Gen. Journal Batch"; VendorNo: Code[20]; BankAccountNo: Code[20]; SustAccountNo: Code[20]; EmissionCO2: Decimal; EmissionCH4: Decimal; EmissionN2O: Decimal)
begin
LibraryERM.CreateGeneralJnlLine(
GenJournalLine,
GenJournalBatch."Journal Template Name",
GenJournalBatch.Name,
GenJournalLine."Document Type"::Invoice,
GenJournalLine."Account Type"::Vendor,
VendorNo,
-LibraryRandom.RandIntInRange(100, 200));

GenJournalLine.Validate("Bal. Account Type", GenJournalLine."Bal. Account Type"::"Bank Account");
GenJournalLine.Validate("Bal. Account No.", BankAccountNo);
GenJournalLine.Validate("Sust. Account No.", SustAccountNo);
GenJournalLine.Validate("Total Emission CH4", EmissionCH4);
GenJournalLine.Validate("Total Emission N2O", EmissionN2O);
GenJournalLine.Validate("Total Emission CO2", EmissionCO2);
GenJournalLine.Modify(true);
end;

local procedure CreateSustainabilityAccount(var AccountCode: Code[20]; var CategoryCode: Code[20]; var SubcategoryCode: Code[20]; i: Integer): Record "Sustainability Account"
begin
CreateSustainabilitySubcategory(CategoryCode, SubcategoryCode, i);
Expand Down Expand Up @@ -901,6 +1081,35 @@ codeunit 148188 "Sust. General Journal Test"
GLPostingPreview.OK().Invoke();
end;

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

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

// Drill down to the temporary preview Sustainability Ledger Entries page (descending Entry No. order).
SustainabilityLedgerEntries.Trap();
GLPostingPreview."No. of Records".DrillDown();

SustainabilityLedgerEntries.First();
SustainabilityLedgerEntries."Entry No.".AssertEquals(-1999999999);
SustainabilityLedgerEntries.Next();
SustainabilityLedgerEntries."Entry No.".AssertEquals(-2000000000);
SustainabilityLedgerEntries.Close();

GLPostingPreview.OK().Invoke();
end;

[PageHandler]
[Scope('OnPrem')]
procedure NavigateFindEntriesHandler(var Navigate: TestPage Navigate)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
namespace Microsoft.Test.Sustainability;

using Microsoft.Finance.GeneralLedger.Preview;
using Microsoft.Sustainability.Journal;
using Microsoft.Sustainability.Posting;

codeunit 148230 "Sust Preview Test Subscriber"
{
EventSubscriberInstance = Manual;
TableNo = "Sustainability Jnl. Line";

trigger OnRun()
var
SustainabilityPostMgt: Codeunit "Sustainability Post Mgt";
GenJnlPostPreview: Codeunit "Gen. Jnl.-Post Preview";
begin
SustainabilityPostMgt.InsertLedgerEntry(Rec);
GenJnlPostPreview.ThrowError();
end;

[EventSubscriber(ObjectType::Codeunit, Codeunit::"Gen. Jnl.-Post Preview", 'OnRunPreview', '', false, false)]
local procedure OnRunPreview(var Result: Boolean; Subscriber: Variant; RecVar: Variant)
var
SustainabilityJnlLine: Record "Sustainability Jnl. Line";
SustPreviewTestSubscriber: Codeunit "Sust Preview Test Subscriber";
begin
SustPreviewTestSubscriber := Subscriber;
SustainabilityJnlLine.Copy(RecVar);
Result := SustPreviewTestSubscriber.Run(SustainabilityJnlLine);
end;
}
Loading
Loading