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

Fix remove from index on update #3937

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

dtakken
Copy link

@dtakken dtakken commented Jun 12, 2024

This change fixed an issue that I came across: Setting a password on a post and updating it from the admin page did not remove it from the index, unless I hit the update button a second time.

Details about the bug can be found here: #3936

This PR makes the last running filter have the final say about adding the post to the index or not.

I found this issue on WordPress 6.4.3 using ElasticPress 4.7.2. As far as I can see the relevant code has not changed since. I did not test this change on the latest ElasticPress version yet.

I tried to whip up a Cypress test to demonstrate the issue but it does not seem to work correctly. I am not familiar with Cypress, some help is appreciated.

Closes #3936

How to test the Change

  1. Create and publish a post
  2. Check that the post is in de index
  3. Change visibility, set a password
  4. Update the post
  5. Check that the post is no longer in the index

Changelog Entry

Fixed - Failure to remove post from index during update for some WordPress versions

Credits

Props @dtakken

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

This change fixes updates that trigger multiple filters for the same
post. Currently, when any of the filters adds the post to the update
queue the post is indexed, even when other filters determine
that the post should not be indexed. This change makes the last
filter have the final word about this.
@felipeelia
Copy link
Member

Hi @dtakken, thanks for sending the PR! We'll have it reviewed and get back to you with our findings/approval. It can take a while though, so I'll ask you some patience :)

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.

BUG: Removal from index on update may fail
2 participants