[bug fixes] Fixed minor bugs in the code #1
Conversation
mayankamencherla
force-pushed
the
bug_fixes
branch
3 times, most recently
from
8bace96
to
af650ad
Compare
mayankamencherla
force-pushed
the
bug_fixes
branch
from
af650ad
to
c27f415
Compare
| @@ -1,14 +1,27 @@ | |||
| <?php | |||
|
|
|||
| require_once __DIR__.'/../razorpay-payments.php'; | |||
| require_once __DIR__.'/../razorpay-sdk/Razorpay.php'; | |||
| $pluginRoot = WP_PLUGIN_DIR . '/' . RAZORPAY_WOOCOMMERCE_PLUGIN; | |||
There was a problem hiding this comment.
Verify if require is necessary here. The script is getting called from razorpay-subscriptions.php, which already includes everything.
| protected $razorpay; | ||
| protected $api; | ||
|
|
||
| const PAYMENT_AUTHORIZED = 'payment.authorized'; |
There was a problem hiding this comment.
Not needed. already defined in parent class
| { | ||
| const RAZORPAY_SUBSCRIPTION_ID = 'razorpay_subscription_id'; | ||
| protected $razorpay; |
There was a problem hiding this comment.
Already defined as protected in parent class. Not needed
| const PAYMENT_AUTHORIZED = 'payment.authorized'; | ||
| const PAYMENT_FAILED = 'payment.failed'; | ||
|
|
||
| function __construct() |
There was a problem hiding this comment.
remove, already same defined in parent class
| @@ -21,11 +34,11 @@ public function process() | |||
| return; | |||
| } | |||
|
|
|||
| if ($this->razorpay->enable_webhook === 'yes' && empty($data['event']) === false) | |||
There was a problem hiding this comment.
Remove process method, exactly same as in parent class
| return $this->processSubscription($orderId, $paymentId); | ||
| $invoiceId = $data['payload']['payment']['entity']['invoice_id']; | ||
|
|
||
| $invoice = $this->api->invoice->fetch($invoiceId); |
| @@ -148,7 +168,7 @@ protected function paymentAuthorized($data) | |||
| * | |||
| * @param $data | |||
| */ | |||
| protected function paymentFailed($data) | |||
| protected function paymentFailed(array $data) | |||
| return $this->processSubscription($orderId, $paymentId); | ||
| $invoiceId = $data['payload']['payment']['entity']['invoice_id']; | ||
|
|
||
| $invoice = $this->getInvoice($invoiceId); |
There was a problem hiding this comment.
change to getSubscription, and throw an exception if no subscription is found
| } | ||
| } | ||
|
|
||
| exit; |
There was a problem hiding this comment.
Don't exit for cases you aren't handling
|
|
||
| class RZP_Subscription_Webhook extends RZP_Webhook | ||
| { | ||
| /** |
There was a problem hiding this comment.
Can you add a similar comment like the one at https://github.com/razorpay/razorpay-woocommerce/pull/60/files#diff-5ceb8b9921fd0248b3e9e9669726b4d1R24 for this plugin as well
| /** | ||
| * Helper method used to handle all subscription processing | ||
| * | ||
| * @param $orderId |
| */ | ||
| protected function processSubscription($paymentId, $subscriptionId, $success = true) | ||
| { | ||
| // |
| catch (Exception $e) | ||
| { | ||
| $message = $e->getMessage(); | ||
| return 'RAZORPAY ERROR: Subscription fetch failed with the message \'' . $message . '\''; |
There was a problem hiding this comment.
Use string interpolation instead for simple strings.
|
|
||
| $orderId = $subscription->notes->woocommerce_order_id; | ||
|
|
||
| if ($success === false) |
includes/razorpay-subscriptions.php
Outdated
| @@ -1,330 +1,216 @@ | |||
| <?php | |||
| /* | |||
There was a problem hiding this comment.
include files don't need this
Please fix this file?
Bug fixes in the code