From 9aa15e9fb2707a93eb913587657d643825b6f976 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 23 Jan 2026 11:28:15 -0500 Subject: [PATCH] Forbid comments on draft cards - Add Card#commentable? method that returns true only for published cards. - CardsController#create ensures that the card is commentable. Co-Authored-By: Claude Opus 4.5 --- app/controllers/cards/comments_controller.rb | 5 +++++ app/models/card/commentable.rb | 4 ++++ app/models/comment.rb | 6 ++++++ test/controllers/cards/comments_controller_test.rb | 10 ++++++++++ test/models/card/commentable_test.rb | 9 +++++++++ test/models/comment_test.rb | 13 +++++++++++++ test/models/concerns/mentions_test.rb | 10 ---------- 7 files changed, 47 insertions(+), 10 deletions(-) diff --git a/app/controllers/cards/comments_controller.rb b/app/controllers/cards/comments_controller.rb index 27c507abe..f2b318813 100644 --- a/app/controllers/cards/comments_controller.rb +++ b/app/controllers/cards/comments_controller.rb @@ -3,6 +3,7 @@ class Cards::CommentsController < ApplicationController before_action :set_comment, only: %i[ show edit update destroy ] before_action :ensure_creatorship, only: %i[ edit update destroy ] + before_action :ensure_card_is_commentable, only: :create def index set_page_and_extract_portion_from @card.comments.chronologically @@ -50,6 +51,10 @@ class Cards::CommentsController < ApplicationController head :forbidden if Current.user != @comment.creator end + def ensure_card_is_commentable + head :forbidden unless @card.commentable? + end + def comment_params params.expect(comment: [ :body, :created_at ]) end diff --git a/app/models/card/commentable.rb b/app/models/card/commentable.rb index 5c07db47e..4296a77a7 100644 --- a/app/models/card/commentable.rb +++ b/app/models/card/commentable.rb @@ -5,6 +5,10 @@ module Card::Commentable has_many :comments, dependent: :destroy end + def commentable? + published? + end + private STORAGE_BATCH_SIZE = 1000 diff --git a/app/models/comment.rb b/app/models/comment.rb index 41e068514..70a36562f 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -8,6 +8,8 @@ class Comment < ApplicationRecord has_rich_text :body + validate :card_is_commentable + scope :chronologically, -> { order created_at: :asc, id: :desc } scope :preloaded, -> { with_rich_text_body.includes(reactions: :reacter) } scope :by_system, -> { joins(:creator).where(creator: { role: :system }) } @@ -22,6 +24,10 @@ class Comment < ApplicationRecord end private + def card_is_commentable + errors.add(:card, "does not allow comments") unless card.commentable? + end + def watch_card_by_creator card.watch_by creator end diff --git a/test/controllers/cards/comments_controller_test.rb b/test/controllers/cards/comments_controller_test.rb index ef6dbf6e2..0103a846f 100644 --- a/test/controllers/cards/comments_controller_test.rb +++ b/test/controllers/cards/comments_controller_test.rb @@ -13,6 +13,16 @@ class Cards::CommentsControllerTest < ActionDispatch::IntegrationTest assert_response :success end + test "create on draft card is forbidden" do + draft_card = boards(:writebook).cards.create!(status: :drafted, creator: users(:kevin)) + + assert_no_difference -> { draft_card.comments.count } do + post card_comments_path(draft_card), params: { comment: { body: "This should be forbidden" } }, as: :json + end + + assert_response :forbidden + end + test "update" do put card_comment_path(cards(:logo), comments(:logo_agreement_kevin)), params: { comment: { body: "I've changed my mind" } }, as: :turbo_stream diff --git a/test/models/card/commentable_test.rb b/test/models/card/commentable_test.rb index f122e3c46..8b3c93f4d 100644 --- a/test/models/card/commentable_test.rb +++ b/test/models/card/commentable_test.rb @@ -1,6 +1,10 @@ require "test_helper" class Card::CommentableTest < ActiveSupport::TestCase + setup do + Current.session = sessions(:david) + end + test "capturing comments" do assert_difference -> { cards(:logo).comments.count }, +1 do cards(:logo).comments.create!(body: "Agreed.") @@ -19,4 +23,9 @@ class Card::CommentableTest < ActiveSupport::TestCase assert cards(:text).watched_by?(users(:kevin)) end + + test "commentable is true for published cards, false for drafts" do + assert cards(:logo).commentable? + assert_not cards(:unfinished_thoughts).commentable? + end end diff --git a/test/models/comment_test.rb b/test/models/comment_test.rb index 13a5e36c3..bd2089f0e 100644 --- a/test/models/comment_test.rb +++ b/test/models/comment_test.rb @@ -5,6 +5,19 @@ class CommentTest < ActiveSupport::TestCase Current.session = sessions(:david) end + test "cannot create comment on a draft card" do + draft_card = cards(:unfinished_thoughts) + + comment = draft_card.comments.build(body: "This should fail") + + assert_not comment.valid? + assert_includes comment.errors[:card], "does not allow comments" + + assert_raises(ActiveRecord::RecordInvalid) do + draft_card.comments.create!(body: "This should raise") + end + end + test "rich text embed variants are processed immediately on attachment" do comment = cards(:logo).comments.create!(body: "Check this out") comment.body.body.attachables # force load diff --git a/test/models/concerns/mentions_test.rb b/test/models/concerns/mentions_test.rb index 48b94e6f1..fca379bf1 100644 --- a/test/models/concerns/mentions_test.rb +++ b/test/models/concerns/mentions_test.rb @@ -70,16 +70,6 @@ class MentionsTest < ActiveSupport::TestCase end end - test "don't create mentions from comments when belonging to unpublished cards" do - perform_enqueued_jobs only: Mention::CreateJob do - card = boards(:writebook).cards.create title: "Cleanup", description: "Some initial content" - - assert_no_difference -> { Mention.count } do - card.comments.create!(body: "Great work on this #{mention_html_for(users(:david))}!") - end - end - end - test "can't mention users that don't have access to the board" do boards(:writebook).update! all_access: false boards(:writebook).accesses.revoke_from(users(:david))