Skip to content

Fixed stale sidebar member counts after member mutations#28191

Open
jonatansberg wants to merge 4 commits into
mainfrom
codex/sidebar-member-count-invalidation
Open

Fixed stale sidebar member counts after member mutations#28191
jonatansberg wants to merge 4 commits into
mainfrom
codex/sidebar-member-count-invalidation

Conversation

@jonatansberg
Copy link
Copy Markdown
Member

@jonatansberg jonatansberg commented May 27, 2026

Summary

  • Fixed stale sidebar member counts by keeping member-changing flows on the existing MembersResponseType invalidation path.
  • Added a dedicated framework useMemberCount() hook that owns the lightweight /members/?limit=1 sidebar count query.
  • Added a narrow cache reconciliation path in useBrowseMembersInfinite() so a fresh unfiltered members-list total updates the existing sidebar count cache when both queries share a query client.
  • Kept filtered and searched member-list totals isolated from the sidebar count because those totals intentionally differ.

Root Cause

Some member-changing paths updated local screen state or used raw upload calls without going through the shared query invalidation mechanics. That left the sidebar's lightweight member-count query stale while the main Members screen could show a newer total.

There was also no shared owner for the sidebar count query shape, so sidebar code knew that member count was implemented as a limit=1 members browse request.

Implementation Notes

  • Member add/import/delete paths now invalidate MembersResponseType queries through the existing admin-x-framework mechanics.
  • The sidebar consumes useMemberCount() from @tryghost/admin-x-framework/api/members, keeping the limit=1 query details inside the framework API.
  • useBrowseMembersInfinite() updates only the existing sidebar count cache entry, and only when the list query is successful, unfiltered, unsearched, non-placeholder/non-previous data, and at least as fresh as the sidebar cache.
  • The self-correction does not create the sidebar query if it is absent and does not estimate count deltas locally.

Validation

  • pnpm --filter @tryghost/admin-x-framework exec eslint src/api/members.ts test/unit/api/members.test.tsx
  • pnpm exec vitest run test/unit/api/members.test.tsx --coverage=false
  • pnpm exec tsc --noEmit
  • pnpm --filter @tryghost/admin-x-framework build
  • pnpm --filter @tryghost/posts exec eslint src/views/members/members.tsx
  • pnpm exec vitest run test/unit/views/members --coverage=false
  • pnpm --filter @tryghost/admin exec eslint src/layout/app-sidebar/nav-content.tsx
  • Earlier targeted import/member-action/Ember bridge tests from the initial invalidation pass.

ref https://linear.app/tryghost/issue/BER-3510/investigate-and-fix-stale-member-count-in-sidebar

ref https://linear.app/tryghost/issue/BER-3510/investigate-and-fix-stale-member-count-in-sidebar

Member count changes can flow through several admin paths, so imports and Ember member changes now use the existing query invalidation path instead of leaving the sidebar count stale.
@jonatansberg jonatansberg changed the title [codex] Fixed stale sidebar member counts after member mutations Fixed stale sidebar member counts after member mutations May 27, 2026
@jonatansberg jonatansberg marked this pull request as ready for review May 27, 2026 13:29
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 925ce2bc-d4bf-4b78-805d-e094f552ab94

📥 Commits

Reviewing files that changed from the base of the PR and between 57d5107 and 1544451.

📒 Files selected for processing (3)
  • apps/admin-x-framework/test/unit/api/members.test.tsx
  • apps/admin/src/ember-bridge/ember-bridge.test.tsx
  • apps/posts/test/unit/views/members/import-members/modal.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/admin-x-framework/test/unit/api/members.test.tsx

Walkthrough

Adds a typed useImportMembers mutation, import types/type-guards, and a FormData builder; centralizes membersPath and member-count query helpers; introduces an internal useBrowseMembersInfiniteQuery and wraps it with useBrowseMembersInfinite to sync a sidebar member-count cache entry. Refactors ImportMembersModal to call useImportMembers.mutateAsync and handle typed responses/errors. Updates tests to cover API request shape, cache invalidation, sidebar count sync/no-sync cases, Ember bridge invalidation, modal integration, and expanded error handling.

Suggested reviewers

  • kevinansfield
  • mike182uk
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the primary fix: stale sidebar member counts are resolved by ensuring proper invalidation after member mutations.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the root cause, implementation approach, and validation steps for fixing stale sidebar member counts.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/sidebar-member-count-invalidation

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 and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f4b34b7481

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +218 to 220
if (isImportMembersAcceptedResponse(importData)) {
dispatch({type: 'UPLOAD_ACCEPTED'});
onComplete?.();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Defer count invalidation until accepted imports finish

For imports that the server accepts into the background queue, importMembers() has already invalidated MembersResponseType by the time this branch runs, but the actual member rows have not been imported yet. The active sidebar count query will refetch the old total during the PROCESSING state and then be marked fresh again, with no later invalidation when the queued job finishes, so large CSV imports still leave the sidebar count stale after completion.

Useful? React with 👍 / 👎.

ref https://linear.app/tryghost/issue/BER-3510/

The members list can receive a fresher unfiltered total than the lightweight sidebar count query, so the shared members hook now reconciles the existing sidebar cache without page-level wiring.
ref https://linear.app/tryghost/issue/BER-3510/

The sidebar should not need to know that member count is implemented with a lightweight members browse query, so the framework members API now owns the count-specific hook and cache key details.
ref https://linear.app/tryghost/issue/BER-3510/

The behavior coverage is still focused on invalidation, cache reconciliation, and import response handling, but repeated setup has been moved into small helpers so the PR carries less low-signal test diff.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1544451f57

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +202 to +206
export const useImportMembers = createMutation<ImportMembersResponseType, ImportMembersPayload>({
method: 'POST',
path: () => '/members/upload/',
body: buildImportMembersFormData,
invalidateQueries: {dataType}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Disable retries for member import uploads

When this non-idempotent CSV upload is routed through createMutation, it inherits useFetchApi's default retry = true. In production, a transient network error or lost response after the server has already accepted/processed the POST can cause the same file to be posted again, queueing or applying the import twice; the previous direct fetch did not retry. Please set this mutation's request options to retry: false.

Useful? React with 👍 / 👎.

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