Mobile API: open, edit, close, reopen and label pull requests

24ca95f · Gabriel Morell · 2026-10-18 17:07

23 files +951 -26
Message
{commit_body(@commit)}

Files changed

modified lib/git_gud/accounts/personal_access_token.ex
+1 −1
@@ -75,7 +75,7 @@ defmodule GitGud.Accounts.PersonalAccessToken do
75 75 "issues:read" => "Read issues and their comments",
76 76 "issues:write" => "Comment on, open and close issues",
77 77 "pulls:read" => "Read pull requests, reviews and comments",
78 − "pulls:write" => "Comment on, review and merge pull requests",
78 + "pulls:write" => "Open, edit, label, comment on, review and merge pull requests",
79 79 "wiki:read" => "Read wiki pages",
80 80 "wiki:write" => "Create and edit wiki pages",
81 81 "packages:read" => "Read published packages and how to install them",
modified lib/git_gud/pull_requests/pull_request.ex
+6 −0
@@ -38,6 +38,12 @@ defmodule GitGud.PullRequests.PullRequest do
38 38 field :moderation_note, :string
39 39 field :original_body, :string
40 40
41 + # The labels attached to it, when loaded (`PullRequests` fills this
42 + # from the polymorphic labelings table; it isn't a column). Declared
43 + # so a loaded pull request is still a `%PullRequest{}` to the type
44 + # checker, rather than a struct with a key it doesn't have.
45 + field :labels, :any, virtual: true
46 +
41 47 belongs_to :repository, Repository
42 48 belongs_to :source_repository, Repository
43 49 belongs_to :source_actor, Actor
modified lib/git_gud_web/controllers/api/mobile/json.ex
+13 −0
@@ -55,6 +55,19 @@ defmodule GitGudWeb.Api.Mobile.JSON do
55 55 def labels(list) when is_list(list), do: Enum.map(list, &label/1)
56 56 def labels(_), do: []
57 57
58 + @doc """
59 + A label in a repository's pickable set, as `Labels.list_labels/1`
60 + returns it: `inherited` when it comes from the organization.
61 + """
62 + def repo_label(l) do
63 + l
64 + |> label()
65 + |> Map.merge(%{
66 + "description" => Map.get(l, :description),
67 + "inherited" => Map.get(l, :inherited?, false)
68 + })
69 + end
70 +
58 71 # ── namespaces + repositories ────────────────────────────────────────
59 72
60 73 def namespace(%Organization{} = org) do
added lib/git_gud_web/controllers/api/mobile/label_controller.ex
+94 −0
@@ -0,0 +1,94 @@
1 +defmodule GitGudWeb.Api.Mobile.LabelController do
2 + @moduledoc """
3 + A repository's labels: the ones a pull request can be given.
4 +
5 + The list is `Labels.list_labels/1` — the repository's own labels and
6 + those it inherits from its organization, minus any it opted out of —
7 + so it's exactly the set the pull request page offers. Labels are
8 + attached to a pull request through `PullRequestController`.
9 +
10 + Creating one makes a repository label, as the repository's labels page
11 + does. That's repository configuration, so it takes a repository admin
12 + (`RepoLive.Header.admin?/2`, the rule behind the page's admin
13 + controls); organization labels are managed on the organization.
14 + """
15 + use GitGudWeb, :controller
16 +
17 + import GitGudWeb.Api.Mobile.Helpers
18 +
19 + alias GitGud.Labels
20 + alias GitGudWeb.Api.Mobile.Auth
21 + alias GitGudWeb.Api.Mobile.JSON
22 +
23 + # A neutral slate, for a label created without a colour.
24 + @default_color "#64748b"
25 +
26 + def index(conn, %{"owner" => owner, "name" => name}) do
27 + with {:ok, user} <- Auth.require_scope(conn, "pulls:read"),
28 + {:ok, repo} <- fetch_repo(conn, user, owner, name) do
29 + json(conn, %{"labels" => Enum.map(Labels.list_labels(repo), &JSON.repo_label/1)})
30 + else
31 + {:error, conn} -> conn
32 + end
33 + end
34 +
35 + @doc """
36 + Create a repository label: `name`, and optionally `color` (`#RRGGBB`)
37 + and `description`. A name the repository can already pick — its own
38 + or inherited — is refused rather than shadowed.
39 + """
40 + def create(conn, %{"owner" => owner, "name" => name} = params) do
41 + with {:ok, user} <- Auth.require_scope(conn, "pulls:write"),
42 + {:ok, repo} <- fetch_repo(conn, user, owner, name),
43 + :ok <- authorize_admin(conn, user, repo, owner, name),
44 + {:ok, label_name} <- require_name(conn, repo, params["label"]) do
45 + attrs = %{
46 + "name" => label_name,
47 + "color" => present(params["color"]) || @default_color,
48 + "description" => present(params["description"])
49 + }
50 +
51 + case Labels.create_label(repo, attrs) do
52 + {:ok, label} ->
53 + conn
54 + |> put_status(201)
55 + |> json(%{"label" => JSON.repo_label(Map.put(label, :inherited?, false))})
56 +
57 + {:error, %Ecto.Changeset{} = cs} ->
58 + send_error(conn, 422, changeset_message(cs))
59 + end
60 + else
61 + {:error, conn} -> conn
62 + end
63 + end
64 +
65 + defp authorize_admin(conn, user, repo, owner, name) do
66 + if GitGudWeb.RepoLive.Header.admin?(user, repo),
67 + do: :ok,
68 + else: {:error, send_error(conn, 403, "requires admin access to #{owner}/#{name}")}
69 + end
70 +
71 + defp require_name(conn, repo, raw) do
72 + label_name = String.trim(to_string(raw || ""))
73 +
74 + cond do
75 + label_name == "" ->
76 + {:error, send_error(conn, 422, "label can't be blank")}
77 +
78 + Enum.any?(Labels.list_labels(repo), &(&1.name == label_name)) ->
79 + {:error, send_error(conn, 422, "a label named #{label_name} already exists")}
80 +
81 + true ->
82 + {:ok, label_name}
83 + end
84 + end
85 +
86 + defp present(v) when is_binary(v) do
87 + case String.trim(v) do
88 + "" -> nil
89 + trimmed -> trimmed
90 + end
91 + end
92 +
93 + defp present(_), do: nil
94 +end
modified lib/git_gud_web/controllers/api/mobile/pull_request_controller.ex
+305 −22
@@ -17,11 +17,21 @@ defmodule GitGudWeb.Api.Mobile.PullRequestController do
17 17 without secrets, until someone with write access says the code may
18 18 run — and that someone is often away from a desk. Those two need
19 19 `ci:write`, since what they start or stop is CI.
20 +
21 + Opening, editing, closing, reopening and labelling go through the same
22 + context functions as `PrLive.New` and `PrLive.Show`, with `actor_id:`,
23 + so their `pull_request` workflows (`opened`, `edited`, `closed`,
24 + `reopened`, `labeled`, `unlabeled`) fire as they do from the web, and
25 + under the same rules: anyone who can read the repository may open a
26 + pull request; its author or a repository admin may edit, close,
27 + reopen and label it. Each answers with what `show/2` returns.
20 28 """
21 29 use GitGudWeb, :controller
22 30
23 31 import GitGudWeb.Api.Mobile.Helpers
24 32
33 + alias GitGud.Events
34 + alias GitGud.Labels
25 35 alias GitGud.Organizations
26 36 alias GitGud.PullRequests
27 37 alias GitGud.PullRequests.PrReview
@@ -50,33 +60,190 @@ defmodule GitGudWeb.Api.Mobile.PullRequestController do
50 60 {:ok, repo} <- fetch_repo(conn, user, owner, name),
51 61 {:ok, n} <- number_param(conn, number),
52 62 {:ok, pr} <- get_pull(conn, repo, n) do
53 − {discussion, line_comments} = Enum.split_with(pr.comments, &is_nil(&1.file_path))
54 −
55 − blockers =
56 − PullRequests.merge_blockers(repo, pr) ++ PullRequests.status_check_blockers(repo, pr)
57 −
58 − json(conn, %{
59 − "pull" => JSON.pull(pr, repo),
60 − "comments" => Enum.map(discussion, &JSON.comment(&1, repo)),
61 − "line_comments" => Enum.map(line_comments, &JSON.comment(&1, repo)),
62 − "reviews" => pr |> PullRequests.latest_reviews() |> Enum.map(&JSON.review/1),
63 − "review_requests" => Enum.map(pr.review_requests, &JSON.review_request/1),
64 − "merge_blockers" => Enum.map(blockers, &JSON.merge_blocker/1),
65 − "my_review" => pr |> PullRequests.review_by(user) |> maybe_review(),
66 − "can_comment" => Organizations.can?(user, repo, :read),
67 − "can_review" => Organizations.can?(user, repo, :read) and pr.author_id != user.id,
68 − "can_merge" => pr.state == "open" and Organizations.can?(user, repo, :write),
69 − "can_override" => Organizations.can?(user, repo, :admin),
70 − # A fork's workflow runs on the current head that nobody with
71 − # write access has approved yet, and whether the viewer may.
72 − "awaiting_approval_runs" => pr |> awaiting_runs() |> Enum.map(&JSON.run/1),
73 − "can_approve_runs" => Organizations.can?(user, repo, :write) == true
74 − })
63 + json(conn, detail(pr, repo, user))
64 + else
65 + {:error, conn} -> conn
66 + end
67 + end
68 +
69 + @doc """
70 + Open a pull request: `head` (a branch), `base` (a branch; the
71 + repository's default when omitted), `title` and `body`. `head_repo`
72 + (`owner/name`) takes the head from a fork, or any repository sharing
73 + this one's history that the caller can read — the pool `PrLive.New`
74 + offers. The pull request opens on the repository in the path.
75 + """
76 + def create(conn, %{"owner" => owner, "name" => name} = params) do
77 + with {:ok, user} <- Auth.require_scope(conn, "pulls:write"),
78 + {:ok, repo} <- fetch_repo(conn, user, owner, name),
79 + {:ok, source} <- head_repo(conn, user, repo, params["head_repo"]),
80 + {:ok, head} <- require_branch(conn, source, params["head"], "head"),
81 + {:ok, base} <- require_branch(conn, repo, base_param(params, repo), "base") do
82 + attrs = %{
83 + "title" => params["title"],
84 + "body" => params["body"],
85 + "source_ref" => head,
86 + "target_ref" => base,
87 + "source_repository_id" => source.id
88 + }
89 +
90 + case PullRequests.create_pull_request(repo, user, attrs) do
91 + {:ok, pr} ->
92 + :ok = Events.record_cross_references(pr, repo, nil, pr.body, user)
93 + pr = PullRequests.get_pull_request!(repo, pr.number)
94 + conn |> put_status(201) |> json(detail(pr, repo, user))
95 +
96 + {:error, :banned} ->
97 + send_error(conn, 403, "you are banned from this repository")
98 +
99 + {:error, :no_changes} ->
100 + send_error(conn, 422, "#{base} already contains #{head} — there's nothing to merge")
101 +
102 + {:error, %Ecto.Changeset{} = cs} ->
103 + send_error(conn, 422, changeset_message(cs))
104 +
105 + {:error, reason} ->
106 + send_error(conn, 422, "could not open the pull request: #{inspect(reason)}")
107 + end
108 + else
109 + {:error, conn} -> conn
110 + end
111 + end
112 +
113 + @doc """
114 + Edit the title and/or the body. Records the same history events as the
115 + web page, and one `edited` workflow dispatch for the change.
116 + """
117 + def update(conn, %{"owner" => owner, "name" => name, "number" => number} = params) do
118 + with {:ok, user} <- Auth.require_scope(conn, "pulls:write"),
119 + {:ok, repo} <- fetch_repo(conn, user, owner, name),
120 + {:ok, n} <- number_param(conn, number),
121 + {:ok, pr} <- get_pull(conn, repo, n),
122 + :ok <- authorize_edit(conn, user, repo, pr),
123 + {:ok, attrs} <- edit_attrs(conn, params) do
124 + case PullRequests.update_pull_request(pr, attrs, actor_id: user.id) do
125 + {:ok, updated} ->
126 + :ok = record_edit(pr, updated, repo, user)
127 + json(conn, detail(PullRequests.get_pull_request!(repo, n), repo, user))
128 +
129 + {:error, %Ecto.Changeset{} = cs} ->
130 + send_error(conn, 422, changeset_message(cs))
131 + end
75 132 else
76 133 {:error, conn} -> conn
77 134 end
78 135 end
79 136
137 + @doc """
138 + Close (`"state": "closed"`) or reopen (`"open"`). A merged pull request
139 + is terminal; asking for the state it's already in changes nothing.
140 + """
141 + def set_state(conn, %{"owner" => owner, "name" => name, "number" => number} = params) do
142 + with {:ok, user} <- Auth.require_scope(conn, "pulls:write"),
143 + {:ok, repo} <- fetch_repo(conn, user, owner, name),
144 + {:ok, n} <- number_param(conn, number),
145 + {:ok, pr} <- get_pull(conn, repo, n),
146 + {:ok, state} <- require_state(conn, params),
147 + :ok <- authorize_state(conn, user, repo, pr) do
148 + cond do
149 + pr.state == "merged" ->
150 + send_error(conn, 422, "pull request ##{n} is merged")
151 +
152 + pr.state == state ->
153 + json(conn, detail(pr, repo, user))
154 +
155 + true ->
156 + case PullRequests.set_state(pr, state, actor_id: user.id) do
157 + {:ok, _} ->
158 + kind = if state == "closed", do: "closed", else: "reopened"
159 + :ok = Events.record(pr, kind, user)
160 + json(conn, detail(PullRequests.get_pull_request!(repo, n), repo, user))
161 +
162 + {:error, :merged} ->
163 + send_error(conn, 422, "a merged pull request can't be reopened")
164 +
165 + {:error, %Ecto.Changeset{} = cs} ->
166 + send_error(conn, 422, changeset_message(cs))
167 + end
168 + end
169 + else
170 + {:error, conn} -> conn
171 + end
172 + end
173 +
174 + @doc """
175 + Attach one of the repository's labels (its own or inherited from its
176 + organization), by `name`. Attaching one already there changes nothing.
177 + """
178 + def add_label(conn, %{"owner" => owner, "name" => name, "number" => number} = params) do
179 + with {:ok, user} <- Auth.require_scope(conn, "pulls:write"),
180 + {:ok, repo} <- fetch_repo(conn, user, owner, name),
181 + {:ok, n} <- number_param(conn, number),
182 + {:ok, pr} <- get_pull(conn, repo, n),
183 + :ok <- authorize_edit(conn, user, repo, pr),
184 + {:ok, label} <- find_label(conn, repo, params["label"]) do
185 + if not labeled?(pr, label) do
186 + :ok = PullRequests.attach_label(pr, label, actor_id: user.id)
187 + :ok = Events.record(pr, "labeled", user, %{name: label.name, color: label.color})
188 + end
189 +
190 + json(conn, detail(PullRequests.get_pull_request!(repo, n), repo, user))
191 + else
192 + {:error, conn} -> conn
193 + end
194 + end
195 +
196 + @doc "Detach a label, by name. Detaching one that isn't there changes nothing."
197 + def remove_label(conn, %{"owner" => owner, "name" => name, "number" => number} = params) do
198 + with {:ok, user} <- Auth.require_scope(conn, "pulls:write"),
199 + {:ok, repo} <- fetch_repo(conn, user, owner, name),
200 + {:ok, n} <- number_param(conn, number),
201 + {:ok, pr} <- get_pull(conn, repo, n),
202 + :ok <- authorize_edit(conn, user, repo, pr) do
203 + # Matched against what's attached rather than the pickable set: an
204 + # org label this repo has since opted out of is still removable.
205 + case Enum.find(pr.labels, &(&1.name == params["label"])) do
206 + nil ->
207 + :ok
208 +
209 + label ->
210 + :ok = PullRequests.detach_label(pr, label, actor_id: user.id)
211 + :ok = Events.record(pr, "unlabeled", user, %{name: label.name, color: label.color})
212 + end
213 +
214 + json(conn, detail(PullRequests.get_pull_request!(repo, n), repo, user))
215 + else
216 + {:error, conn} -> conn
217 + end
218 + end
219 +
220 + # What `show/2` answers, and every write with it.
221 + defp detail(pr, repo, user) do
222 + {discussion, line_comments} = Enum.split_with(pr.comments, &is_nil(&1.file_path))
223 +
224 + blockers =
225 + PullRequests.merge_blockers(repo, pr) ++ PullRequests.status_check_blockers(repo, pr)
226 +
227 + %{
228 + "pull" => JSON.pull(pr, repo),
229 + "comments" => Enum.map(discussion, &JSON.comment(&1, repo)),
230 + "line_comments" => Enum.map(line_comments, &JSON.comment(&1, repo)),
231 + "reviews" => pr |> PullRequests.latest_reviews() |> Enum.map(&JSON.review/1),
232 + "review_requests" => Enum.map(pr.review_requests, &JSON.review_request/1),
233 + "merge_blockers" => Enum.map(blockers, &JSON.merge_blocker/1),
234 + "my_review" => pr |> PullRequests.review_by(user) |> maybe_review(),
235 + "can_comment" => Organizations.can?(user, repo, :read),
236 + "can_review" => Organizations.can?(user, repo, :read) and pr.author_id != user.id,
237 + "can_merge" => pr.state == "open" and Organizations.can?(user, repo, :write),
238 + "can_override" => Organizations.can?(user, repo, :admin),
239 + # A fork's workflow runs on the current head that nobody with
240 + # write access has approved yet, and whether the viewer may.
241 + "awaiting_approval_runs" => pr |> awaiting_runs() |> Enum.map(&JSON.run/1),
242 + "can_approve_runs" => Organizations.can?(user, repo, :write) == true,
243 + "can_edit" => can_edit?(user, repo, pr)
244 + }
245 + end
246 +
80 247 def comment(conn, %{"owner" => owner, "name" => name, "number" => number} = params) do
81 248 with {:ok, user} <- Auth.require_scope(conn, "pulls:write"),
82 249 {:ok, repo} <- fetch_repo(conn, user, owner, name),
@@ -200,6 +367,122 @@ defmodule GitGudWeb.Api.Mobile.PullRequestController do
200 367
201 368 # ── internals ────────────────────────────────────────────────────────
202 369
370 + # `PrLive.Show`'s rule for renaming, re-describing, closing, reopening
371 + # and labelling: the author, or a repository admin as the repository
372 + # pages decide it (`RepoLive.Header.admin?/2`). Restated here because
373 + # the context doesn't enforce it.
374 + defp can_edit?(user, repo, pr),
375 + do: pr.author_id == user.id or GitGudWeb.RepoLive.Header.admin?(user, repo)
376 +
377 + defp authorize_edit(conn, user, repo, pr) do
378 + if can_edit?(user, repo, pr),
379 + do: :ok,
380 + else: {:error, send_error(conn, 403, "only the author or a repository admin can do that")}
381 + end
382 +
383 + defp authorize_state(conn, user, repo, pr), do: authorize_edit(conn, user, repo, pr)
384 +
385 + # The repository the head branch lives in: this one, unless `head_repo`
386 + # names another the caller can read that shares its history.
387 + defp head_repo(_conn, _user, repo, blank) when blank in [nil, ""], do: {:ok, repo}
388 +
389 + defp head_repo(conn, user, repo, raw) when is_binary(raw) do
390 + with [o, n] <- String.split(String.trim(raw), "/", parts: 2),
391 + %{} = found <- GitGud.Repositories.find_by_path(o, n),
392 + %{} = source <-
393 + Enum.find(
394 + GitGud.Repositories.list_related_repositories(repo, user),
395 + &(&1.id == found.id)
396 + ) do
397 + {:ok, source}
398 + else
399 + _ ->
400 + {:error,
401 + send_error(
402 + conn,
403 + 422,
404 + "head_repo #{raw} isn't a repository you can read that shares #{repo.name}'s history"
405 + )}
406 + end
407 + end
408 +
409 + defp head_repo(conn, _user, _repo, _),
410 + do: {:error, send_error(conn, 422, "head_repo must be owner/name")}
411 +
412 + # A branch, not any revision `git` would take: the web form only ever
413 + # offers branches, and a pull request tracks one.
414 + defp require_branch(conn, repo, raw, field) do
415 + branch = String.trim(to_string(raw || ""))
416 +
417 + cond do
418 + branch == "" ->
419 + {:error, send_error(conn, 422, "#{field} can't be blank")}
420 +
421 + match?({:ok, _}, GitGud.Repositories.resolve(repo, "refs/heads/" <> branch)) ->
422 + {:ok, branch}
423 +
424 + true ->
425 + {:error, send_error(conn, 422, "#{field}: no branch named #{branch}")}
426 + end
427 + end
428 +
429 + defp base_param(%{"base" => base}, _repo) when is_binary(base) and base != "", do: base
430 + defp base_param(_params, repo), do: repo.default_branch
431 +
432 + # Title and body as the web page saves them, trimmed; at least one.
433 + defp edit_attrs(conn, params) do
434 + attrs =
435 + for key <- ["title", "body"],
436 + is_binary(params[key]),
437 + into: %{},
438 + do: {key, String.trim(params[key])}
439 +
440 + if attrs == %{},
441 + do: {:error, send_error(conn, 422, "nothing to change — pass a title or a body")},
442 + else: {:ok, attrs}
443 + end
444 +
445 + # The history entries `PrLive.Show` writes for a title or a description
446 + # change, plus the cross-references a new description makes.
447 + defp record_edit(before, updated, repo, user) do
448 + if updated.title != before.title do
449 + :ok =
450 + Events.record(before, "title_changed", user, %{from: before.title, to: updated.title})
451 + end
452 +
453 + if (updated.body || "") != (before.body || "") do
454 + :ok =
455 + Events.record(before, "description_changed", user, %{
456 + from: before.body || "",
457 + to: updated.body || ""
458 + })
459 +
460 + :ok = Events.record_cross_references(updated, repo, before.body, updated.body, user)
461 + end
462 +
463 + :ok
464 + end
465 +
466 + defp require_state(conn, params) do
467 + case params["state"] do
468 + s when s in ["open", "closed"] -> {:ok, s}
469 + _ -> {:error, send_error(conn, 422, "state must be 'open' or 'closed'")}
470 + end
471 + end
472 +
473 + # One of the labels the repository can pick from, by exact name.
474 + defp find_label(conn, repo, raw) do
475 + name = String.trim(to_string(raw || ""))
476 +
477 + case Enum.find(Labels.list_labels(repo), &(&1.name == name)) do
478 + nil when name == "" -> {:error, send_error(conn, 422, "label can't be blank")}
479 + nil -> {:error, send_error(conn, 422, "no label named #{name} in this repository")}
480 + label -> {:ok, label}
481 + end
482 + end
483 +
484 + defp labeled?(pr, label), do: Enum.any?(pr.labels, &(&1.id == label.id))
485 +
203 486 defp awaiting_runs(pr) do
204 487 pr
205 488 |> GitGud.Workflows.awaiting_approval_runs()
modified lib/git_gud_web/live/repo_live/header.ex
+10 −3
@@ -189,7 +189,14 @@ defmodule GitGudWeb.RepoLive.Header do
189 189 |> Phoenix.Component.assign(:repo_tags, GitGud.Tags.tag_names(repo))
190 190 end
191 191
192 − defp admin?(user, %{owner_id: uid, organization_id: nil}), do: user.id == uid
193 − defp admin?(user, %{organization: %{} = org}), do: Organizations.org_admin?(org, user)
194 − defp admin?(_, _), do: false
192 + @doc """
193 + Whether `user` administers `repo` the way the repository pages decide
194 + it (`:can_admin?`): its owner, or an admin of the organization that
195 + owns it. `repo.organization` must be preloaded. Public so the mobile
196 + API applies the same rule as the page it stands in for.
197 + """
198 + def admin?(nil, _repo), do: false
199 + def admin?(user, %{owner_id: uid, organization_id: nil}), do: user.id == uid
200 + def admin?(user, %{organization: %{} = org}), do: Organizations.org_admin?(org, user)
201 + def admin?(_, _), do: false
195 202 end
modified lib/git_gud_web/router.ex
+8 −0
@@ -359,13 +359,21 @@ defmodule GitGudWeb.Router do
359 359 post "/repos/:owner/:name/issues/:number/state", IssueController, :set_state
360 360
361 361 get "/repos/:owner/:name/pulls", PullRequestController, :index
362 + post "/repos/:owner/:name/pulls", PullRequestController, :create
362 363 get "/repos/:owner/:name/pulls/:number", PullRequestController, :show
364 + patch "/repos/:owner/:name/pulls/:number", PullRequestController, :update
365 + post "/repos/:owner/:name/pulls/:number/state", PullRequestController, :set_state
366 + post "/repos/:owner/:name/pulls/:number/labels", PullRequestController, :add_label
367 + delete "/repos/:owner/:name/pulls/:number/labels/:label", PullRequestController, :remove_label
363 368 post "/repos/:owner/:name/pulls/:number/comments", PullRequestController, :comment
364 369 post "/repos/:owner/:name/pulls/:number/reviews", PullRequestController, :review
365 370 post "/repos/:owner/:name/pulls/:number/merge", PullRequestController, :merge
366 371 post "/repos/:owner/:name/pulls/:number/runs/approve", PullRequestController, :approve_runs
367 372 post "/repos/:owner/:name/pulls/:number/runs/reject", PullRequestController, :reject_runs
368 373
374 + get "/repos/:owner/:name/labels", LabelController, :index
375 + post "/repos/:owner/:name/labels", LabelController, :create
376 +
369 377 get "/repos/:owner/:name/wiki", WikiController, :index
370 378 get "/repos/:owner/:name/wiki/:slug", WikiController, :show
371 379
added test/git_gud_web/controllers/api/mobile/mobile_pulls_write_test.exs
+514 −0
@@ -0,0 +1,514 @@
1 +defmodule GitGudWeb.Api.Mobile.MobilePullsWriteTest do
2 + @moduledoc """
3 + Opening, editing, closing, reopening and labelling pull requests over
4 + the mobile API, and listing and creating the labels they take.
5 +
6 + Each write goes through the context function `PrLive.New` /
7 + `PrLive.Show` call, under the same rules — so besides the answers,
8 + these check that each one enqueues the `pull_request` workflow dispatch
9 + the web page would have.
10 + """
11 + use GitGudWeb.ConnCase, async: true
12 + use Oban.Testing, repo: GitGud.Repo
13 +
14 + import GitGud.AccountsFixtures
15 + import GitGud.ForgeFixtures
16 +
17 + alias GitGud.Accounts
18 + alias GitGud.Accounts.PersonalAccessToken
19 + alias GitGud.Git
20 + alias GitGud.PullRequests
21 + alias GitGud.Repositories
22 + alias GitGud.Workflows.Workers.PullRequestDispatch
23 +
24 + @base "/api/mobile/v1"
25 +
26 + defp token_for(user, scopes) do
27 + {:ok, _t, plaintext} =
28 + Accounts.create_personal_access_token(
29 + user,
30 + "tok-#{System.unique_integer([:positive])}",
31 + scopes
32 + )
33 +
34 + plaintext
35 + end
36 +
37 + defp as(conn, token) do
38 + conn
39 + |> put_req_header("authorization", "Bearer #{token}")
40 + |> put_req_header("accept", "application/json")
41 + end
42 +
43 + defp mobile_conn(conn, user), do: as(conn, token_for(user, PersonalAccessToken.mobile_scopes()))
44 +
45 + # `main`, and a `feature` branch one commit ahead of it.
46 + defp seed_branches(repo, user) do
47 + main = seed_main_branch(repo, user)
48 + :ok = Git.update_ref(repo.disk_path, "refs/heads/feature", main, nil)
49 + advance_branch(repo, "feature", user)
50 + :ok
51 + end
52 +
53 + defp api_path(repo, owner), do: "#{@base}/repos/#{owner.handle}/#{repo.name}"
54 +
55 + defp dispatched?(pr, action) do
56 + pr = if is_integer(pr), do: pr, else: pr.id
57 +
58 + all_enqueued(worker: PullRequestDispatch)
59 + |> Enum.any?(&(&1.args["pull_request_id"] == pr and &1.args["action"] == action))
60 + end
61 +
62 + defp public_repo do
63 + {owner, repo} = repository_fixture(%{visibility: "public"})
64 + seed_branches(repo, owner)
65 + {owner, repo}
66 + end
67 +
68 + describe "create" do
69 + setup %{conn: conn} do
70 + {owner, repo} = repository_fixture()
71 + seed_branches(repo, owner)
72 + %{conn: conn, owner: owner, repo: repo, path: api_path(repo, owner) <> "/pulls"}
73 + end
74 +
75 + test "opens one onto the default branch and answers with the detail", ctx do
76 + body =
77 + ctx.conn
78 + |> mobile_conn(ctx.owner)
79 + |> post(ctx.path, %{"head" => "feature", "title" => "Add a feature", "body" => "Why."})
80 + |> json_response(201)
81 +
82 + assert %{"number" => number, "title" => "Add a feature", "state" => "open"} = body["pull"]
83 + assert body["pull"]["source_ref"] == "feature"
84 + assert body["pull"]["target_ref"] == "main"
85 + assert body["pull"]["body"] == "Why."
86 + assert body["can_edit"]
87 + assert is_list(body["merge_blockers"])
88 +
89 + pr = PullRequests.get_pull_request!(ctx.repo, number)
90 + assert pr.author_id == ctx.owner.id
91 + assert dispatched?(pr, "opened")
92 + end
93 +
94 + test "takes an explicit base", ctx do
95 + body =
96 + ctx.conn
97 + |> mobile_conn(ctx.owner)
98 + |> post(ctx.path, %{"head" => "feature", "base" => "main", "title" => "T"})
99 + |> json_response(201)
100 +
101 + assert body["pull"]["target_ref"] == "main"
102 + end
103 +
104 + test "422s without a title, and enqueues nothing", ctx do
105 + body =
106 + ctx.conn
107 + |> mobile_conn(ctx.owner)
108 + |> post(ctx.path, %{"head" => "feature"})
109 + |> json_response(422)
110 +
111 + assert body["message"] =~ "title"
112 + assert all_enqueued(worker: PullRequestDispatch) == []
113 + end
114 +
115 + test "422s on a head or base branch that doesn't exist", ctx do
116 + conn = mobile_conn(ctx.conn, ctx.owner)
117 +
118 + assert %{"message" => head_msg} =
119 + conn
120 + |> post(ctx.path, %{"head" => "nope", "title" => "T"})
121 + |> json_response(422)
122 +
123 + assert head_msg =~ "nope"
124 +
125 + assert %{"message" => base_msg} =
126 + conn
127 + |> post(ctx.path, %{"head" => "feature", "base" => "gone", "title" => "T"})
128 + |> json_response(422)
129 +
130 + assert base_msg =~ "gone"
131 + end
132 +
133 + test "422s without a head", ctx do
134 + assert ctx.conn
135 + |> mobile_conn(ctx.owner)
136 + |> post(ctx.path, %{"title" => "T"})
137 + |> json_response(422)
138 + end
139 +
140 + test "422s when head and base are the same branch", ctx do
141 + body =
142 + ctx.conn
143 + |> mobile_conn(ctx.owner)
144 + |> post(ctx.path, %{"head" => "main", "base" => "main", "title" => "T"})
145 + |> json_response(422)
146 +
147 + assert body["message"] =~ "differ"
148 + end
149 +
150 + test "422s when the base already has everything the head does", ctx do
151 + assert ctx.conn
152 + |> mobile_conn(ctx.owner)
153 + |> post(ctx.path, %{"head" => "main", "base" => "feature", "title" => "T"})
154 + |> json_response(422)
155 + end
156 +
157 + test "needs the pulls:write scope", ctx do
158 + assert ctx.conn
159 + |> as(token_for(ctx.owner, ["pulls:read", "repo:read"]))
160 + |> post(ctx.path, %{"head" => "feature", "title" => "T"})
161 + |> json_response(403)
162 + end
163 +
164 + test "a private repository the caller can't read is a 404", ctx do
165 + assert ctx.conn
166 + |> mobile_conn(user_fixture())
167 + |> post(ctx.path, %{"head" => "feature", "title" => "T"})
168 + |> json_response(404)
169 + end
170 +
171 + test "anyone who can read a public repository may open one, as on the web", %{conn: conn} do
172 + {owner, repo} = public_repo()
173 + contributor = user_fixture()
174 +
175 + body =
176 + conn
177 + |> mobile_conn(contributor)
178 + |> post(api_path(repo, owner) <> "/pulls", %{"head" => "feature", "title" => "T"})
179 + |> json_response(201)
180 +
181 + assert body["pull"]["author"]["handle"] == contributor.handle
182 + refute body["can_merge"]
183 + end
184 +
185 + test "a user banned from the repository is refused", %{conn: conn} do
186 + {owner, repo} = public_repo()
187 + banned = user_fixture()
188 + {:ok, _} = GitGud.Moderation.ban(repo, banned, banned_by: owner)
189 +
190 + assert conn
191 + |> mobile_conn(banned)
192 + |> post(api_path(repo, owner) <> "/pulls", %{"head" => "feature", "title" => "T"})
193 + |> json_response(403)
194 + end
195 + end
196 +
197 + describe "create from a fork" do
198 + test "head_repo takes the head from a repository sharing this one's history", %{
199 + conn: conn
200 + } do
201 + {owner, upstream} = repository_fixture(%{visibility: "public"})
202 + seed_main_branch(upstream, owner)
203 +
204 + contributor = user_fixture()
205 + {_, fork} = repository_fixture(%{owner: contributor, visibility: "public"})
206 +
207 + {_, 0} =
208 + System.cmd(
209 + "git",
210 + ["-C", fork.disk_path, "fetch", upstream.disk_path, "+refs/*:refs/*"],
211 + stderr_to_stdout: true
212 + )
213 +
214 + advance_branch(fork, "main", contributor)
215 + Repositories.refresh_root_commits(upstream)
216 + Repositories.refresh_root_commits(fork)
217 +
218 + body =
219 + conn
220 + |> mobile_conn(contributor)
221 + |> post(api_path(upstream, owner) <> "/pulls", %{
222 + "head" => "main",
223 + "head_repo" => "#{contributor.handle}/#{fork.name}",
224 + "title" => "From my fork"
225 + })
226 + |> json_response(201)
227 +
228 + pr = PullRequests.get_pull_request!(upstream, body["pull"]["number"])
229 + assert pr.source_repository_id == fork.id
230 + assert pr.repository_id == upstream.id
231 + end
232 +
233 + test "head_repo must be related and readable", %{conn: conn} do
234 + {owner, repo} = public_repo()
235 + {stranger, unrelated} = repository_fixture()
236 + seed_branches(unrelated, stranger)
237 +
238 + assert conn
239 + |> mobile_conn(owner)
240 + |> post(api_path(repo, owner) <> "/pulls", %{
241 + "head" => "feature",
242 + "head_repo" => "#{stranger.handle}/#{unrelated.name}",
243 + "title" => "T"
244 + })
245 + |> json_response(422)
246 + end
247 + end
248 +
249 + describe "edit" do
250 + setup %{conn: conn} do
251 + {owner, repo} = public_repo()
252 + author = user_fixture()
253 +
254 + {:ok, pr} =
255 + PullRequests.create_pull_request(repo, author, pr_attrs("feature", "main"))
256 +
257 + %{
258 + conn: conn,
259 + owner: owner,
260 + author: author,
261 + repo: repo,
262 + pr: pr,
263 + path: api_path(repo, owner) <> "/pulls/#{pr.number}"
264 + }
265 + end
266 +
267 + test "the author changes the title and body, firing `edited` once", ctx do
268 + body =
269 + ctx.conn
270 + |> mobile_conn(ctx.author)
271 + |> patch(ctx.path, %{"title" => " Renamed ", "body" => "New description"})
272 + |> json_response(200)
273 +
274 + assert body["pull"]["title"] == "Renamed"
275 + assert body["pull"]["body"] == "New description"
276 +
277 + edited =
278 + all_enqueued(worker: PullRequestDispatch)
279 + |> Enum.filter(&(&1.args["action"] == "edited"))
280 +
281 + assert [_one] = edited
282 +
283 + kinds = ctx.pr |> GitGud.Events.list_for() |> Enum.map(& &1.kind)
284 + assert "title_changed" in kinds
285 + assert "description_changed" in kinds
286 + end
287 +
288 + test "the repository owner may edit someone else's", ctx do
289 + assert ctx.conn
290 + |> mobile_conn(ctx.owner)
291 + |> patch(ctx.path, %{"title" => "Owner's title"})
292 + |> json_response(200)
293 +
294 + assert dispatched?(ctx.pr, "edited")
295 + end
296 +
297 + test "anyone else gets a 403", ctx do
298 + assert ctx.conn
299 + |> mobile_conn(user_fixture())
300 + |> patch(ctx.path, %{"title" => "Mine now"})
301 + |> json_response(403)
302 +
303 + refute dispatched?(ctx.pr, "edited")
304 + end
305 +
306 + test "a blank title is a 422", ctx do
307 + assert %{"message" => msg} =
308 + ctx.conn
309 + |> mobile_conn(ctx.author)
310 + |> patch(ctx.path, %{"title" => " "})
311 + |> json_response(422)
312 +
313 + assert msg =~ "title"
314 + end
315 +
316 + test "nothing to change is a 422", ctx do
317 + assert ctx.conn
318 + |> mobile_conn(ctx.author)
319 + |> patch(ctx.path, %{})
320 + |> json_response(422)
321 + end
322 +
323 + test "an unknown number is a 404", ctx do
324 + assert ctx.conn
325 + |> mobile_conn(ctx.author)
326 + |> patch(api_path(ctx.repo, ctx.owner) <> "/pulls/999", %{"title" => "x"})
327 + |> json_response(404)
328 + end
329 + end
330 +
331 + describe "close and reopen" do
332 + setup %{conn: conn} do
333 + {owner, repo} = public_repo()
334 + author = user_fixture()
335 + {:ok, pr} = PullRequests.create_pull_request(repo, author, pr_attrs("feature", "main"))
336 +
337 + %{
338 + conn: conn,
339 + owner: owner,
340 + author: author,
341 + repo: repo,
342 + pr: pr,
343 + path: api_path(repo, owner) <> "/pulls/#{pr.number}/state"
344 + }
345 + end
346 +
347 + test "the author closes and reopens, firing `closed` and `reopened`", ctx do
348 + conn = mobile_conn(ctx.conn, ctx.author)
349 +
350 + assert %{"pull" => %{"state" => "closed"}} =
351 + conn |> post(ctx.path, %{"state" => "closed"}) |> json_response(200)
352 +
353 + assert dispatched?(ctx.pr, "closed")
354 +
355 + assert %{"pull" => %{"state" => "open"}} =
356 + conn |> post(ctx.path, %{"state" => "open"}) |> json_response(200)
357 +
358 + assert dispatched?(ctx.pr, "reopened")
359 + end
360 +
361 + test "a repository admin may close someone else's", ctx do
362 + assert %{"pull" => %{"state" => "closed"}} =
363 + ctx.conn
364 + |> mobile_conn(ctx.owner)
365 + |> post(ctx.path, %{"state" => "closed"})
366 + |> json_response(200)
367 + end
368 +
369 + test "anyone else gets a 403", ctx do
370 + assert ctx.conn
371 + |> mobile_conn(user_fixture())
372 + |> post(ctx.path, %{"state" => "closed"})
373 + |> json_response(403)
374 +
375 + refute dispatched?(ctx.pr, "closed")
376 + end
377 +
378 + test "an unknown state is a 422", ctx do
379 + assert ctx.conn
380 + |> mobile_conn(ctx.author)
381 + |> post(ctx.path, %{"state" => "merged"})
382 + |> json_response(422)
383 + end
384 +
385 + test "closing a closed one changes nothing", ctx do
386 + {:ok, _} = PullRequests.set_state(ctx.pr, "closed")
387 + before = all_enqueued(worker: PullRequestDispatch) |> length()
388 +
389 + assert %{"pull" => %{"state" => "closed"}} =
390 + ctx.conn
391 + |> mobile_conn(ctx.author)
392 + |> post(ctx.path, %{"state" => "closed"})
393 + |> json_response(200)
394 +
395 + assert length(all_enqueued(worker: PullRequestDispatch)) == before
396 + end
397 + end
398 +
399 + describe "labels" do
400 + setup %{conn: conn} do
401 + {owner, repo} = public_repo()
402 + author = user_fixture()
403 + {:ok, pr} = PullRequests.create_pull_request(repo, author, pr_attrs("feature", "main"))
404 + label = label_fixture(repo, %{"name" => "bug"})
405 +
406 + %{
407 + conn: conn,
408 + owner: owner,
409 + author: author,
410 + repo: repo,
411 + pr: pr,
412 + label: label,
413 + repo_path: api_path(repo, owner),
414 + path: api_path(repo, owner) <> "/pulls/#{pr.number}/labels"
415 + }
416 + end
417 +
418 + test "lists the repository's labels", ctx do
419 + body =
420 + ctx.conn
421 + |> as(token_for(user_fixture(), ["pulls:read"]))
422 + |> get(ctx.repo_path <> "/labels")
423 + |> json_response(200)
424 +
425 + assert [%{"name" => "bug", "color" => "#ef4444", "inherited" => false}] = body["labels"]
426 + end
427 +
428 + test "the author adds and removes one, firing `labeled` and `unlabeled`", ctx do
429 + conn = mobile_conn(ctx.conn, ctx.author)
430 +
431 + assert %{"pull" => %{"labels" => [%{"name" => "bug"}]}} =
432 + conn |> post(ctx.path, %{"label" => "bug"}) |> json_response(200)
433 +
434 + assert dispatched?(ctx.pr, "labeled")
435 +
436 + assert %{"pull" => %{"labels" => []}} =
437 + conn |> delete(ctx.path <> "/bug") |> json_response(200)
438 +
439 + assert dispatched?(ctx.pr, "unlabeled")
440 + end
441 +
442 + test "adding one already there fires nothing more", ctx do
443 + :ok = PullRequests.attach_label(ctx.pr, ctx.label)
444 + before = all_enqueued(worker: PullRequestDispatch) |> length()
445 +
446 + assert ctx.conn
447 + |> mobile_conn(ctx.author)
448 + |> post(ctx.path, %{"label" => "bug"})
449 + |> json_response(200)
450 +
451 + assert length(all_enqueued(worker: PullRequestDispatch)) == before
452 + end
453 +
454 + test "an unknown label is a 422", ctx do
455 + assert ctx.conn
456 + |> mobile_conn(ctx.author)
457 + |> post(ctx.path, %{"label" => "nope"})
458 + |> json_response(422)
459 + end
460 +
461 + test "anyone but the author or an admin gets a 403", ctx do
462 + conn = mobile_conn(ctx.conn, user_fixture())
463 +
464 + assert conn |> post(ctx.path, %{"label" => "bug"}) |> json_response(403)
465 + assert conn |> delete(ctx.path <> "/bug") |> json_response(403)
466 + refute dispatched?(ctx.pr, "labeled")
467 + end
468 +
469 + test "a repository admin creates a label", ctx do
470 + body =
471 + ctx.conn
472 + |> mobile_conn(ctx.owner)
473 + |> post(ctx.repo_path <> "/labels", %{"label" => "needs-docs", "color" => "#3b82f6"})
474 + |> json_response(201)
475 +
476 + assert %{"name" => "needs-docs", "color" => "#3b82f6"} = body["label"]
477 +
478 + # …which can then be attached.
479 + assert ctx.conn
480 + |> mobile_conn(ctx.author)
481 + |> post(ctx.path, %{"label" => "needs-docs"})
482 + |> json_response(200)
483 + end
484 +
485 + test "a created label without a colour gets a default one", ctx do
486 + body =
487 + ctx.conn
488 + |> mobile_conn(ctx.owner)
489 + |> post(ctx.repo_path <> "/labels", %{"label" => "triage"})
490 + |> json_response(201)
491 +
492 + assert body["label"]["color"] =~ ~r/\A#[0-9a-f]{6}\z/
493 + end
494 +
495 + test "creating a label needs a repository admin", ctx do
496 + assert ctx.conn
497 + |> mobile_conn(ctx.author)
498 + |> post(ctx.repo_path <> "/labels", %{"label" => "mine"})
499 + |> json_response(403)
500 + end
501 +
502 + test "creating a duplicate or a bad colour is a 422", ctx do
503 + conn = mobile_conn(ctx.conn, ctx.owner)
504 +
505 + assert conn
506 + |> post(ctx.repo_path <> "/labels", %{"label" => "bug"})
507 + |> json_response(422)
508 +
509 + assert conn
510 + |> post(ctx.repo_path <> "/labels", %{"label" => "x", "color" => "red"})
511 + |> json_response(422)
512 + end
513 + end
514 +end

Parents: f19604a