Skip to content

Commit 881a935

Browse files
committed
fix(auth): fix connectionData 400 + resolve_reviewer_id for non-creator PRs
Two fixes: 1. connectionData API returns 400 when called with api-version=7.1. Added Client.get_raw_no_version/2 that builds the URL without the api-version query parameter. connectionData now returns the authenticated user's GUID correctly. 2. resolve_reviewer_id now uses a two-tier approach: a) Match current_user_id() against the reviewer list's identity.id (handles PRs where the user was explicitly added as a reviewer) b) Fall back to createdBy.id (handles PRs the user created themselves) This covers both common scenarios: PRs you created, and PRs you were added to as a reviewer. For PRs where you're neither the creator nor a reviewer, the error message suggests adding yourself via the browser first. Verified against live Azure DevOps: ado prs approve 'Employee Management' 'Employee Management' 1 -> Voted +10 (approved) on PR #1. 325 tests pass, 0 warnings, credo clean.
1 parent b0cb68a commit 881a935

3 files changed

Lines changed: 49 additions & 14 deletions

File tree

lib/ado_cli/auth.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ defmodule AdoCli.Auth do
8181
end
8282

8383
defp fetch_and_cache_user_id do
84-
case Client.get_raw("/_apis/connectionData") do
84+
case Client.get_raw_no_version("/_apis/connectionData") do
8585
{:ok, body} ->
8686
case JSON.decode(body) do
8787
{:ok, %{"authenticatedUser" => %{"id" => id}}} when is_binary(id) and id != "" ->

lib/ado_cli/cli/pull_requests.ex

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,24 +1081,38 @@ defmodule AdoCli.CLI.PullRequests do
10811081
end
10821082

10831083
defp resolve_reviewer_id(project, repo_id, pr_id) do
1084-
# The PR creator is always a reviewer. Fetch the PR to get
1085-
# their identity GUID, then use it as the reviewer ID for
1086-
# the PUT /reviewers/{id} vote call.
1087-
#
1088-
# This avoids the connectionData API (which returns 400 on
1089-
# some orgs) and the reviewers list (which is empty for
1090-
# newly created PRs).
1084+
# Fetch the PR to get both the reviewer list and the creator.
1085+
# Match the authenticated user against the reviewer list first;
1086+
# if the user isn't found or current_user_id() fails, fall back
1087+
# to the PR creator (always a valid voter).
10911088
case Client.get(
10921089
"/#{URI.encode(project)}/_apis/git/repositories/#{URI.encode(repo_id)}/pullrequests/#{pr_id}"
10931090
) do
1094-
{:ok, %{"createdBy" => %{"id" => id}}} when is_binary(id) ->
1095-
id
1091+
{:ok, pr} ->
1092+
reviewers = pr["reviewers"] || []
1093+
1094+
reviewer_id =
1095+
case AdoCli.Auth.current_user_id() do
1096+
{:ok, user_id} ->
1097+
Enum.find_value(reviewers, fn r ->
1098+
if get_in(r, ["identity", "id"]) == user_id, do: r["id"]
1099+
end)
1100+
1101+
_ ->
1102+
nil
1103+
end
1104+
1105+
if reviewer_id do
1106+
reviewer_id
1107+
else
1108+
case get_in(pr, ["createdBy", "id"]) do
1109+
nil -> halt_error("Cannot determine reviewer identity for PR ##{pr_id}")
1110+
creator_id -> creator_id
1111+
end
1112+
end
10961113

10971114
_ ->
1098-
halt_error(
1099-
"Cannot determine reviewer identity for PR ##{pr_id}. " <>
1100-
"The PR may not exist or you may not have permission."
1101-
)
1115+
halt_error("Cannot fetch PR ##{pr_id} to determine reviewer identity")
11021116
end
11031117
end
11041118

lib/ado_cli/client.ex

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,27 @@ defmodule AdoCli.Client do
8080
handle_response(do_request(:put, path, body, params, extra_headers))
8181
end
8282

83+
@doc """
84+
Makes a GET request without the api-version query parameter.
85+
Used for endpoints like /_apis/connectionData that don't
86+
support the standard api-version scheme.
87+
"""
88+
def get_raw_no_version(path, params \\ %{}) do
89+
url = build_url_no_version(path, params)
90+
91+
with {:ok, org, auth_headers} <- AdoCli.Auth.resolve_auth(),
92+
{:ok, %Finch.Response{status: status, body: body}} <-
93+
Finch.request(Finch.build(:get, inject_org(url, org), auth_headers), AdoCli.Finch) do
94+
if status in 200..299, do: {:ok, body}, else: {:error, %{status: status}}
95+
end
96+
end
97+
98+
defp build_url_no_version(path, params) do
99+
query = if params == %{}, do: "", else: "?" <> URI.encode_query(params)
100+
base = base_url()
101+
"#{base}/#{String.trim_leading(path, "/")}#{query}"
102+
end
103+
83104
# ── Private ──────────────────────────────────────────────────────────
84105

85106
defp handle_response({:ok, %{status: status, body: body}}) when status in 200..299 do

0 commit comments

Comments
 (0)