9 AL/BC patterns: document distribution, price calculation & barcode extensibility - #175
Conversation
…ent Sending Profile, Find Entries, TransferFields) Five rules about Business Central's document distribution architecture, verified against BCApps source and Microsoft Learn. - custom-document-dispatch-must-not-bypass-report-selections - document-print-and-email-actions-call-report-selections-directly - extend-find-entries-navigate-for-new-document-types - extend-report-selection-usage-for-new-document-types - transferfields-mirrored-fields-must-match-type-and-length Wired into al-data-modeling-review.md's worklist cues. Added a disambiguation note on the TransferFields article distinguishing it from the existing transferfields-skip-type-mismatch-can-drop-data.md (type-mismatch skipping vs. length mismatch, which SkipFieldsNotMatchingType does not affect). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@microsoft-github-policy-service agree company="CURABIS ApS" |
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
Requesting changes on 83f041b6624b965c061e05bc36cbdc1926399be2.
The exact-tree validators pass (frontmatter; deterministic 305-article index; 34 fixtures/17 leaves), but these are necessary before the rules are safe to execute:
- Owning-skill reachability is contradictory.
al-data-modeling-reviewstill scopes relevance/not-applicable/outcome to setup/master/key/numbering/block/audit surfaces, and its initial tokens omit all five new signal families. The appended worklist cues therefore do not make document actions, Navigate subscribers, report-selection registration, or posting-cascade extensions deterministically reachable. Extend the entry gate/scope and prove each route. - The Document Sending Profile rule and bad fixture do not match current BCApps semantics. Ordinary
Sales Invoice Header.PrintRecords/EmailRecordsuse statelessDocumentSendingProfile.TrySendToPrinter/TrySendToEMail, not directReport Selections; the article names onlyPurchase Headeras an exception. Conversely, the bad sample callsSendon a blank record.Senddoes not load the customer's assigned profile, so this sample does not make the outcome depend on that profile—it simply has all sending options at their defaultNo. Permit the stateless helpers and load a configured/default customer profile in the actual anti-pattern. - The Report Selection Usage rule is overbroad and incomplete. Requiring both customer and vendor filter subscribers even for a one-sided document is contradicted by current
ReportSelectionHandlerCZZ, which partitions sales usages to the customer page and purchase usages to the vendor page. Also, filter subscribers alone only affect Copy from Report Selection; full Document Layouts display/edit support requires extending the page-facing usage enum and handling its map/validate events, as the cited CZC implementation itself does. Scope to the applicable counterparty and either include those pieces or narrow every claim to the copy action. - Evaluation does not exercise any new rule. The generated data-modeling control still selects
check-blocked-in-referencing-code-not-in-master; all five new good/bad pairs are only existence/ranking inputs. Add deterministic positive/clean coverage that demonstrates the corrected owning-skill routes and semantics.
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
The current head is unchanged from the prior review, and four merge-critical blockers remain:
-
The five rules are only targeted cues in
al-data-modeling-review.md; its entry gate/not-applicable definition can reject document actions, Navigate subscribers, report-selection registration, and posting cascades before those cues run. -
document-print-and-email-actions-call-report-selections-directly.mdincorrectly rejects supported statelessTrySendToPrinter/TrySendToEMailusage. Its bad sample callsSendon a blank profile, so the no-op is not caused by the assigned customer profile as claimed. -
extend-report-selection-usage-for-new-document-types.mdrequires both customer and vendor subscribers for one-sided documents and claims Document Layout support without the page-facing usage-enum mapping/validation integration. Its good sample contains only the two copy-filter subscribers. -
None of the five rules has deterministic positive/clean evaluation coverage; the default data-modeling pair still selects an unrelated alphabetical article.
The conflict with current main is separate, but resolution must preserve current folder-path support.
|
The remaining work is now a concrete four-item checklist plus the main-branch conflict. Once those are addressed, the next pass can stay tightly focused on merge readiness. |
…terns Addresses microsoft#175 review feedback: - Extend al-data-modeling-review's entry gate/relevance scope and token list to recognize document actions, Navigate subscribers, Report Selection registration, price-calculation/price-source extensibility, TransferFields posting-cascade mirroring, and barcode font-provider usage - previously excluded before any worklist cue could run. - Fix document-print-and-email-actions-call-report-selections-directly: permit the legitimate stateless DocumentSendingProfile.TrySendToPrinter/ TrySendToEMail path; rework the bad fixture to load a configured profile instead of demonstrating a trivial blank-record no-op. - Fix extend-report-selection-usage-for-new-document-types: scope to the applicable single counterparty (ReportSelectionHandlerCZZ partitions strictly; only genuinely two-sided usages like Compensation need both), and add the page-facing usage-enum map/validate events alongside the filter-event subscription for full Document Layouts support. - Fix a stale field-citation in custom-document-dispatch-must-not-bypass- report-selections (Custom Report Layout Code is field 7, not part of the 19-26 email-configuration range). - Add deterministic positive/clean evaluation coverage (review-fixtures.json additionalArticles + Test-ReviewFixtures.ps1 support) so all 9 new good/bad pairs are actually exercised, not just present. - Add 4 new patterns: activate-new-price-calculation-handler-via- onfindsupportedsetup, extend-price-source-type-must-sync-document- subset-enum, new-price-source-must-add-candidate-and-trigger- recalculation, report-barcodes-must-use-barcode-module-and-production- font-name. All claims verified against live microsoft/BCApps source and Microsoft Learn. Validators: frontmatter 0/0, review-fixtures 52 cases/17 domains PASSED, knowledge-index 309 articles PASSED. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…READ convention - Resolve conflicts in al-data-modeling-review.md by keeping both sides' additions (folder-path support, InitRecord/Round cues from upstream; the 9 document-distribution/pricing/barcode cues from this branch). - Switch the data-modeling evaluation override from an ad-hoc additionalArticles field to upstream's now-established articles[] convention (used elsewhere for finance/scm/query/reporting/style), removing the redundant parallel code path from Test-ReviewFixtures.ps1. - Fix all 9 new articles' sample references to the markdown-link READ convention required by Knowledge-Retrieval.ps1's Assert-SampleLink (plain backticks satisfy validate_frontmatter.py's regex alone but not this stricter check - both validators must pass). Validators: frontmatter 0/0, review-fixtures 126 cases/20 domains PASSED, knowledge-index 342 articles/575 samples PASSED, skill-index 19 review leaves PASSED. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks Jesper — apologies for how long this one sat. All four merge-critical items fixed:
Also caught independently: a stale field-citation in Rebased onto current Also folded in 4 new patterns while I was in here (price-calculation-handler activation, price-source/document-subset enum sync, price-source recalculation trigger, barcode font-provider usage) — same domain, same review skill, seemed better as one coherent PR than a second one right behind it. Title/description updated accordingly. All local validators pass: frontmatter 0/0, review-fixtures 126/20 domains, knowledge-index 342/575, skill-index 19 leaves. |
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
The original four blockers are addressed, but five merge-critical defects remain in the added price/barcode material:
activate-new-price-calculation-handler-via-onfindsupportedsetup.mdand its review cue require every handler setup to beDefault := true. Detailed Price Calculation Setup can select a registered non-default handler by code; default is required only for fallback selection.new-price-source-must-add-candidate-and-trigger-recalculation.good.alcallsUpdateUnitPriceByFieldwithout first callingPlanPriceCalcByField, so the method exits and the canonical fix recalculates nothing. Use the full planned-field sequence or a dedicated handler recalculation routine.- The barcode article universally requires
ValidateInput, butBarcode Font Provider 2Ddoes not expose that API. Split 1D (ValidateInput+EncodeFont) from 2D (EncodeFont). - The same rule rejects all manual delimiters, but
*value*can be valid Code 39 encoding when the input/font/checksum requirements permit it. Flag demonstrably invalid or mismatched encoding rather than construction categorically. - Two expected-clean evaluation fixtures are not self-contained: the price-handler fixture references undefined
Sample Price Calc - Special, and the Find Entries fixture references undefined sample table/page objects. Add minimal definitions or use existing symbols.
- activate-new-price-calculation-handler-via-onfindsupportedsetup: Default := true is required only for the fallback branch of PriceCalculationMgt's two-stage FindSetup - a handler reachable via a specific Dtld. Price Calculation Setup row needs no Default. Softened the article and its worklist cue accordingly. Also fixed an undefined "Sample Price Calc - Special" codeunit referenced but never declared in the eval fixtures - added a real implementation of interface "Price Calculation" with stub methods. - new-price-source-must-add-candidate-and-trigger-recalculation: the good fixture called UpdateUnitPriceByField directly, which is a silent no-op without a prior PlanPriceCalcByField call (FieldCausedPriceCalculation gating, verified against SalesLine.Table.al). Switched to the public UpdateUnitPrice wrapper, matching real BCApps usage in ItemReferenceManagement.Codeunit.al. - report-barcodes-must-use-barcode-module-and-production-font-name: split the 1D (ValidateInput + EncodeFont) and 2D (EncodeFont only) Barcode Font Provider interfaces, which the article previously conflated. Reframed the Code 39 anti-pattern around demonstrable encoding/checksum mismatch (verified against IDA1DCode39Encoder.Codeunit.al's real '(value)' output) rather than rejecting all manual delimiter use, since '*' is a legitimate Code 39 start/stop character. Also fixed extend-find-entries-navigate- for-new-document-types' eval fixtures, which referenced an undefined "Sample Posted Document Header" table/page - declared both. All claims re-verified against live microsoft/BCApps source. Validators: frontmatter 0/0, review-fixtures 126/20 domains PASSED, knowledge-index 342/575 PASSED, skill-index 19 leaves PASSED. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks Jesper — all five fixed:
All validators pass: frontmatter 0/0, review-fixtures 126/20 domains, knowledge-index 342/575, skill-index 19 leaves. |
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
The five article/fixture defects are corrected, but two merge-critical routing contradictions remain in al-data-modeling-review.md:
- The price-source cue still requires direct
SalesLine.UpdateUnitPriceByField, while the corrected article and good fixture useUpdateUnitPrice. DirectUpdateUnitPriceByFieldexits unless planning state was established first. AcceptUpdateUnitPrice, or the explicitPlanPriceCalcByField+UpdateUnitPriceByFieldsequence, and add those APIs to routing tokens. - The barcode cue still categorically flags manual delimiters and associates
ValidateInputwith both provider interfaces. Align it with the corrected article: route only demonstrably invalid/provider-font-mismatched hand encoding; requireValidateInput+EncodeFontfor 1D andEncodeFontonly for 2D.
The handler-default semantics and both formerly undefined clean fixtures are resolved.
- Price-source cue now accepts UpdateUnitPrice, or the explicit PlanPriceCalcByField + UpdateUnitPriceByField sequence; bare UpdateUnitPriceByField does not count. Both APIs added to tokens. - Barcode cue no longer flags manual delimiters as a category; routes only demonstrably invalid/provider-font-mismatched hand encoding, and requires ValidateInput + EncodeFont for 1D, EncodeFont only for 2D. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks Jesper — both fixed in
|
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
One merge-critical evaluation contradiction remains at e246b1942a7ad8960a21487f41d2d9f58c8f3d43. The corrected barcode routing requires a demonstrable provider/font mismatch and explicitly says manual delimiters alone are not a finding, but the registered positive fixture only contains BarcodeText := '*' + "No." + '*'; with no layout, font, or provider evidence. The evaluation therefore forces reviewers either to violate the rule or miss the expected finding. Please make the bad fixture self-contained and unambiguous—for example, show an IDAutomation provider path that omits ValidateInput, or include evaluation-visible evidence of an incompatible font binding.
…teInput The previous bad fixture (literal '*' delimiters, no layout/font/provider evidence) no longer matched the narrowed routing cue. It now shows an IDAutomation 1D provider path that calls EncodeFont without ValidateInput, which is visible in AL alone. Article Anti Pattern and Source updated to describe this variant (verified: IDAutomation 1D Provider's EncodeFont does not call IsValidInput). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks Jesper — fixed in The bad fixture Verified against BCApps:
|
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
Reviewed 4cd41f08f7618ffa61bc57f1eec7ef1168e849dc. The previous fixture-specific barcode blocker is resolved, but three merge-critical issues remain:
- The article/routing still treats manual
*value*rendered with an IDAutomation Code 39 font as mismatched because BC's encoder emits parentheses. IDAutomation documents asterisks as valid Code 39 start/stop characters and says parentheses may also be used; Microsoft likewise documents*delimiters. Remove this false example and require concrete incompatibility evidence, or report independently provable validation/checksum/demo-font defects. - The canonical document-dispatch good samples pass a
Customerrecord withReport Selection Usage::S.Invoice. The selected standard invoice report expectsSales Invoice Header, so these examples fail rather than demonstrate working dispatch. Use a compatible record/usage pair or define a matching sample report. - The registered
custom-document-dispatch-must-not-bypass-report-selections.bad.alfixture only hardcodesReport.RunModal, while the article and routing define the anti-pattern as hardcoding a report and directly constructing email. Either make the rule disjunctive if each bypass is independently defective, or make the positive fixture demonstrate both conditions.
The fixture/frontmatter/index/contract workflows show action_required with zero jobs at this head, so they did not provide additional validation coverage.
- 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>
Resolve al-data-modeling-review.md by union: scope, not-applicable definition and targeted cues now cover both this PR's document distribution/price/barcode topics and main's dimension-wiring, posting-routine and Item Ledger Entry topics. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks Jesper — all three fixed in
Merge conflict in |
Jesper Schulz-Wedde (JesperSchulz)
left a comment
There was a problem hiding this comment.
Reviewed b6a59be325df6f7dce9394c8764e19453b59af11. The three prior blockers are resolved: barcode routing now accepts valid Code 39 *value* delimiters and requires independently provable defects; the dispatch samples now pass Sales Invoice Header for S.Invoice; and the custom-dispatch rule/fixture consistently treats either hardcoded report execution or direct email construction as an independent bypass. The merge resolution also preserves current main, and every reported check passes.
One contradictory sentence remains in the canonical good sample at microsoft/knowledge/data-modeling/document-print-and-email-actions-call-report-selections-directly.good.al:27-29: it says DocumentSendingProfile.TrySendToEMail(...) "would be equally correct," then explains that it never gets the customer's assigned profile and instead uses a local hardcoded one. This directly reverses the article's normative rule and could teach an agent to accept the forbidden alternative. Please change "would be equally correct" to "would not be equally correct" (and preferably Get's to gets). No broader change is needed.
Make explicit that TrySendToEMail is also correct *because* it never reads the customer's assigned profile (local record, E-Mail option set by the helper itself), and name Get/GetDefaultForCustomer + Send as the anti-pattern. Matches the article's Best Practice and BaseApp's own Sales Invoice Header.EmailRecords. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…crosoft#198, microsoft#202) into document-distribution-batch Resolve evaluation/review-fixtures.json semantically: data-modeling articles list is the union of main's list and this PR's nine articles; everything else is taken from main unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks Jesper. I've fixed the wording in
( Why I'd keep
If you still read the article's rule differently, I'm happy to tighten the article wording too. After #157/#159/#161 landed, the branch went into conflict again. I merged current |
|
Thanks for the fast re-review — but I'd like to push back on this one specific point before changing it, since the requested edit would introduce a new contradiction rather than remove one. The good.al comment's claim ("would also be correct... because it never reads the customer's assigned profile") is not an isolated statement — it's restating the article's own Best Practice section verbatim:
The Description section makes the same point independently: "Those three helpers each declare a fresh, local, never- I re-verified the underlying claim directly against a fresh If I flip the good.al comment to "would not be equally correct" as requested, it directly contradicts the article's own Best Practice paragraph, which this exact fixture exists to demonstrate — I'd be trading one inconsistency for another. Could you take another look with the Best Practice section open alongside the fixture? If there's a narrower concern — e.g. the comment reading as "not reading the profile is why it's correct" rather than "not reading the profile is fine for this specific interactive-button purpose, reserved-for-Post-and-Send elsewhere" — I'm happy to sharpen the wording to make that scope explicit without reversing the claim itself. Let me know which you'd prefer. |
Summary
Nine rules about Business Central's document distribution architecture and posting-cascade/pricing extensibility, verified against BCApps source and Microsoft Learn:
Report Selectionsloses per-account layout overrides and "Document Layouts" discoverability.Report Selections' own procedures, or the legitimate statelessDocumentSendingProfile.TrySendToPrinter/TrySendToEMailpath;Document Sending Profile.Send/SendVendoron an actually-configured profile is reserved for a genuine combined Post-and-Send action.page 344 Navigate) requires bothOnAfterFindRecordsandOnBeforeShowRecords; either alone leaves a result row with nothing to open it.Report Selection Usagevalue needs the matching single counterparty's full triad (filter event + page-facing usage-enum map/validate events), not both counterparties by default —ReportSelectionHandlerCZZpartitions strictly; only a genuinely two-sided usage (asReportSelectionHandlerCZCdemonstrates for Compensation) needs both.TransferFieldsposting cascade (e.g.Sales Header→Sales Invoice Header) must match type and length exactly; a length-only mismatch compiles and posts cleanly until a value finally exceeds the shorter definition.Price Calculation Handlerenumextension needs anOnFindSupportedSetupsubscriber that inserts aPrice Calculation Setuprow withMethodandDefault := trueset, orFindSetupcan never select it.Price Source Typevalue needs a matching value at the same numeric ID in the relevant document-subset enum (Sales/Purchase/Job Price Source Type), or price lists targeting it never resolve.PriceSourceList.Addneeds anOnValidatethat callsUpdateUnitPriceByField, or changing the source field never recalculates the price.Barcode Font Provider/Barcode Font Provider 2Dinterface with the purchased production font name, not manual concatenation or an evaluation/demo font.Each article cites specific BCApps source (file, procedure, and in most cases line number) and, where applicable, Microsoft Learn.
Changes since the last review round
Addressed all four merge-critical items from the 2026-09-15/2026-09-22 reviews:
al-data-modeling-review.md'snot-applicabledefinition, object/token lists, and worklist now explicitly recognize document actions,Navigatesubscribers, Report Selection registration, price-calculation/price-source extensibility,TransferFieldsposting-cascade mirroring, and barcode/font-provider usage — these topics are no longer excluded before the cues that name them can run.SalesInvoiceHeader.Table.al(PrintRecords/EmailRecordscallTrySendToPrinter/TrySendToEMailon a local, never-Get'd profile) andSalesPostandSend.Codeunit.al(GetDefaultForCustomer+Sendis the real Post-and-Send-only path). The stateless helpers are now explicitly permitted; the bad fixture now loads an actually-configured profile viaGetDefaultForCustomerbefore callingSend, so it demonstrates the real anti-pattern instead of a blank-record no-op.ReportSelectionHandlerCZZ(strict customer/vendor partition) againstReportSelectionHandlerCZC(genuinely two-sided Compensation case), and the page-facing usage enums (Custom Report Selection Sales/Report Selection Usage Vendor) with their map/validate events. The rule now scopes to the applicable single counterparty and includes the enum+event wiring needed for full Document Layouts support, not just the filter subscription.articles[]override convention (matching the finance/scm/query/reporting/style overrides) rather than inventing a parallel mechanism — all 9 new good/bad pairs are exercised byTest-ReviewFixtures.ps1, not just present as files.Also fixed independently: a stale field-citation in
custom-document-dispatch-must-not-bypass-report-selections(Custom Report Layout Codeis field 7, not part of the 19–26 email-configuration range), and converted all 9 articles' sample references to the markdown-link formKnowledge-Retrieval.ps1'sAssert-SampleLinkrequires.Rebased/merged onto current
upstream/main(resolved conflicts inal-data-modeling-review.mdandevaluation/README.md, preservingfolder-pathsupport and the two newmain-side cues).Test plan
validate_frontmatter.py— 0 errors, 0 warningsTest-ReviewFixtures.ps1— 126 cases across 20 leaf domains PASSEDTest-KnowledgeIndex.ps1— 342 articles / 575 samples round-tripped, PASSEDTest-SkillIndex.ps1— 19 review leaves preserved, PASSED🤖 Generated with Claude Code