From 96bb942695b52aaf312a7f5abf7f2312a976e006 Mon Sep 17 00:00:00 2001 From: bougyman's bot Date: Sun, 4 Oct 2026 01:40:15 -0400 Subject: [PATCH] fix(issues): paginate team member lookup --- app/lib/linear_cli/linear/user.ex | 21 +- .../cli/commands/issues/mutations_test.exs | 236 +++++++++++++++--- .../linear_cli/cli/profile_defaults_test.exs | 25 +- app/test/linear_cli/linear/user_test.exs | 60 ++++- documents/ash-domain-erd.adoc | 5 +- 5 files changed, 281 insertions(+), 66 deletions(-) diff --git a/app/lib/linear_cli/linear/user.ex b/app/lib/linear_cli/linear/user.ex index a6eac2f..c1d797c 100644 --- a/app/lib/linear_cli/linear/user.ex +++ b/app/lib/linear_cli/linear/user.ex @@ -82,25 +82,8 @@ defmodule LinearCli.Linear.User.Read.ByTeam do @moduledoc false use Ash.Resource.ManualRead - alias LinearCli.Api - alias LinearCli.Linear.User - - def read(query, _ecto_query, _opts, _context) do - team_id = query.arguments.team_id - - document = - "query($id: String!) { team(id: $id) { members(first: 50) { nodes { #{User.base_fields()} } } } }" - - case Api.call(document, %{"id" => team_id}) do - {:ok, %{"team" => %{"members" => %{"nodes" => nodes}}}} -> - {:ok, Enum.map(nodes, &User.from_map/1)} - - {:ok, _} -> - {:ok, []} - - error -> - error - end + def read(query, ecto_query, opts, context) do + LinearCli.Linear.User.Read.ByTeamForLookup.read(query, ecto_query, opts, context) end end diff --git a/app/test/linear_cli/cli/commands/issues/mutations_test.exs b/app/test/linear_cli/cli/commands/issues/mutations_test.exs index 411bcb8..30d84ec 100644 --- a/app/test/linear_cli/cli/commands/issues/mutations_test.exs +++ b/app/test/linear_cli/cli/commands/issues/mutations_test.exs @@ -11,7 +11,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do end defp assignee_members_response(members) do - %{"data" => %{"team" => %{"members" => %{"nodes" => members}}}} + workspace_members_page(members, false, nil) end defp workspace_teams_response(teams) do @@ -92,6 +92,50 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do end) end + defp run_team_member_lookup_error(status, mode, test_pid) do + halt = fn code -> send(test_pid, {:halted, code}) end + + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + decoded = Jason.decode!(body) + query = decoded["query"] + + cond do + mode == :assign and String.contains?(query, "issue(id: $id)") -> + Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) + + String.contains?(query, "members(first: 50, after: $after)") -> + Plug.Conn.resp(conn, status, "upstream unavailable") + + mode == :filter and String.contains?(query, "team(id: $id)") -> + Req.Test.json(conn, %{"data" => %{"team" => team_map()}}) + + true -> + raise "a team member lookup error must stop before later API calls" + end + end) + + args = + case mode do + :assign -> + ["issue", "assign", "--assignee", "Ada", "CRY-1"] + + :filter -> + [ + "issue", + "unassign", + "--no-profile", + "--team", + "ENG", + "--assignee", + "Ada", + "--yes" + ] + end + + capture_stderr(fn stderr -> LinearCli.CLI.main(args, halt, stderr: stderr) end) + end + describe "issue unassign edge cases" do test "sends a null assignee and confirms each issue in text output" do test_pid = self() @@ -230,18 +274,14 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do query = decoded["query"] cond do - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json( conn, - %{ - "data" => %{ - "team" => %{ - "members" => %{ - "nodes" => [%{"id" => "u1", "name" => "Ada", "email" => "ada@example.com"}] - } - } - } - } + workspace_members_page( + [%{"id" => "u1", "name" => "Ada", "email" => "ada@example.com"}], + false, + nil + ) ) String.contains?(query, "team(id: $id)") -> @@ -283,6 +323,79 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do assert output =~ "CRY-1 unassigned" end + test "team-scoped assignee lookup follows later member pages" do + test_pid = self() + first_page_members = Enum.map(1..50, &%{"id" => "u#{&1}", "name" => "Member #{&1}"}) + late_member = %{"id" => "u-late", "name" => "Late Member"} + + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + decoded = Jason.decode!(body) + query = decoded["query"] + + cond do + String.contains?(query, "members(first: 50, after: $after)") -> + case decoded["variables"]["after"] do + nil -> + Req.Test.json(conn, workspace_members_page(first_page_members, true, "member-50")) + + "member-50" -> + Req.Test.json(conn, workspace_members_page([late_member], false, "member-51")) + end + + String.contains?(query, "team(id: $id)") -> + Req.Test.json(conn, %{"data" => %{"team" => team_map()}}) + + String.contains?(query, "issues(filter:") -> + send(test_pid, {:filter, decoded["variables"]["filter"]}) + Req.Test.json(conn, issues_response([])) + + String.contains?(query, "issueUpdate") -> + raise "a dry-run lookup must not mutate" + + true -> + raise "no stub matched query: #{query}" + end + end) + + output = + capture_io(fn -> + assert :ok = + LinearCli.CLI.main([ + "issue", + "unassign", + "--no-profile", + "--team", + "ENG", + "--assignee", + "Late Member", + "--dry-run" + ]) + end) + + assert output =~ "No issues matched." + assert_received {:filter, %{"assignee" => %{"id" => %{"eq" => "u-late"}}}} + end + + test "a team-scoped member HTTP error uses the API handler" do + output = run_team_member_lookup_error(502, :filter, self()) + + assert_received {:halted, 88} + assert output =~ "Linear API returned HTTP 502." + refute output =~ "What the heck is this?" + refute output =~ "upstream unavailable" + end + + test "a team-scoped member authentication error uses the auth handler" do + output = run_team_member_lookup_error(401, :filter, self()) + + assert_received {:halted, 77} + assert output =~ "Linear API authentication failed (HTTP 401)." + assert output =~ "Authentication error, cannot continue" + refute output =~ "What the heck is this?" + refute output =~ "upstream unavailable" + end + test "workspace-wide assignee lookup follows later member pages" do test_pid = self() first_page_members = Enum.map(1..50, &%{"id" => "u#{&1}", "name" => "Member #{&1}"}) @@ -895,7 +1008,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do query = decoded["query"] cond do - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json( conn, assignee_members_response([ @@ -1110,7 +1223,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do %{"query" => query} = Jason.decode!(body) cond do - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json( conn, assignee_members_response([ @@ -1435,7 +1548,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do %{"query" => query} = decoded = Jason.decode!(body) cond do - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json( conn, assignee_members_response([ @@ -2728,7 +2841,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do end defp members_response(members) do - %{"data" => %{"team" => %{"members" => %{"nodes" => members}}}} + workspace_members_page(members, false, nil) end defp issue_assigned(assignee_map) do @@ -2747,7 +2860,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json( conn, members_response([member_map("u2", "Bob"), member_map("u3", "Alice")]) @@ -2783,7 +2896,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json( conn, members_response([member_map("u2", "Bob"), member_map("u3", "Alice")]) @@ -2807,6 +2920,67 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do assert output =~ "assigned to Alice" end + test "--assignee resolves a member after the first page" do + test_pid = self() + first_page_members = Enum.map(1..50, &member_map("u#{&1}", "Member #{&1}")) + late_member = member_map("u-late", "Late Member") + + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + decoded = Jason.decode!(body) + query = decoded["query"] + + cond do + String.contains?(query, "issue(id: $id)") -> + Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) + + String.contains?(query, "members(first: 50, after: $after)") -> + case decoded["variables"]["after"] do + nil -> + Req.Test.json(conn, workspace_members_page(first_page_members, true, "member-50")) + + "member-50" -> + Req.Test.json(conn, workspace_members_page([late_member], false, "member-51")) + end + + String.contains?(query, "issueUpdate") -> + send(test_pid, {:assignee_id, decoded["variables"]["input"]["assigneeId"]}) + Req.Test.json(conn, issue_assigned(late_member)) + + true -> + raise "no stub matched query: #{query}" + end + end) + + output = + capture_io(fn -> + assert :ok = + LinearCli.CLI.main(["issue", "assign", "--assignee", "Late Member", "CRY-1"]) + end) + + assert_received {:assignee_id, "u-late"} + assert output =~ "assigned to Late Member" + end + + test "a team-scoped assignment member HTTP error uses the API handler" do + output = run_team_member_lookup_error(502, :assign, self()) + + assert_received {:halted, 88} + assert output =~ "Linear API returned HTTP 502." + refute output =~ "What the heck is this?" + refute output =~ "upstream unavailable" + end + + test "a team-scoped assignment authentication error uses the auth handler" do + output = run_team_member_lookup_error(401, :assign, self()) + + assert_received {:halted, 77} + assert output =~ "Linear API authentication failed (HTTP 401)." + assert output =~ "Authentication error, cannot continue" + refute output =~ "What the heck is this?" + refute output =~ "upstream unavailable" + end + test "--assignee with unknown name exits 22 (smells bad)" do test_pid = self() halt = fn code -> send(test_pid, {:halted, code}) end @@ -2819,7 +2993,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json( conn, members_response([member_map("u2", "Bob"), member_map("u3", "Alice")]) @@ -2856,7 +3030,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json( conn, members_response([member_map("u2", "Bob"), member_map("u3", "Bobby")]) @@ -2890,7 +3064,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json( conn, members_response([member_map("u2", "Bob"), member_map("u3", "Alice")]) @@ -2923,7 +3097,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([member_map("u2", "Bob")])) String.contains?(query, "issueUpdate") -> @@ -2955,7 +3129,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([member_map("u2", "Bob")])) String.contains?(query, "issueUpdate") -> @@ -2995,7 +3169,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([member_map("u2", "Bob")])) String.contains?(query, "issueUpdate") -> @@ -3026,7 +3200,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([])) true -> @@ -3056,7 +3230,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([member_map("u2", "Bob")])) String.contains?(query, "states {") -> @@ -3117,7 +3291,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([member_map("u2", "Bob")])) String.contains?(query, "states {") -> @@ -3153,7 +3327,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([member_map("u2", "Bob")])) String.contains?(query, "states {") -> @@ -3189,7 +3363,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([member_map("u2", "Bob")])) String.contains?(query, "states {") -> @@ -3230,7 +3404,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([member_map("u2", "Bob")])) String.contains?(query, "issueUpdate") -> @@ -3260,7 +3434,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([member_map("u2", "Bob")])) String.contains?(query, "states {") -> @@ -3314,7 +3488,7 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do String.contains?(query, "issue(id: $id)") -> Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json(conn, members_response([member_map("u2", "Bob")])) String.contains?(query, "states {") -> diff --git a/app/test/linear_cli/cli/profile_defaults_test.exs b/app/test/linear_cli/cli/profile_defaults_test.exs index d004645..f3010c6 100644 --- a/app/test/linear_cli/cli/profile_defaults_test.exs +++ b/app/test/linear_cli/cli/profile_defaults_test.exs @@ -45,6 +45,19 @@ defmodule LinearCli.CLI.ProfileDefaultsTest do defp team_projects(projects), do: %{"data" => %{"team" => %{"projects" => %{"nodes" => projects}}}} + defp team_members_page(members, has_next_page \\ false, end_cursor \\ nil) do + %{ + "data" => %{ + "team" => %{ + "members" => %{ + "edges" => Enum.map(members, &%{"node" => &1, "cursor" => &1["id"]}), + "pageInfo" => %{"hasNextPage" => has_next_page, "endCursor" => end_cursor} + } + } + } + } + end + defp label_response(names) do %{ "data" => %{ @@ -457,18 +470,10 @@ defmodule LinearCli.CLI.ProfileDefaultsTest do query = decoded["query"] cond do - String.contains?(query, "members(first: 50)") -> + String.contains?(query, "members(first: 50, after: $after)") -> Req.Test.json( conn, - %{ - "data" => %{ - "team" => %{ - "members" => %{ - "nodes" => [%{"id" => "u1", "name" => "Ada", "email" => "ada@example.com"}] - } - } - } - } + team_members_page([%{"id" => "u1", "name" => "Ada", "email" => "ada@example.com"}]) ) String.contains?(query, "team(id: $id)") -> diff --git a/app/test/linear_cli/linear/user_test.exs b/app/test/linear_cli/linear/user_test.exs index ace798b..2846fc3 100644 --- a/app/test/linear_cli/linear/user_test.exs +++ b/app/test/linear_cli/linear/user_test.exs @@ -9,10 +9,17 @@ defmodule LinearCli.Linear.UserTest do "data" => %{ "team" => %{ "members" => %{ - "nodes" => [ - %{"id" => "u1", "name" => "Alice", "email" => "alice@example.com"}, - %{"id" => "u2", "name" => "Bob", "email" => "bob@example.com"} - ] + "edges" => [ + %{ + "node" => %{"id" => "u1", "name" => "Alice", "email" => "alice@example.com"}, + "cursor" => "member-1" + }, + %{ + "node" => %{"id" => "u2", "name" => "Bob", "email" => "bob@example.com"}, + "cursor" => "member-2" + } + ], + "pageInfo" => %{"hasNextPage" => false, "endCursor" => "member-2"} } } } @@ -23,6 +30,42 @@ defmodule LinearCli.Linear.UserTest do Linear.team_members("t1") end + test "team_members/1 follows every member page" do + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + decoded = Jason.decode!(body) + query = decoded["query"] + after_cursor = decoded["variables"]["after"] + + assert query =~ "members(first: 50, after: $after)" + + {members, page_info} = + case after_cursor do + nil -> + {[%{"id" => "u1", "name" => "First", "email" => "first@example.com"}], + %{"hasNextPage" => true, "endCursor" => "member-1"}} + + "member-1" -> + {[%{"id" => "u2", "name" => "Later", "email" => "later@example.com"}], + %{"hasNextPage" => false, "endCursor" => "member-2"}} + end + + Req.Test.json(conn, %{ + "data" => %{ + "team" => %{ + "members" => %{ + "edges" => Enum.map(members, &%{"node" => &1, "cursor" => &1["id"]}), + "pageInfo" => page_info + } + } + } + }) + end) + + assert {:ok, members} = Linear.team_members("t1") + assert Enum.map(members, & &1.id) == ["u1", "u2"] + end + test "team_members/1 returns an empty list when team is null" do Req.Test.stub(LinearCli.Api, fn conn -> Req.Test.json(conn, %{"data" => %{"team" => nil}}) @@ -120,6 +163,15 @@ defmodule LinearCli.Linear.UserTest do assert {:error, %Ash.Error.Unknown{}} = Linear.team_members("t1") end + test "team_members/1 normalizes HTTP errors" do + Req.Test.stub(LinearCli.Api, fn conn -> + Plug.Conn.resp(conn, 401, "upstream unavailable") + end) + + assert {:error, %Ash.Error.Unknown{errors: [%{value: [{:http_error, 401}]} | _]}} = + Linear.team_members("t1") + end + test "me/0 returns an unexpected_response error when viewer is null" do # Linear returns {"data": {"viewer": null}} when the API key is valid but # refers to an account that no longer exists. Guard `when is_map(viewer)` diff --git a/documents/ash-domain-erd.adoc b/documents/ash-domain-erd.adoc index 921212e..c0aa87c 100644 --- a/documents/ash-domain-erd.adoc +++ b/documents/ash-domain-erd.adoc @@ -305,7 +305,7 @@ manual-implementation module, and the Linear GraphQL operation it calls. | `:by_team` | read | `Linear.User.Read.ByTeam` -| `team(id: $id) { members(first: 50) { ... } }` — one page for team-scoped assignment +| `team(id: $id) { members(first: 50, after: $after) { ... } }` — follows every page for team-scoped assignment and filtering | `User` | `workspace_team_members` @@ -584,7 +584,8 @@ helper, which caps the batch at 100 and reports more matches. Ordinary issue listing keeps the existing permissive project resolution and 100-record cap. Workspace project lookup uses `Project.workspace_projects`. Workspace assignee lookup follows every page of every accessible team and deduplicates members by -user ID. Team-scoped lookup keeps the existing one-page member action. +user ID. Team-scoped lookup follows every member page for assignment and +assignee filtering. === `LinearCli.Linear.Paginate`