fix(deploy): do not offer the deploy wizard inside a report #388

Merged

allan merged 2 commits from feat/embed-not-configured into release/7.20 2026-10-28 17:08:41 +00:00

Conversation 2 Commits 2 Files Changed 4

+256 -3

hermes commented 2026-10-28 15:08:15 +00:00

Collaborator

Copy link

Copy source

Why

An embedded instance cannot deploy itself. Deployment needs an authenticated, standalone session, and the person looking at a report object is not the person who installs Data Controller.

We hit this while setting up a VA report: with embed=va, the object rendered the Viya Deploy wizard. Terms accepted, credentials filled, DEPLOY clicked - via JS click, a real mouse event and keyboard submit, with console error trapping - and nothing happened, in both a report object and a standalone tab (which reported "Not logged in"). The wizard was never reachable in a way that could work.

What changes

globals.embed is set at app init for embed=va and embed=true. When it is set, the deploy component now renders a short notice instead of the wizard:

Data Controller is not configured

This Data Controller instance has not been deployed yet, so it cannot be
shown inside a report.

Deployment has to be done once, in its own browser tab, by someone who can
write to the application location:
<the standalone URL>

When it is configured, reload the report and this object will show the editor.

No buttons, nothing to click that cannot work. Standalone behaviour is untouched: the T&C step and the wizard appear exactly as before.

Scope

Client only. The redirect to /deploy still happens when the app finds itself unconfigured - all that changed is what that route shows when embedded. Worth following up separately if redirecting at all is wasteful in embed mode (the app could go straight to the notice), but that touches several services and is not needed to fix the misleading screen.

Tests

deploy.component.spec.ts: the wizard is offered standalone, withheld for embed=va and embed=true, and the standalone URL is the current page without the hash route.

## Why An embedded instance cannot deploy itself. Deployment needs an authenticated, standalone session, and the person looking at a report object is not the person who installs Data Controller. We hit this while setting up a VA report: with `embed=va`, the object rendered the Viya Deploy wizard. Terms accepted, credentials filled, DEPLOY clicked - via JS click, a real mouse event and keyboard submit, with console error trapping - and nothing happened, in both a report object and a standalone tab (which reported "Not logged in"). The wizard was never reachable in a way that could work. ## What changes `globals.embed` is set at app init for `embed=va` and `embed=true`. When it is set, the deploy component now renders a short notice instead of the wizard: Data Controller is not configured This Data Controller instance has not been deployed yet, so it cannot be shown inside a report. Deployment has to be done once, in its own browser tab, by someone who can write to the application location: <the standalone URL> When it is configured, reload the report and this object will show the editor. No buttons, nothing to click that cannot work. Standalone behaviour is untouched: the T&C step and the wizard appear exactly as before. ## Scope Client only. The redirect to `/deploy` still happens when the app finds itself unconfigured - all that changed is what that route shows when embedded. Worth following up separately if redirecting at all is wasteful in embed mode (the app could go straight to the notice), but that touches several services and is not needed to fix the misleading screen. ## Tests `deploy.component.spec.ts`: the wizard is offered standalone, withheld for `embed=va` and `embed=true`, and the standalone URL is the current page without the hash route.

hermes added 1 commit 2026-10-28 15:08:15 +00:00

fix(deploy): do not offer the deploy wizard inside a report ... 80e8911af8

Deployment needs an authenticated, standalone session, and the person
looking at a report object is not the person who installs the app. Offering
the wizard there produces a screen whose controls cannot work - the failure
we hit while testing a report: T&C accepted, credentials filled, DEPLOY
clicked, and nothing happened, because the embedded app has no session to
deploy with.

An embedded instance (embed=va or embed=true) now shows what the state is
and where to deploy from, instead of a form that cannot submit. Standalone
behaviour is unchanged.

hermes referenced this pull request 2026-10-28 16:16:16 +00:00

DC in a VA report object never reaches the editor (sandboxed iframe, no session) #390

hermes reviewed 2026-10-28 17:01:07 +00:00

hermes left a comment

Author

Collaborator

Copy link

Copy source

Reviewed PR #388 "fix(deploy): do not offer the deploy wizard inside a report" (3 files, +102/-2) against release/7.20 at 80e8911: the isEmbedded guard and notice in deploy.component.html/ts, the new deploy.component.spec.ts, and the interaction with the existing standalone-URL helper. commit and prettier conventions look right.

