Change ems refresh task complete messaging #22776
Conversation
app/models/ems_refresh.rb
Outdated
| @@ -17,7 +17,7 @@ def self.debug_trace | |||
| cache_with_timeout(:queue_timeout) { MiqEmsRefreshWorker.worker_settings[:queue_timeout] || 60.minutes } | |||
|
|
|||
| def self.queue_refresh_task(target, id = nil) | |||
| queue_refresh(target, id, :create_task => true) | |||
| queue_refresh(target, id, :create_task => true, :action => "EmsRefresh completed successfully") | |||
There was a problem hiding this comment.
@kbrock will this say "EmsRefresh completed successfully" while it is still running? If it failed?
Would :action => "Refreshing relationships and power states" or something like that make more sense? Maybe I'm missing when/where the :action attribute is displayed
There was a problem hiding this comment.
Yea, don't know when task action is displayed, @Fryguy ?
|
So with this change I'm still seeing the target information in the task list |
|
The issue should probably be fixed in https://github.com/ManageIQ/manageiq/blob/master/app/models/ems_refresh.rb#L173 For one, we could remove listing the target if the target is an ems. |
app/models/ems_refresh.rb
Outdated
| task_options = { | ||
| :action => "EmsRefresh(#{ems.name}) [#{targets}]".truncate(255), | ||
| :action => "EmsRefresh(#{ems.name})#{targets.present? ? " #{targets.count} #{targets.first.first.to_s.split(':').last}" : ""}", |
There was a problem hiding this comment.
targets.first.first.to_s.split(':').last
I think this is an indication that we're on the wrong track :/
There was a problem hiding this comment.
target_class = targets.first&.first&.demodulize # first, class, simplified
task_options = {
:action => "EmsRefresh(#{ems.name})#{targets.present? ? " #{targets.count} #{target_class}" : ""}",There was a problem hiding this comment.
I had also wanted to remove the target if the targets were the ems, but for some reason the code got too complicated and thought this was good enough.
There was a problem hiding this comment.
I don't think the message needs to list every VM that is being refreshed.
The customer won't know the VM id.
Maybe just "Manual Refresh" is good enough without the particular id?
There was a problem hiding this comment.
I still think "Refreshing relationships and power states" or whatever the exact wording we use on the UI is fine
taken from: - ManageIQ#22465 - ManageIQ/manageiq-providers-autosde#228 Instead of displaying target information, this now displays a quick user friendly summary
kbrock
force-pushed
the
refresh_task_messaging
branch
from
45f9c0b
to
90fb062
Compare
|
Checked commit kbrock@90fb062 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
taken from:
Instead of displaying target information, this now displays a user friendly message in the ui
This was marked as stale and I liked the change by @galoiring so I put this together
Before
After