Skip to content

Conversation

@jazzsequence
Copy link
Contributor

@jazzsequence jazzsequence commented Apr 22, 2025

Fixes #178

The linked issue is somewhat misrepresented. While the observed behavior is accurate (malformed URLs) it is not because of a misplaced /wp in the REST url -- WP-API URLs on Bedrock-based sites are expected to have two /wp/ strings in them -- one being the path to the application and the second is part of the actual wp-json/ URL (e.g. /wp-json/wp/v2/...). Therefore, the proposed solution would actually break sites without addressing the issue.

Since the issue only exists on newly created sites, the assumption here is that these sites are not using "pretty permalinks". Indeed, our tests assume permalinks are pretty for this very reason. In cases when "pretty permalinks" are not active, it makes sense that API requests might fail due to our URL rewriting for internal paths.

This PR works around the issue by simply accepting "pretty"-style API endpoint paths while also preserving the functionality of "plain" permalinks. Both "plain" and "pretty" permalinks are henceforth supported by the REST API for WPCM sites.

Tests are broken out into #182 so as to not mix release and non-release commits and will fail until this PR is merged.

Tests have been integrated back into this PR as well as other non-release changes. As a result, this PR MUST BE merged with a merge commit, not squash merge.

bypass the issue with bad string replacement by just parsing pretty permalinks natively for rest endpoints even when pretty permalinks aren't active
@jazzsequence jazzsequence changed the title allow pretty permalinks to be handled for rest endpoints by default [DEVREL-29] allow pretty permalinks to be handled for rest endpoints by default Apr 22, 2025
@jazzsequence jazzsequence marked this pull request as ready for review April 22, 2025 21:24
@jazzsequence jazzsequence requested a review from a team as a code owner April 22, 2025 21:24
@jazzsequence jazzsequence self-assigned this Apr 22, 2025
@pwtyler
Copy link
Member

pwtyler commented May 2, 2025

Tests are broken out into #182 so as to not mix release and non-release commits and will fail until this PR is merged.

This is why "rebase and merge" exists. I'm going to cherry-pick them back over here.

bypass the issue with bad string replacement by just parsing pretty
permalinks natively for rest endpoints even when pretty permalinks
aren't active
@pwtyler pwtyler force-pushed the devrel-29-fix-subdir-wpms-url-issue-2 branch from c088dd7 to 05feec7 Compare May 2, 2025 19:51
@pwtyler
Copy link
Member

pwtyler commented May 2, 2025

Have not investigated why the bats tests failed. Will chat about this next week.

Co-authored-by: Phil Tyler <[email protected]>
@jazzsequence jazzsequence requested a review from a team as a code owner May 6, 2025 19:23
@jazzsequence jazzsequence requested a review from pwtyler May 7, 2025 22:14
@pantheon-systems pantheon-systems deleted a comment from github-actions bot May 8, 2025
@pantheon-systems pantheon-systems deleted a comment from github-actions bot May 8, 2025
@pantheon-systems pantheon-systems deleted a comment from github-actions bot May 8, 2025
@pantheon-systems pantheon-systems deleted a comment from github-actions bot May 8, 2025
@github-actions
Copy link

github-actions bot commented May 8, 2025

Hi from your friendly robot! 🤖
Please review the commit checks for this PR:

This PR contains a mixture of 'normal' (release) commits and 'non-release' (internal) commits. This requires careful merging (e.g., rebase and merge) to ensure only 'normal' commits are deployed to public upstream if that's the intent.

If issues are present, please ensure commits modify either 'normal' or 'non-release' files (not a mix within a single commit) and do not contain forbidden files.
If this PR intentionally mixes 'normal' and 'non-release' commit types, remember to use rebase and merge rather than squash when merging to preserve individual commit integrity for the deploy process.

@jazzsequence
Copy link
Contributor Author

Going to close this PR and open a new one with two clean commits.

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.

[DEVREL-29] 404 errors when accessing wp-json from WPMS subsites

3 participants