[ARVADOS] created: 752b9167416a5c8b64bef4946fbc15939b4bfe75
git at public.curoverse.com
git at public.curoverse.com
Wed Jan 14 11:12:23 EST 2015
at 752b9167416a5c8b64bef4946fbc15939b4bfe75 (commit)
commit 752b9167416a5c8b64bef4946fbc15939b4bfe75
Author: Radhika Chippada <radhika at curoverse.com>
Date: Wed Jan 14 11:10:27 2015 -0500
3686: Support sharing pane in repository show page.
Refactor current project sharing work into application area and reuse it for sharing repositories.
Refactor methods used by tests into helper and reuse in repository sharing tests.
diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb
index c757517..f68f1b1 100644
--- a/apps/workbench/app/controllers/application_controller.rb
+++ b/apps/workbench/app/controllers/application_controller.rb
@@ -384,6 +384,53 @@ class ApplicationController < ActionController::Base
%w(Attributes Advanced)
end
+ def set_share_links
+ @user_is_manager = false
+ @share_links = []
+
+ if @object.uuid != current_user.uuid
+ begin
+ @share_links = Link.permissions_for(@object)
+ @user_is_manager = true
+ rescue ArvadosApiClient::AccessForbiddenException,
+ ArvadosApiClient::NotFoundException
+ end
+ end
+ end
+
+ def share_with
+ if not params[:uuids].andand.any?
+ @errors = ["No user/group UUIDs specified to share with."]
+ return render_error(status: 422)
+ end
+ results = {"success" => [], "errors" => []}
+ params[:uuids].each do |shared_uuid|
+ begin
+ Link.create(tail_uuid: shared_uuid, link_class: "permission",
+ name: "can_read", head_uuid: @object.uuid)
+ rescue ArvadosApiClient::ApiError => error
+ error_list = error.api_response.andand[:errors]
+ if error_list.andand.any?
+ results["errors"] += error_list.map { |e| "#{shared_uuid}: #{e}" }
+ else
+ error_code = error.api_status || "Bad status"
+ results["errors"] << "#{shared_uuid}: #{error_code} response"
+ end
+ else
+ results["success"] << shared_uuid
+ end
+ end
+ if results["errors"].empty?
+ results.delete("errors")
+ status = 200
+ else
+ status = 422
+ end
+ respond_to do |f|
+ f.json { render(json: results, status: status) }
+ end
+ end
+
protected
def strip_token_from_path(path)
diff --git a/apps/workbench/app/controllers/projects_controller.rb b/apps/workbench/app/controllers/projects_controller.rb
index 600af8d..a0bf262 100644
--- a/apps/workbench/app/controllers/projects_controller.rb
+++ b/apps/workbench/app/controllers/projects_controller.rb
@@ -30,19 +30,6 @@ class ProjectsController < ApplicationController
end
end
- def set_share_links
- @user_is_manager = false
- @share_links = []
- if @object.uuid != current_user.uuid
- begin
- @share_links = Link.permissions_for(@object)
- @user_is_manager = true
- rescue ArvadosApiClient::AccessForbiddenException,
- ArvadosApiClient::NotFoundException
- end
- end
- end
-
def index_pane_list
%w(Projects)
end
@@ -304,37 +291,4 @@ class ProjectsController < ApplicationController
end
objects_and_names
end
-
- def share_with
- if not params[:uuids].andand.any?
- @errors = ["No user/group UUIDs specified to share with."]
- return render_error(status: 422)
- end
- results = {"success" => [], "errors" => []}
- params[:uuids].each do |shared_uuid|
- begin
- Link.create(tail_uuid: shared_uuid, link_class: "permission",
- name: "can_read", head_uuid: @object.uuid)
- rescue ArvadosApiClient::ApiError => error
- error_list = error.api_response.andand[:errors]
- if error_list.andand.any?
- results["errors"] += error_list.map { |e| "#{shared_uuid}: #{e}" }
- else
- error_code = error.api_status || "Bad status"
- results["errors"] << "#{shared_uuid}: #{error_code} response"
- end
- else
- results["success"] << shared_uuid
- end
- end
- if results["errors"].empty?
- results.delete("errors")
- status = 200
- else
- status = 422
- end
- respond_to do |f|
- f.json { render(json: results, status: status) }
- end
- end
end
diff --git a/apps/workbench/app/controllers/repositories_controller.rb b/apps/workbench/app/controllers/repositories_controller.rb
index b6b3295..5dd9288 100644
--- a/apps/workbench/app/controllers/repositories_controller.rb
+++ b/apps/workbench/app/controllers/repositories_controller.rb
@@ -1,5 +1,15 @@
class RepositoriesController < ApplicationController
+ before_filter :set_share_links, if: -> { defined? @object }
+
def index_pane_list
%w(recent help)
end
+
+ def show_pane_list
+ if @user_is_manager
+ super | %w(Sharing)
+ else
+ super
+ end
+ end
end
diff --git a/apps/workbench/app/views/projects/_show_sharing.html.erb b/apps/workbench/app/views/application/_show_sharing.html.erb
similarity index 88%
rename from apps/workbench/app/views/projects/_show_sharing.html.erb
rename to apps/workbench/app/views/application/_show_sharing.html.erb
index 480f401..23795d3 100644
--- a/apps/workbench/app/views/projects/_show_sharing.html.erb
+++ b/apps/workbench/app/views/application/_show_sharing.html.erb
@@ -30,6 +30,8 @@
else
owner_type = "owning user"
end
+
+ sharing_path = url_for(:controller => params['controller'], :action => 'share_with')
%>
<div class="pull-right">
@@ -42,7 +44,7 @@
multiple: true,
filters: choose_filters[share_class].to_json,
action_method: 'post',
- action_href: share_with_project_path,
+ action_href: sharing_path,
action_name: 'Add',
action_data: {selection_param: 'uuids[]', success: 'tab-refresh'}.to_json),
class: "btn btn-primary btn-sm", remote: true) do %>
@@ -52,16 +54,16 @@
<% end %>
</div>
-<p>Permissions for this project are inherited from the <%= owner_type %>
+<p>Permissions for this <%=@object.class_for_display.downcase%> are inherited from the <%= owner_type %>
<i class="fa fa-fw <%= owner_icon %>"></i>
<%= link_to_if_arvados_object @object.owner_uuid, friendly_name: true %>.
</p>
-<table id="project_sharing" class="topalign table" style="clear: both; margin-top: 1em;">
+<table id="object_sharing" class="topalign table" style="clear: both; margin-top: 1em;">
<tr>
<th>User/Group Name</th>
<th>Email Address</th>
- <th colspan="2">Project Access</th>
+ <th colspan="2"><%=@object.class_for_display%> Access</th>
</tr>
<% @share_links.andand.each do |link|
@@ -104,7 +106,7 @@
<%= link_to(
{action: 'destroy', id: link.uuid, controller: "links"},
{title: 'Revoke', class: 'btn btn-default btn-nodecorate', method: :delete,
- data: {confirm: "Revoke #{link_name}'s access to this project?",
+ data: {confirm: "Revoke #{link_name}'s access to this #{@object.class_for_display.downcase}?",
remote: true}}) do %>
<i class="fa fa-fw fa-trash-o"></i>
<% end %>
diff --git a/apps/workbench/config/routes.rb b/apps/workbench/config/routes.rb
index 86cdc38..6b29d05 100644
--- a/apps/workbench/config/routes.rb
+++ b/apps/workbench/config/routes.rb
@@ -16,7 +16,6 @@ ArvadosWorkbench::Application.routes.draw do
resources :humans
resources :traits
resources :api_client_authorizations
- resources :repositories
resources :virtual_machines
resources :authorized_keys
resources :job_tasks
@@ -24,6 +23,9 @@ ArvadosWorkbench::Application.routes.draw do
post 'cancel', :on => :member
get 'logs', :on => :member
end
+ resources :repositories do
+ post 'share_with', on: :member
+ end
match '/logout' => 'sessions#destroy', via: [:get, :post]
get '/logged_out' => 'sessions#index'
resources :users do
diff --git a/apps/workbench/test/controllers/projects_controller_test.rb b/apps/workbench/test/controllers/projects_controller_test.rb
index 93f794d..8407dc3 100644
--- a/apps/workbench/test/controllers/projects_controller_test.rb
+++ b/apps/workbench/test/controllers/projects_controller_test.rb
@@ -1,6 +1,9 @@
require 'test_helper'
+require 'helpers/share_object_helper'
class ProjectsControllerTest < ActionController::TestCase
+ include ShareObjectHelper
+
test "invited user is asked to sign user agreements on front page" do
get :index, {}, session_for(:inactive)
assert_response :redirect
@@ -61,40 +64,28 @@ class ProjectsControllerTest < ActionController::TestCase
"JSON response missing properly formatted sharing error")
end
- def user_can_manage(user_sym, group_key)
- get(:show, {id: api_fixture("groups")[group_key]["uuid"]},
- session_for(user_sym))
- is_manager = assigns(:user_is_manager)
- assert_not_nil(is_manager, "user_is_manager flag not set")
- if not is_manager
- assert_empty(assigns(:share_links),
- "non-manager has share links set")
- end
- is_manager
- end
-
test "admin can_manage aproject" do
- assert user_can_manage(:admin, "aproject")
+ assert user_can_manage(:admin, api_fixture("groups")["aproject"])
end
test "owner can_manage aproject" do
- assert user_can_manage(:active, "aproject")
+ assert user_can_manage(:active, api_fixture("groups")["aproject"])
end
test "owner can_manage asubproject" do
- assert user_can_manage(:active, "asubproject")
+ assert user_can_manage(:active, api_fixture("groups")["asubproject"])
end
test "viewer can't manage aproject" do
- refute user_can_manage(:project_viewer, "aproject")
+ refute user_can_manage(:project_viewer, api_fixture("groups")["aproject"])
end
test "viewer can't manage asubproject" do
- refute user_can_manage(:project_viewer, "asubproject")
+ refute user_can_manage(:project_viewer, api_fixture("groups")["asubproject"])
end
test "subproject_admin can_manage asubproject" do
- assert user_can_manage(:subproject_admin, "asubproject")
+ assert user_can_manage(:subproject_admin, api_fixture("groups")["asubproject"])
end
test "detect ownership loop in project breadcrumbs" do
diff --git a/apps/workbench/test/controllers/repositories_controller_test.rb b/apps/workbench/test/controllers/repositories_controller_test.rb
index 15d28bf..e45095c 100644
--- a/apps/workbench/test/controllers/repositories_controller_test.rb
+++ b/apps/workbench/test/controllers/repositories_controller_test.rb
@@ -1,4 +1,48 @@
require 'test_helper'
+require 'helpers/share_object_helper'
class RepositoriesControllerTest < ActionController::TestCase
+ include ShareObjectHelper
+
+ [
+ :active, #owner
+ :admin,
+ ].each do |user|
+ test "#{user} shares repository with a user and group" do
+ uuid_list = [api_fixture("groups")["future_project_viewing_group"]["uuid"],
+ api_fixture("users")["future_project_user"]["uuid"]]
+ post(:share_with, {
+ id: api_fixture("repositories")["foo"]["uuid"],
+ uuids: uuid_list,
+ format: "json"},
+ session_for(user))
+ assert_response :success
+ assert_equal(uuid_list, json_response["success"])
+ end
+ end
+
+ test "user with repository read permission cannot add permissions" do
+ share_uuid = api_fixture("users")["project_viewer"]["uuid"]
+ post(:share_with, {
+ id: api_fixture("repositories")["arvados"]["uuid"],
+ uuids: [share_uuid],
+ format: "json"},
+ session_for(:spectator))
+ assert_response 422
+ assert(json_response["errors"].andand.
+ any? { |msg| msg.start_with?("#{share_uuid}: ") },
+ "JSON response missing properly formatted sharing error")
+ end
+
+ test "admin can_manage repository" do
+ assert user_can_manage(:admin, api_fixture("repositories")["foo"])
+ end
+
+ test "owner can_manage repository" do
+ assert user_can_manage(:active, api_fixture("repositories")["foo"])
+ end
+
+ test "viewer cannot manage repository" do
+ refute user_can_manage(:spectator, api_fixture("repositories")["arvados"])
+ end
end
diff --git a/apps/workbench/test/helpers/share_object_helper.rb b/apps/workbench/test/helpers/share_object_helper.rb
new file mode 100644
index 0000000..400fcfb
--- /dev/null
+++ b/apps/workbench/test/helpers/share_object_helper.rb
@@ -0,0 +1,84 @@
+module ShareObjectHelper
+ def show_repository_using(auth_key, repo_key='arvados')
+ repo_uuid = api_fixture('repositories')[repo_key]['uuid']
+ visit(page_with_token(auth_key, "/repositories/#{repo_uuid}"))
+ assert(page.has_text?("push_url"), "not on expected repository page")
+ end
+
+ def show_project_using(auth_key, proj_key='aproject')
+ project_uuid = api_fixture('groups')[proj_key]['uuid']
+ visit(page_with_token(auth_key, "/projects/#{project_uuid}"))
+ assert(page.has_text?("A Project"), "not on expected project page")
+ end
+
+ def share_rows
+ find('#object_sharing').all('tr')
+ end
+
+ def add_share_and_check(share_type, name, obj=nil)
+ assert(page.has_no_text?(name), "project is already shared with #{name}")
+ start_share_count = share_rows.size
+ click_on("Share with #{share_type}")
+ within(".modal-container") do
+ # Order is important here: we should find something that appears in the
+ # modal before we make any assertions about what's not in the modal.
+ # Otherwise, the not-included assertions might falsely pass because
+ # the modal hasn't loaded yet.
+ find(".selectable", text: name).click
+ assert(has_no_selector?(".modal-dialog-preview-pane"),
+ "preview pane available in sharing dialog")
+ if share_type == 'users' and obj and obj['email']
+ assert(page.has_text?(obj['email']), "Did not find user's email")
+ end
+ assert_raises(Capybara::ElementNotFound,
+ "Projects pulldown available from sharing dialog") do
+ click_on "All projects"
+ end
+ click_on "Add"
+ end
+ using_wait_time(Capybara.default_wait_time * 3) do
+ assert(page.has_link?(name),
+ "new share was not added to sharing table")
+ assert_equal(start_share_count + 1, share_rows.size,
+ "new share did not add row to sharing table")
+ end
+ end
+
+ def modify_share_and_check(name)
+ start_rows = share_rows
+ link_row = start_rows.select { |row| row.has_text?(name) }
+ assert_equal(1, link_row.size, "row with new permission not found")
+ within(link_row.first) do
+ click_on("Read")
+ select("Write", from: "share_change_level")
+ click_on("editable-submit")
+ assert(has_link?("Write"),
+ "failed to change access level on new share")
+ click_on "Revoke"
+ if Capybara.current_driver == :selenium
+ page.driver.browser.switch_to.alert.accept
+ else
+ # poltergeist returns true for confirm(), so we don't need to accept.
+ end
+ end
+ wait_for_ajax
+ using_wait_time(Capybara.default_wait_time * 3) do
+ assert(page.has_no_text?(name),
+ "new share row still exists after being revoked")
+ assert_equal(start_rows.size - 1, share_rows.size,
+ "revoking share did not remove row from sharing table")
+ end
+ end
+
+ def user_can_manage(user_sym, fixture)
+ get(:show, {id: fixture["uuid"]}, session_for(user_sym))
+ is_manager = assigns(:user_is_manager)
+ assert_not_nil(is_manager, "user_is_manager flag not set")
+ if not is_manager
+ assert_empty(assigns(:share_links),
+ "non-manager has share links set")
+ end
+ is_manager
+ end
+
+end
diff --git a/apps/workbench/test/integration/projects_test.rb b/apps/workbench/test/integration/projects_test.rb
index ce5b47e..cb06e19 100644
--- a/apps/workbench/test/integration/projects_test.rb
+++ b/apps/workbench/test/integration/projects_test.rb
@@ -1,6 +1,9 @@
require 'integration_helper'
+require 'helpers/share_object_helper'
class ProjectsTest < ActionDispatch::IntegrationTest
+ include ShareObjectHelper
+
setup do
need_javascript
end
@@ -169,71 +172,6 @@ class ProjectsTest < ActionDispatch::IntegrationTest
"Project 5678 should now be inside project 1234")
end
- def show_project_using(auth_key, proj_key='aproject')
- project_uuid = api_fixture('groups')[proj_key]['uuid']
- visit(page_with_token(auth_key, "/projects/#{project_uuid}"))
- assert(page.has_text?("A Project"), "not on expected project page")
- end
-
- def share_rows
- find('#project_sharing').all('tr')
- end
-
- def add_share_and_check(share_type, name, obj=nil)
- assert(page.has_no_text?(name), "project is already shared with #{name}")
- start_share_count = share_rows.size
- click_on("Share with #{share_type}")
- within(".modal-container") do
- # Order is important here: we should find something that appears in the
- # modal before we make any assertions about what's not in the modal.
- # Otherwise, the not-included assertions might falsely pass because
- # the modal hasn't loaded yet.
- find(".selectable", text: name).click
- assert(has_no_selector?(".modal-dialog-preview-pane"),
- "preview pane available in sharing dialog")
- if share_type == 'users' and obj and obj['email']
- assert(page.has_text?(obj['email']), "Did not find user's email")
- end
- assert_raises(Capybara::ElementNotFound,
- "Projects pulldown available from sharing dialog") do
- click_on "All projects"
- end
- click_on "Add"
- end
- using_wait_time(Capybara.default_wait_time * 3) do
- assert(page.has_link?(name),
- "new share was not added to sharing table")
- assert_equal(start_share_count + 1, share_rows.size,
- "new share did not add row to sharing table")
- end
- end
-
- def modify_share_and_check(name)
- start_rows = share_rows
- link_row = start_rows.select { |row| row.has_text?(name) }
- assert_equal(1, link_row.size, "row with new permission not found")
- within(link_row.first) do
- click_on("Read")
- select("Write", from: "share_change_level")
- click_on("editable-submit")
- assert(has_link?("Write"),
- "failed to change access level on new share")
- click_on "Revoke"
- if Capybara.current_driver == :selenium
- page.driver.browser.switch_to.alert.accept
- else
- # poltergeist returns true for confirm(), so we don't need to accept.
- end
- end
- wait_for_ajax
- using_wait_time(Capybara.default_wait_time * 3) do
- assert(page.has_no_text?(name),
- "new share row still exists after being revoked")
- assert_equal(start_rows.size - 1, share_rows.size,
- "revoking share did not remove row from sharing table")
- end
- end
-
test "project viewer can't see project sharing tab" do
show_project_using("project_viewer")
assert(page.has_no_link?("Sharing"),
diff --git a/apps/workbench/test/integration/repositories_test.rb b/apps/workbench/test/integration/repositories_test.rb
new file mode 100644
index 0000000..5648c71
--- /dev/null
+++ b/apps/workbench/test/integration/repositories_test.rb
@@ -0,0 +1,45 @@
+require 'integration_helper'
+require 'helpers/share_object_helper'
+
+class RepositoriesTest < ActionDispatch::IntegrationTest
+ include ShareObjectHelper
+
+ setup do
+ need_javascript
+ end
+
+ [
+ 'active', #owner
+ 'admin'
+ ].each do |user|
+ test "#{user} can manage sharing for another user" do
+ add_user = api_fixture('users')['future_project_user']
+ new_name = ["first_name", "last_name"].map { |k| add_user[k] }.join(" ")
+
+ show_repository_using(user, 'foo')
+ click_on "Sharing"
+ add_share_and_check("users", new_name, add_user)
+ modify_share_and_check(new_name)
+ end
+ end
+
+ [
+ 'active', #owner
+ 'admin'
+ ].each do |user|
+ test "#{user} can manage sharing for another group" do
+ new_name = api_fixture('groups')['future_project_viewing_group']['name']
+
+ show_repository_using("active", 'foo')
+ click_on "Sharing"
+ add_share_and_check("groups", new_name)
+ modify_share_and_check(new_name)
+ end
+ end
+
+ test "spectator does not see repository sharing tab" do
+ show_repository_using("spectator")
+ assert(page.has_no_link?("Sharing"),
+ "read-only repository user sees sharing tab")
+ end
+end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list