UBERF-8053: Github fixes (#6554)

Signed-off-by: Andrey Sobolev <haiodo@gmail.com>

services/github/pod-github/src/sync/comments.ts

@@ -12,8 +12,8 @@ import core, {
   Ref,
   TxOperations
 } from '@hcengineering/core'
-import { LiveQuery } from '@hcengineering/query'
 import github, { DocSyncInfo, GithubIntegrationRepository, GithubProject } from '@hcengineering/github'
+import { LiveQuery } from '@hcengineering/query'
 import { deepEqual } from 'fast-equals'
 import {
   ContainerFocus,
@@ -252,7 +252,7 @@ export class CommentSyncManager implements DocSyncManager {
     }
     if (info.external === undefined) {
       // TODO: Use selected repository
-      const repo = container.repository.find((it) => it._id === parent?.repository)
+      const repo = await this.provider.getRepositoryById(parent?.repository)
       if (repo?.nodeId === undefined) {
         // No need to sync if parent repository is not defined.
         return { needSync: githubSyncVersion }
@@ -303,7 +303,7 @@ export class CommentSyncManager implements DocSyncManager {
     comment: CommentExternalData,
     account: Ref<Account>
   ): Promise<void> {
-    const repository = container.repository.find((it) => it._id === info.repository)
+    const repository = await this.provider.getRepositoryById(info.repository)
     if (repository === undefined) {
       return
     }
@@ -384,7 +384,7 @@ export class CommentSyncManager implements DocSyncManager {
     derivedClient: TxOperations
   ): Promise<DocumentUpdate<DocSyncInfo>> {
     // TODO: Use selected repository
-    const repo = container.repository.find((it) => it._id === parent?.repository)
+    const repo = await this.provider.getRepositoryById(parent?.repository)
     if (repo?.nodeId === undefined) {
       // No need to sync if parent repository is not defined.
       return { needSync: githubSyncVersion }

services/github/pod-github/src/sync/issueBase.ts

@@ -302,8 +302,7 @@ export abstract class IssueSyncManagerBase {
                 const ff = await this.toPlatformField(
                   {
                     container: integration,
-                    project: prj,
-                    repository: repositories.filter((it) => it.githubProject === prj._id)
+                    project: prj
                   },
                   f,
                   target,
@@ -949,17 +948,19 @@ export abstract class IssueSyncManagerBase {
     existing: WithMarkup<Issue>,
     issueExternal: IssueExternalData
   ): Promise<void> {
-    const repo = container.repository.find((it) => it._id === info.repository) as GithubIntegrationRepository
-    await this.addConnectToMessage(
-      existing._class === github.class.GithubPullRequest
-        ? github.string.PullRequestConnectedActivityInfo
-        : github.string.IssueConnectedActivityInfo,
-      existing.space,
-      existing._id,
-      existing._class,
-      issueExternal,
-      repo
-    )
+    const repo = await this.provider.getRepositoryById(info.repository)
+    if (repo != null) {
+      await this.addConnectToMessage(
+        existing._class === github.class.GithubPullRequest
+          ? github.string.PullRequestConnectedActivityInfo
+          : github.string.IssueConnectedActivityInfo,
+        existing.space,
+        existing._id,
+        existing._class,
+        issueExternal,
+        repo
+      )
+    }
   }
 
   async collectIssueUpdate (
@@ -1125,7 +1126,7 @@ export abstract class IssueSyncManagerBase {
     container: IntegrationContainer,
     existingIssue: Issue | undefined,
     external: IssueExternalData
-  ): Promise<IssueSyncTarget | undefined | null> {
+  ): Promise<IssueSyncTarget | undefined> {
     if (existingIssue !== undefined) {
       // Select a milestone project
       if (existingIssue.milestone != null) {
@@ -1133,14 +1134,7 @@ export abstract class IssueSyncManagerBase {
           await this.provider.liveQuery.queryFind<GithubMilestone>(github.mixin.GithubMilestone, {})
         ).find((it) => it._id === existingIssue.milestone)
         if (milestone === undefined) {
-          // Let's search for milestone, and if it doesn't have mixin, return undefined.
-          const mstone = await this.client.findOne(github.mixin.GithubMilestone, {
-            _id: existingIssue.milestone as Ref<GithubMilestone>
-          })
-          if (mstone === undefined) {
-            return undefined
-          }
-          return null
+          return
         }
         return {
           project,

services/github/pod-github/src/sync/issues.ts

@@ -345,7 +345,9 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
         }
         if (info.repository == null) {
           // No need to sync if component it not yet set
-          const repos = container.repository.map((it) => it.name).join(', ')
+          const repos = (await this.provider.getProjectRepositories(container.project._id))
+            .map((it) => it.name)
+            .join(', ')
           this.ctx.error('Not syncing repository === null', {
             url: info.url,
             identifier: (existing as Issue).identifier,
@@ -360,9 +362,11 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
 
     let issueExternal = info.external as IssueExternalData
     if (info.external === undefined && existing !== undefined) {
-      const repository = container.repository.find((it) => it._id === info.repository)
+      const repository = await this.provider.getRepositoryById(info.repository)
       if (repository === undefined) {
-        const repos = container.repository.map((it) => it.name).join(', ')
+        const repos = (await this.provider.getProjectRepositories(container.project._id))
+          .map((it) => it.name)
+          .join(', ')
         this.ctx.error('Not syncing repository === undefined', {
           url: info.url,
           identifier: (existing as Issue).identifier,
@@ -427,11 +431,6 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
       existing as Issue,
       issueExternal
     )
-    if (target === null) {
-      // We need to wait, no milestone data yet.
-      this.ctx.error('target === null, no milestone data yet', { url: info.url })
-      return { needSync: githubSyncVersion }
-    }
     if (target === undefined) {
       target = this.getProjectIssueTarget(container.project, issueExternal)
     }
@@ -573,6 +572,11 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
           container.container,
           issueExternal.body
         )
+        const repo = await this.provider.getRepositoryById(info.repository)
+        if (repo == null) {
+          // No repository, it probable deleted
+          return { needSync: githubSyncVersion }
+        }
         await this.ctx.withLog(
           'create platform issue',
           {},
@@ -590,7 +594,7 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
               info.repository as Ref<GithubIntegrationRepository>,
               container.project,
               taskTypes[0]._id,
-              container.repository.find((it) => it._id === info.repository) as GithubIntegrationRepository & {
+              repo as GithubIntegrationRepository & {
                 repository: IntegrationRepositoryData
               },
               !markdownCompatible

services/github/pod-github/src/sync/projects.ts

@@ -12,11 +12,6 @@ import core, {
   TxOperations,
   generateId
 } from '@hcengineering/core'
-import { getEmbeddedLabel, translate } from '@hcengineering/platform'
-import { LiveQuery } from '@hcengineering/query'
-import task from '@hcengineering/task'
-import tracker, { Milestone } from '@hcengineering/tracker'
-import { RepositoryEvent } from '@octokit/webhooks-types'
 import github, {
   DocSyncInfo,
   GithubFieldMapping,
@@ -25,6 +20,11 @@ import github, {
   GithubProject,
   GithubProjectSyncData
 } from '@hcengineering/github'
+import { getEmbeddedLabel, translate } from '@hcengineering/platform'
+import { LiveQuery } from '@hcengineering/query'
+import task from '@hcengineering/task'
+import tracker, { Milestone } from '@hcengineering/tracker'
+import { RepositoryEvent } from '@octokit/webhooks-types'
 import { deepEqual } from 'fast-equals'
 import { Octokit } from 'octokit'
 import {
@@ -238,6 +238,28 @@ export class ProjectsSyncManager implements DocSyncManager {
           external: data,
           needSync: ''
         })
+
+        // We also need to notify all issues with milestone set to this milestone.
+        const milestonedIds = await this.client.findAll(
+          tracker.class.Issue,
+          { milestone: milestone._id },
+          { projection: { _id: 1 } }
+        )
+        while (milestonedIds.length > 0) {
+          const part = milestonedIds.splice(0, 100)
+          const docInfos = await this.client.findAll(
+            github.class.DocSyncInfo,
+            { _id: { $in: part.map((it) => it._id as unknown as Ref<DocSyncInfo>) } },
+            { projection: { _id: 1 } }
+          )
+          if (docInfos.length > 0) {
+            const ops = derivedClient.apply()
+            for (const d of docInfos) {
+              await ops.update(d, { needSync: '' })
+            }
+            await ops.commit()
+          }
+        }
       }
     }
   }

services/github/pod-github/src/sync/pullrequests.ts

@@ -32,7 +32,6 @@ import github, {
 import task, { TaskType, calcRank, makeRank } from '@hcengineering/task'
 import time, { ToDo, ToDoPriority } from '@hcengineering/time'
 import tracker, { Issue, IssuePriority, IssueStatus, Project } from '@hcengineering/tracker'
-import { OctokitResponse } from '@octokit/types'
 import { ProjectsV2ItemEvent, PullRequestEvent } from '@octokit/webhooks-types'
 import { Octokit } from 'octokit'
 import config from '../config'
@@ -59,7 +58,16 @@ import {
 } from './githubTypes'
 import { GithubIssueData, IssueSyncManagerBase, IssueSyncTarget, WithMarkup } from './issueBase'
 import { syncConfig } from './syncConfig'
-import { errorToObj, getSinceRaw, gqlp, guessStatus, isGHWriteAllowed, syncDerivedDocuments, syncRunner } from './utils'
+import {
+  errorToObj,
+  getSinceRaw,
+  gqlp,
+  guessStatus,
+  isGHWriteAllowed,
+  syncChilds,
+  syncDerivedDocuments,
+  syncRunner
+} from './utils'
 
 type GithubPullRequestData = GithubIssueData &
 Omit<GithubPullRequest, keyof Issue | 'commits' | 'reviews' | 'reviewComments'>
@@ -487,8 +495,8 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
         await this.ctx.withLog(
           'retrieve pull request patch',
           {},
-          async () =>
-            await this.handlePatch(
+          () =>
+            this.handlePatch(
               info,
               container,
               pullRequestExternal,
@@ -526,7 +534,7 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
               info.repository as Ref<GithubIntegrationRepository>,
               container.project,
               taskTypes[0]._id,
-              container.repository.find((it) => it._id === info.repository) as GithubIntegrationRepository,
+              (await this.provider.getRepositoryById(info.repository)) as GithubIntegrationRepository,
               !markdownCompatible
             )
           },
@@ -544,6 +552,9 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
 
         await op.commit()
 
+        // To sync reviews/review threads in case they are created before us.
+        await syncChilds(info, this.client, derivedClient)
+
         return {
           needSync: '',
           external: pullRequestExternal,
@@ -563,7 +574,7 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
           await this.ctx.withLog(
             'update pull request patch',
             {},
-            async () =>
+            async () => {
               await this.handlePatch(
                 info,
                 container,
@@ -575,7 +586,8 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
                 },
                 lastModified,
                 accountGH
-              ),
+              )
+            },
             { url: pullRequestExternal.url }
           )
         }
@@ -966,10 +978,6 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
       existing as Issue,
       pullRequestExternal
     )
-    if (target === null) {
-      // We need to wait, no milestone data yet.
-      return { needSync: '' }
-    }
     if (target === undefined) {
       target = this.getProjectIssueTarget(container.project, pullRequestExternal)
     }
@@ -1085,18 +1093,17 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
     existingPR: Pick<GithubPullRequest, '_id' | 'space' | '_class'>,
     lastModified: number,
     account: Ref<Account>
-  ): Promise<string | null> {
-    let patch: string | null = null
-    const repo = container.repository.find((it) => it._id === info.repository)
+  ): Promise<void> {
+    const repo = await this.provider.getRepositoryById(info.repository)
     if (repo?.nodeId === undefined) {
-      return null
+      return
     }
     if (info.external?.patch !== true) {
-      patch = await this.fetchPatch(pullRequestExternal, container.container.octokit, repo)
+      const { patch, contentType } = await this.fetchPatch(pullRequestExternal, container.container.octokit, repo)
 
       // Update attached patch data.
       const patchAttachment = await this.client.findOne(github.class.GithubPatch, { attachedTo: existingPR._id })
-      const blob = await this.provider.uploadFile(patch, patchAttachment?.file)
+      const blob = await this.provider.uploadFile(patch, patchAttachment?.file, contentType)
       if (blob !== undefined) {
         if (patchAttachment === undefined) {
           await this.client.addCollection(
@@ -1131,7 +1138,6 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
         }
       }
     }
-    return patch
   }
 
   private async createPullRequest (
@@ -1544,8 +1550,9 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
     pullRequest: PullRequestExternalData,
     octokit: Octokit,
     repository: GithubIntegrationRepository
-  ): Promise<string> {
+  ): Promise<{ patch: string, contentType: string }> {
     let patch = ''
+    let contentType = 'application/vnd.github.VERSION.diff'
     try {
       const patchContent = await octokit.rest.pulls.get({
         owner: repository.owner?.login as string,
@@ -1556,12 +1563,13 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
           'X-GitHub-Api-Version': '2022-11-28'
         }
       })
-      patch = ((patchContent as unknown as OctokitResponse<string>).data ?? '').slice(0, 2 * 1024 * 1024)
+      patch = (patchContent.data as unknown as string) ?? ''
+      contentType = patchContent.headers['content-type'] ?? 'application/vnd.github.VERSION.diff'
     } catch (err: any) {
       this.ctx.error('Error', { err })
       Analytics.handleError(err)
     }
-    return patch
+    return { patch, contentType }
   }
 
   async deleteGithubDocument (container: ContainerFocus, account: Ref<Account>, id: string): Promise<void> {

services/github/pod-github/src/sync/reviewComments.ts

@@ -310,11 +310,11 @@ export class ReviewCommentSyncManager implements DocSyncManager {
       return {}
     }
     if (parent === undefined) {
-      return { needSync: '' }
+      return { needSync: githubSyncVersion }
     }
     if (info.external === undefined) {
       // TODO: Use selected repository
-      const repo = container.repository.find((it) => it._id === parent?.repository)
+      const repo = await this.provider.getRepositoryById(parent?.repository)
       if (repo?.nodeId === undefined) {
         // No need to sync if parent repository is not defined.
         return { needSync: githubSyncVersion }
@@ -381,7 +381,7 @@ export class ReviewCommentSyncManager implements DocSyncManager {
     account: Ref<Account>,
     derivedClient: TxOperations
   ): Promise<void> {
-    const repository = container.repository.find((it) => it._id === info.repository)
+    const repository = await this.provider.getRepositoryById(info.repository)
     if (repository === undefined) {
       return
     }
@@ -473,7 +473,7 @@ export class ReviewCommentSyncManager implements DocSyncManager {
     derivedClient: TxOperations
   ): Promise<DocumentUpdate<DocSyncInfo>> {
     // TODO: Use selected repository
-    const repo = container.repository.find((it) => it._id === parent?.repository)
+    const repo = await this.provider.getRepositoryById(parent?.repository)
     if (repo?.nodeId === undefined) {
       // No need to sync if parent repository is not defined.
       return { needSync: githubSyncVersion }

services/github/pod-github/src/sync/reviewThreads.ts

@@ -11,14 +11,14 @@ import core, {
   Ref,
   TxOperations
 } from '@hcengineering/core'
-import { EmptyMarkup } from '@hcengineering/text'
-import { LiveQuery } from '@hcengineering/query'
 import github, {
   DocSyncInfo,
   GithubIntegrationRepository,
   GithubProject,
   GithubReviewThread
 } from '@hcengineering/github'
+import { LiveQuery } from '@hcengineering/query'
+import { EmptyMarkup } from '@hcengineering/text'
 import {
   ContainerFocus,
   DocSyncManager,
@@ -35,7 +35,7 @@ import {
   getUpdatedAtReviewThread,
   reviewThreadDetails
 } from './githubTypes'
-import { collectUpdate, deleteObjects, errorToObj, isGHWriteAllowed, syncDerivedDocuments } from './utils'
+import { collectUpdate, deleteObjects, errorToObj, isGHWriteAllowed, syncChilds, syncDerivedDocuments } from './utils'
 
 import { Analytics } from '@hcengineering/analytics'
 import { PullRequestReviewThreadEvent } from '@octokit/webhooks-types'
@@ -266,11 +266,11 @@ export class ReviewThreadSyncManager implements DocSyncManager {
       return {}
     }
     if (parent === undefined) {
-      return { needSync: '' }
+      return { needSync: githubSyncVersion }
     }
     if (info.external === undefined) {
       // TODO: Use selected repository
-      const repo = container.repository.find((it) => it._id === parent?.repository)
+      const repo = await this.provider.getRepositoryById(parent?.repository)
       if (repo?.nodeId === undefined) {
         // No need to sync if parent repository is not defined.
         return { needSync: githubSyncVersion }
@@ -305,6 +305,9 @@ export class ReviewThreadSyncManager implements DocSyncManager {
     if (existing === undefined) {
       try {
         await this.createReviewThread(info, messageData, parent, review, account)
+
+        // We need trigger comments, if their sync data created before
+        await syncChilds(info, this.client, derivedClient)
         return { needSync: githubSyncVersion, current: messageData }
       } catch (err: any) {
         this.ctx.error('Error', { err })
@@ -327,7 +330,7 @@ export class ReviewThreadSyncManager implements DocSyncManager {
     account: Ref<Account>,
     derivedClient: TxOperations
   ): Promise<void> {
-    const repository = container.repository.find((it) => it._id === info.repository)
+    const repository = await this.provider.getRepositoryById(info.repository)
     if (repository === undefined) {
       return
     }
@@ -422,7 +425,7 @@ export class ReviewThreadSyncManager implements DocSyncManager {
     derivedClient: TxOperations
   ): Promise<DocumentUpdate<DocSyncInfo>> {
     // TODO: Use selected repository
-    const repo = container.repository.find((it) => it._id === parent?.repository)
+    const repo = await this.provider.getRepositoryById(parent?.repository)
     if (repo?.nodeId === undefined) {
       // No need to sync if parent repository is not defined.
       return { needSync: githubSyncVersion }

services/github/pod-github/src/sync/reviews.ts

@@ -11,7 +11,6 @@ import core, {
   Ref,
   TxOperations
 } from '@hcengineering/core'
-import { LiveQuery } from '@hcengineering/query'
 import github, {
   DocSyncInfo,
   GithubIntegrationRepository,
@@ -19,6 +18,7 @@ import github, {
   GithubPullRequestReviewState,
   GithubReview
 } from '@hcengineering/github'
+import { LiveQuery } from '@hcengineering/query'
 import {
   ContainerFocus,
   DocSyncManager,
@@ -29,7 +29,7 @@ import {
   githubSyncVersion
 } from '../types'
 import { PullRequestExternalData, Review as ReviewExternalData, reviewDetails, toReviewState } from './githubTypes'
-import { collectUpdate, deleteObjects, errorToObj, isGHWriteAllowed } from './utils'
+import { collectUpdate, deleteObjects, errorToObj, isGHWriteAllowed, syncChilds } from './utils'
 
 import { Analytics } from '@hcengineering/analytics'
 import { PullRequestReviewEvent, PullRequestReviewSubmittedEvent } from '@octokit/webhooks-types'
@@ -284,11 +284,11 @@ export class ReviewSyncManager implements DocSyncManager {
       return {}
     }
     if (parent === undefined) {
-      return { needSync: '' }
+      return { needSync: githubSyncVersion }
     }
     if (info.external === undefined) {
       // TODO: Use selected repository
-      const repo = container.repository.find((it) => it._id === parent?.repository)
+      const repo = await this.provider.getRepositoryById(parent?.repository)
       if (repo?.nodeId === undefined) {
         // No need to sync if parent repository is not defined.
         return { needSync: githubSyncVersion }
@@ -310,6 +310,8 @@ export class ReviewSyncManager implements DocSyncManager {
     if (existing === undefined) {
       try {
         await this.createReview(info, messageData, parent, review, account)
+
+        await syncChilds(info, this.client, derivedClient)
         return { needSync: githubSyncVersion, current: messageData }
       } catch (err: any) {
         this.ctx.error('Error', { err })
@@ -331,7 +333,7 @@ export class ReviewSyncManager implements DocSyncManager {
     review: ReviewExternalData,
     account: Ref<Account>
   ): Promise<void> {
-    const repository = container.repository.find((it) => it._id === info.repository)
+    const repository = await this.provider.getRepositoryById(info.repository)
     if (repository === undefined) {
       return
     }
@@ -400,7 +402,7 @@ export class ReviewSyncManager implements DocSyncManager {
     derivedClient: TxOperations
   ): Promise<DocumentUpdate<DocSyncInfo>> {
     // TODO: Use selected repository
-    const repo = container.repository.find((it) => it._id === parent?.repository)
+    const repo = await this.provider.getRepositoryById(parent?.repository)
     if (repo?.nodeId === undefined) {
       // No need to sync if parent repository is not defined.
       return { needSync: githubSyncVersion }

services/github/pod-github/src/sync/utils.ts

@@ -16,15 +16,15 @@ import core, {
   Type,
   toIdMap
 } from '@hcengineering/core'
-import { PlatformError, unknownStatus } from '@hcengineering/platform'
-import task, { TaskType, calculateStatuses, createState, findStatusAttr } from '@hcengineering/task'
-import tracker, { IssueStatus } from '@hcengineering/tracker'
 import github, {
   DocSyncInfo,
   GithubIntegrationRepository,
   GithubIssueStateReason,
   GithubProject
 } from '@hcengineering/github'
+import { PlatformError, unknownStatus } from '@hcengineering/platform'
+import task, { TaskType, calculateStatuses, createState, findStatusAttr } from '@hcengineering/task'
+import tracker, { IssueStatus } from '@hcengineering/tracker'
 import { deepEqual } from 'fast-equals'
 import { IntegrationManager, githubExternalSyncVersion } from '../types'
 import { GithubDataType } from './githubTypes'
@@ -411,3 +411,14 @@ export function compareMarkdown (a: string, b: string): boolean {
 
   return na === nb
 }
+
+export async function syncChilds (info: DocSyncInfo, client: TxOperations, derivedClient: TxOperations): Promise<void> {
+  const childInfos = await client.findAll(github.class.DocSyncInfo, { parent: info.url.toLowerCase() })
+  if (childInfos.length > 0) {
+    const ops = derivedClient.apply()
+    for (const child of childInfos) {
+      await ops?.update(child, { needSync: '' })
+    }
+    await ops.commit()
+  }
+}