-
Notifications
You must be signed in to change notification settings - Fork 554
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
Tighten-up experience of blocks by third parties #980
Conversation
@@ -91,6 +91,7 @@ const composeThread = ( | |||
} | |||
|
|||
if (post.author.viewer?.blocking || post.author.viewer?.blockedBy) { | |||
// Globally blocked replies via post.replyBlocked are fully filtered out of results. |
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.
is it possible that we could still return as a #blockedPost
?
seems like this could lead to unexpected behavior 🤔
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.
like say we have 3 users: alice, bob & carol
- bob replies to alice
- carol replies to bob
- then alice blocks bob
what should happen here? I kinda think the thread should show:
- alice's post
- "post by blocked user"
- carol's 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.
i almost wonder if it's worth having two different defs for these like blockedUserPost
& blockNonComplyingPost
(names are kinda longwinded but just to get the point across)
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 think that's a great case to consider and I agree, but I don't think our lexicons support it all that well at the moment, and I don't see a straightforward way to get it in there without some breaking changes. I believe we have the same issue as when a post is deleted too— the #notFoundPost
gets returned rather than a #threadViewPost
containing replies just due to the way the lexicons are setup.
After I realized that and started moving away from that approach, I was thinking about the experience of running into posts that are blocked from view from everyone. In a traditional service folks would not be able to create new replies at all, so no new ones would appear. But in our setting we'll see bad actors publishing replies that get blocked from everyone's view, and my thought process was that we should pretend they weren't allowed to be published in the first place (from the perspective of the appview).
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.
yeah i think the trickery is that:
- posts published after the block occurred are considered invalid
- posts published before the block occurred are still valid
but in order to support that, we have to take into account indexing time when doing this which is something we haven't had to do up till now 🤔
ah idk... this is probably okay for now & if it's leading to weird stuff in the app & confusing people we can come back & fix it up
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 really wanna get in that fix to #threadViewPost, we should coordinate with paul on how we might do it
@@ -42,6 +42,10 @@ export const valuesList = (vals: unknown[]) => { | |||
return sql`(values (${sql.join(vals, sql`), (`)}))` | |||
} | |||
|
|||
export const dbBool = (bool: boolean): 0 | 1 => { |
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.
nice
packages/pds/src/db/migrations/20230504T210229992Z-block-third-party.ts
Outdated
Show resolved
Hide resolved
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.
alright you convinced me not to worry about the replies 😅
lgtm 👍
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.
alright you convinced me not to worry about the replies 😅
lgtm 👍
Replaced by #1378 |
Record embeds are marked as blocked to third party actors when there's a block between the poster and author of the embed. Replies are not surfaced to third party actors when there's a block between the poster and author of the reply.