GITGUD

neiam /gitgud

Git Gud

public · Issues · Pulls · Labels · Forks · Compare · Actions success · Packages

⭐

Log in to mark this repository.

Add assigned reviewers to pull requests

e17cdd6 · Gabriel Morell · 2026-09-10 14:27

9 files +435 -5
Message 
{commit_body(@commit)}

Files changed

modified lib/git_gud/events/event.ex
+1 −1
@@ -13,7 +13,7 @@ defmodule GitGud.Events.Event do
13 13 labeled unlabeled
14 14 moderated unmoderated
15 15 comment_added comment_edited comment_deleted
16 − reviewed
16 + reviewed review_requested review_request_removed
17 17 )
18 18
19 19 schema "events" do
modified lib/git_gud/pull_requests.ex
+69 −1
@@ -18,6 +18,7 @@ defmodule GitGud.PullRequests do
18 18 alias GitGud.PullRequests.PullRequest
19 19 alias GitGud.PullRequests.PrComment
20 20 alias GitGud.PullRequests.PrReview
21 + alias GitGud.PullRequests.PrReviewRequest
21 22 alias GitGud.Repo
22 23 alias GitGud.Repositories
23 24 alias GitGud.Repositories.Numbering
@@ -138,7 +139,8 @@ defmodule GitGud.PullRequests do
138 139 :author,
139 140 :merged_by,
140 141 comments: ^{comments_q, [:author, :source_actor]},
141 − reviews: ^{reviews_q, [:reviewer]}
142 + reviews: ^{reviews_q, [:reviewer]},
143 + review_requests: [:reviewer, :requested_by]
142 144 ])
143 145 |> Repo.one!()
144 146 |> attach_labels()
@@ -685,6 +687,72 @@ defmodule GitGud.PullRequests do
685 687 end
686 688 end
687 689
690 + # ── review requests ──────────────────────────────────────────────────
691 +
692 + @doc """
693 + Ask `reviewer` to review `pr`. Idempotent — asking someone who has
694 + already been asked returns the existing request rather than erroring.
695 + """
696 + def request_review(%PullRequest{} = pr, %User{} = reviewer, requested_by \\ nil) do
697 + attrs = %{
698 + pull_request_id: pr.id,
699 + reviewer_id: reviewer.id,
700 + requested_by_id: requested_by && requested_by.id
701 + }
702 +
703 + case %PrReviewRequest{} |> PrReviewRequest.changeset(attrs) |> Repo.insert() do
704 + {:ok, request} ->
705 + {:ok, Repo.preload(request, [:reviewer, :requested_by])}
706 +
707 + {:error, _cs} ->
708 + case Repo.get_by(PrReviewRequest, pull_request_id: pr.id, reviewer_id: reviewer.id) do
709 + nil -> {:error, :not_requestable}
710 + existing -> {:ok, Repo.preload(existing, [:reviewer, :requested_by])}
711 + end
712 + end
713 + end
714 +
715 + @doc "Withdraw a review request. Returns :ok whether or not one existed."
716 + def remove_review_request(%PullRequest{} = pr, %User{} = reviewer) do
717 + from(r in PrReviewRequest,
718 + where: r.pull_request_id == ^pr.id and r.reviewer_id == ^reviewer.id
719 + )
720 + |> Repo.delete_all()
721 +
722 + :ok
723 + end
724 +
725 + @doc "Everyone asked to review `pr`, with `:reviewer` loaded."
726 + def list_review_requests(%PullRequest{id: id}) do
727 + from(r in PrReviewRequest,
728 + where: r.pull_request_id == ^id,
729 + order_by: [asc: r.id],
730 + preload: [:reviewer, :requested_by]
731 + )
732 + |> Repo.all()
733 + end
734 +
735 + @doc """
736 + Open PRs where `user_id` has been asked to review, newest first.
737 +
738 + Unlike `list_open_awaiting_review/3` this is not inferred from
739 + maintainer status — someone actually asked.
740 + """
741 + def list_open_review_requests(user_id, opts \\ []) when is_integer(user_id) do
742 + limit = Keyword.get(opts, :limit, 10)
743 +
744 + from(p in PullRequest,
745 + join: req in PrReviewRequest,
746 + on: req.pull_request_id == p.id and req.reviewer_id == ^user_id,
747 + join: r in assoc(p, :repository),
748 + where: p.state == "open",
749 + order_by: [desc: req.id],
750 + limit: ^limit,
751 + preload: [:author, repository: {r, [:owner, :organization]}]
752 + )
753 + |> Repo.all()
754 + end
755 +
688 756 @doc """
689 757 The standing verdict per reviewer: their most recent review, newest
690 758 first.
added lib/git_gud/pull_requests/pr_review_request.ex
+26 −0
@@ -0,0 +1,26 @@
1 +defmodule GitGud.PullRequests.PrReviewRequest do
2 + use Ecto.Schema
3 + import Ecto.Changeset
4 +
5 + alias GitGud.Accounts.User
6 + alias GitGud.PullRequests.PullRequest
7 +
8 + schema "pr_review_requests" do
9 + belongs_to :pull_request, PullRequest
10 + belongs_to :reviewer, User
11 + belongs_to :requested_by, User
12 +
13 + timestamps(type: :utc_datetime, updated_at: false)
14 + end
15 +
16 + def changeset(request, attrs) do
17 + request
18 + |> cast(attrs, [:pull_request_id, :reviewer_id, :requested_by_id])
19 + |> validate_required([:pull_request_id, :reviewer_id])
20 + |> foreign_key_constraint(:reviewer_id)
21 + |> unique_constraint([:pull_request_id, :reviewer_id],
22 + name: :pr_review_requests_pull_request_id_reviewer_id_index,
23 + message: "has already been asked to review"
24 + )
25 + end
26 +end
modified lib/git_gud/pull_requests/pull_request.ex
+1 −0
@@ -47,6 +47,7 @@ defmodule GitGud.PullRequests.PullRequest do
47 47
48 48 has_many :comments, PrComment
49 49 has_many :reviews, PrReview
50 + has_many :review_requests, GitGud.PullRequests.PrReviewRequest
50 51
51 52 timestamps(type: :utc_datetime)
52 53 end
modified lib/git_gud_web/components/event_components.ex
+13 −1
@@ -149,7 +149,8 @@ defmodule GitGudWeb.EventComponents do
149 149 defp review_word(_), do: nil
150 150
151 151 defp dot_class("description_changed"), do: "bg-base-300"
152 − defp dot_class("reviewed"), do: "bg-accent"
152 + defp dot_class(k) when k in ["reviewed", "review_requested"], do: "bg-accent"
153 + defp dot_class("review_request_removed"), do: "bg-base-300"
153 154 defp dot_class(k) when k in ["comment_added", "comment_edited"], do: "bg-info"
154 155 defp dot_class("comment_deleted"), do: "bg-error"
155 156 defp dot_class("merged"), do: "bg-secondary"
@@ -223,6 +224,17 @@ defmodule GitGudWeb.EventComponents do
223 224
224 225 defp describe(%{kind: "reviewed"}), do: "reviewed this."
225 226
227 + defp describe(%{kind: "review_requested", data: %{"reviewer" => who}}) when is_binary(who),
228 + do: "asked #{who} to review."
229 +
230 + defp describe(%{kind: "review_requested"}), do: "requested a review."
231 +
232 + defp describe(%{kind: "review_request_removed", data: %{"reviewer" => who}})
233 + when is_binary(who),
234 + do: "withdrew the review request for #{who}."
235 +
236 + defp describe(%{kind: "review_request_removed"}), do: "withdrew a review request."
237 +
226 238 defp describe(%{kind: "comment_added"}), do: "commented."
227 239 defp describe(%{kind: "comment_edited"}), do: "edited a comment."
228 240 defp describe(%{kind: "comment_deleted"}), do: "deleted a comment."
modified lib/git_gud_web/live/page_live/home.ex
+21 −2
@@ -63,6 +63,10 @@ defmodule GitGudWeb.PageLive.Home do
63 63 :my_issues,
64 64 Issues.list_open_authored_by(user.id, accessible, limit: @panel_limit)
65 65 )
66 + |> assign(
67 + :requested_reviews,
68 + PullRequests.list_open_review_requests(user.id, limit: @panel_limit)
69 + )
66 70 |> assign(
67 71 :review_queue,
68 72 PullRequests.list_open_awaiting_review(user.id, maintained, limit: @panel_limit)
@@ -100,6 +104,11 @@ defmodule GitGudWeb.PageLive.Home do
100 104
101 105 # ── view helpers ─────────────────────────────────────────────────────
102 106
107 + # Once someone has actually been asked, say so; otherwise the panel is
108 + # still inferring from maintainer status and should admit it.
109 + defp review_hint([]), do: "Open in repos you maintain, opened by someone else"
110 + defp review_hint(_requested), do: "Requested of you, plus repos you maintain"
111 +
103 112 defp signed_in?(%{user: %{}}), do: true
104 113 defp signed_in?(_), do: false
105 114
@@ -138,6 +147,7 @@ defmodule GitGudWeb.PageLive.Home do
138 147 user={@user}
139 148 my_prs={@my_prs}
140 149 my_issues={@my_issues}
150 + requested_reviews={@requested_reviews}
141 151 review_queue={@review_queue}
142 152 recent_repos={@recent_repos}
143 153 active_runs={@active_runs}
@@ -157,6 +167,7 @@ defmodule GitGudWeb.PageLive.Home do
157 167 attr :user, :map, required: true
158 168 attr :my_prs, :list, required: true
159 169 attr :my_issues, :list, required: true
170 + attr :requested_reviews, :list, required: true
160 171 attr :review_queue, :list, required: true
161 172 attr :recent_repos, :list, required: true
162 173 attr :active_runs, :list, required: true
@@ -187,10 +198,18 @@ defmodule GitGudWeb.PageLive.Home do
187 198 <div class="grid lg:grid-cols-2 gap-4">
188 199 <.panel
189 200 title="Needs your review"
190 − hint="Open in repos you maintain, opened by someone else"
201 + hint={review_hint(@requested_reviews)}
191 202 empty="Nothing waiting on you."
192 − count={length(@review_queue)}
203 + count={length(@requested_reviews) + length(@review_queue)}
193 204 >
205 + <.work_row
206 + :for={pr <- @requested_reviews}
207 + path={~p"/r/#{repo_handle(pr.repository)}/#{pr.repository.name}/pulls/#{pr.number}"}
208 + repo={repo_path(pr.repository)}
209 + number={pr.number}
210 + title={pr.title}
211 + meta={"you were asked to review · #{ago(pr.inserted_at)}"}
212 + />
194 213 <.work_row
195 214 :for={pr <- @review_queue}
196 215 path={~p"/r/#{repo_handle(pr.repository)}/#{pr.repository.name}/pulls/#{pr.number}"}
modified lib/git_gud_web/live/pr_live/show.ex
+109 −0
@@ -149,6 +149,17 @@ defmodule GitGudWeb.PrLive.Show do
149 149
150 150 defp my_review(_assigns), do: nil
151 151
152 + # Asking for a review is the author's or a maintainer's call — the
153 + # same people who can close it.
154 + defp can_request_review?(assigns), do: can_change_state?(assigns)
155 +
156 + defp requested_reviewers(pr), do: pr.review_requests
157 +
158 + # Someone asked to review who hasn't left a verdict yet.
159 + defp awaiting_review?(pr, request) do
160 + not Enum.any?(pr.reviews, &(&1.reviewer_id == request.reviewer_id))
161 + end
162 +
152 163 defp can_review?(%{current_scope: %{user: %{}}}), do: true
153 164 defp can_review?(_assigns), do: false
154 165
@@ -429,6 +440,60 @@ defmodule GitGudWeb.PrLive.Show do
429 440 end
430 441 end
431 442
443 + def handle_event("request_review", %{"handle" => handle}, socket) do
444 + pr = socket.assigns.pr
445 +
446 + cond do
447 + not can_request_review?(socket.assigns) ->
448 + {:noreply, put_flash(socket, :error, "Only the author or a repo admin can do that.")}
449 +
450 + String.trim(handle) == "" ->
451 + {:noreply, socket}
452 +
453 + true ->
454 + case GitGud.Accounts.get_user_by_handle(String.trim(handle)) do
455 + nil ->
456 + {:noreply, put_flash(socket, :error, "No such user.")}
457 +
458 + reviewer ->
459 + case PullRequests.request_review(pr, reviewer, viewer(socket)) do
460 + {:ok, _request} ->
461 + :ok =
462 + Events.record(pr, "review_requested", viewer(socket), %{
463 + reviewer: reviewer.handle
464 + })
465 +
466 + {:noreply,
467 + socket
468 + |> assign(:event_count, Events.count_for(pr))
469 + |> reload()}
470 +
471 + {:error, _} ->
472 + {:noreply, put_flash(socket, :error, "Could not request that review.")}
473 + end
474 + end
475 + end
476 + end
477 +
478 + def handle_event("remove_review_request", %{"id" => id}, socket) do
479 + pr = socket.assigns.pr
480 +
481 + with true <- can_request_review?(socket.assigns),
482 + request when not is_nil(request) <-
483 + Enum.find(pr.review_requests, &(&1.id == String.to_integer(id))) do
484 + :ok = PullRequests.remove_review_request(pr, request.reviewer)
485 +
486 + :ok =
487 + Events.record(pr, "review_request_removed", viewer(socket), %{
488 + reviewer: request.reviewer && request.reviewer.handle
489 + })
490 +
491 + {:noreply, socket |> assign(:event_count, Events.count_for(pr)) |> reload()}
492 + else
493 + _ -> {:noreply, socket}
494 + end
495 + end
496 +
432 497 def handle_event("add_review", %{"pr_review" => attrs}, socket) do
433 498 case socket.assigns.current_scope && socket.assigns.current_scope.user do
434 499 nil ->
@@ -730,6 +795,50 @@ defmodule GitGudWeb.PrLive.Show do
730 795 </section>
731 796
732 797 <section>
798 + <h2 class="font-semibold mb-2">Reviewers</h2>
799 +
800 + <ul :if={requested_reviewers(@pr) != []} class="space-y-1 mb-2">
801 + <li
802 + :for={req <- requested_reviewers(@pr)}
803 + class="flex items-center gap-2 text-sm"
804 + >
805 + <.avatar name={reviewer_name(req.reviewer)} size="xs" alt={reviewer_name(req.reviewer)} />
806 + <span>{reviewer_name(req.reviewer)}</span>
807 + <span :if={awaiting_review?(@pr, req)} class="badge badge-xs badge-ghost">
808 + awaiting review
809 + </span>
810 + <button
811 + :if={can_request_review?(assigns)}
812 + type="button"
813 + phx-click="remove_review_request"
814 + phx-value-id={req.id}
815 + class="btn btn-xs btn-ghost opacity-60 hover:opacity-100 ml-auto"
816 + title="Withdraw request"
817 + >
818 + <.icon name="hero-x-mark" class="size-3" />
819 + </button>
820 + </li>
821 + </ul>
822 +
823 + <p :if={requested_reviewers(@pr) == []} class="text-sm opacity-60 mb-2">
824 + Nobody has been asked to review yet.
825 + </p>
826 +
827 + <form
828 + :if={can_request_review?(assigns)}
829 + phx-submit="request_review"
830 + class="flex items-end gap-2 mb-4"
831 + >
832 + <input
833 + type="text"
834 + name="handle"
835 + placeholder="handle"
836 + aria-label="Request a review from"
837 + class="input input-bordered input-xs font-mono flex-1"
838 + />
839 + <button type="submit" class="btn btn-xs">Request review</button>
840 + </form>
841 +
733 842 <h2 class="font-semibold mb-2">Reviews</h2>
734 843
735 844 <p :if={@pr.reviews == []} class="text-sm opacity-60">No reviews yet.</p>
added priv/repo/migrations/20260910010000_create_pr_review_requests.exs
+24 −0
@@ -0,0 +1,24 @@
1 +defmodule GitGud.Repo.Migrations.CreatePrReviewRequests do
2 + use Ecto.Migration
3 +
4 + # Who has been asked to review a pull request.
5 + #
6 + # Separate from `pr_reviews`, which is the append-only log of verdicts
7 + # actually left: a request is outstanding until the person reviews,
8 + # and stays on the row afterwards so the PR page can show that their
9 + # verdict was asked for rather than volunteered.
10 + def change do
11 + create table(:pr_review_requests) do
12 + add :pull_request_id, references(:pull_requests, on_delete: :delete_all), null: false
13 + add :reviewer_id, references(:users, on_delete: :delete_all), null: false
14 + add :requested_by_id, references(:users, on_delete: :nilify_all)
15 +
16 + timestamps(type: :utc_datetime, updated_at: false)
17 + end
18 +
19 + # One outstanding request per person per PR; asking twice is a no-op.
20 + create unique_index(:pr_review_requests, [:pull_request_id, :reviewer_id])
21 + # "What am I being asked to review?" — the dashboard's query.
22 + create index(:pr_review_requests, [:reviewer_id])
23 + end
24 +end
added test/git_gud_web/live/pr_live/review_request_test.exs
+171 −0
@@ -0,0 +1,171 @@
1 +defmodule GitGudWeb.PrLive.ReviewRequestTest do
2 + @moduledoc """
3 + Assigning reviewers to a pull request, and the dashboard panel that
4 + stops guessing once someone has actually been asked.
5 + """
6 +
7 + use GitGudWeb.ConnCase, async: false
8 +
9 + import Phoenix.LiveViewTest
10 + import GitGud.AccountsFixtures
11 + import GitGud.ForgeFixtures
12 +
13 + alias GitGud.Events
14 + alias GitGud.PullRequests
15 + alias GitGud.Repositories
16 +
17 + defp pr_path(repo, pr) do
18 + handle = Repositories.Storage.repo_handle(GitGud.Repo.preload(repo, [:owner, :organization]))
19 + ~p"/r/#{handle}/#{repo.name}/pulls/#{pr.number}"
20 + end
21 +
22 + defp request!(lv, handle) do
23 + lv |> form("form[phx-submit=request_review]", %{handle: handle}) |> render_submit()
24 + end
25 +
26 + test "a maintainer can ask someone to review", %{conn: conn} do
27 + {owner, repo} = repository_fixture(%{visibility: "public"})
28 + pr = pr_with_branches(repo, owner)
29 + reviewer = user_fixture()
30 +
31 + {:ok, lv, html} = live(log_in_user(conn, owner), pr_path(repo, pr))
32 + assert html =~ "Nobody has been asked to review yet."
33 +
34 + html = request!(lv, reviewer.handle)
35 +
36 + assert html =~ reviewer.handle
37 + assert html =~ "awaiting review"
38 + assert [req] = PullRequests.list_review_requests(pr)
39 + assert req.reviewer_id == reviewer.id
40 + assert req.requested_by_id == owner.id
41 + end
42 +
43 + test "asking the same person twice is a no-op, not an error", %{conn: conn} do
44 + {owner, repo} = repository_fixture(%{visibility: "public"})
45 + pr = pr_with_branches(repo, owner)
46 + reviewer = user_fixture()
47 +
48 + {:ok, lv, _html} = live(log_in_user(conn, owner), pr_path(repo, pr))
49 + _ = request!(lv, reviewer.handle)
50 + _ = request!(lv, reviewer.handle)
51 +
52 + assert length(PullRequests.list_review_requests(pr)) == 1
53 + end
54 +
55 + test "an unknown handle is reported, not swallowed", %{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, "nobody-by-that-name")
61 +
62 + assert html =~ "No such user."
63 + assert PullRequests.list_review_requests(pr) == []
64 + end
65 +
66 + test "an unrelated user gets no request form and can't force one", %{conn: conn} do
67 + {_owner, repo} = repository_fixture(%{visibility: "public"})
68 + author = user_fixture()
69 + pr = pr_with_branches(repo, author)
70 +
71 + outsider = user_fixture()
72 + {:ok, lv, _html} = live(log_in_user(conn, outsider), pr_path(repo, pr))
73 +
74 + refute has_element?(lv, "form[phx-submit=request_review]")
75 + render_hook(lv, "request_review", %{"handle" => outsider.handle})
76 +
77 + assert PullRequests.list_review_requests(pr) == []
78 + end
79 +
80 + test "a request can be withdrawn", %{conn: conn} do
81 + {owner, repo} = repository_fixture(%{visibility: "public"})
82 + pr = pr_with_branches(repo, owner)
83 + reviewer = user_fixture()
84 +
85 + {:ok, lv, _html} = live(log_in_user(conn, owner), pr_path(repo, pr))
86 + _ = request!(lv, reviewer.handle)
87 +
88 + [req] = PullRequests.list_review_requests(pr)
89 + html = render_hook(lv, "remove_review_request", %{"id" => to_string(req.id)})
90 +
91 + assert html =~ "Nobody has been asked to review yet."
92 + assert PullRequests.list_review_requests(pr) == []
93 + end
94 +
95 + test "the awaiting badge clears once they review", %{conn: conn} do
96 + {owner, repo} = repository_fixture(%{visibility: "public"})
97 + pr = pr_with_branches(repo, owner)
98 + reviewer = user_fixture()
99 +
100 + {:ok, lv, _html} = live(log_in_user(conn, owner), pr_path(repo, pr))
101 + _ = request!(lv, reviewer.handle)
102 +
103 + {:ok, rlv, html} = live(log_in_user(conn, reviewer), pr_path(repo, pr))
104 + assert html =~ "awaiting review"
105 +
106 + html =
107 + rlv
108 + |> form("#review-form", %{"pr_review" => %{"state" => "approved", "body" => ""}})
109 + |> render_submit()
110 +
111 + refute html =~ "awaiting review"
112 + end
113 +
114 + test "requesting and withdrawing are recorded by handle", %{conn: conn} do
115 + {owner, repo} = repository_fixture(%{visibility: "public"})
116 + pr = pr_with_branches(repo, owner)
117 + reviewer = user_fixture()
118 +
119 + {:ok, lv, _html} = live(log_in_user(conn, owner), pr_path(repo, pr))
120 + _ = request!(lv, reviewer.handle)
121 + [req] = PullRequests.list_review_requests(pr)
122 + render_hook(lv, "remove_review_request", %{"id" => to_string(req.id)})
123 +
124 + kinds = pr |> Events.list_for() |> Enum.map(& &1.kind)
125 + assert kinds == ["review_requested", "review_request_removed"]
126 +
127 + {:ok, _hist, html} = live(log_in_user(conn, owner), pr_path(repo, pr) <> "/history")
128 + assert html =~ "asked #{reviewer.handle} to review."
129 + assert html =~ "withdrew the review request for #{reviewer.handle}."
130 + end
131 +
132 + describe "the dashboard panel" do
133 + test "a requested review appears for the reviewer", %{conn: conn} do
134 + {owner, repo} = repository_fixture(%{visibility: "public"})
135 + pr = pr_with_branches(repo, owner, %{"title" => "Please review this"})
136 + reviewer = user_fixture()
137 +
138 + {:ok, _req} = PullRequests.request_review(pr, reviewer, owner)
139 +
140 + {:ok, _lv, html} = live(log_in_user(conn, reviewer), ~p"/")
141 +
142 + assert html =~ "Please review this"
143 + assert html =~ "you were asked to review"
144 + assert html =~ "Requested of you, plus repos you maintain"
145 + end
146 +
147 + test "the maintainer inference still fills the panel otherwise", %{conn: conn} do
148 + {owner, repo} = repository_fixture(%{visibility: "public"})
149 + author = user_fixture()
150 + _pr = pr_with_branches(repo, author, %{"title" => "Inferred entry"})
151 +
152 + {:ok, _lv, html} = live(log_in_user(conn, owner), ~p"/")
153 +
154 + assert html =~ "Inferred entry"
155 + assert html =~ "Open in repos you maintain"
156 + end
157 +
158 + test "a closed PR's request drops off the panel", %{conn: conn} do
159 + {owner, repo} = repository_fixture(%{visibility: "public"})
160 + pr = pr_with_branches(repo, owner, %{"title" => "Closed request"})
161 + reviewer = user_fixture()
162 +
163 + {:ok, _req} = PullRequests.request_review(pr, reviewer, owner)
164 + {:ok, _pr} = PullRequests.set_state(pr, "closed")
165 +
166 + {:ok, _lv, html} = live(log_in_user(conn, reviewer), ~p"/")
167 +
168 + refute html =~ "Closed request"
169 + end
170 + end
171 +end

Parents: 4d15a64