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

Don't store _old_ if we're passed nil #175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

artfuldodger
Copy link

Saying, for example user.update(avatar: nil) will result with the avatar_filename set to "old", which is unexpected.

@rmm5t
Copy link
Member

rmm5t commented Oct 15, 2017

Sorry for the awful delay here. Thanks to #179, and after a hiatus, I'm now in the process of catching up with things on this project.

If this issue is still a problem, could you please rebase on master to get the test suite green for this PR?

@artfuldodger
Copy link
Author

@rmm5t No worries! I rebased off master, but I'm not sure if this change is still necessary. Will try to play around some more to see.

@simonc
Copy link
Contributor

simonc commented Apr 26, 2022

Hello there. We're facing this exact issue so I think this change is still necessary 🙂

simonc added a commit to textmaster/carrierwave-mongoid that referenced this pull request Apr 26, 2022
@simonc
Copy link
Contributor

simonc commented Apr 26, 2022

That being said, testing this PR in a custom fork it seems that it should be new_file.path.nil? instead.

simonc added a commit to textmaster/carrierwave-mongoid that referenced this pull request Apr 26, 2022
simonc added a commit to textmaster/carrierwave-mongoid that referenced this pull request Apr 27, 2022
@rmm5t
Copy link
Member

rmm5t commented Jan 19, 2023

@artfuldodger @simonc Sorry for the long delay with this PR. Would someone be willing to investigate this further while also adding a spec to confirm behavior?

@artfuldodger
Copy link
Author

I'm not using Mongo any more, so I'm of no use here

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

Successfully merging this pull request may close these issues.

3 participants