From 8aa99e34b41cb3bddb6212e0a810decad6a2d300 Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@37signals.com>
Date: Tue, 22 Apr 2025 13:48:43 +0200
Subject: [PATCH] Persist notification's creator

Instead of delegating. With a polymorphic relationship, relying on a certain
attribute implies doing things like aliasing "mentioner" to "creator" or similar.
---
 app/models/mention.rb                         |  2 +-
 app/models/notification.rb                    |  2 +-
 app/models/notifier.rb                        |  4 +--
 app/models/notifier/events/base.rb            |  2 +-
 app/models/notifier/mention.rb                |  4 +++
 app/models/user/mentionable.rb                |  2 +-
 db/migrate/20250421120008_create_mentions.rb  |  2 +-
 db/schema.rb                                  | 11 +++---
 db/schema_cache.yml                           | 34 ++++++++++++++++---
 .../cards/readings_controller_test.rb         |  6 ++--
 test/fixtures/notifications.yml               |  3 ++
 11 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/app/models/mention.rb b/app/models/mention.rb
index dd3756fe1..515c3071c 100644
--- a/app/models/mention.rb
+++ b/app/models/mention.rb
@@ -2,6 +2,6 @@ class Mention < ApplicationRecord
   include Notifiable
 
   belongs_to :container, polymorphic: true
-  belongs_to :creator, class_name: "User"
+  belongs_to :mentioner, class_name: "User"
   belongs_to :mentionee, class_name: "User", inverse_of: :mentions
 end
diff --git a/app/models/notification.rb b/app/models/notification.rb
index b4b323a3d..301843987 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -1,5 +1,6 @@
 class Notification < ApplicationRecord
   belongs_to :user
+  belongs_to :creator, class_name: "User"
   belongs_to :source, polymorphic: true
   belongs_to :resource, polymorphic: true
 
@@ -7,7 +8,6 @@ class Notification < ApplicationRecord
   scope :read, -> { where.not(read_at: nil) }
   scope :ordered, -> { order(read_at: :desc, created_at: :desc) }
 
-  delegate :creator, to: :source
   after_create_commit :broadcast_unread
 
   def self.read_all
diff --git a/app/models/notifier.rb b/app/models/notifier.rb
index aa45e4071..f1a06a0ed 100644
--- a/app/models/notifier.rb
+++ b/app/models/notifier.rb
@@ -1,8 +1,6 @@
 class Notifier
   attr_reader :source
 
-  delegate :creator, to: :source
-
   class << self
     def for(source)
       case source
@@ -17,7 +15,7 @@ class Notifier
   def notify
     if should_notify?
       recipients.map do |recipient|
-        Notification.create! user: recipient, source: source, resource: resource
+        Notification.create! user: recipient, source: source, resource: resource, creator: creator
       end
     end
   end
diff --git a/app/models/notifier/events/base.rb b/app/models/notifier/events/base.rb
index 84178e3f4..13a915098 100644
--- a/app/models/notifier/events/base.rb
+++ b/app/models/notifier/events/base.rb
@@ -1,5 +1,5 @@
 class Notifier::Events::Base < Notifier
-  delegate :card, to: :source
+  delegate :card, :creator, to: :source
 
   private
     def should_notify?
diff --git a/app/models/notifier/mention.rb b/app/models/notifier/mention.rb
index 033d32f84..a35e0f4e4 100644
--- a/app/models/notifier/mention.rb
+++ b/app/models/notifier/mention.rb
@@ -7,4 +7,8 @@ class Notifier::Mention < Notifier
     def recipients
       [ source.mentionee ]
     end
+
+    def creator
+      source.mentioner
+    end
 end
diff --git a/app/models/user/mentionable.rb b/app/models/user/mentionable.rb
index 5d1df418e..e3e5a9dbf 100644
--- a/app/models/user/mentionable.rb
+++ b/app/models/user/mentionable.rb
@@ -6,7 +6,7 @@ module User::Mentionable
   end
 
   def mentioned_by(mentioner, at:)
-    mentions.create! container: at, creator: mentioner
+    mentions.create! container: at, mentioner: mentioner
   end
 
   def mentionable_handles
diff --git a/db/migrate/20250421120008_create_mentions.rb b/db/migrate/20250421120008_create_mentions.rb
index 9133e29a0..da4923950 100644
--- a/db/migrate/20250421120008_create_mentions.rb
+++ b/db/migrate/20250421120008_create_mentions.rb
@@ -3,7 +3,7 @@ class CreateMentions < ActiveRecord::Migration[8.1]
     create_table :mentions do |t|
       t.references :container, polymorphic: true, null: false, index: true
       t.references :mentionee, foreign_key: { to_table: :users }, null: false
-      t.references :creator, foreign_key: { to_table: :users }, null: false
+      t.references :mentioner, foreign_key: { to_table: :users }, null: false
 
       t.timestamps
     end
diff --git a/db/schema.rb b/db/schema.rb
index a3f338ebc..2da27d16f 100644
--- a/db/schema.rb
+++ b/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_22_062930) do
+ActiveRecord::Schema[8.1].define(version: 2025_04_22_112857) do
   create_table "accesses", force: :cascade do |t|
     t.integer "collection_id", null: false
     t.datetime "created_at", null: false
