From e88189d4c6657960c9cf3444447dd5013cc63183 Mon Sep 17 00:00:00 2001
From: Rhys Koedijk <rhys@koedijk.co.nz>
Date: Sun, 17 Nov 2024 23:37:25 +1300
Subject: [PATCH] Remove duplicate reviewer identities before creating pull
 requests (#1457)

---
 .../AzureDevOpsWebApiClient.test.ts           | 95 +++++++++++++++++++
 .../azure-devops/AzureDevOpsWebApiClient.ts   |  4 +-
 2 files changed, 97 insertions(+), 2 deletions(-)
 create mode 100644 extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.test.ts

diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.test.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.test.ts
new file mode 100644
index 00000000..d4091b0f
--- /dev/null
+++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.test.ts
@@ -0,0 +1,95 @@
+import { jest } from '@jest/globals';
+
+import { VersionControlChangeType } from 'azure-devops-node-api/interfaces/TfvcInterfaces';
+
+import { AzureDevOpsWebApiClient } from './AzureDevOpsWebApiClient';
+import { ICreatePullRequest } from './interfaces/IPullRequest';
+import exp = require('constants');
+
+jest.mock('azure-devops-node-api');
+jest.mock('azure-pipelines-task-lib/task');
+
+describe('AzureDevOpsWebApiClient', () => {
+  const organisationApiUrl = 'https://dev.azure.com/mock-organization';
+  const accessToken = 'mock-access-token';
+  let client: AzureDevOpsWebApiClient;
+
+  beforeEach(() => {
+    client = new AzureDevOpsWebApiClient(organisationApiUrl, accessToken);
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  describe('createPullRequest', () => {
+    let pr: ICreatePullRequest;
+
+    beforeEach(() => {
+      pr = {
+        project: 'project',
+        repository: 'repository',
+        source: {
+          branch: 'update-branch',
+          commit: 'commit-id',
+        },
+        target: {
+          branch: 'main',
+        },
+        title: 'PR Title',
+        description: 'PR Description',
+        commitMessage: 'Commit Message',
+        changes: [
+          {
+            path: 'file.txt',
+            content: 'hello world',
+            encoding: 'utf-8',
+            changeType: VersionControlChangeType.Add,
+          },
+        ],
+      };
+    });
+
+    it('should create a pull request without duplicate reviewer and assignee identities', async () => {
+      // Arange
+      const mockGetUserId = jest.spyOn(client, 'getUserId').mockResolvedValue('my-user-id');
+      const mockResolveIdentityId = jest
+        .spyOn(client, 'resolveIdentityId')
+        .mockImplementation(async (identity?: string) => {
+          return identity || '';
+        });
+      const mockRestApiPost = jest
+        .spyOn(client as any, 'restApiPost')
+        .mockResolvedValueOnce({
+          commits: [{ commitId: 'new-commit-id' }],
+        })
+        .mockResolvedValueOnce({
+          pullRequestId: 1,
+        });
+      const mockRestApiPatch = jest.spyOn(client as any, 'restApiPatch').mockResolvedValueOnce({
+        count: 1,
+      });
+
+      // Act
+      pr.assignees = ['user1', 'user2'];
+      pr.reviewers = ['user1', 'user3'];
+      const pullRequestId = await client.createPullRequest(pr);
+
+      // Assert
+      expect(mockRestApiPost).toHaveBeenCalledTimes(2);
+      expect((mockRestApiPost.mock.calls[1] as any)[1].reviewers.length).toBe(3);
+      expect((mockRestApiPost.mock.calls[1] as any)[1].reviewers).toContainEqual({
+        id: 'user1',
+        isRequired: true,
+        isFlagged: true,
+      });
+      expect((mockRestApiPost.mock.calls[1] as any)[1].reviewers).toContainEqual({
+        id: 'user2',
+        isRequired: true,
+        isFlagged: true,
+      });
+      expect((mockRestApiPost.mock.calls[1] as any)[1].reviewers).toContainEqual({ id: 'user3' });
+      expect(pullRequestId).toBe(1);
+    });
+  });
+});
diff --git a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts
index aea9a534..427077ef 100644
--- a/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts
+++ b/extension/tasks/dependabotV2/utils/azure-devops/AzureDevOpsWebApiClient.ts
@@ -190,7 +190,7 @@ export class AzureDevOpsWebApiClient {
       if (pr.assignees?.length > 0) {
         for (const assignee of pr.assignees) {
           const identityId = isGuid(assignee) ? assignee : await this.resolveIdentityId(assignee);
-          if (identityId) {
+          if (identityId && !allReviewers.some((r) => r.id === identityId)) {
             allReviewers.push({
               id: identityId,
               isRequired: true,
@@ -204,7 +204,7 @@ export class AzureDevOpsWebApiClient {
       if (pr.reviewers?.length > 0) {
         for (const reviewer of pr.reviewers) {
           const identityId = isGuid(reviewer) ? reviewer : await this.resolveIdentityId(reviewer);
-          if (identityId) {
+          if (identityId && !allReviewers.some((r) => r.id === identityId)) {
             allReviewers.push({
               id: identityId,
             });