Skip to content

Commit

Permalink
Adds functionality to change your username.
Browse files Browse the repository at this point in the history
This functionality affects the User controller and model and view.

In the model, I have replaced the behavior of the edit_user_profile
badness to just add freaking errors to self whenever something is
invalid instead of reporting a string since those errors contain
consistent error messages anyway. It does not return. I will reflect
this in another commit by changing the name to something more
appropriate.

Which means in the model, I have added messages to validations that for
some reason did not have one before. We should put those strings all in
one place eventually for sanity.

In the controller, I updated the, um, update action to look for errors
like a normal controller action would and respond naturally. Some
wonkiness occurs.

This wonkiness is a result of trying to change the field that the
current url is built from. Therefore, let's use the canonical user,
current_user, whenever possible. When redirecting, we have changed the
canonical user, therefore use @user to generate urls.

That wonkiness is in the view. Here, @user is the current image of the
user and current_user is the true user (or preimage, I guess.) So, using
current_user where (honestly, you would expect current_user anyway) you
want to use the stable values and @user when you want the changed fields
is what I've done.

Tests:

Updates tests to reflect changes in error messages.

Adds four acceptance tests to edit_profile_test:

* it updates your username (check that a username can simply be changed)
* it does not update your username if the chosen username exists (make
    sure that you cannot steal somebody else's name)
* it redirects to your new name when you change your username (the
    redirect should take you to your profile, which has a different url
    now)
* it does not allow you to change your username to something invalid (do
    not allow special characters in your name)

Adds one unit test:

* username can be changed (checks that the edit_user_profile method,
    which will become update_profile! actually updates the username with
    the given parameter)
  • Loading branch information
wilkie authored and carols10cents committed Nov 22, 2012
1 parent 5753359 commit eb2f34d
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 22 deletions.
10 changes: 6 additions & 4 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,19 +62,21 @@ def edit
def update
if @user == current_user
response = @user.edit_user_profile(params)
if response == true

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 = @user.errors.full_messages.first

flash[:error] = "Profile could not be saved: #{error_message}"

render :edit
end
else
Expand Down
31 changes: 20 additions & 11 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -341,29 +346,33 @@ def edit_user_profile(params)
self.password = params[:password]
self.save
else
return "Passwords must match"
self.errors.add(:password, "doesn't match confirmation.")
return
end
end

self.email_confirmed = (self.email == params[:email])
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

self.save
return unless self.save

author.name = params[:name]
author.email = params[:email]
author.website = params[:website]
author.bio = params[:bio]
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

return true
end

# A better name would be very welcome.
Expand Down
5 changes: 3 additions & 2 deletions app/views/users/edit.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 49 additions & 5 deletions test/acceptance/edit_profile_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -34,6 +35,48 @@
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"

VCR.use_cassette("update_profile_username") do
click_button "Save"
end

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."

VCR.use_cassette("update_profile_invalid_username") do
click_button "Save"
end

within flash do
assert has_content?("Username contains restricted characters")
end
end

it "updates your password successfully" do
visit "/users/#{@u.username}/edit"

Expand All @@ -59,7 +102,7 @@
end

within flash do
assert has_content?("Profile could not be saved: Passwords must match")
assert has_content?("Profile could not be saved: Password doesn't match confirmation.")
end

assert has_field?("password")
Expand Down Expand Up @@ -179,6 +222,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
Expand Down Expand Up @@ -206,4 +250,4 @@
page.current_url.wont_match(/\/users\/#{u.username}\/edit/)
end
end
end
end
9 changes: 9 additions & 0 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ def stub_superfeedr_request_for_user(user)
end

describe "username" do
it "can be changed" do
u = Fabricate(:user)

stub_superfeedr_request_for_user u

u.edit_user_profile(:username => 'foobar')
assert_equal 'foobar', u.username
end

it "must be unique" do
Fabricate(:user, :username => "steve")
u = Fabricate.build(:user, :username => "steve")
Expand Down

0 comments on commit eb2f34d

Please sign in to comment.