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 #9591 - Added support for "inherit" state. #2230

Closed
wants to merge 1 commit into from

Conversation

ShimShtein
Copy link
Member

Changed the UI so it will possible to create a dropdown with empty value and "inherit" option in the same time.
On the server side, I check for existence of the field in the update to check if I need to inherit the value (if the field does not exists - its value will be inherited).
I did not change the notion of "inherit" in hosts, it is more of a "copy" notion - it copies the value from the hostgroup, but does not change it, if the value is changed. The change will be performed on the first update to the host object.

@unorthodoxgeek
Copy link
Member

I think some tests will be beneficial here.

end

def puppet_ca(f)
OVERRIDE_TEXT = "inherit"
Copy link
Member

Choose a reason for hiding this comment

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

missing N_()?
EDIT: I see we extract this value later in view but still

@@ -400,3 +400,27 @@ function setPowerState(item, status){
power_actions.hide();
$('[rel="twipsy"]').tooltip();
}

function disableButtonToggle(item) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd do item = $(item) at the beginning of the function and work with the benefits of jQuery later.

@orrabin
Copy link
Member

orrabin commented Mar 16, 2015

@ShimShtein please rebase

@ShimShtein
Copy link
Member Author

Rebased

@ares
Copy link
Member

ares commented Apr 29, 2015

@ShimShtein would you mind adding tests for apply_inherited_attributes?

@ShimShtein
Copy link
Member Author

Fixed the explicitly-selected bug

@domcleal
Copy link
Contributor

Various unit tests appear to be failing, could you just have a look at those please? http://ci.theforeman.org/job/test_develop_pr_core/5673/

@ShimShtein
Copy link
Member Author

After rebase getting unknown attribute: @audit_comment error... Seems unrelated to my change, but still looking into it.

@ShimShtein
Copy link
Member Author

Found the error, deep_clone was not a good option for me...

@domcleal
Copy link
Contributor

Tests are still red, this time with a Javascript parse error.

@ShimShtein
Copy link
Member Author

Finally! Found the problem. Now the tests are running (at least locally).

@domcleal
Copy link
Contributor

Merged as 04cb74c for Foreman 1.10, thanks @ShimShtein.

@domcleal domcleal closed this Jul 29, 2015
isratrade pushed a commit to isratrade/foreman that referenced this pull request Apr 17, 2016
Cherry-picked from theforeman#2230

Conflicts:
	app/assets/javascripts/application.js
	app/controllers/api/v2/hosts_controller.rb
	app/controllers/hosts_controller.rb
	app/helpers/hosts_and_hostgroups_helper.rb
	app/models/host/managed.rb
	app/views/hosts/_form.html.erb
	test/unit/host_test.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants