Add Alert Definitions (MiqAlert) REST API support #13967
Conversation
|
@miq-bot add_label enhancement, api, control |
|
@moolitayer @zeari @simon3z please review |
| :identifier: alert_definition | ||
| :options: | ||
| - :collection | ||
| :verbs: *gpd |
There was a problem hiding this comment.
delete action is a resource action, L113:
https://github.com/ManageIQ/manageiq/pull/13967/files#diff-15502b585d5033aa8043cc26237a06baR113
|
@abellotti please review |
| :feature_type: admin | ||
| :hidden: true | ||
| :identifier: alert_definition_delete | ||
|
|
There was a problem hiding this comment.
/cc @h-kataria for new miq_product_features
There was a problem hiding this comment.
@abellotti changes to product features look good.
| expect(alert_definition).to be_truthy | ||
| expect(response.parsed_body["results"].first["description"]).to eq(alert_definition["description"]) | ||
| expect(response.parsed_body["results"].first["db"]).to eq(alert_definition["db"]) | ||
| expect(response.parsed_body["results"].first["expression"]).to eq(alert_definition["expression"]) |
There was a problem hiding this comment.
too many expects, would prefer expect(response.parsed_body).to include(...)
| :identifier: alert_definitions_show_list | ||
| :post: | ||
| - :name: create | ||
| :identifier: alert_definition_new |
There was a problem hiding this comment.
should support delete action on collection too (i.e. bulk deletes) and add test.
| - :name: edit | ||
| :identifier: alert_definition_edit | ||
| - :name: delete | ||
| :identifier: alert_definition_delete |
There was a problem hiding this comment.
also add support for http DELETE on resource and add test.
| module Api | ||
| class AlertDefinitionsController < BaseController | ||
| end | ||
| end |
There was a problem hiding this comment.
we need support code here, minimally for create. I got a 500 internal server error creating an alert definition with bogus attributes.
Need to test for minimally description, also validation of expression attribute so bogus ones aren't created. see arbitration_rules_controller.rb or conditions_controller.rb.
create will need to be rescued returning a BadRequestError.
| module Api | ||
| class AlertDefinitionsController < BaseController | ||
| end | ||
| end |
There was a problem hiding this comment.
also probably code for edit to parse/build MiqExpression.
|
/cc @imtayadeway for another pair or 👀 Thanks. |
|
@abellotti I'm almost certain I addressed all your comments, let me know if I need to change/add something |
config/api.yml
Outdated
| - :name: delete | ||
| :identifier: alert_definition_delete | ||
| - :name: delete | ||
| :identifier: alert_definition_delete |
| :feature_type: admin | ||
| :hidden: true | ||
| :identifier: alert_definition_delete | ||
|
|
Adding CRUD actions support to alert definitions API endpoint
|
Checked commit dkorn@ca15956 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
There was a problem hiding this comment.
Thanks @dkorn for the updates.
we need @h-kataria to review product features and give a 👍 before I can merge. Thanks.
|
@h-kataria quick review will help us here.. thanks |
|
Thanks @h-kataria for lending your 👀 and reviewing. |
|
@dkorn PR Looks good. Thanks for the enhancement !! Merging. /cc @Loicavenel |
| :post: | ||
| - :name: create | ||
| :identifier: alert_definition_new | ||
| - :name: delete |
There was a problem hiding this comment.
@dkorn looks like we're missing bulk edits, i.e. POST action "edit" on the collection. When you get a chance, can you create an enhancement PR for that and add the appropriate test ? Thanks.
There was a problem hiding this comment.
@abellotti PR submitted #14397
Adding CRUD actions support to alert definitions API endpoint.