Skip to content

Commit 113b707

Browse files
felipecrsfelipecrs
committed
Refactor hashtags handling
1 parent 69d72c2 commit 113b707

File tree

14 files changed

+123
-89
lines changed

14 files changed

+123
-89
lines changed

‎lib/modules/platform/gerrit/client.spec.ts‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -561,15 +561,49 @@ describe('modules/platform/gerrit/client', () => {
561561
});
562562
});
563563

564-
describe('deleteHashtag()', () => {
565-
it('deleteHashtag', async () => {
564+
describe('setHashtags()', () => {
565+
it('add hashtags', async () => {
566+
httpMock
567+
.scope(gerritEndpointUrl)
568+
.post('/a/changes/123456/hashtags', {
569+
add: ['hashtag1', 'hashtag2'],
570+
})
571+
.reply(200, gerritRestResponse([]), jsonResultHeader);
572+
await expect(
573+
client.setHashtags(123456, { add: ['hashtag1', 'hashtag2'] }),
574+
).toResolve();
575+
});
576+
577+
it('remove hashtags', async () => {
566578
httpMock
567579
.scope(gerritEndpointUrl)
568580
.post('/a/changes/123456/hashtags', {
569581
remove: ['hashtag1'],
570582
})
571583
.reply(200, gerritRestResponse([]), jsonResultHeader);
572-
await expect(client.deleteHashtag(123456, 'hashtag1')).toResolve();
584+
await expect(
585+
client.setHashtags(123456, { remove: ['hashtag1'] }),
586+
).toResolve();
587+
});
588+
589+
it('add and remove hashtags in single call', async () => {
590+
httpMock
591+
.scope(gerritEndpointUrl)
592+
.post('/a/changes/123456/hashtags', {
593+
add: ['hashtag2', 'hashtag3'],
594+
remove: ['hashtag1'],
595+
})
596+
.reply(200, gerritRestResponse([]), jsonResultHeader);
597+
await expect(
598+
client.setHashtags(123456, {
599+
add: ['hashtag2', 'hashtag3'],
600+
remove: ['hashtag1'],
601+
}),
602+
).toResolve();
603+
});
604+
605+
it('does nothing when no hashtags provided', async () => {
606+
await expect(client.setHashtags(123456, {})).toResolve();
573607
});
574608
});
575609

‎lib/modules/platform/gerrit/client.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
GerritChange,
99
GerritChangeMessageInfo,
1010
GerritFindPRConfig,
11+
GerritHashtagsInput,
1112
GerritMergeableInfo,
1213
GerritProjectInfo,
1314
GerritRequestDetail,
@@ -185,10 +186,21 @@ class GerritClient {
185186
);
186187
}
187188

188-
async deleteHashtag(changeNumber: number, hashtag: string): Promise<void> {
189-
await this.gerritHttp.postJson(`a/changes/${changeNumber}/hashtags`, {
190-
body: { remove: [hashtag] },
191-
});
189+
async setHashtags(
190+
changeNumber: number,
191+
hashtagsInput: GerritHashtagsInput,
192+
): Promise<void> {
193+
if (hashtagsInput.add?.length === 0) {
194+
delete hashtagsInput.add;
195+
}
196+
if (hashtagsInput.remove?.length === 0) {
197+
delete hashtagsInput.remove;
198+
}
199+
if (hashtagsInput.add || hashtagsInput.remove) {
200+
await this.gerritHttp.postJson(`a/changes/${changeNumber}/hashtags`, {
201+
body: hashtagsInput,
202+
});
203+
}
192204
}
193205

194206
async addReviewers(changeNumber: number, reviewers: string[]): Promise<void> {

‎lib/modules/platform/gerrit/index.spec.ts‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,47 @@ describe('modules/platform/gerrit/index', () => {
244244
TAG_PULL_REQUEST_BODY,
245245
);
246246
});
247+
248+
it('updatePr() - with addLabels => add hashtags', async () => {
249+
const change = partial<GerritChange>({});
250+
clientMock.getChange.mockResolvedValueOnce(change);
251+
await gerrit.updatePr({
252+
number: 123456,
253+
prTitle: change.subject,
254+
addLabels: ['label1', 'label2'],
255+
});
256+
expect(clientMock.setHashtags).toHaveBeenCalledExactlyOnceWith(123456, {
257+
add: ['label1', 'label2'],
258+
});
259+
});
260+
261+
it('updatePr() - with removeLabels => remove hashtags', async () => {
262+
const change = partial<GerritChange>({});
263+
clientMock.getChange.mockResolvedValueOnce(change);
264+
await gerrit.updatePr({
265+
number: 123456,
266+
prTitle: change.subject,
267+
removeLabels: ['old-label'],
268+
});
269+
expect(clientMock.setHashtags).toHaveBeenCalledExactlyOnceWith(123456, {
270+
remove: ['old-label'],
271+
});
272+
});
273+
274+
it('updatePr() - with addLabels and removeLabels => update hashtags in single call', async () => {
275+
const change = partial<GerritChange>({});
276+
clientMock.getChange.mockResolvedValueOnce(change);
277+
await gerrit.updatePr({
278+
number: 123456,
279+
prTitle: change.subject,
280+
addLabels: ['new-label'],
281+
removeLabels: ['old-label'],
282+
});
283+
expect(clientMock.setHashtags).toHaveBeenCalledExactlyOnceWith(123456, {
284+
add: ['new-label'],
285+
remove: ['old-label'],
286+
});
287+
});
247288
});
248289

249290
describe('createPr()', () => {
@@ -780,11 +821,10 @@ describe('modules/platform/gerrit/index', () => {
780821
it('deleteLabel() - deletes a label', async () => {
781822
const pro = gerrit.deleteLabel(123456, 'hashtag1');
782823
await expect(pro).resolves.toBeUndefined();
783-
expect(clientMock.deleteHashtag).toHaveBeenCalledTimes(1);
784-
expect(clientMock.deleteHashtag).toHaveBeenCalledExactlyOnceWith(
785-
123456,
786-
'hashtag1',
787-
);
824+
expect(clientMock.setHashtags).toHaveBeenCalledTimes(1);
825+
expect(clientMock.setHashtags).toHaveBeenCalledExactlyOnceWith(123456, {
826+
remove: ['hashtag1'],
827+
});
788828
});
789829
});
790830

‎lib/modules/platform/gerrit/index.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,12 @@ export async function updatePr(prConfig: UpdatePrConfig): Promise<void> {
174174
TAG_PULL_REQUEST_BODY,
175175
);
176176
}
177+
if (prConfig.addLabels?.length || prConfig.removeLabels?.length) {
178+
await client.setHashtags(prConfig.number, {
179+
add: prConfig.addLabels ?? undefined,
180+
remove: prConfig.removeLabels ?? undefined,
181+
});
182+
}
177183
if (prConfig.state && prConfig.state === 'closed') {
178184
await client.abandonChange(prConfig.number);
179185
}
@@ -490,7 +496,7 @@ export async function deleteLabel(
490496
number: number,
491497
label: string,
492498
): Promise<void> {
493-
await client.deleteHashtag(number, label);
499+
await client.setHashtags(number, { remove: [label] });
494500
}
495501

496502
export function ensureCommentRemoval(

‎lib/modules/platform/gerrit/scm.spec.ts‎

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -430,63 +430,5 @@ describe('modules/platform/gerrit/scm', () => {
430430
pushOptions: ['notify=NONE', 'label=Code-Review+2'],
431431
});
432432
});
433-
434-
it('commitAndPush() - existing change with new changes - ensure labels', async () => {
435-
const existingChange = partial<GerritChange>({
436-
_number: 123456,
437-
change_id: '...',
438-
current_revision: 'commitSha',
439-
revisions: {
440-
commitSha: partial<GerritRevisionInfo>({ ref: 'refs/changes/1/2' }),
441-
},
442-
});
443-
clientMock.findChanges.mockResolvedValueOnce([existingChange]);
444-
git.prepareCommit.mockResolvedValueOnce({
445-
commitSha: 'commitSha' as LongCommitSha,
446-
parentCommitSha: 'parentSha' as LongCommitSha,
447-
files: [],
448-
});
449-
git.pushCommit.mockResolvedValueOnce(true);
450-
git.hasDiff.mockResolvedValueOnce(true);
451-
452-
expect(
453-
await gerritScm.commitAndPush({
454-
branchName: 'renovate/dependency-1.x',
455-
baseBranch: 'main',
456-
message: 'commit msg',
457-
files: [],
458-
prTitle: 'pr title',
459-
autoApprove: true,
460-
labels: ['hashtag1', 'hashtag2'],
461-
}),
462-
).toBe('commitSha');
463-
expect(git.prepareCommit).toHaveBeenCalledExactlyOnceWith({
464-
baseBranch: 'main',
465-
branchName: 'renovate/dependency-1.x',
466-
files: [],
467-
message: [
468-
'pr title',
469-
'Renovate-Branch: renovate/dependency-1.x\nChange-Id: ...',
470-
],
471-
prTitle: 'pr title',
472-
autoApprove: true,
473-
force: true,
474-
labels: ['hashtag1', 'hashtag2'],
475-
});
476-
expect(git.fetchRevSpec).toHaveBeenCalledExactlyOnceWith(
477-
'refs/changes/1/2',
478-
);
479-
expect(git.pushCommit).toHaveBeenCalledExactlyOnceWith({
480-
files: [],
481-
sourceRef: 'renovate/dependency-1.x',
482-
targetRef: 'refs/for/main',
483-
pushOptions: [
484-
'notify=NONE',
485-
'label=Code-Review+2',
486-
'hashtag=hashtag1',
487-
'hashtag=hashtag2',
488-
],
489-
});
490-
});
491433
});
492434
});

‎lib/modules/platform/gerrit/scm.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,11 +151,6 @@ export class GerritScm extends DefaultGitScm {
151151
if (commit.autoApprove) {
152152
pushOptions.push('label=Code-Review+2');
153153
}
154-
if (commit.labels) {
155-
for (const label of commit.labels) {
156-
pushOptions.push(`hashtag=${label}`);
157-
}
158-
}
159154
const pushResult = await git.pushCommit({
160155
sourceRef: commit.branchName,
161156
targetRef: `refs/for/${commit.baseBranch!}`,

‎lib/modules/platform/gerrit/types.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,8 @@ export interface GerritMergeableInfo {
143143
| 'CHERRY_PICK';
144144
mergeable: boolean;
145145
}
146+
147+
export interface GerritHashtagsInput {
148+
add?: string[];
149+
remove?: string[];
150+
}

‎lib/workers/repository/config-migration/branch/create.spec.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe('workers/repository/config-migration/branch/create', () => {
5151
message: 'Migrate config renovate.json',
5252
platformCommit: 'auto',
5353
force: true,
54-
labels: [],
54+
prTitle: 'Migrate Renovate config',
5555
});
5656
});
5757

@@ -78,7 +78,7 @@ describe('workers/repository/config-migration/branch/create', () => {
7878
message,
7979
platformCommit: 'auto',
8080
force: true,
81-
labels: [],
81+
prTitle: message,
8282
});
8383
});
8484

@@ -117,7 +117,7 @@ describe('workers/repository/config-migration/branch/create', () => {
117117
message: 'Migrate config renovate.json',
118118
platformCommit: 'auto',
119119
force: true,
120-
labels: [],
120+
prTitle: 'Migrate Renovate config',
121121
});
122122
});
123123

@@ -145,7 +145,7 @@ describe('workers/repository/config-migration/branch/create', () => {
145145
message,
146146
platformCommit: 'auto',
147147
force: true,
148-
labels: [],
148+
prTitle: 'PREFIX: migrate Renovate config',
149149
});
150150
});
151151
});
@@ -175,7 +175,7 @@ describe('workers/repository/config-migration/branch/create', () => {
175175
message,
176176
platformCommit: 'auto',
177177
force: true,
178-
labels: [],
178+
prTitle: `${prefix}: migrate Renovate config`,
179179
});
180180
});
181181

@@ -204,7 +204,7 @@ describe('workers/repository/config-migration/branch/create', () => {
204204
message,
205205
platformCommit: 'auto',
206206
force: true,
207-
labels: [],
207+
prTitle: `${prefix}: migrate Renovate config`,
208208
});
209209
});
210210
});

‎lib/workers/repository/config-migration/branch/create.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ export async function createConfigMigrationBranch(
7474
message: commitMessage.toString(),
7575
platformCommit: config.platformCommit,
7676
force: true,
77-
labels: config.labels,
77+
// Only needed by Gerrit platform
78+
prTitle: commitMessageFactory.getPrTitle(),
7879
});
7980
}

‎lib/workers/repository/config-migration/branch/rebase.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ describe('workers/repository/config-migration/branch/rebase', () => {
110110
message: `Migrate config ${filename}`,
111111
platformCommit: 'auto',
112112
baseBranch: 'dev',
113-
labels: [],
113+
prTitle: 'Migrate Renovate config',
114114
});
115115
},
116116
);

0 commit comments

Comments
 (0)