Blocking someone should unfollow them

[mozzius]

Describe the bug

Title is pretty much it: I think expected behaviour on block should be to unfollow them.

Users might block someone to distance themselves from a bad actor, yet to an outside observer they’re still following them

To Reproduce

Steps to reproduce the behavior:

1. Follow someone
2. Block them
3. Unblock them and observe you’re still following them

Expected behavior

I expect them to be unfollowed when I block them

Details

• Operating system: n/a
• Node version: n/a

Additional context

I understand due to the protocol design currently stopping someone from following you is not possible, however removing your own follows should be very doable

[XaurDesu]

I understand due to the protocol design currently stopping someone from following you is not possible, however removing your own follows should be very doable

this, if i understand the codebase correctly (i'm pretty new here, still reading and getting familiar with this codebase), this is implemented (at least for the purpose of tests, as it's written on atproto/packages/pds/tests/seeds/client.ts, keeping in mind i've found this exact same function in a few other places too) as:

  async unfollow(from: string, to: string) {
    const follow = this.follows[from][to]
    if (!follow) {
      throw new Error('follow does not exist')
    }
    await this.agent.api.app.bsky.graph.follow.delete(
      { repo: from, rkey: follow.uri.rkey },
      this.getHeaders(from),
    )
    delete this.follows[from][to]
  }

On this same file, we can find the implementation of block right below unfollow as:

  async block(from: string, to: string, overrides?: Partial<FollowRecord>) {
    const res = await this.agent.api.app.bsky.graph.block.create(
      { repo: from },
      {
        subject: to,
        createdAt: new Date().toISOString(),
        ...overrides,
      },
      this.getHeaders(from),
    )
    this.blocks[from] ??= {}
    this.blocks[from][to] = new RecordRef(res.uri, res.cid)
    return this.blocks[from][to]
  }

If we assume that from and to from these two functions refer to the same user identifiers, a naive implementation of this would be simply invoking unfollow() from block() in the same way it is referenced on the tests that invoke this file, as follows:

  async blockWhenFollowing(from: string, to: string, overrides?: Partial<FollowRecord>) {
    const res = await this.agent.api.app.bsky.graph.block.create(
      { repo: from },
      {
        subject: to,
        createdAt: new Date().toISOString(),
        ...overrides,
      },
      this.getHeaders(from),
    )
    this.blocks[from] ??= {}
    this.blocks[from][to] = new RecordRef(res.uri, res.cid)
    //we use a try-catch due to unfollow throwing an error when there's no such a follow.
    try {
      await unfollow(from, to)
    } catch(err){
      handleError(err)
    }
    return this.blocks[from][to]
  }

This however, has a few things that worry me. As first of all, these files are mostly from tests, and i'm not sure on how would they differ from the code used in production. On this same line, even if the production functions are exactly the same, both unfollow() from block() are async functions, and i'm scared a nested asyncronous call on such an implementation could give us problems in the future. Is there any way to circumvent this? (Maybe calling both functions from a separate, third function? I'd imagine this could be a good idea if we redirected an user that was following another one at the time of blocking to that one, maybe we can do a check somewhere and send both requests in case a user were to block another one while following them) Besides this as i mentioned, i found these implementations in multiple places at the same time. I'd imagine we should rewrite them everywhere if either unfollow() or block() is to be changed. I'd like to look into this but as i said, i'm pretty new to the codebase. Is there are any places we could check to make a more informed proposal?

[mozzius]

@XaurDesu you want to be looking in the bluesky-social/social-app repo. These are just the tests.

However, the problem is that there's no createBlock endpoint, the client just does this:

async blockAccount() {
    const res = await this.rootStore.agent.app.bsky.graph.block.create(
      {
        repo: this.rootStore.me.did,
      },
      {
        subject: this.did,
        createdAt: new Date().toISOString(),
      },
    )
    this.viewer.blocking = res.uri
    await this.refresh()
  }

Which just directly creates a block record. There's no obvious place to add this feature on the server side to my knowledge

[XaurDesu]

@mozzius would this be a pertinent issue to be leveraged in bluesky-social/social-app? We could probably link to this issue anyhow.

[mozzius]

@mozzius would this be a pertinent issue to be leveraged in bluesky-social/social-app? We could probably link to this issue anyhow.

You can if you'd like, however I maintain that it would be best to do it on the server side

[mozzius]

Paul said that the logic should not be in the PDS: bluesky-social/social-app#1344