Skip to content

621963 Deprecate table 9650 "Custom Report Layout" (Pending, tag 29.0) + country forks - #9695

Open
V-CWurtzen wants to merge 34 commits into
mainfrom
features/621963-deprecate-custom-report-layout-table
Open

621963 Deprecate table 9650 "Custom Report Layout" (Pending, tag 29.0) + country forks#9695
V-CWurtzen wants to merge 34 commits into
mainfrom
features/621963-deprecate-custom-report-layout-table

Conversation

@V-CWurtzen

@V-CWurtzen V-CWurtzen commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What & why

Deprecates table 9650 "Custom Report Layout" (ObsoleteState = Pending, tag 29.0), mirroring the
already-Pending page 9650. The legacy custom-layout table is superseded by the system report-layout
tables — "Tenant Report Layout" and "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 / CLEANSCHEMA step.

Design: the table stays, its legacy surface does not

Table 9650 is marked Pending but deliberately not wrapped in #if not CLEAN29. It has live schema
dependents — TableRelation, CalcFormula and tabledata permissions — that must keep resolving in the
Clean build; wrapping the table would turn those into errors no pragma can suppress.

The legacy surface that can go is obsoleted and compiled out of the Clean build: the layout-description
FlowFields that look up the table, page 9653 "Report Layouts Part" and its part on Report Layout
Selection
, and the consumer controls, triggers and CalcFields calls that fed them.

Where a construct leaves the Clean build it is marked obsolete rather than having the diagnostic
suppressed. The suppressions that remain are scoped to the referencing statement and sit inside the same
#if not CLEAN29 guard, so they retire together with the code they cover.

Scope

W1 BaseApp, Tests and DemoTool; the two CZ localization report-selection pages bound to a wrapped field;
and fork parity for every existing fork of each changed W1 file. Test layers are included because they
compile in the Clean build.

Metadata and reference-guarding only — obsoletion markers, #if not CLEAN29 guards and balanced pragmas.
No functional or behavioural change.

For reviewers

User-visible in the Clean build: the layout description columns on the report-selection setup pages, the
custom-layout column on Interact. Tmpl. Languages, 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.

Fixes AB#621963

@V-CWurtzen
V-CWurtzen requested review from a team July 23, 2026 12:57
@github-actions github-actions Bot added this to the Version 29.0 milestone Jul 23, 2026
@github-actions github-actions Bot added Team: Other GitHub request for other area than SCM, Finance or Integration Ownership: Needs Review Ownership is Other, low confidence, or needs manual correction labels Jul 23, 2026
…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
@V-CWurtzen
V-CWurtzen force-pushed the features/621963-deprecate-custom-report-layout-table branch from 711d21a to 793635e Compare August 3, 2026 17:03
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

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.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

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

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

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

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

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

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Upgrade}$

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.
@V-CWurtzen
V-CWurtzen requested a review from a team as a code owner August 4, 2026 12:46
@github-actions github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label Aug 4, 2026
…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 V-CWurtzen changed the title [621963] Deprecate table 9650 "Custom Report Layout" (Pending, tag 29.0) + country forks 621963 Deprecate table 9650 "Custom Report Layout" (Pending, tag 29.0) + country forks Aug 6, 2026

@darjoo Darrick (darjoo) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many files where the pragma warnings are just disabled, they should be surrounded with if not clean29 tags for removal as well.

…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.
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Data\ Modeling}$

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.
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Privacy}$

The changed Normal field "Custom Report Layout Code" still relies on the table-level DataClassification. Privacy guidance requires an explicit DataClassification on each changed Normal table field.

Knowledge:

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4

@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Privacy}$

The changed Normal fields "Custom Report Layout Code" and "Email Body Layout Code" still rely on the table-level DataClassification. Each changed Normal table field should declare its own DataClassification explicitly.

Knowledge:

The same issue exists in these regional copies — apply the equivalent fix in each:

  • src/Layers/RU/BaseApp/Foundation/Reporting/ReportSelections.Table.al:64 (RU)
    Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4

Comment thread src/Layers/W1/Tests/Job/JobQuoteReportTests.Codeunit.al Outdated
Comment thread src/Layers/W1/Tests/Misc/DocumentSendingPostTests.Codeunit.al Outdated
Comment thread src/Layers/W1/Tests/Misc/DocumentSendingPostTests.Codeunit.al Outdated
@V-CWurtzen

Copy link
Copy Markdown
Contributor Author

The round-1 note is now applied across the PR: every suppression is scoped to its own statement and sits inside the same #if not CLEAN29 guard, so it retires together with the code it covers. Where the construct could carry the obsoletion itself, the suppression is gone rather than moved — the removed page controls are marked ObsoleteState = Pending instead of suppressed, and AS0105 is dropped from the Base Application entirely since it only applies across an app boundary. Placement was measured rather than assumed: each file was compiled with its pragmas blanked to see which lines actually raise a diagnostic, which also removed pairs that raised nothing at all.

On the automated findings above, the recurring themes are the stored Code[20] layout fields, their data classification, and the migration path. Those fields deliberately stay: no schema is removed here and the data has to survive the migration, so they go with the schema drop in a later wave. The obsolete-tag split follows the documented three-version timeline — 29.0 on Pending, 32.0 on Removed. The findings about page 9650 and the upgrade codeunits are stale: the page is wrapped at CLEAN28, so it is absent from a CLEAN29 build altogether, and codeunits 5409, 104054 and 104057 each carry ObsoleteState = Pending with reason and tag.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.


🔄 How to trigger a new status check:

  1. 📤 Push a new commit to the PR branch, or
  2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes:

S1 - Preserve CLEAN29 coverage for migrated layout selection

Several CLEAN29 branches replace a real custom layout with an empty layout code. Tests such as Test Custom Reports and Rep. Selections Std. Stmt. therefore exercise the default layout while still claiming to verify customer-specific layouts. These tests can pass even if tenant-layout selection is broken. Please create a real Tenant Report Layout / Report Layout Selection fixture and assert that the selected non-default layout was used.

S2 - Port behavior tests instead of compiling them out

CLEAN29 disables coverage for Job Quote preview, document-layout email fallback and Sell-to/Bill-to precedence, service-order printing, and digital-voucher email behavior. These are user behaviors, not only table 9650 implementation details. Their legacy setup can be removed, but equivalent tests need to cover the replacement layout model.

This leaves high regression risk in report selection and document emailing even though the current checks pass, because the clean suite no longer exercises the scenarios that would detect those regressions.

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.


🔄 How to trigger a new status check:

  1. 📤 Push a new commit to the PR branch, or
  2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Upgrade}$

The refactor deprecates the Custom Report Layout path, but the upgrade step still clears the existing table 77 "Report Selections"."Custom Report Layout Code" value without copying the replacement selection into "Report Layout Name". Customers who had chosen one of these built-in custom layouts for a report's main (non-email-body) layout will silently lose that selection on upgrade and fall back to the default layout, with no data-migration path preserving their choice.

Knowledge:

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.38.6

#endif
end;
}
#if not CLEAN29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟠\ High\ Severity\ —\ Breaking\ Changes}$

Several shipped table fields are declared only inside #if not CLEAN29 while being newly marked ObsoleteState = Pending, so enabling CLEAN29 deletes them instead of progressing them through Pending -> Removed in a later release. Affected: table 9657 "Custom Report Selection" fields 8 and 22, table 77 "Report Selections" field 22 (both W1 and RU copies), table 5103 "Interaction Tmpl. Language" field 6, and table 9651 "Report Layout Selection" field 7. Published table fields must remain declared through the full Pending -> Removed lifecycle rather than disappearing directly behind a cleanup preprocessor symbol.

Knowledge:

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.38.6

field(21; "Email Body Layout Code"; Code[20])
{
Caption = 'Email Body Custom Layout Code';
#if not CLEAN29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Breaking\ Changes}$

Several fields that remain supported (not obsoleted) after this change lose their Custom Report Layout TableRelation when CLEAN29 is defined. For example, "Report Selections"."Email Body Layout Code" changes from a Custom-Report-Layout-or-HTML choice to an HTML-only relation under CLEAN29, and related code fields in tables 77, 9657, 9651, and 5103 also lose their TableRelation entirely behind CLEAN29 guards. That is a runtime contract change on surviving public fields with no obsolete window of their own; either obsolete those fields too or preserve the existing relation until the replacement field contract is complete.

Agent judgement — not directly backed by a BCQuality knowledge article.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.38.6

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Testing}$

CopyFromReportSelectionsCustomerEmailAttachment now seeds both Email Body Layout Name and Report Layout Name, but the verification helper still only checks the attachment layout name. That means the CLEAN29 migration path for copying the email-body layout can silently break while this test still passes.

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.38.6

else
CustomReportLayoutCode := CustomReportLayout.InitBuiltInLayout(ReportId, CustomReportLayout.Type::Word.AsInteger());
#else
CustomReportLayoutCode := '';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Testing}$

In the CLEAN29 branch these shared statement-test fixtures fall back to CustomReportLayoutCode := '' and then call LibrarySales.CreateCustomerDocumentLayout, whose helper only writes the obsolete custom-layout-code field. The resulting fixture no longer attaches any migrated report layout, so tests such as ...HasCustomDocLayout can pass without exercising the custom-document-layout branch they claim to cover.

Agent judgement — not directly backed by a BCQuality knowledge article.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.38.6

Assert.ExpectedError(ReportIDMustHaveValueErr);
end;

#if not CLEAN29

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$\textbf{🟡\ Medium\ Severity\ —\ Testing}$

UT_CleanEmailBodyLayoutCode_OnReportIdValidate is now compiled out for CLEAN29, but there is no replacement that checks the migrated Email Body/Attachment Layout Name and AppID fields when Report ID changes. The production table logic now clears those new fields too, so a regression can leave stale layout metadata bound to the wrong report without any CLEAN29 test catching it.

Agent judgement — not directly backed by a BCQuality knowledge article.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.38.6

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Upgrade}$

The upgrade path that translates legacy Custom Report Layout codes to the new name-based fields only writes the layout name before clearing the old code. In UpdateReportSelections and UpdateCustomReportSelection, the matching Application ID is not migrated into the paired AppID fields, even though the new schema uses name + AppID to identify a Report Layout List entry. Persist the resolved Application ID alongside each migrated layout name before clearing the legacy code so upgraded rows keep pointing at the exact layout they previously used.

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.38.6

…res/621963-deprecate-custom-report-layout-table
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Ownership: Needs Review Ownership is Other, low confidence, or needs manual correction Team: Other GitHub request for other area than SCM, Finance or Integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants