-
Notifications
You must be signed in to change notification settings - Fork 13.1k
[FIX] Show direct rooms as readonly when one of the users is deactivated #21684
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
[FIX] Show direct rooms as readonly when one of the users is deactivated #21684
Conversation
|
|
||
| setReadOnlyByUserId(_id, readOnly, reactWhenReadOnly) { | ||
| const query = { | ||
| uids: _id, |
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.
this will probably make event multiple DM rooms as read only.. which I'm not sure is the desired 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.
Well, it should mark all the direct conversations with the deactivated user as readonly 🤔
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.
right, is that behavior expected? like if I'm talking to 5 different people, if one of them gets deactivated, should the whole conversation be locked up? this might be already better than what we have today though =)
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.
oh, and I just realised, what if a user get re-activated? this update will make room available again even if there are other users still not active on that multi DM..
let's say there is a multi DM with users: userA, userB and userC.. if userA is deactivated, it will change that DM to read only.. then after that userB get deactivated as well, the room stays read only.. if userA is activated again, the DM should keep as read only, since userB is still not active.
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 we can refine to the point where if a conversation is between 2 and one is locked, the whole conversation should be locked. But if it's between more people, then it shouldn't be locked away, even if there are deactivated members,
Regarding the second, yes 😬 I didn't consider multi dms for this haha I thought that was a different type of room.
Co-authored-by: Diego Sampaio <[email protected]>
| throw new Error('You_have_been_muted'); | ||
| } | ||
|
|
||
| console.error(room); |
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.
oops
| if (room.t === 'd') { | ||
| const recipientId = room.uids.filter((id) => id !== uid)[0]; | ||
| const recipient = await Users.findOneById(recipientId, { projection: { active: 1 } }); | ||
| console.error({ recipientId, recipient }); |
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.
oops too :P
|
|
||
| setReadOnlyByUserId(_id, readOnly, reactWhenReadOnly) { | ||
| const query = { | ||
| uids: _id, |
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.
oh, and I just realised, what if a user get re-activated? this update will make room available again even if there are other users still not active on that multi DM..
let's say there is a multi DM with users: userA, userB and userC.. if userA is deactivated, it will change that DM to read only.. then after that userB get deactivated as well, the room stays read only.. if userA is activated again, the DM should keep as read only, since userB is still not active.
| } | ||
|
|
||
| console.error(room); | ||
| if (room.t === 'd') { |
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.
Actually, I think this is necessary 'cause admins can send messages to read only rooms. And that will mean that they can send messages to deactivated users.
If that's valid, then the validation is not needed.
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments