Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support adding labels to PRs #173

Merged
merged 9 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ The `createPullRequest()` method creates a GitHub Pull request with the files gi
| message | `string` | The commit message for the changes. Default is `'code suggestions'`. We recommend following [conventional commits](https://www.conventionalcommits.org/en/v1.0.0/).|
| force | `boolean` | Whether or not to force push the reference even if the ancestor commits differs. Default is `false`. |
| fork | `boolean` | Whether or not code suggestion should be made from a fork, defaults to `true` (_Note: forking does not work when using `secrets.GITHUB_TOKEN` in an action_). |
| labels | `string[]`| The list of labels to add to the pull request. Default is none. |

#### `logger`
*[Logger](https://www.npmjs.com/package/@types/pino)* <br>
Expand Down Expand Up @@ -231,6 +232,10 @@ Whether or not to force push a reference with different commit history before th
*boolean* <br>
Whether or not to attempt forking to a separate repository. Default value is: `true`.

#### `--labels`
*array* <br>
The list of labels to add to the pull request. Default is none.

### Example
```
code-suggester pr -o foo -r bar -d 'description' -t 'title' -m 'message' --git-dir=.
Expand Down Expand Up @@ -328,6 +333,10 @@ Whether or not maintainers can modify the pull request. Default value is: `true`
*boolean* <br>
Whether or not to attempt forking to a separate repository. Default value is: `true`.

#### `labels`
*array* <br>
The list of labels to add to the pull request. Default is none.

#### Example

The following example is a `.github/workflows/main.yaml` file in repo `Octocat/HelloWorld`. This would add a LICENSE folder to the root `HelloWorld` repo on every pull request if it is not already there.
Expand Down Expand Up @@ -356,6 +365,9 @@ jobs:
message: 'chore(license): add license file'
branch: my-branch
git_dir: '.'
labels: |
bug
priority: p1
```

### Review a Pull Request
Expand Down
6 changes: 6 additions & 0 deletions src/bin/code-suggester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ yargs
default: true,
type: 'boolean',
},
labels: {
describe:
'The list of labels to add to the pull request. Default is none.',
default: [],
type: 'array',
},
})
.command(REVIEW_PR_COMMAND, 'Review an open pull request', {
'upstream-repo': {
Expand Down
1 change: 1 addition & 0 deletions src/bin/workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export function coerceUserCreatePullRequestOptions(): CreatePullRequestUserOptio
primary: yargs.argv.primary as string,
maintainersCanModify: yargs.argv.maintainersCanModify as boolean,
fork: yargs.argv.fork as boolean,
labels: yargs.argv.labels as string[],
};
}

Expand Down
1 change: 1 addition & 0 deletions src/github-handler/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ export {branch} from './branch-handler';
export {commitAndPush} from './commit-and-push-handler';
export * from './pull-request-handler';
export * from './comment-handler';
export * from './issue-handler';
52 changes: 52 additions & 0 deletions src/github-handler/issue-handler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright 2021 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import {BranchDomain, RepoDomain} from '../types';
import {Octokit} from '@octokit/rest';
import {logger} from '../logger';

/**
* Create a GitHub PR on the upstream organization's repo
* Throws an error if the GitHub API fails
* @param {Octokit} octokit The authenticated octokit instance
* @param {RepoDomain} upstream The upstream repository
* @param {BranchDomain} origin The remote origin information that contains the origin branch
* @param {number} issue_number The issue number to add labels to. Can also be a PR number
* @param {string[]} labels The list of labels to apply to the issue/pull request. Default is []. the funciton will no-op.
* @returns {Promise<string[]>} The list of resulting labels after the addition of the given labels
*/
async function addLabels(
octokit: Octokit,
upstream: RepoDomain,
origin: BranchDomain,
issue_number: number,
labels?: string[]
): Promise<string[]> {
if (!labels || labels.length === 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bcoe moved the check on labels into addLabels instead of around the call site.

return [];
}

const labelsResponseData = (
await octokit.issues.addLabels({
owner: upstream.owner,
repo: origin.repo,
issue_number: issue_number,
labels: labels,
})
).data;
logger.info(`Successfully added labels ${labels} to issue: ${issue_number}`);
return labelsResponseData.map(l => l.name);
}

export {addLabels};
10 changes: 10 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,16 @@ async function createPullRequest(
gitHubConfigs.primary
);
logger.info(`Successfully opened pull request: ${prNumber}.`);

// addLabels will no-op if options.labels is undefined or empty.
await handler.addLabels(
octokit,
upstream,
originBranch,
prNumber,
options.labels
);

return prNumber;
}

Expand Down
2 changes: 2 additions & 0 deletions src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ export interface CreatePullRequestUserOptions {
primary?: string;
// Whether or not maintainers can modify the PR. Default is true. (optional)
maintainersCanModify?: boolean;
// The list of labels to apply to the newly created PR. Default is empty. (optional)
labels?: string[];
}

/**
Expand Down
1 change: 1 addition & 0 deletions test/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ describe('Mapping pr yargs to create PR options', () => {
primary: 'primary',
maintainersCanModify: true,
fork: true,
labels: ['automerge'],
};

sandbox.stub(yargs, 'argv').value({_: ['pr'], ...options});
Expand Down
20 changes: 20 additions & 0 deletions test/fixtures/add-labels-response.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[
{
"id": 208045946,
"node_id": "MDU6TGFiZWwyMDgwNDU5NDY=",
"url": "https://api.github.com/repos/octocat/Hello-World/labels/bug",
"name": "bug",
"description": "Something isn't working",
"color": "f29513",
"default": true
},
{
"id": 208045947,
"node_id": "MDU6TGFiZWwyMDgwNDU5NDc=",
"url": "https://api.github.com/repos/octocat/Hello-World/labels/enhancement",
"name": "enhancement",
"description": "New feature or request",
"color": "a2eeef",
"default": false
}
]
110 changes: 110 additions & 0 deletions test/issues.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// Copyright 2021 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import {expect} from 'chai';
import {describe, it, before, afterEach} from 'mocha';
import {octokit, setup} from './util';
import * as sinon from 'sinon';
import {addLabels} from '../src/github-handler/issue-handler';

before(() => {
setup();
});

describe('Adding labels', async () => {
const sandbox = sinon.createSandbox();
const upstream = {owner: 'upstream-owner', repo: 'upstream-repo'};
const origin = {
owner: 'origin-owner',
repo: 'origin-repo',
branch: 'issues-test-branch',
};
const issue_number = 1;
const labels = ['enhancement'];
afterEach(() => {
sandbox.restore();
});

it('Invokes octokit issues add labels on an existing pull request', async () => {
// setup
const responseAddLabelsData = await import(
'./fixtures/add-labels-response.json'
);
const addLabelsResponse = {
headers: {},
status: 200,
url: 'http://fake-url.com',
data: responseAddLabelsData,
};
const stub = sandbox
.stub(octokit.issues, 'addLabels')
.resolves(addLabelsResponse);
// tests
const resultingLabels = await addLabels(
octokit,
upstream,
origin,
issue_number,
labels
);
sandbox.assert.calledOnceWithExactly(stub, {
owner: upstream.owner,
repo: origin.repo,
issue_number: issue_number,
labels: labels,
});
expect(resultingLabels).to.deep.equal(['bug', 'enhancement']);
});

it('No-op undefined labels', async () => {
// setup
const stub = sandbox.stub(octokit.issues, 'addLabels').resolves();
// tests
const resultingLabels = await addLabels(
octokit,
upstream,
origin,
issue_number
);
sandbox.assert.neverCalledWith(stub, sinon.match.any);
expect(resultingLabels).to.deep.equal([]);
});

it('No-op with empty labels', async () => {
Comment on lines +70 to +84
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bcoe added no-op tests.

// setup
const stub = sandbox.stub(octokit.issues, 'addLabels').resolves();
// tests
const resultingLabels = await addLabels(
octokit,
upstream,
origin,
issue_number,
[]
);
sandbox.assert.neverCalledWith(stub, sinon.match.any);
expect(resultingLabels).to.deep.equal([]);
});

it('Passes up the error message with a throw when octokit issues add labels fails', async () => {
// setup
const errorMsg = 'Error message';
sandbox.stub(octokit.issues, 'addLabels').rejects(Error(errorMsg));
try {
await addLabels(octokit, upstream, origin, issue_number, labels);
expect.fail();
} catch (err) {
expect(err.message).to.equal(errorMsg);
}
});
});
30 changes: 30 additions & 0 deletions test/main-make-pr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ describe('Make PR main function', () => {
const primary = 'custom-primary';
const originRepo = 'Hello-World';
const originOwner = 'octocat';
const labelsToAdd = ['automerge'];
const options: CreatePullRequestUserOptions = {
upstreamOwner,
upstreamRepo,
Expand All @@ -48,6 +49,7 @@ describe('Make PR main function', () => {
force,
message,
primary,
labels: labelsToAdd,
};
const oldHeadSha = '7fd1a60b01f91b314f59955a4e4d4e80d8edf11d';
const changes: Changes = new Map();
Expand Down Expand Up @@ -116,6 +118,20 @@ describe('Make PR main function', () => {
expect(testMaintainersCanModify).equals(maintainersCanModify);
expect(testPrimary).equals(primary);
},
addLabels: (
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bcoe Added stub helpers for what actually seems to be the main tests for createPullRequest.

octokit: Octokit,
upstream: {owner: string; repo: string},
originBranch: {owner: string; repo: string; branch: string},
issue_number: number,
labels: string[],
) => {
expect(originBranch.owner).equals(originOwner);
expect(originBranch.repo).equals(originRepo);
expect(originBranch.branch).equals(branch);
expect(upstream.owner).equals(upstreamOwner);
expect(upstream.repo).equals(upstreamRepo);
expect(labels).equals(labelsToAdd);
}
};
const stubMakePr = proxyquire.noCallThru()('../src/', {
'./github-handler': stubHelperHandlers,
Expand Down Expand Up @@ -171,6 +187,20 @@ describe('Make PR main function', () => {
expect(testMaintainersCanModify).equals(maintainersCanModify);
expect(testPrimary).equals(primary);
},
addLabels: (
octokit: Octokit,
upstream: {owner: string; repo: string},
originBranch: {owner: string; repo: string; branch: string},
issue_number: number,
labels: string[],
) => {
expect(originBranch.owner).equals(upstreamOwner);
expect(originBranch.repo).equals(upstreamRepo);
expect(originBranch.branch).equals(branch);
expect(upstream.owner).equals(upstreamOwner);
expect(upstream.repo).equals(upstreamRepo);
expect(labels).equals(labelsToAdd);
}
};
const stubMakePr = proxyquire.noCallThru()('../src/', {
'./github-handler': stubHelperHandlers,
Expand Down