chadtmiller.com/knotless
git that lives in your Atmosphere account
1
10
A pull's reviewers and labels sit in a facts line under its title
crates/knotless-serve/src/web/opening.rs+1−1
| @@ -141,7 +141,7 @@ pub struct Named { | ||
/// An account asked to review a pull, by DID. | ||
#[derive(Deserialize)] | ||
pub struct Asked { | ||
did: String, | ||
pub did: String, | ||
} | ||
| ||
#[derive(Deserialize)] | ||
crates/knotless-serve/src/web/threads.rs+8−1
| @@ -6,6 +6,7 @@ | ||
//! `label` and `unlabel`, `hide` and `unhide`, and `report`, and indexes | ||
//! what it wrote at once, so the page after shows it. | ||
| ||
use super::opening::Asked; | ||
use super::repo::found; | ||
use super::stars::logged_in; | ||
use super::*; | ||
| @@ -280,6 +281,10 @@ pub struct EditInput { | ||
body: Option<String>, | ||
images: Option<Vec<ImageAttachment>>, | ||
draft: Option<bool>, | ||
#[serde(rename = "addReviewers", default)] | ||
add_reviewers: Vec<Asked>, | ||
#[serde(rename = "removeReviewers", default)] | ||
remove_reviewers: Vec<Asked>, | ||
} | ||
| ||
/// `dev.knotless.editItem`. | ||
| @@ -292,9 +297,11 @@ pub async fn edit_item( | ||
let (ctx, repo, url) = acting(&app, viewer, &headers, "edit", &input.repo).await?; | ||
let (title, body) = (input.title.as_deref(), input.body.as_deref()); | ||
let images = blobs(input.images); | ||
let dids = |asked: Vec<Asked>| asked.into_iter().map(|a| a.did).collect(); | ||
let asks = forge::PullAsks { | ||
draft: input.draft, | ||
..Default::default() | ||
add: dids(input.add_reviewers), | ||
remove: dids(input.remove_reviewers), | ||
}; | ||
let edited = forge::edit(&ctx, &url, &input.subject, title, body, &images, &asks); | ||
edited.await.map_err(refused)?; | ||
crates/knotless-serve/tests/gateway/opening.rs+18−1
| @@ -114,8 +114,25 @@ fn opening_pulls_and_issues_from_the_pages() { | ||
asking["reviewers"] = json!([{"did": owner.account.did}]); | ||
let (status, _, opened) = write("createPull", me, asking); | ||
assert_eq!(status, 200, "{opened}"); | ||
let shown = thread(opened["uri"].as_str().unwrap()); | ||
let asked_pull = opened["uri"].as_str().unwrap().to_string(); | ||
let shown = thread(&asked_pull); | ||
assert_eq!(shown["item"]["reviewers"][0]["did"], owner.account.did); | ||
| ||
// And asked for, or no longer, once it is open, as its author; an | ||
// issue asks no one. | ||
let them = json!([{"did": owner.account.did}]); | ||
let stop = json!({"repo": repo, "subject": asked_pull, "removeReviewers": them}); | ||
let (status, _, stopped) = write("editItem", me, stop); | ||
assert_eq!(status, 200, "{stopped}"); | ||
// None asked: the view leaves the list out. | ||
let none = thread(&asked_pull)["item"]["reviewers"].clone(); | ||
assert!(none.as_array().is_none_or(Vec::is_empty), "{none}"); | ||
let again = json!({"repo": repo, "subject": asked_pull, "addReviewers": them}); | ||
assert_eq!(write("editItem", me, again).0, 200); | ||
let shown = thread(&asked_pull); | ||
assert_eq!(shown["item"]["reviewers"][0]["did"], owner.account.did); | ||
let of_issue = json!({"repo": repo, "subject": issue, "addReviewers": them}); | ||
assert_eq!(write("editItem", me, of_issue).0, 400); | ||
| ||
// Labels put on as it opens, by those who may put them on: the | ||
// owner, whose pages offer them, and not the fan, who is refused | ||
docs/design/collaboration.md+8−4
| @@ -83,8 +83,9 @@ a verdict this version does not know. | ||
| ||
An approval is of the head it recorded. Once the pull moves, readers show it | ||
as an approval of an earlier version. A request for changes stands until its | ||
reviewer reviews again. The pages show where each reviewer stands in the | ||
pull's merge box, and `knotless show` lists them as `review:` lines. | ||
reviewer reviews again. The pages show where each reviewer stands under | ||
the pull's title, as a mark on each face that opens to their verdicts, and | ||
again in its merge box; `knotless show` lists them as `review:` lines. | ||
| ||
### Review agents | ||
| ||
| @@ -175,14 +176,17 @@ A pull can also ask accounts to review it (`reviewers`, each by DID, with | ||
when it was asked). Each one asked hears of it as a `review-requested` | ||
notification once the pull is not a draft, as of when it was asked or, | ||
for a draft, when it was marked ready. The pages show those not yet | ||
reviewed in the merge box, beside the reviews. MCP's `show_item` says of | ||
reviewed under the pull's title and in the merge box, beside the reviews. MCP's `show_item` says of | ||
each asked whether the request is `pending`: while the pull is open and | ||
they have not reviewed it; once it is merged or closed, no longer. | ||
| ||
Both are fields of the pull record, so only its author sets them: when | ||
opening it (`knotless pull --draft --reviewer <handle>`) and with `knotless | ||
edit` (`--draft`, `--ready`, `--reviewer`, `--no-reviewer`). On the pages, | ||
the author opens one as a draft with Open as draft, marks it | ||
the author asks the owner and collaborators to review it as it opens, and | ||
once it is open from its reviewers under the title (or Add, when it asks | ||
no one yet), where they also stop asking one who has not reviewed it. The | ||
author opens one as a draft with Open as draft, marks it | ||
ready with the merge box's Ready for review, and makes it a draft again | ||
with Convert to draft in the merge box's More menu. A maintainer | ||
cannot ask someone to review another person's pull: that would need a | ||
docs/reference/api.md+1−1
| @@ -659,7 +659,7 @@ from this site` unless its `Origin` is the site's own. | ||
| `POST /xrpc/dev.knotless.createIssue` | `{repo, title, body?, images?, labels?}`: opens an issue as the account logged in, answering `{uri}`, with `labels` (each `{name}`) put on as `labelItem` would; 400, with the template, for a body that follows none of the repository's, and, before anything is written, for a label the account may not put on | | ||
| `POST /xrpc/dev.knotless.createPull` | `{repo, base, head, branch, title, body?, images?, draft?, labels?, reviewers?}`: opens a pull to merge `branch` of `head`, this repository or a fork, into `base`, as a draft with `draft`, answering `{uri}`, with `labels` put on as `createIssue` does and `reviewers` (each `{did}`) asked to review it; 400 for a branch on neither, a body that follows none of the template, or a label the account may not put on | | ||
| `POST /xrpc/dev.knotless.createComment` | `{repo, subject, body, verdict?, images?, lines?}`: comments on a pull or issue as the account logged in, or with `verdict` (`approve`, `changes`) reviews a pull; `lines`, each `{path, side?, line, startLine?, body}`, comment on the pull's diff, as the review's when it has a body or verdict. A comment as `subject` answers it in its thread. Answers the first it wrote's `{uri}`; 400 for a comment that says nothing | | ||
| `POST /xrpc/dev.knotless.editItem` | `{repo, subject, title?, body?, images?, draft?}`: changes the account's own pull, issue or comment, answering `{uri}`; `draft`, for a pull alone, makes it a draft or with `false` marks it ready. `images`, each `{image, width?, height?}` with a blob and size `uploadImage` gave, are attached to what the body links to ([collaboration.md](../design/collaboration.md#pictures)). It, `createIssue`, `createPull` and `createComment` write each `@handle` of the title and body that resolves as a facet naming its DID ([collaboration.md](../design/collaboration.md#mentions)) | | ||
| `POST /xrpc/dev.knotless.editItem` | `{repo, subject, title?, body?, images?, draft?, addReviewers?, removeReviewers?}`: changes the account's own pull, issue or comment, answering `{uri}`; `draft`, for a pull alone, makes it a draft or with `false` marks it ready, and `addReviewers` and `removeReviewers`, each `[{did}]`, for a pull alone, ask accounts to review it or stop asking them. `images`, each `{image, width?, height?}` with a blob and size `uploadImage` gave, are attached to what the body links to ([collaboration.md](../design/collaboration.md#pictures)). It, `createIssue`, `createPull` and `createComment` write each `@handle` of the title and body that resolves as a facet naming its DID ([collaboration.md](../design/collaboration.md#mentions)) | | ||
| `POST /xrpc/dev.knotless.deleteComment` | `{repo, subject}`: deletes the account's own comment or review, answering `{uri}` | | ||
| `POST /xrpc/dev.knotless.setItemState` | `{repo, subject, state}`: closes (`closed`) or reopens (`open`) a pull or issue, or resolves (`resolved`) or opens (`open`) the thread a comment on a pull's lines starts, answering the state record's `{uri}` | | ||
| `POST /xrpc/dev.knotless.labelItem` | `{repo, subject, label, removed?}`: puts a label on, or takes it off, answering the `{labels}` it carries now | | ||
harness/e2e/flows.spec.mjs+14−2
| @@ -200,6 +200,15 @@ test('opening a pull from a fork', async () => { | ||
const made = await fork() | ||
expect(made?.value.reviewers?.map((r) => r.did)).toContain(who.alice.did) | ||
expect(made?.value.draft).toBe(true) | ||
// Asked no longer, then again, once it is open, from under its title. | ||
await page.locator('.reviewers-fact summary').click() | ||
await page.getByRole('button', { name: 'Stop asking alice.test' }).click() | ||
await expect(page.locator('.reviewers-fact')).toHaveCount(0) | ||
await expect.poll(async () => (await fork())?.value.reviewers ?? []).toHaveLength(0) | ||
await page.locator('.add-menu summary').click() | ||
await page.locator('.add-menu').getByRole('button', { name: 'alice.test' }).click() | ||
await expect(page.locator('.reviewers-fact')).toContainText('1 reviewer') | ||
await expect.poll(async () => (await fork())?.value.reviewers?.map((r) => r.did)).toContain(who.alice.did) | ||
}) | ||
| ||
/** bob's pull from his fork, as his PDS holds it. */ | ||
| @@ -386,8 +395,11 @@ test('logging in to a second account and labelling as the owner', async () => { | ||
await page.getByRole('button', { name: 'Add account' }).click() | ||
}) | ||
await expect.poll(() => loggedInAs(page)).toBe('alice.test') | ||
await page.locator('.label-menu summary').click() | ||
await page.locator('.label-menu').getByRole('button', { name: 'kind/bug' }).click() | ||
// With none yet, a label is put on from Add, then shows under the title. | ||
await page.locator('.add-menu summary').click() | ||
await page.locator('.add-menu').getByRole('button', { name: 'kind/bug' }).click() | ||
await expect(page.locator('.item-facts .labels')).toContainText('kind/bug') | ||
await page.locator('.item-facts .label-menu summary').click() | ||
await expect(page.locator('.label-menu').getByRole('button', { name: 'kind/bug' })).toHaveAttribute('aria-pressed', 'true') | ||
await anew(page, (expect) => expect(page.locator('.labels').getByText('kind/bug')).toBeVisible()) | ||
}) | ||
harness/e2e/private.spec.mjs+3−3
| @@ -174,9 +174,9 @@ test('hearing of it as the owner', async () => { | ||
| ||
test('labelling, closing and hiding as the owner', async () => { | ||
await go(page, asked) | ||
await page.locator('.label-menu summary').click() | ||
await page.locator('.label-menu').getByRole('button', { name: 'kind/secret' }).click() | ||
await expect(page.locator('.label-menu').getByRole('button', { name: 'kind/secret' })).toHaveAttribute('aria-pressed', 'true') | ||
await page.locator('.add-menu summary').click() | ||
await page.locator('.add-menu').getByRole('button', { name: 'kind/secret' }).click() | ||
await expect(page.locator('.item-facts .labels')).toContainText('kind/secret') | ||
await anew(page, (expect) => expect(page.locator('.labels').getByText('kind/secret')).toBeVisible()) | ||
| ||
await page.getByRole('button', { name: 'Close issue' }).click() | ||
lexicons/dev/knotless/editItem.json+11−1
| @@ -17,7 +17,9 @@ | ||
"title": { "type": "string", "maxLength": 3000, "maxGraphemes": 300, "description": "A pull's or an issue's new title." }, | ||
"body": { "type": "string", "maxLength": 20000, "description": "Its new body, in Markdown." }, | ||
"images": { "type": "array", "maxLength": 32, "items": { "type": "ref", "ref": "dev.knotless.defs#image" }, "description": "Pictures uploaded with dev.knotless.uploadImage for the new body. Of these and those it had, the new body's links decide which stay attached." }, | ||
"draft": { "type": "boolean", "description": "For a pull, make it a draft, or with false mark it ready to merge. Left as it is, when absent; any other item refuses it." } | ||
"draft": { "type": "boolean", "description": "For a pull, make it a draft, or with false mark it ready to merge. Left as it is, when absent; any other item refuses it." }, | ||
"addReviewers": { "type": "array", "maxLength": 32, "items": { "type": "ref", "ref": "#reviewer" }, "description": "For a pull, accounts to ask to review it, besides those it asks already. Any other item refuses it." }, | ||
"removeReviewers": { "type": "array", "maxLength": 32, "items": { "type": "ref", "ref": "#reviewer" }, "description": "For a pull, accounts to stop asking to review it. Any other item refuses it." } | ||
} | ||
} | ||
}, | ||
| @@ -35,6 +37,14 @@ | ||
{ "name": "NotFound", "description": "No such repository." }, | ||
{ "name": "AuthenticationRequired", "description": "No account is logged in to the pages, or its session has ended." } | ||
] | ||
}, | ||
"reviewer": { | ||
"type": "object", | ||
"description": "An account asked, or no longer asked, to review a pull.", | ||
"required": ["did"], | ||
"properties": { | ||
"did": { "type": "string", "format": "did", "description": "The account, by DID." } | ||
} | ||
} | ||
} | ||
} | ||
web/src/lib/molecules/AddMenu.sveltenew+42−0
| @@ -0,0 +1,42 @@ | ||
<script lang="ts"> | ||
import type { ActorView } from '#lib/lexicons/dev/knotless/defs.ts'; | ||
import type * as repo from '#lib/lexicons/dev/knotless/repo.ts'; | ||
import Avatar from '#lib/atoms/Avatar.svelte'; | ||
import Icon from '#lib/atoms/Icon.svelte'; | ||
/** | ||
* One menu for the facts a pull or an issue has none of yet, so its | ||
* facts line stays short however many kinds there are: the `people` it | ||
* may `ask` to review it, and the `labels` it may `label` it with. | ||
* Once one is chosen, that fact shows, and is changed from there. | ||
*/ | ||
let { people = [], ask, labels = [], label }: { people?: ActorView[]; ask?: (did: ActorView['did'], removed: boolean) => Promise<unknown>; labels?: repo.Label[]; label?: (name: string, removed: boolean) => Promise<unknown> } = $props(); | ||
const asking = $derived(!!ask && people.length > 0); | ||
const labelling = $derived(!!label && labels.length > 0); | ||
let busy = $state(''); | ||
async function doing(key: string, work: () => Promise<unknown>) { | ||
busy = key; | ||
await work(); | ||
busy = ''; | ||
} | ||
const hex = (color?: string) => (/^#[0-9a-fA-F]{6}$/.test(color ?? '') ? color : undefined); | ||
</script> | ||
| ||
{#if asking || labelling} | ||
<details class="popover filter-menu add-menu" data-held={busy || undefined}> | ||
<summary><Icon name="plus" />Add</summary> | ||
<div class="menu"> | ||
{#if asking} | ||
<div class="menu-head">Ask to review</div> | ||
{#each people as p (p.did)} | ||
<button class="menu-item" type="button" disabled={!!busy} onclick={() => doing(p.did, () => ask!(p.did, false))}>{#if busy === p.did}<span class="spinner"></span>{:else}<Avatar name={p.handle} did={p.did} src={p.avatar} />{/if}<span>{p.handle}</span></button> | ||
{/each} | ||
{/if} | ||
{#if labelling} | ||
<div class="menu-head" class:sub={asking}>Label</div> | ||
{#each labels as l (l.name)} | ||
<button class="menu-item" type="button" disabled={!!busy} onclick={() => doing(l.name, () => label!(l.name, false))}>{#if busy === l.name}<span class="spinner"></span>{:else}<span class="dot" style:--label={hex(l.color)}></span>{/if}<span>{l.name}</span></button> | ||
{/each} | ||
{/if} | ||
</div> | ||
</details> | ||
{/if} | ||
web/src/lib/molecules/LabelPicker.svelte+6−3
| @@ -1,14 +1,17 @@ | ||
<script lang="ts"> | ||
import type * as repo from '#lib/lexicons/dev/knotless/repo.ts'; | ||
import type { Snippet } from 'svelte'; | ||
import Icon from '#lib/atoms/Icon.svelte'; | ||
/** | ||
* A menu of the `labels` the one looking may put on a pull or an issue, | ||
* those it carries (`on`, by name) ticked: choosing one tells `toggle` | ||
* to put it on, or take it off, its tick turning while it does. With | ||
* `gear`, it opens from a gear beside a heading that names it, as a | ||
* sidebar's does, rather than from its name. | ||
* sidebar's does, rather than from its name; with `summary`, from what | ||
* that draws, as a fact of a pull or issue opens from the labels it | ||
* carries. | ||
*/ | ||
let { labels, on, toggle, gear = false }: { labels: repo.Label[]; on: string[]; toggle: (name: string, removed: boolean) => Promise<unknown>; gear?: boolean } = $props(); | ||
let { labels, on, toggle, gear = false, summary }: { labels: repo.Label[]; on: string[]; toggle: (name: string, removed: boolean) => Promise<unknown>; gear?: boolean; summary?: Snippet } = $props(); | ||
let busy = $state(''); | ||
async function pick(name: string, removed: boolean) { | ||
busy = name; | ||
| @@ -19,7 +22,7 @@ | ||
</script> | ||
| ||
<details class="popover filter-menu label-menu" data-held={busy || undefined}> | ||
{#if gear}<summary aria-label="Choose labels" title="Choose labels"><Icon name="gear" /></summary>{:else}<summary>Labels<Icon name="caret" /></summary>{/if} | ||
{#if summary}<summary title="Choose labels">{@render summary()}</summary>{:else if gear}<summary aria-label="Choose labels" title="Choose labels"><Icon name="gear" /></summary>{:else}<summary>Labels<Icon name="caret" /></summary>{/if} | ||
<div class="menu"> | ||
<div class="menu-head">Apply labels</div> | ||
{#each labels as l (l.name)} | ||