Abilities API: when using `ability_class`, the `execute_callback` should not be mandatory

[artpi]

Abilities API allows for extending WP_Ability by providing ability_class during the ability registration. This is meant to unlock complex abilities holding some sort of state or logic that requires multiple helper methods.
In all of those scenarios you would ovewrite execute or do_execute method.

However, because the check for execute_callback is in constructor, then in order to register an ability with ability_class overwrite, you have to BOTH:

• provide do_execute
• Provide a dummy execute_callback

Like so:

<?php

class My_Test_Ability extends WP_Ability {

    protected function do_execute( $input = null ) {
                // Here is some complex logic.
                echo "SUCCESS!!!";
        return array( 'success' => true );
    }
}

add_action( 'wp_abilities_api_init', function() {
    wp_register_ability(
        'test/custom-class-ability',
        array(
            'label'         => 'Test Custom Ability',
            'description'   => 'Test ability with custom class.',
            'category'      => 'site',
            'ability_class' => My_Test_Ability::class,
                        'execute_callback' => function() {
                                // We have to provide an execute callback that will never be called because of checks in wp_ability.
                                return null;
                        },
                        'permission_callback' => function() {
                                return true;
                        },
        )
    );
} );

wp_get_ability('test/custom-class-ability')->execute();

When using ability_class, this dummy callback should not be necessary.

• It is a horrible developer experience because you would assume its not necessary,your ability is not registering and you wonder why -the only trace is doing_it_wrong back where you register the ability, not the place where your ability fails to execute
• It is assuming and mandating an implementation detail (callback) for a class that is most likely using an overwrite
• If the callback is also required and missing, the execute will fail anyway because of the check in do_execute.
• If, by any chance, developer removes the do_execute or execute overwrite, the code path of the dummy execute_callback is now active which may lead to unexpected bugs

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

Abilities API allows for extending WP_Ability by providing ability_class during the ability registration. This is meant to unlock complex abilities holding some sort of state or logic that requires multiple helper methods.
In all of those scenarios you would ovewrite execute or do_execute method.

However, because the check for execute_callback is in constructor, then in order to register an ability with ability_class overwrite, you have to BOTH:

• provide do_execute
• Provide a dummy execute_callback

This PR isolates the execute_callback check only to the base WP_Ability class.

## Testing instructions

Save this as test.php

<?php

class My_Test_Ability extends WP_Ability {

    protected function do_execute( $input = null ) {
                // Here is some complex logic.
                echo "SUCCESS!!!";
        return array( 'success' => true );
    }
}

add_action( 'wp_abilities_api_init', function() {
    wp_register_ability(
        'test/custom-class-ability',
        array(
            'label'         => 'Test Custom Ability',
            'description'   => 'Test ability with custom class.',
            'category'      => 'site',
            'ability_class' => My_Test_Ability::class,
            'permission_callback' => function() {
                    return true;
            },
        )
    );
} );

wp_get_ability('test/custom-class-ability')->execute();

and then run with CLI:

npm run env:cli -- eval-file test.php

How is ongoing development handled for this API? There's still active dev in the plugin repo here: ​https://github.com/WordPress/abilities-api

Is dev handled in the plugin and then ported to core?

How is ongoing development handled for this API? There's still active dev in the plugin repo here: ​https://github.com/WordPress/abilities-api

Is dev handled in the plugin and then ported to core?

Right now given that abilities are in core, the source of true is core, and we should do our changes in core. Probably ​https://github.com/WordPress/abilities-api will be archieved soon.

Left a comment related to the permissions callback, but other than that it looks good.

Done
​https://github.com/WordPress/wordpress-develop/pull/10622/commits/5f091d012fce00490b357e89ab330b1b2e74d161

[jorgefilipecosta]

In 61390:

Abilities API: Enhance WP_Ability validation for execute_callback and permission_callback.

Abilities API allows for extending WP_Ability by providing ability_class during the ability registration. This is meant to unlock complex abilities holding some sort of state or logic that requires multiple helper methods.
In all of those scenarios you would ovewrite execute or do_execute method.
However, because the check for execute_callback is in constructor, then in order to register an ability with ability_class overwrite, you have to BOTH: provide do_execute and provide a dummy execute_callback. The same need happens for permission_callback.
This commit fixes the issue execute_callback and permission_callback are now optional when a class is provided.

Props artpi, swissspidy, jorgefilipecosta, mindctrl.
Fixes #64407.

Committed at ​https://github.com/WordPress/wordpress-develop/commit/5df78618d61c91194c40223cfc07f2eb6d01ac96.