Skip to content

CRY-58: fix(linear): normalize malformed responses in User.Read.ByTeam - #147

Merged
bougyman merged 1 commit into
mainfrom
CRY-58-normalize-user-read-by-team
Aug 18, 2026
Merged

CRY-58: fix(linear): normalize malformed responses in User.Read.ByTeam#147
bougyman merged 1 commit into
mainfrom
CRY-58-normalize-user-read-by-team

Conversation

@bougyman

Copy link
Copy Markdown
Member

Summary

Fixes a defensive-coding gap in User.Read.ByTeam where malformed or
partially-populated API responses could leak a bare map out of the else
clause instead of returning a proper {:ok, []} or {:error, _} tuple.

  • Replace the two-step with match (extract team, then match members) with a
    single deep pattern match, consistent with Label.Read.ByTeam,
    WorkflowState.Read.ByTeam, and Project.Read.ByTeam.
  • The else catch-all {:ok, _} -> {:ok, []} now correctly handles null
    team, missing members key, and missing nodes key as graceful empty
    results.
  • Add 5 new tests covering: valid response, null team, missing members key,
    missing nodes key, and API error propagation.

Closes CRY-58.

Test plan

  • mix test test/linear_cli/linear/user_test.exs — all 6 tests pass
  • mix compile --warnings-as-errors — clean
  • mix test — 260/267 pass; 7 failures are pre-existing GitTest environment failures unrelated to this change

🤖 Generated with Claude Code

Replace the fragile two-step with-match with a single deep pattern match
consistent with Label.Read.ByTeam, WorkflowState.Read.ByTeam, and
Project.Read.ByTeam.  The prior code let a bare map escape the else clause
when the team object existed but the members/nodes keys were absent, which
would propagate up to callers expecting an ok/error tuple.

Add test coverage for valid responses, null team, missing members key,
missing nodes key, and API errors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 18, 2026 14:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@bougyman

Copy link
Copy Markdown
Member Author

Code Review

Issue: CRY-58 — Normalize malformed responses in User.Read.ByTeam

Summary

Single-commit change that replaces a fragile two-step with pattern in User.Read.ByTeam.read/4 with a single deep pattern match, and adds 5 tests covering all edge cases. Clean, minimal, and correct.

Correctness

No issues found. The fix correctly addresses the root cause:

  • Before: The two-step with ({:ok, %{"team" => team}} when is_map(team) then %{"members" => %{"nodes" => nodes}} <- team) let the else clause's error -> error return a bare map when team existed but lacked members or nodes. Callers expect {:ok, list} or {:error, _}.
  • After: A single deep match {:ok, %{"team" => %{"members" => %{"nodes" => nodes}}}} ensures all three keys must be present. Any other {:ok, _} shape falls to {:ok, []} — correct graceful degradation.

I verified Api.call (api.ex:25-54) always returns {:ok, data} or {:error, _}, so the else clause is exhaustive:

  • {:ok, _} → null team, missing members, missing nodes → {:ok, []}
  • error → API/network errors → passed through as-is

Consistency

The pattern now matches all three sibling ByTeam modules exactly:

  • Label.Read.ByTeam (label.ex:107): {:ok, %{"team" => %{"labels" => %{"nodes" => nodes}}}}
  • WorkflowState.Read.ByTeam (workflow_state.ex:62): {:ok, %{"team" => %{"states" => %{"nodes" => nodes}}}}
  • Project.Read.ByTeam (project.ex:183): {:ok, %{"team" => %{"projects" => %{"nodes" => nodes}}}}

The User module is actually more defensive than the siblings — those three omit the else clause entirely, meaning they would leak raw response maps on null team. That's pre-existing and out of scope, but worth noting.

Tests

Five new tests covering all specified edge cases via the code interface (Linear.team_members/1):

  1. Valid response with multiple users
  2. Null team → {:ok, []}
  3. Missing members key → {:ok, []}
  4. Missing nodes key → {:ok, []}
  5. API error propagation → {:error, %Ash.Error.Unknown{}}

Tests use Req.Test.stub consistently with the existing me/0 test.

Quality Suite

  • Compilation: Clean (no warnings)
  • Tests: 262/267 pass. 5 failures are pre-existing GitTest environment issues (worktree setup), unrelated to this change.

Overall Assessment

Approve. The change is minimal, correct, well-tested, and consistent with project conventions. No issues found.

@bougyman
bougyman merged commit 304c202 into main Aug 18, 2026
2 checks passed
@bougyman
bougyman deleted the CRY-58-normalize-user-read-by-team branch August 18, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants