diff --git a/Changelog.md b/Changelog.md index c058c7ccb..8afbfc569 100644 --- a/Changelog.md +++ b/Changelog.md @@ -147,6 +147,7 @@ diaspora.yml file**. The existing settings from 0.4.x and before will not work a * Correct photo count on profile page [#5751](https://github.com/diaspora/diaspora/pull/5751) * Fix mobile sign up from an invitation [#5754](https://github.com/diaspora/diaspora/pull/5754) * Set max-width for tag following button on tag page [#5752](https://github.com/diaspora/diaspora/pull/5752) +* Display error messages for failed password change [#5549](https://github.com/diaspora/diaspora/issues/5549) ## Features * Don't pull jQuery from a CDN by default [#5105](https://github.com/diaspora/diaspora/pull/5105) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 72dbea5c1..70b635a1a 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -4,17 +4,16 @@ class UsersController < ApplicationController before_action :authenticate_user!, :except => [:new, :create, :public, :user_photo] - layout ->(c) { request.format == :mobile ? "application" : "with_header_with_footer" }, only: [:privacy_settings, :edit] + + layout ->(c) { request.format == :mobile ? "application" : "with_header_with_footer" }, + only: [:privacy_settings, :edit, :update] respond_to :html def edit @aspect = :user_edit - @user = current_user - @email_prefs = Hash.new(true) - @user.user_preferences.each do |pref| - @email_prefs[pref.email_type] = false - end + @user = current_user + set_email_preferences end def privacy_settings @@ -26,16 +25,13 @@ class UsersController < ApplicationController @user = current_user if u = user_params - u.delete(:password) if u[:password].blank? - u.delete(:password_confirmation) if u[:password].blank? and u[:password_confirmation].blank? - u.delete(:language) if u[:language].blank? # 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 u[:current_password] && u[:password] && u[:password_confirmation] + elsif params[:change_password] if @user.update_with_password(u) password_changed = true flash[:notice] = I18n.t 'users.update.password_changed' @@ -79,10 +75,17 @@ class UsersController < ApplicationController end end end + set_email_preferences respond_to do |format| format.js { render :nothing => true, :status => 204 } - format.all { redirect_to password_changed ? new_user_session_path : edit_user_path } + format.all do + if password_changed + redirect_to new_user_session_path + else + render :edit + end + end end end @@ -206,4 +209,12 @@ class UsersController < ApplicationController ] ) end + + def set_email_preferences + @email_prefs = Hash.new(true) + + @user.user_preferences.each do |pref| + @email_prefs[pref.email_type] = false + end + end end diff --git a/app/helpers/error_messages_helper.rb b/app/helpers/error_messages_helper.rb index 2d76c5269..46480b1d3 100644 --- a/app/helpers/error_messages_helper.rb +++ b/app/helpers/error_messages_helper.rb @@ -6,11 +6,10 @@ module ErrorMessagesHelper # Render error messages for the given objects. The :message and :header_message options are allowed. def error_messages_for(*objects) options = objects.extract_options! - options[:header_message] ||= I18n.t('error_messages.helper.invalid_fields') 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 => "error_messages") do + content_tag(:div, class: "text-error") 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/passwords/edit.haml b/app/views/passwords/edit.haml index 804481bb8..779b9be0a 100644 --- a/app/views/passwords/edit.haml +++ b/app/views/passwords/edit.haml @@ -9,6 +9,8 @@ = AppConfig.settings.pod_name = form_for(resource, as: resource_name, url: password_path(resource_name), html: {class: "form-horizontal block-form", method: :put }, autocomplete: 'off') do |f| + = f.error_messages + %fieldset %label{for: "user_password"} = t('devise.passwords.edit.new_password') diff --git a/app/views/users/edit.html.haml b/app/views/users/edit.html.haml index f31ee00b4..032517f44 100644 --- a/app/views/users/edit.html.haml +++ b/app/views/users/edit.html.haml @@ -43,8 +43,9 @@ .span12 %h3 = t('.change_password') - = form_for 'user', :url => user_path, :html => { :method => :put } do |f| - = f.error_messages + = form_for @user, :url => user_path, :html => { :method => :put } do |f| + - if (@user.errors.keys & [:password, :password_confirmation, :current_password]).present? + = f.error_messages %p = f.label :current_password, t('.current_password') = f.password_field :current_password, :placeholder => t('.current_password_expl') @@ -58,7 +59,7 @@ .submit_block = link_to t('cancel'), edit_user_path = t('or') - = f.submit t('.change_password'), :class => "btn" + = f.submit t('.change_password'), class: "btn", name: 'change_password' %hr @@ -67,8 +68,6 @@ %h3 = t('.change_language') = form_for 'user', :url => user_path, :html => { :method => :put } do |f| - = f.error_messages - .form-inline = f.select :language, available_language_options = f.submit t('.change_language'), :class => "btn" @@ -81,7 +80,6 @@ %h3#stream-preferences = t('.stream_preferences') = form_for current_user, :url => user_path, :html => { :method => :put } do |f| - = f.error_messages = f.fields_for :stream_preferences do |type| #stream_prefs @@ -106,8 +104,6 @@ %h3#auto-follow-back-preferences = t('.following') = form_for current_user, :url => user_path, :html => { :method => :put } do |f| - = f.error_messages - = f.label :auto_follow_back, :class => "checkbox" do = f.check_box :auto_follow_back = t('.auto_follow_back') @@ -126,8 +122,6 @@ %h3 = t('.receive_email_notifications') = form_for 'user', :url => user_path, :html => { :method => :put } do |f| - = f.error_messages - = f.fields_for :email_preferences do |type| #email_prefs - if current_user.admin? @@ -173,7 +167,7 @@ .small-horizontal-spacer - = f.submit t('.change'), :class => "btn" + = f.submit t('.change'), :class => "btn", :id => "change_email_preferences" %hr diff --git a/features/desktop/change_email.feature b/features/desktop/change_email.feature index 950571a3f..87eed0b01 100644 --- a/features/desktop/change_email.feature +++ b/features/desktop/change_email.feature @@ -11,3 +11,11 @@ Feature: Change email And I follow the "confirm_email" link from the last sent email Then I should see "activated" And my "email" should be "new_email@newplac.es" + + Scenario: Change my email preferences + Given I am signed in + When I go to the users edit page + And I uncheck "user_email_preferences_mentioned" + And I press "change_email_preferences" + Then I should see "Email notifications changed" + And the "user_email_preferences_mentioned" checkbox should not be checked diff --git a/features/desktop/change_password.feature b/features/desktop/change_password.feature index da03e0989..2c62adba2 100644 --- a/features/desktop/change_password.feature +++ b/features/desktop/change_password.feature @@ -11,6 +11,15 @@ Feature: Change password When I sign in with password "newsecret" Then I should be on the stream page + Scenario: Attempt to change my password with invalid input + Given I am signed in + When I go to the edit user page + And I fill out change password section with my password and "too" and "short" + And I press "Change password" + Then I should see "Password change failed" + And I should see "Password is too short" + And I should see "Password confirmation doesn't match" + Scenario: Reset my password Given a user named "Georges Abitbol" with email "forgetful@users.net" Given I am on forgot password page @@ -25,6 +34,18 @@ Feature: Change password And I sign in manually as "georges_abitbol" with password "supersecret" Then I should be on the stream page + Scenario: Attempt to reset password with invalid password + Given a user named "Georges Abitbol" with email "forgetful@users.net" + Given I am on forgot password page + When I fill out forgot password form with "forgetful@users.net" + And I submit forgot password form + When I follow the "Change my password" link from the last sent email + When I fill out reset password form with "too" and "short" + And I press "Change my password" + Then I should be on the user password page + And I should see "Password is too short" + And I should see "Password confirmation doesn't match" + Scenario: Attempt to reset password with invalid email Given I am on forgot password page When I fill out forgot password form with "notanemail" diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 9b339135c..19a3819b9 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -112,9 +112,9 @@ describe UsersController, :type => :controller do }.not_to change(@user, :diaspora_handle) end - it 'redirects to the user edit page' do + it 'renders the user edit page' do put :update, @params - expect(response).to redirect_to edit_user_path + expect(response).to render_template('edit') end it 'responds with a 204 on a js request' do @@ -122,17 +122,21 @@ describe UsersController, :type => :controller do expect(response.status).to eq(204) end - context 'password updates' do - before do - @password_params = {:current_password => 'bluepin7', - :password => "foobaz", - :password_confirmation => "foobaz"} + describe 'password updates' do + let(:password_params) do + {:current_password => 'bluepin7', + :password => "foobaz", + :password_confirmation => "foobaz"} + end + + let(:params) do + {id: @user.id, user: password_params, change_password: 'Change Password'} end it "uses devise's update with password" do - expect(@user).to receive(:update_with_password).with(hash_including(@password_params)) + expect(@user).to receive(:update_with_password).with(hash_including(password_params)) allow(@controller).to receive(:current_user).and_return(@user) - put :update, :id => @user.id, :user => @password_params + put :update, params end end