chore(gerrit): still approve if change was approved another user

renovatebot · Dec 1, 2024 · 5eb2846 · 5eb2846
1 parent 1d8daf5
commit 5eb2846
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 8 deletions.
diff --git a/lib/modules/platform/gerrit/client.spec.ts b/lib/modules/platform/gerrit/client.spec.ts
@@ -5,10 +5,11 @@ import { setBaseUrl } from '../../../util/http/gerrit';
 import type { FindPRConfig } from '../types';
 import { client } from './client';
 import type {
-  GerritChange,
-  GerritChangeMessageInfo,
-  GerritFindPRConfig,
-  GerritMergeableInfo,
+  GerritAccountInfo,
+  type GerritChange,
+  type GerritChangeMessageInfo,
+  type GerritFindPRConfig,
+  type GerritMergeableInfo,
 } from './types';
 
 const gerritEndpointUrl = 'https://dev.gerrit.com/renovate/';
@@ -399,12 +400,14 @@ describe('modules/platform/gerrit/client', () => {
 
   describe('approveChange()', () => {
     it('already approved - do nothing', async () => {
+      const owner = partial<GerritAccountInfo>({ username: 'user' });
       const change = partial<GerritChange>({
         labels: {
           'Code-Review': {
-            all: [{ value: 2 }],
+            all: [{ value: 2, username: owner.username }],
           },
         },
+        owner,
       });
       httpMock
         .scope(gerritEndpointUrl)
@@ -480,6 +483,35 @@ describe('modules/platform/gerrit/client', () => {
       await expect(client.approveChange(123456)).toResolve();
       expect(approveMock.isDone()).toBeTrue();
     });
+
+    it('already approved by another user - approve again', async () => {
+      const owner = partial<GerritAccountInfo>({ username: 'user' });
+      const change = partial<GerritChange>({
+        labels: {
+          'Code-Review': {
+            all: [{ value: 2, username: 'other user' }],
+          },
+        },
+        owner,
+      });
+      httpMock
+        .scope(gerritEndpointUrl)
+        .get(
+          (url) =>
+            url.includes('/a/changes/123456?o=') &&
+            url.includes('o=DETAILED_LABELS'),
+        )
+        .reply(200, gerritRestResponse(change), jsonResultHeader);
+      const approveMock = httpMock
+        .scope(gerritEndpointUrl)
+        .post('/a/changes/123456/revisions/current/review', {
+          labels: { 'Code-Review': +2 },
+          notify: 'NONE',
+        })
+        .reply(200, gerritRestResponse(''), jsonResultHeader);
+      await expect(client.approveChange(123456)).toResolve();
+      expect(approveMock.isDone()).toBeTrue();
+    });
   });
 });
 

diff --git a/lib/modules/platform/gerrit/client.ts b/lib/modules/platform/gerrit/client.ts
@@ -193,17 +193,24 @@ class GerritClient {
 
   async approveChange(changeId: number): Promise<void> {
     const isApproved = await this.checkIfApproved(changeId);
-    if (!isApproved) {
-      await this.setLabel(changeId, 'Code-Review', +2);
+    if (isApproved) {
+      logger.debug(`Change is already approved`);
+      return;
     }
+    await this.setLabel(changeId, 'Code-Review', +2);
   }
 
   async checkIfApproved(changeId: number): Promise<boolean> {
     const change = await client.getChange(changeId, ['DETAILED_LABELS']);
     const reviewLabel = change?.labels?.['Code-Review'];
     return (
       reviewLabel === undefined ||
-      Boolean(reviewLabel.all?.some((label) => label.value === 2))
+      Boolean(
+        reviewLabel.all?.some(
+          (label) =>
+            label.value === 2 && label.username === change.owner.username,
+        ),
+      )
     );
   }
 

diff --git a/lib/modules/platform/gerrit/types.ts b/lib/modules/platform/gerrit/types.ts
@@ -52,6 +52,7 @@ export interface GerritChange {
    */
   revisions: Record<string, GerritRevisionInfo>;
   problems: unknown[];
+  owner: GerritAccountInfo;
 }
 
 export interface GerritCommitInfo {