refactor(frontend): extract shared parts from PaidGate and AdmiralGate (#876)

PaidGate and AdmiralGate were ~95% identical: same state machine
(unlocked / compact-blurred / dismissed-pill / full-upsell-card), same
24h localStorage-backed dismissal logic, differing only in license
predicate, dismiss-storage key, icon, and copy strings. Two reviewers
flagged the duplication after PRs #874 and #875 landed identical
changes in both files; the rule-of-three threshold is met.

Extract the shared parts compositionally rather than as one big config-
driven gate (the latter would just inline both gates' contents behind
8 props of indirection):

- frontend/src/hooks/useDismissalState.ts owns the localStorage
  dismissal pattern. Lives under hooks/ to dodge the
  react-refresh/only-export-components lint rule that would fire if a
  hook coexisted with components in the same file. Validates the stored
  timestamp via Number.isFinite so a hand-edited or stale-extension
  garbage value defaults to "show the upsell" instead of crashing.
- frontend/src/components/tierUpsell.tsx exports CompactBlurredLock,
  DismissedPill, and FullUpsellCard plus a shared TierGateProps
  interface. The compact-mode JSDoc lives on TierGateProps so the doc
  string lives in exactly one place.

PaidGate and AdmiralGate become ~50-line compositions reading like a
state machine. Public API of both gates is byte-stable: all 13+
consumers across the app continue to use <PaidGate featureName="X">
and <AdmiralGate featureName="X" compact> exactly as before.

Two pre-existing security/polish issues fixed in passing while there
is one source of truth for the affected JSX:

- FullUpsellCard's window.open now passes 'noopener,noreferrer' to
  prevent the destination tab from accessing window.opener (reverse
  tabnabbing).
- The Number.parseInt + Number.isFinite guard replaces a bare
  parseInt that would have happily accepted any prefix-numeric input.

Adds a Vitest spec for useDismissalState covering: empty / recent /
expired / non-numeric storage values, dismiss() / restore() side
effects, the 24h boundary on fresh mount, and key independence.
This commit is contained in:
Anso
2026-05-02 04:01:54 -04:00
committed by GitHub
parent b843b89ca4
commit 677f0778e7
5 changed files with 353 additions and 165 deletions
@@ -0,0 +1,92 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { act, renderHook } from '@testing-library/react';
import { useDismissalState } from '../useDismissalState';
const KEY = 'test-dismiss-key';
const DISMISS_DURATION_MS = 24 * 60 * 60 * 1000;
describe('useDismissalState', () => {
beforeEach(() => {
localStorage.clear();
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
});
it('returns dismissed=false when no timestamp is stored', () => {
const { result } = renderHook(() => useDismissalState(KEY));
expect(result.current.dismissed).toBe(false);
});
it('returns dismissed=true when a recent timestamp is stored', () => {
localStorage.setItem(KEY, String(Date.now() - 1000));
const { result } = renderHook(() => useDismissalState(KEY));
expect(result.current.dismissed).toBe(true);
});
it('returns dismissed=false when the stored timestamp is past the 24h window', () => {
localStorage.setItem(KEY, String(Date.now() - DISMISS_DURATION_MS - 1));
const { result } = renderHook(() => useDismissalState(KEY));
expect(result.current.dismissed).toBe(false);
});
it('returns dismissed=false when the stored value is non-numeric garbage', () => {
// Future-proofing: a stale extension or hand-edit could leave a non-
// numeric value at the key; the hook must not crash and must default
// to "show the upsell."
localStorage.setItem(KEY, 'not-a-number');
const { result } = renderHook(() => useDismissalState(KEY));
expect(result.current.dismissed).toBe(false);
});
it('dismiss() flips dismissed to true and writes a parseable timestamp', () => {
const { result } = renderHook(() => useDismissalState(KEY));
expect(result.current.dismissed).toBe(false);
act(() => result.current.dismiss());
expect(result.current.dismissed).toBe(true);
const stored = localStorage.getItem(KEY);
expect(stored).not.toBeNull();
expect(Number.isFinite(Number.parseInt(stored as string, 10))).toBe(true);
});
it('restore() flips dismissed to false and removes the storage entry', () => {
localStorage.setItem(KEY, String(Date.now()));
const { result } = renderHook(() => useDismissalState(KEY));
expect(result.current.dismissed).toBe(true);
act(() => result.current.restore());
expect(result.current.dismissed).toBe(false);
expect(localStorage.getItem(KEY)).toBeNull();
});
it('respects the 24h boundary: a fresh mount past expiry sees dismissed=false', () => {
// Dismiss now, then advance time past the window, then mount fresh.
// The original hook instance keeps its `dismissed: true` state in
// React (lazy initializer runs once), so the boundary is observable
// only on a fresh mount, not via re-render of the same hook.
const { result, unmount } = renderHook(() => useDismissalState(KEY));
act(() => result.current.dismiss());
expect(result.current.dismissed).toBe(true);
unmount();
vi.advanceTimersByTime(DISMISS_DURATION_MS + 1000);
const fresh = renderHook(() => useDismissalState(KEY));
expect(fresh.result.current.dismissed).toBe(false);
});
it('different keys are independent', () => {
const { result: a } = renderHook(() => useDismissalState('key-a'));
const { result: b } = renderHook(() => useDismissalState('key-b'));
act(() => a.current.dismiss());
expect(a.current.dismissed).toBe(true);
expect(b.current.dismissed).toBe(false);
});
});
+40
View File
@@ -0,0 +1,40 @@
import { useState } from 'react';
const DISMISS_DURATION_MS = 24 * 60 * 60 * 1000;
/**
* Tracks the localStorage-backed dismissal flag for an upsell gate.
* `dismiss()` writes the current timestamp and flips state to dismissed;
* `restore()` removes the timestamp so the next render falls through to
* the full upsell card. The dismissal window is 24h; after expiry,
* `localStorage.getItem(key)` still returns a stale timestamp but the
* lazy initializer treats it as expired and returns `dismissed: false`,
* so the gate shows the full upsell again on next mount.
*
* Each hook instance owns its own React state. If two `<PaidGate>`
* mounts share the same key in the same tab, dismissing one does not
* sync to the other until that other re-mounts. Cross-instance pub-sub
* is not provided; the dismissal is a soft UX nicety, not a security
* boundary.
*/
export function useDismissalState(key: string) {
const [dismissed, setDismissed] = useState(() => {
const dismissedAt = localStorage.getItem(key);
if (!dismissedAt) return false;
const ts = Number.parseInt(dismissedAt, 10);
if (!Number.isFinite(ts)) return false;
return Date.now() - ts < DISMISS_DURATION_MS;
});
const dismiss = () => {
localStorage.setItem(key, Date.now().toString());
setDismissed(true);
};
const restore = () => {
localStorage.removeItem(key);
setDismissed(false);
};
return { dismissed, dismiss, restore };
}