Skip to content

Commit

Permalink
Add support for label merge
Browse files Browse the repository at this point in the history
There are two scenarios where handling a label configuration would require merging one label into another:

- The configured label exists in the current labels and its alias exists in the current labels
- Multiple aliases of a configured label exist in the current labels

Previously, "GitHub Label Sync" did not have support for label merge, and would return an error when the user's
configuration would have required a merge operation.

The merge capability is added here.

The GitHub labels API does not provide a merge capability, so merging labels requires the use of the issues API to
add the "expected" label to each issue/PR that has the "actual" label.

From the user's perspective, the outcome of this "merge" operation is no different from any simple "changed" operation:
the "actual" label has been replaced by the "expected" label in the repository labels and all issues/PRs the label was
applied to.
  • Loading branch information
per1234 authored and JakeChampion committed Apr 4, 2022
1 parent 91245b1 commit 65b6359
Show file tree
Hide file tree
Showing 6 changed files with 385 additions and 9 deletions.
22 changes: 22 additions & 0 deletions lib/action-label-diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,28 @@ function actionLabelDiff(options) {
if (diffEntry.type === 'changed') {
return apiClient.updateLabel(repo, diffEntry.name, diffEntry.expected);
}
if (diffEntry.type === 'merge') {
return apiClient.getLabeledIssues(repo, diffEntry.name) // Get all issues and PRs that have the "actual" label
.then((issues) => {
const mergeIssues = issues.filter((issue) => {
// Per-issue label application is needed where the "expected" label is not already present
return !issue.labels.some((label) => {
return label.name === diffEntry.expected.name;
});
});

// Add the "expected" label to the issues
const issueActions = mergeIssues.map((issue) => {
return apiClient.labelIssue(repo, issue.number, diffEntry.expected.name);
});

return Promise.all(issueActions);
})
.then(() => {
// Now that all issues with "actual" label have "expected" label, "actual" label can be deleted
return apiClient.deleteLabel(repo, diffEntry.name);
});
}
if (diffEntry.type === 'added') {
return apiClient.deleteLabel(repo, diffEntry.name);
}
Expand Down
48 changes: 48 additions & 0 deletions lib/github-label-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,54 @@ class ApiClient {
});
}

getLabeledIssues (repo, labelName) {
return new Promise((resolve, reject) => {
const allIssues = [];
const endpoint = `/repos/${repo}/issues`;
const params = {
labels: labelName,
page: 1,
per_page: 100,
state: 'all'
};
const getCallback = (error, status, issues) => {
if (error) {
error.method = 'GET';
error.endpoint = endpoint;
return reject(error);
}
if (status !== 200) {
return reject(new Error(`API responded with ${status} status`));
}
allIssues.push.apply(allIssues, issues);
if (issues.length === params.per_page) {
params.page += 1;
this.apiClient.get(endpoint, params, getCallback);
} else {
resolve(allIssues);
}
};
this.apiClient.get(endpoint, params, getCallback);
});
}

labelIssue (repo, issueNumber, labelName) {
return new Promise((resolve, reject) => {
const endpoint = `/repos/${repo}/issues/${issueNumber}/labels`;
this.apiClient.post(endpoint, {labels: [labelName]}, (error, status) => {
if (error) {
error.method = 'POST';
error.endpoint = endpoint;
return reject(error);
}
if (status !== 200) {
return reject(new Error(`API responded with ${status} status`));
}
resolve();
});
});
}

deleteLabel (repo, labelName) {
labelName = encodeURIComponent(labelName);
return new Promise((resolve, reject) => {
Expand Down
9 changes: 0 additions & 9 deletions lib/github-label-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,6 @@ function githubLabelSync(options) {
stringifyLabelDiff(labelDiff).forEach((diffLine) => {
log.info(format.diff(diffLine));
});

const labelDiffMerges = labelDiff.filter((diff) => {
return diff.type === 'merge';
});
if (labelDiffMerges.length) {
const messages = labelDiffMerges.map(diff => `Configuration of the "${diff.expected.name}" label requires unsupported merge operation`);
return Promise.reject({ message: messages.join('\n\n') });
}

return labelDiff;
})
.then((labelDiff) => {
Expand Down
115 changes: 115 additions & 0 deletions test/unit/lib/action-label-diff.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,121 @@ describe('lib/action-label-diff', () => {
assert.strictEqual(actions[0], updatePromise);
});

it('should convert "merge" diff entries to label API delete promises', () => {
const issues = [
{
number: 11,
labels: [
{
name: 'bar'
}
]
},
{
number: 42,
labels: [
{
name: 'bar'
},
{
name: 'baz'
}
]
}
];
apiClient.getLabeledIssues.returns(Promise.resolve(issues));
apiClient.labelIssue.returns(Promise.resolve());
const deletePromiseValue = 'asdf';
apiClient.deleteLabel.returns(Promise.resolve(deletePromiseValue));
options.diff.push({
name: 'foo',
type: 'merge',
actual: {
name: 'bar',
color: 'ff0000',
description: 'baz'
},
expected: {
name: 'foo',
color: '00ff00',
description: 'baz'
}
});
actions = actionLabelDiff(options);
assert.lengthEquals(actions, 1);
return actions[0]
.then((value) => {
assert.calledOnce(apiClient.getLabeledIssues);
assert.calledWithExactly(apiClient.getLabeledIssues, options.repo, options.diff[0].name);
assert.calledTwice(apiClient.labelIssue);
assert.calledWithExactly(apiClient.labelIssue.getCall(0), options.repo, issues[0].number, options.diff[0].expected.name);
assert.calledWithExactly(apiClient.labelIssue.getCall(1), options.repo, issues[1].number, options.diff[0].expected.name);
assert.calledOnce(apiClient.deleteLabel);
assert.calledWithExactly(apiClient.deleteLabel, options.repo, options.diff[0].name);
assert.strictEqual(value, deletePromiseValue);
});
});

it('should not attempt redundant issue labeling for "merge" diff entries', () => {
const issues = [
{
number: 11,
labels: [
{
name: 'bar'
}
]
},
{
number: 42,
labels: [
{
name: 'bar'
},
{
name: 'foo'
}
]
},
{
number: 123,
labels: [
{
name: 'bar'
},
{
name: 'baz'
}
]
}
];
apiClient.getLabeledIssues.returns(Promise.resolve(issues));
apiClient.labelIssue.returns(Promise.resolve());
apiClient.deleteLabel.returns(Promise.resolve());
options.diff.push({
name: 'foo',
type: 'merge',
actual: {
name: 'bar',
color: 'ff0000',
description: 'baz'
},
expected: {
name: 'foo',
color: '00ff00',
description: 'baz'
}
});
actions = actionLabelDiff(options);
assert.lengthEquals(actions, 1);
return actions[0]
.then(() => {
assert.calledTwice(apiClient.labelIssue);
assert.calledWithExactly(apiClient.labelIssue.getCall(0), options.repo, issues[0].number, options.diff[0].expected.name);
assert.calledWithExactly(apiClient.labelIssue.getCall(1), options.repo, issues[2].number, options.diff[0].expected.name);
});
});

it('should convert "added" diff entries to label API delete promises', () => {
const deletePromise = Promise.resolve();
apiClient.deleteLabel.returns(deletePromise);
Expand Down
Loading

0 comments on commit 65b6359

Please sign in to comment.