Skip to content

Changed members Stripe customer count filter to an NQL aggregate relation#28547

Merged
kevinansfield merged 4 commits into
mainfrom
members-aggregate-customer-count-filter
Jun 12, 2026
Merged

Changed members Stripe customer count filter to an NQL aggregate relation#28547
kevinansfield merged 4 commits into
mainfrom
members-aggregate-customer-count-filter

Conversation

@kevinansfield

@kevinansfield kevinansfield commented Jun 12, 2026

Copy link
Copy Markdown
Member

ref https://linear.app/ghost/issue/BER-3717/
ref TryGhost/NQL#152

The count.active_stripe_customers members filter was hardcoded outside of NQL — the input serializer pattern-matched the exact filter string and the member model injected a bespoke knex subquery. That meant the filter only worked in the exact count.active_stripe_customers:>1 form: combining it with any other filter, or using any other comparison, returned a 400, so it couldn't be composed like a normal members filter.

TryGhost/NQL#152 added a native aggregate relation type to mongo-knex (published as nql 0.13.0 / mongo-knex 0.10.0). This PR removes all of the hardcoded plumbing (serializer extraction, custom model query, repository special-casing) and defines the filter as a regular active_stripe_customers_count aggregate relation on the member model. The public count.active_stripe_customers filter key is unchanged — a filter expansion maps it onto the relation — so existing API consumers are unaffected, but the filter now composes with other filters and supports all numeric comparisons. Non-numeric values fail closed inside mongo-knex and still surface as 400s.

Note: @tryghost/bookshelf-plugins is bumped to 2.2.4 so that bookshelf-filter — the package that builds filter queries — depends on nql 0.13.0 and gets aggregate relation support.

@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR removes custom query handling for count.active_stripe_customers and replaces it with a declarative aggregate and nql filter expansion on the Member model. Serializer extraction/validation and repository option handling for activeStripeCustomersCount are removed. E2E tests are updated for filter composition and non-numeric validation. Dependency pins for @tryghost/nql and @tryghost/bookshelf-plugins are updated.

Possibly related PRs

  • TryGhost/Ghost#28232: Introduced the count.active_stripe_customers:>1 parsing/validation helper that this PR removes.
  • TryGhost/Ghost#28507: Modifies applyCustomQuery/subquery logic for activeStripeCustomersCount and touches the same member filtering surfaces.

Suggested reviewers

  • rob-ghost
  • jonatansberg
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: converting the members Stripe customer count filter from a hardcoded implementation to an NQL aggregate relation.
Description check ✅ Passed The description is directly related to the changeset, explaining the motivation, implementation details, and impact of replacing the hardcoded filter with an NQL aggregate relation.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch members-aggregate-customer-count-filter

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

@nx-cloud

nx-cloud Bot commented Jun 12, 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 df7ac43

Command Status Duration Result
nx build @tryghost/announcement-bar ✅ Succeeded <1s View ↗
nx build @tryghost/portal ✅ Succeeded 1s View ↗
nx build @tryghost/activitypub ✅ Succeeded 3s View ↗
nx build @tryghost/signup-form ✅ Succeeded <1s View ↗
nx build @tryghost/sodo-search ✅ Succeeded <1s View ↗
nx build @tryghost/comments-ui ✅ Succeeded <1s View ↗
nx build @tryghost/admin-toolbar ✅ Succeeded 1s View ↗
nx run @tryghost/admin-x-settings:test:acceptance ✅ Succeeded 9m 49s View ↗
Additional runs (15) ✅ Succeeded ... View ↗

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


☁️ Nx Cloud last updated this comment at 2026-06-12 17:34:48 UTC

@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.76%. Comparing base (9ede96c) to head (df7ac43).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...bers/members-api/repositories/member-repository.js 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #28547      +/-   ##
==========================================
- Coverage   73.76%   73.76%   -0.01%     
==========================================
  Files        1541     1541              
  Lines      132282   132252      -30     
  Branches    15852    15853       +1     
==========================================
- Hits        97584    97557      -27     
- Misses      33713    33730      +17     
+ Partials      985      965      -20     
Flag Coverage Δ
admin-tests 54.89% <ø> (ø)
e2e-tests 75.91% <95.65%> (-0.01%) ⬇️

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.

…tion

ref https://linear.app/ghost/issue/BER-3717/
ref TryGhost/NQL#152

- the `count.active_stripe_customers` filter was hardcoded outside of NQL: the input serializer pattern-matched the exact filter string and the member model injected a bespoke knex subquery, so the filter only worked in the exact `count.active_stripe_customers:>1` form and 400'd when combined with any other filter or comparison
- mongo-knex now supports a native `aggregate` relation type, so the filter is defined as a regular `active_stripe_customers_count` filter relation on the member model and composes with other filters like any other NQL filter; the public filter key is unchanged via a filter expansion
- non-numeric values fail closed inside mongo-knex and still surface as 400s
- `@tryghost/mongo-knex` is temporarily resolved from a local checkout of the NQL branch via a pnpm `link:` override until it's published
ref TryGhost/NQL#152

- the aggregate relation support is now published as nql 0.13.0 / mongo-knex 0.10.0, so the temporary pnpm `link:` override pointing at a local NQL checkout is no longer needed
- bookshelf-filter pins nql@0.12.10 exactly, so nql is force-resolved to the catalog version via a pnpm override to make sure filter queries are built with aggregate relation support
- removed the nql@0.12.10 packageExtension as nothing resolves that version anymore and 0.13.0 declares its lodash dependency properly
@kevinansfield kevinansfield force-pushed the members-aggregate-customer-count-filter branch from f0ee2e5 to 74d97aa Compare June 12, 2026 17:01
@kevinansfield kevinansfield marked this pull request as ready for review June 12, 2026 17:02

@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

🤖 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/repositories/member-repository.js`:
- Around line 966-969: The 400 error is misleading because the guard checks for
either filter or search but the thrown message uses
messages.bulkActionRequiresFilter only; update the thrown error so the message
references both "filter" and "search" (e.g., use or create a
messages.bulkActionRequiresFilterOrSearch key or change the tpl call to include
search) where the code throws in member-repository.js (the if block checking
filter, search, all) so the processLogger/error text reflects "filter, search,
or all=true" when calling tpl(messages.bulkActionRequiresFilter..., {action:
'bulk edit'}).
🪄 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: a208eb59-7df6-4a14-9bbe-1578291c9218

📥 Commits

Reviewing files that changed from the base of the PR and between 9ede96c and 74d97aa.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • ghost/core/core/server/api/endpoints/utils/serializers/input/members.js
  • ghost/core/core/server/models/member.js
  • ghost/core/core/server/services/members/members-api/repositories/member-repository.js
  • ghost/core/test/e2e-api/admin/members.test.js
  • pnpm-workspace.yaml
💤 Files with no reviewable changes (1)
  • ghost/core/core/server/api/endpoints/utils/serializers/input/members.js

ref #28547 (comment)

- the bulk edit/delete guards accept a filter, search, or all=true but the 400 error message only mentioned filter and all=true, making it misleading for callers using search
…ride

ref #28547

- bookshelf-filter 2.2.4 now depends on nql 0.13.0 directly, so filter queries get aggregate relation support without force-resolving nql via a pnpm override

@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

🤖 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/package.json`:
- Line 96: The package.json entry for "`@tryghost/bookshelf-plugins`": "2.2.4"
relies on a transitive dependency "`@tryghost/nql`@0.13.0" (via
"`@tryghost/bookshelf-filter`@2.2.4"); add an explicit dependency for
"`@tryghost/nql`": "0.13.0" to ghost/core/package.json to ensure the required
version is declared directly and avoid relying on transitive resolution; update
the dependencies block where "`@tryghost/bookshelf-plugins`" is declared and run
install to verify.
🪄 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: 95baa41c-668d-4bbb-be0b-36f3ceee9f6d

📥 Commits

Reviewing files that changed from the base of the PR and between 31dbbbd and df7ac43.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (2)
  • ghost/core/package.json
  • pnpm-workspace.yaml
💤 Files with no reviewable changes (1)
  • pnpm-workspace.yaml

Comment thread ghost/core/package.json
@kevinansfield kevinansfield enabled auto-merge (squash) June 12, 2026 17:32
@kevinansfield kevinansfield merged commit 8f22426 into main Jun 12, 2026
54 checks passed
@kevinansfield kevinansfield deleted the members-aggregate-customer-count-filter branch June 12, 2026 17:43
kevinansfield added a commit that referenced this pull request Jun 16, 2026
closes https://linear.app/ghost/issue/BER-3720/

The multiple active subscriptions filter was only reachable through the
warning banner's "View members" link and was deliberately hidden from
the filter bar, because the API only accepted the one hardcoded
`count.active_stripe_customers:>1` filter string — exposing it in the UI
would have produced unrepresentable or broken states.

With #28547 making the filter a
composable NQL field, that restriction is gone, so this makes it behave
like any other members filter:

- "Multiple active subscriptions" appears in the Subscription filter
group directly after "Member status", as a Yes/No select following the
same pattern as the comments moderation "Reported" filter. Yes
serializes to `count.active_stripe_customers:>1`; No inverts it to
`count.active_stripe_customers:<2` to find unaffected members.
- It now shows as a normal removable filter chip and can be combined
with other filters — including when arriving via the banner's "View
members" link, which previously landed on a filtered list with an
invisible filter applied.
- Following the same conditional pattern as the "Offer" filter, it's
only offered when relevant: the count-only query that drives the warning
banner is now shared (extracted to
`useMultipleActiveSubscriptionsCount`) and fetched once at page level,
and the filter stays hidden while that count is zero.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant