-
-
Notifications
You must be signed in to change notification settings - Fork 11.1k
Added UTM dropdowns to Analytics Growth pages #25104
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: main
Are you sure you want to change the base?
Conversation
- Added campaign dropdown to stats and posts Growth views - Integrated UTM data fetching with useUtmGrowthStats hook - Campaign values display without icons/links (they're identifiers, not domains) - Fixed DOM nesting error by removing loading skeleton from GrowthSources - Campaign dropdown appears selected when active
WalkthroughAdds UTM growth support: new types ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (1)
235-257
: Restore loading state while UTM data is fetchingWhen a campaign is selected we return
[]
whileuseUtmGrowthStats
is still fetching, so this branch hits the “No conversions” empty state even though the request is in-flight. That’s misleading and causes a visible flicker before data arrives. Please gate the render onisUtmFetching
(e.g. compute anisLoading
flag and show a skeleton/table loader) before falling back to the empty indicator.-import {Button, EmptyIndicator, GrowthCampaignType, LucideIcon, Sheet, SheetContent, SheetDescription, SheetHeader, SheetTitle, SheetTrigger, Table, TableBody, TableCell, TableFooter, TableHead, TableHeader, TableRow, centsToDollars, formatNumber} from '@tryghost/shade'; +import {Button, EmptyIndicator, GrowthCampaignType, LucideIcon, Sheet, SheetContent, SheetDescription, SheetHeader, SheetTitle, SheetTrigger, SkeletonTable, Table, TableBody, TableCell, TableFooter, TableHead, TableHeader, TableRow, centsToDollars, formatNumber} from '@tryghost/shade'; … - const {data: utmData, isFetching: isUtmFetching} = useUtmGrowthStats({ + const {data: utmData, isFetching: isUtmFetching} = useUtmGrowthStats({ searchParams: { utm_type: utmType, limit: String(limit), order: backendOrderBy }, enabled: !!selectedCampaign }); … + const isLoading = selectedCampaign ? isUtmFetching : !referrersData?.stats; … - {processedData.length > 0 ? ( + {isLoading ? ( + <TableBody> + <TableRow className='last:border-none'> + <TableCell className='border-none py-6' colSpan={appSettings?.paidMembersEnabled ? 4 : 2}> + <SkeletonTable lines={Math.min(limit, 10)} /> + </TableCell> + </TableRow> + </TableBody> + ) : processedData.length > 0 ? ( <GrowthSourcesTableBody currencySymbol={currencySymbol} data={processedData} defaultSourceIconUrl={defaultSourceIconUrl} limit={limit} /> ) : (
🧹 Nitpick comments (4)
apps/stats/src/views/Stats/components/SourceIcon.tsx (1)
11-29
: Preserve newsletter icon; only suppress non-newsletter with empty iconSrc. Also guard onError loop.Move the newsletter check before the empty-src guard; otherwise a “Newsletter” with no iconSrc renders nothing. Add a loop guard to the fallback.
- // Don't render an icon if iconSrc is empty (e.g., for UTM campaign values) - if (!iconSrc) { - return null; - } - - return ( - <> - {displayName.trim().toLowerCase().endsWith('newsletter') ? ( - <LucideIcon.Mail aria-label="Newsletter" className="size-4 text-muted-foreground" /> - ) : ( - <img - alt="" - className="size-4" - src={iconSrc} - onError={(e: React.SyntheticEvent<HTMLImageElement>) => { - e.currentTarget.src = defaultSourceIconUrl; - }} - /> - )} - </> - ); + const isNewsletter = displayName.trim().toLowerCase().endsWith('newsletter'); + if (isNewsletter) { + return <LucideIcon.Mail aria-label="Newsletter" className="size-4 text-muted-foreground" />; + } + // Don't render an icon if iconSrc is empty (e.g., for UTM campaign values) + if (!iconSrc) { + return null; + } + + return ( + <img + alt="" + className="size-4" + src={iconSrc} + onError={(e: React.SyntheticEvent<HTMLImageElement>) => { + if (e.currentTarget.src !== defaultSourceIconUrl) { + e.currentTarget.onerror = null; + e.currentTarget.src = defaultSourceIconUrl; + } + }} + /> + );apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (1)
1-3
: Use @ alias for internal imports (shade guideline).Switch to the @ alias for intra-package imports.
As per coding guidelines
-import {TableFilterDropdownTab, TableFilterTab, TableFilterTabs} from '../table-filter-tabs/table-filter-tabs'; +import {TableFilterDropdownTab, TableFilterTab, TableFilterTabs} from '@/components/features/table-filter-tabs/table-filter-tabs';apps/stats/src/views/Stats/Growth/Growth.tsx (2)
41-47
: Avoid duplicating campaign options; derive from shared constant.Reuse the exported GROWTH_CAMPAIGN_TYPES to keep options in sync.
-const GROWTH_CAMPAIGN_OPTIONS = [ - {value: 'UTM sources', label: 'UTM sources'}, - {value: 'UTM mediums', label: 'UTM mediums'}, - {value: 'UTM campaigns', label: 'UTM campaigns'}, - {value: 'UTM contents', label: 'UTM contents'}, - {value: 'UTM terms', label: 'UTM terms'} -]; +import {GROWTH_CAMPAIGN_TYPES} from '@tryghost/shade'; +const GROWTH_CAMPAIGN_OPTIONS = GROWTH_CAMPAIGN_TYPES.map(v => ({value: v, label: v}));
175-199
: Prefer UtmGrowthTabs to encapsulate tab/dropdown behavior and reduce drift.Replace the custom Tabs + TableFilterDropdownTab block with the shared UtmGrowthTabs for consistent UX and built-in guards.
- Using UtmGrowthTabs removes the need to manually clear selectedCampaign and aligns behavior across apps.
- If keeping the custom Tabs, please confirm TableFilterDropdownTab is designed to be used within generic Tabs, not only TableFilterTabs.
Also applies to: 187-197
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/admin-x-framework/src/api/stats.ts
(3 hunks)apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx
(3 hunks)apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx
(1 hunks)apps/shade/src/index.ts
(1 hunks)apps/stats/src/views/Stats/Growth/Growth.tsx
(4 hunks)apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx
(5 hunks)apps/stats/src/views/Stats/components/SourceIcon.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/shade/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
Use camelCase for functions and variables
Files:
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx
apps/shade/src/index.ts
apps/shade/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
apps/shade/src/components/**/*.{ts,tsx}
: Use CVA (class-variance-authority) for variants where useful
Prefer composable components and compound subcomponents (e.g., Header.Title, Header.Meta, Header.Actions) over heavy prop configs; attach parts as static properties and export as named exports
Files:
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx
apps/shade/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
Use the @ alias for internal imports (e.g., @/lib/utils)
Files:
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx
apps/shade/src/index.ts
apps/shade/src/index.ts
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
Export new UI components from src/index.ts
Files:
apps/shade/src/index.ts
🧠 Learnings (1)
📚 Learning: 2025-09-11T11:56:37.466Z
Learnt from: CR
PR: TryGhost/Ghost#0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-09-11T11:56:37.466Z
Learning: Applies to apps/shade/src/index.ts : Export new UI components from src/index.ts
Applied to files:
apps/shade/src/index.ts
🧬 Code graph analysis (5)
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (2)
apps/shade/src/index.ts (2)
GrowthCampaignType
(53-53)GrowthTabType
(53-53)apps/shade/src/components/features/table-filter-tabs/table-filter-tabs.tsx (3)
TableFilterTabs
(93-93)TableFilterTab
(91-91)TableFilterDropdownTab
(92-92)
apps/admin-x-framework/src/api/stats.ts (1)
apps/admin-x-framework/src/utils/api/hooks.ts (2)
Meta
(9-18)createQuery
(35-61)
apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (5)
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (1)
GrowthCampaignType
(4-4)apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx (1)
GrowthSources
(97-306)apps/admin-x-framework/src/api/referrers.ts (1)
useTopSourcesGrowth
(22-25)apps/stats/src/hooks/useTopSourcesGrowth.ts (1)
useTopSourcesGrowth
(6-23)apps/admin-x-framework/src/api/stats.ts (1)
useUtmGrowthStats
(249-252)
apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx (3)
apps/admin-x-framework/src/index.ts (1)
useParams
(40-40)apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (3)
GrowthTabType
(5-5)GrowthCampaignType
(4-4)UtmGrowthTabs
(28-69)apps/admin-x-framework/src/api/stats.ts (1)
useUtmGrowthStats
(249-252)
apps/stats/src/views/Stats/Growth/Growth.tsx (4)
apps/stats/src/utils/content-helpers.ts (2)
ContentType
(9-9)CONTENT_TYPES
(2-7)apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (1)
GrowthCampaignType
(4-4)apps/shade/src/components/features/table-filter-tabs/table-filter-tabs.tsx (1)
TableFilterDropdownTab
(92-92)apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (1)
GrowthSources
(88-314)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: ActivityPub tests
- GitHub Check: Admin-X Settings tests
- GitHub Check: Unit tests (Node 22.13.1)
🔇 Additional comments (4)
apps/admin-x-framework/src/api/stats.ts (2)
83-95
: UTM growth stats types look good.Shapes align with other stats payloads; no concerns.
218-253
: Hook wiring for /stats/utm-growth/ looks correct.Consistent dataType keying and path usage.
apps/shade/src/index.ts (1)
51-54
: Exports for UtmGrowthTabs and its types are correctly added.Public surface updated as expected.
apps/stats/src/views/Stats/Growth/Growth.tsx (1)
226-235
: Confirm UTM sorting parameter mapping in GrowthSources.Cross-file note: In GrowthSources (apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx), the same backendOrderBy (with 'free_members'→'signups', 'paid_members'→'paid_conversions') is passed to useUtmGrowthStats. The UTM endpoint returns free_members/paid_members/mrr (see apps/admin-x-framework/src/api/stats.ts lines 83-95). It likely expects those field names for ordering, not 'signups'/'paid_conversions'. Please verify and use the unmodified sort for UTM.
Reference: apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx lines around “backendOrderBy” creation and useUtmGrowthStats searchParams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/stats/src/views/Stats/Growth/Growth.tsx (1)
41-47
: Consider extracting campaign logic into a custom hook.The campaign selection logic (state, options, and tab integration) could be extracted into a custom hook like
useGrowthCampaigns()
to improve component organization and reusability.Example extraction:
// hooks/useGrowthCampaigns.ts export function useGrowthCampaigns(utmTrackingEnabled: boolean) { const [selectedCampaign, setSelectedCampaign] = useState<GrowthCampaignType>(''); const clearCampaign = () => setSelectedCampaign(''); return { selectedCampaign, setSelectedCampaign, clearCampaign, isEnabled: utmTrackingEnabled, options: GROWTH_CAMPAIGN_OPTIONS }; }Then in the component:
const campaignState = useGrowthCampaigns(utmTrackingEnabled);Also applies to: 54-54, 175-197
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/stats/src/views/Stats/Growth/Growth.tsx
(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/stats/src/views/Stats/Growth/Growth.tsx (4)
apps/stats/src/utils/content-helpers.ts (2)
ContentType
(9-9)CONTENT_TYPES
(2-7)apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (1)
GrowthCampaignType
(4-4)apps/shade/src/components/features/table-filter-tabs/table-filter-tabs.tsx (1)
TableFilterDropdownTab
(92-92)apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (1)
GrowthSources
(88-314)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Admin-X Settings tests
🔇 Additional comments (5)
apps/stats/src/views/Stats/Growth/Growth.tsx (5)
41-47
: LGTM!The campaign options correctly match the
GrowthCampaignType
definition from shade and provide clear labels for the dropdown.
50-50
: LGTM!The state initialization and labs flag check are implemented correctly with proper optional chaining and type-safe defaults.
Also applies to: 54-54, 58-59
187-197
: LGTM!The campaign dropdown is properly gated behind the UTM tracking feature flag and correctly wired to the component state.
226-226
: LGTM!The conditional rendering logic correctly shows
GrowthSources
for both the Sources tab and campaign selections, and properly passes the campaign context through props.Also applies to: 230-230
175-180
: No changes needed for “campaigns” tab integration. TableFilterDropdownTab uses TabsDropdownTrigger internally, correctly registering the “campaigns” tab.Likely an incorrect or invalid review comment.
9larsons commented: |
9larsons commented: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/admin-x-framework/src/api/stats.ts (1)
96-109
: Stronger typing for getUtmType return (avoid casts and magic strings).Consider returning a precise union and removing the cast for safer usage.
Apply this diff:
+export type UtmParamType = '' | 'utm_source' | 'utm_medium' | 'utm_campaign' | 'utm_content' | 'utm_term'; + -export const getUtmType = (campaign: GrowthCampaignType): string => { - return campaign ? UTM_TYPE_MAP[campaign as Exclude<GrowthCampaignType, ''>] || '' : ''; -}; +export const getUtmType = (campaign: GrowthCampaignType): UtmParamType => { + return campaign ? UTM_TYPE_MAP[campaign as Exclude<GrowthCampaignType, ''>] ?? '' : ''; +};apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (3)
2-2
: Use @ alias for internal imports.Switch to the @ alias for internal modules to follow repo conventions.
As per coding guidelines
-import {TableFilterDropdownTab, TableFilterTab, TableFilterTabs} from '../table-filter-tabs/table-filter-tabs'; +import {TableFilterDropdownTab, TableFilterTab, TableFilterTabs} from '@/components/features/table-filter-tabs/table-filter-tabs';
8-15
: Tighten type usage for campaign types array.Reuse the already imported GrowthCampaignType to simplify the satisfies clause.
-export const GROWTH_CAMPAIGN_TYPES = [ +export const GROWTH_CAMPAIGN_TYPES = [ 'UTM sources', 'UTM mediums', 'UTM campaigns', 'UTM contents', 'UTM terms' -] as const satisfies readonly Exclude<import('@tryghost/admin-x-framework/api/stats').GrowthCampaignType, ''>[]; +] as const satisfies readonly Exclude<GrowthCampaignType, ''>[];
36-52
: Type handler params to avoid casts.Use GrowthTabType and GrowthCampaignType directly.
-const handleTabChange = (tab: string) => { +const handleTabChange = (tab: GrowthTabType | string) => { // Prevent switching to campaigns without selection if (tab === 'campaigns' && !selectedCampaign) { return; } - onTabChange(tab as GrowthTabType); + onTabChange(tab as GrowthTabType);-const handleCampaignChange = (campaign: string) => { - onCampaignChange(campaign as GrowthCampaignType); +const handleCampaignChange = (campaign: GrowthCampaignType | string) => { + onCampaignChange(campaign as GrowthCampaignType); onTabChange('campaigns'); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/admin-x-framework/src/api/stats.ts
(3 hunks)apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx
(3 hunks)apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx
(1 hunks)apps/stats/src/views/Stats/Growth/Growth.tsx
(4 hunks)apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/shade/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
Use camelCase for functions and variables
Files:
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx
apps/shade/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
apps/shade/src/components/**/*.{ts,tsx}
: Use CVA (class-variance-authority) for variants where useful
Prefer composable components and compound subcomponents (e.g., Header.Title, Header.Meta, Header.Actions) over heavy prop configs; attach parts as static properties and export as named exports
Files:
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx
apps/shade/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/shade/AGENTS.md)
Use the @ alias for internal imports (e.g., @/lib/utils)
Files:
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx
🧬 Code graph analysis (5)
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (3)
apps/shade/src/index.ts (2)
GrowthTabType
(53-53)GrowthCampaignType
(53-53)apps/admin-x-framework/src/api/stats.ts (1)
GrowthCampaignType
(96-96)apps/shade/src/components/features/table-filter-tabs/table-filter-tabs.tsx (3)
TableFilterTabs
(93-93)TableFilterTab
(91-91)TableFilterDropdownTab
(92-92)
apps/admin-x-framework/src/api/stats.ts (3)
apps/admin-x-framework/src/utils/api/hooks.ts (2)
Meta
(9-18)createQuery
(35-61)apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (1)
GrowthCampaignType
(5-5)apps/shade/src/index.ts (1)
GrowthCampaignType
(53-53)
apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (4)
apps/admin-x-framework/src/api/stats.ts (3)
GrowthCampaignType
(96-96)getUtmType
(106-108)useUtmGrowthStats
(263-266)apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx (1)
GrowthSources
(100-299)apps/stats/src/hooks/useTopSourcesGrowth.ts (1)
useTopSourcesGrowth
(6-23)apps/admin-x-framework/src/api/referrers.ts (1)
useTopSourcesGrowth
(22-25)
apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx (3)
apps/admin-x-framework/src/index.ts (1)
useParams
(40-40)apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (3)
GrowthTabType
(6-6)GrowthCampaignType
(5-5)UtmGrowthTabs
(29-70)apps/admin-x-framework/src/api/stats.ts (3)
GrowthCampaignType
(96-96)getUtmType
(106-108)useUtmGrowthStats
(263-266)
apps/stats/src/views/Stats/Growth/Growth.tsx (4)
apps/stats/src/utils/content-helpers.ts (2)
ContentType
(9-9)CONTENT_TYPES
(2-7)apps/admin-x-framework/src/api/stats.ts (1)
GrowthCampaignType
(96-96)apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (2)
GrowthCampaignType
(5-5)GROWTH_CAMPAIGN_OPTIONS
(16-19)apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (1)
GrowthSources
(88-304)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ActivityPub tests
- GitHub Check: Admin-X Settings tests
🔇 Additional comments (6)
apps/admin-x-framework/src/api/stats.ts (1)
263-266
: UTM growth hook wiring looks good.Endpoint, dataType, and generic align with the new types.
apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (2)
112-121
: UTM query missing date range/timezone/audience — could desync from selected range.Top sources growth uses date_from/date_to/member_status/timezone; UTMs fetch doesn’t. Likely returns defaults that don’t match the current range.
Please confirm the /stats/utm-growth/ API defaults. If not aligned, pass the same query params you use for sources (date_from/date_to/member_status/timezone, plus order/limit). I can draft a patch once you confirm the expected params.
188-191
: Mapping looks correct (signups → free_members, paid_conversions → paid_members).Good alignment with backend sort keys.
apps/stats/src/views/Stats/Growth/Growth.tsx (1)
218-226
: GrowthSources integration looks correct.Passing selectedCampaign to control UTMs vs sources rendering is aligned.
apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx (2)
128-135
: Verify UTMs query params match the selected range/timezone.Only utm_type and post_id are sent. If the endpoint expects date_from/date_to (and optionally timezone), results may not reflect the current range.
If needed, add date_from/date_to (and timezone) aligned with the page’s range to keep UTMs consistent with other stats. I can draft the patch once you confirm the API shape.
217-229
: Nice integration of UtmGrowthTabs and loading state.Clear UX: tabs, dropdown, separator, and skeleton while loading.
// If a campaign is selected, only show UTM data (not regular sources) | ||
if (selectedCampaign) { | ||
// If we have UTM data and we're not fetching, show it | ||
if (utmData?.stats && !isUtmFetching) { | ||
// UTM values are campaign identifiers, not domains - don't show icons or links | ||
return utmData.stats.map((item) => { | ||
const source = item.utm_value || '(not set)'; | ||
return { | ||
source, | ||
free_members: item.free_members, | ||
paid_members: item.paid_members, | ||
mrr: item.mrr, | ||
iconSrc: '', | ||
displayName: source, | ||
linkUrl: undefined | ||
}; | ||
}); | ||
} | ||
// If fetching or no data yet, return empty array (don't show regular sources) | ||
return []; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid empty-state flicker while UTM data is loading.
Currently returns [] during isUtmFetching, showing “No conversions” momentarily.
Consider rendering a lightweight loading row or SkeletonTable when selectedCampaign && isUtmFetching, and only show empty state after loading completes.
I can provide a patch that adds a loading branch if preferred.
🤖 Prompt for AI Agents
In apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx around lines
149 to 169, when selectedCampaign && isUtmFetching the code returns an empty
array which causes a transient “No conversions” flicker; change this branch to
return a loading placeholder instead (either a single lightweight row object
with placeholders and an isLoading flag the table can detect, or trigger the
existing SkeletonTable/loading component by returning a special value such as
null or a loading prop), so that while utmData is being fetched the UI shows a
skeleton/loading row and only shows the empty state after fetching completes.
<Tabs value={selectedCampaign ? 'campaigns' : selectedContentType} variant='button-sm' onValueChange={(value: string) => { | ||
setSelectedContentType(value as ContentType); | ||
// Clear campaign selection when switching away from campaigns | ||
if (value !== 'campaigns') { | ||
setSelectedCampaign(''); | ||
} | ||
}}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: setting content type to 'campaigns' causes invalid post_type in data fetch.
Don’t set selectedContentType when switching to campaigns.
Apply this diff:
-<Tabs value={selectedCampaign ? 'campaigns' : selectedContentType} variant='button-sm' onValueChange={(value: string) => {
- setSelectedContentType(value as ContentType);
- // Clear campaign selection when switching away from campaigns
- if (value !== 'campaigns') {
- setSelectedCampaign('');
- }
-}}>
+<Tabs value={selectedCampaign ? 'campaigns' : selectedContentType} variant='button-sm' onValueChange={(value: string) => {
+ // Only update content type for content tabs
+ if (value !== 'campaigns') {
+ setSelectedContentType(value as ContentType);
+ setSelectedCampaign('');
+ }
+}}>
Optionally, consider using the shared UtmGrowthTabs component for consistency with Posts UI.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/stats/src/views/Stats/Growth/Growth.tsx around lines 167-173, the
onValueChange handler incorrectly sets selectedContentType to 'campaigns', which
produces an invalid post_type in the data fetch; change the handler so it does
NOT call setSelectedContentType when value === 'campaigns' (instead
setSelectedCampaign or handle campaign selection there), and keep the existing
logic that clears selectedCampaign when switching away from campaigns (i.e.,
only call setSelectedContentType(value as ContentType) when value !==
'campaigns').
Hm yeah I'm getting the same build error locally. but not on |
9larsons commented: |
9larsons commented: |
9larsons commented: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (1)
150-169
: Prevent UTM campaign loading flickerWhen
selectedCampaign
is set we return[]
untilisUtmFetching
settles, so the table renders the “No conversions” empty state between fetches. Keep showing a loading row (or reuse the existing skeleton) while UTMs are fetching instead of an empty array.if (selectedCampaign) { - if (utmData?.stats && !isUtmFetching) { + if (isUtmFetching) { + return loadingRows; + } + if (utmData?.stats) { return utmData.stats.map((item) => { const source = item.utm_value || '(not set)'; return {Populate
loadingRows
with a lightweight placeholder shape for the table (or leverage an existing skeleton hook) so the empty state only appears after loading completes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/admin-x-framework/src/api/stats.ts
(3 hunks)apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx
(3 hunks)apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx
(1 hunks)apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
apps/admin-x-framework/src/api/stats.ts (1)
apps/admin-x-framework/src/utils/api/hooks.ts (2)
Meta
(9-18)createQuery
(35-61)
apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (4)
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (2)
GrowthCampaignType
(4-4)getUtmType
(23-25)apps/admin-x-framework/src/api/referrers.ts (1)
useTopSourcesGrowth
(22-25)apps/stats/src/hooks/useTopSourcesGrowth.ts (1)
useTopSourcesGrowth
(6-23)apps/admin-x-framework/src/api/stats.ts (1)
useUtmGrowthStats
(249-252)
apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx (3)
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (4)
GrowthTabType
(5-5)GrowthCampaignType
(4-4)getUtmType
(23-25)UtmGrowthTabs
(40-81)apps/admin-x-framework/src/api/stats.ts (1)
useUtmGrowthStats
(249-252)apps/posts/src/views/PostAnalytics/Web/components/Sources.tsx (1)
SourcesTable
(19-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: ActivityPub tests
- GitHub Check: Admin-X Settings tests
- GitHub Check: Unit tests (Node 22.13.1)
- GitHub Check: Build & Push
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (1)
152-169
: Still showing an empty-state flicker while UTM data loadsWhen a campaign is selected and the UTM query is in-flight,
processedData
becomes[]
, so the table briefly renders “No conversions” before the results arrive—the same issue already noted earlier. Gate the render behind an explicit loading branch (e.g. deriveisUtmLoading = selectedCampaign && isUtmFetching && !utmData?.stats
and show a skeleton row) instead of returning an empty array during fetch.-import {Button, EmptyIndicator, GrowthCampaignType, LucideIcon, Sheet, SheetContent, SheetDescription, SheetHeader, SheetTitle, SheetTrigger, Table, TableBody, TableCell, TableFooter, TableHead, TableHeader, TableRow, centsToDollars, formatNumber, getUtmType} from '@tryghost/shade'; +import {Button, EmptyIndicator, GrowthCampaignType, LucideIcon, Sheet, SheetContent, SheetDescription, SheetHeader, SheetTitle, SheetTrigger, SkeletonTable, Table, TableBody, TableCell, TableFooter, TableHead, TableHeader, TableRow, centsToDollars, formatNumber, getUtmType} from '@tryghost/shade'; … - const processedData = React.useMemo((): ProcessedReferrerData[] => { + const processedData = React.useMemo((): ProcessedReferrerData[] => { // If a campaign is selected, only show UTM data (not regular sources) if (selectedCampaign) { // If we have UTM data and we're not fetching, show it if (utmData?.stats && !isUtmFetching) { … } // If fetching or no data yet, return empty array (don't show regular sources) return []; } … }, [referrersData, utmData, selectedCampaign, siteUrl, defaultSourceIconUrl, isUtmFetching]); + + const isUtmLoading = Boolean(selectedCampaign && isUtmFetching && !utmData?.stats); … - {processedData.length > 0 ? ( + {isUtmLoading ? ( + <TableBody> + <TableRow className='last:border-none'> + <TableCell className='border-none py-12 group-hover:!bg-transparent' colSpan={appSettings?.paidMembersEnabled ? 4 : 2}> + <SkeletonTable className='mt-0' lines={5} /> + </TableCell> + </TableRow> + </TableBody> + ) : processedData.length > 0 ? ( <GrowthSourcesTableBody currencySymbol={currencySymbol} data={processedData} defaultSourceIconUrl={defaultSourceIconUrl} limit={limit} /> ) : (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx
(3 hunks)apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx
(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx (3)
apps/admin-x-framework/src/index.ts (1)
useParams
(40-40)apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (4)
GrowthTabType
(5-5)GrowthCampaignType
(4-4)getUtmType
(23-25)UtmGrowthTabs
(40-81)apps/admin-x-framework/src/api/stats.ts (1)
useUtmGrowthStats
(249-252)
apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx (3)
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (2)
GrowthCampaignType
(4-4)getUtmType
(23-25)apps/stats/src/hooks/useTopSourcesGrowth.ts (1)
useTopSourcesGrowth
(6-23)apps/admin-x-framework/src/api/stats.ts (1)
useUtmGrowthStats
(249-252)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: ActivityPub tests
- GitHub Check: Admin-X Settings tests
- GitHub Check: Unit tests (Node 22.13.1)
- GitHub Check: Build & Push
// Use the new endpoint with server-side sorting and limiting for regular sources | ||
const {data: referrersData} = useTopSourcesGrowth(range, backendOrderBy, limit); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
showViewAll
can no longer surface extra rows
By forwarding limit
directly to useTopSourcesGrowth
, the API now returns at most the preview-sized set. That means processedData.length
never exceeds limit
, so the “View all” footer can’t appear even when more sources exist, breaking the expanded view. Please keep the preview cap for rendering but request a larger dataset when showViewAll
is enabled.
- const {data: referrersData} = useTopSourcesGrowth(range, backendOrderBy, limit);
+ const queryLimit = showViewAll ? Math.max(limit, 50) : limit;
+ const {data: referrersData} = useTopSourcesGrowth(range, backendOrderBy, queryLimit);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Use the new endpoint with server-side sorting and limiting for regular sources | |
const {data: referrersData} = useTopSourcesGrowth(range, backendOrderBy, limit); | |
// Use the new endpoint with server-side sorting and limiting for regular sources | |
const queryLimit = showViewAll ? Math.max(limit, 50) : limit; | |
const {data: referrersData} = useTopSourcesGrowth(range, backendOrderBy, queryLimit); |
🤖 Prompt for AI Agents
In apps/stats/src/views/Stats/Growth/components/GrowthSources.tsx around lines
109-111, forwarding the UI preview `limit` into useTopSourcesGrowth causes the
API to cap results so processedData.length never exceeds limit and the “View
all” footer never appears; change the call to request a larger dataset when
showViewAll is true (e.g., compute a fetchLimit = showViewAll ? a larger
constant (or undefined/unlimited) : limit), pass fetchLimit to
useTopSourcesGrowth while still using the original preview `limit` when
slicing/rendering the preview, and keep backendOrderBy unchanged so you can
detect processedData.length > limit to show the footer and support expansion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may be the first real bug our e2e tests have caught!
And the CI workflow makes it stupidly easy to see what went wrong.
You can just copy/paste the full command from the CI workflow, and (assuming you have the gh
CLI installed) run it locally to see the test report, then click into the trace to see why it failed.
gh run download 18357523609 -n playwright-report -D /tmp/playwright-$$ && npx playwright show-report /tmp/playwright-$$/playwright-report
Super cool 🎉
I haven't looked super closely at the code just yet, but will take a closer look in the morning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx
(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx (3)
apps/shade/src/components/features/utm-growth-tabs/utm-growth-tabs.tsx (4)
GrowthTabType
(5-5)GrowthCampaignType
(4-4)getUtmType
(23-25)UtmGrowthTabs
(40-81)apps/admin-x-framework/src/api/stats.ts (1)
useUtmGrowthStats
(249-252)apps/posts/src/views/PostAnalytics/Web/components/Sources.tsx (1)
SourcesTable
(19-70)
🔇 Additional comments (8)
apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx (8)
1-13
: LGTM! Clean imports and constant extraction.The imports are well-organized, bringing in the necessary UTM types, components, and hooks. Extracting
TOP_SOURCES_PREVIEW_LIMIT
as a constant is a good practice.
115-135
: Well-structured state and data fetching logic.The conditional fetching logic correctly gates the API call behind multiple checks (feature flag, tab, campaign, postId). The
postId || ''
fallback on line 132 is defensive but safe sinceshouldFetchUtmData
includes!!postId
.
138-153
: Correct data transformation for campaign vs sources view.The useMemo correctly switches between original referrer data and UTM data based on the selected view. Returning
null
when UTM data hasn't loaded yet (lines 145-147) is handled appropriately by the empty state logic downstream.
156-180
: Appropriate handling of UTM campaigns vs domain sources.The code correctly differentiates UTM campaign data (which shouldn't have icons or links, lines 163 and 165) from regular domain-based sources. This is the right approach since UTM parameter values are arbitrary strings, not web domains.
230-242
: Clean integration of UTM tracking UI.The conditional rendering of
UtmGrowthTabs
is properly gated behind the feature flag and growth mode. The inclusion of theSeparator
provides clear visual separation between the tab controls and the data table.
194-204
: Effective dynamic title generation.The titles and descriptions correctly adapt based on the selected campaign (line 194) and mode, providing clear context to users about what data they're viewing.
258-266
: Good empty state handling with test support.The empty state includes a helpful test ID (
data-testid='empty-sources-indicator'
) and provides contextual messaging based on the mode. The growth mode message clearly explains when data will appear.
129-135
: UTM growth stats API verified:/stats/utm-growth
is implemented in Ghost core (stats.js) and correctly handlesutm_type
andpost_id
, returning the expectedUtmGrowthStatsResponseType
.
<SourcesTable | ||
data={topSources} | ||
defaultSourceIconUrl={defaultSourceIconUrl} | ||
getPeriodText={getPeriodText} | ||
headerStyle='card' | ||
mode={mode} | ||
range={range} | ||
> | ||
<CardHeader> | ||
<CardTitle>{cardTitle}</CardTitle> | ||
<CardDescription>{cardDescription}</CardDescription> | ||
</CardHeader> | ||
</SourcesTable> | ||
</> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review the CardHeader placement inside table structure.
The CardHeader
component (lines 251-254) is being passed as children to SourcesTable
and will be rendered inside a <th>
element (see line 33 where {children}
appears in TableHead
). This creates semantically incorrect HTML with complex div structures nested inside table header cells.
While the headerStyle='card'
and variant='cardhead'
suggest this might be intentional, consider:
- Accessibility: Screen readers expect simple text content in
<th>
elements - HTML semantics: Card components are block-level containers and shouldn't be nested in table headers
- Maintainability: This pattern is non-standard and may confuse future developers
Consider refactoring to render the CardHeader
outside the SourcesTable
, similar to how it's done when topSources.length <= 0
(lines 209-212). The table should only contain tabular data in its header, not complex component structures.
Alternative structure:
<>
{utmTrackingEnabled && mode === 'growth' && (
<>...</>
)}
+ <CardHeader>
+ <CardTitle>{cardTitle}</CardTitle>
+ <CardDescription>{cardDescription}</CardDescription>
+ </CardHeader>
<SourcesTable
data={topSources}
defaultSourceIconUrl={defaultSourceIconUrl}
getPeriodText={getPeriodText}
- headerStyle='card'
mode={mode}
range={range}
- >
- <CardHeader>
- <CardTitle>{cardTitle}</CardTitle>
- <CardDescription>{cardDescription}</CardDescription>
- </CardHeader>
- </SourcesTable>
+ />
</>
🤖 Prompt for AI Agents
In apps/posts/src/views/PostAnalytics/Growth/components/GrowthSources.tsx around
lines 243 to 256, the CardHeader/CardTitle/CardDescription are currently passed
as children into SourcesTable which causes those block-level card components to
be rendered inside a <th>; move the CardHeader block out of the SourcesTable and
render it immediately above the table (the same approach used for the
empty-state case at lines ~209-212), keep headerStyle='card' on SourcesTable if
needed for styling, and ensure the table receives only tabular header content
(simple text or aria-labeled header cells) while the CardHeader remains a
sibling element for correct HTML semantics and accessibility.
9larsons commented: |
👀 |
@9larsons am I crazy or are the dropdowns not there? |
9larsons commented: |
Actually, I see it on the Analytics > Growth tab, but not the Post Analytics > Growth tab. Do they just not show if there's no data for the post? |
9larsons commented: |
ref https://linear.app/ghost/issue/PROD-2557/
ref https://linear.app/ghost/issue/PROD-2559/
This PR adds the UTM dropdown to the Growth pages of the Posts and Stats (Analytics) apps. This is currently behind the
utmTracking
flag and wired up to the/stats/utm-growth
endpoint.