[ARVADOS] updated: 18d29e3ea4dc7bc4009e51cf7679b97955f0a324

Git user git at public.curoverse.com
Wed Aug 30 11:08:22 EDT 2017


Summary of changes:
 .../api/app/controllers/application_controller.rb  |   2 +-
 .../arvados/v1/collections_controller.rb           |  36 +---
 .../controllers/arvados/v1/groups_controller.rb    |  16 +-
 services/api/app/models/arvados_model.rb           |  57 +++---
 services/api/app/models/container.rb               |  18 +-
 services/api/app/models/log.rb                     |   3 +
 services/api/config/routes.rb                      |   2 +
 services/api/lib/create_permission_view.sql        |   5 +-
 services/api/lib/trashable.rb                      |  37 ++++
 services/api/test/fixtures/groups.yml              |   7 +
 .../arvados/v1/groups_controller_test.rb           | 204 ++++++++++++---------
 services/api/test/unit/group_test.rb               |  71 ++++++-
 12 files changed, 293 insertions(+), 165 deletions(-)

       via  18d29e3ea4dc7bc4009e51cf7679b97955f0a324 (commit)
      from  f0fe91a70bd7027f1fecefde86d7a599bee4e8fa (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 18d29e3ea4dc7bc4009e51cf7679b97955f0a324
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Tue Aug 29 09:08:19 2017 -0400

    12032: Controller support for group trash.
    
    Conttroller support and testing for contents, index, show, trash, untrash,
    containers.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index a3d8f08..46b96a9 100644
--- a/services/api/app/controllers/application_controller.rb
+++ b/services/api/app/controllers/application_controller.rb
@@ -180,7 +180,7 @@ class ApplicationController < ActionController::Base
   end
 
   def find_objects_for_index
-    @objects ||= model_class.readable_by(*@read_users)
+    @objects ||= model_class.readable_by(*@read_users, {:include_trash => (params[:include_trash] || ['destroy', 'trash', 'untrash'].include?(action_name))})
     apply_where_limit_order_params
   end
 
diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index 87d88fe..0312d95 100644
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -3,9 +3,11 @@
 # SPDX-License-Identifier: AGPL-3.0
 
 require "arvados/keep"
+require "trashable"
 
 class Arvados::V1::CollectionsController < ApplicationController
   include DbCurrentTime
+  include TrashableController
 
   def self._index_requires_parameters
     (super rescue {}).
@@ -16,7 +18,6 @@ class Arvados::V1::CollectionsController < ApplicationController
       })
   end
 
-
   def create
     if resource_attrs[:uuid] and (loc = Keep::Locator.parse(resource_attrs[:uuid]))
       resource_attrs[:portable_data_hash] = loc.to_s
@@ -58,39 +59,6 @@ class Arvados::V1::CollectionsController < ApplicationController
     end
   end
 
-  def destroy
-    if !@object.is_trashed
-      @object.update_attributes!(trash_at: db_current_time)
-    end
-    earliest_delete = (@object.trash_at +
-                       Rails.configuration.blob_signature_ttl.seconds)
-    if @object.delete_at > earliest_delete
-      @object.update_attributes!(delete_at: earliest_delete)
-    end
-    show
-  end
-
-  def trash
-    if !@object.is_trashed
-      @object.update_attributes!(trash_at: db_current_time)
-    end
-    show
-  end
-
-  def untrash
-    if @object.is_trashed
-      @object.trash_at = nil
-
-      if params[:ensure_unique_name]
-        @object.save_with_unique_name!
-      else
-        @object.save!
-      end
-    else
-      raise InvalidStateTransitionError
-    end
-    show
-  end
 
   def find_collections(visited, sp, &b)
     case sp
diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb
index 75ef095..9e0cdf8 100644
--- a/services/api/app/controllers/arvados/v1/groups_controller.rb
+++ b/services/api/app/controllers/arvados/v1/groups_controller.rb
@@ -2,7 +2,19 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
+require "trashable"
+
 class Arvados::V1::GroupsController < ApplicationController
+  include TrashableController
+
+  def self._index_requires_parameters
+    (super rescue {}).
+      merge({
+        include_trash: {
+          type: 'boolean', required: false, description: "Include items whose is_trashed attribute is true."
+        },
+      })
+  end
 
   def self._contents_requires_parameters
     params = _index_requires_parameters.
@@ -152,10 +164,10 @@ class Arvados::V1::GroupsController < ApplicationController
       end.compact
 
       if klass == Collection and params[:include_trash]
-        @objects = klass.unscoped.readable_by(*@read_users).
+        @objects = klass.unscoped.readable_by(*@read_users, {:include_trash => params[:include_trash]}).
           order(request_order).where(where_conds)
       else
-        @objects = klass.readable_by(*@read_users).
+        @objects = klass.readable_by(*@read_users, {:include_trash => params[:include_trash]}).
           order(request_order).where(where_conds)
       end
       klass_limit = limit_all - all_objects.count
diff --git a/services/api/app/models/arvados_model.rb b/services/api/app/models/arvados_model.rb
index ccd8f82..518d4c8 100644
--- a/services/api/app/models/arvados_model.rb
+++ b/services/api/app/models/arvados_model.rb
@@ -254,7 +254,8 @@ class ArvadosModel < ActiveRecord::Base
 
     # Collect the UUIDs of the authorized users.
     sql_table = kwargs.fetch(:table_name, table_name)
-    include_trashed = kwargs.fetch(:include_trashed, false)
+    include_trash = kwargs.fetch(:include_trash, false)
+    query_on = kwargs.fetch(:query_on, self)
 
     sql_conds = []
     user_uuids = users_list.map { |u| u.uuid }
@@ -263,49 +264,53 @@ class ArvadosModel < ActiveRecord::Base
 
     # Check if any of the users are admin.
     if users_list.select { |u| u.is_admin }.any?
-      if !include_trashed
+      if !include_trash
         # exclude rows that are trashed.
         if self.column_names.include? "owner_uuid"
           sql_conds += ["NOT EXISTS(SELECT target_uuid
-                  FROM permission_view pv
+                  FROM permission_view
                   WHERE trashed = 1 AND
-                  (#{sql_table}.uuid = pv.target_uuid OR #{sql_table}.owner_uuid = pv.target_uuid))"]
+                  (#{sql_table}.uuid = target_uuid OR #{sql_table}.owner_uuid = target_uuid))"]
         else
           sql_conds += ["NOT EXISTS(SELECT target_uuid
-                  FROM permission_view pv
+                  FROM permission_view
                   WHERE trashed = 1 AND
-                  (#{sql_table}.uuid = pv.target_uuid))"]
+                  (#{sql_table}.uuid = target_uuid))"]
         end
       end
     else
-      # Match objects which appear in the permission view
-      trash_clause = if !include_trashed then "trashed = 0 AND" else "" end
+      trash_clause = if !include_trash then "trashed = 0 AND" else "" end
 
-      # Match any object (evidently a group or user) whose UUID is
-      # listed explicitly in user_uuids.
+      # Can read object (evidently a group or user) whose UUID is listed
+      # explicitly in user_uuids.
       sql_conds += ["#{sql_table}.uuid IN (:user_uuids)"]
 
+      direct_permission_check = "EXISTS(SELECT 1 FROM permission_view
+                  WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
+                  (#{sql_table}.uuid = target_uuid))"
+
       if self.column_names.include? "owner_uuid"
-        sql_conds += ["EXISTS(SELECT target_uuid
-                  FROM permission_view pv
+        # if an explicit permission row exists for the uuid in question, apply
+        # the "direct_permission_check"
+        # if not, check for permission to read the owner instead
+        sql_conds += ["CASE
+                  WHEN EXISTS(select 1 FROM permission_view where target_uuid = #{sql_table}.uuid)
+                  THEN #{direct_permission_check}
+                  ELSE EXISTS(SELECT 1 FROM permission_view
                   WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
-                  (#{sql_table}.uuid = pv.target_uuid OR
-                  (#{sql_table}.owner_uuid = pv.target_uuid AND pv.target_owner_uuid is NOT NULL)))"]
-        # Match any object whose owner is listed explicitly in
-        # user_uuids.
-        trash_clause = if !include_trashed
+                  (#{sql_table}.owner_uuid = target_uuid AND target_owner_uuid is NOT NULL))
+                  END"]
+        # Can also read if one of the users is the owner of the object.
+        trash_clause = if !include_trash
                          "1 NOT IN (SELECT trashed
-                             FROM permission_view pv
-                             WHERE #{sql_table}.uuid = pv.target_uuid) AND"
+                             FROM permission_view
+                             WHERE #{sql_table}.uuid = target_uuid) AND"
                        else
                          ""
                        end
         sql_conds += ["(#{trash_clause} #{sql_table}.owner_uuid IN (:user_uuids))"]
       else
-        sql_conds += ["EXISTS(SELECT target_uuid
-                  FROM permission_view pv
-                  WHERE user_uuid IN (:user_uuids) AND perm_level >= 1 AND #{trash_clause}
-                  (#{sql_table}.uuid = pv.target_uuid))"]
+        sql_conds += [direct_permission_check]
       end
 
       if sql_table == "links"
@@ -317,9 +322,9 @@ class ArvadosModel < ActiveRecord::Base
       end
     end
 
-    where(sql_conds.join(' OR '),
-          user_uuids: user_uuids,
-          permission_link_classes: ['permission', 'resources'])
+    query_on.where(sql_conds.join(' OR '),
+                    user_uuids: user_uuids,
+                    permission_link_classes: ['permission', 'resources'])
   end
 
   def save_with_unique_name!
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 36c87af..d61ba75 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -297,17 +297,15 @@ class Container < ArvadosModel
   end
 
   def self.readable_by(*users_list)
-    if users_list.select { |u| u.is_admin }.any?
-      return self
+    # Load optional keyword arguments, if they exist.
+    if users_list.last.is_a? Hash
+      kwargs = users_list.pop
+    else
+      kwargs = {}
     end
-    user_uuids = users_list.map { |u| u.uuid }
-    uuid_list = user_uuids + users_list.flat_map { |u| u.groups_i_can(:read) }
-    uuid_list.uniq!
-    permitted = "(SELECT head_uuid FROM links WHERE link_class='permission' AND tail_uuid IN (:uuids))"
-    joins(:container_requests).
-      where("container_requests.uuid IN #{permitted} OR "+
-            "container_requests.owner_uuid IN (:uuids)",
-            uuids: uuid_list)
+    kwargs[:query_on] = joins(:container_requests)
+    users_list.push kwargs
+    ContainerRequest.readable_by(*users_list)
   end
 
   def final?
diff --git a/services/api/app/models/log.rb b/services/api/app/models/log.rb
index 73f143e..7f2d3ef 100644
--- a/services/api/app/models/log.rb
+++ b/services/api/app/models/log.rb
@@ -60,6 +60,9 @@ class Log < ArvadosModel
   end
 
   def self.readable_by(*users_list)
+    if users_list.last.is_a? Hash
+      users_list.pop
+    end
     if users_list.select { |u| u.is_admin }.any?
       return self
     end
diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb
index 2e9d618..5b6fe80 100644
--- a/services/api/config/routes.rb
+++ b/services/api/config/routes.rb
@@ -30,6 +30,8 @@ Server::Application.routes.draw do
       resources :groups do
         get 'contents', on: :collection
         get 'contents', on: :member
+        post 'trash', on: :member
+        post 'untrash', on: :member
       end
       resources :humans
       resources :job_tasks
diff --git a/services/api/lib/create_permission_view.sql b/services/api/lib/create_permission_view.sql
index 639610a..71ac8c4 100644
--- a/services/api/lib/create_permission_view.sql
+++ b/services/api/lib/create_permission_view.sql
@@ -37,9 +37,8 @@ perm_edges (tail_uuid, head_uuid, val, follow, trashed) AS (
               LEFT JOIN groups ON pv.val<3 AND groups.uuid = links.head_uuid
               WHERE links.link_class = 'permission'
        UNION ALL
-       SELECT owner_uuid, uuid, 3, true, CASE
-              WHEN trash_at IS NOT NULL and trash_at < clock_timestamp() THEN 1
-              ELSE 0 END
+       SELECT owner_uuid, uuid, 3, true,
+              CASE WHEN trash_at IS NOT NULL and trash_at < clock_timestamp() THEN 1 ELSE 0 END
               FROM groups
        ),
 perm (val, follow, user_uuid, target_uuid, trashed, startnode) AS (
diff --git a/services/api/lib/trashable.rb b/services/api/lib/trashable.rb
index 1e2f466..38ebaf7 100644
--- a/services/api/lib/trashable.rb
+++ b/services/api/lib/trashable.rb
@@ -89,3 +89,40 @@ module Trashable
     true
   end
 end
+
+module TrashableController
+  def destroy
+    if !@object.is_trashed
+      @object.update_attributes!(trash_at: db_current_time)
+    end
+    earliest_delete = (@object.trash_at +
+                       Rails.configuration.blob_signature_ttl.seconds)
+    if @object.delete_at > earliest_delete
+      @object.update_attributes!(delete_at: earliest_delete)
+    end
+    show
+  end
+
+  def trash
+    if !@object.is_trashed
+      @object.update_attributes!(trash_at: db_current_time)
+    end
+    show
+  end
+
+  def untrash
+    if @object.is_trashed
+      @object.trash_at = nil
+
+      if params[:ensure_unique_name]
+        @object.save_with_unique_name!
+      else
+        @object.save!
+      end
+    else
+      raise InvalidStateTransitionError
+    end
+    show
+  end
+
+end
diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml
index 1cb5817..2411831 100644
--- a/services/api/test/fixtures/groups.yml
+++ b/services/api/test/fixtures/groups.yml
@@ -313,6 +313,13 @@ trashed_subproject:
   owner_uuid: zzzzz-j7d0g-trashedproject1
   name: trashed subproject
   group_class: project
+  is_trashed: false
+
+trashed_subproject3:
+  uuid: zzzzz-j7d0g-trashedproject3
+  owner_uuid: zzzzz-j7d0g-trashedproject1
+  name: trashed subproject 3
+  group_class: project
   trash_at: 2001-01-01T00:00:00Z
   delete_at: 2038-03-01T00:00:00Z
   is_trashed: true
diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb
index 043d665..b80fcb1 100644
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -533,111 +533,145 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
   ### trashed project tests ###
 
   [:active, :admin].each do |auth|
-    test "trashed project is hidden in contents (#{auth})" do
-      authorize_with auth
-      get :contents, {
-            id: users(:active).uuid,
-            format: :json,
-          }
-      item_uuids = json_response['items'].map do |item|
-        item['uuid']
+    # project: to query,    to untrash,    is visible, parent contents listing success
+    [[:trashed_project,     [],                 false, true],
+     [:trashed_project,     [:trashed_project], true,  true],
+     [:trashed_subproject,  [],                 false, false],
+     [:trashed_subproject,  [:trashed_project], true,  true],
+     [:trashed_subproject3, [:trashed_project], false, true],
+     [:trashed_subproject3, [:trashed_subproject3], false, false],
+     [:trashed_subproject3, [:trashed_project, :trashed_subproject3], true, true],
+    ].each do |project, untrash, visible, success|
+
+      test "contents listing #{project} #{untrash} as #{auth}" do
+        authorize_with auth
+        untrash.each do |pr|
+          Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+        end
+        get :contents, {
+              id: groups(project).owner_uuid,
+              format: :json
+            }
+        if success
+          assert_response :success
+          item_uuids = json_response['items'].map do |item|
+            item['uuid']
+          end
+          if visible
+            assert_includes(item_uuids, groups(project).uuid)
+          else
+            assert_not_includes(item_uuids, groups(project).uuid)
+          end
+        else
+          assert_response 404
+        end
       end
-      assert_not_includes(item_uuids, groups(:trashed_project).uuid)
-    end
 
-    test "untrashed project is visible in contents (#{auth})" do
-      authorize_with auth
-      Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
-      get :contents, {
-            id: users(:active).uuid,
-            format: :json,
-          }
-      item_uuids = json_response['items'].map do |item|
-        item['uuid']
+      test "contents of #{project} #{untrash} as #{auth}" do
+        authorize_with auth
+        untrash.each do |pr|
+          Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+        end
+        get :contents, {
+              id: groups(project).uuid,
+              format: :json
+            }
+        if visible
+          assert_response :success
+        else
+          assert_response 404
+        end
       end
-      assert_includes(item_uuids, groups(:trashed_project).uuid)
-    end
 
-    test "trashed project is hidden in index (#{auth})" do
-      authorize_with :active
-      get :index, {
-            format: :json,
-          }
-      item_uuids = json_response['items'].map do |item|
-        item['uuid']
+      test "index #{project} #{untrash} as #{auth}" do
+        authorize_with auth
+        untrash.each do |pr|
+          Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+        end
+        get :index, {
+              format: :json,
+            }
+        assert_response :success
+        item_uuids = json_response['items'].map do |item|
+          item['uuid']
+        end
+        if visible
+          assert_includes(item_uuids, groups(project).uuid)
+        else
+          assert_not_includes(item_uuids, groups(project).uuid)
+        end
       end
-      assert_not_includes(item_uuids, groups(:trashed_project).uuid)
-      assert_not_includes(item_uuids, groups(:trashed_subproject).uuid)
-    end
 
-    test "untrashed project is visible in index (#{auth})" do
-      authorize_with :active
-      Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
-      get :index, {
-            format: :json,
-          }
-      item_uuids = json_response['items'].map do |item|
-        item['uuid']
+      test "show #{project} #{untrash} as #{auth}" do
+        authorize_with auth
+        untrash.each do |pr|
+          Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+        end
+        get :show, {
+              id: groups(project).uuid,
+              format: :json
+            }
+        if visible
+          assert_response :success
+        else
+          assert_response 404
+        end
       end
-      assert_includes(item_uuids, groups(:trashed_project).uuid)
-      assert_includes(item_uuids, groups(:trashed_subproject).uuid)
-    end
 
-    test "cannot get contents of trashed project (#{auth})" do
-      authorize_with :active
-      get :contents, {
-            id: groups(:trashed_project).uuid,
-            format: :json,
-          }
-      assert_response 404
-    end
+      test "show include_trash #{project} #{untrash} as #{auth}" do
+        authorize_with auth
+        untrash.each do |pr|
+          Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+        end
+        get :show, {
+              id: groups(project).uuid,
+              format: :json,
+              include_trash: true
+            }
+        assert_response :success
+      end
 
-    test "cannot get contents of trashed subproject (#{auth})" do
-      authorize_with :active
-      get :contents, {
-            id: groups(:trashed_subproject).uuid,
-            format: :json,
-          }
-      assert_response 404
+      test "index include_trash #{project} #{untrash} as #{auth}" do
+        authorize_with auth
+        untrash.each do |pr|
+          Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+        end
+        get :index, {
+              format: :json,
+              include_trash: true
+            }
+        assert_response :success
+        item_uuids = json_response['items'].map do |item|
+          item['uuid']
+        end
+        assert_includes(item_uuids, groups(project).uuid)
+      end
     end
 
-    test "can get contents of untrashed project (#{auth})" do
-      authorize_with :active
-      Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
-      get :contents, {
+    test "delete project #{auth}" do
+      authorize_with auth
+      [:trashed_project].each do |pr|
+        Group.find_by_uuid(groups(pr).uuid).update! is_trashed: false
+      end
+      assert !Group.find_by_uuid(groups(:trashed_project).uuid).is_trashed
+      post :destroy, {
             id: groups(:trashed_project).uuid,
             format: :json,
           }
-      assert_response 200
-    end
-
-    test "can get contents of untrashed subproject (#{auth})" do
-      authorize_with :active
-      Group.find_by_uuid(groups(:trashed_project).uuid).update! is_trashed: false
-      get :contents, {
-            id: groups(:trashed_subproject).uuid,
-            format: :json,
-          }
-      assert_response 200
+      assert_response :success
+      assert Group.find_by_uuid(groups(:trashed_project).uuid).is_trashed
     end
 
-    test "cannot show trashed project (#{auth})" do
-      authorize_with :active
-      get :show, {
+    test "untrash project #{auth}" do
+      authorize_with auth
+      assert Group.find_by_uuid(groups(:trashed_project).uuid).is_trashed
+      post :untrash, {
             id: groups(:trashed_project).uuid,
             format: :json,
           }
-      assert_response 404
+      assert_response :success
+      assert !Group.find_by_uuid(groups(:trashed_project).uuid).is_trashed
     end
 
-    test "cannot show trashed subproject (#{auth})" do
-      authorize_with :active
-      get :show, {
-            id: groups(:trashed_subproject).uuid,
-            format: :json,
-          }
-      assert_response 404
-    end
   end
-
 end
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index e2c8829..4672acd 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -69,30 +69,87 @@ class GroupTest < ActiveSupport::TestCase
     assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
     g_foo.update! is_trashed: true
     assert Collection.readable_by(users(:active)).where(uuid: col.uuid).empty?
+    assert Collection.readable_by(users(:active), {:include_trash => true}).where(uuid: col.uuid).any?
     g_foo.update! is_trashed: false
     assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
   end
 
+  test "delete group" do
+    set_user_from_auth :active_trustedclient
 
-  test "delete group propagates to subgroups" do
+    g_foo = Group.create!(name: "foo")
+    g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid)
+    g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid)
+
+    assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
+    assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
+    assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).any?
+    g_foo.update! is_trashed: true
+    assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).empty?
+    assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
+    assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
+
+    assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_foo.uuid).any?
+    assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_bar.uuid).any?
+    assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_baz.uuid).any?
+  end
+
+
+  test "delete subgroup" do
     set_user_from_auth :active_trustedclient
 
     g_foo = Group.create!(name: "foo")
     g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid)
-    col = Collection.create!(owner_uuid: g_bar.uuid)
+    g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid)
 
     assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
     assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
-    assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
+    assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).any?
+    g_bar.update! is_trashed: true
+
+    assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
+    assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
+    assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
+
+    assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_bar.uuid).any?
+    assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_baz.uuid).any?
+  end
+
+  test "delete subsubgroup" do
+    set_user_from_auth :active_trustedclient
+
+    g_foo = Group.create!(name: "foo")
+    g_bar = Group.create!(name: "bar", owner_uuid: g_foo.uuid)
+    g_baz = Group.create!(name: "baz", owner_uuid: g_bar.uuid)
+
+    assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
+    assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
+    assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).any?
+    g_baz.update! is_trashed: true
+    assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
+    assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
+    assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
+    assert Group.readable_by(users(:active), {:include_trash => true}).where(uuid: g_baz.uuid).any?
+  end
+
+
+  test "delete group propagates to subgroups" do
+    set_user_from_auth :active_trustedclient
+
+    g_foo = groups(:trashed_project)
+    g_bar = groups(:trashed_subproject)
+    g_baz = groups(:trashed_subproject3)
+    col = collections(:collection_in_trashed_subproject)
 
-    g_foo.update! is_trashed: true
     assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).empty?
     assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
+    assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
     assert Collection.readable_by(users(:active)).where(uuid: col.uuid).empty?
 
     set_user_from_auth :admin
     assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).empty?
     assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).empty?
+    assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
     assert Collection.readable_by(users(:active)).where(uuid: col.uuid).empty?
 
     set_user_from_auth :active_trustedclient
@@ -100,6 +157,12 @@ class GroupTest < ActiveSupport::TestCase
     assert Group.readable_by(users(:active)).where(uuid: g_foo.uuid).any?
     assert Group.readable_by(users(:active)).where(uuid: g_bar.uuid).any?
     assert Collection.readable_by(users(:active)).where(uuid: col.uuid).any?
+
+    # this one should still be deleted.
+    assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).empty?
+
+    g_baz.update! is_trashed: false
+    assert Group.readable_by(users(:active)).where(uuid: g_baz.uuid).any?
   end
 
 end

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list