Merge e00ef8ad94 into 1687b57c99

community/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.bad.al

@@ -0,0 +1,48 @@
+table 50100 "Order Header"
+{
+    fields
+    {
+        field(1; "Entry No."; Integer)
+        {
+            DataClassification = CustomerContent;
+        }
+    }
+
+    keys
+    {
+        key(PK; "Entry No.")
+        {
+            Clustered = true;
+        }
+    }
+
+    // No OnDelete. Deleting a header silently orphans every Order Line that
+    // belonged to it. Nothing errors, and no page shows the stranded rows.
+}
+
+table 50101 "Order Line"
+{
+    fields
+    {
+        field(1; "Line No."; Integer)
+        {
+            DataClassification = CustomerContent;
+        }
+        field(2; "Header Entry No."; Integer)
+        {
+            DataClassification = CustomerContent;
+            // Reads like referential integrity. It is lookup and input validation
+            // only: it propagates a RENAME of the parent key, and cascades nothing
+            // on DELETE.
+            TableRelation = "Order Header"."Entry No.";
+        }
+    }
+
+    keys
+    {
+        key(PK; "Line No.")
+        {
+            Clustered = true;
+        }
+    }
+}

community/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.good.al

@@ -0,0 +1,55 @@
+table 50100 "Order Header"
+{
+    // The owning table needs delete rights on what it owns. Granting D only on the
+    // header is a common miss and makes OnDelete fail for a non-SUPER user.
+    Permissions = tabledata "Order Line" = rd;
+
+    fields
+    {
+        field(1; "Entry No."; Integer)
+        {
+            DataClassification = CustomerContent;
+        }
+    }
+
+    keys
+    {
+        key(PK; "Entry No.")
+        {
+            Clustered = true;
+        }
+    }
+
+    trigger OnDelete()
+    var
+        OrderLine: Record "Order Line";
+    begin
+        OrderLine.SetRange("Header Entry No.", "Entry No.");
+        // Pass false only when Order Line has no OnDelete of its own.
+        OrderLine.DeleteAll(true);
+    end;
+}
+
+table 50101 "Order Line"
+{
+    fields
+    {
+        field(1; "Line No."; Integer)
+        {
+            DataClassification = CustomerContent;
+        }
+        field(2; "Header Entry No."; Integer)
+        {
+            DataClassification = CustomerContent;
+            TableRelation = "Order Header"."Entry No.";
+        }
+    }
+
+    keys
+    {
+        key(PK; "Line No.")
+        {
+            Clustered = true;
+        }
+    }
+}

community/knowledge/data-modeling/owning-table-must-delete-dependents-in-ondelete.md

@@ -0,0 +1,36 @@
+---
+bc-version: [all]
+domain: data-modeling
+keywords: [ondelete, cascade, table-relation, orphan-records, header-line, dependent-records, referential-integrity]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# A table that owns dependent records must delete them in `OnDelete`
+
+## Description
+
+`TableRelation` looks like referential integrity but only performs lookup and input validation. AL has **no cascading delete**: deleting a parent record leaves every dependent row untouched, and no error is raised.
+
+What makes this specifically missable is an asymmetry. The platform *does* keep references correct on **rename** — renaming a record updates it in all other locations that declare a `TableRelation` to it, with no code. Delete has no equivalent. Same relation, same metadata, opposite behaviour. A developer who correctly learns that `TableRelation` "keeps references consistent" from the rename case, and generalises it to delete, ships orphans.
+
+Orphaned rows are usually invisible, because a dependent table rarely has a page of its own. They inflate the table, break later reconciliation, and are re-encountered by duplicate checks when the parent key is reused.
+
+This applies to internal, staging and `SystemMetadata` tables too. A table having no delete action in the UI today is not protection: a permission set that grants `D` on the table is evidence that deletion is anticipated.
+
+See also `validate-table-relation-false-suppresses-rename-propagation.md` for the two preconditions on the rename half of this asymmetry.
+
+## Best Practice
+
+The owning table implements `OnDelete` and deletes its dependents there, filtered on the foreign key. Declare `Permissions = tabledata <dependent> = rd` on the owning table — granting delete rights only on the parent is a common miss that makes the trigger fail for a non-`SUPER` user. This mirrors the base application, where every header table deletes its own lines.
+
+See sample: `owning-table-must-delete-dependents-in-ondelete.good.al`.
+
+## Anti Pattern
+
+A parent table with dependent rows and no `OnDelete` trigger, where the dependent's foreign-key field declares a `TableRelation` back to the parent. The relation reads as if it guarantees integrity; it does not.
+
+Detection signal: a table declares `TableRelation` to table X, and table X has no `OnDelete` trigger. Whether a delete path currently exists in the UI is irrelevant to the finding.
+
+See sample: `owning-table-must-delete-dependents-in-ondelete.bad.al`.

community/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.bad.al

@@ -0,0 +1,35 @@
+table 50121 "Document Link"
+{
+    fields
+    {
+        field(1; "Entry No."; Integer)
+        {
+            DataClassification = CustomerContent;
+        }
+        field(2; "Document No."; Code[20])
+        {
+            DataClassification = CustomerContent;
+            TableRelation = "Source Document"."No.";
+            // Added to silence a validation error while the row is staged, before
+            // the Source Document exists. The relation is still declared, so this
+            // reads as harmless — but it also switches OFF rename propagation.
+            // Renaming a Source Document now leaves this field on the old key,
+            // with no error, and nothing else maintains it.
+            ValidateTableRelation = false;
+        }
+        // Composite value: no TableRelation is possible, and no OnRename on the
+        // owning table maintains it either. Rots the same way, for the other reason.
+        field(3; "Source Key"; Code[50])
+        {
+            DataClassification = CustomerContent;
+        }
+    }
+
+    keys
+    {
+        key(PK; "Entry No.")
+        {
+            Clustered = true;
+        }
+    }
+}

community/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.good.al

@@ -0,0 +1,70 @@
+table 50120 "Source Document"
+{
+    fields
+    {
+        field(1; "No."; Code[20])
+        {
+            DataClassification = CustomerContent;
+        }
+    }
+
+    keys
+    {
+        key(PK; "No.")
+        {
+            Clustered = true;
+        }
+    }
+
+    // "Source Key" below cannot declare a TableRelation, so the platform cannot
+    // repoint it. The owning table carries the relationship by hand.
+    trigger OnRename()
+    var
+        DocumentLink: Record "Document Link";
+    begin
+        // In OnRename, xRec holds the PREVIOUS primary key while Rec holds the new
+        // one — the one trigger where that is true regardless of what drove the rename.
+        DocumentLink.SetRange("Source Key", MakeSourceKey(xRec."No."));
+        if DocumentLink.FindSet(true) then
+            repeat
+                DocumentLink."Source Key" := MakeSourceKey(Rec."No.");
+                DocumentLink.Modify(true);
+            until DocumentLink.Next() = 0;
+    end;
+
+    local procedure MakeSourceKey(DocumentNo: Code[20]): Code[50]
+    begin
+        exit(StrSubstNo('DOC|%1', DocumentNo));
+    end;
+}
+
+table 50121 "Document Link"
+{
+    fields
+    {
+        field(1; "Entry No."; Integer)
+        {
+            DataClassification = CustomerContent;
+        }
+        // Default ValidateTableRelation: the platform repoints this on rename.
+        field(2; "Document No."; Code[20])
+        {
+            DataClassification = CustomerContent;
+            TableRelation = "Source Document"."No.";
+        }
+        // Composite value — no TableRelation can express it, so the parent's
+        // OnRename above maintains it explicitly.
+        field(3; "Source Key"; Code[50])
+        {
+            DataClassification = CustomerContent;
+        }
+    }
+
+    keys
+    {
+        key(PK; "Entry No.")
+        {
+            Clustered = true;
+        }
+    }
+}

