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

rails 5.1.1 deprecation warning for changed? #1059

Closed
valdis opened this issue Jun 8, 2017 · 6 comments
Closed

rails 5.1.1 deprecation warning for changed? #1059

valdis opened this issue Jun 8, 2017 · 6 comments

Comments

@valdis
Copy link

valdis commented Jun 8, 2017

in lib/thinking_sphinx/active_record/callbacks/delta_callbacks.rb

    48: def new_or_changed?
 => 49:   instance.new_record? || instance.changed?
    50: end

throws

DEPRECATION WARNING: The behavior of `changed?` inside of after callbacks will be changing in the next version ofRails. The new return value will reflect the behavior of calling the method after `save` returned (e.g. the opposite of what it returns now). To maintain the current behavior, use `saved_changes?` instead. (called from changed? at app/models/<model.rb>)

P.S. thanks for the gem, it has been very useful!

@pat
Copy link
Owner

pat commented Jun 10, 2017

Thanks for pointing this out. I'm struggling to get the same warning locally though. Can you confirm which versions of Rails and Thinking Sphinx you're using?

pat added a commit that referenced this issue Jun 10, 2017
This change occured in ActiveRecord/Rails 5.1. Hunting for the deprecation raised in #1059 helped me debug this one (though that issue’s not yet resolved).
@avitus
Copy link

avitus commented Jun 22, 2017

I'm getting these deprecation warnings with Thinking Sphinx 3.3 and Rails 5.1.0

@pat
Copy link
Owner

pat commented Jun 23, 2017

@avitus are there specific scenarios where this deprecation warning occurs? e.g.

  • Updating a model instance which has a delta index
  • Updating a model instance which then updates an associated instance, which in turn has a delta index

The more detail, the better :)

I'm not sure what's provoking this specific warning (about using changed?) - it doesn't appear the test suite, which is reasonably thorough - and the use of changed? that @valdis highlights isn't normally called within an after_ callback.

@avitus
Copy link

avitus commented Jun 23, 2017

@pat I actually appear to have an example of each of the scenarios you mentioned.

In one case it is an after_create callback which updates a field in a model instance based on three other fields.

In the second case, I'm essentially creating a type of linked list so when one model instance changes, 'next' and 'prev' pointers in associated models are updated.

Not sure whether that helps at all.

@pat
Copy link
Owner

pat commented Aug 28, 2017

@avitus it may have taken me two months to respond, but that information is definitely helpful.

From what I can tell, both from the scenarios you're describing and the testing I've done locally: these warnings only crop up because an after_x callback invokes a further change on the model, which in turn invokes the before_save on the delta callbacks. I'd expect the internal behaviour here to be fine - it's just that the Rails deprecation warnings aren't smart enough to know about inner callbacks, hence the misleading warning. Not sure if there's much to be done about this. 🤷‍♂️

@pat
Copy link
Owner

pat commented Oct 5, 2017

Closing this, as I think it's more an issue with Rails' deprecation warnings not picking up on nested callbacks.

@pat pat closed this as completed Oct 5, 2017
h3nnn4n pushed a commit to jobscore/thinking-sphinx that referenced this issue May 14, 2019
This change occured in ActiveRecord/Rails 5.1. Hunting for the deprecation raised in pat#1059 helped me debug this one (though that issue’s not yet resolved).
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