From d0e1dd96bbf2b25d457d5e39d8b5a85184592d67 Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Fri, 26 Jul 2024 11:12:12 -0600 Subject: [PATCH 01/17] Reminder scheduler --- app/jobs/event_reminder_job.rb | 8 ++++ app/models/event.rb | 41 +++++++++++++++---- ...25222556_add_reminder_details_to_events.rb | 5 +++ db/schema.rb | 3 +- spec/factories/events.rb | 19 +++++---- spec/models/event_spec.rb | 19 +++++---- 6 files changed, 67 insertions(+), 28 deletions(-) create mode 100644 app/jobs/event_reminder_job.rb create mode 100644 db/migrate/20240725222556_add_reminder_details_to_events.rb diff --git a/app/jobs/event_reminder_job.rb b/app/jobs/event_reminder_job.rb new file mode 100644 index 00000000..41c5aa44 --- /dev/null +++ b/app/jobs/event_reminder_job.rb @@ -0,0 +1,8 @@ +class EventReminderJob < ApplicationJob + def perform(event_id, reminder_id) + event = Event.find(event_id) + nil if event.reminder_details["15_min_reminder_id"] != reminder_id + + # Call Notfier service + end +end diff --git a/app/models/event.rb b/app/models/event.rb index 297c60e3..0f9f1ded 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -2,15 +2,16 @@ # # Table name: events # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# reminder_details :json +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # @@ -18,6 +19,8 @@ # index_events_on_location_id (location_id) # class Event < ApplicationRecord + REMINDER_CADENCE = 15.minutes + belongs_to :location belongs_to :conference @@ -30,4 +33,24 @@ class Event < ApplicationRecord validates :ends_at, presence: true validates_datetime :ends_at, after: :starts_at + + after_save_commit :schedule_reminder + + private + + # TODO: abtract reminder logic + def schedule_reminder + return unless should_schedule_reminder? + + reminder_details["15_min_reminder_id"] = SecureRandom.uuid + save + + deadline = starts_at - REMINDER_CADENCE + EventReminderJob.set(wait_until: deadline).perform_later(id, reminder_details["15_min_reminder_id"]) + end + + def should_schedule_reminder? + saved_change_to_starts_at? && + Time.zone.now < (starts_at - REMINDER_CADENCE) + end end diff --git a/db/migrate/20240725222556_add_reminder_details_to_events.rb b/db/migrate/20240725222556_add_reminder_details_to_events.rb new file mode 100644 index 00000000..78140cc3 --- /dev/null +++ b/db/migrate/20240725222556_add_reminder_details_to_events.rb @@ -0,0 +1,5 @@ +class AddReminderDetailsToEvents < ActiveRecord::Migration[7.1] + def change + add_column :events, :reminder_details, :jsonb, default: {} + end +end diff --git a/db/schema.rb b/db/schema.rb index d7c97655..964c6492 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[7.1].define(version: 2024_06_28_211903) do +ActiveRecord::Schema[7.1].define(version: 2024_07_25_222556) do create_table "conferences", force: :cascade do |t| t.string "name", null: false t.datetime "created_at", null: false @@ -26,6 +26,7 @@ t.integer "conference_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.json "reminder_details", default: {} t.index ["conference_id"], name: "index_events_on_conference_id" t.index ["location_id"], name: "index_events_on_location_id" end diff --git a/spec/factories/events.rb b/spec/factories/events.rb index c60be2f5..11f223f4 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -2,15 +2,16 @@ # # Table name: events # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# reminder_details :json +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 3f088cc8..1e6c35fa 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -2,15 +2,16 @@ # # Table name: events # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# reminder_details :json +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # From 730fd61124bd1d010a1bf0c84e54954ba5887cc4 Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Fri, 26 Jul 2024 12:01:03 -0600 Subject: [PATCH 02/17] Noticed setup and create Event Notifier --- Gemfile | 1 + Gemfile.lock | 3 ++ app/jobs/event_reminder_job.rb | 4 +- app/mailers/event_mailer.rb | 7 ++++ app/models/user.rb | 2 + app/notifiers/application_notifier.rb | 2 + app/notifiers/event_notifier.rb | 7 ++++ app/views/event_mailer/reminder.html.erb | 1 + ...726171427_create_noticed_tables.noticed.rb | 37 +++++++++++++++++++ ...ications_count_to_noticed_event.noticed.rb | 6 +++ db/schema.rb | 26 ++++++++++++- 11 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 app/mailers/event_mailer.rb create mode 100644 app/notifiers/application_notifier.rb create mode 100644 app/notifiers/event_notifier.rb create mode 100644 app/views/event_mailer/reminder.html.erb create mode 100644 db/migrate/20240726171427_create_noticed_tables.noticed.rb create mode 100644 db/migrate/20240726171428_add_notifications_count_to_noticed_event.noticed.rb diff --git a/Gemfile b/Gemfile index 2203bcb3..dbe5c989 100644 --- a/Gemfile +++ b/Gemfile @@ -33,6 +33,7 @@ gem "avo", ">= 3.2.1" # Other gem "bootsnap", require: false gem "inline_svg" +gem "noticed" gem "puma", ">= 5.0" gem "ransack" gem "rqrcode", "~> 2.0" diff --git a/Gemfile.lock b/Gemfile.lock index 80926c09..880af89a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -294,6 +294,8 @@ GEM racc (~> 1.4) nokogiri (1.16.6-x86_64-linux) racc (~> 1.4) + noticed (2.4.0) + rails (>= 6.1.0) pagy (8.6.3) parallel (1.25.1) parser (3.3.3.0) @@ -558,6 +560,7 @@ DEPENDENCIES inline_svg letter_opener (~> 1.10) mission_control-jobs + noticed propshaft pry-byebug puma (>= 5.0) diff --git a/app/jobs/event_reminder_job.rb b/app/jobs/event_reminder_job.rb index 41c5aa44..76769d8f 100644 --- a/app/jobs/event_reminder_job.rb +++ b/app/jobs/event_reminder_job.rb @@ -1,8 +1,8 @@ class EventReminderJob < ApplicationJob def perform(event_id, reminder_id) event = Event.find(event_id) - nil if event.reminder_details["15_min_reminder_id"] != reminder_id + return if event.reminder_details["15_min_reminder_id"] != reminder_id - # Call Notfier service + EventNotifier.with(record: event).deliver(event.users) end end diff --git a/app/mailers/event_mailer.rb b/app/mailers/event_mailer.rb new file mode 100644 index 00000000..34e8b13f --- /dev/null +++ b/app/mailers/event_mailer.rb @@ -0,0 +1,7 @@ +class EventMailer < ApplicationMailer + def reminder + recipient = params[:recipient] + + mail(to: recipient.email) + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 0c10c4f2..ff062d53 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -26,6 +26,8 @@ class User < ApplicationRecord has_and_belongs_to_many :events + has_many :notifications, as: :recipient, dependent: :destroy, class_name: "Noticed::Notification" + validates :email, presence: true, uniqueness: true validates :password_digest, presence: true validates :password, length: {minimum: 8}, if: -> { password.present? } diff --git a/app/notifiers/application_notifier.rb b/app/notifiers/application_notifier.rb new file mode 100644 index 00000000..90c8fd4e --- /dev/null +++ b/app/notifiers/application_notifier.rb @@ -0,0 +1,2 @@ +class ApplicationNotifier < Noticed::Event +end diff --git a/app/notifiers/event_notifier.rb b/app/notifiers/event_notifier.rb new file mode 100644 index 00000000..5ade0453 --- /dev/null +++ b/app/notifiers/event_notifier.rb @@ -0,0 +1,7 @@ +class EventNotifier < ApplicationNotifier + deliver_by :email do |config| + config.mailer = "EventMailer" + config.method = "reminder" + config.params = -> { params } + end +end diff --git a/app/views/event_mailer/reminder.html.erb b/app/views/event_mailer/reminder.html.erb new file mode 100644 index 00000000..8dd57ceb --- /dev/null +++ b/app/views/event_mailer/reminder.html.erb @@ -0,0 +1 @@ +

