diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index cbe6a459e58..f7565912e20 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -56,7 +56,7 @@ Metrics/BlockNesting: # Offense count: 23 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 345 + Max: 342 # Offense count: 72 # Configuration parameters: AllowedMethods, AllowedPatterns. diff --git a/app/controllers/preferences/notification_preferences_controller.rb b/app/controllers/preferences/notification_preferences_controller.rb new file mode 100644 index 00000000000..719910b1f2e --- /dev/null +++ b/app/controllers/preferences/notification_preferences_controller.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Preferences + class NotificationPreferencesController < PreferencesController + private + + def update_preferences + current_user.notification_preferences.update(params[:user_notification_preferences]) + end + end +end diff --git a/app/helpers/user_notification_preferences_helper.rb b/app/helpers/user_notification_preferences_helper.rb new file mode 100644 index 00000000000..74ba534647f --- /dev/null +++ b/app/helpers/user_notification_preferences_helper.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module UserNotificationPreferencesHelper + def user_notification_events + UserNotificationPreferences::EVENTS + end + + def user_notification_mechanisms + UserNotificationPreferences::MECHANISMS + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 07764f124f0..ceee49b5561 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -294,6 +294,10 @@ def default_diary_language=(language) preference.update!(:v => language) end + def notification_preferences + @notification_preferences ||= UserNotificationPreferences.new(self) + end + def home_location? home_lat && home_lon end diff --git a/app/models/user_notification_preferences.rb b/app/models/user_notification_preferences.rb new file mode 100644 index 00000000000..5653c2418d0 --- /dev/null +++ b/app/models/user_notification_preferences.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +class UserNotificationPreferences + extend ActiveModel::Naming + include ActiveModel::Conversion + + EVENTS = %w[ + changeset_comment + diary_comment + direct_message + gpx_import_failure + gpx_import_success + new_follower + note_comment + ].freeze + + MECHANISMS = %w[ + email + ].freeze + + def initialize(user) + @user = user + end + + # Receives a hash in the form `event => [mechanisms...]`. Eg: + # `{:changeset_comment => ["email"], :new_follower => []}` + def update(new_prefs) + updated_records = + EVENTS.map do |event_name| + MECHANISMS.filter_map do |mechanism| + next unless new_prefs.key?(event_name) + + record = @user.preferences.find_or_initialize_by(:k => "notification.#{event_name}.#{mechanism}") + record.v = Array.wrap(new_prefs[event_name]).include?(mechanism) + record + end + end.flatten + + UserPreference.transaction do + updated_records.each(&:save!) + true + end + end + + # One getter method for each event. Required by ActionView + # to render the form, but also generally useful to us. + EVENTS.each do |event_name| + define_method event_name do + prefs = + @user + .preferences + .where("k LIKE 'notification.#{event_name}.%'") + .pluck(:k, :v) + .to_h + .transform_keys { |k| k.split(".").last } + + MECHANISMS.filter do |mechanism| + prefs.key?(mechanism) ? ActiveModel::Type::Boolean.new.cast(prefs[mechanism]) : true + end + end + end +end diff --git a/app/notifiers/changeset_comment_notifier.rb b/app/notifiers/changeset_comment_notifier.rb index b3291cd70b5..5cc56139777 100644 --- a/app/notifiers/changeset_comment_notifier.rb +++ b/app/notifiers/changeset_comment_notifier.rb @@ -8,5 +8,6 @@ class ChangesetCommentNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "UserMailer" config.method = "changeset_comment_notification" + config.if = -> { recipient.notification_preferences.changeset_comment.include?("email") } end end diff --git a/app/notifiers/diary_comment_notifier.rb b/app/notifiers/diary_comment_notifier.rb index afe992ab6a9..0ef515d8e91 100644 --- a/app/notifiers/diary_comment_notifier.rb +++ b/app/notifiers/diary_comment_notifier.rb @@ -8,5 +8,6 @@ class DiaryCommentNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "UserMailer" config.method = "diary_comment_notification" + config.if = -> { recipient.notification_preferences.diary_comment.include?("email") } end end diff --git a/app/notifiers/direct_message_notifier.rb b/app/notifiers/direct_message_notifier.rb index 7814195640f..7374695e6e0 100644 --- a/app/notifiers/direct_message_notifier.rb +++ b/app/notifiers/direct_message_notifier.rb @@ -8,5 +8,6 @@ class DirectMessageNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "UserMailer" config.method = "message_notification" + config.if = -> { recipient.notification_preferences.direct_message.include?("email") } end end diff --git a/app/notifiers/gpx_import_failure_notifier.rb b/app/notifiers/gpx_import_failure_notifier.rb index 598b39a0530..0b6cbe1012a 100644 --- a/app/notifiers/gpx_import_failure_notifier.rb +++ b/app/notifiers/gpx_import_failure_notifier.rb @@ -4,5 +4,6 @@ class GpxImportFailureNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "UserMailer" config.method = "gpx_failure" + config.if = -> { recipient.notification_preferences.gpx_import_failure.include?("email") } end end diff --git a/app/notifiers/gpx_import_success_notifier.rb b/app/notifiers/gpx_import_success_notifier.rb index 6edf67fbf0e..b0beda515be 100644 --- a/app/notifiers/gpx_import_success_notifier.rb +++ b/app/notifiers/gpx_import_success_notifier.rb @@ -8,5 +8,6 @@ class GpxImportSuccessNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "UserMailer" config.method = "gpx_success" + config.if = -> { recipient.notification_preferences.gpx_import_success.include?("email") } end end diff --git a/app/notifiers/new_follower_notifier.rb b/app/notifiers/new_follower_notifier.rb index 1508e206421..12c9e0a691b 100644 --- a/app/notifiers/new_follower_notifier.rb +++ b/app/notifiers/new_follower_notifier.rb @@ -8,5 +8,6 @@ class NewFollowerNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "UserMailer" config.method = "follow_notification" + config.if = -> { recipient.notification_preferences.new_follower.include?("email") } end end diff --git a/app/notifiers/note_comment_notifier.rb b/app/notifiers/note_comment_notifier.rb index 03cbb17941c..98e4a678078 100644 --- a/app/notifiers/note_comment_notifier.rb +++ b/app/notifiers/note_comment_notifier.rb @@ -8,5 +8,6 @@ class NoteCommentNotifier < ApplicationNotifier deliver_by :email do |config| config.mailer = "UserMailer" config.method = "note_comment_notification" + config.if = -> { recipient.notification_preferences.note_comment.include?("email") } end end diff --git a/app/views/preferences/notification_preferences/show.html.erb b/app/views/preferences/notification_preferences/show.html.erb new file mode 100644 index 00000000000..0ab2c016b62 --- /dev/null +++ b/app/views/preferences/notification_preferences/show.html.erb @@ -0,0 +1,27 @@ +<% content_for :heading_class, "pb-0" %> + +<% content_for :heading do %> +

<%= t ".title" %>

+ <%= render :partial => "navigation" %> +<% end %> + +<%= bootstrap_form_for current_user.notification_preferences, :method => :patch, :url => { :action => :update } do |f| %> +
+ <% user_notification_events.each do |event_name| %> +
+ <%= f.collection_check_boxes( + event_name, + user_notification_mechanisms, + :itself, + ->(mechanism) { t(".delivery_mechanisms.#{mechanism}") }, + :label => { + :text => t(".#{event_name}"), + :class => "fw-bold fs-6" + } + ) %> +
+ <% end %> +
+ + <%= f.primary t(".save") %> +<% end %> diff --git a/app/views/preferences/preferences/_navigation.html.erb b/app/views/preferences/preferences/_navigation.html.erb index 00b7cf7bc06..cd6f70e3838 100644 --- a/app/views/preferences/preferences/_navigation.html.erb +++ b/app/views/preferences/preferences/_navigation.html.erb @@ -4,6 +4,9 @@
  • <%= link_to t(".preferences"), basic_preferences_path, :class => { "nav-link" => true, "active" => controller_name == "basic_preferences" } %>
    • +
  • + <%= link_to t(".notification_preferences"), notification_preferences_path, :class => { "nav-link" => true, "active" => controller_name == "notification_preferences" } %> +
  • <%= link_to t(".advanced_preferences"), advanced_preferences_path, :class => { "nav-link" => true, "active" => controller_name == "advanced_preferences" } %>
    • diff --git a/config/locales/en.yml b/config/locales/en.yml index fea147731f1..050e9c9b2a9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2113,8 +2113,9 @@ en: update_success_flash: message: Preferences updated. navigation: - preferences: Preferences - advanced_preferences: Advanced Preferences + preferences: Interface + notification_preferences: Notifications + advanced_preferences: Advanced basic_preferences: show: title: My Preferences @@ -2135,6 +2136,19 @@ en: light: Light dark: Dark save: Update Preferences + notification_preferences: + show: + title: Notification Preferences + changeset_comment: Changeset comments + diary_comment: Diary comments + direct_message: Direct messages + gpx_import_failure: "GPS trace import: failure" + gpx_import_success: "GPX trace import: success" + new_follower: New followers + note_comment: Note comments + save: Update Preferences + delivery_mechanisms: + email: Email advanced_preferences: show: title: My Advanced Preferences diff --git a/config/routes.rb b/config/routes.rb index c0cf75d7e75..e63bed58e68 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -339,6 +339,7 @@ scope :preferences, :module => :preferences do resource :basic_preferences, :path => "basic", :only => [:show, :update] + resource :notification_preferences, :path => "notifications", :only => [:show, :update] resource :advanced_preferences, :path => "advanced", :only => [:show, :update] end get "/preferences", :to => redirect(:path => "/preferences/basic"), :as => nil diff --git a/test/models/user_notification_preferences_test.rb b/test/models/user_notification_preferences_test.rb new file mode 100644 index 00000000000..473a9eb6c6b --- /dev/null +++ b/test/models/user_notification_preferences_test.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require "test_helper" + +class UserNotificationPreferencesTest < ActiveSupport::TestCase + def test_all_enabled_by_default + preferences = UserNotificationPreferences.new(create(:user)) + assert_equal ["email"], preferences.changeset_comment + assert_equal ["email"], preferences.diary_comment + assert_equal ["email"], preferences.direct_message + assert_equal ["email"], preferences.gpx_import_failure + assert_equal ["email"], preferences.gpx_import_success + assert_equal ["email"], preferences.new_follower + assert_equal ["email"], preferences.note_comment + end + + def test_update + preferences = UserNotificationPreferences.new(create(:user)) + preferences.update( + "changeset_comment" => ["email"], + "diary_comment" => [], + "direct_message" => ["email"], + "gpx_import_failure" => ["email"], + "gpx_import_success" => nil, + "new_follower" => [] + ) + + assert_equal ["email"], preferences.changeset_comment + assert_equal [], preferences.diary_comment + assert_equal ["email"], preferences.direct_message + assert_equal ["email"], preferences.gpx_import_failure + assert_equal [], preferences.gpx_import_success + assert_equal [], preferences.new_follower + + # Default value + assert_equal ["email"], preferences.note_comment + end + + def test_update_ignore_invalid_values + preferences = UserNotificationPreferences.new(create(:user)) + + preferences.update("changeset_comment" => ["whatsapp"]) + assert_equal [], preferences.changeset_comment + assert_equal 0, UserPreference.where("k LIKE '%whatsapp%'").count + + preferences.update("changeset_comment" => %w[whatsapp email]) + assert_equal ["email"], preferences.changeset_comment + assert_equal 0, UserPreference.where("k LIKE '%whatsapp%'").count + + preferences.update("imaginary_event" => ["email"]) + assert_equal 0, UserPreference.where("k LIKE '%imaginary_event%'").count + end + + def test_update_ignore_unmentioned_events + preferences = UserNotificationPreferences.new(create(:user)) + preferences.update( + "changeset_comment" => ["email"], + "diary_comment" => [] + ) + assert_equal 1, count_event_preferences_in_db("changeset_comment") + assert_equal 1, count_event_preferences_in_db("diary_comment") + assert_equal 0, count_event_preferences_in_db("direct_message") + assert_equal 0, count_event_preferences_in_db("gpx_import_failure") + assert_equal 0, count_event_preferences_in_db("gpx_import_success") + assert_equal 0, count_event_preferences_in_db("new_follower") + assert_equal 0, count_event_preferences_in_db("note_comment") + assert_equal 0, count_event_preferences_in_db("note_comment") + end + + private + + def count_event_preferences_in_db(event_name) + # A bit paranoid, but want to avoid misspellings, etc that produce + # false positives. + raise "Unknown event #{event_name.inspect}" unless UserNotificationPreferences::EVENTS.include?(event_name) + + UserPreference.where("k LIKE 'notification.#{event_name}.%'").count + end +end diff --git a/test/notifiers/changeset_notifier_test.rb b/test/notifiers/changeset_notifier_test.rb new file mode 100644 index 00000000000..3abf6653d22 --- /dev/null +++ b/test/notifiers/changeset_notifier_test.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "test_helper" + +class ChangesetCommentNotifierTest < ActiveSupport::TestCase + def setup + ActionMailer::Base.deliveries.clear + end + + def test_send_email_when_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("changeset_comment" => ["email"]) + + trigger_notification(candidate_recipient) + + email = ActionMailer::Base.deliveries.first + assert_equal [candidate_recipient.email], email.to + end + + def test_do_not_send_email_when_not_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("changeset_comment" => []) + + trigger_notification(candidate_recipient) + + assert_empty ActionMailer::Base.deliveries + end + + private + + def trigger_notification(changeset_author) + changeset = create(:changeset, :user => changeset_author) + create(:changeset_subscription, :changeset => changeset, :subscriber => changeset_author) + + comment_author = create(:user) + changeset_comment = create(:changeset_comment, :author => comment_author, :changeset => changeset) + + perform_enqueued_jobs do + Nominatim.stub :describe_location, nil do + ChangesetCommentNotifier.with(:record => changeset_comment).deliver + end + end + end +end diff --git a/test/notifiers/diary_comment_notifier_test.rb b/test/notifiers/diary_comment_notifier_test.rb new file mode 100644 index 00000000000..3c54ee3dc04 --- /dev/null +++ b/test/notifiers/diary_comment_notifier_test.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require "test_helper" + +class DiaryCommentNotifierTest < ActiveSupport::TestCase + def setup + ActionMailer::Base.deliveries.clear + end + + def test_send_email_when_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("diary_comment" => ["email"]) + + trigger_notification(candidate_recipient) + + email = ActionMailer::Base.deliveries.first + assert_equal [candidate_recipient.email], email.to + end + + def test_do_not_send_email_when_not_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("diary_comment" => []) + + trigger_notification(candidate_recipient) + + assert_empty ActionMailer::Base.deliveries + end + + private + + def trigger_notification(diary_author) + create(:language, :code => "en") + diary_entry = create(:diary_entry, :user => diary_author) + create(:diary_entry_subscription, :diary_entry => diary_entry, :user => diary_author) + + comment_author = create(:user) + diary_comment = create(:diary_comment, :user => comment_author, :diary_entry => diary_entry) + + perform_enqueued_jobs do + DiaryCommentNotifier.with(:record => diary_comment).deliver + end + end +end diff --git a/test/notifiers/direct_message_notifier_test.rb b/test/notifiers/direct_message_notifier_test.rb new file mode 100644 index 00000000000..cc311ac8b10 --- /dev/null +++ b/test/notifiers/direct_message_notifier_test.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "test_helper" + +class DirectMessageNotifierTest < ActiveSupport::TestCase + def setup + ActionMailer::Base.deliveries.clear + end + + def test_send_email_when_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("direct_message" => ["email"]) + + trigger_notification(candidate_recipient) + + email = ActionMailer::Base.deliveries.first + assert_equal [candidate_recipient.email], email.to + end + + def test_do_not_send_email_when_not_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("direct_message" => []) + + trigger_notification(candidate_recipient) + + assert_empty ActionMailer::Base.deliveries + end + + private + + def trigger_notification(message_recipient) + message = create(:message, :recipient => message_recipient) + + perform_enqueued_jobs do + DirectMessageNotifier.with(:record => message).deliver + end + end +end diff --git a/test/notifiers/gpx_import_failure_notifier_test.rb b/test/notifiers/gpx_import_failure_notifier_test.rb new file mode 100644 index 00000000000..2064a3432e1 --- /dev/null +++ b/test/notifiers/gpx_import_failure_notifier_test.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require "test_helper" + +class GpxImportFailureNotifierTest < ActiveSupport::TestCase + def setup + ActionMailer::Base.deliveries.clear + end + + def test_send_email_when_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("gpx_import_failure" => ["email"]) + + trigger_notification(candidate_recipient) + + email = ActionMailer::Base.deliveries.first + assert_equal [candidate_recipient.email], email.to + end + + def test_do_not_send_email_when_not_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("gpx_import_failure" => []) + + trigger_notification(candidate_recipient) + + assert_empty ActionMailer::Base.deliveries + end + + private + + def trigger_notification(trace_author) + perform_enqueued_jobs do + GpxImportFailureNotifier.with( + :trace_name => "My trace", + :trace_description => "There are others like it, etc", + :trace_tags => %w[vegetarian assume elapse], + :error => "I'm sorry, Dave" + ).deliver(trace_author) + end + end +end diff --git a/test/notifiers/gpx_import_success_notifier_test.rb b/test/notifiers/gpx_import_success_notifier_test.rb new file mode 100644 index 00000000000..8bc8b0135dc --- /dev/null +++ b/test/notifiers/gpx_import_success_notifier_test.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "test_helper" + +class GpxImportSuccessNotifierTest < ActiveSupport::TestCase + def setup + ActionMailer::Base.deliveries.clear + end + + def test_send_email_when_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("gpx_import_success" => ["email"]) + + trigger_notification(candidate_recipient) + + email = ActionMailer::Base.deliveries.first + assert_equal [candidate_recipient.email], email.to + end + + def test_do_not_send_email_when_not_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("gpx_import_success" => []) + + trigger_notification(candidate_recipient) + + assert_empty ActionMailer::Base.deliveries + end + + private + + def trigger_notification(trace_author) + trace = create(:trace, :user => trace_author) + + perform_enqueued_jobs do + GpxImportSuccessNotifier.with(:record => trace, :possible_points => 5).deliver + end + end +end diff --git a/test/notifiers/new_follower_notifier_test.rb b/test/notifiers/new_follower_notifier_test.rb new file mode 100644 index 00000000000..d6ef0c4a17c --- /dev/null +++ b/test/notifiers/new_follower_notifier_test.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "test_helper" + +class NewFollowerNotifierTest < ActiveSupport::TestCase + def setup + ActionMailer::Base.deliveries.clear + end + + def test_send_email_when_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("new_follower" => ["email"]) + + trigger_notification(candidate_recipient) + + email = ActionMailer::Base.deliveries.first + assert_equal [candidate_recipient.email], email.to + end + + def test_do_not_send_email_when_not_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("new_follower" => []) + + trigger_notification(candidate_recipient) + + assert_empty ActionMailer::Base.deliveries + end + + private + + def trigger_notification(followee) + follow = create(:follow, :following => followee) + perform_enqueued_jobs do + NewFollowerNotifier.with(:record => follow).deliver + end + end +end diff --git a/test/notifiers/note_comment_notifier_test.rb b/test/notifiers/note_comment_notifier_test.rb new file mode 100644 index 00000000000..8249aff6afe --- /dev/null +++ b/test/notifiers/note_comment_notifier_test.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "test_helper" + +class NoteCommentNotifierTest < ActiveSupport::TestCase + def setup + ActionMailer::Base.deliveries.clear + end + + def test_send_email_when_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("note_comment" => ["email"]) + + trigger_notification(candidate_recipient) + + email = ActionMailer::Base.deliveries.first + assert_equal [candidate_recipient.email], email.to + end + + def test_do_not_send_email_when_not_subscribed + candidate_recipient = create(:user) + candidate_recipient.notification_preferences.update("note_comment" => []) + + trigger_notification(candidate_recipient) + + assert_empty ActionMailer::Base.deliveries + end + + private + + def trigger_notification(note_author) + note = create(:note, :author => note_author) + create(:note_subscription, :note => note, :user => note_author) + + comment_author = create(:user) + note_comment = create(:note_comment, :author => comment_author, :note => note) + + perform_enqueued_jobs do + Nominatim.stub :describe_location, nil do + NoteCommentNotifier.with(:record => note_comment).deliver + end + end + end +end diff --git a/test/system/notification_preferences_test.rb b/test/system/notification_preferences_test.rb new file mode 100644 index 00000000000..2471616c10a --- /dev/null +++ b/test/system/notification_preferences_test.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "application_system_test_case" + +class NotificationPreferencesTest < ApplicationSystemTestCase + test "toggling preferences" do + ActionMailer::Base.deliveries.clear + + user = create(:user) + sign_in_as(user) + + visit notification_preferences_path + + assert_selector ".notification_preferences input:checked", :count => 7 + + follow1 = create(:follow, :following => user) + perform_enqueued_jobs do + NewFollowerNotifier.with(:record => follow1).deliver + end + email = ActionMailer::Base.deliveries.first + assert_equal 1, email.to.count + assert_equal user.email, email.to.first + ActionMailer::Base.deliveries.clear + + uncheck "user_notification_preferences_new_follower_email" + uncheck "user_notification_preferences_gpx_import_success_email" + click_on "Update Preferences" + + assert_selector ".notification_preferences input:checked", :count => 5 + assert_selector "input#user_notification_preferences_new_follower_email:not(checked)" + + follow2 = create(:follow, :following => user) + perform_enqueued_jobs do + NewFollowerNotifier.with(:record => follow2).deliver + end + assert_empty ActionMailer::Base.deliveries + end +end