@@ -216,12 +216,12 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_22_062930) do
     t.integer "container_id", null: false
     t.string "container_type", null: false
     t.datetime "created_at", null: false
-    t.integer "creator_id", null: false
     t.integer "mentionee_id", null: false
+    t.integer "mentioner_id", null: false
     t.datetime "updated_at", null: false
     t.index ["container_type", "container_id"], name: "index_mentions_on_container"
-    t.index ["creator_id"], name: "index_mentions_on_creator_id"
     t.index ["mentionee_id"], name: "index_mentions_on_mentionee_id"
+    t.index ["mentioner_id"], name: "index_mentions_on_mentioner_id"
   end
 
   create_table "messages", force: :cascade do |t|
@@ -236,6 +236,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_22_062930) do
 
   create_table "notifications", force: :cascade do |t|
     t.datetime "created_at", null: false
+    t.integer "creator_id", null: false
     t.datetime "read_at"
     t.integer "resource_id", null: false
     t.string "resource_type", null: false
@@ -243,6 +244,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_22_062930) do
     t.string "source_type", null: false
     t.datetime "updated_at", null: false
     t.integer "user_id", null: false
+    t.index ["creator_id"], name: "index_notifications_on_creator_id"
     t.index ["resource_type", "resource_id"], name: "index_notifications_on_resource"
     t.index ["source_type", "source_id"], name: "index_notifications_on_source"
     t.index ["user_id", "read_at", "created_at"], name: "index_notifications_on_user_id_and_read_at_and_created_at", order: { read_at: :desc, created_at: :desc }
@@ -339,10 +341,11 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_22_062930) do
   add_foreign_key "collections", "workflows"
   add_foreign_key "events", "cards"
   add_foreign_key "events", "event_summaries", column: "summary_id"
-  add_foreign_key "mentions", "users", column: "creator_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"
   add_foreign_key "pins", "users"
   add_foreign_key "sessions", "users"
diff --git a/db/schema_cache.yml b/db/schema_cache.yml
index 382719da8..73d81743b 100644
--- a/db/schema_cache.yml
+++ b/db/schema_cache.yml
@@ -669,7 +669,16 @@ columns:
     default_function:
     collation:
     comment:
-  - *23
+  - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
+    auto_increment:
+    name: mentioner_id
+    cast_type: *1
+    sql_type_metadata: *2
+    'null': false
+    default:
+    default_function:
+    collation:
+    comment:
   - *8
   - *9
   messages:
@@ -752,6 +761,7 @@ columns:
     default_function:
     collation:
     comment:
+  - *23
   pins:
   - *5
   - *21
@@ -1647,10 +1657,10 @@ indexes:
   mentions:
   - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
     table: mentions
-    name: index_mentions_on_creator_id
+    name: index_mentions_on_mentioner_id
     unique: false
     columns:
-    - creator_id
+    - mentioner_id
     lengths: {}
     orders: {}
     opclasses: {}
@@ -1729,6 +1739,22 @@ indexes:
     comment:
     valid: true
   notifications:
+  - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
+    table: notifications
+    name: index_notifications_on_creator_id
+    unique: false
+    columns:
+    - creator_id
+    lengths: {}
+    orders: {}
+    opclasses: {}
+    where:
+    type:
+    using:
+    include:
+    nulls_not_distinct:
+    comment:
+    valid: true
   - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition
     table: notifications
     name: index_notifications_on_source
@@ -2019,4 +2045,4 @@ indexes:
     comment:
     valid: true
   workflows: []
-version: 20250422062930
+version: 20250422112857
diff --git a/test/controllers/cards/readings_controller_test.rb b/test/controllers/cards/readings_controller_test.rb
index a880e46c7..b6fa6d040 100644
--- a/test/controllers/cards/readings_controller_test.rb
+++ b/test/controllers/cards/readings_controller_test.rb
@@ -15,9 +15,9 @@ class Cards::ReadingsControllerTest < ActionDispatch::IntegrationTest
 
   test "read multiple notifications on card visit" do
     assert_changes -> { notifications(:logo_published_kevin).reload.read? }, from: false, to: true do
-    assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: false, to: true do
-      post card_reading_path(cards(:logo)), as: :turbo_stream
-    end
+      assert_changes -> { notifications(:logo_assignment_kevin).reload.read? }, from: false, to: true do
+        post card_reading_path(cards(:logo)), as: :turbo_stream
+      end
     end
 
     assert_response :success
diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml
index ef44452f7..db6574038 100644
--- a/test/fixtures/notifications.yml
+++ b/test/fixtures/notifications.yml
@@ -3,15 +3,18 @@ logo_published_kevin:
   source: logo_published (Event)
   resource: logo (Card)
   created_at: <%= 1.week.ago %>
+  creator: david
 
 logo_assignment_kevin:
   user: kevin
   source: logo_assignment_km (Event)
   resource: logo (Card)
   created_at: <%= 1.week.ago %>
+  creator: david
 
 layout_commented_kevin:
   user: kevin
   source: layout_commented (Event)
   resource: layout_overflowing_david (Comment)
   created_at: <%= 1.week.ago %>
+  creator: david