Merge pull request #445 from basecamp/flatten-messages

Flatten messages

app/controllers/cards/comments/reactions_controller.rb

@@ -25,7 +25,7 @@ class Cards::Comments::ReactionsController < ApplicationController
 
   private
     def set_comment
-      @comment = Comment.belonging_to_card(@card).find(params[:comment_id])
+      @comment = @card.comments.find(params[:comment_id])
     end
 
     def broadcast_create(reaction)

app/controllers/cards/comments_controller.rb

@@ -5,7 +5,7 @@ class Cards::CommentsController < ApplicationController
   before_action :ensure_creatorship, only: %i[ edit update destroy ]
 
   def create
-    @card.capture Comment.new(comment_params)
+    @card.comments.create!(comment_params)
   end
 
   def show
@@ -25,7 +25,7 @@ class Cards::CommentsController < ApplicationController
 
   private
     def set_comment
-      @comment = Comment.belonging_to_card(@card).find(params[:id])
+      @comment = @card.comments.find(params[:id])
     end
 
     def ensure_creatorship

app/helpers/comments_helper.rb

@@ -1,6 +1,6 @@
 module CommentsHelper
   def new_comment_placeholder(card)
-    if card.creator == Current.user && card.messages.comments.empty?
+    if card.creator == Current.user && card.comments.empty?
       "Next, add some notes, context, pictures, or video about this…"
     else
       "Type your comment…"

app/models/card.rb

@@ -1,11 +1,12 @@
 class Card < ApplicationRecord
-  include Assignable, Colored, Engageable, Eventable, Golden, Mentions,
-    Messages, Pinnable, Closeable, Readable, Searchable, Staged,
+  include Assignable, Colored, Engageable, Eventable, Golden,
+    Mentions, Pinnable, Closeable, Readable, Searchable, Staged,
     Statuses, Taggable, Watchable
 
   belongs_to :collection, touch: true
   belongs_to :creator, class_name: "User", default: -> { Current.user }
 
+  has_many :comments, dependent: :destroy
   has_one_attached :image, dependent: :purge_later
 
   has_markdown :description

app/models/card/eventable.rb

@@ -6,13 +6,12 @@ module Card::Eventable
   included do
     before_create { self.last_active_at = Time.current }
 
-    after_save :track_due_date_change, if: :saved_change_to_due_on?
     after_save :track_title_change, if: :saved_change_to_title?
   end
 
   def event_was_created(event)
     transaction do
-      find_or_capture_event_summary.events << event
+      create_system_comment_for(event)
       touch(:last_active_at)
     end
   end
@@ -22,25 +21,13 @@ module Card::Eventable
       published?
     end
 
-    def find_or_capture_event_summary
-      messages.last&.event_summary || capture(EventSummary.new).event_summary
-    end
-
-    def track_due_date_change
-      if due_on.present?
-        if due_on_before_last_save.nil?
-          track_event("card_due_date_added", particulars: { due_date: due_on })
-        else
-          track_event("card_due_date_changed", particulars: { due_date: due_on })
-        end
-      elsif due_on_before_last_save.present?
-        track_event("card_due_date_removed")
-      end
-    end
-
     def track_title_change
       if title_before_last_save.present?
-        track_event "card_title_changed", particulars: { old_title: title_before_last_save, new_title: title }
+        track_event "title_changed", particulars: { old_title: title_before_last_save, new_title: title }
       end
     end
+
+    def create_system_comment_for(event)
+      SystemCommenter.new(self, event).comment
+    end
 end

app/models/card/eventable/system_commenter.rb

@@ -0,0 +1,56 @@
+class Card::Eventable::SystemCommenter
+  attr_reader :card, :event
+
+  def initialize(card, event)
+    @card, @event = card, event
+  end
+
+  def comment
+    return unless comment_body.present?
+    if comment = find_replaceable_system_comment
+      comment.update! body: comment_body
+    else
+      card.comments.create! creator: User.system, body: comment_body, created_at: event.created_at
+    end
+  end
+
+  private
+    RELATED_EVENTS = [
+      %w[ card_assigned card_unassigned ]
+    ]
+
+    def comment_body
+      case event.action
+      when "card_assigned"
+        "Assigned to #{card.assignees.pluck(:name).to_sentence}."
+      when "card_unassigned"
+        if card.assignees.empty?
+          "Unassigned from #{event.assignees.pluck(:name).to_sentence}."
+        else
+          "Assigned to #{card.assignees.pluck(:name).to_sentence}."
+        end
+      when "card_staged"
+        "#{event.creator.name} moved this to '#{event.stage_name}'."
+      when "card_closed"
+        "Closed by #{ event.creator.name }"
+      when "card_title_changed"
+        "#{event.creator.name} changed title from '#{event.particulars.dig('particulars', 'old_title')}' to '#{event.particulars.dig('particulars', 'new_title')}'."
+      end
+    end
+
+    def find_replaceable_system_comment
+      previous_comment, previous_event = card.comments.last, card.events[-2]
+      if previous_comment&.creator&.system? && replaceable_event?(previous_event, event)
+        previous_comment
+      end
+    end
+
+    def replaceable_event?(candidate_event, replacement_event)
+      candidate_event && replacement_event &&
+        (candidate_event.action == replacement_event.action || related_events?(candidate_event, replacement_event))
+    end
+
+    def related_events?(candidate_event, replacement_event)
+      RELATED_EVENTS.find { |related_actions| related_actions.include?(candidate_event.action) && related_actions.include?(replacement_event.action) }
+    end
+end

app/models/card/messages.rb

@@ -1,11 +0,0 @@
-module Card::Messages
-  extend ActiveSupport::Concern
-
-  included do
-    has_many :messages, -> { chronologically }, dependent: :destroy
-  end
-
-  def capture(messageable)
-    messages.create! messageable: messageable
-  end
-end

app/models/card/readable.rb

@@ -31,8 +31,4 @@ module Card::Readable
     def comment_mentions
       Mention.where(source: comments)
     end
-
-    def comments
-      @comments ||= Comment.where(id: messages.comments.pluck(:messageable_id))
-    end
 end

app/models/card/searchable.rb

@@ -11,7 +11,7 @@ module Card::Searchable
         cards = Card.search(query).select(:id).to_sql
         comments = Comment.search(query).select(:id).to_sql
 
-        left_joins(:messages).where("cards.id in (#{cards}) or messages.messageable_id in (#{comments})").distinct
+        left_joins(:comments).where("cards.id in (#{cards}) or comments.id in (#{comments})").distinct
       else
         none
       end

app/models/comment.rb

@@ -1,5 +1,6 @@
 class Comment < ApplicationRecord
-  include Eventable, Mentions, Messageable, Searchable
+  include Eventable, Mentions, Searchable
+  belongs_to :card, touch: true
 
   belongs_to :creator, class_name: "User", default: -> { Current.user }
   has_many :reactions, dependent: :delete_all
@@ -7,12 +8,11 @@ class Comment < ApplicationRecord
   has_markdown :body
   searchable_by :body_plain_text, using: :comments_search_index, as: :body
 
-  # FIXME: Not a fan of this. Think all references to comment should come directly from the message.
-  scope :belonging_to_card, ->(card) { joins(:message).where(messages: { card_id: card.id }) }
+  scope :chronologically, -> { order created_at: :asc, id: :desc }
 
   after_create_commit :watch_card_by_creator
 
-  delegate :watch_by, to: :card
+  delegate :collection, :watch_by, to: :card
 
   def to_partial_path
     "cards/#{super}"

app/models/comment/eventable.rb

@@ -12,6 +12,10 @@ module Comment::Eventable
   end
 
   private
+    def should_track_event?
+      !creator.system?
+    end
+
     def track_creation
       track_event("created", collection: card.collection)
     end

app/models/concerns/messageable.rb

@@ -1,12 +0,0 @@
-module Messageable
-  extend ActiveSupport::Concern
-
-  TYPES = %w[ Comment EventSummary ]
-
-  included do
-    has_one :message, as: :messageable, touch: true, dependent: :destroy
-    has_one :card, through: :message
-
-    delegate :collection, to: :card
-  end
-end

app/models/event.rb

@@ -4,7 +4,6 @@ class Event < ApplicationRecord
   belongs_to :collection
   belongs_to :creator, class_name: "User"
   belongs_to :eventable, polymorphic: true
-  belongs_to :summary, touch: true, class_name: "EventSummary", optional: true
 
   scope :chronologically, -> { order created_at: :asc, id: :desc }
 

app/models/event_summary.rb

@@ -1,36 +0,0 @@
-class EventSummary < ApplicationRecord
-  include Messageable
-
-  has_many :events, -> { chronologically }, dependent: :delete_all, inverse_of: :summary
-
-  # FIXME: Consider persisting the body and compute at write time.
-  def body
-    events.map { |event| summarize(event) }.join(" ")
-  end
-
-  private
-    delegate :time_ago_in_words, to: "ApplicationController.helpers"
-
-    def summarize(event)
-      case event.action
-      when "card_assigned"
-        "Assigned to #{event.assignees.pluck(:name).to_sentence}."
-      when "card_unassigned"
-        "Unassigned from #{event.assignees.pluck(:name).to_sentence}."
-      when "card_staged"
-        "#{event.creator.name} moved this to '#{event.stage_name}'."
-      when "card_closed"
-        "Closed by #{ event.creator.name }"
-      when "card_unstaged"
-        "#{event.creator.name} removed this from '#{event.stage_name}'."
-      when "card_due_date_added"
-        "#{event.creator.name} set due date to #{event.particulars.dig('particulars', 'due_date').to_date.strftime('%B %-d')}."
-      when "card_due_date_changed"
-        "#{event.creator.name} changed due date to #{event.particulars.dig('particulars', 'due_date').to_date.strftime('%B %-d')}."
-      when "card_due_date_removed"
-        "#{event.creator.name} removed the date."
-      when "card_title_changed"
-        "#{event.creator.name} changed title from '#{event.particulars.dig('particulars', 'old_title')}' to '#{event.particulars.dig('particulars', 'new_title')}'."
-      end
-    end
-end

app/models/message.rb

@@ -1,7 +0,0 @@
-class Message < ApplicationRecord
-  belongs_to :card, touch: true
-
-  delegated_type :messageable, types: Messageable::TYPES, inverse_of: :message, dependent: :destroy
-
-  scope :chronologically, -> { order created_at: :asc, id: :desc }
-end

app/views/cards/_comments.html.erb

@@ -1,6 +1,6 @@
-<% if card.messages.comments.any? %>
+<% if card.comments.any? %>
   <div class="card__detail card__comments">
-    <span aria-hidden="true"><%= card.messages.comments.count %></span>
-    <span class="for-screen-reader"><%= pluralize(card.messages.comments.count, "comment") %></span>
+    <span aria-hidden="true"><%= card.comments.count %></span>
+    <span class="for-screen-reader"><%= pluralize(card.comments.count, "comment") %></span>
   </div>
 <% end %>

app/views/cards/_messages.html.erb

@@ -1,5 +1,5 @@
 <%= messages_tag(card) do %>
-  <%= render partial: "cards/messages/message", collection: card.messages, cached: true %>
+  <%= render partial: "cards/comments/comment", collection: card.comments.chronologically, cached: true %>
   <%= render "cards/comments/new", card: card %>
 
   <div class="comments__subscribers flex flex-column margin-block txt-align-start full-width">

app/views/cards/messages/_message.html.erb

@@ -1,5 +0,0 @@
-<% cache message do %>
-  <%# Template Dependency: cards/comments/comment %>
-  <%# Template Dependency: event_summaries/event_summary %>
-  <%= render partial: message.messageable.to_partial_path, object: message.messageable %>
-<% end %>

db/migrate/20250429162506_flatten_messages_and_comments.rb

@@ -0,0 +1,21 @@
+class FlattenMessagesAndComments < ActiveRecord::Migration[8.1]
+  def change
+    add_reference :comments, :card, null: true, foreign_key: true
+
+    execute <<~SQL
+      UPDATE comments
+      SET card_id = (
+        SELECT messages.card_id
+        FROM messages
+        WHERE messages.messageable_type = 'Comment'
+          AND messages.messageable_id = comments.id
+        LIMIT 1
+      )
+    SQL
+
+    remove_column :events, :summary_id
+    change_column_null :comments, :card_id, false
+    drop_table :messages
+    drop_table :event_summaries
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.1].define(version: 2025_04_25_092727) do
+ActiveRecord::Schema[8.1].define(version: 2025_04_29_162506) do
   create_table "accesses", force: :cascade do |t|
     t.integer "collection_id", null: false
     t.datetime "created_at", null: false
