mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-07 01:46:53 +01:00
Add 15 community knowledge articles from BC Code Intel ingest
Ingests net-new /community knowledge from BC Code Intelligence, surviving the admission test, gray-zone salvage, and dedup against the full corpus. Domains: ui (6), error-handling (3), performance (2), upgrade (1), appsource (1), security (1), telemetry (1). The two BC24 No. Series migration drafts are merged into one article. Adds good/bad AL samples for the clean-fit articles (error-handling, performance, security, telemetry). UI and appsource remain knowledge-only. Validator and knowledge-index checks pass (207 articles). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
6281e7e39a
commit
f5156c61de
29 changed files with 601 additions and 0 deletions
|
|
@ -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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue