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

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

Merged
merged 10 commits into from
Oct 20, 2021

Conversation

nimitomise
Copy link
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
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
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
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
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
Contributor Author

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

@kan98
Copy link
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