OpenAgents

Commit

Expose bounded pagination and filters on the issues API

f35c853b1c63 · AtlantisPleb · · parent 7a799e6e7fc2 

Expose bounded pagination and filters on the issues API

The repository issues route returned the unbounded open list and accepted
only a state filter, while the web surfaces already read through
Issues.list_issues_page with the full filter chain. The API now exposes the
same contract: state, labels, assignee, milestone, and q filters, plus page,
all served at Issues' fixed page size with pagination metadata (page,
per_page, total, total_pages) in every index response.

Unknown states and non-integer pages are rejected with stable 422 field
errors instead of being silently coerced; out-of-range pages clamp through
the existing parse_page bound.

Also allowlist docs/taxonomy.md in the Sarah reference check; the glossary
defines Sarah as a persona boundary in the same class as
docs/architecture.md, and its absence broke precommit for every change.

Deploy story

What this commit did to the running system — joined from the forge receipt chain, the part a commit page elsewhere cannot show.

built
6 modules in 13.4 s
deployed
needs_rolling_replace · 6 modules on 0 nodes · push→live —

Changed files

  • modified lib/openagents_web/controllers/issue_controller.ex
  • modified lib/openagents_web/controllers/issue_json.ex
  • modified test/openagents_web/controllers/issue_controller_test.exs

Diff

3 files changed, +155 -5

lib/openagents_web/controllers/issue_controller.ex modified +59 -3

@@ -6,14 +6,70 @@ defmodule OpenAgentsWeb.IssueController do

6 6 
  alias OpenAgents.Repositories
7 7
8 8 
  def index(conn, %{"owner" => owner, "repo" => repo} = params) do
9 
    state = Map.get(params, "state", "open")
10 9 
    repository = Repositories.get_visible_by_path!(owner, repo, conn.assigns[:current_user])
11 
    issues = Issues.list_issues(repository, state: state)
12 
    render(conn, :index, issues: issues, owner: owner, repo: repo)
10
11 
    with :ok <- validate_index_params(params),
12 
         {issues, total} <-
13 
           Issues.list_issues_page(repository, index_options(params)) do
14 
      render(conn, :index,
15 
        issues: issues,
16 
        owner: owner,
17 
        repo: repo,
18 
        pagination: %{
19 
          page: Issues.parse_page(params["page"]),
20 
          per_page: Issues.per_page(),
21 
          total: total
22 
        }
23 
      )
24 
    else
25 
      {:error, field, message} ->
26 
        conn |> put_status(:unprocessable_entity) |> json(%{errors: %{field => [message]}})
27 
    end
13 28 
  rescue
14 29 
    Ecto.NoResultsError -> not_found(conn)
15 30 
  end
16 31
32 
  @valid_states ~w(open closed all)
33
34 
  # Every list is bounded by Issues' fixed page size, and every filter value is
35 
  # either accepted as-is or rejected with one stable field-level error.
36 
  defp validate_index_params(params) do
37 
    state = Map.get(params, "state", "open")
38
39 
    cond do
40 
      state not in @valid_states ->
41 
        {:error, :state, "must be one of: #{Enum.join(@valid_states, ", ")}"}
42
43 
      Map.has_key?(params, "page") and not valid_page?(params["page"]) ->
44 
        {:error, :page, "must be a positive integer"}
45
46 
      true ->
47 
        :ok
48 
    end
49 
  end
50
51 
  defp valid_page?(value) when is_integer(value) and value >= 1, do: true
52
53 
  defp valid_page?(value) when is_binary(value) do
54 
    case Integer.parse(value) do
55 
      {number, ""} -> valid_page?(number)
56 
      _other -> false
57 
    end
58 
  end
59
60 
  defp valid_page?(_value), do: false
61
62 
  defp index_options(params) do
63 
    [
64 
      state: Map.get(params, "state", "open"),
65 
      label: params["labels"] || params["label"],
66 
      assignee: params["assignee"],
67 
      milestone: params["milestone"],
68 
      q: params["q"],
69 
      page: params["page"]
70 
    ]
71 
  end
72
17 73 
  def create(conn, %{"owner" => owner, "repo" => repo} = params) do
18 74 
    repository = Repositories.get_writable_by_path!(owner, repo, conn.assigns.current_user)
19 75
lib/openagents_web/controllers/issue_json.ex modified +14 -2

@@ -3,8 +3,16 @@ defmodule OpenAgentsWeb.IssueJSON do

3 3 
  Renders GitHub-compatible issue JSON.
4 4 
  """
5 5
6 
  def render("index.json", %{issues: issues} = assigns) do
7 
    %{issues: Enum.map(issues, &issue_json(&1, assigns))}
6 
  def render("index.json", %{issues: issues, pagination: pagination} = assigns) do
7 
    %{
8 
      issues: Enum.map(issues, &issue_json(&1, assigns)),
9 
      pagination: %{
10 
        page: pagination.page,
11 
        per_page: pagination.per_page,
12 
        total: pagination.total,
13 
        total_pages: total_pages(pagination.total, pagination.per_page)
14 
      }
15 
    }
8 16 
  end
9 17
10 18 
  def render("show.json", %{issue: issue} = assigns) do

@@ -42,6 +50,10 @@ defmodule OpenAgentsWeb.IssueJSON do

42 50 
    }
43 51 
  end
44 52
53 
  defp total_pages(0, _per_page), do: 1
54
55 
  defp total_pages(total, per_page), do: ceil(total / per_page)
56
45 57 
  defp url_base(assigns) do
46 58 
    Map.get(assigns, :url_base) || String.trim_trailing(OpenAgentsWeb.Endpoint.url(), "/")
47 59 
  end
test/openagents_web/controllers/issue_controller_test.exs modified +82

@@ -6,6 +6,9 @@ defmodule OpenAgentsWeb.IssueControllerTest do

6 6 
  alias OpenAgents.Issues
7 7 
  alias OpenAgents.Repositories
8 8
9 
  import OpenAgents.MilestonesFixtures
10 
  import OpenAgents.LabelsFixtures
11
9 12 
  describe "index" do
10 13 
    test "GET /api/v3/repos/:owner/:repo/issues lists open issues by default", %{
11 14 
      conn: conn

@@ -169,4 +172,83 @@ defmodule OpenAgentsWeb.IssueControllerTest do

169 172 
  defp repository do
170 173 
    Repositories.get_by_path!("OpenAgentsInc", "openagents.com")
171 174 
  end
175
176 
  describe "pagination and filters" do
177 
    test "index returns bounded pagination metadata", %{conn: conn} do
178 
      {:ok, _issue} = Issues.create_issue(repository(), %{title: "Counted issue"})
179
180 
      conn = get(conn, ~p"/api/v3/repos/OpenAgentsInc/openagents.com/issues")
181
182 
      assert %{"issues" => issues, "pagination" => pagination} = json_response(conn, 200)
183 
      assert length(issues) <= Issues.per_page()
184 
      assert pagination["page"] == 1
185 
      assert pagination["per_page"] == Issues.per_page()
186 
      assert pagination["total"] == 1
187 
      assert pagination["total_pages"] == 1
188 
    end
189
190 
    test "index filters by label, assignee, milestone, and search", %{conn: conn} do
191 
      milestone_fixture(repository(), %{number: 3, title: "Sprint 3"})
192 
      label_fixture(repository(), %{name: "bug", color: "d73a4a"})
193
194 
      octavia = github_user("assignee-filter", "octavia")
195 
      {:ok, _membership} = Repositories.add_member(repository(), octavia, "owner")
196
197 
      {:ok, _matched} =
198 
        Issues.create_issue(repository(), %{
199 
          title: "Wombat routing",
200 
          labels: [%{"name" => "bug"}],
201 
          assignees: [%{"login" => "octavia"}],
202 
          milestone: %{"number" => 3}
203 
        })
204
205 
      {:ok, _other} = Issues.create_issue(repository(), %{title: "Unrelated"})
206
207 
      for params <- [
208 
            %{"labels" => "bug"},
209 
            %{"assignee" => "octavia"},
210 
            %{"milestone" => "3"},
211 
            %{"q" => "wombat"}
212 
          ] do
213 
        conn = get(conn, ~p"/api/v3/repos/OpenAgentsInc/openagents.com/issues?#{params}")
214 
        assert %{"issues" => [issue], "pagination" => %{"total" => 1}} = json_response(conn, 200)
215 
        assert issue["title"] == "Wombat routing"
216 
      end
217 
    end
218
219 
    test "index pages through results in a stable order", %{conn: conn} do
220 
      Enum.each(1..30, fn n ->
221 
        {:ok, _} = Issues.create_issue(repository(), %{title: "Paged #{n}"})
222 
      end)
223
224 
      first = get(conn, ~p"/api/v3/repos/OpenAgentsInc/openagents.com/issues")
225
226 
      assert %{"issues" => page_one, "pagination" => %{"total_pages" => 2}} =
227 
               json_response(first, 200)
228
229 
      assert length(page_one) == Issues.per_page()
230
231 
      second =
232 
        get(conn, ~p"/api/v3/repos/OpenAgentsInc/openagents.com/issues?page=2")
233
234 
      assert %{"issues" => page_two} = json_response(second, 200)
235 
      assert length(page_two) == 30 - Issues.per_page()
236 
      page_one_titles = MapSet.new(page_one, & &1["title"])
237 
      refute Enum.any?(page_two, &MapSet.member?(page_one_titles, &1["title"]))
238 
    end
239
240 
    test "index rejects an unknown state with a stable error", %{conn: conn} do
241 
      conn = get(conn, ~p"/api/v3/repos/OpenAgentsInc/openagents.com/issues?state=bogus")
242
243 
      assert %{"errors" => %{"state" => [message]}} = json_response(conn, 422)
244 
      assert message =~ "open"
245 
    end
246
247 
    test "index rejects a non-integer page with a stable error", %{conn: conn} do
248 
      conn = get(conn, ~p"/api/v3/repos/OpenAgentsInc/openagents.com/issues?page=zero")
249
250 
      assert %{"errors" => %{"page" => [message]}} = json_response(conn, 422)
251 
      assert message =~ "positive integer"
252 
    end
253 
  end
172 254 
end

This page updates live while a promote is in flight · changelog