myexperiment-hackers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[myexperiment-hackers] [3516] trunk/app/controllers: Added more appropri


From: noreply
Subject: [myexperiment-hackers] [3516] trunk/app/controllers: Added more appropriate HTTP status codes to various controllers
Date: Mon, 22 Apr 2013 13:18:30 +0000 (UTC)

Revision
3516
Author
fbacall
Date
2013-04-22 13:18:30 +0000 (Mon, 22 Apr 2013)

Log Message

Added more appropriate HTTP status codes to various controllers

Modified Paths

Diff

Modified: trunk/app/controllers/pictures_controller.rb (3515 => 3516)


--- trunk/app/controllers/pictures_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/pictures_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -130,16 +130,4 @@
   def find_user
     @user = User.find_by_id(params[:user_id])
   end
-
-private
-  
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-    (err = Picture.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to logged_in? ? user_pictures_url(current_user) : '' }
-    end
-  end
 end
-

Modified: trunk/app/controllers/previews_controller.rb (3515 => 3516)


--- trunk/app/controllers/previews_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/previews_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -19,14 +19,14 @@
       user = User.authenticate(credentials[0], credentials[1])
 
       if user.nil?
-        render :nothing => true, :status => "401 Unauthorized"
+        render :nothing => true, :status => 401
         response.headers['WWW-Authenticate'] = "Basic realm=\"#{Conf.sitename} REST API\""
         return
       end
     end
 
     if @context.preview.nil?
-      render :nothing => true, :status => "404 Not Found"
+      render :nothing => true, :status => 404
       return
     end
 
@@ -37,7 +37,7 @@
     end
 
     if Authorization.check('view', auth_object, user) == false
-      render :nothing => true, :status => "401 Unauthorized"
+      render :nothing => true, :status => 401
       response.headers['WWW-Authenticate'] = "Basic realm=\"#{Conf.sitename} REST API\""
       return
     end
@@ -53,7 +53,7 @@
       when 'thumb';  source = 'image'; size = 100; mime_type = 'image/jpeg'
       when 'svg';    source = 'svg';   size = nil; mime_type = 'image/svg+xml'
       else
-        render(:inline => 'Bad preview type', :status => "400 Bad Request")
+        render(:inline => 'Bad preview type', :status => 400)
         return
     end
 
@@ -70,7 +70,7 @@
     end
 
     if content_blob.nil?
-      render :nothing => true, :status => "404 Not Found"
+      render :nothing => true, :status => 404
       return
     end
 
@@ -100,14 +100,13 @@
 
   def find_context
     @context = extract_resource_context(params)
-    return error unless @context
-
-    @context = @context.find_version(params[:version]) if params[:version]
-    return error unless @context
+    if @context.nil?
+      render_404("Resource not found.")
+    elsif params[:version]
+      @context = @context.find_version(params[:version])
+      if @context.nil?
+        render_404("Resource version not found.")
+      end
+    end
   end
-
-  def error
-    render :text => 'Error.'
-  end
 end
-

Modified: trunk/app/controllers/relationships_controller.rb (3515 => 3516)


--- trunk/app/controllers/relationships_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/relationships_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -7,7 +7,7 @@
   
   helper PacksHelper
 
-  before_filter :find_resource_context
+  before_filter :find_and_auth_resource_context
   before_filter :find_resource, :except => [ :edit_relationships, :create ]
 
   # GET /:context_type/:context_id/edit_relationships
@@ -63,25 +63,21 @@
 
   private
 
-  def find_resource
+  def find_and_auth_resource_context
+    @context = extract_resource_context(params)
 
-    @context      = extract_resource_context(params)
-    @relationship = Relationship.find_by_id(params[:id])
-
-    return error if @relationship.nil? || @context.nil? || @relationship.context != @context
-    return error if Authorization.check('view', @context, current_user) == false
+    if @context.nil?
+      render_404("Relationship context not found.")
+    elsif !Authorization.check('view', @context, current_user)
+      render_401("You are not authorized to view this resource's relationships.")
+    end
   end
 
-  def find_resource_context
+  def find_resource
+    @relationship = Relationship.find_by_id(params[:id])
 
-    @context = extract_resource_context(params)
-
-    return false if @context.nil?
-    return false if Authorization.check('view', @context, current_user) == false
+    if @relationship.nil? || @relationship.context != @context
+      render_404("Relationship not found.")
+    end
   end
-
-  def error
-    render :text => 'Error.'
-  end
 end
-

Modified: trunk/app/controllers/reviews_controller.rb (3515 => 3516)


--- trunk/app/controllers/reviews_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/reviews_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -15,8 +15,8 @@
   before_filter :find_reviewable_auth
   
   before_filter :find_reviews, : [ :index ]
-  before_filter :find_review, : [ :show ]
-  before_filter :find_review_auth, : [ :edit, :update, :destroy ]
+  before_filter :find_review, : [ :show, :edit, :update, :destroy ]
+  before_filter :auth_review, : [ :edit, :update, :destroy ]
   
   # declare sweepers and which actions should invoke them
   cache_sweeper :review_sweeper, : [ :create, :update, :delete ]
@@ -123,28 +123,15 @@
   def find_reviewable_auth
     # IMPORTANT NOTE: currently the only reviewable supported is "Workflow".
     # See note at the beginning of this controller for more info.
- 
-    begin
-      # attempt to authenticate the user before you return the reviewable
-      login_required if login_available?
-    
-      workflow = Workflow.find(params[:workflow_id])
-      
-      if Authorization.check('view', workflow, current_user)
-        # remove workflow data from workflow if the user is not authorized for download
-        workflow.content_blob.data = "" unless Authorization.check('download', workflow, current_user)
-        @reviewable = workflow
-      else
-        if logged_in?
-          error("Workflow not found (id not authorized)", "is invalid (not authorized)")
-          return
-        else
-          login_required
-        end
-      end
-    rescue ActiveRecord::RecordNotFound
-      error("Workflow not found", "is invalid")
-      return
+    @reviewable = Workflow.find_by_id(params[:workflow_id])
+
+    if @reviewable.nil?
+      render_404("Workflow not found.")
+    elsif !Authorization.check('view', @reviewable, current_user)
+      # remove workflow data from workflow if the user is not authorized for download
+      render_401("You are not authorized to review this workflow.")
+    else
+      @reviewable.content_blob.data = "" unless Authorization.check('download', @reviewable, current_user)
     end
   end
   
@@ -160,39 +147,13 @@
     if review = @reviewable.reviews.find(:first, :conditions => ["id = ?", params[:id]])
       @review = review
     else
-      error("Review not found", "is invalid")
-      return
+      render_404("Review not found.")
     end
   end
   
-  def find_review_auth
-    if review = @reviewable.reviews.find(:first, :conditions => ["id = ? AND user_id = ?", params[:id], current_user.id])
-      @review = review
-    else
-      error("Review not found or action not authorized", "is invalid (not authorized)")
-      return
+  def auth_review
+    unless @review.user == current_user
+      render_401("You are not authorized to #{action_name} this review.")
     end
   end
-  
-private
-
-  def error(notice, message, attr = nil)
-    flash[:error] = notice
-
-    workflow_id_attr = attr
-    workflow_id_attr = :id if workflow_id_attr.nil?
-
-    (err = Review.new.errors).add(workflow_id_attr, message)
-    
-    respond_to do |format|
-      format.html {
-        if attr
-          redirect_to workflow_reviews_url(params[:workflow_id])
-        else
-          redirect_to workflows_url
-        end
-      }
-    end
-  end
 end
-

Modified: trunk/app/controllers/runners_controller.rb (3515 => 3516)


--- trunk/app/controllers/runners_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/runners_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -136,23 +136,12 @@
       "verify"  => "view"
     }
 
-    runner = TavernaEnactor.find(:first, :conditions => ["id = ?", params[:id]])
-    
-    if runner and Authorization.check(action_permissions[action_name], runner, current_user)
-      @runner = runner
-    else
-      error("Runner not found or action not authorized", "is invalid (not authorized)")
-    end
-  end
-  
-private
+    @runner = TavernaEnactor.find_by_id(params[:id])
 
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-    (err = TavernaEnactor.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to runners_url }
+    if @runner.nil?
+      render_404("Runner not found.")
+    elsif !Authorization.check(action_permissions[action_name], @runner, current_user)
+      render_401("You are not authorized to #{action_permissions[action_name]} this runner.")
     end
   end
 end

Modified: trunk/app/controllers/search_controller.rb (3515 => 3516)


--- trunk/app/controllers/search_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/search_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -20,8 +20,11 @@
     @type = "all" if @type.nil? or @type == ""
 
     if !Conf.search_categories.include?(@type)
-      error(@type)
-      return
+      flash[:error] = "'#{type}' is an invalid search type"
+
+      respond_to do |format|
+        format.html { redirect_to url_for(:controller => "home") }
+      end
     end
 
     if Conf.model_aliases.key?(@type.camelize.singularize)
@@ -181,14 +184,6 @@
 
 private
 
-  def error(type)
-    flash[:error] = "'#{type}' is an invalid search type"
-    
-    respond_to do |format|
-      format.html { redirect_to url_for(:controller => "home") }
-    end
-  end
-
   def search_all
 
     @query = params[:query]

Modified: trunk/app/controllers/services_controller.rb (3515 => 3516)


--- trunk/app/controllers/services_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/services_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -71,19 +71,7 @@
       @contributable_path                 = service_path(@contributable)
 
     rescue ActiveRecord::RecordNotFound
-      error("Service not found", "is invalid")
+      render_404("Service not found.")
     end
   end
-  
-  private
-  
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-     (err = Service.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to services_url }
-    end
-  end
 end
-

Modified: trunk/app/controllers/tags_controller.rb (3515 => 3516)


--- trunk/app/controllers/tags_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/tags_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -59,7 +59,7 @@
   end
   
   def find_tag_and_tagged_with
-    @tag = Tag.find(:first, :conditions => ["id = ?", params[:id]])
+    @tag = Tag.find_by_id(params[:id])
     
     if @tag
       @tagged_with = []
@@ -87,21 +87,12 @@
       
       @tagged_with = @tagged_with.uniq
     else
-      error("Tag not found", "is invalid")
+      render_404("Tag not found.")
     end
   end
   
 private
 
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-    (err = Tag.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to tags_url }
-    end
-  end
-  
   # This needs to be refactored into a library somewhere!
   # (eg: a myExperiment system library)
   def parse_to_internal_type(type)

Modified: trunk/app/controllers/topics_controller.rb (3515 => 3516)


--- trunk/app/controllers/topics_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/topics_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -5,7 +5,7 @@
 
 class TopicsController < ApplicationController
   before_filter :login_required, :except => [:index, :show]
-  
+  before_filter :find_topic, : [:tag_feedback, :topic_feedback]
 
   # declare sweepers and which actions should invoke them
   cache_sweeper :workflow_sweeper, : [ :create, :create_version, :launch, :update, :update_version, :destroy ]
@@ -39,100 +39,75 @@
   end
   
   def tag_feedback
-    # Ensure that all the needed information was provided
-    if params[:topic_id].blank? || params[:user_id].blank? || params[:tag_id].blank? || params[:score].blank?
-      error("Malformed feedback information.", "")
-    else
-      this_topic = Topic.find(params[:topic_id]) rescue error("Invalid topic specified.")	
-      #Ensure the feedback is for the current user
-      if params[:user_id].to_i != current_user.id
-        error("You may only post feedback as yourself.", "")
-        return
-      end
-      # Not allowed to create duplicate feedback
-      if this_topic.topic_tag_feedback.exists?( :user_id => params[:user_id], :tag_id => params[:tag_id] )
-        error( "You may provide feedback only once per topic, tag pair.", "")
-        return
-      end
+    this_topic = @topic
+
+    # Not allowed to create duplicate feedback
+    unless feedback = this_topic.topic_tag_feedback.find_by_user_id_and_tag_id(current_user.id, params[:tag_id])
       #Create the feedback
       feedback = this_topic.topic_tag_feedback.build( :score => params[:score], :submit_dt => Time.new.utc )
-      feedback.user = User.find(params[:user_id])
+      feedback.user = current_user
       feedback.tag = Tag.find(params[:tag_id])
       feedback.save
-      #topic_id => params[:topic_id], :user_id => params[:user_id], :tag_id => params[:tag_id], 
-         
-      # What if the tag doesn't exist?
-      
-      respond_to do |response|
-        # page.html {redirect_to topics_path }
-        response.html {
-          render :update  do |page|
-            page.replace_html "tag_feedback_div_#{params[:topic_id]}_#{params[:tag_id]}", "Thanks!"
-          end
-        }
-      end
     end
+
+    #topic_id => params[:topic_id], :user_id => params[:user_id], :tag_id => params[:tag_id],
+
+    # What if the tag doesn't exist?
+
+    respond_to do |response|
+      # page.html {redirect_to topics_path }
+      response.html {
+        render :update  do |page|
+          page.replace_html "tag_feedback_div_#{params[:topic_id]}_#{params[:tag_id]}", "Thanks!"
+        end
+      }
+    end
   end
   
   def topic_feedback
-  	# Ensure that all the needed information was provided
-    if params[:topic_id].blank? || params[:user_id].blank? || params[:score].blank?
-      error("Malformed feedback information.", "")
-    else
-      this_topic = Topic.find(params[:topic_id]) rescue error("Invalid topic specified.")	
-      #Ensure the feedback is for the current user
-      if params[:user_id].to_i != current_user.id
-        error("You may only post feedback as yourself.", "")
-        return
-      end
-      # Not allowed to create duplicate feedback
-      if this_topic.topic_feedback.exists?( :user_id => params[:user_id] )
-        error( "You may provide feedback only once per topic.", "")
-        return
-      end
+    this_topic = @topic
+    # Not allowed to create duplicate feedback
+    unless feedback = this_topic.topic_feedback.find_by_user_id(current_user.id)
       #Create the feedback
       feedback = this_topic.topic_feedback.build( :score => params[:score], :submit_dt => Time.new.utc )
-      feedback.user = User.find(params[:user_id])
+      feedback.user = current_user
       feedback.save
-      
-      respond_to do |response|
-        # page.html {redirect_to topics_path }
-        response.html {
-          render :update  do |page|
+    end
 
-            if
-              this_topic.name.blank? 
-            then
-              topicName = "Explore this topic" 
-            else
-              topicName = this_topic.name 
-            end
+    respond_to do |response|
+      # page.html {redirect_to topics_path }
+      response.html {
+        render :update  do |page|
 
-            topicLink = "<a href=""
+          if
+            this_topic.name.blank?
+          then
+            topicName = "Explore this topic"
+          else
+            topicName = this_topic.name
+          end
 
-            if feedback.score == 1
-              img_url = 'images/thumbsup_grey.png'
-            else
-              img_url = 'images/thumbsdown_grey.png'
-            end
+          topicLink = "<a href=""
 
-            page.replace_html "topic_feedback_#{params[:topic_id]}", "<img src=''>"
+          if feedback.score == 1
+            img_url = 'images/thumbsup_grey.png'
+          else
+            img_url = 'images/thumbsdown_grey.png'
           end
-        }
-      end
+
+          page.replace_html "topic_feedback_#{params[:topic_id]}", "<img src=''>"
+        end
+      }
     end
   end
 
 private
 
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-    (err = Workflow.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to topics_url }
+  def find_topic
+    @topic = Topic.find_by_id(params[:topic_id])
+
+    if @topic.nil?
+      render_404("Topic not found.")
     end
   end
-  
 end
-

Modified: trunk/app/controllers/userhistory_controller.rb (3515 => 3516)


--- trunk/app/controllers/userhistory_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/userhistory_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -27,26 +27,13 @@
 
   def find_user
     if params[:user_id]
-      begin
-        @user = User.find(params[:user_id])
-    
-      rescue ActiveRecord::RecordNotFound
-        error("User not found", "is invalid", :user_id)
-      end
+      @user = User.find_by_id(params[:user_id])
     else
-      @user = User.find(params[:id])
+      @user = User.find_by_id(params[:id])
     end
-  end
 
-private
-
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-    (err = User.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to users_url }
+    if @user.nil?
+      render_404("User not found.")
     end
   end
-  
 end

Modified: trunk/app/controllers/users_controller.rb (3515 => 3516)


--- trunk/app/controllers/users_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/users_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -14,9 +14,9 @@
   before_filter :login_required, :except => [:index, :new, :create, :search, :all, :confirm_email, :forgot_password, :reset_password] + show_actions
   
   before_filter :find_users, : [:all]
-  before_filter :find_user, : [:destroy] + show_actions
-  before_filter :find_user_auth, : [:edit, :update]
-  
+  before_filter :find_user, : [:destroy, :edit, :update] + show_actions
+  before_filter :auth_user, : [:edit, :update]
+
   # declare sweepers and which actions should invoke them
   cache_sweeper :user_sweeper, : [ :create, :update, :destroy ]
   
@@ -764,52 +764,16 @@
   end
 
   def find_user
-    begin
-      @user = User.find(params[:id], :include => [ :profile, :tags ])
-    rescue ActiveRecord::RecordNotFound
-      error("User not found", "is invalid (not owner)")
-      return
-    end
-    
-    unless @user
-      error("User not found", "is invalid (not owner)")
-      return
-    end
-    
-    unless @user.activated?
-      error("User not activated", "is invalid (not owner)")
-      return
-    end
-  end
+    @user = User.find_by_id(params[:id], :include => [ :profile, :tags ])
 
-  def find_user_auth
-    begin
-      @user = User.find(params[:id], :conditions => ["id = ?", current_user.id])
-    rescue ActiveRecord::RecordNotFound
-      error("User not found (id not authorized)", "is invalid (not owner)")
-      return
+    if @user.nil? || address@hidden
+      render_404("User not found, or not activated.")
     end
-    
-    unless @user
-      error("User not found (or not authorized)", "is invalid (not owner)")
-      return
-    end
-    
-    unless @user.activated?
-      error("User not activated (id not authorized)", "is invalid (not owner)")
-      return
-    end
   end
-  
-private
 
-  def error(notice, message)
-    flash[:error] = notice
-    (err = User.new.errors).add(:id, message)
-    
-    respond_to do |format|
-      format.html { redirect_to users_url }
+  def auth_user
+    unless @user == current_user
+      render_401("You may only manage your own account.")
     end
   end
 end
-

Modified: trunk/app/controllers/workflows_controller.rb (3515 => 3516)


--- trunk/app/controllers/workflows_controller.rb	2013-04-22 12:55:24 UTC (rev 3515)
+++ trunk/app/controllers/workflows_controller.rb	2013-04-22 13:18:30 UTC (rev 3516)
@@ -76,9 +76,7 @@
   
   # POST /workflows/1;rate
   def rate
-    if @workflow.contribution.contributor_type == 'User' and @workflow.contribution.contributor_id == current_user.id
-      error("You cannot rate your own workflow!", "")
-    else
+    unless @workflow.contribution.contributor_type == 'User' and @workflow.contribution.contributor_id == current_user.id
       Rating.delete_all(["rateable_type = ? AND rateable_id = ? AND user_id = ?", @workflow.class.to_s, @workflow.id, current_user.id])
       
       rating = Rating.create(:rateable => @workflow, :user => current_user, :rating => params[:rating])
@@ -927,15 +925,6 @@
     end
   end
 
-  def error(notice, message, attr=:id)
-    flash[:error] = notice
-    (err = Workflow.new.errors).add(attr, message)
-    
-    respond_to do |format|
-      format.html { redirect_to workflows_url }
-    end
-  end
-  
   def construct_options
     valid_keys = ["contributor_id", "contributor_type"]
     

reply via email to

[Prev in Thread] Current Thread [Next in Thread]