[RO] Incorrect Deletion of Webhook Actions in Kibana Rules #159204
Conversation
jcger
added
Team:ResponseOps
jcger
changed the title
|
Pinging @elastic/response-ops (Team:ResponseOps) |
|
Could this be released in v8.8.2 ? |
Yes, I believe that's fine. I think it's possible the uuid could be different when the rule is retrieved again - the idea is that it just provides uniqueness and therefore a key, but the value doesn't need to persist long-term with the action. @ersin-erdal is that correct?
Yes, I think we should get this in 8.8.2 if possible. |
Yes i also think this is ok. Update request sends the uuid generated here to BE but that's also ok. |
Ya, the edit: apparently I was wrong, see two comments below from Ersin, so x-ed out my comment |
We used uuid for summarised alerts. And as summary feature is action based, we need uuid to detect which action is still being throttled or which action is removed from a rule. As we replace the whole rule on update, we would lost the uuids if UI doesn't return them. |
…rong-slack-connector
…ger/kibana into 158167-deleting-wrong-slack-connector
| ); | ||
|
|
||
| // check that the removed action is the right one | ||
| const doesExist = await find.existsByXpath(".//*[text()='myUniqueKey']"); |
There was a problem hiding this comment.
Couldn't think of a better way to check that the right action is removed. Open to suggestions
There was a problem hiding this comment.
In general, we try to use test subjects over xpath expressions where possible. In this case, the xpath way seems reasonable to me.
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
…59204) ## Summary Fixes elastic#158167 The issue occurred because we were using a number called "index" as the React key prop. When we removed the first element, the second one took its place, but React still showed the removed element. To fix this problem, we found a solution by using the uuid field that each action item has. We now generate it for each new action we create in the action form and use that as key I was told to add @pmuellr as you might know if we are missing something 🙇 ### Test: why xpath selector? I had to use the xpath selector to fix a problem we had. The problem was between two actions we set up. They look the same, but the body content is the only different (I've attached a screenshot for more details). We use a third party component for these actions. This component doesn't have any "value" attribute and doesn't add anything besides the HTML text. I tried to find other useful details but couldn't find any. The problem comes up when we try to delete one of the actions. To fix it, we needed to check if that component was missing. We already have tools that can look for missing components, but they don't work with the xpath selector. So, I added a new function that can do this. Now, we can use the xpath selector to look for missing components and fix the problem <details> <summary>See Screenshot</summary> <img src="https://github.com/elastic/kibana/assets/17549662/5447795d-0281-4847-aa85-76d0e5fdec3d"/> </details> ```[tasklist] - [x] Make sure that it's ok to generate the uuid - [x] Test - [x] Do we need to backport? Versions? ``` (cherry picked from commit 55bc8cf)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…9204) (#159443) # Backport This will backport the following commits from `main` to `8.8`: - [[RO] Incorrect Deletion of Webhook Actions in Kibana Rules (#159204)](#159204) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Julian Gernun","email":"17549662+jcger@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-06-12T08:19:20Z","message":"[RO] Incorrect Deletion of Webhook Actions in Kibana Rules (#159204)\n\n## Summary\r\n\r\nFixes #158167\r\n\r\nThe issue occurred because we were using a number called \"index\" as the\r\nReact key prop. When we removed the first element, the second one took\r\nits place, but React still showed the removed element.\r\n\r\nTo fix this problem, we found a solution by using the uuid field that\r\neach action item has. We now generate it for each new action we create\r\nin the action form and use that as key\r\n\r\nI was told to add @pmuellr as you might know if we are missing something\r\n🙇\r\n\r\n### Test: why xpath selector?\r\n\r\nI had to use the xpath selector to fix a problem we had. The problem was\r\nbetween two actions we set up. They look the same, but the body content\r\nis the only different (I've attached a screenshot for more details).\r\n\r\nWe use a third party component for these actions. This component doesn't\r\nhave any \"value\" attribute and doesn't add anything besides the HTML\r\ntext. I tried to find other useful details but couldn't find any.\r\n\r\nThe problem comes up when we try to delete one of the actions. To fix\r\nit, we needed to check if that component was missing. We already have\r\ntools that can look for missing components, but they don't work with the\r\nxpath selector. So, I added a new function that can do this. Now, we can\r\nuse the xpath selector to look for missing components and fix the\r\nproblem\r\n \r\n<details>\r\n<summary>See Screenshot</summary>\r\n<img\r\nsrc=\"https://github.com/elastic/kibana/assets/17549662/5447795d-0281-4847-aa85-76d0e5fdec3d\"/>\r\n</details>\r\n\r\n```[tasklist]\r\n- [x] Make sure that it's ok to generate the uuid\r\n- [x] Test\r\n- [x] Do we need to backport? Versions?\r\n```","sha":"55bc8cf567a437581d37e6e7d1380216667f34cd","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","v8.8.0","v8.9.0"],"number":159204,"url":"#159204 Incorrect Deletion of Webhook Actions in Kibana Rules (#159204)\n\n## Summary\r\n\r\nFixes #158167\r\n\r\nThe issue occurred because we were using a number called \"index\" as the\r\nReact key prop. When we removed the first element, the second one took\r\nits place, but React still showed the removed element.\r\n\r\nTo fix this problem, we found a solution by using the uuid field that\r\neach action item has. We now generate it for each new action we create\r\nin the action form and use that as key\r\n\r\nI was told to add @pmuellr as you might know if we are missing something\r\n🙇\r\n\r\n### Test: why xpath selector?\r\n\r\nI had to use the xpath selector to fix a problem we had. The problem was\r\nbetween two actions we set up. They look the same, but the body content\r\nis the only different (I've attached a screenshot for more details).\r\n\r\nWe use a third party component for these actions. This component doesn't\r\nhave any \"value\" attribute and doesn't add anything besides the HTML\r\ntext. I tried to find other useful details but couldn't find any.\r\n\r\nThe problem comes up when we try to delete one of the actions. To fix\r\nit, we needed to check if that component was missing. We already have\r\ntools that can look for missing components, but they don't work with the\r\nxpath selector. So, I added a new function that can do this. Now, we can\r\nuse the xpath selector to look for missing components and fix the\r\nproblem\r\n \r\n<details>\r\n<summary>See Screenshot</summary>\r\n<img\r\nsrc=\"https://github.com/elastic/kibana/assets/17549662/5447795d-0281-4847-aa85-76d0e5fdec3d\"/>\r\n</details>\r\n\r\n```[tasklist]\r\n- [x] Make sure that it's ok to generate the uuid\r\n- [x] Test\r\n- [x] Do we need to backport? Versions?\r\n```","sha":"55bc8cf567a437581d37e6e7d1380216667f34cd"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"#159204 Incorrect Deletion of Webhook Actions in Kibana Rules (#159204)\n\n## Summary\r\n\r\nFixes #158167\r\n\r\nThe issue occurred because we were using a number called \"index\" as the\r\nReact key prop. When we removed the first element, the second one took\r\nits place, but React still showed the removed element.\r\n\r\nTo fix this problem, we found a solution by using the uuid field that\r\neach action item has. We now generate it for each new action we create\r\nin the action form and use that as key\r\n\r\nI was told to add @pmuellr as you might know if we are missing something\r\n🙇\r\n\r\n### Test: why xpath selector?\r\n\r\nI had to use the xpath selector to fix a problem we had. The problem was\r\nbetween two actions we set up. They look the same, but the body content\r\nis the only different (I've attached a screenshot for more details).\r\n\r\nWe use a third party component for these actions. This component doesn't\r\nhave any \"value\" attribute and doesn't add anything besides the HTML\r\ntext. I tried to find other useful details but couldn't find any.\r\n\r\nThe problem comes up when we try to delete one of the actions. To fix\r\nit, we needed to check if that component was missing. We already have\r\ntools that can look for missing components, but they don't work with the\r\nxpath selector. So, I added a new function that can do this. Now, we can\r\nuse the xpath selector to look for missing components and fix the\r\nproblem\r\n \r\n<details>\r\n<summary>See Screenshot</summary>\r\n<img\r\nsrc=\"https://github.com/elastic/kibana/assets/17549662/5447795d-0281-4847-aa85-76d0e5fdec3d\"/>\r\n</details>\r\n\r\n```[tasklist]\r\n- [x] Make sure that it's ok to generate the uuid\r\n- [x] Test\r\n- [x] Do we need to backport? Versions?\r\n```","sha":"55bc8cf567a437581d37e6e7d1380216667f34cd"}}]}] BACKPORT--> Co-authored-by: Julian Gernun <17549662+jcger@users.noreply.github.com>
…59204) ## Summary Fixes elastic#158167 The issue occurred because we were using a number called "index" as the React key prop. When we removed the first element, the second one took its place, but React still showed the removed element. To fix this problem, we found a solution by using the uuid field that each action item has. We now generate it for each new action we create in the action form and use that as key I was told to add @pmuellr as you might know if we are missing something 🙇 ### Test: why xpath selector? I had to use the xpath selector to fix a problem we had. The problem was between two actions we set up. They look the same, but the body content is the only different (I've attached a screenshot for more details). We use a third party component for these actions. This component doesn't have any "value" attribute and doesn't add anything besides the HTML text. I tried to find other useful details but couldn't find any. The problem comes up when we try to delete one of the actions. To fix it, we needed to check if that component was missing. We already have tools that can look for missing components, but they don't work with the xpath selector. So, I added a new function that can do this. Now, we can use the xpath selector to look for missing components and fix the problem <details> <summary>See Screenshot</summary> <img src="https://github.com/elastic/kibana/assets/17549662/5447795d-0281-4847-aa85-76d0e5fdec3d"/> </details> ```[tasklist] - [x] Make sure that it's ok to generate the uuid - [x] Test - [x] Do we need to backport? Versions? ```
Summary
Fixes #158167
The issue occurred because we were using a number called "index" as the React key prop. When we removed the first element, the second one took its place, but React still showed the removed element.
To fix this problem, we found a solution by using the uuid field that each action item has. We now generate it for each new action we create in the action form and use that as key
I was told to add @pmuellr as you might know if we are missing something 🙇
Test: why xpath selector?
I had to use the xpath selector to fix a problem we had. The problem was between two actions we set up. They look the same, but the body content is the only different (I've attached a screenshot for more details).
We use a third party component for these actions. This component doesn't have any "value" attribute and doesn't add anything besides the HTML text. I tried to find other useful details but couldn't find any.
The problem comes up when we try to delete one of the actions. To fix it, we needed to check if that component was missing. We already have tools that can look for missing components, but they don't work with the xpath selector. So, I added a new function that can do this. Now, we can use the xpath selector to look for missing components and fix the problem
See Screenshot
Tasks