Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#55617 closed defect (bug) (fixed)

/wp/v2/pattern-directory/patterns endpoint: slug parameter has no effect on the response

Reported by: antonvlasenko's profile antonvlasenko Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.0 Priority: normal
Severity: major Version: 6.0
Component: REST API Keywords: has-patch has-unit-tests commit
Focuses: rest-api Cc:

Description

Steps to reproduce the bug:

  1. Install this plugin to allow basic authorization for REST API requests.
  2. Activate the plugin.
  3. Send a GET request to your WordPress instance, e.g.:
curl --user username:user_password "http://your.wordpress.instance/wp-json/wp/v2/pattern-directory/patterns?slug%5B0%5D=visual-navigation-with-rainbow-gradient"

Replace

username:user_password

with the actual credentials.
Replace

http://your.wordpress.instance

with the actual URL of your instance.

  1. Note the response. It should contain at least 1 pattern.
  2. Send a GET request again, but this time replace
    visual-navigation-with-rainbow-gradient
    

with some non-existent pattern. E.g.:

curl --user username:user_password "http://your.wordpress.instance/wp-json/wp/v2/pattern-directory/patterns?slug%5B0%5D=non-existent-pattern"

Expected result:
The endpoint should return an empty response.
Actual result:
The endpoint will return the previous response. slug parameter has no effect on the response.

Change History (18)

This ticket was mentioned in PR #2625 on WordPress/wordpress-develop by anton-vlasenko.


2 years ago
#1

  • Keywords has-patch has-unit-tests added

#2 follow-up: @spacedmonkey
2 years ago

  • Keywords has-patch has-unit-tests removed

I haven't looked to deeply into this, but I noticed that slug was query params in get_collection_params. We could fix that as part of this solution.

#3 @spacedmonkey
2 years ago

  • Milestone changed from Awaiting Review to 6.0

#4 follow-up: @spacedmonkey
2 years ago

  • Focuses rest-api added
  • Keywords needs-patch needs-unit-tests added

#5 in reply to: ↑ 2 @antonvlasenko
2 years ago

Replying to spacedmonkey:

I haven't looked to deeply into this, but I noticed that slug was query params in get_collection_params. We could fix that as part of this solution.

I agree, it would be nice to fix it.
Fixed it in https://github.com/WordPress/wordpress-develop/pull/2625/commits/b1598ab4191ac31cb8f3e019163f7c384a22ad5a

This ticket was mentioned in Slack in #core-restapi by antonvlasenko. View the logs.


2 years ago

#7 in reply to: ↑ 4 @antonvlasenko
2 years ago

I'm not sure why needs-patch and needs-unit-tests added tags were added, because there is a patch and unit tests.
I've fixed all the feedback and failed unit tests.

Last edited 2 years ago by antonvlasenko (previous) (diff)

This ticket was mentioned in Slack in #core by chaion07. View the logs.


2 years ago

#9 @hellofromTonya
2 years ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

Updating the keywords.

#10 @antonvlasenko
2 years ago

I've marked this bug as major because WordPress sends a request to the wp/v2/pattern-directory/patterns endpoint when a user navigates the Site Editor page (/wp-admin/site-editor.php).
It may happen that the list of available patterns supported by the current theme will be incorrect because the wrong response from the wp/v2/pattern-directory/patterns endpoint will get cached (for 1 hour).
I haven't been able to spot any issues on the front end, but the bug is reproducible and can have unpredictable consequences.
This bug was intoruced in the recent changeset.
In my opinion, it should be fixed ASAP.
FYI: @spacedmonkey

Last edited 2 years ago by antonvlasenko (previous) (diff)

anton-vlasenko commented on PR #2625:


2 years ago
#11

There are a number of changes required here.

Thank you for the review, @spacedmonkey!
I've replied to all code review comments.
I'd appreciate the second round of the review.

ndiego commented on PR #2625:


2 years ago
#12

@anton-vlasenko I working on cleaning up the 6.0 Project Board. Are we waiting on anything else for this PR?

anton-vlasenko commented on PR #2625:


2 years ago
#13

@anton-vlasenko I'm working on cleaning up the 6.0 Project Board. Are we waiting on anything else for this PR?

@ndiego
From a technical point of view, this PR is ready to be merged.
2 core commiters have approved it.

I don't know if there are any problems from an organizational point of view.
I'd be grateful if you could please merge it (I don't know if you have the permission to merge to Beta 4).
Thank you.

cc @peterwilsoncc

#14 @hellofromTonya
2 years ago

  • Owner changed from antonvlasenko to hellofromTonya
  • Status changed from assigned to reviewing

As this bug was introduced during the 6.0 beta cycle in [53218] and it has approval, I'll do the review, test, and if ready, commit before Beta 4 release cycle starts.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


2 years ago

#16 @hellofromTonya
2 years ago

  • Keywords commit added

Prepping for commit.

#17 @hellofromTonya
2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 53333:

REST API: Fixes /wp/v2/pattern-directory/patterns endpoint response for slug parameter.

[53218] introduced a bug of a wrong response from the wp/v2/pattern-directory/patterns endpoint with a slug parameter. As the response is cached, it can result in an incorrect list of available patterns supported by the current theme.

This commit resolves by:

  • Limiting the slug to an array in the query parameters.
  • When set, parsing and sorting the slug(s) and then serializing the sorted query args as part of the hashed transient keys.

Props antonvlasenko, timothyblynjacobs, spacedmonkey, costdev, hellofromTonya.

Follow-up to [53218], [53152], [51208].
Fixes #55617.

Note: See TracTickets for help on using tickets.