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.

Sign up for GitHub

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

Jump to bottom

Refactor email input handling to format comma-separated addresses #193128

Merged
merged 7 commits into from
Oct 2, 2024
Merged

Refactor email input handling to format comma-separated addresses #193128

merged 7 commits into from
Oct 2, 2024

Conversation

VriaA
Copy link
Contributor

@VriaA VriaA commented Sep 17, 2024 •
edited by kibanamachine
Loading

Summary

This pull request fixes #189968

  • Introduced getFormattedEmailOptions to split and trim comma-separated email values
  • Updated EuiComboBox to handle email entries for to, cc, and bcc fields
kibana_email_connector.mp4

@VriaA VriaA requested a review from a team as a code owner September 17, 2024 09:36
@VriaA VriaA force-pushed the fix/multiple-email-entries-on-paste branch from d0be951 to 04b7169 Compare September 17, 2024 10:11
@adcoelho adcoelho assigned adcoelho and unassigned adcoelho Sep 17, 2024
@jcger
Copy link
Contributor

jcger commented Sep 18, 2024

Thank you @VriaA for contributing to Kibana! At first glance the PR looks promising, would you be so kind to also add a test?

@jcger
Copy link
Contributor

jcger commented Sep 18, 2024

/ci

@jcger jcger added the Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) label Sep 18, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@VriaA VriaA force-pushed the fix/multiple-email-entries-on-paste branch from 04b7169 to 3198018 Compare September 21, 2024 11:48
@VriaA
Copy link
Contributor Author

VriaA commented Sep 21, 2024

Thank you for taking the time to review my PR @jcger. I've added the test as requested and refactored the function accordingly.

@jcger
Copy link
Contributor

jcger commented Sep 25, 2024

/ci

@jcger jcger added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Sep 25, 2024
@jcger
Copy link
Contributor

jcger commented Sep 25, 2024

/ci

@VriaA
Copy link
Contributor Author

VriaA commented Sep 25, 2024

@elasticmachine merge upstream

@jcger
Copy link
Contributor

jcger commented Sep 26, 2024

Hey @VriaA thank you for adding some unit tests, they look great. Unfortunately it's not enough as we need also to ensure that if the user pastes the email list, the form values are updated as expected. See an example here

kibana/x-pack/plugins/stack_connectors/public/connector_types/email/email_params.test.tsx

Line 38 in 7b76160

test('all params fields is rendered', async () => {
Something like pasting some emails separated by , and then checking the value should be enough

@VriaA
Copy link
Contributor Author

VriaA commented Sep 26, 2024

Thank you for the feedback, @jcger. I’ll check the example you provided and update the tests accordingly.

@VriaA VriaA force-pushed the fix/multiple-email-entries-on-paste branch 2 times, most recently from f435bab to 3f48386 Compare September 27, 2024 11:19
@VriaA
Refactor email input handling to format comma-separated addresses
877c51d 
- Introduced `getFormattedEmailOptions` to split and trim comma-separated email values
- Updated EuiComboBox to handle email entry for `to`, `cc`, and `bcc` fields

Signed-off-by: Oyelola Victoria <[email protected]>
@VriaA
add test and refactor getFormattedEmailOptions
9a0ef27 
Signed-off-by: Oyelola Victoria <[email protected]>
@VriaA VriaA force-pushed the fix/multiple-email-entries-on-paste branch from 3f48386 to 9a0ef27 Compare September 29, 2024 13:41
@jcger
Copy link
Contributor

jcger commented Sep 30, 2024

/ci

@VriaA
Copy link
Contributor Author

VriaA commented Sep 30, 2024

@elasticmachine merge upstream

jcger
jcger reviewed Oct 1, 2024
View reviewed changes
Copy link
Contributor

@jcger jcger left a comment

Choose a reason for hiding this comment

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

Thank you for adding the test I asked you for! Now I've done the actual code review, just a couple of comments, mostly nit picks but it will help us keeping the code cleaner

x-pack/plugins/stack_connectors/public/connector_types/email/email_params.test.tsx Outdated Show resolved Hide resolved
x-pack/plugins/stack_connectors/public/connector_types/email/email_params.test.tsx Outdated Show resolved Hide resolved
x-pack/plugins/stack_connectors/public/connector_types/email/email_params.test.tsx Outdated Show resolved Hide resolved
x-pack/plugins/stack_connectors/public/connector_types/email/email_params.tsx Outdated Show resolved Hide resolved
x-pack/plugins/stack_connectors/public/connector_types/email/email_params.tsx Outdated Show resolved Hide resolved
@VriaA
refactor function and test for formatting comma-separated emails for …
563806b 
…readability and best practices

Signed-off-by: Oyelola Victoria <[email protected]>
@VriaA
Copy link
Contributor Author

VriaA commented Oct 1, 2024

@elasticmachine merge upstream

@jcger
Copy link
Contributor

jcger commented Oct 2, 2024

/ci

@jcger jcger enabled auto-merge (squash) October 2, 2024 07:06
jcger
jcger approved these changes Oct 2, 2024
View reviewed changes
Copy link
Contributor

@jcger jcger left a comment

Choose a reason for hiding this comment

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

Thank you so much! I'll take it from here to make sure it gets merged

@jcger jcger added the v9.0.0 label Oct 2, 2024
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #30 / Serverless observability API Dataset Quality gets the data stream details returns "sizeBytes" correctly

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
stackConnectors 580.2KB 580.4KB +202.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@jcger
Copy link
Contributor

jcger commented Oct 2, 2024

@elasticmachine run docs-build

@jcger jcger merged commit 9174588 into elastic:main Oct 2, 2024
22 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11141385222

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 2, 2024
@VriaA
Refactor email input handling to format comma-separated addresses (el…
ae7ed4d 
…astic#193128)

## Summary
This pull request fixes elastic#189968

- Introduced `getFormattedEmailOptions` to split and trim
comma-separated email values
- Updated `EuiComboBox` to handle email entries for `to`, `cc`, and
`bcc` fields

https://github.com/user-attachments/assets/45a70132-8fd7-426e-81cf-62a6bf216408

---------

Signed-off-by: Oyelola Victoria <[email protected]>
Co-authored-by: Julian Gernun <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 9174588)
@kibanamachine kibanamachine mentioned this pull request Oct 2, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine mentioned this pull request Oct 2, 2024
Merged
kibanamachine added a commit that referenced this pull request Oct 2, 2024
@kibanamachine @VriaA
[8.x] Refactor email input handling to format comma-separated address…
d6edf80 
…es (#193128) (#194669)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Refactor email input handling to format comma-separated addresses
(#193128)](#193128)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Oyelola
Victoria","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-02T09:33:05Z","message":"Refactor
email input handling to format comma-separated addresses (#193128)\n\n##
Summary\r\nThis pull request fixes #189968 \r\n\r\n- Introduced
`getFormattedEmailOptions` to split and trim\r\ncomma-separated email
values\r\n- Updated `EuiComboBox` to handle email entries for `to`,
`cc`, and\r\n`bcc`
fields\r\n\r\n\r\nhttps://github.com/user-attachments/assets/45a70132-8fd7-426e-81cf-62a6bf216408\r\n\r\n---------\r\n\r\nSigned-off-by:
Oyelola Victoria <[email protected]>\r\nCo-authored-by: Julian
Gernun <[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"917458844b51b93d9a62f2e100099a70e1ea4842","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","💝community","v9.0.0","backport:prev-minor"],"title":"Refactor
email input handling to format comma-separated
addresses","number":193128,"url":"https://github.com/elastic/kibana/pull/193128","mergeCommit":{"message":"Refactor
email input handling to format comma-separated addresses (#193128)\n\n##
Summary\r\nThis pull request fixes #189968 \r\n\r\n- Introduced
`getFormattedEmailOptions` to split and trim\r\ncomma-separated email
values\r\n- Updated `EuiComboBox` to handle email entries for `to`,
`cc`, and\r\n`bcc`
fields\r\n\r\n\r\nhttps://github.com/user-attachments/assets/45a70132-8fd7-426e-81cf-62a6bf216408\r\n\r\n---------\r\n\r\nSigned-off-by:
Oyelola Victoria <[email protected]>\r\nCo-authored-by: Julian
Gernun <[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"917458844b51b93d9a62f2e100099a70e1ea4842"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193128","number":193128,"mergeCommit":{"message":"Refactor
email input handling to format comma-separated addresses (#193128)\n\n##
Summary\r\nThis pull request fixes #189968 \r\n\r\n- Introduced
`getFormattedEmailOptions` to split and trim\r\ncomma-separated email
values\r\n- Updated `EuiComboBox` to handle email entries for `to`,
`cc`, and\r\n`bcc`
fields\r\n\r\n\r\nhttps://github.com/user-attachments/assets/45a70132-8fd7-426e-81cf-62a6bf216408\r\n\r\n---------\r\n\r\nSigned-off-by:
Oyelola Victoria <[email protected]>\r\nCo-authored-by: Julian
Gernun <[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>","sha":"917458844b51b93d9a62f2e100099a70e1ea4842"}}]}]
BACKPORT-->

Co-authored-by: Oyelola Victoria <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcger jcger jcger approved these changes

Assignees
No one assigned
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) 💝community release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.16.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pasting with comma separated list of mail adresses should create multiple entries
6 participants
@VriaA @jcger @elasticmachine @kibana-ci @kibanamachine @adcoelho