diff --git a/spirit/comment/history/models.py b/spirit/comment/history/models.py index 04cd852b..29bcce4f 100644 --- a/spirit/comment/history/models.py +++ b/spirit/comment/history/models.py @@ -6,6 +6,7 @@ from django.db import models from django.utils.translation import ugettext_lazy as _ from django.core.urlresolvers import reverse from django.utils import timezone +from django.db import transaction class CommentHistory(models.Model): @@ -26,11 +27,12 @@ class CommentHistory(models.Model): @classmethod def create(cls, comment): - return cls.objects.create( - comment_fk=comment, - comment_html=comment.comment_html, - date=comment.date - ) + with transaction.atomic(): + return cls.objects.create( + comment_fk=comment, + comment_html=comment.comment_html, + date=comment.date + ) @classmethod def create_maybe(cls, comment): diff --git a/spirit/comment/tests.py b/spirit/comment/tests.py index f86e9697..ac0f559a 100644 --- a/spirit/comment/tests.py +++ b/spirit/comment/tests.py @@ -19,7 +19,7 @@ from django.utils.six import BytesIO from ..core.tests import utils from .models import Comment from .forms import CommentForm, CommentMoveForm, CommentImageForm -from .signals import comment_posted, comment_moved +from .signals import comment_moved from .tags import render_comments_form from ..core.utils import markdown from .views import delete as comment_delete @@ -27,6 +27,8 @@ from ..topic.models import Topic from ..category.models import Category from ..user.models import UserProfile from .history.models import CommentHistory +from .utils import comment_posted +from ..topic.notification.models import TopicNotification, MENTION User = get_user_model() @@ -147,19 +149,6 @@ class CommentViewTest(TestCase): form_data) self.assertEqual(response.status_code, 404) - def test_comment_publish_signal(self): - """ - create comment signal - """ - def comment_posted_handler(sender, comment, **kwargs): - self._comment = comment - comment_posted.connect(comment_posted_handler) - - utils.login(self) - form_data = {'comment': 'foobar', } - self.client.post(reverse('spirit:comment:publish', kwargs={'topic_id': self.topic.pk, }), form_data) - self.assertEqual(self._comment.comment, 'foobar') - def test_comment_publish_quote(self): """ create comment quote @@ -337,12 +326,6 @@ class CommentViewTest(TestCase): """ move comments, emit signal """ - self._comments = [] - - def comment_posted_handler(sender, comment, **kwargs): - self._comments.append(comment) - comment_posted.connect(comment_posted_handler) - def comment_moved_handler(sender, comments, topic_from, **kwargs): self._comment_count = len(comments) self._topic_from = topic_from @@ -361,7 +344,6 @@ class CommentViewTest(TestCase): response = self.client.post(reverse('spirit:comment:move', kwargs={'topic_id': self.topic.pk, }), form_data) self.assertEqual(response.status_code, 302) - self.assertListEqual(self._comments, [comment2, comment]) self.assertEqual(self._comment_count, 2) self.assertEqual(self._topic_from, self.topic) @@ -574,3 +556,40 @@ class CommentFormTest(TestCase): files = {'image': SimpleUploadedFile('image.gif', img.read(), content_type='image/gif'), } form = CommentImageForm(data={}, files=files) self.assertFalse(form.is_valid()) + + +class CommentUtilsTest(TestCase): + + def setUp(self): + cache.clear() + self.user = utils.create_user() + self.category = utils.create_category() + self.topic = utils.create_topic(category=self.category, user=self.user) + + def test_comment_posted(self): + """ + * Should create subscription + * Should notify subscribers + * Should notify mentions + """ + # Should create subscription + subscriber = self.user + comment = utils.create_comment(user=subscriber, topic=self.topic) + comment_posted(comment=comment, mentions=None) + self.assertEqual(len(TopicNotification.objects.all()), 1) + self.assertTrue(TopicNotification.objects.get(user=subscriber, topic=self.topic).is_read) + + # Should notify subscribers + user = utils.create_user() + comment = utils.create_comment(user=user, topic=self.topic) + comment_posted(comment=comment, mentions=None) + self.assertEqual(len(TopicNotification.objects.all()), 2) + self.assertFalse(TopicNotification.objects.get(user=subscriber, topic=self.topic).is_read) + + # Should notify mentions + mentioned = utils.create_user() + mentions = {mentioned.username: mentioned, } + comment = utils.create_comment(user=user, topic=self.topic) + comment_posted(comment=comment, mentions=mentions) + self.assertEqual(TopicNotification.objects.get(user=mentioned, comment=comment).action, MENTION) + self.assertFalse(TopicNotification.objects.get(user=mentioned, comment=comment).is_read) \ No newline at end of file diff --git a/spirit/comment/utils.py b/spirit/comment/utils.py new file mode 100644 index 00000000..b05d4b4a --- /dev/null +++ b/spirit/comment/utils.py @@ -0,0 +1,12 @@ +# -*- coding: utf-8 -*- + +from __future__ import unicode_literals + +from ..topic.notification.models import TopicNotification + + +def comment_posted(comment, mentions): + # Todo test detail views + TopicNotification.create_maybe(user=comment.user, topic=comment.topic) + TopicNotification.notify_new_comment(comment=comment) + TopicNotification.notify_new_mentions(comment=comment, mentions=mentions) \ No newline at end of file diff --git a/spirit/comment/views.py b/spirit/comment/views.py index 24be3d2c..bf6fe2fb 100644 --- a/spirit/comment/views.py +++ b/spirit/comment/views.py @@ -17,7 +17,8 @@ from ..topic.models import Topic from .history.models import CommentHistory from .models import Comment from .forms import CommentForm, CommentMoveForm, CommentImageForm -from .signals import comment_posted, comment_moved +from .signals import comment_moved +from .utils import comment_posted @login_required @@ -31,7 +32,7 @@ def publish(request, topic_id, pk=None): if not request.is_limited and form.is_valid(): comment = form.save() - comment_posted.send(sender=comment.__class__, comment=comment, mentions=form.mentions) + comment_posted(comment=comment, mentions=form.mentions) return redirect(request.POST.get('next', comment.get_absolute_url())) else: initial = None @@ -99,7 +100,7 @@ def move(request, topic_id): comments = form.save() for comment in comments: - comment_posted.send(sender=comment.__class__, comment=comment, mentions=None) + comment_posted(comment=comment, mentions=None) comment_moved.send(sender=Comment, comments=comments, topic_from=topic) else: diff --git a/spirit/topic/notification/models.py b/spirit/topic/notification/models.py index 535647b0..b0c9135b 100644 --- a/spirit/topic/notification/models.py +++ b/spirit/topic/notification/models.py @@ -6,6 +6,7 @@ from django.db import models from django.utils.translation import ugettext_lazy as _ from django.conf import settings from django.utils import timezone +from django.db import IntegrityError, transaction from .managers import TopicNotificationQuerySet @@ -61,4 +62,46 @@ class TopicNotification(models.Model): cls.objects\ .filter(user=user, topic=topic)\ - .update(is_read=True) \ No newline at end of file + .update(is_read=True) + + @classmethod + def create_maybe(cls, user, topic): + # Create a dummy notification + return cls.objects.get_or_create( + user=user, + topic=topic, + defaults={ + 'action': COMMENT, + 'is_read': True, + 'is_active': True + } + ) + + @classmethod + def notify_new_comment(cls, comment): + cls.objects\ + .filter(topic=comment.topic, is_active=True, is_read=True)\ + .exclude(user=comment.user)\ + .update(comment=comment, is_read=False, action=COMMENT, date=timezone.now()) + + @classmethod + def notify_new_mentions(cls, comment, mentions): + if not mentions: + return + + # TODO: refactor + for username, user in mentions.items(): + try: + with transaction.atomic(): + cls.objects.create( + user=user, + topic=comment.topic, + comment=comment, + action=MENTION + ) + except IntegrityError: + pass + + cls.objects\ + .filter(user__in=mentions.values(), topic=comment.topic, is_read=True)\ + .update(comment=comment, is_read=False, action=MENTION, date=timezone.now()) \ No newline at end of file diff --git a/spirit/topic/notification/signals.py b/spirit/topic/notification/signals.py index 6a91bb7b..ddf1cee2 100644 --- a/spirit/topic/notification/signals.py +++ b/spirit/topic/notification/signals.py @@ -2,49 +2,10 @@ from __future__ import unicode_literals -from django.utils import timezone from django.db import IntegrityError, transaction -from ...comment.signals import comment_posted from ..private.signals import topic_private_post_create, topic_private_access_pre_create -from .models import TopicNotification, COMMENT, MENTION - - -def notification_comment_posted_handler(sender, comment, **kwargs): - # Create Notification for poster - # if not exists create a dummy one with defaults - try: - TopicNotification.objects.get_or_create(user=comment.user, topic=comment.topic, - defaults={'action': COMMENT, - 'is_read': True, - 'is_active': True}) - except IntegrityError: - pass - - TopicNotification.objects.filter(topic=comment.topic, is_active=True, is_read=True)\ - .exclude(user=comment.user)\ - .update(comment=comment, is_read=False, action=COMMENT, date=timezone.now()) - - -def mention_comment_posted_handler(sender, comment, mentions, **kwargs): - if not mentions: - return - - for username, user in mentions.items(): - try: - with transaction.atomic(): - TopicNotification.objects.create(user=user, topic=comment.topic, - comment=comment, action=MENTION) - except IntegrityError: - pass - - TopicNotification.objects.filter(user__in=mentions.values(), topic=comment.topic, is_read=True)\ - .update(comment=comment, is_read=False, action=MENTION, date=timezone.now()) - - -def comment_posted_handler(sender, comment, mentions, **kwargs): - notification_comment_posted_handler(sender, comment, **kwargs) - mention_comment_posted_handler(sender, comment, mentions, **kwargs) +from .models import TopicNotification, COMMENT def topic_private_post_create_handler(sender, topics_private, comment, **kwargs): @@ -67,6 +28,6 @@ def topic_private_access_pre_create_handler(sender, topic, user, **kwargs): except IntegrityError: pass -comment_posted.connect(comment_posted_handler, dispatch_uid=__name__) + topic_private_post_create.connect(topic_private_post_create_handler, dispatch_uid=__name__) topic_private_access_pre_create.connect(topic_private_access_pre_create_handler, dispatch_uid=__name__) \ No newline at end of file diff --git a/spirit/topic/notification/tests.py b/spirit/topic/notification/tests.py index f0a2913e..ebb81362 100644 --- a/spirit/topic/notification/tests.py +++ b/spirit/topic/notification/tests.py @@ -366,46 +366,6 @@ class TopicNotificationModelsTest(TestCase): comment=self.comment, is_active=True, action=COMMENT, is_read=True) - def test_topic_notification_comment_handler(self): - """ - set is_read=False when a comment is posted - """ - comment = utils.create_comment(topic=self.topic) - comment_posted.send(sender=self.topic.__class__, comment=comment, mentions=None) - self.assertFalse(TopicNotification.objects.get(pk=self.topic_notification.pk).is_read) - - def test_topic_notification_comment_handler_unactive(self): - """ - do nothing if notification is_active=False - """ - TopicNotification.objects.filter(pk=self.topic_notification.pk).update(is_active=False) - comment = utils.create_comment(topic=self.topic_notification.topic) - comment_posted.send(sender=self.topic.__class__, comment=comment, mentions=None) - self.assertTrue(TopicNotification.objects.get(pk=self.topic_notification.pk).is_read) - - def test_topic_notification_mention_handler(self): - """ - create notification on mention - """ - topic = utils.create_topic(self.category) - mentions = {self.user.username: self.user, } - comment = utils.create_comment(topic=topic) - comment_posted.send(sender=self.topic.__class__, comment=comment, mentions=mentions) - self.assertEqual(TopicNotification.objects.get(user=self.user, comment=comment).action, MENTION) - self.assertFalse(TopicNotification.objects.get(user=self.user, comment=comment).is_read) - - def test_topic_notification_mention_handler_unactive(self): - """ - set is_read=False when user gets mentioned - even if is_active=False - """ - TopicNotification.objects.filter(pk=self.topic_notification.pk).update(is_active=False) - mentions = {self.user.username: self.user, } - comment = utils.create_comment(topic=self.topic_notification.topic) - comment_posted.send(sender=self.topic.__class__, comment=comment, mentions=mentions) - self.assertEqual(TopicNotification.objects.get(pk=self.topic_notification.pk).action, MENTION) - self.assertFalse(TopicNotification.objects.get(pk=self.topic_notification.pk).is_read) - def test_topic_private_post_create_handler(self): """ create notifications on topic private created @@ -452,6 +412,84 @@ class TopicNotificationModelsTest(TestCase): notification = TopicNotification.objects.get(user=private.user, topic=private.topic) self.assertTrue(notification.is_read) + def test_topic_notification_create_maybe(self): + """ + Should create a notification if does not exists + """ + user = utils.create_user() + topic = utils.create_topic(self.category) + TopicNotification.create_maybe(user=user, topic=topic) + notification = TopicNotification.objects.get(user=user, topic=topic) + self.assertTrue(notification.is_active) + self.assertTrue(notification.is_read) + self.assertEqual(notification.action, COMMENT) + + # Creating it again should do nothing + TopicNotification.objects.filter(user=user, topic=topic).update(is_active=False) + TopicNotification.create_maybe(user=user, topic=topic) + self.assertFalse(TopicNotification.objects.get(user=user, topic=topic).is_active) + + def test_topic_notification_notify_new_comment(self): + """ + Should set is_read=False to all notifiers/users + """ + creator = utils.create_user() + subscriber = utils.create_user() + topic = utils.create_topic(self.category) + comment = utils.create_comment(user=creator, topic=topic) + TopicNotification.objects.create(user=creator, topic=topic, comment=comment, + is_active=True, is_read=True) + TopicNotification.objects.create(user=subscriber, topic=topic, comment=comment, + is_active=True, is_read=True) + + TopicNotification.notify_new_comment(comment) + notification = TopicNotification.objects.get(user=subscriber, topic=topic) + self.assertTrue(notification.is_active) + self.assertFalse(notification.is_read) + self.assertEqual(notification.action, COMMENT) + + # Author should not be notified of its own comment + notification2 = TopicNotification.objects.get(user=creator, topic=topic) + self.assertTrue(notification2.is_read) + + def test_topic_notification_notify_new_comment_unactive(self): + """ + Should do nothing if notification is unactive + """ + creator = utils.create_user() + subscriber = utils.create_user() + topic = utils.create_topic(self.category) + comment = utils.create_comment(user=creator, topic=topic) + TopicNotification.objects.create(user=subscriber, topic=topic, comment=comment, + is_active=False, is_read=True) + + TopicNotification.notify_new_comment(comment) + notification = TopicNotification.objects.get(user=subscriber, topic=topic) + self.assertTrue(notification.is_read) + + def test_topic_notification_notify_new_mentions(self): + """ + Should notify mentions + """ + topic = utils.create_topic(self.category) + mentions = {self.user.username: self.user, } + comment = utils.create_comment(topic=topic) + TopicNotification.notify_new_mentions(comment=comment, mentions=mentions) + self.assertEqual(TopicNotification.objects.get(user=self.user, comment=comment).action, MENTION) + self.assertFalse(TopicNotification.objects.get(user=self.user, comment=comment).is_read) + + def test_topic_notification_notify_new_mentions_unactive(self): + """ + set is_read=False when user gets mentioned + even if is_active=False + """ + TopicNotification.objects.filter(pk=self.topic_notification.pk).update(is_active=False) + mentions = {self.user.username: self.user, } + comment = utils.create_comment(topic=self.topic_notification.topic) + TopicNotification.notify_new_mentions(comment=comment, mentions=mentions) + self.assertEqual(TopicNotification.objects.get(pk=self.topic_notification.pk).action, MENTION) + self.assertFalse(TopicNotification.objects.get(pk=self.topic_notification.pk).is_read) + class TopicNotificationTemplateTagsTest(TestCase): diff --git a/spirit/topic/private/tests.py b/spirit/topic/private/tests.py index aa3189dd..d916e4a9 100644 --- a/spirit/topic/private/tests.py +++ b/spirit/topic/private/tests.py @@ -8,7 +8,6 @@ from django.core.cache import cache from django.core.urlresolvers import reverse from django.template import Template, Context from django.conf import settings -from django.utils import six from django.utils import timezone from djconfig.utils import override_djconfig @@ -24,6 +23,7 @@ from ...comment.models import Comment from .signals import topic_private_post_create, topic_private_access_pre_create from ..models import Topic from ...comment.bookmark.models import CommentBookmark +from .. import utils as utils_topic class TopicPrivateViewTest(TestCase): @@ -48,22 +48,6 @@ class TopicPrivateViewTest(TestCase): response = self.client.get(reverse('spirit:topic:private:publish')) self.assertEqual(response.status_code, 200) - def test_private_publish_comment_posted_signals(self): - """ - send publish_comment_posted signal - """ - def comment_posted_handler(sender, comment, **kwargs): - self._comment = comment - comment_posted.connect(comment_posted_handler) - - utils.login(self) - form_data = {'comment': 'foo', 'title': 'foobar', 'users': self.user2.username} - response = self.client.post(reverse('spirit:topic:private:publish'), - form_data) - self.assertEqual(response.status_code, 302) - comment = Comment.objects.last() - self.assertEqual(self._comment, comment) - def test_private_publish_topic_private_post_create_signals(self): """ send topic_private_post_create signal @@ -133,6 +117,26 @@ class TopicPrivateViewTest(TestCase): self.assertEqual(response.context['topic'], private.topic) self.assertEqual(list(response.context['comments']), [comment1, comment2]) + def test_topic_private_detail_viewed(self): + """ + Calls utils.topic_viewed + """ + def mocked_topic_viewed(request, topic): + self._user = request.user + self._topic = topic + + org_viewed, utils_topic.topic_viewed = utils_topic.topic_viewed, mocked_topic_viewed + try: + utils.login(self) + category = utils.create_category() + topic = utils.create_topic(category=category, user=self.user) + response = self.client.get(reverse('spirit:topic:detail', kwargs={'pk': topic.pk, 'slug': topic.slug})) + self.assertEqual(response.status_code, 200) + self.assertEqual(self._topic, topic) + self.assertEqual(self._user, self.user) + finally: + utils_topic.topic_viewed = org_viewed + def test_private_access_create(self): """ private topic access creation diff --git a/spirit/topic/private/views.py b/spirit/topic/private/views.py index f06db93e..dbd42aaa 100644 --- a/spirit/topic/private/views.py +++ b/spirit/topic/private/views.py @@ -17,7 +17,7 @@ from ...core import utils from ...core.utils.paginator import paginate, yt_paginate from ...core.utils.ratelimit.decorators import ratelimit from ...comment.forms import CommentForm -from ...comment.signals import comment_posted +from ...comment.utils import comment_posted from ...comment.models import Comment from ..models import Topic from ..utils import topic_viewed @@ -42,7 +42,7 @@ def publish(request, user_id=None): topic = tform.save() cform.topic = topic comment = cform.save() - comment_posted.send(sender=comment.__class__, comment=comment, mentions=None) + comment_posted(comment=comment, mentions=None) tpform.topic = topic topics_private = tpform.save_m2m() topic_private_post_create.send(sender=TopicPrivate, topics_private=topics_private, comment=comment) diff --git a/spirit/topic/tests.py b/spirit/topic/tests.py index 8e14de51..af8ec63b 100644 --- a/spirit/topic/tests.py +++ b/spirit/topic/tests.py @@ -113,25 +113,6 @@ class TopicViewTest(TestCase): response = self.client.get(reverse('spirit:topic:publish', kwargs={'category_id': str(99), })) self.assertEqual(response.status_code, 404) - def test_topic_publish_comment_posted_signals(self): - """ - send publish_comment_posted signal - """ - def comment_posted_handler(sender, comment, **kwargs): - self._comment = comment - comment_posted.connect(comment_posted_handler) - - utils.login(self) - - category = utils.create_category() - form_data = {'title': 'foobar', 'category': category.pk, 'comment': 'foo', - 'choices-TOTAL_FORMS': 2, 'choices-INITIAL_FORMS': 0, 'choice_limit': 1} - response = self.client.post(reverse('spirit:topic:publish'), - form_data) - self.assertEqual(response.status_code, 302) - comment = Comment.objects.last() - self.assertEqual(self._comment, comment) - def test_topic_publish_poll(self): """ POST, create topic + poll diff --git a/spirit/topic/utils.py b/spirit/topic/utils.py index d550008c..a36cf907 100644 --- a/spirit/topic/utils.py +++ b/spirit/topic/utils.py @@ -8,6 +8,7 @@ from .unread.models import TopicUnread def topic_viewed(request, topic): + # Todo test detail views user = request.user comment_number = CommentBookmark.page_to_comment_number(request.GET.get('page', 1)) diff --git a/spirit/topic/views.py b/spirit/topic/views.py index 5db0e26b..68db07f7 100644 --- a/spirit/topic/views.py +++ b/spirit/topic/views.py @@ -13,7 +13,7 @@ from ..core.utils.ratelimit.decorators import ratelimit from ..category.models import Category from ..comment.models import MOVED from ..comment.forms import CommentForm -from ..comment.signals import comment_posted +from ..comment.utils import comment_posted from ..comment.models import Comment from .poll.forms import TopicPollForm, TopicPollChoiceFormSet from .models import Topic @@ -41,7 +41,7 @@ def publish(request, category_id=None): cform.topic = topic comment = cform.save() - comment_posted.send(sender=comment.__class__, comment=comment, mentions=cform.mentions) + comment_posted(comment=comment, mentions=cform.mentions) # Create a poll only if we have choices if pformset.is_filled():