#65414 Refactor wp_ajax_delete_inactive_widgets(), extract wp_delete_inactiv… - #12965
#65414 Refactor wp_ajax_delete_inactive_widgets(), extract wp_delete_inactiv…#12965PANawkar wants to merge 1 commit into
Conversation
…e_widgets(), and add tests.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @pratik.nawkar@xecurify.com. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Thanks for the PR. However, since a review is already in progress for an existing PR, there is no need to submit the same PR again. |
|
Hi @t-hamano, thanks for taking a look, and apologies for the confusion - I didn't mean to duplicate the effort on #12099. I opened this to help pick up on your outstanding review comment there, and also added PHPUnit test coverage for wp_delete_inactive_widgets(), since the original PR didn't have tests yet. Since I don't have push access to @pbearne's branch, I wasn't sure how else to contribute the fix + tests, so I opened this separately - happy to be corrected if there's a better way to do this. Would you be okay reopening this so it can be reviewed, or would you prefer I add these as review comments directly on #12099 instead and let @pbearne fold them in? Either way works for me - just want to help get this one over the line. Thanks again for the thorough review! |
|
@PANawkar Thanks for your contribution!
This is appreciated, but let's avoid it as a general rule, as it can be confusing for other contributors to decide which PR to review. Unless an existing PR is very old or requires urgent attention. For now, I think it's sufficient to leave #12099 as is and just wait for a reply from @pbearne. |
Extracts the widget-removal logic from wp_ajax_delete_inactive_widgets() into a new reusable wp_delete_inactive_widgets() function in wp-admin/includes/widgets.php, per the existing refactor in this PR.
Additionally addresses the outstanding review comment: guards against a PHP 8 TypeError by checking is_array( $inactive_widgets ) before calling count(), since $sidebars_widgets['wp_inactive_widgets'] isn't guaranteed to be an array if the option data is corrupted.
Also adds PHPUnit coverage for wp_delete_inactive_widgets() in tests/phpunit/tests/widgets.php, covering: