Skip to content

[APM2-59] fix underline issue related to disable merchant new order email for on-hold#232

Merged
nimitomise merged 10 commits intomasterfrom
disable-on-hold-order-email
Oct 20, 2021
Merged

[APM2-59] fix underline issue related to disable merchant new order email for on-hold#232
nimitomise merged 10 commits intomasterfrom
disable-on-hold-order-email

Conversation

@nimitomise
Copy link
Copy Markdown
Contributor

@nimitomise nimitomise commented Oct 18, 2021

  1. Objective
  • Fix underline issue related to disable merchant new order email for on-hold.
  • Send notification email to merchant once change order status to processing depending on payment method.

Related information:
Related PR(s): #232

  1. Description of change

Override Woocommerce filter for woocommerce_email_recipient_new_order which able to initiate with status as on-hold to not send order email to merchant.
Override woocommerce_order_status_processing to send notification email depend on payment method

  1. Quality assurance
  • no email send to merchant on complete checkout screen
  • found single email send to merchant when change order status to processing
  • new order email to customer both status as on-hold and processing

🔧 Environments:

Tested locally by pointing OMISE_API_URL to staging-omise

WooCommerce: v5.7.1
WordPress: v5.8
PHP version: 7.1

✏️ Details:

Make sure setting merchant email at /wp-admin/admin.php?page=wc-settings&tab=email&section=wc_email_new_order or /wp-admin/admin.php?page=wc-settings&tab=email to your valid email
Checkout cart with Paynow payment make sure customer email is valid and accessible
Go to customer mail box should still receive new order email
Go to merchant mail box should not receive new order email
Checkout cart with Credit Card and accessible
Go to customer mail box should still receive new order email
Go to merchant mail box should still receive new order email

  1. Impact of the change
    No

  2. Priority of change
    Normal

  3. Additional Notes
    No

@kan98 kan98 self-requested a review October 19, 2021 08:27
@kan98
Copy link
Copy Markdown
Contributor

kan98 commented Oct 19, 2021

just checking if you've tested other payment methods to see they still work as expected 🙂

@nimitomise
Copy link
Copy Markdown
Contributor Author

nimitomise commented Oct 19, 2021

@kan98
Yes. I have checked checkout with internet banking then an email with processing status to customer and an email to merchant.

Comment on lines +94 to +99
public function email_processing_admin_notification( $order_id, $order ) {
$payment_gateway = wc_get_payment_gateway_by_order( $order );
if (is_a( $payment_gateway, 'Omise_Payment' ) && $payment_gateway->enabled_processing_notification) {
WC()->mailer()->get_emails()['WC_Email_New_Order']->trigger( $order_id );
}
}
Copy link
Copy Markdown
Contributor

@kan98 kan98 Oct 19, 2021

Choose a reason for hiding this comment

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

not sure if it's worth refactoring this into a helper or service class. Benefit is we can move this logic out of the payment class and we can write some unit tests and mock a few orders to see they work as expected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let's me see

@tookaa2 tookaa2 requested a review from kan98 October 20, 2021 10:13
@nimitomise
Copy link
Copy Markdown
Contributor Author

Try to add unit test for Helper function but too complicated !!

@kan98
Copy link
Copy Markdown
Contributor

kan98 commented Oct 20, 2021

Try to add unit test for Helper function but too complicated !!

No problem! Thanks for refactoring the function out 🙂 . PR looks good to me 👍🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants