-
-
Notifications
You must be signed in to change notification settings - Fork 924
Add option to disable post, comment reply notifications #5602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@@ -94,6 +94,8 @@ pub struct Post { | |||
/// If a local user posts in a remote community, the comment is hidden until it is confirmed | |||
/// accepted by the community (by receiving it back via federation). | |||
pub federation_pending: bool, | |||
/// True if the post creator gets notified about new top-level comments | |||
pub disable_reply_notifications: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should only be visible to the post creator. Tried to implement it like this:
#[cfg_attr(feature = "full", diesel(
select_expression = select_expr_disable_reply_notifications()))]
#[cfg_attr(feature = "full", ts(optional))]
pub disable_reply_notifications: Option<bool>,
}
#[diesel::dsl::auto_type]
fn select_expr_disable_reply_notifications() -> _ {
person::id
.eq(post::creator_id)
.and(post::disable_reply_notifications)
.nullable()
}
But it throws lots of compilation errors, and anyway it only works if the creator is joined as person
, and not person1
alias (or not joined at all).
dcfeed1
to
7e22dc0
Compare
601ba06
to
b64b957
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be on the post / comment table directly, but should be on post_actions
. Same for comment_actions.
IE disabled_reply_notifications timestamp with time zone
, like the others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think that makes sense because this setting only exists for the author, to disable notifications for direct replies. Having it in the same table also avoids an extra db read.
Im going to make a separate pull request to implement #3069, which will use post_actions
. It will notify about all new comments, not only top level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this essentially be included in that? With the default that you follow your own posts? Seems redundant. Or I guess this is for top-level comments / direct replies only.
But then again, direct replies and all notifs should probably be a part of post_actions
anyway. A user might want to hide direct replies only for a single post.
E post_actions.hide_direct_reply_notifs
and post_actions.follow_all_notifs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes thats the difference, this one is only for direct replies and only for the post creator (because other users dont get notified about them). There will be a separate setting in post_actions
to get notified about all comments in someone elses post.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we moved these both to post_actions
, then not just the post creator, but anyone could do either hide_direct_reply_notifs or follow_all_notifs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other uses dont receive notifications about direct replies on a post they didnt create. So there would be no point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They do receive comment replies on posts they've participated in. Moving this to post_actions
would allow them to block those for a specific post, if they'd like.
IE this could apply not just to the direct replies to the post creator, but to any replies within that post also.
This is a bit similar to how we moved comment_reply
into its own table, rather than just have read
on the comment and post tables themselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this would let the post creator disable replies not just at the top level, but to comments within that post.
Adds an option when creating/editing posts to disable notifications for new replies (similar to Reddit).
The issue also mentions disabling private message notifications but that is handled by #4094. Not sure if it makes sense to disable mentions either.