diff --git a/Gemfile b/Gemfile index 405073cc5..29dc202ab 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 7e29eeb36..e2af2504f 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 c7625286c..dba2f4c3d 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://fizzy.localhost:3006/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. @@ -73,7 +86,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/app/controllers/notifications/settings_controller.rb b/app/controllers/notifications/settings_controller.rb index 5e9faea60..4323ed3c8 100644 --- a/app/controllers/notifications/settings_controller.rb +++ b/app/controllers/notifications/settings_controller.rb @@ -1,7 +1,22 @@ class Notifications::SettingsController < ApplicationController include FilterScoped + 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/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/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 diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 020656380..b97027d9b 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" ], + [ "Every day", "daily" ], + [ "Every week", "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/jobs/notification/bundle/deliver_all_job.rb b/app/jobs/notification/bundle/deliver_all_job.rb new file mode 100644 index 000000000..250b53b6e --- /dev/null +++ b/app/jobs/notification/bundle/deliver_all_job.rb @@ -0,0 +1,9 @@ +class Notification::Bundle::DeliverAllJob < ApplicationJob + queue_as :backend + + def perform + ApplicationRecord.with_each_tenant do |tenant| + Notification::Bundle.deliver_all + end + 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..40e1df479 --- /dev/null +++ b/app/jobs/notification/bundle/deliver_job.rb @@ -0,0 +1,7 @@ +class Notification::Bundle::DeliverJob < ApplicationJob + queue_as :backend + + def perform(bundle) + bundle.deliver + end +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/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 3c34c8148..562bc0e3c 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,4 +1,12 @@ 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 + + private + def default_url_options + super.merge(script_name: Account.sole.slug) + 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 new file mode 100644 index 000000000..06bce5aa0 --- /dev/null +++ b/app/mailers/notification/bundle_mailer.rb @@ -0,0 +1,16 @@ +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, + subject: "Latest Activity in Fizzy" + end +end diff --git a/app/models/notification.rb b/app/models/notification.rb index 943cafd7e..3f5a00aac 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) if user.settings.bundling_emails? + end end diff --git a/app/models/notification/bundle.rb b/app/models/notification/bundle.rb new file mode 100644 index 000000000..5fab5fe7e --- /dev/null +++ b/app/models/notification/bundle.rb @@ -0,0 +1,69 @@ +class Notification::Bundle < ApplicationRecord + belongs_to :user + + 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) 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 + + validate :validate_no_overlapping + + class << self + def deliver_all + due.in_batches do |batch| + jobs = batch.collect { DeliverJob.new(it) } + ActiveJob.perform_all_later jobs + end + end + + def deliver_all_later + DeliverAllJob.perform_later + end + end + + def notifications + user.notifications.where(created_at: window).unread + end + + def deliver + processing! + + Notification::BundleMailer.notification(self).deliver if notifications.any? + + delivered! + end + + def deliver_later + DeliverJob.perform_later(self) + end + + private + def set_default_window + self.starts_at ||= Time.current + self.ends_at ||= user.settings.bundle_aggregation_period.from_now + end + + def window + starts_at..ends_at + 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/app/models/user.rb b/app/models/user.rb index 6c2cc14b8..39475882a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,6 @@ class User < ApplicationRecord - include Accessor, AiQuota, Assignee, Attachable, Mentionable, Named, Role, - Searcher, SignalUser, Staff, Transferable, Conversational + include Accessor, AiQuota, Assignee, Attachable, Configurable, Conversational, Mentionable, Named, + Notifiable, Role, Searcher, SignalUser, Staff, Transferable include Timelined # Depends on Accessor has_one_attached :avatar @@ -10,15 +10,11 @@ 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 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..cec3b6670 --- /dev/null +++ b/app/models/user/configurable.rb @@ -0,0 +1,10 @@ +module User::Configurable + extend ActiveSupport::Concern + + included do + has_one :settings, class_name: "User::Settings", dependent: :destroy + has_many :push_subscriptions, class_name: "Push::Subscription", dependent: :delete_all + + after_create :create_settings, unless: :system? + end +end diff --git a/app/models/user/notifiable.rb b/app/models/user/notifiable.rb new file mode 100644 index 000000000..b0e4ffcad --- /dev/null +++ b/app/models/user/notifiable.rb @@ -0,0 +1,29 @@ +module User::Notifiable + extend ActiveSupport::Concern + + 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) + transaction do + find_or_create_bundle_for(notification) + end + end + + private + 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/app/models/user/settings.rb b/app/models/user/settings.rb new file mode 100644 index 000000000..9db9edd24 --- /dev/null +++ b/app/models/user/settings.rb @@ -0,0 +1,46 @@ +class User::Settings < ApplicationRecord + belongs_to :user + + enum :bundle_email_frequency, %i[ never every_few_hours daily weekly ], + default: :every_few_hours, 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 + + def bundling_emails? + !bundle_email_never? + end + + private + def review_pending_bundles + if bundling_emails? + flush_pending_bundles + else + cancel_pending_bundles + end + end + + def cancel_pending_bundles + user.notification_bundles.pending.find_each do |bundle| + bundle.destroy + end + end + + def flush_pending_bundles + user.notification_bundles.pending.find_each do |bundle| + bundle.deliver_later + end + end +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/layouts/mailer.html.erb b/app/views/layouts/mailer.html.erb index 3aac9002e..793392ca3 100644 --- a/app/views/layouts/mailer.html.erb +++ b/app/views/layouts/mailer.html.erb @@ -3,11 +3,127 @@ - <%= yield %> + + <%= yield %> +
diff --git a/app/views/mailers/notification/bundle_mailer/_header.html.erb b/app/views/mailers/notification/bundle_mailer/_header.html.erb new file mode 100644 index 000000000..b9ba83e74 --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/_header.html.erb @@ -0,0 +1 @@ +

