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.

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

Allow empty spaces un Gsub processor #197815

Merged

Conversation

SoniaSanzV
Copy link
Contributor

@SoniaSanzV SoniaSanzV commented Oct 25, 2024

Closes #191920

Summary

In the pattern field of the GSUB processor, we were validating that the field is not empty. But for this case we must allow spaces as values. To fix this, I've added a second parameter trimString to the emptyField validator. If the parameter is set to true by default so the behavior of the validator doesn't change in all the fields that already use it. But when the parameter is set to false, as in this case, the entry value string is not trimmed, so it only validate to false if the field is actually empty. I also added some unit tests to make sure that this new condition doesn't break the validator.

GSUB.mov

Notes

In the video it can be seen that a banner flashes when saving. It is a bug that already happened before this one. I've opened an issue to address it.

@SoniaSanzV SoniaSanzV added 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 Feature:Ingest Node Pipelines Ingest node pipelines management backport:prev-major Backport to (8.x, 8.18, 8.17, 8.16) the previous major branch and other branches in development v8.17.0 labels Oct 25, 2024
@SoniaSanzV SoniaSanzV requested a review from a team as a code owner October 25, 2024 12:01
@elasticmachine
Copy link
Contributor

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

@SoniaSanzV SoniaSanzV self-assigned this Oct 25, 2024
@SoniaSanzV SoniaSanzV force-pushed the ingestPipelines/gsubProcessor_#191920 branch from 3f4230c to 4e9c771 Compare October 25, 2024 12:52
@SoniaSanzV SoniaSanzV enabled auto-merge (squash) October 25, 2024 15:41
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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
ingestPipelines 405.2KB 405.2KB +3.0B

Page load bundle

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

id before after diff
esUiShared 106.9KB 106.9KB +22.0B

History

cc @SoniaSanzV

@ElenaStoeva ElenaStoeva self-requested a review November 7, 2024 09:56
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! Changes lgtm, tested locally.

