Adds request_retirement start #380
Conversation
d-m-u
changed the title
|
@abellotti can you give me 👀 on this please? |
| msg << " immediately as a request." | ||
| api_log_info(msg) | ||
| klass.make_retire_request(resource.id) | ||
| resource |
There was a problem hiding this comment.
I thought the request_retire needs to return the request and not the resource
| raise BadRequestError, "Must specify an id for retiring a #{type} resource" | ||
| end | ||
| else | ||
| raise ForbiddenError, "User lacking correct permissions to retire a #{type} as a request." |
There was a problem hiding this comment.
maybe update this to mention that the user is lacking permission to approve a request ? (miq_request_approval above)
| if id | ||
| msg = "Retiring #{type} id #{id}" | ||
| resource = resource_search(id, type, klass) | ||
| msg << " immediately as a request." |
There was a problem hiding this comment.
maybe just form the whole msg on line 68, no need for the append here.
d-m-u
force-pushed
the
request_retirement
branch
2 times, most recently
from
756eeef
to
ca6da80
Compare
| @@ -2811,6 +2811,8 @@ | |||
| :identifier: service_edit | |||
| - :name: retire | |||
| :identifier: service_retire | |||
| - :name: request_retire | |||
| :identifier: service_retire | |||
There was a problem hiding this comment.
we'd also need the request_retire actions on the individual resource and not just at the collection level (bulk) for both services and vms. Thanks.
d-m-u
force-pushed
the
request_retirement
branch
7 times, most recently
from
a87be56
to
ed311e9
Compare
d-m-u
changed the title
|
@miq-bot add_label enhancement |
|
@d-m-u Cannot apply the following label because they are not recognized: retirement |
|
@d-m-u Cannot apply the following label because they are not recognized: i_have_no_clue_what_labels_are_even_in_this_repo |
|
@abellotti bump |
spec/requests/vms_spec.rb
Outdated
| ), | ||
| a_hash_including( | ||
| "message" => a_string_matching(/VM Retire - Request Created/), | ||
| "href" => a_string_matching(/\/api\/requests\//), |
There was a problem hiding this comment.
can you update those to use a_string_matching(api_requests_url)
| raise BadRequestError, "Must specify an id for retiring a #{type} resource" | ||
| end | ||
| else | ||
| raise ForbiddenError, "User lacking correct permissions to approve a #{type} retire as a request." |
There was a problem hiding this comment.
can you add a test for this bad path, i.e. authorize user with service_retire and not miq_request_approval ?
| it "supports multiple service retirement in future" do | ||
| api_basic_authorize collection_action_identifier(:services, :retire) | ||
|
|
||
| ret_date = format_retirement_date(Time.zone.now + 2.days) |
There was a problem hiding this comment.
I take it these updates were for rubocop ?
d-m-u
force-pushed
the
request_retirement
branch
3 times, most recently
from
d005775
to
ed3a0f6
Compare
|
Checked commit d-m-u@1e63ed2 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
Thanks @d-m-u for the API Enhancement. LGTM !! 👍 Will merge when 🍏 |
The New And Improved #375.
WIP cause needs specs that pass
New retire as a request uses new make_retire_request this api change is necessary because we must continue to have retire_now for backward compatibility reasons.
https://github.com/ManageIQ/manageiq/blob/master/app/models/mixins/retirement_mixin.rb#L13 is new, we're calling make_request for retirement as well. make_require_request is available for both Vm and Service and OrchStack classes and is immediate (replaces old retire_now except it doesn't replace it, we're not taking the retire_now out for those who still use it). The identifier in miq_product_features isn't changing. Everything should be similar to retire_now, it's just bimodal now.