mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-06 07:06:54 +01:00
Resolve evaluation/review-fixtures.json semantically: data-modeling articles list is the union of main's list and this PR's nine articles; everything else is taken from main unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
commit
704dc23951
111 changed files with 3057 additions and 56 deletions
|
|
@ -0,0 +1,9 @@
|
|||
codeunit 50101 "Batch Job Runner"
|
||||
{
|
||||
procedure AdvanceToNextBusinessDay()
|
||||
begin
|
||||
// Anti-pattern: repurposes the user's session WorkDate as a
|
||||
// scratch variable for unrelated business logic.
|
||||
WorkDate(CalcDate('<1D>', WorkDate()));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,11 @@
|
|||
codeunit 50100 "Posting Date Helper"
|
||||
{
|
||||
procedure GetDefaultPostingDate(): Date
|
||||
var
|
||||
PostingDate: Date;
|
||||
begin
|
||||
// Read the work date to default a value; never write to it.
|
||||
PostingDate := WorkDate();
|
||||
exit(PostingDate);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,58 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [workdate, session-setting, user-control, side-effect]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Application code must not change the WorkDate
|
||||
|
||||
## Description
|
||||
|
||||
The work date is a per-user session setting the user controls from the
|
||||
client (the date shown in the top-right corner, used to default posting
|
||||
dates and date filters). Business logic unrelated to that setting must not
|
||||
call `WorkDate(NewDate)` as a side effect of doing something else — that
|
||||
silently changes what the user sees and defaults to for the rest of their
|
||||
session, a surprising, hard-to-trace behavior change the user never asked
|
||||
for and has no visibility into. This is not a blanket ban on the setter
|
||||
itself: BCApps' own demo-data generators legitimately save the current
|
||||
work date, set a specific one to backdate the data they create, and
|
||||
restore it afterward (see `CreateDemoEDocsBE.Codeunit.al`'s
|
||||
`WorkDate(SampleInvoiceDate)` / `WorkDate(SavedWorkDate)` pair), and test
|
||||
codeunits routinely set `WorkDate` deliberately to control the date context
|
||||
a test runs under (hundreds of calls across BCApps' test suite, for
|
||||
example `SustainabilityPostingTest.Codeunit.al`). Both are the code's
|
||||
*actual purpose*, not a side effect of something unrelated.
|
||||
|
||||
This is a call-direction distinction for the read side: reading the
|
||||
current work date via `WorkDate` (or `WorkDate()` with no argument) is
|
||||
always fine.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Read the work date to default a value. Only write to it when changing it
|
||||
*is* the operation being performed — implementing the user's own
|
||||
work-date/settings action, or a test or demo-data routine that deliberately
|
||||
establishes a date context (saving and restoring the prior value if the
|
||||
routine must leave the session as it found it). Business logic that exists
|
||||
to do something else must never write `WorkDate` as an incidental side
|
||||
effect; if a calculation needs a specific date, pass or compute that date
|
||||
as a local variable instead.
|
||||
|
||||
See sample: [`code-must-not-change-workdate.good.al`](code-must-not-change-workdate.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Setting the work date from within a codeunit, report, or page action whose
|
||||
purpose is unrelated to the user's date preference — for example, a
|
||||
posting or calculation routine that calls `WorkDate(SomeDate)` to make its
|
||||
own logic simpler. This changes session state the user owns for the
|
||||
duration of a call that was never about the work date, and never restores
|
||||
it. This is a different case from a test or demo-data routine explicitly
|
||||
declaring a date context: the anti-pattern is unrelated logic silently
|
||||
mutating state it does not own, not the setter form itself.
|
||||
|
||||
See sample: [`code-must-not-change-workdate.bad.al`](code-must-not-change-workdate.bad.al).
|
||||
|
|
@ -0,0 +1,11 @@
|
|||
table 50111 "Sample Item Card"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(50; Picture; BLOB)
|
||||
{
|
||||
Caption = 'Picture';
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,11 @@
|
|||
table 50110 "Sample Item Card"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(50; Picture; Media)
|
||||
{
|
||||
Caption = 'Picture';
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,49 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [blob, media, mediaset, picture-field, image-field, table-design]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Pictures must be stored in a Media/MediaSet field, not BLOB
|
||||
|
||||
## Description
|
||||
|
||||
`BLOB` is still a valid AL field type for arbitrary binary data, but it is
|
||||
not the right choice for storing pictures or images. The current
|
||||
recommendation is the `Media` field type for a single image, or
|
||||
`MediaSet` when a record needs several independent images (e.g. multiple
|
||||
product photos) — `MediaSet` is a collection of separately-imported media
|
||||
objects, each with its own identity; it does not generate resized variants
|
||||
or thumbnails on its own, and displaying more than one item still requires
|
||||
custom page handling. Media/MediaSet integrate with the platform's
|
||||
picture control and media repository, which a plain `BLOB` field does not
|
||||
— but any derived preview or thumbnail image still has to be generated
|
||||
explicitly and stored in its own field, regardless of which type holds the
|
||||
source image.
|
||||
|
||||
`BLOB` remains the correct choice for genuinely arbitrary binary payloads
|
||||
that are not images and don't benefit from the media pipeline (e.g. a raw
|
||||
file attachment blob unrelated to picture rendering).
|
||||
|
||||
## Best Practice
|
||||
|
||||
Use `Media` for a single image, or `MediaSet` for multiple independent
|
||||
images, for any field that holds a picture.
|
||||
|
||||
See sample: [`pictures-must-use-media-not-blob.good.al`](pictures-must-use-media-not-blob.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A `BLOB` field named "Picture" compiles and stores the image bytes, but
|
||||
it misses the picture control integration and media repository that a
|
||||
`Media`/`MediaSet` field provides for free — the anti pattern is choosing
|
||||
`BLOB` for image storage out of habit rather than recognizing that the
|
||||
field is holding a picture, not generic binary data. A related anti
|
||||
pattern: assuming `MediaSet` gives automatic image variants or thumbnails
|
||||
because it sounds like a collection with derived versions — it is only a
|
||||
collection of independently-imported media objects.
|
||||
|
||||
See sample: [`pictures-must-use-media-not-blob.bad.al`](pictures-must-use-media-not-blob.bad.al).
|
||||
|
|
@ -0,0 +1,17 @@
|
|||
table 50121 "Sample Ledger Entry"
|
||||
{
|
||||
fields
|
||||
{
|
||||
// Anti-pattern: a Ledger table's key must never be user-editable.
|
||||
field(1; "Entry No."; Integer) { }
|
||||
field(2; "Posting Date"; Date) { }
|
||||
field(3; Amount; Decimal) { }
|
||||
}
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.") { Clustered = true; }
|
||||
}
|
||||
// No AutoIncrement, no guard against manual insert/delete — a user or
|
||||
// integration can renumber or remove entries, breaking the Ledger
|
||||
// type's audit-trail guarantee.
|
||||
}
|
||||
|
|
@ -0,0 +1,16 @@
|
|||
table 50120 "Sample Ledger Entry"
|
||||
{
|
||||
fields
|
||||
{
|
||||
// Ledger primary key: Integer "Entry No.", set only by posting.
|
||||
field(1; "Entry No."; Integer) { AutoIncrement = true; }
|
||||
field(2; "Posting Date"; Date) { }
|
||||
field(3; Amount; Decimal) { }
|
||||
}
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.") { Clustered = true; }
|
||||
}
|
||||
// No user-facing Insert/Delete/Modify path is exposed; rows are
|
||||
// created exclusively by the posting routine.
|
||||
}
|
||||
|
|
@ -0,0 +1,99 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: data-modeling
|
||||
keywords: [tables, table-design, naming-conventions, primary-key, master-table, ledger-table, journal-table, register-table, document-table, setup-table, subsidiary-table, supplemental-table]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Tables must match one of Business Central's nine table-type conventions
|
||||
|
||||
## Description
|
||||
|
||||
Business Central's Base Application follows nine recurring table types —
|
||||
Master, Supplemental, Subsidiary, Ledger, Register, Journal, Document,
|
||||
Document History, and Setup. Each type fixes a naming pattern, a
|
||||
primary-key shape, and a set of associated pages. A new or extended table
|
||||
whose design doesn't match the conventions of its own type is either
|
||||
misclassified or built inconsistently with the rest of the application,
|
||||
and should be flagged in review even if it compiles. Before assigning a
|
||||
primary key or naming a new table, first identify which of the nine types
|
||||
it is — that answer fixes almost every other design decision.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Match the table's design to its type:
|
||||
|
||||
1. **Master** (Customer, Item) — one record is the subject; primary key
|
||||
`Code[20]` named `No.`; description field in `DataCaptionFields`; Card +
|
||||
List (+ Statistics) pages.
|
||||
2. **Supplemental** (Currency, Language) — used across functional areas;
|
||||
primary key `Code[10]` named `Code`; one List page, plural name, set as
|
||||
`LookupPageID`.
|
||||
3. **Subsidiary** (Item Vendor) — subsidiary to a Master/Supplemental
|
||||
table; primary key is the parent key field(s), optionally + `Line No.`;
|
||||
page shape depends on whether the table carries its own identity: a
|
||||
pure parent-join table (Item Vendor) typically gets a plain List page
|
||||
filtered by the calling page, while a subsidiary table that supplements
|
||||
a master record with its own identity — parent key + own code, e.g.
|
||||
Ship-to Address, Customer/Vendor Bank Account — commonly gets a
|
||||
List+Card pair instead, for direct editing of that record.
|
||||
4. **Ledger** (Cust. Ledger Entry) — transactional record of a functional
|
||||
area; primary key `Integer` `Entry No.`, always auto-generated by
|
||||
posting, never user-editable, no free add/delete; List page as
|
||||
`LookupPageID`/`DrillDownPageID`.
|
||||
5. **Register** (G/L Register) — table of contents for its Ledger, one row
|
||||
per posting run; primary key `Integer` `No.`, auto-generated; carries
|
||||
`From Entry No.`/`To Entry No.`; List page with a link to the Ledger.
|
||||
6. **Journal** (Resource Journal Line) — where users enter data before
|
||||
posting to a Ledger; primary key Template + Batch + `Integer` `Line No.`;
|
||||
Worksheet page with `AutoSplitKey`, a Posting action, and a link to the
|
||||
Ledger.
|
||||
7. **Document** (Sales Header/Line) — posts to Ledgers via Journals, not
|
||||
directly; Header primary key `Code[20]` `No.` (or + `Option Document
|
||||
Type`); Line primary key = Header key renamed `<Document> No.` +
|
||||
`Integer Line No.`; Document/Card page with a Posting action and a lines
|
||||
subpage.
|
||||
8. **Document History** (Posted Sales Invoice Header/Line) — posted copy of
|
||||
a Document table, created during posting; mirrors the source table's
|
||||
fields; never user-editable; same page shape but the Line-equivalent is
|
||||
a List page, not a Worksheet.
|
||||
9. **Setup** (General Ledger Setup) — exactly one record for a functional
|
||||
area; primary key `Code[10]` named `Primary Key`, always blank; one page
|
||||
with the key field hidden, whose `OnOpenPage` creates the singleton the
|
||||
first time it's opened (`Reset()` → `Get()` → if not found, `Init()` →
|
||||
`Insert()`) rather than assuming the record pre-exists.
|
||||
|
||||
A table named "Setup" that holds more than one record follows Subsidiary
|
||||
rules instead — the name alone is not proof of type. When a table's
|
||||
identity can't be resolved from its definition alone (e.g. a "Setup"-named
|
||||
table with a real business-field key and no page), say so explicitly
|
||||
rather than forcing a classification; settling it requires checking actual
|
||||
row cardinality or call sites, not just the object definition.
|
||||
|
||||
These nine types cover Business Central's *business-record* tables — they
|
||||
are not an exhaustive catalogue of every legitimate table shape. A
|
||||
temporary/buffer table, a work queue, a log or telemetry table, a
|
||||
cross-reference/mapping table with no business meaning of its own, or a
|
||||
staging/working table used only inside one process is not required to fit
|
||||
any of the nine, and forcing one into the nearest-looking type (usually
|
||||
Ledger, because it has an `Integer` key, or Subsidiary, because it has a
|
||||
composite key) produces a harmful redesign recommendation for a table that
|
||||
was never meant to carry that type's guarantees. Apply this rule only when
|
||||
the table's name, fields, or usage genuinely establish it as one of the
|
||||
nine business-record types; when nothing points that way, this rule simply
|
||||
does not apply — that is not the same as an unresolved classification.
|
||||
|
||||
See sample: [`table-design-must-match-bc-table-type-conventions.good.al`](table-design-must-match-bc-table-type-conventions.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A table that mixes conventions from two types — for example, a "Ledger"
|
||||
table with a user-editable primary key that lets users freely insert or
|
||||
delete rows — is not "flexible", it is either misclassified or has skipped
|
||||
a design step. A Ledger table's `Entry No.` must come only from the
|
||||
posting routine; exposing it as an editable field breaks the type's core
|
||||
guarantee that entries are an immutable, sequential audit trail.
|
||||
|
||||
See sample: [`table-design-must-match-bc-table-type-conventions.bad.al`](table-design-must-match-bc-table-type-conventions.bad.al).
|
||||
Loading…
Add table
Add a link
Reference in a new issue