From 2c160a957f7f453c3056dcbc53175a593122492f Mon Sep 17 00:00:00 2001 From: "Josep M. Blanquer" Date: Wed, 10 Aug 2016 22:12:49 -0700 Subject: [PATCH 1/5] Fix corner case bug rendering nil subjects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note: It would only manifest when include_nil is true and there’s an explicit subsection of fields. --- CHANGELOG.md | 1 + lib/praxis-blueprints/renderer.rb | 5 ++++- spec/praxis-blueprints/renderer_spec.rb | 6 ++++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 829f63e..1abbb64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * Include Attributor::Dumpable in Blueprint, so it renders (semi) correctly if rendered with just `true` specified for fields. +* Fix bug rendering subobjects with nil values (manifested when `include_nil: true` there’s an explicit subsection of fields) ## 3.2 diff --git a/lib/praxis-blueprints/renderer.rb b/lib/praxis-blueprints/renderer.rb index 8299b73..b112683 100644 --- a/lib/praxis-blueprints/renderer.rb +++ b/lib/praxis-blueprints/renderer.rb @@ -79,7 +79,10 @@ def _render(object, fields, view = nil, context: Attributor::DEFAULT_ROOT_CONTEX raise Attributor::DumpError, context: context, name: key, type: object.class, original_exception: e end - next if value.nil? && !include_nil + if value.nil? + hash[key] = nil if self.include_nil + next + end if subfields == true hash[key] = case value diff --git a/spec/praxis-blueprints/renderer_spec.rb b/spec/praxis-blueprints/renderer_spec.rb index ca657ab..2770b56 100644 --- a/spec/praxis-blueprints/renderer_spec.rb +++ b/spec/praxis-blueprints/renderer_spec.rb @@ -100,6 +100,7 @@ context 'with include_nil: true' do let(:renderer) { Praxis::Renderer.new(include_nil: true) } + let(:address) { nil } it 'renders attributes with nil values' do output.should have_key :email @@ -108,6 +109,11 @@ output.should have_key :work_address output[:work_address].should be nil end + + it 'renders nil directly for nil subobjects' do + output.should have_key :address + output[:address].should be nil + end end context '#render_collection' do From f7489781c4d47efda41f370249a7f24697732f21 Mon Sep 17 00:00:00 2001 From: "Josep M. Blanquer" Date: Wed, 10 Aug 2016 22:35:21 -0700 Subject: [PATCH 2/5] Bump ruby 2.2.x version for travis. Signed-off-by: Josep M. Blanquer --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3a8e055..1938e08 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ language: ruby cache: bundler sudo: false rvm: - - 2.2.3 + - 2.2.5 - 2.3.1 script: - bundle exec rspec spec From cf5c6533b8d40a1ae7799e9f72f2847889d0596c Mon Sep 17 00:00:00 2001 From: "Josep M. Blanquer" Date: Wed, 10 Aug 2016 23:09:53 -0700 Subject: [PATCH 3/5] Use `.concat` to appease the cops a little bit. Signed-off-by: Josep M. Blanquer --- lib/praxis-blueprints/blueprint.rb | 4 ++-- lib/praxis-blueprints/field_expander.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/praxis-blueprints/blueprint.rb b/lib/praxis-blueprints/blueprint.rb index 18ff3cf..f6308d3 100644 --- a/lib/praxis-blueprints/blueprint.rb +++ b/lib/praxis-blueprints/blueprint.rb @@ -337,11 +337,11 @@ def validate(context = Attributor::DEFAULT_ROOT_CONTEXT) if value.respond_to?(:validating) # really, it's a thing with sub-attributes next if value.validating end - errors.push(*sub_attribute.validate(value, sub_context)) + errors.concat(sub_attribute.validate(value, sub_context)) end self.class.attribute.type.requirements.each do |req| validation_errors = req.validate(keys_with_values, context) - errors.push(*validation_errors) unless validation_errors.empty? + errors.concat(validation_errors) unless validation_errors.empty? end errors ensure diff --git a/lib/praxis-blueprints/field_expander.rb b/lib/praxis-blueprints/field_expander.rb index 6633e6f..f52171d 100644 --- a/lib/praxis-blueprints/field_expander.rb +++ b/lib/praxis-blueprints/field_expander.rb @@ -108,7 +108,7 @@ def expand_with_member_attribute(object, fields = true) new_fields = fields.is_a?(Array) ? fields[0] : fields result = [expand(object.member_attribute.type, new_fields)] - history[object][fields].push(*result) + history[object][fields].concat(result) result end From 3101a5ac6148e9814f25348ec42b9bd1c0d2cc08 Mon Sep 17 00:00:00 2001 From: "Josep M. Blanquer" Date: Wed, 10 Aug 2016 23:18:28 -0700 Subject: [PATCH 4/5] Try to appease the cops about method_missing. Signed-off-by: Josep M. Blanquer --- lib/praxis-blueprints/config_hash.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/praxis-blueprints/config_hash.rb b/lib/praxis-blueprints/config_hash.rb index f92f88b..4751c56 100644 --- a/lib/praxis-blueprints/config_hash.rb +++ b/lib/praxis-blueprints/config_hash.rb @@ -17,7 +17,11 @@ def to_hash @hash end - def method_missing(name, value, *rest, &block) + def respond_to_missing?(_method_name, _include_private = false) + true + end + + def method_missing(name, value, *rest, &block) # rubocop:disable Style/MethodMissing if (existing = @hash[name]) if block existing << [value, block] From fa070ada12e79e88ffbb6f0cb2b0f9d88b153c38 Mon Sep 17 00:00:00 2001 From: "Josep M. Blanquer" Date: Thu, 11 Aug 2016 14:54:39 -0700 Subject: [PATCH 5/5] Build specs for config_hash (to appease coveralls) Signed-off-by: Josep M. Blanquer --- spec/praxis-blueprints/config_hash_spec.rb | 64 ++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 spec/praxis-blueprints/config_hash_spec.rb diff --git a/spec/praxis-blueprints/config_hash_spec.rb b/spec/praxis-blueprints/config_hash_spec.rb new file mode 100644 index 0000000..a922d06 --- /dev/null +++ b/spec/praxis-blueprints/config_hash_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') + +describe Praxis::ConfigHash do + subject(:instance) { Praxis::ConfigHash.new(hash, &block) } + let(:hash) { { one: ['existing'], two: 'dos' } } + let(:block) do + proc { 'abc' } + end + + context 'initialization' do + it 'saves the passed hash' do + expect(subject.hash).to be(hash) + end + end + + context '.from' do + subject(:instance) { Praxis::ConfigHash.from(hash, &block) } + it 'returns an instance' do + expect(subject).to be_kind_of(Praxis::ConfigHash) + expect(subject.hash).to be(hash) + end + end + + context '#to_hash' do + let(:block) do + proc { hash['i_was'] = 'here' } + end + it 'evaluates the block and returns the resulting hash' do + expect(subject.to_hash).to eq(subject.hash) + expect(subject.hash['i_was']).to eq('here') + end + end + + context '#method_missing' do + context 'when keys do not exist in the hash key' do + it 'sets a single value to the hash' do + subject.some_name 'someval' + expect(subject.hash[:some_name]).to eq('someval') + end + it 'sets a multiple values to the hash key' do + subject.some_name 'someval', 'other1', 'other2' + expect(subject.hash[:some_name]).to include('someval', 'other1', 'other2') + end + end + context 'when keys already exist in the hash key' do + it 'adds one value to the hash' do + subject.one'newval' + expect(subject.hash[:one]).to match_array(%w(existing newval)) + end + it 'adds multiple values to the hash key' do + subject.one 'newval', 'other1', 'other2' + expect(subject.hash[:one]).to match_array(%w(existing newval other1 other2)) + end + context 'when passing a value and a block' do + let(:my_block) { proc {} } + it 'adds the tuple to the hash key' do + subject.one 'val', &my_block + expect(subject.hash[:one]).to include(['val', my_block]) + end + end + end + end +end