diff --git a/pr_review_todos.md b/pr_review_todos.md new file mode 100644 index 000000000..95460fda3 --- /dev/null +++ b/pr_review_todos.md @@ -0,0 +1,23 @@ +# PR Review TODOs for PR #444 + +- [x] `template/app/src/analytics/operations.ts:9` Hm, it has daily and weekly stats, so maybe `DailyStatsValues` is not the best name? +- [x] `template/app/src/analytics/operations.ts:34` We can probably just do `findMany` and get first one from it. +- [x] `template/app/src/analytics/operations.ts:38` This assumes that `dailyStatsJob` generates the daily stats and that it is called that way. Might be better if the message here is less specific, or if it didn't happen here at all. +- [x] `template/app/src/analytics/operations.ts:42` Ok so this function says `getAnalyticsDataByDay` but returns both daily Stats and `weeklyStats`, look into that. +- [x] `template/app/src/analytics/operations.ts:5` Is it clear enough here that sources are `PageViewSources`? Maybe it is, but if not we can make it clearer. +- [ ] `template/app/src/analytics/stats.ts:1` Consider renaming to `jobs.ts` and creating an `index.ts` entrypoint file. +- [ ] `template/app/src/analytics/stats.ts:7` This often gets overlooked. Maybe create an interface in `index.ts` that's explicit about which provider is being used. +- [ ] `template/app/src/analytics/stats.ts:11` This type is only being used by the admin dash components. Consider moving it to a central place there. +- [ ] `template/app/src/analytics/stats.ts:18` Let's extract this to a function or make it a one liner with a descriptive variable name. +- [ ] `template/app/src/analytics/stats.ts:24` Investigate whether this needs to be `equals` or it could be, .e.g. the most recent entity from yesterday, or `>=` or `<=` or something like that. +- [ ] `template/app/src/analytics/stats.ts:31` Clarify how this applies to the code below. +- [ ] `template/app/src/analytics/stats.ts:43` The variable names are are unclear and could benefit from a cleaner implementation. Don't use `let`, use `const`. Rethink what is `userDelta` for if there is no `yesterdaysStats`. +- [ ] `template/app/src/analytics/stats.ts:55` Can we abstract the `switch` case away? +- [ ] `template/app/src/analytics/stats.ts:59` `dailyStats` could be renamed to `todaysDailyStats`. +- [ ] `template/app/src/analytics/stats.ts:13` This whole function would benefit a lot by extracting the main parts to helper functions and organizing them logically. +- [ ] `template/app/src/analytics/stats.ts:95` Replace with `upsert`. +- [ ] `template/app/src/analytics/stats.ts:59` It looks like always have just one `DailyStats` entity per day in the DB. We accomplish that by always keeping the date set to midnight (`nowUTC`). Make this explicit or somehow more robust. +- [ ] `template/app/src/analytics/stats.ts:96` `const sources = await getSources();` +- [ ] `template/app/src/analytics/stats.ts:96` Rename to `pageViewSources`. +- [ ] `template/app/src/analytics/stats.ts:120` Can we make the user more aware of why `pageViewSources` are here and what's going on? Can we make it more clear what the connection between `dailyStats` and `pageViewSources` is here? Also, can we extract this `upsert` logic to a helper function because it feels too specific? +- [ ] `template/app/src/analytics/stats.ts:125` This could be extracted to a log provider/helper. \ No newline at end of file diff --git a/template/app/main.wasp b/template/app/main.wasp index f7dda3c35..d1c77b6a5 100644 --- a/template/app/main.wasp +++ b/template/app/main.wasp @@ -238,21 +238,21 @@ query getDownloadFileSignedURL { //#endregion //#region Analytics -query getDailyStats { - fn: import { getDailyStats } from "@src/analytics/operations", - entities: [User, DailyStats] +query getAnalyticsDataByDay { + fn: import { getAnalyticsDataByDay } from "@src/analytics/operations", + entities: [User, DailyAnalytics] } -job dailyStatsJob { +job dailyAnalyticsJob { executor: PgBoss, perform: { - fn: import { calculateDailyStats } from "@src/analytics/stats" + fn: import { calculateDailyAnalytics } from "@src/analytics/stats" }, schedule: { - cron: "0 * * * *" // every hour. useful in production - // cron: "* * * * *" // every minute. useful for debugging + // cron: "0 * * * *" // every hour. useful in production + cron: "* * * * *" // every minute. useful for debugging }, - entities: [User, DailyStats, Logs, PageViewSource] + entities: [User, DailyAnalytics, Logs, PageViewSource] } //#endregion diff --git a/template/app/schema.prisma b/template/app/schema.prisma index eda467170..0f6ac737c 100644 --- a/template/app/schema.prisma +++ b/template/app/schema.prisma @@ -64,7 +64,7 @@ model File { uploadUrl String } -model DailyStats { +model DailyAnalytics { id Int @id @default(autoincrement()) date DateTime @default(now()) @unique @@ -77,7 +77,7 @@ model DailyStats { totalRevenue Float @default(0) totalProfit Float @default(0) - sources PageViewSource[] + pageViewSources PageViewSource[] } model PageViewSource { diff --git a/template/app/src/admin/dashboards/analytics/AnalyticsDashboardPage.tsx b/template/app/src/admin/dashboards/analytics/AnalyticsDashboardPage.tsx index 70382a459..e3728d738 100644 --- a/template/app/src/admin/dashboards/analytics/AnalyticsDashboardPage.tsx +++ b/template/app/src/admin/dashboards/analytics/AnalyticsDashboardPage.tsx @@ -1,5 +1,5 @@ import { type AuthUser } from 'wasp/auth'; -import { useQuery, getDailyStats } from 'wasp/client/operations'; +import { useQuery, getAnalyticsDataByDay } from 'wasp/client/operations'; import TotalSignupsCard from './TotalSignupsCard'; import TotalPageViewsCard from './TotalPageViewsCard'; import TotalPayingUsersCard from './TotalPayingUsersCard'; @@ -13,45 +13,42 @@ import { cn } from '../../../client/cn'; const Dashboard = ({ user }: { user: AuthUser }) => { useRedirectHomeUnlessUserIsAdmin({ user }); - const { data: stats, isLoading, error } = useQuery(getDailyStats); + const { data: analyticsData, isLoading, error } = useQuery(getAnalyticsDataByDay); return (
- - + +
- +
- +
- {!stats && ( + {!analyticsData && (

- No daily stats generated yet -

-

- Stats will appear here once the daily stats job has run + No daily analytics found yet

diff --git a/template/app/src/admin/dashboards/analytics/RevenueAndProfitChart.tsx b/template/app/src/admin/dashboards/analytics/RevenueAndProfitChart.tsx index dca3f54e3..75ee58a3c 100644 --- a/template/app/src/admin/dashboards/analytics/RevenueAndProfitChart.tsx +++ b/template/app/src/admin/dashboards/analytics/RevenueAndProfitChart.tsx @@ -1,7 +1,8 @@ import { ApexOptions } from 'apexcharts'; import React, { useState, useMemo, useEffect } from 'react'; import ReactApexChart from 'react-apexcharts'; -import { type DailyStatsProps } from '../../../analytics/stats'; +import { type DailyAnalytics } from 'wasp/entities'; +import { type DailyAnalyticsProps } from './types'; const options: ApexOptions = { legend: { @@ -109,26 +110,26 @@ interface ChartOneState { }[]; } -const RevenueAndProfitChart = ({ weeklyStats, isLoading }: DailyStatsProps) => { +const RevenueAndProfitChart = ({ dailyAnalyticsFromPastWeek, isLoading }: DailyAnalyticsProps) => { const dailyRevenueArray = useMemo(() => { - if (!!weeklyStats && weeklyStats?.length > 0) { - const sortedWeeks = weeklyStats?.sort((a, b) => { + if (!!dailyAnalyticsFromPastWeek && dailyAnalyticsFromPastWeek?.length > 0) { + const sortedWeeks = dailyAnalyticsFromPastWeek?.sort((a, b) => { return new Date(a.date).getTime() - new Date(b.date).getTime(); }); return sortedWeeks.map((stat) => stat.totalRevenue); } - }, [weeklyStats]); + }, [dailyAnalyticsFromPastWeek]); const daysOfWeekArr = useMemo(() => { - if (!!weeklyStats && weeklyStats?.length > 0) { - const datesArr = weeklyStats?.map((stat) => { + if (!!dailyAnalyticsFromPastWeek && dailyAnalyticsFromPastWeek?.length > 0) { + const datesArr = dailyAnalyticsFromPastWeek?.map((stat) => { // get day of week, month, and day of month const dateArr = stat.date.toString().split(' '); return dateArr.slice(0, 3).join(' '); }); return datesArr; } - }, [weeklyStats]); + }, [dailyAnalyticsFromPastWeek]); const [state, setState] = useState({ series: [ diff --git a/template/app/src/admin/dashboards/analytics/TotalPayingUsersCard.tsx b/template/app/src/admin/dashboards/analytics/TotalPayingUsersCard.tsx index ecb29840f..bd6abeadc 100644 --- a/template/app/src/admin/dashboards/analytics/TotalPayingUsersCard.tsx +++ b/template/app/src/admin/dashboards/analytics/TotalPayingUsersCard.tsx @@ -1,12 +1,12 @@ import { useMemo } from 'react'; import { cn } from '../../../client/cn'; import { UpArrow, DownArrow } from '../../../client/icons/icons-arrows'; -import { type DailyStatsProps } from '../../../analytics/stats'; +import { type DailyAnalyticsProps } from './types'; -const TotalPayingUsersCard = ({ dailyStats, isLoading }: DailyStatsProps) => { +const TotalPayingUsersCard = ({ dailyAnalytics, isLoading }: DailyAnalyticsProps) => { const isDeltaPositive = useMemo(() => { - return !!dailyStats?.paidUserDelta && dailyStats?.paidUserDelta > 0; - }, [dailyStats]); + return !!dailyAnalytics?.paidUserDelta && dailyAnalytics?.paidUserDelta > 0; + }, [dailyAnalytics]); return (
@@ -32,7 +32,7 @@ const TotalPayingUsersCard = ({ dailyStats, isLoading }: DailyStatsProps) => {
-

{dailyStats?.paidUserCount}

+

{dailyAnalytics?.paidUserCount}

Total Paying Users
@@ -42,8 +42,8 @@ const TotalPayingUsersCard = ({ dailyStats, isLoading }: DailyStatsProps) => { 'text-meta-5': !isDeltaPositive, })} > - {isLoading ? '...' : dailyStats?.paidUserDelta !== 0 ? dailyStats?.paidUserDelta : '-'} - {dailyStats?.paidUserDelta !== 0 ? isDeltaPositive ? : : null} + {isLoading ? '...' : dailyAnalytics?.paidUserDelta !== 0 ? dailyAnalytics?.paidUserDelta : '-'} + {dailyAnalytics?.paidUserDelta !== 0 ? isDeltaPositive ? : : null}
diff --git a/template/app/src/admin/dashboards/analytics/TotalRevenueCard.tsx b/template/app/src/admin/dashboards/analytics/TotalRevenueCard.tsx index 1d3faf054..f0a3c1f09 100644 --- a/template/app/src/admin/dashboards/analytics/TotalRevenueCard.tsx +++ b/template/app/src/admin/dashboards/analytics/TotalRevenueCard.tsx @@ -1,22 +1,23 @@ import { useMemo } from 'react'; import { UpArrow, DownArrow } from '../../../client/icons/icons-arrows'; -import { type DailyStatsProps } from '../../../analytics/stats'; +import { type DailyAnalyticsProps } from './types'; -const TotalRevenueCard = ({dailyStats, weeklyStats, isLoading}: DailyStatsProps) => { + +const TotalRevenueCard = ({dailyAnalytics, dailyAnalyticsFromPastWeek, isLoading}: DailyAnalyticsProps) => { const isDeltaPositive = useMemo(() => { - if (!weeklyStats) return false; - return (weeklyStats[0].totalRevenue - weeklyStats[1]?.totalRevenue) > 0; - }, [weeklyStats]); + if (!dailyAnalyticsFromPastWeek) return false; + return (dailyAnalyticsFromPastWeek[0].totalRevenue - dailyAnalyticsFromPastWeek[1]?.totalRevenue) > 0; + }, [dailyAnalyticsFromPastWeek]); const deltaPercentage = useMemo(() => { - if ( !weeklyStats || weeklyStats.length < 2 || isLoading) return; - if ( weeklyStats[1]?.totalRevenue === 0 || weeklyStats[0]?.totalRevenue === 0 ) return 0; + if ( !dailyAnalyticsFromPastWeek || dailyAnalyticsFromPastWeek.length < 2 || isLoading) return; + if ( dailyAnalyticsFromPastWeek[1]?.totalRevenue === 0 || dailyAnalyticsFromPastWeek[0]?.totalRevenue === 0 ) return 0; - weeklyStats.sort((a, b) => b.id - a.id); + dailyAnalyticsFromPastWeek.sort((a, b) => b.id - a.id); - const percentage = ((weeklyStats[0].totalRevenue - weeklyStats[1]?.totalRevenue) / weeklyStats[1]?.totalRevenue) * 100; + const percentage = ((dailyAnalyticsFromPastWeek[0].totalRevenue - dailyAnalyticsFromPastWeek[1]?.totalRevenue) / dailyAnalyticsFromPastWeek[1]?.totalRevenue) * 100; return Math.floor(percentage); - }, [weeklyStats]); + }, [dailyAnalyticsFromPastWeek]); return (
@@ -46,7 +47,7 @@ const TotalRevenueCard = ({dailyStats, weeklyStats, isLoading}: DailyStatsProps)
-

${dailyStats?.totalRevenue}

+

${dailyAnalytics?.totalRevenue}

Total Revenue
diff --git a/template/app/src/admin/dashboards/analytics/TotalSignupsCard.tsx b/template/app/src/admin/dashboards/analytics/TotalSignupsCard.tsx index f497216a0..392ae2d0b 100644 --- a/template/app/src/admin/dashboards/analytics/TotalSignupsCard.tsx +++ b/template/app/src/admin/dashboards/analytics/TotalSignupsCard.tsx @@ -1,12 +1,13 @@ import { useMemo } from 'react'; import { cn } from '../../../client/cn'; import { UpArrow } from '../../../client/icons/icons-arrows'; -import { type DailyStatsProps } from '../../../analytics/stats'; +import { type DailyAnalyticsProps } from './types'; -const TotalSignupsCard = ({ dailyStats, isLoading }: DailyStatsProps) => { + +const TotalSignupsCard = ({ dailyAnalytics, isLoading }: DailyAnalyticsProps) => { const isDeltaPositive = useMemo(() => { - return !!dailyStats?.userDelta && dailyStats.userDelta > 0; - }, [dailyStats]); + return !!dailyAnalytics?.userDelta && dailyAnalytics.userDelta > 0; + }, [dailyAnalytics]); return (
@@ -36,7 +37,7 @@ const TotalSignupsCard = ({ dailyStats, isLoading }: DailyStatsProps) => {
-

{dailyStats?.userCount}

+

{dailyAnalytics?.userCount}

Total Signups
@@ -46,8 +47,8 @@ const TotalSignupsCard = ({ dailyStats, isLoading }: DailyStatsProps) => { 'text-meta-5': !isDeltaPositive, })} > - {isLoading ? '...' : isDeltaPositive ? dailyStats?.userDelta : '-'} - {!!dailyStats && isDeltaPositive && } + {isLoading ? '...' : isDeltaPositive ? dailyAnalytics?.userDelta : '-'} + {!!dailyAnalytics && isDeltaPositive && }
diff --git a/template/app/src/admin/dashboards/analytics/types.ts b/template/app/src/admin/dashboards/analytics/types.ts new file mode 100644 index 000000000..7470d66fe --- /dev/null +++ b/template/app/src/admin/dashboards/analytics/types.ts @@ -0,0 +1,7 @@ +import { type DailyAnalytics } from 'wasp/entities'; + +export type DailyAnalyticsProps = { + dailyAnalytics?: DailyAnalytics; + dailyAnalyticsFromPastWeek?: DailyAnalytics[]; + isLoading?: boolean; +}; \ No newline at end of file diff --git a/template/app/src/analytics/operations.ts b/template/app/src/analytics/operations.ts index c9f0ec3ee..254f033de 100644 --- a/template/app/src/analytics/operations.ts +++ b/template/app/src/analytics/operations.ts @@ -1,17 +1,17 @@ -import { type DailyStats, type PageViewSource } from 'wasp/entities'; -import { HttpError, prisma } from 'wasp/server'; -import { type GetDailyStats } from 'wasp/server/operations'; +import { type DailyAnalytics, type PageViewSource } from 'wasp/entities'; +import { HttpError } from 'wasp/server'; +import { type GetAnalyticsDataByDay } from 'wasp/server/operations'; -type DailyStatsWithSources = DailyStats & { - sources: PageViewSource[]; +type DailyAnalyticsWithSources = DailyAnalytics & { + pageViewSources: PageViewSource[]; }; -type DailyStatsValues = { - dailyStats: DailyStatsWithSources; - weeklyStats: DailyStatsWithSources[]; +type AnalyticsData = { + todaysAnalyticsData: DailyAnalyticsWithSources; + dailyAnalyticsFromPastWeek: DailyAnalyticsWithSources[]; }; -export const getDailyStats: GetDailyStats = async (_args, context) => { +export const getAnalyticsDataByDay: GetAnalyticsDataByDay = async (_args, context) => { if (!context.user) { throw new HttpError(401, 'Only authenticated users are allowed to perform this operation'); } @@ -25,19 +25,23 @@ export const getDailyStats: GetDailyStats = date: 'desc', }, include: { - sources: true, + pageViewSources: true, }, } as const; - const [dailyStats, weeklyStats] = await prisma.$transaction([ - context.entities.DailyStats.findFirst(statsQuery), - context.entities.DailyStats.findMany({ ...statsQuery, take: 7 }), - ]); + const dailyAnalyticsFromPastWeek = await context.entities.DailyAnalytics.findMany({ ...statsQuery, take: 7 }); + const todaysAnalyticsData = dailyAnalyticsFromPastWeek[0]; - if (!dailyStats) { - console.log('\x1b[34mNote: No daily stats have been generated by the dailyStatsJob yet. \x1b[0m'); - return undefined; + if (!todaysAnalyticsData) { + return handleNoDailyAnalyticsFound(); } - return { dailyStats, weeklyStats }; + return { todaysAnalyticsData, dailyAnalyticsFromPastWeek }; }; + +function handleNoDailyAnalyticsFound() { + const LOG_COLOR_BLUE = '\x1b[34m'; + const LOG_COLOR_RESET = '\x1b[0m'; + console.log(`${LOG_COLOR_BLUE}Note: No daily analytics found. ${LOG_COLOR_RESET}`); + return undefined; +} \ No newline at end of file diff --git a/template/app/src/analytics/stats.ts b/template/app/src/analytics/stats.ts index 57e73986b..6ddc323f0 100644 --- a/template/app/src/analytics/stats.ts +++ b/template/app/src/analytics/stats.ts @@ -1,5 +1,5 @@ -import { type DailyStats } from 'wasp/entities'; -import { type DailyStatsJob } from 'wasp/server/jobs'; +import { type DailyAnalytics } from 'wasp/entities'; +import { type DailyAnalyticsJob } from 'wasp/server/jobs'; import Stripe from 'stripe'; import { stripe } from '../payment/stripe/stripeClient'; import { listOrders } from '@lemonsqueezy/lemonsqueezy.js'; @@ -8,9 +8,7 @@ import { getDailyPageViews, getSources } from './providers/plausibleAnalyticsUti import { paymentProcessor } from '../payment/paymentProcessor'; import { SubscriptionStatus } from '../payment/plans'; -export type DailyStatsProps = { dailyStats?: DailyStats; weeklyStats?: DailyStats[]; isLoading?: boolean }; - -export const calculateDailyStats: DailyStatsJob = async (_args, context) => { +export const calculateDailyAnalytics: DailyAnalyticsJob = async (_args, context) => { const nowUTC = new Date(Date.now()); nowUTC.setUTCHours(0, 0, 0, 0); @@ -18,7 +16,7 @@ export const calculateDailyStats: DailyStatsJob = async (_args, con yesterdayUTC.setUTCDate(yesterdayUTC.getUTCDate() - 1); try { - const yesterdaysStats = await context.entities.DailyStats.findFirst({ + const yesterdaysAnalytics = await context.entities.DailyAnalytics.findFirst({ where: { date: { equals: yesterdayUTC, @@ -37,9 +35,9 @@ export const calculateDailyStats: DailyStatsJob = async (_args, con let userDelta = userCount; let paidUserDelta = paidUserCount; - if (yesterdaysStats) { - userDelta -= yesterdaysStats.userCount; - paidUserDelta -= yesterdaysStats.paidUserCount; + if (yesterdaysAnalytics) { + userDelta -= yesterdaysAnalytics.userCount; + paidUserDelta -= yesterdaysAnalytics.paidUserCount; } let totalRevenue; @@ -56,15 +54,15 @@ export const calculateDailyStats: DailyStatsJob = async (_args, con const { totalViews, prevDayViewsChangePercent } = await getDailyPageViews(); - let dailyStats = await context.entities.DailyStats.findUnique({ + let dailyAnalytics = await context.entities.DailyAnalytics.findUnique({ where: { date: nowUTC, }, }); - if (!dailyStats) { - console.log('No daily stat found for today, creating one...'); - dailyStats = await context.entities.DailyStats.create({ + if (!dailyAnalytics) { + console.log('No daily analytics found for today, creating one...'); + dailyAnalytics = await context.entities.DailyAnalytics.create({ data: { date: nowUTC, totalViews, @@ -78,9 +76,9 @@ export const calculateDailyStats: DailyStatsJob = async (_args, con }); } else { console.log('Daily stat found for today, updating it...'); - dailyStats = await context.entities.DailyStats.update({ + dailyAnalytics = await context.entities.DailyAnalytics.update({ where: { - id: dailyStats.id, + id: dailyAnalytics.id, }, data: { totalViews, @@ -111,7 +109,7 @@ export const calculateDailyStats: DailyStatsJob = async (_args, con date: nowUTC, name: source.source, visitors, - dailyStatsId: dailyStats.id, + dailyAnalyticsId: dailyAnalytics.id, }, update: { visitors, @@ -119,12 +117,12 @@ export const calculateDailyStats: DailyStatsJob = async (_args, con }); } - console.table({ dailyStats }); + console.table({ dailyAnalytics }); } catch (error: any) { - console.error('Error calculating daily stats: ', error); + console.error('Error calculating daily analytics: ', error); await context.entities.Logs.create({ data: { - message: `Error calculating daily stats: ${error?.message}`, + message: `Error calculating daily analytics: ${error?.message}`, level: 'job-error', }, });