From be2c6c8263d2e377748869c7e82e5829cd90a548 Mon Sep 17 00:00:00 2001 From: Florent LE BOULCH Date: Thu, 30 May 2024 18:22:22 +0200 Subject: [PATCH] fix: remove the constraint on the branch name --- .../__fixtures__/hooks/pushHookFixture.ts | 73 +++++++++++++++++++ __tests__/review/pushHook.test.ts | 70 +++++++++++++++++- .../share/hookHandlers/pushHookHandler.ts | 2 +- 3 files changed, 142 insertions(+), 3 deletions(-) diff --git a/__tests__/__fixtures__/hooks/pushHookFixture.ts b/__tests__/__fixtures__/hooks/pushHookFixture.ts index 5fcb971..e0694ab 100644 --- a/__tests__/__fixtures__/hooks/pushHookFixture.ts +++ b/__tests__/__fixtures__/hooks/pushHookFixture.ts @@ -72,3 +72,76 @@ export const pushHookFixture = { ], total_commits_count: 4, }; + +export const pushHookFixtureFeatureBranch = { + object_kind: 'push', + before: '95790bf891e76fee5e1747ab589903a6a1f80f22', + after: 'da1560886d4f094c3e6c9ef40349f7d38b5d27d7', + ref: 'refs/heads/feat/add-logo', + checkout_sha: 'da1560886d4f094c3e6c9ef40349f7d38b5d27d7', + user_id: 4, + user_name: 'John Smith', + user_username: 'jsmith', + user_email: 'john@example.com', + user_avatar: + 'https://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=8://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=80', + project_id: projectFixture.id, + project: { + id: projectFixture.id, + name: projectFixture.name, + description: projectFixture.description, + web_url: projectFixture.web_url, + avatar_url: projectFixture.avatar_url, + git_ssh_url: 'git@example.com:mike/diaspora.git', + git_http_url: 'http://example.com/mike/diaspora.git', + namespace: projectFixture.namespace, + visibility_level: 0, + path_with_namespace: projectFixture.path_with_namespace, + default_branch: projectFixture.default_branch, + homepage: 'http://example.com/mike/diaspora', + url: 'git@example.com:mike/diaspora.git', + ssh_url: 'git@example.com:mike/diaspora.git', + http_url: 'http://example.com/mike/diaspora.git', + }, + repository: { + name: 'Diaspora', + url: 'git@example.com:mike/diaspora.git', + description: '', + homepage: 'http://example.com/mike/diaspora', + git_http_url: 'http://example.com/mike/diaspora.git', + git_ssh_url: 'git@example.com:mike/diaspora.git', + visibility_level: 0, + }, + commits: [ + { + id: 'b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327', + message: + 'Update Catalan translation to e38cb41.\n\nSee https://gitlab.com/gitlab-org/gitlab for more information', + title: 'Update Catalan translation to e38cb41.', + timestamp: '2011-12-12T14:27:31+02:00', + url: 'http://example.com/mike/diaspora/commit/b6568db1bc1dcd7f8b4d5a946b0b91f9dacd7327', + author: { + name: 'Jordi Mallach', + email: 'jordi@softcatala.org', + }, + added: ['CHANGELOG'], + modified: ['app/controller/application.rb'], + removed: [], + }, + { + id: 'da1560886d4f094c3e6c9ef40349f7d38b5d27d7', + message: 'fixed readme', + title: 'fixed readme', + timestamp: '2012-01-03T23:36:29+02:00', + url: 'http://example.com/mike/diaspora/commit/da1560886d4f094c3e6c9ef40349f7d38b5d27d7', + author: { + name: 'GitLab dev user', + email: 'gitlabdev@dv6700.(none)', + }, + added: ['CHANGELOG'], + modified: ['app/controller/application.rb'], + removed: [], + }, + ], + total_commits_count: 4, +}; diff --git a/__tests__/review/pushHook.test.ts b/__tests__/review/pushHook.test.ts index 6874144..4ff1da5 100644 --- a/__tests__/review/pushHook.test.ts +++ b/__tests__/review/pushHook.test.ts @@ -1,7 +1,10 @@ import { HTTP_STATUS_NO_CONTENT, HTTP_STATUS_OK } from '@/constants'; import { addReviewToChannel } from '@/core/services/data'; import { slackBotWebClient } from '@/core/services/slack'; -import { pushHookFixture } from '../__fixtures__/hooks/pushHookFixture'; +import { + pushHookFixture, + pushHookFixtureFeatureBranch, +} from '../__fixtures__/hooks/pushHookFixture'; import { mergeRequestFixture } from '../__fixtures__/mergeRequestFixture'; import { fetch } from '../utils/fetch'; import { getGitlabHeaders } from '../utils/getGitlabHeaders'; @@ -25,7 +28,7 @@ describe('review > pushHook', () => { it('should publish a message on related review messages', async () => { // Given - const branchName = pushHookFixture.ref.split('/').pop(); + const branchName = 'master'; const channelId = 'channelId'; mockGitlabCall( @@ -86,6 +89,69 @@ describe('review > pushHook', () => { }); }); + it('should publish a message on related review messages for branch containing a slash', async () => { + // Given + const branchName = 'feat/add-logo'; + const channelId = 'channelId'; + + mockGitlabCall( + `/projects/${pushHookFixtureFeatureBranch.project_id}/merge_requests?source_branch=${branchName}`, + [mergeRequestFixture] + ); + mockGitlabCall( + `/projects/${pushHookFixtureFeatureBranch.project_id}/merge_requests/${mergeRequestFixture.iid}/commits?per_page=100`, + [{ id: pushHookFixtureFeatureBranch.commits[1].id }] + ); + await addReviewToChannel({ + channelId, + mergeRequestIid: mergeRequestFixture.iid, + projectId: mergeRequestFixture.project_id, + ts: 'ts', + }); + + // When + const response = await fetch('/api/v1/homer/gitlab', { + body: pushHookFixtureFeatureBranch, + headers: getGitlabHeaders(), + }); + + // Then + expect(response.status).toEqual(HTTP_STATUS_OK); + expect(slackBotWebClient.chat.postMessage).toHaveBeenNthCalledWith(1, { + blocks: [ + { + text: { + text: '', + type: 'mrkdwn', + }, + type: 'section', + }, + { + elements: [ + { + alt_text: 'gitlabdev.real', + image_url: 'image_24', + type: 'image', + }, + { + text: '*gitlabdev.real*', + type: 'mrkdwn', + }, + { + text: '2 changes', + type: 'plain_text', + }, + ], + type: 'context', + }, + ], + channel: 'channelId', + icon_emoji: ':git-commit:', + text: ':git-commit: New commit(s)', + thread_ts: 'ts', + }); + }); + it('should answer no content status whether there is no commit', async () => { // When const response = await fetch('/api/v1/homer/gitlab', { diff --git a/src/review/commands/share/hookHandlers/pushHookHandler.ts b/src/review/commands/share/hookHandlers/pushHookHandler.ts index 99db011..7a777eb 100644 --- a/src/review/commands/share/hookHandlers/pushHookHandler.ts +++ b/src/review/commands/share/hookHandlers/pushHookHandler.ts @@ -29,7 +29,7 @@ export async function pushHookHandler( return; } - const branchName = ref.split('/').pop() as string; + const branchName = ref.split('/').slice(2).join('/') as string; const mergeRequests = (await fetchMergeRequestsByBranchName(project_id, branchName)).filter(