From e570d6113fdcce32492c398fbf090b5efe389bef Mon Sep 17 00:00:00 2001 From: Jesper Schulz-Wedde Date: Thu, 23 Apr 2026 16:43:42 +0200 Subject: [PATCH] Extract 55 knowledge articles from BC review-agent prompt Adds 55 articles (plus 76 code samples) spanning four new domains and two existing domains, extracted from the internal Business Central review-agent prompt. Content was filtered against BCQuality's remedial-knowledge premise: each article encodes BC-specific behaviour, a CodeCop rule, a platform API semantic, or an anti-false-positive guideline that a capable LLM would otherwise get wrong. New domains: - privacy (11 articles): DataClassification inheritance semantics, the StrSubstNo-defeats-Error-telemetry-classification pitfall, Privacy Notice consent for outgoing requests, anti-false-positives for pages and in-memory data. - upgrade (11 articles): upgrade-codeunit structure, upgrade-tag lifecycle and registration, protected DB reads, DataTransfer for large datasets, InitValue semantics, enum-ordinal preservation, obsolete-workflow, first-install detection. - ui (9 articles): caption capitalization by phrase type, tooltip voice, teaching-tip vs tooltip, tour-tip conventions, character limits, banned terms, ampersand handling, title punctuation. - style (11 articles): label-suffix convention, API page naming, temporary-variable prefix, label properties (Comment/Locked), named invocations, FieldCaption in user messages, OptionCaption pairing, Error-parameter passing, `this` keyword, required parentheses, file naming. Gaps in existing domains: - performance (11 articles): production-scale table catalog (no row counts, per internal-data concern), anti-false-positive for bounded tables, guard-before-Get ordering, redundant-Get-in-OnAfterGetRecord, LockTable in read-only helpers, combined ModifyAll passes, writes in OnAfterGetRecord, SetLoadFields heuristics, temporary-table regressions, FlowField source-table widening, MaintainSQLIndex disabling SIFT. - security (2 articles): environment-specific hardcoded GUIDs, ValidateTableRelation=false on user input. Intentionally excluded: specific production P95 row-count numbers (aggregated internal telemetry); rewritten as categorical guidance on which tables to treat as production-scale without publishing sizes. All articles use `bc-version: [all]` (applies to every BC version, per the new schema sentinel). Validator passes with 0 errors / 0 warnings. --- .../combine-multiple-modifyall-calls.bad.al | 13 ++++++ .../combine-multiple-modifyall-calls.good.al | 16 +++++++ .../combine-multiple-modifyall-calls.md | 26 +++++++++++ ...-not-flag-performance-on-bounded-tables.md | 22 ++++++++++ ...-modify-records-in-onaftergetrecord.bad.al | 15 +++++++ ...modify-records-in-onaftergetrecord.good.al | 35 +++++++++++++++ ...-not-modify-records-in-onaftergetrecord.md | 26 +++++++++++ ...-re-get-rec-inside-onaftergetrecord.bad.al | 34 +++++++++++++++ ...re-get-rec-inside-onaftergetrecord.good.al | 30 +++++++++++++ ...-not-re-get-rec-inside-onaftergetrecord.md | 26 +++++++++++ ...-flowfield-calcformula-to-larger-tables.md | 22 ++++++++++ .../guard-before-get-not-after.bad.al | 15 +++++++ .../guard-before-get-not-after.good.al | 16 +++++++ .../performance/guard-before-get-not-after.md | 26 +++++++++++ ...letemporary-on-api-and-background-pages.md | 22 ++++++++++ .../maintainsqlindex-false-disables-sift.md | 22 ++++++++++ ...fields-on-narrow-tables-and-short-loops.md | 22 ++++++++++ ...-and-write-paths-to-avoid-locktable.bad.al | 14 ++++++ ...and-write-paths-to-avoid-locktable.good.al | 17 ++++++++ ...only-and-write-paths-to-avoid-locktable.md | 26 +++++++++++ ...ledger-entry-tables-as-production-scale.md | 22 ++++++++++ ...lassification-is-a-table-field-property.md | 22 ++++++++++ ...om-isolated-storage-to-plain-fields.bad.al | 13 ++++++ ...m-isolated-storage-to-plain-fields.good.al | 10 +++++ ...i-from-isolated-storage-to-plain-fields.md | 26 +++++++++++ ...-logged-to-telemetry-message-is-not.bad.al | 16 +++++++ ...logged-to-telemetry-message-is-not.good.al | 15 +++++++ ...r-is-logged-to-telemetry-message-is-not.md | 26 +++++++++++ .../flowfields-auto-inherit-systemmetadata.md | 22 ++++++++++ ...in-memory-data-is-not-a-privacy-concern.md | 22 ++++++++++ ...erited-dataclassification-per-field.bad.al | 18 ++++++++ ...rited-dataclassification-per-field.good.al | 23 ++++++++++ ...-inherited-dataclassification-per-field.md | 26 +++++++++++ ...permitted-data-is-not-a-privacy-concern.md | 22 ++++++++++ ...ce-consent-before-outgoing-requests.bad.al | 14 ++++++ ...e-consent-before-outgoing-requests.good.al | 20 +++++++++ ...notice-consent-before-outgoing-requests.md | 26 +++++++++++ ...e-getlasterrortext-before-telemetry.bad.al | 16 +++++++ ...-getlasterrortext-before-telemetry.good.al | 15 +++++++ ...itize-getlasterrortext-before-telemetry.md | 26 +++++++++++ ...ssification-on-every-telemetry-call.bad.al | 16 +++++++ ...sification-on-every-telemetry-call.good.al | 17 ++++++++ ...aclassification-on-every-telemetry-call.md | 26 +++++++++++ ...eaks-error-telemetry-classification.bad.al | 14 ++++++ ...aks-error-telemetry-classification.good.al | 11 +++++ ...d-breaks-error-telemetry-classification.md | 26 +++++++++++ ...validatetablerelation-on-user-input.bad.al | 16 +++++++ ...alidatetablerelation-on-user-input.good.al | 24 +++++++++++ ...ble-validatetablerelation-on-user-input.md | 26 +++++++++++ ...hardcode-environment-specific-guids.bad.al | 14 ++++++ ...ardcode-environment-specific-guids.good.al | 16 +++++++ ...not-hardcode-environment-specific-guids.md | 26 +++++++++++ .../apply-approved-label-suffixes.bad.al | 17 ++++++++ .../apply-approved-label-suffixes.good.al | 20 +++++++++ .../style/apply-approved-label-suffixes.md | 26 +++++++++++ .../style/follow-api-page-naming-rules.bad.al | 22 ++++++++++ .../follow-api-page-naming-rules.good.al | 25 +++++++++++ .../style/follow-api-page-naming-rules.md | 26 +++++++++++ ...comment-on-labels-with-placeholders.bad.al | 15 +++++++ ...omment-on-labels-with-placeholders.good.al | 14 ++++++ ...ude-comment-on-labels-with-placeholders.md | 26 +++++++++++ ...ptioncaption-count-to-optionmembers.bad.al | 22 ++++++++++ ...tioncaption-count-to-optionmembers.good.al | 21 +++++++++ ...ch-optioncaption-count-to-optionmembers.md | 26 +++++++++++ .../name-files-as-object-dot-type-dot-al.md | 22 ++++++++++ ...ers-directly-to-error-no-strsubstno.bad.al | 13 ++++++ ...rs-directly-to-error-no-strsubstno.good.al | 11 +++++ ...ameters-directly-to-error-no-strsubstno.md | 26 +++++++++++ ...emporary-record-variables-with-temp.bad.al | 17 ++++++++ ...mporary-record-variables-with-temp.good.al | 16 +++++++ ...ix-temporary-record-variables-with-temp.md | 26 +++++++++++ ...quire-parentheses-on-function-calls.bad.al | 13 ++++++ ...uire-parentheses-on-function-calls.good.al | 12 ++++++ .../require-parentheses-on-function-calls.md | 26 +++++++++++ ...n-and-tablecaption-in-user-messages.bad.al | 12 ++++++ ...-and-tablecaption-in-user-messages.good.al | 13 ++++++ ...ption-and-tablecaption-in-user-messages.md | 26 +++++++++++ ...d-invocations-instead-of-object-ids.bad.al | 10 +++++ ...-invocations-instead-of-object-ids.good.al | 9 ++++ ...named-invocations-instead-of-object-ids.md | 26 +++++++++++ .../use-this-keyword-in-codeunits.bad.al | 15 +++++++ .../use-this-keyword-in-codeunits.good.al | 21 +++++++++ .../style/use-this-keyword-in-codeunits.md | 26 +++++++++++ ...-are-imperative-and-end-with-period.bad.al | 26 +++++++++++ ...are-imperative-and-end-with-period.good.al | 25 +++++++++++ ...tips-are-imperative-and-end-with-period.md | 26 +++++++++++ .../knowledge/ui/avoid-banned-ui-terms.md | 22 ++++++++++ ...tion-noun-phrase-vs-sentence-phrase.bad.al | 21 +++++++++ ...ion-noun-phrase-vs-sentence-phrase.good.al | 27 ++++++++++++ ...lization-noun-phrase-vs-sentence-phrase.md | 26 +++++++++++ ...-with-specifies-and-end-with-period.bad.al | 26 +++++++++++ ...with-specifies-and-end-with-period.good.al | 25 +++++++++++ ...tart-with-specifies-and-end-with-period.md | 26 +++++++++++ .../ui/respect-ui-text-character-limits.md | 22 ++++++++++ .../ui/titles-have-no-trailing-punctuation.md | 22 ++++++++++ .../tooltips-describe-teaching-tips-guide.md | 22 ++++++++++ .../tour-tips-do-not-use-action-language.md | 22 ++++++++++ ...se-and-not-ampersand-in-ui-captions.bad.al | 20 +++++++++ ...e-and-not-ampersand-in-ui-captions.good.al | 19 ++++++++ .../use-and-not-ampersand-in-ui-captions.md | 26 +++++++++++ ...-onupgrade-triggers-not-inline-code.bad.al | 14 ++++++ ...onupgrade-triggers-not-inline-code.good.al | 31 +++++++++++++ ...from-onupgrade-triggers-not-inline-code.md | 26 +++++++++++ ...-first-install-via-dataversion-zero.bad.al | 15 +++++++ ...first-install-via-dataversion-zero.good.al | 20 +++++++++ ...tect-first-install-via-dataversion-zero.md | 26 +++++++++++ ...ake-external-calls-in-upgrade-codeunits.md | 22 ++++++++++ ...changes-must-be-additive-at-the-end.bad.al | 23 ++++++++++ ...hanges-must-be-additive-at-the-end.good.al | 29 +++++++++++++ ...num-changes-must-be-additive-at-the-end.md | 26 +++++++++++ ...-database-read-in-upgrade-codeunits.bad.al | 19 ++++++++ ...database-read-in-upgrade-codeunits.good.al | 23 ++++++++++ ...very-database-read-in-upgrade-codeunits.md | 26 +++++++++++ ...-does-not-populate-existing-records.bad.al | 14 ++++++ ...does-not-populate-existing-records.good.al | 43 +++++++++++++++++++ ...alue-does-not-populate-existing-records.md | 26 +++++++++++ ...ister-upgrade-tags-with-subscribers.bad.al | 22 ++++++++++ ...ster-upgrade-tags-with-subscribers.good.al | 25 +++++++++++ .../register-upgrade-tags-with-subscribers.md | 26 +++++++++++ ...sential-work-during-upgrade-context.bad.al | 14 ++++++ ...ential-work-during-upgrade-context.good.al | 15 +++++++ ...n-essential-work-during-upgrade-context.md | 26 +++++++++++ ...er-for-large-dataset-initialization.bad.al | 22 ++++++++++ ...r-for-large-dataset-initialization.good.al | 31 +++++++++++++ ...ansfer-for-large-dataset-initialization.md | 26 +++++++++++ ...use-obsolete-pending-before-removed.bad.al | 11 +++++ ...se-obsolete-pending-before-removed.good.al | 13 ++++++ .../use-obsolete-pending-before-removed.md | 26 +++++++++++ ...use-upgrade-tags-not-version-checks.bad.al | 27 ++++++++++++ ...se-upgrade-tags-not-version-checks.good.al | 26 +++++++++++ .../use-upgrade-tags-not-version-checks.md | 26 +++++++++++ 131 files changed, 2799 insertions(+) create mode 100644 microsoft/knowledge/performance/combine-multiple-modifyall-calls.bad.al create mode 100644 microsoft/knowledge/performance/combine-multiple-modifyall-calls.good.al create mode 100644 microsoft/knowledge/performance/combine-multiple-modifyall-calls.md create mode 100644 microsoft/knowledge/performance/do-not-flag-performance-on-bounded-tables.md create mode 100644 microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.bad.al create mode 100644 microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.good.al create mode 100644 microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.md create mode 100644 microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.bad.al create mode 100644 microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.good.al create mode 100644 microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.md create mode 100644 microsoft/knowledge/performance/do-not-retarget-flowfield-calcformula-to-larger-tables.md create mode 100644 microsoft/knowledge/performance/guard-before-get-not-after.bad.al create mode 100644 microsoft/knowledge/performance/guard-before-get-not-after.good.al create mode 100644 microsoft/knowledge/performance/guard-before-get-not-after.md create mode 100644 microsoft/knowledge/performance/keep-sourcetabletemporary-on-api-and-background-pages.md create mode 100644 microsoft/knowledge/performance/maintainsqlindex-false-disables-sift.md create mode 100644 microsoft/knowledge/performance/skip-setloadfields-on-narrow-tables-and-short-loops.md create mode 100644 microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.bad.al create mode 100644 microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.good.al create mode 100644 microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.md create mode 100644 microsoft/knowledge/performance/treat-ledger-entry-tables-as-production-scale.md create mode 100644 microsoft/knowledge/privacy/dataclassification-is-a-table-field-property.md create mode 100644 microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.bad.al create mode 100644 microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.good.al create mode 100644 microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.md create mode 100644 microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.bad.al create mode 100644 microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.good.al create mode 100644 microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.md create mode 100644 microsoft/knowledge/privacy/flowfields-auto-inherit-systemmetadata.md create mode 100644 microsoft/knowledge/privacy/in-memory-data-is-not-a-privacy-concern.md create mode 100644 microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.bad.al create mode 100644 microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.good.al create mode 100644 microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.md create mode 100644 microsoft/knowledge/privacy/pages-displaying-permitted-data-is-not-a-privacy-concern.md create mode 100644 microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.bad.al create mode 100644 microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.good.al create mode 100644 microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.md create mode 100644 microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.bad.al create mode 100644 microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.good.al create mode 100644 microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.md create mode 100644 microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.bad.al create mode 100644 microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.good.al create mode 100644 microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.md create mode 100644 microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.bad.al create mode 100644 microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.good.al create mode 100644 microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.md create mode 100644 microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.bad.al create mode 100644 microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.good.al create mode 100644 microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.md create mode 100644 microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.bad.al create mode 100644 microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.good.al create mode 100644 microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.md create mode 100644 microsoft/knowledge/style/apply-approved-label-suffixes.bad.al create mode 100644 microsoft/knowledge/style/apply-approved-label-suffixes.good.al create mode 100644 microsoft/knowledge/style/apply-approved-label-suffixes.md create mode 100644 microsoft/knowledge/style/follow-api-page-naming-rules.bad.al create mode 100644 microsoft/knowledge/style/follow-api-page-naming-rules.good.al create mode 100644 microsoft/knowledge/style/follow-api-page-naming-rules.md create mode 100644 microsoft/knowledge/style/include-comment-on-labels-with-placeholders.bad.al create mode 100644 microsoft/knowledge/style/include-comment-on-labels-with-placeholders.good.al create mode 100644 microsoft/knowledge/style/include-comment-on-labels-with-placeholders.md create mode 100644 microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.bad.al create mode 100644 microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.good.al create mode 100644 microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.md create mode 100644 microsoft/knowledge/style/name-files-as-object-dot-type-dot-al.md create mode 100644 microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.bad.al create mode 100644 microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.good.al create mode 100644 microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.md create mode 100644 microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.bad.al create mode 100644 microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.good.al create mode 100644 microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.md create mode 100644 microsoft/knowledge/style/require-parentheses-on-function-calls.bad.al create mode 100644 microsoft/knowledge/style/require-parentheses-on-function-calls.good.al create mode 100644 microsoft/knowledge/style/require-parentheses-on-function-calls.md create mode 100644 microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.bad.al create mode 100644 microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.good.al create mode 100644 microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.md create mode 100644 microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.bad.al create mode 100644 microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.good.al create mode 100644 microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.md create mode 100644 microsoft/knowledge/style/use-this-keyword-in-codeunits.bad.al create mode 100644 microsoft/knowledge/style/use-this-keyword-in-codeunits.good.al create mode 100644 microsoft/knowledge/style/use-this-keyword-in-codeunits.md create mode 100644 microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.bad.al create mode 100644 microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.good.al create mode 100644 microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.md create mode 100644 microsoft/knowledge/ui/avoid-banned-ui-terms.md create mode 100644 microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.bad.al create mode 100644 microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.good.al create mode 100644 microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.md create mode 100644 microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.bad.al create mode 100644 microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.good.al create mode 100644 microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.md create mode 100644 microsoft/knowledge/ui/respect-ui-text-character-limits.md create mode 100644 microsoft/knowledge/ui/titles-have-no-trailing-punctuation.md create mode 100644 microsoft/knowledge/ui/tooltips-describe-teaching-tips-guide.md create mode 100644 microsoft/knowledge/ui/tour-tips-do-not-use-action-language.md create mode 100644 microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.bad.al create mode 100644 microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.good.al create mode 100644 microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.md create mode 100644 microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.bad.al create mode 100644 microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.good.al create mode 100644 microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.md create mode 100644 microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.bad.al create mode 100644 microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.good.al create mode 100644 microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.md create mode 100644 microsoft/knowledge/upgrade/do-not-make-external-calls-in-upgrade-codeunits.md create mode 100644 microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.bad.al create mode 100644 microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.good.al create mode 100644 microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.md create mode 100644 microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.bad.al create mode 100644 microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.good.al create mode 100644 microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.md create mode 100644 microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.bad.al create mode 100644 microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.good.al create mode 100644 microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.md create mode 100644 microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.bad.al create mode 100644 microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.good.al create mode 100644 microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.md create mode 100644 microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.bad.al create mode 100644 microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.good.al create mode 100644 microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.md create mode 100644 microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.bad.al create mode 100644 microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.good.al create mode 100644 microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.md create mode 100644 microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.bad.al create mode 100644 microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.good.al create mode 100644 microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.md create mode 100644 microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.bad.al create mode 100644 microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.good.al create mode 100644 microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.md diff --git a/microsoft/knowledge/performance/combine-multiple-modifyall-calls.bad.al b/microsoft/knowledge/performance/combine-multiple-modifyall-calls.bad.al new file mode 100644 index 0000000..3a9e190 --- /dev/null +++ b/microsoft/knowledge/performance/combine-multiple-modifyall-calls.bad.al @@ -0,0 +1,13 @@ +codeunit 51207 "Perf Sample CombineMA Bad" +{ + procedure UpdateTolerance(DocumentNo: Code[20]; ToleranceAmount: Decimal) + var + CustLedgerEntry: Record "Cust. Ledger Entry"; + begin + CustLedgerEntry.SetRange("Document No.", DocumentNo); + CustLedgerEntry.SetRange(Open, true); + // Two scans over the same filtered rows on a 10M-row ledger table. + CustLedgerEntry.ModifyAll("Accepted Payment Tolerance", ToleranceAmount); + CustLedgerEntry.ModifyAll("Accepted Pmt. Disc. Tolerance", false); + end; +} diff --git a/microsoft/knowledge/performance/combine-multiple-modifyall-calls.good.al b/microsoft/knowledge/performance/combine-multiple-modifyall-calls.good.al new file mode 100644 index 0000000..e1ecaad --- /dev/null +++ b/microsoft/knowledge/performance/combine-multiple-modifyall-calls.good.al @@ -0,0 +1,16 @@ +codeunit 51206 "Perf Sample CombineMA Good" +{ + procedure UpdateTolerance(DocumentNo: Code[20]; ToleranceAmount: Decimal) + var + CustLedgerEntry: Record "Cust. Ledger Entry"; + begin + CustLedgerEntry.SetRange("Document No.", DocumentNo); + CustLedgerEntry.SetRange(Open, true); + if CustLedgerEntry.FindSet(true) then + repeat + CustLedgerEntry."Accepted Payment Tolerance" := ToleranceAmount; + CustLedgerEntry."Accepted Pmt. Disc. Tolerance" := false; + CustLedgerEntry.Modify(false); + until CustLedgerEntry.Next() = 0; + end; +} diff --git a/microsoft/knowledge/performance/combine-multiple-modifyall-calls.md b/microsoft/knowledge/performance/combine-multiple-modifyall-calls.md new file mode 100644 index 0000000..21c32ff --- /dev/null +++ b/microsoft/knowledge/performance/combine-multiple-modifyall-calls.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: performance +keywords: [modifyall, bulk-update, filter, scan, recordset] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Combine multiple ModifyAll calls on the same recordset into a single pass + +## Description + +`ModifyAll(Field, Value)` issues a SQL UPDATE against every row matching the record variable's current filters, setting one field. Calling it twice on the same filtered recordset — once per field to update — produces two separate UPDATE statements, each of which has to re-locate the matching rows through the index. On a ledger-entry-scale table with ten million rows and a filter that matches a thousand, the overhead is not a doubling of the update cost but a doubling of the more expensive row-location cost. A single `FindSet(true)` + set-by-set assignment + `Modify(false)` completes both field changes in one pass. + +## Best Practice + +When more than one field needs to change on the same filtered recordset, iterate once with `FindSet(true)` and assign all fields per row. Reserve ModifyAll for the case where a single field change covers the whole update. If the filter set is truly huge and the trigger behaviour differs between fields, consider splitting with concrete evidence — otherwise the single-pass loop wins. + +See sample: `combine-multiple-modifyall-calls.good.al`. + +## Anti Pattern + +Applying `SetRange` against `CustLedgerEntry` on `"Document No."` and then calling `ModifyAll("Accepted Payment Tolerance", ...)` followed by `ModifyAll("Accepted Pmt. Disc. Tolerance", false)` — two scans over the same filtered rows. On Cust. Ledger Entry with production-scale data the redundant second scan is the dominant cost. + +See sample: `combine-multiple-modifyall-calls.bad.al`. diff --git a/microsoft/knowledge/performance/do-not-flag-performance-on-bounded-tables.md b/microsoft/knowledge/performance/do-not-flag-performance-on-bounded-tables.md new file mode 100644 index 0000000..216e727 --- /dev/null +++ b/microsoft/knowledge/performance/do-not-flag-performance-on-bounded-tables.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: performance +keywords: [setup-table, temporary, bounded-table, metadata, migration, false-positive] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Do not flag performance on inherently bounded tables + +## Description + +Several categories of Business Central tables are so small, so rarely accessed, or so in-memory that performance heuristics that make sense on Item Ledger Entry produce noise when applied to them. Temporary records (`TableType = Temporary`, `SourceTableTemporary = true`) live in memory and any access pattern is fast. Singleton setup tables (`Sales & Receivables Setup`, `General Ledger Setup`, `*Setup` tables generally) hold one row per company. Small bounded tables — enum mappings, permission objects, Role IDs — count in the dozens. System metadata tables (`TableMetadata`, `Field`, `AllObjWithCaption`) are bounded by the object catalog. Admin, Migration, Setup, Wizard, and Hybrid* pages are used infrequently with small datasets. + +## Best Practice + +Skip performance findings on these categories unless the code is specifically pathological (unbounded loop that multiplies cost non-linearly). A missing SetLoadFields on a singleton Setup table is not a finding. A Count on a 30-row permission mapping is not a finding. An admin page that iterates a bounded list once per invocation is not a finding. Reserving reviewer attention for the tables where it matters is half the value of the heuristics — noise on bounded tables trains authors to ignore the signal. + +## Anti Pattern + +Flagging `SalesReceivablesSetup.Get()` followed by `SetLoadFields()` on a handful of fields as "missing partial record optimization". Flagging a `FindSet` + loop on `Role ID` mapping because the loop has no SetCurrentKey. Flagging a Migration codeunit for writing many records, when the entire migration runs once per customer. All three burn author attention on cases that are not regressions. diff --git a/microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.bad.al b/microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.bad.al new file mode 100644 index 0000000..fbf5047 --- /dev/null +++ b/microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.bad.al @@ -0,0 +1,15 @@ +pageextension 51209 "Perf Sample NoModifyOAGR Bad" extends "Customer List" +{ + trigger OnAfterGetRecord() + begin + // Every scroll writes to the database. Every OnModify subscriber on + // Customer fires alongside. Write volume scales with mouse-wheel speed. + Rec."Last Warning Flag" := CalcWarning(); + Rec.Modify(); + end; + + local procedure CalcWarning(): Boolean + begin + exit(false); + end; +} diff --git a/microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.good.al b/microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.good.al new file mode 100644 index 0000000..8e4d641 --- /dev/null +++ b/microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.good.al @@ -0,0 +1,35 @@ +page 51208 "Perf Sample NoModifyOAGR Good" +{ + PageType = List; + SourceTable = Customer; + + layout + { + area(Content) + { + repeater(Group) + { + field("No."; Rec."No.") { ApplicationArea = All; } + field(WarningFlag; ShowWarning) + { + ApplicationArea = All; + Caption = 'Warning'; + } + } + } + } + + trigger OnAfterGetRecord() + begin + // Page-local variable. No database write per row. + ShowWarning := CalcWarning(Rec); + end; + + var + ShowWarning: Boolean; + + local procedure CalcWarning(var Customer: Record Customer): Boolean + begin + exit(false); + end; +} diff --git a/microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.md b/microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.md new file mode 100644 index 0000000..8133229 --- /dev/null +++ b/microsoft/knowledge/performance/do-not-modify-records-in-onaftergetrecord.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: performance +keywords: [onaftergetrecord, modify, page, trigger, write-per-scroll] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Do not Modify records inside OnAfterGetRecord + +## Description + +`OnAfterGetRecord` fires for every row the page or repeater renders. On a list page the user scrolls through, the trigger runs hundreds of times per second. A `Modify()` call inside the trigger writes to the database for every row scrolled past — the user's mouse wheel generates the write storm, and the effect compounds with every other subscriber that reacts to the OnModify event. The database activity is usually invisible to the author in development, because the list page loads ten rows; on a production tenant scrolling through thousands of rows, the page becomes the top source of write volume. + +## Best Practice + +Derive display-only state into a page-level variable and bind that variable to the field control instead of writing to `Rec`. If the computed value is genuinely a stored attribute of the record, compute it once at the authoring site (OnValidate, OnInsert) and display the stored value on the list — do not recompute and rewrite on every render. + +See sample: `do-not-modify-records-in-onaftergetrecord.good.al`. + +## Anti Pattern + +An OnAfterGetRecord body that assigns a computed value to `Rec."Warning Flag"` and calls `Rec.Modify()` so the flag persists. The write fires per scroll, per user, per second — and every subscriber on the Rec's OnModify fires alongside. + +See sample: `do-not-modify-records-in-onaftergetrecord.bad.al`. diff --git a/microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.bad.al b/microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.bad.al new file mode 100644 index 0000000..0a5469a --- /dev/null +++ b/microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.bad.al @@ -0,0 +1,34 @@ +page 51203 "Perf Sample ReGetRec Bad" +{ + PageType = List; + SourceTable = "Assembly Line"; + + layout + { + area(Content) + { + repeater(Group) + { + field("No."; Rec."No.") { ApplicationArea = All; } + } + } + } + + trigger OnAfterGetRecord() + var + AssemblyLineRec: Record "Assembly Line"; + begin + // Redundant Get. The page runtime already loaded this row into Rec. + // At list-page scale this fires hundreds of times per scroll. + AssemblyLineRec.Get(Rec."Document Type", Rec."Document No.", Rec."Line No."); + ShowWarning := CheckAvailability(AssemblyLineRec); + end; + + var + ShowWarning: Boolean; + + local procedure CheckAvailability(var AssemblyLine: Record "Assembly Line"): Boolean + begin + exit(false); + end; +} diff --git a/microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.good.al b/microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.good.al new file mode 100644 index 0000000..2594f54 --- /dev/null +++ b/microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.good.al @@ -0,0 +1,30 @@ +page 51202 "Perf Sample ReGetRec Good" +{ + PageType = List; + SourceTable = "Assembly Line"; + + layout + { + area(Content) + { + repeater(Group) + { + field("No."; Rec."No.") { ApplicationArea = All; } + } + } + } + + trigger OnAfterGetRecord() + begin + // Rec already holds the current row's values; no Get needed. + ShowWarning := CheckAvailability(Rec); + end; + + var + ShowWarning: Boolean; + + local procedure CheckAvailability(var AssemblyLine: Record "Assembly Line"): Boolean + begin + exit(false); + end; +} diff --git a/microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.md b/microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.md new file mode 100644 index 0000000..120b079 --- /dev/null +++ b/microsoft/knowledge/performance/do-not-re-get-rec-inside-onaftergetrecord.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: performance +keywords: [onaftergetrecord, get, rec, page-runtime, redundant-fetch] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Do not re-Get the current record inside OnAfterGetRecord + +## Description + +The page runtime loads the current record before firing `OnAfterGetRecord` — `Rec` already holds the row's values when the trigger body runs. Calling `Rec.Get(...)` (or any equivalent Get against the same key) inside the trigger issues a second database round-trip for data the runtime just fetched. On a list page that displays hundreds of rows during a scroll, this turns into hundreds of wasted round-trips per user interaction. The same concern applies to `OnAfterGetCurrRecord` on card and document pages, though the impact is smaller because the trigger fires per selection rather than per row. + +## Best Practice + +Read from `Rec` directly. When a helper method needs a different record, pass `Rec` as an argument or let the helper fetch its own lookup once; do not re-Get the current row. If the code truly needs a fresh value because it was modified by another session, design the refresh explicitly — document it in a comment — rather than paying the cost on every trigger fire. + +See sample: `do-not-re-get-rec-inside-onaftergetrecord.good.al`. + +## Anti Pattern + +An `OnAfterGetRecord` trigger body that starts with `AssemblyLineRec.Get("Document Type", "Document No.", "Line No.")` for the same keys the page runtime has already used — the Get restates what `Rec` already holds. Replace with a direct call against `Rec` (`CheckAvailability(Rec)`). + +See sample: `do-not-re-get-rec-inside-onaftergetrecord.bad.al`. diff --git a/microsoft/knowledge/performance/do-not-retarget-flowfield-calcformula-to-larger-tables.md b/microsoft/knowledge/performance/do-not-retarget-flowfield-calcformula-to-larger-tables.md new file mode 100644 index 0000000..6d1a572 --- /dev/null +++ b/microsoft/knowledge/performance/do-not-retarget-flowfield-calcformula-to-larger-tables.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: performance +keywords: [flowfield, calcformula, regression, source-table, sift] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Do not retarget a FlowField's CalcFormula to a larger source table + +## Description + +A FlowField's CalcFormula is evaluated every time the field is read — every time the page renders, every CalcFields call, every list page filter that references the field. Changing the CalcFormula's source table from a smaller, bounded, or already-filtered table to a larger unfiltered one multiplies the per-read cost. A common shape is the refactor from "Posted X" to "X" — the unposted line table is typically an order of magnitude larger and carries rows that the original FlowField never considered. The change compiles and may look like a simple scope widening; the performance impact is not visible until production load. + +## Best Practice + +When a FlowField CalcFormula changes source table, evaluate the before/after row counts, ensure a SIFT key exists on the new source that matches the formula's filters (see `add-sift-keys-for-flowfields`), and verify no existing callers rely on the tighter scope. If the widening is intentional, the corresponding SIFT keys on the new source must ship in the same PR. + +## Anti Pattern + +Changing a `sum("Posted Expense Report Line"."Amount" where(...))` formula to `sum("Expense Report Line"."Amount" where(...))` without touching the source table's keys. Every list page and dashboard that reads the FlowField now aggregates over the unposted table too, almost always without a supporting SIFT key. diff --git a/microsoft/knowledge/performance/guard-before-get-not-after.bad.al b/microsoft/knowledge/performance/guard-before-get-not-after.bad.al new file mode 100644 index 0000000..da0d391 --- /dev/null +++ b/microsoft/knowledge/performance/guard-before-get-not-after.bad.al @@ -0,0 +1,15 @@ +codeunit 51201 "Perf Sample GuardBeforeGet Bad" +{ + procedure HandleLine(var PurchaseLine: Record "Purchase Line") + var + PurchaseHeader: Record "Purchase Header"; + begin + // Get fires on every call — including the ones that exit immediately below. + PurchaseHeader.Get(PurchaseLine."Document Type", PurchaseLine."Document No."); + + if PurchaseLine."Selected Alloc. Account No." = '' then + exit; + + // Work with PurchaseHeader. + end; +} diff --git a/microsoft/knowledge/performance/guard-before-get-not-after.good.al b/microsoft/knowledge/performance/guard-before-get-not-after.good.al new file mode 100644 index 0000000..5387f0f --- /dev/null +++ b/microsoft/knowledge/performance/guard-before-get-not-after.good.al @@ -0,0 +1,16 @@ +codeunit 51200 "Perf Sample GuardBeforeGet Good" +{ + procedure HandleLine(var PurchaseLine: Record "Purchase Line") + var + PurchaseHeader: Record "Purchase Header"; + begin + // Cheap in-memory check first. Get only when the subsequent code needs the header. + if PurchaseLine."Selected Alloc. Account No." = '' then + exit; + + if not PurchaseHeader.Get(PurchaseLine."Document Type", PurchaseLine."Document No.") then + exit; + + // Work with PurchaseHeader. + end; +} diff --git a/microsoft/knowledge/performance/guard-before-get-not-after.md b/microsoft/knowledge/performance/guard-before-get-not-after.md new file mode 100644 index 0000000..ea93e9d --- /dev/null +++ b/microsoft/knowledge/performance/guard-before-get-not-after.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: performance +keywords: [get, guard, early-exit, wasted-fetch, conditional] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Place guard conditions before Get, not after + +## Description + +A `Record.Get(Key)` is a database round-trip. When the call site also contains an early-exit condition that may fire before the fetched record is used, the order of the two matters: `Get` first followed by a guard that may exit means every call pays the round-trip, including the calls that immediately return. Flipping the order — evaluate the guard first, `Get` only when needed — costs nothing in the happy path and turns the wasted round-trip into zero work on the exit path. The savings compound on hot tables and on code paths entered many times per user action. + +## Best Practice + +Evaluate cheap, in-memory conditions first. Only issue the `Get` (or `FindFirst`, `FindLast`) when the subsequent code actually needs the record's values. For complex procedures with multiple exit conditions, sort them cheapest-first: in-memory checks, then single-record lookups, then set iteration. + +See sample: `guard-before-get-not-after.good.al`. + +## Anti Pattern + +`PurchaseHeader.Get(PurchaseLine."Document Type", PurchaseLine."Document No."); if PurchaseLine."Selected Alloc. Account No." = '' then exit;` — the Get fires on every call; the exit discards the result for every call where `Selected Alloc. Account No.` is blank. + +See sample: `guard-before-get-not-after.bad.al`. diff --git a/microsoft/knowledge/performance/keep-sourcetabletemporary-on-api-and-background-pages.md b/microsoft/knowledge/performance/keep-sourcetabletemporary-on-api-and-background-pages.md new file mode 100644 index 0000000..9702960 --- /dev/null +++ b/microsoft/knowledge/performance/keep-sourcetabletemporary-on-api-and-background-pages.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: performance +keywords: [sourcetabletemporary, tabletype, temporary, api-page, persistence, regression] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Do not remove SourceTableTemporary or TableType = Temporary without understanding the impact + +## Description + +`SourceTableTemporary = true` on a page, and `TableType = Temporary` on a table, mean the underlying record operates in memory — Insert/Modify/Delete mutate the session buffer, not the database. Removing either property converts the same operations to real SQL writes. On an API page that external callers hit at high frequency, on a background task that processes thousands of records, or on a UI page that composes an in-memory list for display, the change from temporary to persistent can turn a lightweight operation into a major source of database load. The refactor is easy to propose ("why is this temporary?") and expensive to regret. + +## Best Practice + +When a diff removes `SourceTableTemporary = true` or `TableType = Temporary`, require justification explaining why persistence is now required and what paths still write. Review the callers for unexpected new writes, transaction scope, trigger fires, and contention. Keep the property unless the change genuinely needs persistence; an unused-looking temporary table on a bounded page is usually there for a reason. + +## Anti Pattern + +A cleanup PR that deletes `SourceTableTemporary = true` from an API page "because the source table already exists". The API now writes to the real table on every call, every consumer's requests reach the database, and the incidental side-effects in the source table's triggers start firing across tenants. diff --git a/microsoft/knowledge/performance/maintainsqlindex-false-disables-sift.md b/microsoft/knowledge/performance/maintainsqlindex-false-disables-sift.md new file mode 100644 index 0000000..acd3eae --- /dev/null +++ b/microsoft/knowledge/performance/maintainsqlindex-false-disables-sift.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: performance +keywords: [maintainsqlindex, sift, sumindexfields, flowfield, calcsums, key] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# MaintainSQLIndex = false on a key disables SIFT for the FlowFields that depend on it + +## Description + +SIFT relies on the underlying SQL index being maintained by the platform. Setting `MaintainSQLIndex = false` on a key drops the SQL index without dropping the AL key declaration — the key compiles, FlowFields that reference its SumIndexFields compile, and CalcSums calls against matching filters compile. At runtime, however, the SIFT optimization silently cannot engage, and every aggregate falls back to a table scan. The symptom is a FlowField whose read time degrades linearly with row count, with no code-level signal pointing at the key property as the cause. + +## Best Practice + +Keep `MaintainSQLIndex = true` (the default) on any key whose SumIndexFields back a FlowField or that callers use with CalcSums. When a key is genuinely unused and the SQL index cost is the concern, remove the key entirely rather than leaving it in place with `MaintainSQLIndex = false`. If the FlowField is still needed, pick a different key that is maintained. + +## Anti Pattern + +A source-table key declared with `SumIndexFields` and `MaintainSQLIndex = false`, with a FlowField referencing those sum fields. The FlowField appears to work in development against small datasets and becomes a full table scan on production-scale data, with no error message and no obvious culprit in the code under review. diff --git a/microsoft/knowledge/performance/skip-setloadfields-on-narrow-tables-and-short-loops.md b/microsoft/knowledge/performance/skip-setloadfields-on-narrow-tables-and-short-loops.md new file mode 100644 index 0000000..33a299e --- /dev/null +++ b/microsoft/knowledge/performance/skip-setloadfields-on-narrow-tables-and-short-loops.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: performance +keywords: [setloadfields, heuristics, narrow-table, short-loop, diminishing-returns] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# SetLoadFields pays off at scale; skip it on narrow tables and short loops + +## Description + +`SetLoadFields` reduces the number of columns the platform hydrates per record. It delivers real savings on wide tables with blob, media, or many text fields when the iteration touches a small subset. Below certain thresholds the accounting flips the other way: narrow tables (fewer than ~10 fields) save almost nothing per row, and short loops (fewer than ~10 iterations) amortize the narrowing over too few fetches to outweigh the extra code and the specification-and-access-set coupling that future edits have to maintain. Recommending SetLoadFields on every Find/Get call produces low-value churn and invites the opposite mistake — listing a field in SetLoadFields and then forgetting to access it, which triggers a second round-trip to load the missing field. + +## Best Practice + +Reach for SetLoadFields when the table is wide (10+ fields, especially with blobs) AND the code path reads a small subset AND the iteration or fetch count is material. When in doubt on a short loop over a narrow table, leave SetLoadFields out; the complexity cost is not earned. The filter-only-field rule from `omit-filter-only-fields-from-setloadfields` still applies: fields used only in filters stay out of the list. + +## Anti Pattern + +A 5-row loop over a 6-field setup table prefaced by `Rec.SetLoadFields(...)`. The author has added two lines of code, coupled the loop to a field specification that needs to be updated on every schema change, and saved nanoseconds. The same pattern applied mechanically to every Find call in a codebase produces hundreds of diffs that do not move the performance needle. diff --git a/microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.bad.al b/microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.bad.al new file mode 100644 index 0000000..cb38dbf --- /dev/null +++ b/microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.bad.al @@ -0,0 +1,14 @@ +codeunit 51205 "Perf Sample LockTable Bad" +{ + procedure GetOrCreate(var AgentStatus: Record "Integer"): Boolean + begin + // Every caller takes an exclusive lock, even the ones that only read. + // Under load the helper becomes the dominant contention point. + AgentStatus.LockTable(); + if not AgentStatus.Get(1) then begin + AgentStatus.Number := 1; + AgentStatus.Insert(); + end; + exit(true); + end; +} diff --git a/microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.good.al b/microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.good.al new file mode 100644 index 0000000..2dba7e3 --- /dev/null +++ b/microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.good.al @@ -0,0 +1,17 @@ +codeunit 51204 "Perf Sample LockTable Good" +{ + procedure GetOrCreate(var AgentStatus: Record "Integer"): Boolean + begin + // Read path: no lock. + if AgentStatus.Get(1) then + exit(true); + + // Write path: lock only when we are about to insert. + AgentStatus.LockTable(); + if not AgentStatus.Get(1) then begin + AgentStatus.Number := 1; + AgentStatus.Insert(); + end; + exit(true); + end; +} diff --git a/microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.md b/microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.md new file mode 100644 index 0000000..6158f84 --- /dev/null +++ b/microsoft/knowledge/performance/split-read-only-and-write-paths-to-avoid-locktable.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: performance +keywords: [locktable, read-only, write-path, contention, helper] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Split read-only and write paths so LockTable runs only when needed + +## Description + +LockTable takes an exclusive write lock on the affected table for the remainder of the transaction. In a helper that is called from many read-only sites and a few write sites, placing LockTable unconditionally at the top serializes every reader on every other reader's lock — the helper becomes a system-wide contention point. The correct shape is a conditional structure: try the read-only path first, and only fall through to LockTable when the code genuinely needs to modify the table. + +## Best Practice + +Factor the helper so readers return immediately without a lock and only writers reach the LockTable call. A common pattern: attempt `Rec.Get()` first; if it returns the row, exit with the value; otherwise LockTable and proceed with the Insert. Document the pattern in a comment on the helper so callers understand why the LockTable is inside a branch. + +See sample: `split-read-only-and-write-paths-to-avoid-locktable.good.al`. + +## Anti Pattern + +A `GetOrCreate` helper that unconditionally calls `Rec.LockTable()` at the top, then Gets the row, then returns it. Every reader now blocks every other reader even though none of them intend to write. Under load the helper becomes the dominant bottleneck. + +See sample: `split-read-only-and-write-paths-to-avoid-locktable.bad.al`. diff --git a/microsoft/knowledge/performance/treat-ledger-entry-tables-as-production-scale.md b/microsoft/knowledge/performance/treat-ledger-entry-tables-as-production-scale.md new file mode 100644 index 0000000..cd38037 --- /dev/null +++ b/microsoft/knowledge/performance/treat-ledger-entry-tables-as-production-scale.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: performance +keywords: [ledger-entry, production-scale, hot-table, item-ledger, gl-entry, sales-invoice-line] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Treat ledger-entry and line-type tables as production-scale when reviewing performance + +## Description + +A handful of Business Central tables grow to millions of rows in production tenants: Item Ledger Entry, Value Entry, G/L Entry, VAT Entry, Customer Ledger Entry, Vendor Ledger Entry, Sales Invoice Line, Purchase Invoice Line, Detailed Cust. Ledg. Entry, Detailed Vendor Ledg. Entry, and equivalent line-type tables. Master-data tables like Customer, Vendor, and Item typically reach the high hundreds of thousands. A performance review that treats these tables with the same latitude as setup tables or small reference lists under-reports real regressions; the same filter-or-key mistake that is invisible on a 50-row table is a full table scan over millions of rows on these. + +## Best Practice + +When a code change touches any of the above tables, demand concrete performance reasoning before accepting it: an appropriate key selection, a SetLoadFields narrowing, filters that use the key prefix, no N+1 inside the iteration. A finding on one of these tables should almost never be downgraded from High to Low on the grounds that "the operation looks small" — at production scale the operation is never small. + +## Anti Pattern + +Applying review heuristics uniformly to all tables. A missing SetCurrentKey on a Setup table changes nothing; the same mistake on Item Ledger Entry turns a list page into a multi-second load. The asymmetry is the whole point of the catalog — knowing which tables warrant the stricter read. diff --git a/microsoft/knowledge/privacy/dataclassification-is-a-table-field-property.md b/microsoft/knowledge/privacy/dataclassification-is-a-table-field-property.md new file mode 100644 index 0000000..c47955f --- /dev/null +++ b/microsoft/knowledge/privacy/dataclassification-is-a-table-field-property.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: privacy +keywords: [dataclassification, table-field, page, api-page, scope] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# DataClassification is a table-field property, not a page property + +## Description + +DataClassification governs how the platform handles a field's data in telemetry, data-subject requests, and retention tooling. It is declared on the table field, not on the page that displays the field. Pages — card pages, list pages, API pages — simply render fields sourced from a table. A privacy issue with classification is always an issue on the table definition; the page is a display surface. + +## Best Practice + +Flag missing or wrong DataClassification on the table field where the data lives. When a field is exposed through an API page or any other page type, the source table's classification governs. Do not report the same issue on every page that happens to include the field. + +## Anti Pattern + +Reporting a privacy finding on `page 50100 "Customer API"` because it exposes an email field, rather than on `table Customer`'s email field. Fix at the source; the page is not the offender and the same correction applied per-page produces churn without changing the data-classification story. diff --git a/microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.bad.al b/microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.bad.al new file mode 100644 index 0000000..718983a --- /dev/null +++ b/microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.bad.al @@ -0,0 +1,13 @@ +tableextension 50911 "Privacy Sample IS Bad" extends "Sales & Receivables Setup" +{ + fields + { + // Refactor moves the delta URL out of encrypted IsolatedStorage into a + // plain table field. Value is now plaintext in SQL, unscoped, indistinguishable + // from non-sensitive content. + field(50100; "Delta Url"; Text[250]) + { + DataClassification = EndUserPseudonymousIdentifiers; + } + } +} diff --git a/microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.good.al b/microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.good.al new file mode 100644 index 0000000..b54ce3a --- /dev/null +++ b/microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.good.al @@ -0,0 +1,10 @@ +codeunit 50910 "Privacy Sample IS Good" +{ + procedure StoreDeltaUrl(DeltaUrl: Text) + var + DeltaKeyTok: Label 'SyncDeltaUrl', Locked = true; + begin + // Sensitive delta URL remains encrypted and scoped to the extension. + IsolatedStorage.SetEncrypted(DeltaKeyTok, DeltaUrl, DataScope::Company); + end; +} diff --git a/microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.md b/microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.md new file mode 100644 index 0000000..4e64bff --- /dev/null +++ b/microsoft/knowledge/privacy/do-not-move-pii-from-isolated-storage-to-plain-fields.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: privacy +keywords: [isolatedstorage, encryption, tokens, refactor, regression] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Do not move PII or secrets from IsolatedStorage to plain table fields + +## Description + +IsolatedStorage with SetEncrypted keeps sensitive values — tokens, URLs carrying identifiers, delta cursors with embedded user context — encrypted at rest and scoped to the extension. Moving the same value to a normal table field is a refactor that looks structural but is a privacy and security regression: the value is now plaintext in SQL, visible to every reader of that table, backed up and replicated as ordinary business data. Reviews of existing integrations frequently see this change justified as "easier to query" — the concern is the storage model, not the ergonomics. + +## Best Practice + +Keep tokens, secrets, personal-context URLs, and similar sensitive values in IsolatedStorage (SetEncrypted) or Azure Key Vault. When a refactor moves the value, require an explicit justification and a mitigating control (restricted-read permission set, value-level encryption, redaction in the access path). Otherwise leave it where it was. + +See sample: `do-not-move-pii-from-isolated-storage-to-plain-fields.good.al`. + +## Anti Pattern + +A diff that deletes an `IsolatedStorage.SetEncrypted` call and writes the same value into a new `Text` column on a business table. The value is now unencrypted, unscoped, and indistinguishable from non-sensitive content to any caller reading the table. + +See sample: `do-not-move-pii-from-isolated-storage-to-plain-fields.bad.al`. diff --git a/microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.bad.al b/microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.bad.al new file mode 100644 index 0000000..f06f679 --- /dev/null +++ b/microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.bad.al @@ -0,0 +1,16 @@ +codeunit 50903 "Privacy Sample ErrorVsMsg Bad" +{ + procedure ConfirmThenFail(var Customer: Record Customer) + var + ConfirmQst: Label 'Send welcome email to %1 at %2?', Comment = '%1 = name, %2 = email'; + FailureWithPiiErr: Text; + begin + if not Confirm(ConfirmQst, false, Customer.Name, Customer."E-Mail") then + exit; + + // Pre-built Text with PII, passed to Error: customer name and email reach telemetry. + FailureWithPiiErr := StrSubstNo( + 'Could not send welcome to %1 at %2.', Customer.Name, Customer."E-Mail"); + Error(FailureWithPiiErr); + end; +} diff --git a/microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.good.al b/microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.good.al new file mode 100644 index 0000000..a3a4b25 --- /dev/null +++ b/microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.good.al @@ -0,0 +1,15 @@ +codeunit 50902 "Privacy Sample ErrorVsMsg Good" +{ + procedure ConfirmThenFail(var Customer: Record Customer) + var + ConfirmQst: Label 'Send welcome email to %1 at %2?', Comment = '%1 = name, %2 = email'; + GenericFailureErr: Label 'The welcome email could not be sent.'; + begin + // Confirm is not logged to telemetry. PII in the prompt is fine. + if not Confirm(ConfirmQst, false, Customer.Name, Customer."E-Mail") then + exit; + + // Error is logged. Keep PII out of the message. + Error(GenericFailureErr); + end; +} diff --git a/microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.md b/microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.md new file mode 100644 index 0000000..cb3ec85 --- /dev/null +++ b/microsoft/knowledge/privacy/error-is-logged-to-telemetry-message-is-not.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: privacy +keywords: [error, message, confirm, notification, telemetry, pii] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Error logs to telemetry; Message, Confirm, and Notification do not + +## Description + +The privacy concern with user-facing text is not what the authenticated user sees — it is what the platform exports to telemetry. Error is captured automatically; Message, Confirm, StrMenu, and Notification are not. Reviews that flag PII in any user-facing dialog over-report. Reviews that ignore PII in Error under-report. The distinction is the delivery surface, not the presence of a person's name on screen. + +## Best Practice + +Free-text business content — customer names, email addresses, document numbers — is acceptable in Message, Confirm, and Notification. Treat Error text as if it will be read by telemetry consumers, because it will be. Use localized Labels with the fewest possible PII placeholders, or system identifiers (SystemId, primary key values) rather than personal data. + +See sample: `error-is-logged-to-telemetry-message-is-not.good.al`. + +## Anti Pattern + +Embedding customer emails, phone numbers, addresses, or names directly into Error strings — either as literals or via pre-built StrSubstNo output — because "the user will see this anyway." The user also sees Message and Confirm, but those are not logged. Error is. + +See sample: `error-is-logged-to-telemetry-message-is-not.bad.al`. diff --git a/microsoft/knowledge/privacy/flowfields-auto-inherit-systemmetadata.md b/microsoft/knowledge/privacy/flowfields-auto-inherit-systemmetadata.md new file mode 100644 index 0000000..5b049d3 --- /dev/null +++ b/microsoft/knowledge/privacy/flowfields-auto-inherit-systemmetadata.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: privacy +keywords: [flowfield, flowfilter, dataclassification, systemmetadata, default] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# FlowFields and FlowFilters automatically inherit DataClassification SystemMetadata + +## Description + +FlowFields and FlowFilters are virtual — they carry no stored data of their own, and their values are computed on demand from the source table the CalcFormula references. The platform classifies them as SystemMetadata automatically and does not require (or respect) a per-field DataClassification declaration. Flagging a FlowField as missing DataClassification, or as under-classified because the computed value may be CustomerContent, is a false positive: the underlying source field carries the classification that matters, and that is what telemetry and compliance tooling inspects. + +## Best Practice + +Leave DataClassification off FlowFields and FlowFilters. If the computed value is sensitive, the fix is to ensure the source table's field has the correct classification. Verify source-field classification rather than trying to re-classify the computed view. + +## Anti Pattern + +Reporting "missing DataClassification" on a FlowField, or attempting to set a FlowField's DataClassification to CustomerContent because the SUM aggregates a sensitive amount. The declaration has no effect; the platform uses the source-field classification. diff --git a/microsoft/knowledge/privacy/in-memory-data-is-not-a-privacy-concern.md b/microsoft/knowledge/privacy/in-memory-data-is-not-a-privacy-concern.md new file mode 100644 index 0000000..415525f --- /dev/null +++ b/microsoft/knowledge/privacy/in-memory-data-is-not-a-privacy-concern.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: privacy +keywords: [memory, dictionary, list, temporary-record, scope, false-positive] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# In-memory variables are not a privacy concern in Business Central + +## Description + +Business Central runs in a managed server environment. Local variables, Dictionary, List, and temporary Record buffers exist only for the duration of the request or session; the runtime reclaims them when the scope exits. Memory dumps are not a realistic threat vector in this architecture, and flagging an in-memory collection of customer emails or names as a privacy issue misstates the product's security model. + +## Best Practice + +Focus privacy review on persistence, transit, and telemetry: what is written to tables, sent over the network, or logged. Treat in-memory handling of personal data as normal business functionality. When an in-memory buffer is copied into IsolatedStorage, a table, or a telemetry call, that downstream write is what gets reviewed. + +## Anti Pattern + +Flagging `Dictionary of [Code[20], Text]`, `List of [Text]`, or `Record Customer temporary` variables that hold customer data during a calculation as a privacy concern. The flag is a false positive that trains authors to avoid a normal pattern and distracts from the persistent storage that does matter. diff --git a/microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.bad.al b/microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.bad.al new file mode 100644 index 0000000..cf1b8fa --- /dev/null +++ b/microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.bad.al @@ -0,0 +1,18 @@ +table 50909 "Privacy Sample Override Bad" +{ + DataClassification = SystemMetadata; + + fields + { + field(1; "Entry No."; Integer) { } + // Customer name inherits SystemMetadata from the table. Subject-access + // and retention tooling treats the value as system housekeeping. + field(2; "Customer Name"; Text[100]) { } + field(3; "E-Mail"; Text[80]) { } + field(4; "Logged At"; DateTime) { } + } + keys + { + key(PK; "Entry No.") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.good.al b/microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.good.al new file mode 100644 index 0000000..2670601 --- /dev/null +++ b/microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.good.al @@ -0,0 +1,23 @@ +table 50908 "Privacy Sample Override Good" +{ + DataClassification = SystemMetadata; + + fields + { + field(1; "Entry No."; Integer) { } + field(2; "Customer Name"; Text[100]) + { + // Table default is SystemMetadata; this field is personal data. + DataClassification = CustomerContent; + } + field(3; "E-Mail"; Text[80]) + { + DataClassification = CustomerContent; + } + field(4; "Logged At"; DateTime) { } + } + keys + { + key(PK; "Entry No.") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.md b/microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.md new file mode 100644 index 0000000..677e9c4 --- /dev/null +++ b/microsoft/knowledge/privacy/override-inherited-dataclassification-per-field.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: privacy +keywords: [dataclassification, inheritance, table-level, field-level, override] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Override inherited DataClassification when a field doesn't fit the table default + +## Description + +When a table declares `DataClassification` at the table level, every field inherits that value unless the field declares its own. This is efficient for homogeneous tables — a SystemMetadata log table whose fields are all system-generated, a CustomerContent transaction table whose fields are all business data. It is a privacy regression when a table is classified SystemMetadata but contains a field that holds personal data: the field silently inherits the wrong classification, and telemetry tooling treats its content as safe to log when it is not. + +## Best Practice + +Review every field on a table with a table-level DataClassification. Fields whose content matches the table's default need no per-field declaration. Fields that carry a different kind of data — a customer name on an otherwise-system-metadata log table, a personal identifier on a mixed-content table — must declare their own DataClassification that overrides the table default. + +See sample: `override-inherited-dataclassification-per-field.good.al`. + +## Anti Pattern + +A table declared `DataClassification = SystemMetadata` with fields like `Customer Name`, `E-Mail`, `Phone No.` — the fields inherit SystemMetadata, which is wrong for CustomerContent. Subject-access-request and retention tooling treats the personal data as system housekeeping. + +See sample: `override-inherited-dataclassification-per-field.bad.al`. diff --git a/microsoft/knowledge/privacy/pages-displaying-permitted-data-is-not-a-privacy-concern.md b/microsoft/knowledge/privacy/pages-displaying-permitted-data-is-not-a-privacy-concern.md new file mode 100644 index 0000000..e28a0d6 --- /dev/null +++ b/microsoft/knowledge/privacy/pages-displaying-permitted-data-is-not-a-privacy-concern.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: privacy +keywords: [page, display, permission, authenticated, false-positive] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Pages displaying data to permitted users are not a privacy concern + +## Description + +Every page in Business Central displays data to an authenticated user who holds the permissions required to see it. The permission system — table permissions, entitlements, field-level restrictions where configured — is the access-control boundary. Flagging a page for showing customer emails, names, addresses, document numbers, or system audit fields treats display as a leak when it is the product's intended function. + +## Best Practice + +Privacy review of pages is about data classification on the source table and about consent on outgoing integrations reached through page actions. Displaying business data to a user with permission to view it is correct behaviour, including on API pages that are gated by the same permission model. + +## Anti Pattern + +Reporting "customer email is shown on the page" or "user ID visible in the list" as privacy findings. The finding does not reflect a privacy regression and redirects the author toward hiding data that the permitted user is entitled to see. The same logic produces noise on Confirm, Message, and Notification that surface business identifiers. diff --git a/microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.bad.al b/microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.bad.al new file mode 100644 index 0000000..0349acf --- /dev/null +++ b/microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.bad.al @@ -0,0 +1,14 @@ +codeunit 50905 "Privacy Sample Consent Bad" +{ + procedure SyncToPartner(var Customer: Record Customer) + var + Client: HttpClient; + Content: HttpContent; + Response: HttpResponseMessage; + begin + // Customer email and name sent externally with no Privacy Notice check + // anywhere in the reachable code path. + Content.WriteFrom(Customer."E-Mail"); + Client.Post('https://partner.example.com/sync', Content, Response); + end; +} diff --git a/microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.good.al b/microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.good.al new file mode 100644 index 0000000..c3aec56 --- /dev/null +++ b/microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.good.al @@ -0,0 +1,20 @@ +codeunit 50904 "Privacy Sample Consent Good" +{ + procedure SyncToPartner(var Customer: Record Customer) + var + PrivacyNotice: Codeunit "Privacy Notice"; + Client: HttpClient; + Content: HttpContent; + Response: HttpResponseMessage; + PartnerNoticeIdTok: Label 'Contoso-PartnerSync', Locked = true; + ConsentRequiredErr: Label 'Consent is required before syncing to the external partner.'; + begin + if PrivacyNotice.GetPrivacyNoticeApprovalState(PartnerNoticeIdTok, false) <> + "Privacy Notice Approval State"::Agreed + then + Error(ConsentRequiredErr); + + Content.WriteFrom(Customer."No."); + Client.Post('https://partner.example.com/sync', Content, Response); + end; +} diff --git a/microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.md b/microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.md new file mode 100644 index 0000000..2ac058a --- /dev/null +++ b/microsoft/knowledge/privacy/require-privacy-notice-consent-before-outgoing-requests.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: privacy +keywords: [privacy-notice, consent, gdpr, httpclient, outgoing-request] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Check Privacy Notice consent before outgoing requests with customer data + +## Description + +Business Central ships a Privacy Notice framework for user consent to third-party integrations. When code sends personal data (emails, names, addresses) to an external service, the concern is not whether the data itself is compliant — the product handles that — but whether the code path has verified the user has agreed to the integration. Missing consent checks on new or modified outgoing paths is the privacy issue to flag; the presence of PII in the payload is not. + +## Best Practice + +Before an outgoing HttpClient call that carries customer data, verify consent via `Codeunit "Privacy Notice".GetPrivacyNoticeApprovalState()` for the integration's registered notice id. The check may live upstream (page OnOpenPage, wizard step) as long as every path that reaches the external call passes through it. Register new integrations via `Codeunit "Privacy Notice Registrations"`. + +See sample: `require-privacy-notice-consent-before-outgoing-requests.good.al`. + +## Anti Pattern + +Adding or modifying an outgoing integration and sending customer data without any `Privacy Notice` check in the reachable code path. Removing an existing consent check from an integration that still sends data externally falls in the same category. + +See sample: `require-privacy-notice-consent-before-outgoing-requests.bad.al`. diff --git a/microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.bad.al b/microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.bad.al new file mode 100644 index 0000000..4ff958b --- /dev/null +++ b/microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.bad.al @@ -0,0 +1,16 @@ +codeunit 50907 "Privacy Sample LastErr Bad" +{ + procedure LogFailure() + var + CategoryTok: Label 'Sync', Locked = true; + FailureTxt: Label 'Operation failed: %1', Comment = '%1 = last error text'; + begin + // GetLastErrorText(true) carries the call stack and field values from + // the failing context. Declared as SystemMetadata but the payload is CustomerContent. + Session.LogMessage( + '0000ABC', StrSubstNo(FailureTxt, GetLastErrorText(true)), + Verbosity::Error, + DataClassification::SystemMetadata, + TelemetryScope::ExtensionPublisher, 'Category', CategoryTok); + end; +} diff --git a/microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.good.al b/microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.good.al new file mode 100644 index 0000000..1c3d8fc --- /dev/null +++ b/microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.good.al @@ -0,0 +1,15 @@ +codeunit 50906 "Privacy Sample LastErr Good" +{ + procedure LogFailure() + var + CategoryTok: Label 'Sync', Locked = true; + GenericMsgTxt: Label 'Sync operation failed. See extended log for details.'; + begin + // Generic message, no GetLastErrorText. Detail goes to an internal log + // the telemetry pipeline does not receive. + Session.LogMessage( + '0000ABC', GenericMsgTxt, Verbosity::Error, + DataClassification::SystemMetadata, + TelemetryScope::ExtensionPublisher, 'Category', CategoryTok); + end; +} diff --git a/microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.md b/microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.md new file mode 100644 index 0000000..496c0ac --- /dev/null +++ b/microsoft/knowledge/privacy/sanitize-getlasterrortext-before-telemetry.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: privacy +keywords: [getlasterrortext, telemetry, callstack, dataclassification, pii] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Sanitize GetLastErrorText before sending to telemetry + +## Description + +`GetLastErrorText` and `GetLastErrorCallStack` return strings built from the failing call site's data — field values, record keys, customer names, filenames. Logging either to telemetry with `DataClassification::SystemMetadata` misstates the content: the actual values are CustomerContent or worse. The true classification is not always SystemMetadata, and silently mislabelling a CustomerContent payload as system data is the specific privacy regression to avoid. + +## Best Practice + +Log a generic error message and either omit GetLastErrorText entirely or classify the telemetry call as `DataClassification::CustomerContent`. Prefer `GetLastErrorText(false)` to exclude the call stack when the text is needed but the stack is not. When in doubt, log a generic summary and persist the detailed error separately in a restricted-access log the telemetry pipeline does not receive. + +See sample: `sanitize-getlasterrortext-before-telemetry.good.al`. + +## Anti Pattern + +`Session.LogMessage(..., StrSubstNo('Operation failed: %1', GetLastErrorText(true)), ..., DataClassification::SystemMetadata, ...)` — the classification is wrong for the payload, and the call stack typically carries customer data from the failing operation into the telemetry stream. + +See sample: `sanitize-getlasterrortext-before-telemetry.bad.al`. diff --git a/microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.bad.al b/microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.bad.al new file mode 100644 index 0000000..f2eb191 --- /dev/null +++ b/microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.bad.al @@ -0,0 +1,16 @@ +codeunit 50913 "Privacy Sample Telemetry Bad" +{ + procedure LogProcessed(var Customer: Record Customer) + var + CategoryTok: Label 'CustomerProcessing', Locked = true; + MsgTemplateTxt: Label 'Processed customer %1', Comment = '%1 = customer name'; + begin + // Declared SystemMetadata; payload is CustomerContent. The message is + // opaque text once built; the pipeline cannot redact. + Session.LogMessage( + '0000001', StrSubstNo(MsgTemplateTxt, Customer.Name), + Verbosity::Normal, + DataClassification::SystemMetadata, + TelemetryScope::All, 'Category', CategoryTok); + end; +} diff --git a/microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.good.al b/microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.good.al new file mode 100644 index 0000000..233e11d --- /dev/null +++ b/microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.good.al @@ -0,0 +1,17 @@ +codeunit 50912 "Privacy Sample Telemetry Good" +{ + procedure LogProcessed(var Customer: Record Customer) + var + CategoryTok: Label 'CustomerProcessing', Locked = true; + ProcessedMsgTxt: Label 'Customer record processed.'; + begin + // Generic message. Business identifier in a custom dimension, + // never a free-text personal name. + Session.LogMessage( + '0000001', ProcessedMsgTxt, Verbosity::Normal, + DataClassification::SystemMetadata, + TelemetryScope::ExtensionPublisher, + 'Category', CategoryTok, + 'CustomerNo', Customer."No."); + end; +} diff --git a/microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.md b/microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.md new file mode 100644 index 0000000..042be42 --- /dev/null +++ b/microsoft/knowledge/privacy/specify-dataclassification-on-every-telemetry-call.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: privacy +keywords: [telemetry, session-logmessage, dataclassification, dimensions, pii] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Specify DataClassification on every telemetry call and keep PII out of the message + +## Description + +`Session.LogMessage` accepts a DataClassification parameter that governs how the platform handles the logged content in the telemetry pipeline. Omitting it is a schema violation the platform cannot repair later. Embedding personal data — emails, names, phone numbers, addresses, filenames of user uploads — in the message string also defeats classification, because the pipeline sees opaque text and cannot selectively redact. + +## Best Practice + +Pass DataClassification explicitly on every Session.LogMessage call. Keep the message a generic, non-identifying sentence and place structured values in custom dimensions where the classification applies per key. Business identifiers (Customer No., Document No., Vendor No.) are acceptable as dimensions; free-text personal data is not. + +See sample: `specify-dataclassification-on-every-telemetry-call.good.al`. + +## Anti Pattern + +`Session.LogMessage('0001', StrSubstNo('Customer %1 processed', Customer.Name), Verbosity::Normal, DataClassification::SystemMetadata, TelemetryScope::All)` — the declared classification is SystemMetadata but the message carries CustomerContent. The payload is logged with the wrong tag; downstream consumers treat it as safe when it is not. + +See sample: `specify-dataclassification-on-every-telemetry-call.bad.al`. diff --git a/microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.bad.al b/microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.bad.al new file mode 100644 index 0000000..f162f6a --- /dev/null +++ b/microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.bad.al @@ -0,0 +1,14 @@ +codeunit 50901 "Privacy Sample StrSubstNo Bad" +{ + procedure FailCustomer(var Customer: Record Customer) + var + ErrorMsg: Text; + begin + // Platform receives a plain Text string. It cannot inspect fields, + // cannot classify, cannot strip. The email and address reach telemetry. + ErrorMsg := StrSubstNo( + 'Customer %1 (%2) at %3 has invalid data', + Customer.Name, Customer."E-Mail", Customer.Address); + Error(ErrorMsg); + end; +} diff --git a/microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.good.al b/microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.good.al new file mode 100644 index 0000000..3ced013 --- /dev/null +++ b/microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.good.al @@ -0,0 +1,11 @@ +codeunit 50900 "Privacy Sample StrSubstNo Good" +{ + procedure FailCustomer(var Customer: Record Customer) + var + CustomerDataInvalidErr: Label 'Customer %1 has invalid data.', Comment = '%1 = Customer No.'; + begin + // Platform sees the Label and the field reference. It inspects the + // field's DataClassification and handles telemetry correctly. + Error(CustomerDataInvalidErr, Customer."No."); + end; +} diff --git a/microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.md b/microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.md new file mode 100644 index 0000000..8210ec8 --- /dev/null +++ b/microsoft/knowledge/privacy/strsubstno-prebuild-breaks-error-telemetry-classification.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: privacy +keywords: [strsubstno, error, telemetry, dataclassification, pii] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Pre-building Error text with StrSubstNo defeats platform PII stripping + +## Description + +Error messages are captured by platform telemetry. When Error receives a format template and field references as substitution arguments (Error('... %1 ...', Customer."No.")), the platform inspects each field's DataClassification and omits sensitive values from telemetry automatically. When the caller pre-builds the message with StrSubstNo and then passes the resulting Text to Error, the platform sees a plain string with no field context and logs the whole thing verbatim — any PII already baked in is exported to telemetry. + +## Best Practice + +Pass the template and the field references directly to Error. Declare the template as a Label with a Comment describing each placeholder. The platform's field-aware classification logic then takes care of what reaches telemetry. + +See sample: `strsubstno-prebuild-breaks-error-telemetry-classification.good.al`. + +## Anti Pattern + +Assigning the output of StrSubstNo to a Text variable and passing that variable to Error. Every substituted value is now part of an opaque string; the platform cannot classify it and logs everything. + +See sample: `strsubstno-prebuild-breaks-error-telemetry-classification.bad.al`. diff --git a/microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.bad.al b/microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.bad.al new file mode 100644 index 0000000..7659ea9 --- /dev/null +++ b/microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.bad.al @@ -0,0 +1,16 @@ +tableextension 51303 "Sec Sample VTR Bad" extends "Sales Header" +{ + fields + { + // Editable user input with validation suppressed and no fallback check. + // The user can type any string; downstream Get against Customer will fail + // or return a wrong row. + field(50102; "Customer No."; Code[20]) + { + Caption = 'Customer no.'; + DataClassification = CustomerContent; + TableRelation = Customer."No."; + ValidateTableRelation = false; + } + } +} diff --git a/microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.good.al b/microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.good.al new file mode 100644 index 0000000..9b76762 --- /dev/null +++ b/microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.good.al @@ -0,0 +1,24 @@ +tableextension 51302 "Sec Sample VTR Good" extends "Sales Header" +{ + fields + { + // User-editable field keeps ValidateTableRelation default (true). + field(50100; "External Customer Ref"; Code[50]) + { + Caption = 'External customer reference'; + DataClassification = CustomerContent; + TableRelation = Customer."No."; + } + + // System-controlled field: validation bypass is acceptable because + // the value is populated by controlled upstream code, not the user. + field(50101; "System Batch Id"; Code[20]) + { + Caption = 'System batch ID'; + DataClassification = SystemMetadata; + TableRelation = "Job Queue Entry".ID; + ValidateTableRelation = false; + Editable = false; + } + } +} diff --git a/microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.md b/microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.md new file mode 100644 index 0000000..0cec0fd --- /dev/null +++ b/microsoft/knowledge/security/do-not-disable-validatetablerelation-on-user-input.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: security +keywords: [validatetablerelation, user-input, lookup, integrity, validation] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Do not set ValidateTableRelation = false on fields that accept user input + +## Description + +`TableRelation` on a field tells the platform that the value must exist as a primary key in the related table. `ValidateTableRelation = false` suppresses that check at validation time. On system-populated fields — values the code sets from a controlled source and never displays as editable — the suppression is acceptable because the integrity guarantee comes from the upstream writer. On a field the user types into (a page field, an import column, an API payload), disabling the validation means any value at all can be written: a non-existent customer number, a typo, a deliberate bad value. The table no longer enforces the relation, and downstream code that Gets the related row with an unguarded lookup breaks. + +## Best Practice + +Leave `ValidateTableRelation = true` (the default) on any field the user can set. When the default would produce unhelpful behaviour — a transient lookup that does not yet exist at validation time, a reference that uses a non-primary-key column — handle it with a targeted OnValidate trigger that performs the semantic check explicitly. Use `ValidateTableRelation = false` only when the field is genuinely system-controlled and the writer has already validated the reference. + +See sample: `do-not-disable-validatetablerelation-on-user-input.good.al`. + +## Anti Pattern + +A `Customer No.` field on an editable page with `TableRelation = Customer."No."` and `ValidateTableRelation = false` and no OnValidate fallback. The user can type any string; the platform accepts it; a later Get against Customer fails or returns the wrong row. + +See sample: `do-not-disable-validatetablerelation-on-user-input.bad.al`. diff --git a/microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.bad.al b/microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.bad.al new file mode 100644 index 0000000..3ed2bff --- /dev/null +++ b/microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.bad.al @@ -0,0 +1,14 @@ +codeunit 51301 "Sec Sample EnvGuid Bad" +{ + procedure GetTenantId(): Text + begin + // Tenant GUID hardcoded. Extension works in one environment, fails in every other. + exit('{12345678-1234-1234-1234-123456789012}'); + end; + + procedure GetAadApplicationId(): Text + begin + // AAD application GUID hardcoded. Same problem, surfaces as an authentication error. + exit('{87654321-4321-4321-4321-210987654321}'); + end; +} diff --git a/microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.good.al b/microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.good.al new file mode 100644 index 0000000..e19a771 --- /dev/null +++ b/microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.good.al @@ -0,0 +1,16 @@ +codeunit 51300 "Sec Sample EnvGuid Good" +{ + procedure KnownSystemId(): Guid + begin + // Stable across tenants and versions — Base Application Id. + exit('{437dbf0e-84ff-417a-965d-ed2bb9650972}'); + end; + + procedure GetTenantId(): Text + var + EnvironmentInformation: Codeunit "Environment Information"; + begin + // Environment-specific values are retrieved at runtime. + exit(EnvironmentInformation.GetTenantId()); + end; +} diff --git a/microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.md b/microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.md new file mode 100644 index 0000000..7fb3775 --- /dev/null +++ b/microsoft/knowledge/security/do-not-hardcode-environment-specific-guids.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: security +keywords: [guid, tenant-id, aad, environment, hardcoded] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Hardcoded GUIDs are only safe for well-known system identifiers + +## Description + +AL code sometimes carries hardcoded GUIDs. Some are platform-defined, stable across tenants and versions, and legitimately constant — the Base Application's ApplicationId (`{437dbf0e-84ff-417a-965d-ed2bb9650972}`) is the canonical example. Others identify a specific tenant, a specific Azure Active Directory application, or a specific environment; these look identical at the source-code level but are environment-bound and break the moment the extension is deployed anywhere else. Shipping an environment-specific GUID as a constant effectively locks the extension to one environment, and the failure mode in other tenants is usually an authentication error with no code-level signal pointing at the literal. + +## Best Practice + +Hardcoded GUIDs are acceptable for well-known system identifiers that are stable across environments — document the identifier with a comment that names what it refers to. For tenant IDs, AAD application IDs, API subscription IDs, and any value that varies by deployment, retrieve at runtime from IsolatedStorage, configuration tables, or the platform APIs that expose the current tenant context. + +See sample: `do-not-hardcode-environment-specific-guids.good.al`. + +## Anti Pattern + +`TenantId := '{12345678-1234-1234-1234-123456789012}';` or `AadApplicationId := '{87654321-...}';` inline in a codeunit. The extension authenticates in one environment and fails in every other; debugging starts from an AAD error message that does not mention the literal. + +See sample: `do-not-hardcode-environment-specific-guids.bad.al`. diff --git a/microsoft/knowledge/style/apply-approved-label-suffixes.bad.al b/microsoft/knowledge/style/apply-approved-label-suffixes.bad.al new file mode 100644 index 0000000..2893eb0 --- /dev/null +++ b/microsoft/knowledge/style/apply-approved-label-suffixes.bad.al @@ -0,0 +1,17 @@ +codeunit 51101 "Style Sample LabelSuffix Bad" +{ + procedure Example() + var + CannotDeleteLine: Label 'Cannot delete this line.'; + Text000: Label 'Update complete'; + UpdateLocation: Label 'Update location?'; + WrongSuffixTok: Label 'Customer %1 not found.', Comment = '%1 = Customer No.'; + CustomerNo: Code[20]; + begin + Error(CannotDeleteLine); + Message(Text000); + if Confirm(UpdateLocation) then + ; + Error(WrongSuffixTok, CustomerNo); + end; +} diff --git a/microsoft/knowledge/style/apply-approved-label-suffixes.good.al b/microsoft/knowledge/style/apply-approved-label-suffixes.good.al new file mode 100644 index 0000000..6557feb --- /dev/null +++ b/microsoft/knowledge/style/apply-approved-label-suffixes.good.al @@ -0,0 +1,20 @@ +codeunit 51100 "Style Sample LabelSuffix Good" +{ + procedure Example() + var + UpdateCompleteMsg: Label 'Update complete.'; + CannotDeleteLineErr: Label 'Cannot delete this line.'; + UpdateLocationQst: Label 'Update location?'; + CustomerNameLbl: Label 'Customer Name'; + HttpsMethodTok: Label 'GET', Locked = true; + TelemetryCustomerUpdatedTxt: Label 'Customer updated.'; + begin + Message(UpdateCompleteMsg); + if Confirm(UpdateLocationQst) then + ; + Session.LogMessage('0001', TelemetryCustomerUpdatedTxt, + Verbosity::Normal, DataClassification::SystemMetadata, + TelemetryScope::ExtensionPublisher); + Error(CannotDeleteLineErr); + end; +} diff --git a/microsoft/knowledge/style/apply-approved-label-suffixes.md b/microsoft/knowledge/style/apply-approved-label-suffixes.md new file mode 100644 index 0000000..59a8f72 --- /dev/null +++ b/microsoft/knowledge/style/apply-approved-label-suffixes.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: style +keywords: [label, textconst, suffix, msg, err, qst, tok, lbl, txt, aa0074] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Suffix every Label and TextConst with its approved usage tag + +## Description + +CodeCop rule AA0074 requires every Label and TextConst to carry a suffix indicating how the value is consumed: `Msg` for Message calls, `Err` for Error calls, `Qst` for Confirm or StrMenu prompts, `Tok` for locked tokens (URLs, JSON keys, short literals with `Locked = true`), `Lbl` for captions and tooltips, and `Txt` for telemetry strings. The suffix is not decoration — it is how the compiler, linter, and reviewer detect misuse (a `Tok` value passed to `Error`, a `Msg` used as an error label). The cost of adopting the convention is one short suffix per declaration; the cost of ignoring it is that every reviewer has to inspect every call site to judge appropriateness. + +## Best Practice + +Name every Label and TextConst with one of `Msg`, `Err`, `Qst`, `Tok`, `Lbl`, or `Txt` at the end. Pick the suffix that matches the consuming call, not the look of the string. When multiple suffixes are grammatically valid (`Tok` vs `Lbl` for a short caption on a locked token) the choice is a judgment call; the violation is missing a suffix or using one inconsistent with the call site. + +See sample: `apply-approved-label-suffixes.good.al`. + +## Anti Pattern + +`CannotDeleteLine: Label 'Cannot delete this line.';` — no suffix, used with Error. `Text000: Label 'Update complete';` — generic name with no suffix at all. `WrongSuffixTok: Label 'Customer %1 not found.'` used with Error — a Tok suffix on an error label. + +See sample: `apply-approved-label-suffixes.bad.al`. diff --git a/microsoft/knowledge/style/follow-api-page-naming-rules.bad.al b/microsoft/knowledge/style/follow-api-page-naming-rules.bad.al new file mode 100644 index 0000000..155d263 --- /dev/null +++ b/microsoft/knowledge/style/follow-api-page-naming-rules.bad.al @@ -0,0 +1,22 @@ +page 51103 "Style Sample ApiPage Bad" +{ + PageType = API; + APIPublisher = 'Contoso-App'; // hyphen not allowed + APIGroup = 'app_1'; // underscore not allowed + APIVersion = 'v2'; // missing minor version + EntityName = 'customers'; // should be singular + EntitySetName = 'customer'; // should be plural + SourceTable = Customer; + // DelayedInsert omitted; composite-key inserts misbehave + + layout + { + area(Content) + { + repeater(Group) + { + field(number; Rec."No.") { } + } + } + } +} diff --git a/microsoft/knowledge/style/follow-api-page-naming-rules.good.al b/microsoft/knowledge/style/follow-api-page-naming-rules.good.al new file mode 100644 index 0000000..8e1bec5 --- /dev/null +++ b/microsoft/knowledge/style/follow-api-page-naming-rules.good.al @@ -0,0 +1,25 @@ +page 51102 "Style Sample ApiPage Good" +{ + PageType = API; + APIPublisher = 'contoso'; + APIGroup = 'app1'; + APIVersion = 'v2.0'; + EntityName = 'customer'; + EntitySetName = 'customers'; + SourceTable = Customer; + DelayedInsert = true; + ODataKeyFields = SystemId; + + layout + { + area(Content) + { + repeater(Group) + { + field(systemId; Rec.SystemId) { } + field(number; Rec."No.") { } + field(displayName; Rec.Name) { } + } + } + } +} diff --git a/microsoft/knowledge/style/follow-api-page-naming-rules.md b/microsoft/knowledge/style/follow-api-page-naming-rules.md new file mode 100644 index 0000000..a203d38 --- /dev/null +++ b/microsoft/knowledge/style/follow-api-page-naming-rules.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: style +keywords: [api-page, apiversion, entityname, entitysetname, apipublisher, apigroup, delayedinsert] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# API pages follow strict naming and property rules that differ from regular pages + +## Description + +Pages declared `PageType = API` are exposed through the OData API surface. The platform enforces a set of conventions that regular pages do not share: `APIPublisher`, `APIGroup`, `EntityName`, and `EntitySetName` must be camelCase alphanumeric only — no spaces, hyphens, or underscores. `APIVersion` must match the pattern `vX.Y` (for example `v2.0`) or the literal `beta`. `EntityName` is the singular form (`customer`); `EntitySetName` is the plural (`customers`). `DelayedInsert = true` is effectively required for the OData insert workflow to behave correctly on composite keys. These rules are platform-enforced and tooling-enforced; violations produce runtime errors or consumer-visible inconsistencies rather than soft warnings. + +## Best Practice + +For every API page: camelCase alphanumeric API properties; `APIVersion` as `vX.Y` or `beta`; singular `EntityName` and plural `EntitySetName`; `DelayedInsert = true`. Keep these properties together near the top of the page definition so reviewers can check the set at a glance. + +See sample: `follow-api-page-naming-rules.good.al`. + +## Anti Pattern + +`APIPublisher = 'Contoso-App'` (hyphen rejected), `EntityName = 'customers'` and `EntitySetName = 'customer'` (swapped), `APIVersion = 'v2'` (missing minor version), `DelayedInsert` omitted. Each violation surfaces only when a consumer exercises the endpoint. + +See sample: `follow-api-page-naming-rules.bad.al`. diff --git a/microsoft/knowledge/style/include-comment-on-labels-with-placeholders.bad.al b/microsoft/knowledge/style/include-comment-on-labels-with-placeholders.bad.al new file mode 100644 index 0000000..b114696 --- /dev/null +++ b/microsoft/knowledge/style/include-comment-on-labels-with-placeholders.bad.al @@ -0,0 +1,15 @@ +codeunit 51107 "Style Sample LabelProps Bad" +{ + procedure Example() + var + // Two placeholders, no Comment. The translator has to guess which + // identifier maps to %1 and which to %2. + CustomerLocationErr: Label 'Customer %1 not found in %2.'; + // URL without Locked: enters the localization pipeline, may be translated. + HttpsUrlLbl: Label 'https://example.com'; + CustomerNo: Code[20]; + LocationCode: Code[10]; + begin + Error(CustomerLocationErr, CustomerNo, LocationCode); + end; +} diff --git a/microsoft/knowledge/style/include-comment-on-labels-with-placeholders.good.al b/microsoft/knowledge/style/include-comment-on-labels-with-placeholders.good.al new file mode 100644 index 0000000..d462a3f --- /dev/null +++ b/microsoft/knowledge/style/include-comment-on-labels-with-placeholders.good.al @@ -0,0 +1,14 @@ +codeunit 51106 "Style Sample LabelProps Good" +{ + procedure Example() + var + CustomerNotFoundErr: Label 'Customer %1 does not exist for document %2.', + Comment = '%1 = Customer No., %2 = Document No.'; + HttpsProtocolTok: Label 'HTTPS', Locked = true; + ShortDescLbl: Label 'Description text', MaxLength = 50; + CustomerNo: Code[20]; + DocumentNo: Code[20]; + begin + Error(CustomerNotFoundErr, CustomerNo, DocumentNo); + end; +} diff --git a/microsoft/knowledge/style/include-comment-on-labels-with-placeholders.md b/microsoft/knowledge/style/include-comment-on-labels-with-placeholders.md new file mode 100644 index 0000000..9064608 --- /dev/null +++ b/microsoft/knowledge/style/include-comment-on-labels-with-placeholders.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: style +keywords: [label, placeholder, comment, locked, maxlength, localization] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Label placeholders need a Comment; locked strings need Locked = true + +## Description + +AL Labels accept optional properties — `Comment`, `Locked`, `MaxLength` — that travel with the string to localization. The Comment is the translator's only signal for what `%1` and `%2` mean; without it, `'Document %1 has errors in %2.'` translates unpredictably because the translator has to guess whether %1 is a document number, document type, or document name. `Locked = true` marks a string as non-translatable — URLs, JSON keys, short command tokens — and keeps the localization pipeline from translating literals that must stay verbatim. `MaxLength` limits how much of the label survives truncation. The Comment is required whenever placeholders are not self-evident; Locked is required on any non-text value. + +## Best Practice + +For placeholders, write `Comment = '%1 = Customer No., %2 = Document Type'` alongside the Label. For URLs, HTTP methods, JSON keys, and similar literals, set `Locked = true` and use the `Tok` suffix (see `apply-approved-label-suffixes`). For captions with a tight visual budget, set `MaxLength` to the enforceable length. When the placeholder meaning is obvious (`'Customer %1 not found.'`) the Comment is optional. + +See sample: `include-comment-on-labels-with-placeholders.good.al`. + +## Anti Pattern + +`CustomerLocationErr: Label 'Customer %1 not found in %2.';` with no Comment — translators will not know which identifier maps to which placeholder. `HttpsUrl: Label 'https://example.com';` with no Locked — the URL enters the localization pipeline and may be translated into a broken address. + +See sample: `include-comment-on-labels-with-placeholders.bad.al`. diff --git a/microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.bad.al b/microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.bad.al new file mode 100644 index 0000000..4931672 --- /dev/null +++ b/microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.bad.al @@ -0,0 +1,22 @@ +table 51113 "Style Sample Option Bad" +{ + fields + { + field(1; "Entry No."; Integer) { } + field(10; Priority; Option) + { + // Four members, three captions. Critical renders with no caption. + OptionMembers = Low,Medium,High,Critical; + OptionCaption = 'Low,Medium,High'; + } + field(20; Status; Option) + { + // Missing OptionCaption entirely. + OptionMembers = Open,Released,Pending; + } + } + keys + { + key(PK; "Entry No.") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.good.al b/microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.good.al new file mode 100644 index 0000000..fda6fa3 --- /dev/null +++ b/microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.good.al @@ -0,0 +1,21 @@ +table 51112 "Style Sample Option Good" +{ + fields + { + field(1; "Entry No."; Integer) { } + field(10; Priority; Option) + { + OptionMembers = Low,Medium,High,Critical; + OptionCaption = 'Low,Medium,High,Critical'; + } + field(20; Status; Option) + { + OptionMembers = Open,Released,Pending; + OptionCaption = 'Open,Released,Pending'; + } + } + keys + { + key(PK; "Entry No.") { Clustered = true; } + } +} diff --git a/microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.md b/microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.md new file mode 100644 index 0000000..6f81865 --- /dev/null +++ b/microsoft/knowledge/style/match-optioncaption-count-to-optionmembers.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: style +keywords: [option, optionmembers, optioncaption, aa0221, aa0223, aa0224] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# OptionCaption must list exactly as many captions as OptionMembers + +## Description + +Option fields declare their values in `OptionMembers` and their localized display text in `OptionCaption`. The two lists are positionally paired — the Nth caption maps to the Nth member — and a mismatch either in count or in intent produces a field that renders blank for some values or shows the wrong caption for others. CodeCop rules AA0221, AA0223, and AA0224 flag the variants of this mistake: missing OptionCaption entirely on non-table-sourced option fields, OptionCaption with a different element count than OptionMembers, and OptionCaption content that does not correspond to the member names. + +## Best Practice + +Whenever OptionMembers is declared, declare OptionCaption with the same number of entries in the same order. For table-sourced option fields, the base table's caption applies and a per-page override is usually unnecessary — the rule applies to option fields defined in pages, reports, and non-table sources. + +See sample: `match-optioncaption-count-to-optionmembers.good.al`. + +## Anti Pattern + +`OptionMembers = Low,Medium,High,Critical;` paired with `OptionCaption = 'Low,Medium,High';` — three captions for four members. `Critical` rows render with the empty caption, or fall back to the member name, depending on where the option is displayed. + +See sample: `match-optioncaption-count-to-optionmembers.bad.al`. diff --git a/microsoft/knowledge/style/name-files-as-object-dot-type-dot-al.md b/microsoft/knowledge/style/name-files-as-object-dot-type-dot-al.md new file mode 100644 index 0000000..8dd4f56 --- /dev/null +++ b/microsoft/knowledge/style/name-files-as-object-dot-type-dot-al.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: style +keywords: [file-name, convention, object-type, al-project] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Name AL files as `..al` + +## Description + +Business Central AL projects follow a consistent file-naming convention: the file name is the object's name, followed by a dot, followed by the object type (`Page`, `Codeunit`, `Table`, `Report`, `Enum`, etc.), followed by `.al`. `CustomerCard.Page.al`, `PostSalesInvoice.Codeunit.al`, `SalesLine.Table.al`. The convention produces an alphabetically-ordered folder that groups all of an entity's objects (`SalesLine.Table.al`, `SalesLine.TableExt.al`, `SalesLineCard.Page.al`) next to each other, and makes navigation by file name in large repos predictable. + +## Best Practice + +Match the file name to the object declaration: PascalCase name, type segment, `.al`. Use `TableExt`, `PageExt`, `EnumExt` for the corresponding extension types. When multiple objects share a file (generally discouraged), name the file after the primary object. + +## Anti Pattern + +`customer_page.al`, `PostSalesInvoiceLogic.al`, `tests_noSeries.al` — all three violate the convention. The first uses snake_case, the second adds a descriptive suffix after the object name, the third prefixes the type instead of suffixing it. Tooling that expects the convention (AL-Go scaffolding, navigation helpers, diff conventions) then misbehaves on these files. diff --git a/microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.bad.al b/microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.bad.al new file mode 100644 index 0000000..947caa1 --- /dev/null +++ b/microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.bad.al @@ -0,0 +1,13 @@ +codeunit 51115 "Style Sample ErrorParams Bad" +{ + procedure Fail(CustomerNo: Code[20]) + var + CustomerNotFoundErr: Label 'Customer %1 does not exist.', Comment = '%1 = Customer No.'; + begin + // Pre-built Text to Error: translation skipped, telemetry opaque. + Error(StrSubstNo(CustomerNotFoundErr, CustomerNo)); + + // Concatenation: translation skipped, hard-coded delimiters baked in. + Error('Customer ' + CustomerNo + ' not found'); + end; +} diff --git a/microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.good.al b/microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.good.al new file mode 100644 index 0000000..57a1775 --- /dev/null +++ b/microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.good.al @@ -0,0 +1,11 @@ +codeunit 51114 "Style Sample ErrorParams Good" +{ + procedure Fail(CustomerNo: Code[20]; DocumentNo: Code[20]) + var + CustomerNotFoundErr: Label 'Customer %1 does not exist for document %2.', + Comment = '%1 = Customer No., %2 = Document No.'; + begin + // Label + arguments passed directly. Translations apply; telemetry classifies per field. + Error(CustomerNotFoundErr, CustomerNo, DocumentNo); + end; +} diff --git a/microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.md b/microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.md new file mode 100644 index 0000000..b7d758b --- /dev/null +++ b/microsoft/knowledge/style/pass-parameters-directly-to-error-no-strsubstno.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: style +keywords: [error, label, strsubstno, concatenation, telemetry, aa0216, aa0217] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Pass Error parameters directly to the Label; do not pre-build with StrSubstNo or concatenation + +## Description + +`Error` accepts a Label and its substitution parameters directly (`Error(CustomerNotFoundErr, CustomerNo, DocumentNo)`). Pre-building the message via `StrSubstNo` and passing the resulting Text, or concatenating parts with `+` and passing the result, compiles but produces two distinct regressions. The localization pipeline can only translate the Label; a pre-built Text is passed through untouched, so non-English users see the English template. Platform telemetry inspects the Label's placeholder arguments for DataClassification; a pre-built Text is opaque, so PII in the arguments is logged verbatim (see `strsubstno-prebuild-breaks-error-telemetry-classification` in the privacy domain). + +## Best Practice + +Declare the Label with placeholders and pass arguments directly to Error: `Error(CustomerNotFoundErr, CustomerNo, DocumentNo)`. Use `Comment` on the Label to document each placeholder (see `include-comment-on-labels-with-placeholders`). `Error('')` is acceptable when the caller is responsible for the surfaced error. + +See sample: `pass-parameters-directly-to-error-no-strsubstno.good.al`. + +## Anti Pattern + +`Error(StrSubstNo(CustomerNotFoundErr, CustomerNo))` — loses translation. `Error(CustomerNotFoundErr + ': ' + CustomerNo)` — loses translation, concatenates hard-coded delimiters. `Error('Customer ' + CustomerNo + ' not found')` — uses no Label at all. + +See sample: `pass-parameters-directly-to-error-no-strsubstno.bad.al`. diff --git a/microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.bad.al b/microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.bad.al new file mode 100644 index 0000000..ba94df9 --- /dev/null +++ b/microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.bad.al @@ -0,0 +1,17 @@ +codeunit 51105 "Style Sample TempPrefix Bad" +{ + procedure BuildWorkingSet() + var + WIPBuffer: Record "Job WIP Buffer" temporary; + Customer: Record Customer; + begin + // Call sites read as persistent. A reviewer cannot tell at a glance + // whether DeleteAll hits the database or the in-memory buffer. + WIPBuffer.DeleteAll(); + if Customer.FindSet() then + repeat + WIPBuffer.Init(); + WIPBuffer.Insert(); + until Customer.Next() = 0; + end; +} diff --git a/microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.good.al b/microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.good.al new file mode 100644 index 0000000..1f7d090 --- /dev/null +++ b/microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.good.al @@ -0,0 +1,16 @@ +codeunit 51104 "Style Sample TempPrefix Good" +{ + procedure BuildWorkingSet() + var + TempJobWIPBuffer: Record "Job WIP Buffer" temporary; + Customer: Record Customer; + begin + // Every read site shows whether the variable is temporary. + TempJobWIPBuffer.DeleteAll(); + if Customer.FindSet() then + repeat + TempJobWIPBuffer.Init(); + TempJobWIPBuffer.Insert(); + until Customer.Next() = 0; + end; +} diff --git a/microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.md b/microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.md new file mode 100644 index 0000000..36d8a6e --- /dev/null +++ b/microsoft/knowledge/style/prefix-temporary-record-variables-with-temp.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: style +keywords: [temporary, record, variable, prefix, naming, temp] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Prefix temporary record variables with "Temp" + +## Description + +A `Record X temporary` variable behaves differently from a persistent Record variable of the same type: Insert/Modify/Delete mutate an in-memory buffer, not the underlying table. Code that mixes persistent and temporary variables of the same type is a recurring source of data-loss bugs — a helper that does `DeleteAll` on what the caller believed was a temporary buffer wipes the real table. The convention across Business Central is to prefix every temporary record variable with `Temp` (`TempJobWIPBuffer`, `TempSalesLine`, `TempCustomer`) so the distinction is visible at every read site, not only at the declaration. + +## Best Practice + +Prefix every temporary-record variable with `Temp`. The prefix goes on the variable name, not the type; the `temporary` keyword remains on the declaration. Matching the prefix against the declaration makes it a one-line check in code review: if the name starts with `Temp`, the declaration ends in `temporary`, and vice versa. + +See sample: `prefix-temporary-record-variables-with-temp.good.al`. + +## Anti Pattern + +`WIPBuffer: Record "Job WIP Buffer" temporary` — the variable reads like a persistent record in every call site below the declaration. A reviewer scanning a mutation call (`WIPBuffer.DeleteAll()`) cannot tell from the call site whether the effect is in-memory or production. + +See sample: `prefix-temporary-record-variables-with-temp.bad.al`. diff --git a/microsoft/knowledge/style/require-parentheses-on-function-calls.bad.al b/microsoft/knowledge/style/require-parentheses-on-function-calls.bad.al new file mode 100644 index 0000000..8a0dba6 --- /dev/null +++ b/microsoft/knowledge/style/require-parentheses-on-function-calls.bad.al @@ -0,0 +1,13 @@ +codeunit 51119 "Style Sample Parentheses Bad" +{ + procedure Example(var Customer: Record Customer) + var + TempBuffer: Record "Integer" temporary; + begin + // Parentheses omitted. The call site reads like a field access. + Customer.Init; + TempBuffer.DeleteAll; + if Customer.FindFirst then + ; + end; +} diff --git a/microsoft/knowledge/style/require-parentheses-on-function-calls.good.al b/microsoft/knowledge/style/require-parentheses-on-function-calls.good.al new file mode 100644 index 0000000..358ffe5 --- /dev/null +++ b/microsoft/knowledge/style/require-parentheses-on-function-calls.good.al @@ -0,0 +1,12 @@ +codeunit 51118 "Style Sample Parentheses Good" +{ + procedure Example(var Customer: Record Customer) + var + TempBuffer: Record "Integer" temporary; + begin + Customer.Init(); + TempBuffer.DeleteAll(); + if Customer.FindFirst() then + ; + end; +} diff --git a/microsoft/knowledge/style/require-parentheses-on-function-calls.md b/microsoft/knowledge/style/require-parentheses-on-function-calls.md new file mode 100644 index 0000000..3c1916b --- /dev/null +++ b/microsoft/knowledge/style/require-parentheses-on-function-calls.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: style +keywords: [parentheses, function-call, aa0008, invocation] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Every function call carries parentheses, even with no arguments + +## Description + +AL allows `Customer.Init`, `TempBuffer.DeleteAll`, and `Customer.FindFirst` without trailing parentheses when the method takes no parameters. CodeCop rule AA0008 requires the parentheses anyway. The reason is readability: without `()`, the reader has to know the member is a method and not a property — an ambiguity that resolves differently for the platform's own APIs (FindFirst is a method; `Name` is a field). With `()`, the call site is visibly a method invocation and a simple grep for `Init(` or `DeleteAll(` finds every usage. + +## Best Practice + +Always write parentheses on method calls, even when empty: `Customer.Init()`, `TempBuffer.DeleteAll()`, `if Customer.FindFirst() then`. Apply the rule to platform methods and to user-defined procedures alike. + +See sample: `require-parentheses-on-function-calls.good.al`. + +## Anti Pattern + +`Customer.Init;`, `TempBuffer.DeleteAll;`, `if Customer.FindFirst then` — all three compile but obscure what is a call and what is a field access. The inconsistency compounds when the same codebase has both conventions. + +See sample: `require-parentheses-on-function-calls.bad.al`. diff --git a/microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.bad.al b/microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.bad.al new file mode 100644 index 0000000..5492368 --- /dev/null +++ b/microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.bad.al @@ -0,0 +1,12 @@ +codeunit 51111 "Style Sample FieldCaption Bad" +{ + procedure Example(var SalesLine: Record "Sales Line") + var + UpdateLocationQst: Label 'Update the %1?', Comment = '%1 = field'; + begin + // FieldName/TableName return English identifiers. User with a non-English + // locale sees the English "Location Code" inside an otherwise translated dialog. + if not Confirm(UpdateLocationQst, true, SalesLine.FieldName("Location Code")) then + exit; + end; +} diff --git a/microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.good.al b/microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.good.al new file mode 100644 index 0000000..cf7693e --- /dev/null +++ b/microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.good.al @@ -0,0 +1,13 @@ +codeunit 51110 "Style Sample FieldCaption Good" +{ + procedure Example(var SalesLine: Record "Sales Line") + var + UpdateLocationQst: Label 'Update the %1?', Comment = '%1 = field caption'; + TableUpdatedMsg: Label 'Updated %1.', Comment = '%1 = table caption'; + begin + // Captions are localized for the current user's language. + if not Confirm(UpdateLocationQst, true, SalesLine.FieldCaption("Location Code")) then + exit; + Message(TableUpdatedMsg, SalesLine.TableCaption()); + end; +} diff --git a/microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.md b/microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.md new file mode 100644 index 0000000..2bd8d6f --- /dev/null +++ b/microsoft/knowledge/style/use-fieldcaption-and-tablecaption-in-user-messages.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: style +keywords: [fieldcaption, tablecaption, fieldname, tablename, localization, user-message] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Use FieldCaption and TableCaption in user messages, not FieldName and TableName + +## Description + +`FieldName` and `TableName` return the object's internal identifier in English — the name the developer typed into the declaration. `FieldCaption` and `TableCaption` return the translated caption for the current user's language. In user-facing messages, errors, confirmations, and notifications, the two pairs diverge the moment the user is running a non-English locale: `FieldName("Location Code")` reads `Location Code` in every language, while `FieldCaption("Location Code")` reads the translated equivalent. Using the wrong one leaks the English identifier into a localized UI and defeats the product's translation work. + +## Best Practice + +In any string the user will read, use `FieldCaption()` and `TableCaption`. Reserve `FieldName` and `TableName` for diagnostic and telemetry contexts where the stable English identifier is preferable. The same rule applies to `XmlPort`, `Query`, and other objects with a caption/name pair. + +See sample: `use-fieldcaption-and-tablecaption-in-user-messages.good.al`. + +## Anti Pattern + +`Confirm(UpdateLocationQst, true, FieldName("Location Code"))`, `Message('Updated %1', TableName())` — both surface English identifiers to a user whose entire UI is in a different language. + +See sample: `use-fieldcaption-and-tablecaption-in-user-messages.bad.al`. diff --git a/microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.bad.al b/microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.bad.al new file mode 100644 index 0000000..1065a25 --- /dev/null +++ b/microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.bad.al @@ -0,0 +1,10 @@ +codeunit 51109 "Style Sample NamedInvoke Bad" +{ + procedure Example(var SalesShptLine: Record "Sales Shipment Line") + begin + // Numeric ID. The reader has to look up 525 and 206 to know what is called. + // If either object is renumbered in a future release, this call silently retargets. + Page.RunModal(525, SalesShptLine); + Report.Run(206, true); + end; +} diff --git a/microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.good.al b/microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.good.al new file mode 100644 index 0000000..634c202 --- /dev/null +++ b/microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.good.al @@ -0,0 +1,9 @@ +codeunit 51108 "Style Sample NamedInvoke Good" +{ + procedure Example(var SalesShptLine: Record "Sales Shipment Line") + begin + // Named invocation: reviewer sees the object, rename of 525 cannot retarget. + Page.RunModal(Page::"Posted Sales Shipment Lines", SalesShptLine); + Report.Run(Report::"Sales - Invoice", true); + end; +} diff --git a/microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.md b/microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.md new file mode 100644 index 0000000..db62cff --- /dev/null +++ b/microsoft/knowledge/style/use-named-invocations-instead-of-object-ids.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: style +keywords: [object-id, page-run, report-run, codeunit-run, named-invocation] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Invoke objects by name, not by numeric ID + +## Description + +AL supports calling `Page.RunModal(525, ...)` or `Report.Run(206, ...)` with a bare numeric ID. The platform accepts the number, but the call site loses every signal that makes the code reviewable and refactor-safe: the reader cannot tell which object is being invoked without looking up 525 in the object catalog, and the renumbering of an object in a future release (legal in AL — IDs are not a stable contract) silently retargets the call to a different object. The `Page::"..."` / `Report::"..."` syntax compiles to the same runtime call but makes the target explicit and binds by name, which is the stable identity. + +## Best Practice + +Write `Page.RunModal(Page::"Posted Sales Shipment Lines", SalesShptLine)` and `Report.Run(Report::"Sales - Invoice", true)`. Apply the same rule to `Codeunit.Run`, `XmlPort.Run`, and similar runtime invocations. Reserve numeric IDs for diagnostic tooling that genuinely needs them. + +See sample: `use-named-invocations-instead-of-object-ids.good.al`. + +## Anti Pattern + +`Page.RunModal(525, SalesShptLine);` — the reader has no idea what page 525 is without a lookup, and a future rename of page 525 or renumber of "Posted Sales Shipment Lines" produces a silent mismatch. + +See sample: `use-named-invocations-instead-of-object-ids.bad.al`. diff --git a/microsoft/knowledge/style/use-this-keyword-in-codeunits.bad.al b/microsoft/knowledge/style/use-this-keyword-in-codeunits.bad.al new file mode 100644 index 0000000..b35982d --- /dev/null +++ b/microsoft/knowledge/style/use-this-keyword-in-codeunits.bad.al @@ -0,0 +1,15 @@ +codeunit 51117 "Style Sample ThisKeyword Bad" +{ + procedure ProcessRecord(var Customer: Record Customer) + begin + // Ambiguous: is ValidateCustomer a local, a global, or a method on + // another codeunit in scope? + ValidateCustomer(Customer); + + // No way to pass the current codeunit without `this`. + end; + + local procedure ValidateCustomer(var Customer: Record Customer) + begin + end; +} diff --git a/microsoft/knowledge/style/use-this-keyword-in-codeunits.good.al b/microsoft/knowledge/style/use-this-keyword-in-codeunits.good.al new file mode 100644 index 0000000..6a6bf94 --- /dev/null +++ b/microsoft/knowledge/style/use-this-keyword-in-codeunits.good.al @@ -0,0 +1,21 @@ +codeunit 51116 "Style Sample ThisKeyword Good" +{ + procedure ProcessRecord(var Customer: Record Customer) + var + Other: Codeunit "Style Sample ThisKeyword Good"; + begin + // Clearly this codeunit's method. + this.ValidateCustomer(Customer); + + // Only way to pass the current codeunit as an argument. + Other.DoWith(this); + end; + + local procedure ValidateCustomer(var Customer: Record Customer) + begin + end; + + procedure DoWith(var Helper: Codeunit "Style Sample ThisKeyword Good") + begin + end; +} diff --git a/microsoft/knowledge/style/use-this-keyword-in-codeunits.md b/microsoft/knowledge/style/use-this-keyword-in-codeunits.md new file mode 100644 index 0000000..08a26f8 --- /dev/null +++ b/microsoft/knowledge/style/use-this-keyword-in-codeunits.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: style +keywords: [this, codeunit, self-reference, aa0248, readability] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Use the `this` keyword for codeunit self-reference + +## Description + +CodeCop rule AA0248 recommends the `this` keyword inside codeunit procedures when referring to the codeunit's own members or passing the codeunit itself to another procedure. AL's scope resolution otherwise blurs global-variable access, local-variable access, and same-codeunit method calls into the same unqualified syntax — a reader of `ValidateCustomer(Customer)` cannot tell at the call site whether `ValidateCustomer` is a local, a global, or a method on a different codeunit in scope. `this.ValidateCustomer(Customer)` removes the ambiguity, and `OtherCodeunit.DoWork(this)` is the only way to pass the current codeunit as a parameter. + +## Best Practice + +In codeunits, prefix same-codeunit method calls with `this.` when the call is ambiguous or when the scope spans more than a few lines. When the current codeunit needs to be passed as an argument, write `this` — there is no alternative syntax. The rule applies to codeunits; pages, reports, and tables have their own scoping. + +See sample: `use-this-keyword-in-codeunits.good.al`. + +## Anti Pattern + +`ValidateCustomer(Customer); SomeOtherCodeunit.DoWork(/* this codeunit? */);` — the first call has ambiguous origin, and the second cannot pass the current codeunit without `this`. The style becomes load-bearing as the codeunit grows past a few small procedures. + +See sample: `use-this-keyword-in-codeunits.bad.al`. diff --git a/microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.bad.al b/microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.bad.al new file mode 100644 index 0000000..a519d8d --- /dev/null +++ b/microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.bad.al @@ -0,0 +1,26 @@ +page 51005 "UI Sample ActionTooltip Bad" +{ + PageType = Card; + SourceTable = "Sales Header"; + + actions + { + area(Processing) + { + action(Post) + { + Caption = 'Post'; + ApplicationArea = All; + // Declarative, not imperative. No period. + ToolTip = 'This will post the invoice'; + } + action(SendForApproval) + { + Caption = 'Send for approval'; + ApplicationArea = All; + // Fragment that repeats the caption and says nothing new. + ToolTip = 'Send for approval'; + } + } + } +} diff --git a/microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.good.al b/microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.good.al new file mode 100644 index 0000000..2dcab7e --- /dev/null +++ b/microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.good.al @@ -0,0 +1,25 @@ +page 51004 "UI Sample ActionTooltip Good" +{ + PageType = Card; + SourceTable = "Sales Header"; + + actions + { + area(Processing) + { + action(Post) + { + Caption = 'Post'; + ApplicationArea = All; + // Imperative verb-first sentence, Sentence case, terminating period. + ToolTip = 'Post the current sales invoice and finalize the transaction.'; + } + action(SendForApproval) + { + Caption = 'Send for approval'; + ApplicationArea = All; + ToolTip = 'Send the document to the approval workflow.'; + } + } + } +} diff --git a/microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.md b/microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.md new file mode 100644 index 0000000..bc2d457 --- /dev/null +++ b/microsoft/knowledge/ui/action-tooltips-are-imperative-and-end-with-period.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: ui +keywords: [tooltip, action, imperative, voice, period] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Action tooltips are imperative, verb-first sentences ending with a period + +## Description + +Action tooltips describe what the user will cause by invoking the action. The house style is an imperative verb-first sentence — `Post the current sales invoice and finalize the transaction.` — not a declarative one ("This will post …") and not a fragment ("Post invoice"). The imperative voice matches how the user reads the action bar: each tooltip completes the sentence "If I click this, the system will …" in the same grammatical form. Shortcut-key hints, when present, belong at the end of the tooltip and are retained verbatim. + +## Best Practice + +Start the tooltip with the verb. Use Sentence case, end with a period, stay within the ~250-character budget. Keep one sentence unless the action genuinely needs two; avoid editorializing ("Easily post …") or narrating ("This action posts …"). Preserve any existing shortcut annotation. + +See sample: `action-tooltips-are-imperative-and-end-with-period.good.al`. + +## Anti Pattern + +`ToolTip = 'This will post the invoice'` — declarative rather than imperative, no period. `ToolTip = 'Post'` — one-word fragment that duplicates the Caption and says nothing new. Both fail the scan-the-action-bar comprehension test. + +See sample: `action-tooltips-are-imperative-and-end-with-period.bad.al`. diff --git a/microsoft/knowledge/ui/avoid-banned-ui-terms.md b/microsoft/knowledge/ui/avoid-banned-ui-terms.md new file mode 100644 index 0000000..efb5c79 --- /dev/null +++ b/microsoft/knowledge/ui/avoid-banned-ui-terms.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: ui +keywords: [terminology, disabled, invalid, whitelist, blacklist, voice] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Avoid banned UI terms; prefer the inclusive and direct replacements + +## Description + +Business Central's UI voice guidelines exclude four terms that carry connotations the product does not want to push onto users: "Disabled" (clinical/negative), "Invalid" (pejorative), "Whitelist" and "Blacklist" (terms with racial associations the industry has moved away from). The replacements read naturally, match the product's warm-and-direct voice, and align with Microsoft's cross-product terminology. The concern applies to user-visible text — captions, tooltips, error messages, notifications — not to variable names or code comments. + +## Best Practice + +Replace "Disabled" with "Turned off" or "Not available". Replace "Invalid" with "Not valid" or "Incorrect". Replace "Whitelist" with "Allow list". Replace "Blacklist" with "Block list". Apply the substitution in all UI text surfaces: Caption, ToolTip, AboutTitle, AboutText, Label values, Message/Confirm/Error strings. + +## Anti Pattern + +`ErrorLbl: Label 'Invalid input.'`, `Caption = 'Disabled Users'`, `ToolTip = 'Specifies the blacklist of blocked senders.'` — all three terms in places the user will read. The fix is literal substitution with the approved alternative. diff --git a/microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.bad.al b/microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.bad.al new file mode 100644 index 0000000..8c82cf1 --- /dev/null +++ b/microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.bad.al @@ -0,0 +1,21 @@ +page 51001 "UI Sample Caption Bad" +{ + PageType = List; + SourceTable = Customer; + + // Noun phrase in Sentence case. Every other list page in the product is Title Case. + Caption = 'Sales orders'; + + actions + { + area(Processing) + { + // Sentence phrase in Title Case. Reads as a typo. + action(PostAndPrint) + { + Caption = 'Post And Print'; + ApplicationArea = All; + } + } + } +} diff --git a/microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.good.al b/microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.good.al new file mode 100644 index 0000000..e3bec15 --- /dev/null +++ b/microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.good.al @@ -0,0 +1,27 @@ +page 51000 "UI Sample Caption Good" +{ + PageType = List; + SourceTable = Customer; + + // Noun-phrase page caption: Title Case. + Caption = 'Sales Orders'; + + actions + { + area(Processing) + { + // Sentence-phrase action caption: Sentence case. + action(PostAndPrint) + { + Caption = 'Post and print'; + ApplicationArea = All; + } + + action(SendEmail) + { + Caption = 'Send email'; + ApplicationArea = All; + } + } + } +} diff --git a/microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.md b/microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.md new file mode 100644 index 0000000..595010f --- /dev/null +++ b/microsoft/knowledge/ui/caption-capitalization-noun-phrase-vs-sentence-phrase.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: ui +keywords: [caption, capitalization, title-case, sentence-case, noun-phrase] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Capitalize captions by phrase type: noun phrase is Title Case, sentence phrase is Sentence case + +## Description + +Business Central UI captions follow a simple capitalization rule that depends on the grammatical shape of the caption, not its location. A caption that is a pure noun phrase — no verb — uses Title Case: each major word capitalized (`Sales Orders`, `Chart of Accounts`, `Payment Terms`). A caption that is an imperative or declarative sentence phrase — contains a verb — uses Sentence case: only the first word and proper nouns capitalized (`Post and print`, `Send email`, `Create flow`). Following the rule makes unrelated captions feel consistent; ignoring it is visibly inconsistent in the user's navigation. + +## Best Practice + +Decide by parsing the caption as a phrase. "Sales Orders" is a thing; Title Case. "Post and print" tells the user to do something; Sentence case. For captions that are literally a single noun (`Save`, `Close`), treat them as sentence phrases — the imperative verb is implied. + +See sample: `caption-capitalization-noun-phrase-vs-sentence-phrase.good.al`. + +## Anti Pattern + +Writing `Caption = 'Sales orders'` on a list page (noun phrase styled as a sentence) or `Caption = 'Post And Print'` on an action (sentence phrase styled as title case). Both read as typos to a native English reader and inconsistent to a translator. + +See sample: `caption-capitalization-noun-phrase-vs-sentence-phrase.bad.al`. diff --git a/microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.bad.al b/microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.bad.al new file mode 100644 index 0000000..3f7eec1 --- /dev/null +++ b/microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.bad.al @@ -0,0 +1,26 @@ +page 51003 "UI Sample FieldTooltip Bad" +{ + PageType = Card; + SourceTable = Customer; + + layout + { + area(Content) + { + group(General) + { + field("Name"; Rec.Name) + { + ApplicationArea = All; + // No "Specifies" opener, no period, a bare fragment. + ToolTip = 'The name of the customer'; + } + field("Balance (LCY)"; Rec."Balance (LCY)") + { + ApplicationArea = All; + ToolTip = 'Balance'; + } + } + } + } +} diff --git a/microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.good.al b/microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.good.al new file mode 100644 index 0000000..100dfcf --- /dev/null +++ b/microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.good.al @@ -0,0 +1,25 @@ +page 51002 "UI Sample FieldTooltip Good" +{ + PageType = Card; + SourceTable = Customer; + + layout + { + area(Content) + { + group(General) + { + field("Name"; Rec.Name) + { + ApplicationArea = All; + ToolTip = 'Specifies the name of the customer.'; + } + field("Balance (LCY)"; Rec."Balance (LCY)") + { + ApplicationArea = All; + ToolTip = 'Shows the current balance in the local currency.'; + } + } + } + } +} diff --git a/microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.md b/microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.md new file mode 100644 index 0000000..e00842c --- /dev/null +++ b/microsoft/knowledge/ui/field-tooltips-start-with-specifies-and-end-with-period.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: ui +keywords: [tooltip, field, specifies, voice, period] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Field tooltips start with "Specifies" and end with a period + +## Description + +Field tooltips describe what a value means, and the Business Central house style for them is a declarative sentence that starts with "Specifies" and ends with a period. The convention is not cosmetic: it yields a consistent voice across thousands of fields so a user scanning several tooltips in quick succession can compare them without re-parsing each opening clause. Alternative phrasings ("Shows …", "The …") are accepted when they describe the field clearly, but "Specifies …" is the default and the easiest to translate consistently. + +## Best Practice + +Write field tooltips as `Specifies .` — a single sentence, Sentence case, terminating period. Keep under the ~250-character tooltip budget (see `respect-ui-text-character-limits`). When the field's meaning is genuinely not a "specifies" sentence, use "Shows …" or a clearly descriptive alternative; avoid bare fragments. + +See sample: `field-tooltips-start-with-specifies-and-end-with-period.good.al`. + +## Anti Pattern + +`ToolTip = 'The name of the customer'` — missing "Specifies" opener, missing period. `ToolTip = 'Customer name'` — a fragment rather than a sentence. Both sit inconsistently next to adjacent "Specifies …" tooltips on the same page. + +See sample: `field-tooltips-start-with-specifies-and-end-with-period.bad.al`. diff --git a/microsoft/knowledge/ui/respect-ui-text-character-limits.md b/microsoft/knowledge/ui/respect-ui-text-character-limits.md new file mode 100644 index 0000000..7577656 --- /dev/null +++ b/microsoft/knowledge/ui/respect-ui-text-character-limits.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: ui +keywords: [caption, tooltip, character-limit, truncation, localization] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Respect Business Central's UI text character limits to avoid truncation + +## Description + +Business Central UI surfaces have practical character limits before the platform truncates or the translator's localization overflows the available space. Authoring captions and tooltips close to the English limit almost guarantees truncation in languages whose translations are longer (German, French, Spanish average 20–40% longer than English). The limits are not hard compiler errors — they are product-quality thresholds that agents should flag at author time so the string reaches localization with room to grow. + +## Best Practice + +Author within these approximate limits (English): action and field captions ~40 chars; field-group, menu-item, page, and dialog titles ~40 chars; button captions ~20 chars; action and field tooltips ~250 chars; dialog text and error messages ~250 chars; notifications ~100 chars; checklist ShortTitleChecklist 34, LongerTitleCard 53, CardDescription 180. Leave headroom for longer translations; at 40/40 in English, German is likely to truncate. + +## Anti Pattern + +`action(RecalculateAndReapplyAllOutstandingCustomerDiscounts) { Caption = 'Recalculate and reapply all outstanding customer discounts'; }` — 58 characters in English, essentially guaranteed to truncate once translated. The fix is to shorten the English caption (`Recalculate customer discounts`, 30 chars) and move the full sentence into the tooltip where the budget is larger. diff --git a/microsoft/knowledge/ui/titles-have-no-trailing-punctuation.md b/microsoft/knowledge/ui/titles-have-no-trailing-punctuation.md new file mode 100644 index 0000000..a076a76 --- /dev/null +++ b/microsoft/knowledge/ui/titles-have-no-trailing-punctuation.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: ui +keywords: [title, caption, page, dialog, punctuation, ellipsis] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Titles carry no trailing punctuation and no trailing ellipsis + +## Description + +Page titles, section titles, FastTab titles, and dialog titles in Business Central are labels, not sentences — they have no trailing period, question mark, or exclamation. Trailing ellipsis ("…" or "...") on a title is specifically a long-standing Windows convention for action buttons that open a dialog, and AL handles that via the action's runtime behaviour rather than the caption text. Adding the ellipsis literally into a page caption or action caption is wrong in both directions: the platform also displays its own ellipsis when appropriate, and the static three dots corrupt translations that adjust punctuation for the locale. + +## Best Practice + +End titles with the last word of the title. Sentence case per the capitalization rule for the phrase type (see `caption-capitalization-noun-phrase-vs-sentence-phrase`). If a dialog needs "…" behaviour, rely on the platform; do not type the characters into the caption string. + +## Anti Pattern + +`Caption = 'Setup wizard...'`, `Caption = 'Sales orders.'`, `page Caption = 'Customer list:'` — all three decorate the title with terminal punctuation that is noise to the reader and a translation headache. diff --git a/microsoft/knowledge/ui/tooltips-describe-teaching-tips-guide.md b/microsoft/knowledge/ui/tooltips-describe-teaching-tips-guide.md new file mode 100644 index 0000000..da920e3 --- /dev/null +++ b/microsoft/knowledge/ui/tooltips-describe-teaching-tips-guide.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: ui +keywords: [tooltip, teaching-tip, abouttitle, abouttext, onboarding] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Tooltips describe what a thing is; teaching tips guide what the user can do with it + +## Description + +Business Central exposes two distinct affordances for explaining the UI: ToolTip and the AboutTitle/AboutText teaching tip. They answer different questions and are complementary, not alternatives. ToolTip answers "What is this field/action?" and is expected on every field and action. The teaching tip answers "What can I do with this page or this important element?" and is reserved for the few entry points where an onboarding hint is worth the user's attention. Authors who put teaching-tip content in tooltips make tooltips noisy; authors who put tooltip content in teaching tips make teaching tips useless. + +## Best Practice + +Write ToolTip as a concise descriptive sentence following the "Specifies …" or imperative voice rules. Reserve AboutTitle/AboutText for the top-level card and list pages where first-time users benefit from discovering the page's purpose and outcome. On list pages, title uses the plural form ("About sales invoices"). On card or document pages, title uses the entity name plus "details" ("About sales invoice details"). + +## Anti Pattern + +A field ToolTip that tells the user "You can create new customers from here and update their payment terms, and the list also shows…" — that is teaching-tip content. Conversely, an AboutText that simply repeats the page Caption tells the user nothing they did not already read in the title bar. diff --git a/microsoft/knowledge/ui/tour-tips-do-not-use-action-language.md b/microsoft/knowledge/ui/tour-tips-do-not-use-action-language.md new file mode 100644 index 0000000..026a005 --- /dev/null +++ b/microsoft/knowledge/ui/tour-tips-do-not-use-action-language.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: ui +keywords: [tour-tip, abouttext, teaching-tip, imperative, onboarding] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Tour tips describe outcomes, not instructions — never tell the user to perform an action during the tour + +## Description + +A tour is a guided sequence of teaching tips that runs over the page while the user is passively watching. The tour framework does not expose the page's actions during the tip — so an `AboutText` that tells the user `Enter the customer name here.` or `Now post the invoice.` asks the user to do something that is not possible in the moment. The result is a confusing first-run experience. Tour content should describe what the element represents and what the user will be able to do with it after the tour completes, in descriptive rather than imperative voice. + +## Best Practice + +Write tour AboutTitle as a short noun-phrase label for the element ("Who you are selling to", "When all is set, you post"). Write AboutText as one or two sentences that describe the outcome or meaning, not steps. Keep the tour itself short — one to four tips total — and let the regular ToolTip carry the per-element detail. + +## Anti Pattern + +`AboutText = 'Enter the customer name here.'` on a tour tip — the action is not active. `AboutText = 'Now post the invoice.'` during a tour — the user cannot, and would not want to mid-tour. Both teach nothing and confuse the reader. diff --git a/microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.bad.al b/microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.bad.al new file mode 100644 index 0000000..8142b2f --- /dev/null +++ b/microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.bad.al @@ -0,0 +1,20 @@ +page 51007 "UI Sample Ampersand Bad" +{ + PageType = Card; + SourceTable = "Sales Header"; + + actions + { + area(Processing) + { + action(PostAndSend) + { + // '&' is being used as "and", not as an accelerator prefix. The + // parser cannot tell; translators re-evaluate every occurrence. + Caption = 'Post & Send'; + ApplicationArea = All; + ToolTip = 'Post and send the document.'; + } + } + } +} diff --git a/microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.good.al b/microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.good.al new file mode 100644 index 0000000..723a70b --- /dev/null +++ b/microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.good.al @@ -0,0 +1,19 @@ +page 51006 "UI Sample Ampersand Good" +{ + PageType = Card; + SourceTable = "Sales Header"; + + actions + { + area(Processing) + { + action(PostAndSend) + { + // "and" written out. Ampersand-s marks 's' as the accelerator key. + Caption = 'Post and &send'; + ApplicationArea = All; + ToolTip = 'Post the document and send it to the customer.'; + } + } + } +} diff --git a/microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.md b/microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.md new file mode 100644 index 0000000..02746fd --- /dev/null +++ b/microsoft/knowledge/ui/use-and-not-ampersand-in-ui-captions.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: ui +keywords: [ampersand, caption, accelerator, translation, voice] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Write "and" in UI captions; keep the ampersand only as an accelerator-key prefix + +## Description + +AL Caption strings use the ampersand character in two distinct ways. Inside a caption, `&` is the accelerator-key prefix — `Caption = '&Post'` underlines the P and makes Alt+P activate the action. Outside that role, `&` is sometimes used as a shortening for the word "and" (`Post & Send`). The first usage is platform-defined and must be preserved. The second is a style choice that the Business Central voice guidelines reject: `Post and send` reads naturally in all supported locales and translates cleanly, while `Post & Send` conveys nothing extra and adds a character that localizers have to re-evaluate. + +## Best Practice + +Use the word "and" in caption text. Keep `&` only when it is immediately followed by a letter chosen as the keyboard accelerator. If both meanings apply, write them explicitly: `Post and &send` uses `s` as the accelerator and spells the conjunction out. + +See sample: `use-and-not-ampersand-in-ui-captions.good.al`. + +## Anti Pattern + +`Caption = 'Post & Send'` as the full caption — the ampersand is meant as "and" but the AL parser cannot tell, and the result is inconsistent with every other "X and Y" caption in the product. + +See sample: `use-and-not-ampersand-in-ui-captions.bad.al`. diff --git a/microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.bad.al b/microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.bad.al new file mode 100644 index 0000000..1d83a48 --- /dev/null +++ b/microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.bad.al @@ -0,0 +1,14 @@ +codeunit 50801 "Upgrade Sample CallMethods Bad" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + var + Customer: Record Customer; + begin + // Inline logic in the trigger body: no tag guard, not testable in isolation, + // re-runs on every upgrade. + Customer.SetRange(Blocked, Customer.Blocked::" "); + Customer.ModifyAll("Some Field", true); + end; +} diff --git a/microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.good.al b/microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.good.al new file mode 100644 index 0000000..e5c7218 --- /dev/null +++ b/microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.good.al @@ -0,0 +1,31 @@ +codeunit 50800 "Upgrade Sample CallMethods Good" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + begin + UpgradeCustomerDefaults(); + UpgradeSalesDocumentDefaults(); + end; + + local procedure UpgradeCustomerDefaults() + var + UpgradeTag: Codeunit "Upgrade Tag"; + begin + if UpgradeTag.HasUpgradeTag(CustomerDefaultsUpgradeTag()) then + exit; + + // Step body omitted + + UpgradeTag.SetUpgradeTag(CustomerDefaultsUpgradeTag()); + end; + + local procedure UpgradeSalesDocumentDefaults() + begin + end; + + local procedure CustomerDefaultsUpgradeTag(): Code[250] + begin + exit('MS-000001-CustomerDefaults-20260501'); + end; +} diff --git a/microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.md b/microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.md new file mode 100644 index 0000000..cf04b13 --- /dev/null +++ b/microsoft/knowledge/upgrade/call-methods-from-onupgrade-triggers-not-inline-code.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [upgrade-codeunit, onupgradepercompany, onupgradeperdatabase, structure] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Call named methods from OnUpgrade triggers; keep the triggers empty of logic + +## Description + +An upgrade codeunit (`Subtype = Upgrade`) runs its triggers once per upgrade scope. Inlining upgrade logic inside the trigger body mixes the entry point with the work, makes individual steps untestable in isolation, and prevents the standard upgrade-tag guard pattern from being applied cleanly. The convention across Business Central's own upgrade codeunits is that `OnUpgradePerCompany` and `OnUpgradePerDatabase` are a list of calls to named local procedures, each implementing one step behind its own upgrade-tag check. + +## Best Practice + +Keep `OnUpgradePerCompany` and `OnUpgradePerDatabase` to a list of `UpgradeXxx();` statements. Put every data migration, default, or correction in a named local procedure whose first action is the upgrade-tag guard. Empty trigger bodies are also acceptable as placeholders on a new codeunit with no current steps. + +See sample: `call-methods-from-onupgrade-triggers-not-inline-code.good.al`. + +## Anti Pattern + +Writing `Customer.ModifyAll(...)`, `TableX.SetRange(...)` + loops, or `DataTransfer.CopyFields()` directly inside the trigger body. The step is untagged, untestable, and re-runs on every upgrade. + +See sample: `call-methods-from-onupgrade-triggers-not-inline-code.bad.al`. diff --git a/microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.bad.al b/microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.bad.al new file mode 100644 index 0000000..e895201 --- /dev/null +++ b/microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.bad.al @@ -0,0 +1,15 @@ +codeunit 50822 "Upgrade Sample FirstInstall Bad" +{ + Subtype = Install; + + trigger OnInstallAppPerCompany() + begin + // Unconditional initialization. Re-install after uninstall either throws + // on primary-key collisions or overwrites existing rows. + InsertDefaultSetup(); + end; + + local procedure InsertDefaultSetup() + begin + end; +} diff --git a/microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.good.al b/microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.good.al new file mode 100644 index 0000000..174e2d9 --- /dev/null +++ b/microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.good.al @@ -0,0 +1,20 @@ +codeunit 50821 "Upgrade Sample FirstInstall Good" +{ + Subtype = Install; + + trigger OnInstallAppPerCompany() + var + AppInfo: ModuleInfo; + begin + NavApp.GetCurrentModuleInfo(AppInfo); + if AppInfo.DataVersion() <> Version.Create('0.0.0.0') then + exit; + + // First-install-only initialization follows here. + InsertDefaultSetup(); + end; + + local procedure InsertDefaultSetup() + begin + end; +} diff --git a/microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.md b/microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.md new file mode 100644 index 0000000..84d72fb --- /dev/null +++ b/microsoft/knowledge/upgrade/detect-first-install-via-dataversion-zero.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [oninstall, dataversion, appinfo, first-install, upgrade-tag] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Detect first install via DataVersion equal to 0.0.0.0 in OnInstall triggers + +## Description + +`OnInstallAppPerCompany` fires on first install and on subsequent re-installs after an uninstall. Code that should only run on the very first install needs to distinguish the two — and the supported way is checking `AppInfo.DataVersion() = Version.Create('0.0.0.0')`, which is the sentinel for "no prior data exists for this app in this tenant". This is the one case where a DataVersion check is correct; steady-state upgrade steps should use upgrade tags instead. + +## Best Practice + +In `OnInstallAppPerCompany`, call `NavApp.GetCurrentModuleInfo(AppInfo)` and exit early when `AppInfo.DataVersion()` is non-zero. The remainder of the trigger body then runs exclusively on first install. For all other version-sensitive upgrade logic, use upgrade tags (see `use-upgrade-tags-not-version-checks`). + +See sample: `detect-first-install-via-dataversion-zero.good.al`. + +## Anti Pattern + +Running initialization unconditionally in `OnInstallAppPerCompany` and relying on primary-key collisions to avoid double-inserts. Re-install scenarios either throw or overwrite existing rows; the install path becomes brittle as the app grows. + +See sample: `detect-first-install-via-dataversion-zero.bad.al`. diff --git a/microsoft/knowledge/upgrade/do-not-make-external-calls-in-upgrade-codeunits.md b/microsoft/knowledge/upgrade/do-not-make-external-calls-in-upgrade-codeunits.md new file mode 100644 index 0000000..57bdde3 --- /dev/null +++ b/microsoft/knowledge/upgrade/do-not-make-external-calls-in-upgrade-codeunits.md @@ -0,0 +1,22 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [upgrade, httpclient, external-service, dotnet, availability] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Do not make external service calls inside upgrade codeunits + +## Description + +The upgrade scope has to complete for the tenant to reach the new version. Any call in the upgrade path that depends on an external service — HttpClient to a partner API, a DotNet interop call, a codeunit that fetches remote configuration — fails closed when the service is unreachable, misconfigured, or slow. The failure blocks the upgrade for every customer whose environment cannot reach the dependency at the moment the upgrade runs, and there is no user present to retry. The scope is specifically code inside codeunits with `Subtype = Upgrade` or reachable from their triggers. + +## Best Practice + +Defer external calls to runtime code that executes after the upgrade — install-triggered tasks, background job queue entries scheduled by the upgrade, or lazy initialization on first use. The upgrade step should compute a local result or mark work to be done, not perform the remote call itself. + +## Anti Pattern + +`HttpClient.Get(...)` or `DotNetType.CallStaticMethod(...)` directly in `OnUpgradePerCompany`, or in a local procedure called from it. The upgrade now depends on network availability to a service the platform does not control. diff --git a/microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.bad.al b/microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.bad.al new file mode 100644 index 0000000..0eb1590 --- /dev/null +++ b/microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.bad.al @@ -0,0 +1,23 @@ +enum 50815 "Upgrade Sample EnumInsert Bad" +{ + Extensible = true; + + value(0; First) { Caption = 'First'; } + + // Inserting at ordinal 1 shifts everything below. Every row that stored + // ordinal 1 before now resolves to NewMiddleValue. + value(1; NewMiddleValue) { Caption = 'New middle value'; } + + value(2; Second) { Caption = 'Second'; } + value(3; Third) { Caption = 'Third'; } +} + +enum 50816 "Upgrade Sample EnumRemove Bad" +{ + Extensible = true; + + value(0; First) { Caption = 'First'; } + // value(1; Second) removed without obsoletion. + // Existing rows storing ordinal 1 no longer resolve to any declared value. + value(2; Third) { Caption = 'Third'; } +} diff --git a/microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.good.al b/microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.good.al new file mode 100644 index 0000000..073a9c6 --- /dev/null +++ b/microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.good.al @@ -0,0 +1,29 @@ +enum 50813 "Upgrade Sample EnumAdditive Good" +{ + Extensible = true; + + value(0; First) { Caption = 'First'; } + value(1; Second) { Caption = 'Second'; } + value(2; Third) { Caption = 'Third'; } + + // New value appended at the next free ordinal. Existing stored ordinals + // (0, 1, 2) keep their meaning. + value(3; NewValue) { Caption = 'New value'; } +} + +enum 50814 "Upgrade Sample EnumRetire Good" +{ + Extensible = true; + + value(0; First) { Caption = 'First'; } + + value(1; Second) + { + Caption = 'Second'; + ObsoleteState = Removed; + ObsoleteReason = 'Replaced by NewValue.'; + ObsoleteTag = '28.0'; + } + + value(2; Third) { Caption = 'Third'; } +} diff --git a/microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.md b/microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.md new file mode 100644 index 0000000..1dcba69 --- /dev/null +++ b/microsoft/knowledge/upgrade/enum-changes-must-be-additive-at-the-end.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [enum, ordinal, obsolete, backward-compatibility, breaking-change] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Enum changes must be additive at the end; never insert or remove values + +## Description + +AL enums store their ordinal on disk. Inserting a new value in the middle of an existing enum shifts every following ordinal by one: every row whose field holds the old ordinal N now resolves to the value that used to be N+1. Removing a value without obsoletion has the same effect. Both changes are data corruption disguised as a code edit and are effectively irreversible once a tenant has upgraded. Adding values at the end is safe — existing ordinals keep their meaning. + +## Best Practice + +Append new enum values at the end, taking the next free ordinal. When a value must be retired, mark it with `ObsoleteState = Removed`, `ObsoleteReason`, and `ObsoleteTag` so tooling and downstream code can detect the deprecation; do not reclaim the ordinal. Renaming the caption on an existing ordinal is fine. + +See sample: `enum-changes-must-be-additive-at-the-end.good.al`. + +## Anti Pattern + +Inserting `value(1; "NewMiddleValue")` between existing `value(0; "First")` and the original `value(1; "Second")`. Every row that stored ordinal 1 before the change now reads as `NewMiddleValue`. The same applies to removing a value outright without obsoletion. + +See sample: `enum-changes-must-be-additive-at-the-end.bad.al`. diff --git a/microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.bad.al b/microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.bad.al new file mode 100644 index 0000000..2003dab --- /dev/null +++ b/microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.bad.al @@ -0,0 +1,19 @@ +codeunit 50807 "Upgrade Sample GuardReads Bad" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + var + Setup: Record "Sales & Receivables Setup"; + Customer: Record Customer; + begin + // Unguarded Get. One tenant whose Setup row is missing blocks the upgrade. + Setup.Get(); + + // Unguarded FindSet. Raises when the table is empty for this tenant. + Customer.FindSet(); + repeat + // per-row work + until Customer.Next() = 0; + end; +} diff --git a/microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.good.al b/microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.good.al new file mode 100644 index 0000000..f98f45d --- /dev/null +++ b/microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.good.al @@ -0,0 +1,23 @@ +codeunit 50806 "Upgrade Sample GuardReads Good" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + begin + UpgradeDefaults(); + end; + + local procedure UpgradeDefaults() + var + Setup: Record "Sales & Receivables Setup"; + Customer: Record Customer; + begin + if not Setup.Get() then + exit; + + if Customer.FindSet() then + repeat + // per-row work + until Customer.Next() = 0; + end; +} diff --git a/microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.md b/microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.md new file mode 100644 index 0000000..f3c6f99 --- /dev/null +++ b/microsoft/knowledge/upgrade/guard-every-database-read-in-upgrade-codeunits.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [upgrade, get, findset, findlast, guard, unblocking] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Guard every database read in upgrade codeunits; never let a missing row block the upgrade + +## Description + +An unguarded `Record.Get()` raises when the row does not exist; an unguarded `FindSet()` or `FindLast()` raises when the result set is empty. In ordinary runtime code those errors surface to a user who can retry. In an upgrade codeunit they abort the upgrade of the tenant and the customer is blocked from getting to the new version. Real-world data is inconsistent enough — missing lookup rows, empty setup tables, skipped modules — that an unguarded read reliably blocks at least one customer per release. + +## Best Practice + +Wrap every Get, FindSet, FindFirst, FindLast, and related call in an `if … then` guard. On the not-found path, either exit the current step or log telemetry and continue; never let the upgrade scope raise. `if Customer.FindSet() then;` (statement terminator as the entire body) is an acceptable pattern when only the side effect of positioning matters. + +See sample: `guard-every-database-read-in-upgrade-codeunits.good.al`. + +## Anti Pattern + +`Customer.Get(CustomerNo);` or `SalesHeader.FindLast();` inside an upgrade procedure. One missing row in one tenant turns every future upgrade into a support ticket. + +See sample: `guard-every-database-read-in-upgrade-codeunits.bad.al`. diff --git a/microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.bad.al b/microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.bad.al new file mode 100644 index 0000000..dd54bbd --- /dev/null +++ b/microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.bad.al @@ -0,0 +1,14 @@ +tableextension 50812 "Upgrade Sample InitValue Bad" extends Customer +{ + fields + { + field(50101; "Is Active"; Boolean) + { + DataClassification = CustomerContent; + Caption = 'Is active'; + // InitValue applies to new records only. + // Every existing customer remains Is Active = false after the upgrade. + InitValue = true; + } + } +} diff --git a/microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.good.al b/microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.good.al new file mode 100644 index 0000000..e26cfa3 --- /dev/null +++ b/microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.good.al @@ -0,0 +1,43 @@ +tableextension 50810 "Upgrade Sample InitValue Good" extends Customer +{ + fields + { + field(50100; "Is Active"; Boolean) + { + DataClassification = CustomerContent; + Caption = 'Is active'; + InitValue = true; + } + } +} + +codeunit 50811 "Upgrade Sample InitValue Good Upg" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + begin + UpgradeExistingCustomersIsActive(); + end; + + local procedure UpgradeExistingCustomersIsActive() + var + Customer: Record Customer; + CustomerDataTransfer: DataTransfer; + UpgradeTag: Codeunit "Upgrade Tag"; + begin + if UpgradeTag.HasUpgradeTag(UpgradeCustomerIsActiveTag()) then + exit; + + CustomerDataTransfer.SetTables(Database::Customer, Database::Customer); + CustomerDataTransfer.AddConstantValue(true, Customer.FieldNo("Is Active")); + CustomerDataTransfer.CopyFields(); + + UpgradeTag.SetUpgradeTag(UpgradeCustomerIsActiveTag()); + end; + + local procedure UpgradeCustomerIsActiveTag(): Code[250] + begin + exit('MS-000006-CustomerIsActive-20260501'); + end; +} diff --git a/microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.md b/microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.md new file mode 100644 index 0000000..6f0b612 --- /dev/null +++ b/microsoft/knowledge/upgrade/initvalue-does-not-populate-existing-records.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [initvalue, field, upgrade, existing-records, migration] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# InitValue on a new field does not populate existing rows + +## Description + +The `InitValue` property sets a field's default for rows created after the field exists. Rows that already exist when the field is added keep the data-type default (empty text, zero, false, epoch date) — InitValue does not retroactively apply. Shipping a new field with `InitValue = true` on an existing table produces a silently inconsistent dataset: new rows match the intended default, existing rows do not, and callers that do not distinguish the two read the wrong state for existing data. + +## Best Practice + +When adding a field to an existing table with a meaningful default, write an upgrade step that populates existing rows with the same value, guarded by its own upgrade tag. Use `DataTransfer` with `AddConstantValue` for set-based initialization (see `use-datatransfer-for-large-dataset-initialization`). Exceptions: brand-new tables, new Boolean fields where `false` is the correct value for existing rows, and informational fields where empty is an acceptable state. + +See sample: `initvalue-does-not-populate-existing-records.good.al`. + +## Anti Pattern + +Adding `field(100; "Is Active"; Boolean) { InitValue = true; }` to an existing business table without upgrade code. New records are Active; every existing record is silently inactive. The bug surfaces later as "why is this data missing from the default report?" + +See sample: `initvalue-does-not-populate-existing-records.bad.al`. diff --git a/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.bad.al b/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.bad.al new file mode 100644 index 0000000..d2ebe49 --- /dev/null +++ b/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.bad.al @@ -0,0 +1,22 @@ +codeunit 50805 "Upgrade Sample TagRegister Bad" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + var + UpgradeTag: Codeunit "Upgrade Tag"; + begin + if UpgradeTag.HasUpgradeTag(FeatureXUpgradeTag()) then + exit; + + UpgradeTag.SetUpgradeTag(FeatureXUpgradeTag()); + end; + + // Missing OnGetPerCompanyUpgradeTags subscriber. + // The tag is set but the platform's upgrade-tag machinery does not know about it. + + local procedure FeatureXUpgradeTag(): Code[250] + begin + exit('MS-000004-FeatureX-20260501'); + end; +} diff --git a/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.good.al b/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.good.al new file mode 100644 index 0000000..0fb5118 --- /dev/null +++ b/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.good.al @@ -0,0 +1,25 @@ +codeunit 50804 "Upgrade Sample TagRegister Good" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + var + UpgradeTag: Codeunit "Upgrade Tag"; + begin + if UpgradeTag.HasUpgradeTag(FeatureXUpgradeTag()) then + exit; + + UpgradeTag.SetUpgradeTag(FeatureXUpgradeTag()); + end; + + [EventSubscriber(ObjectType::Codeunit, Codeunit::"Upgrade Tag", 'OnGetPerCompanyUpgradeTags', '', false, false)] + local procedure RegisterPerCompanyTags(var PerCompanyUpgradeTags: List of [Code[250]]) + begin + PerCompanyUpgradeTags.Add(FeatureXUpgradeTag()); + end; + + local procedure FeatureXUpgradeTag(): Code[250] + begin + exit('MS-000003-FeatureX-20260501'); + end; +} diff --git a/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.md b/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.md new file mode 100644 index 0000000..fd260dd --- /dev/null +++ b/microsoft/knowledge/upgrade/register-upgrade-tags-with-subscribers.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [upgrade-tag, ongetpercompanyupgradetags, ongetperdatabaseupgradetags, registration] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Register every upgrade tag with the matching PerCompany or PerDatabase subscriber + +## Description + +An upgrade tag set via `UpgradeTag.SetUpgradeTag` only participates in the platform's upgrade-tag machinery when it is also registered through `OnGetPerCompanyUpgradeTags` or `OnGetPerDatabaseUpgradeTags` event subscribers on `Codeunit "Upgrade Tag"`. Without registration, the platform cannot enumerate the tag for diagnostic reporting, skipped-step detection, or cross-app coordination. The step still runs and sets the tag, but the tag is effectively invisible to the rest of the upgrade infrastructure. + +## Best Practice + +For every upgrade-tag constant referenced in `HasUpgradeTag`/`SetUpgradeTag`, register it in the subscriber that matches its trigger scope: tags used from `OnUpgradePerCompany` go in `OnGetPerCompanyUpgradeTags`; tags used from `OnUpgradePerDatabase` go in `OnGetPerDatabaseUpgradeTags`. Keep the tag string in a single source (Label or function) and reference it at the guard, the setter, and the registration. + +See sample: `register-upgrade-tags-with-subscribers.good.al`. + +## Anti Pattern + +Adding a new `UpgradeTag.SetUpgradeTag(MyTag())` without the matching `PerCompanyUpgradeTags.Add(MyTag())` in the registration subscriber. The code compiles and the step completes, but the tag is unregistered and the infrastructure is partially disabled. + +See sample: `register-upgrade-tags-with-subscribers.bad.al`. diff --git a/microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.bad.al b/microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.bad.al new file mode 100644 index 0000000..300b5d7 --- /dev/null +++ b/microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.bad.al @@ -0,0 +1,14 @@ +codeunit 50820 "Upgrade Sample SkipContext Bad" +{ + procedure AddReportSelectionEntries() + begin + // No execution-context check. On upgrade, this either throws on + // primary-key conflict or silently overwrites the tenant's + // customized report selections. + InsertDefaultReportSelections(); + end; + + local procedure InsertDefaultReportSelections() + begin + end; +} diff --git a/microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.good.al b/microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.good.al new file mode 100644 index 0000000..04ade74 --- /dev/null +++ b/microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.good.al @@ -0,0 +1,15 @@ +codeunit 50819 "Upgrade Sample SkipContext Good" +{ + procedure AddReportSelectionEntries() + begin + // Existing tenants already have the selections, possibly customized. + if GetExecutionContext() = ExecutionContext::Upgrade then + exit; + + InsertDefaultReportSelections(); + end; + + local procedure InsertDefaultReportSelections() + begin + end; +} diff --git a/microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.md b/microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.md new file mode 100644 index 0000000..39a53df --- /dev/null +++ b/microsoft/knowledge/upgrade/skip-non-essential-work-during-upgrade-context.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [executioncontext, upgrade, reportselections, initialization, install] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Skip non-essential initialization when ExecutionContext is Upgrade + +## Description + +Initialization code that inserts default rows — report selections, number-series, setup-table defaults — is correct on first install and harmful during upgrade. Existing tenants already have these rows, possibly customized; re-running the initialization either fails on primary-key conflicts or silently overwrites customer configuration. The platform exposes `GetExecutionContext()` so the same procedure can be safely called from install and upgrade paths without duplicating the insert logic. + +## Best Practice + +Check `if GetExecutionContext() = ExecutionContext::Upgrade then exit;` at the top of idempotent-on-install-only procedures. Keep the early exit narrow and document the reason. The check should be additive to existing guards, not a replacement for proper primary-key handling in the insert itself. + +See sample: `skip-non-essential-work-during-upgrade-context.good.al`. + +## Anti Pattern + +A procedure that unconditionally inserts a default report-selection, number-series, or setup row, called from both install and upgrade paths. On upgrade it either throws on the conflicting key or overwrites the tenant's existing configuration. + +See sample: `skip-non-essential-work-during-upgrade-context.bad.al`. diff --git a/microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.bad.al b/microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.bad.al new file mode 100644 index 0000000..c232adb --- /dev/null +++ b/microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.bad.al @@ -0,0 +1,22 @@ +codeunit 50809 "Upgrade Sample DataTransfer Bad" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + begin + InitializeNewFlag(); + end; + + local procedure InitializeNewFlag() + var + CustLedgerEntry: Record "Cust. Ledger Entry"; + begin + // Row-at-a-time update over a 10M-row ledger table. Multi-hour upgrade. + CustLedgerEntry.SetRange(Open, true); + if CustLedgerEntry.FindSet(true) then + repeat + CustLedgerEntry."New Flag" := false; + CustLedgerEntry.Modify(); + until CustLedgerEntry.Next() = 0; + end; +} diff --git a/microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.good.al b/microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.good.al new file mode 100644 index 0000000..b45a9b9 --- /dev/null +++ b/microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.good.al @@ -0,0 +1,31 @@ +codeunit 50808 "Upgrade Sample DataTransfer Good" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + begin + InitializeNewFlag(); + end; + + local procedure InitializeNewFlag() + var + CustLedgerEntry: Record "Cust. Ledger Entry"; + CLEDataTransfer: DataTransfer; + UpgradeTag: Codeunit "Upgrade Tag"; + begin + if UpgradeTag.HasUpgradeTag(InitializeNewFlagTag()) then + exit; + + CLEDataTransfer.SetTables(Database::"Cust. Ledger Entry", Database::"Cust. Ledger Entry"); + CLEDataTransfer.AddSourceFilter(CustLedgerEntry.FieldNo(Open), '=%1', true); + CLEDataTransfer.AddConstantValue(false, CustLedgerEntry.FieldNo("New Flag")); + CLEDataTransfer.CopyFields(); + + UpgradeTag.SetUpgradeTag(InitializeNewFlagTag()); + end; + + local procedure InitializeNewFlagTag(): Code[250] + begin + exit('MS-000005-CLEInitializeNewFlag-20260501'); + end; +} diff --git a/microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.md b/microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.md new file mode 100644 index 0000000..7a9afc9 --- /dev/null +++ b/microsoft/knowledge/upgrade/use-datatransfer-for-large-dataset-initialization.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [datatransfer, initvalue, large-dataset, bulk-update, upgrade] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Use DataTransfer to initialize large tables in upgrade; not FindSet plus Modify + +## Description + +An upgrade that populates a new field on millions of existing rows with a FindSet+Modify loop pays a round-trip and a per-row trigger invocation for every row — turning a multi-hour upgrade into a multi-day one on ledger-entry-scale tables. `DataTransfer` pushes the update to SQL as a single set-based operation using source filters and constant values, which is the supported platform mechanism for this scenario. The tradeoff: DataTransfer bypasses validation triggers and event subscribers — if the step depends on trigger logic, that has to be reconstructed explicitly. + +## Best Practice + +Use DataTransfer for field-default initialization on existing tables, especially when the target is a ledger-entry or document-line table. Set tables, add source filters, add constant values, call CopyFields, clear, and repeat for additional slices. When trigger or subscriber behaviour is required, do that work separately against a filtered result set so the bulk update remains set-based. + +See sample: `use-datatransfer-for-large-dataset-initialization.good.al`. + +## Anti Pattern + +`FindSet(true)` + `Modify()` in a loop as the initialization path for a new field across an entire existing table. The resulting upgrade time is proportional to the row count; for a ten-million-row ledger-entry table it is the single largest step in the release. + +See sample: `use-datatransfer-for-large-dataset-initialization.bad.al`. diff --git a/microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.bad.al b/microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.bad.al new file mode 100644 index 0000000..3ec4385 --- /dev/null +++ b/microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.bad.al @@ -0,0 +1,11 @@ +codeunit 50818 "Upgrade Sample Obsolete Bad" +{ + // Straight to Removed with no preceding Pending phase, no ObsoleteReason, + // no ObsoleteTag. Dependents compiled against the previous release hit + // a hard compile error with no migration signal. + [Obsolete('', '')] + procedure CalculateNetAmount(Amount: Decimal): Decimal + begin + Error('Removed.'); + end; +} diff --git a/microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.good.al b/microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.good.al new file mode 100644 index 0000000..2dc1f9c --- /dev/null +++ b/microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.good.al @@ -0,0 +1,13 @@ +codeunit 50817 "Upgrade Sample Obsolete Good" +{ + [Obsolete('Use CalculateNetAmountV2 for the updated rounding semantics.', '28.0')] + procedure CalculateNetAmount(Amount: Decimal): Decimal + begin + exit(Amount); + end; + + procedure CalculateNetAmountV2(Amount: Decimal): Decimal + begin + exit(Amount); + end; +} diff --git a/microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.md b/microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.md new file mode 100644 index 0000000..0c5a4e8 --- /dev/null +++ b/microsoft/knowledge/upgrade/use-obsolete-pending-before-removed.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [obsolete, obsoletestate, obsoletereason, obsoletetag, deprecation] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Deprecate via ObsoleteState Pending first; move to Removed only after the grace window + +## Description + +AL's obsolete workflow is two-stage by design. `ObsoleteState = Pending` keeps the object or member compilable and callable but emits warnings and records the deprecation in metadata. `ObsoleteState = Removed` makes it a compile error for callers. Jumping straight to Removed — or marking Pending without `ObsoleteReason` and `ObsoleteTag` — breaks dependents who had no signal to migrate, and loses the tooling's ability to surface the planned removal in sandbox builds before the production tenant upgrades. + +## Best Practice + +Mark the element `ObsoleteState = Pending` with a concrete `ObsoleteReason` naming the replacement and an `ObsoleteTag` identifying the version the deprecation started. Keep it Pending through at least one major release so dependents have a cycle to migrate. Move to `ObsoleteState = Removed` only in a later release, with the same Reason and Tag retained or updated. + +See sample: `use-obsolete-pending-before-removed.good.al`. + +## Anti Pattern + +`[Obsolete('', '')]` or `ObsoleteState = Removed` applied directly on an element that was public and callable in the previous release, with no preceding Pending phase. Dependents get a hard compile error with no migration signal in the previous version. + +See sample: `use-obsolete-pending-before-removed.bad.al`. diff --git a/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.bad.al b/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.bad.al new file mode 100644 index 0000000..bccfb66 --- /dev/null +++ b/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.bad.al @@ -0,0 +1,27 @@ +codeunit 50803 "Upgrade Sample TagGuard Bad" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + var + AppInfo: ModuleInfo; + begin + NavApp.GetCurrentModuleInfo(AppInfo); + + // Version check: fragile across skipped versions, and every nested branch + // is another place a customer can be stuck if the matching step fails. + if AppInfo.DataVersion().Major < 18 then + UpgradeFeatureA() + else + if AppInfo.DataVersion().Major < 21 then + UpgradeFeatureB(); + end; + + local procedure UpgradeFeatureA() + begin + end; + + local procedure UpgradeFeatureB() + begin + end; +} diff --git a/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.good.al b/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.good.al new file mode 100644 index 0000000..4c90219 --- /dev/null +++ b/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.good.al @@ -0,0 +1,26 @@ +codeunit 50802 "Upgrade Sample TagGuard Good" +{ + Subtype = Upgrade; + + trigger OnUpgradePerCompany() + begin + UpgradeFeatureX(); + end; + + local procedure UpgradeFeatureX() + var + UpgradeTag: Codeunit "Upgrade Tag"; + begin + if UpgradeTag.HasUpgradeTag(FeatureXUpgradeTag()) then + exit; + + // Idempotent, retries cleanly after failure, runs exactly once. + + UpgradeTag.SetUpgradeTag(FeatureXUpgradeTag()); + end; + + local procedure FeatureXUpgradeTag(): Code[250] + begin + exit('MS-000002-FeatureX-20260501'); + end; +} diff --git a/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.md b/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.md new file mode 100644 index 0000000..68f81c0 --- /dev/null +++ b/microsoft/knowledge/upgrade/use-upgrade-tags-not-version-checks.md @@ -0,0 +1,26 @@ +--- +bc-version: [all] +domain: upgrade +keywords: [upgrade-tag, dataversion, version-check, idempotent, guard] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Guard upgrade steps with upgrade tags, not version checks + +## Description + +`DataVersion()` comparisons tie an upgrade step to a specific release cadence: if the step is skipped or fails on one version and the tenant upgrades past the check before the step succeeds, the step never runs. Upgrade tags, managed by `Codeunit "Upgrade Tag"`, record per-step completion in the tenant database. A tag-guarded step runs once, retries cleanly after failure, and remains idempotent across future versions regardless of the version the customer is upgrading from. + +## Best Practice + +Guard each step with `if UpgradeTag.HasUpgradeTag(MyTag()) then exit;` at the top of the procedure. After the step completes, call `UpgradeTag.SetUpgradeTag(MyTag())`. Define the tag string in a `Tok`-suffixed Label or returning function so the same constant is referenced at both the guard and the registration (see `register-upgrade-tags-with-getpercompany-getperdatabase-subscribers`). + +See sample: `use-upgrade-tags-not-version-checks.good.al`. + +## Anti Pattern + +`if MyApp.DataVersion().Major < 18 then UpgradeFeatureA();` — the step runs on every upgrade from a pre-18 version, may fail on partial data, and the next retry re-runs work that already succeeded. Nesting version-check branches (`< 14` → step A, `< 17` → step B) compounds the fragility. + +See sample: `use-upgrade-tags-not-version-checks.bad.al`.