Changed alerting API endpoints urls, bodies and params to follow Kibana STYLEGUIDE #66838
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
YulNaumenko
added
Feature:Alerting
v8.0.0
release_note:skip
|
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
…rls-to-kibana-styleguide # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
YulNaumenko
changed the title
|
@elasticmachine merge upstream |
|
(jenkins upgrade required killing all jobs) |
…m:YulNaumenko/kibana into alerting-change-urls-to-kibana-styleguide
…rls-to-kibana-styleguide # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
|
@elastic/kibana-alerting-services I want to get the team thoughts about renaming |
spong
approved these changes
May 27, 2020
|
I'm +1 on renaming the plugin to |
…rls-to-kibana-styleguide # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
chrisronline
approved these changes
May 28, 2020
|
@jarpy
This is based on the way that the Elasticsearch team has designed their APIs. There isn't a formal convention for this but I've seen a few teams in Kibana follow it whenever an API doesn't fit cleanly within the common interpretation of a RESTful HTTP API, a |
There was a problem hiding this comment.
In Uptime I'm seeing an error when I try to open our alert flyouts. When the flyout opens the plugin crashes.
Primary error message:
TypeError: alertTypesIndex[alertTypeRegistryItem.id] is undefined
Looking briefly at the code changes for Uptime I am not seeing exactly where the error was. I tried moving the alert registration out of mount (which is something we're implementing right now anyway) but that didn't seem to be the cause.
@justinkambic thank you for catching this issue, I will fix it. |
…rls-to-kibana-styleguide # Conflicts: # x-pack/plugins/siem/server/lib/detection_engine/signals/search_after_bulk_create.ts
pmuellr
approved these changes
May 29, 2020
| @@ -4,6 +4,6 @@ | |||
| "kibanaVersion": "kibana", | |||
There was a problem hiding this comment.
should we also change the directory name alerting_example to alerts_example. I'd suggest opening a separate issue for that, 307 files already now, this one isn't critical
| @@ -1,7 +1,7 @@ | |||
| # alerting_builtins plugin | |||
There was a problem hiding this comment.
same as with the alerting example, we probably want this renamed as well, to alerts_builtins, but it's not critical ATM - I don't think anything should be referencing this plugin anyway
|
Noticed there are a lot of functional test failures. Just starting to look at it, on the server side I see a lot of these: There must be some basic security thing going on here, I guess with the URL path change. |
…rls-to-kibana-styleguide # Conflicts: # x-pack/plugins/alerts/server/alerts_client.test.ts # x-pack/plugins/alerts/server/alerts_client_factory.test.ts
…rls-to-kibana-styleguide # Conflicts: # x-pack/plugins/siem/server/lib/detection_engine/routes/rules/validate.test.ts
YulNaumenko
added a commit
to YulNaumenko/kibana
that referenced
this pull request
Jun 2, 2020
…na STYLEGUIDE (elastic#66838) * Changed alerting API endpoints urls, bodies and params to follow Kibana STYLEGUIDE * Changed alerting REST API to keep the pattern 'alerts/alert/{id}' * fixed tests * fixed tests * Fixed jest tests * Renamed plugin from alerting to alerts * fixed tests * fixed tests * Fixed alert type check error * Fixed find api * fixed type checks * fixed tests security issues * Fixed view in app * - Co-authored-by: Elastic Machine <[email protected]> # Conflicts: # .github/CODEOWNERS # rfcs/text/0003_handler_interface.md
mikecote
added
release_note:breaking
and removed
release_note:skip
YulNaumenko
added a commit
that referenced
this pull request
Jun 2, 2020
…na STYLEGUIDE (#66838) (#67919) * Changed alerting API endpoints urls, bodies and params to follow Kibana STYLEGUIDE * Changed alerting REST API to keep the pattern 'alerts/alert/{id}' * fixed tests * fixed tests * Fixed jest tests * Renamed plugin from alerting to alerts * fixed tests * fixed tests * Fixed alert type check error * Fixed find api * fixed type checks * fixed tests security issues * Fixed view in app * - Co-authored-by: Elastic Machine <[email protected]> # Conflicts: # .github/CODEOWNERS # rfcs/text/0003_handler_interface.md
gchaps
added
release_note:plugin_api_changes
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/context/_date_nanos·js.context app context view for date_nanos displays predessors - anchor - successors in right orderStandard OutStack TraceKibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/context/_date_nanos·js.context app context view for date_nanos displays predessors - anchor - successors in right orderStandard OutStack TraceKibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/dashboard/dashboard_filter_bar·js.dashboard app using current data dashboard filter bar filter editor field list shows index pattern of vis when one is addedStandard OutStack Traceand 6 more failures, only showing the first 3. History
To update your PR or re-run it, just comment with: |
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.
Resolve #59799
Dev Docs
Renamed
alertingplugin toalerts.API changes:
BASE_ALERT_API_PATHto'/api/alerts'- according to styleguide it should keep structure/api/plugin_id/api/alert/_findjust to/api/alerts/_find/typesto/list_alert_types/api/alertto POST/api/alerts/alert/api/alert/{id}to GET/api/alerts/alert/{id}/api/alert/{id}PUT/api/alerts/alert/{id}/api/alert/{id}to DELETE/api/alerts/alert/{id}/api/alert/{id}/stateto GET/api/alerts/alert/{id}/state/api/alert/{id}/_enableto POST/api/alerts/alert/{id}/_enable/api/alert/{id}/_disableto POST/api/alerts/alert/{id}/_disable/api/alert/{id}/_mute_allto POST/api/alerts/alert/{id}/_mute_all/api/alert/{alertId}/alert_instance/{alertInstanceId}/_muteto POST/api/alerts/alert/{alertId}/alert_instance/{alertInstanceId}/_mute/api/alert/{id}/_unmute_allto POST/api/alerts/alert/{id}/_unmute_all/api/alert/{id}/_update_api_keyto POST/api/alerts/alert/{id}/_update_api_key/api/alert/{alertId}/alert_instance/{alertInstanceId}/_unmuteto POST/api/alerts/alert/{alertId}/alert_instance/{alertInstanceId}/_unmute