Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

jyeshe
Copy link
Member

@jyeshe jyeshe commented Jan 16, 2025

Description

This PR adds a Steps API based on the GovStack Workflow Process API spec.

Closes #1656

Validation steps

  1. Create a workflow, some runs and a Personal Access Token (on your profile menu)
  2. Using the PAT as Bearer token on the Authorization header, make a GET request to /api/projects/<project_id>/steps/
  3. Notice that the last runs are listed as first results (descending order)
  4. Using one of the step id make a GET request to /api/projects/<project_id>/steps/<step_id>
  5. Make GET requests to /api/projects/<project_id>/steps/ using page and page_size query params (default page_size is 10 and max is 100)

Additional notes for the reviewer

  1. The pagination info on the response was postponed (out of the scope of this PR).
  2. It was decided to expose the routes only under /api/projects

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed a self-review of my code.
  • I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

@jyeshe jyeshe changed the title Add steps API based on pre-existing run API Reintroduce steps API based on pre-existing run API Jan 17, 2025
@jyeshe jyeshe force-pushed the steps-api branch 2 times, most recently from 5809500 to 1b9fd4d Compare January 20, 2025 10:44
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.43%. Comparing base (759b16e) to head (8e9f5b1).
Report is 3 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

@jyeshe jyeshe changed the title Reintroduce steps API based on pre-existing run API Reintroduce steps API based on GovStack spec Jan 21, 2025
@jyeshe jyeshe marked this pull request as ready for review January 21, 2025 08:44
@jyeshe jyeshe self-assigned this Jan 21, 2025
@jyeshe jyeshe requested review from stuartc and elias-ba January 21, 2025 08:49
Copy link
Contributor

@elias-ba elias-ba left a 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.
Copy link
Contributor

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.

Comment on lines +1 to +63
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice and neat 🫖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New Issues
Development

Successfully merging this pull request may close these issues.

Update RunController and JSON api for GovStack compliance
2 participants