-
Notifications
You must be signed in to change notification settings - Fork 432
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 2 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 |
|---|---|---|
| @@ -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.