Fix #1518, check also unavailable views/tasks for projects sync

rdmo/core/managers.py

@@ -1,7 +1,7 @@
 from django.conf import settings
 from django.db import models
 
-from .constants import PERMISSIONS
+from .utils import can_view_unavailable
 
 
 class CurrentSiteQuerySetMixin:
@@ -23,7 +23,7 @@ def filter_group(self, user):
 class AvailabilityQuerySetMixin:
 
     def filter_availability(self, user):
-        if user.has_perms(PERMISSIONS[self.model._meta.label_lower]):
+        if can_view_unavailable(user, self.model):
             return self
         else:
             return self.filter(available=True)

rdmo/core/utils.py

@@ -18,12 +18,18 @@
 from defusedcsv import csv
 from markdown import markdown
 
-from .constants import HUMAN2BYTES_MAPPER
+from .constants import HUMAN2BYTES_MAPPER, PERMISSIONS
 from .pandoc import get_pandoc_content, get_pandoc_content_disposition
 
 log = logging.getLogger(__name__)
 
 
+def can_view_unavailable(user, model) -> bool:
+    if not user:
+        return False
+    return user.has_perms(PERMISSIONS[model._meta.label_lower])
+
+
 def get_script_alias(request):
     return request.path[:-len(request.path_info)]
 

rdmo/projects/handlers/sync_utils.py

@@ -11,11 +11,11 @@
 logger = logging.getLogger(__name__)
 
 
-def sync_task_or_view_to_projects(instance):
+def sync_task_or_view_to_projects(instance, user=None):
     """Ensure the instance is linked to exactly the correct set of projects."""
     project_m2m_field = get_related_field_name_on_model_for_instance(Project, instance)
 
-    target_projects = Project.objects.filter_projects_for_task_or_view(instance)
+    target_projects = Project.objects.filter_projects_for_task_or_view(instance, user=user)
     current_projects = Project.objects.filter(**{project_m2m_field: instance})
 
     to_remove = current_projects.exclude(pk__in=target_projects)
@@ -51,7 +51,7 @@ def sync_tasks_or_views_on_a_project(project, task_or_view):
     """Ensure the project is linked to exactly the correct instances of a model (View/Task)."""
     project_m2m_field = get_related_field_name_on_model_for_instance(Project, task_or_view)
 
-    desired_instances = filter_tasks_or_views_for_project(task_or_view, project)
+    desired_instances = filter_tasks_or_views_for_project(task_or_view, project, user=None)
     current_instances = getattr(project, project_m2m_field).all()
 
     to_remove = current_instances.exclude(pk__in=desired_instances)

rdmo/projects/management/commands/sync_projects.py

@@ -41,11 +41,11 @@ def handle(self, *args, **options):
             self.show_project_tasks_and_views()
 
     def sync_all_tasks_or_views_to_projects(self, model):
-        queryset = model.objects.filter(available=True)
+        queryset = model.objects.all()
         model_name = model._meta.verbose_name_plural
         qs_count = queryset.count()
 
-        self.stdout.write(self.style.SUCCESS(f'Starting sync for {qs_count} available {model_name}...'))
+        self.stdout.write(self.style.SUCCESS(f'Starting sync for {qs_count} {model_name}...'))
         for instance in queryset:
             self.stdout.write(f'- Syncing: {instance}')
             sync_task_or_view_to_projects(instance)
@@ -62,15 +62,13 @@ def show_project_tasks_and_views(self):
             self.stdout.write(f'Project "{project.title}" [id={project.id}]:')
             self.stdout.write(f'- Catalog: {project.catalog.uri}')
 
-            if task_uris:
-                self.stdout.write("- Tasks:")
-                for task_uri in task_uris:
-                    self.stdout.write(f"  - {task_uri}")
+            self.stdout.write("- Tasks:")
+            for task_uri in task_uris:
+                self.stdout.write(f"  - {task_uri}")
 
-            if view_uris:
-                self.stdout.write("- Views:")
-                for view_uri in view_uris:
-                    self.stdout.write(f"  - {view_uri}")
+            self.stdout.write("- Views:")
+            for view_uri in view_uris:
+                self.stdout.write(f"  - {view_uri}")
 
             self.stdout.write()  # add an empty line for spacing between projects
 

rdmo/projects/managers.py

@@ -9,6 +9,7 @@
 
 from rdmo.accounts.utils import is_site_manager
 from rdmo.core.managers import CurrentSiteManagerMixin
+from rdmo.core.utils import can_view_unavailable
 
 
 class ProjectQuerySet(TreeQuerySet):
@@ -71,17 +72,22 @@ def filter_groups(self, groups):
         # projects that have those memberships
         return self.filter(memberships__in=memberships).distinct()
 
-    def filter_projects_for_task_or_view(self, instance):
+    def filter_projects_for_task_or_view(self, instance, user=None):
         # projects that have an unavailable catalog should be disregarded
         qs = self.filter(catalog__available=True)
 
         # when View/Task is not available it should not show for any project
-        if not instance.available:
+        if user is not None:
+            if not can_view_unavailable(user, instance._meta.model) and not instance.available:
+                return self.none()
+        elif not instance.available:
             return self.none()
 
         # when View/Task has any catalogs it can be filtered for those
         if instance.catalogs.exists():
             qs = qs.filter(catalog__in=instance.catalogs.all())
+        else:
+            return self.none()
 
         # when View/Task has any sites it can be filtered for those
         if instance.sites.exists():
@@ -93,6 +99,8 @@ def filter_projects_for_task_or_view(self, instance):
         # when  has any groups it can be filtered for those
         if instance.groups.exists():
             qs = qs.filter_groups(instance.groups.all())
+        else:
+            return self.none()
 
         return qs
 
@@ -266,8 +274,8 @@ def filter_catalogs(self, catalogs=None, exclude_catalogs=None, exclude_null=Tru
     def filter_groups(self, groups):
         return self.get_queryset().filter_groups(groups)
 
-    def filter_projects_for_task_or_view(self, instance):
-        return self.get_queryset().filter_projects_for_task_or_view(instance)
+    def filter_projects_for_task_or_view(self, instance, user=None):
+        return self.get_queryset().filter_projects_for_task_or_view(instance, user=user)
 
 
 class MembershipManager(CurrentSiteManagerMixin, models.Manager):

rdmo/projects/tests/helpers/project_sync/arrange_project_tasks.py

@@ -1,5 +1,6 @@
 from contextlib import contextmanager
 from unittest.mock import patch
+from uuid import uuid4
 
 from django.contrib.auth.models import Group, User
 from django.contrib.sites.models import Site
@@ -35,6 +36,13 @@ def arrange_projects_catalogs_and_tasks():
             )
             for n in one_two_three
         }
+        # Create groups, users and project memberships
+        shared_group = Group.objects.get(name="view_test")
+        for n in one_two_three:
+            _user = User.objects.create(username=f"Sync U{n}-{uuid4().hex}")
+            _user.groups.set([shared_group])
+            # this sets P[n].groups -> U[n].groups
+            Membership.objects.create(user_id=_user.id, project_id=P[n].id, role='owner')
 
         # Arrange the catalogs
         for catalog in C.values():
@@ -56,8 +64,8 @@ def arrange_projects_catalogs_and_tasks():
             task.uri = T_uri.format(n)
             task.save()
             task.sites.clear()
-            task.groups.clear()
             task.catalogs.set([C[n]])
+            task.groups.set([shared_group])
 
         for n in one_two_three:
             P[n].tasks.set([T[n]])
@@ -80,6 +88,13 @@ def arrange_projects_sites_and_tasks():
             )
             for n in one_two_three
         }
+        # Create groups, users and project memberships
+        shared_group = Group.objects.get(name="view_test")
+        for n in one_two_three:
+            _user = User.objects.create(username=f"Sync U{n}-{uuid4().hex}")
+            _user.groups.set([shared_group])
+            # this sets P[n].groups -> U[n].groups
+            Membership.objects.create(user_id=_user.id, project_id=P[n].id, role='owner')
 
         # Arrange the catalogs
         for catalog in C.values():
@@ -99,9 +114,9 @@ def arrange_projects_sites_and_tasks():
         for n, task in T.items():
             task.available = True
             task.save()
-            task.catalogs.clear()
-            task.groups.clear()
             task.sites.set([S[n]])
+            task.catalogs.set(list(C.values()))
+            task.groups.set([shared_group])
 
         for n in one_two_three:
             P[n].tasks.set([T[n]])
@@ -124,9 +139,13 @@ def arrange_projects_groups_and_tasks():
             for n in one_two_three
         }
         # Create groups, users and project memberships
-        G = {n: Group.objects.create(name=f"Sync G{n}") for n in one_two_three}
+        G = {
+            1: Group.objects.get(name="view_test"),
+            2: Group.objects.get(name="editor"),
+            3: Group.objects.get(name="reviewer"),
+        }
         for n in one_two_three:
-            _user = User.objects.create(username=f"Sync U{n}")
+            _user = User.objects.create(username=f"Sync U{n}-{uuid4().hex}")
             _user.groups.set([G[n]])
             # this sets P[1].groups -> U[n].groups
             Membership.objects.create(user_id=_user.id, project_id=P[n].id, role='owner')
@@ -149,8 +168,8 @@ def arrange_projects_groups_and_tasks():
         for n, task in T.items():
             task.available = True
             task.save()
-            task.catalogs.clear()
             task.sites.clear()
+            task.catalogs.set(list(C.values()))
             task.groups.set([G[n]])  # set groups as last so that will be state
 
         for n in one_two_three:

rdmo/projects/tests/helpers/project_sync/arrange_project_views.py

@@ -1,5 +1,6 @@
 from contextlib import contextmanager
 from unittest.mock import patch
+from uuid import uuid4
 
 from django.contrib.auth.models import Group, User
 from django.contrib.sites.models import Site
@@ -33,6 +34,13 @@ def arrange_projects_catalogs_and_views():
             )
             for n in one_two_three
         }
+        # Create groups, users and project memberships
+        shared_group = Group.objects.get(name="view_test")
+        for n in one_two_three:
+            _user = User.objects.create(username=f"Sync U{n}-{uuid4().hex}")
+            _user.groups.set([shared_group])
+            # this sets P[n].groups -> U[n].groups
+            Membership.objects.create(user_id=_user.id, project_id=P[n].id, role='owner')
 
         # Arrange the catalogs
         for catalog in C.values():
@@ -53,8 +61,8 @@ def arrange_projects_catalogs_and_views():
             view.available = True
             view.save()
             view.sites.clear()
-            view.groups.clear()
             view.catalogs.set([C[n]])
+            view.groups.set([shared_group])
 
         # Ensure each project starts with its matching view only
         for n in one_two_three:
@@ -78,6 +86,13 @@ def arrange_projects_sites_and_views():
             )
             for n in one_two_three
         }
+        # Create groups, users and project memberships
+        shared_group = Group.objects.get(name="view_test")
+        for n in one_two_three:
+            _user = User.objects.create(username=f"Sync U{n}-{uuid4().hex}")
+            _user.groups.set([shared_group])
+            # this sets P[n].groups -> U[n].groups
+            Membership.objects.create(user_id=_user.id, project_id=P[n].id, role='owner')
 
         # Arrange the catalogs
         for catalog in C.values():
@@ -97,9 +112,9 @@ def arrange_projects_sites_and_views():
         for n, view in V.items():
             view.available = True
             view.save()
-            view.catalogs.clear()
-            view.groups.clear()
             view.sites.set([S[n]])
+            view.catalogs.set(list(C.values()))
+            view.groups.set([shared_group])
 
         for n in one_two_three:
             P[n].views.set([V[n]])
@@ -123,9 +138,13 @@ def arrange_projects_groups_and_views():
             for n in one_two_three
         }
         # Create groups, users and project memberships
-        G = {n: Group.objects.create(name=f"Sync G{n}") for n in one_two_three}
+        G = {
+            1: Group.objects.get(name="view_test"),
+            2: Group.objects.get(name="editor"),
+            3: Group.objects.get(name="reviewer"),
+        }
         for n in one_two_three:
-            _user = User.objects.create(username=f"Sync U{n}")
+            _user = User.objects.create(username=f"Sync U{n}-{uuid4().hex}")
             _user.groups.set([G[n]])
             # this sets P[1].groups -> U[n].groups
             Membership.objects.create(user_id=_user.id, project_id=P[n].id, role='owner')
@@ -148,8 +167,8 @@ def arrange_projects_groups_and_views():
         for n, view in V.items():
             view.available = True
             view.save()
-            view.catalogs.clear()
             view.sites.clear()
+            view.catalogs.set(list(C.values()))
             view.groups.set([G[n]])  # set groups as last so that will be state
 
         for n in one_two_three:

rdmo/projects/tests/helpers/project_sync/assert_project_views_or_tasks.py

@@ -28,27 +28,26 @@ def assert_all_projects_are_synced_with_instance_m2m_field(instance: Task | View
         project_instances = getattr(project, m2m_field).all()
         project_has_instance = instance in project_instances
 
-        # (e.g. Task/View has no catalogs / no sites / no groups)
-        if not instance_field.exists():
-            if instance_project_field == 'site' and settings.MULTISITE:
-                # MULTISITE=True and no sites on the instance:
-                project_should_have_instance = False
-            else:
-                # For catalogs and groups, and for sites when MULTISITE=False:
-                #   empty field means "applies to all projects"
-                project_should_have_instance = True
+        # Base rules: catalogs + groups must match; sites must match in multisite.
+        if not instance.catalogs.exists():
+            project_should_have_instance = False
+        elif not instance.groups.exists():
+            project_should_have_instance = False
+        elif settings.MULTISITE and not instance.sites.exists():
+            project_should_have_instance = False
         else:
-            if instance_project_field == 'catalog':
-                project_should_have_instance = bool(project.catalog in instance_field.all())
-            elif instance_project_field == 'site':
-                project_should_have_instance = bool(project.site in instance_field.all())
-            elif instance_project_field == 'groups':
-                instance_ids = set(instance_field.values_list('id', flat=True))
-                project_groups_ids = {group.id for group in getattr(project, instance_project_field)}
-                # project must have at least one group and all must be within instance_ids
-                project_should_have_instance = bool(project_groups_ids and project_groups_ids <= instance_ids)
+            catalog_matches = project.catalog in instance.catalogs.all()
+            if instance.sites.exists():
+                site_matches = project.site in instance.sites.all()
             else:
-                raise ValueError("Project field not recognized, should be 'site', 'catalog' or 'groups'")
+                site_matches = not settings.MULTISITE
+            instance_group_ids = set(instance.groups.values_list('id', flat=True))
+            project_group_ids = {group.id for group in project.groups}
+            group_matches = bool(instance_group_ids & project_group_ids)
+            project_should_have_instance = bool(catalog_matches and site_matches and group_matches)
+
+        if instance_project_field == 'site' and not instance_field.exists() and settings.MULTISITE:
+            project_should_have_instance = False
 
         if project_should_have_instance:
             if not project_has_instance: