[Master]-Take Payment Discounts setting does not work as expected in the Suggest Vendor Payments EB routine from the EB Payment Journal in the Belgian version.#9092
Conversation
| DueDate := WorkDate(); | ||
| if PmtDiscDueDate = 0D then | ||
| PmtDiscDueDate := WorkDate(); | ||
| if (IncPmtDiscount) and (PmtDiscDueDate = 0D) then |
There was a problem hiding this comment.
Report 2000019 declares SaveValues = true, so IncPmtDiscount and PmtDiscDueDate are meant to persist across invocations.
The new OnOpenPage logic is if (IncPmtDiscount) and (PmtDiscDueDate = 0D) then PmtDiscDueDate := WorkDate() else PmtDiscDueDate := 0D;. When the checkbox was previously saved as true together with a non-zero discount date (the normal steady state after a first run), reopening the request page evaluates the second condition to false (PmtDiscDueDate <> 0D) and falls into the else branch, which wipes the persisted date back to 0D on every subsequent open. This defeats SaveValues for this field and also risks tripping the existing OnPreDataItem check if IncPmtDiscount and (PmtDiscDueDate < Today) then Confirm(...)/Error(...) on every run, since 0D < Today is always true. Actual impact is functionally significant (silent loss of a persisted user setting plus a spurious confirm/error each run), which would normally warrant major severity, but this is flagged as an unbacked agent finding and capped at minor per BCQuality's agent-finding rules; consider promoting this to a knowledge-backed rule. Recommended
Recommendation:
- only default the date when it is currently blank, independent of clearing on uncheck, e.g. keep
if PmtDiscDueDate = 0D then PmtDiscDueDate := WorkDate();in OnOpenPage (mirroring DueDate/PostingDate) and confine the clear-on-uncheck behavior to the checkbox's own OnValidate trigger.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
Copilot PR ReviewIteration 3 · Outcome: completed Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630 Findings by domainFindings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).
Totals: 0 knowledge-backed · 1 agent findings. Orchestrator pre-filter (13 file(s) excluded)
Findings produced by the Copilot CLI agent against BCQuality at |
|
|
||
| trigger OnValidate() | ||
| begin | ||
| if (IncPmtDiscount) and (PmtDiscDueDate = 0D) then |
There was a problem hiding this comment.
In the IncPmtDiscount field's OnValidate trigger, the new logic 'if (IncPmtDiscount) and (PmtDiscDueDate = 0D) then PmtDiscDueDate := WorkDate() else PmtDiscDueDate := 0D' resets PmtDiscDueDate to 0D whenever IncPmtDiscount is true but PmtDiscDueDate already holds a non-zero value (the else branch fires because the AND condition requires PmtDiscDueDate = 0D).
The prior code only ever initialized an empty date and never clobbered an already-set one. Recommend nesting the checks so a non-zero date is preserved when the checkbox is enabled, and only cleared when it is disabled: guard the WorkDate() default inside 'if IncPmtDiscount then' and only assign 0D in the 'else' for IncPmtDiscount = false.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
if IncPmtDiscount then begin
if PmtDiscDueDate = 0D then
PmtDiscDueDate := WorkDate();
end else
PmtDiscDueDate := 0D;👍 useful · ❤️ especially valuable · 👎 wrong - reply with why
…t-Discounts-not-work-in-EB-Payment-Journal
Agentic PR Review - Round 1Recommendation: Request ChangesWhat this PR doesThis PR makes the BE SuggestionsS1 - Keep discount date when enabled S2 - Test discount-only selection and amount Risk assessment and necessityRisk: This is BE payment proposal logic, so the risk is financial: a wrong branch can suggest invoices at the wrong amount or with the wrong payment discount. No new BaseApp event publisher is involved, but the relevant BE unit-test check is currently failing, so the added coverage has not proved the change yet. Necessity: The work item is a Bug with clear repro steps. Without a fix, the EB routine can use
|
|
Please review S1. |
Workitem:- Bug 641379: [master] [All-e]Take Payment Discounts setting does not work as expected in the Suggest Vendor Payments EB routine from the EB Payment Journal in the Belgian version.
Fixes AB#641379