Skip to content
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

Closed

Conversation

garychapman
Copy link
Contributor

@RocketChat/core

Closes #825 #3861 #3641 #3539
Related #2043

I've taken the great work already done by @alexbrazier and extended it to completion:

  • Added Read Only controls to the Room Info admin panel
  • MessageBox and all other input methods (including drag/drop) are disabled for muted users in the room
  • Users already in a room are muted when it becomes read-only

Thanks also to @timkinnane for his invaluable assistance

alexbrazier and others added 13 commits July 25, 2016 22:18
# 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
@rodrigok
Copy link
Member

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',
Copy link
Contributor

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

Copy link
Contributor Author

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.

@alexbrazier
Copy link
Contributor

@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
Copy link
Contributor

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

Copy link
Contributor Author

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.

@Lucky21110
Copy link

Hello,

When it will be committed into develop ?

@Lucky21110
Copy link

Hello,

How I can edit this on my current release and put it in prod ?
I downloaded all file using git but it looks like different.

When this will be put in prod ?

Long time ago that I'm waiting for that

@geekgonecrazy
Copy link
Contributor

@Lucky21110 I believe we talked on Rocket.Chat. Here

@rodrigok
Copy link
Member

rodrigok commented Sep 3, 2016

@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
@garychapman
Copy link
Contributor Author

@rodrigok Done!

# Conflicts:
#	packages/rocketchat-lib/server/methods/createChannel.coffee
@garychapman
Copy link
Contributor Author

@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...

@geekgonecrazy
Copy link
Contributor

LGTM @RocketChat/Core ?

@alexbrazier alexbrazier mentioned this pull request Sep 6, 2016
2 tasks
@marceloschmidt
Copy link
Member

It's incomplete. The set-readonly permission is never created.
I'll commit and merge in a bit.

@marceloschmidt
Copy link
Member

Merged in 9866b80. Thank you @garychapman and @alexbrazier

@infinitnet
Copy link

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..

@infinitnet
Copy link

By the way what I mentioned above wouldn't be an issue if disabling post-readonly for admins would work, but it doesn't seem to change anything even after restarting rocket.chat. Admins can still post to read-only channels.

@marceloschmidt
Copy link
Member

@infinitnet you're right. There's a bug with our post-readonly permission. When a room is set to readonly, we mute users without post-readonly permission, instead of checking it when the user tries to send a message. So, for a quick dirty solution, after you remove their post-readonly permission, unset then reset the room as readonly. I'll make sure we have another issue for this bug.

@timkinnane
Copy link
Contributor

Those are awesome suggestions @infinitnet. Thanks for the detective work @marceloschmidt.

@geekgonecrazy
Copy link
Contributor

@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.

@infinitnet
Copy link

@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 post-readonly permission from any group, including admin. A feature to manually mute users only in a certain channel would do too (for example toggle "Mute" for each user in "Members List" of a channel).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants