carrierwaveuploader / carrierwave

Classier solution for file uploads for Rails, Sinatra and other Ruby web frameworks
https://github.com/carrierwaveuploader/carrierwave
8.78k stars 1.65k forks source link

Upload fails when calling ActiveRecord model 'dup' with after_create or after_update. #2700

Closed macera closed 9 months ago

macera commented 10 months ago

Upload fails when calling ActiveRecord model 'dup' with after_create or after_update. since version 3.0.0rc

e.g. counter_culture calls dup on after_create, after_update.

rajyan commented 10 months ago

See https://github.com/carrierwaveuploader/carrierwave/issues/2689

I think it's fixed in the latest release (3.0.3).

rajyan commented 10 months ago

Sorry, it wasn't released yet...

macera commented 10 months ago

Thanks for letting me know #2689 ! It is the same problem. But I checked on the master branch and it isn't fixed...

After calling ActiveRecord model 'dup' with after_create or after_update, @cache_id==nil happens with after_save.

carrierwave/lib/carrierwave/uploader/store.rb

def store!(new_file=nil)
  cache!(new_file) if new_file && !cached?
  puts @cache_id #=> nil
  if !cache_only && @file && @cache_id
    with_callbacks(:store, new_file) do
      new_file = storage.store!(@file)
      ...

Therefore, it cannot be uploaded.

rajyan commented 9 months ago

But I checked on the master branch and it isn't fixed...

You're right. I've confirmed that using counter_cluture with carrierwave still doesn't work even after fixing dup bug in https://github.com/carrierwaveuploader/carrierwave/pull/2690 .

The file key is persisted to the DB, but the file is not stored to the storage.

rajyan commented 9 months ago

counter_culture is using dup here https://github.com/magnusvk/counter_culture/blob/a99a73daceba6304954961da3d834eee6d089e0f/lib/counter_culture/counter.rb#L312-L320

rajyan commented 9 months ago

@macera

Could you please confirm this patch fixes your issue?

macera commented 9 months ago

@rajyan Ohhhh...! I've confirmed that the problem has been fixed !

macera commented 9 months ago

@mshibuya Do you have a plan for the next release that includes this fix? I'd appreciate it if you could let me know when.

mshibuya commented 9 months ago

Just released 3.0.4 now! 🚀