-
Notifications
You must be signed in to change notification settings - Fork 436
Bug 642053: [master] [Sustainability] Preview Posting Creates Gaps in Sustainability Ledger Entry Numbers #10051
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
7dce5f7
e185970
e92b217
8977d31
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR adds a new [InternalEvent] OnInsertLedgerEntryOnBeforeInsert in codeunit 6212 "Sustainability Post Mgt", raised immediately before SustainabilityLedgerEntry.Insert(true) and guarded by IsHandled. Codeunit 6228 "Sust. Preview Posting Handler" subscribes to it and, when bound during preview, sets IsHandled := true, which now prevents the real Insert(true) call from ever running during preview. However, that same codeunit 6228 still contains the pre-existing subscriber OnInsertSustLedgEntry on Database::"Sustainability Ledger Entry" 'OnAfterInsertEvent' (the previous, buggy mechanism for capturing preview entries after a real insert). Since the real Insert(true) can no longer occur while codeunit 6228 is bound (the only scenario the OnAfterInsertEvent subscriber is active in), that OnAfterInsertEvent subscriber is now unreachable dead code for the ledger-entry preview path it was written for. Recommend removing (or repurposing) OnInsertSustLedgEntry to avoid confusing future maintainers into thinking it still participates in the preview flow. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
||
| [EventSubscriber(ObjectType::Table, Database::"Sustainability Ledger Entry", 'OnAfterInsertEvent', '', false, false)] | ||
| local procedure OnInsertSustLedgEntry(var Rec: Record "Sustainability Ledger Entry"; RunTrigger: Boolean) | ||
| var | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ codeunit 6212 "Sustainability Post Mgt" | |
| var | ||
| SustainabilityLedgerEntry: Record "Sustainability Ledger Entry"; | ||
| FeatureTelemetry: Codeunit "Feature Telemetry"; | ||
| IsHandled: Boolean; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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(); | ||
|
|
@@ -42,7 +43,11 @@ codeunit 6212 "Sustainability Post Mgt" | |
| UpdateCarbonFeeEmission(SustainabilityLedgerEntry); | ||
|
|
||
| OnBeforeInsertSustainabilityLedgerEntry(SustainabilityLedgerEntry, SustainabilityJnlLine); | ||
| SustainabilityLedgerEntry.Insert(true); | ||
|
|
||
| IsHandled := false; | ||
|
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") | ||
|
|
@@ -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; | ||
|
|
@@ -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]; | ||
|
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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [AI test]? |
||
| 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(''); | ||
|
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]; | ||
|
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(''); | ||
|
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); | ||
|
|
@@ -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) | ||
|
|
||
| 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; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NextSustLedgerPreviewEntryNouses0as its "not initialized" sentinel, but a counter that starts at-2000000000can legitimately reach0. On the next preview insert, this code resets the counter back to-2000000000and 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
There was a problem hiding this comment.
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.