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

Allow non-account-owner admin users to submit the payment activity survey #8739

Merged
merged 8 commits into from
May 2, 2024
5 changes: 5 additions & 0 deletions changelog/fix-8710-payment-activity-survey-all-admin-users
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: fix
Comment: Part of Payment Activity Card behind feature flag. Allow non-accountholder users to submit the feedback survey.


39 changes: 20 additions & 19 deletions includes/admin/class-wc-rest-payments-survey-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,23 +98,15 @@ public function submit_payments_overview_survey( WP_REST_Request $request ): WP_
);
}

// Jetpack connection 1.27.0 created a default value for this constant, but we're using an older version of the package
// https://github.com/Automattic/jetpack/blob/master/projects/packages/connection/CHANGELOG.md#1270---2021-05-25
// - Connection: add the default value of JETPACK__WPCOM_JSON_API_BASE to the Connection Utils class
// this is just a patch so that we don't need to upgrade.
// as an alternative, I could have also used the `jetpack_constant_default_value` filter, but this is shorter and also provides a fallback.
defined( 'JETPACK__WPCOM_JSON_API_BASE' ) || define( 'JETPACK__WPCOM_JSON_API_BASE', 'https://public-api.wordpress.com' );

$wpcom_request = $this->http_client->wpcom_json_api_request_as_user(
'/marketing/survey',
'2',
[
'method' => 'POST',
'headers' => [
'Content-Type' => 'application/json',
'X-Forwarded-For' => \WC_Geolocation::get_ip_address(),
],
$request_args = [
'url' => WC_Payments_API_Client::ENDPOINT_BASE . '/marketing/survey',
'method' => 'POST',
'headers' => [
'Content-Type' => 'application/json',
'X-Forwarded-For' => \WC_Geolocation::get_ip_address(),
],
];
$request_body = wp_json_encode(
[
'site_id' => $this->http_client->get_blog_id(),
'survey_id' => 'wcpay-payment-activity',
Expand All @@ -125,14 +117,23 @@ public function submit_payments_overview_survey( WP_REST_Request $request ): WP_
],
]
);
$is_site_specific = true;
$use_user_token = true;

$wpcom_response = $this->http_client->remote_request(
$request_args,
$request_body,
$is_site_specific,
$use_user_token
);

$wpcom_request_body = json_decode( wp_remote_retrieve_body( $wpcom_request ) );
$wpcom_response_status_code = wp_remote_retrieve_response_code( $wpcom_response );

if ( ! is_wp_error( $wpcom_request ) ) {
if ( 200 === $wpcom_response_status_code ) {
update_option( 'wcpay_survey_payment_overview_submitted', true );
}

return new WP_REST_Response( $wpcom_request_body, wp_remote_retrieve_response_code( $wpcom_request ) );
return new WP_REST_Response( $wpcom_response, $wpcom_response_status_code );
}

/**
Expand Down
2 changes: 1 addition & 1 deletion includes/wc-payment-api/class-wc-payments-http.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function init_hooks() {
* @param array $args - The arguments to passed to Jetpack.
* @param string $body - The body passed on to the HTTP request.
* @param bool $is_site_specific - If true, the site ID will be included in the request url. Defaults to true.
* @param bool $use_user_token - If true, the request will be signed with the user token rather than blog token. Defaults to false.
* @param bool $use_user_token - If true, the request will be signed with the Jetpack connection owner user token rather than blog token. Defaults to false.
Copy link
Member Author

Choose a reason for hiding this comment

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

I've added this distinction here, which would have helped me resolve this issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we change the name of this param? user_token seems misleading.

*
* @return array HTTP response on success.
* @throws API_Exception - If not connected or request failed.
Expand Down
33 changes: 21 additions & 12 deletions tests/unit/admin/test-class-wc-rest-payments-survey-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function setUp(): void {
// Set the user so that we can pass the authentication.
wp_set_current_user( 1 );

$this->http_client_stub = $this->getMockBuilder( WC_Payments_Http::class )->disableOriginalConstructor()->setMethods( [ 'wpcom_json_api_request_as_user' ] )->getMock();
$this->http_client_stub = $this->getMockBuilder( WC_Payments_Http::class )->disableOriginalConstructor()->setMethods( [ 'remote_request' ] )->getMock();
$this->controller = new WC_REST_Payments_Survey_Controller( $this->http_client_stub );
}

Expand All @@ -63,33 +63,42 @@ public function test_empty_rating_returns_400_status_code() {
$this->assertEquals( 400, $response->get_status() );
}


public function test_valid_request_forwards_data_to_jetpack() {
Copy link
Contributor

Choose a reason for hiding this comment

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

How important and useful is this e2e test, considering that the survey has a limited lifespan?

$request_url = WC_Payments_API_Client::ENDPOINT_BASE . '/marketing/survey';

$this->http_client_stub
->expects( $this->any() )
->method( 'wpcom_json_api_request_as_user' )
->method( 'remote_request' )
->with(
$this->stringContains( '/marketing/survey' ),
$this->anything(),
$this->anything(),
// Check the request argument URL is the same.
$this->callback(
function ( $argument ) use ( $request_url ) {
return $request_url === $argument['url'];
}
),
$this->logicalAnd(
$this->arrayHasKey( 'survey_id' ),
$this->arrayHasKey( 'survey_responses' ),
$this->callback(
function ( $argument ) {
return 'wcpay-payment-activity' === $argument['survey_id'];
$json_body = json_decode( $argument, true );
return 'wcpay-payment-activity' === $json_body['survey_id'];
}
),
$this->callback(
function ( $argument ) {
return 'happy' === $argument['survey_responses']['rating'];
$json_body = json_decode( $argument, true );
return 'happy' === $json_body['survey_responses']['rating'];
}
),
$this->callback(
function ( $argument ) {
return 'test comment' === $argument['survey_responses']['comments']['text'];
$json_body = json_decode( $argument, true );
return 'test comment' === $json_body['survey_responses']['comments']['text'];
}
)
)
),
),
$this->isTrue(),
$this->isTrue(),
)
->willReturn(
[
Expand Down
Loading