From 178f718a9b1cab57fbd9df511abe56533f12e129 Mon Sep 17 00:00:00 2001 From: Yamagishi Kazutoshi Date: Wed, 4 Oct 2017 17:22:52 +0900 Subject: [PATCH] Separate notifications preferences from general preferences (#4447) * Separate notifications preferences from general preferences * Refine settings/notifications/show * remove preferences.notifications --- .../settings/notifications_controller.rb | 32 ++++++++++++++++ app/lib/user_settings_decorator.rb | 26 +++++++------ .../settings/notifications/show.html.haml | 25 +++++++++++++ app/views/settings/preferences/show.html.haml | 19 ---------- config/locales/de.yml | 2 +- config/locales/en.yml | 2 +- config/locales/ja.yml | 2 +- config/locales/ko.yml | 2 +- config/locales/oc.yml | 2 +- config/locales/pl.yml | 2 +- config/navigation.rb | 1 + config/routes.rb | 1 + .../settings/notifications_controller_spec.rb | 37 +++++++++++++++++++ .../settings/preferences_controller_spec.rb | 6 --- 14 files changed, 117 insertions(+), 42 deletions(-) create mode 100644 app/controllers/settings/notifications_controller.rb create mode 100644 app/views/settings/notifications/show.html.haml create mode 100644 spec/controllers/settings/notifications_controller_spec.rb diff --git a/app/controllers/settings/notifications_controller.rb b/app/controllers/settings/notifications_controller.rb new file mode 100644 index 00000000000..09839f16eaa --- /dev/null +++ b/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 diff --git a/app/lib/user_settings_decorator.rb b/app/lib/user_settings_decorator.rb index cb1b3c4a927..1053ec488bb 100644 --- a/app/lib/user_settings_decorator.rb +++ b/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 diff --git a/app/views/settings/notifications/show.html.haml b/app/views/settings/notifications/show.html.haml new file mode 100644 index 00000000000..80cd615c7e4 --- /dev/null +++ b/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 diff --git a/app/views/settings/preferences/show.html.haml b/app/views/settings/preferences/show.html.haml index ffb1bbf6ac0..7475e3fd260 100644 --- a/app/views/settings/preferences/show.html.haml +++ b/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 diff --git a/config/locales/de.yml b/config/locales/de.yml index dce86409b1e..d4a925d2319 100644 --- a/config/locales/de.yml +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 3049e0365b0..4a6df8cb280 100644 --- a/config/locales/en.yml +++ b/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 diff --git a/config/locales/ja.yml b/config/locales/ja.yml index 78465e1210a..d637a99ea8c 100644 --- a/config/locales/ja.yml +++ b/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: 二段階認証 diff --git a/config/locales/ko.yml b/config/locales/ko.yml index 3a7636dbb7c..73f3f3a3715 100644 --- a/config/locales/ko.yml +++ b/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단계 인증 diff --git a/config/locales/oc.yml b/config/locales/oc.yml index 0b53b6b2d6c..1f25525a071 100644 --- a/config/locales/oc.yml +++ b/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 diff --git a/config/locales/pl.yml b/config/locales/pl.yml index d49ecfbe697..26a8a9c693b 100644 --- a/config/locales/pl.yml +++ b/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 diff --git a/config/navigation.rb b/config/navigation.rb index 0a6ab6d3db4..215d843b91d 100644 --- a/config/navigation.rb +++ b/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 diff --git a/config/routes.rb b/config/routes.rb index de3c1e0f9c4..8e80e151035 100644 --- a/config/routes.rb +++ b/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] diff --git a/spec/controllers/settings/notifications_controller_spec.rb b/spec/controllers/settings/notifications_controller_spec.rb new file mode 100644 index 00000000000..0bd99344865 --- /dev/null +++ b/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 diff --git a/spec/controllers/settings/preferences_controller_spec.rb b/spec/controllers/settings/preferences_controller_spec.rb index 60fa423023d..0f94316737e 100644 --- a/spec/controllers/settings/preferences_controller_spec.rb +++ b/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