From 057e17c20245812b3788853f2a2dbfb35c9298eb Mon Sep 17 00:00:00 2001 From: Michael Dieringer <65093775+MichaelDieringer@users.noreply.github.com> Date: Fri, 4 Sep 2026 20:43:00 +0200 Subject: [PATCH 1/7] Add 18 community AL/BC patterns across style, data-modeling, web-services, appsource, breaking-changes, performance, and testing Contributed by CURABIS ApS, generalized from patterns observed across real AppSource/PTE development. Each article follows the knowledge file format (frontmatter, Description/Best Practice/Anti Pattern, sibling .good.al/.bad.al samples). --- .../appsource/file-datatype-saas.bad.al | 13 ++++++++ .../appsource/file-datatype-saas.good.al | 17 ++++++++++ .../knowledge/appsource/file-datatype-saas.md | 28 ++++++++++++++++ .../prefer-email-module.bad.al | 13 ++++++++ .../prefer-email-module.good.al | 11 +++++++ .../breaking-changes/prefer-email-module.md | 28 ++++++++++++++++ .../check-post-line-batch-pattern.bad.al | 22 +++++++++++++ .../check-post-line-batch-pattern.good.al | 32 +++++++++++++++++++ .../check-post-line-batch-pattern.md | 28 ++++++++++++++++ .../dimension-management-wiring.bad.al | 13 ++++++++ .../dimension-management-wiring.good.al | 28 ++++++++++++++++ .../dimension-management-wiring.md | 28 ++++++++++++++++ .../do-not-change-primary-key-field-list.md | 28 ++++++++++++++++ .../do-not-change-primary-key.bad.al | 14 ++++++++ .../do-not-change-primary-key.good.al | 24 ++++++++++++++ ...stfield-required-setup-field-before-use.md | 28 ++++++++++++++++ .../testfield-required-setup-field.bad.al | 12 +++++++ .../testfield-required-setup-field.good.al | 9 ++++++ .../document-report-word-layout.bad.al | 16 ++++++++++ .../document-report-word-layout.good.al | 17 ++++++++++ .../document-report-word-layout.md | 28 ++++++++++++++++ ...ld-output-must-not-pollute-project-root.md | 24 ++++++++++++++ .../style/al-identifiers-english.bad.al | 11 +++++++ .../style/al-identifiers-english.good.al | 11 +++++++ .../style/al-identifiers-must-be-english.md | 28 ++++++++++++++++ .../binary-choice-must-be-boolean.bad.al | 22 +++++++++++++ .../binary-choice-must-be-boolean.good.al | 16 ++++++++++ .../style/binary-choice-must-be-boolean.md | 28 ++++++++++++++++ ...hoice-set-must-use-enum-not-integer.bad.al | 22 +++++++++++++ ...oice-set-must-use-enum-not-integer.good.al | 17 ++++++++++ ...ed-choice-set-must-use-enum-not-integer.md | 28 ++++++++++++++++ ...al-functions-must-use-modern-casing.bad.al | 12 +++++++ ...l-functions-must-use-modern-casing.good.al | 12 +++++++ ...sic-al-functions-must-use-modern-casing.md | 28 ++++++++++++++++ ...espace-must-be-verified-from-source.bad.al | 13 ++++++++ ...space-must-be-verified-from-source.good.al | 16 ++++++++++ .../namespace-must-be-verified-from-source.md | 28 ++++++++++++++++ ...ers-require-an-addressable-variable.bad.al | 15 +++++++++ ...rs-require-an-addressable-variable.good.al | 17 ++++++++++ ...ameters-require-an-addressable-variable.md | 28 ++++++++++++++++ ...st-data-must-be-random-and-complete.bad.al | 14 ++++++++ ...t-data-must-be-random-and-complete.good.al | 13 ++++++++ .../test-data-must-be-random-and-complete.md | 28 ++++++++++++++++ ...-page-flowfields-must-be-calcfields.bad.al | 24 ++++++++++++++ ...page-flowfields-must-be-calcfields.good.al | 27 ++++++++++++++++ .../api-page-flowfields-must-be-calcfields.md | 28 ++++++++++++++++ ...y-fields-must-be-editable-on-insert.bad.al | 27 ++++++++++++++++ ...-fields-must-be-editable-on-insert.good.al | 24 ++++++++++++++ ...e-key-fields-must-be-editable-on-insert.md | 28 ++++++++++++++++ ...fields-must-not-be-exposed-directly.bad.al | 24 ++++++++++++++ ...ields-must-not-be-exposed-directly.good.al | 31 ++++++++++++++++++ ...ved-fields-must-not-be-exposed-directly.md | 28 ++++++++++++++++ 52 files changed, 1109 insertions(+) create mode 100644 microsoft/knowledge/appsource/file-datatype-saas.bad.al create mode 100644 microsoft/knowledge/appsource/file-datatype-saas.good.al create mode 100644 microsoft/knowledge/appsource/file-datatype-saas.md create mode 100644 microsoft/knowledge/breaking-changes/prefer-email-module.bad.al create mode 100644 microsoft/knowledge/breaking-changes/prefer-email-module.good.al create mode 100644 microsoft/knowledge/breaking-changes/prefer-email-module.md create mode 100644 microsoft/knowledge/data-modeling/check-post-line-batch-pattern.bad.al create mode 100644 microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al create mode 100644 microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md create mode 100644 microsoft/knowledge/data-modeling/dimension-management-wiring.bad.al create mode 100644 microsoft/knowledge/data-modeling/dimension-management-wiring.good.al create mode 100644 microsoft/knowledge/data-modeling/dimension-management-wiring.md create mode 100644 microsoft/knowledge/data-modeling/do-not-change-primary-key-field-list.md create mode 100644 microsoft/knowledge/data-modeling/do-not-change-primary-key.bad.al create mode 100644 microsoft/knowledge/data-modeling/do-not-change-primary-key.good.al create mode 100644 microsoft/knowledge/data-modeling/testfield-required-setup-field-before-use.md create mode 100644 microsoft/knowledge/data-modeling/testfield-required-setup-field.bad.al create mode 100644 microsoft/knowledge/data-modeling/testfield-required-setup-field.good.al create mode 100644 microsoft/knowledge/performance/document-report-word-layout.bad.al create mode 100644 microsoft/knowledge/performance/document-report-word-layout.good.al create mode 100644 microsoft/knowledge/performance/document-report-word-layout.md create mode 100644 microsoft/knowledge/style/al-build-output-must-not-pollute-project-root.md create mode 100644 microsoft/knowledge/style/al-identifiers-english.bad.al create mode 100644 microsoft/knowledge/style/al-identifiers-english.good.al create mode 100644 microsoft/knowledge/style/al-identifiers-must-be-english.md create mode 100644 microsoft/knowledge/style/binary-choice-must-be-boolean.bad.al create mode 100644 microsoft/knowledge/style/binary-choice-must-be-boolean.good.al create mode 100644 microsoft/knowledge/style/binary-choice-must-be-boolean.md create mode 100644 microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.bad.al create mode 100644 microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.good.al create mode 100644 microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md create mode 100644 microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.bad.al create mode 100644 microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.good.al create mode 100644 microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md create mode 100644 microsoft/knowledge/style/namespace-must-be-verified-from-source.bad.al create mode 100644 microsoft/knowledge/style/namespace-must-be-verified-from-source.good.al create mode 100644 microsoft/knowledge/style/namespace-must-be-verified-from-source.md create mode 100644 microsoft/knowledge/style/var-parameters-require-an-addressable-variable.bad.al create mode 100644 microsoft/knowledge/style/var-parameters-require-an-addressable-variable.good.al create mode 100644 microsoft/knowledge/style/var-parameters-require-an-addressable-variable.md create mode 100644 microsoft/knowledge/testing/test-data-must-be-random-and-complete.bad.al create mode 100644 microsoft/knowledge/testing/test-data-must-be-random-and-complete.good.al create mode 100644 microsoft/knowledge/testing/test-data-must-be-random-and-complete.md create mode 100644 microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.bad.al create mode 100644 microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.good.al create mode 100644 microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.md create mode 100644 microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.bad.al create mode 100644 microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.good.al create mode 100644 microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.md create mode 100644 microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.bad.al create mode 100644 microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al create mode 100644 microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md diff --git a/microsoft/knowledge/appsource/file-datatype-saas.bad.al b/microsoft/knowledge/appsource/file-datatype-saas.bad.al new file mode 100644 index 00000000..05ab0ffc --- /dev/null +++ b/microsoft/knowledge/appsource/file-datatype-saas.bad.al @@ -0,0 +1,13 @@ +codeunit 50104 "Import File Reader" +{ + procedure ImportFile() + var + ImportFile: File; + InStream: InStream; + begin + ImportFile.WriteMode(false); + ImportFile.TextMode(true); + ImportFile.Open('C:\Import\data.txt'); // fails in SaaS — no local filesystem + ImportFile.CreateInStream(InStream); + end; +} diff --git a/microsoft/knowledge/appsource/file-datatype-saas.good.al b/microsoft/knowledge/appsource/file-datatype-saas.good.al new file mode 100644 index 00000000..63b322f5 --- /dev/null +++ b/microsoft/knowledge/appsource/file-datatype-saas.good.al @@ -0,0 +1,17 @@ +codeunit 50104 "Import File Reader" +{ + procedure ImportFile() + var + TempBlob: Codeunit "Temp Blob"; + InStream: InStream; + FileName: Text; + begin + if UploadIntoStream('Import file', '', 'All Files (*.*)|*.*', FileName, InStream) then + ParseStream(InStream); + end; + + local procedure ParseStream(var InStream: InStream) + begin + // parse InStream content here + end; +} diff --git a/microsoft/knowledge/appsource/file-datatype-saas.md b/microsoft/knowledge/appsource/file-datatype-saas.md new file mode 100644 index 00000000..2c226151 --- /dev/null +++ b/microsoft/knowledge/appsource/file-datatype-saas.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: appsource +keywords: [file-datatype, saas, onprem, uploadintostream, downloadfromstream, instream, outstream, streaming] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# The File data type's direct I/O methods are OnPrem-only + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +The classic `File` variable type — `Open`/`Create`/`Read`/`Write`/`Close` against a path on the local or server filesystem — only works on-premises, because there is no accessible filesystem in the SaaS/cloud sandbox. Any extension meant to run in Business Central Online must not rely on `File.Open`, `File.Create`, `File.Read`, or `File.Write` for its core functionality: code built this way compiles but fails, or is silently skipped, in the cloud. + +## Best Practice + +Use the stream-based equivalents: `UploadIntoStream` to read user-selected file content into an `InStream`, and `DownloadFromStream` to write an `OutStream`'s content to a file the user saves. Stage the content in a `TempBlob` between the stream and the rest of the parsing/formatting code. + +See sample: `file-datatype-saas.good.al`. + +## Anti Pattern + +Opening a hardcoded or user-supplied filesystem path with the `File` variable type. This is a strong signal the code was written for on-premises only, or copied from material that predates the cloud-first streaming APIs. + +See sample: `file-datatype-saas.bad.al`. diff --git a/microsoft/knowledge/breaking-changes/prefer-email-module.bad.al b/microsoft/knowledge/breaking-changes/prefer-email-module.bad.al new file mode 100644 index 00000000..299afd99 --- /dev/null +++ b/microsoft/knowledge/breaking-changes/prefer-email-module.bad.al @@ -0,0 +1,13 @@ +codeunit 50103 "Order Confirmation Notifier" +{ + procedure Send(FromName: Text; ToAddress: Text; Subject: Text; Body: Text) + var + Mail: Codeunit Mail; + MailSent: Boolean; + begin + Mail.CreateMessage(FromName, ToAddress, '', Subject, Body, true); + MailSent := Mail.Send(); + if not MailSent then + Message(Mail.GetErrorDesc()); + end; +} diff --git a/microsoft/knowledge/breaking-changes/prefer-email-module.good.al b/microsoft/knowledge/breaking-changes/prefer-email-module.good.al new file mode 100644 index 00000000..c9a1da4d --- /dev/null +++ b/microsoft/knowledge/breaking-changes/prefer-email-module.good.al @@ -0,0 +1,11 @@ +codeunit 50103 "Order Confirmation Notifier" +{ + procedure Send(ToAddress: Text; Subject: Text; Body: Text) + var + Email: Codeunit Email; + EmailMessage: Codeunit "Email Message"; + begin + EmailMessage.Create(ToAddress, Subject, Body, true); + Email.Send(EmailMessage, Enum::"Email Scenario"::Default); + end; +} diff --git a/microsoft/knowledge/breaking-changes/prefer-email-module.md b/microsoft/knowledge/breaking-changes/prefer-email-module.md new file mode 100644 index 00000000..e903ec76 --- /dev/null +++ b/microsoft/knowledge/breaking-changes/prefer-email-module.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: breaking-changes +keywords: [email, codeunit-mail, email-message, email-scenario, email-account, smtp, sending-email] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Send email through the Email module, not Codeunit Mail (397) + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +Older AL code sends email by calling `Codeunit Mail (397)`. Business Central's current extensibility model is a different, richer object set — `Codeunit Email`, table `Email Message`, `enum "Email Scenario"`, and the `Email Account`/`Email Connector` interface (Microsoft 365, Current User, SMTP, or a custom connector). New code built on `Codeunit Mail` inherits its SMTP-era, single-connector assumptions and leaves no Sent/Outbox trail behind. + +## Best Practice + +Build on `Codeunit Email` and table `Email Message`. Route the message through an `Email Scenario` so different document types can use different accounts without the calling code needing to know which account that is, and get a tracked Sent/Outbox/Draft record for free. + +See sample: `prefer-email-module.good.al`. + +## Anti Pattern + +Calling `Codeunit Mail`'s `CreateMessage`/`Send`/`GetErrorDesc`. It still runs, but it is hard-coupled to whatever SMTP setup exists, and leaves no queryable record of what was sent. + +See sample: `prefer-email-module.bad.al`. diff --git a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.bad.al b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.bad.al new file mode 100644 index 00000000..0a13e1c1 --- /dev/null +++ b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.bad.al @@ -0,0 +1,22 @@ +codeunit 50101 "Meter Jnl.-Post" +{ + procedure Post(var MeterJnlLine: Record "Meter Journal Line") + var + MeterLedgEntry: Record "Meter Ledger Entry"; + begin + // Validation, Journal access, and posting all mixed in one routine. + if not Confirm('Post journal lines?') then + exit; + + if MeterJnlLine.FindSet() then + repeat + if MeterJnlLine.Quantity = 0 then + Error('Quantity must not be zero.'); + + MeterLedgEntry.Init(); + MeterLedgEntry.TransferFields(MeterJnlLine); + MeterLedgEntry.Insert(); + MeterJnlLine.Delete(); + until MeterJnlLine.Next() = 0; + end; +} diff --git a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al new file mode 100644 index 00000000..d7384859 --- /dev/null +++ b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al @@ -0,0 +1,32 @@ +codeunit 50101 "Meter Jnl.-Post Line" +{ + procedure PostLine(var MeterJnlLine: Record "Meter Journal Line") + var + MeterLedgEntry: Record "Meter Ledger Entry"; + begin + // Writes exactly one ledger entry; never touches the Journal table. + MeterLedgEntry.Init(); + MeterLedgEntry.TransferFields(MeterJnlLine); + MeterLedgEntry.Insert(); + end; +} + +codeunit 50102 "Meter Jnl.-Post Batch" +{ + procedure PostBatch(var MeterJnlLine: Record "Meter Journal Line") + var + CheckLine: Codeunit "Meter Jnl.-Check Line"; + PostLine: Codeunit "Meter Jnl.-Post Line"; + begin + if MeterJnlLine.FindSet() then + repeat + CheckLine.CheckLine(MeterJnlLine); + until MeterJnlLine.Next() = 0; + + if MeterJnlLine.FindSet() then + repeat + PostLine.PostLine(MeterJnlLine); + MeterJnlLine.Delete(); + until MeterJnlLine.Next() = 0; + end; +} diff --git a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md new file mode 100644 index 00000000..9dd19d53 --- /dev/null +++ b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: data-modeling +keywords: [posting-routine, check-line, post-line, post-batch, companion-codeunit, yes-no-wrapper, journal] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Split posting routines into Check Line / Post Line / Post Batch + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +Every journal-based posting routine in Business Central is split across three companion codeunits with distinct, non-overlapping responsibilities: `Check Line` validates one line, `Post Line` writes exactly one line to the ledger, and `Post Batch` loops both across the journal. A document posting routine (posting one document at a time) calls `Post Line` directly and skips `Post Batch`. A new posting routine that blurs this split either misses functionality other code expects to call directly, or exposes an interaction surface it shouldn't. + +## Best Practice + +`Check Line` reads setup/dimension data only on its first call and shows no UI beyond errors. `Post Line` only operates on the record passed to it — never the Journal table — so it can be called directly by other posting code, including a document posting routine. `Post Batch` is the only one of the three that reads and updates the Journal table, and it is the only one invoked from the Post action on a journal page. A `-Post` document codeunit is never called directly from a page; a page calls a `-Post (Yes/No)` confirmation wrapper instead, so the same `-Post` codeunit can also run unattended from a batch-posting report. + +See sample: `check-post-line-batch-pattern.good.al`. + +## Anti Pattern + +A single monolithic posting codeunit that reads the Journal table, validates lines, writes ledger entries, and shows confirmation dialogs all in one procedure. It cannot be reused by another posting routine without fabricating journal records, and it cannot run unattended because it insists on user interaction. + +See sample: `check-post-line-batch-pattern.bad.al`. diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.bad.al b/microsoft/knowledge/data-modeling/dimension-management-wiring.bad.al new file mode 100644 index 00000000..8e3bce48 --- /dev/null +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.bad.al @@ -0,0 +1,13 @@ +table 50100 "Course" +{ + fields + { + field(1; "No."; Code[20]) { } + field(10; "Global Dimension 1 Code"; Code[20]) + { + // No CaptionClass, no OnValidate call into DimensionManagement. + // Accepts any value; never becomes a Default Dimension record. + TableRelation = "Dimension Value".Code; + } + } +} diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al b/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al new file mode 100644 index 00000000..2af97b84 --- /dev/null +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al @@ -0,0 +1,28 @@ +table 50100 "Course" +{ + fields + { + field(1; "No."; Code[20]) { } + field(10; "Global Dimension 1 Code"; Code[20]) + { + CaptionClass = '1,1,1'; + TableRelation = "Dimension Value".Code where( + "Global Dimension No." = const(1), Blocked = const(false)); + + trigger OnValidate() + var + DimMgt: Codeunit DimensionManagement; + begin + DimMgt.ValidateShortcutDimCode(1, "Global Dimension 1 Code"); + DimMgt.SaveDefaultDim(Database::Course, "No.", FieldNo("Global Dimension 1 Code"), "Global Dimension 1 Code"); + end; + } + } + + trigger OnDelete() + var + DimMgt: Codeunit DimensionManagement; + begin + DimMgt.DeleteDefaultDim(Database::Course, "No."); + end; +} diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.md b/microsoft/knowledge/data-modeling/dimension-management-wiring.md new file mode 100644 index 00000000..c939952b --- /dev/null +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: data-modeling +keywords: [dimensions, dimensionmanagement, global-dimension, shortcut-dimension, default-dimension, validateshortcutdimcode, createdim] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Wire dimension support through DimensionManagement, not ad hoc fields + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +Adding dimension support to a custom master or document table is not just a matter of adding a `Code[20]` field. Business Central expects a specific set of hooks into `Codeunit "Dimension Management"` so a dimension value is validated, persisted as a Default Dimension record, and flows through to transactions the same way it does for every standard table. Skipping any one hook produces a field that looks correct in the designer but silently fails to save, validate, or carry through to postings. + +## Best Practice + +A master table should validate its dimension fields through `ValidateShortcutDimCode` and `SaveDefaultDim`, and create/delete the matching Default Dimension records in `OnInsert`/`OnDelete`. A document table should add Shortcut Dimension fields validated the same way, and call `CreateDim` to pull dimension values from the related master record whenever the field that attaches the document to that master changes. + +See sample: `dimension-management-wiring.good.al`. + +## Anti Pattern + +Adding a dimension-looking field with only a `TableRelation` to Dimension Value, and no call into `DimensionManagement` at all. The field accepts input but never becomes a real Default Dimension record, so it does not validate against blocked values and does not flow into postings. + +See sample: `dimension-management-wiring.bad.al`. diff --git a/microsoft/knowledge/data-modeling/do-not-change-primary-key-field-list.md b/microsoft/knowledge/data-modeling/do-not-change-primary-key-field-list.md new file mode 100644 index 00000000..ffece5dc --- /dev/null +++ b/microsoft/knowledge/data-modeling/do-not-change-primary-key-field-list.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: data-modeling +keywords: [primary-key, clustered-key, table-design, appsource, breaking-change, schema-upgrade] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Never Change a Published Table's Primary or Clustered Key Field List + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +Once a table has shipped — to AppSource, or to any customer environment that has already upgraded onto it — its primary key, and any other key marked `Clustered = true`, is frozen. This includes adding a field to the key, not only removing or reordering one: Business Central identifies existing rows by their key value, so any change to which fields compose that key invalidates every row already stored under the old shape, and the platform's upgrade validation rejects it outright (`AS0009`). This is easy to trip over because it doesn't look like the well-known "don't delete a field" mistake — the field being added is often brand new, and folding a new discriminating dimension straight into the existing key feels like the natural, un-denormalized way to model it. On an unpublished table that is correct; on a published one it is a breaking schema change regardless of which direction the field list changed, and there is no in-place fix once the upgrade is rejected, only reverting the key to its published shape. + +## Best Practice + +Leave a published table's key exactly as shipped. Model a new discriminating dimension as a separate table with its own key instead of adding a field to the existing key, and branch orchestration code by the new dimension rather than filtering one shared table on an extra key field. + +See sample: `do-not-change-primary-key.good.al`. + +## Anti Pattern + +Adding a field to a published table's primary or clustered key to distinguish a new case. This fails AppSource validation or any customer upgrade with `AS0009` as soon as rows already exist under the old key shape, whether the field is being added, removed, or reordered. + +See sample: `do-not-change-primary-key.bad.al`. diff --git a/microsoft/knowledge/data-modeling/do-not-change-primary-key.bad.al b/microsoft/knowledge/data-modeling/do-not-change-primary-key.bad.al new file mode 100644 index 00000000..cc52ae2f --- /dev/null +++ b/microsoft/knowledge/data-modeling/do-not-change-primary-key.bad.al @@ -0,0 +1,14 @@ +table 50100 "Period Stats" +{ + fields + { + field(1; "Period Start"; Date) { } + field(2; Flow; Enum "Some Flow") { } + } + keys + { + // Table already shipped with key(PK; "Period Start"). + // Adding Flow here breaks every upgrade with AS0009. + key(PK; Flow, "Period Start") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/data-modeling/do-not-change-primary-key.good.al b/microsoft/knowledge/data-modeling/do-not-change-primary-key.good.al new file mode 100644 index 00000000..47db11da --- /dev/null +++ b/microsoft/knowledge/data-modeling/do-not-change-primary-key.good.al @@ -0,0 +1,24 @@ +table 50100 "Period Stats" +{ + fields + { + field(1; "Period Start"; Date) { } + } + keys + { + key(PK; "Period Start") { Clustered = true; } + } +} + +table 50101 "Period Stats By Flow" +{ + fields + { + field(1; "Period Start"; Date) { } + field(2; Flow; Enum "Some Flow") { } + } + keys + { + key(PK; "Period Start") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/data-modeling/testfield-required-setup-field-before-use.md b/microsoft/knowledge/data-modeling/testfield-required-setup-field-before-use.md new file mode 100644 index 00000000..2def2191 --- /dev/null +++ b/microsoft/knowledge/data-modeling/testfield-required-setup-field-before-use.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: data-modeling +keywords: [testfield, setup-table, configuration, mandatory-field, silent-fallback, correctness] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# TestField a Setup-Table Value Before Using It in a Correctness-Critical Branch + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +A procedure that reads a field from a setup or configuration table inside a branch where the surrounding logic has already decided that value is required needs to guard against it being blank. A common anti-pattern silently treats "blank" as "feature not wanted": it checks the field for emptiness and falls through to a default instead of raising an error. `Get()` succeeding on the setup record only proves the record exists, not that the specific field was ever configured, so the fallback path makes a missing configuration indistinguishable from a deliberate one — and the wrong outcome, especially in financial, tax, or compliance postings, surfaces silently rather than as a crash a tester would notice. + +## Best Practice + +Once a business rule has decided that a setup-table field's value is required for a branch to behave correctly, call `TestField` on it before use, even though a plain read would "work" by returning a blank or zero without erroring. Write a test that blanks the setup field and asserts the resulting error, so the guard itself is verified rather than merely present. + +See sample: `testfield-required-setup-field.good.al`. + +## Anti Pattern + +Reading a required setup-table field behind a presence check that falls through to a default value instead of erroring. This looks defensive because it never crashes, but it converts "administrator forgot to configure this" into "system silently did something else" — worse than a hard failure, because nobody is told anything went wrong. + +See sample: `testfield-required-setup-field.bad.al`. diff --git a/microsoft/knowledge/data-modeling/testfield-required-setup-field.bad.al b/microsoft/knowledge/data-modeling/testfield-required-setup-field.bad.al new file mode 100644 index 00000000..2ee7bc1c --- /dev/null +++ b/microsoft/knowledge/data-modeling/testfield-required-setup-field.bad.al @@ -0,0 +1,12 @@ +codeunit 50100 "Tax Posting Helper" +{ + procedure GetTaxAccount(var Setup: Record "Sales & Receivables Setup"): Code[20] + var + DefaultTaxAccountTxt: Label 'DEFAULT-TAX'; + begin + Setup.Get(); + if Setup."Tax Account No." <> '' then + exit(Setup."Tax Account No."); + exit(DefaultTaxAccountTxt); + end; +} diff --git a/microsoft/knowledge/data-modeling/testfield-required-setup-field.good.al b/microsoft/knowledge/data-modeling/testfield-required-setup-field.good.al new file mode 100644 index 00000000..d35db264 --- /dev/null +++ b/microsoft/knowledge/data-modeling/testfield-required-setup-field.good.al @@ -0,0 +1,9 @@ +codeunit 50100 "Tax Posting Helper" +{ + procedure GetTaxAccount(var Setup: Record "Sales & Receivables Setup"): Code[20] + begin + Setup.Get(); + Setup.TestField("Tax Account No."); + exit(Setup."Tax Account No."); + end; +} diff --git a/microsoft/knowledge/performance/document-report-word-layout.bad.al b/microsoft/knowledge/performance/document-report-word-layout.bad.al new file mode 100644 index 00000000..0da274f4 --- /dev/null +++ b/microsoft/knowledge/performance/document-report-word-layout.bad.al @@ -0,0 +1,16 @@ +report 50105 "Sales Quote Confirmation" +{ + UsageCategory = Documents; + ApplicationArea = All; + + rendering + { + layout(RDLC) + { + Type = RDLC; + LayoutFile = './Layouts/SalesQuoteConfirmation.rdl'; + } + } + + DefaultRenderingLayout = RDLC; +} diff --git a/microsoft/knowledge/performance/document-report-word-layout.good.al b/microsoft/knowledge/performance/document-report-word-layout.good.al new file mode 100644 index 00000000..f9e5e243 --- /dev/null +++ b/microsoft/knowledge/performance/document-report-word-layout.good.al @@ -0,0 +1,17 @@ +report 50105 "Sales Quote Confirmation" +{ + UsageCategory = Documents; + ApplicationArea = All; + + rendering + { + layout(Word) + { + Type = Word; + LayoutFile = './Layouts/SalesQuoteConfirmation.docx'; + Caption = 'Word Layout'; + } + } + + DefaultRenderingLayout = Word; +} diff --git a/microsoft/knowledge/performance/document-report-word-layout.md b/microsoft/knowledge/performance/document-report-word-layout.md new file mode 100644 index 00000000..9ca9e25d --- /dev/null +++ b/microsoft/knowledge/performance/document-report-word-layout.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: performance +keywords: [report-layout, word-layout, rdlc, document-report, sandbox-app-domain, rendering] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Document reports should default to a Word layout, not RDLC + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +A document report — an invoice, statement, order confirmation, or any report meant to be printed, emailed, or exported as a single-record document — should default to a `Word` rendering layout rather than `RDLC`. RDLC layouts run in a sandboxed app domain that only lives for the current report invocation, which is slower for UI-related actions such as emailing the resulting document, than a Word layout, which is not subject to that sandbox constraint. This does not apply to every report: tabular/list reports with heavy aggregation or calculated columns are still often a better fit for RDLC or Excel. + +## Best Practice + +Set `DefaultRenderingLayout = Word` and define a `Word` layout for reports that represent one structured document per record. Reserve RDLC (or Excel) for reports that represent a data listing rather than a document. + +See sample: `document-report-word-layout.good.al`. + +## Anti Pattern + +Defaulting a document report's layout to RDLC out of habit or because a template happened to use it. This inherits RDLC's sandboxed-app-domain performance cost with no benefit tied to the report's actual content or calculation needs. + +See sample: `document-report-word-layout.bad.al`. diff --git a/microsoft/knowledge/style/al-build-output-must-not-pollute-project-root.md b/microsoft/knowledge/style/al-build-output-must-not-pollute-project-root.md new file mode 100644 index 00000000..eb8c6a39 --- /dev/null +++ b/microsoft/knowledge/style/al-build-output-must-not-pollute-project-root.md @@ -0,0 +1,24 @@ +--- +bc-version: [all] +domain: style +keywords: [build, output, alpackages, duplicate, language-server, app-package, project-root, al0197] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Keep AL Build Output Out of the Project Root + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +When an AL project is built, the compiled `.app` file is placed in the project root by default. Over successive builds, multiple `.app` files accumulate there (e.g. one per version). The AL language server, both in the editor and in build tooling, scans the project folder for symbol packages and can load these compiled artefacts alongside the live source files, which produces `AL0197` duplicate-object errors for every object in the project — with messages that point at source lines rather than at the packaged artefact that is the actual duplicate. The errors are not real; they disappear as soon as the stale `.app` files are removed from the root. + +## Best Practice + +Configure the build output path to a dedicated subfolder that is excluded from language server scanning — for example by setting `al.outputPath` to a folder such as `.output` in `.vscode/settings.json`, or by passing an explicit output path to the build tool being used — and add that folder to `.gitignore`. Before treating an `AL0197` "already declared" error as a source code problem, check the project root for stale `.app` files first; adding root `.app` files to `.gitignore` instead of relocating the output path only hides the accumulation rather than fixing it. + +## Anti Pattern + +Letting `.app` files accumulate in the project root across builds, then debugging the resulting `AL0197` duplicate-object errors as if they were a source code defect instead of first checking for stale build artefacts in the root folder. diff --git a/microsoft/knowledge/style/al-identifiers-english.bad.al b/microsoft/knowledge/style/al-identifiers-english.bad.al new file mode 100644 index 00000000..e4d62da5 --- /dev/null +++ b/microsoft/knowledge/style/al-identifiers-english.bad.al @@ -0,0 +1,11 @@ +codeunit 50100 "Sales Amount Calculator" +{ + procedure BeregnTotalbeloeb(var Salgslinje: Record "Sales Line"): Decimal + var + Beloeb: Decimal; + begin + Salgslinje.CalcSums(Amount); + Beloeb := Salgslinje.Amount; + exit(Beloeb); + end; +} diff --git a/microsoft/knowledge/style/al-identifiers-english.good.al b/microsoft/knowledge/style/al-identifiers-english.good.al new file mode 100644 index 00000000..0b17ec18 --- /dev/null +++ b/microsoft/knowledge/style/al-identifiers-english.good.al @@ -0,0 +1,11 @@ +codeunit 50100 "Sales Amount Calculator" +{ + procedure CalculateTotalAmount(var SalesLine: Record "Sales Line"): Decimal + var + TotalAmount: Decimal; + begin + SalesLine.CalcSums(Amount); + TotalAmount := SalesLine.Amount; + exit(TotalAmount); + end; +} diff --git a/microsoft/knowledge/style/al-identifiers-must-be-english.md b/microsoft/knowledge/style/al-identifiers-must-be-english.md new file mode 100644 index 00000000..1b272dee --- /dev/null +++ b/microsoft/knowledge/style/al-identifiers-must-be-english.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: style +keywords: [identifiers, naming, english, captions, translation] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Write AL Identifiers in English Only + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +All AL identifiers — variables, procedures, parameters, fields, object names, enum values, and label identifiers — must be written in English, regardless of the developer's native language. Translations are handled separately through captions, tooltips, and XLIFF files, never by writing Danish, German, or other non-English identifiers directly into AL source code. This is not a stylistic preference: the public `microsoft/BCApps` codebase, maintained by engineers of many nationalities across hundreds of thousands of lines of AL, uses exclusively English identifiers, with all localization handled via caption properties and XLIFF rather than by changing identifier names. On any multi-contributor codebase, non-English identifiers make the code unreadable to contributors who don't share that language. + +## Best Practice + +Write every identifier in English, and translate developer intent rather than transliterating it — when a requirement is described in another language, the resulting variable, procedure, and field names should still read as English. Captions and tooltips may carry target-language text in the source file, with locale translations managed through XLIFF. + +See sample: `al-identifiers-english.good.al`. + +## Anti Pattern + +Using native-language identifiers such as a Danish variable or procedure name in AL source code, relying on the fact that the code still compiles and runs correctly. This makes the code unreadable to non-native-language contributors and mixes localization concerns into source that should stay language-neutral. + +See sample: `al-identifiers-english.bad.al`. diff --git a/microsoft/knowledge/style/binary-choice-must-be-boolean.bad.al b/microsoft/knowledge/style/binary-choice-must-be-boolean.bad.al new file mode 100644 index 00000000..2201ea3d --- /dev/null +++ b/microsoft/knowledge/style/binary-choice-must-be-boolean.bad.al @@ -0,0 +1,22 @@ +table 50100 "Sales Task" +{ + fields + { + field(1; "No."; Code[20]) { } + field(10; Status; Option) + { + OptionMembers = Active,Blocked; + } + } +} + +codeunit 50100 "Sales Task Check" +{ + procedure IsOverdue(DueDate: Date): Integer + begin + // 0 = No, 1 = Yes + if DueDate < Today then + exit(1); + exit(0); + end; +} diff --git a/microsoft/knowledge/style/binary-choice-must-be-boolean.good.al b/microsoft/knowledge/style/binary-choice-must-be-boolean.good.al new file mode 100644 index 00000000..ac62b0f8 --- /dev/null +++ b/microsoft/knowledge/style/binary-choice-must-be-boolean.good.al @@ -0,0 +1,16 @@ +table 50100 "Sales Task" +{ + fields + { + field(1; "No."; Code[20]) { } + field(10; Blocked; Boolean) { } + } +} + +codeunit 50100 "Sales Task Check" +{ + procedure IsOverdue(DueDate: Date): Boolean + begin + exit(DueDate < Today); + end; +} diff --git a/microsoft/knowledge/style/binary-choice-must-be-boolean.md b/microsoft/knowledge/style/binary-choice-must-be-boolean.md new file mode 100644 index 00000000..33f15187 --- /dev/null +++ b/microsoft/knowledge/style/binary-choice-must-be-boolean.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: style +keywords: [boolean, option, yes-no, magic-number, variable-typing, field-typing] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Binary yes/no choices must be typed as Boolean, not Option or Integer + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +When a field or variable represents exactly two states — yes/no, on/off, active/inactive, blocked/not blocked — it should be typed `Boolean`. Modeling that same two-state choice as an `Option`/`Enum` with two members, or as an `Integer` with two magic-number values, adds a layer of indirection a reader has to resolve before understanding the code, and it invites a multi-branch check where a simple `if X then` would do. This is distinct from a genuine multi-value choice with more than two named states, which legitimately calls for `Enum` — the line is the state count. + +## Best Practice + +Type a true two-state field or variable as `Boolean` and branch on it directly. + +See sample: `binary-choice-must-be-boolean.good.al`. + +## Anti Pattern + +Modeling a yes/no choice as an `Option` with two members, or as an `Integer` with magic-number values, forces every caller to remember which value means what and leaves room for a meaningless third value. + +See sample: `binary-choice-must-be-boolean.bad.al`. diff --git a/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.bad.al b/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.bad.al new file mode 100644 index 00000000..77facce7 --- /dev/null +++ b/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.bad.al @@ -0,0 +1,22 @@ +table 50101 "Course" +{ + fields + { + field(1; "No."; Code[20]) { } + // 1 = Beginner, 2..3 = Intermediate, 4+ = Advanced + field(10; Difficulty; Integer) { } + } +} + +codeunit 50100 "Course Level Helper" +{ + procedure LevelText(Difficulty: Integer): Text + begin + case Difficulty of + 1: + exit('Beginner'); + 2, 3: + exit('Intermediate'); + end; + end; +} diff --git a/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.good.al b/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.good.al new file mode 100644 index 00000000..ef0175b8 --- /dev/null +++ b/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.good.al @@ -0,0 +1,17 @@ +enum 50100 "Course Difficulty" +{ + Extensible = true; + + value(0; Beginner) { } + value(1; Intermediate) { } + value(2; Advanced) { } +} + +table 50101 "Course" +{ + fields + { + field(1; "No."; Code[20]) { } + field(10; Difficulty; Enum "Course Difficulty") { } + } +} diff --git a/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md b/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md new file mode 100644 index 00000000..78e0528a --- /dev/null +++ b/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: style +keywords: [enum, option, integer, magic-number, variable-typing, field-typing] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# A fixed set of named choices must use Enum, not a raw Integer + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +When a variable or field can only take on a fixed set of more than two named, mutually exclusive states — a difficulty level, a document type, a processing status — it should be typed as `Enum` (or `Option` when extending an object that still uses the legacy type). Representing that same state as a plain `Integer` and tracking the meaning of each value in a comment or in a developer's head is a magic-number anti-pattern: the compiler cannot catch an out-of-range value, and branches read as opaque numbers instead of names. This is distinct from a true two-state choice, which should be `Boolean` rather than an enumeration — the line is the state count. + +## Best Practice + +Declare an `Enum` with named values and branch on the enum value, not a raw number. + +See sample: `fixed-choice-set-must-use-enum-not-integer.good.al`. + +## Anti Pattern + +Using a plain `Integer` field with the meaning of each value tracked only in a comment pushes the documentation of the states into something the compiler cannot check and a future maintainer cannot rely on. + +See sample: `fixed-choice-set-must-use-enum-not-integer.bad.al`. diff --git a/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.bad.al b/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.bad.al new file mode 100644 index 00000000..755991b7 --- /dev/null +++ b/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.bad.al @@ -0,0 +1,12 @@ +codeunit 50100 "Sales Task Notify" +{ + procedure NotifyUpdate(Count: Integer; CustomerNo: Code[20]) + begin + MESSAGE('%1 records updated.', Count); + + IF NOT CONFIRM('Delete %1?', FALSE, CustomerNo) THEN + EXIT; + + ERROR(STRSUBSTNO('%1 must not be blank.', CustomerNo)); + end; +} diff --git a/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.good.al b/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.good.al new file mode 100644 index 00000000..a8551b41 --- /dev/null +++ b/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.good.al @@ -0,0 +1,12 @@ +codeunit 50100 "Sales Task Notify" +{ + procedure NotifyUpdate(Count: Integer; CustomerNo: Code[20]) + begin + Message('%1 records updated.', Count); + + if not Confirm('Delete %1?', false, CustomerNo) then + exit; + + Error(StrSubstNo('%1 must not be blank.', CustomerNo)); + end; +} diff --git a/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md b/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md new file mode 100644 index 00000000..42c45104 --- /dev/null +++ b/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: style +keywords: [casing, intrinsic-function, built-in-function, message, error, confirm, strsubstno, legacy, c-al, pascalcase] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Intrinsic AL function calls use modern casing, not legacy ALL-CAPS + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +AL is case-insensitive, so `MESSAGE(...)`, `ERROR(...)`, `CONFIRM(...)`, and `STRSUBSTNO(...)` compile and run identically to `Message(...)`, `Error(...)`, `Confirm(...)`, and `StrSubstNo(...)`. Modern AL — the VS Code tooling's default formatter output, Microsoft's own current samples, and current reference codebases — writes intrinsic/built-in function calls in the casing Microsoft assigns to the function's declared name, typically PascalCase. ALL-CAPS calls are a holdover from classic C/AL and signal code that has not been modernized, even though it compiles and runs correctly. Reserved keywords such as `if`, `begin`, and `for` are a separate, already-tooled concern; intrinsic function names are identifiers, not keywords, so that tooling does not catch ALL-CAPS intrinsic function calls. + +## Best Practice + +Call intrinsic functions in their modern, PascalCase form. + +See sample: `intrinsic-al-functions-must-use-modern-casing.good.al`. + +## Anti Pattern + +ALL-CAPS intrinsic function calls trip no compiler error, but they are a reliable signal that a code block was copied from old C/AL material or outdated training content rather than written against current AL conventions. + +See sample: `intrinsic-al-functions-must-use-modern-casing.bad.al`. diff --git a/microsoft/knowledge/style/namespace-must-be-verified-from-source.bad.al b/microsoft/knowledge/style/namespace-must-be-verified-from-source.bad.al new file mode 100644 index 00000000..f8006a1d --- /dev/null +++ b/microsoft/knowledge/style/namespace-must-be-verified-from-source.bad.al @@ -0,0 +1,13 @@ +namespace Contoso.Extensions; + +using System.Performance; // guessed; never verified against the source file + +codeunit 50100 "Tooling Extension" +{ + procedure Run() + var + ToolingPage: Page "Some Tooling Page"; // resolves in a local build, fails in VS Code + begin + ToolingPage.Run(); + end; +} diff --git a/microsoft/knowledge/style/namespace-must-be-verified-from-source.good.al b/microsoft/knowledge/style/namespace-must-be-verified-from-source.good.al new file mode 100644 index 00000000..2a8e34a3 --- /dev/null +++ b/microsoft/knowledge/style/namespace-must-be-verified-from-source.good.al @@ -0,0 +1,16 @@ +// Verified by reading line 1 of the source file for "Some Tooling Page": +// namespace System.Tooling; + +namespace Contoso.Extensions; + +using System.Tooling; + +codeunit 50100 "Tooling Extension" +{ + procedure Run() + var + ToolingPage: Page "Some Tooling Page"; + begin + ToolingPage.Run(); + end; +} diff --git a/microsoft/knowledge/style/namespace-must-be-verified-from-source.md b/microsoft/knowledge/style/namespace-must-be-verified-from-source.md new file mode 100644 index 00000000..d38fba5f --- /dev/null +++ b/microsoft/knowledge/style/namespace-must-be-verified-from-source.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: style +keywords: [namespace, verification, source-of-truth, using-statement] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Verify a namespace from the object's own source file, never by inference + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +Since Business Central 2024 release wave 1, Microsoft's own objects are organized under a deep `Microsoft.*` namespace tree that has been renamed and restructured repeatedly. Guessing a namespace from an object's name, from an older codebase, or from general familiarity produces a `using` statement that can look plausible, compile in isolation, and still resolve to the wrong object or fail in the AL Language Server that VS Code actually uses to report errors. The only reliable source for an object's namespace is line one of that object's own source file. + +## Best Practice + +Locate the object's source file, read its `namespace` declaration on line one, and copy that exact value into the consuming file's `using` statement. + +See sample: `namespace-must-be-verified-from-source.good.al`. + +## Anti Pattern + +Writing a `using` statement from memory, from an incomplete path, or from a plausible-looking guess. It can compile in one build environment while still failing to resolve in VS Code, because the two use different namespace resolution. + +See sample: `namespace-must-be-verified-from-source.bad.al`. diff --git a/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.bad.al b/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.bad.al new file mode 100644 index 00000000..5c9dbe78 --- /dev/null +++ b/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.bad.al @@ -0,0 +1,15 @@ +codeunit 50100 "Customer Lookup" +{ + procedure GetCustomerName(CustomerNo: Code[20]; var Name: Text[100]) + var + Customer: Record Customer; + begin + if Customer.Get(CustomerNo) then + Name := Customer.Name; + end; + + procedure Sample() + begin + GetCustomerName('10000', 'placeholder'); // compile error: literal is not addressable + end; +} diff --git a/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.good.al b/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.good.al new file mode 100644 index 00000000..3c20da1d --- /dev/null +++ b/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.good.al @@ -0,0 +1,17 @@ +codeunit 50100 "Customer Lookup" +{ + procedure GetCustomerName(CustomerNo: Code[20]; var Name: Text[100]) + var + Customer: Record Customer; + begin + if Customer.Get(CustomerNo) then + Name := Customer.Name; + end; + + procedure Sample() + var + CustName: Text[100]; + begin + GetCustomerName('10000', CustName); + end; +} diff --git a/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.md b/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.md new file mode 100644 index 00000000..485e5ac2 --- /dev/null +++ b/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: style +keywords: [var-parameter, by-reference, pass-by-reference, literal, constant, compile-error, procedure-call] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# A `var` parameter must be called with a variable, never a literal or expression + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +When a procedure declares a parameter with `var`, that parameter is passed by reference: the callee writes back into the caller's own memory location. This means the argument at the call site must be an actual variable, something with an address. A string literal, a numeric constant, or a computed expression has no address to write back to, so passing one to a `var` parameter fails to compile. Before writing a call, check the callee's signature for `var` on each parameter position being supplied a literal or expression — if present, a variable declared in the caller's own scope must be used instead. + +## Best Practice + +Declare a variable in the caller's scope and pass it to the `var` parameter. + +See sample: `var-parameters-require-an-addressable-variable.good.al`. + +## Anti Pattern + +Passing a literal or a computed expression to a `var` parameter position fails to compile, because neither has an address the callee can write back to. + +See sample: `var-parameters-require-an-addressable-variable.bad.al`. diff --git a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.bad.al b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.bad.al new file mode 100644 index 00000000..34c13e48 --- /dev/null +++ b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.bad.al @@ -0,0 +1,14 @@ +[Test] +procedure PostsSalesOrderForCashCustomer() +var + Customer: Record Customer; + SalesHeader: Record "Sales Header"; +begin + // Assumes a 'CASH' customer already exists in the environment — + // fails on any database where it doesn't. + Customer.Get('CASH'); + LibrarySales.CreateSalesHeader( + SalesHeader, SalesHeader."Document Type"::Order, Customer."No."); + + // ... add lines, post, assert ... +end; diff --git a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.good.al b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.good.al new file mode 100644 index 00000000..d2ac678d --- /dev/null +++ b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.good.al @@ -0,0 +1,13 @@ +[Test] +procedure PostsSalesOrderForRandomCustomer() +var + Customer: Record Customer; + SalesHeader: Record "Sales Header"; +begin + // Random, collision-free customer — no assumption about what exists. + LibrarySales.CreateCustomer(Customer); + LibrarySales.CreateSalesHeader( + SalesHeader, SalesHeader."Document Type"::Order, Customer."No."); + + // ... add lines, post, assert ... +end; diff --git a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md new file mode 100644 index 00000000..b6ad3007 --- /dev/null +++ b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: testing +keywords: [testing, test-data, random, library, any] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Generate Test Data Programmatically, Never Assume Existing Records + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +An AL test suite should assume an empty database. Test data must be created programmatically inside the test rather than assuming a specific code, number, or name already exists in the environment — a hardcoded lookup against an assumed-existing record makes the test fail for reasons unrelated to the code under test. Every mandatory field on a created record also needs a value that respects its declared length; a partial setup that merely passes validation is not sufficient. + +## Best Practice + +Use the standard library codeunits (`Library - ERM`, `Library - Inventory`, `Library - Sales`, `Library - Utility`) to create records with collision-free random values, and fill every mandatory field with randomized, correctly-sized data. Reserve hardcoded values for tests that validate an external contract itself — a fixed JSON schema, an EDIFACT message, a counterparty code — where the hardcoded value documents the specification rather than arbitrary test logic. + +See sample: `test-data-must-be-random-and-complete.good.al`. + +## Anti Pattern + +Looking up a record assumed to already exist (a hardcoded payment method or customer number) instead of creating it, or leaving mandatory fields empty or underfilled because validation happens to allow it. + +See sample: `test-data-must-be-random-and-complete.bad.al`. diff --git a/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.bad.al b/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.bad.al new file mode 100644 index 00000000..8e25036d --- /dev/null +++ b/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.bad.al @@ -0,0 +1,24 @@ +page 50100 "Item Availability API" +{ + PageType = API; + APIPublisher = 'contoso'; + APIGroup = 'inventory'; + APIVersion = 'v1.0'; + EntityName = 'itemAvailability'; + EntitySetName = 'itemAvailabilities'; + SourceTable = Item; + + layout + { + area(content) + { + repeater(General) + { + field(itemNo; Rec."No.") { } + field(quantityOnHand; Rec.Inventory) { } + // No OnAfterGetRecord CalcFields — Inventory is a FlowField + // and returns 0 to every consumer. + } + } + } +} diff --git a/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.good.al b/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.good.al new file mode 100644 index 00000000..892be42c --- /dev/null +++ b/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.good.al @@ -0,0 +1,27 @@ +page 50100 "Item Availability API" +{ + PageType = API; + APIPublisher = 'contoso'; + APIGroup = 'inventory'; + APIVersion = 'v1.0'; + EntityName = 'itemAvailability'; + EntitySetName = 'itemAvailabilities'; + SourceTable = Item; + + layout + { + area(content) + { + repeater(General) + { + field(itemNo; Rec."No.") { } + field(quantityOnHand; Rec.Inventory) { } + } + } + } + + trigger OnAfterGetRecord() + begin + Rec.CalcFields(Inventory); + end; +} diff --git a/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.md b/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.md new file mode 100644 index 00000000..10044f3e --- /dev/null +++ b/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: web-services +keywords: [api-page, flowfield, calcfields, odata] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Explicitly Calculate FlowFields on API Pages + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +FlowFields are not stored in the database — Business Central computes them on demand. Regular pages trigger that calculation automatically while rendering, but API pages do not. A FlowField referenced in an API page's layout returns an empty value to external consumers unless it is calculated explicitly, producing a silent data gap in OData responses that is easy to miss in review. + +## Best Practice + +Call `CalcFields` for every FlowField referenced in the page layout from the `OnAfterGetRecord` trigger, combining multiple fields into a single call. + +See sample: `api-page-flowfields-must-be-calcfields.good.al`. + +## Anti Pattern + +Relying on the implicit calculation that regular pages perform. Any FlowField left out of the `CalcFields` call returns a blank value to every API consumer with no visible error. + +See sample: `api-page-flowfields-must-be-calcfields.bad.al`. diff --git a/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.bad.al b/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.bad.al new file mode 100644 index 00000000..8b8e240f --- /dev/null +++ b/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.bad.al @@ -0,0 +1,27 @@ +page 50101 "Customer Info API" +{ + PageType = API; + APIPublisher = 'contoso'; + APIGroup = 'sales'; + APIVersion = 'v1.0'; + EntityName = 'customerInfo'; + EntitySetName = 'customerInfos'; + SourceTable = Customer; + ODataKeyFields = "No."; + InsertAllowed = true; + + layout + { + area(content) + { + repeater(General) + { + field(customerNo; Rec."No.") + { + Editable = false; // consumer must supply "No." on POST — this rejects it + } + field(name; Rec.Name) { } + } + } + } +} diff --git a/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.good.al b/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.good.al new file mode 100644 index 00000000..8506ae66 --- /dev/null +++ b/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.good.al @@ -0,0 +1,24 @@ +page 50101 "Customer Info API" +{ + PageType = API; + APIPublisher = 'contoso'; + APIGroup = 'sales'; + APIVersion = 'v1.0'; + EntityName = 'customerInfo'; + EntitySetName = 'customerInfos'; + SourceTable = Customer; + ODataKeyFields = "No."; + InsertAllowed = true; + + layout + { + area(content) + { + repeater(General) + { + field(customerNo; Rec."No.") { } + field(name; Rec.Name) { } + } + } + } +} diff --git a/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.md b/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.md new file mode 100644 index 00000000..aa83cc1a --- /dev/null +++ b/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: web-services +keywords: [api-page, key-fields, editable, insert, odata] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Keep Consumer-Provided Key Fields Editable on API Pages + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +A field listed in `ODataKeyFields` cannot have `Editable = false` when the API page allows inserts and the field's value must be supplied by the caller. Marking it read-only removes the field from the OData write schema, so a POST that includes it is rejected as an unknown property. This only applies to keys the consumer must supply — a system-generated key such as `SystemId` is a valid exception, since Business Central assigns its value automatically on insert. + +## Best Practice + +Leave every consumer-supplied key field referenced in `ODataKeyFields` without `Editable = false` on pages where `InsertAllowed = true`, so the OData layer accepts it as a writable property on POST. + +See sample: `api-page-key-fields-must-be-editable-on-insert.good.al`. + +## Anti Pattern + +Marking a consumer-provided key field `Editable = false`, out of habit or for perceived safety. This silently breaks create operations with a generic `BadRequest` instead of a clear validation error. + +See sample: `api-page-key-fields-must-be-editable-on-insert.bad.al`. diff --git a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.bad.al b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.bad.al new file mode 100644 index 00000000..9938612d --- /dev/null +++ b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.bad.al @@ -0,0 +1,24 @@ +page 50102 "Project Task API" +{ + PageType = API; + APIPublisher = 'contoso'; + APIGroup = 'jobs'; + APIVersion = 'v1.0'; + EntityName = 'projectTask'; + EntitySetName = 'projectTasks'; + SourceTable = "Project Task"; + + layout + { + area(content) + { + repeater(General) + { + // "Remaining Hours" is a stored field, set inside the + // OnValidate of "Budgeted Hours" — it goes stale whenever + // "Hours Used" changes through any other path. + field(remainingHours; Rec."Remaining Hours") { } + } + } + } +} diff --git a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al new file mode 100644 index 00000000..c26364b7 --- /dev/null +++ b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al @@ -0,0 +1,31 @@ +page 50102 "Project Task API" +{ + PageType = API; + APIPublisher = 'contoso'; + APIGroup = 'jobs'; + APIVersion = 'v1.0'; + EntityName = 'projectTask'; + EntitySetName = 'projectTasks'; + SourceTable = "Project Task"; + + layout + { + area(content) + { + repeater(General) + { + field(remainingHours; RemainingHoursCalc) { } + field(hoursUsed; Rec."Hours Used") { } + } + } + } + + trigger OnAfterGetRecord() + begin + Rec.CalcFields("Hours Used"); + RemainingHoursCalc := Rec."Budgeted Hours" - Rec."Hours Used"; + end; + + var + RemainingHoursCalc: Decimal; +} diff --git a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md new file mode 100644 index 00000000..68cfe159 --- /dev/null +++ b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md @@ -0,0 +1,28 @@ +--- +bc-version: [all] +domain: web-services +keywords: [api-page, derived-fields, exposure, odata] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Recalculate Stored Derived Fields Before Exposing Them on API Pages + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +A stored field whose value is derived from other fields inside an `OnValidate` trigger only updates when that specific trigger fires. If the underlying source data changes through some other path, the stored value goes stale without raising any error. Exposing such a field directly on an API page hands external consumers a snapshot that may be significantly out of date. + +## Best Practice + +Recalculate the derived value in `OnAfterGetRecord` from its authoritative source — typically a FlowField — using a page-level variable, and expose both the recalculated value and the source field so the consumer can verify it. + +See sample: `stored-derived-fields-must-not-be-exposed-directly.good.al`. + +## Anti Pattern + +Exposing the stored field directly via `Rec`, trusting that it was kept in sync by whichever trigger last touched it. + +See sample: `stored-derived-fields-must-not-be-exposed-directly.bad.al`. From cc7c1f2ee077339ba277455f64acdbca94dddee5 Mon Sep 17 00:00:00 2001 From: Michael Dieringer <65093775+MichaelDieringer@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:52:40 +0200 Subject: [PATCH 2/7] Address Jesper Schulz-Wedde's review on PR #156 - Rename 3 articles so their .good.al/.bad.al companion stems match (do-not-change-primary-key, testfield-required-setup-field, al-identifiers-english), fixing the R14 orphan-sample errors. - do-not-change-primary-key.good.al: include Flow in the new table's own primary key so it actually models the discriminating dimension. - al-build-output-must-not-pollute-project-root.md: drop the unsubstantiated AL0197 causal claim and the non-existent al.outputPath setting; reframe as build-artifact hygiene sourced from ALTool --outfolder / al_build outputPath. - prefer-email-module.md: Email Message is Codeunit 8904, not a table; distinguish it from the underlying Sent/Outbox/Draft storage. - file-datatype-saas.md: File.Open/Create/Read/Write fails to compile against a Cloud-scoped project, it does not compile and silently fail at runtime. - namespace-must-be-verified-from-source.md: narrow to "resolve from the referenced object's source or symbols," since source-file line one is not the only authoritative source (symbol packages, comments before the namespace line). - test-data-must-be-random-and-complete.md: drop "assume an empty database" and "collision-free" absolutes; reframe around independence from unrelated business records and reserving explicit values for scenario-defining inputs. - binary-choice-must-be-boolean.md: scope to genuine true/false semantics, not mechanical two-member-enum-to-boolean conversion. - document-report-word-layout.md: scope down to a sourced Microsoft Learn recommendation instead of an unconditional performance guarantee; cite the three Learn pages. - Wire the new articles into their review skills' candidate-selection signals (file-datatype-saas, prefer-email-module, namespace-must-be-verified-from-source, var-parameters-require-an- addressable-variable) so they can actually enter a worklist. Co-Authored-By: Claude Sonnet 5 --- microsoft/knowledge/appsource/file-datatype-saas.md | 2 +- .../knowledge/breaking-changes/prefer-email-module.md | 4 ++-- .../data-modeling/do-not-change-primary-key.good.al | 2 +- ...-key-field-list.md => do-not-change-primary-key.md} | 0 ...before-use.md => testfield-required-setup-field.md} | 0 .../performance/document-report-word-layout.md | 10 ++++++++-- .../al-build-output-must-not-pollute-project-root.md | 10 +++++----- ...rs-must-be-english.md => al-identifiers-english.md} | 0 .../knowledge/style/binary-choice-must-be-boolean.md | 4 ++-- .../style/namespace-must-be-verified-from-source.md | 6 +++--- .../testing/test-data-must-be-random-and-complete.md | 6 ++++-- microsoft/skills/review/al-appsource-review.md | 1 + microsoft/skills/review/al-breaking-changes-review.md | 4 +++- microsoft/skills/review/al-style-review.md | 4 ++-- 14 files changed, 32 insertions(+), 21 deletions(-) rename microsoft/knowledge/data-modeling/{do-not-change-primary-key-field-list.md => do-not-change-primary-key.md} (100%) rename microsoft/knowledge/data-modeling/{testfield-required-setup-field-before-use.md => testfield-required-setup-field.md} (100%) rename microsoft/knowledge/style/{al-identifiers-must-be-english.md => al-identifiers-english.md} (100%) diff --git a/microsoft/knowledge/appsource/file-datatype-saas.md b/microsoft/knowledge/appsource/file-datatype-saas.md index 2c226151..0a601790 100644 --- a/microsoft/knowledge/appsource/file-datatype-saas.md +++ b/microsoft/knowledge/appsource/file-datatype-saas.md @@ -13,7 +13,7 @@ application-area: [all] ## Description -The classic `File` variable type — `Open`/`Create`/`Read`/`Write`/`Close` against a path on the local or server filesystem — only works on-premises, because there is no accessible filesystem in the SaaS/cloud sandbox. Any extension meant to run in Business Central Online must not rely on `File.Open`, `File.Create`, `File.Read`, or `File.Write` for its core functionality: code built this way compiles but fails, or is silently skipped, in the cloud. +The classic `File` variable type — `Open`/`Create`/`Read`/`Write`/`Close` against a path on the local or server filesystem — is scoped OnPrem-only. Code targeting Business Central Online that calls `File.Open`, `File.Create`, `File.Read`, or `File.Write` fails to compile against a Cloud-scoped project; it does not compile successfully and fail or get silently skipped at runtime. Separately, and regardless of the compile-time scoping, no server/local filesystem path is available to an extension actually running in Business Central Online. ## Best Practice diff --git a/microsoft/knowledge/breaking-changes/prefer-email-module.md b/microsoft/knowledge/breaking-changes/prefer-email-module.md index e903ec76..d9137e81 100644 --- a/microsoft/knowledge/breaking-changes/prefer-email-module.md +++ b/microsoft/knowledge/breaking-changes/prefer-email-module.md @@ -13,11 +13,11 @@ application-area: [all] ## Description -Older AL code sends email by calling `Codeunit Mail (397)`. Business Central's current extensibility model is a different, richer object set — `Codeunit Email`, table `Email Message`, `enum "Email Scenario"`, and the `Email Account`/`Email Connector` interface (Microsoft 365, Current User, SMTP, or a custom connector). New code built on `Codeunit Mail` inherits its SMTP-era, single-connector assumptions and leaves no Sent/Outbox trail behind. +Older AL code sends email by calling `Codeunit Mail (397)`. Business Central's current extensibility model is a different, richer object set — `Codeunit Email`, `Codeunit "Email Message"`, `enum "Email Scenario"`, and the `Email Account`/`Email Connector` interface (Microsoft 365, Current User, SMTP, or a custom connector). `Codeunit "Email Message"` is the in-memory object you build the message on; it is not itself the persisted Sent/Outbox/Draft record — that storage is managed separately once the message is queued or sent. New code built on `Codeunit Mail` inherits its SMTP-era, single-connector assumptions and leaves no Sent/Outbox trail behind. ## Best Practice -Build on `Codeunit Email` and table `Email Message`. Route the message through an `Email Scenario` so different document types can use different accounts without the calling code needing to know which account that is, and get a tracked Sent/Outbox/Draft record for free. +Build on `Codeunit Email` and `Codeunit "Email Message"`. Route the message through an `Email Scenario` so different document types can use different accounts without the calling code needing to know which account that is, and get a tracked Sent/Outbox/Draft record for free. See sample: `prefer-email-module.good.al`. diff --git a/microsoft/knowledge/data-modeling/do-not-change-primary-key.good.al b/microsoft/knowledge/data-modeling/do-not-change-primary-key.good.al index 47db11da..4739c49b 100644 --- a/microsoft/knowledge/data-modeling/do-not-change-primary-key.good.al +++ b/microsoft/knowledge/data-modeling/do-not-change-primary-key.good.al @@ -19,6 +19,6 @@ table 50101 "Period Stats By Flow" } keys { - key(PK; "Period Start") { Clustered = true; } + key(PK; Flow, "Period Start") { Clustered = true; } } } diff --git a/microsoft/knowledge/data-modeling/do-not-change-primary-key-field-list.md b/microsoft/knowledge/data-modeling/do-not-change-primary-key.md similarity index 100% rename from microsoft/knowledge/data-modeling/do-not-change-primary-key-field-list.md rename to microsoft/knowledge/data-modeling/do-not-change-primary-key.md diff --git a/microsoft/knowledge/data-modeling/testfield-required-setup-field-before-use.md b/microsoft/knowledge/data-modeling/testfield-required-setup-field.md similarity index 100% rename from microsoft/knowledge/data-modeling/testfield-required-setup-field-before-use.md rename to microsoft/knowledge/data-modeling/testfield-required-setup-field.md diff --git a/microsoft/knowledge/performance/document-report-word-layout.md b/microsoft/knowledge/performance/document-report-word-layout.md index 9ca9e25d..158d7218 100644 --- a/microsoft/knowledge/performance/document-report-word-layout.md +++ b/microsoft/knowledge/performance/document-report-word-layout.md @@ -13,11 +13,17 @@ application-area: [all] ## Description -A document report — an invoice, statement, order confirmation, or any report meant to be printed, emailed, or exported as a single-record document — should default to a `Word` rendering layout rather than `RDLC`. RDLC layouts run in a sandboxed app domain that only lives for the current report invocation, which is slower for UI-related actions such as emailing the resulting document, than a Word layout, which is not subject to that sandbox constraint. This does not apply to every report: tabular/list reports with heavy aggregation or calculated columns are still often a better fit for RDLC or Excel. +For a document report — an invoice, statement, order confirmation, or any report meant to be printed, emailed, or exported as a single-record document — Microsoft's own guidance recommends a `Word` rendering layout over `RDLC`: "RDL layouts can result in slower performance with document reports, regarding actions that are related to the user interface (for example, like sending emails) compared to Word layouts," and "we recommend that you design Word layouts instead of RDL" for this report shape (see Sources). This is a documented recommendation, not a universal guarantee that Word outperforms RDLC for every workload, and it does not apply to every report: tabular/list reports with heavy aggregation or calculated columns are still often a better fit for RDLC or Excel. ## Best Practice -Set `DefaultRenderingLayout = Word` and define a `Word` layout for reports that represent one structured document per record. Reserve RDLC (or Excel) for reports that represent a data listing rather than a document. +For document reports, prefer a `Word` layout (`DefaultRenderingLayout = Word`) over RDLC unless specific layout requirements favor RDLC. Reserve RDLC (or Excel) for reports that represent a data listing rather than a document. + +## Sources + +- [Report Design Overview](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-report-design-overview) +- [Creating an RDL layout report](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-report-performance) +- [Troubleshooting reports / Report performance](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-reports-troubleshooting) See sample: `document-report-word-layout.good.al`. diff --git a/microsoft/knowledge/style/al-build-output-must-not-pollute-project-root.md b/microsoft/knowledge/style/al-build-output-must-not-pollute-project-root.md index eb8c6a39..4e5a57a7 100644 --- a/microsoft/knowledge/style/al-build-output-must-not-pollute-project-root.md +++ b/microsoft/knowledge/style/al-build-output-must-not-pollute-project-root.md @@ -1,24 +1,24 @@ --- bc-version: [all] domain: style -keywords: [build, output, alpackages, duplicate, language-server, app-package, project-root, al0197] +keywords: [build, output, alpackages, artifact-hygiene, outfolder, project-root] technologies: [al] countries: [w1] application-area: [all] --- -# Keep AL Build Output Out of the Project Root +# Write AL Build Artifacts to an Intentional Output Location > Contributions welcome — open a PR to refine or extend this article. ## Description -When an AL project is built, the compiled `.app` file is placed in the project root by default. Over successive builds, multiple `.app` files accumulate there (e.g. one per version). The AL language server, both in the editor and in build tooling, scans the project folder for symbol packages and can load these compiled artefacts alongside the live source files, which produces `AL0197` duplicate-object errors for every object in the project — with messages that point at source lines rather than at the packaged artefact that is the actual duplicate. The errors are not real; they disappear as soon as the stale `.app` files are removed from the root. +An AL project's compiled `.app` file can be written to the project root by default, and current tooling explicitly supports choosing a different destination instead — `ALTool`'s `--outfolder` option and the `al_build` agent tool's `outputPath` parameter both exist for this. The problem this rule addresses is not that root-level output is technically invalid; it is agents leaving generated `.app` files scattered through arbitrary source locations, or treating a compiled artefact as if it were part of the source tree (committing it, editing around it, referencing it as a dependency by hand). ## Best Practice -Configure the build output path to a dedicated subfolder that is excluded from language server scanning — for example by setting `al.outputPath` to a folder such as `.output` in `.vscode/settings.json`, or by passing an explicit output path to the build tool being used — and add that folder to `.gitignore`. Before treating an `AL0197` "already declared" error as a source code problem, check the project root for stale `.app` files first; adding root `.app` files to `.gitignore` instead of relocating the output path only hides the accumulation rather than fixing it. +Write build artifacts to a deliberate, dedicated output location — configured via the build tool actually in use (e.g. `ALTool --outfolder`, or an explicit `outputPath` on the agent build tool) — and add that folder to `.gitignore`. Treat a compiled `.app` as a build artifact, never as a source file to commit or hand-edit around. ## Anti Pattern -Letting `.app` files accumulate in the project root across builds, then debugging the resulting `AL0197` duplicate-object errors as if they were a source code defect instead of first checking for stale build artefacts in the root folder. +Letting `.app` files accumulate in arbitrary or unversioned locations without a deliberate output path, or committing compiled artefacts into source control alongside the AL files that produced them. diff --git a/microsoft/knowledge/style/al-identifiers-must-be-english.md b/microsoft/knowledge/style/al-identifiers-english.md similarity index 100% rename from microsoft/knowledge/style/al-identifiers-must-be-english.md rename to microsoft/knowledge/style/al-identifiers-english.md diff --git a/microsoft/knowledge/style/binary-choice-must-be-boolean.md b/microsoft/knowledge/style/binary-choice-must-be-boolean.md index 33f15187..ad82146c 100644 --- a/microsoft/knowledge/style/binary-choice-must-be-boolean.md +++ b/microsoft/knowledge/style/binary-choice-must-be-boolean.md @@ -13,11 +13,11 @@ application-area: [all] ## Description -When a field or variable represents exactly two states — yes/no, on/off, active/inactive, blocked/not blocked — it should be typed `Boolean`. Modeling that same two-state choice as an `Option`/`Enum` with two members, or as an `Integer` with two magic-number values, adds a layer of indirection a reader has to resolve before understanding the code, and it invites a multi-branch check where a simple `if X then` would do. This is distinct from a genuine multi-value choice with more than two named states, which legitimately calls for `Enum` — the line is the state count. +When a field or variable represents a genuine true/false state — yes/no, on/off, active/inactive, blocked/not blocked — it should be typed `Boolean`. Modeling that same predicate as an `Option`/`Enum` with two members, or as an `Integer` with two magic-number values, adds a layer of indirection a reader has to resolve before understanding the code. This is about semantics, not member count: a domain concept that currently has exactly two named alternatives — Inbound/Outbound, Debit/Credit, Buy/Sell — is not automatically a Boolean in disguise. An `Enum` can be the clearer model there, including when it needs to implement an interface, preserve an existing contract, or leave room for a future third value. The distinction is whether the domain is genuinely a stable predicate, not how many states it currently has. ## Best Practice -Type a true two-state field or variable as `Boolean` and branch on it directly. +Type a field or variable as `Boolean` when the domain concept is inherently a true/false state. Do not replace a meaningful two-option domain model with a Boolean solely because it currently has two values. See sample: `binary-choice-must-be-boolean.good.al`. diff --git a/microsoft/knowledge/style/namespace-must-be-verified-from-source.md b/microsoft/knowledge/style/namespace-must-be-verified-from-source.md index d38fba5f..0be019d0 100644 --- a/microsoft/knowledge/style/namespace-must-be-verified-from-source.md +++ b/microsoft/knowledge/style/namespace-must-be-verified-from-source.md @@ -7,17 +7,17 @@ countries: [w1] application-area: [all] --- -# Verify a namespace from the object's own source file, never by inference +# Resolve a namespace from the referenced object's source or symbols, never by guessing > Contributions welcome — open a PR to refine or extend this article. ## Description -Since Business Central 2024 release wave 1, Microsoft's own objects are organized under a deep `Microsoft.*` namespace tree that has been renamed and restructured repeatedly. Guessing a namespace from an object's name, from an older codebase, or from general familiarity produces a `using` statement that can look plausible, compile in isolation, and still resolve to the wrong object or fail in the AL Language Server that VS Code actually uses to report errors. The only reliable source for an object's namespace is line one of that object's own source file. +Since Business Central 2024 release wave 1, Microsoft's own objects are organized under a deep `Microsoft.*` namespace tree that has been renamed and restructured repeatedly. When adding a `using` directive for an existing AL object (table, codeunit, page, enum, interface, etc.), guessing its namespace from the object's name, from an older codebase, or from general familiarity produces a statement that can look plausible, compile in isolation, and still resolve to the wrong object or fail in the AL Language Server that VS Code actually uses to report errors. The reliable sources are the object's own source file (its `namespace` declaration) or, for a dependency without accessible source, its AL symbol package — not the object's name or a remembered convention. ## Best Practice -Locate the object's source file, read its `namespace` declaration on line one, and copy that exact value into the consuming file's `using` statement. +When referencing an existing AL object, resolve its namespace from that object's actual source file or symbol definition — never infer or invent one from its name, functional area, or naming convention. See sample: `namespace-must-be-verified-from-source.good.al`. diff --git a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md index b6ad3007..64e89a4c 100644 --- a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md +++ b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md @@ -13,11 +13,13 @@ application-area: [all] ## Description -An AL test suite should assume an empty database. Test data must be created programmatically inside the test rather than assuming a specific code, number, or name already exists in the environment — a hardcoded lookup against an assumed-existing record makes the test fail for reasons unrelated to the code under test. Every mandatory field on a created record also needs a value that respects its declared length; a partial setup that merely passes validation is not sufficient. +A BC test company normally contains initialized system/setup data — an AL test suite should not assume an empty database, but it must be independent of unrelated business records: create the records and setup it owns rather than looking up a specific code, number, or name assumed to already exist, since that makes the test fail for reasons unrelated to the code under test. Every mandatory field on a created record also needs a value that respects its declared length; a partial setup that merely passes validation is not sufficient. + +Not every value should be generated, though. Incidental fixture data — identifiers, names, descriptions — should generally come from the standard library codeunits rather than be tied to specific existing data. But values that materially define the scenario under test — amounts, quantities, percentages, dates, thresholds, rounding precision — should stay explicit and deliberately chosen, not randomized: a rounding test needs values placed deliberately around the rounding boundary, not a random one that might miss it entirely. ## Best Practice -Use the standard library codeunits (`Library - ERM`, `Library - Inventory`, `Library - Sales`, `Library - Utility`) to create records with collision-free random values, and fill every mandatory field with randomized, correctly-sized data. Reserve hardcoded values for tests that validate an external contract itself — a fixed JSON schema, an EDIFACT message, a counterparty code — where the hardcoded value documents the specification rather than arbitrary test logic. +Use the standard library codeunits (`Library - ERM`, `Library - Inventory`, `Library - Sales`, `Library - Utility`) to generate incidental fixture values — they produce valid, unique-enough data via number series and controlled randomness, not a mathematical collision-free guarantee — and fill every mandatory field with correctly-sized data. Keep values that define the scenario's expected outcome explicit and fixed. Reserve hardcoded values for tests that validate an external contract itself — a fixed JSON schema, an EDIFACT message, a counterparty code — where the hardcoded value documents the specification rather than arbitrary test logic. See sample: `test-data-must-be-random-and-complete.good.al`. diff --git a/microsoft/skills/review/al-appsource-review.md b/microsoft/skills/review/al-appsource-review.md index 3f6a2218..2efc0493 100644 --- a/microsoft/skills/review/al-appsource-review.md +++ b/microsoft/skills/review/al-appsource-review.md @@ -52,6 +52,7 @@ The following targeted checks cover every current `appsource` article across the - A page or report that repository context identifies as a direct user entry point omits `UsageCategory` or sets it to `None` — `set-usagecategory-on-searchable-entry-points`. Do not select this article based only on object type; exclude supporting parts, dialogs, API pages, and objects intentionally reached through another page. - A `DateTime` assignment adds or subtracts a fixed duration to represent an assumed regional offset — `do-not-hard-code-time-zone-offsets`. Require contextual evidence such as an hour-sized constant, offset-oriented name, or time-zone comment; do not flag deadlines, schedules, or elapsed-time calculations. - For BC v27 or later, `app.json` adds or changes the `help` URL to a path deeper than two levels, or a changed Copilot/context-sensitive help arrangement would ground the app under an overly broad truncated parent — `keep-copilot-help-url-to-two-path-levels`. +- Changed code declares or calls `File.Open`/`File.Create`/`File.Read`/`File.Write` in an app targeting Business Central Online — `file-datatype-saas`. Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`. diff --git a/microsoft/skills/review/al-breaking-changes-review.md b/microsoft/skills/review/al-breaking-changes-review.md index 7f307139..5e2f3aa9 100644 --- a/microsoft/skills/review/al-breaking-changes-review.md +++ b/microsoft/skills/review/al-breaking-changes-review.md @@ -50,7 +50,9 @@ The following targeted checks cover every current `breaking-changes` article: - A published procedure changes parameter count/order/type/name, `var`, return type, or array shape instead of preserving the old signature and adding an overload — `do-not-change-published-procedure-signatures`. - A public procedure/event/interface exposes a credential or other sensitive value through `Text` or an externally callable contract — `do-not-expose-sensitive-data-through-public-api`. - Code already marked obsolete is expanded with new behavior instead of routing new callers to its replacement — `do-not-modify-code-already-marked-obsolete`. -- A shipped table field is deleted, renamed, renumbered, or replaced without retaining the original field as `ObsoleteState = Pending` and migrating its data — `obsolete-table-fields-instead-of-deleting-them`. +- A shipped table field is deleted, renamed, renumbered, or replaced without retaining the original field as `ObsoleteState = Pending` and migrating its data — `obsolete-table-fields-instead-of-deleting-them`. This owns AS0005 field-name changes; do not substitute the namespace article. +- A published object's namespace changes between the base and changed source while its identity otherwise remains — `namespace-is-part-of-published-object-identity`. Do not apply it to a new, unshipped object or to an ordinary object-name change with no namespace change. +- New or changed code calls `Codeunit Mail`'s `CreateMessage`/`Send`/`GetErrorDesc` instead of `Codeunit Email`/`Codeunit "Email Message"` — `prefer-email-module`. For `obsolete-table-fields-instead-of-deleting-them`, compare the baseline ID and name before emitting. When the original field remains under the same ID and name with `ObsoleteState = Pending`, and the replacement uses a new ID, the change follows the rule and must not be flagged. diff --git a/microsoft/skills/review/al-style-review.md b/microsoft/skills/review/al-style-review.md index 5b0b6b6c..0be754aa 100644 --- a/microsoft/skills/review/al-style-review.md +++ b/microsoft/skills/review/al-style-review.md @@ -40,8 +40,8 @@ Discard files that are not applicable. Retain conditionally applicable files onl Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against: - Changed AL objects — especially API pages (`PageType = API`), tables and pages declaring Labels/TextConsts, codeunits issuing `Error`/`Message`/`Confirm`, and any file whose name violates the `..al` convention. -- Changed declarations, weighted toward `: Label '...'`, `: TextConst '...'`, temporary record variables, `DateFormula` declarations and their `Evaluate` call sites, error-handling call sites, and API declarations. -- Tokens extracted from the diff (`Label`, `TextConst`, `Locked`, `Comment`, `MaxLength`, `temporary`, `DateFormula`, `Evaluate`, `CalcDate`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `DelayedInsert`, `FieldCaption`, `TableCaption`, `FieldName`, `TableName`, `Page.RunModal`, `Report.Run`, `StrSubstNo`). +- Changed declarations, weighted toward `: Label '...'`, `: TextConst '...'`, temporary record variables, option fields, `DateFormula` declarations and their `Evaluate` call sites, error-handling call sites, API declarations, and codeunit-internal method calls. +- Tokens extracted from the diff (`Label`, `TextConst`, `Locked`, `Comment`, `MaxLength`, `temporary`, `DateFormula`, `Evaluate`, `CalcDate`, `OptionMembers`, `OptionCaption`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `DelayedInsert`, `FieldCaption`, `TableCaption`, `FieldName`, `TableName`, `Page.RunModal`, `Report.Run`, `this.`, `StrSubstNo`, `namespace`, `using`, `var `). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object or declaration. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. From fa3d04c56c0ce6b577298b9c7817f2641eea815c Mon Sep 17 00:00:00 2001 From: Michael Dieringer <65093775+MichaelDieringer@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:55:45 +0200 Subject: [PATCH 3/7] Fix dimension-management-wiring.md: ValidateShortcutDimCode and CreateDim do not exist on the current DimensionManagement codeunit Verified against microsoft/BCApps: the real master-table validation procedure is ValidateDimValueCode (or ValidateShortcutDimValues when a DimSetID is also needed), and the real document-side inheritance procedure is GetDefaultDimID, not CreateDim. Caught from Jesper Schulz-Wedde's review thread, which had been partially hidden by GitHub's comment folding. Co-Authored-By: Claude Sonnet 5 --- .../data-modeling/dimension-management-wiring.good.al | 2 +- .../knowledge/data-modeling/dimension-management-wiring.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al b/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al index 2af97b84..1352b678 100644 --- a/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al @@ -13,7 +13,7 @@ table 50100 "Course" var DimMgt: Codeunit DimensionManagement; begin - DimMgt.ValidateShortcutDimCode(1, "Global Dimension 1 Code"); + DimMgt.ValidateDimValueCode(1, "Global Dimension 1 Code"); DimMgt.SaveDefaultDim(Database::Course, "No.", FieldNo("Global Dimension 1 Code"), "Global Dimension 1 Code"); end; } diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.md b/microsoft/knowledge/data-modeling/dimension-management-wiring.md index c939952b..8e932abc 100644 --- a/microsoft/knowledge/data-modeling/dimension-management-wiring.md +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.md @@ -1,7 +1,7 @@ --- bc-version: [all] domain: data-modeling -keywords: [dimensions, dimensionmanagement, global-dimension, shortcut-dimension, default-dimension, validateshortcutdimcode, createdim] +keywords: [dimensions, dimensionmanagement, global-dimension, shortcut-dimension, default-dimension, validatedimvaluecode, getdefaultdimid] technologies: [al] countries: [w1] application-area: [all] @@ -17,7 +17,7 @@ Adding dimension support to a custom master or document table is not just a matt ## Best Practice -A master table should validate its dimension fields through `ValidateShortcutDimCode` and `SaveDefaultDim`, and create/delete the matching Default Dimension records in `OnInsert`/`OnDelete`. A document table should add Shortcut Dimension fields validated the same way, and call `CreateDim` to pull dimension values from the related master record whenever the field that attaches the document to that master changes. +A master table should validate its dimension fields through `ValidateDimValueCode` (or `ValidateShortcutDimValues` when a `DimSetID` is also needed) and `SaveDefaultDim`, and create/delete the matching Default Dimension records in `OnInsert`/`OnDelete`. A document table should add Shortcut Dimension fields validated the same way, and call `GetDefaultDimID` to pull inherited dimension values from the related master record whenever the field that attaches the document to that master changes. See sample: `dimension-management-wiring.good.al`. From a28ba1a1d7ccbf3e153cba49f5a023395f62ef64 Mon Sep 17 00:00:00 2001 From: Michael Dieringer <65093775+MichaelDieringer@users.noreply.github.com> Date: Tue, 8 Sep 2026 20:40:21 +0200 Subject: [PATCH 4/7] Address second round of Jesper Schulz-Wedde's review on PR #156 - dimension-management-wiring.md/.good.al: split into the two distinct models the article was conflating - master data (Default Dimension records via ValidateDimValueCode/SaveDefaultDim) vs. transactional/ document data (a single Dimension Set ID assembled via AddDimSource + GetDefaultDimID, verified against BCApps' ExchRateAdjmtProcess.Codeunit.al). Added a compiling document-table example alongside the existing master table one. - Deleted api-page-flowfields-must-be-calcfields (.md/.good.al/.bad.al): Microsoft's own FlowFields documentation states a FlowField used as a control's direct source expression is automatically calculated on any page - no API-page exception is documented, and none could be reproduced. - prefer-email-module.bad.al/.md: Codeunit Mail has no Send/GetErrorDesc members; fixed to the real current 7-argument CreateMessage signature, and corrected the claim that the legacy path "still runs" - its base implementation no longer sends anything, only raises integration events. - check-post-line-batch-pattern.md/.good.al: reframed from a universal invariant to the standard shape, naming the real Gen./Item/CA/Res./Job/ Insurance/Mfg. Item/FA Jnl.-Check Line/-Post Line/-Post Batch codeunits it's based on. Added the missing Check Line companion codeunit so the good fixture is internally complete. - test-data-must-be-random-and-complete.good.al: removed leftover "collision-free" wording contradicting the already-corrected article text. - fixed-choice-set-must-use-enum-not-integer.md: removed the reintroduced state-count heuristic ("the line is the state count"), aligned with binary-choice-must-be-boolean.md's semantics-based distinction. - namespace-must-be-verified-from-source.md: removed the false claim that the compiler and AL Language Server use different namespace-resolution rules. - intrinsic-al-functions-must-use-modern-casing.md: removed the unverified claim that PascalCase is the VS Code formatter's default output. Worklist completeness: added cues for the 8 rules in data-modeling, testing, performance, and web-services that had none (Jesper's explicit ask), plus the same gap in all 7 style rules from this PR (not explicitly named this round, but the identical systemic issue) - 15 cues total across al-data-modeling-review.md, al-testing-review.md, al-performance-review.md, al-web-services-review.md, and al-style-review.md. Co-Authored-By: Claude Sonnet 5 --- .../prefer-email-module.bad.al | 8 +-- .../breaking-changes/prefer-email-module.md | 2 +- .../check-post-line-batch-pattern.good.al | 10 ++++ .../check-post-line-batch-pattern.md | 2 +- .../dimension-management-wiring.good.al | 54 +++++++++++++++++++ .../dimension-management-wiring.md | 11 +++- ...ed-choice-set-must-use-enum-not-integer.md | 2 +- ...sic-al-functions-must-use-modern-casing.md | 2 +- .../namespace-must-be-verified-from-source.md | 4 +- ...t-data-must-be-random-and-complete.good.al | 2 +- ...-page-flowfields-must-be-calcfields.bad.al | 24 --------- ...page-flowfields-must-be-calcfields.good.al | 27 ---------- .../api-page-flowfields-must-be-calcfields.md | 28 ---------- .../skills/review/al-data-modeling-review.md | 4 ++ .../skills/review/al-performance-review.md | 1 + microsoft/skills/review/al-style-review.md | 8 +++ microsoft/skills/review/al-testing-review.md | 1 + .../skills/review/al-web-services-review.md | 2 + 18 files changed, 98 insertions(+), 94 deletions(-) delete mode 100644 microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.bad.al delete mode 100644 microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.good.al delete mode 100644 microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.md diff --git a/microsoft/knowledge/breaking-changes/prefer-email-module.bad.al b/microsoft/knowledge/breaking-changes/prefer-email-module.bad.al index 299afd99..78e593f9 100644 --- a/microsoft/knowledge/breaking-changes/prefer-email-module.bad.al +++ b/microsoft/knowledge/breaking-changes/prefer-email-module.bad.al @@ -1,13 +1,9 @@ codeunit 50103 "Order Confirmation Notifier" { - procedure Send(FromName: Text; ToAddress: Text; Subject: Text; Body: Text) + procedure Send(ToAddress: Text; Subject: Text; Body: Text) var Mail: Codeunit Mail; - MailSent: Boolean; begin - Mail.CreateMessage(FromName, ToAddress, '', Subject, Body, true); - MailSent := Mail.Send(); - if not MailSent then - Message(Mail.GetErrorDesc()); + Mail.CreateMessage(ToAddress, '', '', Subject, Body, false, false); end; } diff --git a/microsoft/knowledge/breaking-changes/prefer-email-module.md b/microsoft/knowledge/breaking-changes/prefer-email-module.md index d9137e81..9219373b 100644 --- a/microsoft/knowledge/breaking-changes/prefer-email-module.md +++ b/microsoft/knowledge/breaking-changes/prefer-email-module.md @@ -23,6 +23,6 @@ See sample: `prefer-email-module.good.al`. ## Anti Pattern -Calling `Codeunit Mail`'s `CreateMessage`/`Send`/`GetErrorDesc`. It still runs, but it is hard-coupled to whatever SMTP setup exists, and leaves no queryable record of what was sent. +Calling `Codeunit Mail`'s `CreateMessage`. It still compiles and runs, but current `Codeunit Mail`'s own implementation of `CreateMessage` no longer sends anything by itself — it only raises integration events for a legacy subscriber to act on — so building new code on it means depending on whatever compatibility shim happens to still be wired up, with no first-class connector selection and no queryable Sent/Outbox/Draft record. `Send` and `GetErrorDesc` are not current members of `Codeunit Mail` at all; do not reference them. See sample: `prefer-email-module.bad.al`. diff --git a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al index d7384859..403a964c 100644 --- a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al +++ b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al @@ -1,3 +1,13 @@ +codeunit 50100 "Meter Jnl.-Check Line" +{ + procedure CheckLine(var MeterJnlLine: Record "Meter Journal Line") + begin + // Reads setup/dimension data only, shows no UI beyond errors. + if MeterJnlLine.Quantity = 0 then + Error('Quantity must not be zero.'); + end; +} + codeunit 50101 "Meter Jnl.-Post Line" { procedure PostLine(var MeterJnlLine: Record "Meter Journal Line") diff --git a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md index 9dd19d53..3514e172 100644 --- a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md +++ b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md @@ -13,7 +13,7 @@ application-area: [all] ## Description -Every journal-based posting routine in Business Central is split across three companion codeunits with distinct, non-overlapping responsibilities: `Check Line` validates one line, `Post Line` writes exactly one line to the ledger, and `Post Batch` loops both across the journal. A document posting routine (posting one document at a time) calls `Post Line` directly and skips `Post Batch`. A new posting routine that blurs this split either misses functionality other code expects to call directly, or exposes an interaction surface it shouldn't. +Business Central's own journal-based posting routines consistently follow a three-codeunit split with distinct, non-overlapping responsibilities — `Codeunit "Gen. Jnl.-Check Line"` / `"Gen. Jnl.-Post Line"` / `"Gen. Jnl.-Post Batch"` for the general journal, and the same `-Check Line` / `-Post Line` / `-Post Batch` shape repeated for Item, Resource, Job, Fixed Asset, Insurance, and Cost Accounting journals: `Check Line` validates one line, `Post Line` writes exactly one line to the ledger, and `Post Batch` loops both across the journal. A document posting routine (posting one document at a time) calls `Post Line` directly and skips `Post Batch`. This is the standard shape to evaluate a new journal-based posting routine against, not a platform-enforced constraint — a routine with a genuinely different transaction/reuse shape may legitimately organize itself differently. But a new routine that blurs this split without a specific reason either misses functionality other code expects to call directly, or exposes an interaction surface it shouldn't. ## Best Practice diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al b/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al index 1352b678..18c95047 100644 --- a/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al @@ -1,3 +1,4 @@ +// Master data: Default Dimension records, no Dimension Set ID field. table 50100 "Course" { fields @@ -26,3 +27,56 @@ table 50100 "Course" DimMgt.DeleteDefaultDim(Database::Course, "No."); end; } + +// Transactional/document data: a single Dimension Set ID, inherited from the +// related master record and overridable via shortcut dimension fields. +table 50101 "Course Registration Header" +{ + fields + { + field(1; "No."; Code[20]) { } + field(2; "Customer No."; Code[20]) + { + TableRelation = Customer; + + trigger OnValidate() + begin + UpdateDimensionSetID(); + end; + } + field(10; "Shortcut Dimension 1 Code"; Code[20]) + { + CaptionClass = '1,1,1'; + TableRelation = "Dimension Value".Code where( + "Global Dimension No." = const(1), Blocked = const(false)); + + trigger OnValidate() + var + DimMgt: Codeunit DimensionManagement; + begin + DimMgt.ValidateShortcutDimValues(1, "Shortcut Dimension 1 Code", "Dimension Set ID"); + end; + } + field(480; "Dimension Set ID"; Integer) + { + Editable = false; + TableRelation = "Dimension Set Entry"."Dimension Set ID"; + } + } + + local procedure UpdateDimensionSetID() + var + Customer: Record Customer; + DimMgt: Codeunit DimensionManagement; + DefaultDimSource: List of [Dictionary of [Integer, Code[20]]]; + GlobalDim2Code: Code[20]; + begin + if not Customer.Get("Customer No.") then + exit; + + DimMgt.AddDimSource(DefaultDimSource, Database::Customer, "Customer No."); + "Dimension Set ID" := + DimMgt.GetDefaultDimID( + DefaultDimSource, '', "Shortcut Dimension 1 Code", GlobalDim2Code, "Dimension Set ID", 0); + end; +} diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.md b/microsoft/knowledge/data-modeling/dimension-management-wiring.md index 8e932abc..9e14371c 100644 --- a/microsoft/knowledge/data-modeling/dimension-management-wiring.md +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.md @@ -13,11 +13,18 @@ application-area: [all] ## Description -Adding dimension support to a custom master or document table is not just a matter of adding a `Code[20]` field. Business Central expects a specific set of hooks into `Codeunit "Dimension Management"` so a dimension value is validated, persisted as a Default Dimension record, and flows through to transactions the same way it does for every standard table. Skipping any one hook produces a field that looks correct in the designer but silently fails to save, validate, or carry through to postings. +Adding dimension support to a custom table is not just a matter of adding a `Code[20]` field, and master tables and document/transactional tables wire into `Codeunit "Dimension Management"` through two different models — treating them as one mechanism is itself the mistake this article corrects: + +- **Master data** (a custom master table, e.g. "Course") persists **Default Dimension** records: each shortcut dimension field validates through `ValidateDimValueCode`, then the result is saved via `SaveDefaultDim`, and `DeleteDefaultDim` removes them again in `OnDelete`. The master record itself carries no `Dimension Set ID` field. +- **Transactional/document data** (a custom document or journal-line table) carries a single **`Dimension Set ID`** field — a pointer to a shared, deduplicated set of dimension values in `Dimension Set Entry`, assembled from whatever the document inherited plus whatever the user overrode. A document does not acquire that ID by calling `SaveDefaultDim`; it builds a source list with `AddDimSource` (naming the related master table and its key, e.g. `Database::Customer`), then calls `GetDefaultDimID` to compute a new `Dimension Set ID` that inherits the master's Default Dimension records. Editing a shortcut dimension field directly on the document validates through `ValidateShortcutDimValues`, which updates the same `Dimension Set ID` in place rather than writing a separate Default Dimension record. + +Skipping the model that actually matches the table's kind produces a field that looks correct in the designer but silently fails to save, validate, or carry through to postings — or, for a document, one that never picks up the customer's/vendor's own dimensions at all. ## Best Practice -A master table should validate its dimension fields through `ValidateDimValueCode` (or `ValidateShortcutDimValues` when a `DimSetID` is also needed) and `SaveDefaultDim`, and create/delete the matching Default Dimension records in `OnInsert`/`OnDelete`. A document table should add Shortcut Dimension fields validated the same way, and call `GetDefaultDimID` to pull inherited dimension values from the related master record whenever the field that attaches the document to that master changes. +For a master table, validate each shortcut dimension field through `ValidateDimValueCode`, save the result with `SaveDefaultDim`, and delete the matching Default Dimension records in `OnDelete`. + +For a document table, when the field that attaches the document to a master record changes (e.g. `Customer No.`), call `AddDimSource` naming that master table and key, then `GetDefaultDimID` to compute the document's new `Dimension Set ID`, inheriting the master's Default Dimension records. Validate the document's own Shortcut Dimension fields through `ValidateShortcutDimValues`, which updates that same `Dimension Set ID` rather than persisting a separate Default Dimension record. See sample: `dimension-management-wiring.good.al`. diff --git a/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md b/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md index 78e0528a..f9adf179 100644 --- a/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md +++ b/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md @@ -13,7 +13,7 @@ application-area: [all] ## Description -When a variable or field can only take on a fixed set of more than two named, mutually exclusive states — a difficulty level, a document type, a processing status — it should be typed as `Enum` (or `Option` when extending an object that still uses the legacy type). Representing that same state as a plain `Integer` and tracking the meaning of each value in a comment or in a developer's head is a magic-number anti-pattern: the compiler cannot catch an out-of-range value, and branches read as opaque numbers instead of names. This is distinct from a true two-state choice, which should be `Boolean` rather than an enumeration — the line is the state count. +When a variable or field represents a fixed set of named, mutually exclusive states — a difficulty level, a document type, a processing status — it should be typed as `Enum` (or `Option` when extending an object that still uses the legacy type). Representing that same state as a plain `Integer` and tracking the meaning of each value in a comment or in a developer's head is a magic-number anti-pattern: the compiler cannot catch an out-of-range value, and branches read as opaque numbers instead of names. This is about semantics, not member count, matching `binary-choice-must-be-boolean.md`'s own distinction: a domain concept that is genuinely a stable true/false predicate belongs in `Boolean` even if someone represents it as a two-value `Enum`, while a domain concept with exactly two current named states is not automatically a Boolean in disguise — it stays an `Enum` when the states are named alternatives rather than a yes/no flag, or when it needs to implement an interface, preserve an existing contract, or leave room for a future third value. ## Best Practice diff --git a/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md b/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md index 42c45104..632c9280 100644 --- a/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md +++ b/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md @@ -13,7 +13,7 @@ application-area: [all] ## Description -AL is case-insensitive, so `MESSAGE(...)`, `ERROR(...)`, `CONFIRM(...)`, and `STRSUBSTNO(...)` compile and run identically to `Message(...)`, `Error(...)`, `Confirm(...)`, and `StrSubstNo(...)`. Modern AL — the VS Code tooling's default formatter output, Microsoft's own current samples, and current reference codebases — writes intrinsic/built-in function calls in the casing Microsoft assigns to the function's declared name, typically PascalCase. ALL-CAPS calls are a holdover from classic C/AL and signal code that has not been modernized, even though it compiles and runs correctly. Reserved keywords such as `if`, `begin`, and `for` are a separate, already-tooled concern; intrinsic function names are identifiers, not keywords, so that tooling does not catch ALL-CAPS intrinsic function calls. +AL is case-insensitive, so `MESSAGE(...)`, `ERROR(...)`, `CONFIRM(...)`, and `STRSUBSTNO(...)` compile and run identically to `Message(...)`, `Error(...)`, `Confirm(...)`, and `StrSubstNo(...)`. Modern AL — Microsoft's own current samples and current reference codebases — writes intrinsic/built-in function calls in the casing Microsoft assigns to the function's declared name, typically PascalCase. This is a codebase-convention claim, not a claim about what the VS Code formatter enforces: the formatter normalizes particular syntax but is not a mechanism for recasing every intrinsic function call, so do not cite formatter behavior as the reason to follow this convention. ALL-CAPS calls are a holdover from classic C/AL and signal code that has not been modernized, even though it compiles and runs correctly. Reserved keywords such as `if`, `begin`, and `for` are a separate, already-tooled concern; intrinsic function names are identifiers, not keywords, so that tooling does not catch ALL-CAPS intrinsic function calls. ## Best Practice diff --git a/microsoft/knowledge/style/namespace-must-be-verified-from-source.md b/microsoft/knowledge/style/namespace-must-be-verified-from-source.md index 0be019d0..39d02103 100644 --- a/microsoft/knowledge/style/namespace-must-be-verified-from-source.md +++ b/microsoft/knowledge/style/namespace-must-be-verified-from-source.md @@ -13,7 +13,7 @@ application-area: [all] ## Description -Since Business Central 2024 release wave 1, Microsoft's own objects are organized under a deep `Microsoft.*` namespace tree that has been renamed and restructured repeatedly. When adding a `using` directive for an existing AL object (table, codeunit, page, enum, interface, etc.), guessing its namespace from the object's name, from an older codebase, or from general familiarity produces a statement that can look plausible, compile in isolation, and still resolve to the wrong object or fail in the AL Language Server that VS Code actually uses to report errors. The reliable sources are the object's own source file (its `namespace` declaration) or, for a dependency without accessible source, its AL symbol package — not the object's name or a remembered convention. +Since Business Central 2024 release wave 1, Microsoft's own objects are organized under a deep `Microsoft.*` namespace tree that has been renamed and restructured repeatedly. When adding a `using` directive for an existing AL object (table, codeunit, page, enum, interface, etc.), guessing its namespace from the object's name, from an older codebase, or from general familiarity produces a statement that can look plausible and still resolve to the wrong object, or fail to resolve at all, once checked against the object's actual current namespace. The reliable sources are the object's own source file (its `namespace` declaration) or, for a dependency without accessible source, its AL symbol package — not the object's name or a remembered convention. ## Best Practice @@ -23,6 +23,6 @@ See sample: `namespace-must-be-verified-from-source.good.al`. ## Anti Pattern -Writing a `using` statement from memory, from an incomplete path, or from a plausible-looking guess. It can compile in one build environment while still failing to resolve in VS Code, because the two use different namespace resolution. +Writing a `using` statement from memory, from an incomplete path, or from a plausible-looking guess. It can appear correct while actually resolving to the wrong object, or fail to resolve, once checked against stale or mismatched symbols, a different build configuration, or the object's actual current source — not because the compiler and the AL Language Server apply different namespace-resolution rules; they don't. See sample: `namespace-must-be-verified-from-source.bad.al`. diff --git a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.good.al b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.good.al index d2ac678d..75518f1a 100644 --- a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.good.al +++ b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.good.al @@ -4,7 +4,7 @@ var Customer: Record Customer; SalesHeader: Record "Sales Header"; begin - // Random, collision-free customer — no assumption about what exists. + // Freshly created customer, owned by this test — no assumption about what exists. LibrarySales.CreateCustomer(Customer); LibrarySales.CreateSalesHeader( SalesHeader, SalesHeader."Document Type"::Order, Customer."No."); diff --git a/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.bad.al b/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.bad.al deleted file mode 100644 index 8e25036d..00000000 --- a/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.bad.al +++ /dev/null @@ -1,24 +0,0 @@ -page 50100 "Item Availability API" -{ - PageType = API; - APIPublisher = 'contoso'; - APIGroup = 'inventory'; - APIVersion = 'v1.0'; - EntityName = 'itemAvailability'; - EntitySetName = 'itemAvailabilities'; - SourceTable = Item; - - layout - { - area(content) - { - repeater(General) - { - field(itemNo; Rec."No.") { } - field(quantityOnHand; Rec.Inventory) { } - // No OnAfterGetRecord CalcFields — Inventory is a FlowField - // and returns 0 to every consumer. - } - } - } -} diff --git a/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.good.al b/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.good.al deleted file mode 100644 index 892be42c..00000000 --- a/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.good.al +++ /dev/null @@ -1,27 +0,0 @@ -page 50100 "Item Availability API" -{ - PageType = API; - APIPublisher = 'contoso'; - APIGroup = 'inventory'; - APIVersion = 'v1.0'; - EntityName = 'itemAvailability'; - EntitySetName = 'itemAvailabilities'; - SourceTable = Item; - - layout - { - area(content) - { - repeater(General) - { - field(itemNo; Rec."No.") { } - field(quantityOnHand; Rec.Inventory) { } - } - } - } - - trigger OnAfterGetRecord() - begin - Rec.CalcFields(Inventory); - end; -} diff --git a/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.md b/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.md deleted file mode 100644 index 10044f3e..00000000 --- a/microsoft/knowledge/web-services/api-page-flowfields-must-be-calcfields.md +++ /dev/null @@ -1,28 +0,0 @@ ---- -bc-version: [all] -domain: web-services -keywords: [api-page, flowfield, calcfields, odata] -technologies: [al] -countries: [w1] -application-area: [all] ---- - -# Explicitly Calculate FlowFields on API Pages - -> Contributions welcome — open a PR to refine or extend this article. - -## Description - -FlowFields are not stored in the database — Business Central computes them on demand. Regular pages trigger that calculation automatically while rendering, but API pages do not. A FlowField referenced in an API page's layout returns an empty value to external consumers unless it is calculated explicitly, producing a silent data gap in OData responses that is easy to miss in review. - -## Best Practice - -Call `CalcFields` for every FlowField referenced in the page layout from the `OnAfterGetRecord` trigger, combining multiple fields into a single call. - -See sample: `api-page-flowfields-must-be-calcfields.good.al`. - -## Anti Pattern - -Relying on the implicit calculation that regular pages perform. Any FlowField left out of the `CalcFields` call returns a blank value to every API consumer with no visible error. - -See sample: `api-page-flowfields-must-be-calcfields.bad.al`. diff --git a/microsoft/skills/review/al-data-modeling-review.md b/microsoft/skills/review/al-data-modeling-review.md index 3fca8ecb..23183045 100644 --- a/microsoft/skills/review/al-data-modeling-review.md +++ b/microsoft/skills/review/al-data-modeling-review.md @@ -54,6 +54,10 @@ The following targeted checks cover every current `data-modeling` article. Treat - A `Media` or `MediaSet` field is assigned directly between different table types or different field IDs instead of registering each shared item with `MediaSet.Insert` — `share-mediaset-items-with-insert-not-field-assignment`. - A custom document header assigns defaults outside an `InitRecord` boundary, calls `InitRecord` before assigning its number, or places UI-independent defaults only in a page trigger — `initialize-document-defaults-in-initrecord`. - Directed `Round` calls use `'<'` as mathematical floor or `'>'` as mathematical ceiling, especially where negative amounts are possible — `round-direction-symbols-use-magnitude`. +- A new field is typed `Code`/`Text` and its `OnValidate` calls `DimensionManagement`/`DimMgt`, or a table adds Shortcut Dimension fields, a `Dimension Set ID` field, or `AddDimSource`/`GetDefaultDimID` — `dimension-management-wiring`. A master table calling `SaveDefaultDim` and a document/journal table computing its own `Dimension Set ID` are two different valid shapes; do not flag a master table for lacking a `Dimension Set ID` field or a document for lacking `SaveDefaultDim`. +- A journal-based posting codeunit is added or changed and validation, Journal-table access, ledger writes, and user-interaction (`Confirm`/dialogs) all occur in one procedure or one codeunit, rather than split across `Check Line`/`Post Line`/`Post Batch`-shaped companions — `check-post-line-batch-pattern`. A document posting routine calling `Post Line` directly without a `Post Batch` companion is not this anti-pattern. +- An existing, already-published table's `keys` block adds, removes, or reorders a field in its primary key or any `Clustered = true` key — `do-not-change-primary-key`. A new table defining its own key for the first time is not this anti-pattern; requires repository/publication context to know the table has already shipped. +- Code reads a setup/configuration-table field inside a branch that has already decided the value is required, and blank/zero is handled with a fallback to a default rather than `TestField`/an equivalent guard — `testfield-required-setup-field`. A read that is genuinely optional in that branch, or one already guarded by `TestField`, is not this anti-pattern. Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`. diff --git a/microsoft/skills/review/al-performance-review.md b/microsoft/skills/review/al-performance-review.md index 7829d705..6109f7b0 100644 --- a/microsoft/skills/review/al-performance-review.md +++ b/microsoft/skills/review/al-performance-review.md @@ -58,6 +58,7 @@ Apply these targeted cues even when simple token overlap would rank the article - Worklist `job-queue-on-hold-does-not-stop-running-work.md` when a running job queue handler polls the entry's `Status` or `On Hold` value as a cancellation signal. Exclude application-owned stop requests that are checked before every bounded unit of work, including the first, when completed work and its checkpoint remain consistent and resume logic clears the request. - Worklist `job-queue-category-code-serializes-conflicting-jobs.md` when two or more job queue entries in the same company are shown by the changed context to require mutual exclusion but have empty or different Job Queue Category Codes. Do not infer a conflict merely because jobs touch the same tables, and do not recommend a category to coordinate across companies, environments, or workers outside the job queue dispatcher. - Worklist `store-scheduled-task-id-to-avoid-duplicate-tasks.md` when `TaskScheduler.CreateTask` runs from initialization, login, setup, or another repeatable path without persisting its returned GUID and checking it with `TaskScheduler.TaskExists` before creating a replacement. Exclude one-shot creation and correctly persisted check-before-create flows; concurrent callers still require serialization around that sequence. +- A new or changed report object whose usage/name/caption identifies it as a single-record document (invoice, statement, order confirmation) sets or retains `DefaultRenderingLayout = RDLC` — `document-report-word-layout.md`. Do not worklist this from a tabular/list report with heavy aggregation or calculated columns; RDLC/Excel remains the better fit there. These targeted inclusions and exclusions override generic token overlap. Do not retain an excluded article solely because the diff contains one of its keywords. diff --git a/microsoft/skills/review/al-style-review.md b/microsoft/skills/review/al-style-review.md index 0be754aa..0cc747e6 100644 --- a/microsoft/skills/review/al-style-review.md +++ b/microsoft/skills/review/al-style-review.md @@ -51,6 +51,14 @@ Apply these high-signal mappings before fuzzy topic ranking: - A `Label` or `TextConst` contains multiple or ambiguous placeholders but has no `Comment`, or its Comment does not explain every placeholder — `label-comment-explains-placeholders.md`. A single placeholder whose meaning is explicit in the text, such as `Customer %1`, is allowed without a Comment and must not be flagged. - A normal two-argument `Evaluate` has a resolved `DateFormula` destination and a hard-coded non-angle-bracket date-formula literal, directly or through a visible constant — `dateformula-evaluate-needs-language-independent-literals.md`. Do not use this cue for dynamic/localized external input, already invariant `<...>` input, or direct `CalcDate(Text, ...)` calls. +- `function-call-parentheses-required.md` applies only to a zero-argument invocation written without `()`. Never worklist it from an invocation that already has parentheses or supplies arguments, including `Error(Label, Arg1, Arg2)`. +- A new `.app` build artifact appears at the project root or another unversioned/arbitrary location, or is added to source control alongside the AL source that produced it — `al-build-output-must-not-pollute-project-root.md`. A deliberate `--outfolder`/`outputPath` destination added to `.gitignore` is the compliant shape, not the signal to flag. +- A new or renamed AL identifier (variable, procedure, parameter, field, object, enum value, or label identifier) contains non-English words — `al-identifiers-english.md`. A caption, tooltip, or other user-facing text value in a non-English language is not this anti-pattern; only the identifier itself is in scope. +- A field or variable is typed `Boolean` and its two possible values are genuinely named domain alternatives (a status pair like Inbound/Outbound, Debit/Credit, Buy/Sell) rather than a true/false predicate, or an `Option`/`Enum`/`Integer` models a domain concept that is intrinsically a yes/no flag — `binary-choice-must-be-boolean.md`. The signal is a semantic mismatch between the type and the domain concept, not the current number of states. +- A field or variable is typed `Integer` with the meaning of each value tracked only in a comment, or an `Enum` with more than two members is proposed as `Boolean`-like — `fixed-choice-set-must-use-enum-not-integer.md`. Do not flag a genuinely two-state `Enum`/`Option` for having "too few" members; that overlaps `binary-choice-must-be-boolean.md` instead when the domain is a true/false predicate. +- A new `using` directive is added for an existing AL object without the diff also showing that object's own `namespace` declaration or a symbol-package lookup backing the choice — `namespace-must-be-verified-from-source.md`. Require repository/dependency context; a single new `using` line cannot itself prove whether the namespace was verified or guessed. +- An intrinsic/built-in AL function call (`MESSAGE`, `ERROR`, `CONFIRM`, `STRSUBSTNO`, etc.) is written in ALL-CAPS or another non-PascalCase form — `intrinsic-al-functions-must-use-modern-casing.md`. +- A procedure call passes a literal or a computed expression (not a caller-scope variable) to a parameter position the callee declares `var` — `var-parameters-require-an-addressable-variable.md`. This is a compile-time-guaranteed shape; flag it only when the callee's declared signature is visible in the diff or resolvable from context. Once the candidate worklist is known, resolve layer-precedence conflicts per READ and record suppressions. diff --git a/microsoft/skills/review/al-testing-review.md b/microsoft/skills/review/al-testing-review.md index c96ac833..c5c49984 100644 --- a/microsoft/skills/review/al-testing-review.md +++ b/microsoft/skills/review/al-testing-review.md @@ -51,6 +51,7 @@ The following targeted checks cover every current `testing` article. Treat each - Test fixture code manually calls `Init`/`Insert`, invents keys or prerequisite records, or bypasses available `LibrarySales`, `LibraryPurchase`, `LibraryERM`, `LibraryInventory`, `LibraryRandom`, or equivalent library codeunits — `use-library-codeunits-for-test-fixtures`. - `asserterror` is added or changed without a following `Assert.ExpectedError`, `Assert.ExpectedErrorCode`, or a purpose-built assertion such as `ExpectedTestFieldError` — `asserterror-needs-expectederror-and-code`. - A test path raises UI and `[HandlerFunctions(...)]` does not match the invoked handlers, or the test has no meaningful evidence of the UI result (for example, it treats a Boolean set before the action as proof of success) — `ui-handlers-in-tests`. A capture/reset/assert-after-`RunModal` pattern is valid. Enqueue/dequeue and `AssertEmpty` are required only when order, count, text, replies, or a scripted sequence is part of the contract. Only nonoptional handlers have to execute: a listed handler declared `[SendNotificationHandler(true)]` or `[RecallNotificationHandler(true)]` is optional by design, so do not treat it as unmatched when the run never raises the notification. +- A test's `[GIVEN]`/setup looks up a hardcoded code/number/name assumed to already exist instead of creating it, leaves a mandatory field on a created record empty or under-sized, or a scenario-defining value (amount, quantity, percentage, date, threshold, rounding precision) is generated/randomized instead of an explicit chosen value — `test-data-must-be-random-and-complete`. Generating incidental fixture values (identifiers, names, descriptions) via the standard library codeunits is the compliant shape, not the signal to flag. Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`. diff --git a/microsoft/skills/review/al-web-services-review.md b/microsoft/skills/review/al-web-services-review.md index 19d07355..c486c88f 100644 --- a/microsoft/skills/review/al-web-services-review.md +++ b/microsoft/skills/review/al-web-services-review.md @@ -40,6 +40,8 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed AL object names and types — especially pages declared with `PageType = API`, API page `part` controls, queries declared with `QueryType = API`, and procedures that expose bound actions. - The changed properties and triggers, weighted toward API page metadata (`APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SourceTable`, `SourceTableTemporary`), navigation metadata (`SubPageLink`, `Multiplicity`, and visible singleton or collection semantics), CRUD guards (`InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`), the `OnOpenPage` trigger, and `OnValidate` triggers on exposed fields. - Webhook subscriber handlers and subscription lifecycle code, especially code that creates or renews subscriptions, handles `validationToken`, schedules from `expirationDateTime`, or targets resources whose eligibility is visible in the diff. +- An API page (`PageType = API`) with `InsertAllowed = true` lists a field in `ODataKeyFields` and that same field control sets `Editable = false` — `api-page-key-fields-must-be-editable-on-insert.md`. A system-generated key such as `SystemId` marked read-only is not this anti-pattern. +- An API page exposes a field via `Rec` directly, and that field is a stored value computed from other fields inside an `OnValidate` trigger rather than a FlowField recalculated in `OnAfterGetRecord` — `stored-derived-fields-must-not-be-exposed-directly.md`. Exposing a genuine FlowField, or a value already recalculated in `OnAfterGetRecord`, is not this anti-pattern. - Tokens extracted from the diff that relate to API surface and behaviour (`PageType`, `QueryType`, `API`, `api-page`, `page-part`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `ODataKeyFields`, `SystemId`, `SubPageLink`, `subpagelink`, `Multiplicity`, `multiplicity`, `Many`, `ZeroOrOne`, `SourceTableTemporary`, `Job Queue Entry`, `webhook`, `webhookSupportedResources`, `webhook-supported-resources`, `subscriptions`, `notificationUrl`, `validationToken`, `validationtoken`, `expirationDateTime`, `expirationdatetime`, `ServiceEnabled`, `WebServiceActionContext`, `SetActionResponse`, `ReadIsolation`, `IsolationLevel`, `ReadCommitted`, `InsertAllowed`, `ModifyAllowed`, `DeleteAllowed`, `Editable`, `SourceTable`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. From faa0bceb86e7ba24b92ec59be09536a3807954ca Mon Sep 17 00:00:00 2001 From: Michael Dieringer <65093775+MichaelDieringer@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:26:51 +0200 Subject: [PATCH 5/7] Fix remaining correctness issues from Jesper's 2026-09-15 re-review - dimension-management-wiring: SaveDefaultDim's third argument is the shortcut dimension number (1-8), not the field's AL field ID; the fixture passed FieldNo(...) = 10. GetDefaultDimID's InheritFromDimSetID must be 0 when recomputing after the linking record changes, not the document's existing Dimension Set ID (which would retain the previous customer's leftover dimensions). Verified against DimensionManagement.Codeunit.al and BankDepositHeader.Table.al in the BCApps reference clone. - check-post-line-batch-pattern: "Post Line writes exactly one line to the ledger" overclaimed - Gen. Jnl.-Post Line alone calls InsertGLEntry from a dozen call sites (balancing entry, VAT, currency rounding, deferrals) and can write several G/L Entries per journal line. Reworded to "posts exactly one journal line" and softened the "distinct, non-overlapping responsibilities" absolute. - namespace-must-be-verified-from-source.bad.al: dropped the "resolves in a local build, fails in VS Code" comment (taught an inherent compiler/language-server disagreement that isn't real); reframed as stale/cached symbols, matching the prose fix already made. - file-datatype-saas.good.al: replaced the deprecated 5-argument UploadIntoStream overload with the current 2-argument one, and actually staged through TempBlob as the article's own Best Practice instructs (the declared TempBlob variable was previously unused). - test-data-must-be-random-and-complete: no longer treats a short-but-valid value as defective merely for being "underfilled" - AL field lengths are maxima, not minimums. Scoped to missing values or a scenario with an explicit length/format requirement (e.g. a truncation test). Updated the al-testing-review.md routing cue to match. - stored-derived-fields-must-not-be-exposed-directly: stopped mandating source-field exposure as part of the core pattern: the good fixture exposed only one of the derived value's two inputs (Hours Used, not Budgeted Hours), making the claimed "so the consumer can verify it" impossible. Reframed as an optional, all-or-nothing addition and fixed the fixture to expose both inputs. Rebased onto upstream/main to resolve conflicts in al-breaking-changes-review.md, al-data-modeling-review.md, al-performance-review.md, and al-style-review.md against merged PRs #148 and #153; all sides' worklist tokens/cues retained. --- .../knowledge/appsource/file-datatype-saas.good.al | 13 ++++++++++--- .../check-post-line-batch-pattern.good.al | 2 +- .../data-modeling/check-post-line-batch-pattern.md | 2 +- .../dimension-management-wiring.good.al | 9 +++++++-- .../data-modeling/dimension-management-wiring.md | 4 ++-- .../namespace-must-be-verified-from-source.bad.al | 2 +- .../test-data-must-be-random-and-complete.md | 4 ++-- ...ived-fields-must-not-be-exposed-directly.good.al | 1 + ...d-derived-fields-must-not-be-exposed-directly.md | 2 +- microsoft/skills/review/al-testing-review.md | 2 +- 10 files changed, 27 insertions(+), 14 deletions(-) diff --git a/microsoft/knowledge/appsource/file-datatype-saas.good.al b/microsoft/knowledge/appsource/file-datatype-saas.good.al index 63b322f5..8703c338 100644 --- a/microsoft/knowledge/appsource/file-datatype-saas.good.al +++ b/microsoft/knowledge/appsource/file-datatype-saas.good.al @@ -3,11 +3,18 @@ codeunit 50104 "Import File Reader" procedure ImportFile() var TempBlob: Codeunit "Temp Blob"; + FromInStream: InStream; + ToOutStream: OutStream; InStream: InStream; - FileName: Text; begin - if UploadIntoStream('Import file', '', 'All Files (*.*)|*.*', FileName, InStream) then - ParseStream(InStream); + if not UploadIntoStream('All Files (*.*)|*.*', FromInStream) then + exit; + + TempBlob.CreateOutStream(ToOutStream); + CopyStream(ToOutStream, FromInStream); + + TempBlob.CreateInStream(InStream); + ParseStream(InStream); end; local procedure ParseStream(var InStream: InStream) diff --git a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al index 403a964c..6bfa93eb 100644 --- a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al +++ b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.good.al @@ -14,7 +14,7 @@ codeunit 50101 "Meter Jnl.-Post Line" var MeterLedgEntry: Record "Meter Ledger Entry"; begin - // Writes exactly one ledger entry; never touches the Journal table. + // Posts exactly one journal line; never touches the Journal table. MeterLedgEntry.Init(); MeterLedgEntry.TransferFields(MeterJnlLine); MeterLedgEntry.Insert(); diff --git a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md index 3514e172..5f2fbf61 100644 --- a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md +++ b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md @@ -13,7 +13,7 @@ application-area: [all] ## Description -Business Central's own journal-based posting routines consistently follow a three-codeunit split with distinct, non-overlapping responsibilities — `Codeunit "Gen. Jnl.-Check Line"` / `"Gen. Jnl.-Post Line"` / `"Gen. Jnl.-Post Batch"` for the general journal, and the same `-Check Line` / `-Post Line` / `-Post Batch` shape repeated for Item, Resource, Job, Fixed Asset, Insurance, and Cost Accounting journals: `Check Line` validates one line, `Post Line` writes exactly one line to the ledger, and `Post Batch` loops both across the journal. A document posting routine (posting one document at a time) calls `Post Line` directly and skips `Post Batch`. This is the standard shape to evaluate a new journal-based posting routine against, not a platform-enforced constraint — a routine with a genuinely different transaction/reuse shape may legitimately organize itself differently. But a new routine that blurs this split without a specific reason either misses functionality other code expects to call directly, or exposes an interaction surface it shouldn't. +Business Central's own journal-based posting routines consistently follow a three-codeunit split, each with one primary responsibility — `Codeunit "Gen. Jnl.-Check Line"` / `"Gen. Jnl.-Post Line"` / `"Gen. Jnl.-Post Batch"` for the general journal, and the same `-Check Line` / `-Post Line` / `-Post Batch` shape repeated for Item, Resource, Job, Fixed Asset, Insurance, and Cost Accounting journals: `Check Line` validates one line, `Post Line` posts exactly one journal line — `Gen. Jnl.-Post Line` itself can write more than one G/L Entry per call (a balancing entry, VAT, currency rounding, deferrals), so "exactly one line" describes its input, not a one-entry-out guarantee — and `Post Batch` loops both across the journal. A document posting routine (posting one document at a time) calls `Post Line` directly and skips `Post Batch`. This is the standard shape to evaluate a new journal-based posting routine against, not a platform-enforced constraint — a routine with a genuinely different transaction/reuse shape may legitimately organize itself differently, and the three codeunits' responsibilities are a useful default split, not a guarantee that every implementation keeps them non-overlapping. But a new routine that blurs this split without a specific reason either misses functionality other code expects to call directly, or exposes an interaction surface it shouldn't. ## Best Practice diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al b/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al index 18c95047..50178651 100644 --- a/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al @@ -15,7 +15,7 @@ table 50100 "Course" DimMgt: Codeunit DimensionManagement; begin DimMgt.ValidateDimValueCode(1, "Global Dimension 1 Code"); - DimMgt.SaveDefaultDim(Database::Course, "No.", FieldNo("Global Dimension 1 Code"), "Global Dimension 1 Code"); + DimMgt.SaveDefaultDim(Database::Course, "No.", 1, "Global Dimension 1 Code"); end; } } @@ -74,9 +74,14 @@ table 50101 "Course Registration Header" if not Customer.Get("Customer No.") then exit; + // Recompute from scratch (InheritFromDimSetID = 0): passing the existing + // "Dimension Set ID" here would inherit dimensions from whichever record + // the document was previously linked to, retaining them even after the + // new customer's defaults have nothing for that dimension. + "Shortcut Dimension 1 Code" := ''; DimMgt.AddDimSource(DefaultDimSource, Database::Customer, "Customer No."); "Dimension Set ID" := DimMgt.GetDefaultDimID( - DefaultDimSource, '', "Shortcut Dimension 1 Code", GlobalDim2Code, "Dimension Set ID", 0); + DefaultDimSource, '', "Shortcut Dimension 1 Code", GlobalDim2Code, 0, 0); end; } diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.md b/microsoft/knowledge/data-modeling/dimension-management-wiring.md index 9e14371c..020c8ee8 100644 --- a/microsoft/knowledge/data-modeling/dimension-management-wiring.md +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.md @@ -15,7 +15,7 @@ application-area: [all] Adding dimension support to a custom table is not just a matter of adding a `Code[20]` field, and master tables and document/transactional tables wire into `Codeunit "Dimension Management"` through two different models — treating them as one mechanism is itself the mistake this article corrects: -- **Master data** (a custom master table, e.g. "Course") persists **Default Dimension** records: each shortcut dimension field validates through `ValidateDimValueCode`, then the result is saved via `SaveDefaultDim`, and `DeleteDefaultDim` removes them again in `OnDelete`. The master record itself carries no `Dimension Set ID` field. +- **Master data** (a custom master table, e.g. "Course") persists **Default Dimension** records: each shortcut dimension field validates through `ValidateDimValueCode`, then the result is saved via `SaveDefaultDim`, and `DeleteDefaultDim` removes them again in `OnDelete`. Both `ValidateDimValueCode` and `SaveDefaultDim` take the shortcut dimension *number* (1-8, matching `General Ledger Setup`'s "Shortcut Dimension N Code" fields) as their first/third argument respectively — not the AL field ID of the table field being validated. The master record itself carries no `Dimension Set ID` field. - **Transactional/document data** (a custom document or journal-line table) carries a single **`Dimension Set ID`** field — a pointer to a shared, deduplicated set of dimension values in `Dimension Set Entry`, assembled from whatever the document inherited plus whatever the user overrode. A document does not acquire that ID by calling `SaveDefaultDim`; it builds a source list with `AddDimSource` (naming the related master table and its key, e.g. `Database::Customer`), then calls `GetDefaultDimID` to compute a new `Dimension Set ID` that inherits the master's Default Dimension records. Editing a shortcut dimension field directly on the document validates through `ValidateShortcutDimValues`, which updates the same `Dimension Set ID` in place rather than writing a separate Default Dimension record. Skipping the model that actually matches the table's kind produces a field that looks correct in the designer but silently fails to save, validate, or carry through to postings — or, for a document, one that never picks up the customer's/vendor's own dimensions at all. @@ -24,7 +24,7 @@ Skipping the model that actually matches the table's kind produces a field that For a master table, validate each shortcut dimension field through `ValidateDimValueCode`, save the result with `SaveDefaultDim`, and delete the matching Default Dimension records in `OnDelete`. -For a document table, when the field that attaches the document to a master record changes (e.g. `Customer No.`), call `AddDimSource` naming that master table and key, then `GetDefaultDimID` to compute the document's new `Dimension Set ID`, inheriting the master's Default Dimension records. Validate the document's own Shortcut Dimension fields through `ValidateShortcutDimValues`, which updates that same `Dimension Set ID` rather than persisting a separate Default Dimension record. +For a document table, when the field that attaches the document to a master record changes (e.g. `Customer No.`), call `AddDimSource` naming that master table and key, then `GetDefaultDimID` to compute the document's new `Dimension Set ID`, inheriting the master's Default Dimension records. Pass `0` for `GetDefaultDimID`'s `InheritFromDimSetID` argument in this case — passing the document's *existing* `Dimension Set ID` instead inherits whatever dimensions were already in it, so a value the previous linked record supplied can survive into the new one even where the new record has no default for that dimension. Validate the document's own Shortcut Dimension fields through `ValidateShortcutDimValues`, which updates that same `Dimension Set ID` in place rather than persisting a separate Default Dimension record. See sample: `dimension-management-wiring.good.al`. diff --git a/microsoft/knowledge/style/namespace-must-be-verified-from-source.bad.al b/microsoft/knowledge/style/namespace-must-be-verified-from-source.bad.al index f8006a1d..0cd54b72 100644 --- a/microsoft/knowledge/style/namespace-must-be-verified-from-source.bad.al +++ b/microsoft/knowledge/style/namespace-must-be-verified-from-source.bad.al @@ -6,7 +6,7 @@ codeunit 50100 "Tooling Extension" { procedure Run() var - ToolingPage: Page "Some Tooling Page"; // resolves in a local build, fails in VS Code + ToolingPage: Page "Some Tooling Page"; // guessed namespace; can still resolve against a stale or cached symbol package, then fail once checked against the object's current source or a freshly downloaded one begin ToolingPage.Run(); end; diff --git a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md index 64e89a4c..9ff1906a 100644 --- a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md +++ b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md @@ -13,7 +13,7 @@ application-area: [all] ## Description -A BC test company normally contains initialized system/setup data — an AL test suite should not assume an empty database, but it must be independent of unrelated business records: create the records and setup it owns rather than looking up a specific code, number, or name assumed to already exist, since that makes the test fail for reasons unrelated to the code under test. Every mandatory field on a created record also needs a value that respects its declared length; a partial setup that merely passes validation is not sufficient. +A BC test company normally contains initialized system/setup data — an AL test suite should not assume an empty database, but it must be independent of unrelated business records: create the records and setup it owns rather than looking up a specific code, number, or name assumed to already exist, since that makes the test fail for reasons unrelated to the code under test. Every mandatory field on a created record also needs an actual value — leaving one blank because setup-time validation happens to allow it produces a record that doesn't reflect a real one and can fail later, elsewhere in the flow (posting, a report, a later assertion), for a reason unrelated to what the test claims to check. A short-but-valid value is not itself a defect: AL field lengths are maxima, not minimums, so a two-character value in a `Text[100]` field is fine unless the scenario specifically depends on the field's length or shape — for example, a test that verifies truncation or a format check needs a value chosen to exercise that boundary, not an arbitrary short one. Not every value should be generated, though. Incidental fixture data — identifiers, names, descriptions — should generally come from the standard library codeunits rather than be tied to specific existing data. But values that materially define the scenario under test — amounts, quantities, percentages, dates, thresholds, rounding precision — should stay explicit and deliberately chosen, not randomized: a rounding test needs values placed deliberately around the rounding boundary, not a random one that might miss it entirely. @@ -25,6 +25,6 @@ See sample: `test-data-must-be-random-and-complete.good.al`. ## Anti Pattern -Looking up a record assumed to already exist (a hardcoded payment method or customer number) instead of creating it, or leaving mandatory fields empty or underfilled because validation happens to allow it. +Looking up a record assumed to already exist (a hardcoded payment method or customer number) instead of creating it, or leaving a mandatory field empty because setup-time validation happens to allow it. Also an anti-pattern, narrower: using a value that doesn't satisfy a scenario's explicit length or format requirement — for example a truncation test that never actually exceeds the field it's meant to overflow. See sample: `test-data-must-be-random-and-complete.bad.al`. diff --git a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al index c26364b7..dfd50b4e 100644 --- a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al +++ b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al @@ -15,6 +15,7 @@ page 50102 "Project Task API" repeater(General) { field(remainingHours; RemainingHoursCalc) { } + field(budgetedHours; Rec."Budgeted Hours") { } field(hoursUsed; Rec."Hours Used") { } } } diff --git a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md index 68cfe159..b209b73b 100644 --- a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md +++ b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md @@ -17,7 +17,7 @@ A stored field whose value is derived from other fields inside an `OnValidate` t ## Best Practice -Recalculate the derived value in `OnAfterGetRecord` from its authoritative source — typically a FlowField — using a page-level variable, and expose both the recalculated value and the source field so the consumer can verify it. +Recalculate the derived value in `OnAfterGetRecord` from its authoritative source — typically a FlowField — using a page-level variable, and expose that recalculated value instead of the stale stored field. Whether to also expose the source fields is a separate design decision, not a requirement of this pattern; keep the API contract scoped to what consumers actually need. If letting the consumer verify the recalculation is itself a requirement, expose every field the calculation reads, not just one of them — a derived value with two inputs needs both exposed, or the "verification" is incomplete. See sample: `stored-derived-fields-must-not-be-exposed-directly.good.al`. diff --git a/microsoft/skills/review/al-testing-review.md b/microsoft/skills/review/al-testing-review.md index c5c49984..de0c9be1 100644 --- a/microsoft/skills/review/al-testing-review.md +++ b/microsoft/skills/review/al-testing-review.md @@ -51,7 +51,7 @@ The following targeted checks cover every current `testing` article. Treat each - Test fixture code manually calls `Init`/`Insert`, invents keys or prerequisite records, or bypasses available `LibrarySales`, `LibraryPurchase`, `LibraryERM`, `LibraryInventory`, `LibraryRandom`, or equivalent library codeunits — `use-library-codeunits-for-test-fixtures`. - `asserterror` is added or changed without a following `Assert.ExpectedError`, `Assert.ExpectedErrorCode`, or a purpose-built assertion such as `ExpectedTestFieldError` — `asserterror-needs-expectederror-and-code`. - A test path raises UI and `[HandlerFunctions(...)]` does not match the invoked handlers, or the test has no meaningful evidence of the UI result (for example, it treats a Boolean set before the action as proof of success) — `ui-handlers-in-tests`. A capture/reset/assert-after-`RunModal` pattern is valid. Enqueue/dequeue and `AssertEmpty` are required only when order, count, text, replies, or a scripted sequence is part of the contract. Only nonoptional handlers have to execute: a listed handler declared `[SendNotificationHandler(true)]` or `[RecallNotificationHandler(true)]` is optional by design, so do not treat it as unmatched when the run never raises the notification. -- A test's `[GIVEN]`/setup looks up a hardcoded code/number/name assumed to already exist instead of creating it, leaves a mandatory field on a created record empty or under-sized, or a scenario-defining value (amount, quantity, percentage, date, threshold, rounding precision) is generated/randomized instead of an explicit chosen value — `test-data-must-be-random-and-complete`. Generating incidental fixture values (identifiers, names, descriptions) via the standard library codeunits is the compliant shape, not the signal to flag. +- A test's `[GIVEN]`/setup looks up a hardcoded code/number/name assumed to already exist instead of creating it, leaves a mandatory field on a created record empty, uses a value that doesn't satisfy the scenario's own explicit length/format requirement (for example a truncation test whose value never exceeds the field), or a scenario-defining value (amount, quantity, percentage, date, threshold, rounding precision) is generated/randomized instead of an explicit chosen value — `test-data-must-be-random-and-complete`. Generating incidental fixture values (identifiers, names, descriptions) via the standard library codeunits is the compliant shape, not the signal to flag, and neither is a short-but-valid value in an otherwise-unremarkable field. Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`. From b983a6da57904c6e4fe37d70a939794ebcf6eb61 Mon Sep 17 00:00:00 2001 From: Michael Dieringer <65093775+MichaelDieringer@users.noreply.github.com> Date: Mon, 21 Sep 2026 22:30:25 +0200 Subject: [PATCH 6/7] Fix remaining READ-convention sample links across this PR's 18 articles The same plain-backtick "See sample: \`x.good.al\`." form fixed on al-methods-limited-during-write-transactions (PR #161) turned up repo-wide on 15 more of this PR's articles - Knowledge-Retrieval.ps1 requires the markdown-link form to associate a sample with its article. All 16 fixed; the four local validators (frontmatter, knowledge-index, knowledge-retrieval, review-fixtures, skill-index) pass. --- microsoft/knowledge/appsource/file-datatype-saas.md | 4 ++-- microsoft/knowledge/breaking-changes/prefer-email-module.md | 4 ++-- .../knowledge/data-modeling/check-post-line-batch-pattern.md | 4 ++-- .../knowledge/data-modeling/dimension-management-wiring.md | 4 ++-- .../knowledge/data-modeling/do-not-change-primary-key.md | 4 ++-- .../knowledge/data-modeling/testfield-required-setup-field.md | 4 ++-- .../knowledge/performance/document-report-word-layout.md | 4 ++-- microsoft/knowledge/style/al-identifiers-english.md | 4 ++-- microsoft/knowledge/style/binary-choice-must-be-boolean.md | 4 ++-- .../style/fixed-choice-set-must-use-enum-not-integer.md | 4 ++-- .../style/intrinsic-al-functions-must-use-modern-casing.md | 4 ++-- .../knowledge/style/namespace-must-be-verified-from-source.md | 4 ++-- .../style/var-parameters-require-an-addressable-variable.md | 4 ++-- .../testing/test-data-must-be-random-and-complete.md | 4 ++-- .../api-page-key-fields-must-be-editable-on-insert.md | 4 ++-- .../stored-derived-fields-must-not-be-exposed-directly.md | 4 ++-- 16 files changed, 32 insertions(+), 32 deletions(-) diff --git a/microsoft/knowledge/appsource/file-datatype-saas.md b/microsoft/knowledge/appsource/file-datatype-saas.md index 0a601790..f46a5655 100644 --- a/microsoft/knowledge/appsource/file-datatype-saas.md +++ b/microsoft/knowledge/appsource/file-datatype-saas.md @@ -19,10 +19,10 @@ The classic `File` variable type — `Open`/`Create`/`Read`/`Write`/`Close` agai Use the stream-based equivalents: `UploadIntoStream` to read user-selected file content into an `InStream`, and `DownloadFromStream` to write an `OutStream`'s content to a file the user saves. Stage the content in a `TempBlob` between the stream and the rest of the parsing/formatting code. -See sample: `file-datatype-saas.good.al`. +See sample: [`file-datatype-saas.good.al`](file-datatype-saas.good.al). ## Anti Pattern Opening a hardcoded or user-supplied filesystem path with the `File` variable type. This is a strong signal the code was written for on-premises only, or copied from material that predates the cloud-first streaming APIs. -See sample: `file-datatype-saas.bad.al`. +See sample: [`file-datatype-saas.bad.al`](file-datatype-saas.bad.al). diff --git a/microsoft/knowledge/breaking-changes/prefer-email-module.md b/microsoft/knowledge/breaking-changes/prefer-email-module.md index 9219373b..6c04b37d 100644 --- a/microsoft/knowledge/breaking-changes/prefer-email-module.md +++ b/microsoft/knowledge/breaking-changes/prefer-email-module.md @@ -19,10 +19,10 @@ Older AL code sends email by calling `Codeunit Mail (397)`. Business Central's c Build on `Codeunit Email` and `Codeunit "Email Message"`. Route the message through an `Email Scenario` so different document types can use different accounts without the calling code needing to know which account that is, and get a tracked Sent/Outbox/Draft record for free. -See sample: `prefer-email-module.good.al`. +See sample: [`prefer-email-module.good.al`](prefer-email-module.good.al). ## Anti Pattern Calling `Codeunit Mail`'s `CreateMessage`. It still compiles and runs, but current `Codeunit Mail`'s own implementation of `CreateMessage` no longer sends anything by itself — it only raises integration events for a legacy subscriber to act on — so building new code on it means depending on whatever compatibility shim happens to still be wired up, with no first-class connector selection and no queryable Sent/Outbox/Draft record. `Send` and `GetErrorDesc` are not current members of `Codeunit Mail` at all; do not reference them. -See sample: `prefer-email-module.bad.al`. +See sample: [`prefer-email-module.bad.al`](prefer-email-module.bad.al). diff --git a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md index 5f2fbf61..e6b1229e 100644 --- a/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md +++ b/microsoft/knowledge/data-modeling/check-post-line-batch-pattern.md @@ -19,10 +19,10 @@ Business Central's own journal-based posting routines consistently follow a thre `Check Line` reads setup/dimension data only on its first call and shows no UI beyond errors. `Post Line` only operates on the record passed to it — never the Journal table — so it can be called directly by other posting code, including a document posting routine. `Post Batch` is the only one of the three that reads and updates the Journal table, and it is the only one invoked from the Post action on a journal page. A `-Post` document codeunit is never called directly from a page; a page calls a `-Post (Yes/No)` confirmation wrapper instead, so the same `-Post` codeunit can also run unattended from a batch-posting report. -See sample: `check-post-line-batch-pattern.good.al`. +See sample: [`check-post-line-batch-pattern.good.al`](check-post-line-batch-pattern.good.al). ## Anti Pattern A single monolithic posting codeunit that reads the Journal table, validates lines, writes ledger entries, and shows confirmation dialogs all in one procedure. It cannot be reused by another posting routine without fabricating journal records, and it cannot run unattended because it insists on user interaction. -See sample: `check-post-line-batch-pattern.bad.al`. +See sample: [`check-post-line-batch-pattern.bad.al`](check-post-line-batch-pattern.bad.al). diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.md b/microsoft/knowledge/data-modeling/dimension-management-wiring.md index 020c8ee8..3e6518c0 100644 --- a/microsoft/knowledge/data-modeling/dimension-management-wiring.md +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.md @@ -26,10 +26,10 @@ For a master table, validate each shortcut dimension field through `ValidateDimV For a document table, when the field that attaches the document to a master record changes (e.g. `Customer No.`), call `AddDimSource` naming that master table and key, then `GetDefaultDimID` to compute the document's new `Dimension Set ID`, inheriting the master's Default Dimension records. Pass `0` for `GetDefaultDimID`'s `InheritFromDimSetID` argument in this case — passing the document's *existing* `Dimension Set ID` instead inherits whatever dimensions were already in it, so a value the previous linked record supplied can survive into the new one even where the new record has no default for that dimension. Validate the document's own Shortcut Dimension fields through `ValidateShortcutDimValues`, which updates that same `Dimension Set ID` in place rather than persisting a separate Default Dimension record. -See sample: `dimension-management-wiring.good.al`. +See sample: [`dimension-management-wiring.good.al`](dimension-management-wiring.good.al). ## Anti Pattern Adding a dimension-looking field with only a `TableRelation` to Dimension Value, and no call into `DimensionManagement` at all. The field accepts input but never becomes a real Default Dimension record, so it does not validate against blocked values and does not flow into postings. -See sample: `dimension-management-wiring.bad.al`. +See sample: [`dimension-management-wiring.bad.al`](dimension-management-wiring.bad.al). diff --git a/microsoft/knowledge/data-modeling/do-not-change-primary-key.md b/microsoft/knowledge/data-modeling/do-not-change-primary-key.md index ffece5dc..37cb3e3e 100644 --- a/microsoft/knowledge/data-modeling/do-not-change-primary-key.md +++ b/microsoft/knowledge/data-modeling/do-not-change-primary-key.md @@ -19,10 +19,10 @@ Once a table has shipped — to AppSource, or to any customer environment that h Leave a published table's key exactly as shipped. Model a new discriminating dimension as a separate table with its own key instead of adding a field to the existing key, and branch orchestration code by the new dimension rather than filtering one shared table on an extra key field. -See sample: `do-not-change-primary-key.good.al`. +See sample: [`do-not-change-primary-key.good.al`](do-not-change-primary-key.good.al). ## Anti Pattern Adding a field to a published table's primary or clustered key to distinguish a new case. This fails AppSource validation or any customer upgrade with `AS0009` as soon as rows already exist under the old key shape, whether the field is being added, removed, or reordered. -See sample: `do-not-change-primary-key.bad.al`. +See sample: [`do-not-change-primary-key.bad.al`](do-not-change-primary-key.bad.al). diff --git a/microsoft/knowledge/data-modeling/testfield-required-setup-field.md b/microsoft/knowledge/data-modeling/testfield-required-setup-field.md index 2def2191..37ad5758 100644 --- a/microsoft/knowledge/data-modeling/testfield-required-setup-field.md +++ b/microsoft/knowledge/data-modeling/testfield-required-setup-field.md @@ -19,10 +19,10 @@ A procedure that reads a field from a setup or configuration table inside a bran Once a business rule has decided that a setup-table field's value is required for a branch to behave correctly, call `TestField` on it before use, even though a plain read would "work" by returning a blank or zero without erroring. Write a test that blanks the setup field and asserts the resulting error, so the guard itself is verified rather than merely present. -See sample: `testfield-required-setup-field.good.al`. +See sample: [`testfield-required-setup-field.good.al`](testfield-required-setup-field.good.al). ## Anti Pattern Reading a required setup-table field behind a presence check that falls through to a default value instead of erroring. This looks defensive because it never crashes, but it converts "administrator forgot to configure this" into "system silently did something else" — worse than a hard failure, because nobody is told anything went wrong. -See sample: `testfield-required-setup-field.bad.al`. +See sample: [`testfield-required-setup-field.bad.al`](testfield-required-setup-field.bad.al). diff --git a/microsoft/knowledge/performance/document-report-word-layout.md b/microsoft/knowledge/performance/document-report-word-layout.md index 158d7218..06159aad 100644 --- a/microsoft/knowledge/performance/document-report-word-layout.md +++ b/microsoft/knowledge/performance/document-report-word-layout.md @@ -25,10 +25,10 @@ For document reports, prefer a `Word` layout (`DefaultRenderingLayout = Word`) o - [Creating an RDL layout report](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-report-performance) - [Troubleshooting reports / Report performance](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-reports-troubleshooting) -See sample: `document-report-word-layout.good.al`. +See sample: [`document-report-word-layout.good.al`](document-report-word-layout.good.al). ## Anti Pattern Defaulting a document report's layout to RDLC out of habit or because a template happened to use it. This inherits RDLC's sandboxed-app-domain performance cost with no benefit tied to the report's actual content or calculation needs. -See sample: `document-report-word-layout.bad.al`. +See sample: [`document-report-word-layout.bad.al`](document-report-word-layout.bad.al). diff --git a/microsoft/knowledge/style/al-identifiers-english.md b/microsoft/knowledge/style/al-identifiers-english.md index 1b272dee..5feda9f8 100644 --- a/microsoft/knowledge/style/al-identifiers-english.md +++ b/microsoft/knowledge/style/al-identifiers-english.md @@ -19,10 +19,10 @@ All AL identifiers — variables, procedures, parameters, fields, object names, Write every identifier in English, and translate developer intent rather than transliterating it — when a requirement is described in another language, the resulting variable, procedure, and field names should still read as English. Captions and tooltips may carry target-language text in the source file, with locale translations managed through XLIFF. -See sample: `al-identifiers-english.good.al`. +See sample: [`al-identifiers-english.good.al`](al-identifiers-english.good.al). ## Anti Pattern Using native-language identifiers such as a Danish variable or procedure name in AL source code, relying on the fact that the code still compiles and runs correctly. This makes the code unreadable to non-native-language contributors and mixes localization concerns into source that should stay language-neutral. -See sample: `al-identifiers-english.bad.al`. +See sample: [`al-identifiers-english.bad.al`](al-identifiers-english.bad.al). diff --git a/microsoft/knowledge/style/binary-choice-must-be-boolean.md b/microsoft/knowledge/style/binary-choice-must-be-boolean.md index ad82146c..ec0c4a5e 100644 --- a/microsoft/knowledge/style/binary-choice-must-be-boolean.md +++ b/microsoft/knowledge/style/binary-choice-must-be-boolean.md @@ -19,10 +19,10 @@ When a field or variable represents a genuine true/false state — yes/no, on/of Type a field or variable as `Boolean` when the domain concept is inherently a true/false state. Do not replace a meaningful two-option domain model with a Boolean solely because it currently has two values. -See sample: `binary-choice-must-be-boolean.good.al`. +See sample: [`binary-choice-must-be-boolean.good.al`](binary-choice-must-be-boolean.good.al). ## Anti Pattern Modeling a yes/no choice as an `Option` with two members, or as an `Integer` with magic-number values, forces every caller to remember which value means what and leaves room for a meaningless third value. -See sample: `binary-choice-must-be-boolean.bad.al`. +See sample: [`binary-choice-must-be-boolean.bad.al`](binary-choice-must-be-boolean.bad.al). diff --git a/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md b/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md index f9adf179..112b977a 100644 --- a/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md +++ b/microsoft/knowledge/style/fixed-choice-set-must-use-enum-not-integer.md @@ -19,10 +19,10 @@ When a variable or field represents a fixed set of named, mutually exclusive sta Declare an `Enum` with named values and branch on the enum value, not a raw number. -See sample: `fixed-choice-set-must-use-enum-not-integer.good.al`. +See sample: [`fixed-choice-set-must-use-enum-not-integer.good.al`](fixed-choice-set-must-use-enum-not-integer.good.al). ## Anti Pattern Using a plain `Integer` field with the meaning of each value tracked only in a comment pushes the documentation of the states into something the compiler cannot check and a future maintainer cannot rely on. -See sample: `fixed-choice-set-must-use-enum-not-integer.bad.al`. +See sample: [`fixed-choice-set-must-use-enum-not-integer.bad.al`](fixed-choice-set-must-use-enum-not-integer.bad.al). diff --git a/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md b/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md index 632c9280..761650b3 100644 --- a/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md +++ b/microsoft/knowledge/style/intrinsic-al-functions-must-use-modern-casing.md @@ -19,10 +19,10 @@ AL is case-insensitive, so `MESSAGE(...)`, `ERROR(...)`, `CONFIRM(...)`, and `ST Call intrinsic functions in their modern, PascalCase form. -See sample: `intrinsic-al-functions-must-use-modern-casing.good.al`. +See sample: [`intrinsic-al-functions-must-use-modern-casing.good.al`](intrinsic-al-functions-must-use-modern-casing.good.al). ## Anti Pattern ALL-CAPS intrinsic function calls trip no compiler error, but they are a reliable signal that a code block was copied from old C/AL material or outdated training content rather than written against current AL conventions. -See sample: `intrinsic-al-functions-must-use-modern-casing.bad.al`. +See sample: [`intrinsic-al-functions-must-use-modern-casing.bad.al`](intrinsic-al-functions-must-use-modern-casing.bad.al). diff --git a/microsoft/knowledge/style/namespace-must-be-verified-from-source.md b/microsoft/knowledge/style/namespace-must-be-verified-from-source.md index 39d02103..52e8bbc1 100644 --- a/microsoft/knowledge/style/namespace-must-be-verified-from-source.md +++ b/microsoft/knowledge/style/namespace-must-be-verified-from-source.md @@ -19,10 +19,10 @@ Since Business Central 2024 release wave 1, Microsoft's own objects are organize When referencing an existing AL object, resolve its namespace from that object's actual source file or symbol definition — never infer or invent one from its name, functional area, or naming convention. -See sample: `namespace-must-be-verified-from-source.good.al`. +See sample: [`namespace-must-be-verified-from-source.good.al`](namespace-must-be-verified-from-source.good.al). ## Anti Pattern Writing a `using` statement from memory, from an incomplete path, or from a plausible-looking guess. It can appear correct while actually resolving to the wrong object, or fail to resolve, once checked against stale or mismatched symbols, a different build configuration, or the object's actual current source — not because the compiler and the AL Language Server apply different namespace-resolution rules; they don't. -See sample: `namespace-must-be-verified-from-source.bad.al`. +See sample: [`namespace-must-be-verified-from-source.bad.al`](namespace-must-be-verified-from-source.bad.al). diff --git a/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.md b/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.md index 485e5ac2..81f6fbb5 100644 --- a/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.md +++ b/microsoft/knowledge/style/var-parameters-require-an-addressable-variable.md @@ -19,10 +19,10 @@ When a procedure declares a parameter with `var`, that parameter is passed by re Declare a variable in the caller's scope and pass it to the `var` parameter. -See sample: `var-parameters-require-an-addressable-variable.good.al`. +See sample: [`var-parameters-require-an-addressable-variable.good.al`](var-parameters-require-an-addressable-variable.good.al). ## Anti Pattern Passing a literal or a computed expression to a `var` parameter position fails to compile, because neither has an address the callee can write back to. -See sample: `var-parameters-require-an-addressable-variable.bad.al`. +See sample: [`var-parameters-require-an-addressable-variable.bad.al`](var-parameters-require-an-addressable-variable.bad.al). diff --git a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md index 9ff1906a..c78a0d58 100644 --- a/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md +++ b/microsoft/knowledge/testing/test-data-must-be-random-and-complete.md @@ -21,10 +21,10 @@ Not every value should be generated, though. Incidental fixture data — identif Use the standard library codeunits (`Library - ERM`, `Library - Inventory`, `Library - Sales`, `Library - Utility`) to generate incidental fixture values — they produce valid, unique-enough data via number series and controlled randomness, not a mathematical collision-free guarantee — and fill every mandatory field with correctly-sized data. Keep values that define the scenario's expected outcome explicit and fixed. Reserve hardcoded values for tests that validate an external contract itself — a fixed JSON schema, an EDIFACT message, a counterparty code — where the hardcoded value documents the specification rather than arbitrary test logic. -See sample: `test-data-must-be-random-and-complete.good.al`. +See sample: [`test-data-must-be-random-and-complete.good.al`](test-data-must-be-random-and-complete.good.al). ## Anti Pattern Looking up a record assumed to already exist (a hardcoded payment method or customer number) instead of creating it, or leaving a mandatory field empty because setup-time validation happens to allow it. Also an anti-pattern, narrower: using a value that doesn't satisfy a scenario's explicit length or format requirement — for example a truncation test that never actually exceeds the field it's meant to overflow. -See sample: `test-data-must-be-random-and-complete.bad.al`. +See sample: [`test-data-must-be-random-and-complete.bad.al`](test-data-must-be-random-and-complete.bad.al). diff --git a/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.md b/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.md index aa83cc1a..84887c19 100644 --- a/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.md +++ b/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.md @@ -19,10 +19,10 @@ A field listed in `ODataKeyFields` cannot have `Editable = false` when the API p Leave every consumer-supplied key field referenced in `ODataKeyFields` without `Editable = false` on pages where `InsertAllowed = true`, so the OData layer accepts it as a writable property on POST. -See sample: `api-page-key-fields-must-be-editable-on-insert.good.al`. +See sample: [`api-page-key-fields-must-be-editable-on-insert.good.al`](api-page-key-fields-must-be-editable-on-insert.good.al). ## Anti Pattern Marking a consumer-provided key field `Editable = false`, out of habit or for perceived safety. This silently breaks create operations with a generic `BadRequest` instead of a clear validation error. -See sample: `api-page-key-fields-must-be-editable-on-insert.bad.al`. +See sample: [`api-page-key-fields-must-be-editable-on-insert.bad.al`](api-page-key-fields-must-be-editable-on-insert.bad.al). diff --git a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md index b209b73b..2d02f938 100644 --- a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md +++ b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.md @@ -19,10 +19,10 @@ A stored field whose value is derived from other fields inside an `OnValidate` t Recalculate the derived value in `OnAfterGetRecord` from its authoritative source — typically a FlowField — using a page-level variable, and expose that recalculated value instead of the stale stored field. Whether to also expose the source fields is a separate design decision, not a requirement of this pattern; keep the API contract scoped to what consumers actually need. If letting the consumer verify the recalculation is itself a requirement, expose every field the calculation reads, not just one of them — a derived value with two inputs needs both exposed, or the "verification" is incomplete. -See sample: `stored-derived-fields-must-not-be-exposed-directly.good.al`. +See sample: [`stored-derived-fields-must-not-be-exposed-directly.good.al`](stored-derived-fields-must-not-be-exposed-directly.good.al). ## Anti Pattern Exposing the stored field directly via `Rec`, trusting that it was kept in sync by whichever trigger last touched it. -See sample: `stored-derived-fields-must-not-be-exposed-directly.bad.al`. +See sample: [`stored-derived-fields-must-not-be-exposed-directly.bad.al`](stored-derived-fields-must-not-be-exposed-directly.bad.al). From 43c9d5319ca665b12948a45c1af5bf957b939bea Mon Sep 17 00:00:00 2001 From: Michael Dieringer <65093775+MichaelDieringer@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:32:35 +0200 Subject: [PATCH 7/7] Fix two merge-critical correctness issues from Jesper's 2026-09-22 review - api-page-key-fields-must-be-editable-on-insert.good.al and stored-derived-fields-must-not-be-exposed-directly.good.al: both were writable API pages missing DelayedInsert = true, contradicting this repo's own api-page-delayedinsert-true rule - the canonical "good" samples were teaching code BCQuality itself flags. - dimension-management-wiring.good.al: UpdateDimensionSetID exited early when Customer.Get failed, leaving the previous customer's shortcut dimension and Dimension Set ID in place - the same staleness bug the InheritFromDimSetID = 0 fix (from the prior review round) was meant to prevent, just triggered by a failed lookup instead of a successful one. Now clears the shortcut field and recomputes with an empty source list on a failed lookup too, so GetDefaultDimID correctly returns an empty Dimension Set ID instead of never running. --- .../dimension-management-wiring.good.al | 18 ++++++++++-------- .../dimension-management-wiring.md | 2 +- ...y-fields-must-be-editable-on-insert.good.al | 1 + ...fields-must-not-be-exposed-directly.good.al | 1 + 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al b/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al index 50178651..b5feae08 100644 --- a/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.good.al @@ -71,15 +71,17 @@ table 50101 "Course Registration Header" DefaultDimSource: List of [Dictionary of [Integer, Code[20]]]; GlobalDim2Code: Code[20]; begin - if not Customer.Get("Customer No.") then - exit; - - // Recompute from scratch (InheritFromDimSetID = 0): passing the existing - // "Dimension Set ID" here would inherit dimensions from whichever record - // the document was previously linked to, retaining them even after the - // new customer's defaults have nothing for that dimension. + // Recompute from scratch (InheritFromDimSetID = 0) whether or not the + // customer lookup succeeds. Passing the existing "Dimension Set ID" + // here would inherit dimensions from whichever record the document + // was previously linked to, and exiting early on a failed Get would + // leave that same stale data in place — both defeat the point of + // this procedure. Clearing the shortcut field and recomputing with + // an empty source list (when the customer doesn't exist) correctly + // clears the document's dimensions instead of leaving old ones. "Shortcut Dimension 1 Code" := ''; - DimMgt.AddDimSource(DefaultDimSource, Database::Customer, "Customer No."); + if Customer.Get("Customer No.") then + DimMgt.AddDimSource(DefaultDimSource, Database::Customer, "Customer No."); "Dimension Set ID" := DimMgt.GetDefaultDimID( DefaultDimSource, '', "Shortcut Dimension 1 Code", GlobalDim2Code, 0, 0); diff --git a/microsoft/knowledge/data-modeling/dimension-management-wiring.md b/microsoft/knowledge/data-modeling/dimension-management-wiring.md index 3e6518c0..d7eec388 100644 --- a/microsoft/knowledge/data-modeling/dimension-management-wiring.md +++ b/microsoft/knowledge/data-modeling/dimension-management-wiring.md @@ -24,7 +24,7 @@ Skipping the model that actually matches the table's kind produces a field that For a master table, validate each shortcut dimension field through `ValidateDimValueCode`, save the result with `SaveDefaultDim`, and delete the matching Default Dimension records in `OnDelete`. -For a document table, when the field that attaches the document to a master record changes (e.g. `Customer No.`), call `AddDimSource` naming that master table and key, then `GetDefaultDimID` to compute the document's new `Dimension Set ID`, inheriting the master's Default Dimension records. Pass `0` for `GetDefaultDimID`'s `InheritFromDimSetID` argument in this case — passing the document's *existing* `Dimension Set ID` instead inherits whatever dimensions were already in it, so a value the previous linked record supplied can survive into the new one even where the new record has no default for that dimension. Validate the document's own Shortcut Dimension fields through `ValidateShortcutDimValues`, which updates that same `Dimension Set ID` in place rather than persisting a separate Default Dimension record. +For a document table, when the field that attaches the document to a master record changes (e.g. `Customer No.`), call `AddDimSource` naming that master table and key, then `GetDefaultDimID` to compute the document's new `Dimension Set ID`, inheriting the master's Default Dimension records. Pass `0` for `GetDefaultDimID`'s `InheritFromDimSetID` argument in this case — passing the document's *existing* `Dimension Set ID` instead inherits whatever dimensions were already in it, so a value the previous linked record supplied can survive into the new one even where the new record has no default for that dimension. Run this same recompute — clear the shortcut field, call `GetDefaultDimID` with no source added — when the lookup on the new key fails (blank or an invalid value), too: exiting early instead leaves the previous record's dimensions in place, which is the same staleness bug the `InheritFromDimSetID = 0` rule exists to prevent. Validate the document's own Shortcut Dimension fields through `ValidateShortcutDimValues`, which updates that same `Dimension Set ID` in place rather than persisting a separate Default Dimension record. See sample: [`dimension-management-wiring.good.al`](dimension-management-wiring.good.al). diff --git a/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.good.al b/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.good.al index 8506ae66..af6c5de1 100644 --- a/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.good.al +++ b/microsoft/knowledge/web-services/api-page-key-fields-must-be-editable-on-insert.good.al @@ -9,6 +9,7 @@ page 50101 "Customer Info API" SourceTable = Customer; ODataKeyFields = "No."; InsertAllowed = true; + DelayedInsert = true; layout { diff --git a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al index dfd50b4e..1ab78dbc 100644 --- a/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al +++ b/microsoft/knowledge/web-services/stored-derived-fields-must-not-be-exposed-directly.good.al @@ -7,6 +7,7 @@ page 50102 "Project Task API" EntityName = 'projectTask'; EntitySetName = 'projectTasks'; SourceTable = "Project Task"; + DelayedInsert = true; layout {