From b911a12833ca4f7cbd2450f1aa3f7c2b6b1cbd29 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 26 Aug 2025 10:15:27 +0200 Subject: [PATCH 01/54] Configure action mailer --- README.md | 2 +- config/environments/beta.rb | 6 +++++- config/environments/development.rb | 5 +++++ config/environments/production.rb | 16 ++++++++++------ config/environments/staging.rb | 6 +++++- 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index c7625286c..58a1a9682 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ Beta is primarily intended for testing product features. Beta tenant is: -- https://fizzy.37signals.works/ +- https://fizzy-beta.37signals.com This environment uses local disk for Active Storage. diff --git a/config/environments/beta.rb b/config/environments/beta.rb index c7f44040b..c67135679 100644 --- a/config/environments/beta.rb +++ b/config/environments/beta.rb @@ -1,7 +1,11 @@ require_relative "production" Rails.application.configure do - config.action_mailer.default_url_options = { host: "%{tenant}.37signals.works" } + config.hosts = [ "fizzy-beta.37signals.com" ] + config.hosts[1..] + + config.action_mailer.smtp_settings[:domain] = config.hosts.first + config.action_mailer.smtp_settings[:address] = "smtp-outbound-staging" + config.action_mailer.default_url_options = { host: config.hosts.first, protocol: "https" } # Let's keep beta on local disk. See https://github.com/basecamp/fizzy/pull/557 for context. config.active_storage.service = :local diff --git a/config/environments/development.rb b/config/environments/development.rb index 24fc6c04c..002ee99ed 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -75,6 +75,11 @@ Rails.application.configure do # Allow all hosts in development config.hosts = nil + config.hosts = %w[ fizzy.localhost localhost 127.0.0.1 ] + + # Host to be used in links rendered from ActionMailer + config.action_mailer.default_url_options = { host: config.hosts.first, port: 3005 } + if Rails.root.join("tmp/solid-queue.txt").exist? config.active_job.queue_adapter = :solid_queue config.solid_queue.connects_to = { database: { writing: :queue } } diff --git a/config/environments/production.rb b/config/environments/production.rb index 73f7f6f1b..664c60834 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -73,8 +73,17 @@ Rails.application.configure do # Set this to true and configure the email server for immediate delivery to raise delivery errors. # config.action_mailer.raise_delivery_errors = false + # Enable DNS rebinding protection and other `Host` header attacks. + config.hosts = [ + "fizzy.37signals.com", + "localhost", + IPAddr.new("172.17.0.0/20") # Docker VPC + ] + # Set host to be used by links generated in mailer templates. - config.action_mailer.default_url_options = { host: "%{tenant}.fizzy.37signals.com" } + config.action_mailer.default_url_options = { host: config.hosts.first, protocol: "https" } + + config.action_mailer.smtp_settings = { domain: config.hosts.first, address: "smtp-outbound", port: 25, enable_starttls_auto: false } # Enable locale fallbacks for I18n (makes lookups for any locale fall back to # the I18n.default_locale when a translation cannot be found). @@ -86,11 +95,6 @@ Rails.application.configure do # Do not dump schema after migrations. config.active_record.dump_schema_after_migration = false - # Enable DNS rebinding protection and other `Host` header attacks. - # config.hosts = [ - # "example.com", # Allow requests from example.com - # /.*\.example\.com/ # Allow requests from subdomains like `www.example.com` - # ] # Skip DNS rebinding protection for the default health check endpoint. # config.host_authorization = { exclude: ->(request) { request.path == "/up" } } end diff --git a/config/environments/staging.rb b/config/environments/staging.rb index 27868fd80..7bee9c845 100644 --- a/config/environments/staging.rb +++ b/config/environments/staging.rb @@ -1,5 +1,9 @@ require_relative "production" Rails.application.configure do - config.action_mailer.default_url_options = { host: "%{tenant}.fizzy.37signals-staging.com" } + config.hosts = [ "fizzy.37signals-staging.com" ] + config.hosts[1..] + + config.action_mailer.smtp_settings[:domain] = config.hosts.first + config.action_mailer.smtp_settings[:address] = "smtp-outbound-staging" + config.action_mailer.default_url_options = { host: config.hosts.first, protocol: "https" } end From 218519d0750e1718bd55d0d1517295b6ee7be483 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 26 Aug 2025 10:21:13 +0200 Subject: [PATCH 02/54] Configure letter opener --- Gemfile | 1 + Gemfile.lock | 9 +++++++++ README.md | 13 +++++++++++++ config/environments/development.rb | 8 ++++++-- lib/tasks/.keep | 0 lib/tasks/dev.rake | 16 ++++++++++++++++ 6 files changed, 45 insertions(+), 2 deletions(-) delete mode 100644 lib/tasks/.keep create mode 100644 lib/tasks/dev.rake diff --git a/Gemfile b/Gemfile index 933cb4e61..2518d7495 100644 --- a/Gemfile +++ b/Gemfile @@ -60,6 +60,7 @@ group :development, :test do gem "bundler-audit", require: false gem "brakeman", require: false gem "rubocop-rails-omakase", require: false + gem "letter_opener" end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index dbb76fd31..d3be19716 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -276,6 +276,8 @@ GEM rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) + childprocess (5.1.0) + logger (~> 1.5) chunky_png (1.4.0) concurrent-ruby (1.3.5) connection_pool (2.5.3) @@ -342,6 +344,12 @@ GEM jwt (3.1.2) base64 language_server-protocol (3.17.0.5) + launchy (3.1.1) + addressable (~> 2.8) + childprocess (~> 5.0) + logger (~> 1.6) + letter_opener (1.10.0) + launchy (>= 2.2, < 4) lint_roller (1.1.0) logger (1.7.0) loofah (2.24.1) @@ -611,6 +619,7 @@ DEPENDENCIES importmap-rails jbuilder kamal! + letter_opener lexxy! mission_control-jobs mocha diff --git a/README.md b/README.md index 58a1a9682..9fc392287 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,19 @@ A typical scenario is making modifications LLM prompts. You need to: Notice that if you pass changing data to the prompt, this mechanism won't work. E.g: if you pass data timestamps. You need to make sure those timestamps are always the same across executions. +### Outbound Emails + +#### Development + +You can view email previews at http://know-it-all.localhost:3005/rails/mailers. + +You can enable or disable [`letter_opener`](https://github.com/ryanb/letter_opener) to +open sent emails automatically with: + + bin/rails dev:email + +Under the hood, this will create or remove `tmp/email-dev.txt`. + ## Environments Fizzy is deployed with Kamal. You'll need to have the 1Password CLI set up in order to access the secrets that are used when deploying. Provided you have that, it should be as simple as `bin/kamal deploy` to the correct environment. diff --git a/config/environments/development.rb b/config/environments/development.rb index 002ee99ed..0703601ec 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -72,8 +72,12 @@ Rails.application.configure do # Raise error when a before_action's only/except options reference missing actions config.action_controller.raise_on_missing_callback_actions = true - # Allow all hosts in development - config.hosts = nil + if Rails.root.join("tmp/email-dev.txt").exist? + config.action_mailer.delivery_method = :letter_opener + config.action_mailer.perform_deliveries = true + else + config.action_mailer.raise_delivery_errors = false + end config.hosts = %w[ fizzy.localhost localhost 127.0.0.1 ] diff --git a/lib/tasks/.keep b/lib/tasks/.keep deleted file mode 100644 index e69de29bb..000000000 diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake new file mode 100644 index 000000000..07dfef697 --- /dev/null +++ b/lib/tasks/dev.rake @@ -0,0 +1,16 @@ +namespace :dev do + desc "Toggle using Letter Opener to preview emails" + task :email do + file_path = Rails.root.join("tmp", "email-dev.txt") + + if File.exist?(file_path) + File.delete(file_path) + puts "Letter Opener turned off" + else + FileUtils.touch(file_path) + puts "Letter Opener turned on" + end + + %x(bin/rails restart) + end +end From cfafc54a814a3bf476e6325d1b2e395ed83cdf7e Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 26 Aug 2025 13:22:22 +0200 Subject: [PATCH 03/54] Deliver emails for bundled notifications (initial WIP) --- README.md | 2 +- .../notification/bundle/deliver_all_job.rb | 5 + app/jobs/notification/bundle/deliver_job.rb | 5 + app/mailers/notification/bundle_mailer.rb | 11 +++ app/models/notification.rb | 5 + app/models/notification/bundle.rb | 51 +++++++++++ app/models/user.rb | 4 +- app/models/user/notifiable.rb | 19 ++++ .../bundle_mailer/notification.html.erb | 9 ++ .../bundle_mailer/notification.text.erb | 1 + config/recurring.yml | 3 + ...50826084559_create_notification_bundles.rb | 15 +++ db/schema.rb | 15 ++- db/schema_cache.yml | 91 ++++++++++++++++++- .../notification/bundle_mailer_preview.rb | 6 ++ test/models/notification/bundle_test.rb | 35 +++++++ 16 files changed, 271 insertions(+), 6 deletions(-) create mode 100644 app/jobs/notification/bundle/deliver_all_job.rb create mode 100644 app/jobs/notification/bundle/deliver_job.rb create mode 100644 app/mailers/notification/bundle_mailer.rb create mode 100644 app/models/notification/bundle.rb create mode 100644 app/models/user/notifiable.rb create mode 100644 app/views/notification/bundle_mailer/notification.html.erb create mode 100644 app/views/notification/bundle_mailer/notification.text.erb create mode 100644 db/migrate/20250826084559_create_notification_bundles.rb create mode 100644 test/mailers/previews/notification/bundle_mailer_preview.rb create mode 100644 test/models/notification/bundle_test.rb diff --git a/README.md b/README.md index 9fc392287..dba2f4c3d 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ You need to make sure those timestamps are always the same across executions. #### Development -You can view email previews at http://know-it-all.localhost:3005/rails/mailers. +You can view email previews at http://fizzy.localhost:3006/rails/mailers. You can enable or disable [`letter_opener`](https://github.com/ryanb/letter_opener) to open sent emails automatically with: diff --git a/app/jobs/notification/bundle/deliver_all_job.rb b/app/jobs/notification/bundle/deliver_all_job.rb new file mode 100644 index 000000000..cdd6093ce --- /dev/null +++ b/app/jobs/notification/bundle/deliver_all_job.rb @@ -0,0 +1,5 @@ +class Notification::Bundle::DeliverAllJob < ApplicationJob + def perform + Notification::Bundle.deliver_all + end +end diff --git a/app/jobs/notification/bundle/deliver_job.rb b/app/jobs/notification/bundle/deliver_job.rb new file mode 100644 index 000000000..b4c64a6c2 --- /dev/null +++ b/app/jobs/notification/bundle/deliver_job.rb @@ -0,0 +1,5 @@ +class Notification::Bundle::DeliverJob < ApplicationJob + def perform(bundle) + bundle.deliver + end +end diff --git a/app/mailers/notification/bundle_mailer.rb b/app/mailers/notification/bundle_mailer.rb new file mode 100644 index 000000000..a3e830b16 --- /dev/null +++ b/app/mailers/notification/bundle_mailer.rb @@ -0,0 +1,11 @@ +class Notification::BundleMailer < ApplicationMailer + def notification(bundle) + @bundle = bundle + @notifications = bundle.notifications + + mail( + to: bundle.user.email_address, + subject: "You have #{@notifications.count} notifications" + ) + end +end \ No newline at end of file diff --git a/app/models/notification.rb b/app/models/notification.rb index 943cafd7e..0b1e2e377 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -10,6 +10,7 @@ class Notification < ApplicationRecord scope :ordered, -> { order(read_at: :desc, created_at: :desc) } after_create_commit :broadcast_unread + after_create :bundle delegate :notifiable_target, to: :source delegate :card, to: :source @@ -40,4 +41,8 @@ class Notification < ApplicationRecord def broadcast_read broadcast_remove_to user, :notifications end + + def bundle + user.bundle(self) + end end diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb new file mode 100644 index 000000000..31b16ac7e --- /dev/null +++ b/app/models/notification/bundle.rb @@ -0,0 +1,51 @@ +class Notification::Bundle < ApplicationRecord + belongs_to :user + + enum :status, %i[ pending processing delivered ] + + before_create :set_default_window + + scope :due, -> { pending.where("ends_at <= ?", Time.current) } + + class << self + def deliver_all + due.in_batches do |batch| + DeliverJob.perform_all_later batch + end + end + + def deliver_all_later + DeliverAllJob.perform_later + end + end + + def add(notification) + update! ends_at: [ ends_at, notification.created_at ].max + end + + def notifications + user.notifications.where(created_at: window) + end + + def deliver + processing! + BundleMailer.notification(self).deliver + delivered! + end + + def deliver_later + DeliverJob.perform_later(self) + end + + private + AGGREGATION_PERIOD = 4.hours + + def set_default_window + self.starts_at ||= Time.current + self.ends_at ||= AGGREGATION_PERIOD.from_now + end + + def window + starts_at..ends_at + end +end diff --git a/app/models/user.rb b/app/models/user.rb index bd8cf990d..21007d711 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,5 @@ class User < ApplicationRecord - include Accessor, Attachable, Assignee, Mentionable, Named, Role, Searcher, + include Accessor, Attachable, Assignee, Mentionable, Named, Notifiable, Role, Searcher, SignalUser, Staff, Transferable, Conversational include Timelined # Depends on Accessor @@ -10,8 +10,6 @@ class User < ApplicationRecord has_many :comments, inverse_of: :creator, dependent: :destroy - has_many :notifications, dependent: :destroy - has_many :filters, foreign_key: :creator_id, inverse_of: :creator, dependent: :destroy has_many :closures, dependent: :nullify has_many :pins, dependent: :destroy diff --git a/app/models/user/notifiable.rb b/app/models/user/notifiable.rb new file mode 100644 index 000000000..0f6cf3b94 --- /dev/null +++ b/app/models/user/notifiable.rb @@ -0,0 +1,19 @@ +module User::Notifiable + extend ActiveSupport::Concern + + included do + has_many :notifications, dependent: :destroy + has_many :notification_bundles, class_name: "Notification::Bundle", dependent: :destroy + end + + def bundle(notification) + transaction do + find_or_create_pending_bundle_for(notification).add(notification) + end + end + + private + def find_or_create_pending_bundle_for(notification) + notification_bundles.pending.last || notification_bundles.create!(starts_at: notification.created_at) + end +end diff --git a/app/views/notification/bundle_mailer/notification.html.erb b/app/views/notification/bundle_mailer/notification.html.erb new file mode 100644 index 000000000..1fd9371f0 --- /dev/null +++ b/app/views/notification/bundle_mailer/notification.html.erb @@ -0,0 +1,9 @@ +

Your Notifications

+ +

You have <%= @notifications.count %> notifications:

+ +
    + <% @notifications.each do |notification| %> +
  • Notification ID: <%= notification.id %>
  • + <% end %> +
\ No newline at end of file diff --git a/app/views/notification/bundle_mailer/notification.text.erb b/app/views/notification/bundle_mailer/notification.text.erb new file mode 100644 index 000000000..8b7c8f424 --- /dev/null +++ b/app/views/notification/bundle_mailer/notification.text.erb @@ -0,0 +1 @@ +PENDING... diff --git a/config/recurring.yml b/config/recurring.yml index 99b3c1f33..66188ec7a 100644 --- a/config/recurring.yml +++ b/config/recurring.yml @@ -17,6 +17,9 @@ production: &production clear_solid_queue_finished_jobs: command: "SolidQueue::Job.clear_finished_in_batches(sleep_between_batches: 0.3)" schedule: every hour at minute 12 + deliver_bundled_notifications: + command: "Notification::Bundle.deliver_all_later" + schedule: every 30 minutes beta: *production staging: *production diff --git a/db/migrate/20250826084559_create_notification_bundles.rb b/db/migrate/20250826084559_create_notification_bundles.rb new file mode 100644 index 000000000..5c3f3ebfb --- /dev/null +++ b/db/migrate/20250826084559_create_notification_bundles.rb @@ -0,0 +1,15 @@ +class CreateNotificationBundles < ActiveRecord::Migration[8.1] + def change + create_table :notification_bundles do |t| + t.references :user, null: false, foreign_key: true, index: { unique: true } + t.datetime :starts_at, null: false + t.datetime :ends_at, null: false + t.integer :status, default: 0, null: false + + t.timestamps + end + + add_index :notification_bundles, %i[ user_id starts_at ends_at ] + add_index :notification_bundles, %i[ user_id status ] + end +end diff --git a/db/schema.rb b/db/schema.rb index b31a7dc51..c27e4b231 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_08_19_105245) do +ActiveRecord::Schema[8.1].define(version: 2025_08_26_084559) do create_table "accesses", force: :cascade do |t| t.datetime "accessed_at" t.integer "collection_id", null: false @@ -295,6 +295,18 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_19_105245) do t.index ["source_type", "source_id"], name: "index_mentions_on_source" end + create_table "notification_bundles", force: :cascade do |t| + t.datetime "created_at", null: false + t.datetime "ends_at", null: false + t.datetime "starts_at", null: false + t.integer "status", default: 0, null: false + t.datetime "updated_at", null: false + t.integer "user_id", null: false + t.index ["user_id", "starts_at", "ends_at"], name: "idx_on_user_id_starts_at_ends_at_7eae5d3ac5" + t.index ["user_id", "status"], name: "index_notification_bundles_on_user_id_and_status" + t.index ["user_id"], name: "index_notification_bundles_on_user_id", unique: true + end + create_table "notifications", force: :cascade do |t| t.datetime "created_at", null: false t.integer "creator_id" @@ -451,6 +463,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_19_105245) do add_foreign_key "events", "collections" add_foreign_key "mentions", "users", column: "mentionee_id" add_foreign_key "mentions", "users", column: "mentioner_id" + add_foreign_key "notification_bundles", "users" add_foreign_key "notifications", "users" add_foreign_key "notifications", "users", column: "creator_id" add_foreign_key "pins", "cards" diff --git a/db/schema_cache.yml b/db/schema_cache.yml index 58f3518f3..811f78807 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -909,6 +909,41 @@ columns: collation: comment: - *9 + notification_bundles: + - *5 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: ends_at + cast_type: *1 + sql_type_metadata: *2 + 'null': false + default: + default_function: + collation: + comment: + - *6 + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: starts_at + cast_type: *1 + sql_type_metadata: *2 + 'null': false + default: + default_function: + collation: + comment: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: status + cast_type: *3 + sql_type_metadata: *4 + 'null': false + default: 0 + default_function: + collation: + comment: + - *9 + - *25 notifications: - *5 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column @@ -1284,6 +1319,7 @@ primary_keys: filters_stages: filters_tags: mentions: id + notification_bundles: id notifications: id pins: id push_subscriptions: id @@ -1332,6 +1368,7 @@ data_sources: filters_stages: true filters_tags: true mentions: true + notification_bundles: true notifications: true pins: true push_subscriptions: true @@ -2294,6 +2331,58 @@ indexes: nulls_not_distinct: comment: valid: true + notification_bundles: + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: notification_bundles + name: idx_on_user_id_starts_at_ends_at_7eae5d3ac5 + unique: false + columns: + - user_id + - starts_at + - ends_at + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: notification_bundles + name: index_notification_bundles_on_user_id + unique: true + columns: + - user_id + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: notification_bundles + name: index_notification_bundles_on_user_id_and_status + unique: false + columns: + - user_id + - status + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true notifications: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: notifications @@ -2771,4 +2860,4 @@ indexes: comment: valid: true workflows: [] -version: 20250819105245 +version: 20250826084559 diff --git a/test/mailers/previews/notification/bundle_mailer_preview.rb b/test/mailers/previews/notification/bundle_mailer_preview.rb new file mode 100644 index 000000000..3c279e916 --- /dev/null +++ b/test/mailers/previews/notification/bundle_mailer_preview.rb @@ -0,0 +1,6 @@ +class Notification::BundleMailerPreview < ActionMailer::Preview + def notification + ApplicationRecord.current_tenant = "1065895976" + Notification::BundleMailer.notification Notification::Bundle.take! + end +end diff --git a/test/models/notification/bundle_test.rb b/test/models/notification/bundle_test.rb new file mode 100644 index 000000000..1033a46a7 --- /dev/null +++ b/test/models/notification/bundle_test.rb @@ -0,0 +1,35 @@ +require "test_helper" + +class Notification::BundleTest < ActiveSupport::TestCase + setup do + @user = users(:david) + end + + test "new notifications are bundled" do + notification = assert_difference -> { @user.notification_bundles.pending.count }, 1 do + @user.notifications.create!(source: events(:logo_published), creator: @user) + end + + bundle = @user.notification_bundles.pending.last + assert_includes bundle.notifications, notification + end + + test "notifications are bundled withing the aggregation period" do + notification_1 = assert_difference -> { @user.notification_bundles.pending.count }, 1 do + @user.notifications.create!(source: events(:logo_published), creator: @user) + end + travel_to 3.hours.from_now + + notification_2 = assert_no_difference -> { @user.notification_bundles.count } do + @user.notifications.create!(source: events(:logo_published), creator: @user) + end + travel_to 3.hours.from_now + + notification_3 = assert_difference -> { @user.notification_bundles.pending.count }, 1 do + @user.notifications.create!(source: events(:logo_published), creator: @user) + end + + bundle_1, bundle_2 = @user.notification_bundles.last(2) + + end +end From c1605b597280ac8a76b0853b120fcc9074e48237 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 26 Aug 2025 14:21:38 +0200 Subject: [PATCH 04/54] Initial templates for notifications --- app/mailers/application_mailer.rb | 5 ++++- app/mailers/notification/bundle_mailer.rb | 11 ++++++----- app/models/notification/bundle.rb | 8 +++++++- .../notification/bundle_mailer/_notification.html.erb | 2 ++ .../notification/bundle_mailer/notification.html.erb | 8 ++------ .../bundle_mailer/notification/event/_body.html.erb | 9 +++++++++ .../bundle_mailer/notification/event/_header.html.erb | 6 ++++++ .../bundle_mailer/notification/mention/_body.html.erb | 9 +++++++++ .../notification/mention/_header.html.erb | 6 ++++++ 9 files changed, 51 insertions(+), 13 deletions(-) create mode 100644 app/views/notification/bundle_mailer/_notification.html.erb create mode 100644 app/views/notification/bundle_mailer/notification/event/_body.html.erb create mode 100644 app/views/notification/bundle_mailer/notification/event/_header.html.erb create mode 100644 app/views/notification/bundle_mailer/notification/mention/_body.html.erb create mode 100644 app/views/notification/bundle_mailer/notification/mention/_header.html.erb diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 3c34c8148..24cf3f1a1 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,4 +1,7 @@ class ApplicationMailer < ActionMailer::Base - default from: "from@example.com" + default from: "The Fizzy team " + layout "mailer" + append_view_path Rails.root.join("app/views/mailers") + helper AvatarsHelper, HtmlHelper end diff --git a/app/mailers/notification/bundle_mailer.rb b/app/mailers/notification/bundle_mailer.rb index a3e830b16..4947981d5 100644 --- a/app/mailers/notification/bundle_mailer.rb +++ b/app/mailers/notification/bundle_mailer.rb @@ -1,11 +1,12 @@ class Notification::BundleMailer < ApplicationMailer + helper NotificationsHelper + def notification(bundle) @bundle = bundle @notifications = bundle.notifications - - mail( + + mail \ to: bundle.user.email_address, - subject: "You have #{@notifications.count} notifications" - ) + subject: "Recent activity in Fizzy" end -end \ No newline at end of file +end diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index 31b16ac7e..74a681888 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -29,7 +29,9 @@ class Notification::Bundle < ApplicationRecord def deliver processing! - BundleMailer.notification(self).deliver + + BundleMailer.notification(self).deliver if has_unread_notifications? + delivered! end @@ -48,4 +50,8 @@ class Notification::Bundle < ApplicationRecord def window starts_at..ends_at end + + def has_unread_notifications? + notifications.unread.any? + end end diff --git a/app/views/notification/bundle_mailer/_notification.html.erb b/app/views/notification/bundle_mailer/_notification.html.erb new file mode 100644 index 000000000..308cf172f --- /dev/null +++ b/app/views/notification/bundle_mailer/_notification.html.erb @@ -0,0 +1,2 @@ +<%= render "notification/bundle_mailer/notification/#{notification.source_type.underscore}/header", notification: notification %> +<%= render "notification/bundle_mailer/notification/#{notification.source_type.underscore}/body", notification: notification %> diff --git a/app/views/notification/bundle_mailer/notification.html.erb b/app/views/notification/bundle_mailer/notification.html.erb index 1fd9371f0..e638a0724 100644 --- a/app/views/notification/bundle_mailer/notification.html.erb +++ b/app/views/notification/bundle_mailer/notification.html.erb @@ -1,9 +1,5 @@ -

Your Notifications

+

Your Notifications since <%= @bundle.starts_at.strftime("%-l%P on %A, %B %-d") %>

You have <%= @notifications.count %> notifications:

-
    - <% @notifications.each do |notification| %> -
  • Notification ID: <%= notification.id %>
  • - <% end %> -
\ No newline at end of file +<%= render partial: "notification/bundle_mailer/notification", collection: @notifications, as: :notification %> diff --git a/app/views/notification/bundle_mailer/notification/event/_body.html.erb b/app/views/notification/bundle_mailer/notification/event/_body.html.erb new file mode 100644 index 000000000..314a1a6dc --- /dev/null +++ b/app/views/notification/bundle_mailer/notification/event/_body.html.erb @@ -0,0 +1,9 @@ +<% event = notification.source %> + +

+ <%= event_notification_title(event) %> +

+ +

+ <%= event_notification_body(event) %> +

diff --git a/app/views/notification/bundle_mailer/notification/event/_header.html.erb b/app/views/notification/bundle_mailer/notification/event/_header.html.erb new file mode 100644 index 000000000..9c1cc48fd --- /dev/null +++ b/app/views/notification/bundle_mailer/notification/event/_header.html.erb @@ -0,0 +1,6 @@ +

#<%= notification.card.id %> <%= notification.card.collection.name %>

+

+ <%= notification.creator.familiar_name %> + <%= image_tag user_avatar_url(notification.creator), alt: notification.creator.name, size: 48 %> +

+ diff --git a/app/views/notification/bundle_mailer/notification/mention/_body.html.erb b/app/views/notification/bundle_mailer/notification/mention/_body.html.erb new file mode 100644 index 000000000..dae1488bb --- /dev/null +++ b/app/views/notification/bundle_mailer/notification/mention/_body.html.erb @@ -0,0 +1,9 @@ +<% mention = notification.source %> + +

+<%= mention.mentioner.first_name %> mentioned you +

+ +

+<%= mention.source.mentionable_content.truncate(200) %> +

diff --git a/app/views/notification/bundle_mailer/notification/mention/_header.html.erb b/app/views/notification/bundle_mailer/notification/mention/_header.html.erb new file mode 100644 index 000000000..9c1cc48fd --- /dev/null +++ b/app/views/notification/bundle_mailer/notification/mention/_header.html.erb @@ -0,0 +1,6 @@ +

#<%= notification.card.id %> <%= notification.card.collection.name %>

+

+ <%= notification.creator.familiar_name %> + <%= image_tag user_avatar_url(notification.creator), alt: notification.creator.name, size: 48 %> +

+ From b47064b47e19eac36ebfe071505b7d2d446dcdfa Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 26 Aug 2025 15:42:16 +0200 Subject: [PATCH 05/54] Don't expand bundles when adding cards. Leave the aggregation time fixed for now --- app/models/notification/bundle.rb | 5 +---- app/models/user/notifiable.rb | 14 +++++++++++--- ...constraint_from_notification_bundles_user_id.rb | 6 ++++++ db/schema.rb | 4 ++-- db/schema_cache.yml | 9 +++++---- 5 files changed, 25 insertions(+), 13 deletions(-) create mode 100644 db/migrate/20250826131458_remove_unique_constraint_from_notification_bundles_user_id.rb diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index 74a681888..84e1058cb 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -6,6 +6,7 @@ class Notification::Bundle < ApplicationRecord before_create :set_default_window scope :due, -> { pending.where("ends_at <= ?", Time.current) } + scope :containing, -> (notification) { where("starts_at <= ? AND ends_at >= ?", notification.created_at, notification.created_at) } class << self def deliver_all @@ -19,10 +20,6 @@ class Notification::Bundle < ApplicationRecord end end - def add(notification) - update! ends_at: [ ends_at, notification.created_at ].max - end - def notifications user.notifications.where(created_at: window) end diff --git a/app/models/user/notifiable.rb b/app/models/user/notifiable.rb index 0f6cf3b94..015186d15 100644 --- a/app/models/user/notifiable.rb +++ b/app/models/user/notifiable.rb @@ -8,12 +8,20 @@ module User::Notifiable def bundle(notification) transaction do - find_or_create_pending_bundle_for(notification).add(notification) + find_or_create_bundle_for(notification) end end private - def find_or_create_pending_bundle_for(notification) - notification_bundles.pending.last || notification_bundles.create!(starts_at: notification.created_at) + def find_or_create_bundle_for(notification) + find_bundle_for(notification) || create_bundle_for(notification) + end + + def find_bundle_for(notification) + notification_bundles.pending.containing(notification).last + end + + def create_bundle_for(notification) + notification_bundles.create!(starts_at: notification.created_at) end end diff --git a/db/migrate/20250826131458_remove_unique_constraint_from_notification_bundles_user_id.rb b/db/migrate/20250826131458_remove_unique_constraint_from_notification_bundles_user_id.rb new file mode 100644 index 000000000..d5c83236c --- /dev/null +++ b/db/migrate/20250826131458_remove_unique_constraint_from_notification_bundles_user_id.rb @@ -0,0 +1,6 @@ +class RemoveUniqueConstraintFromNotificationBundlesUserId < ActiveRecord::Migration[8.1] + def change + remove_index :notification_bundles, :user_id, unique: true + add_index :notification_bundles, %i[ ends_at status ] + end +end diff --git a/db/schema.rb b/db/schema.rb index c27e4b231..234dc517a 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_08_26_084559) do +ActiveRecord::Schema[8.1].define(version: 2025_08_26_131458) do create_table "accesses", force: :cascade do |t| t.datetime "accessed_at" t.integer "collection_id", null: false @@ -302,9 +302,9 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_26_084559) do t.integer "status", default: 0, null: false t.datetime "updated_at", null: false t.integer "user_id", null: false + t.index ["ends_at", "status"], name: "index_notification_bundles_on_ends_at_and_status" t.index ["user_id", "starts_at", "ends_at"], name: "idx_on_user_id_starts_at_ends_at_7eae5d3ac5" t.index ["user_id", "status"], name: "index_notification_bundles_on_user_id_and_status" - t.index ["user_id"], name: "index_notification_bundles_on_user_id", unique: true end create_table "notifications", force: :cascade do |t| diff --git a/db/schema_cache.yml b/db/schema_cache.yml index 811f78807..434e9ad75 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -2352,10 +2352,11 @@ indexes: valid: true - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: notification_bundles - name: index_notification_bundles_on_user_id - unique: true + name: index_notification_bundles_on_ends_at_and_status + unique: false columns: - - user_id + - ends_at + - status lengths: {} orders: {} opclasses: {} @@ -2860,4 +2861,4 @@ indexes: comment: valid: true workflows: [] -version: 20250826084559 +version: 20250826131458 From 8ba6f5fe8e47aba803eb0c73fd83c5d431424e5b Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 26 Aug 2025 16:49:36 +0200 Subject: [PATCH 06/54] Validate we don't create overlapping bundles --- app/models/notification/bundle.rb | 24 +++++++++++++-- test/models/notification/bundle_test.rb | 41 +++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index 84e1058cb..306b9083f 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -3,10 +3,20 @@ class Notification::Bundle < ApplicationRecord enum :status, %i[ pending processing delivered ] - before_create :set_default_window - scope :due, -> { pending.where("ends_at <= ?", Time.current) } scope :containing, -> (notification) { where("starts_at <= ? AND ends_at >= ?", notification.created_at, notification.created_at) } + scope :overlapping_with, -> (other_bundle) { + where( + "(starts_at <= ? AND ends_at >= ?) OR (starts_at <= ? AND ends_at >= ?) OR (starts_at >= ? AND ends_at <= ?)", + other_bundle.starts_at, other_bundle.starts_at, + other_bundle.ends_at, other_bundle.ends_at, + other_bundle.starts_at, other_bundle.ends_at + ) + } + + before_create :set_default_window + + validate :validate_no_overlapping class << self def deliver_all @@ -51,4 +61,14 @@ class Notification::Bundle < ApplicationRecord def has_unread_notifications? notifications.unread.any? end + + def validate_no_overlapping + if overlapping_bundles.exists? + errors.add(:base, "Bundle window overlaps with an existing pending bundle") + end + end + + def overlapping_bundles + user.notification_bundles.where.not(id: id).overlapping_with(self) + end end diff --git a/test/models/notification/bundle_test.rb b/test/models/notification/bundle_test.rb index 1033a46a7..f9bf4abca 100644 --- a/test/models/notification/bundle_test.rb +++ b/test/models/notification/bundle_test.rb @@ -30,6 +30,47 @@ class Notification::BundleTest < ActiveSupport::TestCase end bundle_1, bundle_2 = @user.notification_bundles.last(2) + assert_includes bundle_1.notifications, notification_1 + assert_includes bundle_1.notifications, notification_2 + assert_includes bundle_2.notifications, notification_3 + end + test "overlapping bundles are invalid" do + bundle_1 = @user.notification_bundles.create!( + starts_at: Time.current, + ends_at: 4.hours.from_now, + status: :pending + ) + + bundle_2 = @user.notification_bundles.build( + starts_at: 2.hours.from_now, + ends_at: 6.hours.from_now, + status: :pending + ) + assert_not bundle_2.valid? + + # Bundle with overlapping end time should be invalid + bundle_3 = @user.notification_bundles.build( + starts_at: 2.hours.ago, + ends_at: 2.hours.from_now, + status: :pending + ) + assert_not bundle_3.valid? + + # Bundle completely within another bundle should be invalid + bundle_4 = @user.notification_bundles.build( + starts_at: 1.hour.from_now, + ends_at: 3.hours.from_now, + status: :pending + ) + assert_not bundle_4.valid? + + # Non-overlapping bundle should be valid + bundle_5 = @user.notification_bundles.build( + starts_at: 5.hours.from_now, + ends_at: 9.hours.from_now, + status: :pending + ) + assert bundle_5.valid? end end From 0126aed6d73bcf11c8f6af80616ca1ddbc2513e1 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 26 Aug 2025 16:59:01 +0200 Subject: [PATCH 07/54] Make sure they don't overlap on creation --- app/models/notification/bundle.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index 306b9083f..592e77411 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -4,7 +4,7 @@ class Notification::Bundle < ApplicationRecord enum :status, %i[ pending processing delivered ] scope :due, -> { pending.where("ends_at <= ?", Time.current) } - scope :containing, -> (notification) { where("starts_at <= ? AND ends_at >= ?", notification.created_at, notification.created_at) } + scope :containing, -> (notification) { where("starts_at <= ? AND ends_at > ?", notification.created_at, notification.created_at) } scope :overlapping_with, -> (other_bundle) { where( "(starts_at <= ? AND ends_at >= ?) OR (starts_at <= ? AND ends_at >= ?) OR (starts_at >= ? AND ends_at <= ?)", From bf63df7d4a935a3d787aa01dc9748c87a529a378 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 26 Aug 2025 17:02:51 +0200 Subject: [PATCH 08/54] Format --- app/models/notification/bundle.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index 592e77411..6ec294996 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -4,8 +4,8 @@ class Notification::Bundle < ApplicationRecord enum :status, %i[ pending processing delivered ] scope :due, -> { pending.where("ends_at <= ?", Time.current) } - scope :containing, -> (notification) { where("starts_at <= ? AND ends_at > ?", notification.created_at, notification.created_at) } - scope :overlapping_with, -> (other_bundle) { + scope :containing, ->(notification) { where("starts_at <= ? AND ends_at > ?", notification.created_at, notification.created_at) } + scope :overlapping_with, ->(other_bundle) { where( "(starts_at <= ? AND ends_at >= ?) OR (starts_at <= ? AND ends_at >= ?) OR (starts_at >= ? AND ends_at <= ?)", other_bundle.starts_at, other_bundle.starts_at, From d99a835184605cae30ae0675cf43c634992bdb07 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 26 Aug 2025 17:41:04 +0200 Subject: [PATCH 09/54] Format --- app/models/notification/bundle.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index 6ec294996..2a5e9ed79 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -5,14 +5,14 @@ class Notification::Bundle < ApplicationRecord scope :due, -> { pending.where("ends_at <= ?", Time.current) } scope :containing, ->(notification) { where("starts_at <= ? AND ends_at > ?", notification.created_at, notification.created_at) } - scope :overlapping_with, ->(other_bundle) { + scope :overlapping_with, ->(other_bundle) do where( "(starts_at <= ? AND ends_at >= ?) OR (starts_at <= ? AND ends_at >= ?) OR (starts_at >= ? AND ends_at <= ?)", other_bundle.starts_at, other_bundle.starts_at, other_bundle.ends_at, other_bundle.ends_at, other_bundle.starts_at, other_bundle.ends_at ) - } + end before_create :set_default_window From 47fb35948b992b8b4ef662f9fea24f768d9bfb6b Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Tue, 26 Aug 2025 18:15:08 +0200 Subject: [PATCH 10/54] It does not make sense to differentiate unread notifications --- app/models/notification/bundle.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index 2a5e9ed79..98c2d131b 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -31,13 +31,13 @@ class Notification::Bundle < ApplicationRecord end def notifications - user.notifications.where(created_at: window) + user.notifications.where(created_at: window).unread end def deliver processing! - BundleMailer.notification(self).deliver if has_unread_notifications? + BundleMailer.notification(self).deliver if notifications.any? delivered! end @@ -58,10 +58,6 @@ class Notification::Bundle < ApplicationRecord starts_at..ends_at end - def has_unread_notifications? - notifications.unread.any? - end - def validate_no_overlapping if overlapping_bundles.exists? errors.add(:base, "Bundle window overlaps with an existing pending bundle") From a1529cadd2da0a050602b9f5daf0277dd101026e Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Tue, 26 Aug 2025 20:56:32 -0500 Subject: [PATCH 11/54] First pass at email styles --- app/mailers/notification/bundle_mailer.rb | 2 +- app/views/layouts/mailer.html.erb | 112 +++++++++++++++++- .../bundle_mailer/_notification.html.erb | 12 +- .../bundle_mailer/notification.html.erb | 12 +- .../notification/_footer.html.erb | 3 + .../notification/_header.html.erb | 1 + .../notification/event/_body.html.erb | 6 +- .../notification/event/_header.html.erb | 6 - .../notification/mention/_body.html.erb | 8 +- .../notification/mention/_header.html.erb | 6 - 10 files changed, 139 insertions(+), 29 deletions(-) create mode 100644 app/views/notification/bundle_mailer/notification/_footer.html.erb create mode 100644 app/views/notification/bundle_mailer/notification/_header.html.erb delete mode 100644 app/views/notification/bundle_mailer/notification/event/_header.html.erb delete mode 100644 app/views/notification/bundle_mailer/notification/mention/_header.html.erb diff --git a/app/mailers/notification/bundle_mailer.rb b/app/mailers/notification/bundle_mailer.rb index 4947981d5..683c83ec8 100644 --- a/app/mailers/notification/bundle_mailer.rb +++ b/app/mailers/notification/bundle_mailer.rb @@ -7,6 +7,6 @@ class Notification::BundleMailer < ApplicationMailer mail \ to: bundle.user.email_address, - subject: "Recent activity in Fizzy" + subject: "Latest Activity in Fizzy" end end diff --git a/app/views/layouts/mailer.html.erb b/app/views/layouts/mailer.html.erb index 3aac9002e..19ad2c252 100644 --- a/app/views/layouts/mailer.html.erb +++ b/app/views/layouts/mailer.html.erb @@ -3,11 +3,119 @@ - <%= yield %> + + <%= yield %> +
diff --git a/app/views/notification/bundle_mailer/_notification.html.erb b/app/views/notification/bundle_mailer/_notification.html.erb index 308cf172f..6310379a2 100644 --- a/app/views/notification/bundle_mailer/_notification.html.erb +++ b/app/views/notification/bundle_mailer/_notification.html.erb @@ -1,2 +1,10 @@ -<%= render "notification/bundle_mailer/notification/#{notification.source_type.underscore}/header", notification: notification %> -<%= render "notification/bundle_mailer/notification/#{notification.source_type.underscore}/body", notification: notification %> + + + + + +
<%= image_tag user_avatar_url(notification.creator), alt: notification.creator.name, class: "avatar", size: 48 %> + <%= render "notification/bundle_mailer/notification/header", notification: notification %> + <%= render "notification/bundle_mailer/notification/#{notification.source_type.underscore}/body", notification: notification %> + <%= render "notification/bundle_mailer/notification/footer", notification: notification %> +
diff --git a/app/views/notification/bundle_mailer/notification.html.erb b/app/views/notification/bundle_mailer/notification.html.erb index e638a0724..e3a06d567 100644 --- a/app/views/notification/bundle_mailer/notification.html.erb +++ b/app/views/notification/bundle_mailer/notification.html.erb @@ -1,5 +1,7 @@ -

Your Notifications since <%= @bundle.starts_at.strftime("%-l%P on %A, %B %-d") %>

- -

You have <%= @notifications.count %> notifications:

- -<%= render partial: "notification/bundle_mailer/notification", collection: @notifications, as: :notification %> + + +

Everything since <%= @bundle.starts_at.strftime("%-l%P on %A, %B %-d") %>

+

You have <%= pluralize @notifications.count, "new notification" %>.

+ <%= render partial: "notification/bundle_mailer/notification", collection: @notifications, as: :notification %> + + diff --git a/app/views/notification/bundle_mailer/notification/_footer.html.erb b/app/views/notification/bundle_mailer/notification/_footer.html.erb new file mode 100644 index 000000000..aa1b4a282 --- /dev/null +++ b/app/views/notification/bundle_mailer/notification/_footer.html.erb @@ -0,0 +1,3 @@ +

+ <%= notification.creator.name %> +

diff --git a/app/views/notification/bundle_mailer/notification/_header.html.erb b/app/views/notification/bundle_mailer/notification/_header.html.erb new file mode 100644 index 000000000..2cf916c5d --- /dev/null +++ b/app/views/notification/bundle_mailer/notification/_header.html.erb @@ -0,0 +1 @@ +

#<%= notification.card.id %> | <%= notification.card.collection.name %>

diff --git a/app/views/notification/bundle_mailer/notification/event/_body.html.erb b/app/views/notification/bundle_mailer/notification/event/_body.html.erb index 314a1a6dc..4ad6f204a 100644 --- a/app/views/notification/bundle_mailer/notification/event/_body.html.erb +++ b/app/views/notification/bundle_mailer/notification/event/_body.html.erb @@ -1,9 +1,9 @@ <% event = notification.source %> -

- <%= event_notification_title(event) %> +

+ <%= link_to event_notification_title(event), collection_card_url(notification.card.collection, notification.card) %>

-

+

<%= event_notification_body(event) %>

diff --git a/app/views/notification/bundle_mailer/notification/event/_header.html.erb b/app/views/notification/bundle_mailer/notification/event/_header.html.erb deleted file mode 100644 index 9c1cc48fd..000000000 --- a/app/views/notification/bundle_mailer/notification/event/_header.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -

#<%= notification.card.id %> <%= notification.card.collection.name %>

-

- <%= notification.creator.familiar_name %> - <%= image_tag user_avatar_url(notification.creator), alt: notification.creator.name, size: 48 %> -

- diff --git a/app/views/notification/bundle_mailer/notification/mention/_body.html.erb b/app/views/notification/bundle_mailer/notification/mention/_body.html.erb index dae1488bb..a53a996bf 100644 --- a/app/views/notification/bundle_mailer/notification/mention/_body.html.erb +++ b/app/views/notification/bundle_mailer/notification/mention/_body.html.erb @@ -1,9 +1,9 @@ <% mention = notification.source %> -

-<%= mention.mentioner.first_name %> mentioned you +

+ <%= link_to "#{mention.mentioner.first_name} mentioned you", collection_card_url(notification.card.collection, notification.card) %>

-

-<%= mention.source.mentionable_content.truncate(200) %> +

+ <%= mention.source.mentionable_content.truncate(200) %>

diff --git a/app/views/notification/bundle_mailer/notification/mention/_header.html.erb b/app/views/notification/bundle_mailer/notification/mention/_header.html.erb deleted file mode 100644 index 9c1cc48fd..000000000 --- a/app/views/notification/bundle_mailer/notification/mention/_header.html.erb +++ /dev/null @@ -1,6 +0,0 @@ -

#<%= notification.card.id %> <%= notification.card.collection.name %>

-

- <%= notification.creator.familiar_name %> - <%= image_tag user_avatar_url(notification.creator), alt: notification.creator.name, size: 48 %> -

- From cd6ff41eeaeb35a47691e9e910dac1647a5d938a Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 11:21:11 +0200 Subject: [PATCH 12/54] Fix port for local dev --- config/environments/development.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/environments/development.rb b/config/environments/development.rb index 0703601ec..e8cdb8954 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -82,7 +82,7 @@ Rails.application.configure do config.hosts = %w[ fizzy.localhost localhost 127.0.0.1 ] # Host to be used in links rendered from ActionMailer - config.action_mailer.default_url_options = { host: config.hosts.first, port: 3005 } + config.action_mailer.default_url_options = { host: config.hosts.first, port: 3006 } if Rails.root.join("tmp/solid-queue.txt").exist? config.active_job.queue_adapter = :solid_queue From 45d3bb4b3e5220f385c1ead08c237f4b2c6b22b1 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 11:30:08 +0200 Subject: [PATCH 13/54] Set current tenant in mailers --- app/mailers/application_mailer.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 24cf3f1a1..1ee79fb87 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -4,4 +4,9 @@ class ApplicationMailer < ActionMailer::Base layout "mailer" append_view_path Rails.root.join("app/views/mailers") helper AvatarsHelper, HtmlHelper + + private + def default_url_options + super.merge(script_name: "/#{ApplicationRecord.current_tenant}") + end end From 30d8848057c1958aa4ba423923674190515ed231 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 11:44:11 +0200 Subject: [PATCH 14/54] Move mailer templates to the mailers root --- .../notification/bundle_mailer/_notification.html.erb | 0 .../notification/bundle_mailer/notification.html.erb | 0 .../notification/bundle_mailer/notification.text.erb | 0 .../notification/bundle_mailer/notification/_footer.html.erb | 0 .../notification/bundle_mailer/notification/_header.html.erb | 0 .../notification/bundle_mailer/notification/event/_body.html.erb | 0 .../bundle_mailer/notification/mention/_body.html.erb | 0 7 files changed, 0 insertions(+), 0 deletions(-) rename app/views/{ => mailers}/notification/bundle_mailer/_notification.html.erb (100%) rename app/views/{ => mailers}/notification/bundle_mailer/notification.html.erb (100%) rename app/views/{ => mailers}/notification/bundle_mailer/notification.text.erb (100%) rename app/views/{ => mailers}/notification/bundle_mailer/notification/_footer.html.erb (100%) rename app/views/{ => mailers}/notification/bundle_mailer/notification/_header.html.erb (100%) rename app/views/{ => mailers}/notification/bundle_mailer/notification/event/_body.html.erb (100%) rename app/views/{ => mailers}/notification/bundle_mailer/notification/mention/_body.html.erb (100%) diff --git a/app/views/notification/bundle_mailer/_notification.html.erb b/app/views/mailers/notification/bundle_mailer/_notification.html.erb similarity index 100% rename from app/views/notification/bundle_mailer/_notification.html.erb rename to app/views/mailers/notification/bundle_mailer/_notification.html.erb diff --git a/app/views/notification/bundle_mailer/notification.html.erb b/app/views/mailers/notification/bundle_mailer/notification.html.erb similarity index 100% rename from app/views/notification/bundle_mailer/notification.html.erb rename to app/views/mailers/notification/bundle_mailer/notification.html.erb diff --git a/app/views/notification/bundle_mailer/notification.text.erb b/app/views/mailers/notification/bundle_mailer/notification.text.erb similarity index 100% rename from app/views/notification/bundle_mailer/notification.text.erb rename to app/views/mailers/notification/bundle_mailer/notification.text.erb diff --git a/app/views/notification/bundle_mailer/notification/_footer.html.erb b/app/views/mailers/notification/bundle_mailer/notification/_footer.html.erb similarity index 100% rename from app/views/notification/bundle_mailer/notification/_footer.html.erb rename to app/views/mailers/notification/bundle_mailer/notification/_footer.html.erb diff --git a/app/views/notification/bundle_mailer/notification/_header.html.erb b/app/views/mailers/notification/bundle_mailer/notification/_header.html.erb similarity index 100% rename from app/views/notification/bundle_mailer/notification/_header.html.erb rename to app/views/mailers/notification/bundle_mailer/notification/_header.html.erb diff --git a/app/views/notification/bundle_mailer/notification/event/_body.html.erb b/app/views/mailers/notification/bundle_mailer/notification/event/_body.html.erb similarity index 100% rename from app/views/notification/bundle_mailer/notification/event/_body.html.erb rename to app/views/mailers/notification/bundle_mailer/notification/event/_body.html.erb diff --git a/app/views/notification/bundle_mailer/notification/mention/_body.html.erb b/app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb similarity index 100% rename from app/views/notification/bundle_mailer/notification/mention/_body.html.erb rename to app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb From 75a830e9b39fd44d2f0136b347a4e2bb9eb44888 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 11:46:48 +0200 Subject: [PATCH 15/54] Link to the event/notification source directly E.g: link to comments --- .../bundle_mailer/notification/event/_body.html.erb | 2 +- .../bundle_mailer/notification/mention/_body.html.erb | 2 +- config/routes.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/mailers/notification/bundle_mailer/notification/event/_body.html.erb b/app/views/mailers/notification/bundle_mailer/notification/event/_body.html.erb index 4ad6f204a..89273ef0d 100644 --- a/app/views/mailers/notification/bundle_mailer/notification/event/_body.html.erb +++ b/app/views/mailers/notification/bundle_mailer/notification/event/_body.html.erb @@ -1,7 +1,7 @@ <% event = notification.source %>

- <%= link_to event_notification_title(event), collection_card_url(notification.card.collection, notification.card) %> + <%= link_to event_notification_title(event), notification.source %>

diff --git a/app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb b/app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb index a53a996bf..93ad1989b 100644 --- a/app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb +++ b/app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb @@ -1,7 +1,7 @@ <% mention = notification.source %>

- <%= link_to "#{mention.mentioner.first_name} mentioned you", collection_card_url(notification.card.collection, notification.card) %> + <%= link_to "#{mention.mentioner.first_name} mentioned you", event.source %>

diff --git a/config/routes.rb b/config/routes.rb index 6d55b04e5..769cd0ccb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -173,7 +173,7 @@ Rails.application.routes.draw do end resolve "Event" do |event, options| - polymorphic_path(event.target, options) + polymorphic_path(event.eventable, options) end get "up", to: "rails/health#show", as: :rails_health_check From 1b608ab67b81781745f339e8513221fee7da14a8 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 11:53:49 +0200 Subject: [PATCH 16/54] Fix link source --- .../bundle_mailer/notification/mention/_body.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb b/app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb index 93ad1989b..5cc259b2a 100644 --- a/app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb +++ b/app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb @@ -1,7 +1,7 @@ <% mention = notification.source %>

- <%= link_to "#{mention.mentioner.first_name} mentioned you", event.source %> + <%= link_to "#{mention.mentioner.first_name} mentioned you", mention %>

From 13d053d2859cee6403ea25f9c12d0fe3e2d1beca Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 11:57:01 +0200 Subject: [PATCH 17/54] Flatten dirs --- .../bundle_mailer/{notification => }/_footer.html.erb | 0 .../bundle_mailer/{notification => }/_header.html.erb | 0 .../notification/bundle_mailer/_notification.html.erb | 6 +++--- .../bundle_mailer/{notification => }/event/_body.html.erb | 0 .../bundle_mailer/{notification => }/mention/_body.html.erb | 0 5 files changed, 3 insertions(+), 3 deletions(-) rename app/views/mailers/notification/bundle_mailer/{notification => }/_footer.html.erb (100%) rename app/views/mailers/notification/bundle_mailer/{notification => }/_header.html.erb (100%) rename app/views/mailers/notification/bundle_mailer/{notification => }/event/_body.html.erb (100%) rename app/views/mailers/notification/bundle_mailer/{notification => }/mention/_body.html.erb (100%) diff --git a/app/views/mailers/notification/bundle_mailer/notification/_footer.html.erb b/app/views/mailers/notification/bundle_mailer/_footer.html.erb similarity index 100% rename from app/views/mailers/notification/bundle_mailer/notification/_footer.html.erb rename to app/views/mailers/notification/bundle_mailer/_footer.html.erb diff --git a/app/views/mailers/notification/bundle_mailer/notification/_header.html.erb b/app/views/mailers/notification/bundle_mailer/_header.html.erb similarity index 100% rename from app/views/mailers/notification/bundle_mailer/notification/_header.html.erb rename to app/views/mailers/notification/bundle_mailer/_header.html.erb diff --git a/app/views/mailers/notification/bundle_mailer/_notification.html.erb b/app/views/mailers/notification/bundle_mailer/_notification.html.erb index 6310379a2..20210ac2e 100644 --- a/app/views/mailers/notification/bundle_mailer/_notification.html.erb +++ b/app/views/mailers/notification/bundle_mailer/_notification.html.erb @@ -2,9 +2,9 @@ <%= image_tag user_avatar_url(notification.creator), alt: notification.creator.name, class: "avatar", size: 48 %> - <%= render "notification/bundle_mailer/notification/header", notification: notification %> - <%= render "notification/bundle_mailer/notification/#{notification.source_type.underscore}/body", notification: notification %> - <%= render "notification/bundle_mailer/notification/footer", notification: notification %> + <%= render "notification/bundle_mailer/header", notification: notification %> + <%= render "notification/bundle_mailer/#{notification.source_type.underscore}/body", notification: notification %> + <%= render "notification/bundle_mailer/footer", notification: notification %> diff --git a/app/views/mailers/notification/bundle_mailer/notification/event/_body.html.erb b/app/views/mailers/notification/bundle_mailer/event/_body.html.erb similarity index 100% rename from app/views/mailers/notification/bundle_mailer/notification/event/_body.html.erb rename to app/views/mailers/notification/bundle_mailer/event/_body.html.erb diff --git a/app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb b/app/views/mailers/notification/bundle_mailer/mention/_body.html.erb similarity index 100% rename from app/views/mailers/notification/bundle_mailer/notification/mention/_body.html.erb rename to app/views/mailers/notification/bundle_mailer/mention/_body.html.erb From d9a19a85b1a3ee3b74ffdf1b2b3dda1fb1ccb12c Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 12:03:51 +0200 Subject: [PATCH 18/54] Add text templates counterparts for the bundle mailer --- .../mailers/notification/bundle_mailer/_footer.text.erb | 1 + .../mailers/notification/bundle_mailer/_header.text.erb | 1 + .../notification/bundle_mailer/_notification.text.erb | 6 ++++++ .../mailers/notification/bundle_mailer/event/_body.text.erb | 6 ++++++ .../notification/bundle_mailer/mention/_body.text.erb | 6 ++++++ .../notification/bundle_mailer/notification.text.erb | 5 ++++- 6 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 app/views/mailers/notification/bundle_mailer/_footer.text.erb create mode 100644 app/views/mailers/notification/bundle_mailer/_header.text.erb create mode 100644 app/views/mailers/notification/bundle_mailer/_notification.text.erb create mode 100644 app/views/mailers/notification/bundle_mailer/event/_body.text.erb create mode 100644 app/views/mailers/notification/bundle_mailer/mention/_body.text.erb diff --git a/app/views/mailers/notification/bundle_mailer/_footer.text.erb b/app/views/mailers/notification/bundle_mailer/_footer.text.erb new file mode 100644 index 000000000..6ab8bb14f --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/_footer.text.erb @@ -0,0 +1 @@ +<%= notification.creator.name %> \ No newline at end of file diff --git a/app/views/mailers/notification/bundle_mailer/_header.text.erb b/app/views/mailers/notification/bundle_mailer/_header.text.erb new file mode 100644 index 000000000..db036727d --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/_header.text.erb @@ -0,0 +1 @@ +#<%= notification.card.id %> | <%= notification.card.collection.name %> \ No newline at end of file diff --git a/app/views/mailers/notification/bundle_mailer/_notification.text.erb b/app/views/mailers/notification/bundle_mailer/_notification.text.erb new file mode 100644 index 000000000..fd96276ff --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/_notification.text.erb @@ -0,0 +1,6 @@ +================================================================================ +[<%= notification.creator.name %>] + +<%= render "notification/bundle_mailer/header", notification: notification %> +<%= render "notification/bundle_mailer/#{notification.source_type.underscore}/body", notification: notification %> +<%= render "notification/bundle_mailer/footer", notification: notification %> \ No newline at end of file diff --git a/app/views/mailers/notification/bundle_mailer/event/_body.text.erb b/app/views/mailers/notification/bundle_mailer/event/_body.text.erb new file mode 100644 index 000000000..c7db4eeec --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/event/_body.text.erb @@ -0,0 +1,6 @@ +<% event = notification.source %> + +<%= event_notification_title(event) %> +<%= notification_url(notification.source) %> + +<%= event_notification_body(event) %> \ No newline at end of file diff --git a/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb b/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb new file mode 100644 index 000000000..ae78b3671 --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb @@ -0,0 +1,6 @@ +<% mention = notification.source %> + +<%= mention.mentioner.first_name %> mentioned you +<%= notification_url(mention) %> + +<%= mention.source.mentionable_content.truncate(200) %> \ No newline at end of file diff --git a/app/views/mailers/notification/bundle_mailer/notification.text.erb b/app/views/mailers/notification/bundle_mailer/notification.text.erb index 8b7c8f424..c4c7c7c55 100644 --- a/app/views/mailers/notification/bundle_mailer/notification.text.erb +++ b/app/views/mailers/notification/bundle_mailer/notification.text.erb @@ -1 +1,4 @@ -PENDING... +Everything since <%= @bundle.starts_at.strftime("%-l%P on %A, %B %-d") %> +You have <%= pluralize @notifications.count, "new notification" %>. + +<%= render partial: "notification/bundle_mailer/notification", collection: @notifications, as: :notification %> From 0a32cfd611965b484de284f028972ffdf540d2d2 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 12:06:21 +0200 Subject: [PATCH 19/54] Add newline to separate --- .../mailers/notification/bundle_mailer/_notification.text.erb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/mailers/notification/bundle_mailer/_notification.text.erb b/app/views/mailers/notification/bundle_mailer/_notification.text.erb index fd96276ff..ad2d7f1d4 100644 --- a/app/views/mailers/notification/bundle_mailer/_notification.text.erb +++ b/app/views/mailers/notification/bundle_mailer/_notification.text.erb @@ -3,4 +3,5 @@ <%= render "notification/bundle_mailer/header", notification: notification %> <%= render "notification/bundle_mailer/#{notification.source_type.underscore}/body", notification: notification %> -<%= render "notification/bundle_mailer/footer", notification: notification %> \ No newline at end of file +<%= render "notification/bundle_mailer/footer", notification: notification %> + From ccceeb1699205500eb3142d24f21e435534a9cd5 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 15:48:27 +0200 Subject: [PATCH 20/54] Add method for delivery job --- app/models/notification/bundle.rb | 3 ++- test/models/notification/bundle_test.rb | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index 98c2d131b..78d4b5dd8 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -21,7 +21,8 @@ class Notification::Bundle < ApplicationRecord class << self def deliver_all due.in_batches do |batch| - DeliverJob.perform_all_later batch + jobs = batch.collect { DeliverJob.new(it) } + ActiveJob.perform_all_later jobs end end diff --git a/test/models/notification/bundle_test.rb b/test/models/notification/bundle_test.rb index f9bf4abca..1149ae174 100644 --- a/test/models/notification/bundle_test.rb +++ b/test/models/notification/bundle_test.rb @@ -73,4 +73,22 @@ class Notification::BundleTest < ActiveSupport::TestCase ) assert bundle_5.valid? end + + test "deliver_all delivers due bundles" do + notification = @user.notifications.create!(source: events(:logo_published), creator: @user) + + bundle = @user.notification_bundles.pending.last + + assert bundle.pending? + assert_includes bundle.notifications, notification + + bundle.update!(ends_at: 1.minute.ago) + + perform_enqueued_jobs do + Notification::Bundle.deliver_all + end + + bundle.reload + assert bundle.delivered? + end end From 5a4c40fd56bb2e0998aab608d7e5bbd502907ee1 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 15:51:20 +0200 Subject: [PATCH 21/54] Test to make sure it skips bundles that are not due --- test/models/notification/bundle_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/models/notification/bundle_test.rb b/test/models/notification/bundle_test.rb index 1149ae174..63ccad0b6 100644 --- a/test/models/notification/bundle_test.rb +++ b/test/models/notification/bundle_test.rb @@ -91,4 +91,19 @@ class Notification::BundleTest < ActiveSupport::TestCase bundle.reload assert bundle.delivered? end + + test "deliver_all don't deliver bundles that are not due" do + @user.notifications.create!(source: events(:logo_published), creator: @user) + bundle = @user.notification_bundles.pending.last + + bundle.update!(ends_at: 1.minute.from_now) + + perform_enqueued_jobs do + Notification::Bundle.deliver_all + end + + bundle.reload + assert bundle.pending? + end + end From 88976806b9fdc10d57defd90299fc45267c35760 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 15:56:58 +0200 Subject: [PATCH 22/54] Format --- test/models/notification/bundle_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/models/notification/bundle_test.rb b/test/models/notification/bundle_test.rb index 63ccad0b6..5da167f64 100644 --- a/test/models/notification/bundle_test.rb +++ b/test/models/notification/bundle_test.rb @@ -105,5 +105,4 @@ class Notification::BundleTest < ActiveSupport::TestCase bundle.reload assert bundle.pending? end - end From 93c9855a05ea95868b2f700d42715d2178b4ccb8 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Wed, 27 Aug 2025 16:52:05 +0200 Subject: [PATCH 23/54] Use URL so that the helper works from emails --- app/helpers/avatars_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index f3b9e5e2f..3dccac678 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -28,6 +28,6 @@ module AvatarsHelper end def avatar_image_tag(user, **options) - image_tag user_avatar_path(user), aria: { hidden: "true" }, size: 48, title: user.name, **options + image_tag user_avatar_url(user), aria: { hidden: "true" }, size: 48, title: user.name, **options end end From 9710be65d2754280464f355fc32eb6218187a811 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 27 Aug 2025 10:39:14 -0500 Subject: [PATCH 24/54] Smaller avatars, aligned to the top of the title --- app/views/layouts/mailer.html.erb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app/views/layouts/mailer.html.erb b/app/views/layouts/mailer.html.erb index 19ad2c252..8ff618c11 100644 --- a/app/views/layouts/mailer.html.erb +++ b/app/views/layouts/mailer.html.erb @@ -45,16 +45,18 @@ .avatar { border-radius: 50%; - height: 4em; + height: 3em; margin-right: 0.3em; - max-height: 4em; - min-height: 4em; + max-height: 3em; + min-height: 3em; overflow: hidden; - width: 4em; + width: 3em; } .avatar__container { - width: 4.3em; + padding-top: 1em; + vertical-align: top; + width: 3.5em; } .btn { From 16d2cd4836f32a80408b61f1183d30103c95dc2f Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 27 Aug 2025 10:40:36 -0500 Subject: [PATCH 25/54] Allow a few more more words --- .../mailers/notification/bundle_mailer/mention/_body.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/mailers/notification/bundle_mailer/mention/_body.html.erb b/app/views/mailers/notification/bundle_mailer/mention/_body.html.erb index 5cc259b2a..d08b01f4b 100644 --- a/app/views/mailers/notification/bundle_mailer/mention/_body.html.erb +++ b/app/views/mailers/notification/bundle_mailer/mention/_body.html.erb @@ -5,5 +5,5 @@

- <%= mention.source.mentionable_content.truncate(200) %> + <%= mention.source.mentionable_content.truncate(250) %>

From e3f53488650647f8ef15a0e4d7e4d91c4118480f Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 27 Aug 2025 10:41:30 -0500 Subject: [PATCH 26/54] Plain text layout and spacing, squish whitespace to remove action text junk --- .../notification/bundle_mailer/event/_body.text.erb | 5 +++-- .../notification/bundle_mailer/mention/_body.text.erb | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/views/mailers/notification/bundle_mailer/event/_body.text.erb b/app/views/mailers/notification/bundle_mailer/event/_body.text.erb index c7db4eeec..770e9edaa 100644 --- a/app/views/mailers/notification/bundle_mailer/event/_body.text.erb +++ b/app/views/mailers/notification/bundle_mailer/event/_body.text.erb @@ -1,6 +1,7 @@ <% event = notification.source %> <%= event_notification_title(event) %> -<%= notification_url(notification.source) %> -<%= event_notification_body(event) %> \ No newline at end of file +<%= event_notification_body(event).squish %> + +<%= notification_url(notification.source) %> diff --git a/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb b/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb index ae78b3671..4056ae1b4 100644 --- a/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb +++ b/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb @@ -1,6 +1,7 @@ <% mention = notification.source %> -<%= mention.mentioner.first_name %> mentioned you -<%= notification_url(mention) %> +<%= mention.mentioner.first_name %> mentioned you. -<%= mention.source.mentionable_content.truncate(200) %> \ No newline at end of file +<%= mention.source.mentionable_content.truncate(250) %> + +<%= notification_url(mention) %> From adc91b982312c1923e25cd87a577b08ff64c4a68 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 27 Aug 2025 10:42:02 -0500 Subject: [PATCH 27/54] Can just put this in the header --- app/views/mailers/notification/bundle_mailer/_footer.html.erb | 3 --- app/views/mailers/notification/bundle_mailer/_footer.text.erb | 1 - app/views/mailers/notification/bundle_mailer/_header.html.erb | 2 +- app/views/mailers/notification/bundle_mailer/_header.text.erb | 2 +- .../mailers/notification/bundle_mailer/_notification.html.erb | 1 - .../mailers/notification/bundle_mailer/_notification.text.erb | 2 -- 6 files changed, 2 insertions(+), 9 deletions(-) delete mode 100644 app/views/mailers/notification/bundle_mailer/_footer.html.erb delete mode 100644 app/views/mailers/notification/bundle_mailer/_footer.text.erb diff --git a/app/views/mailers/notification/bundle_mailer/_footer.html.erb b/app/views/mailers/notification/bundle_mailer/_footer.html.erb deleted file mode 100644 index aa1b4a282..000000000 --- a/app/views/mailers/notification/bundle_mailer/_footer.html.erb +++ /dev/null @@ -1,3 +0,0 @@ -

- <%= notification.creator.name %> -

diff --git a/app/views/mailers/notification/bundle_mailer/_footer.text.erb b/app/views/mailers/notification/bundle_mailer/_footer.text.erb deleted file mode 100644 index 6ab8bb14f..000000000 --- a/app/views/mailers/notification/bundle_mailer/_footer.text.erb +++ /dev/null @@ -1 +0,0 @@ -<%= notification.creator.name %> \ No newline at end of file diff --git a/app/views/mailers/notification/bundle_mailer/_header.html.erb b/app/views/mailers/notification/bundle_mailer/_header.html.erb index 2cf916c5d..b9ba83e74 100644 --- a/app/views/mailers/notification/bundle_mailer/_header.html.erb +++ b/app/views/mailers/notification/bundle_mailer/_header.html.erb @@ -1 +1 @@ -

#<%= notification.card.id %> | <%= notification.card.collection.name %>

+

#<%= notification.card.id %> | <%= notification.card.collection.name %> | <%= notification.creator.name %>

diff --git a/app/views/mailers/notification/bundle_mailer/_header.text.erb b/app/views/mailers/notification/bundle_mailer/_header.text.erb index db036727d..311033d33 100644 --- a/app/views/mailers/notification/bundle_mailer/_header.text.erb +++ b/app/views/mailers/notification/bundle_mailer/_header.text.erb @@ -1 +1 @@ -#<%= notification.card.id %> | <%= notification.card.collection.name %> \ No newline at end of file +#<%= notification.card.id %> | <%= notification.card.collection.name %> | <%= notification.creator.name %> \ No newline at end of file diff --git a/app/views/mailers/notification/bundle_mailer/_notification.html.erb b/app/views/mailers/notification/bundle_mailer/_notification.html.erb index 20210ac2e..5dfddcf31 100644 --- a/app/views/mailers/notification/bundle_mailer/_notification.html.erb +++ b/app/views/mailers/notification/bundle_mailer/_notification.html.erb @@ -4,7 +4,6 @@ <%= render "notification/bundle_mailer/header", notification: notification %> <%= render "notification/bundle_mailer/#{notification.source_type.underscore}/body", notification: notification %> - <%= render "notification/bundle_mailer/footer", notification: notification %> diff --git a/app/views/mailers/notification/bundle_mailer/_notification.text.erb b/app/views/mailers/notification/bundle_mailer/_notification.text.erb index ad2d7f1d4..baaee2bf5 100644 --- a/app/views/mailers/notification/bundle_mailer/_notification.text.erb +++ b/app/views/mailers/notification/bundle_mailer/_notification.text.erb @@ -1,7 +1,5 @@ ================================================================================ -[<%= notification.creator.name %>] <%= render "notification/bundle_mailer/header", notification: notification %> <%= render "notification/bundle_mailer/#{notification.source_type.underscore}/body", notification: notification %> -<%= render "notification/bundle_mailer/footer", notification: notification %> From a106069b7e11bda2eb9529cb649b5bcb0917a82f Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 27 Aug 2025 10:42:12 -0500 Subject: [PATCH 28/54] Link to index --- .../mailers/notification/bundle_mailer/notification.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/mailers/notification/bundle_mailer/notification.html.erb b/app/views/mailers/notification/bundle_mailer/notification.html.erb index e3a06d567..4e881739b 100644 --- a/app/views/mailers/notification/bundle_mailer/notification.html.erb +++ b/app/views/mailers/notification/bundle_mailer/notification.html.erb @@ -1,7 +1,7 @@

Everything since <%= @bundle.starts_at.strftime("%-l%P on %A, %B %-d") %>

-

You have <%= pluralize @notifications.count, "new notification" %>.

+

You have <%= link_to pluralize(@notifications.count, "new notification"), notifications_url %>.

<%= render partial: "notification/bundle_mailer/notification", collection: @notifications, as: :notification %> From 2fb3e192b88f6efddb62ce6c034d9ee0d9452632 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 27 Aug 2025 10:55:12 -0500 Subject: [PATCH 29/54] Need to url for collection/card/comment @jorgemanrubia for some reason `url_for` is not including the host in here. It seems to be correctly set in the environment configs --- .../mailers/notification/bundle_mailer/event/_body.text.erb | 2 +- .../mailers/notification/bundle_mailer/mention/_body.text.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/mailers/notification/bundle_mailer/event/_body.text.erb b/app/views/mailers/notification/bundle_mailer/event/_body.text.erb index 770e9edaa..292c2ad7c 100644 --- a/app/views/mailers/notification/bundle_mailer/event/_body.text.erb +++ b/app/views/mailers/notification/bundle_mailer/event/_body.text.erb @@ -4,4 +4,4 @@ <%= event_notification_body(event).squish %> -<%= notification_url(notification.source) %> +<%= url_for(notification.source) %> diff --git a/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb b/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb index 4056ae1b4..7ba307f01 100644 --- a/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb +++ b/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb @@ -4,4 +4,4 @@ <%= mention.source.mentionable_content.truncate(250) %> -<%= notification_url(mention) %> +<%= url_for(mention) %> From eff5185a17a3a399a64b13684d518e41499fe0ac Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Wed, 27 Aug 2025 11:32:16 -0500 Subject: [PATCH 30/54] Stub unsubscribe footer --- app/views/layouts/mailer.html.erb | 6 ++++++ .../notification/bundle_mailer/notification.html.erb | 4 ++++ .../notification/bundle_mailer/notification.text.erb | 9 +++++++++ 3 files changed, 19 insertions(+) diff --git a/app/views/layouts/mailer.html.erb b/app/views/layouts/mailer.html.erb index 8ff618c11..793392ca3 100644 --- a/app/views/layouts/mailer.html.erb +++ b/app/views/layouts/mailer.html.erb @@ -81,6 +81,12 @@ margin-bottom: 0; margin-top: 0; } + + .footer { + border-top: 1px solid #ccc; + margin-top: 3em; + padding-top: 1em; + } .notification { margin-top: 1rem; diff --git a/app/views/mailers/notification/bundle_mailer/notification.html.erb b/app/views/mailers/notification/bundle_mailer/notification.html.erb index 4e881739b..31d2a3b52 100644 --- a/app/views/mailers/notification/bundle_mailer/notification.html.erb +++ b/app/views/mailers/notification/bundle_mailer/notification.html.erb @@ -3,5 +3,9 @@

Everything since <%= @bundle.starts_at.strftime("%-l%P on %A, %B %-d") %>

You have <%= link_to pluralize(@notifications.count, "new notification"), notifications_url %>.

<%= render partial: "notification/bundle_mailer/notification", collection: @notifications, as: :notification %> + diff --git a/app/views/mailers/notification/bundle_mailer/notification.text.erb b/app/views/mailers/notification/bundle_mailer/notification.text.erb index c4c7c7c55..05827cc67 100644 --- a/app/views/mailers/notification/bundle_mailer/notification.text.erb +++ b/app/views/mailers/notification/bundle_mailer/notification.text.erb @@ -2,3 +2,12 @@ Everything since <%= @bundle.starts_at.strftime("%-l%P on %A, %B %-d") %> You have <%= pluralize @notifications.count, "new notification" %>. <%= render partial: "notification/bundle_mailer/notification", collection: @notifications, as: :notification %> +-------------------------------------------------------------------------------- + +Fizzy emails you about new notifications every few hours. + +Change how often you get these: +<%= settings_notifications_url %> + +Unsubscribe from all email notifications: +<%= root_url %> From 65d90b73bd816298b8b859038887b207c08c8868 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 10:32:01 +0200 Subject: [PATCH 31/54] Use URLs instead of paths so that these work from emails Having to do this is not ideal but it's the simpler way of reusing other custom route resolvers. The alternative would be to duplicate the code for resolving comments and such. --- config/routes.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 769cd0ccb..ac9ca1b75 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -165,15 +165,15 @@ Rails.application.routes.draw do end resolve "Mention" do |mention, options| - polymorphic_path(mention.source, options) + polymorphic_url(mention.source, options) end resolve "Notification" do |notification, options| - polymorphic_path(notification.notifiable_target, options) + polymorphic_url(notification.notifiable_target, options) end resolve "Event" do |event, options| - polymorphic_path(event.eventable, options) + polymorphic_url(event.eventable, options) end get "up", to: "rails/health#show", as: :rails_health_check From 169bf490eb53eeaf870b2896cff765b12d726d91 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 10:34:36 +0200 Subject: [PATCH 32/54] Leverage existing polymorphic routes --- .../mailers/notification/bundle_mailer/event/_body.text.erb | 2 +- .../mailers/notification/bundle_mailer/mention/_body.text.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/mailers/notification/bundle_mailer/event/_body.text.erb b/app/views/mailers/notification/bundle_mailer/event/_body.text.erb index 292c2ad7c..5100aff60 100644 --- a/app/views/mailers/notification/bundle_mailer/event/_body.text.erb +++ b/app/views/mailers/notification/bundle_mailer/event/_body.text.erb @@ -4,4 +4,4 @@ <%= event_notification_body(event).squish %> -<%= url_for(notification.source) %> +<%= url_for notification %> diff --git a/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb b/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb index 7ba307f01..da6f5babd 100644 --- a/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb +++ b/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb @@ -4,4 +4,4 @@ <%= mention.source.mentionable_content.truncate(250) %> -<%= url_for(mention) %> +<%= url_for mention %> From 082a10e3d0c2779670e1cadff8cc2454caa2ade5 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 11:20:28 +0200 Subject: [PATCH 33/54] Add notification settings model --- app/models/notification/settings.rb | 6 +++ app/models/user.rb | 6 +-- app/models/user/configurable.rb | 10 ++++ ...0828084732_create_notification_settings.rb | 12 +++++ db/schema.rb | 12 ++++- db/schema_cache.yml | 53 ++++++++++++++++++- test/fixtures/notification/settings.yml | 18 +++++++ test/models/user/configurable_test.rb | 8 +++ 8 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 app/models/notification/settings.rb create mode 100644 app/models/user/configurable.rb create mode 100644 db/migrate/20250828084732_create_notification_settings.rb create mode 100644 test/fixtures/notification/settings.yml create mode 100644 test/models/user/configurable_test.rb diff --git a/app/models/notification/settings.rb b/app/models/notification/settings.rb new file mode 100644 index 000000000..e74f35041 --- /dev/null +++ b/app/models/notification/settings.rb @@ -0,0 +1,6 @@ +class Notification::Settings < ApplicationRecord + belongs_to :user + + enum :bundle_email_frequency, %i[ never every_few_hours daily weekly ], + default: :never, default: :never, prefix: :bundle_email +end diff --git a/app/models/user.rb b/app/models/user.rb index 676ce459a..229518ff3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,6 @@ class User < ApplicationRecord - include Accessor, Attachable, Assignee, Mentionable, Named, Notifiable, Role, Searcher, - SignalUser, Staff, Transferable, Conversational, AiQuota + include Accessor, Attachable, Assignee, Configurable, Mentionable, Named, Notifiable, Role, + Searcher, SignalUser, Staff, Transferable, Conversational, AiQuota include Timelined # Depends on Accessor has_one_attached :avatar @@ -15,8 +15,6 @@ class User < ApplicationRecord has_many :pins, dependent: :destroy has_many :pinned_cards, through: :pins, source: :card has_many :commands, dependent: :destroy - has_many :push_subscriptions, class_name: "Push::Subscription", dependent: :delete_all - has_many :period_activity_summaries, dependent: :destroy normalizes :email_address, with: ->(value) { value.strip.downcase } diff --git a/app/models/user/configurable.rb b/app/models/user/configurable.rb new file mode 100644 index 000000000..28e03a5e5 --- /dev/null +++ b/app/models/user/configurable.rb @@ -0,0 +1,10 @@ +module User::Configurable + extend ActiveSupport::Concern + + included do + has_one :notification_settings, class_name: "Notification::Settings", dependent: :destroy + has_many :push_subscriptions, class_name: "Push::Subscription", dependent: :delete_all + + after_create :create_notification_settings, unless: :system? + end +end diff --git a/db/migrate/20250828084732_create_notification_settings.rb b/db/migrate/20250828084732_create_notification_settings.rb new file mode 100644 index 000000000..cf414eb6e --- /dev/null +++ b/db/migrate/20250828084732_create_notification_settings.rb @@ -0,0 +1,12 @@ +class CreateNotificationSettings < ActiveRecord::Migration[8.1] + def change + create_table :notification_settings do |t| + t.references :user, null: false, foreign_key: true, index: true + t.integer :bundle_email_frequency, default: 0, null: false + + t.timestamps + + t.index %i[ user_id bundle_email_frequency ] + end + end +end diff --git a/db/schema.rb b/db/schema.rb index fa6dd112f..fdff4a4d2 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_08_27_072601) do +ActiveRecord::Schema[8.1].define(version: 2025_08_28_084732) do create_table "accesses", force: :cascade do |t| t.datetime "accessed_at" t.integer "collection_id", null: false @@ -319,6 +319,15 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_27_072601) do t.index ["user_id", "status"], name: "index_notification_bundles_on_user_id_and_status" end + create_table "notification_settings", force: :cascade do |t| + t.integer "bundle_email_frequency", default: 0, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.integer "user_id", null: false + t.index ["user_id", "bundle_email_frequency"], name: "idx_on_user_id_bundle_email_frequency_3ff75afaf4" + t.index ["user_id"], name: "index_notification_settings_on_user_id" + end + create_table "notifications", force: :cascade do |t| t.datetime "created_at", null: false t.integer "creator_id" @@ -477,6 +486,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_27_072601) do add_foreign_key "mentions", "users", column: "mentionee_id" add_foreign_key "mentions", "users", column: "mentioner_id" add_foreign_key "notification_bundles", "users" + add_foreign_key "notification_settings", "users" add_foreign_key "notifications", "users" add_foreign_key "notifications", "users", column: "creator_id" add_foreign_key "pins", "cards" diff --git a/db/schema_cache.yml b/db/schema_cache.yml index 5ba56a0c4..cfb084665 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -989,6 +989,21 @@ columns: comment: - *9 - *18 + notification_settings: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: bundle_email_frequency + cast_type: *3 + sql_type_metadata: *4 + 'null': false + default: 0 + default_function: + collation: + comment: + - *5 + - *6 + - *9 + - *18 notifications: - *5 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column @@ -1366,6 +1381,7 @@ primary_keys: filters_tags: mentions: id notification_bundles: id + notification_settings: id notifications: id pins: id push_subscriptions: id @@ -1416,6 +1432,7 @@ data_sources: filters_tags: true mentions: true notification_bundles: true + notification_settings: true notifications: true pins: true push_subscriptions: true @@ -2464,6 +2481,40 @@ indexes: nulls_not_distinct: comment: valid: true + notification_settings: + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: notification_settings + name: idx_on_user_id_bundle_email_frequency_3ff75afaf4 + unique: false + columns: + - user_id + - bundle_email_frequency + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: notification_settings + name: index_notification_settings_on_user_id + unique: false + columns: + - user_id + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true notifications: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: notifications @@ -2941,4 +2992,4 @@ indexes: comment: valid: true workflows: [] -version: 20250827072601 +version: 20250828084732 diff --git a/test/fixtures/notification/settings.yml b/test/fixtures/notification/settings.yml new file mode 100644 index 000000000..5042d9463 --- /dev/null +++ b/test/fixtures/notification/settings.yml @@ -0,0 +1,18 @@ +_fixture: + model_class: Notification::Settings + +david_settings: + user: david + bundle_email_frequency: never + +jz_settings: + user: jz + bundle_email_frequency: never + +kevin_settings: + user: kevin + bundle_email_frequency: never + +system_settings: + user: system + bundle_email_frequency: never diff --git a/test/models/user/configurable_test.rb b/test/models/user/configurable_test.rb new file mode 100644 index 000000000..b62ac3e18 --- /dev/null +++ b/test/models/user/configurable_test.rb @@ -0,0 +1,8 @@ +require "test_helper" + +class Notification::ConfigurableTest < ActiveSupport::TestCase + test "should create a notification settings for new users" do + user = User.create! name: "Some new user", email_address: "some.new@user.com" + assert user.notification_settings.present? + end +end From 53144a379c83ef4d43d3d18d383341fc15758db7 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 11:56:40 +0200 Subject: [PATCH 34/54] Add option to change the bundled email notification frequency --- .../notifications/settings_controller.rb | 16 +++ app/helpers/notifications_helper.rb | 9 ++ app/models/notification/settings.rb | 6 - app/models/user/configurable.rb | 4 +- app/models/user/settings.rb | 6 + app/views/filters/menu/_places.html.erb | 2 +- .../bundle_mailer/notification.html.erb | 2 +- .../bundle_mailer/notification.text.erb | 2 +- app/views/notifications/_tray.html.erb | 2 +- app/views/notifications/index.html.erb | 2 +- .../notifications/settings/_email.html.erb | 9 ++ .../settings/_push_notifications.html.erb | 26 +++++ .../notifications/settings/show.html.erb | 27 +---- config/routes.rb | 5 +- ..._notification_settings_to_user_settings.rb | 5 + db/schema.rb | 22 ++-- db/schema_cache.yml | 104 +++++++++--------- .../{notification => user}/settings.yml | 2 +- test/models/user/configurable_test.rb | 6 +- 19 files changed, 151 insertions(+), 106 deletions(-) delete mode 100644 app/models/notification/settings.rb create mode 100644 app/models/user/settings.rb create mode 100644 app/views/notifications/settings/_email.html.erb create mode 100644 app/views/notifications/settings/_push_notifications.html.erb create mode 100644 db/migrate/20250828092106_rename_notification_settings_to_user_settings.rb rename test/fixtures/{notification => user}/settings.yml (87%) diff --git a/app/controllers/notifications/settings_controller.rb b/app/controllers/notifications/settings_controller.rb index 5dbf1a867..42608b558 100644 --- a/app/controllers/notifications/settings_controller.rb +++ b/app/controllers/notifications/settings_controller.rb @@ -1,5 +1,21 @@ class Notifications::SettingsController < ApplicationController + before_action :set_settings + def show @collections = Current.user.collections.alphabetically end + + def update + @settings.update!(settings_params) + redirect_to notifications_settings_path, notice: "Settings updated" + end + + private + def set_settings + @settings = Current.user.settings + end + + def settings_params + params.expect(user_settings: :bundle_email_frequency) + end end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 020656380..45e01c2c5 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -65,6 +65,15 @@ module NotificationsHelper end end + def bundle_email_frequency_options_for(settings) + options_for_select([ + [ "Never", "never" ], + [ "Every few hours", "every_few_hours" ], + [ "Daily", "daily" ], + [ "Weekly", "weekly" ] + ], settings.bundle_email_frequency) + end + private def event_notification_action(event) if event.action.card_published? && event.eventable.assigned_to?(event.creator) diff --git a/app/models/notification/settings.rb b/app/models/notification/settings.rb deleted file mode 100644 index e74f35041..000000000 --- a/app/models/notification/settings.rb +++ /dev/null @@ -1,6 +0,0 @@ -class Notification::Settings < ApplicationRecord - belongs_to :user - - enum :bundle_email_frequency, %i[ never every_few_hours daily weekly ], - default: :never, default: :never, prefix: :bundle_email -end diff --git a/app/models/user/configurable.rb b/app/models/user/configurable.rb index 28e03a5e5..cec3b6670 100644 --- a/app/models/user/configurable.rb +++ b/app/models/user/configurable.rb @@ -2,9 +2,9 @@ module User::Configurable extend ActiveSupport::Concern included do - has_one :notification_settings, class_name: "Notification::Settings", dependent: :destroy + has_one :settings, class_name: "User::Settings", dependent: :destroy has_many :push_subscriptions, class_name: "Push::Subscription", dependent: :delete_all - after_create :create_notification_settings, unless: :system? + after_create :create_settings, unless: :system? end end diff --git a/app/models/user/settings.rb b/app/models/user/settings.rb new file mode 100644 index 000000000..eda952eee --- /dev/null +++ b/app/models/user/settings.rb @@ -0,0 +1,6 @@ +class User::Settings < ApplicationRecord + belongs_to :user + + enum :bundle_email_frequency, %i[ never every_few_hours daily weekly ], + default: :never, prefix: :bundle_email +end diff --git a/app/views/filters/menu/_places.html.erb b/app/views/filters/menu/_places.html.erb index f5a295207..b36b8b33e 100644 --- a/app/views/filters/menu/_places.html.erb +++ b/app/views/filters/menu/_places.html.erb @@ -4,7 +4,7 @@ <%= filter_place_menu_item account_settings_path, "Account Settings", "settings" %> <%= filter_place_menu_item user_path(Current.user), "My Profile", "person" %> <%= filter_place_menu_item notifications_path, "Notifications", "bell" %> - <%= filter_place_menu_item settings_notifications_path, "Notification Settings", "settings" %> + <%= filter_place_menu_item notifications_settings_path, "Notification Settings", "settings" %> <%= filter_place_menu_item search_path, "Search", "search" %> <%= filter_place_menu_item workflows_path, "Workflows", "bolt" %> diff --git a/app/views/mailers/notification/bundle_mailer/notification.html.erb b/app/views/mailers/notification/bundle_mailer/notification.html.erb index 31d2a3b52..8dcb6e731 100644 --- a/app/views/mailers/notification/bundle_mailer/notification.html.erb +++ b/app/views/mailers/notification/bundle_mailer/notification.html.erb @@ -4,7 +4,7 @@

You have <%= link_to pluralize(@notifications.count, "new notification"), notifications_url %>.

<%= render partial: "notification/bundle_mailer/notification", collection: @notifications, as: :notification %> diff --git a/app/views/mailers/notification/bundle_mailer/notification.text.erb b/app/views/mailers/notification/bundle_mailer/notification.text.erb index 05827cc67..d447d2f05 100644 --- a/app/views/mailers/notification/bundle_mailer/notification.text.erb +++ b/app/views/mailers/notification/bundle_mailer/notification.text.erb @@ -7,7 +7,7 @@ You have <%= pluralize @notifications.count, "new notification" %>. Fizzy emails you about new notifications every few hours. Change how often you get these: -<%= settings_notifications_url %> +<%= notifications_settings_url %> Unsubscribe from all email notifications: <%= root_url %> diff --git a/app/views/notifications/_tray.html.erb b/app/views/notifications/_tray.html.erb index 7c7845cfa..048730ff2 100644 --- a/app/views/notifications/_tray.html.erb +++ b/app/views/notifications/_tray.html.erb @@ -11,7 +11,7 @@
- <%= link_to settings_notifications_path, + <%= link_to notifications_settings_path, class: "btn borderless tray__notification-settings", title: "Notification settings", data: { action: "dialog#close" } do %> diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb index 4040ac977..1aef53625 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -10,7 +10,7 @@

<%= @page_title %>

- <%= link_to settings_notifications_path, class: "btn" do %> + <%= link_to notifications_settings_path, class: "btn" do %> <%= icon_tag "settings" %> Notification settings <% end %>
diff --git a/app/views/notifications/settings/_email.html.erb b/app/views/notifications/settings/_email.html.erb new file mode 100644 index 000000000..afed110d4 --- /dev/null +++ b/app/views/notifications/settings/_email.html.erb @@ -0,0 +1,9 @@ +
+

Email Settings

+ <%= form_with model: settings, url: notifications_settings_path, method: :patch, local: true, data: { controller: "form" } do |form| %> +
+ <%= form.label :bundle_email_frequency, "Receive an email with unread notifications:" %> + <%= form.select :bundle_email_frequency, bundle_email_frequency_options_for(settings), {}, class: "input input--select", data: { action: "change->form#submit" } %> +
+ <% end %> +
diff --git a/app/views/notifications/settings/_push_notifications.html.erb b/app/views/notifications/settings/_push_notifications.html.erb new file mode 100644 index 000000000..91dd801ab --- /dev/null +++ b/app/views/notifications/settings/_push_notifications.html.erb @@ -0,0 +1,26 @@ +
+

Push notifications are ON

+

Push notifications are OFF

+ + + +
+ + <%= icon_tag "lifebuoy" %> + Help me fix this + Not receiving notifications? + + +
+

When push notifications aren’t working, this can usually be fixed by checking your notification settings to make sure they’re allowed.

+ <%= render partial: "notifications/settings/browser" %> + <%= render partial: "notifications/settings/system" %> + <%= render partial: "notifications/settings/install" %> +
+
+
diff --git a/app/views/notifications/settings/show.html.erb b/app/views/notifications/settings/show.html.erb index 74dd03731..eadfc303b 100644 --- a/app/views/notifications/settings/show.html.erb +++ b/app/views/notifications/settings/show.html.erb @@ -12,32 +12,9 @@ <% end %> -
-

Push notifications are ON

-

Push notifications are OFF

+<%= render "notifications/settings/push_notifications" %> - - -
- - <%= icon_tag "lifebuoy" %> - Help me fix this - Not receiving notifications? - - -
-

When push notifications aren’t working, this can usually be fixed by checking your notification settings to make sure they’re allowed.

- <%= render partial: "notifications/settings/browser" %> - <%= render partial: "notifications/settings/system" %> - <%= render partial: "notifications/settings/install" %> -
-
-
+<%= render "notifications/settings/email", settings: @settings %>

Collections

diff --git a/config/routes.rb b/config/routes.rb index ac9ca1b75..dca643a9b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -54,10 +54,13 @@ Rails.application.routes.draw do end end + namespace :notifications do + resource :settings + end + resources :notifications do scope module: :notifications do get "tray", to: "trays#show", on: :collection - get "settings", to: "settings#show", on: :collection post "readings", to: "readings#create_all", on: :collection, as: :read_all post "reading", to: "readings#create", on: :member, as: :read diff --git a/db/migrate/20250828092106_rename_notification_settings_to_user_settings.rb b/db/migrate/20250828092106_rename_notification_settings_to_user_settings.rb new file mode 100644 index 000000000..42bd243ab --- /dev/null +++ b/db/migrate/20250828092106_rename_notification_settings_to_user_settings.rb @@ -0,0 +1,5 @@ +class RenameNotificationSettingsToUserSettings < ActiveRecord::Migration[8.1] + def change + rename_table :notification_settings, :user_settings + end +end diff --git a/db/schema.rb b/db/schema.rb index fdff4a4d2..afd2ae05d 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_08_28_084732) do +ActiveRecord::Schema[8.1].define(version: 2025_08_28_092106) do create_table "accesses", force: :cascade do |t| t.datetime "accessed_at" t.integer "collection_id", null: false @@ -319,15 +319,6 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_28_084732) do t.index ["user_id", "status"], name: "index_notification_bundles_on_user_id_and_status" end - create_table "notification_settings", force: :cascade do |t| - t.integer "bundle_email_frequency", default: 0, null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.integer "user_id", null: false - t.index ["user_id", "bundle_email_frequency"], name: "idx_on_user_id_bundle_email_frequency_3ff75afaf4" - t.index ["user_id"], name: "index_notification_settings_on_user_id" - end - create_table "notifications", force: :cascade do |t| t.datetime "created_at", null: false t.integer "creator_id" @@ -430,6 +421,15 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_28_084732) do t.datetime "updated_at", null: false end + create_table "user_settings", force: :cascade do |t| + t.integer "bundle_email_frequency", default: 0, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.integer "user_id", null: false + t.index ["user_id", "bundle_email_frequency"], name: "index_user_settings_on_user_id_and_bundle_email_frequency" + t.index ["user_id"], name: "index_user_settings_on_user_id" + end + create_table "users", force: :cascade do |t| t.boolean "active", default: true, null: false t.datetime "created_at", null: false @@ -486,7 +486,6 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_28_084732) do add_foreign_key "mentions", "users", column: "mentionee_id" add_foreign_key "mentions", "users", column: "mentioner_id" add_foreign_key "notification_bundles", "users" - add_foreign_key "notification_settings", "users" add_foreign_key "notifications", "users" add_foreign_key "notifications", "users", column: "creator_id" add_foreign_key "pins", "cards" @@ -497,6 +496,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_28_084732) do add_foreign_key "steps", "cards" add_foreign_key "taggings", "cards" add_foreign_key "taggings", "tags" + add_foreign_key "user_settings", "users" add_foreign_key "watches", "cards" add_foreign_key "watches", "users" add_foreign_key "workflow_stages", "workflows" diff --git a/db/schema_cache.yml b/db/schema_cache.yml index cfb084665..77d7d292a 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -989,21 +989,6 @@ columns: comment: - *9 - *18 - notification_settings: - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: bundle_email_frequency - cast_type: *3 - sql_type_metadata: *4 - 'null': false - default: 0 - default_function: - collation: - comment: - - *5 - - *6 - - *9 - - *18 notifications: - *5 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column @@ -1246,6 +1231,21 @@ columns: - *6 - *35 - *9 + user_settings: + - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column + auto_increment: + name: bundle_email_frequency + cast_type: *3 + sql_type_metadata: *4 + 'null': false + default: 0 + default_function: + collation: + comment: + - *5 + - *6 + - *9 + - *18 users: - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: @@ -1381,7 +1381,6 @@ primary_keys: filters_tags: mentions: id notification_bundles: id - notification_settings: id notifications: id pins: id push_subscriptions: id @@ -1394,6 +1393,7 @@ primary_keys: steps: id taggings: id tags: id + user_settings: id users: id watches: id workflow_stages: id @@ -1432,7 +1432,6 @@ data_sources: filters_tags: true mentions: true notification_bundles: true - notification_settings: true notifications: true pins: true push_subscriptions: true @@ -1445,6 +1444,7 @@ data_sources: steps: true taggings: true tags: true + user_settings: true users: true watches: true workflow_stages: true @@ -2481,40 +2481,6 @@ indexes: nulls_not_distinct: comment: valid: true - notification_settings: - - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: notification_settings - name: idx_on_user_id_bundle_email_frequency_3ff75afaf4 - unique: false - columns: - - user_id - - bundle_email_frequency - lengths: {} - orders: {} - opclasses: {} - where: - type: - using: - include: - nulls_not_distinct: - comment: - valid: true - - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition - table: notification_settings - name: index_notification_settings_on_user_id - unique: false - columns: - - user_id - lengths: {} - orders: {} - opclasses: {} - where: - type: - using: - include: - nulls_not_distinct: - comment: - valid: true notifications: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: notifications @@ -2892,6 +2858,40 @@ indexes: comment: valid: true tags: [] + user_settings: + - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition + table: user_settings + name: index_user_settings_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: user_settings + name: index_user_settings_on_user_id_and_bundle_email_frequency + unique: false + columns: + - user_id + - bundle_email_frequency + lengths: {} + orders: {} + opclasses: {} + where: + type: + using: + include: + nulls_not_distinct: + comment: + valid: true users: - !ruby/object:ActiveRecord::ConnectionAdapters::IndexDefinition table: users @@ -2992,4 +2992,4 @@ indexes: comment: valid: true workflows: [] -version: 20250828084732 +version: 20250828092106 diff --git a/test/fixtures/notification/settings.yml b/test/fixtures/user/settings.yml similarity index 87% rename from test/fixtures/notification/settings.yml rename to test/fixtures/user/settings.yml index 5042d9463..29765f196 100644 --- a/test/fixtures/notification/settings.yml +++ b/test/fixtures/user/settings.yml @@ -1,5 +1,5 @@ _fixture: - model_class: Notification::Settings + model_class: User::Settings david_settings: user: david diff --git a/test/models/user/configurable_test.rb b/test/models/user/configurable_test.rb index b62ac3e18..dbc353936 100644 --- a/test/models/user/configurable_test.rb +++ b/test/models/user/configurable_test.rb @@ -1,8 +1,8 @@ require "test_helper" -class Notification::ConfigurableTest < ActiveSupport::TestCase - test "should create a notification settings for new users" do +class User::ConfigurableTest < ActiveSupport::TestCase + test "should create settings for new users" do user = User.create! name: "Some new user", email_address: "some.new@user.com" - assert user.notification_settings.present? + assert user.settings.present? end end From fb95c8aa92a440bedea2d59fc074d1fecd125982 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 12:02:12 +0200 Subject: [PATCH 35/54] Copy --- app/views/notifications/settings/_email.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/notifications/settings/_email.html.erb b/app/views/notifications/settings/_email.html.erb index afed110d4..b7e5e94a1 100644 --- a/app/views/notifications/settings/_email.html.erb +++ b/app/views/notifications/settings/_email.html.erb @@ -1,5 +1,5 @@
-

Email Settings

+

Email

<%= form_with model: settings, url: notifications_settings_path, method: :patch, local: true, data: { controller: "form" } do |form| %>
<%= form.label :bundle_email_frequency, "Receive an email with unread notifications:" %> From 2adb4571e1b0376878749ccf30525603379f9675 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 12:03:49 +0200 Subject: [PATCH 36/54] Copy --- app/helpers/notifications_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 45e01c2c5..b97027d9b 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -69,8 +69,8 @@ module NotificationsHelper options_for_select([ [ "Never", "never" ], [ "Every few hours", "every_few_hours" ], - [ "Daily", "daily" ], - [ "Weekly", "weekly" ] + [ "Every day", "daily" ], + [ "Every week", "weekly" ] ], settings.bundle_email_frequency) end From c1c359f03063cec4d10d6c21052f6fa5b1e2a68d Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 12:29:40 +0200 Subject: [PATCH 37/54] Review existing bundles when changing the pending bundles --- app/models/notification/bundle.rb | 4 +-- app/models/user/settings.rb | 36 +++++++++++++++++++++++++ test/models/notification/bundle_test.rb | 6 ++--- test/models/user/settings_test.rb | 36 +++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 test/models/user/settings_test.rb diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index 78d4b5dd8..77f8d73ec 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -48,11 +48,9 @@ class Notification::Bundle < ApplicationRecord end private - AGGREGATION_PERIOD = 4.hours - def set_default_window self.starts_at ||= Time.current - self.ends_at ||= AGGREGATION_PERIOD.from_now + self.ends_at ||= user.settings.bundle_aggregation_period.from_now end def window diff --git a/app/models/user/settings.rb b/app/models/user/settings.rb index eda952eee..3de7d8a79 100644 --- a/app/models/user/settings.rb +++ b/app/models/user/settings.rb @@ -3,4 +3,40 @@ class User::Settings < ApplicationRecord enum :bundle_email_frequency, %i[ never every_few_hours daily weekly ], default: :never, prefix: :bundle_email + + after_update :review_pending_bundles, if: :saved_change_to_bundle_email_frequency? + + def bundle_aggregation_period + case bundle_email_frequency + when "every_few_hours" + 4.hours + when "daily" + 1.day + when "weekly" + 1.week + else + 1.day + end + end + + private + def review_pending_bundles + if bundle_email_never? + cancel_pending_bundles + else + reschedule_pending_bundles + end + end + + def cancel_pending_bundles + user.notification_bundles.pending.find_each do |bundle| + bundle.destroy + end + end + + def reschedule_pending_bundles + user.notification_bundles.pending.find_each do |bundle| + bundle.deliver_later + end + end end diff --git a/test/models/notification/bundle_test.rb b/test/models/notification/bundle_test.rb index 5da167f64..a84627b3e 100644 --- a/test/models/notification/bundle_test.rb +++ b/test/models/notification/bundle_test.rb @@ -23,7 +23,7 @@ class Notification::BundleTest < ActiveSupport::TestCase notification_2 = assert_no_difference -> { @user.notification_bundles.count } do @user.notifications.create!(source: events(:logo_published), creator: @user) end - travel_to 3.hours.from_now + travel_to 3.days.from_now notification_3 = assert_difference -> { @user.notification_bundles.pending.count }, 1 do @user.notifications.create!(source: events(:logo_published), creator: @user) @@ -84,7 +84,7 @@ class Notification::BundleTest < ActiveSupport::TestCase bundle.update!(ends_at: 1.minute.ago) - perform_enqueued_jobs do + perform_enqueued_jobs only: Notification::Bundle::DeliverJob do Notification::Bundle.deliver_all end @@ -98,7 +98,7 @@ class Notification::BundleTest < ActiveSupport::TestCase bundle.update!(ends_at: 1.minute.from_now) - perform_enqueued_jobs do + perform_enqueued_jobs only: Notification::Bundle::DeliverJob do Notification::Bundle.deliver_all end diff --git a/test/models/user/settings_test.rb b/test/models/user/settings_test.rb new file mode 100644 index 000000000..a508325df --- /dev/null +++ b/test/models/user/settings_test.rb @@ -0,0 +1,36 @@ +require "test_helper" + +class User::SettingsTest < ActiveSupport::TestCase + setup do + @user = users(:david) + @settings = @user.settings + end + + test "changing the bundle email frequency to never will cancel pending bundles" do + @settings.update!(bundle_email_frequency: :every_few_hours) + bundle = @user.notification_bundles.create! + @settings.update!(bundle_email_frequency: :never) + assert_nil Notification::Bundle.find_by(id: bundle.id) + end + + test "changing the bundle email frequency will deliver pending bundles" do + bundle = @user.notification_bundles.create! + assert bundle.pending? + + perform_enqueued_jobs only: Notification::Bundle::DeliverJob do + @settings.update!(bundle_email_frequency: :daily) + end + + assert bundle.reload.delivered? + end + + test "changing other settings will not affect pending bundles" do + bundle = @user.notification_bundles.create! + + perform_enqueued_jobs only: Notification::Bundle::DeliverJob do + @settings.update!(updated_at: 1.hour.from_now) + end + + assert bundle.reload.pending? + end +end From 960f074b3f77a1b2ea293ee16eecf2e1516a410a Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 12:53:19 +0200 Subject: [PATCH 38/54] Don't bundle new notifications is bundling is disabled --- app/models/notification.rb | 2 +- app/models/user/settings.rb | 10 +++++++--- test/models/notification/bundle_test.rb | 9 +++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/models/notification.rb b/app/models/notification.rb index 0b1e2e377..3f5a00aac 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -43,6 +43,6 @@ class Notification < ApplicationRecord end def bundle - user.bundle(self) + user.bundle(self) if user.settings.bundling_emails? end end diff --git a/app/models/user/settings.rb b/app/models/user/settings.rb index 3de7d8a79..57206cd26 100644 --- a/app/models/user/settings.rb +++ b/app/models/user/settings.rb @@ -19,12 +19,16 @@ class User::Settings < ApplicationRecord end end + def bundling_emails? + !bundle_email_never? + end + private def review_pending_bundles - if bundle_email_never? - cancel_pending_bundles - else + if bundling_emails? reschedule_pending_bundles + else + cancel_pending_bundles end end diff --git a/test/models/notification/bundle_test.rb b/test/models/notification/bundle_test.rb index a84627b3e..9d7848db4 100644 --- a/test/models/notification/bundle_test.rb +++ b/test/models/notification/bundle_test.rb @@ -3,6 +3,7 @@ require "test_helper" class Notification::BundleTest < ActiveSupport::TestCase setup do @user = users(:david) + @user.settings.bundle_email_every_few_hours! end test "new notifications are bundled" do @@ -14,6 +15,14 @@ class Notification::BundleTest < ActiveSupport::TestCase assert_includes bundle.notifications, notification end + test "don't bundle new notifications if bundling is disabled" do + @user.settings.bundle_email_never! + + assert_no_difference -> { @user.notification_bundles.count } do + @user.notifications.create!(source: events(:logo_published), creator: @user) + end + end + test "notifications are bundled withing the aggregation period" do notification_1 = assert_difference -> { @user.notification_bundles.pending.count }, 1 do @user.notifications.create!(source: events(:logo_published), creator: @user) From 243779dbcf4d48816ae30eef8a8488737caf8ac5 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 13:07:27 +0200 Subject: [PATCH 39/54] Copy --- app/views/notifications/settings/_email.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/notifications/settings/_email.html.erb b/app/views/notifications/settings/_email.html.erb index b7e5e94a1..592f4746b 100644 --- a/app/views/notifications/settings/_email.html.erb +++ b/app/views/notifications/settings/_email.html.erb @@ -2,7 +2,7 @@

Email

<%= form_with model: settings, url: notifications_settings_path, method: :patch, local: true, data: { controller: "form" } do |form| %>
- <%= form.label :bundle_email_frequency, "Receive an email with unread notifications:" %> + <%= form.label :bundle_email_frequency, "Receive an email with new notifications:" %> <%= form.select :bundle_email_frequency, bundle_email_frequency_options_for(settings), {}, class: "input input--select", data: { action: "change->form#submit" } %>
<% end %> From e4ab1366bea544fa7453d23a91e2d8ae6a88dbd0 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 16:58:51 +0200 Subject: [PATCH 40/54] Add unsubscribe links and headers for emails https://3.basecamp.com/2914079/buckets/37331921/todos/9002504172 --- .../notifications/unsubscribes_controller.rb | 24 +++++++++++ .../concerns/mailers/unsubscribable.rb | 12 ++++++ app/mailers/notification/bundle_mailer.rb | 4 ++ app/models/user/notifiable.rb | 2 + .../bundle_mailer/notification.html.erb | 2 +- .../bundle_mailer/notification.text.erb | 2 +- .../notifications/unsubscribes/new.html.erb | 7 ++++ .../notifications/unsubscribes/show.html.erb | 3 ++ config/routes.rb | 1 + .../unsubscribes_controller_test.rb | 40 +++++++++++++++++++ 10 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 app/controllers/notifications/unsubscribes_controller.rb create mode 100644 app/mailers/concerns/mailers/unsubscribable.rb create mode 100644 app/views/notifications/unsubscribes/new.html.erb create mode 100644 app/views/notifications/unsubscribes/show.html.erb create mode 100644 test/controllers/notifications/unsubscribes_controller_test.rb diff --git a/app/controllers/notifications/unsubscribes_controller.rb b/app/controllers/notifications/unsubscribes_controller.rb new file mode 100644 index 000000000..f7486d91e --- /dev/null +++ b/app/controllers/notifications/unsubscribes_controller.rb @@ -0,0 +1,24 @@ +class Notifications::UnsubscribesController < ApplicationController + allow_unauthenticated_access + skip_before_action :verify_authenticity_token + + before_action :set_user + + def new + end + + def create + @user.settings.bundle_email_never! + redirect_to notifications_unsubscribe_path(access_token: params[:access_token]) + end + + def show + end + + private + def set_user + unless @user = User.find_by_token_for(:unsubscribe, params[:access_token]) + redirect_to root_path, alert: "Invalid unsubscribe link" + end + end +end diff --git a/app/mailers/concerns/mailers/unsubscribable.rb b/app/mailers/concerns/mailers/unsubscribable.rb new file mode 100644 index 000000000..fb9278df7 --- /dev/null +++ b/app/mailers/concerns/mailers/unsubscribable.rb @@ -0,0 +1,12 @@ +module Mailers::Unsubscribable + extend ActiveSupport::Concern + + included do + after_action :set_unsubscribe_headers + end + + def set_unsubscribe_headers + headers["List-Unsubscribe-Post"] = "List-Unsubscribe=One-Click" + headers["List-Unsubscribe"] = "<#{notifications_unsubscribe_url(access_token: @unsubscribe_token)}>" + end +end diff --git a/app/mailers/notification/bundle_mailer.rb b/app/mailers/notification/bundle_mailer.rb index 683c83ec8..06bce5aa0 100644 --- a/app/mailers/notification/bundle_mailer.rb +++ b/app/mailers/notification/bundle_mailer.rb @@ -1,9 +1,13 @@ class Notification::BundleMailer < ApplicationMailer + include Mailers::Unsubscribable + helper NotificationsHelper def notification(bundle) + @user = bundle.user @bundle = bundle @notifications = bundle.notifications + @unsubscribe_token = @user.generate_token_for(:unsubscribe) mail \ to: bundle.user.email_address, diff --git a/app/models/user/notifiable.rb b/app/models/user/notifiable.rb index 015186d15..b0e4ffcad 100644 --- a/app/models/user/notifiable.rb +++ b/app/models/user/notifiable.rb @@ -4,6 +4,8 @@ module User::Notifiable included do has_many :notifications, dependent: :destroy has_many :notification_bundles, class_name: "Notification::Bundle", dependent: :destroy + + generates_token_for :unsubscribe, expires_in: 1.month end def bundle(notification) diff --git a/app/views/mailers/notification/bundle_mailer/notification.html.erb b/app/views/mailers/notification/bundle_mailer/notification.html.erb index 8dcb6e731..f051cf426 100644 --- a/app/views/mailers/notification/bundle_mailer/notification.html.erb +++ b/app/views/mailers/notification/bundle_mailer/notification.html.erb @@ -5,7 +5,7 @@ <%= render partial: "notification/bundle_mailer/notification", collection: @notifications, as: :notification %> diff --git a/app/views/mailers/notification/bundle_mailer/notification.text.erb b/app/views/mailers/notification/bundle_mailer/notification.text.erb index d447d2f05..0149767e6 100644 --- a/app/views/mailers/notification/bundle_mailer/notification.text.erb +++ b/app/views/mailers/notification/bundle_mailer/notification.text.erb @@ -10,4 +10,4 @@ Change how often you get these: <%= notifications_settings_url %> Unsubscribe from all email notifications: -<%= root_url %> +<%= new_notifications_unsubscribe_url(access_token: @unsubscribe_token) %> diff --git a/app/views/notifications/unsubscribes/new.html.erb b/app/views/notifications/unsubscribes/new.html.erb new file mode 100644 index 000000000..7baaf8e9a --- /dev/null +++ b/app/views/notifications/unsubscribes/new.html.erb @@ -0,0 +1,7 @@ +

Unsubscribing from all email notifications as <%= @user.name %>…

+ +
+ <%= auto_submit_form_with model: @user, url: notifications_unsubscribe_path(access_token: params[:access_token]), method: :post do |form| %> + <%= form.submit "Unsubscribe now", class: "btn" %> + <% end %> +
diff --git a/app/views/notifications/unsubscribes/show.html.erb b/app/views/notifications/unsubscribes/show.html.erb new file mode 100644 index 000000000..3610e92e2 --- /dev/null +++ b/app/views/notifications/unsubscribes/show.html.erb @@ -0,0 +1,3 @@ +

Unsubscribed

+

Thank you <%= @user.name %>, you will no longer receive bundled email notifications.

+

You can <%= link_to "re-enable notifications here", notifications_settings_path %>.

diff --git a/config/routes.rb b/config/routes.rb index dca643a9b..5392868c9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -56,6 +56,7 @@ Rails.application.routes.draw do namespace :notifications do resource :settings + resource :unsubscribe end resources :notifications do diff --git a/test/controllers/notifications/unsubscribes_controller_test.rb b/test/controllers/notifications/unsubscribes_controller_test.rb new file mode 100644 index 000000000..08998d6c6 --- /dev/null +++ b/test/controllers/notifications/unsubscribes_controller_test.rb @@ -0,0 +1,40 @@ +require "test_helper" + +class Notifications::UnsubscribesControllerTest < ActionDispatch::IntegrationTest + setup do + @user = users(:david) + @access_token = @user.generate_token_for(:unsubscribe) + + sign_in_as @user + end + + test "new" do + get new_notifications_unsubscribe_path(access_token: @access_token) + assert_response :success + end + + test "new with bad token" do + get new_notifications_unsubscribe_path(access_token: "bad") + assert_redirected_to root_path + end + + test "create" do + @user.reload.settings.bundle_email_every_few_hours! + assert_changes -> { @user.reload.settings.bundle_email_frequency }, to: "never" do + post notifications_unsubscribe_path(access_token: @access_token) + assert_redirected_to notifications_unsubscribe_path(access_token: @access_token) + end + end + + test "create with bad token" do + assert_no_changes -> { @user.reload.settings.bundle_email_frequency } do + post notifications_unsubscribe_path(access_token: "bad") + assert_redirected_to root_path + end + end + + test "show" do + get notifications_unsubscribe_path(access_token: @access_token) + assert_response :success + end +end From 5c890bbc347f3d070ab6c0c99d77fef996eb6461 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 17:08:22 +0200 Subject: [PATCH 41/54] Bundle every few hours by default --- app/models/user/settings.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user/settings.rb b/app/models/user/settings.rb index 57206cd26..0602220bd 100644 --- a/app/models/user/settings.rb +++ b/app/models/user/settings.rb @@ -2,7 +2,7 @@ class User::Settings < ApplicationRecord belongs_to :user enum :bundle_email_frequency, %i[ never every_few_hours daily weekly ], - default: :never, prefix: :bundle_email + default: :every_few_hours, prefix: :bundle_email after_update :review_pending_bundles, if: :saved_change_to_bundle_email_frequency? From 9ba9a5758b7fd2cc00e20dfcf4993878c2307440 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 17:21:15 +0200 Subject: [PATCH 42/54] Remove hosts for now Docker subnets causing trouble and this is not really part of the email change --- config/environments/beta.rb | 2 -- config/environments/production.rb | 7 ------- config/environments/staging.rb | 2 -- 3 files changed, 11 deletions(-) diff --git a/config/environments/beta.rb b/config/environments/beta.rb index c67135679..41220cbc9 100644 --- a/config/environments/beta.rb +++ b/config/environments/beta.rb @@ -1,8 +1,6 @@ require_relative "production" Rails.application.configure do - config.hosts = [ "fizzy-beta.37signals.com" ] + config.hosts[1..] - config.action_mailer.smtp_settings[:domain] = config.hosts.first config.action_mailer.smtp_settings[:address] = "smtp-outbound-staging" config.action_mailer.default_url_options = { host: config.hosts.first, protocol: "https" } diff --git a/config/environments/production.rb b/config/environments/production.rb index 664c60834..d6b6cc7d9 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -73,13 +73,6 @@ Rails.application.configure do # Set this to true and configure the email server for immediate delivery to raise delivery errors. # config.action_mailer.raise_delivery_errors = false - # Enable DNS rebinding protection and other `Host` header attacks. - config.hosts = [ - "fizzy.37signals.com", - "localhost", - IPAddr.new("172.17.0.0/20") # Docker VPC - ] - # Set host to be used by links generated in mailer templates. config.action_mailer.default_url_options = { host: config.hosts.first, protocol: "https" } diff --git a/config/environments/staging.rb b/config/environments/staging.rb index 7bee9c845..4d96505f1 100644 --- a/config/environments/staging.rb +++ b/config/environments/staging.rb @@ -1,8 +1,6 @@ require_relative "production" Rails.application.configure do - config.hosts = [ "fizzy.37signals-staging.com" ] + config.hosts[1..] - config.action_mailer.smtp_settings[:domain] = config.hosts.first config.action_mailer.smtp_settings[:address] = "smtp-outbound-staging" config.action_mailer.default_url_options = { host: config.hosts.first, protocol: "https" } From 5fe66dd6029a2a33e14cfcd6d0977319c8be1442 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 17:37:35 +0200 Subject: [PATCH 43/54] Fix job class --- app/models/notification/bundle.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb index 77f8d73ec..5fab5fe7e 100644 --- a/app/models/notification/bundle.rb +++ b/app/models/notification/bundle.rb @@ -38,7 +38,7 @@ class Notification::Bundle < ApplicationRecord def deliver processing! - BundleMailer.notification(self).deliver if notifications.any? + Notification::BundleMailer.notification(self).deliver if notifications.any? delivered! end From 02c538bbb78d8f426fc664728bc3cf9d31400039 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 17:38:52 +0200 Subject: [PATCH 44/54] Run for each tenant --- app/jobs/notification/bundle/deliver_all_job.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/jobs/notification/bundle/deliver_all_job.rb b/app/jobs/notification/bundle/deliver_all_job.rb index cdd6093ce..d2df254f8 100644 --- a/app/jobs/notification/bundle/deliver_all_job.rb +++ b/app/jobs/notification/bundle/deliver_all_job.rb @@ -1,5 +1,7 @@ class Notification::Bundle::DeliverAllJob < ApplicationJob def perform - Notification::Bundle.deliver_all + ApplicationRecord.with_each_tenant do |tenant| + Notification::Bundle.deliver_all + end end end From 4427524e539855c0eed011acd19fc5f35d7daeba Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 17:55:12 +0200 Subject: [PATCH 45/54] We do need the host! --- config/environments/beta.rb | 4 ++-- config/environments/production.rb | 4 ++-- config/environments/staging.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/config/environments/beta.rb b/config/environments/beta.rb index 41220cbc9..650f1b084 100644 --- a/config/environments/beta.rb +++ b/config/environments/beta.rb @@ -1,9 +1,9 @@ require_relative "production" Rails.application.configure do - config.action_mailer.smtp_settings[:domain] = config.hosts.first + config.action_mailer.smtp_settings[:domain] = "fizzy-beta.37signals.com" config.action_mailer.smtp_settings[:address] = "smtp-outbound-staging" - config.action_mailer.default_url_options = { host: config.hosts.first, protocol: "https" } + config.action_mailer.default_url_options = { host: "fizzy-beta.37signals.com", protocol: "https" } # Let's keep beta on local disk. See https://github.com/basecamp/fizzy/pull/557 for context. config.active_storage.service = :local diff --git a/config/environments/production.rb b/config/environments/production.rb index d6b6cc7d9..57994db43 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -74,9 +74,9 @@ Rails.application.configure do # config.action_mailer.raise_delivery_errors = false # Set host to be used by links generated in mailer templates. - config.action_mailer.default_url_options = { host: config.hosts.first, protocol: "https" } + config.action_mailer.default_url_options = { host: "fizzy.37signals.com", protocol: "https" } - config.action_mailer.smtp_settings = { domain: config.hosts.first, address: "smtp-outbound", port: 25, enable_starttls_auto: false } + config.action_mailer.smtp_settings = { domain: "fizzy.37signals.com", address: "smtp-outbound", port: 25, enable_starttls_auto: false } # Enable locale fallbacks for I18n (makes lookups for any locale fall back to # the I18n.default_locale when a translation cannot be found). diff --git a/config/environments/staging.rb b/config/environments/staging.rb index 4d96505f1..14bcfdc57 100644 --- a/config/environments/staging.rb +++ b/config/environments/staging.rb @@ -1,7 +1,7 @@ require_relative "production" Rails.application.configure do - config.action_mailer.smtp_settings[:domain] = config.hosts.first + config.action_mailer.smtp_settings[:domain] = "fizzy.37signals-staging.com" config.action_mailer.smtp_settings[:address] = "smtp-outbound-staging" - config.action_mailer.default_url_options = { host: config.hosts.first, protocol: "https" } + config.action_mailer.default_url_options = { host: "fizzy.37signals-staging.com", protocol: "https" } end From c2fe32cdfc8a0806beb926fdeae5e8288c756323 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 17:59:13 +0200 Subject: [PATCH 46/54] Fix email --- app/mailers/application_mailer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 1ee79fb87..700673bc5 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,5 +1,5 @@ class ApplicationMailer < ActionMailer::Base - default from: "The Fizzy team " + default from: "The Fizzy team " layout "mailer" append_view_path Rails.root.join("app/views/mailers") From 8f1a6852fa3c76f2a8c0a823ffd773bf99bcbf59 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Thu, 28 Aug 2025 18:09:13 +0200 Subject: [PATCH 47/54] Preferred form of passing the tenant slug --- app/mailers/application_mailer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 700673bc5..562bc0e3c 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -7,6 +7,6 @@ class ApplicationMailer < ActionMailer::Base private def default_url_options - super.merge(script_name: "/#{ApplicationRecord.current_tenant}") + super.merge(script_name: Account.sole.slug) end end From 0964d019b7329b20e8f1704e7d2f5c03f1fb096a Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Thu, 28 Aug 2025 21:54:37 -0500 Subject: [PATCH 48/54] Style and copy --- app/views/notifications/unsubscribes/show.html.erb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/views/notifications/unsubscribes/show.html.erb b/app/views/notifications/unsubscribes/show.html.erb index 3610e92e2..05b2d647c 100644 --- a/app/views/notifications/unsubscribes/show.html.erb +++ b/app/views/notifications/unsubscribes/show.html.erb @@ -1,3 +1,9 @@ -

Unsubscribed

-

Thank you <%= @user.name %>, you will no longer receive bundled email notifications.

-

You can <%= link_to "re-enable notifications here", notifications_settings_path %>.

+
+

Unsubscribed

+

Thanks, <%= @user.first_name %>! Fizzy won't send you any more email notifications. If you change your mind you can turn them back on in <%= link_to "Notifications Settings", notifications_settings_path %>.

+ + <%= link_to root_path, class: "btn margin-block-start" do %> + <%= icon_tag "arrow-left" %> + Back to Fizzy + <% end %> +
From ca488d4a730310a1fd7cc05c7f3095619230f343 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Thu, 28 Aug 2025 21:55:35 -0500 Subject: [PATCH 49/54] A little more style --- app/views/notifications/unsubscribes/show.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/notifications/unsubscribes/show.html.erb b/app/views/notifications/unsubscribes/show.html.erb index 05b2d647c..fbdaa76e7 100644 --- a/app/views/notifications/unsubscribes/show.html.erb +++ b/app/views/notifications/unsubscribes/show.html.erb @@ -1,8 +1,8 @@ -
+

Unsubscribed

Thanks, <%= @user.first_name %>! Fizzy won't send you any more email notifications. If you change your mind you can turn them back on in <%= link_to "Notifications Settings", notifications_settings_path %>.

- <%= link_to root_path, class: "btn margin-block-start" do %> + <%= link_to root_path, class: "btn btn--link margin-block-start" do %> <%= icon_tag "arrow-left" %> Back to Fizzy <% end %> From 1179fc2a33035b61074abba1bfa705d57fe3c076 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Thu, 28 Aug 2025 22:00:15 -0500 Subject: [PATCH 50/54] A few more tweaks --- app/views/notifications/unsubscribes/show.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/notifications/unsubscribes/show.html.erb b/app/views/notifications/unsubscribes/show.html.erb index fbdaa76e7..3e100b14d 100644 --- a/app/views/notifications/unsubscribes/show.html.erb +++ b/app/views/notifications/unsubscribes/show.html.erb @@ -1,6 +1,6 @@
-

Unsubscribed

-

Thanks, <%= @user.first_name %>! Fizzy won't send you any more email notifications. If you change your mind you can turn them back on in <%= link_to "Notifications Settings", notifications_settings_path %>.

+

You’re unsubscribed

+

Thanks, <%= @user.first_name %>! Fizzy won’t send you any more email notifications. If you change your mind you can turn them back on in <%= link_to "Notifications Settings", notifications_settings_path %>.

<%= link_to root_path, class: "btn btn--link margin-block-start" do %> <%= icon_tag "arrow-left" %> From af9f28d0bb7fdcab933a4f9ca42d59ad67a38358 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Thu, 28 Aug 2025 22:18:11 -0500 Subject: [PATCH 51/54] Two panel design for notifications settings, design and copy for email block --- app/views/notifications/settings/_email.html.erb | 15 +++++++++------ .../settings/_push_notifications.html.erb | 2 +- app/views/notifications/settings/show.html.erb | 15 +++++++++------ 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/app/views/notifications/settings/_email.html.erb b/app/views/notifications/settings/_email.html.erb index 592f4746b..a51aea099 100644 --- a/app/views/notifications/settings/_email.html.erb +++ b/app/views/notifications/settings/_email.html.erb @@ -1,9 +1,12 @@ -
-

Email

- <%= form_with model: settings, url: notifications_settings_path, method: :patch, local: true, data: { controller: "form" } do |form| %> -
- <%= form.label :bundle_email_frequency, "Receive an email with new notifications:" %> - <%= form.select :bundle_email_frequency, bundle_email_frequency_options_for(settings), {}, class: "input input--select", data: { action: "change->form#submit" } %> +
+

Email Notifications

+

Get a single email with all your notifications every few hours, daily, or weekly.

+ <%= form_with model: settings, url: notifications_settings_path, + class: "flex flex-column gap-half", + method: :patch, local: true, data: { controller: "form" } do |form| %> +
+ <%= form.label :bundle_email_frequency, "Email me about new notifications..." %> + <%= form.select :bundle_email_frequency, bundle_email_frequency_options_for(settings), {}, class: "input input--select txt-align-center", data: { action: "change->form#submit" } %>
<% end %>
diff --git a/app/views/notifications/settings/_push_notifications.html.erb b/app/views/notifications/settings/_push_notifications.html.erb index 91dd801ab..047f57a15 100644 --- a/app/views/notifications/settings/_push_notifications.html.erb +++ b/app/views/notifications/settings/_push_notifications.html.erb @@ -1,4 +1,4 @@ -
+

Push notifications are ON

Push notifications are OFF

diff --git a/app/views/notifications/settings/show.html.erb b/app/views/notifications/settings/show.html.erb index eadfc303b..2eb10862c 100644 --- a/app/views/notifications/settings/show.html.erb +++ b/app/views/notifications/settings/show.html.erb @@ -12,11 +12,14 @@ <% end %> -<%= render "notifications/settings/push_notifications" %> +
+
+

Collections

+ <%= render partial: "notifications/settings/collection", collection: @collections, locals: { user: Current.user } %> +
-<%= render "notifications/settings/email", settings: @settings %> - -
-

Collections

- <%= render partial: "notifications/settings/collection", collection: @collections, locals: { user: Current.user } %> +
+ <%= render "notifications/settings/push_notifications" %> + <%= render "notifications/settings/email", settings: @settings %> +
From bec4abad0c0ddb90763d9364f5aef1c41c5aaf79 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Fri, 29 Aug 2025 09:28:32 +0200 Subject: [PATCH 52/54] Rename method to match semantics "rescheduling" was a from an implementation I had dropped --- app/models/user/settings.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/user/settings.rb b/app/models/user/settings.rb index 0602220bd..9db9edd24 100644 --- a/app/models/user/settings.rb +++ b/app/models/user/settings.rb @@ -26,7 +26,7 @@ class User::Settings < ApplicationRecord private def review_pending_bundles if bundling_emails? - reschedule_pending_bundles + flush_pending_bundles else cancel_pending_bundles end @@ -38,7 +38,7 @@ class User::Settings < ApplicationRecord end end - def reschedule_pending_bundles + def flush_pending_bundles user.notification_bundles.pending.find_each do |bundle| bundle.deliver_later end From a9444caf1f0b47004815ff3bc6af30311b173f7a Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Fri, 29 Aug 2025 09:39:48 +0200 Subject: [PATCH 53/54] Add a "backend" queue with less priorities for deliveries --- app/jobs/notification/bundle/deliver_all_job.rb | 2 ++ app/jobs/notification/bundle/deliver_job.rb | 2 ++ app/jobs/push_notification_job.rb | 2 -- config/queue.yml | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/jobs/notification/bundle/deliver_all_job.rb b/app/jobs/notification/bundle/deliver_all_job.rb index d2df254f8..250b53b6e 100644 --- a/app/jobs/notification/bundle/deliver_all_job.rb +++ b/app/jobs/notification/bundle/deliver_all_job.rb @@ -1,4 +1,6 @@ class Notification::Bundle::DeliverAllJob < ApplicationJob + queue_as :backend + def perform ApplicationRecord.with_each_tenant do |tenant| Notification::Bundle.deliver_all diff --git a/app/jobs/notification/bundle/deliver_job.rb b/app/jobs/notification/bundle/deliver_job.rb index b4c64a6c2..40e1df479 100644 --- a/app/jobs/notification/bundle/deliver_job.rb +++ b/app/jobs/notification/bundle/deliver_job.rb @@ -1,4 +1,6 @@ class Notification::Bundle::DeliverJob < ApplicationJob + queue_as :backend + def perform(bundle) bundle.deliver end diff --git a/app/jobs/push_notification_job.rb b/app/jobs/push_notification_job.rb index 128cf6a18..124366967 100644 --- a/app/jobs/push_notification_job.rb +++ b/app/jobs/push_notification_job.rb @@ -1,6 +1,4 @@ class PushNotificationJob < ApplicationJob - queue_as :default - def perform(notification) NotificationPusher.new(notification).push end diff --git a/config/queue.yml b/config/queue.yml index 6a199e535..8ae706455 100644 --- a/config/queue.yml +++ b/config/queue.yml @@ -3,7 +3,7 @@ default: &default - polling_interval: 1 batch_size: 500 workers: - - queues: "*" + - queues: [ "default", "backend" ] threads: 3 processes: <%= ENV.fetch("JOB_CONCURRENCY", 1) %> polling_interval: 0.1 From 48802ab6cd65386d8446b2c8f4875e98c8d39a01 Mon Sep 17 00:00:00 2001 From: Jason Zimdars Date: Sun, 31 Aug 2025 15:22:10 -0500 Subject: [PATCH 54/54] Make collection list scrollable --- app/views/notifications/settings/show.html.erb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/notifications/settings/show.html.erb b/app/views/notifications/settings/show.html.erb index 2eb10862c..89facf29a 100644 --- a/app/views/notifications/settings/show.html.erb +++ b/app/views/notifications/settings/show.html.erb @@ -13,9 +13,11 @@ <% end %>
-
+

Collections

- <%= render partial: "notifications/settings/collection", collection: @collections, locals: { user: Current.user } %> +
+ <%= render partial: "notifications/settings/collection", collection: @collections, locals: { user: Current.user } %> +