Make WordPress Core

Opened 15 months ago

Closed 9 months ago

Last modified 8 months ago

#59810 closed enhancement (duplicate)

Global styles revisions: add route for single styles revisions

Reported by: ramonopoly's profile ramonopoly Owned by: youknowriad's profile youknowriad
Milestone: Priority: normal
Severity: normal Version: 6.4
Component: REST API Keywords: has-patch has-unit-tests gutenberg-merge
Focuses: rest-api Cc:

Description

The Global Styles Revision Controller is a bespoke REST API endpoint that returns revisions for global styles in the Site Editor.

Currently it only has a route to retrieve a collection of revisions get_items.

To make it consistent with other post types, we should add the single revision route, e.g., /wp/v2/global-styles/${ parentId }/revisions/${ revisionsId }

A PR to add this to Gutenberg is here: https://github.com/WordPress/gutenberg/pull/55827

Change History (25)

This ticket was mentioned in PR #5622 on WordPress/wordpress-develop by @ramonopoly.


15 months ago
#1

## What?
Syncing Gutenberg PR:

Adding route for single global styles revisions: /wp/v2/global-styles/${ parentId }/revisions/${ revisionsId }

## Why?

To make global styles revisions compatible with the proposed Core Data revisions API, that is, so that we can fetch single revisions via getRevision().

Trac ticket: https://core.trac.wordpress.org/ticket/59810

@youknowriad commented on PR #5622:


13 months ago
#2

@ramonjd Is it possible to add a unit test for this?

@get_dave commented on PR #5622:


13 months ago
#3

@ramonjd Is it possible to add a unit test for this?

I see unit tests in this PR.

npm run test:php -- --filter WP_REST_Global_Styles_Revisions_Controller_Test

OK (28 tests, 107 assertions)

What am I missing?

@youknowriad commented on PR #5622:


13 months ago
#4

What am I missing?

That my brain is confused with too many tabs maybe :P

You're right there are unit tests.

@ramonopoly commented on PR #5622:


13 months ago
#5

Thanks for double checking, folks!

#6 @youknowriad
13 months ago

  • Owner set to youknowriad
  • Resolution set to fixed
  • Status changed from new to closed

In 57494:

REST API: Add route for single styles revisions.

Adds a route for single global styles revisions: /wp/v2/global-styles/${ parentId }/revisions/${ revisionsId }
This fixes the getRevision actions in the core-data package.

Props ramonopoly, get_dave.
Fixes #59810.

#8 @spacedmonkey
13 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@youknowriad @ramonopoly The fixes applied in #59875 / [57222] needs to applied in this new endpoint.

#9 @ramonopoly
13 months ago

@youknowriad @ramonopoly The fixes applied in #59875 / [57222] needs to applied in this new endpoint.

I'll look into that, thanks for the ping.

I was hoping that the PR Global styles revisions controller: extend with WP_REST_Revisions_Controller #5699 would have taken care of this.

Is the any reason why we couldn't get that patch in, and work on the refactor of the Global Styles endpoint (mentioned in this comment) in a direct follow up?

#10 @ramonopoly
13 months ago

I was hoping that the PR Global styles revisions controller: extend with WP_REST_Revisions_Controller #5699 would have taken care of this.

Confirmed that it will.

#11 @stevenlinx
12 months ago

  • Keywords add-to-field-guide added

#12 @swissspidy
12 months ago

@youknowriad Can you update the status here?

#13 @ramonopoly
12 months ago

Can you update the status here?

If https://github.com/WordPress/wordpress-develop/pull/6105 is committed in a subsequent round of WP 6.5 Beta, then we can close this ticket.

As I understand it, the outstanding issue is as follows:

As mentioned in the comment above, there was a fix to the get_item method of WP_REST_Revisions_Controller to prevent returning a revision whose parent ID property does not match the parent ID.

Here's the relevant Github PR:

https://github.com/WordPress/wordpress-develop/pull/5655/files#diff-17ea2a95dc40f1f9804ed19a22d88a110f88920ca49e503860d00e5f34aef0b2R395

This fix has not yet been ported to WP_REST_Global_Styles_Revisions_Controller

https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-revisions-controller.php#L279

I will prepare a follow up PR to do this in the event that https://github.com/WordPress/wordpress-develop/pull/6105 is not committed for WordPress 6.5.

Thank you!

This ticket was mentioned in PR #6108 on WordPress/wordpress-develop by @ramonopoly.


12 months ago
#14

Brings global styles revisions controller WP_REST_Global_Styles_Revisions_Controller in line with the changes from:

This change is required as WP_REST_Global_Styles_Revisions_Controller doesn't inherit WP_REST_Revisions_Controller.

https://github.com/WordPress/wordpress-develop/pull/6105

[!IMPORTANT]
This PR is not required if https://github.com/WordPress/wordpress-develop/pull/6105 makes it to WordPress 6.5 first.

npm run test:php -- --filter WP_REST_Global_Styles_Revisions_Controller_Test

#18 @swissspidy
12 months ago

  • Keywords add-to-field-guide removed
  • Milestone changed from 6.5 to 6.6

#19 @oglekler
9 months ago

@ramonopoly and @swissspidy, Beta 1 is in 2.5 weeks.. What needs to be done to move this forward?

#20 follow-up: @ramonopoly
9 months ago

@olgekler

Thanks for the ping.

I think the comment in https://core.trac.wordpress.org/ticket/59810#comment:13 is still valid.

Trac ticket #60131 has a patch that would make this ticket redundant.

So if we can get the corresponding patch committed, then we can close this ticket.

I'll give that PR a re-test as soon as I'm back at my computer. I think it's still good.

I'll also comment over on #60131.

Thank you!

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


9 months ago

#22 in reply to: ↑ 20 @hellofromTonya
9 months ago

Replying to ramonopoly:

Trac ticket #60131 has a patch that would make this ticket redundant.

So if we can get the corresponding patch committed, then we can close this ticket.

With commit [58225] in #60131, should this ticket be closed @ramonopoly @swissspidy?

#23 @swissspidy
9 months ago

  • Component changed from General to REST API
  • Milestone 6.6 deleted
  • Resolution set to duplicate
  • Status changed from reopened to closed

Duplicate of #60131.

Looks like it

#24 @ramonopoly
9 months ago

Just to confirm that this ticket is good to close now that https://core.trac.wordpress.org/ticket/60131 is resolved. Thanks, again.

Note: See TracTickets for help on using tickets.