[Extensibility Request] issue 30335: add OnBeforeRecreateSalesLine IsHandled event#9461
[Extensibility Request] issue 30335: add OnBeforeRecreateSalesLine IsHandled event#9461AleksandricMarko wants to merge 1 commit into
Conversation
… Sales Header Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Copilot PR ReviewIteration 1 · Outcome: completed Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf Findings by domainFindings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).
Totals: 1 knowledge-backed · 0 agent findings. Orchestrator pre-filter (2 file(s) excluded)
Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives. |
|
|
qasimikram
left a comment
qasimikram
left a comment
There was a problem hiding this comment.
The event is narrowly placed around the requested CreateSalesLine call and is implemented consistently across the country-layer copies.
S1: Please add regression coverage for this new handled path. Verify that the unhandled path remains unchanged and that a subscriber can create exactly one replacement line without duplicate creation while the existing post-creation processing continues to run correctly.
| ShouldCreateSalsesLine := not TempSalesLine.IsExtendedText(); | ||
| OnRecreateSalesLinesHandleSupplementTypesOnAfterCalcShouldCreateSalsesLine(TempSalesLine, ShouldCreateSalsesLine, SalesLine); | ||
| if ShouldCreateSalsesLine then begin | ||
| CreateSalesLine(TempSalesLine); |
There was a problem hiding this comment.
The new OnBeforeRecreateSalesLine event lets any subscriber set IsHandled := true and suppress the call to CreateSalesLine entirely, but the routine keeps running as if a line had been created: it still checks SalesLine.Type, inserts into TempInteger keyed by SalesLine."Line No.", calls RecreateSalesLinesFillItemChargeAssignment, and raises OnRecreateSalesLinesHandleSupplementTypesOnAfterCreateSalesLine with a SalesLine that is stale (left over from a previous TempSalesLine iteration) rather than reflecting the current TempSalesLine. That leaves TempSalesLine's data silently dropped and downstream logic associated with the wrong line, which is exactly the kind of system inconsistency this guidance warns against. Scope the override to a safe, self-contained computation (or expose an OnAfter... event for adjustments) and run CreateSalesLine unconditionally, rather than making the insert itself skippable.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
| /// <param name="TempSalesLine">The temporary sales line used as source.</param> | ||
| /// <param name="SalesHeader">The parent sales header record.</param> | ||
| [IntegrationEvent(false, false)] | ||
| local procedure OnBeforeRecreateSalesLine(var IsHandled: Boolean; var SalesLine: Record "Sales Line"; var TempSalesLine: Record "Sales Line" temporary; var SalesHeader: Record "Sales Header") |
There was a problem hiding this comment.
OnBeforeRecreateSalesLine is a near-duplicate of the existing OnBeforeCreateSalesLine event: CreateSalesLine already raises OnBeforeCreateSalesLine(TempSalesLine, IsHandled, Rec, SalesLine) at its very top with 'if IsHandled then exit;', exposing the identical override (skip creating the line) with the identical data (TempSalesLine, Rec, SalesLine). A subscriber that wants to replace line creation from this call path can already do so by subscribing to OnBeforeCreateSalesLine; the new event adds a second seam for the same decision instead of reusing the one that already exists, which will leave subscribers unsure which event to pick.
Knowledge:
- microsoft/knowledge/events/prefer-reusing-or-extending-existing-events.md
- microsoft/knowledge/events/do-not-bypass-critical-operations-with-ishandled.md
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.15.4
3 participants
Summary
The issue author needs to conditionally replace the standard sales line recreation logic inside
RecreateSalesLinesHandleSupplementTypesin table 36 "Sales Header". Under specific business conditions based on extension fields, their extension must fully take over sales line creation, which requires bypassing the standardCreateSalesLinecall at the caller level. No existing extensibility point sits immediately before that call, so this PR adds a newIsHandledintegration event that lets subscribers skip the standard creation and run an equivalent custom implementation.Source issue repository: microsoft/AlAppExtensions; issue number: 30335
Changes Made
OnBeforeRecreateSalesLine- newIsHandledintegration event published inRecreateSalesLinesHandleSupplementTypesimmediately beforeCreateSalesLine, allowing subscribers to skip the standard sales line recreation; the same change is propagated to all country layers that have their own copy of the table.Fixes AB#641988