MSlenejennum/647452/new header and footer layouts and report themes - #10489
MSlenejennum/647452/new header and footer layouts and report themes#10489MSlenejennum wants to merge 9 commits into
Conversation
…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
changed the title
…ejennum/647452/New-Header-and-Footer-Layouts-and-Report-Themes
…ejennum/647452/New-Header-and-Footer-Layouts-and-Report-Themes
…ejennum/647452/New-Header-and-Footer-Layouts-and-Report-Themes
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| codeunit 104064 "Upgrade Composite Report Parts" | ||
| { | ||
| Subtype = Upgrade; | ||
| InherentEntitlements = X; | ||
| InherentPermissions = X; |
There was a problem hiding this comment.
Codeunit 104064 "Upgrade Composite Report Parts" is a new upgrade implementation detail but is left at the default public object access. That publishes the object identity to dependent extensions unnecessarily; set Access = Internal unless other apps are expected to bind to this codeunit.
| codeunit 104064 "Upgrade Composite Report Parts" | |
| { | |
| Subtype = Upgrade; | |
| InherentEntitlements = X; | |
| InherentPermissions = X; | |
| codeunit 104064 "Upgrade Composite Report Parts" | |
| { | |
| Access = Internal; | |
| Subtype = Upgrade; | |
| InherentEntitlements = X; | |
| InherentPermissions = X; |
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| CopyStream(PartLayoutOutStream, ResourceInStream); | ||
| end; | ||
|
|
||
| local procedure UpsertPart(PartName: Text[250]; var PartLayout: Codeunit "Temp Blob"; Subtype: Enum "Report Layout Subtype"; Description: Text) |
There was a problem hiding this comment.
SeedPart only converts TryGetPartLayout read failures into false. The write path still calls RemovePart/Delete(true) and TenantReportLayout.Insert(true) outside any handled try path, so a write-time failure will raise an uncaught error and abort install or upgrade instead of logging the part, returning false, and leaving the upgrade tag unset for a later retry. Wrap the write phase in handled error-catching too and make SeedPart log and return false for write failures, not just missing-resource failures.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
|
The publisher-only detail event logs raw 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 |
| ReportLayoutList.SetRange(Name, CopyStr(PartName, 1, MaxStrLen(ReportLayoutList.Name))); | ||
| exit(ReportLayoutList.Count()); | ||
| end; | ||
|
|
There was a problem hiding this comment.
The new upgrade-tag scenarios mutate a per-database upgrade tag, but this non-isolated suite only resets layout rows and the shipped-part pool in Initialize(). That leaves CompositeReportPartsUpgradeTag in whatever state the previous test produced, so later tests or suites sharing the same database can become order-dependent. Restore or clear the tag as part of suite setup/cleanup, the same way the shared part pool is restored.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| exit('MS-629001-ProdDefinitionDisplaySetupUpgradeTag-20260723'); | ||
| end; | ||
| } No newline at end of file | ||
|
|
There was a problem hiding this comment.
The new upgrade-tag getter GetCompositeReportPartsUpgradeTag is duplicated across the BE, IT, RU, and W1 layer copies of "Upgrade Tag Definitions" (codeunit 9998), each hard-coding its own literal tag string. BE, IT, and RU all return the identical literal MS-643211-CompositeReportPartsUpgradeTag-20260820, but the W1 layer returns a different literal, MS-647452-CompositeReportPartsUpgradeTag-20260820. The referenced guidance requires keeping a tag definition shared so every path uses the exact same value; a divergent literal in one of several otherwise-identical per-layer copies is a strong signal of a copy-paste/PR-number mismatch rather than an intentional difference, and makes the tag's provenance (which work item it corresponds to) inconsistent across layers that are meant to mirror each other.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
internal procedure GetCompositeReportPartsUpgradeTag(): Code[250]
begin
exit('MS-643211-CompositeReportPartsUpgradeTag-20260820');
end;Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| { | ||
| Access = Internal; | ||
|
|
||
| procedure SeedDefaultParts() AllPartsSeeded: Boolean |
There was a problem hiding this comment.
SeedDefaultParts is declared with no access modifier (implicitly public) in the new codeunit "Composite Report Parts Mgt.". Although the codeunit itself is Access = Internal (so it is not currently reachable from other extensions), the procedure's own modifier should be set deliberately (internal) rather than left at the implicit public default, per the cited guidance on choosing access modifiers intentionally rather than accepting AL's public-by-default behavior.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
internal procedure SeedDefaultParts() AllPartsSeeded: BooleanKnowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| TenantReportLayout.Description := CopyStr(Description, 1, MaxStrLen(TenantReportLayout.Description)); | ||
| TenantReportLayout."Layout Status" := TenantReportLayout."Layout Status"::Approved; | ||
| TenantReportLayout."MIME Type" := PartMimeType(Subtype); | ||
| PartLayout.CreateInStream(LayoutInStream); |
There was a problem hiding this comment.
SeedPart reads the shipped .docx/.dotx resource inside a TryFunction, but the subsequent TenantReportLayout.Layout.ImportStream(...)/Insert(true) write path in UpsertPart runs outside any try wrapper. A single malformed or unsupported shipped layout resource can therefore raise an unhandled error during OnInstallAppPerDatabase or the upgrade codeunit, aborting the whole install/upgrade operation instead of being logged, skipped, and retried on a later run the way the rest of this best-effort seeding flow is designed to behave. Consider isolating the write/import step so one failing part cannot abort seeding of the remaining parts.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| PerDatabaseUpgradeTags.Add(GetBCUserGroupUpgradeTag()); | ||
| PerDatabaseUpgradeTags.Add(GetRenderWordReportsInPlatformFeatureKeyUpgradeTag()); | ||
| PerDatabaseUpgradeTags.Add(GetRegisterBankAccRecCopilotCapabilityUpgradeTag()); | ||
| PerDatabaseUpgradeTags.Add(GetCompositeReportPartsUpgradeTag()); |
There was a problem hiding this comment.
Registering GetCompositeReportPartsUpgradeTag() in the OnGetPerDatabaseUpgradeTags subscriber (added identically in the W1, BE, IT, and RU Upgrade Tag Definitions codeunits) breaks the retry contract this feature depends on. SetAllUpgradeTags (invoked by Company Initialize whenever any new company is created) calls OnGetPerDatabaseUpgradeTags and marks every returned tag complete if missing, regardless of whether SeedDefaultParts actually succeeded. If BaseApp Install's SeedDefaultReportParts() or the Upgrade Composite Report Parts upgrade codeunit fails to seed all parts (SeedDefaultParts returns false, so the tag is never explicitly set), the very next company creation on that database will mark the tag complete anyway via EnsurePerDatabaseUpgradeTagsExist. From that point on, RunUpgrade() short-circuits on HasDatabaseUpgradeTag(...) and the missing shipped report parts can never be retried by the upgrade codeunit. Keep the explicit SetDatabaseUpgradeTag() call after a successful seed, but do not also register this tag for OnGetPerDatabaseUpgradeTags while failed seeding is meant to remain retryable.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
1 participant
What & why
Composite Layouts compose a report from a theme + header/footer + body, but nothing shipped to
compose from — every tenant had to author its own parts first. This ships the parts and seeds them.
.resources/ReportParts/HeaderFooterDesign/) — 6 external, 5 internal..resources/ReportParts/ReportTheme/) — Default, Calm, Playful (styling-only.dotx).Composite Report Parts Mgt.(9667) — upserts each part intoTenant Report Layoutunderthe Tenant Report Defaults report ID.
BaseApp Install.OnInstallAppPerDatabaseandUpgrade Composite Report Parts(104064), guarded by a new per-database upgrade tag.Three things that look odd but are deliberate (commented in source):
type or content change. Same delete also cleans up the empty-App-ID copy an earlier version seeded.
[TryFunction], the write is outside it — the platform rejects DB writesinside a try function while install/upgrade holds a write transaction.
partial pass is retried next upgrade instead of being gated out forever.
Linked work
AB#647452
How I validated this
What I tested and the outcome
assignment picks them up and renders.
CompositeLayoutTests.Codeunit.al, all passing — covering seeding (subtype and.dotxMIME type), storage under the System App ID, removal of the empty-App-ID copy,idempotent re-seed, tag registration and gating, and the failure path (missing resource is
reported not thrown, tag left unset, next run retries and then records it).
Risk & compatibility
MS-643211-CompositeReportPartsUpgradeTag-20260820(registeredin W1/BE/IT/RU); writes 14 rows to
Tenant Report Layout. Delete-and-reinsert means a tenantpart hand-authored under a reserved name with the System App ID would be replaced.
BaseApp InstallgainsOnInstallAppPerDatabaseandInherentEntitlements/Permissions = X;it stamps the tag so a fresh install doesn't re-seed on first upgrade.
Playful.dotxis ~5.5 MB (embedded assets); the rest are 50–75 KB.0000V42(all-scope) and0000V43(publisher-scoped — carries platform error text).