Skip to content

Conversation

9larsons
Copy link
Contributor

@9larsons 9larsons commented Oct 8, 2025

ref https://linear.app/ghost/issue/PROD-2557/
ref https://linear.app/ghost/issue/PROD-2559/

  • added campaign dropdown to stats and posts Growth views

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.

- 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
@github-actions github-actions bot added the community [triage] Community features and bugs label Oct 8, 2025
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Walkthrough

Adds UTM growth support: new types (UtmGrowthStatItem, UtmGrowthStatsResponseType), a data type constant and hook (useUtmGrowthStats) fetching /stats/utm-growth/ in admin-x-framework; introduces UtmGrowthTabs and related exports (GrowthCampaignType, GrowthTabType, options, mappings) in shade and re-exports them; stats and posts UIs gain campaign-aware tab/dropdown guarded by globalData.labs.utmTracking; GrowthSources accepts selectedCampaign, conditionally fetches/processes UTM stats and switches display logic; SourceIcon now returns null when iconSrc is empty.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly captures the primary change of adding UTM dropdowns to the Analytics Growth pages and directly reflects the major update in the pull request without unnecessary detail or noise.
Description Check ✅ Passed The description clearly describes the addition of the campaign dropdown to the Growth pages in both the Posts and Stats apps, references the feature flag and endpoint, and aligns directly with the implemented changes.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-utm-growth-frtonend

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.

@9larsons 9larsons requested a review from cmraible October 8, 2025 19:20
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 fetching

When a campaign is selected we return [] while useUtmGrowthStats 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 on isUtmFetching (e.g. compute an isLoading 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

📥 Commits

Reviewing files that changed from the base of the PR and between c4ad4f5 and 6271dd6.

📒 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6271dd6 and 8e65387.

📒 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.

Copy link

axolo-co bot commented Oct 8, 2025

9larsons commented:
I'm trying to clean this up a little, heads up.

Copy link

axolo-co bot commented Oct 8, 2025

9larsons commented:
I'm getting a bizarre build error...

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 8e65387 and 3480ed2.

📒 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.

Comment on lines +149 to +169
// 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 [];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Comment on lines +167 to 173
<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('');
}
}}>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

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').

Copy link
Collaborator

cmraible commented Oct 8, 2025

Hm yeah I'm getting the same build error locally. but not on main. Definitely bizarre, since you didn't change anything in shade or activitypub....

Copy link

axolo-co bot commented Oct 8, 2025

9larsons commented:
It was a weird build circular dependency, just pushed a change

Copy link

axolo-co bot commented Oct 8, 2025

9larsons commented:
Again, terrible error message 🤦

Copy link

axolo-co bot commented Oct 8, 2025

9larsons commented:
Ready for you whenever, I think.. I need a break

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 flicker

When selectedCampaign is set we return [] until isUtmFetching 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

📥 Commits

Reviewing files that changed from the base of the PR and between 3480ed2 and 9c0cc3e.

📒 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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 loads

When 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. derive isUtmLoading = 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

📥 Commits

Reviewing files that changed from the base of the PR and between 9c0cc3e and a088815.

📒 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

Comment on lines +109 to +111
// Use the new endpoint with server-side sorting and limiting for regular sources
const {data: referrersData} = useTopSourcesGrowth(range, backendOrderBy, limit);

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Suggested change
// 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.

Copy link
Collaborator

@cmraible cmraible left a 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between a088815 and 700f52e.

📒 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 since shouldFetchUtmData 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 the Separator 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 handles utm_type and post_id, returning the expected UtmGrowthStatsResponseType.

Comment on lines +243 to +256
<SourcesTable
data={topSources}
defaultSourceIconUrl={defaultSourceIconUrl}
getPeriodText={getPeriodText}
headerStyle='card'
mode={mode}
range={range}
>
<CardHeader>
<CardTitle>{cardTitle}</CardTitle>
<CardDescription>{cardDescription}</CardDescription>
</CardHeader>
</SourcesTable>
</>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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:

  1. Accessibility: Screen readers expect simple text content in <th> elements
  2. HTML semantics: Card components are block-level containers and shouldn't be nested in table headers
  3. 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.

Copy link

axolo-co bot commented Oct 9, 2025

9larsons commented:
Yep this was a perfect example 🙌 ! We had a regression on the empty state. We already know the logic in these components is getting unwieldy, especially this growth component with several tabs now, and having these e2e tests is a huge help for simple regressions like this. And the trace was great - super easy to pull & review.

@9larsons 9larsons requested a review from cmraible October 9, 2025 17:07
Copy link
Collaborator

cmraible commented Oct 9, 2025

👀

Copy link
Collaborator

cmraible commented Oct 9, 2025

@9larsons am I crazy or are the dropdowns not there?

Copy link

axolo-co bot commented Oct 9, 2025

9larsons commented:
You got the flag enabled? Maybe it changed w/ the test fix, sec and I'll look

Copy link
Collaborator

cmraible commented Oct 9, 2025

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?

Copy link

axolo-co bot commented Oct 9, 2025

9larsons commented:
https://github.com/TryGhost/Ghost/pull/25104/files#diff-c8a28eb9b9e6edb172f98e9f2d74d0d64cd74020f35a79fec7ddcf8cbc051d5dR230-R240 it should be as long as the flag is enabled. I'll look as soon as I'm done w/ this other review which I'm very close to done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community [triage] Community features and bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants