Fix call to process_tasks to run the right thing #17255
Conversation
|
@tinaafitz can you 👀 for me please? |
| @@ -121,7 +120,7 @@ def retirement_check | |||
| _log.log_backtrace(err) | |||
| end | |||
| end | |||
|
|
|||
| _log.info("#{retirement_due?} : is value for if retirement is due DREWDREWDREW") | |||
There was a problem hiding this comment.
@d-m-u We can remove this message.
| if options[:task] == "refresh_ems" && respond_to?("refresh_ems") | ||
|
|
||
| if options[:task] == 'retire_now' | ||
| options = {:task => options[:task], :userid => User.current_user.try(:userid), :src_ids => options[:ids]} |
There was a problem hiding this comment.
@d-m-u Were we going to use src_id instead of src_ids here?
There was a problem hiding this comment.
we need all the things here
There was a problem hiding this comment.
d-m-u
changed the title
d-m-u
changed the title
d-m-u
force-pushed
the
fixing_retire_request_call
branch
from
b863530 to
33a5aff
Compare
d-m-u
changed the title
d-m-u
force-pushed
the
fixing_retire_request_call
branch
from
33a5aff to
12920a9
Compare
|
@tinaafitz can you review please |
| options = {:task => task, :userid => current_user, :ids => objs, :src_ids => objs} | ||
| (klass.to_s + "RetireRequest").constantize.make_request(@request_id, options, current_user) | ||
| def make_retire_request(*options) | ||
| options = {:src_ids => options.present? ? options.first : id, :src_klass => options.present? ? self : name} |
There was a problem hiding this comment.
options.present? ? self : name
Why self vs name? Shouldn't it always be name?
There was a problem hiding this comment.
it's bimodal, so the UI retirement is always name but we call make_retire_request without options from the mixin in the other case which has the model name as self.
There was a problem hiding this comment.
I guess my point is that if options is present, :src_klass is a class, otherwise it's a string.
irb(main):001:0> a = Module.new { def bd_self; self; end; def bd_name; name; end }
=> #<Module:0x0000000007c3d1b8>
irb(main):002:0> Vm.singleton_class.prepend(a)
=> #<Class:Vm(id: integer, vendor: string, ...)>
irb(main):003:0> Vm.bd_self
=> Vm(id: integer, vendor: string, ...)
irb(main):004:0> Vm.bd_name
=> "Vm"
d-m-u
force-pushed
the
fixing_retire_request_call
branch
from
12920a9 to
79271cf
Compare
d-m-u
changed the title
d-m-u
force-pushed
the
fixing_retire_request_call
branch
from
79271cf to
fe90861
Compare
d-m-u
changed the title
| options = {:task => task, :userid => current_user, :ids => objs, :src_ids => objs} | ||
| (klass.to_s + "RetireRequest").constantize.make_request(@request_id, options, current_user) | ||
| def make_retire_request(*src_ids) | ||
| options = {:src_ids => src_ids.present? ? src_ids.first : id} |
There was a problem hiding this comment.
Can you add some tests? Since the array is not splatted above, I think you'll find that you end up with something like
{:src_ids => [[1,2,3]]}
d-m-u
force-pushed
the
fixing_retire_request_call
branch
3 times, most recently
from
759ff50 to
2135e28
Compare
|
@tinaafitz can you review again please? |
|
@mkanoor can you review please? |
|
@d-m-u |
d-m-u
force-pushed
the
fixing_retire_request_call
branch
from
2135e28 to
50a360d
Compare
d-m-u
force-pushed
the
fixing_retire_request_call
branch
from
50a360d to
e1288e8
Compare
|
Checked commits d-m-u/manageiq@e341d98~...e1288e8 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
process_tasks should call make_retire_request, not make_request.