From c504d07a6c0f6ae15aa22248d0e697c20c01c0ea Mon Sep 17 00:00:00 2001 From: nitely Date: Sat, 1 Sep 2018 01:14:00 -0300 Subject: [PATCH] fixes #237 --- spirit/comment/utils.py | 5 ++++ spirit/comment/views.py | 5 ++-- spirit/topic/notification/managers.py | 3 +++ spirit/topic/notification/models.py | 29 ++++++++++++++++++++ spirit/topic/notification/views.py | 38 ++++++++++++++------------- 5 files changed, 60 insertions(+), 20 deletions(-) diff --git a/spirit/comment/utils.py b/spirit/comment/utils.py index 2a8a7c1dd..5fcb670f2 100644 --- a/spirit/comment/utils.py +++ b/spirit/comment/utils.py @@ -26,3 +26,8 @@ def post_comment_update(comment): comment.comment_html = post_render_static_polls(comment) CommentHistory.create(comment) + + +# XXX add tests +def post_comment_move(comment, topic): + TopicNotification.sync(comment=comment, topic=topic) diff --git a/spirit/comment/views.py b/spirit/comment/views.py index d5a743c00..993e64e12 100644 --- a/spirit/comment/views.py +++ b/spirit/comment/views.py @@ -16,7 +16,7 @@ from ..topic.models import Topic from .models import Comment from .forms import CommentForm, CommentMoveForm, CommentImageForm, CommentFileForm -from .utils import comment_posted, post_comment_update, pre_comment_update +from .utils import comment_posted, post_comment_update, pre_comment_update, post_comment_move @login_required @@ -109,6 +109,7 @@ def move(request, topic_id): for comment in comments: comment_posted(comment=comment, mentions=None) topic.decrease_comment_count() + post_comment_move(comment=comment, topic=topic) else: messages.error(request, render_form_errors(form)) @@ -116,7 +117,7 @@ def move(request, topic_id): def find(request, pk): - comment = get_object_or_404(Comment, pk=pk) + comment = get_object_or_404(Comment.objects.select_related('topic'), pk=pk) comment_number = Comment.objects.filter(topic=comment.topic, date__lte=comment.date).count() url = paginator.get_url(comment.topic.get_absolute_url(), comment_number, diff --git a/spirit/topic/notification/managers.py b/spirit/topic/notification/managers.py index 7791b1d9d..4b7c09012 100644 --- a/spirit/topic/notification/managers.py +++ b/spirit/topic/notification/managers.py @@ -29,3 +29,6 @@ def read(self, user): # returns updated rows count (int) return self.filter(user=user)\ .update(is_read=True) + + def with_related_data(self): + return self.select_related('comment__user__st', 'topic') diff --git a/spirit/topic/notification/models.py b/spirit/topic/notification/models.py index 470b69aab..ab3c21468 100644 --- a/spirit/topic/notification/models.py +++ b/spirit/topic/notification/models.py @@ -40,6 +40,9 @@ class Meta: verbose_name_plural = _("topics notification") def get_absolute_url(self): + if self.topic_id != self.comment.topic_id: + # Out of sync + return self.topic.get_absolute_url() return self.comment.get_absolute_url() @property @@ -117,3 +120,29 @@ def bulk_create(cls, users, comment): is_active=True) for user in users ]) + + # XXX add tests + # XXX fix with migration (see issue #237) + @classmethod + def sync(cls, comment, topic): + # Notifications can go out of sync + # when the comment is no longer + # within the topic (i.e moved). + # User is subscribed to the topic, + # not the comment, so we either update + # it to a newer comment or set it as undefined + if comment.topic_id == topic.pk: + return + next_comment = ( + topic.comment_set + .filter(date__gt=comment.date) + .order_by('date') + .first()) + if next_comment is None: + (cls.objects + .filter(comment=comment, topic=topic) + .update(is_read=True, action=UNDEFINED)) + return + (cls.objects + .filter(comment=comment, topic=topic) + .update(is_read=True, comment=next_comment, action=COMMENT)) diff --git a/spirit/topic/notification/views.py b/spirit/topic/notification/views.py index 00424ff1a..bead7cf96 100644 --- a/spirit/topic/notification/views.py +++ b/spirit/topic/notification/views.py @@ -55,10 +55,11 @@ def index_ajax(request): if not request.is_ajax(): return Http404() - notifications = TopicNotification.objects\ - .for_access(request.user)\ - .order_by("is_read", "-date")\ - .select_related('comment__user__st', 'comment__topic') + notifications = ( + TopicNotification.objects + .for_access(request.user) + .order_by("is_read", "-date") + .with_related_data()) notifications = notifications[:settings.ST_NOTIFICATIONS_PER_PAGE] @@ -66,38 +67,38 @@ def index_ajax(request): { 'user': escape(n.comment.user.username), 'action': n.action, - 'title': escape(n.comment.topic.title), + 'title': escape(n.topic.title), 'url': n.get_absolute_url(), 'is_read': n.is_read } for n in notifications ] - return HttpResponse(json.dumps({'n': notifications, }), content_type="application/json") + return HttpResponse(json.dumps({'n': notifications}), content_type="application/json") @login_required def index_unread(request): - notifications = TopicNotification.objects\ - .for_access(request.user)\ - .filter(is_read=False) + notifications = ( + TopicNotification.objects + .for_access(request.user) + .filter(is_read=False) + .with_related_data()) page = paginate( request, query_set=notifications, lookup_field='date', page_var='notif', - per_page=settings.ST_NOTIFICATIONS_PER_PAGE - ) - next_page_pk = None + per_page=settings.ST_NOTIFICATIONS_PER_PAGE) + next_page_pk = None if page: next_page_pk = page[-1].pk context = { 'page': page, - 'next_page_pk': next_page_pk - } + 'next_page_pk': next_page_pk} return render(request, 'spirit/topic/notification/index_unread.html', context) @@ -105,11 +106,12 @@ def index_unread(request): @login_required def index(request): notifications = yt_paginate( - TopicNotification.objects.for_access(request.user), + TopicNotification.objects + .for_access(request.user) + .with_related_data(), per_page=config.topics_per_page, - page_number=request.GET.get('page', 1) - ) + page_number=request.GET.get('page', 1)) - context = {'notifications': notifications, } + context = {'notifications': notifications} return render(request, 'spirit/topic/notification/index.html', context)