Bug 646432: [master] [Sustainability] Line does not indicate/enforce which Calculation Foundation is in use; all formula input fields are editable - #10183
Conversation
- centralize journal and purchase editability rules - add matrix, fallback, and refresh coverage 🔒 - Generated by Copilot
|
On the Sustainability Journal page, the new SetFormulaInputEditability() recomputation is wired to the 'Manual Input' field's OnValidate trigger, OnAfterGetRecord, and OnAfterGetCurrRecord, but not to the 'Sustainability Account Category' field (Rec."Account Category") OnValidate trigger. Editability of Fuel/Electricity, Distance, Custom Amount, Installation Multiplier, and Time Factor is derived from the account category's Emission Scope/Calculation Foundation (via SustainAccountCategory.Get), so changing the account category on an existing line should immediately re-evaluate which formula-input fields are editable. Today the stale Editable flags persist until the row loses and regains focus (triggering OnAfterGetRecord/OnAfterGetCurrRecord), so a user who changes Account Category without navigating away can end up with formula-input fields still shown editable/non-editable for the previous category. Compare with the equivalent Purchase Order Subform page extension in this same PR, where the analogous field ('Sust. Account No.') OnValidate trigger does call SetFormulaInputEditability() — confirming the Journal page's omission is an inconsistency rather than an intentional design choice. Add an OnValidate trigger to the 'Sustainability Account Category' field calling SetFormulaInputEditability(), mirroring the 'Manual Input' field trigger. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
|
The new Initialize() procedure added to SustainabilityJournalTest.Codeunit.al omits LibraryTestInitialize.OnTestInitialize (and the symmetric OnBeforeTestSuiteInitialize / OnAfterTestSuiteInitialize guards). The sibling codeunit SustainabilityFormulasTest (which already uses Initialize() and IsInitialized in this same PR) follows the full pattern. Without LibraryTestInitialize.OnTestInitialize, test runners that subscribe to those events for per-test reset, diagnostics, or mock teardown will not be notified when any of the three new tests begins. Recommend aligning with the SustainabilityFormulasTest pattern: declare LibraryTestInitialize: Codeunit "Library - Test Initialize", add an IsInitialized guard, call LibraryTestInitialize.OnTestInitialize(Codeunit::"Sustainability Journal Test") at the top of Initialize(), and wrap one-time setup in if IsInitialized then exit with matching OnBeforeTestSuiteInitialize / OnAfterTestSuiteInitialize brackets. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
Agentic PR Review - Round 1Recommendation: Request ChangesWhat this PR doesThis PR centralizes formula input editability in Sustainability Calc. Mgt. and uses it on Sustainability Journal and Purchase Order lines. The matrix matches the existing emission calculation paths: Scope 1 fuel, distance, and installations; Scope 2 fuel/custom; Scope 3 fuel, distance plus installation multiplier, and custom; and Water/Waste custom only on journals. The journal refresh paths and Purchase Order account refresh path are tested. The fix is still incomplete because one affected purchase document surface from the bug evidence keeps the old editable behavior. SuggestionsS1 - Apply the rule to purchase invoices Risk assessment and necessityRisk: The changed helper is internal and the page changes do not alter public APIs or BaseApp event contracts. The regression surface is the Sustainability Journal and purchase document formula UI, where inconsistent editability can still let users enter values that are ignored by emission calculation. Necessity: The bug is valid because the active Calculation Foundation is not shown on the line, and unused editable inputs are misleading. The scope is right for journals and Purchase Orders, but it must cover the stated purchase invoice surface before merge.
|
⚡ - Generated by Copilot
| Enum::"Emission Scope"::"Water/Waste": | ||
| if (not PurchaseSurface) and (CalculationFoundation = Enum::"Calculation Foundation"::Custom) then | ||
| CustomAmountEditable := true; | ||
| end; |
There was a problem hiding this comment.
GetCalculationParameters in 'Sustainability Calc. Mgt.' caches the 'Emission Scope' and 'Calculation Foundation' of a Sustain. Account Category in page-lifetime Dictionary fields (EmissionScopeCache / CalculationFoundationCache) keyed only by account category code, with no invalidation. Because the 'Sustainability Calc. Mgt.' codeunit instance is held as a page-level variable on both the Sustainability Journal page and the Purchase Order Subform, the cache lives for the whole page session. If a user edits an account category's Emission Scope or Calculation Foundation (e.g. via the Sustain. Account Categories page) while a Sustainability Journal or Purchase Order page is already open, the open page keeps showing stale field editability for that category until it is closed and reopened, since the cached values are never refreshed or invalidated. Consider keying/expiring the cache per page-open, or accepting this as a known limitation given account categories are typically static setup data (should be confirmed with the author).
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
|
|
||
| trigger OnAfterGetRecord() | ||
| begin | ||
| SetFormulaInputEditability(); |
There was a problem hiding this comment.
SetFormulaInputEditability is invoked unconditionally from OnAfterGetRecord/OnAfterGetCurrRecord (and from the Sust. Account No. OnValidate trigger) on the Purchase Order Subform even when the sustainability formula fields are not visible (SustainabilityFormulasFieldVisible = false, i.e. 'Use Formulas In Purch. Docs' is off in Sustainability Setup). This causes an unnecessary lookup into 'Sustainability Calc. Mgt.' for every row rendered on a purchase order subform for customers who do not use this feature at all. Guard the call with the existing SustainabilityFormulasFieldVisible flag so the cost is only paid when the fields are actually shown.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
local procedure SetFormulaInputEditability()
begin
if not SustainabilityFormulasFieldVisible then
exit;
SustainabilityCalcMgt.GetFormulaInputEditability(Rec, FuelElectricityEditable, DistanceEditable, CustomAmountEditable, InstallationMultiplierEditable, TimeFactorEditable);
end;Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.33.4
ISSUE:
Sustainability Journal and Purchase Order lines allow editing every formula input, even when the selected account category uses only a subset of those values.
CAUSE:
Formula field editability did not consider the account category's Emission Scope and Calculation Foundation.
SOLUTION:
Added shared editability rules for Sustainability Journal and Purchase Order lines. Only formula inputs used by the selected scope and calculation foundation are editable. Manual Input, account changes, blank setup, and unsupported combinations refresh to the appropriate state.
TESTS:
Added TestPage coverage for the full Journal and Purchase Order matrix, fallback setup, account changes, and Manual Input.
Fixes AB#646432