-
Notifications
You must be signed in to change notification settings - Fork 2k
Stats post detail: prefetch email tab availability and seed it from the email pages #113931
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Changes from 2 commits
7268215
51db32c
c264fc4
111b4d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,10 +6,13 @@ import AsyncLoad from 'calypso/components/async-load'; | |
| import { bumpStat } from 'calypso/lib/analytics/mc'; | ||
| import { getSiteFragment, getStatsDefaultSitePage } from 'calypso/lib/route'; | ||
| import { getMomentSiteZone } from 'calypso/my-sites/stats/hooks/use-moment-site-zone'; | ||
| import { getSite } from 'calypso/state/sites/selectors'; | ||
| import isJetpackModuleActive from 'calypso/state/selectors/is-jetpack-module-active'; | ||
| import { getSite, isSimpleSite } from 'calypso/state/sites/selectors'; | ||
| import getEnvStatsFeatureSupportChecks from 'calypso/state/sites/selectors/get-env-stats-feature-supports'; | ||
| import { setNextLayoutFocus } from 'calypso/state/ui/layout-focus/actions'; | ||
| import { getCurrentLayoutFocus } from 'calypso/state/ui/layout-focus/selectors'; | ||
| import { getSelectedSiteId } from 'calypso/state/ui/selectors'; | ||
| import { postEmailStatsAvailabilityQueryOptions } from './hooks/use-post-email-stats-availability-query'; | ||
| import { rangeOfPeriod, getSiteFilters } from './pages/shared/helpers'; | ||
| import PageLoading from './pages/shared/page-loading'; | ||
| import StatsSite from './site'; | ||
|
|
@@ -416,6 +419,21 @@ export function post( context, next ) { | |
| return next(); | ||
| } | ||
|
|
||
| // Resolve email tab availability in parallel with the chunk download, so the | ||
| // tab strip doesn't pop in after the page renders. | ||
| const state = context.store.getState(); | ||
| const { supportsEmailStats } = getEnvStatsFeatureSupportChecks( state, siteId ); | ||
| // Module state is often not loaded yet at route time (QueryJetpackModules runs | ||
| // after mount, and /me/sites does not carry active_modules), so only a definite | ||
| // "inactive" skips the warm-up; the page's own `enabled` check stays strict. | ||
| const canHaveEmailStats = | ||
| !! supportsEmailStats && | ||
| ( isSimpleSite( state, siteId ) || | ||
| isJetpackModuleActive( state, siteId, 'subscriptions', true ) !== false ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment slightly overpromises here I think - with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in c264fc4 — the gate now reads the module store directly first and only falls back when that is null, so a definite inactive skips the prefetch; comment updated to match. |
||
| if ( canHaveEmailStats && postId > 0 ) { | ||
| context.queryClient.prefetchQuery( postEmailStatsAvailabilityQueryOptions( siteId, postId ) ); | ||
| } | ||
|
|
||
| context.primary = ( | ||
| <StatsPageLoader> | ||
| <AsyncLoad | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||||||||
| import { useQuery } from '@tanstack/react-query'; | ||||||||||||||
| import { queryOptions, useQuery, type QueryClient } from '@tanstack/react-query'; | ||||||||||||||
| import wpcom from 'calypso/lib/wp'; | ||||||||||||||
| import getDefaultQueryParams from './default-query-params'; | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -15,20 +15,11 @@ function queryEmailRate( siteId: number | null, postId: number ): Promise< Email | |||||||||||||
| return wpcom.req.get( `/sites/${ siteId }/stats/opens/emails/${ postId }/rate` ); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Whether a post was ever sent as a newsletter email, based on the email stats themselves | ||||||||||||||
| * rather than post metadata, which does not always reflect what was actually sent. | ||||||||||||||
| */ | ||||||||||||||
| export default function usePostEmailStatsAvailabilityQuery( | ||||||||||||||
| siteId: number | null, | ||||||||||||||
| postId: number, | ||||||||||||||
| enabled = true | ||||||||||||||
| ) { | ||||||||||||||
| return useQuery( { | ||||||||||||||
| export function postEmailStatsAvailabilityQueryOptions( siteId: number | null, postId: number ) { | ||||||||||||||
| return queryOptions( { | ||||||||||||||
| ...getDefaultQueryParams(), | ||||||||||||||
| queryKey: [ 'stats', 'emails', 'opens', 'rate', siteId, postId ], | ||||||||||||||
| queryFn: () => queryEmailRate( siteId, postId ), | ||||||||||||||
| enabled: !! enabled && !! siteId && postId > 0, | ||||||||||||||
| // A "no email stats" answer can be transient while a newsletter is still being sent, | ||||||||||||||
| // so only a positive result is kept for a while. | ||||||||||||||
| staleTime: ( query ) => ( hasEmailStats( query.state.data ) ? 1000 * 60 * 5 : 1000 * 30 ), | ||||||||||||||
|
|
@@ -37,6 +28,37 @@ export default function usePostEmailStatsAvailabilityQuery( | |||||||||||||
| // (the shared defaults set retryOnMount: false). | ||||||||||||||
| retryOnMount: true, | ||||||||||||||
| meta: { persist: false }, | ||||||||||||||
| } ); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Mark a post as having email stats without waiting for the request, for navigations | ||||||||||||||
| * from a page that already proves they exist (e.g. the email detail tabs). The counts | ||||||||||||||
| * are a placeholder; nothing reads them besides hasEmailStats. Real responses win. | ||||||||||||||
| */ | ||||||||||||||
| export function seedPostEmailStatsAvailability( | ||||||||||||||
| queryClient: QueryClient, | ||||||||||||||
| siteId: number | null, | ||||||||||||||
| postId: number | ||||||||||||||
| ) { | ||||||||||||||
| const { queryKey } = postEmailStatsAvailabilityQueryOptions( siteId, postId ); | ||||||||||||||
| if ( queryClient.getQueryData( queryKey ) === undefined ) { | ||||||||||||||
| queryClient.setQueryData( queryKey, { total_sends: 1 } ); | ||||||||||||||
| } | ||||||||||||||
|
dognose24 marked this conversation as resolved.
Comment on lines
+54
to
+57
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I managed to seed a false positive here - open the Email opens URL directly for a post that was never emailed (that page always renders the tabs), click Post traffic, and the post detail shows the email tabs for a post with no email stats. And because the seed is written as fresh data, its 5-min staleTime suppresses the I think seeding as already-stale data fixes both this and the cached-negative case Copilot flagged:
Suggested change
The strip still renders instantly from the cached value, but the query is stale so the mount refetches and the truth wins within one round trip - and that makes it safe to overwrite a stale negative too. It does bring the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great catch, and the stale-seed idea is exactly right — took it in c264fc4 with one refinement: a cached entry that already satisfies |
||||||||||||||
| } | ||||||||||||||
|
Copilot marked this conversation as resolved.
|
||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Whether a post was ever sent as a newsletter email, based on the email stats themselves | ||||||||||||||
| * rather than post metadata, which does not always reflect what was actually sent. | ||||||||||||||
| */ | ||||||||||||||
| export default function usePostEmailStatsAvailabilityQuery( | ||||||||||||||
| siteId: number | null, | ||||||||||||||
| postId: number, | ||||||||||||||
| enabled = true | ||||||||||||||
| ) { | ||||||||||||||
| return useQuery( { | ||||||||||||||
| ...postEmailStatsAvailabilityQueryOptions( siteId, postId ), | ||||||||||||||
| enabled: !! enabled && !! siteId && postId > 0, | ||||||||||||||
| select: hasEmailStats, | ||||||||||||||
| } ); | ||||||||||||||
| } | ||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.