Attachments should be detached when assigned a blank value

[Kyle Fox]

ActiveStorage allows attachments to be attached by assigning a string representing a blob's signed_id. This works _brilliantly_ because it enables the same params conventions used when assigning other attributes through forms → controllers → models:

params = { user: { image: 'eyJfcmF--e31aef3' } }

However, it's surprising that setting the attachment to an empty string does _not_ detach the attachment:

params = { user: { image: '' } } # => InvalidSignature

It feels more Railsy to treat the empty string as a special case that detaches the attachment.

This would allow attachments to be removed in a way that mirrors how they are added (i.e. posting hidden field values) instead of having to call @user.image.detach in a controller. For example, this would handle both attaching and detaching @user.image:

@user.update_attributes(params.require(:user).permit(:name, :email, :image))

Ideally in this scenario, if params[:user][:image] was '' ActiveStorage would call @user.image.detach.

Currently if you assign an empty string to an attachment an ActiveSupport::MessageVerifier::InvalidSignature error is raised.

Apparently this is already how has_many_attached behaves — it'd be nice if has_one_attached behaved similarly.

Any interest in this functionality? If so I'd be happy to put together a patch. 👍

[geo...@basecamp.com]

To be clear, the writer method added by has_many_attached does not already accept empty strings.

What would we do in the following cases? Ignore the empty strings?

user.update! highlights: [ "" ]

user.update! highlights: [ "eyJfcmF--e31aef3", "" ]

George

[Kyle Fox]

Sorry, I should've been more specific in my initial post:

• I'm proposing this change only for has_one_attachment i.e. the ability to use user.update! image: "" in place of user.image.detach
  
• When comparing the behavior to has_many_attached, I mean in the sense that has_many_attached detaches attachments when assigned a blank value (nil or []), i.e. user.update! highlights: nil

What would we do in the following cases? Ignore the empty strings? 

user.update! highlights: [ "" ]
user.update! highlights: [ "eyJfcmF--e31aef3", "" ]

You're right — in these cases it's not clear what should happen.

Currently the empty strings would raise the InvalidSignature error. I suppose it would behave like this, if blank strings were treated as "detachments" / discarded?

• user.update! highlights: [ "" ] would effectively detach all highlights
  
• user.update! highlights: [ "eyJfcmF--e31aef3", "" ] would attach a single blob in the collection, i.e. user.highlights.length == 1
  

Kyle.

[George Claghorn]

I think I’m okay with recognizing a special value so that you can more easily delete an attachment via a form, but given that we can choose any value, I’d prefer something other than an empty string (like "delete" or { "delete" => true }).

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

[Kyle Fox]

Hmmm, accepts_nested_attributes uses the special key "_destroy" to indicate that an associated object should be destroyed (docs).

Maybe ActiveStorage could mirror this, i.e.

1. user.image = "_destroy"
   
2. user.image = { "_destroy" => true }

I'd lean towards "_destroy" because it more closely resembles the format when assigning a blob.signed_id (a single string). But I can also see the case for the hash. Could both be supported, potentially?

Cheers,

Kyle.