add remote site timeline data, fix issues (#817)

projectroles/tests/test_views.py

@@ -1078,6 +1078,7 @@ def setUp(self):
             'projectroles:update',
             kwargs={'project': self.category.sodar_uuid},
         )
+        self.timeline = get_backend_api('timeline_backend')
 
     def test_get_project(self):
         """Test GET with project"""
@@ -1248,7 +1249,6 @@ def test_get_not_found(self):
 
     def test_post_project_superuser(self):
         """Test POST for project as superuser"""
-        timeline = get_backend_api('timeline_backend')
         category_new = self.make_project('NewCat', PROJECT_TYPE_CATEGORY, None)
         self.make_assignment(category_new, self.user, self.role_owner)
         self.assertEqual(Project.objects.all().count(), 3)
@@ -1313,13 +1313,16 @@ def test_post_project_superuser(self):
             )
         # Assert timeline event
         tl_event = (
-            timeline.get_project_events(self.project).order_by('-pk').first()
+            self.timeline.get_project_events(self.project)
+            .order_by('-pk')
+            .first()
         )
         self.assertEqual(tl_event.event_name, 'project_update')
         self.assertIn('title', tl_event.extra_data)
         self.assertIn('description', tl_event.extra_data)
         self.assertIn('parent', tl_event.extra_data)
-        # TODO: Assert remote sites is NOT in once implemented
+        self.assertNotIn('remote_sites', tl_event.description)
+        self.assertNotIn('remote_sites', tl_event.extra_data)
         # No alert or mail, because the owner has not changed
         self.assertEqual(self.app_alert_model.objects.count(), 0)
         self.assertEqual(len(mail.outbox), 0)
@@ -1548,7 +1551,7 @@ def test_post_category_parent(self):
         self.assertEqual(self.app_alert_model.objects.count(), 0)
         self.assertEqual(len(mail.outbox), 0)
 
-    def test_post_project_parent_different_owner(self):
+    def test_post_parent_different_owner(self):
         """Test POST with updated project parent and different parent owner"""
         user_new = self.make_user('user_new')
         self.owner_as_cat.user = user_new
@@ -1579,7 +1582,7 @@ def test_post_project_parent_different_owner(self):
             mail.outbox[0].subject,
         )
 
-    def test_post_project_parent_different_owner_disable_email(self):
+    def test_post_parent_different_owner_disable_email(self):
         """Test POST with updated parent and different parent owner with disabled email"""
         user_new = self.make_user('user_new')
         self.owner_as_cat.user = user_new
@@ -1621,7 +1624,16 @@ def test_post_remote(self):
         self.assertEqual(rp.project, self.project)
         self.assertEqual(rp.site, self.remote_site)
         self.assertEqual(rp.level, REMOTE_LEVEL_READ_ROLES)
-        # TODO: Assert timeline event title and extra data once implemented
+        tl_event = (
+            self.timeline.get_project_events(self.project)
+            .order_by('-pk')
+            .first()
+        )
+        self.assertIn('remote_sites', tl_event.description)
+        self.assertEqual(
+            tl_event.extra_data['remote_sites'],
+            {str(self.remote_site.sodar_uuid): True},
+        )
 
     def test_post_remote_revoke(self):
         """Test POST to revoke existing remote project"""
@@ -1644,6 +1656,16 @@ def test_post_remote_revoke(self):
         self.assertEqual(RemoteProject.objects.count(), 1)
         rp.refresh_from_db()
         self.assertEqual(rp.level, REMOTE_LEVEL_REVOKED)
+        tl_event = (
+            self.timeline.get_project_events(self.project)
+            .order_by('-pk')
+            .first()
+        )
+        self.assertIn('remote_sites', tl_event.description)
+        self.assertEqual(
+            tl_event.extra_data['remote_sites'],
+            {str(self.remote_site.sodar_uuid): False},
+        )
 
     def test_post_remote_enable_revoked(self):
         """Test POST to re-enable revoked remote project"""
@@ -1666,6 +1688,16 @@ def test_post_remote_enable_revoked(self):
         self.assertEqual(RemoteProject.objects.count(), 1)
         rp.refresh_from_db()
         self.assertEqual(rp.level, REMOTE_LEVEL_READ_ROLES)
+        tl_event = (
+            self.timeline.get_project_events(self.project)
+            .order_by('-pk')
+            .first()
+        )
+        self.assertIn('remote_sites', tl_event.description)
+        self.assertEqual(
+            tl_event.extra_data['remote_sites'],
+            {str(self.remote_site.sodar_uuid): True},
+        )
 
     @override_settings(PROJECTROLES_SITE_MODE=SITE_MODE_TARGET)
     def test_post_target_remote(self):

projectroles/views.py

