active_storage_validations icon indicating copy to clipboard operation
active_storage_validations copied to clipboard

Passing :allow_blank option raises

Open joelmoss opened this issue 11 months ago • 4 comments

I am trying to allow a blank attachment during validation...

has_one_attached :photo
validates :photo, attached: true, on: %i[create update], allow_blank: true,
                    content_type: { in: %r{\Aimage/.*\z}, message: :image_content_type_invalid },
                    size: { less_than: 5.megabytes }

But it fails with...

ArgumentError: You cannot pass the :allow_blank option to this validator (ArgumentError)

I can see in the code that the allow_blank option is supported, so why does this not work?

thankyou

joelmoss avatar Feb 27 '24 11:02 joelmoss

Hi @joelmoss The error comes from the attached validator (https://github.com/igorkasyanchuk/active_storage_validations/blob/master/lib/active_storage_validations/attached_validator.rb). It does not accept the allow_blank option when activated, since it requires to attach. It would be contradictory to require to attach something and to allow not to attach something, therefore the error. Maybe the solution is to remove the attached: true validator?

Mth0158 avatar Feb 28 '24 18:02 Mth0158

I am pushing a quick MR to make the message clearer for future users

Mth0158 avatar Mar 12 '24 16:03 Mth0158

👋 here, and thanks for the gem.

I'm also wondering about allowing optional attachement and I went to this issue. I understand with your answer that removing attached: true would work to keep attachement optional.

But it's not very easy to read and guess for other developpers:

for example with this line: validates :logo, content_type: [...] it's not super clear that logo is optional.

Instead of adding a comment in our code everywhere optional attachements are validated, Would it work too by adding explicit attached: false, or attached: nil?

Or should I go to validates :logo, content_type: [...], allow_blank: true (no attached validator, but allow_blank allowed in this case?)

thanks in advance for your answer, if you want I can open a tiny PR on ready to highlight this usecase.

Polo2 avatar Apr 24 '24 08:04 Polo2

Hi from Lyon @Polo2 👋

We could develop something like attached: false, but I think it's not the best option here.

ActiveStorage has_one_attached / has_many_attached association methods, like ActiveRecord has_one and have_many, by default allow to have empty child / children relation. It implicitly "implies" the allow_nil / allow_blank options.

You could write your association with one of these to make you code more explicit. All the gem validators, expect the attached one, allow these options (check our tests at https://github.com/search?q=repo%3Aigorkasyanchuk%2Factive_storage_validations+WorksWithAllRailsCommonValidationOptions&type=code).

I think it's by far the simplest & clearer option, but I am open to discussion :)

Mth0158 avatar Apr 24 '24 09:04 Mth0158

I got hit by this today after I upgraded to newer version of this gem. How do I specify that an attachment is optional now?

This was my code before:

  has_one_attached :profile_picture
  validates :profile_picture, attached: true, allow_blank: true, content_type: ["image/jpg", "image/jpeg", "image/png", "image/gif", "image/webp"]

How should this be modified with the newer version? Should I just drop attached: true?

amit avatar Sep 06 '24 03:09 amit

Hi @amit! Looking at your code, you both require an attachment attached: true and allow no attachment allow_blank: true. If you want to make attachment optional, you have have to remove the attached: true. Plus, you could also remove the allow_blank: true, by default ActiveStorage allows no attachments, so the option is more or less useless :)

Mth0158 avatar Sep 07 '24 15:09 Mth0158

Thanks! So the new code then should be:

  has_one_attached :profile_picture
  validates :profile_picture, content_type: ["image/jpg", "image/jpeg", "image/png", "image/gif", "image/webp"]


Correct? This allows no attachment as valid, but if it is attached, then it needs to comply with the content_type given?

The documentation of attached does not specify what it really means and all examples seem to show the value of true!

amit avatar Sep 07 '24 15:09 amit

@amit that's exactly it! Your code does what you explained. I do agree with you, the readme could be rewritten to explain the available options for each validator, it's not the clearest right now.

Mth0158 avatar Sep 08 '24 08:09 Mth0158