Fixing bug where an error is displayed when saving a new scripted field #10820
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a new scripted field is saved, there is a moment where a mapping error validation message is displayed because there's a field with the same name. Added a `saving` flag that is used to no longer show the validation message in this situation.
Bargs
suggested changes
Mar 21, 2017
| @@ -57,6 +57,8 @@ uiModules | |||
|
|
|||
| self.cancel = redirectAway; | |||
| self.save = function () { | |||
| self.saving = true; | |||
There was a problem hiding this comment.
I'm always wary of flags like this since they can get stuck in an incorrect state. For instance if indexPattern.save fails on line 75 self.saving will be stuck as true.
Instead, what would you think of creating an array of existing field names on controller instantiation (around line 52) and then use that in our template's conditional instead of the live index pattern?
self.existingFieldNames = self.indexPattern.fields.map((field) => {
return field.name;
});
There was a problem hiding this comment.
Seems reasonable, I will make it so.
I've found myself using state flags to denote asynchronous actions and their current state a lot to disable buttons, etc. (especially with React/Flux) but you're right that in this scenario the lack of a .catch could potentially cause issues.
It's also probably worthwhile, in another PR, to add in a .catch so if an error is thrown that we display it to the user.
| @@ -16,7 +16,7 @@ | |||
| class="form-control" | |||
| data-test-subj="editorFieldName"> | |||
| </div> | |||
| <div ng-if="editor.creating && editor.indexPattern.fields.byName[editor.field.name]" class="hintbox"> | |||
There was a problem hiding this comment.
Here we could replace editor.indexPattern.fields.byName[editor.field.name] with editor.existingFieldNames.includes(editor.field.name)
Bargs
approved these changes
Mar 21, 2017
|
LGTM |
kobelb
added a commit
that referenced
this pull request
Apr 10, 2017
…ld (#10820) * Fixing bug where an error is displayed when saving a new scripted field When a new scripted field is saved, there is a moment where a mapping error validation message is displayed because there's a field with the same name. Added a `saving` flag that is used to no longer show the validation message in this situation. * Switching approach to copying fieldNames on instantiation
|
Backported 5.x - 4675c23 |
e40pud
added a commit
to e40pud/kibana
that referenced
this pull request
Nov 20, 2024
e40pud
added a commit
that referenced
this pull request
Nov 22, 2024
## Summary [Internal link](elastic/security-team#10820) to the feature details This is a very first version of the SIEM rules migrations UI functionality. The main goal is to setup and agree on a folder structure where the feature gonna live. Tests covering feature will follow in a separate PR (see [internal link](elastic/security-team#11232) for more details). The code follows the structure of prebuilt rules feature https://github.com/elastic/kibana/tree/main/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table and hidden behind `siemMigrationsEnabled` feature flag. ### Key UI changes * New "SIEM Rules Migrations." rules management sub-page * Navigation between different "finished" migrations * InMemory table with all the translations within the selected migration * Translation details preview flyout with `Translation` and `Overview` tabs * User cannot modify translations via UI ### Testing locally Enable the flag ``` xpack.securitySolution.enableExperimental: ['siemMigrationsEnabled'] ``` ### Screenshot https://github.com/user-attachments/assets/a5a7e777-c5f8-40b4-be1d-1bd07a2729ac
e40pud
added a commit
to e40pud/kibana
that referenced
this pull request
Nov 25, 2024
…ic#200978) ## Summary [Internal link](elastic/security-team#10820) to the feature details This is a very first version of the SIEM rules migrations UI functionality. The main goal is to setup and agree on a folder structure where the feature gonna live. Tests covering feature will follow in a separate PR (see [internal link](elastic/security-team#11232) for more details). The code follows the structure of prebuilt rules feature https://github.com/elastic/kibana/tree/main/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table and hidden behind `siemMigrationsEnabled` feature flag. ### Key UI changes * New "SIEM Rules Migrations." rules management sub-page * Navigation between different "finished" migrations * InMemory table with all the translations within the selected migration * Translation details preview flyout with `Translation` and `Overview` tabs * User cannot modify translations via UI ### Testing locally Enable the flag ``` xpack.securitySolution.enableExperimental: ['siemMigrationsEnabled'] ``` ### Screenshot https://github.com/user-attachments/assets/a5a7e777-c5f8-40b4-be1d-1bd07a2729ac (cherry picked from commit a627e01) # Conflicts: # .github/CODEOWNERS
e40pud
added a commit
that referenced
this pull request
Nov 25, 2024
… (#201545) # Backport This will backport the following commits from `main` to `8.x`: - [[Rules migration][UI] Basic rule migrations UI (#10820) (#200978)](#200978) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Ievgen Sorokopud","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-22T14:48:14Z","message":"[Rules migration][UI] Basic rule migrations UI (#10820) (#200978)\n\n## Summary\r\n\r\n[Internal link](https://github.com/elastic/security-team/issues/10820)\r\nto the feature details\r\n\r\nThis is a very first version of the SIEM rules migrations UI\r\nfunctionality. The main goal is to setup and agree on a folder structure\r\nwhere the feature gonna live. Tests covering feature will follow in a\r\nseparate PR (see [internal\r\nlink](elastic/security-team#11232) for more\r\ndetails).\r\n\r\nThe code follows the structure of prebuilt rules feature\r\nhttps://github.com/elastic/kibana/tree/main/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table\r\nand hidden behind `siemMigrationsEnabled` feature flag.\r\n\r\n### Key UI changes\r\n\r\n* New \"SIEM Rules Migrations.\" rules management sub-page\r\n* Navigation between different \"finished\" migrations\r\n* InMemory table with all the translations within the selected migration\r\n* Translation details preview flyout with `Translation` and `Overview`\r\ntabs\r\n* User cannot modify translations via UI\r\n\r\n### Testing locally\r\n\r\nEnable the flag\r\n\r\n```\r\nxpack.securitySolution.enableExperimental: ['siemMigrationsEnabled']\r\n```\r\n### Screenshot\r\n\r\n\r\nhttps://github.com/user-attachments/assets/a5a7e777-c5f8-40b4-be1d-1bd07a2729ac","sha":"a627e011a892e9eaa7ec234b7a08fc5572801bbc","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Threat Hunting","Team: SecuritySolution","backport:prev-minor"],"number":200978,"url":"https://github.com/elastic/kibana/pull/200978","mergeCommit":{"message":"[Rules migration][UI] Basic rule migrations UI (#10820) (#200978)\n\n## Summary\r\n\r\n[Internal link](https://github.com/elastic/security-team/issues/10820)\r\nto the feature details\r\n\r\nThis is a very first version of the SIEM rules migrations UI\r\nfunctionality. The main goal is to setup and agree on a folder structure\r\nwhere the feature gonna live. Tests covering feature will follow in a\r\nseparate PR (see [internal\r\nlink](elastic/security-team#11232) for more\r\ndetails).\r\n\r\nThe code follows the structure of prebuilt rules feature\r\nhttps://github.com/elastic/kibana/tree/main/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table\r\nand hidden behind `siemMigrationsEnabled` feature flag.\r\n\r\n### Key UI changes\r\n\r\n* New \"SIEM Rules Migrations.\" rules management sub-page\r\n* Navigation between different \"finished\" migrations\r\n* InMemory table with all the translations within the selected migration\r\n* Translation details preview flyout with `Translation` and `Overview`\r\ntabs\r\n* User cannot modify translations via UI\r\n\r\n### Testing locally\r\n\r\nEnable the flag\r\n\r\n```\r\nxpack.securitySolution.enableExperimental: ['siemMigrationsEnabled']\r\n```\r\n### Screenshot\r\n\r\n\r\nhttps://github.com/user-attachments/assets/a5a7e777-c5f8-40b4-be1d-1bd07a2729ac","sha":"a627e011a892e9eaa7ec234b7a08fc5572801bbc"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200978","number":200978,"mergeCommit":{"message":"[Rules migration][UI] Basic rule migrations UI (#10820) (#200978)\n\n## Summary\r\n\r\n[Internal link](https://github.com/elastic/security-team/issues/10820)\r\nto the feature details\r\n\r\nThis is a very first version of the SIEM rules migrations UI\r\nfunctionality. The main goal is to setup and agree on a folder structure\r\nwhere the feature gonna live. Tests covering feature will follow in a\r\nseparate PR (see [internal\r\nlink](elastic/security-team#11232) for more\r\ndetails).\r\n\r\nThe code follows the structure of prebuilt rules feature\r\nhttps://github.com/elastic/kibana/tree/main/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table\r\nand hidden behind `siemMigrationsEnabled` feature flag.\r\n\r\n### Key UI changes\r\n\r\n* New \"SIEM Rules Migrations.\" rules management sub-page\r\n* Navigation between different \"finished\" migrations\r\n* InMemory table with all the translations within the selected migration\r\n* Translation details preview flyout with `Translation` and `Overview`\r\ntabs\r\n* User cannot modify translations via UI\r\n\r\n### Testing locally\r\n\r\nEnable the flag\r\n\r\n```\r\nxpack.securitySolution.enableExperimental: ['siemMigrationsEnabled']\r\n```\r\n### Screenshot\r\n\r\n\r\nhttps://github.com/user-attachments/assets/a5a7e777-c5f8-40b4-be1d-1bd07a2729ac","sha":"a627e011a892e9eaa7ec234b7a08fc5572801bbc"}}]}] BACKPORT-->
paulinashakirova
pushed a commit
to paulinashakirova/kibana
that referenced
this pull request
Nov 26, 2024
…ic#200978) ## Summary [Internal link](elastic/security-team#10820) to the feature details This is a very first version of the SIEM rules migrations UI functionality. The main goal is to setup and agree on a folder structure where the feature gonna live. Tests covering feature will follow in a separate PR (see [internal link](elastic/security-team#11232) for more details). The code follows the structure of prebuilt rules feature https://github.com/elastic/kibana/tree/main/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table and hidden behind `siemMigrationsEnabled` feature flag. ### Key UI changes * New "SIEM Rules Migrations." rules management sub-page * Navigation between different "finished" migrations * InMemory table with all the translations within the selected migration * Translation details preview flyout with `Translation` and `Overview` tabs * User cannot modify translations via UI ### Testing locally Enable the flag ``` xpack.securitySolution.enableExperimental: ['siemMigrationsEnabled'] ``` ### Screenshot https://github.com/user-attachments/assets/a5a7e777-c5f8-40b4-be1d-1bd07a2729ac
CAWilson94
pushed a commit
to CAWilson94/kibana
that referenced
this pull request
Dec 12, 2024
…ic#200978) ## Summary [Internal link](elastic/security-team#10820) to the feature details This is a very first version of the SIEM rules migrations UI functionality. The main goal is to setup and agree on a folder structure where the feature gonna live. Tests covering feature will follow in a separate PR (see [internal link](elastic/security-team#11232) for more details). The code follows the structure of prebuilt rules feature https://github.com/elastic/kibana/tree/main/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/add_prebuilt_rules_table and hidden behind `siemMigrationsEnabled` feature flag. ### Key UI changes * New "SIEM Rules Migrations." rules management sub-page * Navigation between different "finished" migrations * InMemory table with all the translations within the selected migration * Translation details preview flyout with `Translation` and `Overview` tabs * User cannot modify translations via UI ### Testing locally Enable the flag ``` xpack.securitySolution.enableExperimental: ['siemMigrationsEnabled'] ``` ### Screenshot https://github.com/user-attachments/assets/a5a7e777-c5f8-40b4-be1d-1bd07a2729ac
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a new scripted field is saved, there is a moment where a mapping error validation message is displayed because there's a field with the same name. Added a
savingflag that is used to no longer show the validation message in this situation.Resolves #5826