omise / omise-prestashop

Omise PrestaShop Plugin
https://docs.opn.ooo/prestashop-plugin
MIT License
4 stars 7 forks source link

Refactor the process of create Omise charge by extract class #16

Closed nimid closed 7 years ago

nimid commented 7 years ago

1. Objective

Reduce the dependencies and functionality of a controller class, OmisePaymentModuleFrontController, by using a refactoring technique, extract class.

The OmisePaymentModuleFrontController has been used to create Omise charge.

According to the pull request #17, it has emphasized the class, OmisePaymentModuleFrontController, has many dependencies and functionality. This situation may tend to the class has error prone, especially in part of software maintenance.

Including the next steps, this class will been modified to add more functions such as store the order, handle the errors and display the proper result.

So, it will be better to reduce the dependencies and functionality of a class, OmisePaymentModuleFrontController.

Related information: Related issue: - Related ticket: - Required pull request: #17

2. Description of change

3. Quality assurance

Environments:

Details:

Test case 1: Successfully checkout

The screenshot below shows the result page display the successful payment.

prestashop-checkout-success

The screenshot below shows charge detail at Omise dashboard. This charge was created from the above checkout.

omise-dashboard-success-charge

Test case 2: Failed checkout

The screenshot below shows the result page display the error payment.

prestashop-checkout-fail

4. Impact of the change

-

5. Priority of change

Normal

6. Additional notes

The function to handle the create Omise charge error will be developed for the next pull request.

The screenshot below shows the passed unit tests at Travis CI.

travis-success

guzzilar commented 7 years ago

FYI: Im gonna close this PR since this one already merged to #19 and we're going to review this changes in that PR.

Thanks for the changes 👍

Note, the reason we moved is, because PR #14 and #17 were made to introduce Omise Charge service to the plugin but this PR did refactoring code based on those changes (#14, #17).

To make the review process more easy to follow with and more clear the big picture of those changes (including this refactoring changes). We decided to merge all of them into 1 branch, which is #19.