WIP: feat: Missing notification when another user comments on a post the user commented on #142

Draft
typosaurus wants to merge 5 commits from typosaurus/132-missing-notification-when-another-user-comments-on-a-post-th into master
6 changed files with 338 additions and 5 deletions

View File

@ -362,6 +362,29 @@ def create_comment_record(
comment_url,
)
# Notify previous commenters on this post (participation)
posts = get_table("posts")
post = posts.find_one(uid=target_uid)
if not post:
post = posts.find_one(slug=target_uid)
if post:
post_owner_uid = post["user_uid"]
previous_commenters = set()
for c in get_table("comments").find(
target_type="post", target_uid=target_uid, deleted_at=None
):
cu = c["user_uid"]
if cu != user["uid"] and cu != post_owner_uid:
previous_commenters.add(cu)
for cu in previous_commenters:
create_notification(
cu,
"participation",
f"{user['username']} also commented on this post",
user["uid"],
comment_url,
)
create_mention_notifications(content, user["uid"], comment_url)
schedule_correction(user, "comments", comment_uid, request)
schedule_modification(user, "comments", comment_uid, request)

View File

@ -12,6 +12,7 @@ NOTIFICATION_TYPES = [
{"key": "vote", "label": "Upvotes", "description": "Someone ++'d your content"},
{"key": "follow", "label": "Followers", "description": "Someone starts following you"},
{"key": "message", "label": "Direct messages", "description": "Someone sends you a message"},
{"key": "participation", "label": "Post participation", "description": "Someone else comments on a post you also commented on"},
{"key": "badge", "label": "Badges", "description": "You earn a badge"},
{"key": "level", "label": "Level-ups", "description": "You reach a new level"},
{"key": "issue", "label": "Issue tracker", "description": "Updates on issue reports you filed"},
@ -199,3 +200,4 @@ def mark_notifications_read_by_target(user_uid: str, target_url: str) -> int:
clear_unread_cache(user_uid)
return len(ids)

View File

@ -428,7 +428,7 @@ four ways to sign requests.
method="POST",
path="/profile/{username}/notifications",
title="Toggle a notification preference",
summary="Enable or disable one notification type on one channel (in-app or push). Admins may target any user. Types: comment, reply, mention, vote, follow, message, badge, level, issue, reminder, harvest_stolen.",
summary="Enable or disable one notification type on one channel (in-app or push). Admins may target any user. Types: comment, reply, mention, vote, follow, message, participation, badge, level, issue, reminder, harvest_stolen.",
auth="user",
encoding="form",
destructive=True,
@ -447,7 +447,7 @@ four ways to sign requests.
"string",
True,
"vote",
"One of: comment, reply, mention, vote, follow, message, badge, level, issue, reminder, harvest_stolen.",
"One of: comment, reply, mention, vote, follow, message, participation, badge, level, issue, reminder, harvest_stolen.",
),
field(
"channel",
@ -795,5 +795,5 @@ four ways to sign requests.
],
),
],
}
}

View File

@ -3,8 +3,9 @@
# Notification settings
DevPlace notifies you when something involves you: a comment on your post, a reply to your comment, a
mention, an upvote on your work, a new follower, a direct message, a badge, a level-up, or an update
on an issue you filed, or someone gives you an award on your profile. You decide which reach you, and how.
mention, an upvote on your work, a new follower, a direct message, another user comments on a post you
also commented on, a badge, a level-up, or an update on an issue you filed, or someone gives you an
award on your profile. You decide which reach you, and how.
Each notification type is delivered on three independent channels:
@ -40,6 +41,7 @@ immediately - there is no separate save button.
| Upvotes | someone `++`'d your post, comment, project, or gist |
| Followers | someone starts following you |
| Direct messages | someone sends you a message |
| Post participation | someone else comments on a post you also commented on |
| Badges | you earn a badge |
| Level-ups | you reach a new level |
| Issue tracker | there is an update on an issue report you filed |
@ -82,3 +84,5 @@ you to confirm a reset first, since that clears all of your choices).
<a href="/admin/notifications" class="sidebar-link">Notification defaults (admin)</a>
{% endif %}
</div>

View File

