From 37837b3f731670509b4c3a8f22c170ba8efed0ee Mon Sep 17 00:00:00 2001
From: Benjamin Neff <benjamin@coding4coffee.ch>
Date: Tue, 9 Aug 2016 23:17:33 +0200
Subject: [PATCH 1/4] fix style on registration-page after errors

---
 app/assets/stylesheets/registration.scss | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/assets/stylesheets/registration.scss b/app/assets/stylesheets/registration.scss
index 6a98cbc8b..53c65674d 100644
--- a/app/assets/stylesheets/registration.scss
+++ b/app/assets/stylesheets/registration.scss
@@ -1,4 +1,5 @@
-.page-registrations.action-new {
+.page-registrations.action-new,
+.page-registrations.action-create {
   .ball {
     background: image-url('branding/ball.png') no-repeat;
     background-size: contain;

From 86e75a02bb0ecb00f326cb3493cb9e54b91bb17e Mon Sep 17 00:00:00 2001
From: Benjamin Neff <benjamin@coding4coffee.ch>
Date: Wed, 10 Aug 2016 00:11:49 +0200
Subject: [PATCH 2/4] fix privacy settings form submit

---
 app/controllers/users_controller.rb    | 19 ++++++++++++-------
 app/views/users/_privacy_settings.haml |  2 +-
 config/routes.rb                       | 11 ++++++-----
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 367f642fb..e2c43ab31 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -40,12 +40,6 @@ class UsersController < ApplicationController
         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
@@ -92,6 +86,18 @@ class UsersController < ApplicationController
     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
     if params[:user] && params[:user][:current_password] && current_user.valid_password?(params[:user][:current_password])
       current_user.close_account!
@@ -194,7 +200,6 @@ class UsersController < ApplicationController
       :invitation_service,
       :invitation_identifier,
       :show_community_spotlight_in_stream,
-      :strip_exif,
       :auto_follow_back,
       :auto_follow_back_aspect_id,
       :remember_me,
diff --git a/app/views/users/_privacy_settings.haml b/app/views/users/_privacy_settings.haml
index 86038417b..9a881bc51 100644
--- a/app/views/users/_privacy_settings.haml
+++ b/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
diff --git a/config/routes.rb b/config/routes.rb
index cea28508f..1c14ab70b 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -107,11 +107,12 @@ 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.

From be47c6bcd010f31bd2495aeec434681b38f68bfb Mon Sep 17 00:00:00 2001
From: Benjamin Neff <benjamin@coding4coffee.ch>
Date: Wed, 10 Aug 2016 01:34:05 +0200
Subject: [PATCH 3/4] remove redirect hack for devise

only expose routes from devise that we actually use
---
 app/views/layouts/_drawer.mobile.haml    |  2 +-
 config/routes.rb                         | 11 +++++------
 features/desktop/change_email.feature    |  4 ++--
 features/desktop/change_password.feature |  2 +-
 features/desktop/closes_account.feature  |  2 +-
 features/mobile/change_password.feature  |  2 +-
 features/mobile/closes_account.feature   |  2 +-
 7 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/app/views/layouts/_drawer.mobile.haml b/app/views/layouts/_drawer.mobile.haml
index 97e9f6d89..d676a2084 100644
--- a/app/views/layouts/_drawer.mobile.haml
+++ b/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
diff --git a/config/routes.rb b/config/routes.rb
index 1c14ab70b..520e98551 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -115,12 +115,11 @@ Diaspora::Application.routes.draw do
     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'
diff --git a/features/desktop/change_email.feature b/features/desktop/change_email.feature
index 87eed0b01..c13b53275 100644
--- a/features/desktop/change_email.feature
+++ b/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"
diff --git a/features/desktop/change_password.feature b/features/desktop/change_password.feature
index 8096c22d5..f37ebe045 100644
--- a/features/desktop/change_password.feature
+++ b/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"
diff --git a/features/desktop/closes_account.feature b/features/desktop/closes_account.feature
index 5e08c6ca9..613029c13 100644
--- a/features/desktop/closes_account.feature
+++ b/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"
diff --git a/features/mobile/change_password.feature b/features/mobile/change_password.feature
index e191ddb18..f30edb215 100644
--- a/features/mobile/change_password.feature
+++ b/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"
diff --git a/features/mobile/closes_account.feature b/features/mobile/closes_account.feature
index 3f659023f..2854e1d6e 100644
--- a/features/mobile/closes_account.feature
+++ b/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"

From 71ed7446c19c83bdbb542482285bc968d49f0520 Mon Sep 17 00:00:00 2001
From: Benjamin Neff <benjamin@coding4coffee.ch>
Date: Wed, 10 Aug 2016 23:02:15 +0200
Subject: [PATCH 4/4] 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          |   1 +
 app/assets/stylesheets/settings.scss      |   1 +
 app/controllers/users_controller.rb       | 158 ++++++++++++----------
 app/helpers/error_messages_helper.rb      |   2 +-
 app/views/shared/_settings_nav.haml       |   2 +-
 app/views/users/_edit.haml                |  14 +-
 config/routes.rb                          |   3 +-
 spec/controllers/users_controller_spec.rb |   5 -
 8 files changed, 98 insertions(+), 88 deletions(-)

diff --git a/app/assets/stylesheets/base.scss b/app/assets/stylesheets/base.scss
index 6356b0631..e9a7762e9 100644
--- a/app/assets/stylesheets/base.scss
+++ b/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;
 }
diff --git a/app/assets/stylesheets/settings.scss b/app/assets/stylesheets/settings.scss
index e51edfd36..11cb5b13f 100644
--- a/app/assets/stylesheets/settings.scss
+++ b/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;
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index e2c43ab31..b7242c118 100644
--- a/app/controllers/users_controller.rb
+++ b/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
-
-      # 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[: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
 
@@ -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: [
-        :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)
diff --git a/app/helpers/error_messages_helper.rb b/app/helpers/error_messages_helper.rb
index 46480b1d3..73b1cab9d 100644
--- a/app/helpers/error_messages_helper.rb
+++ b/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
diff --git a/app/views/shared/_settings_nav.haml b/app/views/shared/_settings_nav.haml
index 53794791e..e54f3e424 100644
--- a/app/views/shared/_settings_nav.haml
+++ b/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,
diff --git a/app/views/users/_edit.haml b/app/views/users/_edit.haml
index 2f7d0c001..aec5eccc7 100644
--- a/app/views/users/_edit.haml
+++ b/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?
diff --git a/config/routes.rb b/config/routes.rb
index 520e98551..31a740484 100644
--- a/config/routes.rb
+++ b/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
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index d28ad8088..b97fa31cb 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/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',