Changes to CustomButtonEvent. #17885
Conversation
app/models/custom_button_event.rb
Outdated
| end | ||
|
|
||
| def button_name | ||
| full_data[:button_name] |
There was a problem hiding this comment.
@lfu I would suggest capturing the button ID as well. Then this method should use the current button name, incase it gets renamed, and fall back to the name of the button when it was run if the button get deleted.
app/models/custom_button_event.rb
Outdated
| full_data[:automate_entry_point] | ||
| end | ||
|
|
||
| def button_name |
There was a problem hiding this comment.
@lfu Please add tests for these two new methods.
app/models/custom_button_event.rb
Outdated
| virtual_column :automate_entry_point, :type => :string | ||
|
|
||
| def automate_entry_point | ||
| full_data[:automate_entry_point] |
There was a problem hiding this comment.
Based on the virtual column I would expect this to always return a string, so I would suggest adding a to_s incase it comes up nil. My guess is if the UI allows sorting that would cause it to blow us.
cc @h-kataria
There was a problem hiding this comment.
@gmcculloug just tried with userid as nil, seems working
There was a problem hiding this comment.
Good to know, but I have run into issues in the past with list-views and breaking with nil objects. Additionally, the virtual column specifies that it returns :type => :string, so I would argue on that point it should always return an actual string.
There was a problem hiding this comment.
@lfu Can you let me know what you are thinking here. I'm suggesting that virtual columns should return their defined data type, which means we should ensure these methods backing the virtual columns return strings.
There was a problem hiding this comment.
@gmcculloug I agree with what you said. 👍
lfu
force-pushed
the
ansible_reporting
branch
2 times, most recently
from
c6b60f6 to
3145941
Compare
|
Checked commits lfu/manageiq@c7c0189~...4b2afee with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 app/models/mixins/custom_actions_mixin.rb
|
|
Perfect, UI PR will be hopefully finished today. |
|
WIP UI PR ManageIQ/manageiq-ui-classic#4520 |
Add association of CustomButtonEvent to its supporting classes.
Add button name and automate entry point virtual columns to CustomButtonEvent.
Followup of #17764.
https://bugzilla.redhat.com/show_bug.cgi?id=1511126
@miq-bot assign @gmcculloug
@miq-bot add_label automate, enhancement, events
cc @h-kataria