From 711dde088b99de8c2af55c2b878d4767b1ee6450 Mon Sep 17 00:00:00 2001 From: Maxime Brunet <max@brnt.mx> Date: Sun, 16 Jan 2022 02:17:20 -0800 Subject: [PATCH] fix(bitbucket-cloud): Remove reviewers no longer member of the workspace (#13274) Co-authored-by: Michael Kriese <michael.kriese@visualon.de> --- lib/platform/bitbucket/index.spec.ts | 75 +++++++++++++++++++++++++++- lib/platform/bitbucket/index.ts | 70 +++++++++++++++++++------- 2 files changed, 125 insertions(+), 20 deletions(-) diff --git a/lib/platform/bitbucket/index.spec.ts b/lib/platform/bitbucket/index.spec.ts index 2027b0fdcc..b09910050d 100644 --- a/lib/platform/bitbucket/index.spec.ts +++ b/lib/platform/bitbucket/index.spec.ts @@ -803,6 +803,77 @@ describe('platform/bitbucket/index', () => { await bitbucket.updatePr({ number: 5, prTitle: 'title', prBody: 'body' }); expect(httpMock.getTrace()).toMatchSnapshot(); }); + it('removes reviewers no longer member of the workspace when updating pr', async () => { + const notMemberReviewer = { + display_name: 'Bob Smith', + uuid: '{d2238482-2e9f-48b3-8630-de22ccb9e42f}', + account_id: '123', + }; + const memberReviewer = { + display_name: 'Jane Smith', + uuid: '{90b6646d-1724-4a64-9fd9-539515fe94e9}', + account_id: '456', + }; + const scope = await initRepoMock(); + scope + .get('/2.0/repositories/some/repo/pullrequests/5') + .reply(200, { reviewers: [memberReviewer, notMemberReviewer] }) + .put('/2.0/repositories/some/repo/pullrequests/5') + .reply(400, { + type: 'error', + error: { + fields: { + reviewers: [ + 'Bob Smith is not a member of this workspace and cannot be added to this pull request', + ], + }, + message: + 'reviewers: Bob Smith is not a member of this workspace and cannot be added to this pull request', + }, + }) + .head('/2.0/workspaces/some/members/123') + .reply(404) + .head('/2.0/workspaces/some/members/456') + .reply(200) + .put('/2.0/repositories/some/repo/pullrequests/5') + .reply(200); + expect( + await bitbucket.updatePr({ + number: 5, + prTitle: 'title', + prBody: 'body', + }) + ).toBeUndefined(); + }); + it('throws exception when unable to check reviewers workspace membership', async () => { + const reviewer = { + display_name: 'Bob Smith', + uuid: '{d2238482-2e9f-48b3-8630-de22ccb9e42f}', + account_id: '123', + }; + const scope = await initRepoMock(); + scope + .get('/2.0/repositories/some/repo/pullrequests/5') + .reply(200, { reviewers: [reviewer] }) + .put('/2.0/repositories/some/repo/pullrequests/5') + .reply(400, { + type: 'error', + error: { + fields: { + reviewers: [ + 'Bob Smith is not a member of this workspace and cannot be added to this pull request', + ], + }, + message: + 'reviewers: Bob Smith is not a member of this workspace and cannot be added to this pull request', + }, + }) + .head('/2.0/workspaces/some/members/123') + .reply(401); + await expect(() => + bitbucket.updatePr({ number: 5, prTitle: 'title', prBody: 'body' }) + ).rejects.toThrow(new Error('Response code 401 (Unauthorized)')); + }); it('rethrows exception when PR update error not due to inactive reviewers', async () => { const reviewer = { display_name: 'Jane Smith', @@ -941,9 +1012,9 @@ describe('platform/bitbucket/index', () => { it('returns file content in json5 format', async () => { const json5Data = ` - { + { // json5 comment - foo: 'bar' + foo: 'bar' } `; const scope = await initRepoMock(); diff --git a/lib/platform/bitbucket/index.ts b/lib/platform/bitbucket/index.ts index 0d5ae4bf03..7a1fc392a1 100644 --- a/lib/platform/bitbucket/index.ts +++ b/lib/platform/bitbucket/index.ts @@ -52,6 +52,10 @@ const pathSeparator = '/'; let renovateUserUuid: string; +const inactiveReviewersMessage = 'reviewers: Malformed reviewers list'; +const notMemberReviewersMessage = + 'is not a member of this workspace and cannot be added to this pull request'; + export async function initPlatform({ endpoint, username, @@ -751,26 +755,56 @@ export async function updatePr({ } catch (err) { if ( err.statusCode === 400 && - err.body?.error?.message.includes('reviewers: Malformed reviewers list') + [inactiveReviewersMessage, notMemberReviewersMessage].some((m) => + err.body?.error?.message.includes(m) + ) ) { - logger.warn( - { err }, - 'PR contains inactive reviewer accounts. Will try setting only active reviewers' - ); + const sanitizedReviewers: PrReviewer[] = []; // Bitbucket returns a 400 if any of the PR reviewer accounts are now inactive (ie: disabled/suspended) - const activeReviewers: PrReviewer[] = []; - - // Validate that each previous PR reviewer account is still active - for (const reviewer of pr.reviewers) { - const reviewerUser = ( - await bitbucketHttp.getJson<UserResponse>( - `/2.0/users/${reviewer.account_id}` - ) - ).body; - - if (reviewerUser.account_status === 'active') { - activeReviewers.push(reviewer); + if (err.body?.error?.message.includes(inactiveReviewersMessage)) { + logger.warn( + { err }, + 'PR contains inactive reviewer accounts. Will try setting only active reviewers' + ); + + // Validate that each previous PR reviewer account is still active + for (const reviewer of pr.reviewers) { + const reviewerUser = ( + await bitbucketHttp.getJson<UserResponse>( + `/2.0/users/${reviewer.account_id}` + ) + ).body; + + if (reviewerUser.account_status === 'active') { + sanitizedReviewers.push(reviewer); + } + } + } + + // Bitbucket returns a 400 if any of the PR reviewer accounts are no longer members of this workspace + if (err.body?.error?.message.includes(notMemberReviewersMessage)) { + logger.warn( + { err }, + 'PR contains reviewer accounts which are no longer member of this workspace. Will try setting only member reviewers' + ); + + const workspace = config.repository.split('/')[0]; + + // Validate that each previous PR reviewer account is still a member of this workspace + for (const reviewer of pr.reviewers) { + try { + await bitbucketHttp.head( + `/2.0/workspaces/${workspace}/members/${reviewer.account_id}` + ); + + sanitizedReviewers.push(reviewer); + } catch (err) { + // HTTP 404: User cannot be found, or the user is not a member of this workspace. + if (err.response?.statusCode !== 404) { + throw err; + } + } } } @@ -780,7 +814,7 @@ export async function updatePr({ body: { title, description: sanitize(description), - reviewers: activeReviewers, + reviewers: sanitizedReviewers, }, } ); -- GitLab