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

validate_size_of implementation is confused #570

Closed
BrucePerens opened this issue Nov 13, 2020 · 1 comment · Fixed by #579
Closed

validate_size_of implementation is confused #570

BrucePerens opened this issue Nov 13, 2020 · 1 comment · Fixed by #579
Assignees
Labels
documentation This just needs more/better documentation

Comments

@BrucePerens
Copy link

The comments here confuse whether validate_size_of is supposed to validate the numeric value of something, or the size of something. This produces incorrect documentation at https://luckyframework.github.io/avram/Avram/Validations.html#validate_size_of(attribute:Avram::Attribute,min=nil,max=nil,allow_nil:Bool=false)-instance-method
This is also reflected in the incorrect documentation in the guides on the web site, see luckyframework/website#489

  # Validate the size of the attribute is within a `min` and/or `max`
  #
  # ```
  # validate_size_of age, min: 18, max: 100
  # validate_size_of account_balance, min: 500
  # ```
  def validate_size_of(
    attribute : Avram::Attribute,
    min = nil,
    max = nil,
    allow_nil : Bool = false
  )
    if !min.nil? && !max.nil? && min > max
      raise ImpossibleValidation.new(
        attribute: attribute.name,
        message: "size greater than #{min} but less than #{max}")
    end

    unless allow_nil && attribute.value.nil?
      size = attribute.value.to_s.size
@jwoertink jwoertink transferred this issue from luckyframework/lucky Dec 22, 2020
@jwoertink jwoertink added the documentation This just needs more/better documentation label Dec 22, 2020
@jwoertink
Copy link
Member

Yeah, the docs on this are wrong. This method checks that a string length (size) is within a specific range.

Also related: #537 for adding a new method that does what these docs intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This just needs more/better documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants