-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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
Add read only channels (completed) #4034
Add read only channels (completed) #4034
Conversation
# Conflicts: # packages/rocketchat-channel-settings/client/views/channelSettings.coffee # packages/rocketchat-lib/i18n/en.i18n.json
- Any users present when a read-only channel is created are muted - Refactored to remove unnecessary addUsernameByIdAndMute method - Standardised channel info icon
- Completed setReadOnlyById to set/unset muted user array
- messageBox hidden and dropzone disabled for read-only room if user doesn't have permission to post - renamed users-typing class to stream-info, added 'this room is read only' message - setReadOnlyById now removes empty muted array from room record
# Conflicts: # packages/rocketchat-ui-flextab/flex-tab/tabs/membersList.coffee # packages/rocketchat-ui-message/message/messageBox.html
Wow, this is huge and awesome 😄 @garychapman I will try to review your PR ASAP |
@@ -48,7 +48,7 @@ Template.adminRooms.onCreated -> | |||
groups: ['adminrooms'], | |||
id: 'admin-room', | |||
i18nTitle: 'Room_Info', | |||
icon: 'icon-info', | |||
icon: 'icon-info-circled', |
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.
Curious why this changed? I haven't seen the change with this, just curious
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.
Just for consistency with the default channelSettings icon.
@garychapman Thanks for finishing this off! I've been quite busy and didn't find the time. |
users.usernames.forEach (userName) -> | ||
|
||
# lookup the user | ||
user = RocketChat.models.Users.findOneByUsername userName |
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 seems quite expensive everytime. Can you just request the required fields e.g. {fields: {user._id: 1}}
?
Alternatively it might be more efficient to request all the user ids at once e.g.
usernames = @findOne(query, { fields: { usernames: 1 }})
users = RocketChat.models.Users.findUsersByUsernames usernames, {fields: {username: 1}}
users.forEach (user) ->
#Use user._id and user.username
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.
Good catch. I've updated the code.
# Conflicts: # packages/rocketchat-authorization/server/startup.coffee
…/strata.chat into feature/read-only-rooms
@Lucky21110 I believe we talked on Rocket.Chat. Here |
@garychapman Can you fix the conflicts please? |
# Conflicts: # packages/rocketchat-channel-settings/client/views/channelSettings.coffee # packages/rocketchat-channel-settings/client/views/channelSettings.html # packages/rocketchat-channel-settings/server/methods/saveRoomSettings.coffee # packages/rocketchat-lib/server/methods/addUserToRoom.coffee # packages/rocketchat-ui/views/app/room.coffee # server/methods/createChannel.coffee # server/methods/createPrivateGroup.coffee # server/methods/joinRoom.coffee # server/startup/roomPublishes.coffee
@rodrigok Done! |
# Conflicts: # packages/rocketchat-lib/server/methods/createChannel.coffee
@RocketChat/core could someone please lgtm and merge this? I've already done a couple of time-consuming conflict fixes over the past few weeks... |
LGTM @RocketChat/Core ? |
It's incomplete. The |
Merged in 9866b80. Thank you @garychapman and @alexbrazier |
I have one objection about how this has been implemented: Currently any admin user can still send messages in "read only" channels. It would be much better if only the channel owner(s) would be allowed to do that and not every admin just because he/she has admin permissions. In our team we have more admin users than non-admin users, which makes the currently implementation of read-only pretty much useless, because every admin can still speak and there is no way to mute them on a per channel basis.. |
By the way what I mentioned above wouldn't be an issue if disabling |
@infinitnet you're right. There's a bug with our |
Those are awesome suggestions @infinitnet. Thanks for the detective work @marceloschmidt. |
@infinitnet Might consider creating another role instead of making them all admin. With them all admin, all they have to do is go and add that permission back and they would be able to speak. If you're wanting to restrict your admin's it would probably be a good idea to give them their own role with the reduced permissions. Like existing admin role being super admin. Then the new role just basic admins. Just a thought. |
@geekgonecrazy I'm aware that admins could just add the permission to speak in a read-only channel. I don't mean to keep them from doing that. I only want to restrict everyone's ability to speak in a read-only channel to prevent unintentional messages in a channel that gets its content from other sources than user messages (RSS feeds, bots, webhooks, etc.). Moving everyone to a new group seems like a lot of work to accomplish what I need: the ability to remove |
@RocketChat/core
Closes #825 #3861 #3641 #3539
Related #2043
I've taken the great work already done by @alexbrazier and extended it to completion:
Thanks also to @timkinnane for his invaluable assistance