Skip to content

[Bug 641000] Add the CLEAN29 preprocessor for the document report experience - #11074

Open
Morten Rasmussen (v-mortenr) wants to merge 1 commit into
mainfrom
bug/641000
Open

[Bug 641000] Add the CLEAN29 preprocessor for the document report experience#11074
Morten Rasmussen (v-mortenr) wants to merge 1 commit into
mainfrom
bug/641000

Conversation

@v-mortenr

@v-mortenr Morten Rasmussen (v-mortenr) commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

What & why

The DocumentReportExperience feature key gates the composite layout feature — the layout subtype,
the header/footer and theme parts, and the pages that configure them. The feature is GA in 29, so
every piece of that gating is now wrapped in #if not CLEAN29: 29 keeps the feature key exactly as
it behaves today

At the call sites:

  • 19 country Company Information pages — the Reporting group's Visible property, the
    FeatureKeyManagement declaration, the resolve-on-open block, and the
    DocumentReportExperienceEnabled variable.
  • 7 W1 Reporting pages — the Subtype field, the composite layout groups and actions, the
    header/footer and theme part fields, the two OnOpenPage gates that raise FeatureNotEnabledErr,
    and the three labels behind them.

27 files, 266 insertions, 0 deletions.

Linked work

AB#641000 — tracked in ADO (Dynamics SMB); internal cleanup with no public issue.

How I validated this

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings.
  • I ran the change in Business Central and confirmed it behaves as expected.
  • I added or updated tests for the new behavior, or explained below why none are needed.

What I tested and the outcome

  • Static verification only, and I want to be plain about that. Every #if is balanced by an
    #endif across all 27 files, and no reference to DocumentReportExperienceEnabled,
    IsDocumentReportExperienceEnabled or FeatureNotEnabledErr remains outside a guard — checked by
    walking each file and tracking guard depth line by line. Insertions only, no deletions, and no BOM
    or end-of-file newline drift.

  • No tests added or needed. 29 behaviour is unchanged — the gating is byte-for-byte what it was,
    just inside a guard. In later version the flag is gone and the feature is always on, which is what the tests
    already exercise: every relevant test in codeunit 134619 calls EnableDocumentReportExperience()
    in its setup, so they were already running with the feature enabled.

Risk & compatibility

29 is unaffected by design — same feature key, same behaviour, same code paths. The only behavioural
change lands when features goes GA, where the feature becomes unconditional, which is the intent of the GA.

…erience

The Document Report Experience feature key gates the composite layout feature -
the layout subtype, the header/footer and theme parts, and the pages that
configure them. The feature is GA in 29, so every piece of that gating is now
wrapped in #if not CLEAN29: 29 keeps the feature key exactly as it behaves
today, and 30 compiles without the flag, leaving the feature unconditional.

This is the first of the two steps the MCP feature flag went through. Compare
the EnableMcpAccess gating, which was wrapped the same way at GA and then
deleted wholesale once CLEAN28 came due (commit 3ffea8e). Nothing is removed
here; that is the follow-up change when 29 is cleaned up.

Feature Key Management keeps IsDocumentReportExperienceEnabled reading the
feature key, with the procedure and its DocumentReportExperienceTxt label both
inside the guard. No Obsolete attribute: the accessor still has live callers in
29, and the MCP precedent did not mark its equivalent either.

At the call sites:

- 19 country Company Information pages: the Reporting group's Visible property,
  the FeatureKeyManagement declaration, the resolve-on-open block and the
  DocumentReportExperienceEnabled variable.
- 7 W1 Reporting pages: the Subtype field, the composite layout groups and
  actions, the header/footer and theme part fields, the two OnOpenPage gates
  that raise FeatureNotEnabledErr, and the three labels behind them.

#else branches appear only where a term has to survive into 30: the company
default displays call, ReportLayouts' Visible = ... and BodyLayoutSelected plus
its OnOpenPage condition, and ReportLayoutNewDialog's two subtype visibility
assignments.

Where FeatureKeyManagement was the only local of a trigger, the var keyword is
inside the guard too, so 30 does not end up with an empty var section.

Verified statically: every #if is balanced by an #endif in all 27 files, and no
reference to the flag, the accessor or FeatureNotEnabledErr remains outside a
guard. Insertions only, no deletions.
@v-mortenr
Morten Rasmussen (v-mortenr) requested a review from a team September 4, 2026 16:15
@github-actions github-actions Bot added the Team: Integrations GitHub request for Integrations area label Sep 4, 2026
@github-actions github-actions Bot added this to the Version 30.0 milestone Sep 4, 2026
exit(FeatureManagementFacade.IsEnabled(GetSIEAuditFileExportFeatureKeyId()));
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\ —\ Upgrade}$

Feature Key Management schedules IsDocumentReportExperienceEnabled(), the backing DocumentReportExperienceTxt label, and the DocumentReportExperienceEnabled fields for removal via #if not CLEAN29, but none of these symbols carry [Obsolete(reason, tag)] metadata before the CLEAN guard — unlike the existing CLEAN27 precedent in the same codeunit, which marks its retiring procedure [Obsolete(...)] ahead of the #if not CLEAN27 guard. Per BC upgrade conventions, retiring a public procedure should go through an obsolete-pending stage with a documented reason and version tag so extensions calling it get a deprecation warning, rather than the symbol silently disappearing the moment CLEAN29 is defined. Add [Obsolete('...', '29.0')] to IsDocumentReportExperienceEnabled (and any other still-public members being retired) ahead of the CLEAN29 guard.

Knowledge:

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

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Accept

Problem-solution fit: Strong

What this PR does

This PR removes the runtime feature-key gate for the document report experience when the CLEAN29 build flag is active. Older builds keep the existing feature management checks, while CLEAN29 builds always expose the composite layout fields, actions, and lookup behavior.

The preprocessor wrapping is consistent across the changed pages, subscribers, and tests. I did not find a behavior path where older builds lose the feature flag or CLEAN29 builds keep a stale hidden control.

Suggestions

None.

Risk assessment and necessity

Risk: Low to moderate. The change is broad, but it is mechanical and limited to feature visibility and report layout selection. It does not change posting, ledger data, or amount calculation.

Necessity: The change is needed because the feature is no longer optional in CLEAN29. Keeping the old feature-key checks there would leave GA functionality behind a removed toggle.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11074 round=1 by=alexei-dobriansky at=2026-09-04T22:15:10Z lastSha=c4247173bb8f3965e5b92a87d6781a0f1d293de1 reviewKey=35eff01c0b7b653134a12c047e362da74be05f981fdb2d8d26c671106a8713a2 suggestions=none

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.

2 participants