add owner transfer, add tests, update docs (#1391)

docs/source/app_projectroles_usage.rst

@@ -339,6 +339,19 @@ project permissions, or by a site admin using the ``batchupdateroles``
 management command. The latter supports multiple projects in one batch. It is
 also able to send invites to users who have not yet signed up on the site.
 
+Remove All Roles from User
+--------------------------
+
+To easily remove all roles from a user, use the ``removeroles`` management
+command. For owner roles, you can supply the user name of a user for whom to
+transfer those roles. If no owner is supplied, each ownership will be
+transferred to the parent category owner. Example:
+
+.. code-block:: console
+
+    $ ./manage.py removeroles --user alice --owner bob
+
+
 User Status Checking
 --------------------
 

docs/source/major_changes.rst

@@ -16,6 +16,7 @@ v1.1.0 (WIP)
 Release Highlights
 ==================
 
+- Add removeroles management command
 - Add app setting type constants
 - Add app setting definition as objects
 - Update app settings API

projectroles/management/commands/removeroles.py

@@ -10,7 +10,10 @@
 
 from projectroles.management.logging import ManagementCommandLogger
 from projectroles.models import RoleAssignment, SODAR_CONSTANTS
-from projectroles.views import RoleAssignmentDeleteMixin
+from projectroles.views import (
+    RoleAssignmentOwnerTransferMixin,
+    RoleAssignmentDeleteMixin,
+)
 
 logger = ManagementCommandLogger(__name__)
 User = get_user_model()
@@ -23,12 +26,25 @@
 USER_NOT_FOUND_MSG = 'User not found with username: {}'
 
 
-class Command(RoleAssignmentDeleteMixin, BaseCommand):
+class Command(
+    RoleAssignmentOwnerTransferMixin, RoleAssignmentDeleteMixin, BaseCommand
+):
     help = (
         'Remove all roles from a user. Replace owner roles with given user or '
         'parent owner.'
     )
 
+    @classmethod
+    def _get_parent_owner(cls, project, prev_owner):
+        """Return assignment for first parent owner who is not previous owner"""
+        if not project.parent:  # Can't traverse further
+            return None
+        parent_owner_as = project.parent.get_owner()
+        if parent_owner_as.user != prev_owner:
+            return parent_owner_as.user
+        # If the owner is the same, recurse
+        return cls._get_parent_owner(project.parent, prev_owner)
+
     def add_arguments(self, parser):
         parser.add_argument(
             '-u',
@@ -91,25 +107,65 @@ def handle(self, *args, **options):
                 continue
             # Owner role reassignment
             if role_as.role.name == PROJECT_ROLE_OWNER:
-                # TODO: If no set owner, get parent owner
-                # TODO: If parent owner is not found, fail and continue
-                # TODO: If parent owner has existing local role, promote that
-                # TODO: Else add owner role
-                pass
+                # Fail top category if no new owner is specified
+                if not owner and not project.parent:
+                    logger.warning(
+                        'Failed to transfer ownership for top level {} {}: no '
+                        'new owner provided'.format(
+                            project.type.lower(), p_title
+                        )
+                    )
+                    fail_count += 1
+                    continue
+                # Get parent owner if not set
+                if not owner:
+                    owner = self._get_parent_owner(project, user)
+                    # owner = old_owner_as.user
+                # Fail if alternate parent owner is not found
+                if not owner:
+                    logger.warning(
+                        'Failed to transfer ownership in {}: no parent owner '
+                        'found'.format(p_title)
+                    )
+                    fail_count += 1
+                    continue
+                try:
+                    with transaction.atomic():
+                        self.transfer_owner(
+                            project=project,
+                            new_owner=owner,
+                            old_owner_as=role_as,
+                            old_owner_role=None,
+                            notify_old=False,
+                        )
+                        logger.info(
+                            'Transferred ownership in {} to {}'.format(
+                                p_title, owner.username
+                            )
+                        )
+                        role_count += 1
+                except Exception as ex:
+                    logger.error(
+                        'Failed to transfer ownership in {} to {}: {}'.format(
+                            p_title, owner.username, ex
+                        )
+                    )
+                    fail_count += 1
             # Non-owner role removal
             else:
                 try:
                     with transaction.atomic():
                         self.delete_assignment(role_as, None, False)
+                    logger.info(
+                        'Deleted role "{}" from {}'.format(r_name, p_title)
+                    )
+                    role_count += 1
                 except Exception as ex:
                     logger.error(
                         'Failed to delete assignment "{}" from {}: '
                         '{}'.format(r_name, p_title, ex)
                     )
                     fail_count += 1
-                    continue
-                logger.info('Deleted role "{}" from {}'.format(r_name, p_title))
-                role_count += 1
         logger.info(
             'Removed roles from user "{}" ({} OK, {} failed)'.format(
                 user.username, role_count, fail_count

projectroles/tests/test_commands.py

@@ -1069,12 +1069,114 @@ def test_command_remote(self):
         self.assertEqual(self.category.has_role(self.user), True)
         self.assertEqual(self.project.has_role(self.user), True)
 
-    '''
-    def test_command_owner(self):
-        """Test removing owner role without defined owner"""
+    def test_command_owner_unset(self):
+        """Test removing owner with unset new owner"""
         self.assertEqual(self.category.has_role(self.user_owner_cat), True)
+        # Inherited owner role
+        self.assertEqual(self.project.has_role(self.user_owner_cat), True)
+        self.assertEqual(self.project.has_role(self.user_owner), True)
+        self.assertEqual(
+            TimelineEvent.objects.filter(
+                event_name='role_owner_transfer'
+            ).count(),
+            0,
+        )
+        self.assertEqual(
+            AppAlert.objects.filter(alert_name='role_update').count(), 0
+        )
+
+        call_command(self.cmd_name, user=self.user_owner)
+        self.assertEqual(self.category.has_role(self.user_owner_cat), True)
+        self.assertEqual(self.project.has_role(self.user_owner_cat), True)
+        self.assertEqual(self.project.has_role(self.user_owner), False)
+        # Assert local owner roles
+        self.assertEqual(self.category.get_owner().user, self.user_owner_cat)
+        self.assertEqual(self.project.get_owner().user, self.user_owner_cat)
+        self.assertEqual(
+            TimelineEvent.objects.filter(
+                event_name='role_owner_transfer'
+            ).count(),
+            1,
+        )
+        # No alerts for inherited owner or old removed user
+        self.assertEqual(
+            AppAlert.objects.filter(alert_name='role_update').count(), 0
+        )
+
+    def test_command_owner_unset_top_level(self):
+        """Test removing owner with unset new owner at top level (should fail)"""
+        self.assertEqual(self.category.has_role(self.user_owner_cat), True)
+        call_command(self.cmd_name, user=self.user_owner_cat)
+        self.assertEqual(self.category.has_role(self.user_owner_cat), True)
+
+    def test_command_owner_unset_same_owner(self):
+        """Test removing owner with unset new owner and same owner on all levels (should fail)"""
+        self.owner_as.user = self.user_owner_cat
+        self.owner_as.save()
+        self.assertEqual(self.category.get_owner().user, self.user_owner_cat)
+        self.assertEqual(self.project.get_owner().user, self.user_owner_cat)
+        call_command(self.cmd_name, user=self.user_owner_cat)
+        self.assertEqual(self.category.get_owner().user, self.user_owner_cat)
+        self.assertEqual(self.project.get_owner().user, self.user_owner_cat)
+
+    def test_command_owner_set_no_role(self):
+        """Test removing owner role with set new owner without existing role"""
+        user_new = self.make_user('user_new')
+        self.assertEqual(self.category.has_role(self.user_owner_cat), True)
+        self.assertEqual(self.project.has_role(self.user_owner_cat), True)
+        self.assertEqual(self.project.has_role(self.user_owner), True)
+        self.assertEqual(self.project.has_role(user_new), False)
+        self.assertEqual(
+            AppAlert.objects.filter(alert_name='role_update').count(), 0
+        )
+        call_command(self.cmd_name, user=self.user_owner, owner=user_new)
+        self.assertEqual(self.category.has_role(self.user_owner_cat), True)
+        self.assertEqual(self.project.has_role(self.user_owner_cat), True)
+        self.assertEqual(self.project.has_role(self.user_owner), False)
+        self.assertEqual(self.project.get_owner().user, user_new)
+        # Alert for new owner
+        self.assertEqual(
+            AppAlert.objects.filter(alert_name='role_update').count(), 1
+        )
+
+    def test_command_owner_set_no_role_top_level(self):
+        """Test removing owner role with set new owner without existing role on top level"""
+        user_new = self.make_user('user_new')
+        self.assertEqual(self.category.has_role(self.user_owner_cat), True)
+        self.assertEqual(self.project.has_role(self.user_owner_cat), True)
+        self.assertEqual(self.project.has_role(self.user_owner), True)
+        self.assertEqual(self.project.has_role(user_new), False)
+        call_command(self.cmd_name, user=self.user_owner_cat, owner=user_new)
+        self.assertEqual(self.category.has_role(self.user_owner_cat), False)
+        self.assertEqual(self.project.has_role(self.user_owner_cat), False)
+        self.assertEqual(self.project.has_role(self.user_owner), True)
+        self.assertEqual(self.category.get_owner().user, user_new)
+        self.assertEqual(self.project.has_role(user_new), True)
+
+    def test_command_owner_set_existing_role(self):
+        """Test removing owner role with set new owner and existing role"""
+        user_new = self.make_user('user_new')
+        self.make_assignment(self.project, user_new, self.role_contributor)
+        self.assertEqual(self.project.get_owner().user, self.user_owner)
+        self.assertEqual(self.project.has_role(user_new), True)
+        self.assertEqual(
+            RoleAssignment.objects.filter(user=user_new).count(), 1
+        )
+        call_command(self.cmd_name, user=self.user_owner, owner=user_new)
+        self.assertEqual(self.project.get_owner().user, user_new)
+        self.assertEqual(self.project.has_role(self.user_owner), False)
+        self.assertEqual(
+            RoleAssignment.objects.filter(user=user_new).count(), 1
+        )
+
+    def test_command_owner_invalid_new_owner_name(self):
+        """Test removing owner with invalid new owner name (should fail)"""
+        self.assertEqual(self.project.has_role(self.user_owner), True)
+        with self.assertRaises(SystemExit):
+            call_command(
+                self.cmd_name, user=self.user_owner, owner='INVALID-NAME'
+            )
         self.assertEqual(self.project.has_role(self.user_owner), True)
-    '''
 
 
 class TestSyncGroups(TestCase):

projectroles/views.py

@@ -2401,7 +2401,6 @@ def form_valid(self, form):
                 new_owner,
                 old_owner_as,
                 old_owner_role,
-                issuer=self.request.user,
                 request=self.request,
             )
         except Exception as ex: