Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#53317 closed defect (bug) (fixed)

Mark site as no longer fresh in new Widgets screen

Reported by: kevin940726's profile kevin940726 Owned by: noisysocks's profile noisysocks
Milestone: 5.8 Priority: normal
Severity: normal Version: 5.8
Component: Widgets Keywords: has-patch commit
Focuses: Cc:

Description

Originally reported in https://github.com/WordPress/gutenberg/issues/26241

We need to call _delete_option_fresh_site whenever we're saving widgets in the new widgets screen, so that the starter content can be correctly deleted after editing any widgets.

Change History (11)

This ticket was mentioned in PR #1328 on WordPress/wordpress-develop by kevin940726.


2 years ago
#1

  • Keywords has-patch added

Originally reported in https://github.com/WordPress/gutenberg/issues/26241.

The idea is to trigger the _delete_option_fresh_site function when saved. We can reuse the wp_ajax_save-widget action from the old widgets screen, but it could be confusing.

Another idea would be to add new actions to the default filters list and call them in each rest endpoint.

https://github.com/WordPress/wordpress-develop/blob/910c2db68aef58ba14353e8c771dc3cfcd5ee9b5/src/wp-includes/default-filters.php#L245-L248

I'm not sure about the naming of those actions yet though, and open to any suggestions.

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

draganescu commented on PR #1328:


2 years ago
#2

So these ajax actions are registered and called in the admin-ajax page. I am unsure if, while straightforward and not wrong, this solution is good (calling an action as wp_ajax even if it's not really what that means). I believe the second approach explained in the PR's description is better.

noisysocks commented on PR #1328:


2 years ago
#3

Yes, we should add new actions.

Most of the REST API endpoints have actions which let third parties hook into them for extensibility. We forgot to do this for the widget endpoints, though.

For example, in the /posts endpoint, we have the rest_insert_post action which fires after a post is updated or created:

https://github.com/WordPress/wordpress-develop/blob/e37b85fcd7b49846679caef7701c89fcd5c22f0f/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L671

And we have rest_delete_post which fires when a post is deleted:

https://github.com/WordPress/wordpress-develop/blob/e37b85fcd7b49846679caef7701c89fcd5c22f0f/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L1033

Let's add these new actions to the REST API:

  • rest_save_widget and rest_after_save_widget in WP_REST_Widgets_Controller::save_widget().
  • rest_delete_widget in WP_REST_Widgets_Controller::delete_item().
  • rest_save_sidebar and rest_after_save_sidebar in WP_REST_Sidebars_Controller::update_item().

Then let's add save_widget and rest_save_sidebar to the array in default-filters.php that invokes _delete_option_fresh_site.

#4 @noisysocks
2 years ago

  • Milestone changed from Awaiting Review to 5.8
  • Owner set to noisysocks
  • Status changed from new to reviewing
  • Version set to trunk

noisysocks commented on PR #1328:


2 years ago
#5

Thanks @kevin940726!

I reset my database, installed WordPress, and verified that wp option get fresh_site changes from 1 to 0 after adding a new block to the widgets block editor ✅

I then added this to wp-content/mu-plugins/test.php:

{{{php
<?php
add_action(

'rest_save_sidebar',
function( $sidebar, $request ) {

var_dump( $sidebar, $request );

},
10,
2

);
add_action(

'rest_delete_widget',
function( $widget_id, $sidebar_id, $response, $request ) {

var_dump( $widget_id, $sidebar_id, $response, $request );

},
10,
4

);
add_action(

'rest_after_save_widget',
function( $id, $sidebar_id, $request, $creating ) {

var_dump( $id, $sidebar_id, $request, $creating );

},
10,
4

);
}}}

And ran:

http -a admin:password PUT http://wp-git-build.test/wp-json/wp/v2/widgets/meta-2 instance:='{"raw":{"title":"hello"}}'
http -a admin:password POST http://wp-git-build.test/wp-json/wp/v2/widgets sidebar==sidebar-1 id_base=meta instance:='{"raw":{"title":"hello"}}'
http -a admin:password DELETE http://wp-git-build.test/wp-json/wp/v2/widgets/meta-2

Everything was dumping correctly ✅

So, :shipit: looks good to me!

#6 @noisysocks
2 years ago

  • Keywords commit added

#7 @noisysocks
2 years ago

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

In 51068:

REST API: Delete fresh_site option when updating widgets via REST API

Adds new hooks (rest_save_sidebar, rest_delete_widget, rest_after_save_widget)
to the widgets REST API and uses them to delete the fresh_site option when
updating widgets via the REST API. This ensures that starter content isn't
loaded in the Customizer after a user makes changes.

Fixes #53317.
Props kevin940726, garrett-eclipse, andraganescu, hellofromtonya.

#8 @SergeyBiryukov
2 years ago

In 51071:

REST API: Rename the $creating parameter of rest_after_save_widget action to $update.

This brings some consistency with similar actions for posts, e.g. save_post or wp_insert_post.

Follow-up to [51068], [51069].

See #53317.

#9 follow-up: @TimothyBlynJacobs
2 years ago

@SergeyBiryukov r51071 should be reverted IMO. The REST API uses creating consistently.

#10 in reply to: ↑ 9 @SergeyBiryukov
2 years ago

Replying to TimothyBlynJacobs:

r51071 should be reverted IMO. The REST API uses creating consistently.

Ah, yes, indeed. I did a search for the $creating variable in do_action() calls, but missed the usage in DocBlocks, which is where all the other instances were.

Thanks for pointing that out! Will revert shortly.

#11 @SergeyBiryukov
2 years ago

In 51074:

REST API: Restore the $creating parameter of rest_after_save_widget action.

This is consistent with other similar REST API actions.

Partially reverts [51071], except for DocBlock formatting fixes.

Props TimothyBlynJacobs.
See #53317.

Note: See TracTickets for help on using tickets.