Skip to content

Conversation

@gziolo
Copy link
Member

@gziolo gziolo commented Nov 2, 2025

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

Follow-up for #9410.

Raised by @JasonTheAdams and @felixarntz in WordPress/abilities-api#126 (comment):

I'm curious why we're using did_action() and not doing_action?

I did provide similar feedback somewhere else (I believe on the Core PR) - I think allowing registration at any arbitrary point means we're being "forgiving" to developers doing it wrong, which can then lead to far more obscure problems down the line.

I think clearly defined APIs and, given WordPress's procedural nature, clearly defined registration timing are crucial to avoid more complex problems. Better tell the developer they should fix something rather than them running into a problem later that they have no idea on why it's occurring.

Although the code modifications were simple, getting the unit tests to pass was quite an adventure. I'm still not entirely convinced I took the most elegant approach for simulating the proper conditions, but my focus was to make the tests pass with the minimal set of changes necessary.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@gziolo gziolo self-assigned this Nov 2, 2025
@gziolo gziolo requested a review from felixarntz November 2, 2025 17:34
@gziolo gziolo marked this pull request as ready for review November 2, 2025 17:38
@github-actions
Copy link

github-actions bot commented Nov 2, 2025

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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props gziolo, jorgefilipecosta, flixos90, mukesh27.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions
Copy link

github-actions bot commented Nov 2, 2025

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gziolo, looks great!

Comment on lines -21 to -30
// Fire the init hook to allow test ability category registration.
do_action( 'wp_abilities_api_categories_init' );
wp_register_ability_category(
'math',
array(
'label' => 'Math',
'description' => 'Mathematical operations and calculations.',
)
);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this is removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn’t mandatory at this level.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then remove wp_unregister_ability_category( 'math' ); from tear_down method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I’ll look into it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback applied 👍

pento pushed a commit that referenced this pull request Nov 4, 2025
Applies feedback provided that doing_action would be better check on this situation to avoid developers facing future registration timing issues.

Developed in #10452.

Props gziolo, jorgefilipecosta, flixos90, mukesh27, jason_the_adams.
See #64098.

git-svn-id: https://develop.svn.wordpress.org/trunk@61130 602fd350-edb4-49c9-b593-d223f7449a82
@jorgefilipecosta
Copy link
Member

Committed at fd41ccd.

@gziolo gziolo deleted the update/use-doing-action-abilities-api branch November 5, 2025 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants