Permalink
Browse files

throw 404s when the person is no found

  • Loading branch information...
1 parent 69619f9 commit e9d993b8f66d2cd9ca2f8d6f618f334bb4c3f071 @maxwell maxwell committed Aug 9, 2011
@@ -9,6 +9,10 @@ class PeopleController < ApplicationController
respond_to :json, :only => [:index, :show]
respond_to :js, :only => [:tag_index]
+ rescue_from ActiveRecord::RecordNotFound do
+ render :file => "#{Rails.root}/public/404.html", :layout => false, :status => 404
+ end
+
def index
@aspect = :search
params[:q] ||= params[:term] || ''
@@ -61,64 +65,59 @@ def hashes_for_people people, aspects
end
def show
- @person = find_person_from_id_or_username
- if @person && @person.remote? && !user_signed_in?
- render :file => "#{Rails.root}/public/404.html", :layout => false, :status => 404
- return
+ @person = Person.find_from_id_or_username(params)
+
+ if remote_profile_with_no_user_session?
+ raise ActiveRecord::RecordNotFound
end
@post_type = :all
@aspect = :profile
@share_with = (params[:share_with] == 'true')
max_time = params[:max_time] ? Time.at(params[:max_time].to_i) : Time.now
- if @person
- @profile = @person.profile
-
- unless params[:format] == "json" # hovercard
- if current_user
- @contact = current_user.contact_for(@person)
- @aspects_with_person = []
- if @contact && !params[:only_posts]
- @aspects_with_person = @contact.aspects
- @aspect_ids = @aspects_with_person.map(&:id)
- @contacts_of_contact_count = @contact.contacts.count
- @contacts_of_contact = @contact.contacts.limit(8)
-
- else
- @contact ||= Contact.new
- @contacts_of_contact_count = 0
- @contacts_of_contact = []
- end
-
- if (@person != current_user.person) && !@contact.persisted?
- @commenting_disabled = true
- else
- @commenting_disabled = false
- end
- @posts = current_user.posts_from(@person).where(:type => ["StatusMessage", "Reshare", "ActivityStreams::Photo"]).includes(:comments).limit(15).where(StatusMessage.arel_table[:created_at].lt(max_time))
+ @profile = @person.profile
+
+ unless params[:format] == "json" # hovercard
+ if current_user
+ @contact = current_user.contact_for(@person)
+ @aspects_with_person = []
+ if @contact && !params[:only_posts]
+ @aspects_with_person = @contact.aspects
+ @aspect_ids = @aspects_with_person.map(&:id)
+ @contacts_of_contact_count = @contact.contacts.count
+ @contacts_of_contact = @contact.contacts.limit(8)
+
else
- @commenting_disabled = true
- @posts = @person.posts.where(:type => ["StatusMessage", "Reshare", "ActivityStreams::Photo"], :public => true).includes(:comments).limit(15).where(StatusMessage.arel_table[:created_at].lt(max_time)).order('posts.created_at DESC')
+ @contact ||= Contact.new
+ @contacts_of_contact_count = 0
+ @contacts_of_contact = []
end
- @posts.includes(:author => :profile)
- end
- if params[:only_posts]
- render :partial => 'shared/stream', :locals => {:posts => @posts}
- else
- respond_to do |format|
- format.all { respond_with @person, :locals => {:post_type => :all} }
- format.json {
- render :json => @person.to_json(:includes => params[:includes])
- }
+ if (@person != current_user.person) && !@contact.persisted?
+ @commenting_disabled = true
+ else
+ @commenting_disabled = false
end
+ @posts = current_user.posts_from(@person).where(:type => ["StatusMessage", "Reshare", "ActivityStreams::Photo"]).includes(:comments).limit(15).where(StatusMessage.arel_table[:created_at].lt(max_time))
+ else
+ @commenting_disabled = true
+ @posts = @person.posts.where(:type => ["StatusMessage", "Reshare", "ActivityStreams::Photo"], :public => true).includes(:comments).limit(15).where(StatusMessage.arel_table[:created_at].lt(max_time)).order('posts.created_at DESC')
end
+ @posts.includes(:author => :profile)
+ end
+ if params[:only_posts]
+ render :partial => 'shared/stream', :locals => {:posts => @posts}
else
- flash[:error] = I18n.t 'people.show.does_not_exist'
- redirect_to people_path
+ respond_to do |format|
+ format.all { respond_with @person, :locals => {:post_type => :all} }
+ format.json {
+ render :json => @person.to_json(:includes => params[:includes])
+ }
+ end
end
+
end
def retrieve_remote
@@ -156,13 +155,8 @@ def webfinger(account, opts = {})
Resque.enqueue(Job::SocketWebfinger, current_user.id, account, opts)
end
- def find_person_from_id_or_username
- if params[:id].present?
- Person.where(:id => params[:id]).first
- elsif params[:username].present?
- User.find_by_username(params[:username]).person
- else
- nil
- end
+
+ def remote_profile_with_no_user_session?
+ @person && @person.remote? && !user_signed_in?
end
end
View
@@ -49,6 +49,20 @@ def downcase_diaspora_handle
def self.featured_users
AppConfig[:featured_users].present? ? Person.where(:diaspora_handle => AppConfig[:featured_users]) : []
end
+
+
+ def self.find_from_id_or_username(params)
+ p = if params[:id].present?
+ Person.where(:id => params[:id]).first
+ elsif params[:username].present? && u = User.find_by_username(params[:username])
+ u.person
+ else
+ nil
+ end
+ raise ActiveRecord::RecordNotFound unless p.present?
+ p
+ end
+
def self.search_query_string(query)
query = query.downcase
View
@@ -12,13 +12,18 @@
border-right-color: #999;
border-bottom-color: #999;
}
- h1 { font-size: 100%; color: #f00; line-height: 1.5em; }
+ h1 { font-size: 100%; color: #f00; line-height: 1.5em; text-align:center; }
</style>
</head>
<body>
<!-- This file lives in public/404.html -->
- <img src="/images/404.png"/>
-
+ <h1> 404: Not Found </h1>
+ <a href="javascript:history.go(-1)">
+ <img src="/images/404.png"/>
+ </a>
+ <h1>
+ <a href="javascript:history.go(-1)"> Go Back </a>
+ </h1>
</body>
</html>
@@ -117,14 +117,14 @@
end
describe '#show' do
- it "redirects to #index if the id is invalid" do
+ it "404s if the id is invalid" do
get :show, :id => 'delicious'
- response.should redirect_to people_path
+ response.code.should == "404"
end
- it "redirects to #index if no person is found" do
+ it "404s if no person is found" do
get :show, :id => 3920397846
- response.should redirect_to people_path
+ response.code.should == "404"
end
it 'does not allow xss attacks' do
View
@@ -23,6 +23,28 @@
Person.remote =~ [@user.person]
end
end
+
+ describe '.find_person_from_id_or_username' do
+ it 'searchs for a person if id is passed' do
+ Person.find_from_id_or_username(:id => @person.id).id.should == @person.id
+ end
+
+ it 'searchs a person from a user if username is passed' do
+ Person.find_from_id_or_username(:username => @user.username).id.should == @user.person.id
+ end
+
+ it 'throws active record not found exceptions if no person is found via id' do
+ expect{
+ Person.find_from_id_or_username(:id => 213123)
+ }.to raise_error ActiveRecord::RecordNotFound
+ end
+
+ it 'throws active record not found exceptions if no person is found via username' do
+ expect{
+ Person.find_from_id_or_username(:username => 'michael_jackson')
+ }.to raise_error ActiveRecord::RecordNotFound
+ end
+ end
end
describe "delegating" do
it "delegates last_name to the profile" do

0 comments on commit e9d993b

Please sign in to comment.