From 41def3e88e61d3ad5064da4c00337df0d276b3b2 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 13 Aug 2026 11:25:34 +0100 Subject: [PATCH 01/25] Added internal package golden path checks (#29877) Make the internal package golden path enforceable rather than relying on documentation alone. Introduce `compliant`, `migration`, and `exempt` lifecycle states, allowing history-preserving imports to remain intentionally transitional until their separate modernization work. Add a checker for package metadata, TypeScript/ESM structure, exports, shared configuration and workspace dependencies. It uses pnpm as the workspace source of truth and validates the package template without letting template drift redefine the contract. Run the check through local linting and lightweight, path-filtered CI. --- .../SKILL.md | 3 + .../skills/migrate-internal-package/SKILL.md | 2 + .../references/legacy-integration.md | 2 + .github/workflows/ci.yml | 32 ++ package.json | 3 +- packages/README.md | 17 + packages/_template/package.json | 3 + packages/adapters/redirects-base/package.json | 3 + .../adapters/route-settings-base/package.json | 3 + packages/admin-api-schema/package.json | 3 + packages/custom-field-types/package.json | 3 + packages/i18n/package.json | 4 + packages/nql-string/package.json | 3 + packages/parse-email-address/package.json | 3 + packages/testing/test-data/package.json | 4 + scripts/check-internal-packages.js | 348 ++++++++++++++++++ scripts/create-package.js | 14 +- scripts/lib/package-template.js | 12 + scripts/test/check-internal-packages.test.js | 309 ++++++++++++++++ 19 files changed, 764 insertions(+), 7 deletions(-) create mode 100644 scripts/check-internal-packages.js create mode 100644 scripts/lib/package-template.js create mode 100644 scripts/test/check-internal-packages.test.js diff --git a/.agents/skills/convert-internal-package-to-typescript/SKILL.md b/.agents/skills/convert-internal-package-to-typescript/SKILL.md index c125c765d4d..a2e892884a5 100644 --- a/.agents/skills/convert-internal-package-to-typescript/SKILL.md +++ b/.agents/skills/convert-internal-package-to-typescript/SKILL.md @@ -62,6 +62,9 @@ Use the subject `Changed file extensions to TypeScript`. Apply the package contract from `packages/README.md`: shared config packages, minimal package-local config, ESM metadata and exports, standard scripts, and a single compiled output unless a verified consumer requires an exception. +Replace the package's `migration` status with +`ghostPackage.goldenPath: compliant` only after every mechanical golden-path +check passes. Use the subject `Converted to TypeScript`. diff --git a/.agents/skills/migrate-internal-package/SKILL.md b/.agents/skills/migrate-internal-package/SKILL.md index 66c6d952ca3..208eae9eae1 100644 --- a/.agents/skills/migrate-internal-package/SKILL.md +++ b/.agents/skills/migrate-internal-package/SKILL.md @@ -88,6 +88,8 @@ commits manually. After the subtree commit, add focused integration commits that: - make the package private with an internal placeholder version; +- set `ghostPackage.goldenPath` to `migration` and `ghostPackage.reason` to a + concise explanation of the remaining modernization work; - switch Ghost consumers to `workspace:*`; - update the lockfile with `pnpm`; - minimally adapt configuration and tests to work in Ghost; diff --git a/.agents/skills/migrate-internal-package/references/legacy-integration.md b/.agents/skills/migrate-internal-package/references/legacy-integration.md index 8efa2b3e2f3..bc29be43e92 100644 --- a/.agents/skills/migrate-internal-package/references/legacy-integration.md +++ b/.agents/skills/migrate-internal-package/references/legacy-integration.md @@ -9,6 +9,8 @@ modernize the implementation. Change only what Ghost needs to consume and verify the package: - set `"private": true` and the internal placeholder version; +- set `ghostPackage.goldenPath` to `migration` and describe the remaining + modernization work in `ghostPackage.reason`; - point repository metadata at Ghost; - remove public publishing configuration; - switch Ghost consumers to `workspace:*`; diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 06bf1df3222..8fb7fb09df3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -161,6 +161,16 @@ jobs: - 'package.json' - 'scripts/check-agent-skill-links.js' - 'scripts/test/check-agent-skill-links.test.js' + package-standards: + - 'packages/**' + - 'package.json' + - 'pnpm-workspace.yaml' + - 'scripts/check-internal-packages.js' + - 'scripts/create-package.js' + - 'scripts/lib/constants.js' + - 'scripts/lib/package-template.js' + - 'scripts/test/check-internal-packages.test.js' + - '.github/workflows/ci.yml' core: - *shared # Repository documentation and ownership metadata do not affect @@ -279,6 +289,7 @@ jobs: changed_core: ${{ steps.changed.outputs.core }} changed_any_code: ${{ steps.changed.outputs.any-code }} changed_docs: ${{ steps.changed.outputs.docs }} + changed_package_standards: ${{ steps.changed.outputs.package-standards }} changed_tb_cli: ${{ steps.changed.outputs.tb-cli }} # Single gate for the build + browser-E2E lane. True for tags, or when a # changed file could affect a running Ghost instance (see the `e2e` path @@ -421,6 +432,26 @@ jobs: - name: Lint agent skills run: node scripts/check-agent-skill-links.js + job_lint_packages: + name: Lint packages + runs-on: ubuntu-slim + needs: [job_setup] + if: needs.job_setup.outputs.changed_package_standards == 'true' + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + + - uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6.0.9 + + - uses: actions/setup-node@249970729cb0ef3589644e2896645e5dc5ba9c38 # v6 + with: + node-version: ${{ env.NODE_VERSION }} + + - name: Check internal package golden path + run: node scripts/check-internal-packages.js + + - name: Test internal package checker + run: node --test scripts/test/check-internal-packages.test.js + job_i18n: runs-on: ubuntu-latest needs: [job_setup] @@ -2051,6 +2082,7 @@ jobs: job_migration_integrity_check, job_lint, job_lint_docs, + job_lint_packages, job_i18n, job_build_admin, job_pack, diff --git a/package.json b/package.json index 180d7a19ddd..38b41bf79c8 100644 --- a/package.json +++ b/package.json @@ -62,9 +62,10 @@ "docker:rebase": "git fetch ${GHOST_UPSTREAM:-origin} main && git rebase ${GHOST_UPSTREAM:-origin}/main && pnpm install && pnpm nx run-many -t build --projects=@tryghost/shade,@tryghost/admin-x-framework && docker compose -f compose.dev.yaml ${DEV_COMPOSE_FILES} up -d --build --force-recreate ghost-dev", "knip": "knip", "knip:fix": "knip --fix --allow-remove-files=false", - "lint": "pnpm nx run-many -t lint lint:boundaries && pnpm lint:docs", + "lint": "pnpm nx run-many -t lint lint:boundaries && pnpm lint:packages && pnpm lint:docs", "lint:agent-skills": "node scripts/check-agent-skill-links.js", "lint:boundaries": "depcruise ghost/core/core apps --config .dependency-cruiser.cjs", + "lint:packages": "node scripts/check-internal-packages.js", "lint:docs": "pnpm lint:agent-skills", "check": "pnpm lint && pnpm test", "test": "pnpm nx run-many -t test --exclude @tryghost/e2e --exclude ghost-admin", diff --git a/packages/README.md b/packages/README.md index d28314516a7..55fa831f442 100644 --- a/packages/README.md +++ b/packages/README.md @@ -14,6 +14,7 @@ New internal packages are private, TypeScript-only ESM libraries: - use an `@tryghost/` package name; - set `"version": "0.0.0"` and `"private": true`; +- set `"ghostPackage": {"goldenPath": "compliant"}`; - set `"type": "module"`; - keep authored code in `src/**/*.ts` and tests in `test/**/*.ts`; - compile production code to `build/` with `tsc`; @@ -23,6 +24,22 @@ Making a package public or independently versioned is a product and maintenance decision, not a packaging convenience. Establish its compatibility, release and support policy before removing `private` or adding publishing automation. +### Golden path status + +Every private package under `packages/` declares its lifecycle state in +`ghostPackage.goldenPath`: + +- `compliant` means the package is mechanically checked against this document; +- `migration` is temporary while a history-preserving import awaits a separate + modernization PR; +- `exempt` records an intentional long-term exception such as a test-only or + multi-runtime package. + +Both `migration` and `exempt` require a non-empty `ghostPackage.reason`. +Public, independently versioned packages do not declare this metadata because +this internal-only contract does not apply to them. Run `pnpm lint:packages` to +validate the status and all mechanically enforceable rules. + ## Package metadata Use the template's repository, author and license metadata. Point repository diff --git a/packages/_template/package.json b/packages/_template/package.json index b12829ea474..6f69858100f 100644 --- a/packages/_template/package.json +++ b/packages/_template/package.json @@ -3,6 +3,9 @@ "version": "0.0.0", "description": "{{DESCRIPTION}}", "private": true, + "ghostPackage": { + "goldenPath": "compliant" + }, "type": "module", "repository": { "type": "git", diff --git a/packages/adapters/redirects-base/package.json b/packages/adapters/redirects-base/package.json index 05808e44e7c..881bea07d15 100644 --- a/packages/adapters/redirects-base/package.json +++ b/packages/adapters/redirects-base/package.json @@ -3,6 +3,9 @@ "version": "0.0.0", "description": "The adapter-base-redirects package for Ghost.", "private": true, + "ghostPackage": { + "goldenPath": "compliant" + }, "type": "module", "repository": { "type": "git", diff --git a/packages/adapters/route-settings-base/package.json b/packages/adapters/route-settings-base/package.json index 55121464f13..0e010e7cf17 100644 --- a/packages/adapters/route-settings-base/package.json +++ b/packages/adapters/route-settings-base/package.json @@ -3,6 +3,9 @@ "version": "0.0.0", "description": "The adapter-base-route-settings package for Ghost.", "private": true, + "ghostPackage": { + "goldenPath": "compliant" + }, "type": "module", "repository": { "type": "git", diff --git a/packages/admin-api-schema/package.json b/packages/admin-api-schema/package.json index 702d358faf9..e1445503d09 100644 --- a/packages/admin-api-schema/package.json +++ b/packages/admin-api-schema/package.json @@ -3,6 +3,9 @@ "version": "0.0.0", "description": "JSON schemas used to validate Ghost Admin API requests", "private": true, + "ghostPackage": { + "goldenPath": "compliant" + }, "type": "module", "repository": { "type": "git", diff --git a/packages/custom-field-types/package.json b/packages/custom-field-types/package.json index a59f854a616..53d480ae0c8 100644 --- a/packages/custom-field-types/package.json +++ b/packages/custom-field-types/package.json @@ -3,6 +3,9 @@ "version": "0.0.0", "description": "Shared catalog of member custom field types: storage routing and value validation, consumed by Ghost core and admin", "private": true, + "ghostPackage": { + "goldenPath": "compliant" + }, "type": "module", "repository": { "type": "git", diff --git a/packages/i18n/package.json b/packages/i18n/package.json index f54c1fd0cfc..c2f25bf1b42 100644 --- a/packages/i18n/package.json +++ b/packages/i18n/package.json @@ -4,6 +4,10 @@ "repository": "https://github.com/TryGhost/Ghost/tree/main/packages/i18n", "author": "Ghost Foundation", "private": true, + "ghostPackage": { + "goldenPath": "exempt", + "reason": "Ships locale assets and dedicated CommonJS and ESM loaders for browser and server consumers." + }, "main": "index.js", "exports": { ".": { diff --git a/packages/nql-string/package.json b/packages/nql-string/package.json index 3c71ebe89e1..5f9973137d6 100644 --- a/packages/nql-string/package.json +++ b/packages/nql-string/package.json @@ -2,6 +2,9 @@ "name": "@tryghost/nql-string", "version": "0.0.0", "private": true, + "ghostPackage": { + "goldenPath": "compliant" + }, "repository": { "type": "git", "url": "git+https://github.com/TryGhost/Ghost.git", diff --git a/packages/parse-email-address/package.json b/packages/parse-email-address/package.json index 3d55dcfd46c..2b2a1608b07 100644 --- a/packages/parse-email-address/package.json +++ b/packages/parse-email-address/package.json @@ -2,6 +2,9 @@ "name": "@tryghost/parse-email-address", "version": "0.0.0", "private": true, + "ghostPackage": { + "goldenPath": "compliant" + }, "type": "module", "repository": { "type": "git", diff --git a/packages/testing/test-data/package.json b/packages/testing/test-data/package.json index 8dbd4841389..7fcc9fe23c4 100644 --- a/packages/testing/test-data/package.json +++ b/packages/testing/test-data/package.json @@ -2,6 +2,10 @@ "name": "@tryghost/test-data", "version": "0.0.0", "private": true, + "ghostPackage": { + "goldenPath": "exempt", + "reason": "Test-only source package consumed directly as TypeScript without a production build artifact." + }, "type": "module", "repository": "https://github.com/TryGhost/Ghost/tree/main/testing/test-data", "author": "Ghost Foundation", diff --git a/scripts/check-internal-packages.js b/scripts/check-internal-packages.js new file mode 100644 index 00000000000..eb9bd2963ef --- /dev/null +++ b/scripts/check-internal-packages.js @@ -0,0 +1,348 @@ +import {execFile} from 'node:child_process'; +import {readdir, readFile, realpath, stat} from 'node:fs/promises'; +import path from 'node:path'; +import {promisify} from 'node:util'; + +import {ROOT_DIR} from './lib/constants.js'; +import {applyPackageTemplateTokens, isValidPackageName} from './lib/package-template.js'; + +const execFileAsync = promisify(execFile); + +const GOLDEN_PATH_STATUSES = new Set(['compliant', 'migration', 'exempt']); +// These expectations intentionally remain independent of packages/_template. +// Deriving them from the template would allow accidental template drift to +// redefine the contract and approve itself. +const REQUIRED_SCRIPTS = { + build: 'tsc', + 'test:unit': 'NODE_ENV=testing vitest run --coverage', + 'test:types': 'tsc --noEmit -p test/tsconfig.json', + test: "pnpm run '/^test:/'", + 'lint:code': 'eslint src/ --cache', + 'lint:test': 'eslint test/ --cache', + lint: "pnpm run '/^lint:/'" +}; +const REQUIRED_DEV_DEPENDENCIES = { + '@internal/cfg-eslint': 'workspace:*', + '@internal/cfg-typescript': 'workspace:*', + '@internal/cfg-vitest': 'workspace:*', + '@types/node': 'catalog:', + '@vitest/coverage-v8': 'catalog:', + '@typescript/native': 'catalog:', + eslint: 'catalog:', + typescript: 'catalog:', + vitest: 'catalog:' +}; + +async function exists(filePath) { + try { + await stat(filePath); + return true; + } catch { + return false; + } +} + +async function readJson(filePath) { + return JSON.parse(await readFile(filePath, 'utf8')); +} + +async function listWorkspacePackages(rootDirectory) { + const {stdout} = await execFileAsync('pnpm', [ + 'm', 'ls', '--depth', '-1', '--json' + ], { + cwd: rootDirectory, + maxBuffer: 10 * 1024 * 1024 + }); + + return JSON.parse(stdout); +} + +function sameArray(actual, expected) { + return Array.isArray(actual) && actual.length === expected.length && actual.every((value, index) => value === expected[index]); +} + +function addExactValueError(errors, manifestPath, actual, expected, field) { + if (actual !== expected) { + errors.push(`${manifestPath}: ${field} must be ${JSON.stringify(expected)}`); + } +} + +function usesStandardConfigFactory(config, moduleName, factoryName) { + const importPattern = new RegExp(`^\\s*import\\s*\\{\\s*${factoryName}\\s*\\}\\s*from\\s*['"]${moduleName}['"];?\\s*$`, 'm'); + const exportPattern = new RegExp(`^\\s*export\\s+default\\s+${factoryName}\\s*\\(`, 'm'); + return importPattern.test(config) && exportPattern.test(config); +} + +async function findJavaScriptFiles(directory) { + if (!await exists(directory)) { + return []; + } + + const files = []; + for (const entry of await readdir(directory, {withFileTypes: true})) { + const entryPath = path.join(directory, entry.name); + if (entry.isDirectory()) { + files.push(...await findJavaScriptFiles(entryPath)); + } else if (/\.(?:c|m)?jsx?$/.test(entry.name)) { + files.push(entryPath); + } + } + return files; +} + +async function validateConfigFiles(packageDirectory, manifestPath, errors) { + const tsconfigPath = path.join(packageDirectory, 'tsconfig.json'); + const testTsconfigPath = path.join(packageDirectory, 'test', 'tsconfig.json'); + const eslintPath = path.join(packageDirectory, 'eslint.config.mjs'); + const vitestPath = path.join(packageDirectory, 'vitest.config.ts'); + + for (const requiredPath of [tsconfigPath, testTsconfigPath, eslintPath, vitestPath]) { + if (!await exists(requiredPath)) { + errors.push(`${manifestPath}: missing ${path.relative(packageDirectory, requiredPath)}`); + } + } + + if (await exists(tsconfigPath)) { + try { + const config = await readJson(tsconfigPath); + addExactValueError(errors, manifestPath, config.extends, '@internal/cfg-typescript/esm.json', 'tsconfig.json extends'); + addExactValueError(errors, manifestPath, config.compilerOptions?.rootDir, 'src', 'tsconfig.json compilerOptions.rootDir'); + addExactValueError(errors, manifestPath, config.compilerOptions?.outDir, 'build', 'tsconfig.json compilerOptions.outDir'); + if (!sameArray(config.include, ['src/**/*'])) { + errors.push(`${manifestPath}: tsconfig.json include must be ["src/**/*"]`); + } + } catch (error) { + errors.push(`${manifestPath}: invalid tsconfig.json (${error.message})`); + } + } + + if (await exists(testTsconfigPath)) { + try { + const config = await readJson(testTsconfigPath); + addExactValueError(errors, manifestPath, config.extends, '../tsconfig.json', 'test/tsconfig.json extends'); + addExactValueError(errors, manifestPath, config.compilerOptions?.rootDir, '..', 'test/tsconfig.json compilerOptions.rootDir'); + addExactValueError(errors, manifestPath, config.compilerOptions?.noEmit, true, 'test/tsconfig.json compilerOptions.noEmit'); + if (!sameArray(config.include, ['../src/**/*', '**/*'])) { + errors.push(`${manifestPath}: test/tsconfig.json include must be ["../src/**/*", "**/*"]`); + } + } catch (error) { + errors.push(`${manifestPath}: invalid test/tsconfig.json (${error.message})`); + } + } + + if (await exists(eslintPath)) { + const config = await readFile(eslintPath, 'utf8'); + if (!usesStandardConfigFactory(config, '@internal/cfg-eslint', 'nodeLibConfig')) { + errors.push(`${manifestPath}: eslint.config.mjs must use nodeLibConfig from @internal/cfg-eslint`); + } + } + + if (await exists(vitestPath)) { + const config = await readFile(vitestPath, 'utf8'); + if (!usesStandardConfigFactory(config, '@internal/cfg-vitest', 'createVitestConfig')) { + errors.push(`${manifestPath}: vitest.config.ts must use createVitestConfig from @internal/cfg-vitest`); + } + } +} + +async function validateCompliantPackage({rootDirectory, packageDirectory, manifest, workspaceNames, packagePath: explicitPackagePath}) { + const manifestPath = path.relative(rootDirectory, path.join(packageDirectory, 'package.json')); + const packagePath = explicitPackagePath ?? path.relative(rootDirectory, packageDirectory).split(path.sep).join('/'); + const errors = []; + + if (typeof manifest.name !== 'string' || !manifest.name.startsWith('@tryghost/') || !isValidPackageName(manifest.name.slice('@tryghost/'.length))) { + errors.push(`${manifestPath}: name must use the @tryghost/ form`); + } + addExactValueError(errors, manifestPath, manifest.version, '0.0.0', 'version'); + addExactValueError(errors, manifestPath, manifest.private, true, 'private'); + addExactValueError(errors, manifestPath, manifest.type, 'module', 'type'); + addExactValueError(errors, manifestPath, manifest.author, 'Ghost Foundation', 'author'); + addExactValueError(errors, manifestPath, manifest.license, 'MIT', 'license'); + addExactValueError(errors, manifestPath, manifest.repository?.type, 'git', 'repository.type'); + addExactValueError(errors, manifestPath, manifest.repository?.url, 'git+https://github.com/TryGhost/Ghost.git', 'repository.url'); + addExactValueError(errors, manifestPath, manifest.repository?.directory, packagePath, 'repository.directory'); + + if (manifest.publishConfig !== undefined) { + errors.push(`${manifestPath}: compliant internal packages must not define publishConfig`); + } + if (!sameArray(manifest.files, ['build'])) { + errors.push(`${manifestPath}: files must be ["build"]`); + } + + const exports = manifest.exports; + if (!exports || typeof exports !== 'object' || Array.isArray(exports)) { + errors.push(`${manifestPath}: exports must define explicit entry points`); + } else { + for (const [entryPoint, conditions] of Object.entries(exports)) { + if (!conditions || typeof conditions !== 'object' || Array.isArray(conditions)) { + errors.push(`${manifestPath}: exports.${entryPoint} must be an object`); + continue; + } + + if (!sameArray(Object.keys(conditions), ['source', 'types', 'default'])) { + errors.push(`${manifestPath}: exports.${entryPoint} conditions must be source, types, default in that order`); + continue; + } + + for (const condition of ['source', 'types', 'default']) { + if (typeof conditions[condition] !== 'string') { + errors.push(`${manifestPath}: exports.${entryPoint}.${condition} must be a string`); + } + } + + const sourceMatch = typeof conditions.source === 'string' && /^\.\/src\/(.+)\.ts$/.exec(conditions.source); + if (!sourceMatch || sourceMatch[1].split('/').includes('..')) { + errors.push(`${manifestPath}: exports.${entryPoint}.source must point to ./src/.ts`); + continue; + } + + const sourceStem = sourceMatch[1]; + addExactValueError(errors, manifestPath, conditions.types, `./build/${sourceStem}.d.ts`, `exports.${entryPoint}.types`); + addExactValueError(errors, manifestPath, conditions.default, `./build/${sourceStem}.js`, `exports.${entryPoint}.default`); + if (!await exists(path.join(packageDirectory, conditions.source))) { + errors.push(`${manifestPath}: exports.${entryPoint}.source does not exist (${conditions.source})`); + } + } + + const rootExport = exports['.']; + if (!rootExport) { + errors.push(`${manifestPath}: exports must define the "." entry point`); + } else { + if (typeof rootExport.default === 'string') { + addExactValueError(errors, manifestPath, manifest.main, rootExport.default.replace(/^\.\//, ''), 'main'); + } + if (typeof rootExport.types === 'string') { + addExactValueError(errors, manifestPath, manifest.types, rootExport.types.replace(/^\.\//, ''), 'types'); + } + } + } + + for (const [script, expected] of Object.entries(REQUIRED_SCRIPTS)) { + addExactValueError(errors, manifestPath, manifest.scripts?.[script], expected, `scripts.${script}`); + } + for (const [dependency, expected] of Object.entries(REQUIRED_DEV_DEPENDENCIES)) { + addExactValueError(errors, manifestPath, manifest.devDependencies?.[dependency], expected, `devDependencies.${dependency}`); + } + if (!sameArray(manifest.nx?.targets?.build?.outputs, ['{projectRoot}/build'])) { + errors.push(`${manifestPath}: nx.targets.build.outputs must be ["{projectRoot}/build"]`); + } + + for (const section of ['dependencies', 'devDependencies', 'optionalDependencies', 'peerDependencies']) { + for (const [dependency, version] of Object.entries(manifest[section] ?? {})) { + if (dependency !== manifest.name && workspaceNames.has(dependency) && version !== 'workspace:*') { + errors.push(`${manifestPath}: ${section}.${dependency} must use workspace:*`); + } + } + } + + await validateConfigFiles(packageDirectory, manifestPath, errors); + + for (const javascriptFile of await findJavaScriptFiles(path.join(packageDirectory, 'src'))) { + errors.push(`${manifestPath}: authored source must be TypeScript (${path.relative(packageDirectory, javascriptFile)})`); + } + for (const javascriptFile of await findJavaScriptFiles(path.join(packageDirectory, 'test'))) { + errors.push(`${manifestPath}: authored tests must be TypeScript (${path.relative(packageDirectory, javascriptFile)})`); + } + + return errors; +} + +export async function checkInternalPackages(rootDirectory) { + rootDirectory = await realpath(rootDirectory); + const packagesDirectory = path.join(rootDirectory, 'packages'); + let workspacePackages; + try { + workspacePackages = await listWorkspacePackages(rootDirectory); + } catch (error) { + const detail = error.stderr?.trim() || error.message; + return [`Unable to list pnpm workspace packages (${detail})`]; + } + const packageDirectories = workspacePackages + .map(workspace => workspace.path) + .filter(directory => directory.startsWith(`${packagesDirectory}${path.sep}`)) + .sort(); + const packages = []; + const errors = []; + + for (const packageDirectory of packageDirectories) { + const manifestPath = path.join(packageDirectory, 'package.json'); + try { + packages.push({packageDirectory, manifest: await readJson(manifestPath)}); + } catch (error) { + errors.push(`${path.relative(rootDirectory, manifestPath)}: invalid JSON (${error.message})`); + } + } + + const workspaceNames = new Set(); + for (const workspace of workspacePackages) { + if (workspace.name) { + workspaceNames.add(workspace.name); + } + } + + for (const pkg of packages) { + const manifestPath = path.relative(rootDirectory, path.join(pkg.packageDirectory, 'package.json')); + const status = pkg.manifest.ghostPackage?.goldenPath; + + if (pkg.manifest.private !== true) { + if (status !== undefined) { + errors.push(`${manifestPath}: ghostPackage.goldenPath is only valid for private internal packages`); + } + continue; + } + + if (!GOLDEN_PATH_STATUSES.has(status)) { + errors.push(`${manifestPath}: ghostPackage.goldenPath must be one of compliant, migration, exempt`); + continue; + } + + if (status === 'migration' || status === 'exempt') { + if (typeof pkg.manifest.ghostPackage.reason !== 'string' || pkg.manifest.ghostPackage.reason.trim().length === 0) { + errors.push(`${manifestPath}: ghostPackage.reason is required when goldenPath is ${status}`); + } + continue; + } + + errors.push(...await validateCompliantPackage({ + rootDirectory, + packageDirectory: pkg.packageDirectory, + manifest: pkg.manifest, + workspaceNames + })); + } + + const templateManifestPath = path.join(packagesDirectory, '_template', 'package.json'); + let templateManifest; + try { + const templateManifestSource = await readFile(templateManifestPath, 'utf8'); + templateManifest = JSON.parse(applyPackageTemplateTokens(templateManifestSource, { + name: 'template', + directory: 'packages/template', + description: 'Template package' + })); + } catch (error) { + errors.push(`packages/_template/package.json: unreadable or invalid JSON (${error.message})`); + } + + if (templateManifest) { + errors.push(...await validateCompliantPackage({ + rootDirectory, + packageDirectory: path.dirname(templateManifestPath), + packagePath: 'packages/template', + manifest: templateManifest, + workspaceNames + })); + } + + return errors; +} + +if (import.meta.main) { + const errors = await checkInternalPackages(ROOT_DIR); + if (errors.length > 0) { + console.error(`Internal package golden path check failed:\n\n${errors.join('\n')}`); + process.exitCode = 1; + } else { + console.log('All private internal packages have a valid golden path status.'); + } +} diff --git a/scripts/create-package.js b/scripts/create-package.js index bc3e8dcf5ac..19f1ec24291 100644 --- a/scripts/create-package.js +++ b/scripts/create-package.js @@ -5,6 +5,7 @@ import {existsSync} from 'node:fs'; import {join, dirname, relative, resolve, sep} from 'node:path'; import {ROOT_DIR} from './lib/constants.js'; +import {applyPackageTemplateTokens, isValidPackageName} from './lib/package-template.js'; const TEMPLATE_DIR = join(ROOT_DIR, 'packages', '_template'); const PACKAGES_DIR = join(ROOT_DIR, 'packages'); @@ -49,7 +50,7 @@ const name = positionals[0]; if (!name) { fail('Missing package name.'); } -if (!/^[a-z0-9][a-z0-9-]*$/.test(name)) { +if (!isValidPackageName(name)) { fail(`Invalid package name "${name}". Use lowercase kebab-case, e.g. "email-utils" (no @scope — it becomes @tryghost/${name}).`); } @@ -81,10 +82,11 @@ async function walk(dir) { } function applyTokens(text) { - return text - .replaceAll('{{NAME}}', name) - .replaceAll('{{DIRECTORY}}', packageDir) - .replaceAll('{{DESCRIPTION}}', description); + return applyPackageTemplateTokens(text, { + name, + directory: packageDir, + description + }); } await mkdir(dirname(targetDir), {recursive: true}); @@ -108,4 +110,4 @@ for (const file of await walk(targetDir)) { // Clean any stray build artifacts the template dir may have accumulated. await rm(join(targetDir, 'build'), {recursive: true, force: true}); -process.stdout.write(`\n\x1b[32m✓ Created @tryghost/${name}\x1b[0m at ${packageDir} (ESM-only)\n\nNext steps:\n 1. pnpm install # link the new workspace member\n 2. pnpm --filter @tryghost/${name} test\n 3. Add real code in ${packageDir}/src/index.ts\n\n`); +process.stdout.write(`\n\x1b[32m✓ Created @tryghost/${name}\x1b[0m at ${packageDir} (ESM-only)\n\nNext steps:\n 1. pnpm install # link the new workspace member\n 2. cd ${packageDir} && pnpm test\n 3. Add real code in ${packageDir}/src/index.ts\n\n`); diff --git a/scripts/lib/package-template.js b/scripts/lib/package-template.js new file mode 100644 index 00000000000..a930b6f1e87 --- /dev/null +++ b/scripts/lib/package-template.js @@ -0,0 +1,12 @@ +const PACKAGE_NAME_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; + +export function isValidPackageName(name) { + return PACKAGE_NAME_PATTERN.test(name); +} + +export function applyPackageTemplateTokens(text, {name, directory, description}) { + return text + .replaceAll('{{NAME}}', name) + .replaceAll('{{DIRECTORY}}', directory) + .replaceAll('{{DESCRIPTION}}', description); +} diff --git a/scripts/test/check-internal-packages.test.js b/scripts/test/check-internal-packages.test.js new file mode 100644 index 00000000000..8da4c3a64a3 --- /dev/null +++ b/scripts/test/check-internal-packages.test.js @@ -0,0 +1,309 @@ +import assert from 'node:assert/strict'; +import {mkdir, mkdtemp, rm, writeFile} from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; +import {afterEach, test} from 'node:test'; + +import {checkInternalPackages} from '../check-internal-packages.js'; + +const temporaryDirectories = []; + +async function writeJson(filePath, value) { + await mkdir(path.dirname(filePath), {recursive: true}); + await writeFile(filePath, `${JSON.stringify(value, null, 2)}\n`); +} + +async function createRepository() { + const rootDirectory = await mkdtemp(path.join(os.tmpdir(), 'ghost-internal-packages-')); + temporaryDirectories.push(rootDirectory); + await writeJson(path.join(rootDirectory, 'package.json'), { + name: 'test-workspace', + private: true + }); + await writeFile(path.join(rootDirectory, 'pnpm-workspace.yaml'), "packages:\n - 'packages/**'\n - '!packages/_template'\n - 'koenig/*'\n"); + const templateManifest = compliantManifest('packages/template'); + templateManifest.name = '@tryghost/{{NAME}}'; + templateManifest.description = '{{DESCRIPTION}}'; + templateManifest.repository.directory = '{{DIRECTORY}}'; + await createCompliantPackage(rootDirectory, 'packages/_template', templateManifest); + return rootDirectory; +} + +function compliantManifest(directory = 'packages/example') { + return { + name: '@tryghost/example', + version: '0.0.0', + private: true, + type: 'module', + repository: { + type: 'git', + url: 'git+https://github.com/TryGhost/Ghost.git', + directory + }, + author: 'Ghost Foundation', + license: 'MIT', + exports: { + '.': { + source: './src/index.ts', + types: './build/index.d.ts', + default: './build/index.js' + } + }, + main: 'build/index.js', + types: 'build/index.d.ts', + scripts: { + build: 'tsc', + 'test:unit': 'NODE_ENV=testing vitest run --coverage', + 'test:types': 'tsc --noEmit -p test/tsconfig.json', + test: "pnpm run '/^test:/'", + 'lint:code': 'eslint src/ --cache', + 'lint:test': 'eslint test/ --cache', + lint: "pnpm run '/^lint:/'" + }, + files: ['build'], + devDependencies: {...REQUIRED_DEV_DEPENDENCIES_FOR_TEST}, + nx: {targets: {build: {outputs: ['{projectRoot}/build']}}}, + ghostPackage: {goldenPath: 'compliant'} + }; +} + +const REQUIRED_DEV_DEPENDENCIES_FOR_TEST = { + '@internal/cfg-eslint': 'workspace:*', + '@internal/cfg-typescript': 'workspace:*', + '@internal/cfg-vitest': 'workspace:*', + '@types/node': 'catalog:', + '@vitest/coverage-v8': 'catalog:', + '@typescript/native': 'catalog:', + eslint: 'catalog:', + typescript: 'catalog:', + vitest: 'catalog:' +}; + +async function createCompliantPackage(rootDirectory, directory = 'packages/example', manifest = compliantManifest(directory)) { + const packageDirectory = path.join(rootDirectory, directory); + await writeJson(path.join(packageDirectory, 'package.json'), manifest); + await writeJson(path.join(packageDirectory, 'tsconfig.json'), { + extends: '@internal/cfg-typescript/esm.json', + compilerOptions: {rootDir: 'src', outDir: 'build'}, + include: ['src/**/*'] + }); + await writeJson(path.join(packageDirectory, 'test', 'tsconfig.json'), { + extends: '../tsconfig.json', + compilerOptions: {rootDir: '..', noEmit: true}, + include: ['../src/**/*', '**/*'] + }); + await mkdir(path.join(packageDirectory, 'src'), {recursive: true}); + await writeFile(path.join(packageDirectory, 'src', 'index.ts'), 'export const value = true;\n'); + await writeFile(path.join(packageDirectory, 'eslint.config.mjs'), "import {nodeLibConfig} from '@internal/cfg-eslint';\nexport default nodeLibConfig();\n"); + await writeFile(path.join(packageDirectory, 'vitest.config.ts'), "import {createVitestConfig} from '@internal/cfg-vitest';\nexport default createVitestConfig();\n"); + return packageDirectory; +} + +afterEach(async () => { + await Promise.all(temporaryDirectories.splice(0).map(directory => rm(directory, {recursive: true, force: true}))); +}); + +test('accepts a compliant nested private package', async () => { + const rootDirectory = await createRepository(); + await createCompliantPackage(rootDirectory, 'packages/adapters/example'); + + assert.deepEqual(await checkInternalPackages(rootDirectory), []); +}); + +test('reports workspace discovery failures through the checker interface', async () => { + const rootDirectory = await createRepository(); + await writeFile(path.join(rootDirectory, 'pnpm-workspace.yaml'), 'packages: [invalid\n'); + + assert.match( + (await checkInternalPackages(rootDirectory))[0], + /^Unable to list pnpm workspace packages/ + ); +}); + +test('requires every private package to declare its golden path status', async () => { + const rootDirectory = await createRepository(); + const packageDirectory = await createCompliantPackage(rootDirectory); + const manifest = compliantManifest(); + delete manifest.ghostPackage; + await writeJson(path.join(packageDirectory, 'package.json'), manifest); + + assert.deepEqual(await checkInternalPackages(rootDirectory), [ + 'packages/example/package.json: ghostPackage.goldenPath must be one of compliant, migration, exempt' + ]); +}); + +test('accepts documented migration and exempt states without applying the golden path', async () => { + const rootDirectory = await createRepository(); + await writeJson(path.join(rootDirectory, 'packages', 'migrating', 'package.json'), { + name: '@tryghost/migrating', + private: true, + ghostPackage: {goldenPath: 'migration', reason: 'Imported unchanged before a separate modernization PR.'} + }); + await writeJson(path.join(rootDirectory, 'packages', 'special', 'package.json'), { + name: '@tryghost/special', + private: true, + ghostPackage: {goldenPath: 'exempt', reason: 'This is a source-only test helper.'} + }); + + assert.deepEqual(await checkInternalPackages(rootDirectory), []); +}); + +test('requires migration and exempt states to explain the exception', async () => { + const rootDirectory = await createRepository(); + await writeJson(path.join(rootDirectory, 'packages', 'special', 'package.json'), { + name: '@tryghost/special', + private: true, + ghostPackage: {goldenPath: 'exempt'} + }); + + assert.deepEqual(await checkInternalPackages(rootDirectory), [ + 'packages/special/package.json: ghostPackage.reason is required when goldenPath is exempt' + ]); +}); + +test('reports manifest, export, config and source violations together', async () => { + const rootDirectory = await createRepository(); + const packageDirectory = await createCompliantPackage(rootDirectory); + const manifest = compliantManifest(); + manifest.version = '1.0.0'; + manifest.exports['.'] = { + types: './build/index.d.ts', + source: './src/index.ts', + default: './build/index.js' + }; + delete manifest.scripts.build; + await writeJson(path.join(packageDirectory, 'package.json'), manifest); + await writeFile(path.join(packageDirectory, 'src', 'legacy.js'), 'module.exports = {};\n'); + await writeFile(path.join(packageDirectory, 'test', 'legacy.test.js'), 'export {};\n'); + + const errors = await checkInternalPackages(rootDirectory); + assert.ok(errors.includes('packages/example/package.json: version must be "0.0.0"')); + assert.ok(errors.includes('packages/example/package.json: exports.. conditions must be source, types, default in that order')); + assert.ok(errors.includes('packages/example/package.json: scripts.build must be "tsc"')); + assert.ok(errors.includes('packages/example/package.json: authored source must be TypeScript (src/legacy.js)')); + assert.ok(errors.includes('packages/example/package.json: authored tests must be TypeScript (test/legacy.test.js)')); +}); + +test('rejects package names that are not kebab-case', async () => { + const rootDirectory = await createRepository(); + const packageDirectory = await createCompliantPackage(rootDirectory); + const manifest = compliantManifest(); + manifest.name = '@tryghost/example--'; + await writeJson(path.join(packageDirectory, 'package.json'), manifest); + + assert.ok((await checkInternalPackages(rootDirectory)).includes( + 'packages/example/package.json: name must use the @tryghost/ form' + )); +}); + +test('does not accept config factory names that appear only in comments', async () => { + const rootDirectory = await createRepository(); + const packageDirectory = await createCompliantPackage(rootDirectory); + await writeFile(path.join(packageDirectory, 'eslint.config.mjs'), "// import {nodeLibConfig} from '@internal/cfg-eslint';\n// export default nodeLibConfig();\nexport default [];\n"); + await writeFile(path.join(packageDirectory, 'vitest.config.ts'), "// import {createVitestConfig} from '@internal/cfg-vitest';\n// export default createVitestConfig();\nexport default {};\n"); + + const errors = await checkInternalPackages(rootDirectory); + assert.ok(errors.includes('packages/example/package.json: eslint.config.mjs must use nodeLibConfig from @internal/cfg-eslint')); + assert.ok(errors.includes('packages/example/package.json: vitest.config.ts must use createVitestConfig from @internal/cfg-vitest')); +}); + +test('reports non-string export conditions without terminating validation', async () => { + const rootDirectory = await createRepository(); + const packageDirectory = await createCompliantPackage(rootDirectory); + const manifest = compliantManifest(); + manifest.exports['.'] = {source: 42, types: false, default: {}}; + await writeJson(path.join(packageDirectory, 'package.json'), manifest); + + const errors = await checkInternalPackages(rootDirectory); + assert.ok(errors.includes('packages/example/package.json: exports...source must be a string')); + assert.ok(errors.includes('packages/example/package.json: exports...types must be a string')); + assert.ok(errors.includes('packages/example/package.json: exports...default must be a string')); +}); + +test('validates the complete package template contract', async () => { + const rootDirectory = await createRepository(); + const templatePath = path.join(rootDirectory, 'packages', '_template', 'package.json'); + const manifest = compliantManifest('packages/template'); + manifest.name = '@tryghost/{{NAME}}'; + manifest.repository.directory = '{{DIRECTORY}}'; + delete manifest.scripts.build; + await writeJson(templatePath, manifest); + + assert.ok((await checkInternalPackages(rootDirectory)).includes( + 'packages/_template/package.json: scripts.build must be "tsc"' + )); +}); + +test('reports an unreadable package template accurately', async () => { + const rootDirectory = await createRepository(); + await rm(path.join(rootDirectory, 'packages', '_template', 'package.json')); + + assert.match( + (await checkInternalPackages(rootDirectory))[0], + /^packages\/_template\/package\.json: unreadable or invalid JSON/ + ); +}); + +test('ignores public packages but rejects golden path metadata on them', async () => { + const rootDirectory = await createRepository(); + await writeJson(path.join(rootDirectory, 'packages', 'public', 'package.json'), { + name: '@tryghost/public', + version: '1.0.0' + }); + assert.deepEqual(await checkInternalPackages(rootDirectory), []); + + await writeJson(path.join(rootDirectory, 'packages', 'public', 'package.json'), { + name: '@tryghost/public', + version: '1.0.0', + ghostPackage: {goldenPath: 'compliant'} + }); + assert.deepEqual(await checkInternalPackages(rootDirectory), [ + 'packages/public/package.json: ghostPackage.goldenPath is only valid for private internal packages' + ]); +}); + +test('requires dependencies on packages in this workspace to use workspace:*', async () => { + const rootDirectory = await createRepository(); + const packageDirectory = await createCompliantPackage(rootDirectory); + const manifest = compliantManifest(); + manifest.dependencies = {'@tryghost/other': 'catalog:'}; + await writeJson(path.join(packageDirectory, 'package.json'), manifest); + await writeJson(path.join(rootDirectory, 'packages', 'other', 'package.json'), { + name: '@tryghost/other', + version: '1.0.0' + }); + + assert.deepEqual(await checkInternalPackages(rootDirectory), [ + 'packages/example/package.json: dependencies.@tryghost/other must use workspace:*' + ]); +}); + +test('recognizes workspace packages outside packages/', async () => { + const rootDirectory = await createRepository(); + const packageDirectory = await createCompliantPackage(rootDirectory); + const manifest = compliantManifest(); + manifest.dependencies = {'@tryghost/koenig-example': 'catalog:'}; + await writeJson(path.join(packageDirectory, 'package.json'), manifest); + await writeJson(path.join(rootDirectory, 'koenig', 'example', 'package.json'), { + name: '@tryghost/koenig-example' + }); + + assert.deepEqual(await checkInternalPackages(rootDirectory), [ + 'packages/example/package.json: dependencies.@tryghost/koenig-example must use workspace:*' + ]); +}); + +test('requires peer dependencies on workspace packages to use workspace:*', async () => { + const rootDirectory = await createRepository(); + const packageDirectory = await createCompliantPackage(rootDirectory); + const manifest = compliantManifest(); + manifest.peerDependencies = {'@tryghost/koenig-example': '>=1'}; + await writeJson(path.join(packageDirectory, 'package.json'), manifest); + await writeJson(path.join(rootDirectory, 'koenig', 'example', 'package.json'), { + name: '@tryghost/koenig-example' + }); + + assert.deepEqual(await checkInternalPackages(rootDirectory), [ + 'packages/example/package.json: peerDependencies.@tryghost/koenig-example must use workspace:*' + ]); +}); From f146bfaa640fafa61dd643511d20e4c39fd9ff62 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 13 Aug 2026 11:33:51 +0100 Subject: [PATCH 02/25] Added translation and internationalization guides (#29931) We're moving our codebase docs into the repository. The PR was done in two steps - copy and update. The resulting will have one change but have access to that history. The resulting docs are split into Translating Ghost aimed at i18n contributors AND an internal codebase guide for creating translatable copy. --- docs/README.md | 5 ++ docs/contributing/translating-ghost.md | 84 ++++++++++++++++++++++++ docs/practices/internationalization.md | 91 ++++++++++++++++++++++++++ 3 files changed, 180 insertions(+) create mode 100644 docs/contributing/translating-ghost.md create mode 100644 docs/practices/internationalization.md diff --git a/docs/README.md b/docs/README.md index a8df41a1fc0..516597d955c 100644 --- a/docs/README.md +++ b/docs/README.md @@ -63,6 +63,11 @@ Before contributing, please read: 1. [Contributing Guide](../.github/CONTRIBUTING.md) - Guidelines for contributions 2. [Code of Conduct](../.github/CODE_OF_CONDUCT.md) - Community standards +To contribute or add translations, see +[Translating Ghost](contributing/translating-ghost.md). For more detail on +adding translatable product copy, see the +[internationalization guide](practices/internationalization.md). + ### Finding Issues to Work On - [Good First Issues](https://github.com/TryGhost/Ghost/labels/good%20first%20issue) - Great for newcomers diff --git a/docs/contributing/translating-ghost.md b/docs/contributing/translating-ghost.md new file mode 100644 index 00000000000..86a72389f4f --- /dev/null +++ b/docs/contributing/translating-ghost.md @@ -0,0 +1,84 @@ +# Translating Ghost + +Ghost can be translated into many languages. Translations cover Ghost's public +apps, parts of Ghost Core, and emails sent to members. + +Ghost uses [i18next](https://www.i18next.com/) and keeps translations in +[`packages/i18n/locales/`](../../packages/i18n/locales/). Each language has its +own folder containing separate JSON files for Ghost, Portal, Comments, Signup +form, and Search. + +Within each file, the key on the left is the original English string and the +value on the right is its translation. An empty value falls back to the English +string. + +## Translating existing strings + +1. Find your language in `packages/i18n/locales/`. +2. Open the JSON file for the part of Ghost you want to translate: + + | File | Where the translation appears | + | --- | --- | + | `ghost.json` | Ghost Core and emails | + | `portal.json` | Portal | + | `comments.json` | Comments | + | `signup-form.json` | Signup form | + | `search.json` | Search | + +3. Add or improve the translated values. Leave the English keys unchanged. +4. Run the translation checks from the repository root: + + ```bash + pnpm --filter @tryghost/i18n lint:translations + ``` + +5. Commit the changes and open a pull request following the + [contribution workflow](workflow.md). + +Keep every `{variable}` and `` from the English string in the translation. +The words around them can move to suit the language, but their names and +spelling must not change. + +```json +{ + "Welcome back, {name}!": "Bon retour, {name} !" +} +``` + +Translate the meaning of the complete message rather than translating each word +literally. The description for a string in +[`packages/i18n/locales/context.json`](../../packages/i18n/locales/context.json) +explains where it appears and what it is intended to communicate. + +## Adding a language + +Before starting a new language, open an issue or discussion so the locale code +and scope can be agreed. Ghost supports base languages as well as some regional +and script variants. + +To add an agreed language: + +1. Add its code and English label to + [`packages/i18n/lib/locale-data.json`](../../packages/i18n/lib/locale-data.json). +2. From the repository root, run: + + ```bash + pnpm --filter @tryghost/i18n translate + ``` + +3. Translate the generated files in `packages/i18n/locales//`. +4. Run the translation checks and package tests: + + ```bash + pnpm --filter @tryghost/i18n lint:translations + pnpm --filter @tryghost/i18n test + ``` + +5. Commit the locale metadata and translation files together, then open a pull + request. + +## Adding product copy + +If you are adding or changing translatable strings in the code, see the +[internationalization guide](../practices/internationalization.md). It covers +translation helpers, extraction, interpolation, context, and CI checks. diff --git a/docs/practices/internationalization.md b/docs/practices/internationalization.md new file mode 100644 index 00000000000..daaf16fee74 --- /dev/null +++ b/docs/practices/internationalization.md @@ -0,0 +1,91 @@ +# Internationalization + +Use Ghost's internationalization system for product copy that appears in a +supported translatable surface. The shared `packages/i18n` package extracts +English source strings and provides locale resources to Ghost Core and the +public apps. + +For contributing translations or adding a language, see +[Translating Ghost](../contributing/translating-ghost.md). + +## Namespaces + +Translation files live at +`packages/i18n/locales//.json`. The extraction scripts define +five namespaces: + +| Namespace | Source | +| --- | --- | +| `ghost` | Ghost Core, including server, frontend, and member email templates | +| `portal` | Portal | +| `comments` | Comments | +| `signup-form` | Signup form | +| `search` | Search | + +The English string passed to `t()` is the translation key. English locale values +are empty so i18next falls back to that key. + +## Writing translatable copy + +Import and use the `t()` helper established by the app or service you are +changing. Keep a complete sentence in one translation call so translators can +change its word order. + +```jsx +// Do +t('Could not sign in. Please try again.') + +// Do not split one message across translation calls +t('Could not sign in.') + ' ' + t('Please try again.') +``` + +Use named variables for dynamic values: + +```jsx +t('Welcome back, {name}!', {name: member.name}) +``` + +When a message contains a link, button, or other element, keep the full message +in one string and use `@doist/react-interpolate`: + +```jsx +import Interpolate from '@doist/react-interpolate'; + +}} + string={t('Having trouble? Read the help guide.')} +/> +``` + +Do not build a sentence from translated fragments. Preserve the names of +`{variables}` and ``: they form part of the runtime contract and are +validated across locales. + +## Extracting strings + +After adding or changing a source string, run from the repository root: + +```bash +pnpm --filter @tryghost/i18n translate +``` + +This extracts source strings, updates all locale files, and synchronizes +`packages/i18n/locales/context.json`. Add a useful description for each new +entry in `context.json` so translators know where the message appears and what +it means. CI rejects extraction changes and empty context descriptions. + +Commit the source change, generated locale changes, and context changes +together. + +## Checking changes + +Run the package checks from the repository root: + +```bash +pnpm --filter @tryghost/i18n lint:translations +pnpm --filter @tryghost/i18n test +``` + +The translation linter checks that locale values use the variables defined by +their English message. The package tests also run extraction, so review the +resulting diff and commit any expected generated changes. From 002deb8b5c0f9f6888f8d41e95a98a19b00692a3 Mon Sep 17 00:00:00 2001 From: Austin Burdine Date: Thu, 13 Aug 2026 07:16:32 -0400 Subject: [PATCH 03/25] =?UTF-8?q?=E2=9C=A8=20Added=20support=20for=20Docke?= =?UTF-8?q?r=20secrets=20to=20config=20loading=20(#29924)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ref https://github.com/TryGhost/docker-library-ghost/issues/429 - add support for _FILE-suffixed environment variables pointing to local files for secrets loading, in keeping with existing Docker conventions --- ghost/core/core/shared/config/loader.ts | 13 +- ghost/core/core/shared/config/secrets.ts | 83 +++++++++++ .../test/unit/shared/config/loader.test.js | 55 +++++++ .../test/unit/shared/config/secrets.test.ts | 137 ++++++++++++++++++ 4 files changed, 286 insertions(+), 2 deletions(-) create mode 100644 ghost/core/core/shared/config/secrets.ts create mode 100644 ghost/core/test/unit/shared/config/secrets.test.ts diff --git a/ghost/core/core/shared/config/loader.ts b/ghost/core/core/shared/config/loader.ts index 6f240a96e16..51659eca5bc 100644 --- a/ghost/core/core/shared/config/loader.ts +++ b/ghost/core/core/shared/config/loader.ts @@ -2,6 +2,7 @@ import Nconf from 'nconf'; import path from 'node:path'; import {bindAll as bindUrlHelpers, type BoundHelpers} from '@tryghost/config-url-helpers'; import * as localUtils from './utils'; +import {loadSecretsFromEnv, isSecretFileRef} from './secrets'; import {bindAll as bindHelpers, type ConfigHelpers} from './helpers'; const _debug = require('@tryghost/debug')._base; @@ -28,9 +29,17 @@ function loadNconf(options?: LoadNconfOptions): ConfigInstance { // no channel can override the overrides nconf.file('overrides', path.join(baseConfigPath, 'overrides.json')); - // command line arguments take precedence, then environment variables + // command line arguments take precedence, then secret files, then environment variables nconf.argv(); - nconf.env({separator: '__', parseValues: true}); + // secrets are not parsed - a password like `01234` must stay a string + nconf.add('secrets', {type: 'literal', store: loadSecretsFromEnv()}); + nconf.env({ + separator: '__', + parseValues: true, + // the secrets store has already resolved these, so keep the file paths themselves + // out of config - otherwise e.g. `database:connection` gains a bogus `password_FILE` key + transform: ({key, value}: {key: string, value: string}) => (isSecretFileRef(key) ? false : {key, value}) + }); // Now load various config json files nconf.file('custom-env', path.join(customConfigPath, 'config.' + env + '.json')); diff --git a/ghost/core/core/shared/config/secrets.ts b/ghost/core/core/shared/config/secrets.ts new file mode 100644 index 00000000000..56c15e4e940 --- /dev/null +++ b/ghost/core/core/shared/config/secrets.ts @@ -0,0 +1,83 @@ +import fs from 'node:fs'; +import {setWith} from 'lodash'; + +const SUFFIX = '_file'; + +type SecretStore = Record; + +/** + * Whether an env var name is a reference to a secret file rather than a config value itself. + */ +function isSecretFileRef(name: string, separator: string = '__'): boolean { + if (!name.toLowerCase().endsWith(SUFFIX)) { + return false; + } + + const keyPath = name.slice(0, -SUFFIX.length).split(separator); + + // the key must be nested, which keeps unrelated env vars such as SSL_CERT_FILE out + return keyPath.length >= 2 && keyPath.every(segment => !!segment); +} + +/** + * Resolve `_FILE` env vars into config values by reading the file they point at, + * so secrets can be mounted (Docker/Swarm secrets, k8s projected volumes, systemd LoadCredential) + * instead of being passed as plaintext env vars. + * + * database__connection__password_FILE=/run/secrets/db_password + * + * The suffix is matched case-insensitively. + * + * Returns a plain object suitable for an nconf `literal` store. + */ +function loadSecretsFromEnv(env: NodeJS.ProcessEnv = process.env, separator: string = '__'): SecretStore { + const store: SecretStore = {}; + const seen = new Map(); + + for (const [name, filePath] of Object.entries(env)) { + if (!isSecretFileRef(name, separator) || !filePath) { + continue; + } + + const varName = name.slice(0, -SUFFIX.length); + + if (env[varName] !== undefined) { + // new Error is allowed here, as we do not want config to depend on @tryghost/error + // eslint-disable-next-line ghost/ghost-custom/no-native-error + throw new Error(`Cannot set both ${varName} and ${name} - use one or the other.`); + } + + const duplicate = seen.get(varName); + + if (duplicate) { + // env var names are case-sensitive on POSIX, so the same key can be set twice — + // only a conflict if they disagree about which file to read + if (duplicate.filePath !== filePath) { + // eslint-disable-next-line ghost/ghost-custom/no-native-error + throw new Error(`Cannot set both ${duplicate.name} and ${name} to different files - they resolve to the same config key.`); + } + + continue; + } + + seen.set(varName, {name, filePath}); + + let contents: string; + + try { + contents = fs.readFileSync(filePath, 'utf8'); + } catch (err) { + // eslint-disable-next-line ghost/ghost-custom/no-native-error + throw new Error(`Could not read the secret file referenced by ${name}: ${filePath}`, {cause: err}); + } + + // strip a single trailing newline, matching `$(cat file)` behaviour, but leave + // any other surrounding whitespace alone in case it is part of the secret + // setWith with Object keeps numeric key segments as plain objects rather than arrays + setWith(store, varName.split(separator), contents.replace(/\r?\n$/, ''), Object); + } + + return store; +} + +export {loadSecretsFromEnv, isSecretFileRef}; diff --git a/ghost/core/test/unit/shared/config/loader.test.js b/ghost/core/test/unit/shared/config/loader.test.js index 2949d8bc149..669f93864bc 100644 --- a/ghost/core/test/unit/shared/config/loader.test.js +++ b/ghost/core/test/unit/shared/config/loader.test.js @@ -1,4 +1,6 @@ const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); const path = require('path'); const _ = require('lodash'); const configUtils = require('../../../utils/config-utils'); @@ -17,10 +19,18 @@ describe('Config Loader', function () { let originalArgv; let customConfig; let loader; + let tmpDir; + + function writeSecret(contents) { + const filePath = path.join(tmpDir, 'secret'); + fs.writeFileSync(filePath, contents); + return filePath; + } beforeEach(function () { originalEnv = _.clone(process.env); originalArgv = _.clone(process.argv); + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ghost-loader-')); loader = require('../../../../core/shared/config/loader'); // getNodeEnv() reads process.env.NODE_ENV, so drive that directly process.env.NODE_ENV = 'testing'; @@ -35,6 +45,7 @@ describe('Config Loader', function () { afterEach(function () { process.env = originalEnv; process.argv = originalArgv; + fs.rmSync(tmpDir, {recursive: true, force: true}); sinon.restore(); }); @@ -61,6 +72,50 @@ describe('Config Loader', function () { assert.equal(customConfig.get('database:client'), 'stronger'); }); + it('secret file is stronger than file', function () { + process.env.logging__level_FILE = writeSecret('warn\n'); + + customConfig = loader.loadNconf({ + baseConfigPath: path.join(__dirname, '../../../utils/fixtures/config'), + customConfigPath: path.join(__dirname, '../../../utils/fixtures/config') + }); + + assert.equal(customConfig.get('logging:level'), 'warn'); + }); + + it('argv is stronger than a secret file', function () { + process.env.logging__level_FILE = writeSecret('warn\n'); + process.argv[2] = '--logging:level=stronger'; + + customConfig = loader.loadNconf({ + baseConfigPath: path.join(__dirname, '../../../utils/fixtures/config'), + customConfigPath: path.join(__dirname, '../../../utils/fixtures/config') + }); + + assert.equal(customConfig.get('logging:level'), 'stronger'); + }); + + it('does not leak the secret file path into config', function () { + process.env.database__connection__password_FILE = writeSecret('hunter2\n'); + + customConfig = loader.loadNconf({ + baseConfigPath: path.join(__dirname, '../../../utils/fixtures/config'), + customConfigPath: path.join(__dirname, '../../../utils/fixtures/config') + }); + + assert.equal(customConfig.get('database:connection:password_FILE'), undefined); + }); + + it('throws if a value and its secret file are both set', function () { + process.env.logging__level = 'warn'; + process.env.logging__level_FILE = writeSecret('error\n'); + + assert.throws(() => loader.loadNconf({ + baseConfigPath: path.join(__dirname, '../../../utils/fixtures/config'), + customConfigPath: path.join(__dirname, '../../../utils/fixtures/config') + }), /Cannot set both logging__level and logging__level_FILE/); + }); + it('argv or env is NOT stronger than overrides', function () { process.env.paths__corePath = 'try-to-override'; process.argv[2] = '--paths:corePath=try-to-override'; diff --git a/ghost/core/test/unit/shared/config/secrets.test.ts b/ghost/core/test/unit/shared/config/secrets.test.ts new file mode 100644 index 00000000000..0be4d19e569 --- /dev/null +++ b/ghost/core/test/unit/shared/config/secrets.test.ts @@ -0,0 +1,137 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import {loadSecretsFromEnv} from '../../../../core/shared/config/secrets'; + +describe('Config Secrets', function () { + let tmpDir: string; + + function writeSecret(name: string, contents: string): string { + const filePath = path.join(tmpDir, name); + fs.writeFileSync(filePath, contents); + return filePath; + } + + beforeEach(function () { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ghost-secrets-')); + }); + + afterEach(function () { + fs.rmSync(tmpDir, {recursive: true, force: true}); + }); + + it('resolves a nested key from a file', function () { + const filePath = writeSecret('db', 'hunter2'); + + assert.deepEqual(loadSecretsFromEnv({database__connection__password_FILE: filePath}), { + database: {connection: {password: 'hunter2'}} + }); + }); + + it('matches the suffix case-insensitively', function () { + const filePath = writeSecret('db', 'hunter2'); + + assert.deepEqual(loadSecretsFromEnv({database__connection__password_file: filePath}), { + database: {connection: {password: 'hunter2'}} + }); + }); + + it('does not parse values', function () { + const filePath = writeSecret('db', '01234'); + + assert.deepEqual(loadSecretsFromEnv({database__connection__password_FILE: filePath}), { + database: {connection: {password: '01234'}} + }); + }); + + it('strips a single trailing newline but keeps other whitespace', function () { + const trailing = writeSecret('trailing', 'hunter2\n'); + const surrounding = writeSecret('surrounding', ' hunter2 '); + const multiple = writeSecret('multiple', 'hunter2\n\n'); + + assert.deepEqual(loadSecretsFromEnv({a__b_FILE: trailing}), {a: {b: 'hunter2'}}); + assert.deepEqual(loadSecretsFromEnv({a__b_FILE: surrounding}), {a: {b: ' hunter2 '}}); + assert.deepEqual(loadSecretsFromEnv({a__b_FILE: multiple}), {a: {b: 'hunter2\n'}}); + }); + + it('merges multiple secrets under a shared parent', function () { + const user = writeSecret('user', 'ghost'); + const pass = writeSecret('pass', 'hunter2'); + + assert.deepEqual(loadSecretsFromEnv({ + mail__options__auth__user_FILE: user, + mail__options__auth__pass_FILE: pass + }), { + mail: {options: {auth: {user: 'ghost', pass: 'hunter2'}}} + }); + }); + + it('ignores env vars that are not nested config keys', function () { + const filePath = writeSecret('ca', 'not-a-secret'); + + assert.deepEqual(loadSecretsFromEnv({ + SSL_CERT_FILE: filePath, + CURL_CA_BUNDLE: filePath, + _FILE: filePath, + database____password_FILE: filePath + }), {}); + }); + + it('ignores env vars with an empty value', function () { + assert.deepEqual(loadSecretsFromEnv({database__connection__password_FILE: ''}), {}); + }); + + it('keeps numeric key segments as plain objects', function () { + const filePath = writeSecret('token', 'abc'); + const store = loadSecretsFromEnv({adapters__0__token_FILE: filePath}); + + assert.deepEqual(store, {adapters: {0: {token: 'abc'}}}); + assert.equal(Array.isArray(store.adapters), false); + }); + + it('throws if the value and its file reference are both set', function () { + const filePath = writeSecret('db', 'hunter2'); + + assert.throws(() => loadSecretsFromEnv({ + database__connection__password: 'hunter2', + database__connection__password_FILE: filePath + }), /Cannot set both database__connection__password and database__connection__password_FILE/); + }); + + it('allows the same key set twice if both point at the same file', function () { + const filePath = writeSecret('db', 'hunter2'); + + assert.deepEqual(loadSecretsFromEnv({ + database__connection__password_FILE: filePath, + database__connection__password_file: filePath + }), { + database: {connection: {password: 'hunter2'}} + }); + }); + + it('throws if the same key is set twice pointing at different files', function () { + assert.throws(() => loadSecretsFromEnv({ + database__connection__password_FILE: writeSecret('one', 'hunter2'), + database__connection__password_file: writeSecret('two', 'hunter3') + }), /to different files/); + }); + + it('throws if the file cannot be read', function () { + const filePath = path.join(tmpDir, 'nope'); + + assert.throws(() => loadSecretsFromEnv({database__connection__password_FILE: filePath}), (err: Error) => { + assert.match(err.message, /Could not read the secret file referenced by database__connection__password_FILE/); + assert.equal((err.cause as NodeJS.ErrnoException).code, 'ENOENT'); + return true; + }); + }); + + it('supports a custom separator', function () { + const filePath = writeSecret('db', 'hunter2'); + + assert.deepEqual(loadSecretsFromEnv({'database.connection.password_FILE': filePath}, '.'), { + database: {connection: {password: 'hunter2'}} + }); + }); +}); From 7481efc9509812c1d1d40715687be0329cf2d614 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 13 Aug 2026 13:00:20 +0100 Subject: [PATCH 04/25] Updated public app development commands (#29932) Some of the public app READMEs had stale information and were functionally incorrect. This is part of work to bring codebase docs into the codebase, and make sure the information is consistent, correct and coherent. --- apps/admin-toolbar/README.md | 14 ++++++++++---- apps/announcement-bar/README.md | 14 +++++++------- apps/comments-ui/README.md | 11 +++++++---- apps/signup-form/README.md | 31 +++++++++++-------------------- apps/sodo-search/README.md | 12 ++++++------ 5 files changed, 41 insertions(+), 41 deletions(-) diff --git a/apps/admin-toolbar/README.md b/apps/admin-toolbar/README.md index 18679b1eef5..cccf3cd41d5 100644 --- a/apps/admin-toolbar/README.md +++ b/apps/admin-toolbar/README.md @@ -7,18 +7,24 @@ scripts where bundle size matters more than ecosystem compatibility. ## Development +Run `pnpm dev:public` from the monorepo root to start the standard development +environment and the Admin Toolbar watcher. To work on this package by itself, +run these commands from this directory: + ```bash pnpm build # one-off build -pnpm dev # build + preview with watch (started automatically by pnpm dev from root) -pnpm test # build + run tests against UMD bundle +pnpm dev # watch and rebuild umd/admin-toolbar.min.js +pnpm test # build + run tests against the built bundle ``` ## How it's served In production, the script is loaded from jsDelivr via the `adminToolbar` config in `defaults.json`, following the same CDN pattern as portal, comments-ui, and -the other public apps. In development, the Docker Dockerfile overrides the URL -to proxy through Caddy to the local vite preview server on port 4176. +the other public apps. In development, `docker/ghost-dev/Dockerfile` overrides +that URL to `/ghost/assets/admin-toolbar/admin-toolbar.min.js`, which the dev +gateway serves straight off disk from this package's `umd/` directory — so the +watcher's output is picked up on the next request. # Copyright & License diff --git a/apps/announcement-bar/README.md b/apps/announcement-bar/README.md index ad56d5839fa..31402c71829 100644 --- a/apps/announcement-bar/README.md +++ b/apps/announcement-bar/README.md @@ -4,17 +4,17 @@ ### Pre-requisites -- Run `pnpm` in Ghost monorepo root -- Run `pnpm` in this directory +- Run `pnpm setup` in the Ghost monorepo root -### Running via Ghost `pnpm dev` in root folder +### Running via Ghost from the monorepo root + +Start Ghost with the public-app watchers enabled: -Announcement Bar runs automatically when using Ghost's development command from the monorepo root: ```bash -pnpm dev +pnpm dev:public ``` -This starts all frontend apps (including Announcement Bar.) +This starts the standard development environment and the Announcement Bar watcher. To run only the package's build watcher, use `pnpm dev` from this directory. ## Release @@ -32,7 +32,7 @@ In either case, you need sufficient permissions to release `@tryghost` packages 2. Merge the release commit to `main` 3. Wait until a new version of Ghost is released -To use the new version of signup form in Ghost, update the version in Ghost core's default configuration (currently at `core/shared/config/default.json`) +To use the new version of Announcement Bar in Ghost, update the version in Ghost core's default configuration (currently at `core/shared/config/default.json`) # Copyright & License diff --git a/apps/comments-ui/README.md b/apps/comments-ui/README.md index 2df85697efe..223e54a7a54 100644 --- a/apps/comments-ui/README.md +++ b/apps/comments-ui/README.md @@ -6,15 +6,18 @@ Comments widget that is embedded at the bottom of posts in Ghost. ### Pre-requisites -- Run `pnpm` in Ghost monorepo root +- Run `pnpm setup` in the Ghost monorepo root -### Running via Ghost `pnpm dev` in root folder +### Running via Ghost from the monorepo root + +Start Ghost with the public-app watchers enabled: -Comments UI runs automatically when using Ghost's development command from the monorepo root: ```bash -pnpm dev +pnpm dev:public ``` +This starts the standard development environment and the Comments UI watcher. To run only the package's build watcher, use `pnpm dev` from this directory. + ## Release A patch release can be rolled out instantly in production, whereas a minor/major release requires the Ghost monorepo to be updated and released. In either case, you need sufficient permissions to release `@tryghost` packages on NPM. diff --git a/apps/signup-form/README.md b/apps/signup-form/README.md index bb2106ee9c5..b5562b86102 100644 --- a/apps/signup-form/README.md +++ b/apps/signup-form/README.md @@ -6,46 +6,37 @@ Embed a Ghost signup form on any site. ### Pre-requisites -- Run `pnpm` in Ghost monorepo root -- Run `pnpm` in this directory +- Run `pnpm setup` in the Ghost monorepo root -### Running via Ghost `pnpm dev` in root folder +### Running via Ghost from the monorepo root + +Start Ghost with the public-app watchers enabled: -Signup Form runs automatically when using Ghost's development command from the monorepo root: ```bash -pnpm dev +pnpm dev:public ``` -This starts all frontend apps (including Signup Form.) +This starts the standard development environment and the Signup Form watcher. ### Running the standalone demo page Run `pnpm dev:standalone` (in this package folder) to start the standalone development server with HMR for testing/developing the form in isolation. - This serves the demo page at http://localhost:6173 -`pnpm dev` on its own (in this package folder) only builds `umd/signup-form.min.js` and watches for changes — it does not bind a port. The UMD is served by Caddy at http://localhost:2368/ghost/assets/signup-form/signup-form.min.js when you run `pnpm dev` from the monorepo root. +`pnpm dev` on its own (in this package folder) only builds `umd/signup-form.min.js` and watches for changes — it does not bind a port. The UMD is served by Caddy at http://localhost:2368/ghost/assets/signup-form/signup-form.min.js when you run `pnpm dev:public` from the monorepo root. ### Using the UMD build during development Vite by default only supports HRM with an ESM output. But when loading a script on a site as a ESM module (`'); - giftServiceWrapper.service = { - getPreview: sinon.stub().resolves({ - tier: {id: 'tier_1', name: 'Premium'}, - cadence: 'month', - duration: 3 - }) - }; + giftService.getPreview.resolves({ + tier: {id: 'tier_1', name: 'Premium'}, + cadence: 'month', + duration: 3 + }); await controller.giftPreview(req, res); @@ -113,13 +121,11 @@ describe('Gift Preview Controller', function () { }); it('uses monthly cadence label', async function () { - giftServiceWrapper.service = { - getPreview: sinon.stub().resolves({ - tier: {id: 'tier_1', name: 'Premium'}, - cadence: 'month', - duration: 3 - }) - }; + giftService.getPreview.resolves({ + tier: {id: 'tier_1', name: 'Premium'}, + cadence: 'month', + duration: 3 + }); await controller.giftPreview(req, res); @@ -131,13 +137,11 @@ describe('Gift Preview Controller', function () { it('defaults site title to Ghost', async function () { settingsCache.get.withArgs('title').returns(null); - giftServiceWrapper.service = { - getPreview: sinon.stub().resolves({ - tier: {id: 'tier_1', name: 'Premium'}, - cadence: 'year', - duration: 1 - }) - }; + giftService.getPreview.resolves({ + tier: {id: 'tier_1', name: 'Premium'}, + cadence: 'year', + duration: 1 + }); await controller.giftPreview(req, res); @@ -149,13 +153,11 @@ describe('Gift Preview Controller', function () { describe('giftPreviewImage', function () { it('returns a PNG image for a valid gift', async function () { - giftServiceWrapper.service = { - getPreview: sinon.stub().resolves({ - tier: {id: 'tier_1', name: 'Gold'}, - cadence: 'year', - duration: 1 - }) - }; + giftService.getPreview.resolves({ + tier: {id: 'tier_1', name: 'Gold'}, + cadence: 'year', + duration: 1 + }); await controller.giftPreviewImage(req, res); From ec53d7394c3fbddf63cb85f7d0262cea17e55c27 Mon Sep 17 00:00:00 2001 From: Rob Lester Date: Tue, 4 Aug 2026 17:56:22 +0100 Subject: [PATCH 22/25] =?UTF-8?q?=E2=9C=A8=20Released=20the=20React=20memb?= =?UTF-8?q?er=20details=20screen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ref https://linear.app/ghost/issue/BER-3848 The React member details screen has until now only been reachable per-site behind a Labs toggle, and self-hosted sites never read the remote flag manifest that switched it on for Ghost(Pro), so promoting the flag to generally available is what actually puts every site on the React screen, while Ghost(Pro) keeps its kill switch because remote overrides still sit above the GA list. Several browser tests had never enabled the flag and so had always exercised the Ember screen, encoding its behaviour: its straight apostrophe, its heading structure, its clickable label, a one-click enable control React does not offer, and its habit of letting an invalid email reach the server and of settling on a "Saved" state after creating a member. Those tests are brought into line with the screen that now actually serves them. --- .../members/detail/member-activity-feed.tsx | 8 +- .../advanced/labs/private-features.tsx | 4 - .../admin/members/member-details-page.ts | 12 +- .../members-legacy/disable-commenting.test.ts | 23 +- .../admin/members-legacy/members.test.ts | 18 +- .../members/import-custom-fields.test.ts | 5 +- .../members/member-custom-fields.test.ts | 2 +- e2e/tests/admin/members/member-detail.test.ts | 885 ++++++++---------- ghost/core/core/shared/labs.js | 4 +- 9 files changed, 424 insertions(+), 537 deletions(-) diff --git a/apps/admin/src/members/detail/member-activity-feed.tsx b/apps/admin/src/members/detail/member-activity-feed.tsx index c2bc2329b06..9ecf3445a53 100644 --- a/apps/admin/src/members/detail/member-activity-feed.tsx +++ b/apps/admin/src/members/detail/member-activity-feed.tsx @@ -185,8 +185,8 @@ const MemberActivityFeed: React.FC = ({memberId, hasMul // `EmptyIndicator` so the parity assertion for that string holds. if (!memberId) { return ( -
-

Activity

+
+

Activity

{/* Same wrapper padding as the Subscriptions empty state (`member-subscriptions-section.tsx`) so both cards line @@ -204,8 +204,8 @@ const MemberActivityFeed: React.FC = ({memberId, hasMul } return ( -
-

Activity

+
+

Activity

{isLoading ? ( diff --git a/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx b/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx index d95a68b339f..d1fc22aa5b1 100644 --- a/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx +++ b/apps/admin/src/settings/app/components/settings/advanced/labs/private-features.tsx @@ -55,10 +55,6 @@ const features: Feature[] = [{ title: 'Get helper deduplication', description: 'Deduplicate identical {{#get}} helper queries within a single request to avoid redundant database calls', flag: 'getHelperDeduplication' -}, { - title: 'React member details', - description: 'Renders the member detail screen (/members/:id) from the React app instead of the Ember screen. Gates the migration behind a runtime toggle so we can compare both implementations.', - flag: 'memberDetailsReact' }, { title: 'React tag details', description: 'Renders the tag detail screen (/tags/:slug) from the React app instead of the Ember screen. Gates the migration behind a runtime toggle so we can compare both implementations.', diff --git a/e2e/helpers/pages/admin/members/member-details-page.ts b/e2e/helpers/pages/admin/members/member-details-page.ts index cd62314b20e..54c8ed4c9b6 100644 --- a/e2e/helpers/pages/admin/members/member-details-page.ts +++ b/e2e/helpers/pages/admin/members/member-details-page.ts @@ -52,7 +52,6 @@ export class MemberDetailsPage extends AdminPage { readonly saveButton: Locator; readonly savedButton: Locator; - readonly retryButton: Locator; readonly membersBackLink: Locator; readonly copyLinkButton: Locator; @@ -61,14 +60,13 @@ export class MemberDetailsPage extends AdminPage { readonly confirmLeaveButton: Locator; readonly settingsSection: SettingsSection; - readonly activityHeading: Locator; + readonly activityFeed: Locator; readonly disableCommentingModal: Locator; readonly disableCommentingConfirmButton: Locator; readonly disableCommentingCancelButton: Locator; readonly hideCommentsCheckbox: Locator; readonly commentingDisabledIndicator: Locator; - readonly enableCommentingLink: Locator; readonly screenTitle: Locator; readonly logoutConfirmModal: Locator; @@ -104,21 +102,19 @@ export class MemberDetailsPage extends AdminPage { this.saveButton = page.getByRole('button', {name: 'Save'}); this.savedButton = page.getByRole('button', {name: 'Saved'}); - this.retryButton = page.getByRole('button', {name: 'Retry'}); this.membersBackLink = page.locator('[data-test-link="members-back"]').filter({visible: true}); this.copyLinkButton = page.getByRole('button', {name: 'Copy link'}); this.magicLinkInput = page.getByTestId('member-signin-url').filter({visible: true}); this.confirmLeaveButton = page.getByRole('button', {name: 'Leave'}); this.settingsSection = new SettingsSection(page); - this.activityHeading = page.getByRole('heading', {name: 'Activity', level: 4}); + this.activityFeed = page.getByRole('region', {name: 'Activity'}); this.disableCommentingModal = page.getByRole('dialog'); this.disableCommentingConfirmButton = this.disableCommentingModal.getByRole('button', {name: 'Disable commenting'}); this.disableCommentingCancelButton = this.disableCommentingModal.getByRole('button', {name: 'Cancel'}); - this.hideCommentsCheckbox = this.disableCommentingModal.getByText('Hide all previous comments'); + this.hideCommentsCheckbox = this.disableCommentingModal.getByRole('switch', {name: 'Hide all previous comments'}); this.commentingDisabledIndicator = page.getByText('Comments disabled'); - this.enableCommentingLink = page.getByRole('button', {name: 'Enable', exact: true}); this.screenTitle = page.locator('[data-test-screen-title]') .or(page.getByTestId('member-detail-title')) @@ -227,6 +223,6 @@ export class MemberDetailsPage extends AdminPage { } getActivityEventByText(text: string | RegExp): Locator { - return this.activityHeading.locator('..').getByText(text); + return this.activityFeed.getByText(text); } } diff --git a/e2e/tests/admin/members-legacy/disable-commenting.test.ts b/e2e/tests/admin/members-legacy/disable-commenting.test.ts index b9ef85d92e8..fc8a4881ce8 100644 --- a/e2e/tests/admin/members-legacy/disable-commenting.test.ts +++ b/e2e/tests/admin/members-legacy/disable-commenting.test.ts @@ -36,7 +36,9 @@ test.describe('Ghost Admin - Member Detail Disable Commenting', () => { await expect(memberDetailsPage.disableCommentingModal).toBeVisible(); await expect(memberDetailsPage.disableCommentingModal).toContainText('Test Member'); - await expect(memberDetailsPage.disableCommentingModal).toContainText('won\'t be able to comment'); + // Either apostrophe: the warning is what matters, not the glyph the + // copy happens to use. + await expect(memberDetailsPage.disableCommentingModal).toContainText(/won['’]t be able to comment/); await expect(memberDetailsPage.disableCommentingConfirmButton).toBeVisible(); await expect(memberDetailsPage.disableCommentingCancelButton).toBeVisible(); }); @@ -147,25 +149,6 @@ test.describe('Ghost Admin - Member Detail Disable Commenting', () => { await expect(memberDetailsPage.settingsSection.disableCommentingButton).toBeVisible(); await expect(memberDetailsPage.settingsSection.enableCommentingButton).toBeHidden(); }); - - test('enabling via sidebar link removes indicator', async ({page}) => { - const {name} = await memberFactory.create(); - - const membersPage = new MembersPage(page); - await membersPage.goto(); - await membersPage.getMemberByName(name!).click(); - - const memberDetailsPage = new MemberDetailsPage(page); - - await memberDetailsPage.settingsSection.memberActionsButton.click(); - await memberDetailsPage.settingsSection.disableCommentingButton.click(); - await memberDetailsPage.disableCommentingConfirmButton.click(); - await expect(memberDetailsPage.commentingDisabledIndicator).toBeVisible(); - - await memberDetailsPage.enableCommentingLink.click(); - - await expect(memberDetailsPage.commentingDisabledIndicator).toBeHidden(); - }); }); }); diff --git a/e2e/tests/admin/members-legacy/members.test.ts b/e2e/tests/admin/members-legacy/members.test.ts index 9274c25d49c..e75bc4cc7b5 100644 --- a/e2e/tests/admin/members-legacy/members.test.ts +++ b/e2e/tests/admin/members-legacy/members.test.ts @@ -21,7 +21,10 @@ test.describe('Ghost Admin - Legacy Member Detail Flows', () => { const memberDetailsPage = new MemberDetailsPage(page); await memberDetailsPage.fillMemberDetails(memberToCreate.name!, memberToCreate.email, memberToCreate.note!); - await memberDetailsPage.save(); + await memberDetailsPage.saveButton.click(); + // Creating redirects to the new member's own URL; the Save button never + // settles on "Saved" here, so the redirect is the completion signal. + await expect(page).toHaveURL(/#\/members\/[0-9a-f]{24}/); await membersPage.goto(); @@ -38,10 +41,8 @@ test.describe('Ghost Admin - Legacy Member Detail Flows', () => { const memberDetailsPage = new MemberDetailsPage(page); await memberDetailsPage.fillMemberDetails(memberToCreate.name!, memberToCreate.email, memberToCreate.note!); - await memberDetailsPage.saveButton.click(); - await expect(memberDetailsPage.retryButton).toBeVisible(); - await expect(memberDetailsPage.body).toContainText('Invalid Email'); + await expect(memberDetailsPage.saveButton).toBeDisabled(); }); test('updates an existing member', async ({page}) => { @@ -83,11 +84,14 @@ test.describe('Ghost Admin - Legacy Member Detail Flows', () => { await membersPage.openMemberByName(memberToEdit.name!); const memberDetailsPage = new MemberDetailsPage(page); + // Save also starts disabled while the form is untouched, so a valid edit has to + // enable it first for the assertion below to be about the email at all. + await memberDetailsPage.nameInput.fill('Test Member Edited'); + await expect(memberDetailsPage.saveButton).toBeEnabled(); + await memberDetailsPage.emailInput.fill('invalid-email-address'); - await memberDetailsPage.saveButton.click(); - await expect(memberDetailsPage.retryButton).toBeVisible(); - await expect(memberDetailsPage.body).toContainText('Invalid Email'); + await expect(memberDetailsPage.saveButton).toBeDisabled(); }); test('deletes an existing member', async ({page}) => { diff --git a/e2e/tests/admin/members/import-custom-fields.test.ts b/e2e/tests/admin/members/import-custom-fields.test.ts index aad71ae8883..e47d82d1f78 100644 --- a/e2e/tests/admin/members/import-custom-fields.test.ts +++ b/e2e/tests/admin/members/import-custom-fields.test.ts @@ -13,13 +13,12 @@ import {usePerTestIsolation} from '@/helpers/playwright/isolation'; * two things only the browser exercises -- the export -> import loop end to end, and the * mapping step both auto-detecting an exported column and taking a hand-picked target. * - * Behind membersCustomFields (the whole feature) and memberDetailsReact (the member detail - * screen that renders a field's value). + * Behind membersCustomFields, which gates the whole feature. */ usePerTestIsolation(); test.describe('Ghost Admin - Members import with custom fields', () => { - test.use({labs: {membersCustomFields: true, memberDetailsReact: true}}); + test.use({labs: {membersCustomFields: true}}); test('an exported custom field value round-trips back through import, auto-mapped', async ({page}) => { const ts = Date.now(); diff --git a/e2e/tests/admin/members/member-custom-fields.test.ts b/e2e/tests/admin/members/member-custom-fields.test.ts index c200a92fdf8..9171b316b56 100644 --- a/e2e/tests/admin/members/member-custom-fields.test.ts +++ b/e2e/tests/admin/members/member-custom-fields.test.ts @@ -16,7 +16,7 @@ import {usePerTestIsolation} from '@/helpers/playwright/isolation'; usePerTestIsolation(); test.describe('Ghost Admin - Member custom fields', () => { - test.use({labs: {membersCustomFields: true, memberDetailsReact: true}}); + test.use({labs: {membersCustomFields: true}}); test('a field defined in settings takes a value on a member and persists it', async ({page}) => { const fieldName = `Job title ${Date.now()}`; diff --git a/e2e/tests/admin/members/member-detail.test.ts b/e2e/tests/admin/members/member-detail.test.ts index 8f19f0917b4..cc449ca7ff9 100644 --- a/e2e/tests/admin/members/member-detail.test.ts +++ b/e2e/tests/admin/members/member-detail.test.ts @@ -7,16 +7,10 @@ import {usePerTestIsolation} from '@/helpers/playwright/isolation'; /** * Behaviour contract for `/members/:id`. * - * Ember and React implementations of this screen coexist behind the - * `memberDetailsReact` Labs flag, and this file runs the same assertions - * against both by generating one describe block per flag state. No test body - * knows which implementation is rendering it, and none may branch on it. - * - * A test that passes under one block and fails under the other is a real - * user-facing difference between the two screens. That is the point: it makes - * a gap impossible to hide behind a conditional. - * - * Anything true of only one implementation does not belong here. + * The assertions here were written to run against both the Ember and the React + * screen, so they describe what the screen does rather than how it is built. + * Keep them that way: a test that reaches for markup specific to the current + * implementation stops being a contract and starts being a snapshot. */ usePerTestIsolation(); @@ -173,424 +167,414 @@ const captureWrite = async (page: Page, urlPattern: string | RegExp, onCapture?: return sent; }; -for (const {implementation, memberDetailsReact} of [ - {implementation: 'Ember', memberDetailsReact: false}, - {implementation: 'React', memberDetailsReact: true} -] as const) { - test.describe(`Ghost Admin - Member Detail (${implementation})`, () => { - test.use({labs: {memberDetailsReact}}); +test.describe('Ghost Admin - Member Detail', () => { + let memberFactory: MemberFactory; + let memberDetailsPage: MemberDetailsPage; - let memberFactory: MemberFactory; - let memberDetailsPage: MemberDetailsPage; + test.beforeEach(async ({page}) => { + memberFactory = createMemberFactory(page.request); + memberDetailsPage = new MemberDetailsPage(page); + }); - test.beforeEach(async ({page}) => { - memberFactory = createMemberFactory(page.request); - memberDetailsPage = new MemberDetailsPage(page); - }); + test('member name renders in the screen title', async ({page}) => { + const member = await memberFactory.create({name: 'Ada Lovelace', email: 'ada-detail@ghost.org'}); - test('member name renders in the screen title', async ({page}) => { - const member = await memberFactory.create({name: 'Ada Lovelace', email: 'ada-detail@ghost.org'}); + await page.goto(memberPath(member.id)); - await page.goto(memberPath(member.id)); + await expect(memberDetailsPage.screenTitle).toContainText('Ada Lovelace'); + }); - await expect(memberDetailsPage.screenTitle).toContainText('Ada Lovelace'); - }); + test('editing the member name - persists to the server', async ({page}) => { + const member = await memberFactory.create({name: 'Grace', email: 'grace-detail@ghost.org'}); - test('editing the member name - persists to the server', async ({page}) => { - const member = await memberFactory.create({name: 'Grace', email: 'grace-detail@ghost.org'}); + await page.goto(memberPath(member.id)); + await memberDetailsPage.nameInput.fill('Grace Hopper'); + await memberDetailsPage.saveButton.click(); - await page.goto(memberPath(member.id)); - await memberDetailsPage.nameInput.fill('Grace Hopper'); - await memberDetailsPage.saveButton.click(); - - await expect.poll(async () => { - const res = await page.request.get(`/ghost/api/admin/members/${member.id}/`); - const body = await res.json(); - return body?.members?.[0]?.name; - }, {timeout: 10000}).toBe('Grace Hopper'); - }); + await expect.poll(async () => { + const res = await page.request.get(`/ghost/api/admin/members/${member.id}/`); + const body = await res.json(); + return body?.members?.[0]?.name; + }, {timeout: 10000}).toBe('Grace Hopper'); + }); - test('clicking the back link - returns to the members list', async ({page}) => { - const member = await memberFactory.create({name: 'Grace Hopper', email: 'grace-back-detail@ghost.org'}); + test('clicking the back link - returns to the members list', async ({page}) => { + const member = await memberFactory.create({name: 'Grace Hopper', email: 'grace-back-detail@ghost.org'}); - await page.goto(memberPath(member.id)); - await memberDetailsPage.membersBackLink.click(); + await page.goto(memberPath(member.id)); + await memberDetailsPage.membersBackLink.click(); - await expect(page).toHaveURL(/#\/members$/); - }); + await expect(page).toHaveURL(/#\/members$/); + }); - test('impersonation modal - exposes a real signin url', async ({page}) => { - const member = await memberFactory.create({name: 'Alan Turing', email: 'alan-detail@ghost.org'}); + test('impersonation modal - exposes a real signin url', async ({page}) => { + const member = await memberFactory.create({name: 'Alan Turing', email: 'alan-detail@ghost.org'}); - await page.goto(memberPath(member.id)); - await memberDetailsPage.settingsSection.memberActionsButton.click(); - await memberDetailsPage.settingsSection.impersonateButton.click(); + await page.goto(memberPath(member.id)); + await memberDetailsPage.settingsSection.memberActionsButton.click(); + await memberDetailsPage.settingsSection.impersonateButton.click(); - // The url is fetched after the modal opens, so assert on the value to - // let Playwright wait rather than reading the empty initial state. - await expect(memberDetailsPage.magicLinkInput).toHaveValue(/^https?:\/\/.+/); - }); + // The url is fetched after the modal opens, so assert on the value to + // let Playwright wait rather than reading the empty initial state. + await expect(memberDetailsPage.magicLinkInput).toHaveValue(/^https?:\/\/.+/); + }); - test('signing out of all devices - closes the confirmation and stays on the member', async ({page}) => { - const member = await memberFactory.create({name: 'Rear Admiral', email: 'rear-detail@ghost.org'}); + test('signing out of all devices - closes the confirmation and stays on the member', async ({page}) => { + const member = await memberFactory.create({name: 'Rear Admiral', email: 'rear-detail@ghost.org'}); - await page.goto(memberPath(member.id)); - await memberDetailsPage.settingsSection.memberActionsButton.click(); - await memberDetailsPage.settingsSection.signOutOfAllDevices.click(); - // Scoped to the modal so the click can't hit the account-owner - // "Sign out" button in the admin sidebar dropdown. - await memberDetailsPage.logoutConfirmModal.getByRole('button', {name: 'Sign out', exact: true}).click(); - - await expect(memberDetailsPage.logoutConfirmModal).toHaveCount(0); - await expect(page).toHaveURL(new RegExp(`#/members/${member.id}`)); - }); + await page.goto(memberPath(member.id)); + await memberDetailsPage.settingsSection.memberActionsButton.click(); + await memberDetailsPage.settingsSection.signOutOfAllDevices.click(); + // Scoped to the modal so the click can't hit the account-owner + // "Sign out" button in the admin sidebar dropdown. + await memberDetailsPage.logoutConfirmModal.getByRole('button', {name: 'Sign out', exact: true}).click(); - test('deleting a member - returns to the list and removes the record', async ({page}) => { - const member = await memberFactory.create({name: 'Deletable', email: 'delete-detail@ghost.org'}); + await expect(memberDetailsPage.logoutConfirmModal).toHaveCount(0); + await expect(page).toHaveURL(new RegExp(`#/members/${member.id}`)); + }); - await page.goto(memberPath(member.id)); - await memberDetailsPage.settingsSection.memberActionsButton.click(); - await memberDetailsPage.settingsSection.deleteButton.click(); - await memberDetailsPage.settingsSection.confirmDeleteButton.click(); + test('deleting a member - returns to the list and removes the record', async ({page}) => { + const member = await memberFactory.create({name: 'Deletable', email: 'delete-detail@ghost.org'}); - await expect(page).toHaveURL(/#\/members$/); - const res = await page.request.get(`/ghost/api/admin/members/${member.id}/`); - expect(res.status()).toBe(404); - }); + await page.goto(memberPath(member.id)); + await memberDetailsPage.settingsSection.memberActionsButton.click(); + await memberDetailsPage.settingsSection.deleteButton.click(); + await memberDetailsPage.settingsSection.confirmDeleteButton.click(); - test('creating a member - persists to the server and redirects to the detail', async ({page}) => { - const email = 'new-member-detail@ghost.org'; + await expect(page).toHaveURL(/#\/members$/); + const res = await page.request.get(`/ghost/api/admin/members/${member.id}/`); + expect(res.status()).toBe(404); + }); - await page.goto(memberPath('new')); - await memberDetailsPage.nameInput.fill('New Member'); - await memberDetailsPage.emailInput.fill(email); - await memberDetailsPage.saveButton.click(); - - let createdId: string | undefined; - await expect.poll(async () => { - const res = await page.request.get(`/ghost/api/admin/members/?filter=${encodeURIComponent(`email:'${email}'`)}`); - const body = await res.json(); - createdId = body?.members?.[0]?.id; - return body?.members?.[0]?.name; - }, {timeout: 10000}).toBe('New Member'); - await expect(page).toHaveURL(new RegExp(`#/members/${createdId}(\\?|$)`)); - }); + test('creating a member - persists to the server and redirects to the detail', async ({page}) => { + const email = 'new-member-detail@ghost.org'; + + await page.goto(memberPath('new')); + await memberDetailsPage.nameInput.fill('New Member'); + await memberDetailsPage.emailInput.fill(email); + await memberDetailsPage.saveButton.click(); + + let createdId: string | undefined; + await expect.poll(async () => { + const res = await page.request.get(`/ghost/api/admin/members/?filter=${encodeURIComponent(`email:'${email}'`)}`); + const body = await res.json(); + createdId = body?.members?.[0]?.id; + return body?.members?.[0]?.name; + }, {timeout: 10000}).toBe('New Member'); + await expect(page).toHaveURL(new RegExp(`#/members/${createdId}(\\?|$)`)); + }); - test('disabling then re-enabling commenting - clears the disabled indicator', async ({page}) => { - const member = await memberFactory.create({name: 'Commenter', email: 'commenter-toggle@ghost.org'}); + test('disabling then re-enabling commenting - clears the disabled indicator', async ({page}) => { + const member = await memberFactory.create({name: 'Commenter', email: 'commenter-toggle@ghost.org'}); - await page.goto(memberPath(member.id)); - await memberDetailsPage.settingsSection.memberActionsButton.click(); - await memberDetailsPage.settingsSection.disableCommentingButton.click(); - await memberDetailsPage.disableCommentingConfirmButton.click(); + await page.goto(memberPath(member.id)); + await memberDetailsPage.settingsSection.memberActionsButton.click(); + await memberDetailsPage.settingsSection.disableCommentingButton.click(); + await memberDetailsPage.disableCommentingConfirmButton.click(); - await expect(memberDetailsPage.commentingDisabledIndicator).toBeVisible(); + await expect(memberDetailsPage.commentingDisabledIndicator).toBeVisible(); - await memberDetailsPage.settingsSection.memberActionsButton.click(); - await memberDetailsPage.settingsSection.enableCommentingButton.click(); + await memberDetailsPage.settingsSection.memberActionsButton.click(); + await memberDetailsPage.settingsSection.enableCommentingButton.click(); - await expect(memberDetailsPage.commentingDisabledIndicator).toBeHidden(); - }); + await expect(memberDetailsPage.commentingDisabledIndicator).toBeHidden(); + }); - test('sidebar - shows the signup location and created date', async ({page}) => { - // Members created through the API carry no geolocation, so the - // location falls back deterministically. - const member = await memberFactory.create({name: 'Katherine Johnson', email: 'katherine-sidebar@ghost.org'}); + test('sidebar - shows the signup location and created date', async ({page}) => { + // Members created through the API carry no geolocation, so the + // location falls back deterministically. + const member = await memberFactory.create({name: 'Katherine Johnson', email: 'katherine-sidebar@ghost.org'}); - await page.goto(memberPath(member.id)); + await page.goto(memberPath(member.id)); - await expect(page.getByText('Unknown location')).toBeVisible(); - await expect(page.getByText(/Created/)).toBeVisible(); - }); + await expect(page.getByText('Unknown location')).toBeVisible(); + await expect(page.getByText(/Created/)).toBeVisible(); + }); - test('leaving with unsaved changes - warns before navigating away', async ({page}) => { - const member = await memberFactory.create({name: 'Grace Hopper', email: 'grace-unsaved@ghost.org'}); + test('leaving with unsaved changes - warns before navigating away', async ({page}) => { + const member = await memberFactory.create({name: 'Grace Hopper', email: 'grace-unsaved@ghost.org'}); - await page.goto(memberPath(member.id)); - await memberDetailsPage.nameInput.fill('Grace B. Hopper'); - await memberDetailsPage.membersBackLink.click(); + await page.goto(memberPath(member.id)); + await memberDetailsPage.nameInput.fill('Grace B. Hopper'); + await memberDetailsPage.membersBackLink.click(); - await expect(memberDetailsPage.confirmLeaveButton).toBeVisible(); - await memberDetailsPage.confirmLeaveButton.click(); - await expect(page).toHaveURL(/#\/members$/); - }); + await expect(memberDetailsPage.confirmLeaveButton).toBeVisible(); + await memberDetailsPage.confirmLeaveButton.click(); + await expect(page).toHaveURL(/#\/members$/); + }); - test('leaving with unsaved changes via the sidebar - warns before navigating away', async ({page}) => { - // The sidebar navigates with native hash anchors rather than - // client-side router links, so it exercises a different guard path - // than the back link above; both must warn. - const sidebar = new SidebarPage(page); - const member = await memberFactory.create({name: 'Grace Hopper', email: 'grace-unsaved-sidebar@ghost.org'}); + test('leaving with unsaved changes via the sidebar - warns before navigating away', async ({page}) => { + // The sidebar navigates with native hash anchors rather than + // client-side router links, so it exercises a different guard path + // than the back link above; both must warn. + const sidebar = new SidebarPage(page); + const member = await memberFactory.create({name: 'Grace Hopper', email: 'grace-unsaved-sidebar@ghost.org'}); - await page.goto(memberPath(member.id)); - await memberDetailsPage.nameInput.fill('Grace B. Hopper'); - await sidebar.getNavLink('Members').click(); + await page.goto(memberPath(member.id)); + await memberDetailsPage.nameInput.fill('Grace B. Hopper'); + await sidebar.getNavLink('Members').click(); - await expect(memberDetailsPage.confirmLeaveButton).toBeVisible(); - await memberDetailsPage.confirmLeaveButton.click(); - await expect(page).toHaveURL(/#\/members$/); - }); + await expect(memberDetailsPage.confirmLeaveButton).toBeVisible(); + await memberDetailsPage.confirmLeaveButton.click(); + await expect(page).toHaveURL(/#\/members$/); + }); - test('toggling a newsletter - persists the new state', async ({page}) => { - const member = await memberFactory.create({name: 'Newsletter Test', email: 'newsletter-toggle@ghost.org'}); + test('toggling a newsletter - persists the new state', async ({page}) => { + const member = await memberFactory.create({name: 'Newsletter Test', email: 'newsletter-toggle@ghost.org'}); - await page.goto(memberPath(member.id)); - // Wait on the toggle, not the checkbox — Ember hides the real input - // behind a styled span, so the control is never visible itself. - await expect(memberDetailsPage.newsletterSubscriptionToggles.first()).toBeVisible(); - const initiallyChecked = await memberDetailsPage.newsletterSubscriptionCheckboxes.first().isChecked(); + await page.goto(memberPath(member.id)); + // Wait on the toggle, not the checkbox — Ember hides the real input + // behind a styled span, so the control is never visible itself. + await expect(memberDetailsPage.newsletterSubscriptionToggles.first()).toBeVisible(); + const initiallyChecked = await memberDetailsPage.newsletterSubscriptionCheckboxes.first().isChecked(); - await memberDetailsPage.newsletterSubscriptionToggles.first().click(); - await memberDetailsPage.save(); - await page.reload(); + await memberDetailsPage.newsletterSubscriptionToggles.first().click(); + await memberDetailsPage.save(); + await page.reload(); - await expect(memberDetailsPage.newsletterSubscriptionCheckboxes.first()).toBeChecked({checked: !initiallyChecked}); - }); + await expect(memberDetailsPage.newsletterSubscriptionCheckboxes.first()).toBeChecked({checked: !initiallyChecked}); + }); - test('activity feed - view-all link points at this members full activity', async ({page}) => { - const member = await memberFactory.create({name: 'Activity Target', email: 'activity-viewall@ghost.org'}); - // A fresh member's signup event is written asynchronously, so serve a - // known event rather than racing it — the link only renders on the - // populated branch. - await page.route(/\/ghost\/api\/admin\/members\/events\/?\?/, async (route) => { - if (route.request().method() !== 'GET') { - return route.continue(); - } - return route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ - events: [{ - type: 'signup_event', - data: { - id: 'evt-1', - created_at: new Date(0).toISOString(), - member_id: member.id, - member: {id: member.id, name: member.name, email: member.email} - } - }], - meta: {pagination: {}} - }) - }); + test('activity feed - view-all link points at this members full activity', async ({page}) => { + const member = await memberFactory.create({name: 'Activity Target', email: 'activity-viewall@ghost.org'}); + // A fresh member's signup event is written asynchronously, so serve a + // known event rather than racing it — the link only renders on the + // populated branch. + await page.route(/\/ghost\/api\/admin\/members\/events\/?\?/, async (route) => { + if (route.request().method() !== 'GET') { + return route.continue(); + } + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + events: [{ + type: 'signup_event', + data: { + id: 'evt-1', + created_at: new Date(0).toISOString(), + member_id: member.id, + member: {id: member.id, name: member.name, email: member.email} + } + }], + meta: {pagination: {}} + }) }); + }); - await page.goto(memberPath(member.id)); + await page.goto(memberPath(member.id)); - const viewAll = page.getByRole('link', {name: /View all member activity/}); - await expect(viewAll).toBeVisible(); - await expect(viewAll).toHaveAttribute('href', new RegExp(`#/members-activity/?\\?member=${member.id}`)); - }); + const viewAll = page.getByRole('link', {name: /View all member activity/}); + await expect(viewAll).toBeVisible(); + await expect(viewAll).toHaveAttribute('href', new RegExp(`#/members-activity/?\\?member=${member.id}`)); + }); - test.describe('Subscriptions', () => { - // The Subscriptions section is gated on paid members being enabled. - test.use({stripeEnabled: true}); + test.describe('Subscriptions', () => { + // The Subscriptions section is gated on paid members being enabled. + test.use({stripeEnabled: true}); - test('paid subscription - shows the tier, price, interval and renewal date', async ({page}) => { - const member = await memberFactory.create({name: 'Paid Member', email: 'paid-sub@ghost.org'}); - await seedSubscriptions(page, member.id, 'paid', [paidSubscription()]); + test('paid subscription - shows the tier, price, interval and renewal date', async ({page}) => { + const member = await memberFactory.create({name: 'Paid Member', email: 'paid-sub@ghost.org'}); + await seedSubscriptions(page, member.id, 'paid', [paidSubscription()]); - await page.goto(memberPath(member.id)); + await page.goto(memberPath(member.id)); - await expect(page.getByText('Bronze').first()).toBeVisible(); - await expect(page.getByText('10.50').first()).toBeVisible(); - await expect(page.getByText(/month/i).first()).toBeVisible(); - await expect(page.getByText(/Renews 15 Feb 2026/).first()).toBeVisible(); - }); + await expect(page.getByText('Bronze').first()).toBeVisible(); + await expect(page.getByText('10.50').first()).toBeVisible(); + await expect(page.getByText(/month/i).first()).toBeVisible(); + await expect(page.getByText(/Renews 15 Feb 2026/).first()).toBeVisible(); + }); - test('subscription set to cancel - shows remaining access rather than a renewal', async ({page}) => { - const member = await memberFactory.create({name: 'Cancelling Member', email: 'cancelling-sub@ghost.org'}); - await seedSubscriptions(page, member.id, 'paid', [paidSubscription({cancel_at_period_end: true})]); + test('subscription set to cancel - shows remaining access rather than a renewal', async ({page}) => { + const member = await memberFactory.create({name: 'Cancelling Member', email: 'cancelling-sub@ghost.org'}); + await seedSubscriptions(page, member.id, 'paid', [paidSubscription({cancel_at_period_end: true})]); - await page.goto(memberPath(member.id)); + await page.goto(memberPath(member.id)); - await expect(page.getByText(/Has access until\s+15 Feb 2026/).first()).toBeVisible(); - await expect(page.getByText(/Renews/)).toHaveCount(0); - }); + await expect(page.getByText(/Has access until\s+15 Feb 2026/).first()).toBeVisible(); + await expect(page.getByText(/Renews/)).toHaveCount(0); + }); - test('complimentary subscription - shows the tier without a price', async ({page}) => { - const member = await memberFactory.create({name: 'Comp Member', email: 'comp-sub@ghost.org'}); - await seedSubscriptions(page, member.id, 'comped', [compSubscription()]); + test('complimentary subscription - shows the tier without a price', async ({page}) => { + const member = await memberFactory.create({name: 'Comp Member', email: 'comp-sub@ghost.org'}); + await seedSubscriptions(page, member.id, 'comped', [compSubscription()]); - await page.goto(memberPath(member.id)); + await page.goto(memberPath(member.id)); - await expect(page.getByText('Bronze').first()).toBeVisible(); - await expect(page.getByText(/Complimentary/i).first()).toBeVisible(); - }); + await expect(page.getByText('Bronze').first()).toBeVisible(); + await expect(page.getByText(/Complimentary/i).first()).toBeVisible(); + }); - test('gift subscription - offers no actions menu', async ({page}) => { - // A gift is bought by someone else and can't be cancelled or - // revoked from here, so the row deliberately has no menu. - const member = await memberFactory.create({name: 'Gift Member', email: 'gift-sub@ghost.org'}); - await seedSubscriptions(page, member.id, 'gift', [giftSubscription()]); + test('gift subscription - offers no actions menu', async ({page}) => { + // A gift is bought by someone else and can't be cancelled or + // revoked from here, so the row deliberately has no menu. + const member = await memberFactory.create({name: 'Gift Member', email: 'gift-sub@ghost.org'}); + await seedSubscriptions(page, member.id, 'gift', [giftSubscription()]); - await page.goto(memberPath(member.id)); + await page.goto(memberPath(member.id)); - await expect(page.getByText('Bronze').first()).toBeVisible(); - await expect(memberDetailsPage.subscriptionActionsButton).toHaveCount(0); - }); + await expect(page.getByText('Bronze').first()).toBeVisible(); + await expect(memberDetailsPage.subscriptionActionsButton).toHaveCount(0); + }); - test('cancelling a subscription - asks the server to cancel at period end', async ({page}) => { - const member = await memberFactory.create({name: 'Cancel Member', email: 'cancel-action@ghost.org'}); - const subs = [paidSubscription()]; - await seedSubscriptions(page, member.id, 'paid', subs); - // Reflect the write back into the seeded read so the screen can - // re-render from it, the way a real refetch would. - const sent = await captureWrite(page, `**/members/${member.id}/subscriptions/sub_paid_123/**`, (body) => { - subs[0].cancel_at_period_end = body.cancel_at_period_end as boolean; - }); - - await page.goto(memberPath(member.id)); - await memberDetailsPage.subscriptionActionsButton.click(); - await memberDetailsPage.cancelSubscriptionButton.click(); - - // The request is the contract — the server doesn't care which UI sent it. - await expect.poll(() => sent.body?.cancel_at_period_end).toBe(true); + test('cancelling a subscription - asks the server to cancel at period end', async ({page}) => { + const member = await memberFactory.create({name: 'Cancel Member', email: 'cancel-action@ghost.org'}); + const subs = [paidSubscription()]; + await seedSubscriptions(page, member.id, 'paid', subs); + // Reflect the write back into the seeded read so the screen can + // re-render from it, the way a real refetch would. + const sent = await captureWrite(page, `**/members/${member.id}/subscriptions/sub_paid_123/**`, (body) => { + subs[0].cancel_at_period_end = body.cancel_at_period_end as boolean; }); - test('continuing a cancelled subscription - asks the server to resume it', async ({page}) => { - const member = await memberFactory.create({name: 'Continue Member', email: 'continue-action@ghost.org'}); - const subs = [paidSubscription({cancel_at_period_end: true})]; - await seedSubscriptions(page, member.id, 'paid', subs); - const sent = await captureWrite(page, `**/members/${member.id}/subscriptions/sub_paid_123/**`, (body) => { - subs[0].cancel_at_period_end = body.cancel_at_period_end as boolean; - }); + await page.goto(memberPath(member.id)); + await memberDetailsPage.subscriptionActionsButton.click(); + await memberDetailsPage.cancelSubscriptionButton.click(); - await page.goto(memberPath(member.id)); - await memberDetailsPage.subscriptionActionsButton.click(); - await memberDetailsPage.continueSubscriptionButton.click(); + // The request is the contract — the server doesn't care which UI sent it. + await expect.poll(() => sent.body?.cancel_at_period_end).toBe(true); + }); - await expect.poll(() => sent.body?.cancel_at_period_end).toBe(false); + test('continuing a cancelled subscription - asks the server to resume it', async ({page}) => { + const member = await memberFactory.create({name: 'Continue Member', email: 'continue-action@ghost.org'}); + const subs = [paidSubscription({cancel_at_period_end: true})]; + await seedSubscriptions(page, member.id, 'paid', subs); + const sent = await captureWrite(page, `**/members/${member.id}/subscriptions/sub_paid_123/**`, (body) => { + subs[0].cancel_at_period_end = body.cancel_at_period_end as boolean; }); - test('removing a complimentary subscription - puts back only the surviving tiers', async ({page}) => { - const member = await memberFactory.create({name: 'Multi Comp', email: 'multi-comp@ghost.org'}); - await seedSubscriptions(page, member.id, 'comped', [compSubscription()], {tiers: TWO_COMP_TIERS()}); - const sent = await captureWrite(page, new RegExp(`/ghost/api/admin/members/${member.id}/\\??[^/]*$`)); + await page.goto(memberPath(member.id)); + await memberDetailsPage.subscriptionActionsButton.click(); + await memberDetailsPage.continueSubscriptionButton.click(); + + await expect.poll(() => sent.body?.cancel_at_period_end).toBe(false); + }); - await page.goto(memberPath(member.id)); - await memberDetailsPage.removeComplimentarySubscription(); + test('removing a complimentary subscription - puts back only the surviving tiers', async ({page}) => { + const member = await memberFactory.create({name: 'Multi Comp', email: 'multi-comp@ghost.org'}); + await seedSubscriptions(page, member.id, 'comped', [compSubscription()], {tiers: TWO_COMP_TIERS()}); + const sent = await captureWrite(page, new RegExp(`/ghost/api/admin/members/${member.id}/\\??[^/]*$`)); - await expect.poll(() => sentTiers(sent)?.length).toBe(1); - expect(sentTiers(sent)?.[0].id).toBe('tier_silver'); - }); + await page.goto(memberPath(member.id)); + await memberDetailsPage.removeComplimentarySubscription(); + + await expect.poll(() => sentTiers(sent)?.length).toBe(1); + expect(sentTiers(sent)?.[0].id).toBe('tier_silver'); }); + }); - test.describe('New member screen', () => { - // The Subscriptions section is gated on paid members being enabled, so - // stripe has to be on for it to render at all. - test.use({stripeEnabled: true}); + test.describe('New member screen', () => { + // The Subscriptions section is gated on paid members being enabled, so + // stripe has to be on for it to render at all. + test.use({stripeEnabled: true}); - test('newsletters section - renders a toggle list', async ({page}) => { - await page.goto(memberPath('new')); + test('newsletters section - renders a toggle list', async ({page}) => { + await page.goto(memberPath('new')); - await expect(page.getByRole('heading', {name: 'Newsletters', exact: true})).toBeVisible(); - await expect(memberDetailsPage.newsletterSubscriptionToggles.first()).toBeVisible(); - }); + await expect(page.getByRole('heading', {name: 'Newsletters', exact: true})).toBeVisible(); + await expect(memberDetailsPage.newsletterSubscriptionToggles.first()).toBeVisible(); + }); - test('newsletters section - toggles are checked by default', async ({page}) => { - // Newsletters with subscribe_on_signup and members visibility are - // pre-selected on create, so the admin can see what the new member - // will land subscribed to. Assert every toggle rather than the - // first, which would pass even if a later default were missed. - await page.goto(memberPath('new')); - await expect(memberDetailsPage.newsletterSubscriptionToggles.first()).toBeVisible(); - - const count = await memberDetailsPage.newsletterSubscriptionCheckboxes.count(); - expect(count).toBeGreaterThan(0); - for (let i = 0; i < count; i++) { - await expect(memberDetailsPage.newsletterSubscriptionCheckboxes.nth(i)).toBeChecked(); - } - }); + test('newsletters section - toggles are checked by default', async ({page}) => { + // Newsletters with subscribe_on_signup and members visibility are + // pre-selected on create, so the admin can see what the new member + // will land subscribed to. Assert every toggle rather than the + // first, which would pass even if a later default were missed. + await page.goto(memberPath('new')); + await expect(memberDetailsPage.newsletterSubscriptionToggles.first()).toBeVisible(); - test('activity section - shows an empty state', async ({page}) => { - await page.goto(memberPath('new')); + const count = await memberDetailsPage.newsletterSubscriptionCheckboxes.count(); + expect(count).toBeGreaterThan(0); + for (let i = 0; i < count; i++) { + await expect(memberDetailsPage.newsletterSubscriptionCheckboxes.nth(i)).toBeChecked(); + } + }); - await expect(page.getByText('All events related to this member will be shown here.')).toBeVisible(); - }); + test('activity section - shows an empty state', async ({page}) => { + await page.goto(memberPath('new')); - test('subscriptions section - shows an empty state', async ({page}) => { - await page.goto(memberPath('new')); + await expect(page.getByText('All events related to this member will be shown here.')).toBeVisible(); + }); - await expect(page.getByRole('heading', {name: 'Subscriptions', exact: true})).toBeVisible(); - await expect(page.getByRole('heading', {name: 'No subscriptions', exact: true})).toBeVisible(); - }); + test('subscriptions section - shows an empty state', async ({page}) => { + await page.goto(memberPath('new')); + + await expect(page.getByRole('heading', {name: 'Subscriptions', exact: true})).toBeVisible(); + await expect(page.getByRole('heading', {name: 'No subscriptions', exact: true})).toBeVisible(); }); + }); - test.describe('Engagement section', () => { - const stubMemberRead = interceptMemberRead; + test.describe('Engagement section', () => { + const stubMemberRead = interceptMemberRead; - test('member has received no emails - shows an empty state', async ({page}) => { - const member = await memberFactory.create({name: 'Ada Lovelace', email: 'engagement-empty@ghost.org'}); + test('member has received no emails - shows an empty state', async ({page}) => { + const member = await memberFactory.create({name: 'Ada Lovelace', email: 'engagement-empty@ghost.org'}); - await page.goto(memberPath(member.id)); + await page.goto(memberPath(member.id)); - await expect(page.getByRole('heading', {name: 'Engagement'})).toBeVisible(); - await expect(page.getByText(/We[’']ll show Ada[’']s email stats here/)).toBeVisible(); - }); + await expect(page.getByRole('heading', {name: 'Engagement'})).toBeVisible(); + await expect(page.getByText(/We[’']ll show Ada[’']s email stats here/)).toBeVisible(); + }); - test('member has received emails - shows counts and open rate', async ({page}) => { - const member = await memberFactory.create({name: 'Stats Member', email: 'engagement-stats@ghost.org'}); - // Inject the stats so the branch renders deterministically rather - // than depending on what the fixture database happens to hold. - await stubMemberRead(page, member.id, (m) => { - m.email_count = 12; - m.email_opened_count = 9; - m.email_open_rate = 75; - }); - - await page.goto(memberPath(member.id)); - - const engagement = memberDetailsPage.engagementSection; - await expect(engagement.getByText('Emails received')).toBeVisible(); - await expect(engagement.getByText('12', {exact: true})).toBeVisible(); - await expect(engagement.getByText('Emails opened')).toBeVisible(); - await expect(engagement.getByText('9', {exact: true})).toBeVisible(); - await expect(engagement.getByText('Average open rate')).toBeVisible(); - await expect(engagement.getByText(/75\s*%/)).toBeVisible(); + test('member has received emails - shows counts and open rate', async ({page}) => { + const member = await memberFactory.create({name: 'Stats Member', email: 'engagement-stats@ghost.org'}); + // Inject the stats so the branch renders deterministically rather + // than depending on what the fixture database happens to hold. + await stubMemberRead(page, member.id, (m) => { + m.email_count = 12; + m.email_opened_count = 9; + m.email_open_rate = 75; }); - test('email fields absent from the payload - shows the empty state', async ({page}) => { - const member = await memberFactory.create({name: 'Ada Lovelace', email: 'engagement-undef@ghost.org'}); - await stubMemberRead(page, member.id, (m) => { - delete m.email_count; - delete m.email_opened_count; - delete m.email_open_rate; - }); + await page.goto(memberPath(member.id)); - await page.goto(memberPath(member.id)); + const engagement = memberDetailsPage.engagementSection; + await expect(engagement.getByText('Emails received')).toBeVisible(); + await expect(engagement.getByText('12', {exact: true})).toBeVisible(); + await expect(engagement.getByText('Emails opened')).toBeVisible(); + await expect(engagement.getByText('9', {exact: true})).toBeVisible(); + await expect(engagement.getByText('Average open rate')).toBeVisible(); + await expect(engagement.getByText(/75\s*%/)).toBeVisible(); + }); - await expect(page.getByRole('heading', {name: 'Engagement'})).toBeVisible(); - await expect(page.getByText(/We[’']ll show Ada[’']s email stats here/)).toBeVisible(); + test('email fields absent from the payload - shows the empty state', async ({page}) => { + const member = await memberFactory.create({name: 'Ada Lovelace', email: 'engagement-undef@ghost.org'}); + await stubMemberRead(page, member.id, (m) => { + delete m.email_count; + delete m.email_opened_count; + delete m.email_open_rate; }); - test('open rate not yet calculated - shows the placeholder', async ({page}) => { - const member = await memberFactory.create({name: 'Early Stats', email: 'engagement-null@ghost.org'}); - // The server sends a null rate until the member has been sent 5 - // newsletters; both the count and a bare % would be misleading. - await stubMemberRead(page, member.id, (m) => { - m.email_count = 3; - m.email_opened_count = 2; - m.email_open_rate = null; - }); + await page.goto(memberPath(member.id)); - await page.goto(memberPath(member.id)); + await expect(page.getByRole('heading', {name: 'Engagement'})).toBeVisible(); + await expect(page.getByText(/We[’']ll show Ada[’']s email stats here/)).toBeVisible(); + }); - await expect(page.getByText('This metric is calculated once a member has received 5 newsletters.')).toBeVisible(); + test('open rate not yet calculated - shows the placeholder', async ({page}) => { + const member = await memberFactory.create({name: 'Early Stats', email: 'engagement-null@ghost.org'}); + // The server sends a null rate until the member has been sent 5 + // newsletters; both the count and a bare % would be misleading. + await stubMemberRead(page, member.id, (m) => { + m.email_count = 3; + m.email_opened_count = 2; + m.email_open_rate = null; }); + + await page.goto(memberPath(member.id)); + + await expect(page.getByText('This metric is calculated once a member has received 5 newsletters.')).toBeVisible(); }); }); -} +}); /** - * Known divergences between the two implementations. - * - * Everything above this point is generic and must stay that way. A test only - * belongs here when the *behaviour* — not the markup — exists in one - * implementation and not the other, and we have decided not to close the gap. - * Each one records what the divergence is and why it stands, so the difference - * is a deliberate, visible decision rather than a silently narrowed test. + * Behaviours with no counterpart in the generic suite above: they assert an + * affordance specific to this screen, or need a Stripe-enabled environment. */ -test.describe('Ghost Admin - Member Detail - known divergences', () => { +test.describe('Ghost Admin - Member Detail - screen-specific behaviour', () => { + test.use({stripeEnabled: true}); + let memberFactory: MemberFactory; let memberDetailsPage: MemberDetailsPage; @@ -599,184 +583,109 @@ test.describe('Ghost Admin - Member Detail - known divergences', () => { memberDetailsPage = new MemberDetailsPage(page); }); - /** - * Adding a complimentary subscription is the one flow split purely on - * interaction rather than behaviour. Both screens compute the same - * `expiry_at` from the same duration options and send the same request, but - * one picks a tier with radio buttons and the other with a dropdown. Driving - * both from one test would put a branch inside the page object, which buys - * less than it costs — so the assertion is duplicated instead, and the two - * tests must be kept in step. - */ - test.describe('Ember', () => { - test.use({labs: {memberDetailsReact: false}, stripeEnabled: true}); - - test('adding a complimentary subscription - grants the chosen tier forever', async ({page}) => { - const member = await memberFactory.create({name: 'Comp Grant', email: 'comp-grant-ember@ghost.org'}); - const sent = await captureWrite(page, new RegExp(`/ghost/api/admin/members/${member.id}/\\??[^/]*$`)); + test('adding a complimentary subscription - grants the chosen tier forever', async ({page}) => { + const member = await memberFactory.create({name: 'Comp Grant', email: 'comp-grant-react@ghost.org'}); + const sent = await captureWrite(page, new RegExp(`/ghost/api/admin/members/${member.id}/\\??[^/]*$`)); + + await page.goto(memberPath(member.id)); + await page.getByRole('button', {name: /Add complimentary subscription/}).click(); + await page.getByTestId('comp-tier-select').click(); + const option = memberDetailsPage.reactCompTierOptions.first(); + const chosenTierId = await option.getAttribute('data-tier-id'); + await option.click(); + await page.getByTestId('comp-add-confirm').click(); + + await expect.poll(() => sentTiers(sent)?.length).toBe(1); + expect(sentTiers(sent)?.[0].id).toBe(chosenTierId); + expect(sentTiers(sent)?.[0].expiry_at ?? null).toBeNull(); + }); - await page.goto(memberPath(member.id)); - await page.getByRole('button', {name: /Add complimentary subscription/}).click(); - const option = memberDetailsPage.emberCompTierOptions.first(); - const chosenTierId = await option.getAttribute('data-test-tier-option'); - await option.click(); - await memberDetailsPage.emberSaveCompTierButton.click(); + test('invalid email - save is disabled', async ({page}) => { + // The submit is blocked rather than allowed to fail, so there is no + // server error to surface. + const member = await memberFactory.create({name: 'Invalid Email', email: 'valid-react@ghost.org'}); - await expect.poll(() => sentTiers(sent)?.length).toBe(1); - // The tier the admin picked, not merely some tier. - expect(sentTiers(sent)?.[0].id).toBe(chosenTierId); - // Forever is the default, so no expiry is sent. - expect(sentTiers(sent)?.[0].expiry_at ?? null).toBeNull(); - }); - - test('invalid email - save stays enabled and the server rejects it', async ({page}) => { - // Ember validates on submit: Save is always clickable, and the - // failed attempt reports why. React disables Save instead — see the - // React-side counterpart below. - const member = await memberFactory.create({name: 'Invalid Email', email: 'valid-ember@ghost.org'}); + await page.goto(memberPath(member.id)); + await memberDetailsPage.emailInput.fill('not-an-email'); - await page.goto(memberPath(member.id)); - await memberDetailsPage.emailInput.fill('not-an-email'); - await memberDetailsPage.saveButton.click(); + await expect(memberDetailsPage.saveButton).toBeDisabled(); + }); - await expect(memberDetailsPage.retryButton).toBeVisible(); - await expect(memberDetailsPage.body).toContainText('Invalid Email'); + test('failed member load - offers a retry rather than a not-found message', async ({page}) => { + // A failed load renders a recoverable panel rather than dead-ending + // the route. + const member = await memberFactory.create({name: 'Retry Target', email: 'retry-target@ghost.org'}); + let requests = 0; + const memberReadRegex = new RegExp(`/ghost/api/admin/members/${member.id}/\\??[^/]*$`); + await page.route(memberReadRegex, async (route) => { + if (route.request().method() !== 'GET') { + return route.continue(); + } + requests += 1; + if (requests === 1) { + return route.fulfill({status: 500, contentType: 'application/json', body: JSON.stringify({errors: [{message: 'boom'}]})}); + } + return route.continue(); }); - test('commenting can be re-enabled from the sidebar indicator', async ({page}) => { - // Ember offers a one-click Enable next to the "Comments disabled" - // indicator. React only exposes this through the actions menu, which - // the generic suite already covers for both. - const member = await memberFactory.create({name: 'Sidebar Enable', email: 'sidebar-enable@ghost.org'}); + await page.goto(memberPath(member.id)); - await page.goto(memberPath(member.id)); - await memberDetailsPage.settingsSection.memberActionsButton.click(); - await memberDetailsPage.settingsSection.disableCommentingButton.click(); - await memberDetailsPage.disableCommentingConfirmButton.click(); - await expect(memberDetailsPage.commentingDisabledIndicator).toBeVisible(); + const errorPanel = page.getByTestId('member-detail-load-error'); + await expect(errorPanel).toBeVisible(); + // A server error must not be reported as a missing member, anywhere + // on the screen. Asserting only the body copy previously let the + // breadcrumb go on claiming "Member not found" beside a + // "couldn't load" message. + await expect(page.getByText(/not found/i)).toHaveCount(0); + await expect(page.getByText(/couldn[’']t be found/)).toHaveCount(0); - await memberDetailsPage.enableCommentingLink.click(); + await errorPanel.getByRole('button', {name: 'Retry'}).click(); - await expect(memberDetailsPage.commentingDisabledIndicator).toBeHidden(); - }); + await expect(memberDetailsPage.screenTitle).toHaveText('Retry Target'); + await expect(errorPanel).toHaveCount(0); }); - test.describe('React', () => { - test.use({labs: {memberDetailsReact: true}, stripeEnabled: true}); + test.describe('Removing a complimentary subscription', () => { + test.use({stripeEnabled: true}); - // Ember counterpart above — keep the assertions identical. - test('adding a complimentary subscription - grants the chosen tier forever', async ({page}) => { - const member = await memberFactory.create({name: 'Comp Grant', email: 'comp-grant-react@ghost.org'}); + test('preserves the expiry date on surviving tiers', async ({page}) => { + // The server treats a tier arriving without `expiry_at` as null and + // wipes the pivot (`models/member.js` updateTierExpiry), so the + // whole set has to be sent back with expiries intact. + const member = await memberFactory.create({name: 'Multi Comp', email: 'multi-comp-react@ghost.org'}); + await seedSubscriptions(page, member.id, 'comped', [compSubscription()], {tiers: TWO_COMP_TIERS()}); const sent = await captureWrite(page, new RegExp(`/ghost/api/admin/members/${member.id}/\\??[^/]*$`)); await page.goto(memberPath(member.id)); - await page.getByRole('button', {name: /Add complimentary subscription/}).click(); - await page.getByTestId('comp-tier-select').click(); - const option = memberDetailsPage.reactCompTierOptions.first(); - const chosenTierId = await option.getAttribute('data-tier-id'); - await option.click(); - await page.getByTestId('comp-add-confirm').click(); + await memberDetailsPage.removeComplimentarySubscription(); await expect.poll(() => sentTiers(sent)?.length).toBe(1); - expect(sentTiers(sent)?.[0].id).toBe(chosenTierId); - expect(sentTiers(sent)?.[0].expiry_at ?? null).toBeNull(); + expect(sentTiers(sent)?.[0].expiry_at).toBe(SILVER_EXPIRY); }); - test('invalid email - save is disabled', async ({page}) => { - // React blocks the submit rather than letting it fail, so there is - // no server error to surface. Deliberate; the Ember counterpart - // above pins the other behaviour. - const member = await memberFactory.create({name: 'Invalid Email', email: 'valid-react@ghost.org'}); - - await page.goto(memberPath(member.id)); - await memberDetailsPage.emailInput.fill('not-an-email'); - - await expect(memberDetailsPage.saveButton).toBeDisabled(); - }); - - test('failed member load - offers a retry rather than a not-found message', async ({page}) => { - // Ember has no equivalent control: a failed load surfaces as an - // alert and the route errors. React renders a recoverable panel, so - // there is nothing generic to assert. - const member = await memberFactory.create({name: 'Retry Target', email: 'retry-target@ghost.org'}); - let requests = 0; - const memberReadRegex = new RegExp(`/ghost/api/admin/members/${member.id}/\\??[^/]*$`); - await page.route(memberReadRegex, async (route) => { - if (route.request().method() !== 'GET') { - return route.continue(); - } - requests += 1; - if (requests === 1) { - return route.fulfill({status: 500, contentType: 'application/json', body: JSON.stringify({errors: [{message: 'boom'}]})}); - } - return route.continue(); - }); + test('asks for confirmation first', async ({page}) => { + const member = await memberFactory.create({name: 'Confirm Comp', email: 'confirm-comp@ghost.org'}); + await seedSubscriptions(page, member.id, 'comped', [compSubscription()], {tiers: TWO_COMP_TIERS()}); + const sent = await captureWrite(page, new RegExp(`/ghost/api/admin/members/${member.id}/\\??[^/]*$`)); await page.goto(memberPath(member.id)); + await memberDetailsPage.subscriptionActionsButton.first().click(); + await memberDetailsPage.removeComplimentaryButton.click(); - const errorPanel = page.getByTestId('member-detail-load-error'); - await expect(errorPanel).toBeVisible(); - // A server error must not be reported as a missing member, anywhere - // on the screen. Asserting only the body copy previously let the - // breadcrumb go on claiming "Member not found" beside a - // "couldn't load" message. - await expect(page.getByText(/not found/i)).toHaveCount(0); - await expect(page.getByText(/couldn[’']t be found/)).toHaveCount(0); - - await errorPanel.getByRole('button', {name: 'Retry'}).click(); - - await expect(memberDetailsPage.screenTitle).toHaveText('Retry Target'); - await expect(errorPanel).toHaveCount(0); - }); - - test.describe('Removing a complimentary subscription', () => { - test.use({stripeEnabled: true}); - - test('preserves the expiry date on surviving tiers', async ({page}) => { - // The server treats a tier arriving without `expiry_at` as null and - // wipes the pivot (`models/member.js` updateTierExpiry), so the - // whole set has to be sent back with expiries intact. Ember sends - // `{id}` only and destroys the expiry on every surviving comp tier. - // React sends `expiry_at` explicitly. Not generic: a shared version - // of this test would fail on Ember, and we are not backporting. - const member = await memberFactory.create({name: 'Multi Comp', email: 'multi-comp-react@ghost.org'}); - await seedSubscriptions(page, member.id, 'comped', [compSubscription()], {tiers: TWO_COMP_TIERS()}); - const sent = await captureWrite(page, new RegExp(`/ghost/api/admin/members/${member.id}/\\??[^/]*$`)); - - await page.goto(memberPath(member.id)); - await memberDetailsPage.removeComplimentarySubscription(); - - await expect.poll(() => sentTiers(sent)?.length).toBe(1); - expect(sentTiers(sent)?.[0].expiry_at).toBe(SILVER_EXPIRY); - }); - - test('asks for confirmation first', async ({page}) => { - // Ember removes the comp the moment the menu item is clicked. - const member = await memberFactory.create({name: 'Confirm Comp', email: 'confirm-comp@ghost.org'}); - await seedSubscriptions(page, member.id, 'comped', [compSubscription()], {tiers: TWO_COMP_TIERS()}); - const sent = await captureWrite(page, new RegExp(`/ghost/api/admin/members/${member.id}/\\??[^/]*$`)); - - await page.goto(memberPath(member.id)); - await memberDetailsPage.subscriptionActionsButton.first().click(); - await memberDetailsPage.removeComplimentaryButton.click(); - - await expect(page.getByRole('alertdialog', {name: /Remove complimentary subscription/})).toBeVisible(); - expect(sent.body).toBeUndefined(); - }); + await expect(page.getByRole('alertdialog', {name: /Remove complimentary subscription/})).toBeVisible(); + expect(sent.body).toBeUndefined(); }); + }); - test('new member - seeded newsletter defaults do not count as unsaved changes', async ({page}) => { - // React seeds the create form's newsletter defaults and treats that - // seeded state as pristine. Ember's new record is dirty from the - // start, so it traps on an untouched form — a generic version of - // this test would fail there by design. - await page.goto(memberPath('new')); - await expect(memberDetailsPage.newsletterSubscriptionToggles.first()).toBeVisible(); + test('new member - seeded newsletter defaults do not count as unsaved changes', async ({page}) => { + // The create form seeds its newsletter defaults and treats that seeded + // state as pristine, so an untouched form must not trap on leave. + await page.goto(memberPath('new')); + await expect(memberDetailsPage.newsletterSubscriptionToggles.first()).toBeVisible(); - await memberDetailsPage.membersBackLink.click(); + await memberDetailsPage.membersBackLink.click(); - await expect(page).toHaveURL(/#\/members(\?|$)/); - await expect(memberDetailsPage.confirmLeaveButton).toHaveCount(0); - }); + await expect(page).toHaveURL(/#\/members(\?|$)/); + await expect(memberDetailsPage.confirmLeaveButton).toHaveCount(0); }); }); diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index 8e9f97eb7be..27fb55db26c 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -27,7 +27,8 @@ const messages = { // flags in this list always return `true`, allows quick global enable prior to full flag removal const GA_FEATURES = [ - 'automationAnalytics' + 'automationAnalytics', + 'memberDetailsReact' ]; // These features are considered publicly available and can be enabled/disabled by users @@ -51,7 +52,6 @@ const PRIVATE_FEATURES = [ 'themeTranslation', 'pictureImageFormats', 'getHelperDeduplication', - 'memberDetailsReact', 'membersCustomFields', 'paywallImprovements', 'giftSubCustomization', From cac9f333fdfed67fdd72f5ebba6b9701bc628101 Mon Sep 17 00:00:00 2001 From: Rob Lester Date: Tue, 4 Aug 2026 17:57:57 +0100 Subject: [PATCH 23/25] Cleaned up the memberDetailsReact flag and deleted the Ember member screen ref https://linear.app/ghost/issue/BER-3848 With the React member details screen generally available the old Ember screen is unreachable, yet it still has to be kept compiling and passing tests, so it goes along with the flag that used to choose between the two implementations. Removing the Ember member routes lets the member URLs fall through Ember's existing catch-all into React, which is how every already-migrated screen works. Three surviving templates still linked to the deleted route and would have thrown once it was gone, so they now use plain links to the member URL, the same pattern already used elsewhere for React-owned screens. The browser page object also no longer has to match two sets of markup for the same control. --- .../src/layout/app-sidebar/nav-content.tsx | 2 +- apps/admin/src/member-detail-gate.test.tsx | 102 --- apps/admin/src/member-detail-gate.tsx | 16 - ...r-detail-custom-fields.acceptance.test.tsx | 2 +- ...ber-detail-leave-guard.acceptance.test.tsx | 8 +- .../src/members/detail/member-detail.tsx | 6 +- apps/admin/src/routes.tsx | 8 +- .../components/gh-member-details-activity.hbs | 2 +- .../app/components/gh-member-label-input.hbs | 42 - .../app/components/gh-member-label-input.js | 173 ----- .../components/gh-member-settings-form.hbs | 311 -------- .../app/components/gh-member-settings-form.js | 242 ------ .../components/member/activity-feed-empty.hbs | 7 - .../app/components/member/activity-feed.hbs | 70 -- .../app/components/member/activity-feed.js | 36 - .../member/newsletter-preference.hbs | 65 -- .../member/newsletter-preference.js | 94 --- .../member/subscription-detail-box.hbs | 49 -- .../member/subscription-detail-box.js | 17 - .../members/modals/delete-member.hbs | 52 -- .../members/modals/delete-member.js | 47 -- .../members/modals/disable-commenting.hbs | 45 -- .../members/modals/disable-commenting.js | 39 - .../members/modals/logout-member.hbs | 30 - .../members/modals/logout-member.js | 31 - .../app/components/modal-member-tier.hbs | 89 --- .../app/components/modal-member-tier.js | 185 ----- .../app/components/posts/debug.hbs | 8 +- apps/ember-admin/app/controllers/member.js | 352 --------- apps/ember-admin/app/router.js | 2 - apps/ember-admin/app/routes/member.js | 152 ---- apps/ember-admin/app/routes/member/new.js | 6 - apps/ember-admin/app/serializers/member.js | 4 +- apps/ember-admin/app/services/feature.js | 1 - apps/ember-admin/app/styles/app-dark.css | 4 - .../app/styles/components/dropdowns.css | 36 - .../app/styles/layouts/members.css | 21 - apps/ember-admin/app/templates/member.hbs | 146 ---- .../app/utils/subscription-data.js | 225 ------ .../tests/acceptance/members/details-test.js | 621 --------------- .../tests/unit/controllers/member-test.js | 40 - .../tests/unit/services/state-bridge-test.js | 4 +- .../unit/utils/subscription-data-test.js | 734 ------------------ .../admin/members/member-details-page.ts | 62 +- e2e/tests/admin/members/member-detail.test.ts | 11 +- ghost/core/core/shared/labs.js | 3 +- 46 files changed, 38 insertions(+), 4164 deletions(-) delete mode 100644 apps/admin/src/member-detail-gate.test.tsx delete mode 100644 apps/admin/src/member-detail-gate.tsx delete mode 100644 apps/ember-admin/app/components/gh-member-label-input.hbs delete mode 100644 apps/ember-admin/app/components/gh-member-label-input.js delete mode 100644 apps/ember-admin/app/components/gh-member-settings-form.hbs delete mode 100644 apps/ember-admin/app/components/gh-member-settings-form.js delete mode 100644 apps/ember-admin/app/components/member/activity-feed-empty.hbs delete mode 100644 apps/ember-admin/app/components/member/activity-feed.hbs delete mode 100644 apps/ember-admin/app/components/member/activity-feed.js delete mode 100644 apps/ember-admin/app/components/member/newsletter-preference.hbs delete mode 100644 apps/ember-admin/app/components/member/newsletter-preference.js delete mode 100644 apps/ember-admin/app/components/member/subscription-detail-box.hbs delete mode 100644 apps/ember-admin/app/components/member/subscription-detail-box.js delete mode 100644 apps/ember-admin/app/components/members/modals/delete-member.hbs delete mode 100644 apps/ember-admin/app/components/members/modals/delete-member.js delete mode 100644 apps/ember-admin/app/components/members/modals/disable-commenting.hbs delete mode 100644 apps/ember-admin/app/components/members/modals/disable-commenting.js delete mode 100644 apps/ember-admin/app/components/members/modals/logout-member.hbs delete mode 100644 apps/ember-admin/app/components/members/modals/logout-member.js delete mode 100644 apps/ember-admin/app/components/modal-member-tier.hbs delete mode 100644 apps/ember-admin/app/components/modal-member-tier.js delete mode 100644 apps/ember-admin/app/controllers/member.js delete mode 100644 apps/ember-admin/app/routes/member.js delete mode 100644 apps/ember-admin/app/routes/member/new.js delete mode 100644 apps/ember-admin/app/templates/member.hbs delete mode 100644 apps/ember-admin/app/utils/subscription-data.js delete mode 100644 apps/ember-admin/tests/acceptance/members/details-test.js delete mode 100644 apps/ember-admin/tests/unit/controllers/member-test.js delete mode 100644 apps/ember-admin/tests/unit/utils/subscription-data-test.js diff --git a/apps/admin/src/layout/app-sidebar/nav-content.tsx b/apps/admin/src/layout/app-sidebar/nav-content.tsx index 71819d23abb..720bd64aa47 100644 --- a/apps/admin/src/layout/app-sidebar/nav-content.tsx +++ b/apps/admin/src/layout/app-sidebar/nav-content.tsx @@ -16,7 +16,7 @@ import { useIsActiveLink } from "./use-is-active-link"; import { useEmberRouting } from "@/ember-bridge"; import { useFeatureFlag } from "@tryghost/admin-x-framework/hooks"; -const LEGACY_MEMBERS_ACTIVE_ROUTES = ['member', 'member.new', 'members-activity']; +const LEGACY_MEMBERS_ACTIVE_ROUTES = ['members-activity']; function PostsNavItemContent({isActive, to}: {isActive: boolean; to: string}) { return ( diff --git a/apps/admin/src/member-detail-gate.test.tsx b/apps/admin/src/member-detail-gate.test.tsx deleted file mode 100644 index e6f6aaf6028..00000000000 --- a/apps/admin/src/member-detail-gate.test.tsx +++ /dev/null @@ -1,102 +0,0 @@ -import React from 'react'; -import {MemberDetailGate} from './member-detail-gate'; -import {beforeEach, describe, expect, it, vi} from 'vitest'; -import {render, screen, waitFor} from '@testing-library/react'; - -const {mockUseBrowseConfig} = vi.hoisted(() => ({ - mockUseBrowseConfig: vi.fn() -})); - -vi.mock('@tryghost/admin-x-framework/api/config', () => ({ - useBrowseConfig: mockUseBrowseConfig -})); - -vi.mock('./ember-bridge', () => ({ - EmberFallback: () => React.createElement('div', {'data-testid': 'ember-fallback'}), - useEmberFeatureFlag: (flag: string) => { - const stateBridge = window.EmberBridge?.state; - if (!stateBridge?.isFeatureEnabled) { - return undefined; - } - return stateBridge.isFeatureEnabled(flag) ?? null; - } -})); - -vi.mock('./members/detail/member-detail', () => ({ - default: () => React.createElement('div', {'data-testid': 'react-member-detail'}) -})); - -const configResult = (overrides: Record) => ({ - data: undefined, - isError: false, - isLoading: false, - ...overrides -}); - -const withLabs = (labs: Record) => configResult({data: {config: {labs}}}); - -describe('MemberDetailGate', () => { - beforeEach(() => { - mockUseBrowseConfig.mockReset(); - }); - - it('renders Ember while the flag is off', () => { - mockUseBrowseConfig.mockReturnValue(withLabs({memberDetailsReact: false})); - - render(); - - expect(screen.getByTestId('ember-fallback')).toBeInTheDocument(); - }); - - it('renders React while the flag is on', async () => { - mockUseBrowseConfig.mockReturnValue(withLabs({memberDetailsReact: true})); - - render(); - - // The React screen is lazily imported, so it arrives a tick later. - await waitFor(() => { - expect(screen.getByTestId('react-member-detail')).toBeInTheDocument(); - }); - }); - - it('renders Ember when the flag is absent from config', () => { - mockUseBrowseConfig.mockReturnValue(withLabs({})); - - render(); - - expect(screen.getByTestId('ember-fallback')).toBeInTheDocument(); - }); - - it('renders Ember when the config query fails', () => { - // A failed config read must not blank the screen — Ember owns this URL - // by default and still serves it, so degrading to Ember keeps the - // member detail working. Reporting is left to the framework's default - // error handler on useBrowseConfig. - mockUseBrowseConfig.mockReturnValue(configResult({isError: true, data: undefined})); - - render(); - - expect(screen.getByTestId('ember-fallback')).toBeInTheDocument(); - }); - - it('renders Ember when the config query resolves with no data', () => { - mockUseBrowseConfig.mockReturnValue(configResult({data: undefined})); - - render(); - - expect(screen.getByTestId('ember-fallback')).toBeInTheDocument(); - }); - - it('renders nothing while config is loading', () => { - // Deliberately not falling back to Ember here: doing so would un-hide - // the Ember shell and flash the old screen on every cold load for - // admins who have the flag on. - mockUseBrowseConfig.mockReturnValue(configResult({isLoading: true})); - - const {container} = render(); - - expect(screen.queryByTestId('ember-fallback')).not.toBeInTheDocument(); - expect(screen.queryByTestId('react-member-detail')).not.toBeInTheDocument(); - expect(container).toBeEmptyDOMElement(); - }); -}); diff --git a/apps/admin/src/member-detail-gate.tsx b/apps/admin/src/member-detail-gate.tsx deleted file mode 100644 index 7ef1efcd98a..00000000000 --- a/apps/admin/src/member-detail-gate.tsx +++ /dev/null @@ -1,16 +0,0 @@ -import { FlagGatedRoute } from "./flag-gated-route"; -import { lazy } from "react"; - -/** - * Serves `/members/:member_id` — covering both edit (`:member_id`) and create - * (the `new` sentinel) — from the React member detail screen when the - * `memberDetailsReact` Labs flag is on, and from Ember otherwise. The gating - * semantics (loading, error, and flag branching) live in FlagGatedRoute. - */ -const MemberDetailReact = lazy(() => import("./members/detail/member-detail")); - -export function MemberDetailGate() { - return ; -} - -export default MemberDetailGate; diff --git a/apps/admin/src/members/detail/member-detail-custom-fields.acceptance.test.tsx b/apps/admin/src/members/detail/member-detail-custom-fields.acceptance.test.tsx index 34cc4c5959d..a5d3516a430 100644 --- a/apps/admin/src/members/detail/member-detail-custom-fields.acceptance.test.tsx +++ b/apps/admin/src/members/detail/member-detail-custom-fields.acceptance.test.tsx @@ -3,7 +3,7 @@ import {page, userEvent} from 'vitest/browser'; import {fakeAdminEndpoint, fakeMembers, member, renderAdminApp, type Member} from '@test-utils/acceptance'; -const FLAGS = {labs: {memberDetailsReact: true, membersCustomFields: true}}; +const FLAGS = {labs: {membersCustomFields: true}}; const FIELDS = [ {key: 'job_title', name: 'Job title', type: 'short_text', created_at: '2026-07-14T00:00:00.000Z', updated_at: null}, diff --git a/apps/admin/src/members/detail/member-detail-leave-guard.acceptance.test.tsx b/apps/admin/src/members/detail/member-detail-leave-guard.acceptance.test.tsx index 5a29e2aaf8e..9f375d2b909 100644 --- a/apps/admin/src/members/detail/member-detail-leave-guard.acceptance.test.tsx +++ b/apps/admin/src/members/detail/member-detail-leave-guard.acceptance.test.tsx @@ -3,8 +3,6 @@ import {page} from 'vitest/browser'; import {fakeAdminEndpoint, fakeMembers, member, renderAdminApp, type Member} from '@test-utils/acceptance'; -const FLAGS = {labs: {memberDetailsReact: true}}; - function fakeMemberDetailWorld(m: Member) { fakeMembers([m]); fakeAdminEndpoint('GET', new RegExp(`^/members/${m.id}/`), {members: [m]}); @@ -28,7 +26,7 @@ describe('Member detail leave guard', () => { it('guards leaving via the breadcrumb (react-router link) with unsaved edits', async () => { const m = member({name: 'Ada Lovelace'}); fakeMemberDetailWorld(m); - await renderAdminApp(`/members/${m.id}`, FLAGS); + await renderAdminApp(`/members/${m.id}`); await page.getByLabelText('Name').fill('Ada B'); await page.getByTestId('member-detail').getByRole('link', {name: 'Members'}).click(); @@ -39,7 +37,7 @@ describe('Member detail leave guard', () => { it('guards leaving via the sidebar (native hash anchor) with unsaved edits', async () => { const m = member({name: 'Ada Lovelace'}); fakeMemberDetailWorld(m); - await renderAdminApp(`/members/${m.id}`, FLAGS); + await renderAdminApp(`/members/${m.id}`); await page.getByLabelText('Name').fill('Ada B'); await page.getByRole('link', {name: 'Members'}).first().click(); @@ -50,7 +48,7 @@ describe('Member detail leave guard', () => { it('keeps editing on cancel and completes the navigation on Leave', async () => { const m = member({name: 'Ada Lovelace'}); fakeMemberDetailWorld(m); - await renderAdminApp(`/members/${m.id}`, FLAGS); + await renderAdminApp(`/members/${m.id}`); await page.getByLabelText('Name').fill('Ada B'); await page.getByRole('link', {name: 'Members'}).first().click(); diff --git a/apps/admin/src/members/detail/member-detail.tsx b/apps/admin/src/members/detail/member-detail.tsx index 0178b12074c..305e33d8a95 100644 --- a/apps/admin/src/members/detail/member-detail.tsx +++ b/apps/admin/src/members/detail/member-detail.tsx @@ -284,9 +284,9 @@ const MemberDetailPage: React.FC = ({paidMembersEnabled, const blocker = useBlocker(({currentLocation, nextLocation}) => !bypassGuardRef.current && hasUnsavedChanges && currentLocation.pathname !== nextLocation.pathname); // Native `` navigations (the sidebar, links into Ember // routes) never reach the react-router blocker above — see the hook. - // Ember's own guard (`trailing-hash.js`) can't cover this screen either: - // with `memberDetailsReact` on, the Ember member route aborts and never - // registers into the `unsaved-changes` service. + // Ember's own guard (`trailing-hash.js`) can't cover this screen either, + // since the screen has no Ember route to register into the + // `unsaved-changes` service. const anchorGuard = useHashLinkNavigationGuard(hasUnsavedChanges); const isBlocked = blocker.state === 'blocked' || anchorGuard.isBlocked; diff --git a/apps/admin/src/routes.tsx b/apps/admin/src/routes.tsx index 38c398414b6..a4f9874189f 100644 --- a/apps/admin/src/routes.tsx +++ b/apps/admin/src/routes.tsx @@ -13,7 +13,6 @@ import { EmberFallback, ForceUpgradeGuard } from "./ember-bridge"; import type { RouteHandle } from "./ember-bridge"; import HomeRedirect from "./home-redirect"; import { EmberListWithGiftLinks } from "./gift-link-modal-host"; -import { MemberDetailGate } from "./member-detail-gate"; import { TagDetailGate } from "./tag-detail-gate"; import { OnboardingRedirect } from "./onboarding/onboarding-redirect"; import { type AccessRouteHandle, RouteAccessGuard } from "./route-access-guard"; @@ -64,13 +63,8 @@ const membersRoute: RouteObject = { // Covers both edit (`:member_id`) and create (the sentinel `new`) // — real member ids are 24-char hex ObjectIds, so they can't // collide with the literal "new". - // - // MemberDetailGate serves Ember or React depending on the - // `memberDetailsReact` Labs flag; the parent route's - // emberFallbackHandle covers both, since ForceUpgradeGuard checks - // every match rather than just the leaf. path: ":member_id", - Component: MemberDetailGate + lazy: lazyComponent(() => import("./members/detail/member-detail")) } ] }; diff --git a/apps/ember-admin/app/components/gh-member-details-activity.hbs b/apps/ember-admin/app/components/gh-member-details-activity.hbs index f5faa1aac37..3d314e5e279 100644 --- a/apps/ember-admin/app/components/gh-member-details-activity.hbs +++ b/apps/ember-admin/app/components/gh-member-details-activity.hbs @@ -28,7 +28,7 @@ {{/if}}

- View member profile → + View member profile →

diff --git a/apps/ember-admin/app/components/gh-member-label-input.hbs b/apps/ember-admin/app/components/gh-member-label-input.hbs deleted file mode 100644 index ab4e00d819b..00000000000 --- a/apps/ember-admin/app/components/gh-member-label-input.hbs +++ /dev/null @@ -1,42 +0,0 @@ - -
- - {{label.name}} - - {{#if (and @allowEdit label.slug)}} - - {{/if}} -
- diff --git a/apps/ember-admin/app/components/gh-member-label-input.js b/apps/ember-admin/app/components/gh-member-label-input.js deleted file mode 100644 index 0221bcf3486..00000000000 --- a/apps/ember-admin/app/components/gh-member-label-input.js +++ /dev/null @@ -1,173 +0,0 @@ -import Component from '@glimmer/component'; -import {TrackedArray} from 'tracked-built-ins'; -import {action} from '@ember/object'; -import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; -import {tracked} from '@glimmer/tracking'; - -export default class GhMemberLabelInput extends Component { - @service store; - @service labelsManager; - - @tracked _searchedLabels = new TrackedArray(); - - _searchedLabelsQuery = null; - _searchedLabelsMeta = null; - - _powerSelectAPI = null; - - get availableLabels() { - const selectedLabels = this.selectedLabels; - return this.labelsManager.labels.filter(label => !selectedLabels.includes(label)); - } - - get useServerSideSearch() { - return !this.labelsManager.hasLoadedAll; - } - - get selectedLabels() { - if (typeof this.args.labels === 'object') { - if (this.args.labels?.length && typeof this.args.labels[0] === 'string') { - return this.args.labels.map((d) => { - return this.labelsManager.findBySlug(d); - }).filter(Boolean); - } - return this.args.labels || []; - } - return []; - } - - @action - addSearchedLabels(labels) { - const existingIds = new Set([ - ...this.selectedLabels.map(l => l.id), - ...this._searchedLabels.map(l => l.id) - ]); - const deduplicatedLabels = labels.filter(label => !existingIds.has(label.id)); - this._searchedLabels.push(...deduplicatedLabels); - } - - @action - registerPowerSelectAPI(api) { - this._powerSelectAPI = api; - } - - @action - async loadInitialLabels() { - if (!this.labelsManager.hasLoaded) { - await this.labelsManager.loadMoreTask.perform(); - } - } - - @task({drop: true}) - *loadMoreLabelsTask() { - const isSearch = !!this._powerSelectAPI?.searchText; - if (isSearch) { - if (!this.useServerSideSearch) { - return; - } - - if (this.searchLabelsTask.isRunning) { - return; - } - - if (!this._searchedLabelsMeta || (this._searchedLabelsMeta.pagination.pages <= this._searchedLabelsMeta.pagination.page)) { - return; - } - - const page = this._searchedLabelsMeta.pagination.page + 1; - const labels = yield this.labelsManager.searchLabelsTask.perform(this._searchedLabelsQuery, {page}); - this.addSearchedLabels(labels.toArray()); - this._searchedLabelsMeta = labels.meta; - } else { - yield this.labelsManager.loadMoreTask.perform(); - } - } - - @task({restartable: true}) - *searchLabelsTask(term) { - this._searchedLabelsQuery = term; - const labels = yield this.labelsManager.searchLabelsTask.perform(term); - this._searchedLabelsMeta = labels.meta; - - this._searchedLabels = new TrackedArray(); - this.addSearchedLabels(labels.toArray()); - return this._searchedLabels; - } - - @action - showCreateWhen(term) { - const availableLabelNames = this._searchedLabels.map(label => label.name.toLowerCase()); - availableLabelNames.push(...this.selectedLabels.map(label => label.name.toLowerCase())); - - const foundMatchingLabelName = availableLabelNames.includes(term.toLowerCase()); - return !foundMatchingLabelName; - } - - willDestroy() { - super.willDestroy?.(...arguments); - this.store.peekAll('label').forEach((label) => { - if (label.get('isNew')) { - this.store.deleteRecord(label); - } - }); - } - - @action - updateLabels(newLabels) { - let currentLabels = this.selectedLabels; - - // destroy new+unsaved labels that are no longer selected - currentLabels.forEach(function (label) { - if (!newLabels.includes(label) && label.get('isNew')) { - label.destroyRecord(); - } - }); - - this.args.onChange(newLabels); - } - - @action - editLabel(label, event) { - event.stopPropagation(); - this.args.onLabelEdit?.(label.slug); - } - - @action - createLabel(labelName) { - let currentLabels = this.selectedLabels; - let currentLabelNames = currentLabels.map(label => label.get('name').toLowerCase()); - let labelToAdd; - - labelName = labelName.trim(); - - // abort if label is already selected - if (currentLabelNames.includes(labelName.toLowerCase())) { - return; - } - - // find existing label if there is one - labelToAdd = this._findLabelByName(labelName); - - // create new label if no match - if (!labelToAdd) { - labelToAdd = this.store.createRecord('label', { - name: labelName - }); - } - - // push label onto member relationship - currentLabels.pushObject(labelToAdd); - this.args.onChange(currentLabels); - } - - _findLabelByName(name) { - let withMatchingName = function (label) { - if (label.__isSuggestion__) { - return false; - } - return label.name.toLowerCase() === name.toLowerCase(); - }; - return this._searchedLabels.find(withMatchingName) || this.labelsManager.labels.find(withMatchingName); - } -} diff --git a/apps/ember-admin/app/components/gh-member-settings-form.hbs b/apps/ember-admin/app/components/gh-member-settings-form.hbs deleted file mode 100644 index 085995188d1..00000000000 --- a/apps/ember-admin/app/components/gh-member-settings-form.hbs +++ /dev/null @@ -1,311 +0,0 @@ -
-
- - -
-
-
-
- - - - - - - - - - - -
- - - - - - - - - - -

Maximum: 500 characters. You've used - {{gh-count-down-characters this.scratchMember.note 500}}

- -
-
- - {{#if this.canShowMultipleNewsletters}} - - {{/if}} - - {{#if this.membersUtils.paidMembersEnabled}} -

Subscriptions

- - {{#unless this.tiers}} -
-
- {{#unless this.isCreatingComplimentary}} -
- {{svg-jar "no-data-subscription"}} -

No subscriptions

-
- {{/unless}} - - {{#if this.isAddComplimentaryAllowed}} - {{#if this.isCreatingComplimentary}} - - {{else}} - - {{/if}} - {{/if}} -
-
- {{/unless}} - - {{#if this.tiers}} -
-
- {{#each this.tiers as |tier|}} - {{#each tier.subscriptions as |sub index|}} -
-
-
- {{#if sub.hasActiveDiscount}} - {{sub.discountedPrice.currencySymbol}} - {{gh-price-amount sub.discountedPrice.amount}} - {{sub.originalPrice.currencySymbol}}{{gh-price-amount sub.originalPrice.amount}} - {{else}} - {{sub.price.currencySymbol}} - {{gh-price-amount sub.price.amount}} - {{/if}} -
-
{{if (eq sub.price.interval "year") "yearly" "monthly"}}
-
-
-

- {{tier.name}} - {{#if (eq sub.status "canceled")}} - Canceled - {{else if sub.cancel_at_period_end}} - Canceled - {{else if sub.compExpiry}} - Active - {{else if sub.giftExpiry}} - Active - {{else if sub.trialUntil}} - Active - {{else}} - Active - {{/if}} - {{#if (gt tier.subscriptions.length 1)}} - {{tier.subscriptions.length}} subscriptions - {{/if}} -

-
- {{sub.priceLabel}} - {{sub.validityDetails}} -
- -
- {{#if sub.isGift}} - {{! Gift subscriptions have no action menu }} - {{else if sub.isComplimentary}} - - - - {{svg-jar "dotdotdot"}} - - - - -
  • - -
    • -     -     - {{else}} - - - - {{svg-jar "dotdotdot"}} - - - - -
  • - - View Stripe customer - -
    • -
  • -
  • - - View Stripe subscription - -
    • -
  • - {{#if (not-eq sub.status "canceled")}} - {{#if sub.cancel_at_period_end}} - - {{else}} - - {{/if}} - {{/if}} -
    • -     -     - {{/if}} -
    - {{/each}} - - {{#if (eq tier.subscriptions.length 0)}} -
    -
    -
    - Complimentary - Active -
    -
    Created on
    -
    -
    -
    -
    - $ - 0 -
    -
    yearly
    -
    - - - - {{svg-jar "dotdotdot"}} - - - - -
  • - -
    • -     -     -
    -
    - {{/if}} - {{/each}} -
    -
    - {{/if}} - - {{#if (and this.tiers this.isAddComplimentaryAllowed)}} -
    - {{#if this.isCreatingComplimentary}} - - {{else}} - - {{/if}} -
    - {{/if}} - {{/if}} - - -
    - -
    - -
    - -{{#if this.showMemberTierModal}} - - - -{{/if}} diff --git a/apps/ember-admin/app/components/gh-member-settings-form.js b/apps/ember-admin/app/components/gh-member-settings-form.js deleted file mode 100644 index 900cc64701b..00000000000 --- a/apps/ember-admin/app/components/gh-member-settings-form.js +++ /dev/null @@ -1,242 +0,0 @@ -import Component from '@glimmer/component'; -import {action} from '@ember/object'; -import {didCancel, task} from 'ember-concurrency'; -import {getSubscriptionData} from 'ghost-admin/utils/subscription-data'; -import {inject as service} from '@ember/service'; -import {tracked} from '@glimmer/tracking'; - -export default class extends Component { - @service membersUtils; - @service ghostPaths; - @service ajax; - @service store; - @service feature; - @service settings; - - constructor(...args) { - super(...args); - this.member = this.args.member; - this.scratchMember = this.args.scratchMember; - } - - @tracked showMemberTierModal = false; - @tracked tiersList; - @tracked newslettersList; - @tracked loadingSubscriptionId = null; - - get isAddComplimentaryAllowed() { - if (!this.membersUtils.paidMembersEnabled) { - return false; - } - - if (this.member.get('isNew')) { - return false; - } - - if (this.member.get('tiers')?.length > 0) { - return false; - } - - // complimentary subscriptions are assigned to tiers so it only - // makes sense to show the "add complimentary" buttons when there's a - // tier to assign the complimentary subscription to - const hasAnActivePaidTier = !!this.tiersList?.length; - - return hasAnActivePaidTier; - } - - get isCreatingComplimentary() { - return this.args.isSaveRunning; - } - - get tiers() { - let subscriptions = this.member.get('subscriptions') || []; - - // Create the tiers from `subscriptions.price.tier` - let tiers = subscriptions - .map(subscription => (subscription.tier || subscription.price.tier)) - .filter((value, index, self) => { - // Deduplicate by taking the first object by `id` - return typeof value.id !== 'undefined' && self.findIndex(element => (element.tier_id || element.id) === (value.tier_id || value.id)) === index; - }); - - let subsWithPrice = subscriptions.filter(sub => !!sub.price); - let subscriptionData = subsWithPrice.map(sub => getSubscriptionData(sub)); - - return tiers.map((tier) => { - let tierSubscriptions = subscriptionData.filter((subscription) => { - return subscription?.price?.tier?.tier_id === (tier.tier_id || tier.id); - }); - return { - ...tier, - subscriptions: tierSubscriptions - }; - }); - } - - get customer() { - let firstSubscription = this.member.get('subscriptions').firstObject; - let customer = firstSubscription?.customer; - - if (customer) { - return { - ...customer, - startDate: firstSubscription?.startDate - }; - } - return null; - } - - get canShowMultipleNewsletters() { - return ( - this.settings.editorDefaultEmailRecipients !== 'disabled' - ); - } - - @action - updateNewsletterPreference(event) { - if (!event.target.checked) { - this.member.set('newsletters', []); - } else if (this.newslettersList.firstObject) { - const newsletter = this.newslettersList.firstObject; - this.member.set('newsletters', [newsletter]); - } - } - - @action - setup() { - try { - this.fetchTiers.perform(); - this.fetchNewsletters.perform(); - } catch (e) { - // Do not throw cancellation errors - if (didCancel(e)) { - return; - } - - throw e; - } - } - - @action - setProperty(property, value) { - this.args.setProperty(property, value); - } - - @action - updateProperty(event){ - this.args.setProperty(event.currentTarget.name, event.target.value); - } - - @action - setLabels(labels) { - this.member.set('labels', labels); - } - - @action - setMemberNewsletters(newsletters) { - this.member.set('newsletters', newsletters); - } - - @action - closeMemberTierModal() { - this.showMemberTierModal = false; - } - - @action - cancelSubscription(subscriptionId) { - this.loadingSubscriptionId = subscriptionId; - this.cancelSubscriptionTask.perform(subscriptionId); - } - - @action - removeComplimentary(tierId) { - this.loadingSubscriptionId = `complimentary-${tierId}`; - this.removeComplimentaryTask.perform(tierId); - } - - @action - continueSubscription(subscriptionId) { - this.loadingSubscriptionId = subscriptionId; - this.continueSubscriptionTask.perform(subscriptionId); - } - - @task({drop: true}) - *cancelSubscriptionTask(subscriptionId) { - try { - let url = this.ghostPaths.url.api('members', this.member.get('id'), 'subscriptions', subscriptionId); - - let response = yield this.ajax.put(url, { - data: { - cancel_at_period_end: true - } - }); - - this.store.pushPayload('member', response); - return response; - } finally { - this.loadingSubscriptionId = null; - } - } - - @task({drop: true}) - *removeComplimentaryTask(tierId) { - try { - let url = this.ghostPaths.url.api(`members/${this.member.get('id')}`); - let tiers = this.member.get('tiers') || []; - - const updatedTiers = tiers - .filter(tier => tier.id !== tierId) - .map(tier => ({id: tier.id})); - - let response = yield this.ajax.put(url, { - data: { - members: [{ - id: this.member.get('id'), - email: this.member.get('email'), - tiers: updatedTiers - }] - } - }); - - this.store.pushPayload('member', response); - return response; - } finally { - this.loadingSubscriptionId = null; - } - } - - @task({drop: true}) - *continueSubscriptionTask(subscriptionId) { - try { - let url = this.ghostPaths.url.api('members', this.member.get('id'), 'subscriptions', subscriptionId); - - let response = yield this.ajax.put(url, { - data: { - cancel_at_period_end: false - } - }); - - this.store.pushPayload('member', response); - return response; - } finally { - this.loadingSubscriptionId = null; - } - } - - @task({drop: true}) - *fetchTiers() { - this.tiersList = yield this.store.query('tier', {filter: 'type:paid+active:true', include: 'monthly_price,yearly_price'}); - } - - @task({drop: true}) - *fetchNewsletters() { - this.newslettersList = yield this.store.query('newsletter', {filter: 'status:active'}); - if (this.member.get('isNew')) { - const defaultNewsletters = this.newslettersList.filter((newsletter) => { - return newsletter.subscribeOnSignup && newsletter.visibility === 'members'; - }); - this.setMemberNewsletters(defaultNewsletters); - } - } -} diff --git a/apps/ember-admin/app/components/member/activity-feed-empty.hbs b/apps/ember-admin/app/components/member/activity-feed-empty.hbs deleted file mode 100644 index 5b13e14eda0..00000000000 --- a/apps/ember-admin/app/components/member/activity-feed-empty.hbs +++ /dev/null @@ -1,7 +0,0 @@ -
    -
    {{svg-jar "no-data-list"}}
    -

    Activity

    -

    - All events related to this member will be shown here. -

    -
    \ No newline at end of file diff --git a/apps/ember-admin/app/components/member/activity-feed.hbs b/apps/ember-admin/app/components/member/activity-feed.hbs deleted file mode 100644 index c0a35ddb0c1..00000000000 --- a/apps/ember-admin/app/components/member/activity-feed.hbs +++ /dev/null @@ -1,70 +0,0 @@ -

    Activity

    -{{#if @member.isNew}} -
    -
    - -
    -
    -{{else}} - {{#let (members-event-fetcher filter=(members-event-filter member=@member.id excludedEvents=this.excludedEventTypes) pageSize=5 memberId=@member.id) as |eventsFetcher|}} -
    -
    -
    - {{#if eventsFetcher.isLoading}} -
    - {{else if eventsFetcher.data}} - {{#each eventsFetcher.data as |rawEvent|}} - {{#let (parse-member-event rawEvent eventsFetcher.hasMultipleNewsletters) as |event|}} -
    -
    -
    - {{svg-jar event.icon class=event.iconClass}} -
    -
    -
    - - - {{capitalize-first-letter event.action}} - {{#if event.info}} - ({{event.info}}) - {{/if}} - {{#if event.route}} - {{event.join}} - {{event.object}} - {{else if event.url}} - {{event.join}} - {{event.object}} - {{else if event.email}} - {{event.join}} - - {{/if}} - - {{#if event.description}} -
    -
    - {{event.description}} -
    -
    - {{/if}} -     -
    -
    - {{moment-from-now event.timestamp}} -
    -
    -
    -
    - {{/let}} - {{/each}} - -
    - View all member activity → -
    - {{else}} - - {{/if}} -
    -
    -
    - {{/let}} -{{/if}} diff --git a/apps/ember-admin/app/components/member/activity-feed.js b/apps/ember-admin/app/components/member/activity-feed.js deleted file mode 100644 index 29cbda7aff8..00000000000 --- a/apps/ember-admin/app/components/member/activity-feed.js +++ /dev/null @@ -1,36 +0,0 @@ -import Component from '@glimmer/component'; -import {action} from '@ember/object'; -import {inject as service} from '@ember/service'; - -export default class ActivityFeed extends Component { - @service feature; - - linkScrollerTimeout = null; // needs to be global so can be cleared when needed across functions - excludedEventTypes = ['aggregated_click_event']; - - @action - enterLinkURL(event) { - event.stopPropagation(); - const parent = event.target; - const child = event.target.querySelector('span'); - - clearTimeout(this.linkScrollerTimeout); - if (child.offsetWidth > parent.offsetWidth) { - this.linkScrollerTimeout = setTimeout(() => { - parent.classList.add('scroller'); - child.style.transform = `translateX(-${(child.offsetWidth - parent.offsetWidth) + 8}px)`; - }, 100); - } - } - - @action - leaveLinkURL(event) { - event.stopPropagation(); - clearTimeout(this.linkScrollerTimeout); - const parent = event.target; - const child = event.target.querySelector('span'); - - child.style.transform = 'translateX(0)'; - parent.classList.remove('scroller'); - } -} diff --git a/apps/ember-admin/app/components/member/newsletter-preference.hbs b/apps/ember-admin/app/components/member/newsletter-preference.hbs deleted file mode 100644 index d92f876b32e..00000000000 --- a/apps/ember-admin/app/components/member/newsletter-preference.hbs +++ /dev/null @@ -1,65 +0,0 @@ -

    Newsletters

    -
    - {{#unless this.suppressionData.suppressed}} - {{#each this.newsletters as |newsletter|}} -
    -
    -

    {{newsletter.name}}

    -
    -
    - -
    -
    - {{/each}} - {{/unless}} - - {{#if this.suppressionData.suppressed}} -
    - {{#if (eq this.suppressionData.reason 'fail')}} - {{svg-jar "suppression-notice-bounced" class="gh-member-newsletter-icon"}} - {{/if}} - - {{#if (eq this.suppressionData.reason 'spam')}} - {{svg-jar "suppression-notice-flagged" class="gh-member-newsletter-icon"}} - {{/if}} - -

    Email disabled

    - -

    - {{#if (eq this.suppressionData.reason 'fail')}} - Bounced on {{this.suppressionData.date}} - {{/if}} - - {{#if (eq this.suppressionData.reason 'spam')}} - Flagged as spam on {{this.suppressionData.date}} - {{/if}} - - Learn more -

    - - -
    - {{else}} -
    - If disabled, member will not receive newsletter emails -
    - {{/if}} -
    diff --git a/apps/ember-admin/app/components/member/newsletter-preference.js b/apps/ember-admin/app/components/member/newsletter-preference.js deleted file mode 100644 index c1cc6809b5b..00000000000 --- a/apps/ember-admin/app/components/member/newsletter-preference.js +++ /dev/null @@ -1,94 +0,0 @@ -import Component from '@glimmer/component'; -import moment from 'moment-timezone'; -import {action} from '@ember/object'; -import {inject as service} from '@ember/service'; -import {tracked} from '@glimmer/tracking'; - -export default class MembersNewsletterPreference extends Component { - @service ajax; - @service notifications; - @service ghostPaths; - @service store; - - @tracked filterValue; - @tracked isReEnabling = false; - - constructor(...args) { - super(...args); - } - - get newsletters() { - if (this.args.newsletters?.length > 0) { - return this.args.newsletters.map((d) => { - return { - name: d.name, - description: d.description, - subscribed: !!this.args.member?.newsletters?.find((n) => { - return n.id === d.id; - }), - id: d.id, - forId: `${d.id}-checkbox` - }; - }); - } - return []; - } - - get suppressionData() { - const {emailSuppression} = this.args.member; - const timestamp = emailSuppression?.info?.timestamp; - const formattedDate = timestamp ? moment(new Date(timestamp)).format('D MMM YYYY') : null; - - return { - suppressed: emailSuppression?.suppressed, - reason: emailSuppression?.info?.reason, - date: formattedDate - }; - } - - @action - updateNewsletterPreference(newsletter, event) { - let updatedNewsletters = []; - - const selectedNewsletter = this.args.newsletters.find((d) => { - return d.id === newsletter.id; - }); - - if (!event.target.checked) { - updatedNewsletters = this.args.member.newsletters.filter((d) => { - return d.id !== newsletter.id; - }); - } else { - updatedNewsletters = this.args.member.newsletters.filter((d) => { - return d.id !== newsletter.id; - }).concat(selectedNewsletter); - } - this.args.setMemberNewsletters(updatedNewsletters); - } - - @action - async reEnableEmail() { - this.isReEnabling = true; - - try { - const url = `${this.ghostPaths.url.api('members', this.args.member.id)}suppression`; - await this.ajax.delete(url); - - // Refresh the member data to get updated suppression status - await this.store.findRecord('member', this.args.member.id, { - reload: true, - include: 'tiers' - }); - - this.notifications.showNotification('Email re-enabled successfully', { - type: 'success' - }); - } catch (error) { - this.notifications.showAlert('Failed to re-enable email. Please try again.', { - type: 'error' - }); - } finally { - this.isReEnabling = false; - } - } -} diff --git a/apps/ember-admin/app/components/member/subscription-detail-box.hbs b/apps/ember-admin/app/components/member/subscription-detail-box.hbs deleted file mode 100644 index d23beef001d..00000000000 --- a/apps/ember-admin/app/components/member/subscription-detail-box.hbs +++ /dev/null @@ -1,49 +0,0 @@ - -
    -
    -
    -

    - Created — {{@sub.startDate}} -

    - {{#if (eq @sub.attribution.referrerSource 'Unknown')}} - {{else}} -

    - Source — {{@sub.attribution.referrerSource}} -

    - {{/if}} - {{#if (and @sub.attribution @sub.attribution.title)}} -

    - {{#if (and @sub.attribution.id (eq @sub.attribution.type "post"))}} - Page — {{ @sub.attribution.title }} - {{else if @sub.attribution.url}} - Page — {{ @sub.attribution.title }} - {{else}} - Page — {{ @sub.attribution.title }} - {{/if}} -

    - {{/if}} -
    - {{#if this.formattedOffers.length}} -
    -
    - {{#each this.formattedOffers as |offer|}} -

    - {{offer.label}} — {{offer.detail}} -

    - {{/each}} -
    -
    - {{/if}} - {{#if @sub.cancellationReason}} -
    -
    -

    Cancellation reason

    -
    {{@sub.cancellationReason}}
    -
    -
    - {{/if}} -
    -
    diff --git a/apps/ember-admin/app/components/member/subscription-detail-box.js b/apps/ember-admin/app/components/member/subscription-detail-box.js deleted file mode 100644 index d58ac5bef1c..00000000000 --- a/apps/ember-admin/app/components/member/subscription-detail-box.js +++ /dev/null @@ -1,17 +0,0 @@ -import Component from '@glimmer/component'; -import {action} from '@ember/object'; -import {getOfferDisplayData} from 'ghost-admin/utils/subscription-data'; -import {tracked} from '@glimmer/tracking'; - -export default class SubscriptionDetailBox extends Component { - @tracked showDetails = false; - - get formattedOffers() { - return (this.args.sub.offer_redemptions ?? []).map(offer => getOfferDisplayData(offer, this.args.sub)); - } - - @action - toggleSubscriptionExpanded() { - this.showDetails = !this.showDetails; - } -} diff --git a/apps/ember-admin/app/components/members/modals/delete-member.hbs b/apps/ember-admin/app/components/members/modals/delete-member.hbs deleted file mode 100644 index 8301e9b3419..00000000000 --- a/apps/ember-admin/app/components/members/modals/delete-member.hbs +++ /dev/null @@ -1,52 +0,0 @@ -
    -
    -

    Delete member account

    -
    - - -
    -

    - Permanently delete {{@data.member.email}} from Ghost. -

    - - {{#if this.hasActiveStripeSubscriptions}} -
    -
    - -
    -
    - {{/if}} -
    - -
    - - -
    -
    diff --git a/apps/ember-admin/app/components/members/modals/delete-member.js b/apps/ember-admin/app/components/members/modals/delete-member.js deleted file mode 100644 index 78db057f3fd..00000000000 --- a/apps/ember-admin/app/components/members/modals/delete-member.js +++ /dev/null @@ -1,47 +0,0 @@ -import Component from '@glimmer/component'; -import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; -import {tracked} from '@glimmer/tracking'; - -export default class DeleteMemberModal extends Component { - @service notifications; - - @tracked shouldCancelSubscriptions = false; - - get member() { - return this.args.data.member; - } - - get hasActiveStripeSubscriptions() { - const subscriptions = this.member.get('subscriptions'); - - if (!subscriptions || subscriptions.length === 0) { - return false; - } - - const firstActiveStripeSubscription = subscriptions.find((subscription) => { - return ['active', 'trialing', 'unpaid', 'past_due'].includes(subscription.status); - }); - - return firstActiveStripeSubscription !== undefined; - } - - @task({drop: true}) - *deleteMemberTask() { - const options = { - adapterOptions: { - cancel: this.shouldCancelSubscriptions - } - }; - - try { - yield this.member.destroyRecord(options); - this.args.data.afterDelete?.(); - this.args.close(true); - } catch (e) { - this.notifications.showAPIError(e, {key: 'member.delete'}); - this.args.close(false); - throw e; - } - } -} diff --git a/apps/ember-admin/app/components/members/modals/disable-commenting.hbs b/apps/ember-admin/app/components/members/modals/disable-commenting.hbs deleted file mode 100644 index 39591a41081..00000000000 --- a/apps/ember-admin/app/components/members/modals/disable-commenting.hbs +++ /dev/null @@ -1,45 +0,0 @@ - diff --git a/apps/ember-admin/app/components/members/modals/disable-commenting.js b/apps/ember-admin/app/components/members/modals/disable-commenting.js deleted file mode 100644 index 07e657ae0cd..00000000000 --- a/apps/ember-admin/app/components/members/modals/disable-commenting.js +++ /dev/null @@ -1,39 +0,0 @@ -import Component from '@glimmer/component'; -import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; -import {tracked} from '@glimmer/tracking'; - -export default class DisableCommentingModal extends Component { - @service notifications; - @service ajax; - @service ghostPaths; - - @tracked hideComments = false; - - get member() { - return this.args.data.member; - } - - @task({drop: true}) - *disableCommentingTask() { - try { - const url = this.ghostPaths.url.api('members', this.member.id, 'commenting', 'disable'); - yield this.ajax.post(url, { - data: JSON.stringify({ - reason: 'Disabled from member settings', - hide_comments: this.hideComments - }), - contentType: 'application/json' - }); - - this.args.data.afterDisable?.(); - this.notifications.showNotification(`Commenting has been disabled for ${this.member.name || this.member.email}.`, {type: 'success'}); - this.args.close(true); - return true; - } catch (e) { - this.notifications.showAPIError(e, {key: 'member.disable-commenting'}); - this.args.close(false); - throw e; - } - } -} diff --git a/apps/ember-admin/app/components/members/modals/logout-member.hbs b/apps/ember-admin/app/components/members/modals/logout-member.hbs deleted file mode 100644 index 824fd5615ac..00000000000 --- a/apps/ember-admin/app/components/members/modals/logout-member.hbs +++ /dev/null @@ -1,30 +0,0 @@ -
    -
    -

    Sign out member from all devices?

    -
    - - -
    -

    - {{or @data.member.name @data.member.email}} will be signed out of all active sessions, and will need to sign in again upon return. -

    -
    - -
    - - -
    -
    diff --git a/apps/ember-admin/app/components/members/modals/logout-member.js b/apps/ember-admin/app/components/members/modals/logout-member.js deleted file mode 100644 index f07de06de55..00000000000 --- a/apps/ember-admin/app/components/members/modals/logout-member.js +++ /dev/null @@ -1,31 +0,0 @@ -import Component from '@glimmer/component'; -import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; - -export default class LogoutMemberModal extends Component { - @service notifications; - @service ajax; - @service ghostPaths; - - get member() { - return this.args.data.member; - } - - @task({drop: true}) - *logoutMemberTask() { - try { - const url = this.ghostPaths.url.api('/members/', this.member.id, '/sessions/'); - const options = {}; - yield this.ajax.delete(url, options); - - this.args.data.afterLogout?.(); - this.notifications.showNotification(`${this.member.name || this.member.email} has been signed out from all devices.`, {type: 'success'}); - this.args.close(true); - return true; - } catch (e) { - this.notifications.showAPIError(e, {key: 'member.logout'}); - this.args.close(false); - throw e; - } - } -} diff --git a/apps/ember-admin/app/components/modal-member-tier.hbs b/apps/ember-admin/app/components/modal-member-tier.hbs deleted file mode 100644 index 4486cb2c432..00000000000 --- a/apps/ember-admin/app/components/modal-member-tier.hbs +++ /dev/null @@ -1,89 +0,0 @@ -
    -

    Add subscription

    -
    - - -
    -
    -

    - Select a tier for {{or this.member.name this.member.email}}'s - complimentary subscription. -

    - {{#if this.activeSubscriptions.length}} -

    - Adding a complimentary subscription cancels all existing subscriptions of this member. -

    - {{/if}} - {{#if this.loadingTiers}} -
    - -
    - {{else}} -
    - {{#each this.tiers as |tier|}} -
    -
    -
    -
    -

    {{tier.name}}

    -

    {{tier.description}}

    -
    - {{svg-jar "check" class="check"}} -
    -
    -
    -
    - {{/each}} -
    - - - - - {{svg-jar "arrow-down-small"}} - - {{#if (eq this.expiryAt "custom")}} - - {{/if}} - - - {{/if}} -
    -
    - -
    - - - - -
    \ No newline at end of file diff --git a/apps/ember-admin/app/components/modal-member-tier.js b/apps/ember-admin/app/components/modal-member-tier.js deleted file mode 100644 index 733d08e3e6b..00000000000 --- a/apps/ember-admin/app/components/modal-member-tier.js +++ /dev/null @@ -1,185 +0,0 @@ -import ModalComponent from 'ghost-admin/components/modal-base'; -import moment from 'moment-timezone'; -import {action} from '@ember/object'; -import {didCancel, task} from 'ember-concurrency'; -import {inject as service} from '@ember/service'; -import {tracked} from '@glimmer/tracking'; - -export default class ModalMemberTier extends ModalComponent { - @service store; - @service ghostPaths; - @service ajax; - - @tracked price; - @tracked tier; - @tracked tiers = []; - @tracked selectedTier = null; - @tracked loadingTiers = false; - @tracked expiryAt = 'forever'; - @tracked customExpiryDate = moment().startOf('day'); - - @tracked expiryOptions = [ - { - label: 'Forever', - duration: 'forever' - }, - { - label: '1 Week', - duration: 'week' - }, - { - label: '1 Month', - duration: 'month' - }, - { - label: '6 Months', - duration: 'half-year' - }, - { - label: '1 Year', - duration: 'year' - }, - { - label: 'Custom', - duration: 'custom' - } - ]; - - @task({drop: true}) - *fetchTiers() { - this.tiers = yield this.store.query('tier', {filter: 'type:paid+active:true', include: 'monthly_price,yearly_price,benefits'}); - - this.loadingTiers = false; - if (this.tiers.length > 0) { - this.selectedTier = this.tiers.firstObject.id; - } - } - - get activeSubscriptions() { - const subscriptions = this.member.get('subscriptions') || []; - return subscriptions.filter((sub) => { - return ['active', 'trialing', 'unpaid', 'past_due'].includes(sub.status); - }); - } - - get member() { - return this.model; - } - - get cannotAddPrice() { - return !this.price || this.price.amount !== 0; - } - - get minCustomDate() { - return moment().startOf('day'); - } - - @action - setup() { - this.loadingTiers = true; - try { - this.fetchTiers.perform(); - } catch (e) { - // Do not throw cancellation errors - if (didCancel(e)) { - return; - } - - throw e; - } - } - - @action - setTier(tierId) { - this.selectedTier = tierId; - } - - @action - setPrice(price) { - this.price = price; - } - - @action - confirmAction() { - return this.addTier.perform(); - } - - @action - close(event) { - event?.preventDefault?.(); - this.closeModal(); - } - - @action - updateExpiry(expiryDuration) { - this.expiryAt = expiryDuration; - } - - @action - updateCustomExpiryDate(date) { - this.customExpiryDate = moment(date).startOf('day'); - } - - @task({drop: true}) - *addTier() { - const url = `${this.ghostPaths.url.api(`members/${this.member.get('id')}`)}?include=tiers`; - - // Cancel existing active subscriptions for member - for (let i = 0; i < this.activeSubscriptions.length; i++) { - const subscription = this.activeSubscriptions[i]; - const cancelUrl = this.ghostPaths.url.api(`members/${this.member.get('id')}/subscriptions/${subscription.id}`); - yield this.ajax.put(cancelUrl, { - data: { - status: 'canceled' - } - }); - } - - let expiryAt = null; - - if (this.expiryAt === 'week') { - expiryAt = moment.utc().add(7, 'days').startOf('day').toISOString(); - } else if (this.expiryAt === 'month') { - expiryAt = moment.utc().add(1, 'month').startOf('day').toISOString(); - } else if (this.expiryAt === 'half-year') { - expiryAt = moment.utc().add(6, 'months').startOf('day').toISOString(); - } else if (this.expiryAt === 'year') { - expiryAt = moment.utc().add(1, 'year').startOf('day').toISOString(); - } else if (this.expiryAt === 'custom') { - expiryAt = this.customExpiryDate - .endOf('day') - .set('milliseconds', 0) // Prevent db rounding up to the next day - .add(moment().utcOffset(), 'minutes') // Adjust for timezone offset - .toISOString(); - } - const tiersData = { - id: this.selectedTier - }; - if (expiryAt) { - tiersData.expiry_at = expiryAt; - } - const response = yield this.ajax.put(url, { - data: { - members: [{ - id: this.member.get('id'), - email: this.member.get('email'), - tiers: [tiersData] - }] - } - }); - - this.store.pushPayload('member', response); - this.closeModal(); - return response; - } - - actions = { - confirm() { - this.confirmAction(...arguments); - }, - // needed because ModalBase uses .send() for keyboard events - closeModal() { - this.close(); - } - }; -} diff --git a/apps/ember-admin/app/components/posts/debug.hbs b/apps/ember-admin/app/components/posts/debug.hbs index 6d4661a78f6..92ce4d1aa94 100644 --- a/apps/ember-admin/app/components/posts/debug.hbs +++ b/apps/ember-admin/app/components/posts/debug.hbs @@ -68,13 +68,13 @@
    {{#if failure.member.id}} - +

    {{failure.recipient.name}}

    {{failure.recipient.email}}

    -     + {{else}}
    @@ -125,13 +125,13 @@
    {{#if failure.member.id}} - +

    {{failure.recipient.name}}

    {{failure.recipient.email}}

    -     + {{else}}
    diff --git a/apps/ember-admin/app/controllers/member.js b/apps/ember-admin/app/controllers/member.js deleted file mode 100644 index 2e002e76ffa..00000000000 --- a/apps/ember-admin/app/controllers/member.js +++ /dev/null @@ -1,352 +0,0 @@ -import Controller from '@ember/controller'; -import DeleteMemberModal from '../components/members/modals/delete-member'; -import DisableCommentingModal from '../components/members/modals/disable-commenting'; -import EmberObject, {action, defineProperty} from '@ember/object'; -import LogoutMemberModal from '../components/members/modals/logout-member'; -import boundOneWay from 'ghost-admin/utils/bound-one-way'; -import moment from 'moment-timezone'; -import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; -import {tracked} from '@glimmer/tracking'; - -const SCRATCH_PROPS = ['name', 'email', 'note']; - -export default class MemberController extends Controller { - @service ajax; - @service session; - @service dropdown; - @service feature; - @service ghostPaths; - @service membersStats; - @service membersCountCache; - @service modals; - @service notifications; - @service router; - @service labelsManager; - @service stateBridge; - @service store; - - queryParams = [ - {postAnalytics: 'post'}, - {backPath: 'back'} - ]; - - @tracked isLoading = false; - @tracked showImpersonateMemberModal = false; - @tracked modalLabel = null; - @tracked showLabelModal = false; - - _previousLabels = null; - _previousNewsletters = null; - - @tracked directlyFromAnalytics = false; - @tracked postAnalytics = null; - @tracked backPath = null; - - get fromAnalytics() { - if (!this.postAnalytics) { - return null; - } - return [this.postAnalytics]; - } - - get membersListPath() { - if (this.backPath?.startsWith('/members')) { - return this.backPath; - } - - if (this.postAnalytics) { - return `/members?post=${encodeURIComponent(this.postAnalytics)}`; - } - - return '/members'; - } - - get membersListUrl() { - return `#${this.membersListPath}`; - } - - constructor() { - super(...arguments); - this._availableLabels = this.store.peekAll('label'); - } - - // Computed properties ----------------------------------------------------- - - get member() { - return this.model; - } - - get memberTitle() { - if (this.member.isNew) { - return 'New member'; - } - - return this.member.name || this.member.email; - } - - set member(member) { - this.model = member; - } - - get dirtyAttributes() { - return this._hasDirtyAttributes(); - } - - get _labels() { - return this.member.get('labels').map(label => label.name); - } - - get _newsletters() { - return this.member.get('newsletters').map(newsletter => newsletter.id); - } - - get labelModalData() { - let label = this.modalLabel; - let labels = this.availableLabels; - - return { - label, - labels - }; - } - - get availableLabels() { - let labels = this._availableLabels - .filter(label => !label.isNew) - .filter(label => label.id !== null) - .sort((labelA, labelB) => labelA.name.localeCompare(labelB.name, undefined, {ignorePunctuation: true})); - let options = labels.toArray(); - - options.unshiftObject({name: 'All labels', slug: null}); - - return options; - } - - get scratchMember() { - let scratchMember = EmberObject.create({member: this.member}); - SCRATCH_PROPS.forEach(prop => defineProperty(scratchMember, prop, boundOneWay(`member.${prop}`))); - return scratchMember; - } - - get subscribedAt() { - let memberSince = moment(this.member.createdAtUTC).from(moment()); - let createdDate = moment(this.member.createdAtUTC).format('D MMM YYYY'); - return `${createdDate} (${memberSince})`; - } - - invalidateMembersCache() { - this.stateBridge.triggerEmberDataChange('update', 'member', this.member.id, null); - } - - invalidateMemberCommenting() { - this.invalidateMembersCache(); - this.stateBridge.triggerEmberDataChange('update', 'comment', this.member.id, null); - } - - // Actions ----------------------------------------------------------------- - - @action - setInitialRelationshipValues() { - this._previousLabels = this._labels; - this._previousNewsletters = this._newsletters; - } - - @action - toggleLabelModal() { - this.showLabelModal = !this.showLabelModal; - } - - @action - editLabel(label, e) { - if (e) { - e.preventDefault(); - e.stopPropagation(); - } - let modalLabel = this.availableLabels.findBy('slug', label); - this.modalLabel = modalLabel; - this.showLabelModal = !this.showLabelModal; - } - - @action - setProperty(propKey, value) { - this._saveMemberProperty(propKey, value); - } - - @action - confirmDeleteMember() { - this.modals.open(DeleteMemberModal, { - member: this.member, - afterDelete: () => { - this.membersStats.invalidate(); - this.invalidateMembersCache(); - this.membersCountCache.clear(); - this.router.transitionTo(this.membersListPath); - } - }); - } - - @action - confirmLogoutMember() { - this.modals.open(LogoutMemberModal, { - member: this.member, - afterLogout: () => { - this.invalidateMembersCache(); - } - }); - } - - @action - confirmDisableCommenting() { - this.modals.open(DisableCommentingModal, { - member: this.member, - afterDisable: () => { - this.invalidateMemberCommenting(); - this.fetchMemberTask.perform(this.member.id); - } - }); - } - - @action - async confirmEnableCommenting() { - this.dropdown.closeDropdowns(); - try { - const url = this.ghostPaths.url.api('members', this.member.id, 'commenting', 'enable'); - await this.ajax.post(url); - - this.invalidateMemberCommenting(); - - await this.fetchMemberTask.perform(this.member.id); - this.notifications.showNotification(`Commenting has been enabled for ${this.member.name || this.member.email}.`, {type: 'success'}); - } catch (e) { - this.notifications.showAPIError(e, {key: 'member.enable-commenting'}); - } - } - - @action - toggleImpersonateMemberModal() { - this.showImpersonateMemberModal = !this.showImpersonateMemberModal; - } - - @action - closeImpersonateMemberModal() { - this.showImpersonateMemberModal = false; - } - - @action - save() { - return this.saveTask.perform(); - } - - // Tasks ------------------------------------------------------------------- - - @task({drop: true}) - *saveTask() { - let {member, scratchMember} = this; - - // if Cmd+S is pressed before the field loses focus make sure we're - // saving the intended property values - let scratchProps = scratchMember.getProperties(SCRATCH_PROPS); - Object.assign(member, scratchProps); - - try { - const clearCountCache = member.isNew; // clear cache for adding new members so the count is updated without waiting for a refresh - - yield member.save(); - member.updateLabels(); - member.labels.forEach(label => this.labelsManager.addLabel(label)); - this.invalidateMembersCache(); - - this.setInitialRelationshipValues(); - - if (clearCountCache) { - this.membersCountCache.clear(); - } - - // replace 'member.new' route with 'member' route - this.replaceRoute('member', member); - - return member; - } catch (error) { - if (error === undefined) { - // Validation error - return; - } - - if (error.payload && error.payload.errors) { - for (const payloadError of error.payload.errors) { - if (payloadError.type === 'ValidationError' && payloadError.property && (payloadError.context || payloadError.message)) { - member.errors.add(payloadError.property, payloadError.context || payloadError.message); - member.hasValidated.pushObject(payloadError.property); - } - } - return; - } - - throw error; - } - } - - @task - *fetchMemberTask(memberId) { - this.isLoading = true; - - this.member = yield this.store.queryRecord('member', { - id: memberId, - include: 'tiers' - }); - - this.setInitialRelationshipValues(); - - this.isLoading = false; - } - - // Private ----------------------------------------------------------------- - - _saveMemberProperty(propKey, newValue) { - let currentValue = this.member[propKey]; - - if (newValue && typeof newValue === 'string') { - newValue = newValue.trim(); - } - - // avoid modifying empty values and triggering inadvertant unsaved changes modals - if (newValue !== false && !newValue && !currentValue) { - return; - } - - this.member[propKey] = newValue; - } - - _hasDirtyAttributes() { - let member = this.member; - - if (!member || member.isDeleted || member.isDeleting) { - return false; - } - - // member.labels is an array so hasDirtyAttributes doesn't pick up - // changes unless the array ref is changed. - // use sort() to sort of detect same item is re-added - let currentLabels = (this._labels.sort() || []).join(', '); - let previousLabels = (this._previousLabels.sort() || []).join(', '); - if (currentLabels !== previousLabels) { - return true; - } - - // member.newsletters is an array so hasDirtyAttributes doesn't pick up - // changes unless the array ref is changed - // use sort() to sort of detect same item is re-enabled - let currentNewsletters = (this._newsletters.sort() || []).join(', '); - let previousNewsletters = (this._previousNewsletters.sort() || []).join(', '); - if (currentNewsletters !== previousNewsletters) { - return true; - } - - // we've covered all the non-tracked cases we care about so fall - // back on Ember Data's default dirty attribute checks - let {hasDirtyAttributes} = member; - - return hasDirtyAttributes; - } -} diff --git a/apps/ember-admin/app/router.js b/apps/ember-admin/app/router.js index 8db9966b0d2..5059b89d940 100644 --- a/apps/ember-admin/app/router.js +++ b/apps/ember-admin/app/router.js @@ -41,8 +41,6 @@ Router.map(function () { this.route('migrate', {path: '/*platform'}); }); - this.route('member.new', {path: '/members/new'}); - this.route('member', {path: '/members/:member_id'}); this.route('members-activity'); this.route('react-fallback', {path: '/*path'}); diff --git a/apps/ember-admin/app/routes/member.js b/apps/ember-admin/app/routes/member.js deleted file mode 100644 index 145a98c4ff4..00000000000 --- a/apps/ember-admin/app/routes/member.js +++ /dev/null @@ -1,152 +0,0 @@ -import * as Sentry from '@sentry/ember'; -import ConfirmUnsavedChangesModal from '../components/modals/confirm-unsaved-changes'; -import MembersManagementRoute from './members-management'; -import {action} from '@ember/object'; -import {inject as service} from '@ember/service'; - -export default class MembersRoute extends MembersManagementRoute { - @service feature; - @service modals; - @service router; - @service('unsaved-changes') unsavedChanges; - - queryParams = { - postAnalytics: {refreshModel: false} - }; - - _requiresBackgroundRefresh = true; - _unregisterUnsavedChanges = null; - - constructor() { - super(...arguments); - this.router.on('routeWillChange', (transition) => { - this.closeImpersonateModal(transition); - }); - } - - beforeModel(transition) { - super.beforeModel(...arguments); - - // The outer React shell owns sibling URLs like /members/import. - // Ember's recognizer can still match this dynamic route for those - // paths and fire a bogus queryRecord that surfaces as an alert. - // Abort the transition for known React-owned siblings so the React - // shell keeps rendering the page. - const memberId = transition.to?.params?.member_id; - if (memberId === 'import') { - transition.abort(); - return; - } - - // React owns this URL when the flag is on. Aborting keeps the Ember - // subtree unrendered, so the `data-testid` and `data-test-link` - // attributes exist in only one tree and the queryRecord below doesn't - // fire for a screen nobody sees. - if (this.feature.memberDetailsReact === true) { - transition.abort(); - } - } - - model(params) { - this._requiresBackgroundRefresh = false; - - if (params.member_id) { - return this.store.queryRecord('member', {id: params.member_id, include: 'tiers'}); - } else { - return this.store.createRecord('member'); - } - } - - setupController(controller, member, transition) { - super.setupController(...arguments); - - controller.setInitialRelationshipValues(); - - if (this._requiresBackgroundRefresh) { - controller.fetchMemberTask.perform(member.id); - } - - controller.directlyFromAnalytics = false; - if (transition.from?.params?.path?.startsWith('posts/analytics')) { - controller.directlyFromAnalytics = true; - } - - this._registerUnsavedChanges(controller); - } - - resetController(controller, isExiting) { - super.resetController(...arguments); - - // Make sure we clear - if (isExiting) { - controller.set('backPath', null); - controller.set('directlyFromAnalytics', false); - } - - if (isExiting && controller.postAnalytics) { - controller.set('postAnalytics', null); - } - } - - deactivate() { - this._requiresBackgroundRefresh = true; - this._unregisterUnsavedChanges?.(); - this._unregisterUnsavedChanges = null; - } - - @action - save() { - this.controller.save(); - } - - @action - async willTransition(transition) { - return this.unsavedChanges.guardTransition(transition); - } - - closeImpersonateModal(transition) { - // If user navigates away with forward or back button, ensure returning to page - // hides modal - if (transition.from && transition.from.name === this.routeName && transition.targetName) { - let {controller} = this; - - controller.closeImpersonateMemberModal(transition); - } - } - - _registerUnsavedChanges(controller) { - this._unregisterUnsavedChanges?.(); - this._unregisterUnsavedChanges = this.unsavedChanges.register({ - isDirty: () => controller.dirtyAttributes, - confirmLeave: () => this._confirmUnsavedChanges(controller) - }); - } - - async _confirmUnsavedChanges(controller) { - if (controller.saveTask?.isRunning) { - try { - await controller.saveTask.last; - } catch (e) { - // ignore save errors — we'll check dirty state below - } - } - - if (!controller.dirtyAttributes) { - return true; - } - - Sentry.captureMessage('showing unsaved changes modal for members route'); - const shouldLeave = await this.modals.open(ConfirmUnsavedChangesModal); - - if (shouldLeave) { - controller.model.rollbackAttributes(); - return true; - } - - return false; - } - - titleToken() { - return this.controller.member.name; - } -} diff --git a/apps/ember-admin/app/routes/member/new.js b/apps/ember-admin/app/routes/member/new.js deleted file mode 100644 index 30c27fc4aa0..00000000000 --- a/apps/ember-admin/app/routes/member/new.js +++ /dev/null @@ -1,6 +0,0 @@ -import MemberRoute from '../member'; - -export default class NewMemberRoute extends MemberRoute { - controllerName = 'member'; - templateName = 'member'; -} diff --git a/apps/ember-admin/app/serializers/member.js b/apps/ember-admin/app/serializers/member.js index 08de9e1d9c9..5eb2b2082dc 100644 --- a/apps/ember-admin/app/serializers/member.js +++ b/apps/ember-admin/app/serializers/member.js @@ -18,9 +18,7 @@ export default class MemberSerializer extends ApplicationSerializer.extend(Embed delete json.status; delete json.last_seen_at; delete json.comped; - // Tiers are managed via direct API calls in gh-member-settings-form.js - // (removeComplimentaryTask) and modal-member-tier.js (addTier task), - // not through the normal member save flow + // Tiers are managed via direct API calls, not the member save flow delete json.tiers; // Normalize properties diff --git a/apps/ember-admin/app/services/feature.js b/apps/ember-admin/app/services/feature.js index 9408422d489..cd81e8e5a6c 100644 --- a/apps/ember-admin/app/services/feature.js +++ b/apps/ember-admin/app/services/feature.js @@ -80,7 +80,6 @@ export default class FeatureService extends Service { @feature('importMemberTier') importMemberTier; @feature('adminUIRefresh') adminUIRefresh; @feature('editorExcerpt') editorExcerpt; - @feature('memberDetailsReact') memberDetailsReact; @feature('tagDetailsReact') tagDetailsReact; @feature('paywallImprovements') paywallImprovements; @feature('automations') automations; diff --git a/apps/ember-admin/app/styles/app-dark.css b/apps/ember-admin/app/styles/app-dark.css index 503554e86ad..234b8a84f93 100644 --- a/apps/ember-admin/app/styles/app-dark.css +++ b/apps/ember-admin/app/styles/app-dark.css @@ -936,10 +936,6 @@ input, stroke: #fff; } -.gh-member-settings .gh-member-feed { - background: transparent; -} - .gh-member-feed-row { border-bottom: 1px solid var(--grey-900); } diff --git a/apps/ember-admin/app/styles/components/dropdowns.css b/apps/ember-admin/app/styles/components/dropdowns.css index 6ff834449f4..404573c34db 100644 --- a/apps/ember-admin/app/styles/components/dropdowns.css +++ b/apps/ember-admin/app/styles/components/dropdowns.css @@ -285,42 +285,6 @@ margin-right: 0; } -.gh-member-label-input .dropdown-action-icon { - opacity: 0; - transition: opacity ease-in-out 0.15s; - padding: 4px; - margin-top: -2px; - margin-bottom: -2px; - margin-right: 4px; - border-radius: 3px; -} - -.gh-member-settings .gh-member-label-input .dropdown-action-icon { - margin-right: -8px; - padding: 4px 6px; - color: var(--midgrey); -} - -.gh-member-settings .gh-member-label-input .dropdown-action-icon:hover { - color: var(--darkgrey); -} - -.gh-member-label-input li:hover .dropdown-action-icon { - opacity: 1.0; -} - -.gh-member-label-input .dropdown-action-icon:hover { - background: var(--whitegrey-d1); -} - -.gh-member-label-input .dropdown-action-icon svg { - margin: 0; - height: 14px; - width: 14px; - line-height: 1em; - fill: none; -} - /** Post context menu */ diff --git a/apps/ember-admin/app/styles/layouts/members.css b/apps/ember-admin/app/styles/layouts/members.css index 89206ed0efd..e8fb9d615e8 100644 --- a/apps/ember-admin/app/styles/layouts/members.css +++ b/apps/ember-admin/app/styles/layouts/members.css @@ -92,10 +92,6 @@ label[for="member-description"] + p { margin: 0 0 4px; } -.gh-member-settings .gh-main-section.columns-3 { - grid-column-gap: 48px; -} - .gh-member-details { position: sticky; top: 0px; @@ -415,16 +411,6 @@ textarea.gh-member-details-textarea { } @media (max-width: 1160px) { - .gh-member-settings .gh-main-section { - display: flex; - flex-direction: column; - } - - .gh-member-settings .gh-main-section > div { - float: none; - width: 100%; - } - .gh-member-details { position: relative; top: unset; @@ -605,13 +591,6 @@ textarea.gh-member-details-textarea { border-radius: 3px; } -.gh-member-settings .gh-member-feed-no-data { - margin: 0; - padding: 24px 0 28px; - background: transparent; - box-shadow: none; -} - .gh-member-feed-row { display: flex; align-items: center; diff --git a/apps/ember-admin/app/templates/member.hbs b/apps/ember-admin/app/templates/member.hbs deleted file mode 100644 index b8d3a0e2f21..00000000000 --- a/apps/ember-admin/app/templates/member.hbs +++ /dev/null @@ -1,146 +0,0 @@ -
    - -
    - {{#if this.fromAnalytics}} -
    - - Posts - - {{svg-jar "arrow-right-small"}} - - Analytics - - - {{#unless this.directlyFromAnalytics}} - {{svg-jar "arrow-right-small"}} - - Members - - {{/unless}} - - {{svg-jar "arrow-right-small"}} - {{this.memberTitle}} -
    - {{else}} -
    - - Members - - {{svg-jar "arrow-right-small"}} - {{this.memberTitle}} -
    - {{/if}} -
    - -
    - {{#if this.session.user.canManageMembers}} - {{#unless this.member.isNew}} - - - - {{svg-jar "settings"}} - - - - -
  • - -
    • -
  • - -
    • -
  • - {{#if (not-eq this.member.canComment false)}} - - {{else}} - - {{/if}} -
    • -
  • - -
    • -     -     - {{/unless}} - {{/if}} - - -
    -     - -
    -
    - - -
    -
    - -{{#if this.showImpersonateMemberModal}} - -{{/if}} - -{{#if this.showLabelModal}} - -{{/if}} diff --git a/apps/ember-admin/app/utils/subscription-data.js b/apps/ember-admin/app/utils/subscription-data.js deleted file mode 100644 index 77fd2327f27..00000000000 --- a/apps/ember-admin/app/utils/subscription-data.js +++ /dev/null @@ -1,225 +0,0 @@ -import moment from 'moment-timezone'; -import {getNonDecimal, getSymbol} from 'ghost-admin/utils/currency'; - -export function getSubscriptionData(sub) { - const data = { - ...sub, - attribution: { - ...sub.attribution, - referrerSource: sub.attribution?.referrer_source || 'Unknown', - referrerMedium: sub.attribution?.referrer_medium || '-' - }, - startDate: sub.start_date ? moment(sub.start_date).format('D MMM YYYY') : '-', - validUntil: validUntil(sub), - hasEnded: isCanceled(sub), - willEndSoon: isSetToCancel(sub), - cancellationReason: sub.cancellation_reason, - price: { - ...sub.price, - currencySymbol: getSymbol(sub.price.currency), - nonDecimalAmount: getNonDecimal(sub.price.amount) - }, - isComplimentary: isComplimentary(sub), - isGift: isGift(sub), - compExpiry: compExpiry(sub), - giftExpiry: giftExpiry(sub), - trialUntil: trialUntil(sub) - }; - - data.priceLabel = priceLabel(data); - data.validityDetails = validityDetails(data, !!data.priceLabel); - - const discount = getDiscountPrice(sub); - if (discount) { - data.hasActiveDiscount = true; - data.discountedPrice = discount.discountedPrice; - data.originalPrice = discount.originalPrice; - } - - return data; -} - -export function validUntil(sub) { - // If a subscription has been canceled immediately, don't render the end of validity date - // Reason: we don't store the exact cancelation date in the subscription object - if (sub.status === 'canceled' && !sub.cancel_at_period_end) { - return ''; - } - - // Otherwise, show the current period end date - if (sub.current_period_end) { - return moment(sub.current_period_end).format('D MMM YYYY'); - } - - return ''; -} - -export function isActive(sub) { - return ['active', 'trialing', 'past_due', 'unpaid'].includes(sub.status); -} - -export function isComplimentary(sub) { - const compedNickname = sub.plan?.nickname?.toLowerCase() === 'complimentary'; - - return !sub.id && compedNickname; -} - -export function isGift(sub) { - const giftNickname = sub.plan?.nickname?.toLowerCase() === 'gift subscription'; - - return !sub.id && giftNickname; -} - -export function isCanceled(sub) { - return sub.status === 'canceled'; -} - -export function isSetToCancel(sub) { - return sub.cancel_at_period_end && isActive(sub); -} - -export function compExpiry(sub) { - if (!isComplimentary(sub)) { - return undefined; - } - - if (sub.tier && sub.tier.expiry_at) { - return moment(sub.tier.expiry_at).utc().format('D MMM YYYY'); - } - - return undefined; -} - -export function giftExpiry(sub) { - if (!isGift(sub)) { - return undefined; - } - - if (sub.tier && sub.tier.expiry_at) { - return moment(sub.tier.expiry_at).utc().format('D MMM YYYY'); - } - - return undefined; -} - -export function trialUntil(sub) { - const offer = sub.offer; - const isTrial = !offer || offer?.type === 'trial'; - const isTrialActive = isTrial && sub.trial_end_at && moment(sub.trial_end_at).isAfter(new Date(), 'day'); - - if (isTrialActive) { - return moment(sub.trial_end_at).format('D MMM YYYY'); - } - - return undefined; -} - -export function validityDetails(data, separatorNeeded = false) { - const separator = separatorNeeded ? ' – ' : ''; - const space = data.validUntil ? ' ' : ''; - - if (data.isComplimentary) { - if (data.compExpiry) { - return `${separator}Expires ${data.compExpiry}`; - } else { - return ''; - } - } - - if (data.isGift) { - if (data.giftExpiry) { - return `${separator}Expires ${data.giftExpiry}`; - } else { - return ''; - } - } - - if (data.hasEnded) { - return `${separator}Ended${space}${data.validUntil}`; - } - - if (data.willEndSoon) { - return `${separator}Has access until${space}${data.validUntil}`; - } - - if (data.trialUntil) { - return `${separator}Ends ${data.trialUntil}`; - } - - return `${separator}Renews${space}${data.validUntil}`; -} - -export function priceLabel(data) { - if (data.trialUntil) { - return 'Free trial'; - } - - if (data.price.nickname && data.price.nickname.length > 0 && data.price.nickname !== 'Monthly' && data.price.nickname !== 'Yearly') { - return data.price.nickname; - } -} - -export function getOfferDisplayData(offer, sub = {}) { - const isRetention = offer.redemption_type === 'retention'; - const label = isRetention ? 'Retention offer' : 'Signup offer'; - - const isFreeMonths = offer.type === 'percent' && offer.amount === 100 && offer.duration === 'repeating'; - - let discount; - if (offer.type === 'trial') { - discount = `${offer.amount} days free`; - } else if (isFreeMonths) { - discount = `${offer.duration_in_months} ${offer.duration_in_months === 1 ? 'month' : 'months'} free`; - } else if (offer.type === 'fixed') { - discount = `${getSymbol(offer.currency)}${getNonDecimal(offer.amount)} off`; - } else { - discount = `${offer.amount}% off`; - } - - let detail; - if (isRetention) { - const discountEnd = offer.id && sub.next_payment?.discount?.offer_id === offer.id - ? sub.next_payment.discount.end - : null; - - if (discountEnd) { - detail = `${discount} until ${moment(discountEnd).format('MMM YYYY')}`; - } else if (isFreeMonths) { - // "N months free" is self-contained, no need to append "for N months" - detail = discount; - } else if (offer.duration === 'repeating' && offer.duration_in_months) { - detail = `${discount} for ${offer.duration_in_months} ${offer.duration_in_months === 1 ? 'month' : 'months'}`; - } else if (offer.duration === 'forever') { - detail = `${discount} forever`; - } else { - detail = discount; - } - } else { - detail = `${offer.name} (${discount})`; - } - - return {label, detail}; -} - -export function getDiscountPrice(sub) { - if (!sub.next_payment || !sub.next_payment.discount) { - return null; - } - - if (sub.next_payment.amount === sub.next_payment.original_amount) { - return null; - } - - return { - discountedPrice: { - currencySymbol: getSymbol(sub.next_payment.currency), - nonDecimalAmount: getNonDecimal(sub.next_payment.amount), - amount: sub.next_payment.amount - }, - originalPrice: { - currencySymbol: getSymbol(sub.next_payment.currency), - nonDecimalAmount: getNonDecimal(sub.next_payment.original_amount), - amount: sub.next_payment.original_amount - } - }; -} diff --git a/apps/ember-admin/tests/acceptance/members/details-test.js b/apps/ember-admin/tests/acceptance/members/details-test.js deleted file mode 100644 index 8c74c364aca..00000000000 --- a/apps/ember-admin/tests/acceptance/members/details-test.js +++ /dev/null @@ -1,621 +0,0 @@ -import {authenticateSession} from 'ember-simple-auth/test-support'; -import {click, currentURL, fillIn, find, findAll} from '@ember/test-helpers'; -import {enableLabsFlag} from '../../helpers/labs-flag'; -import {enableNewsletters} from '../../helpers/newsletters'; -import {enableStripe} from '../../helpers/stripe'; -import {expect} from 'chai'; -import {setupApplicationTest} from 'ember-mocha'; -import {setupMirage} from 'ember-cli-mirage/test-support'; -import {visit} from '../../helpers/visit'; - -describe('Acceptance: Member details', function () { - let hooks = setupApplicationTest(); - setupMirage(hooks); - - let clock; - let tier; - - function subscriptionSummaryText(tierId) { - const priceLabel = find(`[data-test-tier="${tierId}"] .gh-cp-membertier-pricelabel`)?.textContent?.trim() || ''; - const renewal = find(`[data-test-tier="${tierId}"] .gh-cp-membertier-renewal`)?.textContent?.trim() || ''; - - return [priceLabel, renewal] - .filter(Boolean) - .join(' ') - .replace(/–/g, '-') - .replace(/\s+/g, ' ') - .trim(); - } - - beforeEach(async function () { - this.server.loadFixtures('configs'); - this.server.loadFixtures('settings'); - enableLabsFlag(this.server, 'membersLastSeenFilter'); - enableLabsFlag(this.server, 'membersTimeFilters'); - - enableStripe(this.server); - enableNewsletters(this.server, true); - - // add a default tier that complimentary plans can be assigned to - tier = this.server.create('tier', { - id: '6213b3f6cb39ebdb03ebd810', - name: 'Supporter', - slug: 'supporter', - created_at: '2022-02-21T16:47:02.000Z', - updated_at: '2022-03-03T15:37:02.000Z', - description: null, - monthly_price_id: '6220df272fee0571b5dd0a0a', - yearly_price_id: '6220df272fee0571b5dd0a0b', - type: 'paid', - active: true, - welcome_page_url: '/' - }); - - let role = this.server.create('role', {name: 'Owner'}); - this.server.create('user', {roles: [role]}); - - await authenticateSession(); - }); - - afterEach(function () { - clock?.restore(); - }); - - it('has a known base-state', async function () { - const member = this.server.create('member', { - id: 1, - subscriptions: [ - this.server.create('subscription', { - id: 'sub_1KZGcmEGb07FFvyN9jwrwbKu', - customer: { - id: 'cus_LFmBWoSkB84lnr', - name: 'test', - email: 'test@ghost.org' - }, - plan: { - id: 'price_1KZGc6EGb07FFvyNkK3umKiX', - nickname: 'Monthly', - amount: 500, - interval: 'month', - currency: 'USD' - }, - status: 'canceled', - start_date: '2022-03-03T15:31:27.000Z', - default_payment_card_last4: '4242', - cancel_at_period_end: false, - cancellation_reason: null, - current_period_end: '2022-04-03T15:31:27.000Z', - price: { - id: 'price_1KZGc6EGb07FFvyNkK3umKiX', - price_id: '6220df272fee0571b5dd0a0a', - nickname: 'Monthly', - amount: 500, - interval: 'month', - type: 'recurring', - currency: 'USD', - tier: { - id: 'prod_LFmAAmCnnbzrvL', - name: 'Supporter', - tier_id: tier.id - } - }, - offer: null - }), - this.server.create('subscription', { - id: 'sub_1KZGi6EGb07FFvyNDjZq98g8', - tier, - customer: { - id: 'cus_LFmGicpX4BkQKH', - name: '123', - email: 'test@ghost.org' - }, - plan: { - id: 'price_1KZGc6EGb07FFvyNkK3umKiX', - nickname: 'Monthly', - amount: 500, - interval: 'month', - currency: 'USD' - }, - status: 'active', - start_date: '2022-03-03T15:36:58.000Z', - default_payment_card_last4: '4242', - cancel_at_period_end: false, - cancellation_reason: null, - current_period_end: '2022-04-03T15:36:58.000Z', - price: { - id: 'price_1KZGc6EGb07FFvyNkK3umKiX', - price_id: '6220df272fee0571b5dd0a0a', - nickname: 'Monthly', - amount: 500, - interval: 'month', - type: 'recurring', - currency: 'USD', - tier: { - id: 'prod_LFmAAmCnnbzrvL', - name: 'Supporter', - tier_id: tier.id - } - }, - offer: null - }) - ], - tiers: [ - tier - ] - }); - - await visit(`/members/${member.id}`); - - expect(currentURL()).to.equal(`/members/${member.id}`); - - expect(findAll('[data-test-subscription]').length, 'displays all member subscriptions') - .to.equal(2); - await click('[data-test-button="save"]'); - expect(find('[data-test-button="save"]')).to.not.contain.text('Retry'); - }); - - it('displays correctly one canceled subscription', async function () { - const member = this.server.create('member', { - id: 1, - subscriptions: [ - this.server.create('subscription', { - id: 'sub_1KZGcmEGb07FFvyN9jwrwbKu', - tier, - customer: { - id: 'cus_LFmBWoSkB84lnr', - name: 'test', - email: 'test@ghost.org' - }, - plan: { - id: 'price_1KZGc6EGb07FFvyNkK3umKiX', - nickname: 'Monthly', - amount: 500, - interval: 'month', - currency: 'USD' - }, - status: 'canceled', - start_date: '2022-03-03T15:31:27.000Z', - default_payment_card_last4: '4242', - cancel_at_period_end: false, - cancellation_reason: null, - current_period_end: '2022-04-03T15:31:27.000Z', - price: { - id: 'price_1KZGc6EGb07FFvyNkK3umKiX', - price_id: '6220df272fee0571b5dd0a0a', - nickname: 'Monthly', - amount: 500, - interval: 'month', - type: 'recurring', - currency: 'USD', - tier: { - id: 'prod_LFmAAmCnnbzrvL', - name: 'Supporter', - tier_id: '6213b3f6cb39ebdb03ebd810' - } - }, - offer: null - }) - ], - tiers: [] - }); - - await visit(`/members/${member.id}`); - - expect(currentURL()).to.equal(`/members/${member.id}`); - - expect(findAll('[data-test-subscription]').length, 'displays all member subscriptions') - .to.equal(1); - }); - - it('renders non-discounted subscription prices with trailing zeros', async function () { - const member = this.server.create('member', { - id: 1, - subscriptions: [ - this.server.create('subscription', { - id: 'sub_1KZGi6EGb07FFvyNDjZq98g8', - tier, - customer: { - id: 'cus_LFmGicpX4BkQKH', - name: '123', - email: 'test@ghost.org' - }, - plan: { - id: 'price_1KZGc6EGb07FFvyNkK3umKiX', - nickname: 'Monthly', - amount: 590, - interval: 'month', - currency: 'USD' - }, - status: 'active', - start_date: '2022-03-03T15:36:58.000Z', - default_payment_card_last4: '4242', - cancel_at_period_end: false, - cancellation_reason: null, - current_period_end: '2022-04-03T15:36:58.000Z', - price: { - id: 'price_1KZGc6EGb07FFvyNkK3umKiX', - price_id: '6220df272fee0571b5dd0a0a', - nickname: 'Monthly', - amount: 590, - interval: 'month', - type: 'recurring', - currency: 'USD', - tier: { - id: 'prod_LFmAAmCnnbzrvL', - name: 'Supporter', - tier_id: tier.id - } - }, - offer: null - }) - ], - tiers: [ - tier - ] - }); - - await visit(`/members/${member.id}`); - - const amountElement = find(`[data-test-tier="${tier.id}"] .gh-tier-card-price .amount`); - expect(amountElement.textContent.trim()).to.equal('5.90'); - }); - - it('can add and remove complimentary subscription', async function () { - const member = this.server.create('member', {name: 'Comp Member Test'}); - - await visit(`/members/${member.id}`); - - expect(findAll('[data-test-button="add-complimentary"]').length, '# of add complimentary buttons') - .to.equal(1); - - await click('[data-test-button="add-complimentary"]'); - expect(find('[data-test-modal="member-tier"]'), 'select tier modal').to.exist; - expect(find('[data-test-text="select-tier-desc"]')).to.contain.text('Comp Member Test'); - expect(find('[data-test-tier-option="6213b3f6cb39ebdb03ebd810"]')).to.have.exist; - expect(find('[data-test-tier-option="6213b3f6cb39ebdb03ebd810"]')).to.have.class('active'); - await click('[data-test-button="save-comp-tier"]'); - - expect(findAll('[data-test-subscription]').length, '# of subscription blocks - after add comped') - .to.equal(1); - - await click('[data-test-tier="6213b3f6cb39ebdb03ebd810"] [data-test-button="subscription-actions"]'); - await click('[data-test-tier="6213b3f6cb39ebdb03ebd810"] [data-test-button="remove-complimentary"]'); - - expect(findAll('[data-test-subscription]').length, '# of subscription blocks - after remove comped') - .to.equal(0); - }); - - it('can add complimentary subscription when member has canceled subscriptions', async function () { - const member = this.server.create('member', { - name: 'Comped for canceled sub test', - subscriptions: [ - this.server.create('subscription', { - // tier, // _Not_ included as `tier` when subscription is canceled - status: 'canceled', - price: { - id: 'price_1', - tier: { - id: 'prod_1', - tier_id: tier.id - } - } - }) - ] - }); - - await visit(`/members/${member.id}`); - - expect(findAll('[data-test-button="add-complimentary"]').length, '# of add complimentary buttons') - .to.equal(1); - - await click('[data-test-button="add-complimentary"]'); - await click('[data-test-button="save-comp-tier"]'); - - expect(findAll('[data-test-subscription]').length, '# of subscription blocks - after add comped') - .to.equal(2); - expect(findAll('[data-test-button="add-complimentary"]').length, '# of add complimentary buttons - after add comped') - .to.equal(0); - }); - - it('displays gift subscriptions with expiry text and without an action menu', async function () { - const giftTier = this.server.create('tier', { - id: 'gift-tier-1', - name: 'Gift tier', - slug: 'gift-tier', - created_at: '2022-02-21T16:47:02.000Z', - updated_at: '2022-03-03T15:37:02.000Z', - description: null, - monthly_price_id: 'gift-monthly-price', - yearly_price_id: 'gift-yearly-price', - type: 'paid', - active: true, - welcome_page_url: '/', - expiry_at: '2022-04-03T00:00:00.000Z' - }); - - const member = this.server.create('member', { - id: 1, - status: 'gift', - tiers: [giftTier], - subscriptions: [ - this.server.create('subscription', { - id: '', - tier: giftTier, - customer: { - id: '', - name: 'Gift Receiver', - email: 'gift@example.com' - }, - plan: { - id: '', - nickname: 'Gift subscription', - amount: 0, - interval: 'year', - currency: 'USD' - }, - status: 'active', - start_date: '2022-03-03T15:36:58.000Z', - default_payment_card_last4: '****', - cancel_at_period_end: false, - cancellation_reason: null, - current_period_end: '2022-04-03T15:36:58.000Z', - price: { - id: '', - price_id: '', - nickname: 'Gift subscription', - amount: 0, - interval: 'year', - type: 'recurring', - currency: 'USD', - tier: { - id: '', - tier_id: giftTier.id - } - }, - offer: null - }) - ] - }); - - await visit(`/members/${member.id}`); - - expect(currentURL()).to.equal(`/members/${member.id}`); - - const tierCard = find(`[data-test-tier="${giftTier.id}"]`); - - expect(subscriptionSummaryText(giftTier.id)).to.equal('Gift subscription - Expires 3 Apr 2022'); - expect(tierCard).to.contain.text('Gift subscription'); - expect(find(`[data-test-tier="${giftTier.id}"] [data-test-button="subscription-actions"]`)).to.not.exist; - }); - - it('displays comped subscriptions with expiry text and a complimentary action menu', async function () { - const compedTier = this.server.create('tier', { - id: 'comped-tier-1', - name: 'Comped tier', - slug: 'comped-tier', - created_at: '2022-02-21T16:47:02.000Z', - updated_at: '2022-03-03T15:37:02.000Z', - description: null, - monthly_price_id: 'comped-monthly-price', - yearly_price_id: 'comped-yearly-price', - type: 'paid', - active: true, - welcome_page_url: '/', - expiry_at: '2022-04-03T00:00:00.000Z' - }); - - const member = this.server.create('member', { - id: 1, - status: 'comped', - tiers: [compedTier], - subscriptions: [ - this.server.create('subscription', { - id: '', - tier: compedTier, - customer: { - id: '', - name: 'Comped Receiver', - email: 'comped@example.com' - }, - plan: { - id: '', - nickname: 'Complimentary', - amount: 0, - interval: 'year', - currency: 'USD' - }, - status: 'active', - start_date: '2022-03-03T15:36:58.000Z', - default_payment_card_last4: '****', - cancel_at_period_end: false, - cancellation_reason: null, - current_period_end: '2022-04-03T15:36:58.000Z', - price: { - id: '', - price_id: '', - nickname: 'Complimentary', - amount: 0, - interval: 'year', - type: 'recurring', - currency: 'USD', - tier: { - id: '', - tier_id: compedTier.id - } - }, - offer: null - }) - ] - }); - - await visit(`/members/${member.id}`); - - expect(currentURL()).to.equal(`/members/${member.id}`); - expect(subscriptionSummaryText(compedTier.id)).to.equal('Complimentary - Expires 3 Apr 2022'); - expect(find(`[data-test-tier="${compedTier.id}"] [data-test-button="subscription-actions"]`)).to.exist; - - await click(`[data-test-tier="${compedTier.id}"] [data-test-button="subscription-actions"]`); - expect(find('.tier-actions-menu')).to.contain.text('Remove complimentary subscription'); - }); - - it('displays paid subscriptions with Stripe actions', async function () { - const member = this.server.create('member', { - id: 1, - subscriptions: [ - this.server.create('subscription', { - id: 'sub_paid_1', - tier, - customer: { - id: 'cus_paid_1', - name: 'Paid Member', - email: 'paid@example.com' - }, - plan: { - id: 'price_paid_1', - nickname: 'Monthly', - amount: 500, - interval: 'month', - currency: 'USD' - }, - status: 'active', - start_date: '2022-03-03T15:36:58.000Z', - default_payment_card_last4: '4242', - cancel_at_period_end: false, - cancellation_reason: null, - current_period_end: '2022-04-03T15:36:58.000Z', - price: { - id: 'price_paid_1', - price_id: '6220df272fee0571b5dd0a0a', - nickname: 'Monthly', - amount: 500, - interval: 'month', - type: 'recurring', - currency: 'USD', - tier: { - id: 'prod_paid_1', - tier_id: tier.id - } - }, - offer: null - }) - ], - tiers: [tier] - }); - - await visit(`/members/${member.id}`); - - expect(currentURL()).to.equal(`/members/${member.id}`); - expect(subscriptionSummaryText(tier.id)).to.equal('Renews 3 Apr 2022'); - expect(find(`[data-test-tier="${tier.id}"] [data-test-button="subscription-actions"]`)).to.exist; - - await click(`[data-test-tier="${tier.id}"] [data-test-button="subscription-actions"]`); - expect(find('.tier-actions-menu')).to.contain.text('View Stripe customer'); - expect(find('.tier-actions-menu')).to.contain.text('View Stripe subscription'); - }); - - it('handles multiple tiers', async function () { - const tier2 = this.server.create('tier', { - name: 'Superfan', - slug: 'superfan', - created_at: '2022-02-21T16:47:02.000Z', - updated_at: '2022-03-03T15:37:02.000Z', - description: null, - monthly_price_id: '6220df272fee0571b5dd0a0a', - yearly_price_id: '6220df272fee0571b5dd0a0b', - type: 'paid', - active: true, - welcome_page_url: '/' - }); - - const member = this.server.create('member', {name: 'Multiple tier test'}); - - this.server.create('subscription', {member, tier, status: 'canceled', price: {id: '1', tier: {tier_id: tier.id}}}); - this.server.create('subscription', {member, tier, status: 'canceled', price: {id: '1', tier: {tier_id: tier.id}}}); - this.server.create('subscription', {member, tier: tier2, status: 'canceled', price: {id: '1', tier: {tier_id: tier2.id}}}); - - await visit(`/members/${member.id}`); - - // all subscriptions are shown - expect(findAll('[data-test-subscription]').length, '# of subscriptions shown').to.equal(3); - - // verify correct number of occurrences for each tier name - const supporterCount = findAll('[data-test-text="tier-name"]').filter(el => el.textContent.includes('Supporter')).length; - const superfanCount = findAll('[data-test-text="tier-name"]').filter(el => el.textContent.includes('Superfan')).length; - - expect(supporterCount, '# of Supporter tiers').to.equal(2); - expect(superfanCount, '# of Superfan tiers').to.equal(1); - - // can add complimentary - expect(findAll('[data-test-button="add-complimentary"]').length, '# of add-complimentary buttons').to.equal(1); - await click('[data-test-button="add-complimentary"]'); - await click(`[data-test-tier-option="${tier2.id}"]`); - await click('[data-test-button="save-comp-tier"]'); - - expect(findAll('[data-test-subscription]').length, '# of tiers after comp added').to.equal(4); - expect(findAll('[data-test-text="tier-name"]').filter(el => el.textContent.includes('Supporter')).length, '# of Supporter tiers after comp added').to.equal(2); - expect(findAll('[data-test-text="tier-name"]').filter(el => el.textContent.includes('Superfan')).length, '# of Superfan tiers after comp added').to.equal(2); - }); - - it('does not set comped status when saving member with stale tier data', async function () { - // Regression test: When a member's subscription was cancelled while the admin - // had the member page open, saving the member would incorrectly set status to - // 'comped' because stale tier data was sent in the request. - - // 1. Create a paid member with an active subscription - const member = this.server.create('member', { - name: 'Stale Tier Test', - status: 'paid', - tiers: [tier], - subscriptions: [ - this.server.create('subscription', { - tier, - status: 'active', - plan: { - id: 'price_1', - nickname: 'Monthly', - amount: 500, - interval: 'month', - currency: 'USD' - }, - price: { - id: 'price_1', - price_id: '6220df272fee0571b5dd0a0a', - nickname: 'Monthly', - amount: 500, - interval: 'month', - type: 'recurring', - currency: 'USD', - tier: { - id: 'prod_1', - tier_id: tier.id - } - } - }) - ] - }); - - // 2. Visit member page (loads tier data into client memory) - await visit(`/members/${member.id}`); - expect(currentURL()).to.equal(`/members/${member.id}`); - - // 3. Simulate backend cancellation - remove tier directly in mirage - // This mimics what happens when a subscription is cancelled via Stripe - // while the admin still has the member page open - member.update({ - tiers: [], - status: 'free' - }); - - // 4. Edit the member (change note) and save - // The client still has stale tier data in memory - await fillIn('[data-test-input="member-note"]', 'Testing stale tier data'); - await click('[data-test-button="save"]'); - - // 5. Verify save succeeded and member is still free, not comped - expect(find('[data-test-button="save"]')).to.not.contain.text('Retry'); - - const updatedMember = this.server.schema.members.find(member.id); - expect(updatedMember.status).to.equal('free'); - expect(updatedMember.tiers.length).to.equal(0); - }); -}); diff --git a/apps/ember-admin/tests/unit/controllers/member-test.js b/apps/ember-admin/tests/unit/controllers/member-test.js deleted file mode 100644 index e3da7b35329..00000000000 --- a/apps/ember-admin/tests/unit/controllers/member-test.js +++ /dev/null @@ -1,40 +0,0 @@ -import sinon from 'sinon'; -import {afterEach, beforeEach, describe, it} from 'mocha'; -import {expect} from 'chai'; -import {setupTest} from 'ember-mocha'; - -describe('Unit: Controller: member', function () { - setupTest(); - - let controller; - let triggerEmberDataChange; - - beforeEach(function () { - triggerEmberDataChange = sinon.spy(); - controller = this.owner.lookup('controller:member'); - Object.defineProperty(controller, 'stateBridge', { - configurable: true, - value: {triggerEmberDataChange} - }); - controller.member = {id: 'member-1'}; - }); - - afterEach(function () { - sinon.restore(); - }); - - it('invalidates the React members cache through the Ember bridge when member data changes', function () { - controller.invalidateMembersCache(); - - expect(triggerEmberDataChange.calledOnce).to.be.true; - expect(triggerEmberDataChange.calledWith('update', 'member', 'member-1', null)).to.be.true; - }); - - it('notifies the Ember bridge when member commenting changes', function () { - controller.invalidateMemberCommenting(); - - expect(triggerEmberDataChange.calledTwice).to.be.true; - expect(triggerEmberDataChange.firstCall.calledWith('update', 'member', 'member-1', null)).to.be.true; - expect(triggerEmberDataChange.secondCall.calledWith('update', 'comment', 'member-1', null)).to.be.true; - }); -}); diff --git a/apps/ember-admin/tests/unit/services/state-bridge-test.js b/apps/ember-admin/tests/unit/services/state-bridge-test.js index 837bc0ee456..1e2502f219e 100644 --- a/apps/ember-admin/tests/unit/services/state-bridge-test.js +++ b/apps/ember-admin/tests/unit/services/state-bridge-test.js @@ -65,10 +65,10 @@ describe('Unit: Service: state-bridge', function () { it('exposes the same strict Labs state used by Ember routes', function () { settings.settingsModel = {}; sinon.stub(feature, 'tagDetailsReact').get(() => true); - sinon.stub(feature, 'memberDetailsReact').get(() => 'true'); + sinon.stub(feature, 'adminUIRefresh').get(() => 'true'); expect(service.isFeatureEnabled('tagDetailsReact')).to.be.true; - expect(service.isFeatureEnabled('memberDetailsReact')).to.be.false; + expect(service.isFeatureEnabled('adminUIRefresh')).to.be.false; expect(service.isFeatureEnabled('missingFlag')).to.be.false; }); }); diff --git a/apps/ember-admin/tests/unit/utils/subscription-data-test.js b/apps/ember-admin/tests/unit/utils/subscription-data-test.js deleted file mode 100644 index 25c78a6e4ca..00000000000 --- a/apps/ember-admin/tests/unit/utils/subscription-data-test.js +++ /dev/null @@ -1,734 +0,0 @@ -import moment from 'moment-timezone'; -import {compExpiry, getDiscountPrice, getOfferDisplayData, getSubscriptionData, giftExpiry, isActive, isCanceled, isComplimentary, isGift, isSetToCancel, priceLabel, trialUntil, validUntil, validityDetails} from 'ghost-admin/utils/subscription-data'; -import {describe, it} from 'mocha'; -import {expect} from 'chai'; - -describe('Unit: Util: subscription-data', function () { - describe('validUntil', function () { - it('returns the end of the current billing period when the subscription is canceled at the end of the period', function () { - let sub = { - status: 'canceled', - cancel_at_period_end: true, - current_period_end: '2021-05-31' - }; - expect(validUntil(sub)).to.equal('31 May 2021'); - }); - - it('returns an empty string when the subscription is canceled immediately', function () { - let sub = { - status: 'canceled', - cancel_at_period_end: false, - current_period_end: '2021-05-31' - }; - expect(validUntil(sub)).to.equal(''); - }); - - it('returns the end of the current billing period when the subscription is active', function () { - let sub = { - status: 'active', - cancel_at_period_end: false, - current_period_end: '2021-05-31' - }; - expect(validUntil(sub)).to.equal('31 May 2021'); - }); - - it('returns the end of the current billing period when the subscription is in trial', function () { - let sub = { - status: 'trialing', - cancel_at_period_end: false, - current_period_end: '2021-05-31' - }; - expect(validUntil(sub)).to.equal('31 May 2021'); - }); - - it('returns the end of the current billing period when the subscription is past_due', function () { - let sub = { - status: 'past_due', - cancel_at_period_end: false, - current_period_end: '2021-05-31' - }; - expect(validUntil(sub)).to.equal('31 May 2021'); - }); - - it('returns the end of the current billing period when the subscription is unpaid', function () { - let sub = { - status: 'unpaid', - cancel_at_period_end: false, - current_period_end: '2021-05-31' - }; - expect(validUntil(sub)).to.equal('31 May 2021'); - }); - - // Extra data safety check, mainly for imported subscriptions - it('returns an empty string if the subcription is canceled immediately and has no current_period_start', function () { - let sub = { - status: 'canceled', - cancel_at_period_end: false - }; - expect(validUntil(sub)).to.equal(''); - }); - - // Extra data safety check, mainly for imported subscriptions - it('returns an empty string if the subscription has no current_period_end', function () { - let sub = { - status: 'active', - cancel_at_period_end: false - }; - expect(validUntil(sub)).to.equal(''); - }); - }); - - describe('isActive', function () { - it('returns true for active subscriptions', function () { - let sub = {status: 'active'}; - expect(isActive(sub)).to.be.true; - }); - - it('returns true for trialing subscriptions', function () { - let sub = {status: 'trialing'}; - expect(isActive(sub)).to.be.true; - }); - - it('returns true for past_due subscriptions', function () { - let sub = {status: 'past_due'}; - expect(isActive(sub)).to.be.true; - }); - - it('returns true for unpaid subscriptions', function () { - let sub = {status: 'unpaid'}; - expect(isActive(sub)).to.be.true; - }); - - it('returns false for canceled subscriptions', function () { - let sub = {status: 'canceled'}; - expect(isActive(sub)).to.be.false; - }); - }); - - describe('isComplimentary', function () { - it('returns true for complimentary subscriptions', function () { - let sub = {id: null, plan: {nickname: 'Complimentary'}}; - expect(isComplimentary(sub)).to.be.true; - }); - - it('returns false for paid subscriptions', function () { - let sub = {id: 'sub_123'}; - expect(isComplimentary(sub)).to.be.false; - }); - }); - - describe('isGift', function () { - it('returns true for gift subscriptions', function () { - let sub = {id: null, plan: {nickname: 'Gift subscription'}}; - expect(isGift(sub)).to.be.true; - }); - - it('returns false for complimentary subscriptions', function () { - let sub = {id: null, plan: {nickname: 'Complimentary'}}; - expect(isGift(sub)).to.be.false; - }); - }); - - describe('isCanceled', function () { - it('returns true for canceled subscriptions', function () { - let sub = {status: 'canceled'}; - expect(isCanceled(sub)).to.be.true; - }); - - it('returns false for active subscriptions', function () { - let sub = {status: 'active'}; - expect(isCanceled(sub)).to.be.false; - }); - }); - - describe('isSetToCancel', function () { - it('returns true for subscriptions set to cancel at the end of the period', function () { - let sub = {status: 'active', cancel_at_period_end: true}; - expect(isSetToCancel(sub)).to.be.true; - }); - - it('returns false for canceled subscriptions', function () { - let sub = {status: 'canceled', cancel_at_period_end: true}; - expect(isSetToCancel(sub)).to.be.false; - }); - }); - - describe('trialUntil', function () { - it('returns the trial end date for subscriptions in trial', function () { - let sub = {status: 'trialing', trial_end_at: '2222-05-31'}; - expect(trialUntil(sub)).to.equal('31 May 2222'); - }); - - it('returns the trial end date for trial offers', function () { - let sub = {status: 'active', trial_end_at: '2222-05-31', offer: {type: 'trial'}}; - expect(trialUntil(sub)).to.equal('31 May 2222'); - }); - - it('returns undefined for percent/100/repeating (free months) offers', function () { - let sub = {status: 'active', trial_end_at: '2222-05-31', offer: {type: 'percent', amount: 100, duration: 'repeating'}}; - expect(trialUntil(sub)).to.be.undefined; - }); - - it('returns undefined for subscriptions not in trial', function () { - let sub = {status: 'active'}; - expect(trialUntil(sub)).to.be.undefined; - }); - }); - - describe('compExpiry', function () { - it('returns the expiry date for complimentary subscriptions', function () { - let sub = { - id: null, - plan: {nickname: 'Complimentary'}, - tier: {expiry_at: moment.utc('2021-05-31').toISOString()} - }; - expect(compExpiry(sub)).to.equal('31 May 2021'); - }); - - it('returns undefined for gift subscriptions', function () { - let sub = { - id: null, - plan: {nickname: 'Gift subscription'}, - tier: {expiry_at: moment.utc('2021-05-31').toISOString()} - }; - expect(compExpiry(sub)).to.be.undefined; - }); - - it('returns undefined for paid subscriptions', function () { - let sub = {id: 'sub_123'}; - expect(compExpiry(sub)).to.be.undefined; - }); - }); - - describe('giftExpiry', function () { - it('returns the expiry date for gift subscriptions', function () { - let sub = { - id: null, - plan: {nickname: 'Gift subscription'}, - tier: {expiry_at: moment.utc('2021-05-31').toISOString()} - }; - expect(giftExpiry(sub)).to.equal('31 May 2021'); - }); - - it('returns undefined for complimentary subscriptions', function () { - let sub = { - id: null, - plan: {nickname: 'Complimentary'}, - tier: {expiry_at: moment.utc('2021-05-31').toISOString()} - }; - expect(giftExpiry(sub)).to.be.undefined; - }); - - it('returns undefined for paid subscriptions', function () { - let sub = {id: 'sub_123'}; - expect(giftExpiry(sub)).to.be.undefined; - }); - }); - - describe('priceLabel', function () { - it('returns "Free trial" for trial subscriptions', function () { - let data = {trialUntil: '31 May 2021'}; - expect(priceLabel(data)).to.equal('Free trial'); - }); - - it('returns nothing if the price nickname is the default "monthly" or "yearly"', function () { - let data = {price: {nickname: 'Monthly'}}; - expect(priceLabel(data)).to.be.undefined; - - data = {price: {nickname: 'Yearly'}}; - expect(priceLabel(data)).to.be.undefined; - }); - - it('returns the price nickname for non-default prices', function () { - let data = {price: {nickname: 'Custom'}}; - expect(priceLabel(data)).to.equal('Custom'); - }); - }); - - describe('validityDetails', function () { - it('returns "Expires {compExpiry}" for expired complimentary subscriptions', function () { - let data = { - isComplimentary: true, - compExpiry: '31 May 2021' - }; - expect(validityDetails(data)).to.equal('Expires 31 May 2021'); - }); - - it('returns "" for forever complimentary subscriptions', function () { - let data = { - isComplimentary: true, - compExpiry: undefined - }; - expect(validityDetails(data)).to.equal(''); - }); - - it('returns "Expires {giftExpiry}" for gift subscriptions', function () { - let data = { - isGift: true, - giftExpiry: '31 May 2021' - }; - expect(validityDetails(data)).to.equal('Expires 31 May 2021'); - }); - - it('returns "Ended {validUntil}" for canceled subscriptions', function () { - let data = { - hasEnded: true, - validUntil: '31 May 2021' - }; - expect(validityDetails(data)).to.equal('Ended 31 May 2021'); - }); - - it('returns "Has access until {validUntil}" for set to cancel subscriptions', function () { - let data = { - willEndSoon: true, - validUntil: '31 May 2021' - }; - expect(validityDetails(data)).to.equal('Has access until 31 May 2021'); - }); - - it('returns "Ends {validUntil}" for trial subscriptions', function () { - let data = { - trialUntil: '31 May 2021' - }; - expect(validityDetails(data)).to.equal('Ends 31 May 2021'); - }); - - it('returns "Renews {validUntil}" for active subscriptions', function () { - let data = { - validUntil: '31 May 2021' - }; - expect(validityDetails(data)).to.equal('Renews 31 May 2021'); - }); - }); - - describe('getSubscriptionData', function () { - it('returns the correct data for an active subscription', function () { - let sub = { - id: 'defined', - status: 'active', - cancel_at_period_end: false, - current_period_end: '2021-05-31', - trial_end_at: null, - tier: null, - price: { - currency: 'usd', - amount: 5000 - } - }; - let data = getSubscriptionData(sub); - - expect(data).to.include({ - isComplimentary: false, - isGift: false, - compExpiry: undefined, - giftExpiry: undefined, - hasEnded: false, - validUntil: '31 May 2021', - willEndSoon: false, - trialUntil: undefined, - priceLabel: undefined, - validityDetails: 'Renews 31 May 2021' - }); - }); - - it('returns the correct data for a trial subscription', function () { - let sub = { - id: 'defined', - status: 'trialing', - cancel_at_period_end: false, - current_period_end: '2222-05-31', - trial_end_at: '2222-05-31', - tier: null, - price: { - currency: 'usd', - amount: 5000 - } - }; - let data = getSubscriptionData(sub); - - expect(data).to.include({ - isComplimentary: false, - isGift: false, - compExpiry: undefined, - giftExpiry: undefined, - hasEnded: false, - validUntil: '31 May 2222', - willEndSoon: false, - trialUntil: '31 May 2222', - priceLabel: 'Free trial', - validityDetails: ' – Ends 31 May 2222' - }); - }); - - it('returns renews details for percent/100/repeating (free months) offers', function () { - let sub = { - id: 'defined', - status: 'active', - cancel_at_period_end: false, - current_period_end: '2222-05-31', - offer: { - type: 'percent', - amount: 100, - duration: 'repeating' - }, - tier: null, - price: { - currency: 'usd', - amount: 5000, - nickname: 'Free tier' - } - }; - let data = getSubscriptionData(sub); - - expect(data).to.include({ - isComplimentary: false, - isGift: false, - compExpiry: undefined, - giftExpiry: undefined, - hasEnded: false, - validUntil: '31 May 2222', - willEndSoon: false, - trialUntil: undefined, - priceLabel: 'Free tier', - validityDetails: ' – Renews 31 May 2222' - }); - }); - - it('returns the correct data for an immediately canceled subscription', function () { - let sub = { - id: 'defined', - status: 'canceled', - cancel_at_period_end: false, - current_period_end: '2021-05-31', - trial_end_at: null, - tier: null, - price: { - currency: 'usd', - amount: 5000 - } - }; - let data = getSubscriptionData(sub); - - expect(data).to.include({ - isComplimentary: false, - isGift: false, - compExpiry: undefined, - giftExpiry: undefined, - hasEnded: true, - validUntil: '', - willEndSoon: false, - trialUntil: undefined, - priceLabel: undefined, - validityDetails: 'Ended' - }); - }); - - it('returns the correct data for a subscription set to cancel at the end of the period', function () { - let sub = { - id: 'defined', - status: 'active', - cancel_at_period_end: true, - current_period_end: '2021-05-31', - trial_end_at: null, - tier: null, - price: { - currency: 'usd', - amount: 5000 - } - }; - let data = getSubscriptionData(sub); - - expect(data).to.include({ - isComplimentary: false, - isGift: false, - compExpiry: undefined, - giftExpiry: undefined, - hasEnded: false, - validUntil: '31 May 2021', - willEndSoon: true, - trialUntil: undefined, - priceLabel: undefined, - validityDetails: 'Has access until 31 May 2021' - }); - }); - - it('returns the correct data for a complimentary subscription active forever', function () { - let sub = { - id: null, - status: 'active', - cancel_at_period_end: false, - current_period_end: '2021-05-31', - trial_end_at: null, - plan: { - nickname: 'Complimentary' - }, - tier: { - expiry_at: null - }, - price: { - currency: 'usd', - amount: 0, - nickname: 'Complimentary' - } - }; - let data = getSubscriptionData(sub); - - expect(data).to.include({ - isComplimentary: true, - isGift: false, - compExpiry: undefined, - giftExpiry: undefined, - hasEnded: false, - validUntil: '31 May 2021', - willEndSoon: false, - trialUntil: undefined, - priceLabel: 'Complimentary', - validityDetails: '' - }); - }); - - it('returns the correct data for a complimentary subscription with an expiration date', function () { - let sub = { - id: null, - status: 'active', - cancel_at_period_end: false, - current_period_end: '2021-05-31', - trial_end_at: null, - plan: { - nickname: 'Complimentary' - }, - tier: { - expiry_at: moment.utc('2021-05-31').toISOString() - }, - price: { - currency: 'usd', - amount: 0, - nickname: 'Complimentary' - } - }; - let data = getSubscriptionData(sub); - - expect(data).to.include({ - isComplimentary: true, - isGift: false, - compExpiry: '31 May 2021', - giftExpiry: undefined, - hasEnded: false, - validUntil: '31 May 2021', - willEndSoon: false, - trialUntil: undefined, - priceLabel: 'Complimentary', - validityDetails: ' – Expires 31 May 2021' - }); - }); - - it('returns the correct data for a gift subscription with an expiration date', function () { - let sub = { - id: null, - status: 'active', - cancel_at_period_end: false, - current_period_end: '2021-05-31', - trial_end_at: null, - plan: { - nickname: 'Gift subscription' - }, - tier: { - expiry_at: moment.utc('2021-05-31').toISOString() - }, - price: { - currency: 'usd', - amount: 0, - nickname: 'Gift subscription' - } - }; - let data = getSubscriptionData(sub); - - expect(data).to.include({ - isComplimentary: false, - isGift: true, - compExpiry: undefined, - giftExpiry: '31 May 2021', - hasEnded: false, - validUntil: '31 May 2021', - willEndSoon: false, - trialUntil: undefined, - priceLabel: 'Gift subscription', - validityDetails: ' – Expires 31 May 2021' - }); - }); - - it('sets hasActiveDiscount with discounted and original prices', function () { - const sub = { - id: 'sub_1', - status: 'active', - cancel_at_period_end: false, - current_period_end: '2026-05-31', - price: {currency: 'usd', amount: 5000}, - next_payment: { - amount: 2500, - original_amount: 5000, - currency: 'usd', - discount: {offer_id: 'offer_1', end: '2026-09-01'} - } - }; - - const data = getSubscriptionData(sub); - - expect(data.hasActiveDiscount).to.be.true; - expect(data.discountedPrice).to.deep.equal({currencySymbol: '$', nonDecimalAmount: 25, amount: 2500}); - expect(data.originalPrice).to.deep.equal({currencySymbol: '$', nonDecimalAmount: 50, amount: 5000}); - }); - - it('does not set hasActiveDiscount when no discount', function () { - const sub = { - id: 'sub_1', - status: 'active', - cancel_at_period_end: false, - current_period_end: '2026-05-31', - price: {currency: 'usd', amount: 5000} - }; - - const data = getSubscriptionData(sub); - - expect(data.hasActiveDiscount).to.be.undefined; - }); - }); - - describe('getOfferDisplayData', function () { - const cases = [ - { - name: 'signup + percent', - offer: {redemption_type: 'signup', type: 'percent', name: 'Black Friday', amount: 30}, - expected: {label: 'Signup offer', detail: 'Black Friday (30% off)'} - }, - { - name: 'signup + fixed (USD)', - offer: {redemption_type: 'signup', type: 'fixed', name: 'Welcome Deal', amount: 500, currency: 'USD'}, - expected: {label: 'Signup offer', detail: 'Welcome Deal ($5 off)'} - }, - { - name: 'signup + fixed (EUR)', - offer: {redemption_type: 'signup', type: 'fixed', name: 'Euro Deal', amount: 1000, currency: 'EUR'}, - expected: {label: 'Signup offer', detail: 'Euro Deal (€10 off)'} - }, - { - name: 'signup + trial', - offer: {redemption_type: 'signup', type: 'trial', name: 'Try It', amount: 7}, - expected: {label: 'Signup offer', detail: 'Try It (7 days free)'} - }, - { - name: 'retention + percent + once', - offer: {id: 'offer_once_1', redemption_type: 'retention', type: 'percent', amount: 50, duration: 'once'}, - sub: {next_payment: {discount: {offer_id: 'offer_once_1', end: '2026-02-17T00:00:00.000Z'}}}, - expected: {label: 'Retention offer', detail: '50% off until Feb 2026'} - }, - { - name: 'retention + percent + repeating (no discount end)', - offer: {redemption_type: 'retention', type: 'percent', amount: 50, duration: 'repeating', duration_in_months: 3}, - expected: {label: 'Retention offer', detail: '50% off for 3 months'} - }, - { - name: 'retention + percent + repeating (1 month, no discount end)', - offer: {redemption_type: 'retention', type: 'percent', amount: 50, duration: 'repeating', duration_in_months: 1}, - expected: {label: 'Retention offer', detail: '50% off for 1 month'} - }, - { - name: 'retention + percent + repeating (with discount end)', - offer: {id: 'offer_1', redemption_type: 'retention', type: 'percent', amount: 50, duration: 'repeating', duration_in_months: 3}, - sub: {next_payment: {discount: {offer_id: 'offer_1', end: '2026-02-17T00:00:00.000Z'}}}, - expected: {label: 'Retention offer', detail: '50% off until Feb 2026'} - }, - { - name: 'retention + percent + forever', - offer: {redemption_type: 'retention', type: 'percent', amount: 25, duration: 'forever'}, - expected: {label: 'Retention offer', detail: '25% off forever'} - }, - { - name: 'retention + percent/100/repeating (free months, no discount end)', - offer: {redemption_type: 'retention', type: 'percent', amount: 100, duration: 'repeating', duration_in_months: 1}, - expected: {label: 'Retention offer', detail: '1 month free'} - }, - { - name: 'retention + percent/100/repeating (free months, with discount end)', - offer: {id: 'offer_2', redemption_type: 'retention', type: 'percent', amount: 100, duration: 'repeating', duration_in_months: 1}, - sub: {next_payment: {discount: {offer_id: 'offer_2', end: '2026-02-17T00:00:00.000Z'}}}, - expected: {label: 'Retention offer', detail: '1 month free until Feb 2026'} - }, - { - name: 'retention + discount end does not match offer id', - offer: {id: 'offer_1', redemption_type: 'retention', type: 'percent', amount: 50, duration: 'repeating', duration_in_months: 3}, - sub: {next_payment: {discount: {offer_id: 'offer_other', end: '2026-02-17T00:00:00.000Z'}}}, - expected: {label: 'Retention offer', detail: '50% off for 3 months'} - }, - { - name: 'missing redemption_type defaults to signup label', - offer: {type: 'percent', name: 'Legacy Offer', amount: 15}, - expected: {label: 'Signup offer', detail: 'Legacy Offer (15% off)'} - } - ]; - - cases.forEach(({name, offer, sub, expected}) => { - it(name, function () { - const result = getOfferDisplayData(offer, sub); - expect(result).to.deep.equal(expected); - }); - }); - }); - - describe('getDiscountPrice', function () { - it('returns null when there is no next_payment', function () { - expect(getDiscountPrice({price: {currency: 'usd', amount: 5000}})).to.be.null; - }); - - it('returns null when there is no discount on next_payment', function () { - expect(getDiscountPrice({ - price: {currency: 'usd', amount: 5000}, - next_payment: {amount: 5000, original_amount: 5000, currency: 'usd'} - })).to.be.null; - }); - - it('returns null when discounted amount equals original amount', function () { - expect(getDiscountPrice({ - price: {currency: 'usd', amount: 5000}, - next_payment: { - amount: 5000, - original_amount: 5000, - currency: 'usd', - discount: {offer_id: 'offer_1', end: '2026-09-01'} - } - })).to.be.null; - }); - - it('returns discounted and original prices for USD', function () { - const result = getDiscountPrice({ - price: {currency: 'usd', amount: 5000}, - next_payment: { - amount: 2500, - original_amount: 5000, - currency: 'usd', - discount: {offer_id: 'offer_1', end: '2026-09-01'} - } - }); - expect(result).to.deep.equal({ - discountedPrice: {currencySymbol: '$', nonDecimalAmount: 25, amount: 2500}, - originalPrice: {currencySymbol: '$', nonDecimalAmount: 50, amount: 5000} - }); - }); - - it('returns discounted and original prices for EUR', function () { - const result = getDiscountPrice({ - price: {currency: 'eur', amount: 10000}, - next_payment: { - amount: 7000, - original_amount: 10000, - currency: 'eur', - discount: {offer_id: 'offer_1', end: '2026-09-01'} - } - }); - expect(result).to.deep.equal({ - discountedPrice: {currencySymbol: '€', nonDecimalAmount: 70, amount: 7000}, - originalPrice: {currencySymbol: '€', nonDecimalAmount: 100, amount: 10000} - }); - }); - }); -}); diff --git a/e2e/helpers/pages/admin/members/member-details-page.ts b/e2e/helpers/pages/admin/members/member-details-page.ts index 54c8ed4c9b6..531bce41899 100644 --- a/e2e/helpers/pages/admin/members/member-details-page.ts +++ b/e2e/helpers/pages/admin/members/member-details-page.ts @@ -3,14 +3,12 @@ import {BasePage} from '@/helpers/pages'; import {Locator, Page} from '@playwright/test'; /** - * The member detail screen has two implementations behind the - * `memberDetailsReact` Labs flag, and this page object drives both without - * branching. Where the two render different markup for the same thing, the - * locator matches either. + * Page object for the member detail screen. * - * Role-based locators need no visibility filter — Playwright already skips - * hidden subtrees for them. `getByTestId` and attribute selectors don't, so - * those are filtered explicitly to whichever tree is actually on screen. + * Menu entries are matched as either a menu item or a button, because the + * screen renders both: subscription actions live in a Radix menu, while + * "Add complimentary subscription" is a plain button in the subscriptions + * section. */ const menuAction = (page: Page, name: string) => page.getByRole('menuitem', {name}).or(page.getByRole('button', {name})); @@ -29,8 +27,6 @@ class SettingsSection extends BasePage { super(page); this.memberActionsButton = page.getByTestId('member-actions').filter({visible: true}); - // Ember renders the menu contents as plain buttons; React renders them - // as Radix menu items. Match either. this.impersonateButton = menuAction(page, 'Impersonate'); this.signOutOfAllDevices = menuAction(page, 'Sign out of all devices'); this.disableCommentingButton = menuAction(page, 'Disable commenting'); @@ -77,17 +73,10 @@ export class MemberDetailsPage extends AdminPage { readonly newsletterSubscriptionCheckboxes: Locator; readonly engagementSection: Locator; readonly subscriptionActionsButton: Locator; - // The add-complimentary modal is the one place the two screens differ by - // widget rather than behaviour: Ember picks a tier with radios and saves - // from the modal footer, React uses dropdowns. These name Ember's controls - // so its test can drive them without a raw locator. - readonly emberCompTierOptions: Locator; - readonly reactCompTierOptions: Locator; - readonly emberSaveCompTierButton: Locator; + readonly compTierOptions: Locator; readonly cancelSubscriptionButton: Locator; readonly continueSubscriptionButton: Locator; readonly removeComplimentaryButton: Locator; - readonly addComplimentaryButton: Locator; constructor(page: Page) { super(page); @@ -116,34 +105,18 @@ export class MemberDetailsPage extends AdminPage { this.hideCommentsCheckbox = this.disableCommentingModal.getByRole('switch', {name: 'Hide all previous comments'}); this.commentingDisabledIndicator = page.getByText('Comments disabled'); - this.screenTitle = page.locator('[data-test-screen-title]') - .or(page.getByTestId('member-detail-title')) - .filter({visible: true}); - this.logoutConfirmModal = page.locator('[data-test-modal="logout-member"]') - .or(page.getByTestId('logout-member-modal')) - .filter({visible: true}); - // Ember puts the testid on a styled span sitting next to the real - // checkbox, so the checked state has to be read from the sibling - // input. React puts it straight on the Radix switch, which carries the - // state itself. - this.newsletterSubscriptionCheckboxes = this.newsletterSubscriptionToggles - .locator('..') - .getByRole('checkbox') - .or(this.newsletterSubscriptionToggles.and(page.getByRole('switch'))); + this.screenTitle = page.getByTestId('member-detail-title'); + this.logoutConfirmModal = page.getByRole('alertdialog', {name: 'Sign out member from all devices?'}); + // The testid sits on the Radix switch, which carries the checked state + // itself. + this.newsletterSubscriptionCheckboxes = this.newsletterSubscriptionToggles.and(page.getByRole('switch')); this.engagementSection = page.getByTestId('member-detail-engagement').filter({visible: true}); - // Same control, different attribute: Ember marks it `data-test-button`, - // React `data-testid`. - this.subscriptionActionsButton = page.locator('[data-test-button="subscription-actions"]') - .or(page.getByTestId('subscription-actions')) - .filter({visible: true}); + this.subscriptionActionsButton = page.getByRole('button', {name: 'Subscription menu'}); this.cancelSubscriptionButton = menuAction(page, 'Cancel subscription'); this.continueSubscriptionButton = menuAction(page, 'Continue subscription'); this.removeComplimentaryButton = menuAction(page, 'Remove complimentary subscription'); - this.addComplimentaryButton = menuAction(page, 'Add complimentary subscription'); - this.emberCompTierOptions = page.locator('[data-test-tier-option]'); - this.reactCompTierOptions = page.locator('[data-tier-id]'); - this.emberSaveCompTierButton = page.locator('[data-test-button="save-comp-tier"]'); + this.compTierOptions = page.getByRole('option'); this.customFieldsCard = page.getByTestId('member-custom-fields-field'); this.customFieldModal = page.getByTestId('member-custom-field-edit-modal'); @@ -169,17 +142,12 @@ export class MemberDetailsPage extends AdminPage { /** * Removes a complimentary subscription from the first subscription row. - * One implementation confirms the removal in a dialog and the other acts - * immediately, so the confirm step is best-effort — the caller only cares - * that the removal was requested. + * Removal always asks for confirmation first. */ async removeComplimentarySubscription(): Promise { await this.subscriptionActionsButton.first().click(); await this.removeComplimentaryButton.click(); - const confirm = this.page.getByRole('button', {name: 'Remove', exact: true}); - if (await confirm.isVisible().catch(() => false)) { - await confirm.click(); - } + await this.page.getByRole('button', {name: 'Remove', exact: true}).click(); } async clickNewsletterSubscriptionToggle(index: number = 0) { diff --git a/e2e/tests/admin/members/member-detail.test.ts b/e2e/tests/admin/members/member-detail.test.ts index cc449ca7ff9..0bfac55fdc6 100644 --- a/e2e/tests/admin/members/member-detail.test.ts +++ b/e2e/tests/admin/members/member-detail.test.ts @@ -7,10 +7,9 @@ import {usePerTestIsolation} from '@/helpers/playwright/isolation'; /** * Behaviour contract for `/members/:id`. * - * The assertions here were written to run against both the Ember and the React - * screen, so they describe what the screen does rather than how it is built. - * Keep them that way: a test that reaches for markup specific to the current - * implementation stops being a contract and starts being a snapshot. + * The assertions describe what the screen does rather than how it is built. + * Keep them that way: a test that reaches for implementation-specific markup + * stops being a contract and starts being a snapshot. */ usePerTestIsolation(); @@ -323,7 +322,7 @@ test.describe('Ghost Admin - Member Detail', () => { const member = await memberFactory.create({name: 'Newsletter Test', email: 'newsletter-toggle@ghost.org'}); await page.goto(memberPath(member.id)); - // Wait on the toggle, not the checkbox — Ember hides the real input + // Wait on the toggle, not the checkbox — the real input is hidden // behind a styled span, so the control is never visible itself. await expect(memberDetailsPage.newsletterSubscriptionToggles.first()).toBeVisible(); const initiallyChecked = await memberDetailsPage.newsletterSubscriptionCheckboxes.first().isChecked(); @@ -590,7 +589,7 @@ test.describe('Ghost Admin - Member Detail - screen-specific behaviour', () => { await page.goto(memberPath(member.id)); await page.getByRole('button', {name: /Add complimentary subscription/}).click(); await page.getByTestId('comp-tier-select').click(); - const option = memberDetailsPage.reactCompTierOptions.first(); + const option = memberDetailsPage.compTierOptions.first(); const chosenTierId = await option.getAttribute('data-tier-id'); await option.click(); await page.getByTestId('comp-add-confirm').click(); diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index 27fb55db26c..68114719172 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -27,8 +27,7 @@ const messages = { // flags in this list always return `true`, allows quick global enable prior to full flag removal const GA_FEATURES = [ - 'automationAnalytics', - 'memberDetailsReact' + 'automationAnalytics' ]; // These features are considered publicly available and can be enabled/disabled by users From c0b55b11c4e2555a0651a911a5c7d8b8e94456e1 Mon Sep 17 00:00:00 2001 From: Austin Burdine Date: Thu, 13 Aug 2026 11:30:27 -0400 Subject: [PATCH 24/25] Fixed flaky integration modal acceptance test (#29946) no ref - modal closes automatically, so a manual close button click races the automatic close --- .../src/settings/advanced/integrations.acceptance.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/admin/src/settings/advanced/integrations.acceptance.test.tsx b/apps/admin/src/settings/advanced/integrations.acceptance.test.tsx index 85bf96bb74a..9b2e6b88284 100644 --- a/apps/admin/src/settings/advanced/integrations.acceptance.test.tsx +++ b/apps/admin/src/settings/advanced/integrations.acceptance.test.tsx @@ -122,7 +122,8 @@ describe("Advanced integrations", () => { await modal.getByRole("button", {name: "Save"}).click(); await expect.element(section).toHaveTextContent(/Test description/); expect(editApi.requests).toHaveLength(1); - await modal.getByRole("button", {name: "Close"}).click(); + // The modal closes itself once the saved state resets — clicking Close races that. + await expect.element(modal).not.toBeInTheDocument(); await section.getByText("My integration").hover(); await section.getByRole("button", {name: "Delete"}).click(); From 1a7fc830ad9d4e61e23f2e36f89c74f6cfecb0c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Murat=20=C3=87orlu?= <127687+muratcorlu@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:01:25 +0200 Subject: [PATCH 25/25] Fixed incorrect unit in targetDeliveryWindow JSDoc (#29901) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `MailgunClient#getTargetDeliveryWindow`'s doc comment said the configured value is in seconds, but nothing in the value's actual consumption agrees: it's passed straight through unconverted (`mailgun-client.js`) into `BatchSendingService#getDeliveryDeadline`/`#calculateDeliveryTimes`, where it's added directly to `Date#getTime()` results — i.e. treated as milliseconds. The service's own tests confirm this (e.g. `const targetDeliveryWindow = 300000; // 5 minutes` in batch-sending-service.test.js), so the comment was just describing the wrong unit, not the behavior. Left as a comment-only change since fixing the actual unit (rather than the doc) would silently change delivery timing for anyone who already set `bulkEmail.targetDeliveryWindow` assuming seconds. --- .../core/core/server/services/email-service/sending-service.js | 2 +- ghost/core/core/server/services/lib/mailgun-client.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ghost/core/core/server/services/email-service/sending-service.js b/ghost/core/core/server/services/email-service/sending-service.js index d8aa514c5dc..d11958f7fd2 100644 --- a/ghost/core/core/server/services/email-service/sending-service.js +++ b/ghost/core/core/server/services/email-service/sending-service.js @@ -88,7 +88,7 @@ class SendingService { } /** - * Returns the configured target delivery window in seconds + * Returns the configured target delivery window in milliseconds * * @returns {number} */ diff --git a/ghost/core/core/server/services/lib/mailgun-client.js b/ghost/core/core/server/services/lib/mailgun-client.js index 9e136a024fd..caed01255a7 100644 --- a/ghost/core/core/server/services/lib/mailgun-client.js +++ b/ghost/core/core/server/services/lib/mailgun-client.js @@ -397,7 +397,7 @@ module.exports = class MailgunClient { } /** - * Returns the configured target delivery window in seconds + * Returns the configured target delivery window in milliseconds * Ghost will attempt to deliver emails evenly distributed over this window * * Defaults to 0 (no delay) if not set