[#7981] allow notifications to be fired against multiple artifacts so that Thread- and Forum-level subscriptions work
Project: http://git-wip-us.apache.org/repos/asf/allura/repo Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/7779c3da Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/7779c3da Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/7779c3da Branch: refs/heads/db/7981 Commit: 7779c3da4a85babcc744ff3b08d35fc310234f99 Parents: 81c478f Author: Dave Brondsema <d...@brondsema.net> Authored: Fri Nov 18 15:26:47 2016 -0500 Committer: Dave Brondsema <d...@brondsema.net> Committed: Fri Nov 18 17:53:17 2016 -0500 ---------------------------------------------------------------------- Allura/allura/model/discuss.py | 10 +++++---- Allura/allura/model/notification.py | 29 +++++++++++++------------- Allura/allura/tasks/notification_tasks.py | 4 ++-- Allura/allura/tests/test_tasks.py | 4 ++-- 4 files changed, 25 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/allura/blob/7779c3da/Allura/allura/model/discuss.py ---------------------------------------------------------------------- diff --git a/Allura/allura/model/discuss.py b/Allura/allura/model/discuss.py index 19c0ea5..2af6146 100644 --- a/Allura/allura/model/discuss.py +++ b/Allura/allura/model/discuss.py @@ -758,16 +758,18 @@ class Post(Message, VersionedArtifact, ActivityObject): n = Notification.query.get(_id=msg_id) if n: # 'approved' notification also exists, re-send - n.fire_notification_task(artifact, 'message') + n.fire_notification_task([artifact, self.thread], 'message') else: # 'approved' notification does not exist, create notification_params['message_id'] = msg_id if not n: - n = Notification.post(artifact, 'message', **notification_params) + # artifact is Forum (or artifact like WikiPage) + n = Notification.post(artifact, 'message', + additional_artifacts_to_match_subscriptions=self.thread, + **notification_params) if not n: return - if (hasattr(artifact, "monitoring_email") - and artifact.monitoring_email): + if getattr(artifact, 'monitoring_email', None): if hasattr(artifact, 'notify_post'): if artifact.notify_post: n.send_simple(artifact.monitoring_email) http://git-wip-us.apache.org/repos/asf/allura/blob/7779c3da/Allura/allura/model/notification.py ---------------------------------------------------------------------- diff --git a/Allura/allura/model/notification.py b/Allura/allura/model/notification.py index 955a75d..52ef451 100644 --- a/Allura/allura/model/notification.py +++ b/Allura/allura/model/notification.py @@ -43,7 +43,7 @@ from pylons import tmpl_context as c, app_globals as g from tg import config import pymongo import jinja2 -from paste.deploy.converters import asbool +from paste.deploy.converters import asbool, aslist from ming import schema as S from ming.orm import FieldProperty, ForeignIdProperty, RelationProperty, session @@ -108,24 +108,23 @@ class Notification(MappedClass): ) @classmethod - def post(cls, artifact, topic, **kw): - '''Create a notification and send the notify message''' + def post(cls, artifact, topic, additional_artifacts_to_match_subscriptions=None, **kw): + '''Create a notification and send the notify message''' n = cls._make_notification(artifact, topic, **kw) if n: # make sure notification is flushed in time for task to process it session(n).flush(n) - n.fire_notification_task(artifact, topic) + artifacts = [artifact] + aslist(additional_artifacts_to_match_subscriptions) + n.fire_notification_task(artifacts, topic) return n - def fire_notification_task(self, artifact, topic): + def fire_notification_task(self, artifacts, topic): import allura.tasks.notification_tasks - allura.tasks.notification_tasks.notify.post( - self._id, artifact.index_id(), topic) + allura.tasks.notification_tasks.notify.post(self._id, [a.index_id() for a in artifacts], topic) @classmethod def post_user(cls, user, artifact, topic, **kw): - '''Create a notification and deliver directly to a user's flash - mailbox''' + '''Create a notification and deliver directly to a user's flash mailbox''' try: mbox = Mailbox(user_id=user._id, is_flash=True, project_id=None, @@ -530,23 +529,25 @@ class Mailbox(MappedClass): artifact_index_id=artifact_index_id)).count() != 0 @classmethod - def deliver(cls, nid, artifact_index_id, topic): + def deliver(cls, nid, artifact_index_ids, topic): '''Called in the notification message handler to deliver notification IDs to the appropriate mailboxes. Atomically appends the nids to the appropriate mailboxes. ''' + + artifact_index_ids.append(None) # get tool-wide ("None") and specific artifact subscriptions d = { 'project_id': c.project._id, 'app_config_id': c.app.config._id, - 'artifact_index_id': {'$in': [None, artifact_index_id]}, + 'artifact_index_id': {'$in': artifact_index_ids}, 'topic': {'$in': [None, topic]} } mboxes = cls.query.find(d).all() - log.debug('Delivering notification %s to mailboxes [%s]', nid, ', '.join( - [str(m._id) for m in mboxes])) + log.debug('Delivering notification %s to mailboxes [%s]', nid, ', '.join([str(m._id) for m in mboxes])) for mbox in mboxes: try: mbox.query.update( + # _id is automatically specified by ming's "query", so this matches the current mbox {'$push': dict(queue=nid), '$set': dict(last_modified=datetime.utcnow(), queue_empty=False), @@ -558,7 +559,7 @@ class Mailbox(MappedClass): # mboxes for this notification get skipped and lost forever log.exception( 'Error adding notification: %s for artifact %s on project %s to user %s', - nid, artifact_index_id, c.project._id, mbox.user_id) + nid, artifact_index_ids, c.project._id, mbox.user_id) @classmethod def fire_ready(cls): http://git-wip-us.apache.org/repos/asf/allura/blob/7779c3da/Allura/allura/tasks/notification_tasks.py ---------------------------------------------------------------------- diff --git a/Allura/allura/tasks/notification_tasks.py b/Allura/allura/tasks/notification_tasks.py index a6b7564..e370222 100644 --- a/Allura/allura/tasks/notification_tasks.py +++ b/Allura/allura/tasks/notification_tasks.py @@ -19,7 +19,7 @@ from allura.lib.decorators import task @task -def notify(n_id, ref_id, topic): +def notify(n_id, ref_ids, topic): from allura import model as M - M.Mailbox.deliver(n_id, ref_id, topic) + M.Mailbox.deliver(n_id, ref_ids, topic) M.Mailbox.fire_ready() http://git-wip-us.apache.org/repos/asf/allura/blob/7779c3da/Allura/allura/tests/test_tasks.py ---------------------------------------------------------------------- diff --git a/Allura/allura/tests/test_tasks.py b/Allura/allura/tests/test_tasks.py index 7224ec0..abfa021 100644 --- a/Allura/allura/tests/test_tasks.py +++ b/Allura/allura/tests/test_tasks.py @@ -481,8 +481,8 @@ class TestNotificationTasks(unittest.TestCase): def test_delivers_messages(self): with mock.patch.object(M.Mailbox, 'deliver') as deliver: with mock.patch.object(M.Mailbox, 'fire_ready') as fire_ready: - notification_tasks.notify('42', '52', 'none') - assert deliver.called_with('42', '52', 'none') + notification_tasks.notify('42', ['52'], 'none') + assert deliver.called_with('42', ['52'], 'none') assert fire_ready.called_with()