#<%= 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 new file mode 100644 index 000000000..311033d33 --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/_header.text.erb @@ -0,0 +1 @@ +#<%= 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 new file mode 100644 index 000000000..5dfddcf31 --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/_notification.html.erb @@ -0,0 +1,9 @@ + + + + + +
<%= image_tag user_avatar_url(notification.creator), alt: notification.creator.name, class: "avatar", size: 48 %> + <%= render "notification/bundle_mailer/header", notification: notification %> + <%= render "notification/bundle_mailer/#{notification.source_type.underscore}/body", notification: notification %> +
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..baaee2bf5 --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/_notification.text.erb @@ -0,0 +1,5 @@ +================================================================================ + +<%= render "notification/bundle_mailer/header", notification: notification %> +<%= render "notification/bundle_mailer/#{notification.source_type.underscore}/body", notification: notification %> + diff --git a/app/views/mailers/notification/bundle_mailer/event/_body.html.erb b/app/views/mailers/notification/bundle_mailer/event/_body.html.erb new file mode 100644 index 000000000..89273ef0d --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/event/_body.html.erb @@ -0,0 +1,9 @@ +<% event = notification.source %> + +

+ <%= link_to event_notification_title(event), notification.source %> +

+ +

+ <%= event_notification_body(event) %> +

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..5100aff60 --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/event/_body.text.erb @@ -0,0 +1,7 @@ +<% event = notification.source %> + +<%= event_notification_title(event) %> + +<%= event_notification_body(event).squish %> + +<%= url_for notification %> diff --git a/app/views/mailers/notification/bundle_mailer/mention/_body.html.erb b/app/views/mailers/notification/bundle_mailer/mention/_body.html.erb new file mode 100644 index 000000000..d08b01f4b --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/mention/_body.html.erb @@ -0,0 +1,9 @@ +<% mention = notification.source %> + +

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

+ +

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

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..da6f5babd --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/mention/_body.text.erb @@ -0,0 +1,7 @@ +<% mention = notification.source %> + +<%= mention.mentioner.first_name %> mentioned you. + +<%= mention.source.mentionable_content.truncate(250) %> + +<%= url_for mention %> diff --git a/app/views/mailers/notification/bundle_mailer/notification.html.erb b/app/views/mailers/notification/bundle_mailer/notification.html.erb new file mode 100644 index 000000000..f051cf426 --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/notification.html.erb @@ -0,0 +1,11 @@ + + +

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 %> +

Fizzy emails you about new notifications every few hours.
+ <%= link_to "Change how often you get these", notifications_settings_url %> + or <%= link_to "unsubscribe from all email notifications", new_notifications_unsubscribe_url(access_token: @unsubscribe_token) %>. +

+ + 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..0149767e6 --- /dev/null +++ b/app/views/mailers/notification/bundle_mailer/notification.text.erb @@ -0,0 +1,13 @@ +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: +<%= notifications_settings_url %> + +Unsubscribe from all email notifications: +<%= new_notifications_unsubscribe_url(access_token: @unsubscribe_token) %> 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 c386408d6..c30e46d7c 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -7,7 +7,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..a51aea099 --- /dev/null +++ b/app/views/notifications/settings/_email.html.erb @@ -0,0 +1,12 @@ +
+

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 new file mode 100644 index 000000000..047f57a15 --- /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 a95ed5598..b99ab815b 100644 --- a/app/views/notifications/settings/show.html.erb +++ b/app/views/notifications/settings/show.html.erb @@ -5,34 +5,16 @@

<%= @page_title %>

<% end %> -
-

Push notifications are ON

-

Push notifications are OFF

- - - -
-

Collections

- <%= render partial: "notifications/settings/collection", collection: @collections, locals: { user: Current.user } %> +
+ <%= render "notifications/settings/push_notifications" %> + <%= render "notifications/settings/email", settings: @settings %> +
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..3e100b14d --- /dev/null +++ b/app/views/notifications/unsubscribes/show.html.erb @@ -0,0 +1,9 @@ +
+

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" %> + Back to Fizzy + <% end %> +
diff --git a/config/environments/beta.rb b/config/environments/beta.rb index c7f44040b..650f1b084 100644 --- a/config/environments/beta.rb +++ b/config/environments/beta.rb @@ -1,7 +1,9 @@ require_relative "production" Rails.application.configure do - config.action_mailer.default_url_options = { host: "%{tenant}.37signals.works" } + 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: "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/development.rb b/config/environments/development.rb index 24fc6c04c..e8cdb8954 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -72,8 +72,17 @@ 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 ] + + # Host to be used in links rendered from ActionMailer + 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 diff --git a/config/environments/production.rb b/config/environments/production.rb index 73f7f6f1b..57994db43 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -74,7 +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: "%{tenant}.fizzy.37signals.com" } + config.action_mailer.default_url_options = { host: "fizzy.37signals.com", protocol: "https" } + + 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). @@ -86,11 +88,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..14bcfdc57 100644 --- a/config/environments/staging.rb +++ b/config/environments/staging.rb @@ -1,5 +1,7 @@ require_relative "production" Rails.application.configure do - config.action_mailer.default_url_options = { host: "%{tenant}.fizzy.37signals-staging.com" } + 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: "fizzy.37signals-staging.com", protocol: "https" } 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 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/config/routes.rb b/config/routes.rb index 985445f66..4b6b4d073 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -51,10 +51,14 @@ Rails.application.routes.draw do end end + namespace :notifications do + resource :settings + resource :unsubscribe + 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 @@ -162,15 +166,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.target, options) + polymorphic_url(event.eventable, options) end get "up", to: "rails/health#show", as: :rails_health_check 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/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/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/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 fa6dd112f..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_27_072601) 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 @@ -421,6 +421,15 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_27_072601) 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 @@ -487,6 +496,7 @@ ActiveRecord::Schema[8.1].define(version: 2025_08_27_072601) 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 5ba56a0c4..77d7d292a 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -1231,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: @@ -1378,6 +1393,7 @@ primary_keys: steps: id taggings: id tags: id + user_settings: id users: id watches: id workflow_stages: id @@ -1428,6 +1444,7 @@ data_sources: steps: true taggings: true tags: true + user_settings: true users: true watches: true workflow_stages: true @@ -2841,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 @@ -2941,4 +2992,4 @@ indexes: comment: valid: true workflows: [] -version: 20250827072601 +version: 20250828092106 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 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 diff --git a/test/fixtures/user/settings.yml b/test/fixtures/user/settings.yml new file mode 100644 index 000000000..29765f196 --- /dev/null +++ b/test/fixtures/user/settings.yml @@ -0,0 +1,18 @@ +_fixture: + model_class: User::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/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..9d7848db4 --- /dev/null +++ b/test/models/notification/bundle_test.rb @@ -0,0 +1,117 @@ +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 + 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 "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) + 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.days.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) + 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 + + 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 only: Notification::Bundle::DeliverJob do + Notification::Bundle.deliver_all + end + + 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 only: Notification::Bundle::DeliverJob do + Notification::Bundle.deliver_all + end + + bundle.reload + assert bundle.pending? + end +end diff --git a/test/models/user/configurable_test.rb b/test/models/user/configurable_test.rb new file mode 100644 index 000000000..dbc353936 --- /dev/null +++ b/test/models/user/configurable_test.rb @@ -0,0 +1,8 @@ +require "test_helper" + +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.settings.present? + end +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