🎨 Optimized loading stripe scripts only when it is needed #11499
Conversation
… configured closes TryGhost#11463 Ghost used to always load stripe.js into the frontend of all pages when memberships are enabled, even when Stripe isn't configured / memberships to a page are free. This changes Ghost's behaviour to only load stripe.js when both stripe API tokens are present & not empty (the quickets way to verify that Stripe is fully configured & operational on a blog).
| const stripePublicToken = stripePaymentProcessor.config.public_token; | ||
|
|
||
| var membersHelper = `<script defer src="${getAssetUrl('public/members.js')}"></script>`; | ||
| if (!!stripeSecretToken && stripeSecretToken !== '' && !!stripePublicToken && stripePublicToken !== '') { |
There was a problem hiding this comment.
Think it would be cleaner to reuse isPaymentConfigured method logic straight from membersService https://github.com/TryGhost/Ghost/blob/3.2.0/core/server/services/members/index.js#L26-L29 :
const settings = settingsCache.get('members_subscription_settings');
return !!settings && settings.isPaid && settings.paymentProcessors.length !== 0;and leave a comment about where it's taken from. Also, no need for stripeSecretToken/stripePublicToken checks as we don't allow any other processors so far. YAGNI 😄
Ideally, we'd have a separate flag in the settingsCache or public settings endpoint which would identify if Stripe payment processor is configured, but that goes out of the scope of this problem 🙂 Reason being: using the settings directly couples the "frontend" with Ghost instance internals which we are trying to avoid (frontend is meant to be as independent as possible so that it can be run separately from Ghost API instance in the future). But, for now, think this is an acceptable approach if we leave a note for a person that comes around to refactor this code.
//cc @allouis you might be interested 😉
There was a problem hiding this comment.
@allouis just had another look at this logic in attempt to update the PR and realized the method I referenced in membersService above - isPaymentConfigured is never used, additionally we probably have a leftover flag isPaid hanging in the settings object as it's never changed to anything but it's default false value. Think it's safe to remove both isPaid flag and the isPaymentConfigured method?
There was a problem hiding this comment.
Additionally, we start an instance with paymentProcessors by default just with empty values. So, I think, the solution here is the best one could have to check if payment is actually configured. Will merge this as is after little bit more testing and will clean up the flag and method mentioned above.
naz
changed the title
|
After having a closer look at this think your original solution was the best way to go about it. Thanks for the contribution @svenewers 🎉 |
refs #11499 - Removed unused and confusin isPaymentConfigured because it was basing it's logic on old `isPaid` flag. Having it in the codebase was adding confusion. - `isPaid` config flag still needs a proper cleanup with a migration etc. - Added little post PR merge cleanup
|
Thanks for PR @svenewers. Curious, is it possible to enable memberships without loading |
|
Because you can build subscriptions into any page, there's no current way to detect if it's required. Therefore it is always included. |
|
@ErisDS, thanks for follow-up. This is very unfortunate given it make it impossible to maintain a privacy-conscious website while monetizing paid content using memberships (which would be possible if one could use Stripe payment links or the Stripe API vs “Connect with Stripe” integration). I profoundly appreciate the work that has been put into Ghost (thankful for such a beautiful and significant open source endeavour), but I am uncomfortable with the fact Ghost can see one’s customers and subscriptions on Stripe due to “Connect with Stripe” integration.
As a privacy and security researcher and YouTuber, my audience would be very upset if I used such an integration. 😔 Also, given Stripe.js is considered a tracker (especially when advanced fraud detection is enabled), one has to get explicit user consent before loading I believe this is currently not possible… looks like I really wish one could use Ghost without worrying about above. |
|
@sunknudsen If you do not connect to Stripe, then Stripe will not be loaded - you can then set up your own payment system and sync with Ghost using the API - maybe something like this will help you https://ghost.org/integrations/paypal/ |
|
@allouis Thanks for sharing! I was hoping to enable free and paid tiers which is not possible using member “hack”. That said, given “hack” is as good as it gets currently, I will be releasing an open source project shortly called |
closes #11463
Ghost used to always load stripe.js into the frontend of all pages when memberships are enabled, even when Stripe isn't configured / memberships to a page are free.
This meant that some people's site were unnecessarily loading stripe.js, leading to unnecessarily long loading times & potentially even privacy issues (the stripe.js script sets cookies that might be problematic under GDPR, especially if the people running the site don't expect Stripe to be loaded because they haven't configured it).
This change changes Ghost's behaviour to only load stripe.js when both stripe API tokens are present & not empty (the quickets way to verify that Stripe is fully configured & operational on a blog).