GITGUD

neiam /gitgud

Git Gud

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

⭐

Log in to mark this repository.

Let a repo block merging while changes are requested

1a92edd · Gabriel Morell · 2026-09-10 14:29

6 files +248 -3
Message 
{commit_body(@commit)}

Files changed

modified lib/git_gud/pull_requests.ex
+34 −1
@@ -592,7 +592,8 @@ defmodule GitGud.PullRequests do
592 592 repo = Repositories.get_repository!(pr.repository_id)
593 593 path = repo.disk_path
594 594
595 − with {:ok, current_target_sha} <- Repositories.resolve(repo, pr.target_ref),
595 + with :ok <- check_review_block(repo, pr),
596 + {:ok, current_target_sha} <- Repositories.resolve(repo, pr.target_ref),
596 597 :ok <-
597 598 BranchProtections.check_ref_update(
598 599 repo,
@@ -753,6 +754,38 @@ defmodule GitGud.PullRequests do
753 754 |> Repo.all()
754 755 end
755 756
757 + @doc """
758 + Reviewers whose standing verdict is "changes requested".
759 +
760 + Empty when nobody is blocking. Callers use this both to enforce and
761 + to explain — the PR page names who is waiting on a change.
762 + """
763 + def blocking_reviews(%PullRequest{} = pr) do
764 + pr
765 + |> latest_reviews()
766 + |> Enum.filter(&(&1.state == "changes_requested"))
767 + end
768 +
769 + @doc """
770 + Whether `pr` is merge-blocked by review. Only when the repo opted in
771 + via `block_merge_on_changes_requested`.
772 + """
773 + def review_blocked?(%Repository{block_merge_on_changes_requested: false}, _pr), do: false
774 +
775 + def review_blocked?(%Repository{}, %PullRequest{} = pr), do: blocking_reviews(pr) != []
776 +
777 + # `merge!/3` may be handed a PR without its reviews loaded, so this
778 + # reloads rather than trusting the struct it was given.
779 + defp check_review_block(%Repository{block_merge_on_changes_requested: false}, _pr), do: :ok
780 +
781 + defp check_review_block(%Repository{} = repo, %PullRequest{} = pr) do
782 + loaded = get_pull_request!(repo, pr.number)
783 +
784 + if blocking_reviews(loaded) == [],
785 + do: :ok,
786 + else: {:error, :changes_requested}
787 + end
788 +
756 789 @doc """
757 790 The standing verdict per reviewer: their most recent review, newest
758 791 first.
modified lib/git_gud/repositories/repository.ex
+4 −1
@@ -19,6 +19,8 @@ defmodule GitGud.Repositories.Repository do
19 19 field :pushed_at, :utc_datetime
20 20 field :size_kb, :integer, default: 0
21 21 field :archived, :boolean, default: false
22 + # When true, a standing "changes requested" review blocks merging.
23 + field :block_merge_on_changes_requested, :boolean, default: false
22 24 # SHA-256 of the current runner registration token, if any.
23 25 # Regenerating the token replaces the hash, invalidating the old
24 26 # token immediately.
@@ -64,7 +66,8 @@ defmodule GitGud.Repositories.Repository do
64 66 :default_branch,
65 67 :visibility,
66 68 :archived,
67 − :interaction_policy
69 + :interaction_policy,
70 + :block_merge_on_changes_requested
68 71 ])
69 72 |> validate_inclusion(:visibility, @valid_visibilities)
70 73 |> validate_inclusion(:interaction_policy, @valid_interaction_policies)
modified lib/git_gud_web/live/pr_live/show.ex
+23 −1
@@ -153,6 +153,11 @@ defmodule GitGudWeb.PrLive.Show do
153 153 # same people who can close it.
154 154 defp can_request_review?(assigns), do: can_change_state?(assigns)
155 155
156 + defp blocking_reviewers(pr),
157 + do: pr |> PullRequests.blocking_reviews() |> Enum.map(&reviewer_name(&1.reviewer))
158 +
159 + defp review_blocked?(repo, pr), do: PullRequests.review_blocked?(repo, pr)
160 +
156 161 defp requested_reviewers(pr), do: pr.review_requests
157 162
158 163 # Someone asked to review who hasn't left a verdict yet.
@@ -536,6 +541,10 @@ defmodule GitGudWeb.PrLive.Show do
536 541 |> assign(:event_count, Events.count_for(socket.assigns.pr))
537 542 |> reload()}
538 543
544 + {:error, :changes_requested} ->
545 + {:noreply,
546 + put_flash(socket, :error, "A reviewer has requested changes on this pull request.")}
547 +
539 548 {:error, :conflict} ->
540 549 {:noreply, put_flash(socket, :error, "Merge conflict — cannot merge.")}
541 550
@@ -755,12 +764,25 @@ defmodule GitGudWeb.PrLive.Show do
755 764 <.icon name="hero-clock" class="size-4 opacity-60" /> Mergeability not yet checked.
756 765 <% end %>
757 766 </p>
767 + <p :if={blocking_reviewers(@pr) != []} class="text-sm flex items-start gap-1.5">
768 + <.icon name="hero-hand-raised" class="size-4 text-error shrink-0 mt-0.5" />
769 + <span>
770 + {Enum.join(blocking_reviewers(@pr), ", ")} requested changes.{if review_blocked?(
771 + @repo,
772 + @pr
773 + ),
774 + do:
775 + " Merging is blocked until that verdict changes.",
776 + else:
777 + " You can still merge."}
778 + </span>
779 + </p>
758 780 <div class="flex gap-2">
759 781 <button
760 782 type="button"
761 783 phx-click="merge"
762 784 class="btn btn-sm btn-primary"
763 − disabled={@pr.mergeable != true}
785 + disabled={@pr.mergeable != true or review_blocked?(@repo, @pr)}
764 786 >
765 787 Merge
766 788 </button>
modified lib/git_gud_web/live/repo_live/settings.ex
+9 −0
@@ -192,6 +192,15 @@ defmodule GitGudWeb.RepoLive.Settings do
192 192 ]}
193 193 />
194 194 <.input field={@form[:default_branch]} type="text" label="Default branch" />
195 + <.input
196 + field={@form[:block_merge_on_changes_requested]}
197 + type="checkbox"
198 + label="Block merging while changes are requested"
199 + />
200 + <p class="text-xs opacity-60 -mt-2">
201 + A reviewer's standing "changes requested" stops the merge until they
202 + change their verdict. Off by default.
203 + </p>
195 204 <div>
196 205 <button class="btn btn-primary btn-sm" type="submit">Save</button>
197 206 </div>
added priv/repo/migrations/20260910020000_add_block_merge_on_changes_requested.exs
+14 −0
@@ -0,0 +1,14 @@
1 +defmodule GitGud.Repo.Migrations.AddBlockMergeOnChangesRequested do
2 + use Ecto.Migration
3 +
4 + # Whether a standing "changes requested" verdict blocks merging.
5 + #
6 + # Defaults to false so existing repos keep merging exactly as they do
7 + # today — turning review enforcement on is a decision each repo makes,
8 + # not one this migration makes for them.
9 + def change do
10 + alter table(:repositories) do
11 + add :block_merge_on_changes_requested, :boolean, null: false, default: false
12 + end
13 + end
14 +end
added test/git_gud_web/live/pr_live/review_block_test.exs
+164 −0
@@ -0,0 +1,164 @@
1 +defmodule GitGudWeb.PrLive.ReviewBlockTest do
2 + @moduledoc """
3 + The per-repo setting that lets a standing "changes requested" verdict
4 + block merging.
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.PullRequests
14 + alias GitGud.Repositories
15 +
16 + defp pr_path(repo, pr) do
17 + handle = Repositories.Storage.repo_handle(GitGud.Repo.preload(repo, [:owner, :organization]))
18 + ~p"/r/#{handle}/#{repo.name}/pulls/#{pr.number}"
19 + end
20 +
21 + defp block!(repo) do
22 + {:ok, repo} =
23 + Repositories.update_repository(repo, %{"block_merge_on_changes_requested" => true})
24 +
25 + repo
26 + end
27 +
28 + defp request_changes!(conn, repo, pr, reviewer) do
29 + {:ok, lv, _html} = live(log_in_user(conn, reviewer), pr_path(repo, pr))
30 +
31 + lv
32 + |> form("#review-form", %{"pr_review" => %{"state" => "changes_requested", "body" => "No"}})
33 + |> render_submit()
34 + end
35 +
36 + test "off by default, so existing repos merge as before", %{conn: conn} do
37 + {owner, repo} = repository_fixture(%{visibility: "public"})
38 + pr = pr_with_branches(repo, owner)
39 + reviewer = user_fixture()
40 +
41 + _ = request_changes!(conn, repo, pr, reviewer)
42 +
43 + refute repo.block_merge_on_changes_requested
44 + refute PullRequests.review_blocked?(repo, PullRequests.get_pull_request!(repo, pr.number))
45 + end
46 +
47 + test "the warning names who is blocking even when not enforcing", %{conn: conn} do
48 + {owner, repo} = repository_fixture(%{visibility: "public"})
49 + pr = pr_with_branches(repo, owner)
50 + reviewer = user_fixture()
51 +
52 + _ = request_changes!(conn, repo, pr, reviewer)
53 +
54 + {:ok, _lv, html} = live(log_in_user(conn, owner), pr_path(repo, pr))
55 +
56 + assert html =~ "requested changes."
57 + assert html =~ "You can still merge."
58 + end
59 +
60 + test "with the setting on, the merge button is disabled and says why", %{conn: conn} do
61 + {owner, repo} = repository_fixture(%{visibility: "public"})
62 + pr = pr_with_branches(repo, owner)
63 + reviewer = user_fixture()
64 +
65 + _ = request_changes!(conn, repo, pr, reviewer)
66 + repo = block!(repo)
67 +
68 + {:ok, lv, html} = live(log_in_user(conn, owner), pr_path(repo, pr))
69 +
70 + assert html =~ "Merging is blocked until that verdict changes."
71 + assert has_element?(lv, "button[phx-click=merge][disabled]")
72 + end
73 +
74 + test "the context refuses the merge, not just the button", %{conn: conn} do
75 + {owner, repo} = repository_fixture(%{visibility: "public"})
76 + pr = pr_with_branches(repo, owner)
77 + reviewer = user_fixture()
78 +
79 + _ = request_changes!(conn, repo, pr, reviewer)
80 + repo = block!(repo)
81 +
82 + {:ok, lv, _html} = live(log_in_user(conn, owner), pr_path(repo, pr))
83 + html = render_hook(lv, "merge", %{})
84 +
85 + assert html =~ "A reviewer has requested changes"
86 + assert PullRequests.get_pull_request!(repo, pr.number).state == "open"
87 + end
88 +
89 + test "merge!/2 refuses directly", %{conn: conn} do
90 + {owner, repo} = repository_fixture(%{visibility: "public"})
91 + pr = pr_with_branches(repo, owner)
92 + reviewer = user_fixture()
93 +
94 + _ = request_changes!(conn, repo, pr, reviewer)
95 + repo = block!(repo)
96 +
97 + reloaded = PullRequests.get_pull_request!(repo, pr.number)
98 + assert {:error, :changes_requested} = PullRequests.merge!(reloaded, owner)
99 + end
100 +
101 + test "the block lifts when the reviewer changes their verdict", %{conn: conn} do
102 + {owner, repo} = repository_fixture(%{visibility: "public"})
103 + pr = pr_with_branches(repo, owner)
104 + reviewer = user_fixture()
105 +
106 + _ = request_changes!(conn, repo, pr, reviewer)
107 + repo = block!(repo)
108 +
109 + {:ok, rlv, _html} = live(log_in_user(conn, reviewer), pr_path(repo, pr))
110 +
111 + _ =
112 + rlv
113 + |> form("#review-form", %{"pr_review" => %{"state" => "approved", "body" => "Better"}})
114 + |> render_submit()
115 +
116 + {:ok, lv, html} = live(log_in_user(conn, owner), pr_path(repo, pr))
117 +
118 + refute html =~ "Merging is blocked"
119 + refute has_element?(lv, "button[phx-click=merge][disabled]")
120 + end
121 +
122 + test "another reviewer approving doesn't clear someone else's block", %{conn: conn} do
123 + {owner, repo} = repository_fixture(%{visibility: "public"})
124 + pr = pr_with_branches(repo, owner)
125 + blocker = user_fixture()
126 + approver = user_fixture()
127 +
128 + _ = request_changes!(conn, repo, pr, blocker)
129 + repo = block!(repo)
130 +
131 + {:ok, alv, _html} = live(log_in_user(conn, approver), pr_path(repo, pr))
132 +
133 + _ =
134 + alv
135 + |> form("#review-form", %{"pr_review" => %{"state" => "approved", "body" => ""}})
136 + |> render_submit()
137 +
138 + reloaded = PullRequests.get_pull_request!(repo, pr.number)
139 + assert PullRequests.review_blocked?(repo, reloaded)
140 + end
141 +
142 + test "an admin can turn the setting on from repo settings", %{conn: conn} do
143 + {owner, repo} = repository_fixture()
144 + handle = Repositories.Storage.repo_handle(GitGud.Repo.preload(repo, [:owner, :organization]))
145 +
146 + {:ok, lv, html} = live(log_in_user(conn, owner), ~p"/r/#{handle}/#{repo.name}/settings")
147 +
148 + assert html =~ "Block merging while changes are requested"
149 +
150 + _ =
151 + lv
152 + |> form("form[phx-submit=save]", %{
153 + "repository" => %{
154 + "description" => repo.description || "",
155 + "visibility" => repo.visibility,
156 + "default_branch" => repo.default_branch,
157 + "block_merge_on_changes_requested" => "true"
158 + }
159 + })
160 + |> render_submit()
161 +
162 + assert Repositories.get_repository!(repo.id).block_merge_on_changes_requested
163 + end
164 +end

Parents: e17cdd6