community/knowledge/data-modeling/validate-table-relation-false-suppresses-rename-propagation.md

@@ -0,0 +1,38 @@
+---
+bc-version: [all]
+domain: data-modeling
+keywords: [validatetablerelation, table-relation, rename, onrename, propagation, dangling-reference, soft-relation]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# `ValidateTableRelation = false` suppresses rename propagation, not just input validation
+
+## Description
+
+Renaming a record updates it in all other locations that reference it through a `TableRelation`, with no code. That guarantee has **two** preconditions, and a field failing either one is silently left holding a key that no longer exists.
+
+First, a `TableRelation` must exist. A field whose value is constructed or computed — a composite key, or a value derived from several fields of the target — cannot declare one, so nothing propagates. The field is still a foreign key in intent, but the platform treats it as an opaque value.
+
+Second, and far less obvious: the relation must not carry `ValidateTableRelation = false`. The property name implies it only governs *input validation*, so it looks safe to disable on a field populated by code that already knows the target is valid. It is not. **Disabling it also switches off rename propagation.** The relation still documents intent and still drives lookups, but it no longer keeps the stored value correct.
+
+Both failures are quiet: no error at rename time, and in the first case no input validation either, so a wrong value is never rejected on write.
+
+This is verified behaviour, not inference. A parent renamed once against a child holding three fields — a normal relation, the same relation with `ValidateTableRelation = false`, and a field with no relation — updates only the first.
+
+See also `owning-table-must-delete-dependents-in-ondelete.md` for the delete half of this asymmetry, and `xrec-is-a-before-image-only-in-some-triggers.md` for why `OnRename` is the one trigger where a hand-written fix-up is reliable.
+
+## Best Practice
+
+Leave `ValidateTableRelation` at its default wherever the stored value must stay correct across a rename. When it must be disabled, or when the relationship cannot be expressed as a `TableRelation` at all, the table owning the referenced key carries an explicit `OnRename` that repoints the dependents itself.
+
+See sample: `validate-table-relation-false-suppresses-rename-propagation.good.al`.
+
+## Anti Pattern
+
+`ValidateTableRelation = false` added to silence a validation error, on a field expected to keep tracking its target. The field stops being maintained on rename, and the defect surfaces much later as a reference to a key that no longer exists.
+
+Detection signal: any `ValidateTableRelation = false` on a field that also declares a `TableRelation`. Ask what repoints the value when the target is renamed; if the answer is "the platform", the finding stands.
+
+See sample: `validate-table-relation-false-suppresses-rename-propagation.bad.al`.

community/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.bad.al

@@ -0,0 +1,36 @@
+table 50130 "Service Request"
+{
+    fields
+    {
+        field(1; "No."; Code[20])
+        {
+            DataClassification = CustomerContent;
+        }
+        field(2; Status; Enum "Service Request Status")
+        {
+            DataClassification = CustomerContent;
+        }
+    }
+
+    keys
+    {
+        key(PK; "No.")
+        {
+            Clustered = true;
+        }
+    }
+
+    trigger OnModify()
+    begin
+        // Dead branch under any code-driven Modify: from code xRec mirrors Rec, so
+        // the two Status values are always equal and LogStatusChange never runs.
+        // Editing the field on a page DOES populate xRec, so this passes manual
+        // testing and then silently does nothing in a job queue or API call.
+        if Status <> xRec.Status then
+            LogStatusChange(xRec.Status, Status);
+    end;
+
+    local procedure LogStatusChange(FromStatus: Enum "Service Request Status"; ToStatus: Enum "Service Request Status")
+    begin
+    end;
+}

community/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.good.al

