11837: Fix "include_trash" scope and test case.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curoverse.com>

diff --git a/apps/workbench/test/controllers/trash_items_controller_test.rb b/apps/workbench/test/controllers/trash_items_controller_test.rb
deleted file mode 100644 (file)
index d5b0d90..0000000
--- a/apps/workbench/test/controllers/trash_items_controller_test.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-require 'test_helper'
-
-class TrashItemsControllerTest < ActionController::TestCase
-  reset_api_fixtures :after_each_test, false
-  reset_api_fixtures :after_suite, true
-
-  [
-    :active,
-    :admin,
-  ].each do |user|
-    test "trash index page as #{user}" do
-      get :index, {partial: :trash_rows, format: :json}, session_for(user)
-      assert_response :success
-
-      items = []
-      @response.body.scan(/tr\ data-object-uuid=\\"(.*?)\\"/).each do |uuid,|
-        items << uuid
-      end
-
-      assert_includes(items, api_fixture('collections')['unique_expired_collection']['uuid'])
-      if user == :admin
-        assert_includes(items, api_fixture('collections')['unique_expired_collection2']['uuid'])
-      else
-        assert_not_includes(items, api_fixture('collections')['unique_expired_collection2']['uuid'])
-      end
-    end
-  end
-end

diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 73a7e09c5e69cdd6447d91f1e9d0907c97d143ce..e3c10b78eee427ec093db7483d9e64a47c8212b8 100644 (file)
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -13,7 +13,7 @@ class Arvados::V1::CollectionsController < ApplicationController
 
   def find_objects_for_index
     if params[:include_trash] || ['destroy', 'trash', 'untrash'].include?(action_name)
-      @objects = Collection.readable_by(*@read_users).unscoped
+      @objects = Collection.unscoped.readable_by(*@read_users)
     end
     super
   end

diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index bb33c5595aea267c4dc996545f1d73d3006b3453..d1a0bc579499198c153f465b247fcb5b64d2cd6b 100644 (file)
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -250,7 +250,8 @@ class ArvadosModel < ActiveRecord::Base
 
     # Check if any of the users are admin.  If so, we're done.
     if users_list.select { |u| u.is_admin }.any?
-      return self
+      # Return existing relation with no new filters.
+      return where({})
     end
 
     # Collect the UUIDs of the authorized users.

diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index 055af9e67cffe4fc93d44aea5d70f11b23948b6d..17af916b3df244821bc596bbbc58fad799d8d380 100644 (file)
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1045,7 +1045,7 @@ EOS
 
   [:active, :admin].each do |user|
     test "get trashed collections as #{user}" do
-      authorize_with :active
+      authorize_with user
       get :index, {
         filters: [["is_trashed", "=", true]],
         include_trash: true,