feat: add pr edit, close, and reopen (task 0011)
Add the PR-side state mutations mirroring the issue-side slice: - `pr edit` applies title/body/base/milestone and the recomputed assignee list in one PATCH, with additive label endpoints and (per the ADR 0007 amendment) the dedicated requested-reviewers POST/DELETE endpoints for `--add-reviewer`/`--remove-reviewer`. - `pr close --comment` posts the comment after the PATCH and surfaces a comment-post failure; an already-closed or merged PR is an `already: true` no-op reporting the actual state. - `pr reopen` is an `already: true` no-op when already open. Extract the fetch-then-patch assignee merge into a shared `src/assignees.ts` (`mergeAssignees` + `assigneeLogins`), now used by both `issue edit` and `pr edit`.
This commit was merged in pull request #11.
This commit is contained in:
@@ -14,9 +14,54 @@ Success outputs follow the action-block pattern: `edited:`/`closed:`/`reopened:`
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] `pr edit` applies title, body, milestone, and base changes and outputs `edited: { number, status: "ok" }`
|
||||
- [ ] Label and assignee mutations follow the same rules as `issue edit` (additive endpoints, fetch-then-patch, unapplied-label silent success)
|
||||
- [ ] `--add-reviewer`/`--remove-reviewer` call the requested-reviewers endpoints with `{ reviewers: [login] }`
|
||||
- [ ] `pr close --comment` posts the comment after the PATCH and surfaces a comment failure; closing an already-closed-or-merged PR returns the entity block with `already: true`
|
||||
- [ ] `pr reopen` on an open PR returns the entity block with `already: true`; otherwise outputs `reopened: { number, status: "ok" }`
|
||||
- [ ] Fixture-server tests cover reviewer add/remove, the merged-PR close no-op, and the reopen paths
|
||||
- [x] `pr edit` applies title, body, milestone, and base changes and outputs `edited: { number, status: "ok" }`
|
||||
- [x] Label and assignee mutations follow the same rules as `issue edit` (additive endpoints, fetch-then-patch, unapplied-label silent success)
|
||||
- [x] `--add-reviewer`/`--remove-reviewer` call the requested-reviewers endpoints with `{ reviewers: [login] }`
|
||||
- [x] `pr close --comment` posts the comment after the PATCH and surfaces a comment failure; closing an already-closed-or-merged PR returns the entity block with `already: true`
|
||||
- [x] `pr reopen` on an open PR returns the entity block with `already: true`; otherwise outputs `reopened: { number, status: "ok" }`
|
||||
- [x] Fixture-server tests cover reviewer add/remove, the merged-PR close no-op, and the reopen paths
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
No criteria were dropped or altered; all six are satisfied.
|
||||
|
||||
Decisions made mid-implementation:
|
||||
|
||||
- The close no-op reports the actual state: `merged` for a merged PR (whose raw
|
||||
`state` Gitea reports as `closed`), otherwise the raw state — computed by a
|
||||
small `pullState` helper. `pull.state === "closed"` catches both the closed and
|
||||
merged cases for the short-circuit, matching the spec's "already closed or
|
||||
merged".
|
||||
- `pr reopen` short-circuits only on `state === "open"`, per the spec. A merged
|
||||
PR (state `closed`) therefore falls through to the PATCH and Gitea rejects it as
|
||||
a `VALIDATION_ERROR` — the spec asks for no merged-guard on reopen, mirroring the
|
||||
issue side.
|
||||
- Reviewer mutations are one POST for all `--add-reviewer` and one DELETE for all
|
||||
`--remove-reviewer`, each carrying the whole list — the requested-reviewers
|
||||
endpoints take arrays (ADR 0007 amendment). They are not fetch-then-patch and
|
||||
are not idempotency-checked; a redundant add/remove surfaces whatever Gitea
|
||||
answers.
|
||||
- `--add-label`/`--remove-label`/`--add-assignee`/`--remove-assignee`/`--add-reviewer`/`--remove-reviewer`
|
||||
are repeatable, matching `issue edit`.
|
||||
- Name resolution (milestone, remove-label ids) runs before any mutation so a typo
|
||||
is reported before a change lands. Title/body/base/milestone and the recomputed
|
||||
assignee list travel in a single PATCH; labels and reviewers use their dedicated
|
||||
endpoints afterward.
|
||||
- Added a `VALIDATION_ERROR` when `pr edit` is invoked with no changes, matching
|
||||
`issue edit`.
|
||||
- Review finding (Duplicated Code): extracted the fetch-then-patch merge into a
|
||||
shared `src/assignees.ts` — a pure `mergeAssignees` plus an `assigneeLogins`
|
||||
reader — now used by both `issue edit` and `pr edit`, replacing the inline copy
|
||||
that previously lived in `issue.ts`.
|
||||
|
||||
Follow-ups worth flagging (unaddressed review findings, both judgement calls):
|
||||
|
||||
- The close/reopen state-machine (read state → no-op short-circuit → PATCH `{state}`
|
||||
→ render) is still duplicated between `issue.ts` and `pr.ts`. A shared helper was
|
||||
left unextracted because the two sides diverge in their no-op shape, help
|
||||
suggestions, and the PR-only merged handling, which would make the abstraction
|
||||
leaky.
|
||||
- The no-op output shape differs between the issue side (`message: "Already
|
||||
closed"`) and the PR side (`already: true` + `state`). This is spec-driven — the
|
||||
spec fixes `already: true` for PRs — but the CLI's no-op output is not uniform
|
||||
across the two entities.
|
||||
|
||||
Reference in New Issue
Block a user