3686: Support sharing pane in repository show page.
authorRadhika Chippada <radhika@curoverse.com>
Wed, 14 Jan 2015 16:10:27 +0000 (11:10 -0500)
committerRadhika Chippada <radhika@curoverse.com>
Wed, 14 Jan 2015 16:10:27 +0000 (11:10 -0500)
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.

apps/workbench/app/controllers/application_controller.rb
apps/workbench/app/controllers/projects_controller.rb
apps/workbench/app/controllers/repositories_controller.rb
apps/workbench/app/views/application/_show_sharing.html.erb [moved from apps/workbench/app/views/projects/_show_sharing.html.erb with 88% similarity]
apps/workbench/config/routes.rb
apps/workbench/test/controllers/projects_controller_test.rb
apps/workbench/test/controllers/repositories_controller_test.rb
apps/workbench/test/helpers/share_object_helper.rb [new file with mode: 0644]
apps/workbench/test/integration/projects_test.rb
apps/workbench/test/integration/repositories_test.rb [new file with mode: 0644]

index c7575176dec97fa4762bb3d730fd3ec0ef95b7ca..f68f1b157c80cee89dadbc55b95f6d68a925976d 100644 (file)
@@ -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)
index 600af8d60b6f23309794ba34e6a9a426a9fed663..a0bf262c960384a58e11d34d5949be5b7dc60a38 100644 (file)
@@ -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
index b6b3295ef8a381f71bc6439446307639d9bbb269..5dd9288e121e6f355b4ef8dfbe97832ed25b9a8e 100644 (file)
@@ -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
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 480f401f7ad2750bb35e0499ca68502932a79839..23795d3f04a3a56ec99f30be5d4e438fb7d33875 100644 (file)
@@ -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 %>
   <% 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|
       <%= 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 %>
index 86cdc38171263f25d8ecd34f9b7cdc009bcc4691..6b29d0553d4f0903973f40f951fde82d99ab3144 100644 (file)
@@ -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
index 93f794d900c1af5d597c5a8ff45879de48236662..8407dc324257518b51144ee9827a7de814f8b304 100644 (file)
@@ -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
index 15d28bfee65cab84c0966a199f288b05ee2f2660..e45095c64c2b7d4e865bf2b81bc5553aef854083 100644 (file)
@@ -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 (file)
index 0000000..400fcfb
--- /dev/null
@@ -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
index ce5b47e5d9bf5fef050972379c1124c6389f473d..cb06e19a27c7ec15971f6bd8be93dc984693c633 100644 (file)
@@ -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 (file)
index 0000000..5648c71
--- /dev/null
@@ -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