btcpayserver / woocommerce-plugin

Plugin for connecting Woocommerce with BTCPayServer
https://btcpayserver.org/
Other
51 stars 41 forks source link

Add Feature: On-site modal checkout #26

Open adiled opened 5 years ago

adiled commented 5 years ago

Feature works by switching the external redirect at checkout to btcpay.showInvoice call

Expected flow

If activated in settings

4/2019 Changelog

Please excuse the bulky commit, I had made the changes in distribution build and then ported to repo.

Credits to @astupidmoose for sponsoring and live testing

NicolasDorier commented 5 years ago

Awesome, I know that @astupidmoose worked on it also.

However, I think you should remove the btcpay.js from here and instead cross reference the one configured on the btcpayserver configured for the store. By doing so, you don't need to use setApiUrlPrefix at all. Also, bug fixes on btcpayserver will not need to make a new plugin version.

astupidmoose commented 5 years ago

I could be wrong as I didn't actually code it, but I think it was required to bring in the btcpay.js for the closing and redirection functionality. Else, that functionality would be needed to be added into btcpay.js in the master repo.

When calling it remotely, the window was not closing properly and would not redirect on payment success.

rockstardev commented 5 years ago

Hey @astupidmoose what we talked about doesn't apply here. It is really design question - do we want separate btcpay.js as part of woocommerce plugin or we should keep that modal file in BTCPayServer repository.

I discussed with Mike about us providing paid event... right now the way modal library works is that it only provides close event and then on clientside you should query server and ensure invoice is paid before redirecting.

adiled commented 5 years ago

@Kukks @NicolasDorier

btcpay.js is indeed a redundancy that I missed documenting while my review. I have exposed a function in it to retrieve id of the invoice being shown, which for the purpose @astupidmoose pointed out, enables the status checking of order without making cross origin requests. Would you suggest a PR on btcpayserver? I suppose that would cause a btcpayserver version dependency for the feature though.

rockstardev commented 5 years ago

I think we should have paid event in btcpay.js... then after redirection merchant can server-side verify that invoice is paid before confirming order. We just leave comment in JS library to ensure that people don't auto-ship orders on Javascript events that can easily be faked.

Kukks commented 5 years ago

@m-adilshaikh You can print the invoice id as a js var as well though which would give you global access and not require you to depend on that event to give you the id

adiled commented 5 years ago

@Kukks Yes I was expecting to enqueue order details but the flow is boxed up with WC Gateway API, the request goes async and response is handled by checkout.js I believe, couldn't figure where to intercept.

adiled commented 5 years ago

@Kukks Actually I just realized I am already calling a js function as redirect... I will look into this.

NicolasDorier commented 5 years ago

So do we need to modify btcpay.js on btcpayserver to make things simpler?

adiled commented 5 years ago

@NicolasDorier Not required after d76a23d

astupidmoose commented 5 years ago

@m-adilshaikh I can't get this latest PR to install.

Parse error: syntax error, unexpected '}', expecting ';' in /var/www/html/wp-content/plugins/woocommerce-btcpay/class-wc-gateway-btcpay.php on line 628

astupidmoose commented 5 years ago

Tried installing now

Plugin could not be activated because it triggered a fatal error.

End up getting this error: Fatal error: Uncaught Exception: The BTCPay payment plugin was not installed correctly or the files are corrupt. Please reinstall the plugin. If this message persists after a reinstall, contact the BTCPay team through http://slack.btcpayserver.org with this message. in /var/www/html/wp-content/plugins/btcpay-for-woocommerce/class-wc-gateway-btcpay.php:35 Stack trace: #0 /var/www/html/wp-admin/includes/plugin.php(2050): include() #1 /var/www/html/wp-admin/plugins.php(175): plugin_sandbox_scrape('btcpay-for-wooc...') #2 {main} thrown in /var/www/html/wp-content/plugins/btcpay-for-woocommerce/class-wc-gateway-btcpay.php on line 35

NicolasDorier commented 5 years ago

Any idea @m-adilshaikh ?

NicolasDorier commented 5 years ago

ping @m-adilshaikh (also need rebase)

Kukks commented 5 years ago

What's the status here @m-adilshaikh? Shall we review again? :)

astupidmoose commented 5 years ago

I need to check this out again. The original plugin @m-adilshaikh sent me works and is live in production, but I think it somehow re-added the libs and css file that were previously stripped. The updated versions do not work however.

I'm not sure how. I need to setup another test site to check the latest version.

astupidmoose commented 4 years ago

This code is pretty old and there was a bunch of flaws with it. I think modal checkout would be useful, but I don't think it's safe to push this PR through.

Not sure why it was "approved"

adiled commented 4 years ago

I have rebased and cleaned up the mess of reformatting I made, now I recall this had issues but we were very close, it was pending manual testing, I may have not committed the revisions after review. Perhaps someone can take it from here, I've lost context.