Skip to content

[Project Arches] Microsoft Fabric Export Setup - #11314

Open
Bert Verbeek (Bertverbeek4PS) wants to merge 93 commits into
microsoft:mainfrom
Bertverbeek4PS:projectArches
Open

Bert Verbeek (Bertverbeek4PS) wants to merge 93 commits into
microsoft:mainfrom
Bertverbeek4PS:projectArches

Conversation

@Bertverbeek4PS

@Bertverbeek4PS Bert Verbeek (Bertverbeek4PS) commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

What & why

Export to Microsoft Fabric

Linked work

Fixes AB#649960

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 (required — be specific: scenarios, commands, screenshots for UI changes)

Risk & compatibility

- Introduced permissionset for Fabric Platform Admin to manage access.
- Created pages for managing Fabric Platform Companies, Export Details, Export Summary, and Setup.
- Implemented codeunit for Fabric Platform Management, handling export lifecycle and table management.
- Added functionality to manage Fabric Config Packages and their associated tables.
- Developed test codeunits to validate the behavior of the Fabric Platform functionalities.
- Established a test framework for ensuring the integrity of the Fabric export process.
- Introduced new codeunits for managing credentials and HTTP requests.
- Enhanced permissionset for Fabric Platform Admin to include new codeunits.
- Updated Fabric Platform Setup page to support workspace and lakehouse selection.
- Added functionality for service principal management in the Fabric workspace.
…panies, Fabric Platform Export Details, Fabric Platform Export Summary, and Fabric Platform Tables pages
…ort/export capabilities, and improved user permissions
…edures and updating workspace name handling in Fabric Platform Setup
@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 Sep 15, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 6

Recommendation: Accept with Suggestions

What this PR does

This round creates the setup record when the setup API page opens, uses safer error details for setup and Fabric API failures, keeps the HTTP test seam internal, and adds a guard so automatic package reapply does not abort when the table limit would be exceeded. The changes fit the feature design and the prior package-claim fixes remain in place. The new capacity skip path is sensible, but it should have a direct regression test because it controls install/upgrade behavior.

Status of previous suggestions
ID Title Status Author response
S1 Package deactivation removes manual selections Addressed The package-claim model remains in place.
S2 Package rebuild leaves stale claims Addressed Dropped claims are still released before rebuilding.
S3 Permission sets miss setup codeunits Addressed The required helper permissions remain included.
S4 Project settings are out of date Addressed The generated settings remain committed.
New observations (commits since round 5)

S5 (🟠 Moderate): Cover package reapply capacity skip
Please add a test where an active package is registered again while the selected table count is near the limit. It should prove the skip path does not abort install or upgrade and leaves the package ready for a later reapply.

Risk assessment and necessity

Risk: The feature still spans setup, API pages, permissions, configuration packages, and Fabric service calls. The latest edits reduce customer-facing error detail and install/upgrade risk, but the package reapply capacity path can affect which tables are exported after an upgrade.

Necessity: The changes are needed to make the Fabric export setup robust, keep internal seams out of the public surface, and avoid blocking install or upgrade when the export table cap is reached.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11314 round=6 by=alexei-dobriansky at=2026-09-15T22:13:46Z lastSha=c898eb789117d31e420c28b4c2eda37cc251d23c reviewKey=48f9ebfd57f5ad4dace781192f4da1d6fe2be08225c68638a2f233366ada066d suggestions=S1@c0c4467c:addressed,S2@5f0b06a9:addressed,S3@3f065ef4:addressed,S4@9208267f:addressed,S5@8329720b:new parentRound=5

@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 Sep 16, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 7

Recommendation: Accept with Suggestions

What this PR does

This round adds a direct regression test for the package reapply capacity-skip path. The test creates an active package, fills the selected table list to capacity, re-registers the package with one extra table, and verifies that the package stays active and ready to reapply while the new table is not pushed to the platform. That matches the feature behavior and addresses the previous coverage gap, but the new test should start from the same clean state as the rest of this test codeunit.

Status of previous suggestions
ID Title Status Author response
S1 Package deactivation removes manual selections Addressed Still addressed in the current diff.
S2 Package rebuild leaves stale claims Addressed Still addressed in the current diff.
S3 Permission sets miss setup codeunits Addressed Still addressed in the current diff.
S4 Project settings are out of date Addressed Still addressed in the current diff.
S5 Cover package reapply capacity skip Addressed A new test now covers active package re-registration when the table limit would be exceeded.
New observations (commits since round 6)

S6 (🟠 Moderate): Initialize capacity skip test
Call Initialize() at the start of this test like the surrounding tests. Without clearing package and platform tables, it can depend on data left by another test and fail or pass for the wrong reason.

Risk assessment and necessity

Risk: The latest commit changes only test code, so runtime risk is unchanged. The feature still spans setup, API pages, permissions, configuration packages, and Fabric service calls; the package reapply path controls which tables are exported after install or upgrade.

Necessity: The new test is necessary because the feature intentionally skips automatic reapply at capacity instead of aborting install or upgrade. Keeping the test isolated is also important so it proves that behavior reliably.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11314 round=7 by=alexei-dobriansky at=2026-09-16T22:09:36Z lastSha=3d696cbe988b4a489c7753ce37d1412ecb474681 reviewKey=e7d129db23f37d54b35ef723c3f30795b6d97852b31df55eae3bf623e8c46eb0 suggestions=S1@c0c4467c:addressed,S2@5f0b06a9:addressed,S3@3f065ef4:addressed,S4@9208267f:addressed,S5@8329720b:addressed,S6@31c3de5e:new parentRound=6

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 8

Recommendation: Accept with Suggestions

What this PR does

This round fixes the remaining capacity-skip test isolation issue and adds lifecycle safeguards for Fabric setup. It records recent enable requests, blocks another enable while the setup run is still active, shows a safer reset confirmation, skips start when an export run is already running, and adds tests for these flows.

The previous test isolation finding is addressed because the capacity-skip test now starts from a clean state. The new lifecycle behavior mostly fits the feature, but the enable cooldown still has a timing gap before the async setup summary row is visible, and one test assertion does not fully prove the second enable path.

Status of previous suggestions
ID Title Status Author response
S1 Package deactivation removes manual selections Addressed The claim model still keeps manual and package ownership separate.
S2 Package rebuild leaves stale claims Addressed Dropped package claims are still released before rebuilding active package lines.
S3 Permission sets miss setup codeunits Addressed The setup helper permissions remain included.
S4 Project settings are out of date Addressed The generated country project settings remain committed.
S5 Cover package reapply capacity skip Addressed The capacity-skip path remains covered by a direct regression test.
S6 Initialize capacity skip test Addressed The capacity-skip test now calls Initialize() before arranging package data.
New observations (commits since round 7)

S7 (🟠 Moderate): Cooldown misses pre-summary enable window
Do not treat a missing Setup run row as proof that the last enable request finished. A second Connect to Fabric click can arrive after the timestamp is stored but before the async setup summary row appears, so the duplicate request can still be sent. Block during the cooldown until a terminal setup row is known, or record request state when the enable call starts.

S8 (🟡 Minor): Reset enable seam before assertion
Reset the test subscriber before the second EnableExport call, or assert a call count. The Boolean flag is already true from the first call, so this assertion does not prove the second call reached the seam.

Risk assessment and necessity

Risk: Runtime risk is moderate because the latest code controls repeated Fabric setup and synchronization requests. The reset confirmation reduces accidental destructive action risk, but the cooldown gap can still allow duplicate enable requests during async setup startup.

Necessity: The round-8 changes are useful and targeted. They finish the previous test cleanup and make Fabric setup operations safer, but the cooldown and its test should be tightened so the new guard proves the intended behavior.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11314 round=8 by=alexei-dobriansky at=2026-09-17T22:21:27Z lastSha=87daf17411336d5decdf312ffe65da917359f46e reviewKey=7c1cc5544cdb82e63124d4c5d6d5bdea4644f0d78d9d066eaf431f3a8d2b96c4 suggestions=S1@c0c4467c:addressed,S2@5f0b06a9:addressed,S3@3f065ef4:addressed,S4@9208267f:addressed,S5@8329720b:addressed,S6@31c3de5e:addressed,S7@b62d908b:new,S8@1df8bdda:new parentRound=7

