18340: Only trash project groups on delete calls.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima@curii.com>

diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb
index 8d15bb1c5062a0215a04f60fb9749cd7ad7e35d1..7fbb86c010c01df4d08cbde40c434c7f8b9063ae 100644 (file)
--- a/services/api/app/controllers/arvados/v1/groups_controller.rb
+++ b/services/api/app/controllers/arvados/v1/groups_controller.rb
@@ -10,6 +10,8 @@ class Arvados::V1::GroupsController < ApplicationController
   skip_before_action :find_object_by_uuid, only: :shared
   skip_before_action :render_404_if_no_object, only: :shared
 
+  TRASHABLE_CLASSES = ['project']
+
   def self._index_requires_parameters
     (super rescue {}).
       merge({
@@ -99,6 +101,15 @@ class Arvados::V1::GroupsController < ApplicationController
     end
   end
 
+  def destroy
+    if !TRASHABLE_CLASSES.include?(@object.group_class)
+      return @object.destroy
+      show
+    else
+      super # Calls destroy from TrashableController module
+    end
+  end
+
   def render_404_if_no_object
     if params[:action] == 'contents'
       if !params[:uuid]
@@ -351,8 +362,6 @@ class Arvados::V1::GroupsController < ApplicationController
     @offset = offset_all
   end
 
-  protected
-
   def exclude_home objectlist, klass
     # select records that are readable by current user AND
     #   the owner_uuid is a user (but not the current user) OR

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 79b66f35e83fa0e92cc3394c8a958e25273c803d..4dbccc5eb24edd07cdb09e6f2e400faa9c4f0c81 100644 (file)
--- a/services/api/test/functional/arvados/v1/groups_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb
@@ -538,9 +538,10 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase
     assert_includes(owners, groups(:asubproject).uuid)
   end
 
-  test "delete filter & role groups" do
-    authorize_with :admin
-    [:afiltergroup, :private_role].each do |grp|
+  [:afiltergroup, :private_role].each do |grp|
+    test "delete non-project group #{grp}" do
+      authorize_with :admin
+      assert_not_nil Group.find_by_uuid(groups(grp).uuid)
       assert !Group.find_by_uuid(groups(grp).uuid).is_trashed
       post :destroy, params: {
             id: groups(grp).uuid,