From 54f4bd5144ef29f3cfb86eb6b1f644c58420a7bf Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 24 Apr 2025 07:44:16 +0200 Subject: [PATCH] Generalize event's cards as eventable,add collection Lots to tidy up but this is working --- app/controllers/concerns/events_timeline.rb | 3 +- app/controllers/terminals_controller.rb | 2 +- app/helpers/events_helper.rb | 33 +++---- app/helpers/notifications_helper.rb | 6 +- app/models/card/eventable.rb | 6 +- app/models/event.rb | 12 ++- app/models/event_summary.rb | 2 +- app/models/notifier/event_notifier.rb | 8 +- app/views/events/_event.html.erb | 2 +- .../notification/_event.html.erb | 2 +- app/views/terminals/_event.html.erb | 6 +- db/schema.rb | 13 +-- db/schema_cache.yml | 90 ++++++++++++------- test/fixtures/events.yml | 27 ++++-- test/models/notifier/event_notifier_test.rb | 2 +- 15 files changed, 132 insertions(+), 82 deletions(-) diff --git a/app/controllers/concerns/events_timeline.rb b/app/controllers/concerns/events_timeline.rb index 0d50d2093..c051c370d 100644 --- a/app/controllers/concerns/events_timeline.rb +++ b/app/controllers/concerns/events_timeline.rb @@ -29,12 +29,11 @@ module EventsTimeline end def user_events - Event.where(card: user_cards) + Event.where(collection: collection_filter).where(eventable: user_cards) end def user_cards Current.user.accessible_cards .published_or_drafted_by(Current.user) - .where(collection_id: collection_filter) end end diff --git a/app/controllers/terminals_controller.rb b/app/controllers/terminals_controller.rb index 728ff9983..e2840773a 100644 --- a/app/controllers/terminals_controller.rb +++ b/app/controllers/terminals_controller.rb @@ -1,6 +1,6 @@ class TerminalsController < ApplicationController def show - @events = Event.where(card: Current.user.accessible_cards, creator: Current.user).chronologically.reverse_order.limit(20) + @events = Event.where(eventable: Current.user.accessible_cards, creator: Current.user).chronologically.reverse_order.limit(20) end def edit diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb index 53732b327..1bcacaff5 100644 --- a/app/helpers/events_helper.rb +++ b/app/helpers/events_helper.rb @@ -45,10 +45,13 @@ module EventsHelper start_time = day.beginning_of_day end_time = day.end_of_day - accessible_events = Event.joins(card: :collection) - .merge(Current.user.collections) + collections = Current.user.collections + collections = collections.where(id: params[:collection_ids]) if params[:collection_ids].present? + + # TODO: this needs tidying up + accessible_events = Event.where(eventable_type: "Card") .where(created_at: start_time..end_time) - .where(cards: { collection_id: params[:collection_ids].presence || Current.user.collection_ids }) + .where(collection: collections) headers = { "Added" => accessible_events.where(action: "published").count, @@ -66,30 +69,30 @@ module EventsHelper case event.action when "assigned" if event.assignees.include?(Current.user) - "#{ event.creator == Current.user ? "You" : event.creator.name } will handle #{ event.card.title }".html_safe + "#{ event.creator == Current.user ? "You" : event.creator.name } will handle #{ event.eventable.title }".html_safe else - "#{ event.creator == Current.user ? "You" : event.creator.name } assigned #{ event.assignees.pluck(:name).to_sentence } to #{ event.card.title }".html_safe + "#{ event.creator == Current.user ? "You" : event.creator.name } assigned #{ event.assignees.pluck(:name).to_sentence } to #{ event.eventable.title }".html_safe end when "unassigned" - "#{ event.creator == Current.user ? "You" : event.creator.name } unassigned #{ event.assignees.include?(Current.user) ? "yourself" : event.assignees.pluck(:name).to_sentence } from #{ event.card.title }".html_safe + "#{ event.creator == Current.user ? "You" : event.creator.name } unassigned #{ event.assignees.include?(Current.user) ? "yourself" : event.assignees.pluck(:name).to_sentence } from #{ event.eventable.title }".html_safe when "commented" - "#{ event.creator == Current.user ? "You" : event.creator.name } commented on #{ event.card.title }".html_safe + "#{ event.creator == Current.user ? "You" : event.creator.name } commented on #{ event.eventable.title }".html_safe when "published" - "#{ event.creator == Current.user ? "You" : event.creator.name } added #{ event.card.title }".html_safe + "#{ event.creator == Current.user ? "You" : event.creator.name } added #{ event.eventable.title }".html_safe when "closed" - "#{ event.creator == Current.user ? "You" : event.creator.name } closed #{ event.card.title }".html_safe + "#{ event.creator == Current.user ? "You" : event.creator.name } closed #{ event.eventable.title }".html_safe when "staged" - "#{event.creator == Current.user ? "You" : event.creator.name} moved #{ event.card.title } to the #{event.stage_name} stage".html_safe + "#{event.creator == Current.user ? "You" : event.creator.name} moved #{ event.eventable.title } to the #{event.stage_name} stage".html_safe when "unstaged" - "#{event.creator == Current.user ? "You" : event.creator.name} moved #{ event.card.title } out ofthe #{event.stage_name} stage".html_safe + "#{event.creator == Current.user ? "You" : event.creator.name} moved #{ event.eventable.title } out ofthe #{event.stage_name} stage".html_safe when "due_date_added" - "#{event.creator == Current.user ? "You" : event.creator.name} set the date to #{event.particulars.dig('particulars', 'due_date').to_date.strftime('%B %-d')} on #{ event.card.title }".html_safe + "#{event.creator == Current.user ? "You" : event.creator.name} set the date to #{event.particulars.dig('particulars', 'due_date').to_date.strftime('%B %-d')} on #{ event.eventable.title }".html_safe when "due_date_changed" - "#{event.creator == Current.user ? "You" : event.creator.name} changed the date to #{event.particulars.dig('particulars', 'due_date').to_date.strftime('%B %-d')} on #{ event.card.title }".html_safe + "#{event.creator == Current.user ? "You" : event.creator.name} changed the date to #{event.particulars.dig('particulars', 'due_date').to_date.strftime('%B %-d')} on #{ event.eventable.title }".html_safe when "due_date_removed" - "#{event.creator == Current.user ? "You" : event.creator.name} removed the date on #{ event.card.title }" + "#{event.creator == Current.user ? "You" : event.creator.name} removed the date on #{ event.eventable.title }" when "title_changed" - "#{event.creator == Current.user ? "You" : event.creator.name} renamed #{ event.card.title } (was: '#{event.particulars.dig('particulars', 'old_title')})'".html_safe + "#{event.creator == Current.user ? "You" : event.creator.name} renamed #{ event.eventable.title } (was: '#{event.particulars.dig('particulars', 'old_title')})'".html_safe end end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 5f61f99e9..cbb1774a0 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.eventable.title + when "assigned" then "Assigned to you: " + event.eventable.title + else event.eventable.title end end diff --git a/app/models/card/eventable.rb b/app/models/card/eventable.rb index 55a0245ea..eb06ead4a 100644 --- a/app/models/card/eventable.rb +++ b/app/models/card/eventable.rb @@ -2,7 +2,7 @@ module Card::Eventable extend ActiveSupport::Concern included do - has_many :events, dependent: :destroy + has_many :events, as: :eventable, dependent: :destroy before_create { self.last_active_at = Time.current } @@ -10,9 +10,9 @@ module Card::Eventable after_save :track_title_change, if: :saved_change_to_title? end - def track_event(action, creator: Current.user, **particulars) + def track_event(action, creator: Current.user, collection: self.collection, **particulars) if published? - find_or_capture_event_summary.events.create! action: action, creator: creator, card: self, particulars: particulars + find_or_capture_event_summary.events.create! action:, creator:, collection:, eventable: self, particulars: end end diff --git a/app/models/event.rb b/app/models/event.rb index 1fc2fc5b6..9659ace63 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,12 +1,15 @@ class Event < ApplicationRecord include Notifiable, Particulars + belongs_to :collection belongs_to :creator, class_name: "User" - belongs_to :card + belongs_to :eventable, polymorphic: true + belongs_to :summary, touch: true, class_name: "EventSummary" scope :chronologically, -> { order created_at: :asc, id: :desc } - after_create -> { card.touch(:last_active_at) } + # TODO: Remove dependency with last_active_at via hook + after_create -> { eventable.touch(:last_active_at) } def action super.inquiry @@ -16,11 +19,12 @@ class Event < ApplicationRecord if action.commented? comment else - card + eventable end end + # TODO: This doesn't belong here anymore def initial_assignment? - action == "published" && card.assigned_to?(creator) + action == "published" && eventable.assigned_to?(creator) end end diff --git a/app/models/event_summary.rb b/app/models/event_summary.rb index 99af2d0e1..241d3cc3c 100644 --- a/app/models/event_summary.rb +++ b/app/models/event_summary.rb @@ -1,7 +1,7 @@ class EventSummary < ApplicationRecord include Messageable - has_many :events, -> { chronologically }, dependent: :delete_all, foreign_key: :summary_id + has_many :events, -> { chronologically }, dependent: :delete_all, inverse_of: :summary # FIXME: Consider persisting the body and compute at write time. def body diff --git a/app/models/notifier/event_notifier.rb b/app/models/notifier/event_notifier.rb index ea771bc15..e7b11b4d8 100644 --- a/app/models/notifier/event_notifier.rb +++ b/app/models/notifier/event_notifier.rb @@ -1,12 +1,12 @@ class Notifier::EventNotifier < Notifier - delegate :card, :creator, to: :source + delegate :creator, to: :source delegate :watchers_and_subscribers, to: :card private def recipients case source.action when "assigned" - source.assignees.excluding(card.collection.access_only_users) + source.assignees.excluding(source.collection.access_only_users) when "published" watchers_and_subscribers(include_only_watching: true).without(creator, *card.mentionees) when "commented" @@ -15,4 +15,8 @@ class Notifier::EventNotifier < Notifier watchers_and_subscribers.without(creator) end end + + def card + source.eventable + end end diff --git a/app/views/events/_event.html.erb b/app/views/events/_event.html.erb index 2d37c5665..4bde70fbb 100644 --- a/app/views/events/_event.html.erb +++ b/app/views/events/_event.html.erb @@ -1,4 +1,4 @@ -<% card = event.card %> +<% card = event.eventable %> <% link = event.action == "commented" ? collection_card_path(card.collection, card, anchor: dom_id(event.comment)) : collection_card_path(card.collection, card) %> <%= link_to link, diff --git a/app/views/notifications/notification/_event.html.erb b/app/views/notifications/notification/_event.html.erb index 9a02db061..2dd8d16ee 100644 --- a/app/views/notifications/notification/_event.html.erb +++ b/app/views/notifications/notification/_event.html.erb @@ -4,4 +4,4 @@
<%= event_notification_body(event) %>
-
<%= notification.source.card.collection.name %> · <%= local_datetime_tag(notification.created_at, style: :ago) %>
+
<%= notification.source.collection.name %> · <%= local_datetime_tag(notification.created_at, style: :ago) %>
diff --git a/app/views/terminals/_event.html.erb b/app/views/terminals/_event.html.erb index fd67cd405..b82c37ba6 100644 --- a/app/views/terminals/_event.html.erb +++ b/app/views/terminals/_event.html.erb @@ -1,5 +1,5 @@
  • - <%= link_to event_action_sentence(event), collection_card_path(event.card.collection, event.card), - class: "terminal__link", + <%= link_to event_action_sentence(event), collection_card_path(event.collection, event.eventable), + class: "terminal__link", data: { turbo_frame: "_top" } %> -
    • \ No newline at end of file + diff --git a/db/schema.rb b/db/schema.rb index 3a747df7f..ae4f7e3e4 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_23_114737) do +ActiveRecord::Schema[8.1].define(version: 2025_04_24_051059) do create_table "accesses", force: :cascade do |t| t.integer "collection_id", null: false t.datetime "created_at", null: false @@ -177,14 +177,17 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_23_114737) do create_table "events", force: :cascade do |t| t.string "action", null: false - t.integer "card_id", null: false + t.integer "collection_id", null: false t.datetime "created_at", null: false t.integer "creator_id", null: false + t.integer "eventable_id", null: false + t.string "eventable_type", null: false t.json "particulars", default: {} t.integer "summary_id", null: false t.datetime "updated_at", null: false - t.index ["card_id"], name: "index_events_on_card_id" + t.index ["collection_id"], name: "index_events_on_collection_id" t.index ["creator_id"], name: "index_events_on_creator_id" + t.index ["eventable_type", "eventable_id"], name: "index_events_on_eventable" t.index ["summary_id", "action"], name: "index_events_on_summary_id_and_action" end @@ -237,7 +240,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_23_114737) do t.datetime "created_at", null: false t.integer "creator_id" t.datetime "read_at" - t.integer "source_id" + t.integer "source_id", null: false t.string "source_type", null: false t.datetime "updated_at", null: false t.integer "user_id", null: false @@ -335,7 +338,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_04_23_114737) do add_foreign_key "closures", "cards" add_foreign_key "closures", "users" add_foreign_key "collections", "workflows" - add_foreign_key "events", "cards" + add_foreign_key "events", "collections" add_foreign_key "events", "event_summaries", column: "summary_id" add_foreign_key "mentions", "users", column: "mentionee_id" add_foreign_key "mentions", "users", column: "mentioner_id" diff --git a/db/schema_cache.yml b/db/schema_cache.yml index 42cd125c0..918fa9373 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -80,7 +80,7 @@ columns: default_function: collation: comment: - - &27 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &28 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: user_id cast_type: *1 @@ -435,7 +435,7 @@ columns: default_function: collation: comment: - - &29 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &30 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: title cast_type: *7 @@ -489,11 +489,11 @@ columns: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: all_access - cast_type: &30 !ruby/object:ActiveModel::Type::Boolean + cast_type: &31 !ruby/object:ActiveModel::Type::Boolean precision: scale: limit: - sql_type_metadata: &31 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata + sql_type_metadata: &32 !ruby/object:ActiveRecord::ConnectionAdapters::SqlTypeMetadata sql_type: boolean type: :boolean limit: @@ -545,9 +545,29 @@ columns: default_function: collation: comment: - - *21 + - *22 - *5 - *23 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: eventable_id + cast_type: *1 + sql_type_metadata: *2 + 'null': false + default: + default_function: + collation: + comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: eventable_type + cast_type: *7 + sql_type_metadata: *8 + 'null': false + default: + default_function: + collation: + comment: - *6 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: @@ -617,7 +637,7 @@ columns: comment: filters_tags: - *18 - - &28 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &29 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: tag_id cast_type: *1 @@ -650,7 +670,7 @@ columns: default_function: collation: comment: - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &26 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: source_id cast_type: *1 @@ -660,7 +680,7 @@ columns: default_function: collation: comment: - - &26 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + - &27 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: source_type cast_type: *7 @@ -719,25 +739,16 @@ columns: 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: - *26 - - *9 - *27 + - *9 + - *28 pins: - *21 - *5 - *6 - *9 - - *27 + - *28 reactions: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: @@ -817,24 +828,24 @@ columns: default_function: collation: comment: - - *27 + - *28 taggings: - *21 - *5 - *6 - - *28 + - *29 - *9 tags: - *5 - *6 - - *29 + - *30 - *9 users: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: active - cast_type: *30 - sql_type_metadata: *31 + cast_type: *31 + sql_type_metadata: *32 'null': false default: true default_function: @@ -879,12 +890,12 @@ columns: - *5 - *6 - *9 - - *27 + - *28 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: name: watching - cast_type: *30 - sql_type_metadata: *31 + cast_type: *31 + sql_type_metadata: *32 'null': false default: true default_function: @@ -1493,10 +1504,10 @@ indexes: events: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: events - name: index_events_on_card_id + name: index_events_on_collection_id unique: false columns: - - card_id + - collection_id lengths: {} orders: {} opclasses: {} @@ -1523,6 +1534,23 @@ indexes: nulls_not_distinct: comment: valid: true + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: events + name: index_events_on_eventable + unique: false + columns: + - eventable_type + - eventable_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 @@ -1998,4 +2026,4 @@ indexes: comment: valid: true workflows: [] -version: 20250423114737 +version: 20250424051059 diff --git a/test/fixtures/events.yml b/test/fixtures/events.yml index 61746d7fd..11c8b46e9 100644 --- a/test/fixtures/events.yml +++ b/test/fixtures/events.yml @@ -1,13 +1,15 @@ logo_published: creator: david - card: logo + collection: writebook + eventable: logo (Card) action: published summary: logo_initial_activity created_at: <%= 1.week.ago %> logo_assignment_jz: creator: david - card: logo + collection: writebook + eventable: logo (Card) action: assigned summary: logo_initial_activity particulars: <%= { assignee_ids: [ ActiveRecord::FixtureSet.identify(:jz) ] }.to_json %> @@ -15,7 +17,8 @@ logo_assignment_jz: logo_assignment_km: creator: david - card: logo + collection: writebook + eventable: logo (Card) action: assigned summary: logo_second_activity particulars: <%= { assignee_ids: [ ActiveRecord::FixtureSet.identify(:kevin) ] }.to_json %> @@ -23,14 +26,16 @@ logo_assignment_km: layout_published: creator: david - card: layout + collection: writebook + eventable: layout (Card) action: published summary: layout_initial_activity created_at: <%= 1.week.ago %> layout_commented: creator: david - card: layout + collection: writebook + eventable: layout (Card) action: commented summary: layout_initial_activity particulars: <%= { comment_id: ActiveRecord::FixtureSet.identify(:layout_overflowing_david) }.to_json %> @@ -38,7 +43,8 @@ layout_commented: layout_assignment_jz: creator: david - card: layout + collection: writebook + eventable: layout (Card) action: assigned summary: layout_initial_activity particulars: <%= { assignee_ids: [ ActiveRecord::FixtureSet.identify(:jz) ] }.to_json %> @@ -46,21 +52,24 @@ layout_assignment_jz: text_published: creator: kevin - card: text + collection: writebook + eventable: text (Card) action: published summary: text_initial_activity created_at: <%= 1.week.ago %> shipping_published: creator: kevin - card: shipping + collection: writebook + eventable: shipping (Card) action: published summary: shipping_initial_activity created_at: <%= 1.week.ago %> shipping_closed: creator: kevin - card: shipping + collection: writebook + eventable: shipping (Card) action: closed summary: shipping_initial_activity created_at: <%= 2.days.ago %> diff --git a/test/models/notifier/event_notifier_test.rb b/test/models/notifier/event_notifier_test.rb index ca4f0b4b5..97b7f5f68 100644 --- a/test/models/notifier/event_notifier_test.rb +++ b/test/models/notifier/event_notifier_test.rb @@ -43,7 +43,7 @@ class Notifier::EventNotifierTest < ActiveSupport::TestCase test "links to the card" do Notifier.for(events(:logo_published)).notify - assert_equal cards(:logo), Notification.last.source.card + assert_equal cards(:logo), Notification.last.source.eventable end test "assignment events only create a notification for the assignee" do