Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Improve ES::Model README suggestion for async indexing with after_save_commit #1022

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

Open
trappist wants to merge 1 commit into elastic:main
base: main
Choose a base branch
Loading
from trappist:main

Conversation

Copy link

@trappist trappist commented Mar 15, 2022

I got a lot of test failures using the original suggestion. First because newly created records weren't yet committed when the (test) worker ran, so I switched to after_commit. But after_commit also fires on destroy, so it would fail trying to reindex destroyed records. Rails 6 introduces after_save_commit for this purpose. Prior to Rails 6 it would be after_commit ..., on: [:create, :update], but if you needed to change the next line to after_commit ..., on: :destroy the latter callback would clobber the former, and this prevents that.

* after_commit solves testing problems by ensuring create is committed
* Use rails 6's after_save_commit to avoid reindexing on destroy
Copy link

❌ Author of the following commits did not sign a Contributor Agreement:
9fec676

Please, read and sign the above mentioned agreement if you want to contribute to this project

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@shashankjo shashankjo shashankjo approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /