neiam /gitgud
Git Gud
public · Issues · Pulls · Labels · Forks · Compare · Actions success · Packages
⭐
Log in to mark this repository.
Make comments editable and fold their history into the timeline
6f89f63 · Gabriel Morell · 2026-09-09 22:04
Message
{commit_body(@commit)}
Files changed
modified
lib/git_gud/events/event.ex
+1
−0
@@ -12,6 +12,7 @@ defmodule GitGud.Events.Event do
| 12 | 12 | pushed |
| 13 | 13 | labeled unlabeled |
| 14 | 14 | moderated unmoderated |
| 15 | + comment_added comment_edited comment_deleted | |
| 15 | 16 | ) |
| 16 | 17 | |
| 17 | 18 | schema "events" do |
modified
lib/git_gud/issues.ex
+17
−0
@@ -206,5 +206,22 @@ defmodule GitGud.Issues do
| 206 | 206 | def attach_label(%Issue{id: id}, label), do: Labels.attach(label, {@target_type, id}) |
| 207 | 207 | def detach_label(%Issue{id: id}, label), do: Labels.detach(label, {@target_type, id}) |
| 208 | 208 | |
| 209 | + @doc "Edit a comment's body, stamping `edited_at`." | |
| 210 | + def edit_comment(%IssueComment{} = comment, attrs) do | |
| 211 | + comment | |
| 212 | + |> IssueComment.edit_changeset(attrs) | |
| 213 | + |> Repo.update() | |
| 214 | + end | |
| 215 | + | |
| 216 | + @doc """ | |
| 217 | + Soft-delete a comment. The row stays so the issue's history can | |
| 218 | + keep rendering the edits that happened before it went. | |
| 219 | + """ | |
| 220 | + def delete_comment(%IssueComment{} = comment) do | |
| 221 | + comment | |
| 222 | + |> IssueComment.delete_changeset() | |
| 223 | + |> Repo.update() | |
| 224 | + end | |
| 225 | + | |
| 209 | 226 | def target_type, do: @target_type |
| 210 | 227 | end |
modified
lib/git_gud/issues/issue_comment.ex
+24
−0
@@ -12,6 +12,11 @@ defmodule GitGud.Issues.IssueComment do
| 12 | 12 | field :moderation_note, :string |
| 13 | 13 | field :original_body, :string |
| 14 | 14 | |
| 15 | + field :edited_at, :utc_datetime | |
| 16 | + # Soft delete: the history timeline still has to render this | |
| 17 | + # comment's edits after it's gone. | |
| 18 | + field :deleted_at, :utc_datetime | |
| 19 | + | |
| 15 | 20 | belongs_to :issue, Issue |
| 16 | 21 | belongs_to :author, User |
| 17 | 22 | belongs_to :moderated_by, User |
@@ -26,6 +31,25 @@ defmodule GitGud.Issues.IssueComment do
| 26 | 31 | |> validate_length(:body, min: 1, max: 65_535) |
| 27 | 32 | end |
| 28 | 33 | |
| 34 | + @doc "Body edit by the author or a repo admin. Stamps `edited_at`." | |
| 35 | + def edit_changeset(comment, attrs) do | |
| 36 | + comment | |
| 37 | + |> cast(attrs, [:body]) | |
| 38 | + |> validate_required([:body]) | |
| 39 | + |> validate_length(:body, min: 1, max: 65_535) | |
| 40 | + |> put_change(:edited_at, DateTime.utc_now(:second)) | |
| 41 | + end | |
| 42 | + | |
| 43 | + def delete_changeset(comment) do | |
| 44 | + change(comment, %{deleted_at: DateTime.utc_now(:second)}) | |
| 45 | + end | |
| 46 | + | |
| 47 | + def deleted?(%__MODULE__{deleted_at: nil}), do: false | |
| 48 | + def deleted?(%__MODULE__{}), do: true | |
| 49 | + | |
| 50 | + def edited?(%__MODULE__{edited_at: nil}), do: false | |
| 51 | + def edited?(%__MODULE__{}), do: true | |
| 52 | + | |
| 29 | 53 | def moderate_changeset(comment, moderator, note) do |
| 30 | 54 | original = comment.original_body || comment.body |
| 31 | 55 |
modified
lib/git_gud/pull_requests.ex
+17
−0
@@ -705,5 +705,22 @@ defmodule GitGud.PullRequests do
| 705 | 705 | def attach_label(%PullRequest{id: id}, label), do: Labels.attach(label, {@target_type, id}) |
| 706 | 706 | def detach_label(%PullRequest{id: id}, label), do: Labels.detach(label, {@target_type, id}) |
| 707 | 707 | |
| 708 | + @doc "Edit a comment's body, stamping `edited_at`." | |
| 709 | + def edit_comment(%PrComment{} = comment, attrs) do | |
| 710 | + comment | |
| 711 | + |> PrComment.edit_changeset(attrs) | |
| 712 | + |> Repo.update() | |
| 713 | + end | |
| 714 | + | |
| 715 | + @doc """ | |
| 716 | + Soft-delete a comment. The row stays so the pull request's history can | |
| 717 | + keep rendering the edits that happened before it went. | |
| 718 | + """ | |
| 719 | + def delete_comment(%PrComment{} = comment) do | |
| 720 | + comment | |
| 721 | + |> PrComment.delete_changeset() | |
| 722 | + |> Repo.update() | |
| 723 | + end | |
| 724 | + | |
| 708 | 725 | def target_type, do: @target_type |
| 709 | 726 | end |
modified
lib/git_gud/pull_requests/pr_comment.ex
+24
−0
@@ -19,6 +19,11 @@ defmodule GitGud.PullRequests.PrComment do
| 19 | 19 | field :moderation_note, :string |
| 20 | 20 | field :original_body, :string |
| 21 | 21 | |
| 22 | + field :edited_at, :utc_datetime | |
| 23 | + # Soft delete: the history timeline still has to render this | |
| 24 | + # comment's edits after it's gone. | |
| 25 | + field :deleted_at, :utc_datetime | |
| 26 | + | |
| 22 | 27 | belongs_to :pull_request, PullRequest |
| 23 | 28 | belongs_to :author, User |
| 24 | 29 | belongs_to :source_actor, Actor |
@@ -46,6 +51,25 @@ defmodule GitGud.PullRequests.PrComment do
| 46 | 51 | def federated?(%__MODULE__{source_actor_id: sid}) when is_integer(sid), do: true |
| 47 | 52 | def federated?(_), do: false |
| 48 | 53 | |
| 54 | + @doc "Body edit by the author or a repo admin. Stamps `edited_at`." | |
| 55 | + def edit_changeset(comment, attrs) do | |
| 56 | + comment | |
| 57 | + |> cast(attrs, [:body]) | |
| 58 | + |> validate_required([:body]) | |
| 59 | + |> validate_length(:body, min: 1, max: 65_535) | |
| 60 | + |> put_change(:edited_at, DateTime.utc_now(:second)) | |
| 61 | + end | |
| 62 | + | |
| 63 | + def delete_changeset(comment) do | |
| 64 | + change(comment, %{deleted_at: DateTime.utc_now(:second)}) | |
| 65 | + end | |
| 66 | + | |
| 67 | + def deleted?(%__MODULE__{deleted_at: nil}), do: false | |
| 68 | + def deleted?(%__MODULE__{}), do: true | |
| 69 | + | |
| 70 | + def edited?(%__MODULE__{edited_at: nil}), do: false | |
| 71 | + def edited?(%__MODULE__{}), do: true | |
| 72 | + | |
| 49 | 73 | def moderate_changeset(comment, moderator, note) do |
| 50 | 74 | original = comment.original_body || comment.body |
| 51 | 75 |
modified
lib/git_gud_web/components/event_components.ex
+57
−0
@@ -87,11 +87,64 @@ defmodule GitGudWeb.EventComponents do
| 87 | 87 | <p class="text-xs opacity-50 mt-0.5" title={exact_time(event.inserted_at)}> |
| 88 | 88 | {format_dt(event.inserted_at)} |
| 89 | 89 | </p> |
| 90 | + <.body_diff event={event} /> | |
| 90 | 91 | </li> |
| 91 | 92 | </ol> |
| 92 | 93 | """ |
| 93 | 94 | end |
| 94 | 95 | |
| 96 | + # The before/after of a comment edit, folded away by default. Kept | |
| 97 | + # out of the `<p>` above because `<details>` can't live inside one. | |
| 98 | + # | |
| 99 | + # This renders from the event payload, not from the comment row, so | |
| 100 | + # it still works after the comment is deleted. | |
| 101 | + attr :event, :map, required: true | |
| 102 | + | |
| 103 | + defp body_diff(assigns) do | |
| 104 | + ~H""" | |
| 105 | + <details | |
| 106 | + :if={diffable?(@event)} | |
| 107 | + id={"event-#{@event.id}-diff"} | |
| 108 | + class="mt-1 text-xs" | |
| 109 | + > | |
| 110 | + <summary class="cursor-pointer opacity-60 hover:opacity-100 select-none"> | |
| 111 | + Show what changed | |
| 112 | + </summary> | |
| 113 | + <div class="mt-1 rounded border border-base-300 overflow-x-auto"> | |
| 114 | + <div | |
| 115 | + :for={{marker, line} <- diff_lines(@event)} | |
| 116 | + class={["font-mono whitespace-pre px-2 py-0.5", diff_class(marker)]} | |
| 117 | + > | |
| 118 | + {marker}{line} | |
| 119 | + </div> | |
| 120 | + </div> | |
| 121 | + </details> | |
| 122 | + """ | |
| 123 | + end | |
| 124 | + | |
| 125 | + defp diffable?(%{kind: "comment_edited", data: %{"from" => f, "to" => t}}) | |
| 126 | + when is_binary(f) and is_binary(t), | |
| 127 | + do: true | |
| 128 | + | |
| 129 | + defp diffable?(_), do: false | |
| 130 | + | |
| 131 | + # `List.myers_difference/2` over lines gives the same shape a unified | |
| 132 | + # diff wants: runs of equal, deleted and inserted lines in order. | |
| 133 | + defp diff_lines(%{data: %{"from" => from, "to" => to}}) do | |
| 134 | + List.myers_difference(String.split(from, "\n"), String.split(to, "\n")) | |
| 135 | + |> Enum.flat_map(fn | |
| 136 | + {:eq, lines} -> Enum.map(lines, &{" ", &1}) | |
| 137 | + {:del, lines} -> Enum.map(lines, &{"-", &1}) | |
| 138 | + {:ins, lines} -> Enum.map(lines, &{"+", &1}) | |
| 139 | + end) | |
| 140 | + end | |
| 141 | + | |
| 142 | + defp diff_class("+"), do: "bg-success/15 text-success-content" | |
| 143 | + defp diff_class("-"), do: "bg-error/15 text-error-content" | |
| 144 | + defp diff_class(_), do: "opacity-70" | |
| 145 | + | |
| 146 | + defp dot_class(k) when k in ["comment_added", "comment_edited"], do: "bg-info" | |
| 147 | + defp dot_class("comment_deleted"), do: "bg-error" | |
| 95 | 148 | defp dot_class("merged"), do: "bg-secondary" |
| 96 | 149 | defp dot_class("closed"), do: "bg-error" |
| 97 | 150 | defp dot_class("reopened"), do: "bg-success" |
@@ -148,6 +201,10 @@ defmodule GitGudWeb.EventComponents do
| 148 | 201 | defp describe(%{kind: "labeled"}), do: "added a label." |
| 149 | 202 | defp describe(%{kind: "unlabeled"}), do: "removed a label." |
| 150 | 203 | |
| 204 | + defp describe(%{kind: "comment_added"}), do: "commented." | |
| 205 | + defp describe(%{kind: "comment_edited"}), do: "edited a comment." | |
| 206 | + defp describe(%{kind: "comment_deleted"}), do: "deleted a comment." | |
| 207 | + | |
| 151 | 208 | defp describe(%{kind: "moderated", data: %{"note" => note}}) |
| 152 | 209 | when is_binary(note) and note != "", |
| 153 | 210 | do: "moderated this: #{note}" |
modified
lib/git_gud_web/live/issue_live/show.ex
+144
−9
@@ -28,6 +28,7 @@ defmodule GitGudWeb.IssueLive.Show do
| 28 | 28 | |> assign_comment_form() |
| 29 | 29 | |> assign(:comment_body, "") |
| 30 | 30 | |> assign(:editing_title?, false) |
| 31 | + |> assign(:editing_comment_id, nil) | |
| 31 | 32 | |> assign(:title_error, nil) |
| 32 | 33 | |> assign(:event_count, Events.count_for(issue)) |
| 33 | 34 | |> assign(:page_title, "##{issue.number} #{issue.title}")} |
@@ -48,6 +49,19 @@ defmodule GitGudWeb.IssueLive.Show do
| 48 | 49 | |
| 49 | 50 | defp can_edit_title?(_assigns), do: false |
| 50 | 51 | |
| 52 | + # Same rule as the title: the comment's author, or a repo admin. A | |
| 53 | + # deleted comment is nobody's to edit. | |
| 54 | + defp can_edit_comment?(%{current_scope: %{user: %{id: uid}}} = assigns, comment) do | |
| 55 | + not IssueComment.deleted?(comment) and | |
| 56 | + (assigns.can_admin? or comment.author_id == uid) | |
| 57 | + end | |
| 58 | + | |
| 59 | + defp can_edit_comment?(_assigns, _comment), do: false | |
| 60 | + | |
| 61 | + defp find_comment(socket, id) do | |
| 62 | + Enum.find(socket.assigns.issue.comments, &(&1.id == String.to_integer(id))) | |
| 63 | + end | |
| 64 | + | |
| 51 | 65 | @impl true |
| 52 | 66 | def handle_event("toggle_state", _params, socket) do |
| 53 | 67 | issue = socket.assigns.issue |
@@ -94,6 +108,7 @@ defmodule GitGudWeb.IssueLive.Show do
| 94 | 108 | {:noreply, |
| 95 | 109 | socket |
| 96 | 110 | |> assign(:editing_title?, false) |
| 111 | + |> assign(:editing_comment_id, nil) | |
| 97 | 112 | |> assign(:title_error, nil) |
| 98 | 113 | |> assign(:event_count, Events.count_for(issue)) |
| 99 | 114 | |> reload()} |
@@ -119,6 +134,75 @@ defmodule GitGudWeb.IssueLive.Show do
| 119 | 134 | {:noreply, socket |> assign(:event_count, Events.count_for(issue)) |> reload()} |
| 120 | 135 | end |
| 121 | 136 | |
| 137 | + def handle_event("edit_comment", %{"id" => id}, socket) do | |
| 138 | + case find_comment(socket, id) do | |
| 139 | + nil -> | |
| 140 | + {:noreply, socket} | |
| 141 | + | |
| 142 | + comment -> | |
| 143 | + if can_edit_comment?(socket.assigns, comment), | |
| 144 | + do: {:noreply, assign(socket, :editing_comment_id, comment.id)}, | |
| 145 | + else: {:noreply, socket} | |
| 146 | + end | |
| 147 | + end | |
| 148 | + | |
| 149 | + def handle_event("cancel_edit_comment", _params, socket) do | |
| 150 | + {:noreply, assign(socket, :editing_comment_id, nil)} | |
| 151 | + end | |
| 152 | + | |
| 153 | + def handle_event("save_comment", %{"comment_id" => id, "body" => body}, socket) do | |
| 154 | + comment = find_comment(socket, id) | |
| 155 | + | |
| 156 | + cond do | |
| 157 | + is_nil(comment) or not can_edit_comment?(socket.assigns, comment) -> | |
| 158 | + {:noreply, socket} | |
| 159 | + | |
| 160 | + String.trim(body) == comment.body -> | |
| 161 | + {:noreply, assign(socket, :editing_comment_id, nil)} | |
| 162 | + | |
| 163 | + true -> | |
| 164 | + case Issues.edit_comment(comment, %{"body" => String.trim(body)}) do | |
| 165 | + {:ok, updated} -> | |
| 166 | + :ok = | |
| 167 | + Events.record(socket.assigns.issue, "comment_edited", viewer(socket), %{ | |
| 168 | + comment_id: comment.id, | |
| 169 | + from: comment.body, | |
| 170 | + to: updated.body | |
| 171 | + }) | |
| 172 | + | |
| 173 | + {:noreply, | |
| 174 | + socket | |
| 175 | + |> assign(:editing_comment_id, nil) | |
| 176 | + |> assign(:event_count, Events.count_for(socket.assigns.issue)) | |
| 177 | + |> reload()} | |
| 178 | + | |
| 179 | + {:error, _cs} -> | |
| 180 | + {:noreply, put_flash(socket, :error, "Could not save that comment.")} | |
| 181 | + end | |
| 182 | + end | |
| 183 | + end | |
| 184 | + | |
| 185 | + def handle_event("delete_comment", %{"id" => id}, socket) do | |
| 186 | + comment = find_comment(socket, id) | |
| 187 | + | |
| 188 | + if comment && can_edit_comment?(socket.assigns, comment) do | |
| 189 | + {:ok, _} = Issues.delete_comment(comment) | |
| 190 | + | |
| 191 | + :ok = | |
| 192 | + Events.record(socket.assigns.issue, "comment_deleted", viewer(socket), %{ | |
| 193 | + comment_id: comment.id | |
| 194 | + }) | |
| 195 | + | |
| 196 | + {:noreply, | |
| 197 | + socket | |
| 198 | + |> assign(:editing_comment_id, nil) | |
| 199 | + |> assign(:event_count, Events.count_for(socket.assigns.issue)) | |
| 200 | + |> reload()} | |
| 201 | + else | |
| 202 | + {:noreply, socket} | |
| 203 | + end | |
| 204 | + end | |
| 205 | + | |
| 122 | 206 | def handle_event("update_comment_body", %{"issue_comment" => %{"body" => body}}, socket) do |
| 123 | 207 | {:noreply, assign(socket, :comment_body, body)} |
| 124 | 208 | end |
@@ -140,8 +224,16 @@ defmodule GitGudWeb.IssueLive.Show do
| 140 | 224 | user = socket.assigns.current_scope.user |
| 141 | 225 | |
| 142 | 226 | case Issues.add_comment(socket.assigns.issue, user, attrs) do |
| 143 | − {:ok, _} -> | |
| 144 | − {:noreply, socket |> assign_comment_form() |> assign(:comment_body, "") |> reload()} | |
| 227 | + {:ok, comment} -> | |
| 228 | + :ok = | |
| 229 | + Events.record(socket.assigns.issue, "comment_added", user, %{comment_id: comment.id}) | |
| 230 | + | |
| 231 | + {:noreply, | |
| 232 | + socket | |
| 233 | + |> assign_comment_form() | |
| 234 | + |> assign(:comment_body, "") | |
| 235 | + |> assign(:event_count, Events.count_for(socket.assigns.issue)) | |
| 236 | + |> reload()} | |
| 145 | 237 | |
| 146 | 238 | {:error, cs} -> |
| 147 | 239 | {:noreply, assign(socket, :comment_form, to_form(cs))} |
@@ -257,8 +349,30 @@ defmodule GitGudWeb.IssueLive.Show do
| 257 | 349 | <p class="text-xs opacity-60 mb-1 flex items-center gap-1.5"> |
| 258 | 350 | <.avatar name={author_name(c.author)} size="xs" alt={author_name(c.author)} /> |
| 259 | 351 | {author_name(c.author)} · {format_dt(c.inserted_at)} |
| 352 | + <span :if={GitGud.Issues.IssueComment.edited?(c)} class="italic">· edited</span> | |
| 260 | 353 | </p> |
| 261 | 354 | <div class="flex gap-1"> |
| 355 | + <button | |
| 356 | + :if={can_edit_comment?(assigns, c)} | |
| 357 | + type="button" | |
| 358 | + phx-click="edit_comment" | |
| 359 | + phx-value-id={c.id} | |
| 360 | + class="btn btn-xs btn-ghost opacity-60 hover:opacity-100" | |
| 361 | + title="Edit comment" | |
| 362 | + > | |
| 363 | + <.icon name="hero-pencil-square" class="size-3" /> Edit | |
| 364 | + </button> | |
| 365 | + <button | |
| 366 | + :if={can_edit_comment?(assigns, c)} | |
| 367 | + type="button" | |
| 368 | + phx-click="delete_comment" | |
| 369 | + phx-value-id={c.id} | |
| 370 | + class="btn btn-xs btn-ghost opacity-60 hover:opacity-100" | |
| 371 | + data-confirm="Delete this comment? Its edit history stays on the history page." | |
| 372 | + title="Delete comment" | |
| 373 | + > | |
| 374 | + <.icon name="hero-trash" class="size-3" /> | |
| 375 | + </button> | |
| 262 | 376 | <button |
| 263 | 377 | :if={@current_scope && @current_scope.user} |
| 264 | 378 | type="button" |
@@ -287,13 +401,34 @@ defmodule GitGudWeb.IssueLive.Show do
| 287 | 401 | </button> |
| 288 | 402 | </div> |
| 289 | 403 | </div> |
| 290 | − <GitGudWeb.ModerationComponents.moderated_body | |
| 291 | − target={c} | |
| 292 | − viewer={@current_scope && @current_scope.user} | |
| 293 | − repo={@repo} | |
| 294 | − theme={@editor_theme} | |
| 295 | − empty_text="(empty comment)" | |
| 296 | − /> | |
| 404 | + <%= if @editing_comment_id == c.id do %> | |
| 405 | + <form phx-submit="save_comment" class="space-y-2"> | |
| 406 | + <input type="hidden" name="comment_id" value={c.id} /> | |
| 407 | + <textarea | |
| 408 | + name="body" | |
| 409 | + rows="6" | |
| 410 | + class="textarea textarea-bordered w-full font-mono text-sm" | |
| 411 | + >{c.body}</textarea> | |
| 412 | + <div class="flex justify-end gap-2"> | |
| 413 | + <button type="button" phx-click="cancel_edit_comment" class="btn btn-xs btn-ghost"> | |
| 414 | + Cancel | |
| 415 | + </button> | |
| 416 | + <button type="submit" class="btn btn-xs btn-primary">Save</button> | |
| 417 | + </div> | |
| 418 | + </form> | |
| 419 | + <% else %> | |
| 420 | + <p :if={GitGud.Issues.IssueComment.deleted?(c)} class="text-sm italic opacity-50"> | |
| 421 | + (comment deleted) | |
| 422 | + </p> | |
| 423 | + <GitGudWeb.ModerationComponents.moderated_body | |
| 424 | + :if={not GitGud.Issues.IssueComment.deleted?(c)} | |
| 425 | + target={c} | |
| 426 | + viewer={@current_scope && @current_scope.user} | |
| 427 | + repo={@repo} | |
| 428 | + theme={@editor_theme} | |
| 429 | + empty_text="(empty comment)" | |
| 430 | + /> | |
| 431 | + <% end %> | |
| 297 | 432 | </li> |
| 298 | 433 | </ul> |
| 299 | 434 |
modified
lib/git_gud_web/live/pr_live/show.ex
+143
−9
@@ -59,6 +59,7 @@ defmodule GitGudWeb.PrLive.Show do
| 59 | 59 | |> assign_review_form() |
| 60 | 60 | |> assign(:comment_body, "") |
| 61 | 61 | |> assign(:editing_title?, false) |
| 62 | + |> assign(:editing_comment_id, nil) | |
| 62 | 63 | |> assign(:title_error, nil) |
| 63 | 64 | |> assign(:event_count, Events.count_for(pr)) |
| 64 | 65 | |> assign(:page_title, "##{pr.number} #{pr.title}")} |
@@ -73,6 +74,19 @@ defmodule GitGudWeb.PrLive.Show do
| 73 | 74 | |
| 74 | 75 | defp can_edit_title?(_assigns), do: false |
| 75 | 76 | |
| 77 | + # Same rule as the title: the comment's author, or a repo admin. A | |
| 78 | + # deleted comment is nobody's to edit. | |
| 79 | + defp can_edit_comment?(%{current_scope: %{user: %{id: uid}}} = assigns, comment) do | |
| 80 | + not PrComment.deleted?(comment) and | |
| 81 | + (assigns.can_admin? or comment.author_id == uid) | |
| 82 | + end | |
| 83 | + | |
| 84 | + defp can_edit_comment?(_assigns, _comment), do: false | |
| 85 | + | |
| 86 | + defp find_comment(socket, id) do | |
| 87 | + Enum.find(socket.assigns.pr.comments, &(&1.id == String.to_integer(id))) | |
| 88 | + end | |
| 89 | + | |
| 76 | 90 | defp maybe_probe(pr) do |
| 77 | 91 | if pr.state == "open" do |
| 78 | 92 | PullRequests.check_mergeability(pr) |
@@ -171,6 +185,7 @@ defmodule GitGudWeb.PrLive.Show do
| 171 | 185 | {:noreply, |
| 172 | 186 | socket |
| 173 | 187 | |> assign(:editing_title?, false) |
| 188 | + |> assign(:editing_comment_id, nil) | |
| 174 | 189 | |> assign(:title_error, nil) |
| 175 | 190 | |> assign(:event_count, Events.count_for(pr)) |
| 176 | 191 | |> reload()} |
@@ -196,6 +211,75 @@ defmodule GitGudWeb.PrLive.Show do
| 196 | 211 | {:noreply, socket |> assign(:event_count, Events.count_for(pr)) |> reload()} |
| 197 | 212 | end |
| 198 | 213 | |
| 214 | + def handle_event("edit_comment", %{"id" => id}, socket) do | |
| 215 | + case find_comment(socket, id) do | |
| 216 | + nil -> | |
| 217 | + {:noreply, socket} | |
| 218 | + | |
| 219 | + comment -> | |
| 220 | + if can_edit_comment?(socket.assigns, comment), | |
| 221 | + do: {:noreply, assign(socket, :editing_comment_id, comment.id)}, | |
| 222 | + else: {:noreply, socket} | |
| 223 | + end | |
| 224 | + end | |
| 225 | + | |
| 226 | + def handle_event("cancel_edit_comment", _params, socket) do | |
| 227 | + {:noreply, assign(socket, :editing_comment_id, nil)} | |
| 228 | + end | |
| 229 | + | |
| 230 | + def handle_event("save_comment", %{"comment_id" => id, "body" => body}, socket) do | |
| 231 | + comment = find_comment(socket, id) | |
| 232 | + | |
| 233 | + cond do | |
| 234 | + is_nil(comment) or not can_edit_comment?(socket.assigns, comment) -> | |
| 235 | + {:noreply, socket} | |
| 236 | + | |
| 237 | + String.trim(body) == comment.body -> | |
| 238 | + {:noreply, assign(socket, :editing_comment_id, nil)} | |
| 239 | + | |
| 240 | + true -> | |
| 241 | + case PullRequests.edit_comment(comment, %{"body" => String.trim(body)}) do | |
| 242 | + {:ok, updated} -> | |
| 243 | + :ok = | |
| 244 | + Events.record(socket.assigns.pr, "comment_edited", viewer(socket), %{ | |
| 245 | + comment_id: comment.id, | |
| 246 | + from: comment.body, | |
| 247 | + to: updated.body | |
| 248 | + }) | |
| 249 | + | |
| 250 | + {:noreply, | |
| 251 | + socket | |
| 252 | + |> assign(:editing_comment_id, nil) | |
| 253 | + |> assign(:event_count, Events.count_for(socket.assigns.pr)) | |
| 254 | + |> reload()} | |
| 255 | + | |
| 256 | + {:error, _cs} -> | |
| 257 | + {:noreply, put_flash(socket, :error, "Could not save that comment.")} | |
| 258 | + end | |
| 259 | + end | |
| 260 | + end | |
| 261 | + | |
| 262 | + def handle_event("delete_comment", %{"id" => id}, socket) do | |
| 263 | + comment = find_comment(socket, id) | |
| 264 | + | |
| 265 | + if comment && can_edit_comment?(socket.assigns, comment) do | |
| 266 | + {:ok, _} = PullRequests.delete_comment(comment) | |
| 267 | + | |
| 268 | + :ok = | |
| 269 | + Events.record(socket.assigns.pr, "comment_deleted", viewer(socket), %{ | |
| 270 | + comment_id: comment.id | |
| 271 | + }) | |
| 272 | + | |
| 273 | + {:noreply, | |
| 274 | + socket | |
| 275 | + |> assign(:editing_comment_id, nil) | |
| 276 | + |> assign(:event_count, Events.count_for(socket.assigns.pr)) | |
| 277 | + |> reload()} | |
| 278 | + else | |
| 279 | + {:noreply, socket} | |
| 280 | + end | |
| 281 | + end | |
| 282 | + | |
| 199 | 283 | def handle_event("update_comment_body", %{"pr_comment" => %{"body" => body}}, socket) do |
| 200 | 284 | {:noreply, assign(socket, :comment_body, body)} |
| 201 | 285 | end |
@@ -216,8 +300,15 @@ defmodule GitGudWeb.PrLive.Show do
| 216 | 300 | user = socket.assigns.current_scope.user |
| 217 | 301 | |
| 218 | 302 | case PullRequests.add_comment(socket.assigns.pr, user, attrs) do |
| 219 | − {:ok, _} -> | |
| 220 | − {:noreply, socket |> assign_comment_form() |> assign(:comment_body, "") |> reload()} | |
| 303 | + {:ok, comment} -> | |
| 304 | + :ok = Events.record(socket.assigns.pr, "comment_added", user, %{comment_id: comment.id}) | |
| 305 | + | |
| 306 | + {:noreply, | |
| 307 | + socket | |
| 308 | + |> assign_comment_form() | |
| 309 | + |> assign(:comment_body, "") | |
| 310 | + |> assign(:event_count, Events.count_for(socket.assigns.pr)) | |
| 311 | + |> reload()} | |
| 221 | 312 | |
| 222 | 313 | {:error, cs} -> |
| 223 | 314 | {:noreply, assign(socket, :comment_form, to_form(cs))} |
@@ -448,11 +539,33 @@ defmodule GitGudWeb.PrLive.Show do
| 448 | 539 | <p class="text-xs opacity-60 mb-1 flex items-center gap-1.5"> |
| 449 | 540 | <.avatar name={comment_author(c)} size="xs" alt={comment_author(c)} /> |
| 450 | 541 | {comment_author(c)} · {format_dt(c.inserted_at)} |
| 542 | + <span :if={PrComment.edited?(c)} class="italic">· edited</span> | |
| 451 | 543 | <span :if={c.source_actor_id} class="badge badge-xs badge-accent ml-1"> |
| 452 | 544 | federated |
| 453 | 545 | </span> |
| 454 | 546 | </p> |
| 455 | 547 | <div class="flex gap-1"> |
| 548 | + <button | |
| 549 | + :if={can_edit_comment?(assigns, c)} | |
| 550 | + type="button" | |
| 551 | + phx-click="edit_comment" | |
| 552 | + phx-value-id={c.id} | |
| 553 | + class="btn btn-xs btn-ghost opacity-60 hover:opacity-100" | |
| 554 | + title="Edit comment" | |
| 555 | + > | |
| 556 | + <.icon name="hero-pencil-square" class="size-3" /> Edit | |
| 557 | + </button> | |
| 558 | + <button | |
| 559 | + :if={can_edit_comment?(assigns, c)} | |
| 560 | + type="button" | |
| 561 | + phx-click="delete_comment" | |
| 562 | + phx-value-id={c.id} | |
| 563 | + class="btn btn-xs btn-ghost opacity-60 hover:opacity-100" | |
| 564 | + data-confirm="Delete this comment? Its edit history stays on the history page." | |
| 565 | + title="Delete comment" | |
| 566 | + > | |
| 567 | + <.icon name="hero-trash" class="size-3" /> | |
| 568 | + </button> | |
| 456 | 569 | <button |
| 457 | 570 | :if={@current_scope && @current_scope.user} |
| 458 | 571 | type="button" |
@@ -481,13 +594,34 @@ defmodule GitGudWeb.PrLive.Show do
| 481 | 594 | </button> |
| 482 | 595 | </div> |
| 483 | 596 | </div> |
| 484 | − <GitGudWeb.ModerationComponents.moderated_body | |
| 485 | − target={c} | |
| 486 | − viewer={@current_scope && @current_scope.user} | |
| 487 | − repo={@repo} | |
| 488 | − theme={@editor_theme} | |
| 489 | − empty_text="(empty comment)" | |
| 490 | − /> | |
| 597 | + <%= if @editing_comment_id == c.id do %> | |
| 598 | + <form phx-submit="save_comment" class="space-y-2"> | |
| 599 | + <input type="hidden" name="comment_id" value={c.id} /> | |
| 600 | + <textarea | |
| 601 | + name="body" | |
| 602 | + rows="6" | |
| 603 | + class="textarea textarea-bordered w-full font-mono text-sm" | |
| 604 | + >{c.body}</textarea> | |
| 605 | + <div class="flex justify-end gap-2"> | |
| 606 | + <button type="button" phx-click="cancel_edit_comment" class="btn btn-xs btn-ghost"> | |
| 607 | + Cancel | |
| 608 | + </button> | |
| 609 | + <button type="submit" class="btn btn-xs btn-primary">Save</button> | |
| 610 | + </div> | |
| 611 | + </form> | |
| 612 | + <% else %> | |
| 613 | + <p :if={PrComment.deleted?(c)} class="text-sm italic opacity-50"> | |
| 614 | + (comment deleted) | |
| 615 | + </p> | |
| 616 | + <GitGudWeb.ModerationComponents.moderated_body | |
| 617 | + :if={not PrComment.deleted?(c)} | |
| 618 | + target={c} | |
| 619 | + viewer={@current_scope && @current_scope.user} | |
| 620 | + repo={@repo} | |
| 621 | + theme={@editor_theme} | |
| 622 | + empty_text="(empty comment)" | |
| 623 | + /> | |
| 624 | + <% end %> | |
| 491 | 625 | </li> |
| 492 | 626 | </ul> |
| 493 | 627 |
added
priv/repo/migrations/20260909050000_add_comment_edit_tracking.exs
+18
−0
@@ -0,0 +1,18 @@
| 1 | +defmodule GitGud.Repo.Migrations.AddCommentEditTracking do | |
| 2 | + use Ecto.Migration | |
| 3 | + | |
| 4 | + # Comments become editable and soft-deletable. | |
| 5 | + # | |
| 6 | + # Deletion is soft on purpose: the history timeline interleaves a | |
| 7 | + # comment's edits with everything else that happened, and those | |
| 8 | + # events have to keep rendering after the comment is gone. A hard | |
| 9 | + # delete would leave the timeline pointing at nothing. | |
| 10 | + def change do | |
| 11 | + for table <- [:issue_comments, :pr_comments] do | |
| 12 | + alter table(table) do | |
| 13 | + add :edited_at, :utc_datetime | |
| 14 | + add :deleted_at, :utc_datetime | |
| 15 | + end | |
| 16 | + end | |
| 17 | + end | |
| 18 | +end |
added
test/git_gud_web/live/issue_live/comment_history_test.exs
+251
−0
@@ -0,0 +1,251 @@
| 1 | +defmodule GitGudWeb.IssueLive.CommentHistoryTest do | |
| 2 | + @moduledoc """ | |
| 3 | + Editing and deleting comments, and the way those land in the same | |
| 4 | + timeline as everything else on the history page. | |
| 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.Events | |
| 14 | + alias GitGud.Issues | |
| 15 | + alias GitGud.Repositories | |
| 16 | + | |
| 17 | + defp issue_path(repo, issue) do | |
| 18 | + handle = Repositories.Storage.repo_handle(GitGud.Repo.preload(repo, [:owner, :organization])) | |
| 19 | + ~p"/r/#{handle}/#{repo.name}/issues/#{issue.number}" | |
| 20 | + end | |
| 21 | + | |
| 22 | + defp comment!(issue, user, body) do | |
| 23 | + {:ok, comment} = Issues.add_comment(issue, user, %{"body" => body}) | |
| 24 | + comment | |
| 25 | + end | |
| 26 | + | |
| 27 | + describe "editing" do | |
| 28 | + test "the author can edit their own comment", %{conn: conn} do | |
| 29 | + {user, repo} = repository_fixture() | |
| 30 | + issue = issue_fixture(repo, user) | |
| 31 | + comment!(issue, user, "First draft") | |
| 32 | + | |
| 33 | + {:ok, lv, _html} = live(log_in_user(conn, user), issue_path(repo, issue)) | |
| 34 | + | |
| 35 | + _ = lv |> element("button[phx-click=edit_comment]") |> render_click() | |
| 36 | + | |
| 37 | + html = | |
| 38 | + lv |> form("form[phx-submit=save_comment]", %{body: "Second draft"}) |> render_submit() | |
| 39 | + | |
| 40 | + assert html =~ "Second draft" | |
| 41 | + refute html =~ "First draft" | |
| 42 | + # The byline picks up an edited marker. | |
| 43 | + assert html =~ "edited" | |
| 44 | + end | |
| 45 | + | |
| 46 | + test "an unrelated user gets no edit or delete affordance", %{conn: conn} do | |
| 47 | + {_owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 48 | + author = user_fixture() | |
| 49 | + issue = issue_fixture(repo, author) | |
| 50 | + comment!(issue, author, "Theirs") | |
| 51 | + | |
| 52 | + outsider = user_fixture() | |
| 53 | + {:ok, lv, _html} = live(log_in_user(conn, outsider), issue_path(repo, issue)) | |
| 54 | + | |
| 55 | + refute has_element?(lv, "button[phx-click=edit_comment]") | |
| 56 | + refute has_element?(lv, "button[phx-click=delete_comment]") | |
| 57 | + end | |
| 58 | + | |
| 59 | + test "an unrelated user can't edit by sending the event anyway", %{conn: conn} do | |
| 60 | + {_owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 61 | + author = user_fixture() | |
| 62 | + issue = issue_fixture(repo, author) | |
| 63 | + comment = comment!(issue, author, "Untouchable") | |
| 64 | + | |
| 65 | + outsider = user_fixture() | |
| 66 | + {:ok, lv, _html} = live(log_in_user(conn, outsider), issue_path(repo, issue)) | |
| 67 | + | |
| 68 | + render_hook(lv, "save_comment", %{ | |
| 69 | + "comment_id" => to_string(comment.id), | |
| 70 | + "body" => "Hijacked" | |
| 71 | + }) | |
| 72 | + | |
| 73 | + assert GitGud.Repo.get!(Issues.IssueComment, comment.id).body == "Untouchable" | |
| 74 | + end | |
| 75 | + | |
| 76 | + test "a repo admin can edit someone else's comment", %{conn: conn} do | |
| 77 | + {owner, repo} = repository_fixture(%{visibility: "public"}) | |
| 78 | + author = user_fixture() | |
| 79 | + issue = issue_fixture(repo, author) | |
| 80 | + comment!(issue, author, "Needs a fix") | |
| 81 | + | |
| 82 | + {:ok, lv, _html} = live(log_in_user(conn, owner), issue_path(repo, issue)) | |
| 83 | + | |
| 84 | + _ = lv |> element("button[phx-click=edit_comment]") |> render_click() | |
| 85 | + | |
| 86 | + html = | |
| 87 | + lv |> form("form[phx-submit=save_comment]", %{body: "Fixed by admin"}) |> render_submit() | |
| 88 | + | |
| 89 | + assert html =~ "Fixed by admin" | |
| 90 | + end | |
| 91 | + | |
| 92 | + test "saving an unchanged body records nothing", %{conn: conn} do | |
| 93 | + {user, repo} = repository_fixture() | |
| 94 | + issue = issue_fixture(repo, user) | |
| 95 | + comment!(issue, user, "Same") | |
| 96 | + | |
| 97 | + {:ok, lv, _html} = live(log_in_user(conn, user), issue_path(repo, issue)) | |
| 98 | + _ = lv |> element("button[phx-click=edit_comment]") |> render_click() | |
| 99 | + _ = lv |> form("form[phx-submit=save_comment]", %{body: "Same"}) |> render_submit() | |
| 100 | + | |
| 101 | + # The fixture posts through the context, which doesn't record; | |
| 102 | + # the point here is that the no-op save adds nothing either. | |
| 103 | + assert Events.list_for(issue) == [] | |
| 104 | + end | |
| 105 | + end | |
| 106 | + | |
| 107 | + describe "deleting" do | |
| 108 | + test "a deleted comment's body is replaced by a marker", %{conn: conn} do | |
| 109 | + {user, repo} = repository_fixture() | |
| 110 | + issue = issue_fixture(repo, user) | |
| 111 | + comment = comment!(issue, user, "Regrettable words") | |
| 112 | + | |
| 113 | + {:ok, lv, _html} = live(log_in_user(conn, user), issue_path(repo, issue)) | |
| 114 | + html = render_hook(lv, "delete_comment", %{"id" => to_string(comment.id)}) | |
| 115 | + | |
| 116 | + assert html =~ "(comment deleted)" | |
| 117 | + refute html =~ "Regrettable words" | |
| 118 | + end | |
| 119 | + | |
| 120 | + test "the row survives so history can still render", %{conn: conn} do | |
| 121 | + {user, repo} = repository_fixture() | |
| 122 | + issue = issue_fixture(repo, user) | |
| 123 | + comment = comment!(issue, user, "Will be deleted") | |
| 124 | + | |
| 125 | + {:ok, lv, _html} = live(log_in_user(conn, user), issue_path(repo, issue)) | |
| 126 | + render_hook(lv, "delete_comment", %{"id" => to_string(comment.id)}) | |
| 127 | + | |
| 128 | + row = GitGud.Repo.get!(Issues.IssueComment, comment.id) | |
| 129 | + assert row.deleted_at | |
| 130 | + end | |
| 131 | + | |
| 132 | + test "an edit made before deletion is still shown afterwards", %{conn: conn} do | |
| 133 | + {user, repo} = repository_fixture() | |
| 134 | + issue = issue_fixture(repo, user) | |
| 135 | + comment = comment!(issue, user, "Version one") | |
| 136 | + | |
| 137 | + {:ok, lv, _html} = live(log_in_user(conn, user), issue_path(repo, issue)) | |
| 138 | + _ = lv |> element("button[phx-click=edit_comment]") |> render_click() | |
| 139 | + _ = lv |> form("form[phx-submit=save_comment]", %{body: "Version two"}) |> render_submit() | |
| 140 | + render_hook(lv, "delete_comment", %{"id" => to_string(comment.id)}) | |
| 141 | + | |
| 142 | + {:ok, _hist, html} = live(log_in_user(conn, user), issue_path(repo, issue) <> "/history") | |
| 143 | + | |
| 144 | + assert html =~ "edited a comment." | |
| 145 | + assert html =~ "deleted a comment." | |
| 146 | + # The diff renders from the event payload, not the (now gone) body. | |
| 147 | + assert html =~ "Version one" | |
| 148 | + assert html =~ "Version two" | |
| 149 | + end | |
| 150 | + | |
| 151 | + test "a deleted comment can't be edited", %{conn: conn} do | |
| 152 | + {user, repo} = repository_fixture() | |
| 153 | + issue = issue_fixture(repo, user) | |
| 154 | + comment = comment!(issue, user, "Gone") | |
| 155 | + | |
| 156 | + {:ok, lv, _html} = live(log_in_user(conn, user), issue_path(repo, issue)) | |
| 157 | + render_hook(lv, "delete_comment", %{"id" => to_string(comment.id)}) | |
| 158 | + | |
| 159 | + refute has_element?(lv, "button[phx-click=edit_comment]") | |
| 160 | + end | |
| 161 | + end | |
| 162 | + | |
| 163 | + describe "the interleaved timeline" do | |
| 164 | + test "comment events sit in order with everything else", %{conn: conn} do | |
| 165 | + {user, repo} = repository_fixture() | |
| 166 | + issue = issue_fixture(repo, user, %{"title" => "Start"}) | |
| 167 | + comment = comment!(issue, user, "A comment") | |
| 168 | + :ok = Events.record(issue, "comment_added", user, %{comment_id: comment.id}) | |
| 169 | + | |
| 170 | + {:ok, lv, _html} = live(log_in_user(conn, user), issue_path(repo, issue)) | |
| 171 | + | |
| 172 | + _ = lv |> element("button[phx-click=edit_title]") |> render_click() | |
| 173 | + _ = lv |> form("form[phx-submit=save_title]", %{title: "Renamed"}) |> render_submit() | |
| 174 | + _ = lv |> element("button[phx-click=edit_comment]") |> render_click() | |
| 175 | + | |
| 176 | + _ = | |
| 177 | + lv |> form("form[phx-submit=save_comment]", %{body: "Edited comment"}) |> render_submit() | |
| 178 | + | |
| 179 | + _ = lv |> element("button[phx-click=toggle_state]") |> render_click() | |
| 180 | + | |
| 181 | + kinds = issue |> Events.list_for() |> Enum.map(& &1.kind) | |
| 182 | + assert kinds == ["comment_added", "title_changed", "comment_edited", "closed"] | |
| 183 | + end | |
| 184 | + | |
| 185 | + test "the edit diff is collapsed by default", %{conn: conn} do | |
| 186 | + {user, repo} = repository_fixture() | |
| 187 | + issue = issue_fixture(repo, user) | |
| 188 | + comment = comment!(issue, user, "before") | |
| 189 | + | |
| 190 | + :ok = | |
| 191 | + Events.record(issue, "comment_edited", user, %{ | |
| 192 | + comment_id: comment.id, | |
| 193 | + from: "before", | |
| 194 | + to: "after" | |
| 195 | + }) | |
| 196 | + | |
| 197 | + {:ok, _hist, html} = live(log_in_user(conn, user), issue_path(repo, issue) <> "/history") | |
| 198 | + | |
| 199 | + assert html =~ "<details" | |
| 200 | + assert html =~ "Show what changed" | |
| 201 | + # `<details>` without `open` is collapsed. | |
| 202 | + refute html =~ "<details open" | |
| 203 | + end | |
| 204 | + | |
| 205 | + test "the diff marks removed and added lines", %{conn: conn} do | |
| 206 | + {user, repo} = repository_fixture() | |
| 207 | + issue = issue_fixture(repo, user) | |
| 208 | + comment = comment!(issue, user, "x") | |
| 209 | + | |
| 210 | + :ok = | |
| 211 | + Events.record(issue, "comment_edited", user, %{ | |
| 212 | + comment_id: comment.id, | |
| 213 | + from: "keep\nremove me", | |
| 214 | + to: "keep\nadd me" | |
| 215 | + }) | |
| 216 | + | |
| 217 | + {:ok, _hist, html} = live(log_in_user(conn, user), issue_path(repo, issue) <> "/history") | |
| 218 | + | |
| 219 | + assert html =~ "remove me" | |
| 220 | + assert html =~ "add me" | |
| 221 | + assert html =~ "keep" | |
| 222 | + end | |
| 223 | + | |
| 224 | + test "events without a diff render no details block", %{conn: conn} do | |
| 225 | + {user, repo} = repository_fixture() | |
| 226 | + issue = issue_fixture(repo, user) | |
| 227 | + :ok = Events.record(issue, "closed", user) | |
| 228 | + | |
| 229 | + {:ok, _hist, html} = live(log_in_user(conn, user), issue_path(repo, issue) <> "/history") | |
| 230 | + | |
| 231 | + assert html =~ "closed this." | |
| 232 | + refute html =~ "Show what changed" | |
| 233 | + end | |
| 234 | + | |
| 235 | + test "posting a comment shows up on the history page", %{conn: conn} do | |
| 236 | + {user, repo} = repository_fixture() | |
| 237 | + issue = issue_fixture(repo, user) | |
| 238 | + | |
| 239 | + {:ok, lv, _html} = live(log_in_user(conn, user), issue_path(repo, issue)) | |
| 240 | + | |
| 241 | + _ = | |
| 242 | + lv | |
| 243 | + |> form("#comment-form", %{"issue_comment" => %{"body" => "Hello there"}}) | |
| 244 | + |> render_submit() | |
| 245 | + | |
| 246 | + {:ok, _hist, html} = live(log_in_user(conn, user), issue_path(repo, issue) <> "/history") | |
| 247 | + | |
| 248 | + assert html =~ "commented." | |
| 249 | + end | |
| 250 | + end | |
| 251 | +end |
Parents: ce54fcf