🐛 Fixed unreliable paid members enabled checks (#2405)

refs https://github.com/TryGhost/Team/issues/1650

- Some places only checked for Stripe being connected via the 'connect' method and ignored the 'direct' method
- Updated (where possible) admin to use the new calculated `paid_members_enabled` setting
This commit is contained in:
Simon Backx 2022-05-24 16:53:03 +02:00 committed by GitHub
parent 94c192041d
commit bc1aa493fa
24 changed files with 61 additions and 63 deletions

View File

@ -1212,3 +1212,5 @@ remove|ember-template-lint|no-action|118|28|118|28|0eba6cead2056aa8d89e186bd0318
remove|ember-template-lint|no-action|135|19|135|19|2e591e0b5aa8f903afda1d0f2ffd2183812e6c2f|1652054400000|1662422400000|1665014400000|app/templates/editor.hbs remove|ember-template-lint|no-action|135|19|135|19|2e591e0b5aa8f903afda1d0f2ffd2183812e6c2f|1652054400000|1662422400000|1665014400000|app/templates/editor.hbs
remove|ember-template-lint|no-action|146|19|146|19|c90e6d7f98e739c3a6472176ceeecc896913f2f6|1652054400000|1662422400000|1665014400000|app/templates/editor.hbs remove|ember-template-lint|no-action|146|19|146|19|c90e6d7f98e739c3a6472176ceeecc896913f2f6|1652054400000|1662422400000|1665014400000|app/templates/editor.hbs
remove|ember-template-lint|no-unused-block-params|1|0|1|0|df27de023763e49bb0cde497e82991bbfc16b1b1|1652054400000|1662422400000|1665014400000|lib/koenig-editor/addon/components/koenig-card-audio.hbs remove|ember-template-lint|no-unused-block-params|1|0|1|0|df27de023763e49bb0cde497e82991bbfc16b1b1|1652054400000|1662422400000|1665014400000|lib/koenig-editor/addon/components/koenig-card-audio.hbs
add|ember-template-lint|no-action|20|150|20|150|d6149e8bd18677704c261b7d3e9afeaf8be9f6a6|1653350400000|1663718400000|1668906000000|app/components/modal-portal-settings.hbs
remove|ember-template-lint|no-action|20|150|20|150|d07a2c2968b7c337172eb3d189fcd004f05034d7|1652054400000|1662422400000|1665014400000|app/components/modal-portal-settings.hbs

View File

@ -121,7 +121,7 @@
id="monthly-plan" id="monthly-plan"
name="monthly-plan" name="monthly-plan"
checked={{this.isMonthlyChecked}} checked={{this.isMonthlyChecked}}
disabled={{this.isPaidPriceDisabled}} disabled={{not this.membersUtils.isStripeEnabled}}
class="gh-input post-settings-featured" class="gh-input post-settings-featured"
{{on "click" this.toggleMonthlyPlan}} {{on "click" this.toggleMonthlyPlan}}
data-test-checkbox="featured" data-test-checkbox="featured"
@ -140,7 +140,7 @@
id="yearly-plan" id="yearly-plan"
name="yearly-plan" name="yearly-plan"
checked={{this.isYearlyChecked}} checked={{this.isYearlyChecked}}
disabled={{this.isPaidPriceDisabled}} disabled={{not this.membersUtils.isStripeEnabled}}
class="gh-input post-settings-featured" class="gh-input post-settings-featured"
{{on "click" this.toggleYearlyPlan}} {{on "click" this.toggleYearlyPlan}}
data-test-checkbox="featured" data-test-checkbox="featured"
@ -166,4 +166,4 @@
<span>{{if this.isHidden "Continue" "Save and continue"}}{{svg-jar "arrow-right-tail"}}</span> <span>{{if this.isHidden "Continue" "Save and continue"}}{{svg-jar "arrow-right-tail"}}</span>
</GhTaskButton> </GhTaskButton>
</div> </div>
</div> </div>

View File

@ -46,10 +46,6 @@ export default class GhLaunchWizardSetPricingComponent extends Component {
return envConfig.environment !== 'development' && !/^https:/.test(siteUrl); return envConfig.environment !== 'development' && !/^https:/.test(siteUrl);
} }
get isPaidPriceDisabled() {
return !this.membersUtils.isStripeEnabled;
}
get isFreeDisabled() { get isFreeDisabled() {
return this.settings.get('membersSignupAccess') !== 'all'; return this.settings.get('membersSignupAccess') !== 'all';
} }

View File

@ -100,7 +100,7 @@
/> />
{{/if}} {{/if}}
{{#if this.isStripeConnected}} {{#if this.membersUtils.paidMembersEnabled}}
<h4 class="gh-main-section-header small bn">Subscriptions</h4> <h4 class="gh-main-section-header small bn">Subscriptions</h4>
{{#unless this.tiers}} {{#unless this.tiers}}

View File

@ -24,12 +24,8 @@ export default class extends Component {
@tracked tiersList; @tracked tiersList;
@tracked newslettersList; @tracked newslettersList;
get canShowStripeInfo() {
return !this.member.get('isNew') && this.membersUtils.isStripeEnabled;
}
get isAddComplimentaryAllowed() { get isAddComplimentaryAllowed() {
if (!this.membersUtils.isStripeEnabled) { if (!this.membersUtils.paidMembersEnabled) {
return false; return false;
} }
@ -115,10 +111,6 @@ export default class extends Component {
return null; return null;
} }
get isStripeConnected() {
return this.settings.get('stripeConnectAccountId');
}
@action @action
updateNewsletterPreference(event) { updateNewsletterPreference(event) {
if (!event.target.checked) { if (!event.target.checked) {

View File

@ -131,4 +131,4 @@
@confirm={{action "disconnectStripeConnectIntegration"}} @confirm={{action "disconnectStripeConnectIntegration"}}
@close={{action "closeDisconnectStripeModal"}} @close={{action "closeDisconnectStripeModal"}}
@modifier="action wide" /> @modifier="action wide" />
{{/if}} {{/if}}

View File

@ -13,6 +13,7 @@ export default Component.extend({
ghostPaths: service(), ghostPaths: service(),
ajax: service(), ajax: service(),
settings: service(), settings: service(),
membersUtils: service(),
store: service(), store: service(),
topCurrencies: null, topCurrencies: null,
@ -22,11 +23,11 @@ export default Component.extend({
_scratchStripeYearlyAmount: null, _scratchStripeYearlyAmount: null,
_scratchStripeMonthlyAmount: null, _scratchStripeMonthlyAmount: null,
stripeDirect: false,
// passed in actions // passed in actions
setStripeConnectIntegrationTokenSetting() {}, setStripeConnectIntegrationTokenSetting() {},
stripeDirect: reads('config.stripeDirect'),
/** OLD **/ /** OLD **/
stripeDirectPublicKey: reads('settings.stripePublishableKey'), stripeDirectPublicKey: reads('settings.stripePublishableKey'),
stripeDirectSecretKey: reads('settings.stripeSecretKey'), stripeDirectSecretKey: reads('settings.stripeSecretKey'),
@ -59,6 +60,9 @@ export default Component.extend({
init() { init() {
this._super(...arguments); this._super(...arguments);
// Allow disabling stripe direct keys if stripe is still enabled, while the config is disabled
this.updateStripeDirect();
const noOfTopCurrencies = 5; const noOfTopCurrencies = 5;
this.set('topCurrencies', currencies.slice(0, noOfTopCurrencies).map((currency) => { this.set('topCurrencies', currencies.slice(0, noOfTopCurrencies).map((currency) => {
return { return {
@ -86,12 +90,6 @@ export default Component.extend({
options: this.currencies options: this.currencies
} }
]); ]);
if (this.stripeConnectAccountId) {
this.set('membersStripeOpen', false);
} else {
this.set('membersStripeOpen', true);
}
}, },
actions: { actions: {
@ -154,13 +152,14 @@ export default Component.extend({
disconnectStripeConnectIntegration() { disconnectStripeConnectIntegration() {
this.disconnectStripeConnectIntegration.perform(); this.disconnectStripeConnectIntegration.perform();
},
openStripeSettings() {
this.set('membersStripeOpen', true);
} }
}, },
updateStripeDirect() {
// Allow disabling stripe direct keys if stripe is still enabled, while the config is disabled
this.set('stripeDirect', this.get('config.stripeDirect') || (this.get('membersUtils.isStripeEnabled') && !this.get('settings.stripeConnectAccountId')));
},
validateStripePlans() { validateStripePlans() {
this.get('settings.errors').remove('stripePlans'); this.get('settings.errors').remove('stripePlans');
this.get('settings.hasValidated').removeObject('stripePlans'); this.get('settings.hasValidated').removeObject('stripePlans');
@ -274,7 +273,6 @@ export default Component.extend({
response = yield this.settings.save(); response = yield this.settings.save();
this.set('membersStripeOpen', false);
this.set('stripeConnectSuccess', true); this.set('stripeConnectSuccess', true);
this.onConnected?.(); this.onConnected?.();
@ -292,7 +290,9 @@ export default Component.extend({
}).drop(), }).drop(),
saveSettings: task(function* () { saveSettings: task(function* () {
return yield this.settings.save(); const s = yield this.settings.save();
this.updateStripeDirect();
return s;
}).drop(), }).drop(),
get liveStripeConnectAuthUrl() { get liveStripeConnectAuthUrl() {

View File

@ -106,7 +106,7 @@
{{/if}} {{/if}}
</li> </li>
{{#if (and this.isStripeConnected (not-eq this.settings.membersSignupAccess "none"))}} {{#if (and this.settings.paidMembersEnabled)}}
<li> <li>
<LinkTo @route="offers" @alt="Offers">{{svg-jar "percentage"}}Offers</LinkTo> <LinkTo @route="offers" @alt="Offers">{{svg-jar "percentage"}}Offers</LinkTo>
</li> </li>

View File

@ -54,9 +54,6 @@ export default class Main extends Component.extend(ShortcutsMixin) {
@reads('config.hostSettings.billing.enabled') @reads('config.hostSettings.billing.enabled')
showBilling; showBilling;
@reads('settings.stripeConnectAccountId')
isStripeConnected;
init() { init() {
super.init(...arguments); super.init(...arguments);

View File

@ -167,7 +167,7 @@ export default class MembersFilter extends Component {
}); });
// exclude subscription filters if Stripe isn't connected // exclude subscription filters if Stripe isn't connected
if (!this.settings.get('stripeConnectAccountId')) { if (!this.settings.get('paidMembersEnabled')) {
availableFilters = availableFilters.reject(prop => prop.group === 'Subscription'); availableFilters = availableFilters.reject(prop => prop.group === 'Subscription');
} }

View File

@ -17,7 +17,7 @@
{{#liquid-if isOpen}} {{#liquid-if isOpen}}
<div class="modal-fullsettings-tab-expanded {{if (not-eq this.settings.membersSignupAccess "all") "disabled-overlay"}}" onclick={{action "switchPreviewPage" "signup"}}> <div class="modal-fullsettings-tab-expanded {{if (not-eq this.settings.membersSignupAccess "all") "disabled-overlay"}}" onclick={{action "switchPreviewPage" "signup"}}>
{{#unless this.membersUtils.isStripeEnabled}} {{#unless this.membersUtils.isStripeEnabled}}
<button class="gh-btn gh-btn-link {{unless this.session.user.isAdmin "disabled"}}" type="button" {{on "click" (action "openStripeSettings")}}>Connect to Stripe</button> <button class="gh-btn gh-btn-link {{unless this.session.user.isAdmin "disabled"}}" type="button" {{on "click" (action "openStripeConnect")}}>Connect to Stripe</button>
{{/unless}} {{/unless}}
<GhFormGroup @classNames="gh-members-subscribed-checkbox gh-portal-setting-first mb0"> <GhFormGroup @classNames="gh-members-subscribed-checkbox gh-portal-setting-first mb0">
<div class="flex justify-between items-center"> <div class="flex justify-between items-center">

View File

@ -86,13 +86,13 @@ export default ModalComponent.extend({
const allowedPlans = this.settings.get('portalPlans') || []; const allowedPlans = this.settings.get('portalPlans') || [];
return (this.settings.get('membersSignupAccess') === 'all' && allowedPlans.includes('free')); return (this.settings.get('membersSignupAccess') === 'all' && allowedPlans.includes('free'));
}), }),
isMonthlyChecked: computed('settings.portalPlans.[]', 'isStripeConfigured', function () { isMonthlyChecked: computed('settings.portalPlans.[]', 'membersUtils.paidMembersEnabled', function () {
const allowedPlans = this.settings.get('portalPlans') || []; const allowedPlans = this.settings.get('portalPlans') || [];
return (this.membersUtils.isStripeEnabled && allowedPlans.includes('monthly')); return (this.membersUtils.paidMembersEnabled && allowedPlans.includes('monthly'));
}), }),
isYearlyChecked: computed('settings.portalPlans.[]', 'isStripeConfigured', function () { isYearlyChecked: computed('settings.portalPlans.[]', 'membersUtils.paidMembersEnabled', function () {
const allowedPlans = this.settings.get('portalPlans') || []; const allowedPlans = this.settings.get('portalPlans') || [];
return (this.membersUtils.isStripeEnabled && allowedPlans.includes('yearly')); return (this.membersUtils.paidMembersEnabled && allowedPlans.includes('yearly'));
}), }),
tiers: computed('model.tiers.[]', 'changedTiers.[]', 'isPreloading', function () { tiers: computed('model.tiers.[]', 'changedTiers.[]', 'isPreloading', function () {
const paidTiers = this.model.tiers?.filter(tier => tier.type === 'paid' && tier.active === true); const paidTiers = this.model.tiers?.filter(tier => tier.type === 'paid' && tier.active === true);
@ -243,9 +243,9 @@ export default ModalComponent.extend({
this.set('showLeaveSettingsModal', false); this.set('showLeaveSettingsModal', false);
}, },
openStripeSettings() { openStripeConnect() {
this.isWaitingForStripeConnection = true; this.isWaitingForStripeConnection = true;
this.model.openStripeSettings(); this.model.openStripeConnect();
}, },
leaveSettings() { leaveSettings() {

View File

@ -16,7 +16,7 @@
</form> </form>
<div class="modal-footer"> <div class="modal-footer">
{{#if this.settings.stripeConnectAccountId}} {{#if (and this.membersUtils.isStripeEnabled this.settings.stripeConnectAccountId)}}
<button <button
class="gh-btn gh-btn-black" data-test-button="stripe-connect-ok" type="button" {{action "confirm"}} class="gh-btn gh-btn-black" data-test-button="stripe-connect-ok" type="button" {{action "confirm"}}
{{action (optional this.noop) on="mouseDown"}} {{action (optional this.noop) on="mouseDown"}}

View File

@ -7,6 +7,7 @@ import {inject as service} from '@ember/service';
@classic @classic
export default class ModalStripeConnect extends ModalBase { export default class ModalStripeConnect extends ModalBase {
@service settings; @service settings;
@service membersUtils;
@action @action
setStripeConnectIntegrationTokenSetting(stripeConnectIntegrationToken) { setStripeConnectIntegrationTokenSetting(stripeConnectIntegrationToken) {
@ -34,7 +35,8 @@ export default class ModalStripeConnect extends ModalBase {
@action @action
updateSuccessModifier() { updateSuccessModifier() {
if (this.settings.get('stripeConnectAccountId')) { // Note, we should also check isStripeEnabled because stripeDirect option might be enabled
if (this.membersUtils.get('isStripeEnabled') && this.settings.get('stripeConnectAccountId')) {
if (this.modifier?.indexOf('stripe-connected') === -1) { if (this.modifier?.indexOf('stripe-connected') === -1) {
this.updateModifier(`${this.modifier} stripe-connected`); this.updateModifier(`${this.modifier} stripe-connected`);
} }

View File

@ -11,11 +11,6 @@ export default class SettingsController extends Controller {
showLeaveSettingsModal = false; showLeaveSettingsModal = false;
@action
openStripeSettings() {
this.set('membersStripeOpen', true);
}
@action @action
closeLeaveSettingsModal() { closeLeaveSettingsModal() {
this.set('showLeaveSettingsModal', false); this.set('showLeaveSettingsModal', false);

View File

@ -375,8 +375,8 @@ export default class MembersAccessController extends Controller {
} }
async saveTier() { async saveTier() {
const isStripeConnected = this.settings.get('stripeConnectAccountId'); const paidMembersEnabled = this.settings.get('paidMembersEnabled');
if (this.tier && isStripeConnected) { if (this.tier && paidMembersEnabled) {
const monthlyAmount = Math.round(this.stripeMonthlyAmount * 100); const monthlyAmount = Math.round(this.stripeMonthlyAmount * 100);
const yearlyAmount = Math.round(this.stripeYearlyAmount * 100); const yearlyAmount = Math.round(this.stripeYearlyAmount * 100);

View File

@ -70,6 +70,9 @@ export default Model.extend(ValidationEngine, {
stripeConnectDisplayName: attr('string'), stripeConnectDisplayName: attr('string'),
stripeConnectAccountId: attr('string'), stripeConnectAccountId: attr('string'),
membersEnabled: attr('boolean'),
paidMembersEnabled: attr('boolean'),
/** /**
* Editor settings * Editor settings
*/ */

View File

@ -296,11 +296,11 @@ export default class DashboardStatsService extends Service {
return; return;
} }
if (this.membersUtils.isStripeEnabled) { if (this.membersUtils.paidMembersEnabled) {
yield this.loadPaidTiers(); yield this.loadPaidTiers();
} }
const hasPaidTiers = this.membersUtils.isStripeEnabled && this.activePaidTiers && this.activePaidTiers.length > 0; const hasPaidTiers = this.membersUtils.paidMembersEnabled && this.activePaidTiers && this.activePaidTiers.length > 0;
this.siteStatus = { this.siteStatus = {
hasPaidTiers, hasPaidTiers,

View File

@ -7,9 +7,16 @@ export default class MembersUtilsService extends Service {
@service store; @service store;
get isMembersEnabled() { get isMembersEnabled() {
return this.settings.get('membersSignupAccess') !== 'none'; return this.settings.get('membersEnabled');
} }
get paidMembersEnabled() {
return this.settings.get('paidMembersEnabled');
}
/**
* Note: always use paidMembersEnabled! Only use this getter for the Stripe Connection UI.
*/
get isStripeEnabled() { get isStripeEnabled() {
const stripeDirect = this.config.get('stripeDirect'); const stripeDirect = this.config.get('stripeDirect');

View File

@ -72,9 +72,9 @@
<div class="gh-settings-members-tiersheader"> <div class="gh-settings-members-tiersheader">
<h4 class="gh-main-section-header small bn">Membership tiers</h4> <h4 class="gh-main-section-header small bn">Membership tiers</h4>
{{#if this.session.user.isAdmin}} {{#if this.session.user.isAdmin}}
<button type="button" class="gh-btn gh-btn-outline gh-btn-stripe-status {{if this.isConnectDisallowed "disabled"}} {{if this.settings.stripeConnectAccountId "connected" ""}}" {{on "click" this.openStripeConnect}}> <button type="button" class="gh-btn gh-btn-outline gh-btn-stripe-status {{if this.isConnectDisallowed "disabled"}} {{if this.membersUtils.isStripeEnabled "connected" ""}}" {{on "click" this.openStripeConnect}}>
<span> <span>
{{if this.settings.stripeConnectAccountId "Connected to Stripe" "Stripe not connected"}} {{if this.membersUtils.isStripeEnabled "Connected to Stripe" "Stripe not connected"}}
</span> </span>
</button> </button>
{{/if}} {{/if}}
@ -122,7 +122,7 @@
<p class="gh-expandable-description">Set prices and paid member sign up settings</p> <p class="gh-expandable-description">Set prices and paid member sign up settings</p>
</div> </div>
{{#if this.settings.stripeConnectAccountId}} {{#if this.membersUtils.isStripeEnabled}}
<button type="button" class="gh-btn" {{on "click" (toggle "paidOpen" this)}} data-test-toggle-pub-info><span>{{if this.paidOpen "Close" "Expand"}}</span></button> <button type="button" class="gh-btn" {{on "click" (toggle "paidOpen" this)}} data-test-toggle-pub-info><span>{{if this.paidOpen "Close" "Expand"}}</span></button>
{{else}} {{else}}
<button type="button" class="stripe-connect {{if (or (not this.session.user.isAdmin) this.isConnectDisallowed) "disabled"}}" {{on "click" this.openStripeConnect}}> <button type="button" class="stripe-connect {{if (or (not this.session.user.isAdmin) this.isConnectDisallowed) "disabled"}}" {{on "click" this.openStripeConnect}}>
@ -172,7 +172,7 @@
<GhFullscreenModal @modal="portal-settings" <GhFullscreenModal @modal="portal-settings"
@model={{hash @model={{hash
preloadTask=this.saveSettingsTask preloadTask=this.saveSettingsTask
openStripeSettings=this.openStripeConnect openStripeConnect=this.openStripeConnect
tiers=this.tiers tiers=this.tiers
}} }}
@close={{this.closePortalSettings}} @close={{this.closePortalSettings}}

View File

@ -58,7 +58,7 @@ export default class KoenigCardButtonComponent extends Component {
url: this.config.getSiteUrl('/#/portal/signup/free') url: this.config.getSiteUrl('/#/portal/signup/free')
}]); }]);
if (this.membersUtils.isStripeEnabled) { if (this.membersUtils.paidMembersEnabled) {
urls.push(...[{ urls.push(...[{
name: 'Paid signup', name: 'Paid signup',
url: this.config.getSiteUrl('/#/portal/signup') url: this.config.getSiteUrl('/#/portal/signup')

View File

@ -83,7 +83,7 @@ export default class KoenigCardEmailCtaComponent extends Component {
url: this.config.getSiteUrl('/#/portal/signup/free') url: this.config.getSiteUrl('/#/portal/signup/free')
}]); }]);
if (this.membersUtils.isStripeEnabled) { if (this.membersUtils.paidMembersEnabled) {
urls.push(...[{ urls.push(...[{
name: 'Paid signup', name: 'Paid signup',
url: this.config.getSiteUrl('/#/portal/signup') url: this.config.getSiteUrl('/#/portal/signup')

View File

@ -1281,7 +1281,7 @@ describe('Acceptance: Members filtering', function () {
it('hides paid filters when stripe isn\'t connected', async function () { it('hides paid filters when stripe isn\'t connected', async function () {
// disconnect stripe // disconnect stripe
this.server.db.settings.update({key: 'stripe_connect_account_id'}, {value: null}); this.server.db.settings.update({key: 'paid_members_enabled'}, {value: false});
this.server.createList('member', 10); this.server.createList('member', 10);
await visit('/members'); await visit('/members');

View File

@ -9,4 +9,8 @@ export function enableStripe(server) {
server.db.settings.find({key: 'stripe_connect_publishable_key'}) server.db.settings.find({key: 'stripe_connect_publishable_key'})
? server.db.settings.update({key: 'stripe_connect_publishable_key'}, {value: 'stripe_secret_key'}) ? server.db.settings.update({key: 'stripe_connect_publishable_key'}, {value: 'stripe_secret_key'})
: server.create('setting', {key: 'stripe_connect_publishable_key', value: 'stripe_secret_key', group: 'members'}); : server.create('setting', {key: 'stripe_connect_publishable_key', value: 'stripe_secret_key', group: 'members'});
server.db.settings.find({key: 'paid_members_enabled'})
? server.db.settings.update({key: 'paid_members_enabled'}, {value: true})
: server.create('setting', {key: 'paid_members_enabled', value: true, group: 'members'});
} }