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

[ResponseOps][Connectors]Preconfigured connectors of disabled types show as disabled, but are actually enabled #198792

Conversation

georgianaonoleata1904
Copy link
Contributor

@georgianaonoleata1904 georgianaonoleata1904 commented Nov 4, 2024

Closes #190420

Summary

  • the preconfigured connectors should be displayed as enabled and not have the tooltip icon even if the xpack.actions.enabledActionTypes: [] setting is present in the kibana.yml (to disable all the connector types)

Screenshot 2024-11-04 at 14 38 10

@georgianaonoleata1904 georgianaonoleata1904 added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v9.0.0 backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.17.0 labels Nov 4, 2024
@georgianaonoleata1904 georgianaonoleata1904 self-assigned this Nov 4, 2024
@georgianaonoleata1904 georgianaonoleata1904 requested a review from a team as a code owner November 4, 2024 14:10
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

Not a great reviewer for UX code, but this was my original issue, so thought I'd review.

LGTM, but expecting to see some tests in the peer *.test.ts files.

I compared what we're doing server-side here, and looks like the new UX side processing is the same as server (note, we don't need to worry about system actions here):
https://github.com/cnasikas/kibana/blob/5ad2a2c8c44853da51919625506536dc064641c8/x-pack/plugins/actions/server/action_type_registry.ts#L86-L107

Copy link
Contributor

@adcoelho adcoelho left a comment

Choose a reason for hiding this comment

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

Tested and looks ok but could you please add tests for the new logic in:

  • packages/kbn-alerts-ui-shared/src/rule_form/utils/check_action_type_enabled.test.ts
  • x-pack/plugins/triggers_actions_ui/public/application/sections/actions_connectors_list/components/actions_connectors_list.test.tsx

Copy link
Contributor

@adcoelho adcoelho left a comment

Choose a reason for hiding this comment

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

There is another place where checkActionTypeEnabled is used.

x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_menu.tsx

I don't know exactly where this is but I think it is for the actions for the rule form.

We don't pass isPreconfiguredConnector there so I think this logic will fail. Could you please confirm?

@georgianaonoleata1904
Copy link
Contributor Author

There is another place where checkActionTypeEnabled is used.

x-pack/plugins/triggers_actions_ui/public/application/sections/action_connector_form/action_type_menu.tsx

I don't know exactly where this is but I think it is for the actions for the rule form.

We don't pass isPreconfiguredConnector there so I think this logic will fail. Could you please confirm?

in this case isPreconfiguredConnector is defaulted to false

@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
triggersActionsUi 1.7MB 1.7MB +67.0B

History

cc @georgianaonoleata1904

@georgianaonoleata1904 georgianaonoleata1904 merged commit 56b0ac2 into elastic:main Nov 6, 2024
24 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 6, 2024
…how as disabled, but are actually enabled (elastic#198792)

Closes elastic#190420

## Summary

- the preconfigured connectors should be displayed as enabled and not
have the tooltip icon even if the xpack.actions.enabledActionTypes: []
setting is present in the kibana.yml (to disable all the connector
types)

![Screenshot 2024-11-04 at 14 38
10](https://github.com/user-attachments/assets/ee817087-a079-481b-bf82-b7247f3ea923)

---------

Co-authored-by: Antonio <[email protected]>
(cherry picked from commit 56b0ac2)
@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 6, 2024
…ypes show as disabled, but are actually enabled (#198792) (#199157)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ResponseOps][Connectors]Preconfigured connectors of disabled types
show as disabled, but are actually enabled
(#198792)](#198792)

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

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

<!--BACKPORT [{"author":{"name":"Georgiana-Andreea
Onoleață","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-06T14:43:48Z","message":"[ResponseOps][Connectors]Preconfigured
connectors of disabled types show as disabled, but are actually enabled
(#198792)\n\nCloses
https://github.com/elastic/kibana/issues/190420\r\n\r\n##
Summary\r\n\r\n- the preconfigured connectors should be displayed as
enabled and not\r\nhave the tooltip icon even if the
xpack.actions.enabledActionTypes: []\r\nsetting is present in the
kibana.yml (to disable all the connector\r\ntypes)\r\n\r\n![Screenshot
2024-11-04 at 14
38\r\n10](https://github.com/user-attachments/assets/ee817087-a079-481b-bf82-b7247f3ea923)\r\n\r\n---------\r\n\r\nCo-authored-by:
Antonio
<[email protected]>","sha":"56b0ac2eda55b0fe1a3dc0cd445d704320b69981","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:ResponseOps","v9.0.0","backport:prev-minor","v8.17.0"],"title":"[ResponseOps][Connectors]Preconfigured
connectors of disabled types show as disabled, but are actually
enabled","number":198792,"url":"https://github.com/elastic/kibana/pull/198792","mergeCommit":{"message":"[ResponseOps][Connectors]Preconfigured
connectors of disabled types show as disabled, but are actually enabled
(#198792)\n\nCloses
https://github.com/elastic/kibana/issues/190420\r\n\r\n##
Summary\r\n\r\n- the preconfigured connectors should be displayed as
enabled and not\r\nhave the tooltip icon even if the
xpack.actions.enabledActionTypes: []\r\nsetting is present in the
kibana.yml (to disable all the connector\r\ntypes)\r\n\r\n![Screenshot
2024-11-04 at 14
38\r\n10](https://github.com/user-attachments/assets/ee817087-a079-481b-bf82-b7247f3ea923)\r\n\r\n---------\r\n\r\nCo-authored-by:
Antonio
<[email protected]>","sha":"56b0ac2eda55b0fe1a3dc0cd445d704320b69981"}},"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/198792","number":198792,"mergeCommit":{"message":"[ResponseOps][Connectors]Preconfigured
connectors of disabled types show as disabled, but are actually enabled
(#198792)\n\nCloses
https://github.com/elastic/kibana/issues/190420\r\n\r\n##
Summary\r\n\r\n- the preconfigured connectors should be displayed as
enabled and not\r\nhave the tooltip icon even if the
xpack.actions.enabledActionTypes: []\r\nsetting is present in the
kibana.yml (to disable all the connector\r\ntypes)\r\n\r\n![Screenshot
2024-11-04 at 14
38\r\n10](https://github.com/user-attachments/assets/ee817087-a079-481b-bf82-b7247f3ea923)\r\n\r\n---------\r\n\r\nCo-authored-by:
Antonio
<[email protected]>","sha":"56b0ac2eda55b0fe1a3dc0cd445d704320b69981"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Georgiana-Andreea Onoleață <[email protected]>
mgadewoll pushed a commit to mgadewoll/kibana that referenced this pull request Nov 7, 2024
…how as disabled, but are actually enabled (elastic#198792)

Closes elastic#190420

## Summary

- the preconfigured connectors should be displayed as enabled and not
have the tooltip icon even if the xpack.actions.enabledActionTypes: []
setting is present in the kibana.yml (to disable all the connector
types)

![Screenshot 2024-11-04 at 14 38
10](https://github.com/user-attachments/assets/ee817087-a079-481b-bf82-b7247f3ea923)

---------

Co-authored-by: Antonio <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.17.0 v9.0.0
Projects
None yet
5 participants