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

[Fix #1594] Reset remove attribute after invoking remove_#{image} #1668

Merged
merged 1 commit into from
Sep 11, 2015

Conversation

avgerin0s
Copy link
Member

This fixes the behaviour of write_#{column}_identifier in order to
accept re-uploading a file after the previous one was removed from
the same Uploader's instance.

@avgerin0s
Copy link
Member Author

I'm not sure this should go in this method. Maybe it's a responsibility of write_#{column}_identifier

What do you think? @bensie @plribeiro3000

@plribeiro3000
Copy link
Member

@eavgerinos I'm not sure either but this change its kind of bad. The result of the method will be false when it should reflect if the result of the operation was successful or not.

Maybe we could leave the code as it is if we just pull the super to the end of the method. Thoughts?

@avgerin0s avgerin0s force-pushed the fix-1594 branch 2 times, most recently from eff9ff0 to 2659e4b Compare August 24, 2015 18:48
@avgerin0s
Copy link
Member Author

@plribeiro3000 Fixed. :)

@plribeiro3000
Copy link
Member

👍

@plribeiro3000
Copy link
Member

@eavgerinos If is not to ask too much maybe we could have a test to ensure this won´t break in the future? or at least some documentation?

This fixes the behaviour of write_#{column}_identifier in order to
accept re-uploading a file after the previous one was removed from
the same Uploader's instance.
@avgerin0s
Copy link
Member Author

@plribeiro3000 Added :)

@plribeiro3000
Copy link
Member

Awesome @eavgerinos . You are the best! 👍

avgerin0s added a commit that referenced this pull request Sep 11, 2015
[Fix #1594] Reset remove attribute after invoking remove_#{image}
@avgerin0s avgerin0s merged commit 1e4e7b0 into master Sep 11, 2015
@avgerin0s avgerin0s deleted the fix-1594 branch September 11, 2015 22:54
@avgerin0s
Copy link
Member Author

@plribeiro3000 :)

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.

2 participants