neiam /gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Packages
⭐
Log in to mark this repository.
Let a repo stop authors approving their own pull requests
a7fb497 · Gabriel Morell · 2026-09-10 15:50
Message
{commit_body(@commit)}
Files changed
modified
lib/git_gud/pull_requests.ex
+36
−1
@@ -689,6 +689,24 @@ defmodule GitGud.PullRequests do
| 689 | 689 | def add_review(%PullRequest{} = pr, %User{id: uid} = reviewer, attrs) do |
| 690 | 690 | changeset = PrReview.changeset(%PrReview{pull_request_id: pr.id, reviewer_id: uid}, attrs) |
| 691 | 691 | |
| 692 | + if self_approval_blocked?(pr, uid, changeset) do | |
| 693 | + {:error, :self_approval} | |
| 694 | + else | |
| 695 | + do_add_review_checked(pr, reviewer, changeset) | |
| 696 | + end | |
| 697 | + end | |
| 698 | + | |
| 699 | + # Only approving is withheld — commenting on your own PR, or asking | |
| 700 | + # for changes to it, is ordinary. | |
| 701 | + defp self_approval_blocked?(%PullRequest{} = pr, uid, changeset) do | |
| 702 | + Ecto.Changeset.get_field(changeset, :state) == "approved" and | |
| 703 | + pr.author_id == uid and | |
| 704 | + Repositories.get_repository!(pr.repository_id).block_self_approval | |
| 705 | + end | |
| 706 | + | |
| 707 | + defp do_add_review_checked(pr, reviewer, changeset) do | |
| 708 | + uid = reviewer.id | |
| 709 | + | |
| 692 | 710 | # Re-submitting the verdict you already hold isn't a new review. |
| 693 | 711 | # Without this, pressing Update review repeatedly stacks identical |
| 694 | 712 | # rows into the log for no reason. |
@@ -1012,6 +1030,23 @@ defmodule GitGud.PullRequests do
| 1012 | 1030 | pr |> latest_reviews() |> Enum.count(&(&1.state == "approved")) |
| 1013 | 1031 | end |
| 1014 | 1032 | |
| 1033 | + @doc """ | |
| 1034 | + Approvals that count toward the repo's requirement. | |
| 1035 | + | |
| 1036 | + Where the repo blocks self-approval, an approval the author left | |
| 1037 | + before the rule was turned on stops counting too — otherwise | |
| 1038 | + enabling it would leave already-self-approved PRs satisfying a | |
| 1039 | + requirement they were never meant to. | |
| 1040 | + """ | |
| 1041 | + def effective_approvals(%Repository{block_self_approval: false}, %PullRequest{} = pr), | |
| 1042 | + do: approval_count(pr) | |
| 1043 | + | |
| 1044 | + def effective_approvals(%Repository{}, %PullRequest{} = pr) do | |
| 1045 | + pr | |
| 1046 | + |> latest_reviews() | |
| 1047 | + |> Enum.count(&(&1.state == "approved" and &1.reviewer_id != pr.author_id)) | |
| 1048 | + end | |
| 1049 | + | |
| 1015 | 1050 | @doc """ |
| 1016 | 1051 | Every repo rule this PR currently fails, as a list of reasons: |
| 1017 | 1052 |
@@ -1030,7 +1065,7 @@ defmodule GitGud.PullRequests do
| 1030 | 1065 | end |
| 1031 | 1066 | |
| 1032 | 1067 | approvals = |
| 1033 | − case {repo.required_approvals || 0, approval_count(pr)} do | |
| 1068 | + case {repo.required_approvals || 0, effective_approvals(repo, pr)} do | |
| 1034 | 1069 | {need, have} when need > 0 and have < need -> [{:insufficient_approvals, have, need}] |
| 1035 | 1070 | _ -> [] |
| 1036 | 1071 | end |
modified
lib/git_gud/repositories/repository.ex
+4
−1
@@ -23,6 +23,8 @@ defmodule GitGud.Repositories.Repository do
| 23 | 23 | field :block_merge_on_changes_requested, :boolean, default: false |
| 24 | 24 | # Approving reviews needed before merging. 0 = no requirement. |
| 25 | 25 | field :required_approvals, :integer, default: 0 |
| 26 | + # When true, the PR's own author can't approve it. | |
| 27 | + field :block_self_approval, :boolean, default: false | |
| 26 | 28 | # SHA-256 of the current runner registration token, if any. |
| 27 | 29 | # Regenerating the token replaces the hash, invalidating the old |
| 28 | 30 | # token immediately. |
@@ -70,7 +72,8 @@ defmodule GitGud.Repositories.Repository do
| 70 | 72 | :archived, |
| 71 | 73 | :interaction_policy, |
| 72 | 74 | :block_merge_on_changes_requested, |
| 73 | − :required_approvals | |
| 75 | + :required_approvals, | |
| 76 | + :block_self_approval | |
| 74 | 77 | ]) |
| 75 | 78 | |> validate_inclusion(:visibility, @valid_visibilities) |
| 76 | 79 | |> validate_inclusion(:interaction_policy, @valid_interaction_policies) |
modified
lib/git_gud_web/live/pr_live/reviews.ex
+23
−6
@@ -54,11 +54,12 @@ defmodule GitGudWeb.PrLive.Reviews do
| 54 | 54 | |
| 55 | 55 | defp assign_review_form(socket) do |
| 56 | 56 | mine = my_review(socket.assigns) |
| 57 | + default = if self_approval_blocked?(socket.assigns), do: "commented", else: "approved" | |
| 57 | 58 | |
| 58 | 59 | assign( |
| 59 | 60 | socket, |
| 60 | 61 | :review_form, |
| 61 | − to_form(PrReview.changeset(%PrReview{}, %{state: (mine && mine.state) || "approved"})) | |
| 62 | + to_form(PrReview.changeset(%PrReview{}, %{state: (mine && mine.state) || default})) | |
| 62 | 63 | ) |
| 63 | 64 | end |
| 64 | 65 |
@@ -70,6 +71,20 @@ defmodule GitGudWeb.PrLive.Reviews do
| 70 | 71 | defp can_review?(%{current_scope: %{user: %{}}}), do: true |
| 71 | 72 | defp can_review?(_assigns), do: false |
| 72 | 73 | |
| 74 | + # The repo can withhold approval from the PR's own author. Commenting | |
| 75 | + # and requesting changes on your own PR stay available. | |
| 76 | + defp self_approval_blocked?(%{current_scope: %{user: %{id: uid}}} = assigns), | |
| 77 | + do: assigns.repo.block_self_approval and assigns.pr.author_id == uid | |
| 78 | + | |
| 79 | + defp self_approval_blocked?(_assigns), do: false | |
| 80 | + | |
| 81 | + defp review_options(assigns) do | |
| 82 | + approve = [{"Approve", "approved"}] | |
| 83 | + rest = [{"Request changes", "changes_requested"}, {"Comment", "commented"}] | |
| 84 | + | |
| 85 | + if self_approval_blocked?(assigns), do: rest, else: approve ++ rest | |
| 86 | + end | |
| 87 | + | |
| 73 | 88 | # Asking for a review is the author's or a maintainer's call — the |
| 74 | 89 | # same people who can close it. |
| 75 | 90 | defp can_request_review?(%{current_scope: %{user: %{id: uid}}} = assigns), |
@@ -181,6 +196,9 @@ defmodule GitGudWeb.PrLive.Reviews do
| 181 | 196 | {:noreply, reload(socket)} |
| 182 | 197 | end |
| 183 | 198 | |
| 199 | + {:error, :self_approval} -> | |
| 200 | + {:noreply, put_flash(socket, :error, "You can't approve your own pull request here.")} | |
| 201 | + | |
| 184 | 202 | {:error, cs} -> |
| 185 | 203 | {:noreply, assign(socket, :review_form, to_form(cs))} |
| 186 | 204 | end |
@@ -361,15 +379,14 @@ defmodule GitGudWeb.PrLive.Reviews do
| 361 | 379 | <p :if={my_review(assigns)} class="text-xs opacity-70"> |
| 362 | 380 | You {review_word(my_review(assigns).state)}. Submitting again changes your review. |
| 363 | 381 | </p> |
| 382 | + <p :if={self_approval_blocked?(assigns)} class="text-xs opacity-70"> | |
| 383 | + This repo doesn't let authors approve their own pull requests. | |
| 384 | + </p> | |
| 364 | 385 | <.input |
| 365 | 386 | field={@review_form[:state]} |
| 366 | 387 | type="select" |
| 367 | 388 | label="Review" |
| 368 | − options={[ | |
| 369 | − {"Approve", "approved"}, | |
| 370 | − {"Request changes", "changes_requested"}, | |
| 371 | − {"Comment", "commented"} | |
| 372 | − ]} | |
| 389 | + options={review_options(assigns)} | |
| 373 | 390 | /> |
| 374 | 391 | <.input field={@review_form[:body]} type="textarea" rows="3" label="Body (optional)" /> |
| 375 | 392 | <button type="submit" class="btn btn-xs btn-primary"> |
modified
lib/git_gud_web/live/repo_live/settings.ex
+9
−0
@@ -212,6 +212,15 @@ defmodule GitGudWeb.RepoLive.Settings do
| 212 | 212 | Zero means no requirement. Repo admins can override either rule when |
| 213 | 213 | merging. |
| 214 | 214 | </p> |
| 215 | + <.input | |
| 216 | + field={@form[:block_self_approval]} | |
| 217 | + type="checkbox" | |
| 218 | + label="Stop authors approving their own pull requests" | |
| 219 | + /> | |
| 220 | + <p class="text-xs opacity-60 -mt-2"> | |
| 221 | + They can still comment or request changes on their own PR — only the | |
| 222 | + approval is withheld, and it stops counting toward the requirement above. | |
| 223 | + </p> | |
| 215 | 224 | <div> |
| 216 | 225 | <button class="btn btn-primary btn-sm" type="submit">Save</button> |
| 217 | 226 | </div> |
added
priv/repo/migrations/20260910050000_add_block_self_approval.exs
+14
−0
@@ -0,0 +1,14 @@
| 1 | +defmodule GitGud.Repo.Migrations.AddBlockSelfApproval do | |
| 2 | + use Ecto.Migration | |
| 3 | + | |
| 4 | + # Whether the person who opened a PR may approve it. | |
| 5 | + # | |
| 6 | + # Off by default, like the other two review rules — existing repos | |
| 7 | + # keep behaving as they do rather than having a rule imposed by a | |
| 8 | + # migration. | |
| 9 | + def change do | |
| 10 | + alter table(:repositories) do | |
| 11 | + add :block_self_approval, :boolean, null: false, default: false | |
| 12 | + end | |
| 13 | + end | |
| 14 | +end |
added
test/git_gud_web/live/pr_live/self_approval_test.exs
+156
−0
@@ -0,0 +1,156 @@
| 1 | +defmodule GitGudWeb.PrLive.SelfApprovalTest do | |
| 2 | + @moduledoc """ | |
| 3 | + The per-repo rule stopping a PR's author from approving their own | |
| 4 | + work. | |
| 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 reviews_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}/reviews" | |
| 19 | + end | |
| 20 | + | |
| 21 | + defp block_self!(repo) do | |
| 22 | + {:ok, repo} = Repositories.update_repository(repo, %{"block_self_approval" => true}) | |
| 23 | + repo | |
| 24 | + end | |
| 25 | + | |
| 26 | + defp submit(lv, state, body \\ "") do | |
| 27 | + lv | |
| 28 | + |> form("#review-form", %{"pr_review" => %{"state" => state, "body" => body}}) | |
| 29 | + |> render_submit() | |
| 30 | + end | |
| 31 | + | |
| 32 | + test "off by default, so an author can still approve", %{conn: conn} do | |
| 33 | + {owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 34 | + pr = pr_with_branches(repo, owner) | |
| 35 | + | |
| 36 | + refute repo.block_self_approval | |
| 37 | + | |
| 38 | + {:ok, lv, _html} = live(log_in_user(conn, owner), reviews_path(repo, pr)) | |
| 39 | + _ = submit(lv, "approved") | |
| 40 | + | |
| 41 | + assert PullRequests.approval_count(PullRequests.get_pull_request!(repo, pr.number)) == 1 | |
| 42 | + end | |
| 43 | + | |
| 44 | + test "with the flag on, the author gets no Approve option", %{conn: conn} do | |
| 45 | + {owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 46 | + pr = pr_with_branches(repo, owner) | |
| 47 | + repo = block_self!(repo) | |
| 48 | + | |
| 49 | + {:ok, lv, html} = live(log_in_user(conn, owner), reviews_path(repo, pr)) | |
| 50 | + | |
| 51 | + assert html =~ "doesn't let authors approve their own" | |
| 52 | + refute has_element?(lv, "#review-form option[value=approved]") | |
| 53 | + end | |
| 54 | + | |
| 55 | + test "the author is refused even if they send it anyway", %{conn: conn} do | |
| 56 | + {owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 57 | + pr = pr_with_branches(repo, owner) | |
| 58 | + repo = block_self!(repo) | |
| 59 | + | |
| 60 | + {:ok, lv, _html} = live(log_in_user(conn, owner), reviews_path(repo, pr)) | |
| 61 | + html = render_hook(lv, "add_review", %{"pr_review" => %{"state" => "approved"}}) | |
| 62 | + | |
| 63 | + assert html =~ "can" and html =~ "approve your own" | |
| 64 | + assert PullRequests.get_pull_request!(repo, pr.number).reviews == [] | |
| 65 | + end | |
| 66 | + | |
| 67 | + test "the context refuses regardless of caller", %{conn: _conn} do | |
| 68 | + {owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 69 | + pr = pr_with_branches(repo, owner) | |
| 70 | + _repo = block_self!(repo) | |
| 71 | + | |
| 72 | + assert {:error, :self_approval} = | |
| 73 | + PullRequests.add_review(pr, owner, %{"state" => "approved"}) | |
| 74 | + end | |
| 75 | + | |
| 76 | + test "the author can still comment and request changes on their own PR", %{conn: conn} do | |
| 77 | + {owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 78 | + pr = pr_with_branches(repo, owner) | |
| 79 | + repo = block_self!(repo) | |
| 80 | + | |
| 81 | + {:ok, lv, _html} = live(log_in_user(conn, owner), reviews_path(repo, pr)) | |
| 82 | + _ = submit(lv, "changes_requested", "note to self") | |
| 83 | + | |
| 84 | + assert [review] = PullRequests.get_pull_request!(repo, pr.number).reviews | |
| 85 | + assert review.state == "changes_requested" | |
| 86 | + end | |
| 87 | + | |
| 88 | + test "someone else can still approve", %{conn: conn} do | |
| 89 | + {owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 90 | + pr = pr_with_branches(repo, owner) | |
| 91 | + repo = block_self!(repo) | |
| 92 | + | |
| 93 | + reviewer = user_fixture() | |
| 94 | + {:ok, lv, _html} = live(log_in_user(conn, reviewer), reviews_path(repo, pr)) | |
| 95 | + _ = submit(lv, "approved") | |
| 96 | + | |
| 97 | + reloaded = PullRequests.get_pull_request!(repo, pr.number) | |
| 98 | + assert PullRequests.effective_approvals(repo, reloaded) == 1 | |
| 99 | + end | |
| 100 | + | |
| 101 | + test "an earlier self-approval stops counting once the rule is on", %{conn: conn} do | |
| 102 | + {owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 103 | + pr = pr_with_branches(repo, owner) | |
| 104 | + | |
| 105 | + # Approve while it's still allowed. | |
| 106 | + {:ok, lv, _html} = live(log_in_user(conn, owner), reviews_path(repo, pr)) | |
| 107 | + _ = submit(lv, "approved") | |
| 108 | + | |
| 109 | + reloaded = PullRequests.get_pull_request!(repo, pr.number) | |
| 110 | + assert PullRequests.effective_approvals(repo, reloaded) == 1 | |
| 111 | + | |
| 112 | + # Turning the rule on retroactively discounts it, so enabling the | |
| 113 | + # rule doesn't leave already-self-approved PRs satisfying it. | |
| 114 | + repo = block_self!(repo) | |
| 115 | + assert PullRequests.effective_approvals(repo, reloaded) == 0 | |
| 116 | + end | |
| 117 | + | |
| 118 | + test "a self-approval doesn't satisfy a required-approvals rule", %{conn: conn} do | |
| 119 | + {owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 120 | + pr = pr_with_branches(repo, owner) | |
| 121 | + | |
| 122 | + {:ok, lv, _html} = live(log_in_user(conn, owner), reviews_path(repo, pr)) | |
| 123 | + _ = submit(lv, "approved") | |
| 124 | + | |
| 125 | + {:ok, repo} = | |
| 126 | + Repositories.update_repository(repo, %{ | |
| 127 | + "block_self_approval" => true, | |
| 128 | + "required_approvals" => 1 | |
| 129 | + }) | |
| 130 | + | |
| 131 | + reloaded = PullRequests.get_pull_request!(repo, pr.number) | |
| 132 | + assert [{:insufficient_approvals, 0, 1}] = PullRequests.merge_blockers(repo, reloaded) | |
| 133 | + end | |
| 134 | + | |
| 135 | + test "an admin can set the flag from repo settings", %{conn: conn} do | |
| 136 | + {owner, repo} = repository_fixture() | |
| 137 | + handle = Repositories.Storage.repo_handle(GitGud.Repo.preload(repo, [:owner, :organization])) | |
| 138 | + | |
| 139 | + {:ok, lv, html} = live(log_in_user(conn, owner), ~p"/r/#{handle}/#{repo.name}/settings") | |
| 140 | + assert html =~ "Stop authors approving their own pull requests" | |
| 141 | + | |
| 142 | + _ = | |
| 143 | + lv | |
| 144 | + |> form("form[phx-submit=save]", %{ | |
| 145 | + "repository" => %{ | |
| 146 | + "description" => repo.description || "", | |
| 147 | + "visibility" => repo.visibility, | |
| 148 | + "default_branch" => repo.default_branch, | |
| 149 | + "block_self_approval" => "true" | |
| 150 | + } | |
| 151 | + }) | |
| 152 | + |> render_submit() | |
| 153 | + | |
| 154 | + assert Repositories.get_repository!(repo.id).block_self_approval | |
| 155 | + end | |
| 156 | +end |
Parents: bd60fcb