Leaderboard calc: update profit even when portfolio didn't change (#845)

* Leaderboard calc: remove didProfitChange optimization that was incorrect

* Put back didPortfolioChange for deciding whether to create new history doc.

functions/src/update-metrics.ts

@@ -1,13 +1,12 @@
 import * as functions from 'firebase-functions'
 import * as admin from 'firebase-admin'
-import { groupBy, isEmpty, keyBy, sum, sumBy } from 'lodash'
+import { groupBy, isEmpty, keyBy, last, sortBy, sum, sumBy } from 'lodash'
 import { getValues, log, logMemory, writeAsync } from './utils'
 import { Bet } from '../../common/bet'
 import { Contract } from '../../common/contract'
 import { PortfolioMetrics, User } from '../../common/user'
 import { calculatePayout } from '../../common/calculate'
 import { DAY_MS } from '../../common/util/time'
-import { last } from 'lodash'
 import { getLoanUpdates } from '../../common/loans'
 
 const firestore = admin.firestore()
@@ -88,23 +87,20 @@ export const updateMetricsCore = async () => {
       currentBets
     )
     const lastPortfolio = last(portfolioHistory)
-    const didProfitChange =
+    const didPortfolioChange =
       lastPortfolio === undefined ||
       lastPortfolio.balance !== newPortfolio.balance ||
       lastPortfolio.totalDeposits !== newPortfolio.totalDeposits ||
       lastPortfolio.investmentValue !== newPortfolio.investmentValue
 
-    const newProfit = calculateNewProfit(
+    const newProfit = calculateNewProfit(portfolioHistory, newPortfolio)
-      portfolioHistory,
+
-      newPortfolio,
-      didProfitChange
-    )
     return {
       user,
       newCreatorVolume,
       newPortfolio,
       newProfit,
-      didProfitChange,
+      didPortfolioChange,
     }
   })
 
@@ -120,16 +116,20 @@ export const updateMetricsCore = async () => {
   const nextLoanByUser = keyBy(userPayouts, (payout) => payout.user.id)
 
   const userUpdates = userMetrics.map(
-    ({ user, newCreatorVolume, newPortfolio, newProfit, didProfitChange }) => {
+    ({
+      user,
+      newCreatorVolume,
+      newPortfolio,
+      newProfit,
+      didPortfolioChange,
+    }) => {
       const nextLoanCached = nextLoanByUser[user.id]?.payout ?? 0
       return {
         fieldUpdates: {
           doc: firestore.collection('users').doc(user.id),
           fields: {
             creatorVolumeCached: newCreatorVolume,
-            ...(didProfitChange && {
+            profitCached: newProfit,
-              profitCached: newProfit,
-            }),
             nextLoanCached,
           },
         },
@@ -140,11 +140,7 @@ export const updateMetricsCore = async () => {
             .doc(user.id)
             .collection('portfolioHistory')
             .doc(),
-          fields: {
+          fields: didPortfolioChange ? newPortfolio : {},
-            ...(didProfitChange && {
-              ...newPortfolio,
-            }),
-          },
         },
       }
     }
@@ -171,15 +167,15 @@ const computeVolume = (contractBets: Bet[], since: number) => {
 
 const calculateProfitForPeriod = (
   startTime: number,
-  portfolioHistory: PortfolioMetrics[],
+  descendingPortfolio: PortfolioMetrics[],
   currentProfit: number
 ) => {
-  const startingPortfolio = [...portfolioHistory]
+  const startingPortfolio = descendingPortfolio.find(
-    .reverse() // so we search in descending order (most recent first), for efficiency
+    (p) => p.timestamp < startTime
-    .find((p) => p.timestamp < startTime)
+  )
 
   if (startingPortfolio === undefined) {
-    return 0
+    return currentProfit
   }
 
   const startingProfit = calculateTotalProfit(startingPortfolio)
@@ -233,28 +229,28 @@ const calculateNewPortfolioMetrics = (
 
 const calculateNewProfit = (
   portfolioHistory: PortfolioMetrics[],
-  newPortfolio: PortfolioMetrics,
+  newPortfolio: PortfolioMetrics
-  didProfitChange: boolean
 ) => {
-  if (!didProfitChange) {
-    return {} // early return for performance
-  }
-
   const allTimeProfit = calculateTotalProfit(newPortfolio)
+  const descendingPortfolio = sortBy(
+    portfolioHistory,
+    (p) => p.timestamp
+  ).reverse()
+
   const newProfit = {
     daily: calculateProfitForPeriod(
       Date.now() - 1 * DAY_MS,
-      portfolioHistory,
+      descendingPortfolio,
       allTimeProfit
     ),
     weekly: calculateProfitForPeriod(
       Date.now() - 7 * DAY_MS,
-      portfolioHistory,
+      descendingPortfolio,
       allTimeProfit
     ),
     monthly: calculateProfitForPeriod(
       Date.now() - 30 * DAY_MS,
-      portfolioHistory,
+      descendingPortfolio,
       allTimeProfit
     ),
     allTime: allTimeProfit,