diff --git a/community/knowledge/data-modeling/document-line-prices-follow-prices-including-vat.bad.al b/community/knowledge/data-modeling/document-line-prices-follow-prices-including-vat.bad.al new file mode 100644 index 0000000..453f8a4 --- /dev/null +++ b/community/knowledge/data-modeling/document-line-prices-follow-prices-including-vat.bad.al @@ -0,0 +1,26 @@ +// Demonstration only; independently authored, not copied from BaseApp. +codeunit 50641 "Sales Doc. VAT Basis Bad" +{ + procedure GetNetAndGrossTotals(SalesHeader: Record "Sales Header"; var NetTotal: Decimal; var GrossTotal: Decimal) + var + SalesLine: Record "Sales Line"; + begin + SalesLine.SetRange("Document Type", SalesHeader."Document Type"); + SalesLine.SetRange("Document No.", SalesHeader."No."); + if SalesLine.FindSet() then + repeat + // Wrong: "Line Amount" already includes VAT when the header has Prices Including VAT, + // so NetTotal is gross and VAT is added a second time. Invoice discount is also ignored. + NetTotal += SalesLine."Line Amount"; + GrossTotal += SalesLine."Line Amount" * (1 + SalesLine."VAT %" / 100); + until SalesLine.Next() = 0; + end; + + procedure SetUnitPriceFromNetSourcePrice(var SalesLine: Record "Sales Line"; NetSourcePrice: Decimal) + begin + // Wrong: on a Prices Including VAT document this net price is read as a gross price, + // so the net line amount drops to NetSourcePrice / (1 + "VAT %" / 100). + SalesLine.Validate("Unit Price", NetSourcePrice); + SalesLine.Modify(true); + end; +} diff --git a/community/knowledge/data-modeling/document-line-prices-follow-prices-including-vat.good.al b/community/knowledge/data-modeling/document-line-prices-follow-prices-including-vat.good.al new file mode 100644 index 0000000..14772cf --- /dev/null +++ b/community/knowledge/data-modeling/document-line-prices-follow-prices-including-vat.good.al @@ -0,0 +1,36 @@ +// Demonstration only; independently authored, not copied from BaseApp. +codeunit 50640 "Sales Doc. VAT Basis Good" +{ + procedure GetNetAndGrossTotals(SalesHeader: Record "Sales Header"; var NetTotal: Decimal; var GrossTotal: Decimal) + var + SalesLine: Record "Sales Line"; + begin + SalesLine.SetRange("Document Type", SalesHeader."Document Type"); + SalesLine.SetRange("Document No.", SalesHeader."No."); + // Amount is always net and "Amount Including VAT" always gross, after line and + // invoice discounts, whatever the header's "Prices Including VAT" says. + SalesLine.CalcSums(Amount, "Amount Including VAT"); + NetTotal := SalesLine.Amount; + GrossTotal := SalesLine."Amount Including VAT"; + end; + + procedure SetUnitPriceFromNetSourcePrice(var SalesLine: Record "Sales Line"; NetSourcePrice: Decimal) + var + SalesHeader: Record "Sales Header"; + Currency: Record Currency; + UnitPrice: Decimal; + begin + SalesHeader.Get(SalesLine."Document Type", SalesLine."Document No."); + // Scope of the conversion below: Normal VAT only; Full VAT and Sales Tax need their own handling. + SalesLine.TestField("VAT Calculation Type", SalesLine."VAT Calculation Type"::"Normal VAT"); + Currency.Initialize(SalesHeader."Currency Code"); + + UnitPrice := NetSourcePrice; + // "Unit Price" is gross on a Prices Including VAT document: convert the net source price into that basis. + if SalesHeader."Prices Including VAT" then + UnitPrice := Round(NetSourcePrice * (1 + SalesLine."VAT %" / 100), Currency."Unit-Amount Rounding Precision"); + + SalesLine.Validate("Unit Price", UnitPrice); + SalesLine.Modify(true); + end; +} diff --git a/community/knowledge/data-modeling/document-line-prices-follow-prices-including-vat.md b/community/knowledge/data-modeling/document-line-prices-follow-prices-including-vat.md new file mode 100644 index 0000000..944387a --- /dev/null +++ b/community/knowledge/data-modeling/document-line-prices-follow-prices-including-vat.md @@ -0,0 +1,44 @@ +--- +bc-version: [all] +domain: data-modeling +keywords: [prices-including-vat, unit-price, line-amount, direct-unit-cost, prepmt-line-amount, amount-including-vat, sales-line, purchase-line, net-gross] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Sales and purchase line prices follow the header's Prices Including VAT + +> Contributions welcome — open a PR to refine or extend this article. + +## Description + +Line prices are gross or net depending on the header's `Prices Including VAT`. When the flag is set, `Unit Price` (sales), `Direct Unit Cost` (purchase), `Line Amount`, `Line Discount Amount`, `Inv. Discount Amount`, and `Prepmt. Line Amount` all include VAT; when it is cleared they exclude it. Only `Amount` (always net), `Amount Including VAT` (always gross), and `VAT Base Amount` keep a fixed basis. Code that reads or writes the price fields without looking at the header flag is wrong for every document whose customer or vendor uses the other setting. + +## Best Practice + +When code needs a known basis — exports, integrations, KPIs, custom totals, commission or margin calculations — read `Amount` for net and `Amount Including VAT` for gross. Both are already reduced by line and invoice discounts and are maintained by the line's VAT calculation, so no VAT arithmetic is needed. + +When code writes a price from an external source whose basis is known (an EDI price list, an API payload, a web-shop order), get the document header and convert the source price into the header's basis before validating `Unit Price` or `Direct Unit Cost`, using the line's `VAT %` and the currency's `Unit-Amount Rounding Precision`. Alternatively, set `Prices Including VAT` on the header to match the source before the first line is created. Toggling it later on a header with priced lines asks the user to confirm a recalculation. Without a UI session, or when validation dialogs are hidden, BaseApp converts all line prices without asking. + +The standard price calculation already converts a `Price List Line` whose `Price Includes VAT` differs from the document's setting, so a price it returns is in the document's basis and must not be converted a second time. + +The same rule applies to the purchase mirror fields. On purchase lines, `Unit Cost` and `Unit Cost (LCY)` are derived from `Direct Unit Cost` with VAT removed, so they stay net. + +See sample: [`document-line-prices-follow-prices-including-vat.good.al`](document-line-prices-follow-prices-including-vat.good.al). + +## Anti Pattern + +Treating `Unit Price`, `Direct Unit Cost`, or `Line Amount` as net by default. Typical signals: summing `Line Amount` as a document's net total, computing VAT as `Line Amount * "VAT %" / 100`, putting a known net or gross external price straight into `Unit Price` or `Direct Unit Cost`, or comparing a line price with `Item."Unit Price"` or `Item."Last Direct Cost"` — all without reading the header's `Prices Including VAT`. For a gross-price customer at 19 % VAT, a net total built from `Line Amount` is 19 % too high, and a net price of 100 imported unconverted yields a net revenue of only 84.03. A variable or procedure name ending in `ExclVAT` or `Net` that is fed from `Line Amount` is a strong signal of this mistake. BaseApp's own `CalculateOutstandingAmountExclTax` shows that the name alone guarantees nothing: it returns a value based on `Line Amount`. + +Do not report code that reads the header flag, uses `Amount` or `Amount Including VAT`, or only passes values between two lines of the same document (same basis on both sides). + +See sample: [`document-line-prices-follow-prices-including-vat.bad.al`](document-line-prices-follow-prices-including-vat.bad.al). + +## References + +- [BCApps: Sales Header `Prices Including VAT` OnValidate recalculates line prices](https://github.com/microsoft/BCApps/blob/main/src/Layers/W1/BaseApp/Sales/Document/SalesHeader.Table.al). +- [BCApps: Sales Line `UpdateVATAmounts` derives `Amount` from `Line Amount` per basis](https://github.com/microsoft/BCApps/blob/main/src/Layers/W1/BaseApp/Sales/Document/SalesLine.Table.al). +- [BCApps: `Sales Line CaptionClass Mgmt` switches captions to Incl./Excl. VAT](https://github.com/microsoft/BCApps/blob/main/src/Layers/W1/BaseApp/Sales/Document/SalesLineCaptionClassMgmt.Codeunit.al). +- [BCApps: Purchase Line `UpdateUnitCost` removes VAT from `Direct Unit Cost`](https://github.com/microsoft/BCApps/blob/main/src/Layers/W1/BaseApp/Purchases/Document/PurchaseLine.Table.al). +- [BCApps: `Price Calculation Buffer Mgt.` `ConvertAmountByTax`](https://github.com/microsoft/BCApps/blob/main/src/Layers/W1/BaseApp/Pricing/Calculation/PriceCalculationBufferMgt.Codeunit.al). diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index 5f0e50a..628526d 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -17,6 +17,7 @@ "check-blocked-in-referencing-code-not-in-master", "code-must-not-change-workdate", "custom-document-dispatch-must-not-bypass-report-selections", + "document-line-prices-follow-prices-including-vat", "document-print-and-email-actions-call-report-selections-directly", "extend-find-entries-navigate-for-new-document-types", "extend-price-source-type-must-sync-document-subset-enum", diff --git a/microsoft/skills/review/al-data-modeling-review.md b/microsoft/skills/review/al-data-modeling-review.md index 5f4cec7..16f3a7a 100644 --- a/microsoft/skills/review/al-data-modeling-review.md +++ b/microsoft/skills/review/al-data-modeling-review.md @@ -16,7 +16,7 @@ application-area: [all] Reviews AL source changes against the `data-modeling` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is one of the skills composed by `al-code-review`. -An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. Data-modeling findings are narrow by design — they apply when the review scope contains setup or master tables, their card pages, primary keys, number-series assignment, block enforcement, audit fields, document print/email/Post-and-Send actions, `Navigate` page subscribers, Report Selection registration or dispatch, price-calculation/price-source extensibility, `TransferFields`-based posting-cascade field mirroring, barcode/report-layout font-provider usage, dimension wiring, journal-based posting-routine structure, or Item Ledger Entry document-number lookups after a combined sales post. The skill returns `not-applicable` when none of those apply. +An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. Data-modeling findings are narrow by design — they apply when the review scope contains setup or master tables, their card pages, primary keys, number-series assignment, block enforcement, audit fields, document print/email/Post-and-Send actions, `Navigate` page subscribers, Report Selection registration or dispatch, price-calculation/price-source extensibility, code that reads or writes sales/purchase line price and amount fields, `TransferFields`-based posting-cascade field mirroring, barcode/report-layout font-provider usage, dimension wiring, journal-based posting-routine structure, or Item Ledger Entry document-number lookups after a combined sales post. The skill returns `not-applicable` when none of those apply. ## Source @@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review - The changed AL object names and types — especially `* Setup` singleton tables and Card pages, custom master tables, tableextensions that add master-data fields, document or journal lines that reference a master, document pages/codeunits exposing print/email/Post-and-Send actions, codeunits subscribing to `Navigate`, enumextensions to `"Report Selection Usage"`/`"Price Calculation Handler"`/`"Price Source Type"`, and report objects that render barcodes. - The changed fields, keys, triggers, and procedures, weighted toward `Primary Key`, `No.`, `No. Series`, `Blocked`, `Last Date Modified`, `OnInsert`, `OnModify`, `OnRename`, reference-field `OnValidate`, posting validation, and posting-cascade `TransferFields` calls. -- Tokens extracted from the diff that relate to data modeling (`setup`, `master`, `Primary Key`, `Code[10]`, `Code[20]`, `AutoIncrement`, `SystemId`, `No.`, `No. Series`, `NoSeriesManagement`, `Codeunit "No. Series"`, `GetNextNo`, `IsManual`, `TestManual`, `Blocked`, `TestField`, `Last Date Modified`, `Today`, `WorkDate`, `InsertAllowed`, `DeleteAllowed`, `PageType = Card`, `OnOpenPage`, `GetRecordOnce`, `OnInsert`, `OnModify`, `OnRename`, `InitRecord`, `Round`, `Precision`, `Direction`, `TableRelation`, `tableextension`, `enumextension`, `Media`, `MediaSet`, `Item`, `Count`, `TransferFields`, `Navigate`, `OnAfterFindRecords`, `OnBeforeShowRecords`, `Report Selections`, `Report Selection Usage`, `InsertRecord`, `Document Sending Profile`, `PrintForCust`, `PrintWithDialogForCust`, `PrintWithDialogForVend`, `SendEmailToCust`, `SendEmailToVendor`, `Report.RunModal`, `Report.Run`, `Price Calculation Handler`, `Price Calculation`, `OnFindSupportedSetup`, `Price Calculation Setup`, `Price Source Type`, `PriceSourceList`, `OnAfterAddSources`, `UpdateUnitPrice`, `PlanPriceCalcByField`, `UpdateUnitPriceByField`, `Barcode Font Provider`, `Barcode Font Provider 2D`, `EncodeFont`, `ValidateInput`). +- Tokens extracted from the diff that relate to data modeling (`setup`, `master`, `Primary Key`, `Code[10]`, `Code[20]`, `AutoIncrement`, `SystemId`, `No.`, `No. Series`, `NoSeriesManagement`, `Codeunit "No. Series"`, `GetNextNo`, `IsManual`, `TestManual`, `Blocked`, `TestField`, `Last Date Modified`, `Today`, `WorkDate`, `InsertAllowed`, `DeleteAllowed`, `PageType = Card`, `OnOpenPage`, `GetRecordOnce`, `OnInsert`, `OnModify`, `OnRename`, `InitRecord`, `Round`, `Precision`, `Direction`, `TableRelation`, `tableextension`, `enumextension`, `Media`, `MediaSet`, `Item`, `Count`, `TransferFields`, `Navigate`, `OnAfterFindRecords`, `OnBeforeShowRecords`, `Report Selections`, `Report Selection Usage`, `InsertRecord`, `Document Sending Profile`, `PrintForCust`, `PrintWithDialogForCust`, `PrintWithDialogForVend`, `SendEmailToCust`, `SendEmailToVendor`, `Report.RunModal`, `Report.Run`, `Price Calculation Handler`, `Price Calculation`, `OnFindSupportedSetup`, `Price Calculation Setup`, `Price Source Type`, `PriceSourceList`, `OnAfterAddSources`, `UpdateUnitPrice`, `PlanPriceCalcByField`, `UpdateUnitPriceByField`, `Prices Including VAT`, `Unit Price`, `Direct Unit Cost`, `Line Amount`, `Prepmt. Line Amount`, `Amount Including VAT`, `Barcode Font Provider`, `Barcode Font Provider 2D`, `EncodeFont`, `ValidateInput`). A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. When the diff contains no data-modeling changes by any of the above signals, return `outcome: "not-applicable"` without evaluating files. @@ -66,6 +66,7 @@ The following targeted checks cover every current `data-modeling` article. Treat - An `enumextension` extends `"Price Calculation Handler"` and implements the `Price Calculation` interface, without a matching `OnFindSupportedSetup` subscriber inserting a `Price Calculation Setup` record naming that implementation as the `Implementation` for a `Method`/`Type`/`Asset Type` — `activate-new-price-calculation-handler-via-onfindsupportedsetup`. `Default := true` is only required on that row when it is meant as the fallback for its `Method`/`Type`/`Asset Type` combination; a row meant to be selected only through an explicit, specific `"Dtld. Price Calculation Setup"` row does not need it, so do not flag a missing `Default := true` by itself — flag the missing setup row/subscriber entirely. - An `enumextension` extends `"Price Source Type"` with a new value intended for a sales, purchase, or job price list, without extending the matching document subset enum (`"Sales Price Source Type"`, `"Purchase Price Source Type"`, `"Job Price Source Type"`) with a value at the same numeric ID — `extend-price-source-type-must-sync-document-subset-enum`. - A codeunit subscribes to `"Sales Line - Price"`'s `OnAfterAddSources` to register a custom field as a price source via `PriceSourceList.Add`, but that field has no `OnValidate` (or matching `OnAfterValidate`) that triggers recalculation — either `SalesLine.UpdateUnitPrice()`, or the explicit `SalesLine.PlanPriceCalcByField()` followed by `SalesLine.UpdateUnitPriceByField()`. A bare `UpdateUnitPriceByField` without a preceding `PlanPriceCalcByField` for the same field number does not count as recalculation (it exits without recalculating) — `new-price-source-must-add-candidate-and-trigger-recalculation`. +- Code reads `Unit Price`, `Direct Unit Cost`, `Line Amount`, `Line Discount Amount`, `Inv. Discount Amount`, or `Prepmt. Line Amount` of a `Sales Line`/`Purchase Line` as a known net or gross value (a net/gross total, a VAT computation, a comparison with `Item."Unit Price"`/`"Last Direct Cost"`, an export), or writes a source price of known basis into `Unit Price`/`Direct Unit Cost`, without reading the document header's `Prices Including VAT` — `document-line-prices-follow-prices-including-vat`. Reads of `Amount`/`Amount Including VAT`, prices returned by the standard price calculation, and copies between lines of the same document are not this anti-pattern. - A report hand-constructs a barcode string only where a concrete, independently provable defect is visible: the source value can contain characters outside the symbology's character set and is never validated, a checksum the symbology/setup requires is never applied, or there is concrete evidence of an incompatible font binding. Do not flag manual start/stop delimiters by themselves — `*value*` is a documented, valid Code 39 form for IDAutomation fonts (IDAutomation also accepts parentheses), so delimiter choice alone is never a finding. Also flag module use that does not match the interface: a 1D `"Barcode Font Provider"` path must call both `ValidateInput` and `EncodeFont`; a 2D `"Barcode Font Provider 2D"` path calls `EncodeFont` only (the 2D interface has no `ValidateInput`, so its absence there is not a finding). Separately, flag an otherwise correctly encoded barcode whose report layout names an evaluation/demo font instead of the purchased production font name — `report-barcodes-must-use-barcode-module-and-production-font-name`. - A new field is typed `Code`/`Text` and its `OnValidate` calls `DimensionManagement`/`DimMgt`, or a table adds Shortcut Dimension fields, a `Dimension Set ID` field, or `AddDimSource`/`GetDefaultDimID` — `dimension-management-wiring`. A master table calling `SaveDefaultDim` and a document/journal table computing its own `Dimension Set ID` are two different valid shapes; do not flag a master table for lacking a `Dimension Set ID` field or a document for lacking `SaveDefaultDim`. - A journal-based posting codeunit is added or changed and validation, Journal-table access, ledger writes, and user-interaction (`Confirm`/dialogs) all occur in one procedure or one codeunit, rather than split across `Check Line`/`Post Line`/`Post Batch`-shaped companions — `check-post-line-batch-pattern`. A document posting routine calling `Post Line` directly without a `Post Batch` companion is not this anti-pattern. @@ -100,7 +101,7 @@ Outcome selection: - `completed` — the skill evaluated every worklist item. - `no-knowledge` — no applicable data-modeling knowledge survived filtering. -- `not-applicable` — the diff touches no setup/master table, page, key, numbering, block-check, or audit-field surface, and no document print/email/Post-and-Send action, `Navigate` subscriber, Report Selection registration/dispatch, price-calculation/price-source extensibility point, posting-cascade `TransferFields` mirroring, barcode/report-font-provider usage, dimension wiring, posting-routine structure, or Item-Ledger-Entry-document-number surface. +- `not-applicable` — the diff touches no setup/master table, page, key, numbering, block-check, or audit-field surface, and no document print/email/Post-and-Send action, `Navigate` subscriber, Report Selection registration/dispatch, price-calculation/price-source extensibility point, sales/purchase line price or amount read/write, posting-cascade `TransferFields` mirroring, barcode/report-font-provider usage, dimension wiring, posting-routine structure, or Item-Ledger-Entry-document-number surface. - `partial` — a budget was hit before the worklist was exhausted. - `failed` — an unrecoverable error occurred.