Skip to content

feat(dashboards): nudge repeat dashboard viewers to subscribe - #70700

Merged
vdekrijger merged 18 commits into
masterfrom
dashboard-subscribe-nudge-experiment
Jul 16, 2026
Merged

feat(dashboards): nudge repeat dashboard viewers to subscribe#70700
vdekrijger merged 18 commits into
masterfrom
dashboard-subscribe-nudge-experiment

Conversation

@vdekrijger

@vdekrijger vdekrijger commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Problem

Dashboards are viewed constantly, but subscriptions (scheduled email/Slack delivery) are barely discovered — most people who'd benefit from getting a dashboard delivered on a schedule never notice the feature exists. This is experiment candidate #1 from an uptake analysis of the subscriptions feature.

Changes

Nudges repeat dashboard viewers toward subscriptions via the in-app notifications product plus a sticky toast (originally an inline banner; pivoted because the banner proved too easy to miss in hands-on testing):

  • Triggered when a user has viewed the same dashboard 3+ times in the last 7 days (view recording deduped at 60s), has no existing subscription on it, can edit the dashboard, and is in the test variant of feature flag dashboard-subscribe-nudge (control/test — launch is manual).
  • Delivery is two-layer: a new POST /dashboards/:id/subscribe_nudge endpoint creates a normal-priority in-app notification (new subscription_nudge type, dashboard-scoped access control, 30-day server-side dedupe per user+dashboard that is released if creation no-ops), and the client shows a sticky toast at the qualification moment (persists until acted on) with the same CTA.
  • Both surfaces deep-link to the new-subscription form with ?prefill=nudge, so a notification clicked days later in a fresh session still lands on a prefilled form; the params are consumed and stripped on arrival.
  • The CTA skips the subscriptions list and opens the create form directly, prefilled with the viewer's email as recipient (frequency/day/time/destination already match the form's own weekly Monday-morning email defaults, so only the recipient needed prefilling).

Key pieces:

  • dashboardSubscribeNudgeLogic (new) — records a view timestamp per dashboard in a persisted reducer, prunes anything older than 7 days, and exposes isEligible combining view count, dismissal, and dashboard-type. Only reads the feature flag once isEligible is true — reading a PostHog feature flag value reports its exposure event, so gating the read this way scopes experiment exposure to the population that could actually see the banner.
  • Subscriptions are fetched only for dashboards already past the view threshold (not on every dashboard view), avoiding an extra API call for the vast majority of page views.
  • dashboardLogic gained a small subscriptionPrefill action/reducer so the nudge can hand initial field values to the subscription form without going through URL query params.
  • Three new analytics events: dashboard subscribe nudge shown, dashboard subscribe nudge clicked (carries prefilled and via: toast vs notification), and dashboard subscribe nudge check failed (with error detail) so a broken eligibility check is distinguishable from genuine ineligibility. There is deliberately no dismissed event — dismissal/read state belongs to the notifications product. subscription created additionally carries ai_summary_prefilled.

How did you test this code?

Automated only — no manual browser testing was done.

  • dashboardSubscribeNudgeLogic.test.ts (new, 23 cases): view-count threshold (parameterized), 7-day pruning, dismissal (+ its captured event), and three cases proving the feature-flag read is gated on eligibility (unresolved subscription check, existing subscription, and the eligible/shown path with single-capture dedup). Guards the one behavior explicitly required here — that ineligible views never trigger flag exposure.
  • Extended subscriptionLogic.test.ts (18 cases total): opening the "new" subscription route with a prefill lands with the prefilled values and marks the form changed (so "Create subscription" is enabled immediately); a plain "new" open (no prefill, existing test) now also asserts the form stays unmarked. Catches the prefill either not applying, or applying without enabling submit.
  • pnpm --filter=@posthog/frontend typescript:check and pnpm --filter=@posthog/frontend format — clean on the touched files (pre-existing unrelated errors elsewhere on master, untouched by this change).

Proof of work

CleanShot 2026-07-15 at 11 59 23 CleanShot 2026-07-15 at 11 21 49 CleanShot 2026-07-15 at 11 34 43

AB test

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Docs update

N/A — internal experiment banner, no documented workflow changed.

Review hardening (second commit)

A multi-pass local review (simplify pass + six-reviewer swarm, four passes) hardened the initial implementation before marking ready: view-count dedupe so the 3-view bar means three real visits, subscription re-check on modal close so the banner cannot reappear right after subscribing, prefill lifecycle fixes (survives only on the 'new' form; insight auto-select no longer clobbers the dirty flag on dashboards with tiles), silent-failure conventions for the background check, and bounded localStorage in both dimensions. Both test suites grew accordingly (41 tests total).

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Built end-to-end by Claude (Claude Code) from a detailed brief plus a mid-session scope addition (prefilling the subscription form). Skills invoked: /writing-kea-logics, /writing-tests.

Decisions made along the way:

  • Split "is this dashboard even a candidate" (view count, dismissal, dashboard type — all cheap, always available) from "is there already a subscription" (requires a fetch) into two tiers, so the subscriptions API is only called for the small slice of views that cross the threshold, not every dashboard view.
  • The exposure-gating requirement (don't read the flag for ineligible users) is enforced by only touching featureFlags[key] inside a selector body guarded by isEligible — PostHog's feature flag proxy reports $feature_flag_called on property access, so this is the mechanism, not just a convention.
  • Prefill values are threaded through kea props/actions (dashboardSubscribeNudgeLogic.subscriptionPrefillSubscriptionsModalsubscriptionLogic initialValues), not URL query params, even though the codebase has an existing ?target_type= query-param precedent for the same form. The prefill lives only while the modal route is the 'new' form, and the discard-changes guard is scoped to a captured prefill baseline so an untouched prefilled form never prompts.
  • The prefill is applied via setSubscriptionValues (not folded into the loaded baseline) specifically so the form's dirty-tracking flips to true immediately — otherwise the create button stays disabled until the user edits something, defeating the "essentially one confirm click" goal.

Launch of the dashboard-subscribe-nudge flag is manual and out of scope for this PR.

Adds a dismissible banner on the dashboard scene for users who've viewed
the same dashboard 3+ times in the last 7 days, have no existing
subscription on it, and are in the test variant of the
dashboard-subscribe-nudge flag. The CTA opens the subscriptions modal
directly in create mode, prefilled with the viewer's email as recipient
(frequency/day/time/destination already match the form's own weekly
Monday-morning email defaults).

dashboardSubscribeNudgeLogic tracks per-dashboard view timestamps and
dismissal in persisted reducers, and only reads the feature flag once
view-count, dismissal, and existing-subscription conditions are all
satisfied, so experiment exposure is scoped to the eligible population.
Subscriptions are only fetched for dashboards already past the view
threshold, to avoid an extra API call on every dashboard view.
@vdekrijger vdekrijger self-assigned this Jul 14, 2026
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Bundle size — 🔺 +41.1 KiB (+0.1%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 64.81 MiB · 🔺 +41.1 KiB (+0.1%)

File Size Δ vs base
posthog-app/_parent/products/stamphog/frontend/scenes/StamphogScene/StamphogScene.js 9.9 KiB 🔺 +9.9 KiB (new)
exporter/src/exporter/scenes/ExporterDashboardScene.js 274.7 KiB 🔺 +7.7 KiB (+2.9%)
render-query/src/render-query/render-query.js 21.31 MiB 🔺 +5.3 KiB (+0.0%)
posthog-app/src/scenes/experiments/Experiment.js 224.1 KiB 🔺 +1.7 KiB (+0.8%)
posthog-app/_parent/products/review_hog/frontend/CodeReviewScene.js 48.9 KiB 🔺 +1.1 KiB (+2.4%)

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.22 MiB · 22 files 🔺 +714 B (+0.1%) ███░░░░░░░ 28.4% of 4.29 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.14 MiB · 2,981 files 🔺 +7.0 KiB (+0.1%) █████████░ 88.0% of 9.25 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
789 B src/scenes/ChunkLoadErrorBoundary.tsx
762 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
281.3 KiB ../node_modules/.pnpm/posthog-js@1.402.3/node_modules/posthog-js/dist/rrweb.js
267.7 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
235.5 KiB src/taxonomy/core-filter-definitions-by-group.json
222.9 KiB ../node_modules/.pnpm/posthog-js@1.402.3/node_modules/posthog-js/dist/module.js
164.0 KiB src/queries/validators.js
154.3 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
105.8 KiB src/lib/api.ts
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
92.7 KiB ../packages/quill/packages/quill/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

Dist folder size — 🔺 +2.81 MiB (+0.2%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1315.50 MiB · 🔺 +2.81 MiB (+0.2%)

ℹ️ MCP UI apps size — 31 app(s), 16561.5 KB JS

Built size of each MCP UI app (main.js + styles.css).

App JS CSS
debug 598.2 KB 179.2 KB
action 456.5 KB 179.2 KB
action-list 563.0 KB 179.2 KB
cohort 455.4 KB 179.2 KB
cohort-list 562.0 KB 179.2 KB
email-template 455.3 KB 179.2 KB
error-details 471.1 KB 179.2 KB
error-issue 456.1 KB 179.2 KB
error-issue-list 562.9 KB 179.2 KB
experiment 560.1 KB 179.2 KB
experiment-list 563.8 KB 179.2 KB
experiment-results 561.8 KB 179.2 KB
feature-flag 565.8 KB 179.2 KB
feature-flag-list 569.5 KB 179.2 KB
feature-flag-testing 459.4 KB 179.2 KB
insight-actors 560.8 KB 179.2 KB
invite-email-preview 454.7 KB 179.2 KB
llm-costs 558.1 KB 179.2 KB
session-recording 457.2 KB 179.2 KB
session-summary 462.5 KB 179.2 KB
survey 457.0 KB 179.2 KB
survey-global-stats 560.9 KB 179.2 KB
survey-list 563.7 KB 179.2 KB
survey-stats 560.8 KB 179.2 KB
trace-span 455.8 KB 179.2 KB
trace-span-list 562.9 KB 179.2 KB
workflow 455.8 KB 179.2 KB
workflow-list 562.4 KB 179.2 KB
query-results 743.7 KB 179.2 KB
render-ui 824.0 KB 179.2 KB
visual-review-snapshots 460.3 KB 179.2 KB
Playwright — all passed

All tests passed.

View test results →

⚠️ Django migration SQL — 1 new migration to review

We've detected new migrations on this PR. Review the SQL output for each migration:

products/notifications/backend/migrations/0018_alter_notificationevent_notification_type.py

BEGIN;
--
-- Alter field notification_type on notificationevent
--
-- (no-op)
COMMIT;

Last updated: 2026-07-16 12:53 UTC (81c643d)

Django migration risk — migration analysis complete

We've analyzed your migrations for potential risks.

Summary: 0 Safe | 1 Needs Review | 0 Blocked

⚠️ Needs Review

May have performance impact

notifications.0018_alter_notificationevent_notification_type
  └─ #1 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: notificationevent, field: notification_type, field_type: CharField

Last updated: 2026-07-16 12:53 UTC (81c643d)

ClickHouse migration SQL — none

No ClickHouse migrations in the latest push.

@trunk-io

trunk-io Bot commented Jul 14, 2026

Copy link
Copy Markdown

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

vdekrijger and others added 10 commits July 14, 2026 16:51
Review-swarm findings across four passes, all verified before fixing:
- dedupe double-dispatched dashboard views (60s window) so the 3-view
  eligibility bar means three real visits
- re-check subscription existence after the modal closes so the banner
  cannot reappear right after subscribing
- keep the prefill alive only on the 'new' form route; scope the
  discard-changes guard to a captured prefill baseline
- preserve the prefill's dirty flag through insight auto-select so
  Create stays enabled on dashboards with tiles (plain path unchanged)
- silent-failure conventions for the background check (load-prefixed,
  allow-listed) plus a check-failed analytics event with error detail
- bound both persisted view-log dimensions (100 dashboards, 7 days)
  and cap suppression tracking
- move dismissal to LemonBanner dismissKey, relocate prefill state out
  of shared dashboardLogic, drop dead prefill merge path
Replaces the dashboard subscribe-nudge banner with an in-app
notification plus a transient toast, so qualifying viewers get a
visible push instead of an easily-missed inline banner.

Backend: new subscription_nudge NotificationType and a
POST /dashboards/:id/subscribe_nudge endpoint that sends the requesting
user a normal-priority notification through the notifications facade,
deduplicated per (user, dashboard) via a 30-day cache sentinel. The
notification links to /dashboard/:id/subscriptions/new?prefill=nudge,
so it works days later in a fresh session.

Frontend: dashboardSubscribeNudgeLogic keeps the whole client-side
eligibility chain (deduped view counting, suppression, paywall,
exposure-gated flag read) and now requests the notification once when
showNudge flips true, guarded by a persisted notified marker. On
creation it fires the shown event and a lemonToast with a
"Set up subscription" action. subscriptionLogic builds the prefill
(title from dashboard name, recipient from the current user) from the
?prefill=nudge URL param, reusing the prefill-baseline machinery that
keeps Create enabled without arming the discard prompt. The clicked
event now carries via=toast|notification.

The dismissed event is gone: notification read state belongs to the
notifications product.
The 30-day dedupe sentinel was written before create_notification ran,
so an org with in-app notifications unavailable (or a user opt-out)
silently burned the nudge for 30 days even after the condition cleared.
The server now releases the sentinel when nothing was created, and the
client only records its "notified" marker on created=true, so a later
qualifying visit retries while the sentinel still collapses races.

Also: subscribe_nudge now requires dashboard:write (matching the
viewset's other mutating actions, so a read-scoped API key can no
longer trigger the write path), and the ?prefill=nudge params are
consumed via router replace after applying, so refreshing the form URL
neither re-fires the clicked event nor re-applies a stale prefill.

Updates the sending-notifications skill's "adding a new type" steps to
match the current single icon source and preferences metadata map.
The transient plain-string toast was easy to miss. It now persists
until acted on (autoClose: false, matching the critical notification
toast) and renders an MCP-hint-style body: bell icon, a bold
"You keep coming back to {dashboard}" line, and a muted delivery line,
with the existing "Set up subscription" CTA (which auto-dismisses the
toast via its toastId on click).
The right-aligned CTA from the lemonToast button option squeezed the
text into a narrow column and wrapped the title awkwardly. The toast
body is now a single full-width stack: icon + title row, a muted
description that includes the real 7-day view count, and the
"Set up subscription" CTA below. The CTA moved into the body, so it
dismisses the toast explicitly via its stable toastId before routing
to the prefilled form; the X close from the toast chrome is unchanged.
…iptions

Upsell: a subscription created via the subscribe-nudge flow now starts
with the AI summary enabled — but only when the org has already
accepted AI data processing and the summary quota has headroom, exactly
mirroring the server-side create validation, so the one-click create
never 403s and the consent popover never appears uninvited.

The quota answer loads async after the prefill applies, so the default
is deferred to loadSummaryQuotaSuccess (same pattern as the insight
auto-select): applied at most once, folded into the prefill baseline so
an untouched form still leaves without the discard prompt, and a manual
toggle-off survives later quota reloads. When either gate fails it
silently stays off.

The `subscription created` event gains ai_summary_prefilled so the
upsell's conversion is measurable.
@vdekrijger
vdekrijger marked this pull request as ready for review July 16, 2026 07:19
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested review from a team July 16, 2026 07:19
@pr-assigner-resolver-posthog

Copy link
Copy Markdown

👀 Auto-assigned reviewers

These soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:

  • @PostHog/team-devex (owners.yaml)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). The locator after each owner is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Security Review

The endpoint enforces dashboard team scoping and edit access. The persisted client eligibility state is not scoped by user or team, so dashboard activity and suppression decisions can cross account boundaries on a shared browser.

Reviews (1): Last reviewed commit: "fix(dashboards): shared nudge deep-link ..." | Re-trigger Greptile

Comment thread frontend/src/scenes/dashboard/dashboardSubscribeNudgeStoreLogic.ts Outdated
Comment thread products/dashboards/backend/api/dashboard.py
Free-tier orgs can create subscriptions up to the free-tier cap, so
excluding them from the nudge entirely was too strict. Eligibility now
checks the team-wide subscription count (fetched only for free-tier
candidate dashboards) and fails closed on an unknown count: a proactive
nudge should not advertise an action it cannot confirm. Also clarifies
the candidate-dashboard loader comment.
The nudge store persisted to localStorage under a single unkeyed path, so a
second account signing in on the same browser inherited the first account's
view counts, suppressions, and notified markers.

Key the store on a team+user scope derived from app context at mount time.
kea derives each persisted entry's localStorage key from path(key), so distinct
users and teams now get isolated storage.

Entries written under the old unscoped key are orphaned rather than migrated;
the feature is unlaunched, so there is no meaningful state to carry over.
…cord

The subscribe_nudge dedupe relied solely on a Redis cache sentinel, so losing
the cache (flush, eviction, outage) let a user be nudged again for a dashboard
they had already been nudged about.

Check the notifications facade's has_been_dispatched before creating, so the
server contract is once-ever per (user, dashboard) and no longer depends on
Redis health. The cache sentinel stays in front as the cheap concurrency guard.
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

🦔 Hogbox preview · ✅ ready

▶ Open the preview

🔑 Login test@posthog.com / 12345678 (demo data)
🧩 Running this PR's backend and frontend, on the PostHog :master base
🔗 Link stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access tailnet only (PostHog VPN)
🛠️ Admin inspect & debug state in hogland
💤 Idle sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

commit 81c643d · box box-58cbf3463467 · ready in 884s (push → usable) · build log · rebuilds on every push, torn down on close

@vdekrijger
vdekrijger removed the request for review from a team July 16, 2026 08:47
@vdekrijger
vdekrijger removed the request for review from a team July 16, 2026 08:47
@vdekrijger vdekrijger added stamphog Request AI approval (no full review) reviewhog ($$$) Reviews pull requests before humans do labels Jul 16, 2026
@stamphog

stamphog Bot commented Jul 16, 2026

Copy link
Copy Markdown

Note

🤖 stamphog reviewed 613af277527ea98517306cd4c6e8fb598d2f13ef — verdict: REFUSED

Deterministic gates explicitly denied this PR on size, tier, and a deny-listed migration file — per instructions this alone forces REFUSE/ESCALATE regardless of how clean the diff or its review history look.

  • 👍 on the PR from greptile-apps[bot], hex-security-app[bot].
  • Gates denied: PR exceeds the auto-review size ceiling (808 substantive lines / 18 files vs 800L cap) and is classified T2-never (cross-cutting, 1773 total lines/25 files)
  • Includes a Django migration (products/notifications/backend/migrations/0018_...), which is on the deny-list for automated review regardless of content
  • Cross-team change touching team-analytics-platform, team-devex, team-platform-features, and team-product-analytics owned files
Gate mechanics and policy version
Gate Result
prerequisites all clear
deny-list matches: migrations
size too large for auto-review (808L, 18F substantive, 1773L/25F incl. docs/generated/snapshots — ceiling is 800L)
tier classified as T2-never: T2-never (1773L, 25F, cross-cutting, feat)
stamphog 2.0.0b3 .stamphog/policy.yml @ 3c6ec10 · reviewed head 613af27

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Jul 16, 2026
@vdekrijger
vdekrijger enabled auto-merge (squash) July 16, 2026 12:19
@posthog

posthog Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Visual changes approved by @vdekrijger — baseline updated in 81c643d.

View this run in PostHog

2 changed.

Changed

Snapshot Before After
components-hogcharts-linechart--confidence-band-below-zero--dark before after
components-hogcharts-linechart--confidence-band-below-zero--light before after

2 updated
Run: 5ac1d5a7-b72e-44ef-922d-7cbebb80f2a7

Co-authored-by: vdekrijger <66254686+vdekrijger@users.noreply.github.com>
Comment on lines +280 to +282
DASHBOARD_QUICK_FILTERS_EXPERIMENT: 'dashboard-quick-filters-experiment', // owner: @vdekrijger #team-product-analytics multivariate=control,test
DASHBOARD_SUBSCRIBE_NUDGE: 'dashboard-subscribe-nudge', // owner: #team-analytics-platform multivariate=control,test, nudges repeat dashboard viewers with no subscription toward setting one up
DASHBOARD_SUBSCRIBE_PLACEMENT: 'dashboard-subscribe-placement', // owner: @MattPua #team-analytics-platform multivariate=control,button,menu

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think thomas removed quick filters yesterday, and i removed subsribe placement, probbably need to rebase off master

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in follow-up draft PR #71592. It removes the stale quick-filter and subscribe-placement constants that were reintroduced.

null as number | null,
{
loadTeamSubscriptionCount: async (_?: unknown, breakpoint?: BreakPointFunction) => {
// limit=1 keeps the payload tiny; `count` reflects the team's full total.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: not a useful comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in follow-up draft PR #71592. I removed the low-value payload comment and clarified the loader through naming instead.

Comment on lines +97 to +98
// Asks the backend to deliver the in-app nudge notification. The server dedupes per
// (user, dashboard) and reports whether a notification was actually created.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: can probably remove this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in follow-up draft PR #71592. I removed this comment.

Comment on lines +83 to +85
// Team-wide subscription count, fetched only for free-tier candidates: those orgs can
// create subscriptions until SubscriptionFreeTierLimit.COUNT, and nudging someone already
// at the limit would push them straight into the create-form paywall.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

does this really matter, if our criteria of checking was whether or not they have subscriptions at all?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The check is still needed because it is team-wide and enforces the free-tier creation cap, while the existing-subscription check is scoped to the current dashboard. Follow-up draft PR #71592 renames it to freeTierSubscriptionCount and makes that distinction explicit.

recordRecentSlackChannel(subscription.integration_id, slackChannelId(subscription.target_value))
}
},
applyInsightSelectionDefaults: ({ selectedIds }) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: i was confused for a while at what this was doing cuz of the naming, maybe should be something like

applyDefaultSelectedInsights

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in follow-up draft PR #71592 by renaming the action to applyDefaultSelectedInsights.

Comment on lines +506 to +513
posthog.capture('dashboard subscribe nudge clicked', {
dashboard_id: props.dashboardId,
prefilled: !!values.user?.email,
via:
searchParams[SUBSCRIPTION_PREFILL_PARAMS.viaParam] === SUBSCRIPTION_PREFILL_PARAMS.viaToast
? SUBSCRIPTION_PREFILL_PARAMS.viaToast
: SUBSCRIPTION_PREFILL_PARAMS.viaNotification,
})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

wouldnt it make more sense to add this capture event on the button in the nudge itself? or is it cuz the nudge can come from notifications or the toast

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It stays at route entry because both the toast and notification deep links enter the flow here. Follow-up draft PR #71592 adds a concise comment explaining that choice.

insight_short_id: props.insightShortId,
subscription_id: updatedSub.id,
target_type: updatedSub.target_type,
// True when the nudge flow's deferred AI-summary default was applied to this form.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: not a useful comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in follow-up draft PR #71592. I removed this comment.

Xs: 'xs',
} as const;

export type EnvironmentsDashboardsSubscribeNudgeCreateParams = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this nudge is solely UI right? do we want MCP changes for this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The endpoint is not exposed as an MCP tool: its entry in products/dashboards/mcp/tools.yaml is disabled. This generated API type is expected because MCP codegen consumes the shared OpenAPI schema. Verified while preparing follow-up draft PR #71592.

@vdekrijger
vdekrijger merged commit bb0a950 into master Jul 16, 2026
259 checks passed
@vdekrijger
vdekrijger deleted the dashboard-subscribe-nudge-experiment branch July 16, 2026 14:49
@deployment-status-posthog

deployment-status-posthog Bot commented Jul 16, 2026

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-07-16 15:52 UTC Run
prod-us ✅ Deployed 2026-07-16 16:08 UTC Run
prod-eu ✅ Deployed 2026-07-16 16:07 UTC Run

rnegron pushed a commit that referenced this pull request Jul 16, 2026
Co-authored-by: tests-posthog[bot] <250237707+tests-posthog[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewhog ($$$) Reviews pull requests before humans do

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants