-
Notifications
You must be signed in to change notification settings - Fork 101
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 #17648 - Added support for attributes inheritance #316
Conversation
Ok UI works, tests are passing locally, but can you add new test to check the update methods in both UI and API? Adding also a test for UI edit method would be great too. This is very fragile code with regressions :-( |
CLI works too nice! |
Tests added |
assert_equal hostgroup.medium_id, actual.medium_id | ||
assert_equal hostgroup.ptable_id, actual.ptable_id | ||
assert_equal hostgroup.domain_id, actual.domain_id | ||
end |
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.
Mind to extend this with three more fields - environment, puppet and ca? We had similar issues in the past with these too. It's optional, I buy it just like that too :-)
assert_equal hostgroup.operatingsystem_id, actual.operatingsystem_id | ||
assert_equal hostgroup.medium_id, actual.medium_id | ||
assert_equal hostgroup.ptable_id, actual.ptable_id | ||
assert_equal hostgroup.domain_id, actual.domain_id |
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.
Ditto.
Changed HostConverter.to_managed to automatically call attribute inheritance mechanism when a new set of attributes is introduced.
Extra checks added |
You are hero! Thanks a lot. |
Changed HostConverter.to_managed to automatically call attribute inheritance mechanism when a new set of attributes is introduced.