mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Merge remote-tracking branch 'origin/main' into pr49-review-fixes
This commit is contained in:
commit
28041e80e1
111 changed files with 2331 additions and 65 deletions
15
.claude-plugin/marketplace.json
Normal file
15
.claude-plugin/marketplace.json
Normal file
|
|
@ -0,0 +1,15 @@
|
|||
{
|
||||
"name": "bcquality",
|
||||
"owner": {
|
||||
"name": "microsoft/BCQuality",
|
||||
"url": "https://github.com/microsoft/BCQuality"
|
||||
},
|
||||
"plugins": [
|
||||
{
|
||||
"name": "bcquality",
|
||||
"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.",
|
||||
"version": "0.1.0"
|
||||
}
|
||||
]
|
||||
}
|
||||
9
.claude-plugin/plugin.json
Normal file
9
.claude-plugin/plugin.json
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
{
|
||||
"name": "bcquality",
|
||||
"description": "Quality skills and knowledge for Business Central development. Exposes a review bridge skill that drives the BCQuality Entry protocol over the installed knowledge base.",
|
||||
"version": "0.1.0",
|
||||
"author": {
|
||||
"name": "microsoft/BCQuality",
|
||||
"url": "https://github.com/microsoft/BCQuality"
|
||||
}
|
||||
}
|
||||
31
.github/custom-layer-autoclose.md
vendored
Normal file
31
.github/custom-layer-autoclose.md
vendored
Normal file
|
|
@ -0,0 +1,31 @@
|
|||
Hey @{{AUTHOR}} 👋
|
||||
|
||||
First off — thank you for jumping in and experimenting! It's awesome to see people pushing on the framework. 🎉
|
||||
|
||||
That said, let me gently redirect you, because I think there's a small but important misunderstanding about how the `custom` layer is meant to work:
|
||||
|
||||
The `custom` layer in *this* repo isn't a destination for PRs — it's the designated sandbox inside **your own fork**. Think of it as the "your timeline" branch of the multiverse 🌌: this repo is canon, your fork is where you get to remix the lore without needing anyone's approval. That's the whole point of the layer existing — so you *don't* have to upstream your team-specific or experimental work.
|
||||
|
||||
The intended workflow is:
|
||||
|
||||
1. 🍴 **Fork** BCQuality to your own GitHub account
|
||||
2. Clone *your fork* locally
|
||||
3. Drop your custom agents and knowledge into the `custom` layer **there**
|
||||
4. Commit and push to your fork — no PR back to upstream needed for custom stuff
|
||||
|
||||
That way you get full control, your changes survive upstream updates cleanly, and you can pull in new core releases from this repo whenever you want. ✨
|
||||
|
||||
**Now — here's the fun part:** if while building out your fork you discover knowledge, patterns, or agents that you think would genuinely benefit *everyone* using BCQuality (not just your team), that's exactly what the `/community` layer is for! 🌟 PRs to `/community` here in the upstream repo are absolutely welcome and encouraged — it's how the collective hive mind 🧠 levels up. So please: tinker in your fork, and when you strike gold that's worth sharing, send it our way via `/community`.
|
||||
|
||||
Going to close this PR for now (since it's targeting `custom` rather than `/community`), but please don't read it as a "no" — it's a "yes, but let's route it correctly." 🙏 Happy to help if you hit any snags spinning up your fork, and genuinely looking forward to seeing what you contribute to `/community` down the line.
|
||||
|
||||
<details>
|
||||
<summary>Files in this PR that triggered the auto-close</summary>
|
||||
|
||||
{{FILES}}
|
||||
</details>
|
||||
|
||||
May your merges be conflict-free. 🚀
|
||||
|
||||
---
|
||||
<sub>🤖 This PR was closed automatically by the `Guard custom layer` workflow because it adds or changes content under `/custom/`. If you were only updating the template (`custom/README.md` or a `.gitkeep`), a maintainer can re-open it. If you think this was closed in error, just comment here.</sub>
|
||||
10
.github/new-top-level-flag.md
vendored
Normal file
10
.github/new-top-level-flag.md
vendored
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
<!-- guard:new-top-level -->
|
||||
👋 Heads up @{{AUTHOR}} — and cc maintainers — this PR introduces **new top-level entries** that aren't part of BCQuality's known repository structure:
|
||||
|
||||
{{ENTRIES}}
|
||||
|
||||
This isn't a block — just a flag. 🚩 New top-level folders and files are *usually* unintended (a stray export, a tool's scratch dir, or content that meant to land inside an existing layer like `/community/knowledge/`). BCQuality keeps a deliberately small root: `.github/`, `community/`, `custom/`, `microsoft/`, `skills/`, and `tools/`, plus a handful of root docs.
|
||||
|
||||
**If this was intentional** and the new entry genuinely belongs at the repo root, a maintainer can review and merge as normal — no action needed beyond a quick sanity check. **If it wasn't**, please move the content into the right existing layer (or drop it) and push an update. 🙏
|
||||
|
||||
A maintainer will take a look before merging.
|
||||
108
.github/workflows/flag-new-top-level.yml
vendored
Normal file
108
.github/workflows/flag-new-top-level.yml
vendored
Normal file
|
|
@ -0,0 +1,108 @@
|
|||
name: Flag new top-level entries
|
||||
|
||||
# BCQuality keeps a deliberately small repository root. New top-level folders
|
||||
# or files are almost always unintended — a stray export, a tool's scratch
|
||||
# directory, or content that meant to land inside an existing layer (e.g.
|
||||
# /community/knowledge/). PR #55 leaked exactly this kind of stray folder.
|
||||
#
|
||||
# Unlike the custom-layer guard, this workflow does NOT close the PR. It only
|
||||
# posts a single advisory comment so a maintainer (and the author) can eyeball
|
||||
# the addition. It reads the PR's file LIST via the API and never checks out or
|
||||
# runs PR code.
|
||||
|
||||
on:
|
||||
pull_request_target:
|
||||
types: [opened, reopened, synchronize]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: write
|
||||
issues: write
|
||||
|
||||
jobs:
|
||||
flag:
|
||||
if: github.repository == 'microsoft/BCQuality'
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Check out repository
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
sparse-checkout: |
|
||||
.github/new-top-level-flag.md
|
||||
sparse-checkout-cone-mode: false
|
||||
|
||||
- name: Flag unexpected new top-level entries
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
const fs = require('fs');
|
||||
|
||||
// Known, intended repository root. Anything else added at the root
|
||||
// is flagged for a human to eyeball.
|
||||
const ALLOWED_DIRS = new Set([
|
||||
'.claude-plugin', '.github', 'community', 'custom', 'microsoft', 'skills', 'tools',
|
||||
]);
|
||||
const ALLOWED_FILES = new Set([
|
||||
'.gitignore', 'CODEOWNERS', 'LICENSE', 'README.md',
|
||||
'SECURITY.md', 'agent-consumption.md',
|
||||
]);
|
||||
|
||||
const MARKER = '<!-- guard:new-top-level -->';
|
||||
const { owner, repo } = context.repo;
|
||||
const prNumber = context.payload.pull_request.number;
|
||||
|
||||
const files = await github.paginate(github.rest.pulls.listFiles, {
|
||||
owner, repo, pull_number: prNumber, per_page: 100,
|
||||
});
|
||||
|
||||
// Only consider newly-added paths — a new top-level entry can only
|
||||
// appear via an added file.
|
||||
const added = files
|
||||
.filter((f) => f.status === 'added')
|
||||
.map((f) => f.filename);
|
||||
|
||||
const newDirs = new Set();
|
||||
const newFiles = new Set();
|
||||
for (const p of added) {
|
||||
const slash = p.indexOf('/');
|
||||
if (slash === -1) {
|
||||
// Top-level file.
|
||||
if (!ALLOWED_FILES.has(p)) newFiles.add(p);
|
||||
} else {
|
||||
// Top-level directory.
|
||||
const dir = p.slice(0, slash);
|
||||
if (!ALLOWED_DIRS.has(dir)) newDirs.add(dir);
|
||||
}
|
||||
}
|
||||
|
||||
if (newDirs.size === 0 && newFiles.size === 0) {
|
||||
core.info('No unexpected new top-level entries. Nothing to flag.');
|
||||
return;
|
||||
}
|
||||
|
||||
// Idempotency: don't re-flag on every synchronize.
|
||||
const comments = await github.paginate(github.rest.issues.listComments, {
|
||||
owner, repo, issue_number: prNumber, per_page: 100,
|
||||
});
|
||||
if (comments.some((c) => c.body && c.body.includes(MARKER))) {
|
||||
core.info('Already flagged on this PR. Skipping duplicate comment.');
|
||||
return;
|
||||
}
|
||||
|
||||
const lines = [];
|
||||
for (const d of [...newDirs].sort()) lines.push(`- 📁 \`${d}/\` (new top-level folder)`);
|
||||
for (const f of [...newFiles].sort()) lines.push(`- 📄 \`${f}\` (new top-level file)`);
|
||||
const entries = lines.join('\n');
|
||||
|
||||
core.warning(`Unexpected new top-level entries: ${[...newDirs, ...newFiles].join(', ')}`);
|
||||
|
||||
let body = fs.readFileSync('.github/new-top-level-flag.md', 'utf8');
|
||||
body = body
|
||||
.replace(/{{AUTHOR}}/g, context.payload.pull_request.user.login)
|
||||
.replace(/{{ENTRIES}}/g, entries);
|
||||
|
||||
await github.rest.issues.createComment({
|
||||
owner, repo, issue_number: prNumber, body,
|
||||
});
|
||||
|
||||
core.info(`Flagged PR #${prNumber}.`);
|
||||
88
.github/workflows/guard-custom-layer.yml
vendored
Normal file
88
.github/workflows/guard-custom-layer.yml
vendored
Normal file
|
|
@ -0,0 +1,88 @@
|
|||
name: Guard custom layer
|
||||
|
||||
# The /custom/ layer is a template: in upstream microsoft/BCQuality it stays
|
||||
# empty by default (README.md + .gitkeep placeholders only). Custom knowledge
|
||||
# and skills are partner/customer-specific and belong in a fork, never upstream.
|
||||
#
|
||||
# This workflow auto-closes any PR that adds or changes content under /custom/
|
||||
# (anything beyond the allowed template files). It runs only on the upstream
|
||||
# repo, so forks that legitimately populate /custom/ are unaffected.
|
||||
#
|
||||
# pull_request_target is required so the workflow runs with a token that can
|
||||
# comment on and close the PR (including PRs opened from forks). It only reads
|
||||
# the PR's file LIST via the API and never checks out or executes PR code, so
|
||||
# the elevated token is not exposed to untrusted content.
|
||||
|
||||
on:
|
||||
pull_request_target:
|
||||
types: [opened, reopened, synchronize]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: write
|
||||
issues: write
|
||||
|
||||
jobs:
|
||||
guard:
|
||||
# Never run on forks — a fork's /custom/ content is exactly what's supposed
|
||||
# to live there.
|
||||
if: github.repository == 'microsoft/BCQuality'
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Check out repository
|
||||
uses: actions/checkout@v4
|
||||
with:
|
||||
sparse-checkout: |
|
||||
.github/custom-layer-autoclose.md
|
||||
sparse-checkout-cone-mode: false
|
||||
|
||||
- name: Close PR if it touches the custom layer
|
||||
uses: actions/github-script@v7
|
||||
with:
|
||||
script: |
|
||||
const fs = require('fs');
|
||||
|
||||
// Files under custom/ that ARE allowed to change (the template seed).
|
||||
const ALLOWED = new Set([
|
||||
'custom/README.md',
|
||||
]);
|
||||
// Any .gitkeep under custom/ is also allowed.
|
||||
const isAllowed = (p) =>
|
||||
ALLOWED.has(p) || /^custom\/.*\.gitkeep$/.test(p) || p === 'custom/.gitkeep';
|
||||
|
||||
const { owner, repo } = context.repo;
|
||||
const prNumber = context.payload.pull_request.number;
|
||||
|
||||
const files = await github.paginate(github.rest.pulls.listFiles, {
|
||||
owner, repo, pull_number: prNumber, per_page: 100,
|
||||
});
|
||||
|
||||
// Offending = added/modified/renamed/copied/changed paths under custom/
|
||||
// that are not template files. (We ignore pure deletions.)
|
||||
const offending = files
|
||||
.filter((f) => f.status !== 'removed')
|
||||
.map((f) => f.filename)
|
||||
.filter((p) => p.startsWith('custom/') && !isAllowed(p));
|
||||
|
||||
if (offending.length === 0) {
|
||||
core.info('No disallowed /custom/ changes found. Nothing to do.');
|
||||
return;
|
||||
}
|
||||
|
||||
core.warning(`PR #${prNumber} touches the custom layer: ${offending.join(', ')}`);
|
||||
|
||||
const fileList = offending.map((p) => `- \`${p}\``).join('\n');
|
||||
let body = fs.readFileSync('.github/custom-layer-autoclose.md', 'utf8');
|
||||
body = body
|
||||
.replace(/{{AUTHOR}}/g, context.payload.pull_request.user.login)
|
||||
.replace(/{{FILES}}/g, fileList);
|
||||
|
||||
await github.rest.issues.createComment({
|
||||
owner, repo, issue_number: prNumber, body,
|
||||
});
|
||||
|
||||
await github.rest.pulls.update({
|
||||
owner, repo, pull_number: prNumber, state: 'closed',
|
||||
});
|
||||
|
||||
core.info(`Closed PR #${prNumber}.`);
|
||||
69
.github/workflows/release-version.yml
vendored
Normal file
69
.github/workflows/release-version.yml
vendored
Normal file
|
|
@ -0,0 +1,69 @@
|
|||
# Cuts a BCQuality content release on demand (roughly monthly), NOT on every
|
||||
# commit. Run this workflow manually once the `main` content is ready, and choose
|
||||
# whether to bump the minor (usual periodic content update) or the major
|
||||
# (breaking change).
|
||||
#
|
||||
# The version is a `major.minor` value derived from existing git tags — there is
|
||||
# no VERSION file. The minor is a monotonic counter: it only ever increments and
|
||||
# never resets, even across a major bump, so it uniquely identifies a release.
|
||||
# This workflow computes the next version and tags the current commit as
|
||||
# `v{major}.{minor}`.
|
||||
|
||||
name: Release version
|
||||
|
||||
on:
|
||||
workflow_dispatch:
|
||||
inputs:
|
||||
bump:
|
||||
description: Which part to bump
|
||||
type: choice
|
||||
options:
|
||||
- minor
|
||||
- major
|
||||
default: minor
|
||||
|
||||
# Only tag creation needs write.
|
||||
permissions:
|
||||
contents: write
|
||||
|
||||
concurrency:
|
||||
group: release-version
|
||||
cancel-in-progress: false
|
||||
|
||||
jobs:
|
||||
release:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
fetch-depth: 0
|
||||
|
||||
- name: Compute and tag release
|
||||
shell: bash
|
||||
run: |
|
||||
git fetch --tags --force --quiet
|
||||
tags="$(git tag -l | grep -E '^v[0-9]+\.[0-9]+$' || true)"
|
||||
|
||||
if [[ -z "$tags" ]]; then
|
||||
# First release.
|
||||
major=1
|
||||
minor=0
|
||||
else
|
||||
latest_major="$(printf '%s\n' "$tags" | sed -E 's/^v([0-9]+)\..*/\1/' | sort -n | tail -1)"
|
||||
latest_minor="$(printf '%s\n' "$tags" | sed -E 's/^v[0-9]+\.([0-9]+)$/\1/' | sort -n | tail -1)"
|
||||
minor=$(( latest_minor + 1 )) # monotonic, never resets
|
||||
if [[ "${{ inputs.bump }}" == "major" ]]; then
|
||||
major=$(( latest_major + 1 ))
|
||||
else
|
||||
major="$latest_major"
|
||||
fi
|
||||
fi
|
||||
|
||||
tag="v${major}.${minor}"
|
||||
if git rev-parse -q --verify "refs/tags/${tag}" >/dev/null; then
|
||||
echo "::error::Tag ${tag} already exists"
|
||||
exit 1
|
||||
fi
|
||||
git tag "$tag" "${{ github.sha }}"
|
||||
git push origin "$tag"
|
||||
echo "Released BCQuality ${tag} at ${{ github.sha }}"
|
||||
|
|
@ -1,9 +1,9 @@
|
|||
# Microsoft-endorsed content and skills require maintainer review
|
||||
/microsoft/ @jeschulz
|
||||
/skills/ @jeschulz
|
||||
/microsoft/ @jesperschulz
|
||||
/skills/ @jesperschulz
|
||||
|
||||
# GitHub Actions and CI
|
||||
/.github/ @jeschulz
|
||||
/.github/ @jesperschulz
|
||||
|
||||
# Domain experts — required reviewers for coding rules
|
||||
/microsoft/knowledge/events/ @AleksandricMarko @pchriste-microsoft-com
|
||||
|
|
|
|||
12
README.md
12
README.md
|
|
@ -136,6 +136,18 @@ For the end-to-end flow — from orchestrator trigger through to how output reac
|
|||
│ └── /skills/
|
||||
```
|
||||
|
||||
## Versioning
|
||||
|
||||
BCQuality content is released on demand — roughly monthly, not on every commit. A
|
||||
release is a `major.minor` value derived from git tags, cut manually via the
|
||||
`Release version` workflow: pick whether to bump the minor or the major, and it
|
||||
computes the next version and tags the current `main` as `v{major}.{minor}`.
|
||||
|
||||
- Bump the **minor** for the usual periodic content update; bump the **major**
|
||||
only for a breaking change.
|
||||
- The minor is a **monotonic counter** — it only ever increments and never
|
||||
resets, even across a major bump — so it uniquely identifies a release.
|
||||
|
||||
## Contributing
|
||||
|
||||
Contributions are welcome. Before submitting a PR:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,24 @@
|
|||
---
|
||||
bc-version: [24..]
|
||||
domain: appsource
|
||||
keywords: [app-json, help-url, copilot, grounding, documentation, url-depth, contexturl]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Keep the Copilot help URL to two path levels
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## 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.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Organize per-app documentation so the canonical help page sits no deeper than two path levels, and confirm during testing that Copilot citations resolve to your app's content rather than a broader parent. If your docs naturally nest deeper, give each app a dedicated two-level path it owns.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Setting `help` to a deep, tidy-looking docs path such as `https://contoso.com/docs/products/erp/myapp/setup`. Copilot truncates it to `…/docs/products`, then grounds on everything under that node — pulling in unrelated content and degrading answer quality for your users.
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
table 50120 "FieldError Default Bad"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; "Discount %"; Decimal) { }
|
||||
field(3; "Currency Code"; Code[10]) { }
|
||||
}
|
||||
|
||||
procedure ValidateForRelease()
|
||||
begin
|
||||
// Re-testing a field and handing FieldError a fully-formed sentence.
|
||||
// The framework already prepends the caption and appends the value,
|
||||
// so this renders as "Currency Code The Currency Code field must have
|
||||
// a value. in ..." — caption repeated, capital letter mid-sentence,
|
||||
// stray trailing clause.
|
||||
if "Currency Code" = '' then
|
||||
FieldError("Currency Code", 'The Currency Code field must have a value.');
|
||||
|
||||
if "Discount %" > 100 then
|
||||
FieldError("Discount %", 'The Discount % must not be greater than 100 percent.');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
table 50120 "FieldError Default Good"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; "Discount %"; Decimal) { }
|
||||
field(3; "Currency Code"; Code[10]) { }
|
||||
}
|
||||
|
||||
procedure ValidateForRelease()
|
||||
begin
|
||||
// Plain required-field gate: TestField checks the condition and raises
|
||||
// the error in one call, with caption and record context supplied by
|
||||
// the framework.
|
||||
TestField("Currency Code");
|
||||
|
||||
// Condition already evaluated: pass only a lowercase predicate so it
|
||||
// reads as one sentence after the auto-inserted caption and value.
|
||||
if "Discount %" > 100 then
|
||||
FieldError("Discount %", 'cannot exceed 100');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: error-handling
|
||||
keywords: [fielderror, testfield, error-message, field-caption, lowercase-convention, record-context, validation]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# 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
|
||||
`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
|
||||
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.
|
||||
|
||||
## 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.
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
table 50122 "FieldError vs TestField Bad"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; "Posting Date"; Date) { }
|
||||
field(3; "Amount"; Decimal) { }
|
||||
}
|
||||
|
||||
procedure PostDocument()
|
||||
begin
|
||||
// FieldError performs no comparison and always raises the moment it is
|
||||
// reached, so this "check" terminates PostDocument every time — the
|
||||
// Posting Date is never actually tested, and the amount rule below is
|
||||
// dead code.
|
||||
FieldError("Posting Date", 'must be filled in');
|
||||
|
||||
if IsAmountOutsideAllowedRange("Amount") then
|
||||
Error('Amount is out of range.');
|
||||
end;
|
||||
|
||||
local procedure IsAmountOutsideAllowedRange(Value: Decimal): Boolean
|
||||
begin
|
||||
exit((Value < 0) or (Value > 1000000));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,27 @@
|
|||
table 50122 "FieldError vs TestField Good"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; "Posting Date"; Date) { }
|
||||
field(3; "Amount"; Decimal) { }
|
||||
}
|
||||
|
||||
procedure PostDocument()
|
||||
begin
|
||||
// Simple presence gate: TestField performs the check itself and raises
|
||||
// only when the field is empty. Self-documenting prerequisite.
|
||||
TestField("Posting Date");
|
||||
|
||||
// Business logic has already determined the value is invalid;
|
||||
// FieldError raises a tailored, record-aware message with no
|
||||
// condition of its own.
|
||||
if IsAmountOutsideAllowedRange("Amount") then
|
||||
FieldError("Amount", 'is outside the approved posting range');
|
||||
end;
|
||||
|
||||
local procedure IsAmountOutsideAllowedRange(Value: Decimal): Boolean
|
||||
begin
|
||||
exit((Value < 0) or (Value > 1000000));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: error-handling
|
||||
keywords: [fielderror, testfield, field-validation, onvalidate, error-message, mandatory-field, record-context]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# Choose `TestField` For Conditional Checks And `FieldError` For Already-Failed Validation
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## 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.
|
||||
|
||||
## 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.
|
||||
|
||||
## 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.
|
||||
|
|
@ -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.
|
||||
21
community/knowledge/security/secrets-isolated-storage.bad.al
Normal file
21
community/knowledge/security/secrets-isolated-storage.bad.al
Normal file
|
|
@ -0,0 +1,21 @@
|
|||
table 50134 "Api Setup Bad Sample"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "Primary Key"; Code[10]) { }
|
||||
|
||||
// A secret in an ordinary Text field is readable by anyone with table
|
||||
// permission, ships in RapidStart packages and Excel exports, and
|
||||
// appears in record snapshots. No DataClassification tag makes it safe;
|
||||
// it belongs in IsolatedStorage instead.
|
||||
field(10; "API Key"; Text[250])
|
||||
{
|
||||
DataClassification = CustomerContent;
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Primary Key") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
codeunit 50134 "Api Credential Good Sample"
|
||||
{
|
||||
procedure StoreApiKey(ApiKey: SecretText)
|
||||
begin
|
||||
// Credentials live in IsolatedStorage, invisible to record reads, API
|
||||
// pages, RapidStart packages, and Excel export.
|
||||
IsolatedStorage.Set('ExternalApiKey', ApiKey, DataScope::Module);
|
||||
end;
|
||||
|
||||
procedure GetApiKey() ApiKey: SecretText
|
||||
begin
|
||||
if not IsolatedStorage.Get('ExternalApiKey', DataScope::Module, ApiKey) then
|
||||
Error('The external API key has not been configured.');
|
||||
end;
|
||||
}
|
||||
24
community/knowledge/security/secrets-isolated-storage.md
Normal file
24
community/knowledge/security/secrets-isolated-storage.md
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: security
|
||||
keywords: [isolatedstorage, secrets, api-key, oauth-token, connection-string, table-field, credentials]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# A secret belongs in IsolatedStorage, never in a table field
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## 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.
|
||||
|
||||
## Best Practice
|
||||
|
||||
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`.
|
||||
|
||||
## 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.
|
||||
|
|
@ -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.
|
||||
20
community/knowledge/ui/factbox-design.md
Normal file
20
community/knowledge/ui/factbox-design.md
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: ui
|
||||
keywords: [factbox, subpagelink, listpart, cardpart, page-part, related-information, flowfield-sift]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# Filter ListPart FactBoxes With SubPageLink To The Parent Record
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
A FactBox is a page `part` that surfaces related data beside the main record so users avoid navigating away. Every FactBox runs a database query as its host page loads, so an unfiltered one is a hidden performance tax paid on every page open. The remedial trap: a `ListPart` FactBox with no `SubPageLink` does not show "the related rows" — it loads and pages through the entire source table, because nothing ties it to the host record. This makes correct `SubPageLink` linkage, not visual layout, the load-bearing design decision.
|
||||
|
||||
## Best Practice
|
||||
Give every `ListPart` FactBox a `SubPageLink` that maps a field on the part's source table to a `field()` of the host record (for example `SubPageLink = "Document No." = field("No.")`), so it returns only rows belonging to the current record. Prefer a `CardPart` when you only need summary figures (balance, availability, status) — it reads a single record and avoids list overhead entirely. When a FactBox shows FlowFields, ensure the calculated total is backed by a SIFT key (`MaintainSIFTIndex`) so the sum is read from the index rather than aggregated row-by-row on each load. Keep FactBox count modest and avoid heavy `OnAfterGetRecord` logic in the part.
|
||||
|
||||
## Anti Pattern
|
||||
Adding a `ListPart` FactBox without a `SubPageLink`, expecting it to "just show related lines." The consequence is a full-table scan on every page load that grows with the dataset and is felt worst on list pages, where the FactBox re-queries on each row selection. Reviewer signal: any `part(...)` referencing a list-type page part where the `SubPageLink` property is absent, or a FactBox FlowField filtered on non-indexed fields. A second smell is duplicating data already on the page or stacking many FactBoxes, which multiplies queries for little context gain.
|
||||
20
community/knowledge/ui/fasttab-field-importance.md
Normal file
20
community/knowledge/ui/fasttab-field-importance.md
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: ui
|
||||
keywords: [importance, promoted, additional, fasttab, show-more, summary-line, progressive-disclosure, field-visibility]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# Set Field Importance To Drive FastTab Progressive Disclosure
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## 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.
|
||||
|
||||
## Best Practice
|
||||
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
|
||||
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.
|
||||
20
community/knowledge/ui/page-background-tasks.md
Normal file
20
community/knowledge/ui/page-background-tasks.md
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: ui
|
||||
keywords: [enqueuebackgroundtask, async-calculation, child-session, factbox, cue-tile, onaftergetcurrrecord, responsive-page, read-only]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# Offload Slow Read-Only Page Calculations To Background Tasks
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## 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.
|
||||
|
||||
## Best Practice
|
||||
Move any noticeable read-only computation off the synchronous render path into a background task. Enqueue from `OnAfterGetCurrRecord` so the task is tied to the currently focused record, and pass small payloads through the `Dictionary of [Text, Text]` input/output, converting types with `Format` and `Evaluate`. Keep each task focused on one value or a small related set rather than one large task, and show a placeholder until results land. Because tasks auto-cancel when the page closes, the record changes, or a same-ID task is re-enqueued, always supply sensible defaults and handle the timeout path in `OnPageBackgroundTaskError` — never let critical functionality depend on completion. For tests, drive the task synchronously with `RunPageBackgroundTask`.
|
||||
|
||||
## Anti Pattern
|
||||
Enqueuing from `OnAfterGetRecord` on a list page fires the task for every row, and each cancels the instant the selection moves to the next row — pure wasted child-session churn; a reviewer spots `EnqueueBackgroundTask` called from `OnAfterGetRecord` (or from `OnOpenPage`, where the record context is not yet stable). The other tell is a task codeunit attempting a database write or `Modify`: background tasks run read-only and the write fails at runtime. Inline heavy calculation directly in `OnAfterGetCurrRecord` with no task at all is the baseline smell — it reintroduces the page freeze the feature exists to remove.
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
---
|
||||
bc-version: [21..]
|
||||
domain: ui
|
||||
keywords: [actionref, promoted-actions, area-promoted, promotedcategory, promotedonly, action-bar, legacy-syntax]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# Promote Actions With The Modern `actionref` Syntax, Never The Legacy `Promoted` Properties
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## 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.
|
||||
|
||||
## Best Practice
|
||||
For new pages and page extensions, define actions in their normal `area`, then promote selected ones with `actionref` inside `area(Promoted)`, grouping them under explicit categories such as `Category_Process` and entity-named groups. This keeps each action defined once and referenced where it should appear, supports split buttons via `ShowAs`, and lets an extension promote a base action without redefining it. When extending a page, you may use modern syntax even if the base page used legacy properties (and vice versa) — the no-mixing rule is per-object, not per-dependency-tree.
|
||||
|
||||
## Anti Pattern
|
||||
Setting `Promoted = true` (with `PromotedCategory`, `PromotedOnly`, or `PromotedIsBig`) on actions in new code, or attempting to combine those properties with an `area(Promoted)` block in the same object — the latter fails to compile. The reviewer signal is any `Promoted`-prefixed property on an action in a newly authored page or page extension; flag it and convert to `actionref` (VS Code offers an automated conversion). Note separately that once an action is promoted in a published app, removing the promotion is a breaking change (AS0031/AW0013), so promote conservatively rather than walking it back later.
|
||||
20
community/knowledge/ui/promoted-action-groups.md
Normal file
20
community/knowledge/ui/promoted-action-groups.md
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
---
|
||||
bc-version: [21..]
|
||||
domain: ui
|
||||
keywords: [action-groups, area-promoted, actionref, showas, split-button, group-caption, navigate-group, entity-group]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# Use Standard Promoted Action Group Names And Placements
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## 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.
|
||||
|
||||
## 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. 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
|
||||
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`.
|
||||
|
|
@ -0,0 +1,43 @@
|
|||
// Anti-pattern: an own object with no affix. Another app that also defines a
|
||||
// "Loyalty Tier" table cannot be installed alongside this one.
|
||||
table 50379 "Loyalty Tier"
|
||||
{
|
||||
Caption = 'Loyalty Tier';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[20])
|
||||
{
|
||||
Caption = 'Code';
|
||||
}
|
||||
field(10; Description; Text[100])
|
||||
{
|
||||
Caption = 'Description';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Code")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Anti-pattern (the common half-measure): the extension object carries the
|
||||
// affix, but the field it adds to the standard Customer table does not. That
|
||||
// unaffixed field still collides with any other app that adds "Loyalty Points"
|
||||
// to Customer, and AS0011 flags it.
|
||||
tableextension 50378 "ABC Customer Ext" extends Customer
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(50378; "Loyalty Points"; Integer)
|
||||
{
|
||||
Caption = 'Loyalty Points';
|
||||
DataClassification = CustomerContent;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,40 @@
|
|||
// Own object: the affix "ABC" is carried at object-name level.
|
||||
table 50377 "ABC Loyalty Tier"
|
||||
{
|
||||
Caption = 'Loyalty Tier';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Code"; Code[20])
|
||||
{
|
||||
Caption = 'Code';
|
||||
}
|
||||
field(10; Description; Text[100])
|
||||
{
|
||||
Caption = 'Description';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Code")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Extension of a standard object: the added field is individually affixed,
|
||||
// because the object name (Customer) belongs to the base application.
|
||||
tableextension 50376 "ABC Customer Ext" extends Customer
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(50376; "Loyalty Points ABC"; Integer)
|
||||
{
|
||||
Caption = 'Loyalty Points';
|
||||
DataClassification = CustomerContent;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: appsource
|
||||
keywords: [object-affix, prefix, suffix, as0011, appsourcecop, collision, tableextension]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Apply a reserved affix to objects and to members added to base objects
|
||||
|
||||
## Description
|
||||
|
||||
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.
|
||||
|
||||
## Best Practice
|
||||
|
||||
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`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
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`.
|
||||
|
|
@ -0,0 +1,71 @@
|
|||
table 50372 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
field(20; Blocked; Boolean)
|
||||
{
|
||||
Caption = 'Blocked';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
// Anti-pattern: the block check sits in the master's own trigger. Editing a
|
||||
// blocked member is rare; referencing it is constant, and references never
|
||||
// fire OnModify. So this stops nothing that matters.
|
||||
trigger OnModify()
|
||||
begin
|
||||
TestField(Blocked, false);
|
||||
end;
|
||||
}
|
||||
|
||||
table 50373 "Loyalty Point Entry"
|
||||
{
|
||||
Caption = 'Loyalty Point Entry';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
Caption = 'Entry No.';
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(10; "Member No."; Code[20])
|
||||
{
|
||||
Caption = 'Member No.';
|
||||
TableRelation = "Loyalty Member"."No.";
|
||||
// No block check on the referencing side: a line can freely
|
||||
// reference a blocked member, and posting proceeds unchecked.
|
||||
}
|
||||
field(20; Points; Integer)
|
||||
{
|
||||
Caption = 'Points';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,84 @@
|
|||
table 50370 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
// Blocked is inert data here: the master carries the flag but no logic.
|
||||
field(20; Blocked; Boolean)
|
||||
{
|
||||
Caption = 'Blocked';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
table 50371 "Loyalty Point Entry"
|
||||
{
|
||||
Caption = 'Loyalty Point Entry';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
Caption = 'Entry No.';
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(10; "Member No."; Code[20])
|
||||
{
|
||||
Caption = 'Member No.';
|
||||
TableRelation = "Loyalty Member"."No.";
|
||||
|
||||
trigger OnValidate()
|
||||
var
|
||||
LoyaltyMember: Record "Loyalty Member";
|
||||
begin
|
||||
if "Member No." = '' then
|
||||
exit;
|
||||
// Enforcement lives at the point of use: reject a blocked master
|
||||
// as soon as a line references it.
|
||||
LoyaltyMember.Get("Member No.");
|
||||
LoyaltyMember.TestField(Blocked, false);
|
||||
end;
|
||||
}
|
||||
field(20; Points; Integer)
|
||||
{
|
||||
Caption = 'Points';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
procedure Post()
|
||||
var
|
||||
LoyaltyMember: Record "Loyalty Member";
|
||||
begin
|
||||
// Re-check before committing the transaction, in case the member was
|
||||
// blocked after the line was created.
|
||||
LoyaltyMember.Get("Member No.");
|
||||
LoyaltyMember.TestField(Blocked, false);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [blocked-field, testfield, referencing-code, point-of-use, enforcement, journal-line]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Enforce `Blocked` where the master is used, not in the master itself
|
||||
|
||||
## Description
|
||||
|
||||
The `Blocked` field on a master record (`Item`, `Customer`, `Resource`, or a custom master) is inert data. The master table holds **no** logic that acts on it. Enforcement belongs in the **consuming** code: when a journal line, document line, or posting routine references the master by its `No.`, that referencing object tests the flag at the point of use, e.g. `LoyaltyMember.Get("Member No."); LoyaltyMember.TestField(Blocked, false);` in the line's `OnValidate` and again before posting.
|
||||
|
||||
Putting the block check inside the master's own `OnInsert`/`OnModify` does nothing to stop transactional use: a blocked master is edited rarely, but it is *referenced* constantly, and those references never touch the master's own triggers. Base BC follows this split — `Item.Blocked` is checked by sales/purchase/journal code, not by the `Item` table. A boolean `Blocked` uses `TestField(Blocked, false)`; an option-style block (e.g. `Sales`/`All`) needs the specific option compared at each relevant path.
|
||||
|
||||
## Best Practice
|
||||
|
||||
The referencing line validates `Master.TestField(Blocked, false)` in `OnValidate` of the reference field and re-checks before posting. The master table stays logic-free on `Blocked`.
|
||||
|
||||
See sample: `check-blocked-in-referencing-code-not-in-master.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
The block check sits in the master's own `OnModify`/`OnInsert` (so referencing and posting proceed unchecked), or there is no check at all on the referencing side.
|
||||
|
||||
See sample: `check-blocked-in-referencing-code-not-in-master.bad.al`.
|
||||
|
|
@ -0,0 +1,31 @@
|
|||
table 50361 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
// Anti-pattern: an autoincrement Integer surrogate used as the business key.
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
Caption = 'Entry No.';
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
// No OnInsert, no number series, no "No." code, and no "No. Series" field.
|
||||
// Records get an opaque integer users never see and cannot quote on the phone,
|
||||
// and the master is cut off from BC's standard numbering and manual-entry flow.
|
||||
}
|
||||
|
|
@ -0,0 +1,45 @@
|
|||
table 50360 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
NotBlank = true;
|
||||
}
|
||||
field(2; "No. Series"; Code[20])
|
||||
{
|
||||
Caption = 'No. Series';
|
||||
Editable = false;
|
||||
TableRelation = "No. Series";
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
trigger OnInsert()
|
||||
var
|
||||
LoyaltySetup: Record "Loyalty Setup";
|
||||
NoSeries: Codeunit "No. Series";
|
||||
begin
|
||||
if "No." = '' then begin
|
||||
LoyaltySetup.Get();
|
||||
LoyaltySetup.TestField("Member Nos.");
|
||||
"No. Series" := LoyaltySetup."Member Nos.";
|
||||
"No." := NoSeries.GetNextNo("No. Series");
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [no-series, primary-key, code20, oninsert, autoincrement, number-assignment]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# A master table's `No.` primary key comes from a number series in `OnInsert`
|
||||
|
||||
## Description
|
||||
|
||||
In Business Central, a master table (Customer, Vendor, Item, and any custom equivalent) uses a single primary-key field named `No.` of type `Code[20]`. It is populated from a number series — configured on the feature's application-area setup table — inside the table's `OnInsert` trigger, but only when `No.` is still blank (so a user may still type a manual number when the series allows it). The record also keeps a non-editable `No. Series` `Code[20]` field recording which series produced the number.
|
||||
|
||||
This is not an `Integer` `AutoIncrement` key, a GUID, or the `SystemId`. Those are surrogate/system identifiers that users never see and cannot quote; BC's whole document flow — lookups, filtering, printed references, telephone support — depends on a short, human-readable, business-controlled `No.`. Use the modern assignment API described in `use-no-series-codeunit-not-noseriesmanagement.md`.
|
||||
|
||||
## Best Practice
|
||||
|
||||
`No.` `Code[20]` is the sole primary key; a non-editable `No. Series` `Code[20]` field records the source series. `OnInsert` checks `if "No." = ''`, reads the setup table, `TestField`s the configured series, stores it in `No. Series`, and assigns `No.` from the series.
|
||||
|
||||
See sample: `master-table-no-from-number-series-in-oninsert.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
An `Integer` `AutoIncrement` (or GUID / `SystemId`) primary key used as the business key, with no `OnInsert` number assignment. Records get an opaque identifier no user can reference, and the master no longer participates in the standard numbering and manual-entry behavior every other BC master follows.
|
||||
|
||||
See sample: `master-table-no-from-number-series-in-oninsert.bad.al`.
|
||||
|
|
@ -0,0 +1,39 @@
|
|||
table 50369 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
field(20; "Last Date Modified"; Date)
|
||||
{
|
||||
Caption = 'Last Date Modified';
|
||||
Editable = false;
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
trigger OnModify()
|
||||
begin
|
||||
"Last Date Modified" := Today();
|
||||
end;
|
||||
|
||||
// Missing OnRename: renaming the member changes the primary key without
|
||||
// firing OnModify, so "Last Date Modified" keeps its old, stale value and
|
||||
// change-detection logic downstream skips the renamed record.
|
||||
}
|
||||
|
|
@ -0,0 +1,40 @@
|
|||
table 50368 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
}
|
||||
field(10; Name; Text[100])
|
||||
{
|
||||
Caption = 'Name';
|
||||
}
|
||||
field(20; "Last Date Modified"; Date)
|
||||
{
|
||||
Caption = 'Last Date Modified';
|
||||
Editable = false;
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
trigger OnModify()
|
||||
begin
|
||||
"Last Date Modified" := Today();
|
||||
end;
|
||||
|
||||
trigger OnRename()
|
||||
begin
|
||||
"Last Date Modified" := Today();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [last-date-modified, onmodify, onrename, audit-field, non-editable, stale-value]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Refresh `Last Date Modified` in both `OnModify` and `OnRename`
|
||||
|
||||
## Description
|
||||
|
||||
Master tables carry a non-editable `Last Date Modified` field of type `Date`. It records when the record last changed and is refreshed by table triggers, not by the user. The refresh must happen in **both** `OnModify` and `OnRename`.
|
||||
|
||||
The reason is a BC-specific trap: renaming a record changes its primary key and fires `OnRename` — it does **not** fire `OnModify`. A table that updates `Last Date Modified` only in `OnModify` therefore leaves a stale date behind every rename. Downstream logic that keys on this field (incremental sync, integration deltas, "changed since" reports) then silently skips the renamed record. Assign `Today` (the system date), not `WorkDate`, because the field reflects the real modification moment.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Both `OnModify` and `OnRename` set `"Last Date Modified" := Today();`, and the field is declared `Editable = false` so only the triggers maintain it.
|
||||
|
||||
See sample: `set-last-date-modified-in-onmodify-and-onrename.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Only `OnModify` assigns `Last Date Modified`. After a rename the value is stale, and any process that trusts it to detect changes misses the record.
|
||||
|
||||
See sample: `set-last-date-modified-in-onmodify-and-onrename.bad.al`.
|
||||
|
|
@ -0,0 +1,55 @@
|
|||
table 50366 "Loyalty Setup"
|
||||
{
|
||||
Caption = 'Loyalty Setup';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
// Anti-pattern: an autoincrement key lets the table hold many rows,
|
||||
// so "the setup" is no longer a single, well-known record.
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
Caption = 'Entry No.';
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(10; "Member Nos."; Code[20])
|
||||
{
|
||||
Caption = 'Member Nos.';
|
||||
TableRelation = "No. Series";
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
page 50367 "Loyalty Setup List"
|
||||
{
|
||||
// Anti-pattern: a List page over a setup table invites multiple rows and
|
||||
// never guarantees that a row exists to read.
|
||||
Caption = 'Loyalty Setup List';
|
||||
PageType = List;
|
||||
SourceTable = "Loyalty Setup";
|
||||
UsageCategory = Administration;
|
||||
ApplicationArea = All;
|
||||
|
||||
layout
|
||||
{
|
||||
area(Content)
|
||||
{
|
||||
repeater(Group)
|
||||
{
|
||||
field("Member Nos."; Rec."Member Nos.")
|
||||
{
|
||||
ApplicationArea = All;
|
||||
ToolTip = 'Specifies the number series used to assign member numbers.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,70 @@
|
|||
table 50364 "Loyalty Setup"
|
||||
{
|
||||
Caption = 'Loyalty Setup';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Primary Key"; Code[10])
|
||||
{
|
||||
Caption = 'Primary Key';
|
||||
}
|
||||
field(10; "Member Nos."; Code[20])
|
||||
{
|
||||
Caption = 'Member Nos.';
|
||||
TableRelation = "No. Series";
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Primary Key")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
procedure GetRecordOnce()
|
||||
begin
|
||||
if Rec.Get() then
|
||||
exit;
|
||||
Rec.Init();
|
||||
Rec.Insert();
|
||||
end;
|
||||
}
|
||||
|
||||
page 50365 "Loyalty Setup"
|
||||
{
|
||||
Caption = 'Loyalty Setup';
|
||||
PageType = Card;
|
||||
SourceTable = "Loyalty Setup";
|
||||
UsageCategory = Administration;
|
||||
ApplicationArea = All;
|
||||
InsertAllowed = false;
|
||||
DeleteAllowed = false;
|
||||
|
||||
layout
|
||||
{
|
||||
area(Content)
|
||||
{
|
||||
group(Numbering)
|
||||
{
|
||||
Caption = 'Numbering';
|
||||
field("Member Nos."; Rec."Member Nos.")
|
||||
{
|
||||
ApplicationArea = All;
|
||||
ToolTip = 'Specifies the number series used to assign member numbers.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
trigger OnOpenPage()
|
||||
begin
|
||||
Rec.Reset();
|
||||
if not Rec.Get() then begin
|
||||
Rec.Init();
|
||||
Rec.Insert();
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [setup-table, insertallowed, deleteallowed, getrecordonce, primary-key, card-page]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# A setup table is a singleton: one blank-keyed row, no insert or delete
|
||||
|
||||
## Description
|
||||
|
||||
An application-area setup table (`Sales & Receivables Setup`, `Inventory Setup`, and any custom `* Setup`) holds exactly one record per company. Its primary key is a single `Code[10]` field named `Primary Key`, and the row's value is left blank. Nothing else identifies the row — there is only ever one.
|
||||
|
||||
The setup **card** page enforces the singleton: `InsertAllowed = false` and `DeleteAllowed = false` stop a second row or an empty table, and the page guarantees the row exists on first open — typically `OnOpenPage` with `if not Rec.Get() then begin Rec.Init(); Rec.Insert(); end;`, or a `GetRecordOnce` helper on the table. Consuming code then reads it with a plain `Get()`. The read side needs no access optimization — see `singleton-setup-tables-need-no-access-optimization.md`.
|
||||
|
||||
## Best Practice
|
||||
|
||||
`Primary Key` `Code[10]` is the sole key; the setup is surfaced through a Card page with `InsertAllowed = false`, `DeleteAllowed = false`, and an open-time guard that inserts the blank row if it is missing.
|
||||
|
||||
See sample: `setup-table-is-a-singleton.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
An `Integer` / `AutoIncrement` key, a page that allows insert or delete, or a List page over the setup table. Any of these lets the table hold zero or many rows, so "the setup" becomes ambiguous and `Get()` may fail or read the wrong record.
|
||||
|
||||
See sample: `setup-table-is-a-singleton.bad.al`.
|
||||
|
|
@ -0,0 +1,51 @@
|
|||
table 50363 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
|
||||
trigger OnValidate()
|
||||
begin
|
||||
if "No." = xRec."No." then
|
||||
exit;
|
||||
LoyaltySetup.Get();
|
||||
// Obsolete-pending: NoSeriesManagement.TestManual raises a
|
||||
// deprecation warning and is scheduled for removal.
|
||||
NoSeriesMgt.TestManual(LoyaltySetup."Member Nos.");
|
||||
"No. Series" := '';
|
||||
end;
|
||||
}
|
||||
field(2; "No. Series"; Code[20])
|
||||
{
|
||||
Caption = 'No. Series';
|
||||
Editable = false;
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
var
|
||||
LoyaltySetup: Record "Loyalty Setup";
|
||||
NoSeriesMgt: Codeunit NoSeriesManagement;
|
||||
|
||||
trigger OnInsert()
|
||||
begin
|
||||
if "No." = '' then begin
|
||||
LoyaltySetup.Get();
|
||||
LoyaltySetup.TestField("Member Nos.");
|
||||
// Obsolete-pending legacy assignment call; use codeunit "No. Series".
|
||||
NoSeriesMgt.InitSeries(LoyaltySetup."Member Nos.", xRec."No. Series", 0D, "No.", "No. Series");
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,55 @@
|
|||
table 50362 "Loyalty Member"
|
||||
{
|
||||
Caption = 'Loyalty Member';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20])
|
||||
{
|
||||
Caption = 'No.';
|
||||
|
||||
trigger OnValidate()
|
||||
var
|
||||
NoSeries: Codeunit "No. Series";
|
||||
begin
|
||||
if "No." = xRec."No." then
|
||||
exit;
|
||||
LoyaltySetup.Get();
|
||||
if not NoSeries.IsManual(LoyaltySetup."Member Nos.") then
|
||||
Error(ManualNosNotAllowedErr);
|
||||
"No. Series" := '';
|
||||
end;
|
||||
}
|
||||
field(2; "No. Series"; Code[20])
|
||||
{
|
||||
Caption = 'No. Series';
|
||||
Editable = false;
|
||||
TableRelation = "No. Series";
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
var
|
||||
LoyaltySetup: Record "Loyalty Setup";
|
||||
ManualNosNotAllowedErr: Label 'Numbers are assigned automatically. Allow manual numbers on the No. Series to enter one by hand.';
|
||||
|
||||
trigger OnInsert()
|
||||
var
|
||||
NoSeries: Codeunit "No. Series";
|
||||
begin
|
||||
if "No." = '' then begin
|
||||
LoyaltySetup.Get();
|
||||
LoyaltySetup.TestField("Member Nos.");
|
||||
"No. Series" := LoyaltySetup."Member Nos.";
|
||||
"No." := NoSeries.GetNextNo("No. Series");
|
||||
end;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [22..]
|
||||
domain: data-modeling
|
||||
keywords: [no-series, getnextno, ismanual, noseriesmanagement, obsolete-pending, testmanual]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Assign numbers with codeunit `"No. Series"`, not the obsolete `NoSeriesManagement`
|
||||
|
||||
## Description
|
||||
|
||||
Since 2023 release wave 1 (v22) the number-series API is codeunit **310** `"No. Series"`, called by name in AL. Its methods include `GetNextNo`, `PeekNextNo`, `IsManual`, `TestManual`, and `LookupRelatedNoSeries`. The older codeunit **396** `NoSeriesManagement` and its `InitSeries` / `SelectSeries` / `SetSeries` / `TestManual` methods are marked obsolete-pending: they still compile but raise a deprecation warning and are scheduled for removal, so they must not appear in new code.
|
||||
|
||||
LLMs reproduce the legacy `NoSeriesManagement` pattern because it dominates pre-2023 training data. Prefer the new codeunit: it has a cleaner surface and is the only version that survives the deprecation. (The numbers matter — `310` is the current codeunit; `396` is the legacy one being retired.) Verify signatures on learn.microsoft.com or in the `microsoft/BCApps` source before use.
|
||||
|
||||
## Best Practice
|
||||
|
||||
`OnInsert` assigns the number with `NoSeries.GetNextNo("No. Series")` where `NoSeries` is `Codeunit "No. Series"`. The `No.` field's `OnValidate` guards manual entry by calling `NoSeries.IsManual(...)` (or `TestManual`) before clearing `No. Series`.
|
||||
|
||||
See sample: `use-no-series-codeunit-not-noseriesmanagement.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`NoSeriesMgt.InitSeries(...)` for assignment and `NoSeriesMgt.TestManual(...)` for the manual check, where `NoSeriesMgt` is `Codeunit NoSeriesManagement`. Both are obsolete-pending and emit compiler warnings.
|
||||
|
||||
See sample: `use-no-series-codeunit-not-noseriesmanagement.bad.al`.
|
||||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [21..]
|
||||
domain: style
|
||||
keywords: [abouttitle, abouttext, teaching-tip, onboarding, page]
|
||||
technologies: [al]
|
||||
|
|
|
|||
|
|
@ -0,0 +1,25 @@
|
|||
page 50375 "Sample App Area Bad"
|
||||
{
|
||||
PageType = Card;
|
||||
SourceTable = Customer;
|
||||
layout
|
||||
{
|
||||
area(Content)
|
||||
{
|
||||
group(General)
|
||||
{
|
||||
// Anti-pattern: no ApplicationArea. AS0062 flags this control,
|
||||
// and it is silently hidden in the Web client for profiles whose
|
||||
// enabled areas do not already cover it.
|
||||
field("No."; Rec."No.")
|
||||
{
|
||||
ToolTip = 'Specifies the number that identifies the customer.';
|
||||
}
|
||||
field(Name; Rec.Name)
|
||||
{
|
||||
ToolTip = 'Specifies the customer''s name.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,40 @@
|
|||
page 50374 "Sample App Area Good"
|
||||
{
|
||||
PageType = Card;
|
||||
SourceTable = Customer;
|
||||
layout
|
||||
{
|
||||
area(Content)
|
||||
{
|
||||
group(General)
|
||||
{
|
||||
field("No."; Rec."No.")
|
||||
{
|
||||
ApplicationArea = All;
|
||||
ToolTip = 'Specifies the number that identifies the customer.';
|
||||
}
|
||||
field(Name; Rec.Name)
|
||||
{
|
||||
ApplicationArea = All;
|
||||
ToolTip = 'Specifies the customer''s name.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
actions
|
||||
{
|
||||
area(Processing)
|
||||
{
|
||||
action(Refresh)
|
||||
{
|
||||
ApplicationArea = All;
|
||||
ToolTip = 'Reloads the current record.';
|
||||
|
||||
trigger OnAction()
|
||||
begin
|
||||
CurrPage.Update(false);
|
||||
end;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: style
|
||||
keywords: [application-area, page-control, as0062, appsourcecop, hidden-control, web-client]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Every page control needs an `ApplicationArea` (AppSourceCop AS0062)
|
||||
|
||||
## Description
|
||||
|
||||
A field control on a page or pageextension that has no `ApplicationArea` property is silently hidden in the Web client for every profile whose enabled application areas do not cover it. There is no error and no warning at runtime — the field simply does not appear, which reads as data loss to the user. AppSourceCop AS0062 flags any page control or action that is missing the `ApplicationArea` property, and AppSource technical validation rejects the app until it is set.
|
||||
|
||||
Set the property to an area the app actually enables. `All` makes the control visible under every profile and is the common default; if the app declares narrower areas in `app.json`, use one of those. The property applies to field controls and to actions. This is a sibling concern to `caption-required-on-page-fields.md` and `tooltip-required-on-page-fields.md`; note that the ToolTip requirement is the separate CodeCop rule AA0218, not AS0062.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Every field control and action carries `ApplicationArea = All;` (or a declared area of the app). The value is set once per control and keeps the control visible in the Web client.
|
||||
|
||||
See sample: `applicationarea-required-on-page-controls.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A field control with no `ApplicationArea`. AS0062 flags it, and the control is invisible in the Web client for any profile that does not already enable a matching area.
|
||||
|
||||
See sample: `applicationarea-required-on-page-controls.bad.al`.
|
||||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [25..]
|
||||
domain: style
|
||||
keywords: [this, codeunit, self-reference, aa0248, scope]
|
||||
technologies: [al]
|
||||
|
|
|
|||
|
|
@ -0,0 +1,18 @@
|
|||
codeunit 50409 "Test AssertError Bad"
|
||||
{
|
||||
Subtype = Test;
|
||||
|
||||
[Test]
|
||||
procedure BlankNameIsRejectedWithSpecificError()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.Init();
|
||||
Customer.Name := '';
|
||||
|
||||
// Bare asserterror: passes if ANY error is raised. A relation error,
|
||||
// a permission error, or a typo elsewhere would all satisfy it — so
|
||||
// this never proves the blank-name guard is the thing that fired.
|
||||
asserterror Customer.TestField(Name);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
codeunit 50408 "Test AssertError Good"
|
||||
{
|
||||
Subtype = Test;
|
||||
|
||||
[Test]
|
||||
procedure BlankNameIsRejectedWithSpecificError()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.Init();
|
||||
Customer.Name := '';
|
||||
|
||||
// [WHEN] a mandatory field is blank
|
||||
asserterror Customer.TestField(Name);
|
||||
|
||||
// [THEN] verify the SPECIFIC failure through a reusable Library helper
|
||||
// instead of hardcoding the localized message and the 'TestField' code.
|
||||
// ExpectedTestFieldError centralizes that knowledge, so the test keeps
|
||||
// working when the caption or code changes; FieldCaption avoids pinning
|
||||
// the field name as a literal.
|
||||
Assert.ExpectedTestFieldError(Customer.FieldCaption(Name), '');
|
||||
end;
|
||||
|
||||
var
|
||||
Assert: Codeunit "Library Assert";
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: testing
|
||||
keywords: [asserterror, expectederror, expectederrorcode, negative-test, error-code]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Pin asserterror to a specific error with ExpectedError and ExpectedErrorCode
|
||||
|
||||
## Description
|
||||
|
||||
`asserterror` passes when the guarded statement raises any error at all. That is too permissive for a negative test: a typo, a missing setup record, or a permission failure all raise errors, so a bare `asserterror` can go green while never exercising the rule it claims to verify — false confidence that the validation works. Constrain it. `Assert.ExpectedError(text)` checks the message of the error that was actually raised, and `Assert.ExpectedErrorCode(code)` checks its error code. Together they assert that the specific failure occurred, turning "something went wrong" into "the right thing went wrong for the right reason".
|
||||
|
||||
## Best Practice
|
||||
|
||||
Follow every `asserterror` with a verification of the error it expects, and prefer the reusable `Library Assert` helpers over hardcoded literals. For a mandatory-field check, `Assert.ExpectedTestFieldError(FieldCaption, ExpectedValue)` encapsulates both the message and the `TestField` code, so the test survives caption or code changes and does not repeat that knowledge in every method. For other errors, pair `Assert.ExpectedError` with a stable substring — ideally a shared `Label`, not an inline sentence — and, where known, `Assert.ExpectedErrorCode`. When a needed check is missing from the shared library, extend `Library Assert` (or your own assert library) with a helper rather than hardcoding message text and codes across tests; matching on a code or an invariant fragment keeps the test from going blind to the wrong error when a caption is localized.
|
||||
|
||||
See sample: `asserterror-needs-expectederror-and-code.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`asserterror DoInvalid();` with nothing after it. The test asserts only that the call failed somehow; swap the validation for a different bug and the test still passes, certifying a guard that may no longer fire. A negative test that cannot tell one error from another verifies almost nothing.
|
||||
|
||||
See sample: `asserterror-needs-expectederror-and-code.bad.al`.
|
||||
43
microsoft/knowledge/testing/ui-handlers-in-tests.bad.al
Normal file
43
microsoft/knowledge/testing/ui-handlers-in-tests.bad.al
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
codeunit 50401 "Test UI Handlers Bad"
|
||||
{
|
||||
Subtype = Test;
|
||||
|
||||
// Several wiring mistakes, each of which fails at runtime rather than as a
|
||||
// clean assertion the reviewer can read:
|
||||
// * A UI call with no listed handler -> "unhandled UI" abort (the Message
|
||||
// below has no handler).
|
||||
// * The mirror mistake, listing a handler the path never hits, instead
|
||||
// fails with "handler function was not executed".
|
||||
// * A handler that hardcodes its answer and asserts inline, with no
|
||||
// enqueue/dequeue -> nothing proves the RIGHT dialog fired the RIGHT
|
||||
// number of times, and a failed inline assert can be swallowed by the
|
||||
// calling UI operation.
|
||||
[Test]
|
||||
[HandlerFunctions('ConfirmHandler')]
|
||||
procedure PostDocumentConfirmsAndMessages()
|
||||
begin
|
||||
// No Initialize(): a value leaked by an earlier test corrupts this one.
|
||||
RunPostingThatConfirmsAndMessages();
|
||||
// No AssertEmpty(): a missing or extra dialog goes unnoticed.
|
||||
end;
|
||||
|
||||
local procedure RunPostingThatConfirmsAndMessages()
|
||||
begin
|
||||
// Raises a Confirm AND a Message, but only ConfirmHandler is listed:
|
||||
// the Message has nothing to intercept it -> unhandled-UI runtime abort.
|
||||
if Confirm('Post this document?', false) then
|
||||
Message('Posting completed.');
|
||||
end;
|
||||
|
||||
[ConfirmHandler]
|
||||
procedure ConfirmHandler(Question: Text[1024]; var Reply: Boolean)
|
||||
begin
|
||||
// Hardcoded expectation and hardcoded reply. If the wrong dialog fires,
|
||||
// this inline assert may never surface as the test's verdict.
|
||||
Assert.AreEqual('Post this document?', Question, 'Wrong confirm.');
|
||||
Reply := true;
|
||||
end;
|
||||
|
||||
var
|
||||
Assert: Codeunit "Library Assert";
|
||||
}
|
||||
57
microsoft/knowledge/testing/ui-handlers-in-tests.good.al
Normal file
57
microsoft/knowledge/testing/ui-handlers-in-tests.good.al
Normal file
|
|
@ -0,0 +1,57 @@
|
|||
codeunit 50400 "Test UI Handlers Good"
|
||||
{
|
||||
Subtype = Test;
|
||||
|
||||
[Test]
|
||||
[HandlerFunctions('ConfirmHandler,PostMessageHandler')]
|
||||
procedure PostDocumentConfirmsAndMessages()
|
||||
begin
|
||||
Initialize();
|
||||
|
||||
// [GIVEN] the test enqueues, in interaction order, what each handler
|
||||
// will see and how it should answer: the Confirm's expected
|
||||
// question plus the reply to return, then the expected Message.
|
||||
LibraryVariableStorage.Enqueue('Post this document?'); // expected question (substring)
|
||||
LibraryVariableStorage.Enqueue(true); // reply ConfirmHandler returns
|
||||
LibraryVariableStorage.Enqueue('Posting completed.'); // expected message (substring)
|
||||
|
||||
// [WHEN] the code under test raises the Confirm and then the Message
|
||||
RunPostingThatConfirmsAndMessages();
|
||||
|
||||
// [THEN] every enqueued expectation was consumed exactly once
|
||||
LibraryVariableStorage.AssertEmpty();
|
||||
end;
|
||||
|
||||
local procedure Initialize()
|
||||
begin
|
||||
// Clear leftover values so a value leaked by an earlier test cannot
|
||||
// cascade into this one.
|
||||
LibraryVariableStorage.Clear();
|
||||
end;
|
||||
|
||||
local procedure RunPostingThatConfirmsAndMessages()
|
||||
begin
|
||||
// Stands in for the production routine that confirms, then messages.
|
||||
if Confirm('Post this document?', false) then
|
||||
Message('Posting completed.');
|
||||
end;
|
||||
|
||||
[ConfirmHandler]
|
||||
procedure ConfirmHandler(Question: Text[1024]; var Reply: Boolean)
|
||||
begin
|
||||
// Verify the RIGHT dialog fired (substring match), then return the
|
||||
// reply the test enqueued for it.
|
||||
Assert.ExpectedConfirm(LibraryVariableStorage.DequeueText(), Question);
|
||||
Reply := LibraryVariableStorage.DequeueBoolean();
|
||||
end;
|
||||
|
||||
[MessageHandler]
|
||||
procedure PostMessageHandler(Message: Text[1024])
|
||||
begin
|
||||
Assert.ExpectedMessage(LibraryVariableStorage.DequeueText(), Message);
|
||||
end;
|
||||
|
||||
var
|
||||
Assert: Codeunit "Library Assert";
|
||||
LibraryVariableStorage: Codeunit "Library - Variable Storage";
|
||||
}
|
||||
28
microsoft/knowledge/testing/ui-handlers-in-tests.md
Normal file
28
microsoft/knowledge/testing/ui-handlers-in-tests.md
Normal file
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: testing
|
||||
keywords: [handler, handlerfunctions, confirm, message, strmenu, variable-storage, enqueue, unhandled-ui]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Wire and verify UI handlers with enqueue-driven expectations
|
||||
|
||||
## Description
|
||||
|
||||
A test runs headless: there is no interactive user to answer a dialog. Every UI call the executed path raises — `Confirm`, `Message`, error dialogs, `Page.Run`/`RunModal`, `Report.Run`/`RunModal`, request pages, `StrMenu`, `Notification.Send` — must be intercepted by a handler carrying the matching attribute (`[ConfirmHandler]`, `[MessageHandler]`, `[StrMenuHandler]`, `[ModalPageHandler]`, …) and named in the method's `[HandlerFunctions(...)]`. The list is a two-sided contract: raise a UI call with no listed handler and the platform aborts with an *unhandled UI* error; list a handler the path never hits and it fails with *"handler function was not executed"*. Both are runtime failures — the test never reaches its verdict, so a reviewer sees an infrastructure error instead of a result on the behavior under test.
|
||||
|
||||
Getting the handler *present* is only half the job; the handler must also verify the *right* dialog fired the *right* number of times. Do that by driving handlers from the test, not by hardcoding answers inside them.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Make the test own the expectations and the handlers consume them. Before acting, the test `Enqueue`s — in interaction order — the expected text (a stable substring) and any reply each handler must return. The handler `Dequeue`s the expected text, verifies it with the purpose-built asserts (`Assert.ExpectedMessage`, `Assert.ExpectedConfirm`, `Assert.ExpectedStrMenu` — which match on a fragment, not the full localized caption), then `Dequeue`s and returns its reply. Finish the test body with `LibraryVariableStorage.AssertEmpty` to prove every enqueued interaction fired exactly once, and start each test with an `Initialize` that calls `LibraryVariableStorage.Clear` so a value leaked by an earlier test cannot cascade. List in `[HandlerFunctions]` precisely the handlers the scenario triggers — no superset "just in case", no subset that happens to work today.
|
||||
|
||||
See sample: `ui-handlers-in-tests.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Omitting a handler for a UI call the path raises (unhandled-UI abort), padding the list with a handler the path never reaches ("handler function was not executed"), or writing handlers that hardcode their answer and assert inline with no enqueue/dequeue. The last is the subtle one: nothing proves the correct dialog fired the expected number of times, and an inline assertion that fails inside a handler can be swallowed by the calling UI operation, leaving the suite green while the behavior is broken. Skipping `Initialize`/`AssertEmpty` hides both a leaked queue and a missing or extra dialog.
|
||||
|
||||
See sample: `ui-handlers-in-tests.bad.al`.
|
||||
|
|
@ -0,0 +1,25 @@
|
|||
codeunit 50411 "Test Library Fixtures Bad"
|
||||
{
|
||||
Subtype = Test;
|
||||
|
||||
[Test]
|
||||
procedure OrderUsesHandRolledFixtures()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
SalesHeader: Record "Sales Header";
|
||||
begin
|
||||
// Hand-rolled customer: a chosen "No." with no number-series entry and
|
||||
// none of the mandatory fields a real customer carries. Bypasses the
|
||||
// setup production code assumes and breaks when the schema adds a
|
||||
// required field this test does not set.
|
||||
Customer.Init();
|
||||
Customer."No." := 'X';
|
||||
Customer.Insert();
|
||||
|
||||
SalesHeader.Init();
|
||||
SalesHeader."Document Type" := SalesHeader."Document Type"::Order;
|
||||
SalesHeader."No." := 'SO-X';
|
||||
SalesHeader.Validate("Sell-to Customer No.", Customer."No.");
|
||||
SalesHeader.Insert(true);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
codeunit 50410 "Test Library Fixtures Good"
|
||||
{
|
||||
Subtype = Test;
|
||||
|
||||
[Test]
|
||||
procedure OrderUsesLibraryCreatedFixtures()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
Item: Record Item;
|
||||
SalesHeader: Record "Sales Header";
|
||||
SalesLine: Record "Sales Line";
|
||||
begin
|
||||
// Library codeunits create valid parents: number series, mandatory
|
||||
// fields and table relations are all handled for you.
|
||||
LibrarySales.CreateCustomer(Customer);
|
||||
LibraryInventory.CreateItem(Item);
|
||||
LibrarySales.CreateSalesHeader(SalesHeader, SalesHeader."Document Type"::Order, Customer."No.");
|
||||
LibrarySales.CreateSalesLine(SalesLine, SalesHeader, SalesLine.Type::Item, Item."No.", LibraryRandom.RandInt(10));
|
||||
|
||||
Assert.AreEqual(Customer."No.", SalesHeader."Sell-to Customer No.", 'Header should use the created customer.');
|
||||
end;
|
||||
|
||||
var
|
||||
Assert: Codeunit "Library Assert";
|
||||
LibrarySales: Codeunit "Library - Sales";
|
||||
LibraryInventory: Codeunit "Library - Inventory";
|
||||
LibraryRandom: Codeunit "Library - Random";
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: testing
|
||||
keywords: [library-codeunits, fixtures, test-data, number-series, prerequisite]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Build fixtures with the test Library codeunits, not hand-rolled Init/Insert
|
||||
|
||||
## Description
|
||||
|
||||
BC ships a layer of test Library codeunits — `LibrarySales`, `LibraryPurchase`, `LibraryERM`, `LibraryInventory`, `LibraryRandom` and many more — whose job is to create valid records. `CreateCustomer` assigns a number from the customer number series, fills the mandatory fields, and satisfies the table relations the platform enforces; `CreateItem` does the same for items. Hand-rolling `Customer.Init`/`Customer.Insert` with invented values skips the number series and any field a future app version adds as mandatory, so the fixture is invalid the moment it is created and rots silently as the schema evolves. The library codeunits also encode fixture *ordering*: because a `TableRelation` field is checked on `Validate` and `Insert(true)`, every parent a foreign key points to must already exist when the dependent record is built. Assemble fixtures top-down — customer and item before the sales line that references them — or the relation check aborts the test at runtime with a data error rather than an assertion. Prefer the Library codeunits for prerequisite data: they encode the setup the platform requires and are maintained alongside the base app.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Reach for the matching Library codeunit before writing manual record setup: `LibrarySales.CreateCustomer`, `LibrarySales.CreateSalesHeader`/`CreateSalesLine`, `LibraryInventory.CreateItem`, `LibraryERM.CreateGLAccount`, and `LibraryRandom.RandInt`/`RandDec` for values. Create the prerequisite parents first and reference their primary keys from dependent records, and `Validate` the foreign-key field so the `TableRelation` — and any field-validation logic — runs exactly as it would in production. Pass the records they return into the code under test. The fixtures stay valid across upgrades because the library — not your test — owns the knowledge of what a well-formed record requires.
|
||||
|
||||
See sample: `use-library-codeunits-for-test-fixtures.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`Customer.Init(); Customer."No." := 'X'; Customer.Insert();` — a record with a hand-picked primary key, no number-series entry, and none of the mandatory fields a real customer needs. It compiles and may even insert, but it bypasses setup the production code assumes, and it breaks the first time the schema gains a required field the test does not know about.
|
||||
|
||||
See sample: `use-library-codeunits-for-test-fixtures.bad.al`.
|
||||
|
|
@ -0,0 +1,12 @@
|
|||
// Bad: SetSelectionFilter with cursor-only (no explicit multi-selection) produces
|
||||
// a primary key filter for just that one row. The codeunit receives only that row;
|
||||
// the rest of the visible list is silently skipped with no error raised.
|
||||
trigger OnAction()
|
||||
var
|
||||
PriceListHeader: Record "Price List Header";
|
||||
TempErrorMessage: Record "Error Message" temporary;
|
||||
ProcessingCodeunit: Codeunit "My Batch Processor";
|
||||
begin
|
||||
CurrPage.SetSelectionFilter(PriceListHeader);
|
||||
ProcessingCodeunit.RunBatch(PriceListHeader, TempErrorMessage);
|
||||
end;
|
||||
|
|
@ -0,0 +1,14 @@
|
|||
// Good: check MarkedOnly before deciding which scope to process.
|
||||
// When MarkedOnly is false (cursor-only or Ctrl+A) fall back to Copy(Rec)
|
||||
// so every record visible in the page view is included.
|
||||
trigger OnAction()
|
||||
var
|
||||
PriceListHeader: Record "Price List Header";
|
||||
TempErrorMessage: Record "Error Message" temporary;
|
||||
ProcessingCodeunit: Codeunit "My Batch Processor";
|
||||
begin
|
||||
CurrPage.SetSelectionFilter(PriceListHeader);
|
||||
if not PriceListHeader.MarkedOnly then
|
||||
PriceListHeader.Copy(Rec);
|
||||
ProcessingCodeunit.RunBatch(PriceListHeader, TempErrorMessage);
|
||||
end;
|
||||
28
microsoft/knowledge/ui/set-selection-filter-list-scope.md
Normal file
28
microsoft/knowledge/ui/set-selection-filter-list-scope.md
Normal file
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: ui
|
||||
keywords: [set-selection-filter, marked-only, list-page, bulk-action, batch-action, selection-scope, copy-rec]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Preserve list scope after `SetSelectionFilter`
|
||||
|
||||
## Description
|
||||
|
||||
`CurrPage.SetSelectionFilter(Rec)` behaves differently depending on whether the user explicitly multi-selected rows. When no rows are marked — the cursor is simply positioned on a row — the method writes a primary key filter for that single row and leaves `MarkedOnly` as false. When the user explicitly selected multiple rows, the method marks those records and sets `MarkedOnly` to true. A batch action that calls `SetSelectionFilter` and then passes the record directly to a processing codeunit will therefore silently restrict to one row whenever the user has not made an explicit selection, which is almost never the intended behaviour for an action labelled "Verify All" or "Post All".
|
||||
|
||||
The base platform avoids this ambiguity by routing batch list actions through Reports: the Report request page shows the derived filter and lets the user correct it before running. A direct codeunit call has no such safety net and must resolve the scope explicitly.
|
||||
|
||||
## Best Practice
|
||||
|
||||
After calling `SetSelectionFilter`, test `MarkedOnly`. When it is false — meaning the user made no explicit selection, or selected all rows with Ctrl+A — discard the single-row primary key filter by copying the page source record (`Copy(Rec)`), which carries the full page view including all active filter groups. When `MarkedOnly` is true the user made a deliberate selection and that filter should be respected as-is. Refer to `set-selection-filter-list-scope.good.al` for the pattern.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Passing the result of `SetSelectionFilter` directly to a processing codeunit without checking `MarkedOnly`. When the user runs the action with the cursor on row three and no rows highlighted, the codeunit receives a filter that matches only row three. The action appears to succeed but processes a fraction of the intended scope. The defect is hard to notice because no error is raised and the single-row run completes without complaint. See `set-selection-filter-list-scope.bad.al`.
|
||||
|
||||
## See also
|
||||
|
||||
`Page.SetSelectionFilter` — https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/methods-auto/page/page-setselectionfilter-method
|
||||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
bc-version: [21..]
|
||||
domain: upgrade
|
||||
keywords: [datatransfer, large-dataset, bulk-update, modifyall, copyfields, new-field]
|
||||
technologies: [al]
|
||||
|
|
|
|||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Add table
Add a link
Reference in a new issue