Skip to content

Commit

Permalink
Validate workflows limit on update as well
Browse files Browse the repository at this point in the history
  • Loading branch information
@jyeshe
jyeshe committed Dec 18, 2024
1 parent e32bdbc commit 7dfbc4b
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 44 deletions.
2 changes: 1 addition & 1 deletion lib/lightning/policies/workflows.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ defmodule Lightning.Policies.Workflows do
alias Lightning.Run

@type actions :: :access_write | :access_read
@spec authorize(actions(), User.t() | Runt.t(), Project.t()) ::
@spec authorize(actions(), User.t() | Run.t(), Project.t()) ::
:ok | {:error, :unauthorized}
def authorize(access, %User{} = user, project)
when access in [:access_write, :access_read] do
Expand Down
43 changes: 31 additions & 12 deletions lib/lightning_web/controllers/api/workflows_controller.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,18 @@ defmodule LightningWeb.API.WorkflowsController do
{:error, :too_many_workflows} ->
conn
|> put_status(:unprocessable_entity)
|> json(%{id: nil, error: "Your plan has reached the limit of active workflows."})
|> json(%{
id: nil,
error: "Your plan has reached the limit of active workflows."
})

{:error, :too_many_active_triggers} ->
conn
|> put_status(:unprocessable_entity)
|> json(%{id: nil, error: "Only one trigger can be enabled at a time."})
|> json(%{
id: nil,
error: "A workflow can have only one trigger enabled at a time."
})

result ->
result
Expand Down Expand Up @@ -70,21 +76,35 @@ defmodule LightningWeb.API.WorkflowsController do
{:error, :cannot_replace_trigger} ->
conn
|> put_status(:unprocessable_entity)
|> json(%{id: workflow_id, error: "The triggers cannot be replaced, only edited or added."})
|> json(%{
id: workflow_id,
error: "The triggers cannot be replaced, only edited or added."
})

{:error, :too_many_workflows} ->
conn
|> put_status(:unprocessable_entity)
|> json(%{
id: workflow_id,
error: "Your plan has reached the limit of active workflows."
})

{:error, :too_many_active_triggers} ->
conn
|> put_status(:unprocessable_entity)
|> json(%{id: workflow_id, error: "Only one trigger can be enabled at a time."})
|> json(%{
id: workflow_id,
error: "A workflow can have only one trigger enabled at a time."
})

result ->
result
end
end)
end

defp save_workflow(%{"project_id" => project_id} = params, user), do:
save_workflow(params, project_id, user)
defp save_workflow(%{"project_id" => project_id} = params, user),
do: save_workflow(params, project_id, user)

defp save_workflow(%{triggers: triggers} = workflow, params, user) do
triggers_ids =
Expand All @@ -95,11 +115,10 @@ defmodule LightningWeb.API.WorkflowsController do
active_triggers_count =
params
|> Map.get("triggers", [])
|> Enum.filter(& &1["enabled"])
|> Enum.count()
|> Enum.count(& &1["enabled"])

cond do
Enum.any?(triggers, & &1.id not in triggers_ids) ->
Enum.any?(triggers, &(&1.id not in triggers_ids)) ->
{:error, :cannot_replace_trigger}

active_triggers_count > 1 ->
Expand All @@ -114,9 +133,9 @@ defmodule LightningWeb.API.WorkflowsController do

defp save_workflow(params_or_changeset, project_id, user) do
case UsageLimiter.limit_action(
%Action{type: :activate_workflow},
%Context{project_id: project_id}
) do
%Action{type: :activate_workflow},
%Context{project_id: project_id}
) do
:ok -> Workflows.save_workflow(params_or_changeset, user)
_error -> {:error, :too_many_workflows}
end
Expand Down
115 changes: 84 additions & 31 deletions test/lightning_web/workflows_controller_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,10 @@ defmodule LightningWeb.API.WorkflowsControllerTest do
Jason.encode!(workflow)
)

assert %{"id" => nil, "error" => "Your plan has reached the limit of active workflows."} = json_response(conn, 422)
assert %{
"id" => nil,
"error" => "Your plan has reached the limit of active workflows."
} = json_response(conn, 422)
end

test "returns 422 when there are too many active triggers", %{conn: conn} do
Expand All @@ -184,7 +187,11 @@ defmodule LightningWeb.API.WorkflowsControllerTest do
insert(:project, project_users: [%{user: user}])

workflow =
build(:simple_workflow, name: "workflow", triggers: build_list(2, :trigger), project_id: project.id)
build(:simple_workflow,
name: "workflow",
triggers: build_list(2, :trigger),
project_id: project.id
)

Mox.stub(
Lightning.Extensions.MockUsageLimiter,
Expand All @@ -203,7 +210,10 @@ defmodule LightningWeb.API.WorkflowsControllerTest do
Jason.encode!(workflow)
)

assert %{"id" => nil, "error" => "Your plan has reached the limit of active workflows."} = json_response(conn, 422)
assert %{
"id" => nil,
"error" => "Your plan has reached the limit of active workflows."
} = json_response(conn, 422)
end

test "returns 401 without a token", %{conn: conn} do
Expand Down Expand Up @@ -292,10 +302,50 @@ defmodule LightningWeb.API.WorkflowsControllerTest do

assert json_response(conn, 422) == %{
"id" => workflow.id,
"error" => "The triggers cannot be replaced, only edited or added."
"error" =>
"The triggers cannot be replaced, only edited or added."
}
end

test "returns 422 when workflow limit has been reached", %{conn: conn} do
user = insert(:user)

project =
insert(:project, project_users: [%{user: user}])

insert(:simple_workflow, name: "work1", project: project)

trigger = build(:trigger, enabled: false)

workflow =
insert(:simple_workflow, name: "work2", project: project)
|> with_trigger(trigger)

patch = %{triggers: [%{(workflow.triggers |> hd()) | enabled: true}]}

Mox.stub(
Lightning.Extensions.MockUsageLimiter,
:limit_action,
fn
%{type: :activate_workflow}, _context ->
{:error, :too_many_workflows, %{text: "any"}}
end
)

conn =
conn
|> assign_bearer(user)
|> patch(
~p"/api/projects/#{project.id}/workflows/#{workflow.id}",
Jason.encode!(patch)
)

assert %{
"id" => workflow.id,
"error" => "Your plan has reached the limit of active workflows."
} == json_response(conn, 422)
end

test "returns 422 when there are too many enabled triggers", %{conn: conn} do
user = insert(:user)

Expand All @@ -314,7 +364,8 @@ defmodule LightningWeb.API.WorkflowsControllerTest do
type: :cron,
cron_expression: "0 0 * * *",
enabled: true
) | workflow.triggers
)
| workflow.triggers
]
}

Expand All @@ -328,7 +379,8 @@ defmodule LightningWeb.API.WorkflowsControllerTest do

assert json_response(conn, 422) == %{
"id" => workflow.id,
"error" => "Only one trigger can be enabled at a time."
"error" =>
"A workflow can have only one trigger enabled at a time."
}
end

Expand Down Expand Up @@ -412,30 +464,30 @@ defmodule LightningWeb.API.WorkflowsControllerTest do
insert(:project, project_users: [%{user: user}])

workflow =
insert(:simple_workflow, name: "work1.0", project: project)
|> Repo.reload()
|> Repo.preload([:edges, :jobs, :triggers])

invalid_update =
build(:simple_workflow, name: "work1.1", project: project)
|> then(fn %{
edges: [new_edge | other_new_edges],
jobs: [new_job1 | _other_jobs] = new_jobs,
triggers: [new_trigger]
} ->
Map.merge(workflow, %{
edges: [
%{
new_edge
| source_trigger_id: new_trigger.id,
target_job_id: new_job1.id
}
| other_new_edges
],
jobs: new_jobs,
triggers: [new_trigger]
})
end)
insert(:simple_workflow, name: "work1.0", project: project)
|> Repo.reload()
|> Repo.preload([:edges, :jobs, :triggers])

invalid_update =
build(:simple_workflow, name: "work1.1", project: project)
|> then(fn %{
edges: [new_edge | other_new_edges],
jobs: [new_job1 | _other_jobs] = new_jobs,
triggers: [new_trigger]
} ->
Map.merge(workflow, %{
edges: [
%{
new_edge
| source_trigger_id: new_trigger.id,
target_job_id: new_job1.id
}
| other_new_edges
],
jobs: new_jobs,
triggers: [new_trigger]
})
end)

conn =
conn
Expand All @@ -447,7 +499,8 @@ defmodule LightningWeb.API.WorkflowsControllerTest do

assert json_response(conn, 422) == %{
"id" => workflow.id,
"error" => "The triggers cannot be replaced, only edited or added."
"error" =>
"The triggers cannot be replaced, only edited or added."
}
end

Expand Down

0 comments on commit 7dfbc4b

Please sign in to comment.