Fixes #12180 - lookup_value presence validation moved from lookup_key #2909
Conversation
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
| param_not_posted = override_value.keys.first.to_s == 'match' ? 'Value' : 'Match' # The opposite of override_value is missing | ||
| assert_match /Validation failed: #{param_not_posted} can't be blank/, response['error']['message'] | ||
| assert_response :error | ||
| test "should not create override value without match" do |
There was a problem hiding this comment.
value can be empty unless the lookup_key type is puppetclass
|
There were the following issues with the commit message:
If you don't have a ticket number, please create an issue in Redmine, selecting the appropriate project. More guidelines are available on the Foreman wiki. This message was auto-generated by Foreman's prprocessor |
theforeman-bot
added
Needs testing
Waiting on contributor
and removed
Waiting on contributor
labels
|
Please note that tests are failing. |
| @@ -76,6 +76,10 @@ def self.find_by_name(str) | |||
| nil | |||
| end | |||
|
|
|||
| def reject_invalid_lookup_values(attributes) | |||
There was a problem hiding this comment.
Removing the reject_if completely is what caused the integration test to fail.
When saving through the UI there wasn't a problem but in the integration test the fact that there is a hidden matcher (that is shown once you click + Add Matcher cause the form to refuse to be saved because of the validation added on lookup_value for empty values.
The hidden matcher has :match => "", :value => "" and tries to update on save and of course fails.
Adding reject_if to reject all lookup_values with empty matcher fixes the test.
This is also good to have since the validation on lookup_value is only for PuppetclassLookupKey and we still want to reject any type of lookup_key that doesn't contain a matcher.
|
@amirfefer @orrabin Thanks! It works pretty well from what I've seen, if we want to refactor this code a bit more though, take a look at the inline comments and let me know what you think. I hope it makes sense 😁 |
Global parameters can have empty values so smart variables that are also global should be allowed to accept empty values too. The validation on lookup_value to make sure it isn't empty is in the LookupKey class therefore affecting both VariableLookupKey and PuppetClassLookupKey. This validation should affect on the child class - PuppetClassLookupKey.
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
Merged as 2661491, thanks @amirfefer! |
Global parameters can have empty values so smart variables that are also global should be allowed to accept empty values too.
The validation on lookup_value to make sure it isn't empty is in the LookupKey class therefore affecting both VariableLookupKey and PuppetClassLookupKey.
This validation should affect on the child class - PuppetClassLookupKey.