mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 22:56:55 +01:00
Fix three merge-critical items from Jesper's 2026-09-29 review
- Barcode: drop the false claim that '*value*' is mismatched with the
IDAutomation Code 39 font; '*' is a documented start/stop form and
'(' / ')' an accepted alternative. Cue and article now route only
independently provable validation/checksum/font-binding defects.
- Dispatch good samples (and matching bad samples) now pass a
Sales Invoice Header with the S.Invoice usage, matching the record
the selected report (1306 "Standard Sales - Invoice") expects.
- custom-document-dispatch rule made disjunctive: a hardcoded report
or a hand-built email is each a bypass on its own; scoped to
customer/vendor-facing documents. Bad fixture shows the hardcoded
report alone.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Michael Dieringer
parent
4cd41f08f7
commit
aad3991d9f
7 changed files with 98 additions and 85 deletions
|
|
@ -1,28 +1,20 @@
|
|||
report 50102 "Sample Settlement Doc Bad"
|
||||
codeunit 50102 "Sample Posted Invoice Send"
|
||||
{
|
||||
UsageCategory = ReportsAndAnalysis;
|
||||
ApplicationArea = All;
|
||||
|
||||
dataset
|
||||
{
|
||||
dataitem(Customer; Customer)
|
||||
{
|
||||
column(No_Customer; "No.") { }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50102 "Sample Settlement Document Send"
|
||||
{
|
||||
procedure SendSettlementDocument(var Customer: Record Customer)
|
||||
procedure SendPostedInvoice(SalesInvoiceHeader: Record "Sales Invoice Header")
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.Get(SalesInvoiceHeader."Bill-to Customer No.");
|
||||
Customer.TestField("E-Mail");
|
||||
|
||||
// WRONG: hardcoded report, no Report Selections row backing it.
|
||||
// Works for the default case, but there is nowhere for an admin to
|
||||
// change the report or layout for one specific customer - this
|
||||
// document never shows up on "Document Layouts" at all, and the
|
||||
// only way to change it is a code change and a new release.
|
||||
Report.RunModal(Report::"Sample Settlement Doc Bad", false, false, Customer);
|
||||
// WRONG: the report is hardcoded instead of resolved through the
|
||||
// registered "S.Invoice" usage in Report Selections. This alone is
|
||||
// the defect - no hand-built email is needed for it: a Report
|
||||
// Selections row or a per-customer "Document Layouts" override
|
||||
// that points this usage at a different report or layout is
|
||||
// silently ignored, and the only way to change what this code
|
||||
// prints is a code change and a new release.
|
||||
SalesInvoiceHeader.SetRecFilter();
|
||||
Report.RunModal(Report::"Standard Sales - Invoice", false, false, SalesInvoiceHeader);
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,22 +1,31 @@
|
|||
codeunit 50102 "Sample Settlement Document Send"
|
||||
codeunit 50102 "Sample Posted Invoice Send"
|
||||
{
|
||||
procedure SendSettlementDocument(var Customer: Record Customer)
|
||||
procedure SendPostedInvoice(SalesInvoiceHeader: Record "Sales Invoice Header")
|
||||
var
|
||||
ReportSelections: Record "Report Selections";
|
||||
ReportDistributionMgt: Codeunit "Report Distribution Management";
|
||||
begin
|
||||
// Custom validation specific to this document stays here...
|
||||
CheckReadyToSend(Customer);
|
||||
// Custom validation specific to this dispatch stays here...
|
||||
CheckReadyToSend(SalesInvoiceHeader);
|
||||
|
||||
// ...but dispatch goes through the registered usage, so per-account
|
||||
// ...but dispatch goes through the registered usage. "S.Invoice"
|
||||
// resolves to a report built on "Sales Invoice Header" (by default
|
||||
// report 1306 "Standard Sales - Invoice"), so the record passed in
|
||||
// matches what the selected report expects, and per-account
|
||||
// report/layout overrides and email attachment/body configuration
|
||||
// on Report Selections all apply automatically.
|
||||
SalesInvoiceHeader.SetRecFilter();
|
||||
ReportSelections.SendEmailToCust(
|
||||
"Report Selection Usage"::"S.Invoice".AsInteger(), Customer, Customer."No.",
|
||||
Customer.Name, true, Customer."No.");
|
||||
"Report Selection Usage"::"S.Invoice".AsInteger(), SalesInvoiceHeader, SalesInvoiceHeader."No.",
|
||||
ReportDistributionMgt.GetFullDocumentTypeText(SalesInvoiceHeader), true,
|
||||
SalesInvoiceHeader."Bill-to Customer No.");
|
||||
end;
|
||||
|
||||
local procedure CheckReadyToSend(var Customer: Record Customer)
|
||||
local procedure CheckReadyToSend(SalesInvoiceHeader: Record "Sales Invoice Header")
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.Get(SalesInvoiceHeader."Bill-to Customer No.");
|
||||
Customer.TestField("E-Mail");
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,11 +11,15 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
A codeunit that hardcodes which report to run (`Report.RunModal(MyReportId, ...)`)
|
||||
and builds its own email directly, instead of registering the document
|
||||
A codeunit that hardcodes which report to run (`Report.RunModal(MyReportId, ...)`),
|
||||
or builds its own email directly, instead of registering the document
|
||||
through `table 77 "Report Selections"` and calling its own
|
||||
Print/Email procedures, works for the one case it was written for — and
|
||||
loses everything the platform's registry provides for free. `Report
|
||||
loses everything the platform's registry provides for free. Either
|
||||
bypass is a defect on its own: a hardcoded report ignores the registered
|
||||
report and any per-account layout override even when no email is
|
||||
involved, and a hand-built email ignores the registry's attachment and
|
||||
email-body configuration even when the report itself came from it. `Report
|
||||
Selections` carries its own attachment/email-body configuration per usage
|
||||
(`"Use for Email Attachment"`, `"Use for Email Body"`, `"Email Body Layout
|
||||
Code"`, `"Email Body Layout Type"`), plus a separate per-usage layout
|
||||
|
|
@ -44,9 +48,10 @@ See sample: [`custom-document-dispatch-must-not-bypass-report-selections.good.al
|
|||
|
||||
## Anti Pattern
|
||||
|
||||
A codeunit that runs a hardcoded report ID and builds its own email
|
||||
message directly, with no `Report Selections` row backing it. It works for
|
||||
the default case, but the report/layout cannot be changed per account
|
||||
A codeunit that runs a hardcoded report ID, or builds its own email
|
||||
message directly, for a document that has (or should have) a
|
||||
`Report Selections` usage — each is independently a bypass, and the
|
||||
sample shows the first on its own. It works for the default case, but the report/layout cannot be changed per account
|
||||
without a code change and a new release, and the document is invisible to
|
||||
"Document Layouts" — the standard place every other document's
|
||||
distribution is configured.
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
page 50101 "Sample Settlement Document Card"
|
||||
page 50101 "Sample Posted Invoice Card"
|
||||
{
|
||||
PageType = Card;
|
||||
SourceTable = Customer;
|
||||
SourceTable = "Sales Invoice Header";
|
||||
ApplicationArea = All;
|
||||
Editable = false;
|
||||
|
||||
actions
|
||||
{
|
||||
|
|
@ -16,7 +17,9 @@ page 50101 "Sample Settlement Document Card"
|
|||
|
||||
trigger OnAction()
|
||||
var
|
||||
SalesInvoiceHeader: Record "Sales Invoice Header";
|
||||
DocumentSendingProfile: Record "Document Sending Profile";
|
||||
ReportDistributionMgt: Codeunit "Report Distribution Management";
|
||||
begin
|
||||
// WRONG: this is a plain, on-demand "Email" button, not
|
||||
// part of a combined Post-and-Send action - but this
|
||||
|
|
@ -28,10 +31,13 @@ page 50101 "Sample Settlement Document Card"
|
|||
// Printer = Yes, "E-Mail" = No) turns this button into a
|
||||
// silent no-op, with no indication an unrelated setup
|
||||
// field is why.
|
||||
DocumentSendingProfile.GetDefaultForCustomer(Rec."No.", DocumentSendingProfile);
|
||||
SalesInvoiceHeader := Rec;
|
||||
CurrPage.SetSelectionFilter(SalesInvoiceHeader);
|
||||
DocumentSendingProfile.GetDefaultForCustomer(Rec."Bill-to Customer No.", DocumentSendingProfile);
|
||||
DocumentSendingProfile.Send(
|
||||
"Report Selection Usage"::"S.Invoice".AsInteger(), Rec, Rec."No.", Rec."No.",
|
||||
Rec.Name, Rec.FieldNo("No."), Rec.FieldNo("No."));
|
||||
"Report Selection Usage"::"S.Invoice".AsInteger(), SalesInvoiceHeader, Rec."No.",
|
||||
Rec."Bill-to Customer No.", ReportDistributionMgt.GetFullDocumentTypeText(Rec),
|
||||
SalesInvoiceHeader.FieldNo("Bill-to Customer No."), SalesInvoiceHeader.FieldNo("No."));
|
||||
end;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
page 50101 "Sample Settlement Document Card"
|
||||
page 50101 "Sample Posted Invoice Card"
|
||||
{
|
||||
PageType = Card;
|
||||
SourceTable = Customer;
|
||||
SourceTable = "Sales Invoice Header";
|
||||
ApplicationArea = All;
|
||||
Editable = false;
|
||||
|
||||
actions
|
||||
{
|
||||
|
|
@ -16,7 +17,9 @@ page 50101 "Sample Settlement Document Card"
|
|||
|
||||
trigger OnAction()
|
||||
var
|
||||
SalesInvoiceHeader: Record "Sales Invoice Header";
|
||||
ReportSelections: Record "Report Selections";
|
||||
ReportDistributionMgt: Codeunit "Report Distribution Management";
|
||||
begin
|
||||
// Calls Report Selections directly - the button's outcome
|
||||
// depends only on this customer's registered report/layout,
|
||||
|
|
@ -24,9 +27,13 @@ page 50101 "Sample Settlement Document Card"
|
|||
// DocumentSendingProfile.TrySendToEMail(...) instead would
|
||||
// be equally correct: it never Get's the customer's
|
||||
// actually assigned profile, only a local, hardcoded one.
|
||||
// "S.Invoice" resolves to a report on "Sales Invoice
|
||||
// Header", which is the record passed here.
|
||||
SalesInvoiceHeader := Rec;
|
||||
CurrPage.SetSelectionFilter(SalesInvoiceHeader);
|
||||
ReportSelections.SendEmailToCust(
|
||||
"Report Selection Usage"::"S.Invoice".AsInteger(), Rec, Rec."No.",
|
||||
Rec.Name, true, Rec."No.");
|
||||
"Report Selection Usage"::"S.Invoice".AsInteger(), SalesInvoiceHeader, Rec."No.",
|
||||
ReportDistributionMgt.GetFullDocumentTypeText(Rec), true, Rec."Bill-to Customer No.");
|
||||
end;
|
||||
}
|
||||
action(PrintDocument)
|
||||
|
|
@ -37,10 +44,14 @@ page 50101 "Sample Settlement Document Card"
|
|||
|
||||
trigger OnAction()
|
||||
var
|
||||
SalesInvoiceHeader: Record "Sales Invoice Header";
|
||||
ReportSelections: Record "Report Selections";
|
||||
begin
|
||||
SalesInvoiceHeader := Rec;
|
||||
CurrPage.SetSelectionFilter(SalesInvoiceHeader);
|
||||
ReportSelections.PrintWithDialogForCust(
|
||||
"Report Selection Usage"::"S.Invoice", Rec, true, Rec.FieldNo("No."));
|
||||
"Report Selection Usage"::"S.Invoice", SalesInvoiceHeader, true,
|
||||
SalesInvoiceHeader.FieldNo("Bill-to Customer No."));
|
||||
end;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -56,34 +56,26 @@ See sample: [`report-barcodes-must-use-barcode-module-and-production-font-name.g
|
|||
|
||||
## Anti Pattern
|
||||
|
||||
Constructing a barcode string by hand instead of using the module's
|
||||
provider/encoder API — not because a manual delimiter is inherently
|
||||
wrong (Code 39's own symbology does use `*` as start/stop; Microsoft
|
||||
Learn's font table says so), but because hand-rolled construction is
|
||||
demonstrably mismatched with what the real encoder produces: it skips
|
||||
`ValidateInput` (so a value outside the character set, or needing a
|
||||
checksum/extended-charset setting never applied, reaches the font
|
||||
unvalidated), and IDAutomation 1D Provider's own Code 39 output is
|
||||
wrapped in `(`/`)`, not literal `*` (BCApps test:
|
||||
`EncodeFont('1234', Code39) = '(1234)'`) — the paired font maps those
|
||||
parentheses to the real start/stop glyph, so `'*' + value + '*'` is
|
||||
simply the wrong characters, plus no checksum.
|
||||
Constructing a barcode string by hand where that construction has a
|
||||
concrete, independently provable defect: a source value that can contain
|
||||
characters outside the symbology's character set is never validated, a
|
||||
checksum the symbology or setup requires is never applied, or there is
|
||||
concrete evidence of an incompatible font binding.
|
||||
|
||||
Flag demonstrably invalid or mismatched hand construction, not manual
|
||||
delimiter use as a category — a custom provider paired with a font that
|
||||
genuinely expects literal `*` delimiters is a different, legitimate case.
|
||||
The delimiter itself is not the defect. `*value*` is a documented, valid
|
||||
Code 39 form for IDAutomation fonts (Microsoft Learn's font table and
|
||||
IDAutomation's own manual both give `*` as start/stop); the `(`/`)` that
|
||||
IDAutomation 1D Provider's encoder emits (BCApps test:
|
||||
`EncodeFont('1234', Code39) = '(1234)'`) is an alternative start/stop
|
||||
form the same fonts accept, used to keep `*` out of the human-readable
|
||||
text. Never flag delimiter choice alone.
|
||||
|
||||
A second version of the same mistake: encoding correctly, but naming the
|
||||
evaluation font instead of the purchased one. Both look complete in
|
||||
review and fail silently — the first because the data was never a real
|
||||
barcode, the second because BC online refuses to render it.
|
||||
|
||||
The same gap exists even when the module *is* used: a 1D path that calls
|
||||
`EncodeFont` on `"Barcode Font Provider"` without `ValidateInput`.
|
||||
IDAutomation 1D Provider's `EncodeFont` does not validate on its own, so
|
||||
a value outside the symbology's character set is never rejected — it
|
||||
reaches the font as an unscannable barcode. The sample shows this
|
||||
variant, because it is visible in AL alone without layout evidence.
|
||||
The same validation gap exists when the module *is* used: a 1D path that
|
||||
calls `EncodeFont` on `"Barcode Font Provider"` without `ValidateInput`
|
||||
(IDAutomation 1D Provider's `EncodeFont` does not validate on its own).
|
||||
The sample shows this variant, visible in AL alone. A last version:
|
||||
encoding correctly but naming the evaluation font, which BC online
|
||||
refuses to render.
|
||||
|
||||
See sample: [`report-barcodes-must-use-barcode-module-and-production-font-name.bad.al`](report-barcodes-must-use-barcode-module-and-production-font-name.bad.al).
|
||||
|
||||
|
|
@ -93,17 +85,15 @@ BCApps (`src/System Application/App/Barcode/src/`):
|
|||
`Barcode Provider/Font/BarcodeFontProvider.Interface.al` (1D:
|
||||
`ValidateInput` + `EncodeFont`); `IDAutomation 1D Provider/
|
||||
IDAutomation1DProvider.Codeunit.al` (`EncodeFont` goes straight to the
|
||||
symbology encoder; only `ValidateInput` calls `IsValidInput`); `Barcode Provider 2D/Font/
|
||||
BarcodeFontProvider2D.Interface.al` (2D: only `EncodeFont`); both read
|
||||
fresh from source. `IDAutomation 1D Provider/Encoders/
|
||||
IDA1DCode39Encoder.Codeunit.al` (`codeunit 9204`, regex accepts literal
|
||||
`*` as plain input; `EncodeFont` → `DotNet FontEncoder.Code39`). Split
|
||||
and delimiter mismatch both confirmed live: `.../Inventory/Item/
|
||||
ItemGTINLabel.Report.al` (`report 6625`, validates+encodes 1D, only
|
||||
encodes 2D, same value) and `.../Test/Barcode/.../IDA1DCode39Test.
|
||||
Codeunit.al` (`codeunit 135044`): `EncodeFontSuccessTest('1234', Code39,
|
||||
'(1234)')` — wrapped in `(`/`)`, never literal `*`.
|
||||
symbology encoder; only `ValidateInput` calls `IsValidInput`); `Barcode Provider 2D/Font/BarcodeFontProvider2D.Interface.al`
|
||||
(2D: only `EncodeFont`). `IDAutomation 1D Provider/Encoders/IDA1DCode39Encoder.Codeunit.al`
|
||||
(`codeunit 9204`, regex accepts literal `*`; `EncodeFont` → `DotNet FontEncoder.Code39`).
|
||||
1D/2D split: `.../Inventory/Item/ItemGTINLabel.Report.al` (`report 6625`,
|
||||
validates+encodes 1D, only encodes 2D). Encoder output form: `IDA1DCode39Test.Codeunit.al`
|
||||
(`codeunit 135044`): `EncodeFontSuccessTest('1234', Code39, '(1234)')`.
|
||||
|
||||
Microsoft Learn "Adding Barcodes to Reports" and "Barcode Fonts with
|
||||
Business Central Online" — quoted above, incl. the Code39 row ("`*` is
|
||||
used for both start and stop delimiters").
|
||||
used for both start and stop delimiters"). IDAutomation, "Code 39 Font
|
||||
User Manual" (https://idautomation.com/barcode-fonts/code-39/fontnames/):
|
||||
`*` start/stop, or parentheses to keep `*` out of the human-readable text.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue