neiam /gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Packages
⭐
Log in to mark this repository.
Unify the PR picker as instance/owner-repo/branch on both sides
6e19be9 · Gabriel Morell · 2026-09-09 20:56
Message
{commit_body(@commit)}
Files changed
modified
lib/git_gud_web/live/pr_live/new.ex
+174
−59
@@ -1,7 +1,9 @@
| 1 | 1 | defmodule GitGudWeb.PrLive.New do |
| 2 | 2 | use GitGudWeb, :live_view |
| 3 | 3 | |
| 4 | + alias GitGud.Federation | |
| 4 | 5 | alias GitGud.Federation.Outbound |
| 6 | + alias GitGud.Federation.RemoteActors | |
| 5 | 7 | alias GitGud.Git |
| 6 | 8 | alias GitGud.PullRequests |
| 7 | 9 | alias GitGud.PullRequests.PullRequest |
@@ -40,8 +42,12 @@ defmodule GitGudWeb.PrLive.New do
| 40 | 42 | |> assign(:candidates, candidates) |
| 41 | 43 | |> assign_sides(source, target) |
| 42 | 44 | |> assign_form(PullRequest.create_changeset(%PullRequest{}, attrs)) |
| 43 | − |> assign(:remote_target_url, "") | |
| 44 | − |> assign(:remote_mode?, false) | |
| 45 | + |> assign(:local_instance, Federation.base_url()) | |
| 46 | + |> assign(:base_instance, Federation.base_url()) | |
| 47 | + |> assign(:remote_owner_repo, "") | |
| 48 | + |> assign(:remote_ref, "") | |
| 49 | + |> assign(:conn_state, :untested) | |
| 50 | + |> assign(:conn_message, nil) | |
| 45 | 51 | |> assign(:page_title, "New pull request")} |
| 46 | 52 | end |
| 47 | 53 |
@@ -97,24 +103,93 @@ defmodule GitGudWeb.PrLive.New do
| 97 | 103 | {:noreply, socket |> assign_sides(source, target) |> assign_form(cs)} |
| 98 | 104 | end |
| 99 | 105 | |
| 100 | − def handle_event("toggle_remote", _params, socket) do | |
| 101 | − {:noreply, assign(socket, :remote_mode?, not socket.assigns.remote_mode?)} | |
| 106 | + # The base instance URL is what decides local vs federated — there is | |
| 107 | + # no separate mode to toggle. | |
| 108 | + def handle_event("base_instance", %{"value" => v}, socket) do | |
| 109 | + {:noreply, | |
| 110 | + socket | |
| 111 | + |> assign(:base_instance, v) | |
| 112 | + |> assign(:conn_state, :untested) | |
| 113 | + |> assign(:conn_message, nil)} | |
| 114 | + end | |
| 115 | + | |
| 116 | + def handle_event("remote_field", %{"field" => f, "value" => v}, socket) | |
| 117 | + when f in ~w(remote_owner_repo remote_ref) do | |
| 118 | + socket = assign(socket, String.to_existing_atom(f), v) | |
| 119 | + | |
| 120 | + # Changing which remote repo we mean invalidates the probe. | |
| 121 | + socket = | |
| 122 | + if f == "remote_owner_repo", | |
| 123 | + do: socket |> assign(:conn_state, :untested) |> assign(:conn_message, nil), | |
| 124 | + else: socket | |
| 125 | + | |
| 126 | + {:noreply, socket} | |
| 102 | 127 | end |
| 103 | 128 | |
| 104 | − def handle_event("remote_target_url", %{"value" => v}, socket) do | |
| 105 | − {:noreply, assign(socket, :remote_target_url, v)} | |
| 129 | + def handle_event("test_connection", _params, socket) do | |
| 130 | + case remote_actor_url(socket) do | |
| 131 | + nil -> | |
| 132 | + {:noreply, | |
| 133 | + socket | |
| 134 | + |> assign(:conn_state, :failed) | |
| 135 | + |> assign(:conn_message, "Enter the remote instance URL and owner/repo first.")} | |
| 136 | + | |
| 137 | + url -> | |
| 138 | + {:noreply, probe(socket, url)} | |
| 139 | + end | |
| 106 | 140 | end |
| 107 | 141 | |
| 108 | 142 | def handle_event("save", %{"pull_request" => attrs}, socket) do |
| 109 | 143 | user = socket.assigns.current_scope.user |
| 110 | 144 | |
| 111 | − if socket.assigns.remote_mode? do | |
| 112 | − offer_to_remote(socket, user, attrs) | |
| 113 | − else | |
| 114 | − open_local_pr(socket, user, attrs) | |
| 145 | + cond do | |
| 146 | + not remote?(socket.assigns) -> open_local_pr(socket, user, attrs) | |
| 147 | + socket.assigns.conn_state == :ok -> offer_to_remote(socket, user, attrs) | |
| 148 | + # Belt and braces: the submit button is disabled in this state. | |
| 149 | + true -> {:noreply, put_flash(socket, :error, "Test the connection first.")} | |
| 115 | 150 | end |
| 116 | 151 | end |
| 117 | 152 | |
| 153 | + # Resolving the actor is the whole test: it proves the host answers | |
| 154 | + # ActivityPub, that the object exists, and that it can host a PR. | |
| 155 | + defp probe(socket, url) do | |
| 156 | + case RemoteActors.fetch(url) do | |
| 157 | + {:ok, %{actor_type: "Repository"} = actor} -> | |
| 158 | + socket | |
| 159 | + |> assign(:conn_state, :ok) | |
| 160 | + |> assign(:conn_message, "Reached #{actor.actor_url}. Ready to send.") | |
| 161 | + | |
| 162 | + {:ok, %{actor_type: other}} -> | |
| 163 | + socket | |
| 164 | + |> assign(:conn_state, :failed) | |
| 165 | + |> assign(:conn_message, "That URL is a #{other}, not a repository.") | |
| 166 | + | |
| 167 | + {:error, reason} -> | |
| 168 | + socket | |
| 169 | + |> assign(:conn_state, :failed) | |
| 170 | + |> assign(:conn_message, "Couldn't reach it: #{inspect(reason)}") | |
| 171 | + end | |
| 172 | + end | |
| 173 | + | |
| 174 | + # Remote repo actor URL, composed the same way this instance builds | |
| 175 | + # its own (`<instance>/r/<owner>/<repo>`). | |
| 176 | + defp remote_actor_url(%Phoenix.LiveView.Socket{assigns: a}), do: remote_actor_url(a) | |
| 177 | + | |
| 178 | + defp remote_actor_url(assigns) do | |
| 179 | + instance = String.trim_trailing(String.trim(assigns.base_instance), "/") | |
| 180 | + owner_repo = String.trim(assigns.remote_owner_repo) | |
| 181 | + | |
| 182 | + if instance == "" or owner_repo == "", | |
| 183 | + do: nil, | |
| 184 | + else: instance <> "/r/" <> String.trim_leading(owner_repo, "/") | |
| 185 | + end | |
| 186 | + | |
| 187 | + defp remote?(assigns), | |
| 188 | + do: normalize_url(assigns.base_instance) != normalize_url(assigns.local_instance) | |
| 189 | + | |
| 190 | + defp normalize_url(nil), do: "" | |
| 191 | + defp normalize_url(u), do: u |> String.trim() |> String.trim_trailing("/") |> String.downcase() | |
| 192 | + | |
| 118 | 193 | defp open_local_pr(socket, user, attrs) do |
| 119 | 194 | target = socket.assigns.target_repo |
| 120 | 195 |
@@ -139,14 +214,14 @@ defmodule GitGudWeb.PrLive.New do
| 139 | 214 | |
| 140 | 215 | defp offer_to_remote(socket, user, attrs) do |
| 141 | 216 | source_repo = socket.assigns.source_repo |
| 142 | − target_url = String.trim(socket.assigns.remote_target_url) | |
| 217 | + target_url = remote_actor_url(socket) || "" | |
| 143 | 218 | |
| 144 | 219 | with {:nonblank, true} <- {:nonblank, target_url != ""}, |
| 145 | 220 | {:ok, head_sha} <- Repositories.resolve(source_repo, attrs["source_ref"]) do |
| 146 | 221 | params = %{ |
| 147 | 222 | target_repo_url: target_url, |
| 148 | 223 | source_branch: attrs["source_ref"], |
| 149 | − target_branch: attrs["target_ref"], | |
| 224 | + target_branch: String.trim(socket.assigns.remote_ref), | |
| 150 | 225 | head_hex: Git.to_hex(head_sha), |
| 151 | 226 | title: attrs["title"], |
| 152 | 227 | body: attrs["body"] |
@@ -168,7 +243,7 @@ defmodule GitGudWeb.PrLive.New do
| 168 | 243 | end |
| 169 | 244 | else |
| 170 | 245 | {:nonblank, _} -> |
| 171 | − {:noreply, put_flash(socket, :error, "Enter the remote repository's AP actor URL.")} | |
| 246 | + {:noreply, put_flash(socket, :error, "Enter the remote instance URL and owner/repo.")} | |
| 172 | 247 | |
| 173 | 248 | {:error, :no_such_ref} -> |
| 174 | 249 | {:noreply, put_flash(socket, :error, "Source branch doesn't exist.")} |
@@ -192,54 +267,54 @@ defmodule GitGudWeb.PrLive.New do
| 192 | 267 | latest_run={@latest_run} |
| 193 | 268 | /> |
| 194 | 269 | <h1 class="text-xl font-semibold"> |
| 195 | − New pull request | |
| 196 | − <%= if @remote_mode? do %> | |
| 197 | − <span class="opacity-50">to</span> | |
| 198 | − <span class="badge badge-accent">federated</span> | |
| 199 | − <% end %> | |
| 270 | + New pull request <span :if={remote?(assigns)} class="badge badge-accent">federated</span> | |
| 200 | 271 | </h1> |
| 201 | 272 | |
| 202 | − <p class="text-xs opacity-70"> | |
| 203 | − <button type="button" phx-click="toggle_remote" class="link link-hover"> | |
| 204 | − <%= if @remote_mode? do %> | |
| 205 | − ← Back to opening locally | |
| 206 | − <% else %> | |
| 207 | − Send to a remote repository (ForgeFed Offer) → | |
| 208 | − <% end %> | |
| 209 | − </button> | |
| 210 | − </p> | |
| 211 | − | |
| 212 | − <div :if={@remote_mode?} class="space-y-2 border border-base-300 rounded p-3"> | |
| 213 | − <label class="text-sm font-semibold">Remote repository AP URL</label> | |
| 214 | − <input | |
| 215 | − type="text" | |
| 216 | − value={@remote_target_url} | |
| 217 | − phx-keyup="remote_target_url" | |
| 218 | − phx-debounce="200" | |
| 219 | − placeholder="https://other.host/r/them/proj" | |
| 220 | − class="input input-sm input-bordered w-full font-mono" | |
| 221 | − autocomplete="off" | |
| 222 | − /> | |
| 223 | − <p class="text-xs opacity-60"> | |
| 224 | − The remote will fetch your branch directly. No local PR row is | |
| 225 | − created — status lives on the receiving instance. | |
| 226 | − </p> | |
| 227 | − </div> | |
| 228 | − | |
| 229 | 273 | <.form for={@form} id="new-pr-form" phx-change="validate" phx-submit="save" class="space-y-3"> |
| 230 | − <%!-- base <- head, both sides pickable from the lineage pool. --%> | |
| 231 | − <div class="grid gap-3 sm:grid-cols-[1fr_auto_1fr] sm:items-end border border-base-300 rounded p-3"> | |
| 274 | + <%!-- [instance][owner/repo][branch] <- [instance][owner/repo][branch]. | |
| 275 | + Same instance on both sides is a local PR; a different | |
| 276 | + base instance is a federated Offer. --%> | |
| 277 | + <div class="grid gap-3 sm:grid-cols-[1fr_auto_1fr] sm:items-start border border-base-300 rounded p-3"> | |
| 232 | 278 | <div class="space-y-1"> |
| 233 | 279 | <p class="text-xs font-semibold uppercase tracking-wide opacity-60"> |
| 234 | 280 | Base — where it merges to |
| 235 | 281 | </p> |
| 236 | − <%= if @remote_mode? do %> | |
| 237 | − <p class="text-sm font-mono opacity-70">the remote repository</p> | |
| 238 | − <.input | |
| 239 | − field={@form[:target_ref]} | |
| 240 | − label="Branch (on the remote)" | |
| 282 | + | |
| 283 | + <input | |
| 284 | + type="text" | |
| 285 | + value={@base_instance} | |
| 286 | + phx-keyup="base_instance" | |
| 287 | + phx-debounce="300" | |
| 288 | + aria-label="Base instance URL" | |
| 289 | + class="input input-sm input-ghost w-full font-mono text-xs opacity-60 focus:opacity-100" | |
| 290 | + autocomplete="off" | |
| 291 | + spellcheck="false" | |
| 292 | + /> | |
| 293 | + | |
| 294 | + <%= if remote?(assigns) do %> | |
| 295 | + <input | |
| 241 | 296 | type="text" |
| 297 | + value={@remote_owner_repo} | |
| 298 | + phx-keyup="remote_field" | |
| 299 | + phx-value-field="remote_owner_repo" | |
| 300 | + phx-debounce="300" | |
| 301 | + placeholder="owner/repo" | |
| 302 | + aria-label="Base owner and repository" | |
| 303 | + class="input input-sm input-bordered w-full font-mono" | |
| 304 | + autocomplete="off" | |
| 305 | + spellcheck="false" | |
| 306 | + /> | |
| 307 | + <input | |
| 308 | + type="text" | |
| 309 | + value={@remote_ref} | |
| 310 | + phx-keyup="remote_field" | |
| 311 | + phx-value-field="remote_ref" | |
| 312 | + phx-debounce="300" | |
| 242 | 313 | placeholder="main" |
| 314 | + aria-label="Base branch" | |
| 315 | + class="input input-sm input-bordered w-full font-mono" | |
| 316 | + autocomplete="off" | |
| 317 | + spellcheck="false" | |
| 243 | 318 | /> |
| 244 | 319 | <% else %> |
| 245 | 320 | <.input |
@@ -255,12 +330,22 @@ defmodule GitGudWeb.PrLive.New do
| 255 | 330 | <% end %> |
| 256 | 331 | </div> |
| 257 | 332 | |
| 258 | − <div class="text-center text-sm opacity-50 pb-2 sm:pb-3">←</div> | |
| 333 | + <div class="text-center text-sm opacity-50 pt-6">←</div> | |
| 259 | 334 | |
| 260 | 335 | <div class="space-y-1"> |
| 261 | 336 | <p class="text-xs font-semibold uppercase tracking-wide opacity-60"> |
| 262 | 337 | Head — what you want merged |
| 263 | 338 | </p> |
| 339 | + | |
| 340 | + <%!-- We can only offer branches we hold, so the head side | |
| 341 | + is always this instance. --%> | |
| 342 | + <input | |
| 343 | + type="text" | |
| 344 | + value={@local_instance} | |
| 345 | + readonly | |
| 346 | + aria-label="Head instance URL" | |
| 347 | + class="input input-sm input-ghost w-full font-mono text-xs opacity-60" | |
| 348 | + /> | |
| 264 | 349 | <.input |
| 265 | 350 | field={@form[:source_repository_id]} |
| 266 | 351 | type="select" |
@@ -275,15 +360,41 @@ defmodule GitGudWeb.PrLive.New do
| 275 | 360 | </div> |
| 276 | 361 | </div> |
| 277 | 362 | |
| 278 | − <p :if={!@remote_mode? and @source_repo.id != @target_repo.id} class="text-xs opacity-60"> | |
| 363 | + <div :if={remote?(assigns)} class="space-y-2 border border-base-300 rounded p-3"> | |
| 364 | + <div class="flex items-center gap-3"> | |
| 365 | + <button type="button" phx-click="test_connection" class="btn btn-sm"> | |
| 366 | + Test connection | |
| 367 | + </button> | |
| 368 | + <span class={[ | |
| 369 | + "text-xs", | |
| 370 | + @conn_state == :ok && "text-success", | |
| 371 | + @conn_state == :failed && "text-error", | |
| 372 | + @conn_state == :untested && "opacity-60" | |
| 373 | + ]}> | |
| 374 | + {@conn_message || | |
| 375 | + "Untested — the rest of the form stays locked until this succeeds."} | |
| 376 | + </span> | |
| 377 | + </div> | |
| 378 | + <p class="text-xs opacity-60 font-mono"> | |
| 379 | + {remote_actor_url(assigns) || "…/r/owner/repo"} | |
| 380 | + </p> | |
| 381 | + </div> | |
| 382 | + | |
| 383 | + <p | |
| 384 | + :if={!remote?(assigns) and @source_repo.id != @target_repo.id} | |
| 385 | + class="text-xs opacity-60" | |
| 386 | + > | |
| 279 | 387 | Cross-repository: {repo_label(@source_repo)} → {repo_label(@target_repo)}. The |
| 280 | 388 | PR opens on {repo_label(@target_repo)}. |
| 281 | 389 | </p> |
| 282 | − <.input field={@form[:title]} label="Title" required /> | |
| 283 | − <.input field={@form[:body]} label="Description" type="textarea" rows="6" /> | |
| 284 | − <button type="submit" class="btn btn-primary"> | |
| 285 | − {if @remote_mode?, do: "Send offer", else: "Open PR"} | |
| 286 | − </button> | |
| 390 | + | |
| 391 | + <fieldset disabled={locked?(assigns)} class="space-y-3"> | |
| 392 | + <.input field={@form[:title]} label="Title" required /> | |
| 393 | + <.input field={@form[:body]} label="Description" type="textarea" rows="6" /> | |
| 394 | + <button type="submit" class="btn btn-primary"> | |
| 395 | + {if remote?(assigns), do: "Send offer", else: "Open PR"} | |
| 396 | + </button> | |
| 397 | + </fieldset> | |
| 287 | 398 | </.form> |
| 288 | 399 | </div> |
| 289 | 400 | </Layouts.app> |
@@ -297,6 +408,10 @@ defmodule GitGudWeb.PrLive.New do
| 297 | 408 | end) |
| 298 | 409 | end |
| 299 | 410 | |
| 411 | + # A federated target is unproven until the probe succeeds; everything | |
| 412 | + # past the path picker stays inert until then. | |
| 413 | + defp locked?(assigns), do: remote?(assigns) and assigns.conn_state != :ok | |
| 414 | + | |
| 300 | 415 | defp repo_label(repo) do |
| 301 | 416 | Storage.repo_handle(repo) <> "/" <> repo.name |
| 302 | 417 | end |
modified
test/git_gud_web/live/pr_live/new_cross_repo_test.exs
+87
−0
@@ -111,6 +111,93 @@ defmodule GitGudWeb.PrLive.NewCrossRepoTest do
| 111 | 111 | assert {"must differ from target_ref", _} = cs.errors[:source_ref] |
| 112 | 112 | end |
| 113 | 113 | |
| 114 | + describe "instance path picker" do | |
| 115 | + test "same instance on both sides is a plain local PR", %{conn: conn, upstream: upstream} do | |
| 116 | + {:ok, _lv, html} = live(conn, "#{base(upstream)}/pulls/new") | |
| 117 | + | |
| 118 | + assert html =~ GitGud.Federation.base_url() | |
| 119 | + refute html =~ "Test connection" | |
| 120 | + refute html =~ "federated" | |
| 121 | + # Nothing is locked, so the submit is live. | |
| 122 | + refute html =~ ~s(<fieldset disabled) | |
| 123 | + end | |
| 124 | + | |
| 125 | + test "a different base instance locks the form until the probe passes", %{ | |
| 126 | + conn: conn, | |
| 127 | + upstream: upstream | |
| 128 | + } do | |
| 129 | + {:ok, lv, _html} = live(conn, "#{base(upstream)}/pulls/new") | |
| 130 | + | |
| 131 | + html = | |
| 132 | + lv | |
| 133 | + |> element("input[aria-label='Base instance URL']") | |
| 134 | + |> render_keyup(%{"value" => "https://other.example"}) | |
| 135 | + | |
| 136 | + assert html =~ "federated" | |
| 137 | + assert html =~ "Test connection" | |
| 138 | + assert html =~ "stays locked until this succeeds" | |
| 139 | + assert html =~ ~s(<fieldset disabled) | |
| 140 | + | |
| 141 | + # The composed actor URL is shown so you can see what gets probed. | |
| 142 | + html = | |
| 143 | + lv | |
| 144 | + |> element("input[aria-label='Base owner and repository']") | |
| 145 | + |> render_keyup(%{"value" => "them/proj", "field" => "remote_owner_repo"}) | |
| 146 | + | |
| 147 | + assert html =~ "https://other.example/r/them/proj" | |
| 148 | + end | |
| 149 | + | |
| 150 | + test "a failed probe keeps the form locked and says why", %{ | |
| 151 | + conn: conn, | |
| 152 | + upstream: upstream | |
| 153 | + } do | |
| 154 | + {:ok, lv, _html} = live(conn, "#{base(upstream)}/pulls/new") | |
| 155 | + | |
| 156 | + lv | |
| 157 | + |> element("input[aria-label='Base instance URL']") | |
| 158 | + |> render_keyup(%{"value" => "https://127.0.0.1:1"}) | |
| 159 | + | |
| 160 | + lv | |
| 161 | + |> element("input[aria-label='Base owner and repository']") | |
| 162 | + |> render_keyup(%{"value" => "them/proj", "field" => "remote_owner_repo"}) | |
| 163 | + | |
| 164 | + html = lv |> element("button[phx-click='test_connection']") |> render_click() | |
| 165 | + | |
| 166 | + assert html =~ "Couldn't reach it" or html =~ "Couldn't reach it" | |
| 167 | + assert html =~ ~s(<fieldset disabled) | |
| 168 | + end | |
| 169 | + | |
| 170 | + test "probing with no owner/repo asks for one instead of firing a request", %{ | |
| 171 | + conn: conn, | |
| 172 | + upstream: upstream | |
| 173 | + } do | |
| 174 | + {:ok, lv, _html} = live(conn, "#{base(upstream)}/pulls/new") | |
| 175 | + | |
| 176 | + lv | |
| 177 | + |> element("input[aria-label='Base instance URL']") | |
| 178 | + |> render_keyup(%{"value" => "https://other.example"}) | |
| 179 | + | |
| 180 | + html = lv |> element("button[phx-click='test_connection']") |> render_click() | |
| 181 | + | |
| 182 | + assert html =~ "owner/repo first" | |
| 183 | + end | |
| 184 | + | |
| 185 | + test "submitting to an untested remote is refused", %{conn: conn, upstream: upstream} do | |
| 186 | + {:ok, lv, _html} = live(conn, "#{base(upstream)}/pulls/new") | |
| 187 | + | |
| 188 | + lv | |
| 189 | + |> element("input[aria-label='Base instance URL']") | |
| 190 | + |> render_keyup(%{"value" => "https://other.example"}) | |
| 191 | + | |
| 192 | + html = | |
| 193 | + lv | |
| 194 | + |> form("#new-pr-form", %{"pull_request" => %{"title" => "x", "source_ref" => "main"}}) | |
| 195 | + |> render_submit() | |
| 196 | + | |
| 197 | + assert html =~ "Test the connection first" | |
| 198 | + end | |
| 199 | + end | |
| 200 | + | |
| 114 | 201 | test "the base repo can be switched, reversing the direction", %{ |
| 115 | 202 | conn: conn, |
| 116 | 203 | upstream: upstream, |
Parents: 8753207