@@ -160,9 +160,11 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_25_092727) do
   end
 
   create_table "comments", force: :cascade do |t|
+    t.integer "card_id", null: false
     t.datetime "created_at", null: false
     t.integer "creator_id", null: false
     t.datetime "updated_at", null: false
+    t.index ["card_id"], name: "index_comments_on_card_id"
   end
 
   create_table "creators_filters", id: false, force: :cascade do |t|
@@ -172,11 +174,6 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_25_092727) do
     t.index ["filter_id"], name: "index_creators_filters_on_filter_id"
   end
 
-  create_table "event_summaries", force: :cascade do |t|
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
-  end
-
   create_table "events", force: :cascade do |t|
     t.string "action", null: false
     t.integer "collection_id", null: false
@@ -185,12 +182,11 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_25_092727) do
     t.integer "eventable_id", null: false
     t.string "eventable_type", null: false
     t.json "particulars", default: {}
-    t.integer "summary_id"
     t.datetime "updated_at", null: false
+    t.index ["action"], name: "index_events_on_summary_id_and_action"
     t.index ["collection_id"], name: "index_events_on_collection_id"
     t.index ["creator_id"], name: "index_events_on_creator_id"
     t.index ["eventable_type", "eventable_id"], name: "index_events_on_eventable"
-    t.index ["summary_id", "action"], name: "index_events_on_summary_id_and_action"
   end
 
   create_table "filters", force: :cascade do |t|
@@ -228,16 +224,6 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_25_092727) do
     t.index ["source_type", "source_id"], name: "index_mentions_on_source"
   end
 
-  create_table "messages", force: :cascade do |t|
-    t.integer "card_id", null: false
-    t.datetime "created_at", null: false
-    t.integer "messageable_id", null: false
-    t.string "messageable_type", null: false
-    t.datetime "updated_at", null: false
-    t.index ["card_id"], name: "index_messages_on_card_id"
-    t.index ["messageable_type", "messageable_id"], name: "index_messages_on_messageable", unique: true
-  end
-
   create_table "notifications", force: :cascade do |t|
     t.datetime "created_at", null: false
     t.integer "creator_id"
@@ -294,6 +280,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_25_092727) do
     t.datetime "created_at", null: false
     t.string "title"
     t.datetime "updated_at", null: false
+    t.index ["title"], name: "index_tags_on_account_id_and_title", unique: true
   end
 
   create_table "users", force: :cascade do |t|
@@ -340,11 +327,10 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_25_092727) do
   add_foreign_key "closures", "cards"
   add_foreign_key "closures", "users"
   add_foreign_key "collections", "workflows"
+  add_foreign_key "comments", "cards"
   add_foreign_key "events", "collections"
-  add_foreign_key "events", "event_summaries", column: "summary_id"
   add_foreign_key "mentions", "users", column: "mentionee_id"
   add_foreign_key "mentions", "users", column: "mentioner_id"
-  add_foreign_key "messages", "cards"
   add_foreign_key "notifications", "users"
   add_foreign_key "notifications", "users", column: "creator_id"
   add_foreign_key "pins", "cards"

db/schema_cache.yml

@@ -533,6 +533,7 @@ columns:
   - *22
   - *18
   comments:
+  - *21
   - *5
   - *23
   - *6
@@ -540,10 +541,6 @@ columns:
   creators_filters:
   - *23
   - *18
-  event_summaries:
-  - *5
-  - *6
-  - *9
   events:
   - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
     auto_increment:
@@ -597,16 +594,6 @@ columns:
     default_function:
     collation:
     comment:
-  - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
-    auto_increment:
-    name: summary_id
-    cast_type: *1
-    sql_type_metadata: *2
-    'null': true
-    default:
-    default_function:
-    collation:
-    comment:
   - *9
   filters:
   - *5
@@ -701,31 +688,6 @@ columns:
     collation:
     comment:
   - *9
-  messages:
-  - *21
-  - *5
-  - *6
-  - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
-    auto_increment:
-    name: messageable_id
-    cast_type: *1
-    sql_type_metadata: *2
-    'null': false
-    default:
-    default_function:
-    collation:
-    comment:
-  - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
-    auto_increment:
-    name: messageable_type
-    cast_type: *7
-    sql_type_metadata: *8
-    'null': false
-    default:
-    default_function:
-    collation:
-    comment:
-  - *9
   notifications:
   - *5
   - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
@@ -961,13 +923,11 @@ primary_keys:
   collections_filters:
   comments: id
   creators_filters:
-  event_summaries: id
   events: id
   filters: id
   filters_stages:
   filters_tags:
   mentions: id
-  messages: id
   notifications: id
   pins: id
   reactions: id
@@ -999,13 +959,11 @@ data_sources:
   collections_filters: true
   comments: true
   creators_filters: true
-  event_summaries: true
   events: true
   filters: true
   filters_stages: true
   filters_tags: true
   mentions: true
-  messages: true
   notifications: true
   pins: true
   reactions: true
@@ -1492,7 +1450,23 @@ indexes:
     nulls_not_distinct:
     comment:
     valid: true
-  comments: []
+  comments:
+  - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
+    table: comments
+    name: index_comments_on_card_id
+    unique: false
+    columns:
+    - card_id
+    lengths: {}
+    orders: {}
+    opclasses: {}
+    where:
+    type:
+    using:
+    include:
+    nulls_not_distinct:
+    comment:
+    valid: true
   creators_filters:
   - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
     table: creators_filters
@@ -1526,7 +1500,6 @@ indexes:
     nulls_not_distinct:
     comment:
     valid: true
-  event_summaries: []
   events:
   - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
     table: events
@@ -1582,7 +1555,6 @@ indexes:
     name: index_events_on_summary_id_and_action
     unique: false
     columns:
-    - summary_id
     - action
     lengths: {}
     orders: {}
@@ -1728,40 +1700,6 @@ indexes:
     nulls_not_distinct:
     comment:
     valid: true
-  messages:
-  - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
-    table: messages
-    name: index_messages_on_card_id
-    unique: false
-    columns:
-    - card_id
-    lengths: {}
-    orders: {}
-    opclasses: {}
-    where:
-    type:
-    using:
-    include:
-    nulls_not_distinct:
-    comment:
-    valid: true
-  - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
-    table: messages
-    name: index_messages_on_messageable
-    unique: true
-    columns:
-    - messageable_type
-    - messageable_id
-    lengths: {}
-    orders: {}
-    opclasses: {}
-    where:
-    type:
-    using:
-    include:
-    nulls_not_distinct:
-    comment:
-    valid: true
   notifications:
   - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
     table: notifications
@@ -1967,7 +1905,23 @@ indexes:
     nulls_not_distinct:
     comment:
     valid: true
-  tags: []
+  tags:
+  - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
+    table: tags
+    name: index_tags_on_account_id_and_title
+    unique: true
+    columns:
+    - title
+    lengths: {}
+    orders: {}
+    opclasses: {}
+    where:
+    type:
+    using:
+    include:
+    nulls_not_distinct:
+    comment:
+    valid: true
   users:
   - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
     table: users
@@ -2052,4 +2006,4 @@ indexes:
     comment:
     valid: true
   workflows: []
-version: 20250425092727
+version: 20250429162506

script/generate_comments_from_events.rb

@@ -0,0 +1,11 @@
+#!/usr/bin/env ruby
+
+require_relative "../config/environment"
+
+ApplicationRecord.with_each_tenant do |tenant|
+  Card.find_each do |card|
+    card.events.find_each do |event|
+      Card::Eventable::SystemCommenter.new(card.reload, event).comment
+    end
+  end
+end

test/controllers/cards/comments_controller_test.rb

@@ -6,7 +6,7 @@ class Cards::CommentsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "create" do
-    assert_difference "cards(:logo).messages.comments.count", +1 do
+    assert_difference -> { cards(:logo).comments.count }, +1 do
       post card_comments_path(cards(:logo), params: { comment: { body: "Agreed." } })
     end
 

test/fixtures/comments.yml

@@ -1,10 +1,40 @@
+logo_1:
+  card: logo
+  creator: system
+  created_at: <%= 1.week.ago %>
+
 logo_agreement_jz:
+  card: logo
   creator: jz
   created_at: <%= 2.days.ago %>
 
+logo_3:
+  card: logo
+  creator: system
+  created_at: <%= 1.day.ago %>
+
 logo_agreement_kevin:
+  card: logo
   creator: kevin
   created_at: <%= 2.hours.ago %>
 
+logo_5:
+  card: logo
+  creator: system
+  created_at: <%= 1.hour.ago %>
+
+layout_1:
+  card: layout
+  creator: system
+
 layout_overflowing_david:
+  card: layout
   creator: david
+
+text_1:
+  card: text
+  creator: system
+
+shipping_1:
+  card: shipping
+  creator: system

test/fixtures/event_summaries.yml

@@ -1,17 +0,0 @@
-logo_initial_activity:
-  created_at: <%= 1.week.ago %>
-
-logo_second_activity:
-  created_at: <%= 1.day.ago %>
-
-logo_third_activity:
-  created_at: <%= 1.hour.ago %>
-
-layout_initial_activity:
-  created_at: <%= 1.week.ago %>
-
-text_initial_activity:
-  created_at: <%= 1.week.ago %>
-
-shipping_initial_activity:
-  created_at: <%= 1.week.ago %>

test/fixtures/events.yml

@@ -3,7 +3,6 @@ logo_published:
   collection: writebook
   eventable: logo (Card)
   action: card_published
-  summary: logo_initial_activity
   created_at: <%= 1.week.ago %>
 
 logo_assignment_jz:
@@ -11,7 +10,6 @@ logo_assignment_jz:
   collection: writebook
   eventable: logo (Card)
   action: card_assigned
-  summary: logo_initial_activity
   particulars: <%= { assignee_ids: [ ActiveRecord::FixtureSet.identify(:jz) ] }.to_json %>
   created_at: <%= 1.week.ago + 1.hour %>
 
@@ -20,7 +18,6 @@ logo_assignment_km:
   collection: writebook
   eventable: logo (Card)
   action: card_assigned
-  summary: logo_second_activity
   particulars: <%= { assignee_ids: [ ActiveRecord::FixtureSet.identify(:kevin) ] }.to_json %>
   created_at: <%= 1.day.ago %>
 
@@ -29,7 +26,6 @@ layout_published:
   collection: writebook
   eventable: layout (Card)
   action: card_published
-  summary: layout_initial_activity
   created_at: <%= 1.week.ago %>
 
 layout_commented:
@@ -37,7 +33,6 @@ layout_commented:
   collection: writebook
   eventable: layout_overflowing_david (Comment)
   action: comment_created
-  summary: layout_initial_activity
   created_at: <%= 1.week.ago %>
 
 layout_assignment_jz:
@@ -45,7 +40,6 @@ layout_assignment_jz:
   collection: writebook
   eventable: layout (Card)
   action: card_assigned
-  summary: layout_initial_activity
   particulars: <%= { assignee_ids: [ ActiveRecord::FixtureSet.identify(:jz) ] }.to_json %>
   created_at: <%= 1.hour.ago %>
 
@@ -54,7 +48,6 @@ text_published:
   collection: writebook
   eventable: text (Card)
   action: card_published
-  summary: text_initial_activity
   created_at: <%= 1.week.ago %>
 
 shipping_published:
@@ -62,7 +55,6 @@ shipping_published:
   collection: writebook
   eventable: shipping (Card)
   action: card_published
-  summary: shipping_initial_activity
   created_at: <%= 1.week.ago %>
 
 shipping_closed:
@@ -70,5 +62,4 @@ shipping_closed:
   collection: writebook
   eventable: shipping (Card)
   action: card_closed
-  summary: shipping_initial_activity
   created_at: <%= 2.days.ago %>

test/fixtures/messages.yml

@@ -1,40 +0,0 @@
-logo_1:
-  card: logo
-  messageable: logo_initial_activity (EventSummary)
-  created_at: <%= 1.week.ago %>
-
-logo_2:
-  card: logo
-  messageable: logo_agreement_jz (Comment)
-  created_at: <%= 2.days.ago %>
-
-logo_3:
-  card: logo
-  messageable: logo_second_activity (EventSummary)
-  created_at: <%= 1.day.ago %>
-
-logo_4:
-  card: logo
-  messageable: logo_agreement_kevin (Comment)
-  created_at: <%= 2.hours.ago %>
-
-logo_5:
-  card: logo
-  messageable: logo_third_activity (EventSummary)
-  created_at: <%= 1.hour.ago %>
-
-layout_1:
-  card: layout
-  messageable: layout_initial_activity (EventSummary)
-
-layout_2:
-  card: layout
-  messageable: layout_overflowing_david (Comment)
-
-text_1:
-  card: text
-  messageable: text_initial_activity (EventSummary)
-
-shipping_1:
-  card: shipping
-  messageable: shipping_initial_activity (EventSummary)

