-
Notifications
You must be signed in to change notification settings - Fork 897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add custom_button and custom_button_set copy to service_template copy #18494
Add custom_button and custom_button_set copy to service_template copy #18494
Conversation
ce894ab
to
d8395db
Compare
d8395db
to
8f0d26a
Compare
Checked commits d-m-u/manageiq@303322f~...8f0d26a with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
@miq-bot add_reviewer @tinaafitz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM
template.add_resource(resource, sr) | ||
end | ||
service_resources.each { |service_resource| resource_copy(service_resource, template) } | ||
custom_buttons.each { |custom_button| custom_button_copy(custom_button, template) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the bug ... custom_buttons
are not custom buttons assigned to this service template.
If you want that, you need to use direct_custom_buttons
.
custom_buttons
gives you those, and every single custom button for any ServiceTemplate .. and you don't want to copy those.
(that should fix #18954)
As part of copying service templates (#18450) we should copy custom buts and custom_but_sets on those templates so that's what this does.
The template copy itself got merged yesterday.
Probs best viewed by commit cause the second commit is performance enhancement on tests from the original merge (calling find_by only once per test)