diff --git a/.eslintrc.json b/.eslintrc.json deleted file mode 100644 index 8693844..0000000 --- a/.eslintrc.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "extends": ["next/core-web-vitals", "prettier"], - "rules": { - "react/no-unescaped-entities": "off" - } -} diff --git a/.github/workflows/firebase-deploy-test.yml b/.github/workflows/firebase-deploy-test.yml index 087b592..6b9cea8 100644 --- a/.github/workflows/firebase-deploy-test.yml +++ b/.github/workflows/firebase-deploy-test.yml @@ -8,6 +8,20 @@ on: branches: ["test"] workflow_dispatch: +# Deploys to one Firebase project must not overlap. Two merges landing within +# a few minutes of each other used to start two deploys at once; the second +# one's update of the SSR function was rejected with a 409 ("unable to queue +# the operation") because the first was still mid-flight, and the site kept +# serving the previous build. Note that firebase deploy reports that failure +# as a warning and still exits 0, so the collision does not show up as a red +# check -- keeping deploys serialized is what prevents it, not the exit code. +# cancel-in-progress stays false on purpose -- +# a deploy that is already updating functions should be allowed to finish, +# not killed halfway through. Later runs queue behind it instead. +concurrency: + group: deploy-test + cancel-in-progress: false + env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true #FIREBASE_TOKEN: ${{ secrets.FIREBASE_TOKEN }} @@ -20,6 +34,20 @@ env: NEXT_PUBLIC_GENERATE_STATE: "https://datapipe-test.web.app/api/generateoauthstate" NEXT_PUBLIC_BASE_URL: "https://datapipe-test.web.app" NEXT_PUBLIC_OSF_ENV: "" + # Frontend-only: drives the TestEnvironmentWarning banner gate (see + # components/TestEnvironmentWarning.js). Distinct from NEXT_PUBLIC_OSF_ENV + # above, which is being retired as OSF is deprecated and was left "" here + # -- that meant the banner stopped rendering on this site once OSF_ENV + # went empty. Not echoed into functions/.env; functions has no use for it. + NEXT_PUBLIC_DEPLOY_ENV: "test" + # sandbox.zenodo.org. Without this the TEST site creates real + # depositions on the live Zenodo using the researcher's real account. + NEXT_PUBLIC_ZENODO_ENV: "sandbox." + # Google Picker (folder selection for gdrive experiments). The API key is + # restricted to the Picker API + our domains, so it is safe in the browser + # bundle -- not a secret. The project number is public. + NEXT_PUBLIC_GOOGLE_PICKER_API_KEY: "AIzaSyDLg6uprrY5BPjY4ClVZGSvy7sd_ug0t9M" + NEXT_PUBLIC_GDRIVE_PROJECT_NUMBER: "699904257039" jobs: deploy: @@ -41,16 +69,13 @@ jobs: with: credentials_json: '${{ secrets.GOOGLE_TEST_CREDENTIALS }}' # Replace with the name of your GitHub Actions secret - name: Install firebase tools - run: npm install -g firebase-tools@15.8.0 + run: npm install -g firebase-tools@15.28.1 - name: Enable firebase webframeworks run: firebase experiments:enable webframeworks - name: Install dependencies run: npm ci - - name: Install dependencies and build metadata - working-directory: functions/metadata - run: | - npm ci - npm run build + # functions/metadata is a pre-built vendored dist (see functions/metadata/README.md); + # it is installed as a file: dependency by the functions npm ci below. - name: Create functions environment file working-directory: functions run: | @@ -59,6 +84,34 @@ jobs: echo "REDIRECT_URI=https://datapipe-test.web.app/oauth2/callback" >> .env echo "TOKEN_ENCRYPTION_KEY=${{ secrets.FIRESTORE_KEY_TEST }}" >> .env echo "NEXT_PUBLIC_OSF_ENV=" >> .env + # Google Drive client secret for the deployed test site. Client id and + # redirect uri are non-secret and live in functions/.env.datapipe-test. + echo "GDRIVE_CLIENT_SECRET=${{ secrets.TEST_GDRIVE_CLIENT_SECRET }}" >> .env + # Zenodo OAuth client secret for the deployed test site. Client id, + # redirect uri and ZENODO_ENV are non-secret and live in + # functions/.env.datapipe-test. + echo "ZENODO_CLIENT_SECRET=${{ secrets.TEST_ZENODO_CLIENT_SECRET }}" >> .env + # Resend for the deployed test site (functions/src/mail-delivery.ts). + # SET this one. The test site is the only place mail DELIVERY is + # exercised before production -- the emulator short-circuits before + # sending and the unit suites mock the transport -- so it is where a + # mail change is proved to pass DKIM/SPF and reach an inbox. Use a + # SEPARATE sending-only key from prod, on the same Resend account + # (the From domain is the same, so it has to be the same account). + # Unset is still safe rather than silent: the onmailcreated trigger + # records a terminal MailConfigMissingError on each mail document and + # sends nothing. Reputation and daily quota are shared with prod -- + # see docs/deploy-contact-email.md §2(d). + echo "RESEND_API_KEY=${{ secrets.TEST_RESEND_API_KEY }}" >> .env + # Not secret, so literals here, the same way REDIRECT_URI above is. + # Same sender as production: Resend verifies the DOMAIN + # (jspsych.org), not the deployment, and datapipe-test.web.app is not + # a verified domain -- sending from it is a 403 validation_error, + # which is why contact-email codes never arrived on the test site. + # Only the display name differs, so test mail is recognisable in an + # inbox. + echo "MAIL_FROM=DataPipe (test) " >> .env + echo "MAIL_REPLY_TO=datapipe@jspsych.org" >> .env - name: Install dependencies and build functions working-directory: functions run: | diff --git a/.github/workflows/firebase-deploy.yml b/.github/workflows/firebase-deploy.yml index 39f945f..1cd2ebd 100644 --- a/.github/workflows/firebase-deploy.yml +++ b/.github/workflows/firebase-deploy.yml @@ -8,6 +8,14 @@ on: branches: ["main"] workflow_dispatch: +# See the matching block in firebase-deploy-test.yml: concurrent deploys to the +# same Firebase project collide on the SSR function update and leave the site +# on the old build. Separate group from the test deploy -- they target +# different projects and have no reason to queue behind each other. +concurrency: + group: deploy-production + cancel-in-progress: false + env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true #FIREBASE_TOKEN: ${{ secrets.FIREBASE_TOKEN }} @@ -20,6 +28,9 @@ env: NEXT_PUBLIC_GENERATE_STATE: "https://pipe.jspsych.org/api/generateoauthstate" NEXT_PUBLIC_BASE_URL: "https://pipe.jspsych.org" NEXT_PUBLIC_OSF_ENV: "" + # Empty = the real zenodo.org. Set explicitly rather than relying on the + # default, so production never inherits a sandbox value by accident. + NEXT_PUBLIC_ZENODO_ENV: "" jobs: build: @@ -41,16 +52,13 @@ jobs: with: credentials_json: '${{ secrets.GOOGLE_PRODUCTION_CREDENTIALS }}' - name: Install firebase tools - run: npm install -g firebase-tools@15.8.0 + run: npm install -g firebase-tools@15.28.1 - name: Enable firebase webframeworks run: firebase experiments:enable webframeworks - name: Install dependencies run: npm ci - - name: Install dependencies and build metadata - working-directory: functions/metadata - run: | - npm ci - npm run build + # functions/metadata is a pre-built vendored dist (see functions/metadata/README.md); + # it is installed as a file: dependency by the functions npm ci below. - name: Create functions environment file working-directory: functions run: | @@ -59,6 +67,17 @@ jobs: echo "REDIRECT_URI=https://pipe.jspsych.org/oauth2/callback" >> .env echo "TOKEN_ENCRYPTION_KEY=${{ secrets.FIRESTORE_KEY_PRODUCTION }}" >> .env echo "NEXT_PUBLIC_OSF_ENV=" >> .env + # Resend. functions/src/mail-delivery.ts delivers everything mail.ts + # queues into the `mail` collection; without this key the + # onmailcreated trigger writes a terminal MailConfigMissingError on + # every notification and sends nothing. Scope the key to Sending + # access only -- see docs/deploy-contact-email.md §2, which also + # covers domain verification. + echo "RESEND_API_KEY=${{ secrets.PROD_RESEND_API_KEY }}" >> .env + # Not secret, so literals here, the same way REDIRECT_URI above is. + # MAIL_FROM must be an address on a domain verified in Resend. + echo "MAIL_FROM=DataPipe " >> .env + echo "MAIL_REPLY_TO=datapipe@jspsych.org" >> .env - name: Install dependencies and build functions working-directory: functions run: | diff --git a/.github/workflows/metadata-drift-check.yml b/.github/workflows/metadata-drift-check.yml new file mode 100644 index 0000000..d4b10ee --- /dev/null +++ b/.github/workflows/metadata-drift-check.yml @@ -0,0 +1,74 @@ +# Nudge (never an auto-merge): flags when upstream @jspsych/metadata main has moved past +# the commit DataPipe currently vendors. The vendored copy is pinned + built by +# functions/scripts/sync-metadata.mjs; this job just tells a human it's time to re-sync. +name: Metadata vendor drift check + +on: + schedule: + - cron: "0 12 * * 1" # Mondays 12:00 UTC + workflow_dispatch: + +permissions: + contents: read + issues: write + +jobs: + drift-check: + runs-on: ubuntu-latest + env: + UPSTREAM: jspsych/metadata + ISSUE_TITLE: "[metadata-sync] Vendored @jspsych/metadata is behind upstream main" + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@v6 + + - name: Compare pinned commit against upstream main + id: compare + run: | + PINNED=$(jq -r .commit functions/metadata/VENDORED_FROM.json) + echo "Pinned commit: $PINNED" + if [ -z "$PINNED" ] || [ "$PINNED" = "null" ]; then + echo "::error::Could not read pinned commit from functions/metadata/VENDORED_FROM.json" + exit 1 + fi + UPSTREAM_MAIN=$(gh api "repos/$UPSTREAM/commits/main" --jq .sha) + echo "Upstream main: $UPSTREAM_MAIN" + if [ "$PINNED" = "$UPSTREAM_MAIN" ]; then + echo "in_sync=true" >> "$GITHUB_OUTPUT" + echo "Vendored copy is up to date with upstream main." + exit 0 + fi + # ahead_by = how many commits upstream main is ahead of our pinned commit. + AHEAD=$(gh api "repos/$UPSTREAM/compare/$PINNED...main" --jq .ahead_by) + echo "in_sync=false" >> "$GITHUB_OUTPUT" + echo "ahead=$AHEAD" >> "$GITHUB_OUTPUT" + echo "pinned=$PINNED" >> "$GITHUB_OUTPUT" + echo "upstream=$UPSTREAM_MAIN" >> "$GITHUB_OUTPUT" + + - name: Open or update tracking issue + if: steps.compare.outputs.in_sync == 'false' + run: | + BODY=$(cat < **Superseded — the teal fix below is now moot; the green replaces it.** `#13b24b` on +> white was **2.80:1**: it could never be light-mode text, and never a light-mode solid +> fill under white text, and `solid: brandTeal.600` + `white` was **4.04:1 — a live AA +> failure in both modes.** The teal fix was to flip light to `700` and flip dark's +> *text* to `#1C1F22` on the bright `500` fill. Adopting the logo green retires the +> problem at its source instead: `#2E7D32` clears 4.5:1 on both light surfaces, so +> light-mode green text and a white-on-green solid are both legal for the first time. +> The dark-side flip survives on its merits — computed both ways on `#1C1F22`, a dark +> fill under white text (`800` + `white`) gives fill 3.23 / text 5.13, while the bright +> fill under dark text (`500` + `#1C1F22`) gives fill 5.96 / text 5.96, better on both +> axes. On a dark page a dark green button is a hole; the bright chip reads as a control. + +> **Caveat on `subtle`.** Chakra's `subtle` and `surface` variants paint +> `colorPalette.fg` on `colorPalette.subtle`, and in dark mode that pairing is +> `300` on `900` = **3.91:1**, under the body floor. Material Green 900 is a mid-dark +> green, not the near-black the hand-tuned teal 900 was, and the ramp has nothing +> darker. Text on `brandGreen.subtle` is therefore named explicitly (`50`, above), and +> `variant="subtle"` / `variant="surface"` on `brandGreen` is **not approved for body +> text** until a semantic pairing token exists. No call site uses either variant on +> this palette today. + +**brandOrange** — warning / attention only. + +| Slot | Light | ratio | Dark | ratio | +|---|---|---|---|---| +| `fg` | `800 #7C4606` | 6.63 | `300 #FFB74D` | 7.80 | +| `subtle` (bg) | `50 #FFF3E0` | text `800` → 7.00 | `900 #3E2303` | text `gray.200` → 11.44 | +| `border` | `700 #A85F08` | 4.54 | `400 #FFA726` | 8.52 | + +**`brandOrange` has no `solid`.** Every orange dark enough to hold white text +(`700` = 4.88) has stopped being the brand orange. Orange is a status hue, never a +button fill. + +**brandRed** — irreversible destruction only. + +| Slot | Light | ratio | Dark | ratio | +|---|---|---|---|---| +| `fg` | `700 #A82E16` | 5.92 | `300 #F17761` | 4.86 (on `bg.muted`) | +| `solid` | `700 #A82E16` | fill 6.37 | `600 #D13A1B` | fill 3.42 | +| `contrast` | `white` | 6.85 | `white` | 4.85 | +| `subtle` (bg) | `50 #FDE8E4` | text `800` → 8.30 | `900 #4A1509` | text `gray.200` → 11.80 | +| `border` | `600 #D13A1B` | 4.51 | `400 #EF5A3E` | 4.89 | + +**gray** — neutral controls (the default `colorPalette`). + +| Slot | Light | Dark | +|---|---|---| +| `fg` | `800 #27272a` (13.86) | `200 #e4e4e7` (13.05) | +| `solid` / `contrast` | `800` / `gray.50` → 14.27 | `200` / `gray.900` → 13.96 | +| `subtle` / `muted` / `emphasized` | `100` / `200` / `300` | `800` / `700` / `600` | +| `border` | `500` (4.50) | `500` (3.43) | + +**Status** aliases onto the brand hues — one green, not two: +`ok = brandGreen`, `warning = brandOrange`, `error = brandRed`, `neutral = fg.muted` +(neutral; **no blue**). `brandLime` is legacy and should be deleted once +`JsPsychIcon` is confirmed to be its only consumer. + +### Logo + +The mark (`docs/brand/logo/README.md`) is the source of the brand green and is +authoritative over the ramp, not the other way round. + +| Role | Value | Where | +|---|---|---| +| Bar + chevron 1, light bg | `#2E7D32` | = `brandGreen.800`. The anchor | +| Bar + chevron 1, dark bg | `#F2F5F1` | 15.06 on `#1C1F22`. Not `fg` — the mark keeps its own paper white | +| Chevron 2 (echo) | `#8BC34A` | **Mark only.** Identical in both modes by design | + +**The navbar renders on the dark ground only** (light mode is retired, §2). The bar +keeps its `bg` + `border`-bottom semantics and the mark/wordmark render via +`logo.mark`, resolving to `#F2F5F1` — the mark's own paper white, deliberately not +`fg`. The logo handoff still specifies both grounds; the light-bg values document +the mark itself (README badges, external use), not any shipped surface. + +**Mode-invariant `code.*` tokens** carry the code-specimen "device" (landing terminal +mock, `CodeBlock`, `CodeHints`): `code.bg = gray.950 #111111` (17.57:1 against the +light page — a deliberate object), `code.bg.header`, `code.bg.active`, `code.border = +gray.500` (the seam: 4.50 light / 3.43 dark, one value both modes), `code.border.subtle`, +`code.fg 12.78:1`, `code.fg.strong`, `code.fg.muted 7.37:1`, `code.comment`, +`code.string 10.91:1`, `code.fn 9.38:1`. `_light` and `_dark` are identical *on +purpose*; the invariance is the design. + +**`status.*` aliases** exist as tokens (`status.ok/warning/error/neutral`) mirroring +each palette's `fg` slot in both modes — components never hand-pick status hues. + +**The echo green `#8BC34A` is never a UI color.** It is 2.10:1 on white and off the +Material Green ramp entirely — it exists because it is the one tone that holds against +both `#FFFFFF` and the logo's `#101A14`, inside a mark where it carries no meaning on +its own. It is never text, never a fill, never a border, never a status hue. + +--- + +## 2. Mode strategy + +**Dark is the product's only mode** (owner decision, 2026-08-23). A three-way +System/Light/Dark control shipped briefly once the token migration completed, and +was retired: the light rendering never looked right, and a twice-a-year control was +not worth carrying UI surface and a second design target for every future change. +`pages/_app.js` pins `forcedTheme="dark"` — forced, not merely defaulted, so a +Light/System preference stored in `localStorage` while the toggle existed cannot +resurrect the retired mode. `components/ThemeSelect.js` and the `COLOR_MODE_TOGGLE` +flag (`lib/feature-flags.js`) are deleted; `git log` has the full migration history +if the decision is ever revisited. + +**What survives the retirement.** The token conversion the toggle motivated is kept +and remains mandatory: every component consumes semantic tokens (`fg`, `bg.panel`, +`brandOrange.subtle`, …), never raw color literals. The `_light` branches in +`lib/theme.js` are inert under forced dark and stay in place — they cost nothing, +keep Chakra's token shape idiomatic, and are the escape hatch if this is revisited. +Judge every new color choice against the dark surfaces only. + +--- + +## 3. Typography + +One family for UI. Body, headings, labels, buttons and data all run on the existing +system stack (`-apple-system, BlinkMacSystemFont, Segoe UI, Roboto, …`). **Rubik is +retired entirely.** The wordmark is **Space Grotesk, lockup-only** — the `LogoMark` + +wordmark pairing in `Navbar.js` (600 / 22px / -0.03em), owner-ratified with the |> +logo adoption. It appears nowhere else: not in headings, not in the hero, not in +labels — a display face in UI text is a product-register ban, and the single webfont +load is spent on the brand lockup alone. + +Fixed rem scale, tight ratio, four roles: + +| Role | Size / weight | Color | Notes | +|---|---|---|---| +| Page title | `2xl` (24px) / 700 | `fg` | One per page. Sentence case | +| Section heading | `lg` (18px) / 600 | `fg` | A real ``, sentence case | +| Body | `md` (16px) / 400 | `fg` | Default. Prose measure 65–75ch | +| Supporting | `sm` (14px) / 400 | `fg.muted` | Descriptions, hints, help lines | +| Fine print | `xs` (12px) / 400 | `fg.muted` | Timestamps, IDs. **Never** `fg.subtle` at this size | + +**Sentence case everywhere**, including buttons. `textTransform="uppercase"` + +`letterSpacing="wide"` micro-labels are banned — see §8. Today's codebase is +`fontSize="sm"` 74×, `xs` 19×: a page whose default text is 14px is a page with no +body size. Let 16px be the default and reserve `sm` for genuinely secondary text. + +--- + +## 4. Spacing & layout rhythm + +- **Settings / single-subject content column: `maxW="560px"`.** Confirmed keep — a + correct, confident measure for a scanned page. Dashboard and marketing pages use + `maxW="1100px"`; consolidate the stray `540px` / `960px` / `440px` onto these two. +- Spacing scale: Chakra's 4px base. Use `2 / 3 / 4 / 6 / 8 / 12 / 16` and nothing + else. Within a row: `gap={3}`. Within a section: `gap={4}`. Between routine + sections: `mt={10}`. Before a consequential section (Danger Zone, anything + destructive): `mt={16}`. +- **Spacing carries grouping.** More air means "different subject, higher stakes." + A page whose sections are all `my={6}` apart is telling the reader nothing. + +**Separator policy — committed:** *spacing-only grouping between sections.* Remove +every `` from `pages/admin/account.js` (4×) and +`pages/admin/[experiment_id].js` (4×). A hairline at 1.27:1 is not a weak separator, +it is an absent one, and raising it to a visible 3:1 rule between five sections makes +a settings page look like a spreadsheet. Separators survive only *inside* dense +repeating structures — table row rules, menu group dividers — where they use `border` +or `border.subtle` and are not the primary grouping device. Grouping that spacing +cannot carry alone gets a bordered container (see `SettingsSection` danger variant). + +--- + +## 5. Color semantics + +- **`brandGreen` is the primary action color, app-wide. One primary per screen.** + Every other action on that screen is `outline` or `ghost` on `gray`. +- **`blue` is retired as an action color.** Five `colorPalette="blue"` and five raw + `blue.500` links remain (`ProviderConnections.js:188,247`, `SelectAuth.js:120`, + `OAuthTokenStatus.js:92`, both `oauth2/*` pages, `QueuePanel.js` status map). + Links become `brandGreen.fg` **with a persistent underline** — no green/body text + pair reaches the 3:1 color-difference floor in either mode (2.04 light / 1.36 dark), + so color alone can never mark a link. `globals.css` strips the default underline; + every prose link sets it back explicitly. + Secondary buttons become neutral outline. +- **`brandRed` is exclusively for irreversible destruction** — account deletion, + experiment deletion. Routine, reversible actions (disconnect a provider, unlink a + sign-in method) are **neutral outline**. Red that means "routine" cannot also mean + "final". +- **Status trio** `ok` / `warning` / `error` (+ `neutral`), values in §1. **Status + is never color-alone or icon-alone: a visible text label is mandatory**, always + rendered, never behind a tooltip or `title`. Non-text status marks still clear 3:1. +- **Focus ring:** `2px solid {colorPalette}.focusRing` with a `2px` offset, on *every* + interactive element including icon-only and link-styled controls. Default palette + ring is `brandGreen.focusRing` (3.27:1 worst case light, 5.71:1 dark). + `focusRing="none"` — currently on four `Navbar` links — is banned. + The ring is **keyboard-only**: it paints on `:focus-visible`, never on a bare + `:focus`. A mouse click already tells you what you clicked, so a ring that + outlives the click is visual noise — on the navbar and the docs sidebar it + reads as a stray light box stuck to the last item you touched. Chakra's stock + `link` recipe gets this wrong (it ships `focusRing`, which compiles to + `&:is(:focus, [data-focus])`, while every other recipe uses + `focusVisibleRing`), so `lib/theme.js` re-points the recipe. Suppressing the + pointer case is not the banned `focusRing="none"`: keyboard focus still draws + the identical ring, which is the whole point of the rule above. +- **Semantic z-index scale.** `globals.css:54` has the app's only z-index, an + arbitrary `1000`. Replace with theme tokens and use nothing else: + + | Token | Value | Use | + |---|---|---| + | `docked` | 10 | Sticky table headers | + | `dropdown` | 1000 | Menus, popovers, selects | + | `sticky` | 1100 | Sticky page chrome | + | `banner` | 1200 | `.sticky-alert` / `TestEnvironmentWarning` | + | `modal.backdrop` | 1300 | Dialog backdrop | + | `modal` | 1400 | Dialog content | + | `toast` | 1500 | Transient notifications | + | `tooltip` | 1600 | Tooltips (decoration only — never meaning) | + +--- + +## 6. Component inventory + +Shared primitives live in `components/ui/`. Every interactive one ships all seven +states — default, hover, focus, active, disabled, loading, error — or it does not +ship. + +**Being built now:** + +- **`SettingsSection`** — a real `

` (`lg`/600/`fg`), optional one-line description + in `sm`/`fg.muted` that says what the section is and what depends on it, and the + section body. `variant="danger"` wraps the body in a `1px border.brandRed` container + with `p={5}` and `rounded="md"`. Replaces `SectionLabel` entirely. +- **`StatusIndicator`** — `status` (`ok`/`warning`/`error`/`neutral`) plus a + **mandatory visible `label`**. Icon + text, always both, always rendered. No tooltip + variant exists, so `OAuthTokenStatus`'s hover-only state cannot be reproduced. + Replaces all three of the account page's competing status renderings. +- **`FormErrorAlert`** — the single form-error surface. Takes a human message (mapped + through `lib/auth-errors.js`, never a raw Firebase code), renders on + `brandRed.subtle` with `brandRed.fg` text and `role="alert"`. One pattern for the + three error paths that currently disagree. +- **`ConfirmDialog`** — async `onConfirm` with a loading state on the confirm button; + failures are caught and surfaced **inside the dialog via `FormErrorAlert`**, and the + dialog stays open. **Cancel is always neutral** (`variant="outline"`, + `colorPalette="gray"`) and is the default-focused control; the confirm button carries + `brandRed` when `destructive`, `brandGreen` — the primary — otherwise. The green solid + "Cancel" in `DeleteAccount.js:100` is the exact shape this bans. + +**Anticipated for the wider pass:** + +- **`PageHeader`** — page title, optional one-sentence purpose line, optional back + link. Every page gets one; `/admin/account` currently has a bare `Heading` and no + route back to the dashboard. +- **`EmptyState`** — a heading, one sentence of what goes here and why, and the single + primary action. Text only: no illustration, no mascot, no emoji. +- **`GuidanceLine`** — the standard help line under a section heading: `sm`/`fg.muted`, + consequence before mechanism, with an inline link to `/getting-started` or `/docs` + where one exists. Shown at zero-state as well as one-state. + +--- + +## 7. Motion + +Minimal and purposeful. Motion conveys state change, feedback, loading, or reveal — +nothing else. No scroll-jacking, no parallax, no bounce or spring easing, no +orchestrated page-load sequences. + +- Duration `150–200ms`; easing `ease-out` only (`cubic-bezier(0, 0, 0.2, 1)`). +- Color-mode switches do **not** animate (`disableTransitionOnChange`). +- **Every animation needs a `prefers-reduced-motion` story.** The `.loader` spinner + (`globals.css:26–42`) has none. Its fallback: under + `@media (prefers-reduced-motion: reduce)` drop the `spin` animation, leaving a static + ring plus a visible `aria-live` "Loading…" label — the label is required regardless of + motion preference. Its `white` / `darkblue` borders become `border.subtle` (track) / + `brandGreen.solid` (arc) — the arc-vs-track pair computes 3.47:1 light / 3.54:1 dark; + a `border` track would sit at 1.06:1 against the light arc and the ring would appear + static. +- Skeletons over centered spinners for content loading in place; spinners are for + actions, not regions. + +--- + +## 8. Anti-patterns — codebase-specific bans + +1. **Uppercase tracked eyebrow micro-labels.** `pages/admin/account.js:21–34` + `SectionLabel` renders five of them at `xs` + `uppercase` + `letterSpacing="wide"` + + `gray.500` (3.43:1) — the least legible configuration available, and an eyebrow on + every section is scaffolding by reflex. Real headings, sentence case. +2. **Icon-only or color-only status.** `OAuthTokenStatus` hides its state behind a + hover tooltip: unreachable on touch, unreliable by keyboard, unannounced. +3. **Meaning in `title=` or a tooltip on a disabled control.** + `LinkedAccounts.js:129` explains why unlinking is blocked in a `title` attribute on + a disabled button. The explanation goes in visible text next to the control. +4. **Green / primary-solid cancel buttons.** `DeleteAccount.js:100`. Cancel is neutral; + the loud button is never the safe one. +5. **Raw hex or raw palette steps in components.** `color="white"`, + `bg="greyBackground"`, `bg="black"`, `whiteAlpha.*`, `gray.400`, `blue.500` — tokens + only. Third-party brand SVGs are the sole exception. +6. **Arbitrary z-index values.** `globals.css:54` `z-index: 1000`. Use the §5 scale. +7. **Silent catch blocks that swallow user-facing failures.** `handleConnect` and + `handleDisconnect` in `ProviderConnections.js` end in `console.error` with nothing + rendered; `ChangePassword` discards `auth/requires-recent-login` and shows the word + "Failed". PRODUCT.md principle 5 is "no silent failures" — every failed action + surfaces a mapped, human message through `FormErrorAlert`. +8. **`focusRing="none"`.** Four instances in `Navbar.js`. Keyboard operability is not + optional. +9. **Validation errors before first input.** Gate on touched/dirty, not on value. +11. **Marketing copy that describes a retired product.** The landing page (and any + public surface) must describe what `lib/provider-config.js` actually ships. When + providers change, the landing copy converts in the same slice — stale "OSF" + claims on the trust-deciding surface are a P0, not a copy nit. +10. **Modal as first thought.** Exhaust inline and progressive disclosure first; + dialogs are for confirming consequences, not for holding forms that fit on a page. diff --git a/PRODUCT.md b/PRODUCT.md new file mode 100644 index 0000000..3597b0b --- /dev/null +++ b/PRODUCT.md @@ -0,0 +1,108 @@ +# Product + +## Register + +product + +## Users + +Behavioral scientists — psychology, cognitive science, linguistics, education — +running online experiments (usually jsPsych) and needing the resulting data to +land somewhere durable and citable without standing up a server. + +They are domain experts, not web developers. Many are graduate students or +postdocs configuring this once for a study that will then run unattended for +weeks. Their technical confidence varies enormously: some write their own +plugins, others are pasting a snippet from a tutorial. + +Two states of mind bring someone to the account settings page: + +- **First-time setup.** A new signup cannot create an experiment until a + storage provider is connected. Settings is a *required step in activation*, + not a maintenance screen — the researcher is here because the product sent + them here, and they are blocked until they finish. +- **Rare maintenance.** An established user returning once every few months to + rotate an expired token, change a password, add a sign-in method, or + disconnect a provider. They have forgotten how this page works. Nothing here + is muscle memory. + +Neither persona visits often. Nothing on this page can rely on recall. + +## Product Purpose + +DataPipe is free, grant-funded infrastructure that accepts data from a running +experiment over a simple HTTP API and writes it to a repository the researcher +controls (Google Drive, Dataverse, Zenodo; OSF historically). It exists so that +"born-open data" is the path of least resistance rather than a project in +itself. + +Success is invisibility: an experiment collects for six weeks and every +participant's data arrives, without the researcher logging in once. The product +is working when nobody thinks about it. + +That inverts the usual attention economics. Because the researcher is almost +never looking, the interface's real job is to make the *consequential* states — +a token about to expire, a provider not connected, a sign-in method that would +lock them out — legible in the few seconds of attention it ever gets. + +## Brand Personality + +**Trustworthy, plain, unfussy.** + +Infrastructure that stays out of the way. Calm and legible; no marketing +energy, no persuasion, no celebration. The voice states what is true and what +will happen next, in the researcher's own vocabulary, without hedging or +jargon. Confidence is expressed through precision and through never losing +data — not through visual assertiveness. + +Emotional goal: quiet certainty. A researcher should leave this interface +believing their data is safe, and should be able to say exactly where it went. + +## Anti-references + +- **SaaS growth-marketing UI.** No gradient hero metrics, upsell nudges, + engagement prompts, confetti, or celebratory language. This is grant-funded + academic infrastructure, not a conversion funnel. Nothing on screen should be + trying to get the researcher to do more of anything. +- **OSF's own interface.** The tool being migrated away from. Do not inherit + its density, deep nesting, or navigational ambiguity. +- **Enterprise admin console.** No sprawling nav trees, role matrices, or dense + configuration tables. One researcher owns one account; the IA should say so. +- **Playful / consumer app.** No mascots, illustrated empty states, emoji, or + animated flourishes. Wrong register for a tool holding irreplaceable research + data. + +## Design Principles + +1. **Assume no recall.** Every visit is effectively a first visit. Labels, + states, and consequences must be readable cold, without memory of a previous + session or of documentation read months ago. +2. **Consequence before mechanism.** Say what will happen to the researcher's + data and access first; explain the machinery second, and only if it helps + them act. "Your experiments stopped sending data" beats "refresh token + expired." +3. **Never strand a researcher.** Destructive or lock-out-adjacent actions — + unlinking a last sign-in method, disconnecting a provider mid-study, + deleting an account — must be prevented or explained in terms of what is + lost, never merely confirmed. +4. **Legibility over density.** A page glanced at twice a year earns its space + by being scannable, not by fitting more in. Status is a first-class citizen, + not a decoration on a row. +5. **Practice what you preach.** DataPipe argues for open, careful data + handling. The interface should visibly embody that care — accurate states, + honest errors, no silent failures. + +## Accessibility & Inclusion + +Target **WCAG 2.1 AA**. + +- Body text ≥4.5:1 against the permanently dark `#1C1F22` surface; large text + and non-text UI boundaries ≥3:1. `lib/theme.js` already re-points the gray + palette for the dark surface with measured ratios — hold that line. +- Status must never be conveyed by color or icon alone; every state needs a + text equivalent. +- Full keyboard operability with a visible focus ring on every interactive + element, including icon-only and link-styled controls. +- Honor `prefers-reduced-motion` for any motion added. +- Users span a wide age range and include international researchers reading + English as a second language; favor plain vocabulary over idiom. diff --git a/__tests__/404.test.jsx b/__tests__/404.test.jsx new file mode 100644 index 0000000..581be29 --- /dev/null +++ b/__tests__/404.test.jsx @@ -0,0 +1,41 @@ +import { render, screen } from "@testing-library/react"; +import { ChakraProvider } from "@chakra-ui/react"; +import { system } from "../lib/theme"; +import "@testing-library/jest-dom"; + +import Custom404 from "../pages/404"; + +// pages/404.js has no getLayout override, so _app.js supplies the Navbar + +// Footer chrome at runtime -- this renders the page component in isolation, +// the same scope index.test.jsx uses for Home. No firebase/context/font +// mocks are needed here: the page imports nothing but Chakra and next/link. +function renderCustom404() { + return render( + + + + ); +} + +describe("Custom404", () => { + it("renders a 'page not found' heading", () => { + renderCustom404(); + expect( + screen.getByRole("heading", { name: /page not found/i }) + ).toBeInTheDocument(); + }); + + it("links back to the homepage", () => { + renderCustom404(); + expect( + screen.getByRole("link", { name: /go to the homepage/i }) + ).toHaveAttribute("href", "/"); + }); + + it("links to the documentation", () => { + renderCustom404(); + expect( + screen.getByRole("link", { name: /documentation/i }) + ).toHaveAttribute("href", "/docs"); + }); +}); diff --git a/__tests__/auth-errors.test.js b/__tests__/auth-errors.test.js new file mode 100644 index 0000000..b8ee4aa --- /dev/null +++ b/__tests__/auth-errors.test.js @@ -0,0 +1,115 @@ +import { + isCancelledAuthError, + messageForAuthError, +} from "../lib/auth-errors"; + +describe("isCancelledAuthError", () => { + it("treats the three popup-dismissal codes as cancellations", () => { + expect(isCancelledAuthError("auth/popup-closed-by-user")).toBe(true); + expect(isCancelledAuthError("auth/cancelled-popup-request")).toBe(true); + expect(isCancelledAuthError("auth/user-cancelled")).toBe(true); + }); + + it("does not swallow real failures", () => { + expect(isCancelledAuthError("auth/email-already-in-use")).toBe(false); + expect(isCancelledAuthError("auth/popup-blocked")).toBe(false); + expect(isCancelledAuthError(undefined)).toBe(false); + }); +}); + +describe("messageForAuthError email collisions", () => { + // The bug this pins: an ORCID account has no email address, so the first + // federated provider a researcher links is the first thing that can collide + // with an account they already had. Linking then fails with + // auth/email-already-in-use, and the sign-in wording told them to "sign in + // using the method you set up originally, then add GitHub from your account + // settings" -- which is precisely what they had just done. + it("does not send a linking researcher back to the account page they are on", () => { + const message = messageForAuthError( + "auth/email-already-in-use", + "GitHub", + "link" + ); + expect(message).not.toMatch(/account settings/i); + expect(message).toMatch(/different DataPipe account/i); + expect(message).toMatch(/GitHub/); + }); + + it("keeps the front-door advice on the sign-in path", () => { + const message = messageForAuthError( + "auth/email-already-in-use", + "Google", + "signIn" + ); + expect(message).toMatch(/account settings/i); + }); + + it("defaults to the sign-in wording when no mode is given", () => { + expect(messageForAuthError("auth/email-already-in-use", "Google")).toBe( + messageForAuthError("auth/email-already-in-use", "Google", "signIn") + ); + }); + + it("gives the same linking advice for the sign-in-only sibling code", () => { + expect( + messageForAuthError( + "auth/account-exists-with-different-credential", + "GitHub", + "link" + ) + ).toBe(messageForAuthError("auth/email-already-in-use", "GitHub", "link")); + }); + + it("says there is no self-service merge rather than implying a retry helps", () => { + const message = messageForAuthError( + "auth/email-already-in-use", + "GitHub", + "link" + ); + expect(message).toMatch(/Contact page/); + expect(message).not.toMatch(/try again/i); + }); +}); + +describe("messageForAuthError fallbacks", () => { + it("names the operation that actually failed", () => { + expect(messageForAuthError("auth/internal-error", "GitHub", "signIn")).toMatch( + /sign-in/ + ); + expect(messageForAuthError("auth/internal-error", "GitHub", "link")).toMatch( + /link your GitHub account/ + ); + expect( + messageForAuthError("auth/internal-error", "GitHub", "unlink") + ).toMatch(/unlink your GitHub account/); + }); + + it("falls back to sign-in wording for an unrecognised mode", () => { + expect(messageForAuthError("auth/internal-error", "GitHub", "wat")).toMatch( + /sign-in/ + ); + }); + + it("has a usable message when the provider name is unknown", () => { + expect(messageForAuthError("auth/internal-error")).toMatch( + /that provider/ + ); + }); +}); + +describe("messageForAuthError mode-independent codes", () => { + it.each([ + ["auth/credential-already-in-use", /already linked to a DataPipe account/], + ["auth/provider-already-linked", /already linked to a DataPipe account/], + ["auth/popup-blocked", /blocked the sign-in window/], + ["auth/operation-not-allowed", /not enabled for DataPipe/], + ["auth/no-such-provider", /not linked to this account/], + ["auth/unauthorized-domain", /not authorized for sign-in/], + ["auth/network-request-failed", /Check your connection/], + ["auth/requires-recent-login", /sign out and sign back in/], + ])("%s reads the same in every mode", (code, pattern) => { + for (const mode of ["signIn", "link", "unlink"]) { + expect(messageForAuthError(code, "GitHub", mode)).toMatch(pattern); + } + }); +}); diff --git a/__tests__/auth-providers.test.js b/__tests__/auth-providers.test.js new file mode 100644 index 0000000..b00e03c --- /dev/null +++ b/__tests__/auth-providers.test.js @@ -0,0 +1,122 @@ +import { + AUTH_PROVIDERS, + AUTH_PROVIDER_LIST, + ORCID_PROVIDER_ID, + PASSWORD_PROVIDER_ID, + canUnlink, + getAuthProviderByProviderId, + linkedProviderIds, +} from "../lib/auth-providers"; +import { AUTH_PROVIDER_ICONS } from "../components/AuthProviderIcons"; + +describe("AUTH_PROVIDERS registry", () => { + it("offers exactly Google, ORCID and GitHub", () => { + expect(Object.keys(AUTH_PROVIDERS).sort()).toEqual([ + "github", + "google", + "orcid", + ]); + }); + + it("does NOT include OSF -- it is being removed as a sign-in method", () => { + expect(AUTH_PROVIDERS.osf).toBeUndefined(); + expect( + AUTH_PROVIDER_LIST.some((entry) => /osf/i.test(entry.providerId)) + ).toBe(false); + }); + + it("maps each entry to the Firebase provider id Firebase itself reports", () => { + expect(AUTH_PROVIDERS.google.providerId).toBe("google.com"); + expect(AUTH_PROVIDERS.github.providerId).toBe("github.com"); + // Firebase requires generic OIDC ids to carry the "oidc." prefix, and + // this string must match the Identity Platform console registration + // exactly or sign-in cannot be routed. + expect(AUTH_PROVIDERS.orcid.providerId).toBe("oidc.orcid"); + expect(ORCID_PROVIDER_ID).toBe("oidc.orcid"); + }); + + it("builds a usable Firebase AuthProvider for every entry", () => { + for (const entry of AUTH_PROVIDER_LIST) { + const provider = entry.makeProvider(); + expect(provider.providerId).toBe(entry.providerId); + } + }); + + it("requests the openid scope for ORCID, which will not issue an id_token without it", () => { + expect(AUTH_PROVIDERS.orcid.makeProvider().getScopes()).toContain("openid"); + }); + + it("requests user:email for GitHub, which otherwise withholds private addresses", () => { + expect(AUTH_PROVIDERS.github.makeProvider().getScopes()).toContain( + "user:email" + ); + }); + + it("flags ORCID as not guaranteeing an email address", () => { + // Researchers routinely keep their ORCID email private, so a successful + // ORCID sign-in can yield user.email === null. Anything that assumes an + // address must consult this rather than assume. + expect(AUTH_PROVIDERS.orcid.providesEmail).toBe(false); + expect(AUTH_PROVIDERS.google.providesEmail).toBe(true); + expect(AUTH_PROVIDERS.github.providesEmail).toBe(true); + }); + + it("has an icon for every entry", () => { + for (const entry of AUTH_PROVIDER_LIST) { + expect(AUTH_PROVIDER_ICONS[entry.id]).toBeDefined(); + } + }); +}); + +describe("getAuthProviderByProviderId", () => { + it("resolves a Firebase provider id back to its registry entry", () => { + expect(getAuthProviderByProviderId("google.com")).toBe( + AUTH_PROVIDERS.google + ); + expect(getAuthProviderByProviderId("oidc.orcid")).toBe(AUTH_PROVIDERS.orcid); + }); + + it("returns null for methods that are not popup providers", () => { + // "password" is a real sign-in method but deliberately not in the + // registry -- mapping over it would render a button that cannot work. + expect(getAuthProviderByProviderId(PASSWORD_PROVIDER_ID)).toBeNull(); + expect(getAuthProviderByProviderId("facebook.com")).toBeNull(); + expect(getAuthProviderByProviderId(undefined)).toBeNull(); + }); +}); + +describe("linkedProviderIds", () => { + it("reads the provider ids off a Firebase user", () => { + expect( + linkedProviderIds({ + providerData: [{ providerId: "google.com" }, { providerId: "password" }], + }) + ).toEqual(["google.com", "password"]); + }); + + it("is empty for an OSF custom-token session and for no user at all", () => { + // This is the exact signal AddSignInMethodBanner keys on: OSF sign-in + // mints a custom token, which carries no federated provider and no + // password, so providerData is empty. Zero linked providers means the + // account is reachable ONLY by the flow that is being removed. + expect(linkedProviderIds({ providerData: [] })).toEqual([]); + expect(linkedProviderIds({})).toEqual([]); + expect(linkedProviderIds(null)).toEqual([]); + }); +}); + +describe("canUnlink", () => { + it("allows unlinking while another method remains", () => { + expect(canUnlink(["google.com", "github.com"], "google.com")).toBe(true); + // A password counts as a way back in, so the federated one can go. + expect(canUnlink(["google.com", "password"], "google.com")).toBe(true); + }); + + it("refuses to unlink the only remaining method", () => { + // Otherwise the researcher is locked out of an account that still owns + // their experiments. + expect(canUnlink(["google.com"], "google.com")).toBe(false); + expect(canUnlink([], "google.com")).toBe(false); + expect(canUnlink(undefined, "google.com")).toBe(false); + }); +}); diff --git a/__tests__/change-password.test.jsx b/__tests__/change-password.test.jsx new file mode 100644 index 0000000..3fbf5f0 --- /dev/null +++ b/__tests__/change-password.test.jsx @@ -0,0 +1,118 @@ +import { render, screen, within, fireEvent, waitFor } from "@testing-library/react"; +import { ChakraProvider } from "@chakra-ui/react"; +import "@testing-library/jest-dom"; +import { system } from "../lib/theme"; + +const mockUpdatePassword = jest.fn(); +jest.mock("firebase/auth", () => ({ + updatePassword: (...args) => mockUpdatePassword(...args), +})); + +jest.mock("../lib/firebase", () => ({ + auth: { currentUser: { uid: "user-1" } }, +})); + +import ChangePassword from "../components/account/ChangePassword"; + +function renderComponent() { + return render( + + + + ); +} + +// Opens the dialog and returns it scoped with `within`. The trigger button +// and the dialog's own submit button share the exact same accessible name +// ("Change Password"), and Chakra hides the background from assistive tech +// asynchronously once the dialog settles -- relying on that timing to +// disambiguate the two is racy, so every query below is scoped to the +// dialog element itself instead. +async function openDialog() { + fireEvent.click(screen.getByRole("button", { name: /^Change Password$/i })); + const dialog = await screen.findByRole("dialog"); + return within(dialog); +} + +beforeEach(() => { + jest.clearAllMocks(); +}); + +// Chakra's Dialog (Ark UI/zag-js underneath) opens and closes through its own +// state machine, which updates asynchronously relative to the click that +// triggers it -- so every interaction with it below is awaited (findBy*) +// rather than asserted on synchronously, same as finalize-control.test.jsx. +describe("ChangePassword", () => { + it("does not show validation errors before the fields are touched", async () => { + renderComponent(); + const dialog = await openDialog(); + + // The critique's exact failure mode: "Password must be at least 12 + // characters" rendering the instant the dialog opens, before a keystroke. + expect(dialog.getByLabelText(/New Password/i)).toBeInTheDocument(); + expect( + dialog.queryByText(/Password must be at least 12 characters/i) + ).not.toBeInTheDocument(); + expect(dialog.queryByText(/Passwords do not match/i)).not.toBeInTheDocument(); + }); + + it("shows the length error only after the new-password field is blurred", async () => { + renderComponent(); + const dialog = await openDialog(); + + const newPassword = dialog.getByLabelText(/New Password/i); + fireEvent.change(newPassword, { target: { value: "short" } }); + expect( + dialog.queryByText(/Password must be at least 12 characters/i) + ).not.toBeInTheDocument(); + + fireEvent.blur(newPassword); + expect( + dialog.getByText(/Password must be at least 12 characters/i) + ).toBeInTheDocument(); + }); + + it("keeps the dialog open and renders the mapped error when updatePassword fails", async () => { + mockUpdatePassword.mockRejectedValue({ code: "auth/requires-recent-login" }); + renderComponent(); + const dialog = await openDialog(); + + fireEvent.change(dialog.getByLabelText(/New Password/i), { + target: { value: "a-long-enough-password" }, + }); + fireEvent.change(dialog.getByLabelText(/Confirm Password/i), { + target: { value: "a-long-enough-password" }, + }); + fireEvent.click(dialog.getByRole("button", { name: /^Change Password$/i })); + + expect( + await dialog.findByText( + /For security, sign out and sign back in, then change your password\./i + ) + ).toBeInTheDocument(); + // Still open: the field is still on screen. + expect(dialog.getByLabelText(/New Password/i)).toBeInTheDocument(); + }); + + it("closes and shows a transient success indicator when updatePassword succeeds", async () => { + mockUpdatePassword.mockResolvedValue(); + renderComponent(); + const dialog = await openDialog(); + + fireEvent.change(dialog.getByLabelText(/New Password/i), { + target: { value: "a-long-enough-password" }, + }); + fireEvent.change(dialog.getByLabelText(/Confirm Password/i), { + target: { value: "a-long-enough-password" }, + }); + fireEvent.click(dialog.getByRole("button", { name: /^Change Password$/i })); + + await waitFor(() => expect(screen.getByText("Success")).toBeInTheDocument()); + // The dialog's own close is a separate async transition from the submit + // -- wait for the field to actually leave the DOM rather than asserting + // synchronously against the exit animation's in-between state. + await waitFor(() => + expect(screen.queryByLabelText(/New Password/i)).not.toBeInTheDocument() + ); + }); +}); diff --git a/__tests__/citation-page.test.jsx b/__tests__/citation-page.test.jsx new file mode 100644 index 0000000..b020eec --- /dev/null +++ b/__tests__/citation-page.test.jsx @@ -0,0 +1,115 @@ +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import { ChakraProvider } from "@chakra-ui/react"; +import { system } from "../lib/theme"; +import "@testing-library/jest-dom"; + +// The page imports DocsLayout for its getLayout, and DocsLayout imports the +// Navbar -- so the module graph reaches firebase, the fonts and the user +// context even though none of them render here. Same three mocks as +// index.test.jsx, for the same reason: a missing export is a module-load +// crash, not a failing assertion. +jest.mock("../lib/firebase", () => ({ + auth: { currentUser: null }, + db: {}, +})); + +jest.mock("next/font/google", () => ({ + Rubik: () => ({ className: "mock-rubik" }), + Space_Grotesk: () => ({ className: "mock-space-grotesk" }), +})); + +jest.mock("../lib/context", () => ({ + UserContext: require("react").createContext({ user: null, loading: false }), +})); + +import CitationPage from "../pages/docs/citation"; +import { CITATION, CITATION_APA, CITATION_BIBTEX } from "../lib/citation"; + +// The page component only, without its DocsLayout getLayout wrapper -- the +// same scope 404.test.jsx uses. The layout brings the sidebar and a router, +// and neither is what this page's behavior lives in. +function renderCitationPage() { + return render( + + + + ); +} + +// jsdom has no clipboard. `navigator.clipboard` is not configurable in every +// jsdom version, so it is defined rather than assigned. +function mockClipboard(writeText) { + Object.defineProperty(navigator, "clipboard", { + value: { writeText }, + configurable: true, + writable: true, + }); +} + +describe("CitationPage", () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("shows the reference in APA and BibTeX", () => { + renderCitationPage(); + + expect(screen.getByRole("heading", { name: "APA" })).toBeInTheDocument(); + expect(screen.getByRole("heading", { name: "BibTeX" })).toBeInTheDocument(); + // The rendered APA reference is assembled from fields rather than from + // CITATION_APA, so this checks the two agree on the part most likely to + // drift if someone edits one of them. + expect(screen.getByText(/2499–2506/)).toBeInTheDocument(); + expect(screen.getByText(/@article\{deleeuw2024datapipe/)).toBeInTheDocument(); + }); + + it("copies the plain-text APA reference", async () => { + const writeText = jest.fn().mockResolvedValue(undefined); + mockClipboard(writeText); + + renderCitationPage(); + fireEvent.click(screen.getByRole("button", { name: /copy apa citation/i })); + + await waitFor(() => expect(writeText).toHaveBeenCalledWith(CITATION_APA)); + // Confirmation is a visible label, not only the icon swap. + await waitFor(() => expect(screen.getByText("Copied")).toBeInTheDocument()); + }); + + it("copies the BibTeX entry", async () => { + const writeText = jest.fn().mockResolvedValue(undefined); + mockClipboard(writeText); + + renderCitationPage(); + fireEvent.click(screen.getByRole("button", { name: /copy code/i })); + + await waitFor(() => expect(writeText).toHaveBeenCalledWith(CITATION_BIBTEX)); + }); + + it("says what to do instead when the clipboard rejects", async () => { + // Insecure origin, or clipboard permission denied. The old unawaited + // write reported success here; the button must not. + const writeText = jest.fn().mockRejectedValue(new Error("denied")); + mockClipboard(writeText); + jest.spyOn(console, "error").mockImplementation(() => {}); + + renderCitationPage(); + fireEvent.click(screen.getByRole("button", { name: /copy apa citation/i })); + + await waitFor(() => + expect(screen.getByText(/could not copy/i)).toBeInTheDocument() + ); + expect(screen.queryByText("Copied")).not.toBeInTheDocument(); + }); + + it("builds the BibTeX entry from the same fields as the APA reference", () => { + // Not a rendering test: the point of lib/citation.js is that one paper + // cannot be two papers. A page range typed twice is exactly how that + // fails, so both formats are checked against the single field. + expect(CITATION_APA).toContain(CITATION.pages); + expect(CITATION_BIBTEX).toContain(CITATION.pages.replace("–", "--")); + expect(CITATION_APA).toContain(CITATION.journal); + expect(CITATION_BIBTEX).toContain(CITATION.journal); + expect(CITATION_APA).toContain(CITATION.doi); + expect(CITATION_BIBTEX).toContain(CITATION.doi); + }); +}); diff --git a/__tests__/confirm-dialog.test.jsx b/__tests__/confirm-dialog.test.jsx new file mode 100644 index 0000000..44dacfe --- /dev/null +++ b/__tests__/confirm-dialog.test.jsx @@ -0,0 +1,108 @@ +import { useState } from "react"; +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import { ChakraProvider } from "@chakra-ui/react"; +import "@testing-library/jest-dom"; +import { system } from "../lib/theme"; +import ConfirmDialog from "../components/ui/ConfirmDialog"; + +// A thin controlled wrapper, the same shape every real caller uses +// (Dialog.Root's open/onOpenChange contract), so the dialog can be opened, +// closed, and reopened the way DeleteAccount/ProviderConnections drive it. +function Harness({ onConfirm, destructive }) { + const [open, setOpen] = useState(true); + return ( + <> + + setOpen(e.open)} + title="Disconnect Google Drive?" + confirmLabel="Disconnect" + destructive={destructive} + onConfirm={onConfirm} + > +

3 experiments are currently sending data to Google Drive.

+ + + ); +} + +function renderDialog(props) { + return render( + + + + ); +} + +describe("ConfirmDialog", () => { + it("renders the title, body, and both actions", () => { + renderDialog({ onConfirm: jest.fn() }); + + expect(screen.getByText("Disconnect Google Drive?")).toBeInTheDocument(); + expect( + screen.getByText(/3 experiments are currently sending data/i) + ).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Cancel" })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Disconnect" }) + ).toBeInTheDocument(); + }); + + it("Cancel is always the neutral outline button, never brandGreen solid", () => { + renderDialog({ onConfirm: jest.fn() }); + const cancel = screen.getByRole("button", { name: "Cancel" }); + // Chakra v3 recipes resolve variant/colorPalette into data attributes + // rather than literal class names, so assert on those rather than on + // computed colors (jsdom does not run the CSS engine). + expect(cancel).not.toHaveAttribute("data-colorPalette", "brandGreen"); + }); + + it("calls onConfirm and closes on success", async () => { + const onConfirm = jest.fn(() => Promise.resolve()); + renderDialog({ onConfirm }); + + fireEvent.click(screen.getByRole("button", { name: "Disconnect" })); + + await waitFor(() => expect(onConfirm).toHaveBeenCalledTimes(1)); + await waitFor(() => + expect(screen.queryByText("Disconnect Google Drive?")).not.toBeInTheDocument() + ); + }); + + it("keeps the dialog open and shows error.message when onConfirm throws", async () => { + const onConfirm = jest.fn(() => + Promise.reject(new Error("Could not reach DataPipe. Check your connection and try again.")) + ); + renderDialog({ onConfirm }); + + fireEvent.click(screen.getByRole("button", { name: "Disconnect" })); + + expect( + await screen.findByText(/Could not reach DataPipe/i) + ).toBeInTheDocument(); + // Still open: the title and Cancel button are still on screen. + expect(screen.getByText("Disconnect Google Drive?")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Cancel" })).toBeInTheDocument(); + }); + + it("clears a previous error when reopened", async () => { + const onConfirm = jest.fn(() => Promise.reject(new Error("Something went wrong."))); + renderDialog({ onConfirm }); + + fireEvent.click(screen.getByRole("button", { name: "Disconnect" })); + await screen.findByText("Something went wrong."); + + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + await waitFor(() => + expect(screen.queryByText("Disconnect Google Drive?")).not.toBeInTheDocument() + ); + + fireEvent.click(screen.getByRole("button", { name: "Reopen" })); + // Chakra's Dialog (Ark UI/zag-js underneath) opens through its own state + // machine, asynchronously relative to the click that triggers it, so + // this is awaited like any other async UI update. + expect(await screen.findByText("Disconnect Google Drive?")).toBeInTheDocument(); + expect(screen.queryByText("Something went wrong.")).not.toBeInTheDocument(); + }); +}); diff --git a/__tests__/connect-callback-page.test.jsx b/__tests__/connect-callback-page.test.jsx new file mode 100644 index 0000000..fb15b69 --- /dev/null +++ b/__tests__/connect-callback-page.test.jsx @@ -0,0 +1,150 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import { ChakraProvider } from "@chakra-ui/react"; +import { system } from "../lib/theme"; +import "@testing-library/jest-dom"; + +const mockGetIdToken = jest.fn(() => Promise.resolve("id-token-123")); +jest.mock("../lib/firebase", () => ({ + auth: { currentUser: { uid: "user-1", getIdToken: () => mockGetIdToken() } }, + db: {}, +})); + +// UserContext is re-provided per test via +// so signed-in vs signed-out can vary within this file without re-mocking +// the module. +jest.mock("../lib/context", () => ({ + UserContext: require("react").createContext({ user: null, loading: false }), +})); + +const mockPush = jest.fn(); +let mockQuery = {}; +jest.mock("next/router", () => ({ + useRouter: () => ({ query: mockQuery, push: mockPush, isReady: true }), +})); + +import { UserContext } from "../lib/context"; +import ConnectCallbackPage from "../pages/oauth2/connect"; + +function renderPage({ user = { uid: "user-1" } } = {}) { + return render( + + + + + + ); +} + +beforeEach(() => { + jest.clearAllMocks(); + mockGetIdToken.mockClear(); + mockGetIdToken.mockImplementation(() => Promise.resolve("id-token-123")); + global.fetch = jest.fn(); + localStorage.clear(); + mockQuery = {}; +}); + +describe("oauth2/connect callback page", () => { + it("9. happy path posts to connectprovider and routes to /admin/account on success", async () => { + mockQuery = { code: "auth-code-1", state: "state-abc" }; + localStorage.setItem("latestCSRFToken", "state-abc"); + localStorage.setItem("providerConnectFlow", "gdrive"); + global.fetch.mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ success: true }), + }); + + renderPage(); + + await waitFor(() => expect(global.fetch).toHaveBeenCalled()); + const [url, options] = global.fetch.mock.calls[0]; + expect(url).toBe("/api/connectprovider"); + expect(JSON.parse(options.body)).toEqual({ + provider: "gdrive", + code: "auth-code-1", + state: "state-abc", + uid: "user-1", + idToken: "id-token-123", + }); + + // `?connected=gdrive` is the signal ProviderConnections.js + // (components/account/ProviderConnections.js) reads to show its "Linked + // Google Drive successfully." banner -- this page unmounts entirely on + // the redirect round trip, so a query param on the destination URL is + // the only way that information survives the trip. + await waitFor(() => + expect(mockPush).toHaveBeenCalledWith("/admin/account?connected=gdrive") + ); + }); + + it("10. state mismatch shows error UI and does not call connectprovider", async () => { + mockQuery = { code: "auth-code-1", state: "state-abc" }; + localStorage.setItem("latestCSRFToken", "different-state"); + localStorage.setItem("providerConnectFlow", "gdrive"); + global.fetch.mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ success: true }), + }); + + renderPage(); + + // Copy changed from the raw "Invalid state parameter. Possible CSRF + // attack." to a calm, actionable message (DESIGN.md / PRODUCT.md + // "consequence before mechanism") -- assert on the new wording rather + // than the old machine text. + await waitFor(() => + expect( + screen.getByText(/expired|out of order/i) + ).toBeInTheDocument() + ); + expect( + screen.getByRole("link", { name: /admin.*account|account/i }) + ).toHaveAttribute("href", "/admin/account"); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("11. signed-out user shows error UI with a sign-in link and does not call connectprovider", async () => { + mockQuery = { code: "auth-code-1", state: "state-abc" }; + localStorage.setItem("latestCSRFToken", "state-abc"); + localStorage.setItem("providerConnectFlow", "gdrive"); + global.fetch.mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ success: true }), + }); + + renderPage({ user: null }); + + await waitFor(() => + expect( + screen.getByRole("link", { name: /sign in/i }) + ).toBeInTheDocument() + ); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("12. a provider's access_denied is a neutral cancellation, not an error alert", async () => { + mockQuery = { error: "access_denied" }; + + renderPage(); + + await waitFor(() => + expect(screen.getByText(/cancelled the connection/i)).toBeInTheDocument() + ); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("13. opening the page with no code/state shows an error with a way back instead of spinning forever", async () => { + mockQuery = {}; + + renderPage(); + + await waitFor(() => + expect( + screen.getByText(/start the connection again/i) + ).toBeInTheDocument() + ); + expect( + screen.getByRole("link", { name: /account/i }) + ).toHaveAttribute("href", "/admin/account"); + }); +}); diff --git a/__tests__/contact-email-gate.test.jsx b/__tests__/contact-email-gate.test.jsx new file mode 100644 index 0000000..a48fa2f --- /dev/null +++ b/__tests__/contact-email-gate.test.jsx @@ -0,0 +1,204 @@ +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import { ChakraProvider } from "@chakra-ui/react"; +import { system } from "../lib/theme"; +import "@testing-library/jest-dom"; + +// Same mocking shape as __tests__/provider-connections.test.jsx: mock +// ../lib/firebase, ../lib/context, firebase/firestore, and +// react-firebase-hooks/firestore, then wrap in ChakraProvider. +jest.mock("../lib/firebase", () => ({ + auth: { currentUser: { uid: "user-1" } }, + db: {}, +})); + +const mockUser = { uid: "user-1", email: "researcher@example.edu" }; +jest.mock("../lib/context", () => ({ + UserContext: require("react").createContext({ user: null, loading: true }), +})); + +const mockSetDoc = jest.fn(() => Promise.resolve()); +jest.mock("firebase/firestore", () => ({ + doc: jest.fn(() => ({})), + setDoc: (...args) => mockSetDoc(...args), +})); + +jest.mock("react-firebase-hooks/firestore", () => ({ + useDocumentData: jest.fn(), +})); + +// AuthCheck reads useRouter for the pathname passed to SignInForm and an +// effect that pushes fallbackRoute when signed out -- neither path is +// exercised by these tests (every case here is signed in), but the module +// still has to resolve. +jest.mock("next/router", () => ({ + __esModule: true, + default: { push: jest.fn() }, + useRouter: () => ({ push: jest.fn(), pathname: "/admin" }), +})); + +import { UserContext } from "../lib/context"; +import { useDocumentData } from "react-firebase-hooks/firestore"; +import AuthCheck from "../components/AuthCheck"; + +function renderGated(userValue = { user: mockUser, loading: false }) { + return render( + + + +
Protected dashboard content
+ + + + ); +} + +beforeEach(() => { + jest.clearAllMocks(); + mockSetDoc.mockClear(); + mockSetDoc.mockResolvedValue(); +}); + +describe("AuthCheck's contact-email gate", () => { + it("users/{uid} doc with no contactEmail: gate copy renders, children absent", () => { + useDocumentData.mockReturnValue([{ email: "" }, false, undefined]); + + renderGated(); + + expect(screen.getByText("Add an email address")).toBeInTheDocument(); + expect( + screen.getByText(/DataPipe needs a way to reach you/i) + ).toBeInTheDocument(); + expect( + screen.queryByText("Protected dashboard content") + ).not.toBeInTheDocument(); + }); + + it("users/{uid} doc with a valid contactEmail: children render, gate absent", () => { + useDocumentData.mockReturnValue([ + { contactEmail: "lab@example.edu", contactEmailVerified: false }, + false, + undefined, + ]); + + renderGated(); + + expect(screen.getByText("Protected dashboard content")).toBeInTheDocument(); + expect(screen.queryByText("Add an email address")).not.toBeInTheDocument(); + }); + + it("synthetic OSF fallback address does not satisfy the gate", () => { + // The case a naive truthiness check gets wrong -- oauth2-callback.ts + // seeds this exact shape as a fallback, and it is undeliverable. + useDocumentData.mockReturnValue([ + { contactEmail: "user-abc12@osf.io", email: "user-abc12@osf.io" }, + false, + undefined, + ]); + + renderGated(); + + expect(screen.getByText("Add an email address")).toBeInTheDocument(); + expect( + screen.queryByText("Protected dashboard content") + ).not.toBeInTheDocument(); + }); + + it("loading users/{uid}: neither the gate nor the children render", () => { + useDocumentData.mockReturnValue([undefined, true, undefined]); + + renderGated(); + + expect(screen.queryByText("Add an email address")).not.toBeInTheDocument(); + expect( + screen.queryByText("Protected dashboard content") + ).not.toBeInTheDocument(); + }); + + it("a users/{uid} read error fails OPEN: children render, not the gate", () => { + useDocumentData.mockReturnValue([ + undefined, + false, + new Error("permission-denied"), + ]); + + renderGated(); + + expect(screen.getByText("Protected dashboard content")).toBeInTheDocument(); + expect(screen.queryByText("Add an email address")).not.toBeInTheDocument(); + }); + + it("malformed input shows a field error and never calls setDoc", () => { + useDocumentData.mockReturnValue([{ email: "" }, false, undefined]); + + renderGated(); + + fireEvent.change(screen.getByLabelText(/Email address/i), { + target: { value: "not-an-email" }, + }); + fireEvent.click(screen.getByRole("button", { name: /Save and continue/i })); + + expect(screen.getByText(/Enter a valid email address/i)).toBeInTheDocument(); + expect(mockSetDoc).not.toHaveBeenCalled(); + }); + + it("a valid submit writes exactly the four allowed keys, contactEmailVerified false", async () => { + useDocumentData.mockReturnValue([{ email: "" }, false, undefined]); + + renderGated(); + + fireEvent.change(screen.getByLabelText(/Email address/i), { + target: { value: "researcher@example.edu" }, + }); + fireEvent.click(screen.getByRole("button", { name: /Save and continue/i })); + + await waitFor(() => expect(mockSetDoc).toHaveBeenCalled()); + + const [, payload, options] = mockSetDoc.mock.calls[0]; + expect(Object.keys(payload).sort()).toEqual( + [ + "contactEmail", + "contactEmailVerified", + "contactEmailUpdatedAt", + "contactEmailSource", + ].sort() + ); + expect(payload.contactEmail).toBe("researcher@example.edu"); + expect(payload.contactEmailVerified).toBe(false); + expect(options).toEqual({ merge: true }); + }); + + it("has no control labelled Skip or Later -- there is no skip", () => { + useDocumentData.mockReturnValue([{ email: "" }, false, undefined]); + + renderGated(); + + expect(screen.queryByText(/^Skip$/i)).not.toBeInTheDocument(); + expect(screen.queryByText(/^Later$/i)).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /skip/i }) + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /later/i }) + ).not.toBeInTheDocument(); + }); + + it("/admin/account's requireContactEmail={false} bypasses the gate entirely", () => { + // Doc has no contactEmail at all -- would trip the gate on any other + // route -- but requireContactEmail={false} must render children + // unconditionally, without even subscribing to useDocumentData. + useDocumentData.mockReturnValue([undefined, false, undefined]); + + render( + + + +
Account settings content
+ + + + ); + + expect(screen.getByText("Account settings content")).toBeInTheDocument(); + expect(screen.queryByText("Add an email address")).not.toBeInTheDocument(); + }); +}); diff --git a/__tests__/contact-email-section.test.jsx b/__tests__/contact-email-section.test.jsx new file mode 100644 index 0000000..d8e5cfe --- /dev/null +++ b/__tests__/contact-email-section.test.jsx @@ -0,0 +1,260 @@ +import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import { ChakraProvider } from "@chakra-ui/react"; +import { system } from "../lib/theme"; +import "@testing-library/jest-dom"; + +// Same mocking shape as __tests__/provider-connections.test.jsx: mock +// ../lib/firebase (auth.currentUser.getIdToken, matching DeleteAccount.js / +// FinalizeControl.js's fetch-with-bearer-token idiom, NOT a Firestore write -- +// this file only calls the two P3 HTTP endpoints), ../lib/context (for +// user.uid, used only by the P1 edit-form's setDoc call), and firebase/firestore. +const mockGetIdToken = jest.fn(() => Promise.resolve("id-token-123")); +jest.mock("../lib/firebase", () => ({ + auth: { currentUser: { uid: "user-1", getIdToken: () => mockGetIdToken() } }, + db: {}, +})); + +jest.mock("../lib/context", () => ({ + UserContext: require("react").createContext({ user: { uid: "user-1" }, loading: false }), +})); + +const mockSetDoc = jest.fn(() => Promise.resolve()); +jest.mock("firebase/firestore", () => ({ + doc: jest.fn(() => ({})), + setDoc: (...args) => mockSetDoc(...args), +})); + +import ContactEmail from "../components/account/ContactEmail"; + +function renderComponent(data) { + return render( + + + + ); +} + +beforeEach(() => { + jest.clearAllMocks(); + mockGetIdToken.mockClear(); + mockGetIdToken.mockImplementation(() => Promise.resolve("id-token-123")); + mockSetDoc.mockClear(); + mockSetDoc.mockResolvedValue(); + global.fetch = jest.fn(); +}); + +describe("ContactEmail — states", () => { + it("verified: shows StatusIndicator ok/Confirmed, no Verify or Resend affordance", () => { + renderComponent({ contactEmail: "researcher@example.edu", contactEmailVerified: true }); + + expect(screen.getByText("Confirmed")).toBeInTheDocument(); + expect(screen.queryByText("Not confirmed yet")).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /^Verify$/i })).not.toBeInTheDocument(); + }); + + it("unverified: shows StatusIndicator neutral/Not confirmed yet plus a Verify action", () => { + renderComponent({ contactEmail: "researcher@example.edu", contactEmailVerified: false }); + + expect(screen.getByText("Not confirmed yet")).toBeInTheDocument(); + expect(screen.queryByText("Confirmed")).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: /^Verify$/i })).toBeInTheDocument(); + }); + + it("no address on file: neither Confirmed, Not confirmed yet, nor Verify render", () => { + renderComponent({ contactEmail: "" }); + + expect(screen.getByText("No address on file")).toBeInTheDocument(); + expect(screen.queryByText("Not confirmed yet")).not.toBeInTheDocument(); + expect(screen.queryByText("Confirmed")).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /^Verify$/i })).not.toBeInTheDocument(); + }); +}); + +describe("ContactEmail — sending a code", () => { + it("Verify calls sendcontactemailverification with the bearer token, then opens the code form", async () => { + global.fetch.mockResolvedValue({ ok: true, json: () => Promise.resolve({ success: true }) }); + + renderComponent({ contactEmail: "researcher@example.edu", contactEmailVerified: false }); + fireEvent.click(screen.getByRole("button", { name: /^Verify$/i })); + + await waitFor(() => expect(global.fetch).toHaveBeenCalled()); + const [url, options] = global.fetch.mock.calls[0]; + expect(url).toBe("/api/sendcontactemailverification"); + expect(options.method).toBe("POST"); + expect(options.headers.Authorization).toBe("Bearer id-token-123"); + expect(mockGetIdToken).toHaveBeenCalled(); + + expect(await screen.findByLabelText(/Verification code/i)).toBeInTheDocument(); + expect( + screen.getByText(/Enter the 6-digit code we sent to researcher@example\.edu/i) + ).toBeInTheDocument(); + // The Verify button that triggered the send is gone once the form is + // open -- Resend (inside the form) replaces it. + expect(screen.queryByRole("button", { name: /^Verify$/i })).not.toBeInTheDocument(); + }); + + it("a send failure (e.g. no contact email) is shown via FormErrorAlert, in human copy, and does not open the form", async () => { + global.fetch.mockResolvedValue({ + ok: false, + json: () => + Promise.resolve({ + error: "Add a contact email address before requesting a code.", + code: "no-contact-email", + }), + }); + + renderComponent({ contactEmail: "researcher@example.edu", contactEmailVerified: false }); + fireEvent.click(screen.getByRole("button", { name: /^Verify$/i })); + + expect( + await screen.findByText(/Add a contact email address before requesting a code/i) + ).toBeInTheDocument(); + // The raw machine code must never reach the page. + expect(screen.queryByText(/no-contact-email/i)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/Verification code/i)).not.toBeInTheDocument(); + }); + + it("a rate-limited resend still opens the code form -- a code is already out there and usable", async () => { + global.fetch.mockResolvedValue({ + ok: false, + status: 429, + json: () => + Promise.resolve({ + error: "Please wait a moment before requesting another code.", + code: "rate-limited", + }), + }); + + renderComponent({ contactEmail: "researcher@example.edu", contactEmailVerified: false }); + fireEvent.click(screen.getByRole("button", { name: /^Verify$/i })); + + expect(await screen.findByLabelText(/Verification code/i)).toBeInTheDocument(); + expect( + screen.getByText(/Please wait a moment before requesting another code/i) + ).toBeInTheDocument(); + }); +}); + +describe("ContactEmail — entering a code", () => { + async function openCodeForm() { + global.fetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ success: true }), + }); + renderComponent({ contactEmail: "researcher@example.edu", contactEmailVerified: false }); + fireEvent.click(screen.getByRole("button", { name: /^Verify$/i })); + await screen.findByLabelText(/Verification code/i); + } + + it("Confirm calls verifycontactemail with the bearer token and the entered code", async () => { + await openCodeForm(); + global.fetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ success: true }), + }); + + fireEvent.change(screen.getByLabelText(/Verification code/i), { + target: { value: "482913" }, + }); + fireEvent.click(screen.getByRole("button", { name: /^Confirm$/i })); + + await waitFor(() => expect(global.fetch).toHaveBeenCalledTimes(2)); + const [url, options] = global.fetch.mock.calls[1]; + expect(url).toBe("/api/verifycontactemail"); + expect(options.method).toBe("POST"); + expect(options.headers.Authorization).toBe("Bearer id-token-123"); + expect(JSON.parse(options.body)).toEqual({ code: "482913" }); + + // Success closes the form; contactEmailVerified itself flips only via + // the parent's live users/{uid} subscription (a prop update this test + // does not simulate), so the form closing is the observable effect here. + await waitFor(() => + expect(screen.queryByLabelText(/Verification code/i)).not.toBeInTheDocument() + ); + }); + + it("non-digit input is stripped and the field is capped at 6 characters", async () => { + await openCodeForm(); + + const input = screen.getByLabelText(/Verification code/i); + fireEvent.change(input, { target: { value: "4a8b2c913x" } }); + + expect(input.value).toBe("482913"); + }); + + it("a wrong-code failure surfaces through FormErrorAlert with human copy, never the raw error code", async () => { + await openCodeForm(); + global.fetch.mockResolvedValueOnce({ + ok: false, + json: () => Promise.resolve({ error: "That code is incorrect.", code: "invalid-code" }), + }); + + fireEvent.change(screen.getByLabelText(/Verification code/i), { + target: { value: "000001" }, + }); + fireEvent.click(screen.getByRole("button", { name: /^Confirm$/i })); + + expect(await screen.findByText(/That code is incorrect/i)).toBeInTheDocument(); + expect(screen.queryByText(/invalid-code/i)).not.toBeInTheDocument(); + // Stays open so the researcher can retry without re-requesting a code. + expect(screen.getByLabelText(/Verification code/i)).toBeInTheDocument(); + }); + + it("an expired-code failure maps to its own human sentence", async () => { + await openCodeForm(); + global.fetch.mockResolvedValueOnce({ + ok: false, + json: () => Promise.resolve({ error: "That code has expired.", code: "expired" }), + }); + + fireEvent.change(screen.getByLabelText(/Verification code/i), { + target: { value: "000001" }, + }); + fireEvent.click(screen.getByRole("button", { name: /^Confirm$/i })); + + expect(await screen.findByText(/expired. Request a new one/i)).toBeInTheDocument(); + }); + + it("submitting fewer than 6 digits is refused client-side, with no request sent", async () => { + await openCodeForm(); + global.fetch.mockClear(); + + fireEvent.change(screen.getByLabelText(/Verification code/i), { + target: { value: "42" }, + }); + fireEvent.click(screen.getByRole("button", { name: /^Confirm$/i })); + + // Exact string: the helper line says "Enter the 6-digit code we sent + // to..." so a loose regex would double-match. + expect(await screen.findByText("Enter the 6-digit code.")).toBeInTheDocument(); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("Resend re-calls sendcontactemailverification and clears the entered code", async () => { + await openCodeForm(); + fireEvent.change(screen.getByLabelText(/Verification code/i), { + target: { value: "111111" }, + }); + global.fetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve({ success: true }), + }); + + fireEvent.click(screen.getByRole("button", { name: /^Resend$/i })); + + await waitFor(() => expect(global.fetch).toHaveBeenCalledTimes(2)); + expect(global.fetch.mock.calls[1][0]).toBe("/api/sendcontactemailverification"); + await waitFor(() => expect(screen.getByLabelText(/Verification code/i).value).toBe("")); + }); + + it("Cancel closes the form without calling verify", async () => { + await openCodeForm(); + global.fetch.mockClear(); + + fireEvent.click(screen.getByRole("button", { name: /^Cancel$/i })); + + expect(screen.queryByLabelText(/Verification code/i)).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: /^Verify$/i })).toBeInTheDocument(); + expect(global.fetch).not.toHaveBeenCalled(); + }); +}); diff --git a/__tests__/experiment-creation.test.js b/__tests__/experiment-creation.test.js new file mode 100644 index 0000000..aca5c6e --- /dev/null +++ b/__tests__/experiment-creation.test.js @@ -0,0 +1,182 @@ +// nanoid v5 ships ESM-only and isn't transformed by the default Jest config +// (node_modules is excluded); experiment-creation.js imports it at the top +// level for the (untested-here) OSF path, so it must be mocked even though +// createProviderExperiment itself never calls it. +jest.mock("nanoid", () => ({ + customAlphabet: () => () => "mocked-id", +})); + +jest.mock("../lib/firebase", () => ({ + auth: { currentUser: null }, + db: {}, +})); + +// firebase/firestore's doc/writeBatch/arrayUnion (used by the untested-here +// OSF path in this module) must not touch a real Firestore instance. +jest.mock("firebase/firestore", () => ({ + doc: jest.fn(() => ({})), + writeBatch: jest.fn(() => ({ + set: jest.fn(), + update: jest.fn(), + commit: jest.fn(() => Promise.resolve()), + })), + arrayUnion: jest.fn((v) => v), +})); + +import { createProviderExperiment } from "../lib/experiment-creation"; +import { auth } from "../lib/firebase"; + +function mockUser({ uid = "user-123", idToken = "id-token-abc" } = {}) { + return { + uid, + getIdToken: jest.fn().mockResolvedValue(idToken), + }; +} + +describe("createProviderExperiment", () => { + beforeEach(() => { + global.fetch = jest.fn(); + }); + + afterEach(() => { + jest.resetAllMocks(); + auth.currentUser = null; + }); + + it("omits parentFolderId from the request body when not provided", async () => { + auth.currentUser = mockUser(); + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ experimentID: "exp-1" }), + }); + + const result = await createProviderExperiment("gdrive", "My Experiment"); + + expect(global.fetch).toHaveBeenCalledTimes(1); + const [url, options] = global.fetch.mock.calls[0]; + expect(url).toBe("/api/createexperiment"); + const body = JSON.parse(options.body); + expect(body).toEqual({ + provider: "gdrive", + title: "My Experiment", + uid: "user-123", + idToken: "id-token-abc", + }); + expect(body.parentFolderId).toBeUndefined(); + expect(result).toEqual({ experimentId: "exp-1" }); + }); + + it("forwards parentFolderId in the request body when provided", async () => { + auth.currentUser = mockUser(); + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ experimentID: "exp-2" }), + }); + + const result = await createProviderExperiment( + "gdrive", + "My Experiment", + "folder-xyz" + ); + + const [, options] = global.fetch.mock.calls[0]; + const body = JSON.parse(options.body); + expect(body.parentFolderId).toBe("folder-xyz"); + expect(body).toEqual({ + provider: "gdrive", + title: "My Experiment", + uid: "user-123", + idToken: "id-token-abc", + parentFolderId: "folder-xyz", + }); + expect(result).toEqual({ experimentId: "exp-2" }); + }); + + it("omits parentFolderId when it is falsy (empty string)", async () => { + auth.currentUser = mockUser(); + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ experimentID: "exp-3" }), + }); + + await createProviderExperiment("gdrive", "My Experiment", ""); + + const [, options] = global.fetch.mock.calls[0]; + const body = JSON.parse(options.body); + expect(body.parentFolderId).toBeUndefined(); + }); + + it("throws when the user is not authenticated", async () => { + auth.currentUser = null; + + await expect( + createProviderExperiment("gdrive", "My Experiment") + ).rejects.toThrow("User not authenticated"); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("throws the server error message when the request fails", async () => { + auth.currentUser = mockUser(); + global.fetch.mockResolvedValue({ + ok: false, + status: 403, + json: async () => ({ error: "Forbidden" }), + }); + + await expect( + createProviderExperiment("gdrive", "My Experiment") + ).rejects.toThrow("Forbidden"); + }); + + it("forwards researcherInput in the request body when provided", async () => { + auth.currentUser = mockUser(); + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ experimentID: "exp-4" }), + }); + + const researcherInput = { + collectionAlias: "my-lab", + authorName: "Smith, Jane", + contactEmail: "jane@example.edu", + description: "A study about things", + }; + + const result = await createProviderExperiment( + "dataverse", + "My Experiment", + undefined, + researcherInput + ); + + const [, options] = global.fetch.mock.calls[0]; + const body = JSON.parse(options.body); + expect(body.researcherInput).toEqual(researcherInput); + expect(body).toEqual({ + provider: "dataverse", + title: "My Experiment", + uid: "user-123", + idToken: "id-token-abc", + researcherInput, + }); + expect(result).toEqual({ experimentId: "exp-4" }); + }); + + it("omits researcherInput from the request body when empty or undefined", async () => { + auth.currentUser = mockUser(); + global.fetch.mockResolvedValue({ + ok: true, + json: async () => ({ experimentID: "exp-5" }), + }); + + await createProviderExperiment("gdrive", "My Experiment", undefined, undefined); + let [, options] = global.fetch.mock.calls[0]; + let body = JSON.parse(options.body); + expect(body.researcherInput).toBeUndefined(); + + await createProviderExperiment("gdrive", "My Experiment", undefined, {}); + [, options] = global.fetch.mock.calls[1]; + body = JSON.parse(options.body); + expect(body.researcherInput).toBeUndefined(); + }); +}); diff --git a/__tests__/experiment-info.test.jsx b/__tests__/experiment-info.test.jsx new file mode 100644 index 0000000..dc17a31 --- /dev/null +++ b/__tests__/experiment-info.test.jsx @@ -0,0 +1,53 @@ +import { render, screen } from "@testing-library/react"; +import { ChakraProvider } from "@chakra-ui/react"; +import { system } from "../lib/theme"; +import "@testing-library/jest-dom"; + +import ExperimentInfo from "../components/dashboard/ExperimentInfo"; + +function renderInfo(data) { + return render( + + + + ); +} + +describe("ExperimentInfo — legacy OSF experiments (pinned regression)", () => { + it("12. renders OSF Project and OSF Data Component links for legacy experiments", () => { + renderInfo({ + id: "exp1", + osfRepo: "abc12", + osfComponent: "def34", + sessions: 3, + }); + + // Sentence case per DESIGN.md §3 -- these labels were "OSF Project" and + // "OSF Data Component" before the dashboard conversion. + expect(screen.getByText("OSF project")).toBeInTheDocument(); + expect(screen.getByText("OSF data component")).toBeInTheDocument(); + expect(screen.getByRole("link", { name: /abc12/ })).toBeInTheDocument(); + expect(screen.getByRole("link", { name: /def34/ })).toBeInTheDocument(); + }); +}); + +describe("ExperimentInfo — provider-aware rendering", () => { + it("13. renders Google Drive Folder link for gdrive experiments; OSF labels are absent", () => { + renderInfo({ + id: "exp2", + storageProvider: "gdrive", + providerContainer: { folderId: "folder123" }, + sessions: 5, + }); + + expect(screen.getByText("Google Drive Folder")).toBeInTheDocument(); + const link = screen.getByRole("link", { name: /Open folder/ }); + expect(link).toHaveAttribute( + "href", + "https://drive.google.com/drive/folders/folder123" + ); + + expect(screen.queryByText("OSF project")).not.toBeInTheDocument(); + expect(screen.queryByText("OSF data component")).not.toBeInTheDocument(); + }); +}); diff --git a/__tests__/experiment-validation.test.jsx b/__tests__/experiment-validation.test.jsx new file mode 100644 index 0000000..417a8f1 --- /dev/null +++ b/__tests__/experiment-validation.test.jsx @@ -0,0 +1,290 @@ +import { + act, + render, + screen, + fireEvent, + waitFor, +} from "@testing-library/react"; +import { ChakraProvider } from "@chakra-ui/react"; +import { system } from "../lib/theme"; +import "@testing-library/jest-dom"; + +import ExperimentValidation from "../components/dashboard/ExperimentValidation"; + +const setDocMock = jest.fn(() => Promise.resolve()); +jest.mock("firebase/firestore", () => ({ + setDoc: (...args) => setDocMock(...args), + doc: jest.fn(() => ({})), +})); +jest.mock("../lib/firebase", () => ({ db: {} })); + +function renderWithChakra(ui) { + return render({ui}); +} + +const experiment = (overrides = {}) => ({ + id: "exp-1", + useValidation: true, + allowJSON: true, + allowCSV: true, + requiredFields: ["trial_type"], + ...overrides, +}); + +const focusFieldInput = async () => { + const input = screen.getByRole("textbox"); + await act(async () => { + input.focus(); + }); + return input; +}; + +const typeAndCommit = (input, text) => { + fireEvent.input(input, { + target: { value: text }, + inputType: "insertText", + }); + fireEvent.keyDown(input, { key: "Enter" }); +}; + +beforeEach(() => { + setDocMock.mockClear(); +}); + +describe("ExperimentValidation — required fields", () => { + it("shows the stored list as pills", () => { + renderWithChakra( + + ); + expect(screen.getByText("trial_type")).toBeInTheDocument(); + expect(screen.getByText("rt")).toBeInTheDocument(); + }); + + it("does not write on mount", async () => { + renderWithChakra(); + await act(async () => {}); + expect(setDocMock).not.toHaveBeenCalled(); + }); + + it("saves the list when a field is added", async () => { + renderWithChakra(); + + typeAndCommit(await focusFieldInput(), "rt"); + + await waitFor(() => expect(setDocMock).toHaveBeenCalledTimes(1)); + expect(setDocMock.mock.calls[0][1]).toEqual({ + allowJSON: true, + allowCSV: true, + requiredFields: ["trial_type", "rt"], + }); + }); + + it("saves the list when a field is removed", async () => { + renderWithChakra( + + ); + + fireEvent.click( + screen.getByRole("button", { name: /remove field trial_type/i }) + ); + + await waitFor(() => expect(setDocMock).toHaveBeenCalledTimes(1)); + expect(setDocMock.mock.calls[0][1].requiredFields).toEqual(["rt"]); + }); + + it("saves the trimmed name, not what was typed", async () => { + // The whole point. `" rt "` in the document is a field no submission has, + // and the resulting 400 does not say so. + renderWithChakra( + + ); + + typeAndCommit(await focusFieldInput(), " rt "); + + await waitFor(() => expect(setDocMock).toHaveBeenCalledTimes(1)); + expect(setDocMock.mock.calls[0][1].requiredFields).toEqual(["rt"]); + }); + + it("does not write when a duplicate is refused", async () => { + renderWithChakra(); + + typeAndCommit(await focusFieldInput(), "trial_type"); + await act(async () => {}); + + expect(setDocMock).not.toHaveBeenCalled(); + expect(screen.getByText(/already in the list/i)).toBeInTheDocument(); + }); + + it("draws no pill for a legacy [\"\"] document", () => { + // Documents written by the old textarea can hold a single empty string. An + // empty pill has nothing to draw, so it would be a chip the researcher can + // neither see nor delete. + const { container } = renderWithChakra( + + ); + expect( + container.querySelectorAll("[data-part='item-preview']") + ).toHaveLength(0); + }); + + it("reverts the list and explains when the write fails", async () => { + setDocMock.mockImplementationOnce(() => Promise.reject(new Error("nope"))); + const consoleError = jest + .spyOn(console, "error") + .mockImplementation(() => {}); + + renderWithChakra(); + + typeAndCommit(await focusFieldInput(), "rt"); + + await waitFor(() => + expect(screen.getByText(/could not save your validation rules/i)) + .toBeInTheDocument() + ); + // The pill has to go with the message: a list that still shows `rt` while + // Firestore does not hold it is the lie SettingsRow exists to prevent. + expect(screen.queryByText("rt")).not.toBeInTheDocument(); + expect(screen.getByText("trial_type")).toBeInTheDocument(); + + consoleError.mockRestore(); + }); + + it("hides the required-fields control when validation is off", () => { + renderWithChakra( + + ); + expect(screen.queryByText("Required fields")).not.toBeInTheDocument(); + }); +}); + +const masterSwitch = () => + screen.getByRole("checkbox", { name: /Check submissions before storing/i }); + +// Regression: the format checkboxes render as SettingsRow's `children`, and +// those children used to sit inside the row's `Field.Root`. Ark hands every +// Switch AND Checkbox in a Field the same hidden-input id +// (`ids: { hiddenInput: field?.ids.control }`), and `Checkbox.Root` is itself +// a `