Skip to content

Set custom field values on a member over the Admin API - #29373

Merged
rob-ghost merged 4 commits into
mainfrom
feat/ber-3790-member-custom-field-values
Jul 17, 2026
Merged

Set custom field values on a member over the Admin API#29373
rob-ghost merged 4 commits into
mainfrom
feat/ber-3790-member-custom-field-values

Conversation

@rob-ghost

@rob-ghost rob-ghost commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

ref BER-3799

The second vertical of membership custom fields: the fields defined in Settings (#29321, now merged to main) can now hold a value against a member, over the Admin API. Ships behind the membersCustomFields flag.

API only — no Admin UI. The Member Details entry form is a separate issue (BER-3800) that builds on this contract.

What this adds

A member's values are part of the member. GET /members/:id returns a custom_fields object keyed by field key, and PUT /members/:id accepts {members: [{custom_fields: {...}}]} on the same save that changes their name.

Writes merge rather than replace: only the keys present are touched, and null (or an empty string) clears a value. A caller that has never heard of a field cannot erase it. Values are validated against the field's own type from the shared catalog before anything is written, so one bad value fails the whole call rather than leaving half of it applied — and for a composite the error points at the offending sub-field (custom_fields.home_address.postal_code).

A field a member has no value for is absent, not null: "not set" is the absence of a row, so it stays one state rather than two.

Editing a member's custom fields fires the standard member.edited event — the audit action and the event webhooks (and any other consumers) listen to — exactly as changing a member's labels or newsletters does, and without bumping updated_at. Custom fields live in their own table, so a values-only edit doesn't touch the member row and the save emits nothing on its own; the edit declares the change into the member's change-set — the way a labels edit does — so its edited lifecycle fires the usual signals and the aggregate change is visible to downstream consumers. An integration syncing members therefore sees a custom-field change like any other member edit.

Setting values while creating a member is a later vertical, so POST /members with custom_fields is rejected explicitly rather than silently dropped.

Why values ride the member body

Ghost's member API already draws this line, and values fall on the near side of it.

Things with their own lifecycle, or that are commands, get sub-routes: /members/:id/subscriptions/:subscription_id exists because a subscription lives in Stripe and can be cancelled; /members/:id/commenting/enable is an action. Plain member attributes ride the body: labels, newsletters and tiers are all separate tables joined to the member, and not one of them has a sub-route.

A custom field value has no independent lifecycle, no external system, no operations beyond set and clear, and it cascade-deletes with the member. It also has no permission of its own — the definitions permissions migration in #29321 already decided that "values ride on the existing member edit permission". An API that modelled values as a separate resource would contradict a decision that already shipped.

There's a product reason too: the premise of the feature is that publishers extend the member record. If Ghost's fields are on the member and the publisher's are at a different URL, publisher-defined fields are structurally second-class.

Architecture

The catalog stays the single source of truth for what a field type is. This vertical is its first real consumer beyond the type enum: it supplies both the validation for a value and the storage type that routes the value to a column.

Definitions and values are owned by two sibling services. CustomFieldDefinitionsService (renamed here from the generic service.ts) owns the definitions table and serves the Admin API's custom-field endpoints. CustomFieldValuesService owns the values table and rides the member read/write path. The values service reads the definitions table directly — via a shared active-fields query in queries.ts — rather than depending on the definitions service, matching how the other knex services here talk to the database and keeping the read and write paths consistent.

flowchart TB
    subgraph FE["Frontend (Admin)"]
        MemberDetails["Member Details<br/>(next vertical — BER-3800)"]
    end

    subgraph SHARED["Shared"]
        Catalog["Field Catalog<br/>Field Type → Storage Type + value validation"]
    end

    subgraph BE["Backend (Ghost Core)"]
        MembersAPI["Members Admin API<br/>auth · permissions · flag"]
        BREAD["Member BREAD<br/>read · edit · browse"]
        ValuesSvc["CustomFieldValuesService<br/>read/write values"]
        DefsSvc["CustomFieldDefinitionsService<br/>manage definitions"]
        Codecs["Storage Codecs<br/>text → value_text · json → value_json"]
    end

    subgraph DB["Persistence"]
        DefTable[("members_custom_fields")]
        ValTable[("members_custom_field_values")]
    end

    MemberDetails -.-> MembersAPI
    MembersAPI --> BREAD --> ValuesSvc
    ValuesSvc --> Codecs --> ValTable
    ValuesSvc -- "reads active fields directly" --> DefTable
    DefsSvc --> DefTable
    Catalog -.-> ValuesSvc
    Catalog -.-> Codecs
Loading

Persistence. One row per (member, field) — the table the definitions migration deferred. The definition's type maps through the catalog to a storage type, which picks the column: exactly one of value_text / value_json is populated per row, and the other is explicitly nulled on every write rather than left to convention. Splitting text from json keeps text-backed values directly queryable rather than behind a JSON extraction, which is what filtering will need next.

Values hang off the definition's internal id, not its key: the key is the wire handle, the id is the stable spine. The FK column is custom_field_id (not member_custom_field_id) so knex's derived unique-index name stays within MySQL's 64-character identifier limit. Both foreign keys cascade, so a member's values vanish with the member and a definition's values are tied to the right definition forever. The unique constraint is what makes a write an upsert rather than a read-then-write race.

erDiagram
    members ||--o{ members_custom_field_values : "has values"
    members_custom_fields ||--o{ members_custom_field_values : "defines"

    members_custom_fields {
        string id PK
        string key UK
        string name UK
        string type
        string status
    }

    members_custom_field_values {
        string id PK
        string custom_field_id FK "cascade"
        string member_id FK "cascade"
        text value_text "one of these two"
        text value_json "is populated"
    }
Loading

Unique on (member_id, custom_field_id).

Read by default, browse on request

A read returns values whenever the flag is on, with no include. That's the convention: a member read already returns labels, tiers and newsletters unasked — include=tiers is a no-op on read — and include is reserved for things that are expensive or aggregate (email_recipients, counts). One flat lookup on a read that already issues a dozen queries is neither. The flag, not an include, is what protects consumers that predate the feature; an include would outlive the flag and become permanent API surface.

Browse is the opposite and keeps ?include=custom_fields, exactly as products/tiers already behave: a read always carries them, a list only when asked. Browse reads the whole page in one query, so it cannot become an N+1. This keeps the existing "identical member format for read, edit and browse" invariant intact, which is why that test now asks for include=tiers,custom_fields the same way it already asked for tiers.

Limits

Bounds are stated in the unit the storage is actually measured in.

Bound
short_text 255 characters
long_text 65,535 bytes
address 255 per line / city / state, 32 postal code, 2 country

long_text was previously bounded in characters while the column is sized in bytes, so a multibyte value inside the stated limit overflowed it — 65,535 emoji is four times over. 65,535 bytes matches what Shopify documents for its metafields, and matches a MySQL TEXT column exactly, so the column is a second guard rather than a formality. The address sub-fields had no maximum at all, which meant a composite with unbounded members had no bound: an address accepted a megabyte in a single line.

There is deliberately no cap on the number of field definitions yet. Worth knowing that leaves total per-member storage unbounded regardless of the per-value limits, and capping count is the cheaper lever if that becomes a concern.

Scope

In: reading and writing values on a member (GET/PUT /members/:id), values on browse via include, every field type including the composite address.

Out (later verticals): the Member Details entry form (BER-3800), creating a member with values in one call, NQL filtering and segmentation on values, and values in CSV import/export.

Known trade-off

custom_fields reaches the service through a wrapper around the members input validator. The member body schemas live in @tryghost/admin-api-schema — an external, pinned package — and set additionalProperties: false, so ajv silently strips any property the schema doesn't declare, with no error to explain where the values went. The wrapper stashes the key across validation and restores it, gated on the flag so it is a no-op when the feature is off.

It's a seam, not a fix. Declaring custom_fields upstream and deleting the wrapper is tracked as BER-3801.

How to review

Four atomic commits, each a single concern building on the last:

  1. Values table — one row per (member, field), cascade from both sides, unique on (member, field). custom_field_id keeps the index name within MySQL's identifier limit; value_text/value_json are TEXT, sized to the catalog's byte bound.
  2. Value service — introduces CustomFieldValuesService (and renames the existing service to CustomFieldDefinitionsService), the first real consumer of the shared catalog: validation per field type, storage-type routing, merge semantics, and validation split from application so a bad value writes nothing. Includes the byte/address bounds and the shared active-fields query the values service uses to read definitions directly.
  3. Admin API — the members endpoint, serializers, the validator wrapper, and the value service injected through the BREAD service the same way giftService already is. Read-by-default with browse via include.
  4. Labs comment fix — unrelated to the feature, found while working out why a private flag moved E2E snapshots (the comment named two constants that don't exist and pointed at the wrong file).

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Adds a members_custom_field_values table, schema typing, storage codecs, and a values service for reading and writing member custom-field data. Member APIs now support custom-field validation, persistence, read responses, browse inclusion, clearing, and lifecycle events behind the membersCustomFields flag. Field validation adds byte-aware long-text limits and address sub-field limits. Export coverage, migrations, schema integrity, and end-to-end and unit tests are updated.

Suggested labels: preview

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: setting member custom field values through the Admin API.
Description check ✅ Passed The description is directly about the same feature and matches the implemented changes and scope.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ber-3790-member-custom-field-values

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the migration [pull request] Includes migration for review label Jul 15, 2026
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

It looks like this PR contains a migration 👀
Here's the checklist for reviewing migrations:

General requirements

  • ⚠️ Tested performance on staging database servers, as performance on local machines is not comparable to a production environment
  • Satisfies idempotency requirement (both up() and down())
  • Does not reference models
  • Filename is in the correct format (and correctly ordered)
  • Targets the next minor version
  • All code paths have appropriate log messages
  • Uses the correct utils
  • Contains a minimal changeset
  • Does not mix DDL/DML operations
  • Tested in MySQL and SQLite

Schema changes

  • Both schema change and related migration have been implemented
  • For index changes: has been performance tested for large tables
  • For new tables/columns: fields use the appropriate predefined field lengths
  • For new tables/columns: field names follow the appropriate conventions
  • Does not drop a non-alpha table outside of a major version

Data changes

  • Mass updates/inserts are batched appropriately
  • Does not loop over large tables/datasets
  • Defends against missing or invalid data
  • For settings updates: follows the appropriate guidelines

Review notes

One migration, 6.53/2026-07-15-20-22-53-add-members-custom-field-values.js: a single addTable call creating a new table. Pure DDL, no backfill, nothing dropped.

Evidence for the checked items

Item Basis
Idempotency (up() and down()) Both guard on hasTable before acting — addTable in core/server/data/migrations/utils/tables.js
Does not reference models The migration imports addTable and nothing else
Filename format and order Generated by pnpm migrate:create. Ordering is load-bearing here: 2026-07-15-20-22-53 sorts after the definitions table (2026-07-14-16-47-54) in the same series, which it needs, since it has an FK to members_custom_fields.id
Targets next minor 6.53; core is already 6.53.0-rc.0 (bumped by the definitions migration on the base branch)
Log messages on all paths addTable logs info when it acts and warn when it skips, on both up() and down()
Correct utils addTable
Minimal changeset Migration + schema.js + exporter table list + exporter test + integrity hash
No DDL/DML mixing CREATE TABLE only
MySQL and SQLite Both, and this box caught a real bug — see below. SQLite via knex-migrator init + the full e2e/legacy/integration suites; MySQL 8.4 locally (migration + the custom-fields suite + test:legacy) and via the Acceptance tests (mysql8) job
Field lengths id and both FK columns string(24) (ObjectID); value_text / value_json are TEXT (65,535 bytes), sized deliberately to match the catalog's byte bound on long_text. Verified on MySQL that they are text and not longtext — a distinction SQLite doesn't make
Field names members_* table convention, *_id FK convention, created_at / updated_at. The FK is custom_field_id, not the member_custom_field_id the referenced table would imply — see below
Does not drop a table Adds only

Struck through

  • Index performance on large tables — the only index is the unique constraint on (member_id, member_custom_field_id), created with the table itself. No index is added to any existing table.
  • All four data-change items — the migration contains no DML. Nothing is updated, inserted, looped over, or backfilled, and no settings are touched.

⚠️ Raised: staging performance is not tested

Left unchecked deliberately rather than struck through, because it isn't irrelevant — just not something I can do.

The reasoning for why I believe it's low risk, so a reviewer can sanity-check it rather than take it on trust:

  • It's a CREATE TABLE for a new, empty table. No backfill, no ALTER on an existing table, no index added to a populated table.
  • The FKs to members and members_custom_fields are validated against zero child rows, so there's no scan of members — which is the table that's actually large on Pro.
  • Both referenced columns are primary keys, so no index needs creating to support the constraints.

What I'd still want a second opinion on: whether adding FK constraints that reference a large members table takes a metadata lock worth caring about on Pro at peak. That's the one part local testing genuinely can't answer, and it needs someone with staging access.

Why the FK is custom_field_id

Worth explaining, because the obvious name is wrong and someone will want to "fix" it.

The referenced table is members_custom_fields, so the column ought to be member_custom_field_id. It was, and the migration failed on MySQL:

ER_TOO_LONG_IDENT: Identifier name
'members_custom_field_values_member_id_member_custom_field_id_unique'
is too long

knex derives the unique index name from the table plus both columns, which came to 67 characters against MySQL's 64-character limit. SQLite has no such limit, so it passed locally and every SQLite CI job, and only the MySQL job caught it. Shortening the column brings the index to 60 and the FK to 51. There's a comment on the column recording this, so the rename doesn't look arbitrary.

This is the checklist item earning its keep: I'd verified the migration on SQLite and would have called it tested. It's also why the box above is now backed by an actual MySQL 8.4 run rather than by CI alone.

Two things not on the checklist, worth knowing

  1. The migration lives in one commit with the final schema baked in. The PR was since squashed, so Added the members_custom_field_values table carries the migration as it will land: TEXT (not LONGTEXT) value columns sized to the catalog's byte bound, and the FK named custom_field_id (not member_custom_field_id) so knex's derived unique-index name stays within MySQL's 64-char limit. There's no "review at HEAD vs at the introducing commit" caveat — the introducing commit already has the final form.
  2. The new table is in BACKUP_TABLES, so it's included in a full export (alongside members and members_custom_fields), and in the exporter test's table list.

@nx-cloud

nx-cloud Bot commented Jul 15, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit db23b9d

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 54s View ↗
nx run ghost:test:integration ✅ Succeeded 3m View ↗
nx run @tryghost/admin:test:acceptance ✅ Succeeded 4m 43s View ↗
nx run ghost:test:legacy ✅ Succeeded 3m 16s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 42s View ↗
nx run-many -t test:unit -p ghost,@tryghost/cus... ✅ Succeeded 33s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 21s View ↗
nx run @tryghost/activitypub:test:acceptance ✅ Succeeded 42s View ↗
Additional runs (4) ✅ Succeeded ... View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-16 23:59:19 UTC

@github-actions

Copy link
Copy Markdown
Contributor

E2E Tests Failed

To view the Playwright test report locally, run:

REPORT_DIR=$(mktemp -d) && gh run download 29454904873 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR"

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.75701% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.26%. Comparing base (88a1a1d) to head (db23b9d).

Files with missing lines Patch % Lines
...r/services/members-custom-fields/values-service.ts 94.58% 11 Missing ⚠️
...re/server/services/members-custom-fields/schema.ts 97.72% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #29373      +/-   ##
==========================================
+ Coverage   74.17%   74.26%   +0.09%     
==========================================
  Files        1592     1595       +3     
  Lines      139000   139505     +505     
  Branches    16858    16935      +77     
==========================================
+ Hits       103100   103602     +502     
+ Misses      34881    34852      -29     
- Partials     1019     1051      +32     
Flag Coverage Δ
e2e-tests 76.33% <97.75%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
ghost/core/core/server/services/members/members-api/services/member-bread-service.js (1)

36-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Correct the custom-fields service interface.

The class calls getValuesForMembers and assertValuesValid, but the typedef declares getValues and omits validation.

Proposed fix
- * `@prop` {{getValues: (memberId: string) => Promise<Record<string, unknown>>, setValues: (memberId: string, values: unknown) => Promise<void>}} service
+ * `@prop` {{
+ *   getValuesForMembers: (memberIds: string[]) => Promise<Map<string, Record<string, unknown>>>,
+ *   assertValuesValid: (values: unknown) => Promise<void>,
+ *   setValues: (memberId: string, values: unknown) => Promise<void>
+ * }} service
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@ghost/core/core/server/services/members/members-api/services/member-bread-service.js`
around lines 36 - 39, Update the ICustomFieldsServiceWrapper typedef to declare
the actual custom-fields service methods used by the class: replace getValues
with getValuesForMembers and add assertValuesValid, preserving accurate
parameter and return types based on their call sites.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@ghost/core/core/server/services/members/members-api/services/member-bread-service.js`:
- Around line 529-532: Update the member edit flow around the member update and
custom-field setValues calls to execute both writes within one shared
transaction, preserving the pre-validation behavior before any mutation. Pass
the transaction context into setValues, and update the members custom-fields
service contract and implementation so setValues reuses the supplied transaction
instead of opening an independent one; ensure both writes commit or roll back
together.

---

Nitpick comments:
In
`@ghost/core/core/server/services/members/members-api/services/member-bread-service.js`:
- Around line 36-39: Update the ICustomFieldsServiceWrapper typedef to declare
the actual custom-fields service methods used by the class: replace getValues
with getValuesForMembers and add assertValuesValid, preserving accurate
parameter and return types based on their call sites.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c88103f8-a307-4535-8643-4940bb5fd339

📥 Commits

Reviewing files that changed from the base of the PR and between 2ceab58 and 8426068.

⛔ Files ignored due to path filters (3)
  • ghost/core/test/e2e-api/admin/__snapshots__/member-commenting.test.js.snap is excluded by !**/*.snap
  • ghost/core/test/e2e-api/admin/__snapshots__/members-edit-subscriptions.test.js.snap is excluded by !**/*.snap
  • ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap is excluded by !**/*.snap
📒 Files selected for processing (20)
  • ghost/core/core/server/api/endpoints/members.js
  • ghost/core/core/server/api/endpoints/utils/serializers/input/members.js
  • ghost/core/core/server/api/endpoints/utils/serializers/output/members.js
  • ghost/core/core/server/api/endpoints/utils/validators/input/members.js
  • ghost/core/core/server/data/exporter/table-lists.js
  • ghost/core/core/server/data/migrations/versions/6.53/2026-07-15-20-22-53-add-members-custom-field-values.js
  • ghost/core/core/server/data/schema/schema.js
  • ghost/core/core/server/services/members-custom-fields/schema.ts
  • ghost/core/core/server/services/members-custom-fields/service.ts
  • ghost/core/core/server/services/members-custom-fields/storage.ts
  • ghost/core/core/server/services/members/api.js
  • ghost/core/core/server/services/members/members-api/members-api.js
  • ghost/core/core/server/services/members/members-api/services/member-bread-service.js
  • ghost/core/core/shared/labs.js
  • ghost/core/test/e2e-api/admin/member-custom-fields.test.ts
  • ghost/core/test/e2e-api/admin/members.test.js
  • ghost/core/test/integration/exporter/exporter.test.js
  • ghost/core/test/unit/server/data/schema/integrity.test.js
  • ghost/core/test/unit/server/services/members/members-api/services/members-bread-service.test.js
  • packages/custom-field-types/src/index.ts

@github-actions

Copy link
Copy Markdown
Contributor

E2E Tests Failed

To view the Playwright test report locally, run:

REPORT_DIR=$(mktemp -d) && gh run download 29455631540 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR"

@rob-ghost
rob-ghost force-pushed the feat/ber-3790-member-custom-field-values branch 2 times, most recently from c38999f to d53ea25 Compare July 16, 2026 10:21
Base automatically changed from feat/custom-field-definitions to main July 16, 2026 10:33
@rob-ghost
rob-ghost force-pushed the feat/ber-3790-member-custom-field-values branch 2 times, most recently from b61814d to 48e2b5b Compare July 16, 2026 12:58
@rob-ghost
rob-ghost force-pushed the feat/ber-3790-member-custom-field-values branch from 48e2b5b to 8b59c74 Compare July 16, 2026 20:07
ref BER-3799

The value side of member custom fields: one row per (member, field
definition), the table the definitions migration deferred. FKs
cascade-delete from both members_custom_fields and members, with a unique
constraint on (member_id, custom_field_id). The FK column is named
custom_field_id, not member_custom_field_id, so knex's derived
unique-index name stays within MySQL's 64-character identifier limit.
Exactly one of value_text/value_json is populated per row, routed by the
field type's storage type; both are TEXT (65,535 bytes), sized to the
catalog's byte bound on long_text. No new permissions - values ride the
existing member edit permission. Lands in the same 6.53 series as the
definitions table. Verified on MySQL 8.4 and SQLite.
ref BER-3799

Introduces CustomFieldValuesService, owning the members_custom_field_values
table: getValuesForMembers/getValues read a whole page in one query so a member
list can't become an N+1, and assertValuesValid/setValues write. Writes are
merge semantics — only keys present are touched, null or empty string clears a
value, unknown keys are rejected, and every value is validated before any write
so one bad value fails the whole call. Composite (address) errors carry the
sub-field path.

This is the first real consumer of the shared @tryghost/custom-field-types
catalog beyond the type enum: FIELD_TYPES[type].value validates a value and
storageType routes it to value_text or value_json via new storage codecs that
always null the unused column. The catalog now bounds long_text in bytes
(65,535) rather than characters, because the storage column is sized in bytes
and a multibyte value inside a character bound would overflow it; this matches
how Shopify documents its metafield limits. Address sub-fields are bounded too
(line1/line2/city/state 255, postal_code 32); previously only country had a
bound, so an address accepted a megabyte in a single line.

The existing definitions service is renamed from CustomFieldsService/service.ts
to CustomFieldDefinitionsService/definitions-service.ts, and the module now
exports definitionsService alongside valuesService — once there are two
services, an unqualified "service" meaning "definitions" is confusing. The
values service reads members_custom_fields directly for both the read join and
the write-time active-field lookup rather than depending on the definitions
service, matching how the other services here talk to the database.
@rob-ghost
rob-ghost force-pushed the feat/ber-3790-member-custom-field-values branch from 8b59c74 to f1b7e4b Compare July 16, 2026 21:08
@rob-ghost
rob-ghost marked this pull request as ready for review July 16, 2026 21:24

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@ghost/core/core/server/services/members/members-api/services/member-bread-service.js`:
- Around line 527-547: Remove the memberBodyEmpty input-shape check and
determine whether the native member save was a no-op from the returned model’s
_changed attributes. Update the manual member.edited event condition near the
custom-field write so it fires when custom fields are written and the native
save produced no changes, including full PUT payloads with unchanged native
properties.
- Around line 593-607: Update the custom-field event condition in the member
save flow to inspect the Bookshelf model’s `_changed` state, rather than relying
only on `memberBodyEmpty`. Emit `member.edited` for non-empty planned
custom-field changes when the native save produced no changed attributes, while
preserving the existing behavior when native member changes already emitted the
event.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ffa2521e-a5b2-4c53-8be5-f1face849858

📥 Commits

Reviewing files that changed from the base of the PR and between 7d2bfa6 and f1b7e4b.

📒 Files selected for processing (18)
  • ghost/core/core/server/api/endpoints/member-custom-fields.ts
  • ghost/core/core/server/api/endpoints/members.js
  • ghost/core/core/server/api/endpoints/utils/serializers/input/members.js
  • ghost/core/core/server/api/endpoints/utils/serializers/output/members.js
  • ghost/core/core/server/api/endpoints/utils/validators/input/members.js
  • ghost/core/core/server/data/exporter/table-lists.js
  • ghost/core/core/server/data/migrations/versions/6.53/2026-07-15-20-22-53-add-members-custom-field-values.js
  • ghost/core/core/server/data/schema/schema.js
  • ghost/core/core/server/services/members-custom-fields/definitions-service.ts
  • ghost/core/core/server/services/members-custom-fields/index.ts
  • ghost/core/core/server/services/members-custom-fields/queries.ts
  • ghost/core/core/server/services/members-custom-fields/schema.ts
  • ghost/core/core/server/services/members-custom-fields/storage.ts
  • ghost/core/core/server/services/members-custom-fields/values-service.ts
  • ghost/core/core/server/services/members/api.js
  • ghost/core/core/server/services/members/members-api/members-api.js
  • ghost/core/core/server/services/members/members-api/repositories/member-repository.js
  • ghost/core/core/server/services/members/members-api/services/member-bread-service.js
🚧 Files skipped from review as they are similar to previous changes (6)
  • ghost/core/core/server/data/migrations/versions/6.53/2026-07-15-20-22-53-add-members-custom-field-values.js
  • ghost/core/core/server/services/members/members-api/members-api.js
  • ghost/core/core/server/services/members-custom-fields/storage.ts
  • ghost/core/core/server/services/members/api.js
  • ghost/core/core/server/services/members-custom-fields/schema.ts
  • ghost/core/core/server/data/exporter/table-lists.js

@rob-ghost
rob-ghost force-pushed the feat/ber-3790-member-custom-field-values branch from f1b7e4b to 6905787 Compare July 16, 2026 21:40

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
ghost/core/test/e2e-api/admin/member-custom-fields.test.ts (1)

580-593: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that custom-field-only edits preserve updated_at.

This test covers the lifecycle requirement but not the related contract that values-only edits must leave the member timestamp unchanged.

Proposed test extension
 const memberId = await createMember();
+const before = (await agent.get(`members/${memberId}/`).expectStatus(200)).body.members[0].updated_at;
 await models.Base.knex('actions').where('resource_id', memberId).del();

 const editedEvents = await countMemberEditedEvents(memberId, () => setValues(memberId, {[field.key]: 'Ghosts'}));
+const after = (await agent.get(`members/${memberId}/`).expectStatus(200)).body.members[0].updated_at;

 assert.equal(editedEvents, 1);
 assert.equal((await memberEditedActions(memberId)).length, 1);
+assert.equal(after, before);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ghost/core/test/e2e-api/admin/member-custom-fields.test.ts` around lines 580
- 593, Extend the test for custom-field-only edits in the member.edited
lifecycle case to capture the member’s updated_at before setValues and assert it
remains unchanged afterward. Keep the existing event and audit-action assertions
intact, using the member lookup mechanism already established in the test suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ghost/core/test/e2e-api/admin/member-custom-fields.test.ts`:
- Around line 580-593: Extend the test for custom-field-only edits in the
member.edited lifecycle case to capture the member’s updated_at before setValues
and assert it remains unchanged afterward. Keep the existing event and
audit-action assertions intact, using the member lookup mechanism already
established in the test suite.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 69859b2c-8105-4d20-8e0e-8f1c27cd9452

📥 Commits

Reviewing files that changed from the base of the PR and between f1b7e4b and 6905787.

⛔ Files ignored due to path filters (4)
  • ghost/core/test/e2e-api/admin/__snapshots__/member-commenting.test.js.snap is excluded by !**/*.snap
  • ghost/core/test/e2e-api/admin/__snapshots__/members-edit-subscriptions.test.js.snap is excluded by !**/*.snap
  • ghost/core/test/e2e-api/admin/__snapshots__/members.test.js.snap is excluded by !**/*.snap
  • ghost/core/test/e2e-api/members/__snapshots__/webhooks.test.js.snap is excluded by !**/*.snap
📒 Files selected for processing (12)
  • ghost/core/core/server/api/endpoints/members.js
  • ghost/core/core/server/api/endpoints/utils/serializers/input/members.js
  • ghost/core/core/server/api/endpoints/utils/serializers/output/members.js
  • ghost/core/core/server/api/endpoints/utils/validators/input/members.js
  • ghost/core/core/server/services/members/api.js
  • ghost/core/core/server/services/members/members-api/members-api.js
  • ghost/core/core/server/services/members/members-api/repositories/member-repository.js
  • ghost/core/core/server/services/members/members-api/services/member-bread-service.js
  • ghost/core/core/shared/labs.js
  • ghost/core/test/e2e-api/admin/member-custom-fields.test.ts
  • ghost/core/test/e2e-api/admin/members.test.js
  • ghost/core/test/unit/server/services/members/members-api/services/members-bread-service.test.js
🚧 Files skipped from review as they are similar to previous changes (10)
  • ghost/core/core/server/api/endpoints/utils/serializers/output/members.js
  • ghost/core/core/server/api/endpoints/members.js
  • ghost/core/test/unit/server/services/members/members-api/services/members-bread-service.test.js
  • ghost/core/core/server/services/members/api.js
  • ghost/core/core/server/services/members/members-api/members-api.js
  • ghost/core/core/server/api/endpoints/utils/serializers/input/members.js
  • ghost/core/test/e2e-api/admin/members.test.js
  • ghost/core/core/server/services/members/members-api/repositories/member-repository.js
  • ghost/core/core/shared/labs.js
  • ghost/core/core/server/services/members/members-api/services/member-bread-service.js

ref BER-3799

GET /members/:id now returns a custom_fields object keyed by field key
whenever the flag is on, no include needed, since a member read already
returns labels, tiers and newsletters unasked. PUT /members/:id accepts
{members:[{custom_fields:{...}}]} on the member's existing save. Values
ride the member body rather than a sub-resource because Ghost already
draws that line: labels/newsletters/tiers are separate tables carried on
the member, while sub-routes are for things with their own lifecycle or
commands. A value has no lifecycle of its own and already rides the
member edit permission.

The value service is injected through api.js -> members-api.js ->
MemberBREADService, mirroring giftService. Bad values are validated
before the member is touched, so a 422 doesn't return with the rest of
the edit applied. Browse keeps ?include=custom_fields (opt-in, like
products/tiers), fetching the whole page in one query to avoid an N+1,
preserving the identical-member-format invariant across read, edit and
browse. The input validator wraps jsonSchema.validate to stash
custom_fields across validation because @tryghost/admin-api-schema sets
additionalProperties:false and strips the key; it's flag-gated so it's a
no-op when off. Member snapshots gain a custom_fields key because E2E
fixtures enable every private flag; production is unaffected while the
flag is off.
ref BER-3799

The comment claimed all flags in GA_FEATURES, BETA_FEATURES, and
ALPHA_FEATURES are globally enabled during E2E tests, which was wrong on
three counts: BETA_FEATURES and ALPHA_FEATURES don't exist (the constants
are PUBLIC_BETA_FEATURES and PRIVATE_FEATURES), GA_FEATURES are always true
everywhere rather than being a test behaviour, and the mechanism lives in
enableAllLabsFeatures in test/utils/fixture-utils.js — not labs.js — which
enables every key in WRITABLE_KEYS_ALLOWLIST on each fixture init. The
comment now names the real mechanism and file, and spells out the
consequence: a response key behind a private flag still moves E2E snapshots
even though the flag is off in production.
@rob-ghost
rob-ghost force-pushed the feat/ber-3790-member-custom-field-values branch from 6905787 to db23b9d Compare July 16, 2026 23:02
@rob-ghost
rob-ghost merged commit d8cc811 into main Jul 17, 2026
94 of 98 checks passed
@rob-ghost
rob-ghost deleted the feat/ber-3790-member-custom-field-values branch July 17, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

migration [pull request] Includes migration for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant