Permalink
Browse files

Factoring out User.find_by_case_insensitive_username and using it in …

…a bunch of places where we weren't but should have been.

The name's a bit long but it says what it's doing... an improved name would be much appreciated.
  • Loading branch information...
carols10cents committed Sep 10, 2011
1 parent 5b1c40a commit 26444ea95ec8da12d4e74764bf52bdaad18e7776
@@ -14,15 +14,15 @@ def index
end
def show
-
- user = User.first :username => /^#{Regexp.escape(params[:id])}$/i
+ user = User.find_by_case_insensitive_username(params[:id])
if user.nil?
render :file => "#{Rails.root}/public/404.html", :status => 404
elsif user.username != params[:id] # case difference
redirect_to "/users/#{user.username}"
else
set_params_page
+
@author = user.author
@updates = user.updates
@updates = @updates.paginate(:page => params[:page], :per_page => params[:per_page])
@@ -34,7 +34,7 @@ def show
end
def edit
- @user = User.first :username => params[:id]
+ @user = User.find_by_case_insensitive_username(params[:id])
# While it might be cool to edit other people's profiles, we probably
# shouldn't let you do that. We're no fun.
@@ -46,7 +46,7 @@ def edit
end
def update
- @user = User.first :username => params[:id]
+ @user = User.find_by_case_insensitive_username(params[:id])
if @user == current_user
response = @user.edit_user_profile(params)
if response == true
@@ -123,61 +123,78 @@ def create_from_email
# Since it's only two lines, we don't bother to do a redirect, and
# it's arguably better to display them as two different resources.
# Whatevs.
+ # Except we ARE doing a redirect??? /me shakes fist at steve
def feed
- feed = User.first(:username => params[:id]).feed
- redirect_to "/feeds/#{feed.id}.atom"
+ user = User.find_by_case_insensitive_username(params[:id])
+ if user
+ redirect_to "/feeds/#{user.feed.id}.atom"
+ else
+ render :file => "#{Rails.root}/public/404.html", :status => 404
+ end
end
# Who do you think is a really neat person? This page will show it to the
# world, so pick wisely!
def following
- set_params_page
+ @user = User.find_by_case_insensitive_username(params[:id])
- # XXX: case insensitive username
- @user = User.first(:username => params[:id])
- @feeds = @user.following
+ if @user.nil?
+ render :file => "#{Rails.root}/public/404.html", :status => 404
+ elsif @user.username != params[:id] # case difference
+ redirect_to "/users/#{@user.username}/following"
+ else
+ set_params_page
- @feeds = @feeds.paginate(:page => params[:page], :per_page => params[:per_page], :order => :id.desc)
+ @feeds = @user.following
- set_pagination_buttons(@feeds)
+ @feeds = @feeds.paginate(:page => params[:page], :per_page => params[:per_page], :order => :id.desc)
- @authors = @feeds.map{|f| f.author}
+ set_pagination_buttons(@feeds)
- if @user == current_user
- title = "You're following"
- else
- title = "@#{@user.username} is following"
- end
+ @authors = @feeds.map{|f| f.author}
+
+ if @user == current_user
+ title = "You're following"
+ else
+ title = "@#{@user.username} is following"
+ end
- respond_to do |format|
- format.html { render "users/list", :locals => {:title => title} }
- format.json { render :json => @authors }
+ respond_to do |format|
+ format.html { render "users/list", :locals => {:title => title} }
+ format.json { render :json => @authors }
+ end
end
end
# This shows off how cool you are: I hope you've got the biggest number of
# followers. Only one way to find out...
def followers
- set_params_page
+ @user = User.find_by_case_insensitive_username(params[:id])
- # XXX: case insensitive username
- @user = User.first(:username => params[:id])
- @feeds = @user.followers
+ if @user.nil?
+ render :file => "#{Rails.root}/public/404.html", :status => 404
+ elsif @user.username != params[:id] # case difference
+ redirect_to "/users/#{@user.username}/followers"
+ else
+ set_params_page
- @feeds = @feeds.paginate(:page => params[:page], :per_page => params[:per_page], :order => :id.desc)
+ @feeds = @user.followers
- set_pagination_buttons(@feeds)
+ @feeds = @feeds.paginate(:page => params[:page], :per_page => params[:per_page], :order => :id.desc)
- @authors = @feeds.map{|f| f.author}
+ set_pagination_buttons(@feeds)
- #build title
- if @user == current_user
- title = "Your followers"
- else
- title = "@#{@user.username}'s followers"
- end
+ @authors = @feeds.map{|f| f.author}
+
+ #build title
+ if @user == current_user
+ title = "Your followers"
+ else
+ title = "@#{@user.username}'s followers"
+ end
- render "users/list", :locals => {:title => title}
+ render "users/list", :locals => {:title => title}
+ end
end
def confirm_email
@@ -226,7 +243,6 @@ def forgot_password_confirm
render "login/forgot_password_confirm"
end
-
def reset_password_new
if not logged_in?
redirect_to "/forgot_password"
@@ -282,7 +298,6 @@ def reset_password_create
end
end
-
# Public reset password page, accessible via a valid token. Tokens are only
# valid for 2 days and are unique to that user. The user is found using the
# token and the reset password page is rendered
View
@@ -384,6 +384,11 @@ def edit_user_profile(params)
return true
end
+ # A better name would be very welcome.
+ def self.find_by_case_insensitive_username(username)
+ User.first(:username => /^#{Regexp.escape(username)}$/i)
+ end
+
private
def same_email?(email_param)
@@ -0,0 +1,18 @@
+require 'require_relative' if RUBY_VERSION[0,3] == '1.8'
+require_relative 'acceptance_helper'
+
+describe "feeds" do
+ include AcceptanceHelper
+
+ it "redirects to the username's atom feed with the right case" do
+ u = Factory(:user)
+ url = "http://www.example.com/feeds/#{u.feed.id}.atom"
+ visit "/users/#{u.username.upcase}/feed"
+ assert_equal url, page.current_url
+ end
+
+ it "404s if that username doesnt exist" do
+ visit "/users/nonexistent/feed"
+ assert_match "The page you were looking for doesn't exist.", page.body
+ end
+end
@@ -153,6 +153,19 @@
assert_match "#{u.username} is following", page.body
end
+ it "redirects to the correct case" do
+ u = Factory(:user, :username => "dfnkt")
+
+ visit "/users/#{u.username.upcase}/following"
+ assert_match "#{u.username} is following", page.body
+ assert_match /\/users\/#{u.username}\/following$/, page.current_url
+ end
+
+ it "404s if the requested user does not exist" do
+ visit "/users/nonexistent/following"
+ assert_match "The page you were looking for doesn't exist.", page.body
+ end
+
it "has a nice message if not following anyone" do
u = Factory(:user, :username => "dfnkt")
@@ -241,6 +254,19 @@
assert_match "#{u.username}'s followers", page.body
end
+ it "redirects to the correct case" do
+ u = Factory(:user, :username => "dfnkt")
+
+ visit "/users/#{u.username.upcase}/followers"
+ assert_match "#{u.username}'s followers", page.body
+ assert_match /\/users\/#{u.username}\/followers$/, page.current_url
+ end
+
+ it "404s if the requested user does not exist" do
+ visit "/users/nonexistent/followers"
+ assert_match "The page you were looking for doesn't exist.", page.body
+ end
+
it "has a nice message if not followed by anyone" do
u = Factory(:user, :username => "dfnkt")
@@ -53,4 +53,22 @@
assert_match page.body, /#{bio_text}/
end
+
+ it "doesn't let you update someone else's profile" do
+ u = Factory(:user)
+ visit "/users/#{u.username}/edit"
+ assert_match /\/users\/#{u.username}$/, page.current_url
+ end
+
+ it "does let you update your profile even if you use a different case in the url" do
+ u = Factory(:user, :username => "LADY_GAGA")
+ a = Factory(: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
+ click_button "Save"
+
+ assert_match page.body, /#{bio_text}/
+ end
end
View
@@ -285,4 +285,26 @@ def stub_superfeedr_request_for_user(user)
refute u.timeline.include? u2_update
end
end
+
+ describe "self#find_by_case_insensitive_username" do
+ before do
+ @u = Factory.create(:user, :username => "oMg")
+ end
+
+ it "returns the user if we use the same case" do
+ assert_equal @u, User.find_by_case_insensitive_username("oMg")
+ end
+
+ it "returns the user if we use a different case" do
+ assert_equal @u, User.find_by_case_insensitive_username("OmG")
+ end
+
+ it "returns nil if no user matches" do
+ assert_equal nil, User.find_by_case_insensitive_username("blah")
+ end
+
+ it "escapes regex chars so that regexing isnt allowed" do
+ assert_equal nil, User.find_by_case_insensitive_username(".mg")
+ end
+ end
end

0 comments on commit 26444ea

Please sign in to comment.