Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Old uploads stay in storage after update #159

Open
avgerin0s opened this issue Apr 13, 2016 · 4 comments
Open

bug: Old uploads stay in storage after update #159

avgerin0s opened this issue Apr 13, 2016 · 4 comments

Comments

@avgerin0s
Copy link
Member

When updating a document's attachment, the old attachment does not get deleted.

An example application that demonstrates this bug can be found here.

The related test looks like this

test "it deletes old images upon update" do
    doc = Zedocument.new

    # Initial image and creation
    doc.remote_filename_url = "https://www.ruby-lang.org/images/header-ruby-logo.png"
    doc.save

    old_path = doc.filename.path

    assert(File.exists?(old_path))

    # Update the image
    doc.remote_filename_url = "http://memesvault.com/wp-content/uploads/Wat-Meme-Old-Lady-01.jpg"
    doc.save

    new_path = doc.filename.path

    assert_not_equal(new_path, old_path)

    assert(File.exists?(new_path))
    assert_not(File.exists?(old_path))
  end

An example workaround I used for a forked version is this for carrierwave-mongoid as well as this for carrierwave because mongoid returns a Hash as with Strings keys for changes.

If you agree with those changes I can craft a PR for carrierwave-mongoid and I can merge the carrierwave changes.

@avgerin0s
Copy link
Member Author

The same problem continues to exist for carrierwave v0.11

@avgerin0s avgerin0s changed the title bug: Old images stay in storage after update bug: Old uploads stay in storage after update Apr 13, 2016
@rmm5t
Copy link
Member

rmm5t commented Apr 13, 2016

Yes, a PR would be much appreciated. Thanks.

@rmm5t
Copy link
Member

rmm5t commented Oct 9, 2016

Related to #125?

@leoarnold
Copy link
Contributor

I translated @avgerin0s's initial example to RSpec, but was not able to reproduce.

The following spec passes with all versions of Ruby, Mongoid and CarrierWave:

  describe "Issue 159" do
    let(:model_class) { reset_mongo_class }

    it 'deletes the old path' do
      doc = model_class.new

      # Initial image and creation
      doc.remote_image_url = 'https://picsum.photos/500'
      doc.save

      old_path = doc.image.path

      expect(File).to exist(old_path)

      # Update the image
      doc.remote_image_url = 'https://picsum.photos/200'
      doc.save

      new_path = doc.image.path

      expect(new_path).to_not eq(old_path)

      expect(File).to exist(new_path)
      expect(File).to_not exist(old_path)
    end
  end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants