Remove write access to town square. Hide it when no unread messages. #831

stanchan · 2018-02-17T01:41:53Z

Summary

Hide edit able elements for Town Square for non admin users when "ExperimentalTownSquareIsReadOnly": true.
Hide:

channel view actions: post textbox, Edit channel header, Edit channel purpose, Rename channel.
post view actions: reply, pin, emoji, + emoji icon.
Also, hide town square link on the LHS when there are no unread messages and it is not favorited.

These changes were discussed in the original PRs:

webapp: 0a9c047
server: Config to make town square read only mattermost#7140
redux: Remove Post when error returns for TownSquareIsReadOnly=true mattermost-redux#222

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

Ran make check-style to check for style errors (required for all pull requests)
Ran make test to ensure unit and component tests passed
Touches critical sections of the codebase (auth, posting, etc.)

cc @jason @dmeza

dmeza · 2018-02-17T17:19:29Z

@jasonblais here's a gif of the changes from an admin and non admin point of view.

Some context on the changes. People began to misuse/troll through town square by changing header, display name, lots of emojis. They wanted a way out. It was compounded because of the bug of every reload going to town square and also since it was slow to load because of the amount of emojis and join messages.

The changes are:

For admins will have activated all functionality for town square and the channel is always visible in the LHS (no changes).
For non admins they can not take any action in town square and it will be hidden in the LHS if user is not on town square, there are no unread messages or it's not favorited. It's searchable through command+K.

There are changes to the mobile application that will be committed back soon. Let me know if you'd like to discuss.

jasonblais · 2018-02-17T18:32:59Z

@dmeza

Most of the proposed changes for team admins and members make sense. Have left notes for 1 - 4:

Post textbox: In v4.7.0 and later, the textbox is actually disabled already. You can test it on pre-release.mattermost.com.

Would that work for your team? As it might seem as a bug to end users if the textbox is missing in Town Square.

Emoji icon and File upload icon in post textbox: already hidden in the textbox, as part of 1) above
Reply icon: would prefer to keep this. For instance, a System Admin might make an announcement, and then replies with updates. Having them in a single comment thread can be useful. That being said, commenting is disabled as shown below. Would you have thoughts?

Hide town square on the LHS: This I'm worried about as it could be confusing to end users.

Assuming we don't allow people to misuse/troll Town Square, and that the reload issue is fixed, would people still want Town Square to be hidden from the sidebar?

The worry I have is how it might impact initial on-boarding. Could new users have an empty channel sidebar, if they're not added to channels (and Town Square isn't on LHS)?

I'll test the expected cases for system admins, team admins and members on these items, but agree with the proposed changes of not allowing non-system admins to:

Edit channel header
Edit channel purpose
Rename channel
Pin a message
Emoji react to a message

dmeza · 2018-02-19T17:32:59Z

@jasonblais I'll show 1. and 2. and get an answer.
For 3. Admins have all the functionality and can reply, so your case would be covered. Non admin users have reply restricted.
For 4. on-boarding wouldn't be a problem for of non admins. Initially a user starts in town square and has another channel available (off-topic). If the user removes all other channels, it's taken to town square and it's visible on the LHS:

jasonblais · 2018-02-20T05:04:40Z

3 - That's true, but would it be beneficial to allow the user to view all the replies in the comment thread on RHS?

Maybe we remove the Reply option from the [...] menu (since the non-admin can't reply to the post), but keep the reply arrow when applicable. What do you think?

True. The other worry I have is inconsistency, since we don't hide any other channels at the moment.

After this PR, which limits people from misuse/trolling in Town Square, and that the reload issue is fixed, would people still want Town Square to be hidden from the sidebar?

dmeza · 2018-02-20T15:12:41Z

@jasonblais I'll review 3. with the team.

I understand it, but in this case town square is the only channel that's different. With this option enabled it's read only and it's the only channel you can not leave. It's already not consistent with the other channels.

One of the most requests features is to be able to leave town-square. Users can leave off-topic, but not town-square. They only want to see the channels that they joined. They say it's distracting and frustrating given the amount of users. The idea is to reduce noise as much as possible.

jasonblais · 2018-02-20T15:36:20Z

Thanks @dmeza, I'll bring this up with our UX team

herooftimeandspace · 2018-02-20T17:16:13Z

I'm fine with notifying the user that the channel is read-only in the input box.
Also good. Our change also removes the ability to react to an system admin post that has existing reactions, which 1. does not address.
I don't see the value of the RHS in a read-only announcement room where only admins will post. The likelihood that a threaded conversation would be difficult to follow seems low to me.
I could see gating behind a second flag like "ExperimentalHideDefaultChannelinLHS" or even turning this into a toggle in Account Settings.

jasonblais · 2018-03-02T00:32:08Z

Sorry for the huge delay in replying. I think majority of what you said makes sense, but I just want to run it by Lindsay as well.

Most of the Mattermost team has been travelling this week, so our responses times are slower than usual. Bare with us.

dmeza · 2018-03-02T01:29:20Z

@jasonblais rebased to latest from master and fixed conflicts. Removed the change to hide the Post textbox.

dmeza · 2018-03-07T17:30:40Z

Rebased to latest from master. Added logic to hide links in header to invite and set header when "ExperimentalTownSquareIsReadOnly": true.

jasonblais · 2018-03-14T19:19:54Z

Discussed with our team.

Summarizing the above discussion below:

Don't allow non-system admins to (team agrees):

Edit channel header
Edit channel purpose
Rename channel
Pin a message
Emoji react to a message

Moreover, for non-system admins, disable textbox, and emoji icon & file upload icon in the post textbox (already implemented)
@herooftimeandspace @dmeza We're wondering how much benefit is there for removing the reply arrow for read-only channels? It adds an additional case to test and maintain for upcoming releases. It might be simpler to just keep it, since there isn't any harm for the user. Thoughts?
Given this setting is experimental and you're main users of it, we're okay trying this out where Town Square is hidden when it's read. We will test it on pre-release once it's merged and see how it feels. If it's against user expectations, we'll consider adding it as a separate config flag before we ship. Does that sound good?

herooftimeandspace · 2018-03-14T21:10:10Z

Looks good.
We have this in our staging environment. I think the experience is better, thank you.
As long as the reply sidebar also disables textbox, emoji icon and file upload icon, then I'm satisified. Let's listen for feedback on this though. Some users may feel misled by a button that results in no actions available.
Thanks!

jasonblais · 2018-03-14T21:12:29Z

Yeah, it does disable them similar to the center channel. Agree we can listen to user feedback too.

@dmeza if you can help update the PR for 3 (don't hide reply arrow), I'll test this PR and we should be good to merge it soon.

dmeza · 2018-03-15T18:39:29Z

@jasonblais rebased to latest from MM master and fixed conflicts. Made changes to allow view and click on reply for all users. Hide actions from RHS for non admins.
I think this is all the changes requested.

jasonblais · 2018-03-16T13:00:38Z

@jwilander Would you be able to help SSH to the spinmint and set ExperimentalTownSquareIsReadOnly in config.json to true? I'll test the changes.

I know PMs should be able to do ssh to the server and make the change, but I haven't got around to it yet..

jwilander · 2018-03-16T15:10:51Z

@jasonblais done

jasonblais · 2018-03-16T15:30:22Z

@jwilander Seems town square isn't in read-only after logging into a member account. Wondering if the config.json update went true or if there's a bug.

hmhealey · 2018-03-19T13:40:52Z

components/channel_view/index.js

@@ -29,11 +31,16 @@ function mapStateToProps(state) {
 const config = getConfig(state);
 const enableTutorial = config.EnableTutorial === 'true';
 const tutorialStep = getInt(state, Preferences.TUTORIAL_STEP, getCurrentUserId(state), TutorialSteps.FINISHED);
+ const channel = getCurrentChannel(state);
+ const isReadOnly = channel && channel.name === Constants.DEFAULT_CHANNEL &&
+ !UserStore.isSystemAdminForCurrentUser() &&


We shouldn't be using the UserStore directly here. You could instead import the isCurrentUserSystemAdmin selector from 'mattermost-redux/selectors/entities/users` and use that instead

Also, should this be just system admin or can a team admin still modify the town square?

hmhealey · 2018-03-19T13:43:29Z

components/channel_view/index.js

@@ -29,11 +31,16 @@ function mapStateToProps(state) {
 const config = getConfig(state);
 const enableTutorial = config.EnableTutorial === 'true';
 const tutorialStep = getInt(state, Preferences.TUTORIAL_STEP, getCurrentUserId(state), TutorialSteps.FINISHED);
+ const channel = getCurrentChannel(state);
+ const isReadOnly = channel && channel.name === Constants.DEFAULT_CHANNEL &&


Could you make this into a selector function? I think you'll need this in the mobile repo as well, so it could live in the redux repo. It could be something like isCurrentChannelReadOnly

hmhealey · 2018-03-19T13:44:21Z

components/rhs_thread/index.js

 if (selected) {
 posts = getPostsForThread(state, {rootId: selected.id});
 channel = getChannel(state, selected.channel_id);
+ isReadOnly = channel && channel.name === Constants.DEFAULT_CHANNEL &&


That selector would also be used here

hmhealey · 2018-03-19T13:46:09Z

components/channel_view/index.js


 return {
 channelId,
 deactivatedChannel: getDeactivatedChannel(state, channelId),
 showTutorial: enableTutorial && tutorialStep <= TutorialSteps.INTRO_SCREENS,
+ isReadOnly,


If we had a selector, would it be simpler if we just had the connected components grab that value from the store themselves when they need it instead of passing it up from here? There's a lot of layout components that have been changed by this PR just to pass around isReadOnly to their children.

hmhealey · 2018-03-19T13:46:28Z

components/sidebar/index.js

@@ -78,6 +78,7 @@ function mapStateToProps(state) {
 unreads: getUnreads(state),
 showCreatePublicChannelOption,
 showCreatePrivateChannelOption,
+ isTownSquareReadOnly: !isSystemAdmin && config.ExperimentalTownSquareIsReadOnly === 'true',


dmeza · 2018-03-22T18:45:54Z

@hmhealey made the changes requested and rebased to latest from master.
This is the redux PR: mattermost/mattermost-redux#429

jwilander

LGTM

hmhealey · 2018-03-26T15:19:31Z

Now waiting on mattermost/mattermost-redux#429

dmeza · 2018-03-26T15:56:16Z

Rebased to latest from master and tested that everything is working because of change on mattermost/mattermost-redux#429

dmeza · 2018-03-27T21:52:16Z

@jasonblais @jwilander will this be merged for 4.9?

jasonblais · 2018-03-27T21:55:33Z

@dmeza Looks like it's ready to merge, but there are some conflicting files.

@jwilander @hmhealey any concerns with merging this today for v4.9?

Hide reply, pin, emoji actions in town square when "ExperimentalTownSquareIsReadOnly": true for non admin users. Hide + emoticon icon so not action can be taken for regular users. For regular users hide town square when ExperimentalTownSquareIsReadOnly=true and there are no unread messages and it is not favorited. Allow reply on ReadOnly channel and hide RHS actions Refactor to move isReadOnly logic to redux selectors. Also moved to the specific impacted component.

dmeza · 2018-03-28T04:06:54Z

@jasonblais @jwilander @hmhealey rebased to latest from master, fixed conflicts and tested that it's working.

jwilander · 2018-03-28T15:38:57Z

Awesome work on this @dmeza ! Glad it's going in :)

… track if a user is a System Admin (#831) * added setUserRoles to Client4 to pass the roles in analytics 'user_actual_role' event * only set user_actual_role in case of admin user * check for presence of roles * cleaner user role access, reported role names * changed role name to be more consistent

jasonblais added 1: PM Review Requires review by a product manager Setup Old Test Server Triggers the creation of a test server labels Feb 17, 2018

jasonblais self-requested a review February 17, 2018 18:33

jasonblais self-assigned this Feb 17, 2018

dmeza force-pushed the remove_write_access_to_town_hall_to_master branch from e0babc8 to 5df95b1 Compare March 2, 2018 01:26

dmeza force-pushed the remove_write_access_to_town_hall_to_master branch from 5df95b1 to d4da8cd Compare March 7, 2018 17:20

dmeza force-pushed the remove_write_access_to_town_hall_to_master branch 2 times, most recently from a561864 to 1a03d05 Compare March 15, 2018 18:37

jasonblais removed the Setup Old Test Server Triggers the creation of a test server label Mar 16, 2018

mattermost deleted a comment from mattermod Mar 16, 2018

jasonblais added the Setup Old Test Server Triggers the creation of a test server label Mar 16, 2018

hmhealey suggested changes Mar 19, 2018

View reviewed changes

dmeza force-pushed the remove_write_access_to_town_hall_to_master branch 3 times, most recently from abd823e to 2138b71 Compare March 22, 2018 05:15

csduarte mentioned this pull request Mar 22, 2018

Add selectors to check if current channel and a channel is ReadOnly. mattermost/mattermost-redux#429

Merged

1 task

jarredwitt mentioned this pull request Mar 22, 2018

ExperimentalHideReadOnlyTownSquare config and readonly PostTexbox placeholder mattermost/mattermost-mobile#1476

Merged

jwilander approved these changes Mar 23, 2018

View reviewed changes

hmhealey added the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Mar 26, 2018

hmhealey approved these changes Mar 26, 2018

View reviewed changes

hmhealey removed the 2: Dev Review Requires review by a core commiter label Mar 26, 2018

dmeza force-pushed the remove_write_access_to_town_hall_to_master branch from 2138b71 to e8b247b Compare March 26, 2018 15:39

dmeza force-pushed the remove_write_access_to_town_hall_to_master branch from e8b247b to 96e050a Compare March 28, 2018 04:01

jasonblais added 4: Reviews Complete All reviewers have approved the pull request and removed Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) labels Mar 28, 2018

jwilander merged commit bafea1d into mattermost:master Mar 28, 2018

amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Mar 28, 2018

lindalumitchell added the Tests/Done Release tests have been written label Apr 3, 2018

jasonblais mentioned this pull request May 9, 2018

Docs for 4.10 mattermost/docs#1995

Merged

dmeza deleted the remove_write_access_to_town_hall_to_master branch December 7, 2018 18:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove write access to town square. Hide it when no unread messages. #831

Remove write access to town square. Hide it when no unread messages. #831

stanchan commented Feb 17, 2018

dmeza commented Feb 17, 2018

jasonblais commented Feb 17, 2018

dmeza commented Feb 19, 2018 •

edited

Loading

jasonblais commented Feb 20, 2018

dmeza commented Feb 20, 2018

jasonblais commented Feb 20, 2018

herooftimeandspace commented Feb 20, 2018

jasonblais commented Mar 2, 2018

dmeza commented Mar 2, 2018

dmeza commented Mar 7, 2018

jasonblais commented Mar 14, 2018 •

edited

Loading

herooftimeandspace commented Mar 14, 2018

jasonblais commented Mar 14, 2018

dmeza commented Mar 15, 2018

jasonblais commented Mar 16, 2018

jwilander commented Mar 16, 2018

jasonblais commented Mar 16, 2018

hmhealey Mar 19, 2018

hmhealey Mar 19, 2018 •

edited

Loading

hmhealey Mar 19, 2018

hmhealey Mar 19, 2018

hmhealey Mar 19, 2018

hmhealey Mar 19, 2018

dmeza commented Mar 22, 2018

jwilander left a comment

hmhealey commented Mar 26, 2018

dmeza commented Mar 26, 2018

dmeza commented Mar 27, 2018

jasonblais commented Mar 27, 2018

dmeza commented Mar 28, 2018

jwilander commented Mar 28, 2018

Remove write access to town square. Hide it when no unread messages. #831

Remove write access to town square. Hide it when no unread messages. #831

Conversation

stanchan commented Feb 17, 2018

Summary

Checklist

dmeza commented Feb 17, 2018

jasonblais commented Feb 17, 2018

dmeza commented Feb 19, 2018 • edited Loading

jasonblais commented Feb 20, 2018

dmeza commented Feb 20, 2018

jasonblais commented Feb 20, 2018

herooftimeandspace commented Feb 20, 2018

jasonblais commented Mar 2, 2018

dmeza commented Mar 2, 2018

dmeza commented Mar 7, 2018

jasonblais commented Mar 14, 2018 • edited Loading

herooftimeandspace commented Mar 14, 2018

jasonblais commented Mar 14, 2018

dmeza commented Mar 15, 2018

jasonblais commented Mar 16, 2018

jwilander commented Mar 16, 2018

jasonblais commented Mar 16, 2018

hmhealey Mar 19, 2018

Choose a reason for hiding this comment

hmhealey Mar 19, 2018 • edited Loading

Choose a reason for hiding this comment

hmhealey Mar 19, 2018

Choose a reason for hiding this comment

hmhealey Mar 19, 2018

Choose a reason for hiding this comment

hmhealey Mar 19, 2018

Choose a reason for hiding this comment

hmhealey Mar 19, 2018

Choose a reason for hiding this comment

dmeza commented Mar 22, 2018

jwilander left a comment

Choose a reason for hiding this comment

hmhealey commented Mar 26, 2018

dmeza commented Mar 26, 2018

dmeza commented Mar 27, 2018

jasonblais commented Mar 27, 2018

dmeza commented Mar 28, 2018

jwilander commented Mar 28, 2018

dmeza commented Feb 19, 2018 •

edited

Loading

jasonblais commented Mar 14, 2018 •

edited

Loading

hmhealey Mar 19, 2018 •

edited

Loading