@@ -0,0 +1,58 @@
+table 50130 "Service Request"
+{
+    fields
+    {
+        field(1; "No."; Code[20])
+        {
+            DataClassification = CustomerContent;
+        }
+        field(2; Status; Enum "Service Request Status")
+        {
+            DataClassification = CustomerContent;
+        }
+    }
+
+    keys
+    {
+        key(PK; "No.")
+        {
+            Clustered = true;
+        }
+    }
+
+    // OnModify: xRec mirrors Rec when the write came from code, so it cannot be
+    // used as a before-image. Re-read the stored row instead — this behaves the
+    // same whether a page, a job queue or an API drove the write.
+    trigger OnModify()
+    var
+        Previous: Record "Service Request";
+    begin
+        if Previous.Get("No.") and (Previous.Status <> Status) then
+            LogStatusChange(Previous.Status, Status);
+    end;
+
+    // OnRename: xRec IS the before-image of the primary key here, whatever drove
+    // the rename. This is the one trigger where the idiom is reliable.
+    trigger OnRename()
+    begin
+        RepointDependents(xRec."No.", "No.");
+    end;
+
+    // OnDelete: xRec reflects the record being removed.
+    trigger OnDelete()
+    begin
+        ArchiveRequest(xRec."No.");
+    end;
+
+    local procedure LogStatusChange(FromStatus: Enum "Service Request Status"; ToStatus: Enum "Service Request Status")
+    begin
+    end;
+
+    local procedure RepointDependents(OldNo: Code[20]; NewNo: Code[20])
+    begin
+    end;
+
+    local procedure ArchiveRequest(RequestNo: Code[20])
+    begin
+    end;
+}

community/knowledge/data-modeling/xrec-is-a-before-image-only-in-some-triggers.md

@@ -0,0 +1,36 @@
+---
+bc-version: [all]
+domain: data-modeling
+keywords: [xrec, before-image, onmodify, onrename, oninsert, ondelete, table-trigger, page-driven]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# `xRec` is a before-image in `OnRename` and `OnDelete`, but mirrors `Rec` in `OnInsert` and `OnModify` from code
+
+## Description
+
+`xRec` is widely believed to be "the previous record" inside every table trigger, and — in reaction to that — is often dismissed with the folk rule *"`xRec` only works from a page, never from code"*. Both are wrong, and the second is wrong in the place it matters most.
+
+The behaviour is per-trigger. In `OnRename` and `OnDelete`, `xRec` is a genuine before-image regardless of what drove the write. In `OnInsert` and `OnModify`, a **code-driven** write leaves `xRec` mirroring `Rec` — there is no before-image at all — while a **page-driven** write does supply one.
+
+That combination produces a defect that is unusually hard to catch. A comparison such as `if Rec.Status <> xRec.Status then` inside `OnModify` works when a tester clicks through a page, and silently never fires when the same code path runs from a job queue, a batch routine, or an API call. It fails as a no-op, not as an error.
+
+The `OnRename` case is the useful half: because `xRec` there holds the previous primary key even from code, it is the one place a hand-written key fix-up is reliable. Note that in `OnRename` only the key differs between `Rec` and `xRec` — non-key field values are identical on both sides.
+
+See also `validate-table-relation-false-suppresses-rename-propagation.md`, which describes when such a hand-written `OnRename` fix-up is required.
+
+## Best Practice
+
+Use `xRec` for the previous key in `OnRename`, and for the record being removed in `OnDelete`. In `OnModify`, obtain the before-image by re-reading the stored row rather than trusting `xRec`, so the logic behaves identically whether a page, a job queue or an API drove the write.
+
+See sample: `xrec-is-a-before-image-only-in-some-triggers.good.al`.
+
+## Anti Pattern
+
+Comparing `Rec` against `xRec` inside `OnModify` (or `OnInsert`) to detect a change. From code the two are equal, so the branch is dead and whatever it guards never happens.
+
+Detection signal: any read of `xRec` inside `OnModify` or `OnInsert`. Treat "but it works when I test it on the page" as confirmation of the defect rather than a refutation.
+
+See sample: `xrec-is-a-before-image-only-in-some-triggers.bad.al`.