Revert "feat(gitlab,azure): try approving before auto-merge"

This reverts commit c679c28d01.

lib/modules/platform/azure/__snapshots__/index.spec.ts.snap

@@ -217,10 +217,6 @@ exports[`modules/platform/azure/index updatePr(prNo, title, body, platformPrOpti
 ]
 `;
 
-exports[`modules/platform/azure/index updatePr(prNo, title, body, platformPrOptions) should not re-approve if the PR is already approved 1`] = `undefined`;
-
-exports[`modules/platform/azure/index updatePr(prNo, title, body, platformPrOptions) should re-approve if the PR was approved by another user 1`] = `undefined`;
-
 exports[`modules/platform/azure/index updatePr(prNo, title, body, platformPrOptions) should re-approve the PR 1`] = `undefined`;
 
 exports[`modules/platform/azure/index updatePr(prNo, title, body, platformPrOptions) should reopen the PR 1`] = `

lib/modules/platform/azure/index.spec.ts

@@ -1149,7 +1149,7 @@ describe('modules/platform/azure/index', () => {
       const updateFn = jest
         .fn(() => prUpdateResult)
         .mockName('createPullRequestReviewer');
-      azureApi.gitApi.mockImplementation(
+      azureApi.gitApi.mockImplementationOnce(
         () =>
           ({
             createPullRequest: jest.fn(() => prResult),
@@ -1293,7 +1293,7 @@ describe('modules/platform/azure/index', () => {
         isRequired: false,
       };
       const updateFn = jest.fn(() => prUpdateResult);
-      azureApi.gitApi.mockImplementation(
+      azureApi.gitApi.mockImplementationOnce(
         () =>
           ({
             updatePullRequest: jest.fn(() => prResult),
@@ -1313,82 +1313,6 @@ describe('modules/platform/azure/index', () => {
       expect(updateFn).toHaveBeenCalled();
       expect(pr).toMatchSnapshot();
     });
-
-    it('should not re-approve if the PR is already approved', async () => {
-      await initRepo({ repository: 'some/repo' });
-      const prResult = {
-        pullRequestId: 456,
-        createdBy: {
-          id: 123,
-          url: 'user-url',
-        },
-      };
-      const prUpdateResult = {
-        reviewerUrl: prResult.createdBy.url,
-        vote: AzurePrVote.Approved,
-        isFlagged: false,
-        isRequired: false,
-      };
-      const updateFn = jest.fn(() => prUpdateResult);
-      azureApi.gitApi.mockImplementation(
-        () =>
-          ({
-            updatePullRequest: jest.fn(() => prResult),
-            createPullRequestReviewer: updateFn,
-            getPullRequestById: jest.fn(() => ({
-              pullRequestId: prResult.pullRequestId,
-              createdBy: prResult.createdBy,
-              reviewers: [{ vote: AzurePrVote.Approved, id: 123 }],
-            })),
-          }) as any,
-      );
-      const pr = await azure.updatePr({
-        number: prResult.pullRequestId,
-        prTitle: 'The Title',
-        prBody: 'Hello world',
-        platformPrOptions: { autoApprove: true },
-      });
-      expect(updateFn).not.toHaveBeenCalled();
-      expect(pr).toMatchSnapshot();
-    });
-
-    it('should re-approve if the PR was approved by another user', async () => {
-      await initRepo({ repository: 'some/repo' });
-      const prResult = {
-        pullRequestId: 456,
-        createdBy: {
-          id: 123,
-          url: 'user-url',
-        },
-      };
-      const prUpdateResult = {
-        reviewerUrl: prResult.createdBy.url,
-        vote: AzurePrVote.Approved,
-        isFlagged: false,
-        isRequired: false,
-      };
-      const updateFn = jest.fn(() => prUpdateResult);
-      azureApi.gitApi.mockImplementation(
-        () =>
-          ({
-            updatePullRequest: jest.fn(() => prResult),
-            createPullRequestReviewer: updateFn,
-            getPullRequestById: jest.fn(() => ({
-              pullRequestId: prResult.pullRequestId,
-              createdBy: prResult.createdBy,
-              reviewers: [{ vote: AzurePrVote.Approved, id: 321 }],
-            })),
-          }) as any,
-      );
-      const pr = await azure.updatePr({
-        number: prResult.pullRequestId,
-        prTitle: 'The Title',
-        prBody: 'Hello world',
-        platformPrOptions: { autoApprove: true },
-      });
-      expect(updateFn).toHaveBeenCalled();
-      expect(pr).toMatchSnapshot();
-    });
   });
 
   describe('ensureComment', () => {

lib/modules/platform/azure/index.ts

@@ -513,7 +513,18 @@ export async function createPr({
     );
   }
   if (platformPrOptions?.autoApprove) {
-    await approvePr(pr);
+    await azureApiGit.createPullRequestReviewer(
+      {
+        reviewerUrl: pr.createdBy!.url,
+        vote: AzurePrVote.Approved,
+        isFlagged: false,
+        isRequired: false,
+      },
+      config.repoId,
+      // TODO #22198
+      pr.pullRequestId!,
+      pr.createdBy!.id!,
+    );
   }
   await Promise.all(
     labels!.map((label) =>
@@ -570,7 +581,19 @@ export async function updatePr({
     objToUpdate.status = PullRequestStatus.Abandoned;
   }
   if (platformPrOptions?.autoApprove) {
-    await approvePr(prNo);
+    const pr = await azureApiGit.getPullRequestById(prNo, config.project);
+    await azureApiGit.createPullRequestReviewer(
+      {
+        reviewerUrl: pr.createdBy!.url,
+        vote: AzurePrVote.Approved,
+        isFlagged: false,
+        isRequired: false,
+      },
+      config.repoId,
+      // TODO #22198
+      pr.pullRequestId!,
+      pr.createdBy!.id!,
+    );
   }
 
   const updatedPr = await azureApiGit.updatePullRequest(
@@ -992,37 +1015,3 @@ export async function deleteLabel(
   const azureApiGit = await azureApi.gitApi();
   await azureApiGit.deletePullRequestLabels(config.repoId, prNumber, label);
 }
-
-export async function approvePr(
-  prNumberOrPr: number | GitPullRequest,
-): Promise<void> {
-  const azureApiGit = await azureApi.gitApi();
-  const pr =
-    typeof prNumberOrPr === 'number'
-      ? await azureApiGit.getPullRequestById(prNumberOrPr, config.project)
-      : prNumberOrPr;
-
-  const isApproved = pr.reviewers?.some(
-    (reviewer) =>
-      reviewer.vote === AzurePrVote.Approved &&
-      reviewer.id === pr.createdBy?.id,
-  );
-
-  if (isApproved) {
-    logger.debug('PR is already approved');
-    return;
-  }
-
-  await azureApiGit.createPullRequestReviewer(
-    {
-      reviewerUrl: pr.createdBy!.url,
-      vote: AzurePrVote.Approved,
-      isFlagged: false,
-      isRequired: false,
-    },
-    config.repoId,
-    // TODO #22198
-    pr.pullRequestId!,
-    pr.createdBy!.id!,
-  );
-}

lib/modules/platform/gitlab/index.spec.ts

@@ -128,18 +128,9 @@ describe('modules/platform/gitlab/index', () => {
     });
 
     it(`should reuse existing gitAuthor`, async () => {
-      httpMock
+      httpMock.scope(gitlabApiHost).get('/api/v4/version').reply(200, {
-        .scope(gitlabApiHost)
+        version: '13.3.6-ee',
-        .get('/api/v4/user')
+      });
-        .reply(200, {
-          email: 'different@email.com',
-          name: 'Different Name',
-          id: 1,
-        })
-        .get('/api/v4/version')
-        .reply(200, {
-          version: '13.3.6-ee',
-        });
       expect(
         await gitlab.initPlatform({
           token: 'some-token',
@@ -1909,7 +1900,6 @@ describe('modules/platform/gitlab/index', () => {
       .get('/api/v4/user')
       .reply(200, {
         email: 'a@b.com',
-        id: 1,
         name: 'Renovate Bot',
       })
       .get('/api/v4/version')
@@ -2788,10 +2778,6 @@ describe('modules/platform/gitlab/index', () => {
           target_branch: 'master',
           description: 'the-body',
         })
-        .get('/api/v4/projects/undefined/merge_requests/12345/approvals')
-        .reply(200, {
-          approved_by: [],
-        })
         .post('/api/v4/projects/undefined/merge_requests/12345/approve')
         .reply(200);
       expect(
@@ -2825,8 +2811,6 @@ describe('modules/platform/gitlab/index', () => {
           target_branch: 'master',
           description: 'the-body',
         })
-        .get('/api/v4/projects/undefined/merge_requests/12345/approvals')
-        .replyWithError('some error')
         .post('/api/v4/projects/undefined/merge_requests/12345/approve')
         .replyWithError('some error');
       await expect(
@@ -3155,104 +3139,6 @@ describe('modules/platform/gitlab/index', () => {
           description: 'body',
           state: 'opened',
         })
-        .get('/api/v4/projects/undefined/merge_requests/1/approvals')
-        .reply(200, {
-          approved_by: [],
-        })
-        .post('/api/v4/projects/undefined/merge_requests/1/approve')
-        .reply(200);
-      await expect(
-        gitlab.updatePr({
-          number: 1,
-          prTitle: 'title',
-          prBody: 'body',
-          platformPrOptions: {
-            autoApprove: true,
-          },
-        }),
-      ).toResolve();
-    });
-
-    it('does not auto-approve if already approved', async () => {
-      await initPlatform('13.3.6-ee');
-      httpMock
-        .scope(gitlabApiHost)
-        .get(
-          '/api/v4/projects/undefined/merge_requests?per_page=100&scope=created_by_me',
-        )
-        .reply(200, [
-          {
-            iid: 1,
-            source_branch: 'branch-a',
-            title: 'branch a pr',
-            description: 'a merge request',
-            state: 'opened',
-          },
-        ])
-        .put('/api/v4/projects/undefined/merge_requests/1')
-        .reply(200, {
-          iid: 1,
-          source_branch: 'branch-a',
-          title: 'title',
-          description: 'body',
-          state: 'opened',
-        })
-        .get('/api/v4/projects/undefined/merge_requests/1/approvals')
-        .reply(200, {
-          approved_by: [
-            {
-              user: {
-                id: 1,
-              },
-            },
-          ],
-        });
-      await expect(
-        gitlab.updatePr({
-          number: 1,
-          prTitle: 'title',
-          prBody: 'body',
-          platformPrOptions: {
-            autoApprove: true,
-          },
-        }),
-      ).toResolve();
-    });
-
-    it('auto-approves if approved by another user', async () => {
-      await initPlatform('13.3.6-ee');
-      httpMock
-        .scope(gitlabApiHost)
-        .get(
-          '/api/v4/projects/undefined/merge_requests?per_page=100&scope=created_by_me',
-        )
-        .reply(200, [
-          {
-            iid: 1,
-            source_branch: 'branch-a',
-            title: 'branch a pr',
-            description: 'a merge request',
-            state: 'opened',
-          },
-        ])
-        .put('/api/v4/projects/undefined/merge_requests/1')
-        .reply(200, {
-          iid: 1,
-          source_branch: 'branch-a',
-          title: 'title',
-          description: 'body',
-          state: 'opened',
-        })
-        .get('/api/v4/projects/undefined/merge_requests/1/approvals')
-        .reply(200, {
-          approved_by: [
-            {
-              user: {
-                id: 2,
-              },
-            },
-          ],
-        })
         .post('/api/v4/projects/undefined/merge_requests/1/approve')
         .reply(200);
       await expect(

lib/modules/platform/gitlab/index.ts

@@ -65,7 +65,6 @@ import { getMR, updateMR } from './merge-request';
 import { LastPipelineId } from './schema';
 import type {
   GitLabMergeRequest,
-  GitLabMergeRequestApprovals,
   GitlabComment,
   GitlabIssue,
   GitlabPr,
@@ -95,7 +94,6 @@ const defaults = {
 export const id = 'gitlab';
 
 let draftPrefix = DRAFT_PREFIX;
-let renovateUserId: number;
 
 export async function initPlatform({
   endpoint,
@@ -116,22 +114,19 @@ export async function initPlatform({
   };
   let gitlabVersion: string;
   try {
-    const user = (
-      await gitlabApi.getJson<{
-        email: string;
-        name: string;
-        id: number;
-        commit_email?: string;
-      }>(`user`, { token })
-    ).body;
-
-    renovateUserId = user.id;
     if (!gitAuthor) {
+      const user = (
+        await gitlabApi.getJson<{
+          email: string;
+          name: string;
+          id: number;
+          commit_email?: string;
+        }>(`user`, { token })
+      ).body;
       platformConfig.gitAuthor = `${user.name} <${
         user.commit_email ?? user.email
       }>`;
     }
-
     // istanbul ignore if: experimental feature
     if (process.env.RENOVATE_X_PLATFORM_VERSION) {
       gitlabVersion = process.env.RENOVATE_X_PLATFORM_VERSION;
@@ -716,26 +711,6 @@ async function tryPrAutomerge(
 }
 
 async function approvePr(pr: number): Promise<void> {
-  try {
-    const { body: approvals } =
-      await gitlabApi.getJson<GitLabMergeRequestApprovals>(
-        `projects/${config.repository}/merge_requests/${pr}/approvals`,
-      );
-
-    const isApproved = approvals.approved_by?.some(
-      (approval) => approval.user?.id === renovateUserId,
-    );
-
-    if (isApproved) {
-      logger.debug('MR is already approved');
-      return;
-    }
-  } catch (err) {
-    logger.warn(
-      { err },
-      'GitLab: Error checking if the merge request is approved',
-    );
-  }
   try {
     await gitlabApi.postJson(
       `projects/${config.repository}/merge_requests/${pr}/approve`,

lib/modules/platform/gitlab/types.ts

@@ -77,7 +77,3 @@ export interface GitlabUserStatus {
   emoji?: string;
   availability: 'not_set' | 'busy';
 }
-
-export interface GitLabMergeRequestApprovals {
-  approved_by?: { user?: GitLabUser }[];
-}