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

Add filter for search_pattern in handle_generate_endpoint #1872

Open
cguagenti opened this issue Apr 8, 2024 · 3 comments
Open

Add filter for search_pattern in handle_generate_endpoint #1872

cguagenti opened this issue Apr 8, 2024 · 3 comments

Comments

@cguagenti
Copy link

Context:

I'm testing Faust with WordPress Bedrock, and due to the different structure, when previewing content, it results in a 404 error.

This issue is caused by the $search_pattern in handle_generate_endpoint().

function handle_generate_endpoint() {
$search_pattern = ':^' . site_url( '/generate', 'relative' ) . ':';
if ( ! preg_match( $search_pattern, $_SERVER['REQUEST_URI'] ) ) { // phpcs:ignore WordPress.Security
return;
}

Because Bedrock returns the prefix /wp/ in site_url( '/generate', 'relative' ) (/wp/generate) while $_SERVER['REQUEST_URI'] contains only /generate[...], it always fails at the statement below:

if ( ! preg_match( $search_pattern, $_SERVER['REQUEST_URI'] ) ) { // phpcs:ignore WordPress.Security
	return;
}

Suggestion

Since site_url() returns the URL for the current site where WordPress applications are accessible, the best way to move forward is to use a callback filter (apply_filters) so anyone can customize it with a callback filter.

/**
* Filter 'faustwp_generate_endpoint_search_pattern'.
*
* @param string  $value   The search pattern.
*/
$search_pattern = apply_filters( 'faustwp_generate_endpoint_search_pattern', $search_pattern );

Example of how to use the filter

add_filter('faustwp_generate_endpoint_search_pattern', function ($value) {
	return str_replace('/wp/', '/', $value);
});
@jan-clockworkwp
Copy link

Hey @theodesp, I am confirming that @cguagenti suggestion solves previews pages 404 issue on non-WordPress standard installations where admin url differs from the siteurl. Can we hope to have that filter added to the official package, or maybe as an option in the plugin options? Many thanks.

@ChrisWiegman
Copy link
Member

I like your idea here but I don't think it solves the whole problem. We actually have a 2nd ticket right now, #1882, that looks to solve Bedrock paths with a JS filter in Faust. In isolation both approaches would solve the specific symptoms but the larger issue of Bedrock and similar architecture remains. I would rather solve this upstream where 1. a single change will make sure Bedrock and similar work without multiple filters, etc and 2. it should be solvable whether you're using Faust or not. To further discuss an approach I've opened wp-graphql/wp-graphql#3145 as a result.

@justlevine
Copy link
Contributor

justlevine commented May 31, 2024

On quick review, the underlying issue is that Faust is conflating/inconsistently using the site_url, home_url, and internal Faust frontend_uri, both on the backend and frontend sides.

(I assume the decision to create this endpoint on parse_request instead of as a real endpoint (RESTful or otherwise) was borne out of this as well ).

The immediate solution from a code quality, tech debt and DX pov would be to drop the optimistic home_url === site_url === wpUrl, and use the correct ones on the frontend (long term also in the plugin, but that's beyond the scope if this particular SERVER['REQUEST_URI] check).

I'm assuming that's the premise for the exploration happening in wp-graphql/wp-graphql#3145, but I'm not seeing the why we would want to query the siteUrl via GraphQL before we use it elsewhere in the frontend (the request in that ticket).

This would seem to make much more sense as an optional .env constant (that inherits the home url if unset)

@ChrisWiegman can you confirm/correct my assessment?

@ChrisWiegman ChrisWiegman assigned theodesp and unassigned ChrisWiegman Jun 5, 2024
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

5 participants