Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# This migration comes from co_plan (originally 20260527000000)
class AddDeletedAtToCoplanComments < ActiveRecord::Migration[8.1]
def change
add_column :coplan_comments, :deleted_at, :datetime

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like this should be in a compound index of some sort?

end
end
1 change: 1 addition & 0 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 35 additions & 0 deletions engine/app/controllers/coplan/api/v1/comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,41 @@ def discard
render json: { thread_id: thread.id, status: thread.status }
end

def destroy
# Scope the lookup to this plan's comments so an ID from another
# plan returns 404 rather than being acted on. (The policy also
# gates on authorship, but scoping here keeps the resource
# boundary explicit and the 404 correct.)
comment = @plan.comments.find_by(id: params[:id])

unless comment
render json: { error: "Comment not found" }, status: :not_found
return
end

policy = CommentPolicy.new(current_user, comment)
unless policy.delete?
render json: { error: "Not authorized" }, status: :forbidden
return
end

Comments::SoftDelete.call(comment: comment, actor: current_user)

thread = comment.comment_thread
if thread.reload.empty?
Broadcaster.remove_to(@plan, target: ActionView::RecordIdentifier.dom_id(thread))
else
Broadcaster.replace_to(
@plan,
target: ActionView::RecordIdentifier.dom_id(comment),
partial: "coplan/comments/comment",
locals: { comment: comment }
)
end

render json: { comment_id: comment.id, deleted_at: comment.deleted_at }
end

def reply
thread = @plan.comment_threads.find_by(id: params[:id])
unless thread
Expand Down
26 changes: 26 additions & 0 deletions engine/app/controllers/coplan/comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,32 @@ def create
end
end

def destroy
comment = @thread.comments.find(params[:id])
policy = CommentPolicy.new(current_user, comment)
unless policy.delete?
redirect_to plan_path(@plan), alert: "Not authorized to delete this comment." and return
end

Comments::SoftDelete.call(comment: comment, actor: current_user)

if @thread.reload.empty?
Broadcaster.remove_to(@plan, target: ActionView::RecordIdentifier.dom_id(@thread))
else
Broadcaster.replace_to(
@plan,
target: ActionView::RecordIdentifier.dom_id(comment),
partial: "coplan/comments/comment",
locals: { comment: comment }
)
end

respond_to do |format|
format.turbo_stream { render turbo_stream: [] }
format.html { redirect_to plan_path(@plan), notice: "Comment deleted." }
end
end

private

def set_plan
Expand Down
2 changes: 1 addition & 1 deletion engine/app/controllers/coplan/plans_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def index

def show
authorize!(@plan, :show?)
@threads = @plan.comment_threads.includes(:comments, :created_by_user).order(:created_at)
@threads = @plan.comment_threads.with_kept_comments.includes(:comments, :created_by_user).order(:created_at)
@references = @plan.references.order(reference_type: :asc, created_at: :desc)
PlanViewer.track(plan: @plan, user: current_user)
end
Expand Down
7 changes: 7 additions & 0 deletions engine/app/helpers/coplan/plan_events_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ def render_event_summary(event)
url = event.before_value.to_s
label = title || url
safe_join(["Removed reference ", content_tag(:span, label, class: "history-split__event-link")])
when "comment_deleted"
preview = event.metadata.is_a?(Hash) ? event.metadata["body_preview"].to_s.presence : nil
if preview
safe_join(["Deleted comment: ", content_tag(:em, preview)])
else
"Deleted comment"
end
else
# Fallback for unknown / future event types — still useful, never blank.
"#{event.event_type}: #{event.before_value || "—"} → #{event.after_value || "—"}"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Controller } from "@hotwired/stimulus"

// Reveals per-viewer actions (currently just Delete) when this comment
// belongs to the signed-in user. Broadcasts render once for all viewers
// with no current_user, so the server emits the affordance for every
// human comment and lets each browser decide whether to show it. The
// server still enforces auth on submit — this is UX, not security.
export default class extends Controller {
static values = { authorId: String, authorType: String }
static targets = ["delete"]

connect() {
const me = document.querySelector("meta[name='coplan-current-user-id']")?.content
const isMine = this.authorTypeValue === "human" &&
!!me &&
this.authorIdValue === me
if (isMine && this.hasDeleteTarget) {
this.deleteTarget.hidden = false
}
}
}
12 changes: 11 additions & 1 deletion engine/app/models/coplan/comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,22 @@ class Comment < ApplicationRecord
after_create_commit :track_comment_created
# Runs on save (not just create) so adding a mention via edit also
# notifies. ProcessMentions uses find_or_create_by to dedupe.
after_save_commit :process_mentions, if: :saved_change_to_body_markdown?
after_save_commit :process_mentions, if: -> { saved_change_to_body_markdown? && !deleted? }

