Public: anyone can read this; the crew can change it.
The /people page speaks the home page's language, with looks before and after #9702
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/9646-the-people-page-speaks-the-home-page-s-l → main 3 commits fork 3 threads · 3 unresolved
2 files
changed +107
−19
src/web/pages/people.ts +41 −19
| @@ -17,15 +17,32 @@ import { notFoundPage, settingsLayout, signInFirst } from "../chrome.ts"; | ||
| 17 | 17 | import { field, readForm } from "../form.ts"; |
| 18 | 18 | import { html } from "../html.ts"; |
| 19 | 19 | import { flashTo, page } from "../layout.ts"; |
| 20 | import { badge, btn, card, empty, userLink } from "../ui.ts"; | |
| 20 | import { badge, btn, empty, userLink } from "../ui.ts"; | |
| 21 | 21 | import { accountSections } from "./origins.ts"; |
| 22 | 22 | |
| 23 | /** | |
| 24 | * The apps each username made, by repo name. An admin sees every app, private | |
| 25 | * ones included: this page is admin-only and an app is one row per repo, so | |
| 26 | * no environment is counted twice. | |
| 27 | */ | |
| 28 | export function appsMade(ctx: Ctx): Map<string, string[]> { | |
| 29 | const rows = ctx.app.db | |
| 30 | .query<{ username: string; name: string }, []>( | |
| 31 | `SELECT u.username, r.name FROM app a JOIN repo r ON r.id = a.repo_id JOIN user u ON u.id = r.owner_id ORDER BY r.name COLLATE NOCASE`, | |
| 32 | ) | |
| 33 | .all(); | |
| 34 | const made = new Map<string, string[]>(); | |
| 35 | for (const r of rows) made.set(r.username, [...(made.get(r.username) ?? []), r.name]); | |
| 36 | return made; | |
| 37 | } | |
| 38 | ||
| 23 | 39 | // GET /settings/people |
| 24 | 40 | export function peoplePage(ctx: Ctx): Response { |
| 25 | 41 | if (!ctx.auth.user) return signInFirst(ctx); |
| 26 | 42 | if (!ctx.auth.user.isAdmin) return notFoundPage(ctx); |
| 27 | 43 | notePhase(ctx.request, "session", takeSessionTime(ctx.request)); |
| 28 | 44 | const rows = timePhase(ctx.request, "people", () => listPeople(ctx.app.db)); |
| 45 | const made = appsMade(ctx); | |
| 29 | 46 | const rendering = performance.now(); |
| 30 | 47 | const line = (p: Person) => { |
| 31 | 48 | const role = p.role; |
| @@ -44,10 +61,15 @@ export function peoplePage(ctx: Ctx): Response { | ||
| 44 | 61 | <input type="hidden" name="role" value="guest" /> |
| 45 | 62 | ${btn("Make guest", { type: "submit", size: "sm" })} |
outdated
Principle 2 and the issue's 'a line of what they made': the muted 'N apps' count is only a number. Show what they made, for example the app names as links, or at least a count that links to their apps. Also, this behaviour is untested: add a test that fails without it (0, 1, N apps; 'no allowance'). |
||
| 46 | 63 | </form>`; |
| 64 | const apps = made.get(p.username) ?? []; | |
| 65 | const count = apps.length; | |
| 66 | const shown = apps.slice(0, 3).map((n) => html`<a href="/${p.username}/${n}">${n}</a>`); | |
| 67 | const standing = p.usdPerMonth === null ? "no allowance" : `$${p.usdPerMonth.toFixed(2)} a month`; | |
| 47 | 68 | return html`<tr> |
| 48 | <td>${userLink(p.username)}</td> | |
| 49 | <td>${badge(role, { tone: role === "guest" ? "default" : "success" })}</td> | |
| 50 | <td>${p.usdPerMonth === null ? "none" : `$${p.usdPerMonth.toFixed(2)} a month`}</td> | |
| 69 | <td>${userLink(p.username)}<div class="small muted">${count === 0 ? "No apps yet" : count === 1 ? "1 app" : `${count} apps`}${ | |
| 70 | count === 0 ? "" : html`: ${shown.flatMap((a, i) => (i === 0 ? [a] : [", ", a]))}${count > 3 ? ` and ${count - 3} more` : ""}` | |
| 71 | }</div></td> | |
| 72 | <td>${badge(role, { tone: role === "guest" ? "default" : "success" })} <span class="small muted">${standing}</span></td> | |
| 51 | 73 | <td>${act}</td> |
outdated
Principle 1: the heading 'Guests, members and admins' and then a paragraph of allowance and role-limit explanation under it. One confident sentence should say what the page is for. Cut the paragraph down to one plain sentence, in the words of docs/WRITING.md, and let the role limits live where they are acted on. |
||
| 52 | 74 | </tr>`; |
| 53 | 75 | }; |
| @@ -57,21 +79,21 @@ export function peoplePage(ctx: Ctx): Response { | ||
| 57 | 79 | body: html`<div class="page-head"><h1>People</h1></div> |
| 58 | 80 | ${settingsLayout( |
| 59 | 81 | accountSections("people", { admin: true }), |
| 60 | card( | |
| 61 | rows.length === 0 | |
| 62 | ? empty({ icon: "users", title: "Nobody has joined yet." }) | |
| 63 | : html`<div class="table-scroll" data-people> | |
| 64 | <table class="table" id="people"> | |
| 65 | <thead><tr><th>Account</th><th>Role</th><th>Allowance</th><th></th></tr></thead> | |
| 66 | <tbody>${rows.map(line)}</tbody> | |
| 67 | </table> | |
| 68 | </div>`, | |
| 69 | { | |
| 70 | head: html`<h2>Guests, members and admins</h2> | |
| 71 | <p class="muted">A member may be given a starter model allowance (up to | |
| 72 | $${GRANT_MAX_USD} a month). ${roleLimits()}</p>`, | |
| 73 | }, | |
| 74 | ), | |
| 82 | html`<section class="settings-block"> | |
| 83 | <h2>Guests, members and admins</h2> | |
| 84 | <p class="muted">Make a guest a member, with an allowance if you like.</p> | |
| 85 | ${ | |
| 86 | rows.length === 0 | |
| 87 | ? empty({ icon: "users", title: "Nobody has joined yet." }) | |
| 88 | : html`<div class="table-scroll" data-people> | |
| 89 | <table class="table" id="people"> | |
| 90 | <thead><tr><th>Person</th><th>Standing</th><th></th></tr></thead> | |
| 91 | <tbody>${rows.map(line)}</tbody> | |
| 92 | </table> | |
| 93 | </div> | |
| 94 | <p class="small muted">Allowances go up to $${GRANT_MAX_USD} a month. ${roleLimits()}</p>` | |
| 95 | } | |
| 96 | </section>`, | |
| 75 | 97 | "Account settings", |
| 76 | 98 | )}`, |
| 77 | 99 | }); |
test/people-apps-made.test.ts added +66
| @@ -0,0 +1,66 @@ | ||
| 1 | /** `/settings/people` says what each person made: 0, 1 and many apps as links, and "no allowance" for a member without one. */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtempSync, rmSync } from "node:fs"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { createUser } from "../src/core/users.ts"; | |
| 8 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 9 | import { appsMade } from "../src/web/pages/people.ts"; | |
| 10 | import { quietLogs } from "./helpers/globals.ts"; | |
| 11 | ||
| 12 | quietLogs(); | |
| 13 | ||
| 14 | let root = ""; | |
| 15 | let app: App; | |
| 16 | let servers: Servers; | |
| 17 | let cookie = ""; | |
| 18 | ||
| 19 | const seed = async (username: string, admin = false) => { | |
| 20 | const u = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@forge.local`, password: "a-good-passphrase", adminIfFirst: admin }); | |
| 21 | if (!u.ok) throw new Error(u.error); | |
| 22 | return u.value; | |
| 23 | }; | |
| 24 | const makeApp = (ownerId: string, name: string, visibility: "public" | "private") => { | |
| 25 | const repo = `r-${name}`; | |
| 26 | app.db.query("INSERT INTO repo (id, owner_id, name, visibility) VALUES (?1, ?2, ?3, ?4)").run(repo, ownerId, name, visibility); | |
| 27 | app.db.query("INSERT INTO app (id, repo_id, slug, access) VALUES (?1, ?2, ?3, ?4)").run(`a-${name}`, repo, name, visibility); | |
| 28 | }; | |
| 29 | ||
| 30 | beforeAll(async () => { | |
| 31 | root = mkdtempSync(join(tmpdir(), "forge-people-apps-")); | |
| 32 | app = createApp({ dataDir: root, port: 0 }); | |
| 33 | await seed("alice", true); | |
| 34 | const one = await seed("one-app"); | |
| 35 | const two = await seed("two-apps"); | |
| 36 | await seed("no-apps"); | |
| 37 | makeApp(one.id, "solo", "public"); | |
| 38 | makeApp(two.id, "zeta", "private"); | |
| 39 | makeApp(two.id, "alpha", "public"); | |
| 40 | servers = await serve(app); | |
| 41 | const signin = await fetch(`${servers.url}/signin`, { method: "POST", headers: { origin: servers.url }, body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }), redirect: "manual" }); | |
| 42 | cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!; | |
| 43 | }); | |
| 44 | afterAll(() => { | |
| 45 | servers.stop(); | |
| 46 | app.close(); | |
| 47 | rmSync(root, { recursive: true, force: true }); | |
| 48 | }); | |
| 49 | ||
| 50 | test("appsMade lists every owner's apps by name, private ones too", () => { | |
| 51 | const made = appsMade({ app } as never); | |
| 52 | expect(made.get("one-app")).toEqual(["solo"]); | |
| 53 | expect(made.get("two-apps")).toEqual(["alpha", "zeta"]); | |
| 54 | expect(made.has("no-apps")).toBe(false); | |
| 55 | }); | |
| 56 | ||
| 57 | test("each person's row says how many apps and links them; no allowance is said plainly", async () => { | |
| 58 | const text = await (await fetch(`${servers.url}/settings/people`, { headers: { cookie } })).text(); | |
| 59 | const row = (name: string) => text.split("<tr>").find((r) => r.includes(`<a href="/${name}"`))!.replace(/\s+/g, " "); | |
| 60 | expect(row("no-apps")).toContain("No apps yet"); | |
| 61 | expect(row("one-app")).toContain("1 app"); | |
| 62 | expect(row("one-app")).toContain('<a href="/one-app/solo">solo</a>'); | |
| 63 | expect(row("two-apps")).toContain("2 apps"); | |
| 64 | expect(row("two-apps")).toContain('<a href="/two-apps/alpha">alpha</a>, <a href="/two-apps/zeta">zeta</a>'); | |
| 65 | expect(row("two-apps")).toContain("no allowance"); | |
| 66 | }); |
Add a test for appsMade. It counts every app joined to a repo the user owns, so say whether private repos or apps of other environments should appear in an admin's count. If they should not, filter them out here.