Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

signal handlers for syncing of project views and tasks #1218

Open
wants to merge 8 commits into
base: 2.3.0
Choose a base branch
from
8 changes: 6 additions & 2 deletions rdmo/core/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove newline.

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))


Expand Down
7 changes: 5 additions & 2 deletions rdmo/core/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -332,7 +334,8 @@

PROJECT_SEND_INVITE = True

PROJECT_REMOVE_VIEWS = True
PROJECT_VIEWS_SYNC = True
PROJECT_TASKS_SYNC = True

PROJECT_CREATE_RESTRICTED = False
PROJECT_CREATE_GROUPS = []
Expand Down
6 changes: 4 additions & 2 deletions rdmo/projects/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct. from . import handlers just tells django to "load" the file initially. We do nothing with the imported functions. m2m_changed_views and project_save_views do not even exist, maybe this is a leftover that somehow does not cause an error?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should also be settings.PROJECT_REMOVE_VIEWS or settings.PROJECT_VIEWS_SYNC since we just want to load the file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah now I get it, you created a handlers module. Then handlers.py should be deleted and its content should move to handlers/__init__.py (or a file in the directory).

from .handlers import m2m_changed_views, project_save_views # noqa: F401
if settings.PROJECT_TASKS_SYNC:
from .handlers import m2m_changed_tasks, project_save_tasks # noqa: F401
42 changes: 42 additions & 0 deletions rdmo/projects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,16 @@ class Meta:
'catalog': forms.RadioSelect()
}

def save(self, commit=True, *args, **kwargs):
if 'cancel' in self.data:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was just still missing right? ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did "Cancel" save the new instance? The update view was probably just a fuction at one point and cancel was handled there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to check again, if this was needed or not (cancel seems to work already)

return self.instance

# if the catalog is the same, do nothing
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to prevent calling the save method if nothing was changed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, but shouldn't this be handled by the UpdateView... maybe because we use only parts of the model here.

if self.instance.catalog.id == self.cleaned_data.get('catalog'):
return self.instance

return super().save(commit=commit)


class ProjectUpdateTasksForm(forms.ModelForm):

Expand All @@ -180,6 +190,22 @@ class Meta:
'tasks': forms.CheckboxSelectMultiple()
}

def save(self, commit=True, *args, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is just about not saving the instance when nothing changed, right?

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):

Expand All @@ -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):

Expand Down
69 changes: 17 additions & 52 deletions rdmo/projects/handlers.py
Original file line number Diff line number Diff line change
@@ -1,60 +1,25 @@
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.db.models.signals import post_save
from django.dispatch import receiver

from rdmo.projects.models import Membership, Project
from rdmo.questions.models import Catalog
from rdmo.projects.models import Project
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(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]) \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should check here if the catalog actually changed. Lets discuss in Zoom.

.exclude(catalogs=None)

@receiver(m2m_changed, sender=View.groups.through)
def m2m_changed_view_groups_signal(sender, instance, **kwargs):
groups = instance.groups.all()
catalogs = instance.catalogs.all()
for view in view_candidates:
instance.views.remove(view)

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()
# 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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or not?


# 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)
for view in view_candidates:
instance.views.add(view)
Empty file.
109 changes: 109 additions & 0 deletions rdmo/projects/handlers/generic_handlers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
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, pk_set, instance, project_field):
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This strengthens my dislike for docstrings ...

Update project relationships for m2m_changed signals.

Args:
action (str): The m2m_changed action (post_add, post_remove, post_clear).
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 = Catalog.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 = 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, 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).
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 = Site.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 = Site.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, 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).
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 = 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()

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 = 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()

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)
41 changes: 41 additions & 0 deletions rdmo/projects/handlers/m2m_changed_tasks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

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, pk_set, **kwargs):
m2m_catalogs_changed_projects_sync_signal_handler(
action=action,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are actually not keyword arguments. I also think it would be nicer if the generic handlers would have the same order of args as the original handler.

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, pk_set, **kwargs):
m2m_sites_changed_projects_sync_signal_handler(
action=action,
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, pk_set, **kwargs):
m2m_groups_changed_projects_sync_signal_handler(
action=action,
pk_set=pk_set,
instance=instance,
project_field='tasks'
)
42 changes: 42 additions & 0 deletions rdmo/projects/handlers/m2m_changed_views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@

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, pk_set, **kwargs):
m2m_catalogs_changed_projects_sync_signal_handler(
action=action,
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, pk_set, **kwargs):
m2m_sites_changed_projects_sync_signal_handler(
action=action,
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, pk_set, **kwargs):
m2m_groups_changed_projects_sync_signal_handler(
action=action,
pk_set=pk_set,
instance=instance,
project_field='views'
)
Loading
Loading