changed how not found objects get handled with pundit

app/controllers/work_item_states_controller.rb

@@ -30,10 +30,11 @@ def update
   def show
     if @state_item.nil?
       render nothing: true, status: :not_found and return
-    end
-    authorize @state_item
-    respond_to do |format|
-      format.json { render json: @state_item.serializable_hash }
+    else
+      authorize @state_item
+      respond_to do |format|
+        format.json { render json: @state_item.serializable_hash }
+      end
     end
   end
 

app/controllers/work_items_controller.rb

@@ -10,12 +10,17 @@ def index
     filter unless request.format == :json
     sort
     page_results(@items)
-    respond_to do |format|
-      format.json {
-        json_list = @paged_results.map { |item| item.serializable_hash(except: [:node, :pid]) }
-        render json: {count: @count, next: @next, previous: @previous, results: json_list}
-      }
-      format.html
+    if @items.nil? || @items.empty?
+      render nothing: true, status: :not_found and return
+    else
+      authorize @items
+      respond_to do |format|
+        format.json {
+          json_list = @paged_results.map { |item| item.serializable_hash(except: [:node, :pid]) }
+          render json: {count: @count, next: @next, previous: @previous, results: json_list}
+        }
+        format.html
+      end
     end
   end
 
@@ -31,15 +36,19 @@ def create
   end
 
   def show
-    authorize @work_item
-    respond_to do |format|
-      if current_user.admin?
-        item = @work_item.serializable_hash
-      else
-        item = @work_item.serializable_hash(except: [:node, :pid])
+    if @work_item
+      authorize @work_item
+      respond_to do |format|
+        if current_user.admin?
+          item = @work_item.serializable_hash
+        else
+          item = @work_item.serializable_hash(except: [:node, :pid])
+        end
+        format.json { render json: item }
+        format.html
       end
-      format.json { render json: item }
-      format.html
+    else
+      render nothing: true, status: :not_found and return
     end
   end
 
@@ -111,7 +120,11 @@ def set_restoration_status
     restore = Pharos::Application::PHAROS_ACTIONS['restore']
     @item = WorkItem.where(object_identifier: params[:object_identifier],
                            action: restore).order(created_at: :desc).first
-    authorize (@item || WorkItem.new), :set_restoration_status?
+    if @item.nil?
+      render nothing: true, status: :not_found and return
+    else
+      authorize @item
+    end
     if @item
       succeeded = @item.update_attributes(params_for_status_update)
     end
@@ -277,7 +290,6 @@ def set_items
     end
     params[:institution].present? ? @institution = Institution.find(params[:institution]) : @institution = current_user.institution
     params[:sort] = 'date' if params[:sort].nil?
-    authorize @items
     @items = @items
       .created_before(params[:created_before])
       .created_after(params[:created_after])
@@ -320,17 +332,6 @@ def set_item
                                     bag_date: params[:bag_date]).first
       end
     end
-    if @work_item
-      params[:id] = @work_item.id
-    else
-      # API callers **DEPEND** on getting a 404 if the record does
-      # not exist. This is how they know that an item has not started
-      # the ingestion process. So if @work_item is nil, return
-      # 404 now. Otherwise, the call to authorize below will result
-      # in a 500 error from pundit.
-      raise ActiveRecord::RecordNotFound
-    end
-    authorize @work_item, :show?
   end
 
   def rewrite_params_for_sqlite

app/policies/institution_policy.rb

@@ -22,7 +22,7 @@ def index_through_institution?
   end
 
   def show?
-    record.nil? || user.admin? ||  (user.institution_id == record.id)
+    user.admin? ||  (user.institution_id == record.id)
   end
 
   def edit?

app/policies/work_item_policy.rb

@@ -9,11 +9,7 @@ def new?
   end
 
   def index?
-    record.first.nil? || user.admin? ||  (user.institution.id == record.first.institution_id)
-  end
-
-  def search?
-    record.first.nil? || user.admin? || (user.institution.id == record.first.institution_id)
+    user.admin? ||  (user.institution.id == record.first.institution_id)
   end
 
   def admin_api?
@@ -25,7 +21,7 @@ def admin_show?
   end
 
   def show?
-    record.nil? || user.admin? || (user.institution.id == record.institution_id)
+    user.admin? || (user.institution.id == record.institution_id)
   end
 
   def update?
@@ -42,7 +38,7 @@ def destroy?
   end
 
   def set_restoration_status?
-    record.nil? || user.admin?
+    user.admin?
   end
 
   def items_for_delete?

config/routes.rb

@@ -55,6 +55,7 @@
   post '/api/v2/item_state', to: 'work_item_states#create', format: :json
   put '/api/v2/item_state/:work_item_id', to: 'work_item_states#update', format: :json
   get '/api/v2/item_state/:work_item_id', to: 'work_item_states#show', format: :json
+  get '/api/v2/item_state/:id', to: 'work_item_states#show', format: :json
 
   # CATALOG ROUTES
   post 'search/', to: 'catalog#search', format: [:json, :html], as: :search

spec/controllers/work_item_states_controller_spec.rb

@@ -6,6 +6,7 @@
   let!(:object) { FactoryGirl.create(:intellectual_object, institution: institution, access: 'institution') }
   let!(:item) { FactoryGirl.create(:work_item, institution: institution, intellectual_object: object, object_identifier: object.identifier, action: Pharos::Application::PHAROS_ACTIONS['fixity'], status: Pharos::Application::PHAROS_STATUSES['success']) }
   let!(:other_item) { FactoryGirl.create(:work_item, institution: institution, intellectual_object: object, object_identifier: object.identifier) }
+  let!(:lonely_item) { FactoryGirl.create(:work_item, institution: institution, intellectual_object: object, object_identifier: object.identifier) }
   let!(:state_item) { FactoryGirl.create(:work_item_state, work_item: item, state: '{JSON data}') }
 
   describe 'POST #create' do
@@ -17,7 +18,6 @@
       post :create, work_item_state: { state: '{JSON data}', work_item_id: other_item.id, action: 'Success' }, format: :json
       expect(response).to be_success
       assigns(:state_item).action.should eq('Success')
-      # State '{JSON data}' should be compressed
       assigns(:state_item).state.bytes.should eq("x\x9C\xAB\xF6\n\xF6\xF7SHI,I\xAC\x05\x00\x16\x90\x03\xED".bytes)
       assigns(:state_item).unzipped_state.should eq('{JSON data}')
       assigns(:state_item).work_item_id.should eq(other_item.id)
@@ -56,6 +56,11 @@
         assigns(:state_item).state.bytes.should eq("x\x9C\xAB\xF6\n\xF6\xF7SHI,I\xAC\x05\x00\x16\x90\x03\xED".bytes)
         assigns(:state_item).unzipped_state.should eq('{JSON data}')
       end
+
+      it 'responds with a 404 error if the state item does not exist' do
+        get :show, work_item_id: lonely_item.id, format: :json
+        expect(response.status).to eq(404)
+      end
     end
   end
 

spec/controllers/work_items_controller_spec.rb

@@ -37,6 +37,11 @@
         get :index
         assigns(:institution).should eq( admin_user.institution)
       end
+
+      it 'responds with a 404 when no results are found' do
+        get :index, name: 'name_of_something_not_found'
+        expect(response.status).to eq(404)
+      end
     end
 
     describe 'for institutional admin' do
@@ -84,11 +89,8 @@
       end
 
       it 'returns 404, not 500, for item not found' do
-        expect {
-          get :show,
-              etag: 'does not exist',
-              name: 'duznot igzist',
-              bag_date: '1901-01-01' }.to raise_error(ActiveRecord::RecordNotFound)
+        get :show, etag: 'does not exist', name: 'duznot igzist', bag_date: '1901-01-01'
+        expect(response.status).to eq(404)
       end
     end