VRFS-3316 : Review cleanup, new tests

web/app/controllers/api_reviews_controller.rb

@@ -1,13 +1,12 @@
 require 'sanitize'
 class ApiReviewsController < ApiController
-
   before_filter :api_signed_in_user, :except => [:index]
-  #before_filter :auth_user, :only => [:create, :update, :delete]
   before_filter :lookup_review_summary, :only => [:details]
   before_filter :lookup_review, :only => [:update, :delete, :show]
 
   respond_to :json
 
+  # List review summaries according to params:
   def index
     summaries = ReviewSummary.index(params[:review])
     @reviews = summaries.paginate(page: params[:page], per_page: params[:per_page])
@@ -26,12 +25,14 @@ class ApiReviewsController < ApiController
     respond_with_model(@review)
   end
 
+  # List reviews matching targets for given review summary:
   def details
     reviews = Review.index(:target_id=>@review_summary.target_id)
     @reviews = reviews.paginate(page: params[:page], per_page: params[:per_page])
     respond_with @reviews, responder: ApiResponder, :status => 200
   end
 
+  # Update a review:
   def update
     mods = params[:mods]
     if mods.present?
@@ -42,8 +43,10 @@ class ApiReviewsController < ApiController
     respond_with_model(@review)
   end
 
+  # Mark a review as deleted:
   def delete
     @review.deleted_at = Time.now()
+    @review
     @review.save
     render :json => {}, status: 204
   end
@@ -58,6 +61,6 @@ private
     arel = Review.where("id=?", params[:id])
     arel = arel.where("user_id=?", current_user) unless current_user.admin
     @review = arel.first
-    puts "FFFound: #{@review}"
+    raise ActiveRecord::RecordNotFound, "Couldn't find review matching #{arel}" if @review.nil?
   end
 end

web/spec/controllers/api_reviews_controller_spec.rb

@@ -2,8 +2,6 @@ require 'spec_helper'
 
 describe ApiReviewsController do
   render_views
-
-
   before(:all) do
     @logged_in_user = FactoryGirl.create(:user)
   end
@@ -35,6 +33,7 @@ describe ApiReviewsController do
     before :each do
       @review=Review.create!(target:@target, rating:5, description: "blah", user_id: @logged_in_user.id)
     end
+
     it "basic" do
       post :update, id:@review.id, mods: {rating:4, description: "not blah"}, :format=>'json'
       response.should be_success
@@ -42,12 +41,18 @@ describe ApiReviewsController do
       @review.rating.should eq(4)
       @review.description.should eq("not blah")
     end
+
+    it "bad identifier" do
+      post :update, id:2112, mods: {rating:4, description: "not blah"}, :format=>'json'
+      response.status.should eql(404)
+    end
   end
 
   describe "delete" do
     before :each do
       @review=Review.create!(target:@target, rating:5, description: "blah", user_id: @logged_in_user.id)
     end
+
     it "marks review as deleted" do
       delete :delete, id:@review.id
       response.should be_success
@@ -68,18 +73,14 @@ describe ApiReviewsController do
       get :index, format: 'json'
       response.should be_success
       json = JSON.parse(response.body)
-      json.should have(0).items
-      json = JSON.parse(response.body)
-      puts "response.inspect: #{JSON.pretty_generate(json)}"
+      json.should have(0).items      
 
       ReviewSummary.index.should have(0).items
       Review.reduce()
       ReviewSummary.index.should have(2).items
       get :index, format: 'json'
       json = JSON.parse(response.body)
-      json.should have(2).items
-      puts "response.inspect: #{JSON.pretty_generate(json)}"
-
+      json.should have(2).item
     end
 
     it "details" do
@@ -87,7 +88,6 @@ describe ApiReviewsController do
       Review.reduce()
       ReviewSummary.index.should have(2).items
 
-
       summaries = ReviewSummary.index
       get :details, :review_summary_id=>summaries[0].id, format: 'json'
       response.should be_success
@@ -98,8 +98,6 @@ describe ApiReviewsController do
       response.should be_success
       json = JSON.parse(response.body)
       json.should have(5).items
-
-      puts "details: #{JSON.pretty_generate(json)}"
     end
 
     it "paginates details" do