-
Notifications
You must be signed in to change notification settings - Fork 49
dev: add register_ability_args filter [Proposal]
#74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dev: add register_ability_args filter [Proposal]
#74
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces the wp_ability_args filter to the Abilities API, allowing developers to modify ability arguments before validation and instantiation. This provides a centralized mechanism for extending and customizing ability behavior.
- Adds a filter hook in the registry's register method to modify ability arguments
- Includes comprehensive documentation with usage examples
- Uses wp_ prefix to avoid naming collisions before core integration
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| includes/abilities-api/class-wp-abilities-registry.php | Adds the wp_ability_args filter in the register method before validation |
| docs/5.hooks.md | Documents the new filter with detailed usage examples and parameters |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #74 +/- ##
============================================
+ Coverage 84.84% 85.64% +0.79%
Complexity 102 102
============================================
Files 16 16
Lines 772 773 +1
Branches 86 86
============================================
+ Hits 655 662 +7
+ Misses 117 111 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <[email protected]>
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
gziolo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some minor feedback. Overall, I'm very much in favor of the proposed extensibility option.
|
We will also need some basic unit test coverage to illustrate the expected behavior better and prevent future regressions. I plan to add some tests today to: We can coordinate how to ensure we avoid more complex merge conflicts. |
For your information. Tests for hooks live in https://github.com/WordPress/abilities-api/blob/trunk/tests/unit/abilities-api/wpAbility.php. Docs for hooks live in https://github.com/WordPress/abilities-api/blob/trunk/docs/6.hooks.md. |
wp_ability_args filter [Proposal]register_ability_args filter [Proposal]
|
@galatanovidiu, this proposed filter |
Co-authored-by: Greg Ziółkowski <[email protected]>
|
@gziolo feedback addressed, tests added, and merged the action/filter docs (structure too, not just pasting the two together). PTAL and let me know what you think 🙏 |
gziolo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition to the documentation and test coverage. Thank you for addressing my feedback.
I left some small notes to address before merging changes 🚀
Co-authored-by: Greg Ziółkowski <[email protected]>
What
This PR exposes the
register_ability_argsfilter as a way to centrally extend and change the behavior of registered abilities.Why
Part of Proposal: Provide extensibility for individual abilities #39
This is a counterproposal to Propose filters to use with the registered ability #37 :
WP_Abilities_Registry::register()makes behavior robust againstWP_Abilityclass overloads and keeps the lifecycle + validation intact. In other words: filtering abilities remains idempotent to their non-filtered versions.How
changed towp_ability_argsis used before the merge to core, but it should be changed toabilities_apias part of 6.9 IMO.register_ability_args