test/fixtures/users.yml

@@ -17,3 +17,7 @@ kevin:
   email_address: kevin@37signals.com
   password_digest: <%= digest %>
   role: admin
+
+system:
+  name: System
+  role: system

test/integration/card_messages.rb

@@ -1,29 +0,0 @@
-require "test_helper"
-
-class CardMessagesTest < ActionDispatch::IntegrationTest
-  setup do
-    sign_in_as :kevin
-  end
-
-  test "messages system" do
-    # Create a card
-    post collection_cards_path(collections(:writebook))
-    card = Card.last
-    assert_equal 1, card.messages.count
-    assert_predicate card.messages.last, :event_summary?
-    assert_equal "created", card.messages.last.messageable.events.sole.action
-
-    # Comment on it
-    post collection_card_comments_path(collections(:writebook), card), params: { comment: { body: "Agreed." } }
-    assert_equal 2, card.messages.count
-    assert_predicate card.messages.last, :comment?
-    assert_equal "Agreed.", card.messages.last.messageable.body
-
-    # Assign it
-    post collection_card_assignments_path(collections(:writebook), card), params: { assignee_id: users(:kevin).id }
-    assert_equal 3, card.messages.count
-    assert_predicate card.messages.last, :event_summary?
-    assert_equal 1, card.messages.last.event_summary.events.count
-    assert_equal "card_assigned", card.messages.last.messageable.events.last.action
-  end
-end

test/models/card/closeable_test.rb

@@ -1,6 +1,10 @@
 require "test_helper"
 
 class Card::CloseableTest < ActiveSupport::TestCase
+  setup do
+    Current.session = sessions(:david)
+  end
+
   test "closed scope" do
     assert_equal [ cards(:shipping) ], Card.closed
     assert_not_includes Card.open, cards(:shipping)

test/models/card/commentable_test.rb

@@ -6,7 +6,7 @@ class Card::CommentableTest < ActiveSupport::TestCase
     assert_not cards(:text).watched_by?(users(:kevin))
 
     with_current_user(:kevin) do
-      cards(:text).capture Comment.new(body: "This sounds interesting!")
+      cards(:text).comments.create!(body: "This sounds interesting!")
     end
 
     assert cards(:text).watched_by?(users(:kevin))

test/models/card/eventable/system_commenter_test.rb

@@ -0,0 +1,68 @@
+require "test_helper"
+
+class Card::Eventable::SystemCommenterTest < ActiveSupport::TestCase
+  setup do
+    Current.session = sessions(:david)
+    @card = cards(:text)
+  end
+
+  test "card_assigned" do
+    assert_system_comment "Assigned to Kevin" do
+      @card.toggle_assignment users(:kevin)
+    end
+  end
+
+  test "card_unassigned" do
+    @card.toggle_assignment users(:kevin)
+    @card.comments.destroy_all # To skip deduplication logic
+
+    assert_system_comment "Unassigned from Kevin" do
+      @card.toggle_assignment users(:kevin)
+    end
+  end
+
+  test "card_staged" do
+    assert_system_comment "David moved this to 'In progress'" do
+      @card.change_stage_to workflow_stages(:qa_in_progress)
+    end
+  end
+
+  test "card_closed" do
+    assert_system_comment "Closed by David" do
+      @card.close
+    end
+  end
+
+  test "card_title_changed" do
+    assert_system_comment "David changed title from 'The text is too small' to 'Make text larger'" do
+      @card.update! title: "Make text larger"
+    end
+  end
+
+  test "don't duplicate comments for the same kind of events" do
+    @card.change_stage_to workflow_stages(:qa_in_progress)
+
+    assert_no_difference -> { @card.comments.count } do
+      @card.reload.change_stage_to workflow_stages(:qa_on_hold)
+      assert_match /David moved this to 'On hold'/i, @card.comments.last.body.to_plain_text
+    end
+  end
+
+  test "don't duplicate comments for related events" do
+    @card.toggle_assignment users(:kevin)
+
+    assert_no_difference -> { @card.comments.count } do
+      @card.reload.toggle_assignment users(:kevin)
+      assert_match /Unassigned from Kevin/i, @card.comments.last.body.to_plain_text
+    end
+  end
+
+  private
+    def assert_system_comment(expected_comment)
+      assert_difference -> { @card.comments.count }, 1 do
+        yield
+        assert @card.comments.last.creator.system?
+        assert_match Regexp.new(expected_comment.strip, Regexp::IGNORECASE), @card.comments.last.body.to_plain_text.strip
+      end
+    end
+end

test/models/card/messages_test.rb

@@ -4,6 +4,6 @@ class Card::MessagesTest < ActiveSupport::TestCase
   test "creating a card does not create a message by default" do
     card = collections(:writebook).cards.create! creator: users(:kevin), title: "New"
 
-    assert_empty card.messages
+    assert_empty card.comments
   end
 end

test/models/card_test.rb

@@ -6,11 +6,11 @@ class CardTest < ActiveSupport::TestCase
   end
 
   test "capturing messages" do
-    assert_difference "cards(:logo).messages.comments.count", +1 do
-      cards(:logo).capture Comment.new(body: "Agreed.")
+    assert_difference -> { cards(:logo).comments.count }, +1 do
+      cards(:logo).comments.create!(body: "Agreed.")
     end
 
-    assert_equal "Agreed.", cards(:logo).messages.comments.last.messageable.body.to_plain_text.chomp
+    assert_equal "Agreed.", cards(:logo).comments.last.body.to_plain_text.chomp
   end
 
   test "assignment states" do
@@ -21,7 +21,7 @@ class CardTest < ActiveSupport::TestCase
   test "assignment toggling" do
     assert cards(:logo).assigned_to?(users(:kevin))
 
-    assert_difference({ "cards(:logo).assignees.count" => -1, "Event.count" => +1 }) do
+    assert_difference({ -> { cards(:logo).assignees.count } => -1, -> { Event.count } => +1 }) do
       cards(:logo).toggle_assignment users(:kevin)
     end
     assert_not cards(:logo).assigned_to?(users(:kevin))
@@ -98,8 +98,8 @@ class CardTest < ActiveSupport::TestCase
 
   test "mentioning" do
     card = collections(:writebook).cards.create! title: "Insufficient haggis", creator: users(:kevin)
-    cards(:logo).capture Comment.new(body: "I hate haggis")
-    cards(:text).capture Comment.new(body: "I love haggis")
+    cards(:logo).comments.create!(body: "I hate haggis")
+    cards(:text).comments.create!(body: "I love haggis")
 
     assert_equal [ card, cards(:logo), cards(:text) ].sort, Card.mentioning("haggis").sort
   end

test/models/comment_test.rb

@@ -6,8 +6,8 @@ class CommentTest < ActiveSupport::TestCase
   end
 
   test "searchable by body" do
-    message = cards(:logo).capture Comment.new(body: "I'd prefer something more rustic")
+    comment = cards(:logo).comments.create!(body: "I'd prefer something more rustic")
 
-    assert_includes Comment.search("something rustic"), message.comment
+    assert_includes Comment.search("something rustic"), comment
   end
 end

test/models/event_summary_test.rb

@@ -1,8 +0,0 @@
-require "test_helper"
-
-class EventSummaryTest < ActiveSupport::TestCase
-  test "body" do
-    assert_equal " Assigned to JZ.", event_summaries(:logo_initial_activity).body
-    assert_equal "Assigned to Kevin.", event_summaries(:logo_second_activity).body
-  end
-end

test/models/filter_test.rb

@@ -7,8 +7,8 @@ class FilterTest < ActiveSupport::TestCase
       @new_card = @new_collection.cards.create!
       @new_card.update!(stage: workflow_stages(:qa_on_hold))
 
-      cards(:layout).capture Comment.new(body: "I hate haggis")
-      cards(:logo).capture Comment.new(body: "I love haggis")
+      cards(:layout).comments.create!(body: "I hate haggis")
+      cards(:logo).comments.create!(body: "I love haggis")
       cards(:logo).update(stage: workflow_stages(:qa_on_hold))
     end