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

Fixed atomic reindex to ensure init when check_settings=true. #453

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

metheglin
Copy link

@metheglin metheglin commented Nov 27, 2024

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

What problem is this fixing?

Atomic reindex (ex: Product.reindex) is always going to create tmp index with empty setting. And index move completed, previous settings disappeared. Not only specified settings inside algoliasearch block, but manual setting on dashboard.

How to Reproduce

app/models/product.rb

class Product < ApplicationRecord
  algoliasearch check_settings: true do
    attribute :title
    attribute :content
    attribute :updated_at_i
    
    customRanking ['desc(updated_at_i)']
    searchableAttributes ['title']
    indexLanguages ['ja']
  end
end

in rails console for instance

Product.reindex
  # Check dashboard, all specified settings are disappeared.
  # Not only specified settings inside algoliasearch block, but manual setting on dashboard.

Describe your change

I understood the check_settings specification like this:

When check_settings: true (default), all the index operation going under ActiveRecord respect the specified settings in algoliasearch block.
On the other hand when check_settings: false, users just want to manage it on dashboard. So when new tmp index is to create, it just takes over the settings from existing index.

If my understanding is true, I guess current implementation accidentally missed algolia_ensure_init for tmp index when check_settings=true. I have imported that case from previous version below.

https://github.com/algolia/algoliasearch-rails/blob/9c1a0a46ca3dd4487cc3461c755a3fef144db491/lib/algoliasearch-rails.rb#L554C23-L554C86

Copy link
Contributor

@DevinCodes DevinCodes left a comment

Choose a reason for hiding this comment

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

Hey @metheglin , thank you for your PR! Did you test out your changes by any chance to confirm the behaviour after this change is as expected? 🙂

@metheglin
Copy link
Author

@DevinCodes

Thank you for reply.
I only confirmed that my problem resolved. With version 3.0.0, Product.reindex always rollback every settings even though settings described in ActiveRecord model (See "How to Reproduce"). With this PR version, I can check that settings described in model are well maintained on dashboard with latest data.

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