fix addremotesite issues (#1425, #1426), add tests (#352)

bihealth · Jun 5, 2024 · d450c46 · d450c46
1 parent ff5395f
commit d450c46
Show file tree

Hide file tree

Showing 4 changed files with 232 additions and 75 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -36,6 +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)
 - **Timeline**
     - ``sodar_uuid`` field in ``TimelineEventObjectRef`` model (#1415)
     - REST API views (#1350)
@@ -94,6 +95,8 @@ Fixed
     - Legacy public guest access in child category breaks category updating (#1404)
     - Incorrect DAL widget highlight colour after upgrade (#1412)
     - ``ProjectStarringAjaxView`` creating redundant database objects (#1416)
+    - ``addremotesite`` crash in ``TimelineAPI.add_event()`` (#1425)
+    - ``addremotesite`` allows creation of site with mode identical to host (#1426)
 - **Sodarcache**
     - REST API set view ``app_name`` incorrectly set (#1405)
 

diff --git a/projectroles/management/commands/addremotesite.py b/projectroles/management/commands/addremotesite.py
@@ -6,9 +6,9 @@
 import re
 import sys
 
+from django.conf import settings
 from django.contrib import auth
 from django.core.management.base import BaseCommand
-from django.db import transaction
 
 from projectroles.management.logging import ManagementCommandLogger
 from projectroles.models import RemoteSite, SODAR_CONSTANTS
@@ -93,65 +93,66 @@ def add_arguments(self, parser):
         )
 
     def handle(self, *args, **options):
+        timeline = get_backend_api('timeline_backend')
         logger.info('Creating remote site..')
         name = options['name']
         url = options['url']
-        # Validate url
+
+        # Validate URL
         if not url.startswith('http://') and not url.startswith('https://'):
             url = ''.join(['http://', url])
         pattern = re.compile(r'(http|https)://.*\..*')
         if not pattern.match(url):
             logger.error('Invalid URL "{}"'.format(url))
             sys.exit(1)
 
-        mode = options['mode'].upper()
-        # Validate mode
-        if mode not in [SITE_MODE_SOURCE, SITE_MODE_TARGET]:
-            if mode in [SITE_MODE_PEER]:
-                logger.error('Creating PEER sites is not allowed')
-            else:
-                logger.error('Unkown mode "{}"'.format(mode))
+        # Validate site mode
+        site_mode = options['mode'].upper()
+        host_mode = settings.PROJECTROLES_SITE_MODE
+        if site_mode not in [SITE_MODE_SOURCE, SITE_MODE_TARGET]:
+            logger.error('Invalid mode "{}"'.format(site_mode))
+            sys.exit(1)
+        if site_mode == host_mode:
+            logger.error('Attempting to create site with the same mode as host')
             sys.exit(1)
-
-        description = options['description']
-        secret = options['secret']
-        user_diplsay = options['user_display']
-        suppress_error = options['suppress_error']
 
         # Validate whether site exists
-        name_exists = bool(len(RemoteSite.objects.filter(name=name)))
-        url_exists = bool(len(RemoteSite.objects.filter(url=url)))
+        name_exists = RemoteSite.objects.filter(name=name).count()
+        url_exists = RemoteSite.objects.filter(url=url).count()
         if name_exists or url_exists:
             err_msg = 'Remote site exists with {} "{}"'.format(
                 'name' if name_exists else 'URL', name if name_exists else url
             )
-            if not suppress_error:
+            if not options['suppress_error']:
                 logger.error(err_msg)
                 sys.exit(1)
             else:
                 logger.info(err_msg)
                 sys.exit(0)
 
-        with transaction.atomic():
-            create_values = {
-                'name': name,
-                'url': url,
-                'mode': mode,
-                'description': description,
-                'secret': secret,
-                'user_display': user_diplsay,
-            }
-            site = RemoteSite.objects.create(**create_values)
-
-        timeline = get_backend_api('timeline_backend')
-        timeline.add_event(
-            project=None,
-            app_name='projectroles',
-            event_name='remote_site_create',
-            description=description,
-            classified=True,
-            status_type=timeline.TL_STATUS_OK,
-        )
+        create_kw = {
+            'name': name,
+            'url': url,
+            'mode': site_mode,
+            'description': options['description'],
+            'secret': options['secret'],
+            'user_display': options['user_display'],
+        }
+        site = RemoteSite.objects.create(**create_kw)
+
+        if timeline:
+            tl_event = timeline.add_event(
+                project=None,
+                app_name='projectroles',
+                user=None,
+                event_name='{}_site_create'.format(site_mode.lower()),
+                description='create {} remote site {{{}}} via management '
+                'command'.format(site_mode.lower(), 'remote_site'),
+                classified=True,
+                status_type=timeline.TL_STATUS_OK,
+                extra_data=create_kw,
+            )
+            tl_event.add_object(obj=site, label='remote_site', name=site.name)
         logger.info(
             'Created remote site "{}" with mode {}'.format(site.name, site.mode)
         )