invisible-college / tawk.space

Social video chats
https://tawk.space
Apache License 2.0
14 stars 1 forks source link

Users should be able to move others around #10

Open karth295 opened 7 years ago

karth295 commented 7 years ago

Currently you can only move yourself in tawk. I can think of a couple reasons to move others:

  1. A new user is confused and they're in their own group where nobody can hear them. An experienced user can help them out by moving them to a group.

  2. Create a side conversation with somebody (one of the original use cases of tawk)

Users shouldn't be able to modify others' /connection objects directly, so we can create a new piece of state: /groupmod.

sb["/groupmod/user_id"] = new_group_id

The server will update the user's connection object, and broadcast the change to everybody.

toomim commented 7 years ago

What is this groupmod? How does it work? My first thought would be create a /groups state, like:

sb["/groups"] = [["<connection1>", "<connection2">, ...], [...]]

karth295 commented 7 years ago

I think /groups makes sense for the fetch -- that state lives on the client but should really be on the server.

On the other hand, it seems sketchy to let users save the entire /groups state directly.

  1. I shouldn't be able to modify other attributes of other people's connection state.
  2. What if two people save different versions of /groups at the same time? Doesn't one modification get dropped? It seems cleaner for the server to decide the ordering and ensure all updates actually happen.

Maybe this is better:

save(/connection/user_id/group, group_id) fetch(/connection/user_id/group) -> group_id

fetch(/groups) -> {group1: [user1, user2], ...} save(/groups) -> Error.

toomim commented 7 years ago

For issue 1: The /groups array wouldn't let you modify each connection object. It would just contain an array of connection IDs. (Actually, statebus should give each connection an ID—a key—automatically. I started writing code to do that, but it's not finished yet.)

For issue 2: Statebus will automatically handle this once we implement the versioning diffsync. But for now, let's look at the clobbered changes in two cases:

  1. We clobber a client adding or removing themselves from the tawk.space entirely
  2. We clobber a client moving from one group to another

Case 2 isn't too bad. People don't move groups very often, so the clobbers will be infrequent, and easy to recover from (just drag again).

Case 1 can be recovered from by checking that each user in fetch('/connections').all is present in exactly one group, and adding/removing them if not. This can happen in a reactive function:

bus(->
  cons = fetch('/connections').all
  groups = fetch('/groups')
  for c in cons
    # check that it exists in a group exactly once
    found = false
    for g in groups
      for i in g
        if i == c
          if found
            # then delete element i from the group g using e.g. array.slice
          found = true
        # Make sure this client is live in /connections.all
        if cons.indexOf(i) == -1
          # then delete element i from the group g using e.g. array.slice
    if not found
      groups[0].push(c)
)

Actually, we could put this function on the server, which would ensure for all clients that the data remains consistent.

But my personal inclination would probably be to just ignore this bug, and use it as motivation for us to finish the versioned diffsync... :P