diff --git a/app/admin/users/page.tsx b/app/admin/users/page.tsx index 78576f9..fecd895 100644 --- a/app/admin/users/page.tsx +++ b/app/admin/users/page.tsx @@ -5,7 +5,9 @@ import { auth } from '@/lib/auth'; import { isBunnyUploadsFeatureEnabled, isStripeBillingEnabled } from '@/lib/feature-flags'; import { buildBillingAccessWhereInput, + buildEffectiveBillingStatusWhereInput, getBillingStatusLabel, + getEffectiveBillingStatus, hasBillingAccess, } from '@/lib/billing'; import { redirect } from 'next/navigation'; @@ -71,7 +73,9 @@ function getOwnBillingAccess( user.stripeCancelAtPeriodEnd || user.subscriptionStatus === BillingSubscriptionStatus.CANCELED; let endsAt: Date | null = null; - if (user.subscriptionStatus === BillingSubscriptionStatus.TRIALING) { + // The effective status, so a cardless trial (stored as FREE) still shows the + // date its access runs out instead of an open-ended "Active access". + if (getEffectiveBillingStatus(user, now) === BillingSubscriptionStatus.TRIALING) { endsAt = user.trialEndsAt; } else if (isEnding) { endsAt = user.stripeCurrentPeriodEnd ?? user.stripeCancelAt; @@ -114,6 +118,20 @@ const STATUS_FILTERS: BillingSubscriptionStatus[] = [ BillingSubscriptionStatus.FREE, ]; +// Sorting by subscription happens in memory (see canSortInDb), so the order the +// database would have used for the enum has to be spelled out. Same order as the +// enum is declared in the schema. +const STATUS_SORT_ORDER: BillingSubscriptionStatus[] = [ + BillingSubscriptionStatus.FREE, + BillingSubscriptionStatus.TRIALING, + BillingSubscriptionStatus.ACTIVE, + BillingSubscriptionStatus.PAST_DUE, + BillingSubscriptionStatus.CANCELED, + BillingSubscriptionStatus.UNPAID, + BillingSubscriptionStatus.INCOMPLETE, + BillingSubscriptionStatus.INCOMPLETE_EXPIRED, +]; + const ACCESS_FILTERS: Array<{ value: AccessFilter; label: string }> = [ { value: 'ALL', label: 'All Access' }, { value: 'ACTIVE', label: 'Has Access' }, @@ -219,7 +237,9 @@ function getSortIndicator( function canSortInDb(sortBy: SortBy): boolean { return ( sortBy === 'user' || - sortBy === 'subscription' || + // 'subscription' is deliberately absent: it sorts on the effective status, + // which lives on trialEndsAt as much as on the stored column, so a cardless + // trial would otherwise sort among the free accounts it is not shown with. sortBy === 'joinedDate' || sortBy === 'workspacesOwned' || sortBy === 'projectsOwned' || @@ -237,10 +257,6 @@ function getUsersOrderBy( return [{ name: sortDirection }, { email: sortDirection }, createdAtTieBreaker]; } - if (sortBy === 'subscription') { - return [{ subscriptionStatus: sortDirection }, createdAtTieBreaker]; - } - if (sortBy === 'joinedDate') { return [{ createdAt: sortDirection }]; } @@ -319,7 +335,7 @@ export default async function AdminUsersPage({ } if (statusFilter !== 'ALL') { - filters.push({ subscriptionStatus: statusFilter }); + filters.push(buildEffectiveBillingStatusWhereInput(statusFilter, now)); } if (accessFilter !== 'ALL') { @@ -388,6 +404,7 @@ export default async function AdminUsersPage({ email: string | null; createdAt: Date; subscriptionStatus: BillingSubscriptionStatus; + effectiveStatus: BillingSubscriptionStatus; trialEndsAt: Date | null; stripeCurrentPeriodEnd: Date | null; stripeCancelAtPeriodEnd: boolean; @@ -412,6 +429,7 @@ export default async function AdminUsersPage({ paginatedUsers = users.map((user) => ({ ...user, + effectiveStatus: getEffectiveBillingStatus(user, now), invitedMembersCount: user.ownedWorkspaces.reduce( (total, workspace) => total + workspace._count.members, 0 @@ -425,6 +443,7 @@ export default async function AdminUsersPage({ const usersWithMetrics = users.map((user) => ({ ...user, + effectiveStatus: getEffectiveBillingStatus(user, now), invitedMembersCount: user.ownedWorkspaces.reduce( (total, workspace) => total + workspace._count.members, 0 @@ -437,7 +456,11 @@ export default async function AdminUsersPage({ const sortedUsers = usersWithMetrics.sort((a, b) => { let comparison = 0; - if (sortBy === 'invitedMembers') { + if (sortBy === 'subscription') { + comparison = + STATUS_SORT_ORDER.indexOf(a.effectiveStatus) - + STATUS_SORT_ORDER.indexOf(b.effectiveStatus); + } else if (sortBy === 'invitedMembers') { comparison = a.invitedMembersCount - b.invitedMembersCount; } else if (sortBy === 'bunnyUpload') { comparison = a.bunnyUploadBytes - b.bunnyUploadBytes; @@ -758,8 +781,8 @@ export default async function AdminUsersPage({ return (
- - {getBillingStatusLabel(user.subscriptionStatus)} + + {getBillingStatusLabel(user.effectiveStatus)} {access.hasAppAccess ? ( diff --git a/lib/admin-stats.ts b/lib/admin-stats.ts index 8c57937..e042b0e 100644 --- a/lib/admin-stats.ts +++ b/lib/admin-stats.ts @@ -4,6 +4,7 @@ import { r2Client, R2_BUCKET_NAME } from '@/lib/r2'; import { ListObjectsV2Command, type ListObjectsV2CommandInput } from '@aws-sdk/client-s3'; import { isBunnyUploadsEnabled, isStripeBillingEnabled } from '@/lib/feature-flags'; import { getStripe, getStripePriceId } from '@/lib/stripe'; +import { buildCardlessTrialWhereInput } from '@/lib/billing'; import { logError } from '@/lib/logger'; const BUNNY_API_BASE = 'https://video.bunnycdn.com'; @@ -551,10 +552,17 @@ export const getCachedStripeStats = unstable_cache( if (!isStripeBillingEnabled()) return null; try { - const statusCounts = await db.user.groupBy({ - by: ['subscriptionStatus'], - _count: { id: true }, - }); + const now = new Date(); + // The cardless trial leaves `subscriptionStatus` at FREE, so the group-by + // alone counted every trial as a free user and reported "On Trial" as zero. + // Counted separately and moved across the two buckets below. + const [statusCounts, cardlessTrialUsers] = await Promise.all([ + db.user.groupBy({ + by: ['subscriptionStatus'], + _count: { id: true }, + }), + db.user.count({ where: buildCardlessTrialWhereInput(now) }), + ]); const counts: Record = {}; for (const row of statusCounts) { @@ -562,10 +570,13 @@ export const getCachedStripeStats = unstable_cache( } const activeSubscribers = counts['ACTIVE'] ?? 0; - const trialingUsers = counts['TRIALING'] ?? 0; + const trialingUsers = (counts['TRIALING'] ?? 0) + cardlessTrialUsers; const pastDueUsers = counts['PAST_DUE'] ?? 0; const canceledUsers = counts['CANCELED'] ?? 0; - const freeUsers = counts['FREE'] ?? 0; + // Clamped because the two queries above see two different snapshots: a signup + // landing between them can be counted as a trial without having been counted + // as free, which would otherwise report a negative number of free users. + const freeUsers = Math.max(0, (counts['FREE'] ?? 0) - cardlessTrialUsers); // UNPAID, INCOMPLETE and INCOMPLETE_EXPIRED belonged to none of the five buckets // above, so those users were counted nowhere and the totals silently did not add // up to the user table. diff --git a/lib/analytics/scoreboard.ts b/lib/analytics/scoreboard.ts index 8cf3f72..1c58511 100644 --- a/lib/analytics/scoreboard.ts +++ b/lib/analytics/scoreboard.ts @@ -379,7 +379,14 @@ export async function getScoreboard(options?: { weeks?: number }): Promise NOW() + THEN 'TRIALING' + ELSE u."subscriptionStatus"::text + END AS status, ua.channel, ua.self_reported, COUNT(e.id) FILTER (WHERE e.occurred_at >= NOW() - INTERVAL '7 days')::int @@ -393,7 +400,9 @@ export async function getScoreboard(options?: { weeks?: number }): Promise NOW()) + GROUP BY u.id, u.name, u.email, u."subscriptionStatus", u."trialEndsAt", ua.channel, + ua.self_reported ORDER BY MAX(e.occurred_at) ASC NULLS FIRST LIMIT ${PAID_ACCOUNT_LIMIT + 1} `, diff --git a/lib/billing.ts b/lib/billing.ts index 79169a6..49f660e 100644 --- a/lib/billing.ts +++ b/lib/billing.ts @@ -258,6 +258,73 @@ export function getBillingStatusLabel(status: BillingSubscriptionStatus) { } } +/** + * A `where` matching the accounts whose only entitlement is a running cardless + * trial: no Stripe subscription behind them, so `subscriptionStatus` is FREE. + */ +export function buildCardlessTrialWhereInput(now: Date = new Date()): Prisma.UserWhereInput { + return { + subscriptionStatus: BillingSubscriptionStatus.FREE, + trialEndsAt: { gt: now }, + }; +} + +/** + * The status to show for an account, which is not always the one Stripe stored. + * + * The cardless trial writes `trialEndsAt` and nothing else, because there is no + * Stripe subscription behind it to report `trialing`. `subscriptionStatus` stays + * FREE, so anything reading that column alone showed a running trial as a free + * account: the admin dashboard counted every trial under "Free Users" and left + * "On Trial" at zero. Access is already resolved from the date in + * `hasBillingAccess`, so what is displayed follows the same date. + * + * Only FREE is overridden. Every other status means Stripe has an opinion about + * this account (an abandoned checkout leaves INCOMPLETE while the trial runs on), + * and that opinion is the more useful of the two to show. + */ +export function getEffectiveBillingStatus( + subject: Pick, + now: Date = new Date() +): BillingSubscriptionStatus { + if ( + subject.subscriptionStatus === BillingSubscriptionStatus.FREE && + hasActiveTrial(subject.trialEndsAt, now) + ) { + return BillingSubscriptionStatus.TRIALING; + } + + return subject.subscriptionStatus; +} + +/** + * A `where` that filters on the displayed status rather than the stored one, so + * an admin asking for "Trialing" is handed the cardless trials and one asking + * for "Free" is not. + */ +export function buildEffectiveBillingStatusWhereInput( + status: BillingSubscriptionStatus, + now: Date = new Date() +): Prisma.UserWhereInput { + if (status === BillingSubscriptionStatus.TRIALING) { + return { + OR: [ + { subscriptionStatus: BillingSubscriptionStatus.TRIALING }, + buildCardlessTrialWhereInput(now), + ], + }; + } + + if (status === BillingSubscriptionStatus.FREE) { + return { + subscriptionStatus: BillingSubscriptionStatus.FREE, + OR: [{ trialEndsAt: null }, { trialEndsAt: { lte: now } }], + }; + } + + return { subscriptionStatus: status }; +} + /** * Grants the cardless trial, once per account, and reports whether this call is * the one that granted it. diff --git a/tests/api/analytics-scoreboard.test.ts b/tests/api/analytics-scoreboard.test.ts index 517741e..aa0079c 100644 --- a/tests/api/analytics-scoreboard.test.ts +++ b/tests/api/analytics-scoreboard.test.ts @@ -162,7 +162,8 @@ describe('getScoreboard', () => { const busy = await createUser({ subscriptionStatus: 'ACTIVE' }); const silent = await createUser({ subscriptionStatus: 'ACTIVE' }); const trialing = await createUser({ subscriptionStatus: 'TRIALING' }); - await createUser({ subscriptionStatus: 'FREE' }); + // Free with nothing left to run, so it stays out of the table. + await createUser({ subscriptionStatus: 'FREE', trialEndsAt: null }); await seedEvent({ name: 'VIDEO_ADDED', occurredAt: daysAgo(2), userId: busy.id }); await seedEvent({ name: 'SHARE_LINK_CREATED', occurredAt: daysAgo(20), userId: busy.id }); @@ -185,6 +186,27 @@ describe('getScoreboard', () => { expect(busyRow?.valueEvents7).toBe(1); expect(busyRow?.valueEvents30).toBe(2); }); + + // A cardless trial has no Stripe subscription to hold the status, so it sits at + // FREE with only a date behind it. Filtering on the status column alone left + // every trial account out of this table and out of the at-risk list with it. + it('includes a cardless trial and reports it as trialing', async () => { + const cardless = await createUser({ + subscriptionStatus: 'FREE', + trialEndsAt: new Date(Date.now() + 24 * 60 * 60 * 1000), + }); + const expired = await createUser({ + subscriptionStatus: 'FREE', + trialEndsAt: new Date(Date.now() - 24 * 60 * 60 * 1000), + }); + + const scoreboard = await getScoreboard({ weeks: 4 }); + const ids = scoreboard.paidAccounts.map((row) => row.userId); + + expect(ids).toEqual([cardless.id]); + expect(ids).not.toContain(expired.id); + expect(scoreboard.paidAccounts[0]?.status).toBe('TRIALING'); + }); }); // The cohort comparison is another block of raw SQL, and the part most easily diff --git a/tests/api/lib-admin-stats.test.ts b/tests/api/lib-admin-stats.test.ts index c05bf3d..d00419b 100644 --- a/tests/api/lib-admin-stats.test.ts +++ b/tests/api/lib-admin-stats.test.ts @@ -869,9 +869,11 @@ describe('getCachedStripeStats', () => { await createUser({ subscriptionStatus: 'TRIALING' }); await createUser({ subscriptionStatus: 'PAST_DUE' }); await createUser({ subscriptionStatus: 'CANCELED' }); - await createUser({ subscriptionStatus: 'FREE' }); - await createUser({ subscriptionStatus: 'FREE' }); - await createUser({ subscriptionStatus: 'FREE' }); + // Free means free: no trial left to run, or these would be counted as the + // cardless trials they would then be. + await createUser({ subscriptionStatus: 'FREE', trialEndsAt: null }); + await createUser({ subscriptionStatus: 'FREE', trialEndsAt: null }); + await createUser({ subscriptionStatus: 'FREE', trialEndsAt: null }); const stripe = stubStripePrice({ unit_amount: 1900, currency: 'eur' }); expect(await getCachedStripeStats()).toEqual({ @@ -887,6 +889,45 @@ describe('getCachedStripeStats', () => { expect(stripe.retrievedPriceIds).toEqual(['price_admin_stats_test']); }); + // The cardless trial writes only trialEndsAt, so the account sits at FREE with no + // Stripe subscription behind it. Counting the status column alone reported every + // one of them as a free user and left "On Trial" at zero on the dashboard. + it('counts a cardless trial as trialing rather than as free', async () => { + await createUser({ subscriptionStatus: 'FREE', trialEndsAt: new Date(Date.now() + 60_000) }); + await createUser({ subscriptionStatus: 'FREE', trialEndsAt: null }); + stubStripePrice({ unit_amount: 1900, currency: 'usd' }); + + const stats = await getCachedStripeStats(); + + expect(stats?.trialingUsers).toBe(1); + expect(stats?.freeUsers).toBe(1); + }); + + it('counts an expired trial back as a free user', async () => { + await createUser({ subscriptionStatus: 'FREE', trialEndsAt: new Date(Date.now() - 60_000) }); + stubStripePrice({ unit_amount: 1900, currency: 'usd' }); + + const stats = await getCachedStripeStats(); + + expect(stats?.trialingUsers).toBe(0); + expect(stats?.freeUsers).toBe(1); + }); + + // A Stripe trial is already TRIALING and is not sitting in the FREE bucket, so it + // must not be added on top of the cardless count. + it('does not double count a Stripe trial that also carries a trial end date', async () => { + await createUser({ + subscriptionStatus: 'TRIALING', + trialEndsAt: new Date(Date.now() + 60_000), + }); + stubStripePrice({ unit_amount: 1900, currency: 'usd' }); + + const stats = await getCachedStripeStats(); + + expect(stats?.trialingUsers).toBe(1); + expect(stats?.freeUsers).toBe(0); + }); + // UNPAID, INCOMPLETE and INCOMPLETE_EXPIRED are real values of the enum that used to // belong to none of the reported buckets, so those users were counted nowhere and the // five totals silently did not add up to the user table. They must not be folded into diff --git a/tests/unit/lib/billing.test.ts b/tests/unit/lib/billing.test.ts index 6e777a6..5a6ac63 100644 --- a/tests/unit/lib/billing.test.ts +++ b/tests/unit/lib/billing.test.ts @@ -4,11 +4,14 @@ import { BillingSubscriptionStatus } from '@prisma/client'; import { DEFAULT_TRIAL_PERIOD_DAYS, buildBillingAccessWhereInput, + buildCardlessTrialWhereInput, + buildEffectiveBillingStatusWhereInput, buildExpiredBillingWhereInput, getBillingAccessEndDate, getBillingOverview, getBillingStatusLabel, getDefaultTrialEndsAt, + getEffectiveBillingStatus, getOrCreateStripeCustomerId, getStorageCleanupEligibleAt, getStripeCheckoutState, @@ -546,6 +549,90 @@ describe('getBillingStatusLabel', () => { }); }); +describe('getEffectiveBillingStatus', () => { + const future = new Date(NOW.getTime() + DAY_MS); + const past = new Date(NOW.getTime() - DAY_MS); + + // The bug this exists for: a cardless trial writes trialEndsAt and nothing + // else, so the admin panel read every live trial as a free account. + it('reports a cardless trial as trialing', () => { + expect( + getEffectiveBillingStatus( + { subscriptionStatus: BillingSubscriptionStatus.FREE, trialEndsAt: future }, + NOW + ) + ).toBe(BillingSubscriptionStatus.TRIALING); + }); + + it('reports an expired trial as free again', () => { + expect( + getEffectiveBillingStatus( + { subscriptionStatus: BillingSubscriptionStatus.FREE, trialEndsAt: past }, + NOW + ) + ).toBe(BillingSubscriptionStatus.FREE); + }); + + it('reports a free account with no trial as free', () => { + expect( + getEffectiveBillingStatus( + { subscriptionStatus: BillingSubscriptionStatus.FREE, trialEndsAt: null }, + NOW + ) + ).toBe(BillingSubscriptionStatus.FREE); + }); + + // Anything Stripe has an opinion about keeps that opinion. An abandoned + // checkout leaves INCOMPLETE while the trial runs on, and INCOMPLETE is the + // more useful half of that to show. + it.each(ALL_STATUSES.filter((status) => status !== BillingSubscriptionStatus.FREE))( + 'leaves the stored status %s alone even during a running trial', + (status) => { + expect( + getEffectiveBillingStatus({ subscriptionStatus: status, trialEndsAt: future }, NOW) + ).toBe(status); + } + ); +}); + +describe('buildCardlessTrialWhereInput', () => { + it('matches a free account whose trial is still running', () => { + expect(buildCardlessTrialWhereInput(NOW)).toEqual({ + subscriptionStatus: BillingSubscriptionStatus.FREE, + trialEndsAt: { gt: NOW }, + }); + }); +}); + +describe('buildEffectiveBillingStatusWhereInput', () => { + it('folds cardless trials into the trialing filter', () => { + expect(buildEffectiveBillingStatusWhereInput(BillingSubscriptionStatus.TRIALING, NOW)).toEqual({ + OR: [ + { subscriptionStatus: BillingSubscriptionStatus.TRIALING }, + { subscriptionStatus: BillingSubscriptionStatus.FREE, trialEndsAt: { gt: NOW } }, + ], + }); + }); + + it('keeps cardless trials out of the free filter', () => { + expect(buildEffectiveBillingStatusWhereInput(BillingSubscriptionStatus.FREE, NOW)).toEqual({ + subscriptionStatus: BillingSubscriptionStatus.FREE, + OR: [{ trialEndsAt: null }, { trialEndsAt: { lte: NOW } }], + }); + }); + + it.each( + ALL_STATUSES.filter( + (status) => + status !== BillingSubscriptionStatus.FREE && status !== BillingSubscriptionStatus.TRIALING + ) + )('matches the stored column alone for %s', (status) => { + expect(buildEffectiveBillingStatusWhereInput(status, NOW)).toEqual({ + subscriptionStatus: status, + }); + }); +}); + describe('selectAuthoritativeSubscription', () => { const ENTITLED_PRICE = 'price_entitled';