Fixed Top Posts stuck loading or hiding data when web analytics is off#29103
Conversation
- useTopPostsViews is gated with `enabled: webAnalyticsEnabled` (#28713) to stop it polling Ghost when analytics is off, but the raw `isLoading` it returns was fed straight into the card's loading state - React Query v4 keeps `isLoading` true forever for a disabled query that never fetched, so with web analytics off the card never left its skeleton loading state - only report loading while the query is actually allowed to run, matching the pattern already used by useTinybirdQuery for the same enabled/disabled split
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughChangesThe admin-x-framework query hook factories now gate Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 10m 41s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 1s | View ↗ |
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 33s | View ↗ |
nx run-many -t test:unit -p @tryghost/admin-x-f... |
✅ Succeeded | 6m 49s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 55s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 4m 16s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 3m 1s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 32s | View ↗ |
Additional runs (5) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-04 19:39:14 UTC
…ries - the previous commit worked around React Query v4's stuck-`isLoading` behavior only at the Top Posts call site, but every hook built on createQuery/createPaginatedQuery/createInfiniteQuery in admin-x-framework has the same issue for any `enabled: false` query that never fetches - fixed it once in the three shared factories instead: `isLoading` is now gated on `fetchStatus !== 'idle'`, so a disabled query correctly reports not-loading instead of loading-forever - reverted the local workaround in the Stats Overview page now that the underlying hook behaves correctly
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
9larsons
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/stats/src/views/Stats/Overview/overview.tsx (1)
78-78: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse
isInitialLoadingfor the top-posts skeleton
useTopPostsViewsalready exposesisInitialLoading; using that here avoids the manualwebAnalyticsEnabled &&gate while keeping the disabled-query loading state correct.🤖 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 `@apps/stats/src/views/Stats/Overview/overview.tsx` at line 78, The top-posts skeleton loading state in Overview is gating on a manual webAnalyticsEnabled check instead of using the query’s built-in initial loading state. Update the logic around useTopPostsViews in Overview so it relies on isInitialLoading from the hook, and remove the extra manual enabled-condition check for the skeleton visibility.
🤖 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 `@apps/stats/src/views/Stats/Overview/overview.tsx`:
- Line 78: The top-posts skeleton loading state in Overview is gating on a
manual webAnalyticsEnabled check instead of using the query’s built-in initial
loading state. Update the logic around useTopPostsViews in Overview so it relies
on isInitialLoading from the hook, and remove the extra manual enabled-condition
check for the skeleton visibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b2c369ae-aae8-4edf-931d-c4ba6d5b0098
📒 Files selected for processing (1)
apps/stats/src/views/Stats/Overview/overview.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/admin-x-framework/src/utils/api/hooks.ts (1)
62-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the duplicated fetch-gate logic.
The same comment and one-line computation is repeated verbatim across all three factories. A small shared helper (e.g.
isQueryLoading(result)) would avoid drift if this logic needs to change again.♻️ Suggested helper
+const isQueryLoading = (result: {isLoading: boolean; fetchStatus: string}) => + result.isLoading && result.fetchStatus !== 'idle'; + export const createQuery = ... return { ...result, data, - isLoading: result.isLoading && result.fetchStatus !== 'idle' + isLoading: isQueryLoading(result) };Also applies to: 108-115, 152-158
🤖 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 `@apps/admin-x-framework/src/utils/api/hooks.ts` around lines 62 - 68, The fetch-status gating logic for `isLoading` is duplicated across the query factories, so extract it into a shared helper such as `isQueryLoading(result)` and use that helper in each factory that returns `{ ...result, data, isLoading }`. Keep the current behavior exactly the same by centralizing the `result.isLoading && result.fetchStatus !== 'idle'` check in one place, so the implementations in the affected hook factories stay consistent and easier to update.
🤖 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 `@apps/admin-x-framework/src/utils/api/hooks.ts`:
- Around line 62-68: The fetch-status gating logic for `isLoading` is duplicated
across the query factories, so extract it into a shared helper such as
`isQueryLoading(result)` and use that helper in each factory that returns `{
...result, data, isLoading }`. Keep the current behavior exactly the same by
centralizing the `result.isLoading && result.fetchStatus !== 'idle'` check in
one place, so the implementations in the affected hook factories stay consistent
and easier to update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea6768fe-8ce5-44d9-999b-cc7c03567ac4
📒 Files selected for processing (1)
apps/admin-x-framework/src/utils/api/hooks.ts
- useTopPostsViews was gated behind `enabled: webAnalyticsEnabled`, so the whole request (and all the Ghost-DB-backed data it returns: authors, sent/ opened counts, member attribution, clicks) was skipped whenever web analytics was disabled or unconfigured, not just the view counts - getTopPostsViews already degrades gracefully without a tinybirdClient (posts-stats-service.js) - it still returns recently published posts from Ghost's own DB, it just can't attach real view numbers - the browser never talks to Tinybird directly for this endpoint, so gating the fetch bought no protection against Tinybird load; it only threw away data Ghost could serve on its own - removed the gate so the query always runs; the views metric itself is still correctly hidden via showWebAnalytics in the TopPosts component
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
9larsons
changed the title
no ref - postsWithViews is built by mapping over viewsData, so whenever viewsData is empty (no tinybirdClient, or Tinybird returned nothing for the range) the posts/authors/attribution/click queries feeding that lookup were run for nothing - the result was always going to be [] - skip that block entirely when viewsData is empty and go straight to the recency-based backfill query, which already covers this case - no behavior change: same tests (posts.test.js) pass unmodified
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
no ref - trimmed comments that restated what the diff already makes clear
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
9larsons
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #29103 +/- ##
==========================================
- Coverage 74.30% 74.30% -0.01%
==========================================
Files 1565 1565
Lines 135712 135716 +4
Branches 16463 16461 -2
==========================================
- Hits 100841 100838 -3
- Misses 33842 33848 +6
- Partials 1029 1030 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Summary
createQuery/createPaginatedQuery/createInfiniteQueryinadmin-x-frameworknow deriveisLoadingfromfetchStatus !== 'idle'instead of passing React Query's rawisLoadingthrough. A disabled query that never fetches otherwise reportsisLoading: trueforever, sincestatusonly leaves'loading'on an actual fetch dispatch.enabled: webAnalyticsEnabledfromuseTopPostsViewsin the Stats Overview page.getTopPostsViewsalready returns real DB-backed data (posts, authors, sent/opened counts, member attribution, clicks) when Tinybird isn't configured — gating the fetch discarded all of that, not just view counts. The views metric stays hidden via the existingshowWebAnalyticscheck inTopPosts.getTopPostsViewsthat ran unconditionally but was always discarded when there was no views data to enrich.Test plan
apps/admin-x-frameworkunit tests pass (161 tests)apps/statsunit tests pass (234 tests)ghost/coretest/unit/server/services/stats/posts.test.jspasses unmodified (46 tests)eslintclean on all changed files