Fix review event mapping and body validation
- Map CLI 'approve' to Forgejo's expected 'APPROVED' event. - Require a non-empty body for request-changes and comment events. - Update API test expectation and add CLI tests for body validation.
This commit is contained in:
parent
fdb8dacebc
commit
6b0b3729f3
3 changed files with 45 additions and 4 deletions
|
|
@ -747,7 +747,7 @@ pr
|
|||
const config = loadConfig();
|
||||
const client = ForgejoClient.fromConfig(config);
|
||||
const eventMap = {
|
||||
approve: 'APPROVE',
|
||||
approve: 'APPROVED',
|
||||
'request-changes': 'REQUEST_CHANGES',
|
||||
comment: 'COMMENT',
|
||||
};
|
||||
|
|
@ -756,7 +756,11 @@ pr
|
|||
console.error(`Invalid review event: ${options.event}. Must be approve, request-changes, or comment.`);
|
||||
process.exit(1);
|
||||
}
|
||||
const body = readBodyOption(options) || '';
|
||||
const body = (readBodyOption(options) || '').trim();
|
||||
if (event !== 'APPROVED' && body.length === 0) {
|
||||
console.error(`Review event ${options.event} requires a non-empty body. Use -b/--body or --body-file.`);
|
||||
process.exit(1);
|
||||
}
|
||||
await client.createPullRequestReview(options.owner, options.repo, options.number, event, body);
|
||||
console.log(`Review submitted on !${options.number}: ${event}.`);
|
||||
} catch (err) {
|
||||
|
|
|
|||
|
|
@ -163,10 +163,10 @@ test('createPullRequestComment posts to the issue comments endpoint', async () =
|
|||
test('createPullRequestReview posts the review event and body', async () => {
|
||||
const calls = mockFetch(() => jsonResponse({ id: 88 }));
|
||||
const client = new ForgejoClient('https://forge.test', 'tok');
|
||||
await client.createPullRequestReview('owner', 'repo', 7, 'APPROVE', 'Ship it.');
|
||||
await client.createPullRequestReview('owner', 'repo', 7, 'APPROVED', 'Ship it.');
|
||||
assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/owner/repo/pulls/7/reviews');
|
||||
assert.equal(calls[0].opts.method, 'POST');
|
||||
const body = JSON.parse(calls[0].opts.body);
|
||||
assert.equal(body.event, 'APPROVE');
|
||||
assert.equal(body.event, 'APPROVED');
|
||||
assert.equal(body.body, 'Ship it.');
|
||||
});
|
||||
|
|
|
|||
|
|
@ -111,3 +111,40 @@ test('pr review rejects an invalid event before any network call', () => {
|
|||
fs.unlinkSync(cfg);
|
||||
}
|
||||
});
|
||||
|
||||
test('pr review request-changes rejects a missing body before any network call', () => {
|
||||
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
|
||||
fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
|
||||
try {
|
||||
const res = run(['pr', 'review', '-o', 'o', '-r', 'r', '-n', '1', '--event', 'request-changes'], { STOKE_CONFIG_FILE: cfg });
|
||||
assert.equal(res.status, 1);
|
||||
assert.match(res.stderr, /requires a non-empty body/);
|
||||
} finally {
|
||||
fs.unlinkSync(cfg);
|
||||
}
|
||||
});
|
||||
|
||||
test('pr review comment rejects a whitespace-only body before any network call', () => {
|
||||
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
|
||||
fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
|
||||
try {
|
||||
const res = run(['pr', 'review', '-o', 'o', '-r', 'r', '-n', '1', '--event', 'comment', '-b', ' '], { STOKE_CONFIG_FILE: cfg });
|
||||
assert.equal(res.status, 1);
|
||||
assert.match(res.stderr, /requires a non-empty body/);
|
||||
} finally {
|
||||
fs.unlinkSync(cfg);
|
||||
}
|
||||
});
|
||||
|
||||
test('pr review approve allows an empty body before any network call', () => {
|
||||
const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
|
||||
fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
|
||||
try {
|
||||
const res = run(['pr', 'review', '-o', 'o', '-r', 'r', '-n', '1', '--event', 'approve'], { STOKE_CONFIG_FILE: cfg });
|
||||
// It fails at the network call, not at validation.
|
||||
assert.equal(res.status, 1);
|
||||
assert.doesNotMatch(res.stderr, /requires a non-empty body/);
|
||||
} finally {
|
||||
fs.unlinkSync(cfg);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue