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

Changed actions API endpoints urls to follow Kibana STYLEGUIDE #65936

Merged
merged 19 commits into from
May 25, 2020
Merged

Changed actions API endpoints urls to follow Kibana STYLEGUIDE #65936

merged 19 commits into from
May 25, 2020

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented May 9, 2020 •
edited
Loading

Resolve #63130 and partially cover some tasks from issue 59799:

  1. Changed actions BASE_ACTION_API_PATH to '/api/actions' - according to styleguide it should keep structure /api/plugin_id
  2. Changed endpoint /_getAll just to /
  3. Changed /types to /list_action_types
  4. Changes POST /api/action to POST /api/actions/action
  5. Changed GET /api/action/{id} to GET /api/actions/action/{id}
  6. Changed PUT /api/action/{id} PUT /api/actions/action/{id}
  7. Changed DELETE /api/action/{id} to DELETE /api/actions/action/{id}
  8. Changed POST /api/action/{id}/_execute to POST /api/actions/action/{id}/_execute

@YulNaumenko YulNaumenko added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Actions Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.9.0 labels May 9, 2020
@YulNaumenko YulNaumenko requested review from a team as code owners May 9, 2020 00:06
@YulNaumenko YulNaumenko requested a review from a team May 9, 2020 00:06
@YulNaumenko YulNaumenko self-assigned this May 9, 2020
@elasticmachine
Copy link
Contributor

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

@YulNaumenko
Fixed tests
c0f23ed
@YulNaumenko
Merge remote-tracking branch 'upstream/master' into actions-api-follo…
c8cb68f 
…w-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
fixed test
65e7b49
@YulNaumenko
Merge remote-tracking branch 'upstream/master' into actions-api-follo…
182cb6e 
…w-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
fixed test
7325481
@YulNaumenko
Merge remote-tracking branch 'upstream/master' into actions-api-follo…
6b9da6a 
…w-kibana-styleguide

# Conflicts:
#	x-pack/plugins/siem/public/containers/case/api.test.tsx
@YulNaumenko
resolved conflicts
7eb0322
@YulNaumenko YulNaumenko changed the title [Draft] Changed actions API endpoints urls to follow Kibana STYLEGUIDE Changed actions API endpoints urls to follow Kibana STYLEGUIDE May 13, 2020
@YulNaumenko
Merge remote-tracking branch 'upstream/master' into actions-api-follo…
2e537fb 
…w-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
Fixed siem tests
f9dad61
@YulNaumenko
Fixed failing test
70fdbe6
@YulNaumenko
fixed readme and test
9013b45
gmmorris
gmmorris approved these changes May 21, 2020
View reviewed changes
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 :)

x-pack/plugins/actions/README.md
### `GET /api/action/_getAll`: Get all actions
### `GET /api/actions`: Get all actions
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this 👍

@mikecote mikecote self-requested a review May 21, 2020 12:13
pmuellr
pmuellr requested changes May 21, 2020
View reviewed changes
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.

I haven't done a full review yet, but the current URLs suggested in the PR aren't sufficiently unique yet. Posted a comment with some suggestions ...

x-pack/plugins/actions/README.md Show resolved Hide resolved
pmuellr
pmuellr approved these changes May 21, 2020
View reviewed changes
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.

Beyond the figuring out a new naming schema (as noted in comment), the code LGTM.

x-pack/plugins/actions/README.md Show resolved Hide resolved
FrankHassanabad
FrankHassanabad approved these changes May 22, 2020
View reviewed changes
Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

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

Looked over the siem sections and it looks like all URL changes. LGTM

chrisronline
chrisronline approved these changes May 22, 2020
View reviewed changes
Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

LGTM

@YulNaumenko
Fixed type checks
2426d26
@YulNaumenko
fixed type check
01cfa51
@YulNaumenko
Merge remote-tracking branch 'upstream/master' into actions-api-follo…
8d258d6 
…w-kibana-styleguide

# Conflicts:
#	x-pack/plugins/monitoring/public/components/alerts/configuration/step1.test.tsx
#	x-pack/plugins/triggers_actions_ui/public/application/lib/action_connector_api.ts
#	x-pack/test/alerting_api_integration/spaces_only/tests/actions/type_not_enabled.ts
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@YulNaumenko YulNaumenko merged commit d09bd63 into elastic:master May 25, 2020
jloleysens added a commit that referenced this pull request May 26, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into feature/pipel…
f06f9e4 
…ine-editor

* 'master' of github.com:elastic/kibana: (129 commits)
  [Canvas] Force embeddables to refresh when renderable reevaluated (#67133)
  [Canvas] Better handling navigating to/from canvas (#66407)
  [Ingest pipelines] Fix schema validation for simulate and update routes (#67199)
  do not use es from setup (#67277)
  Auto expand replicas for event log (#67286)
  Observability & APM do not use elasticsearch client provided via setup contract  (#67263)
  Fix privileges check when security is not enabled (#67308)
  add IIS home (#66918)
  [ML] Adding additional job service endpoint tests (#66892)
  [Ingest Manager] Update fleet internal doc with latest flags (#67193)
  [Discover] Deangularize the loading spinner (#67165)
  Add `application.navigateToUrl` core API (#67110)
  Improve indexpattern without timefield functional test (#67031)
  KibanaContext in index pattern managment ui (#66985)
  Fix Azure metrics tutorial inside the App Home/ Add data area (#66901)
  add azure logs home (#66910)
  fix: rum agent should work correctly on new platform (#67037)
  [test_utils/Testbed] Move to src/test_utils folder (OSS) (#66898)
  only block registration when appRoute contains the exact basePath (#67125)
  Changed actions API endpoints urls to follow Kibana STYLEGUIDE (#65936)
  ...

# Conflicts:
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form_fields.tsx
@YulNaumenko YulNaumenko mentioned this pull request May 26, 2020
Merged
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request May 26, 2020
@YulNaumenko
Changed actions API endpoints urls to follow Kibana STYLEGUIDE (elast…
a906981 
…ic#65936)

* Changed actions API endpoints urls to follow Kibana STYLEGUIDE

* Fixed tests

* fixed test

* fixed test

* resolved conflicts

* Fixed siem tests

* Fixed failing test

* fixed readme and test

* Changed actions api urls to follow the template 'api/{plugin}/{type}/{id}

* Fixed type checks

* Fixed tests and API

* fixed tests

* Fixed type checks

* fixed type check
@YulNaumenko YulNaumenko mentioned this pull request May 26, 2020
Closed
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request May 26, 2020
@YulNaumenko
Changed actions API endpoints urls to follow Kibana STYLEGUIDE (elast…
bc4588e 
…ic#65936)

* Changed actions API endpoints urls to follow Kibana STYLEGUIDE

* Fixed tests

* fixed test

* fixed test

* resolved conflicts

* Fixed siem tests

* Fixed failing test

* fixed readme and test

* Changed actions api urls to follow the template 'api/{plugin}/{type}/{id}

* Fixed type checks

* Fixed tests and API

* fixed tests

* Fixed type checks

* fixed type check
# Conflicts:
#	x-pack/plugins/siem/public/cases/containers/api.test.tsx
#	x-pack/plugins/siem/public/cases/containers/api.ts
#	x-pack/test/case_api_integration/basic/tests/cases/push_case.ts
YulNaumenko added a commit that referenced this pull request May 26, 2020
@YulNaumenko
Changed actions API endpoints urls to follow Kibana STYLEGUIDE (#65936)…
61b9893 
… (#67360)

* Changed actions API endpoints urls to follow Kibana STYLEGUIDE

* Fixed tests

* fixed test

* fixed test

* resolved conflicts

* Fixed siem tests

* Fixed failing test

* fixed readme and test

* Changed actions api urls to follow the template 'api/{plugin}/{type}/{id}

* Fixed type checks

* Fixed tests and API

* fixed tests

* Fixed type checks

* fixed type check
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request May 26, 2020
@YulNaumenko
Changed actions API endpoints urls to follow Kibana STYLEGUIDE (elast…
862bfb0 
…ic#65936)

* Changed actions API endpoints urls to follow Kibana STYLEGUIDE

* Fixed tests

* fixed test

* fixed test

* resolved conflicts

* Fixed siem tests

* Fixed failing test

* fixed readme and test

* Changed actions api urls to follow the template 'api/{plugin}/{type}/{id}

* Fixed type checks

* Fixed tests and API

* fixed tests

* Fixed type checks

* fixed type check
# Conflicts:
#	x-pack/plugins/siem/public/cases/containers/api.test.tsx
#	x-pack/plugins/siem/public/cases/containers/api.ts
#	x-pack/test/case_api_integration/basic/tests/cases/push_case.ts
@YulNaumenko YulNaumenko mentioned this pull request May 26, 2020
Merged
YulNaumenko added a commit that referenced this pull request May 27, 2020
@YulNaumenko
[7.8] Changed actions API endpoints urls to follow Kibana STYLEGUIDE (#…
5d2fd2f 
…65936) (#67429)

* Changed actions API endpoints urls to follow Kibana STYLEGUIDE (#65936)

* Changed actions API endpoints urls to follow Kibana STYLEGUIDE

* Fixed tests

* fixed test

* fixed test

* resolved conflicts

* Fixed siem tests

* Fixed failing test

* fixed readme and test

* Changed actions api urls to follow the template 'api/{plugin}/{type}/{id}

* Fixed type checks

* Fixed tests and API

* fixed tests

* Fixed type checks

* fixed type check
# Conflicts:
#	x-pack/plugins/siem/public/cases/containers/api.test.tsx
#	x-pack/plugins/siem/public/cases/containers/api.ts
#	x-pack/test/case_api_integration/basic/tests/cases/push_case.ts

* -
@mikecote mikecote added release_note:breaking and removed release_note:skip Skip the PR/issue when compiling release notes labels May 27, 2020
@rayafratkina
Copy link
Contributor

@YulNaumenko @bmcconaghy this is labeled as release_note:breaking. Are there end user-visible changes in this? If not, it should be release_note:dev_docs

@bmcconaghy
Copy link
Contributor

API level, so dev_docs @rayafratkina

@bmcconaghy bmcconaghy added release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. and removed release_note:breaking labels Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmuellr pmuellr pmuellr approved these changes

@chrisronline chrisronline chrisronline approved these changes

@gmmorris gmmorris gmmorris approved these changes

@FrankHassanabad FrankHassanabad FrankHassanabad approved these changes

@mikecote mikecote Awaiting requested review from mikecote

Assignees

@YulNaumenko YulNaumenko

Labels
Feature:Actions release_note:plugin_api_changes Contains a Plugin API changes section for the breaking plugin API changes section. Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.8.0 v7.9.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Alerting] url collision between pre-registered actions and existing GET urls