Merge microsoft/main into document-distribution-batch

Resolve al-data-modeling-review.md by union: scope, not-applicable
definition and targeted cues now cover both this PR's document
distribution/price/barcode topics and main's dimension-wiring,
posting-routine and Item Ledger Entry topics.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Michael Dieringer 2026-09-29 16:47:38 +02:00
commit b6a59be325
109 changed files with 3379 additions and 46 deletions

View file

@ -30,9 +30,10 @@ function Assert-ThrowsLike {
} }
$generator = Join-Path $Root 'tools/Build-SkillIndex.ps1' $generator = Join-Path $Root 'tools/Build-SkillIndex.ps1'
$resolver = Join-Path $Root 'tools/Resolve-SkillWorklist.ps1'
$indexSchema = Join-Path $Root 'schemas/skill-index.schema.json' $indexSchema = Join-Path $Root 'schemas/skill-index.schema.json'
$reportSchema = Join-Path $Root 'schemas/findings-report.schema.json' $reportSchema = Join-Path $Root 'schemas/findings-report.schema.json'
foreach ($path in $generator, $indexSchema, $reportSchema) { foreach ($path in $generator, $resolver, $indexSchema, $reportSchema) {
if (-not (Test-Path -LiteralPath $path -PathType Leaf)) { if (-not (Test-Path -LiteralPath $path -PathType Leaf)) {
throw "Required contract file not found: $path" throw "Required contract file not found: $path"
} }
@ -235,9 +236,83 @@ Output.
Assert-ThrowsLike -Pattern '*Nested super-skills are not supported*' -Action { Assert-ThrowsLike -Pattern '*Nested super-skills are not supported*' -Action {
& $generator -BCQualityRoot $fixtureRoot -IndexPath (Join-Path $tmp 'nested.json') & $generator -BCQualityRoot $fixtureRoot -IndexPath (Join-Path $tmp 'nested.json')
} }
Remove-Item -LiteralPath (Join-Path $fixtureSkills 'al-nested-review.md') -Force
$validSuper = @'
---
kind: action-skill
id: al-code-review
version: 1
title: Review
description: Test super-skill.
inputs: [file-path]
outputs: [findings-report]
sub-skills:
- microsoft/skills/review/al-leaf-review.md
---
# Review
## Source
Source.
## Relevance
Relevance.
## Worklist
Worklist.
## Action
Action.
## Output
Output.
'@
Set-Content -LiteralPath $superPath -Value $validSuper -Encoding utf8NoBOM
$customSkills = Join-Path -Path $fixtureRoot -ChildPath 'custom/skills/review'
New-Item -ItemType Directory -Path $customSkills -Force | Out-Null
$customLeaf = $leaf.Replace('title: Leaf', 'title: Custom Leaf')
Set-Content -LiteralPath (Join-Path $customSkills 'custom-leaf-review.md') -Value $customLeaf -Encoding utf8NoBOM
$layeredPath = Join-Path $tmp 'layered.json'
& $generator -BCQualityRoot $fixtureRoot -IndexPath $layeredPath | Out-Null
$layeredIndex = Get-Content -LiteralPath $layeredPath -Raw | ConvertFrom-Json
$layeredLeaves = @($layeredIndex.skills | Where-Object id -eq 'al-leaf-review')
if ($layeredLeaves.Count -ne 2) {
throw "Expected both layered al-leaf-review implementations, found $($layeredLeaves.Count)."
}
if ((@($layeredLeaves.layer | Sort-Object) -join ',') -cne 'custom,microsoft') {
throw 'Layered al-leaf-review implementations did not preserve custom and microsoft records.'
}
$resolved = & $resolver -BCQualityRoot $fixtureRoot -IndexPath $layeredPath -SuperSkillPath (
'microsoft/skills/review/al-code-review.md'
)
if ($resolved.subSkills.Count -ne 1 -or
$resolved.subSkills[0].path -cne 'custom/skills/review/custom-leaf-review.md') {
throw 'The custom implementation did not win the layered leaf slot.'
}
$microsoftOnly = & $resolver -BCQualityRoot $fixtureRoot -IndexPath $layeredPath -SuperSkillPath (
'microsoft/skills/review/al-code-review.md'
) -EnabledLayers microsoft
if ($microsoftOnly.subSkills.Count -ne 1 -or
$microsoftOnly.subSkills[0].path -cne 'microsoft/skills/review/al-leaf-review.md') {
throw 'Disabling the custom layer did not fall back to the Microsoft implementation.'
}
$customDisabled = & $resolver -BCQualityRoot $fixtureRoot -IndexPath $layeredPath -SuperSkillPath (
'microsoft/skills/review/al-code-review.md'
) -DisabledSkills 'custom/skills/review/custom-leaf-review.md'
if ($customDisabled.subSkills.Count -ne 1 -or
$customDisabled.subSkills[0].path -cne 'microsoft/skills/review/al-leaf-review.md') {
throw 'Disabling the custom implementation did not fall back to Microsoft.'
}
Set-Content -LiteralPath (Join-Path $customSkills 'duplicate-leaf-review.md') -Value $customLeaf -Encoding utf8NoBOM
Assert-ThrowsLike -Pattern '*Duplicate action-skill IDs within a layer: custom:al-leaf-review*' -Action {
& $generator -BCQualityRoot $fixtureRoot -IndexPath (Join-Path $tmp 'duplicate-layer.json')
}
} }
finally { finally {
Remove-Item -LiteralPath $tmp -Recurse -Force -ErrorAction SilentlyContinue Remove-Item -LiteralPath $tmp -Recurse -Force -ErrorAction SilentlyContinue
} }
Write-Output "Skill-index check PASSED: deterministic, schema-valid, and all $($expectedLeaves.Count) review leaves preserved in order." Write-Output "Skill-index check PASSED: deterministic, schema-valid, layered overrides resolved, and all $($expectedLeaves.Count) review leaves preserved in order."

View file

@ -688,12 +688,16 @@ def run(root: Path) -> Report:
if domain_dir.is_dir(): if domain_dir.is_dir():
validate_samples_in_domain(domain_dir, root, report) validate_samples_in_domain(domain_dir, root, report)
# Third pass: R24 unique ids within kind # Third pass: R24 unique ids within kind. Layered action-skill overrides
# may share an id, but two definitions in one layer are ambiguous.
by_kind: dict[str, dict[str, list[Path]]] = {} by_kind: dict[str, dict[str, list[Path]]] = {}
for rec in skill_records: for rec in skill_records:
if rec.skill_id is None: if rec.skill_id is None:
continue continue
by_kind.setdefault(rec.kind, {}).setdefault(rec.skill_id, []).append(rec.path) scope = rec.kind
if rec.kind == "action-skill":
scope = f"{rec.kind}:{rec.path.relative_to(root).parts[0]}"
by_kind.setdefault(scope, {}).setdefault(rec.skill_id, []).append(rec.path)
for kind, by_id in by_kind.items(): for kind, by_id in by_kind.items():
for sid, paths in by_id.items(): for sid, paths in by_id.items():
if len(paths) > 1: if len(paths) > 1:

View file

@ -12,7 +12,26 @@ jobs:
steps: steps:
- name: Check out repository - name: Check out repository
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
fetch-depth: 0
- name: Collect changed paths
shell: pwsh
env:
BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }}
run: |
$paths = if (-not $env:BASE_SHA -or $env:BASE_SHA -match '^0+$') {
@(git diff-tree --no-commit-id --name-only -r $env:GITHUB_SHA)
} else {
@(git diff --name-only $env:BASE_SHA $env:GITHUB_SHA)
}
$paths | Set-Content -LiteralPath "$env:RUNNER_TEMP/changed-paths.txt" -Encoding utf8NoBOM
- name: Validate review evaluation corpus - name: Validate review evaluation corpus
shell: pwsh shell: pwsh
run: ./tools/Test-ReviewFixtures.ps1 -Root . -PrepareDirectory "$env:RUNNER_TEMP/bcquality-review-fixtures" run: |
./tools/Test-ReviewFixtures.ps1 -Root . `
-PrepareDirectory "$env:RUNNER_TEMP/bcquality-review-fixtures" `
-ChangedPathsFile "$env:RUNNER_TEMP/changed-paths.txt" `
-CoverageReportPath "$env:RUNNER_TEMP/review-coverage.json"
Get-Content -LiteralPath "$env:RUNNER_TEMP/review-coverage.json"

View file

@ -44,6 +44,15 @@ Otherwise the layers are additive. A matching filename alone does not suppress
an article; the [READ contract](../skills/read.md#layer-precedence) governs an article; the [READ contract](../skills/read.md#layer-precedence) governs
knowledge conflicts. Review reports record displaced knowledge in `suppressed`. knowledge conflicts. Review reports record displaced knowledge in `suppressed`.
Action skills use the same layer order but override by frontmatter `id`. A
custom leaf with the same `id` as a Community or Microsoft leaf replaces that
leaf in every super-skill slot while its layer is enabled. The files may have
different names. IDs must remain unique within each layer. Disabling the custom
layer or the custom skill path makes composition fall back to the next enabled
implementation. Hosts should build the skill index and use
`tools/Resolve-SkillWorklist.ps1`; they must not implement this selection from
filenames.
Layer selection is **not an access-control boundary**. A plugin installation Layer selection is **not an access-control boundary**. A plugin installation
still contains excluded layers on disk. An integration requiring genuine still contains excluded layers on disk. An integration requiring genuine
exclusion must remove denied files from its own content copy before the agent exclusion must remove denied files from its own content copy before the agent

View file

@ -49,16 +49,20 @@ only result.
action skills to run. Do not reproduce its routing logic. action skills to run. Do not reproduce its routing logic.
3. Execute every dispatched action skill with the exact input subset in its 3. Execute every dispatched action skill with the exact input subset in its
dispatch record. Read `skills/read.md` and `skills/do.md` on demand. dispatch record. Read `skills/read.md` and `skills/do.md` on demand.
4. When an action skill declares `sub-skills`, execute every relevant leaf as a 4. When an action skill declares `sub-skills`, resolve its ordered leaf slots
discrete invocation. Leaves are independent and may be scheduled serially with `tools/Resolve-SkillWorklist.ps1`, passing the enabled layers and
or concurrently. disabled skill paths from the task context. Execute every resolved leaf as
a discrete invocation. Leaves are independent and may be scheduled serially
or concurrently.
5. Capture the exact Task return as the immutable raw audit payload and primary 5. Capture the exact Task return as the immutable raw audit payload and primary
transport. Preserve it unchanged in private artifacts or host logs. Before transport. Preserve it unchanged in private artifacts or host logs. Before
the full DO acceptance gate, create a normalized candidate only for DO's the full DO acceptance gate, create a normalized candidate only for DO's
bounded optional-range case, record that normalization separately in private bounded optional-range case, record that normalization separately in private
telemetry, and accept the candidate only if the entire copy passes the telemetry, and accept the candidate only if the entire copy passes the
unchanged strict gate. The accepted report contains no undeclared telemetry unchanged strict gate. Use `tools/Validate-FindingsReport.ps1`, passing the
fields. exact source paths and fully retrieved article paths; pass `-SkillKind super`
for the final rolled-up report. The accepted report contains no undeclared
telemetry fields.
6. Collect each accepted findings-report into `sub-results` in the declared 6. Collect each accepted findings-report into `sub-results` in the declared
`sub-skills` order, not completion order. Run the super-skill self-review `sub-skills` order, not completion order. Run the super-skill self-review
only after all leaves have finished. only after all leaves have finished.
@ -92,6 +96,8 @@ A compatible runner:
- invokes every worklisted leaf exactly once unless a documented retry replaces - invokes every worklisted leaf exactly once unless a documented retry replaces
a failed attempt; a failed attempt;
- resolves same-ID leaf implementations by `custom > community > microsoft`,
preserves declared slot order, and falls back when a higher layer is disabled;
- keeps leaf contexts isolated and passes only the inputs they declare; - keeps leaf contexts isolated and passes only the inputs they declare;
- preserves each raw Task return unchanged for audit and distinguishes it from - preserves each raw Task return unchanged for audit and distinguishes it from
any normalized accepted copy; any normalized accepted copy;

View file

@ -4,6 +4,15 @@ The evaluation is convention-driven. The harness discovers every `<layer>/skills
`review-fixtures.json` contains only global thresholds and optional exceptional overrides. An override may select a different `article`, add context when the generic convention cannot express a scenario, or use an `articles` array when one domain needs explicit regression coverage for several paired articles. Specify either `article` or `articles`, not both. The first selected article retains the stable `<domain>-bad` and `<domain>-good` manifest IDs; additional articles use slug-qualified IDs. Overrides should remain empty in the normal case. `review-fixtures.json` contains only global thresholds and optional exceptional overrides. An override may select a different `article`, add context when the generic convention cannot express a scenario, or use an `articles` array when one domain needs explicit regression coverage for several paired articles. Specify either `article` or `articles`, not both. The first selected article retains the stable `<domain>-bad` and `<domain>-good` manifest IDs; additional articles use slug-qualified IDs. Overrides should remain empty in the normal case.
CI also measures selected paired articles against every effective article that
has both AL companions. A changed paired article must be selected by the
domain convention or an override. When adding it would not provide a useful
deterministic regression, add a narrow `coverageWaivers` entry with its exact
article path and a non-empty reason. Waivers are reviewable exceptions, not a
substitute for domain coverage. The generated coverage report includes totals
and per-domain ratios; the ratio is informational, while changed-file coverage
is mandatory.
Model-facing preparation hashes case IDs, neutralizes `Good`/`Bad` object-name tokens, and removes full-line sample comments so neither the article slug, domain, nor expected outcome reveals the answer. Model-facing preparation hashes case IDs, neutralizes `Good`/`Bad` object-name tokens, and removes full-line sample comments so neither the article slug, domain, nor expected outcome reveals the answer.
The SCM `articles` override deliberately selects every rule in the initial The SCM `articles` override deliberately selects every rule in the initial
@ -36,6 +45,13 @@ pwsh ./tools/Test-ReviewFixtures.ps1 -Root .
This credential-free check proves every selected leaf maps to a same-named knowledge domain with at least one complete AL sample pair and that all configured overrides are valid. This credential-free check proves every selected leaf maps to a same-named knowledge domain with at least one complete AL sample pair and that all configured overrides are valid.
To reproduce the changed-file gate and emit the same measurable report as CI:
```powershell
git diff --name-only origin/main...HEAD | Set-Content .changed-paths.txt
pwsh ./tools/Test-ReviewFixtures.ps1 -Root . -ChangedPathsFile .changed-paths.txt -CoverageReportPath .coverage.json
```
## Run a fast-model evaluation ## Run a fast-model evaluation
1. Prepare neutral inputs: 1. Prepare neutral inputs:

View file

@ -3,6 +3,7 @@
"selection": "first-paired-al-article", "selection": "first-paired-al-article",
"minimumExpectedRecall": 1.0, "minimumExpectedRecall": 1.0,
"minimumCleanRate": 1.0, "minimumCleanRate": 1.0,
"coverageWaivers": [],
"overrides": { "overrides": {
"agents": { "agents": {
"article": "wire-all-three-agent-interfaces" "article": "wire-all-three-agent-interfaces"
@ -89,7 +90,14 @@
"reconcile-warehouse-adjustments-with-the-item-ledger", "reconcile-warehouse-adjustments-with-the-item-ledger",
"post-transfers-through-shipment-and-receipt-codeunits", "post-transfers-through-shipment-and-receipt-codeunits",
"use-date-aware-availability-for-promising", "use-date-aware-availability-for-promising",
"carry-out-requisition-actions-through-the-standard-workflow" "carry-out-requisition-actions-through-the-standard-workflow",
"derive-base-quantities-through-the-line-unit-of-measure"
]
},
"security": {
"articles": [
"al-has-no-built-in-htmlencode",
"do-not-concatenate-external-text-into-setfilter"
] ]
}, },
"style": { "style": {
@ -102,14 +110,23 @@
"article": "telemetry-event-id-stable-unique" "article": "telemetry-event-id-stable-unique"
}, },
"testing": { "testing": {
"article": "ui-handlers-in-tests" "articles": [
"ui-handlers-in-tests",
"reset-per-test-state-before-the-isinitialized-guard"
]
}, },
"upgrade": { "upgrade": {
"article": "initvalue-does-not-update-existing-rows", "article": "initvalue-does-not-update-existing-rows",
"context": "The extended table existed in the previous app version and already contains rows." "context": "The extended table existed in the previous app version and already contains rows."
}, },
"web-services": { "web-services": {
"article": "expose-systemid-as-the-api-key" "articles": [
"expose-systemid-as-the-api-key",
"handle-httpclient-platform-failure-before-response-access",
"check-http-status-before-consuming-response-body",
"check-json-null-before-converting-values",
"format-exchanged-values-with-standard-format-9"
]
} }
} }
} }

View file

@ -0,0 +1,13 @@
codeunit 50104 "Import File Reader"
{
procedure ImportFile()
var
ImportFile: File;
InStream: InStream;
begin
ImportFile.WriteMode(false);
ImportFile.TextMode(true);
ImportFile.Open('C:\Import\data.txt'); // fails in SaaS — no local filesystem
ImportFile.CreateInStream(InStream);
end;
}

View file

@ -0,0 +1,24 @@
codeunit 50104 "Import File Reader"
{
procedure ImportFile()
var
TempBlob: Codeunit "Temp Blob";
FromInStream: InStream;
ToOutStream: OutStream;
InStream: InStream;
begin
if not UploadIntoStream('All Files (*.*)|*.*', FromInStream) then
exit;
TempBlob.CreateOutStream(ToOutStream);
CopyStream(ToOutStream, FromInStream);
TempBlob.CreateInStream(InStream);
ParseStream(InStream);
end;
local procedure ParseStream(var InStream: InStream)
begin
// parse InStream content here
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: appsource
keywords: [file-datatype, saas, onprem, uploadintostream, downloadfromstream, instream, outstream, streaming]
technologies: [al]
countries: [w1]
application-area: [all]
---
# The File data type's direct I/O methods are OnPrem-only
> Contributions welcome — open a PR to refine or extend this article.
## Description
The classic `File` variable type — `Open`/`Create`/`Read`/`Write`/`Close` against a path on the local or server filesystem — is scoped OnPrem-only. Code targeting Business Central Online that calls `File.Open`, `File.Create`, `File.Read`, or `File.Write` fails to compile against a Cloud-scoped project; it does not compile successfully and fail or get silently skipped at runtime. Separately, and regardless of the compile-time scoping, no server/local filesystem path is available to an extension actually running in Business Central Online.
## Best Practice
Use the stream-based equivalents: `UploadIntoStream` to read user-selected file content into an `InStream`, and `DownloadFromStream` to write an `OutStream`'s content to a file the user saves. Stage the content in a `TempBlob` between the stream and the rest of the parsing/formatting code.
See sample: [`file-datatype-saas.good.al`](file-datatype-saas.good.al).
## Anti Pattern
Opening a hardcoded or user-supplied filesystem path with the `File` variable type. This is a strong signal the code was written for on-premises only, or copied from material that predates the cloud-first streaming APIs.
See sample: [`file-datatype-saas.bad.al`](file-datatype-saas.bad.al).

View file

@ -0,0 +1,9 @@
codeunit 50103 "Order Confirmation Notifier"
{
procedure Send(ToAddress: Text; Subject: Text; Body: Text)
var
Mail: Codeunit Mail;
begin
Mail.CreateMessage(ToAddress, '', '', Subject, Body, false, false);
end;
}

View file

@ -0,0 +1,11 @@
codeunit 50103 "Order Confirmation Notifier"
{
procedure Send(ToAddress: Text; Subject: Text; Body: Text)
var
Email: Codeunit Email;
EmailMessage: Codeunit "Email Message";
begin
EmailMessage.Create(ToAddress, Subject, Body, true);
Email.Send(EmailMessage, Enum::"Email Scenario"::Default);
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: breaking-changes
keywords: [email, codeunit-mail, email-message, email-scenario, email-account, smtp, sending-email]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Send email through the Email module, not Codeunit Mail (397)
> Contributions welcome — open a PR to refine or extend this article.
## Description
Older AL code sends email by calling `Codeunit Mail (397)`. Business Central's current extensibility model is a different, richer object set — `Codeunit Email`, `Codeunit "Email Message"`, `enum "Email Scenario"`, and the `Email Account`/`Email Connector` interface (Microsoft 365, Current User, SMTP, or a custom connector). `Codeunit "Email Message"` is the in-memory object you build the message on; it is not itself the persisted Sent/Outbox/Draft record — that storage is managed separately once the message is queued or sent. New code built on `Codeunit Mail` inherits its SMTP-era, single-connector assumptions and leaves no Sent/Outbox trail behind.
## Best Practice
Build on `Codeunit Email` and `Codeunit "Email Message"`. Route the message through an `Email Scenario` so different document types can use different accounts without the calling code needing to know which account that is, and get a tracked Sent/Outbox/Draft record for free.
See sample: [`prefer-email-module.good.al`](prefer-email-module.good.al).
## Anti Pattern
Calling `Codeunit Mail`'s `CreateMessage`. It still compiles and runs, but current `Codeunit Mail`'s own implementation of `CreateMessage` no longer sends anything by itself — it only raises integration events for a legacy subscriber to act on — so building new code on it means depending on whatever compatibility shim happens to still be wired up, with no first-class connector selection and no queryable Sent/Outbox/Draft record. `Send` and `GetErrorDesc` are not current members of `Codeunit Mail` at all; do not reference them.
See sample: [`prefer-email-module.bad.al`](prefer-email-module.bad.al).

View file

@ -0,0 +1,22 @@
codeunit 50101 "Meter Jnl.-Post"
{
procedure Post(var MeterJnlLine: Record "Meter Journal Line")
var
MeterLedgEntry: Record "Meter Ledger Entry";
begin
// Validation, Journal access, and posting all mixed in one routine.
if not Confirm('Post journal lines?') then
exit;
if MeterJnlLine.FindSet() then
repeat
if MeterJnlLine.Quantity = 0 then
Error('Quantity must not be zero.');
MeterLedgEntry.Init();
MeterLedgEntry.TransferFields(MeterJnlLine);
MeterLedgEntry.Insert();
MeterJnlLine.Delete();
until MeterJnlLine.Next() = 0;
end;
}

View file

@ -0,0 +1,42 @@
codeunit 50100 "Meter Jnl.-Check Line"
{
procedure CheckLine(var MeterJnlLine: Record "Meter Journal Line")
begin
// Reads setup/dimension data only, shows no UI beyond errors.
if MeterJnlLine.Quantity = 0 then
Error('Quantity must not be zero.');
end;
}
codeunit 50101 "Meter Jnl.-Post Line"
{
procedure PostLine(var MeterJnlLine: Record "Meter Journal Line")
var
MeterLedgEntry: Record "Meter Ledger Entry";
begin
// Posts exactly one journal line; never touches the Journal table.
MeterLedgEntry.Init();
MeterLedgEntry.TransferFields(MeterJnlLine);
MeterLedgEntry.Insert();
end;
}
codeunit 50102 "Meter Jnl.-Post Batch"
{
procedure PostBatch(var MeterJnlLine: Record "Meter Journal Line")
var
CheckLine: Codeunit "Meter Jnl.-Check Line";
PostLine: Codeunit "Meter Jnl.-Post Line";
begin
if MeterJnlLine.FindSet() then
repeat
CheckLine.CheckLine(MeterJnlLine);
until MeterJnlLine.Next() = 0;
if MeterJnlLine.FindSet() then
repeat
PostLine.PostLine(MeterJnlLine);
MeterJnlLine.Delete();
until MeterJnlLine.Next() = 0;
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: data-modeling
keywords: [posting-routine, check-line, post-line, post-batch, companion-codeunit, yes-no-wrapper, journal]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Split posting routines into Check Line / Post Line / Post Batch
> Contributions welcome — open a PR to refine or extend this article.
## Description
Business Central's own journal-based posting routines consistently follow a three-codeunit split, each with one primary responsibility — `Codeunit "Gen. Jnl.-Check Line"` / `"Gen. Jnl.-Post Line"` / `"Gen. Jnl.-Post Batch"` for the general journal, and the same `<Journal>-Check Line` / `<Journal>-Post Line` / `<Journal>-Post Batch` shape repeated for Item, Resource, Job, Fixed Asset, Insurance, and Cost Accounting journals: `Check Line` validates one line, `Post Line` posts exactly one journal line — `Gen. Jnl.-Post Line` itself can write more than one G/L Entry per call (a balancing entry, VAT, currency rounding, deferrals), so "exactly one line" describes its input, not a one-entry-out guarantee — and `Post Batch` loops both across the journal. A document posting routine (posting one document at a time) calls `Post Line` directly and skips `Post Batch`. This is the standard shape to evaluate a new journal-based posting routine against, not a platform-enforced constraint — a routine with a genuinely different transaction/reuse shape may legitimately organize itself differently, and the three codeunits' responsibilities are a useful default split, not a guarantee that every implementation keeps them non-overlapping. But a new routine that blurs this split without a specific reason either misses functionality other code expects to call directly, or exposes an interaction surface it shouldn't.
## Best Practice
`Check Line` reads setup/dimension data only on its first call and shows no UI beyond errors. `Post Line` only operates on the record passed to it — never the Journal table — so it can be called directly by other posting code, including a document posting routine. `Post Batch` is the only one of the three that reads and updates the Journal table, and it is the only one invoked from the Post action on a journal page. A `-Post` document codeunit is never called directly from a page; a page calls a `-Post (Yes/No)` confirmation wrapper instead, so the same `-Post` codeunit can also run unattended from a batch-posting report.
See sample: [`check-post-line-batch-pattern.good.al`](check-post-line-batch-pattern.good.al).
## Anti Pattern
A single monolithic posting codeunit that reads the Journal table, validates lines, writes ledger entries, and shows confirmation dialogs all in one procedure. It cannot be reused by another posting routine without fabricating journal records, and it cannot run unattended because it insists on user interaction.
See sample: [`check-post-line-batch-pattern.bad.al`](check-post-line-batch-pattern.bad.al).

View file

@ -0,0 +1,13 @@
table 50100 "Course"
{
fields
{
field(1; "No."; Code[20]) { }
field(10; "Global Dimension 1 Code"; Code[20])
{
// No CaptionClass, no OnValidate call into DimensionManagement.
// Accepts any value; never becomes a Default Dimension record.
TableRelation = "Dimension Value".Code;
}
}
}

View file

@ -0,0 +1,89 @@
// Master data: Default Dimension records, no Dimension Set ID field.
table 50100 "Course"
{
fields
{
field(1; "No."; Code[20]) { }
field(10; "Global Dimension 1 Code"; Code[20])
{
CaptionClass = '1,1,1';
TableRelation = "Dimension Value".Code where(
"Global Dimension No." = const(1), Blocked = const(false));
trigger OnValidate()
var
DimMgt: Codeunit DimensionManagement;
begin
DimMgt.ValidateDimValueCode(1, "Global Dimension 1 Code");
DimMgt.SaveDefaultDim(Database::Course, "No.", 1, "Global Dimension 1 Code");
end;
}
}
trigger OnDelete()
var
DimMgt: Codeunit DimensionManagement;
begin
DimMgt.DeleteDefaultDim(Database::Course, "No.");
end;
}
// Transactional/document data: a single Dimension Set ID, inherited from the
// related master record and overridable via shortcut dimension fields.
table 50101 "Course Registration Header"
{
fields
{
field(1; "No."; Code[20]) { }
field(2; "Customer No."; Code[20])
{
TableRelation = Customer;
trigger OnValidate()
begin
UpdateDimensionSetID();
end;
}
field(10; "Shortcut Dimension 1 Code"; Code[20])
{
CaptionClass = '1,1,1';
TableRelation = "Dimension Value".Code where(
"Global Dimension No." = const(1), Blocked = const(false));
trigger OnValidate()
var
DimMgt: Codeunit DimensionManagement;
begin
DimMgt.ValidateShortcutDimValues(1, "Shortcut Dimension 1 Code", "Dimension Set ID");
end;
}
field(480; "Dimension Set ID"; Integer)
{
Editable = false;
TableRelation = "Dimension Set Entry"."Dimension Set ID";
}
}
local procedure UpdateDimensionSetID()
var
Customer: Record Customer;
DimMgt: Codeunit DimensionManagement;
DefaultDimSource: List of [Dictionary of [Integer, Code[20]]];
GlobalDim2Code: Code[20];
begin
// Recompute from scratch (InheritFromDimSetID = 0) whether or not the
// customer lookup succeeds. Passing the existing "Dimension Set ID"
// here would inherit dimensions from whichever record the document
// was previously linked to, and exiting early on a failed Get would
// leave that same stale data in place — both defeat the point of
// this procedure. Clearing the shortcut field and recomputing with
// an empty source list (when the customer doesn't exist) correctly
// clears the document's dimensions instead of leaving old ones.
"Shortcut Dimension 1 Code" := '';
if Customer.Get("Customer No.") then
DimMgt.AddDimSource(DefaultDimSource, Database::Customer, "Customer No.");
"Dimension Set ID" :=
DimMgt.GetDefaultDimID(
DefaultDimSource, '', "Shortcut Dimension 1 Code", GlobalDim2Code, 0, 0);
end;
}

View file

@ -0,0 +1,35 @@
---
bc-version: [all]
domain: data-modeling
keywords: [dimensions, dimensionmanagement, global-dimension, shortcut-dimension, default-dimension, validatedimvaluecode, getdefaultdimid]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Wire dimension support through DimensionManagement, not ad hoc fields
> Contributions welcome — open a PR to refine or extend this article.
## Description
Adding dimension support to a custom table is not just a matter of adding a `Code[20]` field, and master tables and document/transactional tables wire into `Codeunit "Dimension Management"` through two different models — treating them as one mechanism is itself the mistake this article corrects:
- **Master data** (a custom master table, e.g. "Course") persists **Default Dimension** records: each shortcut dimension field validates through `ValidateDimValueCode`, then the result is saved via `SaveDefaultDim`, and `DeleteDefaultDim` removes them again in `OnDelete`. Both `ValidateDimValueCode` and `SaveDefaultDim` take the shortcut dimension *number* (1-8, matching `General Ledger Setup`'s "Shortcut Dimension N Code" fields) as their first/third argument respectively — not the AL field ID of the table field being validated. The master record itself carries no `Dimension Set ID` field.
- **Transactional/document data** (a custom document or journal-line table) carries a single **`Dimension Set ID`** field — a pointer to a shared, deduplicated set of dimension values in `Dimension Set Entry`, assembled from whatever the document inherited plus whatever the user overrode. A document does not acquire that ID by calling `SaveDefaultDim`; it builds a source list with `AddDimSource` (naming the related master table and its key, e.g. `Database::Customer`), then calls `GetDefaultDimID` to compute a new `Dimension Set ID` that inherits the master's Default Dimension records. Editing a shortcut dimension field directly on the document validates through `ValidateShortcutDimValues`, which updates the same `Dimension Set ID` in place rather than writing a separate Default Dimension record.
Skipping the model that actually matches the table's kind produces a field that looks correct in the designer but silently fails to save, validate, or carry through to postings — or, for a document, one that never picks up the customer's/vendor's own dimensions at all.
## Best Practice
For a master table, validate each shortcut dimension field through `ValidateDimValueCode`, save the result with `SaveDefaultDim`, and delete the matching Default Dimension records in `OnDelete`.
For a document table, when the field that attaches the document to a master record changes (e.g. `Customer No.`), call `AddDimSource` naming that master table and key, then `GetDefaultDimID` to compute the document's new `Dimension Set ID`, inheriting the master's Default Dimension records. Pass `0` for `GetDefaultDimID`'s `InheritFromDimSetID` argument in this case — passing the document's *existing* `Dimension Set ID` instead inherits whatever dimensions were already in it, so a value the previous linked record supplied can survive into the new one even where the new record has no default for that dimension. Run this same recompute — clear the shortcut field, call `GetDefaultDimID` with no source added — when the lookup on the new key fails (blank or an invalid value), too: exiting early instead leaves the previous record's dimensions in place, which is the same staleness bug the `InheritFromDimSetID = 0` rule exists to prevent. Validate the document's own Shortcut Dimension fields through `ValidateShortcutDimValues`, which updates that same `Dimension Set ID` in place rather than persisting a separate Default Dimension record.
See sample: [`dimension-management-wiring.good.al`](dimension-management-wiring.good.al).
## Anti Pattern
Adding a dimension-looking field with only a `TableRelation` to Dimension Value, and no call into `DimensionManagement` at all. The field accepts input but never becomes a real Default Dimension record, so it does not validate against blocked values and does not flow into postings.
See sample: [`dimension-management-wiring.bad.al`](dimension-management-wiring.bad.al).

View file

@ -0,0 +1,14 @@
table 50100 "Period Stats"
{
fields
{
field(1; "Period Start"; Date) { }
field(2; Flow; Enum "Some Flow") { }
}
keys
{
// Table already shipped with key(PK; "Period Start").
// Adding Flow here breaks every upgrade with AS0009.
key(PK; Flow, "Period Start") { Clustered = true; }
}
}

View file

@ -0,0 +1,24 @@
table 50100 "Period Stats"
{
fields
{
field(1; "Period Start"; Date) { }
}
keys
{
key(PK; "Period Start") { Clustered = true; }
}
}
table 50101 "Period Stats By Flow"
{
fields
{
field(1; "Period Start"; Date) { }
field(2; Flow; Enum "Some Flow") { }
}
keys
{
key(PK; Flow, "Period Start") { Clustered = true; }
}
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: data-modeling
keywords: [primary-key, clustered-key, table-design, appsource, breaking-change, schema-upgrade]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Never Change a Published Table's Primary or Clustered Key Field List
> Contributions welcome — open a PR to refine or extend this article.
## Description
Once a table has shipped — to AppSource, or to any customer environment that has already upgraded onto it — its primary key, and any other key marked `Clustered = true`, is frozen. This includes adding a field to the key, not only removing or reordering one: Business Central identifies existing rows by their key value, so any change to which fields compose that key invalidates every row already stored under the old shape, and the platform's upgrade validation rejects it outright (`AS0009`). This is easy to trip over because it doesn't look like the well-known "don't delete a field" mistake — the field being added is often brand new, and folding a new discriminating dimension straight into the existing key feels like the natural, un-denormalized way to model it. On an unpublished table that is correct; on a published one it is a breaking schema change regardless of which direction the field list changed, and there is no in-place fix once the upgrade is rejected, only reverting the key to its published shape.
## Best Practice
Leave a published table's key exactly as shipped. Model a new discriminating dimension as a separate table with its own key instead of adding a field to the existing key, and branch orchestration code by the new dimension rather than filtering one shared table on an extra key field.
See sample: [`do-not-change-primary-key.good.al`](do-not-change-primary-key.good.al).
## Anti Pattern
Adding a field to a published table's primary or clustered key to distinguish a new case. This fails AppSource validation or any customer upgrade with `AS0009` as soon as rows already exist under the old key shape, whether the field is being added, removed, or reordered.
See sample: [`do-not-change-primary-key.bad.al`](do-not-change-primary-key.bad.al).

View file

@ -0,0 +1,12 @@
codeunit 50130 "Sample Item Ledger Lookup"
{
procedure GetPostedItemLedgerEntries(var SalesHeader: Record "Sales Header"; var ItemLedgerEntry: Record "Item Ledger Entry")
var
LibrarySales: Codeunit "Library - Sales";
InvoiceNo: Code[20];
begin
InvoiceNo := LibrarySales.PostSalesDocument(SalesHeader, true, true);
ItemLedgerEntry.SetRange("Document No.", InvoiceNo);
if ItemLedgerEntry.FindSet() then;
end;
}

View file

@ -0,0 +1,13 @@
codeunit 50130 "Sample Item Ledger Lookup"
{
procedure GetPostedItemLedgerEntries(var SalesHeader: Record "Sales Header"; var ItemLedgerEntry: Record "Item Ledger Entry")
var
LibrarySales: Codeunit "Library - Sales";
ShippingNo: Code[20];
begin
LibrarySales.PostSalesDocument(SalesHeader, true, true);
ShippingNo := SalesHeader."Last Shipping No.";
ItemLedgerEntry.SetRange("Document No.", ShippingNo);
if ItemLedgerEntry.FindSet() then;
end;
}

View file

@ -0,0 +1,26 @@
---
bc-version: [all]
domain: data-modeling
keywords: [item-ledger-entry, document-no, last-shipping-no, ship-and-invoice, posting, sales-order]
technologies: [al]
countries: [w1]
application-area: [all]
---
# After Ship-and-Invoice posting, Item Ledger Entry carries the shipment document number
## Description
Posting a sales order with both Ship and Invoice in one call creates the Item Ledger Entry during the shipment leg of that combined post, so the entry's `Document No.` is stamped with the value assigned to the shipment — `Sales Header."Last Shipping No."` — not the posted sales invoice number the posting call returns. Code that filters Item Ledger Entry by the invoice number instead finds nothing: `SetRange`/`FindSet` simply return zero rows, with no error to signal the mistake.
## Best Practice
After posting a sales order with Ship and Invoice together, read `SalesHeader."Last Shipping No."` (populated during the post) and filter Item Ledger Entry by that value, not by the invoice number the posting routine returns.
See sample: [`item-ledger-entry-document-no-follows-last-shipping-no.good.al`](item-ledger-entry-document-no-follows-last-shipping-no.good.al).
## Anti Pattern
Filtering Item Ledger Entry by the posted sales invoice number after a combined Ship-and-Invoice post. The filter compiles and runs without error but matches zero rows, because the entry belongs to the shipment leg of the posting, not the invoice leg.
See sample: [`item-ledger-entry-document-no-follows-last-shipping-no.bad.al`](item-ledger-entry-document-no-follows-last-shipping-no.bad.al).

View file

@ -0,0 +1,12 @@
codeunit 50100 "Tax Posting Helper"
{
procedure GetTaxAccount(var Setup: Record "Sales & Receivables Setup"): Code[20]
var
DefaultTaxAccountTxt: Label 'DEFAULT-TAX';
begin
Setup.Get();
if Setup."Tax Account No." <> '' then
exit(Setup."Tax Account No.");
exit(DefaultTaxAccountTxt);
end;
}

View file

@ -0,0 +1,9 @@
codeunit 50100 "Tax Posting Helper"
{
procedure GetTaxAccount(var Setup: Record "Sales & Receivables Setup"): Code[20]
begin
Setup.Get();
Setup.TestField("Tax Account No.");
exit(Setup."Tax Account No.");
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: data-modeling
keywords: [testfield, setup-table, configuration, mandatory-field, silent-fallback, correctness]
technologies: [al]
countries: [w1]
application-area: [all]
---
# TestField a Setup-Table Value Before Using It in a Correctness-Critical Branch
> Contributions welcome — open a PR to refine or extend this article.
## Description
A procedure that reads a field from a setup or configuration table inside a branch where the surrounding logic has already decided that value is required needs to guard against it being blank. A common anti-pattern silently treats "blank" as "feature not wanted": it checks the field for emptiness and falls through to a default instead of raising an error. `Get()` succeeding on the setup record only proves the record exists, not that the specific field was ever configured, so the fallback path makes a missing configuration indistinguishable from a deliberate one — and the wrong outcome, especially in financial, tax, or compliance postings, surfaces silently rather than as a crash a tester would notice.
## Best Practice
Once a business rule has decided that a setup-table field's value is required for a branch to behave correctly, call `TestField` on it before use, even though a plain read would "work" by returning a blank or zero without erroring. Write a test that blanks the setup field and asserts the resulting error, so the guard itself is verified rather than merely present.
See sample: [`testfield-required-setup-field.good.al`](testfield-required-setup-field.good.al).
## Anti Pattern
Reading a required setup-table field behind a presence check that falls through to a default value instead of erroring. This looks defensive because it never crashes, but it converts "administrator forgot to configure this" into "system silently did something else" — worse than a hard failure, because nobody is told anything went wrong.
See sample: [`testfield-required-setup-field.bad.al`](testfield-required-setup-field.bad.al).

View file

@ -0,0 +1,16 @@
report 50105 "Sales Quote Confirmation"
{
UsageCategory = Documents;
ApplicationArea = All;
rendering
{
layout(RDLC)
{
Type = RDLC;
LayoutFile = './Layouts/SalesQuoteConfirmation.rdl';
}
}
DefaultRenderingLayout = RDLC;
}

View file

@ -0,0 +1,17 @@
report 50105 "Sales Quote Confirmation"
{
UsageCategory = Documents;
ApplicationArea = All;
rendering
{
layout(Word)
{
Type = Word;
LayoutFile = './Layouts/SalesQuoteConfirmation.docx';
Caption = 'Word Layout';
}
}
DefaultRenderingLayout = Word;
}

View file

@ -0,0 +1,34 @@
---
bc-version: [all]
domain: performance
keywords: [report-layout, word-layout, rdlc, document-report, sandbox-app-domain, rendering]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Document reports should default to a Word layout, not RDLC
> Contributions welcome — open a PR to refine or extend this article.
## Description
For a document report — an invoice, statement, order confirmation, or any report meant to be printed, emailed, or exported as a single-record document — Microsoft's own guidance recommends a `Word` rendering layout over `RDLC`: "RDL layouts can result in slower performance with document reports, regarding actions that are related to the user interface (for example, like sending emails) compared to Word layouts," and "we recommend that you design Word layouts instead of RDL" for this report shape (see Sources). This is a documented recommendation, not a universal guarantee that Word outperforms RDLC for every workload, and it does not apply to every report: tabular/list reports with heavy aggregation or calculated columns are still often a better fit for RDLC or Excel.
## Best Practice
For document reports, prefer a `Word` layout (`DefaultRenderingLayout = Word`) over RDLC unless specific layout requirements favor RDLC. Reserve RDLC (or Excel) for reports that represent a data listing rather than a document.
## Sources
- [Report Design Overview](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-report-design-overview)
- [Creating an RDL layout report](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-report-performance)
- [Troubleshooting reports / Report performance](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-reports-troubleshooting)
See sample: [`document-report-word-layout.good.al`](document-report-word-layout.good.al).
## Anti Pattern
Defaulting a document report's layout to RDLC out of habit or because a template happened to use it. This inherits RDLC's sandboxed-app-domain performance cost with no benefit tied to the report's actual content or calculation needs.
See sample: [`document-report-word-layout.bad.al`](document-report-word-layout.bad.al).

View file

@ -0,0 +1,26 @@
codeunit 50181 "Scanner Receipt Import Bad"
{
procedure PostScannedReceipt(ItemNo: Code[20]; LocationCode: Code[10]; UnitOfMeasureCode: Code[10]; ScannedQuantity: Decimal; DocumentNo: Code[20])
var
ItemJournalLine: Record "Item Journal Line";
ItemJnlPostLine: Codeunit "Item Jnl.-Post Line";
begin
if ScannedQuantity <= 0 then
Error(PositiveQuantityErr);
ItemJournalLine.Init();
ItemJournalLine.Validate("Posting Date", WorkDate());
ItemJournalLine.Validate("Entry Type", ItemJournalLine."Entry Type"::"Positive Adjmt.");
ItemJournalLine.Validate("Document No.", DocumentNo);
ItemJournalLine.Validate("Item No.", ItemNo);
ItemJournalLine.Validate("Location Code", LocationCode);
ItemJournalLine."Unit of Measure Code" := UnitOfMeasureCode;
ItemJournalLine.Quantity := ScannedQuantity;
ItemJournalLine."Quantity (Base)" := ScannedQuantity;
ItemJnlPostLine.RunWithCheck(ItemJournalLine);
end;
var
PositiveQuantityErr: Label 'The scanned quantity must be greater than zero.';
}

View file

@ -0,0 +1,25 @@
codeunit 50180 "Scanner Receipt Import Good"
{
procedure PostScannedReceipt(ItemNo: Code[20]; LocationCode: Code[10]; UnitOfMeasureCode: Code[10]; ScannedQuantity: Decimal; DocumentNo: Code[20])
var
ItemJournalLine: Record "Item Journal Line";
ItemJnlPostLine: Codeunit "Item Jnl.-Post Line";
begin
if ScannedQuantity <= 0 then
Error(PositiveQuantityErr);
ItemJournalLine.Init();
ItemJournalLine.Validate("Posting Date", WorkDate());
ItemJournalLine.Validate("Entry Type", ItemJournalLine."Entry Type"::"Positive Adjmt.");
ItemJournalLine.Validate("Document No.", DocumentNo);
ItemJournalLine.Validate("Item No.", ItemNo);
ItemJournalLine.Validate("Location Code", LocationCode);
ItemJournalLine.Validate("Unit of Measure Code", UnitOfMeasureCode);
ItemJournalLine.Validate(Quantity, ScannedQuantity);
ItemJnlPostLine.RunWithCheck(ItemJournalLine);
end;
var
PositiveQuantityErr: Label 'The scanned quantity must be greater than zero.';
}

View file

@ -0,0 +1,37 @@
---
bc-version: [all]
domain: scm
keywords: [quantity-base, qty-per-unit-of-measure, unit-of-measure-code, unit-of-measure-management, calcbaseqty, getqtyperunitofmeasure, qty-rounding-precision, item-journal-line]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Derive base quantities through the line's unit of measure
## Description
Inventory, item ledger entries, reservations, item tracking, and warehouse quantities are measured in the item's base unit of measure. Document and journal lines hold `Quantity` in the line's `"Unit of Measure Code"`, together with `"Qty. per Unit of Measure"` and base-unit fields such as `"Quantity (Base)"`. Ten boxes of twelve pieces are 120 base units, not 10. When a line's `Quantity` is validated, the table derives the base quantity through its `CalcBaseQty` procedure. That procedure calls `"Unit of Measure Management".CalcBaseQty` with the line's quantity rounding precision and raises an error when rounding would turn a non-zero quantity into a zero base quantity. Code that bypasses this conversion creates a line whose quantity and base quantity disagree, or makes a stock decision in the wrong unit.
## Best Practice
On a document or journal line, validate `"Unit of Measure Code"` before `Quantity`, and validate both. Validating the unit of measure sets `"Qty. per Unit of Measure"` from the item unit of measure; validating the quantity then fills the base fields with the correct rounding. Compare line quantities with inventory or availability in base units, for example `"Quantity (Base)"` or `"Outstanding Qty. (Base)"`.
Outside a line, get the factor with `"Unit of Measure Management".GetQtyPerUnitOfMeasure(Item, UnitOfMeasureCode)` and convert with its `CalcBaseQty` or `CalcQtyFromBase` procedures instead of multiplying by hand. Pass the item unit's quantity rounding precision where the available overload accepts it.
Reading these fields for display, reporting, or a temporary buffer that is never posted is not a conversion defect. Code that proves the line uses the base unit of measure (`"Qty. per Unit of Measure"` equal to 1) is also correct, but don't assume this from the item alone, because a line can use another unit.
See sample: [`derive-base-quantities-through-the-line-unit-of-measure.good.al`](derive-base-quantities-through-the-line-unit-of-measure.good.al).
## Anti Pattern
Assigning `Quantity` directly on an item journal, sales, purchase, or transfer line and then inserting, modifying, or posting it. Also assigning a base field such as `"Quantity (Base)" := Quantity`, multiplying by a hard-coded or separately looked-up factor without the line's rounding, or comparing a line's `Quantity` with `Item.Inventory` or another base-unit value. Detection signal: a direct `:=` to `Quantity`, `"Qty. per Unit of Measure"`, or a `(Base)` quantity field on a persisted or posted line, or a comparison between a non-base line quantity and an inventory quantity.
See sample: [`derive-base-quantities-through-the-line-unit-of-measure.bad.al`](derive-base-quantities-through-the-line-unit-of-measure.bad.al).
## References
- [Set up units of measure, including quantity rounding precision](https://learn.microsoft.com/en-us/dynamics365/business-central/inventory-how-setup-units-of-measure)
- [BCApps: Unit of Measure Management conversions](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/BaseApp/Foundation/UOM/UnitofMeasureManagement.Codeunit.al)
- [BCApps: Item Journal Line quantity validation and CalcBaseQty](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/BaseApp/Inventory/Journal/ItemJournalLine.Table.al)
- [BCApps: Sales Line quantity validation and CalcBaseQty](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/BaseApp/Sales/Document/SalesLine.Table.al)

View file

@ -0,0 +1,11 @@
codeunit 50161 "Cancel External Quotes Bad"
{
procedure CancelQuote(ExternalDocumentNo: Text)
var
SalesHeader: Record "Sales Header";
begin
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Quote);
SalesHeader.SetFilter("External Document No.", ExternalDocumentNo);
SalesHeader.DeleteAll(true);
end;
}

View file

@ -0,0 +1,25 @@
codeunit 50160 "Cancel External Quotes Good"
{
procedure CancelQuote(ExternalDocumentNo: Code[35])
var
SalesHeader: Record "Sales Header";
begin
if ExternalDocumentNo = '' then
Error(MissingExternalDocumentNoErr);
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Quote);
SalesHeader.SetRange("External Document No.", ExternalDocumentNo);
SalesHeader.DeleteAll(true);
end;
procedure CancelQuotes(ExternalDocumentNos: List of [Code[35]])
var
ExternalDocumentNo: Code[35];
begin
foreach ExternalDocumentNo in ExternalDocumentNos do
CancelQuote(ExternalDocumentNo);
end;
var
MissingExternalDocumentNoErr: Label 'The external document number is missing.';
}

View file

@ -0,0 +1,36 @@
---
bc-version: [all]
domain: security
keywords: [setfilter, setrange, filter-expression, filter-injection, external-input, wildcard, deleteall, modifyall]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Do not concatenate external text into a SetFilter expression
## Description
The `String` argument of `SetFilter` is a filter expression, not a value. Characters such as `..`, `|`, `&`, `<`, `>`, `=`, `*`, `?`, `@`, parentheses, and single quotes are operators. Text from a user, request page, API payload, file, or another system that is concatenated into that expression can therefore change which records match: `*` matches every value, `A|B` widens an exact lookup to two values, `10000..` becomes a range, and `J & V` becomes a conjunction or an invalid filter. `SetFilter` with an empty expression applies no filter at all. When the filtered record is then modified, deleted, exported, or used for a permission-relevant decision, the procedure acts on records the caller never identified.
## Best Practice
Treat an externally supplied identifier as a value. Use `SetRange(Field, Value)` for equality and `SetRange(Field, FromValue, ToValue)` for a typed range; neither parses operators. Reject an empty identifier explicitly when "no value" must not mean "every record". For several external values, apply `SetRange` once per value instead of joining them with `|`.
Use `SetFilter` when an operator is part of the procedure's own contract. Keep the operator in a constant expression and pass operands through replacement fields (`%1`, `%2`) of the field's data type, such as a `Date` or `Decimal` operand for `'>=%1'`. Do not rely on wrapping text in single quotes to neutralize it: according to the filter syntax, quotes protect `&`, `(`, `)`, `=`, and `|`, but `*` still acts as a wildcard inside `'J & V*'`.
Text that the user deliberately entered as a filter is supposed to be parsed. Do not report a request-page or `FilterPageBuilder` filter, a `GetFilters`/`GetView` round trip, a FlowFilter, or a field whose documented purpose is to hold a filter expression. Constant filter strings and operands produced by the extension's own code are also not external input.
See sample: [`do-not-concatenate-external-text-into-setfilter.good.al`](do-not-concatenate-external-text-into-setfilter.good.al).
## Anti Pattern
`Rec.SetFilter(Field, ExternalText)` or `Rec.SetFilter(Field, Prefix + ExternalText + Suffix)` where the text is meant to identify one record or a known list of records, with no validation that restricts it to a literal value. The finding is strongest when the resulting set is written by `Modify`, `ModifyAll`, `Delete`, or `DeleteAll`, or is returned to an external caller. Detection signal: a non-constant expression, concatenation, or `StrSubstNo` result passed as the `String` argument of `SetFilter`, where the operand comes from a parameter, page field, JSON/XML value, file line, or HTTP request.
See sample: [`do-not-concatenate-external-text-into-setfilter.bad.al`](do-not-concatenate-external-text-into-setfilter.bad.al).
## References
- [Record.SetFilter method, including the empty-filter remark](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-setfilter-method)
- [Filtering with SetRange and SetFilter](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-setcurrentkey-setrange-setfilter-getrangemin-and-getrangemax-methods)
- [Filter criteria, operators, and values that contain symbols](https://learn.microsoft.com/en-us/dynamics365/business-central/ui-enter-criteria-filters#filter-criteria-and-operators)

View file

@ -1,4 +1,4 @@
permissionset 50203 "Sec Sample Report Runner" permissionset 50203 "Sec Sample Report Runner"
{ {
Permissions = tabledata "G/L Entry" = ri; Permissions = tabledata "G/L Entry" = r;
} }

View file

@ -1,7 +1,7 @@
--- ---
bc-version: [all] bc-version: [all]
domain: security domain: security
keywords: [permissionset, indirect-permissions, ri, ii, mi, di, code-mediated] keywords: [permissionset, indirect-permissions, lowercase, code-mediated]
technologies: [al] technologies: [al]
countries: [w1] countries: [w1]
application-area: [all] application-area: [all]
@ -15,8 +15,12 @@ In a `permissionset`, uppercase letters (`R`, `I`, `M`, `D`) grant **direct** pe
## Best Practice ## Best Practice
Use indirect permissions (`ri`, `ii`, `mi`, `di`) when a role needs access to a sensitive table only through a specific codeunit or report — for example, a "Report Runner" role that reads `G/L Entry` only via published reports. Pair the indirect grant with the codeunit or report that mediates access; that object's own permissions (or InherentPermissions) supply the direct rights. Document why indirect permissions are required in the permission set or in the consuming object's comments. See sample: [`indirect-permissions-for-elevated-access.good.al`](indirect-permissions-for-elevated-access.good.al). Use indirect permissions (the lowercase letters `r`, `i`, `m`, `d`) when a role needs access to a sensitive table only through a specific codeunit or report — for example, a "Report Runner" role that reads `G/L Entry` only via published reports, granted `tabledata "G/L Entry" = r`. Each letter is one permission: `ri` grants indirect read **and** indirect insert, so grant only the letters the role needs. Pair the indirect grant with the codeunit or report that mediates access; that object's own permissions (or InherentPermissions) supply the direct rights. Document why indirect permissions are required in the permission set or in the consuming object's comments. See sample: [`indirect-permissions-for-elevated-access.good.al`](indirect-permissions-for-elevated-access.good.al).
## Anti Pattern ## Anti Pattern
Granting `RIMD` on a sensitive table when the role only needs to view it through a report — for example `tabledata "G/L Entry" = RIMD` on a "Report Runner" role. Users assigned that role can now query and modify ledger entries directly through any client that respects the permission, bypassing the report entirely. Reviewers should look for uppercase grants on system-of-record tables (G/L Entry, ledger entries, posted documents) where the consuming code path is clearly read-through-report or read-through-API. See sample: [`indirect-permissions-for-elevated-access.bad.al`](indirect-permissions-for-elevated-access.bad.al). Granting `RIMD` on a sensitive table when the role only needs to view it through a report — for example `tabledata "G/L Entry" = RIMD` on a "Report Runner" role. Users assigned that role can now query and modify ledger entries directly through any client that respects the permission, bypassing the report entirely. Reviewers should look for uppercase grants on system-of-record tables (G/L Entry, ledger entries, posted documents) where the consuming code path is clearly read-through-report or read-through-API. See sample: [`indirect-permissions-for-elevated-access.bad.al`](indirect-permissions-for-elevated-access.bad.al).
## References
[Permissions property](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/properties/devenv-permissions-property) and [Permissions on database objects](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-permissions-on-database-objects) define one letter per permission: `R`/`r` read, `I`/`i` insert, `M`/`m` modify, `D`/`d` delete, uppercase for direct and lowercase for indirect.

View file

@ -0,0 +1,24 @@
---
bc-version: [all]
domain: style
keywords: [build, output, alpackages, artifact-hygiene, outfolder, project-root]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Write AL Build Artifacts to an Intentional Output Location
> Contributions welcome — open a PR to refine or extend this article.
## Description
An AL project's compiled `.app` file can be written to the project root by default, and current tooling explicitly supports choosing a different destination instead — `ALTool`'s `--outfolder` option and the `al_build` agent tool's `outputPath` parameter both exist for this. The problem this rule addresses is not that root-level output is technically invalid; it is agents leaving generated `.app` files scattered through arbitrary source locations, or treating a compiled artefact as if it were part of the source tree (committing it, editing around it, referencing it as a dependency by hand).
## Best Practice
Write build artifacts to a deliberate, dedicated output location — configured via the build tool actually in use (e.g. `ALTool --outfolder`, or an explicit `outputPath` on the agent build tool) — and add that folder to `.gitignore`. Treat a compiled `.app` as a build artifact, never as a source file to commit or hand-edit around.
## Anti Pattern
Letting `.app` files accumulate in arbitrary or unversioned locations without a deliberate output path, or committing compiled artefacts into source control alongside the AL files that produced them.

View file

@ -0,0 +1,11 @@
codeunit 50100 "Sales Amount Calculator"
{
procedure BeregnTotalbeloeb(var Salgslinje: Record "Sales Line"): Decimal
var
Beloeb: Decimal;
begin
Salgslinje.CalcSums(Amount);
Beloeb := Salgslinje.Amount;
exit(Beloeb);
end;
}

View file

@ -0,0 +1,11 @@
codeunit 50100 "Sales Amount Calculator"
{
procedure CalculateTotalAmount(var SalesLine: Record "Sales Line"): Decimal
var
TotalAmount: Decimal;
begin
SalesLine.CalcSums(Amount);
TotalAmount := SalesLine.Amount;
exit(TotalAmount);
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [identifiers, naming, english, captions, translation]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Write AL Identifiers in English Only
> Contributions welcome — open a PR to refine or extend this article.
## Description
All AL identifiers — variables, procedures, parameters, fields, object names, enum values, and label identifiers — must be written in English, regardless of the developer's native language. Translations are handled separately through captions, tooltips, and XLIFF files, never by writing Danish, German, or other non-English identifiers directly into AL source code. This is not a stylistic preference: the public `microsoft/BCApps` codebase, maintained by engineers of many nationalities across hundreds of thousands of lines of AL, uses exclusively English identifiers, with all localization handled via caption properties and XLIFF rather than by changing identifier names. On any multi-contributor codebase, non-English identifiers make the code unreadable to contributors who don't share that language.
## Best Practice
Write every identifier in English, and translate developer intent rather than transliterating it — when a requirement is described in another language, the resulting variable, procedure, and field names should still read as English. Captions and tooltips may carry target-language text in the source file, with locale translations managed through XLIFF.
See sample: [`al-identifiers-english.good.al`](al-identifiers-english.good.al).
## Anti Pattern
Using native-language identifiers such as a Danish variable or procedure name in AL source code, relying on the fact that the code still compiles and runs correctly. This makes the code unreadable to non-native-language contributors and mixes localization concerns into source that should stay language-neutral.
See sample: [`al-identifiers-english.bad.al`](al-identifiers-english.bad.al).

View file

@ -0,0 +1,22 @@
table 50100 "Sales Task"
{
fields
{
field(1; "No."; Code[20]) { }
field(10; Status; Option)
{
OptionMembers = Active,Blocked;
}
}
}
codeunit 50100 "Sales Task Check"
{
procedure IsOverdue(DueDate: Date): Integer
begin
// 0 = No, 1 = Yes
if DueDate < Today then
exit(1);
exit(0);
end;
}

View file

@ -0,0 +1,16 @@
table 50100 "Sales Task"
{
fields
{
field(1; "No."; Code[20]) { }
field(10; Blocked; Boolean) { }
}
}
codeunit 50100 "Sales Task Check"
{
procedure IsOverdue(DueDate: Date): Boolean
begin
exit(DueDate < Today);
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [boolean, option, yes-no, magic-number, variable-typing, field-typing]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Binary yes/no choices must be typed as Boolean, not Option or Integer
> Contributions welcome — open a PR to refine or extend this article.
## Description
When a field or variable represents a genuine true/false state — yes/no, on/off, active/inactive, blocked/not blocked — it should be typed `Boolean`. Modeling that same predicate as an `Option`/`Enum` with two members, or as an `Integer` with two magic-number values, adds a layer of indirection a reader has to resolve before understanding the code. This is about semantics, not member count: a domain concept that currently has exactly two named alternatives — Inbound/Outbound, Debit/Credit, Buy/Sell — is not automatically a Boolean in disguise. An `Enum` can be the clearer model there, including when it needs to implement an interface, preserve an existing contract, or leave room for a future third value. The distinction is whether the domain is genuinely a stable predicate, not how many states it currently has.
## Best Practice
Type a field or variable as `Boolean` when the domain concept is inherently a true/false state. Do not replace a meaningful two-option domain model with a Boolean solely because it currently has two values.
See sample: [`binary-choice-must-be-boolean.good.al`](binary-choice-must-be-boolean.good.al).
## Anti Pattern
Modeling a yes/no choice as an `Option` with two members, or as an `Integer` with magic-number values, forces every caller to remember which value means what and leaves room for a meaningless third value.
See sample: [`binary-choice-must-be-boolean.bad.al`](binary-choice-must-be-boolean.bad.al).

View file

@ -0,0 +1,22 @@
table 50101 "Course"
{
fields
{
field(1; "No."; Code[20]) { }
// 1 = Beginner, 2..3 = Intermediate, 4+ = Advanced
field(10; Difficulty; Integer) { }
}
}
codeunit 50100 "Course Level Helper"
{
procedure LevelText(Difficulty: Integer): Text
begin
case Difficulty of
1:
exit('Beginner');
2, 3:
exit('Intermediate');
end;
end;
}

View file

@ -0,0 +1,17 @@
enum 50100 "Course Difficulty"
{
Extensible = true;
value(0; Beginner) { }
value(1; Intermediate) { }
value(2; Advanced) { }
}
table 50101 "Course"
{
fields
{
field(1; "No."; Code[20]) { }
field(10; Difficulty; Enum "Course Difficulty") { }
}
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [enum, option, integer, magic-number, variable-typing, field-typing]
technologies: [al]
countries: [w1]
application-area: [all]
---
# A fixed set of named choices must use Enum, not a raw Integer
> Contributions welcome — open a PR to refine or extend this article.
## Description
When a variable or field represents a fixed set of named, mutually exclusive states — a difficulty level, a document type, a processing status — it should be typed as `Enum` (or `Option` when extending an object that still uses the legacy type). Representing that same state as a plain `Integer` and tracking the meaning of each value in a comment or in a developer's head is a magic-number anti-pattern: the compiler cannot catch an out-of-range value, and branches read as opaque numbers instead of names. This is about semantics, not member count, matching `binary-choice-must-be-boolean.md`'s own distinction: a domain concept that is genuinely a stable true/false predicate belongs in `Boolean` even if someone represents it as a two-value `Enum`, while a domain concept with exactly two current named states is not automatically a Boolean in disguise — it stays an `Enum` when the states are named alternatives rather than a yes/no flag, or when it needs to implement an interface, preserve an existing contract, or leave room for a future third value.
## Best Practice
Declare an `Enum` with named values and branch on the enum value, not a raw number.
See sample: [`fixed-choice-set-must-use-enum-not-integer.good.al`](fixed-choice-set-must-use-enum-not-integer.good.al).
## Anti Pattern
Using a plain `Integer` field with the meaning of each value tracked only in a comment pushes the documentation of the states into something the compiler cannot check and a future maintainer cannot rely on.
See sample: [`fixed-choice-set-must-use-enum-not-integer.bad.al`](fixed-choice-set-must-use-enum-not-integer.bad.al).

View file

@ -0,0 +1,12 @@
codeunit 50100 "Sales Task Notify"
{
procedure NotifyUpdate(Count: Integer; CustomerNo: Code[20])
begin
MESSAGE('%1 records updated.', Count);
IF NOT CONFIRM('Delete %1?', FALSE, CustomerNo) THEN
EXIT;
ERROR(STRSUBSTNO('%1 must not be blank.', CustomerNo));
end;
}

View file

@ -0,0 +1,12 @@
codeunit 50100 "Sales Task Notify"
{
procedure NotifyUpdate(Count: Integer; CustomerNo: Code[20])
begin
Message('%1 records updated.', Count);
if not Confirm('Delete %1?', false, CustomerNo) then
exit;
Error(StrSubstNo('%1 must not be blank.', CustomerNo));
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [casing, intrinsic-function, built-in-function, message, error, confirm, strsubstno, legacy, c-al, pascalcase]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Intrinsic AL function calls use modern casing, not legacy ALL-CAPS
> Contributions welcome — open a PR to refine or extend this article.
## Description
AL is case-insensitive, so `MESSAGE(...)`, `ERROR(...)`, `CONFIRM(...)`, and `STRSUBSTNO(...)` compile and run identically to `Message(...)`, `Error(...)`, `Confirm(...)`, and `StrSubstNo(...)`. Modern AL — Microsoft's own current samples and current reference codebases — writes intrinsic/built-in function calls in the casing Microsoft assigns to the function's declared name, typically PascalCase. This is a codebase-convention claim, not a claim about what the VS Code formatter enforces: the formatter normalizes particular syntax but is not a mechanism for recasing every intrinsic function call, so do not cite formatter behavior as the reason to follow this convention. ALL-CAPS calls are a holdover from classic C/AL and signal code that has not been modernized, even though it compiles and runs correctly. Reserved keywords such as `if`, `begin`, and `for` are a separate, already-tooled concern; intrinsic function names are identifiers, not keywords, so that tooling does not catch ALL-CAPS intrinsic function calls.
## Best Practice
Call intrinsic functions in their modern, PascalCase form.
See sample: [`intrinsic-al-functions-must-use-modern-casing.good.al`](intrinsic-al-functions-must-use-modern-casing.good.al).
## Anti Pattern
ALL-CAPS intrinsic function calls trip no compiler error, but they are a reliable signal that a code block was copied from old C/AL material or outdated training content rather than written against current AL conventions.
See sample: [`intrinsic-al-functions-must-use-modern-casing.bad.al`](intrinsic-al-functions-must-use-modern-casing.bad.al).

View file

@ -0,0 +1,13 @@
namespace Contoso.Extensions;
using System.Performance; // guessed; never verified against the source file
codeunit 50100 "Tooling Extension"
{
procedure Run()
var
ToolingPage: Page "Some Tooling Page"; // guessed namespace; can still resolve against a stale or cached symbol package, then fail once checked against the object's current source or a freshly downloaded one
begin
ToolingPage.Run();
end;
}

View file

@ -0,0 +1,16 @@
// Verified by reading line 1 of the source file for "Some Tooling Page":
// namespace System.Tooling;
namespace Contoso.Extensions;
using System.Tooling;
codeunit 50100 "Tooling Extension"
{
procedure Run()
var
ToolingPage: Page "Some Tooling Page";
begin
ToolingPage.Run();
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [namespace, verification, source-of-truth, using-statement]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Resolve a namespace from the referenced object's source or symbols, never by guessing
> Contributions welcome — open a PR to refine or extend this article.
## Description
Since Business Central 2024 release wave 1, Microsoft's own objects are organized under a deep `Microsoft.*` namespace tree that has been renamed and restructured repeatedly. When adding a `using` directive for an existing AL object (table, codeunit, page, enum, interface, etc.), guessing its namespace from the object's name, from an older codebase, or from general familiarity produces a statement that can look plausible and still resolve to the wrong object, or fail to resolve at all, once checked against the object's actual current namespace. The reliable sources are the object's own source file (its `namespace` declaration) or, for a dependency without accessible source, its AL symbol package — not the object's name or a remembered convention.
## Best Practice
When referencing an existing AL object, resolve its namespace from that object's actual source file or symbol definition — never infer or invent one from its name, functional area, or naming convention.
See sample: [`namespace-must-be-verified-from-source.good.al`](namespace-must-be-verified-from-source.good.al).
## Anti Pattern
Writing a `using` statement from memory, from an incomplete path, or from a plausible-looking guess. It can appear correct while actually resolving to the wrong object, or fail to resolve, once checked against stale or mismatched symbols, a different build configuration, or the object's actual current source — not because the compiler and the AL Language Server apply different namespace-resolution rules; they don't.
See sample: [`namespace-must-be-verified-from-source.bad.al`](namespace-must-be-verified-from-source.bad.al).

View file

@ -0,0 +1,15 @@
codeunit 50100 "Customer Lookup"
{
procedure GetCustomerName(CustomerNo: Code[20]; var Name: Text[100])
var
Customer: Record Customer;
begin
if Customer.Get(CustomerNo) then
Name := Customer.Name;
end;
procedure Sample()
begin
GetCustomerName('10000', 'placeholder'); // compile error: literal is not addressable
end;
}

View file

@ -0,0 +1,17 @@
codeunit 50100 "Customer Lookup"
{
procedure GetCustomerName(CustomerNo: Code[20]; var Name: Text[100])
var
Customer: Record Customer;
begin
if Customer.Get(CustomerNo) then
Name := Customer.Name;
end;
procedure Sample()
var
CustName: Text[100];
begin
GetCustomerName('10000', CustName);
end;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: style
keywords: [var-parameter, by-reference, pass-by-reference, literal, constant, compile-error, procedure-call]
technologies: [al]
countries: [w1]
application-area: [all]
---
# A `var` parameter must be called with a variable, never a literal or expression
> Contributions welcome — open a PR to refine or extend this article.
## Description
When a procedure declares a parameter with `var`, that parameter is passed by reference: the callee writes back into the caller's own memory location. This means the argument at the call site must be an actual variable, something with an address. A string literal, a numeric constant, or a computed expression has no address to write back to, so passing one to a `var` parameter fails to compile. Before writing a call, check the callee's signature for `var` on each parameter position being supplied a literal or expression — if present, a variable declared in the caller's own scope must be used instead.
## Best Practice
Declare a variable in the caller's scope and pass it to the `var` parameter.
See sample: [`var-parameters-require-an-addressable-variable.good.al`](var-parameters-require-an-addressable-variable.good.al).
## Anti Pattern
Passing a literal or a computed expression to a `var` parameter position fails to compile, because neither has an address the callee can write back to.
See sample: [`var-parameters-require-an-addressable-variable.bad.al`](var-parameters-require-an-addressable-variable.bad.al).

View file

@ -0,0 +1,57 @@
codeunit 50411 "Test Sales Setup Initialize Bad"
{
Subtype = Test;
[Test]
[HandlerFunctions('CustomerCardHandler')]
procedure CustomerCardOpensForSelectedCustomer()
var
Customer: Record Customer;
begin
Initialize();
LibrarySales.CreateCustomer(Customer);
LibraryVariableStorage.Enqueue(Customer."No.");
Page.RunModal(Page::"Customer Card", Customer);
LibraryVariableStorage.AssertEmpty();
end;
[Test]
procedure StockoutWarningCanBeDisabled()
var
SalesSetup: Record "Sales & Receivables Setup";
begin
Initialize();
LibrarySales.SetStockoutWarning(false);
SalesSetup.Get();
Assert.IsFalse(SalesSetup."Stockout Warning", 'The stockout warning was not disabled.');
end;
local procedure Initialize()
begin
if IsInitialized then
exit;
LibraryVariableStorage.Clear();
LibrarySetupStorage.Restore();
LibrarySales.SetStockoutWarning(true);
IsInitialized := true;
LibrarySetupStorage.SaveSalesSetup();
end;
[ModalPageHandler]
procedure CustomerCardHandler(var CustomerCard: TestPage "Customer Card")
begin
Assert.AreEqual(LibraryVariableStorage.DequeueText(), CustomerCard."No.".Value(), 'The customer card opened for the wrong customer.');
end;
var
Assert: Codeunit Assert;
LibrarySales: Codeunit "Library - Sales";
LibrarySetupStorage: Codeunit "Library - Setup Storage";
LibraryVariableStorage: Codeunit "Library - Variable Storage";
IsInitialized: Boolean;
}

View file

@ -0,0 +1,62 @@
codeunit 50410 "Test Sales Setup Initialize Good"
{
Subtype = Test;
[Test]
[HandlerFunctions('CustomerCardHandler')]
procedure CustomerCardOpensForSelectedCustomer()
var
Customer: Record Customer;
begin
Initialize();
LibrarySales.CreateCustomer(Customer);
LibraryVariableStorage.Enqueue(Customer."No.");
Page.RunModal(Page::"Customer Card", Customer);
LibraryVariableStorage.AssertEmpty();
end;
[Test]
procedure StockoutWarningCanBeDisabled()
var
SalesSetup: Record "Sales & Receivables Setup";
begin
Initialize();
LibrarySales.SetStockoutWarning(false);
SalesSetup.Get();
Assert.IsFalse(SalesSetup."Stockout Warning", 'The stockout warning was not disabled.');
end;
local procedure Initialize()
begin
LibraryTestInitialize.OnTestInitialize(Codeunit::"Test Sales Setup Initialize Good");
LibraryVariableStorage.Clear();
LibrarySetupStorage.Restore();
if IsInitialized then
exit;
LibraryTestInitialize.OnBeforeTestSuiteInitialize(Codeunit::"Test Sales Setup Initialize Good");
LibrarySales.SetStockoutWarning(true);
IsInitialized := true;
LibrarySetupStorage.SaveSalesSetup();
LibraryTestInitialize.OnAfterTestSuiteInitialize(Codeunit::"Test Sales Setup Initialize Good");
end;
[ModalPageHandler]
procedure CustomerCardHandler(var CustomerCard: TestPage "Customer Card")
begin
Assert.AreEqual(LibraryVariableStorage.DequeueText(), CustomerCard."No.".Value(), 'The customer card opened for the wrong customer.');
end;
var
Assert: Codeunit Assert;
LibrarySales: Codeunit "Library - Sales";
LibrarySetupStorage: Codeunit "Library - Setup Storage";
LibraryTestInitialize: Codeunit "Library - Test Initialize";
LibraryVariableStorage: Codeunit "Library - Variable Storage";
IsInitialized: Boolean;
}

View file

@ -0,0 +1,41 @@
---
bc-version: [all]
domain: testing
keywords: [initialize, isinitialized, library-test-initialize, ontestinitialize, library-variable-storage, library-setup-storage, test-fixture, test-codeunit]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Reset per-test state before the IsInitialized guard
## Description
Standard Business Central test codeunits call a local `Initialize` procedure at the start of every test method. Global variables in a test codeunit keep their values between the codeunit's test methods, so a Boolean such as `IsInitialized` lets `Initialize` run expensive shared setup only once. The procedure therefore has two parts with different lifetimes: work that must run before **every** test, and one-time setup behind the guard. If per-test reset is placed after the guard, it runs only for the first test. Values left in `Library - Variable Storage` by a failed test, or setup records a test changed, then leak into later tests, which pass or fail depending on execution order.
## Best Practice
Call `Initialize()` as the first statement of every test method. Inside it, keep this order, which the Base Application tests follow:
1. Per-test work, before the guard: raise `"Library - Test Initialize".OnTestInitialize`, call `LibraryVariableStorage.Clear()`, and call `LibrarySetupStorage.Restore()` when setup tables were saved.
2. `if IsInitialized then exit;`
3. One-time work: raise `OnBeforeTestSuiteInitialize`, create the shared fixture and setup values, set `IsInitialized := true`, save the setup tables that tests may change (for example `LibrarySetupStorage.SaveSalesSetup()`), and raise `OnAfterTestSuiteInitialize`.
Create data that a single test changes inside that test, not in the shared fixture. Base Application suites also commit after the one-time setup so the shared fixture survives each test's transaction; whether that commit is valid depends on the test transaction model and runner isolation, see [`transactionmodel-attribute-governs-test-transactions.md`](transactionmodel-attribute-governs-test-transactions.md) and [`testisolation-belongs-on-the-test-runner.md`](testisolation-belongs-on-the-test-runner.md).
A test codeunit with no shared setup and no queued values doesn't need an `Initialize` procedure. Don't report its absence on its own.
See sample: [`reset-per-test-state-before-the-isinitialized-guard.good.al`](reset-per-test-state-before-the-isinitialized-guard.good.al).
## Anti Pattern
`if IsInitialized then exit;` as the first statement of `Initialize`, followed by `LibraryVariableStorage.Clear()`, `LibrarySetupStorage.Restore()`, or other reset calls that are then skipped for every test after the first. A related defect is a test method in a codeunit that uses the pattern but doesn't call `Initialize()`, so it runs with whatever state the previous test left. Detection signal: in a `Subtype = Test` codeunit, a reset call placed after the `IsInitialized` exit, or a `[Test]` procedure that uses shared globals or queued values without first calling `Initialize()`.
See sample: [`reset-per-test-state-before-the-isinitialized-guard.bad.al`](reset-per-test-state-before-the-isinitialized-guard.bad.al).
## References
- [BCApps: `Initialize` in the ERM Sales Document tests](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/Tests/ERM-Sales/ERMSalesDocument.Codeunit.al)
- [BCApps: Library - Test Initialize events](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/Tests/ApplicationTestLibrary/LibraryTestInitialize.Codeunit.al)
- [BCApps: Library - Setup Storage](https://github.com/microsoft/BCApps/blob/4abbb8ff848cdcb4e1187fc7a3e2da0612dd0d2b/src/Layers/W1/Tests/ApplicationTestLibrary/LibrarySetupStorage.Codeunit.al)
- [Testing the application](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-testing-application)

View file

@ -0,0 +1,14 @@
[Test]
procedure PostsSalesOrderForCashCustomer()
var
Customer: Record Customer;
SalesHeader: Record "Sales Header";
begin
// Assumes a 'CASH' customer already exists in the environment —
// fails on any database where it doesn't.
Customer.Get('CASH');
LibrarySales.CreateSalesHeader(
SalesHeader, SalesHeader."Document Type"::Order, Customer."No.");
// ... add lines, post, assert ...
end;

View file

@ -0,0 +1,13 @@
[Test]
procedure PostsSalesOrderForRandomCustomer()
var
Customer: Record Customer;
SalesHeader: Record "Sales Header";
begin
// Freshly created customer, owned by this test — no assumption about what exists.
LibrarySales.CreateCustomer(Customer);
LibrarySales.CreateSalesHeader(
SalesHeader, SalesHeader."Document Type"::Order, Customer."No.");
// ... add lines, post, assert ...
end;

View file

@ -0,0 +1,30 @@
---
bc-version: [all]
domain: testing
keywords: [testing, test-data, random, library, any]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Generate Test Data Programmatically, Never Assume Existing Records
> Contributions welcome — open a PR to refine or extend this article.
## Description
A BC test company normally contains initialized system/setup data — an AL test suite should not assume an empty database, but it must be independent of unrelated business records: create the records and setup it owns rather than looking up a specific code, number, or name assumed to already exist, since that makes the test fail for reasons unrelated to the code under test. Every mandatory field on a created record also needs an actual value — leaving one blank because setup-time validation happens to allow it produces a record that doesn't reflect a real one and can fail later, elsewhere in the flow (posting, a report, a later assertion), for a reason unrelated to what the test claims to check. A short-but-valid value is not itself a defect: AL field lengths are maxima, not minimums, so a two-character value in a `Text[100]` field is fine unless the scenario specifically depends on the field's length or shape — for example, a test that verifies truncation or a format check needs a value chosen to exercise that boundary, not an arbitrary short one.
Not every value should be generated, though. Incidental fixture data — identifiers, names, descriptions — should generally come from the standard library codeunits rather than be tied to specific existing data. But values that materially define the scenario under test — amounts, quantities, percentages, dates, thresholds, rounding precision — should stay explicit and deliberately chosen, not randomized: a rounding test needs values placed deliberately around the rounding boundary, not a random one that might miss it entirely.
## Best Practice
Use the standard library codeunits (`Library - ERM`, `Library - Inventory`, `Library - Sales`, `Library - Utility`) to generate incidental fixture values — they produce valid, unique-enough data via number series and controlled randomness, not a mathematical collision-free guarantee — and fill every mandatory field with correctly-sized data. Keep values that define the scenario's expected outcome explicit and fixed. Reserve hardcoded values for tests that validate an external contract itself — a fixed JSON schema, an EDIFACT message, a counterparty code — where the hardcoded value documents the specification rather than arbitrary test logic.
See sample: [`test-data-must-be-random-and-complete.good.al`](test-data-must-be-random-and-complete.good.al).
## Anti Pattern
Looking up a record assumed to already exist (a hardcoded payment method or customer number) instead of creating it, or leaving a mandatory field empty because setup-time validation happens to allow it. Also an anti-pattern, narrower: using a value that doesn't satisfy a scenario's explicit length or format requirement — for example a truncation test that never actually exceeds the field it's meant to overflow.
See sample: [`test-data-must-be-random-and-complete.bad.al`](test-data-must-be-random-and-complete.bad.al).

View file

@ -0,0 +1,10 @@
codeunit 50132 "Sample Customer Type Library"
{
procedure CreateCustomerType(var CustomerType: Record "Customer Type")
begin
CustomerType.Init();
CustomerType.Code := 'TEST001';
CustomerType.Description := 'Test Customer Type';
CustomerType.Insert(true);
end;
}

View file

@ -0,0 +1,21 @@
codeunit 50132 "Sample Customer Type Library"
{
var
LibraryUtility: Codeunit "Library - Utility";
procedure CreateCustomerType(var CustomerType: Record "Customer Type")
begin
CustomerType.Init();
// Code is shorter than GenerateGUID()'s 10 characters, and this field's
// uniqueness matters, so use GenerateRandomCodeWithLength: it opens the
// real (non-temporary) table and loops until the value doesn't collide.
// GenerateRandomCode would not do this — it opens the table as temporary,
// so its own emptiness check never inspects real rows.
CustomerType.Code :=
LibraryUtility.GenerateRandomCodeWithLength(CustomerType.FieldNo(Code), Database::"Customer Type", MaxStrLen(CustomerType.Code));
// Description is long enough to hold the full GenerateGUID() value
// untruncated, and only needs to be incidental, not verified-unique.
CustomerType.Description := CopyStr(LibraryUtility.GenerateGUID(), 1, MaxStrLen(CustomerType.Description));
CustomerType.Insert(true);
end;
}

View file

@ -0,0 +1,33 @@
---
bc-version: [all]
domain: testing
keywords: [generateguid, library-utility, test-fixtures, uniqueness, generaterandomcode, maxstrlen]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Generate unique test fixture values with LibraryUtility helpers, not hardcoded literals
## Description
A fixture helper that assigns a hardcoded literal to a primary-key field, or to any field the test relies on as a unique lookup identifier, collides the moment two tests, or two runs of the same test, create that fixture without cleanup — and a literal longer than the field allows raises a truncation or insert error. An ordinary descriptive field carries no such constraint: two rows with the same description do not collide on insert, and a deterministic descriptive value is often exactly what an exact-match assertion needs, so none of this applies to it. `LibraryUtility.GenerateGUID()` is not a real GUID — it is a `Code[10]` number-series value (`GU00000000`–`GU99999999`) — and it returns the full 10 characters unshortened. Truncating it yourself with `CopyStr(..., 1, MaxStrLen(ShorterField))` for a field under 10 characters is unsafe: the changing digits sit at the right end and are exactly what gets cut off, so consecutive calls into a short field can produce the same truncated value. `GenerateGUID()` is only safe as-is for a field that holds the full 10 characters.
## Best Practice
For a field that holds the full 10 characters, assign `LibraryUtility.GenerateGUID()` directly. For a shorter field, do not truncate a GUID yourself — but also do not assume every `LibraryUtility` helper verifies uniqueness against the real table, because they don't all behave the same way:
- `GenerateRandomCode(FieldNo, TableNo)` opens the target table as a **temporary** `RecordRef`: the buffer starts and stays empty, so its `repeat...until RecRef.IsEmpty()` loop always exits after one iteration — despite taking `TableNo`, it never checks the real table, and it never retries even within its own call. Its value is the rightmost `FieldRef.Length` characters of `GenerateGUID()`'s sequential `GU00000000`–`GU99999999` series, so for a short field that window of digits cycles: a 1-character field repeats every 10 calls, a 2-character field every 100, and so on. It is a finite short-field namespace with a low collision *chance* within one test run — not a guarantee at any scope, unlike the table-checking helpers below.
- `GenerateRandomCodeWithLength(FieldNo, TableNo, CodeLength)` opens the real (non-temporary) table and loops until the generated value doesn't collide — a genuine verified-unique guarantee — but it returns `Code[10]` regardless of the requested `CodeLength`, so it's only useful for a field of 10 characters or fewer.
- `GenerateRandomCode20(FieldNo, TableNo)` is the same real, verified-against-the-table pattern as `GenerateRandomCodeWithLength`, sized for a `Code[20]` field.
- `GenerateRandomXMLText(Length)` performs no table lookup at all — it's a plain random-text generator, appropriate for a descriptive/incidental field where uniqueness doesn't matter, not for a value that needs to be collision-checked.
Pick `GenerateRandomCodeWithLength`/`GenerateRandomCode20` when the test genuinely needs a code verified unique against the table; use `GenerateRandomCode`/`GenerateGUID`/`GenerateRandomXMLText` for incidental values where a low collision *chance* is enough.
See sample: [`use-generateguid-for-unique-test-fixture-values.good.al`](use-generateguid-for-unique-test-fixture-values.good.al).
## Anti Pattern
Hardcoding a primary-key or unique-lookup fixture value such as `'TEST001'`, which collides across parallel or repeated test runs — a fixed descriptive value is not this anti-pattern, since the field carries no uniqueness constraint. Equally an anti-pattern: truncating `GenerateGUID()`'s result with `CopyStr(..., 1, MaxStrLen(Field))` for a field shorter than 10 characters — the truncation removes the part of the value that actually varies.
See sample: [`use-generateguid-for-unique-test-fixture-values.bad.al`](use-generateguid-for-unique-test-fixture-values.bad.al).

View file

@ -0,0 +1,27 @@
codeunit 50134 "Sample Customer Type Edit Test"
{
Subtype = Test;
[Test]
procedure CustomerTypeFieldNotEditable_WhenLocked()
var
Assert: Codeunit Assert;
CustomerType: Record "Customer Type";
CustomerTypeCard: TestPage "Customer Type Card";
begin
// [GIVEN] a customer type record whose Locked flag is set
CustomerType.Init();
CustomerType.Locked := true;
CustomerType.Insert(true);
// [WHEN] the page is opened in VIEW mode — editability logic that only
// applies in edit mode is not exercised the same way
CustomerTypeCard.OpenView();
CustomerTypeCard.GoToRecord(CustomerType);
// [THEN] wrong function: Enabled() does not verify editability
Assert.IsFalse(CustomerTypeCard.Description.Enabled(), 'Description should not be editable while Locked is set.');
CustomerTypeCard.Close();
end;
}

View file

@ -0,0 +1,26 @@
codeunit 50133 "Sample Customer Type Edit Test"
{
Subtype = Test;
[Test]
procedure CustomerTypeFieldNotEditable_WhenLocked()
var
Assert: Codeunit Assert;
CustomerType: Record "Customer Type";
CustomerTypeCard: TestPage "Customer Type Card";
begin
// [GIVEN] a customer type record whose Locked flag is set
CustomerType.Init();
CustomerType.Locked := true;
CustomerType.Insert(true);
// [WHEN] the page is opened in edit mode on that record
CustomerTypeCard.OpenEdit();
CustomerTypeCard.GoToRecord(CustomerType);
// [THEN] the field's actual editable state reflects the lock
Assert.IsFalse(CustomerTypeCard.Description.Editable(), 'Description should not be editable while Locked is set.');
CustomerTypeCard.Close();
end;
}

View file

@ -0,0 +1,26 @@
---
bc-version: [all]
domain: testing
keywords: [testpage, editable, openedit, ui-state, field-verification]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Verify field editability with TestPage.Editable(), opened in edit mode
## Description
Whether a field can actually be changed is a distinct state from whether it is shown or enabled — `Editable()` and `Enabled()` are separate `TestField` functions. Verifying editability also requires opening the `TestPage` with `OpenEdit()`, not `OpenView()`: `OpenView()` opens the page in view mode, so it does not exercise the field's own conditional editability logic the way an actual edit-mode session does.
## Best Practice
Open the `TestPage` with `OpenEdit()`, navigate to the relevant record, then assert against `TestPageField.Editable()` to verify whether the field can be changed under the given precondition.
See sample: [`use-testpage-editable-to-verify-field-editability.good.al`](use-testpage-editable-to-verify-field-editability.good.al).
## Anti Pattern
Asserting `Enabled()` (or checking nothing at all) when the actual claim is about editability, or opening the page with `OpenView()` when the field's editability depends on business logic that only applies in edit mode.
See sample: [`use-testpage-editable-to-verify-field-editability.bad.al`](use-testpage-editable-to-verify-field-editability.bad.al).

View file

@ -0,0 +1,14 @@
codeunit 50131 "Sample Customer Type UI Test"
{
Subtype = Test;
[Test]
procedure CustomerTypeFieldIsEnabledOnCustomerCard()
var
CustomerCard: TestPage "Customer Card";
begin
// Confirms only that the page opens - never checks the field's actual UI state
CustomerCard.OpenView();
CustomerCard.Close();
end;
}

View file

@ -0,0 +1,15 @@
codeunit 50131 "Sample Customer Type UI Test"
{
Subtype = Test;
[Test]
procedure CustomerTypeFieldIsEnabledOnCustomerCard()
var
Assert: Codeunit Assert;
CustomerCard: TestPage "Customer Card";
begin
CustomerCard.OpenView();
Assert.IsTrue(CustomerCard."Customer Type".Enabled(), 'Customer Type should be enabled on the Customer Card.');
Assert.IsTrue(CustomerCard."Customer Type".Visible(), 'Customer Type should be visible on the Customer Card.');
end;
}

View file

@ -0,0 +1,26 @@
---
bc-version: [all]
domain: testing
keywords: [testpage, visible, enabled, ui-state, headless-test, field-verification]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Verify field visibility and enabled state with TestPage.Visible()/.Enabled()
## Description
A UI test codeunit does not need to inspect table or page properties indirectly to confirm a field is shown or enabled under given conditions. The `TestPage` object exposes a `Visible()` and an `Enabled()` function on each field, reflecting the page's actual rendered state, callable directly from a `[Test]` procedure. `Enabled()` and `Editable()` are distinct states — this article covers visibility/enabled state specifically; see `use-testpage-editable-to-verify-field-editability.md` for verifying whether a field can actually be changed.
## Best Practice
Open the `TestPage`, navigate to the relevant record if needed, then assert against `TestPageField.Visible()` and `TestPageField.Enabled()` to verify the field's shown/enabled state, rather than checking an unrelated table/page property or skipping the check.
See sample: [`use-testpage-visible-enabled-to-verify-field-ui-state.good.al`](use-testpage-visible-enabled-to-verify-field-ui-state.good.al).
## Anti Pattern
A test that opens the `TestPage` but never asserts against `Visible()`/`Enabled()` on the field in question — confirming only that the page opens, not that the field behaves as expected.
See sample: [`use-testpage-visible-enabled-to-verify-field-ui-state.bad.al`](use-testpage-visible-enabled-to-verify-field-ui-state.bad.al).

View file

@ -0,0 +1,27 @@
page 50101 "Customer Info API"
{
PageType = API;
APIPublisher = 'contoso';
APIGroup = 'sales';
APIVersion = 'v1.0';
EntityName = 'customerInfo';
EntitySetName = 'customerInfos';
SourceTable = Customer;
ODataKeyFields = "No.";
InsertAllowed = true;
layout
{
area(content)
{
repeater(General)
{
field(customerNo; Rec."No.")
{
Editable = false; // consumer must supply "No." on POST — this rejects it
}
field(name; Rec.Name) { }
}
}
}
}

View file

@ -0,0 +1,25 @@
page 50101 "Customer Info API"
{
PageType = API;
APIPublisher = 'contoso';
APIGroup = 'sales';
APIVersion = 'v1.0';
EntityName = 'customerInfo';
EntitySetName = 'customerInfos';
SourceTable = Customer;
ODataKeyFields = "No.";
InsertAllowed = true;
DelayedInsert = true;
layout
{
area(content)
{
repeater(General)
{
field(customerNo; Rec."No.") { }
field(name; Rec.Name) { }
}
}
}
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: web-services
keywords: [api-page, key-fields, editable, insert, odata]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Keep Consumer-Provided Key Fields Editable on API Pages
> Contributions welcome — open a PR to refine or extend this article.
## Description
A field listed in `ODataKeyFields` cannot have `Editable = false` when the API page allows inserts and the field's value must be supplied by the caller. Marking it read-only removes the field from the OData write schema, so a POST that includes it is rejected as an unknown property. This only applies to keys the consumer must supply — a system-generated key such as `SystemId` is a valid exception, since Business Central assigns its value automatically on insert.
## Best Practice
Leave every consumer-supplied key field referenced in `ODataKeyFields` without `Editable = false` on pages where `InsertAllowed = true`, so the OData layer accepts it as a writable property on POST.
See sample: [`api-page-key-fields-must-be-editable-on-insert.good.al`](api-page-key-fields-must-be-editable-on-insert.good.al).
## Anti Pattern
Marking a consumer-provided key field `Editable = false`, out of habit or for perceived safety. This silently breaks create operations with a generic `BadRequest` instead of a clear validation error.
See sample: [`api-page-key-fields-must-be-editable-on-insert.bad.al`](api-page-key-fields-must-be-editable-on-insert.bad.al).

View file

@ -0,0 +1,21 @@
codeunit 50100 "HTTP Status Handling Bad"
{
procedure GetCustomer(CustomerId: Guid): JsonObject
var
Client: HttpClient;
Response: HttpResponseMessage;
CustomerJson: JsonObject;
ResponseText: Text;
begin
if not Client.Get(
StrSubstNo('https://api.example.com/customers/%1', CustomerId),
Response)
then
Error('The customer service could not be reached.');
// A completed request can still contain a 4xx or 5xx error document.
Response.Content().ReadAs(ResponseText);
CustomerJson.ReadFrom(ResponseText);
exit(CustomerJson);
end;
}

View file

@ -0,0 +1,23 @@
codeunit 50100 "HTTP Status Handling Good"
{
procedure GetCustomer(CustomerId: Guid): JsonObject
var
Client: HttpClient;
Response: HttpResponseMessage;
CustomerJson: JsonObject;
ResponseText: Text;
begin
if not Client.Get(
StrSubstNo('https://api.example.com/customers/%1', CustomerId),
Response)
then
Error('The customer service could not be reached.');
if not Response.IsSuccessStatusCode() then
Error('The customer service returned HTTP status %1.', Response.HttpStatusCode());
Response.Content().ReadAs(ResponseText);
CustomerJson.ReadFrom(ResponseText);
exit(CustomerJson);
end;
}

View file

@ -0,0 +1,31 @@
---
bc-version: [all]
domain: web-services
keywords: [httpclient, httpresponsemessage, issuccessstatuscode, httpstatuscode, response-body, json]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Check HTTP status before consuming the response body
## Description
A successful AL `HttpClient` call only confirms that the platform completed the HTTP exchange. The server can still return `4xx` or `5xx`, often with an error document whose shape differs from the expected success payload. Parsing that body as business data can produce misleading parse errors, incomplete records, or decisions based on an error response.
## Best Practice
After handling any platform or transport failure, check `HttpResponseMessage.IsSuccessStatusCode()` or the expected `HttpStatusCode()` before interpreting the response body as a success payload. Handle non-success status explicitly and include safe diagnostic context when appropriate. A bounded error body may be read for diagnostics, but it must not enter the success parsing path.
See sample: [`check-http-status-before-consuming-response-body.good.al`](check-http-status-before-consuming-response-body.good.al).
## Anti Pattern
Checking only the Boolean result of `Get`, `Post`, `Put`, `Delete`, or `Send` and then parsing `Response.Content()` as the expected payload. The Boolean can be `true` for any HTTP status, including authentication failures, throttling, validation errors, and server failures.
See sample: [`check-http-status-before-consuming-response-body.bad.al`](check-http-status-before-consuming-response-body.bad.al).
## References
- [HttpResponseMessage.IsSuccessStatusCode method](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/methods-auto/httpresponsemessage/httpresponsemessage-issuccessstatuscode-method)
- [HttpClient data type](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/methods-auto/httpclient/httpclient-data-type)

View file

@ -0,0 +1,11 @@
codeunit 50173 "Contact Payload Reader Bad"
{
procedure ApplyPayload(var Contact: Record Contact; Payload: JsonObject)
var
Token: JsonToken;
begin
if Payload.Get('email', Token) then
Contact.Validate("E-Mail", CopyStr(Token.AsValue().AsText(), 1, MaxStrLen(Contact."E-Mail")));
Contact.Modify(true);
end;
}

View file

@ -0,0 +1,28 @@
codeunit 50172 "Contact Payload Reader Good"
{
procedure ApplyPayload(var Contact: Record Contact; Payload: JsonObject)
var
EmailAddress: Text;
begin
if TryGetText(Payload, 'email', EmailAddress) then
Contact.Validate("E-Mail", CopyStr(EmailAddress, 1, MaxStrLen(Contact."E-Mail")));
Contact.Modify(true);
end;
local procedure TryGetText(Payload: JsonObject; PropertyName: Text; var Value: Text): Boolean
var
Token: JsonToken;
begin
if not Payload.Get(PropertyName, Token) then
exit(false);
if not Token.IsValue() then
Error(NotAValueErr, PropertyName);
if Token.AsValue().IsNull() then
exit(false);
Value := Token.AsValue().AsText();
exit(true);
end;
var
NotAValueErr: Label 'The property %1 must contain a single value.', Comment = '%1 = JSON property name';
}

View file

@ -0,0 +1,35 @@
---
bc-version: [all]
domain: web-services
keywords: [jsonobject, jsontoken, jsonvalue, isnull, asvalue, astext, asdecimal, optional-property, json-null, payload-parsing]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Check for JSON null before converting a value
## Description
An optional property in a JSON payload can be missing or present with the value `null`, and the two cases behave differently in AL. When the Boolean result is captured, `JsonObject.Get` returns `false` for a missing key but `true` for `"email": null`, because the key exists. The conversion methods on `JsonValue` (`AsText`, `AsCode`, `AsDecimal`, `AsInteger`, `AsDate`, `AsBoolean`, and the others) fail with a runtime error when the value is `NULL` or `UNDEFINED`. Code that guards only with `Get` therefore passes its tests with the property omitted and fails in production when the sender serializes an empty field as `null`, which many services do by default.
## Best Practice
For every property that the contract allows to be optional or nullable, check three things before converting: that `Get` (or `SelectToken`) returned `true`, that the token `IsValue()` rather than an object or array, and that `AsValue().IsNull()` is `false`. Put this in one small helper per target type and decide explicitly what a missing or null property means: a default, leaving the field unchanged, or a validation error that names the property.
Required properties can still fail fast, but with an error that states the missing or null property instead of a generic conversion error. Keep a numeric or date conversion strict when the contract says the value must be a number or a date; `IsNull` covers only `null`, not a value of the wrong type.
See sample: [`check-json-null-before-converting-values.good.al`](check-json-null-before-converting-values.good.al).
## Anti Pattern
`if Json.Get('email', Token) then Email := Token.AsValue().AsText();` or an unguarded `Token.AsValue().AsDecimal()` on a property that the external contract allows to be `null`. The `Get` check makes the code look defensive, but it doesn't handle a present `null`. Detection signal: an `As<Type>()` call on a `JsonValue` obtained from an external payload with no preceding `IsNull()` check on the same token, where the property isn't documented as always non-null.
See sample: [`check-json-null-before-converting-values.bad.al`](check-json-null-before-converting-values.bad.al).
## References
- [JsonObject.Get method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/jsonobject/jsonobject-get-method)
- [JsonValue.AsText method: fails on NULL or UNDEFINED](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/jsonvalue/jsonvalue-astext-method)
- [JsonValue.IsNull method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/jsonvalue/jsonvalue-isnull-method)
- [JsonToken data type](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/jsontoken/jsontoken-data-type)

View file

@ -0,0 +1,27 @@
codeunit 50171 "Exchange Rate Export Bad"
{
procedure SendRate(CurrencyCode: Code[10]; StartingDate: Date; ExchangeRate: Decimal)
var
Client: HttpClient;
Response: HttpResponseMessage;
RequestUrl: Text;
begin
RequestUrl := StrSubstNo(RateUrlTok, CurrencyCode, Format(StartingDate), Format(ExchangeRate));
if not Client.Get(RequestUrl, Response) then
Error(RequestFailedErr);
if not Response.IsSuccessStatusCode() then
Error(RateRejectedErr, Response.HttpStatusCode());
end;
procedure ReadRate(RateText: Text) ExchangeRate: Decimal
begin
if not Evaluate(ExchangeRate, RateText) then
Error(InvalidRateErr, RateText);
end;
var
RateUrlTok: Label 'https://rates.example.com/rates?currency=%1&date=%2&rate=%3', Locked = true;
RequestFailedErr: Label 'The exchange rate service could not be reached.';
RateRejectedErr: Label 'The exchange rate service rejected the rate. Status code: %1.', Comment = '%1 = HTTP status code';
InvalidRateErr: Label 'The exchange rate %1 is not a valid decimal number.', Comment = '%1 = received value';
}

View file

@ -0,0 +1,27 @@
codeunit 50170 "Exchange Rate Export Good"
{
procedure SendRate(CurrencyCode: Code[10]; StartingDate: Date; ExchangeRate: Decimal)
var
Client: HttpClient;
Response: HttpResponseMessage;
RequestUrl: Text;
begin
RequestUrl := StrSubstNo(RateUrlTok, CurrencyCode, Format(StartingDate, 0, 9), Format(ExchangeRate, 0, 9));
if not Client.Get(RequestUrl, Response) then
Error(RequestFailedErr);
if not Response.IsSuccessStatusCode() then
Error(RateRejectedErr, Response.HttpStatusCode());
end;
procedure ReadRate(RateText: Text) ExchangeRate: Decimal
begin
if not Evaluate(ExchangeRate, RateText, 9) then
Error(InvalidRateErr, RateText);
end;
var
RateUrlTok: Label 'https://rates.example.com/rates?currency=%1&date=%2&rate=%3', Locked = true;
RequestFailedErr: Label 'The exchange rate service could not be reached.';
RateRejectedErr: Label 'The exchange rate service rejected the rate. Status code: %1.', Comment = '%1 = HTTP status code';
InvalidRateErr: Label 'The exchange rate %1 is not a valid decimal number.', Comment = '%1 = received value';
}

View file

@ -0,0 +1,39 @@
---
bc-version: [all]
domain: web-services
keywords: [format, evaluate, standard-format-9, xml-format, locale, regional-settings, decimal-separator, data-exchange, integration]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Format exchanged values with standard format 9
## Description
`Format(Value)` uses standard format 0, the display format, and follows the current user's regional settings. The same decimal renders as `-76.543,21` for a European region and `-76,543.21` for English (US); the same date renders as `05-04-21` or `04/05/21`. Text built this way and sent outside Business Central (an HTTP query string or body, an XML or CSV file, a signature or hash input, or an external key) changes with the user or job queue session that produces it. The receiver can reject it or, worse, misread it. `Evaluate` without a format number has the same dependency when it parses machine-generated text.
## Best Practice
Use `Format(Value, 0, 9)` for machine-readable text. Standard format 9 is the XML format and doesn't depend on the region: `-76543.21` for a decimal, `2021-04-05` for a date, `04:35:55.553` for a time, `true`/`false` for a Boolean, and a UTC `DateTime` such as `2021-04-05T03:35:55.553Z`. Parse such text with `Evaluate(Variable, Text, 9)`.
Prefer typed APIs when they exist. `JsonObject.Add` and `JsonValue.SetValue` with a `Decimal`, `Date`, or `Boolean` argument write a JSON value without going through display text. An XMLport handles this with `FormatEvaluate = Xml`.
For `Enum` and `Option` values, format 9 produces the ordinal number, not the name. When the external contract exchanges names, map them explicitly; see [`api-enum-values-are-a-contract-by-name-not-ordinal.md`](api-enum-values-are-a-contract-by-name-not-ordinal.md).
Text shown to a person (messages, captions, report columns, notifications) should keep the regional display format. `Code`, `Text`, and `Guid` values don't need format 9 because their standard formats don't vary by region.
See sample: [`format-exchanged-values-with-standard-format-9.good.al`](format-exchanged-values-with-standard-format-9.good.al).
## Anti Pattern
`Format(Amount)`, `Format(PostingDate)`, or `Format(SomeDateTime)` concatenated into a URL, request body, XML or CSV line, file name, or hash input. Passing the `Decimal` or `Date` itself to `StrSubstNo` for such text has the same effect, because `StrSubstNo` formats it with the display format. Also `Evaluate(DecimalOrDateVariable, ExternalText)` without format number 9 on text received from another system. The code usually works for the developer's own region and fails for users or job queue sessions in another one. Detection signal: `Format` with one argument, or with a format number other than 9, applied to a `Decimal`, `Date`, `Time`, `DateTime`, or `Boolean` on a path that writes to an `HttpContent`, `HttpRequestMessage`, `OutStream`, `XmlDocument`, or file.
See sample: [`format-exchanged-values-with-standard-format-9.bad.al`](format-exchanged-values-with-standard-format-9.bad.al).
## References
- [Formatting values, dates, and time: standard formats by region and format 9](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-format-property)
- [System.Format method](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/system/system-format-joker-integer-integer-method)
- [System.Evaluate method and format number 9](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/system/system-evaluate-method)
- [FormatEvaluate property for XMLports](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/properties/devenv-formatevaluate-property)

View file

@ -0,0 +1,18 @@
codeunit 50100 "Http Platform Failure Bad"
{
procedure GetCustomer(CustomerId: Guid): Text
var
Client: HttpClient;
Response: HttpResponseMessage;
ResponseText: Text;
RequestSucceeded: Boolean;
begin
RequestSucceeded := Client.Get(
StrSubstNo('https://api.example.com/customers/%1', CustomerId),
Response);
// Response content is unavailable when the platform call failed.
Response.Content().ReadAs(ResponseText);
exit(ResponseText);
end;
}

View file

@ -0,0 +1,18 @@
codeunit 50100 "Http Platform Failure Good"
{
procedure GetCustomer(CustomerId: Guid): Text
var
Client: HttpClient;
Response: HttpResponseMessage;
ResponseText: Text;
begin
if not Client.Get(
StrSubstNo('https://api.example.com/customers/%1', CustomerId),
Response)
then
Error('The customer service could not be reached.');
Response.Content().ReadAs(ResponseText);
exit(ResponseText);
end;
}

View file

@ -0,0 +1,31 @@
---
bc-version: [all]
domain: web-services
keywords: [httpclient, transport-failure, boolean-return, httpresponsemessage, content, runtime-error]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Handle HttpClient platform failure before accessing the response
## Description
AL `HttpClient` methods can fail before a usable HTTP response exists because of an invalid request, DNS or network failure, certificate validation, timeout, a disabled extension setting, or the response-size limit. When code captures the optional Boolean return value, `false` reports this platform or transport failure. The accompanying `HttpResponseMessage` is not safe to consume; accessing its content after the failed call can raise another error and obscure the original failure.
## Best Practice
When capturing the Boolean return value from `Get`, `Post`, `Put`, `Delete`, or `Send`, stop the current response-processing path immediately when it is `false`. Report or propagate the transport failure without reading status, headers, or content. Omitting the optional Boolean is also valid when fail-fast behavior is intended: the runtime then raises an error if the operation cannot execute.
See sample: [`handle-httpclient-platform-failure-before-response-access.good.al`](handle-httpclient-platform-failure-before-response-access.good.al).
## Anti Pattern
Capturing a failed call in a Boolean and then reading `Response.Content()`, parsing the body, or otherwise treating `Response` as usable. Do not report omission of the Boolean by itself; that form deliberately delegates failure propagation to the runtime.
See sample: [`handle-httpclient-platform-failure-before-response-access.bad.al`](handle-httpclient-platform-failure-before-response-access.bad.al).
## References
- [HttpClient.Send method](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/methods-auto/httpclient/httpclient-send-method)
- [Call external services with HttpClient](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/devenv-httpclient)

View file

@ -0,0 +1,24 @@
page 50102 "Project Task API"
{
PageType = API;
APIPublisher = 'contoso';
APIGroup = 'jobs';
APIVersion = 'v1.0';
EntityName = 'projectTask';
EntitySetName = 'projectTasks';
SourceTable = "Project Task";
layout
{
area(content)
{
repeater(General)
{
// "Remaining Hours" is a stored field, set inside the
// OnValidate of "Budgeted Hours" — it goes stale whenever
// "Hours Used" changes through any other path.
field(remainingHours; Rec."Remaining Hours") { }
}
}
}
}

View file

@ -0,0 +1,33 @@
page 50102 "Project Task API"
{
PageType = API;
APIPublisher = 'contoso';
APIGroup = 'jobs';
APIVersion = 'v1.0';
EntityName = 'projectTask';
EntitySetName = 'projectTasks';
SourceTable = "Project Task";
DelayedInsert = true;
layout
{
area(content)
{
repeater(General)
{
field(remainingHours; RemainingHoursCalc) { }
field(budgetedHours; Rec."Budgeted Hours") { }
field(hoursUsed; Rec."Hours Used") { }
}
}
}
trigger OnAfterGetRecord()
begin
Rec.CalcFields("Hours Used");
RemainingHoursCalc := Rec."Budgeted Hours" - Rec."Hours Used";
end;
var
RemainingHoursCalc: Decimal;
}

View file

@ -0,0 +1,28 @@
---
bc-version: [all]
domain: web-services
keywords: [api-page, derived-fields, exposure, odata]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Recalculate Stored Derived Fields Before Exposing Them on API Pages
> Contributions welcome — open a PR to refine or extend this article.
## Description
A stored field whose value is derived from other fields inside an `OnValidate` trigger only updates when that specific trigger fires. If the underlying source data changes through some other path, the stored value goes stale without raising any error. Exposing such a field directly on an API page hands external consumers a snapshot that may be significantly out of date.
## Best Practice
Recalculate the derived value in `OnAfterGetRecord` from its authoritative source — typically a FlowField — using a page-level variable, and expose that recalculated value instead of the stale stored field. Whether to also expose the source fields is a separate design decision, not a requirement of this pattern; keep the API contract scoped to what consumers actually need. If letting the consumer verify the recalculation is itself a requirement, expose every field the calculation reads, not just one of them — a derived value with two inputs needs both exposed, or the "verification" is incomplete.
See sample: [`stored-derived-fields-must-not-be-exposed-directly.good.al`](stored-derived-fields-must-not-be-exposed-directly.good.al).
## Anti Pattern
Exposing the stored field directly via `Rec`, trusting that it was kept in sync by whichever trigger last touched it.
See sample: [`stored-derived-fields-must-not-be-exposed-directly.bad.al`](stored-derived-fields-must-not-be-exposed-directly.bad.al).

View file

@ -52,6 +52,7 @@ The following targeted checks cover every current `appsource` article across the
- A page or report that repository context identifies as a direct user entry point omits `UsageCategory` or sets it to `None` — `set-usagecategory-on-searchable-entry-points`. Do not select this article based only on object type; exclude supporting parts, dialogs, API pages, and objects intentionally reached through another page. - A page or report that repository context identifies as a direct user entry point omits `UsageCategory` or sets it to `None` — `set-usagecategory-on-searchable-entry-points`. Do not select this article based only on object type; exclude supporting parts, dialogs, API pages, and objects intentionally reached through another page.
- A `DateTime` assignment adds or subtracts a fixed duration to represent an assumed regional offset — `do-not-hard-code-time-zone-offsets`. Require contextual evidence such as an hour-sized constant, offset-oriented name, or time-zone comment; do not flag deadlines, schedules, or elapsed-time calculations. - A `DateTime` assignment adds or subtracts a fixed duration to represent an assumed regional offset — `do-not-hard-code-time-zone-offsets`. Require contextual evidence such as an hour-sized constant, offset-oriented name, or time-zone comment; do not flag deadlines, schedules, or elapsed-time calculations.
- For BC v27 or later, `app.json` adds or changes the `help` URL to a path deeper than two levels, or a changed Copilot/context-sensitive help arrangement would ground the app under an overly broad truncated parent — `keep-copilot-help-url-to-two-path-levels`. - For BC v27 or later, `app.json` adds or changes the `help` URL to a path deeper than two levels, or a changed Copilot/context-sensitive help arrangement would ground the app under an overly broad truncated parent — `keep-copilot-help-url-to-two-path-levels`.
- Changed code declares or calls `File.Open`/`File.Create`/`File.Read`/`File.Write` in an app targeting Business Central Online — `file-datatype-saas`.
Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`. Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.

View file

@ -50,7 +50,9 @@ The following targeted checks cover every current `breaking-changes` article:
- A published procedure changes parameter count/order/type/name, `var`, return type, or array shape instead of preserving the old signature and adding an overload — `do-not-change-published-procedure-signatures`. - A published procedure changes parameter count/order/type/name, `var`, return type, or array shape instead of preserving the old signature and adding an overload — `do-not-change-published-procedure-signatures`.
- A public procedure/event/interface exposes a credential or other sensitive value through `Text` or an externally callable contract — `do-not-expose-sensitive-data-through-public-api`. - A public procedure/event/interface exposes a credential or other sensitive value through `Text` or an externally callable contract — `do-not-expose-sensitive-data-through-public-api`.
- Code already marked obsolete is expanded with new behavior instead of routing new callers to its replacement — `do-not-modify-code-already-marked-obsolete`. - Code already marked obsolete is expanded with new behavior instead of routing new callers to its replacement — `do-not-modify-code-already-marked-obsolete`.
- A shipped table field is deleted, renamed, renumbered, or replaced without retaining the original field as `ObsoleteState = Pending` and migrating its data — `obsolete-table-fields-instead-of-deleting-them`. - A shipped table field is deleted, renamed, renumbered, or replaced without retaining the original field as `ObsoleteState = Pending` and migrating its data — `obsolete-table-fields-instead-of-deleting-them`. This owns AS0005 field-name changes; do not substitute the namespace article.
- A published object's namespace changes between the base and changed source while its identity otherwise remains — `namespace-is-part-of-published-object-identity`. Do not apply it to a new, unshipped object or to an ordinary object-name change with no namespace change.
- New or changed code calls `Codeunit Mail`'s `CreateMessage`/`Send`/`GetErrorDesc` instead of `Codeunit Email`/`Codeunit "Email Message"` — `prefer-email-module`.
For `obsolete-table-fields-instead-of-deleting-them`, compare the baseline ID and name before emitting. When the original field remains under the same ID and name with `ObsoleteState = Pending`, and the replacement uses a new ID, the change follows the rule and must not be flagged. For `obsolete-table-fields-instead-of-deleting-them`, compare the baseline ID and name before emitting. When the original field remains under the same ID and name with `ObsoleteState = Pending`, and the replacement uses a new ID, the change follows the rule and must not be flagged.

View file

@ -46,8 +46,12 @@ The sub-skills invoked by this skill are those listed in frontmatter `sub-skills
Hosts that orchestrate leaves mechanically SHOULD run Hosts that orchestrate leaves mechanically SHOULD run
`tools/Build-SkillIndex.ps1` and resolve this skill by `id: al-code-review`. `tools/Build-SkillIndex.ps1` and resolve this skill by `id: al-code-review`.
The generated `subSkills` array preserves the frontmatter order and avoids The generated `subSkills` array preserves the frontmatter slot order and
host-specific Markdown parsing. avoids host-specific Markdown parsing. Before invoking leaves, run
`tools/Resolve-SkillWorklist.ps1` with this skill's path and the task's enabled
layers and disabled skill paths. Each declared path supplies a leaf `id`; the
resolver selects the highest-precedence enabled implementation with that `id`
without changing slot order.
## Relevance ## Relevance

View file

@ -16,7 +16,7 @@ application-area: [all]
Reviews AL source changes against the `data-modeling` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is one of the skills composed by `al-code-review`. Reviews AL source changes against the `data-modeling` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is one of the skills composed by `al-code-review`.
An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. Data-modeling findings are narrow by design — they apply when the review scope contains setup or master tables, their card pages, primary keys, number-series assignment, block enforcement, audit fields, document print/email/Post-and-Send actions, `Navigate` page subscribers, Report Selection registration or dispatch, price-calculation/price-source extensibility, `TransferFields`-based posting-cascade field mirroring, or barcode/report-layout font-provider usage. The skill returns `not-applicable` when none of those apply. An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. Data-modeling findings are narrow by design — they apply when the review scope contains setup or master tables, their card pages, primary keys, number-series assignment, block enforcement, audit fields, document print/email/Post-and-Send actions, `Navigate` page subscribers, Report Selection registration or dispatch, price-calculation/price-source extensibility, `TransferFields`-based posting-cascade field mirroring, barcode/report-layout font-provider usage, dimension wiring, journal-based posting-routine structure, or Item Ledger Entry document-number lookups after a combined sales post. The skill returns `not-applicable` when none of those apply.
## Source ## Source
@ -46,6 +46,7 @@ A file enters the candidate worklist when its `keywords` intersect the extracted
The following targeted checks cover every current `data-modeling` article. Treat each as a candidate-selection cue: when the signal appears in changed code, add the named article to the worklist and evaluate it in Action. The following targeted checks cover every current `data-modeling` article. Treat each as a candidate-selection cue: when the signal appears in changed code, add the named article to the worklist and evaluate it in Action.
- A `* Setup` table or its page changes singleton structure, uses a nonblank or generated key, permits insert/delete, uses a List page, or does not ensure the blank-keyed row exists — `setup-table-is-a-singleton`. - A `* Setup` table or its page changes singleton structure, uses a nonblank or generated key, permits insert/delete, uses a List page, or does not ensure the blank-keyed row exists — `setup-table-is-a-singleton`.
- Code reads `Item Ledger Entry."Document No."` (or `"Last Shipping No."`/`"Last Posting No."`) after a combined Ship+Invoice **sales** post — `item-ledger-entry-document-no-follows-last-shipping-no`. This is a sales-specific rule: purchase combined posting is Receive+Invoice and uses receiving fields such as `"Last Receiving No."`, not the shipment/document-number behavior this article describes. Do not worklist it from purchase posting code.
- A custom master table changes its primary key, `No.`/`No. Series` fields, or `OnInsert` without assigning a blank `No.` from setup through a number series — `master-table-no-from-number-series-in-oninsert`. - A custom master table changes its primary key, `No.`/`No. Series` fields, or `OnInsert` without assigning a blank `No.` from setup through a number series — `master-table-no-from-number-series-in-oninsert`.
- BC v22 or later code introduces or retains `NoSeriesManagement`, `InitSeries`, `SelectSeries`, or `SetSeries`, or number assignment/manual-entry checks do not use codeunit `"No. Series"` methods such as `GetNextNo`, `IsManual`, or `TestManual` — `use-no-series-codeunit-not-noseriesmanagement`. - BC v22 or later code introduces or retains `NoSeriesManagement`, `InitSeries`, `SelectSeries`, or `SetSeries`, or number assignment/manual-entry checks do not use codeunit `"No. Series"` methods such as `GetNextNo`, `IsManual`, or `TestManual` — `use-no-series-codeunit-not-noseriesmanagement`.
- A master gains or changes `Blocked`, or a document line, journal line, reference-field `OnValidate`, or posting routine uses that master without `TestField(Blocked, false)` at the point of use; also cue when the check is placed only in the master's own triggers — `check-blocked-in-referencing-code-not-in-master`. - A master gains or changes `Blocked`, or a document line, journal line, reference-field `OnValidate`, or posting routine uses that master without `TestField(Blocked, false)` at the point of use; also cue when the check is placed only in the master's own triggers — `check-blocked-in-referencing-code-not-in-master`.
@ -63,6 +64,10 @@ The following targeted checks cover every current `data-modeling` article. Treat
- An `enumextension` extends `"Price Source Type"` with a new value intended for a sales, purchase, or job price list, without extending the matching document subset enum (`"Sales Price Source Type"`, `"Purchase Price Source Type"`, `"Job Price Source Type"`) with a value at the same numeric ID — `extend-price-source-type-must-sync-document-subset-enum`. - An `enumextension` extends `"Price Source Type"` with a new value intended for a sales, purchase, or job price list, without extending the matching document subset enum (`"Sales Price Source Type"`, `"Purchase Price Source Type"`, `"Job Price Source Type"`) with a value at the same numeric ID — `extend-price-source-type-must-sync-document-subset-enum`.
- A codeunit subscribes to `"Sales Line - Price"`'s `OnAfterAddSources` to register a custom field as a price source via `PriceSourceList.Add`, but that field has no `OnValidate` (or matching `OnAfterValidate`) that triggers recalculation — either `SalesLine.UpdateUnitPrice(<field no.>)`, or the explicit `SalesLine.PlanPriceCalcByField(<field no.>)` followed by `SalesLine.UpdateUnitPriceByField(<same field no.>)`. A bare `UpdateUnitPriceByField` without a preceding `PlanPriceCalcByField` for the same field number does not count as recalculation (it exits without recalculating) — `new-price-source-must-add-candidate-and-trigger-recalculation`. - A codeunit subscribes to `"Sales Line - Price"`'s `OnAfterAddSources` to register a custom field as a price source via `PriceSourceList.Add`, but that field has no `OnValidate` (or matching `OnAfterValidate`) that triggers recalculation — either `SalesLine.UpdateUnitPrice(<field no.>)`, or the explicit `SalesLine.PlanPriceCalcByField(<field no.>)` followed by `SalesLine.UpdateUnitPriceByField(<same field no.>)`. A bare `UpdateUnitPriceByField` without a preceding `PlanPriceCalcByField` for the same field number does not count as recalculation (it exits without recalculating) — `new-price-source-must-add-candidate-and-trigger-recalculation`.
- A report hand-constructs a barcode string only where a concrete, independently provable defect is visible: the source value can contain characters outside the symbology's character set and is never validated, a checksum the symbology/setup requires is never applied, or there is concrete evidence of an incompatible font binding. Do not flag manual start/stop delimiters by themselves — `*value*` is a documented, valid Code 39 form for IDAutomation fonts (IDAutomation also accepts parentheses), so delimiter choice alone is never a finding. Also flag module use that does not match the interface: a 1D `"Barcode Font Provider"` path must call both `ValidateInput` and `EncodeFont`; a 2D `"Barcode Font Provider 2D"` path calls `EncodeFont` only (the 2D interface has no `ValidateInput`, so its absence there is not a finding). Separately, flag an otherwise correctly encoded barcode whose report layout names an evaluation/demo font instead of the purchased production font name — `report-barcodes-must-use-barcode-module-and-production-font-name`. - A report hand-constructs a barcode string only where a concrete, independently provable defect is visible: the source value can contain characters outside the symbology's character set and is never validated, a checksum the symbology/setup requires is never applied, or there is concrete evidence of an incompatible font binding. Do not flag manual start/stop delimiters by themselves — `*value*` is a documented, valid Code 39 form for IDAutomation fonts (IDAutomation also accepts parentheses), so delimiter choice alone is never a finding. Also flag module use that does not match the interface: a 1D `"Barcode Font Provider"` path must call both `ValidateInput` and `EncodeFont`; a 2D `"Barcode Font Provider 2D"` path calls `EncodeFont` only (the 2D interface has no `ValidateInput`, so its absence there is not a finding). Separately, flag an otherwise correctly encoded barcode whose report layout names an evaluation/demo font instead of the purchased production font name — `report-barcodes-must-use-barcode-module-and-production-font-name`.
- A new field is typed `Code`/`Text` and its `OnValidate` calls `DimensionManagement`/`DimMgt`, or a table adds Shortcut Dimension fields, a `Dimension Set ID` field, or `AddDimSource`/`GetDefaultDimID` — `dimension-management-wiring`. A master table calling `SaveDefaultDim` and a document/journal table computing its own `Dimension Set ID` are two different valid shapes; do not flag a master table for lacking a `Dimension Set ID` field or a document for lacking `SaveDefaultDim`.
- A journal-based posting codeunit is added or changed and validation, Journal-table access, ledger writes, and user-interaction (`Confirm`/dialogs) all occur in one procedure or one codeunit, rather than split across `Check Line`/`Post Line`/`Post Batch`-shaped companions — `check-post-line-batch-pattern`. A document posting routine calling `Post Line` directly without a `Post Batch` companion is not this anti-pattern.
- An existing, already-published table's `keys` block adds, removes, or reorders a field in its primary key or any `Clustered = true` key — `do-not-change-primary-key`. A new table defining its own key for the first time is not this anti-pattern; requires repository/publication context to know the table has already shipped.
- Code reads a setup/configuration-table field inside a branch that has already decided the value is required, and blank/zero is handled with a fallback to a default rather than `TestField`/an equivalent guard — `testfield-required-setup-field`. A read that is genuinely optional in that branch, or one already guarded by `TestField`, is not this anti-pattern.
Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`. Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.
@ -92,7 +97,7 @@ Outcome selection:
- `completed` — the skill evaluated every worklist item. - `completed` — the skill evaluated every worklist item.
- `no-knowledge` — no applicable data-modeling knowledge survived filtering. - `no-knowledge` — no applicable data-modeling knowledge survived filtering.
- `not-applicable` — the diff touches no setup/master table, page, key, numbering, block-check, or audit-field surface, and no document print/email/Post-and-Send action, `Navigate` subscriber, Report Selection registration/dispatch, price-calculation/price-source extensibility point, posting-cascade `TransferFields` mirroring, or barcode/report-font-provider usage. - `not-applicable` — the diff touches no setup/master table, page, key, numbering, block-check, or audit-field surface, and no document print/email/Post-and-Send action, `Navigate` subscriber, Report Selection registration/dispatch, price-calculation/price-source extensibility point, posting-cascade `TransferFields` mirroring, barcode/report-font-provider usage, dimension wiring, posting-routine structure, or Item-Ledger-Entry-document-number surface.
- `partial` — a budget was hit before the worklist was exhausted. - `partial` — a budget was hit before the worklist was exhausted.
- `failed` — an unrecoverable error occurred. - `failed` — an unrecoverable error occurred.

View file

@ -22,6 +22,13 @@ An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-pat
Use READ's **Bounded retrieval for review skills** workflow with `-Domain error-handling`. Consume every catalog page across enabled layers before applying this leaf's Relevance and Worklist; preserve each exact catalog path and open complete bodies only for exact paths selected by the Worklist. If the helper or prepared index is unavailable or invalid, use READ's explicit path-discovery and bounded native-read fallback. Use READ's **Bounded retrieval for review skills** workflow with `-Domain error-handling`. Consume every catalog page across enabled layers before applying this leaf's Relevance and Worklist; preserve each exact catalog path and open complete bodies only for exact paths selected by the Worklist. If the helper or prepared index is unavailable or invalid, use READ's explicit path-discovery and bounded native-read fallback.
When the review scope contains outbound `HttpClient.Get` or `HttpClient.Post` calls, including resolved call paths, also retrieve every catalog page with `-Domain web-services`, using the same known task dimensions and enabled layers. Restrict supplementary candidates to these article slugs across enabled layers; the links identify their canonical owners:
- [`handle-httpclient-platform-failure-before-response-access`](../../knowledge/web-services/handle-httpclient-platform-failure-before-response-access.md)
- [`check-http-status-before-consuming-response-body`](../../knowledge/web-services/check-http-status-before-consuming-response-body.md)
Retain each selected catalog row's exact `path`; do not invent paths for missing, pruned, or disabled entries. Apply this leaf's Relevance, Worklist, and READ layer precedence to the supplementary candidates before retrieving complete bodies with `Get-KnowledgeArticles.ps1`. Apply each selected article's own scope and exceptions when evaluating code and agent-finding candidates. Use READ's bounded path-discovery fallback over these same sources when needed. This supplements error-handling knowledge, not the scope of the review with unrelated web-services concerns.
## Relevance ## Relevance
Apply the frontmatter matching rules defined in READ (*Frontmatter matching semantics*) against the task context: Apply the frontmatter matching rules defined in READ (*Frontmatter matching semantics*) against the task context:
@ -40,6 +47,7 @@ Narrow the relevant files to the subset that applies to the changes under review
- The changed AL object names and types — especially codeunits that post or validate, tables and table extensions with `OnValidate` triggers, and any procedure that raises errors or orchestrates a batch over records. - The changed AL object names and types — especially codeunits that post or validate, tables and table extensions with `OnValidate` triggers, and any procedure that raises errors or orchestrates a batch over records.
- The changed procedures and triggers, weighted toward `OnValidate`/`OnInsert`/`OnModify` triggers, posting and validation routines, and procedures attributed with `[ErrorBehavior(...)]` or `[TryFunction]`. - The changed procedures and triggers, weighted toward `OnValidate`/`OnInsert`/`OnModify` triggers, posting and validation routines, and procedures attributed with `[ErrorBehavior(...)]` or `[TryFunction]`.
- Tokens extracted from the diff that relate to error surfacing and diagnostics (`Error`, `ErrorInfo`, `FieldError`, `TestField`, `Title`, `Message`, `DetailedMessage`, `AddAction`, `AddNavigationAction`, `RecordId`, `PageNo`, `ErrorBehavior`, `Collect`, `HasCollectedErrors`, `GetCollectedErrors`, `ClearCollectedErrors`, `ErrorType`, `Internal`, `Client`, `TryFunction`, `GetLastErrorText`, Boolean assignment). - Tokens extracted from the diff that relate to error surfacing and diagnostics (`Error`, `ErrorInfo`, `FieldError`, `TestField`, `Title`, `Message`, `DetailedMessage`, `AddAction`, `AddNavigationAction`, `RecordId`, `PageNo`, `ErrorBehavior`, `Collect`, `HasCollectedErrors`, `GetCollectedErrors`, `ClearCollectedErrors`, `ErrorType`, `Internal`, `Client`, `TryFunction`, `GetLastErrorText`, Boolean assignment).
- For the outbound HTTP call paths identified in Source, include `HttpClient`, `Get`, `Post`, `HttpResponseMessage`, response use, and caller failure handling (including `[TryFunction]` call sites) in keyword and topic matching.
- Resolve changed standalone call targets; when the target declaration has `[TryFunction]`, worklist the ignored-return rule even if the declaration itself is unchanged. Only assignment and conditional use activate try semantics. - Resolve changed standalone call targets; when the target declaration has `[TryFunction]`, worklist the ignored-return rule even if the declaration itself is unchanged. Only assignment and conditional use activate try semantics.
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.

View file

@ -58,6 +58,7 @@ Apply these targeted cues even when simple token overlap would rank the article
- Worklist `job-queue-on-hold-does-not-stop-running-work.md` when a running job queue handler polls the entry's `Status` or `On Hold` value as a cancellation signal. Exclude application-owned stop requests that are checked before every bounded unit of work, including the first, when completed work and its checkpoint remain consistent and resume logic clears the request. - Worklist `job-queue-on-hold-does-not-stop-running-work.md` when a running job queue handler polls the entry's `Status` or `On Hold` value as a cancellation signal. Exclude application-owned stop requests that are checked before every bounded unit of work, including the first, when completed work and its checkpoint remain consistent and resume logic clears the request.
- Worklist `job-queue-category-code-serializes-conflicting-jobs.md` when two or more job queue entries in the same company are shown by the changed context to require mutual exclusion but have empty or different Job Queue Category Codes. Do not infer a conflict merely because jobs touch the same tables, and do not recommend a category to coordinate across companies, environments, or workers outside the job queue dispatcher. - Worklist `job-queue-category-code-serializes-conflicting-jobs.md` when two or more job queue entries in the same company are shown by the changed context to require mutual exclusion but have empty or different Job Queue Category Codes. Do not infer a conflict merely because jobs touch the same tables, and do not recommend a category to coordinate across companies, environments, or workers outside the job queue dispatcher.
- Worklist `store-scheduled-task-id-to-avoid-duplicate-tasks.md` when `TaskScheduler.CreateTask` runs from initialization, login, setup, or another repeatable path without persisting its returned GUID and checking it with `TaskScheduler.TaskExists` before creating a replacement. Exclude one-shot creation and correctly persisted check-before-create flows; concurrent callers still require serialization around that sequence. - Worklist `store-scheduled-task-id-to-avoid-duplicate-tasks.md` when `TaskScheduler.CreateTask` runs from initialization, login, setup, or another repeatable path without persisting its returned GUID and checking it with `TaskScheduler.TaskExists` before creating a replacement. Exclude one-shot creation and correctly persisted check-before-create flows; concurrent callers still require serialization around that sequence.
- A new or changed report object whose usage/name/caption identifies it as a single-record document (invoice, statement, order confirmation) sets or retains `DefaultRenderingLayout = RDLC` — `document-report-word-layout.md`. Do not worklist this from a tabular/list report with heavy aggregation or calculated columns; RDLC/Excel remains the better fit there.
These targeted inclusions and exclusions override generic token overlap. Do not retain an excluded article solely because the diff contains one of its keywords. These targeted inclusions and exclusions override generic token overlap. Do not retain an excluded article solely because the diff contains one of its keywords.

View file

@ -77,6 +77,7 @@ paths; they select articles, not findings. Facts and exceptions stay in articles
| Registered warehouse quantity/physical-adjustment synchronization, `"Directed Put-away and Pick"`, `"Adjustment Bin Code"`, `"Warehouse Adjustment"`, or `"Calculate Whse. Adjustment"` and the resulting item-journal posting | `reconcile-warehouse-adjustments-with-the-item-ledger` | | Registered warehouse quantity/physical-adjustment synchronization, `"Directed Put-away and Pick"`, `"Adjustment Bin Code"`, `"Warehouse Adjustment"`, or `"Calculate Whse. Adjustment"` and the resulting item-journal posting | `reconcile-warehouse-adjustments-with-the-item-ledger` |
| `"Transfer Header"`/`"Transfer Line"` shipment/receipt completion, transfer posting publishers, in-transit/document-link changes, or item-journal posting presented as transfer-order completion | `post-transfers-through-shipment-and-receipt-codeunits` | | `"Transfer Header"`/`"Transfer Line"` shipment/receipt completion, transfer posting publishers, in-transit/document-link changes, or item-journal posting presented as transfer-order completion | `post-transfers-through-shipment-and-receipt-codeunits` |
| `Inventory`, `CalcQtyAvailableToPromise`, or stock sums used in a dated supply/demand promise, including changed location/variant/date filters and source-demand context | `use-date-aware-availability-for-promising` | | `Inventory`, `CalcQtyAvailableToPromise`, or stock sums used in a dated supply/demand promise, including changed location/variant/date filters and source-demand context | `use-date-aware-availability-for-promising` |
| Direct assignment to `Quantity`, `"Unit of Measure Code"`, `"Qty. per Unit of Measure"`, or a `(Base)` quantity field on a persisted or posted item journal, sales, purchase, or transfer line, or a line quantity compared with a base-unit inventory value | `derive-base-quantities-through-the-line-unit-of-measure` |
| `"Requisition Line"` action-message execution, accepted planning suggestions, `"Req. Wksh.-Make Order"`, `CarryOutBatchAction`, or linked supply creation/change plus requisition-line deletion | `carry-out-requisition-actions-through-the-standard-workflow` | | `"Requisition Line"` action-message execution, accepted planning suggestions, `"Req. Wksh.-Make Order"`, `CarryOutBatchAction`, or linked supply creation/change plus requisition-line deletion | `carry-out-requisition-actions-through-the-standard-workflow` |
Route clean supported calls through the same cues, not just suspicious writes. Route clean supported calls through the same cues, not just suspicious writes.

View file

@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review
- The changed AL object names and types — especially permission sets, codeunits handling authentication or authorization, objects touching `Isolated Storage`, `OAuth2` flows, web service endpoints, API pages, event publishers, and RecordRef helpers. - The changed AL object names and types — especially permission sets, codeunits handling authentication or authorization, objects touching `Isolated Storage`, `OAuth2` flows, web service endpoints, API pages, event publishers, and RecordRef helpers.
- The changed procedures and triggers, weighted toward those that call `HttpClient`, validate or compose URLs, write to telemetry, read or write secrets, unwrap SecretText, manipulate record-level security, expose var Boolean guard parameters, or bypass the permission model (for example, `RecordRef.Open`, `Record.WritePermission`, direct table access from a non-owning app). - The changed procedures and triggers, weighted toward those that call `HttpClient`, validate or compose URLs, write to telemetry, read or write secrets, unwrap SecretText, manipulate record-level security, expose var Boolean guard parameters, or bypass the permission model (for example, `RecordRef.Open`, `Record.WritePermission`, direct table access from a non-owning app).
- Tokens extracted from the diff that relate to security concerns (`IsolatedStorage`, `SetEncrypted`, `OAuth2`, `SecretText`, `Unwrap`, `NonDebuggable`, `Password`, `Token`, `HttpClient`, `Uri`, `AreURIsHaveSameHost`, `IsValidURIPattern`, `RecordRef`, `RecordId`, `TransferFields`, `Codeunit.Run`, `Access = Internal`, `internalsVisibleTo`, `Open`, `IntegrationEvent`, `SkipValidation`, `HasAccess`, `Permission`, `UserSecurityId`, `Commit`). - Tokens extracted from the diff that relate to security concerns (`IsolatedStorage`, `SetEncrypted`, `OAuth2`, `SecretText`, `Unwrap`, `NonDebuggable`, `Password`, `Token`, `HttpClient`, `Uri`, `AreURIsHaveSameHost`, `IsValidURIPattern`, `RecordRef`, `RecordId`, `TransferFields`, `Codeunit.Run`, `Access = Internal`, `internalsVisibleTo`, `Open`, `IntegrationEvent`, `SkipValidation`, `HasAccess`, `Permission`, `UserSecurityId`, `Commit`, `SetFilter`).
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.

View file

@ -40,8 +40,8 @@ Discard files that are not applicable. Retain conditionally applicable files onl
Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against: Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
- Changed AL objects — especially API pages (`PageType = API`), tables and pages declaring Labels/TextConsts, codeunits issuing `Error`/`Message`/`Confirm`, and any file whose name violates the `<ObjectName>.<ObjectType>.al` convention. - Changed AL objects — especially API pages (`PageType = API`), tables and pages declaring Labels/TextConsts, codeunits issuing `Error`/`Message`/`Confirm`, and any file whose name violates the `<ObjectName>.<ObjectType>.al` convention.
- Changed declarations, weighted toward `: Label '...'`, `: TextConst '...'`, temporary record variables, `DateFormula` declarations and their `Evaluate` call sites, error-handling call sites, and API declarations. - Changed declarations, weighted toward `: Label '...'`, `: TextConst '...'`, temporary record variables, option fields, `DateFormula` declarations and their `Evaluate` call sites, error-handling call sites, API declarations, and codeunit-internal method calls.
- Tokens extracted from the diff (`Label`, `TextConst`, `Locked`, `Comment`, `MaxLength`, `temporary`, `DateFormula`, `Evaluate`, `CalcDate`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `DelayedInsert`, `FieldCaption`, `TableCaption`, `FieldName`, `TableName`, `Page.RunModal`, `Report.Run`, `StrSubstNo`). - Tokens extracted from the diff (`Label`, `TextConst`, `Locked`, `Comment`, `MaxLength`, `temporary`, `DateFormula`, `Evaluate`, `CalcDate`, `OptionMembers`, `OptionCaption`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `DelayedInsert`, `FieldCaption`, `TableCaption`, `FieldName`, `TableName`, `Page.RunModal`, `Report.Run`, `this.`, `StrSubstNo`, `namespace`, `using`, `var `).
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object or declaration. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object or declaration. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
@ -51,6 +51,14 @@ Apply these high-signal mappings before fuzzy topic ranking:
- A `Label` or `TextConst` contains multiple or ambiguous placeholders but has no `Comment`, or its Comment does not explain every placeholder — `label-comment-explains-placeholders.md`. A single placeholder whose meaning is explicit in the text, such as `Customer %1`, is allowed without a Comment and must not be flagged. - A `Label` or `TextConst` contains multiple or ambiguous placeholders but has no `Comment`, or its Comment does not explain every placeholder — `label-comment-explains-placeholders.md`. A single placeholder whose meaning is explicit in the text, such as `Customer %1`, is allowed without a Comment and must not be flagged.
- A normal two-argument `Evaluate` has a resolved `DateFormula` destination and a hard-coded non-angle-bracket date-formula literal, directly or through a visible constant — `dateformula-evaluate-needs-language-independent-literals.md`. Do not use this cue for dynamic/localized external input, already invariant `<...>` input, or direct `CalcDate(Text, ...)` calls. - A normal two-argument `Evaluate` has a resolved `DateFormula` destination and a hard-coded non-angle-bracket date-formula literal, directly or through a visible constant — `dateformula-evaluate-needs-language-independent-literals.md`. Do not use this cue for dynamic/localized external input, already invariant `<...>` input, or direct `CalcDate(Text, ...)` calls.
- `function-call-parentheses-required.md` applies only to a zero-argument invocation written without `()`. Never worklist it from an invocation that already has parentheses or supplies arguments, including `Error(Label, Arg1, Arg2)`.
- A new `.app` build artifact appears at the project root or another unversioned/arbitrary location, or is added to source control alongside the AL source that produced it — `al-build-output-must-not-pollute-project-root.md`. A deliberate `--outfolder`/`outputPath` destination added to `.gitignore` is the compliant shape, not the signal to flag.
- A new or renamed AL identifier (variable, procedure, parameter, field, object, enum value, or label identifier) contains non-English words — `al-identifiers-english.md`. A caption, tooltip, or other user-facing text value in a non-English language is not this anti-pattern; only the identifier itself is in scope.
- A field or variable is typed `Boolean` and its two possible values are genuinely named domain alternatives (a status pair like Inbound/Outbound, Debit/Credit, Buy/Sell) rather than a true/false predicate, or an `Option`/`Enum`/`Integer` models a domain concept that is intrinsically a yes/no flag — `binary-choice-must-be-boolean.md`. The signal is a semantic mismatch between the type and the domain concept, not the current number of states.
- A field or variable is typed `Integer` with the meaning of each value tracked only in a comment, or an `Enum` with more than two members is proposed as `Boolean`-like — `fixed-choice-set-must-use-enum-not-integer.md`. Do not flag a genuinely two-state `Enum`/`Option` for having "too few" members; that overlaps `binary-choice-must-be-boolean.md` instead when the domain is a true/false predicate.
- A new `using` directive is added for an existing AL object without the diff also showing that object's own `namespace` declaration or a symbol-package lookup backing the choice — `namespace-must-be-verified-from-source.md`. Require repository/dependency context; a single new `using` line cannot itself prove whether the namespace was verified or guessed.
- An intrinsic/built-in AL function call (`MESSAGE`, `ERROR`, `CONFIRM`, `STRSUBSTNO`, etc.) is written in ALL-CAPS or another non-PascalCase form — `intrinsic-al-functions-must-use-modern-casing.md`.
- A procedure call passes a literal or a computed expression (not a caller-scope variable) to a parameter position the callee declares `var` — `var-parameters-require-an-addressable-variable.md`. This is a compile-time-guaranteed shape; flag it only when the callee's declared signature is visible in the diff or resolvable from context.
Once the candidate worklist is known, resolve layer-precedence conflicts per READ and record suppressions. Once the candidate worklist is known, resolve layer-precedence conflicts per READ and record suppressions.

Some files were not shown because too many files have changed in this diff Show more