Skip to content

[29.x] Composite Report Parts: seed via a report extension instead of install/upgrade code - #11128

Open
MSlenejennum wants to merge 2 commits into
releases/29.xfrom
MSlenejennum/649264/New-Header-and-Footer-Layouts-and-Report-Themes
Open

[29.x] Composite Report Parts: seed via a report extension instead of install/upgrade code#11128
MSlenejennum wants to merge 2 commits into
releases/29.xfrom
MSlenejennum/649264/New-Header-and-Footer-Layouts-and-Report-Themes

Conversation

@MSlenejennum

@MSlenejennum MSlenejennum commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replaces the SeedPart-based install/upgrade seeding of the shipped header/footer designs and report themes with a declarative report extension (CompositeLayout.reportExt.al, extending "Tenant Report Defaults") that lists all 14 parts as layout() entries.
  • Removes "Composite Report Parts Mgt.", "Upgrade Composite Report Parts", the per-database upgrade tag (W1/BE/IT/RU), the BaseApp Install seeding trigger, and the composite-report-part tests added to CompositeLayoutTests — none of it is needed once the platform registers the layouts natively instead of the app writing them into Tenant Report Layout itself.
  • Moves the 14 part resource files from .resources/ReportParts/ to Foundation/Reporting/ReportParts/, alongside the report extension that references them via LayoutFile.

This is a [29.x] backport of the same rework applied on main in #11137.

AB#649844

Test plan

  • Build the affected app(s) locally with no new analyzer warnings.
  • Confirm in Business Central that all 14 External/Internal Header & Footer Designs and Report Themes show up under "Tenant Report Defaults" via the Composite Layout report extension.

@MSlenejennum
MSlenejennum requested review from a team September 7, 2026 11:05
@MSlenejennum
MSlenejennum requested a review from a team as a code owner September 7, 2026 11:05
@github-actions github-actions Bot added the Team: Integrations GitHub request for Integrations area label Sep 7, 2026
@github-actions github-actions Bot added this to the Version 29.1 milestone Sep 7, 2026
Comment thread src/Layers/W1/BaseApp/Foundation/Reporting/CompositeReportPartsMgt.Codeunit.al Outdated
Comment thread src/Layers/W1/BaseApp/Foundation/Reporting/CompositeReportPartsMgt.Codeunit.al Outdated
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This PR adds shipped composite report header/footer designs and report themes, and seeds them during install, upgrade, and company open. The seeding code uses upsert, stores rows under the app ID, logs bad resource files, and includes tests for permissions, tags, and missing resources.

The normal install and upgrade path looks coherent. The resource names match the package, the resource archives are valid Word packages, and the System Initialization event publisher exists at company open. One bootstrap path still has a guard issue.

Problem-solution fit

Fit: Partial

The change covers normal install and upgrade and tests most of the new behavior. It does not fully cover the company-open fallback because the shared database tag can be set before that fallback runs.

Suggestions

S1 (🔴 High): Company-open fallback can skip missing parts
The fallback exits as soon as the database upgrade tag exists. CompanyInitialize sets registered per-database tags for new companies, so a database that missed this seed can be tagged before this subscriber runs and the parts stay missing. Please check for the expected seeded part before exiting, or use a guard that is written only after this seeding succeeds.

Risk assessment and necessity

Risk: The new code writes Tenant Report Layout rows at install, upgrade, and company open. Wrong gating can leave new or pre-built databases without the shipped header/footer and theme parts. App builds were green; unit test jobs were still in progress when checked.

Necessity: The change is needed so composite report layouts have out-of-box parts. The scope is mostly right, but the fallback guard should match the bootstrap scenario before merge.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11128 round=1 by=alexei-dobriansky at=2026-09-07T22:05:36Z lastSha=9712bef51dc313ea59344b212c348e39a28ad313 reviewKey=1839e1d8241c1d98ad98bfd627082ab29ff9cdf6646a3656fc5bc5166cd8aff5 suggestions=S1@1e5646ab

@MSlenejennum MSlenejennum self-assigned this Sep 8, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 2

Recommendation: Request Changes

What this PR does

The latest commits make shipped part seeding report whether every part was seeded, and they record the database tag only after a full pass. They also keep resource-read failures non-fatal while leaving database writes outside the try function.

That fixes the partial resource-failure path, but it does not fix the company-open fallback guard. The fallback still exits only because the registered database tag exists, so the bootstrap path can still skip the missing seed data.

Status of previous suggestions
ID Title Status Author response
S1 Company-open fallback can skip missing parts Not addressed
New observations (commits since round 1)

None - the remaining blocker is the previous suggestion, which is still open.

Risk assessment and necessity

Risk: The new code writes Tenant Report Layout rows during install, upgrade, and company open. If the tag is set before the company-open fallback runs, tenants can still miss the shipped header/footer and theme parts.

Necessity: The change is needed so composite report layouts have the shipped parts. The scope is right, but the fallback guard must prove the data exists before it exits.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11128 round=2 by=alexei-dobriansky at=2026-09-08T22:08:33Z lastSha=5e75c2b91241b6e578e999e491c3235ec0bbe99f reviewKey=22e5490068a81c8568318eab9f963c164ec2790ea6afad9dcc04ff0fccc8d652 suggestions=S1@1e5646ab:notaddressed parentRound=1

…tension

The shipped header/footer designs and report themes no longer need to be
written into Tenant Report Layout by install/upgrade code running with
elevated permissions. Declaring them as layout() entries on a "Composite
Layout" report extension for "Tenant Report Defaults" lets the platform
register them natively, so the seeding codeunit, its upgrade pass, the
per-database upgrade tag (W1/BE/IT/RU), and their test suite are removed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@MSlenejennum
MSlenejennum force-pushed the MSlenejennum/649264/New-Header-and-Footer-Layouts-and-Report-Themes branch from 5e75c2b to 2b29609 Compare September 10, 2026 20:03
@MSlenejennum MSlenejennum changed the title [29.x] New Header & footer designs and Report Themes for Composite Layouts [29.x] Composite Report Parts: seed via a report extension instead of install/upgrade code Sep 10, 2026
…ention

Every other report extension in BaseApp uses ReportExt.al (capital R and E).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 3

Recommendation: Request Changes

What this PR does

The force-pushed diff registers 14 shipped header/footer and theme parts as rendering layouts on Tenant Report Defaults. It no longer uses install, upgrade, or company-open code to seed Tenant Report Layout rows.

That removes the previous fallback problem, because the old seeding path is gone. However, the new registration object does not publish successfully: validation reports that report ID 2000000001 already exists in the base application.

Status of previous suggestions
ID Title Status Author response
S1 Company-open fallback can skip missing parts Addressed The new diff removes the install/upgrade/company-open seeding path.
New observations (commits since round 2)

S2 (🔴 High): Report extension blocks publishing
reportextension 9666 "Composite Layout" extends "Tenant Report Defaults" currently makes validation fail with Report ID 2000000001 already existing in the base application. Use a supported registration path for these shipped layouts, or the app cannot publish and the parts will not be available.

Risk assessment and necessity

Risk: The new object blocks app/test-app publishing, so the shipped report parts cannot be delivered in this state. The binary layout files are passive once registration is fixed.

Necessity: The change is needed to provide shipped composite header/footer and theme parts without fragile install/upgrade seeding. The scope is right, but the registration approach must publish successfully.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11128 round=3 by=alexei-dobriansky at=2026-09-11T13:12:38Z lastSha=90085578e59883df871be6eb079e63c959a0df6d reviewKey=98781b7118336473629e7716dc173e68dd2a9645797f53f84f26c8bc95d818fe suggestions=S1@1e5646ab:addressed,S2@b90e9f30:new parentRound=2

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.

5 participants