Permalink
Browse files

Show multiple error messages when there are multiple errors saving a …

…profile edit

Reuse the errors partial from the signup process; make the message more general.
  • Loading branch information...
1 parent be0545d commit 5e405f64af823c6c8293cb79cb723cab4443a873 @carols10cents carols10cents committed Nov 22, 2012
@@ -73,9 +73,9 @@ def update
redirect_to user_path(@user)
else
- error_message = @user.errors.full_messages.first
-
- flash[:error] = "Profile could not be saved: #{error_message}"
+ error_message = render_to_string :partial => 'users/errors',
+ :locals => {:user => @user}
+ flash[:error] = error_message.html_safe
render :edit
end
View
@@ -341,15 +341,6 @@ def self.authenticate(username, pass)
# Edit profile information
def update_profile!(params)
- unless params[:password].blank?
- if params[:password] == params[:password_confirm]
- self.password = params[:password]
- self.save
- else
- self.errors.add(:password, "doesn't match confirmation.")
- return false
- end
- end
params[:email] = nil if params[:email].blank?
@@ -360,21 +351,48 @@ def update_profile!(params)
self.always_send_to_twitter = params[:user] && params[:user][:always_send_to_twitter].to_i
- return false unless self.save
+ # I can't figure out how to use a real rails validator to confirm that
+ # password matches password_confirm, since these two attributes are
+ # virtual and we only want to check this in this particular case of
+ # updating a user.
+
+ # Additionally, running the other validations clears self.errors, so
+ # we need to add our own errors AFTER calling valid?. But we shouldn't
+ # save the record at all if the password change isn't valid.
+
+ self.valid?
+
+ unless params[:password].blank?
+ if params[:password] == params[:password_confirm]
+ self.password = params[:password]
+ self.save
+ else
+ self.errors.add(:password, "doesn't match confirmation.")
+ end
+ end
- author.username = params[:username]
- author.name = params[:name]
- author.email = params[:email]
- author.website = params[:website]
- author.bio = params[:bio]
- author.save
+ # Calling valid? again here would make the validators run again, which
+ # would clear self.errors again. We may have added an error about the
+ # password not matching the confirmation.
+ if self.errors.present?
+ return false
+ else
+ self.save
- # TODO: Send out notice to other nodes
- # To each remote domain that is following you via hub
- # and to each remote domain that you follow via salmon
- author.feed.ping_hubs
+ author.username = params[:username]
+ author.name = params[:name]
+ author.email = params[:email]
+ author.website = params[:website]
+ author.bio = params[:bio]
+ author.save
- return self
+ # TODO: Send out notice to other nodes
+ # To each remote domain that is following you via hub
+ # and to each remote domain that you follow via salmon
+ author.feed.ping_hubs
+
+ return self
+ end
end
# A better name would be very welcome.
@@ -1,7 +1,7 @@
- if user && user.errors.any?
%div{:id => "error"}
%div{:id => "error_explanation"}
- %strong= "#{pluralize(user.errors.count, "error")} prohibited your account from being created:"
+ %strong= "Sorry, #{pluralize(user.errors.count, "error")} we need you to fix:"
%ul
- user.errors.full_messages.each do |msg|
%li= msg
@@ -102,12 +102,33 @@
end
within flash do
- assert has_content?("Profile could not be saved: Password doesn't match confirmation.")
+ assert has_content?("Sorry, 1 error we need you to fix:")
+ assert has_content?("Password doesn't match confirmation.")
end
assert has_field?("password")
end
+ it "shows multiple error messages if there are multiple problems" do
+ visit "/users/#{@u.username}/edit"
+
+ fill_in "username", :with => "something too_long&with invalid#chars."
+
+ fill_in "password", :with => "new_password"
+ fill_in "password_confirm", :with => "bunk"
+
+ VCR.use_cassette("update_profile_multiple_errors") do
+ click_button "Save"
+ end
+
+ within flash do
+ assert has_content?("Sorry, 3 errors we need you to fix:")
+ assert has_content?("Password doesn't match confirmation.")
+ assert has_content?("Username contains restricted characters.")
+ assert has_content?("Username must be 17 characters or fewer.")
+ end
+ end
+
it "verifies your email if you change it" do
visit "/users/#{@u.username}/edit"
email = "new_email@new_email.com"
@@ -25,7 +25,7 @@
fill_in "password", :with => "baseball"
click_button "Log in"
- assert_match /1 error prohibited your account from being created:/, page.body
+ assert_match /Sorry, 1 error we need you to fix:/, page.body
assert_match /contains restricted characters\./, page.body
end
@@ -34,7 +34,7 @@
fill_in "password", :with => "baseball"
click_button "Log in"
- assert_match /1 error prohibited your account from being created:/, page.body
+ assert_match /Sorry, 1 error we need you to fix:/, page.body
assert_match /Username can't be blank/, page.body
end
@@ -70,7 +70,7 @@
click_button "Log in"
- assert_match /1 error prohibited your account from being created:/, page.body
+ assert_match /Sorry, 1 error we need you to fix:/, page.body
assert_match /Username must be 17 characters or fewer\./, page.body
end
end
@@ -86,7 +86,7 @@
fill_in "username", :with => "taken"
click_button "Finish Signup"
- assert_match /1 error prohibited your account from being created:/, page.body
+ assert_match /Sorry, 1 error we need you to fix:/, page.body
assert_match /Username has already been taken/, page.body
fill_in "username", :with => "nottaken"

0 comments on commit 5e405f6

Please sign in to comment.