-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Iterate on widget REST API endpoints #29649
Conversation
Size Change: +4.09 kB (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
These changes are looking great to me. Will review more carefully after the tests are updated. |
This is going great! 👏 |
I've fixed all test failures and as such will mark this as ready to review. I'm still working on adding new tests for the new functionality. |
OK, I added a few tests which cover the new functionality. I couldn't figure out how to write a test for It would be nice to DRY up the unit tests. Probably we can use data providers to test all of the combinations of methods ( |
Checks are failing because of #29752. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please provide more detail on this change? Most of the changes seem to make this less RESTFUL and remove data from the api. I don't see what problem you are trying to solve here. What exactly was wrong with the apis in the first place?
I added a little more justification to each of the dot points underneath "Changes" in this PR's description. Let me know if you'd like to elaborate on anything in particular there. |
Spoke to @TimothyBJacobs about making this more RESTful. Let's:
|
Makes the /wp/v2/widget and /wp/v2/widget-types endpoints: - Work with widget instance attributes that cannot be serialized to JSON - Less reliant on widget implementation details - Work better with widgets that don't extend WP_Widget
6737d4b
to
1fa6816
Compare
We can be reasonably confident that this doesn't break any existing functionality on the frontend because there are E2E tests in adding-widgets.test.js that cover displaying widgets, saving widgets, etc. |
I think it would be helpful to call out what is being deprecated in the Gutenberg plugin release notes, so I've added Here's what can be included in the release notes:
|
This is ready for re-review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also have one question, but I think this is looking good.
Thanks for reviewing! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good
Thank you @TimothyBJacobs and @spacedmonkey! |
Summary
Makes the
/wp/v2/widget
and/wp/v2/widget-types
endpoints:WP_Widget
.All changes are backwards compatible so that changes to the frontend can be made in a separate PR.
Changes
GET /wp/v2/widgets
&GET /wp/v2/widgets/:id
settings
in favour ofinstance
.instance
supports attributes that cannot be serialised to JSON viainstance.encoded
andinstance.hash
.Why: Not all widgets have settings that can be represented as JSON. See Legacy Widget add support for widgets with attributes that can't be serialised into JSON #28902.
instance
supports opt-in raw representation of attributes that can be serialised to JSON viainstance.raw
.Why: If we know that a widget does have settings that can be represented as JSON, we should expose that so that the block editor can perform e.g. transformations.
instance
is set tonull
when widget does not extendWP_Widget
.Why: If a widget does not extend
WP_Widget
, we cannot tell ahead of time what settings it has. All that we can say is that it has a HTML form and can accept form data from that HTML form.rendered
andrendered_form
to work for widgets that don't extendWP_Widget
by using lower level APIs.Why: If a widget does not extend
WP_Widget
, we need to e.g. invoke the callback in$wp_registered_widgets
to display it instead of callingWP_Widget::display()
.widget_class
.Why: Widgets that do not extend
WP_Widget
do not have a class. Also, clients should not need to know about PHP class names.number
.Why: Widgets that do not extend
WP_Widget
do not have a number. Also,id
can already be used to uniquely identify the widget.name
anddescription
.Why: These are redundant with fields that are in
/wp/v2/widget-types
.widgets-api.php
.Why: This allows for less code duplication when merged to Core.
POST /wp/v2/widgets
&PUT /wp/v2/widgets/:id
settings
in favour ofinstance
.instance
supports attributes that cannot be serialised to JSON viainstance.encoded
andinstance.hash
.Why: Not all widgets have settings that can be represented as JSON. See Legacy Widget add support for widgets with attributes that can't be serialised into JSON #28902.
instance
supports opt-in raw representation of attributes that can be serialised to JSON viainstance.raw
.Why: If we know that a widget does have settings that can be represented as JSON, we should expose that so that the block editor can perform e.g. transformations.
instance
cannot be set when the widget does not extendWP_Widget
.Why: If a widget does not extend
WP_Widget
, we cannot tell ahead of time what settings it has. All that we can say is that it has a HTML form and can accept form data from that HTML form.form_data
.Why: This lets clients create an update widgets that do not extend
WP_Widget
by taking the form fromrendered_form
and submitting it to the endpoint.$wp_registered_widget_updates
API.Why: Not all widgets extend
WP_Widget
and so we cannot useWP_Widget::update
.widgets-api.php
.Why: This allows for less code duplication when merged to Core.
GET /wp/v2/widget-types
&GET /wp/v2/widget-types/:id
option_name
.Why: This is an implementation detail and cannot be known ahead of time when the widget does not extend
WP_Widget
.widget_class
.Why: Widgets that do not extend
WP_Widget
do not have a class. Also, clients should not need to know about PHP class names.customize_selective_refresh
.Why: This data is only ever needed by the Customizer.
$wp_registered_widgets
API.Why: Not all widgets extend
WP_Widget
and so we cannot useWP_Widget::id_base
.POST /wp/v2/widget-types/:id/form-renderer
/wp/v2/widget-types/:id/encode
.Why: The primary purpose of this endpoint now is to transform widget form data into an encoded instance object. This is how the Legacy Widget block will interact with the REST API. See Legacy Widget add support for widgets with attributes that can't be serialised into JSON #28902 (comment).
Examples
View examples
Using https://httpie.io. I've manually removed headers,
_links
, and deprecated properties for brevity.Viewing a widget that extends
WP_Widget
Viewing a widget that supports raw instance attributes
Viewing a widget that does not extend
WP_Widget
Updating a widget that extends
WP_Widget
Updating a widget that supports raw instance attributes
Updating a widget that does not extend
WP_Widget
Transforming form data into an encoded instance