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.

Sign up for GitHub

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

Jump to bottom

[Alerting][Connectors] Removing ability to set refresh=true from ES Index Connector UI #158102

Merged
merged 5 commits into from
May 23, 2023
Merged

[Alerting][Connectors] Removing ability to set refresh=true from ES Index Connector UI #158102

merged 5 commits into from
May 23, 2023

Conversation

ymao1
Copy link
Contributor

@ymao1 ymao1 commented May 18, 2023 •
edited
Loading

Resolves #155303

Summary

Removes the refresh switch from the EX Index Connector UI

Before:
Screenshot 2023-05-18 at 2 42 46 PM

After:
Screenshot 2023-05-18 at 2 50 09 PM

No changes to the API so this is not a breaking change. Previously created Index Connectors should still work as before.

To Verify

Verify previously created connectors can be updated

  1. Create an ES Index connector with refresh=true on main or in a previous version
  2. Switch to this branch and verify the connector still loads and changes to the connector preserves the refresh value

Verify creating connector from UI

  1. On this branch, create an ES Index connector and verify that refresh is set to false

Verify creating connector from API

  1. On this branch, use the API to create an ES connector and verify you can successfully create one with either refresh set to true or false

Checklist

@ymao1 ymao1 changed the title Removing refresh switch from UI [Alerting][Connectors] Removing ability to set refresh=true from ES Index Connector UI May 18, 2023
@ymao1 ymao1 self-assigned this May 18, 2023
@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) Feature:Actions/ConnectorTypes Issues related to specific Connector Types on the Actions Framework v8.9.0 labels May 18, 2023
@ymao1 ymao1 marked this pull request as ready for review May 18, 2023 20:51
@ymao1 ymao1 requested a review from a team as a code owner May 18, 2023 20:51
@elasticmachine
Copy link
Contributor

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

ymao1
ymao1 commented May 18, 2023
View reviewed changes
docs/management/connectors/action-types/index.asciidoc
@@ -26,8 +26,7 @@ image::management/connectors/images/index-connector.png[Index connector]
[[index-connector-configuration]]
==== Connector configuration

Index connectors must have a name and an {es} index. You can optionally set the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lcawl I removed references to refresh in the docs but did not touch the API as we are not changing the API (no breaking changes), just removing the ability to set the value in the UI.

ymao1
ymao1 commented May 18, 2023
View reviewed changes
docs/management/connectors/action-types/index.asciidoc
@@ -46,10 +45,8 @@ xpack.actions.preconfigured:
actionTypeId: .index
config:
index: .kibana
refresh: true <1>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lcawl I also remove this from the preconfigured connector config because we want to discourage people from setting the value to true. When not set, it will default to false

@ymao1 ymao1 requested a review from lcawl May 18, 2023 20:54
doakalexi
doakalexi approved these changes May 22, 2023
View reviewed changes
Copy link
Contributor

@doakalexi doakalexi left a comment

Choose a reason for hiding this comment

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

Tested locally, LTGM!

lcawl
lcawl approved these changes May 23, 2023
View reviewed changes
Copy link
Contributor

@lcawl lcawl left a comment

Choose a reason for hiding this comment

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

Docs LGTM, thanks!

@kibana-ci
Copy link
Collaborator

💚 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
stackConnectors 423.7KB 422.8KB -897.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 399 403 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 479 483 +4
total +6

History

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

cc @ymao1

@ymao1 ymao1 merged commit b50c358 into elastic:main May 23, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 23, 2023
@ymao1 ymao1 deleted the remove-refresh branch May 23, 2023 18:34
delanni pushed a commit to delanni/kibana that referenced this pull request May 25, 2023
@ymao1 @delanni
[Alerting][Connectors] Removing ability to set refresh=true from ES…
2130fd0 
… Index Connector UI (elastic#158102)

Resolves elastic#155303

## Summary

Removes the refresh switch from the EX Index Connector UI

Before:
<img width="422" alt="Screenshot 2023-05-18 at 2 42 46 PM"
src="https://github.com/elastic/kibana/assets/13104637/1a2e2ff4-5247-43f7-89ae-776a23be9e95">

After:
<img width="410" alt="Screenshot 2023-05-18 at 2 50 09 PM"
src="https://github.com/elastic/kibana/assets/13104637/2be21cc6-f128-44b5-bd53-cf6945c6f729">

No changes to the API so this is not a breaking change. Previously
created Index Connectors should still work as before.

## To Verify

**Verify previously created connectors can be updated**
1. Create an ES Index connector with refresh=true on `main` or in a
previous version
2. Switch to this branch and verify the connector still loads and
changes to the connector preserves the `refresh` value

**Verify creating connector from UI**
1. On this branch, create an ES Index connector and verify that
`refresh` is set to `false`

**Verify creating connector from API**
1. On this branch, use the API to create an ES connector and verify you
can successfully create one with either refresh set to `true` or `false`

### Checklist

- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lcawl lcawl lcawl approved these changes

@doakalexi doakalexi doakalexi approved these changes

Assignees

@ymao1 ymao1

Labels
backport:skip This commit does not require backporting Feature:Actions/ConnectorTypes Issues related to specific Connector Types on the Actions Framework 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) v8.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[UI] Remove ability to set refresh: true on the Index connector
6 participants
@ymao1 @elasticmachine @kibana-ci @lcawl @doakalexi @kibanamachine