-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reintroduce steps API based on GovStack spec #2833
base: main
Are you sure you want to change the base?
Conversation
5809500
to
1b9fd4d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2833 +/- ##
==========================================
+ Coverage 91.37% 91.43% +0.06%
==========================================
Files 340 342 +2
Lines 12225 12253 +28
==========================================
+ Hits 11171 11204 +33
+ Misses 1054 1049 -5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job @jyeshe, this is lovely and packed. Just left one suggestion on the CHANGELOG line but nothing very important. 👏🏽
@@ -17,6 +17,9 @@ and this project adheres to | |||
|
|||
### Added | |||
|
|||
- Reintroduce steps (old runs) API based on GovStack spec. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we mention that we are reintroducing them in the runs api. That way we are clear that it has no UI implications.
defmodule LightningWeb.API.StepController do | ||
use LightningWeb, :controller | ||
|
||
alias Lightning.Invocation | ||
alias Lightning.Policies.Permissions | ||
alias Lightning.Policies.ProjectUsers | ||
alias Lightning.Projects | ||
|
||
@valid_params ~w(page page_size project_id) | ||
@max_page_size Application.compile_env( | ||
:lightning, | ||
LightningWeb.API.StepController | ||
)[:max_page_size] || 100 | ||
|
||
action_fallback LightningWeb.FallbackController | ||
|
||
def index(conn, %{"project_id" => project_id} = params) do | ||
with :ok <- validate_params(params), | ||
:ok <- authorize_read(conn, project_id) do | ||
pagination_attrs = | ||
params | ||
|> Map.take(["page", "page_size"]) | ||
|> Map.update( | ||
"page_size", | ||
@max_page_size, | ||
&min(@max_page_size, String.to_integer(&1)) | ||
) | ||
|
||
page = | ||
project_id | ||
|> Projects.get_project!() | ||
|> Invocation.list_steps_for_project(pagination_attrs) | ||
|
||
render(conn, "index.json", %{page: page, conn: conn}) | ||
end | ||
end | ||
|
||
def show(conn, %{"project_id" => project_id, "id" => id}) do | ||
with :ok <- authorize_read(conn, project_id) do | ||
step = Invocation.get_step_with_job!(id) | ||
render(conn, "show.json", %{step: step, conn: conn}) | ||
end | ||
end | ||
|
||
defp validate_params(params) do | ||
with [] <- Map.keys(params) -- @valid_params, | ||
{_n, ""} <- Integer.parse(params["page"] || "1"), | ||
{_n, ""} <- Integer.parse(params["page_size"] || "1") do | ||
:ok | ||
else | ||
_invalid -> {:error, :bad_request} | ||
end | ||
end | ||
|
||
defp authorize_read(conn, project_id) do | ||
Permissions.can( | ||
ProjectUsers, | ||
:access_project, | ||
conn.assigns.current_resource, | ||
%{project_id: project_id} | ||
) | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice and neat 🫖
Description
This PR adds a Steps API based on the GovStack Workflow Process API spec.
Closes #1656
Validation steps
/api/projects/<project_id>/steps/
/api/projects/<project_id>/steps/<step_id>
/api/projects/<project_id>/steps/
usingpage
andpage_size
query params (defaultpage_size
is 10 and max is 100)Additional notes for the reviewer
/api/projects
AI Usage
Please disclose how you've used AI in this work (it's cool, we just want to know!):
You can read more details in our Responsible AI Policy
Pre-submission checklist
:owner
,:admin
,:editor
,:viewer
)