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

[Alerting] Edit alert should show and update all actions with deleted connectors #86838

Merged

Conversation

ymao1
Copy link
Contributor

@ymao1 ymao1 commented Dec 22, 2020

Resolves #86695

Summary

Fixed bug where not all actions using a deleted connector were showing up on the "Edit Alert" flyout. Updated behavior when creating new connector from the broken connector callout so that all actions using the broken connector are updated to use the new connector instead of just the action you clicked on.

To verify:

  1. Create a connector.
  2. Create an alert with multiple actions that use the connector from (1)
  3. Delete the connector from (1)
  4. Edit the alert from (2)
  5. Verify that multiple action sections show up, one for each of the actions that use the deleted connector
  6. Verify that the Save button is enabled if you remove all the actions for the deleted connector
  7. Verify that if you click "Create new connector" from one of the actions for the deleted connector, the new connector is applied to all the actions that used the deleted connector and the Save button is enabled.

Checklist

Delete any items that are not applicable to this PR.

@ymao1 ymao1 self-assigned this Dec 22, 2020
@ymao1 ymao1 added Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.12.0 v8.0.0 labels Dec 23, 2020
@ymao1 ymao1 marked this pull request as ready for review December 23, 2020 15:38
@ymao1 ymao1 requested a review from a team as a code owner December 23, 2020 15:38
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM and described bug is disappeared.

@ymao1
Copy link
Contributor Author

ymao1 commented Dec 23, 2020

If #86837 is merged first, new functional test will need to be updated.

@gmmorris
Copy link
Contributor

It was going so well :)
But then when I tried to edit the alert I clicked on the button to create a new connector (in place of the deleted one), filled the pop-up form and hit save - BANG, it exploded. 😬

Screenshot 2020-12-31 at 11 45 36

@gmmorris
Copy link
Contributor

Spotted another hint on the server side:

Screenshot 2020-12-31 at 11 49 59

@gmmorris
Copy link
Contributor

Taking a closer look to try and figure out the flow I'm getting the feeling ActionForm does too many things and should probably be broken down into smaller components.
It feels like we have some weird state flow with setActiveActionItem and now setBrokenActionItemId, that (unless I'm misunderstanding the flow) could cause some confusion when you have multiple broken connectors etc.

Is it worth taking this opportunity to break it down? 🤔 It isn't a blocker, just feel it might make maintaining this easier going forward.

@ymao1
Copy link
Contributor Author

ymao1 commented Jan 4, 2021

@elasticmachine merge upstream

@ymao1
Copy link
Contributor Author

ymao1 commented Jan 4, 2021

Taking a closer look to try and figure out the flow I'm getting the feeling ActionForm does too many things and should probably be broken down into smaller components.
It feels like we have some weird state flow with setActiveActionItem and now setBrokenActionItemId, that (unless I'm misunderstanding the flow) could cause some confusion when you have multiple broken connectors etc.

Is it worth taking this opportunity to break it down? 🤔 It isn't a blocker, just feel it might make maintaining this easier going forward.

@gmmorris See if this commit makes things clearer. Instead of calling out a "broken action item", I changed the existing update logic to take in an array of indices to update when the Add Connector is saved.

I was also unable to reproduce the errors that you mentioned, so if you have more specific instructions for reproducing them, please let me know. I was just testing with the server log and index actions....were you using a more complicated connector?

@ymao1
Copy link
Contributor Author

ymao1 commented Jan 5, 2021

@elasticmachine merge upstream

@ymao1
Copy link
Contributor Author

ymao1 commented Jan 5, 2021

@elasticmachine merge upstream

@gmmorris
Copy link
Contributor

gmmorris commented Jan 6, 2021

Taking a closer look to try and figure out the flow I'm getting the feeling ActionForm does too many things and should probably be broken down into smaller components.
It feels like we have some weird state flow with setActiveActionItem and now setBrokenActionItemId, that (unless I'm misunderstanding the flow) could cause some confusion when you have multiple broken connectors etc.
Is it worth taking this opportunity to break it down? 🤔 It isn't a blocker, just feel it might make maintaining this easier going forward.

@gmmorris See if this commit makes things clearer. Instead of calling out a "broken action item", I changed the existing update logic to take in an array of indices to update when the Add Connector is saved.

I was also unable to reproduce the errors that you mentioned, so if you have more specific instructions for reproducing them, please let me know. I was just testing with the server log and index actions....were you using a more complicated connector?

Yeah, that does look clearer... I still think it can br broken down into smaller components but this does feel clearer, so that can wait to a future commit. 👍

@gmmorris
Copy link
Contributor

gmmorris commented Jan 6, 2021

I couldn't recreate the errors I saw, butI have found an edge case which seems wrong.
I created two Server Logs and one webhook, created an alert using all three connectors then deleted the webhook and one Server Log.

When I opened the alert to edit I got this:
Screenshot 2021-01-06 at 17 23 37

Seeing as there is a Server Log connector - I should be able to select that one, no? It only offers me the option of creating a new one.
I tried creating a new one and it worked... then I could chose the existing one. But I shouldn't have to do that.

Copy link
Contributor

@gmmorris gmmorris left a comment

Choose a reason for hiding this comment

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

LGTM except for the inability to select an existing Connector in place of the one I had was deleted.

@ymao1
Copy link
Contributor Author

ymao1 commented Jan 6, 2021

I couldn't recreate the errors I saw, butI have found an edge case which seems wrong.
I created two Server Logs and one webhook, created an alert using all three connectors then deleted the webhook and one Server Log.

When I opened the alert to edit I got this:
Screenshot 2021-01-06 at 17 23 37

Seeing as there is a Server Log connector - I should be able to select that one, no? It only offers me the option of creating a new one.
I tried creating a new one and it worked... then I could chose the existing one. But I shouldn't have to do that.

Since this was existing behavior and not introduced by this PR, I have created this issue to capture this bug

@ymao1
Copy link
Contributor Author

ymao1 commented Jan 6, 2021

@elasticmachine merge upstream

@kibanamachine
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
triggersActionsUi 1.5MB 1.5MB +52.0B

Distributable file count

id before after diff
default 47256 48019 +763

History

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

@ymao1 ymao1 merged commit ff2d0f4 into elastic:master Jan 6, 2021
ymao1 added a commit to ymao1/kibana that referenced this pull request Jan 6, 2021
… connectors (elastic#86838)

* Showing all broken connectors and updating all matching broken connectors on new connector create

* Adding unit test

* Adding functional test

* Fixing functional test

* Simplifying logic

* Fixing functional test

* Fixing functional test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
ymao1 added a commit that referenced this pull request Jan 6, 2021
… connectors (#86838) (#87572)

* Showing all broken connectors and updating all matching broken connectors on new connector create

* Adding unit test

* Adding functional test

* Fixing functional test

* Simplifying logic

* Fixing functional test

* Fixing functional test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@mikecote mikecote added release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes labels Feb 2, 2021
@ymao1 ymao1 deleted the alerting/multiple-broken-connector-actions branch February 4, 2021 15:23
jcger added a commit that referenced this pull request Apr 25, 2024
## Summary

We were overwriting the `actionTypeId` of the first action in the
"create connector" callback. The value assigned was the `actionTypeId`
of the newly created action, meaning that we would have converted the
first action to be the same as the second one. This fix changes the
`actionTypeId` not for the first option but for all current
`activeActionItem.indices`.

Previously, we did add that override to fix a bug related to the slack
connector #155722. Its test
should cover us from breaking it back again. I did test it manually just
in case and it seems to be working still. Feel free to test it too.

Also, if you are wondering why `activeActionItems.indices` is an array
of numbers. The user might be using the same connector in more than one
action and then delete the connector. In case this happens, and the user
clicks on "edit rule", they will be able to restore both actions by just
creating the connector once. In order to be able to restore all affected
actions, their index is being stored as a number[]. More info here
#86838

Closes #181407

---------

Co-authored-by: Antonio <antoniodcoelho@gmail.com>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 25, 2024
…c#181604)

## Summary

We were overwriting the `actionTypeId` of the first action in the
"create connector" callback. The value assigned was the `actionTypeId`
of the newly created action, meaning that we would have converted the
first action to be the same as the second one. This fix changes the
`actionTypeId` not for the first option but for all current
`activeActionItem.indices`.

Previously, we did add that override to fix a bug related to the slack
connector elastic#155722. Its test
should cover us from breaking it back again. I did test it manually just
in case and it seems to be working still. Feel free to test it too.

Also, if you are wondering why `activeActionItems.indices` is an array
of numbers. The user might be using the same connector in more than one
action and then delete the connector. In case this happens, and the user
clicks on "edit rule", they will be able to restore both actions by just
creating the connector once. In order to be able to restore all affected
actions, their index is being stored as a number[]. More info here
elastic#86838

Closes elastic#181407

---------

Co-authored-by: Antonio <antoniodcoelho@gmail.com>
(cherry picked from commit 60c6cdb)
kibanamachine added a commit that referenced this pull request Apr 25, 2024
…181604) (#181691)

# Backport

This will backport the following commits from `main` to `8.14`:
- [[MGMTEX] Fix action data override when adding a second action
(#181604)](#181604)

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

### 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":"2024-04-25T10:42:28Z","message":"[MGMTEX]
Fix action data override when adding a second action (#181604)\n\n##
Summary\r\n\r\nWe were overwriting the `actionTypeId` of the first
action in the\r\n\"create connector\" callback. The value assigned was
the `actionTypeId`\r\nof the newly created action, meaning that we would
have converted the\r\nfirst action to be the same as the second one.
This fix changes the\r\n`actionTypeId` not for the first option but for
all current\r\n`activeActionItem.indices`.\r\n\r\nPreviously, we did add
that override to fix a bug related to the slack\r\nconnector
#155722. Its test\r\nshould
cover us from breaking it back again. I did test it manually just\r\nin
case and it seems to be working still. Feel free to test it
too.\r\n\r\nAlso, if you are wondering why `activeActionItems.indices`
is an array\r\nof numbers. The user might be using the same connector in
more than one\r\naction and then delete the connector. In case this
happens, and the user\r\nclicks on \"edit rule\", they will be able to
restore both actions by just\r\ncreating the connector once. In order to
be able to restore all affected\r\nactions, their index is being stored
as a number[]. More info
here\r\nhttps://github.com//pull/86838\r\n\r\nCloses
https://github.com/elastic/kibana/issues/181407\r\n\r\n---------\r\n\r\nCo-authored-by:
Antonio
<antoniodcoelho@gmail.com>","sha":"60c6cdb9985570a6f58bbf3860539de64ace9aa6","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","Feature:Alerting/RuleActions","v8.13.0","v8.14.0","v8.15.0"],"title":"[MGMTEX]
Fix action data override when adding a second
action","number":181604,"url":"https://github.com/elastic/kibana/pull/181604","mergeCommit":{"message":"[MGMTEX]
Fix action data override when adding a second action (#181604)\n\n##
Summary\r\n\r\nWe were overwriting the `actionTypeId` of the first
action in the\r\n\"create connector\" callback. The value assigned was
the `actionTypeId`\r\nof the newly created action, meaning that we would
have converted the\r\nfirst action to be the same as the second one.
This fix changes the\r\n`actionTypeId` not for the first option but for
all current\r\n`activeActionItem.indices`.\r\n\r\nPreviously, we did add
that override to fix a bug related to the slack\r\nconnector
#155722. Its test\r\nshould
cover us from breaking it back again. I did test it manually just\r\nin
case and it seems to be working still. Feel free to test it
too.\r\n\r\nAlso, if you are wondering why `activeActionItems.indices`
is an array\r\nof numbers. The user might be using the same connector in
more than one\r\naction and then delete the connector. In case this
happens, and the user\r\nclicks on \"edit rule\", they will be able to
restore both actions by just\r\ncreating the connector once. In order to
be able to restore all affected\r\nactions, their index is being stored
as a number[]. More info
here\r\nhttps://github.com//pull/86838\r\n\r\nCloses
https://github.com/elastic/kibana/issues/181407\r\n\r\n---------\r\n\r\nCo-authored-by:
Antonio
<antoniodcoelho@gmail.com>","sha":"60c6cdb9985570a6f58bbf3860539de64ace9aa6"}},"sourceBranch":"main","suggestedTargetBranches":["8.13","8.14"],"targetPullRequestStates":[{"branch":"8.13","label":"v8.13.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.14","label":"v8.14.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/181604","number":181604,"mergeCommit":{"message":"[MGMTEX]
Fix action data override when adding a second action (#181604)\n\n##
Summary\r\n\r\nWe were overwriting the `actionTypeId` of the first
action in the\r\n\"create connector\" callback. The value assigned was
the `actionTypeId`\r\nof the newly created action, meaning that we would
have converted the\r\nfirst action to be the same as the second one.
This fix changes the\r\n`actionTypeId` not for the first option but for
all current\r\n`activeActionItem.indices`.\r\n\r\nPreviously, we did add
that override to fix a bug related to the slack\r\nconnector
#155722. Its test\r\nshould
cover us from breaking it back again. I did test it manually just\r\nin
case and it seems to be working still. Feel free to test it
too.\r\n\r\nAlso, if you are wondering why `activeActionItems.indices`
is an array\r\nof numbers. The user might be using the same connector in
more than one\r\naction and then delete the connector. In case this
happens, and the user\r\nclicks on \"edit rule\", they will be able to
restore both actions by just\r\ncreating the connector once. In order to
be able to restore all affected\r\nactions, their index is being stored
as a number[]. More info
here\r\nhttps://github.com//pull/86838\r\n\r\nCloses
https://github.com/elastic/kibana/issues/181407\r\n\r\n---------\r\n\r\nCo-authored-by:
Antonio
<antoniodcoelho@gmail.com>","sha":"60c6cdb9985570a6f58bbf3860539de64ace9aa6"}}]}]
BACKPORT-->

Co-authored-by: Julian Gernun <17549662+jcger@users.noreply.github.com>
jcger added a commit to jcger/kibana that referenced this pull request Apr 25, 2024
…c#181604)

## Summary

We were overwriting the `actionTypeId` of the first action in the
"create connector" callback. The value assigned was the `actionTypeId`
of the newly created action, meaning that we would have converted the
first action to be the same as the second one. This fix changes the
`actionTypeId` not for the first option but for all current
`activeActionItem.indices`.

Previously, we did add that override to fix a bug related to the slack
connector elastic#155722. Its test
should cover us from breaking it back again. I did test it manually just
in case and it seems to be working still. Feel free to test it too.

Also, if you are wondering why `activeActionItems.indices` is an array
of numbers. The user might be using the same connector in more than one
action and then delete the connector. In case this happens, and the user
clicks on "edit rule", they will be able to restore both actions by just
creating the connector once. In order to be able to restore all affected
actions, their index is being stored as a number[]. More info here
elastic#86838

Closes elastic#181407

---------

Co-authored-by: Antonio <antoniodcoelho@gmail.com>
(cherry picked from commit 60c6cdb)

# Conflicts:
#	x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alert_create_flyout.ts
jcger added a commit that referenced this pull request Apr 25, 2024
…181604) (#181721)

# Backport

This will backport the following commits from `main` to `8.13`:
- [[MGMTEX] Fix action data override when adding a second action
(#181604)](#181604)

<!--- Backport version: 8.9.8 -->

### 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":"2024-04-25T10:42:28Z","message":"[MGMTEX]
Fix action data override when adding a second action (#181604)\n\n##
Summary\r\n\r\nWe were overwriting the `actionTypeId` of the first
action in the\r\n\"create connector\" callback. The value assigned was
the `actionTypeId`\r\nof the newly created action, meaning that we would
have converted the\r\nfirst action to be the same as the second one.
This fix changes the\r\n`actionTypeId` not for the first option but for
all current\r\n`activeActionItem.indices`.\r\n\r\nPreviously, we did add
that override to fix a bug related to the slack\r\nconnector
#155722. Its test\r\nshould
cover us from breaking it back again. I did test it manually just\r\nin
case and it seems to be working still. Feel free to test it
too.\r\n\r\nAlso, if you are wondering why `activeActionItems.indices`
is an array\r\nof numbers. The user might be using the same connector in
more than one\r\naction and then delete the connector. In case this
happens, and the user\r\nclicks on \"edit rule\", they will be able to
restore both actions by just\r\ncreating the connector once. In order to
be able to restore all affected\r\nactions, their index is being stored
as a number[]. More info
here\r\nhttps://github.com//pull/86838\r\n\r\nCloses
https://github.com/elastic/kibana/issues/181407\r\n\r\n---------\r\n\r\nCo-authored-by:
Antonio
<antoniodcoelho@gmail.com>","sha":"60c6cdb9985570a6f58bbf3860539de64ace9aa6","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","Feature:Alerting/RuleActions","v8.13.0","v8.14.0","v8.15.0"],"number":181604,"url":"https://github.com/elastic/kibana/pull/181604","mergeCommit":{"message":"[MGMTEX]
Fix action data override when adding a second action (#181604)\n\n##
Summary\r\n\r\nWe were overwriting the `actionTypeId` of the first
action in the\r\n\"create connector\" callback. The value assigned was
the `actionTypeId`\r\nof the newly created action, meaning that we would
have converted the\r\nfirst action to be the same as the second one.
This fix changes the\r\n`actionTypeId` not for the first option but for
all current\r\n`activeActionItem.indices`.\r\n\r\nPreviously, we did add
that override to fix a bug related to the slack\r\nconnector
#155722. Its test\r\nshould
cover us from breaking it back again. I did test it manually just\r\nin
case and it seems to be working still. Feel free to test it
too.\r\n\r\nAlso, if you are wondering why `activeActionItems.indices`
is an array\r\nof numbers. The user might be using the same connector in
more than one\r\naction and then delete the connector. In case this
happens, and the user\r\nclicks on \"edit rule\", they will be able to
restore both actions by just\r\ncreating the connector once. In order to
be able to restore all affected\r\nactions, their index is being stored
as a number[]. More info
here\r\nhttps://github.com//pull/86838\r\n\r\nCloses
https://github.com/elastic/kibana/issues/181407\r\n\r\n---------\r\n\r\nCo-authored-by:
Antonio
<antoniodcoelho@gmail.com>","sha":"60c6cdb9985570a6f58bbf3860539de64ace9aa6"}},"sourceBranch":"main","suggestedTargetBranches":["8.13"],"targetPullRequestStates":[{"branch":"8.13","label":"v8.13.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.14","label":"v8.14.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/181691","number":181691,"state":"MERGED","mergeCommit":{"sha":"36d2b9310c5388c444f90c520b6563360caa5d99","message":"[8.14]
[MGMTEX] Fix action data override when adding a second action (#181604)
(#181691)\n\n# Backport\n\nThis will backport the following commits from
`main` to `8.14`:\n- [[MGMTEX] Fix action data override when adding a
second
action\n(#181604)](https://github.com/elastic/kibana/pull/181604)\n\n<!---
Backport version: 9.4.3 -->\n\n### Questions ?\nPlease refer to the
[Backport
tool\ndocumentation](https://github.com/sqren/backport)\n\n<!--BACKPORT
[{\"author\":{\"name\":\"Julian\nGernun\",\"email\":\"17549662+jcger@users.noreply.github.com\"},\"sourceCommit\":{\"committedDate\":\"2024-04-25T10:42:28Z\",\"message\":\"[MGMTEX]\nFix
action data override when adding a second action
(#181604)\\n\\n##\nSummary\\r\\n\\r\\nWe were overwriting the
`actionTypeId` of the first\naction in the\\r\\n\\\"create connector\\\"
callback. The value assigned was\nthe `actionTypeId`\\r\\nof the newly
created action, meaning that we would\nhave converted the\\r\\nfirst
action to be the same as the second one.\nThis fix changes
the\\r\\n`actionTypeId` not for the first option but for\nall
current\\r\\n`activeActionItem.indices`.\\r\\n\\r\\nPreviously, we did
add\nthat override to fix a bug related to the
slack\\r\\nconnector\nhttps://github.com//issues/155722.
Its test\\r\\nshould\ncover us from breaking it back again. I did test
it manually just\\r\\nin\ncase and it seems to be working still. Feel
free to test it\ntoo.\\r\\n\\r\\nAlso, if you are wondering why
`activeActionItems.indices`\nis an array\\r\\nof numbers. The user might
be using the same connector in\nmore than one\\r\\naction and then
delete the connector. In case this\nhappens, and the user\\r\\nclicks on
\\\"edit rule\\\", they will be able to\nrestore both actions by
just\\r\\ncreating the connector once. In order to\nbe able to restore
all affected\\r\\nactions, their index is being stored\nas a number[].
More
info\nhere\\r\\nhttps://github.com//pull/86838\\r\\n\\r\\nCloses\nhttps://github.com//issues/181407\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by:\nAntonio\n<antoniodcoelho@gmail.com>\",\"sha\":\"60c6cdb9985570a6f58bbf3860539de64ace9aa6\",\"branchLabelMapping\":{\"^v8.15.0$\":\"main\",\"^v(\\\\d+).(\\\\d+).\\\\d+$\":\"$1.$2\"}},\"sourcePullRequest\":{\"labels\":[\"release_note:fix\",\"Team:ResponseOps\",\"Feature:Alerting/RuleActions\",\"v8.13.0\",\"v8.14.0\",\"v8.15.0\"],\"title\":\"[MGMTEX]\nFix
action data override when adding a
second\naction\",\"number\":181604,\"url\":\"https://github.com/elastic/kibana/pull/181604\",\"mergeCommit\":{\"message\":\"[MGMTEX]\nFix
action data override when adding a second action
(#181604)\\n\\n##\nSummary\\r\\n\\r\\nWe were overwriting the
`actionTypeId` of the first\naction in the\\r\\n\\\"create connector\\\"
callback. The value assigned was\nthe `actionTypeId`\\r\\nof the newly
created action, meaning that we would\nhave converted the\\r\\nfirst
action to be the same as the second one.\nThis fix changes
the\\r\\n`actionTypeId` not for the first option but for\nall
current\\r\\n`activeActionItem.indices`.\\r\\n\\r\\nPreviously, we did
add\nthat override to fix a bug related to the
slack\\r\\nconnector\nhttps://github.com//issues/155722.
Its test\\r\\nshould\ncover us from breaking it back again. I did test
it manually just\\r\\nin\ncase and it seems to be working still. Feel
free to test it\ntoo.\\r\\n\\r\\nAlso, if you are wondering why
`activeActionItems.indices`\nis an array\\r\\nof numbers. The user might
be using the same connector in\nmore than one\\r\\naction and then
delete the connector. In case this\nhappens, and the user\\r\\nclicks on
\\\"edit rule\\\", they will be able to\nrestore both actions by
just\\r\\ncreating the connector once. In order to\nbe able to restore
all affected\\r\\nactions, their index is being stored\nas a number[].
More
info\nhere\\r\\nhttps://github.com//pull/86838\\r\\n\\r\\nCloses\nhttps://github.com//issues/181407\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by:\nAntonio\n<antoniodcoelho@gmail.com>\",\"sha\":\"60c6cdb9985570a6f58bbf3860539de64ace9aa6\"}},\"sourceBranch\":\"main\",\"suggestedTargetBranches\":[\"8.13\",\"8.14\"],\"targetPullRequestStates\":[{\"branch\":\"8.13\",\"label\":\"v8.13.0\",\"branchLabelMappingKey\":\"^v(\\\\d+).(\\\\d+).\\\\d+$\",\"isSourceBranch\":false,\"state\":\"NOT_CREATED\"},{\"branch\":\"8.14\",\"label\":\"v8.14.0\",\"branchLabelMappingKey\":\"^v(\\\\d+).(\\\\d+).\\\\d+$\",\"isSourceBranch\":false,\"state\":\"NOT_CREATED\"},{\"branch\":\"main\",\"label\":\"v8.15.0\",\"branchLabelMappingKey\":\"^v8.15.0$\",\"isSourceBranch\":true,\"state\":\"MERGED\",\"url\":\"https://github.com/elastic/kibana/pull/181604\",\"number\":181604,\"mergeCommit\":{\"message\":\"[MGMTEX]\nFix
action data override when adding a second action
(#181604)\\n\\n##\nSummary\\r\\n\\r\\nWe were overwriting the
`actionTypeId` of the first\naction in the\\r\\n\\\"create connector\\\"
callback. The value assigned was\nthe `actionTypeId`\\r\\nof the newly
created action, meaning that we would\nhave converted the\\r\\nfirst
action to be the same as the second one.\nThis fix changes
the\\r\\n`actionTypeId` not for the first option but for\nall
current\\r\\n`activeActionItem.indices`.\\r\\n\\r\\nPreviously, we did
add\nthat override to fix a bug related to the
slack\\r\\nconnector\nhttps://github.com//issues/155722.
Its test\\r\\nshould\ncover us from breaking it back again. I did test
it manually just\\r\\nin\ncase and it seems to be working still. Feel
free to test it\ntoo.\\r\\n\\r\\nAlso, if you are wondering why
`activeActionItems.indices`\nis an array\\r\\nof numbers. The user might
be using the same connector in\nmore than one\\r\\naction and then
delete the connector. In case this\nhappens, and the user\\r\\nclicks on
\\\"edit rule\\\", they will be able to\nrestore both actions by
just\\r\\ncreating the connector once. In order to\nbe able to restore
all affected\\r\\nactions, their index is being stored\nas a number[].
More
info\nhere\\r\\nhttps://github.com//pull/86838\\r\\n\\r\\nCloses\nhttps://github.com//issues/181407\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by:\nAntonio\n<antoniodcoelho@gmail.com>\",\"sha\":\"60c6cdb9985570a6f58bbf3860539de64ace9aa6\"}}]}]\nBACKPORT-->\n\nCo-authored-by:
Julian Gernun
<17549662+jcger@users.noreply.github.com>"}},{"branch":"main","label":"v8.15.0","labelRegex":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/181604","number":181604,"mergeCommit":{"message":"[MGMTEX]
Fix action data override when adding a second action (#181604)\n\n##
Summary\r\n\r\nWe were overwriting the `actionTypeId` of the first
action in the\r\n\"create connector\" callback. The value assigned was
the `actionTypeId`\r\nof the newly created action, meaning that we would
have converted the\r\nfirst action to be the same as the second one.
This fix changes the\r\n`actionTypeId` not for the first option but for
all current\r\n`activeActionItem.indices`.\r\n\r\nPreviously, we did add
that override to fix a bug related to the slack\r\nconnector
#155722. Its test\r\nshould
cover us from breaking it back again. I did test it manually just\r\nin
case and it seems to be working still. Feel free to test it
too.\r\n\r\nAlso, if you are wondering why `activeActionItems.indices`
is an array\r\nof numbers. The user might be using the same connector in
more than one\r\naction and then delete the connector. In case this
happens, and the user\r\nclicks on \"edit rule\", they will be able to
restore both actions by just\r\ncreating the connector once. In order to
be able to restore all affected\r\nactions, their index is being stored
as a number[]. More info
here\r\nhttps://github.com//pull/86838\r\n\r\nCloses
https://github.com/elastic/kibana/issues/181407\r\n\r\n---------\r\n\r\nCo-authored-by:
Antonio
<antoniodcoelho@gmail.com>","sha":"60c6cdb9985570a6f58bbf3860539de64ace9aa6"}}]}]
BACKPORT-->
kpatticha pushed a commit to kpatticha/kibana that referenced this pull request Apr 26, 2024
…c#181604)

## Summary

We were overwriting the `actionTypeId` of the first action in the
"create connector" callback. The value assigned was the `actionTypeId`
of the newly created action, meaning that we would have converted the
first action to be the same as the second one. This fix changes the
`actionTypeId` not for the first option but for all current
`activeActionItem.indices`.

Previously, we did add that override to fix a bug related to the slack
connector elastic#155722. Its test
should cover us from breaking it back again. I did test it manually just
in case and it seems to be working still. Feel free to test it too.

Also, if you are wondering why `activeActionItems.indices` is an array
of numbers. The user might be using the same connector in more than one
action and then delete the connector. In case this happens, and the user
clicks on "edit rule", they will be able to restore both actions by just
creating the connector once. In order to be able to restore all affected
actions, their index is being stored as a number[]. More info here
elastic#86838

Closes elastic#181407

---------

Co-authored-by: Antonio <antoniodcoelho@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting release_note:fix Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Alerting] Unable to save when editing alert with multiple actions using deleted connector
6 participants