Skip to content

Commit

Permalink
Fixes #12180 - lookup_value presence validation moved from lookup_key
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
amirfefer authored and dLobatog committed Dec 15, 2015
1 parent 706de3c commit d4b8428
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 9 deletions.
10 changes: 4 additions & 6 deletions app/models/lookup_keys/lookup_key.rb
Expand Up @@ -82,6 +82,10 @@ def self.find_by_name(str)
nil
end

def reject_invalid_lookup_values(attributes)
attributes[:match].empty?
end

def audit_class
self
end
Expand Down Expand Up @@ -117,12 +121,6 @@ def path=(v)
write_attribute(:path, using_default ? nil : v)
end

def reject_invalid_lookup_values(attributes)
attributes[:match].empty? ||
(attributes[:value].blank? &&
(attributes[:use_puppet_default].nil? || attributes[:use_puppet_default] == "0"))
end

def default_value_before_type_cast
return read_attribute(:default_value) if errors[:default_value].present?
value_before_type_cast default_value
Expand Down
2 changes: 1 addition & 1 deletion app/models/lookup_value.rb
Expand Up @@ -29,7 +29,7 @@ class LookupValue < ActiveRecord::Base
scoped_search :in => :lookup_key, :on => :key, :rename => :lookup_key, :complete_value => true

def value_present?
self.errors.add(:value, :blank) if value.nil? && !use_puppet_default
self.errors.add(:value, :blank) if value.to_s.empty? && !use_puppet_default && lookup_key.puppet?
end

def value=(val)
Expand Down
13 changes: 11 additions & 2 deletions test/functional/api/v2/override_values_controller_test.rb
Expand Up @@ -70,15 +70,24 @@ class Api::V2::OverrideValuesControllerTest < ActionController::TestCase

[{ :value => 'xyz=10'}, { :match => 'os=string'}].each do |override_value|
test "should not create override value without #{override_value.keys.first}" do
lookup_key = FactoryGirl.create(:variable_lookup_key, :puppetclass => puppetclasses(:two))
lookup_key = FactoryGirl.create(:puppetclass_lookup_key, :as_smart_class_param, :puppetclass => puppetclasses(:two))
refute lookup_key.override
assert_difference('LookupValue.count', 0) do
post :create, { :smart_variable_id => lookup_key.id, :override_value => override_value }
post :create, {:smart_class_parameter_id => lookup_key.id, :override_value => override_value}
end
response = ActiveSupport::JSON.decode(@response.body)
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
end
end

test "should create override value without value for smart variable" do
lookup_key = FactoryGirl.create(:variable_lookup_key, :puppetclass => puppetclasses(:two))
refute lookup_key.override
assert_difference('LookupValue.count', 1) do
post :create, {:smart_variable_id => lookup_key.id, :override_value => { :match => 'os=string'}}
end
assert_response :success
end
end
22 changes: 22 additions & 0 deletions test/unit/lookup_value_test.rb
Expand Up @@ -223,4 +223,26 @@ def setup
refute_valid @value
end
end

context "when key type is puppetclass lookup and value is empty" do
def setup
@key = FactoryGirl.create(:puppetclass_lookup_key, :as_smart_class_param,
:with_override, :with_use_puppet_default,
:key_type => 'string',
:puppetclass => puppetclasses(:one))
@value = FactoryGirl.build_stubbed(:lookup_value, :value => "",
:match => "hostgroup=Common",
:lookup_key_id => @key.id,
:use_puppet_default => true)
end

test "value is validated if use_puppet_default is true" do
assert_valid @value
end

test "value is not validated if use_puppet_default is false" do
@value.use_puppet_default = false
refute_valid @value
end
end
end

0 comments on commit d4b8428

Please sign in to comment.