The event will start soon

\ No newline at end of file diff --git a/db/migrate/20240726171427_create_noticed_tables.noticed.rb b/db/migrate/20240726171427_create_noticed_tables.noticed.rb new file mode 100644 index 00000000..cf7b236e --- /dev/null +++ b/db/migrate/20240726171427_create_noticed_tables.noticed.rb @@ -0,0 +1,37 @@ +# This migration comes from noticed (originally 20231215190233) +class CreateNoticedTables < ActiveRecord::Migration[6.1] + def change + primary_key_type, foreign_key_type = primary_and_foreign_key_types + create_table :noticed_events, id: primary_key_type do |t| + t.string :type + t.belongs_to :record, polymorphic: true, type: foreign_key_type + if t.respond_to?(:jsonb) + t.jsonb :params + else + t.json :params + end + + t.timestamps + end + + create_table :noticed_notifications, id: primary_key_type do |t| + t.string :type + t.belongs_to :event, null: false, type: foreign_key_type + t.belongs_to :recipient, polymorphic: true, null: false, type: foreign_key_type + t.datetime :read_at + t.datetime :seen_at + + t.timestamps + end + end + + private + + def primary_and_foreign_key_types + config = Rails.configuration.generators + setting = config.options[config.orm][:primary_key_type] + primary_key_type = setting || :primary_key + foreign_key_type = setting || :bigint + [primary_key_type, foreign_key_type] + end +end diff --git a/db/migrate/20240726171428_add_notifications_count_to_noticed_event.noticed.rb b/db/migrate/20240726171428_add_notifications_count_to_noticed_event.noticed.rb new file mode 100644 index 00000000..cf4f56fb --- /dev/null +++ b/db/migrate/20240726171428_add_notifications_count_to_noticed_event.noticed.rb @@ -0,0 +1,6 @@ +# This migration comes from noticed (originally 20240129184740) +class AddNotificationsCountToNoticedEvent < ActiveRecord::Migration[6.1] + def change + add_column :noticed_events, :notifications_count, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 964c6492..f6eb8282 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[7.1].define(version: 2024_07_25_222556) do +ActiveRecord::Schema[7.1].define(version: 2024_07_26_171428) do create_table "conferences", force: :cascade do |t| t.string "name", null: false t.datetime "created_at", null: false @@ -70,6 +70,30 @@ t.index ["name", "conference_id"], name: "index_locations_on_name_and_conference_id", unique: true end + create_table "noticed_events", force: :cascade do |t| + t.string "type" + t.string "record_type" + t.bigint "record_id" + t.json "params" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.integer "notifications_count" + t.index ["record_type", "record_id"], name: "index_noticed_events_on_record" + end + + create_table "noticed_notifications", force: :cascade do |t| + t.string "type" + t.bigint "event_id", null: false + t.string "recipient_type", null: false + t.bigint "recipient_id", null: false + t.datetime "read_at", precision: nil + t.datetime "seen_at", precision: nil + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["event_id"], name: "index_noticed_notifications_on_event_id" + t.index ["recipient_type", "recipient_id"], name: "index_noticed_notifications_on_recipient" + end + create_table "profiles", force: :cascade do |t| t.string "name" t.string "bio" From 5d4eaf0b4f293b80567af6e4830a4edd9b226f31 Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Fri, 26 Jul 2024 17:53:40 -0600 Subject: [PATCH 03/17] Include multiple reminder cadence --- app/jobs/event_reminder_job.rb | 6 ++--- app/models/event.rb | 30 ++++++++++++++++-------- app/notifiers/event_notifier.rb | 12 ++++++++++ app/views/event_mailer/reminder.html.erb | 3 ++- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/app/jobs/event_reminder_job.rb b/app/jobs/event_reminder_job.rb index 76769d8f..2610bca4 100644 --- a/app/jobs/event_reminder_job.rb +++ b/app/jobs/event_reminder_job.rb @@ -1,8 +1,8 @@ class EventReminderJob < ApplicationJob - def perform(event_id, reminder_id) + def perform(event_id:, reminder_type:, reminder_id:) event = Event.find(event_id) - return if event.reminder_details["15_min_reminder_id"] != reminder_id + return if event.reminder_details["#{reminder_type}_id"] != reminder_id - EventNotifier.with(record: event).deliver(event.users) + EventNotifier.with(record: event, reminder_type: reminder_type).deliver(event.users) end end diff --git a/app/models/event.rb b/app/models/event.rb index 0f9f1ded..bc23a599 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -19,7 +19,11 @@ # index_events_on_location_id (location_id) # class Event < ApplicationRecord - REMINDER_CADENCE = 15.minutes + REMINDER_CADENCE = { + first_reminder: 15.minutes, + second_reminder: 10.minutes, + last_reminder: 0.minutes + }.freeze belongs_to :location belongs_to :conference @@ -40,17 +44,23 @@ class Event < ApplicationRecord # TODO: abtract reminder logic def schedule_reminder - return unless should_schedule_reminder? + return unless saved_change_to_starts_at? - reminder_details["15_min_reminder_id"] = SecureRandom.uuid - save + REMINDER_CADENCE.each do |reminder_type, cadence| + deadline = starts_at - cadence + next unless deadline > Time.zone.now - deadline = starts_at - REMINDER_CADENCE - EventReminderJob.set(wait_until: deadline).perform_later(id, reminder_details["15_min_reminder_id"]) - end + reminder_id = SecureRandom.uuid + reminder_details["#{reminder_type}_id"] = reminder_id + save - def should_schedule_reminder? - saved_change_to_starts_at? && - Time.zone.now < (starts_at - REMINDER_CADENCE) + EventReminderJob + .set(wait_until: deadline) + .perform_later( + event_id: id, + reminder_type: reminder_type, + reminder_id: reminder_id + ) + end end end diff --git a/app/notifiers/event_notifier.rb b/app/notifiers/event_notifier.rb index 5ade0453..0247984f 100644 --- a/app/notifiers/event_notifier.rb +++ b/app/notifiers/event_notifier.rb @@ -4,4 +4,16 @@ class EventNotifier < ApplicationNotifier config.method = "reminder" config.params = -> { params } end + + notification_methods do + def time_to_start_message + cadence = Event::REMINDER_CADENCE.fetch(params[:reminder_type]) + + if params[:reminder_type] == :last_reminder + "Starting Now" + else + "Starting in #{cadence} minutes" + end + end + end end diff --git a/app/views/event_mailer/reminder.html.erb b/app/views/event_mailer/reminder.html.erb index 8dd57ceb..be676342 100644 --- a/app/views/event_mailer/reminder.html.erb +++ b/app/views/event_mailer/reminder.html.erb @@ -1 +1,2 @@ -

The event will start soon

\ No newline at end of file +

<%= params[:notification].time_to_start_message %>

+

<%= params[:record].title %>

\ No newline at end of file From a34201cd026c35c87f3c90f08389247103229ea2 Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Fri, 26 Jul 2024 18:31:39 -0600 Subject: [PATCH 04/17] Notifications preferences --- Gemfile | 1 + Gemfile.lock | 3 +++ app/models/user.rb | 3 +++ app/notifiers/event_notifier.rb | 2 +- .../20240726235658_add_notifications_preferences_to_users.rb | 5 +++++ db/schema.rb | 3 ++- spec/factories/users.rb | 1 + spec/models/user_spec.rb | 1 + 8 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20240726235658_add_notifications_preferences_to_users.rb diff --git a/Gemfile b/Gemfile index dbe5c989..0ed9314f 100644 --- a/Gemfile +++ b/Gemfile @@ -37,6 +37,7 @@ gem "noticed" gem "puma", ">= 5.0" gem "ransack" gem "rqrcode", "~> 2.0" +gem "store_attribute" gem "tzinfo-data", platforms: %i[windows jruby] gem "validates_timeliness", "~> 7.0.0.beta1" diff --git a/Gemfile.lock b/Gemfile.lock index 880af89a..b6dc8e02 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -486,6 +486,8 @@ GEM rubocop-performance (~> 1.21.0) stimulus-rails (1.3.3) railties (>= 6.0.0) + store_attribute (1.2.0) + activerecord (>= 6.0) stringio (3.1.1) strscan (3.1.0) tailwindcss-rails (2.6.1-arm64-darwin) @@ -582,6 +584,7 @@ DEPENDENCIES sqlite3 (~> 1.4) standard stimulus-rails + store_attribute tailwindcss-rails (~> 2.6) turbo-rails tzinfo-data diff --git a/app/models/user.rb b/app/models/user.rb index ff062d53..c9d214ad 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -6,6 +6,7 @@ # email :string not null # in_app_notifications_enabled :boolean default(TRUE), not null # mail_notifications_enabled :boolean default(TRUE), not null +# notification_preferences :json # password_digest :string not null # role :string # created_at :datetime not null @@ -34,6 +35,8 @@ class User < ApplicationRecord enum role: {user: "user", admin: "admin"} + store_attribute :notification_preferences, :email_notifications_enabled?, :boolean, default: -> { true } + generates_token_for :password_reset, expires_in: PASSWORD_RESET_EXPIRATION do password_salt&.last(10) end diff --git a/app/notifiers/event_notifier.rb b/app/notifiers/event_notifier.rb index 0247984f..ca58a276 100644 --- a/app/notifiers/event_notifier.rb +++ b/app/notifiers/event_notifier.rb @@ -2,7 +2,7 @@ class EventNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "EventMailer" config.method = "reminder" - config.params = -> { params } + config.if = -> { recipient.email_notifications_enabled? } end notification_methods do diff --git a/db/migrate/20240726235658_add_notifications_preferences_to_users.rb b/db/migrate/20240726235658_add_notifications_preferences_to_users.rb new file mode 100644 index 00000000..ef2e9b02 --- /dev/null +++ b/db/migrate/20240726235658_add_notifications_preferences_to_users.rb @@ -0,0 +1,5 @@ +class AddNotificationsPreferencesToUsers < ActiveRecord::Migration[7.1] + def change + add_column :users, :notification_preferences, :jsonb, default: {} + end +end diff --git a/db/schema.rb b/db/schema.rb index f6eb8282..51071399 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[7.1].define(version: 2024_07_26_171428) do +ActiveRecord::Schema[7.1].define(version: 2024_07_26_235658) do create_table "conferences", force: :cascade do |t| t.string "name", null: false t.datetime "created_at", null: false @@ -129,6 +129,7 @@ t.string "password_digest", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.json "notification_preferences", default: {} t.index ["email"], name: "index_users_on_email", unique: true end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 7f25f9e4..f7c0b951 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -6,6 +6,7 @@ # email :string not null # in_app_notifications_enabled :boolean default(TRUE), not null # mail_notifications_enabled :boolean default(TRUE), not null +# notification_preferences :json # password_digest :string not null # role :string # created_at :datetime not null diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index db4430e7..2fdeaf66 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -6,6 +6,7 @@ # email :string not null # in_app_notifications_enabled :boolean default(TRUE), not null # mail_notifications_enabled :boolean default(TRUE), not null +# notification_preferences :json # password_digest :string not null # role :string # created_at :datetime not null From 408470bf8cbee79f7a1f5b0a5e31d226815792ac Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Fri, 26 Jul 2024 18:51:18 -0600 Subject: [PATCH 05/17] Abstract reminder logic from Event model --- app/models/event.rb | 32 +---------------------- app/models/event/reminder_scheduler.rb | 36 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 31 deletions(-) create mode 100644 app/models/event/reminder_scheduler.rb diff --git a/app/models/event.rb b/app/models/event.rb index bc23a599..8fe43825 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -19,11 +19,7 @@ # index_events_on_location_id (location_id) # class Event < ApplicationRecord - REMINDER_CADENCE = { - first_reminder: 15.minutes, - second_reminder: 10.minutes, - last_reminder: 0.minutes - }.freeze + include ReminderScheduler belongs_to :location belongs_to :conference @@ -37,30 +33,4 @@ class Event < ApplicationRecord validates :ends_at, presence: true validates_datetime :ends_at, after: :starts_at - - after_save_commit :schedule_reminder - - private - - # TODO: abtract reminder logic - def schedule_reminder - return unless saved_change_to_starts_at? - - REMINDER_CADENCE.each do |reminder_type, cadence| - deadline = starts_at - cadence - next unless deadline > Time.zone.now - - reminder_id = SecureRandom.uuid - reminder_details["#{reminder_type}_id"] = reminder_id - save - - EventReminderJob - .set(wait_until: deadline) - .perform_later( - event_id: id, - reminder_type: reminder_type, - reminder_id: reminder_id - ) - end - end end diff --git a/app/models/event/reminder_scheduler.rb b/app/models/event/reminder_scheduler.rb new file mode 100644 index 00000000..398101d3 --- /dev/null +++ b/app/models/event/reminder_scheduler.rb @@ -0,0 +1,36 @@ +module Event::ReminderScheduler + extend ActiveSupport::Concern + + REMINDER_CADENCE = { + first_reminder: 15.minutes, + second_reminder: 10.minutes, + last_reminder: 0.minutes + }.freeze + + included do + after_save_commit :schedule_reminder + end + + private + + def schedule_reminder + return unless saved_change_to_starts_at? + + REMINDER_CADENCE.each do |reminder_type, cadence| + deadline = starts_at - cadence + next unless deadline > Time.zone.now + + reminder_id = SecureRandom.uuid + reminder_details["#{reminder_type}_id"] = reminder_id + save + + EventReminderJob + .set(wait_until: deadline) + .perform_later( + event_id: id, + reminder_type: reminder_type, + reminder_id: reminder_id + ) + end + end +end From 3147d36235f7cd3b47c6044e03f006505218e20b Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Fri, 26 Jul 2024 19:06:11 -0600 Subject: [PATCH 06/17] Undo notifications preferences --- Gemfile | 1 - Gemfile.lock | 3 --- app/models/user.rb | 3 --- app/notifiers/event_notifier.rb | 2 +- .../20240726235658_add_notifications_preferences_to_users.rb | 5 ----- db/schema.rb | 3 +-- spec/factories/users.rb | 1 - spec/models/user_spec.rb | 1 - 8 files changed, 2 insertions(+), 17 deletions(-) delete mode 100644 db/migrate/20240726235658_add_notifications_preferences_to_users.rb diff --git a/Gemfile b/Gemfile index 0ed9314f..dbe5c989 100644 --- a/Gemfile +++ b/Gemfile @@ -37,7 +37,6 @@ gem "noticed" gem "puma", ">= 5.0" gem "ransack" gem "rqrcode", "~> 2.0" -gem "store_attribute" gem "tzinfo-data", platforms: %i[windows jruby] gem "validates_timeliness", "~> 7.0.0.beta1" diff --git a/Gemfile.lock b/Gemfile.lock index b6dc8e02..880af89a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -486,8 +486,6 @@ GEM rubocop-performance (~> 1.21.0) stimulus-rails (1.3.3) railties (>= 6.0.0) - store_attribute (1.2.0) - activerecord (>= 6.0) stringio (3.1.1) strscan (3.1.0) tailwindcss-rails (2.6.1-arm64-darwin) @@ -584,7 +582,6 @@ DEPENDENCIES sqlite3 (~> 1.4) standard stimulus-rails - store_attribute tailwindcss-rails (~> 2.6) turbo-rails tzinfo-data diff --git a/app/models/user.rb b/app/models/user.rb index c9d214ad..ff062d53 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -6,7 +6,6 @@ # email :string not null # in_app_notifications_enabled :boolean default(TRUE), not null # mail_notifications_enabled :boolean default(TRUE), not null -# notification_preferences :json # password_digest :string not null # role :string # created_at :datetime not null @@ -35,8 +34,6 @@ class User < ApplicationRecord enum role: {user: "user", admin: "admin"} - store_attribute :notification_preferences, :email_notifications_enabled?, :boolean, default: -> { true } - generates_token_for :password_reset, expires_in: PASSWORD_RESET_EXPIRATION do password_salt&.last(10) end diff --git a/app/notifiers/event_notifier.rb b/app/notifiers/event_notifier.rb index ca58a276..52c42642 100644 --- a/app/notifiers/event_notifier.rb +++ b/app/notifiers/event_notifier.rb @@ -2,7 +2,7 @@ class EventNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "EventMailer" config.method = "reminder" - config.if = -> { recipient.email_notifications_enabled? } + config.if = -> { recipient.profile.mail_notifications } end notification_methods do diff --git a/db/migrate/20240726235658_add_notifications_preferences_to_users.rb b/db/migrate/20240726235658_add_notifications_preferences_to_users.rb deleted file mode 100644 index ef2e9b02..00000000 --- a/db/migrate/20240726235658_add_notifications_preferences_to_users.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddNotificationsPreferencesToUsers < ActiveRecord::Migration[7.1] - def change - add_column :users, :notification_preferences, :jsonb, default: {} - end -end diff --git a/db/schema.rb b/db/schema.rb index 51071399..f6eb8282 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[7.1].define(version: 2024_07_26_235658) do +ActiveRecord::Schema[7.1].define(version: 2024_07_26_171428) do create_table "conferences", force: :cascade do |t| t.string "name", null: false t.datetime "created_at", null: false @@ -129,7 +129,6 @@ t.string "password_digest", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.json "notification_preferences", default: {} t.index ["email"], name: "index_users_on_email", unique: true end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index f7c0b951..7f25f9e4 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -6,7 +6,6 @@ # email :string not null # in_app_notifications_enabled :boolean default(TRUE), not null # mail_notifications_enabled :boolean default(TRUE), not null -# notification_preferences :json # password_digest :string not null # role :string # created_at :datetime not null diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2fdeaf66..db4430e7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -6,7 +6,6 @@ # email :string not null # in_app_notifications_enabled :boolean default(TRUE), not null # mail_notifications_enabled :boolean default(TRUE), not null -# notification_preferences :json # password_digest :string not null # role :string # created_at :datetime not null From 748266be9a35c4ad54a24d406b385b9b298bc9ed Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Fri, 26 Jul 2024 19:34:57 -0600 Subject: [PATCH 07/17] minor fix --- app/views/event_mailer/reminder.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/event_mailer/reminder.html.erb b/app/views/event_mailer/reminder.html.erb index be676342..a9fae578 100644 --- a/app/views/event_mailer/reminder.html.erb +++ b/app/views/event_mailer/reminder.html.erb @@ -1,2 +1,2 @@

<%= params[:notification].time_to_start_message %>

-

<%= params[:record].title %>

\ No newline at end of file +

<%= params[:record].title %>

From 7e6cbe9059329f09067b6f7b2047618b1b34cd47 Mon Sep 17 00:00:00 2001 From: Alan Soto Valle Date: Wed, 31 Jul 2024 08:52:13 -0600 Subject: [PATCH 08/17] Update app/notifiers/event_notifier.rb Co-authored-by: Sergio-e <33036058+Sergio-e@users.noreply.github.com> --- app/notifiers/event_notifier.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/notifiers/event_notifier.rb b/app/notifiers/event_notifier.rb index 52c42642..91184d5c 100644 --- a/app/notifiers/event_notifier.rb +++ b/app/notifiers/event_notifier.rb @@ -2,7 +2,7 @@ class EventNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "EventMailer" config.method = "reminder" - config.if = -> { recipient.profile.mail_notifications } + config.if = -> { recipient.profile&.mail_notifications } end notification_methods do From 0aa26e7111f8cc420ee6b07e5a298cdcf30ece60 Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Wed, 31 Jul 2024 12:35:31 -0600 Subject: [PATCH 09/17] Apply recurring_tasks approach --- app/jobs/event_reminder_job.rb | 13 ++++--- app/models/event.rb | 25 +++++++------ app/models/event/reminder_scheduler.rb | 36 ------------------- config/solid_queue.yml | 4 +++ ...25222556_add_reminder_details_to_events.rb | 5 --- db/schema.rb | 1 - spec/factories/events.rb | 19 +++++----- spec/models/event_spec.rb | 19 +++++----- 8 files changed, 45 insertions(+), 77 deletions(-) delete mode 100644 app/models/event/reminder_scheduler.rb delete mode 100644 db/migrate/20240725222556_add_reminder_details_to_events.rb diff --git a/app/jobs/event_reminder_job.rb b/app/jobs/event_reminder_job.rb index 2610bca4..239e0e82 100644 --- a/app/jobs/event_reminder_job.rb +++ b/app/jobs/event_reminder_job.rb @@ -1,8 +1,13 @@ class EventReminderJob < ApplicationJob - def perform(event_id:, reminder_type:, reminder_id:) - event = Event.find(event_id) - return if event.reminder_details["#{reminder_type}_id"] != reminder_id + def perform + now = Time.zone.now - EventNotifier.with(record: event, reminder_type: reminder_type).deliver(event.users) + Event::REMINDER_CADENCE.each do |reminder_type, cadence| + starts_at = now + cadence + + Event.where(starts_at: starts_at.beginning_of_minute..starts_at.end_of_minute).find_each do |event| + EventNotifier.with(record: event, reminder_type: reminder_type).deliver(event.users) + end + end end end diff --git a/app/models/event.rb b/app/models/event.rb index 8fe43825..4de100a8 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -2,16 +2,15 @@ # # Table name: events # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# reminder_details :json -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # @@ -19,7 +18,11 @@ # index_events_on_location_id (location_id) # class Event < ApplicationRecord - include ReminderScheduler + REMINDER_CADENCE = { + first_reminder: 15.minutes, + second_reminder: 10.minutes, + last_reminder: 0.minutes + }.freeze belongs_to :location belongs_to :conference diff --git a/app/models/event/reminder_scheduler.rb b/app/models/event/reminder_scheduler.rb deleted file mode 100644 index 398101d3..00000000 --- a/app/models/event/reminder_scheduler.rb +++ /dev/null @@ -1,36 +0,0 @@ -module Event::ReminderScheduler - extend ActiveSupport::Concern - - REMINDER_CADENCE = { - first_reminder: 15.minutes, - second_reminder: 10.minutes, - last_reminder: 0.minutes - }.freeze - - included do - after_save_commit :schedule_reminder - end - - private - - def schedule_reminder - return unless saved_change_to_starts_at? - - REMINDER_CADENCE.each do |reminder_type, cadence| - deadline = starts_at - cadence - next unless deadline > Time.zone.now - - reminder_id = SecureRandom.uuid - reminder_details["#{reminder_type}_id"] = reminder_id - save - - EventReminderJob - .set(wait_until: deadline) - .perform_later( - event_id: id, - reminder_type: reminder_type, - reminder_id: reminder_id - ) - end - end -end diff --git a/config/solid_queue.yml b/config/solid_queue.yml index 26a0d751..a7f7da63 100644 --- a/config/solid_queue.yml +++ b/config/solid_queue.yml @@ -2,6 +2,10 @@ default: &default dispatchers: - polling_interval: 1 batch_size: 500 + recurring_tasks: + event_reminder: + class: EventReminderJob + schedule: every minute workers: - queues: "*" threads: 3 diff --git a/db/migrate/20240725222556_add_reminder_details_to_events.rb b/db/migrate/20240725222556_add_reminder_details_to_events.rb deleted file mode 100644 index 78140cc3..00000000 --- a/db/migrate/20240725222556_add_reminder_details_to_events.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddReminderDetailsToEvents < ActiveRecord::Migration[7.1] - def change - add_column :events, :reminder_details, :jsonb, default: {} - end -end diff --git a/db/schema.rb b/db/schema.rb index 17041758..61b16417 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -54,7 +54,6 @@ t.integer "conference_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.json "reminder_details", default: {} t.index ["conference_id"], name: "index_events_on_conference_id" t.index ["location_id"], name: "index_events_on_location_id" end diff --git a/spec/factories/events.rb b/spec/factories/events.rb index 11f223f4..c60be2f5 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -2,16 +2,15 @@ # # Table name: events # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# reminder_details :json -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 1e6c35fa..3f088cc8 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -2,16 +2,15 @@ # # Table name: events # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# reminder_details :json -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # From 25640c1bad85911d06bb5f822eac1b93e0d35050 Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Thu, 1 Aug 2024 11:26:07 -0600 Subject: [PATCH 10/17] Specs for EventReminderjob --- Gemfile | 1 + Gemfile.lock | 2 ++ spec/jobs/event_reminder_job_spec.rb | 42 ++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 spec/jobs/event_reminder_job_spec.rb diff --git a/Gemfile b/Gemfile index 9d4cafdc..6a6faff7 100644 --- a/Gemfile +++ b/Gemfile @@ -78,4 +78,5 @@ group :test do gem "rails-controller-testing" gem "rspec-instafail", require: false gem "rspec-retry" + gem "timecop" end diff --git a/Gemfile.lock b/Gemfile.lock index ed34baf0..4d98674f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -508,6 +508,7 @@ GEM railties (>= 7.0.0) thor (1.3.1) thread_safe (0.3.6) + timecop (0.9.10) timeliness (0.4.5) timeout (0.4.1) tty-which (0.5.0) @@ -598,6 +599,7 @@ DEPENDENCIES standard stimulus-rails tailwindcss-rails (~> 2.6) + timecop turbo-rails tzinfo-data validates_timeliness (~> 7.0.0.beta1) diff --git a/spec/jobs/event_reminder_job_spec.rb b/spec/jobs/event_reminder_job_spec.rb new file mode 100644 index 00000000..783e020e --- /dev/null +++ b/spec/jobs/event_reminder_job_spec.rb @@ -0,0 +1,42 @@ +require "rails_helper" + +RSpec.describe EventReminderJob, type: :job do + let(:conference) { create(:conference) } + let(:location) { create(:location, conference: conference) } + let(:user) { create(:user) } + let(:now) { Time.zone.now } + + before do + Timecop.freeze(now) + end + + after do + Timecop.return + end + + it "delivers notifications on schedule" do + event_starting_times = ["7:00", "8:18", "9:50", "10:01", "12:59", "14:15", "15:33", "16:00", "17:30", "18:11"] + + events = event_starting_times.map do |time| + starts_at = Time.zone.parse(time) + event = create(:event, conference: conference, location: location, starts_at: starts_at, ends_at: starts_at + 30.minutes) + event.users << user + event + end + + events.each do |event| + Event::REMINDER_CADENCE.each do |reminder_type, cadence| + Timecop.travel(event.starts_at - cadence) + + expect { + described_class.perform_now + }.to change { + Noticed::Notification.joins(:event).where(noticed_events: {record: event}, recipient: user).count + }.by(1) + + nots = Noticed::Notification.joins(:event).where(noticed_events: {record: event}, recipient: user) + expect(nots.last.params[:reminder_type]).to eq(reminder_type) + end + end + end +end From 8e5293760ef224a01708fdaa9766101a3b9a35a7 Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Thu, 1 Aug 2024 11:30:23 -0600 Subject: [PATCH 11/17] Update rexml --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 4d98674f..38515098 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -392,7 +392,7 @@ GEM io-console (~> 0.5) request_store (1.7.0) rack (>= 1.4) - rexml (3.3.2) + rexml (3.3.4) strscan rouge (4.3.0) rqrcode (2.2.0) From c69edd495055d58b3c35945de4c6e8df17ea7a01 Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Fri, 2 Aug 2024 10:02:40 -0600 Subject: [PATCH 12/17] Rename notifier --- app/jobs/session_reminder_job.rb | 2 +- .../{session_notifier.rb => session_reminder_notifier.rb} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename app/notifiers/{session_notifier.rb => session_reminder_notifier.rb} (89%) diff --git a/app/jobs/session_reminder_job.rb b/app/jobs/session_reminder_job.rb index ae8aadc7..4a1bb3a5 100644 --- a/app/jobs/session_reminder_job.rb +++ b/app/jobs/session_reminder_job.rb @@ -6,7 +6,7 @@ def perform starts_at = now + cadence Session.where(starts_at: starts_at.beginning_of_minute..starts_at.end_of_minute).find_each do |session| - SessionNotifier.with(record: session, reminder_type: reminder_type).deliver(session.users) + SessionReminderNotifier.with(record: session, reminder_type: reminder_type).deliver(session.users) end end end diff --git a/app/notifiers/session_notifier.rb b/app/notifiers/session_reminder_notifier.rb similarity index 89% rename from app/notifiers/session_notifier.rb rename to app/notifiers/session_reminder_notifier.rb index 36f00231..d8fd0833 100644 --- a/app/notifiers/session_notifier.rb +++ b/app/notifiers/session_reminder_notifier.rb @@ -1,4 +1,4 @@ -class SessionNotifier < ApplicationNotifier +class SessionReminderNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "SessionMailer" config.method = "reminder" From e3f83c3eda49381adde33fec98b91d36aef09e29 Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Fri, 2 Aug 2024 12:10:25 -0600 Subject: [PATCH 13/17] Prevent delivering diplicated reminders --- app/jobs/session_reminder_job.rb | 16 ++++++++--- app/models/session.rb | 25 ++++++++--------- app/notifiers/session_reminder_notifier.rb | 8 +++--- app/views/event_mailer/reminder.html.erb | 2 -- app/views/session_mailer/reminder.html.erb | 2 ++ ...162104_add_reminder_details_to_sessions.rb | 5 ++++ db/schema.rb | 3 ++- spec/factories/sessions.rb | 19 ++++++------- spec/jobs/session_reminder_job_spec.rb | 27 ++++++++++++++++--- spec/models/session_spec.rb | 19 ++++++------- 10 files changed, 79 insertions(+), 47 deletions(-) delete mode 100644 app/views/event_mailer/reminder.html.erb create mode 100644 app/views/session_mailer/reminder.html.erb create mode 100644 db/migrate/20240802162104_add_reminder_details_to_sessions.rb diff --git a/app/jobs/session_reminder_job.rb b/app/jobs/session_reminder_job.rb index 4a1bb3a5..3d6133c4 100644 --- a/app/jobs/session_reminder_job.rb +++ b/app/jobs/session_reminder_job.rb @@ -2,11 +2,19 @@ class SessionReminderJob < ApplicationJob def perform now = Time.zone.now - Session::REMINDER_CADENCE.each do |reminder_type, cadence| - starts_at = now + cadence + Session::REMINDER_TIME_BEFORE_EVENT.each do |time_before_session| + reminder_time = now + time_before_session - Session.where(starts_at: starts_at.beginning_of_minute..starts_at.end_of_minute).find_each do |session| - SessionReminderNotifier.with(record: session, reminder_type: reminder_type).deliver(session.users) + Session.where(starts_at: reminder_time.beginning_of_minute..reminder_time.end_of_minute).find_each do |session| + next if session.reminder_details["delivered_reminder_times"]&.include?(time_before_session.inspect) + + SessionReminderNotifier + .with(record: session, time_before_session: time_before_session.inspect) + .deliver(session.users) + + session.reminder_details["delivered_reminder_times"] ||= [] + session.reminder_details["delivered_reminder_times"] << time_before_session.inspect + session.save! end end end diff --git a/app/models/session.rb b/app/models/session.rb index b608a13a..60aab567 100644 --- a/app/models/session.rb +++ b/app/models/session.rb @@ -2,15 +2,16 @@ # # Table name: sessions # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# reminder_details :json +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # @@ -18,11 +19,7 @@ # index_sessions_on_location_id (location_id) # class Session < ApplicationRecord - REMINDER_CADENCE = { - first_reminder: 15.minutes, - second_reminder: 10.minutes, - last_reminder: 0.minutes - }.freeze + REMINDER_TIME_BEFORE_EVENT = [5.minutes].freeze belongs_to :location belongs_to :conference diff --git a/app/notifiers/session_reminder_notifier.rb b/app/notifiers/session_reminder_notifier.rb index d8fd0833..d66453fb 100644 --- a/app/notifiers/session_reminder_notifier.rb +++ b/app/notifiers/session_reminder_notifier.rb @@ -6,13 +6,11 @@ class SessionReminderNotifier < ApplicationNotifier end notification_methods do - def time_to_start_message - cadence = Session::REMINDER_CADENCE.fetch(params[:reminder_type]) - - if params[:reminder_type] == :last_reminder + def title + if params[:time_before_session].match?(/^0\s/) "Starting Now" else - "Starting in #{cadence} minutes" + "Starting in about #{params[:time_before_session]}" end end end diff --git a/app/views/event_mailer/reminder.html.erb b/app/views/event_mailer/reminder.html.erb deleted file mode 100644 index a9fae578..00000000 --- a/app/views/event_mailer/reminder.html.erb +++ /dev/null @@ -1,2 +0,0 @@ -

<%= params[:notification].time_to_start_message %>

-

<%= params[:record].title %>

diff --git a/app/views/session_mailer/reminder.html.erb b/app/views/session_mailer/reminder.html.erb new file mode 100644 index 00000000..4b2c1ccc --- /dev/null +++ b/app/views/session_mailer/reminder.html.erb @@ -0,0 +1,2 @@ +

<%= params[:notification].title %>

+

<%= params[:record].title %>

diff --git a/db/migrate/20240802162104_add_reminder_details_to_sessions.rb b/db/migrate/20240802162104_add_reminder_details_to_sessions.rb new file mode 100644 index 00000000..89090a8e --- /dev/null +++ b/db/migrate/20240802162104_add_reminder_details_to_sessions.rb @@ -0,0 +1,5 @@ +class AddReminderDetailsToSessions < ActiveRecord::Migration[7.1] + def change + add_column :sessions, :reminder_details, :jsonb, default: {} + end +end diff --git a/db/schema.rb b/db/schema.rb index 194e3578..f8a526fa 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[7.1].define(version: 2024_07_26_171428) do +ActiveRecord::Schema[7.1].define(version: 2024_08_02_162104) do create_table "active_storage_attachments", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -106,6 +106,7 @@ t.integer "conference_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.json "reminder_details", default: {} t.index ["conference_id"], name: "index_sessions_on_conference_id" t.index ["location_id"], name: "index_sessions_on_location_id" end diff --git a/spec/factories/sessions.rb b/spec/factories/sessions.rb index 4b386136..330df464 100644 --- a/spec/factories/sessions.rb +++ b/spec/factories/sessions.rb @@ -2,15 +2,16 @@ # # Table name: sessions # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# reminder_details :json +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # diff --git a/spec/jobs/session_reminder_job_spec.rb b/spec/jobs/session_reminder_job_spec.rb index 9b001134..912c00f1 100644 --- a/spec/jobs/session_reminder_job_spec.rb +++ b/spec/jobs/session_reminder_job_spec.rb @@ -25,8 +25,8 @@ end sessions.each do |session| - Session::REMINDER_CADENCE.each do |reminder_type, cadence| - Timecop.travel(session.starts_at - cadence) + Session::REMINDER_TIME_BEFORE_EVENT.each do |time_before_session| + Timecop.travel(session.starts_at - time_before_session) expect { described_class.perform_now @@ -35,8 +35,29 @@ }.by(1) nots = Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user) - expect(nots.last.params[:reminder_type]).to eq(reminder_type) + expect(nots.last.params[:time_before_session]).to eq(time_before_session.inspect) end end end + + it "prevents multiple deliveries of the same notification" do + session = create(:session, conference: conference, location: location, starts_at: now + 1.hour, ends_at: now + 1.hour + 30.minutes) + session.users << user + + Session::REMINDER_TIME_BEFORE_EVENT.each do |time_before_session| + Timecop.travel(session.starts_at - time_before_session) + + expect { + described_class.perform_now + }.to change { + Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count + }.by(1) + + expect { + described_class.perform_now + }.not_to change { + Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count + } + end + end end diff --git a/spec/models/session_spec.rb b/spec/models/session_spec.rb index a58b2bea..32076fa0 100644 --- a/spec/models/session_spec.rb +++ b/spec/models/session_spec.rb @@ -2,15 +2,16 @@ # # Table name: sessions # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# reminder_details :json +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # From a82c08ff67984966d31959ecc05198cd5d74710b Mon Sep 17 00:00:00 2001 From: Alan Soto Date: Fri, 2 Aug 2024 19:27:49 -0600 Subject: [PATCH 14/17] Delivers lost reminder in next job iteration --- app/jobs/session_reminder_job.rb | 8 ++++--- app/models/user.rb | 4 ++++ spec/jobs/session_reminder_job_spec.rb | 30 ++++++++++++++++++++++++-- 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/app/jobs/session_reminder_job.rb b/app/jobs/session_reminder_job.rb index 3d6133c4..8979fe5f 100644 --- a/app/jobs/session_reminder_job.rb +++ b/app/jobs/session_reminder_job.rb @@ -1,16 +1,18 @@ class SessionReminderJob < ApplicationJob def perform now = Time.zone.now - Session::REMINDER_TIME_BEFORE_EVENT.each do |time_before_session| reminder_time = now + time_before_session + two_iterations_ago = now - 2.minutes - Session.where(starts_at: reminder_time.beginning_of_minute..reminder_time.end_of_minute).find_each do |session| + Session + .where(starts_at: two_iterations_ago.beginning_of_minute..reminder_time.end_of_minute) + .find_each do |session| next if session.reminder_details["delivered_reminder_times"]&.include?(time_before_session.inspect) SessionReminderNotifier .with(record: session, time_before_session: time_before_session.inspect) - .deliver(session.users) + .deliver(session.users.with_at_least_one_notification_enabled) session.reminder_details["delivered_reminder_times"] ||= [] session.reminder_details["delivered_reminder_times"] << time_before_session.inspect diff --git a/app/models/user.rb b/app/models/user.rb index 165fa861..8f526e32 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -36,6 +36,10 @@ class User < ApplicationRecord accepts_nested_attributes_for :profile + scope :with_at_least_one_notification_enabled, -> { + joins(:profile).where("profiles.in_app_notifications = ? OR profiles.mail_notifications = ?", true, true) + } + generates_token_for :password_reset, expires_in: PASSWORD_RESET_EXPIRATION do password_salt&.last(10) end diff --git a/spec/jobs/session_reminder_job_spec.rb b/spec/jobs/session_reminder_job_spec.rb index 912c00f1..e99347c5 100644 --- a/spec/jobs/session_reminder_job_spec.rb +++ b/spec/jobs/session_reminder_job_spec.rb @@ -19,7 +19,8 @@ sessions = session_starting_times.map do |time| starts_at = Time.zone.parse(time) - session = create(:session, conference: conference, location: location, starts_at: starts_at, ends_at: starts_at + 30.minutes) + session = create(:session, conference: conference, location: location, + starts_at: starts_at, ends_at: starts_at + 30.minutes) session.users << user session end @@ -41,7 +42,8 @@ end it "prevents multiple deliveries of the same notification" do - session = create(:session, conference: conference, location: location, starts_at: now + 1.hour, ends_at: now + 1.hour + 30.minutes) + session = create(:session, conference: conference, location: location, + starts_at: now + 1.hour, ends_at: now + 1.hour + 30.minutes) session.users << user Session::REMINDER_TIME_BEFORE_EVENT.each do |time_before_session| @@ -60,4 +62,28 @@ } end end + + it "delivers lost reminders in next iteration" do + stub_const("Session::REMINDER_TIME_BEFORE_EVENT", [5.minutes]) + + sessions = (0..5).map do |index| + starts_at = now + 5.minutes + index.minutes + + session = create(:session, conference: conference, location: location, + title: "Session #{index}", starts_at: starts_at, ends_at: starts_at + 30.minutes) + session.users << user + session + end + + (0..6).each do |index| + Timecop.travel(now + index.minutes) + next if index == 2 || index == 4 + + described_class.perform_now + end + + sessions.each do |session| + expect(session.reload.reminder_details["delivered_reminder_times"]).to include("5 minutes") + end + end end From dca0b7f116b4bb7e7e9e3b641dd63209ed195b66 Mon Sep 17 00:00:00 2001 From: Sergio-e <33036058+Sergio-e@users.noreply.github.com> Date: Wed, 7 Aug 2024 11:11:55 -0600 Subject: [PATCH 15/17] Session reminder improvements --- app/avo/resources/session.rb | 8 +- app/jobs/session_reminder_job.rb | 35 +++-- app/models/feature.rb | 9 ++ app/models/session.rb | 24 ++- app/views/main/index.html.erb | 2 + db/migrate/20240628204535_create_sessions.rb | 1 + ...162104_add_reminder_details_to_sessions.rb | 5 - db/schema.rb | 4 +- spec/factories/sessions.rb | 20 +-- spec/jobs/session_reminder_job_spec.rb | 140 ++++++++++-------- spec/models/session_spec.rb | 20 +-- 11 files changed, 154 insertions(+), 114 deletions(-) create mode 100644 app/models/feature.rb delete mode 100644 db/migrate/20240802162104_add_reminder_details_to_sessions.rb diff --git a/app/avo/resources/session.rb b/app/avo/resources/session.rb index a146fb02..48ba4654 100644 --- a/app/avo/resources/session.rb +++ b/app/avo/resources/session.rb @@ -13,8 +13,12 @@ def fields field :id, as: :id field :title, as: :text, sortable: true field :description, as: :textarea - field :starts_at, as: :date_time, help: Time.zone.name, sortable: true - field :ends_at, as: :date_time, help: Time.zone.name, sortable: true + field :starts_at, as: :date_time, + help: "The datetime field will use your browser's current timezone.", sortable: true, + format: "FFFF" + field :ends_at, as: :date_time, + help: "The datetime field will use your browser's current timezone.", sortable: true, + format: "FFFF" field :location, as: :belongs_to field :conference, as: :belongs_to field :speakers, as: :has_and_belongs_to_many, can_create: false diff --git a/app/jobs/session_reminder_job.rb b/app/jobs/session_reminder_job.rb index 8979fe5f..5db29d85 100644 --- a/app/jobs/session_reminder_job.rb +++ b/app/jobs/session_reminder_job.rb @@ -1,22 +1,33 @@ class SessionReminderJob < ApplicationJob + REMINDER_TIME_BEFORE_EVENT = [5.minutes].freeze + def perform + if Feature.disabled?(:session_reminders) + return Rails.logger.info("Skipping session reminders. Feature is disabled") + end + now = Time.zone.now - Session::REMINDER_TIME_BEFORE_EVENT.each do |time_before_session| - reminder_time = now + time_before_session - two_iterations_ago = now - 2.minutes + Rails.logger.info "Searching for sessions to remind users about. Time is #{now}" - Session - .where(starts_at: two_iterations_ago.beginning_of_minute..reminder_time.end_of_minute) - .find_each do |session| - next if session.reminder_details["delivered_reminder_times"]&.include?(time_before_session.inspect) + REMINDER_TIME_BEFORE_EVENT.each do |time_before_session| + # Grace time is the time window in which we send reminders that should have been sent already. + grace_time = 2.minutes - SessionReminderNotifier - .with(record: session, time_before_session: time_before_session.inspect) - .deliver(session.users.with_at_least_one_notification_enabled) + start_reminder_time = (now + time_before_session - grace_time).end_of_minute + end_reminder_time = (start_reminder_time + grace_time).beginning_of_minute - session.reminder_details["delivered_reminder_times"] ||= [] - session.reminder_details["delivered_reminder_times"] << time_before_session.inspect + Rails.logger.info "Searching for sessions with starts_at between #{start_reminder_time} and #{end_reminder_time}" + + Session.where(starts_at: start_reminder_time..end_reminder_time).find_each do |session| + next if session.sent_reminders.include?(time_before_session.inspect) + + session.sent_reminders << time_before_session.inspect + session.sent_reminders.uniq! session.save! + + SessionReminderNotifier + .with(record: session, time_before_session: time_before_session.inspect) + .deliver(session.attendees.with_at_least_one_notification_enabled) end end end diff --git a/app/models/feature.rb b/app/models/feature.rb new file mode 100644 index 00000000..ba7cae99 --- /dev/null +++ b/app/models/feature.rb @@ -0,0 +1,9 @@ +class Feature + def self.enabled?(feature) + ENV["#{feature.to_s.upcase}_ENABLED"] == "true" + end + + def self.disabled?(feature) + !enabled?(feature) + end +end diff --git a/app/models/session.rb b/app/models/session.rb index 60aab567..b97db248 100644 --- a/app/models/session.rb +++ b/app/models/session.rb @@ -2,16 +2,16 @@ # # Table name: sessions # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# reminder_details :json -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# sent_reminders :json +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # @@ -19,13 +19,11 @@ # index_sessions_on_location_id (location_id) # class Session < ApplicationRecord - REMINDER_TIME_BEFORE_EVENT = [5.minutes].freeze - belongs_to :location belongs_to :conference has_and_belongs_to_many :speakers - has_and_belongs_to_many :users # attendees + has_and_belongs_to_many :attendees, class_name: "User", join_table: "sessions_users" has_and_belongs_to_many :tags validates :title, presence: true diff --git a/app/views/main/index.html.erb b/app/views/main/index.html.erb index b2576511..49526b72 100644 --- a/app/views/main/index.html.erb +++ b/app/views/main/index.html.erb @@ -1,2 +1,4 @@

