Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pull request #139 (released in 0.14.0) breaks message: :improbable_phone option #140

Closed
monfresh opened this issue Apr 28, 2016 · 1 comment

Comments

@monfresh
Copy link
Contributor

In #139, the following code was added:

def options_value(option)
  if options[option].is_a?(Symbol)
    @record.send(options[option])
  else
    options[option]
  end
end

This assumes that any symbol is a method that the record can respond to, which is a false assumption to make. For example, I have this in my app, and upgrading to 0.14.1 breaks it because :improbable_phone is not a User method.

validates_plausible_phone :mobile,
                          country_code: 'US',
                          presence: true,
                          if: :needs_mobile_validation?,
                          message: :improbable_phone

To properly test this feature, two scenarios should be added:

  • A symbol that doesn't correspond to a Model method is passed as an option
  • A symbol that does correspond to a Model method is passed to the message option

In both cases, the method should not be sent to the Model. The reason for the first is obvious, since it will throw a NoMethodError. For the second, it's because you might have an i18n key with the same name as a Model method. In that case, you want the i18n key to be called, not the Model method.

Here is a correct solution:

def options_value(option)
  value = options[option]

  return value if option == :message

  if value.is_a?(Symbol) && @record.respond_to?(value)
    @record.send(value)
  else
    value
  end
end
@monfresh monfresh changed the title Pull request #139 (released in 0.14.1) breaks message: :improbable_phone option Pull request #139 (released in 0.14.0) breaks message: :improbable_phone option Apr 28, 2016
@monfresh
Copy link
Contributor Author

I'll be sending a PR soon with a better solution than what I said above. If the Model doesn't respond to a method and the option is a symbol, we actually do want to raise an error to let the user know they passed in a nonexistent method, except if the option is message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant