fix: firstReviewAt calculation (#77)

* fix: firstReviewAt calculation

* fix: build

Author: waltergalvao

apps/api/src/app/github/services/github-code-review.service.ts

@@ -13,7 +13,7 @@ import {
   PullRequest,
   PullRequestTracking,
 } from "@prisma/client";
-import { parallel, pick } from "radash";
+import { parallel, pick, sort } from "radash";
 import {
   getReviewCompareTime,
   getTimeForReview,
@@ -23,6 +23,7 @@ import { parseNullableISO } from "../../../lib/date";
 import { ResourceNotFoundException } from "../../errors/exceptions/resource-not-found.exception";
 import { SweetQueue, addJob } from "../../../bull-mq/queues";
 import { findWorkspaceByGitInstallationId } from "../../workspaces/services/workspace.service";
+import { parseISO } from "date-fns";
 
 interface Author {
   id: string;
@@ -102,9 +103,10 @@ export const syncCodeReviews = async (
 
   await upsertCodeReviewRequests(pullRequest, reviewRequests);
   await upsertCodeReviews(pullRequest, reviews);
+
   await updatePullRequestTracking(
     pullRequest,
-    reviews,
+    reviewEvents,
     parseNullableISO(firstReviewerRequestedAt)
   );
   await upsertActivityEvents(pullRequest, reviewEvents);
@@ -239,7 +241,12 @@ const fetchPullRequestReviews = async (
     const pullRequest = response.node;
     const { nodes, pageInfo } = pullRequest.reviews;
 
-    for (const review of nodes) {
+    // Sort chronologically
+    const sortedReviews: any[] = sort(nodes, (node) =>
+      parseISO(node.submittedAt).getTime()
+    );
+
+    for (const review of sortedReviews) {
       const authorHandle = review.author.login;
 
       // Ignore self-reviews
@@ -424,12 +431,12 @@ const upsertCodeReviews = async (
 
 const updatePullRequestTracking = async (
   pullRequest: PullRequest & { tracking: PullRequestTracking | null },
-  reviews: ReviewData[],
+  reviewEvents: ReviewDataWithId[],
   firstReviewerRequestedAt: Date | null
 ) => {
-  const firstReviewAt = parseNullableISO(reviews[0]?.createdAt);
+  const firstReviewAt = parseNullableISO(reviewEvents[0]?.createdAt);
   const firstApprovalAt = parseNullableISO(
-    reviews.find((review) => review.state === CodeReviewState.APPROVED)
+    reviewEvents.find((review) => review.state === CodeReviewState.APPROVED)
       ?.createdAt
   );
 

apps/api/src/lib/logger.ts

@@ -24,71 +24,79 @@ const loggableFields = {
   pullRequest: ["id", "title", "number", "createdAt", "updatedAt"],
 };
 
-export const logger = {
-  info: (msg: string, obj?: object) => {
-    const cleanObj = { ...(obj || {}) };
+const sanitize = (payload?: object) => {
+  const sanitizedPayload = { ...(payload || {}) };
 
-    for (const key of Object.keys(cleanObj)) {
-      if (
-        loggableFields[key] &&
-        typeof cleanObj[key] === "object" &&
-        cleanObj[key] !== null
-      ) {
-        cleanObj[key] = pick(cleanObj[key], loggableFields[key]);
-      }
+  for (const key of Object.keys(sanitizedPayload)) {
+    if (
+      loggableFields[key] &&
+      typeof sanitizedPayload[key] === "object" &&
+      sanitizedPayload[key] !== null
+    ) {
+      sanitizedPayload[key] = pick(sanitizedPayload[key], loggableFields[key]);
     }
+  }
+
+  return sanitizedPayload;
+};
 
-    pinoLogger.info(cleanObj, msg);
+export const logger = {
+  info: (msg: string, payload?: object) => {
+    const sanitizedPayload = sanitize(payload);
+
+    pinoLogger.info(sanitizedPayload, msg);
     addBreadcrumb({
       message: msg,
       category: "log",
       level: "info",
-      data: obj,
+      data: payload,
     });
   },
-  warn: (msg: string, obj?: object) => {
-    pinoLogger.warn(obj || {}, msg);
+  warn: (msg: string, payload?: object) => {
+    pinoLogger.warn(payload || {}, msg);
     addBreadcrumb({
       message: msg,
       category: "log",
       level: "warning",
-      data: obj,
+      data: payload,
     });
   },
-  error: (msg: string, obj?: object) => {
-    pinoLogger.error(obj || {}, msg);
+  error: (msg: string, payload?: object) => {
+    pinoLogger.error(payload || {}, msg);
     addBreadcrumb({
       message: msg,
       category: "log",
       level: "error",
-      data: obj,
+      data: payload,
     });
   },
-  debug: (msg: string, obj?: object) => {
-    pinoLogger.debug(obj || {}, msg);
+  debug: (msg: string, payload?: object) => {
+    const cleanObj = sanitize(payload);
+
+    pinoLogger.debug(cleanObj || {}, msg);
     addBreadcrumb({
       message: msg,
       category: "log",
       level: "debug",
-      data: obj,
+      data: payload,
     });
   },
-  trace: (msg: string, obj?: object) => {
-    pinoLogger.trace(obj || {}, msg);
+  trace: (msg: string, payload?: object) => {
+    pinoLogger.trace(payload || {}, msg);
     addBreadcrumb({
       message: msg,
       category: "log",
       level: "debug",
-      data: obj,
+      data: payload,
     });
   },
-  fatal: (msg: string, obj?: object) => {
-    pinoLogger.fatal(obj || {}, msg);
+  fatal: (msg: string, payload?: object) => {
+    pinoLogger.fatal(payload || {}, msg);
     addBreadcrumb({
       message: msg,
       category: "log",
       level: "fatal",
-      data: obj,
+      data: payload,
     });
   },
 };