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 #1571] - Do not write to ActiveModel::Dirty changes when assigning ... #1635

Merged
merged 1 commit into from May 20, 2015

Conversation

avgerin0s
Copy link
Member

...something blank to a mounter that was originally blank

This is a WIP. I'm not sure it doesn't break anything yet and want to experiment with it before merging.

Feedback is more than welcome

@avgerin0s
Copy link
Member Author

@bensie @plribeiro3000 can you take a look at this please?

@plribeiro3000
Copy link
Member

I believe this is a safe change. I might be missing something but its a really simple change. 👍

@bensie
Copy link
Member

bensie commented May 1, 2015

👍

@rusikf
Copy link

rusikf commented May 18, 2015

Change looks useful, but I think this line is not 'easy readable':
if !(new_file.blank? && send(:#{column}).blank?)
I am not sure, but may be there is a more convenient way to validate empty/blank value ?

@avgerin0s
Copy link
Member Author

@rusikf I can't think of a better way than blank?. Suggestions are always welcome :)

@bensie
Copy link
Member

bensie commented May 19, 2015

I agree @eavgerinos this is fine as-is.

@avgerin0s
Copy link
Member Author

Will merge it tomorrow after giving it a last crash-test. ^_^


context "and the previous value was an empty string" do
before do
@event.image = ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you can this guy above to a before block inside the context when empty string is assigned and get a little dry. What do you think?

…ng something blank to a mounter that was originally blank
avgerin0s added a commit that referenced this pull request May 20, 2015
[Fix #1571] - Do not write to ActiveModel::Dirty changes when assigning ...
@avgerin0s avgerin0s merged commit d7b18dd into master May 20, 2015
@avgerin0s avgerin0s deleted the fix_1571 branch May 20, 2015 20:35
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.

None yet

4 participants