Refactor activity totals handling and improve logging; update database schema for assignee tracking

austenstone · austenstone · commit e5f1c524e309 · 2025-01-31T15:41:00.000-05:00
diff --git a/backend/src/controllers/adoption.controller.ts b/backend/src/controllers/adoption.controller.ts
@@ -33,7 +33,7 @@ class AdoptionController {
 
   async getAdoptionTotals(req: Request, res: Response): Promise<void> {
     try {
-      const totals = await SeatsService.getMembersActivityTotals(req.query);
+      const totals = await SeatsService.getMembersActivityTotals2(req.query);
       res.status(200).json(totals);
     } catch (error) {
       res.status(500).json(error);
diff --git a/backend/src/controllers/seats.controller.ts b/backend/src/controllers/seats.controller.ts
@@ -42,15 +42,6 @@ class SeatsController {
       res.status(500).json(error);
     }
   }
-
-  async getActivityTotals(req: Request, res: Response): Promise<void> {
-    try {
-      const totals = await SeatsService.getMembersActivityTotals(req.query);
-      res.status(200).json(totals);
-    } catch (error) {
-      res.status(500).json(error);
-    }
-  }
 }
 
 export default new SeatsController();
diff --git a/backend/src/database.ts b/backend/src/database.ts
@@ -1,6 +1,6 @@
 import updateDotenv from 'update-dotenv';
 import logger from './services/logger.js';
-import mongoose, { Schema } from 'mongoose';
+import mongoose, { mongo, Schema } from 'mongoose';
 import util from 'util';
 
 class Database {
@@ -34,10 +34,11 @@ class Database {
       });
       mongoose.set('debug', (collectionName: string, methodName: string, ...methodArgs: unknown[]) => {
         const msgMapper = (m: unknown) => {
-          return util.inspect(m, false, 10, true)
+            return util.inspect(m, false, 10, true)
             .replace(/\n/g, '').replace(/\s{2,}/g, ' ');
         };
-        logger.debug(`\x1B[0;36mMongoose:\x1B[0m: ${collectionName}.${methodName}` + `(${methodArgs.map(msgMapper).join(', ')})`);
+        // logger.debug(`\x1B[0;36mMongoose:\x1B[0m: ${collectionName}.${methodName}` + `(${methodArgs.map(msgMapper).join(', ')})`);
+        logger.debug(`[Mongoose] ${collectionName}.${methodName}(${methodArgs.map(msgMapper).join(', ')})`);
       });
 
     } catch (error) {
@@ -241,11 +242,7 @@ class Database {
     });
 
     seatsSchema.index({ org: 1, queryAt: 1, last_activity_at: -1 });
-    seatsSchema.index({ team: 1, member: 1 }, { unique: true });
-    seatsSchema.index({ createdAt: 1 });
-    seatsSchema.index({ queryAt: 1 });
-    seatsSchema.index({ assignee: 1 });
-    seatsSchema.index({ assignee_id: 1 });
+    seatsSchema.index({ org: 1, team: 1, queryAt: 1, assignee_id: 1 }, { unique: true });
 
     mongoose.model('Seats', seatsSchema);
 
@@ -286,10 +283,12 @@ class Database {
 
     const activityTotalsSchema = new mongoose.Schema({
       org: String,
-      member_id: {
+      assignee: {
         type: Schema.Types.ObjectId,
         ref: 'Member'
       },
+      assignee_id: Number,
+      assignee_login: String,
       date: Date,
       total_active_time_ms: Number,
       last_activity_at: Date,
@@ -298,7 +297,7 @@ class Database {
       timestamps: true
     });
     
-    activityTotalsSchema.index({ org: 1, date: 1, member_id: 1 }, { unique: true });
+    activityTotalsSchema.index({ org: 1, date: 1, assignee: 1 }, { unique: true });
     activityTotalsSchema.index({ date: 1 }); // For date range queries
     
     mongoose.model('ActivityTotals', activityTotalsSchema);
diff --git a/backend/src/services/copilot.seats.service.ts b/backend/src/services/copilot.seats.service.ts
@@ -35,16 +35,13 @@ class SeatsService {
       .sort({ queryAt: -1 })  // -1 for descending order
       .select('queryAt');
 
-    //console.log(`Latest query at: ${latestQuery?.queryAt}`);
-
     const seats = await Seats.find({
       ...(org ? { org } : {}),
       queryAt: latestQuery?.queryAt
     })
       .populate('assignee')
       .sort({ last_activity_at: -1 });  // DESC ordering ⬇️
 
-    console.log(`Found ${seats.length} seats for ${org || 'all orgs'} at ${latestQuery?.queryAt}`);
     return seats;
   }
 
@@ -124,7 +121,7 @@ class SeatsService {
       }
     }));
     await Members.bulkWrite(memberUpdates);
-    
+
     const updatedMembers = await Members.find({
       org,
       id: { $in: data.map(seat => seat.assignee.id) }
@@ -140,7 +137,6 @@ class SeatsService {
       assignee: updatedMembers.find(m => m.id === seat.assignee.id)?._id
     }));
     const seatResults = await Seats.insertMany(seatsData);
-    console.log(`Inserted ${seatResults.length} seats for ${org} at ${queryAt}`);
 
     const adoptionData = {
       enterprise: null,
@@ -160,33 +156,65 @@ class SeatsService {
     await adoptionService.createAdoption(adoptionData);
 
     const today = new Date(queryAt);
-    today.setUTCHours(0,0,0,0);
+    // add 1 to day
+    // today.setDate(today.getDate() + 1);
+    today.setUTCHours(0, 0, 0, 0);
     const activityUpdates = seatResults.map(seat => {
-      if (!seat.last_activity_at) return null;
-  
+      if (!seat.assignee_login) {
+        console.warn('Missing assignee_login for seat:', seat);
+        return null;
+      }
       return {
         updateOne: {
           filter: {
             org,
-            member_id: seat.assignee,
-            date: today,
-            $or: [
-              { last_activity_at: { $lt: seat.last_activity_at } },
-              { last_activity_at: { $exists: false } }
-            ]
+            assignee: seat.assignee,
+            assignee_id: seat.assignee_id,
+            assignee_login: seat.assignee_login,
+            date: today
           },
-          update: {
-            $inc: { total_active_time_ms: 30 * 60 * 1000 }, // 30 min per activity
+          update: [{
             $set: {
-              last_activity_at: seat.last_activity_at,
-              last_activity_editor: seat.last_activity_editor
+              total_active_time_ms: {
+                $cond: {
+                  if: { $eq: [seat.last_activity_at, null] },
+                  then: { $ifNull: ["$total_active_time_ms", 0] },
+                  else: {
+                    $add: [
+                      { $ifNull: ["$total_active_time_ms", 0] },
+                      {
+                        $cond: {
+                          if: {
+                            $and: [
+                              {
+                                $or: [
+                                  { $eq: ["$last_activity_at", null] },
+                                  { $lt: ["$last_activity_at", seat.last_activity_at] }
+                                ]
+                              },
+                              { $gt: [seat.last_activity_at, today] }
+                            ]
+                          },
+                          then: 1,
+                          else: 0
+                        }
+                      }
+                    ]
+                  }
+                }
+              }
             }
-          },
+          }, {
+            $set: {
+              last_activity_editor: seat.last_activity_editor,
+              last_activity_at: seat.last_activity_at
+            }
+          }],
           upsert: true
         }
       };
     }).filter(update => update !== null);
-  
+
     if (activityUpdates.length > 0) {
       await ActivityTotals.bulkWrite(activityUpdates);
     }
@@ -351,39 +379,49 @@ class SeatsService {
   }) {
     const ActivityTotals = mongoose.model('ActivityTotals');
     const { org, since, until } = params;
-  
+
     const match: any = {};
     if (org) match.org = org;
-    if (since) match.date = { $gte: new Date(since) };
-    if (until) match.date = { ...match.date, $lte: new Date(until) };
-  
+    if (since || until) {
+      match.date = {
+        ...(since && { $gte: new Date(since) }),
+        ...(until && { $lte: new Date(until) })
+      };
+    }
+
+    console.log('getting totals', match);
     const totals = await ActivityTotals.aggregate([
+      // Match documents within date range and org
       { $match: match },
+      // First group by date AND assignee_login
       {
         $group: {
-          _id: "$member_id",
-          total_time: { $sum: "$total_active_time_ms" }
+          _id: {
+            date: "$date",
+            login: "$assignee_login"
+          },
+          daily_time: { $sum: "$total_active_time_ms" }
         }
       },
+      // Then group by just assignee_login to sum across days
       {
-        $lookup: {
-          from: 'members',
-          localField: '_id',
-          foreignField: '_id',
-          as: 'member'
+        $group: {
+          _id: "$_id.login",
+          total_time: { $sum: "$daily_time" }
         }
       },
-      { $unwind: '$member' },
+      // Sort by total time descending
       { $sort: { total_time: -1 } },
+      // Project final fields
       {
         $project: {
           _id: 0,
-          login: '$member.login',
+          login: '$_id',
           total_time: 1
         }
       }
     ]);
-  
+
     return totals.map(t => [t.login, t.total_time]);
   }
 }
diff --git a/backend/src/services/logger.ts b/backend/src/services/logger.ts
@@ -40,6 +40,12 @@ const logger = bunyan.createLogger({
       level: 'error',
       stream: process.stderr
     },
+    {
+      path: `${logsDir}/error.json`,
+      period: '1d',
+      count: 14,
+      level: 'error'
+    },
     {
       path: `${logsDir}/debug.json`,
       period: '1d',
diff --git a/backend/src/services/query.service.ts b/backend/src/services/query.service.ts
@@ -4,7 +4,7 @@ import { insertUsage } from '../models/usage.model.js';
 import SeatService, { SeatEntry } from '../services/copilot.seats.service.js';
 import { App, Octokit } from 'octokit';
 import { MetricDailyResponseType } from '../models/metrics.model.js';
-import mongoose from 'mongoose';
+import mongoose, { MongooseError } from 'mongoose';
 import metricsService from './metrics.service.js';
 import teamsService from './teams.service.js';
 import adoptionService from './adoption.service.js';
@@ -175,6 +175,10 @@ class QueryService {
       return result;
     } catch (error) {
       logger.debug(error)
+      if (error instanceof Error) {
+        logger.error('Error updating seat assignments', error.message);
+        return;
+      }
       logger.error('Error querying copilot seat assignments');
     }
   }
diff --git a/frontend/src/app/main/copilot/copilot-dashboard/dashboard-card/active-users-chart/active-users-chart.component.ts b/frontend/src/app/main/copilot/copilot-dashboard/dashboard-card/active-users-chart/active-users-chart.component.ts
@@ -40,7 +40,7 @@ export class ActiveUsersChartComponent implements OnChanges {
       pointFormat: '<span style="padding:0">{point.y: .1f} hours</span>',
       headerFormat: '',
       formatter: function () {
-        const hours = (this.y || 0) / (1000 * 60 * 60); // Convert ms to hours
+        const hours = (this.y || 0); // Convert ms to hours
         return `<span style="padding:0">@${this.key}</span><br>
         <span style="padding:0">${hours.toFixed(1)} hours</span>`;
       },

Original file line number	Diff line number	Diff line change
`@@ -42,15 +42,6 @@ class SeatsController {`
`42`	`42`	`res.status(500).json(error);`
`43`	`43`	`}`
`44`	`44`	`}`
`45`		`-`
`46`		`- async getActivityTotals(req: Request, res: Response): Promise<void> {`
`47`		`- try {`
`48`		`- const totals = await SeatsService.getMembersActivityTotals(req.query);`
`49`		`- res.status(200).json(totals);`
`50`		`- } catch (error) {`
`51`		`- res.status(500).json(error);`
`52`		`- }`
`53`		`- }`
`54`	`45`	`}`
`55`	`46`
`56`	`47`	`export default new SeatsController();`