-
Notifications
You must be signed in to change notification settings - Fork 2.7k
MM-20371 migrate add_groups_to_team_modal to ts #7567
MM-20371 migrate add_groups_to_team_modal to ts #7567
Conversation
@@ -129,16 +144,17 @@ export default class AddGroupsToTeamModal extends React.PureComponent { | |||
this.setState({saving: true}); | |||
|
|||
groupIDs.forEach(async (groupID) => { | |||
const {error} = await this.props.actions.linkGroupSyncable(groupID, this.props.currentTeamId, Groups.SYNCABLE_TYPE_TEAM, {auto_add: true}); | |||
const {error} = await this.props.actions.linkGroupSyncable(groupID, this.props.currentTeamId, Groups.SYNCABLE_TYPE_TEAM, {auto_add: true, scheme_admin: false}); |
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.
I'm not sure if it not changes behaviour, but type requires it.
Related issue: mattermost/mattermost-redux#1362
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.
that is a good question.
@nevyangelova could you have a look at it, since you are more proficient with redux?
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.
As far as I can see the scheme_admin prop is required by the server so it can pass validation and it looks like a recent add, do you have any concerns with passing that prop @komik966 ?
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.
@nevyangelova
http body before my changes:
{ "auto_add": true }
http body after my changes:
{ "auto_add": true, "scheme_admin": false }
When body comes to the server, it goes through this path:
My concern is if server treats nil
same way as false
.
f7f6341
to
5c27767
Compare
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.
There is nothing breaking here, just some remarks to keep things consistent
components/add_groups_to_team_modal/add_groups_to_team_modal.test.tsx
Outdated
Show resolved
Hide resolved
components/add_groups_to_team_modal/add_groups_to_team_modal.tsx
Outdated
Show resolved
Hide resolved
@@ -129,16 +144,17 @@ export default class AddGroupsToTeamModal extends React.PureComponent { | |||
this.setState({saving: true}); | |||
|
|||
groupIDs.forEach(async (groupID) => { | |||
const {error} = await this.props.actions.linkGroupSyncable(groupID, this.props.currentTeamId, Groups.SYNCABLE_TYPE_TEAM, {auto_add: true}); | |||
const {error} = await this.props.actions.linkGroupSyncable(groupID, this.props.currentTeamId, Groups.SYNCABLE_TYPE_TEAM, {auto_add: true, scheme_admin: false}); |
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.
that is a good question.
@nevyangelova could you have a look at it, since you are more proficient with redux?
components/add_groups_to_team_modal/add_groups_to_team_modal.tsx
Outdated
Show resolved
Hide resolved
components/add_groups_to_team_modal/add_groups_to_team_modal.tsx
Outdated
Show resolved
Hide resolved
components/add_groups_to_team_modal/add_groups_to_team_modal.tsx
Outdated
Show resolved
Hide resolved
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.
LGTM
@@ -129,16 +144,17 @@ export default class AddGroupsToTeamModal extends React.PureComponent { | |||
this.setState({saving: true}); | |||
|
|||
groupIDs.forEach(async (groupID) => { | |||
const {error} = await this.props.actions.linkGroupSyncable(groupID, this.props.currentTeamId, Groups.SYNCABLE_TYPE_TEAM, {auto_add: true}); | |||
const {error} = await this.props.actions.linkGroupSyncable(groupID, this.props.currentTeamId, Groups.SYNCABLE_TYPE_TEAM, {auto_add: true, scheme_admin: false}); |
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.
As far as I can see the scheme_admin prop is required by the server so it can pass validation and it looks like a recent add, do you have any concerns with passing that prop @komik966 ?
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.
LGTM ... thanks! ✨
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.
Thank you @komik966
Tested, looks good to merge.
- Verified add group to team modal in Sys. Console > Team configuration.
@michelengelen Can you please help merge. Thanks 🙂
Test server destroyed |
@jgilliam17 done! 👍🏻 |
Added guest test cases [MM-33127] Fixing admin console search clear icon (mattermost#7578) Automatic Merge Fixing MM-33118 (mattermost#7574) Automatic Merge MM-33209 Downgrade serialize error (mattermost#7586) Hiding custom statuses config when feature flag is disabled (mattermost#7582) Automatic Merge [MM-20386] migrate popover_bar to typescript (mattermost#7539) Co-authored-by: Mattermod <[email protected]> MM-31740 Gif picker empty when no results are displayed (mattermost#7501) Automatic Merge dont focus the logs table (mattermost#7519) Co-authored-by: Mattermod <[email protected]> [MM-32691] - Send email when the users are not able to join a workspace (mattermost#7553) * [MM-32691] - Send email when the users are not able to join a workspace * make i18n-extract * Update redux * Silently send email * Restore package-lock * Update package-lock with mattermost-redux hash * Restore package.json * Update package.json with mattermost-redux hash Co-authored-by: Mattermod <[email protected]> [MM-32641] - Add the 'Invite Members' module to the end-user onboarding flow (mattermost#7580) * [MM-32641] - Add the 'Invite Members' module to the end-user onboarding flow * Fix tsx linting [MM-32690] - Add 'Notify Admin' button when end-users are presented with the user limit reached modal (mattermost#7579) * [MM-32690] - Add 'Notify Admin' button when end-users are presented with the user limit reached modal * Remove comment * Fix snaps * Fix import MM-33237 Update webpack-livereload-plugin (mattermost#7590) fix several failed tests (mattermost#7583) MM-20371 migrate add_groups_to_team_modal to ts (mattermost#7567) [MM-32098] Add Cypress Test for Support Packet Generation (mattermost#7558) Automatic Merge Cypress/E2E: Fix E2E due to recent change in at-mention suggestion list (mattermost#7565) * fix e2e due to recent change in at-mention suggestion list * Apply suggestions from code review Co-authored-by: Prapti <[email protected]> * Apply suggestions from code review Co-authored-by: Caleb Roseland <[email protected]> Co-authored-by: Prapti <[email protected]> Co-authored-by: Caleb Roseland <[email protected]> Cypress/E2E: Fix failed tests on theme (mattermost#7589) * fix failed tests on theme * add TM4J keys MM-33245 Fix casing on contenthash for CSS (mattermost#7594) Removed unused roles requests state objects (mattermost#7562) MM-33285 Turn off snapshotting of node_modules and minor webpack changes (mattermost#7598) * Remove default option * Use contenthash instead of the now-deprecated hash * Turn of snapshotting of node_modules to fix dev:watch Update links in README.md (mattermost#7603) Translations plus fix (mattermost#7606) * Update translation files Updated by "Cleanup translation files" hook in Weblate. Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/ Update translation files Updated by "Cleanup translation files" hook in Weblate. Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/ * Translated using Weblate (Korean) Currently translated at 79.4% (3561 of 4484 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/ko/ Translated using Weblate (Korean) Currently translated at 78.8% (3537 of 4484 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/ko/ * Translated using Weblate (Swedish) Currently translated at 100.0% (4483 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/sv/ Translated using Weblate (Swedish) Currently translated at 100.0% (4483 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/sv/ Translated using Weblate (Swedish) Currently translated at 100.0% (4484 of 4484 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/sv/ * Translated using Weblate (German) Currently translated at 87.8% (3937 of 4484 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/de/ * Translated using Weblate (Dutch) Currently translated at 100.0% (4483 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/nl/ Translated using Weblate (Dutch) Currently translated at 100.0% (4483 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/nl/ Translated using Weblate (Dutch) Currently translated at 100.0% (4484 of 4484 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/nl/ * Translated using Weblate (Japanese) Currently translated at 100.0% (4484 of 4484 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/ja/ * Translated using Weblate (Turkish) Currently translated at 100.0% (4483 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/tr/ Translated using Weblate (Turkish) Currently translated at 100.0% (4483 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/tr/ Translated using Weblate (Turkish) Currently translated at 100.0% (4483 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/tr/ * Translated using Weblate (Chinese (Simplified)) Currently translated at 98.2% (4406 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/zh_Hans/ * Translated using Weblate (Bulgarian) Currently translated at 100.0% (4483 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/bg/ Translated using Weblate (Bulgarian) Currently translated at 100.0% (4483 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/bg/ * Translated using Weblate (Romanian) Currently translated at 100.0% (4483 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/ro/ * Translated using Weblate (German) Currently translated at 87.8% (3938 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/de/ * Translated using Weblate (German) Currently translated at 87.8% (3938 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/de/ * Translated using Weblate (German) Currently translated at 88.0% (3949 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/de/ * Translated using Weblate (German) Currently translated at 88.0% (3949 of 4483 strings) Translation: mattermost-languages-shipped/mattermost-webapp Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-webapp_master/de/ * Remove korean empty translation Co-authored-by: Hosted Weblate <[email protected]> Co-authored-by: 카리스턱 <[email protected]> Co-authored-by: MArtin Johnson <[email protected]> Co-authored-by: Friederike J <[email protected]> Co-authored-by: Tom De Moor <[email protected]> Co-authored-by: kaakaa <[email protected]> Co-authored-by: Kaya Zeren <[email protected]> Co-authored-by: aeomin <[email protected]> Co-authored-by: Nikolai Zahariev <[email protected]> Co-authored-by: Viorel-Cosmin Miron <[email protected]> Co-authored-by: Elisabeth Kulzer <[email protected]> Fix zoom on pdf previews (mattermost#7600) Cypress/E2E: Reorganize and add keys for Elasticsearch autocomplete - users (mattermost#7591) * fix e2e due to recent change in at-mention suggestion list * reorganize tests for Elasticsearch automplete for users * Update users_spec.js Changing comment from statement to verification Co-authored-by: Prapti <[email protected]> [MM-33132] - Subscription stats are not updated when a user is deactivated/activated (mattermost#7595) [MM-33107][MM-33312] fix rhs over textbox (mattermost#7581) * [MM-33107] fix rhs over textbox * improve transition * [MM-33312] move the sidebar up when in tablet size Co-authored-by: = <=> Co-authored-by: Mattermod <[email protected]> Updating font size for add people to channel modal (mattermost#7525) adding rtl word order support for the web app (text is still left aligned to look good on an ltr localized ui) (mattermost#7283) Co-authored-by: Anton Wolkov <[email protected]> Co-authored-by: Mattermod <[email protected]> update compass font icon (mattermost#7559) Co-authored-by: Mattermod <[email protected]> Update NOTICE.txt (mattermost#7611) Automatic Merge Campaign/applytheme sidebar header bg (mattermost#7383) * Remove usage of changeCss() for modal-header background (mattermost#6700) Closes [https://github.com/mattermost/mattermost-server/issues/15867](https://github.com/mattermost/mattermost-server/issues/15867) * Migrate changeCSS() to CSS variable in utils/utils.jsx in 550. (mattermost#6713) * MM-29458: Migrate changeCSS() to CSS variable in utils/utils.jsx (mattermost#6981) * Update sass/layout/_webhooks.scss - ensure new line at the end of the file * Address MM-29457 * Removed unecessary line Co-authored-by: Archana Choudhary <[email protected]> Co-authored-by: Haardik Dharma <[email protected]> Co-authored-by: Mustafa İlhan <[email protected]> Co-authored-by: Mattermod <[email protected]> Guest test cases added removed old file
Summary
Ticket Link
mattermost/mattermost#16734