@SoniaSanzV SoniaSanzV merged commit 2f7f8e3 into elastic:main Nov 7, 2024
22 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.15, 8.16, 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 7, 2024
Closes [elastic#191920](elastic#191920)

## Summary
In the pattern field of the GSUB processor, we were validating that the
field is not empty. But for this case we must allow spaces as values. To
fix this, I've added a second parameter `trimString` to the `emptyField`
validator. If the parameter is set to true by default so the behavior of
the validator doesn't change in all the fields that already use it. But
when the parameter is set to false, as in this case, the entry value
string is not trimmed, so it only validate to false if the field is
actually empty. I also added some unit tests to make sure that this new
condition doesn't break the validator.

https://github.com/user-attachments/assets/049f7424-00c6-4bb2-ab89-2ce158ac4c4e

#### Notes
In the video it can be seen that a banner flashes when saving. It is a
bug that already happened before this one. I've opened [an
issue](elastic#197810) to address it.

(cherry picked from commit 2f7f8e3)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 7, 2024
Closes [elastic#191920](elastic#191920)

## Summary
In the pattern field of the GSUB processor, we were validating that the
field is not empty. But for this case we must allow spaces as values. To
fix this, I've added a second parameter `trimString` to the `emptyField`
validator. If the parameter is set to true by default so the behavior of
the validator doesn't change in all the fields that already use it. But
when the parameter is set to false, as in this case, the entry value
string is not trimmed, so it only validate to false if the field is
actually empty. I also added some unit tests to make sure that this new
condition doesn't break the validator.

https://github.com/user-attachments/assets/049f7424-00c6-4bb2-ab89-2ce158ac4c4e

#### Notes
In the video it can be seen that a banner flashes when saving. It is a
bug that already happened before this one. I've opened [an
issue](elastic#197810) to address it.

(cherry picked from commit 2f7f8e3)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 7, 2024
Closes [elastic#191920](elastic#191920)

## Summary
In the pattern field of the GSUB processor, we were validating that the
field is not empty. But for this case we must allow spaces as values. To
fix this, I've added a second parameter `trimString` to the `emptyField`
validator. If the parameter is set to true by default so the behavior of
the validator doesn't change in all the fields that already use it. But
when the parameter is set to false, as in this case, the entry value
string is not trimmed, so it only validate to false if the field is
actually empty. I also added some unit tests to make sure that this new
condition doesn't break the validator.

https://github.com/user-attachments/assets/049f7424-00c6-4bb2-ab89-2ce158ac4c4e

#### Notes
In the video it can be seen that a banner flashes when saving. It is a
bug that already happened before this one. I've opened [an
issue](elastic#197810) to address it.

(cherry picked from commit 2f7f8e3)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.15
8.16
8.x

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

Questions ?

Please refer to the Backport tool documentation

@ElenaStoeva ElenaStoeva added backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) and removed backport:prev-major Backport to (8.x, 8.18, 8.17, 8.16) the previous major branch and other branches in development labels Nov 7, 2024
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 7, 2024
Closes [elastic#191920](elastic#191920)

## Summary
In the pattern field of the GSUB processor, we were validating that the
field is not empty. But for this case we must allow spaces as values. To
fix this, I've added a second parameter `trimString` to the `emptyField`
validator. If the parameter is set to true by default so the behavior of
the validator doesn't change in all the fields that already use it. But
when the parameter is set to false, as in this case, the entry value
string is not trimmed, so it only validate to false if the field is
actually empty. I also added some unit tests to make sure that this new
condition doesn't break the validator.

https://github.com/user-attachments/assets/049f7424-00c6-4bb2-ab89-2ce158ac4c4e

#### Notes
In the video it can be seen that a banner flashes when saving. It is a
bug that already happened before this one. I've opened [an
issue](elastic#197810) to address it.

(cherry picked from commit 2f7f8e3)
@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 added a commit that referenced this pull request Nov 7, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [Allow empty spaces un Gsub processor
(#197815)](#197815)

<!--- 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-11-07T11:09:06Z","message":"Allow
empty spaces un Gsub processor (#197815)\n\nCloses
[#191920](https://github.com/elastic/kibana/issues/191920)\r\n\r\n##
Summary\r\nIn the pattern field of the GSUB processor, we were
validating that the\r\nfield is not empty. But for this case we must
allow spaces as values. To\r\nfix this, I've added a second parameter
`trimString` to the `emptyField`\r\nvalidator. If the parameter is set
to true by default so the behavior of\r\nthe validator doesn't change in
all the fields that already use it. But\r\nwhen the parameter is set to
false, as in this case, the entry value\r\nstring is not trimmed, so it
only validate to false if the field is\r\nactually empty. I also added
some unit tests to make sure that this new\r\ncondition doesn't break
the
validator.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/049f7424-00c6-4bb2-ab89-2ce158ac4c4e\r\n\r\n\r\n\r\n####
Notes\r\nIn the video it can be seen that a banner flashes when saving.
It is a\r\nbug that already happened before this one. I've opened
[an\r\nissue](#197810) to
address
it.","sha":"2f7f8e34db1fe38cc3a587a753533992398df691","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Kibana
Management","release_note:skip","v9.0.0","Feature:Ingest Node
Pipelines","backport:prev-major","v8.17.0"],"title":"Allow empty spaces
un Gsub
processor","number":197815,"url":"https://github.com/elastic/kibana/pull/197815","mergeCommit":{"message":"Allow
empty spaces un Gsub processor (#197815)\n\nCloses
[#191920](https://github.com/elastic/kibana/issues/191920)\r\n\r\n##
Summary\r\nIn the pattern field of the GSUB processor, we were
validating that the\r\nfield is not empty. But for this case we must
allow spaces as values. To\r\nfix this, I've added a second parameter
`trimString` to the `emptyField`\r\nvalidator. If the parameter is set
to true by default so the behavior of\r\nthe validator doesn't change in
all the fields that already use it. But\r\nwhen the parameter is set to
false, as in this case, the entry value\r\nstring is not trimmed, so it
only validate to false if the field is\r\nactually empty. I also added
some unit tests to make sure that this new\r\ncondition doesn't break
the
validator.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/049f7424-00c6-4bb2-ab89-2ce158ac4c4e\r\n\r\n\r\n\r\n####
Notes\r\nIn the video it can be seen that a banner flashes when saving.
It is a\r\nbug that already happened before this one. I've opened
[an\r\nissue](#197810) to
address
it.","sha":"2f7f8e34db1fe38cc3a587a753533992398df691"}},"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/197815","number":197815,"mergeCommit":{"message":"Allow
empty spaces un Gsub processor (#197815)\n\nCloses
[#191920](https://github.com/elastic/kibana/issues/191920)\r\n\r\n##
Summary\r\nIn the pattern field of the GSUB processor, we were
validating that the\r\nfield is not empty. But for this case we must
allow spaces as values. To\r\nfix this, I've added a second parameter
`trimString` to the `emptyField`\r\nvalidator. If the parameter is set
to true by default so the behavior of\r\nthe validator doesn't change in
all the fields that already use it. But\r\nwhen the parameter is set to
false, as in this case, the entry value\r\nstring is not trimmed, so it
only validate to false if the field is\r\nactually empty. I also added
some unit tests to make sure that this new\r\ncondition doesn't break
the
validator.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/049f7424-00c6-4bb2-ab89-2ce158ac4c4e\r\n\r\n\r\n\r\n####
Notes\r\nIn the video it can be seen that a banner flashes when saving.
It is a\r\nbug that already happened before this one. I've opened
[an\r\nissue](#197810) to
address
it.","sha":"2f7f8e34db1fe38cc3a587a753533992398df691"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Sonia Sanz Vivas <[email protected]>
mbondyra pushed a commit to mbondyra/kibana that referenced this pull request Nov 8, 2024
Closes [elastic#191920](elastic#191920)

## Summary
In the pattern field of the GSUB processor, we were validating that the
field is not empty. But for this case we must allow spaces as values. To
fix this, I've added a second parameter `trimString` to the `emptyField`
validator. If the parameter is set to true by default so the behavior of
the validator doesn't change in all the fields that already use it. But
when the parameter is set to false, as in this case, the entry value
string is not trimmed, so it only validate to false if the field is
actually empty. I also added some unit tests to make sure that this new
condition doesn't break the validator.


https://github.com/user-attachments/assets/049f7424-00c6-4bb2-ab89-2ce158ac4c4e



#### Notes
In the video it can be seen that a banner flashes when saving. It is a
bug that already happened before this one. I've opened [an
issue](elastic#197810) to address it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) Feature:Ingest Node Pipelines Ingest node pipelines management 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.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GSUB Processor in UI cannot deal with spaces as values.
4 participants