Fixes #15814 - Reset override params when override is off #3737
Conversation
daviddavis
force-pushed
the
temp/20160816154358
branch
4 times, most recently
from
7c2ed3c
to
1e35728
Compare
| @@ -376,6 +376,20 @@ def setup | |||
| end | |||
| end | |||
|
|
|||
| test "override params are reset after override changes back to false" do | |||
| @key = FactoryGirl.create(:puppetclass_lookup_key, :as_smart_class_param, | |||
There was a problem hiding this comment.
rubocop doesn't like the indentation here, it should be 2 spaces instead of 4
daviddavis
force-pushed
the
temp/20160816154358
branch
2 times, most recently
from
44bfa1b
to
446fc9a
Compare
|
@orrabin I added a migration and updated |
daviddavis
force-pushed
the
temp/20160816154358
branch
from
446fc9a
to
12b8461
Compare
|
[test] |
| @@ -203,6 +204,16 @@ def cast_default_value | |||
| true | |||
| end | |||
|
|
|||
| def reset_override_params | |||
| if override_changed? && !override? | |||
There was a problem hiding this comment.
nitpick: this can be moved to the before_validation and not get here at all unless override has changed.
There was a problem hiding this comment.
I don't see a benefit to doing so but if you think it's better, then I can.
EDIT: I don't see the harm in doing so either though so I'll go ahead and update.
|
Tested both with api and ui - when I change override to false, all three turn to false. |
daviddavis
force-pushed
the
temp/20160816154358
branch
4 times, most recently
from
41d01be
to
2466cb9
Compare
|
@orrabin updated |
|
@daviddavis thanks, waiting for the tests to be green |
domcleal
added
Waiting on contributor
Needs re-review
and removed
Not yet reviewed
Waiting on contributor
labels
daviddavis
force-pushed
the
temp/20160816154358
branch
from
2466cb9
to
d49c4f0
Compare
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
@orrabin @tbrisker I updated the code to reset the params after validation which means not having to touch the check_override_selected method. Please review and test it out. Thanks. By the way, I am working on the katello failure here: Katello/katello#6260 |
| @@ -31,7 +31,7 @@ | |||
| # Debug mode disables concatenation and preprocessing of assets. | |||
| # This option may cause significant delays in view rendering with a large | |||
| # number of complex assets. | |||
| config.assets.debug = true | |||
| config.assets.debug = false | |||
There was a problem hiding this comment.
accidentally added to the commit?
There was a problem hiding this comment.
Ugh yea. Thanks.
daviddavis
force-pushed
the
temp/20160816154358
branch
from
d49c4f0
to
0611ac2
Compare
|
[test] |
|
merged as e21a3a4, thanks @daviddavis! |
No description provided.