-
Notifications
You must be signed in to change notification settings - Fork 356
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
Remove Playbook Service Templates from list of Available resources #613
Remove Playbook Service Templates from list of Available resources #613
Conversation
8ff205b
to
2dee57a
Compare
@@ -1422,7 +1422,7 @@ def rearrange_groups_array | |||
|
|||
def get_available_resources(kls) | |||
@edit[:new][:available_resources] = {} | |||
kls.constantize.all.each do |r| | |||
kls.constantize.all.reject { |st| st.type == "ServiceTemplateAnsiblePlaybook" }.each do |r| |
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.
Better to query for only the items you want:
kls.constantize.where("type is null or type != 'ServiceTemplateAnsiblePlaybook'").each do |r|
cc @syncrou
@gmcculloug made suggested change. |
@@ -1422,7 +1422,7 @@ def rearrange_groups_array | |||
|
|||
def get_available_resources(kls) | |||
@edit[:new][:available_resources] = {} | |||
kls.constantize.all.each do |r| | |||
kls.constantize.where("type is null or type != 'ServiceTemplateAnsiblePlaybook'").each do |r| |
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.
@kbrock is this the preferred query? We can fix in follow-up PR - just drawing your attention here.
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.
Not rush to get this in, if there is a better query we should update this PR.
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.
@kbrock ping
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.
if there is an index on type
, then Postgres can do an index scan. But since we're bringing back all that data, not sure what we buy.
I don't know another way to write this query.
good catch on the type is null
because I tend to miss this null nuance with type != 'abc'
.
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.
since you are only using the id
and name
, I'd only select those columns.
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.
@kbrock Good point. @h-kataria So this line becomes:
kls.constantize.where("type is null or type != 'ServiceTemplateAnsiblePlaybook'").select(:id, :name).each do |r|
This pull request is not mergeable. Please rebase and repush. |
40796e7
to
d3e223b
Compare
d3e223b
to
cb6c343
Compare
@gmcculloug made suggested changes. |
Checked commits h-kataria/manageiq-ui-classic@8b7d2f2~...cb6c343 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
Thanks. @dclarizio I think this is good to merge once tests are green. |
https://www.pivotaltracker.com/story/show/141220225
@gmcculloug please review
before:
after: