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

Not allow # in index template name #195776

Merged
merged 7 commits into from
Oct 16, 2024
Merged

Not allow # in index template name #195776

merged 7 commits into from
Oct 16, 2024

Conversation

SoniaSanzV
Copy link
Contributor

@SoniaSanzV SoniaSanzV commented Oct 10, 2024 •
edited by kibanamachine
Loading

Closes #81870

Summary

When creating a new Template, it did not prevent entering a #, but when saving the template the user received an error. The hash has been added to the list of invalid characters for the name.

81870.mov

@SoniaSanzV SoniaSanzV requested a review from a team as a code owner October 10, 2024 13:58
@sabarasaba sabarasaba added Feature:Index Management Index and index templates UI Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v9.0.0 backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.16.0 labels Oct 10, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-management (Team:Kibana Management)

sabarasaba
sabarasaba reviewed Oct 11, 2024
View reviewed changes
Copy link
Member

@sabarasaba sabarasaba left a comment

Choose a reason for hiding this comment

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

Thanks for patching this up @SoniaSanzV! Code changes lgtm, tested locally.

I couldnt find any existing tests associated with this specific form validation, do you think is worth perhaps adding a simple one for doing that? 🤔 Perhaps is also a good oportunity to figure out what the process for adding tests looks like

@SoniaSanzV
Copy link
Contributor Author

Thanks for patching this up @SoniaSanzV! Code changes lgtm, tested locally.

I couldnt find any existing tests associated with this specific form validation, do you think is worth perhaps adding a simple one for doing that? 🤔 Perhaps is also a good oportunity to figure out what the process for adding tests looks like

Sure! I can do that!

@SoniaSanzV
Copy link
Contributor Author

SoniaSanzV commented Oct 15, 2024 •
edited
Loading

I've been working on the test for a couple of days but, after talking to @ElenaStoeva we agreed that a simple unit test that only adds a character to an existing functionality does not worth all this time. The problem is that when you add a value to the input using the testing library, the getErrorsMessages() method doesn't received the failures from the validations.

ElenaStoeva
ElenaStoeva approved these changes Oct 15, 2024
View reviewed changes
Copy link
Contributor

@ElenaStoeva ElenaStoeva left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @SoniaSanzV, LGTM!

@SoniaSanzV SoniaSanzV enabled auto-merge (squash) October 15, 2024 15:59
@SoniaSanzV
Copy link
Contributor Author

/ci

@SoniaSanzV SoniaSanzV merged commit 4339f84 into elastic:main Oct 16, 2024
21 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #55 / Entity Analytics - Risk Engine @ess @serverless @serverlessQA init_and_status_apis status api should disable / enable risk engine

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
indexManagement 45.5KB 45.6KB +4.0B

History

cc @SoniaSanzV

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 16, 2024
@SoniaSanzV
Not allow # in index template name (elastic#195776)
96112d4 
Closes [elastic#81870](elastic#81870)

## Summary

When creating a new Template, it did not prevent entering a #, but when
saving the template the user received an error. The hash has been added
to the list of invalid characters for the name.

https://github.com/user-attachments/assets/2b59d245-c96f-4215-ad89-a3201bef5e94
(cherry picked from commit 4339f84)
@kibanamachine kibanamachine mentioned this pull request Oct 16, 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

@SoniaSanzV SoniaSanzV deleted the #81870 branch October 16, 2024 07:28
kibanamachine added a commit that referenced this pull request Oct 16, 2024
@kibanamachine @SoniaSanzV
[8.x] Not allow # in index template name (#195776) (#196485)
e061949 
# Backport

This will backport the following commits from `main` to `8.x`:
- [Not allow # in index template name
(#195776)](#195776)

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

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

<!--BACKPORT [{"author":{"name":"Sonia Sanz
Vivas","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-16T07:16:36Z","message":"Not
allow # in index template name (#195776)\n\nCloses
[#81870](https://github.com/elastic/kibana/issues/81870)\r\n\r\n##
Summary\r\n\r\nWhen creating a new Template, it did not prevent entering
a #, but when\r\nsaving the template the user received an error. The
hash has been added\r\nto the list of invalid characters for the
name.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/2b59d245-c96f-4215-ad89-a3201bef5e94","sha":"4339f8465720c507351a2e65a6fe881a10dcd938","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Index
Management","Team:Kibana
Management","release_note:skip","v9.0.0","backport:prev-minor","v8.16.0"],"title":"Not
allow # in index template
name","number":195776,"url":"https://github.com/elastic/kibana/pull/195776","mergeCommit":{"message":"Not
allow # in index template name (#195776)\n\nCloses
[#81870](https://github.com/elastic/kibana/issues/81870)\r\n\r\n##
Summary\r\n\r\nWhen creating a new Template, it did not prevent entering
a #, but when\r\nsaving the template the user received an error. The
hash has been added\r\nto the list of invalid characters for the
name.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/2b59d245-c96f-4215-ad89-a3201bef5e94","sha":"4339f8465720c507351a2e65a6fe881a10dcd938"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195776","number":195776,"mergeCommit":{"message":"Not
allow # in index template name (#195776)\n\nCloses
[#81870](https://github.com/elastic/kibana/issues/81870)\r\n\r\n##
Summary\r\n\r\nWhen creating a new Template, it did not prevent entering
a #, but when\r\nsaving the template the user received an error. The
hash has been added\r\nto the list of invalid characters for the
name.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/2b59d245-c96f-4215-ad89-a3201bef5e94","sha":"4339f8465720c507351a2e65a6fe881a10dcd938"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

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

@sabarasaba sabarasaba sabarasaba left review comments

@ElenaStoeva ElenaStoeva ElenaStoeva approved these changes

Assignees

@SoniaSanzV SoniaSanzV

Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Feature:Index Management Index and index templates UI release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.16.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Index Management][Index Templates] Template should not allow # in the name
5 participants
@SoniaSanzV @elasticmachine @kibanamachine @sabarasaba @ElenaStoeva