Fix 5.1.0 regression from using saved_changes instead of changes

[paulsturgess]

The PR I raised: #2130 successfully removed the deprecation warnings but unfortunately introduced a regression.

Rails 5.1.0 saved_changes behaves differently to changes: it returns the previous non existent change for the images column as nil instead of an empty array.

saved_changes returns:

{"id"=>[nil, 1], "images"=>[nil, ["test.jpeg"]]}

whereas changes returns:

{
 "id"=>[nil, 1],
 "images"=>
  [[],
   [#<#<Class:0x00000003dbe158>:0x0000000322a828
     @cache_id=nil,
     @cache_storage=#<CarrierWave::Storage::File:0x0000000321cbd8 @uploader=#<#<Class:0x00000003dbe158>:0x0000000322a828 ...>>,
     @file=#<CarrierWave::SanitizedFile:0x000000028dfd90 @content_type="image/jpeg", @file="/app/spec/public/uploads/test.jpeg", @original_filename=nil>,
     @filename="test.jpeg",
     @model=#<Event:0x0000000345cb78 id: 1, image: nil, images: ["test.jpeg"], textfile: nil, textfiles: nil, foo: nil>,
     @mounted_as=:images,
     @original_filename="test.jpeg",
     @storage=#<CarrierWave::Storage::File:0x000000031f33f0 @uploader=#<#<Class:0x00000003dbe158>:0x0000000322a828 ...>>,
     @versions={}>]]
}

[mshibuya]

We need failing spec on this issue, could you provide one?

[paulsturgess]

@mshibuya Looking again there are two specs for removing images that already cover this:

• /spec/orm/activerecord_spec.rb:1179
• /spec/orm/activerecord_spec.rb:1147

They failed on master in this build and now in the build for this PR they pass.

Those two specs specifically test the CarrierWave::Mounter when being used with ActiveRecord and call remove_previous as part of the after_comit callback "remove_previously_stored_#{column}".

[mshibuya]

Sorry for that, thanks ❤️