Interface to configure notification preferences

.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.

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

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

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

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

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

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

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

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

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

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

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

app/views/preferences/notification_preferences/show.html.erb

@@ -0,0 +1,27 @@
+<% content_for :heading_class, "pb-0" %>
+
+<% content_for :heading do %>
+  <h1><%= t ".title" %></h1>
+  <%= render :partial => "navigation" %>
+<% end %>
+
+<%= bootstrap_form_for current_user.notification_preferences, :method => :patch, :url => { :action => :update } do |f| %>
+  <div class="notification_preferences">
+    <% user_notification_events.each do |event_name| %>
+      <div class="mb-3">
+        <%= 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"
+              }
+            ) %>
+      </div>
+    <% end %>
+  </div>
+
+  <%= f.primary t(".save") %>
+<% end %>

app/views/preferences/preferences/_navigation.html.erb

@@ -4,6 +4,9 @@
   <li class="nav-item">
     <%= link_to t(".preferences"), basic_preferences_path, :class => { "nav-link" => true, "active" => controller_name == "basic_preferences" } %>
   </li>
+  <li class="nav-item">
+    <%= link_to t(".notification_preferences"), notification_preferences_path, :class => { "nav-link" => true, "active" => controller_name == "notification_preferences" } %>
+  </li>
   <li class="nav-item">
     <%= link_to t(".advanced_preferences"), advanced_preferences_path, :class => { "nav-link" => true, "active" => controller_name == "advanced_preferences" } %>
   </li>

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

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

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

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