Skip to content

Commit

Permalink
Added ability to disable Newsletter clicks toggle in settings (#22090)
Browse files Browse the repository at this point in the history
ref
https://linear.app/ghost/issue/ENG-1974/create-config-option-to-forcibly-disable-email-track-clicks

- With the ability to override a setting via configuration, we also need
to disable the setting's toggle in the UI to signal to the user that it
cannot be changed.
- This commit sets `disabled: true` on the `Newsletter clicks` toggle if
`is_read_only` is set to `true` on the `email_track_clicks` setting
returned from the API, and establishes a pattern that we can use for
other settings in the future, if desired.
  • Loading branch information
cmraible authored Jan 31, 2025
1 parent 3ee9f43 commit 376b920
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 3 deletions.
9 changes: 9 additions & 0 deletions apps/admin-x-framework/src/api/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export type SettingValue = string | boolean | null;
export type Setting = {
key: string;
value: SettingValue;
is_read_only?: boolean;
}

export type SettingsResponseMeta = Meta & {
Expand Down Expand Up @@ -82,6 +83,14 @@ export function getSettingValue<ValueType = SettingValue>(settings: Setting[] |
return setting?.value as ValueType || null;
}

export function isSettingReadOnly(settings: Setting[] | null | undefined, key: string): boolean | undefined {
if (!settings) {
return undefined;
}
const setting = settings.find(d => d.key === key);
return setting?.is_read_only || false;
}

export function checkStripeEnabled(settings: Setting[], config: Config) {
const hasSetting = (key: string) => settings.some(setting => setting.key === key && setting.value);

Expand Down
8 changes: 6 additions & 2 deletions apps/admin-x-framework/src/test/acceptance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,17 @@ export async function mockApi<Requests extends Record<string, MockRequestConfig>
return {lastApiRequests};
}

export function updatedSettingsResponse(newSettings: Array<{ key: string, value: string | boolean | null }>) {
export function updatedSettingsResponse(newSettings: Array<{ key: string, value: string | boolean | null, is_read_only?: boolean }>) {
return {
...responseFixtures.settings,
settings: responseFixtures.settings.settings.map((setting) => {
const newSetting = newSettings.find(({key}) => key === setting.key);

return {key: setting.key, value: newSetting?.value || setting.value};
return {
key: setting.key,
value: newSetting?.value !== undefined ? newSetting.value : setting.value,
...(newSetting?.is_read_only ? {is_read_only: true} : {})
};
})
};
}
Expand Down
59 changes: 59 additions & 0 deletions apps/admin-x-framework/test/unit/utils/api/settings.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import {getSettingValue, getSettingValues, isSettingReadOnly} from '../../../../src/api/settings';

describe('settings utils', function () {
describe('getSettingValue', function () {
it('returns the value of a setting', function () {
const settings = [
{key: 'test_key', value: 'test_value'}
];
const value = getSettingValue(settings, 'test_key');
expect(value).toEqual('test_value');
});

it('returns null if settings is null', function () {
const settings = undefined;
const value = getSettingValue(settings, 'test_key');
expect(value).toEqual(null);
});
});

describe('getSettingValues', function () {
it('returns the values of multiple settings', function () {
const settings = [
{key: 'test_key', value: 'test_value'},
{key: 'test_key_2', value: 'test_value_2'}
];
const values = getSettingValues(settings, ['test_key', 'test_key_2']);
expect(values).toEqual(['test_value', 'test_value_2']);
});

it('returns undefined for missing keys', function () {
const values = getSettingValues([], ['test_key', 'test_key_2']);
expect(values).toEqual([undefined, undefined]);
});
});

describe('isSettingReadOnly', function () {
it('returns true if the setting has an override', function () {
const settings = [
{key: 'test_key', is_read_only: true, value: 'test_value'}
];
const value = isSettingReadOnly(settings, 'test_key');
expect(value).toEqual(true);
});

it('returns false if the setting does not have an override', function () {
const settings = [
{key: 'test_key', is_read_only: false, value: 'test_value'}
];
const value = isSettingReadOnly(settings, 'test_key');
expect(value).toEqual(false);
});

it('returns undefined if settings is falsy', function () {
const settings = undefined;
const value = isSettingReadOnly(settings, 'test_key');
expect(value).toEqual(undefined);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import TopLevelGroup from '../../TopLevelGroup';
import useSettingGroup from '../../../hooks/useSettingGroup';
import {Button, Separator, SettingGroupContent, Toggle, withErrorBoundary} from '@tryghost/admin-x-design-system';
import {getSettingValues} from '@tryghost/admin-x-framework/api/settings';
import {getSettingValues, isSettingReadOnly} from '@tryghost/admin-x-framework/api/settings';
import {usePostsExports} from '@tryghost/admin-x-framework/api/posts';

const Analytics: React.FC<{ keywords: string[] }> = ({keywords}) => {
Expand All @@ -20,6 +20,8 @@ const Analytics: React.FC<{ keywords: string[] }> = ({keywords}) => {
'email_track_opens', 'email_track_clicks', 'members_track_sources', 'outbound_link_tagging'
]) as boolean[];

const isEmailTrackClicksReadOnly = isSettingReadOnly(localSettings, 'email_track_clicks');

const handleToggleChange = (key: string, e: React.ChangeEvent<HTMLInputElement>) => {
updateSetting(key, e.target.checked);
handleEditingChange(true);
Expand Down Expand Up @@ -63,6 +65,7 @@ const Analytics: React.FC<{ keywords: string[] }> = ({keywords}) => {
<Toggle
checked={trackEmailClicks}
direction='rtl'
disabled={isEmailTrackClicksReadOnly}
gap='gap-0'
label='Newsletter clicks'
labelClasses='py-4 w-full'
Expand Down
24 changes: 24 additions & 0 deletions apps/admin-x-settings/test/acceptance/membership/analytics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,28 @@ test.describe('Analytics settings', async () => {
const hasDownloadUrl = lastApiRequests.postsExport?.url?.includes('/posts/export/?limit=1000');
expect(hasDownloadUrl).toBe(true);
});

test('Supports read only settings', async ({page}) => {
await mockApi({page, requests: {
...globalDataRequests,
browseSettings: {method: 'GET', path: /^\/settings\/\?group=/, response: updatedSettingsResponse([
{key: 'members_track_sources', value: false},
{key: 'email_track_opens', value: false},
{key: 'email_track_clicks', value: false, is_read_only: true},
{key: 'outbound_link_tagging', value: false}
])}
}});

await page.goto('/');

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

await expect(section).toBeVisible();

const newsletterClicksToggle = await section.getByLabel(/Newsletter clicks/);

await expect(newsletterClicksToggle).not.toBeChecked();

await expect(newsletterClicksToggle).toBeDisabled();
});
});

0 comments on commit 376b920

Please sign in to comment.