Skip to content

Commit cbe8a59

Browse files
Ref34tRef34tjustlevinegziolojohnbillion
authored
Fix has_permission() return type inconsistency (Issue #67) (#76)
* Rename has_permission() to check_permission() with deprecation logic * Update tests to use check_permission() and add deprecation test * Fix remaining has_permission test usage that was causing CI failure * Remove specific deprecation test that was causing CI failures * Fix internal execute() method to use new check_permission() method * Add test coverage for deprecated has_permission method * Use WordPress test framework method for deprecated function coverage per maintainer feedback * Switch to @expectedDeprecated PHPDoc annotation for cleaner deprecation test --------- Co-authored-by: Mohamed Khaled <mohamed.khaled@9hdigital.com> Co-authored-by: Greg Ziółkowski <grzegorz@gziolo.pl> Unlinked contributors: mohamed.khaled@9hdigital.com. Co-authored-by: Ref34t <mokhaled@git.wordpress.org> Co-authored-by: justlevine <justlevine@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org> Co-authored-by: johnbillion <johnbillion@git.wordpress.org>
1 parent a33e2be commit cbe8a59

File tree

5 files changed

+86
-19
lines changed

5 files changed

+86
-19
lines changed

docs/2.getting-started.md

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,27 @@ add_action( 'admin_init', 'my_plugin_use_ability' );
133133
function my_plugin_use_ability() {
134134
$ability = wp_get_ability( 'my-plugin/get-site-title' );
135135

136-
if ( $ability && $ability->has_permission() ) {
137-
$site_title = $ability->execute();
138-
// $site_title now holds the result of get_bloginfo('name')
139-
// error_log( 'Site Title: ' . $site_title );
136+
if ( $ability ) {
137+
// Check permissions first - always use is_wp_error() to handle errors properly
138+
$permission = $ability->check_permission();
139+
if ( is_wp_error( $permission ) ) {
140+
// Handle permission error
141+
error_log( 'Permission error: ' . $permission->get_error_message() );
142+
return;
143+
} elseif ( $permission ) {
144+
// Permission granted - safe to execute
145+
$site_title = $ability->execute();
146+
if ( is_wp_error( $site_title ) ) {
147+
// Handle execution error
148+
error_log( 'Execution error: ' . $site_title->get_error_message() );
149+
} else {
150+
// $site_title now holds the result of get_bloginfo('name')
151+
// error_log( 'Site Title: ' . $site_title );
152+
}
153+
} else {
154+
// Permission denied
155+
error_log( 'Permission denied for ability execution' );
156+
}
140157
}
141158
}
142159
```

docs/4.using-abilities.md

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,14 @@ if ( $ability ) {
124124
}
125125
```
126126

127-
**Checking Permissions (`$ability->has_permission()`)**
127+
**Checking Permissions (`$ability->check_permission()`)**
128128

129-
Before executing an ability, you can check if the current user has permission:
129+
Before executing an ability, you can check if the current user has permission. The `check_permission()` method returns either `true`, `false`, or a `WP_Error` object, so you must use `is_wp_error()` to handle errors properly:
130+
131+
```php
132+
// Method signature:
133+
// check_permission( $input = null )
134+
```
130135

131136
```php
132137
$ability = wp_get_ability( 'my-plugin/update-option' );
@@ -136,12 +141,17 @@ if ( $ability ) {
136141
'option_value' => 'New Site Name',
137142
);
138143

139-
// Check permission before execution
140-
if ( $ability->has_permission( $input ) ) {
144+
// Check permission before execution - always use is_wp_error() first
145+
$permission = $ability->check_permission( $input );
146+
if ( is_wp_error( $permission ) ) {
147+
// Handle permission check error (validation, callback error, etc.)
148+
echo 'Permission check failed: ' . $permission->get_error_message();
149+
} elseif ( $permission ) {
150+
// Permission granted - safe to execute
141151
$result = $ability->execute( $input );
142152
if ( is_wp_error( $result ) ) {
143-
// Handle WP_Error
144-
echo 'Error: ' . $result->get_error_message();
153+
// Handle execution error
154+
echo 'Execution error: ' . $result->get_error_message();
145155
} else {
146156
// Use $result
147157
if ( $result['success'] ) {
@@ -150,6 +160,7 @@ if ( $ability ) {
150160
}
151161
}
152162
} else {
163+
// Permission denied
153164
echo 'You do not have permission to execute this ability.';
154165
}
155166
}

includes/abilities-api/class-wp-ability.php

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,12 +312,12 @@ protected function validate_input( $input = null ) {
312312
*
313313
* The input is validated against the input schema before it is passed to to permission callback.
314314
*
315-
* @since 0.1.0
315+
* @since N.E.X.T
316316
*
317317
* @param mixed $input Optional. The input data for permission checking. Default `null`.
318318
* @return bool|\WP_Error Whether the ability has the necessary permission.
319319
*/
320-
public function has_permission( $input = null ) {
320+
public function check_permission( $input = null ) {
321321
$is_valid = $this->validate_input( $input );
322322
if ( is_wp_error( $is_valid ) ) {
323323
return $is_valid;
@@ -330,6 +330,24 @@ public function has_permission( $input = null ) {
330330
return call_user_func( $this->permission_callback, $input );
331331
}
332332

333+
/**
334+
* Checks whether the ability has the necessary permissions (deprecated).
335+
*
336+
* The input is validated against the input schema before it is passed to to permission callback.
337+
*
338+
* @deprecated N.E.X.T Use check_permission() instead.
339+
* @see WP_Ability::check_permission()
340+
*
341+
* @since 0.1.0
342+
*
343+
* @param mixed $input Optional. The input data for permission checking. Default `null`.
344+
* @return bool|\WP_Error Whether the ability has the necessary permission.
345+
*/
346+
public function has_permission( $input = null ) {
347+
_deprecated_function( __METHOD__, 'N.E.X.T', 'WP_Ability::check_permission()' );
348+
return $this->check_permission( $input );
349+
}
350+
333351
/**
334352
* Executes the ability callback.
335353
*
@@ -394,7 +412,7 @@ protected function validate_output( $output ) {
394412
* @return mixed|\WP_Error The result of the ability execution, or WP_Error on failure.
395413
*/
396414
public function execute( $input = null ) {
397-
$has_permissions = $this->has_permission( $input );
415+
$has_permissions = $this->check_permission( $input );
398416
if ( true !== $has_permissions ) {
399417
if ( is_wp_error( $has_permissions ) ) {
400418
if ( 'ability_invalid_input' === $has_permissions->get_error_code() ) {

includes/rest-api/endpoints/class-wp-rest-abilities-run-controller.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ public function run_ability_permissions_check( $request ) {
163163
}
164164

165165
$input = $this->get_input_from_request( $request );
166-
if ( ! $ability->has_permission( $input ) ) {
166+
if ( ! $ability->check_permission( $input ) ) {
167167
return new \WP_Error(
168168
'rest_ability_cannot_execute',
169169
__( 'Sorry, you are not allowed to execute this ability.' ),

tests/unit/abilities-api/wpRegisterAbility.php

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public function test_register_valid_ability(): void {
134134
$this->assertSame( self::$test_ability_args['output_schema'], $result->get_output_schema() );
135135
$this->assertSame( self::$test_ability_args['meta'], $result->get_meta() );
136136
$this->assertTrue(
137-
$result->has_permission(
137+
$result->check_permission(
138138
array(
139139
'a' => 2,
140140
'b' => 3,
@@ -164,7 +164,7 @@ public function test_register_ability_no_permissions(): void {
164164
$result = wp_register_ability( self::$test_ability_name, self::$test_ability_args );
165165

166166
$this->assertFalse(
167-
$result->has_permission(
167+
$result->check_permission(
168168
array(
169169
'a' => 2,
170170
'b' => 3,
@@ -289,7 +289,7 @@ public function test_permission_callback_no_input_schema_match(): void {
289289

290290
$result = wp_register_ability( self::$test_ability_name, self::$test_ability_args );
291291

292-
$actual = $result->has_permission(
292+
$actual = $result->check_permission(
293293
array(
294294
'a' => 2,
295295
'b' => 3,
@@ -308,6 +308,27 @@ public function test_permission_callback_no_input_schema_match(): void {
308308
);
309309
}
310310

311+
/**
312+
* Tests that deprecated has_permission() method still works.
313+
*
314+
* @expectedDeprecated WP_Ability::has_permission
315+
*/
316+
public function test_has_permission_deprecated_coverage(): void {
317+
do_action( 'abilities_api_init' );
318+
319+
$result = wp_register_ability( self::$test_ability_name, self::$test_ability_args );
320+
321+
// Test that deprecated method still works
322+
$this->assertTrue(
323+
$result->has_permission(
324+
array(
325+
'a' => 2,
326+
'b' => 3,
327+
)
328+
)
329+
);
330+
}
331+
311332
/**
312333
* Tests permission callback receiving input for contextual permission checks.
313334
*/
@@ -325,7 +346,7 @@ public function test_permission_callback_receives_input(): void {
325346

326347
// Test with a > b (should be allowed)
327348
$this->assertTrue(
328-
$result->has_permission(
349+
$result->check_permission(
329350
array(
330351
'a' => 5,
331352
'b' => 3,
@@ -342,7 +363,7 @@ public function test_permission_callback_receives_input(): void {
342363

343364
// Test with a < b (should be denied)
344365
$this->assertFalse(
345-
$result->has_permission(
366+
$result->check_permission(
346367
array(
347368
'a' => 2,
348369
'b' => 8,

0 commit comments

Comments
 (0)