From 9065876fd2e454db99dce5ac537c73cb2624a0fd Mon Sep 17 00:00:00 2001 From: David Wallace Date: Wed, 8 Jan 2025 10:52:48 +0100 Subject: [PATCH 1/7] feat(project,handlers): add sync for views and tasks, #966 #1198 Signed-off-by: David Wallace --- rdmo/core/settings.py | 3 +- rdmo/projects/apps.py | 6 +- rdmo/projects/handlers.py | 60 -------- rdmo/projects/handlers/__init__.py | 0 rdmo/projects/handlers/generic_handlers.py | 110 +++++++++++++++ rdmo/projects/handlers/project_tasks.py | 44 ++++++ rdmo/projects/handlers/project_views.py | 45 ++++++ rdmo/projects/handlers/utils.py | 10 ++ rdmo/projects/managers.py | 14 ++ rdmo/projects/tests/helpers.py | 5 + rdmo/projects/tests/test_handlers.py | 61 --------- rdmo/projects/tests/test_handlers_tasks.py | 151 +++++++++++++++++++++ rdmo/projects/tests/test_handlers_views.py | 116 ++++++++++++++++ testing/config/settings/base.py | 3 +- 14 files changed, 503 insertions(+), 125 deletions(-) delete mode 100644 rdmo/projects/handlers.py create mode 100644 rdmo/projects/handlers/__init__.py create mode 100644 rdmo/projects/handlers/generic_handlers.py create mode 100644 rdmo/projects/handlers/project_tasks.py create mode 100644 rdmo/projects/handlers/project_views.py create mode 100644 rdmo/projects/handlers/utils.py create mode 100644 rdmo/projects/tests/helpers.py delete mode 100644 rdmo/projects/tests/test_handlers.py create mode 100644 rdmo/projects/tests/test_handlers_tasks.py create mode 100644 rdmo/projects/tests/test_handlers_views.py diff --git a/rdmo/core/settings.py b/rdmo/core/settings.py index c6610be4de..53293abf63 100644 --- a/rdmo/core/settings.py +++ b/rdmo/core/settings.py @@ -332,7 +332,8 @@ PROJECT_SEND_INVITE = True -PROJECT_REMOVE_VIEWS = True +PROJECT_VIEWS_SYNC = True +PROJECT_TASKS_SYNC = True PROJECT_CREATE_RESTRICTED = False PROJECT_CREATE_GROUPS = [] diff --git a/rdmo/projects/apps.py b/rdmo/projects/apps.py index 178bb4ccd3..d66f40735d 100644 --- a/rdmo/projects/apps.py +++ b/rdmo/projects/apps.py @@ -10,5 +10,7 @@ class ProjectsConfig(AppConfig): def ready(self): from . import rules # noqa: F401 - if settings.PROJECT_REMOVE_VIEWS: - from . import handlers # noqa: F401 + if settings.PROJECT_VIEWS_SYNC: + from .handlers import project_views # noqa: F401 + if settings.PROJECT_TASKS_SYNC: + from .handlers import project_tasks # noqa: F401 diff --git a/rdmo/projects/handlers.py b/rdmo/projects/handlers.py deleted file mode 100644 index 6f65709cbe..0000000000 --- a/rdmo/projects/handlers.py +++ /dev/null @@ -1,60 +0,0 @@ -import logging - -from django.contrib.auth.models import User -from django.contrib.sites.models import Site -from django.db.models.signals import m2m_changed -from django.dispatch import receiver - -from rdmo.projects.models import Membership, Project -from rdmo.questions.models import Catalog -from rdmo.views.models import View - -logger = logging.getLogger(__name__) - - -@receiver(m2m_changed, sender=View.catalogs.through) -def m2m_changed_view_catalog_signal(sender, instance, **kwargs): - catalogs = instance.catalogs.all() - - if catalogs: - catalog_candidates = Catalog.objects.exclude(id__in=[catalog.id for catalog in catalogs]) - - # Remove catalog candidates for all sites - projects = Project.objects.filter(catalog__in=catalog_candidates, views=instance) - for proj in projects: - proj.views.remove(instance) - - -@receiver(m2m_changed, sender=View.sites.through) -def m2m_changed_view_sites_signal(sender, instance, **kwargs): - sites = instance.sites.all() - catalogs = instance.catalogs.all() - - if sites: - site_candidates = Site.objects.exclude(id__in=[site.id for site in sites]) - if not catalogs: - # if no catalogs are selected, update all - catalogs = Catalog.objects.all() - - # Restrict chosen catalogs for chosen sites - projects = Project.objects.filter(site__in=site_candidates, catalog__in=catalogs, views=instance) - for project in projects: - project.views.remove(instance) - - -@receiver(m2m_changed, sender=View.groups.through) -def m2m_changed_view_groups_signal(sender, instance, **kwargs): - groups = instance.groups.all() - catalogs = instance.catalogs.all() - - if groups: - users = User.objects.exclude(groups__in=groups) - memberships = [membership.id for membership in Membership.objects.filter(role='owner', user__in=users)] - if not catalogs: - # if no catalogs are selected, update all - catalogs = Catalog.objects.all() - - # Restrict chosen catalogs for chosen groups - projects = Project.objects.filter(memberships__in=list(memberships), catalog__in=catalogs, views=instance) - for project in projects: - project.views.remove(instance) diff --git a/rdmo/projects/handlers/__init__.py b/rdmo/projects/handlers/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/rdmo/projects/handlers/generic_handlers.py b/rdmo/projects/handlers/generic_handlers.py new file mode 100644 index 0000000000..1c061dcece --- /dev/null +++ b/rdmo/projects/handlers/generic_handlers.py @@ -0,0 +1,110 @@ +from django.contrib.auth.models import User + +from rdmo.projects.models import Membership, Project + +from .utils import add_instance_to_projects, remove_instance_from_projects + + +def m2m_catalogs_changed_projects_sync_signal_handler(action, related_model, pk_set, instance, project_field): + """ + Update project relationships for m2m_changed signals. + + Args: + action (str): The m2m_changed action (post_add, post_remove, post_clear). + related_model (Model): The related model (e.g., Catalog). + pk_set (set): The set of primary keys for the related model instances. + instance (Model): The instance being updated (e.g., View or Task). + project_field (str): The field on Project to update (e.g., 'views', 'tasks'). + """ + if action == 'post_remove' and pk_set: + related_instances = related_model.objects.filter(pk__in=pk_set) + projects_to_change = Project.objects.filter_catalogs(catalogs=related_instances).filter( + **{project_field: instance} + ) + remove_instance_from_projects(projects_to_change, project_field, instance) + + elif action == 'post_clear': + projects_to_change = Project.objects.filter(**{project_field: instance}) + remove_instance_from_projects(projects_to_change, project_field, instance) + + elif action == 'post_add' and pk_set: + related_instances = related_model.objects.filter(pk__in=pk_set) + projects_to_change = Project.objects.filter_catalogs(catalogs=related_instances).exclude( + **{project_field: instance} + ) + add_instance_to_projects(projects_to_change, project_field, instance) + + +def m2m_sites_changed_projects_sync_signal_handler(action, model, pk_set, instance, project_field): + """ + Synchronize Project relationships for m2m_changed signals triggered by site updates. + + Args: + action (str): The m2m_changed action (post_add, post_remove, post_clear). + model (Model): The related model (e.g., Site). + pk_set (set): The set of primary keys for the related model instances. + instance (Model): The instance being updated (e.g., View or Task). + project_field (str): The field on Project to update (e.g., 'views', 'tasks'). + """ + if action == 'post_remove' and pk_set: + related_sites = model.objects.filter(pk__in=pk_set) + catalogs = instance.catalogs.all() + + projects_to_change = Project.objects.filter_catalogs(catalogs=catalogs).filter( + site__in=related_sites, + **{project_field: instance} + ) + remove_instance_from_projects(projects_to_change, project_field, instance) + + elif action == 'post_clear': + projects_to_change = Project.objects.filter_catalogs().filter(**{project_field: instance}) + remove_instance_from_projects(projects_to_change, project_field, instance) + + elif action == 'post_add' and pk_set: + related_sites = model.objects.filter(pk__in=pk_set) + catalogs = instance.catalogs.all() + + projects_to_change = Project.objects.filter_catalogs(catalogs=catalogs).filter( + site__in=related_sites + ).exclude(**{project_field: instance}) + add_instance_to_projects(projects_to_change, project_field, instance) + + +def m2m_groups_changed_projects_sync_signal_handler(action, model, pk_set, instance, project_field): + """ + Synchronize Project relationships for m2m_changed signals triggered by group updates. + + Args: + action (str): The m2m_changed action (post_add, post_remove, post_clear). + model (Model): The related model (e.g., Group). + pk_set (set): The set of primary keys for the related model instances. + instance (Model): The instance being updated (e.g., View or Task). + project_field (str): The field on Project to update (e.g., 'views', 'tasks'). + """ + if action == 'post_remove' and pk_set: + related_groups = model.objects.filter(pk__in=pk_set) + users = User.objects.filter(groups__in=related_groups) + memberships = Membership.objects.filter(role='owner', user__in=users).values_list('id', flat=True) + catalogs = instance.catalogs.all() + + projects_to_change = Project.objects.filter_catalogs(catalogs=catalogs).filter( + memberships__in=memberships, + **{project_field: instance} + ) + remove_instance_from_projects(projects_to_change, project_field, instance) + + elif action == 'post_clear': + # Remove all linked projects regardless of catalogs + projects_to_change = Project.objects.filter_catalogs().filter(**{project_field: instance}) + remove_instance_from_projects(projects_to_change, project_field, instance) + + elif action == 'post_add' and pk_set: + related_groups = model.objects.filter(pk__in=pk_set) + users = User.objects.filter(groups__in=related_groups) + memberships = Membership.objects.filter(role='owner', user__in=users).values_list('id', flat=True) + catalogs = instance.catalogs.all() + + projects_to_change = Project.objects.filter_catalogs(catalogs=catalogs).filter( + memberships__in=memberships + ).exclude(**{project_field: instance}) + add_instance_to_projects(projects_to_change, project_field, instance) diff --git a/rdmo/projects/handlers/project_tasks.py b/rdmo/projects/handlers/project_tasks.py new file mode 100644 index 0000000000..04dc19c05f --- /dev/null +++ b/rdmo/projects/handlers/project_tasks.py @@ -0,0 +1,44 @@ + +from django.db.models.signals import m2m_changed +from django.dispatch import receiver + +from rdmo.tasks.models import Task + +from .generic_handlers import ( + m2m_catalogs_changed_projects_sync_signal_handler, + m2m_groups_changed_projects_sync_signal_handler, + m2m_sites_changed_projects_sync_signal_handler, +) + + +@receiver(m2m_changed, sender=Task.catalogs.through) +def m2m_changed_task_catalog_signal(sender, instance, action, model, pk_set, **kwargs): + m2m_catalogs_changed_projects_sync_signal_handler( + action=action, + related_model=model, + pk_set=pk_set, + instance=instance, + project_field='tasks', + ) + + +@receiver(m2m_changed, sender=Task.sites.through) +def m2m_changed_task_sites_signal(sender, instance, action, model, pk_set, **kwargs): + m2m_sites_changed_projects_sync_signal_handler( + action=action, + model=model, + pk_set=pk_set, + instance=instance, + project_field='tasks' + ) + + +@receiver(m2m_changed, sender=Task.groups.through) +def m2m_changed_task_groups_signal(sender, instance, action, model, pk_set, **kwargs): + m2m_groups_changed_projects_sync_signal_handler( + action=action, + model=model, + pk_set=pk_set, + instance=instance, + project_field='tasks' + ) diff --git a/rdmo/projects/handlers/project_views.py b/rdmo/projects/handlers/project_views.py new file mode 100644 index 0000000000..458109c735 --- /dev/null +++ b/rdmo/projects/handlers/project_views.py @@ -0,0 +1,45 @@ + +from django.db.models.signals import m2m_changed +from django.dispatch import receiver + +from rdmo.views.models import View + +from .generic_handlers import ( + m2m_catalogs_changed_projects_sync_signal_handler, + m2m_groups_changed_projects_sync_signal_handler, + m2m_sites_changed_projects_sync_signal_handler, +) + + +@receiver(m2m_changed, sender=View.catalogs.through) +def m2m_changed_view_catalog_signal(sender, instance, action, model, pk_set, **kwargs): + m2m_catalogs_changed_projects_sync_signal_handler( + action=action, + related_model=model, + pk_set=pk_set, + instance=instance, + project_field='views', + ) + + + +@receiver(m2m_changed, sender=View.sites.through) +def m2m_changed_view_sites_signal(sender, instance, action, model, pk_set, **kwargs): + m2m_sites_changed_projects_sync_signal_handler( + action=action, + model=model, + pk_set=pk_set, + instance=instance, + project_field='views' + ) + + +@receiver(m2m_changed, sender=View.groups.through) +def m2m_changed_view_groups_signal(sender, instance, action, model, pk_set, **kwargs): + m2m_groups_changed_projects_sync_signal_handler( + action=action, + model=model, + pk_set=pk_set, + instance=instance, + project_field='views' + ) diff --git a/rdmo/projects/handlers/utils.py b/rdmo/projects/handlers/utils.py new file mode 100644 index 0000000000..3175010938 --- /dev/null +++ b/rdmo/projects/handlers/utils.py @@ -0,0 +1,10 @@ + + +def remove_instance_from_projects(projects, project_field, instance): + for project in projects: + getattr(project, project_field).remove(instance) + + +def add_instance_to_projects(projects, project_field, instance): + for project in projects: + getattr(project, project_field).add(instance) diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index a6d49e4ff6..369b592f46 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -35,6 +35,16 @@ def filter_visibility(self, user): visibility_filter = Q(visibility__isnull=False) & sites_filter & groups_filter return self.filter(Q(user=user) | visibility_filter) + def filter_catalogs(self, catalogs=None, exclude_catalogs=None, exclude_null=True): + catalogs_filter = Q() + if exclude_null: + catalogs_filter &= Q(catalog__isnull=False) + if catalogs: + catalogs_filter &= Q(catalog__in=catalogs) + if exclude_catalogs: + catalogs_filter &= ~Q(catalog__in=exclude_catalogs) + return self.filter(catalogs_filter) + class MembershipQuerySet(models.QuerySet): @@ -167,6 +177,10 @@ def filter_user(self, user): def filter_visibility(self, user): return self.get_queryset().filter_visibility(user) + def filter_catalogs(self, catalogs=None, exclude_catalogs=None, exclude_null=True): + return self.get_queryset().filter_catalogs(catalogs=catalogs, exclude_catalogs=exclude_catalogs, + exclude_null=exclude_null) + class MembershipManager(CurrentSiteManagerMixin, models.Manager): diff --git a/rdmo/projects/tests/helpers.py b/rdmo/projects/tests/helpers.py new file mode 100644 index 0000000000..db04dd17da --- /dev/null +++ b/rdmo/projects/tests/helpers.py @@ -0,0 +1,5 @@ + + +def assert_other_projects_unchanged(other_projects, initial_tasks_state): + for other_project in other_projects: + assert set(other_project.tasks.values_list('id', flat=True)) == set(initial_tasks_state[other_project.id]) diff --git a/rdmo/projects/tests/test_handlers.py b/rdmo/projects/tests/test_handlers.py deleted file mode 100644 index 83a9c3c3c8..0000000000 --- a/rdmo/projects/tests/test_handlers.py +++ /dev/null @@ -1,61 +0,0 @@ -import itertools - -import pytest - -from django.contrib.auth.models import Group -from django.contrib.sites.models import Site - -from rdmo.projects.models import Project -from rdmo.questions.models import Catalog -from rdmo.views.models import View - -view_update_tests = [ - # tuples of: view_id, sites, catalogs, groups, project_id, project_exists - ('3', [], [], [], '10', True), - ('3', [2], [], [], '10', False), - ('3', [1, 2, 3], [], [], '10', True), - ('3', [], [2], [], '10', False), - ('3', [2], [2], [], '10', False), - ('3', [1, 2, 3], [2], [], '10', False), - ('3', [], [1, 2], [], '10', True), - ('3', [2], [1, 2], [], '10', False), - ('3', [1, 2, 3], [1, 2], [], '10', True), - - ('3', [], [], [1], '10', False), - ('3', [2], [], [1], '10', False), - ('3', [1, 2, 3], [], [1], '10', False), - ('3', [], [2], [1], '10', False), - ('3', [2], [2], [1], '10', False), - ('3', [1, 2, 3], [2], [1], '10', False), - ('3', [], [1, 2], [1], '10', False), - ('3', [2], [1, 2], [1], '10', False), - ('3', [1, 2, 3], [1, 2], [1], '10', False), - - ('3', [], [], [1, 2, 3, 4], '10', False), - ('3', [2], [], [1, 2, 3, 4], '10', False), - ('3', [1, 2, 3], [], [1, 2, 3, 4], '10', False), - ('3', [], [2], [1, 2, 3, 4], '10', False), - ('3', [2], [2], [1, 2, 3, 4], '10', False), - ('3', [1, 2, 3], [2], [1, 2, 3, 4], '10', False), - ('3', [], [1, 2], [1, 2, 3, 4], '10', False), - ('3', [2], [1, 2], [1, 2, 3, 4], '10', False), - ('3', [1, 2, 3], [1, 2], [1, 2, 3, 4], '10', False) -] - -@pytest.mark.parametrize('view_id,sites,catalogs,groups,project_id,project_exists', view_update_tests) -def test_update_projects(db, view_id, sites, catalogs, groups, project_id, project_exists): - view = View.objects.get(pk=view_id) - - view.sites.set(Site.objects.filter(pk__in=sites)) - view.catalogs.set(Catalog.objects.filter(pk__in=catalogs)) - view.groups.set(Group.objects.filter(pk__in=groups)) - - assert sorted(itertools.chain.from_iterable(view.sites.all().values_list('pk'))) == sites - assert sorted(itertools.chain.from_iterable(view.catalogs.all().values_list('pk'))) == catalogs - assert sorted(itertools.chain.from_iterable(view.groups.all().values_list('pk'))) == groups - - if not project_exists: - with pytest.raises(Project.DoesNotExist): - Project.objects.filter(views=view).get(pk=project_id) - else: - assert Project.objects.filter(views=view).get(pk=project_id) diff --git a/rdmo/projects/tests/test_handlers_tasks.py b/rdmo/projects/tests/test_handlers_tasks.py new file mode 100644 index 0000000000..cf710fd09a --- /dev/null +++ b/rdmo/projects/tests/test_handlers_tasks.py @@ -0,0 +1,151 @@ + +from django.contrib.auth.models import Group + +from rdmo.projects.models import Project +from rdmo.tasks.models import Task + +from .helpers import assert_other_projects_unchanged + +project_id = 10 +task_id = 1 +group_name = 'view_test' + +def test_project_tasks_sync_when_adding_or_removing_a_catalog_to_or_from_a_task(db, settings): + assert settings.PROJECT_TASKS_SYNC + + # Setup: Create a catalog, a task, and a project using the catalog + project = Project.objects.get(id=project_id) + catalog = project.catalog + other_projects = Project.objects.exclude(catalog=catalog) # All other projects + task = Task.objects.get(id=task_id) # This task does not have catalogs in the fixture + task.catalogs.clear() + initial_project_tasks = project.tasks.values_list('id', flat=True) + + # Save initial state of tasks for other projects + initial_other_project_tasks = { + i.id: list(i.tasks.values_list('id', flat=True)) + for i in other_projects + } + + # Ensure the project does not have the task initially + assert task not in project.tasks.all() + + ## Tests for .add and .remove + # Add the catalog to the task and assert that the project now includes the task + task.catalogs.add(catalog) + assert task in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + # Remove the catalog from the task and assert that the project no longer includes the task + task.catalogs.remove(catalog) + assert task not in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + ## Tests for .set and .clear + # Add the catalog to the task and assert that the project now includes the task + task.catalogs.set([catalog]) + assert task in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + # Remove all catalogs from the task and assert that the project no longer includes the task + task.catalogs.clear() + assert task not in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + # Assert that the initial project tasks are unchanged + assert set(project.tasks.values_list('id', flat=True)) == set(initial_project_tasks) + + +def test_project_tasks_sync_when_adding_or_removing_a_site_to_or_from_a_task(db, settings): + assert settings.PROJECT_TASKS_SYNC + + # Setup: Get an existing project, its associated site, and create a task + project = Project.objects.get(id=project_id) + site = project.site + other_projects = Project.objects.exclude(site=site) # All other projects + task = Task.objects.get(id=task_id) # This task does not have sites in the fixture + task.sites.clear() # Ensure the task starts without any sites + project.tasks.remove(task) + initial_project_tasks = project.tasks.values_list('id', flat=True) + + # Save initial state of tasks for other projects + initial_other_project_tasks = { + i.id: list(i.tasks.values_list('id', flat=True)) + for i in other_projects + } + + # Ensure the project does not have the task initially + assert task not in project.tasks.all() + + ## Tests for .add and .remove + # Add the site to the task and assert that the project now includes the task + task.sites.add(site) + assert task in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + # Remove the site from the task and assert that the project no longer includes the task + task.sites.remove(site) + assert task not in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + ## Tests for .set and .clear + # Add the site to the task and assert that the project now includes the task + task.sites.set([site]) + assert task in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + # Clear all sites from the task and assert that the project no longer includes the task + task.sites.clear() + assert task not in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + # Assert that the initial project tasks are unchanged + assert set(project.tasks.values_list('id', flat=True)) == set(initial_project_tasks) + + +def test_project_tasks_sync_when_adding_or_removing_a_group_to_or_from_a_task(db, settings): + assert settings.PROJECT_TASKS_SYNC + + # Setup: Get an existing project, its associated group, and create a task + project = Project.objects.get(id=project_id) + user = project.owners.first() # Get the first user associated with the project + group = Group.objects.filter(name=group_name).first() # Get a test group + user.groups.add(group) + other_projects = Project.objects.exclude(memberships__user=user) # All other projects + task = Task.objects.get(id=task_id) # This task does not have groups in the fixture + task.groups.clear() # Ensure the task starts without any groups + initial_project_tasks = project.tasks.values_list('id', flat=True) + + # Save initial state of tasks for other projects + initial_other_project_tasks = { + i.id: list(i.tasks.values_list('id', flat=True)) + for i in other_projects + } + + # Ensure the project does not have the task initially + assert task not in project.tasks.all() + + ## Tests for .add and .remove + # Add the group to the task and assert that the project now includes the task + task.groups.add(group) + assert task in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + # Remove the group from the task and assert that the project no longer includes the task + task.groups.remove(group) + assert task not in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + ## Tests for .set and .clear + # Add the group to the task and assert that the project now includes the task + task.groups.set([group]) + assert task in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + # Clear all groups from the task and assert that the project no longer includes the task + task.groups.clear() + assert task not in project.tasks.all() + assert_other_projects_unchanged(other_projects, initial_other_project_tasks) + + # Assert that the initial project tasks are unchanged + assert set(project.tasks.values_list('id', flat=True)) == set(initial_project_tasks) diff --git a/rdmo/projects/tests/test_handlers_views.py b/rdmo/projects/tests/test_handlers_views.py new file mode 100644 index 0000000000..d88c6ca20c --- /dev/null +++ b/rdmo/projects/tests/test_handlers_views.py @@ -0,0 +1,116 @@ + +from django.contrib.auth.models import Group + +from rdmo.projects.models import Project +from rdmo.views.models import View + +project_id = 10 +view_id = 3 +group_name = 'view_test' + +def test_project_views_sync_when_adding_or_removing_a_catalog_to_or_from_a_view(db, settings): + assert settings.PROJECT_VIEWS_SYNC + + # Setup: Create a catalog, a view, and a project using the catalog + project = Project.objects.get(id=project_id) + catalog = project.catalog + view = View.objects.get(id=view_id) # this view does not have catalogs in fixture + view.catalogs.clear() + initial_project_views = project.views.values_list('id', flat=True) + + # # Initially, the project should not have the view + assert view not in project.views.all() + + ## Tests for .add and .remove + # Add the catalog to the view and assert that the project now includes the view + view.catalogs.add(catalog) + assert view in project.views.all() + + # Remove the catalog from the view and assert that the project should no longer include the view + view.catalogs.remove(catalog) + assert view not in project.views.all() + + ## Tests for .set and .clear + # Add the catalog to the view and assert that the project now includes the view + view.catalogs.set([catalog]) + assert view in project.views.all() + + # Remove the catalog from the view and assert that the project should no longer include the view + view.catalogs.clear() + assert view not in project.views.all() + + # assert that the initial project views are unchanged + assert set(project.views.values_list('id', flat=True)) == set(initial_project_views) + + +def test_project_views_sync_when_adding_or_removing_a_site_to_or_from_a_view(db, settings): + assert settings.PROJECT_VIEWS_SYNC + + # Setup: Get an existing project and its associated site and create a view + project = Project.objects.get(id=project_id) + site = project.site + view = View.objects.get(id=view_id) # This view does not have sites in the fixture + view.sites.clear() # Ensure the view starts without any sites + initial_project_views = project.views.values_list('id', flat=True) + + # Ensure initial state: The project should not have the view + assert view not in project.views.all() + + ## Tests for .add and .remove + # Add the site to the view and assert that the project now includes the view + view.sites.add(site) + assert view in project.views.all() + + # Remove the site from the view and assert that the project should no longer include the view + view.sites.remove(site) + assert view not in project.views.all() + + ## Tests for .set and .clear + # Add the site to the view and assert that the project now includes the view + view.sites.set([site]) + assert view in project.views.all() + + # Clear all sites from the view and assert that the project should no longer include the view + view.sites.clear() + assert view not in project.views.all() + + # Assert that the initial project views are unchanged + assert set(project.views.values_list('id', flat=True)) == set(initial_project_views) + + +def test_project_views_sync_when_adding_or_removing_a_group_to_or_from_a_view(db, settings): + assert settings.PROJECT_VIEWS_SYNC + + # Setup: Get an existing project, its associated group, and create a view + project = Project.objects.get(id=project_id) + # breakpoint() + user = project.owners.first() # Get the first user associated with the project + group = Group.objects.filter(name=group_name).first() # Get the first group the user belongs to + user.groups.add(group) + view = View.objects.get(id=view_id) # This view does not have groups in the fixture + view.groups.clear() # Ensure the view starts without any groups + initial_project_views = project.views.values_list('id', flat=True) + + # Ensure initial state: The project should not have the view + assert view not in project.views.all() + + ## Tests for .add and .remove + # Add the group to the view and assert that the project now includes the view + view.groups.add(group) + assert view in project.views.all() + + # Remove the group from the view and assert that the project should no longer include the view + view.groups.remove(group) + assert view not in project.views.all() + + ## Tests for .set and .clear + # Add the group to the view and assert that the project now includes the view + view.groups.set([group]) + assert view in project.views.all() + + # Clear all groups from the view and assert that the project should no longer include the view + view.groups.clear() + assert view not in project.views.all() + + # Assert that the initial project views are unchanged + assert set(project.views.values_list('id', flat=True)) == set(initial_project_views) diff --git a/testing/config/settings/base.py b/testing/config/settings/base.py index 3517e314a4..4133ff8ec4 100644 --- a/testing/config/settings/base.py +++ b/testing/config/settings/base.py @@ -69,7 +69,8 @@ PROJECT_SEND_INVITE = True -PROJECT_REMOVE_VIEWS = True +PROJECT_VIEWS_SYNC = True +PROJECT_TASKS_SYNC = True PROJECT_SNAPSHOT_EXPORTS = [ ('xml', _('RDMO XML'), 'rdmo.projects.exports.RDMOXMLExport'), From 009d592936866f3cbfd5e82e30f59b68460f8274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20Schr=C3=B6der?= Date: Fri, 22 Nov 2024 20:13:21 +0100 Subject: [PATCH 2/7] fix: remove views if catalog is changed # Conflicts: # rdmo/projects/handlers.py --- rdmo/projects/handlers.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 rdmo/projects/handlers.py diff --git a/rdmo/projects/handlers.py b/rdmo/projects/handlers.py new file mode 100644 index 0000000000..7014bedfcc --- /dev/null +++ b/rdmo/projects/handlers.py @@ -0,0 +1,25 @@ +from django.db.models.signals import post_save +from django.dispatch import receiver + +from rdmo.projects.models import Project +from rdmo.views.models import View + + +@receiver(post_save, sender=Project) +def update_views_on_catalog_change(sender, instance, **kwargs): + # remove views that are no longer available + view_candidates = instance.views.exclude(catalogs__in=[instance.catalog]) \ + .exclude(catalogs=None) + + for view in view_candidates: + instance.views.remove(view) + + # add views that are now available + view_candidates = View.objects.exclude(id__in=[v.id for v in instance.views.all()]) \ + .filter_current_site() \ + .filter_catalog(instance.catalog) + # .filter_group(self.request.user) \ + # .filter_availability(self.request.user).exists() + + for view in view_candidates: + instance.views.add(view) From edb842e8706169918e7ec2701d45befd6d49b6ab Mon Sep 17 00:00:00 2001 From: David Wallace Date: Wed, 22 Jan 2025 10:45:28 +0100 Subject: [PATCH 3/7] feat(project, handlers): refactor and add handler for project save, change in catalog Signed-off-by: David Wallace --- rdmo/core/managers.py | 8 +++- rdmo/projects/apps.py | 4 +- rdmo/projects/forms.py | 42 +++++++++++++++++++ rdmo/projects/handlers/generic_handlers.py | 25 ++++++----- ...{project_tasks.py => m2m_changed_tasks.py} | 9 ++-- ...{project_views.py => m2m_changed_views.py} | 9 ++-- rdmo/projects/handlers/project_save_tasks.py | 28 +++++++++++++ rdmo/projects/handlers/project_save_views.py | 28 +++++++++++++ rdmo/projects/tests/helpers.py | 31 ++++++++++++++ ...rs_tasks.py => test_handlers_m2m_tasks.py} | 0 ...rs_views.py => test_handlers_m2m_views.py} | 0 .../tests/test_handlers_project_save.py | 30 +++++++++++++ rdmo/tasks/managers.py | 13 ++++++ rdmo/views/managers.py | 12 ++++++ 14 files changed, 210 insertions(+), 29 deletions(-) rename rdmo/projects/handlers/{project_tasks.py => m2m_changed_tasks.py} (74%) rename rdmo/projects/handlers/{project_views.py => m2m_changed_views.py} (74%) create mode 100644 rdmo/projects/handlers/project_save_tasks.py create mode 100644 rdmo/projects/handlers/project_save_views.py rename rdmo/projects/tests/{test_handlers_tasks.py => test_handlers_m2m_tasks.py} (100%) rename rdmo/projects/tests/{test_handlers_views.py => test_handlers_m2m_views.py} (100%) create mode 100644 rdmo/projects/tests/test_handlers_project_save.py diff --git a/rdmo/core/managers.py b/rdmo/core/managers.py index e1f68ebf70..2243bf3277 100644 --- a/rdmo/core/managers.py +++ b/rdmo/core/managers.py @@ -12,8 +12,12 @@ def filter_current_site(self): class GroupsQuerySetMixin: - def filter_group(self, user): - groups = user.groups.all() + def filter_group(self, users): + + if not isinstance(users, (list, tuple, models.QuerySet)): + users = [users] + + groups = {group for user in users for group in user.groups.all()} return self.filter(models.Q(groups=None) | models.Q(groups__in=groups)) diff --git a/rdmo/projects/apps.py b/rdmo/projects/apps.py index d66f40735d..07dfc6a64d 100644 --- a/rdmo/projects/apps.py +++ b/rdmo/projects/apps.py @@ -11,6 +11,6 @@ def ready(self): from . import rules # noqa: F401 if settings.PROJECT_VIEWS_SYNC: - from .handlers import project_views # noqa: F401 + from .handlers import m2m_changed_views, project_save_views # noqa: F401 if settings.PROJECT_TASKS_SYNC: - from .handlers import project_tasks # noqa: F401 + from .handlers import m2m_changed_tasks, project_save_tasks # noqa: F401 diff --git a/rdmo/projects/forms.py b/rdmo/projects/forms.py index 3d2d1a220a..dd0228240b 100644 --- a/rdmo/projects/forms.py +++ b/rdmo/projects/forms.py @@ -160,6 +160,16 @@ class Meta: 'catalog': forms.RadioSelect() } + def save(self, commit=True, *args, **kwargs): + if 'cancel' in self.data: + return self.instance + + # if the catalog is the same, do nothing + if self.instance.catalog.id == self.cleaned_data.get('catalog'): + return self.instance + + return super().save(commit=commit) + class ProjectUpdateTasksForm(forms.ModelForm): @@ -180,6 +190,22 @@ class Meta: 'tasks': forms.CheckboxSelectMultiple() } + def save(self, commit=True, *args, **kwargs): + if 'cancel' in self.data: + return self.instance + + # If the tasks are the same, do nothing + current_tasks = set(self.instance.tasks.values_list('id', flat=True)) + new_tasks = set(self.cleaned_data.get('tasks').values_list('id', flat=True)) if self.cleaned_data.get( + 'tasks') else set() + + if current_tasks == new_tasks: + return self.instance + + # Save the updated tasks + self.instance.tasks.set(self.cleaned_data.get('tasks')) + return super().save(commit=commit) + class ProjectUpdateViewsForm(forms.ModelForm): @@ -200,6 +226,22 @@ class Meta: 'views': forms.CheckboxSelectMultiple() } + def save(self, commit=True, *args, **kwargs): + if 'cancel' in self.data: + return self.instance + + # If the views are the same, do nothing + current_views = set(self.instance.views.values_list('id', flat=True)) + new_views = ( set(self.cleaned_data.get('views').values_list('id', flat=True)) + if self.cleaned_data.get('views') else set() + ) + + if current_views == new_views: + return self.instance + + # Save the updated views + self.instance.views.set(self.cleaned_data.get('views')) + return super().save(commit=commit) class ProjectUpdateParentForm(forms.ModelForm): diff --git a/rdmo/projects/handlers/generic_handlers.py b/rdmo/projects/handlers/generic_handlers.py index 1c061dcece..6aae8561fe 100644 --- a/rdmo/projects/handlers/generic_handlers.py +++ b/rdmo/projects/handlers/generic_handlers.py @@ -1,23 +1,24 @@ -from django.contrib.auth.models import User +from django.contrib.auth.models import Group, User +from django.contrib.sites.models import Site from rdmo.projects.models import Membership, Project +from ...questions.models import Catalog from .utils import add_instance_to_projects, remove_instance_from_projects -def m2m_catalogs_changed_projects_sync_signal_handler(action, related_model, pk_set, instance, project_field): +def m2m_catalogs_changed_projects_sync_signal_handler(action, pk_set, instance, project_field): """ Update project relationships for m2m_changed signals. Args: action (str): The m2m_changed action (post_add, post_remove, post_clear). - related_model (Model): The related model (e.g., Catalog). pk_set (set): The set of primary keys for the related model instances. instance (Model): The instance being updated (e.g., View or Task). project_field (str): The field on Project to update (e.g., 'views', 'tasks'). """ if action == 'post_remove' and pk_set: - related_instances = related_model.objects.filter(pk__in=pk_set) + related_instances = Catalog.objects.filter(pk__in=pk_set) projects_to_change = Project.objects.filter_catalogs(catalogs=related_instances).filter( **{project_field: instance} ) @@ -28,26 +29,25 @@ def m2m_catalogs_changed_projects_sync_signal_handler(action, related_model, pk_ remove_instance_from_projects(projects_to_change, project_field, instance) elif action == 'post_add' and pk_set: - related_instances = related_model.objects.filter(pk__in=pk_set) + related_instances = Catalog.objects.filter(pk__in=pk_set) projects_to_change = Project.objects.filter_catalogs(catalogs=related_instances).exclude( **{project_field: instance} ) add_instance_to_projects(projects_to_change, project_field, instance) -def m2m_sites_changed_projects_sync_signal_handler(action, model, pk_set, instance, project_field): +def m2m_sites_changed_projects_sync_signal_handler(action, pk_set, instance, project_field): """ Synchronize Project relationships for m2m_changed signals triggered by site updates. Args: action (str): The m2m_changed action (post_add, post_remove, post_clear). - model (Model): The related model (e.g., Site). pk_set (set): The set of primary keys for the related model instances. instance (Model): The instance being updated (e.g., View or Task). project_field (str): The field on Project to update (e.g., 'views', 'tasks'). """ if action == 'post_remove' and pk_set: - related_sites = model.objects.filter(pk__in=pk_set) + related_sites = Site.objects.filter(pk__in=pk_set) catalogs = instance.catalogs.all() projects_to_change = Project.objects.filter_catalogs(catalogs=catalogs).filter( @@ -61,7 +61,7 @@ def m2m_sites_changed_projects_sync_signal_handler(action, model, pk_set, instan remove_instance_from_projects(projects_to_change, project_field, instance) elif action == 'post_add' and pk_set: - related_sites = model.objects.filter(pk__in=pk_set) + related_sites = Site.objects.filter(pk__in=pk_set) catalogs = instance.catalogs.all() projects_to_change = Project.objects.filter_catalogs(catalogs=catalogs).filter( @@ -70,19 +70,18 @@ def m2m_sites_changed_projects_sync_signal_handler(action, model, pk_set, instan add_instance_to_projects(projects_to_change, project_field, instance) -def m2m_groups_changed_projects_sync_signal_handler(action, model, pk_set, instance, project_field): +def m2m_groups_changed_projects_sync_signal_handler(action, pk_set, instance, project_field): """ Synchronize Project relationships for m2m_changed signals triggered by group updates. Args: action (str): The m2m_changed action (post_add, post_remove, post_clear). - model (Model): The related model (e.g., Group). pk_set (set): The set of primary keys for the related model instances. instance (Model): The instance being updated (e.g., View or Task). project_field (str): The field on Project to update (e.g., 'views', 'tasks'). """ if action == 'post_remove' and pk_set: - related_groups = model.objects.filter(pk__in=pk_set) + related_groups = Group.objects.filter(pk__in=pk_set) users = User.objects.filter(groups__in=related_groups) memberships = Membership.objects.filter(role='owner', user__in=users).values_list('id', flat=True) catalogs = instance.catalogs.all() @@ -99,7 +98,7 @@ def m2m_groups_changed_projects_sync_signal_handler(action, model, pk_set, insta remove_instance_from_projects(projects_to_change, project_field, instance) elif action == 'post_add' and pk_set: - related_groups = model.objects.filter(pk__in=pk_set) + related_groups = Group.objects.filter(pk__in=pk_set) users = User.objects.filter(groups__in=related_groups) memberships = Membership.objects.filter(role='owner', user__in=users).values_list('id', flat=True) catalogs = instance.catalogs.all() diff --git a/rdmo/projects/handlers/project_tasks.py b/rdmo/projects/handlers/m2m_changed_tasks.py similarity index 74% rename from rdmo/projects/handlers/project_tasks.py rename to rdmo/projects/handlers/m2m_changed_tasks.py index 04dc19c05f..40190cbd3a 100644 --- a/rdmo/projects/handlers/project_tasks.py +++ b/rdmo/projects/handlers/m2m_changed_tasks.py @@ -12,10 +12,9 @@ @receiver(m2m_changed, sender=Task.catalogs.through) -def m2m_changed_task_catalog_signal(sender, instance, action, model, pk_set, **kwargs): +def m2m_changed_task_catalog_signal(sender, instance, action, pk_set, **kwargs): m2m_catalogs_changed_projects_sync_signal_handler( action=action, - related_model=model, pk_set=pk_set, instance=instance, project_field='tasks', @@ -23,10 +22,9 @@ def m2m_changed_task_catalog_signal(sender, instance, action, model, pk_set, **k @receiver(m2m_changed, sender=Task.sites.through) -def m2m_changed_task_sites_signal(sender, instance, action, model, pk_set, **kwargs): +def m2m_changed_task_sites_signal(sender, instance, action, pk_set, **kwargs): m2m_sites_changed_projects_sync_signal_handler( action=action, - model=model, pk_set=pk_set, instance=instance, project_field='tasks' @@ -34,10 +32,9 @@ def m2m_changed_task_sites_signal(sender, instance, action, model, pk_set, **kwa @receiver(m2m_changed, sender=Task.groups.through) -def m2m_changed_task_groups_signal(sender, instance, action, model, pk_set, **kwargs): +def m2m_changed_task_groups_signal(sender, instance, action, pk_set, **kwargs): m2m_groups_changed_projects_sync_signal_handler( action=action, - model=model, pk_set=pk_set, instance=instance, project_field='tasks' diff --git a/rdmo/projects/handlers/project_views.py b/rdmo/projects/handlers/m2m_changed_views.py similarity index 74% rename from rdmo/projects/handlers/project_views.py rename to rdmo/projects/handlers/m2m_changed_views.py index 458109c735..c946ab4c5a 100644 --- a/rdmo/projects/handlers/project_views.py +++ b/rdmo/projects/handlers/m2m_changed_views.py @@ -12,10 +12,9 @@ @receiver(m2m_changed, sender=View.catalogs.through) -def m2m_changed_view_catalog_signal(sender, instance, action, model, pk_set, **kwargs): +def m2m_changed_view_catalog_signal(sender, instance, action, pk_set, **kwargs): m2m_catalogs_changed_projects_sync_signal_handler( action=action, - related_model=model, pk_set=pk_set, instance=instance, project_field='views', @@ -24,10 +23,9 @@ def m2m_changed_view_catalog_signal(sender, instance, action, model, pk_set, **k @receiver(m2m_changed, sender=View.sites.through) -def m2m_changed_view_sites_signal(sender, instance, action, model, pk_set, **kwargs): +def m2m_changed_view_sites_signal(sender, instance, action, pk_set, **kwargs): m2m_sites_changed_projects_sync_signal_handler( action=action, - model=model, pk_set=pk_set, instance=instance, project_field='views' @@ -35,10 +33,9 @@ def m2m_changed_view_sites_signal(sender, instance, action, model, pk_set, **kwa @receiver(m2m_changed, sender=View.groups.through) -def m2m_changed_view_groups_signal(sender, instance, action, model, pk_set, **kwargs): +def m2m_changed_view_groups_signal(sender, instance, action, pk_set, **kwargs): m2m_groups_changed_projects_sync_signal_handler( action=action, - model=model, pk_set=pk_set, instance=instance, project_field='views' diff --git a/rdmo/projects/handlers/project_save_tasks.py b/rdmo/projects/handlers/project_save_tasks.py new file mode 100644 index 0000000000..99acb76a6a --- /dev/null +++ b/rdmo/projects/handlers/project_save_tasks.py @@ -0,0 +1,28 @@ +from django.db.models.signals import post_save, pre_save +from django.dispatch import receiver + +from rdmo.projects.models import Project +from rdmo.tasks.models import Task + +DEFERRED_SYNC_TASKS_KEY = '_deferred_sync_tasks' + +@receiver(pre_save, sender=Project) +def pre_save_project_sync_tasks_from_catalog(sender, instance, raw, update_fields, **kwargs): + if raw or (update_fields and 'catalog' not in update_fields): + return + + # Fetch the original catalog from the database + original_instance = sender.objects.get(id=instance.id) + if original_instance.catalog == instance.catalog: + # Do nothing if the catalog has not changed + return + + # Defer synchronization of views + setattr(instance, DEFERRED_SYNC_TASKS_KEY, True) + +@receiver(post_save, sender=Project) +def post_save_project_sync_tasks_from_catalog(sender, instance, created, raw, **kwargs): + if getattr(instance, DEFERRED_SYNC_TASKS_KEY, None) or (created and not raw): + # For existing projects with catalog changes, use deferred views + instance.views.set(Task.objects.filter_available_tasks_for_project(instance)) + delattr(instance, DEFERRED_SYNC_TASKS_KEY) diff --git a/rdmo/projects/handlers/project_save_views.py b/rdmo/projects/handlers/project_save_views.py new file mode 100644 index 0000000000..38c3040fe6 --- /dev/null +++ b/rdmo/projects/handlers/project_save_views.py @@ -0,0 +1,28 @@ +from django.db.models.signals import post_save, pre_save +from django.dispatch import receiver + +from rdmo.projects.models import Project +from rdmo.views.models import View + +DEFERRED_SYNC_VIEWS_KEY = '_deferred_sync_views' + +@receiver(pre_save, sender=Project) +def pre_save_project_sync_views_from_catalog(sender, instance, raw, update_fields, **kwargs): + if raw or (update_fields and 'catalog' not in update_fields): + return + + # Fetch the original catalog from the database + original_instance = sender.objects.get(id=instance.id) + if original_instance.catalog == instance.catalog: + # Do nothing if the catalog has not changed + return + + # Defer synchronization of views + setattr(instance, DEFERRED_SYNC_VIEWS_KEY, True) + +@receiver(post_save, sender=Project) +def post_save_project_sync_views_from_catalog(sender, instance, created, raw, update_fields, **kwargs): + if getattr(instance, DEFERRED_SYNC_VIEWS_KEY, None) or (created and not raw): + # For existing projects with catalog changes, use deferred views + instance.views.set(View.objects.filter_available_views_for_project(instance)) + delattr(instance, DEFERRED_SYNC_VIEWS_KEY) diff --git a/rdmo/projects/tests/helpers.py b/rdmo/projects/tests/helpers.py index db04dd17da..999d8a9a92 100644 --- a/rdmo/projects/tests/helpers.py +++ b/rdmo/projects/tests/helpers.py @@ -1,5 +1,36 @@ +from collections import defaultdict + +from rdmo.questions.models import Catalog +from rdmo.views.models import View def assert_other_projects_unchanged(other_projects, initial_tasks_state): for other_project in other_projects: assert set(other_project.tasks.values_list('id', flat=True)) == set(initial_tasks_state[other_project.id]) + + + +def get_catalog_view_mapping(): + """ + Generate a mapping of catalogs to their associated views. + Includes all catalogs, even those with no views, and adds `sites` and `groups` for each view. + """ + # Initialize an empty dictionary for the catalog-to-views mapping + catalog_views_mapping = defaultdict(list) + + # Populate the mapping for all catalogs + for catalog in Catalog.objects.all(): + catalog_views_mapping[catalog.id] = [] + + # Iterate through all views and enrich the mapping + for view in View.objects.prefetch_related('sites', 'groups'): + if view.catalogs.exists(): # Only include views with valid catalogs + for catalog in view.catalogs.all(): + catalog_views_mapping[catalog.id].append({ + 'id': view.id, + 'sites': list(view.sites.values_list('id', flat=True)), + 'groups': list(view.groups.values_list('id', flat=True)) + }) + + # Convert defaultdict to a regular dictionary + return dict(catalog_views_mapping) diff --git a/rdmo/projects/tests/test_handlers_tasks.py b/rdmo/projects/tests/test_handlers_m2m_tasks.py similarity index 100% rename from rdmo/projects/tests/test_handlers_tasks.py rename to rdmo/projects/tests/test_handlers_m2m_tasks.py diff --git a/rdmo/projects/tests/test_handlers_views.py b/rdmo/projects/tests/test_handlers_m2m_views.py similarity index 100% rename from rdmo/projects/tests/test_handlers_views.py rename to rdmo/projects/tests/test_handlers_m2m_views.py diff --git a/rdmo/projects/tests/test_handlers_project_save.py b/rdmo/projects/tests/test_handlers_project_save.py new file mode 100644 index 0000000000..1a0fb5661c --- /dev/null +++ b/rdmo/projects/tests/test_handlers_project_save.py @@ -0,0 +1,30 @@ +from rdmo.projects.models import Project +from rdmo.questions.models import Catalog +from rdmo.views.models import View + +from .helpers import get_catalog_view_mapping + +project_id = 10 + + +def test_project_views_sync_when_changing_the_catalog_on_a_project(db, settings): + assert settings.PROJECT_VIEWS_SYNC + + # Setup: Create a catalog, a view, and a project using the catalog + project = Project.objects.get(id=project_id) + initial_project_views = set(project.views.values_list('id', flat=True)) + assert initial_project_views == {1,2,3} # from the fixture + + catalog_view_mapping = get_catalog_view_mapping() + for catalog_id, view_ids in catalog_view_mapping.items(): + if project.catalog_id == catalog_id: + continue # catalog will not change + project.catalog = Catalog.objects.get(id=catalog_id) + project.save() + + # TODO this filter_available_views_for_project method needs to tested explicitly + available_views = set(View.objects + .filter_available_views_for_project(project) + .values_list('id', flat=True) + ) + assert set(project.views.values_list('id', flat=True)) == available_views diff --git a/rdmo/tasks/managers.py b/rdmo/tasks/managers.py index 0251eab9f0..d30e05218b 100644 --- a/rdmo/tasks/managers.py +++ b/rdmo/tasks/managers.py @@ -15,6 +15,16 @@ class TaskQuestionSet(CurrentSiteQuerySetMixin, GroupsQuerySetMixin, Availabilit def filter_catalog(self, catalog): return self.filter(models.Q(catalogs=None) | models.Q(catalogs=catalog)) + def filter_available_views_for_project(self, project): + return (self + .filter(sites=project.site) + .filter(catalogs=project.catalog) + .filter_group(project.owners.all()) + .filter(available=True) + .exclude(catalogs=None) + ) + + class TaskManager(CurrentSiteManagerMixin, GroupsManagerMixin, AvailabilityManagerMixin, models.Manager): @@ -23,3 +33,6 @@ def get_queryset(self): def filter_catalog(self, catalog): return self.get_queryset().filter_catalog(catalog) + + def filter_available_tasks_for_project(self, project): + return self.get_queryset().filter_available_views_for_project(project) diff --git a/rdmo/views/managers.py b/rdmo/views/managers.py index 4c584e4839..b279be2217 100644 --- a/rdmo/views/managers.py +++ b/rdmo/views/managers.py @@ -15,6 +15,15 @@ class ViewQuestionSet(CurrentSiteQuerySetMixin, GroupsQuerySetMixin, Availabilit def filter_catalog(self, catalog): return self.filter(models.Q(catalogs=None) | models.Q(catalogs=catalog)) + def filter_available_views_for_project(self, project): + return (self + .filter(sites=project.site) + .filter(catalogs=project.catalog) + .filter_group(project.owners.all()) + .filter(available=True) + .exclude(catalogs=None) + ) + class ViewManager(CurrentSiteManagerMixin, GroupsManagerMixin, AvailabilityManagerMixin, models.Manager): @@ -23,3 +32,6 @@ def get_queryset(self): def filter_catalog(self, catalog): return self.get_queryset().filter_catalog(catalog) + + def filter_available_views_for_project(self, project): + return self.get_queryset().filter_available_views_for_project(project) From ca409928d2425f191e66585d2b5f51b0529eda61 Mon Sep 17 00:00:00 2001 From: David Wallace Date: Wed, 22 Jan 2025 12:15:23 +0100 Subject: [PATCH 4/7] fix(project,handlers): ignore signals at project create Signed-off-by: David Wallace --- rdmo/projects/handlers/project_save_tasks.py | 7 ++++++- rdmo/projects/handlers/project_save_views.py | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/rdmo/projects/handlers/project_save_tasks.py b/rdmo/projects/handlers/project_save_tasks.py index 99acb76a6a..cfd48eb4c0 100644 --- a/rdmo/projects/handlers/project_save_tasks.py +++ b/rdmo/projects/handlers/project_save_tasks.py @@ -8,7 +8,10 @@ @receiver(pre_save, sender=Project) def pre_save_project_sync_tasks_from_catalog(sender, instance, raw, update_fields, **kwargs): - if raw or (update_fields and 'catalog' not in update_fields): + if (raw or + instance.id is None or + (update_fields and 'catalog' not in update_fields) + ): return # Fetch the original catalog from the database @@ -22,6 +25,8 @@ def pre_save_project_sync_tasks_from_catalog(sender, instance, raw, update_field @receiver(post_save, sender=Project) def post_save_project_sync_tasks_from_catalog(sender, instance, created, raw, **kwargs): + if not hasattr(instance, DEFERRED_SYNC_TASKS_KEY): + return if getattr(instance, DEFERRED_SYNC_TASKS_KEY, None) or (created and not raw): # For existing projects with catalog changes, use deferred views instance.views.set(Task.objects.filter_available_tasks_for_project(instance)) diff --git a/rdmo/projects/handlers/project_save_views.py b/rdmo/projects/handlers/project_save_views.py index 38c3040fe6..2888740f72 100644 --- a/rdmo/projects/handlers/project_save_views.py +++ b/rdmo/projects/handlers/project_save_views.py @@ -8,7 +8,10 @@ @receiver(pre_save, sender=Project) def pre_save_project_sync_views_from_catalog(sender, instance, raw, update_fields, **kwargs): - if raw or (update_fields and 'catalog' not in update_fields): + if (raw or + instance.id is None or + (update_fields and 'catalog' not in update_fields) + ): return # Fetch the original catalog from the database @@ -22,6 +25,8 @@ def pre_save_project_sync_views_from_catalog(sender, instance, raw, update_field @receiver(post_save, sender=Project) def post_save_project_sync_views_from_catalog(sender, instance, created, raw, update_fields, **kwargs): + if not hasattr(instance, DEFERRED_SYNC_VIEWS_KEY): + return if getattr(instance, DEFERRED_SYNC_VIEWS_KEY, None) or (created and not raw): # For existing projects with catalog changes, use deferred views instance.views.set(View.objects.filter_available_views_for_project(instance)) From d93f15f64ef15898173477a71b39df57c08341d4 Mon Sep 17 00:00:00 2001 From: David Wallace Date: Thu, 23 Jan 2025 09:54:15 +0100 Subject: [PATCH 5/7] refactor(project,handlers): simplify pre and post save Signed-off-by: David Wallace --- rdmo/projects/handlers/project_save_tasks.py | 26 +++++++++----------- rdmo/projects/handlers/project_save_views.py | 23 ++++++++--------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/rdmo/projects/handlers/project_save_tasks.py b/rdmo/projects/handlers/project_save_tasks.py index cfd48eb4c0..40938a1556 100644 --- a/rdmo/projects/handlers/project_save_tasks.py +++ b/rdmo/projects/handlers/project_save_tasks.py @@ -8,26 +8,24 @@ @receiver(pre_save, sender=Project) def pre_save_project_sync_tasks_from_catalog(sender, instance, raw, update_fields, **kwargs): - if (raw or - instance.id is None or - (update_fields and 'catalog' not in update_fields) - ): + if raw or (update_fields and 'catalog' not in update_fields): return - # Fetch the original catalog from the database - original_instance = sender.objects.get(id=instance.id) - if original_instance.catalog == instance.catalog: - # Do nothing if the catalog has not changed - return + if instance.id is not None: + # Fetch the original catalog from the database + if sender.objects.get(id=instance.id).catalog == instance.catalog: + # Do nothing if the catalog has not changed + return # Defer synchronization of views setattr(instance, DEFERRED_SYNC_TASKS_KEY, True) + @receiver(post_save, sender=Project) -def post_save_project_sync_tasks_from_catalog(sender, instance, created, raw, **kwargs): - if not hasattr(instance, DEFERRED_SYNC_TASKS_KEY): +def post_save_project_sync_tasks_from_catalog(sender, instance, created, raw, update_fields, **kwargs): + if raw or (update_fields and 'catalog' not in update_fields): return - if getattr(instance, DEFERRED_SYNC_TASKS_KEY, None) or (created and not raw): - # For existing projects with catalog changes, use deferred views - instance.views.set(Task.objects.filter_available_tasks_for_project(instance)) + + if hasattr(instance, DEFERRED_SYNC_TASKS_KEY): + instance.tasks.set(Task.objects.filter_available_tasks_for_project(instance).values_list('id', flat=True)) delattr(instance, DEFERRED_SYNC_TASKS_KEY) diff --git a/rdmo/projects/handlers/project_save_views.py b/rdmo/projects/handlers/project_save_views.py index 2888740f72..4e96c9448a 100644 --- a/rdmo/projects/handlers/project_save_views.py +++ b/rdmo/projects/handlers/project_save_views.py @@ -8,26 +8,23 @@ @receiver(pre_save, sender=Project) def pre_save_project_sync_views_from_catalog(sender, instance, raw, update_fields, **kwargs): - if (raw or - instance.id is None or - (update_fields and 'catalog' not in update_fields) - ): + if raw or (update_fields and 'catalog' not in update_fields): return - # Fetch the original catalog from the database - original_instance = sender.objects.get(id=instance.id) - if original_instance.catalog == instance.catalog: - # Do nothing if the catalog has not changed - return + if instance.id is not None: + # Fetch the original catalog from the database + if sender.objects.get(id=instance.id).catalog == instance.catalog: + # Do nothing if the catalog has not changed + return # Defer synchronization of views setattr(instance, DEFERRED_SYNC_VIEWS_KEY, True) @receiver(post_save, sender=Project) def post_save_project_sync_views_from_catalog(sender, instance, created, raw, update_fields, **kwargs): - if not hasattr(instance, DEFERRED_SYNC_VIEWS_KEY): + if raw or (update_fields and 'catalog' not in update_fields): return - if getattr(instance, DEFERRED_SYNC_VIEWS_KEY, None) or (created and not raw): - # For existing projects with catalog changes, use deferred views - instance.views.set(View.objects.filter_available_views_for_project(instance)) + + if hasattr(instance, DEFERRED_SYNC_VIEWS_KEY): + instance.views.set(View.objects.filter_available_views_for_project(instance).values_list('id', flat=True)) delattr(instance, DEFERRED_SYNC_VIEWS_KEY) From 4dc9f04e84329391065ddd7f6654bbfd4a9192e9 Mon Sep 17 00:00:00 2001 From: David Wallace Date: Fri, 24 Jan 2025 11:45:52 +0100 Subject: [PATCH 6/7] fix(managers,tasks,views): use project.owners for filter Signed-off-by: David Wallace --- rdmo/tasks/managers.py | 2 +- rdmo/views/managers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rdmo/tasks/managers.py b/rdmo/tasks/managers.py index d30e05218b..a8e3c4f696 100644 --- a/rdmo/tasks/managers.py +++ b/rdmo/tasks/managers.py @@ -19,7 +19,7 @@ def filter_available_views_for_project(self, project): return (self .filter(sites=project.site) .filter(catalogs=project.catalog) - .filter_group(project.owners.all()) + .filter_group(project.owners) .filter(available=True) .exclude(catalogs=None) ) diff --git a/rdmo/views/managers.py b/rdmo/views/managers.py index b279be2217..e656ef9bf0 100644 --- a/rdmo/views/managers.py +++ b/rdmo/views/managers.py @@ -19,7 +19,7 @@ def filter_available_views_for_project(self, project): return (self .filter(sites=project.site) .filter(catalogs=project.catalog) - .filter_group(project.owners.all()) + .filter_group(project.owners) .filter(available=True) .exclude(catalogs=None) ) From e19be6459c513294b8b18bec01357ade674b2236 Mon Sep 17 00:00:00 2001 From: David Wallace Date: Fri, 24 Jan 2025 12:29:52 +0100 Subject: [PATCH 7/7] feat(project, update): prevent views or tasks update when sync is enabled Signed-off-by: David Wallace --- rdmo/core/settings.py | 4 +++- rdmo/projects/serializers/v1/__init__.py | 7 +++++++ .../templates/projects/project_detail_issues.html | 4 ++-- .../templates/projects/project_detail_views.html | 4 ++-- rdmo/projects/tests/test_viewset_project.py | 14 ++++++++++++++ rdmo/projects/views/project.py | 13 +++++++++++-- 6 files changed, 39 insertions(+), 7 deletions(-) diff --git a/rdmo/core/settings.py b/rdmo/core/settings.py index 53293abf63..bd74dd25a2 100644 --- a/rdmo/core/settings.py +++ b/rdmo/core/settings.py @@ -209,7 +209,9 @@ 'PROJECT_IMPORTS_LIST', 'PROJECT_SEND_ISSUE', 'PROJECT_QUESTIONS_AUTOSAVE', - 'NESTED_PROJECTS' + 'NESTED_PROJECTS', + 'PROJECT_VIEWS_SYNC', + 'PROJECT_TASKS_SYNC' ] SETTINGS_API = [ diff --git a/rdmo/projects/serializers/v1/__init__.py b/rdmo/projects/serializers/v1/__init__.py index 8e61748193..3a441b089d 100644 --- a/rdmo/projects/serializers/v1/__init__.py +++ b/rdmo/projects/serializers/v1/__init__.py @@ -4,6 +4,7 @@ from django.utils.translation import gettext_lazy as _ from rest_framework import serializers +from rest_framework.exceptions import ValidationError from rdmo.questions.models import Catalog from rdmo.services.validators import ProviderValidator @@ -93,6 +94,12 @@ class Meta: ProjectParentValidator() ] + def validate_views(self, value): + """Block updates to views if syncing is enabled.""" + if settings.PROJECT_VIEWS_SYNC and value: + raise ValidationError(_('Updating views is not allowed.')) + return value + class ProjectCopySerializer(ProjectSerializer): diff --git a/rdmo/projects/templates/projects/project_detail_issues.html b/rdmo/projects/templates/projects/project_detail_issues.html index 0e51a4257f..9bf77532e2 100644 --- a/rdmo/projects/templates/projects/project_detail_issues.html +++ b/rdmo/projects/templates/projects/project_detail_issues.html @@ -21,7 +21,7 @@

{% trans 'Tasks' %}

{% trans 'Time frame' %} {% trans 'Status' %} - {% if can_change_project %} + {% if can_change_project and not settings.PROJECT_TASKS_SYNC %} @@ -67,7 +67,7 @@

{% trans 'Tasks' %}

{% else %} - {% if can_change_project %} + {% if can_change_project and not settings.PROJECT_TASKS_SYNC %}

diff --git a/rdmo/projects/templates/projects/project_detail_views.html b/rdmo/projects/templates/projects/project_detail_views.html index ae3ce20fa7..09f9915cf5 100644 --- a/rdmo/projects/templates/projects/project_detail_views.html +++ b/rdmo/projects/templates/projects/project_detail_views.html @@ -19,7 +19,7 @@

{% trans 'Views' %}

{% trans 'View' %} {% trans 'Description' %} - {% if can_change_project %} + {% if can_change_project and not settings.PROJECT_VIEWS_SYNC %} @@ -45,7 +45,7 @@

{% trans 'Views' %}

{% else %} - {% if can_change_project %} + {% if can_change_project and not settings.PROJECT_VIEWS_SYNC %}

diff --git a/rdmo/projects/tests/test_viewset_project.py b/rdmo/projects/tests/test_viewset_project.py index f45351743b..0b51f5972b 100644 --- a/rdmo/projects/tests/test_viewset_project.py +++ b/rdmo/projects/tests/test_viewset_project.py @@ -429,6 +429,20 @@ def test_update_parent(db, client, username, password, project_id): assert Project.objects.get(pk=project_id).parent == project.parent +def test_update_project_views_not_allowed(db, client, settings): + assert settings.PROJECT_VIEWS_SYNC + client.login(username='owner', password='owner') + + url = reverse(urlnames['detail'], args=[project_id]) + data = { + 'views': [1] + } + response = client.put(url, data, content_type='application/json') + + assert response.status_code == 400 + assert 'Updating views is not allowed' in ' '.join(response.json()['views']) + + @pytest.mark.parametrize('username,password', users) @pytest.mark.parametrize('project_id', projects) def test_delete(db, client, username, password, project_id): diff --git a/rdmo/projects/views/project.py b/rdmo/projects/views/project.py index 9760fbd3b6..6eade947ac 100644 --- a/rdmo/projects/views/project.py +++ b/rdmo/projects/views/project.py @@ -64,11 +64,20 @@ def get_context_data(self, **kwargs): context['catalogs'] = Catalog.objects.filter_current_site() \ .filter_group(self.request.user) \ .filter_availability(self.request.user) - context['tasks_available'] = Task.objects.filter_current_site() \ + + if settings.PROJECT_TASKS_SYNC: + # tasks should be synced, the user can not change them + context['tasks_available'] = project.tasks.exists() + else: + context['tasks_available'] = Task.objects.filter_current_site() \ .filter_catalog(self.object.catalog) \ .filter_group(self.request.user) \ .filter_availability(self.request.user).exists() - context['views_available'] = View.objects.filter_current_site() \ + if settings.PROJECT_VIEWS_SYNC: + # views should be synced, the user can not change them + context['views_available'] = project.views.exists() + else: + context['views_available'] = View.objects.filter_current_site() \ .filter_catalog(self.object.catalog) \ .filter_group(self.request.user) \ .filter_availability(self.request.user).exists()