From aa3acfeaa84158f192daf5e4969171e48055a2cf Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 11:21:30 +0200 Subject: [PATCH 01/50] Add plain text mentions --- app/controllers/cards/readings_controller.rb | 2 +- app/helpers/notifications_helper.rb | 45 ++--- app/jobs/generate_notifications_job.rb | 7 - app/jobs/mention/collect_job.rb | 7 + app/jobs/notify_recipients_job.rb | 7 + app/models/card.rb | 7 +- app/models/card/eventable.rb | 3 +- app/models/card/mentions.rb | 12 ++ app/models/concerns/mentions.rb | 33 ++++ app/models/concerns/notifiable.rb | 13 +- app/models/event.rb | 22 ++- app/models/mention.rb | 7 + app/models/message.rb | 4 + app/models/message/mentions.rb | 12 ++ app/models/notification.rb | 5 +- app/models/notifier.rb | 33 ++-- app/models/notifier/assigned.rb | 6 - app/models/notifier/closed.rb | 2 - app/models/notifier/commented.rb | 6 - app/models/notifier/events/assigned.rb | 6 + app/models/notifier/events/base.rb | 12 ++ app/models/notifier/events/closed.rb | 2 + app/models/notifier/events/commented.rb | 6 + app/models/notifier/{ => events}/published.rb | 2 +- app/models/user.rb | 8 +- app/models/user/mentionable.rb | 20 ++ app/models/user/named.rb | 19 ++ .../notifications/_notification.html.erb | 10 +- .../notification/_event.html.erb | 7 + .../notification/_mention.html.erb | 11 ++ db/migrate/20250421120008_create_mentions.rb | 11 ++ ...50422054950_add_source_to_notifications.rb | 11 ++ ...emove_unused_columns_from_notifications.rb | 6 + db/schema.rb | 25 ++- db/schema_cache.yml | 178 +++++++++++++----- test/fixtures/notifications.yml | 9 +- test/models/concerns/mentions_test.rb | 23 +++ test/models/notifier_test.rb | 16 +- test/models/user/mentionable_test.rb | 7 + test/models/user/named_test.rb | 38 ++++ test/test_helper.rb | 1 + 41 files changed, 493 insertions(+), 168 deletions(-) delete mode 100644 app/jobs/generate_notifications_job.rb create mode 100644 app/jobs/mention/collect_job.rb create mode 100644 app/jobs/notify_recipients_job.rb create mode 100644 app/models/card/mentions.rb create mode 100644 app/models/concerns/mentions.rb create mode 100644 app/models/mention.rb create mode 100644 app/models/message/mentions.rb delete mode 100644 app/models/notifier/assigned.rb delete mode 100644 app/models/notifier/closed.rb delete mode 100644 app/models/notifier/commented.rb create mode 100644 app/models/notifier/events/assigned.rb create mode 100644 app/models/notifier/events/base.rb create mode 100644 app/models/notifier/events/closed.rb create mode 100644 app/models/notifier/events/commented.rb rename app/models/notifier/{ => events}/published.rb (67%) create mode 100644 app/models/user/mentionable.rb create mode 100644 app/models/user/named.rb create mode 100644 app/views/notifications/notification/_event.html.erb create mode 100644 app/views/notifications/notification/_mention.html.erb create mode 100644 db/migrate/20250421120008_create_mentions.rb create mode 100644 db/migrate/20250422054950_add_source_to_notifications.rb create mode 100644 db/migrate/20250422062930_remove_unused_columns_from_notifications.rb create mode 100644 test/models/concerns/mentions_test.rb create mode 100644 test/models/user/mentionable_test.rb create mode 100644 test/models/user/named_test.rb diff --git a/app/controllers/cards/readings_controller.rb b/app/controllers/cards/readings_controller.rb index 2d8b51ac6..efd06e601 100644 --- a/app/controllers/cards/readings_controller.rb +++ b/app/controllers/cards/readings_controller.rb @@ -2,7 +2,7 @@ class Cards::ReadingsController < ApplicationController include CardScoped def create - @notifications = Current.user.notifications.where(card: @card) + @notifications = Current.user.notifications.where(source: @card.events) @notifications.each(&:read) end end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 656edad54..f1cd88a6c 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,21 +1,25 @@ module NotificationsHelper - def notification_title(notification) - if notification.resource.is_a? Comment - "RE: " + notification.card.title - elsif notification_event_action(notification) == "assigned" - "Assigned to you: " + notification.card.title + def event_notification_title(event) + if event.commented? + "RE: " + event.card.title + elsif event.assignment? + "Assigned to you: " + event.card.title else - notification.card.title + event.card.title end end - def notification_body(notification) - name = notification.creator.name + def event_notification_body(event) + name = event.creator.name - case notification_event_action(notification) - when "closed" then "Closed by #{name}" - when "published" then "Added by #{name}" - else name + if event.closed? + "Closed by #{name}" + elsif event.published? + "Added by #{name}" + elsif event.commented? + "#{strip_tags(event.comment.body_html).blank? ? "#{name} replied" : "#{event.creator.name}:" } #{strip_tags(event.comment.body_html).truncate(200)}" + else + name end end @@ -36,8 +40,8 @@ module NotificationsHelper class: "notification__unread_indicator btn borderless", title: "Mark as read", data: { turbo_frame: "_top" } do - concat(image_tag("remove-med.svg", class: "unread_icon", size: 12, aria: { hidden: true })) - concat(tag.span("Mark as read", class: "for-screen-reader")) + concat(image_tag("remove-med.svg", class: "unread_icon", size: 12, aria: { hidden: true })) + concat(tag.span("Mark as read", class: "for-screen-reader")) end end @@ -46,17 +50,4 @@ module NotificationsHelper tag.div id: "next_page", data: { controller: "fetch-on-visible", fetch_on_visible_url_value: notifications_path(page: @page.next_param) } end end - - private - def notification_event_action(notification) - if notification_is_for_initial_assignement?(notification) - "assigned" - else - notification.event.action - end - end - - def notification_is_for_initial_assignement?(notification) - notification.event.action == "published" && notification.card.assigned_to?(notification.user) - end end diff --git a/app/jobs/generate_notifications_job.rb b/app/jobs/generate_notifications_job.rb deleted file mode 100644 index e386b7b37..000000000 --- a/app/jobs/generate_notifications_job.rb +++ /dev/null @@ -1,7 +0,0 @@ -class GenerateNotificationsJob < ApplicationJob - queue_as :default - - def perform(event) - event.generate_notifications - end -end diff --git a/app/jobs/mention/collect_job.rb b/app/jobs/mention/collect_job.rb new file mode 100644 index 000000000..088e240dd --- /dev/null +++ b/app/jobs/mention/collect_job.rb @@ -0,0 +1,7 @@ +class Mention::CollectJob < ApplicationJob + queue_as :default + + def perform(record, mentioner:) + record.collect_mentions(mentioner:) + end +end diff --git a/app/jobs/notify_recipients_job.rb b/app/jobs/notify_recipients_job.rb new file mode 100644 index 000000000..2f14c2e96 --- /dev/null +++ b/app/jobs/notify_recipients_job.rb @@ -0,0 +1,7 @@ +class NotifyRecipientsJob < ApplicationJob + queue_as :default + + def perform(notifiable) + notifiable.notify_recipients + end +end diff --git a/app/models/card.rb b/app/models/card.rb index 541d761a9..57ff542da 100644 --- a/app/models/card.rb +++ b/app/models/card.rb @@ -1,13 +1,10 @@ class Card < ApplicationRecord - include Assignable, Colored, Engageable, Eventable, Golden, - Messages, Notifiable, Pinnable, Closeable, Searchable, Staged, - Statuses, Taggable, Watchable + include Assignable, Colored, Engageable, Eventable, Golden, Mentions, + Messages, Pinnable, Closeable, Searchable, Staged, Statuses, Taggable, Watchable belongs_to :collection, touch: true belongs_to :creator, class_name: "User", default: -> { Current.user } - has_many :notifications, dependent: :destroy - has_one_attached :image, dependent: :purge_later has_markdown :description diff --git a/app/models/card/eventable.rb b/app/models/card/eventable.rb index 1d3c4e13b..55a0245ea 100644 --- a/app/models/card/eventable.rb +++ b/app/models/card/eventable.rb @@ -12,8 +12,7 @@ module Card::Eventable def track_event(action, creator: Current.user, **particulars) if published? - event = find_or_capture_event_summary.events.create! action: action, creator: creator, card: self, particulars: particulars - event.generate_notifications_later + find_or_capture_event_summary.events.create! action: action, creator: creator, card: self, particulars: particulars end end diff --git a/app/models/card/mentions.rb b/app/models/card/mentions.rb new file mode 100644 index 000000000..dd135f24c --- /dev/null +++ b/app/models/card/mentions.rb @@ -0,0 +1,12 @@ +module Card::Mentions + extend ActiveSupport::Concern + + included do + include ::Mentions + end + + private + def mentionable_content + description.to_plain_text + end +end diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb new file mode 100644 index 000000000..8de010a98 --- /dev/null +++ b/app/models/concerns/mentions.rb @@ -0,0 +1,33 @@ +module Mentions + extend ActiveSupport::Concern + + included do + has_many :mentions, as: :container, dependent: :destroy + after_commit :collect_mentions_later, on: %i[ create update ] + end + + def collect_mentions(mentioner: Current.user) + scan_mentionees.each do |mentionee| + mentionee.mentioned_by mentioner, at: self + end + end + + private + def collect_mentions_later + Mention::CollectJob.perform_later(self, mentioner: Current.user) + end + + def scan_mentionees + scan_mentioned_handles.filter_map do |mention| + mentionable_users.find { |user| user.mentionable_handles.include?(mention) } + end + end + + def mentionable_users + collection.users + end + + def scan_mentioned_handles + (mentionable_content || "").scan(/(? { card.touch(:last_active_at) } - def commented? - action == "commented" + def assignment? + action == "assigned" || initial_assignment? end - def generate_notifications - Notifier.for(self)&.generate + # E.g: completed? is true if action == "completed" + def method_missing(method_name, *args, &block) + if method_name.to_s.end_with?("?") + action == method_name.to_s.chomp("?") + else + super + end end - def generate_notifications_later - GenerateNotificationsJob.perform_later self - end + private + def initial_assignment? + action == "published" && card.assigned_to?(creator) + end end diff --git a/app/models/mention.rb b/app/models/mention.rb new file mode 100644 index 000000000..5f263d9be --- /dev/null +++ b/app/models/mention.rb @@ -0,0 +1,7 @@ +class Mention < ApplicationRecord + include Notifiable + + belongs_to :container, polymorphic: true + belongs_to :mentionee, class_name: "User", inverse_of: :mentions + belongs_to :mentioner, class_name: "User" +end diff --git a/app/models/message.rb b/app/models/message.rb index 7d950b3b7..2947c99b5 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -1,7 +1,11 @@ class Message < ApplicationRecord + include Mentions + belongs_to :card, touch: true delegated_type :messageable, types: Messageable::TYPES, inverse_of: :message, dependent: :destroy scope :chronologically, -> { order created_at: :asc, id: :desc } + + delegate :collection, to: :card end diff --git a/app/models/message/mentions.rb b/app/models/message/mentions.rb new file mode 100644 index 000000000..fc8282413 --- /dev/null +++ b/app/models/message/mentions.rb @@ -0,0 +1,12 @@ +module Message::Mentions + extend ActiveSupport::Concern + + included do + include ::Mentions + end + + private + def mentionable_content + messageable.try(:body_plain_text) + end +end diff --git a/app/models/notification.rb b/app/models/notification.rb index 9fbb94a5b..b4b323a3d 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -1,14 +1,13 @@ class Notification < ApplicationRecord belongs_to :user - belongs_to :event - belongs_to :card + belongs_to :source, polymorphic: true belongs_to :resource, polymorphic: true scope :unread, -> { where(read_at: nil) } scope :read, -> { where.not(read_at: nil) } scope :ordered, -> { order(read_at: :desc, created_at: :desc) } - delegate :creator, to: :event + 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 19626be5f..455a900e3 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -1,40 +1,37 @@ class Notifier - attr_reader :event + attr_reader :source - delegate :creator, to: :event + delegate :creator, to: :source class << self - def for(event) - "Notifier::#{event.action.classify}".safe_constantize&.new(event) + def for(source) + case source + when Event + "Notifier::Events::#{source.action.classify}".safe_constantize&.new(source) + when Mention + Notifier::Mentions + end end end - def generate + def notify if should_notify? recipients.map do |recipient| - Notification.create! user: recipient, event: event, card: card, resource: resource + Notification.create! user: recipient, source: source, resource: resource end end end private - def initialize(event) - @event = event + def initialize(source) + @source = source end def should_notify? - !event.creator.system? - end - - def recipients - card.watchers_and_subscribers.without(creator) + !creator.system? end def resource - card - end - - def card - event.summary.message.card + source end end diff --git a/app/models/notifier/assigned.rb b/app/models/notifier/assigned.rb deleted file mode 100644 index 843376435..000000000 --- a/app/models/notifier/assigned.rb +++ /dev/null @@ -1,6 +0,0 @@ -class Notifier::Assigned < Notifier - private - def recipients - event.assignees.excluding(card.collection.access_only_users) - end -end diff --git a/app/models/notifier/closed.rb b/app/models/notifier/closed.rb deleted file mode 100644 index f090b0308..000000000 --- a/app/models/notifier/closed.rb +++ /dev/null @@ -1,2 +0,0 @@ -class Notifier::Closed < Notifier -end diff --git a/app/models/notifier/commented.rb b/app/models/notifier/commented.rb deleted file mode 100644 index 42ad4e5c8..000000000 --- a/app/models/notifier/commented.rb +++ /dev/null @@ -1,6 +0,0 @@ -class Notifier::Commented < Notifier - private - def resource - event.comment - end -end diff --git a/app/models/notifier/events/assigned.rb b/app/models/notifier/events/assigned.rb new file mode 100644 index 000000000..fb106c677 --- /dev/null +++ b/app/models/notifier/events/assigned.rb @@ -0,0 +1,6 @@ +class Notifier::Events::Assigned < Notifier::Events::Base + private + def recipients + source.assignees.excluding(card.collection.access_only_users) + end +end diff --git a/app/models/notifier/events/base.rb b/app/models/notifier/events/base.rb new file mode 100644 index 000000000..b0e9d0489 --- /dev/null +++ b/app/models/notifier/events/base.rb @@ -0,0 +1,12 @@ +class Notifier::Events::Base < Notifier + delegate :card, to: :source + + private + def resource + card + end + + def recipients + card.watchers_and_subscribers.without(creator) + end +end diff --git a/app/models/notifier/events/closed.rb b/app/models/notifier/events/closed.rb new file mode 100644 index 000000000..133ca75c8 --- /dev/null +++ b/app/models/notifier/events/closed.rb @@ -0,0 +1,2 @@ +class Notifier::Events::Closed < Notifier::Events::Base +end diff --git a/app/models/notifier/events/commented.rb b/app/models/notifier/events/commented.rb new file mode 100644 index 000000000..55497a117 --- /dev/null +++ b/app/models/notifier/events/commented.rb @@ -0,0 +1,6 @@ +class Notifier::Events::Commented < Notifier::Events::Base + private + def resource + source.comment + end +end diff --git a/app/models/notifier/published.rb b/app/models/notifier/events/published.rb similarity index 67% rename from app/models/notifier/published.rb rename to app/models/notifier/events/published.rb index ce3917e8f..ff30d0219 100644 --- a/app/models/notifier/published.rb +++ b/app/models/notifier/events/published.rb @@ -1,4 +1,4 @@ -class Notifier::Published < Notifier +class Notifier::Events::Published < Notifier::Events::Base private def recipients card.watchers_and_subscribers(include_only_watching: true).without(creator) diff --git a/app/models/user.rb b/app/models/user.rb index a22841c42..c09455bfb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,5 @@ class User < ApplicationRecord - include Accessor, Assignee, Avatar, Role, Transferable + include Accessor, Assignee, Avatar, Mentionable, Named, Role, Transferable has_many :sessions, dependent: :destroy has_secure_password validations: false @@ -15,12 +15,6 @@ class User < ApplicationRecord normalizes :email_address, with: ->(value) { value.strip.downcase } - scope :alphabetically, -> { order("lower(name)") } - - def initials - name.to_s.scan(/\b\p{L}/).join.upcase - end - def deactivate transaction do sessions.delete_all diff --git a/app/models/user/mentionable.rb b/app/models/user/mentionable.rb new file mode 100644 index 000000000..0d1c9fe36 --- /dev/null +++ b/app/models/user/mentionable.rb @@ -0,0 +1,20 @@ +module User::Mentionable + extend ActiveSupport::Concern + + included do + has_many :mentions, dependent: :destroy, inverse_of: :mentionee + end + + def mentioned_by(mentioner, at:) + mentions.create! container: at, mentioner: + end + + def mentionable_handles + [ initials, first_name, first_name_with_last_name_initial ].collect(&:downcase) + end + + private + def first_name_with_last_name_initial + "#{first_name}#{last_name&.first}" + end +end diff --git a/app/models/user/named.rb b/app/models/user/named.rb new file mode 100644 index 000000000..67420ec72 --- /dev/null +++ b/app/models/user/named.rb @@ -0,0 +1,19 @@ +module User::Named + extend ActiveSupport::Concern + + included do + scope :alphabetically, -> { order("lower(name)") } + end + + def first_name + name.split(/\s/).first + end + + def last_name + name.split(/\s/, 2).last + end + + def initials + name.scan(/\b\p{L}/).join.upcase + end +end diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb index 8cb22eeb1..36ec74370 100644 --- a/app/views/notifications/_notification.html.erb +++ b/app/views/notifications/_notification.html.erb @@ -5,15 +5,7 @@
- <%= notification_title(notification) %> -
- <% if notification.event.action == "commented" %> - <%= "#{strip_tags(notification.event.comment.body_html).blank? ? "#{notification.event.creator.name} replied" : "#{notification.event.creator.name}:" } #{strip_tags(notification.event.comment.body_html).truncate(200)}" %> - <% else %> - <%= notification_body(notification) %> - <% end %> -
-
<%= notification.card.collection.name %> · <%= local_datetime_tag(notification.created_at, style: :ago) %>
+ <%= render "notifications/notification/#{notification.source_type.underscore}", notification: notification %>
<% end %> <% end %> diff --git a/app/views/notifications/notification/_event.html.erb b/app/views/notifications/notification/_event.html.erb new file mode 100644 index 000000000..b7b1231db --- /dev/null +++ b/app/views/notifications/notification/_event.html.erb @@ -0,0 +1,7 @@ +<%= event = notification.source %> + +<%= event_notification_title(event) %> +
+ <%= event_notification_body(event) %> +
+
<%= notification.source.card.collection.name %> · <%= local_datetime_tag(notification.created_at, style: :ago) %>
diff --git a/app/views/notifications/notification/_mention.html.erb b/app/views/notifications/notification/_mention.html.erb new file mode 100644 index 000000000..996095e6d --- /dev/null +++ b/app/views/notifications/notification/_mention.html.erb @@ -0,0 +1,11 @@ +<% event = notification.event %> + +<%= event_notification_title(notification) %> +
+ <% if event.action == "commented" %> + <%= "#{strip_tags(event.comment.body_html).blank? ? "#{event.creator.name} replied" : "#{event.creator.name}:" } #{strip_tags(event.comment.body_html).truncate(200)}" %> + <% else %> + <%= event_notification_body(notification) %> + <% end %> +
+
<%= event.card.collection.name %> · <%= local_datetime_tag(notification.created_at, style: :ago) %>
diff --git a/db/migrate/20250421120008_create_mentions.rb b/db/migrate/20250421120008_create_mentions.rb new file mode 100644 index 000000000..da4923950 --- /dev/null +++ b/db/migrate/20250421120008_create_mentions.rb @@ -0,0 +1,11 @@ +class CreateMentions < ActiveRecord::Migration[8.1] + def change + 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 :mentioner, foreign_key: { to_table: :users }, null: false + + t.timestamps + end + end +end diff --git a/db/migrate/20250422054950_add_source_to_notifications.rb b/db/migrate/20250422054950_add_source_to_notifications.rb new file mode 100644 index 000000000..7108d680b --- /dev/null +++ b/db/migrate/20250422054950_add_source_to_notifications.rb @@ -0,0 +1,11 @@ +class AddSourceToNotifications < ActiveRecord::Migration[8.1] + def change + add_reference :notifications, :source, polymorphic: true, index: true + + execute <<~SQL + update notifications set source_type = 'Event' + SQL + + change_column_null :notifications, :source_type, false + end +end diff --git a/db/migrate/20250422062930_remove_unused_columns_from_notifications.rb b/db/migrate/20250422062930_remove_unused_columns_from_notifications.rb new file mode 100644 index 000000000..30f689c88 --- /dev/null +++ b/db/migrate/20250422062930_remove_unused_columns_from_notifications.rb @@ -0,0 +1,6 @@ +class RemoveUnusedColumnsFromNotifications < ActiveRecord::Migration[8.1] + def change + remove_column :notifications, :event_id + remove_column :notifications, :card_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 3a56a34ae..12c2edba1 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_18_150259) do +ActiveRecord::Schema[8.1].define(version: 2025_04_22_062930) do create_table "accesses", force: :cascade do |t| t.integer "collection_id", null: false t.datetime "created_at", null: false @@ -212,6 +212,18 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_18_150259) do t.index ["tag_id"], name: "index_filters_tags_on_tag_id" end + create_table "mentions", force: :cascade do |t| + t.integer "container_id", null: false + t.string "container_type", null: false + t.datetime "created_at", 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 ["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| t.integer "card_id", null: false t.datetime "created_at", null: false @@ -223,17 +235,16 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_18_150259) do end create_table "notifications", force: :cascade do |t| - t.integer "card_id", null: false t.datetime "created_at", null: false - t.integer "event_id", null: false t.datetime "read_at" t.integer "resource_id", null: false t.string "resource_type", null: false + t.integer "source_id" + t.string "source_type", null: false t.datetime "updated_at", null: false t.integer "user_id", null: false - t.index ["card_id"], name: "index_notifications_on_card_id" - t.index ["event_id"], name: "index_notifications_on_event_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 } t.index ["user_id"], name: "index_notifications_on_user_id" end @@ -328,9 +339,9 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_18_150259) 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: "mentionee_id" + add_foreign_key "mentions", "users", column: "mentioner_id" add_foreign_key "messages", "cards" - add_foreign_key "notifications", "cards" - add_foreign_key "notifications", "events" add_foreign_key "notifications", "users" add_foreign_key "pins", "cards" add_foreign_key "pins", "users" diff --git a/db/schema_cache.yml b/db/schema_cache.yml index 8152c393d..bab7bb58f 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -637,6 +637,50 @@ columns: default_function: collation: comment: + mentions: + - *5 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: container_type + cast_type: *7 + sql_type_metadata: *8 + 'null': false + default: + default_function: + collation: + comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: container_id + cast_type: *1 + sql_type_metadata: *2 + 'null': false + default: + default_function: + collation: + comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: mentionee_id + cast_type: *1 + sql_type_metadata: *2 + 'null': false + default: + default_function: + collation: + comment: + - !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: + - *6 + - *9 messages: - *5 - *21 @@ -664,18 +708,7 @@ columns: - *9 notifications: - *5 - - *21 - *6 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: event_id - cast_type: *1 - sql_type_metadata: *2 - 'null': false - default: - default_function: - collation: - comment: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: read_at @@ -708,6 +741,26 @@ columns: comment: - *9 - *28 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: source_type + cast_type: *7 + sql_type_metadata: *8 + 'null': false + default: + default_function: + collation: + comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: source_id + cast_type: *1 + sql_type_metadata: *2 + 'null': true + default: + default_function: + collation: + comment: pins: - *5 - *21 @@ -921,6 +974,7 @@ primary_keys: filters: id filters_stages: filters_tags: + mentions: id messages: id notifications: id pins: id @@ -958,6 +1012,7 @@ data_sources: filters: true filters_stages: true filters_tags: true + mentions: true messages: true notifications: true pins: true @@ -1598,6 +1653,56 @@ indexes: nulls_not_distinct: comment: valid: true + mentions: + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: mentions + name: index_mentions_on_mentioner_id + unique: false + columns: + - mentioner_id + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: mentions + name: index_mentions_on_mentionee_id + unique: false + columns: + - mentionee_id + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: mentions + name: index_mentions_on_container + unique: false + columns: + - container_type + - container_id + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true messages: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: messages @@ -1633,6 +1738,23 @@ indexes: comment: valid: true notifications: + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: notifications + name: index_notifications_on_source + unique: false + columns: + - source_type + - source_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_user_id @@ -1686,38 +1808,6 @@ indexes: nulls_not_distinct: comment: valid: true - - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: notifications - name: index_notifications_on_event_id - unique: false - columns: - - event_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_card_id - unique: false - columns: - - card_id - lengths: {} - orders: {} - opclasses: {} - where: - type: - using: - include: - nulls_not_distinct: - comment: - valid: true pins: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: pins @@ -1938,4 +2028,4 @@ indexes: comment: valid: true workflows: [] -version: 20250418150259 +version: 20250422062930 diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml index 7579f8eb6..ef44452f7 100644 --- a/test/fixtures/notifications.yml +++ b/test/fixtures/notifications.yml @@ -1,20 +1,17 @@ logo_published_kevin: user: kevin - event: logo_published - card: logo + source: logo_published (Event) resource: logo (Card) created_at: <%= 1.week.ago %> logo_assignment_kevin: user: kevin - event: logo_assignment_km - card: logo + source: logo_assignment_km (Event) resource: logo (Card) created_at: <%= 1.week.ago %> layout_commented_kevin: user: kevin - event: layout_commented - card: layout + source: layout_commented (Event) resource: layout_overflowing_david (Comment) created_at: <%= 1.week.ago %> diff --git a/test/models/concerns/mentions_test.rb b/test/models/concerns/mentions_test.rb new file mode 100644 index 000000000..3ed1c4d49 --- /dev/null +++ b/test/models/concerns/mentions_test.rb @@ -0,0 +1,23 @@ +require "test_helper" + +class MentionsTest < ActiveSupport::TestCase + setup do + Current.session = sessions(:david) + end + + test "collect mentions on create" do + assert_difference -> { Mention.count }, +1 do + perform_enqueued_jobs only: Mention::CollectJob do + collections(:writebook).cards.create title: "Cleanup", description: "Did you finish up with the cleanup, @david?" + end + end + end + + test "collect mentions on update" do + assert_difference -> { Mention.count }, +1 do + perform_enqueued_jobs only: Mention::CollectJob do + cards(:logo).update! description: "Did you finish up with the cleanup, @david?" + end + end + end +end diff --git a/test/models/notifier_test.rb b/test/models/notifier_test.rb index 1e01e18e8..2e915bded 100644 --- a/test/models/notifier_test.rb +++ b/test/models/notifier_test.rb @@ -2,7 +2,7 @@ require "test_helper" class NotifierTest < ActiveSupport::TestCase test "for returns the matching notifier class for the event" do - assert_kind_of Notifier::Published, Notifier.for(events(:logo_published)) + assert_kind_of Notifier::Events::Published, Notifier.for(events(:logo_published)) end test "generate does not create notifications if the event was system-generated" do @@ -10,12 +10,12 @@ class NotifierTest < ActiveSupport::TestCase events(:logo_published).update!(creator: User.system) assert_no_difference -> { Notification.count } do - Notifier.for(events(:logo_published)).generate + Notifier.for(events(:logo_published)).notify end end test "creates a notification for each watcher, other than the event creator" do - notifications = Notifier.for(events(:layout_commented)).generate + notifications = Notifier.for(events(:layout_commented)).notify assert_equal [ users(:kevin) ], notifications.map(&:user) end @@ -23,19 +23,19 @@ class NotifierTest < ActiveSupport::TestCase test "does not create a notification for access-only users" do collections(:writebook).access_for(users(:kevin)).access_only! - notifications = Notifier.for(events(:layout_commented)).generate + notifications = Notifier.for(events(:layout_commented)).notify assert_equal [ users(:kevin) ], notifications.map(&:user) end test "the published event creates notifications for subscribers as well as watchers" do - notifications = Notifier.for(events(:logo_published)).generate + notifications = Notifier.for(events(:logo_published)).notify assert_equal users(:kevin, :jz).sort, notifications.map(&:user).sort end test "links to the card" do - Notifier.for(events(:logo_published)).generate + Notifier.for(events(:logo_published)).notify assert_equal cards(:logo), Notification.last.resource end @@ -44,7 +44,7 @@ class NotifierTest < ActiveSupport::TestCase collections(:writebook).access_for(users(:jz)).watching! collections(:writebook).access_for(users(:kevin)).everything! - notifications = Notifier.for(events(:logo_assignment_jz)).generate + notifications = Notifier.for(events(:logo_assignment_jz)).notify assert_equal [ users(:jz) ], notifications.map(&:user) end @@ -52,7 +52,7 @@ class NotifierTest < ActiveSupport::TestCase test "assignment events do not notify users who are access-only for the collection" do collections(:writebook).access_for(users(:jz)).access_only! - notifications = Notifier.for(events(:logo_assignment_jz)).generate + notifications = Notifier.for(events(:logo_assignment_jz)).notify assert_empty notifications end diff --git a/test/models/user/mentionable_test.rb b/test/models/user/mentionable_test.rb new file mode 100644 index 000000000..43e70792c --- /dev/null +++ b/test/models/user/mentionable_test.rb @@ -0,0 +1,7 @@ +require 'test_helper' + +class User::MentionableTest < ActiveSupport::TestCase + test "mentionable handles" do + assert_equal [ "dhh", "david", "davidh" ], User.new(name: "David Heinemeier-Hansson").mentionable_handles + end +end diff --git a/test/models/user/named_test.rb b/test/models/user/named_test.rb new file mode 100644 index 000000000..8993dbbf8 --- /dev/null +++ b/test/models/user/named_test.rb @@ -0,0 +1,38 @@ +require "test_helper" + +class User::NamedTest < ActiveSupport::TestCase + test "initials" do + assert_initials "M", name: "Michael" + assert_initials "SD", name: "Salvador Dali" + assert_initials "LMM", name: "Lin-Manuel Miranda" + assert_initials "OCD", name: "O'Conor Díez" + assert_initials "ACG", name: "Anne Christine García" + assert_initials "ÁL", name: "Ángela López" + end + + test "first name" do + assert_first_name "Michael", "Michael" + assert_first_name "Salvador", "Salvador Dali" + assert_first_name "Lin-Manuel", "Lin-Manuel Miranda" + assert_first_name "Anne", "Anne Christine García" + end + + test "last name" do + assert_last_name "Dali", "Salvador Dali" + assert_last_name "Miranda", "Lin_Manuel Miranda" + assert_last_name "Christine García", "Anne Christine García" + end + + private + def assert_initials(expected, **attributes) + assert_equal expected, User.new(attributes).initials + end + + def assert_first_name(expected, name) + assert_equal expected, User.new(name: name).first_name + end + + def assert_last_name(expected, name) + assert_equal expected, User.new(name: name).last_name + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 1447430b1..cb34fd812 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -10,6 +10,7 @@ module ActiveSupport # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. fixtures :all + include ActiveJob::TestHelper include CardTestHelper, ChangeTestHelper, SessionTestHelper end end From 4f064734053d9d854666f6a0ed1fb93befa6aa21 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 12:02:48 +0200 Subject: [PATCH 02/50] Notifications for mentions showing up --- app/helpers/notifications_helper.rb | 4 +- app/models/mention.rb | 2 +- app/models/notifier.rb | 6 +- app/models/notifier/events/base.rb | 4 + app/models/notifier/mention.rb | 10 + app/models/user/mentionable.rb | 2 +- .../notification/_event.html.erb | 2 +- .../notification/_mention.html.erb | 12 +- db/migrate/20250421120008_create_mentions.rb | 2 +- db/schema.rb | 6 +- db/schema_cache.yml | 883 +++++++++--------- test/models/notifier_test.rb | 8 +- 12 files changed, 474 insertions(+), 467 deletions(-) create mode 100644 app/models/notifier/mention.rb diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index f1cd88a6c..23ba12b38 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -25,8 +25,10 @@ module NotificationsHelper def notification_tag(notification, &) tag.div id: dom_id(notification), class: "notification tray__item border-radius txt-normal" do + # TODO: Temporary, I'll remove this. Right now, we support linking comments, but we should be linking messages. + resource = notification.resource.is_a?(Message) ? notification.resource.comment : notification.resource concat( - link_to(notification.resource, + link_to(resource, class: "notification__content border-radius shadow fill-white flex align-start txt-align-start gap flex-item-grow max-width border txt-ink", data: { action: "click->dialog#close", turbo_frame: "_top" }, &) diff --git a/app/models/mention.rb b/app/models/mention.rb index 5f263d9be..dd3756fe1 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 :mentionee, class_name: "User", inverse_of: :mentions - belongs_to :mentioner, class_name: "User" end diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 455a900e3..aa45e4071 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -8,8 +8,8 @@ class Notifier case source when Event "Notifier::Events::#{source.action.classify}".safe_constantize&.new(source) - when Mention - Notifier::Mentions + when ::Mention + Notifier::Mention.new(source) end end end @@ -28,7 +28,7 @@ class Notifier end def should_notify? - !creator.system? + true end def resource diff --git a/app/models/notifier/events/base.rb b/app/models/notifier/events/base.rb index b0e9d0489..84178e3f4 100644 --- a/app/models/notifier/events/base.rb +++ b/app/models/notifier/events/base.rb @@ -2,6 +2,10 @@ class Notifier::Events::Base < Notifier delegate :card, to: :source private + def should_notify? + !creator.system? + end + def resource card end diff --git a/app/models/notifier/mention.rb b/app/models/notifier/mention.rb new file mode 100644 index 000000000..033d32f84 --- /dev/null +++ b/app/models/notifier/mention.rb @@ -0,0 +1,10 @@ +class Notifier::Mention < Notifier + private + def resource + source.container + end + + def recipients + [ source.mentionee ] + end +end diff --git a/app/models/user/mentionable.rb b/app/models/user/mentionable.rb index 0d1c9fe36..5d1df418e 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, mentioner: + mentions.create! container: at, creator: mentioner end def mentionable_handles diff --git a/app/views/notifications/notification/_event.html.erb b/app/views/notifications/notification/_event.html.erb index b7b1231db..9a02db061 100644 --- a/app/views/notifications/notification/_event.html.erb +++ b/app/views/notifications/notification/_event.html.erb @@ -1,4 +1,4 @@ -<%= event = notification.source %> +<% event = notification.source %> <%= event_notification_title(event) %>
diff --git a/app/views/notifications/notification/_mention.html.erb b/app/views/notifications/notification/_mention.html.erb index 996095e6d..5dc61b1af 100644 --- a/app/views/notifications/notification/_mention.html.erb +++ b/app/views/notifications/notification/_mention.html.erb @@ -1,11 +1,5 @@ -<% event = notification.event %> - -<%= event_notification_title(notification) %> +<%= "OMG" %>
- <% if event.action == "commented" %> - <%= "#{strip_tags(event.comment.body_html).blank? ? "#{event.creator.name} replied" : "#{event.creator.name}:" } #{strip_tags(event.comment.body_html).truncate(200)}" %> - <% else %> - <%= event_notification_body(notification) %> - <% end %> + YOU WERE MENTIONED!
-
<%= event.card.collection.name %> · <%= local_datetime_tag(notification.created_at, style: :ago) %>
+
<%= local_datetime_tag(notification.created_at, style: :ago) %>
diff --git a/db/migrate/20250421120008_create_mentions.rb b/db/migrate/20250421120008_create_mentions.rb index da4923950..9133e29a0 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 :mentioner, foreign_key: { to_table: :users }, null: false + t.references :creator, foreign_key: { to_table: :users }, null: false t.timestamps end diff --git a/db/schema.rb b/db/schema.rb index 12c2edba1..a3f338ebc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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| @@ -339,8 +339,8 @@ 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 "pins", "cards" diff --git a/db/schema_cache.yml b/db/schema_cache.yml index bab7bb58f..382719da8 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -31,7 +31,17 @@ columns: default_function: collation: comment: - - &6 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &28 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: user_id + cast_type: *1 + sql_type_metadata: *2 + 'null': false + default: + default_function: + collation: + comment: + - &8 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: created_at cast_type: &3 !ruby/object:ActiveRecord::Type::DateTime @@ -50,26 +60,6 @@ columns: default_function: collation: comment: - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: involvement - cast_type: &7 !ruby/object:ActiveModel::Type::String - true: t - false: f - precision: - scale: - limit: - sql_type_metadata: &8 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata - sql_type: varchar - type: :string - limit: - precision: - scale: - 'null': false - default: watching - default_function: - collation: - comment: - &9 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: updated_at @@ -80,9 +70,65 @@ columns: default_function: collation: comment: - - &28 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: user_id + name: involvement + cast_type: &6 !ruby/object:ActiveModel::Type::String + true: t + false: f + precision: + scale: + limit: + sql_type_metadata: &7 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata + sql_type: varchar + type: :string + limit: + precision: + scale: + 'null': false + default: watching + default_function: + collation: + comment: + accounts: + - *5 + - &10 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: name + cast_type: *6 + sql_type_metadata: *7 + 'null': false + default: + default_function: + collation: + comment: + - *8 + - *9 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: join_code + cast_type: *6 + sql_type_metadata: *7 + 'null': true + default: + default_function: + collation: + comment: + action_text_markdowns: + - *5 + - &11 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: record_type + cast_type: *6 + sql_type_metadata: *7 + 'null': false + default: + default_function: + collation: + comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: record_id cast_type: *1 sql_type_metadata: *2 'null': false @@ -90,32 +136,7 @@ columns: default_function: collation: comment: - accounts: - - *5 - - *6 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: join_code - cast_type: *7 - sql_type_metadata: *8 - 'null': true - default: - default_function: - collation: - comment: - - &10 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: name - cast_type: *7 - sql_type_metadata: *8 - 'null': false - default: - default_function: - collation: - comment: - - *9 - action_text_markdowns: - - *5 + - *10 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: content @@ -136,41 +157,22 @@ columns: default_function: collation: comment: - - *6 - - *10 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: record_id - cast_type: *1 - sql_type_metadata: *2 - 'null': false - default: - default_function: - collation: - comment: - - &13 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: record_type - cast_type: *7 - sql_type_metadata: *8 - 'null': false - default: - default_function: - collation: - comment: + - *8 - *9 active_storage_attachments: - *5 - - &16 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - *10 + - *11 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: blob_id - cast_type: &11 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3Adapter::SQLite3Integer + name: record_id + cast_type: &12 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3Adapter::SQLite3Integer precision: scale: limit: max: 9223372036854775808 min: -9223372036854775808 - sql_type_metadata: &12 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata + sql_type_metadata: &13 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata sql_type: bigint type: :integer limit: @@ -181,24 +183,22 @@ columns: default_function: collation: comment: - - *6 - - *10 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &16 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: record_id - cast_type: *11 - sql_type_metadata: *12 + name: blob_id + cast_type: *12 + sql_type_metadata: *13 'null': false default: default_function: collation: comment: - - *13 + - *8 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: slug - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': true default: default_function: @@ -206,11 +206,11 @@ columns: comment: active_storage_blobs: - *5 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &17 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: byte_size - cast_type: *11 - sql_type_metadata: *12 + name: key + cast_type: *6 + sql_type_metadata: *7 'null': false default: default_function: @@ -218,10 +218,10 @@ columns: comment: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: checksum - cast_type: *7 - sql_type_metadata: *8 - 'null': true + name: filename + cast_type: *6 + sql_type_metadata: *7 + 'null': false default: default_function: collation: @@ -229,34 +229,13 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: content_type - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': true default: default_function: collation: comment: - - *6 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: filename - cast_type: *7 - sql_type_metadata: *8 - 'null': false - default: - default_function: - collation: - comment: - - &17 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: key - cast_type: *7 - sql_type_metadata: *8 - 'null': false - default: - default_function: - collation: - comment: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: metadata @@ -270,21 +249,42 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: service_name - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': false default: default_function: collation: comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: byte_size + cast_type: *12 + sql_type_metadata: *13 + 'null': false + default: + default_function: + collation: + comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: checksum + cast_type: *6 + sql_type_metadata: *7 + 'null': true + default: + default_function: + collation: + comment: + - *8 active_storage_variant_records: - *5 - *16 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: variation_digest - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': false default: default_function: @@ -295,26 +295,16 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: value - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': true default: default_function: collation: comment: - - *6 + - *8 - *9 assignees_filters: - - &19 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: assignee_id - cast_type: *1 - sql_type_metadata: *2 - 'null': false - default: - default_function: - collation: - comment: - &18 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: filter_id @@ -325,7 +315,18 @@ columns: default_function: collation: comment: + - &19 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: assignee_id + cast_type: *1 + sql_type_metadata: *2 + 'null': false + default: + default_function: + collation: + comment: assigners_filters: + - *18 - &20 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: assigner_id @@ -336,11 +337,9 @@ columns: default_function: collation: comment: - - *18 assignments: - *5 - *19 - - *20 - &21 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: card_id @@ -351,8 +350,9 @@ columns: default_function: collation: comment: - - *6 + - *8 - *9 + - *20 card_engagements: - *5 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column @@ -365,17 +365,27 @@ columns: default_function: collation: comment: - - *6 + - *8 - *9 card_goldnesses: - *5 - *21 - - *6 + - *8 - *9 cards: - *5 - - *22 - - *6 + - &30 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: title + cast_type: *6 + sql_type_metadata: *7 + 'null': true + default: + default_function: + collation: + comment: + - *8 + - *9 - &23 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: creator_id @@ -405,16 +415,7 @@ columns: default_function: collation: comment: - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: last_active_at - cast_type: *3 - sql_type_metadata: *4 - 'null': false - default: - default_function: - collation: - comment: + - *22 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: stage_id @@ -435,46 +436,33 @@ columns: default_function: collation: comment: - - &30 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: title - cast_type: *7 - sql_type_metadata: *8 - 'null': true - default: - default_function: - collation: - comment: - - *9 - closure_reasons: - - *5 - - *6 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: label - cast_type: *7 - sql_type_metadata: *8 - 'null': true - default: - default_function: - collation: - comment: - - *9 - closures: - - *5 - - *21 - - *6 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: reason - cast_type: *7 - sql_type_metadata: *8 + name: last_active_at + cast_type: *3 + sql_type_metadata: *4 'null': false default: default_function: collation: comment: + closure_reasons: + - *5 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: label + cast_type: *6 + sql_type_metadata: *7 + 'null': true + default: + default_function: + collation: + comment: + - *8 - *9 + closures: + - *5 + - *21 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: user_id @@ -485,8 +473,24 @@ columns: default_function: collation: comment: + - *8 + - *9 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: reason + cast_type: *6 + sql_type_metadata: *7 + 'null': false + default: + default_function: + collation: + comment: collections: - *5 + - *23 + - *10 + - *8 + - *9 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: all_access @@ -505,10 +509,6 @@ columns: default_function: collation: comment: - - *6 - - *23 - - *10 - - *9 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: workflow_id @@ -520,45 +520,23 @@ columns: collation: comment: collections_filters: - - *22 - *18 + - *22 comments: - *5 - - *6 - *23 + - *8 - *9 creators_filters: - - *23 - *18 + - *23 event_summaries: - *5 - - *6 + - *8 - *9 events: - *5 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: action - cast_type: *7 - sql_type_metadata: *8 - 'null': false - default: - default_function: - collation: - comment: - - *21 - - *6 - *23 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: due_date - cast_type: *24 - sql_type_metadata: *25 - 'null': true - default: - default_function: - collation: - comment: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: particulars @@ -577,6 +555,18 @@ columns: default_function: collation: comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: action + cast_type: *6 + sql_type_metadata: *7 + 'null': false + default: + default_function: + collation: + comment: + - *8 + - *9 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: summary_id @@ -587,11 +577,32 @@ columns: default_function: collation: comment: - - *9 + - *21 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: due_date + cast_type: *24 + sql_type_metadata: *25 + 'null': true + default: + default_function: + collation: + comment: filters: - *5 - - *6 - *23 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: params_digest + cast_type: *6 + sql_type_metadata: *7 + 'null': false + default: + default_function: + collation: + comment: + - *8 + - *9 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: fields @@ -602,17 +613,6 @@ columns: default_function: collation: comment: - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: params_digest - cast_type: *7 - sql_type_metadata: *8 - 'null': false - default: - default_function: - collation: - comment: - - *9 filters_stages: - *18 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column @@ -642,8 +642,8 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: container_type - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': false default: default_function: @@ -669,22 +669,22 @@ columns: default_function: collation: comment: + - *23 + - *8 + - *9 + messages: + - *5 + - *21 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: mentioner_id - cast_type: *1 - sql_type_metadata: *2 + name: messageable_type + cast_type: *6 + sql_type_metadata: *7 'null': false default: default_function: collation: comment: - - *6 - - *9 - messages: - - *5 - - *21 - - *6 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: messageable_id @@ -695,26 +695,17 @@ columns: 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: + - *8 - *9 notifications: - *5 - - *6 + - *28 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: read_at - cast_type: *3 - sql_type_metadata: *4 - 'null': true + name: resource_type + cast_type: *6 + sql_type_metadata: *7 + 'null': false default: default_function: collation: @@ -729,23 +720,23 @@ columns: default_function: collation: comment: + - *8 + - *9 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: resource_type - cast_type: *7 - sql_type_metadata: *8 - 'null': false + name: read_at + cast_type: *3 + sql_type_metadata: *4 + 'null': true default: default_function: collation: comment: - - *9 - - *28 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: source_type - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': false default: default_function: @@ -764,9 +755,9 @@ columns: pins: - *5 - *21 - - *6 - - *9 - *28 + - *8 + - *9 reactions: - *5 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column @@ -779,6 +770,16 @@ columns: default_function: collation: comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: reacter_id + cast_type: *1 + sql_type_metadata: *2 + 'null': false + default: + default_function: + collation: + comment: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: content @@ -799,24 +800,14 @@ columns: default_function: collation: comment: - - *6 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: reacter_id - cast_type: *1 - sql_type_metadata: *2 - 'null': false - default: - default_function: - collation: - comment: + - *8 - *9 schema_migrations: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: version - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': false default: default_function: @@ -824,42 +815,63 @@ columns: comment: sessions: - *5 - - *6 + - *28 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: ip_address - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': true default: default_function: collation: comment: - - *9 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: user_agent - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': true default: default_function: collation: comment: - - *28 + - *8 + - *9 taggings: - *5 - *21 - - *6 - *29 + - *8 - *9 tags: - *5 - - *6 - *30 + - *8 - *9 users: - *5 + - *10 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: email_address + cast_type: *6 + sql_type_metadata: *7 + 'null': true + default: + default_function: + collation: + comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: password_digest + cast_type: *6 + sql_type_metadata: *7 + 'null': true + default: + default_function: + collation: + comment: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: active @@ -870,45 +882,22 @@ columns: default_function: collation: comment: - - *6 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: email_address - cast_type: *7 - sql_type_metadata: *8 - 'null': true - default: - default_function: - collation: - comment: - - *10 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: password_digest - cast_type: *7 - sql_type_metadata: *8 - 'null': true - default: - default_function: - collation: - comment: + - *8 + - *9 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: role - cast_type: *7 - sql_type_metadata: *8 + cast_type: *6 + sql_type_metadata: *7 'null': false default: member default_function: collation: comment: - - *9 watches: - *5 - - *21 - - *6 - - *9 - *28 + - *21 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: watching @@ -919,21 +908,10 @@ columns: default_function: collation: comment: + - *8 + - *9 workflow_stages: - *5 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: color - cast_type: *7 - sql_type_metadata: *8 - 'null': true - default: - default_function: - collation: - comment: - - *6 - - *10 - - *9 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: workflow_id @@ -944,10 +922,23 @@ columns: default_function: collation: comment: + - *10 + - *8 + - *9 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: color + cast_type: *6 + sql_type_metadata: *7 + 'null': true + default: + default_function: + collation: + comment: workflows: - *5 - - *6 - *10 + - *8 - *9 primary_keys: accesses: id @@ -1029,9 +1020,10 @@ indexes: accesses: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: accesses - name: index_accesses_on_user_id - unique: false + name: index_accesses_on_collection_id_and_user_id + unique: true columns: + - collection_id - user_id lengths: {} orders: {} @@ -1061,10 +1053,9 @@ indexes: valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: accesses - name: index_accesses_on_collection_id_and_user_id - unique: true + name: index_accesses_on_user_id + unique: false columns: - - collection_id - user_id lengths: {} orders: {} @@ -1184,22 +1175,6 @@ indexes: valid: true ar_internal_metadata: [] assignees_filters: - - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: assignees_filters - name: index_assignees_filters_on_filter_id - unique: false - columns: - - filter_id - lengths: {} - orders: {} - opclasses: {} - where: - type: - using: - include: - nulls_not_distinct: - comment: - valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: assignees_filters name: index_assignees_filters_on_assignee_id @@ -1216,10 +1191,9 @@ indexes: nulls_not_distinct: comment: valid: true - assigners_filters: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: assigners_filters - name: index_assigners_filters_on_filter_id + table: assignees_filters + name: index_assignees_filters_on_filter_id unique: false columns: - filter_id @@ -1233,6 +1207,7 @@ indexes: nulls_not_distinct: comment: valid: true + assigners_filters: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: assigners_filters name: index_assigners_filters_on_assigner_id @@ -1249,6 +1224,22 @@ indexes: nulls_not_distinct: comment: valid: true + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: assigners_filters + name: index_assigners_filters_on_filter_id + unique: false + columns: + - filter_id + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true assignments: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: assignments @@ -1318,6 +1309,22 @@ indexes: comment: valid: true cards: + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: cards + name: index_cards_on_collection_id + unique: false + columns: + - collection_id + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: cards name: index_cards_on_stage_id @@ -1351,30 +1358,15 @@ indexes: nulls_not_distinct: comment: valid: true - - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: cards - name: index_cards_on_collection_id - unique: false - columns: - - collection_id - lengths: {} - orders: {} - opclasses: {} - where: - type: - using: - include: - nulls_not_distinct: - comment: - valid: true closure_reasons: [] closures: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: closures - name: index_closures_on_user_id + name: index_closures_on_card_id_and_created_at unique: false columns: - - user_id + - card_id + - created_at lengths: {} orders: {} opclasses: {} @@ -1403,11 +1395,10 @@ indexes: valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: closures - name: index_closures_on_card_id_and_created_at + name: index_closures_on_user_id unique: false columns: - - card_id - - created_at + - user_id lengths: {} orders: {} opclasses: {} @@ -1419,22 +1410,6 @@ indexes: comment: valid: true collections: - - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: collections - name: index_collections_on_workflow_id - unique: false - columns: - - workflow_id - lengths: {} - orders: {} - opclasses: {} - where: - type: - using: - include: - nulls_not_distinct: - comment: - valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: collections name: index_collections_on_creator_id @@ -1451,13 +1426,12 @@ indexes: nulls_not_distinct: comment: valid: true - collections_filters: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: collections_filters - name: index_collections_filters_on_filter_id + table: collections + name: index_collections_on_workflow_id unique: false columns: - - filter_id + - workflow_id lengths: {} orders: {} opclasses: {} @@ -1468,6 +1442,7 @@ indexes: nulls_not_distinct: comment: valid: true + collections_filters: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: collections_filters name: index_collections_filters_on_collection_id @@ -1484,11 +1459,9 @@ indexes: nulls_not_distinct: comment: valid: true - comments: [] - creators_filters: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: creators_filters - name: index_creators_filters_on_filter_id + table: collections_filters + name: index_collections_filters_on_filter_id unique: false columns: - filter_id @@ -1502,6 +1475,8 @@ indexes: nulls_not_distinct: comment: valid: true + comments: [] + creators_filters: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: creators_filters name: index_creators_filters_on_creator_id @@ -1518,8 +1493,40 @@ indexes: nulls_not_distinct: comment: valid: true + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: creators_filters + name: index_creators_filters_on_filter_id + unique: false + columns: + - filter_id + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true event_summaries: [] events: + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: events + name: index_events_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: events name: index_events_on_summary_id_and_action @@ -1553,22 +1560,6 @@ indexes: nulls_not_distinct: comment: valid: true - - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: events - name: index_events_on_card_id - unique: false - columns: - - card_id - lengths: {} - orders: {} - opclasses: {} - where: - type: - using: - include: - nulls_not_distinct: - comment: - valid: true filters: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: filters @@ -1656,10 +1647,10 @@ indexes: mentions: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: mentions - name: index_mentions_on_mentioner_id + name: index_mentions_on_creator_id unique: false columns: - - mentioner_id + - creator_id lengths: {} orders: {} opclasses: {} @@ -1706,11 +1697,10 @@ indexes: messages: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: messages - name: index_messages_on_messageable - unique: true + name: index_messages_on_card_id + unique: false columns: - - messageable_type - - messageable_id + - card_id lengths: {} orders: {} opclasses: {} @@ -1723,10 +1713,11 @@ indexes: valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: messages - name: index_messages_on_card_id - unique: false + name: index_messages_on_messageable + unique: true columns: - - card_id + - messageable_type + - messageable_id lengths: {} orders: {} opclasses: {} @@ -1755,22 +1746,6 @@ indexes: nulls_not_distinct: comment: valid: true - - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: notifications - name: index_notifications_on_user_id - unique: false - columns: - - user_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_user_id_and_read_at_and_created_at @@ -1808,10 +1783,9 @@ indexes: nulls_not_distinct: comment: valid: true - pins: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: pins - name: index_pins_on_user_id + table: notifications + name: index_notifications_on_user_id unique: false columns: - user_id @@ -1825,6 +1799,7 @@ indexes: nulls_not_distinct: comment: valid: true + pins: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: pins name: index_pins_on_card_id @@ -1858,6 +1833,22 @@ indexes: nulls_not_distinct: comment: valid: true + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: pins + name: index_pins_on_user_id + unique: false + columns: + - user_id + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true reactions: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: reactions @@ -1912,9 +1903,10 @@ indexes: taggings: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: taggings - name: index_taggings_on_tag_id - unique: false + name: index_taggings_on_card_id_and_tag_id + unique: true columns: + - card_id - tag_id lengths: {} orders: {} @@ -1928,10 +1920,9 @@ indexes: valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: taggings - name: index_taggings_on_card_id_and_tag_id - unique: true + name: index_taggings_on_tag_id + unique: false columns: - - card_id - tag_id lengths: {} orders: {} @@ -1980,10 +1971,10 @@ indexes: watches: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: watches - name: index_watches_on_user_id + name: index_watches_on_card_id unique: false columns: - - user_id + - card_id lengths: {} orders: {} opclasses: {} @@ -1996,10 +1987,10 @@ indexes: valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: watches - name: index_watches_on_card_id + name: index_watches_on_user_id unique: false columns: - - card_id + - user_id lengths: {} orders: {} opclasses: {} diff --git a/test/models/notifier_test.rb b/test/models/notifier_test.rb index 2e915bded..987ac84f2 100644 --- a/test/models/notifier_test.rb +++ b/test/models/notifier_test.rb @@ -14,7 +14,13 @@ class NotifierTest < ActiveSupport::TestCase end end - test "creates a notification for each watcher, other than the event creator" do + test "creates a notification for each watcher, other than the event creator (events)" do + notifications = Notifier.for(events(:layout_commented)).notify + + assert_equal [ users(:kevin) ], notifications.map(&:user) + end + + test "creates a notification for each watcher (mentions)" do notifications = Notifier.for(events(:layout_commented)).notify assert_equal [ users(:kevin) ], notifications.map(&:user) From 8aa99e34b41cb3bddb6212e0a810decad6a2d300 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 13:48:43 +0200 Subject: [PATCH 03/50] 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 From 22df208f9a97419bb8aeff16bf0e93d2220f3c44 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 14:11:21 +0200 Subject: [PATCH 04/50] Render proper content for mentions, don't send mentions to yourself --- app/models/card/mentions.rb | 7 +++---- app/models/mention.rb | 4 ++++ app/models/message/mentions.rb | 7 +++---- app/models/notifier.rb | 6 +++--- app/models/notifier/events/base.rb | 4 ---- app/models/notifier/mention.rb | 6 +++++- .../notification/_mention.html.erb | 6 ++++-- ...50422112857_add_creator_to_notifications.rb | 18 ++++++++++++++++++ db/schema.rb | 2 +- db/schema_cache.yml | 11 ++++++++++- test/models/user/mentionable_test.rb | 2 +- 11 files changed, 52 insertions(+), 21 deletions(-) create mode 100644 db/migrate/20250422112857_add_creator_to_notifications.rb diff --git a/app/models/card/mentions.rb b/app/models/card/mentions.rb index dd135f24c..79334b240 100644 --- a/app/models/card/mentions.rb +++ b/app/models/card/mentions.rb @@ -5,8 +5,7 @@ module Card::Mentions include ::Mentions end - private - def mentionable_content - description.to_plain_text - end + def mentionable_content + description.to_plain_text + end end diff --git a/app/models/mention.rb b/app/models/mention.rb index 515c3071c..fd70e075a 100644 --- a/app/models/mention.rb +++ b/app/models/mention.rb @@ -4,4 +4,8 @@ class Mention < ApplicationRecord belongs_to :container, polymorphic: true belongs_to :mentioner, class_name: "User" belongs_to :mentionee, class_name: "User", inverse_of: :mentions + + def self_mention? + mentioner == mentionee + end end diff --git a/app/models/message/mentions.rb b/app/models/message/mentions.rb index fc8282413..f42ac5e1b 100644 --- a/app/models/message/mentions.rb +++ b/app/models/message/mentions.rb @@ -5,8 +5,7 @@ module Message::Mentions include ::Mentions end - private - def mentionable_content - messageable.try(:body_plain_text) - end + def mentionable_content + messageable.try(:body_plain_text) + end end diff --git a/app/models/notifier.rb b/app/models/notifier.rb index f1a06a0ed..72c16ff3d 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -4,9 +4,9 @@ class Notifier class << self def for(source) case source - when Event + when Event "Notifier::Events::#{source.action.classify}".safe_constantize&.new(source) - when ::Mention + when ::Mention Notifier::Mention.new(source) end end @@ -26,7 +26,7 @@ class Notifier end def should_notify? - true + !creator.system? end def resource diff --git a/app/models/notifier/events/base.rb b/app/models/notifier/events/base.rb index 13a915098..6852ba2b2 100644 --- a/app/models/notifier/events/base.rb +++ b/app/models/notifier/events/base.rb @@ -2,10 +2,6 @@ class Notifier::Events::Base < Notifier delegate :card, :creator, to: :source private - def should_notify? - !creator.system? - end - def resource card end diff --git a/app/models/notifier/mention.rb b/app/models/notifier/mention.rb index a35e0f4e4..2085c42ed 100644 --- a/app/models/notifier/mention.rb +++ b/app/models/notifier/mention.rb @@ -5,7 +5,11 @@ class Notifier::Mention < Notifier end def recipients - [ source.mentionee ] + if source.self_mention? + [] + else + [ source.mentionee ] + end end def creator diff --git a/app/views/notifications/notification/_mention.html.erb b/app/views/notifications/notification/_mention.html.erb index 5dc61b1af..96abde715 100644 --- a/app/views/notifications/notification/_mention.html.erb +++ b/app/views/notifications/notification/_mention.html.erb @@ -1,5 +1,7 @@ -<%= "OMG" %> +<% mention = notification.source %> + +<%= mention.mentioner.first_name %> mentioned you
- YOU WERE MENTIONED! + <%= mention.container.mentionable_content.truncate(200) %>
<%= local_datetime_tag(notification.created_at, style: :ago) %>
diff --git a/db/migrate/20250422112857_add_creator_to_notifications.rb b/db/migrate/20250422112857_add_creator_to_notifications.rb new file mode 100644 index 000000000..63635565c --- /dev/null +++ b/db/migrate/20250422112857_add_creator_to_notifications.rb @@ -0,0 +1,18 @@ +class AddCreatorToNotifications < ActiveRecord::Migration[8.1] + def change + add_reference :notifications, :creator, null: true, foreign_key: { to_table: :users } + + execute <<~SQL + UPDATE notifications + SET creator_id = ( + SELECT events.creator_id + FROM events + WHERE events.id = notifications.source_id + AND notifications.source_type = 'Event' + ) + WHERE source_type = 'Event'; + SQL + + change_column_null :notifications, :creator_id, true + end +end diff --git a/db/schema.rb b/db/schema.rb index 2da27d16f..2dbc1c529 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -236,7 +236,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_22_112857) do create_table "notifications", force: :cascade do |t| t.datetime "created_at", null: false - t.integer "creator_id", null: false + t.integer "creator_id" t.datetime "read_at" t.integer "resource_id", null: false t.string "resource_type", null: false diff --git a/db/schema_cache.yml b/db/schema_cache.yml index 73d81743b..eb67d0f45 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -761,7 +761,16 @@ columns: default_function: collation: comment: - - *23 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: creator_id + cast_type: *1 + sql_type_metadata: *2 + 'null': true + default: + default_function: + collation: + comment: pins: - *5 - *21 diff --git a/test/models/user/mentionable_test.rb b/test/models/user/mentionable_test.rb index 43e70792c..48fb339a1 100644 --- a/test/models/user/mentionable_test.rb +++ b/test/models/user/mentionable_test.rb @@ -1,4 +1,4 @@ -require 'test_helper' +require "test_helper" class User::MentionableTest < ActiveSupport::TestCase test "mentionable handles" do From 9db675e589c06e89aa6b64b427cbaabb4ef41de4 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 14:15:40 +0200 Subject: [PATCH 05/50] Format --- app/models/notifier.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 72c16ff3d..ab63d62ff 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -5,9 +5,9 @@ class Notifier def for(source) case source when Event - "Notifier::Events::#{source.action.classify}".safe_constantize&.new(source) + "Notifier::Events::#{source.action.classify}".safe_constantize&.new(source) when ::Mention - Notifier::Mention.new(source) + Notifier::Mention.new(source) end end end From d5f0359338421f9f35a203f330b708201191dcc2 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 14:19:45 +0200 Subject: [PATCH 06/50] Extract mentionable content using reflection to remove duplicated logic and the need for additional concerns --- app/models/card/mentions.rb | 11 ----------- app/models/concerns/mentions.rb | 6 ++++++ app/models/message/mentions.rb | 11 ----------- 3 files changed, 6 insertions(+), 22 deletions(-) delete mode 100644 app/models/card/mentions.rb delete mode 100644 app/models/message/mentions.rb diff --git a/app/models/card/mentions.rb b/app/models/card/mentions.rb deleted file mode 100644 index 79334b240..000000000 --- a/app/models/card/mentions.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Card::Mentions - extend ActiveSupport::Concern - - included do - include ::Mentions - end - - def mentionable_content - description.to_plain_text - end -end diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index 8de010a98..fb891ef75 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -12,6 +12,12 @@ module Mentions end end + def mentionable_content + self.class.reflect_on_all_associations(:has_one).filter { it.klass == ActionText::Markdown }.collect do |association| + send(association.name).to_plain_text + end.join(" ") + end + private def collect_mentions_later Mention::CollectJob.perform_later(self, mentioner: Current.user) diff --git a/app/models/message/mentions.rb b/app/models/message/mentions.rb deleted file mode 100644 index f42ac5e1b..000000000 --- a/app/models/message/mentions.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Message::Mentions - extend ActiveSupport::Concern - - included do - include ::Mentions - end - - def mentionable_content - messageable.try(:body_plain_text) - end -end From beac61699c1ed11ff550fbdf1083246dffde89a3 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 14:21:26 +0200 Subject: [PATCH 07/50] Not needed --- app/models/concerns/mentions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index fb891ef75..4bcd458a6 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -34,6 +34,6 @@ module Mentions end def scan_mentioned_handles - (mentionable_content || "").scan(/(? Date: Tue, 22 Apr 2025 14:22:23 +0200 Subject: [PATCH 08/50] Rename to match actual action --- app/helpers/notifications_helper.rb | 2 +- app/models/event.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 23ba12b38..1bbd3b508 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -2,7 +2,7 @@ module NotificationsHelper def event_notification_title(event) if event.commented? "RE: " + event.card.title - elsif event.assignment? + elsif event.assigned? "Assigned to you: " + event.card.title else event.card.title diff --git a/app/models/event.rb b/app/models/event.rb index 3fceff6d1..1131bf5e0 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -12,7 +12,7 @@ class Event < ApplicationRecord after_create -> { card.touch(:last_active_at) } - def assignment? + def assigned? action == "assigned" || initial_assignment? end From c75d4cb3294553001903673d83eb17a9ae0323fd Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 15:26:56 +0200 Subject: [PATCH 09/50] Fix broken permalinks --- app/views/cards/comments/_comment.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/cards/comments/_comment.html.erb b/app/views/cards/comments/_comment.html.erb index 427fae8ad..2881ee438 100644 --- a/app/views/cards/comments/_comment.html.erb +++ b/app/views/cards/comments/_comment.html.erb @@ -11,7 +11,7 @@ <%= link_to comment.creator.name, comment.creator, class: "txt-ink btn btn--plain fill-transparent", data: { turbo_frame: "_top" } %> - <%= link_to card_path(comment.card, anchor: "comment_#{comment.id}"), class: "txt-undecorated txt-uppercase" do %> + <%= link_to collection_card_path(comment.card.collection, comment.card, anchor: "comment_#{comment.id}"), class: "txt-undecorated txt-uppercase" do %> <%= local_datetime_tag comment.created_at, style: :shortdate, class: "txt-ink translucent" %> <% end %> From 81961a3523d2c1aaa30e1d817ccc98e4909f2514 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 15:30:03 +0200 Subject: [PATCH 10/50] Fix unclosed tag --- app/views/events/_day.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/events/_day.html.erb b/app/views/events/_day.html.erb index 416d67e63..7426e1cc8 100644 --- a/app/views/events/_day.html.erb +++ b/app/views/events/_day.html.erb @@ -2,7 +2,7 @@ data-related-element-highlight-class="event--related"> <% if events.any? %>

- <%= event_day_title(activity_day) %> + <%= event_day_title(activity_day) %>

From 9b4fec936edba460230274926d303cda4e46ff33 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 16:17:37 +0200 Subject: [PATCH 11/50] Move mentions to comments - It does not make sense to scan "event summaries" for notifications. - Placing them at the message level prevents us from using the generic approach to extract the mentionable content --- app/helpers/notifications_helper.rb | 13 ++++++++++--- app/models/comment.rb | 2 +- app/models/concerns/messageable.rb | 2 ++ app/models/message.rb | 4 ---- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 1bbd3b508..b5c802ecf 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -25,10 +25,8 @@ module NotificationsHelper def notification_tag(notification, &) tag.div id: dom_id(notification), class: "notification tray__item border-radius txt-normal" do - # TODO: Temporary, I'll remove this. Right now, we support linking comments, but we should be linking messages. - resource = notification.resource.is_a?(Message) ? notification.resource.comment : notification.resource concat( - link_to(resource, + link_to(notification_resource_path(notification), class: "notification__content border-radius shadow fill-white flex align-start txt-align-start gap flex-item-grow max-width border txt-ink", data: { action: "click->dialog#close", turbo_frame: "_top" }, &) @@ -37,6 +35,15 @@ module NotificationsHelper end end + def notification_resource_path(notification) + if notification.resource.is_a?(Comment) + # TODO: Extract a direct path for these + collection_card_path(notification.resource.card.collection, notification.resource.card, anchor: "comment_#{notification.resource.id}") + else + notification.resource + end + end + def notification_mark_read_button(notification) button_to read_notification_path(notification), class: "notification__unread_indicator btn borderless", diff --git a/app/models/comment.rb b/app/models/comment.rb index 8922b30b7..630589e41 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -1,5 +1,5 @@ class Comment < ApplicationRecord - include Messageable, Searchable + include Mentions, Messageable, Searchable belongs_to :creator, class_name: "User", default: -> { Current.user } has_many :reactions, dependent: :delete_all diff --git a/app/models/concerns/messageable.rb b/app/models/concerns/messageable.rb index 219ee1197..d4c4f83fe 100644 --- a/app/models/concerns/messageable.rb +++ b/app/models/concerns/messageable.rb @@ -6,5 +6,7 @@ module Messageable included do has_one :message, as: :messageable, touch: true, dependent: :destroy has_one :card, through: :message + + delegate :collection, to: :message end end diff --git a/app/models/message.rb b/app/models/message.rb index 2947c99b5..7d950b3b7 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -1,11 +1,7 @@ class Message < ApplicationRecord - include Mentions - belongs_to :card, touch: true delegated_type :messageable, types: Messageable::TYPES, inverse_of: :message, dependent: :destroy scope :chronologically, -> { order created_at: :asc, id: :desc } - - delegate :collection, to: :card end From a567e662e21751483dd9bb0595afca73ec505422 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 17:23:47 +0200 Subject: [PATCH 12/50] Fix: wrong delegation --- app/models/concerns/messageable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/messageable.rb b/app/models/concerns/messageable.rb index d4c4f83fe..d3dbca06c 100644 --- a/app/models/concerns/messageable.rb +++ b/app/models/concerns/messageable.rb @@ -7,6 +7,6 @@ module Messageable has_one :message, as: :messageable, touch: true, dependent: :destroy has_one :card, through: :message - delegate :collection, to: :message + delegate :collection, to: :card end end From cdadc0373a7d847ef0343ba6010906c027c21117 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 22 Apr 2025 18:10:27 +0200 Subject: [PATCH 13/50] Don't trigger unless the mentionable content changes This is also needed to prevent incorrect triggering on cascade touches! --- app/models/concerns/mentions.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index 4bcd458a6..82a9aa093 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -3,7 +3,8 @@ module Mentions included do has_many :mentions, as: :container, dependent: :destroy - after_commit :collect_mentions_later, on: %i[ create update ] + before_save :save_mentionable_content_before_save + after_commit :collect_mentions_later, on: %i[ create update ], if: :mentionable_content_changed? end def collect_mentions(mentioner: Current.user) @@ -19,10 +20,18 @@ module Mentions end private + def save_mentionable_content_before_save + @mentionable_content_before_safe = self.class.find(id).mentionable_content unless new_record? + end + def collect_mentions_later Mention::CollectJob.perform_later(self, mentioner: Current.user) end + def mentionable_content_changed? + @mentionable_content_before_safe != mentionable_content + end + def scan_mentionees scan_mentioned_handles.filter_map do |mention| mentionable_users.find { |user| user.mentionable_handles.include?(mention) } From c3cd873588845252775b7a1a0bd3d88a758d0d29 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 10:30:30 +0200 Subject: [PATCH 14/50] Rename collect => create --- app/jobs/mention/collect_job.rb | 7 ------- app/jobs/mention/create_job.rb | 7 +++++++ app/models/concerns/mentions.rb | 8 ++++---- test/models/concerns/mentions_test.rb | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) delete mode 100644 app/jobs/mention/collect_job.rb create mode 100644 app/jobs/mention/create_job.rb diff --git a/app/jobs/mention/collect_job.rb b/app/jobs/mention/collect_job.rb deleted file mode 100644 index 088e240dd..000000000 --- a/app/jobs/mention/collect_job.rb +++ /dev/null @@ -1,7 +0,0 @@ -class Mention::CollectJob < ApplicationJob - queue_as :default - - def perform(record, mentioner:) - record.collect_mentions(mentioner:) - end -end diff --git a/app/jobs/mention/create_job.rb b/app/jobs/mention/create_job.rb new file mode 100644 index 000000000..364ea1272 --- /dev/null +++ b/app/jobs/mention/create_job.rb @@ -0,0 +1,7 @@ +class Mention::CreateJob < ApplicationJob + queue_as :default + + def perform(record, mentioner:) + record.create_mentions(mentioner:) + end +end diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index 82a9aa093..e7bc3972b 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -4,10 +4,10 @@ module Mentions included do has_many :mentions, as: :container, dependent: :destroy before_save :save_mentionable_content_before_save - after_commit :collect_mentions_later, on: %i[ create update ], if: :mentionable_content_changed? + after_save_commit :create_mentions_later, if: :mentionable_content_changed? end - def collect_mentions(mentioner: Current.user) + def create_mentions(mentioner: Current.user) scan_mentionees.each do |mentionee| mentionee.mentioned_by mentioner, at: self end @@ -24,8 +24,8 @@ module Mentions @mentionable_content_before_safe = self.class.find(id).mentionable_content unless new_record? end - def collect_mentions_later - Mention::CollectJob.perform_later(self, mentioner: Current.user) + def create_mentions_later + Mention::CreateJob.perform_later(self, mentioner: Current.user) end def mentionable_content_changed? diff --git a/test/models/concerns/mentions_test.rb b/test/models/concerns/mentions_test.rb index 3ed1c4d49..b9e5dc751 100644 --- a/test/models/concerns/mentions_test.rb +++ b/test/models/concerns/mentions_test.rb @@ -7,7 +7,7 @@ class MentionsTest < ActiveSupport::TestCase test "collect mentions on create" do assert_difference -> { Mention.count }, +1 do - perform_enqueued_jobs only: Mention::CollectJob do + perform_enqueued_jobs only: Mention::CreateJob do collections(:writebook).cards.create title: "Cleanup", description: "Did you finish up with the cleanup, @david?" end end @@ -15,7 +15,7 @@ class MentionsTest < ActiveSupport::TestCase test "collect mentions on update" do assert_difference -> { Mention.count }, +1 do - perform_enqueued_jobs only: Mention::CollectJob do + perform_enqueued_jobs only: Mention::CreateJob do cards(:logo).update! description: "Did you finish up with the cleanup, @david?" end end From af8701567ab2d80ff7b1307fcf1f156ae0d7e80a Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 10:35:18 +0200 Subject: [PATCH 15/50] Extract method --- app/models/concerns/mentions.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index e7bc3972b..b24627672 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -14,12 +14,16 @@ module Mentions end def mentionable_content - self.class.reflect_on_all_associations(:has_one).filter { it.klass == ActionText::Markdown }.collect do |association| + markdown_associations.collect do |association| send(association.name).to_plain_text end.join(" ") end private + def markdown_associations + self.class.reflect_on_all_associations(:has_one).filter { it.klass == ActionText::Markdown } + end + def save_mentionable_content_before_save @mentionable_content_before_safe = self.class.find(id).mentionable_content unless new_record? end From 68e2e9607657bea5aeb7613bd80e3c5af4a95f0b Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 10:44:42 +0200 Subject: [PATCH 16/50] Fix indentation --- app/helpers/notifications_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index b5c802ecf..613776bac 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -46,9 +46,9 @@ module NotificationsHelper def notification_mark_read_button(notification) button_to read_notification_path(notification), - class: "notification__unread_indicator btn borderless", - title: "Mark as read", - data: { turbo_frame: "_top" } do + class: "notification__unread_indicator btn borderless", + title: "Mark as read", + data: { turbo_frame: "_top" } do concat(image_tag("remove-med.svg", class: "unread_icon", size: 12, aria: { hidden: true })) concat(tag.span("Mark as read", class: "for-screen-reader")) end From b510ca031879dd5829338dbe67df3f4e32e283b6 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 10:56:00 +0200 Subject: [PATCH 17/50] Don't choke for nil values --- app/models/concerns/mentions.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index b24627672..35d1a42f8 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -15,8 +15,8 @@ module Mentions def mentionable_content markdown_associations.collect do |association| - send(association.name).to_plain_text - end.join(" ") + send(association.name)&.to_plain_text + end.compact.join(" ") end private From ab54cafd84489cbaa449f7f660ecc63537964988 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 11:06:43 +0200 Subject: [PATCH 18/50] Inline event notifiers --- app/models/event.rb | 5 ++++- app/models/notifier.rb | 8 ++++---- app/models/notifier/event.rb | 23 +++++++++++++++++++++++ app/models/notifier/events/assigned.rb | 6 ------ app/models/notifier/events/base.rb | 12 ------------ app/models/notifier/events/closed.rb | 2 -- app/models/notifier/events/commented.rb | 6 ------ app/models/notifier/events/published.rb | 6 ------ test/models/notifier_test.rb | 2 +- 9 files changed, 32 insertions(+), 38 deletions(-) create mode 100644 app/models/notifier/event.rb delete mode 100644 app/models/notifier/events/assigned.rb delete mode 100644 app/models/notifier/events/base.rb delete mode 100644 app/models/notifier/events/closed.rb delete mode 100644 app/models/notifier/events/commented.rb delete mode 100644 app/models/notifier/events/published.rb diff --git a/app/models/event.rb b/app/models/event.rb index 1131bf5e0..5eb095039 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -16,7 +16,10 @@ class Event < ApplicationRecord action == "assigned" || initial_assignment? end - # E.g: completed? is true if action == "completed" + def action + super.inquiry + end + def method_missing(method_name, *args, &block) if method_name.to_s.end_with?("?") action == method_name.to_s.chomp("?") diff --git a/app/models/notifier.rb b/app/models/notifier.rb index ab63d62ff..651d6898f 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -4,10 +4,10 @@ class Notifier class << self def for(source) case source - when Event - "Notifier::Events::#{source.action.classify}".safe_constantize&.new(source) - when ::Mention - Notifier::Mention.new(source) + when ::Event + Notifier::Event.new(source) + when ::Mention + Notifier::Mention.new(source) end end end diff --git a/app/models/notifier/event.rb b/app/models/notifier/event.rb new file mode 100644 index 000000000..0893cb7e5 --- /dev/null +++ b/app/models/notifier/event.rb @@ -0,0 +1,23 @@ +class Notifier::Event < Notifier + delegate :card, :creator, to: :source + + private + def resource + if source.action.commented? + source.comment + else + card + end + end + + def recipients + case source.action + when "assigned" + source.assignees.excluding(card.collection.access_only_users) + when "published" + card.watchers_and_subscribers(include_only_watching: true).without(creator) + else + card.watchers_and_subscribers.without(creator) + end + end +end diff --git a/app/models/notifier/events/assigned.rb b/app/models/notifier/events/assigned.rb deleted file mode 100644 index fb106c677..000000000 --- a/app/models/notifier/events/assigned.rb +++ /dev/null @@ -1,6 +0,0 @@ -class Notifier::Events::Assigned < Notifier::Events::Base - private - def recipients - source.assignees.excluding(card.collection.access_only_users) - end -end diff --git a/app/models/notifier/events/base.rb b/app/models/notifier/events/base.rb deleted file mode 100644 index 6852ba2b2..000000000 --- a/app/models/notifier/events/base.rb +++ /dev/null @@ -1,12 +0,0 @@ -class Notifier::Events::Base < Notifier - delegate :card, :creator, to: :source - - private - def resource - card - end - - def recipients - card.watchers_and_subscribers.without(creator) - end -end diff --git a/app/models/notifier/events/closed.rb b/app/models/notifier/events/closed.rb deleted file mode 100644 index 133ca75c8..000000000 --- a/app/models/notifier/events/closed.rb +++ /dev/null @@ -1,2 +0,0 @@ -class Notifier::Events::Closed < Notifier::Events::Base -end diff --git a/app/models/notifier/events/commented.rb b/app/models/notifier/events/commented.rb deleted file mode 100644 index 55497a117..000000000 --- a/app/models/notifier/events/commented.rb +++ /dev/null @@ -1,6 +0,0 @@ -class Notifier::Events::Commented < Notifier::Events::Base - private - def resource - source.comment - end -end diff --git a/app/models/notifier/events/published.rb b/app/models/notifier/events/published.rb deleted file mode 100644 index ff30d0219..000000000 --- a/app/models/notifier/events/published.rb +++ /dev/null @@ -1,6 +0,0 @@ -class Notifier::Events::Published < Notifier::Events::Base - private - def recipients - card.watchers_and_subscribers(include_only_watching: true).without(creator) - end -end diff --git a/test/models/notifier_test.rb b/test/models/notifier_test.rb index 987ac84f2..e1245a19b 100644 --- a/test/models/notifier_test.rb +++ b/test/models/notifier_test.rb @@ -2,7 +2,7 @@ require "test_helper" class NotifierTest < ActiveSupport::TestCase test "for returns the matching notifier class for the event" do - assert_kind_of Notifier::Events::Published, Notifier.for(events(:logo_published)) + assert_kind_of Notifier::Event, Notifier.for(events(:logo_published)) end test "generate does not create notifications if the event was system-generated" do From 8b6cfccdeda507a3269fdeb52e4fbc6ad542563b Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 11:20:33 +0200 Subject: [PATCH 19/50] Rename to clarify usage --- app/models/concerns/mentions.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index 35d1a42f8..f8784332d 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -3,7 +3,8 @@ module Mentions included do has_many :mentions, as: :container, dependent: :destroy - before_save :save_mentionable_content_before_save + has_many :mentionees, through: :mentions + before_save :remember_mentionable_content_before_save after_save_commit :create_mentions_later, if: :mentionable_content_changed? end @@ -24,7 +25,7 @@ module Mentions self.class.reflect_on_all_associations(:has_one).filter { it.klass == ActionText::Markdown } end - def save_mentionable_content_before_save + def remember_mentionable_content_before_save @mentionable_content_before_safe = self.class.find(id).mentionable_content unless new_record? end @@ -33,7 +34,7 @@ module Mentions end def mentionable_content_changed? - @mentionable_content_before_safe != mentionable_content + @mentionable_content_before_safe.present? && @mentionable_content_before_safe != mentionable_content end def scan_mentionees From 8ebb3f2fa9a3f4c1ee4377201ccb9828ec2b964b Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 11:38:34 +0200 Subject: [PATCH 20/50] We need to remember on after_touch so that the mechanism triggers when touch-cascading records automatically via associations --- app/models/concerns/mentions.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index f8784332d..5661c8eca 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -5,6 +5,7 @@ module Mentions has_many :mentions, as: :container, dependent: :destroy has_many :mentionees, through: :mentions before_save :remember_mentionable_content_before_save + after_touch :remember_mentionable_content_before_save after_save_commit :create_mentions_later, if: :mentionable_content_changed? end @@ -26,7 +27,7 @@ module Mentions end def remember_mentionable_content_before_save - @mentionable_content_before_safe = self.class.find(id).mentionable_content unless new_record? + @mentionable_content_before_save ||= self.class.find(id).mentionable_content unless new_record? end def create_mentions_later @@ -34,7 +35,7 @@ module Mentions end def mentionable_content_changed? - @mentionable_content_before_safe.present? && @mentionable_content_before_safe != mentionable_content + new_record? || @mentionable_content_before_save != mentionable_content end def scan_mentionees From edcc9e32791f9bcffd3449c0461742db182225ea Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 12:38:06 +0200 Subject: [PATCH 21/50] Add mentionees as watchers --- app/models/card.rb | 6 ++++++ app/models/card/watchable.rb | 6 ++++-- app/models/collection/accessible.rb | 4 ++++ app/models/comment.rb | 2 ++ app/models/mention.rb | 7 +++++++ app/models/notifier/event.rb | 18 ++++++++++++++++-- app/models/user/accessor.rb | 4 ++++ 7 files changed, 43 insertions(+), 4 deletions(-) diff --git a/app/models/card.rb b/app/models/card.rb index 57ff542da..93eec061c 100644 --- a/app/models/card.rb +++ b/app/models/card.rb @@ -23,6 +23,8 @@ class Card < ApplicationRecord end end + delegate :accessible_to?, to: :collection + def title=(new_title) self[:title] = new_title.presence || "Untitled" end @@ -30,4 +32,8 @@ class Card < ApplicationRecord def cache_key [ super, collection.name ].compact.join("/") end + + def was_mentioned(mention) + watch_by(mention.mentionee) + end end diff --git a/app/models/card/watchable.rb b/app/models/card/watchable.rb index f511ddc7e..94e5e27d9 100644 --- a/app/models/card/watchable.rb +++ b/app/models/card/watchable.rb @@ -13,7 +13,9 @@ module Card::Watchable end def watch_by(user) - watches.where(user: user).first_or_create.update!(watching: true) + if accessible_to?(user) + watches.where(user: user).first_or_create.update!(watching: true) + end end def unwatch_by(user) @@ -21,7 +23,7 @@ module Card::Watchable end def watchers_and_subscribers(include_only_watching: false) - involvements = include_only_watching ? [ :watching, :everything ] : :everything + involvements = include_only_watching ? [:watching, :everything] : :everything subscribers = collection.users.where(accesses: { involvement: involvements }) User.where(id: subscribers.pluck(:id) + diff --git a/app/models/collection/accessible.rb b/app/models/collection/accessible.rb index fc99cc217..1436a4d1b 100644 --- a/app/models/collection/accessible.rb +++ b/app/models/collection/accessible.rb @@ -32,6 +32,10 @@ module Collection::Accessible accesses.find_by(user: user) end + def accessible_to?(user) + access_for(user).present? + end + private def grant_access_to_everyone accesses.grant_to(User.all) if all_access_previously_changed?(to: true) diff --git a/app/models/comment.rb b/app/models/comment.rb index 630589e41..9b3fd2704 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -13,6 +13,8 @@ class Comment < ApplicationRecord after_create_commit :watch_card_by_creator, :track_commented_card after_destroy_commit :cleanup_events + delegate :watch_by, to: :card + def to_partial_path "cards/#{super}" end diff --git a/app/models/mention.rb b/app/models/mention.rb index fd70e075a..2534eb631 100644 --- a/app/models/mention.rb +++ b/app/models/mention.rb @@ -5,7 +5,14 @@ class Mention < ApplicationRecord belongs_to :mentioner, class_name: "User" belongs_to :mentionee, class_name: "User", inverse_of: :mentions + after_create :add_mentionee_as_watcher + def self_mention? mentioner == mentionee end + + private + def add_mentionee_as_watcher + container.watch_by(mentionee) + end end diff --git a/app/models/notifier/event.rb b/app/models/notifier/event.rb index 0893cb7e5..7374763e2 100644 --- a/app/models/notifier/event.rb +++ b/app/models/notifier/event.rb @@ -1,5 +1,6 @@ class Notifier::Event < Notifier delegate :card, :creator, to: :source + delegate :watchers_and_subscribers, to: :card private def resource @@ -15,9 +16,22 @@ class Notifier::Event < Notifier when "assigned" source.assignees.excluding(card.collection.access_only_users) when "published" - card.watchers_and_subscribers(include_only_watching: true).without(creator) + watchers_and_subscribers(include_only_watching: true).without(creator, *mentionees) + when "commented" + watchers_and_subscribers.without(creator, *mentionees) else - card.watchers_and_subscribers.without(creator) + watchers_and_subscribers.without(creator) + end + end + + def mentionees + case source.action + when "published" + source.card.mentionees + when "commented" + source.comment.mentionees + else + [] end end end diff --git a/app/models/user/accessor.rb b/app/models/user/accessor.rb index 89534b777..ebf5dde59 100644 --- a/app/models/user/accessor.rb +++ b/app/models/user/accessor.rb @@ -9,6 +9,10 @@ module User::Accessor after_create_commit :grant_access_to_collections end + def can_access_card?(card) + card.collection.accessible_to?(self) + end + private def grant_access_to_collections Access.insert_all Collection.all_access.pluck(:id).collect { |collection_id| { collection_id: collection_id, user_id: id } } From 755c9b45c7fa26cce2ab64b25076045c64550492 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 12:38:33 +0200 Subject: [PATCH 22/50] Rename and fix problem where after_touch was overriding the saved content --- app/models/concerns/mentions.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index 5661c8eca..883ebc40a 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -4,8 +4,8 @@ module Mentions included do has_many :mentions, as: :container, dependent: :destroy has_many :mentionees, through: :mentions - before_save :remember_mentionable_content_before_save - after_touch :remember_mentionable_content_before_save + before_save :remember_mentionable_content + after_touch :remember_mentionable_content after_save_commit :create_mentions_later, if: :mentionable_content_changed? end @@ -26,8 +26,8 @@ module Mentions self.class.reflect_on_all_associations(:has_one).filter { it.klass == ActionText::Markdown } end - def remember_mentionable_content_before_save - @mentionable_content_before_save ||= self.class.find(id).mentionable_content unless new_record? + def remember_mentionable_content + @mentionable_content_before_save ||= self.class.find(id).mentionable_content if id && !previously_new_record? end def create_mentions_later @@ -35,7 +35,8 @@ module Mentions end def mentionable_content_changed? - new_record? || @mentionable_content_before_save != mentionable_content + puts "Was #{previously_new_record?}" + previously_new_record? || @mentionable_content_before_save != mentionable_content end def scan_mentionees From 2e3ffcfb9dac08fbf9157d898b3aaef1e5f6d2d4 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 12:44:36 +0200 Subject: [PATCH 23/50] Remove identation --- app/models/notifier.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 651d6898f..00c5bbdda 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -4,10 +4,10 @@ class Notifier class << self def for(source) case source - when ::Event - Notifier::Event.new(source) - when ::Mention - Notifier::Mention.new(source) + when ::Event + Notifier::Event.new(source) + when ::Mention + Notifier::Mention.new(source) end end end From 043230394eae9abec32ddba03039d1e2f6f8d15f Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 13:08:00 +0200 Subject: [PATCH 24/50] Tidy up event helper code Also, revert to using a helper method to determine the event action, since this is purely a view concern. Keep method at the event model to identify initial assignments. --- app/helpers/notifications_helper.rb | 36 ++++++++++++++++++----------- app/models/concerns/mentions.rb | 1 - app/models/event.rb | 19 +++------------ 3 files changed, 25 insertions(+), 31 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 613776bac..cc63fb5d5 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,25 +1,20 @@ module NotificationsHelper def event_notification_title(event) - if event.commented? - "RE: " + event.card.title - elsif event.assigned? - "Assigned to you: " + event.card.title - else - event.card.title + case event_notification_action(event) + when "commented" then "RE: " + event.card.title + when "assigned" then "Assigned to you: " + event.card.title + else event.card.title end end def event_notification_body(event) name = event.creator.name - if event.closed? - "Closed by #{name}" - elsif event.published? - "Added by #{name}" - elsif event.commented? - "#{strip_tags(event.comment.body_html).blank? ? "#{name} replied" : "#{event.creator.name}:" } #{strip_tags(event.comment.body_html).truncate(200)}" - else - name + case event_notification_action(event) + when "closed" then "Closed by #{name}" + when "published" then "Added by #{name}" + when "commented" then comment_notification_body(event) + else name end end @@ -59,4 +54,17 @@ module NotificationsHelper tag.div id: "next_page", data: { controller: "fetch-on-visible", fetch_on_visible_url_value: notifications_path(page: @page.next_param) } end end + + private + def event_notification_action(event) + if event.initial_assignment? + "assigned" + else + event.action + end + end + + def comment_notification_body(event) + "#{strip_tags(event.comment.body_html).blank? ? "#{name} replied" : "#{event.creator.name}:" } #{strip_tags(event.comment.body_html).truncate(200)}" + end end diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index 883ebc40a..41a818794 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -35,7 +35,6 @@ module Mentions end def mentionable_content_changed? - puts "Was #{previously_new_record?}" previously_new_record? || @mentionable_content_before_save != mentionable_content end diff --git a/app/models/event.rb b/app/models/event.rb index 5eb095039..f95dc0785 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -12,24 +12,11 @@ class Event < ApplicationRecord after_create -> { card.touch(:last_active_at) } - def assigned? - action == "assigned" || initial_assignment? - end - def action - super.inquiry + super&.inquiry end - def method_missing(method_name, *args, &block) - if method_name.to_s.end_with?("?") - action == method_name.to_s.chomp("?") - else - super - end + def initial_assignment? + action == "published" && card.assigned_to?(creator) end - - private - def initial_assignment? - action == "published" && card.assigned_to?(creator) - end end From 7c0aff06805f1bf668210fbb982d70efed1eafdb Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 13:36:39 +0200 Subject: [PATCH 25/50] Don't indent case --- app/helpers/notifications_helper.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index cc63fb5d5..7ed4813aa 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,9 +1,9 @@ module NotificationsHelper def event_notification_title(event) case event_notification_action(event) - when "commented" then "RE: " + event.card.title - when "assigned" then "Assigned to you: " + event.card.title - else event.card.title + when "commented" then "RE: " + event.card.title + when "assigned" then "Assigned to you: " + event.card.title + else event.card.title end end @@ -11,10 +11,10 @@ module NotificationsHelper name = event.creator.name case event_notification_action(event) - when "closed" then "Closed by #{name}" - when "published" then "Added by #{name}" - when "commented" then comment_notification_body(event) - else name + when "closed" then "Closed by #{name}" + when "published" then "Added by #{name}" + when "commented" then comment_notification_body(event) + else name end end From 696732a90a63a716beba5a862f94bededd3dc3b7 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 13:41:34 +0200 Subject: [PATCH 26/50] Rename mentions' container => source --- app/models/concerns/mentions.rb | 2 +- app/models/mention.rb | 4 +- app/models/notifier/mention.rb | 10 +++-- app/models/user/mentionable.rb | 2 +- .../notification/_mention.html.erb | 2 +- db/migrate/20250421120008_create_mentions.rb | 2 +- db/schema.rb | 6 +-- db/schema_cache.yml | 43 ++++++++----------- 8 files changed, 32 insertions(+), 39 deletions(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index 41a818794..9e00ed3f7 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -2,7 +2,7 @@ module Mentions extend ActiveSupport::Concern included do - has_many :mentions, as: :container, dependent: :destroy + has_many :mentions, as: :source, dependent: :destroy has_many :mentionees, through: :mentions before_save :remember_mentionable_content after_touch :remember_mentionable_content diff --git a/app/models/mention.rb b/app/models/mention.rb index 2534eb631..2c6c95750 100644 --- a/app/models/mention.rb +++ b/app/models/mention.rb @@ -1,7 +1,7 @@ class Mention < ApplicationRecord include Notifiable - belongs_to :container, polymorphic: true + belongs_to :source, polymorphic: true belongs_to :mentioner, class_name: "User" belongs_to :mentionee, class_name: "User", inverse_of: :mentions @@ -13,6 +13,6 @@ class Mention < ApplicationRecord private def add_mentionee_as_watcher - container.watch_by(mentionee) + source.watch_by(mentionee) end end diff --git a/app/models/notifier/mention.rb b/app/models/notifier/mention.rb index 2085c42ed..ac1a3d413 100644 --- a/app/models/notifier/mention.rb +++ b/app/models/notifier/mention.rb @@ -1,18 +1,20 @@ class Notifier::Mention < Notifier + alias mention source + private def resource - source.container + mention.source end def recipients - if source.self_mention? + if mention.self_mention? [] else - [ source.mentionee ] + [ mention.mentionee ] end end def creator - source.mentioner + mention.mentioner end end diff --git a/app/models/user/mentionable.rb b/app/models/user/mentionable.rb index e3e5a9dbf..ab1906659 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, mentioner: mentioner + mentions.create! source: at, mentioner: mentioner end def mentionable_handles diff --git a/app/views/notifications/notification/_mention.html.erb b/app/views/notifications/notification/_mention.html.erb index 96abde715..7ed8ffdc5 100644 --- a/app/views/notifications/notification/_mention.html.erb +++ b/app/views/notifications/notification/_mention.html.erb @@ -2,6 +2,6 @@ <%= mention.mentioner.first_name %> mentioned you
- <%= mention.container.mentionable_content.truncate(200) %> + <%= mention.source.mentionable_content.truncate(200) %>
<%= local_datetime_tag(notification.created_at, style: :ago) %>
diff --git a/db/migrate/20250421120008_create_mentions.rb b/db/migrate/20250421120008_create_mentions.rb index da4923950..db27f7b2c 100644 --- a/db/migrate/20250421120008_create_mentions.rb +++ b/db/migrate/20250421120008_create_mentions.rb @@ -1,7 +1,7 @@ class CreateMentions < ActiveRecord::Migration[8.1] def change create_table :mentions do |t| - t.references :container, polymorphic: true, null: false, index: true + t.references :source, polymorphic: true, null: false, index: true t.references :mentionee, foreign_key: { to_table: :users }, null: false t.references :mentioner, foreign_key: { to_table: :users }, null: false diff --git a/db/schema.rb b/db/schema.rb index 2dbc1c529..49b1827d2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -213,15 +213,15 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_22_112857) do end create_table "mentions", force: :cascade do |t| - t.integer "container_id", null: false - t.string "container_type", null: false t.datetime "created_at", null: false t.integer "mentionee_id", null: false t.integer "mentioner_id", null: false + t.integer "source_id", null: false + t.string "source_type", null: false t.datetime "updated_at", null: false - t.index ["container_type", "container_id"], name: "index_mentions_on_container" t.index ["mentionee_id"], name: "index_mentions_on_mentionee_id" t.index ["mentioner_id"], name: "index_mentions_on_mentioner_id" + t.index ["source_type", "source_id"], name: "index_mentions_on_source" end create_table "messages", force: :cascade do |t| diff --git a/db/schema_cache.yml b/db/schema_cache.yml index eb67d0f45..dc7cfae88 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -374,7 +374,7 @@ columns: - *9 cards: - *5 - - &30 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &31 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: title cast_type: *6 @@ -494,11 +494,11 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: all_access - cast_type: &31 !ruby/object:ActiveModel::Type::Boolean + cast_type: &32 !ruby/object:ActiveModel::Type::Boolean precision: scale: limit: - sql_type_metadata: &32 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata + sql_type_metadata: &33 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata sql_type: boolean type: :boolean limit: @@ -627,7 +627,7 @@ columns: comment: filters_tags: - *18 - - &29 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &30 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: tag_id cast_type: *1 @@ -639,9 +639,9 @@ columns: comment: mentions: - *5 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &29 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: container_type + name: source_type cast_type: *6 sql_type_metadata: *7 'null': false @@ -651,7 +651,7 @@ columns: comment: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: container_id + name: source_id cast_type: *1 sql_type_metadata: *2 'null': false @@ -741,16 +741,7 @@ columns: default_function: collation: comment: - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: source_type - cast_type: *6 - sql_type_metadata: *7 - 'null': false - default: - default_function: - collation: - comment: + - *29 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: source_id @@ -860,12 +851,12 @@ columns: taggings: - *5 - *21 - - *29 + - *30 - *8 - *9 tags: - *5 - - *30 + - *31 - *8 - *9 users: @@ -894,8 +885,8 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: active - cast_type: *31 - sql_type_metadata: *32 + cast_type: *32 + sql_type_metadata: *33 'null': false default: true default_function: @@ -920,8 +911,8 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: watching - cast_type: *31 - sql_type_metadata: *32 + cast_type: *32 + sql_type_metadata: *33 'null': false default: true default_function: @@ -1698,11 +1689,11 @@ indexes: valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: mentions - name: index_mentions_on_container + name: index_mentions_on_source unique: false columns: - - container_type - - container_id + - source_type + - source_id lengths: {} orders: {} opclasses: {} From 2eae58725ad965849a6df8339a9a2cde26a19d04 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 13:52:47 +0200 Subject: [PATCH 27/50] Remove notifications' resource relationship Is confusing since "source" already captures the origin of the notifications. It's needed to interpret things at rendering time, so we can query things as needed there. --- app/helpers/notifications_helper.rb | 8 ++-- app/models/notification.rb | 1 - app/models/notifier.rb | 2 +- app/models/notifier/event.rb | 8 ---- app/models/notifier/mention.rb | 4 -- ...423114737_remove_notification_resources.rb | 5 +++ db/schema.rb | 5 +-- db/schema_cache.yml | 39 +------------------ test/fixtures/notifications.yml | 3 -- test/models/notifier_test.rb | 2 +- 10 files changed, 13 insertions(+), 64 deletions(-) create mode 100644 db/migrate/20250423114737_remove_notification_resources.rb diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 7ed4813aa..3e06682ef 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -31,11 +31,11 @@ module NotificationsHelper end def notification_resource_path(notification) - if notification.resource.is_a?(Comment) - # TODO: Extract a direct path for these - collection_card_path(notification.resource.card.collection, notification.resource.card, anchor: "comment_#{notification.resource.id}") + if notification.source.action.commented? + card = notification.source.card + collection_card_path(card.collection, card, anchor: "comment_#{notification.source.comment.id}") else - notification.resource + notification.source.card end end diff --git a/app/models/notification.rb b/app/models/notification.rb index 301843987..1b1c91b5c 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -2,7 +2,6 @@ class Notification < ApplicationRecord belongs_to :user belongs_to :creator, class_name: "User" belongs_to :source, polymorphic: true - belongs_to :resource, polymorphic: true scope :unread, -> { where(read_at: nil) } scope :read, -> { where.not(read_at: nil) } diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 00c5bbdda..8b0b9d2ce 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -15,7 +15,7 @@ class Notifier def notify if should_notify? recipients.map do |recipient| - Notification.create! user: recipient, source: source, resource: resource, creator: creator + Notification.create! user: recipient, source: source, creator: creator end end end diff --git a/app/models/notifier/event.rb b/app/models/notifier/event.rb index 7374763e2..f5db7c146 100644 --- a/app/models/notifier/event.rb +++ b/app/models/notifier/event.rb @@ -3,14 +3,6 @@ class Notifier::Event < Notifier delegate :watchers_and_subscribers, to: :card private - def resource - if source.action.commented? - source.comment - else - card - end - end - def recipients case source.action when "assigned" diff --git a/app/models/notifier/mention.rb b/app/models/notifier/mention.rb index ac1a3d413..feea686f4 100644 --- a/app/models/notifier/mention.rb +++ b/app/models/notifier/mention.rb @@ -2,10 +2,6 @@ class Notifier::Mention < Notifier alias mention source private - def resource - mention.source - end - def recipients if mention.self_mention? [] diff --git a/db/migrate/20250423114737_remove_notification_resources.rb b/db/migrate/20250423114737_remove_notification_resources.rb new file mode 100644 index 000000000..6c19062f7 --- /dev/null +++ b/db/migrate/20250423114737_remove_notification_resources.rb @@ -0,0 +1,5 @@ +class RemoveNotificationResources < ActiveRecord::Migration[8.1] + def change + remove_reference :notifications, :resource, polymorphic: true, index: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 49b1827d2..595cf2346 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_112857) do +ActiveRecord::Schema[8.1].define(version: 2025_04_23_114737) do create_table "accesses", force: :cascade do |t| t.integer "collection_id", null: false t.datetime "created_at", null: false @@ -238,14 +238,11 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_22_112857) do t.datetime "created_at", null: false t.integer "creator_id" t.datetime "read_at" - t.integer "resource_id", null: false - t.string "resource_type", null: false t.integer "source_id" 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 } t.index ["user_id"], name: "index_notifications_on_user_id" diff --git a/db/schema_cache.yml b/db/schema_cache.yml index dc7cfae88..9e58a9422 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -709,26 +709,6 @@ columns: notifications: - *5 - *28 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: resource_type - cast_type: *6 - sql_type_metadata: *7 - 'null': false - default: - default_function: - collation: - comment: - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: resource_id - cast_type: *1 - sql_type_metadata: *2 - 'null': false - default: - default_function: - collation: - comment: - *8 - *9 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column @@ -1792,23 +1772,6 @@ indexes: nulls_not_distinct: comment: valid: true - - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: notifications - name: index_notifications_on_resource - unique: false - columns: - - resource_type - - resource_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_user_id @@ -2045,4 +2008,4 @@ indexes: comment: valid: true workflows: [] -version: 20250422112857 +version: 20250423114737 diff --git a/test/fixtures/notifications.yml b/test/fixtures/notifications.yml index db6574038..bea0e4d33 100644 --- a/test/fixtures/notifications.yml +++ b/test/fixtures/notifications.yml @@ -1,20 +1,17 @@ logo_published_kevin: user: 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 diff --git a/test/models/notifier_test.rb b/test/models/notifier_test.rb index e1245a19b..8dc9c8afe 100644 --- a/test/models/notifier_test.rb +++ b/test/models/notifier_test.rb @@ -43,7 +43,7 @@ class NotifierTest < ActiveSupport::TestCase test "links to the card" do Notifier.for(events(:logo_published)).notify - assert_equal cards(:logo), Notification.last.resource + assert_equal cards(:logo), Notification.last.source.card end test "assignment events only create a notification for the assignee" do From 07d3447e7e62e3ddaecac4da1c2d60c50ea642d0 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 14:29:13 +0200 Subject: [PATCH 28/50] Resolve notifications URLs --- app/helpers/notifications_helper.rb | 11 +---------- config/routes.rb | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 3e06682ef..4568ad82a 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -21,7 +21,7 @@ module NotificationsHelper def notification_tag(notification, &) tag.div id: dom_id(notification), class: "notification tray__item border-radius txt-normal" do concat( - link_to(notification_resource_path(notification), + link_to(notification, class: "notification__content border-radius shadow fill-white flex align-start txt-align-start gap flex-item-grow max-width border txt-ink", data: { action: "click->dialog#close", turbo_frame: "_top" }, &) @@ -30,15 +30,6 @@ module NotificationsHelper end end - def notification_resource_path(notification) - if notification.source.action.commented? - card = notification.source.card - collection_card_path(card.collection, card, anchor: "comment_#{notification.source.comment.id}") - else - notification.source.card - end - end - def notification_mark_read_button(notification) button_to read_notification_path(notification), class: "notification__unread_indicator btn borderless", diff --git a/config/routes.rb b/config/routes.rb index 61271338a..5ff480121 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -103,6 +103,23 @@ Rails.application.routes.draw do route_for :collection_card, comment.card.collection, comment.card, options end + resolve "Mention" do |mention, options| + polymorphic_path(mention.source, options) + end + + resolve "Notification" do |notification, options| + source = if notification.source.is_a?(Event) + if notification.source.action.commented? + notification.source.comment + else + notification.source.card + end + else + notification.source + end + + polymorphic_path(source, options) + end get "up", to: "rails/health#show", as: :rails_health_check get "manifest" => "rails/pwa#manifest", as: :pwa_manifest From ab73417e7393bbdcb06ff7a14c6f0f0788466e74 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 14:30:09 +0200 Subject: [PATCH 29/50] Leverage existing resolve route for comments --- app/views/cards/comments/_comment.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/cards/comments/_comment.html.erb b/app/views/cards/comments/_comment.html.erb index 2881ee438..55aa89459 100644 --- a/app/views/cards/comments/_comment.html.erb +++ b/app/views/cards/comments/_comment.html.erb @@ -11,7 +11,7 @@ <%= link_to comment.creator.name, comment.creator, class: "txt-ink btn btn--plain fill-transparent", data: { turbo_frame: "_top" } %> - <%= link_to collection_card_path(comment.card.collection, comment.card, anchor: "comment_#{comment.id}"), class: "txt-undecorated txt-uppercase" do %> + <%= link_to comment, class: "txt-undecorated txt-uppercase" do %> <%= local_datetime_tag comment.created_at, style: :shortdate, class: "txt-ink translucent" %> <% end %> From dc5f64ab30175ee431cbde21554125eef845b45f Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 14:37:09 +0200 Subject: [PATCH 30/50] Remove redundant default queue declaration --- app/jobs/card/auto_close_all_due_job.rb | 2 -- app/jobs/card/auto_reconsider_all_stagnated_job.rb | 2 -- app/jobs/mention/create_job.rb | 2 -- app/jobs/notify_recipients_job.rb | 2 -- app/jobs/remove_abandoned_creations_job.rb | 2 -- 5 files changed, 10 deletions(-) diff --git a/app/jobs/card/auto_close_all_due_job.rb b/app/jobs/card/auto_close_all_due_job.rb index 6cb4f646b..d964dae23 100644 --- a/app/jobs/card/auto_close_all_due_job.rb +++ b/app/jobs/card/auto_close_all_due_job.rb @@ -1,6 +1,4 @@ class Card::AutoCloseAllDueJob < ApplicationJob - queue_as :default - def perform ApplicationRecord.with_each_tenant do |tenant| Card.auto_close_all_due diff --git a/app/jobs/card/auto_reconsider_all_stagnated_job.rb b/app/jobs/card/auto_reconsider_all_stagnated_job.rb index 83f87a648..b7178f916 100644 --- a/app/jobs/card/auto_reconsider_all_stagnated_job.rb +++ b/app/jobs/card/auto_reconsider_all_stagnated_job.rb @@ -1,6 +1,4 @@ class Card::AutoReconsiderAllStagnatedJob < ApplicationJob - queue_as :default - def perform ApplicationRecord.with_each_tenant do |tenant| Card.auto_reconsider_all_stagnated diff --git a/app/jobs/mention/create_job.rb b/app/jobs/mention/create_job.rb index 364ea1272..816782754 100644 --- a/app/jobs/mention/create_job.rb +++ b/app/jobs/mention/create_job.rb @@ -1,6 +1,4 @@ class Mention::CreateJob < ApplicationJob - queue_as :default - def perform(record, mentioner:) record.create_mentions(mentioner:) end diff --git a/app/jobs/notify_recipients_job.rb b/app/jobs/notify_recipients_job.rb index 2f14c2e96..6083622fd 100644 --- a/app/jobs/notify_recipients_job.rb +++ b/app/jobs/notify_recipients_job.rb @@ -1,6 +1,4 @@ class NotifyRecipientsJob < ApplicationJob - queue_as :default - def perform(notifiable) notifiable.notify_recipients end diff --git a/app/jobs/remove_abandoned_creations_job.rb b/app/jobs/remove_abandoned_creations_job.rb index 1444313a2..6ca63169d 100644 --- a/app/jobs/remove_abandoned_creations_job.rb +++ b/app/jobs/remove_abandoned_creations_job.rb @@ -1,6 +1,4 @@ class RemoveAbandonedCreationsJob < ApplicationJob - queue_as :default - def perform ApplicationRecord.with_each_tenant do |tenant| Card.remove_abandoned_creations From f43629f84926e67bbb323c8a5d5b841f49b1ff38 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 14:39:10 +0200 Subject: [PATCH 31/50] Format --- app/models/card/watchable.rb | 2 +- app/models/notifier/event.rb | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models/card/watchable.rb b/app/models/card/watchable.rb index 94e5e27d9..8e719f756 100644 --- a/app/models/card/watchable.rb +++ b/app/models/card/watchable.rb @@ -23,7 +23,7 @@ module Card::Watchable end def watchers_and_subscribers(include_only_watching: false) - involvements = include_only_watching ? [:watching, :everything] : :everything + involvements = include_only_watching ? [ :watching, :everything ] : :everything subscribers = collection.users.where(accesses: { involvement: involvements }) User.where(id: subscribers.pluck(:id) + diff --git a/app/models/notifier/event.rb b/app/models/notifier/event.rb index f5db7c146..a98835f5b 100644 --- a/app/models/notifier/event.rb +++ b/app/models/notifier/event.rb @@ -5,24 +5,24 @@ class Notifier::Event < Notifier private def recipients case source.action - when "assigned" + when "assigned" source.assignees.excluding(card.collection.access_only_users) - when "published" + when "published" watchers_and_subscribers(include_only_watching: true).without(creator, *mentionees) - when "commented" + when "commented" watchers_and_subscribers.without(creator, *mentionees) - else + else watchers_and_subscribers.without(creator) end end def mentionees case source.action - when "published" + when "published" source.card.mentionees - when "commented" + when "commented" source.comment.mentionees - else + else [] end end From e446080367dc39684967ee3022c23cbc90110198 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 14:45:44 +0200 Subject: [PATCH 32/50] Don't create duplicated mentions E.g: when editing a comment or description with a mention --- app/models/user/mentionable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user/mentionable.rb b/app/models/user/mentionable.rb index ab1906659..7a930ab02 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! source: at, mentioner: mentioner + mentions.find_or_create_by! source: at, mentioner: mentioner end def mentionable_handles From 469fb9f04979db6cb90a00cbea4d6e70cb733e17 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:02:31 +0200 Subject: [PATCH 33/50] Simplify the logic to determine action text changes --- app/models/concerns/mentions.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index 9e00ed3f7..fde4e8ca1 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -4,8 +4,6 @@ module Mentions included do has_many :mentions, as: :source, dependent: :destroy has_many :mentionees, through: :mentions - before_save :remember_mentionable_content - after_touch :remember_mentionable_content after_save_commit :create_mentions_later, if: :mentionable_content_changed? end @@ -26,18 +24,14 @@ module Mentions self.class.reflect_on_all_associations(:has_one).filter { it.klass == ActionText::Markdown } end - def remember_mentionable_content - @mentionable_content_before_save ||= self.class.find(id).mentionable_content if id && !previously_new_record? + def mentionable_content_changed? + markdown_associations.any? { send(it.name).content_previously_changed? } end def create_mentions_later Mention::CreateJob.perform_later(self, mentioner: Current.user) end - def mentionable_content_changed? - previously_new_record? || @mentionable_content_before_save != mentionable_content - end - def scan_mentionees scan_mentioned_handles.filter_map do |mention| mentionable_users.find { |user| user.mentionable_handles.include?(mention) } From 6ff9f7a154347df72493b84963452b846327259e Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:07:02 +0200 Subject: [PATCH 34/50] Fix identation --- app/models/notifier/event.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/notifier/event.rb b/app/models/notifier/event.rb index a98835f5b..fb32d4178 100644 --- a/app/models/notifier/event.rb +++ b/app/models/notifier/event.rb @@ -6,24 +6,24 @@ class Notifier::Event < Notifier def recipients case source.action when "assigned" - source.assignees.excluding(card.collection.access_only_users) + source.assignees.excluding(card.collection.access_only_users) when "published" - watchers_and_subscribers(include_only_watching: true).without(creator, *mentionees) + watchers_and_subscribers(include_only_watching: true).without(creator, *mentionees) when "commented" - watchers_and_subscribers.without(creator, *mentionees) + watchers_and_subscribers.without(creator, *mentionees) else - watchers_and_subscribers.without(creator) + watchers_and_subscribers.without(creator) end end def mentionees case source.action when "published" - source.card.mentionees + source.card.mentionees when "commented" - source.comment.mentionees + source.comment.mentionees else - [] + [] end end end From a9df2d48d11f028963504f2b3945e8e9a18d8388 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:17:51 +0200 Subject: [PATCH 35/50] Remove delegate --- app/models/card.rb | 2 -- app/models/card/watchable.rb | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/models/card.rb b/app/models/card.rb index 93eec061c..808ff9467 100644 --- a/app/models/card.rb +++ b/app/models/card.rb @@ -23,8 +23,6 @@ class Card < ApplicationRecord end end - delegate :accessible_to?, to: :collection - def title=(new_title) self[:title] = new_title.presence || "Untitled" end diff --git a/app/models/card/watchable.rb b/app/models/card/watchable.rb index 8e719f756..8c5a3696b 100644 --- a/app/models/card/watchable.rb +++ b/app/models/card/watchable.rb @@ -13,7 +13,7 @@ module Card::Watchable end def watch_by(user) - if accessible_to?(user) + if collection.accessible_to?(user) watches.where(user: user).first_or_create.update!(watching: true) end end From ad58de9e764dbcc6d4a185d9d384846540296a7c Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:22:18 +0200 Subject: [PATCH 36/50] Remove access checks when watching https://github.com/basecamp/fizzy/pull/425#discussion_r2056039581 --- app/models/card/watchable.rb | 4 +--- app/models/collection/accessible.rb | 4 ---- app/models/user/accessor.rb | 4 ---- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/app/models/card/watchable.rb b/app/models/card/watchable.rb index 8c5a3696b..f511ddc7e 100644 --- a/app/models/card/watchable.rb +++ b/app/models/card/watchable.rb @@ -13,9 +13,7 @@ module Card::Watchable end def watch_by(user) - if collection.accessible_to?(user) - watches.where(user: user).first_or_create.update!(watching: true) - end + watches.where(user: user).first_or_create.update!(watching: true) end def unwatch_by(user) diff --git a/app/models/collection/accessible.rb b/app/models/collection/accessible.rb index 1436a4d1b..fc99cc217 100644 --- a/app/models/collection/accessible.rb +++ b/app/models/collection/accessible.rb @@ -32,10 +32,6 @@ module Collection::Accessible accesses.find_by(user: user) end - def accessible_to?(user) - access_for(user).present? - end - private def grant_access_to_everyone accesses.grant_to(User.all) if all_access_previously_changed?(to: true) diff --git a/app/models/user/accessor.rb b/app/models/user/accessor.rb index ebf5dde59..89534b777 100644 --- a/app/models/user/accessor.rb +++ b/app/models/user/accessor.rb @@ -9,10 +9,6 @@ module User::Accessor after_create_commit :grant_access_to_collections end - def can_access_card?(card) - card.collection.accessible_to?(self) - end - private def grant_access_to_collections Access.insert_all Collection.all_access.pluck(:id).collect { |collection_id| { collection_id: collection_id, user_id: id } } From 4bef466df2b0ead5ad5c5f16969ce16986f5765c Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:22:41 +0200 Subject: [PATCH 37/50] 1-line --- app/models/concerns/mentions.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index fde4e8ca1..665ab5be4 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -14,9 +14,7 @@ module Mentions end def mentionable_content - markdown_associations.collect do |association| - send(association.name)&.to_plain_text - end.compact.join(" ") + markdown_associations.collect { |association| send(association.name)&.to_plain_text }.compact.join(" ") end private From e8e95bd5115bc7f8709098def1f114fdf3187087 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:27:25 +0200 Subject: [PATCH 38/50] Use it --- app/models/concerns/mentions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index 665ab5be4..efcacac52 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -14,7 +14,7 @@ module Mentions end def mentionable_content - markdown_associations.collect { |association| send(association.name)&.to_plain_text }.compact.join(" ") + markdown_associations.collect { |it| send(it.name)&.to_plain_text }.compact.join(" ") end private From ea66845e71bdbc8a77b17af34c487edb25cd9328 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:27:58 +0200 Subject: [PATCH 39/50] There is always an action --- app/models/event.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/event.rb b/app/models/event.rb index f95dc0785..395f96971 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -13,7 +13,7 @@ class Event < ApplicationRecord after_create -> { card.touch(:last_active_at) } def action - super&.inquiry + super.inquiry end def initial_assignment? From 53eba9bd0bfe2647ac40a914bf90761562d5d48e Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:28:28 +0200 Subject: [PATCH 40/50] Favor after_create_commit to keep transactions fast --- app/models/mention.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/mention.rb b/app/models/mention.rb index 2c6c95750..cbf4ad1b3 100644 --- a/app/models/mention.rb +++ b/app/models/mention.rb @@ -5,7 +5,7 @@ class Mention < ApplicationRecord belongs_to :mentioner, class_name: "User" belongs_to :mentionee, class_name: "User", inverse_of: :mentions - after_create :add_mentionee_as_watcher + after_create_commit :add_mentionee_as_watcher def self_mention? mentioner == mentionee From e964c5f9c9d129151f79a22faa1ff8e6e4edf5b8 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:31:26 +0200 Subject: [PATCH 41/50] Not used! --- app/models/notifier.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 8b0b9d2ce..08ea8c9ad 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -28,8 +28,4 @@ class Notifier def should_notify? !creator.system? end - - def resource - source - end end From 6f98f2f573f83163c0521e8e50a02a2d3e590518 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:32:22 +0200 Subject: [PATCH 42/50] Order methods --- app/models/concerns/mentions.rb | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index efcacac52..f6decd635 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -18,6 +18,20 @@ module Mentions end private + def scan_mentionees + scan_mentioned_handles.filter_map do |mention| + mentionable_users.find { |user| user.mentionable_handles.include?(mention) } + end + end + + def scan_mentioned_handles + mentionable_content.scan(/(? Date: Wed, 23 Apr 2025 15:37:28 +0200 Subject: [PATCH 43/50] Inline, no need to duplicate the conditional --- app/models/notifier/event.rb | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/app/models/notifier/event.rb b/app/models/notifier/event.rb index fb32d4178..0fc31914a 100644 --- a/app/models/notifier/event.rb +++ b/app/models/notifier/event.rb @@ -8,22 +8,11 @@ class Notifier::Event < Notifier when "assigned" source.assignees.excluding(card.collection.access_only_users) when "published" - watchers_and_subscribers(include_only_watching: true).without(creator, *mentionees) + watchers_and_subscribers(include_only_watching: true).without(creator, *card.mentionees) when "commented" - watchers_and_subscribers.without(creator, *mentionees) + watchers_and_subscribers.without(creator, *source.comment.mentionees) else watchers_and_subscribers.without(creator) end end - - def mentionees - case source.action - when "published" - source.card.mentionees - when "commented" - source.comment.mentionees - else - [] - end - end end From 970a51db28460fa0e0f924cca491fdc93198f9c0 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 15:49:22 +0200 Subject: [PATCH 44/50] No need for it! --- app/models/concerns/mentions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/mentions.rb b/app/models/concerns/mentions.rb index f6decd635..08e0362cb 100644 --- a/app/models/concerns/mentions.rb +++ b/app/models/concerns/mentions.rb @@ -14,7 +14,7 @@ module Mentions end def mentionable_content - markdown_associations.collect { |it| send(it.name)&.to_plain_text }.compact.join(" ") + markdown_associations.collect { send(it.name)&.to_plain_text }.compact.join(" ") end private From 5dac14ab50b4213a1f538adb93c4acf764116b3f Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 16:06:20 +0200 Subject: [PATCH 45/50] Move logic to the model --- app/models/event.rb | 8 ++++++++ app/models/notification.rb | 8 ++++++++ config/routes.rb | 12 +----------- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/app/models/event.rb b/app/models/event.rb index 395f96971..7fa70bd77 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -16,6 +16,14 @@ class Event < ApplicationRecord super.inquiry end + def target + if action.commented? + comment + else + card + end + end + def initial_assignment? action == "published" && card.assigned_to?(creator) end diff --git a/app/models/notification.rb b/app/models/notification.rb index 1b1c91b5c..35c2ac702 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -21,6 +21,14 @@ class Notification < ApplicationRecord read_at.present? end + def target + if source.is_a?(Event) + source.target + else + source + end + end + private def broadcast_unread broadcast_prepend_later_to user, :notifications, target: "notifications" diff --git a/config/routes.rb b/config/routes.rb index 5ff480121..5ba46a8f8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -108,17 +108,7 @@ Rails.application.routes.draw do end resolve "Notification" do |notification, options| - source = if notification.source.is_a?(Event) - if notification.source.action.commented? - notification.source.comment - else - notification.source.card - end - else - notification.source - end - - polymorphic_path(source, options) + polymorphic_path(notification.target, options) end get "up", to: "rails/health#show", as: :rails_health_check From f258b05b239f8b8e095ca141c61dfb707e31b4b6 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 16:15:08 +0200 Subject: [PATCH 46/50] Delegate target to the notifiable objects --- app/models/concerns/notifiable.rb | 4 ++++ app/models/event.rb | 2 +- app/models/mention.rb | 4 ++++ app/models/notification.rb | 10 ++-------- app/models/notifier.rb | 8 ++++---- config/routes.rb | 2 +- test/models/notifier_test.rb | 2 +- 7 files changed, 17 insertions(+), 15 deletions(-) diff --git a/app/models/concerns/notifiable.rb b/app/models/concerns/notifiable.rb index 7c3531ed2..4723229d9 100644 --- a/app/models/concerns/notifiable.rb +++ b/app/models/concerns/notifiable.rb @@ -11,6 +11,10 @@ module Notifiable Notifier.for(self)&.notify end + def notifiable_target + self + end + private def notify_recipients_later NotifyRecipientsJob.perform_later self diff --git a/app/models/event.rb b/app/models/event.rb index 7fa70bd77..0fe643bb5 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -16,7 +16,7 @@ class Event < ApplicationRecord super.inquiry end - def target + def notifiable_target if action.commented? comment else diff --git a/app/models/mention.rb b/app/models/mention.rb index cbf4ad1b3..c14237976 100644 --- a/app/models/mention.rb +++ b/app/models/mention.rb @@ -11,6 +11,10 @@ class Mention < ApplicationRecord mentioner == mentionee end + def notifiable_target + source + end + private def add_mentionee_as_watcher source.watch_by(mentionee) diff --git a/app/models/notification.rb b/app/models/notification.rb index 35c2ac702..01444ce61 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -9,6 +9,8 @@ class Notification < ApplicationRecord after_create_commit :broadcast_unread + delegate :notifiable_target, to: :source + def self.read_all update!(read_at: Time.current) end @@ -21,14 +23,6 @@ class Notification < ApplicationRecord read_at.present? end - def target - if source.is_a?(Event) - source.target - else - source - end - end - private def broadcast_unread broadcast_prepend_later_to user, :notifications, target: "notifications" diff --git a/app/models/notifier.rb b/app/models/notifier.rb index 08ea8c9ad..548b3869c 100644 --- a/app/models/notifier.rb +++ b/app/models/notifier.rb @@ -4,10 +4,10 @@ class Notifier class << self def for(source) case source - when ::Event - Notifier::Event.new(source) - when ::Mention - Notifier::Mention.new(source) + when Event + EventNotifier.new(source) + when Mention + MentionNotifier.new(source) end end end diff --git a/config/routes.rb b/config/routes.rb index 5ba46a8f8..0acad2014 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -108,7 +108,7 @@ Rails.application.routes.draw do end resolve "Notification" do |notification, options| - polymorphic_path(notification.target, options) + polymorphic_path(notification.notifiable_target, options) end get "up", to: "rails/health#show", as: :rails_health_check diff --git a/test/models/notifier_test.rb b/test/models/notifier_test.rb index 8dc9c8afe..d27379464 100644 --- a/test/models/notifier_test.rb +++ b/test/models/notifier_test.rb @@ -2,7 +2,7 @@ require "test_helper" class NotifierTest < ActiveSupport::TestCase test "for returns the matching notifier class for the event" do - assert_kind_of Notifier::Event, Notifier.for(events(:logo_published)) + assert_kind_of Notifier::EventNotifier, Notifier.for(events(:logo_published)) end test "generate does not create notifications if the event was system-generated" do From 5749ad77e5307699cd76f12405ee42ca28c3023a Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 16:19:41 +0200 Subject: [PATCH 47/50] Rename classes --- app/models/notifier/{event.rb => event_notifier.rb} | 2 +- app/models/notifier/{mention.rb => mention_notifier.rb} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename app/models/notifier/{event.rb => event_notifier.rb} (92%) rename app/models/notifier/{mention.rb => mention_notifier.rb} (82%) diff --git a/app/models/notifier/event.rb b/app/models/notifier/event_notifier.rb similarity index 92% rename from app/models/notifier/event.rb rename to app/models/notifier/event_notifier.rb index 0fc31914a..ea771bc15 100644 --- a/app/models/notifier/event.rb +++ b/app/models/notifier/event_notifier.rb @@ -1,4 +1,4 @@ -class Notifier::Event < Notifier +class Notifier::EventNotifier < Notifier delegate :card, :creator, to: :source delegate :watchers_and_subscribers, to: :card diff --git a/app/models/notifier/mention.rb b/app/models/notifier/mention_notifier.rb similarity index 82% rename from app/models/notifier/mention.rb rename to app/models/notifier/mention_notifier.rb index feea686f4..c3920df8a 100644 --- a/app/models/notifier/mention.rb +++ b/app/models/notifier/mention_notifier.rb @@ -1,4 +1,4 @@ -class Notifier::Mention < Notifier +class Notifier::MentionNotifier < Notifier alias mention source private From 25456046597936748e999f6d569dcffea4d27fd3 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 16:49:42 +0200 Subject: [PATCH 48/50] Add tests --- test/fixtures/mentions.yml | 4 +++ test/models/concerns/mentions_test.rb | 10 +------ .../event_notifier_test.rb} | 18 ++++++++++++- test/models/notifier/mention_notifier_test.rb | 27 +++++++++++++++++++ test/models/user/mentionable_test.rb | 11 ++++++++ 5 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/mentions.yml rename test/models/{notifier_test.rb => notifier/event_notifier_test.rb} (78%) create mode 100644 test/models/notifier/mention_notifier_test.rb diff --git a/test/fixtures/mentions.yml b/test/fixtures/mentions.yml new file mode 100644 index 000000000..70373600f --- /dev/null +++ b/test/fixtures/mentions.yml @@ -0,0 +1,4 @@ +logo_card_david_mention_by_jz: + source: logo (card) + mentioner: jz + mentionee: david diff --git a/test/models/concerns/mentions_test.rb b/test/models/concerns/mentions_test.rb index b9e5dc751..3e856ab16 100644 --- a/test/models/concerns/mentions_test.rb +++ b/test/models/concerns/mentions_test.rb @@ -5,19 +5,11 @@ class MentionsTest < ActiveSupport::TestCase Current.session = sessions(:david) end - test "collect mentions on create" do + test "create mentions when creating messages" do assert_difference -> { Mention.count }, +1 do perform_enqueued_jobs only: Mention::CreateJob do collections(:writebook).cards.create title: "Cleanup", description: "Did you finish up with the cleanup, @david?" end end end - - test "collect mentions on update" do - assert_difference -> { Mention.count }, +1 do - perform_enqueued_jobs only: Mention::CreateJob do - cards(:logo).update! description: "Did you finish up with the cleanup, @david?" - end - end - end end diff --git a/test/models/notifier_test.rb b/test/models/notifier/event_notifier_test.rb similarity index 78% rename from test/models/notifier_test.rb rename to test/models/notifier/event_notifier_test.rb index d27379464..ca4f0b4b5 100644 --- a/test/models/notifier_test.rb +++ b/test/models/notifier/event_notifier_test.rb @@ -1,6 +1,6 @@ require "test_helper" -class NotifierTest < ActiveSupport::TestCase +class Notifier::EventNotifierTest < ActiveSupport::TestCase test "for returns the matching notifier class for the event" do assert_kind_of Notifier::EventNotifier, Notifier.for(events(:logo_published)) end @@ -62,4 +62,20 @@ class NotifierTest < ActiveSupport::TestCase assert_empty notifications end + + test "don't create notifications on publish for mentionees" do + users(:kevin).mentioned_by(users(:david), at: cards(:logo)) + + assert_no_difference -> { users(:kevin).notifications.count } do + Notifier.for(events(:logo_published)).notify + end + end + + test "don't create notifications on comment for mentionees" do + users(:david).mentioned_by(users(:kevin), at: cards(:layout)) + + assert_no_difference -> { users(:david).notifications.count } do + Notifier.for(events(:layout_commented)).notify + end + end end diff --git a/test/models/notifier/mention_notifier_test.rb b/test/models/notifier/mention_notifier_test.rb new file mode 100644 index 000000000..833f55ec8 --- /dev/null +++ b/test/models/notifier/mention_notifier_test.rb @@ -0,0 +1,27 @@ +require "test_helper" + +class Notifier::EventNotifierTest < ActiveSupport::TestCase + test "for returns the matching notifier class for the mention" do + assert_kind_of Notifier::MentionNotifier, Notifier.for(mentions(:logo_card_david_mention_by_jz)) + end + + test "notify the mentionee" do + users(:kevin).mentioned_by(users(:david), at: cards(:logo)) + + assert_no_difference -> { users(:kevin).notifications.count } do + Notifier.for(mentions(:logo_card_david_mention_by_jz)).notify + end + end + + test "create notifications for mentionee" do + assert_no_difference -> { users(:david).notifications.count } do + Notifier.for(events(:layout_commented)).notify + end + end + + test "don't create notifications for self-mentions" do + assert_no_difference -> { users(:jz).notifications.count } do + Notifier.for(events(:layout_commented)).notify + end + end +end diff --git a/test/models/user/mentionable_test.rb b/test/models/user/mentionable_test.rb index 48fb339a1..7231553c0 100644 --- a/test/models/user/mentionable_test.rb +++ b/test/models/user/mentionable_test.rb @@ -4,4 +4,15 @@ class User::MentionableTest < ActiveSupport::TestCase test "mentionable handles" do assert_equal [ "dhh", "david", "davidh" ], User.new(name: "David Heinemeier-Hansson").mentionable_handles end + + test "mentioned by" do + assert_difference -> { users(:david).mentions.count }, +1 do + users(:david).mentioned_by users(:jz), at: cards(:logo) + end + + # No dups + assert_no_difference -> { users(:david).mentions.count }, +1 do + users(:david).mentioned_by users(:jz), at: cards(:logo) + end + end end From bd5a41a0a1cd921f1c5dcf3e455ce56e4d16ef91 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 17:25:11 +0200 Subject: [PATCH 49/50] Refresh migrations --- db/schema.rb | 1 - db/schema_cache.yml | 56 +++++++++++++++++++-------------------------- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 595cf2346..3a747df7f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -180,7 +180,6 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_23_114737) do t.integer "card_id", null: false t.datetime "created_at", null: false t.integer "creator_id", null: false - t.date "due_date" t.json "particulars", default: {} t.integer "summary_id", null: false t.datetime "updated_at", null: false diff --git a/db/schema_cache.yml b/db/schema_cache.yml index 9e58a9422..7859651e6 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -31,7 +31,7 @@ columns: default_function: collation: comment: - - &28 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &26 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: user_id cast_type: *1 @@ -374,7 +374,7 @@ columns: - *9 cards: - *5 - - &31 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &29 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: title cast_type: *6 @@ -399,12 +399,12 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: due_on - cast_type: &24 !ruby/object:ActiveRecord::Type::Date + cast_type: !ruby/object:ActiveRecord::Type::Date precision: scale: limit: timezone: - sql_type_metadata: &25 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata + sql_type_metadata: !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata sql_type: date type: :date limit: @@ -494,11 +494,11 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: all_access - cast_type: &32 !ruby/object:ActiveModel::Type::Boolean + cast_type: &30 !ruby/object:ActiveModel::Type::Boolean precision: scale: limit: - sql_type_metadata: &33 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata + sql_type_metadata: &31 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata sql_type: boolean type: :boolean limit: @@ -540,11 +540,11 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: particulars - cast_type: &26 !ruby/object:ActiveRecord::Type::Json + cast_type: &24 !ruby/object:ActiveRecord::Type::Json precision: scale: limit: - sql_type_metadata: &27 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata + sql_type_metadata: &25 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata sql_type: json type: :json limit: @@ -578,16 +578,6 @@ columns: collation: comment: - *21 - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: due_date - cast_type: *24 - sql_type_metadata: *25 - 'null': true - default: - default_function: - collation: - comment: filters: - *5 - *23 @@ -606,8 +596,8 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: fields - cast_type: *26 - sql_type_metadata: *27 + cast_type: *24 + sql_type_metadata: *25 'null': false default: "{}" default_function: @@ -627,7 +617,7 @@ columns: comment: filters_tags: - *18 - - &30 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &28 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: tag_id cast_type: *1 @@ -639,7 +629,7 @@ columns: comment: mentions: - *5 - - &29 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &27 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: source_type cast_type: *6 @@ -708,7 +698,7 @@ columns: - *9 notifications: - *5 - - *28 + - *26 - *8 - *9 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column @@ -721,7 +711,7 @@ columns: default_function: collation: comment: - - *29 + - *27 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: source_id @@ -745,7 +735,7 @@ columns: pins: - *5 - *21 - - *28 + - *26 - *8 - *9 reactions: @@ -805,7 +795,7 @@ columns: comment: sessions: - *5 - - *28 + - *26 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: ip_address @@ -831,12 +821,12 @@ columns: taggings: - *5 - *21 - - *30 + - *28 - *8 - *9 tags: - *5 - - *31 + - *29 - *8 - *9 users: @@ -865,8 +855,8 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: active - cast_type: *32 - sql_type_metadata: *33 + cast_type: *30 + sql_type_metadata: *31 'null': false default: true default_function: @@ -886,13 +876,13 @@ columns: comment: watches: - *5 - - *28 + - *26 - *21 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: watching - cast_type: *32 - sql_type_metadata: *33 + cast_type: *30 + sql_type_metadata: *31 'null': false default: true default_function: From 3793718e72074caa6bf1a6b03b48eb0f51a56389 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 23 Apr 2025 17:25:36 +0200 Subject: [PATCH 50/50] Fix concern I lost during merge --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 833c41580..90539679b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,5 @@ class User < ApplicationRecord - include Accessor, Assignee, Named, Role, Transferable + include Accessor, Assignee, Mentionable, Named, Role, Transferable has_one_attached :avatar