From 7db778c5de54724209d524317181032709b00270 Mon Sep 17 00:00:00 2001 From: "Elias W. BA" Date: Wed, 10 Jul 2024 10:56:38 +0000 Subject: [PATCH] Prevent simultaneous edits to a workflow (#2241) * Detect presence of other users in the workflow edit page for a specific workflow, and compute their priorities depending on the time their joined * Display banners and toggle edit/read mode * Refactor code and handle UI glitches * Test the whole feature * More tests * More tests * More tests * Update CHANGELOG.md * Disable Canvas * Refactor code for disabling canvas when in snapshot mode and / or in low priority mode * Remove few console.log * Fix type check error * Fix failing tests * Viewers are not promoted to high priority * Switch to latest version when promoting user to high priority, but handle the case of a real snapshot * The banner is pushing down the canvas which then pushes the zoom panel under the viewport * Test events in low priority mode, and fix regression / bug on deleting an edge * Fix infinite loop on reloading workflow * Restore button disabled * Handle code review change requests * Change flash messages language, less technical * Fix disabled regression after rebase * Repo.exists?/1 doesn't need limit * Constrain Workflow lock_version check to given workflow * fix up language * Slight improvement on test consistancy --------- Co-authored-by: Stuart Corbishley Co-authored-by: Taylor Downs --- CHANGELOG.md | 3 + .../js/workflow-diagram/WorkflowDiagram.tsx | 1 + .../components/ErrorMessage.tsx | 1 - assets/js/workflow-diagram/nodes/Job.tsx | 9 +- assets/js/workflow-diagram/types.ts | 3 +- .../workflow-diagram/util/from-workflow.tsx | 5 +- assets/js/workflow-editor/index.ts | 4 + assets/js/workflow-editor/store.ts | 9 + lib/lightning/application.ex | 1 + lib/lightning/workflows.ex | 5 + lib/lightning/workflows/presence.ex | 194 ++++++++ .../live/workflow_live/components.ex | 63 ++- lib/lightning_web/live/workflow_live/edit.ex | 309 ++++++++++-- .../live/workflow_live/job_view.ex | 35 +- .../live/workflow_live/workflow_params.ex | 27 +- .../live/workflow_live/edit_test.exs | 33 +- .../workflow_live/user_presences_test.exs | 451 ++++++++++++++++++ test/support/workflow_live_helpers.ex | 4 + test/test_helper.exs | 1 - 19 files changed, 1065 insertions(+), 93 deletions(-) create mode 100644 lib/lightning/workflows/presence.ex create mode 100644 test/lightning_web/live/workflow_live/user_presences_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9217fbc8c2..0c35462c38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ and this project adheres to ### Added +- Handle Simultaneous Editors Working In Same Workflow + [#1949](https://github.com/OpenFn/lightning/issues/1949) + ### Changed ### Fixed diff --git a/assets/js/workflow-diagram/WorkflowDiagram.tsx b/assets/js/workflow-diagram/WorkflowDiagram.tsx index b74f0b4c6d..78c0e1b526 100644 --- a/assets/js/workflow-diagram/WorkflowDiagram.tsx +++ b/assets/js/workflow-diagram/WorkflowDiagram.tsx @@ -68,6 +68,7 @@ export default React.forwardRef( jobs: state.jobs, triggers: state.triggers, edges: state.edges, + disabled: state.disabled, }), shallow ); diff --git a/assets/js/workflow-diagram/components/ErrorMessage.tsx b/assets/js/workflow-diagram/components/ErrorMessage.tsx index 88ea475654..66d25b12d0 100644 --- a/assets/js/workflow-diagram/components/ErrorMessage.tsx +++ b/assets/js/workflow-diagram/components/ErrorMessage.tsx @@ -3,7 +3,6 @@ import React from 'react'; import { ExclamationCircleIcon } from '@heroicons/react/24/outline'; const ErrorMessage: React.FC> = ({ children }) => { - console.log(children); return (

diff --git a/assets/js/workflow-diagram/nodes/Job.tsx b/assets/js/workflow-diagram/nodes/Job.tsx index 7133818480..abd234829a 100644 --- a/assets/js/workflow-diagram/nodes/Job.tsx +++ b/assets/js/workflow-diagram/nodes/Job.tsx @@ -14,11 +14,10 @@ const JobNode = ({ ...props }: NodeProps) => { const toolbar = () => [ - props.data?.allowPlaceholder && - !props.data?.disabled && [ - , - , - ], + props.data?.allowPlaceholder && [ + , + , + ], ]; const adaptorIconsData = useAdaptorIcons(); diff --git a/assets/js/workflow-diagram/types.ts b/assets/js/workflow-diagram/types.ts index 1d5965ace7..4c302090e3 100644 --- a/assets/js/workflow-diagram/types.ts +++ b/assets/js/workflow-diagram/types.ts @@ -6,7 +6,6 @@ export namespace Lightning { id: string; name: string; workflow_id: string; - disabled: boolean; // Not technically from Lightning, but we'll infer this and scribble it placeholder?: boolean; @@ -49,7 +48,6 @@ export namespace Lightning { error_path?: boolean; errors: any; condition_label?: string; - disabled: boolean; } export type Workflow = { @@ -58,6 +56,7 @@ export namespace Lightning { triggers: TriggerNode[]; jobs: JobNode[]; edges: Edge[]; + disabled: boolean; }; } diff --git a/assets/js/workflow-diagram/util/from-workflow.tsx b/assets/js/workflow-diagram/util/from-workflow.tsx index 791cff941d..bbb4a501e5 100644 --- a/assets/js/workflow-diagram/util/from-workflow.tsx +++ b/assets/js/workflow-diagram/util/from-workflow.tsx @@ -45,7 +45,8 @@ const fromWorkflow = ( placeholders: Flow.Model = { nodes: [], edges: [] }, selectedId: string | null ): Flow.Model => { - const allowPlaceholder = placeholders.nodes.length === 0; + const allowPlaceholder = + placeholders.nodes.length === 0 && !workflow.disabled; const process = ( items: Array, @@ -135,7 +136,7 @@ const fromWorkflow = ( const sortedEdges = edges.sort(sortOrderForSvg); - return { nodes, edges: sortedEdges }; + return { nodes, edges: sortedEdges, disabled: workflow.disabled }; }; export default fromWorkflow; diff --git a/assets/js/workflow-editor/index.ts b/assets/js/workflow-editor/index.ts index 3b4f854360..57769cfea1 100644 --- a/assets/js/workflow-editor/index.ts +++ b/assets/js/workflow-editor/index.ts @@ -108,6 +108,10 @@ export default { this.workflowStore.getState().applyPatches(response.patches); }); + this.handleEvent('set-disabled', (response: { disabled: boolean }) => { + this.workflowStore.getState().setDisabled(response.disabled); + }); + this.handleEvent<{ href: string; patch: boolean }>('navigate', e => { const id = new URL(window.location.href).searchParams.get('s'); diff --git a/assets/js/workflow-editor/store.ts b/assets/js/workflow-editor/store.ts index fcda311015..3337923da2 100644 --- a/assets/js/workflow-editor/store.ts +++ b/assets/js/workflow-editor/store.ts @@ -24,6 +24,7 @@ export type WorkflowProps = { triggers: Lightning.TriggerNode[]; jobs: Lightning.JobNode[]; edges: Lightning.Edge[]; + disabled: boolean; }; export interface WorkflowState extends WorkflowProps { @@ -36,6 +37,7 @@ export interface WorkflowState extends WorkflowProps { ) => T | undefined; onChange: (pendingAction: PendingAction) => void; applyPatches: (patches: Patch[]) => void; + setDisabled: (value: boolean) => void; } // Immer's Patch type has an array of strings for the path, but RFC 6902 @@ -74,6 +76,7 @@ export const createWorkflowStore = ( triggers: [], jobs: [], edges: [], + disabled: false, }; // Calculate the next state using Immer, and then call the onChange callback @@ -216,6 +219,12 @@ export const createWorkflowStore = ( set(state => applyPatches(state, immerPatches)); }, onChange, + setDisabled: (value: boolean) => { + set(state => ({ + ...state, + disabled: value, + })); + }, })); }; diff --git a/lib/lightning/application.ex b/lib/lightning/application.ex index 6b31a7d698..7f5e0e2449 100644 --- a/lib/lightning/application.ex +++ b/lib/lightning/application.ex @@ -101,6 +101,7 @@ defmodule Lightning.Application do auth_providers_cache_childspec, # Start the Endpoint (http/https) LightningWeb.Endpoint, + Lightning.Workflows.Presence, adaptor_registry_childspec, adaptor_service_childspec, {Lightning.TaskWorker, name: :cli_task_worker}, diff --git a/lib/lightning/workflows.ex b/lib/lightning/workflows.ex index b9a3852e5d..b2b26a1739 100644 --- a/lib/lightning/workflows.ex +++ b/lib/lightning/workflows.ex @@ -405,4 +405,9 @@ defmodule Lightning.Workflows do def jobs_ordered_subquery do from(j in Job, order_by: [asc: j.inserted_at]) end + + def has_newer_version?(%Workflow{lock_version: version, id: id}) do + from(w in Workflow, where: w.lock_version > ^version and w.id == ^id) + |> Repo.exists?() + end end diff --git a/lib/lightning/workflows/presence.ex b/lib/lightning/workflows/presence.ex new file mode 100644 index 0000000000..f01660cfff --- /dev/null +++ b/lib/lightning/workflows/presence.ex @@ -0,0 +1,194 @@ +defmodule Lightning.Workflows.Presence do + @moduledoc """ + Handles user presence tracking within the Workflow canvas page. + + This module leverages Phoenix.Presence to track user sessions, manage user priorities, + and list active presences on specified topics. + """ + use Phoenix.Presence, + otp_app: :lightning, + pubsub_server: Lightning.PubSub + + alias LightningWeb.Endpoint + + defstruct user: nil, joined_at: nil, active_sessions: 0 + + @doc """ + Creates a new `UserPresence` struct. + + ## Parameters + + - `user`: The user data to be included in the presence. + - `joined_at`: The timestamp when the user joined, in microseconds. + - `active_sessions`: The number of active sessions for the user (default is 0). + + ## Examples + + iex> Lightning.Workflows.Presence.new_user_presence(%User{id: 1}, 1625597762000000) + %Lightning.Workflows.Presence{ + user: %User{id: 1}, + joined_at: 1625597762000000, + active_sessions: 0 + } + + """ + def new_user_presence(user, joined_at, active_sessions \\ 0) do + %__MODULE__{ + user: user, + joined_at: joined_at, + active_sessions: active_sessions + } + end + + @doc """ + Tracks the presence of a user on a given topic. + + ## Parameters + + - `user`: The user to be tracked. + - `topic`: The topic to track the user on. + - `pid`: The process identifier for the user's session. + + ## Examples + + iex> Lightning.Workflows.Presence.track_user_presence(%User{id: 1}, "room:lobby", self()) + :ok + + """ + def track_user_presence(user, topic, pid) do + joined_at = System.system_time(:microsecond) + + track(pid, topic, user.id, %{ + user: user, + joined_at: joined_at + }) + + Endpoint.subscribe(topic) + end + + @doc """ + Lists all presences for a given topic. + + ## Parameters + + - `topic`: The topic to list the presences for. + + ## Examples + + iex> Lightning.Workflows.Presence.list_presences("workflow:canvas") + [%Lightning.Workflows.Presence{user: %User{id: 1}, ...}, ...] + + """ + def list_presences(topic) do + topic + |> list_presences_by_topic() + |> group_presences_by_user() + |> extract_presences() + end + + @doc """ + Builds a summary of presences with details about the current user's presence, promotable presences, + and edit priority. + + ## Parameters + + - `presences` (list): A list of presence records, each containing user information and a joined_at timestamp. + - `params` (map): A map containing the following keys: + - `:current_user_presence` - The presence record for the current user. + - `:current_user` - The current user record. + - `:view_only_users_ids` - A list of user IDs who have view-only permissions. + + ## Returns + + - `map`: A map containing the following keys: + - `:presences` - The sorted list of all presences. + - `:prior_user_presence` - The presence record with edit priority. + - `:current_user_presence` - The presence record for the current user. + - `:has_presence_edit_priority` - A boolean indicating if the current user has edit priority. + + ## Examples + + iex> presences = [ + ...> %{user: %{id: 1}, joined_at: ~N[2024-07-03 12:00:00], active_sessions: 1}, + ...> %{user: %{id: 2}, joined_at: ~N[2024-07-03 12:05:00], active_sessions: 1}, + ...> %{user: %{id: 3}, joined_at: ~N[2024-07-03 12:10:00], active_sessions: 1} + ...> ] + iex> params = %{ + ...> current_user_presence: %{user: %{id: 1}, joined_at: ~N[2024-07-03 12:00:00], active_sessions: 1}, + ...> current_user: %{id: 1}, + ...> view_only_users_ids: [2] + ...> } + iex> build_presences_summary(presences, params) + %{ + presences: [ + %{user: %{id: 1}, joined_at: ~N[2024-07-03 12:00:00], active_sessions: 1}, + %{user: %{id: 2}, joined_at: ~N[2024-07-03 12:05:00], active_sessions: 1}, + %{user: %{id: 3}, joined_at: ~N[2024-07-03 12:10:00], active_sessions: 1} + ], + prior_user_presence: %{user: %{id: 3}, joined_at: ~N[2024-07-03 12:10:00], active_sessions: 1}, + current_user_presence: %{user: %{id: 1}, joined_at: ~N[2024-07-03 12:00:00], active_sessions: 1}, + has_presence_edit_priority: true + } + + """ + def build_presences_summary(presences, params) do + %{ + current_user_presence: current_user_presence, + current_user: current_user, + view_only_users_ids: view_only_users_ids + } = params + + presences = Enum.sort_by(presences, & &1.joined_at) + + current_user_presence = + Enum.find(presences, current_user_presence, fn presence -> + presence.user.id == current_user.id + end) + + presences_promotable = + Enum.reject(presences, fn presence -> + presence.user.id in view_only_users_ids + end) + + prior_user_presence = + if length(presences_promotable) > 0 do + List.first(presences_promotable) + else + current_user_presence + end + + has_presence_edit_priority = + current_user_presence.user.id == prior_user_presence.user.id && + current_user_presence.active_sessions <= 1 + + %{ + presences: presences, + prior_user_presence: prior_user_presence, + current_user_presence: current_user_presence, + has_presence_edit_priority: has_presence_edit_priority + } + end + + defp list_presences_by_topic(topic) do + list(topic) + |> Enum.flat_map(fn {_user_id, %{metas: metas}} -> metas end) + end + + defp group_presences_by_user(presences) do + Enum.group_by(presences, & &1.user.id) + end + + defp extract_presences(grouped_presences) do + grouped_presences + |> Enum.map(fn {_id, group} -> + active_sessions = length(group) + presence = List.first(group) + + new_user_presence( + presence.user, + presence.joined_at, + active_sessions + ) + end) + end +end diff --git a/lib/lightning_web/live/workflow_live/components.ex b/lib/lightning_web/live/workflow_live/components.ex index 5ac80b9f7a..404cd15e52 100644 --- a/lib/lightning_web/live/workflow_live/components.ex +++ b/lib/lightning_web/live/workflow_live/components.ex @@ -484,6 +484,7 @@ defmodule LightningWeb.WorkflowLive.Components do end attr :form, :map, required: true + attr :disabled, :boolean, default: false def workflow_name_field(assigns) do ~H""" @@ -497,7 +498,7 @@ defmodule LightningWeb.WorkflowLive.Components do >

- <.text_input form={f} has_errors={f.errors[:name]} /> + <.text_input form={f} has_errors={f.errors[:name]} disabled={@disabled} /> <%= if f.errors[:name] do %> @@ -531,7 +532,8 @@ defmodule LightningWeb.WorkflowLive.Components do :name, class: @classes, required: true, - placeholder: "Untitled" + placeholder: "Untitled", + disabled: @disabled ) %>
@@ -730,4 +732,61 @@ defmodule LightningWeb.WorkflowLive.Components do """ end + + def workflow_info_banner(assigns) do + ~H""" +
+
+
+ +
+
+

+ <%= @message %> +

+
+
+
+ """ + end + + def online_users(assigns) do + ~H""" +
+ <.render_user + :for={%{user: online_user} <- @presences} + :if={online_user.id != @current_user.id} + id={"#{@id}-#{online_user.id}"} + user={online_user} + prior={@prior_user.id == online_user.id} + /> +
+ """ + end + + defp render_user(assigns) do + ~H""" + + + <%= user_name(@user) %> + + + """ + end + + defp user_name(user) do + String.at(user.first_name, 0) <> + if is_nil(user.last_name), + do: "", + else: String.at(user.last_name, 0) + end end diff --git a/lib/lightning_web/live/workflow_live/edit.ex b/lib/lightning_web/live/workflow_live/edit.ex index c45fd47931..57e9704425 100644 --- a/lib/lightning_web/live/workflow_live/edit.ex +++ b/lib/lightning_web/live/workflow_live/edit.ex @@ -20,6 +20,7 @@ defmodule LightningWeb.WorkflowLive.Edit do alias Lightning.Services.UsageLimiter alias Lightning.Workflows alias Lightning.Workflows.Job + alias Lightning.Workflows.Presence alias Lightning.Workflows.Snapshot alias Lightning.Workflows.Trigger alias Lightning.Workflows.Workflow @@ -27,6 +28,7 @@ defmodule LightningWeb.WorkflowLive.Edit do alias LightningWeb.Components.Form alias LightningWeb.WorkflowLive.Helpers alias LightningWeb.WorkflowNewLive.WorkflowParams + alias Phoenix.LiveView.JS require Lightning.Run @@ -49,7 +51,16 @@ defmodule LightningWeb.WorkflowLive.Edit do ~p"/projects/#{assigns.project}/w/#{assigns.workflow}" end, workflow_form: to_form(assigns.changeset), - save_and_run_disabled: save_and_run_disabled?(assigns) + save_and_run_disabled: save_and_run_disabled?(assigns), + display_banner: + !assigns.has_presence_edit_priority && + assigns.current_user.id not in assigns.view_only_users_ids && + assigns.snapshot_version_tag == "latest", + banner_message: + banner_message( + assigns.current_user_presence, + assigns.prior_user_presence + ) ) ~H""" @@ -57,7 +68,14 @@ defmodule LightningWeb.WorkflowLive.Edit do <:header> <:title> - <.workflow_name_field form={@workflow_form} /> + <.workflow_name_field + form={@workflow_form} + disabled={ + !@can_edit_workflow or + @snapshot_version_tag != "latest" or + !@has_presence_edit_priority + } + />
+
+ +
-
+
@@ -88,7 +117,7 @@ defmodule LightningWeb.WorkflowLive.Edit do type="button" phx-click="switch-version" phx-value-type="commit" - color_class="text-white bg-primary-600 hover:bg-primary-700" + color_class="text-white bg-primary-600 hover:bg-primary-700 disabled:bg-primary-300" > Switch to latest version @@ -109,11 +138,13 @@ defmodule LightningWeb.WorkflowLive.Edit do changeset={@changeset} can_edit_workflow={@can_edit_workflow} snapshot_version_tag={@snapshot_version_tag} + has_presence_priority={@has_presence_edit_priority} />
+
<%!-- Job Edit View --%>
@@ -132,6 +163,10 @@ defmodule LightningWeb.WorkflowLive.Edit do snapshot={@snapshot} snapshot_version={@snapshot_version_tag} current_user={@current_user} + display_banner={@display_banner} + banner_message={@banner_message} + presences={@presences} + prior_user_presence={@prior_user_presence} project={@project} socket={@socket} follow_run_id={@follow_run && @follow_run.id} @@ -172,7 +207,8 @@ defmodule LightningWeb.WorkflowLive.Edit do dataclips={@selectable_dataclips} disabled={ !@can_run_workflow || - @snapshot_version_tag != "latest" + @snapshot_version_tag != "latest" || + !@has_presence_edit_priority } project={@project} admin_contacts={@admin_contacts} @@ -206,7 +242,10 @@ defmodule LightningWeb.WorkflowLive.Edit do :if={display_switcher(@snapshot, @workflow)} id={@selected_job.id} label="Latest Version" - disabled={job_deleted?(@selected_job, @workflow)} + disabled={ + job_deleted?(@selected_job, @workflow) || + !@has_presence_edit_priority + } version={@snapshot_version_tag} /> @@ -241,7 +280,8 @@ defmodule LightningWeb.WorkflowLive.Edit do selected_dataclip_wiped?( @manual_run_form, @selectable_dataclips - ) || @snapshot_version_tag != "latest" + ) || @snapshot_version_tag != "latest" || + !@has_presence_edit_priority } > <%= if processing(@follow_run) do %> @@ -277,7 +317,8 @@ defmodule LightningWeb.WorkflowLive.Edit do aria-haspopup="true" disabled={ @save_and_run_disabled || - @snapshot_version_tag != "latest" + @snapshot_version_tag != "latest" || + !@has_presence_edit_priority } phx-click={show_dropdown("create-new-work-order")} > @@ -314,7 +355,8 @@ defmodule LightningWeb.WorkflowLive.Edit do form={@manual_run_form.id} disabled={ @save_and_run_disabled || - @snapshot_version_tag != "latest" + @snapshot_version_tag != "latest" || + !@has_presence_edit_priority } > <.icon name="hero-play-solid" class="w-4 h-4 mr-1" /> @@ -328,6 +370,7 @@ defmodule LightningWeb.WorkflowLive.Edit do changeset={@changeset} can_edit_workflow={@can_edit_workflow} snapshot_version_tag={@snapshot_version_tag} + has_presence_priority={@has_presence_edit_priority} />
@@ -394,6 +437,12 @@ defmodule LightningWeb.WorkflowLive.Edit do ~p"/projects/#{@project.id}/w/#{@workflow.id}?s=#{@selected_job.id}" } /> + <.workflow_info_banner + :if={@display_banner} + id={"canvas-banner-#{@current_user.id}"} + position="absolute" + message={@banner_message} + /> <.form id="workflow-form" for={@workflow_form} @@ -424,8 +473,8 @@ defmodule LightningWeb.WorkflowLive.Edit do <.job_form on_change={&send_form_changed/1} editable={ - @can_edit_workflow && - @snapshot_version_tag == "latest" + @can_edit_workflow && @snapshot_version_tag == "latest" && + @has_presence_edit_priority } form={jf} project_user={@project_user} @@ -450,7 +499,8 @@ defmodule LightningWeb.WorkflowLive.Edit do class="focus:ring-red-500 bg-red-600 hover:bg-red-700 disabled:bg-red-300" disabled={ !@can_edit_workflow or @has_child_edges or @is_first_job or - @snapshot_version_tag != "latest" + @snapshot_version_tag != "latest" || + !@has_presence_edit_priority } tooltip={ deletion_tooltip_message( @@ -496,7 +546,8 @@ defmodule LightningWeb.WorkflowLive.Edit do on_change={&send_form_changed/1} disabled={ !@can_edit_workflow or - @snapshot_version_tag != "latest" + @snapshot_version_tag != "latest" || + !@has_presence_edit_priority } can_write_webhook_auth_method={@can_write_webhook_auth_method} webhook_url={webhook_url(@selected_trigger)} @@ -525,7 +576,8 @@ defmodule LightningWeb.WorkflowLive.Edit do form={ef} disabled={ !@can_edit_workflow or - @snapshot_version_tag != "latest" + @snapshot_version_tag != "latest" || + !@has_presence_edit_priority } cancel_url={close_url(assigns, :selected_edge, :unselect)} /> @@ -554,6 +606,22 @@ defmodule LightningWeb.WorkflowLive.Edit do """ end + defp banner_message(current_user_presence, prior_user_presence) do + prior_user_name = + "#{prior_user_presence.user.first_name} #{prior_user_presence.user.last_name}" + + cond do + current_user_presence.active_sessions > 1 -> + "You can't edit this workflow because you have #{current_user_presence.active_sessions} active sessions. Close your other sessions to enable editing." + + current_user_presence.user.id != prior_user_presence.user.id -> + "This workflow is currently locked for editing because a collaborator (#{prior_user_name}) is currently working on it. You can inspect this workflow and its associated steps but cannot make edits." + + true -> + nil + end + end + @spec close_url(map(), atom(), atom()) :: String.t() defp close_url(assigns, type, selection) do query_params = %{ @@ -856,10 +924,14 @@ defmodule LightningWeb.WorkflowLive.Edit do @impl true def mount(_params, _session, %{assigns: assigns} = socket) do + view_only_users_ids = + assigns.project |> view_only_users() |> Enum.map(fn pu -> pu.user.id end) + {:ok, socket |> authorize() |> assign( + view_only_users_ids: view_only_users_ids, active_menu_item: :overview, expanded_job: nil, follow_run: nil, @@ -882,13 +954,15 @@ defmodule LightningWeb.WorkflowLive.Edit do oauth_clients: OauthClients.list_clients(assigns.project), show_wiped_dataclip_selector: false, admin_contacts: Projects.list_project_admin_emails(assigns.project.id) - )} + ) + |> assign(initial_presence_summary(socket.assigns.current_user))} end @impl true def handle_params(params, _url, socket) do {:noreply, apply_action(socket, socket.assigns.live_action, params) + |> track_user_presence() |> apply_query_params(params) |> maybe_show_manual_run() |> tap(fn socket -> @@ -927,15 +1001,7 @@ defmodule LightningWeb.WorkflowLive.Edit do _ -> # TODO we shouldn't be calling Repo from here - workflow = - Workflows.get_workflow(workflow_id) - |> Lightning.Repo.preload([ - :edges, - triggers: Trigger.with_auth_methods_query(), - jobs: - {Workflows.jobs_ordered_subquery(), - [:credential, steps: Invocation.Query.any_step()]} - ]) + workflow = get_workflow_by_id(workflow_id) if workflow do run_id = Map.get(params, "a") @@ -955,6 +1021,24 @@ defmodule LightningWeb.WorkflowLive.Edit do end end + defp track_user_presence(socket) do + if connected?(socket) && socket.assigns.snapshot_version_tag == "latest" do + Presence.track_user_presence( + socket.assigns.current_user, + "workflow-#{socket.assigns.workflow.id}:presence", + self() + ) + end + + socket + end + + defp view_only_users(project) do + Lightning.Repo.preload(project, project_users: [:user]) + |> Map.get(:project_users) + |> Enum.filter(fn pu -> pu.role == :viewer end) + end + defp snapshot_by_version(workflow_id, version), do: Snapshot.get_by_version(workflow_id, version) @@ -1000,11 +1084,20 @@ defmodule LightningWeb.WorkflowLive.Edit do url = ~p"/projects/#{project.id}/w/#{workflow.id}?#{query_params}" + if version != "latest" do + Presence.untrack( + self(), + "workflow-#{socket.assigns.workflow.id}:presence", + socket.assigns.current_user.id + ) + end + socket |> assign(changeset: next_changeset) |> assign(workflow_params: next_params) |> assign(snapshot_version_tag: version) |> push_event("patches-applied", %{patches: patches}) + |> maybe_disable_canvas() |> push_patch(to: url) end end @@ -1022,7 +1115,8 @@ defmodule LightningWeb.WorkflowLive.Edit do patches = WorkflowParams.to_patches(prev_params, next_params) - lock_version = Ecto.Changeset.get_field(next_changeset, :lock_version) + lock_version = + Ecto.Changeset.get_field(next_changeset, :lock_version) query_params = socket.assigns.query_params @@ -1067,13 +1161,17 @@ defmodule LightningWeb.WorkflowLive.Edit do can_edit_workflow: can_edit_workflow, has_child_edges: has_child_edges, is_first_job: is_first_job, - snapshot_version_tag: tag + snapshot_version_tag: tag, + has_presence_edit_priority: has_presence_edit_priority } = socket.assigns with true <- can_edit_workflow || :not_authorized, true <- !has_child_edges || :has_child_edges, true <- !is_first_job || :is_first_job, - true <- tag == "latest" || :view_only do + true <- tag == "latest" || :view_only, + true <- + has_presence_edit_priority || + :presence_low_priority do edges_to_delete = Ecto.Changeset.get_assoc(changeset, :edges, :struct) |> Enum.filter(&(&1.target_job_id == id)) @@ -1105,7 +1203,15 @@ defmodule LightningWeb.WorkflowLive.Edit do socket |> put_flash( :error, - "Cannot delete a node in snapshot mode, switch to latest" + "Cannot delete a step in snapshot mode, switch to latest" + )} + + :presence_low_priority -> + {:noreply, + socket + |> put_flash( + :error, + "Cannot delete a step in view-only mode" )} end end @@ -1116,14 +1222,18 @@ defmodule LightningWeb.WorkflowLive.Edit do workflow_params: initial_params, can_edit_workflow: can_edit_workflow, selected_edge: selected_edge, - snapshot_version_tag: tag + snapshot_version_tag: tag, + has_presence_edit_priority: has_presence_edit_priority } = socket.assigns with true <- can_edit_workflow || :not_authorized, true <- (selected_edge && is_nil(selected_edge.source_trigger_id)) || :is_initial_edge, - true <- tag == "latest" || :view_only do + true <- tag == "latest" || :view_only, + true <- + has_presence_edit_priority || + :presence_low_priority do edges_to_delete = Ecto.Changeset.get_assoc(changeset, :edges, :struct) |> Enum.filter(&(&1.id == id)) @@ -1152,6 +1262,14 @@ defmodule LightningWeb.WorkflowLive.Edit do :error, "Cannot delete an edge in snapshot mode, switch to latest" )} + + :presence_low_priority -> + {:noreply, + socket + |> put_flash( + :error, + "Cannot delete an edge in view-only mode" + )} end end @@ -1178,12 +1296,16 @@ defmodule LightningWeb.WorkflowLive.Edit do project: project, workflow_params: initial_params, can_edit_workflow: can_edit_workflow, - snapshot_version_tag: tag + snapshot_version_tag: tag, + has_presence_edit_priority: has_presence_edit_priority } = socket.assigns with true <- can_edit_workflow || :not_authorized, - true <- tag == "latest" || :view_only do + true <- tag == "latest" || :view_only, + true <- + has_presence_edit_priority || + :presence_low_priority do next_params = case params do %{"workflow" => params} -> @@ -1238,6 +1360,14 @@ defmodule LightningWeb.WorkflowLive.Edit do "Cannot save in snapshot mode, switch to the latest version." )} + :presence_low_priority -> + {:noreply, + socket + |> put_flash( + :error, + "Cannot save in view-only mode" + )} + :not_authorized -> {:noreply, socket @@ -1310,11 +1440,15 @@ defmodule LightningWeb.WorkflowLive.Edit do current_user: current_user, changeset: changeset, project: %{id: project_id}, - snapshot_version_tag: tag + snapshot_version_tag: tag, + has_presence_edit_priority: has_presence_edit_priority } = socket.assigns with true <- can_run_workflow? || :not_authorized, true <- tag == "latest" || :view_only, + true <- + has_presence_edit_priority || + :presence_low_priority, :ok <- UsageLimiter.limit_action(%Action{type: :new_run}, %Context{ project_id: project_id @@ -1339,7 +1473,7 @@ defmodule LightningWeb.WorkflowLive.Edit do {:error, %{text: message}} -> {:noreply, put_flash(socket, :error, message)} - {:error, changeset} -> + {:error, _changeset} -> { :noreply, socket @@ -1357,6 +1491,11 @@ defmodule LightningWeb.WorkflowLive.Edit do {:noreply, socket |> put_flash(:error, "Cannot rerun in snapshot mode, switch to latest.")} + + :presence_low_priority -> + {:noreply, + socket + |> put_flash(:error, "Cannot rerun in view-only mode")} end end @@ -1370,13 +1509,17 @@ defmodule LightningWeb.WorkflowLive.Edit do workflow_params: workflow_params, can_edit_workflow: can_edit_workflow, can_run_workflow: can_run_workflow, - snapshot_version_tag: tag + snapshot_version_tag: tag, + has_presence_edit_priority: has_presence_edit_priority } = socket.assigns socket = socket |> apply_params(workflow_params, :workflow) with true <- (can_run_workflow && can_edit_workflow) || :not_authorized, true <- tag == "latest" || :view_only, + true <- + has_presence_edit_priority || + :presence_low_priority, {:ok, %{workorder: workorder, workflow: workflow}} <- Helpers.save_and_run( socket.assigns.changeset, @@ -1421,6 +1564,11 @@ defmodule LightningWeb.WorkflowLive.Edit do socket |> put_flash(:error, "Cannot run in snapshot mode, switch to latest.")} + :presence_low_priority -> + {:noreply, + socket + |> put_flash(:error, "Cannot run in view-only mode")} + {:error, %{text: message}} -> {:noreply, put_flash(socket, :error, message)} end @@ -1469,8 +1617,82 @@ defmodule LightningWeb.WorkflowLive.Edit do |> assign(follow_run: run)} end + def handle_info(%{event: "presence_diff", payload: _diff}, socket) do + summary = + "workflow-#{socket.assigns.workflow.id}:presence" + |> Presence.list_presences() + |> Presence.build_presences_summary(socket.assigns) + + {:noreply, + socket + |> assign(summary) + |> maybe_switch_workflow_version() + |> maybe_disable_canvas()} + end + def handle_info(%{}, socket), do: {:noreply, socket} + defp maybe_disable_canvas(socket) do + %{ + has_presence_edit_priority: has_edit_priority, + snapshot_version_tag: version, + can_edit_workflow: can_edit_workflow + } = socket.assigns + + disabled = !(has_edit_priority && version == "latest" && can_edit_workflow) + + push_event(socket, "set-disabled", %{disabled: disabled}) + end + + defp maybe_switch_workflow_version(socket) do + %{ + workflow: workflow, + prior_user_presence: prior_presence, + current_user: current_user, + selected_run: selected_run + } = socket.assigns + + if prior_presence.user.id == current_user.id && + Workflows.has_newer_version?(workflow) do + reloaded_workflow = get_workflow_by_id(workflow.id) + + socket = assign(socket, workflow: reloaded_workflow) + + if selected_run do + toggle_latest_version(socket) + else + commit_latest_version(socket) + end + else + socket + end + end + + defp get_workflow_by_id(workflow_id) do + Workflows.get_workflow(workflow_id) + |> Lightning.Repo.preload([ + :edges, + triggers: Trigger.with_auth_methods_query(), + jobs: + {Workflows.jobs_ordered_subquery(), + [:credential, steps: Invocation.Query.any_step()]} + ]) + end + + defp initial_presence_summary(current_user) do + init_user_presence = %Presence{ + user: current_user, + active_sessions: 1 + } + + %{ + presences: [], + prior_user_presence: init_user_presence, + current_user_presence: init_user_presence, + has_presence_edit_priority: true + } + end + defp maybe_show_manual_run(socket) do case socket.assigns do %{selected_job: nil} -> @@ -1662,7 +1884,7 @@ defmodule LightningWeb.WorkflowLive.Edit do end defp assign_workflow(socket, workflow, snapshot) do - {changeset, snapshot_version_tag} = + {changeset, version} = if snapshot.lock_version == workflow.lock_version do {Ecto.Changeset.change(workflow), "latest"} else @@ -1672,8 +1894,9 @@ defmodule LightningWeb.WorkflowLive.Edit do socket |> assign(workflow: workflow) |> assign(snapshot: snapshot) - |> assign(snapshot_version_tag: snapshot_version_tag) + |> assign(snapshot_version_tag: version) |> assign_changeset(changeset) + |> maybe_disable_canvas() end defp apply_params(socket, params, type) do @@ -1911,14 +2134,14 @@ defmodule LightningWeb.WorkflowLive.Edit do attr :can_edit_workflow, :boolean, required: true attr :changeset, Ecto.Changeset, required: true attr :snapshot_version_tag, :string, required: true + attr :has_presence_priority, :boolean, required: true defp save_workflow_button(assigns) do - alias Phoenix.LiveView.JS - %{ can_edit_workflow: can_edit_workflow, changeset: changeset, - snapshot_version_tag: snapshot_version_tag + snapshot_version_tag: snapshot_version_tag, + has_presence_priority: has_presence_priority } = assigns assigns = @@ -1926,7 +2149,7 @@ defmodule LightningWeb.WorkflowLive.Edit do |> assign( disabled: !can_edit_workflow or !changeset.valid? or - snapshot_version_tag != "latest" + snapshot_version_tag != "latest" || !has_presence_priority ) ~H""" @@ -1935,7 +2158,9 @@ defmodule LightningWeb.WorkflowLive.Edit do disabled={@disabled} form="workflow-form" phx-disconnected={JS.set_attribute({"disabled", ""})} - phx-connected={!@disabled && JS.remove_attribute("disabled")} + phx-connected={ + !@disabled && !@has_presence_priority && JS.remove_attribute("disabled") + } > Save diff --git a/lib/lightning_web/live/workflow_live/job_view.ex b/lib/lightning_web/live/workflow_live/job_view.ex index 16f52d0868..3333c55ed5 100644 --- a/lib/lightning_web/live/workflow_live/job_view.ex +++ b/lib/lightning_web/live/workflow_live/job_view.ex @@ -62,6 +62,10 @@ defmodule LightningWeb.WorkflowLive.JobView do attr :follow_run_id, :any, default: nil attr :snapshot, :any, required: true attr :snapshot_version, :any, required: true + attr :display_banner, :boolean, default: false + attr :banner_message, :string, default: "" + attr :presences, :list, required: true + attr :prior_user_presence, :any, required: true slot :footer @@ -73,7 +77,7 @@ defmodule LightningWeb.WorkflowLive.JobView do def job_edit_view(assigns) do {editor_disabled?, editor_disabled_message, editor_panel_title} = - editor_disabled?(assigns.form.source.data) + editor_disabled?(assigns) assigns = assigns @@ -112,6 +116,12 @@ defmodule LightningWeb.WorkflowLive.JobView do "You are viewing a snapshot of this workflow that was taken on #{Lightning.Helpers.format_date(@snapshot.inserted_at)}" } /> +
<.offline_indicator /> <.link @@ -131,6 +141,12 @@ defmodule LightningWeb.WorkflowLive.JobView do
+ <%= for slot <- @collapsible_panel do %> <.collapsible_panel @@ -207,12 +223,19 @@ defmodule LightningWeb.WorkflowLive.JobView do """ end - defp editor_disabled?(%Lightning.Workflows.Job{}), do: {false, "", "Editor"} + defp editor_disabled?(params) do + cond do + is_struct(params.form.source.data, Lightning.Workflows.Snapshot.Job) -> + {true, "Cannot edit in snapshot mode, switch to the latest version.", + "Editor (read-only)"} - defp editor_disabled?(%Lightning.Workflows.Snapshot.Job{}), - do: - {true, "Cannot edit in snapshot mode, switch to the latest version.", - "Editor (read-only)"} + params.display_banner -> + {true, "Cannot edit in low priority access.", "Editor (read-only)"} + + true -> + {false, "", "Editor"} + end + end defp credential_block(assigns) do ~H""" diff --git a/lib/lightning_web/live/workflow_live/workflow_params.ex b/lib/lightning_web/live/workflow_live/workflow_params.ex index aa3eb711d0..416ae97c67 100644 --- a/lib/lightning_web/live/workflow_live/workflow_params.ex +++ b/lib/lightning_web/live/workflow_live/workflow_params.ex @@ -70,28 +70,19 @@ defmodule LightningWeb.WorkflowNewLive.WorkflowParams do contain atom values. """ @spec to_map(Ecto.Changeset.t()) :: %{String.t() => any()} - def to_map(%Ecto.Changeset{data: %Lightning.Workflows.Snapshot{}} = changeset) do - changeset - |> to_serializable(&Ecto.Changeset.get_embed/2) - |> add_disabled_to_nested(true) - end - def to_map(%Ecto.Changeset{} = changeset) do - changeset - |> to_serializable(&Ecto.Changeset.get_assoc/2) - |> add_disabled_to_nested(false) + changeset |> to_serializable(embed_or_assoc_getter(changeset)) end - defp add_disabled_to_nested(map, disabled_value) do - map - |> Map.update("jobs", [], &add_disabled_to_list(&1, disabled_value)) - |> Map.update("edges", [], &add_disabled_to_list(&1, disabled_value)) - |> Map.update("triggers", [], &add_disabled_to_list(&1, disabled_value)) - end + defp embed_or_assoc_getter(%Ecto.Changeset{ + data: %Lightning.Workflows.Snapshot{} + }), + do: &Ecto.Changeset.get_embed/2 - defp add_disabled_to_list(list, disabled_value) do - Enum.map(list, fn item -> Map.put(item, "disabled", disabled_value) end) - end + defp embed_or_assoc_getter(%Ecto.Changeset{ + data: %Lightning.Workflows.Workflow{} + }), + do: &Ecto.Changeset.get_assoc/2 defp to_serializable(%Ecto.Changeset{} = changeset, accessor) when is_function(accessor) do diff --git a/test/lightning_web/live/workflow_live/edit_test.exs b/test/lightning_web/live/workflow_live/edit_test.exs index 2f3629f2d1..71bbc438e6 100644 --- a/test/lightning_web/live/workflow_live/edit_test.exs +++ b/test/lightning_web/live/workflow_live/edit_test.exs @@ -9,11 +9,11 @@ defmodule LightningWeb.WorkflowLive.EditTest do import Ecto.Query alias Lightning.Helpers - alias Lightning.Workflows.Snapshot - alias Lightning.Workflows alias Lightning.Repo - alias LightningWeb.CredentialLiveHelpers + alias Lightning.Workflows + alias Lightning.Workflows.Snapshot alias Lightning.Workflows.Workflow + alias LightningWeb.CredentialLiveHelpers setup :register_and_log_in_user setup :create_project_for_current_user @@ -104,7 +104,6 @@ defmodule LightningWeb.WorkflowLive.EditTest do "name" => ["Job name can't be blank."] } }, - %{op: "add", path: "/jobs/0/disabled", value: false}, %{op: "add", path: "/jobs/0/body", value: ""}, %{ op: "add", @@ -363,6 +362,10 @@ defmodule LightningWeb.WorkflowLive.EditTest do version = String.slice(snapshot.id, 0..6) + view + |> element("a[href='/projects/#{project.id}/w']", "Workflows") + |> render_click() + {:ok, view, _html} = live( conn, @@ -451,20 +454,22 @@ defmodule LightningWeb.WorkflowLive.EditTest do last_job = List.last(snapshot.jobs) last_edge = List.last(snapshot.edges) - force_event(view, :save) =~ - "Cannot save in snapshot mode, switch to the latest version." + assert force_event(view, :save) =~ + "Cannot save in snapshot mode, switch to the latest version." + + assert force_event(view, :delete_node, last_job) =~ + "Cannot delete a step in snapshot mode, switch to latest" - force_event(view, :delete_node, last_job) =~ - "Cannot delete a node in snapshot mode, switch to latest" + view |> select_node(last_edge, snapshot.lock_version) - force_event(view, :delete_edge, last_edge) =~ - "Cannot delete a node in snapshot mode, switch to latest" + assert force_event(view, :delete_edge, last_edge) =~ + "Cannot delete an edge in snapshot mode, switch to latest" - force_event(view, :manual_run_submit, %{}) =~ - "Cannot run in snapshot mode, switch to latest." + assert force_event(view, :manual_run_submit, %{}) =~ + "Cannot run in snapshot mode, switch to latest." - force_event(view, :rerun, nil, nil) =~ - "Cannot rerun in snapshot mode, switch to latest." + assert force_event(view, :rerun, nil, nil) =~ + "Cannot rerun in snapshot mode, switch to latest." assert view |> element( diff --git a/test/lightning_web/live/workflow_live/user_presences_test.exs b/test/lightning_web/live/workflow_live/user_presences_test.exs new file mode 100644 index 0000000000..d69bf765d0 --- /dev/null +++ b/test/lightning_web/live/workflow_live/user_presences_test.exs @@ -0,0 +1,451 @@ +defmodule LightningWeb.WorkflowLive.UserPresencesTest do + use LightningWeb.ConnCase, async: true + + import Phoenix.LiveViewTest + import Lightning.WorkflowLive.Helpers + import Lightning.Factories + + describe "When only one visitor, no presence is detected" do + test "in canvas", %{conn: conn} do + amy = + insert(:user, + email: "amy@openfn.org", + first_name: "Amy", + last_name: "Ly" + ) + + project = + insert(:project, + project_users: [ + %{user: amy, role: :owner} + ] + ) + + %{workflow: workflow} = create_workflow(%{project: project}) + + amy_session = log_in_user(conn, amy) + + {:ok, amy_view, _html} = + live( + amy_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version]}" + ) + + # Neither banner nor online users components are rendered in Canvas + refute amy_view |> has_element?("#canvas-online-users-#{amy.id}") + refute amy_view |> has_element?("#canvas-banner") + + # We do not render current user in the list of online users + refute render(amy_view) =~ amy.first_name + end + + test "in inspector", %{conn: conn} do + amy = + insert(:user, + email: "amy@openfn.org", + first_name: "Amy", + last_name: "Ly" + ) + + project = + insert(:project, + project_users: [ + %{user: amy, role: :owner} + ] + ) + + %{workflow: %{jobs: [job | _]} = workflow} = + create_workflow(%{project: project}) + + amy_session = log_in_user(conn, amy) + + {:ok, amy_view, _html} = + live( + amy_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version, s: job.id, m: "expand"]}" + ) + + # Neither banner nor online users components are rendered in Canvas + refute amy_view |> has_element?("#inspector-online-users-#{amy.id}") + refute amy_view |> has_element?("#inspector-banner") + + # We do not render current user in the list of online users + refute render(amy_view) =~ amy.first_name + end + end + + describe "When workflow has many visitors, presences is detected and edit is disabled" do + test "in canvas", %{conn: conn} do + amy = + insert(:user, + email: "amy@openfn.org", + first_name: "Amy", + last_name: "Ly" + ) + + ana = + insert(:user, + email: "ana@openfn.org", + first_name: "Ana", + last_name: "Ba" + ) + + aly = + insert(:user, + email: "aly@openfn.org", + first_name: "Aly", + last_name: "Sy" + ) + + project = + insert(:project, + project_users: [ + %{user: amy, role: :owner}, + %{user: ana, role: :admin}, + %{user: aly, role: :editor} + ] + ) + + %{workflow: workflow} = create_workflow(%{project: project}) + + amy_session = log_in_user(conn, amy) + ana_session = log_in_user(conn, ana) + aly_session = log_in_user(conn, aly) + + {:ok, amy_view, _html} = + live( + amy_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version]}" + ) + + refute amy_view |> has_element?("#canvas-online-users-#{amy.id}") + refute amy_view |> has_element?("#canvas-banner") + + refute render(amy_view) =~ amy.first_name + + refute render(amy_view) =~ ana.first_name + refute render(amy_view) =~ aly.first_name + + # Ana joins + {:ok, ana_view, _html} = + live( + ana_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version]}" + ) + + refute amy_view |> has_element?("#canvas-online-users-#{amy.id}") + assert amy_view |> has_element?("#canvas-online-users-#{ana.id}") + refute amy_view |> has_element?("#canvas-banner-#{amy.id}") + + wait_for_canvas_disabled_state(ana_view, false) + + refute ana_view |> has_element?("#canvas-online-users-#{ana.id}") + assert ana_view |> has_element?("#canvas-online-users-#{amy.id}") + assert ana_view |> has_element?("#canvas-banner-#{ana.id}") + + assert ana_view |> element("#canvas-banner-#{ana.id}") |> render() =~ + "This workflow is currently locked for editing because a collaborator (Amy Ly) is currently working on it. You can inspect this workflow and its associated steps but cannot make edits." + + refute render(amy_view) =~ amy.first_name + assert render(amy_view) =~ ana.first_name + + refute render(ana_view) =~ ana.first_name + assert render(ana_view) =~ amy.first_name + + # Aly joins + {:ok, aly_view, _html} = + live( + aly_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version]}" + ) + + refute amy_view |> has_element?("#canvas-online-users-#{amy.id}") + assert amy_view |> has_element?("#canvas-online-users-#{ana.id}") + assert amy_view |> has_element?("#canvas-online-users-#{aly.id}") + refute amy_view |> has_element?("#canvas-banner-#{amy.id}") + + refute ana_view |> has_element?("#canvas-online-users-#{ana.id}") + assert ana_view |> has_element?("#canvas-online-users-#{amy.id}") + assert ana_view |> has_element?("#canvas-online-users-#{aly.id}") + assert ana_view |> has_element?("#canvas-banner-#{ana.id}") + + assert ana_view |> element("#canvas-banner-#{ana.id}") |> render() =~ + "This workflow is currently locked for editing because a collaborator (Amy Ly) is currently working on it. You can inspect this workflow and its associated steps but cannot make edits." + + wait_for_canvas_disabled_state(aly_view, false) + + refute aly_view |> has_element?("#canvas-online-users-#{aly.id}") + assert aly_view |> has_element?("#canvas-online-users-#{amy.id}") + assert aly_view |> has_element?("#canvas-online-users-#{ana.id}") + assert aly_view |> has_element?("#canvas-banner-#{aly.id}") + + assert aly_view |> element("#canvas-banner-#{aly.id}") |> render() =~ + "This workflow is currently locked for editing because a collaborator (Amy Ly) is currently working on it. You can inspect this workflow and its associated steps but cannot make edits." + + refute render(amy_view) =~ amy.first_name + assert render(amy_view) =~ ana.first_name + assert render(amy_view) =~ aly.first_name + + refute render(ana_view) =~ ana.first_name + assert render(ana_view) =~ amy.first_name + assert render(ana_view) =~ aly.first_name + + refute render(aly_view) =~ aly.first_name + assert render(aly_view) =~ amy.first_name + assert render(aly_view) =~ ana.first_name + + last_job = workflow.jobs |> List.last() + last_edge = workflow.edges |> List.last() + + assert force_event(ana_view, :save) =~ + "Cannot save in view-only mode" + + ana_view + |> select_node(last_job, workflow.lock_version) + + assert force_event(ana_view, :delete_node, last_job) =~ + "Cannot delete a step in view-only mode" + + ana_view + |> select_node(last_edge, workflow.lock_version) + + assert force_event(ana_view, :delete_edge, last_edge) =~ + "Cannot delete an edge in view-only mode" + + assert force_event(ana_view, :manual_run_submit, %{}) =~ + "Cannot run in view-only mode" + + assert force_event(ana_view, :rerun, nil, nil) =~ + "Cannot rerun in view-only mode" + end + + test "in inspector", %{conn: conn} do + amy = + insert(:user, + email: "amy@openfn.org", + first_name: "Amy", + last_name: "Ly" + ) + + ana = + insert(:user, + email: "ana@openfn.org", + first_name: "Ana", + last_name: "Ba" + ) + + aly = + insert(:user, + email: "aly@openfn.org", + first_name: "Aly", + last_name: "Sy" + ) + + project = + insert(:project, + project_users: [ + %{user: amy, role: :owner}, + %{user: ana, role: :admin}, + %{user: aly, role: :editor} + ] + ) + + %{workflow: %{jobs: [job | _]} = workflow} = + create_workflow(%{project: project}) + + amy_session = log_in_user(conn, amy) + ana_session = log_in_user(conn, ana) + aly_session = log_in_user(conn, aly) + + {:ok, amy_view, _html} = + live( + amy_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version, s: job.id, m: "expand"]}" + ) + + wait_for_canvas_disabled_state(amy_view, false) + + refute amy_view |> has_element?("#inspector-online-users-#{amy.id}") + refute amy_view |> has_element?("#inspector-banner") + + refute render(amy_view) =~ amy.first_name + + refute render(amy_view) =~ ana.first_name + refute render(amy_view) =~ aly.first_name + + # Ana joins + {:ok, ana_view, _html} = + live( + ana_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version, s: job.id, m: "expand"]}" + ) + + wait_for_canvas_disabled_state(ana_view, true) + + refute amy_view |> has_element?("#inspector-online-users-#{amy.id}") + assert amy_view |> has_element?("#inspector-online-users-#{ana.id}") + refute amy_view |> has_element?("#inspector-banner-#{amy.id}") + + refute ana_view |> has_element?("#inspector-online-users-#{ana.id}") + assert ana_view |> has_element?("#inspector-online-users-#{amy.id}") + assert ana_view |> has_element?("#inspector-banner-#{ana.id}") + + assert ana_view |> element("#canvas-banner-#{ana.id}") |> render() =~ + "This workflow is currently locked for editing because a collaborator (Amy Ly) is currently working on it. You can inspect this workflow and its associated steps but cannot make edits." + + refute render(amy_view) =~ amy.first_name + assert render(amy_view) =~ ana.first_name + + refute render(ana_view) =~ ana.first_name + assert render(ana_view) =~ amy.first_name + + # Aly joins + {:ok, aly_view, _html} = + live( + aly_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version, s: job.id, m: "expand"]}" + ) + + wait_for_canvas_disabled_state(aly_view, false) + + refute amy_view |> has_element?("#inspector-online-users-#{amy.id}") + assert amy_view |> has_element?("#inspector-online-users-#{ana.id}") + assert amy_view |> has_element?("#inspector-online-users-#{aly.id}") + refute amy_view |> has_element?("#inspector-banner-#{amy.id}") + + refute ana_view |> has_element?("#inspector-online-users-#{ana.id}") + assert ana_view |> has_element?("#inspector-online-users-#{amy.id}") + assert ana_view |> has_element?("#inspector-online-users-#{aly.id}") + assert ana_view |> has_element?("#inspector-banner-#{ana.id}") + + assert ana_view |> element("#canvas-banner-#{ana.id}") |> render() =~ + "This workflow is currently locked for editing because a collaborator (Amy Ly) is currently working on it. You can inspect this workflow and its associated steps but cannot make edits." + + refute aly_view |> has_element?("#inspector-online-users-#{aly.id}") + assert aly_view |> has_element?("#inspector-online-users-#{amy.id}") + assert aly_view |> has_element?("#inspector-online-users-#{ana.id}") + assert aly_view |> has_element?("#inspector-banner-#{aly.id}") + + assert aly_view |> element("#canvas-banner-#{aly.id}") |> render() =~ + "This workflow is currently locked for editing because a collaborator (Amy Ly) is currently working on it. You can inspect this workflow and its associated steps but cannot make edits." + + refute render(amy_view) =~ amy.first_name + assert render(amy_view) =~ ana.first_name + assert render(amy_view) =~ aly.first_name + + refute render(ana_view) =~ ana.first_name + assert render(ana_view) =~ amy.first_name + assert render(ana_view) =~ aly.first_name + + refute render(aly_view) =~ aly.first_name + assert render(aly_view) =~ amy.first_name + assert render(aly_view) =~ ana.first_name + end + end + + describe "When workflow has many visits from same user, multiple sessions are detected and edit is disabled" do + test "in canvas", %{conn: conn} do + amy = + insert(:user, + email: "amy@openfn.org", + first_name: "Amy", + last_name: "Ly" + ) + + project = + insert(:project, + project_users: [ + %{user: amy, role: :owner} + ] + ) + + %{workflow: workflow} = create_workflow(%{project: project}) + + amy_session = log_in_user(conn, amy) + another_amy_session = log_in_user(conn, amy) + + {:ok, amy_view, _html} = + live( + amy_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version]}" + ) + + # Amy joins in another session + {:ok, another_amy_view, _html} = + live( + another_amy_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version]}" + ) + + wait_for_canvas_disabled_state(amy_view, false) + assert amy_view |> has_element?("#canvas-banner-#{amy.id}") + + wait_for_canvas_disabled_state(another_amy_view, false) + assert another_amy_view |> has_element?("#canvas-banner-#{amy.id}") + + assert amy_view |> element("#canvas-banner-#{amy.id}") |> render() =~ + "You can't edit this workflow because you have 2 active sessions. Close your other sessions to enable editing." + + assert another_amy_view |> element("#canvas-banner-#{amy.id}") |> render() =~ + "You can't edit this workflow because you have 2 active sessions. Close your other sessions to enable editing." + end + + test "in inspector", %{conn: conn} do + amy = + insert(:user, + email: "amy@openfn.org", + first_name: "Amy", + last_name: "Ly" + ) + + project = + insert(:project, + project_users: [ + %{user: amy, role: :owner} + ] + ) + + %{workflow: %{jobs: [job | _]} = workflow} = + create_workflow(%{project: project}) + + amy_session = log_in_user(conn, amy) + another_amy_session = log_in_user(conn, amy) + + {:ok, amy_view, _html} = + live( + amy_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version, s: job.id, m: "expand"]}" + ) + + # Amy joins in another session + {:ok, another_amy_view, _html} = + live( + another_amy_session, + ~p"/projects/#{project.id}/w/#{workflow.id}?#{[v: workflow.lock_version, s: job.id, m: "expand"]}" + ) + + wait_for_canvas_disabled_state(amy_view, false) + assert amy_view |> has_element?("#inspector-banner-#{amy.id}") + + wait_for_canvas_disabled_state(another_amy_view, false) + assert another_amy_view |> has_element?("#inspector-banner-#{amy.id}") + + assert amy_view |> element("#inspector-banner-#{amy.id}") |> render() =~ + "You can't edit this workflow because you have 2 active sessions. Close your other sessions to enable editing." + + assert another_amy_view + |> element("#inspector-banner-#{amy.id}") + |> render() =~ + "You can't edit this workflow because you have 2 active sessions. Close your other sessions to enable editing." + end + end + + # HACK: This is a workaround to wait for presence to be syncronized. + # This is used above to wait for other users to have their presence + # updated, so we can assert that they can see other users. + defp wait_for_canvas_disabled_state(view, state) do + {ref, _, _} = view.proxy + + assert_receive({^ref, {:push_event, "set-disabled", %{disabled: ^state}}}) + end +end diff --git a/test/support/workflow_live_helpers.ex b/test/support/workflow_live_helpers.ex index 9171ee4e27..e81e6e9892 100644 --- a/test/support/workflow_live_helpers.ex +++ b/test/support/workflow_live_helpers.ex @@ -136,6 +136,10 @@ defmodule Lightning.WorkflowLive.Helpers do |> render_click("manual_run_submit", %{"manual" => params}) end + def force_event(view, :switch_workflow_version, type) do + view |> render_click("switch-version", %{"type" => type}) + end + def force_event(view, :rerun, run_id, step_id) do view |> render_click("rerun", %{"run_id" => run_id, "step_id" => step_id}) diff --git a/test/test_helper.exs b/test/test_helper.exs index 4b0383ad8c..4988b6dacd 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -3,7 +3,6 @@ Code.put_compiler_option(:warnings_as_errors, true) # Rexbug.start("ExUnit.Server.add_sync_module/_") Mox.defmock(Lightning.AuthProviders.OauthHTTPClient.Mock, for: Tesla.Adapter) -# Mox.defmock(Lightning.GithubClient.Mock, for: Tesla.Adapter) Mox.defmock(Lightning.Tesla.Mock, for: Tesla.Adapter) :ok = Application.ensure_started(:ex_machina)