…nd cooldown logic; update test to reset state before re-enabling
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 9

Recommendation: Accept with Suggestions

What this PR does

This round tightens the Fabric enable cooldown. It records the enable request before the test seam and only allows another request inside the cooldown when a setup run that started after that request has already reached a non-running state.

The duplicate-click gap from the prior round is addressed because a missing setup summary row no longer counts as a finished setup run. The new test reset also proves that the second enable call reaches the seam. One retry edge case remains because the timestamp is stored before the local client ID and secret validation can fail.

Status of previous suggestions
ID Title Status Author response
S1 Package deactivation removes manual selections Addressed Manual and package ownership remain separated.
S2 Package rebuild leaves stale claims Addressed Dropped package claims are still released before active package lines are rebuilt.
S3 Permission sets miss setup codeunits Addressed Setup helper execute permissions remain included.
S4 Project settings are out of date Addressed Generated country project settings remain committed.
S5 Cover package reapply capacity skip Not addressed The test is present, but current validation still fails in that test codeunit because PackageLine is unused.
S6 Initialize capacity skip test Addressed The capacity-skip test now calls Initialize() before arranging package data.
S7 Cooldown misses pre-summary enable window Addressed The cooldown check no longer treats a missing setup row as finished.
S8 Reset enable seam before assertion Addressed The test subscriber state is reset before the second enable call.
New observations (commits since round 8)

S9 (🟠 Moderate): Avoid cooldown after failed enable
Set LastEnableRequestedAt only after the client ID and secret checks pass, while still doing it before the platform enable call. The current order can start the cooldown when validation throws, so the next corrected attempt is blocked even though no enable request was sent.

Risk assessment and necessity

Risk: The feature controls Fabric setup lifecycle requests, package selection, API pages, permissions, and persistent setup state. The latest cooldown change reduces duplicate enable requests, but current W1 validation still reports an AA0137 analyzer error in the package-capacity test, and the new timestamp order can delay a normal setup retry.

Necessity: The feature is a coherent Fabric export setup increment. The latest change is necessary to prevent a second enable request while the asynchronous setup run has not yet produced a summary row, but failed local validation should not count as a real enable request.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11314 round=9 by=alexei-dobriansky at=2026-09-18T22:30:11Z lastSha=3883d18be322b27289dca6aabdcd1fad7f461523 reviewKey=6a9fa784c29a1be8b4caba72b71a0b98a9f426aa51ff9650c2ba21fb7ad73895 suggestions=S1@c0c4467c:addressed,S2@5f0b06a9:addressed,S3@3f065ef4:addressed,S4@9208267f:addressed,S5@8329720b:notaddressed,S6@31c3de5e:addressed,S7@b62d908b:addressed,S8@1df8bdda:addressed,S9@72e22729:new parentRound=8

…from RegisterPackageSkipsReapplyWhenCapacityWouldBeExceeded procedure
…ed constants for improved readability and maintainability
…kageLineExists procedure by using a labeled constant for expected table output

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.

Three functional issues to address before merge.

Comment thread src/Apps/W1/MicrosoftFabricExport/app/app.json Outdated
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 10

Recommendation: Accept with Suggestions

What this PR does

This round only cleans up tests. It removes the unused PackageLine variable from the package-capacity test and moves repeated token, secret, and expected-message text into locked labels.

The previous analyzer issue is addressed. The remaining retry concern is unchanged because EnableExport() still stores LastEnableRequestedAt before the client ID and client secret validation that can throw.

Status of previous suggestions
ID Title Status Author response
S1 Package deactivation removes manual selections Addressed Manual and package ownership remain separated.
S2 Package rebuild leaves stale claims Addressed Dropped package claims are still released before active package lines are rebuilt.
S3 Permission sets miss setup codeunits Addressed Setup helper execute permissions remain included.
S4 Project settings are out of date Addressed Generated country project settings remain committed.
S5 Cover package reapply capacity skip Addressed The unused PackageLine local was removed from the capacity-skip test.
S6 Initialize capacity skip test Addressed The capacity-skip test still calls Initialize() before arranging package data.
S7 Cooldown misses pre-summary enable window Addressed The cooldown check still waits for a setup summary row that started after the request.
S8 Reset enable seam before assertion Addressed The test subscriber state is still reset before the second enable call.
S9 Avoid cooldown after failed enable Not addressed SetLastEnableRequestedAt(CurrentDateTime()) still runs before client ID and client secret validation.
New observations (commits since round 9)

None - the latest commits only address the prior test cleanup and label analyzer/readability items.

Risk assessment and necessity

Risk: The feature controls Fabric setup lifecycle requests, package selection, API pages, permissions, and persistent setup state. The latest test-only cleanup lowers analyzer risk, but the open cooldown ordering issue can still delay a normal retry after failed setup validation.

Necessity: The feature is a coherent Fabric export setup increment. The latest cleanup is necessary to keep the test app clean, and moving the enable timestamp after local validation is still needed so failed validation does not look like a real enable request.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=11314 round=10 by=alexei-dobriansky at=2026-09-21T22:19:21Z lastSha=55bf9e7d114dc00e3b776ecbb256ca0b0b1ba6ce reviewKey=3a1cde8125810a8f125694ad718edf77377b6de12a206e25d91e67c0646cc2c9 suggestions=S1@c0c4467c:addressed,S2@5f0b06a9:addressed,S3@3f065ef4:addressed,S4@9208267f:addressed,S5@8329720b:addressed,S6@31c3de5e:addressed,S7@b62d908b:addressed,S8@1df8bdda:addressed,S9@72e22729:notaddressed parentRound=9

…sions in FabricExpAdmin.PermissionSet, and refine delete permissions in FabricConfigPackageSubform; enhance test codeunits with permissions setup

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.

Follow-up on the previously resolved threads: these remaining parts are still outstanding.

@@ -0,0 +1,53 @@
{
"id": "a1b2c3d4-e5f6-7890-abcd-ef1234567890",

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.

The production app ID was replaced, but the test app still uses the example GUID a1b2c3d4-e5f6-7890-abcd-ef1234567890. Please generate a real GUID here as well and update internalsVisibleTo in the production manifest.

PageType = List;
SourceTable = "Tenant Fabric Tables";
ApplicationArea = All;
InsertAllowed = false;

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.

Disabling deletion on the package subform fixes only half of the claim issue. This page still has DeleteAllowed = true by default, so a direct delete removes the platform row without calling ReleaseTable() and leaves its Manual claim orphaned. Please disable direct deletion here and provide a mediated remove action if removal is required.

tabledata "Tenant Fabric Export Summary" = R,
tabledata "Tenant Fabric Setup" = RIMD,
tabledata "Tenant Fabric Table Fields" = RIMD,
tabledata "Tenant Fabric Tables" = R,

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.

The company permission was updated, but this table remains read-only while Fabric Platform Tables exposes Fabric Schema Type as editable. A user assigned only this role will still get a Modify permission error. Either make that field read-only and route changes through a privileged codeunit, or grant the required direct Modify permission while keeping deletion mediated.

tabledata "Tenant Fabric Enum Mapping" = R,
tabledata "Tenant Fabric Export Details" = R,
tabledata "Tenant Fabric Export Summary" = R,
tabledata "Tenant Fabric Setup" = R,

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.

Fabric Platform Setup embeds Fabric Companies FactBox and Fabric Tables FactBox, but this role still grants neither FactBox page execute permissions nor read access to Tenant Fabric Companies. A user assigned only this role may fail when opening the setup page. Please add those permissions or remove the setup page from this role.

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

Labels

ADO: Sync Creates an Azure DevOps work item if needed and indicates that one is linked AL: Apps (W1) Add-on apps for W1 Build: scripts & configs Build scripts and configuration files From Fork Pull request is coming from a fork Ownership: Manual Preserve the manually selected team ownership Ownership: Needs Review Ownership is Other, low confidence, or needs manual correction Team: Integrations GitHub request for Integrations area Team: Other GitHub request for other area than SCM, Finance or Integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants