diff --git a/projectroles/tests/test_views.py b/projectroles/tests/test_views.py index e84fba66..739547b0 100644 --- a/projectroles/tests/test_views.py +++ b/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): diff --git a/projectroles/views.py b/projectroles/views.py index 204e1f64..35777a8f 100644 --- a/projectroles/views.py +++ b/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 = (