Separate notifications preferences from general preferences (#4447)

* Separate notifications preferences from general preferences

* Refine settings/notifications/show

* remove preferences.notifications

app/controllers/settings/notifications_controller.rb

@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+class Settings::NotificationsController < ApplicationController
+  layout 'admin'
+
+  before_action :authenticate_user!
+
+  def show; end
+
+  def update
+    user_settings.update(user_settings_params.to_h)
+
+    if current_user.save
+      redirect_to settings_notifications_path, notice: I18n.t('generic.changes_saved_msg')
+    else
+      render :show
+    end
+  end
+
+  private
+
+  def user_settings
+    UserSettingsDecorator.new(current_user)
+  end
+
+  def user_settings_params
+    params.require(:user).permit(
+      notification_emails: %i(follow follow_request reblog favourite mention digest),
+      interactions: %i(must_be_follower must_be_following)
+    )
+  end
+end

app/lib/user_settings_decorator.rb

@@ -15,17 +15,17 @@ class UserSettingsDecorator
   private
 
   def process_update
-    user.settings['notification_emails'] = merged_notification_emails
-    user.settings['interactions'] = merged_interactions
-    user.settings['default_privacy'] = default_privacy_preference
-    user.settings['default_sensitive'] = default_sensitive_preference
-    user.settings['unfollow_modal'] = unfollow_modal_preference
-    user.settings['boost_modal'] = boost_modal_preference
-    user.settings['delete_modal'] = delete_modal_preference
-    user.settings['auto_play_gif'] = auto_play_gif_preference
-    user.settings['system_font_ui'] = system_font_ui_preference
-    user.settings['noindex'] = noindex_preference
-    user.settings['theme'] = theme_preference
+    user.settings['notification_emails'] = merged_notification_emails if change?('notification_emails')
+    user.settings['interactions']        = merged_interactions if change?('interactions')
+    user.settings['default_privacy']     = default_privacy_preference if change?('setting_default_privacy')
+    user.settings['default_sensitive']   = default_sensitive_preference if change?('setting_default_sensitive')
+    user.settings['unfollow_modal']      = unfollow_modal_preference if change?('setting_unfollow_modal')
+    user.settings['boost_modal']         = boost_modal_preference if change?('setting_boost_modal')
+    user.settings['delete_modal']        = delete_modal_preference if change?('setting_delete_modal')
+    user.settings['auto_play_gif']       = auto_play_gif_preference if change?('setting_auto_play_gif')
+    user.settings['system_font_ui']      = system_font_ui_preference if change?('setting_system_font_ui')
+    user.settings['noindex']             = noindex_preference if change?('setting_noindex')
+    user.settings['theme']               = theme_preference if change?('theme')
   end
 
   def merged_notification_emails
@@ -83,4 +83,8 @@ class UserSettingsDecorator
   def coerce_values(params_hash)
     params_hash.transform_values { |x| x == '1' }
   end
+
+  def change?(key)
+    !settings[key].nil?
+  end
 end

app/views/settings/notifications/show.html.haml

@@ -0,0 +1,25 @@
+- content_for :page_title do
+  = t('settings.notifications')
+
+= simple_form_for current_user, url: settings_notifications_path, html: { method: :put } do |f|
+  = render 'shared/error_messages', object: current_user
+
+  .fields-group
+    = f.simple_fields_for :notification_emails, hash_to_object(current_user.settings.notification_emails) do |ff|
+      = ff.input :follow, as: :boolean, wrapper: :with_label
+      = ff.input :follow_request, as: :boolean, wrapper: :with_label
+      = ff.input :reblog, as: :boolean, wrapper: :with_label
+      = ff.input :favourite, as: :boolean, wrapper: :with_label
+      = ff.input :mention, as: :boolean, wrapper: :with_label
+ 
+  .fields-group
+    = f.simple_fields_for :notification_emails, hash_to_object(current_user.settings.notification_emails) do |ff|
+      = ff.input :digest, as: :boolean, wrapper: :with_label
+
+  .fields-group
+    = f.simple_fields_for :interactions, hash_to_object(current_user.settings.interactions) do |ff|
+      = ff.input :must_be_follower, as: :boolean, wrapper: :with_label
+      = ff.input :must_be_following, as: :boolean, wrapper: :with_label
+
+  .actions
+    = f.button :button, t('generic.save_changes'), type: :submit

app/views/settings/preferences/show.html.haml

@@ -18,25 +18,6 @@
 
     = f.input :setting_default_sensitive, as: :boolean, wrapper: :with_label
 
-  %h4= t 'preferences.notifications'
-
-  .fields-group
-    = f.simple_fields_for :notification_emails, hash_to_object(current_user.settings.notification_emails) do |ff|
-      = ff.input :follow, as: :boolean, wrapper: :with_label
-      = ff.input :follow_request, as: :boolean, wrapper: :with_label
-      = ff.input :reblog, as: :boolean, wrapper: :with_label
-      = ff.input :favourite, as: :boolean, wrapper: :with_label
-      = ff.input :mention, as: :boolean, wrapper: :with_label
-
-  .fields-group
-    = f.simple_fields_for :notification_emails, hash_to_object(current_user.settings.notification_emails) do |ff|
-      = ff.input :digest, as: :boolean, wrapper: :with_label
-
-  .fields-group
-    = f.simple_fields_for :interactions, hash_to_object(current_user.settings.interactions) do |ff|
-      = ff.input :must_be_follower, as: :boolean, wrapper: :with_label
-      = ff.input :must_be_following, as: :boolean, wrapper: :with_label
-
   %h4= t 'preferences.other'
 
   .fields-group

config/locales/de.yml

@@ -319,7 +319,6 @@ de:
     truncate: "…"
   preferences:
     languages: Sprachen
-    notifications: Benachrichtigungen
     other: Weiteres
     publishing: Beiträge
     web: Web
@@ -390,6 +389,7 @@ de:
     export: Datenexport
     followers: Autorisierte Folgende
     import: Datenimport
+    notifications: Benachrichtigungen
     preferences: Einstellungen
     settings: Einstellungen
     two_factor_authentication: Zwei-Faktor-Authentisierung

config/locales/en.yml

@@ -395,7 +395,6 @@ en:
     truncate: "…"
   preferences:
     languages: Languages
-    notifications: Notifications
     other: Other
     publishing: Publishing
     web: Web
@@ -466,6 +465,7 @@ en:
     export: Data export
     followers: Authorized followers
     import: Import
+    notifications: Notifications
     preferences: Preferences
     settings: Settings
     two_factor_authentication: Two-factor Authentication

config/locales/ja.yml

@@ -395,7 +395,6 @@ ja:
     truncate: "…"
   preferences:
     languages: 言語
-    notifications: 通知
     other: その他
     publishing: 投稿
     web: ウェブ
@@ -466,6 +465,7 @@ ja:
     export: データのエクスポート
     followers: 信頼済みのインスタンス
     import: データのインポート
+    notifications: 通知
     preferences: ユーザー設定
     settings: 設定
     two_factor_authentication: 二段階認証

config/locales/ko.yml

@@ -393,7 +393,6 @@ ko:
     truncate: "…"
   preferences:
     languages: 언어
-    notifications: 알림
     other: 기타
     publishing: 퍼블리싱
     web: 웹
@@ -464,6 +463,7 @@ ko:
     export: 데이터 내보내기
     followers: 신뢰 중인 인스턴스
     import: 데이터 가져오기
+    notifications: 알림
     preferences: 사용자 설정
     settings: 설정
     two_factor_authentication: 2단계 인증

config/locales/oc.yml

@@ -473,7 +473,6 @@ oc:
     truncate: "…"
   preferences:
     languages: Lengas
-    notifications: Notificacions
     other: Autre
     publishing: Publicar
     web: Interfàcia Web
@@ -544,6 +543,7 @@ oc:
     export: Export donadas
     followers: Seguidors autorizats
     import: Importar
+    notifications: Notificacions
     preferences: Preferéncias
     settings: Paramètres
     two_factor_authentication: Autentificacion en dos temps

config/locales/pl.yml

@@ -396,7 +396,6 @@ pl:
     truncate: "…"
   preferences:
     languages: Języki
-    notifications: Powiadomienia
     other: Pozostałe
     publishing: Publikowanie
     web: Sieć
@@ -467,6 +466,7 @@ pl:
     export: Eksportowanie danych
     followers: Autoryzowani śledzący
     import: Importowanie danych
+    notifications: Powiadomienia
     preferences: Preferencje
     settings: Ustawienia
     two_factor_authentication: Uwierzytelnianie dwuetapowe

config/navigation.rb

@@ -7,6 +7,7 @@ SimpleNavigation::Configuration.run do |navigation|
     primary.item :settings, safe_join([fa_icon('cog fw'), t('settings.settings')]), settings_profile_url do |settings|
       settings.item :profile, safe_join([fa_icon('user fw'), t('settings.edit_profile')]), settings_profile_url
       settings.item :preferences, safe_join([fa_icon('sliders fw'), t('settings.preferences')]), settings_preferences_url
+      settings.item :notifications, safe_join([fa_icon('bell fw'), t('settings.notifications')]), settings_notifications_url
       settings.item :password, safe_join([fa_icon('lock fw'), t('auth.change_password')]), edit_user_registration_url, highlights_on: %r{/auth/edit|/settings/delete}
       settings.item :two_factor_authentication, safe_join([fa_icon('mobile fw'), t('settings.two_factor_authentication')]), settings_two_factor_authentication_url, highlights_on: %r{/settings/two_factor_authentication}
       settings.item :import, safe_join([fa_icon('cloud-upload fw'), t('settings.import')]), settings_import_url

config/routes.rb

@@ -67,6 +67,7 @@ Rails.application.routes.draw do
   namespace :settings do
     resource :profile, only: [:show, :update]
     resource :preferences, only: [:show, :update]
+    resource :notifications, only: [:show, :update]
     resource :import, only: [:show, :create]
 
     resource :export, only: [:show]

spec/controllers/settings/notifications_controller_spec.rb

@@ -0,0 +1,37 @@
+require 'rails_helper'
+
+describe Settings::NotificationsController do
+  render_views
+
+  let(:user) { Fabricate(:user) }
+
+  before do
+    sign_in user, scope: :user
+  end
+
+  describe 'GET #show' do
+    it 'returns http success' do
+      get :show
+      expect(response).to have_http_status(:success)
+    end
+  end
+
+  describe 'PUT #update' do
+    it 'updates notifications settings' do
+      user.settings['notification_emails'] = user.settings['notification_emails'].merge('follow' => false)
+      user.settings['interactions'] = user.settings['interactions'].merge('must_be_follower' => true)
+
+      put :update, params: {
+        user: {
+          notification_emails: { follow: '1' },
+          interactions: { must_be_follower: '0' },
+        }
+      }
+
+      expect(response).to redirect_to(settings_notifications_path)
+      user.reload
+      expect(user.settings['notification_emails']['follow']).to be true
+      expect(user.settings['interactions']['must_be_follower']).to be false
+    end
+  end
+end

spec/controllers/settings/preferences_controller_spec.rb

@@ -29,15 +29,11 @@ describe Settings::PreferencesController do
     it 'updates user settings' do
       user.settings['boost_modal'] = false
       user.settings['delete_modal'] = true
-      user.settings['notification_emails'] = user.settings['notification_emails'].merge('follow' => false)
-      user.settings['interactions'] = user.settings['interactions'].merge('must_be_follower' => true)
 
       put :update, params: {
         user: {
           setting_boost_modal: '1',
           setting_delete_modal: '0',
-          notification_emails: { follow: '1' },
-          interactions: { must_be_follower: '0' },
         }
       }
 
@@ -45,8 +41,6 @@ describe Settings::PreferencesController do
       user.reload
       expect(user.settings['boost_modal']).to be true
       expect(user.settings['delete_modal']).to be false
-      expect(user.settings['notification_emails']['follow']).to be true
-      expect(user.settings['interactions']['must_be_follower']).to be false
     end
   end
 end