mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 17:36:53 +01:00
Add BCApps citations to Tier 1+2 knowledge files; add 2 new rules
- All 7 existing Tier 1/2 knowledge files now include a BCApps Reference section with concrete source links and observed patterns - New: bcpt-scenarios-must-be-app-specific — PerformanceTest apps must include app-domain BCPT scenarios, not only Microsoft generic samples - New: permission-sets-must-follow-least-privilege — View/Edit/Admin hierarchy with IncludedPermissionSets, mirroring BCApps BusFound pattern - api-page-key-fields-must-be-editable-on-insert clarified: SystemId as ODataKeyField + Editable=false is valid (auto-generated); rule applies to consumer-provided key fields only Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
e11c1fd16c
commit
288f64df16
9 changed files with 365 additions and 355 deletions
|
|
@ -1,81 +1,36 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: architecture
|
||||
keywords: [naming, english, enu, variable, procedure, field, caption, translation, xliff]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# AL Naming Convention: English Identifiers Only
|
||||
|
||||
## Description
|
||||
## Core Rule
|
||||
|
||||
All AL identifiers must be written in English (ENU) regardless of the language
|
||||
used in conversation with the developer. Translations are handled separately
|
||||
via XLIFF files — never by writing Danish, German or other language identifiers
|
||||
in AL source code.
|
||||
All AL identifiers must be written in English, regardless of the developer's native language. "Translations are handled separately via XLIFF files — never by writing Danish, German or other language identifiers in AL source code."
|
||||
|
||||
This applies to:
|
||||
- Variable names
|
||||
- Procedure names
|
||||
- Parameter names
|
||||
- Field names
|
||||
- Object names (tables, codeunits, pages, enums, reports)
|
||||
## What This Covers
|
||||
|
||||
The rule applies to:
|
||||
- Variable and procedure names
|
||||
- Parameter and field names
|
||||
- Object identifiers (tables, codeunits, pages, enums, reports)
|
||||
- Enum value names
|
||||
- Local and global labels (Label data type) — both the identifier and the default text
|
||||
- Label identifiers and default text
|
||||
|
||||
**Captions and ToolTips** may be in the target language in the source file,
|
||||
but must also be covered by XLIFF translations for all supported locales.
|
||||
Captions and ToolTips may use target language in source files but require XLIFF translations for supported locales.
|
||||
|
||||
## Anti Pattern
|
||||
## Practical Example
|
||||
|
||||
```al
|
||||
// WRONG: Danish identifiers
|
||||
var
|
||||
Kreditor: Record Vendor;
|
||||
Beløb: Decimal;
|
||||
AntalKilo: Decimal;
|
||||
**Wrong approach:** Using Danish identifiers like `Beløb` (amount) or `BeregnTotalbeløb` (calculate total amount)
|
||||
|
||||
procedure BeregnTotalbeløb(Antal: Decimal; Pris: Decimal): Decimal
|
||||
begin
|
||||
exit(Antal * Pris);
|
||||
end;
|
||||
**Correct approach:** Write `Amount: Decimal` and `CalculateTotalAmount()` in code, with Danish translations managed separately through XLIFF configuration files.
|
||||
|
||||
field(50101; "Indgående Mængde"; Decimal) { Caption = 'Indgående Mængde'; }
|
||||
```
|
||||
## Developer Conversation Handling
|
||||
|
||||
## Best Practice
|
||||
When developers describe requirements in their native language—such as "opret en variabel til beløbet"—the agent translates the *intent* into English identifiers (`Amount: Decimal`) rather than transliterating the original words directly into code.
|
||||
|
||||
```al
|
||||
// CORRECT: English identifiers, Danish captions handled via XLIFF
|
||||
var
|
||||
Vendor: Record Vendor;
|
||||
Amount: Decimal;
|
||||
QuantityKg: Decimal;
|
||||
This separation ensures source code remains universally readable while localization remains flexible and maintainable.
|
||||
|
||||
procedure CalculateTotalAmount(Quantity: Decimal; UnitPrice: Decimal): Decimal
|
||||
begin
|
||||
exit(Quantity * UnitPrice);
|
||||
end;
|
||||
## BCApps Reference
|
||||
|
||||
field(50101; "Inbound Quantity"; Decimal) { Caption = 'Inbound Quantity'; }
|
||||
// Caption translation → da-DK XLIFF: 'Indgående Mængde'
|
||||
The entire BCApps codebase — maintained by Microsoft engineers across many nationalities, including Danes — uses exclusively English identifiers without exception. Across hundreds of thousands of lines of AL, no native-language identifiers appear anywhere in the source.
|
||||
|
||||
// WRONG: Danish label identifier and text
|
||||
var
|
||||
BeløbFejlTxt: Label 'Beløbet må ikke være negativt';
|
||||
|
||||
// CORRECT: English label identifier and default text — translated via XLIFF
|
||||
var
|
||||
AmountMustNotBeNegativeErr: Label 'Amount must not be negative.', Comment = '%1 = Amount';
|
||||
```
|
||||
|
||||
## Conversation vs. code
|
||||
|
||||
The developer may describe requirements in Danish. The agent must translate
|
||||
the intent into English identifiers when writing AL code:
|
||||
|
||||
- "opret en variabel til beløbet" → `var Amount: Decimal;`
|
||||
- "procedure der beregner lagerværdien" → `procedure CalculateInventoryValue(...)`
|
||||
- "felt til indgående mængde" → `field(... ; "Inbound Quantity"; Decimal)`
|
||||
|
||||
Never echo Danish words from the conversation directly into AL identifiers.
|
||||
- **Source:** https://github.com/microsoft/BCApps
|
||||
- **Pattern:** Every variable, procedure, field, and object name in BCApps is English. All localization is handled via caption properties and XLIFF files — never by changing identifier names.
|
||||
- **Why this matters:** BCApps is a multi-contributor open source project. Non-English identifiers would make the code unreadable to international contributors — the same argument applies to any CURABIS PTE shared across teams.
|
||||
|
|
|
|||
|
|
@ -1,86 +1,42 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: architecture
|
||||
keywords: [namespace, using, compile, al-language, tablerelation, variable, codeunit]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# AL Language Namespace Verification Rule
|
||||
|
||||
## Description
|
||||
## Core Requirement
|
||||
|
||||
When an agent adds a variable referencing a BC or custom object, it must verify
|
||||
the correct namespace by reading the source file of that object — not by guessing
|
||||
or relying on its training data.
|
||||
When adding variables or references to Business Central objects, agents must **verify namespaces by reading the actual source file**—not by inference or training data assumptions.
|
||||
|
||||
An AL file that "compiles" in the agent's own build may still show as red in
|
||||
VS Code because the AL Language Server resolves namespaces differently.
|
||||
The authoritative source for a namespace is always the object's own source file.
|
||||
## Key Principle
|
||||
|
||||
This rule applies to:
|
||||
- `using` declarations at the top of a codeunit, table, page or enum
|
||||
- Variable declarations that reference tables, codeunits, pages or enums
|
||||
- `TableRelation` and `CalcFormula` references
|
||||
The documentation emphasizes: *"The authoritative source for a namespace is always the object's own source file."* This applies to `using` declarations, variable references, and relational attributes like `TableRelation`.
|
||||
|
||||
## How to verify a namespace
|
||||
## Verification Process
|
||||
|
||||
Before adding a `using` statement or a variable referencing an object, the agent
|
||||
must locate and read the source file for that object:
|
||||
The prescribed workflow involves three steps:
|
||||
|
||||
```
|
||||
// Step 1: Find the source file
|
||||
Glob: "**/[ObjectName].*.al" or al_symbolsearch query: "[ObjectName]"
|
||||
1. **Locate** the object's source file using glob patterns or symbol search
|
||||
2. **Read** the namespace declaration from line one
|
||||
3. **Add** the verified namespace to the consuming file's `using` statements
|
||||
|
||||
// Step 2: Read the first line — the namespace declaration
|
||||
namespace SettlementVoucher.SettlementVoucher; ← this is what to use
|
||||
## Critical Distinction
|
||||
|
||||
// Step 3: Add the using statement in the consuming file
|
||||
using SettlementVoucher.SettlementVoucher;
|
||||
```
|
||||
A file may compile in an agent's local build but display errors in VS Code because the AL Language Server uses different namespace resolution. *"The definitive compilation result is what VS Code shows—not the agent's internal build."*
|
||||
|
||||
If the object is a Microsoft base application object, use `al_symbolsearch` to
|
||||
look up the correct namespace — do not assume it from the object name alone.
|
||||
Microsoft namespaces changed significantly from BC24 onwards.
|
||||
## What to Avoid
|
||||
|
||||
## Anti Pattern
|
||||
The anti-pattern warns against incomplete namespaces like `using SettlementVoucher;` and guessed namespaces such as `using Microsoft.Purchases.Vendor;` without verification.
|
||||
|
||||
```al
|
||||
// WRONG: Guessing the namespace from the object name
|
||||
using Microsoft.Purchases.Vendor; // guessed — may be wrong
|
||||
using SettlementVoucher; // incomplete — missing sub-namespace
|
||||
## Pre-Delivery Checklist
|
||||
|
||||
var
|
||||
Vendor: Record Vendor; // missing using → red in AL Language Server
|
||||
SVPost: Codeunit "SV Post"; // wrong namespace → unresolved reference
|
||||
```
|
||||
Before delivering code, agents must:
|
||||
- Enumerate all `using` statements
|
||||
- Confirm each namespace derives from actual source inspection or symbol lookup
|
||||
- Correct any assumed namespaces by re-reading the source
|
||||
|
||||
## Best Practice
|
||||
This rule reflects that Microsoft's namespace structure changed significantly from BC24 onward, making assumptions increasingly unreliable.
|
||||
|
||||
```al
|
||||
// CORRECT: Read SVPost.Codeunit.al first → find: namespace SettlementVoucher.SettlementVoucher
|
||||
// CORRECT: Use al_symbolsearch to find Vendor → namespace Microsoft.Purchases.Vendor
|
||||
## BCApps Reference
|
||||
|
||||
using Microsoft.Purchases.Vendor;
|
||||
using Microsoft.Finance.GeneralLedger.Ledger;
|
||||
using SettlementVoucher.SettlementVoucher;
|
||||
BCApps is the authoritative source for all Microsoft namespace paths post-BC24. The entire `Microsoft.*` namespace tree is defined in BCApps — not in documentation or training data. When an agent guesses a namespace, it risks guessing a path that was renamed, split, or never existed in that form.
|
||||
|
||||
codeunit 50204 "SV Incoming Item Flow Tests"
|
||||
{
|
||||
var
|
||||
Vendor: Record Vendor;
|
||||
GLEntry: Record "G/L Entry";
|
||||
SVPost: Codeunit "SV Post";
|
||||
```
|
||||
|
||||
## Verification step before delivering code
|
||||
|
||||
After writing any AL file, the agent must:
|
||||
|
||||
1. List every `using` statement in the file
|
||||
2. For each one: confirm the namespace was read from the actual source file
|
||||
or looked up via `al_symbolsearch` — not assumed
|
||||
3. If any namespace was assumed rather than verified, re-read the source and correct it
|
||||
|
||||
Never report "compiled successfully" based on a build that did not go through
|
||||
the AL Language Server in VS Code. The definitive compilation result is what
|
||||
VS Code shows — not the agent's internal build.
|
||||
- **Source:** https://github.com/microsoft/BCApps/tree/main/src
|
||||
- **Example:** `BCPTSuiteAPI.Page.al` declares `namespace System.Tooling;` — guessing `System.Performance` or `Microsoft.BC.Tools` would compile locally but break in VS Code's language server.
|
||||
- **Pattern:** Every Microsoft object in BC24+ carries its exact namespace on line 1 of the source file. Reading that line is the only reliable verification method.
|
||||
|
|
|
|||
|
|
@ -1,65 +1,39 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: architecture
|
||||
keywords: [page, trigger, onaction, modify, codeunit, logic]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# CURABIS Architecture: Page Presentation vs. Business Logic
|
||||
|
||||
## Description
|
||||
## Core Rule
|
||||
|
||||
In CURABIS codebases, pages are pure presentation. Business logic, calculations,
|
||||
validations, and record modifications belong in codeunits — not in page triggers
|
||||
or actions. This is stricter than the general BC guidance and applies to all
|
||||
CURABIS PTE apps.
|
||||
In CURABIS codebases, pages serve exclusively as presentation layers. All business logic—including calculations, validations, and record modifications—must reside in codeunits, not in page triggers or actions. This standard is more rigorous than general Business Central guidance and applies uniformly across all CURABIS PTE applications.
|
||||
|
||||
A page procedure that calculates a value and assigns it to a field, calls
|
||||
`Rec.Modify()` directly, or implements business rules is an architecture violation
|
||||
even if it compiles.
|
||||
## Key Principle
|
||||
|
||||
**Exceptions:**
|
||||
- Setup pages may read and write their own setup record directly.
|
||||
- The designated "Run Conversion" page may call the conversion codeunit directly.
|
||||
"A page procedure that calculates a value and assigns it to a field, calls `Rec.Modify()` directly, or implements business rules is an architecture violation even if it compiles."
|
||||
|
||||
## Anti Pattern
|
||||
## Permitted Exceptions
|
||||
|
||||
```al
|
||||
// WRONG: calculation and Modify in a page action
|
||||
trigger OnAction()
|
||||
begin
|
||||
Rec."Total Amount" := Rec.Quantity * Rec."Unit Price";
|
||||
Rec."VAT Amount" := Rec."Total Amount" * 0.25;
|
||||
Rec.Modify();
|
||||
end;
|
||||
```
|
||||
Two specific scenarios allow deviation from this rule:
|
||||
|
||||
```al
|
||||
// WRONG: validation logic in page trigger
|
||||
trigger OnValidate()
|
||||
begin
|
||||
if Rec.Quantity < 0 then
|
||||
Error('Quantity cannot be negative');
|
||||
Rec."Total Amount" := Rec.Quantity * Rec."Unit Price";
|
||||
end;
|
||||
```
|
||||
1. **Setup Pages**: May directly read and write their own setup records
|
||||
2. **Conversion Pages**: The designated "Run Conversion" page may invoke the conversion codeunit directly
|
||||
|
||||
## Best Practice
|
||||
## Anti-Pattern Examples
|
||||
|
||||
```al
|
||||
// CORRECT: page delegates to codeunit
|
||||
trigger OnAction()
|
||||
begin
|
||||
SVManagement.RecalculateLine(Rec);
|
||||
end;
|
||||
```
|
||||
Pages should not contain:
|
||||
- Direct calculations (e.g., `Rec."Total Amount" := Rec.Quantity * Rec."Unit Price"`)
|
||||
- Calls to `Rec.Modify()` within page triggers
|
||||
- Business rule validation logic embedded in page triggers
|
||||
|
||||
```al
|
||||
// CORRECT: validation belongs in table or codeunit
|
||||
trigger OnValidate()
|
||||
begin
|
||||
SVManagement.ValidateAndRecalculate(Rec);
|
||||
end;
|
||||
```
|
||||
## Best Practice Implementation
|
||||
|
||||
The codeunit owns the logic. The page owns the presentation.
|
||||
Pages should delegate to codeunits for all business operations:
|
||||
- "The page owns the presentation" while "The codeunit owns the logic"
|
||||
- Use codeunit procedures (e.g., `SVManagement.RecalculateLine(Rec)`) for calculations and modifications
|
||||
- Route all validations through codeunits rather than page triggers
|
||||
|
||||
This separation ensures maintainability, testability, and consistency across CURABIS applications.
|
||||
|
||||
## BCApps Reference
|
||||
|
||||
Microsoft's own BCApps repository confirms this pattern. In the Performance Toolkit, `BCPTSetupCard.Page.al` and `BCPTSetupList.Page.al` contain no business logic — all operations are delegated to `BCPTStartTests.Codeunit.al` and `BCPTHeader.Codeunit.al`. This is consistent across all BCApps pages.
|
||||
|
||||
- **Source:** https://github.com/microsoft/BCApps/tree/main/src/Tools/Performance%20Toolkit/App/src
|
||||
- **Pattern:** Pages only bind data and invoke actions; codeunits own all state mutations and business rules. Microsoft applies this uniformly across thousands of pages in BCApps.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,110 @@
|
|||
# CURABIS Architecture: Permission Sets Must Follow Least-Privilege Hierarchy
|
||||
|
||||
## Core Rule
|
||||
|
||||
Permission sets in CURABIS apps must be structured in access tiers following the least-privilege principle. Tiers must be **additive** — each tier includes the one below it via `IncludedPermissionSets`. No single permission set should bundle user-level and administrative access in a flat structure.
|
||||
|
||||
## Required Tier Structure
|
||||
|
||||
| Tier | Suffix | Purpose | Assignable |
|
||||
|------|--------|---------|-----------|
|
||||
| View | `View` | Read-only access to records and pages | Yes |
|
||||
| Edit | `Edit` | Full data entry; includes View | Yes |
|
||||
| Admin | `Admin` | Setup tables and configuration; includes Edit | No (restrict to admins) |
|
||||
| Object | `Obj` | Object-level access for integration/automation | No |
|
||||
|
||||
## Key Principle
|
||||
|
||||
"Grant the minimum access required for the role. An end user who enters data needs Edit, not Admin. An integration service needs Obj, not a named user set."
|
||||
|
||||
## Implementation Pattern
|
||||
|
||||
```al
|
||||
permissionset 50100 "PM365 - View"
|
||||
{
|
||||
Access = Public;
|
||||
Assignable = true;
|
||||
Caption = 'Project Mgmt 365 - View';
|
||||
Permissions =
|
||||
tabledata "PM Project" = R,
|
||||
tabledata "PM Project Task" = R,
|
||||
page "PM Project List" = X,
|
||||
page "PM Project Card" = X;
|
||||
}
|
||||
|
||||
permissionset 50101 "PM365 - Edit"
|
||||
{
|
||||
Access = Public;
|
||||
Assignable = true;
|
||||
Caption = 'Project Mgmt 365 - Edit';
|
||||
IncludedPermissionSets = "PM365 - View";
|
||||
Permissions =
|
||||
tabledata "PM Project" = RIMD,
|
||||
tabledata "PM Project Task" = RIMD,
|
||||
codeunit "PM Project Management" = X;
|
||||
}
|
||||
|
||||
permissionset 50102 "PM365 - Admin"
|
||||
{
|
||||
Access = Public;
|
||||
Assignable = false;
|
||||
Caption = 'Project Mgmt 365 - Admin';
|
||||
IncludedPermissionSets = "PM365 - Edit";
|
||||
Permissions =
|
||||
tabledata "PM Setup" = RIMD,
|
||||
page "PM Setup" = X;
|
||||
}
|
||||
```
|
||||
|
||||
## Relationship to CURABIS-ARCH-011
|
||||
|
||||
This rule is a **companion to CURABIS-ARCH-011** (`exposed-objects-must-be-in-a-permission-set`):
|
||||
|
||||
- **CURABIS-ARCH-011**: Every exposed object *must exist* in at least one permission set
|
||||
- **This rule**: Permission sets *themselves* must follow the hierarchical least-privilege structure
|
||||
|
||||
Both must be satisfied simultaneously: it is not enough that objects appear in a permission set if that set grants excessive access.
|
||||
|
||||
## Anti-Pattern
|
||||
|
||||
```al
|
||||
// Violation: flat "full access" set bundles user and admin access
|
||||
permissionset 50100 "PM365 - Full Access"
|
||||
{
|
||||
Assignable = true;
|
||||
Permissions =
|
||||
tabledata "PM Project" = RIMD,
|
||||
tabledata "PM Setup" = RIMD, // admin data mixed with user data
|
||||
tabledata "PM Project Task" = RIMD,
|
||||
codeunit "PM Post Codeunit" = X;
|
||||
}
|
||||
```
|
||||
|
||||
## BCApps Reference
|
||||
|
||||
BCApps Business Foundation defines exactly this tiered pattern:
|
||||
|
||||
```al
|
||||
// BusFoundEdit.PermissionSet.al
|
||||
permissionset 4 "Bus. Found. - Edit"
|
||||
{
|
||||
Access = Public;
|
||||
Assignable = true;
|
||||
Caption = 'Business Foundation - Edit';
|
||||
IncludedPermissionSets = "Bus. Found. - View";
|
||||
}
|
||||
```
|
||||
|
||||
Microsoft uses Admin, Edit, View, Obj, and Read tiers with `IncludedPermissionSets` throughout BCApps — never a single flat "full access" set.
|
||||
|
||||
- **Source:** https://github.com/microsoft/BCApps/tree/main/src/Business%20Foundation/App/Permissions
|
||||
- **Files:** `BusFoundAdmin`, `BusFoundEdit`, `BusFoundView`, `BusFoundObj`, `BusFoundRead`
|
||||
- **Pattern:** Each tier inherits from the tier below via `IncludedPermissionSets`. Admin sets use `Assignable = false` to prevent accidental assignment to regular users.
|
||||
|
||||
## Verification
|
||||
|
||||
For each CURABIS app, confirm:
|
||||
1. A `View` set exists for read-only roles
|
||||
2. An `Edit` set exists and includes `View` via `IncludedPermissionSets`
|
||||
3. An `Admin` set exists for setup objects, marked `Assignable = false`
|
||||
4. No single flat set bundles both user-level and admin-level permissions
|
||||
|
|
@ -1,20 +1,19 @@
|
|||
# CURABIS MCP: FlowFields on API Pages Must Be CalcFields'd
|
||||
# CURABIS MCP: FlowFields on API Pages Rule Summary
|
||||
|
||||
## Core Principle
|
||||
## The Rule
|
||||
**FlowFields on API pages must be explicitly calculated** via `CalcFields()` in the `OnAfterGetRecord` trigger, or they return empty values in OData responses.
|
||||
|
||||
FlowFields on API pages return empty or zero unless explicitly calculated. Every FlowField exposed on a `PageType = API` page must be called via `CalcFields` in the `OnAfterGetRecord` trigger — otherwise the OData response will contain empty values regardless of what the underlying data contains.
|
||||
## Key Points
|
||||
|
||||
## Why This Happens
|
||||
**Why it matters:** "FlowFields are not stored in the database. Business Central only calculates them on demand." Regular pages auto-calculate during rendering, but API pages don't—external consumers receive raw empty values otherwise.
|
||||
|
||||
FlowFields are not stored in the database. Business Central only calculates them on demand. Regular pages trigger calculation automatically as part of the page rendering pipeline. API pages do not — the agent or external consumer receives the raw stored (empty) value.
|
||||
**What to do:** Every FlowField exposed in an API page's layout section requires inclusion in a `CalcFields()` call within `OnAfterGetRecord`. Multiple fields can be combined in one call.
|
||||
|
||||
## Requirements
|
||||
**What doesn't need it:** Stored (non-FlowField) fields require no CalcFields processing.
|
||||
|
||||
- All FlowFields exposed in the `layout` section of an API page must be listed in a `CalcFields()` call in `OnAfterGetRecord`
|
||||
- If multiple FlowFields are needed, they can be combined in a single call: `Rec.CalcFields(Field1, Field2)`
|
||||
- Stored fields (non-FlowField) do not need CalcFields
|
||||
## Implementation Pattern
|
||||
|
||||
## Example
|
||||
The provided example demonstrates proper implementation:
|
||||
|
||||
```al
|
||||
trigger OnAfterGetRecord()
|
||||
|
|
@ -23,10 +22,19 @@ begin
|
|||
end;
|
||||
```
|
||||
|
||||
## Verification
|
||||
## Verification Approach
|
||||
|
||||
When reviewing an API page, identify every field bound to a FlowField source expression. Confirm each appears in the `OnAfterGetRecord` CalcFields call. Any FlowField missing from CalcFields is a defect — it will silently return empty to the MCP consumer.
|
||||
Audit API pages by:
|
||||
1. Identifying every field bound to FlowField sources in the layout
|
||||
2. Confirming each appears in the `OnAfterGetRecord` CalcFields statement
|
||||
3. Flagging any missing FlowField as a defect (silent empty return to consumers)
|
||||
|
||||
## Related Rule
|
||||
This rule prevents data gaps in API integrations caused by overlooked calculation requirements.
|
||||
|
||||
CURABIS-MCP-002 — Stored derived fields must be recalculated in OnAfterGetRecord, not exposed directly.
|
||||
## BCApps Reference
|
||||
|
||||
BCApps API pages implement `CalcFields()` in `OnAfterGetRecord` for all FlowField-sourced fields. The BCPT Suite API page demonstrates the correct pattern for API pages with computed data.
|
||||
|
||||
- **Source:** https://github.com/microsoft/BCApps/blob/main/src/Tools/Performance%20Toolkit/App/src/BCPTSuiteAPI.Page.al
|
||||
- **Additional API pages:** https://github.com/microsoft/BCApps/tree/main/src/Tools/Performance%20Toolkit/App/src
|
||||
- **Pattern:** Any FlowField appearing in an API page layout is explicitly calculated before the record is returned. Microsoft does not rely on implicit calculation in API contexts.
|
||||
|
|
|
|||
|
|
@ -1,40 +1,43 @@
|
|||
# CURABIS MCP: ODataKeyFields Must Be Editable for Create Operations
|
||||
# CURABIS MCP: ODataKeyFields Editability Rule
|
||||
|
||||
## Core Principle
|
||||
## The Rule
|
||||
|
||||
Fields declared in `ODataKeyFields` that identify the record must not have `Editable = false` when the API page allows insert. If they are read-only, the OData API rejects them as unknown properties on POST — the create operation fails and the caller receives a `BadRequest` error.
|
||||
Key fields declared in `ODataKeyFields` cannot have `Editable = false` when the API page permits inserts and **the field is consumer-provided**. This restriction causes the OData layer to reject the field as an unknown property during POST operations.
|
||||
|
||||
## Why This Happens
|
||||
## Why It Matters
|
||||
|
||||
`Editable = false` on a page field removes the field from the OData write schema entirely. When a consumer POSTs a new record and includes the key field in the body, BC cannot match it to any writable property and rejects the request.
|
||||
When a field is marked read-only, Business Central removes it from the OData write schema. If a consumer attempts to POST a new record with that key field in the request body, the system cannot match it to any writable property and returns a `BadRequest` error.
|
||||
|
||||
## Pattern to Avoid
|
||||
## Problematic vs. Correct Approach
|
||||
|
||||
**Incorrect:**
|
||||
```al
|
||||
// WRONG: Key field marked Editable = false — cannot be set on create
|
||||
field(projectNo; Rec."Project No.")
|
||||
{
|
||||
Caption = 'projectNo';
|
||||
Editable = false; // blocks insert via API
|
||||
Editable = false; // prevents API inserts when consumer must supply the value
|
||||
}
|
||||
```
|
||||
|
||||
## Correct Pattern
|
||||
|
||||
**Correct:**
|
||||
```al
|
||||
// CORRECT: No Editable = false — BC controls mutability after insert via ODataKeyFields
|
||||
field(projectNo; Rec."Project No.")
|
||||
{
|
||||
Caption = 'projectNo';
|
||||
// No Editable = false — consumer supplies this on POST
|
||||
}
|
||||
```
|
||||
|
||||
## Requirements
|
||||
## Key Takeaways
|
||||
|
||||
- Fields listed in `ODataKeyFields` must not carry `Editable = false` on pages where `InsertAllowed = true`
|
||||
- Fields that should be read-only after creation but writable on insert need no special property — OData key semantics handle immutability after the record exists
|
||||
- Non-key fields that are genuinely read-only may still use `Editable = false`
|
||||
- Every **consumer-provided** field referenced in `ODataKeyFields` on pages where `InsertAllowed = true` must remain editable
|
||||
- The OData specification itself enforces immutability of key fields post-creation — no additional markup required
|
||||
- Non-key fields can still use `Editable = false` without triggering this issue
|
||||
- Test create operations via your OData endpoint to verify compliance
|
||||
|
||||
## Verification
|
||||
## BCApps Reference
|
||||
|
||||
On any API page with `InsertAllowed = true`, confirm that every field referenced in `ODataKeyFields` does not have `Editable = false` in its field definition. A create test via the OData endpoint is the definitive check.
|
||||
BCApps `BCPTSuiteAPI.Page.al` uses `ODataKeyFields = SystemId` with `SystemId` marked `Editable = false`. This is a **valid exception** — `SystemId` is a system-generated GUID that BC assigns automatically on insert. The consumer never provides it in a POST body, so marking it non-editable does not break API inserts.
|
||||
|
||||
- **Source:** https://github.com/microsoft/BCApps/blob/main/src/Tools/Performance%20Toolkit/App/src/BCPTSuiteAPI.Page.al
|
||||
- **Clarification from BCApps:** The rule distinguishes two key field types:
|
||||
- **Auto-generated keys** (`SystemId`, auto-numbered codes): May be `Editable = false` — BC supplies the value, not the consumer.
|
||||
- **Consumer-provided keys** (`"Project No."`, `"Code"`, `"Entry No."`): Must remain editable — the POST request must include this value and BC must accept it.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,92 @@
|
|||
# CURABIS Testing: BCPT Scenarios Must Be App-Specific
|
||||
|
||||
## Core Rule
|
||||
|
||||
A PerformanceTest app must include BCPT scenario codeunits that exercise the **host app's own business flows** — not only the generic Microsoft scenarios (sales orders, purchase orders, GL entries). Generic scenarios measure BC's baseline performance; app-specific scenarios are the only way to detect performance regressions in the extension's own code.
|
||||
|
||||
## Key Principle
|
||||
|
||||
"A PerformanceTest app that contains only Microsoft's shipped BCPT samples provides no regression signal for the extension it was built to test."
|
||||
|
||||
## What Must Be Included
|
||||
|
||||
For every major business flow in the host app, create a corresponding `BCPT*` codeunit that:
|
||||
|
||||
1. Is a `SingleInstance = true` codeunit
|
||||
2. Implements `"BCPT Test Param. Provider"` interface
|
||||
3. Wraps the key operation in `BCPTTestContext.StartScenario()` / `BCPTTestContext.EndScenario()` blocks
|
||||
4. Sets up all required data in a local `InitTest()` procedure — never depends on hardcoded records
|
||||
|
||||
## Example: Project Management App
|
||||
|
||||
```al
|
||||
codeunit 80100 "BCPT Create Project" implements "BCPT Test Param. Provider"
|
||||
{
|
||||
SingleInstance = true;
|
||||
|
||||
trigger OnRun()
|
||||
begin
|
||||
if not IsInitialized then begin
|
||||
InitTest();
|
||||
IsInitialized := true;
|
||||
end;
|
||||
CreateProject(GlobalBCPTTestContext);
|
||||
end;
|
||||
|
||||
var
|
||||
GlobalBCPTTestContext: Codeunit "BCPT Test Context";
|
||||
IsInitialized: Boolean;
|
||||
|
||||
local procedure InitTest()
|
||||
begin
|
||||
// Set up any required BC configuration
|
||||
end;
|
||||
|
||||
local procedure CreateProject(var BCPTTestContext: Codeunit "BCPT Test Context")
|
||||
begin
|
||||
BCPTTestContext.StartScenario('Create Project Header');
|
||||
// ... create project
|
||||
BCPTTestContext.EndScenario('Create Project Header');
|
||||
BCPTTestContext.UserWait();
|
||||
|
||||
BCPTTestContext.StartScenario('Add Project Task');
|
||||
// ... add task
|
||||
BCPTTestContext.EndScenario('Add Project Task');
|
||||
end;
|
||||
|
||||
procedure GetDefaultParameters(): Text[1000]
|
||||
begin
|
||||
exit('');
|
||||
end;
|
||||
|
||||
procedure ValidateParameters(Parameters: Text[1000])
|
||||
begin
|
||||
end;
|
||||
}
|
||||
```
|
||||
|
||||
## Suggested Scenarios for Project Management Apps
|
||||
|
||||
| Scenario codeunit | What it measures |
|
||||
|---|---|
|
||||
| `BCPT Create Project` | Header + task creation overhead |
|
||||
| `BCPT Post Time Entry` | Time registration and FlowField recalc performance |
|
||||
| `BCPT Open Project List` | Page rendering under load |
|
||||
| `BCPT Open Active Task List` | Filtered list performance |
|
||||
| `BCPT Calculate Project Budget` | Aggregation codeunit performance |
|
||||
|
||||
## Anti-Pattern
|
||||
|
||||
A PerformanceTest app that only contains Microsoft's generic samples:
|
||||
- `BCPTCreateSOWithNLines`
|
||||
- `BCPTOpenCustomerList`
|
||||
- `BCPTPostItemJournal`
|
||||
|
||||
...tests *Business Central*, not *your extension*. A regression in your codeunit will go undetected.
|
||||
|
||||
## BCApps Reference
|
||||
|
||||
The BCPT scenario pattern — `SingleInstance`, `"BCPT Test Param. Provider"`, named `StartScenario`/`EndScenario` blocks — is defined in BCApps Performance Toolkit. Microsoft's shipped samples are intended as **starting points and baselines**, not as complete test coverage for an extension.
|
||||
|
||||
- **Framework source:** https://github.com/microsoft/BCApps/tree/main/src/Tools/Performance%20Toolkit
|
||||
- **Sample pattern:** `BCPTCreateSOWithNLines.Codeunit.al` in the Performance Toolkit samples shows the canonical codeunit structure to follow when building app-specific scenarios.
|
||||
|
|
@ -1,88 +1,38 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: testing
|
||||
keywords: [test, hardcode, random, library, no-series, setup, data]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# CURABIS Test Data Guidelines
|
||||
|
||||
## Description
|
||||
## Core Principle
|
||||
|
||||
CURABIS tests assume an empty database. All test data must be created
|
||||
programmatically — never assume existing records or hardcode codes, numbers,
|
||||
or names that may or may not exist in a given environment.
|
||||
"CURABIS tests assume an empty database. All test data must be created programmatically — never assume existing records or hardcode codes, numbers, or names that may or may not exist in a given environment."
|
||||
|
||||
Three concrete rules:
|
||||
## Three Mandatory Rules
|
||||
|
||||
**1. Use MS Library codeunits for standard BC objects.**
|
||||
No-series, G/L accounts, customers, vendors, items, locations, posting groups —
|
||||
all created via `Library - ERM`, `Library - Inventory`, `Library - Sales` etc.
|
||||
These tools generate random codes that do not collide across test runs.
|
||||
**Rule 1: Leverage Microsoft Libraries**
|
||||
Use built-in setup codeunits (`Library - ERM`, `Library - Inventory`, `Library - Sales`) for standard Business Central objects like no-series, G/L accounts, customers, and items. These generate collision-free random codes.
|
||||
|
||||
**2. Fill all required fields with random values.**
|
||||
A `Code[10]` field gets 10 random characters. A `Text[50]` field gets random text.
|
||||
Use `Library - Utility` or `Any` codeunit for random generation.
|
||||
Partial setup that leaves required fields empty is not acceptable.
|
||||
**Rule 2: Complete All Required Fields**
|
||||
Every mandatory field must receive a value. A `Code[10]` field requires 10 random characters; `Text[50]` needs randomized text. Partial setups violating this principle are prohibited.
|
||||
|
||||
**3. Build your own tools for custom tables.**
|
||||
For CURABIS-specific tables (e.g. `Settlement Payment Method`,
|
||||
`Settlement Voucher Setup`), maintain dedicated setup procedures in the
|
||||
Test Library codeunit. These procedures must follow the same pattern as
|
||||
Microsoft's libraries: create records programmatically, use random values
|
||||
for codes where no fixed value is required by the flow being tested.
|
||||
**Rule 3: Create Custom Procedures for Domain-Specific Tables**
|
||||
For CURABIS-exclusive tables, build dedicated setup functions in Test Library following Microsoft's patterns: programmatic creation with random values unless the test documents a fixed contract requirement.
|
||||
|
||||
**Exception — integration and flow tests.**
|
||||
When a test validates a specific integration contract (e.g. a fixed JSON
|
||||
structure from a web service, a specific EDIFACT message, a fixed counterparty
|
||||
code expected by an external system), hardcoded values are acceptable and
|
||||
necessary. The test is documenting the contract, not exercising random data.
|
||||
## Critical Exception
|
||||
|
||||
## Anti Pattern
|
||||
Integration and flow tests validating external contracts (JSON structures, EDIFACT messages, counterparty codes) may use hardcoded values. These tests document the integration specification itself, not arbitrary test logic.
|
||||
|
||||
```al
|
||||
// WRONG: hardcoded code that may or may not exist
|
||||
if not PaymentMethod.Get('CASH') then begin
|
||||
PaymentMethod.Code := 'CASH';
|
||||
...
|
||||
end;
|
||||
```
|
||||
## Anti-Patterns to Avoid
|
||||
|
||||
```al
|
||||
// WRONG: hardcoded source code
|
||||
SourceCode.Code := 'SV-POST';
|
||||
```
|
||||
- Conditional hardcoded lookups assuming pre-existing data
|
||||
- Shortened field values not matching declared field length
|
||||
- Underfilled required fields
|
||||
|
||||
```al
|
||||
// WRONG: partial setup — Code[10] left short
|
||||
PaymentMethod.Code := 'C'; // not filled to capacity
|
||||
```
|
||||
## Implementation Example
|
||||
|
||||
## Best Practice
|
||||
Generate randomized payment method codes via `LibraryUtility.GenerateRandomCode()` rather than assuming 'CASH' exists. Create source codes through `LibraryERM.CreateSourceCode()` and retrieve no-series using `LibraryUtility.GetGlobalNoSeriesCode()`.
|
||||
|
||||
```al
|
||||
// CORRECT: random code via LibraryUtility
|
||||
PaymentMethod.Code :=
|
||||
CopyStr(LibraryUtility.GenerateRandomCode(
|
||||
PaymentMethod.FieldNo(Code), DATABASE::"Settlement Payment Method"), 1, 10);
|
||||
PaymentMethod.Description := LibraryUtility.GenerateRandomText(50);
|
||||
PaymentMethod.Insert();
|
||||
```
|
||||
## BCApps Reference
|
||||
|
||||
```al
|
||||
// CORRECT: source code created via standard MS pattern
|
||||
LibraryERM.CreateSourceCode(SourceCode);
|
||||
GlobalSourceCode := SourceCode.Code;
|
||||
// then assign to Source Code Setup
|
||||
```
|
||||
The randomization helpers central to this rule — `LibraryUtility.GenerateRandomCode()`, `LibraryERM.CreateSourceCode()`, `LibraryUtility.GetGlobalNoSeriesCode()` — are implemented and maintained in BCApps. BCApps test code never hardcodes record identifiers like `'CASH'`, `'10000'`, or `'70000'`; all test data is generated programmatically.
|
||||
|
||||
```al
|
||||
// CORRECT: no-series via MS library
|
||||
GlobalNoSeriesCode := LibraryUtility.GetGlobalNoSeriesCode();
|
||||
```
|
||||
|
||||
```al
|
||||
// CORRECT: hardcoded in integration test — documenting a contract
|
||||
// [SCENARIO] Inbound ORDRSP with fixed order reference from Allnet Germany
|
||||
ExpectedOrderRef := 'ORD-2026-00001'; // fixed by integration contract
|
||||
```
|
||||
- **Source:** https://github.com/microsoft/BCApps/tree/main/src/Tools/Test%20Framework
|
||||
- **Pattern:** BCApps test codeunits create every required record from scratch using library helpers that guarantee uniqueness per test run. The CURABIS rule mirrors this approach exactly.
|
||||
- **Note:** The `BCPTCreateSOWithNLines.Codeunit.al` sample in BCApps uses `Customer.get('10000')` as a fallback — this is a BCPT performance scenario (not a correctness test) and explicitly acknowledges this deviation. Correctness tests must never do this.
|
||||
|
|
|
|||
|
|
@ -1,71 +1,33 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: testing
|
||||
keywords: [test, library, setup, initialize, suppresscommit, asserterror]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
# CURABIS Test Library Standards
|
||||
|
||||
## Description
|
||||
## Core Rules
|
||||
|
||||
In CURABIS test apps, all test setup is centralized in a dedicated Test Library
|
||||
codeunit (e.g. `SV Test Library`). Individual test procedures must not call
|
||||
BC standard library codeunits (`LibrarySales`, `LibraryInventory`, etc.) directly.
|
||||
The documentation establishes three critical testing practices for CURABIS AL applications:
|
||||
|
||||
Additionally, two rules apply to every test that calls a posting codeunit:
|
||||
1. **Centralized Setup**: "all test setup is centralized in a dedicated Test Library codeunit" rather than individual test procedures calling BC standard libraries directly.
|
||||
|
||||
1. `SetSuppressCommit(true)` must be called before `Run()` to prevent data
|
||||
from leaking between tests.
|
||||
2. `asserterror` must always be followed by `Assert.ExpectedErrorCode()` or
|
||||
`Assert.ExpectedError()` — a naked `asserterror` passes on any error,
|
||||
not just the expected one.
|
||||
2. **Suppress Commits**: `SetSuppressCommit(true)` must execute before `Run()` to isolate test data and prevent cross-test contamination.
|
||||
|
||||
## Anti Pattern
|
||||
3. **Assertion After asserterror**: Every `asserterror` statement requires a subsequent `Assert.ExpectedErrorCode()` or `Assert.ExpectedError()` call to validate the specific error, preventing false passes from unexpected exceptions.
|
||||
|
||||
```al
|
||||
// WRONG: inline setup bypassing the test library
|
||||
procedure MyTest()
|
||||
var
|
||||
Item: Record Item;
|
||||
begin
|
||||
LibraryInventory.CreateItem(Item); // do not call directly
|
||||
// ...
|
||||
end;
|
||||
```
|
||||
## Key Violations
|
||||
|
||||
```al
|
||||
// WRONG: posting without SuppressCommit
|
||||
SVPost.Run(SVHeader); // commits to test database
|
||||
```
|
||||
The anti-patterns section highlights three common mistakes:
|
||||
|
||||
```al
|
||||
// WRONG: naked asserterror
|
||||
asserterror SVPost.Run(SVHeader);
|
||||
// no assertion follows — passes on any error
|
||||
```
|
||||
- Bypassing the test library by directly invoking BC standard codeunits like `LibraryInventory`
|
||||
- Executing posting operations without suppressing commits, which "commits to test database"
|
||||
- Using "naked asserterror" that "passes on any error, not just the expected one"
|
||||
|
||||
## Best Practice
|
||||
## Correct Implementation
|
||||
|
||||
```al
|
||||
// CORRECT: delegate to test library
|
||||
procedure MyTest()
|
||||
var
|
||||
Item: Record Item;
|
||||
begin
|
||||
SVLib.GivenScrapItem(Item); // test library owns setup
|
||||
// ...
|
||||
end;
|
||||
```
|
||||
The best practice section demonstrates the preferred approach: delegating setup operations to the test library (e.g., `SVLib.GivenScrapItem()`), enabling `SuppressCommit` before posting operations, and pairing error assertions with specific error code validations.
|
||||
|
||||
```al
|
||||
// CORRECT: SuppressCommit before Run
|
||||
SVPost.SetSuppressCommit(true);
|
||||
SVPost.Run(SVHeader);
|
||||
```
|
||||
These guidelines ensure test isolation, maintainability, and reliability across CURABIS test suites.
|
||||
|
||||
```al
|
||||
// CORRECT: asserterror followed by assertion
|
||||
asserterror SVPost.Run(SVHeader);
|
||||
Assert.ExpectedErrorCode('Dialog');
|
||||
```
|
||||
## BCApps Reference
|
||||
|
||||
The test library pattern originates from BCApps. The Microsoft-maintained test framework libraries (`Library - ERM`, `Library - Inventory`, `Library - Sales`, `Library - Utility`, etc.) are defined in BCApps and establish the canonical pattern for centralized, reusable test setup. CURABIS's own Test Library codeunit follows this same structural model.
|
||||
|
||||
- **Source:** https://github.com/microsoft/BCApps/tree/main/src/Tools/Test%20Framework
|
||||
- **Pattern:** Microsoft never writes inline setup logic inside individual test procedures. All setup is routed through library codeunits that can be reused, versioned, and maintained independently of the test cases themselves.
|
||||
- **Why this matters:** BCApps Test Framework is the ground truth for how BC testing is intended to work. Deviating from this pattern creates test suites that are harder to maintain and more likely to share state across tests.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue