Add CMFRT best practices and anti-patterns documentation for various coding standards

custom/knowledge/patterns/cmfrt-labels-local-scope.bad.al

@@ -0,0 +1,18 @@
+codeunit 55026 "CMFRT AQ ServiceWarehouseMeth"
+{
+    var
+        // Global labels: shared between unrelated procedures, outlive their
+        // callers as dead text, and %1/%2 are undocumented for translators.
+        CreateTransferOrdersQst: Label 'Create 2 transfer orders for job %1?';
+        TransferOrdersCreatedMsg: Label 'Transfer orders %1 and %2 created.';
+
+    local procedure DoCMFRTAQMakeTransferOrders(var Job: Record Job)
+    var
+        ConfirmMgt: Codeunit "Confirm Management";
+    begin
+        if not ConfirmMgt.GetResponseOrDefault(StrSubstNo(CreateTransferOrdersQst, Job."No."), true) then
+            exit;
+        // ... create orders ...
+        Message(TransferOrdersCreatedMsg, 'T-001', 'T-002');
+    end;
+}

custom/knowledge/patterns/cmfrt-labels-local-scope.good.al

@@ -0,0 +1,14 @@
+codeunit 55026 "CMFRT AQ ServiceWarehouseMeth"
+{
+    local procedure DoCMFRTAQMakeTransferOrders(var Job: Record Job)
+    var
+        ConfirmMgt: Codeunit "Confirm Management";
+        CreateTransferOrdersQst: Label 'Create 2 transfer orders for job %1?', Comment = '%1 = Job No.';
+        TransferOrdersCreatedMsg: Label 'Transfer orders %1 and %2 created.', Comment = '%1 = Transfer Order No. 1, %2 = Transfer Order No. 2';
+    begin
+        if not ConfirmMgt.GetResponseOrDefault(StrSubstNo(CreateTransferOrdersQst, Job."No."), true) then
+            exit;
+        // ... create orders ...
+        Message(TransferOrdersCreatedMsg, 'T-001', 'T-002');
+    end;
+}

custom/knowledge/patterns/cmfrt-labels-local-scope.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: patterns
+keywords: [label, local-scope, global-var, comment-attribute, placeholder, translation, strsubstno]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# CMFRT Labels are local with documented placeholders
+
+## Description
+
+`Label` variables in CMFRT code are declared in the `var` section of the one procedure that uses them, not in the codeunit's global `var` section. Every label whose text contains `%1`-style placeholders carries a `Comment` attribute documenting each placeholder (`Comment = '%1 = Job No.'`). Global label sections accumulate text that no procedure references anymore, and undocumented placeholders leave translators guessing what will be substituted.
+
+## Best Practice
+
+Declare each label local to the procedure (usually the `Do<Name>` procedure) that passes it to `Error`, `Message`, `Confirm Management`, or `StrSubstNo`. Add `Comment` naming every placeholder. When review moves logic into a `Do` procedure, move its labels with it.
+
+See sample: `cmfrt-labels-local-scope.good.al`.
+
+## Anti Pattern
+
+Labels declared in the codeunit-level `var` section, or placeholder labels without a `Comment` attribute. Global labels outlive their callers as dead text, are shared between unrelated procedures, and their missing placeholder documentation produces mistranslations that only surface in localized builds.
+
+See sample: `cmfrt-labels-local-scope.bad.al`.

custom/knowledge/patterns/cmfrt-maxstrlen-copystr.bad.al

@@ -0,0 +1,9 @@
+codeunit 55028 "CMFRT AQ FS Item Attr Push"
+{
+    local procedure DoCMFRTAQInitAPILogEntry(var CMFRTAQAPILog: Record "CMFRT AQ API Log"; ErrorText: Text)
+    begin
+        // Literal lengths diverge from the field definition on the first schema change.
+        CMFRTAQAPILog."CMFRT AQ User ID" := CopyStr(UserId(), 1, 50);
+        CMFRTAQAPILog."CMFRT AQ Error Message" := CopyStr(ErrorText, 1, 250);
+    end;
+}

custom/knowledge/patterns/cmfrt-maxstrlen-copystr.good.al

@@ -0,0 +1,8 @@
+codeunit 55028 "CMFRT AQ FS Item Attr Push"
+{
+    local procedure DoCMFRTAQInitAPILogEntry(var CMFRTAQAPILog: Record "CMFRT AQ API Log"; ErrorText: Text)
+    begin
+        CMFRTAQAPILog."CMFRT AQ User ID" := CopyStr(UserId(), 1, MaxStrLen(CMFRTAQAPILog."CMFRT AQ User ID"));
+        CMFRTAQAPILog."CMFRT AQ Error Message" := CopyStr(ErrorText, 1, MaxStrLen(CMFRTAQAPILog."CMFRT AQ Error Message"));
+    end;
+}

custom/knowledge/patterns/cmfrt-maxstrlen-copystr.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: patterns
+keywords: [maxstrlen, copystr, truncation, magic-number, field-length, overflow, string]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# CMFRT use MaxStrLen in CopyStr, never a literal length
+
+## Description
+
+When CMFRT code truncates a text value to fit a field, the length argument of `CopyStr` must be `MaxStrLen(TargetRecord.TargetField)`, never a numeric literal. The field is the single source of truth for its own length; a hardcoded number silently diverges the moment the field is widened or the code is copied to a field of a different size, producing either needless truncation or a runtime overflow error.
+
+## Best Practice
+
+Write `Rec."CMFRT AQ User ID" := CopyStr(UserId(), 1, MaxStrLen(Rec."CMFRT AQ User ID"));`. The expression stays correct through any future field-length change and documents intent: truncate to whatever fits the target.
+
+See sample: `cmfrt-maxstrlen-copystr.good.al`.
+
+## Anti Pattern
+
+`CopyStr(UserId(), 1, 50)` — the literal encodes the field length at the time of writing. Widening the field leaves data truncated at the stale length; narrowing it turns the assignment into a runtime "length exceeds" error that only fires on long values in production.
+
+See sample: `cmfrt-maxstrlen-copystr.bad.al`.

custom/knowledge/patterns/cmfrt-validate-not-assign.bad.al

@@ -0,0 +1,14 @@
+codeunit 55029 "CMFRT AQ FS Job Creator"
+{
+    local procedure CMFRTAQFillJobFromBuffer(var WorkOrderBuffer: Record "CMFRT AQ FS WO Buffer"; var Job: Record Job)
+    begin
+        Job.Init();
+        Job."No." := WorkOrderBuffer."CMFRT AQ No.";
+        // Direct assignment skips OnValidate: dependent fields stay empty,
+        // posting-group checks and field subscribers never run.
+        Job.Description := WorkOrderBuffer."CMFRT AQ Description";
+        Job."Sell-to Customer No." := WorkOrderBuffer."CMFRT AQ Sell-to Customer No.";
+        Job."Ship-to Address" := WorkOrderBuffer."CMFRT AQ Ship-to Address";
+        Job."Location Code" := WorkOrderBuffer."CMFRT AQ Location Code";
+    end;
+}

custom/knowledge/patterns/cmfrt-validate-not-assign.good.al

@@ -0,0 +1,14 @@
+codeunit 55029 "CMFRT AQ FS Job Creator"
+{
+    local procedure CMFRTAQFillJobFromBuffer(var WorkOrderBuffer: Record "CMFRT AQ FS WO Buffer"; var Job: Record Job)
+    begin
+        Job.Init();
+        // Primary key: direct assignment, never Validate.
+        Job."No." := WorkOrderBuffer."CMFRT AQ No.";
+        // Business fields: Validate so OnValidate logic runs.
+        Job.Validate(Description, WorkOrderBuffer."CMFRT AQ Description");
+        Job.Validate("Sell-to Customer No.", WorkOrderBuffer."CMFRT AQ Sell-to Customer No.");
+        Job.Validate("Ship-to Address", WorkOrderBuffer."CMFRT AQ Ship-to Address");
+        Job.Validate("Location Code", WorkOrderBuffer."CMFRT AQ Location Code");
+    end;
+}

custom/knowledge/patterns/cmfrt-validate-not-assign.md

@@ -0,0 +1,26 @@
+---
+bc-version: [all]
+domain: patterns
+keywords: [validate, assignment, onvalidate, field-population, buffer, creator, direct-assignment]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# CMFRT populate real tables via Validate, not direct assignment
+
+## Description
+
+When CMFRT code populates fields on a real (non-buffer) table — Job, Sales Header, Sales Line, Job Planning Line, Ship-to Address, Transfer Header and the like — each field must be set with `Record.Validate(Field, Value)` so the field's `OnValidate` trigger logic runs. Direct assignment (`Record.Field := Value`) silently skips validation, dependent-field copying, posting-group checks, and any subscriber logic attached to the field. Two exceptions apply: primary-key and document-number fields that are set as part of record identity are assigned directly (validating them can renumber or re-key the record), and buffer/staging tables are always filled by direct assignment because they carry no business logic.
+
+## Best Practice
+
+In creator and method codeunits that transfer buffer values into real tables, call `Validate` for every business field: `Job.Validate(Description, Buffer."CMFRT AQ Description");`. Keep primary-key fields (`Job."No." := ...`, `SalesLine."Document No." := ...`) as direct assignments, set immediately after `Init()`. Fill buffer tables by direct assignment.
+
+See sample: `cmfrt-validate-not-assign.good.al`.
+
+## Anti Pattern
+
+Filling a real table field-by-field with `:=`. The record is inserted with unvalidated data: posting-group checks never run, dependent fields (ship-to copies, cost fields, status transitions) stay empty or stale, and downstream extensions subscribed to `OnValidate` never fire. The defect surfaces later as inconsistent data that is hard to trace back to the skipped trigger.
+
+See sample: `cmfrt-validate-not-assign.bad.al`.