mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 06:36:55 +01:00
* knowledge(events): ChangeCompany leaves triggers and trigger-event subscribers running in the calling company ChangeCompany redirects only the data access of a record variable; Learn states that triggers still run in the current company. The database trigger events are raised on every database operation and only pass RunTrigger to the subscriber, so Insert(false) after ChangeCompany still runs every subscriber in the calling company. Generated code either assumes the record 'becomes' a target-company record, or switches RunTrigger off and hand-copies the trigger logic, leaving the subscribers writing to the wrong company; none of the tested runs reached StartSession with the company parameter. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * knowledge(events): qualify StartSession async semantics and fix concurrency-unsafe sample key Addresses PR #152 review: StartSession is a fire-and-forget background session (Ok reports only whether it started, not whether the codeunit succeeded, and errors inside it do not propagate), so the Best Practice now scopes the recommendation and calls out the durable status/error channel a synchronous-success write needs. The good sample's FindLast()+1 entry-number pattern raced under concurrent background sessions; switched to AutoIncrement, which the platform guarantees is unique across concurrent transactions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * knowledge(events): serialize the setup-counter increment; promote article and wire the review skill PR #152 round 3 (JesperSchulz): - The good sample's OnAfterInsertEvent subscriber still raced on the shared "Transfer Setup Good" singleton (Get/increment/Modify); AutoIncrement only protected the request key. Added TransferSetup.LockTable() before Get() to serialize concurrent background sessions. - Promoted changecompany-runs-triggers-in-the-calling-company from community/knowledge/events/ to microsoft/knowledge/events/, and wired ChangeCompany/StartSession/RunTrigger tokens plus a targeted detection cue into microsoft/skills/review/al-events-review.md so a diff containing the anti-pattern reliably worklists this article, preserving the documented RunTrigger=false hand-off exception. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: waldo1001 <12088142+waldo1001@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
91 lines
2.7 KiB
AL
91 lines
2.7 KiB
AL
codeunit 50100 "Transfer Request Bad"
|
|
{
|
|
// Self-contained demonstration of the anti pattern. Not derived from base-app source.
|
|
procedure RequestFromCompany(TargetCompany: Text[30]; ItemNo: Code[20]; Quantity: Decimal)
|
|
var
|
|
TransferRequest: Record "Transfer Request Bad";
|
|
TransferSetup: Record "Transfer Setup Bad";
|
|
begin
|
|
TransferRequest.ChangeCompany(TargetCompany);
|
|
TransferSetup.ChangeCompany(TargetCompany);
|
|
TransferSetup.Get();
|
|
|
|
TransferRequest.Init();
|
|
TransferRequest."Entry No." := NextEntryNo(TargetCompany);
|
|
TransferRequest."Item No." := ItemNo;
|
|
TransferRequest.Quantity := Quantity;
|
|
// OnInsert is skipped below, so the default is copied by hand from the target company's setup.
|
|
TransferRequest."Location Code" := TransferSetup."Default Location Code";
|
|
// The OnAfterInsertEvent subscriber still fires, in the calling company, and grows the caller's counter.
|
|
TransferRequest.Insert(false);
|
|
|
|
TransferSetup."Open Requests" += 1;
|
|
TransferSetup.Modify();
|
|
end;
|
|
|
|
local procedure NextEntryNo(TargetCompany: Text[30]): Integer
|
|
var
|
|
LastRequest: Record "Transfer Request Bad";
|
|
begin
|
|
LastRequest.ChangeCompany(TargetCompany);
|
|
if LastRequest.FindLast() then
|
|
exit(LastRequest."Entry No." + 1);
|
|
exit(1);
|
|
end;
|
|
}
|
|
|
|
table 50100 "Transfer Request Bad"
|
|
{
|
|
DataClassification = CustomerContent;
|
|
|
|
fields
|
|
{
|
|
field(1; "Entry No."; Integer) { }
|
|
field(2; "Item No."; Code[20]) { }
|
|
field(3; Quantity; Decimal) { }
|
|
field(4; "Location Code"; Code[10]) { }
|
|
}
|
|
|
|
keys
|
|
{
|
|
key(PK; "Entry No.") { Clustered = true; }
|
|
}
|
|
|
|
trigger OnInsert()
|
|
var
|
|
TransferSetup: Record "Transfer Setup Bad";
|
|
begin
|
|
TransferSetup.Get();
|
|
"Location Code" := TransferSetup."Default Location Code";
|
|
end;
|
|
}
|
|
|
|
table 50101 "Transfer Setup Bad"
|
|
{
|
|
DataClassification = CustomerContent;
|
|
|
|
fields
|
|
{
|
|
field(1; "Primary Key"; Code[10]) { }
|
|
field(2; "Default Location Code"; Code[10]) { }
|
|
field(3; "Open Requests"; Integer) { }
|
|
}
|
|
|
|
keys
|
|
{
|
|
key(PK; "Primary Key") { Clustered = true; }
|
|
}
|
|
}
|
|
|
|
codeunit 50101 "Transfer Request Count Bad"
|
|
{
|
|
[EventSubscriber(ObjectType::Table, Database::"Transfer Request Bad", OnAfterInsertEvent, '', false, false)]
|
|
local procedure CountOpenRequest(var Rec: Record "Transfer Request Bad"; RunTrigger: Boolean)
|
|
var
|
|
TransferSetup: Record "Transfer Setup Bad";
|
|
begin
|
|
TransferSetup.Get();
|
|
TransferSetup."Open Requests" += 1;
|
|
TransferSetup.Modify();
|
|
end;
|
|
}
|