Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
codeunit 50130 "Sample Item Ledger Lookup"
{
procedure GetPostedItemLedgerEntries(var SalesHeader: Record "Sales Header"; var ItemLedgerEntry: Record "Item Ledger Entry")
var
LibrarySales: Codeunit "Library - Sales";
InvoiceNo: Code[20];
begin
InvoiceNo := LibrarySales.PostSalesDocument(SalesHeader, true, true);
ItemLedgerEntry.SetRange("Document No.", InvoiceNo);
ItemLedgerEntry.FindSet();
end;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
codeunit 50130 "Sample Item Ledger Lookup"
{
procedure GetPostedItemLedgerEntries(var SalesHeader: Record "Sales Header"; var ItemLedgerEntry: Record "Item Ledger Entry")
var
LibrarySales: Codeunit "Library - Sales";
ShippingNo: Code[20];
begin
LibrarySales.PostSalesDocument(SalesHeader, true, true);
ShippingNo := SalesHeader."Last Shipping No.";
ItemLedgerEntry.SetRange("Document No.", ShippingNo);
ItemLedgerEntry.FindSet();
end;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
bc-version: [all]
domain: data-modeling
keywords: [item-ledger-entry, document-no, last-shipping-no, ship-and-invoice, posting, sales-order]
technologies: [al]
countries: [w1]
application-area: [all]
---

# After Ship-and-Invoice posting, Item Ledger Entry carries the shipment document number

## Description

Posting a sales order with both Ship and Invoice in one call creates the Item Ledger Entry during the shipment leg of that combined post, so the entry's `Document No.` is stamped with the value assigned to the shipment — `Sales Header."Last Shipping No."` — not the posted sales invoice number the posting call returns. Code that filters Item Ledger Entry by the invoice number instead finds nothing: `SetRange`/`FindSet` simply return zero rows, with no error to signal the mistake.

## Best Practice

After posting a sales order with Ship and Invoice together, read `SalesHeader."Last Shipping No."` (populated during the post) and filter Item Ledger Entry by that value, not by the invoice number the posting routine returns.

See sample: `item-ledger-entry-document-no-follows-last-shipping-no.good.al`.

## Anti Pattern

Filtering Item Ledger Entry by the posted sales invoice number after a combined Ship-and-Invoice post. The filter compiles and runs without error but matches zero rows, because the entry belongs to the shipment leg of the posting, not the invoice leg.

See sample: `item-ledger-entry-document-no-follows-last-shipping-no.bad.al`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
codeunit 50132 "Sample Customer Type Library"
{
procedure CreateCustomerType(var CustomerType: Record "Customer Type")
begin
CustomerType.Init();
CustomerType.Code := 'TEST001';
CustomerType.Description := 'Test Customer Type';
CustomerType.Insert(true);
end;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
codeunit 50132 "Sample Customer Type Library"
{
var
LibraryUtility: Codeunit "Library - Utility";

procedure CreateCustomerType(var CustomerType: Record "Customer Type")
begin
CustomerType.Init();
// Code is shorter than GenerateGUID()'s 10 characters, and this field's
// uniqueness matters, so use GenerateRandomCodeWithLength: it opens the
// real (non-temporary) table and loops until the value doesn't collide.
// GenerateRandomCode would not do this — it opens the table as temporary,
// so its own emptiness check never inspects real rows.
CustomerType.Code :=
LibraryUtility.GenerateRandomCodeWithLength(CustomerType.FieldNo(Code), Database::"Customer Type", MaxStrLen(CustomerType.Code));
// Description is long enough to hold the full GenerateGUID() value
// untruncated, and only needs to be incidental, not verified-unique.
CustomerType.Description := CopyStr(LibraryUtility.GenerateGUID(), 1, MaxStrLen(CustomerType.Description));
CustomerType.Insert(true);
end;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
bc-version: [all]
domain: testing
keywords: [generateguid, library-utility, test-fixtures, uniqueness, generaterandomcode, maxstrlen]
technologies: [al]
countries: [w1]
application-area: [all]
---

# Generate unique test fixture values with LibraryUtility helpers, not hardcoded literals

## Description

A fixture helper that assigns a hardcoded literal to a primary-key or descriptive field collides the moment two tests, or two runs of the same test, create that fixture without cleanup, and a literal longer than the field allows raises a truncation or insert error. `LibraryUtility.GenerateGUID()` is not a real GUID — it is a `Code[10]` number-series value (`GU00000000`–`GU99999999`) — and it returns the full 10 characters unshortened. Truncating it yourself with `CopyStr(..., 1, MaxStrLen(ShorterField))` for a field under 10 characters is unsafe: the changing digits sit at the right end and are exactly what gets cut off, so consecutive calls into a short field can produce the same truncated value. `GenerateGUID()` is only safe as-is for a field that holds the full 10 characters.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please restrict the collision rule to primary/unique-key fields or values the test explicitly relies on as unique lookup identifiers. An ordinary descriptive field is not unique, so two rows with the same description do not collide on insert; deterministic descriptive text is also often required for exact assertions. The anti-pattern at line 31 and the testing review cue should be narrowed consistently so agents do not flag valid descriptive test data.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked at 1d05811b3da1eaa4acefea533d2764bf62951d7b: this is still unresolved. Please narrow the article description/anti-pattern and the testing worklist cue to primary/unique-key fields or values explicitly used as unique lookup identifiers. Deterministic descriptive values remain valid and should not be flagged.


## Best Practice

For a field that holds the full 10 characters, assign `LibraryUtility.GenerateGUID()` directly. For a shorter field, do not truncate a GUID yourself — but also do not assume every `LibraryUtility` helper verifies uniqueness against the real table, because they don't all behave the same way:

- `GenerateRandomCode(FieldNo, TableNo)` opens the target table as a **temporary** `RecordRef`, so its own emptiness check never inspects real rows — despite taking `TableNo`, it does not verify against the actual table. It's safe to use for its non-colliding-*within-a-single-test-run* value (derived from `GenerateGUID()`'s own number series), not for a guarantee against pre-existing or leftover data.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GenerateRandomCode cannot guarantee non-collision even within one test run for short fields. It takes the rightmost FieldRef.Length characters of the sequential GenerateGUID() value but does not remember earlier temporary results; a Code[1] value repeats every 10 calls, Code[2] every 100, and so on. Please remove the within-run guarantee and describe it consistently with line 25 as a finite short-field namespace with only a low collision chance. Keep the real-table helpers for cases that require verified uniqueness.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked against current LibraryUtility.Codeunit.al at BCApps 42d69968a63e2fd951bba15e3dc51b50df932ffe: this guarantee is still false. GenerateRandomCode keeps no history after opening a temporary RecordRef; its rightmost n digits repeat every 10^n calls. Please describe it only as a finite short-field namespace with collision risk, and reserve verified uniqueness for the real-table helpers.

- `GenerateRandomCodeWithLength(FieldNo, TableNo, CodeLength)` opens the real (non-temporary) table and loops until the generated value doesn't collide — a genuine verified-unique guarantee — but it returns `Code[10]` regardless of the requested `CodeLength`, so it's only useful for a field of 10 characters or fewer.
- `GenerateRandomCode20(FieldNo, TableNo)` is the same real, verified-against-the-table pattern as `GenerateRandomCodeWithLength`, sized for a `Code[20]` field.
- `GenerateRandomXMLText(Length)` performs no table lookup at all — it's a plain random-text generator, appropriate for a descriptive/incidental field where uniqueness doesn't matter, not for a value that needs to be collision-checked.

Pick `GenerateRandomCodeWithLength`/`GenerateRandomCode20` when the test genuinely needs a code verified unique against the table; use `GenerateRandomCode`/`GenerateGUID`/`GenerateRandomXMLText` for incidental values where a low collision *chance* is enough.

See sample: `use-generateguid-for-unique-test-fixture-values.good.al`.

## Anti Pattern

Hardcoding a fixture value such as `'TEST001'` or a short descriptive literal, which collides across parallel or repeated test runs. Equally an anti-pattern: truncating `GenerateGUID()`'s result with `CopyStr(..., 1, MaxStrLen(Field))` for a field shorter than 10 characters — the truncation removes the part of the value that actually varies.

See sample: `use-generateguid-for-unique-test-fixture-values.bad.al`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
codeunit 50134 "Sample Customer Type Edit Test"
{
Subtype = Test;

[Test]
procedure CustomerTypeFieldNotEditable_WhenLocked()
var
Assert: Codeunit Assert;
CustomerType: Record "Customer Type";
CustomerTypeCard: TestPage "Customer Type Card";
begin
// [GIVEN] a customer type record whose Locked flag is set
CustomerType.Init();
CustomerType.Locked := true;
CustomerType.Insert(true);

// [WHEN] the page is opened in VIEW mode — editability logic that only
// applies in edit mode is not exercised the same way
CustomerTypeCard.OpenView();
CustomerTypeCard.GoToRecord(CustomerType);

// [THEN] wrong function: Enabled() does not verify editability
Assert.IsFalse(CustomerTypeCard.Description.Enabled(), 'Description should not be editable while Locked is set.');

CustomerTypeCard.Close();
end;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
codeunit 50133 "Sample Customer Type Edit Test"
{
Subtype = Test;

[Test]
procedure CustomerTypeFieldNotEditable_WhenLocked()
var
Assert: Codeunit Assert;
CustomerType: Record "Customer Type";
CustomerTypeCard: TestPage "Customer Type Card";
begin
// [GIVEN] a customer type record whose Locked flag is set
CustomerType.Init();
CustomerType.Locked := true;
CustomerType.Insert(true);

// [WHEN] the page is opened in edit mode on that record
CustomerTypeCard.OpenEdit();
CustomerTypeCard.GoToRecord(CustomerType);

// [THEN] the field's actual editable state reflects the lock
Assert.IsFalse(CustomerTypeCard.Description.Editable(), 'Description should not be editable while Locked is set.');

CustomerTypeCard.Close();
end;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
bc-version: [all]
domain: testing
keywords: [testpage, editable, openedit, ui-state, field-verification]
technologies: [al]
countries: [w1]
application-area: [all]
---

# Verify field editability with TestPage.Editable(), opened in edit mode

## Description

Whether a field can actually be changed is a distinct state from whether it is shown or enabled — `Editable()` and `Enabled()` are separate `TestField` functions. Verifying editability also requires opening the `TestPage` with `OpenEdit()`, not `OpenView()`: `OpenView()` opens the page in view mode, so it does not exercise the field's own conditional editability logic the way an actual edit-mode session does.

## Best Practice

Open the `TestPage` with `OpenEdit()`, navigate to the relevant record, then assert against `TestPageField.Editable()` to verify whether the field can be changed under the given precondition.

See sample: `use-testpage-editable-to-verify-field-editability.good.al`.

## Anti Pattern

Asserting `Enabled()` (or checking nothing at all) when the actual claim is about editability, or opening the page with `OpenView()` when the field's editability depends on business logic that only applies in edit mode.

See sample: `use-testpage-editable-to-verify-field-editability.bad.al`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
codeunit 50131 "Sample Customer Type UI Test"
{
Subtype = Test;

[Test]
procedure CustomerTypeFieldIsEnabledOnCustomerCard()
var
CustomerCard: TestPage "Customer Card";
begin
// Confirms only that the page opens - never checks the field's actual UI state
CustomerCard.OpenView();
CustomerCard.Close();
end;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
codeunit 50131 "Sample Customer Type UI Test"
{
Subtype = Test;

[Test]
procedure CustomerTypeFieldIsEnabledOnCustomerCard()
var
Assert: Codeunit Assert;
CustomerCard: TestPage "Customer Card";
begin
CustomerCard.OpenView();
Assert.IsTrue(CustomerCard."Customer Type".Enabled(), 'Customer Type should be enabled on the Customer Card.');
Assert.IsTrue(CustomerCard."Customer Type".Visible(), 'Customer Type should be visible on the Customer Card.');
end;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
bc-version: [all]
domain: testing
keywords: [testpage, visible, enabled, ui-state, headless-test, field-verification]
technologies: [al]
countries: [w1]
application-area: [all]
---

# Verify field visibility and enabled state with TestPage.Visible()/.Enabled()

## Description

A UI test codeunit does not need to inspect table or page properties indirectly to confirm a field is shown or enabled under given conditions. The `TestPage` object exposes a `Visible()` and an `Enabled()` function on each field, reflecting the page's actual rendered state, callable directly from a `[Test]` procedure. `Enabled()` and `Editable()` are distinct states — this article covers visibility/enabled state specifically; see `use-testpage-editable-to-verify-field-editability.md` for verifying whether a field can actually be changed.

## Best Practice

Open the `TestPage`, navigate to the relevant record if needed, then assert against `TestPageField.Visible()` and `TestPageField.Enabled()` to verify the field's shown/enabled state, rather than checking an unrelated table/page property or skipping the check.

See sample: `use-testpage-visible-enabled-to-verify-field-ui-state.good.al`.

## Anti Pattern

A test that opens the `TestPage` but never asserts against `Visible()`/`Enabled()` on the field in question — confirming only that the page opens, not that the field behaves as expected.

See sample: `use-testpage-visible-enabled-to-verify-field-ui-state.bad.al`.
1 change: 1 addition & 0 deletions microsoft/skills/review/al-data-modeling-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ A file enters the candidate worklist when its `keywords` intersect the extracted
The following targeted checks cover every current `data-modeling` article. Treat each as a candidate-selection cue: when the signal appears in changed code, add the named article to the worklist and evaluate it in Action.

- A `* Setup` table or its page changes singleton structure, uses a nonblank or generated key, permits insert/delete, uses a List page, or does not ensure the blank-keyed row exists — `setup-table-is-a-singleton`.
- Code reads `Item Ledger Entry."Document No."` (or `"Last Shipping No."`/`"Last Posting No."`) after a combined Ship+Invoice **sales** post — `item-ledger-entry-document-no-follows-last-shipping-no`. This is a sales-specific rule: purchase combined posting is Receive+Invoice and uses receiving fields such as `"Last Receiving No."`, not the shipment/document-number behavior this article describes. Do not worklist it from purchase posting code.
- A custom master table changes its primary key, `No.`/`No. Series` fields, or `OnInsert` without assigning a blank `No.` from setup through a number series — `master-table-no-from-number-series-in-oninsert`.
- BC v22 or later code introduces or retains `NoSeriesManagement`, `InitSeries`, `SelectSeries`, or `SetSeries`, or number assignment/manual-entry checks do not use codeunit `"No. Series"` methods such as `GetNextNo`, `IsManual`, or `TestManual` — `use-no-series-codeunit-not-noseriesmanagement`.
- A master gains or changes `Blocked`, or a document line, journal line, reference-field `OnValidate`, or posting routine uses that master without `TestField(Blocked, false)` at the point of use; also cue when the check is placed only in the master's own triggers — `check-blocked-in-referencing-code-not-in-master`.
Expand Down
Loading