[ARVADOS] updated: 0e3369b7179c4e483faf681e67279d762feaa33c
Git user
git at public.curoverse.com
Thu Jun 8 15:34:42 EDT 2017
Summary of changes:
apps/workbench/test/integration/trash_test.rb | 13 +++---------
.../arvados/v1/collections_controller.rb | 2 +-
services/api/app/models/arvados_model.rb | 3 ++-
.../arvados/v1/collections_controller_test.rb | 23 ++++++++++++++++++++++
4 files changed, 29 insertions(+), 12 deletions(-)
via 0e3369b7179c4e483faf681e67279d762feaa33c (commit)
via b7a20a386bb6bf582c4bc92d8718b883417d4c48 (commit)
via aa12c56d26cb72824ddd2399e17e846cd9d9b483 (commit)
from 1595481c3e4cc42cfa5a47bbe501eb6e6f83db9f (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 0e3369b7179c4e483faf681e67279d762feaa33c
Merge: 1595481 b7a20a3
Author: radhika <radhika at curoverse.com>
Date: Thu Jun 8 15:34:25 2017 -0400
closes #11837
Merge branch '11837-trash-access'
commit b7a20a386bb6bf582c4bc92d8718b883417d4c48
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Jun 8 14:21:21 2017 -0400
11837: Fix "include_trash" scope and test case.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at 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
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 73a7e09..e3c10b7 100644
--- 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 bb33c55..d1a0bc5 100644
--- 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 055af9e..17af916 100644
--- 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,
commit aa12c56d26cb72824ddd2399e17e846cd9d9b483
Author: radhika <radhika at curoverse.com>
Date: Thu Jun 8 13:15:52 2017 -0400
11837: write tests
diff --git a/apps/workbench/test/controllers/trash_items_controller_test.rb b/apps/workbench/test/controllers/trash_items_controller_test.rb
new file mode 100644
index 0000000..d5b0d90
--- /dev/null
+++ b/apps/workbench/test/controllers/trash_items_controller_test.rb
@@ -0,0 +1,28 @@
+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/apps/workbench/test/integration/trash_test.rb b/apps/workbench/test/integration/trash_test.rb
index cd2fa39..857e57a 100644
--- a/apps/workbench/test/integration/trash_test.rb
+++ b/apps/workbench/test/integration/trash_test.rb
@@ -15,8 +15,8 @@ class TrashTest < ActionDispatch::IntegrationTest
assert_text deleted['name']
assert_text expired1['name']
- assert_text expired2['name']
- assert_no_text 'foo_file'
+ assert_no_text expired2['name'] # not readable by this user
+ assert_no_text 'foo_file' # not trash
# Un-trash one item using selection dropdown
within('tr', text: deleted['name']) do
@@ -33,12 +33,6 @@ class TrashTest < ActionDispatch::IntegrationTest
assert_text expired1['name'] # this should still be there
assert_no_text deleted['name'] # this should no longer be here
- # expired2 is not editable by me; checkbox and recycle button shouldn't be offered
- within('tr', text: expired2['name']) do
- assert_nil first('input')
- assert_nil first('.fa-recycle')
- end
-
# Un-trash another item using the recycle button
within('tr', text: expired1['name']) do
first('.fa-recycle').click
@@ -46,7 +40,6 @@ class TrashTest < ActionDispatch::IntegrationTest
wait_for_ajax
- assert_text expired2['name']
assert_no_text expired1['name']
# verify that the two un-trashed items are now shown in /collections page
@@ -58,7 +51,7 @@ class TrashTest < ActionDispatch::IntegrationTest
test "trash page with search" do
deleted = api_fixture('collections')['deleted_on_next_sweep']
- expired = api_fixture('collections')['unique_expired_collection2']
+ expired = api_fixture('collections')['unique_expired_collection']
visit page_with_token('active', "/trash")
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 7618985..055af9e 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -1042,4 +1042,27 @@ EOS
}
assert_response 422
end
+
+ [:active, :admin].each do |user|
+ test "get trashed collections as #{user}" do
+ authorize_with :active
+ get :index, {
+ filters: [["is_trashed", "=", true]],
+ include_trash: true,
+ }
+ assert_response :success
+
+ items = []
+ json_response["items"].each do |coll|
+ items << coll['uuid']
+ end
+
+ assert_includes(items, collections('unique_expired_collection')['uuid'])
+ if user == :admin
+ assert_includes(items, collections('unique_expired_collection2')['uuid'])
+ else
+ assert_not_includes(items, collections('unique_expired_collection2')['uuid'])
+ end
+ end
+ end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list