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

Fixes #14802 - Correctly determines if working with Host/Hostgroups page #5994

Merged
merged 1 commit into from Apr 26, 2016

Conversation

parthaa
Copy link
Contributor

@parthaa parthaa commented Apr 25, 2016

Prior to this commit the hosts and hostroups pages used the same erbs
They were just identifying what constitutes a host page vs hostgroup
page in the wrong manner.
It used -> @host.nil? to determine this which doesnt work for all
situations.

This commit instead centralizes that logic in a nice helper and uses a
simpler 'check the controller name' approach

@parthaa
Copy link
Contributor Author

parthaa commented Apr 25, 2016

All I changed in all the erbs is "using_hostgroups_page" became "using_hostgroups_page?"

@parthaa parthaa changed the title Refs #14257-Correctly determines if working with Host/Hostgroups page Fixes #14802-Correctly determines if working with Host/Hostgroups page Apr 25, 2016
@jlsherrill
Copy link
Member

@parthaa theforeman-bot is complaining because of the lack of whitespae around the '-'.

Should be: #14802 - Correctly not #14802-Correctly

@parthaa parthaa changed the title Fixes #14802-Correctly determines if working with Host/Hostgroups page Fixes #14802 - Correctly determines if working with Host/Hostgroups page Apr 25, 2016
@parthaa
Copy link
Contributor Author

parthaa commented Apr 25, 2016

Should be: #14802 - Correctly not #14802-Correctly

Thanks

@jlsherrill
Copy link
Member

Code looks fine to me, haven't tested it yet though

Prior to this commit the hosts and hostroups pages used the same erbs
They were just identifying what constitutes a host page vs hostgroup
page in the wrong manner.
It used -> @host.nil? to determine this which doesnt work for all
situations.

This commit instead centralizes that logic in a nice helper and uses a
simpler 'check the controller name' approach
@mccun934
Copy link
Member

ACK from a user perspective. tested a series of provisioning use-cases with and without hostgroups as well as editing existing hosts and it properly assigned the media as selected.

Also checked some error handling on empty fields, worked well after an update partha did

@bbuckingham
Copy link
Member

ACK from code perspective.

@mccun934
Copy link
Member

merging

@bbuckingham bbuckingham merged commit d6738d7 into Katello:master Apr 26, 2016
@parthaa parthaa deleted the host-hostgroups branch November 3, 2018 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants