Add post_actions.disable_notifications (fixes #3042) #5826
Conversation
| } | ||
|
|
||
| Ok(recipient_ids) | ||
| } |
There was a problem hiding this comment.
Split this spaghetti code into separate functions with only very minor logic changes. You can see the changes by checking the individual commits.
Nutomic
requested review from
SleeplessOne1917,
dessalines,
dullbananas and
phiresky
as code owners
Nutomic
force-pushed
the
disable_reply_notifications_2
branch
from
8be0de6 to
3c6bbe1
Compare
| context, | ||
| ) | ||
| .await | ||
| .is_err(); |
There was a problem hiding this comment.
This check was previously missing so it sent mentions even in communities that were blocked by the user (and for blocked users, instances).
| local_instance_id, | ||
| ) | ||
| .await?; | ||
| let updated_comment_id = updated_comment.id; |
There was a problem hiding this comment.
Seems like this was only called to get recipient_ids, so I changed it to empty vec.
Nutomic
changed the title
Nutomic
force-pushed
the
disable_reply_notifications_2
branch
from
cbe1e0f to
9a0dc85
Compare
|
Question, is the field |
Nutomic
force-pushed
the
disable_reply_notifications_2
branch
from
38a9667 to
5929494
Compare
That might be a relic of websockets needing to update the recipients live, or before we added the |
| ALTER TABLE post_actions | ||
| ADD COLUMN disable_notifications bool DEFAULT FALSE; |
There was a problem hiding this comment.
We also have the case, where either a post creator, or any bystander, might want to be notified about all new comments within a post.
Since these are exclusive, I think we should probably create a notifications enum column, and a PostNotifications type with :
RepliesAndMentions(default)AllCommentsMute
Otherwise, we'd have a disable(or mute)_notifications bool, and a follow_all_comments bool, but those could conflict.
| use lemmy_db_views_site::api::SuccessResponse; | ||
| use lemmy_utils::error::LemmyResult; | ||
|
|
||
| pub async fn disable_post_notifications( |
There was a problem hiding this comment.
Should probably be changed to update_post_notifications , since there should be several options.
|
Removed recipient_ids from api and changed post_actions field to enum. Will add the logic for post following in #5604. So this PR can be merged already. |
|
Gonna merge this, use #5604 for any additional feedback. |
3 participants
Replaces #5602 by adding
post_actions.disable_notificationswhich can be set via/api/v4/post/disable_notifications. If this is true the user wont be notified about any replies or mentions in the post. Also cleaned up the logic forsend_local_notifs().