RocketChat / Rocket.Chat

The communications platform that puts data protection first.
https://rocket.chat/
Other
40.65k stars 10.65k forks source link

Reactions aren't transferred when changing usernames or deleting accounts #20425

Open aKn1ghtOut opened 3 years ago

aKn1ghtOut commented 3 years ago

Description:

Once you change your username or delete the account, the reactions aren't transferred/removed. This can turn out to be a significant flaw for its usage as a polling mechanism or even new accounts having the remnant reactions of an old/changed account.

The reactions are stored in the message objects in a very raw format. If you :heart_eyes: react something, it is stored as "reactions" : { ":heart_eyes:" : { "usernames" : [ username ] }} This makes querying to find reactions by username infeasible. It would be better to migrate to a structure like: "reactions" : [ { "type": ":heart_eyes:", "usernames": [ username ] }] This might take a bit more space in the document but allows manipulation of the reactions by usernames.

Steps to reproduce:

Expected behavior:

For the reactions to be transferred/deleted.

Actual behavior:

The reactions aren't changed.

Server Setup Information:

Client Setup Information

Additional context

Relevant logs:

aKn1ghtOut commented 3 years ago

A non-breaking approach I also had in mind was to store the ids of all the messages a user reacts to in the user's document. This would keep the documents compatible with the current structure if that is a hard requirement.

KevLehman commented 3 years ago

@aKn1ghtOut yes. I like having the reactions in the message context (because a reaction is a property of a message). And yes, I like the idea of changing the structure we store reactions to use plain objects instead of dynamic keys.

In the current state, querying for messages a user reacted to can be done by using aggregations, but it's impossible to update them in a single query, we would need 2 queries (one to fetch and one to store the updated messages) and, apart from race conditions, the load the server may experiment during this would be a huge problem.

However, we would need to find a "backwards compatible" way of updating the structure (with a migration script or something) so current reactions don't get lost.

Thanks for bringing up this issue!