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

Code refactoring, simplifying the 'process_refund' method #157

Merged
merged 3 commits into from Apr 3, 2020

Conversation

guzzilar
Copy link
Contributor

@guzzilar guzzilar commented Mar 19, 2020

1. Objective

The main concept of this refactoring is about implementing the refund feature for Alipay and Installment. However, looking at the current code and found that the process_refund method has been implemented separately in Credit Card and TrueMoney Wallet payment methods.

Therefore this pull request is submitted to reduce the duplicated code so we can easily implement the refund feature to those 2 payment methods without producing any more duplicated.

Related information:
Related issue(s): T19965 (internal ticket)

2. Description of change

  • Removing process_refund method out from Omise_Payment_Truemoney and Omise_Payment_Creditcard classes (move to its parent class, Omise_Payment).

  • Implementing the process_refund method back to Omise_Payment class (so it can be shared between each payment method class that are supported for the refund feature.

  • Tweaking messages that are related to the refund feature. Making it more lean, and less redundant, clearer message.

3. Quality assurance

🔧 Environments:

  • WooCommerce: v3.7.1
  • WordPress: v5.3.2
  • PHP version: 7.0.16
  • Omise plugin version: Omise-WooCommerce 3.11 (optional, in case of submitting a new issue)

✏️ Details:

To make sure that this refactoring will not break any current feature, there are 3 main points we should cover.
1. Making sure we can make a refund using TrueMoney Wallet payment method.
1.1. After placed an order, at WooCommerce Order page, you can create a refund here.
truemoney-refund-step-01

1.2. If succeed, there will be a note that the order has been refunded.
truemoney-refund-step-02

1.3. Double check at Omise Dashboard to make sure that the refund has been done properly.
truemoney-refund-step-03

2. Making sure we can make a refund using Credit Card payment method.

card-refund-step1

card-refund-step2

card-refund-step3

3. Making sure we can make a refund using Credit Card payment method and the plugin can detect and add a proper note accordingly if that refund is being treated as voided.

card-void-step1

card-void-step2

card-void-step3

Also, testing and making sure that the plugin can handle failure cases properly.

4. Making sure that the plugin will be raising a proper failure message in case if it cannot retrieve an order object from a given order ID.

Note, this case is an extremely rare case, as the only chance that this case would happen is when the database gets corrupted.

Also note, to test this case, you will need to modify code, at file:
includes/gateway/class-omise-payment.php
Line: 240. Add a random string after an order id.

Screen Shot 2563-03-19 at 23 51 15

By doing that, when you try to create a refund, the plugin will raise an error that, it cannot retrieve order from the given order ID.

refund-failed-1

5. You mat try modify another part of the process_refund code to see different cases of failure message. In this case, when the Omise Charge object cannot be retrieve.
At the same file as the case [4], but adding a random string at the line 250.
Screen Shot 2563-03-19 at 23 59 51

You will get an error message saying that charge cannot be retrieve.
refund-failed-2

6. Or, at the line 252 of the same file, you may add a random amount that make the refund failed.
Screen Shot 2563-03-20 at 00 01 49

By trying to create a refund with amount that is higher than its charge's amount, you will receive a message saying that refund failed.
refund-failed-3

7. Also, making sure that those orders that are placed using payment methods that don't support for the refund feature, won't be able to create a refund via Omise.
Placing an order using Internet Banking payment method will not show "Refund via Omise" button at the order detail page.
refund-no-support

4. Impact of the change

Nothing

5. Priority of change

Normal

6. Additional Notes

This PR is focusing on relocating the process_refund method.
There will be 3 more PRs coming.

  1. Make use of $reason variable that is in the 3rd argument of process_refund. (in WooCommerce, you can add a reason of refunding, which we can pass that reason to the metadata parameter when creating a new Refund object)

  2. Adding refund feature to Alipay and Installment payment methods.

  3. As from the internal ticket that WooCommerce cannot sync up refund status from Omise Dashboard. One more dedicated PR will be coming to handle this issue.

@guzzilar guzzilar changed the title [WIP] Code refactoring, moving the 'process_refund' method up to the payment's parent class. [WIP] Code refactoring, simplifying the 'process_refund' method Mar 19, 2020
@guzzilar guzzilar changed the title [WIP] Code refactoring, simplifying the 'process_refund' method Code refactoring, simplifying the 'process_refund' method Mar 19, 2020
@guzzilar
Copy link
Contributor Author

guzzilar commented Mar 19, 2020

Here is a use case after this refactoring.
https://github.com/omise/omise-woocommerce/pull/158/files#diff-3f1dd1d765734b8bbc1ab3abf4d36ff6R12

By adding this line, it will enable payment methods to be able to create a refund on WooCommerce user interface.

Copy link

@jonrandy jonrandy left a comment

Choose a reason for hiding this comment

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

A couple of English things, and a function name

includes/gateway/class-omise-payment.php Show resolved Hide resolved
includes/gateway/class-omise-payment.php Outdated Show resolved Hide resolved
includes/gateway/class-omise-payment.php Outdated Show resolved Hide resolved
@guzzilar
Copy link
Contributor Author

guzzilar commented Apr 2, 2020

@jonrandy thanks for pointing out 👍
Fixed here 287dcb6

@guzzilar guzzilar merged commit 1c35f3a into master Apr 3, 2020
@guzzilar guzzilar deleted the clean-paymentmodule-refund branch April 3, 2020 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants