diff --git a/.changeset/changelogs/@tryghost!admin@0.0.0.md b/.changeset/changelogs/@tryghost!admin@0.0.0.md new file mode 100644 index 00000000000..7d3a904b877 --- /dev/null +++ b/.changeset/changelogs/@tryghost!admin@0.0.0.md @@ -0,0 +1,7 @@ +## 0.0.0 + +### Patch Changes + +- Updated dependencies: + - @tryghost/kg-unsplash-selector@0.4.5 + - @tryghost/koenig-lexical@1.9.4 diff --git a/.changeset/changelogs/@tryghost!kg-default-nodes@2.2.1.md b/.changeset/changelogs/@tryghost!kg-default-nodes@2.2.1.md new file mode 100644 index 00000000000..30d8bda1d9d --- /dev/null +++ b/.changeset/changelogs/@tryghost!kg-default-nodes@2.2.1.md @@ -0,0 +1,5 @@ +## 2.2.1 + +### Patch Changes + +- Updated Ghost docs links to new URLs diff --git a/.changeset/changelogs/@tryghost!kg-unsplash-selector@0.4.5.md b/.changeset/changelogs/@tryghost!kg-unsplash-selector@0.4.5.md new file mode 100644 index 00000000000..185da1ca42b --- /dev/null +++ b/.changeset/changelogs/@tryghost!kg-unsplash-selector@0.4.5.md @@ -0,0 +1,7 @@ +## 0.4.5 + +### Patch Changes + +- Changed Tailwind color classes from the grey spelling to gray, matching the palette used by consuming apps; koenig-lexical aliases gray to the same scale so those utilities are generated in its own bundle + +- Fixed the Unsplash selector not following dark mode diff --git a/.changeset/changelogs/@tryghost!koenig-lexical@1.9.4.md b/.changeset/changelogs/@tryghost!koenig-lexical@1.9.4.md new file mode 100644 index 00000000000..6ecdd22fccc --- /dev/null +++ b/.changeset/changelogs/@tryghost!koenig-lexical@1.9.4.md @@ -0,0 +1,9 @@ +## 1.9.4 + +### Patch Changes + +- Changed Tailwind color classes from the grey spelling to gray, matching the palette used by consuming apps; koenig-lexical aliases gray to the same scale so those utilities are generated in its own bundle + +- Updated Ghost docs links to new URLs + +- Fixed the Unsplash selector not following dark mode diff --git a/.changeset/gray-scale-spelling.md b/.changeset/gray-scale-spelling.md new file mode 100644 index 00000000000..e44643b72c7 --- /dev/null +++ b/.changeset/gray-scale-spelling.md @@ -0,0 +1,6 @@ +--- +"@tryghost/kg-unsplash-selector": patch +"@tryghost/koenig-lexical": patch +--- + +Changed Tailwind color classes from the grey spelling to gray, matching the palette used by consuming apps; koenig-lexical aliases gray to the same scale so those utilities are generated in its own bundle diff --git a/.changeset/ledger.yaml b/.changeset/ledger.yaml index 7fb4f25bb27..31caf54ebd1 100644 --- a/.changeset/ledger.yaml +++ b/.changeset/ledger.yaml @@ -1,124 +1,133 @@ -'@tryghost/adapter-base-cache@0.2.0': +"@tryghost/adapter-base-cache@0.2.0": dir: packages/adapters/cache-base intents: - major-areas-fail - upset-dogs-sort -'@tryghost/adapter-base-scheduling@0.1.1': +"@tryghost/adapter-base-scheduling@0.1.1": dir: packages/adapters/scheduling-base intents: - major-areas-fail - plain-singers-cheat -'@tryghost/adapter-base-scheduling@0.2.0': +"@tryghost/adapter-base-scheduling@0.2.0": dir: packages/adapters/scheduling-base intents: - fifty-maps-sing -'@tryghost/adapter-base-scheduling@0.2.1': +"@tryghost/adapter-base-scheduling@0.2.1": dir: packages/adapters/scheduling-base intents: - typed-logging-args -'@tryghost/adapter-base-scheduling@0.2.2': +"@tryghost/adapter-base-scheduling@0.2.2": dir: packages/adapters/scheduling-base intents: - open-shirts-arrive - young-spiders-request -'@tryghost/adapter-base-scheduling@0.2.3': +"@tryghost/adapter-base-scheduling@0.2.3": dir: packages/adapters/scheduling-base intents: - brown-rules-cut -'@tryghost/adapter-base-sso@0.1.1': +"@tryghost/adapter-base-sso@0.1.1": dir: packages/adapters/sso-base intents: - major-areas-fail - plain-singers-cheat -'@tryghost/adapter-base-sso@0.1.2': +"@tryghost/adapter-base-sso@0.1.2": dir: packages/adapters/sso-base intents: - open-shirts-arrive -'@tryghost/adapter-base-sso@0.1.3': +"@tryghost/adapter-base-sso@0.1.3": dir: packages/adapters/sso-base intents: - brown-rules-cut -'@tryghost/kg-card-factory@5.2.4': +"@tryghost/kg-card-factory@5.2.4": dir: koenig/kg-card-factory intents: - common-squids-argue -'@tryghost/kg-clean-basic-html@4.3.4': +"@tryghost/kg-clean-basic-html@4.3.4": dir: koenig/kg-clean-basic-html intents: - common-squids-argue - six-parents-shout -'@tryghost/kg-converters@1.2.4': +"@tryghost/kg-converters@1.2.4": dir: koenig/kg-converters intents: - tidy-types-check -'@tryghost/kg-converters@1.2.5': +"@tryghost/kg-converters@1.2.5": dir: koenig/kg-converters intents: - common-squids-argue -'@tryghost/kg-default-cards@10.3.4': +"@tryghost/kg-default-cards@10.3.4": dir: koenig/kg-default-cards intents: - spacy-poodles-hunt -'@tryghost/kg-default-cards@10.3.5': +"@tryghost/kg-default-cards@10.3.5": dir: koenig/kg-default-cards intents: - common-squids-argue - six-parents-shout -'@tryghost/kg-default-nodes@2.1.4': +"@tryghost/kg-default-nodes@2.1.4": dir: koenig/kg-default-nodes intents: - floppy-bobcats-spend -'@tryghost/kg-default-nodes@2.1.5': +"@tryghost/kg-default-nodes@2.1.5": dir: koenig/kg-default-nodes intents: - spacy-poodles-hunt - warm-hotels-make -'@tryghost/kg-default-nodes@2.2.0': +"@tryghost/kg-default-nodes@2.2.0": dir: koenig/kg-default-nodes intents: - clean-flags-retire - common-squids-argue - six-parents-shout -'@tryghost/kg-default-transforms@1.3.4': +"@tryghost/kg-default-nodes@2.2.1": + dir: koenig/kg-default-nodes + intents: + - khaki-poets-clap +"@tryghost/kg-default-transforms@1.3.4": dir: koenig/kg-default-transforms intents: - common-squids-argue -'@tryghost/kg-html-to-lexical@1.4.0': +"@tryghost/kg-html-to-lexical@1.4.0": dir: koenig/kg-html-to-lexical intents: - common-squids-argue - six-parents-shout -'@tryghost/kg-lexical-html-renderer@1.5.0': +"@tryghost/kg-lexical-html-renderer@1.5.0": dir: koenig/kg-lexical-html-renderer intents: - common-squids-argue - six-parents-shout -'@tryghost/kg-markdown-html-renderer@7.2.4': +"@tryghost/kg-markdown-html-renderer@7.2.4": dir: koenig/kg-markdown-html-renderer intents: - common-squids-argue - four-rings-relate -'@tryghost/kg-unsplash-selector@0.4.4': +"@tryghost/kg-unsplash-selector@0.4.4": dir: koenig/kg-unsplash-selector intents: - common-squids-argue - petite-schools-accept - six-parents-shout - swift-guests-grin -'@tryghost/kg-utils@1.1.4': +"@tryghost/kg-unsplash-selector@0.4.5": + dir: koenig/kg-unsplash-selector + intents: + - gray-scale-spelling + - tidy-mangos-repeat +"@tryghost/kg-utils@1.1.4": dir: koenig/kg-utils intents: - common-squids-argue - four-rings-relate -'@tryghost/koenig-lexical@1.9.0': +"@tryghost/koenig-lexical@1.9.0": dir: koenig/koenig-lexical intents: - bright-walls-preview -'@tryghost/koenig-lexical@1.9.1': +"@tryghost/koenig-lexical@1.9.1": dir: koenig/koenig-lexical intents: - warm-hotels-make -'@tryghost/koenig-lexical@1.9.2': +"@tryghost/koenig-lexical@1.9.2": dir: koenig/koenig-lexical intents: - funky-bikes-chew @@ -128,10 +137,16 @@ - petite-forks-lick - petite-schools-accept - six-parents-shout -'@tryghost/koenig-lexical@1.9.3': +"@tryghost/koenig-lexical@1.9.3": dir: koenig/koenig-lexical intents: - plenty-moons-smile +"@tryghost/koenig-lexical@1.9.4": + dir: koenig/koenig-lexical + intents: + - gray-scale-spelling + - khaki-poets-clap + - tidy-mangos-repeat ghost-storage-base@3.0.0: dir: packages/adapters/storage-base intents: diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index f72508a753d..853e13ba86a 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -8,7 +8,7 @@ # Tinybird Analytics # Tinybird data pipelines and services require review from designated owners -**/tinybird/ @9larsons @cmraible @evanhahn @troyciesco +**/tinybird/ @cmraible @evanhahn @troyciesco # @tryghost/parse-email-address /packages/parse-email-address/ @EvanHahn diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2941317350..bfc2b3662bf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -253,17 +253,17 @@ jobs: AFFECTED_ARG="" fi - AFFECTED_PROJECTS=$(pnpm -s nx show projects ${AFFECTED_ARG} --json) + AFFECTED_PROJECTS=$(pnpm nx show projects ${AFFECTED_ARG} --json) echo "affected_projects=$AFFECTED_PROJECTS" >> "$GITHUB_OUTPUT" # string list for use in run-many commands - AFFECTED_PROJECTS_STR=$(pnpm -s nx show projects ${AFFECTED_ARG} --sep=, | tr -d '\n') + AFFECTED_PROJECTS_STR=$(pnpm nx show projects ${AFFECTED_ARG} --sep=, | tr -d '\n') echo "affected_projects_str=$AFFECTED_PROJECTS_STR" >> "$GITHUB_OUTPUT" UNIT_TEST_AFFECTED_ARG="$AFFECTED_ARG" if [[ "${{ steps.changed.outputs.unit-test-globals }}" == 'true' ]]; then UNIT_TEST_AFFECTED_ARG="" fi - UNIT_TEST_PROJECTS_STR=$(pnpm -s nx show projects ${UNIT_TEST_AFFECTED_ARG} --withTarget test:unit --sep=, | tr -d '\n') + UNIT_TEST_PROJECTS_STR=$(pnpm nx show projects ${UNIT_TEST_AFFECTED_ARG} --withTarget test:unit --sep=, | tr -d '\n') if [[ "${{ steps.changed.outputs.core-unit-test-globals }}" == 'true' ]]; then UNIT_TEST_PROJECTS_STR=$(printf '%s\n%s\n' "$UNIT_TEST_PROJECTS_STR" "ghost" | awk 'NF && !seen[$0]++' | paste -sd, -) fi @@ -271,13 +271,13 @@ jobs: # "i18n" tag = packages whose source is scanned by @tryghost/i18n's # translate:* scripts (not packages that merely import @tryghost/i18n). - I18N_PROJECTS=$(pnpm -s nx show projects ${AFFECTED_ARG} --projects 'tag:i18n' --sep=, | tr -d '\n') + I18N_PROJECTS=$(pnpm nx show projects ${AFFECTED_ARG} --projects 'tag:i18n' --sep=, | tr -d '\n') echo "affected_i18n_projects=${I18N_PROJECTS}" >> "$GITHUB_OUTPUT" # "playwright" tag = projects whose test:acceptance suite runs in # job_apps_acceptance-tests. Tag-based rather than directory-based so # the matrix isn't coupled to the workspace layout (apps/*, koenig/*, ...). - PLAYWRIGHT_PROJECTS=$(pnpm -s nx show projects ${AFFECTED_ARG} \ + PLAYWRIGHT_PROJECTS=$(pnpm nx show projects ${AFFECTED_ARG} \ --withTarget test:acceptance \ --projects 'tag:playwright' \ --json) @@ -335,8 +335,13 @@ jobs: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 - - name: Fetch main branch - run: git fetch --no-tags origin main + - name: Fetch PR base branch + # Through env, never interpolated into the script: a branch name can + # carry shell metacharacters, and template expansion happens before the + # shell parses the command. + env: + PR_BASE_REF: ${{ github.event.pull_request.base.ref }} + run: git fetch --no-tags origin "+refs/heads/${PR_BASE_REF}:refs/remotes/origin/${PR_BASE_REF}" - uses: ./.github/actions/setup-node-pnpm with: @@ -349,6 +354,7 @@ jobs: - name: Check app version bump env: PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PR_BASE_REF: ${{ github.event.pull_request.base.ref }} PR_COMPARE_SHA: ${{ github.event.pull_request.head.sha }} run: node scripts/check-app-version-bump.js @@ -358,6 +364,7 @@ jobs: - name: Check for missing changesets env: PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PR_BASE_REF: ${{ github.event.pull_request.base.ref }} PR_COMPARE_SHA: ${{ github.event.pull_request.head.sha }} run: node scripts/change-check.js @@ -374,11 +381,17 @@ jobs: fetch-depth: 0 - name: Fetch PR base branch - run: git fetch --no-tags origin "${{ github.event.pull_request.base.ref }}" + # Through env, never interpolated into the script: a branch name can + # carry shell metacharacters, and template expansion happens before the + # shell parses the command. + env: + PR_BASE_REF: ${{ github.event.pull_request.base.ref }} + run: git fetch --no-tags origin "+refs/heads/${PR_BASE_REF}:refs/remotes/origin/${PR_BASE_REF}" - name: Check migration integrity env: PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PR_BASE_REF: ${{ github.event.pull_request.base.ref }} PR_COMPARE_SHA: ${{ github.event.pull_request.head.sha }} run: node scripts/check-migration-integrity.cjs @@ -996,7 +1009,7 @@ jobs: # resolve the project root since apps live in apps/* and koenig/* run: | APP_NAME="${{ matrix.app }}" - APP_ROOT=$(pnpm -s nx show project ${{ matrix.app }} --json | jq -r .root) + APP_ROOT=$(pnpm nx show project ${{ matrix.app }} --json | jq -r .root) { echo "name=${APP_NAME#@tryghost/}" echo "root=$APP_ROOT" diff --git a/.oxfmtrc.json b/.oxfmtrc.json index 1a98c0ce48e..613f7c39ff0 100644 --- a/.oxfmtrc.json +++ b/.oxfmtrc.json @@ -17,6 +17,7 @@ "koenig/kg-lexical-html-renderer/**", "koenig/kg-simplemde/debug/**", "koenig/koenig-lexical/**", - "packages/i18n/locales/**" + "packages/i18n/locales/**", + ".changeset/ledger.yaml" ] } diff --git a/apps/activitypub/src/components/feed/table-of-contents.tsx b/apps/activitypub/src/components/feed/table-of-contents.tsx index d9a8bd23fc4..eb1a1355341 100644 --- a/apps/activitypub/src/components/feed/table-of-contents.tsx +++ b/apps/activitypub/src/components/feed/table-of-contents.tsx @@ -221,7 +221,7 @@ const TableOfContentsView: React.FC = ({ {items.map((item) => ( + ) : undefined; return ( : } + addButtonClassName={cn( + hasFilters && (useConsolidatedFilterUI ? 'gap-0 !px-3 text-[0px]' : 'border-none'), + )} + addButtonIcon={ + useConsolidatedFilterUI ? ( + hasFilters ? ( + + ) : ( + + ) + ) : hasFilters ? ( + + ) : ( + + ) + } addButtonText={hasFilters ? 'Add filter' : 'Filter'} allowMultiple={false} - className={`[&>button]:order-last ${hasFilters ? '[&>button]:border-none' : 'w-auto'}`} + className={cn('[&>button]:order-last', !hasFilters && 'w-auto')} + clearButton={outlinedClearButton} clearButtonClassName="font-normal text-muted-foreground" clearButtonIcon={} clearButtonText="Clear" diff --git a/apps/admin/src/layout/app-sidebar/user-menu.tsx b/apps/admin/src/layout/app-sidebar/user-menu.tsx index 5ca9307ec8c..16366468147 100644 --- a/apps/admin/src/layout/app-sidebar/user-menu.tsx +++ b/apps/admin/src/layout/app-sidebar/user-menu.tsx @@ -150,7 +150,7 @@ function UserMenu(props: UserMenuProps) { diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/components/complete-step.tsx b/apps/admin/src/members/components/bulk-action-modals/import-members/components/complete-step.tsx index d500b95e4c8..1c1cc0bc53e 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/components/complete-step.tsx +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/components/complete-step.tsx @@ -29,7 +29,7 @@ export function CompleteStep({ importResponse, onReset, onClose }: CompleteStepP <> {importResponse.importedCount > 0 && ( <> -
+

{formatNumber(importResponse.errorCount)}{' '} {importResponse.errorCount === 1 ? 'member was' : 'members were'} skipped due to diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/components/init-step.tsx b/apps/admin/src/members/components/bulk-action-modals/import-members/components/init-step.tsx index 009b139e5f4..0a159740ef9 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/components/init-step.tsx +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/components/init-step.tsx @@ -57,8 +57,8 @@ export function InitStep({ fileError, onClose, onDropAccepted, onDropRejected }: > {({ isDragActive, isDragReject }) => ( <> - - + + {isDragReject ? 'The file type you uploaded is not supported' : isDragActive diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/csv.test.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/csv.test.ts index 5c3c39dc8ed..0bcf957d3c5 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/csv.test.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/csv.test.ts @@ -99,14 +99,14 @@ describe('csv helpers', () => { email: 'b@example.com', labels: [], newsletters: [{ name: 'Daily News' }], - 'custom_fields.topic': 'ghosts', + 'metafields.custom.topic': 'ghosts', error: 'nope', }, ]); const header = output.split('\n')[0].trimEnd(); expect(header).toContain('"newsletters"'); - expect(header).toContain('"custom_fields.topic"'); + expect(header).toContain('"metafields.custom.topic"'); expect(header.endsWith('"error"')).toBe(true); }); diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/csv.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/csv.ts index 8ab19802393..3e12373eb1f 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/csv.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/csv.ts @@ -1,3 +1,4 @@ +import { isCustomFieldColumn } from '@tryghost/admin-x-framework/api/member-custom-fields'; import Papa from 'papaparse'; import { z } from 'zod'; @@ -88,7 +89,7 @@ function toExportErrorRow(row: RawErrorRow): Record { } for (const [key, value] of Object.entries(row)) { - if (key.startsWith('custom_fields.')) { + if (isCustomFieldColumn(key)) { shaped[key] = cell(value); } } diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.test.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.test.ts index 22b307c8fcf..61e11d47dd3 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.test.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/field-targets.test.ts @@ -10,7 +10,7 @@ const membershipFields = [ const customColumn = ( overrides: Partial = {}, ): MemberCustomFieldCsvColumn => ({ - value: 'custom_fields.nickname', + value: 'metafields.custom.nickname', fieldName: 'Nickname', label: 'Nickname', type: 'short_text', @@ -45,7 +45,7 @@ describe('field targets', () => { membershipFields: [], customFieldColumns: [ customColumn({ - value: 'custom_fields.shipping_address.city', + value: 'metafields.custom.shipping_address.city', fieldName: 'Shipping Address', partLabel: 'City', label: 'Shipping Address (City)', @@ -56,7 +56,7 @@ describe('field targets', () => { expect(allTargets(groups)).toEqual([ { - value: 'custom_fields.shipping_address.city', + value: 'metafields.custom.shipping_address.city', source: 'custom', fieldName: 'Shipping Address', partLabel: 'City', @@ -139,7 +139,7 @@ describe('field targets', () => { membershipFields, customFieldColumns: [ customColumn({ - value: 'custom_fields.name.city', + value: 'metafields.custom.name.city', fieldName: 'Name', partLabel: 'City', label: 'Name (City)', @@ -153,7 +153,7 @@ describe('field targets', () => { it('marks only against the targets in the list it is given', () => { const custom = [ - customColumn({ value: 'custom_fields.tier', fieldName: 'Tier', label: 'Tier' }), + customColumn({ value: 'metafields.custom.tier', fieldName: 'Tier', label: 'Tier' }), ]; expect(badged(fieldTargets({ membershipFields, customFieldColumns: custom }))).toEqual([]); diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/import-members-modal.tsx b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/import-members-modal.tsx index 708b94c726f..c76086f5d16 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/import-members-modal.tsx +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/import-members-modal.tsx @@ -81,17 +81,9 @@ export function ImportMembersModal({ // mutation puts it into the cached list, so there is no window where a row points at a // column the picker cannot name yet. const customFieldColumns = useMemo( - () => memberCustomFieldCsvColumns(customFieldsData?.members_custom_fields ?? []), + () => memberCustomFieldCsvColumns(customFieldsData ?? []), [customFieldsData], ); - // The file-reader effect waits for this before its first parse: the custom field - // definitions must be loaded or auto-detection would miss custom_fields.* columns on a - // fast upload. It flips false -> true once and stays true (a refetch keeps data defined), - // so readiness never re-triggers the read. - // Ready, or never going to be. A failed query has no representation in `data`, so waiting - // on `data` alone leaves the file unparsed and the step on a spinner with nothing said — - // for a query whose only job is to add targets to a list. - // Failing it costs the custom fields; blocking on it costs the import. const customFieldsReady = customFieldsData !== undefined || customFieldsFailed; // Detection options are read inside the effect through this ref rather than as deps, so // a later refetch of the options can't re-run the read and overwrite a mapping the user diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping-step.tsx b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping-step.tsx index 68239b4b93c..917da8fc984 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping-step.tsx +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping-step.tsx @@ -222,7 +222,7 @@ export function MappingStep({ let field; try { const response = await createField({ name, type }); - field = response.members_custom_fields?.[0]; + field = response.members_metafields?.[0]; } catch (error) { reportCreateFailure(error); return; @@ -351,15 +351,10 @@ export function MappingStep({ })) : []; - // What this import writes: one entry per column in the file — the field it fills, empty for - // a column left out, or null for a column in the import with no field chosen yet. - // Everything asking what is being imported reads this one value, so the checks below and - // the request itself cannot disagree. - // - // Empty rather than omitted, because the importer carries a column the mapping does not - // name through under its own header — which is how an unnamed custom_fields.* column - // survives to be read. Leaving a column out of the mapping is the opposite of leaving it - // out of the import. + // Every column gets an entry, including the ones this import skips. The importer treats a + // column the mapping does not mention as "pass it through under its own header" — which + // for a custom-field column means importing it. Skipping a column therefore means naming + // it with an empty target, never leaving it out. const importMapping: Record = Object.fromEntries( currentlyDisplayedData.map((row) => [row.key, isImported(row) ? row.mapTo : '']), ); diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.test.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.test.ts index a5a9d0f6660..354eb4e18ae 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.test.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.test.ts @@ -20,44 +20,44 @@ describe('custom fields mapping helpers', () => { { label: 'Nickname', fieldName: 'Nickname', - value: 'custom_fields.nickname', + value: 'metafields.custom.nickname', type: 'short_text', }, { label: 'Shipping Address (Line 1)', fieldName: 'Shipping Address', partLabel: 'Line 1', - value: 'custom_fields.shipping_address.line1', + value: 'metafields.custom.shipping_address.line1', type: 'address', }, { label: 'Shipping Address (First name)', fieldName: 'Shipping Address', partLabel: 'First name', - value: 'custom_fields.shipping_address.first_name', + value: 'metafields.custom.shipping_address.first_name', type: 'address', }, ]; it('auto-detects a custom field column by its namespaced header', () => { const mapping = detectFieldTypes( - [{ email: 'user@example.com', 'custom_fields.nickname': 'Bex' }], + [{ email: 'user@example.com', 'metafields.custom.nickname': 'Bex' }], { customFieldColumns }, ); - expect(mapping['custom_fields.nickname']).toBe('custom_fields.nickname'); + expect(mapping['metafields.custom.nickname']).toBe('metafields.custom.nickname'); }); // The /name/i heuristic must not claim a custom field column containing "name". it('does not map a name-like custom field column to the member name', () => { const mapping = detectFieldTypes( - [{ email: 'user@example.com', 'custom_fields.shipping_address.first_name': 'Bex' }], + [{ email: 'user@example.com', 'metafields.custom.shipping_address.first_name': 'Bex' }], { customFieldColumns }, ); expect(mapping.name).toBeUndefined(); - expect(mapping['custom_fields.shipping_address.first_name']).toBe( - 'custom_fields.shipping_address.first_name', + expect(mapping['metafields.custom.shipping_address.first_name']).toBe( + 'metafields.custom.shipping_address.first_name', ); }); @@ -65,7 +65,7 @@ describe('custom fields mapping helpers', () => { // or a hand-built file) must not be claimed as the member name. it('does not map a name-like namespaced column that has no offered target', () => { const mapping = detectFieldTypes([ - { email: 'user@example.com', 'custom_fields.former_field.last_name': 'Bex' }, + { email: 'user@example.com', 'metafields.custom.former_field.last_name': 'Bex' }, ]); expect(mapping.name).toBeUndefined(); @@ -75,13 +75,13 @@ describe('custom fields mapping helpers', () => { // dropped) just because its values look like email addresses. it('does not bind an email-valued custom field column to the member email', () => { const mapping = detectFieldTypes( - [{ 'custom_fields.contact_email': 'contact@example.com', email: 'member@example.com' }], + [{ 'metafields.custom.contact_email': 'contact@example.com', email: 'member@example.com' }], { customFieldColumns: [ { label: 'Contact email', fieldName: 'Contact email', - value: 'custom_fields.contact_email', + value: 'metafields.custom.contact_email', type: 'short_text', }, ], @@ -89,19 +89,19 @@ describe('custom fields mapping helpers', () => { ); expect(mapping.email).toBe('email'); - expect(mapping['custom_fields.contact_email']).toBe('custom_fields.contact_email'); + expect(mapping['metafields.custom.contact_email']).toBe('metafields.custom.contact_email'); }); describe('suggestedFieldName', () => { it('strips the namespace and un-slugs a column from a Ghost export', () => { - expect(suggestedFieldName('custom_fields.nickname')).toBe('Nickname'); - expect(suggestedFieldName('custom_fields.favorite-animal')).toBe('Favorite animal'); + expect(suggestedFieldName('metafields.custom.nickname')).toBe('Nickname'); + expect(suggestedFieldName('metafields.custom.favorite-animal')).toBe('Favorite animal'); }); // The part is a separate question the form asks, so it must not end up in the name. it('names the field, not the part, for a composite column', () => { - expect(suggestedFieldName('custom_fields.home-address.city')).toBe('Home address'); - expect(suggestedFieldName('custom_fields.home-address.postal_code')).toBe('Home address'); + expect(suggestedFieldName('metafields.custom.home-address.city')).toBe('Home address'); + expect(suggestedFieldName('metafields.custom.home-address.postal_code')).toBe('Home address'); }); it('keeps a column from anywhere else as the publisher wrote it', () => { diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.ts index c75015546cf..f57d9277053 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/custom-fields/mapping.ts @@ -30,11 +30,10 @@ export { * touched. It is a starting point either way, and the form lets them edit it. */ export function suggestedFieldName(column: string): string { - // A custom field column is `custom_fields.` or `custom_fields..`. The name - // being suggested is the field's, so the part is dropped: `custom_fields.home-address.city` - // names a field called "Home address" whose City part this column holds, and the part is - // asked for separately. A bare `custom_fields` column has no key, so it suggests the namespace itself. - const segments = isCustomFieldColumn(column) ? column.split('.').slice(1, 2) : [column]; - const words = (segments[0] ?? column).replace(/[._-]+/g, ' ').trim(); + // A custom field column is `metafields..[.]`. The name being + // suggested is the field's, so only the key segment is kept: the part is asked for + // separately. + const key = isCustomFieldColumn(column) ? column.split('.')[2] : column; + const words = (key ?? column).replace(/[._-]+/g, ' ').trim(); return words.charAt(0).toUpperCase() + words.slice(1); } diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.test.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.test.ts index 5908dd4c3e3..e3b830e201b 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.test.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/mapping.test.ts @@ -129,44 +129,44 @@ describe('mapping helpers', () => { { label: 'Nickname', fieldName: 'Nickname', - value: 'custom_fields.nickname', + value: 'metafields.custom.nickname', type: 'short_text', }, { label: 'Shipping Address (Line 1)', fieldName: 'Shipping Address', partLabel: 'Line 1', - value: 'custom_fields.shipping_address.line1', + value: 'metafields.custom.shipping_address.line1', type: 'address', }, { label: 'Shipping Address (First name)', fieldName: 'Shipping Address', partLabel: 'First name', - value: 'custom_fields.shipping_address.first_name', + value: 'metafields.custom.shipping_address.first_name', type: 'address', }, ]; it('auto-detects a custom field column by its namespaced header', () => { const mapping = detectFieldTypes( - [{ email: 'user@example.com', 'custom_fields.nickname': 'Bex' }], + [{ email: 'user@example.com', 'metafields.custom.nickname': 'Bex' }], { customFieldColumns }, ); - expect(mapping['custom_fields.nickname']).toBe('custom_fields.nickname'); + expect(mapping['metafields.custom.nickname']).toBe('metafields.custom.nickname'); }); // The /name/i heuristic must not claim a custom field column containing "name". it('does not map a name-like custom field column to the member name', () => { const mapping = detectFieldTypes( - [{ email: 'user@example.com', 'custom_fields.shipping_address.first_name': 'Bex' }], + [{ email: 'user@example.com', 'metafields.custom.shipping_address.first_name': 'Bex' }], { customFieldColumns }, ); expect(mapping.name).toBeUndefined(); - expect(mapping['custom_fields.shipping_address.first_name']).toBe( - 'custom_fields.shipping_address.first_name', + expect(mapping['metafields.custom.shipping_address.first_name']).toBe( + 'metafields.custom.shipping_address.first_name', ); }); @@ -174,7 +174,7 @@ describe('mapping helpers', () => { // or a hand-built file) must not be claimed as the member name. it('does not map a name-like namespaced column that has no offered target', () => { const mapping = detectFieldTypes([ - { email: 'user@example.com', 'custom_fields.former_field.last_name': 'Bex' }, + { email: 'user@example.com', 'metafields.custom.former_field.last_name': 'Bex' }, ]); expect(mapping.name).toBeUndefined(); @@ -184,13 +184,13 @@ describe('mapping helpers', () => { // dropped) just because its values look like email addresses. it('does not bind an email-valued custom field column to the member email', () => { const mapping = detectFieldTypes( - [{ 'custom_fields.contact_email': 'contact@example.com', email: 'member@example.com' }], + [{ 'metafields.custom.contact_email': 'contact@example.com', email: 'member@example.com' }], { customFieldColumns: [ { label: 'Contact email', fieldName: 'Contact email', - value: 'custom_fields.contact_email', + value: 'metafields.custom.contact_email', type: 'short_text', }, ], @@ -198,6 +198,6 @@ describe('mapping helpers', () => { ); expect(mapping.email).toBe('email'); - expect(mapping['custom_fields.contact_email']).toBe('custom_fields.contact_email'); + expect(mapping['metafields.custom.contact_email']).toBe('metafields.custom.contact_email'); }); }); diff --git a/apps/admin/src/members/components/bulk-action-modals/import-members/upload.test.ts b/apps/admin/src/members/components/bulk-action-modals/import-members/upload.test.ts index f44e459fdfc..040c057ea2e 100644 --- a/apps/admin/src/members/components/bulk-action-modals/import-members/upload.test.ts +++ b/apps/admin/src/members/components/bulk-action-modals/import-members/upload.test.ts @@ -151,7 +151,7 @@ describe('buildImportResponse', () => { // A reason may quote a cell the publisher wrote, and a CSV cell legally holds // both a comma and a newline. const punctuated = - 'custom_fields.home_address.country: Enter a 2-letter country code, like US.'; + 'metafields.custom.home_address.country: Enter a 2-letter country code, like US.'; const multiline = '"Gold\nPlan" is not a valid tier.'; const result = buildImportResponse({ meta: { @@ -270,13 +270,13 @@ describe('buildImportResponse', () => { invalid: [ { email: 'a@test.com', - 'custom_fields.home_address.country': 'IRL', + 'metafields.custom.home_address.country': 'IRL', errors: [ 'Missing email address', - 'custom_fields.home_address.country: Enter a 2-letter country code, like US.', + 'metafields.custom.home_address.country: Enter a 2-letter country code, like US.', ], error: - 'Missing email address\ncustom_fields.home_address.country: Enter a 2-letter country code, like US.', + 'Missing email address\nmetafields.custom.home_address.country: Enter a 2-letter country code, like US.', }, ], }, @@ -289,11 +289,11 @@ describe('buildImportResponse', () => { const [header] = csv.split('\r\n'); expect(header).toBe( - '"email","name","note","stripe_customer_id","created_at","labels","custom_fields.home_address.country","gift_id","error"', + '"email","name","note","stripe_customer_id","created_at","labels","metafields.custom.home_address.country","gift_id","error"', ); expect(csv).toContain('"IRL"'); expect(csv).toContain( - '"Missing email address\ncustom_fields.home_address.country: Enter a 2-letter country code, like US."', + '"Missing email address\nmetafields.custom.home_address.country: Enter a 2-letter country code, like US."', ); }); }); diff --git a/apps/admin/src/members/components/members-filters.tsx b/apps/admin/src/members/components/members-filters.tsx index 4be383bf7e7..437a1362d8b 100644 --- a/apps/admin/src/members/components/members-filters.tsx +++ b/apps/admin/src/members/components/members-filters.tsx @@ -1,9 +1,10 @@ -import { CUSTOM_FIELDS_PREFIX } from '@/members/member-fields'; +import { METAFIELDS_FIELD_PREFIX } from '@/members/member-fields'; import { keyBelow } from '@/shared/filters'; import ManageViewPopover from './manage-view-popover'; import React, { useCallback, useMemo } from 'react'; import { Button } from '@tryghost/shade/components'; import { type Filter, Filters } from '@tryghost/shade/patterns'; +import { Inline } from '@tryghost/shade/primitives'; import { LucideIcon, cn } from '@tryghost/shade/utils'; import { buildOfferOptions, @@ -31,6 +32,7 @@ import { useTierValueSource, } from '@/shared/filter-sources'; import type { MemberView } from '@/members/hooks/use-member-views'; +import { useFeatureFlag } from '@tryghost/admin-x-framework/hooks'; interface MembersFiltersProps { filters: Filter[]; @@ -142,22 +144,26 @@ const MembersFilters: React.FC = ({ // fields ride along so a saved segment on a since-archived field still renders its // read-only pill. const { data: customFieldsData } = useCustomFieldDefinitionsIncludingArchived(); - const catalogCustomFields = customFieldsData?.members_custom_fields ?? EMPTY_CUSTOM_FIELDS; + const catalogCustomFields = customFieldsData ?? EMPTY_CUSTOM_FIELDS; // The picker offers active fields only. const customFields = useMemo( () => catalogCustomFields.filter((field) => field.status === 'active'), [catalogCustomFields], ); - const referencedCustomFieldNames = useReferencedKeys(filters, CUSTOM_FIELDS_PREFIX); + const referencedCustomFieldIdentities = useReferencedKeys(filters, METAFIELDS_FIELD_PREFIX); const referencedCustomFieldKeys = useMemo( - () => new Set(referencedCustomFieldNames), - [referencedCustomFieldNames], + () => new Set(referencedCustomFieldIdentities), + [referencedCustomFieldIdentities], ); const archivedCustomFields = useMemo( () => catalogCustomFields - .filter((field) => field.status === 'archived' && referencedCustomFieldKeys.has(field.key)) - .map((field) => ({ key: field.key, name: field.name })), + .filter( + (field) => + field.status === 'archived' && + referencedCustomFieldKeys.has(`${field.namespace}.${field.key}`), + ) + .map((field) => ({ namespace: field.namespace, key: field.key, name: field.name })), [catalogCustomFields, referencedCustomFieldKeys], ); @@ -182,22 +188,34 @@ const MembersFilters: React.FC = ({ }); const hasFilters = filters.length > 0; + const useConsolidatedFilterUI = useFeatureFlag('postsListReact'); const showIconOnlyTrigger = iconOnly && !hasFilters; const addFilterButtonClassName = cn( 'bg-white dark:bg-background', showIconOnlyTrigger && 'min-w-[34px] gap-0 !px-3 text-[0px] lg:min-w-0 lg:gap-1.5 lg:px-3 lg:text-base', + hasFilters && (useConsolidatedFilterUI ? 'gap-0 !px-3 text-[0px]' : 'border-none'), ); const clearAndSaveButtons = hasFilters ? ( -

+ {nql && ( @@ -208,16 +226,28 @@ const MembersFilters: React.FC = ({ onDeleted={() => onFiltersChange([])} /> )} -
+ ) : undefined; return ( : } + addButtonIcon={ + useConsolidatedFilterUI ? ( + hasFilters ? ( + + ) : ( + + ) + ) : hasFilters ? ( + + ) : ( + + ) + } addButtonText={hasFilters ? 'Add filter' : 'Filter'} allowMultiple={true} - className={`[&>button]:order-last ${hasFilters ? 'sm:!pr-40 [&>button]:border-none' : 'w-auto'}`} + className={cn('[&>button]:order-last', hasFilters ? 'sm:!pr-40' : 'w-auto')} clearButton={clearAndSaveButtons} fields={filterFields} filters={displayFilters} diff --git a/apps/admin/src/members/custom-fields/addressing.test.ts b/apps/admin/src/members/custom-fields/addressing.test.ts new file mode 100644 index 00000000000..e3fd280078f --- /dev/null +++ b/apps/admin/src/members/custom-fields/addressing.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest'; +import { customFieldAddressing } from './addressing'; +import { parseFilterToAst } from '@/shared/filters'; + +function ast(filter: string) { + const node = parseFilterToAst(filter); + + if (!node) { + throw new Error(`could not parse: ${filter}`); + } + + return node; +} + +describe('customFieldAddressing bound to a key', () => { + const bound = customFieldAddressing({ namespace: 'custom', key: 'shipping' }); + + it('claims a compound naming its own key', () => { + expect( + bound.matchCompound?.(ast("(metafields.key:'custom.shipping'+metafields.value:~'x')")), + ).not.toBeNull(); + }); + + it('refuses a compound naming another field', () => { + expect( + bound.matchCompound?.(ast("(metafields.key:'custom.billing'+metafields.value:~'x')")), + ).toBeNull(); + }); + + it('refuses a lone key clause naming another field', () => { + expect(bound.matchCompound?.(ast("metafields.key:'custom.billing'"))).toBeNull(); + }); + + it('leaves other fields readable by the unbound template', () => { + const template = customFieldAddressing(); + expect( + template.matchCompound?.(ast("(metafields.key:'custom.billing'+metafields.value:~'x')")), + ).not.toBeNull(); + }); +}); diff --git a/apps/admin/src/members/custom-fields/addressing.ts b/apps/admin/src/members/custom-fields/addressing.ts index adcf2145d86..8d148b3dd77 100644 --- a/apps/admin/src/members/custom-fields/addressing.ts +++ b/apps/admin/src/members/custom-fields/addressing.ts @@ -1,6 +1,6 @@ import { escapeNqlString } from '@tryghost/nql-string'; +import { formatIdentity, parseIdentity } from '@tryghost/custom-field-types/identity'; import { - keyBelow, PRESENCE_OPERATORS, getCompoundChildren, readNegatedString, @@ -8,17 +8,52 @@ import { } from '@/shared/filters'; import type { CompoundMatch, PresenceAddressing } from '@/shared/filters'; -const RELATION = 'custom_fields'; +const RELATION = 'metafields'; const KEY_ATTRIBUTE = `${RELATION}.key`; const VALUE_ATTRIBUTE = `${RELATION}.value`; -const PATH_ATTRIBUTE = `${RELATION}.path`; -export const CUSTOM_FIELD_KEY_PREFIX = 'custom_fields.'; +export const METAFIELDS_FIELD_PREFIX = 'metafields.'; + +export interface MetafieldIdentity { + namespace: string; + key: string; +} + +export function metafieldFieldId( + field: MetafieldIdentity, +): `${typeof METAFIELDS_FIELD_PREFIX}${string}` { + return `${METAFIELDS_FIELD_PREFIX}${field.namespace}.${field.key}`; +} + +export function parseMetafieldFieldId(id: string): MetafieldIdentity | null { + if (!id.startsWith(METAFIELDS_FIELD_PREFIX)) { + return null; + } + const [namespace, key, ...rest] = id.slice(METAFIELDS_FIELD_PREFIX.length).split('.'); + if (!namespace || !key || rest.length > 0) { + return null; + } + return { namespace, key }; +} + +function identityOf(field: MetafieldIdentity, subfield: string): string { + return formatIdentity({ namespace: field.namespace, key: field.key, partPath: subfield || null }); +} + +function parseIdentityValue( + raw: unknown, +): { namespace: string; key: string; subfield: string } | null { + const identity = typeof raw === 'string' ? parseIdentity(raw) : null; + if (!identity) { + return null; + } + return { namespace: identity.namespace, key: identity.key, subfield: identity.partPath ?? '' }; +} export const CUSTOM_FIELD_SET_OPERATORS = PRESENCE_OPERATORS; -function keyClause(fieldKey: string): string { - return `${KEY_ATTRIBUTE}:${escapeNqlString(fieldKey)}`; +function keyClause(identity: string): string { + return `${KEY_ATTRIBUTE}:${escapeNqlString(identity)}`; } function readValues(values: unknown[]): { subfield: string; value: unknown } { @@ -30,50 +65,59 @@ function readValues(values: unknown[]): { subfield: string; value: unknown } { }; } -export function customFieldAddressing(boundKey?: string): PresenceAddressing { +function fieldFromContext( + bound: MetafieldIdentity | undefined, + params: Record, +): MetafieldIdentity | null { + if (bound) { + return bound; + } + if (params.namespace && params.key) { + return { namespace: params.namespace, key: params.key }; + } + return null; +} + +export function customFieldAddressing(bound?: MetafieldIdentity): PresenceAddressing { return { presenceOperators: CUSTOM_FIELD_SET_OPERATORS, address(predicate, ctx) { - const fieldKey = boundKey ?? ctx.params.key; + const field = fieldFromContext(bound, ctx.params); const { subfield, value } = readValues(predicate.values); - if (!fieldKey) { + if (!field) { return null; } return { - valueKey: subfield ? `${VALUE_ATTRIBUTE}.${subfield}` : VALUE_ATTRIBUTE, - companions: [keyClause(fieldKey)], + valueKey: VALUE_ATTRIBUTE, + companions: [keyClause(identityOf(field, subfield))], values: [value], }; }, - // The shape of these clauses is not ours to choose. The members API rewrites them into a - // single lookup over the rows holding custom field values, and it only accepts two forms: - // a lone key clause, or a key clause grouped with one path or value clause. See - // ghost/core/core/server/services/members-custom-fields/filter.ts. - // - // A minus sign inside the group also negates the whole lookup, so `key:'x'+path:-'country'` - // asks for members with no x/country value at all, not for one whose part is something - // else. Anything else is either a 400 or a quietly wrong set of members. + // The shape of these clauses is not ours to choose. Ghost's members endpoint rewrites them + // into a single lookup over the table holding custom-field values, and accepts only two + // forms: a key clause alone, or a key clause grouped with exactly one value clause. The + // key names the exact leaf — namespace, key, and for a multi-part value the part — so a + // part's presence is the bare key clause for that leaf, and its negation asks for members + // with no such leaf stored. addressPresence(predicate, ctx) { - const fieldKey = boundKey ?? ctx.params.key; + const field = fieldFromContext(bound, ctx.params); const { subfield } = readValues(predicate.values); - if (!fieldKey) { + if (!field) { return null; } + const identity = identityOf(field, subfield); + if (predicate.operator === 'is-set') { - return subfield - ? [`(${keyClause(fieldKey)}+${PATH_ATTRIBUTE}:${escapeNqlString(subfield)})`] - : [keyClause(fieldKey)]; + return [keyClause(identity)]; } - return subfield - ? [`(${keyClause(fieldKey)}+${PATH_ATTRIBUTE}:-${escapeNqlString(subfield)})`] - : [`${KEY_ATTRIBUTE}:-${escapeNqlString(fieldKey)}`]; + return [`${KEY_ATTRIBUTE}:-${escapeNqlString(identity)}`]; }, match() { @@ -81,31 +125,43 @@ export function customFieldAddressing(boundKey?: string): PresenceAddressing { }, matchCompound(node): CompoundMatch | null { + const ownsIdentity = (candidate: { namespace: string; key: string }) => + bound === undefined || + (candidate.namespace === bound.namespace && candidate.key === bound.key); + const children = getCompoundChildren(node, '$and'); if (!children) { - const keyValue = node[KEY_ATTRIBUTE]; + const identity = parseIdentityValue(node[KEY_ATTRIBUTE]); + + if (identity) { + if (!ownsIdentity(identity)) { + return null; + } - if (typeof keyValue === 'string') { return { kind: 'predicate', predicate: { - field: `${CUSTOM_FIELD_KEY_PREFIX}${keyValue}`, + field: metafieldFieldId(identity), operator: 'is-set', - values: ['', ''], + values: [identity.subfield, ''], }, }; } - const negatedKey = readNegatedString(keyValue); + const negated = parseIdentityValue(readNegatedString(node[KEY_ATTRIBUTE])); + + if (negated) { + if (!ownsIdentity(negated)) { + return null; + } - if (negatedKey !== null) { return { kind: 'predicate', predicate: { - field: `${CUSTOM_FIELD_KEY_PREFIX}${negatedKey}`, + field: metafieldFieldId(negated), operator: 'is-not-set', - values: ['', ''], + values: [negated.subfield, ''], }, }; } @@ -117,56 +173,26 @@ export function customFieldAddressing(boundKey?: string): PresenceAddressing { return null; } - let fieldKey: string | undefined; - let valueEntry: { subfield: string; raw: unknown } | undefined; - let pathEntry: { subfield: string; negated: boolean } | undefined; + let identity: { namespace: string; key: string; subfield: string } | undefined; + let valueRaw: unknown; + let hasValue = false; for (const child of children) { - if (typeof child[KEY_ATTRIBUTE] === 'string') { - fieldKey = child[KEY_ATTRIBUTE]; + const parsed = parseIdentityValue(child[KEY_ATTRIBUTE]); + if (parsed) { + identity = parsed; } - - for (const childKey of Object.keys(child)) { - if (childKey === VALUE_ATTRIBUTE) { - valueEntry = { subfield: '', raw: child[childKey] }; - } else if (keyBelow(childKey, VALUE_ATTRIBUTE)) { - valueEntry = { - subfield: keyBelow(childKey, VALUE_ATTRIBUTE) ?? '', - raw: child[childKey], - }; - } else if (childKey === PATH_ATTRIBUTE) { - const raw = child[childKey]; - const negatedPath = readNegatedString(raw); - - if (typeof raw === 'string') { - pathEntry = { subfield: raw, negated: false }; - } else if (negatedPath !== null) { - pathEntry = { subfield: negatedPath, negated: true }; - } - } + if (VALUE_ATTRIBUTE in child) { + valueRaw = child[VALUE_ATTRIBUTE]; + hasValue = true; } } - if (!fieldKey) { - return null; - } - - if (pathEntry) { - return { - kind: 'predicate', - predicate: { - field: `${CUSTOM_FIELD_KEY_PREFIX}${fieldKey}`, - operator: pathEntry.negated ? 'is-not-set' : 'is-set', - values: [pathEntry.subfield, ''], - }, - }; - } - - if (!valueEntry) { + if (!identity || !hasValue || !ownsIdentity(identity)) { return null; } - const comparator = toComparator(valueEntry.raw); + const comparator = toComparator(valueRaw); if (!comparator) { return null; @@ -174,8 +200,8 @@ export function customFieldAddressing(boundKey?: string): PresenceAddressing { return { kind: 'value', - field: `${CUSTOM_FIELD_KEY_PREFIX}${fieldKey}`, - leadingValues: [valueEntry.subfield], + field: metafieldFieldId(identity), + leadingValues: [identity.subfield], comparator, }; }, diff --git a/apps/admin/src/members/custom-fields/filter-fields.test.ts b/apps/admin/src/members/custom-fields/filter-fields.test.ts new file mode 100644 index 00000000000..e2b17c280b1 --- /dev/null +++ b/apps/admin/src/members/custom-fields/filter-fields.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest'; +import { PART_FILTER_TYPE, SCALAR_KIND_FILTER_TYPE, customFieldDescriptor } from './filter-fields'; +import { MEMBER_CUSTOM_FIELD_KINDS } from '@tryghost/admin-x-framework/api/member-custom-fields'; +import type { + MemberCustomFieldKind, + MemberCustomFieldPartType, +} from '@tryghost/admin-x-framework/api/member-custom-fields'; +import type { FilterTypeId } from '@/shared/filters'; + +type ScalarKind = Exclude; + +// Compile-time assertions: this file is type-checked by `tsc -b`, so each expected +// error going away fails the build. +// @ts-expect-error -- must not compile, or SCALAR_KIND_FILTER_TYPE is no longer exhaustive +const _mappingWithAMissingKindDoesNotCompile: { [K in ScalarKind]: FilterTypeId } = { + text: 'text', + date: 'plain_date', +}; +const _mappingWithAnUnknownKindDoesNotCompile: { [K in ScalarKind]: FilterTypeId } = { + ...SCALAR_KIND_FILTER_TYPE, + // @ts-expect-error -- must not compile, or SCALAR_KIND_FILTER_TYPE accepts undeclared kinds + boolean: 'scalar', +}; +void _mappingWithAMissingKindDoesNotCompile; +void _mappingWithAnUnknownKindDoesNotCompile; + +describe('SCALAR_KIND_FILTER_TYPE', () => { + it('maps every scalar kind the shared catalog declares', () => { + const scalarKinds = MEMBER_CUSTOM_FIELD_KINDS.filter((kind) => kind !== 'record'); + expect(Object.keys(SCALAR_KIND_FILTER_TYPE).sort()).toEqual([...scalarKinds].sort()); + }); +}); + +describe('a composite field descriptor', () => { + it('filters parts as text and starts the whole field at presence', () => { + const descriptor = customFieldDescriptor({ + namespace: 'custom', + key: 'shipping', + name: 'Shipping', + type: 'address', + }); + + expect(descriptor.type).toBe('text'); + expect(descriptor.ui.defaultOperator).toBe('is-set'); + }); +}); + +// @ts-expect-error -- must not compile, or PART_FILTER_TYPE is no longer exhaustive +const _partMappingWithAMissingTypeDoesNotCompile: { + [P in MemberCustomFieldPartType]: FilterTypeId; +} = { + short_text: 'text', + postal_code: 'text', +}; +void _partMappingWithAMissingTypeDoesNotCompile; + +describe('PART_FILTER_TYPE', () => { + it('filters every part type the same way, because a composite is read with one semantics', () => { + expect(new Set(Object.values(PART_FILTER_TYPE)).size).toBe(1); + }); +}); diff --git a/apps/admin/src/members/custom-fields/filter-fields.ts b/apps/admin/src/members/custom-fields/filter-fields.ts index 5e554afca14..9b2b2a51126 100644 --- a/apps/admin/src/members/custom-fields/filter-fields.ts +++ b/apps/admin/src/members/custom-fields/filter-fields.ts @@ -1,46 +1,66 @@ -import { CUSTOM_FIELD_SET_OPERATORS, customFieldAddressing } from './addressing'; -import { filterType } from '@/shared/filters'; -import { memberCustomFieldKind } from '@tryghost/admin-x-framework/api/member-custom-fields'; +import { METAFIELDS_FIELD_PREFIX, customFieldAddressing, metafieldFieldId } from './addressing'; +import { + memberCustomFieldKind, + memberCustomFieldParts, +} from '@tryghost/admin-x-framework/api/member-custom-fields'; import type { FieldDescriptor, FieldProvider, FilterTypeId } from '@/shared/filters'; import type { MemberCustomField, MemberCustomFieldKind, + MemberCustomFieldPartType, } from '@tryghost/admin-x-framework/api/member-custom-fields'; -const FILTER_TYPE_FOR_KIND: Record = { +export const SCALAR_KIND_FILTER_TYPE: { + [K in Exclude]: FilterTypeId; +} = { text: 'text', date: 'plain_date', number: 'number', - record: 'text', }; -export const CUSTOM_FIELD_CLAUSE = 'custom_fields.'; +export const PART_FILTER_TYPE: { [P in MemberCustomFieldPartType]: FilterTypeId } = { + short_text: 'text', + postal_code: 'text', + country_code: 'text', +}; + +function compositeFilterType(type: MemberCustomField['type']): FilterTypeId { + const partFilterTypes = [ + ...new Set((memberCustomFieldParts(type) ?? []).map((p) => PART_FILTER_TYPE[p.type])), + ]; + + if (partFilterTypes.length > 1) { + throw new Error( + `The parts of '${type}' filter as different types (${partFilterTypes.join(', ')}), ` + + 'but the filter engine reads a composite with a single semantics. Build per-part ' + + 'dispatch into the codec before mapping a part type away from its siblings.', + ); + } + + return partFilterTypes[0] ?? 'text'; +} + +export const CUSTOM_FIELD_CLAUSE = METAFIELDS_FIELD_PREFIX; export interface CustomFieldDefinition { + namespace: string; key: string; name: string; type: MemberCustomField['type']; } -function filterTypeFor(type: MemberCustomField['type']): FilterTypeId { - return FILTER_TYPE_FOR_KIND[memberCustomFieldKind(type)]; -} - export function customFieldDescriptor(definition: CustomFieldDefinition): FieldDescriptor { - const type = filterTypeFor(definition.type); - const isRecord = memberCustomFieldKind(definition.type) === 'record'; + const kind = memberCustomFieldKind(definition.type); return { - key: `custom_fields.${definition.key}`, + key: metafieldFieldId(definition), icon: 'text', - type, - addressing: customFieldAddressing(definition.key), + type: kind === 'record' ? compositeFilterType(definition.type) : SCALAR_KIND_FILTER_TYPE[kind], + addressing: customFieldAddressing(definition), ui: { label: definition.name, type: 'custom', - defaultOperator: isRecord - ? CUSTOM_FIELD_SET_OPERATORS[0] - : (filterType(type).defaultOperator ?? CUSTOM_FIELD_SET_OPERATORS[0]), + ...(kind === 'record' ? { defaultOperator: 'is-set' } : {}), }, } as FieldDescriptor; } diff --git a/apps/admin/src/members/custom-fields/filter-renderer.test.tsx b/apps/admin/src/members/custom-fields/filter-renderer.test.tsx new file mode 100644 index 00000000000..b98dcf0224a --- /dev/null +++ b/apps/admin/src/members/custom-fields/filter-renderer.test.tsx @@ -0,0 +1,147 @@ +import { describe, expect, it, vi } from 'vitest'; +import { fireEvent, render, screen } from '@testing-library/react'; +import CustomFieldFilterRenderer from './filter-renderer'; +import type { FilterFieldConfig } from '@tryghost/shade/patterns'; + +vi.mock('@/shared/member-custom-fields/use-definitions', () => ({ + useCustomFieldDefinitionsIncludingArchived: () => ({ + data: [ + { + namespace: 'custom', + key: 'birthday', + name: 'Birthday', + type: 'short_text', + status: 'active', + }, + { namespace: 'custom', key: 'shipping', name: 'Shipping', type: 'address', status: 'active' }, + ], + }), +})); + +const PRESENCE_ONLY = [ + { value: 'is-set', label: 'is set' }, + { value: 'is-not-set', label: 'is not set' }, +]; + +const TEXT_OPERATORS = [ + { value: 'is', label: 'is' }, + { value: 'is-not', label: 'is not' }, + { value: 'contains', label: 'contains' }, + { value: 'does-not-contain', label: 'does not contain' }, + { value: 'starts-with', label: 'starts with' }, + { value: 'ends-with', label: 'ends with' }, + ...PRESENCE_ONLY, +]; + +function renderPill({ + operators, + defaultOperator, + operator, + onOperatorChange = () => {}, + fieldKey = 'metafields.custom.birthday', + label = 'Birthday', + values = ['', ''], +}: { + operators: FilterFieldConfig['operators']; + defaultOperator?: string; + operator: string; + onOperatorChange?: (operator: string) => void; + fieldKey?: string; + label?: string; + values?: string[]; +}) { + return render( + {}} + onOperatorChange={onOperatorChange} + />, + ); +} + +describe('CustomFieldFilterRenderer operators', () => { + it('offers only the operators the field declares', async () => { + renderPill({ operators: PRESENCE_ONLY, defaultOperator: 'is-set', operator: 'is-set' }); + + fireEvent.pointerDown(screen.getByLabelText('Birthday operator')); + await screen.findByRole('menu'); + + expect(screen.getByRole('menuitem', { name: 'is set' })).toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: 'contains' })).not.toBeInTheDocument(); + }); + + it('keeps an operator the field declares, even one outside the text vocabulary', () => { + const onOperatorChange = vi.fn(); + renderPill({ + operators: [{ value: 'is-or-less', label: 'is on or before' }, ...PRESENCE_ONLY], + defaultOperator: 'is-or-less', + operator: 'is-or-less', + onOperatorChange, + }); + + expect(onOperatorChange).not.toHaveBeenCalled(); + }); + + it('coerces an undeclared operator to the field default', () => { + const onOperatorChange = vi.fn(); + renderPill({ + operators: PRESENCE_ONLY, + defaultOperator: 'is-set', + operator: 'contains', + onOperatorChange, + }); + + expect(onOperatorChange).toHaveBeenCalledWith('is-set'); + }); + + it('still offers the full text vocabulary to a text field', async () => { + const onOperatorChange = vi.fn(); + renderPill({ + operators: TEXT_OPERATORS, + defaultOperator: 'contains', + operator: 'contains', + onOperatorChange, + }); + + expect(onOperatorChange).not.toHaveBeenCalled(); + fireEvent.pointerDown(screen.getByLabelText('Birthday operator')); + await screen.findByRole('menu'); + expect(screen.getByRole('menuitem', { name: 'contains' })).toBeInTheDocument(); + }); + + it('narrows a whole composite to presence, and opens up once a part is chosen', async () => { + const whole = renderPill({ + operators: TEXT_OPERATORS, + defaultOperator: 'is-set', + operator: 'is-set', + fieldKey: 'metafields.custom.shipping', + label: 'Shipping', + }); + + fireEvent.pointerDown(screen.getByLabelText('Shipping operator')); + await screen.findByRole('menu'); + expect(screen.getByRole('menuitem', { name: 'is set' })).toBeInTheDocument(); + expect(screen.queryByRole('menuitem', { name: 'contains' })).not.toBeInTheDocument(); + whole.unmount(); + + renderPill({ + operators: TEXT_OPERATORS, + defaultOperator: 'is-set', + operator: 'contains', + fieldKey: 'metafields.custom.shipping', + label: 'Shipping', + values: ['city', 'London'], + }); + + fireEvent.pointerDown(screen.getByLabelText('Shipping operator')); + await screen.findByRole('menu'); + expect(screen.getByRole('menuitem', { name: 'contains' })).toBeInTheDocument(); + }); +}); diff --git a/apps/admin/src/members/custom-fields/filter-renderer.tsx b/apps/admin/src/members/custom-fields/filter-renderer.tsx index 8718fede17e..b7b4e4db0d8 100644 --- a/apps/admin/src/members/custom-fields/filter-renderer.tsx +++ b/apps/admin/src/members/custom-fields/filter-renderer.tsx @@ -1,11 +1,28 @@ import React, { useEffect } from 'react'; -import { CUSTOM_FIELDS_PREFIX, CUSTOM_FIELD_OPERATORS } from '@/members/member-fields'; +import { parseMetafieldFieldId } from './addressing'; import { CUSTOM_FIELD_SET_OPERATORS } from './addressing'; import { FilterSegmentInput, FilterSegmentSelect } from '@tryghost/shade/patterns'; import { createOperatorOptions, listsOperator } from '@/shared/filters'; import { memberCustomFieldParts } from '@tryghost/admin-x-framework/api/member-custom-fields'; import { useCustomFieldDefinitionsIncludingArchived } from '@/shared/member-custom-fields/use-definitions'; -import type { CustomRendererProps } from '@tryghost/shade/patterns'; +import type { CustomRendererProps, FilterFieldConfig } from '@tryghost/shade/patterns'; + +// "Is set" and "is not set" apply to a field of any value type. +const PRESENCE_ONLY_OPTIONS = createOperatorOptions(CUSTOM_FIELD_SET_OPERATORS); + +function offeredOperators(field: FilterFieldConfig, wholeComposite: boolean) { + const declared = field.operators?.length ? field.operators : PRESENCE_ONLY_OPTIONS; + const options = wholeComposite + ? declared.filter((option) => listsOperator(CUSTOM_FIELD_SET_OPERATORS, option.value)) + : declared; + const ids = options.map((option) => option.value); + const fallback = + field.defaultOperator && ids.includes(field.defaultOperator) + ? field.defaultOperator + : (ids[0] ?? 'is-set'); + + return { options, ids, fallback }; +} const CustomFieldFilterRenderer: React.FC> = ({ field, @@ -16,9 +33,9 @@ const CustomFieldFilterRenderer: React.FC> = ({ readOnly, }) => { const { data } = useCustomFieldDefinitionsIncludingArchived(); - const definitions = data?.members_custom_fields ?? []; + const definitions = data ?? []; - const fieldKey = (field.key ?? '').slice(CUSTOM_FIELDS_PREFIX.length); + const fieldKey = parseMetafieldFieldId(field.key ?? '')?.key ?? ''; const definition = definitions.find((candidate) => candidate.key === fieldKey); const parts = definition ? (memberCustomFieldParts(definition.type) ?? []).map(({ key, label }) => ({ @@ -32,15 +49,18 @@ const CustomFieldFilterRenderer: React.FC> = ({ const [subfield = '', value = ''] = values; const isWholeField = subfield === ''; - const operators = - isComposite && isWholeField ? CUSTOM_FIELD_SET_OPERATORS : CUSTOM_FIELD_OPERATORS; + const { + options: operatorOptions, + ids: operators, + fallback: fallbackOperator, + } = offeredOperators(field, isComposite && isWholeField); useEffect(() => { - if (readOnly || !onOperatorChange || listsOperator(operators, operator)) { + if (readOnly || !onOperatorChange || operators.includes(operator)) { return; } - onOperatorChange('is-set'); - }, [readOnly, operator, operators, onOperatorChange]); + onOperatorChange(fallbackOperator); + }, [readOnly, operator, operators, fallbackOperator, onOperatorChange]); const needsValue = !listsOperator(CUSTOM_FIELD_SET_OPERATORS, operator); const partOptions = [{ value: '', label: 'Any' }, ...parts]; @@ -61,7 +81,7 @@ const CustomFieldFilterRenderer: React.FC> = ({ {onOperatorChange && ( | undefined; + metafields: Record | undefined> | undefined; disabled?: boolean; } @@ -174,7 +171,9 @@ const MemberCustomFieldEditModal: React.FC<{ // Strip this field's key prefix so the input sees '' / 'subfield' keys. const inputErrors = Object.fromEntries( Object.entries(errors).map(([key, message]) => [ - key === field.key ? '' : key.slice(field.key.length + 1), + key === `${field.namespace}.${field.key}` + ? '' + : key.slice(`${field.namespace}.${field.key}`.length + 1), message, ]), ); @@ -185,8 +184,8 @@ const MemberCustomFieldEditModal: React.FC<{ // refuses casual dismissal — Cancel is the one explicit way to discard, // so typed values can never be lost by a stray click. const isDirty = !dequal( - getEditableCustomFieldValues({ [field.key]: value }), - getEditableCustomFieldValues({ [field.key]: initialValue }), + getEditableCustomFieldValues({ [field.namespace]: { [field.key]: value } }), + getEditableCustomFieldValues({ [field.namespace]: { [field.key]: initialValue } }), ); const onSave = () => { @@ -205,7 +204,7 @@ const MemberCustomFieldEditModal: React.FC<{ setSaveAttempted(true); return; } - editMutation.mutate(buildCustomFieldSavePayload(memberId, field.key, value), { + editMutation.mutate(buildCustomFieldSavePayload(memberId, field, value), { onSuccess: () => { toast.success(`${field.name} saved`); onClose(); @@ -301,12 +300,12 @@ const MemberCustomFieldEditModal: React.FC<{ */ const MemberCustomFieldsField: React.FC = ({ memberId, - customFields, + metafields, disabled, }) => { const { data, isLoading } = useCustomFieldDefinitions(); - const fields = data?.members_custom_fields ?? []; - const values = getEditableCustomFieldValues(customFields); + const fields = data ?? []; + const values = getEditableCustomFieldValues(metafields); const [editingField, setEditingField] = React.useState(null); if (isLoading || fields.length === 0) { @@ -330,7 +329,11 @@ const MemberCustomFieldsField: React.FC = ({