Skip to content

Commit

Permalink
Do not include v param in workflow links when latest (#2984)
Browse files Browse the repository at this point in the history
* Do not include  param in workflow links when it is latest

* do not add v param after saving workflow

* fix credo warnings
  • Loading branch information
midigofrank authored Mar 6, 2025
1 parent 596132b commit 5ea7657
Show file tree
Hide file tree
Showing 9 changed files with 180 additions and 33 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ and this project adheres to

### Changed

- Do not include `v` param in workflow links when it is latest
[#2941](https://github.com/OpenFn/lightning/issues/2941)

### Fixed

## [v2.10.16] - 2025-02-28
Expand Down
15 changes: 12 additions & 3 deletions lib/lightning_web/live/run_live/components.ex
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ defmodule LightningWeb.RunLive.Components do
"""
end

attr :workflow_version, :integer, required: true
attr :step, Lightning.Invocation.Step, required: true
attr :is_clone, :boolean, default: false
attr :run_id, :string
Expand Down Expand Up @@ -192,7 +193,7 @@ defmodule LightningWeb.RunLive.Components do
<.link
class="pl-1"
patch={
~p"/projects/#{@project_id}/w/#{@step.snapshot.workflow_id}?#{%{v: @step.snapshot.lock_version, a: @run_id, m: "expand", s: @job.id}}" <> "#log"
~p"/projects/#{@project_id}/w/#{@step.snapshot.workflow_id}?#{maybe_add_snapshot_version(%{a: @run_id, m: "expand", s: @job.id}, @step.snapshot.lock_version, @workflow_version)}" <> "#log"
}
>
<.icon
Expand Down Expand Up @@ -226,6 +227,14 @@ defmodule LightningWeb.RunLive.Components do
"""
end

defp maybe_add_snapshot_version(params, snapshot_version, workflow_version) do
if snapshot_version != workflow_version do
Map.merge(params, %{v: snapshot_version})
else
params
end
end

def loading_filler(assigns) do
~H"""
<.detail_list class="animate-pulse">
Expand Down Expand Up @@ -380,8 +389,8 @@ defmodule LightningWeb.RunLive.Components do
aria-label="Inspect this step"
class="cursor-pointer"
navigate={
~p"/projects/#{@project_id}/w/#{@step.snapshot.workflow_id}"
<> "?v=#{@step.snapshot.lock_version}&a=#{@run.id}&m=expand&s=#{@job.id}#log"
~p"/projects/#{@project_id}/w/#{@step.snapshot.workflow_id}?#{maybe_add_snapshot_version(%{a: @run.id, m: "expand", s: @job.id}, @step.snapshot.lock_version, @workflow_version)}"
<> "#log"
}
>
<.icon
Expand Down
4 changes: 4 additions & 0 deletions lib/lightning_web/live/run_live/run_viewer_live.ex
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ defmodule LightningWeb.RunLive.RunViewerLive do
>
<.step_item
step={step}
workflow_version={@workflow.lock_version}
run_id={run.id}
job_id={@job_id}
is_clone={
Expand All @@ -121,6 +122,7 @@ defmodule LightningWeb.RunLive.RunViewerLive do
>
<.step_item
step={step}
workflow_version={@workflow.lock_version}
run_id={run.id}
job_id={@job_id}
is_clone={
Expand Down Expand Up @@ -165,6 +167,7 @@ defmodule LightningWeb.RunLive.RunViewerLive do
>
<.step_item
step={step}
workflow_version={@workflow.lock_version}
run_id={run.id}
job_id={@job_id}
is_clone={
Expand Down Expand Up @@ -213,6 +216,7 @@ defmodule LightningWeb.RunLive.RunViewerLive do
>
<.step_item
step={step}
workflow_version={@workflow.lock_version}
run_id={run.id}
job_id={@job_id}
is_clone={
Expand Down
1 change: 1 addition & 0 deletions lib/lightning_web/live/run_live/show.ex
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ defmodule LightningWeb.RunLive.Show do
<.link patch={"?step=#{step.id}"}>
<.step_item
step={step}
workflow_version={@workflow.lock_version}
is_clone={
DateTime.compare(step.inserted_at, run.inserted_at) == :lt
}
Expand Down
4 changes: 1 addition & 3 deletions lib/lightning_web/live/workflow_live/dashboard_components.ex
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,7 @@ defmodule LightningWeb.WorkflowLive.DashboardComponents do
<div class="flex flex-1 items-center truncate">
<.link
id={"workflow-card-#{@workflow.id}"}
navigate={
~p"/projects/#{@project.id}/w/#{@workflow.id}?v=#{@workflow.lock_version}"
}
navigate={~p"/projects/#{@project.id}/w/#{@workflow.id}"}
role="button"
>
<div class="text-sm">
Expand Down
39 changes: 24 additions & 15 deletions lib/lightning_web/live/workflow_live/edit.ex
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ defmodule LightningWeb.WorkflowLive.Edit do
<.expand_job_editor
base_url={@base_url}
snapshot_lock_version={@snapshot && @snapshot.lock_version}
snapshot_version_tag={@snapshot_version_tag}
job={@selected_job}
selected_run={@selected_run}
form={@workflow_form}
Expand Down Expand Up @@ -802,10 +803,19 @@ defmodule LightningWeb.WorkflowLive.Edit do

query_string =
query_params
|> Enum.reject(fn {_k, v} -> is_nil(v) end)
|> Enum.reject(fn {k, v} ->
is_nil(v) or (k == "v" and assigns.snapshot_version_tag == "latest")
end)
|> URI.encode_query()
|> then(fn query ->
if byte_size(query) > 0 do
"?" <> query
else
query
end
end)

"#{assigns[:base_url]}?#{query_string}"
"#{assigns[:base_url]}#{query_string}"
end

defp display_switcher(snapshot, workflow) do
Expand Down Expand Up @@ -956,7 +966,7 @@ defmodule LightningWeb.WorkflowLive.Edit do
~H"""
<.link
id={"open-inspector-#{@job.id}"}
patch={"#{@base_url}?s=#{@job.id}&m=expand" <> (if @snapshot_lock_version, do: "&v=#{@snapshot_lock_version}", else: "") <> (if @selected_run, do: "&a=#{@selected_run}", else: "")}
patch={"#{@base_url}?s=#{@job.id}&m=expand" <> (if @snapshot_lock_version && @snapshot_version_tag != "latest", do: "&v=#{@snapshot_lock_version}", else: "") <> (if @selected_run, do: "&a=#{@selected_run}", else: "")}
class={@button_classes}
>
<.icon name="hero-code-bracket" class="w-4 h-4 text-grey-400" />
Expand Down Expand Up @@ -1277,12 +1287,9 @@ defmodule LightningWeb.WorkflowLive.Edit do

patches = WorkflowParams.to_patches(prev_params, next_params)

lock_version = Ecto.Changeset.get_field(next_changeset, :lock_version)

query_params =
socket.assigns.query_params
|> Map.reject(fn {_k, v} -> is_nil(v) end)
|> Map.put("v", lock_version)
|> Map.reject(fn {k, v} -> is_nil(v) or k == "v" end)

url = ~p"/projects/#{project.id}/w/#{workflow.id}?#{query_params}"

Expand Down Expand Up @@ -1317,13 +1324,9 @@ defmodule LightningWeb.WorkflowLive.Edit do

patches = WorkflowParams.to_patches(prev_params, next_params)

lock_version =
Ecto.Changeset.get_field(next_changeset, :lock_version)

query_params =
socket.assigns.query_params
|> Map.reject(fn {_k, v} -> is_nil(v) end)
|> Map.put("v", lock_version)
|> Map.reject(fn {k, v} -> is_nil(v) or k == "v" end)

url = ~p"/projects/#{project.id}/w/#{workflow.id}?#{query_params}"

Expand Down Expand Up @@ -1532,8 +1535,8 @@ defmodule LightningWeb.WorkflowLive.Edit do

query_params =
socket.assigns.query_params
|> Map.put("v", workflow.lock_version)
|> Map.reject(fn {_key, value} -> is_nil(value) end)
|> Map.drop(["v"])

flash_msg =
"Workflow saved successfully." <>
Expand Down Expand Up @@ -2439,8 +2442,14 @@ defmodule LightningWeb.WorkflowLive.Edit do
params =
query_params
|> Map.put("a", run.id)
|> Map.put("v", version)
|> Enum.reject(fn {_k, v} -> is_nil(v) end)
|> Map.reject(fn {_k, v} -> is_nil(v) end)
|> then(fn params ->
if workflow.lock_version == version do
Map.drop(params, ["v"])
else
Map.put(params, "v", version)
end
end)

socket
|> push_patch(to: ~p"/projects/#{project}/w/#{workflow}?#{params}")
Expand Down
128 changes: 128 additions & 0 deletions test/lightning_web/live/run_live/components_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,46 @@ defmodule LightningWeb.RunLive.ComponentsTest do

assert has_run_step_link?(html, workflow.project, run, first_step)

# snapshot version is not included in the link when it is latest
assert workflow.lock_version == snapshot.lock_version

assert html
|> Floki.find(
~s{a[href='#{~p"/projects/#{workflow.project}/w/#{workflow}?#{%{a: run.id, m: "expand", s: job_1.id}}"}#log']}
)
|> Enum.any?()

refute html
|> Floki.find(
~s{a[href='#{~p"/projects/#{workflow.project}/w/#{workflow}?#{%{a: run.id, m: "expand", s: job_1.id, v: snapshot.lock_version}}"}#log']}
)
|> Enum.any?()

# if the snapshot version is not latest, the version is included

html =
render_component(&Components.step_list_item/1,
step: first_step,
run: run,
workflow_version: workflow.lock_version + 1,
project_id: project_id,
can_run_workflow: true,
can_edit_data_retention: true
)
|> Floki.parse_fragment!()

refute html
|> Floki.find(
~s{a[href='#{~p"/projects/#{workflow.project}/w/#{workflow}?#{%{a: run.id, m: "expand", s: job_1.id}}"}#log']}
)
|> Enum.any?()

assert html
|> Floki.find(
~s{a[href='#{~p"/projects/#{workflow.project}/w/#{workflow}?#{%{a: run.id, m: "expand", s: job_1.id, v: snapshot.lock_version}}"}#log']}
)
|> Enum.any?()

html =
render_component(&Components.step_list_item/1,
step: second_step,
Expand Down Expand Up @@ -213,6 +253,94 @@ defmodule LightningWeb.RunLive.ComponentsTest do
refute html =~ "This step was originally executed in a previous run"
end

describe "step_item/1" do
test "renders link to inspector correctly" do
%{triggers: [trigger], jobs: [job_1 | _rest]} =
workflow = insert(:complex_workflow)

{:ok, snapshot} = Workflows.Snapshot.create(workflow)

dataclip = insert(:dataclip)
output_dataclip = insert(:dataclip)

%{runs: [run]} =
insert(:workorder,
workflow: workflow,
trigger: trigger,
dataclip: dataclip,
snapshot: snapshot,
runs: [
%{
state: :failed,
dataclip: dataclip,
starting_trigger: trigger,
snapshot: snapshot,
steps: [
insert(:step,
job: job_1,
snapshot: snapshot,
input_dataclip: dataclip,
output_dataclip: output_dataclip,
finished_at: build(:timestamp),
exit_reason: "fail"
)
]
}
]
)

[first_step] = run.steps

project_id = workflow.project_id

# snapshot version is not included in the link when it is latest
html =
render_component(&Components.step_item/1,
step: first_step,
run_id: run.id,
workflow_version: workflow.lock_version,
project_id: project_id
)
|> Floki.parse_fragment!()

assert workflow.lock_version == snapshot.lock_version

assert html
|> Floki.find(
~s{a[href='#{~p"/projects/#{workflow.project}/w/#{workflow}?#{%{a: run.id, m: "expand", s: job_1.id}}"}#log']}
)
|> Enum.any?()

refute html
|> Floki.find(
~s{a[href='#{~p"/projects/#{workflow.project}/w/#{workflow}?#{%{a: run.id, m: "expand", s: job_1.id, v: snapshot.lock_version}}"}#log']}
)
|> Enum.any?()

# snapshot version is included in the link when it is outdated
html =
render_component(&Components.step_item/1,
step: first_step,
run_id: run.id,
workflow_version: workflow.lock_version + 1,
project_id: project_id
)
|> Floki.parse_fragment!()

refute html
|> Floki.find(
~s{a[href='#{~p"/projects/#{workflow.project}/w/#{workflow}?#{%{a: run.id, m: "expand", s: job_1.id}}"}#log']}
)
|> Enum.any?()

assert html
|> Floki.find(
~s{a[href='#{~p"/projects/#{workflow.project}/w/#{workflow}?#{%{a: run.id, m: "expand", s: job_1.id, v: snapshot.lock_version}}"}#log']}
)
|> Enum.any?()
end
end

test "no rerun button is displayed when user can't rerun a job" do
%{triggers: [trigger], jobs: [job | _rest]} =
workflow = insert(:simple_workflow)
Expand Down
13 changes: 4 additions & 9 deletions test/lightning_web/live/workflow_live/edit_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ defmodule LightningWeb.WorkflowLive.EditTest do
click_save(view)

assert %{id: workflow_id} =
workflow =
Lightning.Repo.one(
from w in Workflow,
where:
Expand All @@ -235,7 +234,7 @@ defmodule LightningWeb.WorkflowLive.EditTest do

assert_patched(
view,
~p"/projects/#{project.id}/w/#{workflow_id}?#{[m: "expand", s: job.id, v: workflow.lock_version]}"
~p"/projects/#{project.id}/w/#{workflow_id}?#{[m: "expand", s: job.id]}"
)

assert render(view) =~ "Workflow saved"
Expand Down Expand Up @@ -1961,11 +1960,9 @@ defmodule LightningWeb.WorkflowLive.EditTest do
# submit workflow form
view |> form("#workflow-form") |> render_submit()

workflow = Lightning.Repo.reload(workflow)

assert_patched(
view,
~p"/projects/#{project}/w/#{workflow}?#{[m: "expand", s: job_1.id, v: workflow.lock_version]}"
~p"/projects/#{project}/w/#{workflow}?#{[m: "expand", s: job_1.id]}"
)

# manual run form still has the body
Expand Down Expand Up @@ -2209,7 +2206,7 @@ defmodule LightningWeb.WorkflowLive.EditTest do

assert_patched(
view,
~p"/projects/#{project.id}/w/#{workflow.id}?#{[m: "expand", s: job.id, v: workflow.lock_version]}"
~p"/projects/#{project.id}/w/#{workflow.id}?#{[m: "expand", s: job.id]}"
)

assert render(view) =~ "Workflow saved and synced to GitHub successfully."
Expand Down Expand Up @@ -2276,11 +2273,9 @@ defmodule LightningWeb.WorkflowLive.EditTest do
|> form("#workflow-form")
|> render_submit(%{"github-sync" => %{"commit_message" => "some message"}})

workflow = Repo.reload!(workflow)

assert_patched(
view,
~p"/projects/#{project.id}/w/#{workflow.id}?#{[s: job_2.id, v: workflow.lock_version]}"
~p"/projects/#{project.id}/w/#{workflow.id}?#{[s: job_2.id]}"
)

assert render(view) =~
Expand Down
Loading

0 comments on commit 5ea7657

Please sign in to comment.