Public: anyone can read this; the crew can change it.
An admin reaches Settings › Features from the page of an app that declares a feature, and the empty state links the docs #9685
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9680-an-admin-reaches-settings-features-from → main 3 commits fork 1 thread · 1 unresolved
7 files
changed +108
−9
docs/features/absorb.md +1 −1
| @@ -86,7 +86,7 @@ This section is ready for the owner to share with Ben. | ||
| 86 | 86 | |
| 87 | 87 | ## Finding it, and what the switch does |
| 88 | 88 | |
| 89 | A site admin sees a "Features" link on the app strip of an absorbed app (an app not yet absorbed is reached from Settings → Features: finding one that merely declares a feature would cost a git read on every app page). The same list, switch and settings are in the terminal: the "Features" palette entry. Beside Absorb or Un-absorb, a line says what restarts and whose menu gains or loses the entry. A refused settings save re-draws the form with what was typed and marks the refused field. | |
| 89 | A site admin (a person, not an agent) sees a "Features" link on the app strip of any app whose default branch declares a feature, absorbed or not, and of any absorbed app; nobody else does. The strip is on the app's code page, not its other tabs. The declaration is read from git once per commit and cached, so later page views cost no git read. The Settings → Features page lists the same apps, and its empty state links this guide. The same list, switch and settings are in the terminal: the "Features" palette entry. Beside Absorb or Un-absorb, a line says what restarts and whose menu gains or loses the entry. A refused settings save re-draws the form with what was typed and marks the refused field. | |
| 90 | 90 | |
| 91 | 91 | ## The menu entry is the admin's, not the deploy's |
| 92 | 92 |
src/actions/features.ts +14 −1
| @@ -21,7 +21,7 @@ import type { App } from "../app.ts"; | ||
| 21 | 21 | import { readFileAt } from "../apps/checkout.ts"; |
| 22 | 22 | import { checkSetting, featureEnv, parseFeature, settingEnvName, type FeatureSpec } from "../apps/feature.ts"; |
| 23 | 23 | import { appsRuntime } from "../apps/runtime.ts"; |
| 24 | import { getAppById, getAppBySlug, listApps, type AppRecord } from "../core/apps.ts"; | |
| 24 | import { getAppByRepo, getAppById, getAppBySlug, listApps, type AppRecord } from "../core/apps.ts"; | |
| 25 | 25 | import { |
| 26 | 26 | absorb, |
| 27 | 27 | keepDeclaration, |
| @@ -119,6 +119,19 @@ async function readDeclaration(dir: string, tip: string): Promise<Result<Feature | ||
| 119 | 119 | return parseFeature(parsed as Record<string, unknown>); |
| 120 | 120 | } |
| 121 | 121 | |
| 122 | /** | |
| 123 | * Whether the link to an app's feature shows on its page: the app is absorbed | |
| 124 | * or its default branch declares a feature, a site admin's only, and read through `declaredFeature`'s cache by | |
| 125 | * commit, so a page view costs a git read only the first time a tip is seen. | |
| 126 | */ | |
| 127 | export async function declaresFeature(app: App, actor: User | undefined, repo: Repo): Promise<boolean> { | |
| 128 | if (!siteAdmin(actor)) return false; | |
| 129 | const record = getAppByRepo(app.db, repo.id); | |
| 130 | if (record !== undefined && getAbsorbed(app.db, record.id) !== undefined) return true; | |
| 131 | const read = await declaredFeature(app, repo); | |
| 132 | return read.ok && read.value !== undefined; | |
| 133 | } | |
| 134 | ||
| 122 | 135 | function viewOf(record: AppRecord, repo: Repo, app: App, spec: FeatureSpec | undefined, problem: string | null): FeatureView { |
| 123 | 136 | const kept = featureSettings(app.db, record.id); |
| 124 | 137 | const absorbed = getAbsorbed(app.db, record.id); |
src/web/pages/apps.ts +3 −4
| @@ -12,7 +12,6 @@ | ||
| 12 | 12 | * 404 for everyone. |
| 13 | 13 | */ |
| 14 | 14 | |
| 15 | import { getAbsorbed } from "../../core/features.ts"; | |
| 16 | 15 | import { storageLimitMessage } from "../../core/storage.ts"; |
| 17 | 16 | import { repositoryLimitMessage } from "../../actions/repos.ts"; |
| 18 | 17 | import { appLimitWords } from "../../actions/app_limit.ts"; |
| @@ -572,7 +571,7 @@ export function testCopyLink(url: string): Html { | ||
| 572 | 571 | * way to the app's page in the studio, where its environments are run. |
| 573 | 572 | * Nothing for a repository that is not an app. |
| 574 | 573 | */ |
| 575 | export function appStrip(ctx: Ctx, repo: Repo): Html { | |
| 574 | export function appStrip(ctx: Ctx, repo: Repo, declares: boolean): Html { | |
| 576 | 575 | const view = appView(ctx.app, ctx.auth.user, repo); |
| 577 | 576 | if (!view.ok) return html``; |
| 578 | 577 | const { app: record, environments, serving } = view.value; |
| @@ -609,8 +608,8 @@ export function appStrip(ctx: Ctx, repo: Repo): Html { | ||
| 609 | 608 | )} |
| 610 | 609 | ${when(pace, (line) => html`<span class="muted">${line}</span>`)} |
| 611 | 610 | ${when( |
| 612 | ctx.auth.user?.isAdmin === true && ctx.auth.user.kind !== "agent" && getAbsorbed(ctx.app.db, record.id) !== undefined, | |
| 613 | () => html`<a href="/settings/features/${encodeURIComponent(record.slug)}" title="This app is one of the forge's features">Features</a>`, | |
| 611 | declares, // already gated on a site admin by `declaresFeature` | |
| 612 | () => html`<a href="/settings/features/${encodeURIComponent(record.slug)}" title="This app declares a feature: absorb it or change its settings">Features</a>`, | |
| 614 | 613 | )} |
| 615 | 614 | </div>`; |
| 616 | 615 | } |
src/web/pages/features.ts +2 −1
| @@ -96,6 +96,7 @@ export async function featuresPage(ctx: Ctx): Promise<Response> { | ||
| 96 | 96 | icon: "rocket", |
| 97 | 97 | title: "No app declares a feature yet", |
| 98 | 98 | note: 'An app declares one with a "feature" block in its forge.json: a name, a page and typed settings. Absorbing it puts it in the forge\'s nav and keeps its settings here.', |
| 99 | action: html`<p><a href="${DOCS}">How an app declares a feature</a></p>`, | |
| 99 | 100 | })} |
| 100 | 101 | </div>` |
| 101 | 102 | : html`<ul class="list setting-list"> |
| @@ -106,7 +107,7 @@ export async function featuresPage(ctx: Ctx): Promise<Response> { | ||
| 106 | 107 | <h2>Features</h2>`, |
| 107 | 108 | pad: false, |
| 108 | 109 | }, |
| 109 | )}${when(listed.value.length === 0, () => html`<p class="muted small"><a href="${DOCS}">How an app declares a feature</a></p>`)}`; | |
| 110 | )}`; | |
| 110 | 111 | const response = shell(ctx, "Features", body); |
| 111 | 112 | notePhase(ctx.request, "render", performance.now() - rendering); |
| 112 | 113 | return response; |
src/web/pages/repo.ts +3 −1
| @@ -76,6 +76,7 @@ import { notePhase, takeSessionTime, timePhase, timePhaseAsync } from "../../cor | ||
| 76 | 76 | import { liveClientJs, liveMount } from "../live.ts"; |
| 77 | 77 | import { appStrip } from "./apps.ts"; |
| 78 | 78 | import { studioApp } from "../studio/app.ts"; |
| 79 | import { declaresFeature } from "../../actions/features.ts"; | |
| 79 | 80 | import { getAppByRepo } from "../../core/apps.ts"; |
| 80 | 81 | import { field, readForm } from "../form.ts"; |
| 81 | 82 | import { html, raw, when, type Html } from "../html.ts"; |
| @@ -1383,6 +1384,7 @@ async function codeHome(ctx: Ctx, repo: Repo): Promise<Response> { | ||
| 1383 | 1384 | following !== undefined && canWrite(ctx.app.db, repo, ctx.auth.user) |
| 1384 | 1385 | ? null |
| 1385 | 1386 | : upstreamOffer(ctx.app, ctx.auth.user, repo, parent, drift?.ahead); |
| 1387 | const declares = isApp && (await timePhaseAsync(ctx.request, "feature", () => declaresFeature(ctx.app, ctx.auth.user, repo))); | |
| 1386 | 1388 | const rendering = performance.now(); |
| 1387 | 1389 | const body = html`${scopeHeader(ctx, scope, "code", { |
| 1388 | 1390 | branchCount: refs.branches.length, |
| @@ -1393,7 +1395,7 @@ async function codeHome(ctx: Ctx, repo: Repo): Promise<Response> { | ||
| 1393 | 1395 | : html`<p class="muted latest-wave">Latest: |
| 1394 | 1396 | <a href="${repoBase(repo)}/waves/${encodeURIComponent(latest.version)}">${latest.version}</a> |
| 1395 | 1397 | · ${latest.headline} · ${latest.date}</p>`} |
| 1396 | ${frozenBlock(ctx, repo)} ${appStrip(ctx, repo)} | |
| 1398 | ${frozenBlock(ctx, repo)} ${appStrip(ctx, repo, declares)} | |
| 1397 | 1399 | ${ctx.auth.user?.id === repo.ownerId && isContributionFork(ctx.app.db, repo.id) |
| 1398 | 1400 | ? contributionBar(repo) |
| 1399 | 1401 | : following === undefined |
test/features-empty.test.ts added +50
| @@ -0,0 +1,50 @@ | ||
| 1 | /** Settings › Features with no declaring app: the empty state links the guide. */ | |
| 2 | ||
| 3 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 4 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 5 | import { tmpdir } from "node:os"; | |
| 6 | import { join } from "node:path"; | |
| 7 | import { createApp, type App } from "../src/app.ts"; | |
| 8 | import { createUser } from "../src/core/users.ts"; | |
| 9 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 10 | import { quietLogs } from "./helpers/globals.ts"; | |
| 11 | ||
| 12 | quietLogs(); | |
| 13 | ||
| 14 | let dataDir = ""; | |
| 15 | let app: App; | |
| 16 | let servers: Servers; | |
| 17 | ||
| 18 | beforeAll(async () => { | |
| 19 | dataDir = await mkdtemp(join(tmpdir(), "forge-features-empty-")); | |
| 20 | app = createApp({ dataDir, port: 0, host: "forge.test" }); | |
| 21 | servers = await serve(app); | |
| 22 | const made = await createUser(app.db, { | |
| 23 | origin: "seed", | |
| 24 | trusted: true, | |
| 25 | username: "ada", | |
| 26 | email: "[email protected]", | |
| 27 | password: "a-good-passphrase", | |
| 28 | adminIfFirst: true, | |
| 29 | }); | |
| 30 | if (!made.ok) throw new Error(made.error); | |
| 31 | }, 60_000); | |
| 32 | ||
| 33 | afterAll(async () => { | |
| 34 | servers?.stop(); | |
| 35 | app?.close(); | |
| 36 | if (dataDir) await rm(dataDir, { recursive: true, force: true }); | |
| 37 | }); | |
| 38 | ||
| 39 | test("with no feature declared, the empty state carries the guide's link", async () => { | |
| 40 | const signed = await fetch(`${servers.url}/signin`, { | |
| 41 | method: "POST", | |
| 42 | body: new URLSearchParams({ username: "ada", password: "a-good-passphrase" }), | |
| 43 | redirect: "manual", | |
| 44 | }); | |
| 45 | const cookie = `${(signed.headers.get("set-cookie") ?? "").split(";")[0]!}; forge_mode=dev`; | |
| 46 | const page = await (await fetch(`${servers.url}/settings/features`, { headers: { cookie } })).text(); | |
| 47 | expect(page).toContain("No app declares a feature yet"); | |
| 48 | const empty = page.slice(page.indexOf("No app declares a feature yet")); | |
| 49 | expect(empty.slice(0, empty.indexOf("</div>"))).toContain('href="/trevato/forge/blob/main/docs/features/absorb.md"'); | |
| 50 | }); |
test/features.test.ts +35 −1
| @@ -15,7 +15,7 @@ import { join } from "node:path"; | ||
| 15 | 15 | import { statements } from "./helpers/statements.ts"; |
| 16 | 16 | import { expectWork } from "./helpers/work.ts"; |
| 17 | 17 | import { createApp, type App } from "../src/app.ts"; |
| 18 | import { absorbApp, acceptDeclaration, absorbedForNav, declaredFeature, changeFeatureSettings, getFeature, unabsorbApp } from "../src/actions/features.ts"; | |
| 18 | import { absorbApp, acceptDeclaration, declaresFeature, absorbedForNav, declaredFeature, changeFeatureSettings, getFeature, unabsorbApp } from "../src/actions/features.ts"; | |
| 19 | 19 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; |
| 20 | 20 | import { checkSetting, featureEnv, parseFeature, settingEnvName } from "../src/apps/feature.ts"; |
| 21 | 21 | import { featureVarsAt } from "../src/apps/feature_env.ts"; |
| @@ -222,6 +222,40 @@ describe("absorbing an app", () => { | ||
| 222 | 222 | expect(seen.ok && seen.value.absorbed).toBe(false); |
| 223 | 223 | }); |
| 224 | 224 | |
| 225 | test("an admin sees Features on the page of a declaring app that is not absorbed; nobody else does", async () => { | |
| 226 | expect(getAbsorbed(app.db, appId)).toBeUndefined(); | |
| 227 | const link = 'href="/settings/features/radar"'; | |
| 228 | expect(await (await web(adaCookie, "/cy/radar/files")).text()).toContain(link); | |
| 229 | // The owner reads the page, but is no site admin. | |
| 230 | const owned = await web(cyCookie, "/cy/radar/files"); | |
| 231 | expect(owned.status).toBe(200); | |
| 232 | expect(await owned.text()).not.toContain(link); | |
| 233 | const made0 = await createUser(app.db, { origin: "seed", trusted: true, username: "pixel", email: "[email protected]", password: "a-good-passphrase" }); | |
| 234 | if (!made0.ok) throw new Error(made0.error); | |
| 235 | const bot = made0.value; | |
| 236 | promote(app.db, bot); | |
| 237 | // An agent, even one holding the admin flag, is not offered it. | |
| 238 | app.db.query("UPDATE user SET is_agent = 1, is_admin = 1 WHERE id = ?1").run(bot.id); | |
| 239 | const botToken = createToken(app.db, bot.id, "read", "ro").token; | |
| 240 | const asBot = await fetch(`${servers.url}/cy/radar/files`, { headers: { authorization: `Bearer ${botToken}` } }); | |
| 241 | const botBody = await asBot.text(); | |
| 242 | expect({ status: asBot.status, link: botBody.includes(link) }).toEqual({ status: 200, link: false }); | |
| 243 | expect(botBody).toContain("app-strip"); | |
| 244 | const agent = { ...ada, kind: "agent" as const }; | |
| 245 | expect(await declaresFeature(app, agent, repo)).toBe(false); | |
| 246 | expect(await declaresFeature(app, ada, repo)).toBe(true); | |
| 247 | expect(await declaresFeature(app, undefined, repo)).toBe(false); | |
| 248 | // An app whose forge.json declares none has no link. | |
| 249 | const plain = await createRepo(app, cy, { name: "bare", visibility: "private", description: "" }); | |
| 250 | if (!plain.ok) throw new Error(String(plain.error)); | |
| 251 | await commitToBranch(app, cy, plain.value, { message: "plain", files: [{ path: "forge.json", content: JSON.stringify({ app: { run: "x" } }) }] }); | |
| 252 | const made = insertApp(app.db, { repoId: plain.value.id, slug: "bare", access: "private" }); | |
| 253 | if (!made.ok) throw new Error(made.error); | |
| 254 | const page = await web(adaCookie, "/cy/bare/files"); | |
| 255 | expect(page.status).toBe(200); | |
| 256 | expect(await page.text()).not.toContain("/settings/features/bare"); | |
| 257 | }); | |
| 258 | ||
| 225 | 259 | test("only a site admin absorbs: the repository's own admin is refused, a stranger learns nothing", async () => { |
| 226 | 260 | const byOwner = await absorbApp(app, cy, "radar"); |
| 227 | 261 | expect(!byOwner.ok && byOwner.error).toBe("forbidden"); |
The admin and agent test is repeated here although
declaresFeaturealready gates on it. Makedeclaresrequired and test it at page level for an agent (see the review body).