Bugfix reindex! when all of targeted records are to delete and nothing to save

[metheglin]

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Related Issue
Need Doc update	no

What problem is this fixing?

How to reproduce

app/models/product.rb

class Product < ApplicationRecord
  enum status: {
    draft: 0,
    opened: 1,
  }

  algoliasearch if: :algolia_published? do
    attribute :title
    attribute :content
  end

  def algolia_published?
    opened?
  end
end

in rails console for instance

Product.draft.limit(3).reindex!

Got the error below

NoMethodError: undefined method `task_id' for nil:NilClass

/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/algoliasearch-rails-3.0.0/lib/algoliasearch-rails.rb:464:in `block (2 levels) in algolia_reindex!'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/relation/batches.rb:138:in `block in find_in_batches'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/relation/batches.rb:245:in `block in in_batches'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/relation/batches.rb:229:in `loop'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/relation/batches.rb:229:in `in_batches'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/relation/batches.rb:137:in `find_in_batches'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/querying.rb:22:in `find_in_batches'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/algoliasearch-rails-3.0.0/lib/algoliasearch-rails.rb:913:in `algolia_find_in_batches'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/algoliasearch-rails-3.0.0/lib/algoliasearch-rails.rb:449:in `block in algolia_reindex!'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/algoliasearch-rails-3.0.0/lib/algoliasearch-rails.rb:442:in `each'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/algoliasearch-rails-3.0.0/lib/algoliasearch-rails.rb:442:in `algolia_reindex!'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/relation/delegation.rb:108:in `public_send'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/relation/delegation.rb:108:in `block in method_missing'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/relation.rb:880:in `_scoping'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/relation.rb:428:in `scoping'
/Users/metheglin/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/activerecord-7.0.2.3/lib/active_record/relation/delegation.rb:108:in `method_missing'

... (Logs continue but let me refrain from share because it includes project information)

Cause

This error happens because I'm using conditional index.
In reindex!, if any records to be deleted, it deletes from algolia and remove from activerecord target.

algoliasearch-rails/lib/algoliasearch-rails.rb

Line 453 in f8f599d

AlgoliaSearch.client.delete_objects(index_name, ids.select { |id| !id.blank? })

And then the rest of activerecord target to be saved.

algoliasearch-rails/lib/algoliasearch-rails.rb

Line 464 in f8f599d

last_task = AlgoliaSearch.client.save_objects(index_name, objects).last.task_id

But when all of the activerecord target to be deleted, there is nothing to be saved. Thus AlgoliaSearch.client.save_objects(index_name, objects) returns empty array. And then .last got nil, .task_id got undefined method.

Describe your change

I just added &. to call task_id. Sorry, I have no idea if I could use &. operator considering supported ruby version.

UPDATED: Since I reflected it's not good to use &., I have added present? check on return value of save_objects explicitly. be4ee11

When saving objects is empty, it seems there is no reason to wait task. So with using present? check, setting last_task as nil seems good enough to handle this case.
But I'm not so sure if it should wait delete_objects task. However, I think it doesn't break existing behavior with this change at least.

[DevinCodes]

Thank you for the PR @metheglin ! 🙇‍♂️

I'll merge & release both your PRs first thing tomorrow morning