Permalink
Browse files

Merge remote-tracking branch 'origin/feature/change_username'

Conflicts:
	test/models/user_test.rb
  • Loading branch information...
2 parents 3193d97 + 04dc420 commit 0295cace143676d82193c0cc1615b9a20afeac6d @carols10cents carols10cents committed Dec 27, 2012
View
@@ -58,7 +58,7 @@ group :development, :test do
gem "show_me_the_cookies", "~> 1.1.0"
gem "rocco", :git => "git://github.com/rtomayko/rocco.git"
gem "pygmentize", "~> 0.0.3"
- gem "mocha", "~> 0.13.0"
+ gem "mocha", "~> 0.13.0", :require => false
gem "vcr", "~> 1.10.3"
gem "simplecov", "~> 0.4.0", :require => false
gem "launchy", "~> 2.0.5"
@@ -61,20 +61,22 @@ def edit
def update
if @user == current_user
- response = @user.edit_user_profile(params)
- if response == true
+ @user.update_profile!(params)
+ unless @user.errors.any?
unless @user.email.blank? || @user.email_confirmed
Notifier.send_confirm_email_notification(@user.email, @user.create_token)
flash[:notice] = "A link to confirm your updated email address has been sent to #{@user.email}."
else
flash[:notice] = "Profile saved!"
end
- redirect_to user_path(params[:id])
-
+ redirect_to user_path(@user)
else
- flash[:error] = "Profile could not be saved: #{response}"
+ error_message = render_to_string :partial => 'users/errors',
+ :locals => {:user => @user}
+ flash[:error] = error_message.html_safe
+
render :edit
end
else
View
@@ -37,11 +37,16 @@ class User
key :always_send_to_twitter, Integer, :default => 1
validate :email_already_confirmed
- validates_uniqueness_of :username, :allow_nil => :true, :case_sensitive => false
+ validates_uniqueness_of :username,
+ :allow_nil => :true,
+ :case_sensitive => false,
+ :message => "has already been taken."
# The maximum is arbitrary
# Twitter has 15, let's be different
- validates_length_of :username, :maximum => 17, :message => "must be 17 characters or fewer."
+ validates_length_of :username,
+ :maximum => 17,
+ :message => "must be 17 characters or fewer."
# Validate users don't have special characters in their username
validate :no_malformed_username
@@ -333,35 +338,59 @@ def self.authenticate(username, pass)
end
# Edit profile information
- def edit_user_profile(params)
- unless params[:password].nil? or params[:password].empty?
+ def update_profile!(params)
+
+ params[:email] = nil if params[:email].blank?
+
+ self.username = params[:username]
+
+ self.email_confirmed = self.email == params[:email]
+ self.email = params[:email]
+
+ self.always_send_to_twitter = params[:user] && params[:user][:always_send_to_twitter].to_i
+
+ # 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
- return "Passwords must match"
+ self.errors.add(:password, "doesn't match confirmation.")
end
end
- self.email_confirmed = (self.email == params[:email])
- self.email = params[:email]
-
- self.always_send_to_twitter = params[:user] && params[:user][:always_send_to_twitter].to_i
-
- self.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
- author.name = params[:name]
- author.email = params[:email]
- author.website = params[:website]
- author.bio = params[:bio]
- author.save
+ author.username = params[:username]
+ author.name = params[:name]
+ author.email = params[:email]
+ author.website = params[:website]
+ author.bio = params[:bio]
+ author.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
+ # 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 true
+ return self
+ end
end
# A better name would be very welcome.
@@ -397,9 +426,10 @@ def no_malformed_username
end
def email_already_confirmed
+ return if self.email.blank?
if User.where(:email => self.email,
- :email_confirmed => true,
- :username.ne => self.username).count > 0
+ :email_confirmed => true,
+ :id.ne => self.id).count > 0
errors.add(:email, "is already taken.")
end
end
@@ -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
@@ -4,10 +4,11 @@
= render :partial => "shared/sidebar/gravatar", :locals => {:user => @user}
#profile
- %h3= "Edit Profile for @#{@user.username}"
+ %h3= "Edit Profile for @#{current_user.username}"
- = form_for @user, :html => {:name => "profile_update_form", :id => "profile-update"} do |f|
+ = form_for current_user, :html => {:name => "profile_update_form", :id => "profile-update"} do |f|
.form-section
+ = render :partial => "shared/field", :locals => {:obj => @user.author, :attr => :username, :params => params}
= render :partial => "shared/field", :locals => {:obj => @user.author, :attr => :name, :params => params}
.form-section
@@ -15,9 +15,10 @@
assert has_link? "Edit"
end
- attributes_without_confirmation = {"name" => "Mark Zuckerberg",
- "website" => "http://test.com",
- "bio" => "To be or not to be"}
+ attributes_without_confirmation = {"username" => "foobar",
+ "name" => "Mark Zuckerberg",
+ "website" => "http://test.com",
+ "bio" => "To be or not to be"}
attributes_without_confirmation.each do |key, value|
it "updates your #{key}" do
@@ -34,6 +35,44 @@
end
end
+ it "does not update your username if the chosen username already exists" do
+ visit "/users/#{@u.username}/edit"
+
+ u = Fabricate(:user, :username => "foobar")
+
+ fill_in "username", :with => "foobar"
+
+ click_button "Save"
+
+ within flash do
+ assert has_content?("Username has already been taken")
+ end
+ end
+
+ it "redirects to your new name when you change your username" do
+ visit "/users/#{@u.username}/edit"
+
+ fill_in "username", :with => "foobar"
+
+ VCR.use_cassette("update_profile_username") do
+ click_button "Save"
+ end
+
+ assert_match /\/users\/foobar$/, page.current_url
+ end
+
+ it "does not allow you to change your username to something invalid" do
+ visit "/users/#{@u.username}/edit"
+
+ fill_in "username", :with => "#foobar."
+
+ click_button "Save"
+
+ within flash do
+ assert has_content?("Username contains restricted characters")
+ end
+ end
+
it "updates your password successfully" do
visit "/users/#{@u.username}/edit"
@@ -54,17 +93,34 @@
fill_in "password", :with => "new_password"
fill_in "password_confirm", :with => "bunk"
- VCR.use_cassette("update_profile_password_mismatch") do
- click_button "Save"
- end
+ click_button "Save"
within flash do
- assert has_content?("Profile could not be saved: Passwords must match")
+ 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"
+
+ click_button "Save"
+
+ 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"
@@ -179,6 +235,7 @@
u = Fabricate(:user, :username => "LADY_GAGA")
a = Fabricate(:authorization, :user => u)
log_in(u, a.uid)
+
visit "/users/lady_gaga/edit"
bio_text = "To be or not to be"
fill_in "bio", :with => bio_text
@@ -206,4 +263,4 @@
page.current_url.wont_match(/\/users\/#{u.username}\/edit/)
end
end
-end
+end
@@ -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,0 +1,26 @@
+---
+- !ruby/struct:VCR::HTTPInteraction
+ request: !ruby/struct:VCR::Request
+ method: :post
+ uri: http://rstatus.superfeedr.com:80/
+ body: hub.mode=publish&hub.url=http%3A%2F%2Ffoo.example.com%2Ffeeds%2F50a5845865588f0a03000b96.atom
+ headers:
+ content-type:
+ - application/x-www-form-urlencoded
+ response: !ruby/struct:VCR::Response
+ status: !ruby/struct:VCR::ResponseStatus
+ code: 204
+ message: No Content
+ headers:
+ date:
+ - Fri, 16 Nov 2012 00:15:32 GMT
+ status:
+ - 204 No Content
+ x-runtime:
+ - '2'
+ set-cookie:
+ - ''
+ cache-control:
+ - no-cache
+ body:
+ http_version: '1.1'
Oops, something went wrong. Retry.

0 comments on commit 0295cac

Please sign in to comment.