dry-rb / dry-schema

Coercion and validation for data structures
https://dry-rb.org/gems/dry-schema
MIT License
425 stars 109 forks source link

Fix sum type errors when using I18n backend #331

Closed tylerhunt closed 4 years ago

tylerhunt commented 4 years ago

Resolve issue with error handling of custom sum types. The error message code was hardcoded to expect the YAML backend, which was causing an error when the I18n backend was used.

Wasn’t sure if a spec should be included or where it should be added, but this covers the patch:

# frozen_string_literal: true

RSpec.describe "Custom type errors" do
  let(:container_with_types) { Dry::Schema::TypeContainer.new }

  before do
    container_with_types.register(
      "params.string_or_integer",
      Types::Strict::String | Types::Strict::Decimal
    )

    stub_const "ContainerWithTypes", container_with_types
  end

  let(:klass) do
    class Test::CustomTypeSchema < Dry::Schema::Params
      define do
        config.types = ContainerWithTypes

        required(:age).filled(:string_or_integer)
      end
    end
  end

  let(:params) {
    {
      age: 4.5
    }
  }

  subject(:schema) { klass.new.call(params) }

  before do
    klass.definition.configure { |config| config.messages.backend = backend }
  end

  context 'with YAML backend' do
    let(:backend) { :yaml }

    it "provides a valid error message" do
      expect(subject.errors[:age])
        .to include 'must be a string or must be a decimal'
    end
  end

  context 'with I18n backend' do
    let(:backend) { :i18n }

    it "provides a valid error message" do
      expect(subject.errors[:age])
        .to include 'must be a string or must be a decimal'
    end
  end
end

Fixes #328.

solnic commented 4 years ago

Thanks for working on this fix! ❤️

Wasn’t sure if a spec should be included or where it should be added,

yes, specs for bugs are always required, otherwise we may accidentally break things. Please add it to spec/integration/schema/custom_types_spec.rb.

tylerhunt commented 4 years ago

@solnic Specs have been added. Thanks!