mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Merge pull request #7 from microsoft/preview/extract-review-agent-knowledge
Extract 55 knowledge articles from BC review-agent prompt
This commit is contained in:
commit
ca80df9226
131 changed files with 2799 additions and 0 deletions
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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; }
|
||||
}
|
||||
}
|
||||
|
|
@ -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; }
|
||||
}
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
26
microsoft/knowledge/style/apply-approved-label-suffixes.md
Normal file
26
microsoft/knowledge/style/apply-approved-label-suffixes.md
Normal file
|
|
@ -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`.
|
||||
|
|
@ -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.") { }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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) { }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
26
microsoft/knowledge/style/follow-api-page-naming-rules.md
Normal file
26
microsoft/knowledge/style/follow-api-page-naming-rules.md
Normal file
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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; }
|
||||
}
|
||||
}
|
||||
|
|
@ -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; }
|
||||
}
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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 `<ObjectName>.<ObjectType>.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.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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(<field>)` 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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
26
microsoft/knowledge/style/use-this-keyword-in-codeunits.md
Normal file
26
microsoft/knowledge/style/use-this-keyword-in-codeunits.md
Normal file
|
|
@ -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`.
|
||||
|
|
@ -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';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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`.
|
||||
22
microsoft/knowledge/ui/avoid-banned-ui-terms.md
Normal file
22
microsoft/knowledge/ui/avoid-banned-ui-terms.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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`.
|
||||
|
|
@ -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';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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 <what the field represents>.` — 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`.
|
||||
22
microsoft/knowledge/ui/respect-ui-text-character-limits.md
Normal file
22
microsoft/knowledge/ui/respect-ui-text-character-limits.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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.
|
||||
|
|
@ -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.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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`.
|
||||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Add table
Add a link
Reference in a new issue