Review a proposal from the phone: approve, request changes or comment from its Reviews section (refs #4) af9f2e514c72
- Author
- Andy McCall @andymccall <[email protected]>
- via
- agent
agent:claude@andymccall <[email protected]> · keyless, under your grant for changes,refs,seal · key 62a52df03159
- When
- · 2026-10-27 22:00 UTC
- Signature
- ◈ sealed by agent:claude@andymccall
- Hash
af9f2e514c728d0b211e16a698eb9bde4521747c3a4976ac0ba8433c8f831369- Depends on
- 40ab4df121e0 57c0824300db 5ae4af48ddd2 98dbd1220929 aa21eb5e0e5b e2df27a81e5d
- Depended on by
- 840c75813f86 0ca942a9a1e4 ca672a3ada1f
- In bookmarks
- main
- In tags
- v0.1.0 v0.1.1 v0.1.2 v0.1.3 v0.1.4 v0.1.5 v0.1.6 v0.1.7 v0.1.8 v0.1.9
- Edits
- 13 atoms, 0 tree operations, 7 files
Browse the tree at this change
modified CHANGELOG.md +1 −0
| … | 1 unchanged lines |
||
| 2 | 2 | ||
| 3 | 3 | ## Unreleased |
|
| 4 | 4 | ||
| 5 | + | - **Review a proposal from the phone (#4):** an open proposal's Reviews section takes a review: Approve, Request changes or Comment, with what you think (needed to ask for changes or to comment), for a phone signed in to change things. An approval from the phone is not sealed, as it holds no key of yours: where a repository needs sealed approvals it does not count yet. |
|
| 5 | 6 | - **A proposal's files and their diffs (#4):** a proposal's Changes section opens Files changed: each file it changes, folding open to its diff (old and new line numbers, added and removed lines tinted), as its page on the hub shows them, from the hub's `GET …/proposals/N/diff` (OxygenHub 0.9.10). The change screen and this one share the same diff list. |
|
| 6 | 7 | - **Clone a repository from the phone (#3):** the repository screen's Clone action shows its `oxy clone` command, selectable, with a button that copies it. |
|
| 7 | 8 | ||
| … | 19 unchanged lines |
modified app/src/main/java/io/oxygenhub/android/api/HubClient.kt +10 −0
| … | 128 unchanged lines |
||
| 129 | 129 | ||
| 130 | 130 | suspend fun change(owner: String, name: String, hash: String): ChangeDetail |
|
| 131 | 131 | ||
| 132 | + | /** Review a proposal (#4): [verdict] is `approve`, `changes` or `comment`, with [body] said. */ |
|
| 133 | + | suspend fun review(owner: String, name: String, number: Long, verdict: String, body: String) |
|
| 134 | + | ||
| 132 | 135 | /** The files a proposal changes, each with its diff, as the proposal page shows them (#4). */ |
|
| 133 | 136 | suspend fun proposalDiff(owner: String, name: String, number: Long): List<FileChange> |
|
| 134 | 137 | ||
| … | 125 unchanged lines |
||
| 260 | 263 | }, |
|
| 261 | 264 | History.serializer(), |
|
| 262 | 265 | ) |
|
| 266 | + | ||
| 267 | + | override suspend fun review(owner: String, name: String, number: Long, verdict: String, body: String) { |
|
| 268 | + | sendJson("POST", url("repos", owner, name, "proposals", number.toString(), "reviews"), buildJsonObject { |
|
| 269 | + | put("verdict", verdict) |
|
| 270 | + | put("body", body) |
|
| 271 | + | }, ReviewPosted.serializer()) |
|
| 272 | + | } |
|
| 263 | 273 | ||
| 264 | 274 | override suspend fun proposalDiff(owner: String, name: String, number: Long): List<FileChange> = |
|
| 265 | 275 | get(url("repos", owner, name, "proposals", number.toString(), "diff"), ListSerializer(FileChange.serializer())) |
|
| … | 138 unchanged lines |
modified app/src/main/java/io/oxygenhub/android/api/Models.kt +4 −0
| … | 450 unchanged lines |
||
| 451 | 451 | /** What `PUT /user/push` answers (#16): whether the hub sends alerts at all. */ |
|
| 452 | 452 | @Serializable |
|
| 453 | 453 | data class PushRegistered(val alerts: Boolean = false) |
|
| 454 | + | ||
| 455 | + | /** What `POST …/reviews` answers; the app only needs that it was taken. */ |
|
| 456 | + | @Serializable |
|
| 457 | + | data class ReviewPosted(val verdict: String = "") |
modified app/src/main/java/io/oxygenhub/android/ui/proposals/ProposalScreen.kt +31 −3
| 1 | 1 | package io.oxygenhub.android.ui.proposals |
|
| 2 | 2 | ||
| 3 | + | import androidx.compose.material3.OutlinedTextField |
|
| 3 | 4 | import androidx.compose.foundation.clickable |
|
| 4 | 5 | import androidx.compose.foundation.layout.Arrangement |
|
| 5 | 6 | import androidx.compose.foundation.layout.Box |
|
| … | 95 unchanged lines |
||
| 101 | 102 | Header(page.proposal) |
|
| 102 | 103 | Landing(page.proposal, action, canWrite, vm::land, vm::setAutoLand, onSignInAgain, vm::dismiss) |
|
| 103 | 104 | Checks(page.checks) { if (it.url.isNotBlank()) uri.openUri(it.url) } |
|
| 104 | - | Reviews(page.proposal) |
|
| 105 | + | Reviews(page.proposal, canReview = canWrite && page.proposal.state == "open", busy = action == LandAction.Working, onReview = vm::review) |
|
| 105 | 106 | Changes(page.proposal, onOpenFiles) |
|
| 106 | 107 | if (page.proposal.body.isNotBlank()) { |
|
| 107 | 108 | Section("Description") { |
|
| … | 158 unchanged lines |
||
| 266 | 267 | } |
|
| 267 | 268 | ||
| 268 | 269 | @Composable |
|
| 269 | - | private fun Reviews(p: Proposal) { |
|
| 270 | - | if (p.approvals.isEmpty() && p.requestedReviewers.isEmpty()) return |
|
| 270 | + | private fun Reviews(p: Proposal, canReview: Boolean, busy: Boolean, onReview: (verdict: String, body: String) -> Unit) { |
|
| 271 | + | if (p.approvals.isEmpty() && p.requestedReviewers.isEmpty() && !canReview) return |
|
| 271 | 272 | Section("Reviews") { |
|
| 272 | 273 | Column(Modifier.padding(vertical = 4.dp)) { |
|
| 274 | + | if (canReview) ReviewBox(busy, onReview) |
|
| 273 | 275 | p.approvals.forEach { a -> |
|
| 274 | 276 | ListItem( |
|
| 275 | 277 | leadingContent = { Icon(Icons.Outlined.CheckCircle, null, tint = LocalStatusColors.current.ok) }, |
|
| … | 13 unchanged lines |
||
| 289 | 291 | } |
|
| 290 | 292 | } |
|
| 291 | 293 | ||
| 294 | + | /** |
|
| 295 | + | * Review it from the phone (#4): approve, ask for changes, or comment. An |
|
| 296 | + | * approval from here is not sealed (the phone holds no key of yours), so |
|
| 297 | + | * where the repository needs sealed approvals it does not count yet. |
|
| 298 | + | */ |
|
| 299 | + | @Composable |
|
| 300 | + | private fun ReviewBox(busy: Boolean, onReview: (verdict: String, body: String) -> Unit) { |
|
| 301 | + | var text by rememberSaveable { mutableStateOf("") } |
|
| 302 | + | Column(Modifier.padding(horizontal = 16.dp, vertical = 8.dp), verticalArrangement = Arrangement.spacedBy(8.dp)) { |
|
| 303 | + | OutlinedTextField( |
|
| 304 | + | value = text, |
|
| 305 | + | onValueChange = { text = it }, |
|
| 306 | + | label = { Text("Review") }, |
|
| 307 | + | placeholder = { Text("What you think; needed to ask for changes") }, |
|
| 308 | + | modifier = Modifier.fillMaxWidth(), |
|
| 309 | + | minLines = 2, |
|
| 310 | + | enabled = !busy, |
|
| 311 | + | ) |
|
| 312 | + | Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { |
|
| 313 | + | Button(onClick = { onReview("approve", text); text = "" }, enabled = !busy) { Text("Approve") } |
|
| 314 | + | OutlinedButton(onClick = { onReview("changes", text); text = "" }, enabled = !busy && text.isNotBlank()) { Text("Request changes") } |
|
| 315 | + | TextButton(onClick = { onReview("comment", text); text = "" }, enabled = !busy && text.isNotBlank()) { Text("Comment") } |
|
| 316 | + | } |
|
| 317 | + | } |
|
| 318 | + | } |
|
| 319 | + | ||
| 292 | 320 | @Composable |
|
| 293 | 321 | private fun Changes(p: Proposal, onOpenFiles: () -> Unit) { |
|
| 294 | 322 | if (p.changes.isEmpty()) return |
|
| … | 28 unchanged lines |
modified app/src/main/java/io/oxygenhub/android/ui/proposals/ProposalViewModel.kt +10 −0
| … | 83 unchanged lines |
||
| 84 | 84 | } |
|
| 85 | 85 | } |
|
| 86 | 86 | ||
| 87 | + | /** Review it (#4): approve, ask for changes, or comment, with [body]. */ |
|
| 88 | + | fun review(verdict: String, body: String) = act { p -> |
|
| 89 | + | api.review(owner, name, number, verdict, body.trim()) |
|
| 90 | + | p to when (verdict) { |
|
| 91 | + | "approve" -> "Approved #$number." |
|
| 92 | + | "changes" -> "Asked for changes on #$number." |
|
| 93 | + | else -> "Commented on #$number." |
|
| 94 | + | } |
|
| 95 | + | } |
|
| 96 | + | ||
| 87 | 97 | fun dismiss() { |
|
| 88 | 98 | _action.value = LandAction.Idle |
|
| 89 | 99 | } |
|
| … | 32 unchanged lines |
modified app/src/test/java/io/oxygenhub/android/FakeHubApi.kt +3 −0
| … | 51 unchanged lines |
||
| 52 | 52 | issues.invoke(state).also { calls += "issues $owner/$name ${state.query}" } |
|
| 53 | 53 | override suspend fun inbox(limit: Int): Inbox = inbox.invoke().also { calls += "inbox" } |
|
| 54 | 54 | override suspend fun registerPush(fcmToken: String, publicKey: String): Boolean = true.also { calls += "push" } |
|
| 55 | + | override suspend fun review(owner: String, name: String, number: Long, verdict: String, body: String) { |
|
| 56 | + | calls += "review $verdict $body" |
|
| 57 | + | } |
|
| 55 | 58 | var proposalDiff: () -> List<FileChange> = { emptyList() } |
|
| 56 | 59 | override suspend fun proposalDiff(owner: String, name: String, number: Long): List<FileChange> = |
|
| 57 | 60 | proposalDiff.invoke().also { calls += "proposalDiff" } |
|
| … | 117 unchanged lines |
modified app/src/test/java/io/oxygenhub/android/ui/ProposalViewModelTest.kt +11 −0
| … | 60 unchanged lines |
||
| 61 | 61 | } |
|
| 62 | 62 | ||
| 63 | 63 | @Test |
|
| 64 | + | fun aReviewIsSentAndSaysSo() { |
|
| 65 | + | val vm = ProposalViewModel(api, "andymccall", "oxygenhub", 300) |
|
| 66 | + | vm.review("changes", " Needs a test ") |
|
| 67 | + | assertTrue("review changes Needs a test" in api.calls) |
|
| 68 | + | assertEquals(LandAction.Done("Asked for changes on #300."), vm.action.value) |
|
| 69 | + | vm.dismiss() |
|
| 70 | + | vm.review("approve", "") |
|
| 71 | + | assertEquals(LandAction.Done("Approved #300."), vm.action.value) |
|
| 72 | + | } |
|
| 73 | + | ||
| 74 | + | @Test |
|
| 64 | 75 | fun aRefusalShowsTheHubsReason() { |
|
| 65 | 76 | api.land = { throw HubException.Http(409, "required checks not passed on the projected state: test") } |
|
| 66 | 77 | val vm = ProposalViewModel(api, "andymccall", "oxygenhub", 300) |
|
| … | 38 unchanged lines |