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

withAPIData URL parameters and Plain Permalinks #3215

Closed
2 tasks
Soean opened this issue Oct 28, 2017 · 9 comments · Fixed by #4877
Closed
2 tasks

withAPIData URL parameters and Plain Permalinks #3215

Soean opened this issue Oct 28, 2017 · 9 comments · Fixed by #4877
Assignees
Labels
Core REST API Task Task for Core REST API efforts [Feature] Permalink The permalink of a post or page and the experience of setting or editing it [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.

Comments

@Soean
Copy link
Member

Soean commented Oct 28, 2017

Issue Overview

If you use plain permalinks and the withAPIData URL has parameters, the request fails because of two question marks.

Example: /index.php?rest_route=/wp/v2/users?context=edit&per_page=100 Failed to load resource: the server responded with a status of 404 (Not Found)

Steps to Reproduce (for bugs)

  1. Enable Plain Permalinks in the settings.
  2. Open the Gutenberg Editor
  3. See the JS Error in the Console

Related Issues and/or PRs

#3121 Delete post fails with plain permalinks and force=true

Todos

  • Tests
  • Documentation
@RickorDD
Copy link
Contributor

RickorDD commented Oct 28, 2017

the same problem with
rest_route=/wp/v2/posts/id?force=true
that call with all languages but not English (United States)

Is this the correct call?
rest_route=/wp/v2/posts/id&force=true

But force=true is not the correct call to delete a post to trash

@Soean
Copy link
Member Author

Soean commented Oct 28, 2017

@RickorDD The force problem is a core wp-api.js bug: https://core.trac.wordpress.org/ticket/40672#comment:19

@aduth
Copy link
Member

aduth commented Oct 28, 2017

I expect the solution is to URL encode the REST path if it is appended to the root as a query argument?

Arguably might be the responsibility of the core wp.apiRequest to handle, since this is where the concatenation occurs:

https://github.com/WordPress/WordPress/blob/cb7483b59543cd973cb2a7c2d4f9a5852e5e9d5c/wp-includes/js/api-request.js#L40-L42

@aduth aduth added Core REST API Task Task for Core REST API efforts [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked. labels Oct 28, 2017
@aduth
Copy link
Member

aduth commented Oct 30, 2017

@rachelbaker
Copy link
Contributor

Taking a look at this...

@bobbingwide
Copy link
Contributor

It would be nice if Gutenberg could perform a common sense check in gutenberg_pre_init() to see if plain permalinks are being used and WordPress < 5..., or whichever version has the fix for TRAC 42382.

@danielbachhuber
Copy link
Member

@aduth Could we shim this in Gutenberg for the time being?

@aduth
Copy link
Member

aduth commented Jan 24, 2018

@aduth Could we shim this in Gutenberg for the time being?

Probably. We could either replace the api-request.js dependency wholesale, or override buildAjaxOptions to modify options before calling the original function.

apiRequest.buildAjaxOptions = flow( [
	( options ) => {
		// Fix `options.path` ...
	},
	apiRequest.buildAjaxOptions,
] );

@aduth
Copy link
Member

aduth commented Feb 5, 2018

Fix at #4877

@designsimply designsimply added the [Feature] Permalink The permalink of a post or page and the experience of setting or editing it label Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core REST API Task Task for Core REST API efforts [Feature] Permalink The permalink of a post or page and the experience of setting or editing it [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants