diff --git a/.changeset/bible-reader-highlights-api.md b/.changeset/bible-reader-highlights-api.md new file mode 100644 index 00000000..2fbeb750 --- /dev/null +++ b/.changeset/bible-reader-highlights-api.md @@ -0,0 +1,15 @@ +--- +'@youversion/platform-react-hooks': patch +'@youversion/platform-react-ui': patch +--- + +BibleReader highlights now wire to the highlights API behind an internal dark-launch flag; the localStorage highlight store is removed. + +- Highlights are server-only account data (YPE-1034): the temporary `youversion-platform:highlights:` localStorage store is deleted outright, with no migration. +- A new internal `useBibleReaderHighlights` seam fetches the current chapter's highlights via `useHighlights`, renders them through an in-memory optimistic overlay (reverted on write failure), and collapses contiguous verse selections into range USFMs (e.g. `JHN.3.16-18`) on the wire. +- Everything is gated on an internal `HIGHLIGHTS_LIVE` flag (currently off) plus an authenticated session: while off or signed out, the color row is inert — no fetches, no writes, no rendered highlights. Signing out un-renders highlights immediately. +- `@youversion/platform-react-hooks` now exports `YouVersionAuthContext`, the no-throw alternative to `useYVAuth` for components that must tolerate a missing auth provider (its error message already advertised the export). +- `useApiData` (the fetch engine behind all data hooks, e.g. `useHighlights`, `usePassage`) fixes and behavior changes: + - An `enabled` false→true flip now triggers the fetch even when the caller's deps are unchanged — previously a hook that mounted disabled (e.g. waiting on auth) never fetched after being enabled. + - Disabling now clears `data` (and `error`) instead of keeping the last response, so stale account data cannot linger after sign-out or an auth user switch. + - Responses are now latest-wins across `refetch` too: a stale in-flight request (e.g. a refetch for a previous chapter) resolving after a newer fetch no longer overwrites its data. diff --git a/.changeset/fix-highlights-api-contract.md b/.changeset/fix-highlights-api-contract.md index 10fab280..74f6ecbf 100644 --- a/.changeset/fix-highlights-api-contract.md +++ b/.changeset/fix-highlights-api-contract.md @@ -11,5 +11,4 @@ Fix `HighlightsClient` to match the live highlights API contract. The client pre - `getHighlights` now requires `version_id` and `passage_id` (verse or chapter USFM), and `deleteHighlight` requires `version_id`, matching the API's required parameters; `useHighlights` options are updated accordingly - `getHighlights` now treats a `204` (no highlights for the passage) as an empty collection instead of throwing - `createHighlight` normalizes `color` to lowercase before sending, since the API accepts lowercase hex only -- Added `getRecentColors()` (`GET /v1/highlights/recent-colors`) to `HighlightsClient` and exposed it from `useHighlights` for building color pickers - API responses are validated with Zod and mapped from the wire shape diff --git a/.changeset/fix-highlights-jam-issues.md b/.changeset/fix-highlights-jam-issues.md new file mode 100644 index 00000000..942fa0d8 --- /dev/null +++ b/.changeset/fix-highlights-jam-issues.md @@ -0,0 +1,15 @@ +--- +'@youversion/platform-core': patch +'@youversion/platform-react-hooks': patch +'@youversion/platform-react-ui': patch +--- + +Fix four highlights-stack bugs surfaced by a staging session, plus a fill fade-in. + +- **Invisible primary button (ui):** the `default` button variant paired `bg-background` with `text-primary-foreground`, resolving to white-on-white in the light theme (the highlight permission dialog's Continue button was invisible). It now uses the standard `bg-primary` / `text-primary-foreground` pairing. `YouVersionAuthButton` pins `bg-background` explicitly so its neutral brand surface is unchanged. +- **Optimistic overlay dropped before the write is visible (ui):** after a successful highlight write the seam hook dropped the optimistic overlay as soon as ANY refetch landed, trusting it as server truth. Under read-after-write lag (staging slowness, prod read replicas) that GET often did not yet reflect the write, so a highlight flickered out and back on apply, or a removed highlight reappeared. The overlay is now retired only once a fetch actually reflects the write (apply → the verse shows the written color; remove → the verse no longer shows the removed color); until then the overlay wins. Reset paths (chapter/version change, sign-out) release any write the server never converges on. +- **Refetch coalescing (hooks behavior change):** `useHighlights.createHighlight` / `deleteHighlight` no longer refetch on each call. The sole consumer (`useBibleReaderHighlights`) now issues a single refetch after each batch settles — success or failure — so highlighting verses `[2,3,5]` fires two POSTs but only one GET (previously two). Batches with partial failures settle per sub-write: succeeded writes reconcile to server truth, only the failed writes' verses revert. +- **DELETE by range is unsupported (core + ui):** range passage-ids (e.g. `JHN.1.2-3`) returned a non-2xx from staging and threw. The remove path now sends one DELETE per verse (`JHN.1.2`, `JHN.1.3`); POST/apply still collapses to ranges, which works. The `HighlightsClient.deleteHighlight` docstring no longer claims range delete is supported (marked unverified pending API-team confirmation). Large removals issue more DELETEs but still coalesce to a single refetch. +- **Highlight fade-in (core styles):** verse highlight fills now transition their background color (~250ms, disabled under `prefers-reduced-motion`) instead of popping in, matching the Bible app. The selection underline and layout are untouched. + +Known/accepted trade-off (pending product sign-off): the optimistic overlay holds the locally-written value until a fetch reflects that write. A concurrent change from another device to the same verse therefore renders stale until navigation or the next write on this client. This is the deliberate cost of eliminating the flicker/ghost bugs above; documented in code comments in `useBibleReaderHighlights`. diff --git a/.changeset/highlight-auth-flow.md b/.changeset/highlight-auth-flow.md new file mode 100644 index 00000000..0ccb614d --- /dev/null +++ b/.changeset/highlight-auth-flow.md @@ -0,0 +1,19 @@ +--- +'@youversion/platform-core': patch +'@youversion/platform-react-hooks': patch +'@youversion/platform-react-ui': patch +--- + +Add the highlight auth flow: a color tap in BibleReader without a session or the `highlights` permission now stashes the intent and runs the two-path grant flow (YPE-1034, still behind the internal `HIGHLIGHTS_LIVE` flag). + +- **Core**: new `DataExchangeClient.updateToken` (`POST /data-exchange/token`, 201 → `{ token }`, Zod-validated) plus `buildDataExchangeUrl` / `parseDataExchangeCallback` / `handleDataExchangeCallback` for the hosted just-in-time grant. Sign-in and data-exchange callbacks now parse `granted_permissions` and seed an optimistic permission cache on `YouVersionPlatformConfiguration` (`grantedPermissions`, `hasPermission`, `saveGrantedPermissions`, `removeGrantedPermission`); the cache is cleared on sign-out and a 401/403 invalidates it (server truth wins). `SignInWithYouVersionResult` gains a `permissions` field. +- **Hooks**: new `useHighlightAuthActions` exposing the one-fell-swoop sign-in (requesting `highlights`), the just-in-time data-exchange redirect, the permission-cache reads/invalidation, and the data-exchange return handler. +- **UI**: `useBibleReaderHighlights` now runs the state machine — pending highlights persist to `sessionStorage` (~10-minute expiry) to survive the redirect round-trip and apply automatically on a granted return; a just-in-time permission confirm dialog (`HighlightPermissionDialog`, copy matched to the native SDK) gates the data-exchange grant. Write failures route by status: 401/403 invalidates the cache, keeps the pending highlight, and re-prompts; 5xx/network reverts the optimistic overlay and discards. Apply/remove writes are serialized through a FIFO queue with per-verse ownership so overlapping operations settle to the last-issued state. Copy/share-only behavior when no auth provider is configured is unchanged. + +Deferred follow-ups (documented at each code site in `bible-reader-highlights-machine.ts`, accepted for dark launch): + +1. A 401 from an expired token (not a missing permission) misroutes to the permission re-prompt; follow-up is distinguishing auth-expiry from permission-denied at the `isPermissionError` boundary. +2. A failure while applying a resumed pending highlight only logs + reverts (the pending intent was cleared before the write), so a 401 there loses the highlight instead of keep-pending + re-prompt; follow-up is routing resume-write failures through the standard apply failure handling. +3. ~~A 401/403 on `remove` opens the permission dialog with no pending highlight.~~ Resolved in the PR-288 xstate rewrite: a remove failure now invalidates the cache without re-prompting. + +Note: the signed-out one-fell-swoop redirect described above is superseded by the sign-in dialog introduced in the PR-288 xstate rewrite (see the separate changeset) — a signed-out color tap now opens a dialog before OAuth launches. diff --git a/.changeset/recent-highlight-colors.md b/.changeset/recent-highlight-colors.md new file mode 100644 index 00000000..5df3f80e --- /dev/null +++ b/.changeset/recent-highlight-colors.md @@ -0,0 +1,7 @@ +--- +'@youversion/platform-react-ui': patch +--- + +BibleReader's verse action popover now marks active swatches with a checkmark (YPE-1034 PR3, still behind the internal `HIGHLIGHTS_LIVE` flag). + +- **Checkmark swap**: active/remove swatches now render a 24px checkmark (`icons/check`) instead of the X, matching iOS (platform-sdk-swift #179). Same theme-invariant `#121212` fill and identical behavior — tapping still removes the highlight. diff --git a/.changeset/xstate-highlights-flow.md b/.changeset/xstate-highlights-flow.md new file mode 100644 index 00000000..78471611 --- /dev/null +++ b/.changeset/xstate-highlights-flow.md @@ -0,0 +1,13 @@ +--- +'@youversion/platform-react-ui': patch +--- + +Rewrite the BibleReader highlights flow as an explicit xstate v5 statechart (YPE-1034 / PR-288, still behind the internal `HIGHLIGHTS_LIVE` flag). `useBibleReaderHighlights` is now a thin adapter over `bibleReaderHighlightsMachine`; the machine (authored with `setup()` and named guards/actions/actors, so it is Stately-visualizable) owns the whole flow — optimistic overlay, serialized writes with per-verse ownership, reconcile, the auth flow, and the resume-on-return path. A mermaid statechart lives in `docs/highlight-flow-statechart.md`. All previously-shipped invariants are preserved (writes serialized, per-verse ownership tokens, per-sub-write settlement, apply collapses to ranges / remove DELETEs per verse, one refetch per settled batch, scope-change drops the overlay, `live` gates rendering/fetching). + +Behavior changes: + +- **Sign-in dialog first (signed out):** a signed-out color tap now opens a `SignInDialog` (introducing the app, with an optional integrator prompt) instead of redirecting to OAuth immediately. Confirm launches the sign-in redirect requesting `highlights`; decline/dismiss discards the pending highlight and keeps the verse selection. The dialog's `appName` comes from `YouVersionPlatformConfiguration.appName` (falling back to "This app") and its prompt from `signInPromptMessage`. The hook return gains `signInDialogOpen`, `confirmSignInDialog`, and `cancelSignInDialog`. +- **Flag-off hides the highlights UI:** when `HIGHLIGHTS_LIVE` is off, `VerseActionPopover` hides the color row and the remove (checkmark) circles entirely via a new `highlightsEnabled` prop — only Copy / Share remain. Previously the row still rendered while taps were inert. +- **"Vapor" fix:** a deleted highlight no longer briefly reappears when a stale read-replica fetch lands after the delete settled. The reconcile step no longer retires remove-overlay entries; a removed verse is held until a reset path (scope change, sign-out, or a newer write). Apply-side convergence is unchanged. This matches the already-accepted trade-off (a concurrent same-verse edit from another device renders stale until navigation or the next write). +- **Remove-failure no longer re-prompts:** a 401/403 on a remove invalidates the permission cache but no longer opens the permission dialog (there is no pending highlight to resume), resolving a documented deferred wart. +- **Resume-failure now re-prompts like a user apply:** a 401/403 on a pending highlight resumed after sign-in / data-exchange re-stashes that highlight (using its own scope, so a cross-chapter return re-prompts on the right passage) and re-opens the permission dialog, instead of silently dropping the user's original color tap. Previously this path only logged + reverted (a documented deferred wart from the pre-rewrite hook). diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 00000000..f6702d7b --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,85 @@ +# Ubiquitous Language + +Glossary of domain terms for the YouVersion Platform SDK. Terms here are +canonical: code, docs, and conversation should use them exactly. + +## Highlight + +A user-owned color marking on a Bible passage, stored on the user's +YouVersion account. Highlights are **account data, not device data**: they +require an authenticated user and are never persisted locally by the SDK +(decided in YPE-1034, superseding the temporary localStorage store from +YPE-642 / ADR-001). + +Identified by the pair (**Bible version**, **passage**). A highlight has +exactly one **color**. + +## Passage + +A verse or contiguous verse range in one chapter of one Bible version, +identified by a USFM string (`JHN.3.16`, `JHN.3.16-18`). A chapter USFM +(`JHN.3`) is a passage *scope* used for querying, not a highlightable unit. + +## Bible version + +A translation/edition of the Bible, identified by a numeric id. The SDK +calls this `version_id`; the highlights wire API calls the same value +`bible_id`. The SDK name is canonical in public types; mapping happens at +the API boundary only. + +## Color + +A highlight's fill, a 6-character lowercase hex string without `#` +(`fff9b1`). Uppercase input is accepted and normalized at the API boundary. + +## Verse selection + +The ephemeral set of verses the reader has tapped in `BibleReader`. Drives +the verse action popover. Never persisted; cleared on navigation. + +## Self-contained mode + +The default `BibleReader` posture (YPE-1034): the reader fetches and writes +highlights itself through the SDK's own auth session. The color row is +always offered; tapping a color when the user lacks a session or the +highlights permission enters the highlight auth flow (July 9 2026 sync, +superseding the earlier hide-when-signed-out idea). + +## Highlights permission + +The per-app grant that authorizes highlight reads/writes for a user. It is +**not an OIDC scope**: it travels as `requested_permissions[]=highlights` +alongside `scope` at authorize time (PR #280) and is granted via a data +exchange consent. Permissions are open-ended strings, never enums (more +arrive later, e.g. verse notes). The **server is the source of truth** for +whether it is granted; any client-side permission cache is optimistic only. + +## Pending highlight + +The user's stashed highlight intent (verses + color + timestamp) while the +highlight auth flow is in flight. Survives a redirect round-trip via +sessionStorage; expires stale (~10 min) so an abandoned round-trip can +never apply a highlight during a later sign-in. Discarded on decline, +cancel, or failure. + +## Highlight auth flow + +The state machine (see the React Web auth state machine doc) that turns a +color tap into an applied highlight across two user paths: one-fell-swoop +(sign-in requesting the highlights permission together) and just-in-time +(already signed in, permission confirm dialog → data exchange grant). +Cancellation at any point keeps the verse selection intact and discards +only the pending highlight. + +## Controlled mode + +A planned `BibleReader` posture (YPE-3705): the host application supplies +rendered highlights as data and receives highlight-intent events, and the +reader makes no highlight network calls. The host owns auth, persistence, +and conflict rules. Used by native hosts (RN Expo SDK) embedding the web +reader. + +## Verse action popover + +The floating action bar (YPE-642) that appears over a verse selection, +offering highlight colors, copy, and share. diff --git a/docs/adr/YPE-1034-highlights-server-only.md b/docs/adr/YPE-1034-highlights-server-only.md new file mode 100644 index 00000000..326cecf3 --- /dev/null +++ b/docs/adr/YPE-1034-highlights-server-only.md @@ -0,0 +1,52 @@ +# YPE-1034 — Wire the highlights API into BibleReader + +Status: **Decided** (grilling session 2026-07-10, Cam + Dustin) +Component: `packages/ui/src/components/bible-reader.tsx` (+ hooks `useHighlights`) +Inputs: July 9 2026 highlights sync (Notion: "React Web SDK Highlights: +Implementation Brief"), auth state machine doc, YPE-642 gotchas doc, +Swift reference PR platform-sdk-swift#179. + +## ADR-001 — Highlights are server-only; the localStorage store is removed + +**Supersedes ADR-001 in [YPE-642](./YPE-642-verse-action-popover.md).** + +### Decision + +The localStorage highlight store (`youversion-platform:highlights:`) +is deleted outright — no migration, no signed-out fallback. Highlights are +fetched from and written to `/v1/highlights` exclusively, through the SDK's +authenticated session. A user with no session (or whose app lacks the +`highlights` permission) enters the highlight auth flow when they tap a color; +their intent is stashed as a **pending highlight** (sessionStorage, ~10-minute +expiry), never as a persisted local highlight. + +The only local traces of highlight state are: +- the in-memory optimistic overlay while a write is in flight, +- the pending highlight during an auth round-trip, +- an optimistic localStorage cache of the *permission* grant (not highlight + data), which the server can invalidate at any time via 401/403. + +### Why + +- Highlights are account data. A browser-profile copy silently diverges from + the user's YouVersion account and dies at the browser boundary. +- YPE-642's store was explicitly a stand-in for this ticket (its ADR-001 said + "server sync is a separate ticket" — this is that ticket). +- The SDK is pre-1.0 with few consumers; the migration code for weeks-old + throwaway data would outlive its usefulness. +- Two persistence paths (API + local) double the state machine and create an + unanswerable merge question on sign-in. + +### Consequences + +- Sign-out immediately un-renders all highlights. +- Signed-out readers see no highlights; that is correct, not a regression. +- The RN Expo / native-host story (YPE-3705) supplies highlights via + controlled props instead — it does not resurrect local persistence. + +### Alternatives rejected + +- **localStorage for signed-out + API for signed-in:** merge-on-sign-in + conflicts, doubled state machine, data that still dies per-device. +- **One-time migration of existing local highlights:** permanent code for + transient data nobody has accumulated meaningfully. diff --git a/docs/adr/YPE-642-verse-action-popover.md b/docs/adr/YPE-642-verse-action-popover.md index ec13acf8..eb522547 100644 --- a/docs/adr/YPE-642-verse-action-popover.md +++ b/docs/adr/YPE-642-verse-action-popover.md @@ -30,6 +30,9 @@ gone.** #131's `VerseActionPopover` (correct AC logic, tested, already uses Radi ## Decisions (ADRs) ### ADR-001 — localStorage only this PR +> **Superseded** by [YPE-1034 ADR-001](./YPE-1034-highlights-server-only.md): +> highlights are server-only; the localStorage store is removed. + Highlights persist client-side only. Server sync is a **separate ticket**. No network, no API client this PR. @@ -143,6 +146,11 @@ Selection is always enabled in BibleReader (no opt-out prop for now; YAGNI). - Tunable; swap to a ring/bg later if Figma says otherwise. ## As-built notes (deviations from the design above) +- **ADR-005 active-swatch icon (YPE-1034 PR3):** the 24px **X** on active/remove + swatches was replaced with a 24px **checkmark** (`icons/check`), matching iOS + (platform-sdk-swift #179). Same Text/Everdark (`--yv-gray-50` = `#121212`, + theme-invariant) fill and identical behavior — tapping still removes the + highlight; the swatch's `Clear highlight` aria-label is unchanged. - **ADR-004 revised:** selection + highlights live in `Content`, **not** Root context. Copy/Share/anchor all need the rendered verse DOM (which lives in Content), so Root ownership would fragment the feature. BibleTextView stays diff --git a/docs/highlight-flow-statechart.md b/docs/highlight-flow-statechart.md new file mode 100644 index 00000000..038ebb40 --- /dev/null +++ b/docs/highlight-flow-statechart.md @@ -0,0 +1,102 @@ +# BibleReader highlights flow — statechart (PR-288) + +The BibleReader highlights flow is an [xstate v5](https://stately.ai/docs) state +machine: [`packages/ui/src/components/bible-reader-highlights-machine.ts`](../packages/ui/src/components/bible-reader-highlights-machine.ts). +`useBibleReaderHighlights` is a thin adapter that feeds React-owned inputs (auth, +the `HIGHLIGHTS_LIVE` flag, the fetched highlights, the scope) into the machine +as events and reads back the dialog states + the rendered verse map. + +The machine is authored with `setup()` and named guards/actions/actors so it is +statically analyzable — paste the source into the [Stately visualizer](https://stately.ai/viz) +to explore it interactively. + +## States and events + +- **`booting`** → routes to `disabled` or `enabled` from the initial input. +- **`disabled`** — the flag is off **or** no auth provider is mounted. Fully + inert: no fetch, no writes, no dialogs. A color tap resolves to `noop`. +- **`enabled`** — a parallel state with two independent regions: + - **`flow`** — the auth / dialog flow. + - `resuming` consumes the data-exchange return exactly once, then routes on + the pending highlight + auth + permission. + - `awaitingAuth` waits for the authenticated flip after a redirect return. + - `idle` is interactive. + - `signInDialog` / `permissionDialog` are the two consent dialogs. + - **`writer`** — serialized optimistic writes. `idle → writing → checkQueue`, + processing one queued operation at a time so a DELETE can never overtake an + in-flight POST for the same verse. + +`TAP_COLOR` forks in `flow`: authorized (`applied`) → optimistic write; signed +out → `signInDialog`; signed in without the permission → `permissionDialog`. +Both dialog paths stash a pending highlight (10-min `sessionStorage` TTL) so the +intent survives the full-page redirect and resumes on a granted return. + +```mermaid +stateDiagram-v2 + [*] --> booting + booting --> disabled: flag off / no provider + booting --> enabled: flag on & provider + + disabled --> enabled: AUTH_CHANGED (enabled) + enabled --> disabled: AUTH_CHANGED (disabled) + + state disabled { + note right of disabled + TAP_COLOR → outcome "noop" + no fetch / writes / dialogs + end note + } + + state enabled { + -- + state flow { + [*] --> resuming + resuming --> idle: no pending + resuming --> awaitingAuth: pending & not authed + resuming --> idle: pending & authed & permission / applyPending + resuming --> permissionDialog: pending & authed & no permission + + awaitingAuth --> idle: authed & permission / applyPending + awaitingAuth --> permissionDialog: authed & no permission + + idle --> idle: TAP_COLOR authorized / optimistic write + idle --> signInDialog: TAP_COLOR signed out / stash pending + idle --> permissionDialog: TAP_COLOR no permission / stash pending + + signInDialog --> idle: CONFIRM_SIGN_IN / start sign-in redirect + signInDialog --> idle: DECLINE_SIGN_IN / clear pending + + permissionDialog --> idle: CONFIRM_PERMISSION / start data-exchange + permissionDialog --> idle: CANCEL_PERMISSION / clear pending + + idle --> permissionDialog: PERMISSION_LOST (401/403 on write) + } + -- + state writer { + [*] --> w_idle + w_idle --> writing: queue has work + writing --> checkQueue: processWrite done / settle + shift + checkQueue --> writing: queue has work + checkQueue --> w_idle: queue empty + } + } +``` + +_(`ENQUEUE`, `AUTH_CHANGED`, `HIGHLIGHTS_UPDATED`, and `SCOPE_CHANGED` are handled +without leaving the current state: they update context and let the `always` +guards re-route. `HIGHLIGHTS_UPDATED` also runs the overlay reconcile.)_ + +## The "vapor" fix + +Reported on staging: a deleted highlight reappears for a split second, then +disappears. Root cause (confirmed): the reconcile step retired a REMOVE overlay +entry as soon as any fetch reflected the removal; a later response from a stale +read replica that still contained the highlight then had nothing suppressing it, +so the verse repainted until the next fetch cleared it. + +Fix: `reconcileOverlay` never retires remove-overlay entries — a removed verse's +optimistic `null` is held until a reset path (scope change, sign-out, or a newer +write re-claiming the verse). Apply entries still retire on reflection, keeping +the tested apply-convergence behavior. This is exactly the PR's already-accepted +trade-off: a concurrent same-verse edit from another device renders stale until +navigation or the next write on this client. diff --git a/packages/core/src/SignInWithYouVersionResult.ts b/packages/core/src/SignInWithYouVersionResult.ts index 83383c32..fb550b4e 100644 --- a/packages/core/src/SignInWithYouVersionResult.ts +++ b/packages/core/src/SignInWithYouVersionResult.ts @@ -14,6 +14,7 @@ type SignInWithYouVersionResultProps = { name?: string; profilePicture?: string; email?: string; + permissions?: string[]; }; export class SignInWithYouVersionResult { public readonly accessToken: string | undefined; @@ -23,6 +24,12 @@ export class SignInWithYouVersionResult { public readonly name: string | undefined; public readonly profilePicture: string | undefined; public readonly email: string | undefined; + /** + * Data-exchange permissions the server reported as granted for this sign-in + * (parsed from `granted_permissions` on the callback). Empty when the callback + * carried none. Additive: existing consumers can ignore it. + */ + public permissions: string[]; constructor({ accessToken, @@ -32,6 +39,7 @@ export class SignInWithYouVersionResult { name, profilePicture, email, + permissions, }: SignInWithYouVersionResultProps) { this.accessToken = accessToken; this.expiryDate = expiresIn ? new Date(Date.now() + expiresIn * 1000) : new Date(); @@ -40,5 +48,6 @@ export class SignInWithYouVersionResult { this.name = name; this.profilePicture = profilePicture; this.email = email; + this.permissions = permissions ?? []; } } diff --git a/packages/core/src/Users.ts b/packages/core/src/Users.ts index cf1678c3..571c5bd7 100644 --- a/packages/core/src/Users.ts +++ b/packages/core/src/Users.ts @@ -3,6 +3,7 @@ import { YouVersionUserInfo } from './YouVersionUserInfo'; import { YouVersionPlatformConfiguration } from './YouVersionPlatformConfiguration'; import { SignInWithYouVersionPKCEAuthorizationRequestBuilder } from './SignInWithYouVersionPKCE'; import { SignInWithYouVersionResult } from './SignInWithYouVersionResult'; +import { parseGrantedPermissions } from './permissions'; export class YouVersionAPIUsers { /** @@ -133,7 +134,8 @@ export class YouVersionAPIUsers { ); // Persist the decoded user profile so it survives reloads without - // retaining the ID token itself. + // retaining the ID token itself. This must happen before the permission + // cache is seeded below, since grants are scoped to the current user. YouVersionPlatformConfiguration.saveUserInfo({ id: result.yvpUserId, name: result.name, @@ -141,6 +143,17 @@ export class YouVersionAPIUsers { avatar_url: result.profilePicture, }); + // Surface + persist the data-exchange permissions the server granted. The + // server echoes them as `granted_permissions` on the callback URL (comma- + // or space-separated, param may repeat). This seeds the optimistic + // permission cache so a one-fell-swoop sign-in that requested `highlights` + // can apply a pending highlight on return without a probe round-trip. + const grantedPermissions = parseGrantedPermissions(urlParams); + result.permissions = grantedPermissions; + if (grantedPermissions.length > 0) { + YouVersionPlatformConfiguration.saveGrantedPermissions(grantedPermissions); + } + // Clean up localStorage localStorage.removeItem('youversion-auth-code-verifier'); localStorage.removeItem('youversion-auth-redirect-uri'); diff --git a/packages/core/src/YouVersionPlatformConfiguration.ts b/packages/core/src/YouVersionPlatformConfiguration.ts index 1e46f94f..daa8bf66 100644 --- a/packages/core/src/YouVersionPlatformConfiguration.ts +++ b/packages/core/src/YouVersionPlatformConfiguration.ts @@ -15,6 +15,8 @@ export class YouVersionPlatformConfiguration { private static _apiHost: string = 'api.youversion.com'; private static _refreshTokenKey: string | null = null; private static _expiryDateKey: string | null = null; + private static _signInPromptMessage: string | undefined = undefined; + private static _appName: string | undefined = undefined; private static getOrSetInstallationId(): string { if (typeof window === 'undefined') { @@ -73,6 +75,146 @@ export class YouVersionPlatformConfiguration { public static clearAuthTokens(): void { this.saveAuthData(null, null, null); this.saveUserInfo(null); + this.clearGrantedPermissions(); + this.clearDataExchangeInitiator(); + } + + /** + * Optimistic cache of the data-exchange permissions the server told us are + * granted (seeded from `granted_permissions` on the sign-in / data-exchange + * callbacks). It is optimistic only: the server is the source of truth, and a + * 401/403 on a permissioned request invalidates the relevant entry via + * {@link removeGrantedPermission}. + * + * The cache is scoped to the signed-in user: it is persisted as + * `{ userId, permissions }` and only read back when `userId` matches the + * current {@link storedUserInfo}. This prevents one user's grants from leaking + * to a later user who signs in without a {@link clearAuthTokens} in between. + */ + private static readonly grantedPermissionsKey = 'youversion-platform:granted-permissions'; + + /** The id of the user the cache is scoped to, or `null` when signed out. */ + private static get currentUserId(): string | null { + return this.storedUserInfo?.id ?? null; + } + + /** + * Reads the stored `{ userId, permissions }` entry, or `null` when absent, + * malformed, or in the legacy bare-array format (which is treated as absent). + */ + private static readStoredGrants(): { userId: string; permissions: string[] } | null { + if (typeof localStorage === 'undefined') return null; + const raw = localStorage.getItem(this.grantedPermissionsKey); + if (!raw) return null; + try { + const parsed: unknown = JSON.parse(raw); + if (typeof parsed !== 'object' || parsed === null) return null; + const record = parsed as Record; + // Reject the legacy bare-array format and any other malformed shape. + if (typeof record.userId !== 'string' || !Array.isArray(record.permissions)) { + return null; + } + return { + userId: record.userId, + permissions: record.permissions.filter( + (entry): entry is string => typeof entry === 'string', + ), + }; + } catch { + return null; + } + } + + private static writeStoredGrants(userId: string, permissions: string[]): void { + localStorage.setItem(this.grantedPermissionsKey, JSON.stringify({ userId, permissions })); + } + + public static get grantedPermissions(): string[] { + const userId = this.currentUserId; + if (!userId) return []; + const stored = this.readStoredGrants(); + if (stored?.userId !== userId) return []; + return stored.permissions; + } + + /** + * Merges `permissions` into the cache (union) when the cached entry belongs to + * the current user; a different (or absent) owner is replaced wholesale. + * No-ops when signed out, since grants must be scoped to a user. + */ + public static saveGrantedPermissions(permissions: string[]): void { + if (typeof localStorage === 'undefined') return; + const userId = this.currentUserId; + if (!userId) return; + const merged = new Set([...this.grantedPermissions, ...permissions]); + this.writeStoredGrants(userId, [...merged]); + } + + /** + * Replaces the cache with exactly `permissions` for the current user (used to + * reconcile the cache with the authoritative set the server returns on a + * data-exchange grant). No-ops when signed out. + */ + public static setGrantedPermissions(permissions: string[]): void { + if (typeof localStorage === 'undefined') return; + const userId = this.currentUserId; + if (!userId) return; + this.writeStoredGrants(userId, [...new Set(permissions)]); + } + + /** Drops a single permission from the cache — used to honor a server 401/403. */ + public static removeGrantedPermission(permission: string): void { + if (typeof localStorage === 'undefined') return; + const userId = this.currentUserId; + if (!userId) return; + const next = this.grantedPermissions.filter((entry) => entry !== permission); + this.writeStoredGrants(userId, next); + } + + public static clearGrantedPermissions(): void { + if (typeof localStorage === 'undefined') return; + localStorage.removeItem(this.grantedPermissionsKey); + } + + /** + * The id of the user who initiated the pending data-exchange redirect. + * + * The just-in-time data-exchange flow is fire-and-forget: it full-page + * redirects to a hosted consent page and the grant comes back on the return + * URL. If a different user signs in on another tab before the redirect + * returns, the grant would otherwise be saved under whoever is signed in when + * the callback loads. Recording the initiating user here lets the callback + * verify the same user is still signed in before honoring the grant. + */ + private static readonly dataExchangeInitiatorKey = 'youversion-platform:data-exchange-initiator'; + + /** + * Records the current user as the initiator of a data-exchange redirect. + * No-ops when signed out — the just-in-time flow only starts authenticated + * (minting the token requires an access token), so a missing initiator on + * return is treated as untrusted by {@link dataExchangeInitiator}'s consumer. + */ + public static saveDataExchangeInitiator(): void { + if (typeof localStorage === 'undefined') return; + const userId = this.currentUserId; + if (!userId) return; + localStorage.setItem(this.dataExchangeInitiatorKey, userId); + } + + /** The initiating user's id for the pending data exchange, or `null`. */ + public static get dataExchangeInitiator(): string | null { + if (typeof localStorage === 'undefined') return null; + return localStorage.getItem(this.dataExchangeInitiatorKey); + } + + public static clearDataExchangeInitiator(): void { + if (typeof localStorage === 'undefined') return; + localStorage.removeItem(this.dataExchangeInitiatorKey); + } + + /** Optimistic check against the permission cache. Server 401/403 still wins. */ + public static hasPermission(permission: string): boolean { + return this.grantedPermissions.includes(permission); } public static get accessToken(): string | null { @@ -149,4 +291,29 @@ export class YouVersionPlatformConfiguration { static set expiryDateKey(value: string) { this._expiryDateKey = value; } + + /** + * The integrator's own pitch line shown in the sign-in dialog. Optional; not + * persisted (it is supplied by configuration on each app load). + */ + static get signInPromptMessage(): string | undefined { + return this._signInPromptMessage; + } + + static set signInPromptMessage(value: string | undefined) { + this._signInPromptMessage = value; + } + + /** + * The integrator's display name used in the sign-in dialog copy (e.g. + * "{appName} wants to connect to your YouVersion Bible App account"). Optional; + * not persisted (it is supplied by configuration on each app load). + */ + static get appName(): string | undefined { + return this._appName; + } + + static set appName(value: string | undefined) { + this._appName = value; + } } diff --git a/packages/core/src/__tests__/YouVersionPlatformConfiguration.test.ts b/packages/core/src/__tests__/YouVersionPlatformConfiguration.test.ts index 9a38cb0c..a248c7cc 100644 --- a/packages/core/src/__tests__/YouVersionPlatformConfiguration.test.ts +++ b/packages/core/src/__tests__/YouVersionPlatformConfiguration.test.ts @@ -344,4 +344,30 @@ describe('YouVersionPlatformConfiguration', () => { expect(firstId).toBe(mockUUID); }); }); + + describe('signInPromptMessage', () => { + it('should default to undefined and get/set the value', () => { + expect(YouVersionPlatformConfiguration.signInPromptMessage).toBeUndefined(); + + YouVersionPlatformConfiguration.signInPromptMessage = 'Save your highlights across devices.'; + expect(YouVersionPlatformConfiguration.signInPromptMessage).toBe( + 'Save your highlights across devices.', + ); + + YouVersionPlatformConfiguration.signInPromptMessage = undefined; + expect(YouVersionPlatformConfiguration.signInPromptMessage).toBeUndefined(); + }); + }); + + describe('appName', () => { + it('should default to undefined and get/set the value', () => { + expect(YouVersionPlatformConfiguration.appName).toBeUndefined(); + + YouVersionPlatformConfiguration.appName = 'Acme Bible'; + expect(YouVersionPlatformConfiguration.appName).toBe('Acme Bible'); + + YouVersionPlatformConfiguration.appName = undefined; + expect(YouVersionPlatformConfiguration.appName).toBeUndefined(); + }); + }); }); diff --git a/packages/core/src/__tests__/data-exchange.test.ts b/packages/core/src/__tests__/data-exchange.test.ts new file mode 100644 index 00000000..6f2ed03a --- /dev/null +++ b/packages/core/src/__tests__/data-exchange.test.ts @@ -0,0 +1,247 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { http, HttpResponse } from 'msw'; +import { ApiClient } from '../client'; +import { + DataExchangeClient, + buildDataExchangeUrl, + parseDataExchangeCallback, + handleDataExchangeCallback, +} from '../data-exchange'; +import { YouVersionPlatformConfiguration } from '../YouVersionPlatformConfiguration'; +import { server } from './setup'; + +const apiHost = process.env.YVP_API_HOST; + +/** + * Stubs `window` with a location parsed from `href` plus a spyable + * `history.replaceState`. `localStorage` is left as the test polyfill so + * {@link YouVersionPlatformConfiguration} reads/writes real (mock) storage. + */ +function stubLocation(href: string): ReturnType { + const replaceState = vi.fn(); + const url = new URL(href); + vi.stubGlobal('window', { + location: { href, search: url.search }, + history: { replaceState }, + }); + return replaceState; +} + +describe('DataExchangeClient.updateToken', () => { + let client: DataExchangeClient; + + beforeEach(() => { + client = new DataExchangeClient( + new ApiClient({ apiHost, appKey: 'test-app', installationId: 'test-installation' }), + ); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + }); + + it('POSTs requested_permissions with app-key query + Bearer auth and returns the token', async () => { + let seenAuth: string | null = null; + let seenBody: unknown = null; + let seenUrl = ''; + server.use( + http.post(`https://${apiHost}/data-exchange/token`, async ({ request }) => { + seenAuth = request.headers.get('Authorization'); + seenBody = await request.json(); + seenUrl = request.url; + return HttpResponse.json({ token: 'dx-token-123' }, { status: 201 }); + }), + ); + + const token = await client.updateToken(['highlights'], 'my-access-token'); + + expect(token).toBe('dx-token-123'); + expect(seenAuth).toBe('Bearer my-access-token'); + expect(seenBody).toEqual({ requested_permissions: ['highlights'] }); + expect(seenUrl).toContain('app-key=test-app'); + }); + + it('throws (401 = not permitted) when the server rejects', async () => { + server.use( + http.post( + `https://${apiHost}/data-exchange/token`, + () => new HttpResponse(null, { status: 401 }), + ), + ); + + await expect(client.updateToken(['highlights'], 'tok')).rejects.toThrow(); + }); + + it('throws when the response fails schema validation', async () => { + server.use( + http.post(`https://${apiHost}/data-exchange/token`, () => + HttpResponse.json({ not_a_token: true }, { status: 201 }), + ), + ); + + await expect(client.updateToken(['highlights'], 'tok')).rejects.toThrow( + /Unexpected data exchange token response/, + ); + }); +}); + +describe('buildDataExchangeUrl', () => { + it('builds the hosted consent URL with token + both app-key params', () => { + const url = new URL(buildDataExchangeUrl('tok-9', 'app-42', 'api.example.com')); + expect(url.origin + url.pathname).toBe('https://api.example.com/data-exchange'); + expect(url.searchParams.get('token')).toBe('tok-9'); + expect(url.searchParams.get('app_key')).toBe('app-42'); + expect(url.searchParams.get('x-yvp-app-key')).toBe('app-42'); + }); +}); + +describe('parseDataExchangeCallback', () => { + it('returns null when there is no data_exchange_status', () => { + expect(parseDataExchangeCallback('?state=abc&code=1')).toBeNull(); + }); + + it('parses a granted return with granted_permissions', () => { + expect( + parseDataExchangeCallback('?data_exchange_status=granted&granted_permissions=highlights'), + ).toEqual({ status: 'granted', grantedPermissions: ['highlights'] }); + }); + + it('maps cancel verbatim and treats anything else as failure', () => { + expect(parseDataExchangeCallback('?data_exchange_status=cancel')).toEqual({ + status: 'cancel', + grantedPermissions: [], + }); + expect(parseDataExchangeCallback('?data_exchange_status=weird')).toEqual({ + status: 'failure', + grantedPermissions: [], + }); + // Present but empty value → failure. + expect(parseDataExchangeCallback('?data_exchange_status=')).toEqual({ + status: 'failure', + grantedPermissions: [], + }); + }); +}); + +describe('handleDataExchangeCallback URL cleanup', () => { + beforeEach(() => { + // A signed-in user who is also the flow initiator, so a `granted` return is + // honored and these tests can focus on URL cleanup. + YouVersionPlatformConfiguration.clearAuthTokens(); + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-a', name: 'A' }); + YouVersionPlatformConfiguration.saveDataExchangeInitiator(); + }); + + afterEach(() => { + YouVersionPlatformConfiguration.clearAuthTokens(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('strips only the data-exchange params, preserving app params and the hash', () => { + const replaceState = stubLocation( + 'https://app.example.com/read?tab=notes&ref=abc&data_exchange_status=granted&granted_permissions=highlights#section', + ); + + const result = handleDataExchangeCallback(); + expect(result).toEqual({ status: 'granted', grantedPermissions: ['highlights'] }); + + expect(replaceState).toHaveBeenCalledTimes(1); + const cleaned = new URL(replaceState.mock.calls[0]![2] as string); + expect(cleaned.searchParams.get('tab')).toBe('notes'); + expect(cleaned.searchParams.get('ref')).toBe('abc'); + expect(cleaned.searchParams.has('data_exchange_status')).toBe(false); + expect(cleaned.searchParams.has('granted_permissions')).toBe(false); + expect(cleaned.hash).toBe('#section'); + }); + + it('leaves no dangling "?" when only exchange params were present', () => { + const replaceState = stubLocation( + 'https://app.example.com/read?data_exchange_status=granted&granted_permissions=highlights', + ); + + handleDataExchangeCallback(); + + const cleanedUrl = replaceState.mock.calls[0]![2] as string; + expect(cleanedUrl).toBe('https://app.example.com/read'); + expect(cleanedUrl).not.toContain('?'); + }); + + it('returns null and does not touch history when the URL is not a data-exchange return', () => { + const replaceState = stubLocation('https://app.example.com/read?tab=notes'); + expect(handleDataExchangeCallback()).toBeNull(); + expect(replaceState).not.toHaveBeenCalled(); + }); +}); + +describe('handleDataExchangeCallback grant safety (initiating user)', () => { + const GRANTED_URL = + 'https://app.example.com/read?data_exchange_status=granted&granted_permissions=highlights'; + + beforeEach(() => { + YouVersionPlatformConfiguration.clearAuthTokens(); + }); + + afterEach(() => { + YouVersionPlatformConfiguration.clearAuthTokens(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('saves the grant when the initiating user is still signed in on return', () => { + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-a', name: 'A' }); + YouVersionPlatformConfiguration.saveDataExchangeInitiator(); + + stubLocation(GRANTED_URL); + const result = handleDataExchangeCallback(); + + expect(result).toEqual({ status: 'granted', grantedPermissions: ['highlights'] }); + expect(YouVersionPlatformConfiguration.grantedPermissions).toContain('highlights'); + // Consumed on return so it cannot authorize a later, unrelated callback. + expect(YouVersionPlatformConfiguration.dataExchangeInitiator).toBeNull(); + }); + + it('discards the grant when a different user signed in mid-redirect', () => { + // User A initiates the flow... + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-a', name: 'A' }); + YouVersionPlatformConfiguration.saveDataExchangeInitiator(); + // ...then user B signs in on another tab before the redirect returns. + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-b', name: 'B' }); + + const replaceState = stubLocation(GRANTED_URL); + const result = handleDataExchangeCallback(); + + // Grant is not honored; the result degrades to a failed exchange. + expect(result).toEqual({ status: 'failure', grantedPermissions: [] }); + + // Cache untouched for the signed-in user (B)... + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + // ...and for the initiating user (A) once they sign back in. + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-a', name: 'A' }); + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + + // URL cleanup still happens on mismatch. + expect(replaceState).toHaveBeenCalledTimes(1); + const cleaned = new URL(replaceState.mock.calls[0]![2] as string); + expect(cleaned.searchParams.has('data_exchange_status')).toBe(false); + expect(cleaned.searchParams.has('granted_permissions')).toBe(false); + }); + + it('discards the grant when no initiator was recorded (legacy pending state)', () => { + // Signed-in user, but no initiator was ever stored for this return. + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-a', name: 'A' }); + + const replaceState = stubLocation(GRANTED_URL); + const result = handleDataExchangeCallback(); + + expect(result).toEqual({ status: 'failure', grantedPermissions: [] }); + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + + // URL cleanup still happens. + expect(replaceState).toHaveBeenCalledTimes(1); + const cleaned = new URL(replaceState.mock.calls[0]![2] as string); + expect(cleaned.searchParams.has('data_exchange_status')).toBe(false); + expect(cleaned.searchParams.has('granted_permissions')).toBe(false); + }); +}); diff --git a/packages/core/src/__tests__/handlers.ts b/packages/core/src/__tests__/handlers.ts index 9b89ab9a..6b64952b 100644 --- a/packages/core/src/__tests__/handlers.ts +++ b/packages/core/src/__tests__/handlers.ts @@ -123,15 +123,6 @@ export const handlers = [ }); }), - http.get(`https://${apiHost}/v1/highlights/recent-colors`, ({ request }) => { - const authError = requireBearerAuth(request); - if (authError) return authError; - - return HttpResponse.json({ - data: [{ color: 'fffe00' }, { color: '5dff79' }], - }); - }), - http.post(`https://${apiHost}/v1/highlights`, async ({ request }) => { const authError = requireBearerAuth(request); if (authError) return authError; diff --git a/packages/core/src/__tests__/highlights.test.ts b/packages/core/src/__tests__/highlights.test.ts index e296b436..762720a6 100644 --- a/packages/core/src/__tests__/highlights.test.ts +++ b/packages/core/src/__tests__/highlights.test.ts @@ -117,43 +117,6 @@ describe('HighlightsClient', () => { }); }); - describe('getRecentColors', () => { - it('returns the ordered list of hex colors with Bearer auth', async () => { - const fetchSpy = vi.spyOn(global, 'fetch'); - - const colors = await highlightsClient.getRecentColors('test-token'); - - expect(colors).toEqual(['fffe00', '5dff79']); - - const [url, init] = fetchSpy.mock.calls[0] as [string, RequestInit]; - expect(url).toContain('/v1/highlights/recent-colors'); - expect((init.headers as Record).Authorization).toBe('Bearer test-token'); - }); - - it('returns an empty list when the API responds 204', async () => { - server.use( - http.get( - `https://${apiHost}/v1/highlights/recent-colors`, - () => new HttpResponse(null, { status: 204 }), - ), - ); - - await expect(highlightsClient.getRecentColors('test-token')).resolves.toEqual([]); - }); - - it('throws a helpful error when the API response is malformed', async () => { - server.use( - http.get(`https://${apiHost}/v1/highlights/recent-colors`, () => - HttpResponse.json({ data: [{ not_a_color: true }] }), - ), - ); - - await expect(highlightsClient.getRecentColors('test-token')).rejects.toThrow( - 'Unexpected highlights API response', - ); - }); - }); - describe('createHighlight', () => { it('POSTs the enveloped body ({ request_id, highlight: { bible_id, ... } })', async () => { let capturedBody: unknown; diff --git a/packages/core/src/__tests__/permissions.test.ts b/packages/core/src/__tests__/permissions.test.ts new file mode 100644 index 00000000..e1a20871 --- /dev/null +++ b/packages/core/src/__tests__/permissions.test.ts @@ -0,0 +1,140 @@ +/** + * @vitest-environment jsdom + */ +import { describe, it, expect, beforeEach } from 'vitest'; +import { parseGrantedPermissions } from '../permissions'; +import { YouVersionPlatformConfiguration } from '../YouVersionPlatformConfiguration'; + +describe('parseGrantedPermissions', () => { + it('parses a single value', () => { + const params = new URLSearchParams('granted_permissions=highlights'); + expect(parseGrantedPermissions(params)).toEqual(['highlights']); + }); + + it('splits comma- and space-separated values and de-duplicates', () => { + const params = new URLSearchParams('granted_permissions=highlights,votd%20bibles'); + expect(parseGrantedPermissions(params)).toEqual(['highlights', 'votd', 'bibles']); + }); + + it('unions repeated params', () => { + const params = new URLSearchParams( + 'granted_permissions=highlights&granted_permissions=votd,highlights', + ); + expect(parseGrantedPermissions(params)).toEqual(['highlights', 'votd']); + }); + + it('returns [] when the param is absent', () => { + expect(parseGrantedPermissions(new URLSearchParams('state=x'))).toEqual([]); + }); +}); + +describe('YouVersionPlatformConfiguration permission cache', () => { + const storageKey = 'youversion-platform:granted-permissions'; + + beforeEach(() => { + localStorage.clear(); + // The cache is scoped to the signed-in user; establish one for the cases + // that exercise reads/writes for a single user. + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-a' }); + }); + + it('starts empty and merges granted permissions without duplicates', () => { + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + expect(YouVersionPlatformConfiguration.hasPermission('highlights')).toBe(false); + + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights', 'votd']); + + expect(YouVersionPlatformConfiguration.grantedPermissions.sort()).toEqual([ + 'highlights', + 'votd', + ]); + expect(YouVersionPlatformConfiguration.hasPermission('highlights')).toBe(true); + }); + + it('setGrantedPermissions overwrites the cache (reconcile)', () => { + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights', 'votd']); + YouVersionPlatformConfiguration.setGrantedPermissions(['highlights']); + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual(['highlights']); + }); + + it('removeGrantedPermission honors a server 401/403 invalidation', () => { + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights', 'votd']); + YouVersionPlatformConfiguration.removeGrantedPermission('highlights'); + expect(YouVersionPlatformConfiguration.hasPermission('highlights')).toBe(false); + expect(YouVersionPlatformConfiguration.hasPermission('votd')).toBe(true); + }); + + it('clearAuthTokens also clears the permission cache', () => { + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + YouVersionPlatformConfiguration.clearAuthTokens(); + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + }); + + it('tolerates malformed stored JSON', () => { + localStorage.setItem(storageKey, '{not json'); + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + }); + + it('ignores the legacy bare-array format', () => { + localStorage.setItem(storageKey, JSON.stringify(['highlights'])); + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + expect(YouVersionPlatformConfiguration.hasPermission('highlights')).toBe(false); + }); + + it('returns [] when signed out even if an entry is stored', () => { + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual(['highlights']); + + // Drop the user without clearing the cache entry. + YouVersionPlatformConfiguration.saveUserInfo(null); + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + }); + + describe('user scoping (security)', () => { + it('does not leak grants to a different user who signs in without clearAuthTokens', () => { + // User A grants highlights. + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + expect(YouVersionPlatformConfiguration.hasPermission('highlights')).toBe(true); + + // User B signs in (new session) without clearAuthTokens first. + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-b' }); + + // B must not inherit A's grant. + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + expect(YouVersionPlatformConfiguration.hasPermission('highlights')).toBe(false); + }); + + it("a callback without a granted_permissions echo does not resurrect the prior user's grants", () => { + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-b' }); + + // A `granted` return with no permissions echoed → nothing is saved. + // B's cache stays empty rather than falling back to A's entry. + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + }); + + it('a different user replaces the entry wholesale on the next save', () => { + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights', 'votd']); + + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-b' }); + YouVersionPlatformConfiguration.saveGrantedPermissions(['bibles']); + + // Only B's grant is present; A's are gone. + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual(['bibles']); + + // And A cannot read them back either. + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-a' }); + expect(YouVersionPlatformConfiguration.grantedPermissions).toEqual([]); + }); + + it('same-user merge still accumulates grants', () => { + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + YouVersionPlatformConfiguration.saveGrantedPermissions(['votd']); + expect(YouVersionPlatformConfiguration.grantedPermissions.sort()).toEqual([ + 'highlights', + 'votd', + ]); + }); + }); +}); diff --git a/packages/core/src/auth-token.ts b/packages/core/src/auth-token.ts new file mode 100644 index 00000000..6ed2b5de --- /dev/null +++ b/packages/core/src/auth-token.ts @@ -0,0 +1,29 @@ +import { YouVersionPlatformConfiguration } from './YouVersionPlatformConfiguration'; + +/** + * Resolves the auth token from an explicit `lat` or the ambient platform + * configuration. Shared by the service clients (highlights, data exchange) so + * they resolve the token identically. + * + * The implicit fallback reads from `YouVersionPlatformConfiguration`, which is + * backed by browser `localStorage`. In any environment without it (Node.js + * tests, SSR) there is intentionally no ambient token: server-side callers must + * pass `lat` explicitly rather than relying on this fallback. + * + * @param lat Optional explicit long access token. + * @param action Human-readable action for the error message tail (e.g. + * `'accessing highlights'`), rendered as + * `...sign in before .` + * @throws Error if no token is available. + */ +export function resolveAuthToken(lat: string | undefined, action: string): string { + if (lat) { + return lat; + } + const token = + typeof localStorage === 'undefined' ? null : YouVersionPlatformConfiguration.accessToken; + if (!token) { + throw new Error(`Authentication required. Please provide a token or sign in before ${action}.`); + } + return token; +} diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 3b6b3848..9f3afe13 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -6,6 +6,19 @@ type QueryParams = Record; type RequestData = Record; type RequestHeaders = Record; +/** + * Returns the HTTP status code attached to an error thrown by an API client, + * or undefined when the error did not come from an HTTP response (network + * failure, timeout, validation error). This is the supported way to branch on + * status codes; the error's internal shape is not part of the public API. + */ +export function getHttpStatus(error: unknown): number | undefined { + if (error instanceof Error && 'status' in error && typeof error.status === 'number') { + return error.status; + } + return undefined; +} + /** * ApiClient is a lightweight HTTP client for interacting with the API using fetch. * It provides convenient methods for GET and POST requests with typed responses. diff --git a/packages/core/src/data-exchange.ts b/packages/core/src/data-exchange.ts new file mode 100644 index 00000000..155f3445 --- /dev/null +++ b/packages/core/src/data-exchange.ts @@ -0,0 +1,173 @@ +import { z } from 'zod'; +import type { ApiClient } from './client'; +import { YouVersionPlatformConfiguration } from './YouVersionPlatformConfiguration'; +import { resolveAuthToken } from './auth-token'; +import { parseGrantedPermissions } from './permissions'; + +/** + * Data exchange is YouVersion's just-in-time permission grant flow: a signed-in + * user who has not yet granted an app a data-exchange permission (e.g. + * `highlights`) is sent to a hosted consent page, and returns with the grant. + * + * The browser flow is: + * 1. {@link DataExchangeClient.updateToken} mints a short-lived token + * (`POST /data-exchange/token`). + * 2. The app full-page redirects to {@link buildDataExchangeUrl}. + * 3. On return, the hosted page appends `data_exchange_status` and + * `granted_permissions`, parsed by {@link parseDataExchangeCallback}. + * + * Mirrors the Swift SDK's `YouVersionAPI.DataExchange` contract. + */ + +const DataExchangeTokenResponseSchema = z.object({ + token: z.string().min(1), +}); + +export class DataExchangeClient { + private client: ApiClient; + + constructor(client: ApiClient) { + this.client = client; + } + + /** + * Reads the auth token from the argument or the ambient platform + * configuration, mirroring {@link HighlightsClient}. Server-side callers + * (no `localStorage`) must pass `lat` explicitly. + */ + private getAuthToken(lat?: string): string { + return resolveAuthToken(lat, 'requesting a data exchange'); + } + + /** + * Mints a short-lived data-exchange token for the given permissions. + * + * `POST https:///data-exchange/token?app-key=` with + * `Authorization: Bearer ` and body + * `{"requested_permissions": [...]}`. Expects `201 { "token": "..." }`. + * + * @throws when no app key or access token is available, or the server + * responds with a non-2xx status (401 = not permitted), or the response + * fails schema validation. + */ + async updateToken(permissions: string[], lat?: string): Promise { + const appKey = this.client.config.appKey; + if (!appKey) { + throw new Error('App key is required to request a data exchange token.'); + } + + const response = await this.client.post( + `/data-exchange/token`, + { requested_permissions: [...permissions].sort() }, + { 'app-key': appKey }, + { Authorization: `Bearer ${this.getAuthToken(lat)}` }, + ); + + const parsed = DataExchangeTokenResponseSchema.safeParse(response); + if (!parsed.success) { + throw new Error(`Unexpected data exchange token response: ${parsed.error.message}`); + } + return parsed.data.token; + } +} + +/** + * Builds the hosted data-exchange consent URL the app full-page redirects to. + * Matches the Swift SDK: `token`, `app_key`, and `x-yvp-app-key` query params. + */ +export function buildDataExchangeUrl( + token: string, + appKey: string, + apiHost: string = YouVersionPlatformConfiguration.apiHost, +): string { + const url = new URL(`https://${apiHost}/data-exchange`); + url.searchParams.set('token', token); + url.searchParams.set('app_key', appKey); + url.searchParams.set('x-yvp-app-key', appKey); + return url.toString(); +} + +/** + * The query params the data-exchange flow appends to the callback URL (parsed by + * {@link parseDataExchangeCallback}). {@link handleDataExchangeCallback} strips + * exactly these on cleanup, leaving any unrelated app params untouched. + */ +const DATA_EXCHANGE_CALLBACK_PARAMS = ['data_exchange_status', 'granted_permissions'] as const; + +export type DataExchangeStatus = 'granted' | 'cancel' | 'failure'; + +export type DataExchangeCallbackResult = { + status: DataExchangeStatus; + grantedPermissions: string[]; +}; + +/** + * Parses a data-exchange return from a URL query string. Pure — no DOM. Returns + * `null` when the query has no `data_exchange_status` (i.e. not a data-exchange + * return). `data_exchange_status` maps `granted`/`cancel` verbatim; anything + * else (including a missing value) is treated as `failure`. + */ +export function parseDataExchangeCallback(search: string): DataExchangeCallbackResult | null { + const params = new URLSearchParams(search); + if (!params.has('data_exchange_status')) return null; + const raw = params.get('data_exchange_status'); + const status: DataExchangeStatus = + raw === 'granted' ? 'granted' : raw === 'cancel' ? 'cancel' : 'failure'; + return { status, grantedPermissions: parseGrantedPermissions(params) }; +} + +/** + * Browser entry point for handling a data-exchange return on page load. Reads + * `window.location.search`; on a `granted` return it reconciles the permission + * cache with the server-reported `granted_permissions`, then strips the query + * params (mirroring the sign-in callback cleanup). Returns the parsed result, or + * `null` when the current URL is not a data-exchange return. + * + * Cleanup surgically removes only the data-exchange params, preserving any + * unrelated app query params and the hash fragment. + * + * Grant safety: the just-in-time data-exchange flow is a fire-and-forget + * full-page redirect, so the user signed in when this callback loads is not + * guaranteed to be the one who started the flow (e.g. user B signs in on another + * tab mid-redirect). The grant is only saved when the initiator recorded at + * {@link YouVersionPlatformConfiguration.saveDataExchangeInitiator} still matches + * the current user. Any mismatch — a different user, or a missing initiator (a + * legacy pending state or a return we did not originate) — fails closed: the + * grant is discarded and the result is downgraded to a `failure` so callers + * re-prompt instead of proceeding as granted. Dropping a legitimate grant is + * harmless — it just re-prompts — whereas saving one for the wrong user is not. + * + * The sign-in callback (see `Users.ts`) needs no such check: it derives the user + * profile from the same ID token that carries `granted_permissions`, so the + * grant is inherently bound to the correct user. + */ +export function handleDataExchangeCallback(): DataExchangeCallbackResult | null { + if (typeof window === 'undefined') return null; + const parsed = parseDataExchangeCallback(window.location.search); + if (!parsed) return null; + + // Read and clear the initiator up front so a stale value can never authorize a + // later, unrelated return. + const initiator = YouVersionPlatformConfiguration.dataExchangeInitiator; + YouVersionPlatformConfiguration.clearDataExchangeInitiator(); + + let result = parsed; + if (parsed.status === 'granted' && parsed.grantedPermissions.length > 0) { + const currentUserId = YouVersionPlatformConfiguration.storedUserInfo?.id ?? null; + if (initiator && currentUserId && initiator === currentUserId) { + YouVersionPlatformConfiguration.saveGrantedPermissions(parsed.grantedPermissions); + } else { + // Initiator missing or a different user is signed in: discard the grant + // and degrade to a failed exchange. + result = { status: 'failure', grantedPermissions: [] }; + } + } + + const cleanUrl = new URL(window.location.href); + for (const param of DATA_EXCHANGE_CALLBACK_PARAMS) { + cleanUrl.searchParams.delete(param); + } + window.history.replaceState({}, '', cleanUrl.toString()); + + return result; +} diff --git a/packages/core/src/highlights.ts b/packages/core/src/highlights.ts index c1f3354a..a9459354 100644 --- a/packages/core/src/highlights.ts +++ b/packages/core/src/highlights.ts @@ -1,11 +1,10 @@ import { z } from 'zod'; import type { ApiClient } from './client'; import type { Collection, Highlight, CreateHighlight } from './types'; -import { YouVersionPlatformConfiguration } from './YouVersionPlatformConfiguration'; +import { resolveAuthToken } from './auth-token'; import { HighlightCollectionWireSchema, HighlightWireSchema, - RecentHighlightColorsWireSchema, toHighlight, } from './schemas/highlight'; @@ -58,21 +57,7 @@ export class HighlightsClient { * @throws Error if no token is available. */ private getAuthToken(lat?: string): string { - if (lat) { - return lat; - } - // The implicit token fallback reads from `YouVersionPlatformConfiguration`, - // which is backed by browser `localStorage`. In any environment without it - // (Node.js tests, SSR) there is intentionally no ambient token: server-side - // callers must pass `lat` explicitly rather than relying on this fallback. - const token = - typeof localStorage === 'undefined' ? null : YouVersionPlatformConfiguration.accessToken; - if (!token) { - throw new Error( - 'Authentication required. Please provide a token or sign in before accessing highlights.', - ); - } - return token; + return resolveAuthToken(lat, 'accessing highlights'); } private authHeaders(lat?: string): Record { @@ -163,33 +148,6 @@ export class HighlightsClient { }; } - /** - * Fetches the user's recently used highlight colors, followed by the default - * colors, as hex strings (no `#`). Useful for building a color picker. - * Requires OAuth with read_highlights scope. - * @param lat Optional long access token. If not provided, retrieves from YouVersionPlatformConfiguration. - * @returns An ordered list of hex color strings. - */ - async getRecentColors(lat?: string): Promise { - const response = await this.client.get( - `/v1/highlights/recent-colors`, - undefined, - this.authHeaders(lat), - ); - - // The API returns 204 (empty body) when there is nothing to return. - if (response === '' || response == null) { - return []; - } - - const parsed = RecentHighlightColorsWireSchema.safeParse(response); - if (!parsed.success) { - throw new Error(`Unexpected highlights API response: ${parsed.error.message}`); - } - - return parsed.data.data.map((entry) => entry.color); - } - /** * Creates or updates a highlight on a passage. * Verse ranges may be used in the passage_id attribute. @@ -230,7 +188,13 @@ export class HighlightsClient { /** * Clears highlights for a passage. * Requires OAuth with write_highlights scope. - * @param passageId The passage identifier (USFM format, e.g., "MAT.1.1" or "MAT.1.1-5"). + * + * UNVERIFIED / likely unsupported: passing a verse RANGE (e.g. "MAT.1.1-5") + * returned a non-2xx from staging (observed with `JHN.1.2-3`), even though the + * API stores highlights per verse and POST accepts ranges. Until the API team + * confirms range delete, callers should send one DELETE per verse passage-id + * (e.g. "MAT.1.1"). See YPE-1034. + * @param passageId The passage identifier (single-verse USFM, e.g., "MAT.1.1"). * @param options Query parameters; `version_id` is required by the API (sent as `bible_id`). * @param lat Optional long access token. If not provided, retrieves from YouVersionPlatformConfiguration. * @returns Promise that resolves when highlights are deleted (204 response). diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 884b741b..15359732 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,4 +1,4 @@ -export { ApiClient } from './client'; +export { ApiClient, getHttpStatus } from './client'; export { BibleClient } from './bible'; export { LanguagesClient, type GetLanguagesOptions } from './languages'; export { OrganizationsClient } from './organizations'; @@ -7,6 +7,15 @@ export { type GetHighlightsOptions, type DeleteHighlightOptions, } from './highlights'; +export { + DataExchangeClient, + buildDataExchangeUrl, + parseDataExchangeCallback, + handleDataExchangeCallback, + type DataExchangeStatus, + type DataExchangeCallbackResult, +} from './data-exchange'; +export { parseGrantedPermissions } from './permissions'; export * from './StorageStrategy'; export * from './Users'; export * from './YouVersionUserInfo'; diff --git a/packages/core/src/permissions.ts b/packages/core/src/permissions.ts new file mode 100644 index 00000000..bc77c918 --- /dev/null +++ b/packages/core/src/permissions.ts @@ -0,0 +1,27 @@ +/** + * YouVersion data-exchange permissions are open-ended strings (not enums): the + * `highlights` permission ships now, more (e.g. verse notes) arrive later. The + * server communicates granted permissions back to the app on the sign-in and + * data-exchange callbacks via `granted_permissions` query param(s). + * + * These helpers are pure (no DOM/storage), so they can be unit-tested and reused + * by both the sign-in callback handler and the data-exchange callback handler. + */ + +/** + * Parses `granted_permissions` from a set of callback query params. + * + * The param may repeat and each value may pack several permissions separated by + * a comma or whitespace (mirrors the Swift SDK's split on `,`/` `). Returns a + * de-duplicated list, order-preserving on first appearance. + */ +export function parseGrantedPermissions(params: URLSearchParams): string[] { + const seen = new Set(); + for (const value of params.getAll('granted_permissions')) { + for (const part of value.split(/[,\s]+/)) { + const trimmed = part.trim(); + if (trimmed) seen.add(trimmed); + } + } + return [...seen]; +} diff --git a/packages/core/src/schemas/highlight.ts b/packages/core/src/schemas/highlight.ts index 298cc401..a935b80c 100644 --- a/packages/core/src/schemas/highlight.ts +++ b/packages/core/src/schemas/highlight.ts @@ -27,16 +27,6 @@ export const HighlightCollectionWireSchema = z.object({ export type HighlightCollectionWire = z.infer; -/** - * Wire format for `GET /v1/highlights/recent-colors`, a collection of hex color - * strings (recently used followed by default colors). - */ -export const RecentHighlightColorsWireSchema = z.object({ - data: z.array(z.object({ color: z.string().regex(HEX_COLOR_REGEX) })), -}); - -export type RecentHighlightColorsWire = z.infer; - const _HighlightSchema = z.object({ /** Bible version identifier (sent to / received from the API as `bible_id`) */ version_id: z.number().int().positive(), diff --git a/packages/core/src/styles/bible-reader.css b/packages/core/src/styles/bible-reader.css index cc0c289b..9c34a632 100644 --- a/packages/core/src/styles/bible-reader.css +++ b/packages/core/src/styles/bible-reader.css @@ -123,6 +123,20 @@ display: inline; } + /* Fade highlight fills in/out instead of popping. The fill is painted as an + inline `background-color` on `.yv-v[v]` (see verse.tsx); transitioning only + that property animates apply/remove without touching the selection + underline (text-decoration) and without any layout shift. */ + & .yv-v { + transition: background-color 250ms ease; + } + + @media (prefers-reduced-motion: reduce) { + & .yv-v { + transition: none; + } + } + /* Only show pointer cursor when verses are selectable */ &[data-selectable='true'] .yv-v, &[data-selectable='true'] .verse { diff --git a/packages/hooks/src/context/index.ts b/packages/hooks/src/context/index.ts index e2183601..6f4da546 100644 --- a/packages/hooks/src/context/index.ts +++ b/packages/hooks/src/context/index.ts @@ -1,2 +1,6 @@ export * from './YouVersionContext'; export * from './YouVersionProvider'; +// The raw auth context (no-throw alternative to `useYVAuth` for consumers that +// must tolerate a missing auth provider). Its own error message already +// advertises it as importable from this package. +export { YouVersionAuthContext } from './YouVersionAuthContext'; diff --git a/packages/hooks/src/index.ts b/packages/hooks/src/index.ts index c6b39d73..0d44d571 100644 --- a/packages/hooks/src/index.ts +++ b/packages/hooks/src/index.ts @@ -17,6 +17,7 @@ export * from './useBibleClient'; export * from './usePassage'; export * from './useVOTD'; export * from './useHighlights'; +export * from './useHighlightAuthActions'; export * from './useLanguages'; export * from './useLanguage'; export * from './useTheme'; diff --git a/packages/hooks/src/internal/useApiClient.ts b/packages/hooks/src/internal/useApiClient.ts new file mode 100644 index 00000000..03f4efcf --- /dev/null +++ b/packages/hooks/src/internal/useApiClient.ts @@ -0,0 +1,44 @@ +'use client'; + +import { useContext, useMemo } from 'react'; +import { ApiClient } from '@youversion/platform-core'; +import { YouVersionContext } from '../context'; + +/** + * Reads {@link YouVersionContext} and returns a memoized {@link ApiClient} built + * from its config. Shared by the per-service client hooks so the client + * construction (and its dependency array) lives in one place. + * + * By default it throws when no app key is configured (the contract every data + * hook relies on). Pass `{ optional: true }` for the no-provider-tolerant + * variant that returns `null` instead — used by `useHighlightAuthActions`, + * which must not throw when no provider is mounted. + */ +export function useApiClient(options: { optional: true }): ApiClient | null; +export function useApiClient(options?: { optional?: false }): ApiClient; +export function useApiClient(options: { optional?: boolean } = {}): ApiClient | null { + const context = useContext(YouVersionContext); + const optional = options.optional ?? false; + + return useMemo(() => { + if (!context?.appKey) { + if (optional) return null; + throw new Error( + 'YouVersion context not found. Make sure your component is wrapped with YouVersionProvider and an API key is provided.', + ); + } + + return new ApiClient({ + appKey: context.appKey, + apiHost: context.apiHost, + installationId: context.installationId, + additionalHeaders: context.additionalHeaders, + }); + }, [ + context?.appKey, + context?.apiHost, + context?.installationId, + context?.additionalHeaders, + optional, + ]); +} diff --git a/packages/hooks/src/useApiData.test.tsx b/packages/hooks/src/useApiData.test.tsx new file mode 100644 index 00000000..ca122c64 --- /dev/null +++ b/packages/hooks/src/useApiData.test.tsx @@ -0,0 +1,216 @@ +/** + * @vitest-environment jsdom + */ +import { act, renderHook, waitFor } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { useApiData, type UseApiDataOptions } from './useApiData'; + +type Deferred = { + promise: Promise; + resolve: (value: T) => void; + reject: (reason: unknown) => void; +}; + +function createDeferred(): Deferred { + let resolve!: (value: T) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +describe('useApiData — enabled transitions', () => { + it('fetches when enabled flips from false to true with unchanged deps', async () => { + const fetchFn = vi.fn().mockResolvedValue('payload'); + + const { result, rerender } = renderHook( + ({ enabled }: { enabled: boolean }) => useApiData(fetchFn, ['stable-dep'], { enabled }), + { initialProps: { enabled: false } }, + ); + + // Disabled on mount (e.g. auth still resolving): no request goes out. + expect(fetchFn).not.toHaveBeenCalled(); + expect(result.current.loading).toBe(false); + expect(result.current.data).toBeNull(); + + // Auth resolves: enabled flips true while the caller's deps are unchanged. + rerender({ enabled: true }); + + await waitFor(() => { + expect(result.current.data).toBe('payload'); + }); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + + it('clears data (without refetching) when enabled flips from true to false', async () => { + const fetchFn = vi.fn().mockResolvedValue('user-a-data'); + + const { result, rerender } = renderHook( + ({ enabled }: { enabled: boolean }) => useApiData(fetchFn, ['stable-dep'], { enabled }), + { initialProps: { enabled: true } }, + ); + + await waitFor(() => { + expect(result.current.data).toBe('user-a-data'); + }); + + // Sign-out (or auth switching users): stale account data must not linger. + rerender({ enabled: false }); + + expect(result.current.data).toBeNull(); + expect(result.current.error).toBeNull(); + expect(result.current.loading).toBe(false); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); +}); + +describe('useApiData — stale responses (latest wins)', () => { + it('ignores a stale refetch response that resolves after a newer fetch', async () => { + const deferreds: Deferred[] = []; + const fetchFn = vi.fn(() => { + const deferred = createDeferred(); + deferreds.push(deferred); + return deferred.promise; + }); + + const { result, rerender } = renderHook( + ({ scope }: { scope: string }) => useApiData(fetchFn, [scope]), + { initialProps: { scope: 'JHN.3' } }, + ); + + // Initial fetch for JHN.3 resolves normally. + await act(async () => { + deferreds[0]!.resolve('JHN.3 data'); + await Promise.resolve(); + }); + expect(result.current.data).toBe('JHN.3 data'); + + // A refetch for JHN.3 goes out (e.g. after a highlight write)… + act(() => { + result.current.refetch(); + }); + expect(deferreds).toHaveLength(2); + + // …then the user navigates to JHN.4, whose fetch resolves first. + rerender({ scope: 'JHN.4' }); + expect(deferreds).toHaveLength(3); + await act(async () => { + deferreds[2]!.resolve('JHN.4 data'); + await Promise.resolve(); + }); + expect(result.current.data).toBe('JHN.4 data'); + + // The stale JHN.3 refetch finally lands — it must not clobber JHN.4. + await act(async () => { + deferreds[1]!.resolve('stale JHN.3 data'); + await Promise.resolve(); + }); + expect(result.current.data).toBe('JHN.4 data'); + expect(result.current.loading).toBe(false); + }); + + it('ignores a stale error from an invalidated request', async () => { + const deferreds: Deferred[] = []; + const fetchFn = vi.fn(() => { + const deferred = createDeferred(); + deferreds.push(deferred); + return deferred.promise; + }); + + const { result, rerender } = renderHook( + ({ scope }: { scope: string }) => useApiData(fetchFn, [scope]), + { initialProps: { scope: 'JHN.3' } }, + ); + + // Navigate away while the first request is still in flight. + rerender({ scope: 'JHN.4' }); + await act(async () => { + deferreds[1]!.resolve('JHN.4 data'); + await Promise.resolve(); + }); + expect(result.current.data).toBe('JHN.4 data'); + + // The abandoned JHN.3 request fails late — the error must not surface. + await act(async () => { + deferreds[0]!.reject(new Error('stale failure')); + await Promise.resolve(); + }); + expect(result.current.error).toBeNull(); + expect(result.current.data).toBe('JHN.4 data'); + }); +}); + +describe('useApiData — existing behavior', () => { + it('does not fetch when enabled is false for the whole lifetime', () => { + const fetchFn = vi.fn().mockResolvedValue('never'); + const options: UseApiDataOptions = { enabled: false }; + + const { result } = renderHook(() => useApiData(fetchFn, ['dep'], options)); + + expect(fetchFn).not.toHaveBeenCalled(); + expect(result.current.loading).toBe(false); + expect(result.current.data).toBeNull(); + }); + + it('keeps a stable refetch identity across re-renders with an inline fetchFn', async () => { + // Callers pass a fresh inline arrow every render; refetch must not churn. + const { result, rerender } = renderHook( + ({ scope }: { scope: string }) => useApiData(() => Promise.resolve(scope), ['stable-dep']), + { initialProps: { scope: 'a' } }, + ); + + await waitFor(() => { + expect(result.current.data).toBe('a'); + }); + + const firstRefetch = result.current.refetch; + rerender({ scope: 'b' }); + rerender({ scope: 'c' }); + + expect(result.current.refetch).toBe(firstRefetch); + }); + + it('refetch uses the latest inline fetchFn after re-renders', async () => { + // The ref must hold the newest closure so a refetch reflects current props. + const { result, rerender } = renderHook( + ({ scope }: { scope: string }) => useApiData(() => Promise.resolve(scope), ['stable-dep']), + { initialProps: { scope: 'a' } }, + ); + + await waitFor(() => { + expect(result.current.data).toBe('a'); + }); + + // Deps unchanged, so no effect-driven refetch; only the closure updates. + rerender({ scope: 'b' }); + + act(() => { + result.current.refetch(); + }); + + await waitFor(() => { + expect(result.current.data).toBe('b'); + }); + }); + + it('refetches on demand', async () => { + const fetchFn = vi.fn().mockResolvedValueOnce('first').mockResolvedValueOnce('second'); + + const { result } = renderHook(() => useApiData(fetchFn, ['dep'])); + + await waitFor(() => { + expect(result.current.data).toBe('first'); + }); + + act(() => { + result.current.refetch(); + }); + + await waitFor(() => { + expect(result.current.data).toBe('second'); + }); + expect(fetchFn).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/hooks/src/useApiData.ts b/packages/hooks/src/useApiData.ts index 91f95a32..1969668c 100644 --- a/packages/hooks/src/useApiData.ts +++ b/packages/hooks/src/useApiData.ts @@ -1,6 +1,6 @@ 'use client'; -import { useState, useEffect, useCallback } from 'react'; +import { useState, useEffect, useCallback, useRef } from 'react'; export type UseApiDataOptions = { enabled?: boolean; @@ -24,48 +24,72 @@ export function useApiData( const [loading, setLoading] = useState(true); const [error, setError] = useState(null); + // Monotonic sequence per issued request: only the latest-issued request may + // commit state. This covers refetch-initiated requests too, which a + // per-effect cancel closure would miss — a stale refetch (e.g. for a + // previous chapter) resolving after a newer fetch must not overwrite it. + const requestSeqRef = useRef(0); + + // Hold the (typically inline) `fetchFn` in a ref, refreshed every render, so + // it can stay out of `fetchData`'s deps below. Otherwise a new closure each + // render would churn `fetchData` — and therefore `refetch` — identity every + // render. `fetchData` reads the ref only when a fetch actually starts, and + // the ref is updated during render before any effect runs, so a dep-change + // fetch still uses the latest `fetchFn`. + const fetchFnRef = useRef(fetchFn); + fetchFnRef.current = fetchFn; + const fetchData = useCallback(() => { + const requestSeq = ++requestSeqRef.current; + if (!enabled) { + // Disabling drops previously fetched data instead of keeping it: the + // usual reason to disable is that the data must no longer be shown + // (signed out, auth switched to a different user), and leaking stale + // account data across sessions is worse than a refetch on re-enable. + setData(null); setLoading(false); + setError(null); return; } - let canceled = false; - setLoading(true); setError(null); - fetchFn() + fetchFnRef + .current() .then((result) => { - if (!canceled) { + if (requestSeq === requestSeqRef.current) { setData(result); } }) .catch((err) => { - if (!canceled) { + if (requestSeq === requestSeqRef.current) { setError(err as Error); } }) .finally(() => { - if (!canceled) { + if (requestSeq === requestSeqRef.current) { setLoading(false); } }); - - return () => { - canceled = true; - }; - }, [fetchFn, enabled]); + }, [enabled]); const refetch = useCallback(() => { fetchData(); }, [fetchData]); + // `enabled` rides alongside the caller-supplied deps so a false→true flip + // (e.g. auth resolving after mount, with the caller's deps unchanged) + // actually triggers the fetch. useEffect(() => { - const cleanup = fetchData(); - return cleanup; - // @eslint-disable-next-line react-hooks/exhaustive-deps - }, deps); + fetchData(); + return () => { + // Invalidate any in-flight request (effect- or refetch-initiated) when + // the deps change or the component unmounts. + requestSeqRef.current++; + }; + }, [...deps, enabled]); return { data, loading, error, refetch }; } diff --git a/packages/hooks/src/useBibleClient.ts b/packages/hooks/src/useBibleClient.ts index 1673f47f..3dbd064b 100644 --- a/packages/hooks/src/useBibleClient.ts +++ b/packages/hooks/src/useBibleClient.ts @@ -1,26 +1,10 @@ 'use client'; -import { useContext, useMemo } from 'react'; -import { YouVersionContext } from './context'; -import { BibleClient, ApiClient } from '@youversion/platform-core'; +import { useMemo } from 'react'; +import { BibleClient } from '@youversion/platform-core'; +import { useApiClient } from './internal/useApiClient'; export function useBibleClient(): BibleClient { - const context = useContext(YouVersionContext); - - return useMemo(() => { - if (!context?.appKey) { - throw new Error( - 'YouVersion context not found. Make sure your component is wrapped with YouVersionProvider and an API key is provided.', - ); - } - - return new BibleClient( - new ApiClient({ - appKey: context.appKey, - apiHost: context.apiHost, - installationId: context.installationId, - additionalHeaders: context.additionalHeaders, - }), - ); - }, [context?.apiHost, context?.appKey, context?.installationId, context?.additionalHeaders]); + const apiClient = useApiClient(); + return useMemo(() => new BibleClient(apiClient), [apiClient]); } diff --git a/packages/hooks/src/useHighlightAuthActions.test.tsx b/packages/hooks/src/useHighlightAuthActions.test.tsx new file mode 100644 index 00000000..00b683d2 --- /dev/null +++ b/packages/hooks/src/useHighlightAuthActions.test.tsx @@ -0,0 +1,85 @@ +import { renderHook } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { ReactNode } from 'react'; +import { + DataExchangeClient, + YouVersionAPIUsers, + YouVersionPlatformConfiguration, +} from '@youversion/platform-core'; +import { YouVersionContext } from './context'; +import { YouVersionAuthContext } from './context/YouVersionAuthContext'; +import { useHighlightAuthActions } from './useHighlightAuthActions'; + +function wrapper({ children }: { children: ReactNode }) { + return ( + + + {children} + + + ); +} + +describe('useHighlightAuthActions', () => { + beforeEach(() => { + localStorage.clear(); + // A writable location stub so href assignment doesn't hit jsdom navigation. + Object.defineProperty(window, 'location', { + value: { href: 'https://host.example/read' }, + writable: true, + configurable: true, + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('reads and invalidates the highlights permission cache', () => { + // The permission cache is scoped to the signed-in user, so establish one. + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-1' }); + const { result } = renderHook(() => useHighlightAuthActions(), { wrapper }); + + expect(result.current.hasHighlightsPermission()).toBe(false); + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + expect(result.current.hasHighlightsPermission()).toBe(true); + + result.current.invalidateHighlightsPermission(); + expect(result.current.hasHighlightsPermission()).toBe(false); + }); + + it('starts sign-in requesting profile + highlights via the configured redirect', async () => { + const signIn = vi.spyOn(YouVersionAPIUsers, 'signIn').mockResolvedValue(undefined); + const { result } = renderHook(() => useHighlightAuthActions(), { wrapper }); + + await result.current.startSignInForHighlights(); + + expect(signIn).toHaveBeenCalledWith( + 'https://host.example/callback', + ['profile'], + ['highlights'], + ); + }); + + it('mints a data-exchange token then redirects to the hosted consent page', async () => { + const updateToken = vi + .spyOn(DataExchangeClient.prototype, 'updateToken') + .mockResolvedValue('dx-token'); + const { result } = renderHook(() => useHighlightAuthActions(), { wrapper }); + + await result.current.startDataExchangeForHighlights(); + + expect(updateToken).toHaveBeenCalledWith(['highlights']); + expect(window.location.href).toContain('https://api.example.com/data-exchange'); + expect(window.location.href).toContain('token=dx-token'); + expect(window.location.href).toContain('app_key=app-1'); + }); +}); diff --git a/packages/hooks/src/useHighlightAuthActions.ts b/packages/hooks/src/useHighlightAuthActions.ts new file mode 100644 index 00000000..ce1140f3 --- /dev/null +++ b/packages/hooks/src/useHighlightAuthActions.ts @@ -0,0 +1,107 @@ +'use client'; + +import { useCallback, useContext, useMemo } from 'react'; +import { + DataExchangeClient, + buildDataExchangeUrl, + handleDataExchangeCallback, + SignInWithYouVersionPermission, + YouVersionAPIUsers, + YouVersionPlatformConfiguration, + type DataExchangeCallbackResult, +} from '@youversion/platform-core'; +import { YouVersionContext } from './context'; +import { YouVersionAuthContext } from './context/YouVersionAuthContext'; +import { useApiClient } from './internal/useApiClient'; + +const HIGHLIGHTS_PERMISSION = SignInWithYouVersionPermission.highlights; + +/** + * The two-path auth actions the highlight auth flow (YPE-1034) needs, plus the + * permission-cache reads. Kept UI-agnostic: it returns primitives the seam hook + * (`useBibleReaderHighlights`) orchestrates; it renders nothing. + * + * Reads `YouVersionAuthContext` defensively (via `useContext`, not `useYVAuth`) + * so it never throws when no auth provider is mounted — the seam hook treats "no + * provider" and "signed out" the same and simply never invokes the redirects. + */ +export function useHighlightAuthActions(): { + /** Optimistic cache read; a 401/403 on a write is still the ultimate check. */ + hasHighlightsPermission: () => boolean; + /** Drops the cached `highlights` grant so the next attempt re-prompts. */ + invalidateHighlightsPermission: () => void; + /** + * Reconciles the permission cache from a data-exchange return and cleans the + * URL. Returns the parsed return (or `null` when this load is not one). + */ + consumeDataExchangeReturn: () => DataExchangeCallbackResult | null; + /** + * One-fell-swoop: full-page redirect to sign-in, requesting the `highlights` + * permission alongside `profile`. Returns to `redirectUrl` (or the auth + * provider's configured `redirectUri`, falling back to the current URL). + */ + startSignInForHighlights: (redirectUrl?: string) => Promise; + /** + * Just-in-time: mint a data-exchange token then full-page redirect to the + * hosted consent page for the `highlights` permission. + */ + startDataExchangeForHighlights: () => Promise; +} { + const context = useContext(YouVersionContext); + const authContext = useContext(YouVersionAuthContext); + const redirectUri = authContext?.redirectUri; + + const apiClient = useApiClient({ optional: true }); + const dataExchangeClient = useMemo( + () => (apiClient ? new DataExchangeClient(apiClient) : null), + [apiClient], + ); + + const hasHighlightsPermission = useCallback( + () => YouVersionPlatformConfiguration.hasPermission(HIGHLIGHTS_PERMISSION), + [], + ); + + const invalidateHighlightsPermission = useCallback( + () => YouVersionPlatformConfiguration.removeGrantedPermission(HIGHLIGHTS_PERMISSION), + [], + ); + + const consumeDataExchangeReturn = useCallback(() => handleDataExchangeCallback(), []); + + const startSignInForHighlights = useCallback( + async (redirectUrl?: string) => { + const url = + redirectUrl ?? + redirectUri ?? + (typeof window !== 'undefined' ? window.location.href : undefined); + if (!url) { + throw new Error('A redirect URL is required to start sign-in for highlights.'); + } + await YouVersionAPIUsers.signIn(url, ['profile'], [HIGHLIGHTS_PERMISSION]); + }, + [redirectUri], + ); + + const startDataExchangeForHighlights = useCallback(async () => { + if (!dataExchangeClient || !context?.appKey) { + throw new Error('YouVersion context is required to start a data exchange.'); + } + const token = await dataExchangeClient.updateToken([HIGHLIGHTS_PERMISSION]); + // Record who started the flow so the callback only saves the grant if the + // same user is still signed in when it returns (guards against a different + // user signing in on another tab mid-redirect). + YouVersionPlatformConfiguration.saveDataExchangeInitiator(); + if (typeof window !== 'undefined') { + window.location.href = buildDataExchangeUrl(token, context.appKey, context.apiHost); + } + }, [dataExchangeClient, context?.appKey, context?.apiHost]); + + return { + hasHighlightsPermission, + invalidateHighlightsPermission, + consumeDataExchangeReturn, + startSignInForHighlights, + startDataExchangeForHighlights, + }; +} diff --git a/packages/hooks/src/useHighlights.test.tsx b/packages/hooks/src/useHighlights.test.tsx index 7e8a456e..ed65c255 100644 --- a/packages/hooks/src/useHighlights.test.tsx +++ b/packages/hooks/src/useHighlights.test.tsx @@ -54,20 +54,17 @@ describe('useHighlights', () => { let mockGetHighlights: Mock; let mockCreateHighlight: Mock; let mockDeleteHighlight: Mock; - let mockGetRecentColors: Mock; beforeEach(() => { mockGetHighlights = vi.fn().mockResolvedValue(mockHighlights); mockCreateHighlight = vi.fn().mockResolvedValue(mockHighlight); mockDeleteHighlight = vi.fn().mockResolvedValue(undefined); - mockGetRecentColors = vi.fn().mockResolvedValue(['fffe00', '5dff79']); (HighlightsClient as unknown as ReturnType).mockImplementation(function () { return { getHighlights: mockGetHighlights, createHighlight: mockCreateHighlight, deleteHighlight: mockDeleteHighlight, - getRecentColors: mockGetRecentColors, }; }); @@ -221,13 +218,15 @@ describe('useHighlights', () => { }); describe('createHighlight mutation', () => { - it('should create highlight and refetch', async () => { + it('should create highlight WITHOUT auto-refetching (callers coalesce refetches)', async () => { const wrapper = createYVWrapper(); const { result } = renderHook(() => useHighlights(defaultOptions), { wrapper }); await waitFor(() => { expect(result.current.loading).toBe(false); }); + // The mount fetch. + expect(mockGetHighlights).toHaveBeenCalledTimes(1); const createData: CreateHighlight = { version_id: 111, @@ -244,6 +243,13 @@ describe('useHighlights', () => { const created = await createPromise; expect(created).toEqual(mockHighlight); + // No implicit GET after the write — the mount fetch is still the only one. + // A batching caller issues one refetch() after the whole batch settles. + await Promise.resolve(); + expect(mockGetHighlights).toHaveBeenCalledTimes(1); + + // An explicit refetch still works and issues exactly one GET. + result.current.refetch(); await waitFor(() => { expect(mockGetHighlights).toHaveBeenCalledTimes(2); }); @@ -275,13 +281,14 @@ describe('useHighlights', () => { }); describe('deleteHighlight mutation', () => { - it('should delete highlight and refetch', async () => { + it('should delete highlight WITHOUT auto-refetching (callers coalesce refetches)', async () => { const wrapper = createYVWrapper(); const { result } = renderHook(() => useHighlights(defaultOptions), { wrapper }); await waitFor(() => { expect(result.current.loading).toBe(false); }); + expect(mockGetHighlights).toHaveBeenCalledTimes(1); const deletePromise = result.current.deleteHighlight('MAT.1.1', { version_id: 111 }); @@ -291,9 +298,9 @@ describe('useHighlights', () => { await deletePromise; - await waitFor(() => { - expect(mockGetHighlights).toHaveBeenCalledTimes(2); - }); + // No implicit GET after the delete. + await Promise.resolve(); + expect(mockGetHighlights).toHaveBeenCalledTimes(1); }); it('should handle delete error', async () => { @@ -314,18 +321,4 @@ describe('useHighlights', () => { expect(mockGetHighlights).toHaveBeenCalledTimes(1); }); }); - - describe('getRecentColors', () => { - it('delegates to the client and returns the color list', async () => { - const wrapper = createYVWrapper(); - const { result } = renderHook(() => useHighlights(defaultOptions), { wrapper }); - - await waitFor(() => { - expect(result.current.loading).toBe(false); - }); - - await expect(result.current.getRecentColors()).resolves.toEqual(['fffe00', '5dff79']); - expect(mockGetRecentColors).toHaveBeenCalledTimes(1); - }); - }); }); diff --git a/packages/hooks/src/useHighlights.ts b/packages/hooks/src/useHighlights.ts index 278067cb..d286cca8 100644 --- a/packages/hooks/src/useHighlights.ts +++ b/packages/hooks/src/useHighlights.ts @@ -1,9 +1,8 @@ 'use client'; import { useMemo, useCallback } from 'react'; -import { useContext } from 'react'; -import { YouVersionContext } from './context'; -import { HighlightsClient, ApiClient } from '@youversion/platform-core'; +import { HighlightsClient } from '@youversion/platform-core'; +import { useApiClient } from './internal/useApiClient'; import { useApiData, type UseApiDataOptions } from './useApiData'; import { type GetHighlightsOptions, @@ -23,25 +22,10 @@ export function useHighlights( refetch: () => void; createHighlight: (data: CreateHighlight) => Promise; deleteHighlight: (passageId: string, deleteOptions: DeleteHighlightOptions) => Promise; - getRecentColors: () => Promise; } { - const context = useContext(YouVersionContext); + const apiClient = useApiClient(); - const highlightsClient = useMemo(() => { - if (!context?.appKey) { - throw new Error( - 'YouVersion context not found. Make sure your component is wrapped with YouVersionProvider and an API key is provided.', - ); - } - return new HighlightsClient( - new ApiClient({ - appKey: context.appKey, - apiHost: context.apiHost, - installationId: context.installationId, - additionalHeaders: context.additionalHeaders, - }), - ); - }, [context?.apiHost, context?.appKey, context?.installationId, context?.additionalHeaders]); + const highlightsClient = useMemo(() => new HighlightsClient(apiClient), [apiClient]); // The dep array keys on the primitive fields of `options` rather than the // object reference, so an inline `{ version_id, passage_id }` literal doesn't @@ -55,25 +39,20 @@ export function useHighlights( }, ); + // NOTE: these mutations intentionally do NOT auto-refetch. A single logical + // apply/remove can fan out into several writes (one per contiguous run for + // apply, one per verse for delete); auto-refetching per call would issue a + // GET per write. Callers coalesce instead — issue the batch, then `refetch()` + // once after it settles. The seam hook (`useBibleReaderHighlights`) is the + // sole consumer and does exactly that. const createHighlight = useCallback( - async (data: CreateHighlight): Promise => { - const result = await highlightsClient.createHighlight(data); - refetch(); - return result; - }, - [highlightsClient, refetch], + (data: CreateHighlight): Promise => highlightsClient.createHighlight(data), + [highlightsClient], ); const deleteHighlight = useCallback( - async (passageId: string, deleteOptions: DeleteHighlightOptions): Promise => { - await highlightsClient.deleteHighlight(passageId, deleteOptions); - refetch(); - }, - [highlightsClient, refetch], - ); - - const getRecentColors = useCallback( - (): Promise => highlightsClient.getRecentColors(), + (passageId: string, deleteOptions: DeleteHighlightOptions): Promise => + highlightsClient.deleteHighlight(passageId, deleteOptions), [highlightsClient], ); @@ -84,6 +63,5 @@ export function useHighlights( refetch, createHighlight, deleteHighlight, - getRecentColors, }; } diff --git a/packages/hooks/src/useLanguageClient.ts b/packages/hooks/src/useLanguageClient.ts index 4fa6bd25..15e9ccf5 100644 --- a/packages/hooks/src/useLanguageClient.ts +++ b/packages/hooks/src/useLanguageClient.ts @@ -1,26 +1,10 @@ 'use client'; -import { useContext, useMemo } from 'react'; -import { YouVersionContext } from './context'; -import { ApiClient, LanguagesClient } from '@youversion/platform-core'; +import { useMemo } from 'react'; +import { LanguagesClient } from '@youversion/platform-core'; +import { useApiClient } from './internal/useApiClient'; export function useLanguagesClient(): LanguagesClient { - const context = useContext(YouVersionContext); - - return useMemo(() => { - if (!context?.appKey) { - throw new Error( - 'YouVersion context not found. Make sure your component is wrapped with YouVersionProvider and an API key is provided.', - ); - } - - return new LanguagesClient( - new ApiClient({ - appKey: context.appKey, - apiHost: context.apiHost, - installationId: context.installationId, - additionalHeaders: context.additionalHeaders, - }), - ); - }, [context?.apiHost, context?.appKey, context?.installationId, context?.additionalHeaders]); + const apiClient = useApiClient(); + return useMemo(() => new LanguagesClient(apiClient), [apiClient]); } diff --git a/packages/hooks/src/useOrganizationsClient.ts b/packages/hooks/src/useOrganizationsClient.ts index d1224d4f..2b642de9 100644 --- a/packages/hooks/src/useOrganizationsClient.ts +++ b/packages/hooks/src/useOrganizationsClient.ts @@ -1,26 +1,10 @@ 'use client'; -import { useContext, useMemo } from 'react'; -import { YouVersionContext } from './context'; -import { ApiClient, OrganizationsClient } from '@youversion/platform-core'; +import { useMemo } from 'react'; +import { OrganizationsClient } from '@youversion/platform-core'; +import { useApiClient } from './internal/useApiClient'; export function useOrganizationsClient(): OrganizationsClient { - const context = useContext(YouVersionContext); - - return useMemo(() => { - if (!context?.appKey) { - throw new Error( - 'YouVersion context not found. Make sure your component is wrapped with YouVersionProvider and an API key is provided.', - ); - } - - return new OrganizationsClient( - new ApiClient({ - appKey: context.appKey, - apiHost: context.apiHost, - installationId: context.installationId, - additionalHeaders: context.additionalHeaders, - }), - ); - }, [context?.apiHost, context?.appKey, context?.installationId, context?.additionalHeaders]); + const apiClient = useApiClient(); + return useMemo(() => new OrganizationsClient(apiClient), [apiClient]); } diff --git a/packages/ui/package.json b/packages/ui/package.json index 008f178d..b8c94271 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -51,14 +51,17 @@ "@radix-ui/react-slot": "1.2.3", "@radix-ui/react-tabs": "1.1.13", "@radix-ui/react-use-controllable-state": "^1.2.2", + "@xstate/react": "^6.1.0", "@youversion/platform-core": "workspace:*", "@youversion/platform-react-hooks": "workspace:*", + "better-result": "2.9.2", "class-variance-authority": "0.7.1", "clsx": "2.1.1", - "tailwind-merge": "3.3.1", "i18next": "^26.0.0", "react-i18next": "^17.0.0", - "tw-animate-css": "1.4.0" + "tailwind-merge": "3.3.1", + "tw-animate-css": "1.4.0", + "xstate": "5.32.4" }, "peerDependencies": { "react": ">=19.1.0 <20.0.0", diff --git a/packages/ui/src/components/YouVersionAuthButton.tsx b/packages/ui/src/components/YouVersionAuthButton.tsx index 019a4e43..1ae3bb5d 100644 --- a/packages/ui/src/components/YouVersionAuthButton.tsx +++ b/packages/ui/src/components/YouVersionAuthButton.tsx @@ -198,6 +198,11 @@ export const YouVersionAuthButton = React.forwardRef; + +/** hex color means "optimistically applied", `null` means "optimistically removed". */ +export type HighlightOverlay = Record; + +type ReconcileEntry = { op: 'apply' | 'remove'; color: string }; + +/** One serialized unit of work on the write queue. */ +type WriteOp = { + kind: 'apply' | 'remove'; + color: string; + verses: number[]; + scope: HighlightScope; + /** Per-verse ownership token; a failed write only reverts verses it still owns. */ + token: object; + /** Whether the optimistic overlay was painted for this op (owns its verses). */ + paint: boolean; +}; + +type WriteResult = { + op: WriteOp; + failures: BibleReaderHighlightError[]; + failedVerses: number[]; + succeededVerses: number[]; +}; + +/** + * The live SDK service functions the actors/actions call. Passed in once via + * `input` as a stable React ref so the machine spawn never changes yet always + * sees the current closures. + */ +export type HighlightServices = { + createHighlight: (data: { + version_id: number; + passage_id: string; + color: string; + }) => Promise; + deleteHighlight: (passageId: string, options: { version_id: number }) => Promise; + refetch: () => void; + hasHighlightsPermission: () => boolean; + invalidateHighlightsPermission: () => void; + consumeDataExchangeReturn: () => { status: string } | null; + startSignInForHighlights: () => Promise; + startDataExchangeForHighlights: () => Promise; +}; + +export type HighlightServicesRef = { current: HighlightServices }; + +export type HighlightsMachineInput = { + services: HighlightServicesRef; + scope: HighlightScope; + flagOn: boolean; + hasAuthProvider: boolean; + isAuthenticated: boolean; +}; + +export type TapOutcome = 'applied' | 'flow' | 'noop'; + +type HighlightsContext = { + services: HighlightServicesRef; + scope: HighlightScope; + flagOn: boolean; + hasAuthProvider: boolean; + isAuthenticated: boolean; + serverColors: ServerColors; + overlay: HighlightOverlay; + reconcile: Map; + writeIntent: Map; + queue: WriteOp[]; + dataExchangeConsumed: boolean; + /** Set by the TAP_COLOR handlers so the adapter can read the synchronous outcome. */ + lastTapOutcome: TapOutcome; +}; + +export type HighlightsEvent = + | { type: 'TAP_COLOR'; color: string; verses: number[] } + | { type: 'REMOVE'; color: string; verses: number[] } + | { + type: 'AUTH_CHANGED'; + flagOn: boolean; + hasAuthProvider: boolean; + isAuthenticated: boolean; + } + | { type: 'HIGHLIGHTS_UPDATED'; serverColors: ServerColors } + | { type: 'SCOPE_CHANGED'; scope: HighlightScope } + | { type: 'CONFIRM_SIGN_IN' } + | { type: 'DECLINE_SIGN_IN' } + | { type: 'CONFIRM_PERMISSION' } + | { type: 'CANCEL_PERMISSION' } + | { type: 'ENQUEUE'; op: WriteOp } + | { type: 'PERMISSION_LOST' }; + +// ── Error boundary + helpers (module-level, reused by the write actor) ──────── + +/** + * The error type the write boundary converts to. Core clients throw; we catch + * here (via better-result) so a failed write can never take the reader down — + * the failure is logged and the optimistic overlay reverted. + */ +export class BibleReaderHighlightError extends Error { + readonly operation: 'apply' | 'remove'; + readonly passageId: string; + readonly cause: unknown; + + constructor(operation: 'apply' | 'remove', passageId: string, cause: unknown) { + super(`Highlight ${operation} failed for ${passageId}`); + this.name = 'BibleReaderHighlightError'; + this.operation = operation; + this.passageId = passageId; + this.cause = cause; + } +} + +/** The range USFM for one contiguous run: `{2,3} -> "JHN.3.2-3"`, `{5,5} -> "JHN.3.5"`. */ +function runToPassageId(book: string, chapter: string, run: VerseRun): string { + return run.start === run.end + ? `${book}.${chapter}.${run.start}` + : `${book}.${chapter}.${run.start}-${run.end}`; +} + +/** Expands a contiguous run back into its verse numbers: `{2,4} -> [2,3,4]`. */ +function versesInRun(run: VerseRun): number[] { + const verses: number[] = []; + for (let verse = run.start; verse <= run.end; verse++) verses.push(verse); + return verses; +} + +/** + * Pulls an HTTP status off a thrown ApiClient error (possibly wrapped in our own + * `BibleReaderHighlightError`). Defers to core's `getHttpStatus` for the actual + * status contract so the error shape stays owned by the client layer. + */ +function extractStatus(error: unknown): number | undefined { + if (error instanceof BibleReaderHighlightError) return getHttpStatus(error.cause); + return getHttpStatus(error); +} + +/** + * A 401/403 means the app lost (or never had) the highlights permission. + * + * DEFERRED (accepted for dark launch): a 401 from an *expired token* (not a + * missing permission) also lands here and misroutes to the permission re-prompt + * instead of a token refresh / re-auth. Follow-up: distinguish auth-expiry from + * permission-denied at this boundary. + */ +function isPermissionError(error: unknown): boolean { + const status = extractStatus(error); + return status === 401 || status === 403; +} + +function logWriteFailures(failures: BibleReaderHighlightError[], color: string): void { + for (const failure of failures) { + console.error( + `[YouVersion SDK] Failed to ${failure.operation} highlight ` + + `(passage ${failure.passageId}, color ${color})`, + failure, + ); + } +} + +function opToPending(op: WriteOp): PendingHighlight { + return { + verses: op.verses, + color: op.color, + versionId: op.scope.versionId, + book: op.scope.book, + chapter: op.scope.chapter, + timestamp: Date.now(), + }; +} + +export function scopesEqual(a: HighlightScope, b: HighlightScope): boolean { + return a.versionId === b.versionId && a.book === b.book && a.chapter === b.chapter; +} + +/** The rendered verse→color map: server truth with the optimistic overlay applied. */ +export function selectHighlightedVerses( + serverColors: ServerColors, + overlay: HighlightOverlay, +): Record { + const map: Record = { ...serverColors }; + for (const [verse, color] of Object.entries(overlay)) { + if (color === null) delete map[Number(verse)]; + else map[Number(verse)] = color; + } + return map; +} + +// ── The write actor ────────────────────────────────────────────────────────── + +/** + * Performs one queued write. Apply POSTs collapse contiguous runs to range + * USFMs (one POST per run); remove DELETEs are PER VERSE (range DELETE is + * unsupported server-side). Every sub-write is caught with better-result so the + * actor always RESOLVES with a per-sub-write settlement — a batch can partially + * succeed and the two halves need opposite treatment in `settleWrite`. + */ +const processWrite = fromPromise( + async ({ input }) => { + const svc = input.services.current; + const { op } = input; + const failures: BibleReaderHighlightError[] = []; + const failedVerses: number[] = []; + const succeededVerses: number[] = []; + + if (op.kind === 'apply') { + const runs = collapseVerseRuns(op.verses); + const results = await Promise.all( + runs.map((run) => + Result.tryPromise({ + try: () => + svc.createHighlight({ + version_id: op.scope.versionId, + passage_id: runToPassageId(op.scope.book, op.scope.chapter, run), + color: op.color, + }), + catch: (cause) => + new BibleReaderHighlightError( + 'apply', + runToPassageId(op.scope.book, op.scope.chapter, run), + cause, + ), + }), + ), + ); + runs.forEach((run, index) => { + const result = results[index]; + if (result === undefined) return; // unreachable: 1:1 with runs + const runVerses = versesInRun(run); + if (Result.isError(result)) { + failures.push(result.error); + failedVerses.push(...runVerses); + } else { + succeededVerses.push(...runVerses); + } + }); + } else { + const results = await Promise.all( + op.verses.map((verse) => { + const passageId = `${op.scope.book}.${op.scope.chapter}.${verse}`; + return Result.tryPromise({ + try: () => svc.deleteHighlight(passageId, { version_id: op.scope.versionId }), + catch: (cause) => new BibleReaderHighlightError('remove', passageId, cause), + }); + }), + ); + op.verses.forEach((verse, index) => { + const result = results[index]; + if (result === undefined) return; // unreachable: 1:1 with verses + if (Result.isError(result)) { + failures.push(result.error); + failedVerses.push(verse); + } else { + succeededVerses.push(verse); + } + }); + } + + return { op, failures, failedVerses, succeededVerses }; + }, +); + +// ── Machine ─────────────────────────────────────────────────────────────────── + +export const bibleReaderHighlightsMachine = setup({ + types: { + context: {} as HighlightsContext, + events: {} as HighlightsEvent, + input: {} as HighlightsMachineInput, + }, + actors: { processWrite }, + guards: { + isEnabledNow: ({ context }) => context.flagOn && context.hasAuthProvider, + isDisabledNow: ({ context }) => !context.flagOn || !context.hasAuthProvider, + scopeIsDifferent: ({ context, event }) => + event.type === 'SCOPE_CHANGED' && !scopesEqual(context.scope, event.scope), + queueHasWork: ({ context }) => context.queue.length > 0, + signedOut: ({ context }) => !context.isAuthenticated, + + // ── TAP_COLOR fork ── + tapInert: ({ event }) => event.type === 'TAP_COLOR' && event.verses.length === 0, + tapCanWrite: ({ context, event }) => + event.type === 'TAP_COLOR' && + event.verses.length > 0 && + context.isAuthenticated && + context.services.current.hasHighlightsPermission(), + tapNeedsSignIn: ({ context, event }) => + event.type === 'TAP_COLOR' && event.verses.length > 0 && !context.isAuthenticated, + + // ── resume fork ── + noPending: () => readPendingHighlight() === null, + pendingNotAuthed: ({ context }) => readPendingHighlight() !== null && !context.isAuthenticated, + pendingAuthedHasPermission: ({ context }) => + readPendingHighlight() !== null && + context.isAuthenticated && + context.services.current.hasHighlightsPermission(), + pendingAuthedNoPermission: ({ context }) => + readPendingHighlight() !== null && + context.isAuthenticated && + !context.services.current.hasHighlightsPermission(), + }, + actions: { + setOutcomeNoop: assign({ lastTapOutcome: () => 'noop' as TapOutcome }), + + assignAuth: assign(({ event }) => { + if (event.type !== 'AUTH_CHANGED') return {}; + return { + flagOn: event.flagOn, + hasAuthProvider: event.hasAuthProvider, + isAuthenticated: event.isAuthenticated, + }; + }), + + /** Sign-out / going disabled must not leave a stale overlay to resurface later. */ + resetWriteStateIfSignedOut: assign(({ event }) => { + if (event.type !== 'AUTH_CHANGED' || event.isAuthenticated) return {}; + return { + overlay: {}, + reconcile: new Map(), + writeIntent: new Map(), + queue: [], + }; + }), + + resetForScopeChange: assign(({ event }) => { + if (event.type !== 'SCOPE_CHANGED') return {}; + // Verse numbers collide across scopes: drop the overlay + reconcile + // expectations synchronously. In-flight writes carry their own scope and + // still settle correctly. This is also the escape hatch for a + // never-converging write — navigating away releases it. + return { + scope: event.scope, + overlay: {}, + reconcile: new Map(), + }; + }), + + /** + * Store the freshly fetched server truth and reconcile the overlay against + * it. Apply entries retire once the fetch reflects the written color; remove + * entries are HELD (the vapor fix — see the file header). + */ + reconcileOverlay: assign(({ context, event }) => { + if (event.type !== 'HIGHLIGHTS_UPDATED') return {}; + const serverColors = event.serverColors; + if (context.reconcile.size === 0) return { serverColors }; + + const overlay = { ...context.overlay }; + const reconcile = new Map(context.reconcile); + let changed = false; + for (const [verse, entry] of context.reconcile) { + if (entry.op !== 'apply') continue; // remove entries never retire (vapor fix) + if (serverColors[verse] === entry.color) { + reconcile.delete(verse); + if (verse in overlay) { + delete overlay[verse]; + changed = true; + } + } + } + return changed ? { serverColors, overlay, reconcile } : { serverColors, reconcile }; + }), + + consumeDataExchangeOnce: assign(({ context }) => { + if (context.dataExchangeConsumed) return {}; + const returned = context.services.current.consumeDataExchangeReturn(); + // Act on a decline/failure the moment the return is consumed — BEFORE the + // auth gate. The shipped auth provider hydrates asynchronously, so this + // runs unauthenticated on the first pass after a redirect return; a decline + // means the intent is dead regardless of who ends up signed in. + if (returned && returned.status !== 'granted') { + clearPendingHighlight(); + } + return { dataExchangeConsumed: true }; + }), + + /** Optimistically paint + claim + enqueue a user apply (TAP_COLOR authorized path). */ + startApplyWrite: enqueueActions(({ enqueue, context, event }) => { + if (event.type !== 'TAP_COLOR') return; + const color = event.color.toLowerCase(); + const verses = event.verses; + const token = {}; + enqueue.assign(({ context: current }) => { + const writeIntent = new Map(current.writeIntent); + const reconcile = new Map(current.reconcile); + const overlay = { ...current.overlay }; + for (const verse of verses) { + writeIntent.set(verse, token); + // A newer write supersedes any older reconciliation still pending for + // this verse, so the reconcile effect can't retire/hold the fresh + // overlay against the old write's target. + reconcile.delete(verse); + overlay[verse] = color; + } + return { writeIntent, reconcile, overlay, lastTapOutcome: 'applied' as TapOutcome }; + }); + const op: WriteOp = { + kind: 'apply', + color, + verses, + scope: context.scope, + token, + paint: true, + }; + enqueue.raise({ type: 'ENQUEUE', op }); + }), + + /** + * A tap that must enter the auth flow (signed out → sign-in dialog; signed in + * without permission → permission dialog): stash the intent so it survives a + * redirect round-trip / resumes after the grant, and report the `flow` + * outcome so the caller keeps the verse selection. + */ + stashPendingTap: enqueueActions(({ enqueue, context, event }) => { + if (event.type !== 'TAP_COLOR') return; + stashPendingHighlight({ + verses: event.verses, + color: event.color.toLowerCase(), + versionId: context.scope.versionId, + book: context.scope.book, + chapter: context.scope.chapter, + timestamp: Date.now(), + }); + enqueue.assign({ lastTapOutcome: () => 'flow' as TapOutcome }); + }), + + /** Remove: only clear verses currently rendered in the given color. */ + startRemoveWrite: enqueueActions(({ enqueue, context, event }) => { + if (event.type !== 'REMOVE') return; + if (!context.isAuthenticated) return; // nothing rendered to remove when signed out + const color = event.color.toLowerCase(); + const rendered = selectHighlightedVerses(context.serverColors, context.overlay); + const targetVerses = event.verses.filter((verse) => rendered[verse] === color); + if (targetVerses.length === 0) return; + + const token = {}; + enqueue.assign(({ context: current }) => { + const writeIntent = new Map(current.writeIntent); + const reconcile = new Map(current.reconcile); + const overlay = { ...current.overlay }; + for (const verse of targetVerses) { + writeIntent.set(verse, token); + reconcile.delete(verse); + overlay[verse] = null; + } + return { writeIntent, reconcile, overlay }; + }); + const op: WriteOp = { + kind: 'remove', + color, + verses: targetVerses, + scope: context.scope, + token, + paint: true, + }; + enqueue.raise({ type: 'ENQUEUE', op }); + }), + + /** + * Apply a pending highlight after a granted return. Writes to the PENDING's + * own scope even if the user returned on a different chapter; only paints the + * overlay when that scope matches what is on screen. + */ + applyPendingHighlight: enqueueActions(({ enqueue, context }) => { + const pending = readPendingHighlight(); + if (!pending) return; + clearPendingHighlight(); + const scope: HighlightScope = { + versionId: pending.versionId, + book: pending.book, + chapter: pending.chapter, + }; + const paint = scopesEqual(scope, context.scope); + const token = {}; + if (paint) { + enqueue.assign(({ context: current }) => { + const writeIntent = new Map(current.writeIntent); + const reconcile = new Map(current.reconcile); + const overlay = { ...current.overlay }; + for (const verse of pending.verses) { + writeIntent.set(verse, token); + reconcile.delete(verse); + overlay[verse] = pending.color; + } + return { writeIntent, reconcile, overlay }; + }); + } + // Resume-applied writes route through the SAME failure handling as a + // user-initiated apply: a 401/403 re-stashes the pending highlight (from + // the op's own scope, so a cross-scope return still re-prompts on the + // right passage) and re-opens the permission dialog, rather than silently + // dropping the user's original color tap. Being an `apply` is what earns + // that re-prompt at the consumer site (see `settleWrite`). + const op: WriteOp = { + kind: 'apply', + color: pending.color, + verses: pending.verses, + scope, + token, + paint, + }; + enqueue.raise({ type: 'ENQUEUE', op }); + }), + + enqueueOp: assign(({ context, event }) => { + if (event.type !== 'ENQUEUE') return {}; + return { queue: [...context.queue, event.op] }; + }), + + shiftQueue: assign(({ context }) => ({ queue: context.queue.slice(1) })), + + /** + * Per-sub-write settlement of a finished batch: succeeded verses register for + * reconciliation (overlay holds until a fetch reflects the write), failed + * verses revert (only if still owned by this op's token), exactly one refetch + * fires, and failures route by status (401/403 → invalidate + maybe re-prompt; + * network/5xx → discard pending). + */ + settleWrite: enqueueActions(({ enqueue, event }) => { + // Wired only to the processWrite `onDone`; the done event carries `output` + // but is not part of the public event union, so read it via a cast. + const { op, failures, failedVerses, succeededVerses } = ( + event as unknown as { output: WriteResult } + ).output; + + enqueue.assign(({ context: current }) => { + const overlay = { ...current.overlay }; + const reconcile = new Map(current.reconcile); + for (const verse of succeededVerses) { + if (current.writeIntent.get(verse) === op.token) { + reconcile.set(verse, { op: op.kind, color: op.color }); + } + } + for (const verse of failedVerses) { + if (current.writeIntent.get(verse) === op.token && verse in overlay) { + delete overlay[verse]; + } + } + return { overlay, reconcile }; + }); + + // Exactly one GET per settled batch, success or failure — this is what + // reconciles partial successes to server truth now that the hooks layer no + // longer auto-refetches per write. + enqueue(({ context: current }) => current.services.current.refetch()); + + if (failures.length === 0) return; + + enqueue(() => logWriteFailures(failures, op.color)); + + const permissionDenied = failures.some(isPermissionError); + if (permissionDenied) { + // Server says the permission is gone (or never applied): invalidate the + // optimistic cache. Server truth wins. + enqueue(({ context: current }) => + current.services.current.invalidateHighlightsPermission(), + ); + // Only an apply keeps the pending highlight + re-prompts: a user apply + // and a resume-applied pending both re-stash + re-open the permission + // dialog so the original tap is never silently lost. Removes deliberately + // don't re-prompt (fixing the old deferred wart): with no pending + // highlight, a re-prompt's post-grant resume was a no-op — invalidate the + // cache only, and the next apply re-enters the flow. + if (op.kind === 'apply') { + // Keep this highlight pending and re-prompt the just-in-time dialog. + enqueue(() => stashPendingHighlight(opToPending(op))); + enqueue.raise({ type: 'PERMISSION_LOST' }); + } + } else if (op.kind === 'apply') { + // Network / 5xx: overlay already reverted; drop pending. INTENTIONAL for + // resumed writes too — transient failures consume the intent uniformly + // (the user is authed now; a re-tap just works, and the failure surfaces + // via the snackbar, YPE-3873). Re-stashing instead would auto-apply the + // highlight on a later mount within the pending TTL with no user action, + // which is worse than asking for one more tap. + enqueue(() => clearPendingHighlight()); + } + }), + + // ── Dialog side effects (fire-and-forget redirects, matching the hook) ── + startSignIn: ({ context }) => { + void context.services.current.startSignInForHighlights().catch((error: unknown) => { + console.error('[YouVersion SDK] Failed to start sign-in for highlights', error); + clearPendingHighlight(); + }); + }, + startDataExchange: ({ context }) => { + void context.services.current.startDataExchangeForHighlights().catch((error: unknown) => { + console.error('[YouVersion SDK] Failed to start data exchange for highlights', error); + clearPendingHighlight(); + }); + }, + clearPending: () => clearPendingHighlight(), + }, +}).createMachine({ + id: 'bibleReaderHighlights', + context: ({ input }) => ({ + services: input.services, + scope: input.scope, + flagOn: input.flagOn, + hasAuthProvider: input.hasAuthProvider, + isAuthenticated: input.isAuthenticated, + serverColors: {}, + overlay: {}, + reconcile: new Map(), + writeIntent: new Map(), + queue: [], + dataExchangeConsumed: false, + lastTapOutcome: 'noop', + }), + // These three inputs update context regardless of the active state; state + // moves are driven by `always` guards that read the freshly-assigned context. + on: { + AUTH_CHANGED: { actions: ['assignAuth', 'resetWriteStateIfSignedOut'] }, + HIGHLIGHTS_UPDATED: { actions: 'reconcileOverlay' }, + SCOPE_CHANGED: { guard: 'scopeIsDifferent', actions: 'resetForScopeChange' }, + }, + initial: 'booting', + states: { + booting: { + always: [{ guard: 'isEnabledNow', target: 'enabled' }, { target: 'disabled' }], + }, + + disabled: { + // Flag off or no auth provider: fully inert. Taps resolve to noop; removes + // are ignored. Copy/share (owned by the caller) are unaffected. + always: [{ guard: 'isEnabledNow', target: 'enabled' }], + on: { + TAP_COLOR: { actions: 'setOutcomeNoop' }, + REMOVE: {}, + }, + }, + + enabled: { + always: [{ guard: 'isDisabledNow', target: 'disabled' }], + type: 'parallel', + states: { + flow: { + initial: 'resuming', + on: { + // A write's 401/403 (raised by settleWrite) re-opens the JIT dialog. + PERMISSION_LOST: { target: '.permissionDialog' }, + // Taps are handled here so any flow substate resolves them. + TAP_COLOR: [ + { guard: 'tapInert', actions: 'setOutcomeNoop' }, + { guard: 'tapCanWrite', target: '.idle', actions: 'startApplyWrite' }, + { guard: 'tapNeedsSignIn', target: '.signInDialog', actions: 'stashPendingTap' }, + { target: '.permissionDialog', actions: 'stashPendingTap' }, + ], + REMOVE: { actions: 'startRemoveWrite' }, + }, + states: { + resuming: { + entry: 'consumeDataExchangeOnce', + always: [ + { guard: 'noPending', target: 'idle' }, + { guard: 'pendingNotAuthed', target: 'awaitingAuth' }, + { + guard: 'pendingAuthedHasPermission', + target: 'idle', + actions: 'applyPendingHighlight', + }, + { target: 'permissionDialog' }, + ], + }, + awaitingAuth: { + // Wait for the authenticated flip, then resolve the pending intent. + // No self-target: when still unauthenticated, none match and we stay. + always: [ + { guard: 'noPending', target: 'idle' }, + { + guard: 'pendingAuthedHasPermission', + target: 'idle', + actions: 'applyPendingHighlight', + }, + { guard: 'pendingAuthedNoPermission', target: 'permissionDialog' }, + ], + }, + idle: {}, + signInDialog: { + on: { + CONFIRM_SIGN_IN: { target: 'idle', actions: 'startSignIn' }, + DECLINE_SIGN_IN: { target: 'idle', actions: 'clearPending' }, + }, + }, + permissionDialog: { + // The host can sign the user out while this dialog is open. A + // confirm would then start a data exchange that rejects + // unauthenticated, so route back to idle and drop the pending + // intent as soon as the sign-out lands (assignAuth on the root + // AUTH_CHANGED updates isAuthenticated, then this always fires). + // permissionDialog is only ever entered while authenticated, so + // this guard never misfires on entry. + always: [{ guard: 'signedOut', target: 'idle', actions: 'clearPending' }], + on: { + CONFIRM_PERMISSION: { target: 'idle', actions: 'startDataExchange' }, + CANCEL_PERMISSION: { target: 'idle', actions: 'clearPending' }, + }, + }, + }, + }, + + writer: { + initial: 'idle', + on: { + // FIFO enqueue works in any writer state; `idle.always` starts it. + ENQUEUE: { actions: 'enqueueOp' }, + }, + states: { + idle: { + always: [{ guard: 'queueHasWork', target: 'writing' }], + }, + writing: { + invoke: { + src: 'processWrite', + input: ({ context }) => ({ services: context.services, op: context.queue[0]! }), + onDone: { target: 'checkQueue', actions: ['settleWrite', 'shiftQueue'] }, + // processWrite catches internally and never rejects, but keep the + // queue alive if it ever does. + onError: { target: 'checkQueue', actions: 'shiftQueue' }, + }, + }, + checkQueue: { + always: [{ guard: 'queueHasWork', target: 'writing' }, { target: 'idle' }], + }, + }, + }, + }, + }, + }, +}); diff --git a/packages/ui/src/components/bible-reader.tsx b/packages/ui/src/components/bible-reader.tsx index 99ee9e07..f042a627 100644 --- a/packages/ui/src/components/bible-reader.tsx +++ b/packages/ui/src/components/bible-reader.tsx @@ -37,9 +37,14 @@ import { LoaderIcon } from './icons/loader'; import { PersonIcon } from './icons/person'; import { Button } from './ui/button'; import { Popover, PopoverClose, PopoverContent, PopoverTrigger } from './ui/popover'; +import { useBibleReaderHighlights } from './use-bible-reader-highlights'; import { VerseActionPopover } from './verse-action-popover'; +import { HighlightPermissionDialog } from './highlight-permission-dialog'; +import { SignInDialog } from './sign-in-dialog'; import { BibleTextView, getCleanVerseText, type FootnoteData } from './verse'; import { buildVerseReference, buildVerseShareText, joinVerseTexts } from '@/lib/verse-share'; +import { isHighlightsLive } from '@/lib/feature-flags'; +import { YouVersionPlatformConfiguration } from '@youversion/platform-core'; type BibleReaderContextType = { book: string; @@ -503,40 +508,41 @@ function Content() { }, [book, chapter]); // ---- Verse selection + highlights ------------------------------------------ - // Selection is ephemeral; highlights persist to localStorage only for now - // (ADR-001) in the future `highlight` API shape: keyed by full passage_id USFM - // and scoped by versionId/bible_id (ADR-002). The reader DOM ref lets us anchor - // the popover and pull clean verse text for Copy / Share. + // Selection is ephemeral (ADR-007 in YPE-642). Highlights are server-only + // account data (ADR-001 in YPE-1034): fetched and written through + // useBibleReaderHighlights, dark-launched behind the internal HIGHLIGHTS_LIVE + // flag. The reader DOM ref lets us anchor the popover and pull clean verse + // text for Copy / Share. const readerRef = useRef(null); const [selectedVerses, setSelectedVerses] = useState([]); const [popoverOpen, setPopoverOpen] = useState(false); const [anchorElement, setAnchorElement] = useState(null); const lastSelectionRef = useRef([]); - const [highlightStore, setHighlightStore] = useState>({}); - - const highlightsStorageKey = `youversion-platform:highlights:${versionId}`; - - // Clear the store synchronously (during render) the moment the version key - // changes. The load effect below runs *after* paint, so without this the - // previous version's highlights would paint over the new text for one frame — - // their `${book}.${chapter}.N` keys collide with the new version's verses. - const [loadedHighlightsKey, setLoadedHighlightsKey] = useState(highlightsStorageKey); - if (loadedHighlightsKey !== highlightsStorageKey) { - setLoadedHighlightsKey(highlightsStorageKey); - setHighlightStore({}); - } - // Load this version's highlights when the version changes (client-only). - useEffect(() => { - let data: Record = {}; - try { - const raw = localStorage.getItem(highlightsStorageKey); - if (raw) data = JSON.parse(raw) as Record; - } catch { - // Ignore (unavailable or malformed storage). - } - setHighlightStore(data); - }, [highlightsStorageKey]); + const { + highlightedVerses, + highlightsInteractive, + apply: applyHighlight, + remove: removeHighlight, + permissionDialogOpen, + onPermissionDialogOpenChange, + confirmPermissionDialog, + cancelPermissionDialog, + signInDialogOpen, + confirmSignInDialog, + cancelSignInDialog, + } = useBibleReaderHighlights({ versionId, book, chapter }); + + // The color row / clear-highlight affordances only render when the highlights + // feature is live (dark-launch flag) AND can actually function. Without a + // YouVersionAuthProvider mounted the machine is inert (taps resolve to noop), + // so the swatch row would be dead — hide it for copy/share-only integrators. + // Copy / Share are always available. + const highlightsEnabled = isHighlightsLive() && highlightsInteractive; + // Copy shown to the sign-in dialog. Falls back to a neutral label when the + // integrator hasn't set `YouVersionPlatformConfiguration.appName`. + const signInAppName = YouVersionPlatformConfiguration.appName ?? 'This app'; + const signInPromptMessage = YouVersionPlatformConfiguration.signInPromptMessage; // Navigating away (book/chapter/version) drops the selection — those verses no // longer exist on screen (ADR-007). @@ -547,18 +553,6 @@ function Content() { lastSelectionRef.current = []; }, [book, chapter, versionId]); - // Derive the visible chapter's highlights (verse number → hex) from the store. - const chapterPrefix = `${book}.${chapter}.`; - const highlightedVerses = useMemo(() => { - const map: Record = {}; - for (const [passageId, color] of Object.entries(highlightStore)) { - if (!passageId.startsWith(chapterPrefix)) continue; - const verseNum = parseInt(passageId.slice(chapterPrefix.length), 10); - if (verseNum) map[verseNum] = color; - } - return map; - }, [highlightStore, chapterPrefix]); - // Distinct colors present in the current selection → drives the X (remove) circles. const activeHighlights = useMemo( () => @@ -570,14 +564,6 @@ function Content() { [selectedVerses, highlightedVerses], ); - function persistHighlights(next: Record) { - try { - localStorage.setItem(highlightsStorageKey, JSON.stringify(next)); - } catch { - // Ignore (private mode / quota exceeded). - } - } - function closeAndClearSelection() { setPopoverOpen(false); setSelectedVerses([]); @@ -607,26 +593,21 @@ function Content() { } function handleHighlight(color: string) { - const next = { ...highlightStore }; - for (const verse of selectedVerses) { - next[`${book}.${chapter}.${verse}`] = color; - } - setHighlightStore(next); - persistHighlights(next); + const outcome = applyHighlight(color, selectedVerses); + // Entering the auth flow (sign-in redirect or the permission confirm dialog) + // keeps the verse selection and popover intact so a cancel leaves the reader + // exactly where it was (YPE-1034 decision 7). An immediate apply — or an + // inert no-op — clears as before. + if (outcome === 'flow') return; closeAndClearSelection(); } function handleClearHighlight(color: string) { - const next = { ...highlightStore }; - for (const verse of selectedVerses) { - const passageId = `${book}.${chapter}.${verse}`; - if (next[passageId] === color) delete next[passageId]; - } - setHighlightStore(next); - persistHighlights(next); + removeHighlight(color, selectedVerses); // Multiple colors active → keep open so the user can remove others (AC 8a); - // last color removed → dismiss (AC 8). + // last color removed → dismiss (AC 8). `highlightedVerses` still holds the + // pre-removal snapshot here — the optimistic overlay lands next render. const hasRemaining = selectedVerses.some((verse) => { const current = highlightedVerses[verse]; return current && current !== color; @@ -781,6 +762,7 @@ function Content() { activeHighlights={activeHighlights} selectedVerses={selectedVerses} highlightedVerses={highlightedVerses} + highlightsEnabled={highlightsEnabled} anchorElement={anchorElement} scrollRoot={scrollContainerRef.current} onHighlight={handleHighlight} @@ -790,6 +772,26 @@ function Content() { theme={background} /> + + + { + if (!open) cancelSignInDialog(); + }} + appName={signInAppName} + promptMessage={signInPromptMessage} + onConfirm={confirmSignInDialog} + onDecline={cancelSignInDialog} + theme={background} + /> + {showLoadingOverlay ? (
void; + /** User accepted → start the data-exchange grant. */ + onConfirm: () => void; + /** User declined → discard the pending highlight. */ + onCancel: () => void; + theme?: 'light' | 'dark'; +}; + +/** + * Just-in-time permission confirm dialog for the highlight auth flow + * (YPE-1034). Copy is verbatim from the Swift SDK (`dataExchange.highlights.*`). + * Accepting starts the data-exchange grant; declining/dismissing discards only + * the pending highlight and leaves the verse selection intact. + */ +export const HighlightPermissionDialog: FC = ({ + open, + onOpenChange, + onConfirm, + onCancel, + theme = 'light', +}) => { + const { t } = useTranslation(undefined, { i18n }); + + return ( + + + + +
+ + {t('dataExchangeHighlightsQuestion')} + + + {t('dataExchangeHighlightsExplanation')} + +
+ +
+ + +
+ + + + ); +}; diff --git a/packages/ui/src/components/icons/check.tsx b/packages/ui/src/components/icons/check.tsx new file mode 100644 index 00000000..32bdeba2 --- /dev/null +++ b/packages/ui/src/components/icons/check.tsx @@ -0,0 +1,19 @@ +import type { ComponentProps, ReactElement } from 'react'; + +export function CheckIcon(props: ComponentProps<'svg'>): ReactElement { + return ( + + + + ); +} diff --git a/packages/ui/src/components/sign-in-dialog.test.tsx b/packages/ui/src/components/sign-in-dialog.test.tsx new file mode 100644 index 00000000..984106a7 --- /dev/null +++ b/packages/ui/src/components/sign-in-dialog.test.tsx @@ -0,0 +1,112 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { SignInDialog } from './sign-in-dialog'; + +describe('SignInDialog', () => { + const defaultProps = { + open: true, + onOpenChange: vi.fn(), + appName: 'Acme Bible', + onConfirm: vi.fn(), + onDecline: vi.fn(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('Rendering', () => { + it('renders the eyebrow, logo, body, and both buttons', () => { + render(); + + // Eyebrow caption + expect(screen.getByText('INTRODUCING')).toBeTruthy(); + // YouVersion Platform logo (svg with role img) + expect(screen.getByRole('img', { name: 'YouVersion Platform' })).toBeTruthy(); + // Body paragraph + expect( + screen.getByText(/wants to connect to your YouVersion Bible App account/), + ).toBeTruthy(); + // Primary + secondary buttons + expect(screen.getByRole('button', { name: 'Yes Please' })).toBeTruthy(); + expect(screen.getByRole('button', { name: 'No Thanks' })).toBeTruthy(); + }); + + it('does not render content when open is false', () => { + render(); + expect(screen.queryByRole('dialog')).toBeNull(); + }); + }); + + describe('Integrator prompt message', () => { + it('is hidden when promptMessage is unset', () => { + render(); + expect(screen.queryByText(/Loving this reading plan/)).toBeNull(); + }); + + it('is shown when promptMessage is provided', () => { + render(); + expect(screen.getByText(/Loving this reading plan\?/)).toBeTruthy(); + }); + }); + + describe('appName interpolation', () => { + it('interpolates the appName into the body paragraph', () => { + render(); + expect( + screen.getByText( + 'Acme Bible wants to connect to your YouVersion Bible App account. This will allow them to see and interact with your Bible App activity. Would you like to proceed?', + ), + ).toBeTruthy(); + }); + }); + + describe('Callbacks', () => { + it('calls onConfirm when "Yes Please" is clicked', () => { + const onConfirm = vi.fn(); + render(); + fireEvent.click(screen.getByRole('button', { name: 'Yes Please' })); + expect(onConfirm).toHaveBeenCalledTimes(1); + }); + + it('calls onDecline when "No Thanks" is clicked', () => { + const onDecline = vi.fn(); + render(); + fireEvent.click(screen.getByRole('button', { name: 'No Thanks' })); + expect(onDecline).toHaveBeenCalledTimes(1); + }); + }); + + describe('Accessibility', () => { + it('renders a dialog labelled by the eyebrow title', () => { + render(); + const dialog = screen.getByRole('dialog'); + expect(dialog).toBeTruthy(); + // Radix wires aria-labelledby to the DialogTitle ("INTRODUCING"). + expect(dialog).toHaveAccessibleName('INTRODUCING'); + }); + + it('has a described-by body paragraph', () => { + render(); + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveAccessibleDescription(/wants to connect to your YouVersion Bible App/); + }); + }); + + describe('Styling', () => { + it('has the data-yv-sdk scoping attribute', () => { + render(); + expect(screen.getByRole('dialog').getAttribute('data-yv-sdk')).not.toBeNull(); + }); + + it('defaults to the light theme', () => { + render(); + expect(screen.getByRole('dialog').getAttribute('data-yv-theme')).toBe('light'); + }); + + it('applies the dark theme attribute', () => { + render(); + expect(screen.getByRole('dialog').getAttribute('data-yv-theme')).toBe('dark'); + }); + }); +}); diff --git a/packages/ui/src/components/sign-in-dialog.tsx b/packages/ui/src/components/sign-in-dialog.tsx new file mode 100644 index 00000000..6d7d55a3 --- /dev/null +++ b/packages/ui/src/components/sign-in-dialog.tsx @@ -0,0 +1,99 @@ +import type { FC } from 'react'; +import * as DialogPrimitive from '@radix-ui/react-dialog'; +import { useTranslation } from 'react-i18next'; +import i18n from '@/i18n'; +import { cn } from '../lib/utils'; +import { YouVersionLogo } from './icons/youversion-logo'; +import { Button } from './ui/button'; + +type SignInDialogProps = { + open: boolean; + onOpenChange: (open: boolean) => void; + /** The integrating app's display name, interpolated into the body copy. */ + appName: string; + /** + * Optional pitch line supplied by the integrator (from + * `YouVersionPlatformConfiguration`, passed in by the wiring layer). Hidden + * entirely when unset. + */ + promptMessage?: string; + /** User tapped "Yes Please" → launch the OAuth sign-in flow. */ + onConfirm: () => void; + /** User tapped "No Thanks" / dismissed → abandon the sign-in flow. */ + onDecline: () => void; + theme?: 'light' | 'dark'; +}; + +/** + * "Sign in with YouVersion" introduction dialog (YPE-1034). Shown when a + * signed-out user taps a highlight color, before OAuth launches. Copy is + * verbatim from the Swift SDK's `SignInWithYouVersionView` (`signIn.*`). + * Presentational only — accepting/declining is delegated to the callbacks; the + * component performs no OAuth, network, or config reads. + */ +export const SignInDialog: FC = ({ + open, + onOpenChange, + appName, + promptMessage, + onConfirm, + onDecline, + theme = 'light', +}) => { + const { t } = useTranslation(undefined, { i18n }); + + return ( + + + + +
+ + {t('signInIntroducing')} + + +
+ + {promptMessage ? ( +

+ “{promptMessage}” +

+ ) : null} + + + {t('signInParagraph', { appName })} + + +
+ + +
+ + + + ); +}; diff --git a/packages/ui/src/components/ui/button.test.tsx b/packages/ui/src/components/ui/button.test.tsx new file mode 100644 index 00000000..a2e7e140 --- /dev/null +++ b/packages/ui/src/components/ui/button.test.tsx @@ -0,0 +1,21 @@ +/** + * @vitest-environment jsdom + */ +import { render } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { Button } from './button'; + +describe('Button — default variant colors', () => { + it('paints the primary surface, not background-on-foreground (regression: invisible button)', () => { + // The `default` variant must pair `bg-primary` with `text-primary-foreground`. + // The previous `bg-background` + `text-primary-foreground` pairing resolved to + // white-on-white in the light theme, so the Continue button on the highlight + // permission dialog was invisible. Guard against regressing to that pairing. + const { getByRole } = render(); + const className = getByRole('button').className; + + expect(className).toContain('yv:bg-primary'); + expect(className).toContain('yv:text-primary-foreground'); + expect(className).not.toContain('yv:bg-background'); + }); +}); diff --git a/packages/ui/src/components/ui/button.tsx b/packages/ui/src/components/ui/button.tsx index 9f1b0a41..ddab8de7 100644 --- a/packages/ui/src/components/ui/button.tsx +++ b/packages/ui/src/components/ui/button.tsx @@ -9,7 +9,7 @@ const buttonVariants = cva( { variants: { variant: { - default: 'yv:bg-background yv:text-primary-foreground yv:hover:bg-background/90', + default: 'yv:bg-primary yv:text-primary-foreground yv:hover:bg-primary/90', destructive: 'yv:bg-destructive yv:text-white yv:hover:bg-destructive/90 yv:focus-visible:ring-destructive/20 yv:dark:focus-visible:ring-destructive/40 yv:dark:bg-destructive/60', outline: diff --git a/packages/ui/src/components/use-bible-reader-highlights.auth-flow.integration.test.tsx b/packages/ui/src/components/use-bible-reader-highlights.auth-flow.integration.test.tsx new file mode 100644 index 00000000..c710ffe7 --- /dev/null +++ b/packages/ui/src/components/use-bible-reader-highlights.auth-flow.integration.test.tsx @@ -0,0 +1,518 @@ +/** + * @vitest-environment jsdom + * + * Integration coverage for the highlight auth flow state machine (YPE-1034 PR2) + * through the REAL `useHighlights` + `useHighlightAuthActions` hooks. Nothing + * from the hooks package is module-mocked; only the core clients' network + * methods (`HighlightsClient` / `DataExchangeClient` prototypes) and the sign-in + * redirect are stubbed at the boundary — the same discipline that caught PR 1's + * worst bug. + */ +import { act, renderHook, waitFor } from '@testing-library/react'; +import { + DataExchangeClient, + HighlightsClient, + YouVersionAPIUsers, + YouVersionPlatformConfiguration, + type YouVersionUserInfo, +} from '@youversion/platform-core'; +import { YouVersionAuthContext, YouVersionContext } from '@youversion/platform-react-hooks'; +import type { ReactNode } from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { HIGHLIGHTS_LIVE, setHighlightsLive } from '@/lib/feature-flags'; +import { readPendingHighlight } from '@/lib/pending-highlight'; +import { useBibleReaderHighlights } from './use-bible-reader-highlights'; + +const mockUserInfo = { id: 'user-1', name: 'Test User' } as unknown as YouVersionUserInfo; + +let signedIn = false; + +function Providers({ children }: { children: ReactNode }) { + return ( + + + {children} + + + ); +} + +const options = { versionId: 111, book: 'JHN', chapter: '3' }; + +function setLocation(href: string) { + const url = new URL(href); + Object.defineProperty(window, 'location', { + value: { href: url.href, search: url.search, origin: url.origin, pathname: url.pathname }, + writable: true, + configurable: true, + }); +} + +function httpError(status: number): Error { + return Object.assign(new Error(`HTTP ${status}`), { status }); +} + +beforeEach(() => { + vi.restoreAllMocks(); + localStorage.clear(); + sessionStorage.clear(); + signedIn = false; + setHighlightsLive(true); + // The permission cache is user-scoped: persist a matching userInfo (as the + // auth provider does at sign-in) so granted permissions are readable. This + // alone grants nothing — the cache stays empty until a grant lands. + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-1', name: 'Test User' }); + setLocation('https://host.example/read'); + vi.spyOn(window.history, 'replaceState').mockImplementation(vi.fn()); + vi.spyOn(HighlightsClient.prototype, 'getHighlights').mockResolvedValue({ + data: [], + next_page_token: null, + }); +}); + +afterEach(() => { + vi.restoreAllMocks(); + setHighlightsLive(HIGHLIGHTS_LIVE); + localStorage.clear(); + sessionStorage.clear(); +}); + +describe('highlight auth flow — one-fell-swoop (signed out)', () => { + it('color tap opens the sign-in dialog and stashes pending; confirm starts sign-in; granted return applies', async () => { + const signIn = vi.spyOn(YouVersionAPIUsers, 'signIn').mockResolvedValue(undefined); + const createHighlight = vi + .spyOn(HighlightsClient.prototype, 'createHighlight') + .mockResolvedValue({ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }); + + const { result, unmount } = renderHook(() => useBibleReaderHighlights(options), { + wrapper: Providers, + }); + + // NEW BEHAVIOR (PR-288): a signed-out color tap opens the sign-in dialog + // instead of redirecting immediately. It stashes the pending intent and does + // NOT launch OAuth until the user confirms. + act(() => { + expect(result.current.apply('FFFE00', [16])).toBe('flow'); + }); + + expect(result.current.signInDialogOpen).toBe(true); + const pending = readPendingHighlight(); + expect(pending).toMatchObject({ verses: [16], color: 'fffe00', versionId: 111, chapter: '3' }); + expect(signIn).not.toHaveBeenCalled(); + expect(createHighlight).not.toHaveBeenCalled(); + + // Confirm → launch the full-page sign-in redirect requesting `highlights`. + act(() => { + result.current.confirmSignInDialog(); + }); + expect(signIn).toHaveBeenCalledWith( + 'https://host.example/callback', + ['profile'], + ['highlights'], + ); + + // The confirm triggers a full-page redirect; simulate the reload on the + // granted return — a fresh mount, now signed in with the permission granted + // and the pending highlight still in sessionStorage. + unmount(); + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + signedIn = true; + renderHook(() => useBibleReaderHighlights(options), { wrapper: Providers }); + + await waitFor(() => { + expect(createHighlight).toHaveBeenCalledWith({ + version_id: 111, + passage_id: 'JHN.3.16', + color: 'fffe00', + }); + }); + // Pending consumed (the write is the proof it applied). + expect(readPendingHighlight()).toBeNull(); + }); +}); + +describe('highlight auth flow — sign-in dialog (signed out)', () => { + it('a color tap opens the sign-in dialog and stashes pending without launching OAuth', () => { + const signIn = vi.spyOn(YouVersionAPIUsers, 'signIn').mockResolvedValue(undefined); + + const { result } = renderHook(() => useBibleReaderHighlights(options), { wrapper: Providers }); + + act(() => { + expect(result.current.apply('fffe00', [16])).toBe('flow'); + }); + + expect(result.current.signInDialogOpen).toBe(true); + expect(readPendingHighlight()).toMatchObject({ verses: [16], color: 'fffe00' }); + expect(signIn).not.toHaveBeenCalled(); + }); + + it('declining the sign-in dialog discards the pending highlight and does not sign in', () => { + const signIn = vi.spyOn(YouVersionAPIUsers, 'signIn').mockResolvedValue(undefined); + + const { result } = renderHook(() => useBibleReaderHighlights(options), { wrapper: Providers }); + + act(() => { + result.current.apply('fffe00', [16]); + }); + expect(result.current.signInDialogOpen).toBe(true); + expect(readPendingHighlight()).not.toBeNull(); + + act(() => { + result.current.cancelSignInDialog(); + }); + expect(result.current.signInDialogOpen).toBe(false); + expect(readPendingHighlight()).toBeNull(); + expect(signIn).not.toHaveBeenCalled(); + }); +}); + +describe('highlight auth flow — just-in-time (signed in, no permission)', () => { + beforeEach(() => { + signedIn = true; + }); + + it('opens the confirm dialog and stashes pending; confirm starts data exchange', async () => { + const updateToken = vi + .spyOn(DataExchangeClient.prototype, 'updateToken') + .mockResolvedValue('dx-token'); + const createHighlight = vi.spyOn(HighlightsClient.prototype, 'createHighlight'); + + const { result } = renderHook(() => useBibleReaderHighlights(options), { wrapper: Providers }); + + act(() => { + expect(result.current.apply('fffe00', [16])).toBe('flow'); + }); + + expect(result.current.permissionDialogOpen).toBe(true); + expect(readPendingHighlight()).toMatchObject({ verses: [16], color: 'fffe00' }); + expect(createHighlight).not.toHaveBeenCalled(); + + await act(async () => { + result.current.confirmPermissionDialog(); + await Promise.resolve(); + }); + + expect(updateToken).toHaveBeenCalledWith(['highlights']); + await waitFor(() => { + expect(window.location.href).toContain('https://api.example.com/data-exchange'); + }); + // Pending survives the redirect so the resume effect can apply it on return. + expect(readPendingHighlight()).not.toBeNull(); + }); + + it('declining the dialog discards only the pending highlight', () => { + const { result } = renderHook(() => useBibleReaderHighlights(options), { wrapper: Providers }); + + act(() => { + result.current.apply('fffe00', [16]); + }); + expect(readPendingHighlight()).not.toBeNull(); + + act(() => { + result.current.cancelPermissionDialog(); + }); + expect(result.current.permissionDialogOpen).toBe(false); + expect(readPendingHighlight()).toBeNull(); + }); + + it('closes the dialog and clears pending when the host signs the user out mid-flow', () => { + const updateToken = vi + .spyOn(DataExchangeClient.prototype, 'updateToken') + .mockResolvedValue('dx-token'); + + const { result, rerender } = renderHook(() => useBibleReaderHighlights(options), { + wrapper: Providers, + }); + + act(() => { + result.current.apply('fffe00', [16]); + }); + expect(result.current.permissionDialogOpen).toBe(true); + expect(readPendingHighlight()).not.toBeNull(); + + // The host signs the user out while the confirm dialog is still open. A + // confirm now would start a data exchange that rejects unauthenticated, so + // the flow must route back out of the dialog and drop the pending intent. + signedIn = false; + rerender(); + + expect(result.current.permissionDialogOpen).toBe(false); + expect(readPendingHighlight()).toBeNull(); + + // A confirm after the auto-close is a no-op: no data exchange is started. + act(() => { + result.current.confirmPermissionDialog(); + }); + expect(updateToken).not.toHaveBeenCalled(); + }); +}); + +describe('highlight auth flow — data-exchange return', () => { + it('applies the pending highlight on a granted return (async session hydration)', async () => { + signedIn = false; + setLocation( + 'https://host.example/read?data_exchange_status=granted&granted_permissions=highlights', + ); + // Record the initiator as the redirect leg would have — the callback only + // saves a grant for the user who started the exchange. + YouVersionPlatformConfiguration.saveDataExchangeInitiator(); + // Pre-stash a pending highlight as the confirm path would have. + sessionStorage.setItem( + 'youversion-platform:pending-highlight', + JSON.stringify({ + verses: [16], + color: 'fffe00', + versionId: 111, + book: 'JHN', + chapter: '3', + timestamp: Date.now(), + }), + ); + const createHighlight = vi + .spyOn(HighlightsClient.prototype, 'createHighlight') + .mockResolvedValue({ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }); + + const { rerender } = renderHook(() => useBibleReaderHighlights(options), { + wrapper: Providers, + }); + + // The session resolves after mount, as the shipped provider does. + signedIn = true; + rerender(); + + await waitFor(() => { + expect(createHighlight).toHaveBeenCalledWith({ + version_id: 111, + passage_id: 'JHN.3.16', + color: 'fffe00', + }); + }); + expect(YouVersionPlatformConfiguration.hasPermission('highlights')).toBe(true); + expect(readPendingHighlight()).toBeNull(); + }); + + it('re-stashes pending and re-prompts when the resumed write fails 401 (async session hydration)', async () => { + // Same granted-return path as above, but the resumed POST comes back 401. + // The user's original color tap must NOT be silently lost: the pending is + // re-stashed (from the op's own scope), the permission cache is invalidated, + // and the permission dialog re-opens — the same handling as a user apply. + vi.spyOn(console, 'error').mockImplementation(vi.fn()); + signedIn = false; + setLocation( + 'https://host.example/read?data_exchange_status=granted&granted_permissions=highlights', + ); + // Record the initiator as the redirect leg would have (see test above). + YouVersionPlatformConfiguration.saveDataExchangeInitiator(); + sessionStorage.setItem( + 'youversion-platform:pending-highlight', + JSON.stringify({ + verses: [16], + color: 'fffe00', + versionId: 111, + book: 'JHN', + chapter: '3', + timestamp: Date.now(), + }), + ); + const createHighlight = vi + .spyOn(HighlightsClient.prototype, 'createHighlight') + .mockRejectedValue(httpError(401)); + + const { result, rerender } = renderHook(() => useBibleReaderHighlights(options), { + wrapper: Providers, + }); + + signedIn = true; + rerender(); + + await waitFor(() => { + expect(result.current.permissionDialogOpen).toBe(true); + }); + // The resumed write was attempted, then failed. + expect(createHighlight).toHaveBeenCalledWith({ + version_id: 111, + passage_id: 'JHN.3.16', + color: 'fffe00', + }); + // Server truth wins: cache invalidated, and the pending is re-stashed from the + // op's own scope so a post-grant confirm can resume it. + expect(YouVersionPlatformConfiguration.hasPermission('highlights')).toBe(false); + expect(readPendingHighlight()).toMatchObject({ + verses: [16], + color: 'fffe00', + versionId: 111, + chapter: '3', + }); + }); + + it('discards pending on a cancelled return and does not re-open the dialog (async session hydration)', async () => { + // Mount signed OUT: the shipped YouVersionAuthProvider hydrates userInfo + // asynchronously, so the first effect run after a redirect return is always + // unauthenticated. The cancel discard must survive that flip — consuming + // the status on run 1 and acting on it on run 2 is the bug this pins. + signedIn = false; + setLocation('https://host.example/read?data_exchange_status=cancel'); + sessionStorage.setItem( + 'youversion-platform:pending-highlight', + JSON.stringify({ + verses: [16], + color: 'fffe00', + versionId: 111, + book: 'JHN', + chapter: '3', + timestamp: Date.now(), + }), + ); + const createHighlight = vi.spyOn(HighlightsClient.prototype, 'createHighlight'); + + const { result, rerender } = renderHook(() => useBibleReaderHighlights(options), { + wrapper: Providers, + }); + + // The session resolves after mount. + signedIn = true; + rerender(); + + await waitFor(() => { + expect(readPendingHighlight()).toBeNull(); + }); + expect(result.current.permissionDialogOpen).toBe(false); + expect(createHighlight).not.toHaveBeenCalled(); + }); + + it('discards pending on a failure return and does not re-open the dialog (async session hydration)', async () => { + signedIn = false; + setLocation('https://host.example/read?data_exchange_status=something-unexpected'); + sessionStorage.setItem( + 'youversion-platform:pending-highlight', + JSON.stringify({ + verses: [16], + color: 'fffe00', + versionId: 111, + book: 'JHN', + chapter: '3', + timestamp: Date.now(), + }), + ); + const createHighlight = vi.spyOn(HighlightsClient.prototype, 'createHighlight'); + + const { result, rerender } = renderHook(() => useBibleReaderHighlights(options), { + wrapper: Providers, + }); + + signedIn = true; + rerender(); + + await waitFor(() => { + expect(readPendingHighlight()).toBeNull(); + }); + expect(result.current.permissionDialogOpen).toBe(false); + expect(createHighlight).not.toHaveBeenCalled(); + }); +}); + +describe('highlight auth flow — write failure routing', () => { + beforeEach(() => { + signedIn = true; + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + }); + + it('401 invalidates the permission cache, keeps pending, and re-prompts', async () => { + vi.spyOn(console, 'error').mockImplementation(vi.fn()); + vi.spyOn(HighlightsClient.prototype, 'createHighlight').mockRejectedValue(httpError(401)); + + const { result } = renderHook(() => useBibleReaderHighlights(options), { wrapper: Providers }); + + act(() => { + expect(result.current.apply('fffe00', [16])).toBe('applied'); + }); + + await waitFor(() => { + expect(result.current.permissionDialogOpen).toBe(true); + }); + // Cache invalidated (server truth wins), overlay reverted, pending KEPT. + expect(YouVersionPlatformConfiguration.hasPermission('highlights')).toBe(false); + expect(result.current.highlightedVerses).toEqual({}); + expect(readPendingHighlight()).toMatchObject({ verses: [16], color: 'fffe00' }); + }); + + it('5xx reverts the overlay, logs, and discards pending without re-prompting', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(vi.fn()); + vi.spyOn(HighlightsClient.prototype, 'createHighlight').mockRejectedValue(httpError(500)); + + const { result } = renderHook(() => useBibleReaderHighlights(options), { wrapper: Providers }); + + act(() => { + result.current.apply('fffe00', [16]); + }); + + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({}); + }); + expect(result.current.permissionDialogOpen).toBe(false); + expect(readPendingHighlight()).toBeNull(); + expect(YouVersionPlatformConfiguration.hasPermission('highlights')).toBe(true); + expect(consoleError).toHaveBeenCalledWith( + expect.stringContaining('Failed to apply highlight'), + expect.anything(), + ); + }); +}); + +describe('highlight auth flow — operation queue', () => { + beforeEach(() => { + signedIn = true; + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + }); + + it('serializes overlapping apply→remove so DELETE never overtakes the in-flight POST', async () => { + const order: string[] = []; + let releaseCreate!: () => void; + const createGate = new Promise((resolve) => { + releaseCreate = resolve; + }); + + vi.spyOn(HighlightsClient.prototype, 'createHighlight').mockImplementation(async () => { + order.push('create:start'); + await createGate; + order.push('create:end'); + return { version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }; + }); + vi.spyOn(HighlightsClient.prototype, 'deleteHighlight').mockImplementation(async () => { + order.push('delete:start'); + await Promise.resolve(); + }); + + const { result } = renderHook(() => useBibleReaderHighlights(options), { wrapper: Providers }); + + // Apply then immediately remove the same verse. + act(() => { + result.current.apply('fffe00', [16]); + }); + act(() => { + result.current.remove('fffe00', [16]); + }); + + // Optimistic: last-issued (remove) wins the visual state. + expect(result.current.highlightedVerses).toEqual({}); + + // The DELETE must not start until the POST has fully settled. + await waitFor(() => expect(order).toContain('create:start')); + expect(order).not.toContain('delete:start'); + + releaseCreate(); + + await waitFor(() => expect(order).toContain('delete:start')); + expect(order).toEqual(['create:start', 'create:end', 'delete:start']); + // Settles to the last-issued operation's state. + expect(result.current.highlightedVerses).toEqual({}); + }); +}); diff --git a/packages/ui/src/components/use-bible-reader-highlights.integration.test.tsx b/packages/ui/src/components/use-bible-reader-highlights.integration.test.tsx new file mode 100644 index 00000000..c78992e3 --- /dev/null +++ b/packages/ui/src/components/use-bible-reader-highlights.integration.test.tsx @@ -0,0 +1,451 @@ +/** + * @vitest-environment jsdom + * + * Integration coverage for the seam hook through the REAL `useHighlights` and + * `useApiData` (nothing from the hooks package is module-mocked; only the core + * client's network method is stubbed). This exists because wholesale-mocking + * `useHighlights` hid a real bug: `useApiData`'s fetch effect didn't re-run + * when `enabled` flipped false→true, so a signed-in session resolving after + * mount never fetched highlights at all. + */ +import { act, renderHook, waitFor } from '@testing-library/react'; +import { + HighlightsClient, + YouVersionPlatformConfiguration, + type Collection, + type Highlight, + type YouVersionUserInfo, +} from '@youversion/platform-core'; +import { YouVersionAuthContext, YouVersionContext } from '@youversion/platform-react-hooks'; +import type { ReactNode } from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { HIGHLIGHTS_LIVE, setHighlightsLive } from '@/lib/feature-flags'; +import { useBibleReaderHighlights } from './use-bible-reader-highlights'; + +function collection(data: Highlight[]): Collection { + return { data, next_page_token: null }; +} + +const mockUserInfo = { id: 'user-1', name: 'Test User' } as unknown as YouVersionUserInfo; + +let signedIn = false; + +function Providers({ children }: { children: ReactNode }) { + return ( + + + {children} + + + ); +} + +const defaultOptions = { versionId: 111, book: 'JHN', chapter: '3' }; + +beforeEach(() => { + vi.restoreAllMocks(); + signedIn = false; + setHighlightsLive(true); +}); + +afterEach(() => { + vi.restoreAllMocks(); + setHighlightsLive(HIGHLIGHTS_LIVE); + localStorage.clear(); + sessionStorage.clear(); +}); + +describe('useBibleReaderHighlights — real useHighlights/useApiData', () => { + it('fetches and renders highlights when auth resolves after mount (enabled false→true)', async () => { + const getHighlights = vi.spyOn(HighlightsClient.prototype, 'getHighlights').mockResolvedValue({ + data: [ + { version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }, + { version_id: 111, passage_id: 'JHN.3.17', color: '5dff79' }, + ], + next_page_token: null, + }); + + // Mount signed out — mirrors YouVersionAuthProvider, which initializes + // userInfo to null and resolves the session asynchronously. + const { result, rerender } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: Providers, + }); + + expect(getHighlights).not.toHaveBeenCalled(); + expect(result.current.highlightedVerses).toEqual({}); + + // The session resolves: only `enabled` flips — no other dep changes. + signedIn = true; + rerender(); + + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00', 17: '5dff79' }); + }); + expect(getHighlights).toHaveBeenCalledTimes(1); + expect(getHighlights).toHaveBeenCalledWith({ version_id: 111, passage_id: 'JHN.3' }); + }); + + it('un-renders highlights immediately on sign-out and does not refetch', async () => { + const getHighlights = vi.spyOn(HighlightsClient.prototype, 'getHighlights').mockResolvedValue({ + data: [{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }], + next_page_token: null, + }); + + signedIn = true; + const { result, rerender } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: Providers, + }); + + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + }); + + signedIn = false; + rerender(); + + expect(result.current.highlightedVerses).toEqual({}); + await act(async () => { + await Promise.resolve(); + }); + expect(getHighlights).toHaveBeenCalledTimes(1); + }); +}); + +/** + * Write-path coverage through the REAL `useHighlights` + `useApiData` chain + * (only `HighlightsClient.prototype` network methods are spied). Auth is + * mounted signed OUT and flipped, matching the async session hydration of the + * shipped provider — never a synchronously-signed-in mount. + */ +describe('useBibleReaderHighlights — write reconciliation (Fix 2/3/4)', () => { + function mountFlipped() { + localStorage.clear(); + // The permission cache is user-scoped: persist a matching userInfo so the + // seeded grant is readable (the auth provider does this at sign-in). + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-1', name: 'Test User' }); + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); + const view = renderHook(() => useBibleReaderHighlights(defaultOptions), { wrapper: Providers }); + signedIn = true; + view.rerender(); + return view; + } + + it('apply: overlay wins when the post-write GET does not yet reflect the write (lag)', async () => { + // Mount GET and the single post-write refetch both come back empty — the + // server hasn't caught up to our POST yet (read-after-write lag). + const getHighlights = vi + .spyOn(HighlightsClient.prototype, 'getHighlights') + .mockResolvedValue(collection([])); + const createHighlight = vi + .spyOn(HighlightsClient.prototype, 'createHighlight') + .mockResolvedValue({ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }); + + const { result } = mountFlipped(); + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(1)); + + act(() => { + result.current.apply('fffe00', [16]); + }); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + + await waitFor(() => expect(createHighlight).toHaveBeenCalledTimes(1)); + // Exactly one coalesced refetch after the write. + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(2)); + // The GET is still empty, but the overlay wins — the highlight stays painted. + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + }); + + it('apply: once a GET reflects the write, the overlay retires and server truth renders', async () => { + const getHighlights = vi + .spyOn(HighlightsClient.prototype, 'getHighlights') + .mockResolvedValueOnce(collection([])) + .mockResolvedValue( + collection([{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }]), + ); + const createHighlight = vi + .spyOn(HighlightsClient.prototype, 'createHighlight') + .mockResolvedValue({ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }); + + const { result } = mountFlipped(); + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(1)); + + act(() => { + result.current.apply('fffe00', [16]); + }); + await waitFor(() => expect(createHighlight).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(2)); + + // The refetch reflects the write; the verse renders (now from server truth). + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + }); + + // Distinguish retired-overlay from still-masking-overlay (both render + // fffe00 above): the server now reports verse 16 in a DIFFERENT color, and + // a write to another verse triggers the next fetch. If verse 16's entry + // were still pending, its overlay would keep masking with fffe00; retired, + // the server's color must render. + getHighlights.mockResolvedValue( + collection([ + { version_id: 111, passage_id: 'JHN.3.16', color: '00d6ff' }, + { version_id: 111, passage_id: 'JHN.3.20', color: 'fffe00' }, + ]), + ); + act(() => { + result.current.apply('fffe00', [20]); + }); + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(3)); + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({ 16: '00d6ff', 20: 'fffe00' }); + }); + }); + + it('apply partial failure: succeeded run stays painted, failed run reverts, one refetch (regression)', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(vi.fn()); + // Post-write GET reflects the run that succeeded server-side ([2,3]). + const getHighlights = vi + .spyOn(HighlightsClient.prototype, 'getHighlights') + .mockResolvedValueOnce(collection([])) + .mockResolvedValue( + collection([ + { version_id: 111, passage_id: 'JHN.3.2', color: 'fffe00' }, + { version_id: 111, passage_id: 'JHN.3.3', color: 'fffe00' }, + ]), + ); + const createHighlight = vi + .spyOn(HighlightsClient.prototype, 'createHighlight') + .mockImplementation((data) => { + if (data.passage_id === 'JHN.3.5') return Promise.reject(new Error('network down')); + return Promise.resolve({ version_id: 111, passage_id: data.passage_id, color: data.color }); + }); + + const { result } = mountFlipped(); + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(1)); + + act(() => { + result.current.apply('fffe00', [2, 3, 5]); + }); + // Optimistic: all three painted. + expect(result.current.highlightedVerses).toEqual({ 2: 'fffe00', 3: 'fffe00', 5: 'fffe00' }); + + await waitFor(() => expect(createHighlight).toHaveBeenCalledTimes(2)); + // The refetch still fires despite the failure — exactly one for the batch. + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(2)); + + // Succeeded run [2,3] stays painted (reconciled to server truth); failed + // run [5] reverted. Before this fix the WHOLE batch reverted with no + // refetch, erasing the persisted 2-3 until the next navigation/write. + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({ 2: 'fffe00', 3: 'fffe00' }); + }); + await act(async () => { + await Promise.resolve(); + }); + expect(getHighlights).toHaveBeenCalledTimes(2); + consoleError.mockRestore(); + }); + + it('remove partial failure: no ghosts — succeeded DELETEs stay un-painted, failed one reverts (regression)', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(vi.fn()); + // Server starts with [2,3,4] green; the post-batch GET is a stale snapshot + // that still contains all three (read-after-write lag on the deletes). + const getHighlights = vi.spyOn(HighlightsClient.prototype, 'getHighlights').mockResolvedValue( + collection([ + { version_id: 111, passage_id: 'JHN.3.2', color: '5dff79' }, + { version_id: 111, passage_id: 'JHN.3.3', color: '5dff79' }, + { version_id: 111, passage_id: 'JHN.3.4', color: '5dff79' }, + ]), + ); + const deleteHighlight = vi + .spyOn(HighlightsClient.prototype, 'deleteHighlight') + .mockImplementation((passageId) => { + if (passageId === 'JHN.3.3') return Promise.reject(new Error('network down')); + return Promise.resolve(undefined); + }); + + const { result } = mountFlipped(); + await waitFor(() => + expect(result.current.highlightedVerses).toEqual({ + 2: '5dff79', + 3: '5dff79', + 4: '5dff79', + }), + ); + + act(() => { + result.current.remove('5dff79', [2, 3, 4]); + }); + expect(result.current.highlightedVerses).toEqual({}); + + await waitFor(() => expect(deleteHighlight).toHaveBeenCalledTimes(3)); + // One refetch despite the failure. + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(2)); + + // Verses 2 and 4 were deleted server-side: their remove overlay holds + // against the stale snapshot (no ghosts). Verse 3's DELETE failed: it + // reverts and renders highlighted again from the fetched data. + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({ 3: '5dff79' }); + }); + await act(async () => { + await Promise.resolve(); + }); + expect(getHighlights).toHaveBeenCalledTimes(2); + consoleError.mockRestore(); + }); + + it('apply total failure: everything reverts and the batch still refetches exactly once', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(vi.fn()); + const getHighlights = vi + .spyOn(HighlightsClient.prototype, 'getHighlights') + .mockResolvedValue(collection([])); + vi.spyOn(HighlightsClient.prototype, 'createHighlight').mockRejectedValue( + new Error('network down'), + ); + + const { result } = mountFlipped(); + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(1)); + + act(() => { + result.current.apply('fffe00', [16, 17]); + }); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00', 17: 'fffe00' }); + + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({}); + }); + // The batch refetch fires on the all-fail path too — and only once. + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(2)); + await act(async () => { + await Promise.resolve(); + }); + expect(getHighlights).toHaveBeenCalledTimes(2); + consoleError.mockRestore(); + }); + + it('remove: overlay wins when the post-delete GET still contains the removed row (lag)', async () => { + // Server starts with the row and, due to lag, still returns it after DELETE. + const getHighlights = vi + .spyOn(HighlightsClient.prototype, 'getHighlights') + .mockResolvedValue( + collection([{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }]), + ); + const deleteHighlight = vi + .spyOn(HighlightsClient.prototype, 'deleteHighlight') + .mockResolvedValue(undefined); + + const { result } = mountFlipped(); + await waitFor(() => expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' })); + + act(() => { + result.current.remove('fffe00', [16]); + }); + expect(result.current.highlightedVerses).toEqual({}); + + await waitFor(() => + expect(deleteHighlight).toHaveBeenCalledWith('JHN.3.16', { version_id: 111 }), + ); + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(2)); + // The GET still contains the row, but the removal overlay wins — no resurrection. + expect(result.current.highlightedVerses).toEqual({}); + }); + + it('apply: a genuine write failure reverts the optimistic overlay', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(vi.fn()); + vi.spyOn(HighlightsClient.prototype, 'getHighlights').mockResolvedValue(collection([])); + vi.spyOn(HighlightsClient.prototype, 'createHighlight').mockRejectedValue( + new Error('network down'), + ); + + const { result } = mountFlipped(); + await waitFor(() => { + // wait for the mount fetch to settle + expect(result.current.highlightedVerses).toEqual({}); + }); + + act(() => { + result.current.apply('fffe00', [16]); + }); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({}); + }); + consoleError.mockRestore(); + }); + + it('apply of [2,3,5] POSTs two runs but issues exactly one refetch (Fix 3)', async () => { + const getHighlights = vi + .spyOn(HighlightsClient.prototype, 'getHighlights') + .mockResolvedValue(collection([])); + const createHighlight = vi + .spyOn(HighlightsClient.prototype, 'createHighlight') + .mockResolvedValue({ version_id: 111, passage_id: 'JHN.3.2', color: 'fffe00' }); + + const { result } = mountFlipped(); + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(1)); + + act(() => { + result.current.apply('fffe00', [2, 3, 5]); + }); + + await waitFor(() => expect(createHighlight).toHaveBeenCalledTimes(2)); + expect(createHighlight).toHaveBeenCalledWith({ + version_id: 111, + passage_id: 'JHN.3.2-3', + color: 'fffe00', + }); + expect(createHighlight).toHaveBeenCalledWith({ + version_id: 111, + passage_id: 'JHN.3.5', + color: 'fffe00', + }); + + // One refetch for the whole batch → 2 GETs total (mount + 1). + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(2)); + await act(async () => { + await Promise.resolve(); + }); + expect(getHighlights).toHaveBeenCalledTimes(2); + }); + + it('remove of a contiguous [2,3] issues per-verse DELETEs and one refetch (Fix 4 + 3)', async () => { + const getHighlights = vi.spyOn(HighlightsClient.prototype, 'getHighlights').mockResolvedValue( + collection([ + { version_id: 111, passage_id: 'JHN.3.2', color: 'fffe00' }, + { version_id: 111, passage_id: 'JHN.3.3', color: 'fffe00' }, + ]), + ); + const deleteHighlight = vi + .spyOn(HighlightsClient.prototype, 'deleteHighlight') + .mockResolvedValue(undefined); + + const { result } = mountFlipped(); + await waitFor(() => + expect(result.current.highlightedVerses).toEqual({ 2: 'fffe00', 3: 'fffe00' }), + ); + + act(() => { + result.current.remove('fffe00', [2, 3]); + }); + + await waitFor(() => expect(deleteHighlight).toHaveBeenCalledTimes(2)); + // Per-verse passage ids, NOT the range `JHN.3.2-3`. + expect(deleteHighlight).toHaveBeenCalledWith('JHN.3.2', { version_id: 111 }); + expect(deleteHighlight).toHaveBeenCalledWith('JHN.3.3', { version_id: 111 }); + + // One coalesced refetch for the whole removal → 2 GETs total (mount + 1). + await waitFor(() => expect(getHighlights).toHaveBeenCalledTimes(2)); + await act(async () => { + await Promise.resolve(); + }); + expect(getHighlights).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/ui/src/components/use-bible-reader-highlights.test.tsx b/packages/ui/src/components/use-bible-reader-highlights.test.tsx new file mode 100644 index 00000000..5d238cdd --- /dev/null +++ b/packages/ui/src/components/use-bible-reader-highlights.test.tsx @@ -0,0 +1,490 @@ +/** + * @vitest-environment jsdom + */ +import { act, renderHook, waitFor } from '@testing-library/react'; +import type { Collection, Highlight } from '@youversion/platform-core'; +import { useHighlights, YouVersionAuthContext } from '@youversion/platform-react-hooks'; +import { + YouVersionPlatformConfiguration, + type YouVersionUserInfo, +} from '@youversion/platform-core'; +import type { ReactNode } from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { HIGHLIGHTS_LIVE, setHighlightsLive } from '@/lib/feature-flags'; +import { useBibleReaderHighlights } from './use-bible-reader-highlights'; + +vi.mock('@youversion/platform-react-hooks', async () => { + const actual = await vi.importActual('@youversion/platform-react-hooks'); + return { + ...actual, + useHighlights: vi.fn(), + }; +}); + +const mockUserInfo = { id: 'user-1', name: 'Test User' } as unknown as YouVersionUserInfo; + +function makeCollection(data: Highlight[]): Collection { + return { data, next_page_token: null }; +} + +function mockUseHighlights( + overrides: Partial> = {}, +): ReturnType { + const value: ReturnType = { + highlights: makeCollection([]), + loading: false, + error: null, + refetch: vi.fn(), + createHighlight: vi.fn().mockResolvedValue({ + version_id: 111, + passage_id: 'JHN.3.16', + color: 'fffe00', + }), + deleteHighlight: vi.fn().mockResolvedValue(undefined), + ...overrides, + }; + vi.mocked(useHighlights).mockReturnValue(value); + return value; +} + +/** + * Auth wrapper whose signed-in state can be flipped between rerenders (the + * wrapper re-runs on `rerender()`, picking up the new value). + */ +let signedIn = true; +function AuthWrapper({ children }: { children: ReactNode }) { + return ( + + {children} + + ); +} + +const defaultOptions = { versionId: 111, book: 'JHN', chapter: '3' }; + +beforeEach(() => { + vi.clearAllMocks(); + localStorage.clear(); + sessionStorage.clear(); + signedIn = true; + setHighlightsLive(true); + // The permission cache is user-scoped, so it only takes effect once a matching + // userInfo is persisted (the auth provider does this at sign-in). Seed both so + // the authorized-write path is exercised. The auth-flow branches (missing + // session/permission) have their own dedicated coverage. + YouVersionPlatformConfiguration.saveUserInfo({ id: 'user-1', name: 'Test User' }); + YouVersionPlatformConfiguration.saveGrantedPermissions(['highlights']); +}); + +afterEach(() => { + setHighlightsLive(HIGHLIGHTS_LIVE); + localStorage.clear(); + sessionStorage.clear(); +}); + +describe('useBibleReaderHighlights — flag off (dark launch)', () => { + it('is fully inert: fetch disabled, empty map, writes are no-ops', () => { + setHighlightsLive(false); + const mocked = mockUseHighlights({ + highlights: makeCollection([{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }]), + }); + + const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + + // The fetch gate is `enabled: false` — useApiData skips the request entirely. + expect(vi.mocked(useHighlights)).toHaveBeenCalledWith( + { version_id: 111, passage_id: 'JHN.3' }, + { enabled: false }, + ); + // Even stale fetched data must not render while the flag is off. + expect(result.current.highlightedVerses).toEqual({}); + + act(() => { + result.current.apply('fffe00', [16, 17]); + result.current.remove('fffe00', [16]); + }); + expect(mocked.createHighlight).not.toHaveBeenCalled(); + expect(mocked.deleteHighlight).not.toHaveBeenCalled(); + expect(result.current.highlightedVerses).toEqual({}); + }); +}); + +describe('useBibleReaderHighlights — auth guarding', () => { + it('renders without crashing when no auth provider is mounted, treated as signed out', () => { + const mocked = mockUseHighlights(); + + // No wrapper: YouVersionAuthContext is null (useYVAuth would throw here). + const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions)); + + expect(vi.mocked(useHighlights)).toHaveBeenCalledWith( + { version_id: 111, passage_id: 'JHN.3' }, + { enabled: false }, + ); + expect(result.current.highlightedVerses).toEqual({}); + + act(() => { + result.current.apply('fffe00', [16]); + }); + expect(mocked.createHighlight).not.toHaveBeenCalled(); + }); + + it('is non-interactive with no auth provider, even with the flag on (inert color row)', () => { + mockUseHighlights(); + + // Flag on but no auth provider: the machine is disabled, so a color tap can + // never do anything. The color-swatch row must not render — this flag is + // what BibleReader ANDs with the feature flag to hide it. + const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions)); + expect(result.current.highlightsInteractive).toBe(false); + }); + + it('is interactive when an auth provider is mounted (flag on), even signed out', () => { + mockUseHighlights(); + signedIn = false; + + // Signed out but with an auth provider present: a tap still enters the + // sign-in flow, so the row stays interactive. + const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + expect(result.current.highlightsInteractive).toBe(true); + }); + + it('clears rendered highlights immediately when the user signs out', () => { + mockUseHighlights({ + highlights: makeCollection([{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }]), + }); + + const { result, rerender } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + + // Sign out: the mock still returns fetched data (like a not-yet-cleared + // cache would), so this pins the hook's own render gate. + signedIn = false; + rerender(); + expect(result.current.highlightedVerses).toEqual({}); + }); +}); + +describe('useBibleReaderHighlights — fetched highlights', () => { + it('maps per-verse USFMs for the current chapter into the verse map', () => { + mockUseHighlights({ + highlights: makeCollection([ + { version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }, + { version_id: 111, passage_id: 'JHN.3.17', color: '5DFF79' }, // uppercase from server + { version_id: 111, passage_id: 'JHN.4.1', color: '00d6ff' }, // other chapter + { version_id: 999, passage_id: 'JHN.3.2', color: '00d6ff' }, // other version + ]), + }); + + const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + + expect(vi.mocked(useHighlights)).toHaveBeenCalledWith( + { version_id: 111, passage_id: 'JHN.3' }, + { enabled: true }, + ); + expect(result.current.highlightedVerses).toEqual({ + 16: 'fffe00', + 17: '5dff79', + }); + }); +}); + +describe('useBibleReaderHighlights — apply', () => { + it('applies optimistically and POSTs contiguous runs as range USFMs', async () => { + const mocked = mockUseHighlights(); + + const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + + act(() => { + result.current.apply('FFFE00', [16, 17, 18, 20]); + }); + + // Optimistic: rendered before any network round-trip settles. + expect(result.current.highlightedVerses).toEqual({ + 16: 'fffe00', + 17: 'fffe00', + 18: 'fffe00', + 20: 'fffe00', + }); + + await waitFor(() => { + expect(mocked.createHighlight).toHaveBeenCalledTimes(2); + }); + expect(mocked.createHighlight).toHaveBeenCalledWith({ + version_id: 111, + passage_id: 'JHN.3.16-18', + color: 'fffe00', // lowercased on the wire + }); + expect(mocked.createHighlight).toHaveBeenCalledWith({ + version_id: 111, + passage_id: 'JHN.3.20', + color: 'fffe00', + }); + // Two POSTs (two runs) but a SINGLE coalesced refetch for the batch (Fix 3). + expect(mocked.refetch).toHaveBeenCalledTimes(1); + }); + + it('reverts the optimistic overlay and logs when the write fails', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(vi.fn()); + const mocked = mockUseHighlights({ + createHighlight: vi.fn().mockRejectedValue(new Error('network down')), + }); + + const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + + act(() => { + result.current.apply('fffe00', [16, 17]); + }); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00', 17: 'fffe00' }); + + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({}); + }); + expect(mocked.createHighlight).toHaveBeenCalledTimes(1); + expect(consoleError).toHaveBeenCalledWith( + expect.stringContaining('Failed to apply highlight'), + expect.objectContaining({ passageId: 'JHN.3.16-17' }), + ); + consoleError.mockRestore(); + }); +}); + +describe('useBibleReaderHighlights — overlay reconciliation (Fix 2)', () => { + it('holds the overlay until a fetch REFLECTS the write, then retires it to server truth', async () => { + const mocked = mockUseHighlights(); + + const { result, rerender } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + + act(() => { + result.current.apply('fffe00', [16]); + }); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + + // Let the write settle successfully. + await waitFor(() => { + expect(mocked.createHighlight).toHaveBeenCalledTimes(1); + }); + await act(async () => { + await Promise.resolve(); + }); + + // The post-write GET lands but does NOT yet contain the write (read-after- + // write lag). The overlay must WIN — no flicker out and back. + mockUseHighlights({ highlights: makeCollection([]) }); + rerender(); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + + // A later GET reflects the write (verse present in the written color). + mockUseHighlights({ + highlights: makeCollection([{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }]), + }); + rerender(); + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + }); + + // Prove the overlay entry was retired: with the entry gone, server truth now + // drives the verse, so clearing it server-side un-paints it. + mockUseHighlights({ highlights: makeCollection([]) }); + rerender(); + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({}); + }); + }); + + it('holds the overlay when the server converges on a DIFFERENT color (overlay wins until navigation)', async () => { + const mocked = mockUseHighlights(); + + const { result, rerender } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + + act(() => { + result.current.apply('fffe00', [16]); + }); + await waitFor(() => { + expect(mocked.createHighlight).toHaveBeenCalledTimes(1); + }); + await act(async () => { + await Promise.resolve(); + }); + + // A GET returns the verse in a color that is NOT what we wrote. That doesn't + // reflect our write, so the overlay is held rather than snapping to it. + mockUseHighlights({ + highlights: makeCollection([{ version_id: 111, passage_id: 'JHN.3.16', color: '00d6ff' }]), + }); + rerender(); + await act(async () => { + await Promise.resolve(); + }); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + }); +}); + +describe('useBibleReaderHighlights — vapor bug (removed highlight resurrection)', () => { + // Regression for the staging "vapor" report: a deleted highlight reappears for + // a split second, then disappears. Root cause: the reconcile step retired a + // REMOVE overlay entry as soon as any fetch reflected the removal; a later + // response from a stale read replica that still contained the highlight then + // had nothing suppressing it, so the verse repainted until the next fetch. + it('a stale fetch after a settled+reflected DELETE does not resurrect the removed highlight', async () => { + const mocked = mockUseHighlights({ + highlights: makeCollection([{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }]), + }); + + const { result, rerender } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + + act(() => { + result.current.remove('fffe00', [16]); + }); + // Optimistic removal. + expect(result.current.highlightedVerses).toEqual({}); + + await waitFor(() => { + expect(mocked.deleteHighlight).toHaveBeenCalledTimes(1); + }); + await act(async () => { + await Promise.resolve(); + }); + + // Fetch A reflects the removal (server no longer shows the color). + mockUseHighlights({ highlights: makeCollection([]) }); + rerender(); + expect(result.current.highlightedVerses).toEqual({}); + + // Fetch B is a STALE read replica that still contains the removed highlight. + // The removal overlay must be HELD so the highlight does not resurrect. + mockUseHighlights({ + highlights: makeCollection([{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }]), + }); + rerender(); + expect(result.current.highlightedVerses).toEqual({}); + + // Fetch C is consistent again (still removed) — no flicker at any point. + mockUseHighlights({ highlights: makeCollection([]) }); + rerender(); + expect(result.current.highlightedVerses).toEqual({}); + }); +}); + +describe('useBibleReaderHighlights — remove', () => { + it('removes optimistically and DELETEs one passage-id per verse (never a range)', async () => { + const mocked = mockUseHighlights({ + highlights: makeCollection([ + { version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }, + { version_id: 111, passage_id: 'JHN.3.17', color: 'fffe00' }, + { version_id: 111, passage_id: 'JHN.3.18', color: '5dff79' }, // different color, stays + ]), + }); + + const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + + act(() => { + result.current.remove('fffe00', [16, 17, 18]); + }); + + // Optimistic: yellow verses gone immediately, green untouched. + expect(result.current.highlightedVerses).toEqual({ 18: '5dff79' }); + + // Contiguous [16,17] must NOT collapse to `JHN.3.16-17` — range delete is + // unsupported server-side (Fix 4). One DELETE per verse instead. + await waitFor(() => { + expect(mocked.deleteHighlight).toHaveBeenCalledTimes(2); + }); + expect(mocked.deleteHighlight).toHaveBeenCalledWith('JHN.3.16', { version_id: 111 }); + expect(mocked.deleteHighlight).toHaveBeenCalledWith('JHN.3.17', { version_id: 111 }); + // The whole removal coalesces into a single refetch (Fix 3). + expect(mocked.refetch).toHaveBeenCalledTimes(1); + }); + + it('reverts the optimistic removal and logs when the delete fails', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(vi.fn()); + mockUseHighlights({ + highlights: makeCollection([{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }]), + deleteHighlight: vi.fn().mockRejectedValue(new Error('network down')), + }); + + const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + + act(() => { + result.current.remove('fffe00', [16]); + }); + expect(result.current.highlightedVerses).toEqual({}); + + await waitFor(() => { + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + }); + expect(consoleError).toHaveBeenCalledWith( + expect.stringContaining('Failed to remove highlight'), + expect.objectContaining({ passageId: 'JHN.3.16' }), + ); + consoleError.mockRestore(); + }); + + it('is a no-op when no selected verse is rendered in the given color', () => { + const mocked = mockUseHighlights({ + highlights: makeCollection([{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }]), + }); + + const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), { + wrapper: AuthWrapper, + }); + + act(() => { + result.current.remove('5dff79', [16]); + }); + expect(mocked.deleteHighlight).not.toHaveBeenCalled(); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + }); +}); + +describe('useBibleReaderHighlights — scope changes', () => { + it('drops the optimistic overlay when the chapter changes', () => { + const neverSettles = new Promise(vi.fn()); + mockUseHighlights({ + createHighlight: vi.fn().mockReturnValue(neverSettles), + }); + + const { result, rerender } = renderHook( + (props: { versionId: number; book: string; chapter: string }) => + useBibleReaderHighlights(props), + { wrapper: AuthWrapper, initialProps: defaultOptions }, + ); + + act(() => { + result.current.apply('fffe00', [16]); + }); + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + + rerender({ versionId: 111, book: 'JHN', chapter: '4' }); + expect(result.current.highlightedVerses).toEqual({}); + }); +}); diff --git a/packages/ui/src/components/use-bible-reader-highlights.ts b/packages/ui/src/components/use-bible-reader-highlights.ts new file mode 100644 index 00000000..15025352 --- /dev/null +++ b/packages/ui/src/components/use-bible-reader-highlights.ts @@ -0,0 +1,276 @@ +'use client'; + +import { isHighlightsLive } from '@/lib/feature-flags'; +import { + bibleReaderHighlightsMachine, + scopesEqual, + selectHighlightedVerses, + type HighlightScope, + type HighlightServices, + type ServerColors, +} from './bible-reader-highlights-machine'; +import { + useHighlightAuthActions, + useHighlights, + YouVersionAuthContext, +} from '@youversion/platform-react-hooks'; +import { useActorRef, useSelector } from '@xstate/react'; +import { useContext, useEffect, useMemo, useRef } from 'react'; + +export type UseBibleReaderHighlightsOptions = { + versionId: number; + book: string; + chapter: string; +}; + +export type UseBibleReaderHighlightsReturn = { + /** Verse number → hex color (lowercase, no `#`) for the current chapter. */ + highlightedVerses: Record; + /** + * Highlights the given verses in `color`. Bridge-safe: primitives only. + * When the user has a session and the highlights permission this writes + * optimistically (`'applied'`); otherwise it opens the sign-in dialog (signed + * out) or the just-in-time permission dialog (signed in, no permission) and + * returns `'flow'`. Returns `'noop'` when highlighting is inert (flag off, no + * verses, or no auth provider). The caller uses the outcome to decide whether + * to keep the verse selection: `'flow'` keeps it so cancelling the dialog + * leaves the selection and popover intact. + */ + apply: (color: string, verses: number[]) => 'applied' | 'flow' | 'noop'; + /** Clears the given verses that are currently highlighted in `color`. */ + remove: (color: string, verses: number[]) => void; + /** + * Whether highlighting can actually function in this mount — i.e. a + * `YouVersionAuthProvider` is present so a color tap can enter the auth flow + * and writes can reach the API. `false` for copy/share-only integrators (no + * auth provider), where the machine sits in `disabled` and taps resolve to + * `noop`. The caller ANDs this with the feature flag to decide whether to + * render the (otherwise inert) color-swatch row. + */ + highlightsInteractive: boolean; + /** Whether the just-in-time permission confirm dialog is open. */ + permissionDialogOpen: boolean; + /** Controlled open-change for the permission confirm dialog. */ + onPermissionDialogOpenChange: (open: boolean) => void; + /** User accepted the dialog → start the data-exchange grant (full-page redirect). */ + confirmPermissionDialog: () => void; + /** User declined/dismissed the dialog → discard the pending highlight. */ + cancelPermissionDialog: () => void; + /** Whether the sign-in introduction dialog is open (signed-out color tap). */ + signInDialogOpen: boolean; + /** User accepted the sign-in dialog → start the sign-in redirect. */ + confirmSignInDialog: () => void; + /** User declined/dismissed the sign-in dialog → discard the pending highlight. */ + cancelSignInDialog: () => void; +}; + +/** + * Parses the fetched highlights into a verse→color map for the current scope, + * exactly as `selectHighlightedVerses` expects the server side. + */ +function parseServerColors( + highlights: { data?: { version_id: number; passage_id: string; color: string }[] } | null, + versionId: number, + chapterUsfm: string, +): ServerColors { + const map: ServerColors = {}; + const versePrefix = `${chapterUsfm}.`; + for (const highlight of highlights?.data ?? []) { + if (highlight.version_id !== versionId) continue; + if (!highlight.passage_id.startsWith(versePrefix)) continue; + const verse = parseInt(highlight.passage_id.slice(versePrefix.length), 10); + if (verse > 0) map[verse] = highlight.color.toLowerCase(); + } + return map; +} + +function serverColorsEqual(a: ServerColors, b: ServerColors): boolean { + const aKeys = Object.keys(a); + if (aKeys.length !== Object.keys(b).length) return false; + for (const key of aKeys) { + if (a[Number(key)] !== b[Number(key)]) return false; + } + return true; +} + +/** + * BibleReader's seam onto the highlights API (YPE-1034, self-contained mode). A + * THIN adapter over `bibleReaderHighlightsMachine` (PR-288): it reads auth + + * flag + fetched highlights from React and feeds them to the machine as events, + * exposes the machine's dialog states + write commands, and derives the rendered + * verse map. All flow/write logic (optimistic overlay, serialized writes, + * per-verse ownership, reconcile, auth flow, the vapor fix) lives in the + * machine; see that file for the invariants and the statechart. + * + * Rendering and fetching are gated on `isHighlightsLive() && isAuthenticated`. + * With no auth provider the reader keeps the PR-1 posture: no fetch, no writes, + * and a color tap never enters the auth flow — copy/share still work. + */ +export function useBibleReaderHighlights({ + versionId, + book, + chapter, +}: UseBibleReaderHighlightsOptions): UseBibleReaderHighlightsReturn { + // Read the auth context directly instead of `useYVAuth`, which throws when no + // auth provider is mounted. + const authContext = useContext(YouVersionAuthContext); + const hasAuthProvider = authContext !== null; + const isAuthenticated = Boolean(authContext?.userInfo); + const flagOn = isHighlightsLive(); + const live = flagOn && isAuthenticated; + + const { + hasHighlightsPermission, + invalidateHighlightsPermission, + consumeDataExchangeReturn, + startSignInForHighlights, + startDataExchangeForHighlights, + } = useHighlightAuthActions(); + + const chapterUsfm = `${book}.${chapter}`; + const { highlights, createHighlight, deleteHighlight, refetch } = useHighlights( + { version_id: versionId, passage_id: chapterUsfm }, + { enabled: live }, + ); + + // A stable ref bag of the live SDK service closures. Passed once to the machine + // via `input`; the machine reads `.current` at call time so it always sees the + // latest closures without re-spawning. + const servicesRef = useRef(null as unknown as HighlightServices); + servicesRef.current = { + createHighlight, + deleteHighlight, + refetch, + hasHighlightsPermission, + invalidateHighlightsPermission, + consumeDataExchangeReturn, + startSignInForHighlights, + startDataExchangeForHighlights, + }; + + const scope: HighlightScope = useMemo( + () => ({ versionId, book, chapter }), + [versionId, book, chapter], + ); + + const actorRef = useActorRef(bibleReaderHighlightsMachine, { + input: { + services: servicesRef, + scope, + flagOn, + hasAuthProvider, + isAuthenticated, + }, + }); + + // ── Feed React-owned inputs to the machine ────────────────────────────────── + useEffect(() => { + actorRef.send({ type: 'AUTH_CHANGED', flagOn, hasAuthProvider, isAuthenticated }); + }, [actorRef, flagOn, hasAuthProvider, isAuthenticated]); + + useEffect(() => { + actorRef.send({ type: 'SCOPE_CHANGED', scope }); + }, [actorRef, scope]); + + // Parse the fetch into server truth and forward it whenever it changes. The + // machine reconciles the optimistic overlay against it. + // + // `useApiData` swaps `highlights` for a fresh object on every refetch, even + // when the content is byte-identical. Parsing off that identity would mint a + // new `serverColors` each time and cascade a new `highlightedVerses` reference + // → a chapter-wide verse-style re-sweep (verse.tsx keys a useLayoutEffect on + // it). Hold the prior parsed reference when the content is unchanged so the + // downstream memos stay reference-stable across no-op refetches. (This is + // separate from `lastSentServerColorsRef`, which dedups machine sends.) + const parsedServerColorsRef = useRef(null); + const serverColors = useMemo(() => { + const parsed = parseServerColors(highlights, versionId, chapterUsfm); + const previous = parsedServerColorsRef.current; + if (previous !== null && serverColorsEqual(previous, parsed)) { + return previous; + } + parsedServerColorsRef.current = parsed; + return parsed; + }, [highlights, versionId, chapterUsfm]); + const lastSentServerColorsRef = useRef(null); + useEffect(() => { + if ( + lastSentServerColorsRef.current !== null && + serverColorsEqual(lastSentServerColorsRef.current, serverColors) + ) { + return; + } + lastSentServerColorsRef.current = serverColors; + actorRef.send({ type: 'HIGHLIGHTS_UPDATED', serverColors }); + }, [actorRef, serverColors]); + + // ── Rendered verse map ────────────────────────────────────────────────────── + const overlay = useSelector(actorRef, (state) => state.context.overlay); + const machineScope = useSelector(actorRef, (state) => state.context.scope); + const highlightedVerses = useMemo(() => { + // Gate on `live`: sign-out or flag-off must render nothing this very render, + // including optimistic overlay entries still in the machine. + if (!live) return {}; + // Only apply the overlay when the machine's scope matches the current one. + // On a synchronous scope change (before the SCOPE_CHANGED effect runs) the + // machine scope still points at the old chapter, so the overlay is skipped + // and the new chapter renders from server truth alone — verse numbers + // collide across chapters. + if (!scopesEqual(machineScope, scope)) return { ...serverColors }; + return selectHighlightedVerses(serverColors, overlay); + }, [live, serverColors, overlay, machineScope, scope]); + + // ── Dialog state + commands ───────────────────────────────────────────────── + const signInDialogOpen = useSelector(actorRef, (state) => + state.matches({ enabled: { flow: 'signInDialog' } }), + ); + const permissionDialogOpen = useSelector(actorRef, (state) => + state.matches({ enabled: { flow: 'permissionDialog' } }), + ); + + const api = useMemo< + Pick< + UseBibleReaderHighlightsReturn, + | 'apply' + | 'remove' + | 'onPermissionDialogOpenChange' + | 'confirmPermissionDialog' + | 'cancelPermissionDialog' + | 'confirmSignInDialog' + | 'cancelSignInDialog' + > + >( + () => ({ + apply: (color, verses) => { + actorRef.send({ type: 'TAP_COLOR', color, verses }); + return actorRef.getSnapshot().context.lastTapOutcome; + }, + remove: (color, verses) => { + actorRef.send({ type: 'REMOVE', color, verses }); + }, + onPermissionDialogOpenChange: (open) => { + // Dismiss via outside-click / Escape is a decline: discard the pending + // highlight but leave the verse selection untouched (the caller owns it). + if (!open) actorRef.send({ type: 'CANCEL_PERMISSION' }); + }, + confirmPermissionDialog: () => actorRef.send({ type: 'CONFIRM_PERMISSION' }), + cancelPermissionDialog: () => actorRef.send({ type: 'CANCEL_PERMISSION' }), + confirmSignInDialog: () => actorRef.send({ type: 'CONFIRM_SIGN_IN' }), + cancelSignInDialog: () => actorRef.send({ type: 'DECLINE_SIGN_IN' }), + }), + [actorRef], + ); + + return { + highlightedVerses, + // Interactivity mirrors the machine's enabled/disabled gate: with no auth + // provider the machine is inert and the color row must not render. The flag + // is ANDed in by the caller. (`live` also folds in `isAuthenticated`, which + // we intentionally exclude here — a signed-out tap still enters the sign-in + // flow, so the row stays interactive.) + highlightsInteractive: hasAuthProvider, + permissionDialogOpen, + signInDialogOpen, + ...api, + }; +} diff --git a/packages/ui/src/components/verse-action-popover.test.tsx b/packages/ui/src/components/verse-action-popover.test.tsx index 074b8902..8eb2b090 100644 --- a/packages/ui/src/components/verse-action-popover.test.tsx +++ b/packages/ui/src/components/verse-action-popover.test.tsx @@ -467,4 +467,89 @@ describe('VerseActionPopover', () => { expect(applyButtons).toHaveLength(5); }); }); + + function applyButtons() { + return screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply')); + } + + function clearButtons() { + return screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear')); + } + + describe('Active swatch checkmark', () => { + // The checkmark path (Swift #179) starts at these coords — asserts we render + // the check, not the old X. + const CHECK_PATH_PREFIX = 'M19.6627'; + + it('renders a checkmark (not an X) on the active/remove swatch', () => { + render( + ([HIGHLIGHT_COLORS[0]])} + selectedVerses={[1]} + highlightedVerses={{ 1: HIGHLIGHT_COLORS[0] }} + />, + ); + + const removeButton = screen.getByRole('button', { name: /Clear highlight/ }); + const path = removeButton.querySelector('svg path'); + expect(path?.getAttribute('d')).toContain(CHECK_PATH_PREFIX); + }); + + it('apply swatches render no icon', () => { + render(); + applyButtons().forEach((btn) => { + expect(btn.querySelector('svg')).toBeNull(); + }); + }); + + it('tapping the checkmark swatch still removes the highlight', () => { + const onClearHighlight = vi.fn(); + render( + ([HIGHLIGHT_COLORS[0]])} + selectedVerses={[1]} + highlightedVerses={{ 1: HIGHLIGHT_COLORS[0] }} + onClearHighlight={onClearHighlight} + />, + ); + + fireEvent.click(screen.getByRole('button', { name: /Clear highlight/ })); + expect(onClearHighlight).toHaveBeenCalledWith(HIGHLIGHT_COLORS[0]); + }); + }); + + describe('Highlights disabled (flag off)', () => { + it('hides the color row and remove circles but keeps Copy / Share', () => { + render( + ([HIGHLIGHT_COLORS[0]])} + selectedVerses={[1]} + highlightedVerses={{ 1: HIGHLIGHT_COLORS[0] }} + />, + ); + + // No color group, no apply circles, no remove circles. + expect(screen.queryByRole('group', { name: 'Highlight colors' })).toBeNull(); + expect(applyButtons()).toHaveLength(0); + expect(clearButtons()).toHaveLength(0); + + // Copy / Share remain. + expect(screen.getByText('Copy')).toBeTruthy(); + expect(screen.getByText('Share')).toBeTruthy(); + }); + + it('still shows the color row by default (highlightsEnabled defaults to true)', () => { + render(); + expect(screen.getByRole('group', { name: 'Highlight colors' })).toBeTruthy(); + expect(applyButtons()).toHaveLength(5); + }); + }); }); diff --git a/packages/ui/src/components/verse-action-popover.tsx b/packages/ui/src/components/verse-action-popover.tsx index ab6fe181..ac91bf1f 100644 --- a/packages/ui/src/components/verse-action-popover.tsx +++ b/packages/ui/src/components/verse-action-popover.tsx @@ -5,15 +5,15 @@ import i18n from '@/i18n'; import { cn } from '../lib/utils'; import { BoxStackIcon } from './icons/box-stack'; import { BoxArrowUpIcon } from './icons/box-arrow-up'; -import { XIcon } from './icons/x'; +import { CheckIcon } from './icons/check'; type Measurable = { getBoundingClientRect: () => DOMRect }; /** * Highlight colors, as 6-digit lowercase hex (no `#`) so they map 1:1 onto the - * future API `highlight.color` field (/^[0-9a-f]{6}$/). Order is the canonical - * apply order: yellow, green, blue, orange, pink. Hardcoded to match the - * YouVersion iOS app exactly. + * API `highlight.color` field (/^[0-9a-f]{6}$/). Order is the canonical apply + * order: yellow, green, blue, orange, pink. Hardcoded to match the YouVersion + * iOS app exactly. */ export const HIGHLIGHT_COLORS = ['fffe00', '5dff79', '00d6ff', 'ffc66f', 'ff95ef'] as const; @@ -32,6 +32,12 @@ type VerseActionPopoverProps = { * reachable instead of leaving with the verse. Omit for a purely anchored bar. */ scrollRoot?: HTMLElement | null; + /** + * Whether the highlights UI is available. When `false` (the `HIGHLIGHTS_LIVE` + * dark-launch flag is off) the color row and the remove (checkmark) circles are + * hidden entirely — only Copy / Share remain. Defaults to `true`. + */ + highlightsEnabled?: boolean; onHighlight: (color: string) => void; onClearHighlight: (color: string) => void; onCopy: () => void; @@ -41,12 +47,12 @@ type VerseActionPopoverProps = { type ColorCircleProps = { color: string; - showX: boolean; + showRemove: boolean; label: string; onClick: () => void; }; -function ColorCircle({ color, showX, label, onClick }: ColorCircleProps) { +function ColorCircle({ color, showRemove, label, onClick }: ColorCircleProps) { return ( ); } @@ -103,6 +111,7 @@ export const VerseActionPopover: FC = ({ highlightedVerses, anchorElement, scrollRoot, + highlightsEnabled = true, onHighlight, onClearHighlight, onCopy, @@ -194,10 +203,10 @@ export const VerseActionPopover: FC = ({ ? HIGHLIGHT_COLORS : HIGHLIGHT_COLORS.filter((c) => !activeHighlights.has(c)); - // X (remove) circles come first, then apply circles in canonical order. + // Remove (checkmark) circles come first, then apply circles in canonical order. const colorCircles = [ - ...activeColors.map((color) => ({ color, showX: true, key: `${color}-clear` })), - ...colorsToApply.map((color) => ({ color, showX: false, key: `${color}-apply` })), + ...activeColors.map((color) => ({ color, showRemove: true, key: `${color}-clear` })), + ...colorsToApply.map((color) => ({ color, showRemove: false, key: `${color}-apply` })), ]; // Snapshot of everything the Content renders. While open we keep it fresh; the @@ -275,24 +284,30 @@ export const VerseActionPopover: FC = ({ )} -
- {view.colorCircles.map(({ color, showX, key }) => ( - (showX ? onClearHighlight(color) : onHighlight(color))} - /> - ))} -
+ {/* Highlights UI is hidden entirely when the feature is off (flag off): + only Copy / Share remain. */} + {highlightsEnabled && ( + <> +
+ {view.colorCircles.map(({ color, showRemove, key }) => ( + (showRemove ? onClearHighlight(color) : onHighlight(color))} + /> + ))} +
- {/* Separator */} -