From 6fde14a92c45997806f2990f439849cbe2c72dde Mon Sep 17 00:00:00 2001 From: TLSM Date: Wed, 12 Jul 2023 00:11:24 -0400 Subject: [PATCH] Use reddit-style notifs without context The notifications schema is already set up for reddit-style chronological notifications. We simply have to remove the logic that builds the context and patch up a few places in the frontend that were expecting full reply trees. `Comment.header_msg` previously expected the user's own comment to be top-level in comment replies. Logic is revised to expect the actual reply. `files.routes.front.notifications_main` now has reduced query volume because we aren't expiring the session when marking notifications read. This also allows us to remove the unused `comms` variable, which is a makeshift "pattern" of storing duplicate database replies across a commit so the templates don't requery (thus losing data attached to the runtime object, like `c.unread` / `c.notif_utc`). We move the `is_notification_page` flag to the route callers rather than templates checking `request.path`. Minor UI style: "Clear all notifications" -> "Mark All Read", since this was a persistent point of user confusion upstream, with people expecting their inbox to empty out. Also less margin between notifs to be consistent with tighter comment display elsewhere and removed need to separate groups of comments vs single comments. --- files/classes/comment.py | 8 ++--- files/routes/front.py | 50 +++++++----------------------- files/templates/comments.html | 5 ++- files/templates/notifications.html | 2 +- 4 files changed, 19 insertions(+), 46 deletions(-) diff --git a/files/classes/comment.py b/files/classes/comment.py index bd0c945a4..ab1ee92d0 100644 --- a/files/classes/comment.py +++ b/files/classes/comment.py @@ -359,7 +359,7 @@ class Comment(CreatedBase): return self.is_message and not self.sentto @lazy - def header_msg(self, v, is_notification_page:bool, reply_count:int) -> str: + def header_msg(self, v, is_notification_page: bool) -> str: ''' Returns a message that is in the header for a comment, usually for display on a notification page. @@ -367,9 +367,9 @@ class Comment(CreatedBase): if self.post: post_html:str = f"{self.post.realtitle(v)}" if v: - if v.id == self.author_id and reply_count: - text = f"Comment {'Replies' if reply_count != 1 else 'Reply'}" - elif v.id == self.post.author_id and self.level == 1: + if self.level > 1 and v.id == self.parent_comment.author_id: + text = "Comment Reply" + elif self.level == 1 and v.id == self.post.author_id: text = "Post Reply" elif self.parent_submission in v.subscribed_idlist(): text = "Subscribed Thread" diff --git a/files/routes/front.py b/files/routes/front.py index f2560b9ea..ce488f9af 100644 --- a/files/routes/front.py +++ b/files/routes/front.py @@ -62,47 +62,17 @@ def notifications_main(v: User): next_exists = (len(comments) > 25) comments = comments[:25] - cids = [x[0].id for x in comments] - - comms = get_comments(cids, v=v) - - listing = [] for c, n in comments: - if n.created_utc > 1620391248: c.notif_utc = n.created_utc - if not n.read: - n.read = True - c.unread = True - g.db.add(n) + c.notif_utc = n.created_utc + c.unread = not n.read + n.read = True - if c.parent_submission: - if c.replies2 == None: - c.replies2 = (c.child_comments - .filter(or_(Comment.author_id == v.id, Comment.id.in_(cids))) - .order_by(Comment.created_utc.desc()).all()) - for x in c.replies2: - if x.replies2 == None: x.replies2 = [] - count = 0 - while (count < 50 and c.parent_comment - and (c.parent_comment.author_id == v.id or c.parent_comment.id in cids)): - count += 1 - c = c.parent_comment - if c.replies2 == None: - c.replies2 = (c.child_comments - .filter(or_(Comment.author_id == v.id, Comment.id.in_(cids))) - .order_by(Comment.created_utc.desc()).all()) - for x in c.replies2: - if x.replies2 == None: - x.replies2 = (x.child_comments - .filter(or_(Comment.author_id == v.id, Comment.id.in_(cids))) - .order_by(Comment.created_utc.desc()).all()) - else: - while c.parent_comment: - c = c.parent_comment - c.replies2 = c.child_comments.order_by(Comment.id).all() - - if c not in listing: listing.append(c) + listing: list[Comment] = [c for c, _ in comments] + # TODO: commit after request rendered, then default session expiry is fine + g.db.expire_on_commit = False g.db.commit() + g.db.expire_on_commit = True if request.headers.get("Authorization"): return {"data": [x.json for x in listing]} @@ -113,7 +83,8 @@ def notifications_main(v: User): next_exists=next_exists, page=page, standalone=True, - render_replies=True, + render_replies=False, + is_notification_page=True, ) @@ -153,6 +124,7 @@ def notifications_posts(v: User): page=page, standalone=True, render_replies=True, + is_notification_page=True, ) @@ -177,6 +149,7 @@ def notifications_modmail(v: User): page=page, standalone=True, render_replies=True, + is_notification_page=True, ) @@ -210,6 +183,7 @@ def notifications_messages(v: User): page=page, standalone=True, render_replies=True, + is_notification_page=True, ) diff --git a/files/templates/comments.html b/files/templates/comments.html index 361ba21e1..fb34992bc 100644 --- a/files/templates/comments.html +++ b/files/templates/comments.html @@ -10,7 +10,6 @@ {%- set score = c.score_str(render_ctx) -%} {%- set downs = c.downvotes_str(render_ctx) -%} {% set replies = c.replies(v) %} - {%- set is_notification_page = request.path.startswith('/notifications') -%} {% if not c.visibility_state(v)[0] %} {% if c.show_descendants(v) %} @@ -61,10 +60,10 @@ {%- set voted = c.voted_display(v) -%} {% if standalone and level==1 %} -
+
{% if c.post and c.post.over_18 %}+18{% endif %} - {{c.header_msg(v, is_notification_page, replies | length) | safe}} + {{c.header_msg(v, is_notification_page) | safe}}
{% endif %} diff --git a/files/templates/notifications.html b/files/templates/notifications.html index 947795b91..2ea5df8cf 100644 --- a/files/templates/notifications.html +++ b/files/templates/notifications.html @@ -40,7 +40,7 @@
-Clear all notifications +Mark All Read