mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Tighten mandatory-field review guidance
Require explicit ShowMandatory in the good sample, acknowledge that NotBlank marking is unreliable, and limit findings to visible editable controls on paths where users must supply a value. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 89dba8c8-6529-4b60-956f-875a59be499d
This commit is contained in:
parent
189f0df11b
commit
f2cb7cd90b
2 changed files with 4 additions and 3 deletions
|
|
@ -45,6 +45,7 @@ page 50543 "Sample Shipping Agents"
|
|||
{
|
||||
ApplicationArea = All;
|
||||
ToolTip = 'Specifies the code of the shipping agent.';
|
||||
ShowMandatory = true;
|
||||
}
|
||||
field(Description; Rec.Description)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -13,15 +13,15 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
`ShowMandatory` draws the red asterisk on a page field and, per the platform documentation, enforces no validation. The reverse holds too: code that enforces a value — `TestField` in `OnInsert`/`OnModify`, a `NotBlank` table field, a mandatory setup value — changes nothing about how the field renders. Because the two halves are independent, it is easy to ship a field that the code requires but the UI presents as optional. `NotBlank` does not close the gap: the documentation confines any marking it may contribute to primary-key fields, states that on any other field a value which was never entered is not validated at all, and notes that `ShowMandatory` overrides whatever marking `NotBlank` would contribute — so `ShowMandatory` is the only property to rely on for the asterisk. The gap is widest on a list page with `DelayedInsert = true`, where the enforcing error surfaces only when the user leaves the row — after the rest of the line is typed, with nothing having indicated which field was missing.
|
||||
`ShowMandatory` draws the red asterisk on a page field and, per the platform documentation, enforces no validation. The reverse is not reliable: code that enforces a value — `TestField` in `OnInsert`/`OnModify`, a `NotBlank` table field, a mandatory setup value — does not guarantee that the page field renders as mandatory. Because the two halves are independent, it is easy to ship a field that the code requires but the UI presents as optional. Microsoft documents that `NotBlank` can mark primary-key fields, but current client behavior does not do so consistently; on non-primary-key fields, a value that was never entered is not validated at all. `ShowMandatory` also overrides any marking `NotBlank` would contribute, so set it explicitly when the page must communicate a requirement. The gap is widest on a list page with `DelayedInsert = true`, where the enforcing error surfaces only when the user leaves the row — after the rest of the line is typed, with nothing having indicated which field was missing.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Set `ShowMandatory = true` on every page field whose value the code requires before the record can be committed, and leave the enforcement in place: the property is presentation, `TestField`/`Error` is the guarantee, and the two belong together in the same change. When the requirement is conditional, bind `ShowMandatory` to a Boolean variable or field that mirrors the condition the enforcement checks — the base application drives `Vendor Invoice No.` on the Purchase Invoice page from an `Ext. Doc. No. Mandatory` setup flag this way. Two expression limits are worth knowing: the property cannot call an AL method, so compute the value into a variable first, and a numeric field that has a default value counts as filled, so it never shows the asterisk. See sample: `showmandatory-on-code-required-page-fields.good.al`.
|
||||
Set `ShowMandatory = true` on every visible, editable page field whose value the user must supply before the record can be committed or an action can complete, and leave the enforcement in place: the property is presentation, `TestField`/`Error` is the guarantee, and the two belong together in the same change. When the requirement is conditional, bind `ShowMandatory` to a Boolean variable or field that mirrors the condition the enforcement checks — the base application drives `Vendor Invoice No.` on the Purchase Invoice page from an `Ext. Doc. No. Mandatory` setup flag this way. Two expression limits are worth knowing: the property cannot call an AL method, so compute the value into a variable first, and a numeric field that has a default value counts as filled, so it never shows the asterisk. See sample: `showmandatory-on-code-required-page-fields.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A required field with no mandatory marker: the table's `OnInsert` or the page's `OnInsertRecord` calls `TestField` on a field, or `NotBlank` on a non-primary-key field is expected to force entry, while the page field bound to it carries no `ShowMandatory`. On a `DelayedInsert = true` list page the user fills the row, leaves it, and gets an error naming a field that never looked different from the optional ones. Reviewer signal: a `TestField` or `Error` naming a field in `OnInsert`, `OnInsertRecord`, `OnModify`, or `OnValidate`, or `NotBlank = true` on a table field, with no `ShowMandatory` on the corresponding page field — exactly the review finding on the `Job Assigned Resources` page linked below. Setting `ShowMandatory = false` on a required field is the same defect stated explicitly, and per the documentation it also overrides any marking `NotBlank` would otherwise contribute. See sample: `showmandatory-on-code-required-page-fields.bad.al`.
|
||||
A required field with no mandatory marker: the table's `OnInsert` or the page's `OnInsertRecord` calls `TestField` on a field, or `NotBlank` is expected to force entry, while the page field bound to it carries no `ShowMandatory`. On a `DelayedInsert = true` list page the user fills the row, leaves it, and gets an error naming a field that never looked different from the optional ones. Reviewer signal: code on the relevant commit or action path requires the user to supply a field, the corresponding page control is visible and editable, and its `ShowMandatory` property is missing or does not mirror the same condition. A `TestField` or `Error` elsewhere in `OnValidate` or `OnModify` is not sufficient evidence: the field may be populated by code, non-editable, or required only for another path. Setting `ShowMandatory = false` on a field that is unconditionally required on the current path is the same defect stated explicitly, and per the documentation it also overrides any marking `NotBlank` would otherwise contribute. See sample: `showmandatory-on-code-required-page-fields.bad.al`.
|
||||
|
||||
## See also
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue