621963 Deprecate table 9650 "Custom Report Layout" (Pending, tag 29.0) + country forks - #9695
621963 Deprecate table 9650 "Custom Report Layout" (Pending, tag 29.0) + country forks#9695V-CWurtzen wants to merge 18 commits into
Conversation
github-actions
Bot
added
Other
…with CLEAN29 guards and country-fork parity Marks table 9650 as ObsoleteState Pending with ObsoleteTag 29.0; the functionality is superseded by the system tables Tenant Report Layout and Report Layout Selection. The table itself stays present in the Clean build because it has live schema dependents - TableRelation, CalcFormula and tabledata permissions - that must keep resolving, so plumbing references keep construct-scoped AL0432 pragmas. The legacy surface that can go is obsoleted and wrapped in #if not CLEAN29: the four layout description FlowFields (Report Selections 22, Custom Report Selection 8 and 22, Interaction Template Language 6) and page 9653 Report Layouts Part, with every consumer control, page trigger and CalcFields call guarded to match. Country-fork parity is applied across all 21 forks, plus the two CZ localization pages bound to the wrapped fields; the test layers are guarded as well, since they are compiled in the Clean build. Verified with a local alc compile of the worktree in both configurations with the CI analyzer set (CodeCop, AppSourceCop, PTECop, UICop): exit 0 for Default and for Clean with the full CLEAN symbol set. No schema is removed and there is no data-migration change - the table stays in the cloud-migration replication set. Fixes AB#621963
711d21a to
793635e
Compare
|
This PR marks table 9650 "Custom Report Layout" obsolete, but "Report Layout Selection" still models a live dependency on it through field 6's TableRelation and field 7's CalcFormula. Because "Report Layout Selection" is called out as the replacement surface, leaving these fields active keeps new selections coupled to the legacy table and makes the v29 data-model transition incomplete. Either obsolete these compatibility fields too or remap them to the tenant report layout model instead of continuing to reference the obsolete table. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.26.4 |
…layout surface CI round 2 was green in Default and red in all 21 Clean jobs on two AppSourceCop classes the Default build never applies. The Clean build sets obsoleteTagMinAllowedMajorMinor to 99.9 (build/scripts/GuardingV2ExtensionsHelper.psm1), which makes every surviving reference to a Pending object an AS0105 error regardless of AL0432 pragmas: the 62 live reference sites now suppress AL0432 and AS0105 together, following IntrastatJnlLine.Table.al. Suppressions inside CLEAN29 guards are deliberately left untouched, because the reference is compiled out there and the diagnostic cannot fire. The Clean build also validates breaking changes against the released baseline, which reports the CLEAN29-wrapped page controls as AS0032 - the seven report-selection setup pages, Customer and Vendor Report Selections, and the Custom Layouts part on Report Layout Selection. Those objects carry an object-scoped AS0032 suppression placed outside the guard, as in page 9075 RapidStart Services Activities, mirrored to the four country forks that wrap the same field. Only pragma lines change: 148 added, 124 replaced, every pair balanced, CRLF and BOM preserved. Pre-existing AL0432 pragmas guarding other obsoletions are untouched, so their Clean-build errors stay visible. The changed-file set is unchanged, so the fork gate verdict still holds. A local alc compile in the Clean configuration introduces no compiler errors; the AppSourceCop classes themselves are verifiable only in CI.
|
The Custom Report Layout obsoletion cleanup is incomplete: this PR adds AL0432/AS0105 suppressions for declarations and TableRelation targets, but many live statements still reference the newly obsolete table outside any active AL0432 disable block. For example, Validate("Email Body Layout Code", CustomReportLayout.Code); here, CustomReportLayout.SetRange/DeleteAll() in the CompanyInitialize subscribers, and many CustomReportLayout.* calls in the report-selection/report-layout tests remain unsuppressed. That leaves the codebase with obsolete-reference warnings after table 9650 is marked ObsoleteState = Pending; either extend the AL0432 suppression to the full statement blocks or finish migrating those paths off Custom Report Layout. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.26.4 |
|
Table 9651 "Report Layout Selection" still exposes non-obsolete FlowField "Report Layout Description" as lookup("Custom Report Layout".Description ...), even though the source table is now pending obsolete. Unlike the other description FlowFields changed in this PR, this one is neither wrapped in CLEAN29 removal guards nor marked obsolete, so removing table 9650 later will leave this field uncompilable. Obsolete this field on the same schedule or move it to the replacement report-layout source. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.26.4 |
|
This PR marks table 9650 "Custom Report Layout" obsolete, but several non-obsolete code fields still keep TableRelation dependencies on it (for example W1/RU Report Selections, Custom Report Selection, Report Layout Selection, and Interaction Tmpl. Language). The added AL0432 suppressions only hide the warning; they do not remove the schema dependency, so the CLEAN29 removal plan cannot complete cleanly until these fields are either obsoleted on the same schedule or redirected to the replacement report-layout model. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.26.4 |
|
After this PR marks table 9650 "Custom Report Layout" obsolete, several changed test codeunits still declare or accept Record "Custom Report Layout" outside both #if not CLEAN28/29 and local #pragma warning disable AL0432 guards. Examples include src/Layers/W1/Tests/Report/TestCustomReports.Codeunit.al:16, src/Layers/W1/Tests/Report/ReportLayoutTest.Codeunit.al:100, and src/Layers/APAC/Tests/Misc/ReportSelectionsTests.Codeunit.al:984, with the same pattern repeated in the localized clones. Because the PR already had to add AL0432 suppressions for this table elsewhere, these remaining direct references are inconsistent and can still fail warnings-as-errors builds; wrap the remaining declarations/signatures or move them behind the appropriate CLEAN guard. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.26.4 |
Clean round 3 left 16 AS0105 errors, all of them table 9650 referencing itself: the Permissions TableData line, five record variable declarations, and eleven event-subscriber parameters. These were the self-references left unguarded as an open convention call; the Clean build obsoleteTagMinAllowedMajorMinor gate settles it. Suppressed once at object scope rather than with 34 individual pragmas - an obsolete object referring to itself is expected, and the only other tag-gated references in the file point at page 9650 and already sit inside #if not CLEAN28, so nothing unrelated is masked. The table has no country forks. Two lines, byte-clean, fork set unchanged.
…ng the test projects Clean round 4 reduced AS0105 from 16 to 2. The BaseApp remainder was CopyCompany.Report.al, where the dataitem assignment sits below the pragma covering its variable declaration; the pragma now spans the whole OnAfterGetRecord trigger. Because a build job aborts at its first failing project, the test projects had never been reached: enumerating live references with the CLEAN mask and pragma ranges found 45 further sites in 13 files under W1/Tests, Sustainability tests and EnforcedDigitalVouchers tests. Those take one object-scoped suppression per file rather than 90 site-level pragma lines, since the tests exercise the obsolete table deliberately and are removed with it. Touching those W1 test files made the fork gate demand four country forks that were absent from the changelist - CH and RU PaymentServicesTest, ES ServiceOrders, RU MailServiceTest - so the same suppression is mirrored there; the fork set re-validates at 0 missing across 90 files. Known remainder, deliberately out of scope: about 90 further live sites in country Tests layers, which BCApps CI never compiles - they matter only to the internal NAV pipeline and are tracked as a follow-up.
…ayout Test Clean round 5 cleared AS0105 and AS0032 entirely and left one error: LibraryTablesUT is unused in Report Layout Test, because its only call site sits inside the CLEAN29 block added earlier, and CodeCop escalates AA0137 to an error in the Clean build. Suppressed at the declaration in W1 and the four country forks that carry the same shape, following the precedent in PurchPost and AzureOpenAIImpl. Five files, ten lines, pragmas balanced, fork set unchanged at 0 missing.
Round 6 was green on all 21 Clean jobs except CZ, the only job that compiles src/Apps/CZ. Both pages guard the Email Body Layout Description control behind CLEAN29, so the released baseline has a field the Clean binary lacks. Same object-scoped suppression outside the guard as the eight W1 pages and four country BaseApp forks. Two lines each, balanced, byte-clean, fork set unchanged at 90 files.
V-CWurtzen
changed the title
Darrick (darjoo)
left a comment
Darrick (darjoo)
left a comment
There was a problem hiding this comment.
There are many files where the pragma warnings are just disabled, they should be surrounded with if not clean29 tags for removal as well.
| Caption = 'Custom Report Layout'; | ||
| DataPerCompany = false; | ||
| ObsoleteReason = 'Replaced by system tables "Tenant Report Layout" and "Report Layout Selection". This table will be removed in a future version.'; | ||
| ObsoleteState = Pending; |
There was a problem hiding this comment.
Add
#if not CLEAN 29
ObsoleteState = Pending
#else
ObsoleteState = Removed
#endif
…nd CompanyInitialize sites First slice of the rework requested in review: pragma-only suppression replaced by CLEAN29 wrapping. Table 9650 now flips to ObsoleteState Removed in the else branch, which is the documented pattern for tables per Microsoft's deprecation guidelines, with the tag left stable outside the directive. Wraps 31 mechanical single-line sites - permission set tabledata lines, TableRelation properties, Database references and the CopyCompany dataitem - and the CompanyInitialize subscriber across W1 and ten country forks, where the declaration and its two statements are wrapped separately because that procedure also cleans up four unrelated tables and wrapping it whole would drop them from the Clean build. Wraps the InteractTmplLanguages custom-layout control together with its OnOpenPage probe and both page variables, which would otherwise be left unused, and adds the page-scoped AS0032 the disappearing control requires. NOT PUSHABLE as it stands: with the header reading Removed under CLEAN29, the fifteen remaining record-variable sites, six unclassified sites, one CalcFormula and roughly ninety country-Tests sites are still unwrapped and would be hard errors in the Clean build. Attachment GetCustomLayoutCodeLength is deliberately left on an unconditional pragma, commented in place, as a type-only reference whose wrapping would remove the HTML attachment path - raised for the reviewer.
Completes the pragma-to-wrap conversion the review asked for, on everything the product code owns. Whole-object wraps where the object exists only for the obsoleted table: the feature migration codeunit, both upgrade codeunits, and the two lookup procedures. Line-level wraps where the enclosing procedure also does unrelated work, so that work survives the Clean build. Three sites needed an else branch rather than a wrap, because the legacy call sits inside an if-else whose other branch must keep running: the demo data helper, the layout selection validate trigger, and the report layout selection page, which now fills its display variable straight from the modern sources. The public ApplyUpgradeToReports is marked Obsolete before its wrap, since a published member deprecates through the attribute rather than vanishing. Field 7 Report Layout Description is obsoleted and wrapped: it was only a staging buffer between the layout source and the displayed variable, and the control binds to the variable. Field 6 stays because it stores the selection. The cloud migration test keeps the table in the migration set by numeric id with the name in a comment, matching table 500 Deposits Page Setup, so the table still replicates without a compile-time reference. Attachment GetCustomLayoutCodeLength no longer touches the table at all: the number is the width of a code prefix already stored inside existing attachment blobs, so it comes from a Code[20] and the file needs no suppression. Two latent defects fixed along the way, neither of which produced a diagnostic: a guarded case label whose exit statement sat outside the guard, so in the Clean build it would have attached to the previous branch; and a guarded assignment that left TableNo at zero while the code below still ran. Verified: no live unsuppressed references under W1, Apps W1 or Tools; fork set 0 missing across 92 files; pragmas and directives balanced; byte clean.
|
UpgradeCustomReportImpl migrates legacy custom-layout references in Report Selections, Report Layout Selection, and Custom Report Selection, but it does not migrate table 5103 "Interaction Tmpl. Language". That table's "Custom Layout Code" field keeps its legacy value and its TableRelation to the now-obsoleted "Custom Report Layout" table is compiled out under CLEAN29 (see InteractionTmplLanguage.Table.al), while CreateHTMLCustomLayoutAttachment() still prefers "Custom Layout Code" over "Report Layout Name" when both are unset. Existing rows are left pointing at an obsoleted layout code with no upgrade path to the new Report Layout system. Add an upgrade step that resolves each row's legacy custom-layout code to "Report Layout Name"/"Report Layout AppID" and clears "Custom Layout Code", mirroring the migration already done for the other three tables. Agent judgement — not directly backed by a BCQuality knowledge article. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.31.4 |
…rphaned Five defects, all one root cause: the earlier mechanical pass guarded the line carrying the obsolete name rather than the construct that line belongs to. A dataitem opens a brace block, so closing the guard after its declaration left the body orphaned and the Clean configuration failed to parse, which produced most of a 230-error cascade. A case label owns the statement beneath it, so two labels were guarded while their exit and assignment stayed live, one of them a second instance of a shape already fixed once. And a using directive is owned by whatever still references the namespace: deleting the Attachment dependency left one unused in both configurations, which is what failed the previous round in Default, while wrapping controls and declarations left four more unused in Clean only. Verified locally before pushing rather than after: alc compiles the app in both configurations down to two pre-existing AL0126 errors in a file this branch never touched, caused by System symbols older than the CI artifact pin. A structural check reports zero guards that split a brace block. Fork set 0 missing across 92 files, directives and pragmas balanced, byte clean.
|
"TestCustomReports" in ES and NA has the same partial CLEAN29 cleanup problem: the PR wraps CustomerReportSelectionHandler, but src/Layers/ES/Tests/Report/TestCustomReports.Codeunit.al and src/Layers/NA/Tests/Report/TestCustomReports.Codeunit.al still retain other top-level Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.31.4 |
|
The version-upgrade path for retiring table 9650 "Custom Report Layout" is incomplete. The only Subtype = Upgrade migration codeunit (UpgradeCustomReportImpl.Codeunit.al) filters to Built-In = true before migrating, so tenant-created (Built-In = false) custom layouts are never migrated to the system replacement during upgrade. The only code touching Built-In = false layouts is FeatureReportSelection.codeunit.al, which is a manual/feature-data-update path, not an automatic upgrade codeunit, and it only rewrites Report Layout Selection rows, not the Custom Report Selection / Report Selections references. Customers can reach the CLEAN29 removal with live custom-layout data that was never automatically migrated, causing silent data/functionality loss. Knowledge:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.31.4 |
…rd errors With the table reading ObsoleteState Removed under CLEAN29, every surviving reference is an AL0433 that no pragma suppresses. Wraps the two dead library procedures in LibraryMarketing that CI round 12 flagged, and the twenty-one constructs in ReportLayoutTest that its ten references reach - seventeen test procedures and four helpers, because a shared setup helper is also called by fifteen live tests covering the new system layouts. Guarding only the reference there would have left its Custom RDLC and Custom Word branches assigning nothing, changing test behaviour instead of removing it. Verified with a Clean-mask reference scan and an orphan scan; directives balanced 13 of 13 and 2 of 2.
Sweeps the six test files where the obsolete table is genuinely incidental, chosen by measuring each file's transitive caller closure rather than its reference count: ServiceOrders, ERMRSProcessData, DigitalVouchersTests, DocumentSendingPostTests, JobQuoteReportTests and MailServiceTest all sit between zero and seventeen percent of their codeunit. In ERMRSProcessData the reference sits inside a shared Initialize with fifty-three callers, so its declaration and its DeleteAll statement are guarded separately and the procedure stays live, matching the CompanyInitialize pattern. Also mirrors the wraps into the ES and RU forks that the presence-only gate requires once their W1 originals are touched. Verified with a reference scan, an orphan scan that found and closed seven AA0137 declarations the wraps had orphaned, and a per-file directive balance.
|
In CLEAN29 builds, page 9650 "Custom Report Layouts" still declares the "Migrate to System Layouts" action, whose OnAction trigger instantiates Suggested fix (apply manually — could not be anchored as a one-click suggestion): #if not CLEAN29
action(MigrateToSystemLayouts)
{
ApplicationArea = Basic, Suite;
Caption = 'Migrate to System Layouts';
ToolTip = 'Migrate the selected custom report layouts to system layouts.';
trigger OnAction()
var
#pragma warning disable AL0432
CustomReportLayout: Record "Custom Report Layout";
FeatureReportSelection: Codeunit "Feature - Report Selection";
#pragma warning restore AL0432
begin
CustomReportLayout.Copy(Rec);
CurrPage.SetSelectionFilter(CustomReportLayout);
FeatureReportSelection.MigrateCustomReportLayouts(CustomReportLayout);
end;
}
#endifLine mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
…yzer wave round 14 masked Fixes the four round-14 diagnoses (AA0228 in JobQuoteReportTests, AL0792 in W1/ES ServiceOrders and DigitalVouchersTests) and closes the second wave those compile errors hid from the analyzers: two orphaned local procedures in DigitalVouchersTests found by the orphan gate, plus the global they alone used. Sweeps the four remaining public-matrix files: ReportSelectionsTests per-test with the vendor helper rewritten table-free, PaymentServicesTest keeping its shared Initialize path live with the layout plumbing guarded, TestCustomReports with dual-branch guards so the FullMod selections survive in Clean with empty layout codes rather than deleting thirteen live tests, and SustainabilityReportTests statement-level since its dataset assertions never depended on the layout. RepSelectionsStdStmt (5 refs) is deliberately deferred to the next push. Verified by pragma-blind reference scan, orphan scan, dead-using scan (three AL0792 found and guarded pre-push), directive balance, and a clean merge probe.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| end; | ||
|
|
||
| CustomLayoutDescription := Rec."Report Layout Description"; | ||
| #else |
There was a problem hiding this comment.
In the CLEAN29 branch, the displayed layout description on 'Report Layout Selection' is now always derived from tenant/default layouts, even when the stored selection still has Type = 'Custom Layout' and a legacy 'Custom Report Layout Code'. This makes the old invalid-custom-layout error path unreachable and silently masks stale data as a built-in selection instead of surfacing or normalizing the invalid state.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| field(21; "Email Body Layout Code"; Code[20]) | ||
| { | ||
| Caption = 'Email Body Custom Layout Code'; | ||
| #if not CLEAN29 |
There was a problem hiding this comment.
In CLEAN29 builds, 'Email Body Layout Code' on 'Report Selections' no longer validates against 'Custom Report Layout' (the TableRelation and CalcFields call are compiled out), but a non-empty legacy code is still treated as a configured email body. That lets stale/migrated legacy codes bypass the later 'you must select an email body or attachment' validation instead of failing fast or being cleared, so the send path can proceed referencing an obsolete layout. Consider clearing or rejecting a non-empty legacy 'Email Body Layout Code' once CLEAN29 is active.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| @@ -1,3 +1,4 @@ | |||
| #pragma warning disable AL0432, AS0105 | |||
There was a problem hiding this comment.
The PR wraps the newly-obsolete Custom Report Layout references in the Payment Services test clones by disabling warnings for the entire object with a file-scope '#pragma warning disable AL0432, AS0105' (opened right after the top-of-file using directives and closed only at the final '}') in src/Layers/W1/Tests/Misc/PaymentServicesTest.Codeunit.al, src/Layers/CH/Tests/Misc/PaymentServicesTest.Codeunit.al, and src/Layers/RU/Tests/Misc/PaymentServicesTest.Codeunit.al. In all three files the obsolete usage is confined to the report-selection helper routines near SetupReportSelections/GetCustomBodyLayout, so the file-scope suppression now also hides any future/unrelated AL0432 or AS0105 obsoletion diagnostics anywhere else in these ~2000-line test codeunits. Scope the pragma to just the obsolete record declarations/procedure bodies (as most other files in this PR correctly do) instead of blanketing the whole object.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
…-layer sweep Fixes the AA0205 class the new-warnings gate surfaced (the vendor helper now passes an empty literal in W1 and its four forks, the Clean-only else-branches assign explicitly, and CreateInteractTemplLang's else-branch gets the assignment that would have failed the first Clean warnings compare), reverts the Sustainability using-guard whose namespace the live enum references still need, wraps the CI-confirmed masked orphan in DocumentSendingPostTests, and completes RepSelectionsStdStmt statement-level so no test is deleted. Mirrors every W1 treatment into the country test layers (ReportSelectionsTests, ReportLayoutTest with its orphaned helper, TestCustomReports, PaymentServicesTest) and sweeps the two US-local files, leaving zero unguarded references to the removed table in any layer.
|
Table 9650 "Custom Report Layout" is now marked obsolete, but the public "Custom Report Layout Code"-family fields that AppSource extensions commonly consume remain non-obsolete on tables such as "Report Selections", "Report Layout Selection", and "Custom Report Selection". That leaves ISV extensions with no deprecation signal on the surface they actually reference, so they can keep taking new dependencies on a path that will eventually break once the legacy layout table is fully removed. Mark those surviving code fields obsolete as well, or clearly publish the supported replacement contract now. Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
|
This PR introduces the CLEAN29 preprocessor gate for table 9650 "Custom Report Layout" and its dependents for the first time, but the CLEAN29 cleanup is incomplete: VerifySalesInvoiceCustomReportNotRevertsBackToRdlc (W1/APAC/CZ/ES/RU Report Selections Tests clones) is still only guarded by the older #if not CLEAN28 while it declares and uses Record "Custom Report Layout" directly. Once a future CLEAN29 baseline removes table 9650, these tests stop compiling. Guard this test with #if not CLEAN29 (or rewrite it against the supported layout-selection API) consistently across all clones. Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
| InsertReportSelections( | ||
| ReportSelections, GetStandardStatementReportID(), false, false, '', ReportSelections.Usage::"C.Statement"); | ||
|
|
||
| #if not CLEAN29 |
There was a problem hiding this comment.
The newly-added CLEAN29 branches in the customer-statement test fixtures replace the custom layout setup with CustomReportLayoutCode := '' instead of building a real CLEAN29 layout fixture, so on BC29+ these tests no longer configure any customer-specific report layout. Tests intended to exercise custom document layout behavior now silently exercise the default-layout path instead, so a regression in layout selection could pass unnoticed. Build a real CLEAN29 report-layout fixture (e.g. via Report Layout Selection / tenant layout setup) or add an assertion proving a non-default layout was chosen.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| LibrarySales.CreateCustomer(CustomerPartialMod); | ||
| LibrarySales.CreateCustomer(CustomerNoMod); | ||
| Clear(CustomReportSelection); | ||
| #if not CLEAN29 |
There was a problem hiding this comment.
In Initialize(), the newly-added CLEAN29 path deletes all Report Layout Selection rows and seeds every "full mod" customer report selection via AssignCustomLayoutToCustomer(..., '') — an empty layout code. That collapses the intended custom-layout fixtures back to the default-layout path, so TestPrintQuotes, TestPrintOrders, TestPrintInvoices, and TestPrintCreditMemos no longer prove that per-customer custom layouts are resolved on BC29+ (the same pattern is copied into the ES and NA variants). Replace the blank layout code with a real CLEAN29 layout fixture, or assert that a non-default layout was selected.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
|
UpdateReportSelections() (invoked from the now Pending/CLEAN29-gated Upgrade Custom Report Impl. codeunit) clears the legacy "Custom Report Layout Code" on Report Selections rows but never writes a replacement report-layout name/AppID. This migration logic is unchanged by this PR, but the PR is what schedules the surrounding codeunit and table 9650 for permanent removal at CLEAN29, so this is the last opportunity to fix the gap before the migration path disappears: tenants that selected a built-in attachment layout will silently lose that selection once the legacy table is gone. Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
|
UpdateReportLayoutSelection() migrates a shared "Report Layout Selection" table (DataPerCompany = false, keyed by "Company Name") but writes every migrated "Tenant Report Layout Selection" row with CompanyName() — the executing company — instead of the source row's own "Company Name" field, and then clears the legacy code across all matching rows regardless of company. If this upgrade codeunit runs per company, running it in Company A will copy every other company's layout selections into Company A and blank the legacy code for all of them. Unchanged by this diff, but this PR is what locks the codeunit behind CLEAN29/ObsoleteState=Pending, making this the last release where the bug can be fixed before the code is gone. Suggested fix (apply manually — could not be anchored as a one-click suggestion): ReportLayoutSelection.SetRange("Custom Report Layout Code", CustomReportLayoutCode);
if ReportLayoutSelection.FindSet(true) then
repeat
TenantReportLayoutSelection."App ID" := ReportLayoutList."Application ID";
TenantReportLayoutSelection."Company Name" := ReportLayoutSelection."Company Name";
TenantReportLayoutSelection."Layout Name" := ReportLayoutList.Name;
TenantReportLayoutSelection."Report ID" := ReportLayoutSelection."Report ID";
TenantReportLayoutSelection."User ID" := EmptyGuid;
if not TenantReportLayoutSelection.Insert(true) then;
until ReportLayoutSelection.Next() = 0;
ReportLayoutSelection.ModifyAll("Custom Report Layout Code", '');Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
…ass orphaned Round 16 cleared every error family from round 15 and left exactly one diagnostic: AA0137 on the Usage option global in Report Layout Test, whose four uses were all wrapped in round 13 - invisible until this round because Tests-Report had never compiled far enough for its analyzers to run. Wraps the declaration in W1 and mirrors it to the NA, APAC, BE and FI forks, which the public matrix never checks but carry the same orphan for the internal pipeline.
| } No newline at end of file | ||
| } | ||
| #endif | ||
| #pragma warning restore AS0088 No newline at end of file |
There was a problem hiding this comment.
Four files touched by this PR end without a trailing newline ('\ No newline at end of file' in the diff), and in each case the final line added by this PR is a #pragma warning restore directive. This violates the standard convention that every text file end with a newline, and the PR already modifies the final lines of these files. Affected files: src/Layers/W1/BaseApp/OtherCapabilities/Upgrade/UpgradeCustomReportImpl.Codeunit.al (final line #pragma warning restore AS0088), src/Layers/W1/BaseApp/OtherCapabilities/Upgrade/UpgradeCustomReportLayouts.Codeunit.al (final line #pragma warning restore AS0088), src/Layers/W1/Tests/Report/ReportLayoutTest.Codeunit.al (final line #pragma warning restore AL0432, AS0105), src/Apps/W1/Sustainability/test/src/SustainabilityReportTests.Codeunit.al (final line #pragma warning restore AL0432, AS0105).
Recommendation:
- append a trailing newline after the final line in each file.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| { | ||
| Caption = 'Custom Report Layout Code'; | ||
| Editable = false; | ||
| #if not CLEAN29 |
There was a problem hiding this comment.
In tables 77 "Report Selections", 9651 "Report Layout Selection", and 9657 "Custom Report Selection", the stored Code[20] fields that reference the now-deprecated "Custom Report Layout" table (field 18/7 "Custom Report Layout Code", field 21 "Email Body Layout Code" in table 77; field 6 "Custom Report Layout Code" in table 9651; fields 7 and 21 in table 9657) have their TableRelation guarded by #if not CLEAN29 but are not themselves marked with ObsoleteState/ObsoleteReason/ObsoleteTag. Meanwhile the sibling FlowField description fields on the same tables (e.g. field 22 "Email Body Layout Description" in table 77, field 7 "Report Layout Description" in table 9651, fields 8 and 22 in table 9657) do carry the full ObsoleteState = Pending / ObsoleteReason / ObsoleteTag = '29.0' triad. This is an inconsistency within the same PR: consumers of the stored Code fields get no deprecation signal even though those codes become semantically meaningless once the referenced table is removed under CLEAN29.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
|
Table 5103 "Interaction Tmpl. Language" field "Custom Layout Code" is the legacy layout selector being replaced by the newer layout-name/app-id model, but it keeps shipping as a non-obsolete field even though this PR removes its legacy table relation behind CLEAN29 and the upgrade code moves/clears its value. Mark it ObsoleteState = Pending now so dependent extensions get a warning window before any later removal, consistent with the same pattern applied to tables 77, 9657, and 9651. Knowledge: Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
| @@ -1,3 +1,4 @@ | |||
| #pragma warning disable AL0432, AS0105 | |||
There was a problem hiding this comment.
The new #pragma warning disable AL0432, AS0105 at the top of this test codeunit suppresses obsolete-reference diagnostics for the entire file, while the PR only introduces a few #if not CLEAN29 islands. Scope the pragma to the smallest affected declaration/procedure or to each CLEAN29 block so unrelated obsolete references added later in the file still surface.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| @@ -1,3 +1,4 @@ | |||
| #pragma warning disable AL0432, AS0105 | |||
There was a problem hiding this comment.
The new file-level #pragma warning disable AL0432, AS0105 hides obsolete-reference warnings across the whole test codeunit even though the obsolete code is confined to scattered #if not CLEAN29 sections. Tighten the suppression to the affected members or individual CLEAN29 blocks to keep future warning coverage intact.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| @@ -1,3 +1,4 @@ | |||
| #pragma warning disable AL0432, AS0105 | |||
There was a problem hiding this comment.
This PR adds #pragma warning disable AL0432, AS0105 for the entire Test Custom Reports codeunit, but the obsolete references are limited to specific CLEAN29-gated variables, tests, and helpers. Narrow the suppression to those local regions so the rest of the file keeps reporting new obsolete references normally.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| using System.Security.User; | ||
| using System.TestLibraries.Utilities; | ||
|
|
||
| #pragma warning disable AL0432, AS0105 |
There was a problem hiding this comment.
The added #pragma warning disable AL0432, AS0105 at line 60 suppresses obsolete-reference diagnostics for almost the entire Service Orders test codeunit, although only a couple of CLEAN29-gated sections actually need it. Keep the suppression adjacent to the guarded members instead of making it file-wide, otherwise unrelated obsolete references in the rest of the file will be masked.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| CreateSalesInvoice(SalesHeader, Customer[1]); | ||
| LibrarySales.PostSalesDocument(SalesHeader, true, true); | ||
|
|
||
| #if not CLEAN29 |
There was a problem hiding this comment.
PrintCustomerStatementWhenCustomer...HasCustomDocLayout still claims to cover a customer with a custom document layout, but the CLEAN29 branch now sets CustomReportLayoutCode := '' and does not create any tenant report layout selection. That weakens the scenario to a default-layout case, so it no longer proves the filtered statement run behaves correctly when a tenant-specific layout is present. Create/select a tenant layout in the CLEAN29 path or rename the test to match the narrower behavior.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| DATABASE::Customer, CustomerFullMod."No.", CustomReportSelection.Usage::"S.Quote", REPORT::"Standard Sales - Quote", | ||
| CustomReportLayout.Code); | ||
| #else | ||
| AssignCustomLayoutToCustomer( |
There was a problem hiding this comment.
The CLEAN29 branch replaces every custom-layout setup with AssignCustomLayoutToCustomer(..., ''), so the suite still runs the quote/order/invoice/statement print scenarios without creating or selecting any tenant report layout. Those tests can now pass while the new tenant-layout selection path is broken, because they only exercise report selection with the default layout. Add CLEAN29 setup that creates/selects a tenant report layout, or split these into explicit default-layout tests.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
| field(7; "Custom Report Layout Code"; Code[20]) | ||
| { | ||
| Caption = 'Custom Report Layout Code'; | ||
| #if not CLEAN29 |
There was a problem hiding this comment.
Table 9657 "Custom Report Selection" has the same gap: "Custom Report Layout Code" and "Email Body Layout Code" are clearly part of the retired custom-layout path, but only the related description FlowFields get Pending/Reason/Tag metadata. Per the obsoletion guidance, fields slated for removal should carry deprecation metadata before removal so dependent extensions get a warning and replacement guidance.
Knowledge:
- microsoft/knowledge/upgrade/obsoletion-requires-reason-and-tag.md
- microsoft/knowledge/upgrade/obsolete-pending-to-removed-staging.md
- microsoft/knowledge/breaking-changes/obsolete-table-fields-instead-of-deleting-them.md
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4
|
In table 9651 "Report Layout Selection", field 6 "Custom Report Layout Code" is still the legacy selector being phased out, but only field 7 "Report Layout Description" is marked obsolete. The migration code clears field 6 after moving selections to the new layout mechanism, so keeping field 6 live without Pending/Reason/Tag metadata leaves callers on an undocumented legacy surface. Knowledge:
Line mapping was unavailable, so this was posted as an issue comment. 👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
|
Table 77 "Report Selections" still treats "Custom Report Layout Code" and "Email Body Layout Code" as the legacy custom-layout surface: their old TableRelation logic is now CLEAN29-gated, and the upgrade path migrates data away from them, but neither code field is marked ObsoleteState = Pending with a reason/tag the way the sibling description FlowFields are. That leaves extension consumers with no deprecation metadata for fields this PR is retiring. Knowledge:
The same issue exists in these regional copies — apply the equivalent fix in each:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.32.4 |
2 participants
What & why
Deprecates table 9650 "Custom Report Layout" (
ObsoleteState = Pending,ObsoleteTag = '29.0'),mirroring the already-Pending page 9650. The legacy custom-layout table is superseded by the system
report-layout tables ("Tenant Report Layout" / "Report Layout Selection", surfaced by the system
Report Layouts page). Marking it Pending starts the retirement clock and points partners at the modern
surface. No schema is removed and no data-migration behaviour changes — the table stays defined and
present, and remains in the cloud-migration replication set; physical removal is a later
Removed/CLEANSCHEMAstep.Design: the table stays present, its legacy surface does not
Table 9650 is marked Pending but is deliberately not wrapped in
#if not CLEAN29. It has live schemadependents —
TableRelation,CalcFormulaandtabledatapermissions — that must keep resolving in theClean build; wrapping the table would turn those into
AL0118errors, which no pragma can suppress.The legacy surface that can go is obsoleted and compiled out of the Clean build:
Report Selection "Custom Report Description" and "Email Body Layout Description", Interaction Tmpl.
Language "Custom Layout Description" — each
ObsoleteState = Pending(tag 29.0) and#if not CLEAN29-wrapped;page guarded;
CalcFieldscall that fed them.Report Layout Selection field 7 is left present on purpose: it is assigned from the modern
Tenant Report Layout Selection, so obsoleting it would break the modern page.Satisfying the Clean-build gates
The Clean configuration applies three checks the Default build does not, none of them plain compiler
diagnostics. Each is handled with the pattern already established in this repo:
AS0105obsoleteTagMinAllowedMajorMinor = 99.9, so any surviving reference to a Pending object is an error —AL0432pragmas do not apply to itAL0432, AS0105together; the table's references to itself are covered by one object-scoped pragmaIntrastatJnlLine.Table.alAS0032AA0137PurchPost,AzureOpenAIImplSuppressions inside
#if not CLEAN29blocks are deliberately left untouched — the reference is compiledout there, so the diagnostic cannot fire and a pragma would be noise. Pre-existing
AL0432pragmasguarding other obsoletions are untouched, so their Clean-build errors stay visible.
Scope
90 files. W1: BaseApp, Tests and DemoTool.
src/Apps/CZ: the two localization report-selectionpages bound to a wrapped field. Country forks (21): parity for every existing fork of each changed W1
file, applied surgically at each fork's own lines —
CompanyInitialize(AT/BE/CH/DACH/ES/FI/NA/NL/NO/RU),DemoTool/CreateRapidStartPackage(BE/CZ/IN/RU),d365busfullaccess(GB/RU),ReportSelectionPurchase/ReportSelectionSalespages (ES/RU, NO/RU), and RUReportSelections.Table.al(full mirror). Test layers are included because they are compiled in the Clean build.
Metadata and reference-guarding only: obsoletion markers,
#if not CLEAN29guards and balanced pragmas.No functional or behavioural change.
Validation
Verify App Changes(fork gate) green.the change touched a W1 file not previously in the set.
AppSourceCop.jsonand thereleased baseline are not reproducible locally — so they were driven from the job logs rather than a
local run.
For reviewers
User-visible in the Clean build: the layout description columns on the report-selection setup pages
(Sales / Purchase / Service / Job / Reminder / Customer / Vendor) and the Custom Layouts part on
Report Layout Selection disappear. The functionality is superseded by the system tables, but it is a
visible change and worth acknowledging as such.
Two convention calls I would rather have ruled than guess:
CLEANSCHEMA32wrap +Pending → Removedflip on the table. The deprecation guidelines prescribeit and 75 of 77 shipped Pending tables in
src/carry it; this one would be the exception. Adopt now,or as a follow-up story?
tabledatareferences. Precedent guards them with#if not CLEAN29, but thatprecedent is for tables compiled out. Since 9650 stays present, guarding would withdraw a permission
for a table that still exists. I found no shipped analogue.
Known remainder, deliberately out of scope: roughly 90 further live
AS0105sites in country Testslayers. BCApps CI never compiles those layers so they cannot affect this PR, but the internal pipeline will
see them — tracked as a follow-up.
Fixes AB#621963