WordPress.org

Make WordPress Core

Opened 12 days ago

Closed 10 days ago

Last modified 9 days ago

#53317 closed defect (bug) (fixed)

Mark site as no longer fresh in new Widgets screen

Reported by: kevin940726 Owned by: noisysocks
Milestone: 5.8 Priority: normal
Severity: normal Version: trunk
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.


11 days ago

  • 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

#2 @prbot
11 days ago

draganescu commented on PR #1328:

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.

#3 @prbot
11 days ago

noisysocks commented on PR #1328:

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
11 days ago

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

#5 @prbot
11 days ago

noisysocks commented on PR #1328:

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
11 days ago

  • Keywords commit added

#7 @noisysocks
10 days 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
9 days 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
9 days ago

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

#10 in reply to: ↑ 9 @SergeyBiryukov
9 days 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
9 days 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.