@ -0,0 +1,263 @@
# retoor <retoor@molodetz.nl>
import time
import pytest
import requests
from tests.conftest import BASE_URL
from devplacepy.database import get_table, refresh_snapshot, set_setting
_COUNTER = [0]
@pytest.fixture(scope="module", autouse=True)
def _participation_settings(app_server):
for key, value in {
"rate_limit_per_minute": "1000000",
"rate_limit_window_seconds": "60",
"registration_open": "1",
"maintenance_mode": "0",
"max_upload_size_mb": "10",
"allowed_file_types": "",
"max_attachments_per_resource": "10",
"session_max_age_days": "7",
"session_remember_days": "30",
"news_service_interval": "3600",
"news_grade_threshold": "7",
}.items():
set_setting(key, value)
yield
def _db_user(username):
refresh_snapshot()
return get_table("users").find_one(username=username)
def _unique(prefix="pn"):
_COUNTER[0] += 1
return f"{prefix}{int(time.time() * 1000)}{_COUNTER[0]}"
def _signup():
name = _unique("pnuser")
s = requests.Session()
s.post(
f"{BASE_URL}/auth/signup",
data={
"username": name,
"email": f"{name}@t.dev",
"password": "secret123",
"confirm_password": "secret123",
},
allow_redirects=True,
)
return s, name
def _create_post(session):
r = session.post(
f"{BASE_URL}/posts/create",
data={
"title": _unique("pnpost"),
"content": "Post for participation notification test.",
"topic": "devlog",
},
allow_redirects=False,
)
slug = r.headers["location"].split("/posts/")[-1]
refresh_snapshot()
return get_table("posts").find_one(slug=slug)["uid"]
def _comment_on_post(session, post_uid, content):
r = session.post(
f"{BASE_URL}/comments/create",
data={
"content": content,
"target_type": "post",
"post_uid": post_uid,
"target_uid": post_uid,
},
allow_redirects=False,
)
assert r.status_code in (302, 303), (
f"Comment creation failed: {r.status_code} {r.text[:300]}"
)
def _reply_to_comment(session, post_uid, parent_uid, content):
r = session.post(
f"{BASE_URL}/comments/create",
data={
"content": content,
"target_type": "post",
"post_uid": post_uid,
"target_uid": post_uid,
"parent_uid": parent_uid,
},
allow_redirects=False,
)
assert r.status_code in (302, 303), (
f"Reply creation failed: {r.status_code} {r.text[:300]}"
)
def _find_comment(post_uid, content):
refresh_snapshot()
return get_table("comments").find_one(target_uid=post_uid, content=content)
def _notifications_for(user_uid):
refresh_snapshot()
return list(
get_table("notifications").find(
user_uid=user_uid, order_by=["-created_at"]
)
)
def _participation_notifications_for(user_uid):
refresh_snapshot()
return list(
get_table("notifications").find(
user_uid=user_uid, type="participation", order_by=["-created_at"]
)
)
def test_participation_notification_sent(app_server):
"""User A receives a participation notification when User B comments
on a post that User A previously commented on."""
owner_session, owner_name = _signup()
post_uid = _create_post(owner_session)
a_session, a_name = _signup()
b_session, b_name = _signup()
_comment_on_post(a_session, post_uid, "User A first comment")
_comment_on_post(b_session, post_uid, "User B comment")
a_user = _db_user(a_name)
assert a_user is not None
participation_notifs = _participation_notifications_for(a_user["uid"])
assert len(participation_notifs) >= 1, (
f"User {a_name} should have at least one participation notification, "
f"got {len(participation_notifs)}"
)
latest = participation_notifs[0]
assert latest["type"] == "participation"
assert b_name in latest["message"], (
f"Expected notification message to contain {b_name!r}, "
f"got {latest['message']!r}"
)
assert "also commented" in latest["message"], (
f"Expected 'also commented' in message, got {latest['message']!r}"
)
def test_post_owner_no_participation_duplicate(app_server):
"""Post owner does NOT receive a participation notification.
They already receive the 'comment' notification."""
owner_session, owner_name = _signup()
post_uid = _create_post(owner_session)
a_session, a_name = _signup()
b_session, b_name = _signup()
_comment_on_post(a_session, post_uid, "User A comment for owner test")
_comment_on_post(b_session, post_uid, "User B comment for owner test")
owner = _db_user(owner_name)
assert owner is not None
participation_notifs = _participation_notifications_for(owner["uid"])
assert len(participation_notifs) == 0, (
f"Post owner {owner_name} should have zero participation notifications, "
f"got {len(participation_notifs)}: "
f"{[n['message'] for n in participation_notifs]}"
)
all_notifs = _notifications_for(owner["uid"])
comment_notifs = [n for n in all_notifs if n["type"] == "comment"]
assert len(comment_notifs) >= 1, (
f"Post owner should have at least one 'comment' notification, "
f"got {len(comment_notifs)}"
)
def test_commenter_no_self_notification(app_server):
"""Commenter does NOT receive a participation notification
for their own comment."""
owner_session, _ = _signup()
post_uid = _create_post(owner_session)
a_session, a_name = _signup()
_comment_on_post(a_session, post_uid, "User A self-test comment")
b_session, b_name = _signup()
_comment_on_post(b_session, post_uid, "User B self-test comment")
b_user = _db_user(b_name)
assert b_user is not None
participation_notifs = _participation_notifications_for(b_user["uid"])
self_notifs = [
n for n in participation_notifs if b_name in n["message"]
]
assert len(self_notifs) == 0, (
f"User {b_name} should not have a participation notification "
f"about themselves, got {len(self_notifs)}"
)
def test_reply_triggers_participation(app_server):
"""A reply to a comment triggers participation notifications
for other previous commenters (excluding the reply author and post owner).
The implementation sends participation for every comment on a post,
both top-level and replies."""
owner_session, owner_name = _signup()
post_uid = _create_post(owner_session)
a_session, a_name = _signup()
_comment_on_post(a_session, post_uid, "User A top-level comment")
a_comment = _find_comment(post_uid, "User A top-level comment")
c_session, c_name = _signup()
_comment_on_post(c_session, post_uid, "User C third participant comment")
b_session, b_name = _signup()
_reply_to_comment(b_session, post_uid, a_comment["uid"], "User B reply")
a_user = _db_user(a_name)
c_user = _db_user(c_name)
a_participation = _participation_notifications_for(a_user["uid"])
c_participation = _participation_notifications_for(c_user["uid"])
a_has_participation = any(
b_name in n["message"] for n in a_participation
)
c_has_participation = any(
b_name in n["message"] for n in c_participation
)
assert a_has_participation, (
f"User {a_name} (parent commenter) should receive a participation "
f"notification when a reply is posted on the same post. "
f"Notifications: {[n['message'] for n in a_participation]}"
)
assert c_has_participation, (
f"User {c_name} (previous commenter) should receive a participation "
f"notification when a reply is posted on the same post. "
f"Notifications: {[n['message'] for n in c_participation]}"
)
b_user = _db_user(b_name)
b_participation = _participation_notifications_for(b_user["uid"])
b_self = [n for n in b_participation if b_name in n["message"]]
assert len(b_self) == 0, (
f"Reply author {b_name} should not receive a participation "
f"notification about themselves."
)

View File

@ -46,3 +46,44 @@ def test_member_cannot_set_default(bob):
)
assert response.status_code in (302, 303, 403)
assert get_notification_default("level", "push") is True
def test_admin_page_renders_participation_defaults(alice):
_, admin_user = alice
admin = _user_notification_prefs(admin_user["username"])
response = requests.get(
f"{BASE_URL}/admin/notifications",
headers={"X-API-KEY": admin["api_key"]},
allow_redirects=False,
)
assert response.status_code == 200
html = response.text
assert "Post participation" in html, (
"Admin notifications page should contain 'Post participation' label"
)
assert "Someone else comments on a post you also commented on" in html, (
"Admin notifications page should contain participation description"
)
assert 'data-type="participation" data-channel="in_app"' in html, (
"Participation in_app row should exist"
)
assert 'data-type="participation" data-channel="in_app" checked' in html, (
"Participation in_app default should be checked (on)"
)
assert 'data-type="participation" data-channel="push"' in html, (
"Participation push row should exist"
)
assert 'data-type="participation" data-channel="push" checked' in html, (
"Participation push default should be checked (on)"
)
assert 'data-type="participation" data-channel="telegram"' in html, (
"Participation telegram row should exist"
)
assert (
'data-type="participation" data-channel="telegram" checked' not in html
), "Participation telegram default should NOT be checked (off)"