valid? rejects any attribute that is an empty string.

[BrucePerens]

In save_operation.cr, we get automatic validation of any non-autogenerated and non-nilable fields through this code:

 def required_attributes
      Tuple.new(
        {% for attribute in attributes %}
          {% if !attribute[:nilable] && !attribute[:autogenerated] %}
            {{ attribute[:name] }},
          {% end %}
        {% end %}
      )
    end

This then gets called in valid? in the statement validate_required *required_attributes. valdate_required checks for blank_for_validates_required? which ends up checking for blank? not nil?. As a result, you can't save any records with attributes that are empty-but-not-nil strings.

[BrucePerens]

Paul claims this is intended behavior. However, it causes the programmer to declare more union types (String|Nil if you want empty strings, rather than just String), which I feel increases the potential for incorrect code. In general I tend to avoid nilable types, for correctness sake.

It is also going to be necessary to turn off empty-string-rejection when Lucky is used with legacy databases.

[jwoertink]

I'm currently working on refactoring operations. You can hop in the discussion here luckyframework/lucky#1209

I agree with Paul that blank strings should be rejected by require validations; however, I want to add the ability to override this. In that discussion I make mention of the ruby mutations gem which does this same thing. If a value is required, a blank string is treated like a nil value and fails, but if you are ok with having a blank string, they have an option empty: true. I'd probably call it allow_blank: true, but same sort of idea.

This is still in planning stage, but imagine something like this:

before do
  validate_required content, allow_blank: true
end

This would tell content that it has to be a String, but it's ok to be a blank string.

[paulcsmith]

@jwoertink I think rather than overloading the validations we should instead allow people to skip all default validations model by model (or on all models by changing the BaseModel). Similar to what we do with default_columns. In fact...we can probably use default columns in a hacky way to get this working:

table do
  skip_default_validations # can add this to `default_columns` in BaseModel if desired
end

This would skip generating the default validations and would leave it up to the user to decide how they want to validate data.

This would be reasonably simple to add, and would make it clear to users reading the model that those validations are not being run.

I think we'd also need a configuration option in Lucky (and maybe Avram?) that tells Lucky to return "" instead of nil when accessing params. Lucky.configure &.allow_empty_params = true

This should cover the use-cases for legacy databases and for new ones that opt-to store empty strings.

[paulcsmith]

To clarify: we can start with just the skip_default_validations as that will allow people to use legacy databases with Avram. The change for params is really for people who want to design their database where empty string represents nil. Which is a fine thing to do but just want to be clear they are no dependent on each other and can be released separately

[sjackson]

Hi, is the skip_default_validations option available yet?

[jwoertink]

Hey @sjackson, It's not available yet; however, I would like to get it in for the next release, so I'm going to start looking at it either today or tomorrow.

[jwoertink]

There's now a PR up to add this in. I've added this in as an opt-in escape-hatch at the SaveOperation level.