veritrans / SNAP-Woocommerce

MIT License
19 stars 17 forks source link

Suggestion to make Snap payment pop-up on checkout page, so that it allow customer to revise their cart #70

Open erikdemarco opened 2 years ago

erikdemarco commented 2 years ago

Currently the payment procedure is very confusing. Snap payment should be popup inside a checkout page (it should never clear the cart before payment is finished!)

Lets say a visitor already ready to checkout, Then he click pay and gets redirected to 'order-pay' page. But he forget something and want to edit his order. then he press go back just to be shown cart page is now empty.

I never see such payment procedure in any big marketplace before. People sometimes make mistake, we should allow people to edit their order.

So I propose the popup box should be opened in checkout page. then gets redirected and clearing cart only if the payment is successful. If visitor decide to fix something, he can just close the snap payment popup. and continue editing the order.

rizdaprasetya commented 2 years ago

Hi @erikdemarco thanks for reaching out and expressing your feedback 👋

So I propose the popup box should be opened in checkout page. then gets redirected and clearing cart only if the payment is successful. If visitor decide to fix something, he can just close the snap payment popup. and continue editing the order.

First, yes agree, we also think that would be the ideal scenario 👍 . We even tried to implement that in the past actually, and not just for WooCommerce(WC) but also multiple other CMSes. But unfortunately throughout those iterations, we found that there are several limitations along the way using that flow:

Those doesn't happen for every case, but are valid issues for few cases like that, hence we cannot risk it (for now). It overall feels less stable experience, because of cases like that badly impacting the overall experience for general users of the plugin. Note that we are not just developing this plugin for specific needs of few users, we are trying to cover most general users needs. And in this case, the current flow is more stable for most users.

You may ask, why not handle each of that edge-cases? It may be possible, but there will be many edge-cases that need to be identified, handled differently & properly, it may further end up bloating the implementation, more codes == more scenarios == more chance for bug to creep-in, etc. And there's always chance a new WC update can break all custom those. So for more stable experience, we opt-in to follow the recommended/standard WC flow. This flow is also the case for many other CMSes, not specific to WC, that the order/cart should be locked during payment, to minimize un-sync records.

I never see such payment procedure in any big marketplace before.

Correct, they are also coding their marketplace from-scratch, not by using ready-to-use CMS. You likely are a WP dev, you may know how can it be really tricky to implement custom functionality which may require you to "go against" the WP technical design & limitations. There's always risk of breaking/unstable due to future WP updates.

TLDR; At that time we found that the risk of having that flow out-weight the benefit unfortunately. Though we believe that ideal flow should be the aim, so in the future we'll try to re-asses when it become more technically viable. We appreciate that your feedback is aligned with us 😄

rizdaprasetya commented 2 years ago

Forgot to mention, for now there's alternative flow that you can use, by using this feature: https://docs.midtrans.com/en/snap/with-plugins?id=advanced-specific-payment-buttons

You can allow customer to see & choose the payment method from the checkout page first, so in case they want to revise their cart, they can do it before choosing them. From tech perspective, even implementing such flow did also end up a bit "bloating" the implementation, but at least this flow is currently more stable.

Edit: as this plugin is also open sourced, Merchant's dev are free to use/modify the code as needed in order to cover their own specific needs.

erikdemarco commented 2 years ago

"the payment is recorded on Midtrans, but WC flow is interrupted and cart/order is not submitted." Can you explain it more. What do you mean by order is not submitted? Before woocommerce can process payment, Order already saved thus woocommerce issue order_id, look here:

=> https://github.com/woocommerce/woocommerce/blob/eb7a72737190c3396d826286aad6f7ebf216fd13/plugins/woocommerce/includes/class-wc-checkout.php#L1172

After that woocommerce will call process_order_payment() and will load logic based on chosen payment gateway.

=> https://github.com/woocommerce/woocommerce/blob/eb7a72737190c3396d826286aad6f7ebf216fd13/plugins/woocommerce/includes/class-wc-checkout.php#L1195

After that it will issue redirect-url. This redirect url is usually a payment gateway's page. So user gets redirected to payment gateway's cehckout page to finish the payment. Until this process order is already saved in store's DB.

After payment gateway received the payment. payment gateway will call store's callback url to notify "hey we have received payment for this order id" Then the store updating the order status for that order id and send email to customer to notify the updated order status. After that callback finished, payment gateway will redirect visitor to the store's thankyou page and see their updated status.

Ok lets say during the payment gateway fail to calling the store's callback url. Most payment gateway usually have retry ability such as: => https://developers.xendit.co/api-reference/#delivery-attempts-and-retries Then the user will gets notified when order status is updated

Ok lets say even after retring, it still fail. User can contact store manually. Complain why the order status is not changed but the payment is sent. Then the store staff can check it manually. Its very rare case to have callback fail after multiple retry.

So in which part/process the woocommerce order not get saved? If the woocommerce order is not saved yet, it will never process payment.

rizdaprasetya commented 2 years ago

Before woocommerce can process payment, Order already saved thus woocommerce issue order_id

@erikdemarco Good point, TBH we haven't take a deeper look how WC flow work in these recent versions, so our understanding may not be up to date or deep enough. Assume we implemented the proposed flow, does that means:

My current assumption, the cart will already be cleared, so that even customer close the Snap pop-up the cart will not able to be revised.

Most payment gateway usually have retry ability

Correct, we do have that, it rarely fails. The blocker part is more on previous point. Also another tricky part:

Yes it does sound straight forward on plan, but when we actually tried to implement it, all the tricky & edge cases start to reveal themselves. One may not know if one haven't tried it.

Not mentioning we still haven't figure out to minimize risk of frontend/JS conflict on checkout page, if merchant's decided to have "fancy" plugin/WP setup.

erikdemarco commented 2 years ago

"on checkout page, before Snap popup appear, will the order be saved already by WC? Will that means the cart is cleared?"

If you dont call WC()->cart->empty_cart() inside process_payment() the cart will never gets cleared.

"if the cart is cleared before Snap pop-up, can customer revise their cart after canceling the pop-up? (can the proposed goal be achieved?"

Its possible if somehow we store the cart items in the session to restore it later. But its too complicated compared just keep the cart intact.

"if the customer's cart is not cleared/submitted during the payment, when the payment's frontend interrupted, then gateway's webhook call the store to say "hey update your order status", that webhook handler is expected to now be responsible to "clear" the customer's cart (asynchronously from backend). Webhook don't have the session/cookie context of the customer's, so it doesn't know exactly which customer's cart to clear, except to manually query the order to find out. Which means more DB call, and is not a standard flow of WC, means custom implementation."

You dont need to worry about that. Woocommerce is smart enough to automatically clear the cart for you. You just need to change the order to processing/completed.

"Not mentioning we still haven't figure out to minimize risk of frontend/JS conflict on checkout page, if merchant's decided to have "fancy" plugin/WP setup."

I think this is best practice. This is how woocommerce standard paypal works. They didnt clear the cart during process_payment: => https://github.com/woocommerce/woocommerce/blob/3611d4643791bad87a0d3e6e73e031bb80447417/plugins/woocommerce/includes/gateways/paypal/class-wc-gateway-paypal.php#L321

For some gateway like manual payment like bacs,cheque, they clear it. For automatic payment like paypal. they didnt clear it after the payment is complete.

rizdaprasetya commented 2 years ago

If you dont call WC()->cart->empty_cart() inside process_payment() the cart will never gets cleared.

Good point, was kinda unsure if clearing cart is manual or automatic when order is submitted.

Its possible if somehow we store the cart items in the session to restore it later. But its too complicated compared just keep the cart intact.

Yes, should avoid this approach.

You dont need to worry about that. Woocommerce is smart enough to automatically clear the cart for you. You just need to change the order to processing/completed.

Oh that's great, wasn't aware of it.

I think this is best practice. This is how woocommerce standard paypal works. They didnt clear the cart during process_payment

Thanks for the reference. It's tricky to know "best practice" for WC/WP, because the dev docs sometimes does not provide enough resource. In the official WC guide for gateway, it mentions about empty_cart(), so we assumed that was the best practice, or at least the standard flow. But then as you linked, in their own 1st party code (Paypal WC gateway was built by WC team) they don't empty_cart(). It's like we are expected to always deep dive into their source code to figure out their version of best practice.

Ok given these insights you provided, it seems the proposed flow is now more likely able to be implemented, we'll check further. Also we still need to check for edge cases & other risks though. Like whether there any cases order is: submitted in WC, recorded in Midtrans, but WC cart doesn't get cleared; basically "un-sync states issue". Test whether it breaks anything or has unexpected side-effect. There's also a factor whether existing merchant will like the proposed flow or not. Whether to make it a feature-switch or forcefully use it as default flow. Whether it will get prioritized based on risk-benefit.

So we may need some time for the actual implementation, just as heads up. But anyway thanks, we are few steps closer to it now.

rizdaprasetya commented 2 years ago

Woocommerce is smart enough to automatically clear the cart for you. You just need to change the order to processing/completed.

Identified one potential edge case:

erikdemarco commented 2 years ago

"Identified one potential edge case"

I know you going to say that. Thats why I say "For some gateway like manual payment like bacs,cheque, they clear it. For automatic payment like paypal. they didnt clear it after the payment is complete."

Look at tokopedia or shopee. They will clear the cart if the payment method is "transfer bank". but they didnt clear the cart if the payment method is credit card, so customer can still click back button to edit order. So its depends on the chosen payment method.

If you think it logically, People using bank transfer can't do anything anymore to their order (so the order considered finished) Then people using credit card still needs to type their card information, so they can still go back to change something (so its not considered finished yet)

rizdaprasetya commented 2 years ago

Thats why I say "For some gateway like manual payment like bacs,cheque, they clear it. For automatic payment like paypal. they didnt clear it after the payment is complete."

Ok, got it, I just realized what was the context you were trying to say.

Correct, that's also what I was thinking, bank-transfer once chosen by customer, should clear the WC cart. But

Then as workaround:

Are you ok with that? or have any ideas?

erikdemarco commented 2 years ago

"we cannot clear it via "change the order to processing/completed", because the payment status is not completed."

Yes you can clear it via 'process_payment' i said earlier. So each method should have each own classes. see this: => https://github.com/woocommerce/woocommerce/blob/27eba955ddd38c3606a03c3266b8f447afbe6b1d/plugins/woocommerce/includes/gateways/bacs/class-wc-gateway-bacs.php#L370

By the way, I wanna ask. does midtrans have example on php implemention for each of their payment gateway system? So developer can easily replicate to their own system. Just like what doku did: https://github.com/PTNUSASATUINTIARTHA-DOKU/jokul-php-library

Because midtrans have too many way how to implement and its a bit confusing. Its more easy to understand via example code.

rizdaprasetya commented 2 years ago

Oh I see, you were referring to specific bank-transfer payment method button. In that case, yes that's straight forward, we can clear cart synchronously during process_payment before opening Snap-popup on checkout page. But the downside is once customer chose that button, cart is not able to be revised.

My issue above was referring & trying to figure out the solution for to the main original button (that says "All Supported Payment Method"), because that 1 button contains multiple payment-methods, so we cannot identify which one customer chose before Snap pop-up appear. So can't just clear cart during process_payment.

Yes we do have, (long before they even have that actually). But yes, we do have more features so it can get more overwhelming. But specific for Snap (which is being used in this WC gateway plugin), you can find it under this section.

We also have some tips to run the code easily without cloning code to local. Also all our library repos have sample code, so each repo can be cloned and dev can try to run the samples immediately.

erikdemarco commented 2 years ago

My issue above was referring & trying to figure out the solution for to the main original button (that says "All Supported Payment Method"), because that 1 button contains multiple payment-methods, so we cannot identify which one customer chose before Snap pop-up appear. So can't just clear cart during process_payment.

Then you can just change the order status to 'on-hold' it will also clear the cart.

Yes we do have, (long before they even have that actually). But yes, we do have more features so it can get more overwhelming. But specific for Snap (which is being used in this WC gateway plugin), you can find it under this section.

Ok thank you for the link. Is there anyway to contact you directly via email in case i need technical help about midtrans? Because When I contact via support and asking about technical stuff, they seems not helpful at all.

rizdaprasetya commented 2 years ago

Then you can just change the order status to 'on-hold' it will also clear the cart.

Sounds good, that will be possible then. Will do some test on it.

Unfortunately by company's policy by default all support inquiries should have to go through that official Support Contact, so it can be properly tracked & recorded. That team supposed to forward/escalate technical question to my team, not sure why sometimes that doesn't happen properly. I'll provide it as feedback to the team. You can also open Github Issue on that Midtrans PHP repo, my team will try to answer technical question as soon as we are available, should have better response time than mailing to me. We don't usually share mail publicly to avoid bot & spam. But anyway, you can find the address there.