From 09fedcacf02a90a021ce9e18c0eb4bec1ef48302 Mon Sep 17 00:00:00 2001 From: Ken Sternberg Date: Wed, 18 Oct 2023 08:58:50 -0700 Subject: [PATCH] web: reify the data loop I was very unhappy with the "update this dot-path" mechanism I was using earlier; it was hard for me to read and understand what was happening, and I wrote the darned thing. I decided instead to go with a hard substitution model; each phase of the wizard is responsible for updating the *entire* payload, mostly by creating a new payload and substituting the field value associated with the event. On the receiver, we have to do that *again* to handle the swapping of providers when the user chooses one and then another. It looks clunky, and it is, but it's *legible*; a junior dev could understand what it's doing, and that's the goal. --- .../admin/applications/wizard/BasePanel.ts | 2 +- .../applications/wizard/ContextIdentity.ts | 2 +- .../wizard/ak-application-wizard.ts | 20 ++++++++++++++++--- ...-application-wizard-application-details.ts | 2 ++ ...ion-wizard-authentication-method-choice.ts | 5 ++++- ...k-application-wizard-commit-application.ts | 1 - .../wizard/methods/BaseProviderPanel.ts | 2 ++ ...ak-application-context-display-for-test.ts | 2 +- web/src/admin/applications/wizard/types.ts | 2 +- 9 files changed, 29 insertions(+), 9 deletions(-) diff --git a/web/src/admin/applications/wizard/BasePanel.ts b/web/src/admin/applications/wizard/BasePanel.ts index a5847d24d..a395fc4b3 100644 --- a/web/src/admin/applications/wizard/BasePanel.ts +++ b/web/src/admin/applications/wizard/BasePanel.ts @@ -2,7 +2,7 @@ import { WizardPanel } from "@goauthentik/components/ak-wizard-main/types"; import { AKElement } from "@goauthentik/elements/Base"; import { CustomEmitterElement } from "@goauthentik/elements/utils/eventEmitter"; -import { consume } from "@lit/context"; +import { consume } from "@lit-labs/context"; import { query } from "@lit/reactive-element/decorators.js"; import { styles as AwadStyles } from "./BasePanel.css"; diff --git a/web/src/admin/applications/wizard/ContextIdentity.ts b/web/src/admin/applications/wizard/ContextIdentity.ts index 7b41644e4..f03f147a6 100644 --- a/web/src/admin/applications/wizard/ContextIdentity.ts +++ b/web/src/admin/applications/wizard/ContextIdentity.ts @@ -1,4 +1,4 @@ -import { createContext } from "@lit/context"; +import { createContext } from "@lit-labs/context"; import { ApplicationWizardState } from "./types"; diff --git a/web/src/admin/applications/wizard/ak-application-wizard.ts b/web/src/admin/applications/wizard/ak-application-wizard.ts index f2e959108..e88ab4f25 100644 --- a/web/src/admin/applications/wizard/ak-application-wizard.ts +++ b/web/src/admin/applications/wizard/ak-application-wizard.ts @@ -1,8 +1,7 @@ -import { merge } from "@goauthentik/common/merge"; import { AkWizard } from "@goauthentik/components/ak-wizard-main/AkWizard"; import { CustomListenerElement } from "@goauthentik/elements/utils/eventEmitter"; -import { ContextProvider } from "@lit/context"; +import { ContextProvider } from "@lit-labs/context"; import { msg } from "@lit/localize"; import { customElement, state } from "lit/decorators.js"; @@ -97,7 +96,22 @@ export class ApplicationWizard extends CustomListenerElement( this.requestUpdate(); } - this.wizardState = merge(this.wizardState, update) as ApplicationWizardState; + // Being extremely explicit about how a wizard state gets built, so that we preserve as much + // information as possible. This is much more predictable than using a generic merge. + + this.wizardState = { + ...this.wizardState, + app: { + ...this.wizardState.app ?? {}, + ...update.app ?? {} + }, + provider: { + ...this.wizardState.provider ?? {}, + ...update.provider ?? {} + }, + providerModel: update.providerModel + } + this.wizardStateProvider.setValue(this.wizardState); this.requestUpdate(); } diff --git a/web/src/admin/applications/wizard/application/ak-application-wizard-application-details.ts b/web/src/admin/applications/wizard/application/ak-application-wizard-application-details.ts index 13e439d76..5a19eaf35 100644 --- a/web/src/admin/applications/wizard/application/ak-application-wizard-application-details.ts +++ b/web/src/admin/applications/wizard/application/ak-application-wizard-application-details.ts @@ -27,7 +27,9 @@ export class ApplicationWizardApplicationDetails extends BasePanel { const value = target.type === "checkbox" ? target.checked : target.value; this.dispatchWizardUpdate({ update: { + ...this.wizard, app: { + ...this.wizard.app, [target.name]: value, }, }, diff --git a/web/src/admin/applications/wizard/auth-method-choice/ak-application-wizard-authentication-method-choice.ts b/web/src/admin/applications/wizard/auth-method-choice/ak-application-wizard-authentication-method-choice.ts index 593406d66..38e43afbf 100644 --- a/web/src/admin/applications/wizard/auth-method-choice/ak-application-wizard-authentication-method-choice.ts +++ b/web/src/admin/applications/wizard/auth-method-choice/ak-application-wizard-authentication-method-choice.ts @@ -25,7 +25,10 @@ export class ApplicationWizardAuthenticationMethodChoice extends BasePanel { handleChoice(ev: InputEvent) { const target = ev.target as HTMLInputElement; this.dispatchWizardUpdate({ - update: { providerModel: target.value }, + update: { + ...this.wizard, + providerModel: target.value + }, status: this.validator() ? "valid" : "invalid", }); } diff --git a/web/src/admin/applications/wizard/commit/ak-application-wizard-commit-application.ts b/web/src/admin/applications/wizard/commit/ak-application-wizard-commit-application.ts index aa21bd073..1cf58d9f0 100644 --- a/web/src/admin/applications/wizard/commit/ak-application-wizard-commit-application.ts +++ b/web/src/admin/applications/wizard/commit/ak-application-wizard-commit-application.ts @@ -133,7 +133,6 @@ export class ApplicationWizardCommitApplication extends BasePanel { if (body["provider"] !== undefined) { errs = [...errs, msg("In the Provider:"), ...spaceify(body["provider"])]; } - console.log(body, errs); return errs; } diff --git a/web/src/admin/applications/wizard/methods/BaseProviderPanel.ts b/web/src/admin/applications/wizard/methods/BaseProviderPanel.ts index ce60213e1..baafc7408 100644 --- a/web/src/admin/applications/wizard/methods/BaseProviderPanel.ts +++ b/web/src/admin/applications/wizard/methods/BaseProviderPanel.ts @@ -10,7 +10,9 @@ export class ApplicationWizardProviderPageBase extends BasePanel { const value = target.type === "checkbox" ? target.checked : target.value; this.dispatchWizardUpdate({ update: { + ...this.wizard, provider: { + ...this.wizard.provider, [target.name]: value, }, }, diff --git a/web/src/admin/applications/wizard/stories/ak-application-context-display-for-test.ts b/web/src/admin/applications/wizard/stories/ak-application-context-display-for-test.ts index fa6ecc8da..fa418199e 100644 --- a/web/src/admin/applications/wizard/stories/ak-application-context-display-for-test.ts +++ b/web/src/admin/applications/wizard/stories/ak-application-context-display-for-test.ts @@ -1,4 +1,4 @@ -import { consume } from "@lit/context"; +import { consume } from "@lit-labs/context"; import { customElement } from "@lit/reactive-element/decorators/custom-element.js"; import { state } from "@lit/reactive-element/decorators/state.js"; import { LitElement, html } from "lit"; diff --git a/web/src/admin/applications/wizard/types.ts b/web/src/admin/applications/wizard/types.ts index 0ebe7aa8a..ea1f40ecc 100644 --- a/web/src/admin/applications/wizard/types.ts +++ b/web/src/admin/applications/wizard/types.ts @@ -29,7 +29,7 @@ export interface ApplicationWizardState { type StatusType = "invalid" | "valid" | "submitted" | "failed"; export type ApplicationWizardStateUpdate = { - update?: Partial; + update?: ApplicationWizardState; status?: StatusType; };