Fix user settings style after submit

Fixed:
* wrong url
* broken navigation
* broken design
after saving the user settings

Fixes #5847

app/assets/stylesheets/base.scss

@@ -20,6 +20,7 @@ body {
 .page-tags,
 .page-user_applications,
 .page-users.action-edit,
+.page-users.action-update,
 .page-users.action-privacy_settings {
   background-color: $main-background;
 }

app/assets/stylesheets/settings.scss

@@ -6,6 +6,7 @@
 .page-services.action-index,
 .page-user_applications,
 .page-users.action-edit,
+.page-users.action-update,
 .page-users.action-privacy_settings {
   .framed-content {
     padding-left: 10px;

app/controllers/users_controller.rb

@@ -7,7 +7,6 @@ class UsersController < ApplicationController
   respond_to :html
 
   def edit
-    @aspect = :user_edit
     @user = current_user
     set_email_preferences
   end
@@ -18,71 +17,23 @@ class UsersController < ApplicationController
 
   def update
     password_changed = false
+    user_data = user_params
     @user = current_user
 
-    if u = user_params
+    if user_data
-
-      # change email notifications
-      if u[:email_preferences]
-        @user.update_user_preferences(u[:email_preferences])
-        flash[:notice] = I18n.t "users.update.email_notifications_changed"
       # change password
-      elsif params[:change_password]
+      if params[:change_password]
-        if @user.update_with_password(u)
+        password_changed = change_password(user_data)
-          password_changed = true
+      else
-          flash[:notice] = I18n.t "users.update.password_changed"
+        update_user(user_data)
-        else
-          flash[:error] = I18n.t "users.update.password_not_changed"
-        end
-      elsif u[:show_community_spotlight_in_stream] || u[:getting_started]
-        if @user.update_attributes(u)
-          flash[:notice] = I18n.t "users.update.settings_updated"
-        else
-          flash[:notice] = I18n.t "users.update.settings_not_updated"
-        end
-      elsif u[:language]
-        if @user.update_attributes(u)
-          I18n.locale = @user.language
-          flash[:notice] = I18n.t "users.update.language_changed"
-        else
-          flash[:error] = I18n.t "users.update.language_not_changed"
-        end
-      elsif u[:email]
-        @user.unconfirmed_email = u[:email]
-        if @user.save
-          @user.send_confirm_email
-          if @user.unconfirmed_email
-            flash[:notice] = I18n.t "users.update.unconfirmed_email_changed"
-          end
-        else
-          @user.reload # match user object with the database
-          flash[:error] = I18n.t "users.update.unconfirmed_email_not_changed"
-        end
-      elsif u[:auto_follow_back]
-        if  @user.update_attributes(u)
-          flash[:notice] = I18n.t "users.update.follow_settings_changed"
-        else
-          flash[:error] = I18n.t "users.update.follow_settings_not_changed"
-        end
-      elsif u[:color_theme]
-        if @user.update_attributes(u)
-          flash[:notice] = I18n.t "users.update.color_theme_changed"
-        else
-          flash[:error] = I18n.t "users.update.color_theme_not_changed"
-        end
       end
     end
-    set_email_preferences
 
-    respond_to do |format|
+    if password_changed
-      format.js   { render :nothing => true, :status => 204 }
+      redirect_to new_user_session_path
-      format.all  do
+    else
-        if password_changed
+      set_email_preferences
-          redirect_to new_user_session_path
+      render :edit
-        else
-          render :edit
-        end
-      end
     end
   end
 
@@ -188,6 +139,7 @@ class UsersController < ApplicationController
 
   private
 
+  # rubocop:disable Metrics/MethodLength
   def user_params
     params.fetch(:user).permit(
       :email,
@@ -197,25 +149,85 @@ class UsersController < ApplicationController
       :language,
       :color_theme,
       :disable_mail,
-      :invitation_service,
-      :invitation_identifier,
       :show_community_spotlight_in_stream,
       :auto_follow_back,
       :auto_follow_back_aspect_id,
-      :remember_me,
       :getting_started,
-      email_preferences: [
+      email_preferences: %i(
-        :someone_reported,
+        someone_reported
-        :also_commented,
+        also_commented
-        :mentioned,
+        mentioned
-        :comment_on_post,
+        comment_on_post
-        :private_message,
+        private_message
-        :started_sharing,
+        started_sharing
-        :liked,
+        liked
-        :reshared
+        reshared
-      ]
+      )
     )
   end
+  # rubocop:enable Metrics/MethodLength
+
+  def update_user(user_data)
+    if user_data[:email_preferences]
+      change_email_preferences(user_data)
+    elsif user_data[:language]
+      change_language(user_data)
+    elsif user_data[:email]
+      change_email(user_data)
+    elsif user_data[:auto_follow_back]
+      change_settings(user_data, "users.update.follow_settings_changed", "users.update.follow_settings_not_changed")
+    elsif user_data[:color_theme]
+      change_settings(user_data, "users.update.color_theme_changed", "users.update.color_theme_not_changed")
+    else
+      change_settings(user_data)
+    end
+  end
+
+  def change_password(user_data)
+    if @user.update_with_password(user_data)
+      flash[:notice] = t("users.update.password_changed")
+      true
+    else
+      flash.now[:error] = t("users.update.password_not_changed")
+      false
+    end
+  end
+
+  # change email notifications
+  def change_email_preferences(user_data)
+    @user.update_user_preferences(user_data[:email_preferences])
+    flash.now[:notice] = t("users.update.email_notifications_changed")
+  end
+
+  def change_language(user_data)
+    if @user.update_attributes(user_data)
+      I18n.locale = @user.language
+      flash.now[:notice] = t("users.update.language_changed")
+    else
+      flash.now[:error] = t("users.update.language_not_changed")
+    end
+  end
+
+  def change_email(user_data)
+    @user.unconfirmed_email = user_data[:email]
+    if @user.save
+      @user.send_confirm_email
+      if @user.unconfirmed_email
+        flash.now[:notice] = t("users.update.unconfirmed_email_changed")
+      end
+    else
+      @user.reload # match user object with the database
+      flash.now[:error] = t("users.update.unconfirmed_email_not_changed")
+    end
+  end
+
+  def change_settings(user_data, successful="users.update.settings_updated", error="users.update.settings_not_updated")
+    if @user.update_attributes(user_data)
+      flash.now[:notice] = t(successful)
+    else
+      flash.now[:error] = t(error)
+    end
+  end
 
   def set_email_preferences
     @email_prefs = Hash.new(true)

app/helpers/error_messages_helper.rb

@@ -9,7 +9,7 @@ module ErrorMessagesHelper
     options[:message] ||= I18n.t('error_messages.helper.correct_the_following_errors_and_try_again')
     messages = objects.compact.map { |o| o.errors.full_messages }.flatten
     unless messages.empty?
-      content_tag(:div, class: "text-error") do
+      content_tag(:div, class: "text-danger") do
         list_items = messages.map { |msg| content_tag(:li, msg) }
         content_tag(:h2, options[:header_message]) + content_tag(:p, options[:message]) + content_tag(:ul, list_items.join.html_safe)
       end

app/views/shared/_settings_nav.haml

@@ -6,7 +6,7 @@
     = link_to t("profile"), edit_profile_path,
       class: current_page?(edit_profile_path) ? "list-group-item active" : "list-group-item"
     = link_to t("account"), edit_user_path,
-      class: current_page?(edit_user_path) ? "list-group-item active" : "list-group-item"
+      class: request.path == edit_user_path ? "list-group-item active" : "list-group-item"
     = link_to t("privacy"), privacy_settings_path,
       class: current_page?(privacy_settings_path) ? "list-group-item active" : "list-group-item"
     = link_to t("_services"), services_path,

app/views/users/_edit.haml

@@ -19,7 +19,7 @@
           %i.entypo-lock.gray.settings-visibility{title: t("users.edit.your_email_private")}
 
 
-        = form_for "user", url: user_path,
+        = form_for "user", url: edit_user_path,
           html: {method: :put, class: "form-horizontal col-md-12", id: "email-form"} do |f|
           = f.error_messages
           .form-group
@@ -32,7 +32,7 @@
     .row
       .col-md-12
         %h3= t(".change_password")
-        = form_for @user, url: user_path, html: {method: :put, class: "form-horizontal"} do |f|
+        = form_for @user, url: edit_user_path, html: {method: :put, class: "form-horizontal"} do |f|
           - if (@user.errors.keys & %i(password password_confirmation current_password)).present?
             = f.error_messages
           .form-group
@@ -59,7 +59,7 @@
     .row
       .col-md-12
         %h3= t(".change_language")
-        = form_for "user", url: user_path, html: {method: :put} do |f|
+        = form_for "user", url: edit_user_path, html: {method: :put} do |f|
           .form-inline.clearfix
             = f.select :language, available_language_options, {}, class: "form-control form-group"
             = f.submit t(".change_language"), class: "btn btn-primary pull-right"
@@ -68,7 +68,7 @@
     .row
       .col-md-12
         %h3= t(".change_color_theme")
-        = form_for "user", url: user_path, html: {method: :put} do |f|
+        = form_for "user", url: edit_user_path, html: {method: :put} do |f|
           .form-inline.clearfix
             = f.select :color_theme, available_color_themes, {}, class: "form-control form-group"
             = f.submit t(".change_color_theme"), class: "btn btn-primary pull-right"
@@ -78,7 +78,7 @@
       .col-md-12
         %h3#stream-preferences
           = t(".stream_preferences")
-        = form_for current_user, url: user_path, html: {method: :put} do |f|
+        = form_for current_user, url: edit_user_path, html: {method: :put} do |f|
 
           = f.fields_for :stream_preferences do
             #stream_prefs
@@ -98,7 +98,7 @@
       .col-md-12
         %h3#auto-follow-back-preferences
           = t(".following")
-        = form_for current_user, url: user_path, html: {method: :put, class: "form-horizontal"} do |f|
+        = form_for current_user, url: edit_user_path, html: {method: :put, class: "form-horizontal"} do |f|
           = f.label :auto_follow_back, class: "checkbox-inline" do
             = f.check_box :auto_follow_back
             = t(".auto_follow_back")
@@ -120,7 +120,7 @@
       .col-md-12
         %h3
           = t(".receive_email_notifications")
-        = form_for "user", url: user_path, html: {method: :put} do |f|
+        = form_for "user", url: edit_user_path, html: {method: :put} do |f|
           = f.fields_for :email_preferences do |type|
             #email_prefs
               - if current_user.admin?

config/routes.rb

@@ -98,7 +98,8 @@ Diaspora::Application.routes.draw do
 
   # Users and people
 
-  resource :user, :only => [:edit, :update, :destroy], :shallow => true do
+  resource :user, only: %i(edit destroy), shallow: true do
+    put :edit, action: :update
     get :getting_started_completed
     post :export_profile
     get :download_profile

spec/controllers/users_controller_spec.rb

@@ -118,11 +118,6 @@ describe UsersController, :type => :controller do
       expect(response).to render_template('edit')
     end
 
-    it 'responds with a 204 on a js request' do
-      put :update, @params.merge(:format => :js)
-      expect(response.status).to eq(204)
-    end
-
     describe 'password updates' do
       let(:password_params) do
         {:current_password => 'bluepin7',