Fixing bug where comment replies wouldn't be sent to blocked instances. #4595
Conversation
dessalines
commented
Apr 4, 2024
dessalines
commented
- Instance blocks should only affect communities, not comments.
- Fixes [Bug]: Notifications not coming through from users on a (user) blocked instance #4590
- Instance blocks should only affect communities, not comments. - Fixes #4590
dessalines
requested review from
Nutomic,
phiresky and
SleeplessOne1917
as code owners
crates/api_common/src/utils.rs
Outdated
| #[tracing::instrument(skip_all)] | ||
| async fn check_instance_block( |
There was a problem hiding this comment.
This was only run from the send_local_notifs function, which creates comment_replies, mentions, and emails, none of which need to be blocked in an InstanceBlock.
| person.id, | ||
| parent_creator_id, | ||
| person.instance_id, |
There was a problem hiding this comment.
Maybe this should instead be fixed by passing community.instance_id, so that all communities from that instance are ignored but not the users? Seems like that would be consistent with the way instance blocking works in general.
There was a problem hiding this comment.
I spent a few minutes thinking about it, and I think you're right. Its the community instance_id that should be blocked.
…instances." This reverts commit 1349aa3.
- Also refactor send_local_notifs slightly, since it has to fetch the community now. - Fixes #4590
| @@ -91,16 +90,19 @@ pub async fn build_post_response( | |||
| #[tracing::instrument(skip_all)] | |||
| pub async fn send_local_notifs( | |||
| mentions: Vec<MentionData>, | |||
| comment: &Comment, | |||
| comment_id: CommentId, | |||
There was a problem hiding this comment.
Rather than pre-fetch and provide the comment, post, and community, I realized it'd be faster to just have the comment_id as a param here, and below do a single CommentView fetch to get all the necessary info.
