Skip to content
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

#2871 Params search works with nested hostgroups #834

Closed
wants to merge 1 commit into from

Conversation

dLobatog
Copy link
Member

@dLobatog dLobatog commented Aug 5, 2013

This tiny tiny PR allows you to search (UI or API) on parameters that are inherited from hostgroups.

For instance, imagine a host 'foo.cern.ch', and a hostgroup 'bar/baz'. If hostgroup 'bar' defines a parameter 'firewall = on', 'foo.cern.ch' will inherit it. Nonetheless, if you use the search bar, and search for 'params.firewall = true', it will not show 'foo.cern.ch' unless you move it to '/bar/' instead of '/bar/baz'

@lzap
Copy link
Member

lzap commented Aug 6, 2013

Daniel, thanks for the contribution. Taking for testing.

@domcleal
Copy link
Contributor

domcleal commented Aug 6, 2013

This is good, thanks. Could you please add a test?

I'd suggest adding a new hostgroup fixture which uses ancestry, then adding to test/unit/host_test.rb, see the existing search_for test.

@@ -114,7 +114,7 @@ def param_conditions(p)
when 'OsParameter'
conditions << "hosts.operatingsystem_id = #{param.reference_id}"
when 'GroupParameter'
conditions << "hosts.hostgroup_id = #{param.reference_id}"
conditions << "hosts.hostgroup_id IN (#{(param.hostgroup.children.map(&:id) << param.reference_id).join(', ')})"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use child_ids or descendant_ids instead?
e.g.

conditions << "hosts.hostgroup_id IN (#{(param.hostgroup.descendant_ids << param.reference_id).join(', ')})"

@dLobatog
Copy link
Member Author

dLobatog commented Aug 6, 2013

Added tests and used subtree_ids instead of iterating over the children to get their ids,

@@ -114,7 +114,7 @@ def param_conditions(p)
when 'OsParameter'
conditions << "hosts.operatingsystem_id = #{param.reference_id}"
when 'GroupParameter'
conditions << "hosts.hostgroup_id = #{param.reference_id}"
conditions << "hosts.hostgroup_id IN (#{(param.hostgroup.subtree_ids << param.reference_id).join(', ')})"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subtree_ids already includes param.reference_id, you can write:

conditions << "hosts.hostgroup_id IN (#{param.hostgroup.subtree_ids.join(', ')})"

Tests for search in params inherited from hostgroups

Use subtree ids to avoid adding parent id on inherited params search
@dLobatog
Copy link
Member Author

dLobatog commented Aug 7, 2013

@abenari Thanks, I just updated the PR with your suggestion

@domcleal
Copy link
Contributor

domcleal commented Aug 7, 2013

Thanks for the contribution @elobato, merged as 98dd4a0.

The ticket's much appreciated, I've amended the commit message into the standard format to reference the redmine ticket number.

@domcleal domcleal closed this Aug 7, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants