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

Restore current blog when the_post triggers outside the loop. #2144

Closed
wants to merge 2 commits into from

Conversation

gonzomir
Copy link
Contributor

@gonzomir gonzomir commented Mar 20, 2021

Description of the Change

As described in #2121, in multisite environment and searching the whole network when the_post is triggered outside the loop, the current site is not restored if the first result is from another blog. To fix the issue I'm checking if the WP_Query instance is in the loop and if not, calling restore_current_blog(). To do this properly I had to add second argument to ElasticPress\Indexable\Post\QueryIntegration->maybe_switch_to_blog().

Alternate Designs

In the project where I first discovered the issue I added another function to the_post to restore the current blog, but that is not necessary if the issue can be addressed at it's roots.

Benefits

Fix the issue, described in #2121.

Possible Drawbacks

I can't find any.

Verification Process

I tested that the issue dissappears in my test setup and made sure current tests are passing.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Issue #2121.

Changelog Entry

Fix: Restore current blog when the_post triggers outside the loop in multisite environment and the whole network is searched if the first result is from another blog.

@gonzomir
Copy link
Contributor Author

I stand corrected, as I didn't run all tests, obviously my changes affect more than I expected...

@felipeelia felipeelia added this to the 3.5.7 milestone Mar 26, 2021
@felipeelia felipeelia modified the milestones: 3.5.7, 3.6.0 Apr 9, 2021
@felipeelia
Copy link
Member

A few things we have to do before merging this one:

  1. Add a default null value to $query. if that is null, then use the global query (just to be safe people aren't relying on this function using just one parameter)
  2. Add another @since there explaining the introduction of the parameter
  3. Looking at that function, we noticed that it can have some performance issues on multi-sites with several different sites. We should add another paragraph to the docblock stating that.

@gonzomir is this something you'd like to do? Otherwise, we can take over the PR and implement it (obviously giving you the credits for the original code.)

Thanks in advance!

@gonzomir
Copy link
Contributor Author

@felipeelia Yes, I can do that next week.

@felipeelia
Copy link
Member

@gonzomir we went ahead and took over the PR just adding those final details. I'm closing this one now and hopefully, the code will be part of our next release. Thank you very much!

@felipeelia felipeelia closed this Aug 1, 2021
@jeffpaul jeffpaul added this to the 3.6.2 milestone Aug 24, 2021
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.

3 participants