Add read only channels

[alexbrazier]

@RocketChat/core

Closes #825

Add read only channels and private groups.

• Mute all users when they join a read only channel
• Allow room owner or admin to manually unmute user who has permission to post
• Allow user owner or admin to change existing room between read only (will mute/unmute ALL users in room)
• Added mute icon next to user to quickly check if user is muted (visible in all channels - not just read only) - see screenshot

Currently it doesn't post in the channel when someone joins, or if someone switches channel between read only states, but it does post if an admin adds a user, or manually mutes/unmutes user - any thoughts on the expected behaviour of when to post system message?

Bot tells user they have been muted if they try to post in a read only channel (unless they have been given permission)

• Make read-only use a permission for allowing to post
• Add room controls to suppress notification types

[RichardFoxworthy]

Looks good @alexbrazier - can you make the announcements piece configurable? ie allow an Admin to specify if the event of a new user joining the channel should be posted to channel or not?

Also, can the read-only channel also and simultaneously be a default channel? ie have all new users automatically be enrolled into the read-only channel

[timkinnane]

Hi Alex, this is great work. I'm not part of the core team so I can't speak to the general acceptance, but I've tested your feature branch on my own fork.

We were working on our own solution to this which is slightly different in that we intended to allow settings for room event notifications to be set individually, as well as choosing which roles are exempt to muting. i.e. I noticed admins are muted in read-only rooms and must be appointed to allow posting, personally I think that's undesirable or at least should be configurable.

There's a strange, probably unintended result of not firing the channel join message, in that the channel leave message still fires, but you don't see people rejoin. So over time the channel will appear to be entirely abandoned, even if that's not the case.

Also as you've used rocketbot to send the response to the user when they try posting: "You have been muted and cannot speak in this room". What happens if rocketbot is removed/disabled in an instance? Couldn't that notice be a system alert instead, or potentially just disable the message input field - setting the notice as placeholder text on the input instead:

[ 📎 ][ You have been muted and cannot speak in this room. 😶 ][ ^ ]

Lastly, I noticed what appears to be a pretty critical bug, but its hard to reproduce. When you login and the default room is read-only, if you were previously a member of that room (before this functionality was enabled) everything hangs until to you do a browser reload.

[alexbrazier]

@RichardFoxworthy Do you think it should be a channel setting to make announcements, or a global setting? I don't want to fill up the channel info bar, and channel creation with too many settings to choose from, but if people think it is better per channel, then I can add it.
I will have a look into how users join a default channel, to see if it is already covered by the read only setting, if not I'll add it, but good point, as I didn't think about this!

@timkinnane It would be good to see what you guys have already done if you have something working?
The way that I have tried to implement this is to make it use the muting feature that already exists as it seems to provide the functionality that we need without reinventing the wheel. I went for the approach of muting all users, then you can individually set who has permission to talk (as the room creator, you may not even want to talk yourself, it it is just for a bot, or a non tech leader who doesn't know how to setup a channel). I thought this was better rather than having loads of settings to configure? Are there any situations where you would want to give all bots permission to talk, or all admins etc.?

Using rocketbot to tell a user that they are muted is the default behaviour for muted users before. If you are worried about rocketbot being disabled then maybe we should make normal muting messages a system message as well? If not, I could change the message so instead of saying that the user is muted, I could tell them that it's a read only channel?

I will try to reproduce the bug you mentioned tonight, and try to fix it. Let me know if you have any ideas as to why this is happening.

Thanks for the feedback!

[timkinnane]

Thanks for the detailed response @alexbrazier. This community has such devoted contributors :)

I've made our feature branch available here.

We also use the standard muting behaviour, but we've tried to improve the default implementation when the whole room is muted/read-only. We hooked into onClientBeforeSendMessage to suppress the sending of messages in the client, so they don't show up in the room. I think the default of a muted user seeing their messages in the room, but no one else does, provides a confusing experience. I think If they can't post, they shouldn't be able to post. I see this as a quick fix step forward (as opposed to changing the default muting behaviour), but maybe we should be disabling the text input entirely. What's your opinion?

We're just testing functionality, so the controls are pretty basic, just two buttons, hard coded to mute the user role in the current room. Ideally that would be a multi-select list with every role. Admins or room owners would be able to select any roles that should be muted. The next part we haven't got to yet, are the controls to suppress the notification types. I think maybe your approach of adding the controls to the room info panel makes more sense, we just haven't focused much on the UI yet.

There's a diverse range of use cases in the world of Rocket.Chat so its dangerous to make assumptions what will work best for everyone, or even what works now which might change with scale, so I err on the side of dynamic configuration when planning features - even if that means a couple more controls as long as their UI is clean and intuitive. My comments on #825 mention some of the other issues that can be addressed if we keep the configuration open.

We have a handful of bots that need to post in most rooms and we're running multiple instances for different communities with different needs for different rooms and users in a variety of roles who join at different times. So we don't want to rely on manual actions to fulfil what I see as a permission based behaviour, when the role system can be implemented easier at scale.

Oh yeah, lastly yes I do think the "you are muted response" should be a system response, not from rocketbot. We have branded instances with no Rocket.Chat references so rocketbot makes no sense to suddenly appear to our users for this one message type. I'm not sure that message works with internationalisation the way all other system alerts do either. It seems like possibly an artifact from early days, I'm assuming no one will mind if we change it.

As for the bug. Try adding a dummy user to the room before its read only, then while they're not online make the room read only. Switch to another browser and login as the dummy user, it seems to hang on the first screen trying to restore the room state. Might be an unrelated bug though, there's a few in the current develop branch.

Have a look what we've done so far. Would be great if we can come up with a merged approach.

[alexbrazier]

@timkinnane I've had a look at your changes, and they look pretty good! I like how you have used the onClientBeforeSendMessage hook, which I might implement in my change (although probably going to hold off for a bit until everyone can agree a solution). Only question is when you add the callback for afterJoinRoom, and beforeJoinDefaultChannels, does that only hook onto the current room? I'm not really sure how the callbacks work, but I would assume that you could only set the same callback once, so once to set one room to read only, you are setting future users to join any room to be read only?

I like the idea of changing the muted users to just disable the text box with the message in your comment above, as rocketbot doesn't seem to be used anywhere else, so I think we should go with that approach, and just change it for muting in general.

Something I would questing is whether we want settings to say which user groups are allowed to post in read only channels.

• Firstly because it might be hard to make this look nice on the frontend, as we may end up with too many settings related to read-only rooms, which I feel will go unused by a lot of users.
• Secondly because I'm not sure if in most cases you would be able to say "all admins and bots are allowed to post in the room", as there may be some bots or admins that you don't want to be able to post. Potential so

Do you (or anyone else) have any thoughts on these points - e.g. where to put the setting?
If you do have lots of read only channels, and lots of users that should always be able to post in read only rooms, maybe the solution is to add another permission called "read-only-user" or something, that is always allowed to make posts?

I think I will add the setting to disable room system notifications, such a users joining and leaving, as this can be a setting for all channels, even non read only.

I will take a look at that bug tomorrow, and see if I can recreate it. Thanks for giving a detailed description to recreate.

[timkinnane]

Great ideas @alexbrazier. Firstly just wanted to credit @nazclarion, as he actually coded the solution, but we worked on the spec together.

Regarding the callbacks, I'm not actually sure why we're doing both beforeJoinDefaultChannels and afterJoinRoom. Will check with Nazar on that. I think we attached the mute check on every room join, to mute users who join after the room is made read-only because when you apply it, it only mutes current joined members. How did you address that?

I like the idea of a single permission to overrule read-only. That allows everything we wanted with roles, without having to specify each role with access in the room settings. I'd suggest something more semantic though, like override-read-only or can-post-read-only.

Sounds like there's three distinct parts to the solution now, to take the advantages of both approaches and address the related issues.

1. Make read-only use a permission for allowing to post
2. Modify muted behaviour in client to provide more consistent UX and avoid rocketbot
3. Add room controls to suppress notification types.

Are you happy to make changes to your PR and proceed with part 1? I won't bother submitting our own PR if we can join forces 😄

Part 2 needs a distinct issue and possibly further discussion - I've just created that: #3869

We have some code written for part 3, its not a priority so we didn't get far, but I'll dig that up and share it. Don't know if that's something you've got time or intention to address as well? We can do it, but might not come back to it for a while.

[timkinnane]

FYI, here's the info from @nazclarion on joinDefaultChannels. We had to use both hooks because joinDefaultChannels does not use standard joinRoom method where afterJoinRoom runs, instead, it joins users directly - by modifying collection, like: RocketChat.models.Rooms.addUsernameById room._id, user.username

[alexbrazier]

Sounds good @timkinnane, I'm glad we could find a solution. I will make changes to my PR to do part 1, and may do part 3 as well.

[alexbrazier]

@timkinnane I looked into the bug you mentioned, and tried it on the develop branch, and it appears to be happening there as well, so I'm assuming its an issue with develop. I tried clearing my db, but still doesn't work. I have created the issue #3872 for it. Let me know if you get the same issues with the latest version of develop as well if you have time to test it, otherwise I will try clearing everything and starting again in case something has been cached somewhere.

[alexbrazier]

@RocketChat/core @timkinnane: I think this is ready to be checked and merged

[rodrigok]

Why are you doing this query?

[rodrigok]

@alexbrazier first of all, this PR is amazing, we are very happy to see the community engaged like this 😄

Can you explain some things to me?

1. Muted users can't post messages in that channel, right?
2. If item 1 is right, where in the code you are blocking muted users to send messages?

We are migrating part of our core to use a new type of cache at server side, and we are removing the array of usernames from the db.
Can we store the stated of muted inside the user's subscription instead of inside the room?

[timkinnane]

I'm just dropping back in to this to say I also think it's awesome. Thanks @alexbrazier. I've been busy but hoping to test your feature branch by next week and work on improvements either before or after PR is accepted.

If there's anything you need specific help with let me know.

[alexbrazier]

Thanks! @rodrigok to answer your questions, muted users can't post in the channel and this feature is already implemented, so I didn't have to change anything.

So are you moving users who are in a room to only be stored in the subscriptions? If so the problem with doing the same for muted users is that a muted user can leave a room, and rejoin and they wont be muted anymore (unless its a read only room) as the subscription that contained muted information gets deleted

[rodrigok]

@alexbrazier you are right, but I don't see this as a huge problem, we can live with that right now. Will work for read only rooms, for other rooms if you want to mute the user maybe you should consider to exclude the user 😄

[timkinnane]

@rodrigok Can you confirm that means apart from read-only rooms, there's no capacity to mute someone without kicking from the room entirely? Or at least if you do, they can just exit and rejoin to unmute themselves? Seems like that's breaking a pretty core feature.

[alexbrazier]

Updated by #4034