Skip to content

Add shared CompanyMetadata data source for report layouts#9094

Open
v-mortenr wants to merge 5 commits into
microsoft:mainfrom
v-mortenr:features/640703/company-metadata-subscriber
Open

Add shared CompanyMetadata data source for report layouts#9094
v-mortenr wants to merge 5 commits into
microsoft:mainfrom
v-mortenr:features/640703/company-metadata-subscriber

Conversation

@v-mortenr

@v-mortenr v-mortenr commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Work item: AB#640703

Adds a shared CompanyMetadata data source so report layouts can bind one shared company block instead of each report hand-coding Company Information fields.

What

  • ReportManagement subscribes to the platform GetCompanyMetadata business event and populates the shared company block (name, formatted address repeater, phone/fax/email/home page, bank details, VAT/registration/giro, logo) from Company Information.
  • New Company Metadata Builder codeunit (Access = Internal): typed setters centralize the wire keys so the subscriber never hand-writes raw key strings; includes the address-line repeater and the paired field captions.
  • CompanyDisplayName is sourced from the Company record (CompanyProperty.DisplayName(), falling back to CompanyName()) to match how the platform builds ReportRequest — not Company Information "Name 2".
  • Empty-safe: absent Company Information fields emit present-but-empty values.
  • Adds Tests-Report coverage for values, captions, the display-name source, empty-safe behavior, and the address repeater.

@github-actions github-actions Bot added the From Fork Pull request is coming from a fork label Jul 4, 2026
@github-actions github-actions Bot added the Linked Issue is linked to a Azure Boards work item label Jul 4, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Jul 4, 2026
@github-actions github-actions Bot added the needs-approval Workflow runs require maintainer approval to start label Jul 4, 2026
@v-mortenr
v-mortenr force-pushed the features/640703/company-metadata-subscriber branch 3 times, most recently from 5935848 to 617853f Compare July 4, 2026 19:40
Work item: AB#640703

Subscribe to the platform GetCompanyMetadata business event in ReportManagement
and populate a shared company block (name, formatted address, phone/fax/email/
home page, bank, VAT/registration, giro, logo) from Company Information, so report
layouts can bind one shared source instead of hand-coding company fields.

- New Company Metadata Builder codeunit: typed setters centralize the wire keys
  (the subscriber never hand-writes raw key strings), including the address-line
  repeater and paired field captions.
- CompanyDisplayName is sourced from the Company record (CompanyProperty.DisplayName,
  fallback to CompanyName) to match how the platform builds ReportRequest, not
  Company Information "Name 2".
- Empty-safe: absent Company Information fields emit present-but-empty values.
- Add Tests-Report coverage for values, captions, display-name source, empty-safe
  behavior, and the address repeater.
@v-mortenr
v-mortenr force-pushed the features/640703/company-metadata-subscriber branch from 617853f to f115f52 Compare July 6, 2026 08:10
@JesperSchulz JesperSchulz added the Integration GitHub request for Integration area label Jul 7, 2026
Add Company Metadata Builder.PopulateFromCompanyInformation to own the
Company Information field -> wire-key mapping (name, formatted address,
phone/fax/email/home page, VAT/registration, bank, and giro fields with
their captions). ReportManagement.GetCompanyMetadata is now orchestration
only: fetch the record, set the derived display name and logo, populate the
block, and write. The emitted payload is unchanged.

Prefix the new ReportManagement self-calls with this. (AA0248).

Work item: AB#640703
@github-actions github-actions Bot removed the needs-approval Workflow runs require maintainer approval to start label Jul 7, 2026
@v-mortenr
v-mortenr marked this pull request as ready for review July 7, 2026 13:31
@v-mortenr
v-mortenr requested a review from a team July 7, 2026 13:31
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Copilot PR Review

Iteration 2 · Outcome: completed

All four applicable sub-skills (performance, security, privacy, style) completed and evaluated their worklists with no findings. al-upgrade-review and al-ui-review returned not-applicable: the diff touches no upgrade/install/schema/enum surface and contains no page, pageextension, pagecustomization, or control add-in files.

Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630

Sub-skills skipped

  • al-upgrade-review — not-applicable
  • al-ui-review — not-applicable

Orchestrator pre-filter (13 file(s) excluded)

  • layer-disabled (knowledge) : 13 file(s)

Findings produced by the Copilot CLI agent against BCQuality at 822cae1b2771ac25f665f73369f69093bd4fd630. Reply 👎 on any inline comment to flag false positives.

Codeunit 139596 was already used by the Shopify test app
(ShpfyItemAttrAsOptionTest), which caused an object-ID conflict when the
test apps are installed together. Move Company Metadata Test to 139593
(unused, next to the sibling Report Layouts Test).

Work item: AB#640703
@mynjj mynjj added the Finance GitHub request for Finance area label Jul 17, 2026
@github-actions github-actions Bot removed the Finance GitHub request for Finance area label Jul 17, 2026
mynjj
mynjj previously approved these changes Jul 17, 2026
Bring in platform uptake (BCArtifact 29.0.52770.0) so CI can find the
platform event this change depends on. See PR microsoft#9094.
The country build matrix failed in the test compile with
AL0185 Codeunit 'Library Assert' is missing.

CompanyMetadataTest (codeunit 139593) references codeunit 130002
"Library Assert", which is provided by the separate "Library Assert"
app, not by the Assert codeunit (130000) already pulled in via
Tests-TestLibraries. Declare the missing dependency, matching how
sibling test apps (Tests-Job, Tests-Misc) reference it.
/// </summary>
/// <param name="CompanyMetadata">The JSON object to merge the company payload into; existing keys with the same names are overwritten.</param>
[InherentPermissions(PermissionObjectType::TableData, Database::"Company Information", 'r')]
procedure GetCompanyMetadata(var CompanyMetadata: JsonObject)

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

GetCompanyMetadata declares [InherentPermissions(PermissionObjectType::TableData, Database::"Company Information", 'r')] as its only inherent grant, but the body's transitive call chain reads more than Company Information. PopulateFromCompanyInformation calls FormatAddress.Company(AddrArray, CompanyInfo), which calls the shared FormatAddr helper; when Company Information's Country/Region Code is blank it reads General Ledger Setup (GetGLSetup -> GLSetup.Get()), and when it is set it reads Country/Region (Country.Get(CountryCode)). Neither table is covered by the declared InherentPermissions, so a caller whose own permission set lacks read access to Country/Region or General Ledger Setup will hit a permission error inside this 'empty-safe' procedure. This is a new, unconditional cost: GetCompanyMetadata is wired as a static EventSubscriber on the platform's GetCompanyMetadata event, so it now runs on every report execution across the system (not just reports that previously called FormatAddress.Company), extending the permission surface report generation depends on. Align the InherentPermissions attribute with the procedure's actual transitive reads (add Country/Region and General Ledger Setup 'r', or restructure so FormatAddress's own permission needs are declared where the read happens).

Knowledge:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

From Fork Pull request is coming from a fork Integration GitHub request for Integration area Linked Issue is linked to a Azure Boards work item

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants