analytics pair programming review

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. 

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
 

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 {

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 (
     <DefaultLayout user={user}>
       <div className='relative'>
         <div className={cn({
-          'opacity-25': !stats,
+          'opacity-25': !analyticsData,
         })}>
           <div className='grid grid-cols-1 gap-4 md:grid-cols-2 md:gap-6 xl:grid-cols-4 2xl:gap-7.5'>
             <TotalPageViewsCard
-              totalPageViews={stats?.dailyStats.totalViews}
-              prevDayViewsChangePercent={stats?.dailyStats.prevDayViewsChangePercent}
+              totalPageViews={analyticsData?.todaysAnalyticsData.totalViews}
+              prevDayViewsChangePercent={analyticsData?.todaysAnalyticsData.prevDayViewsChangePercent}
             />
             <TotalRevenueCard
-              dailyStats={stats?.dailyStats}
-              weeklyStats={stats?.weeklyStats}
+              dailyAnalytics={analyticsData?.todaysAnalyticsData}
+              dailyAnalyticsFromPastWeek={analyticsData?.dailyAnalyticsFromPastWeek}
               isLoading={isLoading}
             />
-            <TotalPayingUsersCard dailyStats={stats?.dailyStats} isLoading={isLoading} />
-            <TotalSignupsCard dailyStats={stats?.dailyStats} isLoading={isLoading} />
+            <TotalPayingUsersCard dailyAnalytics={analyticsData?.todaysAnalyticsData} isLoading={isLoading} />
+            <TotalSignupsCard dailyAnalytics={analyticsData?.todaysAnalyticsData} isLoading={isLoading} />
           </div>
 
           <div className='mt-4 grid grid-cols-12 gap-4 md:mt-6 md:gap-6 2xl:mt-7.5 2xl:gap-7.5'>
-            <RevenueAndProfitChart weeklyStats={stats?.weeklyStats} isLoading={isLoading} />
+            <RevenueAndProfitChart dailyAnalyticsFromPastWeek={analyticsData?.dailyAnalyticsFromPastWeek} isLoading={isLoading} />
 
             <div className='col-span-12 xl:col-span-8'>
-              <SourcesTable sources={stats?.dailyStats?.sources} />
+              <SourcesTable sources={analyticsData?.todaysAnalyticsData?.pageViewSources} />
             </div>
           </div>
         </div>
 
-        {!stats && (
+        {!analyticsData && (
           <div className='absolute inset-0 flex items-start justify-center bg-white/50 dark:bg-boxdark-2/50'>
             <div className='rounded-lg bg-white p-8 shadow-lg dark:bg-boxdark'>
               <p className='text-2xl font-bold text-boxdark dark:text-white'>
-                No daily stats generated yet
-              </p>
-              <p className='mt-2 text-sm text-bodydark2'>
-                Stats will appear here once the daily stats job has run
+                No daily analytics found yet
               </p>
             </div>
           </div>

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<ChartOneState>({
     series: [

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 (
     <div className='rounded-sm border border-stroke bg-white py-6 px-7.5 shadow-default dark:border-strokedark dark:bg-boxdark'>
@@ -32,7 +32,7 @@ const TotalPayingUsersCard = ({ dailyStats, isLoading }: DailyStatsProps) => {
 
       <div className='mt-4 flex items-end justify-between'>
         <div>
-          <h4 className='text-title-md font-bold text-black dark:text-white'>{dailyStats?.paidUserCount}</h4>
+          <h4 className='text-title-md font-bold text-black dark:text-white'>{dailyAnalytics?.paidUserCount}</h4>
           <span className='text-sm font-medium'>Total Paying Users</span>
         </div>
 
@@ -42,8 +42,8 @@ const TotalPayingUsersCard = ({ dailyStats, isLoading }: DailyStatsProps) => {
             'text-meta-5': !isDeltaPositive,
           })}
         >
-          {isLoading ? '...' : dailyStats?.paidUserDelta !== 0 ? dailyStats?.paidUserDelta : '-'}
-          {dailyStats?.paidUserDelta !== 0 ? isDeltaPositive ? <UpArrow /> : <DownArrow /> : null}
+          {isLoading ? '...' : dailyAnalytics?.paidUserDelta !== 0 ? dailyAnalytics?.paidUserDelta : '-'}
+          {dailyAnalytics?.paidUserDelta !== 0 ? isDeltaPositive ? <UpArrow /> : <DownArrow /> : null}
         </span>
       </div>
     </div>

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 (
     <div className='rounded-sm border border-stroke bg-white py-6 px-7.5 shadow-default dark:border-strokedark dark:bg-boxdark'>
@@ -46,7 +47,7 @@ const TotalRevenueCard = ({dailyStats, weeklyStats, isLoading}: DailyStatsProps)
 
       <div className='mt-4 flex items-end justify-between'>
         <div>
-          <h4 className='text-title-md font-bold text-black dark:text-white'>${dailyStats?.totalRevenue}</h4>
+          <h4 className='text-title-md font-bold text-black dark:text-white'>${dailyAnalytics?.totalRevenue}</h4>
           <span className='text-sm font-medium'>Total Revenue</span>
         </div>
 

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 (
     <div className='rounded-sm border border-stroke bg-white py-6 px-7.5 shadow-default dark:border-strokedark dark:bg-boxdark'>
@@ -36,7 +37,7 @@ const TotalSignupsCard = ({ dailyStats, isLoading }: DailyStatsProps) => {
 
       <div className='mt-4 flex items-end justify-between'>
         <div>
-          <h4 className='text-title-md font-bold text-black dark:text-white'>{dailyStats?.userCount}</h4>
+          <h4 className='text-title-md font-bold text-black dark:text-white'>{dailyAnalytics?.userCount}</h4>
           <span className='text-sm font-medium'>Total Signups</span>
         </div>
 
@@ -46,8 +47,8 @@ const TotalSignupsCard = ({ dailyStats, isLoading }: DailyStatsProps) => {
             'text-meta-5': !isDeltaPositive,
           })}
         >
-          {isLoading ? '...' : isDeltaPositive ? dailyStats?.userDelta : '-'}
-          {!!dailyStats && isDeltaPositive && <UpArrow />}
+          {isLoading ? '...' : isDeltaPositive ? dailyAnalytics?.userDelta : '-'}
+          {!!dailyAnalytics && isDeltaPositive && <UpArrow />}
         </span>
       </div>
     </div>

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;
+}; 

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<void, DailyStatsValues | undefined> = async (_args, context) => {
+export const getAnalyticsDataByDay: GetAnalyticsDataByDay<void, AnalyticsData | undefined> = 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<void, DailyStatsValues | undefined> =
       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;
+}