-
Notifications
You must be signed in to change notification settings - Fork 10
GPAPI Subscriptions Sompatibility #41
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
base: master
Are you sure you want to change the base?
Conversation
src/Gateways/AbstractGateway.php
Outdated
|
||
const TXN_TYPE_PAYPAL_INITIATE = 'initiatePayment'; | ||
// Subscription | ||
const TXN_TYPE_SUBSCRIPTION_PAYMENT = "subscriptionPayment"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use single quotes here, for consistency with the rest of the constants
src/Gateways/AbstractGateway.php
Outdated
$order->get_transaction_id() | ||
); | ||
// If the order contains a subscription, add the muti-use token to the order meta for repeat payments. | ||
if(function_exists("wcs_order_contains_subscription") && wcs_order_contains_subscription($order) && $is_successful){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not respect the white spacing coding standards (https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/#whitespace).
It needs to be rewritten to: if ( function_exists( 'wcs_order_contains_subscription' ) && wcs_order_contains_subscription( $order ) && $is_successful ) {
src/Gateways/AbstractGateway.php
Outdated
); | ||
// If the order contains a subscription, add the muti-use token to the order meta for repeat payments. | ||
if(function_exists("wcs_order_contains_subscription") && wcs_order_contains_subscription($order) && $is_successful){ | ||
$order->add_meta_data("_GP_multi_use_token", $response->token, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The white spaces comment applies in here too.
src/Gateways/Clients/SdkClient.php
Outdated
} | ||
} | ||
// Checks if order contains a subscription and requests a muti-use token. | ||
if(function_exists("wcs_order_contains_subscription")){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The white spaces comment applies in here too.
src/Gateways/Clients/SdkClient.php
Outdated
} | ||
// Checks if order contains a subscription and requests a muti-use token. | ||
if(function_exists("wcs_order_contains_subscription")){ | ||
if(isset($this->builder_args['orderId']) && wcs_order_contains_subscription(wc_get_order($this->builder_args['orderId'][0]))){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The white spaces comment applies in here too, except for the ['orderId'] part, where it is fine as it is, according to the coding standards.
src/Gateways/GpApiGateway.php
Outdated
$response = $this->client->submit_request( $request ); | ||
$client_trans_ref = $response->transactionReference->clientTransactionId; | ||
if(parent::handle_response($request,$response)){ | ||
$renewal_order->add_order_note(__("Subscription Renewal Successful \r\n Transaction Reference: ". $client_trans_ref)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For adding variables to translatable strings you will need to use the 'sprintf' function. You are also missing the translation domain.
Please check other strings translated in the plugin.
https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/
src/Gateways/GpApiGateway.php
Outdated
|
||
public function renew_subscription($amount_to_charge, $renewal_order) | ||
{ | ||
$renewal_order_subscriptions = wcs_get_subscriptions_for_renewal_order($renewal_order->get_id(),array("order_type"=>"parent")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The white spaces comment applies to this function too.
|
||
defined('ABSPATH') || exit; | ||
|
||
class SubscriptionRequest extends AbstractRequest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The opening braces and whitespace comments apply in here too.
@@ -0,0 +1,91 @@ | |||
<?php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code in this file seems to be indented using spaces and not tabs. This needs to be fixed.
src/Gateways/GpApiGateway.php
Outdated
* Handles subscription renewals, gets the muti-use token from the order meta and charges it. | ||
* @return bool | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This white line should be removed.
src/Gateways/Clients/SdkClient.php
Outdated
} | ||
} | ||
// Checks if order contains a subscription and requests a muti-use token. | ||
if( function_exists( "wcs_order_contains_subscription" ) ){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a space after the 'if' keyword too. This applies to the other files too.
|
||
return $response; | ||
} | ||
private function get_muti_use_token(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a space before the brace for this method and the below ones, for consistency.
No description provided.