mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 17:36:53 +01:00
Compare commits
No commits in common. "main" and "v1.0" have entirely different histories.
306 changed files with 1307 additions and 5466 deletions
|
|
@ -9,10 +9,7 @@
|
||||||
"name": "bcquality",
|
"name": "bcquality",
|
||||||
"source": "./",
|
"source": "./",
|
||||||
"description": "Business Central AL quality knowledge base and review skills, packaged as an installable plugin. Ships the entire BCQuality tree (skills, knowledge, tools) so the Entry routing protocol runs against the installed clone.",
|
"description": "Business Central AL quality knowledge base and review skills, packaged as an installable plugin. Ships the entire BCQuality tree (skills, knowledge, tools) so the Entry routing protocol runs against the installed clone.",
|
||||||
"version": "0.1.0",
|
"version": "0.1.0"
|
||||||
"skills": [
|
|
||||||
"./skills/bcquality-al-review/"
|
|
||||||
]
|
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -5,17 +5,5 @@
|
||||||
"author": {
|
"author": {
|
||||||
"name": "microsoft/BCQuality",
|
"name": "microsoft/BCQuality",
|
||||||
"url": "https://github.com/microsoft/BCQuality"
|
"url": "https://github.com/microsoft/BCQuality"
|
||||||
},
|
}
|
||||||
"repository": "https://github.com/microsoft/BCQuality",
|
|
||||||
"license": "MIT",
|
|
||||||
"keywords": [
|
|
||||||
"bc",
|
|
||||||
"al",
|
|
||||||
"business-central",
|
|
||||||
"code-review",
|
|
||||||
"quality"
|
|
||||||
],
|
|
||||||
"skills": [
|
|
||||||
"./skills/bcquality-al-review/"
|
|
||||||
]
|
|
||||||
}
|
}
|
||||||
34
.github/scripts/validate_frontmatter.py
vendored
34
.github/scripts/validate_frontmatter.py
vendored
|
|
@ -62,7 +62,6 @@ ISO_ALPHA2 = re.compile(r"^[a-z]{2}$")
|
||||||
RANGE_SHORTHAND = re.compile(r"^(\d+)\.\.(\d+)?$")
|
RANGE_SHORTHAND = re.compile(r"^(\d+)\.\.(\d+)?$")
|
||||||
FENCED_CODE_BLOCK = re.compile(r"^```", re.MULTILINE)
|
FENCED_CODE_BLOCK = re.compile(r"^```", re.MULTILINE)
|
||||||
HEADING_H2 = re.compile(r"^##\s+(.+?)\s*$", re.MULTILINE)
|
HEADING_H2 = re.compile(r"^##\s+(.+?)\s*$", re.MULTILINE)
|
||||||
SAMPLE_REFERENCE = re.compile(r"`([a-z0-9]+(?:-[a-z0-9]+)*\.(?:good|bad)\.[a-z0-9]+)`")
|
|
||||||
|
|
||||||
|
|
||||||
# --- Diagnostics ------------------------------------------------------------
|
# --- Diagnostics ------------------------------------------------------------
|
||||||
|
|
@ -223,13 +222,6 @@ def validate_knowledge(path: Path, parsed: Parsed, report: Report) -> None:
|
||||||
if "domain" in fm:
|
if "domain" in fm:
|
||||||
if not isinstance(fm["domain"], str) or not fm["domain"].strip():
|
if not isinstance(fm["domain"], str) or not fm["domain"].strip():
|
||||||
report.error(path, "R04", "domain must be a non-empty string", 1)
|
report.error(path, "R04", "domain must be a non-empty string", 1)
|
||||||
elif fm["domain"] != path.parent.name:
|
|
||||||
report.error(
|
|
||||||
path,
|
|
||||||
"R27",
|
|
||||||
f"frontmatter domain '{fm['domain']}' must match directory '{path.parent.name}'",
|
|
||||||
1,
|
|
||||||
)
|
|
||||||
|
|
||||||
# R05 keywords
|
# R05 keywords
|
||||||
if "keywords" in fm:
|
if "keywords" in fm:
|
||||||
|
|
@ -485,16 +477,7 @@ def validate_samples_in_domain(domain_dir: Path, root: Path, report: Report) ->
|
||||||
"""R14: every non-.md file must match <slug>.<kind>.<ext> with <slug>.md present."""
|
"""R14: every non-.md file must match <slug>.<kind>.<ext> with <slug>.md present."""
|
||||||
if not domain_dir.is_dir():
|
if not domain_dir.is_dir():
|
||||||
return
|
return
|
||||||
articles = {p.stem: p for p in domain_dir.glob("*.md")}
|
article_slugs = {p.stem for p in domain_dir.glob("*.md")}
|
||||||
article_slugs = set(articles)
|
|
||||||
article_texts: dict[str, str] = {}
|
|
||||||
for slug, article in articles.items():
|
|
||||||
try:
|
|
||||||
article_texts[slug] = article.read_text(encoding="utf-8")
|
|
||||||
except UnicodeDecodeError:
|
|
||||||
# R01 reports this during the article pass.
|
|
||||||
continue
|
|
||||||
|
|
||||||
for entry in domain_dir.iterdir():
|
for entry in domain_dir.iterdir():
|
||||||
if not entry.is_file() or entry.suffix == ".md":
|
if not entry.is_file() or entry.suffix == ".md":
|
||||||
continue
|
continue
|
||||||
|
|
@ -508,24 +491,9 @@ def validate_samples_in_domain(domain_dir: Path, root: Path, report: Report) ->
|
||||||
kind = m.group("kind")
|
kind = m.group("kind")
|
||||||
if slug not in article_slugs:
|
if slug not in article_slugs:
|
||||||
report.error(entry, "R14", f"orphan sample: no matching article '{slug}.md' in {domain_dir.relative_to(root).as_posix()}")
|
report.error(entry, "R14", f"orphan sample: no matching article '{slug}.md' in {domain_dir.relative_to(root).as_posix()}")
|
||||||
elif entry.name not in article_texts.get(slug, ""):
|
|
||||||
report.error(
|
|
||||||
entry,
|
|
||||||
"R28",
|
|
||||||
f"sample is not referenced by its article '{slug}.md'",
|
|
||||||
)
|
|
||||||
if kind not in VALID_SAMPLE_KINDS:
|
if kind not in VALID_SAMPLE_KINDS:
|
||||||
report.warn(entry, "R14", f"non-standard sample kind '{kind}'; standard kinds are {sorted(VALID_SAMPLE_KINDS)}")
|
report.warn(entry, "R14", f"non-standard sample kind '{kind}'; standard kinds are {sorted(VALID_SAMPLE_KINDS)}")
|
||||||
|
|
||||||
for slug, article in articles.items():
|
|
||||||
for sample_name in SAMPLE_REFERENCE.findall(article_texts.get(slug, "")):
|
|
||||||
if not (domain_dir / sample_name).is_file():
|
|
||||||
report.error(
|
|
||||||
article,
|
|
||||||
"R28",
|
|
||||||
f"referenced sample does not exist: '{sample_name}'",
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
# --- Orchestration ----------------------------------------------------------
|
# --- Orchestration ----------------------------------------------------------
|
||||||
|
|
||||||
|
|
|
||||||
18
.github/workflows/review-fixtures.yml
vendored
18
.github/workflows/review-fixtures.yml
vendored
|
|
@ -1,18 +0,0 @@
|
||||||
name: Validate AL review fixtures
|
|
||||||
|
|
||||||
on:
|
|
||||||
pull_request:
|
|
||||||
branches: [main]
|
|
||||||
push:
|
|
||||||
branches: [main]
|
|
||||||
|
|
||||||
jobs:
|
|
||||||
validate-review-fixtures:
|
|
||||||
runs-on: ubuntu-latest
|
|
||||||
steps:
|
|
||||||
- name: Check out repository
|
|
||||||
uses: actions/checkout@v4
|
|
||||||
|
|
||||||
- name: Validate review evaluation corpus
|
|
||||||
shell: pwsh
|
|
||||||
run: ./tools/Test-ReviewFixtures.ps1 -Root . -PrepareDirectory "$env:RUNNER_TEMP/bcquality-review-fixtures"
|
|
||||||
19
README.md
19
README.md
|
|
@ -18,8 +18,6 @@ Poor fit: "Use HTTPS instead of HTTP." "Don't hardcode secrets." "Keep transacti
|
||||||
|
|
||||||
The practical consequence: when a code-review agent flags something it shouldn't have, or misses something it should have caught, the remedy is a new knowledge file. When it already behaves correctly on a topic, no file is needed.
|
The practical consequence: when a code-review agent flags something it shouldn't have, or misses something it should have caught, the remedy is a new knowledge file. When it already behaves correctly on a topic, no file is needed.
|
||||||
|
|
||||||
A file that *prevents* a false positive — documenting why a pattern is legitimate so the agent stops flagging it — is as valid as one that catches a defect: negative clarifications are first-class knowledge files. What never belongs is a BC fact hard-coded into a skill. Skills are finders and appliers; knowledge files are what the agent knows. See [`skills/do.md`](skills/do.md) and [`skills/write.md`](skills/write.md).
|
|
||||||
|
|
||||||
## What's in this repo
|
## What's in this repo
|
||||||
|
|
||||||
BCQuality contains **knowledge** and **skills**. It does not contain agents. Agents that consume BCQuality ship with [AL-Go](https://github.com/microsoft/AL-Go) and other orchestrators.
|
BCQuality contains **knowledge** and **skills**. It does not contain agents. Agents that consume BCQuality ship with [AL-Go](https://github.com/microsoft/AL-Go) and other orchestrators.
|
||||||
|
|
@ -90,9 +88,18 @@ Code examples belong in separate files, not in the knowledge file itself. Knowle
|
||||||
|
|
||||||
## Scope
|
## Scope
|
||||||
|
|
||||||
The current curated corpus is focused on **technical AL code review**: AppSource and compatibility, data modeling, error handling, events, interfaces, performance, privacy, Query objects, security, style, telemetry, testing, UI, upgrade, and web services. These are the domains backed by knowledge files and registered review leaves today.
|
BCQuality covers Business Central broadly — the application domains it supports, the technologies used to extend it, and the practices that keep implementations healthy. The scope includes:
|
||||||
|
|
||||||
Business Central functional domains (Finance, Supply Chain Management, Manufacturing, Jobs, Warehousing, Service), PowerShell, pipelines, and Power Platform remain valid future repository scope, but they are **not current coverage claims** until corresponding knowledge and action skills exist. Consumers should derive supported review scope from the live knowledge index and dispatched skills, not from roadmap breadth.
|
- **Business Central domains** — Finance, Supply Chain Management, Manufacturing, Jobs, Warehousing, Service, and the many other functional areas BC covers. Domain knowledge helps agents understand the business context they are working in.
|
||||||
|
- AL language patterns and anti-patterns
|
||||||
|
- PowerShell scripting for BC
|
||||||
|
- Pipelines (AL-Go, GitHub Actions)
|
||||||
|
- Business Central APIs
|
||||||
|
- Power Platform integration
|
||||||
|
- Telemetry and KQL
|
||||||
|
- AppSource lifecycle
|
||||||
|
|
||||||
|
A BC developer's actual job spans all of this, and BCQuality reflects that.
|
||||||
|
|
||||||
## How agents consume BCQuality
|
## How agents consume BCQuality
|
||||||
|
|
||||||
|
|
@ -115,7 +122,6 @@ For the end-to-end flow — from orchestrator trigger through to how output reac
|
||||||
|
|
||||||
```
|
```
|
||||||
├── /skills/ # Global: entry-point skill + meta-skill contracts (READ, DO, WRITE)
|
├── /skills/ # Global: entry-point skill + meta-skill contracts (READ, DO, WRITE)
|
||||||
├── /evaluation/ # Neutral good/bad review fixtures and scoring contract
|
|
||||||
├── /.github/ # Actions and workflows
|
├── /.github/ # Actions and workflows
|
||||||
├── /microsoft/ # Microsoft-endorsed layer
|
├── /microsoft/ # Microsoft-endorsed layer
|
||||||
│ ├── /knowledge/ # Knowledge files by domain
|
│ ├── /knowledge/ # Knowledge files by domain
|
||||||
|
|
@ -149,12 +155,9 @@ Contributions are welcome. Before submitting a PR:
|
||||||
1. Read the knowledge file format above — frontmatter and sections are validated by CI.
|
1. Read the knowledge file format above — frontmatter and sections are validated by CI.
|
||||||
2. Keep files atomic: one concern per file, under 100 lines.
|
2. Keep files atomic: one concern per file, under 100 lines.
|
||||||
3. Target your contribution to the right layer — most community contributions go in `/community/knowledge/`.
|
3. Target your contribution to the right layer — most community contributions go in `/community/knowledge/`.
|
||||||
4. Adding a BC fact — or stopping the agent from flagging a false positive — is a knowledge file, not a skill edit. If a PR changes *what* a review skill flags, the change almost certainly belongs in a knowledge file. See [`skills/write.md`](skills/write.md).
|
|
||||||
|
|
||||||
CI runs validation on every PR. If your knowledge file has schema violations, missing sections, code blocks, or exceeds 100 lines, the check will fail with a clear error message.
|
CI runs validation on every PR. If your knowledge file has schema violations, missing sections, code blocks, or exceeds 100 lines, the check will fail with a clear error message.
|
||||||
|
|
||||||
Companion samples must be referenced by filename from their article, and every referenced sample must exist. The review evaluation corpus under [`evaluation/`](evaluation/) adds one positive and one clean control for every registered AL review leaf; see [`evaluation/README.md`](evaluation/README.md) for credential-free validation and optional fast-model scoring.
|
|
||||||
|
|
||||||
## License
|
## License
|
||||||
|
|
||||||
[MIT](LICENSE)
|
[MIT](LICENSE)
|
||||||
|
|
|
||||||
|
|
@ -21,7 +21,7 @@ flowchart LR
|
||||||
E -->|3 dispatch record| A
|
E -->|3 dispatch record| A
|
||||||
A -->|4 invoke dispatched skill| S[Action skill<br/>e.g. al-code-review]
|
A -->|4 invoke dispatched skill| S[Action skill<br/>e.g. al-code-review]
|
||||||
S -->|5 execute| P[Source → Relevance<br/>→ Worklist → Action<br/>reading READ · DO on demand]
|
S -->|5 execute| P[Source → Relevance<br/>→ Worklist → Action<br/>reading READ · DO on demand]
|
||||||
P -->|6 emit| R[Findings · Domain labels<br/>· References · Confidence]
|
P -->|6 emit| R[Findings · References<br/>· Confidence]
|
||||||
R -->|7 integrate| O
|
R -->|7 integrate| O
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
@ -65,7 +65,6 @@ The output contract is defined in the DO meta-skill so that every action skill
|
||||||
|
|
||||||
- **Outcome** — `completed`, `not-applicable`, `no-knowledge`, `partial`, or `failed`. An orchestrator can distinguish a clean run from a no-op from a failure without guessing.
|
- **Outcome** — `completed`, `not-applicable`, `no-knowledge`, `partial`, or `failed`. An orchestrator can distinguish a clean run from a no-op from a failure without guessing.
|
||||||
- **Findings** — what the skill observed (severity, message, optional location).
|
- **Findings** — what the skill observed (severity, message, optional location).
|
||||||
- **Domain** — the producer-owned, human-readable display label on each review finding.
|
|
||||||
- **References** — structured objects (`path` plus optional commit `sha`) pointing to the knowledge files that informed each finding.
|
- **References** — structured objects (`path` plus optional commit `sha`) pointing to the knowledge files that informed each finding.
|
||||||
- **Confidence** — per-finding evidence strength.
|
- **Confidence** — per-finding evidence strength.
|
||||||
- **Suppressed** — knowledge files that were discarded by layer precedence or configuration, so reviewers can see what was overridden.
|
- **Suppressed** — knowledge files that were discarded by layer precedence or configuration, so reviewers can see what was overridden.
|
||||||
|
|
@ -79,12 +78,12 @@ The orchestrator turns findings into PR comments, build gates, or IDE diagnostic
|
||||||
|
|
||||||
BCQuality is an **additive** knowledge layer. The agent surfaces two kinds of findings, both shaped to the same DO output contract:
|
BCQuality is an **additive** knowledge layer. The agent surfaces two kinds of findings, both shaped to the same DO output contract:
|
||||||
|
|
||||||
- **Knowledge-backed findings** carry one or more entries in `references[]` pointing at BCQuality knowledge files. Their `id` is the primary file's repo-relative path. Leaf sub-skills set `domain` to their human-readable display label, and super-skills preserve it verbatim during rollup.
|
- **Knowledge-backed findings** carry one or more entries in `references[]` pointing at BCQuality knowledge files. Their `id` is the primary file's repo-relative path. These are produced by leaf sub-skills and rolled up by super-skills.
|
||||||
- **Agent findings** carry an empty `references: []`, use a slug `id` prefixed `agent:`, and have `confidence` capped at `medium`. A leaf can emit one strictly within its own domain and uses that leaf's display label. A super-skill can emit a cross-cutting agent finding with `from-sub-skill: "agent"` and `domain: "Agent"`. Their `message` is self-contained because there is no knowledge-file footer to fall back on.
|
- **Agent findings** are surfaced by a super-skill from its own self-review pass when no BCQuality knowledge file backs the concern. They are tagged with `from-sub-skill: "agent"`, carry an empty `references: []`, use a slug `id` prefixed `agent:`, and have `confidence` capped at `medium`. Their `message` is self-contained because there is no knowledge-file footer to fall back on.
|
||||||
|
|
||||||
Before a skill emits an agent finding, it validates the candidate against the BCQuality knowledge already loaded for the task: a matching file upgrades the candidate to a knowledge-backed finding (and merges or deduplicates against relevant existing output); a contradicting file suppresses the candidate. Only candidates with no BCQuality coverage become agent findings.
|
Before a super-skill emits an agent finding, it validates the candidate against the BCQuality knowledge already loaded for the task: a matching file upgrades the candidate to a knowledge-backed finding (and merges or deduplicates against the relevant sub-skill output); a contradicting file suppresses the candidate. Only candidates with no BCQuality coverage become agent findings.
|
||||||
|
|
||||||
Orchestrators MUST tolerate an absent `domain` in reports from older producers. When it is present, treat it as display text rather than an identifier: preserve the full string and its case, whitespace, punctuation, and non-ASCII characters, escaping only for the target rendering format. Do not tokenize it on spaces or use a lowercased or slugified form as the sole metadata or deduplication key, because distinct labels can collapse to the same slug. Retain the exact string, use a lossless encoding, or use a collision-resistant digest instead. Orchestrators MAY render knowledge-backed and agent findings differently and MAY apply independent severity floors; `references: []` and the `agent:` id prefix distinguish agent findings, while `from-sub-skill: "agent"` identifies those emitted by the super-skill itself.
|
Orchestrators MAY render the two kinds differently — for example, by labelling agent findings or routing them to a separate review domain — and MAY apply independent severity floors. The `from-sub-skill: "agent"` marker is the contract.
|
||||||
|
|
||||||
## Why this architecture
|
## Why this architecture
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
---
|
---
|
||||||
bc-version: [27..]
|
bc-version: [24..]
|
||||||
domain: appsource
|
domain: appsource
|
||||||
keywords: [app-json, help-url, copilot, grounding, documentation, url-depth, contexturl]
|
keywords: [app-json, help-url, copilot, grounding, documentation, url-depth, contexturl]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
|
|
@ -9,6 +9,8 @@ application-area: [all]
|
||||||
|
|
||||||
# Keep the Copilot help URL to two path levels
|
# Keep the Copilot help URL to two path levels
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
The `help` URL declared in `app.json` is what Copilot uses to ground answers about your app. That URL may be at most **two path levels** deep (for example `https://contoso.com/docs/myapp`). If you point it at a deeper path (three or more segments), Copilot does not use the URL as given: it truncates to the first two levels, drops any fragments and query strings, and then grounds on **all** content beneath that two-level path. The failure is silent — there is no build error — and the practical effect is worse answers, because Copilot may ingest sibling apps' documentation that lives under the same two-level parent.
|
The `help` URL declared in `app.json` is what Copilot uses to ground answers about your app. That URL may be at most **two path levels** deep (for example `https://contoso.com/docs/myapp`). If you point it at a deeper path (three or more segments), Copilot does not use the URL as given: it truncates to the first two levels, drops any fragments and query strings, and then grounds on **all** content beneath that two-level path. The failure is silent — there is no build error — and the practical effect is worse answers, because Copilot may ingest sibling apps' documentation that lives under the same two-level parent.
|
||||||
|
|
@ -9,7 +9,7 @@ table 50120 "FieldError Default Bad"
|
||||||
|
|
||||||
procedure ValidateForRelease()
|
procedure ValidateForRelease()
|
||||||
begin
|
begin
|
||||||
// This re-tests a field and gives FieldError a fully formed sentence.
|
// Re-testing a field and handing FieldError a fully-formed sentence.
|
||||||
// The framework already prepends the caption and appends the value,
|
// The framework already prepends the caption and appends the value,
|
||||||
// so this renders as "Currency Code The Currency Code field must have
|
// so this renders as "Currency Code The Currency Code field must have
|
||||||
// a value. in ..." — caption repeated, capital letter mid-sentence,
|
// a value. in ..." — caption repeated, capital letter mid-sentence,
|
||||||
|
|
@ -9,8 +9,9 @@ table 50120 "FieldError Default Good"
|
||||||
|
|
||||||
procedure ValidateForRelease()
|
procedure ValidateForRelease()
|
||||||
begin
|
begin
|
||||||
// TestField checks this required-field condition and raises the error
|
// Plain required-field gate: TestField checks the condition and raises
|
||||||
// with caption and record context supplied by the framework.
|
// the error in one call, with caption and record context supplied by
|
||||||
|
// the framework.
|
||||||
TestField("Currency Code");
|
TestField("Currency Code");
|
||||||
|
|
||||||
// Condition already evaluated: pass only a lowercase predicate so it
|
// Condition already evaluated: pass only a lowercase predicate so it
|
||||||
|
|
@ -8,15 +8,13 @@ application-area: [all]
|
||||||
---
|
---
|
||||||
# Rely On FieldError's Auto-Generated Context And Pass Only A Lowercase Predicate
|
# Rely On FieldError's Auto-Generated Context And Pass Only A Lowercase Predicate
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
`Rec.FieldError(FieldNo)` does not just print the text you give it. Business Central automatically prepends the field caption, appends the current field value (when non-blank), and suffixes the table name and primary-key values for record identification. The optional second argument is only the middle predicate of that sentence — e.g. `"must be unique"`, not a whole self-contained message. Misunderstanding this leads to messages that duplicate the caption and value or read as broken grammar, because the framework's surrounding text is built to join a lowercase fragment.
|
`Rec.FieldError(FieldNo)` does not just print the text you give it. Business Central automatically prepends the field caption, appends the current field value (when non-blank), and suffixes the table name and primary-key values for record identification. The optional second argument is only the middle predicate of that sentence — e.g. `"must be unique"`, not a whole self-contained message. Misunderstanding this leads to messages that duplicate the caption and value or read as broken grammar, because the framework's surrounding text is built to join a lowercase fragment.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
For a plain required-field check, prefer `TestField`, which tests the condition and raises the error in one call. When the condition is non-trivial and has already been evaluated, call `FieldError(FieldNo)` with no message to get the localized default (`must have a value`, `is not valid`, etc.), or pass a short lowercase predicate such as `FieldError(FieldNo, 'must be a positive number')`. Start the custom text with a lowercase letter so it reads as one sentence with the auto-inserted caption, and use a field-number reference (or the field token) rather than a hard-coded field name so captions and translations stay correct. Let the framework supply the caption, value, table, and key context for you.
|
For a plain required-field check, prefer `TestField`, which tests the condition and raises the error in one call. When the condition is non-trivial and has already been evaluated, call `FieldError(FieldNo)` with no message to get the localized default (`must have a value`, `is not valid`, etc.), or pass a short lowercase predicate such as `FieldError(FieldNo, 'must be a positive number')`. Start the custom text with a lowercase letter so it reads as one sentence with the auto-inserted caption, and use a field-number reference (or the field token) rather than a hard-coded field name so captions and translations stay correct. Let the framework supply the caption, value, table, and key context for you.
|
||||||
|
|
||||||
See sample: `fielderror-default-message-logic.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
Re-testing a condition you already evaluated, or passing a fully formed sentence like `'The Amount field must be positive.'` to `FieldError`. The result reads as `Amount The Amount field must be positive. in Gen. Journal Line ...` — capital letter mid-sentence, caption and value repeated, and a stray trailing clause. Reviewer signals: a `FieldError` argument that names the field, restates the current value, starts with a capital letter, or ends with a period. Each is a sign the author treated `FieldError` like `Error` instead of as a predicate slotted into framework-generated context.
|
Re-testing a condition you already evaluated, or passing a fully formed sentence like `'The Amount field must be positive.'` to `FieldError`. The result reads as `Amount The Amount field must be positive. in Gen. Journal Line ...` — capital letter mid-sentence, caption and value repeated, and a stray trailing clause. Reviewer signals: a `FieldError` argument that names the field, restates the current value, starts with a capital letter, or ends with a period. Each is a sign the author treated `FieldError` like `Error` instead of as a predicate slotted into framework-generated context.
|
||||||
|
|
||||||
See sample: `fielderror-default-message-logic.bad.al`.
|
|
||||||
|
|
@ -9,7 +9,7 @@ table 50122 "FieldError vs TestField Bad"
|
||||||
|
|
||||||
procedure PostDocument()
|
procedure PostDocument()
|
||||||
begin
|
begin
|
||||||
// FieldError performs no comparison and raises as soon as it is
|
// FieldError performs no comparison and always raises the moment it is
|
||||||
// reached, so this "check" terminates PostDocument every time — the
|
// reached, so this "check" terminates PostDocument every time — the
|
||||||
// Posting Date is never actually tested, and the amount rule below is
|
// Posting Date is never actually tested, and the amount rule below is
|
||||||
// dead code.
|
// dead code.
|
||||||
|
|
@ -9,8 +9,8 @@ table 50122 "FieldError vs TestField Good"
|
||||||
|
|
||||||
procedure PostDocument()
|
procedure PostDocument()
|
||||||
begin
|
begin
|
||||||
// TestField performs this simple presence check and raises only when
|
// Simple presence gate: TestField performs the check itself and raises
|
||||||
// the field is empty. Self-documenting prerequisite.
|
// only when the field is empty. Self-documenting prerequisite.
|
||||||
TestField("Posting Date");
|
TestField("Posting Date");
|
||||||
|
|
||||||
// Business logic has already determined the value is invalid;
|
// Business logic has already determined the value is invalid;
|
||||||
|
|
@ -8,15 +8,13 @@ application-area: [all]
|
||||||
---
|
---
|
||||||
# Choose `TestField` For Conditional Checks And `FieldError` For Already-Failed Validation
|
# Choose `TestField` For Conditional Checks And `FieldError` For Already-Failed Validation
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
`TestField` and `FieldError` look interchangeable but behave differently, and choosing the wrong one produces either dead code or a check that never fires. `TestField` performs the comparison itself and throws only when the field is empty or does not match the supplied value; `FieldError` performs no comparison and always raises an error the moment it is reached. Both attach the field caption and the record's primary-key context to the message automatically, which is why neither should be replaced by a hand-built `Error` call that interpolates the field name as a literal.
|
`TestField` and `FieldError` look interchangeable but behave differently, and choosing the wrong one produces either dead code or a check that never fires. `TestField` performs the comparison itself and throws only when the field is empty or does not match the supplied value; `FieldError` performs no comparison and always raises an error the moment it is reached. Both attach the field caption and the record's primary-key context to the message automatically, which is why neither should be replaced by a hand-built `Error` call that interpolates the field name as a literal.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
Use `TestField` when the condition is a simple presence-or-equality check on a single field — mandatory-field gates and prerequisite checks at the top of a procedure read clearly and self-document intent. Use `FieldError` inside an `OnValidate` trigger or a validation procedure where surrounding business logic has already determined the value is invalid and you want a specific, custom message. Rely on the built-in field-and-record context both methods add rather than re-stating the field name in the text.
|
Use `TestField` when the condition is a simple presence-or-equality check on a single field — mandatory-field gates and prerequisite checks at the top of a procedure read clearly and self-document intent. Use `FieldError` inside an `OnValidate` trigger or a validation procedure where surrounding business logic has already determined the value is invalid and you want a specific, custom message. Rely on the built-in field-and-record context both methods add rather than re-stating the field name in the text.
|
||||||
|
|
||||||
See sample: `fielderror-vs-testfield.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
Calling `FieldError` to "test" a field — placing it on a path that is reached unconditionally and expecting it to validate — terminates execution every time because `FieldError` never evaluates a condition. The inverse smell is reaching for `TestField` when the rule needs a tailored message, then bolting a vague generic string onto a check that cannot express the real business reason. A reviewer can spot the first by a `FieldError` that is not guarded by a preceding `if`, and the second by a `TestField` whose intent comment describes a condition more complex than presence or equality.
|
Calling `FieldError` to "test" a field — placing it on a path that is reached unconditionally and expecting it to validate — terminates execution every time because `FieldError` never evaluates a condition. The inverse smell is reaching for `TestField` when the rule needs a tailored message, then bolting a vague generic string onto a check that cannot express the real business reason. A reviewer can spot the first by a `FieldError` that is not guarded by a preceding `if`, and the second by a `TestField` whose intent comment describes a condition more complex than presence or equality.
|
||||||
|
|
||||||
See sample: `fielderror-vs-testfield.bad.al`.
|
|
||||||
|
|
@ -0,0 +1,13 @@
|
||||||
|
codeunit 50124 "Sales Line Guard Bad Sample"
|
||||||
|
{
|
||||||
|
// A throw here executes synchronously inside the transaction of the write
|
||||||
|
// that fired the event. With no per-record savepoint, it rolls back ALL
|
||||||
|
// uncommitted work since the last COMMIT — the entire batch, not just this
|
||||||
|
// line. One bad row discards every row imported before it.
|
||||||
|
[EventSubscriber(ObjectType::Table, Database::"Sales Line", 'OnAfterInsertEvent', '', false, false)]
|
||||||
|
local procedure OnAfterInsertSalesLine(var Rec: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
if Rec.Quantity <= 0 then
|
||||||
|
Rec.FieldError(Quantity, 'must be greater than zero');
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,33 @@
|
||||||
|
codeunit 50124 "Batch Import Good Sample"
|
||||||
|
{
|
||||||
|
procedure ImportAll(var StagingLine: Record "Sales Line")
|
||||||
|
var
|
||||||
|
FailedCount: Integer;
|
||||||
|
begin
|
||||||
|
if StagingLine.FindSet() then
|
||||||
|
repeat
|
||||||
|
// Isolate each record behind a Codeunit.Run boundary: a failure
|
||||||
|
// inside the run rolls back only that record's work, and the
|
||||||
|
// batch continues instead of discarding everything.
|
||||||
|
if not Codeunit.Run(Codeunit::"Batch Import One Line", StagingLine) then
|
||||||
|
FailedCount += 1;
|
||||||
|
until StagingLine.Next() = 0;
|
||||||
|
|
||||||
|
if FailedCount > 0 then
|
||||||
|
Message('%1 line(s) were skipped; the rest were imported.', FailedCount);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50125 "Batch Import One Line"
|
||||||
|
{
|
||||||
|
TableNo = "Sales Line";
|
||||||
|
|
||||||
|
trigger OnRun()
|
||||||
|
begin
|
||||||
|
// Validation lives here. If it throws, only this line rolls back,
|
||||||
|
// because the caller wrapped the call in Codeunit.Run.
|
||||||
|
Rec.TestField("No.");
|
||||||
|
Rec.TestField(Quantity);
|
||||||
|
Rec.Insert(true);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,24 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: error-handling
|
||||||
|
keywords: [table-events, oninsert, onmodify, ondelete, transaction, rollback, commit, batch, subscriber]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# A throw in a table-event subscriber rolls back the whole batch
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Table-trigger event subscribers (`OnAfterInsertEvent`, `OnAfterModifyEvent`, `OnAfterDeleteEvent`, and their `OnBefore` counterparts) execute synchronously inside the transaction of the write that fired them. Because AL runs on a single implicit transaction with no per-record savepoint, an error raised in such a subscriber rolls back **all work since the last `COMMIT`** — not just the record that triggered it. In a batch loop with no intermediate `COMMIT`s, a single failing record discards the entire batch. The intuition that subscriber validation fails only the current record is wrong on the BC platform.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Decide the failure granularity deliberately. If a batch must continue past individual failures, do not throw from the table-event subscriber — collect the error (for example via `ErrorInfo`/collectible errors) and let the loop continue, or isolate each record's work behind a `Codeunit.Run` / `if Codeunit.Run() then` boundary so its failure rolls back only that record. Insert intermediate `COMMIT`s only with full awareness of the durability trade-off.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Putting `Error`/`TestField`/`FieldError` validation inside a table-event subscriber and assuming it rejects just the offending record during bulk processing. The first failure unwinds every uncommitted record in the run, turning a one-row data problem into a whole-batch rollback.
|
||||||
|
|
@ -0,0 +1,11 @@
|
||||||
|
codeunit 50130 "Purge Orders Bad Sample"
|
||||||
|
{
|
||||||
|
procedure PurgeCancelledLines(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
// Assumes DeleteAll fires OnDelete and cascades to reservation entries
|
||||||
|
// and item applications. It does not: parameterless DeleteAll() is
|
||||||
|
// DeleteAll(false) and skips OnDelete, so the rows vanish but their
|
||||||
|
// dependent records are orphaned.
|
||||||
|
SalesLine.DeleteAll();
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,16 @@
|
||||||
|
codeunit 50130 "Purge Orders Good Sample"
|
||||||
|
{
|
||||||
|
procedure PurgeCancelledLines(var SalesLine: Record "Sales Line")
|
||||||
|
begin
|
||||||
|
// These lines have OnDelete cleanup (reservation entries, item
|
||||||
|
// application). Pass true so DeleteAll runs OnDelete per record and the
|
||||||
|
// cleanup actually happens — the row-by-row cost is accepted on purpose.
|
||||||
|
SalesLine.DeleteAll(true);
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure PurgeStagingBuffer(var TempBuffer: Record "Name/Value Buffer" temporary)
|
||||||
|
begin
|
||||||
|
// No OnDelete logic to run: the fast, set-based form is correct here.
|
||||||
|
TempBuffer.DeleteAll();
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,24 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: performance
|
||||||
|
keywords: [deleteall, ondelete, run-trigger, set-based-delete, bulk-delete, triggers, validation]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# DeleteAll skips OnDelete unless you pass RunTrigger
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
`Record.DeleteAll()` — equivalently `DeleteAll(false)` — translates to a single set-based SQL `DELETE` and **does not** run AL `OnDelete` triggers or field/table validations. Only database-level referential constraints still apply. To run `OnDelete` logic you must call `DeleteAll(true)`, which then deletes record-by-record and forfeits the set-based performance, making it equivalent to a `FindSet` loop calling `Delete(true)`. The common misconception, which training data reproduces, is that `DeleteAll` iterates and fires `OnDelete` per record; it does not. (Parameterless `Delete()` likewise defaults to `Delete(false)` and skips `OnDelete`.)
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Use `DeleteAll()` / `DeleteAll(false)` for bulk deletion only when no AL `OnDelete` cleanup is required — it is the fast, set-based form. When `OnDelete` logic must run (cascading deletes, ledger cleanup, integration events), pass `DeleteAll(true)` and accept the row-by-row cost, or refactor the cleanup to run explicitly before the bulk delete.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Calling `DeleteAll()` and assuming dependent records, integration events, or validation side effects are handled by `OnDelete`. The deletion succeeds but the AL-side cleanup never runs, leaving orphaned data — and adding a manual `FindSet`/`Delete` loop "for safety" reintroduces the per-record cost the set-based form was chosen to avoid.
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
codeunit 50132 "LoadFields Bad Sample"
|
||||||
|
{
|
||||||
|
procedure TotalReleasedAmount(): Decimal
|
||||||
|
var
|
||||||
|
SalesHeader: Record "Sales Header";
|
||||||
|
Total: Decimal;
|
||||||
|
begin
|
||||||
|
// "Currency Code" is not listed. The helper takes SalesHeader BY VALUE,
|
||||||
|
// so the copy neither shares the load set nor updates the enumerator:
|
||||||
|
// reading the unlisted field triggers a fresh JIT load (an extra Get)
|
||||||
|
// on EVERY iteration, quietly reversing the saving.
|
||||||
|
SalesHeader.SetLoadFields("Amount Including VAT", Status);
|
||||||
|
if SalesHeader.FindSet() then
|
||||||
|
repeat
|
||||||
|
if IsLocalReleased(SalesHeader) then
|
||||||
|
Total += SalesHeader."Amount Including VAT";
|
||||||
|
until SalesHeader.Next() = 0;
|
||||||
|
exit(Total);
|
||||||
|
end;
|
||||||
|
|
||||||
|
local procedure IsLocalReleased(SalesHeader: Record "Sales Header"): Boolean
|
||||||
|
begin
|
||||||
|
exit((SalesHeader.Status = SalesHeader.Status::Released) and
|
||||||
|
(SalesHeader."Currency Code" = ''));
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,24 @@
|
||||||
|
codeunit 50132 "LoadFields Good Sample"
|
||||||
|
{
|
||||||
|
procedure TotalReleasedAmount(): Decimal
|
||||||
|
var
|
||||||
|
SalesHeader: Record "Sales Header";
|
||||||
|
Total: Decimal;
|
||||||
|
begin
|
||||||
|
// Every field read anywhere downstream is listed — including the one
|
||||||
|
// the by-var helper reads — so no JIT load is ever triggered.
|
||||||
|
SalesHeader.SetLoadFields("Amount Including VAT", Status, "Currency Code");
|
||||||
|
if SalesHeader.FindSet() then
|
||||||
|
repeat
|
||||||
|
if IsLocalReleased(SalesHeader) then
|
||||||
|
Total += SalesHeader."Amount Including VAT";
|
||||||
|
until SalesHeader.Next() = 0;
|
||||||
|
exit(Total);
|
||||||
|
end;
|
||||||
|
|
||||||
|
local procedure IsLocalReleased(var SalesHeader: Record "Sales Header"): Boolean
|
||||||
|
begin
|
||||||
|
exit((SalesHeader.Status = SalesHeader.Status::Released) and
|
||||||
|
(SalesHeader."Currency Code" = ''));
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,24 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: performance
|
||||||
|
keywords: [setloadfields, partial-records, just-in-time-load, jit-load, round-trip, pass-by-value, enumerator]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Reading an unlisted field after SetLoadFields triggers a JIT load
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
`SetLoadFields` loads only the named fields, but the trap is what happens when code later reads a field that was *not* listed: the platform silently issues a **just-in-time (JIT) load** — an implicit `Get` that fetches the missing field(s) in a second database round-trip. A single JIT load can erase the saving; the real danger is a JIT that repeats per record. The optimization is only a win if the listed set covers every field touched anywhere downstream, not just in the immediate code block.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Before adding `SetLoadFields`, audit the *whole* access lifecycle of the record variable — every field read in the loop body, in called procedures, in `OnValidate`/`OnAfterGetRecord`, and in anything that receives the record — and list all of them via `SetLoadFields`/`AddLoadFields`. Be especially careful when passing a partial record **by value**: the copy does not share the load set and its enumerator is not updated, so a helper that reads an unlisted field re-triggers the JIT on *every* iteration. Pass by `var` where you can (a JIT then updates the enumerator, so later iterations don't re-load), or call `AddLoadFields` before passing by value. If you cannot enumerate the fields confidently, prefer not to call `SetLoadFields` at all. See the existing guidance on when partial records pay off (`use-setloadfields-for-partial-records`).
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Adding `SetLoadFields(Field1, Field2)` at the top of a loop, then reading `Field3` deeper in the body or inside a by-value helper. The code compiles and returns correct data, but pays a hidden JIT round-trip — and in the by-value case it repeats once per row, quietly reversing the gain. JIT loads also introduce `Inconsistent read` / record-modified race errors that a full non-partial load avoids. Reviewer signal: a `SetLoadFields` list that omits a field later read through that record variable, especially a record passed by value to a procedure that reads a field the caller never listed.
|
||||||
|
|
@ -0,0 +1,28 @@
|
||||||
|
table 50100 "Customer Feedback"
|
||||||
|
{
|
||||||
|
fields
|
||||||
|
{
|
||||||
|
field(1; "Feedback No."; Code[20])
|
||||||
|
{
|
||||||
|
// No DataClassification declared. Defaults to ToBeClassified.
|
||||||
|
}
|
||||||
|
field(2; "Contact Name"; Text[100])
|
||||||
|
{
|
||||||
|
DataClassification = ToBeClassified;
|
||||||
|
}
|
||||||
|
field(3; "Email"; Text[80])
|
||||||
|
{
|
||||||
|
// Personal data classified as CustomerContent understates privacy impact.
|
||||||
|
DataClassification = CustomerContent;
|
||||||
|
}
|
||||||
|
field(4; "Feedback Text"; Text[2048])
|
||||||
|
{
|
||||||
|
DataClassification = ToBeClassified;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
keys
|
||||||
|
{
|
||||||
|
key(PK; "Feedback No.") { Clustered = true; }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,36 @@
|
||||||
|
table 50100 "Customer Feedback"
|
||||||
|
{
|
||||||
|
fields
|
||||||
|
{
|
||||||
|
field(1; "Feedback No."; Code[20])
|
||||||
|
{
|
||||||
|
DataClassification = SystemMetadata;
|
||||||
|
}
|
||||||
|
field(2; "Contact Name"; Text[100])
|
||||||
|
{
|
||||||
|
DataClassification = EndUserIdentifiableInformation;
|
||||||
|
}
|
||||||
|
field(3; "Email"; Text[80])
|
||||||
|
{
|
||||||
|
DataClassification = EndUserIdentifiableInformation;
|
||||||
|
}
|
||||||
|
field(4; "Product Code"; Code[20])
|
||||||
|
{
|
||||||
|
DataClassification = CustomerContent;
|
||||||
|
}
|
||||||
|
field(5; "Feedback Text"; Text[2048])
|
||||||
|
{
|
||||||
|
// When uncertain between CustomerContent and EUII, prefer the stronger protection.
|
||||||
|
DataClassification = EndUserIdentifiableInformation;
|
||||||
|
}
|
||||||
|
field(6; "Submitted DateTime"; DateTime)
|
||||||
|
{
|
||||||
|
DataClassification = SystemMetadata;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
keys
|
||||||
|
{
|
||||||
|
key(PK; "Feedback No.") { Clustered = true; }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: security
|
||||||
|
keywords: [dataclassification, gdpr, privacy, euii, compliance]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Classify every field with DataClassification
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Every field on every AL table and table extension must have a resolved `DataClassification` value, either declared directly on the field or inherited from a table-level default. The value drives GDPR tooling, data-subject requests, retention policies, and audit reporting — all of which rely on the field metadata to know what data to include, anonymize, or delete. A field with no field-level property and no table-level default resolves to `ToBeClassified`, which is a compliance gap, not a neutral state.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Choose the narrowest value that accurately describes the field's content: `EndUserIdentifiableInformation` for data that directly identifies a person, `EndUserPseudonymousIdentifiers` for indirect identifiers, `CustomerContent` for business operational data, `SystemMetadata` for system-generated housekeeping, `AccountData` for tenant/billing, `OrganizationIdentifiableInformation` for organization-level identifiers. Use a table-level default for homogeneous tables, and override individual fields whose content differs from that default. When uncertain between two values, pick the stronger protection.
|
||||||
|
|
||||||
|
See sample: `classify-every-field-with-dataclassification.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Leaving `DataClassification = ToBeClassified` on a field, omitting classification when the table has no default, or relying on a table-level default that understates a field's actual content. Code in this state fails compliance audits and breaks the subject-access-request and retention tooling that depends on the property being set correctly.
|
||||||
|
|
||||||
|
See sample: `classify-every-field-with-dataclassification.bad.al`.
|
||||||
|
|
@ -9,6 +9,8 @@ application-area: [all]
|
||||||
|
|
||||||
# Compose permission sets with IncludedPermissionSets
|
# Compose permission sets with IncludedPermissionSets
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
The `IncludedPermissionSets` property lets one AL permission set reference another, composing rights out of smaller building blocks. Combined with `Assignable = false` on the building blocks, an extension can ship focused per-module units (a table-data cluster, an API-access cluster) and assemble role-shaped sets that include them. Adding an object updates one building block, and every role-shaped set that includes it inherits the change automatically — instead of drifting apart across duplicated definitions.
|
The `IncludedPermissionSets` property lets one AL permission set reference another, composing rights out of smaller building blocks. Combined with `Assignable = false` on the building blocks, an extension can ship focused per-module units (a table-data cluster, an API-access cluster) and assemble role-shaped sets that include them. Adding an object updates one building block, and every role-shaped set that includes it inherits the change automatically — instead of drifting apart across duplicated definitions.
|
||||||
|
|
@ -9,6 +9,8 @@ application-area: [all]
|
||||||
|
|
||||||
# Do not grant rights beyond a user's entitlement
|
# Do not grant rights beyond a user's entitlement
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
Entitlements are license-level caps on what a user can access, derived automatically from the BC license tier. Permission sets are application-level grants administered on top of the entitlement. A permission set can only grant within the entitlement's boundaries; grants beyond those boundaries are silently clipped at runtime. This means a permission set authored and validated in a developer sandbox (with a broad license) can appear to work correctly there and fail silently in a customer tenant where users hold a narrower entitlement.
|
Entitlements are license-level caps on what a user can access, derived automatically from the BC license tier. Permission sets are application-level grants administered on top of the entitlement. A permission set can only grant within the entitlement's boundaries; grants beyond those boundaries are silently clipped at runtime. This means a permission set authored and validated in a developer sandbox (with a broad license) can appear to work correctly there and fail silently in a customer tenant where users hold a narrower entitlement.
|
||||||
|
|
@ -9,6 +9,8 @@ application-area: [all]
|
||||||
|
|
||||||
# Prefer OAuth2 over API keys for external HTTP calls
|
# Prefer OAuth2 over API keys for external HTTP calls
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
External HTTP integrations from AL can authenticate using OAuth 2.0 (client-credentials for service-to-service, authorization-code for user-delegated), API keys, basic authentication, or credentials in URLs. The mechanisms differ substantially in the blast radius of a leaked secret and in how cleanly tokens can be rotated. OAuth-issued tokens expire on their own schedule and rotate cleanly; API keys and basic-auth passwords typically have to be rotated manually and usually live unencrypted in a configuration table. When the partner supports OAuth, the difference is a material security improvement, not a stylistic preference.
|
External HTTP integrations from AL can authenticate using OAuth 2.0 (client-credentials for service-to-service, authorization-code for user-delegated), API keys, basic authentication, or credentials in URLs. The mechanisms differ substantially in the blast radius of a leaked secret and in how cleanly tokens can be rotated. OAuth-issued tokens expire on their own schedule and rotate cleanly; API keys and basic-auth passwords typically have to be rotated manually and usually live unencrypted in a configuration table. When the partner supports OAuth, the difference is a material security improvement, not a stylistic preference.
|
||||||
|
|
@ -19,6 +19,9 @@ codeunit 50100 "Customer Temp Processor"
|
||||||
until Customer.Next() = 0;
|
until Customer.Next() = 0;
|
||||||
|
|
||||||
ProcessCustomerBuffer(TempCustomer);
|
ProcessCustomerBuffer(TempCustomer);
|
||||||
|
|
||||||
|
// Explicit cleanup on the normal exit path.
|
||||||
|
TempCustomer.DeleteAll();
|
||||||
exit(true);
|
exit(true);
|
||||||
end;
|
end;
|
||||||
|
|
||||||
|
|
@ -9,13 +9,15 @@ application-area: [all]
|
||||||
|
|
||||||
# Protect sensitive data in temporary tables
|
# Protect sensitive data in temporary tables
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
A temporary record copies data out of the source table into session memory. The platform does not automatically enforce the source table's permission model on the copy, and a value written to a temporary buffer can outlive the procedure that put it there if the buffer is a global or is passed upward. Code that places sensitive rows into a temporary table is therefore responsible for the checks and cleanup the source table would otherwise provide.
|
A temporary record copies data out of the source table into session memory. The platform does not automatically enforce the source table's permission model on the copy, and a value written to a temporary buffer can outlive the procedure that put it there if the buffer is a global or is passed upward. Code that places sensitive rows into a temporary table is therefore responsible for the checks and cleanup the source table would otherwise provide.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Validate the caller's read permission on the source table before populating the temporary buffer. Keep the buffer's lifetime as short as the work requires, and prefer local temporary variables over globals for anything carrying sensitive data — a local buffer's contents are discarded automatically when the procedure returns. When a buffer must be global or is passed back to callers, delete its contents on every exit path — including error paths — so sensitive values do not linger.
|
Validate the caller's read permission on the source table before populating the temporary buffer. Keep the buffer's lifetime as short as the work requires, and delete its contents on every exit path — including error paths — so sensitive values do not linger. Prefer local temporary variables over globals for anything carrying sensitive data.
|
||||||
|
|
||||||
See sample: `protect-sensitive-data-in-temporary-tables.good.al`.
|
See sample: `protect-sensitive-data-in-temporary-tables.good.al`.
|
||||||
|
|
||||||
|
|
@ -4,7 +4,7 @@ codeunit 50134 "Api Credential Good Sample"
|
||||||
begin
|
begin
|
||||||
// Credentials live in IsolatedStorage, invisible to record reads, API
|
// Credentials live in IsolatedStorage, invisible to record reads, API
|
||||||
// pages, RapidStart packages, and Excel export.
|
// pages, RapidStart packages, and Excel export.
|
||||||
IsolatedStorage.SetEncrypted('ExternalApiKey', ApiKey, DataScope::Module);
|
IsolatedStorage.Set('ExternalApiKey', ApiKey, DataScope::Module);
|
||||||
end;
|
end;
|
||||||
|
|
||||||
procedure GetApiKey() ApiKey: SecretText
|
procedure GetApiKey() ApiKey: SecretText
|
||||||
|
|
@ -9,18 +9,16 @@ application-area: [all]
|
||||||
|
|
||||||
# A secret belongs in IsolatedStorage, never in a table field
|
# A secret belongs in IsolatedStorage, never in a table field
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
API keys, OAuth tokens, client secrets, and connection strings must not be stored in an ordinary table `Text` field — not even on a hidden setup table. A regular field is exposed through record reads, page display, RapidStart and Excel export, report datasets, and surfaces in `DataClassification` review; anyone with table permission can read it. The correct home is `IsolatedStorage`, which is invisible to database queries, API pages, and configuration packages. The storage-*location* decision is the rule here; how to scope and encrypt the value once it is in IsolatedStorage is covered separately.
|
API keys, OAuth tokens, client secrets, and connection strings must not be stored in an ordinary table `Text` field — not even on a hidden setup table. A regular field is exposed through record reads, page display, RapidStart and Excel export, report datasets, and surfaces in `DataClassification` review; anyone with table permission can read it. The correct home is `IsolatedStorage`, which is invisible to database queries, API pages, and configuration packages. The storage-*location* decision is the rule here; how to scope and encrypt the value once it is in IsolatedStorage is covered separately.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Persist every credential in `IsolatedStorage`, write it at the point of capture, and read it only when needed. Prefer `SetEncrypted` when the value fits its documented length limit. On BC24 and later, carry the value through the `SecretText` overloads; on earlier releases, keep any required `Text` handling inside a `[NonDebuggable]` boundary. Choose the `DataScope` that matches the credential's lifetime. See `isolatedstorage-datascope-module-vs-company`, `isolatedstorage-setencrypted-for-sensitive-values`, and `secrettext-for-credentials` for those separate concerns.
|
Persist every credential with `IsolatedStorage`, write it at the point of capture, and read it only when needed. For the per-secret details — choosing the right `DataScope`, encrypting at rest, and typing the value as `SecretText` so it cannot leak into logs — follow `isolatedstorage-datascope-module-vs-company`, `isolatedstorage-setencrypted-for-sensitive-values`, and `secrettext-for-credentials`.
|
||||||
|
|
||||||
See sample: `secrets-isolated-storage.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
A "Setup" or "Connection" table carrying a `Text` field named `API Key`, `Password`, or `Client Secret`. The value is now readable by any object with table permission, ships in RapidStart packages and Excel exports, and appears in record snapshots — a credential disclosure that no amount of encryption-in-transit elsewhere makes up for. Reviewer signal: a secret-shaped field declared on a table instead of an `IsolatedStorage` call.
|
A "Setup" or "Connection" table carrying a `Text` field named `API Key`, `Password`, or `Client Secret`. The value is now readable by any object with table permission, ships in RapidStart packages and Excel exports, and appears in record snapshots — a credential disclosure that no amount of encryption-in-transit elsewhere makes up for. Reviewer signal: a secret-shaped field declared on a table instead of an `IsolatedStorage` call.
|
||||||
|
|
||||||
See sample: `secrets-isolated-storage.bad.al`.
|
|
||||||
|
|
@ -0,0 +1,17 @@
|
||||||
|
codeunit 50136 "Telemetry Bad Sample"
|
||||||
|
{
|
||||||
|
procedure LogSyncDiagnostic(RecordsProcessed: Integer)
|
||||||
|
var
|
||||||
|
Dimensions: Dictionary of [Text, Text];
|
||||||
|
begin
|
||||||
|
Dimensions.Add('recordsProcessed', Format(RecordsProcessed));
|
||||||
|
|
||||||
|
// TelemetryScope::All pushes this internal diagnostic into every
|
||||||
|
// customer's Application Insights too, inflating their ingestion cost
|
||||||
|
// and burying their own signals in noise. ExtensionPublisher is the
|
||||||
|
// correct scope for publisher-only diagnostics.
|
||||||
|
Session.LogMessage(
|
||||||
|
'SYNC001', 'Nightly sync completed.', Verbosity::Normal,
|
||||||
|
DataClassification::SystemMetadata, TelemetryScope::All, Dimensions);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,15 @@
|
||||||
|
codeunit 50136 "Telemetry Good Sample"
|
||||||
|
{
|
||||||
|
procedure LogSyncDiagnostic(RecordsProcessed: Integer)
|
||||||
|
var
|
||||||
|
Dimensions: Dictionary of [Text, Text];
|
||||||
|
begin
|
||||||
|
Dimensions.Add('recordsProcessed', Format(RecordsProcessed));
|
||||||
|
|
||||||
|
// A diagnostic only the publisher acts on: route it to the publisher's
|
||||||
|
// own Application Insights, not the customer's environment resource.
|
||||||
|
Session.LogMessage(
|
||||||
|
'SYNC001', 'Nightly sync completed.', Verbosity::Normal,
|
||||||
|
DataClassification::SystemMetadata, TelemetryScope::ExtensionPublisher, Dimensions);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,24 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: telemetry
|
||||||
|
keywords: [telemetry, session-logmessage, telemetryscope, application-insights, extensionpublisher, ingestion-cost]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Default TelemetryScope to ExtensionPublisher, not All
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
The `TelemetryScope` parameter of `Session.LogMessage` (and `LogError`) controls *where* a custom telemetry signal is routed, not just whether it is emitted. `TelemetryScope::ExtensionPublisher` sends the signal only to the extension publisher's own Application Insights resource. `TelemetryScope::All` sends it to **both** the publisher's resource **and** the customer's environment-level Application Insights resource. The distinction is easy to get wrong because both values compile and both "emit telemetry" — but `All` silently adds to the customer's ingestion volume and cost.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Default to `TelemetryScope::ExtensionPublisher` for diagnostic telemetry that only the publisher acts on. Reserve `TelemetryScope::All` for signals the customer's own administrators are expected to monitor and act on (for example, a business event surfaced to their environment telemetry). Treat the choice as a deliberate routing decision per signal, not a copy-paste default.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Emitting all custom telemetry with `TelemetryScope::All` "to be safe." This pushes the publisher's internal diagnostics into every customer's Application Insights, inflating their ingestion cost and burying their own signals in noise — a footgun a code reviewer can catch by flagging `All` on any signal the customer would not act on.
|
||||||
|
|
@ -8,11 +8,13 @@ application-area: [all]
|
||||||
---
|
---
|
||||||
# Set Field Importance To Drive FastTab Progressive Disclosure
|
# Set Field Importance To Drive FastTab Progressive Disclosure
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
A FastTab field's `Importance` property controls whether the field is visible immediately, hidden behind "Show more", or surfaced on the collapsed FastTab header summary line. The three values are `Standard` (the default, shown in the expanded FastTab), `Promoted` (also rendered on the FastTab header when the tab is collapsed), and `Additional` (hidden until the user clicks "Show more"). Misusing these values either clutters the summary line or buries fields users need on every transaction, so reviewers should treat `Importance` as a deliberate layout decision rather than an afterthought.
|
A FastTab field's `Importance` property controls whether the field is visible immediately, hidden behind "Show more", or surfaced on the collapsed FastTab header summary line. The three values are `Standard` (the default, shown in the expanded FastTab), `Promoted` (also rendered on the FastTab header when the tab is collapsed), and `Additional` (hidden until the user clicks "Show more"). Misusing these values either clutters the summary line or buries fields users need on every transaction, so reviewers should treat `Importance` as a deliberate layout decision rather than an afterthought.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
Promote only the small set of identifying fields per FastTab that users must read at a glance without expanding — name, status, key amount — so the collapsed header summary line stays scannable. Leave the everyday working fields at `Standard`, and push rarely-touched fields (legacy compatibility fields, system timestamps, seldom-changed configuration) to `Additional`. Note that field-level `Importance = Promoted` is unrelated to action promotion on the page action bar; it governs FastTab field visibility only. Do not rely on initial expand or collapse state, which you cannot set programmatically and which the platform may personalize per user — design assuming any FastTab may be collapsed.
|
Promote only the two to four identifying fields per FastTab that users must read at a glance without expanding — name, status, key amount — so the collapsed header summary line stays scannable. Leave the everyday working fields at `Standard`, and push rarely-touched fields (legacy compatibility fields, system timestamps, seldom-changed configuration) to `Additional`. Note that field-level `Importance = Promoted` is unrelated to action promotion on the page action bar; it governs FastTab field visibility only. Do not rely on initial expand or collapse state, which you cannot set programmatically and which the platform may personalize per user — design assuming any FastTab may be collapsed.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
Setting `Importance = Promoted` on most fields of a FastTab so "everything is important" defeats progressive disclosure: the collapsed summary line overflows and conveys nothing at a glance. The opposite failure is marking frequently edited fields `Additional`, forcing users to click "Show more" on every record. A detectable signal is a FastTab whose fields are nearly all `Promoted`, or a FastTab containing only `Additional` fields, which renders as an empty tab until expanded.
|
Setting `Importance = Promoted` on most fields of a FastTab so "everything is important" defeats progressive disclosure: the collapsed summary line overflows and conveys nothing at a glance. The opposite failure is marking frequently edited fields `Additional`, forcing users to click "Show more" on every record. A detectable signal is a FastTab whose fields are nearly all `Promoted`, or a FastTab containing only `Additional` fields, which renders as an empty tab until expanded.
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
---
|
---
|
||||||
bc-version: [15..]
|
bc-version: [all]
|
||||||
domain: ui
|
domain: ui
|
||||||
keywords: [enqueuebackgroundtask, async-calculation, child-session, factbox, cue-tile, onaftergetcurrrecord, responsive-page, read-only]
|
keywords: [enqueuebackgroundtask, async-calculation, child-session, factbox, cue-tile, onaftergetcurrrecord, responsive-page, read-only]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
|
|
@ -8,6 +8,8 @@ application-area: [all]
|
||||||
---
|
---
|
||||||
# Offload Slow Read-Only Page Calculations To Background Tasks
|
# Offload Slow Read-Only Page Calculations To Background Tasks
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
Pages that compute statistics, aggregates, or external lookups inline block the page from rendering until the calculation finishes, producing a visible freeze on FactBoxes, cue tiles, and calculated fields. Business Central provides page background tasks: `CurrPage.EnqueueBackgroundTask` runs a dedicated codeunit in a read-only child session and returns values via `OnPageBackgroundTaskCompleted`, so the page opens immediately and fills in computed values as they arrive. This matters because users should never wait on a calculation they may not need. The mechanism has specific rules that are easy to get wrong, which is why it warrants an explicit pattern.
|
Pages that compute statistics, aggregates, or external lookups inline block the page from rendering until the calculation finishes, producing a visible freeze on FactBoxes, cue tiles, and calculated fields. Business Central provides page background tasks: `CurrPage.EnqueueBackgroundTask` runs a dedicated codeunit in a read-only child session and returns values via `OnPageBackgroundTaskCompleted`, so the page opens immediately and fills in computed values as they arrive. This matters because users should never wait on a calculation they may not need. The mechanism has specific rules that are easy to get wrong, which is why it warrants an explicit pattern.
|
||||||
|
|
||||||
|
|
@ -8,6 +8,8 @@ application-area: [all]
|
||||||
---
|
---
|
||||||
# Promote Actions With The Modern `actionref` Syntax, Never The Legacy `Promoted` Properties
|
# Promote Actions With The Modern `actionref` Syntax, Never The Legacy `Promoted` Properties
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
Business Central 2022 release wave 2 (v21) introduced the `area(Promoted)` block with `actionref` as the way to promote page actions, separating an action's definition from its promotion. The older approach set `Promoted`, `PromotedCategory`, `PromotedOnly`, and `PromotedIsBig` directly on each action. The two syntaxes cannot be mixed within a single page or page extension, and choosing the legacy one entangles definition with presentation, making the action bar harder to maintain and to extend.
|
Business Central 2022 release wave 2 (v21) introduced the `area(Promoted)` block with `actionref` as the way to promote page actions, separating an action's definition from its promotion. The older approach set `Promoted`, `PromotedCategory`, `PromotedOnly`, and `PromotedIsBig` directly on each action. The two syntaxes cannot be mixed within a single page or page extension, and choosing the legacy one entangles definition with presentation, making the action bar harder to maintain and to extend.
|
||||||
|
|
||||||
|
|
@ -8,11 +8,13 @@ application-area: [all]
|
||||||
---
|
---
|
||||||
# Use Standard Promoted Action Group Names And Placements
|
# Use Standard Promoted Action Group Names And Placements
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
Business Central ships a fixed vocabulary of promoted action groups, and users build muscle memory around where each kind of action lives. When you define `area(Promoted)` groups, reusing the standard caption and placement for a given action class makes the page feel native; inventing your own caption or putting an action in the wrong group forces every user to relearn your page. Frontier models tend to emit plausible-but-nonstandard captions (`Go To`, `Vendor Actions`, `Related`) instead of the established BC names, which is exactly what breaks cross-page consistency.
|
Business Central ships a fixed vocabulary of promoted action groups, and users build muscle memory around where each kind of action lives. When you define `area(Promoted)` groups, reusing the standard caption and placement for a given action class makes the page feel native; inventing your own caption or putting an action in the wrong group forces every user to relearn your page. Frontier models tend to emit plausible-but-nonstandard captions (`Go To`, `Vendor Actions`, `Related`) instead of the established BC names, which is exactly what breaks cross-page consistency.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
Map each action to its conventional group and use the exact standard caption: `Home`/`Process` for data-modifying and workflow actions (entity/card/document pages use `Home`, lists and worksheets use `Process`); an entity-named group (`Customer`, `Item`, `Order`) for navigation tied to the current record (statistics, ledger entries, dimensions); `Navigate` for related pages that are useful regardless of the selected record; `Report` for printing and analysis; and the workflow groups `Posting`, `Release`, `Approve`, `Request Approval`, and `Prepare` for their respective document lifecycle actions. Standard guidance recommends `ShowAs = SplitButton` for `Posting` (Post / Post and Print / Preview) and `Release` (Release / Reopen), while the other common groups normally render as standard groups. Use a split button elsewhere only for closely related alternatives with an obvious primary action. The first enabled and visible action becomes the primary button, so place the expected default first and remember that extensions or personalization can reorder it. Within a common group keep the same action sequence you see on the matching base-app page (for example, mirror Sales Order for a sales document) so order stays predictable.
|
Map each action to its conventional group and use the exact standard caption: `Home`/`Process` for data-modifying and workflow actions (entity/card/document pages use `Home`, lists and worksheets use `Process`); an entity-named group (`Customer`, `Item`, `Order`) for navigation tied to the current record (statistics, ledger entries, dimensions); `Navigate` for related pages that are useful regardless of the selected record; `Report` for printing and analysis; and the workflow groups `Posting`, `Release`, `Approve`, `Request Approval`, and `Prepare` for their respective document lifecycle actions. Only `Posting` (Post / Post and Print / Preview) and `Release` (Release / Reopen) should render as split buttons via `ShowAs = SplitButton`; everything else is a normal dropdown. Within a common group keep the same action sequence you see on the matching base-app page (e.g. mirror Sales Order for a sales document) so order stays predictable.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
Custom captions for what is really a standard group (`Vendor Actions` instead of the `Vendor` entity group, `Go To` instead of `Navigate`), posting or statistics actions dropped into the wrong group, or many tiny one-action groups that fragment the ribbon. The reviewer signal is an `area(Promoted)` block whose `group` captions do not match the base-application names for the same page type, or a split button whose actions are unrelated or lack an obvious primary operation.
|
Custom captions for what is really a standard group (`Vendor Actions` instead of the `Vendor` entity group, `Go To` instead of `Navigate`), posting or statistics actions dropped into the wrong group, or many tiny one-action groups that fragment the ribbon. The reviewer signal is an `area(Promoted)` block whose `group` captions do not match the base-application names for the same page type, or a `ShowAs = SplitButton` on anything other than `Posting`/`Release`.
|
||||||
20
community/knowledge/ui/split-button-standard-groups.md
Normal file
20
community/knowledge/ui/split-button-standard-groups.md
Normal file
|
|
@ -0,0 +1,20 @@
|
||||||
|
---
|
||||||
|
bc-version: [21..]
|
||||||
|
domain: ui
|
||||||
|
keywords: [showas, splitbutton, promoted-actions, actionref, posting-actions, release-action, action-bar]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
# Reserve `ShowAs = SplitButton` For Standard Posting And Release Groups
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
Setting `ShowAs = SplitButton` on a `group` inside `area(Promoted)` renders a primary one-click button with a dropdown of related alternatives, where the FIRST `actionref` in the group becomes the primary (left) button. Business Central users have learned this pattern from the two standard groups it ships with — Posting (`Post`, `Post and Print`, `Post and Send`, `Preview Posting`) and Release (`Release`, `Reopen`). Inventing new split-button groups for unrelated actions, or ordering the dropdown so the most common action is not first, breaks that learned muscle memory and makes users guess what the left button will do.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
Use `ShowAs = SplitButton` only when all hold: the actions are genuinely variations of one operation, there is an obvious most-frequent primary, and the dropdown stays at roughly two to four items. Place that primary action as the first `actionref` so it occupies the left button; order the remaining refs by descending frequency. Outside the Posting and Release conventions, treat a new split-button group as something to justify, not a default — a plain promoted group or category is usually the safer choice and keeps the action bar predictable.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
Grouping unrelated actions under one split button to save toolbar space — for example pairing `Post` with `Delete`, or `Release` with `Print` — so the left button performs whatever happens to be listed first. The reviewer signal is a group with `ShowAs = SplitButton` whose member `actionref`s do not share a verb or workflow, a primary that is not the most common action, or a dropdown padded well beyond four items. Each makes the immediate left-click unpredictable and costs the user the very click the split button was meant to save.
|
||||||
20
community/knowledge/upgrade/no-series-bc24-migration.md
Normal file
20
community/knowledge/upgrade/no-series-bc24-migration.md
Normal file
|
|
@ -0,0 +1,20 @@
|
||||||
|
---
|
||||||
|
bc-version: [24..]
|
||||||
|
domain: upgrade
|
||||||
|
keywords: [no-series, noseriesmanagement, codeunit-310, getnextno, peeknextno, testmanual, arerelated, no-series-batch, business-foundation, obsolete-codeunit]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
# Migrate No. Series Calls From NoSeriesManagement To The BC24 No. Series Module
|
||||||
|
|
||||||
|
> Contributions welcome — open a PR to refine or extend this article.
|
||||||
|
|
||||||
|
## Description
|
||||||
|
In BC24 (2024 Wave 1) Microsoft moved number generation into the Business Foundation `No. Series` codeunit (310) and obsoleted the legacy `NoSeriesManagement` codeunit (396). Code that still declares `Codeunit NoSeriesManagement` or calls its methods compiles only against the temporary obsolete shim and will break once Microsoft removes it. The new API is not a drop-in rename: the facade exposes a small, specific set of real methods, parameter shapes changed, and the old single method that both previewed and consumed a number was split into two. Getting the mapping wrong silently consumes numbers when you only meant to preview, leaving gaps in the sequence.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
Replace the `NoSeriesManagement` variable with `Codeunit "No. Series"` and map each call deliberately using the facade's actual methods — `GetNextNo`, `PeekNextNo`, `GetLastNoUsed`, `TestManual`, `IsManual`, and `AreRelated`. Use `GetNextNo(SeriesCode, RefDate)` only when you intend to consume and advance the series for a committed document, and `PeekNextNo(SeriesCode, RefDate)` for any display, validation, or preview-posting path where you must not consume. Replace `InitSeries` with a guarded `if "No." = '' then "No." := NoSeries.GetNextNo(...)`. Map `SelectSeries` to `LookupRelatedNoSeries`, relationship checks the old code did by hand to `AreRelated`, and both `TestManual` and `ManualNoAllowed` to `TestManual` (which now raises its own error). For multi-document allocation use `Codeunit "No. Series - Batch"` and persist its state once with `SaveState` instead of committing per iteration. Treat the migration as an opportunity to add preview-posting support, since `PeekNextNo` now makes that trivial.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
Mechanically swapping the codeunit reference while keeping the old boolean call shape. The legacy `GetNextNo(Series, Date, false)` meant "peek" and `GetNextNo(Series, Date, true)` meant "consume"; the new `GetNextNo` always consumes and takes no boolean. Equally common is inventing validation helpers such as `IsValidNo`, `VerifySeriesExists`, `IsValidForDate`, or `TryGetNextNo` — these names are not on the `No. Series` or `No. Series - Batch` codeunits and will not compile, a frequent LLM hallucination for this migration. A reviewer can detect the defect by the residual third boolean argument, by any lingering `NoSeriesMgt`/`NoSeriesManagement` identifier, by a fabricated method name, or by an `OnBeforeGetNextNo`/`OnAfterGetNextNo` subscriber — those events were removed without replacement, so that logic must be rewritten as inline pre/post procedures, not re-subscribed. A subtler signal is `GetNextNo` used merely to display a preview, which silently advances the series and creates number gaps; that should be `PeekNextNo`.
|
||||||
|
|
@ -1,56 +0,0 @@
|
||||||
# AL review evaluation
|
|
||||||
|
|
||||||
The evaluation is convention-driven. For every `microsoft/skills/review/al-<domain>-review.md` leaf, the harness finds `microsoft/knowledge/<domain>/`, selects the first article (by filename) with both `.bad.al` and `.good.al` companions, and derives the expected positive and clean control automatically. Adding a conforming leaf requires no scoring-contract edit.
|
|
||||||
|
|
||||||
`review-fixtures.json` contains only global thresholds and optional exceptional overrides. An override may select a different article or add context when the generic convention cannot express a scenario. It should remain empty in the normal case.
|
|
||||||
|
|
||||||
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.
|
|
||||||
|
|
||||||
## Validate the corpus
|
|
||||||
|
|
||||||
```powershell
|
|
||||||
pwsh ./tools/Test-ReviewFixtures.ps1 -Root .
|
|
||||||
```
|
|
||||||
|
|
||||||
This credential-free check proves every registered leaf maps to a same-named knowledge domain with at least one complete AL sample pair and that all configured overrides are valid.
|
|
||||||
|
|
||||||
## Run a fast-model evaluation
|
|
||||||
|
|
||||||
1. Prepare neutral inputs:
|
|
||||||
|
|
||||||
```powershell
|
|
||||||
pwsh ./tools/Test-ReviewFixtures.ps1 -Root . -PrepareDirectory ./.evaluation-run
|
|
||||||
```
|
|
||||||
|
|
||||||
This is also the CI path. It derives all cases, builds the current index, requires the convention-selected article to rank naturally into the candidate cutoff, and prepares the neutral requests.
|
|
||||||
|
|
||||||
2. For a fast/small model, use one fresh invocation per `request-case-*.json`. Each request embeds the exact leaf instructions, that domain's candidate index rows with authoritative paths, and one opaque case. The model opens only matching articles and copies finding IDs from `candidateArticles[].path`. Save each response with the matching `result-case-*.json` name in the same directory.
|
|
||||||
|
|
||||||
`request-<domain>.json` files provide optional two-case leaf batches; save those as `result-<domain>.json`. Directory scoring prefers `result-case-*.json` when present and otherwise falls back to `result-*.json`. `review-request.json` is an optional all-domains stress test for larger models. Neither batch form is the preferred fast-model profile.
|
|
||||||
|
|
||||||
3. Save only this result shape:
|
|
||||||
|
|
||||||
```json
|
|
||||||
{
|
|
||||||
"cases": [
|
|
||||||
{
|
|
||||||
"id": "case-a1b2c3d4",
|
|
||||||
"findings": [
|
|
||||||
{ "id": "microsoft/knowledge/appsource/object-affixes-prevent-collisions.md" }
|
|
||||||
]
|
|
||||||
}
|
|
||||||
]
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
Include every case. A clean control has an empty `findings` array.
|
|
||||||
|
|
||||||
4. Score all per-leaf results together:
|
|
||||||
|
|
||||||
```powershell
|
|
||||||
pwsh ./tools/Test-ReviewFixtures.ps1 -Root . -ResultsDirectory ./.evaluation-run
|
|
||||||
```
|
|
||||||
|
|
||||||
For a single combined stress-test result, use `-ResultsPath` instead.
|
|
||||||
|
|
||||||
The committed gate requires full expected recall, the exact convention-derived article ID, and no findings on clean controls.
|
|
||||||
|
|
@ -1,39 +0,0 @@
|
||||||
{
|
|
||||||
"version": 2,
|
|
||||||
"selection": "first-paired-al-article",
|
|
||||||
"minimumExpectedRecall": 1.0,
|
|
||||||
"minimumCleanRate": 1.0,
|
|
||||||
"overrides": {
|
|
||||||
"appsource": {
|
|
||||||
"context": "AppSourceCop mandatoryAffixes is configured to ABC."
|
|
||||||
},
|
|
||||||
"breaking-changes": {
|
|
||||||
"article": "do-not-expose-sensitive-data-through-public-api"
|
|
||||||
},
|
|
||||||
"events": {
|
|
||||||
"article": "initialize-ishandled-to-false-before-publishing"
|
|
||||||
},
|
|
||||||
"interfaces": {
|
|
||||||
"article": "set-defaultimplementation-on-enum"
|
|
||||||
},
|
|
||||||
"performance": {
|
|
||||||
"article": "use-isempty-for-existence-check"
|
|
||||||
},
|
|
||||||
"privacy": {
|
|
||||||
"article": "no-pii-in-telemetry-message-string"
|
|
||||||
},
|
|
||||||
"style": {
|
|
||||||
"article": "label-comment-explains-placeholders"
|
|
||||||
},
|
|
||||||
"telemetry": {
|
|
||||||
"article": "telemetry-event-id-stable-unique"
|
|
||||||
},
|
|
||||||
"upgrade": {
|
|
||||||
"article": "initvalue-does-not-update-existing-rows",
|
|
||||||
"context": "The extended table existed in the previous app version and already contains rows."
|
|
||||||
},
|
|
||||||
"web-services": {
|
|
||||||
"article": "expose-systemid-as-the-api-key"
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
@ -11,18 +11,18 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
An AppSource extension must prevent name collisions through its registered affix or, on BC23 and later for objects it owns, a namespace with at least two levels. The affix still applies to every field, key, control, or action added to a base-application object; see `two-level-namespace-replaces-object-affix-not-extension-member-affix.md`. Without either mechanism, two apps that both define a `Loyalty Tier` table cannot coexist, and two apps that add an unaffixed `Loyalty Points` field to `Customer` still collide regardless of their namespaces.
|
An AppSource extension must carry a reserved affix — a prefix or a suffix of at least three characters — on the names of the objects it owns **and** on any field, key, control, or action it adds to a base-application object. The affix is registered with Microsoft; when two coexisting extensions would otherwise collide, the registrant of the affix wins. Without it, two apps that both add a `Loyalty Points` field to `Customer`, or both define a `Loyalty Tier` table, cannot be installed side by side.
|
||||||
|
|
||||||
AppSourceCop enforces this. The primary rule is AS0011 ("An affix is required"); the affixes are configured through `mandatoryAffixes` (and `mandatoryPrefix`) in `AppSourceCop.json`. Two placements matter and are easy to get half-right: an object you define carries the affix at **object-name** level, while a member you add to a **standard** object carries the affix on that **member's** name. Adding an affixed object is not enough — an unaffixed field bolted onto `Customer` still collides and still fails validation.
|
AppSourceCop enforces this. The primary rule is AS0011 ("An affix is required"); the affixes are configured through `mandatoryAffixes` (and `mandatoryPrefix`) in `AppSourceCop.json`. Two placements matter and are easy to get half-right: an object you define carries the affix at **object-name** level, while a member you add to a **standard** object carries the affix on that **member's** name. Adding an affixed object is not enough — an unaffixed field bolted onto `Customer` still collides and still fails validation.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Own objects use the registered affix (for example `ABC Loyalty Tier`) or, when targeting BC23 or later, a qualifying namespace. Every field or action added to a standard object remains individually affixed (for example `Loyalty Points ABC` on a `Customer` tableextension).
|
Own objects are named with the affix (e.g. a table `ABC Loyalty Tier`), and every field or action added to a standard object is individually affixed (e.g. `Loyalty Points ABC` on a `Customer` tableextension).
|
||||||
|
|
||||||
See sample: `object-affixes-prevent-collisions.good.al`.
|
See sample: `object-affixes-prevent-collisions.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
An owned object with neither a qualifying namespace nor an affix, an unaffixed extension member, or the common half-measure where the extension object carries the affix but a field it adds to a standard table does not. AS0011 flags the missing collision protection and the field can still collide with another app.
|
Unaffixed object or member names, or the common half-measure: the extension object carries the affix but a field it adds to a standard table does not. AS0011 flags the missing affix and the field can still collide with another app.
|
||||||
|
|
||||||
See sample: `object-affixes-prevent-collisions.bad.al`.
|
See sample: `object-affixes-prevent-collisions.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -1,43 +0,0 @@
|
||||||
table 50476 "Rental Setup Bad"
|
|
||||||
{
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; "Primary Key"; Code[10]) { }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
page 50477 "Rental Setup Bad"
|
|
||||||
{
|
|
||||||
PageType = Card;
|
|
||||||
SourceTable = "Rental Setup Bad";
|
|
||||||
|
|
||||||
layout
|
|
||||||
{
|
|
||||||
area(Content)
|
|
||||||
{
|
|
||||||
field("Primary Key"; Rec."Primary Key")
|
|
||||||
{
|
|
||||||
ApplicationArea = All;
|
|
||||||
Caption = 'Primary Key';
|
|
||||||
ToolTip = 'Specifies the setup record.';
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
codeunit 50478 "Rental Setup Mgt. Bad"
|
|
||||||
{
|
|
||||||
procedure Initialize()
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
||||||
permissionset 50479 "Rental User"
|
|
||||||
{
|
|
||||||
Assignable = true;
|
|
||||||
// The setup page opens, but saving or running setup logic requires SUPER.
|
|
||||||
Permissions =
|
|
||||||
page "Rental Setup Bad" = X;
|
|
||||||
}
|
|
||||||
|
|
@ -1,45 +0,0 @@
|
||||||
table 50472 "Rental Setup"
|
|
||||||
{
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; "Primary Key"; Code[10]) { }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
page 50473 "Rental Setup"
|
|
||||||
{
|
|
||||||
PageType = Card;
|
|
||||||
SourceTable = "Rental Setup";
|
|
||||||
|
|
||||||
layout
|
|
||||||
{
|
|
||||||
area(Content)
|
|
||||||
{
|
|
||||||
field("Primary Key"; Rec."Primary Key")
|
|
||||||
{
|
|
||||||
ApplicationArea = All;
|
|
||||||
Caption = 'Primary Key';
|
|
||||||
ToolTip = 'Specifies the setup record.';
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
codeunit 50474 "Rental Setup Mgt."
|
|
||||||
{
|
|
||||||
procedure Initialize()
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
||||||
permissionset 50475 "Rental Manager"
|
|
||||||
{
|
|
||||||
Assignable = true;
|
|
||||||
Permissions =
|
|
||||||
tabledata "Rental Setup" = RIMD,
|
|
||||||
table "Rental Setup" = X,
|
|
||||||
page "Rental Setup" = X,
|
|
||||||
codeunit "Rental Setup Mgt." = X;
|
|
||||||
}
|
|
||||||
|
|
@ -1,26 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: appsource
|
|
||||||
keywords: [permission-set, super, appsource, setup, usage, tabledata, execute, submission]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# AppSource permission sets must cover setup and usage without SUPER
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
An AppSource app must provide permission sets that let assigned users complete the app's setup and normal usage without `SUPER`. The requirement is about complete effective grants, not about naming the permission set after the app. A package can compile and install with missing tabledata or execute permissions, then fail only when Marketplace validation or a real non-SUPER user reaches the omitted path.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Trace every setup page, normal page, report, codeunit, and tabledata operation exposed by the app and cover it through assignable role permission sets composed from focused non-assignable sets. Validate setup and representative workflows as a user assigned only those app roles. Grant the minimum required operations; completeness is not a reason to use wildcards.
|
|
||||||
|
|
||||||
See sample: `permission-sets-cover-setup-and-usage-without-super.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Shipping no permission set, omitting a tabledata or execute grant used by the app's own UI, or instructing users and validators to assign `SUPER` when setup fails. Do not flag a permission-set name that differs from the app name; no such naming requirement exists.
|
|
||||||
|
|
||||||
See sample: `permission-sets-cover-setup-and-usage-without-super.bad.al`.
|
|
||||||
|
|
@ -1,22 +0,0 @@
|
||||||
namespace Contoso;
|
|
||||||
|
|
||||||
table 50462 "Rental Agreement"
|
|
||||||
{
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; "No."; Code[20]) { }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
tableextension 50463 "Rental Customer Ext" extends Customer
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(50463; "Loyalty Points"; Integer)
|
|
||||||
{
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
@ -1,22 +0,0 @@
|
||||||
namespace Contoso.Rentals;
|
|
||||||
|
|
||||||
table 50460 "Rental Agreement"
|
|
||||||
{
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; "No."; Code[20]) { }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
tableextension 50461 "Rental Customer Ext" extends Customer
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(50461; "Loyalty Points RNT"; Integer)
|
|
||||||
{
|
|
||||||
DataClassification = CustomerContent;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
@ -1,26 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [23..]
|
|
||||||
domain: appsource
|
|
||||||
keywords: [namespace, two-level, affix, prefix, suffix, as0011, tableextension, pageextension]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# A two-level namespace replaces an object affix, not an extension-member affix
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
Current AppSource naming guidance accepts a namespace with at least two levels, such as `Contoso.Rentals`, instead of a registered prefix or suffix on the names of objects the app owns. The namespace does not qualify members added to another publisher's object: fields, keys, controls, and actions introduced through table or page extensions still share the target object's flat member namespace and still need the registered affix.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Choose one collision strategy for owned objects: a registered affix or a globally meaningful namespace with at least two levels. Regardless of that choice, apply the registered affix to every member added to a base or third-party object. Keep the affix configured for AppSourceCop so member validation remains deterministic.
|
|
||||||
|
|
||||||
See sample: `two-level-namespace-replaces-object-affix-not-extension-member-affix.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Using `namespace Contoso;` as though one level satisfied the AppSource alternative, or declaring `namespace Contoso.Rentals;` and then adding an unaffixed `Loyalty Points` field to `Customer`. The namespace distinguishes the extension's own objects; it cannot disambiguate members on Customer.
|
|
||||||
|
|
||||||
See sample: `two-level-namespace-replaces-object-affix-not-extension-member-affix.bad.al`.
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
codeunit 50305 "Net Amount Api Good"
|
codeunit 50305 "Net Amount Api Good"
|
||||||
{
|
{
|
||||||
// Old name kept during the warning window. The tag records when obsoletion
|
// Old name kept and marked obsolete: callers still compile but get a warning
|
||||||
// began; a later release deletes the method after consumers have migrated.
|
// pointing at the replacement, with a tag recording the removal target version.
|
||||||
[Obsolete('Use CalculateNetAmount instead.', '25.0')]
|
[Obsolete('Use CalculateNetAmount instead.', '25.0')]
|
||||||
procedure CalcNet(GrossAmount: Decimal; TaxRate: Decimal): Decimal
|
procedure CalcNet(GrossAmount: Decimal; TaxRate: Decimal): Decimal
|
||||||
begin
|
begin
|
||||||
|
|
|
||||||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
Deleting or renaming a published procedure (or object) in a single release is a hard break: dependent extensions that reference it stop compiling the moment they pick up the new version, with no warning window to migrate. AL provides staged deprecation so consumers get advance notice. A procedure uses `[Obsolete('reason', 'tag')]`: it remains callable but callers receive a compiler warning naming the replacement and the version in which obsoletion began. Methods do not have `ObsoleteState`; after the deprecation window, the method is deleted, commonly through versioned preprocessor cleanup. Objects and fields instead use the `ObsoleteState = Pending` to `Removed` property progression.
|
Deleting or renaming a published procedure (or object) in a single release is a hard break: dependent extensions that reference it stop compiling the moment they pick up the new version, with no warning window to migrate. AL provides a staged deprecation lifecycle precisely so consumers get advance notice. For a procedure, apply the `[Obsolete('reason', 'tag')]` attribute: the member keeps working but every caller gets a compiler warning naming the replacement and the target version. The member stays through a deprecation window — at least one major release — before it is finally removed. Object- and field-level members use the matching `ObsoleteState = Pending` → `Removed` property progression. LLMs trained to "clean up" code often delete or rename the old member immediately, skipping the window entirely.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
When a published procedure is superseded, keep it in place and mark it `[Obsolete('Use CalculateNetAmount instead.', '25.0')]`, where the message names the replacement and the tag records when the method became obsolete. Have the obsolete member forward to the new one so behavior is preserved during the window. Only after the deprecation window has elapsed should a later release delete the method. For an object or field, use `Pending` during the warning window and `Removed` afterward.
|
When a published procedure is superseded, keep it in place and mark it `[Obsolete('Use CalculateNetAmount instead.', '25.0')]`, where the message names the replacement and the tag records the target version for removal. Have the obsolete member forward to the new one so behavior is preserved during the window. Only after the deprecation window has elapsed — a later release — change its state to removed. This gives every dependent app a compile-time signal and time to migrate before anything actually disappears.
|
||||||
|
|
||||||
See sample: `deprecate-public-members-with-the-obsolete-lifecycle.good.al`.
|
See sample: `deprecate-public-members-with-the-obsolete-lifecycle.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Renaming or deleting the published `CalcNet` procedure in place — replacing it with `CalculateNetAmount` and nothing else — so consumers calling `CalcNet` break immediately with no deprecation notice. Detection: a previously shipped non-`local` procedure that vanished or was renamed between versions with no `[Obsolete]` marker left behind during a prior warning window. Do not suggest `ObsoleteState = Removed` for a method; that property belongs to supported object and element types.
|
Renaming or deleting the published `CalcNet` procedure in place — replacing it with `CalculateNetAmount` and nothing else — so consumers calling `CalcNet` break immediately with no deprecation notice. Detection: a previously shipped non-`local` procedure that vanished or was renamed between versions with no `[Obsolete]` marker left behind on a kept member. Mark it obsolete and keep it for a window instead.
|
||||||
|
|
||||||
See sample: `deprecate-public-members-with-the-obsolete-lifecycle.bad.al`.
|
See sample: `deprecate-public-members-with-the-obsolete-lifecycle.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -1,10 +1,10 @@
|
||||||
codeunit 50320 "Payment Client Good"
|
codeunit 50320 "Payment Client Good"
|
||||||
{
|
{
|
||||||
var
|
var
|
||||||
AccessToken: SecretText;
|
AccessToken: Text;
|
||||||
|
|
||||||
// Credential remains SecretText as it flows inward and is stored.
|
// Credential flows inward through an internal setter and never leaves the object.
|
||||||
internal procedure SetAccessToken(NewToken: SecretText)
|
internal procedure SetAccessToken(NewToken: Text)
|
||||||
begin
|
begin
|
||||||
AccessToken := NewToken;
|
AccessToken := NewToken;
|
||||||
end;
|
end;
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
---
|
---
|
||||||
bc-version: [23..]
|
bc-version: [all]
|
||||||
domain: breaking-changes
|
domain: breaking-changes
|
||||||
keywords: [sensitive-data, secrettext, token, credential, public-api, access-boundary]
|
keywords: [sensitive-data, secrettext, token, credential, public-api, access-boundary]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
|
|
|
||||||
|
|
@ -1,9 +0,0 @@
|
||||||
// This published object previously used namespace Contoso.Rentals.
|
|
||||||
namespace Contoso.RentalManagement;
|
|
||||||
|
|
||||||
codeunit 50467 "Rental Agreement Mgt."
|
|
||||||
{
|
|
||||||
procedure CreateAgreement()
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,8 +0,0 @@
|
||||||
namespace Contoso.Rentals;
|
|
||||||
|
|
||||||
codeunit 50466 "Rental Agreement Mgt."
|
|
||||||
{
|
|
||||||
procedure CreateAgreement()
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,26 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [23..]
|
|
||||||
domain: breaking-changes
|
|
||||||
keywords: [namespace, published-object, dependency, breaking-change, as0007, compile-time-identity]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Treat a published namespace as part of object identity
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
AL resolves an object by namespace and name. Once an app ships and dependent extensions compile against that identity, changing the namespace breaks their references even when the object name and ID stay unchanged. AppSourceCop AS0007 rejects changing the namespace of published objects; namespaces are therefore not a cosmetic folder-like label that can be reorganized after release.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Choose a globally meaningful namespace before first publication and keep it stable. Add new functional areas beneath that structure without moving existing published objects. If an identity must move, use the platform's supported move/obsoletion lifecycle rather than a source-only namespace rename.
|
|
||||||
|
|
||||||
See sample: `namespace-is-part-of-published-object-identity.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Changing `namespace Contoso.Rentals;` to `namespace Contoso.RentalManagement;` as a cleanup while leaving the object name and ID untouched. Every dependent `using` directive and qualified reference targets the old identity and stops compiling.
|
|
||||||
|
|
||||||
See sample: `namespace-is-part-of-published-object-identity.bad.al`.
|
|
||||||
|
|
@ -3,10 +3,9 @@ table 50311 "Customer Profile Bad"
|
||||||
fields
|
fields
|
||||||
{
|
{
|
||||||
field(1; "No."; Code[20]) { }
|
field(1; "No."; Code[20]) { }
|
||||||
// Breaking: the published Email field at ID 3 was renamed while retaining
|
// Breaking: the published "Email" field was renamed in place. Dependent
|
||||||
// the ID. The good example keeps Email at ID 3 and adds a separate field.
|
// extensions that reference "Email" stop compiling, and the data stored in
|
||||||
// AppSourceCop AS0005 rejects the compatibility change; retaining the ID
|
// the old column is orphaned on upgrade.
|
||||||
// does not by itself mean the stored column was dropped and re-created.
|
field(2; "Contact Email"; Text[80]) { }
|
||||||
field(3; "Contact Email"; Text[80]) { }
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -3,10 +3,10 @@ table 50310 "Customer Profile Good"
|
||||||
fields
|
fields
|
||||||
{
|
{
|
||||||
field(1; "No."; Code[20]) { }
|
field(1; "No."; Code[20]) { }
|
||||||
// Replacement is a separate field under an otherwise unused ID.
|
// Replacement field shipped alongside the old one.
|
||||||
field(2; "Contact Email"; Text[80]) { }
|
field(2; "Contact Email"; Text[80]) { }
|
||||||
// Old field keeps its original ID, name, and type and is marked Pending so
|
// Old field kept and marked Pending so dependent code keeps compiling and
|
||||||
// dependent code keeps compiling while an upgrade codeunit migrates its data.
|
// an upgrade codeunit can copy its data before it is finally removed.
|
||||||
field(3; "Email"; Text[80])
|
field(3; "Email"; Text[80])
|
||||||
{
|
{
|
||||||
ObsoleteState = Pending;
|
ObsoleteState = Pending;
|
||||||
|
|
|
||||||
|
|
@ -7,20 +7,20 @@ countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
# Obsolete published table fields instead of deleting, renaming, or renumbering them
|
# Obsolete published table fields instead of deleting or renaming them
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
A shipped table field carries both a source-level contract and persisted data. Renaming a field while retaining its ID is prohibited by AppSourceCop AS0005 and can break dependent extensions, but it is not inherently a drop-and-readd operation and should not be described as automatic data loss. Deleting the field or replacing it under a different ID is the data-loss risk: the old field storage is no longer represented unless data is migrated. The supported path is to keep the old field and obsolete it, add a replacement under a new ID, and migrate values before later removal.
|
A table field that has shipped carries two contracts at once: extensions reference it by name, and the database holds data in its column. Deleting the field, or renaming it (which the platform treats as drop-plus-add), breaks dependent code at compile time and discards the stored data — a silent data-loss event on upgrade. The fix is the same staged lifecycle used for objects: set `ObsoleteState = Pending` together with `ObsoleteReason` and an `ObsoleteTag` naming the target version, ship the new field alongside, migrate data during the window, and only switch the old field to `ObsoleteState = Removed` in a later release once nothing depends on it. LLMs often "tidy" a schema by renaming a field in place, not realizing this is both a breaking change and a data-loss risk.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Keep the old field's ID, name, and type unchanged. Add the replacement as a separate field under an unused ID, then mark the old field `ObsoleteState = Pending` with an `ObsoleteReason` that names the replacement and an `ObsoleteTag` recording the obsoletion version. Keep the old field readable so an upgrade codeunit can copy its data during the deprecation window. Move it to `ObsoleteState = Removed` only in a later release, after the window has passed and data has migrated.
|
Add the replacement field, then mark the old field `ObsoleteState = Pending` with an `ObsoleteReason` that names the replacement and an `ObsoleteTag` carrying the target version (for example `'25.0'`). Keep the obsolete field readable so an upgrade codeunit can copy its data into the new field during the deprecation window. Move it to `ObsoleteState = Removed` only in a later major version, after the window has passed and data has migrated.
|
||||||
|
|
||||||
See sample: `obsolete-table-fields-instead-of-deleting-them.good.al`.
|
See sample: `obsolete-table-fields-instead-of-deleting-them.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Renaming published `Email` to `Contact Email` with the same ID violates the compatibility contract and AS0005, even though the retained ID does not itself imply a fresh empty column. Deleting `Email` or changing its ID additionally risks losing its stored values. Detection: any previously shipped field whose name changes at the same ID, or whose original ID disappears without the unchanged field being retained as `Pending` and its data migrated to a separate replacement field.
|
Renaming the published `Email` field to `Contact Email` directly in the table — or deleting it — so dependent extensions that reference `Email` break and the column's stored values are orphaned on upgrade. Detection: a previously shipped field removed or renamed in a table or table extension with no `ObsoleteState = Pending` step preserving the original. Obsolete the field through the lifecycle instead.
|
||||||
|
|
||||||
See sample: `obsolete-table-fields-instead-of-deleting-them.bad.al`.
|
See sample: `obsolete-table-fields-instead-of-deleting-them.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -1,18 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: breaking-changes
|
|
||||||
keywords: [table-field, tableextension, relocation, field-id, obsoletestate, breaking-change, false-positive]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Relocating a field to a tableextension in the same app is not a deletion
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
Moving a field out of a base-table definition (or a base-app layer modification of one) into a tableextension that `extends` the same table, within the same app and keeping the same field ID and name, is a relocation — not a deletion or a rename. After the move the field still exists on the table: `Rec."Field Name"` and the field ID resolve exactly as before, so dependent extensions that reference the field continue to compile. Nothing in the field's public contract is removed or renamed, so the deprecation lifecycle that protects a genuinely removed field does not apply. LLM reviewers frequently misread the two-sided diff — the field disappearing from the base object and reappearing in the tableextension — as a shipped field being deleted and illegally re-added under the same ID, and demand `ObsoleteState = Pending` staging that this refactor does not need.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Recognize a field that is removed from a base table (or base-app layer) and re-declared in a tableextension of the same table, with the same field ID and name, as a same-app relocation. Do not flag it as a deleted or renamed shipped field, and do not require `ObsoleteState = Pending`, `ObsoleteReason`, `ObsoleteTag`, or a deprecation window for the move itself. The `obsolete-table-fields-instead-of-deleting-them` and `obsolete-pending-to-removed-staging` rules apply to fields that leave the table's contract entirely, not to fields relocated within the same app under an unchanged ID.
|
|
||||||
|
|
@ -1,24 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: breaking-changes
|
|
||||||
keywords: [released-baseline, unreleased, rename, renumber, obsolete, api-stability, false-positive]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Changing an unreleased symbol is not a breaking change
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
Breaking-change rules protect contracts that have already shipped to customers or are exposed to external extensions. A symbol — an object, field, key, enum value, or procedure — that is new in this app, was introduced and then changed within the same still-unreleased development cycle, or belongs to an app that has no released version yet, can be renamed, renumbered, or removed freely. There is no shipped contract to break, so the change is not a breaking change.
|
|
||||||
|
|
||||||
Release status is established from the diff, the app's `app.json` version, or a released baseline. An app whose `app.json` version has no corresponding released baseline (for example a `1.0.0.0` app that has never shipped) has no protected surface.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Before treating a rename, renumber, or removal as breaking, establish that the affected symbol was present in a released baseline. Do not flag changes to symbols that are new in the current unreleased cycle or that belong to an app with no released version. When release status cannot be established from the diff, `app.json`, or a released baseline, omit the finding rather than assert a break.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Reporting a breaking change for a rename, renumber, or removal without confirming the symbol shipped in a released version — for example flagging a break on an app whose `app.json` version has no released baseline.
|
|
||||||
|
|
@ -1,26 +0,0 @@
|
||||||
table 50441 "Source Media Bad"
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; Code; Code[20]) { }
|
|
||||||
field(10; Pictures; MediaSet) { }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
table 50442 "Target Media Bad"
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; Code; Code[20]) { }
|
|
||||||
field(20; Pictures; MediaSet) { }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
codeunit 50443 "Share Media Bad"
|
|
||||||
{
|
|
||||||
procedure CopyPictures(Source: Record "Source Media Bad"; var Target: Record "Target Media Bad")
|
|
||||||
begin
|
|
||||||
Target.Pictures := Source.Pictures;
|
|
||||||
Target.Modify(true);
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,29 +0,0 @@
|
||||||
table 50438 "Source Media Good"
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; Code; Code[20]) { }
|
|
||||||
field(10; Pictures; MediaSet) { }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
table 50439 "Target Media Good"
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; Code; Code[20]) { }
|
|
||||||
field(20; Pictures; MediaSet) { }
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
codeunit 50440 "Share Media Good"
|
|
||||||
{
|
|
||||||
procedure CopyPictures(Source: Record "Source Media Good"; var Target: Record "Target Media Good")
|
|
||||||
var
|
|
||||||
Index: Integer;
|
|
||||||
begin
|
|
||||||
for Index := 1 to Source.Pictures.Count() do
|
|
||||||
Target.Pictures.Insert(Source.Pictures.Item(Index));
|
|
||||||
Target.Modify(true);
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,26 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: data-modeling
|
|
||||||
keywords: [mediaset, media, insert, field-assignment, tenant-media, delete-integrity, sharing]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Share MediaSet items with Insert instead of field assignment
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
`Media` and `MediaSet` fields store IDs that reference tenant media system tables. When a record is deleted, the runtime looks for other references only in the same table and field index; it does not scan every table. Directly assigning a media-set field between different table types copies the ID without registering a separate media-set reference, so deleting one record can remove media that the other record still appears to reference.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
When sharing media between different tables, iterate the source `MediaSet` and call `Target.MediaSetField.Insert(Source.MediaSetField.Item(Index))`, then modify the target record. Direct field assignment is safe only when source and target are the same record subtype and use the same field ID. This concern is about reference/delete integrity, not the separate performance cost of `ModifyAll` on tables with media fields.
|
|
||||||
|
|
||||||
See sample: `share-mediaset-items-with-insert-not-field-assignment.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
`Target.Picture := Source.Picture;` where the two variables refer to different table types or different media-field IDs. The code copies an opaque ID, but the platform does not know that two independent fields now share the media object.
|
|
||||||
|
|
||||||
See sample: `share-mediaset-items-with-insert-not-field-assignment.bad.al`.
|
|
||||||
|
|
@ -1,35 +0,0 @@
|
||||||
enum 50434 "Relation Type Bad"
|
|
||||||
{
|
|
||||||
Extensible = true;
|
|
||||||
|
|
||||||
value(0; Customer) { }
|
|
||||||
}
|
|
||||||
|
|
||||||
table 50435 "Related Entity Bad"
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; Type; Enum "Relation Type Bad") { }
|
|
||||||
field(2; "Related No."; Code[20])
|
|
||||||
{
|
|
||||||
// This unconditional relation wins before extension branches run.
|
|
||||||
TableRelation = Customer;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
enumextension 50436 "Relation Type Bad Ext" extends "Relation Type Bad"
|
|
||||||
{
|
|
||||||
value(10; Resource) { }
|
|
||||||
}
|
|
||||||
|
|
||||||
tableextension 50437 "Related Entity Bad Ext" extends "Related Entity Bad"
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
modify("Related No.")
|
|
||||||
{
|
|
||||||
TableRelation = if (Type = const(Resource)) Resource;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
@ -1,37 +0,0 @@
|
||||||
enum 50430 "Relation Type Good"
|
|
||||||
{
|
|
||||||
Extensible = true;
|
|
||||||
|
|
||||||
value(0; Customer) { }
|
|
||||||
value(1; Item) { }
|
|
||||||
}
|
|
||||||
|
|
||||||
table 50431 "Related Entity Good"
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
field(1; Type; Enum "Relation Type Good") { }
|
|
||||||
field(2; "Related No."; Code[20])
|
|
||||||
{
|
|
||||||
TableRelation =
|
|
||||||
if (Type = const(Customer)) Customer
|
|
||||||
else if (Type = const(Item)) Item;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
enumextension 50432 "Relation Type Resource" extends "Relation Type Good"
|
|
||||||
{
|
|
||||||
value(10; Resource) { }
|
|
||||||
}
|
|
||||||
|
|
||||||
tableextension 50433 "Related Entity Resource" extends "Related Entity Good"
|
|
||||||
{
|
|
||||||
fields
|
|
||||||
{
|
|
||||||
modify("Related No.")
|
|
||||||
{
|
|
||||||
TableRelation = if (Type = const(Resource)) Resource;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
@ -1,26 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: data-modeling
|
|
||||||
keywords: [tablerelation, tableextension, enumextension, additive, top-down, unconditional-relation]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Design TableRelation branches for additive top-down extension
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
A `tableextension` can add to an existing `TableRelation`, but the combined relation is evaluated top-down after the original value. The first unconditional relation wins. An extension branch appended after an unconditional base relation is therefore unreachable, even though the extension compiles and appears to describe the new enum value correctly.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
When a relation is designed to follow an extensible enum, express the base cases as conditional branches and leave no unconditional catch-all ahead of future extension branches. An enum extension can then append a condition for its new value. When extending a field you do not own, inspect the original `TableRelation`; do not claim that an appended condition overrides an unconditional relation.
|
|
||||||
|
|
||||||
See sample: `table-relation-extensions-are-additive-and-top-down.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
A base field has an unconditional `TableRelation = Customer;` and a `tableextension` adds `if (Type = const(Resource)) Resource`. The original unconditional branch always wins, so the new enum value still validates and looks up against Customer. The concern is evaluation order, not `ValidateTableRelation`; free-form input is covered separately by security guidance.
|
|
||||||
|
|
||||||
See sample: `table-relation-extensions-are-additive-and-top-down.bad.al`.
|
|
||||||
|
|
@ -15,13 +15,10 @@ codeunit 50185 "Collect Errors Good Sample"
|
||||||
until Item.Next() = 0;
|
until Item.Next() = 0;
|
||||||
|
|
||||||
if HasCollectedErrors() then begin
|
if HasCollectedErrors() then begin
|
||||||
// The default is false; true retrieves and clears the collection.
|
CollectedErrors := GetCollectedErrors();
|
||||||
CollectedErrors := GetCollectedErrors(true);
|
|
||||||
// This blocking aggregate intentionally retains messages only.
|
|
||||||
foreach CollectedError in CollectedErrors do
|
foreach CollectedError in CollectedErrors do
|
||||||
ErrorText += CollectedError.Message() + '\';
|
ErrorText += CollectedError.Message() + '\';
|
||||||
Error(ErrorInfo.Create(
|
Message('The following must be fixed before posting:\%1', ErrorText);
|
||||||
StrSubstNo('The following must be fixed before posting:\%1', ErrorText), false));
|
|
||||||
end;
|
end;
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
@ -33,10 +30,8 @@ codeunit 50186 "Collect Errors Item Check"
|
||||||
trigger OnRun()
|
trigger OnRun()
|
||||||
begin
|
begin
|
||||||
if Rec.Description = '' then
|
if Rec.Description = '' then
|
||||||
Error(ErrorInfo.Create(
|
Error('Item %1 has no description.', Rec."No.");
|
||||||
StrSubstNo('Item %1 has no description.', Rec."No."), true));
|
|
||||||
if Rec."Unit Cost" <= 0 then
|
if Rec."Unit Cost" <= 0 then
|
||||||
Error(ErrorInfo.Create(
|
Error('Item %1 must have a positive unit cost.', Rec."No.");
|
||||||
StrSubstNo('Item %1 must have a positive unit cost.', Rec."No."), true));
|
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
---
|
---
|
||||||
bc-version: [19..]
|
bc-version: [all]
|
||||||
domain: error-handling
|
domain: error-handling
|
||||||
keywords: [collectible-errors, errorbehavior, collect, getcollectederrors, hascollectederrors, validation, batch]
|
keywords: [collectible-errors, errorbehavior, collect, getcollectederrors, hascollectederrors, validation, batch]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
By default a procedure stops on the first `Error`, so a user fixing ten bad rows must rerun the operation ten times. The collectible-errors feature postpones error handling to the end of the call: a procedure attributed `[ErrorBehavior(ErrorBehavior::Collect)]` keeps running as collectible errors occur and gathers them, so all failures can be presented together. `GetCollectedErrors()` returns a `List of [ErrorInfo]` for the handler to inspect, but does not clear the collection by default; pass `true` to retrieve and clear in one call, or call `ClearCollectedErrors()` explicitly after retrieving. A handler can copy record information into a custom error page as Microsoft Learn demonstrates, or deliberately format only the messages into a final blocking error as this article's sample does.
|
By default a procedure stops on the first `Error`, so a user fixing ten bad rows must rerun the operation ten times. The collectible-errors feature postpones error handling to the end of the call: a procedure attributed `[ErrorBehavior(ErrorBehavior::Collect)]` keeps running as errors occur and gathers them, so all failures can be presented together. The collected errors are read with `HasCollectedErrors()` and `GetCollectedErrors()` (which returns a `List of [ErrorInfo]`); `ClearCollectedErrors()` empties the buffer. This is a platform mechanism most LLMs are unaware of — they reach for a manually concatenated `Text` buffer or a temporary error table instead.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Mark the orchestrating procedure `[ErrorBehavior(ErrorBehavior::Collect)]` and run each item's validation so one failure doesn't abandon the rest — typically by calling the per-item routine through `Codeunit.Run`. When the run finishes, inspect `HasCollectedErrors()`, retrieve and clear the list with `GetCollectedErrors(true)`, and fail the operation with the collected messages. The sample intentionally produces a text aggregate and does not claim to retain record/field metadata in the final error. If that metadata is needed, map each `ErrorInfo` to a custom error UI before clearing, following the Microsoft Learn pattern. Do not replace validation failure with `Message`: clearing collected errors suppresses the platform failure, so the custom handler must still block the invalid operation.
|
Mark the orchestrating procedure `[ErrorBehavior(ErrorBehavior::Collect)]` and run each item's validation so one failure doesn't abandon the rest — typically by calling the per-item routine through `Codeunit.Run`. When the run finishes, inspect `HasCollectedErrors()` and surface `GetCollectedErrors()` to the user as a single list. Always handle the collected errors yourself: the platform's own guidance is that any errors still in the collected list when the procedure ends are concatenated into one dialog, which is hard for users to read.
|
||||||
|
|
||||||
See sample: `collect-validation-errors-with-errorbehavior.good.al`.
|
See sample: `collect-validation-errors-with-errorbehavior.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Three shapes signal trouble. Hand-rolled accumulation reimplements collection and prevents the handler from receiving individual `ErrorInfo` values. A `Collect` procedure that never handles the collection falls back to the concatenated platform dialog. Finally, code that calls parameterless `GetCollectedErrors()`, assumes it cleared the list, and only shows a `Message` can both leave the errors collected and allow invalid processing to continue.
|
Two shapes signal trouble. The first is hand-rolled accumulation — appending messages to a `Text` variable and showing them at the end — which reimplements the platform feature, loses each error's `ErrorInfo` structure, and skips telemetry classification. The second is applying `[ErrorBehavior(ErrorBehavior::Collect)]` but never calling `HasCollectedErrors`/`GetCollectedErrors`, so every collected error spills into the platform's concatenated end-of-procedure dialog. Detection: a `Collect` attribute with no matching `GetCollectedErrors` call, or a per-row loop that builds an error string by concatenation.
|
||||||
|
|
||||||
See sample: `collect-validation-errors-with-errorbehavior.bad.al`.
|
See sample: `collect-validation-errors-with-errorbehavior.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ codeunit 50190 "Error Type Good Sample"
|
||||||
if not BucketInitialized(BucketId) then begin
|
if not BucketInitialized(BucketId) then begin
|
||||||
InternalErr.ErrorType := ErrorType::Internal;
|
InternalErr.ErrorType := ErrorType::Internal;
|
||||||
InternalErr.Message := StrSubstNo('Ledger bucket %1 was not initialized before posting.', BucketId);
|
InternalErr.Message := StrSubstNo('Ledger bucket %1 was not initialized before posting.', BucketId);
|
||||||
|
InternalErr.DetailedMessage := 'Internal invariant violated. Inspect the call stack captured in telemetry.';
|
||||||
Error(InternalErr);
|
Error(InternalErr);
|
||||||
end;
|
end;
|
||||||
end;
|
end;
|
||||||
|
|
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
---
|
---
|
||||||
bc-version: [14..]
|
bc-version: [all]
|
||||||
domain: error-handling
|
domain: error-handling
|
||||||
keywords: [errorinfo, errortype, internal, client, telemetry, diagnostics, generic-message]
|
keywords: [errorinfo, errortype, internal, client, telemetry, diagnostics, generic-message]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
|
|
@ -15,7 +15,7 @@ application-area: [all]
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Reserve `ErrorType::Internal` for errors the user cannot act on: corrupted internal state, an unreachable branch, a contract a caller violated. Set a precise, detail-rich `Message` for telemetry, raise it via `Error(ErrorInfo)`, and let the platform show the user a generic dialog. Keep `ErrorType::Client` (or a plain `Error`) for failures the user is expected to read and resolve — validation messages, missing setup, business-rule violations. The test is simple: if the message only makes sense to a developer, mark it `Internal`.
|
Reserve `ErrorType::Internal` for errors the user cannot act on: corrupted internal state, an unreachable branch, a contract a caller violated. Set a precise, detail-rich `Message` and `DetailedMessage` for telemetry, raise it via `Error(ErrorInfo)`, and let the platform show the user a generic dialog. Keep `ErrorType::Client` (or a plain `Error`) for failures the user is expected to read and resolve — validation messages, missing setup, business-rule violations. The test is simple: if the message only makes sense to a developer, mark it `Internal`.
|
||||||
|
|
||||||
See sample: `errortype-internal-vs-client-for-diagnostics.good.al`.
|
See sample: `errortype-internal-vs-client-for-diagnostics.good.al`.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,17 +0,0 @@
|
||||||
codeunit 50301 "Try Return Bad"
|
|
||||||
{
|
|
||||||
procedure ImportDocument()
|
|
||||||
begin
|
|
||||||
// Ignoring the Boolean result makes this an ordinary, throwing call.
|
|
||||||
TryImportDocument();
|
|
||||||
end;
|
|
||||||
|
|
||||||
[TryFunction]
|
|
||||||
local procedure TryImportDocument()
|
|
||||||
begin
|
|
||||||
Error(SourceRejectedErr);
|
|
||||||
end;
|
|
||||||
|
|
||||||
var
|
|
||||||
SourceRejectedErr: Label 'The source document was rejected.';
|
|
||||||
}
|
|
||||||
|
|
@ -1,18 +0,0 @@
|
||||||
codeunit 50300 "Try Return Good"
|
|
||||||
{
|
|
||||||
procedure ImportDocument()
|
|
||||||
begin
|
|
||||||
if not TryImportDocument() then
|
|
||||||
Error(ImportFailedErr);
|
|
||||||
end;
|
|
||||||
|
|
||||||
[TryFunction]
|
|
||||||
local procedure TryImportDocument()
|
|
||||||
begin
|
|
||||||
Error(SourceRejectedErr);
|
|
||||||
end;
|
|
||||||
|
|
||||||
var
|
|
||||||
ImportFailedErr: Label 'The document could not be imported.';
|
|
||||||
SourceRejectedErr: Label 'The source document was rejected.';
|
|
||||||
}
|
|
||||||
|
|
@ -1,30 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [13..]
|
|
||||||
domain: error-handling
|
|
||||||
keywords: [tryfunction, try-method, boolean-return, ignored-return-value, error-propagation]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Consume a TryFunction return value to enable try semantics
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
A procedure marked `[TryFunction]` catches errors only when the caller uses its Boolean return value. An assignment or conditional makes the invocation a try-method call; a bare call is treated as an ordinary procedure call and exposes errors as usual. The attribute alone does not make every invocation non-throwing.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Consume the result directly: assign it to a Boolean or use the call in an `if` condition. Handle `false` immediately while the last-error state still describes that failure.
|
|
||||||
|
|
||||||
See sample: `ignored-tryfunction-return-disables-try-semantics.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Calling a `[TryFunction]` procedure as a standalone statement and assuming the attribute suppresses its errors. The call has ordinary error semantics because its Boolean result is ignored.
|
|
||||||
|
|
||||||
See sample: `ignored-tryfunction-return-disables-try-semantics.bad.al`.
|
|
||||||
|
|
||||||
## See also
|
|
||||||
|
|
||||||
`microsoft/knowledge/performance/use-tryfunction-for-error-catching-not-rollback.md` owns transaction rollback expectations after a try method has actually caught an error.
|
|
||||||
|
|
@ -1,24 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: error-handling
|
|
||||||
keywords: [oninsertrecord, onmodifyrecord, ondeleterecord, onquerypage, boolean-trigger, exit, false-positive]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Page record triggers return true by default; a missing exit(true) does not block the operation
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
The Boolean page record triggers `OnInsertRecord`, `OnModifyRecord`, `OnDeleteRecord`, and `OnQueryClosePage` return `true` by default. When the trigger body omits an explicit return value, the platform treats the result as `true` and the operation proceeds. Only an explicit `exit(false)` — or a reachable code path that returns `false` — cancels the insert, modify, delete, or page close.
|
|
||||||
|
|
||||||
This is a defined exception to the ordinary Boolean method rule, where the default return is `false`. Reviewers unfamiliar with the exception sometimes read a page record trigger that has no `exit(true)` and conclude the operation is blocked; it is not.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Do not claim that a missing `exit(true)` blocks or prevents an insert, modify, or delete, and do not recommend adding `exit(true)` "to let the operation proceed" — that is already the default. Evaluate these triggers only for an explicit or reachable `exit(false)`/false-returning path that would cancel the operation unintentionally.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Flagging `OnInsertRecord`, `OnModifyRecord`, `OnDeleteRecord`, or `OnQueryClosePage` as defective because it "does not return `true`", or asserting that inserts/modifies/deletes will silently fail without an explicit `exit(true)`. The default return already permits the operation.
|
|
||||||
|
|
@ -1,26 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: error-handling
|
|
||||||
keywords: [get, record-not-found, runtime-error, return-value, boolean-method, false-positive]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# An unchecked Record.Get raises an error when the record is missing; it is not silently ignored
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
`Record.Get` returns a Boolean, but its behavior when no record is found depends on whether the return value is consumed. When the return value is used — inside `if Rec.Get(...) then`, or assigned to a variable — a missing record yields `false` and execution continues. When `Rec.Get(...)` is called as a bare statement and the return value is not used, the platform raises a runtime "record not found" error if the record does not exist. A bare `Rec.Get(Key)` therefore acts as an assertion that the record exists: it does not swallow or silently ignore a missing record. This mirrors other AL find methods, where an unconsumed return value lets the platform enforce the not-found error.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Do not claim that a `Record.Get` whose return value is unused silently ignores a missing record or hides an error. Treat a bare `Rec.Get(...)` statement as an intentional existence assertion that already throws when the record is absent. Recommend an explicit existence check only when the surrounding logic must continue gracefully rather than error out.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Flagging a bare `Rec.Get(Key)` statement as a defect because "the return value is ignored, so a missing record is swallowed", or recommending it be wrapped in `if Rec.Get(...) then ... else Error(...)` to "handle the not-found case" — the unchecked call already raises an error when the record is missing.
|
|
||||||
|
|
||||||
## See also
|
|
||||||
|
|
||||||
- `ignored-tryfunction-return-disables-try-semantics.md` — a different case where ignoring a Boolean return value changes behavior.
|
|
||||||
|
|
@ -1,14 +1,21 @@
|
||||||
// Demonstration-only AL. Version 1 exposed PostDocument(SalesHeader).
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
codeunit 50251 "Param Append Bad Sample"
|
codeunit 50251 "Param Append Bad Sample"
|
||||||
{
|
{
|
||||||
procedure PostDocument(var SalesHeader: Record "Sales Header")
|
procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
||||||
|
var
|
||||||
|
IsHandled: Boolean;
|
||||||
begin
|
begin
|
||||||
// Existing callers cannot supply the newly required argument.
|
IsHandled := false;
|
||||||
OnBeforePostDocument(SalesHeader);
|
// Anti-pattern: 'CalledFromBatch' was inserted before the existing
|
||||||
|
// IsHandled parameter, shifting it and breaking the argument positions
|
||||||
|
// every existing subscriber relied on.
|
||||||
|
OnBeforePostDocument(SalesHeader, CalledFromBatch, IsHandled);
|
||||||
|
if IsHandled then
|
||||||
|
exit;
|
||||||
end;
|
end;
|
||||||
|
|
||||||
[IntegrationEvent(false, false)]
|
[IntegrationEvent(false, false)]
|
||||||
procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean; var IsHandled: Boolean)
|
||||||
begin
|
begin
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
// Demonstration-only AL. Version 1 had SalesHeader and IsHandled parameters.
|
// Demonstration-only AL. Not compiled by CI; illustrates the article.
|
||||||
codeunit 50250 "Param Append Good Sample"
|
codeunit 50250 "Param Append Good Sample"
|
||||||
{
|
{
|
||||||
procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
procedure PostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean)
|
||||||
|
|
@ -6,24 +6,15 @@ codeunit 50250 "Param Append Good Sample"
|
||||||
IsHandled: Boolean;
|
IsHandled: Boolean;
|
||||||
begin
|
begin
|
||||||
IsHandled := false;
|
IsHandled := false;
|
||||||
// Subscribers bind by name, so the new parameter can sit between the
|
// The new 'CalledFromBatch' parameter was appended at the end of the
|
||||||
// existing parameters without breaking subscribers that omit it.
|
// existing signature, so existing subscribers needed no re-mapping.
|
||||||
OnBeforePostDocument(SalesHeader, CalledFromBatch, IsHandled);
|
OnBeforePostDocument(SalesHeader, IsHandled, CalledFromBatch);
|
||||||
if IsHandled then
|
if IsHandled then
|
||||||
exit;
|
exit;
|
||||||
end;
|
end;
|
||||||
|
|
||||||
[IntegrationEvent(false, false)]
|
[IntegrationEvent(false, false)]
|
||||||
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; CalledFromBatch: Boolean; var IsHandled: Boolean)
|
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean; CalledFromBatch: Boolean)
|
||||||
begin
|
begin
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
||||||
codeunit 50252 "Existing Param Subscriber"
|
|
||||||
{
|
|
||||||
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Param Append Good Sample", 'OnBeforePostDocument', '', false, false)]
|
|
||||||
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean)
|
|
||||||
begin
|
|
||||||
IsHandled := SalesHeader."No." = '';
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
|
||||||
|
|
@ -1,26 +1,26 @@
|
||||||
---
|
---
|
||||||
bc-version: [all]
|
bc-version: [all]
|
||||||
domain: events
|
domain: events
|
||||||
keywords: [event-parameters, signature, backward-compatibility, public-event, local-event, internal-event, appsourcecop, as0024, as0025]
|
keywords: [event-parameters, signature, backward-compatibility, append, onbefore, integration-event, versioning]
|
||||||
technologies: [al]
|
technologies: [al]
|
||||||
countries: [w1]
|
countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
# Event parameter additions depend on publisher access, not position
|
# Add new event parameters at the end
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
Event subscribers bind publisher parameters by name and can omit parameters they do not use. A `local` or `internal` Business or Integration event can therefore gain a parameter at any position without breaking subscriber-only consumers; appending is not a compatibility requirement. A public event is also a public procedure that dependent extensions can raise, so adding a required parameter anywhere breaks callers under AppSourceCop AS0024.
|
Adding a parameter to an existing event publisher changes its signature. Appending the new parameter at the end of the parameter list keeps the change easy to review and track: existing subscribers still bind to the leading parameters, and the diff is a single clean addition. Inserting a parameter in the middle makes diffs noisy and harder to review, and obscures the history of how the signature evolved. New parameters belong after the existing ones.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Add a parameter directly only when the shipped event publisher is `local` or `internal`. Place it where the signature is clearest; existing subscribers continue binding the parameters they name. For a public event, keep the original publisher unchanged and introduce a new event with the expanded contract.
|
When extending an existing publisher, append the new parameter after all existing ones, including after a trailing `var IsHandled: Boolean` when present. Subscribers that already match keep working against the leading parameters, and the change stays a one-line addition that is trivial to review.
|
||||||
|
|
||||||
See sample: `add-new-event-parameters-at-the-end.good.al`.
|
See sample: `add-new-event-parameters-at-the-end.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Appending a parameter to a public event and assuming its position makes the change compatible. Existing external callers still lack the new required argument. Conversely, do not flag a parameter inserted among existing parameters on a `local` or `internal` Business or Integration event merely because it was not appended.
|
Inserting a new parameter in the middle of an existing event's signature, shifting every subsequent parameter and making the change noisy and harder to review. Detection: a changed event signature where an added parameter appears before existing parameters rather than at the tail of the list.
|
||||||
|
|
||||||
See sample: `add-new-event-parameters-at-the-end.bad.al`.
|
See sample: `add-new-event-parameters-at-the-end.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -1,18 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: events
|
|
||||||
keywords: [event-parameters, signature, subscriber-binding, backward-compatibility, integration-event, breaking-change, false-positive]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Adding a parameter to an event is not a breaking change
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
Adding a parameter to an existing event publisher does not break existing subscribers. AL binds a subscriber to a publisher by the event name, and the subscriber's parameter list only has to be a subset of the publisher's, matched by name and type. A subscriber that does not declare the new parameter keeps compiling and keeps binding — it simply ignores the addition. This holds for `IntegrationEvent` and `BusinessEvent` publishers, and even more plainly for `local` events. Appending the new parameter at the end keeps the change a clean, reviewable addition (see `add-new-event-parameters-at-the-end`). LLM reviewers often misreport the mere presence of a new event parameter as a "breaking event signature change" that breaks subscribers, which is incorrect.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Do not flag the addition of a parameter to an event publisher as a breaking or signature-breaking change, and do not claim it breaks existing subscribers. Genuine, separate concerns are covered by their own rules — a parameter inserted in the middle of the list rather than appended (`add-new-event-parameters-at-the-end`), or a parameter that carries no meaningful value — and should be raised on those grounds, not framed as a backward-compatibility break.
|
|
||||||
|
|
@ -8,9 +8,9 @@ codeunit 50291 "New OnBefore Bad Sample"
|
||||||
begin
|
begin
|
||||||
Total := 100;
|
Total := 100;
|
||||||
|
|
||||||
// Anti-pattern: IsHandled was bolted onto the existing OnAfter event.
|
// Anti-pattern: IsHandled was bolted onto the existing
|
||||||
// Regardless of compiler compatibility, this changes a notification
|
// OnAfterCalculateTotal, changing its contract and breaking every
|
||||||
// into an override contract that existing subscribers did not expect.
|
// subscriber that matched the original signature.
|
||||||
OnAfterCalculateTotal(SalesHeader, Total, IsHandled);
|
OnAfterCalculateTotal(SalesHeader, Total, IsHandled);
|
||||||
end;
|
end;
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,14 +0,0 @@
|
||||||
// Demonstration-only AL. Version 1 used [IntegrationEvent(true, true, false)].
|
|
||||||
codeunit 50531 "Shipment Events Bad"
|
|
||||||
{
|
|
||||||
procedure NotifyShipment(ShipmentNo: Code[20])
|
|
||||||
begin
|
|
||||||
OnShipmentCreated(ShipmentNo);
|
|
||||||
end;
|
|
||||||
|
|
||||||
// Version 2 mutates all three contract-significant arguments in place.
|
|
||||||
[IntegrationEvent(false, false, true)]
|
|
||||||
local procedure OnShipmentCreated(ShipmentNo: Code[20])
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,21 +0,0 @@
|
||||||
// Demonstration-only AL. The Isolated argument requires runtime 9.0 / BC20.
|
|
||||||
codeunit 50530 "Shipment Events"
|
|
||||||
{
|
|
||||||
procedure NotifyShipment(ShipmentNo: Code[20])
|
|
||||||
begin
|
|
||||||
OnShipmentCreated(ShipmentNo);
|
|
||||||
OnShipmentCreatedIsolated(ShipmentNo);
|
|
||||||
end;
|
|
||||||
|
|
||||||
// Preserve the shipped attribute contract.
|
|
||||||
[IntegrationEvent(true, true, false)]
|
|
||||||
local procedure OnShipmentCreated(ShipmentNo: Code[20])
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
|
|
||||||
// Publish a new event for different isolation and sender semantics.
|
|
||||||
[IntegrationEvent(false, false, true)]
|
|
||||||
local procedure OnShipmentCreatedIsolated(ShipmentNo: Code[20])
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,26 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: events
|
|
||||||
keywords: [event-attribute, includesender, globalvaraccess, isolated-event, compatibility, integration-event, business-event, appsourcecop, as0021, as0101]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Do not change shipped event attribute flags
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
`IncludeSender` and, on Integration events, `GlobalVarAccess` have been event-contract flags since runtime 1.0. Removing sender or global access breaks subscribers, so AppSourceCop AS0021 prevents changing those flags from `true` to `false`. On runtime 9.0 and later (Business Central 2022 release wave 1, BC20), `Isolated` also controls transaction, error, and rollback behavior; AS0101 prevents adding, removing, or changing that argument.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Keep every available attribute argument exactly as shipped. If new subscribers need different sender/global exposure, publish a new event with the desired flags. Apply the same rule to `Isolated` only on BC20 or later, where that argument exists. Raise both events while the original contract is supported, and choose preferred flags only when designing a new event.
|
|
||||||
|
|
||||||
See sample: `do-not-change-shipped-event-attribute-flags.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Changing a shipped event's `IncludeSender` or `GlobalVarAccess` to modernize its design, including replacing `IncludeSender` with an explicit parameter. On BC20 or later, adding, removing, or toggling `Isolated` is equally contract-significant. Even a change that leaves old subscribers compiling can alter observable execution or exposure; version the event instead.
|
|
||||||
|
|
||||||
See sample: `do-not-change-shipped-event-attribute-flags.bad.al`.
|
|
||||||
|
|
@ -8,13 +8,13 @@ codeunit 50260 "Reuse Event Good Sample"
|
||||||
IsHandled := false;
|
IsHandled := false;
|
||||||
// A single event, extended with CustomerNo appended at the end, covers
|
// A single event, extended with CustomerNo appended at the end, covers
|
||||||
// the need; no second event is raised beside it.
|
// the need; no second event is raised beside it.
|
||||||
OnBeforeProcessOrder(SalesHeader, IsHandled, CustomerNo);
|
OnBeforeProcessOrder(SalesHeader, CustomerNo, IsHandled);
|
||||||
if IsHandled then
|
if IsHandled then
|
||||||
exit;
|
exit;
|
||||||
end;
|
end;
|
||||||
|
|
||||||
[IntegrationEvent(false, false)]
|
[IntegrationEvent(false, false)]
|
||||||
local procedure OnBeforeProcessOrder(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean; CustomerNo: Code[20])
|
local procedure OnBeforeProcessOrder(var SalesHeader: Record "Sales Header"; CustomerNo: Code[20]; var IsHandled: Boolean)
|
||||||
begin
|
begin
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -7,20 +7,20 @@ countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
# Prefer this over IncludeSender in new codeunit events
|
# Prefer this over IncludeSender in codeunit events
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
When designing a new publisher, setting `IncludeSender` to `true` on `[IntegrationEvent]` or `[BusinessEvent]` gives subscribers the publishing object as an implicit sender parameter. From Business Central 2024 release wave 2, a codeunit can instead pass itself explicitly with the `this` keyword as a normal, strongly-typed `Sender` parameter. Explicit passing makes the sender visible and typed in the signature. This is new-event design guidance only: never change `IncludeSender` on an event that has already shipped.
|
Some publishers set `IncludeSender` to `true` on `[IntegrationEvent]` or `[BusinessEvent]` so subscribers receive the publishing object as an implicit sender parameter. From Business Central 2024 release wave 2, a codeunit can instead pass itself explicitly with the `this` keyword as a normal, strongly-typed `Sender` parameter. Explicit passing is clearer at both the publisher and the subscriber: the sender appears in the signature, it is concretely typed to the publishing codeunit, and it avoids the implicit-parameter mechanics of `IncludeSender`. Reserve `IncludeSender = true` for cases where the sender genuinely cannot be passed explicitly. This guidance applies to code targeting Business Central 2024 release wave 2 or later, where the `this` keyword is available.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
For a new event, declare the publisher `[IntegrationEvent(false, false)]` with an explicit `Sender: Codeunit "…"` parameter and raise it with `this`, for example `OnBeforeProcessOrder(OrderNo, this);`. Subscribers then receive a typed sender they can call directly.
|
Declare the publisher `[IntegrationEvent(false, false)]` with an explicit `Sender: Codeunit "…"` parameter and raise it with `this`, for example `OnBeforeProcessOrder(OrderNo, this);`. Subscribers then receive a typed sender they can call directly.
|
||||||
|
|
||||||
See sample: `prefer-this-over-includesender-in-codeunit-events.good.al`.
|
See sample: `prefer-this-over-includesender-in-codeunit-events.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Designing a new codeunit event with `[IntegrationEvent(true, …)]` solely to hand subscribers the publisher instance, where `this` could be passed explicitly as a typed parameter. Do not apply this rule by mutating a shipped event's attribute flags.
|
Relying on `[IntegrationEvent(true, …)]` solely to hand subscribers the publisher instance, where a codeunit could pass `this` explicitly as a typed parameter. Detection: `IncludeSender = true` on a codeunit event whose only purpose is to expose the sender, in code targeting Business Central 2024 release wave 2 or later.
|
||||||
|
|
||||||
See sample: `prefer-this-over-includesender-in-codeunit-events.bad.al`.
|
See sample: `prefer-this-over-includesender-in-codeunit-events.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -20,14 +20,13 @@ codeunit 50225 "Reservation Post Good Sample"
|
||||||
var
|
var
|
||||||
IsHandled: Boolean;
|
IsHandled: Boolean;
|
||||||
begin
|
begin
|
||||||
IsHandled := false;
|
|
||||||
OnBeforeReserve(ReservationEntry, IsHandled);
|
OnBeforeReserve(ReservationEntry, IsHandled);
|
||||||
if not IsHandled then begin
|
if IsHandled then
|
||||||
ReservationEntry.Reserved := true;
|
exit;
|
||||||
ReservationEntry.Modify(true);
|
|
||||||
end;
|
ReservationEntry.Reserved := true;
|
||||||
|
ReservationEntry.Modify(true);
|
||||||
|
|
||||||
// OnAfter reports completion whether a subscriber or the base body handled it.
|
|
||||||
OnAfterReserve(ReservationEntry);
|
OnAfterReserve(ReservationEntry);
|
||||||
end;
|
end;
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,15 +0,0 @@
|
||||||
// Demonstration-only AL. Version 1 exposed var Score as an Integer.
|
|
||||||
codeunit 50521 "Customer Scoring Events Bad"
|
|
||||||
{
|
|
||||||
procedure ScoreCustomer(CustomerNo: Code[20]; ScoreText: Text)
|
|
||||||
begin
|
|
||||||
OnCustomerScored(CustomerNo, ScoreText);
|
|
||||||
end;
|
|
||||||
|
|
||||||
// 'local' limits raising, not subscription. Renaming Score to ScoreText,
|
|
||||||
// changing its type, and removing var all break existing subscribers.
|
|
||||||
[IntegrationEvent(false, false)]
|
|
||||||
local procedure OnCustomerScored(CustomerNo: Code[20]; ScoreText: Text)
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,24 +0,0 @@
|
||||||
// Demonstration-only AL. Version 1 had CustomerNo and var Score parameters.
|
|
||||||
codeunit 50520 "Customer Scoring Events"
|
|
||||||
{
|
|
||||||
procedure ScoreCustomer(CustomerNo: Code[20]; Reason: Text; var Score: Integer)
|
|
||||||
begin
|
|
||||||
OnCustomerScored(CustomerNo, Reason, Score);
|
|
||||||
end;
|
|
||||||
|
|
||||||
// Adding Reason between existing parameters preserves subscriber bindings.
|
|
||||||
[IntegrationEvent(false, false)]
|
|
||||||
local procedure OnCustomerScored(CustomerNo: Code[20]; Reason: Text; var Score: Integer)
|
|
||||||
begin
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
||||||
codeunit 50522 "Existing Scoring Subscriber"
|
|
||||||
{
|
|
||||||
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Customer Scoring Events", 'OnCustomerScored', '', false, false)]
|
|
||||||
local procedure OnCustomerScored(CustomerNo: Code[20]; var Score: Integer)
|
|
||||||
begin
|
|
||||||
if CustomerNo = '' then
|
|
||||||
Score := 0;
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,26 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: events
|
|
||||||
keywords: [local-event, internal-event, event-subscriber, compatibility, access-modifier, integration-event, business-event, parameter-name, var-parameter, appsourcecop]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Treat local and internal events as subscriber contracts
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
The `local` and `internal` access modifiers on Business and Integration event publishers restrict who can raise the procedure; they do not prevent dependent extensions from subscribing. Once shipped, the event name and each existing parameter's name, type/subtype, and value-versus-`var` passing mode are compatibility contracts even when the publisher is not public. Parameter order is not a subscriber contract because subscribers bind the parameters they use by name. This differs from `[InternalEvent]`, which is module-only except for modules named by `internalsVisibleTo`.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Preserve a shipped Business or Integration event's identity and every existing parameter's name, type/subtype, and passing mode regardless of the procedure access modifier. AS0025 protects names and types, while AS0063 and AS0077 protect removal and addition of `var`. New parameters may be added at any position on a `local` or `internal` event because subscribers can omit them; public event procedures follow the stricter caller contract described by `add-new-event-parameters-at-the-end`.
|
|
||||||
|
|
||||||
See sample: `treat-local-and-internal-events-as-subscriber-contracts.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Renaming or removing an existing parameter, changing its type/subtype, or adding/removing its `var` modifier because the event publisher procedure is `local` or `internal`. AppSourceCop checks these subscriber-breaking changes because dependent event subscribers can still bind to the event. Reordering unchanged parameters, or inserting a new parameter among them, is not this anti-pattern.
|
|
||||||
|
|
||||||
See sample: `treat-local-and-internal-events-as-subscriber-contracts.bad.al`.
|
|
||||||
|
|
@ -7,7 +7,6 @@ codeunit 50220 "Shipping Charge Good Sample"
|
||||||
begin
|
begin
|
||||||
// Give extensions a sanctioned seam to replace the calculation, then
|
// Give extensions a sanctioned seam to replace the calculation, then
|
||||||
// skip the default logic when a subscriber has handled it.
|
// skip the default logic when a subscriber has handled it.
|
||||||
IsHandled := false;
|
|
||||||
OnBeforeCalculateShippingCharge(OrderAmount, Charge, IsHandled);
|
OnBeforeCalculateShippingCharge(OrderAmount, Charge, IsHandled);
|
||||||
if IsHandled then
|
if IsHandled then
|
||||||
exit(Charge);
|
exit(Charge);
|
||||||
|
|
|
||||||
|
|
@ -1,16 +0,0 @@
|
||||||
// Demonstration-only AL. Version 1 shipped with only CalculateAmount().
|
|
||||||
interface "I Shipping Quote Bad"
|
|
||||||
{
|
|
||||||
procedure CalculateAmount(): Decimal;
|
|
||||||
|
|
||||||
// Added in version 2: every existing implementer now fails to compile.
|
|
||||||
procedure CalculateDeliveryDate(): Date;
|
|
||||||
}
|
|
||||||
|
|
||||||
codeunit 50511 "Existing Shipping Quote" implements "I Shipping Quote Bad"
|
|
||||||
{
|
|
||||||
procedure CalculateAmount(): Decimal
|
|
||||||
begin
|
|
||||||
exit(10);
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,23 +0,0 @@
|
||||||
// Demonstration-only AL. Interface inheritance requires runtime 14.0 / BC25.
|
|
||||||
interface "I Shipping Quote"
|
|
||||||
{
|
|
||||||
procedure CalculateAmount(): Decimal;
|
|
||||||
}
|
|
||||||
|
|
||||||
interface "I Shipping Quote V2" extends "I Shipping Quote"
|
|
||||||
{
|
|
||||||
procedure CalculateDeliveryDate(): Date;
|
|
||||||
}
|
|
||||||
|
|
||||||
codeunit 50510 "Shipping Quote V2" implements "I Shipping Quote V2"
|
|
||||||
{
|
|
||||||
procedure CalculateAmount(): Decimal
|
|
||||||
begin
|
|
||||||
exit(10);
|
|
||||||
end;
|
|
||||||
|
|
||||||
procedure CalculateDeliveryDate(): Date
|
|
||||||
begin
|
|
||||||
exit(Today() + 1);
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Add table
Add a link
Reference in a new issue