From 45b2de0a44edf52edc1d8a2c928476bddb482b78 Mon Sep 17 00:00:00 2001 From: James Southern Date: Thu, 9 Aug 2018 18:12:07 -0600 Subject: [PATCH] Add mark all read button to unread notifications This will only mark notifications up to the timestamp of the most recent one shown on the unread notifications page where it was clicked. --- .../models/comment/comment_notification.py | 10 +- tildes/tildes/resources/comment.py | 28 ++++- tildes/tildes/routes.py | 7 +- .../templates/notifications_unread.jinja2 | 15 ++- tildes/tildes/views/api/web/comment.py | 102 ++++++++++++------ 5 files changed, 124 insertions(+), 38 deletions(-) diff --git a/tildes/tildes/models/comment/comment_notification.py b/tildes/tildes/models/comment/comment_notification.py index 192c975..a2afd52 100644 --- a/tildes/tildes/models/comment/comment_notification.py +++ b/tildes/tildes/models/comment/comment_notification.py @@ -2,8 +2,9 @@ from datetime import datetime import re -from typing import List, Tuple +from typing import Any, List, Sequence, Tuple +from pyramid.security import Allow, DENY_ALL from sqlalchemy import Boolean, Column, ForeignKey, Integer, TIMESTAMP from sqlalchemy.dialects.postgresql import ENUM from sqlalchemy.orm import relationship, Session @@ -66,6 +67,13 @@ class CommentNotification(DatabaseModel): self.comment = comment self.notification_type = notification_type + def __acl__(self) -> Sequence[Tuple[str, Any, str]]: + """Pyramid security ACL.""" + acl = [] + acl.append((Allow, self.user_id, 'mark_read')) + acl.append(DENY_ALL) + return acl + @property def is_comment_reply(self) -> bool: """Return whether this is a comment reply notification.""" diff --git a/tildes/tildes/resources/comment.py b/tildes/tildes/resources/comment.py index 66f8310..cb33805 100644 --- a/tildes/tildes/resources/comment.py +++ b/tildes/tildes/resources/comment.py @@ -1,10 +1,11 @@ """Root factories for comments.""" +from pyramid.httpexceptions import HTTPForbidden from pyramid.request import Request from webargs.pyramidparser import use_kwargs from tildes.lib.id import id36_to_id -from tildes.models.comment import Comment +from tildes.models.comment import Comment, CommentNotification from tildes.resources import get_resource from tildes.schemas.comment import CommentSchema @@ -22,3 +23,28 @@ def comment_by_id36(request: Request, comment_id36: str) -> Comment: ) return get_resource(request, query) + + +@use_kwargs( + CommentSchema(only=('comment_id36',)), + locations=('matchdict',), +) +def notification_by_comment_id36( + request: Request, + comment_id36: str, +) -> CommentNotification: + """Get a comment notification specified by {comment_id36} in the route. + + Looks up a comment notification for the logged-in user with the + {comment_id36} specified in the route. + """ + if not request.user: + raise HTTPForbidden + + comment_id = id36_to_id(comment_id36) + query = request.query(CommentNotification).filter_by( + user=request.user, + comment_id=comment_id, + ) + + return get_resource(request, query) diff --git a/tildes/tildes/routes.py b/tildes/tildes/routes.py index 38b025e..4b5522d 100644 --- a/tildes/tildes/routes.py +++ b/tildes/tildes/routes.py @@ -6,7 +6,10 @@ from pyramid.config import Configurator from pyramid.request import Request from pyramid.security import Allow, Authenticated -from tildes.resources.comment import comment_by_id36 +from tildes.resources.comment import ( + comment_by_id36, + notification_by_comment_id36, +) from tildes.resources.group import group_by_path from tildes.resources.message import message_conversation_by_id36 from tildes.resources.topic import topic_by_id36 @@ -158,7 +161,7 @@ def add_intercooler_routes(config: Configurator) -> None: add_ic_route( 'comment_mark_read', '/comments/{comment_id36}/mark_read', - factory=comment_by_id36, + factory=notification_by_comment_id36, ) add_ic_route( diff --git a/tildes/tildes/templates/notifications_unread.jinja2 b/tildes/tildes/templates/notifications_unread.jinja2 index beab3d8..998ceed 100644 --- a/tildes/tildes/templates/notifications_unread.jinja2 +++ b/tildes/tildes/templates/notifications_unread.jinja2 @@ -5,7 +5,20 @@ {% block title %}Unread notifications{% endblock %} -{% block main_heading %}Unread notifications{% endblock %} +{% block main_heading %}Unread notifications + {% if notifications and not request.user.auto_mark_notifications_read %} + + {% endif %} + +{% endblock %} {% block content %} {% if notifications %} diff --git a/tildes/tildes/views/api/web/comment.py b/tildes/tildes/views/api/web/comment.py index 412ec33..647e52c 100644 --- a/tildes/tildes/views/api/web/comment.py +++ b/tildes/tildes/views/api/web/comment.py @@ -1,9 +1,11 @@ """Web API endpoints related to comments.""" +from marshmallow.fields import Boolean from pyramid.request import Request from pyramid.response import Response from sqlalchemy.dialects.postgresql import insert from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import joinedload from sqlalchemy.orm.exc import FlushError from webargs.pyramidparser import use_kwargs from zope.sqlalchemy import mark_changed @@ -22,6 +24,41 @@ from tildes.views import IC_NOOP from tildes.views.decorators import ic_view_config +def _increment_topic_comments_seen( + request: Request, + comment: Comment, +) -> None: + """Increment the number of comments in a topic the user has viewed. + + If the user has the "track comment visits" feature enabled, we want to + increment the number of comments they've seen in the thread that the + comment came from, so that they don't *both* get a notification as well as + have the thread highlight with "(1 new)". This should only happen if their + last visit was before the comment was posted, however. Below, this is + implemented as a INSERT ... ON CONFLICT DO UPDATE so that it will insert + a new topic visit with 1 comment if they didn't previously have one at + all. + """ + if request.user.track_comment_visits: + statement = ( + insert(TopicVisit.__table__) + .values( + user_id=request.user.user_id, + topic_id=comment.topic_id, + visit_time=utc_now(), + num_comments=1, + ) + .on_conflict_do_update( + constraint=TopicVisit.__table__.primary_key, + set_={'num_comments': TopicVisit.num_comments + 1}, + where=TopicVisit.visit_time < comment.created_time, + ) + ) + + request.db_session.execute(statement) + mark_changed(request.db_session) + + @ic_view_config( route_name='topic_comments', request_method='POST', @@ -287,41 +324,40 @@ def untag_comment(request: Request, name: CommentTagOption) -> Response: request_method='PUT', permission='mark_read', ) -def mark_read_comment(request: Request) -> Response: - """Mark a comment read (clear all notifications).""" - comment = request.context - - request.query(CommentNotification).filter( - CommentNotification.user == request.user, - CommentNotification.comment == comment, - ).update( - {CommentNotification.is_unread: False}, synchronize_session=False) - - # If the user has the "track comment visits" feature enabled, we want to - # increment the number of comments they've seen in the thread that the - # comment came from, so that they don't *both* get a notification as well - # as have the thread highlight with "(1 new)". This should only happen if - # their last visit was before the comment was posted, however. - # Below, this is implemented as a INSERT ... ON CONFLICT DO UPDATE so that - # it will insert a new topic visit with 1 comment if they didn't previously - # have one at all. - if request.user.track_comment_visits: - statement = ( - insert(TopicVisit.__table__) - .values( - user_id=request.user.user_id, - topic_id=comment.topic_id, - visit_time=utc_now(), - num_comments=1, - ) - .on_conflict_do_update( - constraint=TopicVisit.__table__.primary_key, - set_={'num_comments': TopicVisit.num_comments + 1}, - where=TopicVisit.visit_time < comment.created_time, +@use_kwargs({'mark_all_previous': Boolean(missing=False)}) +def put_mark_comments_read( + request: Request, + mark_all_previous: bool, +) -> Response: + """Mark comment(s) read, clearing notifications. + + The "main" notification (request.context) will always be marked read, and + if the query param mark_all_previous is Truthy, all notifications prior to + that one will be marked read as well. + """ + notification = request.context + + if mark_all_previous: + prev_notifications = ( + request.query(CommentNotification).filter( + CommentNotification.user == request.user, + CommentNotification.is_unread == True, # noqa + CommentNotification.created_time <= notification.created_time, ) + .options(joinedload(CommentNotification.comment)) + .all() ) - request.db_session.execute(statement) - mark_changed(request.db_session) + for comment_notification in prev_notifications: + comment_notification.is_unread = False + _increment_topic_comments_seen( + request, + comment_notification.comment + ) + + return Response('Your comment notifications have been cleared.') + + notification.is_unread = False + _increment_topic_comments_seen(request, notification.comment) return IC_NOOP