neiam /gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Packages
⭐
Log in to mark this repository.
Move reviewer controls onto their own tab
f0ed5e3 · Gabriel Morell · 2026-09-10 14:40
Message
{commit_body(@commit)}
Files changed
modified
lib/git_gud_web/components/pr_nav.ex
+8
−1
@@ -12,9 +12,10 @@ defmodule GitGudWeb.PrNav do
| 12 | 12 | attr :handle, :string, required: true |
| 13 | 13 | attr :repo, :map, required: true |
| 14 | 14 | attr :pr, :map, required: true |
| 15 | − attr :active, :atom, required: true, values: [:conversation, :files, :history] | |
| 15 | + attr :active, :atom, required: true, values: [:conversation, :files, :reviews, :history] | |
| 16 | 16 | attr :files_count, :integer, default: nil |
| 17 | 17 | attr :comments_count, :integer, default: nil |
| 18 | + attr :reviews_count, :integer, default: nil | |
| 18 | 19 | attr :events_count, :integer, default: nil |
| 19 | 20 | |
| 20 | 21 | def nav(assigns) do |
@@ -32,6 +33,12 @@ defmodule GitGudWeb.PrNav do
| 32 | 33 | label="Files changed" |
| 33 | 34 | count={@files_count} |
| 34 | 35 | /> |
| 36 | + <.tab | |
| 37 | + navigate={~p"/r/#{@handle}/#{@repo.name}/pulls/#{@pr.number}/reviews"} | |
| 38 | + active?={@active == :reviews} | |
| 39 | + label="Reviews" | |
| 40 | + count={@reviews_count} | |
| 41 | + /> | |
| 35 | 42 | <.tab |
| 36 | 43 | navigate={~p"/r/#{@handle}/#{@repo.name}/pulls/#{@pr.number}/history"} |
| 37 | 44 | active?={@active == :history} |
added
lib/git_gud_web/live/pr_live/reviews.ex
+311
−0
@@ -0,0 +1,311 @@
| 1 | +defmodule GitGudWeb.PrLive.Reviews do | |
| 2 | + @moduledoc """ | |
| 3 | + The review screen: who has been asked to review, what verdict each of | |
| 4 | + them currently holds, and the form for leaving or changing your own. | |
| 5 | + | |
| 6 | + Split off the conversation page for the same reason the diff was — | |
| 7 | + these are controls you use while reviewing, not while reading the | |
| 8 | + discussion, and they were competing for the same column. | |
| 9 | + """ | |
| 10 | + | |
| 11 | + use GitGudWeb, :live_view | |
| 12 | + | |
| 13 | + alias GitGud.Events | |
| 14 | + alias GitGud.PullRequests | |
| 15 | + alias GitGud.PullRequests.PrReview | |
| 16 | + alias GitGud.Repositories | |
| 17 | + alias GitGud.Repositories.Storage | |
| 18 | + | |
| 19 | + @impl true | |
| 20 | + def mount(%{"owner" => owner, "name" => name, "number" => num}, _session, socket) do | |
| 21 | + repo = | |
| 22 | + Repositories.get_repository_by_path!(owner, name) | |
| 23 | + |> GitGud.Repo.preload([:owner, :organization]) | |
| 24 | + | |
| 25 | + pr = PullRequests.get_pull_request!(repo, String.to_integer(num)) | |
| 26 | + | |
| 27 | + {:ok, | |
| 28 | + socket | |
| 29 | + |> assign(:repo, repo) | |
| 30 | + |> assign(:handle, Storage.repo_handle(repo)) | |
| 31 | + |> GitGudWeb.RepoLive.Header.assign_chrome(repo) | |
| 32 | + |> assign(:pr, pr) | |
| 33 | + |> assign_review_form() | |
| 34 | + |> assign(:page_title, "Reviews · ##{pr.number} #{pr.title}")} | |
| 35 | + end | |
| 36 | + | |
| 37 | + defp reload(socket) do | |
| 38 | + pr = PullRequests.get_pull_request!(socket.assigns.repo, socket.assigns.pr.number) | |
| 39 | + socket |> assign(:pr, pr) |> assign_review_form() | |
| 40 | + end | |
| 41 | + | |
| 42 | + defp viewer(socket), do: socket.assigns.current_scope && socket.assigns.current_scope.user | |
| 43 | + | |
| 44 | + defp assign_review_form(socket) do | |
| 45 | + mine = my_review(socket.assigns) | |
| 46 | + | |
| 47 | + assign( | |
| 48 | + socket, | |
| 49 | + :review_form, | |
| 50 | + to_form(PrReview.changeset(%PrReview{}, %{state: (mine && mine.state) || "approved"})) | |
| 51 | + ) | |
| 52 | + end | |
| 53 | + | |
| 54 | + defp my_review(%{current_scope: %{user: %{} = user}, pr: pr}), | |
| 55 | + do: PullRequests.review_by(pr, user) | |
| 56 | + | |
| 57 | + defp my_review(_assigns), do: nil | |
| 58 | + | |
| 59 | + defp can_review?(%{current_scope: %{user: %{}}}), do: true | |
| 60 | + defp can_review?(_assigns), do: false | |
| 61 | + | |
| 62 | + # Asking for a review is the author's or a maintainer's call — the | |
| 63 | + # same people who can close it. | |
| 64 | + defp can_request_review?(%{current_scope: %{user: %{id: uid}}} = assigns), | |
| 65 | + do: assigns.can_admin? or assigns.pr.author_id == uid | |
| 66 | + | |
| 67 | + defp can_request_review?(_assigns), do: false | |
| 68 | + | |
| 69 | + defp requested_reviewers(pr), do: pr.review_requests | |
| 70 | + | |
| 71 | + defp awaiting_review?(pr, request), | |
| 72 | + do: not Enum.any?(pr.reviews, &(&1.reviewer_id == request.reviewer_id)) | |
| 73 | + | |
| 74 | + defp conversation_count(pr), do: Enum.count(pr.comments, &is_nil(&1.file_path)) | |
| 75 | + | |
| 76 | + defp review_badge("approved"), do: "badge-success" | |
| 77 | + defp review_badge("changes_requested"), do: "badge-error" | |
| 78 | + defp review_badge(_), do: "badge-ghost" | |
| 79 | + | |
| 80 | + defp review_word("approved"), do: "approved" | |
| 81 | + defp review_word("changes_requested"), do: "requested changes" | |
| 82 | + defp review_word(_), do: "commented" | |
| 83 | + | |
| 84 | + defp reviewer_name(%{handle: h}) when is_binary(h), do: h | |
| 85 | + defp reviewer_name(%{email: e}) when is_binary(e), do: e |> String.split("@") |> hd() | |
| 86 | + defp reviewer_name(_), do: "anonymous" | |
| 87 | + | |
| 88 | + @impl true | |
| 89 | + def handle_event("request_review", %{"handle" => handle}, socket) do | |
| 90 | + pr = socket.assigns.pr | |
| 91 | + | |
| 92 | + cond do | |
| 93 | + not can_request_review?(socket.assigns) -> | |
| 94 | + {:noreply, put_flash(socket, :error, "Only the author or a repo admin can do that.")} | |
| 95 | + | |
| 96 | + String.trim(handle) == "" -> | |
| 97 | + {:noreply, socket} | |
| 98 | + | |
| 99 | + true -> | |
| 100 | + case GitGud.Accounts.get_user_by_handle(String.trim(handle)) do | |
| 101 | + nil -> | |
| 102 | + {:noreply, put_flash(socket, :error, "No such user.")} | |
| 103 | + | |
| 104 | + reviewer -> | |
| 105 | + case PullRequests.request_review(pr, reviewer, viewer(socket)) do | |
| 106 | + {:ok, _request} -> | |
| 107 | + :ok = | |
| 108 | + Events.record(pr, "review_requested", viewer(socket), %{ | |
| 109 | + reviewer: reviewer.handle | |
| 110 | + }) | |
| 111 | + | |
| 112 | + {:noreply, reload(socket)} | |
| 113 | + | |
| 114 | + {:error, _} -> | |
| 115 | + {:noreply, put_flash(socket, :error, "Could not request that review.")} | |
| 116 | + end | |
| 117 | + end | |
| 118 | + end | |
| 119 | + end | |
| 120 | + | |
| 121 | + def handle_event("remove_review_request", %{"id" => id}, socket) do | |
| 122 | + pr = socket.assigns.pr | |
| 123 | + | |
| 124 | + with true <- can_request_review?(socket.assigns), | |
| 125 | + request when not is_nil(request) <- | |
| 126 | + Enum.find(pr.review_requests, &(&1.id == String.to_integer(id))) do | |
| 127 | + :ok = PullRequests.remove_review_request(pr, request.reviewer) | |
| 128 | + | |
| 129 | + :ok = | |
| 130 | + Events.record(pr, "review_request_removed", viewer(socket), %{ | |
| 131 | + reviewer: request.reviewer && request.reviewer.handle | |
| 132 | + }) | |
| 133 | + | |
| 134 | + {:noreply, reload(socket)} | |
| 135 | + else | |
| 136 | + _ -> {:noreply, socket} | |
| 137 | + end | |
| 138 | + end | |
| 139 | + | |
| 140 | + def handle_event("add_review", %{"pr_review" => attrs}, socket) do | |
| 141 | + case viewer(socket) do | |
| 142 | + nil -> | |
| 143 | + {:noreply, put_flash(socket, :error, "Sign in to review.")} | |
| 144 | + | |
| 145 | + user -> | |
| 146 | + previous = my_review(socket.assigns) | |
| 147 | + pr = socket.assigns.pr | |
| 148 | + | |
| 149 | + case PullRequests.add_review(pr, user, attrs) do | |
| 150 | + {:ok, review} -> | |
| 151 | + :ok = | |
| 152 | + Events.record(pr, "reviewed", user, %{ | |
| 153 | + from: previous && previous.state, | |
| 154 | + to: review.state | |
| 155 | + }) | |
| 156 | + | |
| 157 | + {:noreply, reload(socket)} | |
| 158 | + | |
| 159 | + {:error, cs} -> | |
| 160 | + {:noreply, assign(socket, :review_form, to_form(cs))} | |
| 161 | + end | |
| 162 | + end | |
| 163 | + end | |
| 164 | + | |
| 165 | + @impl true | |
| 166 | + def render(assigns) do | |
| 167 | + ~H""" | |
| 168 | + <Layouts.app flash={@flash} current_scope={@current_scope}> | |
| 169 | + <div class="space-y-4"> | |
| 170 | + <GitGudWeb.RepoLive.Header.header | |
| 171 | + repo={@repo} | |
| 172 | + handle={@handle} | |
| 173 | + current_scope={@current_scope} | |
| 174 | + has_packages?={@has_packages?} | |
| 175 | + can_admin?={@can_admin?} | |
| 176 | + latest_run={@latest_run} | |
| 177 | + open_pulls={@open_pulls} | |
| 178 | + /> | |
| 179 | + | |
| 180 | + <header> | |
| 181 | + <p class="text-xs opacity-60"> | |
| 182 | + <.link navigate={~p"/r/#{@handle}/#{@repo.name}/pulls"} class="link link-hover"> | |
| 183 | + Back to pull requests | |
| 184 | + </.link> | |
| 185 | + </p> | |
| 186 | + <h1 class="text-2xl font-semibold">#{@pr.number} — {@pr.title}</h1> | |
| 187 | + </header> | |
| 188 | + | |
| 189 | + <GitGudWeb.PrNav.nav | |
| 190 | + handle={@handle} | |
| 191 | + repo={@repo} | |
| 192 | + pr={@pr} | |
| 193 | + active={:reviews} | |
| 194 | + comments_count={conversation_count(@pr)} | |
| 195 | + reviews_count={length(PullRequests.latest_reviews(@pr))} | |
| 196 | + events_count={Events.count_for(@pr)} | |
| 197 | + /> | |
| 198 | + | |
| 199 | + <section> | |
| 200 | + <h2 class="font-semibold mb-2">Reviewers</h2> | |
| 201 | + | |
| 202 | + <ul :if={requested_reviewers(@pr) != []} class="space-y-1 mb-2"> | |
| 203 | + <li :for={req <- requested_reviewers(@pr)} class="flex items-center gap-2 text-sm"> | |
| 204 | + <.avatar | |
| 205 | + name={reviewer_name(req.reviewer)} | |
| 206 | + size="xs" | |
| 207 | + alt={reviewer_name(req.reviewer)} | |
| 208 | + /> | |
| 209 | + <span>{reviewer_name(req.reviewer)}</span> | |
| 210 | + <span :if={awaiting_review?(@pr, req)} class="badge badge-xs badge-ghost"> | |
| 211 | + awaiting review | |
| 212 | + </span> | |
| 213 | + <button | |
| 214 | + :if={can_request_review?(assigns)} | |
| 215 | + type="button" | |
| 216 | + phx-click="remove_review_request" | |
| 217 | + phx-value-id={req.id} | |
| 218 | + class="btn btn-xs btn-ghost opacity-60 hover:opacity-100 ml-auto" | |
| 219 | + title="Withdraw request" | |
| 220 | + > | |
| 221 | + <.icon name="hero-x-mark" class="size-3" /> | |
| 222 | + </button> | |
| 223 | + </li> | |
| 224 | + </ul> | |
| 225 | + | |
| 226 | + <p :if={requested_reviewers(@pr) == []} class="text-sm opacity-60 mb-2"> | |
| 227 | + Nobody has been asked to review yet. | |
| 228 | + </p> | |
| 229 | + | |
| 230 | + <form | |
| 231 | + :if={can_request_review?(assigns)} | |
| 232 | + phx-submit="request_review" | |
| 233 | + class="flex items-end gap-2" | |
| 234 | + > | |
| 235 | + <input | |
| 236 | + type="text" | |
| 237 | + name="handle" | |
| 238 | + placeholder="handle" | |
| 239 | + aria-label="Request a review from" | |
| 240 | + class="input input-bordered input-xs font-mono flex-1 max-w-xs" | |
| 241 | + /> | |
| 242 | + <button type="submit" class="btn btn-xs">Request review</button> | |
| 243 | + </form> | |
| 244 | + </section> | |
| 245 | + | |
| 246 | + <section> | |
| 247 | + <h2 class="font-semibold mb-2">Reviews</h2> | |
| 248 | + | |
| 249 | + <p :if={@pr.reviews == []} class="text-sm opacity-60">No reviews yet.</p> | |
| 250 | + | |
| 251 | + <%!-- The standing verdict per reviewer. Reviews stay | |
| 252 | + append-only, so this is the latest each of them left. --%> | |
| 253 | + <ul :if={@pr.reviews != []} class="space-y-2"> | |
| 254 | + <li | |
| 255 | + :for={r <- PullRequests.latest_reviews(@pr)} | |
| 256 | + class="border border-base-300 rounded p-3 text-sm" | |
| 257 | + > | |
| 258 | + <p class="opacity-60 text-xs mb-1 flex items-center gap-1.5"> | |
| 259 | + <span class={["badge badge-xs", review_badge(r.state)]}>{review_word(r.state)}</span> | |
| 260 | + {reviewer_name(r.reviewer)} | |
| 261 | + </p> | |
| 262 | + <p :if={r.body}>{r.body}</p> | |
| 263 | + </li> | |
| 264 | + </ul> | |
| 265 | + | |
| 266 | + <details | |
| 267 | + :if={length(@pr.reviews) > length(PullRequests.latest_reviews(@pr))} | |
| 268 | + class="mt-2 text-xs" | |
| 269 | + > | |
| 270 | + <summary class="cursor-pointer opacity-60 hover:opacity-100 select-none"> | |
| 271 | + Show all {length(@pr.reviews)} reviews | |
| 272 | + </summary> | |
| 273 | + <ul class="mt-2 space-y-1"> | |
| 274 | + <li :for={r <- Enum.reverse(@pr.reviews)} class="opacity-70"> | |
| 275 | + <span class={["badge badge-xs", review_badge(r.state)]}>{review_word(r.state)}</span> | |
| 276 | + {reviewer_name(r.reviewer)} | |
| 277 | + </li> | |
| 278 | + </ul> | |
| 279 | + </details> | |
| 280 | + | |
| 281 | + <.form | |
| 282 | + :if={can_review?(assigns)} | |
| 283 | + for={@review_form} | |
| 284 | + id="review-form" | |
| 285 | + phx-submit="add_review" | |
| 286 | + class="mt-3 space-y-2 max-w-lg" | |
| 287 | + > | |
| 288 | + <p :if={my_review(assigns)} class="text-xs opacity-70"> | |
| 289 | + You {review_word(my_review(assigns).state)}. Submitting again changes your review. | |
| 290 | + </p> | |
| 291 | + <.input | |
| 292 | + field={@review_form[:state]} | |
| 293 | + type="select" | |
| 294 | + label="Review" | |
| 295 | + options={[ | |
| 296 | + {"Approve", "approved"}, | |
| 297 | + {"Request changes", "changes_requested"}, | |
| 298 | + {"Comment", "commented"} | |
| 299 | + ]} | |
| 300 | + /> | |
| 301 | + <.input field={@review_form[:body]} type="textarea" rows="3" label="Body (optional)" /> | |
| 302 | + <button type="submit" class="btn btn-xs btn-primary"> | |
| 303 | + {if my_review(assigns), do: "Update review", else: "Submit review"} | |
| 304 | + </button> | |
| 305 | + </.form> | |
| 306 | + </section> | |
| 307 | + </div> | |
| 308 | + </Layouts.app> | |
| 309 | + """ | |
| 310 | + end | |
| 311 | +end |
modified
lib/git_gud_web/live/pr_live/show.ex
+11
−220
@@ -8,7 +8,6 @@ defmodule GitGudWeb.PrLive.Show do
| 8 | 8 | alias GitGud.Labels |
| 9 | 9 | alias GitGud.PullRequests |
| 10 | 10 | alias GitGud.PullRequests.PrComment |
| 11 | − alias GitGud.PullRequests.PrReview | |
| 12 | 11 | alias GitGud.Repositories |
| 13 | 12 | alias GitGud.Repositories.Storage |
| 14 | 13 |
@@ -56,7 +55,6 @@ defmodule GitGudWeb.PrLive.Show do
| 56 | 55 | |> assign(:loaded_diff_idx, GitGudWeb.DiffComponents.default_loaded(diff_files)) |
| 57 | 56 | |> assign(:all_labels, Labels.list_labels(repo)) |
| 58 | 57 | |> assign_comment_form() |
| 59 | − |> assign_review_form() | |
| 60 | 58 | |> assign(:comment_body, "") |
| 61 | 59 | |> assign(:editing_title?, false) |
| 62 | 60 | |> assign(:editing_comment_id, nil) |
@@ -133,44 +131,11 @@ defmodule GitGudWeb.PrLive.Show do
| 133 | 131 | defp assign_comment_form(socket), |
| 134 | 132 | do: assign(socket, :comment_form, to_form(PrComment.changeset(%PrComment{}, %{}))) |
| 135 | 133 | |
| 136 | − defp assign_review_form(socket) do | |
| 137 | − mine = my_review(socket.assigns) | |
| 138 | − | |
| 139 | − assign( | |
| 140 | − socket, | |
| 141 | − :review_form, | |
| 142 | − to_form(PrReview.changeset(%PrReview{}, %{state: (mine && mine.state) || "approved"})) | |
| 143 | − ) | |
| 144 | − end | |
| 145 | − | |
| 146 | − defp my_review(%{current_scope: %{user: %{} = user}, pr: pr}), | |
| 147 | − do: PullRequests.review_by(pr, user) | |
| 148 | − | |
| 149 | − defp my_review(_assigns), do: nil | |
| 150 | − | |
| 151 | − # Asking for a review is the author's or a maintainer's call — the | |
| 152 | − # same people who can close it. | |
| 153 | − defp can_request_review?(assigns), do: can_change_state?(assigns) | |
| 154 | − | |
| 155 | 134 | defp blocking_reviewers(pr), |
| 156 | 135 | do: pr |> PullRequests.blocking_reviews() |> Enum.map(&reviewer_name(&1.reviewer)) |
| 157 | 136 | |
| 158 | 137 | defp review_blocked?(repo, pr), do: PullRequests.review_blocked?(repo, pr) |
| 159 | 138 | |
| 160 | − defp requested_reviewers(pr), do: pr.review_requests | |
| 161 | − | |
| 162 | − # Someone asked to review who hasn't left a verdict yet. | |
| 163 | − defp awaiting_review?(pr, request) do | |
| 164 | − not Enum.any?(pr.reviews, &(&1.reviewer_id == request.reviewer_id)) | |
| 165 | − end | |
| 166 | − | |
| 167 | − defp can_review?(%{current_scope: %{user: %{}}}), do: true | |
| 168 | − defp can_review?(_assigns), do: false | |
| 169 | − | |
| 170 | − defp review_badge("approved"), do: "badge-success" | |
| 171 | − defp review_badge("changes_requested"), do: "badge-error" | |
| 172 | − defp review_badge(_), do: "badge-ghost" | |
| 173 | − | |
| 174 | 139 | defp review_word("approved"), do: "approved" |
| 175 | 140 | defp review_word("changes_requested"), do: "requested changes" |
| 176 | 141 | defp review_word(_), do: "commented" |
@@ -391,89 +356,6 @@ defmodule GitGudWeb.PrLive.Show do
| 391 | 356 | end |
| 392 | 357 | end |
| 393 | 358 | |
| 394 | − def handle_event("request_review", %{"handle" => handle}, socket) do | |
| 395 | − pr = socket.assigns.pr | |
| 396 | − | |
| 397 | − cond do | |
| 398 | − not can_request_review?(socket.assigns) -> | |
| 399 | − {:noreply, put_flash(socket, :error, "Only the author or a repo admin can do that.")} | |
| 400 | − | |
| 401 | − String.trim(handle) == "" -> | |
| 402 | − {:noreply, socket} | |
| 403 | − | |
| 404 | − true -> | |
| 405 | − case GitGud.Accounts.get_user_by_handle(String.trim(handle)) do | |
| 406 | − nil -> | |
| 407 | − {:noreply, put_flash(socket, :error, "No such user.")} | |
| 408 | − | |
| 409 | − reviewer -> | |
| 410 | − case PullRequests.request_review(pr, reviewer, viewer(socket)) do | |
| 411 | − {:ok, _request} -> | |
| 412 | − :ok = | |
| 413 | − Events.record(pr, "review_requested", viewer(socket), %{ | |
| 414 | − reviewer: reviewer.handle | |
| 415 | − }) | |
| 416 | − | |
| 417 | − {:noreply, | |
| 418 | − socket | |
| 419 | − |> assign(:event_count, Events.count_for(pr)) | |
| 420 | − |> reload()} | |
| 421 | − | |
| 422 | − {:error, _} -> | |
| 423 | − {:noreply, put_flash(socket, :error, "Could not request that review.")} | |
| 424 | − end | |
| 425 | − end | |
| 426 | − end | |
| 427 | − end | |
| 428 | − | |
| 429 | − def handle_event("remove_review_request", %{"id" => id}, socket) do | |
| 430 | − pr = socket.assigns.pr | |
| 431 | − | |
| 432 | − with true <- can_request_review?(socket.assigns), | |
| 433 | − request when not is_nil(request) <- | |
| 434 | − Enum.find(pr.review_requests, &(&1.id == String.to_integer(id))) do | |
| 435 | − :ok = PullRequests.remove_review_request(pr, request.reviewer) | |
| 436 | − | |
| 437 | − :ok = | |
| 438 | − Events.record(pr, "review_request_removed", viewer(socket), %{ | |
| 439 | − reviewer: request.reviewer && request.reviewer.handle | |
| 440 | − }) | |
| 441 | − | |
| 442 | − {:noreply, socket |> assign(:event_count, Events.count_for(pr)) |> reload()} | |
| 443 | − else | |
| 444 | − _ -> {:noreply, socket} | |
| 445 | − end | |
| 446 | − end | |
| 447 | − | |
| 448 | − def handle_event("add_review", %{"pr_review" => attrs}, socket) do | |
| 449 | − case socket.assigns.current_scope && socket.assigns.current_scope.user do | |
| 450 | − nil -> | |
| 451 | − {:noreply, put_flash(socket, :error, "Sign in to review.")} | |
| 452 | − | |
| 453 | − user -> | |
| 454 | − previous = my_review(socket.assigns) | |
| 455 | − pr = socket.assigns.pr | |
| 456 | − | |
| 457 | − case PullRequests.add_review(pr, user, attrs) do | |
| 458 | − {:ok, review} -> | |
| 459 | − :ok = | |
| 460 | − Events.record(pr, "reviewed", user, %{ | |
| 461 | − from: previous && previous.state, | |
| 462 | − to: review.state | |
| 463 | − }) | |
| 464 | − | |
| 465 | − {:noreply, | |
| 466 | − socket | |
| 467 | − |> assign(:event_count, Events.count_for(pr)) | |
| 468 | − |> reload() | |
| 469 | − |> assign_review_form()} | |
| 470 | − | |
| 471 | − {:error, cs} -> | |
| 472 | − {:noreply, assign(socket, :review_form, to_form(cs))} | |
| 473 | − end | |
| 474 | − end | |
| 475 | − end | |
| 476 | − | |
| 477 | 359 | def handle_event("merge", _params, socket) do |
| 478 | 360 | user = socket.assigns.current_scope.user |
| 479 | 361 |
@@ -777,109 +659,18 @@ defmodule GitGudWeb.PrLive.Show do
| 777 | 659 | </section> |
| 778 | 660 | |
| 779 | 661 | <section> |
| 780 | − <h2 class="font-semibold mb-2">Reviewers</h2> | |
| 781 | − | |
| 782 | − <ul :if={requested_reviewers(@pr) != []} class="space-y-1 mb-2"> | |
| 783 | − <li | |
| 784 | − :for={req <- requested_reviewers(@pr)} | |
| 785 | − class="flex items-center gap-2 text-sm" | |
| 786 | − > | |
| 787 | − <.avatar name={reviewer_name(req.reviewer)} size="xs" alt={reviewer_name(req.reviewer)} /> | |
| 788 | − <span>{reviewer_name(req.reviewer)}</span> | |
| 789 | − <span :if={awaiting_review?(@pr, req)} class="badge badge-xs badge-ghost"> | |
| 790 | − awaiting review | |
| 791 | − </span> | |
| 792 | − <button | |
| 793 | − :if={can_request_review?(assigns)} | |
| 794 | − type="button" | |
| 795 | − phx-click="remove_review_request" | |
| 796 | − phx-value-id={req.id} | |
| 797 | − class="btn btn-xs btn-ghost opacity-60 hover:opacity-100 ml-auto" | |
| 798 | − title="Withdraw request" | |
| 799 | − > | |
| 800 | − <.icon name="hero-x-mark" class="size-3" /> | |
| 801 | − </button> | |
| 802 | − </li> | |
| 803 | − </ul> | |
| 804 | − | |
| 805 | − <p :if={requested_reviewers(@pr) == []} class="text-sm opacity-60 mb-2"> | |
| 806 | − Nobody has been asked to review yet. | |
| 807 | − </p> | |
| 808 | − | |
| 809 | − <form | |
| 810 | − :if={can_request_review?(assigns)} | |
| 811 | − phx-submit="request_review" | |
| 812 | − class="flex items-end gap-2 mb-4" | |
| 813 | − > | |
| 814 | − <input | |
| 815 | − type="text" | |
| 816 | − name="handle" | |
| 817 | − placeholder="handle" | |
| 818 | − aria-label="Request a review from" | |
| 819 | − class="input input-bordered input-xs font-mono flex-1" | |
| 820 | − /> | |
| 821 | − <button type="submit" class="btn btn-xs">Request review</button> | |
| 822 | − </form> | |
| 823 | − | |
| 824 | − <h2 class="font-semibold mb-2">Reviews</h2> | |
| 825 | − | |
| 826 | − <p :if={@pr.reviews == []} class="text-sm opacity-60">No reviews yet.</p> | |
| 827 | − | |
| 828 | − <%!-- The standing verdict per reviewer. Reviews stay | |
| 829 | − append-only, so this is the latest each of them left. --%> | |
| 830 | − <ul :if={@pr.reviews != []} class="space-y-2"> | |
| 831 | − <li | |
| 832 | − :for={r <- PullRequests.latest_reviews(@pr)} | |
| 833 | − class="border border-base-300 rounded p-3 text-sm" | |
| 834 | − > | |
| 835 | − <p class="opacity-60 text-xs mb-1 flex items-center gap-1.5"> | |
| 836 | − <span class={["badge badge-xs", review_badge(r.state)]}>{review_word(r.state)}</span> | |
| 837 | − {reviewer_name(r.reviewer)} · {format_dt(r.inserted_at)} | |
| 838 | − </p> | |
| 839 | − <p :if={r.body}>{r.body}</p> | |
| 840 | − </li> | |
| 841 | − </ul> | |
| 842 | − | |
| 843 | − <details | |
| 844 | − :if={length(@pr.reviews) > length(PullRequests.latest_reviews(@pr))} | |
| 845 | − class="mt-2 text-xs" | |
| 846 | − > | |
| 847 | − <summary class="cursor-pointer opacity-60 hover:opacity-100 select-none"> | |
| 848 | − Show all {length(@pr.reviews)} reviews | |
| 849 | − </summary> | |
| 850 | − <ul class="mt-2 space-y-1"> | |
| 851 | − <li :for={r <- Enum.reverse(@pr.reviews)} class="opacity-70"> | |
| 852 | − <span class={["badge badge-xs", review_badge(r.state)]}>{review_word(r.state)}</span> | |
| 853 | − {reviewer_name(r.reviewer)} · {format_dt(r.inserted_at)} | |
| 854 | − </li> | |
| 855 | − </ul> | |
| 856 | − </details> | |
| 857 | − | |
| 858 | − <.form | |
| 859 | − :if={can_review?(assigns)} | |
| 860 | − for={@review_form} | |
| 861 | − id="review-form" | |
| 862 | − phx-submit="add_review" | |
| 863 | − class="mt-3 space-y-2" | |
| 662 | + <.link | |
| 663 | + navigate={~p"/r/#{@handle}/#{@repo.name}/pulls/#{@pr.number}/reviews"} | |
| 664 | + class="flex items-center justify-between rounded-lg border border-base-300 p-3 transition-colors hover:border-primary/50" | |
| 864 | 665 | > |
| 865 | − <p :if={my_review(assigns)} class="text-xs opacity-70"> | |
| 866 | − You {review_word(my_review(assigns).state)}. Submitting again changes your review. | |
| 867 | − </p> | |
| 868 | − <.input | |
| 869 | − field={@review_form[:state]} | |
| 870 | − type="select" | |
| 871 | − label="Review" | |
| 872 | − options={[ | |
| 873 | − {"Approve", "approved"}, | |
| 874 | − {"Request changes", "changes_requested"}, | |
| 875 | − {"Comment", "commented"} | |
| 876 | − ]} | |
| 877 | − /> | |
| 878 | − <.input field={@review_form[:body]} type="textarea" rows="3" label="Body (optional)" /> | |
| 879 | − <button type="submit" class="btn btn-xs btn-primary"> | |
| 880 | − {if my_review(assigns), do: "Update review", else: "Submit review"} | |
| 881 | − </button> | |
| 882 | − </.form> | |
| 666 | + <span class="flex items-center gap-2 text-sm"> | |
| 667 | + <.icon name="hero-check-badge" class="size-4 text-primary" /> Reviewers and reviews | |
| 668 | + <span :if={blocking_reviewers(@pr) != []} class="badge badge-xs badge-error"> | |
| 669 | + changes requested | |
| 670 | + </span> | |
| 671 | + </span> | |
| 672 | + <.icon name="hero-arrow-right-micro" class="size-4 opacity-60" /> | |
| 673 | + </.link> | |
| 883 | 674 | </section> |
| 884 | 675 | |
| 885 | 676 | <section> |
modified
lib/git_gud_web/router.ex
+1
−0
@@ -218,6 +218,7 @@ defmodule GitGudWeb.Router do
| 218 | 218 | live "/r/:owner/:name/pulls/:number", PrLive.Show, :show |
| 219 | 219 | live "/r/:owner/:name/pulls/:number/history", PrLive.History, :index |
| 220 | 220 | live "/r/:owner/:name/pulls/:number/files", PrLive.Files, :index |
| 221 | + live "/r/:owner/:name/pulls/:number/reviews", PrLive.Reviews, :index | |
| 221 | 222 | live "/r/:owner/:name/wiki", WikiLive.Index, :index |
| 222 | 223 | live "/r/:owner/:name/wiki/:slug", WikiLive.Show, :show |
| 223 | 224 | live "/r/:owner/:name/actions", WorkflowLive.Index, :index |
modified
test/git_gud_web/live/pr_live/files_page_test.exs
+1
−0
@@ -62,6 +62,7 @@ defmodule GitGudWeb.PrLive.FilesPageTest do
| 62 | 62 | |
| 63 | 63 | assert html =~ "Conversation" |
| 64 | 64 | assert html =~ "Files changed" |
| 65 | + assert html =~ "Reviews" | |
| 65 | 66 | assert html =~ "History" |
| 66 | 67 | assert has_element?(lv, ~s{a[aria-current="page"]}, active) |
| 67 | 68 | end |
modified
test/git_gud_web/live/pr_live/review_block_test.exs
+3
−3
@@ -26,7 +26,7 @@ defmodule GitGudWeb.PrLive.ReviewBlockTest do
| 26 | 26 | end |
| 27 | 27 | |
| 28 | 28 | defp request_changes!(conn, repo, pr, reviewer) do |
| 29 | − {:ok, lv, _html} = live(log_in_user(conn, reviewer), pr_path(repo, pr)) | |
| 29 | + {:ok, lv, _html} = live(log_in_user(conn, reviewer), pr_path(repo, pr) <> "/reviews") | |
| 30 | 30 | |
| 31 | 31 | lv |
| 32 | 32 | |> form("#review-form", %{"pr_review" => %{"state" => "changes_requested", "body" => "No"}}) |
@@ -106,7 +106,7 @@ defmodule GitGudWeb.PrLive.ReviewBlockTest do
| 106 | 106 | _ = request_changes!(conn, repo, pr, reviewer) |
| 107 | 107 | repo = block!(repo) |
| 108 | 108 | |
| 109 | − {:ok, rlv, _html} = live(log_in_user(conn, reviewer), pr_path(repo, pr)) | |
| 109 | + {:ok, rlv, _html} = live(log_in_user(conn, reviewer), pr_path(repo, pr) <> "/reviews") | |
| 110 | 110 | |
| 111 | 111 | _ = |
| 112 | 112 | rlv |
@@ -128,7 +128,7 @@ defmodule GitGudWeb.PrLive.ReviewBlockTest do
| 128 | 128 | _ = request_changes!(conn, repo, pr, blocker) |
| 129 | 129 | repo = block!(repo) |
| 130 | 130 | |
| 131 | − {:ok, alv, _html} = live(log_in_user(conn, approver), pr_path(repo, pr)) | |
| 131 | + {:ok, alv, _html} = live(log_in_user(conn, approver), pr_path(repo, pr) <> "/reviews") | |
| 132 | 132 | |
| 133 | 133 | _ = |
| 134 | 134 | alv |
modified
test/git_gud_web/live/pr_live/review_request_test.exs
+3
−2
@@ -16,7 +16,7 @@ defmodule GitGudWeb.PrLive.ReviewRequestTest do
| 16 | 16 | |
| 17 | 17 | defp pr_path(repo, pr) do |
| 18 | 18 | handle = Repositories.Storage.repo_handle(GitGud.Repo.preload(repo, [:owner, :organization])) |
| 19 | − ~p"/r/#{handle}/#{repo.name}/pulls/#{pr.number}" | |
| 19 | + ~p"/r/#{handle}/#{repo.name}/pulls/#{pr.number}/reviews" | |
| 20 | 20 | end |
| 21 | 21 | |
| 22 | 22 | defp request!(lv, handle) do |
@@ -124,7 +124,8 @@ defmodule GitGudWeb.PrLive.ReviewRequestTest do
| 124 | 124 | kinds = pr |> Events.list_for() |> Enum.map(& &1.kind) |
| 125 | 125 | assert kinds == ["review_requested", "review_request_removed"] |
| 126 | 126 | |
| 127 | − {:ok, _hist, html} = live(log_in_user(conn, owner), pr_path(repo, pr) <> "/history") | |
| 127 | + hist = String.replace_suffix(pr_path(repo, pr), "/reviews", "/history") | |
| 128 | + {:ok, _hist, html} = live(log_in_user(conn, owner), hist) | |
| 128 | 129 | assert html =~ "asked #{reviewer.handle} to review." |
| 129 | 130 | assert html =~ "withdrew the review request for #{reviewer.handle}." |
| 130 | 131 | end |
modified
test/git_gud_web/live/pr_live/review_test.exs
+7
−4
@@ -16,7 +16,7 @@ defmodule GitGudWeb.PrLive.ReviewTest do
| 16 | 16 | |
| 17 | 17 | defp pr_path(repo, pr) do |
| 18 | 18 | handle = Repositories.Storage.repo_handle(GitGud.Repo.preload(repo, [:owner, :organization])) |
| 19 | − ~p"/r/#{handle}/#{repo.name}/pulls/#{pr.number}" | |
| 19 | + ~p"/r/#{handle}/#{repo.name}/pulls/#{pr.number}/reviews" | |
| 20 | 20 | end |
| 21 | 21 | |
| 22 | 22 | defp submit_review(lv, state, body \\ "") do |
@@ -126,7 +126,8 @@ defmodule GitGudWeb.PrLive.ReviewTest do
| 126 | 126 | assert event.data["to"] == "approved" |
| 127 | 127 | assert event.data["from"] == nil |
| 128 | 128 | |
| 129 | − {:ok, _hist, html} = live(log_in_user(conn, reviewer), pr_path(repo, pr) <> "/history") | |
| 129 | + hist = String.replace_suffix(pr_path(repo, pr), "/reviews", "/history") | |
| 130 | + {:ok, _hist, html} = live(log_in_user(conn, reviewer), hist) | |
| 130 | 131 | assert html =~ "reviewed:" |
| 131 | 132 | assert html =~ "approved" |
| 132 | 133 | end |
@@ -144,7 +145,8 @@ defmodule GitGudWeb.PrLive.ReviewTest do
| 144 | 145 | assert second.data["from"] == "approved" |
| 145 | 146 | assert second.data["to"] == "changes_requested" |
| 146 | 147 | |
| 147 | − {:ok, _hist, html} = live(log_in_user(conn, reviewer), pr_path(repo, pr) <> "/history") | |
| 148 | + hist = String.replace_suffix(pr_path(repo, pr), "/reviews", "/history") | |
| 149 | + {:ok, _hist, html} = live(log_in_user(conn, reviewer), hist) | |
| 148 | 150 | assert html =~ "changed their review from" |
| 149 | 151 | assert html =~ "changes requested" |
| 150 | 152 | end |
@@ -157,7 +159,8 @@ defmodule GitGudWeb.PrLive.ReviewTest do
| 157 | 159 | {:ok, lv, _html} = live(log_in_user(conn, reviewer), pr_path(repo, pr)) |
| 158 | 160 | _ = submit_review(lv, "approved") |
| 159 | 161 | |
| 160 | − {:ok, owner_lv, _html} = live(log_in_user(conn, owner), pr_path(repo, pr)) | |
| 162 | + conv = String.replace_suffix(pr_path(repo, pr), "/reviews", "") | |
| 163 | + {:ok, owner_lv, _html} = live(log_in_user(conn, owner), conv) | |
| 161 | 164 | _ = owner_lv |> element("button", "Close") |> render_click() |
| 162 | 165 | |
| 163 | 166 | kinds = pr |> Events.list_for() |> Enum.map(& &1.kind) |
Parents: 51deb4e