From 49f7e83839365a737ac4131e58ba863a9fddbe0e Mon Sep 17 00:00:00 2001
From: Adam Setch <adam.setch@outlook.com>
Date: Tue, 10 Sep 2024 09:23:45 -0400
Subject: [PATCH] feat: improve robustness of subject user handling

Signed-off-by: Adam Setch <adam.setch@outlook.com>
---
 src/utils/subject.test.ts | 31 +++++++++++++++++++++++
 src/utils/subject.ts      | 52 +++++++++++++++++++++------------------
 2 files changed, 59 insertions(+), 24 deletions(-)

diff --git a/src/utils/subject.test.ts b/src/utils/subject.test.ts
index 7aa7d814f..ae27b6a84 100644
--- a/src/utils/subject.test.ts
+++ b/src/utils/subject.test.ts
@@ -18,6 +18,7 @@ import {
   getCheckSuiteAttributes,
   getGitifySubjectDetails,
   getLatestReviewForReviewers,
+  getSubjectUser,
   getWorkflowRunAttributes,
   parseLinkedIssuesFromPr,
 } from './subject';
@@ -1332,6 +1333,36 @@ describe('utils/subject.ts', () => {
       expect(result).toBeNull();
     });
   });
+
+  describe('getSubjectUser', () => {
+    it('returns null when all users are null', () => {
+      const result = getSubjectUser([null, null]);
+
+      expect(result).toBeNull();
+    });
+
+    it('returns first user', () => {
+      const result = getSubjectUser([mockAuthor, null]);
+
+      expect(result).toEqual({
+        login: mockAuthor.login,
+        html_url: mockAuthor.html_url,
+        avatar_url: mockAuthor.avatar_url,
+        type: mockAuthor.type,
+      });
+    });
+
+    it('returns second user if first is null', () => {
+      const result = getSubjectUser([null, mockAuthor]);
+
+      expect(result).toEqual({
+        login: mockAuthor.login,
+        html_url: mockAuthor.html_url,
+        avatar_url: mockAuthor.avatar_url,
+        type: mockAuthor.type,
+      });
+    });
+  });
 });
 
 function mockDiscussionNode(
diff --git a/src/utils/subject.ts b/src/utils/subject.ts
index 065a71940..08d65abcf 100644
--- a/src/utils/subject.ts
+++ b/src/utils/subject.ts
@@ -139,12 +139,7 @@ async function getGitifySubjectForCommit(
 
   return {
     state: null,
-    user: {
-      login: user.login,
-      html_url: user.html_url,
-      avatar_url: user.avatar_url,
-      type: user.type,
-    },
+    user: getSubjectUser([user]),
   };
 }
 
@@ -230,12 +225,7 @@ async function getGitifySubjectForIssue(
   return {
     number: issue.number,
     state: issue.state_reason ?? issue.state,
-    user: {
-      login: issueCommentUser?.login ?? issue.user.login,
-      html_url: issueCommentUser?.html_url ?? issue.user.html_url,
-      avatar_url: issueCommentUser?.avatar_url ?? issue.user.avatar_url,
-      type: issueCommentUser?.type ?? issue.user.type,
-    },
+    user: getSubjectUser([issueCommentUser, issue.user]),
     comments: issue.comments,
     labels: issue.labels?.map((label) => label.name) ?? [],
     milestone: issue.milestone,
@@ -277,12 +267,7 @@ async function getGitifySubjectForPullRequest(
   return {
     number: pr.number,
     state: prState,
-    user: {
-      login: prCommentUser?.login ?? pr.user.login,
-      html_url: prCommentUser?.html_url ?? pr.user.html_url,
-      avatar_url: prCommentUser?.avatar_url ?? pr.user.avatar_url,
-      type: prCommentUser?.type ?? pr.user.type,
-    },
+    user: getSubjectUser([prCommentUser, pr.user]),
     reviews: reviews,
     comments: pr.comments,
     labels: pr.labels?.map((label) => label.name) ?? [],
@@ -371,12 +356,7 @@ async function getGitifySubjectForRelease(
 
   return {
     state: null,
-    user: {
-      login: release.author.login,
-      html_url: release.author.html_url,
-      avatar_url: release.author.avatar_url,
-      type: release.author.type,
-    },
+    user: getSubjectUser([release.author]),
   };
 }
 
@@ -428,3 +408,27 @@ function getWorkflowRunStatus(statusDisplayName: string): CheckSuiteStatus {
       return null;
   }
 }
+
+/**
+ * Construct the notification subject user based on an order prioritized list of users
+ * @param users array of users in order or priority
+ * @returns the subject user
+ */
+export function getSubjectUser(users: User[]): SubjectUser {
+  let subjectUser: SubjectUser = null;
+
+  for (const user of users) {
+    if (user) {
+      subjectUser = {
+        login: user.login,
+        html_url: user.html_url,
+        avatar_url: user.avatar_url,
+        type: user.type,
+      };
+
+      return subjectUser;
+    }
+  }
+
+  return subjectUser;
+}