Skip to content

Commit

Permalink
✨ Added option to disable free signups (#21862)
Browse files Browse the repository at this point in the history 
ref https://linear.app/ghost/issue/ENG-1235

- problem: today, when a publisher removes the "free" tier from the
Portal settings, it doesn't disable free signups entirely. It removes
the free tier from the Portal UI, but free signup is still possible via
other avenues (signup form in themes, embeddable signup form on another
site, direct API call). This creates confusion/frustration for
publishers who thought they disabled free signups, but are still getting
unwanted free signups (spam). There is no way to disable free signups
entirely.

- solution: introduced a new "paid-members only" subscription access
setting, which blocks all free signups at the API level. If chosen, the
free tier is hidden in Portal and all free signup are blocked at the API
level with a readable error message (`This site only accepts paid
members.`)

![CleanShot 2024-12-10 at 09 09
28@2x](https://github.com/user-attachments/assets/c71b38b4-0d23-429c-a743-00772e82c787)
  • Loading branch information
@sagzy
sagzy authored Dec 19, 2024
1 parent 9602c93 commit e67e241
Show file tree
Hide file tree
Showing 93 changed files with 657 additions and 234 deletions.
5 changes: 3 additions & 2 deletions apps/admin-x-design-system/src/global/form/Checkbox.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ export interface CheckboxProps {
key?: string;
checked?: boolean;
separator?: boolean;
testId?: string;
}

const Checkbox: React.FC<CheckboxProps> = ({title, label, value, onChange, disabled, error, hint, checked, separator}) => {
const Checkbox: React.FC<CheckboxProps> = ({title, label, value, onChange, disabled, error, hint, checked, separator, testId}) => {
const id = useId();

const handleCheckedChange = (isChecked: boolean | 'indeterminate') => {
Expand All @@ -29,7 +30,7 @@ const Checkbox: React.FC<CheckboxProps> = ({title, label, value, onChange, disab
<div className={`flex flex-col gap-1 ${separator && 'pb-2'}`}>
{title && <Heading grey={true} level={6}>{title}</Heading>}
<label className={`flex cursor-pointer items-start ${title && '-mb-1 mt-1'}`} htmlFor={id}>
<CheckboxPrimitive.Root className="mt-0.5 flex h-4 w-4 cursor-pointer appearance-none items-center justify-center rounded-[3px] border border-solid border-grey-500 bg-white outline-none data-[state=checked]:border-black data-[state=indeterminate]:border-black data-[state=checked]:bg-black data-[state=indeterminate]:bg-black" defaultChecked={checked} disabled={disabled} id={id} value={value} onCheckedChange={handleCheckedChange}>
<CheckboxPrimitive.Root className="mt-0.5 flex h-4 w-4 cursor-pointer appearance-none items-center justify-center rounded-[3px] border border-solid border-grey-500 bg-white outline-none data-[state=checked]:border-black data-[state=indeterminate]:border-black data-[state=checked]:bg-black data-[state=indeterminate]:bg-black" data-testid={testId} defaultChecked={checked} disabled={disabled} id={id} value={value} onCheckedChange={handleCheckedChange}>
<CheckboxPrimitive.Indicator>
<svg fill="none" height="11" viewBox="0 0 10 11" width="10">
<path d="M1 5.88889L4.6 9L9 1" stroke="white" strokeLinecap="round" strokeWidth="2"/>
Expand Down
13 changes: 13 additions & 0 deletions apps/admin-x-framework/src/test/acceptance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,19 @@ export async function chooseOptionInSelect(select: Locator, optionText: string |
await select.page().locator('[data-testid="select-option"]', {hasText: optionText}).click();
}

export async function getOptionsFromSelect(select: Locator): Promise<string[]> {
// Open the select dropdown
await select.click();

const options = await select.page().locator('[data-testid="select-option"]');
const optionTexts = await options.allTextContents();

// Close the select dropdown
await select.press('Escape');

return optionTexts;
}

export async function testUrlValidation(input: Locator, textToEnter: string, expectedResult: string, expectedError?: string) {
await input.fill(textToEnter);
await input.blur();
Expand Down
9 changes: 7 additions & 2 deletions apps/admin-x-settings/src/components/settings/membership/Access.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,15 @@ const MEMBERS_SIGNUP_ACCESS_OPTIONS = [
label: 'Anyone can sign up',
hint: 'All visitors will be able to subscribe and sign in'
},
{
value: 'paid',
label: 'Paid-members only',
hint: 'A paid Stripe subscription is required to sign up'
},
{
value: 'invite',
label: 'Only people I invite',
hint: 'People can sign in from your site but won\'t be able to sign up'
label: 'Invite-only',
hint: 'People can sign in but won\'t be able to sign up'
},
{
value: 'none',
Expand Down
18 changes: 9 additions & 9 deletions apps/admin-x-settings/src/components/settings/membership/portal/SignupOptions.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,21 +62,21 @@ const SignupOptions: React.FC<{
}
};

// This is a bit unclear in current admin, maybe we should add a message if the settings are disabled?
const isDisabled = membersSignupAccess !== 'all';

const isSignupAllowed = membersSignupAccess === 'all' || membersSignupAccess === 'paid';
const isFreeSignupAllowed = membersSignupAccess === 'all';
const isStripeEnabled = checkStripeEnabled(localSettings, config!);

let tiersCheckboxes: CheckboxProps[] = [];

if (localTiers) {
localTiers.forEach((tier) => {
if (tier.type === 'free') {
if (tier.type === 'free' && isFreeSignupAllowed) {
tiersCheckboxes.push({
checked: (portalPlans.includes('free')),
disabled: isDisabled,
disabled: !isSignupAllowed,
label: tier.name,
value: 'free',
testId: 'free-tier-checkbox',
onChange: (checked) => {
if (portalPlans.includes('free') && !checked) {
portalPlans.splice(portalPlans.indexOf('free'), 1);
Expand Down Expand Up @@ -119,7 +119,7 @@ const SignupOptions: React.FC<{
<Toggle
checked={Boolean(portalName)}
direction='rtl'
disabled={isDisabled}
disabled={!isSignupAllowed}
label='Display name in signup form'
onChange={e => updateSetting('portal_name', e.target.checked)}
/>
Expand All @@ -135,7 +135,7 @@ const SignupOptions: React.FC<{
checkboxes={[
{
checked: portalPlans.includes('monthly'),
disabled: isDisabled,
disabled: !isSignupAllowed,
label: 'Monthly',
value: 'monthly',
onChange: () => {
Expand All @@ -144,7 +144,7 @@ const SignupOptions: React.FC<{
},
{
checked: portalPlans.includes('yearly'),
disabled: isDisabled,
disabled: !isSignupAllowed,
label: 'Yearly',
value: 'yearly',
onChange: () => {
Expand Down Expand Up @@ -179,7 +179,7 @@ const SignupOptions: React.FC<{

{portalSignupTermsHtml?.toString() && <Toggle
checked={Boolean(portalSignupCheckboxRequired)}
disabled={isDisabled}
disabled={!isSignupAllowed}
label='Require agreement'
labelStyle='heading'
onChange={e => updateSetting('portal_signup_checkbox_required', e.target.checked)}
Expand Down
4 changes: 1 addition & 3 deletions apps/admin-x-settings/src/utils/getPortalPreviewUrl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ export const getPortalPreviewUrl = ({settings, config, tiers, siteData, selected
const portalBase = '/?v=modal-portal-settings#/portal/preview';

const portalPlans: string[] = JSON.parse(getSettingValue<string>(settings, 'portal_plans') || '');
const membersSignupAccess = getSettingValue<string>(settings, 'members_signup_access');
const allowSelfSignup = membersSignupAccess === 'all' && (!checkStripeEnabled(settings, config) || portalPlans.includes('free'));

const settingsParam = new URLSearchParams();
settingsParam.append('button', getSettingValue(settings, 'portal_button') ? 'true' : 'false');
Expand All @@ -36,7 +34,7 @@ export const getPortalPreviewUrl = ({settings, config, tiers, siteData, selected
settingsParam.append('buttonIcon', encodeURIComponent(getSettingValue(settings, 'portal_button_icon') || 'icon-1'));
settingsParam.append('signupButtonText', encodeURIComponent(getSettingValue(settings, 'portal_button_signup_text') || ''));
settingsParam.append('membersSignupAccess', getSettingValue(settings, 'members_signup_access') || 'all');
settingsParam.append('allowSelfSignup', allowSelfSignup ? 'true' : 'false');
settingsParam.append('allowSelfSignup', getSettingValue<string>(settings, 'allow_self_signup') || 'false');
settingsParam.append('signupTermsHtml', getSettingValue(settings, 'portal_signup_terms_html') || '');
settingsParam.append('signupCheckboxRequired', getSettingValue(settings, 'portal_signup_checkbox_required') ? 'true' : 'false');
settingsParam.append('portalProducts', portalTiers.join(',')); // assuming that it might be more than 1
Expand Down
22 changes: 17 additions & 5 deletions apps/admin-x-settings/test/acceptance/membership/access.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {chooseOptionInSelect, mockApi, responseFixtures, updatedSettingsResponse} from '@tryghost/admin-x-framework/test/acceptance';
import {chooseOptionInSelect, getOptionsFromSelect, mockApi, responseFixtures, updatedSettingsResponse} from '@tryghost/admin-x-framework/test/acceptance';
import {expect, test} from '@playwright/test';
import {globalDataRequests} from '../../utils/acceptance';

Expand All @@ -17,17 +17,29 @@ test.describe('Access settings', async () => {

const section = page.getByTestId('access');

// Check current selected values
await expect(section.getByText('Anyone can sign up')).toHaveCount(1);
await expect(section.getByText('Public')).toHaveCount(1);
await expect(section.getByText('Nobody')).toHaveCount(1);

await chooseOptionInSelect(section.getByTestId('subscription-access-select'), 'Only people I invite');
await chooseOptionInSelect(section.getByTestId('default-post-access-select'), /^Members only$/);
await chooseOptionInSelect(section.getByTestId('commenting-select'), 'All members');
const subscriptionAccessSelect = section.getByTestId('subscription-access-select');
const defaultPostAccessSelect = section.getByTestId('default-post-access-select');
const commentingSelect = section.getByTestId('commenting-select');

// Check available options
await expect(getOptionsFromSelect(subscriptionAccessSelect)).resolves.toEqual(['Anyone can sign up', 'Paid-members only', 'Invite-only', 'Nobody']);
await expect(getOptionsFromSelect(defaultPostAccessSelect)).resolves.toEqual(['Public', 'Members only', 'Paid-members only', 'Specific tiers']);
await expect(getOptionsFromSelect(commentingSelect)).resolves.toEqual(['All members', 'Paid-members only', 'Nobody']);

// Edit access settings to new values
await chooseOptionInSelect(subscriptionAccessSelect, 'Invite-only');
await chooseOptionInSelect(defaultPostAccessSelect, /^Members only$/);
await chooseOptionInSelect(commentingSelect, 'All members');

await section.getByRole('button', {name: 'Save'}).click();

await expect(section.getByText('Only people I invite')).toHaveCount(1);
// Check that the new values are now displayed
await expect(section.getByText('Invite-only')).toHaveCount(1);
await expect(section.getByText('Members only')).toHaveCount(1);
await expect(section.getByText('All members')).toHaveCount(1);

Expand Down
44 changes: 40 additions & 4 deletions apps/admin-x-settings/test/acceptance/membership/portal.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {expect, test} from '@playwright/test';
import {globalDataRequests} from '../../utils/acceptance';
import {mockApi, mockSitePreview, responseFixtures} from '@tryghost/admin-x-framework/test/acceptance';
import {mockApi, mockSitePreview, responseFixtures, updatedSettingsResponse} from '@tryghost/admin-x-framework/test/acceptance';

test.describe('Portal Settings', async () => {
test('Loads Portal Preview Modal', async ({page}) => {
Expand All @@ -22,7 +22,7 @@ test.describe('Portal Settings', async () => {
await page.waitForSelector('[data-testid="portal-modal"]');
});

test('can toggle portal signup options', async ({page}) => {
test('can set portal signup options', async ({page}) => {
const {lastApiRequests} = await mockApi({page, requests: {
...globalDataRequests,
tiers: {method: 'GET', path: '/tiers/', response: responseFixtures.tiers},
Expand All @@ -44,8 +44,16 @@ test.describe('Portal Settings', async () => {

const modal = await page.getByTestId('portal-modal');

await modal.getByRole('switch').click();
await modal.getByRole('checkbox').click();
const displayNameToggle = await modal.getByLabel('Display name in signup form');
expect(displayNameToggle).toBeVisible();
expect(displayNameToggle).toBeChecked();

const freeTierCheckbox = await modal.getByTestId('free-tier-checkbox');
expect(freeTierCheckbox).toBeVisible();
expect(freeTierCheckbox).toBeChecked();

await freeTierCheckbox.click();
await displayNameToggle.click();
await modal.getByRole('button', {name: 'Save'}).click();

expect(lastApiRequests.editTiers?.body).toMatchObject({
Expand All @@ -56,6 +64,34 @@ test.describe('Portal Settings', async () => {
});
});

test('free tier is hidden from portal signup options if the site is set to paid-members only', async ({page}) => {
await mockApi({page, requests: {
...globalDataRequests,
// Set site to paid-members only
browseSettings: {...globalDataRequests.browseSettings, response: updatedSettingsResponse([
{key: 'members_signup_access', value: 'paid'}
])},
// Free tier is available in the tiers list
browseTiers: {method: 'GET', path: '/tiers/', response: responseFixtures.tiers}
}});

await mockSitePreview({
page,
url: 'http://localhost:2368/?v=modal-portal-settings#/portal/preview?button=true&name=true&isFree=true&isMonthly=true&isYearly=true&page=signup&buttonIcon=icon-1&signupButtonText=Subscribe&membersSignupAccess=all&allowSelfSignup=true&signupTermsHtml=&signupCheckboxRequired=false&portalProducts=6511005e14c14a231e49af15&portalPrices=free%252Cmonthly%252Cyearly&accentColor=%2523FF1A75&buttonStyle=icon-and-text&disableBackground=false',
response: '<html><head><style></style></head><body><div>PortalPreview</div></body></html>'
});

await page.goto('/');
const section = await page.getByTestId('portal');
await section.getByRole('button', {name: 'Customize'}).click();
await page.waitForSelector('[data-testid="portal-modal"]');
const modal = await page.getByTestId('portal-modal');

// In Portal settings, the free tier is hidden because the site is set to paid-members only, even if available in the tiers list
const freeTierCheckbox = await modal.getByTestId('free-tier-checkbox');
expect(freeTierCheckbox).not.toBeVisible();
});

test('can toggle portal Look & Feel options', async ({page}) => {
const {lastApiRequests} = await mockApi({page, requests: {
...globalDataRequests,
Expand Down
4 changes: 2 additions & 2 deletions apps/portal/src/App.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {transformPortalAnchorToRelative} from './utils/transform-portal-anchor-t
import {getActivePage, isAccountPage, isOfferPage} from './pages';
import ActionHandler from './actions';
import './App.css';
import {hasRecommendations, allowCompMemberUpgrade, createPopupNotification, getCurrencySymbol, getFirstpromoterId, getPriceIdFromPageQuery, getProductCadenceFromPrice, getProductFromId, getQueryPrice, getSiteDomain, isActiveOffer, isComplimentaryMember, isInviteOnlySite, isPaidMember, isRecentMember, isSentryEventAllowed, removePortalLinkFromUrl} from './utils/helpers';
import {hasRecommendations, allowCompMemberUpgrade, createPopupNotification, hasAvailablePrices, getCurrencySymbol, getFirstpromoterId, getPriceIdFromPageQuery, getProductCadenceFromPrice, getProductFromId, getQueryPrice, getSiteDomain, isActiveOffer, isComplimentaryMember, isInviteOnly, isPaidMember, isRecentMember, isSentryEventAllowed, removePortalLinkFromUrl} from './utils/helpers';
import {handleDataAttributes} from './data-attributes';

import i18nLib from '@tryghost/i18n';
Expand Down Expand Up @@ -927,7 +927,7 @@ export default class App extends React.Component {
getContextPage({site, page, member}) {
/**Set default page based on logged-in status */
if (!page || page === 'default') {
const loggedOutPage = isInviteOnlySite({site}) ? 'signin' : 'signup';
const loggedOutPage = isInviteOnly({site}) || !hasAvailablePrices({site}) ? 'signin' : 'signup';
page = member ? 'accountHome' : loggedOutPage;
}

Expand Down
6 changes: 3 additions & 3 deletions apps/portal/src/components/PopupModal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {getFrameStyles} from './Frame.styles';
import Pages, {getActivePage} from '../pages';
import PopupNotification from './common/PopupNotification';
import PoweredBy from './common/PoweredBy';
import {getSiteProducts, isInviteOnlySite, isCookiesDisabled, hasFreeProductPrice} from '../utils/helpers';
import {getSiteProducts, hasAvailablePrices, isInviteOnly, isCookiesDisabled, hasFreeProductPrice} from '../utils/helpers';

const StylesWrapper = () => {
return {
Expand Down Expand Up @@ -142,7 +142,7 @@ class PopupContent extends React.Component {

render() {
const {page, pageQuery, site, customSiteUrl} = this.context;
const products = getSiteProducts({site});
const products = getSiteProducts({site, pageQuery});
const noOfProducts = products.length;

getActivePage({page});
Expand Down Expand Up @@ -177,7 +177,7 @@ class PopupContent extends React.Component {
break;
}

if (noOfProducts > 1 && !isInviteOnlySite({site, pageQuery})) {
if (noOfProducts > 1 && !isInviteOnly({site}) && hasAvailablePrices({site, pageQuery})) {
if (page === 'signup') {
pageClass += ' full-size';
popupSize = 'full';
Expand Down
4 changes: 2 additions & 2 deletions apps/portal/src/components/TriggerButton.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {ReactComponent as ButtonIcon3} from '../images/icons/button-icon-3.svg';
import {ReactComponent as ButtonIcon4} from '../images/icons/button-icon-4.svg';
import {ReactComponent as ButtonIcon5} from '../images/icons/button-icon-5.svg';
import TriggerButtonStyle from './TriggerButton.styles';
import {isInviteOnlySite, isSigninAllowed} from '../utils/helpers';
import {hasAvailablePrices, isInviteOnly, isSigninAllowed} from '../utils/helpers';
import {hasMode} from '../utils/check-mode';

const ICON_MAPPING = {
Expand Down Expand Up @@ -176,7 +176,7 @@ class TriggerButtonContent extends React.Component {
}

if (isSigninAllowed({site})) {
const page = isInviteOnlySite({site}) ? 'signin' : 'signup';
const page = isInviteOnly({site}) || !hasAvailablePrices({site}) ? 'signin' : 'signup';
this.context.onAction('openPopup', {page});
return;
}
Expand Down
4 changes: 2 additions & 2 deletions apps/portal/src/components/pages/AccountHomePage/components/SubscribeButton.js
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import AppContext from '../../../../AppContext';
import ActionButton from '../../../common/ActionButton';
import {isSignupAllowed} from '../../../../utils/helpers';
import {isSignupAllowed, hasAvailablePrices} from '../../../../utils/helpers';
import {useContext} from 'react';

const SubscribeButton = () => {
const {site, action, brandColor, onAction, t} = useContext(AppContext);

if (!isSignupAllowed({site})) {
if (!isSignupAllowed({site}) || !hasAvailablePrices({site})) {
return null;
}
const isRunning = ['checkoutPlan:running'].includes(action);
Expand Down
Loading

0 comments on commit e67e241

Please sign in to comment.