Fix #132: Missing notification when another user comments on a post the user commented on #133

Open

typosaurus wants to merge 1 commits from typosaurus/ticket-132 into master

typosaurus commented

2026-07-24 16:03:45 +02:00

Collaborator

Resolves #132.

Plan

Implementation Plan: Notify Previous Commenters on New Comments

1. Add New Notification Type

File: devplacepy/database/notifications.py

Add "thread_comment" to the NOTIFICATION_TYPES tuple (or list).
Insert it in alphabetical order (or append at the end) after "system". No message template needed here; the message will be constructed at call site.

2. Modify `create_comment_record()` in `devplacepy/content.py`

Location: After existing notification logic (line 361), before the audit event emission (line 373).
Scope: This change must apply to both top-level comments and replies, but only for targets of type "post".

Steps:

After the existing if target_type == "post": block, add a new inner block that fires only when the comment is not a reply (parent_uid is None) or, for consistency, after the reply notification has been sent. A simpler approach: place the new logic after the existing if-else block, guarded by if target_type == "post":.
Query the comments table for distinct user_uid values where:
- target_type = 'post'
- target_uid = post_uid (the post UID, already available as target_uid)
- deleted_at = None
- Exclude the current comment author (user["uid"])
- Exclude the post owner (already notified via "comment" type)
- Exclude the parent comment author, if this is a reply (already notified via "reply" type)
Use a single SQL query with DISTINCT or fetch all rows and deduplicate in Python.

For each qualifying user_uid, call:

create_notification(
    user_uid,
    "thread_comment",
    f"{user['username']} also commented on a post you commented on",
    user["uid"],
    comment_url,
)

Ensure no duplicate notifications are created if a user qualifies for both a reply and this new notification (already handled by exclusions).

Edge cases:

Post has no previous commenters → no new notifications sent.
Previous commenter is also the post author → already excluded.
Comment author is also a previous commenter → excluded.

3. Update E2E Test Coverage

File: tests/e2e/notifications/index.py

Add a new test function, e.g. test_thread_comment_notification, that:

Registers three users: post_owner, first_commenter, second_commenter.
post_owner creates a post.
first_commenter adds a top-level comment on that post.
second_commenter adds another top-level comment on the same post.
Asserts that first_commenter receives exactly one notification of type "thread_comment" (message should contain "also commented").
Asserts that post_owner receives exactly one notification of type "comment" (unchanged).
Optionally verify that second_commenter receives no self-notification.

Place this test after existing comment notification tests (around line 381). Use the same Playwright fixtures and helpers as the surrounding tests to avoid introducing new dependencies.

4. Run Verification

Execute the project’s test command:

pip install -e '.[dev]' -q && python -m pytest tests/e2e/notifications/index.py -x

Initially target the single file to validate the new test. Then run the full suite:

pip install -e '.[dev]' -q && python -m pytest

to confirm no regressions.

Definition of Done

"thread_comment" is added to NOTIFICATION_TYPES in devplacepy/database/notifications.py.
In devplacepy/content.py, create_comment_record() contains new logic that queries the comments table for prior commenters on the same post (excluding the comment author, post owner, and parent commenter if replying) and calls create_notification() for each with type "thread_comment".
A new e2e test test_thread_comment_notification exists in tests/e2e/notifications/index.py that verifies a previous commenter receives a notification when another user comments on the same post.
The project test command (pip install -e '.[dev]' -q && python -m pytest) passes with zero errors.
All existing e2e tests for comments and notifications (including Playwright-based tests) continue to pass with no new failures.

Opened automatically by Typosaurus.

Resolves #132. ## Plan ## Implementation Plan: Notify Previous Commenters on New Comments ### 1. Add New Notification Type **File:** `devplacepy/database/notifications.py` - Add `"thread_comment"` to the `NOTIFICATION_TYPES` tuple (or list). - Insert it in alphabetical order (or append at the end) after `"system"`. No message template needed here; the message will be constructed at call site. ### 2. Modify `create_comment_record()` in `devplacepy/content.py` **Location:** After existing notification logic (line 361), before the audit event emission (line 373). **Scope:** This change must apply to both top-level comments and replies, but only for targets of type `"post"`. **Steps:** 1. After the existing `if target_type == "post":` block, add a new inner block that fires only when the comment is not a reply (`parent_uid is None`) or, for consistency, after the reply notification has been sent. A simpler approach: place the new logic **after** the existing `if-else` block, guarded by `if target_type == "post":`. 2. Query the `comments` table for distinct `user_uid` values where: - `target_type = 'post'` - `target_uid = post_uid` (the post UID, already available as `target_uid`) - `deleted_at = None` - **Exclude** the current comment author (`user["uid"]`) - **Exclude** the post owner (already notified via `"comment"` type) - **Exclude** the parent comment author, if this is a reply (already notified via `"reply"` type) Use a single SQL query with `DISTINCT` or fetch all rows and deduplicate in Python. 3. For each qualifying `user_uid`, call: ```python create_notification( user_uid, "thread_comment", f"{user['username']} also commented on a post you commented on", user["uid"], comment_url, ) ``` 4. Ensure no duplicate notifications are created if a user qualifies for both a reply and this new notification (already handled by exclusions). **Edge cases:** - Post has no previous commenters → no new notifications sent. - Previous commenter is also the post author → already excluded. - Comment author is also a previous commenter → excluded. ### 3. Update E2E Test Coverage **File:** `tests/e2e/notifications/index.py` Add a new test function, e.g. `test_thread_comment_notification`, that: 1. Registers three users: `post_owner`, `first_commenter`, `second_commenter`. 2. `post_owner` creates a post. 3. `first_commenter` adds a top-level comment on that post. 4. `second_commenter` adds another top-level comment on the same post. 5. Asserts that `first_commenter` receives exactly one notification of type `"thread_comment"` (message should contain `"also commented"`). 6. Asserts that `post_owner` receives exactly one notification of type `"comment"` (unchanged). 7. Optionally verify that `second_commenter` receives no self-notification. Place this test after existing comment notification tests (around line 381). Use the same Playwright fixtures and helpers as the surrounding tests to avoid introducing new dependencies. ### 4. Run Verification Execute the project’s test command: ```bash pip install -e '.[dev]' -q && python -m pytest tests/e2e/notifications/index.py -x ``` Initially target the single file to validate the new test. Then run the full suite: ```bash pip install -e '.[dev]' -q && python -m pytest ``` to confirm no regressions. --- ## Definition of Done - [ ] `"thread_comment"` is added to `NOTIFICATION_TYPES` in `devplacepy/database/notifications.py`. - [ ] In `devplacepy/content.py`, `create_comment_record()` contains new logic that queries the `comments` table for prior commenters on the same post (excluding the comment author, post owner, and parent commenter if replying) and calls `create_notification()` for each with type `"thread_comment"`. - [ ] A new e2e test `test_thread_comment_notification` exists in `tests/e2e/notifications/index.py` that verifies a previous commenter receives a notification when another user comments on the same post. - [ ] The project test command (`pip install -e '.[dev]' -q && python -m pytest`) passes with zero errors. - [ ] All existing e2e tests for comments and notifications (including Playwright-based tests) continue to pass with no new failures. Opened automatically by Typosaurus.

typosaurus added 1 commit 2026-07-24 16:03:46 +02:00