Bug #18340

Deleting 'role' and 'filter' groups should delete them immediately

Added by Peter Amstutz about 1 month ago. Updated 18 days ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
11/17/2021
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

originally brought up in #18339

side Q: how does should trash interact with 'role' groups? It appears a trashed 'role' group continues to be traversed for permissions until it is actually deleted from the system. Consider whether trashing a role group should either (a) not be allowed if there are outgoing permissions, (b) the operation of putting it in the trash should also delete all the outgoing permissions, or (c) the concept of trash doesn't apply to the role group, and it is always deleted immediately.

From discussion:

We probably want option (c), non-project groups should skip being trashed and be deleted immediately.


Subtasks

Task #18354: Review 18340-delete-role-filter-groupsResolvedLucas Di Pentima

History

#1 Updated by Peter Amstutz about 1 month ago

  • Description updated (diff)

#2 Updated by Lucas Di Pentima 26 days ago

  • Assigned To set to Lucas Di Pentima

#3 Updated by Lucas Di Pentima 19 days ago

  • Status changed from New to In Progress

#4 Updated by Lucas Di Pentima 19 days ago

Updates at 43d9cc591 - branch 18340-delete-role-filter-groups
Test run: https://ci.arvados.org/job/developer-run-tests/2802/

  • Only use TrashableController.destroy on project groups.
  • Adds test.

#5 Updated by Ward Vandewege 19 days ago

Lucas Di Pentima wrote:

Updates at 43d9cc591 - branch 18340-delete-role-filter-groups
Test run: https://ci.arvados.org/job/developer-run-tests/2802/

  • Only use TrashableController.destroy on project groups.
  • Adds test.

Just one question, why the removal of `protected` on line 354? Otherwise, LGTM, thanks!

#6 Updated by Lucas Di Pentima 19 days ago

Just for the record: the protected declaration was duplicated on that file.

#7 Updated by Lucas Di Pentima 19 days ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados-private:commit:arvados|2a13c742cdca6fbae46a9ce1c6ae044633d03ce3.

#8 Updated by Peter Amstutz 18 days ago

  • Release set to 45

Also available in: Atom PDF