From 5cf83f6f73a62ddaab327e04d43f0936797b891c Mon Sep 17 00:00:00 2001 From: Stuart Corbishley Date: Fri, 4 Aug 2023 09:49:19 +0200 Subject: [PATCH] Prevent pushHistoryPatches from using an old location There are situations where the browsers location.href hasn't been changed yet but the component has already mounted, leaving the diagram to navigate with a selection query param on the workflow list url instead of the workflow edit url. So we store the url in the liveview process and set it as a `data-` attr on the hook. Since we can't trust the browsers location we have to track it elsewhere. --- assets/js/workflow-editor/index.ts | 96 ++++++++++++++------ lib/lightning_web/live/workflow_live/edit.ex | 13 +-- 2 files changed, 72 insertions(+), 37 deletions(-) diff --git a/assets/js/workflow-editor/index.ts b/assets/js/workflow-editor/index.ts index 131b8be182..68567c35e6 100644 --- a/assets/js/workflow-editor/index.ts +++ b/assets/js/workflow-editor/index.ts @@ -10,27 +10,38 @@ import { createWorkflowStore, } from './store'; -type WorkflowEditorEntrypoint = PhoenixHook<{ - _isMounting: boolean; - _pendingWorker: Promise; - abortController: AbortController | null; - component: ReturnType | null; - componentModule: Promise<{ mount: typeof mount }>; - getWorkflowParams(): void; - getItem( - id?: string - ): Lightning.TriggerNode | Lightning.JobNode | Lightning.Edge | undefined; - handleWorkflowParams(payload: { workflow_params: WorkflowProps }): void; - maybeMountComponent(): void; - onSelectionChange(id?: string): void; - pendingChanges: PendingAction[]; - processPendingChanges(): void; - pushPendingChange( - pendingChange: PendingAction, - abortController: AbortController - ): Promise; - workflowStore: ReturnType; -}>; +type AttributeMutationRecord = MutationRecord & { + attributeName: string; + oldValue: string; +}; + +type WorkflowEditorEntrypoint = PhoenixHook< + { + _isMounting: boolean; + _pendingWorker: Promise; + abortController: AbortController | null; + component: ReturnType | null; + componentModule: Promise<{ mount: typeof mount }>; + getWorkflowParams(): void; + getItem( + id?: string + ): Lightning.TriggerNode | Lightning.JobNode | Lightning.Edge | undefined; + handleWorkflowParams(payload: { workflow_params: WorkflowProps }): void; + maybeMountComponent(): void; + onSelectionChange(id?: string): void; + pendingChanges: PendingAction[]; + processPendingChanges(): void; + pushPendingChange( + pendingChange: PendingAction, + abortController: AbortController + ): Promise; + workflowStore: ReturnType; + observer: MutationObserver | null; + setupObserver(): void; + baseUrl: string | null; + }, + { baseUrl: string | null } +>; const createNewWorkflow = () => { const triggers = [ @@ -61,6 +72,17 @@ const createNewWorkflow = () => { export default { mounted(this: WorkflowEditorEntrypoint) { + // Workaround for situations where the hook is mounted before the + // browser has updated window.location.href - it's rare, but it happens. + // Without it, the hook will try to push a history patch to the wrong + // URL. + const { baseUrl } = this.el.dataset; + if (!baseUrl) { + throw new Error('WorkflowEditor requires a data-base-url attribute'); + } + + this.baseUrl = baseUrl; + console.debug('WorkflowEditor hook mounted'); this._pendingWorker = Promise.resolve(); @@ -102,6 +124,26 @@ export default { // between the current state and the server state and send those diffs // to the server. }, + setupObserver() { + this.observer = new MutationObserver(mutations => { + mutations.forEach(mutation => { + const { attributeName, oldValue } = mutation as AttributeMutationRecord; + + if (attributeName == 'data-base-url') { + const newValue = this.el.getAttribute(attributeName); + + if (oldValue !== newValue) { + this.baseUrl = newValue; + } + } + }); + }); + + this.observer.observe(this.el, { + attributeFilter: ['data-base-url'], + attributeOldValue: true, + }); + }, getItem(id?: string) { if (id) { const { jobs, triggers, edges } = this.workflowStore.getState(); @@ -114,7 +156,7 @@ export default { } }, onSelectionChange(id?: string) { - const currentUrl = new URL(window.location.href); + const currentUrl = new URL(this.baseUrl!); const nextUrl = new URL(currentUrl); const idExists = this.getItem(id); @@ -143,13 +185,9 @@ export default { } }, destroyed() { - if (this.component) { - this.component.unmount(); - } - - if (this.abortController) { - this.abortController.abort(); - } + this.component?.unmount(); + this.abortController?.abort(); + this.observer?.disconnect(); console.debug('WorkflowEditor destroyed'); }, diff --git a/lib/lightning_web/live/workflow_live/edit.ex b/lib/lightning_web/live/workflow_live/edit.ex index 04fbf40e19..452159dbcf 100644 --- a/lib/lightning_web/live/workflow_live/edit.ex +++ b/lib/lightning_web/live/workflow_live/edit.ex @@ -67,6 +67,7 @@ defmodule LightningWeb.WorkflowLive.Edit do phx-hook="WorkflowEditor" class="grow" id={"editor-#{@workflow.id}"} + data-base-url={@current_url} phx-update="ignore" > <%!-- Before Editor component has mounted --%> @@ -312,6 +313,7 @@ defmodule LightningWeb.WorkflowLive.Edit do |> authorize() |> assign( active_menu_item: :projects, + current_url: nil, expanded_job: nil, follow_run_id: nil, page_title: "", @@ -319,9 +321,9 @@ defmodule LightningWeb.WorkflowLive.Edit do selected_job: nil, selected_trigger: nil, selection_mode: nil, + selection_params: %{"s" => nil, "m" => nil}, workflow: nil, - workflow_params: %{}, - selection_params: %{"s" => nil, "m" => nil} + workflow_params: %{} )} end @@ -330,7 +332,7 @@ defmodule LightningWeb.WorkflowLive.Edit do {:noreply, apply_action(socket, socket.assigns.live_action, params) |> apply_selection_params(params) - |> assign_url(url)} + |> assign(current_url: url)} end def apply_action(socket, :new, _params) do @@ -665,11 +667,6 @@ defmodule LightningWeb.WorkflowLive.Edit do end end - defp assign_url(socket, url) do - socket - |> assign(url: URI.parse(url)) - end - defp build_next_path(socket, workflow) do %{project: project, selection_params: selection_params} = socket.assigns