• client/src/app/deploy/deploy.component.spec.ts:24-56 - the spec asserts component.isEmbedded and nothing else, so it does not verify the behaviour the PR claims (wizard offered standalone, withheld for embed=va/embed=true). A template regression - dropping the !isEmbedded from the step === 0 and step > 0 guards - leaves every test green. The repo already renders components in TestBed in six specs (e.g. query.component.spec.ts); a shallow render asserting #TCS is present standalone and absent when globals.embed is 'va' would test the guard rather than a getter that mirrors it.
• client/src/app/deploy/deploy.component.spec.ts:49-51 - expect(component.deployUrl).not.toContain('#') cannot fail: window.location.pathname never contains a fragment, so the assertion holds even if the getter regressed to location.href. Pull the URL construction into a pure helper (standaloneUrl(href: string)) and test it with a hash-bearing input such as ${origin}/path?x=1#/deploy, asserting the query and fragment are both dropped.
• client/src/app/deploy/deploy.component.ts:59-63 and 77-79 - the component now has two standalone-URL computations: deployUrl (origin + pathname, drops query and fragment) and onNavigateToHome (location.href.split('#')[0], keeps the query string, so a URL still carrying ?embed=va reopens embedded). Have onNavigateToHome use the same helper so there is one definition of "the standalone URL".

3 findings above for review.

Reviewed PR #388 "fix(deploy): do not offer the deploy wizard inside a report" (3 files, +102/-2) against release/7.20 at 80e8911: the isEmbedded guard and notice in deploy.component.html/ts, the new deploy.component.spec.ts, and the interaction with the existing standalone-URL helper. commit and prettier conventions look right. - client/src/app/deploy/deploy.component.spec.ts:24-56 - the spec asserts `component.isEmbedded` and nothing else, so it does not verify the behaviour the PR claims (wizard offered standalone, withheld for embed=va/embed=true). A template regression - dropping the `!isEmbedded` from the `step === 0` and `step > 0` guards - leaves every test green. The repo already renders components in TestBed in six specs (e.g. query.component.spec.ts); a shallow render asserting `#TCS` is present standalone and absent when `globals.embed` is 'va' would test the guard rather than a getter that mirrors it. - client/src/app/deploy/deploy.component.spec.ts:49-51 - `expect(component.deployUrl).not.toContain('#')` cannot fail: `window.location.pathname` never contains a fragment, so the assertion holds even if the getter regressed to `location.href`. Pull the URL construction into a pure helper (`standaloneUrl(href: string)`) and test it with a hash-bearing input such as `${origin}/path?x=1#/deploy`, asserting the query and fragment are both dropped. - client/src/app/deploy/deploy.component.ts:59-63 and 77-79 - the component now has two standalone-URL computations: `deployUrl` (origin + pathname, drops query and fragment) and `onNavigateToHome` (`location.href.split('#')[0]`, keeps the query string, so a URL still carrying `?embed=va` reopens embedded). Have onNavigateToHome use the same helper so there is one definition of "the standalone URL". 3 findings above for review.

hermes added 1 commit 2026-10-28 17:05:52 +00:00

fix(deploy): one definition of the standalone URL, and test the guards ... 459ff2da37

Review follow-up on the embed guard.

The spec asserted the isEmbedded getter, which mirrors the template rather
than exercising it: dropping !isEmbedded from the @if guards left every test
green. The spec now renders the component in a TestBed and asserts the DOM -
#TCS present standalone and absent for embed=va and embed=true, and the
deploy section withheld after the terms are accepted when embedded. Both
guards were mutation-checked by removing them, which fails the spec.

deployUrl and onNavigateToHome computed the standalone URL two different
ways: origin+pathname in one, location.href.split('#')[0] in the other,
which kept the query string, so a tab opened from ?embed=va#/deploy stayed
embedded. Both now call standaloneUrl() in standalone-url.ts, which parses
the href and drops the query and the fragment. It is tested with a
hash-bearing input, and the navigate test asserts the opened URL carries
neither '?' nor '#', so a regression to split('#') fails.

hermes commented 2026-10-28 17:05:52 +00:00

Author

Collaborator

Copy link

Copy source

All three points addressed in 459ff2d.

The spec now renders the component. deploy.component.spec.ts configures a TestBed (NO_ERRORS_SCHEMA for the wizard's child components) and asserts the DOM: #TCS present standalone, absent for embed=va and for embed=true with #not-configured present instead, the notice's link pointing at the standalone URL, and .autodeploy-section withheld after the terms are accepted when embedded (present standalone). I mutation-checked both guards rather than trusting the green run: removing !isEmbedded from @if (!isEmbedded && step === 0) fails two tests, removing it from @if (!isEmbedded && step > 0) fails the deploy-section test. Both guards are back in place.

Note: query.component.spec.ts constructs its class directly rather than using TestBed - the TestBed precedent is the service and mock specs (app.service.spec.ts, sas.service.spec.ts). This is the first component spec here that renders a template, so the pattern is new to the folder.

The URL construction is now a pure helper. standalone-url.ts exports standaloneUrl(href: string), which parses with URL and returns origin + pathname. It is tested directly with hash-bearing input: <origin>/path?x=1#/deploy -> <origin>/path, <origin>/app?embed=va -> <origin>/app, plus a relative /app/#/deploy.

One definition, both call sites. deployUrl and onNavigateToHome both call the helper; onNavigateToHome no longer uses location.href.split('#')[0], which kept the query and so reopened ?embed=va embedded. A test spies on window.open and asserts the opened URL contains neither ? nor # - Karma serves the runner with a query string, so a regression to split('#') fails it rather than passing vacuously (which is what the old not.toContain('#') on pathname did).

Verification: ng test --include='src/app/deploy/**/*.spec.ts' under ChromeHeadlessCI - 11 SUCCESS, exit 0; prettier --check clean on the folder. I could not run the full client suite here (no Chrome binary in the base image until CHROME_BIN=/usr/bin/chromium was set, and the full suite is long), so CI remains the authority for that.

All three points addressed in 459ff2d. **The spec now renders the component.** `deploy.component.spec.ts` configures a TestBed (NO_ERRORS_SCHEMA for the wizard's child components) and asserts the DOM: `#TCS` present standalone, absent for `embed=va` and for `embed=true` with `#not-configured` present instead, the notice's link pointing at the standalone URL, and `.autodeploy-section` withheld after the terms are accepted when embedded (present standalone). I mutation-checked both guards rather than trusting the green run: removing `!isEmbedded` from `@if (!isEmbedded && step === 0)` fails two tests, removing it from `@if (!isEmbedded && step > 0)` fails the deploy-section test. Both guards are back in place. Note: `query.component.spec.ts` constructs its class directly rather than using TestBed - the TestBed precedent is the service and mock specs (`app.service.spec.ts`, `sas.service.spec.ts`). This is the first component spec here that renders a template, so the pattern is new to the folder. **The URL construction is now a pure helper.** `standalone-url.ts` exports `standaloneUrl(href: string)`, which parses with `URL` and returns origin + pathname. It is tested directly with hash-bearing input: `<origin>/path?x=1#/deploy` -> `<origin>/path`, `<origin>/app?embed=va` -> `<origin>/app`, plus a relative `/app/#/deploy`. **One definition, both call sites.** `deployUrl` and `onNavigateToHome` both call the helper; `onNavigateToHome` no longer uses `location.href.split('#')[0]`, which kept the query and so reopened `?embed=va` embedded. A test spies on `window.open` and asserts the opened URL contains neither `?` nor `#` - Karma serves the runner with a query string, so a regression to `split('#')` fails it rather than passing vacuously (which is what the old `not.toContain('#')` on `pathname` did). Verification: `ng test --include='src/app/deploy/**/*.spec.ts'` under ChromeHeadlessCI - 11 SUCCESS, exit 0; `prettier --check` clean on the folder. I could not run the full client suite here (no Chrome binary in the base image until `CHROME_BIN=/usr/bin/chromium` was set, and the full suite is long), so CI remains the authority for that.

allan merged commit 65699bdd39 into release/7.20 2026-10-28 17:08:41 +00:00

allan deleted branch feat/embed-not-configured 2026-10-28 17:08:41 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

hermes

Labels

Clear labels

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

wontfix

This won't be fixed

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

allan (Allan) dcbot DylanLeong henrik hermes (hermes) mihajlo sabir sead trevor Yury

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dc/dc#388