Skip to content

Commit 393f8e2

Browse files
NssGouravNssGourav
committed
fix: make revision guard draft conversion optional
Signed-off-by: Gourav NSS <gourav341111@gmail.com>
1 parent 9158691 commit 393f8e2

4 files changed

Lines changed: 114 additions & 26 deletions

File tree

‎.github/scripts/revision-guard/index.js‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ module.exports = async function revisionGuard({ github, context, core }) {
1818
!repo?.repo ||
1919
!pr ||
2020
typeof pr.number !== 'number' ||
21-
typeof pr.node_id !== 'string' ||
2221
reviewState !== 'changes_requested'
2322
) {
2423
core?.info?.('Skipping revision guard due to missing or non-matching payload data.');
@@ -30,27 +29,30 @@ module.exports = async function revisionGuard({ github, context, core }) {
3029
return;
3130
}
3231

33-
if (isDraft(pr)) {
34-
core?.info?.(`Skipping PR #${pr.number} because it is already a draft.`);
35-
return;
36-
}
32+
const labelsToRemove = getPresentManagedLabels(pr.labels);
3733

38-
try {
39-
await convertToDraft(github, {
40-
pullRequestId: pr.node_id,
41-
});
42-
core?.info?.(`Converted PR #${pr.number} to draft.`);
43-
} catch (error) {
44-
core?.error?.(`Failed to convert PR #${pr.number} to draft: ${error.message}`);
45-
if (!process.env.REVIEWBOT_TOKEN) {
34+
if (!isDraft(pr)) {
35+
if (process.env.REVIEWBOT_TOKEN && typeof pr.node_id === 'string') {
36+
try {
37+
await convertToDraft(github, {
38+
pullRequestId: pr.node_id,
39+
});
40+
core?.info?.(`Converted PR #${pr.number} to draft.`);
41+
} catch (error) {
42+
core?.error?.(
43+
`Failed to convert PR #${pr.number} to draft: ${error.message}. ` +
44+
'Continuing with managed label cleanup.'
45+
);
46+
}
47+
} else {
4648
core?.info?.(
47-
'Hint: configure REVIEWBOT_TOKEN with permission to convert pull requests to draft.'
49+
`Skipping draft conversion for PR #${pr.number}; REVIEWBOT_TOKEN is not configured.`
4850
);
4951
}
50-
throw error;
52+
} else {
53+
core?.info?.(`PR #${pr.number} is already a draft.`);
5154
}
5255

53-
const labelsToRemove = getPresentManagedLabels(pr.labels);
5456
if (labelsToRemove.length === 0) {
5557
core?.info?.(`No managed labels to remove for PR #${pr.number}.`);
5658
return;
@@ -71,6 +73,6 @@ module.exports = async function revisionGuard({ github, context, core }) {
7173
`Failed to remove labels from PR #${pr.number}: ${error.message}. ` +
7274
`Labels to remove: ${labelsToRemove.join(', ')}.`
7375
);
74-
// Don't re-throw; draft conversion succeeded and is the primary goal
76+
throw error;
7577
}
7678
};

‎.github/scripts/revision-guard/index.test.js‎

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,19 @@ function freshRequire() {
2020
function createGithubMock() {
2121
const removedLabels = [];
2222
const graphqlCalls = [];
23+
let graphqlError;
2324

2425
return {
2526
removedLabels,
2627
graphqlCalls,
28+
failGraphql(error) {
29+
graphqlError = error;
30+
},
2731
graphql: async (query, variables) => {
2832
graphqlCalls.push({ query, variables });
33+
if (graphqlError) {
34+
throw graphqlError;
35+
}
2936
return {
3037
convertPullRequestToDraft: {
3138
pullRequest: {
@@ -69,22 +76,22 @@ function createContext(overrides = {}) {
6976
describe('revision-guard index', () => {
7077
beforeEach(() => {
7178
delete process.env.REVISION_GUARD_MANAGED_LABELS;
79+
delete process.env.REVIEWBOT_TOKEN;
7280
});
7381

7482
afterEach(() => {
7583
delete process.env.REVISION_GUARD_MANAGED_LABELS;
84+
delete process.env.REVIEWBOT_TOKEN;
7685
});
7786

78-
it('converts a ready PR to draft and removes only managed labels', async () => {
87+
it('removes only managed labels without draft conversion when no review bot token is configured', async () => {
7988
const handler = freshRequire();
8089
const github = createGithubMock();
8190
const context = createContext();
8291

8392
await handler({ github, context, core: { info() {} } });
8493

85-
assert.equal(github.graphqlCalls.length, 1);
86-
assert.match(github.graphqlCalls[0].query, /convertPullRequestToDraft/);
87-
assert.deepEqual(github.graphqlCalls[0].variables, { pullRequestId: 'PR_node_42' });
94+
assert.equal(github.graphqlCalls.length, 0);
8895
assert.deepEqual(github.removedLabels, [
8996
'queue:committers',
9097
'status: ready-to-merge',
@@ -110,7 +117,7 @@ describe('revision-guard index', () => {
110117
assert.equal(github.removedLabels.length, 0);
111118
});
112119

113-
it('skips already-draft PRs', async () => {
120+
it('removes managed labels from already-draft PRs without converting again', async () => {
114121
const handler = freshRequire();
115122
const github = createGithubMock();
116123
const context = createContext({
@@ -126,7 +133,40 @@ describe('revision-guard index', () => {
126133
await handler({ github, context, core: { info() {} } });
127134

128135
assert.equal(github.graphqlCalls.length, 0);
129-
assert.equal(github.removedLabels.length, 0);
136+
assert.deepEqual(github.removedLabels, ['queue:committers']);
137+
});
138+
139+
it('converts to draft when a review bot token is configured', async () => {
140+
process.env.REVIEWBOT_TOKEN = 'token';
141+
const handler = freshRequire();
142+
const github = createGithubMock();
143+
const context = createContext();
144+
145+
await handler({ github, context, core: { info() {} } });
146+
147+
assert.equal(github.graphqlCalls.length, 1);
148+
assert.match(github.graphqlCalls[0].query, /convertPullRequestToDraft/);
149+
assert.deepEqual(github.graphqlCalls[0].variables, { pullRequestId: 'PR_node_42' });
150+
assert.deepEqual(github.removedLabels, [
151+
'queue:committers',
152+
'status: ready-to-merge',
153+
]);
154+
});
155+
156+
it('still removes managed labels when draft conversion fails', async () => {
157+
process.env.REVIEWBOT_TOKEN = 'token';
158+
const handler = freshRequire();
159+
const github = createGithubMock();
160+
github.failGraphql(new Error('Resource not accessible by integration'));
161+
const context = createContext();
162+
163+
await handler({ github, context, core: { error() {}, info() {} } });
164+
165+
assert.equal(github.graphqlCalls.length, 1);
166+
assert.deepEqual(github.removedLabels, [
167+
'queue:committers',
168+
'status: ready-to-merge',
169+
]);
130170
});
131171

132172
it('uses configurable managed labels and still removes defaults', async () => {
@@ -149,9 +189,7 @@ describe('revision-guard index', () => {
149189

150190
await handler({ github, context, core: { info() {} } });
151191

152-
// Draft conversion must also fire for configurable-label scenarios.
153-
assert.equal(github.graphqlCalls.length, 1);
154-
assert.deepEqual(github.graphqlCalls[0].variables, { pullRequestId: 'PR_node_45' });
192+
assert.equal(github.graphqlCalls.length, 0);
155193
// Custom labels AND the matching default (queue:committers) must both be removed.
156194
assert.deepEqual(github.removedLabels, ['queue:committers', 'custom: one', 'custom: two']);
157195
});

‎.github/scripts/revision-guard/package.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"name": "revision-guard",
3+
"version": "1.0.0",
4+
"description": "Revision guard automation scripts and tests",
5+
"private": true,
6+
"main": "index.js",
7+
"scripts": {
8+
"test": "node --test index.test.js"
9+
}
10+
}

‎.github/workflows/test-revision-guard.yml‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
name: Test Revision Guard
2+
3+
on:
4+
push:
5+
paths:
6+
- '.github/scripts/revision-guard/**'
7+
- '.github/workflows/test-revision-guard.yml'
8+
pull_request:
9+
paths:
10+
- '.github/scripts/revision-guard/**'
11+
- '.github/workflows/test-revision-guard.yml'
12+
13+
permissions:
14+
contents: read
15+
16+
jobs:
17+
test:
18+
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false
19+
runs-on: hl-sdk-py-lin-md
20+
permissions:
21+
contents: read
22+
steps:
23+
- name: Harden the runner
24+
uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4
25+
with:
26+
egress-policy: audit
27+
28+
- name: Checkout repository
29+
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
30+
31+
- name: Setup Node.js
32+
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
33+
with:
34+
node-version: '20'
35+
36+
- name: Run Tests
37+
working-directory: .github/scripts/revision-guard
38+
run: npm test

0 commit comments

Comments
 (0)