diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index 68904529..1c63947a 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -3,7 +3,16 @@ "selection": "first-paired-al-article", "minimumExpectedRecall": 1.0, "minimumCleanRate": 1.0, - "coverageWaivers": [], + "coverageWaivers": [ + { + "path": "microsoft/knowledge/reporting/do-not-rename-or-remove-shipped-report-dataset-columns.md", + "reason": "The defect is a change relative to a released version; a single neutralized sample cannot show which column names shipped before." + }, + { + "path": "microsoft/knowledge/reporting/reportextension-layouts-are-not-selected-automatically.md", + "reason": "The defect is the expectation that the layout becomes the default; the code alone is identical to a legitimate optional layout once comments are removed." + } + ], "overrides": { "agents": { "article": "wire-all-three-agent-interfaces" @@ -104,6 +113,8 @@ "query": { "articles": [ "dataitemtablefilter-cannot-be-overwritten-at-runtime", + "every-non-aggregated-query-column-is-a-group-key", + "query-dataitems-default-to-left-outer-join", "reopening-query-resets-cursor-but-keeps-filters", "set-query-filters-before-open", "setfilter-overwrites-query-columnfilter" @@ -115,6 +126,7 @@ "currreport-break-ends-the-current-trigger", "currreport-quit-rolls-back-and-skips-onpostreport", "currreport-skip-does-not-stop-trigger-code", + "declare-report-layouts-in-the-rendering-section", "report-output-in-a-loop-needs-one-client-download", "reportextension-dataitem-trigger-order-is-explicit", "reportextension-report-triggers-run-after-base-triggers", diff --git a/microsoft/knowledge/query/every-non-aggregated-query-column-is-a-group-key.bad.al b/microsoft/knowledge/query/every-non-aggregated-query-column-is-a-group-key.bad.al new file mode 100644 index 00000000..0fff9f45 --- /dev/null +++ b/microsoft/knowledge/query/every-non-aggregated-query-column-is-a-group-key.bad.al @@ -0,0 +1,36 @@ +// Demonstration only. Shows the wrong pattern: a display column changes the grouping of a totals query. + +query 50231 "Customer Sales Totals Bad" +{ + QueryType = Normal; + + elements + { + dataitem(CustLedgerEntry; "Cust. Ledger Entry") + { + DataItemTableFilter = "Document Type" = const(Invoice); + column(CustomerNo; "Customer No.") { } + // BAD: Document No. varies within each customer, so the query now returns one row per + // customer per invoice instead of one total per customer. + column(DocumentNo; "Document No.") { } + column(SalesLCY; "Sales (LCY)") + { + Method = Sum; + } + } + } +} + +codeunit 50232 "Customer Sales Lookup Bad" +{ + procedure GetSalesForCustomer(CustomerNo: Code[20]): Decimal + var + CustomerSalesTotals: Query "Customer Sales Totals Bad"; + begin + CustomerSalesTotals.SetRange(CustomerNo, CustomerNo); + CustomerSalesTotals.Open(); + // Reads only the first invoice's partial total. + if CustomerSalesTotals.Read() then + exit(CustomerSalesTotals.SalesLCY); + end; +} diff --git a/microsoft/knowledge/query/every-non-aggregated-query-column-is-a-group-key.good.al b/microsoft/knowledge/query/every-non-aggregated-query-column-is-a-group-key.good.al new file mode 100644 index 00000000..6d075ed4 --- /dev/null +++ b/microsoft/knowledge/query/every-non-aggregated-query-column-is-a-group-key.good.al @@ -0,0 +1,32 @@ +// Demonstration only. Shows the recommended pattern: only the grouping column is a non-aggregated column. + +query 50233 "Customer Sales Totals Good" +{ + QueryType = Normal; + + elements + { + dataitem(CustLedgerEntry; "Cust. Ledger Entry") + { + DataItemTableFilter = "Document Type" = const(Invoice); + column(CustomerNo; "Customer No.") { } + column(SalesLCY; "Sales (LCY)") + { + Method = Sum; + } + } + } +} + +codeunit 50234 "Customer Sales Lookup Good" +{ + procedure GetSalesForCustomer(CustomerNo: Code[20]): Decimal + var + CustomerSalesTotals: Query "Customer Sales Totals Good"; + begin + CustomerSalesTotals.SetRange(CustomerNo, CustomerNo); + CustomerSalesTotals.Open(); + if CustomerSalesTotals.Read() then + exit(CustomerSalesTotals.SalesLCY); + end; +} diff --git a/microsoft/knowledge/query/every-non-aggregated-query-column-is-a-group-key.md b/microsoft/knowledge/query/every-non-aggregated-query-column-is-a-group-key.md new file mode 100644 index 00000000..c3eab6d5 --- /dev/null +++ b/microsoft/knowledge/query/every-non-aggregated-query-column-is-a-group-key.md @@ -0,0 +1,32 @@ +--- +bc-version: [all] +domain: query +keywords: [query-totals, method, sum, count, average, group-by, grouping, aggregate, query-column] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Every non-aggregated query column is a group key + +## Description + +As soon as one query column sets `Method` to `Sum`, `Count`, `Average`, `Min`, or `Max`, the query groups its result by all other columns, including columns of other data items and columns that use a date method (`Day`, `Month`, `Year`). There is no explicit GROUP BY; the grouping is implied by the column list. Adding a column "just to display it", such as a document number, a description, or a posting date, therefore silently changes the grain of the result: one total per customer becomes one total per customer per document, and the code that reads one row per customer reads the first partial total or adds duplicates. + +Integer `Average` also uses integer division, so averages of `Integer` fields lose their fractional part. + +## Best Practice + +Treat the non-aggregated column list as the GROUP BY clause. Include only the columns that define the grain you want, and get descriptive values (names, descriptions) either from a column that is constant within the group or from a separate lookup after reading the totals. When a column is needed only for filtering, use `filter` elements or `ColumnFilter`/`DataItemTableFilter` instead of a `column`. Average `Decimal` fields, or compute the average in AL from `Sum` and `Count`. + +See sample: [`every-non-aggregated-query-column-is-a-group-key.good.al`](every-non-aggregated-query-column-is-a-group-key.good.al). + +## Anti Pattern + +A totals query, meaning one with any aggregate `Method`, that also contains a `column` whose value varies within the intended group (document number, line number, entry number, posting date without a date method), while the query's name or consuming code expects one row per group, for example via a single `Read()` or a dictionary keyed by the group field. + +See sample: [`every-non-aggregated-query-column-is-a-group-key.bad.al`](every-non-aggregated-query-column-is-a-group-key.bad.al). + +## References + +[Aggregating data in query objects](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-query-totals-grouping) (an aggregate method groups the dataset by the other columns; integer Average uses integer division) and [Method property](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/properties/devenv-method-property) (date-method columns remain part of the group). diff --git a/microsoft/knowledge/query/query-dataitems-default-to-left-outer-join.bad.al b/microsoft/knowledge/query/query-dataitems-default-to-left-outer-join.bad.al new file mode 100644 index 00000000..4656c577 --- /dev/null +++ b/microsoft/knowledge/query/query-dataitems-default-to-left-outer-join.bad.al @@ -0,0 +1,36 @@ +// Demonstration only. A sales-document count includes customers without any document. + +query 50227 "Cust With Sales Docs Bad" +{ + QueryType = Normal; + + elements + { + dataitem(Customer; Customer) + { + column(CustomerNo; "No.") { } + column(CustomerName; Name) { } + // BAD: no child filter rejects nulls, so unmatched customers survive the default join. + dataitem(SalesHeader; "Sales Header") + { + DataItemLink = "Sell-to Customer No." = Customer."No."; + column(OrderNo; "No.") { } + } + } + } +} + +codeunit 50228 "Sales Doc Reminder Bad" +{ + procedure CountSalesDocsToRemind(): Integer + var + CustomersWithSalesDocs: Query "Cust With Sales Docs Bad"; + Total: Integer; + begin + CustomersWithSalesDocs.Open(); + while CustomersWithSalesDocs.Read() do + Total += 1; + CustomersWithSalesDocs.Close(); + exit(Total); + end; +} diff --git a/microsoft/knowledge/query/query-dataitems-default-to-left-outer-join.good.al b/microsoft/knowledge/query/query-dataitems-default-to-left-outer-join.good.al new file mode 100644 index 00000000..9453feb5 --- /dev/null +++ b/microsoft/knowledge/query/query-dataitems-default-to-left-outer-join.good.al @@ -0,0 +1,68 @@ +// Demonstration only. InnerJoin excludes unmatched rows; a null-rejecting child filter is also valid. + +query 50229 "Cust With Sales Docs Good" +{ + QueryType = Normal; + + elements + { + dataitem(Customer; Customer) + { + column(CustomerNo; "No.") { } + column(CustomerName; Name) { } + dataitem(SalesHeader; "Sales Header") + { + DataItemLink = "Sell-to Customer No." = Customer."No."; + SqlJoinType = InnerJoin; + column(OrderNo; "No.") { } + } + } + } +} + +codeunit 50230 "Sales Doc Reminder Good" +{ + procedure CountSalesDocsToRemind(): Integer + var + CustomersWithSalesDocs: Query "Cust With Sales Docs Good"; + Total: Integer; + begin + CustomersWithSalesDocs.Open(); + while CustomersWithSalesDocs.Read() do + Total += 1; + CustomersWithSalesDocs.Close(); + exit(Total); + end; + + procedure CountOpenOrdersToRemind(): Integer + var + CustomersWithOpenOrders: Query "Cust With Open Orders"; + Total: Integer; + begin + CustomersWithOpenOrders.Open(); + while CustomersWithOpenOrders.Read() do + Total += 1; + CustomersWithOpenOrders.Close(); + exit(Total); + end; +} + +query 50235 "Cust With Open Orders" +{ + QueryType = Normal; + + elements + { + dataitem(Customer; Customer) + { + column(CustomerNo; "No.") { } + column(CustomerName; Name) { } + dataitem(SalesHeader; "Sales Header") + { + DataItemLink = "Sell-to Customer No." = Customer."No."; + DataItemTableFilter = "Document Type" = const(Order), Status = const(Open); + column(OrderNo; "No.") { } + } + } + } +} diff --git a/microsoft/knowledge/query/query-dataitems-default-to-left-outer-join.md b/microsoft/knowledge/query/query-dataitems-default-to-left-outer-join.md new file mode 100644 index 00000000..a0d1a5da --- /dev/null +++ b/microsoft/knowledge/query/query-dataitems-default-to-left-outer-join.md @@ -0,0 +1,34 @@ +--- +bc-version: [all] +domain: query +keywords: [sqljointype, left-outer-join, inner-join, dataitemlink, query-dataitem, null-columns, join] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# A lower query data item without SqlJoinType is a left outer join + +## Description + +When a lower `dataitem` in a query sets `DataItemLink` but omits `SqlJoinType`, the join is `LeftOuterJoin`. Before subsequent filtering, it keeps every row of the upper data item, including rows with no matching record below; for those rows the lower data item's columns are null and are read in AL as blank or zero. Developers coming from SQL, where a plain `JOIN` is an inner join, can incorrectly count these unmatched rows as sales documents or ledger entries. + +The final dataset also depends on filters. A null-rejecting child filter applied in the SQL `WHERE` clause removes unmatched rows: for example, `DataItemTableFilter = "Document Type" = const(Order), Status = const(Open)` on Sales Header rejects the null child rows of customers without matching orders. In that case, changing only the join to `InnerJoin` does not fix a row-selection defect; both joins return the same rows. + +## Best Practice + +Prefer an explicit `SqlJoinType` to communicate intent. Use `InnerJoin` when only matched rows are meaningful. Keep `LeftOuterJoin` when unmatched upper rows are intended, for example to list customers with zero sales, and make the consuming code handle the blank lower columns. A null-rejecting child filter is also a valid way to exclude unmatched rows; evaluate the effective filters and consuming code before reporting a missing join type as a defect. + +See sample: [`query-dataitems-default-to-left-outer-join.good.al`](query-dataitems-default-to-left-outer-join.good.al). + +## Anti Pattern + +A query whose lower data item has a `DataItemLink` and no `SqlJoinType`, where unmatched rows survive the effective filters and are consumed as matches, counted as documents or entries, or read without handling blank lower values. Require evidence of a wrong result, not just a missing property or a suggestive query name. + +Do not flag an explicit `SqlJoinType = LeftOuterJoin`, an implicit one where the consumer clearly expects and handles unmatched rows, or a query whose null-rejecting child filters already eliminate unmatched rows. The clean companion includes both an explicit inner join and an implicit outer join constrained by such a filter. + +See sample: [`query-dataitems-default-to-left-outer-join.bad.al`](query-dataitems-default-to-left-outer-join.bad.al). + +## References + +[Linking and joining data items to define the query dataset](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-query-links-joins#how-to-link-and-join-data-items) (by default, SqlJoinType is LeftOuterJoin; unmatched rows get null values), [SqlJoinType property](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/properties/devenv-sqljointype-property), and [Filtering in Query objects](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-query-filters) (data-item filters restrict the resulting dataset). diff --git a/microsoft/knowledge/reporting/declare-report-layouts-in-the-rendering-section.bad.al b/microsoft/knowledge/reporting/declare-report-layouts-in-the-rendering-section.bad.al new file mode 100644 index 00000000..df332269 --- /dev/null +++ b/microsoft/knowledge/reporting/declare-report-layouts-in-the-rendering-section.bad.al @@ -0,0 +1,22 @@ +// Demonstration only. Shows the wrong pattern: a new report declares its layouts with the legacy properties. + +report 50223 "Orders By Salesperson Bad" +{ + UsageCategory = ReportsAndAnalysis; + ApplicationArea = All; + // BAD: legacy, type-based layout properties on a new report. + DefaultLayout = Word; + WordLayout = './layouts/OpenOrdersBySalesperson.docx'; + ExcelLayout = './layouts/OpenOrdersBySalesperson.xlsx'; + + dataset + { + dataitem(SalesHeader; "Sales Header") + { + DataItemTableView = where("Document Type" = const(Order)); + column(No; "No.") { } + column(SalespersonCode; "Salesperson Code") { } + column(Amount; Amount) { } + } + } +} diff --git a/microsoft/knowledge/reporting/declare-report-layouts-in-the-rendering-section.good.al b/microsoft/knowledge/reporting/declare-report-layouts-in-the-rendering-section.good.al new file mode 100644 index 00000000..9d4bd5d4 --- /dev/null +++ b/microsoft/knowledge/reporting/declare-report-layouts-in-the-rendering-section.good.al @@ -0,0 +1,37 @@ +// Demonstration only. Shows the recommended pattern: named layouts in the rendering section and a default chosen by name. + +report 50224 "Orders By Salesperson Good" +{ + UsageCategory = ReportsAndAnalysis; + ApplicationArea = All; + DefaultRenderingLayout = OpenOrdersWord; + + dataset + { + dataitem(SalesHeader; "Sales Header") + { + DataItemTableView = where("Document Type" = const(Order)); + column(No; "No.") { } + column(SalespersonCode; "Salesperson Code") { } + column(Amount; Amount) { } + } + } + + rendering + { + layout(OpenOrdersWord) + { + Type = Word; + LayoutFile = './layouts/OpenOrdersBySalesperson.docx'; + Caption = 'Open orders (Word)'; + Summary = 'Printable list of open sales orders per salesperson.'; + } + layout(OpenOrdersExcel) + { + Type = Excel; + LayoutFile = './layouts/OpenOrdersBySalesperson.xlsx'; + Caption = 'Open orders (Excel)'; + Summary = 'Open sales orders for analysis in Excel.'; + } + } +} diff --git a/microsoft/knowledge/reporting/declare-report-layouts-in-the-rendering-section.md b/microsoft/knowledge/reporting/declare-report-layouts-in-the-rendering-section.md new file mode 100644 index 00000000..365894d4 --- /dev/null +++ b/microsoft/knowledge/reporting/declare-report-layouts-in-the-rendering-section.md @@ -0,0 +1,34 @@ +--- +bc-version: [20..] +domain: reporting +keywords: [rendering, defaultrenderinglayout, defaultlayout, rdlclayout, wordlayout, excellayout, layoutfile, legacy-layout] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Declare report layouts in the rendering section, not with the legacy layout properties + +## Description + +Since runtime 9.0 (BC20), reports declare layouts in a `rendering` section of named `layout` entries, each with `Type`, `LayoutFile`, `Caption`, and `Summary`, and choose the default by name with `DefaultRenderingLayout`. The older properties `DefaultLayout`, `RDLCLayout`, `WordLayout`, and `ExcelLayout` allow only one layout per type, select the default by type, and Microsoft documents that they should be avoided because they will be deprecated. Code that reads them also loads the full layout into memory even when it only checks whether a layout of that type exists. + +Agents still produce the legacy properties because most published samples and older Base Application code use them. + +## Best Practice + +In new or changed reports, declare every layout in `rendering`, set `DefaultRenderingLayout` on the report, and give each layout a `Caption` and `Summary` so users can choose between them on the **Report Layouts** page. When touching a report that still uses the legacy properties, convert it with the AL code action **Convert to 'Rendering'**. + +See sample: [`declare-report-layouts-in-the-rendering-section.good.al`](declare-report-layouts-in-the-rendering-section.good.al). + +## Anti Pattern + +A new report, or a changed report's layout declaration, that uses `DefaultLayout` together with `RDLCLayout`, `WordLayout`, or `ExcelLayout`. + +Do not flag unchanged legacy declarations in code outside the review scope, or projects whose target runtime is below 9.0. + +See sample: [`declare-report-layouts-in-the-rendering-section.bad.al`](declare-report-layouts-in-the-rendering-section.bad.al). + +## References + +[DefaultLayout property](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/properties/devenv-defaultlayout-property) (avoid the legacy properties; they will be deprecated; reading them loads the full layout), [Declare report layouts in AL](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-report-layout-declaration), and [Defining multiple report layouts](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-multiple-report-layouts). diff --git a/microsoft/knowledge/reporting/do-not-rename-or-remove-shipped-report-dataset-columns.bad.al b/microsoft/knowledge/reporting/do-not-rename-or-remove-shipped-report-dataset-columns.bad.al new file mode 100644 index 00000000..0471e9c7 --- /dev/null +++ b/microsoft/knowledge/reporting/do-not-rename-or-remove-shipped-report-dataset-columns.bad.al @@ -0,0 +1,31 @@ +// Demonstration only. Shows the wrong pattern: a released report's dataset columns are renamed and removed. +// Version 1.0 of this report shipped with columns No_, Name_, Balance_LCY and Phone_No. + +report 50225 "Customer Balance Overview Bad" +{ + UsageCategory = ReportsAndAnalysis; + ApplicationArea = All; + DefaultRenderingLayout = BalanceExcel; + + dataset + { + dataitem(Customer; Customer) + { + // BAD: renamed from No_, Name_ and Balance_LCY for naming style. User-defined Excel and Word + // layouts created from version 1.0 still bind to the old names. + column(CustomerNo; "No.") { } + column(CustomerName; Name) { } + column(BalanceLCY; "Balance (LCY)") { } + // BAD: Phone_No was removed because the layout in this repository no longer uses it. + } + } + + rendering + { + layout(BalanceExcel) + { + Type = Excel; + LayoutFile = './layouts/CustomerBalanceOverview.xlsx'; + } + } +} diff --git a/microsoft/knowledge/reporting/do-not-rename-or-remove-shipped-report-dataset-columns.good.al b/microsoft/knowledge/reporting/do-not-rename-or-remove-shipped-report-dataset-columns.good.al new file mode 100644 index 00000000..f987eaa9 --- /dev/null +++ b/microsoft/knowledge/reporting/do-not-rename-or-remove-shipped-report-dataset-columns.good.al @@ -0,0 +1,32 @@ +// Demonstration only. Shows the recommended pattern: released column names stay, and new data is added as new columns. +// Version 1.0 of this report shipped with columns No_, Name_, Balance_LCY and Phone_No. + +report 50226 "Customer Balance Overview Good" +{ + UsageCategory = ReportsAndAnalysis; + ApplicationArea = All; + DefaultRenderingLayout = BalanceExcel; + + dataset + { + dataitem(Customer; Customer) + { + column(No_; "No.") { } + column(Name_; Name) { } + column(Balance_LCY; "Balance (LCY)") { } + // Kept for layouts outside this repository, although the shipped layout no longer shows it. + column(Phone_No; "Phone No.") { } + // New in version 2.0: added, not renamed. + column(Credit_Limit_LCY; "Credit Limit (LCY)") { } + } + } + + rendering + { + layout(BalanceExcel) + { + Type = Excel; + LayoutFile = './layouts/CustomerBalanceOverview.xlsx'; + } + } +} diff --git a/microsoft/knowledge/reporting/do-not-rename-or-remove-shipped-report-dataset-columns.md b/microsoft/knowledge/reporting/do-not-rename-or-remove-shipped-report-dataset-columns.md new file mode 100644 index 00000000..fed09742 --- /dev/null +++ b/microsoft/knowledge/reporting/do-not-rename-or-remove-shipped-report-dataset-columns.md @@ -0,0 +1,34 @@ +--- +bc-version: [all] +domain: reporting +keywords: [report-dataset, column, rename, remove, user-defined-layout, excel-layout, word-layout, data-contract, compatibility] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Do not rename or remove columns of a report dataset that has shipped + +## Description + +A report's dataset column names are the contract that every layout binds to, including layouts that are not in your source code. An Excel layout lists the column names it uses in its *Data* worksheet; these names must exist in the dataset and cannot be renamed. Word layouts bind to the dataset's custom XML part, and RDL layouts reference field names. Customers and partners create user-defined layouts from your report on the **Report Layouts** page, and other extensions can add layouts through report extensions. Renaming a `column(...)` or deleting it compiles cleanly and updates the layouts in your project, but breaks those other layouts: fields render empty, or the layout fails validation or rendering. + +Agents treat a column name like a private identifier and "tidy" names (`No_` to `DocumentNo`), or delete columns that their own layout no longer uses. + +## Best Practice + +Once a report has been released, keep existing column names and keep columns that layouts may use. Add new columns instead of renaming, and keep old ones populated (they can stay unused by your own layouts). If a column must change, ship a new report or a new layout-compatible column alongside the old one, and communicate it as a breaking change for custom layouts. + +See sample: [`do-not-rename-or-remove-shipped-report-dataset-columns.good.al`](do-not-rename-or-remove-shipped-report-dataset-columns.good.al). + +## Anti Pattern + +A diff that renames the name of an existing `column(Name; Source)` in a released report or report extension, or removes such a column, justified by naming style or by "no layout uses it" based only on the layouts in the repository. + +Changing a column's source expression while keeping its name and type is not this defect. Neither is changing a report that has never been released (see the unreleased-symbol guidance in breaking-changes). + +See sample: [`do-not-rename-or-remove-shipped-report-dataset-columns.bad.al`](do-not-rename-or-remove-shipped-report-dataset-columns.bad.al). + +## References + +[Creating an Excel layout report, How Excel layouts work](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-howto-excel-report-layout#how-excel-layouts-work) (Data worksheet fields must exist in the dataset and can't be renamed), [Report and document layouts overview](https://learn.microsoft.com/en-us/dynamics365/business-central/ui-manage-report-layouts#layout-sources) (user-defined layouts), and [Update custom report layouts](https://learn.microsoft.com/en-us/dynamics365/business-central/ui-update-report-layouts) (deleted fields can't be handled by the automatic layout update). diff --git a/microsoft/knowledge/reporting/reportextension-layouts-are-not-selected-automatically.bad.al b/microsoft/knowledge/reporting/reportextension-layouts-are-not-selected-automatically.bad.al new file mode 100644 index 00000000..3214220b --- /dev/null +++ b/microsoft/knowledge/reporting/reportextension-layouts-are-not-selected-automatically.bad.al @@ -0,0 +1,23 @@ +// Demonstration only. Shows the wrong pattern: the new layout is assumed to replace the base report's layout. + +reportextension 50220 "Customer List With Region Bad" extends "Customer - List" +{ + dataset + { + add(Customer) + { + column(Region_Code; "Territory Code") { } + } + } + + rendering + { + // BAD: the region column is shown only in this layout, and nothing selects it. After deployment + // the report keeps printing its own default layout, so users never see the region. + layout(CustomerListWithRegion) + { + Type = Word; + LayoutFile = './layouts/CustomerListWithRegion.docx'; + } + } +} diff --git a/microsoft/knowledge/reporting/reportextension-layouts-are-not-selected-automatically.good.al b/microsoft/knowledge/reporting/reportextension-layouts-are-not-selected-automatically.good.al new file mode 100644 index 00000000..ef9330f4 --- /dev/null +++ b/microsoft/knowledge/reporting/reportextension-layouts-are-not-selected-automatically.good.al @@ -0,0 +1,24 @@ +// Demonstration only. Shows the recommended pattern: the extension layout is an explicitly named, described option. +// Making it the company default is a documented setup step on the Report Layouts page, not an assumption. + +reportextension 50221 "Customer List With Region Good" extends "Customer - List" +{ + dataset + { + add(Customer) + { + column(Region_Code; "Territory Code") { } + } + } + + rendering + { + layout(CustomerListWithRegion) + { + Type = Word; + LayoutFile = './layouts/CustomerListWithRegion.docx'; + Caption = 'Customer list with region'; + Summary = 'Customer list including the territory. Choose it on the request page, or set it as default on the Report Layouts page.'; + } + } +} diff --git a/microsoft/knowledge/reporting/reportextension-layouts-are-not-selected-automatically.md b/microsoft/knowledge/reporting/reportextension-layouts-are-not-selected-automatically.md new file mode 100644 index 00000000..567c970d --- /dev/null +++ b/microsoft/knowledge/reporting/reportextension-layouts-are-not-selected-automatically.md @@ -0,0 +1,34 @@ +--- +bc-version: [20..] +domain: reporting +keywords: [reportextension, rendering, layout, defaultrenderinglayout, report-layout-selection, report-layouts, word-layout, excel-layout] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# A layout added in a report extension is not used until it is selected + +## Description + +A `reportextension` can add layouts through its `rendering` section, but deploying the extension does not make any of them the report's layout. `DefaultRenderingLayout` can be set only on the `report` object, not on a report extension, so the base report keeps its own default. The new layout appears on the **Report Layouts** page and in the request page's **Report Layout** field, and is used only when a user or administrator picks it, or sets it as the default for the company in **Report Layouts** or **Report Layout Selection**. + +Agents that add columns to a report extension together with a new layout often assume the new layout replaces the base one, and describe the change as complete. Users keep printing the old layout without the new fields. + +## Best Practice + +When a report extension ships a layout meant to be used by default, treat selecting it as a deployment or setup step: document it, or provide an explicit, opt-in setup action that sets the layout as the company default through the application's report layout selection. Give the layout a `Caption` and `Summary` so users can find it. If the extension's new dataset columns are only useful in the new layout, say so in the release notes. + +See sample: [`reportextension-layouts-are-not-selected-automatically.good.al`](reportextension-layouts-are-not-selected-automatically.good.al). + +## Anti Pattern + +A `reportextension` whose `rendering` layout is expected to replace the base layout automatically, for example a change described as "the report now shows X" when X appears only in the extension's layout, with no selection step. `DefaultRenderingLayout` placed in a report extension is a compile error, not a working override. + +Do not flag report extensions that add layouts as optional alternatives, or that add dataset columns only for user-defined layouts. + +See sample: [`reportextension-layouts-are-not-selected-automatically.bad.al`](reportextension-layouts-are-not-selected-automatically.bad.al). + +## References + +[Defining multiple report layouts](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-multiple-report-layouts) (`DefaultRenderingLayout` can't be set on report extension objects), [Report design overview](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-report-design-overview#report-layouts) (a report extension layout isn't used automatically when deployed), and [Set the layout used by a report](https://learn.microsoft.com/en-us/dynamics365/business-central/ui-set-report-layout). diff --git a/microsoft/skills/review/al-query-review.md b/microsoft/skills/review/al-query-review.md index e97a20ae..018cadc3 100644 --- a/microsoft/skills/review/al-query-review.md +++ b/microsoft/skills/review/al-query-review.md @@ -28,7 +28,7 @@ Return `not-applicable` when the input contains no Query object declaration and ## Worklist -Match relevant entries against changed `query` objects, variables typed as `Query`, and the tokens `QueryType`, `dataitem`, `column`, `DataItemLink`, `SqlJoinType`, `SetFilter`, `SetRange`, `Open`, `Read`, `Close`, and `Clear`. +Match relevant entries against changed `query` objects, variables typed as `Query`, and the tokens `QueryType`, `dataitem`, `column`, `DataItemLink`, `SqlJoinType`, `InnerJoin`, `LeftOuterJoin`, `Method`, `Sum`, `Count`, `Average`, `Min`, `Max`, `SetFilter`, `SetRange`, `Open`, `Read`, `Close`, and `Clear`. The following targeted checks cover every current `query` article: @@ -36,6 +36,8 @@ The following targeted checks cover every current `query` article: - `SetFilter` or `SetRange` occurs after `Open()` without a new `Open()` before the next `Read()` — `set-query-filters-before-open`. - A runtime `SetFilter` or `SetRange` replaces a `ColumnFilter` on the same column or filter row, while later code relies on the declarative restriction remaining effective — `setfilter-overwrites-query-columnfilter`. - An already-open query is opened again as if that advanced the cursor, or a query variable is reused for an independent operation without `Clear` even though old filters must not carry over — `reopening-query-resets-cursor-but-keeps-filters`. +- A lower `dataitem` declares `DataItemLink` without `SqlJoinType`, unmatched rows survive the effective filters, and the consuming code incorrectly counts them as documents/entries or uses blank lower columns as matches — `query-dataitems-default-to-left-outer-join`. Require an observable row-selection defect, not a name or missing property alone. Do not match an explicit `LeftOuterJoin`, an implicit one whose consumer visibly expects unmatched rows, or null-rejecting child filters in the resulting SQL `WHERE` clause that already exclude unmatched rows (for example Sales Header `Document Type = Order` and `Status = Open`). +- A query with an aggregate `Method` (`Sum`, `Count`, `Average`, `Min`, `Max`) also declares a non-aggregated `column` whose value varies within the intended group (document, line, or entry number; a date without a date method), while the name or consuming code expects one row per group — `every-non-aggregated-query-column-is-a-group-key`. Resolve layer conflicts per READ. When no query knowledge exists, emit `no-knowledge`; when knowledge exists but no article matches the changed Query usage, emit `completed` with no findings. diff --git a/microsoft/skills/review/al-reporting-review.md b/microsoft/skills/review/al-reporting-review.md index 52f8f537..cd0a4552 100644 --- a/microsoft/skills/review/al-reporting-review.md +++ b/microsoft/skills/review/al-reporting-review.md @@ -28,7 +28,7 @@ Return `not-applicable` when the input contains no Report or ReportExtension dec ## Worklist -Match relevant entries against changed `report` and `reportextension` objects, variables typed as `Report`, and the tokens `CurrReport`, `Skip`, `Break`, `Quit`, `Run`, `RunModal`, `RunRequestPage`, `Execute`, `Print`, `SaveAs`, `DownloadFromStream`, `Data Compression`, `SetTableView`, `DataItemTableView`, `OnPreReport`, `OnPostReport`, `OnPreDataItem`, `OnAfterGetRecord`, and report-extension dataset triggers. +Match relevant entries against changed `report` and `reportextension` objects, variables typed as `Report`, and the tokens `CurrReport`, `Skip`, `Break`, `Quit`, `Run`, `RunModal`, `RunRequestPage`, `Execute`, `Print`, `SaveAs`, `DownloadFromStream`, `Data Compression`, `SetTableView`, `DataItemTableView`, `OnPreReport`, `OnPostReport`, `OnPreDataItem`, `OnAfterGetRecord`, `rendering`, `layout`, `DefaultRenderingLayout`, `DefaultLayout`, `RDLCLayout`, `WordLayout`, `ExcelLayout`, `column`, and report-extension dataset triggers. Apply this targeted check even when token overlap would rank the article below the worklist cutoff: @@ -41,6 +41,9 @@ Apply this targeted check even when token overlap would rank the article below t - A ReportExtension `OnPreReport` prepares state consumed by the base `OnPreReport`, or its `OnPostReport` prepares state already consumed by the base `OnPostReport` — `reportextension-report-triggers-run-after-base-triggers`. Require visible base behavior or equivalent established evidence. - A report's `DataItemTableView` and a caller's `SetTableView` apply mutually exclusive filters to the same field — `settableview-cannot-broaden-dataitemtableview`. Require both views or equivalent direct evidence; `SetTableView` alone is not a finding. - The value returned by `Report.RunRequestPage()` reaches `Report.Execute`, `Report.Print`, or `Report.SaveAs` without an empty-string cancellation check — `stop-when-runrequestpage-returns-empty-parameters`. +- A new or changed `report` declares layouts with `DefaultLayout`, `RDLCLayout`, `WordLayout`, or `ExcelLayout` instead of a `rendering` section with `DefaultRenderingLayout` — `declare-report-layouts-in-the-rendering-section`. Do not match unchanged legacy declarations or a target runtime below 9.0. +- A `reportextension` adds a `rendering` layout that the change, its description, or its new dataset columns depend on being used by default, with no selection step — `reportextension-layouts-are-not-selected-automatically`. Do not match layouts added as optional alternatives. +- A diff renames the name of an existing `column(...)` of a released report or report extension, or removes such a column — `do-not-rename-or-remove-shipped-report-dataset-columns`. Requires comparison evidence (a diff or release history); a folder snapshot alone cannot show a rename. Do not match a changed source expression that keeps the column name and type, or a report that has never been released. Resolve layer conflicts per READ. When no reporting knowledge exists, emit `no-knowledge`; when knowledge exists but no article matches the changed report code, emit `completed` with no findings.