Reduce duplicated code from offline payment methods to a dedicated class#168
Reduce duplicated code from offline payment methods to a dedicated class#168
Conversation
e1db86b to
f9f2a50
Compare
f9f2a50 to
06f9708
Compare
| 'description' => apply_filters( 'omise_charge_params_description', 'WooCommerce Order id ' . $order_id, $order ), | ||
| 'source' => array( 'type' => $this->source_type ), | ||
| 'metadata' => $metadata | ||
| ) ); |
There was a problem hiding this comment.
Note that return_uri has been removed. It is because Offline payment methods don't require for the return_uri parameter.
| abstract class Omise_Payment_Offline extends Omise_Payment { | ||
| /** | ||
| * A string of Omise Source's type | ||
| * (expecting for either 'paynow' or 'bill_payment_tesco_lotus'). |
There was a problem hiding this comment.
Comment should probably go as no doubt more offline payment methods will be added
There was a problem hiding this comment.
@jonrandy any suggestion? (can't think of other message than adding a word at the moment).
/**
* A string of Omise Source's type
* 'bill_payment_tesco_lotus', 'paynow' (at the moment).
*/There was a problem hiding this comment.
Just remove that line of the comment, the rest is fine
There was a problem hiding this comment.
If we must have an example, we can say:
(e.g.
paynoworbill_payment_tesco_lotus)
Then we're not limited when we inevitably add more source types
| abstract class Omise_Payment_Offline extends Omise_Payment { | ||
| /** | ||
| * A string of Omise Source's type | ||
| * (expecting for either 'paynow' or 'bill_payment_tesco_lotus'). |
There was a problem hiding this comment.
If we must have an example, we can say:
(e.g.
paynoworbill_payment_tesco_lotus)
Then we're not limited when we inevitably add more source types
|
@guzzilar not sure that this PR will fix issue what if the merchant would like to use live mode, but the setting is automatically changed to test mode? |
|
@bigomise Can I have more detail about that case? Never heard that issue before. |
…ine payment methods to reduce duplicated code.
b1abb19 to
1bb2c5e
Compare
| ); | ||
|
|
||
| return OmiseCharge::create( array( | ||
| 'amount' => Omise_Money::to_subunit( $total, $currency ), |
There was a problem hiding this comment.
In the future, we may want to consider passing custom expires_at params
1. Objective
This pull request is aiming to reduce duplicated code that was discovered during the implementation of API Localization #156
To reduce a place where we have to apply the L10n for those API response messages.
Related information:
Related issue(s): T21114 (internal ticket)
2. Description of change
• Implementing a dedicated class
Omise_Payment_Offlinefor those offline payment methods to reduce duplicated code.3. Quality assurance
🔧 Environments:
✏️ Details:
As we're refactoring the
chargeandresultmethods, we should be making sure that those 2 offline payment methods can be created properly.1.1. Charge can be created properly.
1.2. All the params are assigned properly (price, description, metadata)2.1. Charge can be created properly.
2.2. All the params are assigned properly (price, description, metadata)4. Impact of the change
No
5. Priority of change
Normal
6. Additional Notes
This
Omise_Payment_Offlineclass may get modified again once we switch back to work on #149 Konbini payment (as Konbini requires more parameters to be passed when creating a new charge)