scope :kept, -> { where(deleted_at: nil) }

def agent?
agent_name.present? || author_type.in?(%w[local_agent cloud_persona])
end

def deleted?
deleted_at.present?
end

def soft_delete!
update!(deleted_at: Time.current)
end

# Resolves the comment author to a CoPlan::User instance, or nil for
# author types that don't map to a user (cloud_persona, system).
def author
Expand Down
10 changes: 10 additions & 0 deletions engine/app/models/coplan/comment_thread.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ class CommentThread < ApplicationRecord
scope :current, -> { where(out_of_date: false) }
scope :active, -> { where(status: OPEN_STATUSES, out_of_date: false) }
scope :archived, -> { where("status NOT IN (?) OR out_of_date = ?", OPEN_STATUSES, true) }
# Threads with at least one non-deleted comment. A thread whose only
# comments have all been soft-deleted is effectively gone — hide it
# from the doc view rather than leaving a dangling anchor + popover.
scope :with_kept_comments, -> {
where(id: CoPlan::Comment.kept.select(:comment_thread_id))
}

def empty?
comments.kept.none?
end

# Transforms anchor positions through intervening version edits using OT.
# Threads without positional data (anchor_start/anchor_end/anchor_revision)
Expand Down
1 change: 1 addition & 0 deletions engine/app/models/coplan/plan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class Plan < ApplicationRecord
has_many :plan_collaborators, dependent: :destroy
has_many :collaborators, through: :plan_collaborators, source: :user
has_many :comment_threads, dependent: :destroy
has_many :comments, through: :comment_threads
has_many :edit_sessions, dependent: :destroy
has_one :edit_lease, dependent: :destroy
has_many :plan_tags, dependent: :destroy
Expand Down
1 change: 1 addition & 0 deletions engine/app/models/coplan/plan_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class PlanEvent < ApplicationRecord
tag_removed
reference_added
reference_removed
comment_deleted
].freeze

belongs_to :plan
Expand Down
7 changes: 7 additions & 0 deletions engine/app/policies/coplan/comment_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module CoPlan
class CommentPolicy < ApplicationPolicy
def delete?
record.author_type == "human" && record.author_id == user&.id
end
end
end
40 changes: 40 additions & 0 deletions engine/app/services/coplan/comments/soft_delete.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
module CoPlan
module Comments
# Soft-deletes a comment and records the event in the plan history feed
# in a single transaction. Idempotent — calling on an already-deleted
# comment is a no-op so retries or double-clicks don't write duplicate
# history entries.
class SoftDelete
BODY_PREVIEW_LENGTH = 120

def self.call(**kwargs)
new(**kwargs).call
end

def initialize(comment:, actor:)
@comment = comment
@actor = actor
end

def call
return @comment if @comment.deleted?

ActiveRecord::Base.transaction do
@comment.soft_delete!
Plans::LogEvent.call(
plan: @comment.comment_thread.plan,
actor: @actor,
event_type: "comment_deleted",
metadata: {
comment_id: @comment.id,
thread_id: @comment.comment_thread_id,
body_preview: @comment.body_markdown.to_s.truncate(BODY_PREVIEW_LENGTH)
}
)
end

@comment
end
end
end
end
1 change: 1 addition & 0 deletions engine/app/services/coplan/plans/log_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def default_field_for(event_type)
when "plan_type_changed" then "plan_type"
when "tag_added", "tag_removed" then "tags"
when "reference_added", "reference_removed" then "references"
when "comment_deleted" then "comments"
end
end

