Merge pull request #6965 from SuperTux88/5847-fix-settings-page

Fix settings page after submit

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/registration.scss

-.page-registrations.action-new {
+.page-registrations.action-new,
+.page-registrations.action-create {
   .ball {
     background: image-url('branding/ball.png') no-repeat;
     background-size: contain;

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,78 +17,36 @@ class UsersController < ApplicationController
 
   def update
     password_changed = false
+    user_data = user_params
     @user = current_user
 
-    if u = user_params
-
-      # change email notifications
-      if u[:email_preferences]
-        @user.update_user_preferences(u[:email_preferences])
-        flash[:notice] = I18n.t "users.update.email_notifications_changed"
+    if user_data
       # change password
-      elsif params[:change_password]
-        if @user.update_with_password(u)
-          password_changed = true
-          flash[:notice] = I18n.t "users.update.password_changed"
-        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[:strip_exif]
-        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
+      if params[:change_password]
+        password_changed = change_password(user_data)
+      else
+        update_user(user_data)
       end
     end
-    set_email_preferences
 
-    respond_to do |format|
-      format.js   { render :nothing => true, :status => 204 }
-      format.all  do
-        if password_changed
-          redirect_to new_user_session_path
-        else
-          render :edit
-        end
-      end
+    if password_changed
+      redirect_to new_user_session_path
+    else
+      set_email_preferences
+      render :edit
+    end
+  end
+
+  def update_privacy_settings
+    privacy_params = params.fetch(:user).permit(:strip_exif)
+
+    if current_user.update_attributes(strip_exif: privacy_params[:strip_exif])
+      flash[:notice] = t("users.update.settings_updated")
+    else
+      flash[:error] = t("users.update.settings_not_updated")
     end
+
+    redirect_to :back
   end
 
   def destroy
@@ -182,6 +139,7 @@ class UsersController < ApplicationController
 
   private
 
+  # rubocop:disable Metrics/MethodLength
   def user_params
     params.fetch(:user).permit(
       :email,
@@ -191,26 +149,85 @@ class UsersController < ApplicationController
       :language,
       :color_theme,
       :disable_mail,
-      :invitation_service,
-      :invitation_identifier,
       :show_community_spotlight_in_stream,
-      :strip_exif,
       :auto_follow_back,
       :auto_follow_back_aspect_id,
-      :remember_me,
       :getting_started,
-      email_preferences: [
-        :someone_reported,
-        :also_commented,
-        :mentioned,
-        :comment_on_post,
-        :private_message,
-        :started_sharing,
-        :liked,
-        :reshared
-      ]
+      email_preferences: %i(
+        someone_reported
+        also_commented
+        mentioned
+        comment_on_post
+        private_message
+        started_sharing
+        liked
+        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/layouts/_drawer.mobile.haml

@@ -30,6 +30,6 @@
           = t("layouts.header.profile")
           = person_image_tag(current_user, size: :thumb_small)
       %li= link_to t("_contacts"), contacts_path
-      %li= link_to t("layouts.header.settings"), users_edit_path
+      %li= link_to t("layouts.header.settings"), edit_user_path
       %li= link_to t("layouts.application.toggle"), toggle_mobile_path
       %li= link_to t("layouts.header.logout"), destroy_user_session_path, method: :delete

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?

app/views/users/_privacy_settings.haml

@@ -3,7 +3,7 @@
     %h3
       = t(".title")
 
-    = form_for current_user, url: user_path, html: {method: :put} do |f|
+    = form_for current_user, url: update_privacy_settings_path, html: {method: :put} do |f|
       = f.error_messages
 
       = f.fields_for :stream_preferences do

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
@@ -107,19 +108,19 @@ Diaspora::Application.routes.draw do
   end
 
   controller :users do
-    get 'public/:username'          => :public,           :as => 'users_public'
-    get 'getting_started'           => :getting_started,  :as => 'getting_started'
-    get 'privacy'                   => :privacy_settings, :as => 'privacy_settings'
-    get 'getting_started_completed' => :getting_started_completed
-    get 'confirm_email/:token'      => :confirm_email,    :as => 'confirm_email'
+    get "public/:username"          => :public,                  :as => :users_public
+    get "getting_started"           => :getting_started,         :as => :getting_started
+    get "confirm_email/:token"      => :confirm_email,           :as => :confirm_email
+    get "privacy"                   => :privacy_settings,        :as => :privacy_settings
+    put "privacy"                   => :update_privacy_settings, :as => :update_privacy_settings
+    get "getting_started_completed" => :getting_started_completed
   end
 
-  # This is a hack to overide a route created by devise.
-  # I couldn't find anything in devise to skip that route, see Bug #961
-  get 'users/edit' => redirect('/user/edit')
-
-  devise_for :users, :controllers => {:registrations => "registrations",
-                                      :sessions      => "sessions"}
+  devise_for :users, controllers: {sessions: :sessions}, skip: :registration
+  devise_scope :user do
+    get "/users/sign_up" => "registrations#new",    :as => :new_user_registration
+    post "/users"        => "registrations#create", :as => :user_registration
+  end
 
   #legacy routes to support old invite routes
   get 'users/invitation/accept' => 'invitations#edit'

features/desktop/change_email.feature

@@ -3,7 +3,7 @@ Feature: Change email
 
   Scenario: Change my email
     Given I am signed in
-    When I go to the users edit page
+    When I go to the edit user page
     And I fill in the following:
       | user_email         | new_email@newplac.es           |
     And I press "Change email"
@@ -14,7 +14,7 @@ Feature: Change email
 
   Scenario: Change my email preferences
     Given I am signed in
-    When I go to the users edit page
+    When I go to the edit user page
     And I uncheck "user_email_preferences_mentioned"
     And I press "change_email_preferences"
     Then I should see "Email notifications changed"

features/desktop/change_password.feature

@@ -3,7 +3,7 @@ Feature: Change password
 
   Scenario: Change my password
     Given I am signed in
-    When I go to the users edit page
+    When I go to the edit user page
     And I fill out change password section with my password and "newsecret" and "newsecret"
     And I press "Change password"
     Then I should see "Password changed"

features/desktop/closes_account.feature

@@ -6,7 +6,7 @@ Feature: Close account
 
   Scenario: user closes account
     Given I am signed in
-    When I go to the users edit page
+    When I go to the edit user page
     And I click on selector "#close_account"
     Then I should see a modal
     And I should see "Hey, please don’t go!" within "#closeAccountModal"

features/mobile/change_password.feature

@@ -6,7 +6,7 @@ Feature: Change password
 
   Scenario: Change my password
     Given I am signed in on the mobile website
-    When I go to the users edit page
+    When I go to the edit user page
     And I fill out change password section with my password and "newsecret" and "newsecret"
     And I press "Change password"
     Then I should see "Password changed"

features/mobile/closes_account.feature

@@ -6,7 +6,7 @@ Feature: Close account
 
   Scenario: user closes account
     Given I am signed in on the mobile website
-    When I go to the users edit page
+    When I go to the edit user page
     And I click on selector "#close_account"
     Then I should see a modal
     And I should see "Hey, please don’t go!" within "#closeAccountModal"

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',