Skip to content

Update changed_attributes to attributes_in_database #738

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

Closed
wants to merge 1 commit into from

Conversation

jkeam
Copy link

@jkeam jkeam commented Aug 29, 2017

This PR addresses:

#714

Basically changed_attributes has been deprecated in Rails 5.1. This PR hopefully addresses that. Please let me know if I missed anything.

@Geesu
Copy link
Contributor

Geesu commented Nov 28, 2017

Can we get this merged?

karmi added a commit that referenced this pull request Dec 4, 2017
…k with older Rails versions

This patch builds on work in #738 by @jkeam and #758 by @Geesu to prevent deprecation warnings on Rails 5.1
due to changes in the handling of "changed attributes", originally reported by @davedash in #714.

It's primary focus is to restore the compatibility with older Rails versions (so we don't break compatibility
without proper version bump and in a single isolated commit), and to make the naming a bit more descriptive.

First, the condition has been changed to work with the `changes_to_save` and `changes` methods, as opposed
to the original `changed_attributes` / `attributes_in_database` naming. This communicates much more clearly
the intent, and the original usage of `changed_attributes` has been misleading.

Also, the "internal instance variable" which keeps track of the changes has been renamed to `@__changed_model_attributes`,
in order to cleary differentiate it from the `changed_attributes` method name in older ActiveRecord versions.

Closes #758
karmi added a commit that referenced this pull request Dec 4, 2017
…k with older Rails versions

This patch builds on work in #738 by @jkeam and #758 by @Geesu to prevent deprecation warnings on Rails 5.1
due to changes in the handling of "changed attributes", originally reported by @davedash in #714.

It's primary focus is to restore the compatibility with older Rails versions (so we don't break compatibility
without proper version bump and in a single isolated commit), and to make the naming a bit more descriptive.

First, the condition has been changed to work with the `changes_to_save` and `changes` methods, as opposed
to the original `changed_attributes` / `attributes_in_database` naming. This communicates much more clearly
the intent, and the original usage of `changed_attributes` has been misleading.

Also, the "internal instance variable" which keeps track of the changes has been renamed to `@__changed_model_attributes`,
in order to cleary differentiate it from the `changed_attributes` method name in older ActiveRecord versions.

Closes #758
@karmi
Copy link
Contributor

karmi commented Dec 4, 2017

Hi @jkeam, please accept my apologies for not attending to this patch soon enough.

I've spent some time with the problem, working both through your and @Geesu's patches, and finally merged the solution. My original code using the changed_attributes was quite misleading. I've corrected it in 7815039, and also restored the compatibility with older Rails versions.

Can you please evaluate current master or 5.x branch if it solves your problem? (I've also released a new version to Rubygems.)

@jkeam
Copy link
Author

jkeam commented Dec 4, 2017

Just tested against the 5.x branch and everything looks good! Thanks again @karmi and everyone else who helped on this.

@jkeam jkeam closed this Dec 4, 2017
@karmi
Copy link
Contributor

karmi commented Dec 4, 2017

Thank you for the info, @jkeam, and sorry again for such a long delay!

jordan-brough pushed a commit to Lostmyname/elasticsearch-rails that referenced this pull request Jan 9, 2018
…k with older Rails versions

This patch builds on work in elastic#738 by @jkeam and elastic#758 by @Geesu to prevent deprecation warnings on Rails 5.1
due to changes in the handling of "changed attributes", originally reported by @davedash in elastic#714.

It's primary focus is to restore the compatibility with older Rails versions (so we don't break compatibility
without proper version bump and in a single isolated commit), and to make the naming a bit more descriptive.

First, the condition has been changed to work with the `changes_to_save` and `changes` methods, as opposed
to the original `changed_attributes` / `attributes_in_database` naming. This communicates much more clearly
the intent, and the original usage of `changed_attributes` has been misleading.

Also, the "internal instance variable" which keeps track of the changes has been renamed to `@__changed_model_attributes`,
in order to cleary differentiate it from the `changed_attributes` method name in older ActiveRecord versions.

Closes elastic#758
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

3 participants