From 33d6d66c71f87a060cb07090f861e3ee51b2ecc5 Mon Sep 17 00:00:00 2001
From: Mikko Nieminen <mikko.nieminen@bih-charite.de>
Date: Thu, 6 Jun 2024 10:38:46 +0200
Subject: [PATCH] add syncgroup tests (#352), refactor

---
 CHANGELOG.rst                                 |  2 +-
 .../management/commands/syncgroups.py         | 26 ++++-----
 projectroles/tests/test_commands.py           | 54 +++++++++++++++++++
 3 files changed, 64 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 647d03f8..a39be07e 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -36,7 +36,7 @@ Added
     - ``SODARUserAdditionalEmail`` model (#874)
     - ``is_source_site()`` and ``is_target_site()`` rule predicates
     - ``settings_link`` kwarg in ``send_generic_email()`` (#1418)
-    - ``addremotesite`` command tests (#352)
+    - ``addremotesite`` and ``syncgroups`` command tests (#352)
 - **Timeline**
     - ``sodar_uuid`` field in ``TimelineEventObjectRef`` model (#1415)
     - REST API views (#1350)
diff --git a/projectroles/management/commands/syncgroups.py b/projectroles/management/commands/syncgroups.py
index e3db91d6..8503add8 100644
--- a/projectroles/management/commands/syncgroups.py
+++ b/projectroles/management/commands/syncgroups.py
@@ -2,7 +2,6 @@
 
 from django.contrib import auth
 from django.core.management.base import BaseCommand
-from django.db import transaction
 
 from projectroles.management.logging import ManagementCommandLogger
 
@@ -14,24 +13,17 @@
 class Command(BaseCommand):
     help = 'Synchronizes user groups based on user name'
 
-    def add_arguments(self, parser):
-        pass
-
     def handle(self, *args, **options):
         logger.info('Synchronizing user groups..')
-        with transaction.atomic():
-            for user in User.objects.all():
-                user.groups.clear()
-                user.save()  # Group is updated during save
-
-                if user.groups.count() > 0:
-                    logger.info(
-                        'Group set: {} -> {}'.format(
-                            user.username, user.groups.first().name
-                        )
+        for user in User.objects.all().order_by('username'):
+            user.groups.clear()
+            user.save()  # Group is updated during save
+            if user.groups.count() > 0:
+                logger.info(
+                    'Group set: {} -> {}'.format(
+                        user.username, user.groups.first().name
                     )
+                )
         logger.info(
-            'Synchronized groups for {} users'.format(
-                User.objects.all().count()
-            )
+            'Synchronized groups for {} users'.format(User.objects.count())
         )
diff --git a/projectroles/tests/test_commands.py b/projectroles/tests/test_commands.py
index 49eb849f..e1cc0e0b 100644
--- a/projectroles/tests/test_commands.py
+++ b/projectroles/tests/test_commands.py
@@ -35,6 +35,9 @@
     DEV_USER_NAMES,
     DEFAULT_PASSWORD,
 )
+from projectroles.management.commands.syncgroups import (
+    Command as SyncGroupsCommand,
+)
 
 from projectroles.models import (
     RoleAssignment,
@@ -67,6 +70,7 @@
 SITE_MODE_SOURCE = SODAR_CONSTANTS['SITE_MODE_SOURCE']
 SITE_MODE_PEER = SODAR_CONSTANTS['SITE_MODE_PEER']
 SITE_MODE_TARGET = SODAR_CONSTANTS['SITE_MODE_TARGET']
+SYSTEM_USER_GROUP = SODAR_CONSTANTS['SYSTEM_USER_GROUP']
 
 # Local constants
 EXAMPLE_APP_NAME = 'example_project_app'
@@ -75,6 +79,7 @@
 REMOTE_SITE_NAME = 'Test Site'
 REMOTE_SITE_URL = 'https://example.com'
 REMOTE_SITE_SECRET = build_secret(32)
+USER_DOMAIN = 'EXAMPLE'
 
 
 class TestAddRemoteSite(
@@ -870,6 +875,55 @@ def test_command_project_user_scope(self):
             )
 
 
+class TestSyncGroups(TestCase):
+    """Tests for syncgroups command"""
+
+    def setUp(self):
+        self.command = SyncGroupsCommand()
+
+    def test_sync_system(self):
+        """Test sync with system username"""
+        user = self.make_user('user')
+        user.groups.all().delete()  # Remove groups
+        self.assertEqual(user.groups.count(), 0)
+        self.command.handle()
+        self.assertEqual(user.groups.count(), 1)
+        group = user.groups.first()
+        self.assertIsNotNone(group)
+        self.assertEqual(group.name, SYSTEM_USER_GROUP)
+
+    def test_sync_system_existing(self):
+        """Test sync with system username and existing group"""
+        user = self.make_user('user')
+        self.assertEqual(user.groups.count(), 1)
+        self.command.handle()
+        self.assertEqual(user.groups.count(), 1)
+        group = user.groups.first()
+        self.assertIsNotNone(group)
+        self.assertEqual(group.name, SYSTEM_USER_GROUP)
+
+    def test_sync_domain(self):
+        """Test sync with domain username"""
+        user = self.make_user('user@' + USER_DOMAIN)
+        user.groups.all().delete()
+        self.assertEqual(user.groups.count(), 0)
+        self.command.handle()
+        self.assertEqual(user.groups.count(), 1)
+        group = user.groups.first()
+        self.assertIsNotNone(group)
+        self.assertEqual(group.name, USER_DOMAIN.lower())
+
+    def test_sync_domain_existing(self):
+        """Test sync with domain username and existing group"""
+        user = self.make_user('user@' + USER_DOMAIN)
+        self.assertEqual(user.groups.count(), 1)
+        self.command.handle()
+        self.assertEqual(user.groups.count(), 1)
+        group = user.groups.first()
+        self.assertIsNotNone(group)
+        self.assertEqual(group.name, USER_DOMAIN.lower())
+
+
 @override_settings(DEBUG=True)
 class TestCreateDevUsers(TestCase):
     """Tests for createdevusers command"""