Delete notifications when deleting comments #499

已合併
vbrandl merged 6 commit from fix/delete-notifications into master 2019-03-26 11:45:17 +00:00
vbrandl 評論 2019-03-23 15:11:50 +00:00 (Migrated from github.com)

This PR implements a new function find_for_comment on Notification to list all notifications for a specific comment. The delete function on Comment is changed to list and delete all connected notifications.

I wasn't able to test the changes yet since I don't have the test environment set up, so the PR is WIP for now.

This should close #463 and close #500.

This PR implements a new function `find_for_comment` on `Notification` to list all notifications for a specific comment. The `delete` function on `Comment` is changed to list and delete all connected notifications. I wasn't able to test the changes yet since I don't have the test environment set up, so the PR is `WIP` for now. This should close #463 and close #500.
codecov[bot] 評論 2019-03-23 15:41:28 +00:00 (Migrated from github.com)

Codecov Report

Merging #499 into master will decrease coverage by 0.05%.
The diff coverage is 19.04%.

@@            Coverage Diff             @@
##           master     #499      +/-   ##
==========================================
- Coverage   26.89%   26.83%   -0.06%     
==========================================
  Files          65       65              
  Lines        8966     9000      +34     
==========================================
+ Hits         2411     2415       +4     
- Misses       6555     6585      +30
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/499?src=pr&el=h1) Report > Merging [#499](https://codecov.io/gh/Plume-org/Plume/pull/499?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/bdfad844d7939e757929d1643132a9346f82dd66?src=pr&el=desc) will **decrease** coverage by `0.05%`. > The diff coverage is `19.04%`. ```diff @@ Coverage Diff @@ ## master #499 +/- ## ========================================== - Coverage 26.89% 26.83% -0.06% ========================================== Files 65 65 Lines 8966 9000 +34 ========================================== + Hits 2411 2415 +4 - Misses 6555 6585 +30 ```
vbrandl 評論 2019-03-25 20:33:12 +00:00 (Migrated from github.com)

The following changes have been made:

  • Mentions inside comments are now properly inserted into the database (comment_id is set, instead of post_id)
  • Notifications for mentions are deleted, when the comment gets deleted
  • Notifications for comments enabled

The following use cases were tested:

  • Comment on a post, check for notification and delete comment again -> notification gets deleted too
  • Mention one or multiple user(s) in a comment, check notifications for all users and delete comment again -> notifications for all users are deleted
The following changes have been made: * Mentions inside comments are now properly inserted into the database (`comment_id` is set, instead of `post_id`) * Notifications for mentions are deleted, when the comment gets deleted * Notifications for comments enabled The following use cases were tested: * Comment on a post, check for notification and delete comment again -> notification gets deleted too * Mention one or multiple user(s) in a comment, check notifications for all users and delete comment again -> notifications for all users are deleted

Mentions inside comments are now properly inserted into the database (comment_id is set, instead of post_id)

Does this mean it fixes #500 too?

> Mentions inside comments are now properly inserted into the database (comment_id is set, instead of post_id) Does this mean it fixes #500 too?
vbrandl 評論 2019-03-25 20:43:25 +00:00 (Migrated from github.com)

No. That's another issue. In that issue, mentioning and mentioned user are confused somewhere, I guess.

No. That's another issue. In that issue, mentioning and mentioned user are confused somewhere, I guess.
vbrandl 評論 2019-03-25 23:11:01 +00:00 (Migrated from github.com)

It actually looks like this also closes #500.

It actually looks like this also closes #500.
elegaanz (Migrated from github.com) approved these changes 2019-03-26 11:44:47 +00:00
elegaanz (Migrated from github.com) left a comment

It is a bit weird to receive two notifications for the same comment when you are mentioned in a comment under your own article, but it is not really related to this PR, and otherwise it works great. Thank you. 😊

It is a bit weird to receive two notifications for the same comment when you are mentioned in a comment under your own article, but it is not really related to this PR, and otherwise it works great. Thank you. :blush:
登入 才能加入這對話。
No reviewers
未選擇里程碑
No project
No assignees
2 participant
訊息
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Plume/Plume#499
No description provided.