Skip to content

MSlenejennum/647452/new header and footer layouts and report themes - #10489

Open
MSlenejennum wants to merge 37 commits into
mainfrom
MSlenejennum/647452/New-Header-and-Footer-Layouts-and-Report-Themes
Open

MSlenejennum/647452/new header and footer layouts and report themes#10489
MSlenejennum wants to merge 37 commits into
mainfrom
MSlenejennum/647452/New-Header-and-Footer-Layouts-and-Report-Themes

Conversation

@MSlenejennum

@MSlenejennum MSlenejennum commented Aug 21, 2026

Copy link
Copy Markdown

AB#647452

Seed shipped header/footer and report theme parts on install and upgrade

Adds the out-of-box Composite Layout parts (11 header/footer designs, 3 themes) and the
install/upgrade pass that writes them into the shared pool under Tenant Report Defaults. The page
and lookup helper are already in main; this supplies their content.

New: codeunit 9667 "Composite Report Parts Mgt.", codeunit 104064 "Upgrade Composite Report Parts", 14 resources under .resources/ReportParts/.
Changed: BaseAppInstall seeds on install; UpgradeTagDefinitions (W1/BE/IT/RU) registers the
per-database tag; CompositeLayoutTests now 42 tests.

Behaviour

  • Parts are stored under this app's own App ID, resolved via NavApp.GetCurrentModuleInfo. That ID
    is part of the Tenant Report Layout key and of the <guid>::<name> reference every assignment
    stores.
  • Seeding deletes and re-inserts, because the platform will not let an existing layout's type and
    content be modified.
  • The pass is not gated by its upgrade tag — it runs on every upgrade and overwrites tenant edits
    to the shipped parts.
    Deliberate: these are app content, not tenant data. Copy a part to make a
    variant.
  • A part the app no longer ships is removed and its assignments cleared, so dropping a SeedPart
    call actually takes it out of the pool.
  • A part whose resource cannot be read is logged to telemetry and skipped, not raised — an uncaught
    error here would abort install or upgrade.

MSlenejennum and others added 2 commits August 21, 2026 09:53
…upgrade

Adds the reusable Composite Layout parts that ship with the Base Application -
11 header/footer designs and 3 report themes - and the code that writes them
into the shared pool under Tenant Report Defaults, so an administrator can
assign them to any report from the report themes and header/footer setup page.

- .resources/ReportParts: the shipped layout files
- Composite Report Parts Mgt. (9667): seeds the pool. Safe to call repeatedly,
  and each part is written in isolation so one that cannot be written is
  reported to telemetry and skipped rather than failing install or upgrade
- BaseApp Install: seeds per database on install
- Upgrade Composite Report Parts: seeds per database on upgrade, guarded by an
  upgrade tag that is only recorded when every part was written, so a partial
  seed is retried by the next upgrade
- Upgrade Tag Definitions in W1, BE, IT and RU: the tag and its per-database
  registration

Assigning the parts to report layouts is not part of this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@MSlenejennum MSlenejennum changed the title M slenejennum/647452/new header and footer layouts and report themes MSlenejennum/647452/new header and footer layouts and report themes Aug 21, 2026
@github-actions github-actions Bot added the Team: Integrations GitHub request for Integrations area label Aug 21, 2026
…ejennum/647452/New-Header-and-Footer-Layouts-and-Report-Themes
@github-actions github-actions Bot added this to the Version 29.0 milestone Aug 21, 2026
…ejennum/647452/New-Header-and-Footer-Layouts-and-Report-Themes

@MSlenejennum MSlenejennum left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Look through

@MSlenejennum
MSlenejennum marked this pull request as ready for review August 21, 2026 10:53
@MSlenejennum
MSlenejennum requested review from a team August 21, 2026 10:53
@MSlenejennum
MSlenejennum requested a review from a team as a code owner August 21, 2026 10:53
…ejennum/647452/New-Header-and-Footer-Layouts-and-Report-Themes
Comment thread src/Layers/W1/BaseApp/Foundation/Reporting/CompositeReportPartsMgt.Codeunit.al Outdated
@github-actions

Copy link
Copy Markdown
Contributor

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

The publisher-only detail event logs raw ErrorText after explicitly noting that platform error text can echo customer content, but the event is still classified as DataClassification::SystemMetadata. Reclassify that payload to a customer-data classification that matches the possible content, or stop emitting the raw error text and keep only sanitized metadata.

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/Report/CompositeLayoutTests.Codeunit.al
Comment thread src/Layers/W1/BaseApp/Foundation/Reporting/CompositeReportPartsMgt.Codeunit.al Outdated
Comment thread src/Layers/W1/BaseApp/Foundation/Reporting/CompositeReportPartsMgt.Codeunit.al Outdated
Comment thread src/Layers/W1/BaseApp/OtherCapabilities/Upgrade/UpgradeTagDefinitions.Codeunit.al Outdated
Comment thread src/Layers/W1/BaseApp/Foundation/Reporting/CompositeReportPartsMgt.Codeunit.al Outdated
@github-actions github-actions Bot removed the needs-approval Workflow runs require maintainer approval to start label Aug 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

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

The new shipped theme resource src/Layers/W1/BaseApp/.resources/ReportParts/ReportTheme/Playful.dotx is 5.46 MB, roughly 97x larger than its sibling themes Default.dotx and Calm.dotx (~56 KB each). Unzipping it shows a single word/media/image1.png accounting for 5.41 MB of the file, almost certainly an uncompressed or excessively high-resolution background image. Because CompositeReportPartsMgt.SeedDefaultParts seeds this resource into every tenant/company's Tenant Report Layout table on install and upgrade, the bloat is baked into every customer's database, inflates the app package size shipped to every tenant, and slows install/upgrade seeding. Re-export/compress the source image (e.g. downsample and use PNG/JPEG compression appropriate for a print/report background) before shipping, and add a size sanity check to the theme-authoring process.

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{🟠\ High\ Severity\ —\ Performance}$

ClearTestReportLayouts iterates Tenant Report Layout filtered only on "Report ID" to queue the primary key into a temporary buffer for later deletion, but it does not call SetLoadFields before FindSet(), so it still materializes every field of each matched layout row (including the large Layout blob) just to read the key. Add SetLoadFields("Report ID", Name) before FindSet() so the loop loads only the key fields it actually uses.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

        TenantReportLayout.SetLoadFields("Report ID", Name);
        TenantReportLayout.SetRange("Report ID", BodyReportID);
        if TenantReportLayout.FindSet() then

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

Comment thread src/Layers/W1/Tests/Report/CompositeLayoutTests.Codeunit.al
Comment thread src/Layers/W1/Tests/Report/CompositeLayoutTests.Codeunit.al
@github-actions

Copy link
Copy Markdown
Contributor

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

GetCompositeReportPartsUpgradeTag() (added to UpgradeTagDefinitions in W1/BE/IT/RU) is never added to the RegisterPerDatabaseTags list (the [EventSubscriber] on Codeunit::"Upgrade Tag", 'OnGetPerDatabaseUpgradeTags') in src/Layers/W1/BaseApp/OtherCapabilities/Upgrade/UpgradeTagDefinitions.Codeunit.al, unlike every other per-database upgrade tag defined in that file (e.g. GetNewISVPlansUpgradeTag, GetSharePointConnectionUpgradeTag, etc., all of which are registered). That registration list is what the platform uses to mark tags as already-applied for brand-new installations, so upgrade logic tied to them does not redundantly run for a tenant that never needed the migration. Because 'Upgrade Composite Report Parts' is Subtype = Upgrade with OnUpgradePerDatabase, omitting the registration is inconsistent with the established pattern for every sibling tag in this same file and should be reviewed for correctness (does the platform's automatic per-database tag pre-seeding on install rely on this list to avoid a redundant SeedShippedParts() re-run through a later upgrade pass?).

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\ —\ Performance}$

ClearTestReportLayouts iterates "Tenant Report Layout" without SetLoadFields before FindSet, so every field of every row is materialized even though only Name is read before the row is re-fetched for deletion.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

        TenantReportLayout.SetRange("Report ID", BodyReportID);
        TenantReportLayout.SetLoadFields(Name);
        if TenantReportLayout.FindSet() then

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

Co-authored-by: MSlenejennum <276599928+MSlenejennum@users.noreply.github.com>
@github-actions github-actions Bot added needs-approval Workflow runs require maintainer approval to start and removed needs-approval Workflow runs require maintainer approval to start labels Aug 24, 2026
Comment thread src/Layers/W1/Tests/Report/CompositeLayoutTests.Codeunit.al
Comment thread src/Layers/W1/Tests/Report/CompositeLayoutTests.Codeunit.al
Comment thread src/Layers/W1/Tests/Report/CompositeLayoutTests.Codeunit.al
Three defects behind the two failing composite layout tests.

ClearAssignments built its Report Layout List record and set Application
ID to the app the part was seeded under. An assignment references the
Application ID that Report Layout List reports, which is the empty GUID
for a part stored in Tenant Report Layout, so the key never matched and
nothing was cleared. It now looks the part up instead.

SeedPart relied on TryGetPartLayout to catch a missing resource, but
NavApp.GetResource raises an error a try function cannot catch, so
ResourceNotReadableErr was never reached. NavApp.ListResources decides
whether the resource is there before it is read.

The error stays ErrorType::Internal, so the message goes to telemetry
rather than to the user. Message is composed with StrSubstNo, which
telemetry replaces with a placeholder, so the part name and the resource
are carried in CustomDimensions where they survive.

SeedPartRaisesWhenTheResourceIsMissing no longer asserts on the message.
An Internal error shows a generic text and keeps the message out of
GetLastErrorText, so no assertion can reach it. That it raises, and that
no half-seeded row is left, is what stays observable.
@v-mortenr
Morten Rasmussen (v-mortenr) force-pushed the MSlenejennum/647452/New-Header-and-Footer-Layouts-and-Report-Themes branch from 1435351 to cef1145 Compare August 25, 2026 12:51
The error builder always appended GetLastErrorText, which reads as an
empty platform error on the path where the resource is simply not in the
app - there is no platform failure to report there. The builder now takes
the detail as a parameter, so a missing resource says so and an unreadable
one still carries the platform error.

Also collapses a double blank line left between two test methods.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team: Integrations GitHub request for Integrations area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants