Refuse a self-request, and label the remove-reviewer control

0f83059 · Gabriel Morell · 2026-09-10 14:49

3 files +56 -4
Message
{commit_body(@commit)}

Files changed

modified lib/git_gud/pull_requests.ex
+7 −1
@@ -852,7 +852,13 @@ defmodule GitGud.PullRequests do
852 852 Ask `reviewer` to review `pr`. Idempotentasking someone who has
853 853 already been asked returns the existing request rather than erroring.
854 854 """
855 def request_review(%PullRequest{} = pr, %User{} = reviewer, requested_by \\ nil) do
855 + def request_review(pr, reviewer, requested_by \\ nil)
856 +
857 + # Asking yourself to review is a no-op dressed up as an action.
858 + def request_review(%PullRequest{}, %User{id: rid}, %User{id: rid}) when is_integer(rid),
859 + do: {:error, :self_request}
860 +
861 + def request_review(%PullRequest{} = pr, %User{} = reviewer, requested_by) do
856 862 attrs = %{
857 863 pull_request_id: pr.id,
858 864 reviewer_id: reviewer.id,
modified lib/git_gud_web/live/pr_live/reviews.ex
+6 −3
@@ -111,6 +111,9 @@ defmodule GitGudWeb.PrLive.Reviews do
111 111
112 112 {:noreply, reload(socket)}
113 113
114 + {:error, :self_request} ->
115 + {:noreply, put_flash(socket, :error, "You can't request a review from yourself.")}
116 +
114 117 {:error, _} ->
115 118 {:noreply, put_flash(socket, :error, "Could not request that review.")}
116 119 end
@@ -215,10 +218,10 @@ defmodule GitGudWeb.PrLive.Reviews do
215 218 type="button"
216 219 phx-click="remove_review_request"
217 220 phx-value-id={req.id}
218 class="btn btn-xs btn-ghost opacity-60 hover:opacity-100 ml-auto"
219 title="Withdraw request"
221 + class="btn btn-xs btn-ghost opacity-70 hover:opacity-100 ml-auto"
222 + aria-label={"Remove #{reviewer_name(req.reviewer)} as a reviewer"}
220 223 >
221 <.icon name="hero-x-mark" class="size-3" />
224 + <.icon name="hero-x-mark" class="size-3" /> Remove
222 225 </button>
223 226 </li>
224 227 </ul>
modified test/git_gud_web/live/pr_live/review_request_test.exs
+43 −0
@@ -52,6 +52,49 @@ defmodule GitGudWeb.PrLive.ReviewRequestTest do
52 52 assert length(PullRequests.list_review_requests(pr)) == 1
53 53 end
54 54
55 + test "you can't request a review from yourself", %{conn: conn} do
56 + {owner, repo} = repository_fixture(%{visibility: "public"})
57 + pr = pr_with_branches(repo, owner)
58 +
59 + {:ok, lv, _html} = live(log_in_user(conn, owner), pr_path(repo, pr))
60 + html = request!(lv, owner.handle)
61 +
62 + assert html =~ "You can"
63 + assert html =~ "request a review from yourself"
64 + assert PullRequests.list_review_requests(pr) == []
65 + end
66 +
67 + test "the context refuses a self-request whatever the caller", %{conn: _conn} do
68 + {owner, repo} = repository_fixture(%{visibility: "public"})
69 + pr = pr_with_branches(repo, owner)
70 +
71 + assert {:error, :self_request} = PullRequests.request_review(pr, owner, owner)
72 + end
73 +
74 + test "a maintainer can still ask someone else on their own PR", %{conn: conn} do
75 + {owner, repo} = repository_fixture(%{visibility: "public"})
76 + pr = pr_with_branches(repo, owner)
77 + other = user_fixture()
78 +
79 + {:ok, lv, _html} = live(log_in_user(conn, owner), pr_path(repo, pr))
80 + _ = request!(lv, other.handle)
81 +
82 + assert [req] = PullRequests.list_review_requests(pr)
83 + assert req.reviewer_id == other.id
84 + end
85 +
86 + test "the remove control is labelled, not just an icon", %{conn: conn} do
87 + {owner, repo} = repository_fixture(%{visibility: "public"})
88 + pr = pr_with_branches(repo, owner)
89 + reviewer = user_fixture()
90 +
91 + {:ok, lv, _html} = live(log_in_user(conn, owner), pr_path(repo, pr))
92 + html = request!(lv, reviewer.handle)
93 +
94 + assert html =~ "Remove"
95 + assert has_element?(lv, "button[phx-click=remove_review_request]", "Remove")
96 + end
97 +
55 98 test "an unknown handle is reported, not swallowed", %{conn: conn} do
56 99 {owner, repo} = repository_fixture(%{visibility: "public"})
57 100 pr = pr_with_branches(repo, owner)

Parents: c56c729