Skip to content
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

API: create REST endpoint for 'features/available' #39442

Merged
merged 7 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
Expand Up @@ -823,24 +823,6 @@ public function disconnect_blog() {
return false;
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

I removed this long deprecated method which was calling the now deprecated method Jetpack_XMLRPC_Methods::features_available.

* Deprecated: This method is no longer part of the Connection package and now lives on the Jetpack plugin.
*
* Returns what features are available. Uses the slug of the module files.
*
* @deprecated since 1.25.0
* @see Jetpack_XMLRPC_Methods::features_available() in the Jetpack plugin
*
* @return array
*/
public function features_available() {
_deprecated_function( __METHOD__, '1.25.0', 'Jetpack_XMLRPC_Methods::features_available()' );
if ( class_exists( 'Jetpack_XMLRPC_Methods' ) ) {
return Jetpack_XMLRPC_Methods::features_available();
}
return array();
}

/**
* Deprecated: This method is no longer part of the Connection package and now lives on the Jetpack plugin.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really sure about the major here and all the plugins. It makes sense to have a major in the connection package, but I don't think it makes sense for the plugins

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's fine to have it that way.
Since Connection changelog is "major", and all those plugins rely on it, it's logical to extend the "major" version increase to all of them.

Copy link
Contributor

Choose a reason for hiding this comment

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

What it really comes down to is "will this change potentially break things for people (properly) using the plugin?"

In this case, I'd say "probably not". I doubt the plugin exposes the functionality of this package method in its own UI or APIs. Anything using the package itself should itself be depending on the package rather than relying on any of our other plugins to provide it.

(I think we can ignore #17221 for the purposes of deciding if a plugin change is major).

Copy link
Contributor

Choose a reason for hiding this comment

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

Many thanks for replying @anomiex. I will update the PR accordingly and change the significance of the changelogs in the plugins to minor then

Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,21 @@ public static function register_endpoints() {
),
)
);

/**
* Get the list of available Jetpack features.
*
* @since $$next-version$$
*/
register_rest_route(
'jetpack/v4',
'/features/available',
array(
'methods' => WP_REST_Server::READABLE,
'callback' => array( static::class, 'get_features_available' ),
'permission_callback' => array( static::class, 'get_features_permission_check' ),
)
);
}

/**
Expand Down Expand Up @@ -4467,4 +4482,38 @@ public static function get_intro_offers() {
)
);
}

/**
* Return the list of available features.
*
* @return array
*/
public static function get_features_available() {
$raw_modules = Jetpack::get_available_modules();
$modules = array();
foreach ( $raw_modules as $module ) {
$modules[] = Jetpack::get_module_slug( $module );
}

return $modules;
}

/**
* Verify that the API client is allowed to replace user token.
*
* @since 1.29.0
*
* @return bool|WP_Error
*/
public static function get_features_permission_check() {
if ( ! Rest_Authentication::is_signed_with_blog_token() ) {
$message = esc_html__(
'You do not have the correct user permissions to perform this action. Please contact your site admin if you think this is a mistake.',
'jetpack'
);
return new WP_Error( 'invalid_permission_update_user_token', $message, array( 'status' => rest_authorization_required_code() ) );
}

return true;
}
} // class end
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: other

Create the 'features/available' REST endpoint.
2 changes: 2 additions & 0 deletions projects/plugins/jetpack/class-jetpack-xmlrpc-methods.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public static function xmlrpc_methods( $methods ) {
/**
* Returns what features are available. Uses the slug of the module files.
*
* @deprecated $$next-version$$
* @see Jetpack_Core_Json_Api_Endpoints::get_features_available()
* @return array
*/
public static function features_available() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,50 @@ public function test_jetpack_rest_api_get_authentication_success_with_blog_token
$this->assertSame( 0, get_current_user_id() );
}

/**
* @author darssen
*
* Test the 'features/available' endpoint authentication.
*
* @since $$next-version$$
*/
public function test_jetpack_rest_api_get_features_available_authentication_success() {
add_filter( 'pre_option_jetpack_private_options', array( $this, 'mock_jetpack_private_options' ), 10, 2 );
$token = 'pretend_this_is_valid_blog_token:1:0';
$timestamp = (string) time();
$nonce = 'testing123';
$body_hash = '';

$_GET['token'] = $token;
$_GET['timestamp'] = $timestamp;
$_GET['nonce'] = $nonce;
$_GET['body-hash'] = $body_hash;
$_GET['signature'] = base64_encode(
hash_hmac(
'sha1',
implode(
"\n",
array(
$token,
$timestamp,
$nonce,
$body_hash,
'GET',
'example.org',
'80',
'/jetpack/v4/features/available',
'qstest=yep',
)
) . "\n",
'secret_blog',
true
)
);
$this->request = new WP_REST_Request( 'GET', '/jetpack/v4/features/available' );
$response = $this->server->dispatch( $this->request );
$this->assertEquals( 200, $response->get_status() );
}

/**
* @author jnylen0
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1224,4 +1224,17 @@ public function test_post_seen_wc_connection_modal_success() {
$this->assertResponseStatus( 200, $response );
$this->assertResponseData( array( 'success' => true ), $response );
}

/**
* Test the 'features/available' endpoint, unauthorized.
*
* @since $$next-version$$
*/
public function test_features_available_unauthorized() {
// Create REST request in JSON format and dispatch
$response = $this->create_and_get_request( 'features/available' );

$this->assertResponseStatus( 401, $response );
$this->assertResponseData( array( 'code' => 'invalid_permission_update_user_token' ), $response );
}
} // class end
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: removed

Connection: Removed deprecated method features_available
Loading