chore(gerrit): improve auto-approve on new patchset workaround

renovatebot · Nov 30, 2024 · 82fe8e1 · 82fe8e1
1 parent 46f8eed
commit 82fe8e1
Show file tree

Hide file tree

Showing 6 changed files with 8 additions and 87 deletions.
diff --git a/lib/modules/platform/gerrit/client.spec.ts b/lib/modules/platform/gerrit/client.spec.ts
@@ -473,74 +473,6 @@ describe('modules/platform/gerrit/client', () => {
       expect(approveMock.isDone()).toBeTrue();
     });
   });
-
-  describe('wasApprovedBy()', () => {
-    it('label not exists', () => {
-      expect(
-        client.wasApprovedBy(partial<GerritChange>({}), 'user'),
-      ).toBeFalse();
-    });
-
-    it('not approved by anyone', () => {
-      expect(
-        client.wasApprovedBy(
-          partial<GerritChange>({
-            labels: {
-              'Code-Review': {
-                all: [],
-              },
-            },
-          }),
-          'user',
-        ),
-      ).toBeFalse();
-    });
-
-    it('not approved but with +1', () => {
-      expect(
-        client.wasApprovedBy(
-          partial<GerritChange>({
-            labels: {
-              'Code-Review': {
-                all: [{ value: 1, username: 'user' }],
-              },
-            },
-          }),
-          'user',
-        ),
-      ).toBeFalse();
-    });
-
-    it('approved by given user', () => {
-      expect(
-        client.wasApprovedBy(
-          partial<GerritChange>({
-            labels: {
-              'Code-Review': {
-                all: [{ value: 2, username: 'user' }],
-              },
-            },
-          }),
-          'user',
-        ),
-      ).toBeTrue();
-    });
-
-    it('approved by given other', () => {
-      expect(
-        client.wasApprovedBy(
-          partial<GerritChange>({
-            labels: {
-              'Code-Review': {
-                all: [{ value: 2, username: 'other' }],
-              },
-            },
-          }),
-          'user',
-        ),
-      ).toBeFalse();
-    });
-  });
 });
 
 function gerritRestResponse(body: any): any {

diff --git a/lib/modules/platform/gerrit/client.ts b/lib/modules/platform/gerrit/client.ts
@@ -202,15 +202,6 @@ class GerritClient {
     );
   }
 
-  wasApprovedBy(change: GerritChange, username: string): boolean {
-    const reviewLabel = change?.labels?.['Code-Review'];
-    return Boolean(
-      reviewLabel?.all?.some(
-        (label) => label.value === 2 && label.username === username,
-      ),
-    );
-  }
-
   normalizeMessage(message: string): string {
     //the last \n was removed from gerrit after the comment was added...
     return message.substring(0, 0x4000).trim();

diff --git a/lib/modules/platform/gerrit/scm.spec.ts b/lib/modules/platform/gerrit/scm.spec.ts
@@ -370,7 +370,6 @@ describe('modules/platform/gerrit/scm', () => {
         },
       });
       clientMock.findChanges.mockResolvedValueOnce([existingChange]);
-      clientMock.wasApprovedBy.mockReturnValueOnce(true);
       git.prepareCommit.mockResolvedValueOnce({
         commitSha: 'commitSha' as LongCommitSha,
         parentCommitSha: 'parentSha' as LongCommitSha,
@@ -386,6 +385,7 @@ describe('modules/platform/gerrit/scm', () => {
           message: 'commit msg',
           files: [],
           prTitle: 'pr title',
+          autoApprove: true,
         }),
       ).toBe('commitSha');
       expect(git.prepareCommit).toHaveBeenCalledWith({
@@ -397,6 +397,7 @@ describe('modules/platform/gerrit/scm', () => {
           'Renovate-Branch: renovate/dependency-1.x\nChange-Id: ...',
         ],
         prTitle: 'pr title',
+        autoApprove: true,
         force: true,
       });
       expect(git.fetchRevSpec).toHaveBeenCalledWith('refs/changes/1/2');
@@ -405,10 +406,6 @@ describe('modules/platform/gerrit/scm', () => {
         sourceRef: 'renovate/dependency-1.x',
         targetRef: 'refs/for/main',
       });
-      expect(clientMock.wasApprovedBy).toHaveBeenCalledWith(
-        existingChange,
-        'user',
-      );
       expect(clientMock.approveChange).toHaveBeenCalledWith(123456);
     });
   });

diff --git a/lib/modules/platform/gerrit/scm.ts b/lib/modules/platform/gerrit/scm.ts
@@ -135,11 +135,8 @@ export class GerritScm extends DefaultGitScm {
           files: commit.files,
         });
         if (pushResult) {
-          //existingChange was the old change before commit/push. we need to approve again, if it was previously approved from renovate
-          if (
-            existingChange &&
-            client.wasApprovedBy(existingChange, username)
-          ) {
+          // New patchsets dismisses the approval, so we need to approve it again
+          if (existingChange && commit.autoApprove) {
             await client.approveChange(existingChange._number);
           }
           return commitSha;

diff --git a/lib/util/git/types.ts b/lib/util/git/types.ts
@@ -85,6 +85,8 @@ export interface CommitFilesConfig {
   platformCommit?: PlatformCommitOptions;
   /** Only needed by Gerrit platform */
   prTitle?: string;
+  /** Only needed by Gerrit platform */
+  autoApprove?: boolean;
 }
 
 export interface PushFilesConfig {

diff --git a/lib/workers/repository/update/branch/commit.ts b/lib/workers/repository/update/branch/commit.ts
@@ -62,5 +62,7 @@ export function commitFilesToBranch(
     platformCommit: config.platformCommit,
     // Only needed by Gerrit platform
     prTitle: config.prTitle,
+    // Only needed by Gerrit platform
+    autoApprove: config.autoApprove,
   });
 }