Skip to content

Conversation

@JasonTheAdams
Copy link
Member

This introduces the ability for formally deprecate an Ability, which will be an important system for allowing ability iteration without resulting in multiple discoverable abilities of the same kind. It tackles this on a few fronts:

  1. Adds a _deprecated_ability() method
  2. Adds a meta.deprecated property to WP_Ability
  3. Calls _deprecated_ability in WP_Ability::do_execute() if the ability is deprecated

To do:

  • Hide deprecated abilities from wp_get_abilities()
  • Add ability to fetch deprecated abilities from wp_get_abilities()
  • Add parameter to Abilities endpoint for filtering by deprecated state

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

@github-actions
Copy link

Hi @JasonTheAdams! 👋

Thank you for your contribution to WordPress! 💖

It looks like this is your first pull request to wordpress-develop. Here are a few things to be aware of that may help you out!

No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description.

Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making.

More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook.

Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook.

If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook.

The Developer Hub also documents the various coding standards that are followed:

Thank you,
The WordPress Project

@JasonTheAdams
Copy link
Member Author

Some topics to discuss:

  1. Should we add a way for an Ability to mark which version it was deprecated in?
  2. Should we add a way for an Ability to specify which Ability should be used to replace it?
  3. How do we want to handle filtering in wp_get_abiliites()?

@github-actions
Copy link

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.

@JasonTheAdams
Copy link
Member Author

Version

When looking over _deprecated_function() I saw that it has a $version parameter. I'm honestly not sure how useful that is here, especially since most abilities won't be introduced by WordPress.

If it is useful, then we'd need a way for an Ability to note the version, since this function is called automatically within do_execute(). I threw in a meta.deprecated_version in there for now, but I could also see allowing:

meta.deprecated = false;

meta.deprecated = true;

meta.deprecated = array(
	'version' => '1.2.3',
	'replacement' => 'foo/v2/my-ability'
); // implied true with further information

I'm kind of partial to the array structure as it only adds a single property to meta and is nice and clear.

Replacement

This does feel more valuable, as it would be nice for someone using an Ability to know which Ability is intended to be its successor, if there is one available. As above, we could have a meta.deprecated_replacement type of property, or else include it in the array structure.

Filtering in wp_get_abilities()

We purposely put off filtering in a previous bit of work because we realized it shouldn't act like WP_Query or something like that since this is all in memory. We put it off as we didn't see an immediate need to provide filtering since the person could just do array_filter( wp_get_abilities(), function() { ... } to get what they want.

That argument still applies here, but if the intent is to have deprecated abilities hidden by default, then that method falls apart. So we either have to introduce filtering or else not include it and have it such that folks filter it out themselves if that's their intent. The REST endpoint would work fine this way. The issue with this is that if we introduce filtering later and want deprecated abilities filtered by default, then we're changing the function's behavior.

We could just introduce a wp_get_abilities( array $args ) which is array( 'deprecated' => false ' ) by default, so passing wp_get_abilities( array() ) would get everything. At that point we can think about further filters later. But I could even be talked into a plain $include_deprecated = false boolean parameter, as I still think that most filtering is better done with something like array_filter().

Open to thoughts!

@justlevine
Copy link

Hey @JasonTheAdams I touched a bit on your more general comments over on https://core.trac.wordpress.org/ticket/64209#comment:13, but tl;dr I agree with your comparison to ability filtering, and think any deprecation pattern should also be done as a holistic enhancement once the needs are more clearly defined. Which yeah will prob only come after/alongside a basic mechanism for filtering (but IMO no need to tunnel vision that in the rush to ship this)

(Syndication is one directional to Trac, replying here to signpost future visitors.)

PS: congrats on your "first contribution to WordPress" 🎉🙃🎉

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.

2 participants