Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-39216 - Hide group sync in channel details if not Enterprise #9137

Merged
merged 2 commits into from
Oct 14, 2021
Merged

MM-39216 - Hide group sync in channel details if not Enterprise #9137

merged 2 commits into from
Oct 14, 2021

Conversation

marianunez
Copy link
Member

@marianunez marianunez commented Oct 13, 2021 •
edited
Loading

Summary

Group sync is only available in Enteprise so it should not be allowed in the channel details configuration. This PR adds the license check missing in that screen.

Ticket Link

https://mattermost.atlassian.net/browse/MM-39216

Screenshots

Starter and Professional Enterprise
Screen Shot 2021-10-13 at 9 31 37 PM Screen Shot 2021-10-13 at 9 32 22 PM

Release Note

NONE

@mm-cloud-bot
Copy link

@marianunez: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

@marianunez marianunez added this to the v6.1.0 milestone Oct 13, 2021
@marianunez marianunez added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Oct 14, 2021
marianunez
marianunez commented Oct 14, 2021
View reviewed changes
components/admin_console/team_channel_settings/channel/details/channel_modes.tsx
Comment on lines +80 to +85
) : (
<FormattedMessage
id='admin.channel_settings.channel_details.isPublicDescr'
defaultMessage='If `public` the channel is discoverable and any user can join, or if `private` invitations are required. Toggle to convert public channels to private. When Group Sync is enabled, private channels cannot be converted to public.'
/>
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change here, just my linter formatting change

@marianunez marianunez added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Oct 14, 2021
@hahmadia
Copy link
Contributor

hahmadia commented Oct 14, 2021 •
edited
Loading

The backend guards against this already I assume? It's just that the front end hadn't been fixed, right?

hahmadia
hahmadia approved these changes Oct 14, 2021
View reviewed changes
Copy link
Contributor

@hahmadia hahmadia left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked.

To add groups to a group sync, you need to have LDAP groups.
To have LDAP groups, you need E20 license (which we do guard against)

Therefore I don't see any issues with this PR

Having this property set on a channel on it's own seems to not do anything other than do conditional rendering on the front-end.

neallred
neallred approved these changes Oct 14, 2021
View reviewed changes
Copy link
Contributor

@neallred neallred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@marianunez
Copy link
Member Author

I checked.

To add groups to a group sync, you need to have LDAP groups. To have LDAP groups, you need E20 license (which we do guard against)

Thanks for double checking Hossein!

@marianunez marianunez removed the 2: Dev Review Requires review by a core commiter label Oct 14, 2021
stevemudie
stevemudie approved these changes Oct 14, 2021
View reviewed changes
Copy link
Contributor

@stevemudie stevemudie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Group sync unavailable in any SKU except Enterprise as expected.

Looks good!

@stevemudie stevemudie added QA Review Done and removed 3: QA Review Requires review by a QA tester labels Oct 14, 2021
@marianunez marianunez added the 4: Reviews Complete All reviewers have approved the pull request label Oct 14, 2021
@marianunez marianunez merged commit f7ff8f0 into master Oct 14, 2021
@marianunez marianunez deleted the group-sync-channel-permission branch October 14, 2021 15:04
@mattermod mattermod removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Oct 14, 2021
@mm-cloud-bot
Copy link

Test server destroyed

1 similar comment
@mm-cloud-bot
Copy link

Test server destroyed

@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Oct 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stevemudie stevemudie stevemudie approved these changes

@neallred neallred neallred approved these changes

@hahmadia hahmadia hahmadia approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation QA Review Done release-note-none
Projects
None yet
Milestone
v6.1.0
7 participants
@marianunez @mm-cloud-bot @hahmadia @stevemudie @neallred @mattermod @amyblais