UPD: dav_events: refactored the location of the signal handlers to

reduce the number of class methods on the workflow class.

dav_events/apps.py

@@ -2,8 +2,6 @@ from django.core.exceptions import ImproperlyConfigured
 
 from dav_base.config.apps import AppConfig as _AppConfig, DefaultSetting
 
-from . import signals
-
 DEFAULT_SETTINGS = (
     DefaultSetting('enable_email_on_status_update', False),
     DefaultSetting('enable_email_on_update', False),
@@ -31,6 +29,6 @@ class AppConfig(_AppConfig):
     default_settings = DEFAULT_SETTINGS
 
     def ready(self):
-        from .workflow import DefaultWorkflow
-        signals.event_updated.connect(DefaultWorkflow.send_emails_on_event_update)
-        signals.event_status_updated.connect(DefaultWorkflow.send_emails_on_event_status_update)
+        from . import signals
+        signals.event_updated.connect(signals.send_emails_on_event_update)
+        signals.event_status_updated.connect(signals.send_emails_on_event_status_update)

dav_events/signals.py

@@ -3,3 +3,16 @@ from django.dispatch import Signal
 event_created = Signal(providing_args=['event'])
 event_updated = Signal(providing_args=['event', 'diff', 'user'])
 event_status_updated = Signal(providing_args=['event', 'flag'])
+
+
+def send_emails_on_event_update(sender, **kwargs):
+    event = kwargs.get('event')
+    diff = kwargs.get('diff')
+    updater = kwargs.get('user')
+    event.workflow.send_emails_on_update(diff, updater)
+
+
+def send_emails_on_event_status_update(sender, **kwargs):
+    event = kwargs.get('event')
+    flag = kwargs.get('flag')
+    event.workflow.send_emails_on_status_update(flag)

dav_events/workflow.py

@@ -33,7 +33,7 @@ class BasicWorkflow(object):
         self._event = event
 
     #
-    # Status
+    # Status changes
     #
     def get_number(self):
         event = self._event
@@ -284,83 +284,26 @@ class BasicWorkflow(object):
         return status_list
 
     #
-    # Permission
+    # Notifications (loose coupled via signals)
     #
-    def has_permission(self, user, permission):
-        if user.is_superuser:
-            return True
-
+    def send_emails_on_update(self, diff, updater):
         event = self._event
 
-        if permission == 'view':
-            if user == event.owner:
-                return True
-            if has_role(user, 'manager_super'):
-                return True
-            if has_role(user, 'manager_{}'.format(event.sport.lower())):
-                return True
-            if has_role(user, 'publisher') and self.has_reached_status('accepted'):
-                return True
-        elif permission == 'submit':
-            if user == event.owner:
-                return True
-        elif permission == 'accept':
-            if has_role(user, 'manager_super'):
-                return True
-            if has_role(user, 'manager_{}'.format(event.sport.lower())):
-                return True
-        elif permission == 'publish':
-            if has_role(user, 'publisher'):
-                return True
-        elif permission == 'update':
-            if not self.has_reached_status('submitted'):
-                if user == event.owner:
-                    return True
-            elif not self.has_reached_status('accepted'):
-                if has_role(user, 'manager_super'):
-                    return True
-                if has_role(user, 'manager_{}'.format(event.sport.lower())):
-                    return True
-            elif has_role(user, 'publisher'):
-                return True
-        return False
-
-    # TODO: is a class method a good idea?
-    @classmethod
-    def has_global_permission(cls, user, permission):
-        if user.is_superuser:
-            return True
-
-        if permission == 'export':
-            return has_role(user, 'publisher')
-        return False
-
-    #
-    # Signal handlers
-    #
-    # TODO: the signal handlers should not be part of the workflow class,
-    #  but call a method from the workflow of the particular event.
-    @classmethod
-    def send_emails_on_event_update(cls, sender, **kwargs):
-        event = kwargs.get('event')
-        diff = kwargs.get('diff')
-        updater = kwargs.get('user')
-
         app_config = apps.get_containing_app_config(__package__)
         if not app_config.settings.enable_email_on_update:
             return
 
         if len(diff) < 1:
-            logger.debug('send_emails_on_event_update(): No diff data -> Skip sending mails.')
+            logger.debug('send_emails_on_update(): No diff data -> Skip sending mails.')
             return
         diff_text = '\n'.join(diff[3:])
 
         # Who should be informed about the update?
         recipients = [event.owner]
-        if event.workflow.has_reached_status('submitted'):
+        if self.has_reached_status('submitted'):
             # If the event is already submitted, add managers to the recipients.
             recipients += get_users_by_role('manager_{}'.format(event.sport.lower()))
-            if event.workflow.has_reached_status('accepted'):
+            if self.has_reached_status('accepted'):
                 # If the event is already published, add publishers to the recipients.
                 recipients += get_users_by_role('publisher_web')
                 recipients += get_users_by_role('publisher_facebook')
@@ -370,10 +313,8 @@ class BasicWorkflow(object):
                 email = emails.EventUpdatedMail(recipient=recipient, event=event, editor=updater, diff=diff_text)
                 email.send()
 
-    @classmethod
-    def send_emails_on_event_status_update(cls, sender, **kwargs):
-        event = kwargs.get('event')
-        flag = kwargs.get('flag')
+    def send_emails_on_status_update(self, flag):
+        event = self._event
         updater = flag.user
 
         app_config = apps.get_containing_app_config(__package__)
@@ -439,6 +380,58 @@ class BasicWorkflow(object):
                                                               confirm_publication_action=action)
                     email.send()
 
+    #
+    # Permission
+    #
+    def has_permission(self, user, permission):
+        if user.is_superuser:
+            return True
+
+        event = self._event
+
+        if permission == 'view':
+            if user == event.owner:
+                return True
+            if has_role(user, 'manager_super'):
+                return True
+            if has_role(user, 'manager_{}'.format(event.sport.lower())):
+                return True
+            if has_role(user, 'publisher') and self.has_reached_status('accepted'):
+                return True
+        elif permission == 'submit':
+            if user == event.owner:
+                return True
+        elif permission == 'accept':
+            if has_role(user, 'manager_super'):
+                return True
+            if has_role(user, 'manager_{}'.format(event.sport.lower())):
+                return True
+        elif permission == 'publish':
+            if has_role(user, 'publisher'):
+                return True
+        elif permission == 'update':
+            if not self.has_reached_status('submitted'):
+                if user == event.owner:
+                    return True
+            elif not self.has_reached_status('accepted'):
+                if has_role(user, 'manager_super'):
+                    return True
+                if has_role(user, 'manager_{}'.format(event.sport.lower())):
+                    return True
+            elif has_role(user, 'publisher'):
+                return True
+        return False
+
+    # TODO: is a class method a good idea?
+    @classmethod
+    def has_global_permission(cls, user, permission):
+        if user.is_superuser:
+            return True
+
+        if permission == 'export':
+            return has_role(user, 'publisher')
+        return False
+
     #
     # Misc logic
     #