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.
This commit is contained in:
TLSM 2023-07-12 00:11:24 -04:00 committed by Ben Rog-Wilhelm
parent 695e6b6dbd
commit 6fde14a92c
4 changed files with 19 additions and 46 deletions

View file

@ -359,7 +359,7 @@ class Comment(CreatedBase):
return self.is_message and not self.sentto return self.is_message and not self.sentto
@lazy @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 Returns a message that is in the header for a comment, usually for
display on a notification page. display on a notification page.
@ -367,9 +367,9 @@ class Comment(CreatedBase):
if self.post: if self.post:
post_html:str = f"<a href=\"{self.post.permalink}\">{self.post.realtitle(v)}</a>" post_html:str = f"<a href=\"{self.post.permalink}\">{self.post.realtitle(v)}</a>"
if v: if v:
if v.id == self.author_id and reply_count: if self.level > 1 and v.id == self.parent_comment.author_id:
text = f"Comment {'Replies' if reply_count != 1 else 'Reply'}" text = "Comment Reply"
elif v.id == self.post.author_id and self.level == 1: elif self.level == 1 and v.id == self.post.author_id:
text = "Post Reply" text = "Post Reply"
elif self.parent_submission in v.subscribed_idlist(): elif self.parent_submission in v.subscribed_idlist():
text = "Subscribed Thread" text = "Subscribed Thread"

View file

@ -62,47 +62,17 @@ def notifications_main(v: User):
next_exists = (len(comments) > 25) next_exists = (len(comments) > 25)
comments = 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: for c, n in comments:
if n.created_utc > 1620391248: c.notif_utc = n.created_utc c.notif_utc = n.created_utc
if not n.read: c.unread = not n.read
n.read = True n.read = True
c.unread = True
g.db.add(n)
if c.parent_submission: listing: list[Comment] = [c for c, _ in comments]
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)
# TODO: commit after request rendered, then default session expiry is fine
g.db.expire_on_commit = False
g.db.commit() g.db.commit()
g.db.expire_on_commit = True
if request.headers.get("Authorization"): if request.headers.get("Authorization"):
return {"data": [x.json for x in listing]} return {"data": [x.json for x in listing]}
@ -113,7 +83,8 @@ def notifications_main(v: User):
next_exists=next_exists, next_exists=next_exists,
page=page, page=page,
standalone=True, standalone=True,
render_replies=True, render_replies=False,
is_notification_page=True,
) )
@ -153,6 +124,7 @@ def notifications_posts(v: User):
page=page, page=page,
standalone=True, standalone=True,
render_replies=True, render_replies=True,
is_notification_page=True,
) )
@ -177,6 +149,7 @@ def notifications_modmail(v: User):
page=page, page=page,
standalone=True, standalone=True,
render_replies=True, render_replies=True,
is_notification_page=True,
) )
@ -210,6 +183,7 @@ def notifications_messages(v: User):
page=page, page=page,
standalone=True, standalone=True,
render_replies=True, render_replies=True,
is_notification_page=True,
) )

View file

@ -10,7 +10,6 @@
{%- set score = c.score_str(render_ctx) -%} {%- set score = c.score_str(render_ctx) -%}
{%- set downs = c.downvotes_str(render_ctx) -%} {%- set downs = c.downvotes_str(render_ctx) -%}
{% set replies = c.replies(v) %} {% set replies = c.replies(v) %}
{%- set is_notification_page = request.path.startswith('/notifications') -%}
{% if not c.visibility_state(v)[0] %} {% if not c.visibility_state(v)[0] %}
{% if c.show_descendants(v) %} {% if c.show_descendants(v) %}
@ -61,10 +60,10 @@
{%- set voted = c.voted_display(v) -%} {%- set voted = c.voted_display(v) -%}
{% if standalone and level==1 %} {% if standalone and level==1 %}
<div class="post-info post-row-cid-{{c.id}} mb-1 mr-2 {% if is_notification_page %}mt-5{% else %}mt-3{% endif %}"> <div class="post-info post-row-cid-{{c.id}} mb-1 mr-2 mt-3">
{% if c.post and c.post.over_18 %}<span class="badge badge-danger text-small-extra mr-1">+18</span>{% endif %} {% if c.post and c.post.over_18 %}<span class="badge badge-danger text-small-extra mr-1">+18</span>{% endif %}
<span class="align-top"> <span class="align-top">
<span class="font-weight-bold">{{c.header_msg(v, is_notification_page, replies | length) | safe}}</span> <span class="font-weight-bold">{{c.header_msg(v, is_notification_page) | safe}}</span>
</span> </span>
</div> </div>
{% endif %} {% endif %}

View file

@ -40,7 +40,7 @@
</div> </div>
<a class="btn btn-primary mt-3 ml-3" role="button" onclick="post_toast(this,'/clear', '1')">Clear all notifications</a> <a class="btn btn-primary mt-3 ml-3" role="button" onclick="post_toast(this, '/clear', '1')">Mark All Read</a>
<div class="notifs px-3 p-md-0"> <div class="notifs px-3 p-md-0">