This application fixes the bug with respect to the wizard-level context being updated incorrectly.

Understandings:

- To use uncontrolled inputs, which I prefer, the context object should not be a state or property
  at the level of consumers; it should not automatically re-render with every keystroke, i.e. "The
  React Way."  We're using Web Components, [client-side
  validation](https://developer.mozilla.org/en-US/docs/Learn/Forms/Form_validation) exists on the
  platform already, and live-validation is problematic for any number of reasons.
- The trade-off is that it is now necessary to re-render the target page of the wizard de-novo, but
  that's not really as big a deal as it sounds. Lit is ready to do that... and then nothing else
  until we request a change-of-page. Excellent.
- The top level context *must* be a state, but it's better if it's a state never actually used by
  the top-level context container. The debate about whether or not to make that container a dumb one
  (`<slot></slot>`) or to merge it with the top-level object continues; here, I've merged it with
  the top-level wizard object, but that object does not refer to the state variable being managed in
  its render pass, so changes to it do not cause a re-render of the whole wizard. The purpose of the
  top-level page is to manage the *steps*, not the *content of any step*. A step may change
  dynamically based on the content of a step, but that's the same thing as *which step*. Lesson:
  always know what your state is *about*.
- Deep merging is a complex subject, but here it's appropriate to our needs.
This commit is contained in:
Ken Sternberg 2023-08-21 12:12:55 -07:00
parent 570516c9c4
commit 02ef4365d4
9 changed files with 273 additions and 27 deletions

View File

@ -2,7 +2,7 @@ import { AKElement } from "@goauthentik/elements/Base";
import { CustomEmitterElement } from "@goauthentik/elements/utils/eventEmitter";
import { consume } from "@lit-labs/context";
import { query, state } from "@lit/reactive-element/decorators.js";
import { query } from "@lit/reactive-element/decorators.js";
import { styles as AwadStyles } from "./BasePanel.css";
@ -23,21 +23,9 @@ export class ApplicationWizardPageBase extends CustomEmitterElement(AKElement) {
@consume({ context: applicationWizardContext })
public wizard!: WizardState;
shouldUpdate(changedProperties: Map<string, any>) {
if (!this.rendered) {
this.rendered = true;
return true;
}
return (changedProperties.size !== 0)
}
// This used to be more complex; now it just establishes the event name.
dispatchWizardUpdate(update: Partial<WizardState>) {
// TODO: Incorporate this into the message heading upward: "the current step is valid."
this.dispatchCustomEvent("ak-application-wizard-update", {
...this.wizard,
...update,
});
this.dispatchCustomEvent("ak-application-wizard-update", { update });
}
}

View File

@ -1,3 +1,4 @@
import { merge } from "@goauthentik/common/merge";
import "@goauthentik/components/ak-wizard-main";
import { AKElement } from "@goauthentik/elements/Base";
import { CustomListenerElement } from "@goauthentik/elements/utils/eventEmitter";
@ -13,7 +14,7 @@ import PFBase from "@patternfly/patternfly/patternfly-base.css";
import applicationWizardContext from "./ak-application-wizard-context-name";
import { steps } from "./steps";
import { WizardState, WizardStateEvent } from "./types";
import { OneOfProvider, WizardState, WizardStateEvent } from "./types";
// my-context.ts
@ -30,6 +31,7 @@ export class ApplicationWizard extends CustomListenerElement(AKElement) {
* Providing a context at the root element
*/
@provide({ context: applicationWizardContext })
@state()
wizardState: WizardState = {
step: 0,
providerType: "",
@ -43,6 +45,8 @@ export class ApplicationWizard extends CustomListenerElement(AKElement) {
@property()
prompt = msg("Create");
providerCache: Map<string, OneOfProvider> = new Map();
constructor() {
super();
this.handleUpdate = this.handleUpdate.bind(this);
@ -60,12 +64,19 @@ export class ApplicationWizard extends CustomListenerElement(AKElement) {
// And this is where all the special cases go...
handleUpdate(event: CustomEvent<WizardStateEvent>) {
delete event.detail.target;
const newWizardState: WizardState = event.detail;
const update = event.detail.update;
// When the user sets the authentication method type, the corresponding authentication
// method page becomes available.
if (newWizardState.providerType !== "") {
// Are we changing provider type? If so, swap the caches of the various provider types the
// user may have filled in, and enable the next step.
const providerType = update.providerType;
if (
providerType &&
typeof providerType === "string" &&
providerType !== this.wizardState.providerType
) {
this.providerCache.set(this.wizardState.providerType, this.wizardState.provider);
const prevProvider = this.providerCache.get(providerType);
this.wizardState.provider = prevProvider ?? {};
const newSteps = [...this.steps];
const method = newSteps.find(({ id }) => id === "auth-method");
if (!method) {
@ -74,7 +85,9 @@ export class ApplicationWizard extends CustomListenerElement(AKElement) {
method.disabled = false;
this.steps = newSteps;
}
this.wizardState = newWizardState;
this.wizardState = merge(this.wizardState, update) as WizardState;
console.log(JSON.stringify(this.wizardState, null, 2));
}
render(): TemplateResult {

View File

@ -25,15 +25,12 @@ export class ApplicationWizardApplicationDetails extends BasePanel {
const value = target.type === "checkbox" ? target.checked : target.value;
this.dispatchWizardUpdate({
application: {
...this.wizard.application,
[target.name]: value,
},
});
}
render(): TemplateResult {
console.log("HUH?");
return html` <form class="pf-c-form pf-m-horizontal" @input=${this.handleChange}>
<ak-text-input
name="name"

View File

@ -0,0 +1,90 @@
import { policyOptions } from "@goauthentik/admin/applications/ApplicationForm";
import { first } from "@goauthentik/common/utils";
import "@goauthentik/components/ak-radio-input";
import "@goauthentik/components/ak-switch-input";
import "@goauthentik/components/ak-text-input";
import "@goauthentik/elements/forms/FormGroup";
import "@goauthentik/elements/forms/FormGroup";
import "@goauthentik/elements/forms/HorizontalFormElement";
import { msg } from "@lit/localize";
import { customElement } from "@lit/reactive-element/decorators/custom-element.js";
import { TemplateResult, html } from "lit";
import { ifDefined } from "lit/directives/if-defined.js";
import BasePanel from "../BasePanel";
@customElement("ak-application-wizard-commit-application")
export class ApplicationWizardCommitApplication extends BasePanel {
handleChange(ev: Event) {
if (!ev.target) {
console.warn(`Received event with no target: ${ev}`);
return;
}
const target = ev.target as HTMLInputElement;
const value = target.type === "checkbox" ? target.checked : target.value;
this.dispatchWizardUpdate({
application: {
[target.name]: value,
},
});
}
render(): TemplateResult {
return html` <form class="pf-c-form pf-m-horizontal" @input=${this.handleChange}>
<ak-text-input
name="name"
value=${ifDefined(this.wizard.application?.name)}
label=${msg("Name")}
required
help=${msg("Application's display Name.")}
></ak-text-input>
<ak-text-input
name="slug"
value=${ifDefined(this.wizard.application?.slug)}
label=${msg("Slug")}
required
help=${msg("Internal application name used in URLs.")}
></ak-text-input>
<ak-text-input
name="group"
value=${ifDefined(this.wizard.application?.group)}
label=${msg("Group")}
help=${msg(
"Optionally enter a group name. Applications with identical groups are shown grouped together.",
)}
></ak-text-input>
<ak-radio-input
label=${msg("Policy engine mode")}
required
name="policyEngineMode"
.options=${policyOptions}
.value=${this.wizard.application?.policyEngineMode}
></ak-radio-input>
<ak-form-group>
<span slot="header"> ${msg("UI settings")} </span>
<div slot="body" class="pf-c-form">
<ak-text-input
name="metaLaunchUrl"
label=${msg("Launch URL")}
value=${ifDefined(this.wizard.application?.metaLaunchUrl)}
help=${msg(
"If left empty, authentik will try to extract the launch URL based on the selected provider.",
)}
></ak-text-input>
<ak-switch-input
name="openInNewTab"
?checked=${first(this.wizard.application?.openInNewTab, false)}
label=${msg("Open in new tab")}
help=${msg(
"If checked, the launch URL will open in a new browser tab or window from the user's application library.",
)}
>
</ak-switch-input>
</div>
</ak-form-group>
</form>`;
}
}
export default ApplicationWizardApplicationDetails;

View File

@ -10,7 +10,6 @@ export class ApplicationWizardProviderPageBase extends BasePanel {
const value = target.type === "checkbox" ? target.checked : target.value;
this.dispatchWizardUpdate({
provider: {
...this.wizard.provider,
[target.name]: value,
},
});

View File

@ -33,8 +33,19 @@ export const steps: WizardStep[] = [
renderer: () =>
html`<ak-application-wizard-authentication-method></ak-application-wizard-authentication-method>`,
disabled: true,
nextButtonLabel: msg("Next"),
backButtonLabel: msg("Back"),
valid: true,
},
{
id: "commit-application",
label: "Submit New Application",
renderer: () =>
html`<ak-application-wizard-commit-application></ak-application-wizard-commit-application>`,
disabled: true,
nextButtonLabel: msg("Submit"),
backButtonLabel: msg("Back"),
valid: true,
},
];

View File

@ -23,4 +23,4 @@ export interface WizardState {
provider: OneOfProvider;
}
export type WizardStateEvent = WizardState & { target?: HTMLInputElement };
export type WizardStateEvent = { update: Partial<WizardState> };

View File

@ -0,0 +1,28 @@
import { ReactiveController } from 'lit';
import type { ReactiveControllerHost } from 'lit';
export class ApplicationWizardController implements ReactiveController {
host: ReactiveControllerHost;
value = new Date();
timeout: number;
private _timerID?: number;
constructor(host: ReactiveControllerHost, timeout = 1000) {
(this.host = host).addController(this);
this.timeout = timeout;
}
hostConnected() {
// Start a timer when the host is connected
this._timerID = setInterval(() => {
this.value = new Date();
// Update the host with new value
this.host.requestUpdate();
}, this.timeout);
}
hostDisconnected() {
// Clear the timer when the host is disconnected
clearInterval(this._timerID);
this._timerID = undefined;
}
}

120
web/src/common/merge.ts Normal file
View File

@ -0,0 +1,120 @@
/** Taken from: https://github.com/zellwk/javascript/tree/master
*
* We have added some typescript annotations, but this is such a rich feature with deep nesting
* we'll just have to watch it closely for any issues. So far there don't seem to be any.
*
*/
function objectType<T>(value: T) {
return Object.prototype.toString.call(value);
}
// Creates a deep clone for each value
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function cloneDescriptorValue(value: any) {
// Arrays
if (objectType(value) === "[object Array]") {
const array = [];
for (let v of value) {
v = cloneDescriptorValue(v);
array.push(v);
}
return array;
}
// Objects
if (objectType(value) === "[object Object]") {
const obj = {};
const props = Object.keys(value);
for (const prop of props) {
const descriptor = Object.getOwnPropertyDescriptor(value, prop);
if (!descriptor) {
continue;
}
if (descriptor.value) {
descriptor.value = cloneDescriptorValue(descriptor.value);
}
Object.defineProperty(obj, prop, descriptor);
}
return obj;
}
// Other Types of Objects
if (objectType(value) === "[object Date]") {
return new Date(value.getTime());
}
if (objectType(value) === "[object Map]") {
const map = new Map();
for (const entry of value) {
map.set(entry[0], cloneDescriptorValue(entry[1]));
}
return map;
}
if (objectType(value) === "[object Set]") {
const set = new Set();
for (const entry of value.entries()) {
set.add(cloneDescriptorValue(entry[0]));
}
return set;
}
// Types we don't need to clone or cannot clone.
// Examples:
// - Primitives don't need to clone
// - Functions cannot clone
return value;
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function _merge(output: Record<string, any>, input: Record<string, any>) {
const props = Object.keys(input);
for (const prop of props) {
// Prevents Prototype Pollution
if (prop === "__proto__") continue;
const descriptor = Object.getOwnPropertyDescriptor(input, prop);
if (!descriptor) {
continue;
}
const value = descriptor.value;
if (value) descriptor.value = cloneDescriptorValue(value);
// If don't have prop => Define property
// [ken@goauthentik] Using `hasOwn` is preferable over
// the basic identity test, according to Typescript.
if (!Object.hasOwn(output, prop)) {
Object.defineProperty(output, prop, descriptor);
continue;
}
// If have prop, but type is not object => Overwrite by redefining property
if (typeof output[prop] !== "object") {
Object.defineProperty(output, prop, descriptor);
continue;
}
// If have prop, but type is Object => Concat the arrays together.
if (objectType(descriptor.value) === "[object Array]") {
output[prop] = output[prop].concat(descriptor.value);
continue;
}
// If have prop, but type is Object => Merge.
_merge(output[prop], descriptor.value);
}
}
export function merge(...sources: Array<object>) {
const result = {};
for (const source of sources) {
_merge(result, source);
}
return result;
}
export default merge;