@@ -933,8 +933,12 @@ def call_project_modify_api(cls, method_name, revert_name, method_args):
 class ProjectModifyMixin(ProjectModifyPluginViewMixin):
     """Mixin for Project creation/updating in UI and API views"""
 
+    #: Remote site fields
+    site_fields = {}
+
     @staticmethod
     def _get_old_project_data(project):
+        """Get existing data from project fields"""
         return {
             'title': project.title,
             'parent': project.parent,
@@ -944,6 +948,21 @@ def _get_old_project_data(project):
             'public_guest_access': project.public_guest_access,
         }
 
+    @classmethod
+    def _get_remote_project_data(cls, project):
+        """Return existing remote project data"""
+        ret = {}
+        existing_sites = []
+        for rp in RemoteProject.objects.filter(project=project):
+            ret[str(rp.site.sodar_uuid)] = rp.level == REMOTE_LEVEL_READ_ROLES
+            existing_sites.append(rp.site.sodar_uuid)
+        # Sites not yet added
+        for rs in RemoteSite.objects.filter(
+            mode=SITE_MODE_TARGET, user_display=True, owner_modifiable=True
+        ).exclude(sodar_uuid__in=existing_sites):
+            ret[str(rs.sodar_uuid)] = False
+        return ret
+
     @staticmethod
     def _get_app_settings(data, instance, user):
         """
@@ -994,8 +1013,9 @@ def _get_app_settings(data, instance, user):
                     project_settings[s_name] = s_data
         return project_settings
 
-    @staticmethod
-    def _get_project_update_data(old_data, project, project_settings):
+    def _get_project_update_data(
+        self, old_data, project, old_sites, sites, project_settings
+    ):
         extra_data = {}
         upd_fields = []
         if old_data['title'] != project.title:
@@ -1014,7 +1034,20 @@ def _get_project_update_data(old_data, project, project_settings):
             extra_data['public_guest_access'] = project.public_guest_access
             upd_fields.append('public_guest_access')
 
-        # TODO: Handle remote sites here
+        # Remote projects
+        if (
+            settings.PROJECTROLES_SITE_MODE == SITE_MODE_SOURCE
+            and project.type == PROJECT_TYPE_PROJECT
+        ):
+            for s in [f.split('.')[1] for f in self.site_fields]:
+                if 'remote_sites' not in upd_fields and (
+                    s not in old_sites or bool(old_sites[s]) != bool(sites[s])
+                ):
+                    upd_fields.append('remote_sites')
+            if 'remote_sites' in upd_fields:
+                extra_data['remote_sites'] = {
+                    k: bool(v) for k, v in sites.items()
+                }
 
         # Settings
         for k, v in project_settings.items():
@@ -1037,9 +1070,76 @@ def _get_timeline_ok_status():
         else:
             return timeline.TL_STATUS_OK
 
+    def _update_remote_sites(self, project, data):
+        """Update project for remote sites"""
+        ret = {}
+        for f in self.site_fields:
+            site_uuid = f.split('.')[1]
+            site = RemoteSite.objects.filter(sodar_uuid=site_uuid).first()
+            # TODO: Validate site here
+            value = data[f]
+            rp = RemoteProject.objects.filter(
+                site=site, project=project
+            ).first()
+            modify = None
+            if rp and (
+                (value and rp.level != REMOTE_LEVEL_READ_ROLES)
+                or (not value and rp.level == REMOTE_LEVEL_READ_ROLES)
+            ):
+                rp.level = (
+                    REMOTE_LEVEL_READ_ROLES if value else REMOTE_LEVEL_REVOKED
+                )
+                rp.save()
+                modify = 'Updated'
+            elif not rp and value:  # Only create if value is True
+                rp = RemoteProject.objects.create(
+                    project_uuid=project.sodar_uuid,
+                    project=project,
+                    site=site,
+                    level=REMOTE_LEVEL_READ_ROLES,
+                )
+                modify = 'Created'
+            if modify:
+                logger.debug(
+                    '{} RemoteProject for site "{}" ({}): {}'.format(
+                        modify, site.name, site.sodar_uuid, rp.level
+                    )
+                )
+            ret[site_uuid] = rp and rp.level == REMOTE_LEVEL_READ_ROLES
+        return ret
+
     @classmethod
+    def _update_settings(cls, project, project_settings):
+        """Update project settings"""
+        is_remote = project.is_remote()
+        for k, v in project_settings.items():
+            _, plugin_name, setting_name = k.split('.', 3)
+            # Skip updating global settings on target site
+            if is_remote:
+                # TODO: Optimize (this can require a lot of queries)
+                s_def = app_settings.get_definition(
+                    setting_name, plugin_name=plugin_name
+                )
+                if app_settings.get_global_value(s_def):
+                    continue
+            app_settings.set(
+                plugin_name=k.split('.')[1],
+                setting_name=k.split('.')[2],
+                value=v,
+                project=project,
+                validate=True,
+            )
+
     def _create_timeline_event(
-        cls, project, action, owner, old_data, project_settings, request
+        self,
+        project,
+        action,
+        owner,
+        old_data,
+        old_sites,
+        sites,
+        project_settings,
+        request,
     ):
         timeline = get_backend_api('timeline_backend')
         if not timeline:
@@ -1065,9 +1165,8 @@ def _create_timeline_event(
 
         else:  # Update
             tl_desc = 'update ' + type_str.lower()
-            # TODO: Provide remote sites here
-            extra_data, upd_fields = cls._get_project_update_data(
-                old_data, project, project_settings
+            extra_data, upd_fields = self._get_project_update_data(
+                old_data, project, old_sites, sites, project_settings
             )
             if extra_data.get('parent'):  # Convert parent object into UUID
                 extra_data['parent'] = str(extra_data['parent'].sodar_uuid)
@@ -1086,64 +1185,6 @@ def _create_timeline_event(
             tl_event.add_object(owner, 'owner', owner.username)
         return tl_event
 
-    @classmethod
-    def _update_settings(cls, project, project_settings):
-        """Update project settings"""
-        is_remote = project.is_remote()
-        for k, v in project_settings.items():
-            _, plugin_name, setting_name = k.split('.', 3)
-            # Skip updating global settings on target site
-            if is_remote:
-                # TODO: Optimize (this can require a lot of queries)
-                s_def = app_settings.get_definition(
-                    setting_name, plugin_name=plugin_name
-                )
-                if app_settings.get_global_value(s_def):
-                    continue
-            app_settings.set(
-                plugin_name=k.split('.')[1],
-                setting_name=k.split('.')[2],
-                value=v,
-                project=project,
-                validate=True,
-            )
-
-    def _update_remote_sites(self, project, data, site_fields):
-        """Update remote sites"""
-        for f in site_fields:
-            site_uuid = f.split('.')[1]
-            site = RemoteSite.objects.filter(sodar_uuid=site_uuid).first()
-            # TODO: Validate site
-            value = data[f]
-            rp = RemoteProject.objects.filter(
-                site=site, project=project
-            ).first()
-            modify = None
-            if rp and (
-                (value and rp.level != REMOTE_LEVEL_READ_ROLES)
-                or (not value and rp.level == REMOTE_LEVEL_READ_ROLES)
-            ):
-                rp.level = (
-                    REMOTE_LEVEL_READ_ROLES if value else REMOTE_LEVEL_REVOKED
-                )
-                rp.save()
-                modify = 'Updated'
-            elif value:  # Only create non-existing project if value is True
-                rp = RemoteProject.objects.create(
-                    project_uuid=project.sodar_uuid,
-                    project=project,
-                    site=site,
-                    level=REMOTE_LEVEL_READ_ROLES,
-                )
-                modify = 'Created'
-            if modify:
-                logger.debug(
-                    '{} RemoteProject for site "{}" ({}): {}'.format(
-                        modify, site.name, site.sodar_uuid, rp.level
-                    )
-                )
-            # TODO: Return (only!) updated data
-
     @classmethod
     def _notify_users(cls, project, action, owner, old_parent, request):
         """
@@ -1283,14 +1324,15 @@ def modify_project(self, data, request, instance=None):
             owner = old_project.get_owner().user
 
         # Create/update RemoteProject objects
+        old_sites = {}
+        sites = {}
         if (
             settings.PROJECTROLES_SITE_MODE == SITE_MODE_SOURCE
             and project.type == PROJECT_TYPE_PROJECT
         ):
-            site_fields = [f for f in data if f.startswith('remote_site.')]
-            # TODO: Get old RemoteProject data (store in old_data)
-            # TODO: Return data to be compared with old data here
-            self._update_remote_sites(project, data, site_fields)
+            self.site_fields = [f for f in data if f.startswith('remote_site.')]
+            old_sites = self._get_remote_project_data(project)
+            sites = self._update_remote_sites(project, data)
 
         # Get app settings, store old settings
         project_settings = self._get_app_settings(data, project, request.user)
@@ -1299,9 +1341,15 @@ def modify_project(self, data, request, instance=None):
             old_settings = json.loads(json.dumps(project_settings))  # Copy
 
         # Create timeline event
-        # TODO: Add remote project data here
         tl_event = self._create_timeline_event(
-            project, action, owner, old_data, project_settings, request
+            project,
+            action,
+            owner,
+            old_data,
+            old_sites,
+            sites,
+            project_settings,
+            request,
         )
         # Get old parent for project moving
         old_parent = (