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.

Sign up for GitHub

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

Alternative solution to handle forwarded "account.updated" webhook #612

Merged
merged 4 commits into from
Apr 28, 2020
Merged

Alternative solution to handle forwarded "account.updated" webhook #612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4cae19d
Handle forwarded account.updated webhook
vbelolapotkov Apr 17, 2020
e2651a1
Add webhooks logging in debug mode
vbelolapotkov Apr 20, 2020
a653bd7
Update webhook handler method
vbelolapotkov Apr 27, 2020
30fd86e
Fix WC_REST_Payments_Webhook_Controller unit tests
vbelolapotkov Apr 27, 2020
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
30 changes: 23 additions & 7 deletions includes/admin/class-wc-rest-payments-webhook-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,24 @@ class WC_REST_Payments_Webhook_Controller extends WC_Payments_REST_Controller {
*/
private $wcpay_db;

/**
* WC Payments Account.
*
* @var WC_Payments_Account
*/
private $account;

/**
* WC_REST_Payments_Webhook_Controller constructor.
*
* @param WC_Payments_API_Client $api_client WC_Payments_API_Client instance.
* @param WC_Payments_DB $wcpay_db WC_Payments_DB instance.
* @param WC_Payments_Account $account WC_Payments_Account instance.
*/
public function __construct( WC_Payments_API_Client $api_client, WC_Payments_DB $wcpay_db ) {
public function __construct( WC_Payments_API_Client $api_client, WC_Payments_DB $wcpay_db, WC_Payments_Account $account ) {
parent::__construct( $api_client );
$this->wcpay_db = $wcpay_db;
$this->account = $account;
}

/**
Expand Down Expand Up @@ -75,13 +84,17 @@ public function handle_webhook( $request ) {

try {
// Extract information about the webhook event.
$event_type = $this->read_rest_property( $body, 'type' );
$event_data = $this->read_rest_property( $body, 'data' );
$event_object = $this->read_rest_property( $event_data, 'object' );
$event_type = $this->read_rest_property( $body, 'type' );

Logger::debug( 'Webhook recieved: ' . $event_type );
Logger::debug( 'Webhook body: ' . var_export( $body, true ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_export

switch ( $event_type ) {
case 'charge.refund.updated':
$this->process_webhook_refund_updated( $event_object );
$this->process_webhook_refund_updated( $body );
break;
case 'account.updated':
$this->account->refresh_account_data();
break;
}
} catch ( WC_Payments_Rest_Request_Exception $e ) {
Expand All @@ -98,12 +111,15 @@ public function handle_webhook( $request ) {
/**
* Process webhook refund updated.
*
* @param array $event_object The event that triggered the webhook.
* @param array $event_body The event that triggered the webhook.
*
* @throws WC_Payments_Rest_Request_Exception Required parameters not found.
* @throws Exception Unable to resolve charge ID to order.
*/
private function process_webhook_refund_updated( $event_object ) {
private function process_webhook_refund_updated( $event_body ) {
$event_data = $this->read_rest_property( $event_body, 'data' );
$event_object = $this->read_rest_property( $event_data, 'object' );

// First, check the reason for the update. We're only interesting in a status of failed.
$status = $this->read_rest_property( $event_object, 'status' );
if ( 'failed' !== $status ) {
Expand Down
23 changes: 22 additions & 1 deletion includes/class-wc-payments-account.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,10 +431,31 @@ private function get_cached_account_data() {
}

// Cache the account details so we don't call the server every time.
set_transient( self::ACCOUNT_TRANSIENT, $account, 2 * HOUR_IN_SECONDS );
$this->cache_account( $account );
return $account;
}

/**
* Caches account data for two hours
*
* @param array $account - Account data to cache.
*/
private function cache_account( $account ) {
set_transient( self::ACCOUNT_TRANSIENT, $account, 2 * HOUR_IN_SECONDS );
}

/**
* Refetches account data.
*/
public function refresh_account_data() {
try {
delete_transient( self::ACCOUNT_TRANSIENT );
$this->get_cached_account_data();
} catch ( Exception $e ) {
WCPay\Logger::error( "Failed to refresh account data. Error: $e" );
}
}

/**
* Checks if the cached account can be used in the current plugin state.
*
Expand Down
2 changes: 1 addition & 1 deletion includes/class-wc-payments.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ public static function init_rest_api() {
$timeline_controller->register_routes();

include_once WCPAY_ABSPATH . 'includes/admin/class-wc-rest-payments-webhook-controller.php';
$webhook_controller = new WC_REST_Payments_Webhook_Controller( self::$api_client, self::$db_helper );
$webhook_controller = new WC_REST_Payments_Webhook_Controller( self::$api_client, self::$db_helper, self::$account );
$webhook_controller->register_routes();
}

Expand Down
10 changes: 6 additions & 4 deletions tests/admin/test-class-wc-rest-payments-webhook.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,14 @@ public function setUp() {
->disableOriginalConstructor()
->getMock();

$account = new WC_Payments_Account( $mock_api_client );

$this->mock_db_wrapper = $this->getMockBuilder( WC_Payments_DB::class )
->disableOriginalConstructor()
->setMethods( array( 'order_from_charge_id' ) )
->getMock();

$this->controller = new WC_REST_Payments_Webhook_Controller( $mock_api_client, $this->mock_db_wrapper );
$this->controller = new WC_REST_Payments_Webhook_Controller( $mock_api_client, $this->mock_db_wrapper, $account );

// Setup a test request.
$this->request = new WP_REST_Request(
Expand Down Expand Up @@ -113,7 +115,7 @@ public function test_webhook_with_no_type_property() {
*/
public function test_webhook_with_no_object_property() {
// Setup test request data.
$this->request_body['type'] = 'unknown.webhook.event';
$this->request_body['type'] = 'charge.refund.updated';
unset( $this->request_body['data']['object'] );
$this->request->set_body( wp_json_encode( $this->request_body ) );

Expand All @@ -128,11 +130,11 @@ public function test_webhook_with_no_object_property() {
}

/**
* Test a webhook with no object property.
* Test a webhook with no data property.
*/
public function test_webhook_with_no_data_property() {
// Setup test request data.
$this->request_body['type'] = 'unknown.webhook.event';
$this->request_body['type'] = 'charge.refund.updated';
unset( $this->request_body['data'] );
$this->request->set_body( wp_json_encode( $this->request_body ) );

Expand Down
61 changes: 61 additions & 0 deletions tests/test-class-wc-payments-account.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,4 +369,65 @@ public function test_try_is_stripe_connected_returns_true_when_connected_with_li

remove_filter( 'wcpay_dev_mode', '__return_true' );
}

public function test_refresh_account_data_with_empty_cache() {
$expected_account = [
'account_id' => 'acc_test',
'live_publishable_key' => 'pk_test_',
'test_publishable_key' => 'pk_live_',
'has_pending_requirements' => true,
'current_deadline' => 12345,
'is_live' => true,
];

// Make sure cache is clear.
delete_transient( WC_Payments_Account::ACCOUNT_TRANSIENT );

$this->mock_api_client->expects( $this->once() )->method( 'get_account_data' )->will( $this->returnValue( $expected_account ) );
$this->wcpay_account->refresh_account_data();

$cached_account = get_transient( WC_Payments_Account::ACCOUNT_TRANSIENT );
$this->assertEquals( $expected_account, $cached_account, 'Account is not cached' );
}

public function test_refresh_account_data_with_existing_cache() {
$expected_account = [
'account_id' => 'acc_test',
'live_publishable_key' => 'pk_test_',
'test_publishable_key' => 'pk_live_',
'has_pending_requirements' => true,
'current_deadline' => 12345,
'is_live' => true,
];

$existing_cache = $expected_account;
$existing_cache['current_deadline'] = 11111;
set_transient( WC_Payments_Account::ACCOUNT_TRANSIENT, $existing_cache );

$this->mock_api_client->expects( $this->once() )->method( 'get_account_data' )->will( $this->returnValue( $expected_account ) );
$this->wcpay_account->refresh_account_data();

$cached_account = get_transient( WC_Payments_Account::ACCOUNT_TRANSIENT );
$this->assertEquals( $expected_account, $cached_account, 'Cached account is not updated' );
}

public function test_refresh_account_data_clears_cache_on_failure() {
$account = [
'account_id' => 'acc_test',
'live_publishable_key' => 'pk_test_',
'test_publishable_key' => 'pk_live_',
'has_pending_requirements' => true,
'current_deadline' => 12345,
'is_live' => true,
];
set_transient( WC_Payments_Account::ACCOUNT_TRANSIENT, $account );

$this->mock_api_client->expects( $this->once() )->method( 'get_account_data' )->will(
$this->throwException( new WC_Payments_API_Exception( 'test', 'wcpay_account_not_found', 401 ) )
);
$this->wcpay_account->refresh_account_data();

$cached_account = get_transient( WC_Payments_Account::ACCOUNT_TRANSIENT );
$this->assertEquals( [], $cached_account, 'Cached account is not cleared' );
}
}