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

is_admin() checks and AJAX context #2148

Closed
felipeelia opened this issue Mar 26, 2021 · 8 comments
Closed

is_admin() checks and AJAX context #2148

felipeelia opened this issue Mar 26, 2021 · 8 comments

Comments

@felipeelia
Copy link
Member

Describe the bug

As shared in #2114 by @moritzlang, several of our conditionals for is_admin() should take into account AJAX requests, where is_admin() is true.

We already have the ep_ajax_wp_query_integration filter that could (and should) be used in those tests. To keep things clean, we can create a new utils function called is_admin_or_ajax() that would contain both conditionals in it.

Steps to Reproduce

Run a WP_Query with ep_integrate in an AJAX request and see that even the weighting fields are included.

Expected behavior

Queries running via AJAX should mimic the behavior of the front end, especially when ep_ajax_wp_query_integration is true.

Additional context

Places we've identified that should use that new function:

@moritzlang
Copy link

Hi @felipeelia,

Regarding the issue description:

Steps to Reproduce

Run a WP_Query with ep_integrate in an AJAX request and see that even the weighting fields are included.

Just to keep this correct: no weighting fields are included in this case. Also fuzziness is not changed!

I found another issue occurring regarding the is_admin() checks when ep_ajax_wp_query_integration is set to true (I need your help on this one):

Steps to Reproduce:

  • ep_ajax_wp_query_integration is enabled
  • Protected Content is (by default) disabled

Since ep_ajax_wp_query_integration evaluates to true, elasticpress will be used for every ajax query (this is the related line in the code). Included ajax queries made by the wp backend (e.g. media library search). But because protected contents are not indexed, the search in the media library will return no results.

Solution:
We need to differentiate between ajax calls from the backend and ajax calls from the frontend (is this a good way to deal with this? (see end of post). Additionally we have to take into account the "Protected Content" setting (How to evaluate if this is enabled? Is there a filter?), which enables elasticpress in the wp admin backend.

The condition in Search.php would look something like this:

if ( ! ( $protected_content_enabled || ( ! is_admin_request() && $admin_integration ) ) ) {
  return;
}

Other is_admin() checks can then be removed, since the evaluation happens in Search.php and there should be no differentiation in other parts of the code.
I already incorporated a similar solution of this into my code base, which works fine.

@felipeelia felipeelia modified the milestone: 3.6.0 Apr 9, 2021
@moritzlang
Copy link

Hi @felipeelia,

will this be fixed with the release of 3.6.0?

@lkraav
Copy link

lkraav commented Jul 1, 2021

Based on https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/load.php#L951 core convention seems to be:

// Protect the admin backend.
if ( is_admin() && ! wp_doing_ajax() ) {
  return true;
}

@felipeelia
Copy link
Member Author

Unfortunately, it wasn't @moritzlang . We've been actively working on it though, as it'll be needed to a feature we are creating but probably it'll be merged only for 3.7.0. Stay tuned!
And thanks for the tip, @lkraav!

@mckdemps
Copy link
Contributor

@JakePT Do you have an update on this? Does this overlap with the work you're doing on the modal search?

@JakePT
Copy link
Contributor

JakePT commented Jul 14, 2021

@mckdemps I did implement something for this for that feature. I'll look into how feasible it is to maybe pull it out and merge it separately.

@danielecelsa
Copy link

Unfortunately, it wasn't @moritzlang . We've been actively working on it though, as it'll be needed to a feature we are creating but probably it'll be merged only for 3.7.0. Stay tuned!
And thanks for the tip, @lkraav!

May I ask when you are planning to release version 3.7.0?

@felipeelia
Copy link
Member Author

@danielecelsa the new function will be part of 3.6.2 planned for the next couple of days. Stay tuned!

@felipeelia felipeelia added this to the 3.6.2 milestone Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants