UBERF-7863: Fix duplicate review comment (#6827)

Signed-off-by: Andrey Sobolev <haiodo@gmail.com>
This commit is contained in:
Andrey Sobolev 2024-10-07 15:33:36 +07:00 committed by GitHub
parent 1d09ab7503
commit 030bbb589b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 45 additions and 16 deletions

View File

@ -156,8 +156,8 @@ export class CommentSyncManager implements DocSyncManager {
derivedClient: TxOperations,
integration: IntegrationContainer
): Promise<void> {
const { repository } = await this.provider.getProjectAndRepository(event.repository.node_id)
if (repository === undefined) {
const { repository: repo } = await this.provider.getProjectAndRepository(event.repository.node_id)
if (repo === undefined) {
this.ctx.info('No project for repository', {
repository: event.repository,
workspace: this.provider.getWorkspaceId().name
@ -168,11 +168,12 @@ export class CommentSyncManager implements DocSyncManager {
const account = (await this.provider.getAccountU(event.sender))?._id ?? core.account.System
switch (event.action) {
case 'created': {
await this.createSyncData(event, derivedClient, repository)
await this.createSyncData(event, derivedClient, repo)
break
}
case 'deleted': {
const syncData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (event.comment.url ?? '').toLowerCase()
})
if (syncData !== undefined) {
@ -183,6 +184,7 @@ export class CommentSyncManager implements DocSyncManager {
}
case 'edited': {
const commentData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (event.comment.url ?? '').toLowerCase()
})
@ -221,6 +223,7 @@ export class CommentSyncManager implements DocSyncManager {
repo: GithubIntegrationRepository
): Promise<void> {
const commentData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (createdEvent.comment.url ?? '').toLowerCase()
})
@ -266,6 +269,7 @@ export class CommentSyncManager implements DocSyncManager {
if (parent === undefined) {
// Find parent by issue url
parent = await this.client.findOne(github.class.DocSyncInfo, {
space: container.project._id,
url: (comment.html_url.split('#')?.[0] ?? '').toLowerCase()
})
}

View File

@ -201,6 +201,7 @@ export abstract class IssueSyncManagerBase {
}
)) as any
const syncData = await this.client.findOne(github.class.DocSyncInfo, {
space: prj._id,
url: (actualContent.node.content.url ?? '').toLowerCase()
})
@ -353,7 +354,8 @@ export abstract class IssueSyncManagerBase {
}
syncData =
syncData ?? (await this.client.findOne(github.class.DocSyncInfo, { url: (external.url ?? '').toLowerCase() }))
syncData ??
(await this.client.findOne(github.class.DocSyncInfo, { space: prj._id, url: (external.url ?? '').toLowerCase() }))
if (syncData !== undefined) {
const doc: Issue | undefined = await this.client.findOne<Issue>(syncData.objectClass, {
@ -1262,7 +1264,10 @@ export abstract class IssueSyncManagerBase {
err: any,
_class: Ref<Class<Doc>> = tracker.class.Issue
): Promise<void> {
const syncData = await this.client.findOne(github.class.DocSyncInfo, { url: url.toLowerCase() })
const syncData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: url.toLowerCase()
})
if (syncData === undefined) {
await derivedClient?.createDoc(github.class.DocSyncInfo, repo.githubProject as Ref<GithubProject>, {
url,

View File

@ -203,6 +203,7 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
}
case 'deleted': {
const syncData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (event.issue.html_url ?? '').toLowerCase()
})
if (syncData !== undefined) {
@ -291,6 +292,7 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
repo: GithubIntegrationRepository
): Promise<void> {
const issueSyncData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (issueExternal.url ?? '').toLowerCase()
})
if (issueSyncData === undefined) {
@ -452,7 +454,7 @@ export class IssueSyncManager extends IssueSyncManagerBase implements DocSyncMan
break
}
await this.provider.doSyncFor(attachedDocs)
await this.provider.doSyncFor(attachedDocs, container.project)
for (const child of attachedDocs) {
await derivedClient.update(child, { createId })
}

View File

@ -302,6 +302,7 @@ export class PullRequestSyncManager extends IssueSyncManagerBase implements DocS
}
case 'synchronize': {
const syncData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (externalData.url ?? '').toLowerCase()
})
if (syncData !== undefined) {

View File

@ -213,6 +213,7 @@ export class ReviewCommentSyncManager implements DocSyncManager {
}
case 'deleted': {
const reviewData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (event.comment.html_url ?? '').toLowerCase()
})
if (reviewData !== undefined) {
@ -232,6 +233,7 @@ export class ReviewCommentSyncManager implements DocSyncManager {
}
case 'edited': {
const reviewData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (event.comment.html_url ?? '').toLowerCase()
})
@ -280,6 +282,7 @@ export class ReviewCommentSyncManager implements DocSyncManager {
externalData: ReviewCommentExternalData
): Promise<void> {
const reviewData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (createdEvent.comment.html_url ?? '').toLowerCase()
})
@ -331,9 +334,10 @@ export class ReviewCommentSyncManager implements DocSyncManager {
if (info.reviewThreadId === undefined && reviewComment.replyTo?.url !== undefined) {
const rthread = await derivedClient.findOne(github.class.GithubReviewComment, {
space: container.project._id,
url: reviewComment.replyTo?.url?.toLowerCase()
})
if (rthread !== undefined) {
if (rthread !== undefined && info.reviewThreadId !== rthread.reviewThreadId) {
info.reviewThreadId = rthread.reviewThreadId
await derivedClient.update(info, { reviewThreadId: info.reviewThreadId })
}
@ -519,8 +523,10 @@ export class ReviewCommentSyncManager implements DocSyncManager {
external: reviewExternal,
current: existing,
repository: repo._id,
parent: parent.url.toLocaleLowerCase(),
needSync: githubSyncVersion,
externalVersion: githubExternalSyncVersion
externalVersion: githubExternalSyncVersion,
reviewThreadId: info.reviewThreadId ?? existingReview.reviewThreadId
}
// We need to update in current promise, to prevent event changes.
await derivedClient.update(info, upd)

View File

@ -205,6 +205,7 @@ export class ReviewThreadSyncManager implements DocSyncManager {
case 'unresolved': {
const isResolved = event.action === 'resolved'
const reviewData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: event.thread.node_id.toLocaleLowerCase()
})
@ -241,6 +242,7 @@ export class ReviewThreadSyncManager implements DocSyncManager {
}
const reviewPR = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (reviewData.parent ?? '').toLowerCase()
})
if (reviewPR !== undefined) {
@ -516,6 +518,7 @@ export class ReviewThreadSyncManager implements DocSyncManager {
.map((it) => (it.parent ?? '').toLowerCase())
.filter((it, idx, arr) => it != null && arr.indexOf(it) === idx)
const parents = await derivedClient.findAll(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: {
$in: allParents
}

View File

@ -198,6 +198,7 @@ export class ReviewSyncManager implements DocSyncManager {
await this.createSyncData(event, derivedClient, repo, externalData)
const parentDoc = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (event.pull_request.html_url ?? '').toLowerCase()
})
if (parentDoc !== undefined) {
@ -211,6 +212,7 @@ export class ReviewSyncManager implements DocSyncManager {
}
case 'dismissed': {
const reviewData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (event.review.html_url ?? '').toLowerCase()
})
@ -254,6 +256,7 @@ export class ReviewSyncManager implements DocSyncManager {
externalData: ReviewExternalData
): Promise<void> {
const reviewData = await this.client.findOne(github.class.DocSyncInfo, {
space: repo.githubProject as Ref<GithubProject>,
url: (createdEvent.review.html_url ?? '').toLowerCase()
})

View File

@ -340,6 +340,7 @@ export async function syncDerivedDocuments<T extends { url: string }> (
extra?: any
): Promise<void> {
const childDocsOfClass = await derivedClient.findAll(github.class.DocSyncInfo, {
space: prj._id,
objectClass,
parent: (parentDoc.url ?? '').toLowerCase(),
...query
@ -352,7 +353,7 @@ export async function syncDerivedDocuments<T extends { url: string }> (
if (existing === undefined) {
await derivedClient.createDoc<DocSyncInfo>(github.class.DocSyncInfo, prj._id, {
objectClass,
url: r.url.toLowerCase(),
url: (r.url ?? '').toLowerCase(),
needSync: '', // we need to sync to retrieve patch in background
githubNumber: 0,
repository: repo._id,

View File

@ -111,7 +111,7 @@ export interface IntegrationManager {
event: T
) => Promise<void>
doSyncFor: (docs: DocSyncInfo[]) => Promise<void>
doSyncFor: (docs: DocSyncInfo[], project: GithubProject) => Promise<void>
getWorkspaceId: () => WorkspaceIdWithUrl
getBranding: () => Branding | null

View File

@ -1114,7 +1114,7 @@ export class GithubWorker implements IntegrationManager {
}
private async performSync (projects: GithubProject[], repositories: GithubIntegrationRepository[]): Promise<boolean> {
const _projects = projects.map((it) => it._id)
const _projects = toIdMap(projects)
const _repositories = repositories.map((it) => it._id)
const docs = await this.ctx.with(
@ -1126,7 +1126,7 @@ export class GithubWorker implements IntegrationManager {
{
needSync: { $ne: githubSyncVersion },
externalVersion: { $in: [githubExternalSyncVersion, '#'] },
space: { $in: _projects },
space: { $in: Array.from(_projects.keys()) },
repository: { $in: [null, ..._repositories] }
},
{
@ -1142,18 +1142,21 @@ export class GithubWorker implements IntegrationManager {
this.previousWait += docs.length
this.ctx.info('Syncing', { docs: docs.length, workspace: this.workspace.name })
await this.doSyncFor(docs)
const bySpace = groupByArray(docs, (it) => it.space)
for (const [k, v] of bySpace.entries()) {
await this.doSyncFor(v, _projects.get(k as Ref<GithubProject>) as GithubProject)
}
}
return docs.length !== 0
}
async doSyncFor (docs: DocSyncInfo[]): Promise<void> {
async doSyncFor (docs: DocSyncInfo[], project: GithubProject): Promise<void> {
const byClass = this.groupByClass(docs)
// We need to reorder based on our sync mappers
for (const [_class, clDocs] of byClass.entries()) {
await this.syncClass(_class, clDocs)
await this.syncClass(_class, clDocs, project)
}
}
@ -1220,12 +1223,13 @@ export class GithubWorker implements IntegrationManager {
}
}
async syncClass (_class: Ref<Class<Doc>>, syncInfo: DocSyncInfo[]): Promise<void> {
async syncClass (_class: Ref<Class<Doc>>, syncInfo: DocSyncInfo[], project: GithubProject): Promise<void> {
const externalDocs = await this._client.findAll<Doc>(_class, {
_id: { $in: syncInfo.map((it) => it._id as Ref<Doc>) }
})
const parents = await this._client.findAll<DocSyncInfo>(github.class.DocSyncInfo, {
space: project._id,
url: { $in: syncInfo.map((it) => it.parent?.toLowerCase()).filter((it) => it) as string[] }
})