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

[MM-49821] Allow pasting multiple emails into invite modal #12052

Merged
merged 14 commits into from
Mar 6, 2023
Merged

[MM-49821] Allow pasting multiple emails into invite modal #12052

merged 14 commits into from
Mar 6, 2023

Conversation

kostaspt
Copy link
Contributor

@kostaspt kostaspt commented Jan 23, 2023 •
edited
Loading

Summary

When pasting multiple valid emails separated with , ,, ; it will be handled as if a user typed all those and pressed enter afterward.

Ticket Link

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

Related Pull Requests

N/A

Screenshots

Kapture.2023-01-23.at.17.31.58.mp4

Release Note

Handle multiple emails at once when inviting users.

@kostaspt kostaspt added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 24, 2023
@kostaspt
Copy link
Contributor Author

/update-branch

@kostaspt kostaspt added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester 1: UX Review Requires review by a UX Designer labels Jan 25, 2023
@mattermost-build
Copy link
Contributor

E2E tests not automatically triggered, because PR has no approval yet. Please ask a developer to review and then try again to attach the QA label.

@kostaspt kostaspt requested a review from laneycs January 26, 2023 13:55
@kostaspt
Copy link
Contributor Author

@laneycs Do you think we're okay with this state so we can move on to dev review and QA? Or is there more functionality we want to include at this stage?

laneycs
laneycs approved these changes Jan 26, 2023
View reviewed changes
Copy link

@laneycs laneycs left a comment

Choose a reason for hiding this comment

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

@kostaspt We are ready to roll. Noting that this has been tested and approved by 3 team members in https://community.mattermost.com/private-core/pl/qgoyid7m93fhjbrnfsw4md7d6y

@kostaspt kostaspt removed 1: PM Review Requires review by a product manager 1: UX Review Requires review by a UX Designer labels Jan 26, 2023
@mkraft
Copy link
Contributor

mkraft commented Jan 26, 2023

Is this meant to be draft still or ready for us to review?

@kostaspt kostaspt marked this pull request as ready for review January 30, 2023 08:54
@kostaspt
Copy link
Contributor Author

Is this meant to be draft still or ready for us to review?

I had in mind to convert it to regular PR, but I forgot – sorry.

BenCookie95
BenCookie95 suggested changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@BenCookie95 BenCookie95 left a comment

Choose a reason for hiding this comment

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

I have to add Telemetry to track the number of people that are added from pasting vs typing (https://mattermost.atlassian.net/browse/MM-49773) but I will have to wait for this to merge in order to utilize the new functionality. Do you want to add that part of the telemetry in this PR?

Maybe we could add an optional callback prop to that increments the paste count and use trackEvent in the submit function?

@kostaspt
Copy link
Contributor Author

@BenCookie95 Sounds good. I'll add this part to this PR.

@mkraft mkraft self-requested a review January 31, 2023 13:05
@BenCookie95
Copy link
Contributor

@BenCookie95 Sounds good. I'll add this part to this PR.

Thanks Kostas!

@kostaspt kostaspt removed the 2: Dev Review Requires review by a core commiter label Feb 7, 2023
furqanmlk
furqanmlk reviewed Feb 7, 2023
View reviewed changes
Copy link
Contributor

@furqanmlk furqanmlk left a comment

Choose a reason for hiding this comment

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

Hi @kostaspt, I found two issues.

  1. Other special characters are also getting accepted.
    Example: [email protected] % [email protected]

  2. The email of the Deactivated user is getting accepted. Please see the below.

Filmage.2023-02-08_012120.mp4

@kostaspt
Copy link
Contributor Author

kostaspt commented Feb 8, 2023 •
edited by jira bot
Loading

  1. Not sure if this is an issue, as the % it's just being discarded. It's actually 3 tokens separated by a space, so: [email protected], %, [email protected] -> valid, discarded, valid
    But maybe we should handle this some other way, e.g., have a red chip with the "discarded" items cc @annapohorielova111

  2. Your right, I'll look into it. It's related to how I did the search for the username. (It's funny how the MM-50229 frustrates you as well 😛)

@kostaspt
Copy link
Contributor Author

kostaspt commented Feb 8, 2023

@furqanmlk Not sure if this is better, but beyond that is potentially a different UX direction.

Kapture.2023-02-08.at.15.53.23.mp4

This was referenced Feb 8, 2023
Merged
Closed
@kostaspt
Copy link
Contributor Author

kostaspt commented Feb 9, 2023

/update-branch

@furqanmlk furqanmlk self-requested a review February 27, 2023 15:09
@furqanmlk
Copy link
Contributor

/update-branch

@github-actions
Copy link

github-actions bot commented Feb 28, 2023 •
edited
Loading

Test Results

       1 files     815 suites   14m 16s ⏱️
6 930 tests 6 929 ✔️ 1 💤 0 ❌
7 106 runs  7 105 ✔️ 1 💤 0 ❌

Results for commit f109000.

♻️ This comment has been updated with latest results.

@furqanmlk
Copy link
Contributor

/e2e-test

@mattermost-build
Copy link
Contributor

Successfully triggered E2E testing!
GitLab pipeline | Test dashboard

@furqanmlk
Copy link
Contributor

@kostaspt
Can you please fix the conflict ?

Merge branch 'master' into MM-49821-ability-accept-pasting-multiple-e…
f109000 
…mail

# Conflicts:
#	components/widgets/inputs/users_emails_input.tsx
furqanmlk
furqanmlk approved these changes Mar 3, 2023
View reviewed changes
Copy link
Contributor

@furqanmlk furqanmlk left a comment

Choose a reason for hiding this comment

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

Working as expected

@kostaspt kostaspt removed the 3: QA Review Requires review by a QA tester label Mar 6, 2023
@kostaspt kostaspt changed the title [MM-49821] Ability to accept pasting multiple email addresses into the invite modal [MM-49821] Allow pasting multiple emails into invite modal Mar 6, 2023
@kostaspt kostaspt merged commit 230a0eb into master Mar 6, 2023
@kostaspt kostaspt deleted the MM-49821-ability-accept-pasting-multiple-email branch March 6, 2023 18:37
@mattermost-build mattermost-build removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Mar 6, 2023
@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 Mar 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mkraft mkraft mkraft approved these changes

@furqanmlk furqanmlk furqanmlk approved these changes

@laneycs laneycs laneycs approved these changes

@BenCookie95 BenCookie95 BenCookie95 approved these changes

Assignees
No one assigned
Labels
Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation Lifecycle/frozen release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@kostaspt @mattermost-build @mkraft @BenCookie95 @furqanmlk @mm-cloud-bot @laneycs @lieut-data @amyblais