Merge pull request desktop#16097 from desktop/pr-comment-notifications-support

Basic support for Pull Request comment notifications

Author: sergiou87

app/src/lib/api.ts

@@ -742,6 +742,79 @@ export class API {
     }
   }
 
+  /**
+   * Fetch an issue comment (i.e. a comment on an issue or pull request).
+   *
+   * @param owner The owner of the repository
+   * @param name The name of the repository
+   * @param commentId The ID of the comment
+   *
+   * @returns The comment if it was found, null if it wasn't, or an error
+   * occurred.
+   */
+  public async fetchIssueComment(
+    owner: string,
+    name: string,
+    commentId: string
+  ): Promise<IAPIComment | null> {
+    try {
+      const response = await this.request(
+        'GET',
+        `repos/${owner}/${name}/issues/comments/${commentId}`
+      )
+      if (response.status === HttpStatusCode.NotFound) {
+        log.warn(
+          `fetchIssueComment: '${owner}/${name}/issues/comments/${commentId}' returned a 404`
+        )
+        return null
+      }
+      return await parsedResponse<IAPIComment>(response)
+    } catch (e) {
+      log.warn(
+        `fetchIssueComment: an error occurred for '${owner}/${name}/issues/comments/${commentId}'`,
+        e
+      )
+      return null
+    }
+  }
+
+  /**
+   * Fetch a pull request review comment (i.e. a comment that was posted as part
+   * of a review of a pull request).
+   *
+   * @param owner The owner of the repository
+   * @param name The name of the repository
+   * @param commentId The ID of the comment
+   *
+   * @returns The comment if it was found, null if it wasn't, or an error
+   * occurred.
+   */
+  public async fetchPullRequestReviewComment(
+    owner: string,
+    name: string,
+    commentId: string
+  ): Promise<IAPIComment | null> {
+    try {
+      const response = await this.request(
+        'GET',
+        `repos/${owner}/${name}/pulls/comments/${commentId}`
+      )
+      if (response.status === HttpStatusCode.NotFound) {
+        log.warn(
+          `fetchPullRequestReviewComment: '${owner}/${name}/pulls/comments/${commentId}' returned a 404`
+        )
+        return null
+      }
+      return await parsedResponse<IAPIComment>(response)
+    } catch (e) {
+      log.warn(
+        `fetchPullRequestReviewComment: an error occurred for '${owner}/${name}/pulls/comments/${commentId}'`,
+        e
+      )
+      return null
+    }
+  }
+
   /** Fetch a repo by its owner and name. */
   public async fetchRepository(
     owner: string,
@@ -1095,7 +1168,7 @@ export class API {
     }
   }
 
-  /** Fetches all comments from a given pull request. */
+  /** Fetches all review comments from a given pull request. */
   public async fetchPullRequestComments(
     owner: string,
     name: string,
@@ -1114,6 +1187,25 @@ export class API {
     }
   }
 
+  /** Fetches all comments from a given issue. */
+  public async fetchIssueComments(
+    owner: string,
+    name: string,
+    issueNumber: string
+  ) {
+    try {
+      const path = `/repos/${owner}/${name}/issues/${issueNumber}/comments`
+      const response = await this.request('GET', path)
+      return await parsedResponse<IAPIComment[]>(response)
+    } catch (e) {
+      log.debug(
+        `failed fetching issue comments for ${owner}/${name}/issues/${issueNumber}`,
+        e
+      )
+      return []
+    }
+  }
+
   /**
    * Get the combined status for the given ref.
    */

app/src/lib/feature-flag.ts

@@ -127,3 +127,7 @@ export function enablePreventClosingWhileUpdating(): boolean {
 export function enablePushPullFetchDropdown(): boolean {
   return enableBetaFeatures()
 }
+
+export function enablePullRequestCommentNotifications(): boolean {
+  return enableDevelopmentFeatures()
+}

app/src/lib/stores/app-store.ts

@@ -95,6 +95,7 @@ import {
   IAPIOrganization,
   getEndpointForRepository,
   IAPIFullRepository,
+  IAPIComment,
 } from '../api'
 import { shell } from '../app-shell'
 import {
@@ -602,6 +603,10 @@ export class AppStore extends TypedBaseStore<IAppState> {
       this.onPullRequestReviewSubmitNotification
     )
 
+    this.notificationsStore.onPullRequestCommentNotification(
+      this.onPullRequestCommentNotification
+    )
+
     onShowInstallingUpdate(this.onShowInstallingUpdate)
   }
 
@@ -7324,6 +7329,33 @@ export class AppStore extends TypedBaseStore<IAppState> {
     })
   }
 
+  private onPullRequestCommentNotification = async (
+    repository: RepositoryWithGitHubRepository,
+    pullRequest: PullRequest,
+    comment: IAPIComment
+  ) => {
+    const selectedRepository =
+      this.selectedRepository ?? (await this._selectRepository(repository))
+
+    const state = this.repositoryStateCache.get(repository)
+
+    const { branchesState } = state
+    const { tip } = branchesState
+    const currentBranch = tip.kind === TipState.Valid ? tip.branch : null
+
+    return this._showPopup({
+      type: PopupType.PullRequestComment,
+      shouldCheckoutBranch:
+        currentBranch !== null && currentBranch.name !== pullRequest.head.ref,
+      shouldChangeRepository:
+        selectedRepository === null ||
+        selectedRepository.hash !== repository.hash,
+      comment,
+      pullRequest,
+      repository,
+    })
+  }
+
   public async _startPullRequest(repository: Repository) {
     const { tip, defaultBranch } =
       this.repositoryStateCache.get(repository).branchesState

app/src/lib/stores/notifications-debug-store.ts

@@ -76,42 +76,19 @@ export class NotificationsDebugStore {
 
     const ghRepository = repository.gitHubRepository
 
-    const issueComments = await api.fetchPullRequestComments(
+    const issueComments = await api.fetchIssueComments(
       ghRepository.owner.login,
       ghRepository.name,
       pullRequestNumber.toString()
     )
 
-    const issueCommentIds = new Set(issueComments.map(c => c.id))
-
-    // Fetch review comments of type COMMENTED and with no body
-    const allReviews = await api.fetchPullRequestReviews(
+    const reviewComments = await api.fetchPullRequestComments(
       ghRepository.owner.login,
       ghRepository.name,
       pullRequestNumber.toString()
     )
 
-    const commentedReviewsWithNoBody = allReviews.filter(
-      review => review.state === 'COMMENTED' && !review.body
-    )
-
-    const allReviewComments = await Promise.all(
-      commentedReviewsWithNoBody.map(review =>
-        api.fetchPullRequestReviewComments(
-          ghRepository.owner.login,
-          ghRepository.name,
-          pullRequestNumber.toString(),
-          review.id.toString()
-        )
-      )
-    )
-
-    // Only reviews with one comment, and that comment is not an issue comment
-    const singleReviewComments = allReviewComments
-      .flatMap(comments => (comments.length === 1 ? comments : []))
-      .filter(comment => !issueCommentIds.has(comment.id))
-
-    return [...issueComments, ...singleReviewComments]
+    return [...issueComments, ...reviewComments]
   }
 
   /** Simulate a notification for the given pull request review. */

app/src/lib/stores/notifications-store.ts

@@ -7,7 +7,7 @@ import {
 } from '../../models/repository'
 import { ForkContributionTarget } from '../../models/workflow-preferences'
 import { getPullRequestCommitRef, PullRequest } from '../../models/pull-request'
-import { API, APICheckConclusion } from '../api'
+import { API, APICheckConclusion, IAPIComment } from '../api'
 import {
   createCombinedCheckFromChecks,
   getLatestCheckRunsByName,
@@ -24,6 +24,7 @@ import {
   AliveStore,
   DesktopAliveEvent,
   IDesktopChecksFailedAliveEvent,
+  IDesktopPullRequestCommentAliveEvent,
   IDesktopPullRequestReviewSubmitAliveEvent,
 } from './alive-store'
 import { setBoolean, getBoolean } from '../local-storage'
@@ -36,6 +37,7 @@ import {
   ValidNotificationPullRequestReview,
 } from '../valid-notification-pull-request-review'
 import { NotificationCallback } from 'desktop-notifications/dist/notification-callback'
+import { enablePullRequestCommentNotifications } from '../feature-flag'
 
 type OnChecksFailedCallback = (
   repository: RepositoryWithGitHubRepository,
@@ -51,6 +53,12 @@ type OnPullRequestReviewSubmitCallback = (
   review: ValidNotificationPullRequestReview
 ) => void
 
+type OnPullRequestCommentCallback = (
+  repository: RepositoryWithGitHubRepository,
+  pullRequest: PullRequest,
+  comment: IAPIComment
+) => void
+
 /**
  * The localStorage key for whether the user has enabled high-signal
  * notifications.
@@ -72,6 +80,8 @@ export class NotificationsStore {
   private onChecksFailedCallback: OnChecksFailedCallback | null = null
   private onPullRequestReviewSubmitCallback: OnPullRequestReviewSubmitCallback | null =
     null
+  private onPullRequestCommentCallback: OnPullRequestCommentCallback | null =
+    null
   private cachedCommits: Map<string, Commit> = new Map()
   private skipCommitShas: Set<string> = new Set()
   private skipCheckRuns: Set<number> = new Set()
@@ -119,9 +129,90 @@ export class NotificationsStore {
         return this.handleChecksFailedEvent(e, skipNotification)
       case 'pr-review-submit':
         return this.handlePullRequestReviewSubmitEvent(e, skipNotification)
+      case 'pr-comment':
+        return this.handlePullRequestCommentEvent(e, skipNotification)
     }
   }
 
+  private async handlePullRequestCommentEvent(
+    event: IDesktopPullRequestCommentAliveEvent,
+    skipNotification: boolean
+  ) {
+    if (!enablePullRequestCommentNotifications()) {
+      return
+    }
+
+    const repository = this.repository
+    if (repository === null) {
+      return
+    }
+
+    if (!this.isValidRepositoryForEvent(repository, event)) {
+      if (this.isRecentRepositoryEvent(event)) {
+        this.statsStore.recordPullRequestReviewNotiificationFromRecentRepo()
+      } else {
+        this.statsStore.recordPullRequestReviewNotiificationFromNonRecentRepo()
+      }
+      return
+    }
+
+    const pullRequests = await this.pullRequestCoordinator.getAllPullRequests(
+      repository
+    )
+    const pullRequest = pullRequests.find(
+      pr => pr.pullRequestNumber === event.pull_request_number
+    )
+
+    // If the PR is not in cache, it probably means the user didn't work on it
+    // recently, so we don't want to show a notification.
+    if (pullRequest === undefined) {
+      return
+    }
+
+    // Fetch comment from API depending on event subtype
+    const api = await this.getAPIForRepository(repository.gitHubRepository)
+    if (api === null) {
+      return
+    }
+
+    const comment =
+      event.subtype === 'issue-comment'
+        ? await api.fetchIssueComment(event.owner, event.repo, event.comment_id)
+        : await api.fetchPullRequestReviewComment(
+            event.owner,
+            event.repo,
+            event.comment_id
+          )
+
+    if (comment === null) {
+      return
+    }
+
+    const title = `@${comment.user.login} commented your pull request`
+    const body = `${pullRequest.title} #${
+      pullRequest.pullRequestNumber
+    }\n${truncateWithEllipsis(comment.body, 50)}`
+    const onClick = () => {
+      // TODO: this.statsStore.recordPullRequestReviewNotificationClicked(review.state)
+
+      this.onPullRequestCommentCallback?.(repository, pullRequest, comment)
+    }
+
+    if (skipNotification) {
+      onClick()
+      return
+    }
+
+    showNotification({
+      title,
+      body,
+      userInfo: event,
+      onClick,
+    })
+
+    // TODO: this.statsStore.recordPullRequestReviewNotificationShown(review.state)
+  }
+
   private async handlePullRequestReviewSubmitEvent(
     event: IDesktopPullRequestReviewSubmitAliveEvent,
     skipNotification: boolean
@@ -475,4 +566,11 @@ export class NotificationsStore {
   ) {
     this.onPullRequestReviewSubmitCallback = callback
   }
+
+  /** Observe when the user reacted to a "PR comment" notification. */
+  public onPullRequestCommentNotification(
+    callback: OnPullRequestCommentCallback
+  ) {
+    this.onPullRequestCommentCallback = callback
+  }
 }