From 0b275e178723108ab3156285b04e9f90d5f96683 Mon Sep 17 00:00:00 2001 From: David Wallace Date: Tue, 3 Feb 2026 09:13:24 +0100 Subject: [PATCH 1/3] Fix #1518, check (un)available views/tasks for sync to projects Signed-off-by: David Wallace --- rdmo/projects/handlers/sync_utils.py | 6 ++-- .../management/commands/sync_projects.py | 4 +-- rdmo/projects/managers.py | 12 ++++--- .../tests/test_command_sync_projects.py | 32 +++++++++++++++++++ ...test_handlers_task_changes_availability.py | 12 +++++++ ...test_handlers_view_changes_availability.py | 12 +++++++ rdmo/projects/utils.py | 13 ++++++-- rdmo/projects/views/project.py | 4 +-- rdmo/projects/views/project_create.py | 4 +-- rdmo/projects/views/project_update.py | 4 +-- rdmo/projects/viewsets.py | 4 +-- 11 files changed, 88 insertions(+), 19 deletions(-) diff --git a/rdmo/projects/handlers/sync_utils.py b/rdmo/projects/handlers/sync_utils.py index 8eace26145..1491c98123 100644 --- a/rdmo/projects/handlers/sync_utils.py +++ b/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) diff --git a/rdmo/projects/management/commands/sync_projects.py b/rdmo/projects/management/commands/sync_projects.py index 13681ac630..985255ba7e 100644 --- a/rdmo/projects/management/commands/sync_projects.py +++ b/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) diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index 7925349935..14bb40ddd8 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -8,6 +8,7 @@ from mptt.querysets import TreeQuerySet from rdmo.accounts.utils import is_site_manager +from rdmo.core.constants import PERMISSIONS from rdmo.core.managers import CurrentSiteManagerMixin @@ -71,12 +72,15 @@ 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 user.has_perms(PERMISSIONS[instance._meta.label_lower]) 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 @@ -266,8 +270,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): diff --git a/rdmo/projects/tests/test_command_sync_projects.py b/rdmo/projects/tests/test_command_sync_projects.py index 7b1f49ff80..6841c4e08f 100644 --- a/rdmo/projects/tests/test_command_sync_projects.py +++ b/rdmo/projects/tests/test_command_sync_projects.py @@ -60,6 +60,38 @@ def test_command_sync_projects_for_views(settings, enable_project_views_sync): assert_all_projects_are_synced_with_instance_m2m_field(view, 'catalogs') +@pytest.mark.django_db +def test_command_sync_projects_removes_unavailable_views(settings, enable_project_views_sync): + assert settings.PROJECT_VIEWS_SYNC + + P, _, V = arrange_projects_catalogs_and_views() + + assert set(P[1].views.all()) == {V[1]} + + V[1].available = False + V[1].save() + + call_command('sync_projects', '--views') + + assert set(P[1].views.all()) == set() + + +@pytest.mark.django_db +def test_command_sync_projects_removes_unavailable_tasks(settings, enable_project_tasks_sync): + assert settings.PROJECT_TASKS_SYNC + + P, _, T = arrange_projects_catalogs_and_tasks() + + assert set(P[1].tasks.all()) == {T[1]} + + T[1].available = False + T[1].save() + + call_command('sync_projects', '--tasks') + + assert set(P[1].tasks.all()) == set() + + @pytest.mark.django_db def test_command_sync_projects_show_and_tasks_displays_output(settings, enable_project_tasks_sync, capsys): diff --git a/rdmo/projects/tests/test_handlers_task_changes_availability.py b/rdmo/projects/tests/test_handlers_task_changes_availability.py index b3f8baf57e..ad9bbdcb2f 100644 --- a/rdmo/projects/tests/test_handlers_task_changes_availability.py +++ b/rdmo/projects/tests/test_handlers_task_changes_availability.py @@ -36,3 +36,15 @@ def test_project_tasks_sync_when_updating_available_on_a_task(settings, enable_p assert set(P[1].tasks.all()) == {T[1]} assert set(P[2].tasks.all()) == {T[2], T[1]} assert set(P[3].tasks.all()) == {T[3]} + + +@pytest.mark.django_db +def test_sync_project_tasks_filters_unavailable_tasks(): + P, _, T = arrange_projects_catalogs_and_tasks() + + assert T[1] in set(P[1].tasks.all()) + + T[1].available = False + T[1].save() + + assert set(P[1].tasks.all()) == set() diff --git a/rdmo/projects/tests/test_handlers_view_changes_availability.py b/rdmo/projects/tests/test_handlers_view_changes_availability.py index 5fa0a3603b..4be1fe8125 100644 --- a/rdmo/projects/tests/test_handlers_view_changes_availability.py +++ b/rdmo/projects/tests/test_handlers_view_changes_availability.py @@ -35,3 +35,15 @@ def test_project_views_sync_when_updating_available_on_a_view(settings, enable_p assert set(P[1].views.all()) == {V[1]} assert set(P[2].views.all()) == {V[2], V[1]} assert set(P[3].views.all()) == {V[3]} # in non-multisite it can be added + + +@pytest.mark.django_db +def test_sync_project_views_filters_unavailable_views(): + P, _, V = arrange_projects_catalogs_and_views() + + assert set(P[1].views.all()) == {V[1]} + + V[1].available = False + V[1].save() + + assert set(P[1].views.all()) == set() diff --git a/rdmo/projects/utils.py b/rdmo/projects/utils.py index 4bd983f0f1..d4a2519d23 100644 --- a/rdmo/projects/utils.py +++ b/rdmo/projects/utils.py @@ -373,12 +373,21 @@ def send_contact_message(request, subject, message): cc=[request.user.email], reply_to=[request.user.email]) -def filter_tasks_or_views_for_project(task_or_view, project) -> TaskQuerySet | ViewQuerySet: - queryset = ( task_or_view.objects +def filter_tasks_or_views_for_project( + task_or_view, + project, + user=None, +) -> TaskQuerySet | ViewQuerySet: + queryset = (task_or_view.objects .filter(Q(catalogs=None) | Q(catalogs=project.catalog)) .filter(Q(groups=None) | Q(groups__in=project.groups)) ) + if user is not None: + queryset = queryset.filter_availability(user) + else: + queryset = queryset.filter(available=True) + if settings.MULTISITE: return queryset.filter(sites=project.site) else: diff --git a/rdmo/projects/views/project.py b/rdmo/projects/views/project.py index 04600c6c16..71f1c23195 100644 --- a/rdmo/projects/views/project.py +++ b/rdmo/projects/views/project.py @@ -67,7 +67,7 @@ def get_context_data(self, **kwargs): context['tasks_available'] = project.tasks.exists() else: context['tasks_available'] = ( - filter_tasks_or_views_for_project(Task, project).filter_availability(self.request.user).exists() + filter_tasks_or_views_for_project(Task, project, user=self.request.user).exists() ) if settings.PROJECT_VIEWS_SYNC: @@ -75,7 +75,7 @@ def get_context_data(self, **kwargs): context['views_available'] = project.views.exists() else: context['views_available'] = ( - filter_tasks_or_views_for_project(View, project).filter_availability(self.request.user).exists() + filter_tasks_or_views_for_project(View, project, user=self.request.user).exists() ) ancestors_import = [] diff --git a/rdmo/projects/views/project_create.py b/rdmo/projects/views/project_create.py index 2e78b0d608..25397a75f6 100644 --- a/rdmo/projects/views/project_create.py +++ b/rdmo/projects/views/project_create.py @@ -49,13 +49,13 @@ def form_valid(self, form): # add all tasks to project if not settings.PROJECT_TASKS_SYNC: - tasks = filter_tasks_or_views_for_project(Task, form.instance).filter_availability(self.request.user) + tasks = filter_tasks_or_views_for_project(Task, form.instance, user=self.request.user) for task in tasks: form.instance.tasks.add(task) # add all views to project if not settings.PROJECT_VIEWS_SYNC: - views = filter_tasks_or_views_for_project(View, form.instance).filter_availability(self.request.user) + views = filter_tasks_or_views_for_project(View, form.instance, user=self.request.user) for view in views: form.instance.views.add(view) diff --git a/rdmo/projects/views/project_update.py b/rdmo/projects/views/project_update.py index 805444e1c3..05e30c9d29 100644 --- a/rdmo/projects/views/project_update.py +++ b/rdmo/projects/views/project_update.py @@ -127,7 +127,7 @@ def dispatch(self, request, *args, **kwargs): def get_form_kwargs(self): form_kwargs = super().get_form_kwargs() form_kwargs.update({ - 'tasks': filter_tasks_or_views_for_project(Task, self.object).filter_availability(self.request.user) + 'tasks': filter_tasks_or_views_for_project(Task, self.object, user=self.request.user) }) return form_kwargs @@ -149,7 +149,7 @@ def dispatch(self, request, *args, **kwargs): def get_form_kwargs(self): form_kwargs = super().get_form_kwargs() form_kwargs.update({ - 'views': filter_tasks_or_views_for_project(View, self.object).filter_availability(self.request.user) + 'views': filter_tasks_or_views_for_project(View, self.object, user=self.request.user) }) return form_kwargs diff --git a/rdmo/projects/viewsets.py b/rdmo/projects/viewsets.py index d6df96c770..45b0ee1429 100644 --- a/rdmo/projects/viewsets.py +++ b/rdmo/projects/viewsets.py @@ -409,13 +409,13 @@ def perform_create(self, serializer): # add all tasks to project if self.request.data.get('tasks') is None: if not settings.PROJECT_TASKS_SYNC: - for task in filter_tasks_or_views_for_project(Task, project).filter_availability(self.request.user): + for task in filter_tasks_or_views_for_project(Task, project, user=self.request.user): project.tasks.add(task) if self.request.data.get('views') is None: # add all views to project if not settings.PROJECT_VIEWS_SYNC: - for view in filter_tasks_or_views_for_project(View, project).filter_availability(self.request.user): + for view in filter_tasks_or_views_for_project(View, project, user=self.request.user): project.views.add(view) From 00ef62b18d3abd7e41db43b2cd06d5ea3f27caa8 Mon Sep 17 00:00:00 2001 From: David Wallace Date: Tue, 3 Feb 2026 10:41:58 +0100 Subject: [PATCH 2/3] Fix #1518, do not sync when catalogs or groups is empty Signed-off-by: David Wallace --- .../management/commands/sync_projects.py | 16 ++++---- rdmo/projects/managers.py | 4 ++ .../project_sync/arrange_project_tasks.py | 31 +++++++++++++--- .../project_sync/arrange_project_views.py | 31 +++++++++++++--- .../assert_project_views_or_tasks.py | 37 +++++++++---------- .../tests/test_handlers_m2m_tasks_catalogs.py | 12 +++--- .../tests/test_handlers_m2m_tasks_groups.py | 12 +++--- .../tests/test_handlers_m2m_tasks_sites.py | 2 +- .../tests/test_handlers_m2m_views_catalogs.py | 12 +++--- .../tests/test_handlers_m2m_views_groups.py | 12 +++--- .../tests/test_handlers_m2m_views_sites.py | 2 +- rdmo/projects/utils.py | 4 +- 12 files changed, 107 insertions(+), 68 deletions(-) diff --git a/rdmo/projects/management/commands/sync_projects.py b/rdmo/projects/management/commands/sync_projects.py index 985255ba7e..ce55de1772 100644 --- a/rdmo/projects/management/commands/sync_projects.py +++ b/rdmo/projects/management/commands/sync_projects.py @@ -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}") - - if view_uris: - self.stdout.write("- Views:") - for view_uri in view_uris: - self.stdout.write(f" - {view_uri}") + self.stdout.write("- Tasks:") + for task_uri in task_uris: + self.stdout.write(f" - {task_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 diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index 14bb40ddd8..d1ff5640e3 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -86,6 +86,8 @@ def filter_projects_for_task_or_view(self, instance, user=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(): @@ -97,6 +99,8 @@ def filter_projects_for_task_or_view(self, instance, user=None): # 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 diff --git a/rdmo/projects/tests/helpers/project_sync/arrange_project_tasks.py b/rdmo/projects/tests/helpers/project_sync/arrange_project_tasks.py index 7f8bf1eb01..f127fb7bd2 100644 --- a/rdmo/projects/tests/helpers/project_sync/arrange_project_tasks.py +++ b/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: diff --git a/rdmo/projects/tests/helpers/project_sync/arrange_project_views.py b/rdmo/projects/tests/helpers/project_sync/arrange_project_views.py index 43ecc6c2b4..da01e6fe3f 100644 --- a/rdmo/projects/tests/helpers/project_sync/arrange_project_views.py +++ b/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: diff --git a/rdmo/projects/tests/helpers/project_sync/assert_project_views_or_tasks.py b/rdmo/projects/tests/helpers/project_sync/assert_project_views_or_tasks.py index ba592196a4..7897542734 100644 --- a/rdmo/projects/tests/helpers/project_sync/assert_project_views_or_tasks.py +++ b/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: diff --git a/rdmo/projects/tests/test_handlers_m2m_tasks_catalogs.py b/rdmo/projects/tests/test_handlers_m2m_tasks_catalogs.py index 52a7d94c13..b75a7b9367 100644 --- a/rdmo/projects/tests/test_handlers_m2m_tasks_catalogs.py +++ b/rdmo/projects/tests/test_handlers_m2m_tasks_catalogs.py @@ -19,13 +19,13 @@ def test_project_tasks_sync_when_updating_task_catalogs(settings, enable_project assert set(P[2].tasks.all()) == {T[2]} assert set(P[3].tasks.all()) == {T[3]} - # === Update: T1 has no catalogs → it should appear in all projects === + # === Update: T1 has no catalogs → it should not appear in any projects === T[1].catalogs.clear() - assert set(P[1].tasks.all()) == {T[1]} # should remain unchanged - assert set(P[2].tasks.all()) == {T[2], T[1]} - assert set(P[3].tasks.all()) == {T[3], T[1]} - # additionally, all of the projects should have T1 - assert Project.objects.filter(tasks=T[1]).count() == Project.objects.all().count() + assert set(P[1].tasks.all()) == set() + assert set(P[2].tasks.all()) == {T[2]} + assert set(P[3].tasks.all()) == {T[3]} + # additionally, no project should have T1 + assert Project.objects.filter(tasks=T[1]).count() == 0 assert_all_projects_are_synced_with_instance_m2m_field(T[1],'catalogs') # === Update: (from empty) add C1 to T1 → it should appear in P1 only again === diff --git a/rdmo/projects/tests/test_handlers_m2m_tasks_groups.py b/rdmo/projects/tests/test_handlers_m2m_tasks_groups.py index ab67c71339..9ff59391d3 100644 --- a/rdmo/projects/tests/test_handlers_m2m_tasks_groups.py +++ b/rdmo/projects/tests/test_handlers_m2m_tasks_groups.py @@ -18,13 +18,13 @@ def test_project_tasks_sync_when_updating_task_groups(settings, enable_project_t assert set(P[2].tasks.all()) == {T[2]} assert set(P[3].tasks.all()) == {T[3]} - # === Update: V1 has no catalogs → it should appear in all projects === + # === Update: V1 has no groups → it should not appear in any projects === T[1].groups.clear() - assert set(P[1].tasks.all()) == {T[1]} # should remain unchanged - assert set(P[2].tasks.all()) == {T[2], T[1]} - assert set(P[3].tasks.all()) == {T[3], T[1]} - # additionally, all of the projects should have T1 - assert Project.objects.filter(tasks=T[1]).count() == Project.objects.all().count() + assert set(P[1].tasks.all()) == set() + assert set(P[2].tasks.all()) == {T[2]} + assert set(P[3].tasks.all()) == {T[3]} + # additionally, no project should have T1 + assert Project.objects.filter(tasks=T[1]).count() == 0 assert_all_projects_are_synced_with_instance_m2m_field(T[1], 'groups') # === Update: (from empty) add C1 to V1 → it should appear in P1 only again === diff --git a/rdmo/projects/tests/test_handlers_m2m_tasks_sites.py b/rdmo/projects/tests/test_handlers_m2m_tasks_sites.py index fbeca8a44b..1ce96eda73 100644 --- a/rdmo/projects/tests/test_handlers_m2m_tasks_sites.py +++ b/rdmo/projects/tests/test_handlers_m2m_tasks_sites.py @@ -24,7 +24,7 @@ def test_project_tasks_sync_when_updating_task_sites(settings, enable_project_ta assert set(P[2].tasks.all()) == {T[2], T[1]} assert set(P[3].tasks.all()) == {T[3], T[1]} # additionally, all of the projects should have T1 - assert Project.objects.filter(tasks=T[1]).count() == Project.objects.all().count() + assert Project.objects.filter(tasks=T[1]).count() == len(P) assert_all_projects_are_synced_with_instance_m2m_field(T[1], 'sites') # === Update: (from empty) add C1 to V1 → it should appear in P1 only again === diff --git a/rdmo/projects/tests/test_handlers_m2m_views_catalogs.py b/rdmo/projects/tests/test_handlers_m2m_views_catalogs.py index 029d3b1518..b8a270304e 100644 --- a/rdmo/projects/tests/test_handlers_m2m_views_catalogs.py +++ b/rdmo/projects/tests/test_handlers_m2m_views_catalogs.py @@ -18,13 +18,13 @@ def test_project_views_sync_when_updating_catalogs_on_a_view(settings, enable_pr assert set(P[2].views.all()) == {V[2]} assert set(P[3].views.all()) == {V[3]} - # === Update: V1 has no catalogs → it should appear in all projects === + # === Update: V1 has no catalogs → it should not appear in any projects === V[1].catalogs.clear() - assert set(P[1].views.all()) == {V[1]} # should remain unchanged - assert set(P[2].views.all()) == {V[2], V[1]} - assert set(P[3].views.all()) == {V[3], V[1]} - # additionally, all of the projects should have T1 - assert Project.objects.filter(views=V[1]).count() == Project.objects.all().count() + assert set(P[1].views.all()) == set() + assert set(P[2].views.all()) == {V[2]} + assert set(P[3].views.all()) == {V[3]} + # additionally, no project should have V1 + assert Project.objects.filter(views=V[1]).count() == 0 assert_all_projects_are_synced_with_instance_m2m_field(V[1], 'catalogs') # === Update: (from empty) add C1 to V1 → it should appear in P1 only again === diff --git a/rdmo/projects/tests/test_handlers_m2m_views_groups.py b/rdmo/projects/tests/test_handlers_m2m_views_groups.py index e2f8c65eee..b4d2d8623b 100644 --- a/rdmo/projects/tests/test_handlers_m2m_views_groups.py +++ b/rdmo/projects/tests/test_handlers_m2m_views_groups.py @@ -18,13 +18,13 @@ def test_project_views_sync_when_updating_view_groups(settings, enable_project_v assert set(P[2].views.all()) == {V[2]} assert set(P[3].views.all()) == {V[3]} - # === Update: V1 has no catalogs → it should appear in all projects === + # === Update: V1 has no groups → it should not appear in any projects === V[1].groups.clear() - assert set(P[1].views.all()) == {V[1]} # should remain unchanged - assert set(P[2].views.all()) == {V[2], V[1]} - assert set(P[3].views.all()) == {V[3], V[1]} - # additionally, all of the projects should have T1 - assert Project.objects.filter(views=V[1]).count() == Project.objects.all().count() + assert set(P[1].views.all()) == set() + assert set(P[2].views.all()) == {V[2]} + assert set(P[3].views.all()) == {V[3]} + # additionally, no project should have V1 + assert Project.objects.filter(views=V[1]).count() == 0 assert_all_projects_are_synced_with_instance_m2m_field(V[1], 'groups') # === Update: (from empty) add C1 to V1 → it should appear in P1 only again === diff --git a/rdmo/projects/tests/test_handlers_m2m_views_sites.py b/rdmo/projects/tests/test_handlers_m2m_views_sites.py index 7c01b99102..cf58d3ae7c 100644 --- a/rdmo/projects/tests/test_handlers_m2m_views_sites.py +++ b/rdmo/projects/tests/test_handlers_m2m_views_sites.py @@ -24,7 +24,7 @@ def test_project_views_sync_when_updating_view_sites(settings, enable_project_vi assert set(P[2].views.all()) == {V[2], V[1]} assert set(P[3].views.all()) == {V[3], V[1]} # additionally, all of the projects should have T1 - assert Project.objects.filter(views=V[1]).count() == Project.objects.all().count() + assert Project.objects.filter(views=V[1]).count() == len(P) assert_all_projects_are_synced_with_instance_m2m_field(V[1], 'sites') # === Update: (from empty) add C1 to V1 → it should appear in P1 only again === diff --git a/rdmo/projects/utils.py b/rdmo/projects/utils.py index d4a2519d23..ba000aea3e 100644 --- a/rdmo/projects/utils.py +++ b/rdmo/projects/utils.py @@ -379,8 +379,8 @@ def filter_tasks_or_views_for_project( user=None, ) -> TaskQuerySet | ViewQuerySet: queryset = (task_or_view.objects - .filter(Q(catalogs=None) | Q(catalogs=project.catalog)) - .filter(Q(groups=None) | Q(groups__in=project.groups)) + .filter(catalogs=project.catalog) + .filter(groups__in=project.groups) ) if user is not None: From b2aded18ac07ee41c925bba7f21ed830840f3602 Mon Sep 17 00:00:00 2001 From: David Wallace Date: Tue, 3 Feb 2026 11:17:20 +0100 Subject: [PATCH 3/3] Refactor to new core.utils.can_view_unavailable Signed-off-by: David Wallace --- rdmo/core/managers.py | 4 ++-- rdmo/core/utils.py | 8 +++++++- rdmo/projects/managers.py | 4 ++-- rdmo/projects/utils.py | 8 +++++++- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/rdmo/core/managers.py b/rdmo/core/managers.py index c660592231..33083e3b37 100644 --- a/rdmo/core/managers.py +++ b/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) diff --git a/rdmo/core/utils.py b/rdmo/core/utils.py index d198111ca8..c447c7c5c3 100644 --- a/rdmo/core/utils.py +++ b/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)] diff --git a/rdmo/projects/managers.py b/rdmo/projects/managers.py index d1ff5640e3..ffb2f6ff08 100644 --- a/rdmo/projects/managers.py +++ b/rdmo/projects/managers.py @@ -8,8 +8,8 @@ from mptt.querysets import TreeQuerySet from rdmo.accounts.utils import is_site_manager -from rdmo.core.constants import PERMISSIONS from rdmo.core.managers import CurrentSiteManagerMixin +from rdmo.core.utils import can_view_unavailable class ProjectQuerySet(TreeQuerySet): @@ -78,7 +78,7 @@ def filter_projects_for_task_or_view(self, instance, user=None): # when View/Task is not available it should not show for any project if user is not None: - if not user.has_perms(PERMISSIONS[instance._meta.label_lower]) and not instance.available: + if not can_view_unavailable(user, instance._meta.model) and not instance.available: return self.none() elif not instance.available: return self.none() diff --git a/rdmo/projects/utils.py b/rdmo/projects/utils.py index ba000aea3e..da03b109a5 100644 --- a/rdmo/projects/utils.py +++ b/rdmo/projects/utils.py @@ -13,7 +13,7 @@ from rdmo.core.mail import send_mail from rdmo.core.plugins import get_plugins -from rdmo.core.utils import remove_double_newlines +from rdmo.core.utils import can_view_unavailable, remove_double_newlines from rdmo.tasks.managers import TaskQuerySet from rdmo.views.managers import ViewQuerySet @@ -378,6 +378,12 @@ def filter_tasks_or_views_for_project( project, user=None, ) -> TaskQuerySet | ViewQuerySet: + if user is not None: + if not can_view_unavailable(user, task_or_view._meta.model) and not task_or_view.available: + return task_or_view.objects.none() + elif not task_or_view.available: + return task_or_view.objects.none() + queryset = (task_or_view.objects .filter(catalogs=project.catalog) .filter(groups__in=project.groups)