Public: anyone can read this; the crew can change it.
Where visitors come from, for the forge and for one app, speaks the home page's language #9898
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9891-where-visitors-come-from-for-the-forge-a → main 3 commits fork 4 threads · 4 unresolved
2 files
changed +115
−10
src/web/pages/sources.ts +15 −10
| @@ -31,7 +31,7 @@ import { | ||
| 31 | 31 | import { notFoundPage, settingsLayout, signInFirst } from "../chrome.ts"; |
| 32 | 32 | import { escapeHtml, html, raw, type Html } from "../html.ts"; |
| 33 | 33 | import { page } from "../layout.ts"; |
| 34 | import { btn, card, empty, seg } from "../ui.ts"; | |
| 34 | import { btn, seg } from "../ui.ts"; | |
| 35 | 35 | import { accountSections } from "./origins.ts"; |
| 36 | 36 | import { settingsPageResponse, timedAdminRepo } from "./settings.ts"; |
| 37 | 37 | import { getAppByRepo } from "../../core/apps.ts"; |
| @@ -105,15 +105,19 @@ function columns(days: readonly Counted[], label: string): Html { | ||
| 105 | 105 | return raw(`<svg viewBox="0 0 ${CHART_W} ${CHART_H}" width="${CHART_W}" height="${CHART_H}" role="img" aria-label="${escapeHtml(label)}">${bars.join("")}</svg>`); |
| 106 | 106 | } |
| 107 | 107 | |
| 108 | function block(title: Html | string, content: Html): Html { | |
| 109 | return html`<section class="settings-block"><h2>${title}</h2>${content}</section>`; | |
| 110 | } | |
| 111 | ||
| 108 | 112 | function panel(q: Query, title: string, rows: readonly Counted[], dimension: Dimension | null): Html { |
| 109 | return card( | |
| 113 | return block( | |
| 114 | title, | |
| 110 | 115 | rows.length === 0 |
| 111 | 116 | ? html`<p class="muted small">None in the window.</p>` |
| 112 | 117 | : html`<div class="table-scroll"> |
| 113 | 118 | <table class="table" data-dense> |
| 114 | <caption class="sr-only">${title}</caption> | |
| 115 | 119 | <thead> |
| 116 | <tr><th scope="col">${title}</th><th scope="col" class="num">Visits</th></tr> | |
| 120 | <tr><th scope="col">Value</th><th scope="col" class="num">Visits</th></tr> | |
| 117 | 121 | </thead> |
| 118 | 122 | <tbody> |
| 119 | 123 | ${rows.map( |
| @@ -143,7 +147,8 @@ function filters(q: Query): Html { | ||
| 143 | 147 | |
| 144 | 148 | function path(q: Query, now: number, db: Ctx["app"]["db"]): Html { |
| 145 | 149 | const rows = q.steps.length === 0 ? [] : funnel(db, q.scope, q.window, q.filter, q.steps, now); |
| 146 | return card( | |
| 150 | return block( | |
| 151 | "A path to a goal", | |
| 147 | 152 | html`<p class="muted small mb"> |
| 148 | 153 | Name the steps a visit takes, in order, ending in a goal: a path such as |
| 149 | 154 | <code>/signup</code> (a path also counts its sub-pages) or an outbound |
| @@ -186,7 +191,8 @@ function github(db: Ctx["app"]["db"], q: Query, now: number): Html { | ||
| 186 | 191 | GitHub's own traffic is not pulled: set <code>FORGE_GITHUB_TOKEN</code> (a token that may read repository |
| 187 | 192 | traffic) and <code>FORGE_GITHUB_REPOS</code> (<code>owner/name,…</code>) and it is stored daily, since GitHub keeps only 14 days. |
| 188 | 193 | </p>`; |
| 189 | return card( | |
| 194 | return block( | |
| 195 | "GitHub", | |
| 190 | 196 | html`<div class="table-scroll"> |
| 191 | 197 | <table class="table" data-dense> |
| 192 | 198 | <caption class="sr-only">GitHub repository traffic, pulled daily</caption> |
| @@ -205,7 +211,6 @@ function github(db: Ctx["app"]["db"], q: Query, now: number): Html { | ||
| 205 | 211 | </tbody> |
| 206 | 212 | </table> |
| 207 | 213 | </div>`, |
| 208 | { head: html`<h2>GitHub</h2>` }, | |
| 209 | 214 | ); |
| 210 | 215 | } |
| 211 | 216 | |
| @@ -224,8 +229,9 @@ export function sourcesBody(ctx: Ctx, q: Query, label: string): Html { | ||
| 224 | 229 | </div> |
| 225 | 230 | ${filters(q)} |
| 226 | 231 | ${empties |
| 227 | ? empty({ icon: "users", title: `No visits recorded for ${label} in ${WINDOWS[q.window]} days`, note: "Views are written every few seconds; a page counts when it is left." }) | |
| 228 | : html`${card( | |
| 232 | ? html`<p class="muted">No visits recorded for ${label} in ${WINDOWS[q.window]} days.</p>` | |
| 233 | : html`${block( | |
outdated
Principle 1 and 8: title and note are fused into one long line, and data-empty is used nowhere else. Say it in one short sentence. |
||
| 234 | "Visits", | |
| 229 | 235 | html`<dl class="stats"> |
| 230 | 236 | <div><dt>${scraping ? "Scraper visits" : "Visits"}</dt><dd>${figure(q, r.visits, { number: scraping ? "scrapers" : "visits" })}</dd></div> |
| 231 | 237 | <div><dt>Pages per visit</dt><dd><a href="${probe(q, { number: "page_views" })}" title="How this was counted">${r.pagesPerVisit}</a></dd></div> |
| @@ -240,7 +246,6 @@ export function sourcesBody(ctx: Ctx, q: Query, label: string): Html { | ||
| 240 | 246 | ${panel(q, "Landing page", r.landings, "landing")} |
| 241 | 247 | ${panel(q, "Where people left (last page)", r.exits, "exit")} |
| 242 | 248 | ${panel(q, `Outbound clicks to ${outboundNames().join(", ")}`, r.outbound, null)}`} |
| 243 | <h2 class="mt">A path to a goal</h2> | |
| 244 | 249 | ${path(q, now, db)} |
| 245 | 250 | ${q.scope === "forge" ? github(db, q, now) : null} |
| 246 | 251 | <p class="small muted mt"> |
test/sources-language.test.ts added +100
| @@ -0,0 +1,100 @@ | ||
| 1 | /** | |
| 2 | * The sources view speaks the home page's language (#9891): plain sections | |
| 3 | * with a heading and a table, no boxes; values from visitors stay escaped. | |
| 4 | */ | |
| 5 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 6 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 7 | import { tmpdir } from "node:os"; | |
| 8 | import { join } from "node:path"; | |
| 9 | import { createRepo } from "../src/actions/repos.ts"; | |
| 10 | import { createApp, type App } from "../src/app.ts"; | |
| 11 | import { createApp as insertApp } from "../src/core/apps.ts"; | |
| 12 | import { createSession } from "../src/core/sessions.ts"; | |
outdated
Duplicate import from views_store.ts; merge into one. |
||
| 13 | import { createUser } from "../src/core/users.ts"; | |
| 14 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 15 | import { flushViews, noteView } from "../src/perf/views_store.ts"; | |
| 16 | ||
| 17 | let dir: string; | |
| 18 | let app: App; | |
| 19 | let servers: Servers; | |
| 20 | let cookie: string; | |
| 21 | let appId: string; | |
| 22 | ||
| 23 | beforeAll(async () => { | |
| 24 | dir = await mkdtemp(join(tmpdir(), "forge-sources-lang-")); | |
| 25 | app = createApp({ dataDir: dir, port: 0 }); | |
| 26 | servers = await serve(app); | |
| 27 | const made = await createUser(app.db, { origin: "seed", trusted: true, username: "ada", email: "[email protected]", password: "correct-horse-battery", isAdmin: true }); | |
| 28 | if (!made.ok) throw new Error(made.error); | |
| 29 | cookie = `forge_session=${createSession(app.db, made.value.id).token}`; | |
| 30 | const repo = await createRepo(app, made.value, { name: "notes", visibility: "public", description: "" }); | |
| 31 | if (!repo.ok) throw new Error(String(repo.error)); | |
| 32 | const row = insertApp(app.db, { repoId: repo.value.id, slug: "notes", access: "public" }); | |
| 33 | if (!row.ok) throw new Error(row.error); | |
| 34 | appId = row.value.id; | |
| 35 | }); | |
| 36 | afterAll(async () => { | |
| 37 | await servers.stop(); | |
| 38 | await rm(dir, { recursive: true, force: true }); | |
| 39 | }); | |
| 40 | ||
| 41 | const count = (s: string, re: RegExp): number => (s.match(re) ?? []).length; | |
| 42 | const get = async (path: string): Promise<string> => (await fetch(`${servers.url}${path}`, { headers: { cookie } })).text(); | |
| 43 | ||
| 44 | test("empty window: one h1, plain sections, no cards", async () => { | |
| 45 | const body = await get("/settings/traffic/sources"); | |
| 46 | expect(count(body, /<h1[ >]/g)).toBe(1); | |
| 47 | expect(body).not.toContain('class="card'); | |
| 48 | expect(body).not.toContain("card-body"); | |
| 49 | expect(body).toContain("No visits recorded for this forge in 7 days."); | |
| 50 | expect(body).toContain("<h2>A path to a goal</h2>"); | |
| 51 | }); | |
| 52 | ||
| 53 | test("the app page: one h1, plain sections, no cards, escaped values", async () => { | |
| 54 | const empty = await get("/ada/notes/settings/app/traffic"); | |
| 55 | expect(count(empty, /<h1[ >]/g)).toBe(1); | |
| 56 | expect(empty).not.toContain('class="card'); | |
| 57 | expect(empty).toContain("No visits recorded for this app in 7 days."); | |
| 58 | noteView(app.db, { | |
| 59 | scope: appId, | |
| 60 | path: "/", | |
| 61 | origin: { refHost: "", source: "<script>alert(2)</script>", medium: "", campaign: "", out: "" }, | |
| 62 | client: "c2", | |
| 63 | agent: "Mozilla/5.0 (X11; Linux x86_64) Chrome/130.0 Safari/537.36", | |
| 64 | at: Date.now(), | |
| 65 | }); | |
| 66 | flushViews(app.db); | |
| 67 | const body = await get("/ada/notes/settings/app/traffic"); | |
outdated
Add a test for the per-app traffic page (one h1, no cards, h2 per panel) and for the empty text; the issue says both pages. |
||
| 68 | expect(count(body, /<h1[ >]/g)).toBe(1); | |
| 69 | expect(body).not.toContain('class="card'); | |
| 70 | expect(body).not.toContain("card-body"); | |
| 71 | for (const h of ["Visits", "Source", "Medium (utm_medium)", "Campaign (utm_campaign)", "Landing page", "Where people left (last page)"]) | |
| 72 | expect(body).toContain(`<h2>${h}</h2>`); | |
| 73 | expect(body).toContain("<svg"); | |
| 74 | expect(body).not.toContain("<script>alert(2)</script>"); | |
| 75 | expect(body).toContain("<script>"); | |
| 76 | }); | |
| 77 | ||
| 78 | test("with visits: sections with h2 and tables, filter links, chart, escaping", async () => { | |
| 79 | noteView(app.db, { | |
| 80 | scope: "forge", | |
| 81 | path: "/explore", | |
| 82 | origin: { refHost: "", source: "<script>alert(1)</script>", medium: "", campaign: "<b>x</b>", out: "" }, | |
| 83 | client: "c1", | |
| 84 | agent: "Mozilla/5.0 (X11; Linux x86_64) Chrome/130.0 Safari/537.36", | |
| 85 | at: Date.now(), | |
| 86 | }); | |
| 87 | flushViews(app.db); | |
| 88 | const body = await get("/settings/traffic/sources"); | |
| 89 | expect(count(body, /<h1[ >]/g)).toBe(1); | |
| 90 | expect(body).not.toContain('class="card'); | |
| 91 | expect(body).not.toContain("card-body"); | |
| 92 | for (const h of ["Visits", "Source", "Medium (utm_medium)", "Campaign (utm_campaign)", "Landing page", "Where people left (last page)"]) | |
| 93 | expect(body).toContain(`<h2>${h}</h2>`); | |
| 94 | expect(body).toContain("None in the window."); | |
| 95 | expect(body).toContain("<svg"); | |
| 96 | expect(body).toContain("source=%3Cscript%3E"); | |
| 97 | expect(body).not.toContain("<script>alert(1)</script>"); | |
| 98 | expect(body).not.toContain("<b>x</b>"); | |
| 99 | expect(body).toContain("<script>"); | |
| 100 | }); |
Principle 1: the h2 already names the panel; the caption and column header below repeat it. Drop one.