From 232a983a8aaf644042002bcb06edd5ecb67f52a1 Mon Sep 17 00:00:00 2001 From: Tobias Pfeiffer Date: Fri, 17 Jul 2020 14:20:27 +0200 Subject: [PATCH 1/2] Make Result#error?(key) return true for rule and schema errors Not super happy with this as of now :) Exchanged usage of error? with schema_error? in the code base as otherwise there thankfully were a bunch of failures. The implementation of error? itself is basically stolen from its counterpart at `Schema::Result`. It makes sense to check if there are any errors for this but not sure if we shouldn't rather still call out to schema_result.error? Also, root/base behavior is a bit of a mystery. --- lib/dry/validation/contract.rb | 4 ++-- lib/dry/validation/evaluator.rb | 2 +- lib/dry/validation/result.rb | 9 ++++++++- lib/dry/validation/rule.rb | 2 +- spec/integration/result_spec.rb | 27 +++++++++++++++++++++++++++ 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/lib/dry/validation/contract.rb b/lib/dry/validation/contract.rb index 08b4b0e6..e0e15c65 100644 --- a/lib/dry/validation/contract.rb +++ b/lib/dry/validation/contract.rb @@ -123,14 +123,14 @@ def error?(result, spec) return path.expand.any? { |nested_path| error?(result, nested_path) } end - return true if result.error?(path) + return true if result.schema_error?(path) path .to_a[0..-2] .any? { |key| curr_path = Schema::Path[path.keys[0..path.keys.index(key)]] - return false unless result.error?(curr_path) + return false unless result.schema_error?(curr_path) result.errors.any? { |err| (other = Schema::Path[err.path]).same_root?(curr_path) && other == curr_path diff --git a/lib/dry/validation/evaluator.rb b/lib/dry/validation/evaluator.rb index 48737e8d..31984053 100644 --- a/lib/dry/validation/evaluator.rb +++ b/lib/dry/validation/evaluator.rb @@ -175,7 +175,7 @@ def key? # # @api public def schema_error?(path) - result.error?(path) + result.schema_error?(path) end # Check if there are any errors on the current rule diff --git a/lib/dry/validation/result.rb b/lib/dry/validation/result.rb index 65fb6e10..2db2c39f 100644 --- a/lib/dry/validation/result.rb +++ b/lib/dry/validation/result.rb @@ -101,8 +101,15 @@ def failure? # Check if values include an error for the provided key # - # @api private + # @api public def error?(key) + errors.any? { |msg| Schema::Path[msg.path].include?(Schema::Path[key]) } + end + + # Check if the base schema (without rules) includes an error for the provided key + # + # @api private + def schema_error?(key) schema_result.error?(key) end diff --git a/lib/dry/validation/rule.rb b/lib/dry/validation/rule.rb index 077aef1b..043e0966 100644 --- a/lib/dry/validation/rule.rb +++ b/lib/dry/validation/rule.rb @@ -86,7 +86,7 @@ def each(*macros, &block) values[root].each_with_index do |_, idx| path = [*Schema::Path[root].to_a, idx] - next if result.error?(path) + next if result.schema_error?(path) evaluator = with(macros: macros, keys: [path], &block) diff --git a/spec/integration/result_spec.rb b/spec/integration/result_spec.rb index 683f1443..20a9b757 100644 --- a/spec/integration/result_spec.rb +++ b/spec/integration/result_spec.rb @@ -50,6 +50,33 @@ end end + describe "#error?" do + subject { result } + + let(:schema_result) do + double(:schema_result, message_set: [], to_h: {email: "jane@doe.org"}) + end + + let(:result) do + Dry::Validation::Result.new(schema_result) do |r| + r.add_error(Dry::Validation::Message.new("root error", path: [nil])) + r.add_error(Dry::Validation::Message.new("email error", path: [:email])) + end + end + + it "reports an error on email" do + expect(subject.error?(:email)).to eq true + end + + it "reports an error on 'root' when asked for [nil]" do + expect(subject.error?([nil])).to eq true + end + + it "doesn't report errors on non existing keys" do + expect(subject.error?(:nonexistant)).to eq false + end + end + describe "#inspect" do let(:params) do double(:params, message_set: [], to_h: {}) From 4288faeb7a8d523bd4366a07208ca00fd35da1fa Mon Sep 17 00:00:00 2001 From: Tobias Pfeiffer Date: Tue, 21 Jul 2020 12:23:28 +0200 Subject: [PATCH 2/2] Address PR feedback :tada: --- spec/integration/result_spec.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/spec/integration/result_spec.rb b/spec/integration/result_spec.rb index 20a9b757..617ee877 100644 --- a/spec/integration/result_spec.rb +++ b/spec/integration/result_spec.rb @@ -51,8 +51,6 @@ end describe "#error?" do - subject { result } - let(:schema_result) do double(:schema_result, message_set: [], to_h: {email: "jane@doe.org"}) end @@ -65,15 +63,15 @@ end it "reports an error on email" do - expect(subject.error?(:email)).to eq true + expect(result.error?(:email)).to be(true) end - it "reports an error on 'root' when asked for [nil]" do - expect(subject.error?([nil])).to eq true + it "reports an error on 'root' when asked for nil" do + expect(result.error?([nil])).to be(true) end it "doesn't report errors on non existing keys" do - expect(subject.error?(:nonexistant)).to eq false + expect(result.error?(:nonexistant)).to be(false) end end