fix remote sync app settings issues (#1355, #1356)

CHANGELOG.rst

@@ -22,12 +22,15 @@ Changed
     - Upgrade LDAP dependencies (#1348)
 - **Projectroles**
     - Improve remote site deletion UI text labels (#1349)
+    - Store remote sync app setting foreign key UUIDs as strings (#1356)
 
 Fixed
 -----
 
 - **General**
     - Invalid env var retrieval for ``AUTH_LDAP*_START_TLS`` (#1351)
+- **Projectroles**
+    - Remote sync ``user_name`` crash with <0.13.3 target sites (#1355)
 
 
 v0.13.3 (2023-12-06)

docs/source/major_changes.rst

@@ -17,6 +17,7 @@ Release Highlights
 ==================
 
 - Add missing LDAP settings in siteinfo
+- Fix remote sync crash with target sites using SODAR Core <0.13.3
 - General bug fixes and minor updates
 
 Breaking Changes

projectroles/remote_projects.py

@@ -6,6 +6,7 @@
 import urllib
 
 from copy import deepcopy
+from packaging import version
 
 from django.conf import settings
 from django.contrib import auth
@@ -166,13 +167,14 @@ def _add_user(cls, sync_data, user):
         return sync_data
 
     @classmethod
-    def _add_app_setting(cls, sync_data, app_setting, all_defs):
+    def _add_app_setting(cls, sync_data, app_setting, all_defs, add_user_name):
         """
         Add app setting to sync data on source site.
 
         :param sync_data: Sync data to be updated (dict)
         :param app_setting: AppSetting object
         :param all_defs: All settings defs
+        :param add_user_name: Add user_name to sync data (boolean)
         :return: Updated sync_data (dict)
         """
         if app_setting.app_plugin:
@@ -193,29 +195,38 @@ def _add_app_setting(cls, sync_data, app_setting, all_defs):
             'app_plugin': app_setting.app_plugin.name
             if app_setting.app_plugin
             else None,
-            'project_uuid': app_setting.project.sodar_uuid
+            'project_uuid': str(app_setting.project.sodar_uuid)
             if app_setting.project
             else None,
-            'user_uuid': app_setting.user.sodar_uuid
-            if app_setting.user
-            else None,
-            'user_name': app_setting.user.username
+            'user_uuid': str(app_setting.user.sodar_uuid)
             if app_setting.user
             else None,
             'local': local,
         }
+        if add_user_name:
+            sync_data['app_settings'][str(app_setting.sodar_uuid)][
+                'user_name'
+            ] = (app_setting.user.username if app_setting.user else None)
         return sync_data
 
     # Source Site API functions ------------------------------------------------
 
-    def get_source_data(self, target_site):
+    def get_source_data(self, target_site, req_version=None):
         """
         Get user and project data on a source site to be synchronized into a
         target site.
 
         :param target_site: RemoteSite object for target site
+        :param req_version: Request version (string)
         :return: Dict
         """
+        # TODO: Remove user_name workaround when API backwards compatibility to
+        #       <0.13.3 is removed
+        if not req_version:
+            from projectroles.views_api import CORE_API_DEFAULT_VERSION
+
+            req_version = CORE_API_DEFAULT_VERSION
+        add_user_name = version.parse(req_version) >= version.parse('0.13.3')
         sync_data = {
             'users': {},
             'projects': {},
@@ -242,7 +253,9 @@ def get_source_data(self, target_site):
             # NOTE: Also provide global settings in case they are not yet set
             for a in AppSetting.objects.filter(project=project):
                 try:
-                    sync_data = self._add_app_setting(sync_data, a, all_defs)
+                    sync_data = self._add_app_setting(
+                        sync_data, a, all_defs, add_user_name
+                    )
                 except Exception as ex:
                     logger.error(
                         'Failed to add app setting "{}.settings.{}" '

projectroles/tests/test_remote_projects_api.py

@@ -489,7 +489,7 @@ def test_get_settings(self):
             'value': set_ip_restrict.value,
             'value_json': {},
             'app_plugin': None,
-            'project_uuid': self.project.sodar_uuid,
+            'project_uuid': str(self.project.sodar_uuid),
             'user_uuid': None,
             'user_name': None,
             'local': False,
@@ -502,7 +502,7 @@ def test_get_settings(self):
             'value': None,
             'value_json': set_ip_allowlist.value_json,
             'app_plugin': None,
-            'project_uuid': self.project.sodar_uuid,
+            'project_uuid': str(self.project.sodar_uuid),
             'user_uuid': None,
             'user_name': None,
             'local': False,
@@ -515,13 +515,48 @@ def test_get_settings(self):
             'value': '1',
             'value_json': {},
             'app_plugin': None,
-            'project_uuid': self.project.sodar_uuid,
-            'user_uuid': self.user_source.sodar_uuid,
+            'project_uuid': str(self.project.sodar_uuid),
+            'user_uuid': str(self.user_source.sodar_uuid),
             'user_name': self.user_source.username,
             'local': True,
         }
         self.assertEqual(set_data, expected)
 
+    def test_get_settings_legacy(self):
+        """Test getting data with legacy SODAR Core version"""
+        # See issue #1355
+        self.make_remote_project(
+            project_uuid=self.project.sodar_uuid,
+            site=self.target_site,
+            level=REMOTE_LEVEL_READ_ROLES,
+        )
+        set_star = self.make_setting(
+            app_name='projectroles',
+            name='project_star',
+            setting_type='BOOLEAN',
+            value=True,
+            project=self.project,
+            user=self.user_source,
+        )
+        sync_data = self.remote_api.get_source_data(
+            self.target_site, req_version='0.13.2'
+        )
+
+        self.assertEqual(len(sync_data['app_settings']), 1)
+        set_data = sync_data['app_settings'][str(set_star.sodar_uuid)]
+        expected = {
+            'name': set_star.name,
+            'type': set_star.type,
+            'value': '1',
+            'value_json': {},
+            'app_plugin': None,
+            'project_uuid': str(self.project.sodar_uuid),
+            'user_uuid': str(self.user_source.sodar_uuid),
+            'local': True,
+        }
+        self.assertEqual(set_data, expected)
+        self.assertNotIn('user_name', set_data)  # No user_name here
+
     def test_get_revoked(self):
         """Test getting data with REVOKED level"""
         user_source_new = self.make_user('new_source_user')

projectroles/tests/test_views_api.py

@@ -3614,6 +3614,7 @@ class TestRemoteProjectGetAPIView(
     RoleAssignmentMixin,
     RemoteSiteMixin,
     RemoteProjectMixin,
+    AppSettingMixin,
     SODARAPIViewTestMixin,
     TestViewsBase,
 ):
@@ -3661,22 +3662,64 @@ def test_get(self):
             reverse(
                 'projectroles:api_remote_get',
                 kwargs={'secret': REMOTE_SITE_SECRET},
-            )
+            ),
+            **self.get_accept_header()
         )
         self.assertEqual(response.status_code, 200)
-        expected = self.remote_api.get_source_data(self.target_site)
         response_dict = json.loads(response.content.decode('utf-8'))
+        expected = self.remote_api.get_source_data(self.target_site)
         self.assertEqual(response_dict, expected)
 
     def test_get_invalid_secret(self):
         """Test retrieving project data with invalid secret (should fail)"""
         response = self.client.get(
             reverse(
                 'projectroles:api_remote_get', kwargs={'secret': build_secret()}
-            )
+            ),
+            **self.get_accept_header()
         )
         self.assertEqual(response.status_code, 401)
 
+    def test_get_legacy_version(self):
+        """Test retrieving project data with legacy target site version"""
+        # See issue #1355
+        set_star = self.make_setting(
+            app_name='projectroles',
+            name='project_star',
+            setting_type='BOOLEAN',
+            value=True,
+            project=self.project,
+            user=self.user,
+        )
+        response = self.client.get(
+            reverse(
+                'projectroles:api_remote_get',
+                kwargs={'secret': REMOTE_SITE_SECRET},
+            ),
+            **self.get_accept_header(version='0.13.2')
+        )
+        self.assertEqual(response.status_code, 200)
+        response_dict = json.loads(response.content.decode('utf-8'))
+        expected = self.remote_api.get_source_data(
+            self.target_site, req_version='0.13.2'
+        )
+        self.assertEqual(response_dict, expected)
+        # Assert user_name is not present in PROJECT_USER setting
+        set_data = response_dict['app_settings'][str(set_star.sodar_uuid)]
+        self.assertNotIn('user_name', set_data)
+
+    def test_get_unsupported_version(self):
+        """Test retrieving project data with legacy target site version"""
+        # See issue #1355
+        response = self.client.get(
+            reverse(
+                'projectroles:api_remote_get',
+                kwargs={'secret': REMOTE_SITE_SECRET},
+            ),
+            **self.get_accept_header(version='0.12.0')
+        )
+        self.assertEqual(response.status_code, 406)
+
 
 # IP Allowing Tests ------------------------------------------------------------
 

projectroles/views_api.py

@@ -1246,7 +1246,7 @@ def get_object(self):
 class RemoteProjectGetAPIView(CoreAPIBaseMixin, APIView):
     """API view for retrieving remote projects from a source site"""
 
-    permission_classes = (AllowAny,)  # We check the secret in get()/post()
+    permission_classes = (AllowAny,)  # We check the secret in get()
 
     def get(self, request, *args, **kwargs):
         remote_api = RemoteProjectAPI()
@@ -1257,7 +1257,13 @@ def get(self, request, *args, **kwargs):
             )
         except RemoteSite.DoesNotExist:
             return Response('Remote site not found, unauthorized', status=401)
-        sync_data = remote_api.get_source_data(target_site)
+        # Pass request version to get_source_data()
+        accept_header = request.headers.get('Accept')
+        if accept_header and 'version=' in accept_header:
+            req_version = accept_header[accept_header.find('version=') + 8 :]
+        else:
+            req_version = CORE_API_DEFAULT_VERSION
+        sync_data = remote_api.get_source_data(target_site, req_version)
         # Update access date for target site remote projects
         target_site.projects.all().update(date_access=timezone.now())
         return Response(sync_data, status=200)