Getting CouldNotSetAttributeError to validate_uniqueness_of for postgres in 3.0

[celsoMartins]

I searched in issues and noticed that a recent commit could solve it. But no lucky here. I have a special set method to configure the name as we want. I still getting the following error:

Error:

set on the model:

the failing spec:

spec_helper

--EDIT--
Gemfile

--EDIT--

I'm downgrading to a previous version until we find what is happening.

Thanks

[jasnow]

Try "is_expected.to validate_uniqueness_of(:name).case_insensitive" in the spec.

[celsoMartins]

No lucky.

BTW, I'm also getting errors on validate_numericality_of.

Thanks.

[jasnow]

Try "capitalize" method as in your model. The BTW example problem is documented in #784

[celsoMartins]

Like this?

it { is_expected.to validate_uniqueness_of(:nome).capitalize }

It even was recognized.

And I understood correctly the other issue? I need to get rid of my spec? With shoulda I cannot test validate_numericality_of anymore? Really? If it is true, I do prefer keep the 2.8. Seems like should becomes obtrusive a lot, huh?

it { expect(workforce).to validate_numericality_of(:amount_centavos).is_greater_than(0) }

[jasnow]

Let me regroup.

Is your code public? If so, I will prove it privately before any more "TRY" statements.

[celsoMartins]

Unfortunately not. =(

But I can share some pieces in code format. But we have some sensitive informations on our repo.

And you can simulate it easily.

  def nome=(nome)
    self[:nome] = nome.try(:squish).try(:mb_chars).try(:capitalize).try(:to_s)
  end

The spec with our last try.

it { is_expected.to validate_uniqueness_of(:nome).case_insensitive }

EDIT: Please let me know if you need more information to investigate.

Thanks

[jasnow]

Probably need more model and migration code associated with *nome.
I added the above two code pieces into existing app and got:

Failures:

  1) Celso 
     Failure/Error: it { is_expected.to validate_uniqueness_of(:nome).case_insensitive }
     ActiveRecord::StatementInvalid:
       PG::UndefinedTable: ERROR:  relation "celsos" does not exist
       LINE 5:                WHERE a.attrelid = '"celsos"'::regclass
                                                 ^
       :               SELECT a.attname, format_type(a.atttypid, a.atttypmod),
                            pg_get_expr(d.adbin, d.adrelid), a.attnotnull, a.atttypid, a.atttypmod
                       FROM pg_attribute a LEFT JOIN pg_attrdef d
                         ON a.attrelid = d.adrelid AND a.attnum = d.adnum
                      WHERE a.attrelid = '"celsos"'::regclass
                        AND a.attnum > 0 AND NOT a.attisdropped
                      ORDER BY a.attnum

[celsoMartins]

Thanks for the prompt attention.

I'll show the migration related to "nome". Sry for the pt-br in code. We started with pt-br in 2013, but now we are migrating to en.

class CreateMateriais < ActiveRecord::Migration
  def change
    create_table :materiais do |t|
      t.string   :nome, null: false
    end

    add_index :materiais, :nome, unique: true
  end
end

On model, we have:

# == Schema Information
#
# Table name: materiais
#
#  id                  :integer          not null, primary key
#  nome                :string           not null
#
# Indexes
#
#  index_materiais_on_nome                 (nome) UNIQUE
#

class Material < ActiveRecord::Base

  alias_attribute :name, :nome
  validates_presence_of :nome
  validates_uniqueness_of :nome

  def nome=(nome)
    self[:nome] = nome.try(:squish).try(:mb_chars).try(:capitalize).try(:to_s)
  end

  def to_s
    nome
  end
end

I think that now you have all relevant informations.

Thanks.

[jasnow]

Not the solution but hint so far.

Found out that if I comment out "..try(:squish).try(:mb_chars).try(:capitalize).try(:to_s)" in app/models file then my spec is green else I get this:

Failure/Error: it { is_expected.to validate_uniqueness_of(:nome) }
Shoulda::Matchers::ActiveModel::AllowValueMatcher::CouldNotSetAttributeError:
Expected Class to be able to set nome to "a", but got "A" instead.

[celsoMartins]

Yes... it is exactly my problem.

I'll not comment this line and spread out this logic in all files that is important, because shoulda can not deal with setters anymore. =S

My hint now: In 2.8 everything works fine.

Have you tried (maybe with some specs) to define a setter (whatever what it does) and check the behaviour?

Thanks.

[jasnow]

Just a FYI. If you set shoulda-matchers to release 3.0.0.rc1, you get a different error:

     Failure/Error: it { is_expected.to validate_uniqueness_of(:nome) }
       Did not expect errors to include "has already been taken" when nome is set to "a",
       got errors:
       * "has already been taken" (attribute: nome, value: "A")

Have you tried (maybe with some specs) to define a setter (whatever what it does) and check the behaviour?

No

[tovodeverett]

I'm running into similar failures after converting from 2.8.0 to 3.0.0. I'm using https://github.com/mdeering/attribute_normalizer extensively, and it seems that the use of automatic case normalization on the setters confuses shoulda-matchers in the new version because it first verifies that it can successfully set the value, but when you change the value it throws an error.

I just found that this is documented in the documentation: http://matchers.shoulda.io/docs/v3.0.0/Shoulda/Matchers/ActiveModel.html#allow_value-instance_method and go to Caveats. I'm going to attempt the workaround and report back.

[tovodeverett]

I'm trying to work my way through the Shoulda-Matchers code to figure out how to get ignoring_interference_by_writer working with validate_uniqueness_of.

[celsoMartins]

@jasnow crazy things. =S

@tovodeverett I've tried it since 10pm to 3am, last night. But unsuccessfully. Please let me know if you have a better luck.

Thanks

[tovodeverett]

Here's my monkey patch for 3.0.0 - stick this in spec_helper.rb until the powers that be implement it (or something similar).

module Shoulda
  module Matchers
    module ActiveModel
      class ValidationMatcher
        def ignoring_interference_by_writer
          @ignoring_interference_by_writer = true
          self
        end

        def allow_value_matcher(value, message)
          matcher = AllowValueMatcher.new(value).for(@attribute).
            with_message(message)

          if defined?(@context)
            matcher.on(@context)
          end

          if strict?
            matcher.strict
          end

          if @ignoring_interference_by_writer
            matcher.ignoring_interference_by_writer
          end

          matcher
        end

        def disallow_value_matcher(value, message)
          matcher = DisallowValueMatcher.new(value).for(@attribute).
            with_message(message)

          if defined?(@context)
            matcher.on(@context)
          end

          if strict?
            matcher.strict
          end

          if @ignoring_interference_by_writer
            matcher.ignoring_interference_by_writer
          end

          matcher
        end
      end
    end
  end
end

This allows any matcher based on ActiveModel::ValidationMatcher to take advantage of ignoring_interference_by_writer. That means you can now stick that on the end of your validate_uniqueness_of call and be in business.

[mcmire]

Hey @celsoMartins. Sorry that 3.0 has been a little painful for you.

Basically, the matcher is attempting to set "a" on the attribute you're testing, but you're intercepting this in the model and converting that "a" to "A", and so the matcher is confused and isn't smart enough to know that's what should be happening. This is part of an overall change to the allow_value matcher and I'm afraid this exception has been showing its face in other matchers since they use allow_value internally.

I can't think of a better way to solve this than by using the ignoring_interference_by_writer qualifier that I added to allow_value (this is exactly the reason I added it). I think adding it to ValidationMatcher (and validate_numericality_of) is a good idea.

[mcmire]

Actually, I just read @jasnow's comment about how you'd get a different failure message under 3.0.0.rc1 (and, thus, 3.0.0) even if you used ignoring_interference_by_writer. I believe you'd get that because of this change to validate_uniqueness_of.

But I think this raises a larger point. Since you're saying should validate_uniqueness_of(:nome), these are the assertions that should be getting run:

Given an existing Material with nome set to "a",

1. a new Material with nome set to "a" should not be valid (and should have an error message on nome of "has already been taken")
2. a new Material with nome set to "A" should be valid (and should not have that error message)

The problem is that since you're upcasing nome in the model behind the scenes, you're actually making these assertions:

Given an existing Material with nome set to "A",

1. a new Material with nome set to "A" should not be valid (and should have an error message on nome of "has already been taken")
2. a new Material with nome set to "A" should be valid (and should not have that error message)

So this should explain why we're now raising a CouldNotSetAttributeError: there's a deeper issue here, in that it's difficult for validate_uniqueness_of to know what to do here.

Now, I can step in as a human and guess that you want to get rid of the second assertion and just make one:

Given an existing Material with nome set to "A", a new record with nome set to "A" should not be valid (and should have an error message on nome of "has already been taken")

but as it stands, there isn't a way for the matcher to honor this request. Maybe there should be one, but I think this is one of those case where it'd work better if you wrote the test yourself:

it "ensures that nome is unique" do
  existing_material = Material.create!(nome: "SOMETHING")
  new_material = Material.new(nome: "SOMETHING")
  new_material.valid?
  expect(new_material.errors[:nome]).to include("has already been taken")
end

[mcmire]

@tovodeverett Can you open a new issue with some of the matchers that failed for you?

[masterkain]

     Failure/Error: it { is_expected.to validate_numericality_of(:media_file_size) }
     Shoulda::Matchers::ActiveModel::AllowValueMatcher::CouldNotSetAttributeError:
       Expected Class to be able to set media_file_size to "abcd", but got 0 instead.

no particular setter, just the default value in database column, using postgres 9.4

[mcmire]

3.1.0 is now released with a proper fix for this.