Address Jesper Schulz-Wedde's review on PR #158

- use-generateguid-for-unique-test-fixture-values.md: GenerateGUID()
  is a Code[10] number-series value, not a real GUID; truncating it
  with CopyStr for a shorter field cuts off the changing digits. Point
  to GenerateRandomCode/GenerateRandomCodeWithLength/GenerateRandomXMLText
  instead, which verify uniqueness against the actual table.
- Split use-testpage-visible-enabled-to-verify-field-ui-state.md: drop
  its editability claim (the sample opens with OpenView() and asserts
  Enabled(), which verifies enabled state, not editability — Editable()
  and Enabled() are distinct TestField methods). New companion article
  use-testpage-editable-to-verify-field-editability.md covers Editable()
  with OpenEdit() specifically.
- Wire GenerateGUID/CopyStr and TestPage Visible/Enabled/Editable cues
  into al-testing-review.md, and the Item Ledger Entry/Last Shipping No.
  posting cue into al-data-modeling-review.md.

The Item Ledger Entry article itself was independently verified against
current BCApps source and needs no changes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.good.al

@@ -6,7 +6,13 @@ codeunit 50132 "Sample Customer Type Library"
     procedure CreateCustomerType(var CustomerType: Record "Customer Type")
     begin
         CustomerType.Init();
-        CustomerType.Code := CopyStr(LibraryUtility.GenerateGUID(), 1, MaxStrLen(CustomerType.Code));
+        // Code is shorter than GenerateGUID()'s 10 characters, so use
+        // GenerateRandomCode instead of truncating a GUID ourselves — it
+        // verifies uniqueness against the table rather than just returning
+        // a truncated slice of the number series.
+        CustomerType.Code := LibraryUtility.GenerateRandomCode(CustomerType.FieldNo(Code), Database::"Customer Type");
+        // Description is long enough to hold the full GenerateGUID() value
+        // untruncated, so no uniqueness verification is needed here.
         CustomerType.Description := CopyStr(LibraryUtility.GenerateGUID(), 1, MaxStrLen(CustomerType.Description));
         CustomerType.Insert(true);
     end;

microsoft/knowledge/testing/use-generateguid-for-unique-test-fixture-values.md

@@ -1,26 +1,26 @@
 ---
 bc-version: [all]
 domain: testing
-keywords: [generateguid, library-utility, test-fixtures, uniqueness, copystr, maxstrlen]
+keywords: [generateguid, library-utility, test-fixtures, uniqueness, generaterandomcode, maxstrlen]
 technologies: [al]
 countries: [w1]
 application-area: [all]
 ---
 
-# Generate unique test fixture values with LibraryUtility.GenerateGUID()
+# 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()` returns a value that is unique per call and long enough to guarantee no collision; paired with `CopyStr(..., 1, MaxStrLen(Field))` it fits any fixed-length `Code` or `Text` field safely.
+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.
 
 ## Best Practice
 
-For a fixture field that must be unique across test runs, assign `CopyStr(LibraryUtility.GenerateGUID(), 1, MaxStrLen(TargetField))` rather than a literal string.
+For a field that holds the full 10 characters, assign `LibraryUtility.GenerateGUID()` directly. For a shorter or arbitrary-length field, use `LibraryUtility.GenerateRandomCode(FieldNo, TableNo)` (or `GenerateRandomCodeWithLength`/`GenerateRandomXMLText(Length)` for a specific length) instead of truncating a GUID yourself — these generate the value and verify it is actually unique against the target table, rather than relying on the number series alone.
 
 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. It collides across parallel or repeated test runs, and a value longer than the field's length limit is either silently truncated or raises an insert error.
+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`.

microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.bad.al

@@ -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;
+}

microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.good.al

@@ -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;
+}

microsoft/knowledge/testing/use-testpage-editable-to-verify-field-editability.md

@@ -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`.

microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.good.al

@@ -9,7 +9,7 @@ codeunit 50131 "Sample Customer Type UI Test"
         CustomerCard: TestPage "Customer Card";
     begin
         CustomerCard.OpenView();
-        Assert.IsTrue(CustomerCard."Customer Type".Enabled(), 'Customer Type should be editable on the Customer Card.');
+        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;
 }

microsoft/knowledge/testing/use-testpage-visible-enabled-to-verify-field-ui-state.md

@@ -7,15 +7,15 @@ countries: [w1]
 application-area: [all]
 ---
 
-# Verify field visibility and editability with TestPage.Visible()/.Enabled()
+# 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 editable 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 after `OpenView()`.
+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 UI state, rather than checking an unrelated table/page property or skipping the check.
+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`.
 

microsoft/skills/review/al-data-modeling-review.md

@@ -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/purchase post — `item-ledger-entry-document-no-follows-last-shipping-no`.
 - 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`.

microsoft/skills/review/al-testing-review.md

@@ -50,6 +50,8 @@ The following targeted checks cover every current `testing` article. Treat each
 - A permission-sensitive test uses `TestPermissions = Disabled`, claims to test a restricted user without `"Permissions Mock"`/`"Library - Lower Permissions"`, or declares `[TestPermissions(...)]` without applying that context — `permission-tests-must-lower-the-execution-context`.
 - Test fixture code manually calls `Init`/`Insert`, invents keys or prerequisite records, or bypasses available `LibrarySales`, `LibraryPurchase`, `LibraryERM`, `LibraryInventory`, `LibraryRandom`, or equivalent library codeunits — `use-library-codeunits-for-test-fixtures`.
 - `asserterror` is added or changed without a following `Assert.ExpectedError`, `Assert.ExpectedErrorCode`, or a purpose-built assertion such as `ExpectedTestFieldError` — `asserterror-needs-expectederror-and-code`.
+- Fixture code calls `LibraryUtility.GenerateGUID()` or `CopyStr` against it — `use-generateguid-for-unique-test-fixture-values`.
+- A test asserts against a `TestPage` field's `.Visible()` or `.Enabled()` — `use-testpage-visible-enabled-to-verify-field-ui-state`. When the assertion is against `.Editable()`, or the page is opened with `OpenEdit()` specifically to check editability — `use-testpage-editable-to-verify-field-editability`.
 - A test path raises UI and `[HandlerFunctions(...)]` does not match the invoked handlers, or the test has no meaningful evidence of the UI result (for example, it treats a Boolean set before the action as proof of success) — `ui-handlers-in-tests`. A capture/reset/assert-after-`RunModal` pattern is valid. Enqueue/dequeue and `AssertEmpty` are required only when order, count, text, replies, or a scripted sequence is part of the contract. Only nonoptional handlers have to execute: a listed handler declared `[SendNotificationHandler(true)]` or `[RecallNotificationHandler(true)]` is optional by design, so do not treat it as unmatched when the run never raises the notification.
 
 Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.