Refactor payments-stripe to only use getStripeInstanceForShop

[rattrayalex-stripe]

Resolves #4942 (review)
Impact: minor
Type: refactor

Issue

This is a followup to #4942

1. There was a string constant ("reaction-stripe") being duplicated in many places.
2. Some code referenced paymentMethod.paymentPluginName, but should have instead referenced the above constant.
3. Use of connectAuth.access_token should probably be deprecated from payments-stripe, as it should only be set in the marketplace plugin.
4. We were instantiating stripe in several places, potentially in inconsistent ways.

Solution

1. Make a constants file with STRIPE_PACKAGE_NAME.
2. Get rid of getStripeApiKey, move its connectAuth functionality into getStripeInstanceForShop with a warning log.
3. Separate out getStripePackageForShop since I thought it would be needed elsewhere, but I'm not sure this is the case.

I have a few questions for the reviewer (presumably @aldeed ):

1. getStripeInstance and getStripePackage are now only called from one file. Should they be moved into that file?
2. Is the constant in the right place, and does the name seem good?
3. Is Packages.findOne() actually asynchronous? If not, I can remove await from calls to instantiate Stripe, which would be nice.

Breaking changes

Previously, certain calls to instantiate stripe would throw an error if you did not have an api_key set. Now, those same calls will succeed, but use connectAuth.access_token if it is set. This is a breaking change, though my intuition is that no users would have relied on this behavior while also having connectAuth.access_token set 🤞.

Testing

I ran the unit tests locally.

[aldeed]

@rattrayalex-stripe Thanks! Can you merge latest develop branch into this branch? It will delete the checkout client files that you modified because they were for the old checkout UI, which is gone in the next release. The recommendation now is to build your own or start with the example storefront.

To answer your questions:

• getStripeInstance and getStripePackage may at some point be needed in other files (we plan to add a stored card method, for example). So I'd keep them separated.
• Constant name and location is fine
• Yes Packages.findOne() returns a Promise.

[rattrayalex-stripe]

Interesting! Will that use the new checkout (currently in beta)? Should make things much easier for users.

In any case, rebased!

[aldeed]

@rattrayalex-stripe There is a test failing on CI: https://circleci.com/gh/reactioncommerce/reaction/37940?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-checks-link

Looks like one of the import paths for the constant needs to be fixed.

I wasn't aware of the new full-page checkout and the example storefront does not currently use it, but we may want to implement that as an alternative example. Reaction will not be shipping with a storefront anymore, but there will be example storefronts and docs with storefront development instructions. So it is up to individual installations whether they want to migrate to the new checkout page, but we can ensure that it works on the server API side.

[rattrayalex-stripe]

Oops, thanks for catching that - maybe caused by the rebase.

Should be good now!

And interesting re; storefronts... Let me know if you need help adding checkout compatibility!

[rattrayalex-stripe]

Whoops, just noticed this is still open 😅 ping @aldeed
I'm actually interested in making one or two deeper contributions involving some new API's we're launching. Any chance you'd be up to discuss real quick over email/phone? my email is github_username.replace('-', '@') + '.com'

[aldeed]

@rattrayalex-stripe Thanks. Last thing I remember was pulling this down to test but I'm not sure I ever completed that testing before getting busy with other things. I'll prioritize for this week. And I'll send you an email.

[rattrayalex-stripe]

Awesome, thanks!

[aldeed]

Finally got this tested and merged! Thanks @rattrayalex-stripe