Expand Down
55 changes: 39 additions & 16 deletions engine/app/views/coplan/comments/_comment.html.erb
Original file line number Diff line number Diff line change
@@ -1,18 +1,41 @@
<div class="comment" id="<%= dom_id(comment) %>">
<div class="comment__header text-sm text-muted">
<% author = comment_author_user(comment) %>
<% if author %>
<%= user_avatar(author) %>
<%# Per-viewer affordances (Delete) are hidden client-side by the
comment_actions Stimulus controller — broadcasts render once with no
`current_user`, so we ship the button on every human comment and let
the browser remove it for non-authors. Server still enforces auth. %>
<div class="comment <%= 'comment--deleted' if comment.deleted? %>"
id="<%= dom_id(comment) %>"
data-controller="coplan--comment-actions"
data-coplan--comment-actions-author-id-value="<%= comment.author_id %>"
data-coplan--comment-actions-author-type-value="<%= comment.author_type %>">
<% if comment.deleted? %>
<div class="comment__body text-sm text-muted">
<em>Comment deleted</em>
</div>
<% else %>
<div class="comment__header text-sm text-muted">
<% author = comment_author_user(comment) %>
<% if author %>
<%= user_avatar(author) %>
<% end %>
<strong><%= comment_author_name(comment) %></strong>
<% if comment.agent? %>
<span class="badge badge--agent">agent</span>
<% end %>
· <%= time_ago_in_words(comment.created_at) %> ago
</div>
<div class="comment__body">
<%= cache(comment) do %>
<%= render_markdown(comment.body_markdown) %>
<% end %>
</div>
<% if comment.author_type == "human" %>
<div class="comment__actions" data-coplan--comment-actions-target="delete" hidden>
<%= button_to "Delete",
plan_comment_thread_comment_path(comment.comment_thread.plan, comment.comment_thread, comment),
method: :delete,
form: { data: { turbo_confirm: "Delete this comment?" } },
class: "btn btn--secondary btn--sm" %>
</div>
<% end %>
<strong><%= comment_author_name(comment) %></strong>
<% if comment.agent? %>
<span class="badge badge--agent">agent</span>
<% end %>
· <%= time_ago_in_words(comment.created_at) %> ago
</div>
<div class="comment__body">
<%= cache(comment) do %>
<%= render_markdown(comment.body_markdown) %>
<% end %>
</div>
<% end %>
</div>
3 changes: 3 additions & 0 deletions engine/app/views/layouts/coplan/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
<meta name="color-scheme" content="<%= signed_in? && current_user.theme_preference != 'system' ? current_user.theme_preference : 'light dark' %>">
<%= csrf_meta_tags %>
<%= csp_meta_tag %>
<% if signed_in? %>
<meta name="coplan-current-user-id" content="<%= current_user.id %>">
<% end %>
<% if CoPlan.configuration.web_push_configured? %>
<meta name="coplan-vapid-public-key" content="<%= CoPlan.configuration.vapid_public_key %>">
<meta name="coplan-service-worker-url" content="<%= coplan.service_worker_path %>">
Expand Down
5 changes: 4 additions & 1 deletion engine/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
patch :discard
patch :reopen
end
resources :comments, only: [:create]
resources :comments, only: [:create, :destroy]
end
end

Expand Down Expand Up @@ -42,6 +42,9 @@
patch :resolve, on: :member
patch :discard, on: :member
end
# Deletes an individual comment (by comment ID, not thread ID).
# Distinct from the routes above, which key off thread ID.
delete "comments/:id/delete", to: "comments#destroy", as: :destroy_comment
resources :references, only: [:index, :create, :destroy]
end
resources :references, only: [] do
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddDeletedAtToCoplanComments < ActiveRecord::Migration[8.1]
def change
add_column :coplan_comments, :deleted_at, :datetime
end
end
24 changes: 24 additions & 0 deletions spec/models/comment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,28 @@
}.not_to have_enqueued_job(CoPlan::NotificationJob)
end
end

describe "soft delete" do
let(:comment) { create(:comment) }

it "soft_delete! sets deleted_at" do
expect { comment.soft_delete! }.to change { comment.reload.deleted_at }.from(nil)
expect(comment.deleted?).to be(true)
end

it "kept scope excludes deleted comments" do
kept = create(:comment)
deleted = create(:comment)
deleted.soft_delete!

expect(CoPlan::Comment.kept).to include(kept)
expect(CoPlan::Comment.kept).not_to include(deleted)
end

it "does not re-fire ProcessMentions when soft-deleting" do
comment # create before block, so ProcessMentions runs once on creation
expect(CoPlan::Comments::ProcessMentions).not_to receive(:call)
comment.soft_delete!
end
end
end
22 changes: 22 additions & 0 deletions spec/models/comment_thread_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,28 @@
expect(thread_record).to be_valid
end

describe "kept comments lifecycle" do
it "with_kept_comments excludes threads whose only comment is soft-deleted" do
with_live = create(:comment_thread, plan: plan, plan_version: plan.current_plan_version, created_by_user: user)
create(:comment, comment_thread: with_live)

all_deleted = create(:comment_thread, plan: plan, plan_version: plan.current_plan_version, created_by_user: user)
c = create(:comment, comment_thread: all_deleted)
c.soft_delete!

result = CoPlan::CommentThread.with_kept_comments
expect(result).to include(with_live)
expect(result).not_to include(all_deleted)
end

it "#empty? is true when all comments are soft-deleted, false otherwise" do
c = create(:comment, comment_thread: thread_record)
expect(thread_record).not_to be_empty
c.soft_delete!
expect(thread_record.reload).to be_empty
end
end

it "validates status inclusion" do
thread_record.status = "invalid"
expect(thread_record).not_to be_valid
Expand Down
Loading
Loading