Homepage

<%= link_to "Profile", profile_path(current_profile&.uuid) %> + +

Current time is <%= Time.zone.now %>

diff --git a/db/migrate/20240628204535_create_sessions.rb b/db/migrate/20240628204535_create_sessions.rb index 98688eee..80911da9 100644 --- a/db/migrate/20240628204535_create_sessions.rb +++ b/db/migrate/20240628204535_create_sessions.rb @@ -5,6 +5,7 @@ def change t.string :description t.datetime :starts_at, null: false t.datetime :ends_at, null: false + t.json :sent_reminders, default: [] t.references :location, null: false, foreign_key: true t.references :conference, null: false, foreign_key: true diff --git a/db/migrate/20240802162104_add_reminder_details_to_sessions.rb b/db/migrate/20240802162104_add_reminder_details_to_sessions.rb deleted file mode 100644 index 89090a8e..00000000 --- a/db/migrate/20240802162104_add_reminder_details_to_sessions.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddReminderDetailsToSessions < ActiveRecord::Migration[7.1] - def change - add_column :sessions, :reminder_details, :jsonb, default: {} - end -end diff --git a/db/schema.rb b/db/schema.rb index f8a526fa..813ed035 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[7.1].define(version: 2024_08_02_162104) do +ActiveRecord::Schema[7.1].define(version: 2024_07_26_171428) do create_table "active_storage_attachments", force: :cascade do |t| t.string "name", null: false t.string "record_type", null: false @@ -102,11 +102,11 @@ t.string "description" t.datetime "starts_at", null: false t.datetime "ends_at", null: false + t.json "sent_reminders", default: [] t.integer "location_id", null: false t.integer "conference_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.json "reminder_details", default: {} t.index ["conference_id"], name: "index_sessions_on_conference_id" t.index ["location_id"], name: "index_sessions_on_location_id" end diff --git a/spec/factories/sessions.rb b/spec/factories/sessions.rb index 330df464..561d5903 100644 --- a/spec/factories/sessions.rb +++ b/spec/factories/sessions.rb @@ -2,16 +2,16 @@ # # Table name: sessions # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# reminder_details :json -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# sent_reminders :json +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # diff --git a/spec/jobs/session_reminder_job_spec.rb b/spec/jobs/session_reminder_job_spec.rb index e99347c5..308eb6a7 100644 --- a/spec/jobs/session_reminder_job_spec.rb +++ b/spec/jobs/session_reminder_job_spec.rb @@ -7,83 +7,103 @@ let(:now) { Time.zone.now } before do + ENV["SESSION_REMINDERS_ENABLED"] = "true" Timecop.freeze(now) end after do + ENV["SESSION_REMINDERS_ENABLED"] = "false" Timecop.return end - it "delivers notifications on schedule" do - session_starting_times = ["7:00", "8:18", "9:50", "10:01", "12:59", "14:15", "15:33", "16:00", "17:30", "18:11"] - - sessions = session_starting_times.map do |time| - starts_at = Time.zone.parse(time) - session = create(:session, conference: conference, location: location, - starts_at: starts_at, ends_at: starts_at + 30.minutes) - session.users << user - session - end - - sessions.each do |session| - Session::REMINDER_TIME_BEFORE_EVENT.each do |time_before_session| - Timecop.travel(session.starts_at - time_before_session) - - expect { - described_class.perform_now - }.to change { - Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count - }.by(1) - - nots = Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user) - expect(nots.last.params[:time_before_session]).to eq(time_before_session.inspect) - end - end + it "doesn't deliver the notification if the reminder window is yet to pass" do + starts_at = Time.zone.parse("7:00") + session = create(:session, conference: conference, location: location, + starts_at: starts_at, ends_at: starts_at + 30.minutes) + session.attendees << user + + Timecop.travel(Time.zone.parse("6:50")) + + expect { + described_class.perform_now + }.not_to change { + Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count + } + + expect(session.sent_reminders).to eq([]) end - it "prevents multiple deliveries of the same notification" do + it "doesn't deliver the notification if the reminder window has already passed" do + starts_at = Time.zone.parse("7:00") session = create(:session, conference: conference, location: location, - starts_at: now + 1.hour, ends_at: now + 1.hour + 30.minutes) - session.users << user - - Session::REMINDER_TIME_BEFORE_EVENT.each do |time_before_session| - Timecop.travel(session.starts_at - time_before_session) - - expect { - described_class.perform_now - }.to change { - Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count - }.by(1) - - expect { - described_class.perform_now - }.not_to change { - Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count - } - end + starts_at: starts_at, ends_at: starts_at + 30.minutes) + session.attendees << user + + Timecop.travel(Time.zone.parse("6:59")) + + expect { + described_class.perform_now + }.not_to change { + Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count + } + + expect(session.sent_reminders).to eq([]) end - it "delivers lost reminders in next iteration" do - stub_const("Session::REMINDER_TIME_BEFORE_EVENT", [5.minutes]) + it "delivers the notification if the reminder window is active" do + starts_at = Time.zone.parse("7:00") + session = create(:session, conference: conference, location: location, + starts_at: starts_at, ends_at: starts_at + 30.minutes) + session.attendees << user - sessions = (0..5).map do |index| - starts_at = now + 5.minutes + index.minutes + Timecop.travel(Time.zone.parse("6:55")) - session = create(:session, conference: conference, location: location, - title: "Session #{index}", starts_at: starts_at, ends_at: starts_at + 30.minutes) - session.users << user - session - end + expect { + described_class.perform_now + }.to change { + Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count + }.by(1) - (0..6).each do |index| - Timecop.travel(now + index.minutes) - next if index == 2 || index == 4 + session.reload + expect(session.sent_reminders).to eq(["5 minutes"]) + end + it "delivers the notification if the reminder window is active (grace period)" do + starts_at = Time.zone.parse("7:00") + session = create(:session, conference: conference, location: location, + starts_at: starts_at, ends_at: starts_at + 30.minutes) + session.attendees << user + + Timecop.travel(Time.zone.parse("6:56")) + + expect { + described_class.perform_now + }.to change { + Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count + }.by(1) + + session.reload + expect(session.sent_reminders).to eq(["5 minutes"]) + end + + it "prevents multiple deliveries of the same notification" do + starts_at = Time.zone.parse("7:00") + session = create(:session, conference: conference, location: location, + starts_at: starts_at, ends_at: starts_at + 30.minutes) + session.attendees << user + + Timecop.travel(Time.zone.parse("6:55")) + + expect { described_class.perform_now - end + }.to change { + Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count + }.by(1) - sessions.each do |session| - expect(session.reload.reminder_details["delivered_reminder_times"]).to include("5 minutes") - end + expect { + described_class.perform_now + }.not_to change { + Noticed::Notification.joins(:event).where(noticed_events: {record: session}, recipient: user).count + } end end diff --git a/spec/models/session_spec.rb b/spec/models/session_spec.rb index 32076fa0..fc0bb434 100644 --- a/spec/models/session_spec.rb +++ b/spec/models/session_spec.rb @@ -2,16 +2,16 @@ # # Table name: sessions # -# id :integer not null, primary key -# description :string -# ends_at :datetime not null -# reminder_details :json -# starts_at :datetime not null -# title :string not null -# created_at :datetime not null -# updated_at :datetime not null -# conference_id :integer not null -# location_id :integer not null +# id :integer not null, primary key +# description :string +# ends_at :datetime not null +# sent_reminders :json +# starts_at :datetime not null +# title :string not null +# created_at :datetime not null +# updated_at :datetime not null +# conference_id :integer not null +# location_id :integer not null # # Indexes # From 76c18a05b017b60561930af5a3ee3fb58927ca9f Mon Sep 17 00:00:00 2001 From: Sergio-e <33036058+Sergio-e@users.noreply.github.com> Date: Wed, 7 Aug 2024 11:53:24 -0600 Subject: [PATCH 16/17] Add sent_reminders to AVO session --- app/avo/resources/session.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/avo/resources/session.rb b/app/avo/resources/session.rb index 48ba4654..5f395ef3 100644 --- a/app/avo/resources/session.rb +++ b/app/avo/resources/session.rb @@ -19,6 +19,7 @@ def fields field :ends_at, as: :date_time, help: "The datetime field will use your browser's current timezone.", sortable: true, format: "FFFF" + field :sent_reminders, only_on: :show field :location, as: :belongs_to field :conference, as: :belongs_to field :speakers, as: :has_and_belongs_to_many, can_create: false From 534b3b96b2fbdbf1c33794cbe0be78da6b58366a Mon Sep 17 00:00:00 2001 From: Sergio-e <33036058+Sergio-e@users.noreply.github.com> Date: Wed, 7 Aug 2024 11:55:35 -0600 Subject: [PATCH 17/17] Add feature flag readme --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 1bdd7018..92bb707a 100644 --- a/README.md +++ b/README.md @@ -82,3 +82,7 @@ Run tests by using `bundle exec rspec`. - If you want to see the logs you can use `:log`, e.g. `it "xxx", :log do` - Use `data-test-id` to find elements instead of classes/ids, e.g. `data-test-id="decline_modal"` - Use the methods in the `DataTestId` module to select HTML elements, e.g., `find_dti("decline_modal")` + +## Feature Flags + +Use ENV variables to enable features, the name should follow the convention `"#{feature_name}_ENABLED"`. For example, to enable the `payment` feature, use `ENV["PAYMENT_ENABLED"]="true"`.