diff --git a/frontend/src/components/ShareDialog.test.tsx b/frontend/src/components/ShareDialog.test.tsx index 657e5b13..7c598f1f 100644 --- a/frontend/src/components/ShareDialog.test.tsx +++ b/frontend/src/components/ShareDialog.test.tsx @@ -47,12 +47,22 @@ vi.mock('../lib/publications-client', () => ({ DisplayNameRequiredError: MockDisplayNameRequiredError, })); +const fireSealMock = vi.fn(); +vi.mock('./shared/SealMoment', () => ({ + useSealMoment: () => ({ fire: fireSealMock, moment: null }), +})); + import { ShareDialog } from './ShareDialog'; // ShareDialog renders (the confirmed-public "Your profile" row, the // no-friends "Friends page" link, and the guest sign-in prompt) — all need a // Router context, mirroring this codebase's established MemoryRouter wrap. -function renderDialog(props: { resourceId?: string; resourceLabel: string; onClose: () => void }) { +function renderDialog(props: { + resourceId?: string; + resourceLabel: string; + colorIdentity?: string[]; + onClose: () => void; +}) { return render( @@ -81,6 +91,7 @@ beforeEach(() => { revokedAt: null, }); getPublicationMock.mockResolvedValue(null); + fireSealMock.mockClear(); useAuth.setState({ user: { id: 'u1', username: 'alice', role: 'user' }, status: 'authed', @@ -213,6 +224,59 @@ describe('ShareDialog — going Public', () => { }); }); +describe('ShareDialog — first-publish seal (E150)', () => { + it('fires the seal with the deck colour identity on a genuine first publish', async () => { + publishDeckMock.mockResolvedValue({ + slug: 'seal-deck', + url: 'https://spellcontrol.com/d/seal-deck', + publishedAt: 1, + updatedAt: 1, + unpublishedAt: null, + viewCount: 0, + copyCount: 0, + isFirstPublish: true, + }); + + renderDialog({ + resourceId: 'd-seal-first', + resourceLabel: 'Test Deck', + colorIdentity: ['G', 'U'], + onClose: () => {}, + }); + + fireEvent.click(screen.getByRole('radio', { name: 'Public' })); + fireEvent.click( + await screen.findByRole('button', { name: 'Make it public — anyone can view' }) + ); + + await waitFor(() => expect(publishDeckMock).toHaveBeenCalledWith('d-seal-first')); + await waitFor(() => expect(fireSealMock).toHaveBeenCalledWith(['G', 'U'])); + }); + + it('never fires the seal on a republish (isFirstPublish: false)', async () => { + publishDeckMock.mockResolvedValue({ + slug: 'seal-deck-2', + url: 'https://spellcontrol.com/d/seal-deck-2', + publishedAt: 1, + updatedAt: 1, + unpublishedAt: null, + viewCount: 3, + copyCount: 1, + isFirstPublish: false, + }); + + renderDialog({ resourceId: 'd-seal-republish', resourceLabel: 'Test Deck', onClose: () => {} }); + + fireEvent.click(screen.getByRole('radio', { name: 'Public' })); + fireEvent.click( + await screen.findByRole('button', { name: 'Make it public — anyone can view' }) + ); + + await waitFor(() => expect(publishDeckMock).toHaveBeenCalledWith('d-seal-republish')); + expect(fireSealMock).not.toHaveBeenCalled(); + }); +}); + describe('ShareDialog — reopening an already-published deck', () => { it('pre-selects Public with no extraneous createShare call', async () => { getPublicationMock.mockResolvedValue({ diff --git a/frontend/src/components/ShareDialog.tsx b/frontend/src/components/ShareDialog.tsx index 94a95e06..a872b531 100644 --- a/frontend/src/components/ShareDialog.tsx +++ b/frontend/src/components/ShareDialog.tsx @@ -2,6 +2,7 @@ import { useEffect, useId, useRef, useState } from 'react'; import { Link } from 'react-router-dom'; import { Modal } from './Modal'; import { ShareQrCode } from './shared/ShareQrCode'; +import { useSealMoment } from './shared/SealMoment'; import { createShare, listShares, revokeShare, shareUrl } from '../lib/share-client'; import { DisplayNameRequiredError, @@ -11,6 +12,7 @@ import { unpublishDeck, type Publication, } from '../lib/publications-client'; +import { shouldCelebrateFirstPublish } from '../lib/first-publish-celebration'; import { updateProfile } from '../lib/auth-api'; import { listFriends, type Friend } from '../lib/friends-client'; import { isNativePlatform } from '../lib/platform'; @@ -34,6 +36,10 @@ interface Props { resourceId?: string; /** Display title for what's being shared, e.g. "Edric Combo" or "your collection". */ resourceLabel: string; + /** Deck-only: the commander(s)' color identity, for the first-publish seal + * moment's motes (E150). Omit for identity-less / non-deck shares — the + * seal falls back to gold, per STYLE_GUIDE's "colours are honest" ruling. */ + colorIdentity?: string[]; onClose: () => void; } @@ -50,7 +56,7 @@ interface Props { * PLAN.md §A1). The two systems compose: a link, a friends, a direct-to- * Alice share, and a public listing of one resource can all coexist. */ -export function ShareDialog({ kind, resourceId, resourceLabel, onClose }: Props) { +export function ShareDialog({ kind, resourceId, resourceLabel, colorIdentity, onClose }: Props) { // Share links are per-account (they stay tied to the owner and are // revocable), so a guest can't mint one — prompt them to sign in instead. const isGuest = useAuth((s) => s.status === 'guest'); @@ -95,6 +101,12 @@ export function ShareDialog({ kind, resourceId, resourceLabel, onClose }: Props) const previousLadderRef = useRef('link'); const confirmBlockRef = useRef(null); const displayNameId = useId(); + // First-publish seal (E150): this dialog never navigates away on publish + // (it stays open showing the live link), so — unlike the creation-time + // fieldsets — it's safe to fire directly here rather than handing off to a + // landing page. Covers both entry surfaces that reuse this dialog as-is: + // the deck-editor visibility chip and the post-create DeckPublishNudge. + const { fire: fireSealMoment, moment: sealMoment } = useSealMoment(); // A direct share can't mint until a recipient is chosen — hold off until then. const awaitingRecipient = audience === 'direct' && !addresseeId; @@ -240,6 +252,9 @@ export function ShareDialog({ kind, resourceId, resourceLabel, onClose }: Props) setPublication(pub); setPendingPublicConfirm(false); setNeedsDisplayName(false); + if (shouldCelebrateFirstPublish(resourceId, pub.isFirstPublish)) { + fireSealMoment(colorIdentity); + } } catch (err) { // Defense in depth: even if the client's cached displayName looked // set, a display_name_required 400 re-shows the same sub-step rather @@ -431,6 +446,7 @@ export function ShareDialog({ kind, resourceId, resourceLabel, onClose }: Props) dismissable={!working} className="choice-dialog share-dialog" > + {sealMoment}

Share {resourceLabel}

diff --git a/frontend/src/components/deck/DeckPublishNudge.css b/frontend/src/components/deck/DeckPublishNudge.css new file mode 100644 index 00000000..e32434ad --- /dev/null +++ b/frontend/src/components/deck/DeckPublishNudge.css @@ -0,0 +1,73 @@ +/* DeckPublishNudge.css — post-create visibility nudge (E150). + Layout mirrors NavMigrationTip.css (in-flow row, not sticky — this is + page-content, not global chrome); the action + dismiss cluster mirrors + .auto-link-banner-actions (modals-dialogs.css). Same --info tint as + NavMigrationTip: informational, not a warning or an identity-confirmation. */ + +.deck-publish-nudge { + display: flex; + align-items: center; + gap: var(--space-4); + flex-wrap: wrap; + padding: var(--space-3) var(--space-4); + margin-bottom: var(--space-4); + background: color-mix(in srgb, var(--info) 12%, var(--surface-raised)); + border: 1px solid var(--info); + border-radius: var(--radius-lg); + font-size: var(--text-sm); + color: var(--text-primary); +} + +.deck-publish-nudge p { + flex: 1 1 220px; + margin: 0; + line-height: 1.45; +} + +.deck-publish-nudge-actions { + display: flex; + align-items: center; + gap: var(--space-2); + flex-shrink: 0; +} + +.deck-publish-nudge-dismiss { + position: relative; + flex-shrink: 0; + display: inline-flex; + align-items: center; + justify-content: center; + width: 1.75rem; + height: 1.75rem; + border: 0; + border-radius: 50%; + background: transparent; + color: var(--text-secondary); + cursor: pointer; +} + +@media (hover: hover) and (pointer: fine) { + .deck-publish-nudge-dismiss:hover { + background: var(--surface); + color: var(--text-primary); + } +} + +.deck-publish-nudge-dismiss:focus-visible { + outline: 2px solid var(--accent); + outline-offset: 2px; +} + +/* 44px touch target without inflating the visible glyph — mirrors + .nav-migration-tip-dismiss / .set-filter-chip-x. */ +@media (pointer: coarse) { + .deck-publish-nudge-dismiss::after { + content: ''; + position: absolute; + top: 50%; + left: 50%; + width: 44px; + height: 44px; + transform: translate(-50%, -50%); + } +} diff --git a/frontend/src/components/deck/DeckPublishNudge.test.tsx b/frontend/src/components/deck/DeckPublishNudge.test.tsx new file mode 100644 index 00000000..8d70b455 --- /dev/null +++ b/frontend/src/components/deck/DeckPublishNudge.test.tsx @@ -0,0 +1,87 @@ +// @vitest-environment happy-dom +import { fireEvent, render, screen } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useAuth } from '../../store/auth'; + +vi.mock('../../lib/platform', () => ({ isNativePlatform: vi.fn(() => false) })); +vi.mock('@capacitor/share', () => ({ Share: { share: vi.fn() } })); +vi.mock('../../lib/publications-client', () => ({ + getPublication: () => Promise.resolve(null), + publishDeck: () => Promise.reject(new Error('not used in this suite')), + unpublishDeck: () => Promise.resolve(), + publicationUrl: (slug: string) => `https://spellcontrol.com/d/${slug}`, + DisplayNameRequiredError: class extends Error {}, +})); +vi.mock('../../lib/share-client', () => ({ + createShare: () => Promise.resolve({ token: 'tok', audience: 'link' }), + listShares: () => Promise.resolve([]), + revokeShare: () => Promise.resolve(), + shareUrl: (token: string) => `https://spellcontrol.com/s/${token}`, +})); + +import { DeckPublishNudge } from './DeckPublishNudge'; + +function renderNudge() { + return render( + + + + ); +} + +beforeEach(() => { + useAuth.setState({ + user: { id: 'u1', username: 'alice', role: 'user' }, + status: 'authed', + error: null, + autoLinkedAt: null, + profile: { + displayName: 'Alice', + bio: null, + avatarCardId: null, + avatarCardName: null, + avatarImageUrl: null, + }, + }); +}); + +describe('DeckPublishNudge', () => { + it('renders the nudge with a Share action for an authed user', () => { + renderNudge(); + expect(screen.getByRole('status').textContent).toContain('Only you can see this deck'); + expect(screen.getByRole('button', { name: 'Share…' })).toBeTruthy(); + }); + + it('renders nothing for a guest — ShareDialog would just hit a sign-in wall', () => { + useAuth.setState({ + user: null, + status: 'guest', + error: null, + autoLinkedAt: null, + profile: null, + }); + const { container } = renderNudge(); + expect(container.firstChild).toBeNull(); + }); + + it('dismiss hides it immediately, for the rest of this render', () => { + const { container } = renderNudge(); + fireEvent.click(screen.getByRole('button', { name: 'Dismiss' })); + expect(container.firstChild).toBeNull(); + }); + + it('opens ShareDialog on Share…, and closing it (for any reason) retires the nudge', async () => { + const { container } = renderNudge(); + fireEvent.click(screen.getByRole('button', { name: 'Share…' })); + + const dialogTitle = await screen.findByText('Share Test Deck'); + expect(dialogTitle).toBeTruthy(); + + fireEvent.click(screen.getByRole('radio', { name: 'Private' })); + // Private on an already-private deck is a no-op that still lets Done close it. + fireEvent.click(await screen.findByRole('button', { name: 'Done' })); + + expect(container.querySelector('.deck-publish-nudge')).toBeNull(); + }); +}); diff --git a/frontend/src/components/deck/DeckPublishNudge.tsx b/frontend/src/components/deck/DeckPublishNudge.tsx new file mode 100644 index 00000000..2ed2190b --- /dev/null +++ b/frontend/src/components/deck/DeckPublishNudge.tsx @@ -0,0 +1,71 @@ +import { useState } from 'react'; +import { X } from 'lucide-react'; +import { ShareDialog } from '../ShareDialog'; +import { useAuth } from '../../store/auth'; +import './DeckPublishNudge.css'; + +interface Props { + deckId: string; + deckName: string; + colorIdentity?: string[]; +} + +/** + * One-shot, dismissible post-create nudge toward the persistent + * DeckVisibilityChip above it (E150) — for the two entry surfaces that skip + * the creation-time visibility fieldset entirely (CopyDeckButton's one-tap + * copy; a multi-file import that lands on a single deck). The caller + * mounts this only when it's confirmed the deck was JUST created by one of + * those flows (a router-state one-shot flag, mirroring `justGenerated`) — + * decks created via /decks/new or the single-deck import path already + * surfaced the same choice inline (see `usePublishOnCreate`), so nudging + * again here would be redundant noise, not discoverability. + * + * "One-row, dismissible, non-displacing" per STYLE_GUIDE's insight-surface + * ruling — shape mirrors NavMigrationTip (in-flow, not sticky) crossed with + * AutoLinkBanner's action-button row. Local `dismissed` state only: this is + * scoped to a single page visit (the one-shot flag never survives a fresh + * navigation), so there's nothing to persist across sessions. + */ +export function DeckPublishNudge({ deckId, deckName, colorIdentity }: Props) { + const isGuest = useAuth((s) => s.status === 'guest'); + const [dismissed, setDismissed] = useState(false); + const [shareOpen, setShareOpen] = useState(false); + + // A guest can't publish — ShareDialog would just show its sign-in prompt, + // which isn't what "share it when you're ready" promises. + if (isGuest || dismissed) return null; + + return ( +
+

Only you can see this deck. Share it when you're ready.

+
+ + +
+ {shareOpen && ( + { + setShareOpen(false); + // Engaging with Share at all — publish or not — is enough + // intent to retire the nudge for the rest of this visit. + setDismissed(true); + }} + /> + )} +
+ ); +} diff --git a/frontend/src/components/deck/DeckVisibilityChip.tsx b/frontend/src/components/deck/DeckVisibilityChip.tsx index a34e6547..9c5cc55c 100644 --- a/frontend/src/components/deck/DeckVisibilityChip.tsx +++ b/frontend/src/components/deck/DeckVisibilityChip.tsx @@ -7,6 +7,8 @@ import './DeckVisibilityChip.css'; interface Props { deckId: string; deckName: string; + /** Forwarded to ShareDialog for the first-publish seal's motes (E150). */ + colorIdentity?: string[]; } const VISIBILITY_META: Record = { @@ -27,7 +29,7 @@ const VISIBILITY_META: Record { setOpen(false); refetch(); diff --git a/frontend/src/components/deck/ImportDeckDialog.tsx b/frontend/src/components/deck/ImportDeckDialog.tsx index 4452b376..3a2433ba 100644 --- a/frontend/src/components/deck/ImportDeckDialog.tsx +++ b/frontend/src/components/deck/ImportDeckDialog.tsx @@ -16,6 +16,7 @@ import { isValidCommander, isPdhCommanderEligible } from '../../lib/commanders'; import { areValidPartners, canHavePartner } from '@/deck-builder/lib/partnerUtils'; import { isNativePlatform } from '../../lib/platform'; import { pickNativeFiles } from '../../lib/native-file-picker'; +import { usePublishOnCreate, type PublishOutcome } from '../../lib/use-publish-on-create'; const DECK_IMPORT_MIME = ['text/csv', 'text/tab-separated-values', 'text/plain']; import { @@ -180,6 +181,35 @@ export function ImportDeckDialog({ onClose, format: initialFormat = 'commander' const decks = useDecksStore((s) => s.decks); const buildDeckFromResult = useBuildDeckFromImport(); + // ── Visibility (creation-time choice, E150) ───────────────────────────── + // Single-deck paths only (paste/merge — see the fieldset's render guard + // below); a multi-file batch that lands on ONE deck instead gets the + // lighter DeckPublishNudge on arrival (promptVisibility, set in + // commitBatch) rather than this fieldset — see the PR description for why. + const onPublishSettled = useCallback( + (id: string, outcome?: PublishOutcome) => { + onClose(); + navigate( + `/decks/${id}`, + outcome ? { state: { justPublished: outcome.isFirstPublish } } : undefined + ); + }, + [onClose, navigate] + ); + const { + canPublish, + publicDisabledReason, + visibility, + setVisibility, + publishing, + needsDisplayName, + displayNameDraft, + setDisplayNameDraft, + publishAfterCreate, + saveDisplayNameAndPublish, + cancelDisplayName, + } = usePublishOnCreate(onPublishSettled); + const [selectedFormat, setSelectedFormat] = useState(initialFormat); const formatConfig = DECK_FORMAT_CONFIGS[selectedFormat]; const [step, setStep] = useState('input'); @@ -249,10 +279,22 @@ export function ImportDeckDialog({ onClose, format: initialFormat = 'commander' partner: ScryfallCard | null = null ) => { const id = buildDeckFromResult(result, commander, name, selectedFormat, { partner }); + if (visibility === 'public' && canPublish) { + void publishAfterCreate(id); + return; + } onClose(); navigate(`/decks/${id}`); }, - [buildDeckFromResult, navigate, onClose, selectedFormat] + [ + buildDeckFromResult, + navigate, + onClose, + selectedFormat, + visibility, + canPublish, + publishAfterCreate, + ] ); const processSingleResult = useCallback( @@ -510,7 +552,16 @@ export function ImportDeckDialog({ onClose, format: initialFormat = 'commander' ); } onClose(); - navigate(ids.length === 1 ? `/decks/${ids[0]}` : '/decks'); + // A batch that lands on exactly one deck (a single staged file, or every + // other draft skipped/removed) gets the lighter post-create nudge on + // arrival — this multi-draft review screen never showed the visibility + // fieldset, so there's no explicit choice to honor either way. A batch + // that lands on several decks has no single editor to nudge on, so it + // stays as-is (E150 — see the PR description for the full rationale). + navigate( + ids.length === 1 ? `/decks/${ids[0]}` : '/decks', + ids.length === 1 ? { state: { promptVisibility: true } } : undefined + ); }, [okDrafts, decks, buildDeckFromResult, navigate, onClose]); // --- File input / drag-drop -------------------------------------------- @@ -590,12 +641,61 @@ export function ImportDeckDialog({ onClose, format: initialFormat = 'commander' const title = step === 'review' ? 'Review import' : step === 'batch' ? 'Review decks' : 'Import deck'; + // The deck already exists at this point (only reachable after a create + // succeeded but the publish itself needs a display name) — replace the + // whole modal rather than layering this over now-stale step content, + // mirroring ShareDialog's own display_name_required fallback. + if (needsDisplayName) { + return ( + +
+

Set a display name

+
+
+

+ Publishing shows your display name on the deck page — set one to continue. +

+
+ + setDisplayNameDraft(e.target.value)} + /> +
+
+
+ + +
+ + ); + } + return (

{title}

@@ -765,6 +865,46 @@ export function ImportDeckDialog({ onClose, format: initialFormat = 'commander' /> )} + {/* Only the paths that create exactly one deck (paste, or a + staged batch merged into one) get the creation-time choice — + mirrors /decks/new's single fieldset. "Separate decks" (N + results) has no single editor to land the choice on; those + decks stay private, publishable afterward per-deck like + today, and a single-result landing gets the lighter + DeckPublishNudge instead (see commitBatch). */} + {(batchFiles.length === 0 || batchMode === 'merge') && ( +
+
Visibility
+
+ + +
+

+ {visibility === 'public' + ? 'Anyone can find it at a stable link and on your profile.' + : 'Only you can see this deck.'} + {!canPublish && ` ${publicDisabledReason}`} +

+
+ )} Continue ({batchFiles.length} file{batchFiles.length === 1 ? '' : 's'}) @@ -1138,7 +1278,7 @@ export function ImportDeckDialog({ onClose, format: initialFormat = 'commander' type="button" className="btn btn-primary" onClick={handlePasteImport} - disabled={isLoading || !pasteText.trim()} + disabled={isLoading || !pasteText.trim() || publishing} > Import @@ -1179,7 +1319,7 @@ export function ImportDeckDialog({ onClose, format: initialFormat = 'commander' type="button" className="btn btn-primary" onClick={handleConfirmReview} - disabled={!canConfirmReview} + disabled={!canConfirmReview || publishing} > Create deck diff --git a/frontend/src/components/deck/ImportDeckDialog.visibility.test.tsx b/frontend/src/components/deck/ImportDeckDialog.visibility.test.tsx new file mode 100644 index 00000000..0ebaa1a4 --- /dev/null +++ b/frontend/src/components/deck/ImportDeckDialog.visibility.test.tsx @@ -0,0 +1,225 @@ +// @vitest-environment happy-dom +/** + * Tests for the creation-time visibility fieldset on ImportDeckDialog's + * single-deck (paste) path — the second consumer of usePublishOnCreate, + * the same shared choke point DeckNewPage.test.tsx exercises. Uses the + * 'standard' format (hasCommander: false) so a clean, empty import takes + * the direct finalize branch with no commander-picker/review-step detour, + * mirroring DeckNewPage.test.tsx's own format choice for the same reason. + */ +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { useAuth } from '../../store/auth'; +import { DisplayNameRequiredError, type PublishResult } from '../../lib/publications-client'; +import type { DeckImportResponse } from '../../types'; + +const navigateMock = vi.fn(); +vi.mock('react-router-dom', async (importOriginal) => { + const real = await importOriginal(); + return { ...real, useNavigate: () => navigateMock }; +}); + +vi.mock('../../store/decks', () => ({ + useDecksStore: (sel: (s: { decks: unknown[] }) => unknown) => sel({ decks: [] }), +})); + +const buildDeckFromResultMock = vi.fn(() => 'new-deck-id'); +vi.mock('../../lib/build-deck-from-import', () => ({ + useBuildDeckFromImport: () => buildDeckFromResultMock, +})); + +const importDeckTextMock = vi.fn<() => Promise>(); +vi.mock('../../lib/api', () => ({ + importDeckText: () => importDeckTextMock(), + importDeckFile: vi.fn(), +})); + +vi.mock('../../lib/sync', () => ({ + isOnline: () => true, + onSyncedChange: () => () => {}, +})); + +const updateProfileMock = vi.fn(); +vi.mock('../../lib/auth-api', () => ({ + updateProfile: (patch: { displayName: string }) => updateProfileMock(patch), +})); + +const publishDeckMock = vi.fn<() => Promise>(); +vi.mock('../../lib/publications-client', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + publishDeck: () => publishDeckMock(), + publicationUrl: (slug: string) => `https://spellcontrol.com/d/${slug}`, + }; +}); + +// Never rendered by the 'standard'-format paths under test (no commander +// step), but still imported transitively — stub it out rather than pull in +// its live useCollectionStore (IndexedDB) dependency, mirroring +// DeckNewPage.test.tsx's identical stub for the same reason. +vi.mock('./CommanderSearch', () => ({ CommanderSearch: () => null })); + +import { ImportDeckDialog } from './ImportDeckDialog'; + +const CLEAN_RESULT: DeckImportResponse = { + commander: null, + companion: null, + cards: [], + unresolvedNames: [], + fetchErrors: [], + detectedFormat: '', + cardCount: 0, +}; + +const PUB: PublishResult = { + slug: 'my-deck', + url: 'https://spellcontrol.com/d/my-deck', + publishedAt: 1, + updatedAt: 1, + unpublishedAt: null, + viewCount: 0, + copyCount: 0, + isFirstPublish: true, +}; + +function renderDialog() { + const onClose = vi.fn(); + const utils = render( + + + + ); + return { onClose, ...utils }; +} + +/** Standard has no commander step, so a clean paste import finalizes + * immediately — mirrors DeckNewPage.test.tsx's selectStandardFormat(). */ +function selectStandardFormat() { + fireEvent.click(screen.getByRole('radio', { name: 'Standard' })); +} + +function pasteAndImport() { + fireEvent.change(screen.getByPlaceholderText(/Lightning Strike/), { + target: { value: '4 Lightning Bolt' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Import' })); +} + +beforeEach(() => { + useAuth.setState({ + user: { id: 'u1', username: 'alice', role: 'user' }, + status: 'authed', + error: null, + autoLinkedAt: null, + profile: { + displayName: 'Alice', + bio: null, + avatarCardId: null, + avatarCardName: null, + avatarImageUrl: null, + }, + }); + navigateMock.mockClear(); + buildDeckFromResultMock.mockClear().mockReturnValue('new-deck-id'); + importDeckTextMock.mockReset().mockResolvedValue(CLEAN_RESULT); + updateProfileMock.mockReset(); + publishDeckMock.mockReset().mockResolvedValue(PUB); +}); +afterEach(() => localStorage.clear()); + +describe('ImportDeckDialog — creation-time visibility', () => { + it('shows the Visibility fieldset on the single-deck (no staged files) path, defaulting to Private', () => { + renderDialog(); + selectStandardFormat(); + expect(screen.getByRole('radio', { name: 'Private' }).getAttribute('aria-checked')).toBe( + 'true' + ); + expect((screen.getByRole('radio', { name: 'Public' }) as HTMLButtonElement).disabled).toBe( + false + ); + }); + + it('never calls publishDeck when Private (the default) is kept, and closes + navigates with no router state', async () => { + const { onClose } = renderDialog(); + selectStandardFormat(); + pasteAndImport(); + + await waitFor(() => expect(buildDeckFromResultMock).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id')); + expect(onClose).toHaveBeenCalledTimes(1); + expect(publishDeckMock).not.toHaveBeenCalled(); + }); + + it('publishes after creating when Public is selected, closes, and navigates with justPublished: true', async () => { + const { onClose } = renderDialog(); + selectStandardFormat(); + fireEvent.click(screen.getByRole('radio', { name: 'Public' })); + pasteAndImport(); + + await waitFor(() => expect(publishDeckMock).toHaveBeenCalledTimes(1)); + await waitFor(() => + expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id', { + state: { justPublished: true }, + }) + ); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('on display_name_required, swaps to the inline substep instead of navigating, then completes on save', async () => { + publishDeckMock.mockRejectedValueOnce(new DisplayNameRequiredError()); + updateProfileMock.mockResolvedValue({ + displayName: 'Bob', + bio: null, + avatarCardId: null, + avatarCardName: null, + avatarImageUrl: null, + }); + const { onClose } = renderDialog(); + selectStandardFormat(); + fireEvent.click(screen.getByRole('radio', { name: 'Public' })); + pasteAndImport(); + + await screen.findByText('Set a display name'); + expect(navigateMock).not.toHaveBeenCalled(); + expect(onClose).not.toHaveBeenCalled(); + + fireEvent.change(screen.getByLabelText('Display name'), { target: { value: 'Bob' } }); + fireEvent.click(screen.getByRole('button', { name: 'Save & continue' })); + + await waitFor(() => expect(updateProfileMock).toHaveBeenCalledWith({ displayName: 'Bob' })); + await waitFor(() => expect(publishDeckMock).toHaveBeenCalledTimes(2)); + await waitFor(() => + expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id', { + state: { justPublished: true }, + }) + ); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('disables the Public radio for a guest, with a sign-in hint, and never blocks creation', () => { + useAuth.setState({ + user: null, + status: 'guest', + error: null, + autoLinkedAt: null, + profile: null, + }); + renderDialog(); + selectStandardFormat(); + + const publicRadio = screen.getByRole('radio', { name: 'Public' }) as HTMLButtonElement; + expect(publicRadio.disabled).toBe(true); + expect(screen.getByText(/Sign in to publish/)).toBeTruthy(); + + // The Import button itself must still be enabled for a guest — only + // gated on having text to import, never on publish eligibility. + fireEvent.change(screen.getByPlaceholderText(/Lightning Strike/), { + target: { value: '4 Lightning Bolt' }, + }); + expect((screen.getByRole('button', { name: 'Import' }) as HTMLButtonElement).disabled).toBe( + false + ); + }); +}); diff --git a/frontend/src/components/shared/CopyDeckButton.test.tsx b/frontend/src/components/shared/CopyDeckButton.test.tsx index 2d87f6a5..291567df 100644 --- a/frontend/src/components/shared/CopyDeckButton.test.tsx +++ b/frontend/src/components/shared/CopyDeckButton.test.tsx @@ -59,7 +59,9 @@ describe('CopyDeckButton', () => { expect(recordDeckCopyMock).toHaveBeenCalledTimes(1); expect(recordDeckCopyMock).toHaveBeenCalledWith('korvold-treasure'); expect(copySharedDeckMock).toHaveBeenCalledWith(deck(), 'korvold-treasure'); - expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id'); + expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id', { + state: { promptVisibility: true }, + }); }); it('does not fire recordDeckCopy at all when no slug is present (copying from /s/:token)', () => { @@ -67,7 +69,9 @@ describe('CopyDeckButton', () => { fireEvent.click(screen.getByRole('button')); expect(recordDeckCopyMock).not.toHaveBeenCalled(); expect(copySharedDeckMock).toHaveBeenCalledWith(deck(), undefined); - expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id'); + expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id', { + state: { promptVisibility: true }, + }); }); it('still copies, toasts, and navigates even when recordDeckCopy rejects', async () => { @@ -75,7 +79,9 @@ describe('CopyDeckButton', () => { renderButton('korvold-treasure'); expect(() => fireEvent.click(screen.getByRole('button'))).not.toThrow(); expect(copySharedDeckMock).toHaveBeenCalledTimes(1); - expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id'); + expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id', { + state: { promptVisibility: true }, + }); // Observe the rejection after assertions so it doesn't leak into another // test as an unhandled rejection — the button itself never awaits it. await recordDeckCopyMock.mock.results[0]!.value.catch(() => {}); diff --git a/frontend/src/components/shared/CopyDeckButton.tsx b/frontend/src/components/shared/CopyDeckButton.tsx index d23de354..34ead556 100644 --- a/frontend/src/components/shared/CopyDeckButton.tsx +++ b/frontend/src/components/shared/CopyDeckButton.tsx @@ -29,7 +29,11 @@ export function CopyDeckButton({ data, variant = 'bar', slug }: Props) { // actual copy above (recordDeckCopy already swallows its own errors). if (slug) void recordDeckCopy(slug); toast.show({ message: 'Copied to your decks', tone: 'success' }); - void navigate(`/decks/${id}`); + // promptVisibility (E150): a one-tap copy skips the creation-time + // visibility fieldset entirely, so the editor shows a post-create + // DeckPublishNudge instead — mirrors `justGenerated`'s one-shot + // router-state pattern. + void navigate(`/decks/${id}`, { state: { promptVisibility: true } }); } if (variant === 'block') { diff --git a/frontend/src/lib/first-publish-celebration.test.ts b/frontend/src/lib/first-publish-celebration.test.ts new file mode 100644 index 00000000..a9258753 --- /dev/null +++ b/frontend/src/lib/first-publish-celebration.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; +import { shouldCelebrateFirstPublish } from './first-publish-celebration'; + +// Every case uses its own deckId — the guard is a module-level Set shared +// across the whole test run, so reusing an id would leak state between cases. + +describe('shouldCelebrateFirstPublish', () => { + it('celebrates a genuine first publish', () => { + expect(shouldCelebrateFirstPublish('deck-a', true)).toBe(true); + }); + + it('never celebrates a republish (isFirstPublish: false), even for a fresh deckId', () => { + expect(shouldCelebrateFirstPublish('deck-b', false)).toBe(false); + }); + + it('does not celebrate the same deck twice, even across two genuine-first-publish reports', () => { + expect(shouldCelebrateFirstPublish('deck-c', true)).toBe(true); + expect(shouldCelebrateFirstPublish('deck-c', true)).toBe(false); + }); + + it('a later republish of an already-celebrated deck still reads false', () => { + expect(shouldCelebrateFirstPublish('deck-d', true)).toBe(true); + expect(shouldCelebrateFirstPublish('deck-d', false)).toBe(false); + }); + + it('celebrates each distinct deck independently', () => { + expect(shouldCelebrateFirstPublish('deck-e', true)).toBe(true); + expect(shouldCelebrateFirstPublish('deck-f', true)).toBe(true); + }); +}); diff --git a/frontend/src/lib/first-publish-celebration.ts b/frontend/src/lib/first-publish-celebration.ts new file mode 100644 index 00000000..4c2ecbbd --- /dev/null +++ b/frontend/src/lib/first-publish-celebration.ts @@ -0,0 +1,32 @@ +/** + * The single dedup choke point for the first-publish seal moment (E150), + * shared by every entry surface that can turn a deck public for the first + * time — the deck-editor visibility chip and post-create nudge (both via + * ShareDialog's `doPublish`), and the creation-time fieldset on /decks/new + * and the single-deck import flow (via `usePublishOnCreate`, which can't + * fire directly — see its own doc comment — and instead hands the outcome + * to DeckEditorPage's `justPublished` landing effect). All four routes + * funnel through this one function so "once per deck per app-open" holds + * regardless of entry surface, mirroring the canonical module-level-Set + * pattern (`celebratedDeckComplete` in DeckDisplay.tsx, + * `celebratedBinderCleared` in BinderDriftBanner.tsx) but centralized here + * instead of forked per file, since this guard has more than one call site. + */ +const celebratedFirstPublish = new Set(); + +/** + * True exactly once per deckId: the first time a publish response reports a + * genuine first-ever publish (`isFirstPublish`, derived from the server's + * 201-vs-200 — see `publishDeck` in publications-client.ts). A refresh-while- + * live or a republish after unpublish reports `isFirstPublish: false` and + * never celebrates, satisfying "republish is not a first publish" even + * without consulting the Set. The Set itself guards the rarer case of this + * being called twice for the same genuine first publish (defensive, matches + * the established pattern) and persists for the life of the module — i.e. + * once per app-open, same lifetime as the two per-file precedents above. + */ +export function shouldCelebrateFirstPublish(deckId: string, isFirstPublish: boolean): boolean { + if (!isFirstPublish || celebratedFirstPublish.has(deckId)) return false; + celebratedFirstPublish.add(deckId); + return true; +} diff --git a/frontend/src/lib/publications-client.test.ts b/frontend/src/lib/publications-client.test.ts index 1d259483..012cdebc 100644 --- a/frontend/src/lib/publications-client.test.ts +++ b/frontend/src/lib/publications-client.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { + DeckNotSyncedYetError, DisplayNameRequiredError, getPublication, listMyPublications, @@ -92,18 +93,26 @@ describe('listMyPublications', () => { }); describe('publishDeck', () => { - it('POSTs and returns the publication', async () => { + it('POSTs and returns the publication, flagged isFirstPublish on a 201 (genuine insert)', async () => { const fetchSpy = vi .spyOn(globalThis, 'fetch') .mockResolvedValue(jsonResponse({ publication: PUB }, { status: 201 })); const out = await publishDeck('d1'); - expect(out).toEqual(PUB); + expect(out).toEqual({ ...PUB, isFirstPublish: true }); expect(fetchSpy).toHaveBeenCalledWith( '/api/publications/decks/d1', expect.objectContaining({ method: 'POST', credentials: 'include' }) ); }); + it('flags isFirstPublish false on a 200 (refresh-while-live or republish, existing row updated)', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue( + jsonResponse({ publication: PUB }, { status: 200 }) + ); + const out = await publishDeck('d1'); + expect(out).toEqual({ ...PUB, isFirstPublish: false }); + }); + it('throws DisplayNameRequiredError specifically on a display_name_required 400', async () => { vi.spyOn(globalThis, 'fetch').mockResolvedValue( jsonResponse( @@ -114,6 +123,22 @@ describe('publishDeck', () => { await expect(publishDeck('d1')).rejects.toBeInstanceOf(DisplayNameRequiredError); }); + it('throws DeckNotSyncedYetError specifically on a "Deck not found." 404 (fresh deck racing its own sync)', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue( + jsonResponse({ error: 'Deck not found.' }, { status: 404 }) + ); + await expect(publishDeck('d1')).rejects.toBeInstanceOf(DeckNotSyncedYetError); + }); + + it('throws a plain Error (not DeckNotSyncedYetError) for an unrelated 404', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue( + jsonResponse({ error: 'This deck needs a name before it can be published.' }, { status: 404 }) + ); + const err = await publishDeck('d1').catch((e: unknown) => e); + expect(err).not.toBeInstanceOf(DeckNotSyncedYetError); + expect(err).toBeInstanceOf(Error); + }); + it('throws a plain Error (not DisplayNameRequiredError) for any other failure', async () => { vi.spyOn(globalThis, 'fetch').mockResolvedValue( jsonResponse({ error: 'This deck needs a name before it can be published.' }, { status: 400 }) diff --git a/frontend/src/lib/publications-client.ts b/frontend/src/lib/publications-client.ts index 3e46028e..2ef159ab 100644 --- a/frontend/src/lib/publications-client.ts +++ b/frontend/src/lib/publications-client.ts @@ -37,6 +37,20 @@ export class DisplayNameRequiredError extends Error { } } +/** Thrown by publishDeck() when the server 404s with {error:'Deck not + * found.'} for a deckId the caller just created. The local persist that + * writes a new deck to the server is fire-and-forget from the store's + * perspective (store/decks.ts's sync subscriber) — a publish attempt fired + * immediately after `createDeck()` can race ahead of it. Never surfaced to + * the user directly; `usePublishOnCreate`'s one bounded retry covers the + * gap instead. */ +export class DeckNotSyncedYetError extends Error { + constructor() { + super('Deck not found.'); + this.name = 'DeckNotSyncedYetError'; + } +} + /** * One row of the caller's own publications list (`GET /api/publications/decks`) * — deliberately thinner than `Publication` (no `url`/`publishedAt`): the @@ -77,8 +91,21 @@ export async function getPublication(deckId: string): Promise { +export async function publishDeck(deckId: string): Promise { const res = await fetch(apiUrl(`/api/publications/decks/${encodeURIComponent(deckId)}`), { method: 'POST', credentials: 'include', @@ -86,10 +113,11 @@ export async function publishDeck(deckId: string): Promise { if (!res.ok) { const message = await readError(res, 'Failed to publish deck.'); if (message === 'display_name_required') throw new DisplayNameRequiredError(); + if (res.status === 404 && message === 'Deck not found.') throw new DeckNotSyncedYetError(); throw new Error(message); } const body = (await res.json()) as { publication: Publication }; - return body.publication; + return { ...body.publication, isFirstPublish: res.status === 201 }; } /** Unpublish. Silently no-ops if already unpublished (mirrors revokeShare). */ diff --git a/frontend/src/lib/use-publish-on-create.test.ts b/frontend/src/lib/use-publish-on-create.test.ts new file mode 100644 index 00000000..435c2a70 --- /dev/null +++ b/frontend/src/lib/use-publish-on-create.test.ts @@ -0,0 +1,267 @@ +// @vitest-environment happy-dom +/** + * Hook-level coverage for the creation-time publish choke point (E150): + * every branch DeckNewPage's fieldset and ImportDeckDialog's fieldset both + * route through, so it's tested once here instead of forked per surface. + */ +import { act, renderHook } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { useAuth } from '../store/auth'; +import { useToastsStore } from '../store/toasts'; +import type { PublishResult } from './publications-client'; + +let online = true; +vi.mock('./sync', () => ({ + isOnline: () => online, + onSyncedChange: () => () => {}, +})); + +const publishDeckMock = vi.fn<() => Promise>(); +vi.mock('./publications-client', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + publishDeck: () => publishDeckMock(), + publicationUrl: (slug: string) => `https://spellcontrol.com/d/${slug}`, + }; +}); + +const updateProfileMock = vi.fn(); +vi.mock('./auth-api', () => ({ + updateProfile: (patch: { displayName: string }) => updateProfileMock(patch), +})); + +import { DeckNotSyncedYetError, DisplayNameRequiredError } from './publications-client'; +import { usePublishOnCreate } from './use-publish-on-create'; + +const PUB_FIRST: PublishResult = { + slug: 'my-deck', + url: 'https://spellcontrol.com/d/my-deck', + publishedAt: 1, + updatedAt: 1, + unpublishedAt: null, + viewCount: 0, + copyCount: 0, + isFirstPublish: true, +}; + +function setAuthed() { + useAuth.setState({ + user: { id: 'u1', username: 'alice', role: 'user' }, + status: 'authed', + error: null, + autoLinkedAt: null, + profile: { + displayName: 'Alice', + bio: null, + avatarCardId: null, + avatarCardName: null, + avatarImageUrl: null, + }, + }); +} + +beforeEach(() => { + online = true; + setAuthed(); + publishDeckMock.mockReset().mockResolvedValue(PUB_FIRST); + updateProfileMock.mockReset(); + useToastsStore.setState({ toasts: [] }); +}); +afterEach(() => useToastsStore.setState({ toasts: [] })); + +describe('usePublishOnCreate — gating', () => { + it('canPublish is true when authed + online, with no disabled reason', () => { + const { result } = renderHook(() => usePublishOnCreate(vi.fn())); + expect(result.current.canPublish).toBe(true); + expect(result.current.publicDisabledReason).toBeNull(); + }); + + it('disables publishing for a guest, with a sign-in reason', () => { + useAuth.setState({ + user: null, + status: 'guest', + error: null, + autoLinkedAt: null, + profile: null, + }); + const { result } = renderHook(() => usePublishOnCreate(vi.fn())); + expect(result.current.canPublish).toBe(false); + expect(result.current.publicDisabledReason).toBe('Sign in to publish.'); + }); + + it('disables publishing while offline, with a reconnect reason', () => { + online = false; + const { result } = renderHook(() => usePublishOnCreate(vi.fn())); + expect(result.current.canPublish).toBe(false); + expect(result.current.publicDisabledReason).toBe("You're offline — reconnect to publish."); + }); + + it('snaps a selected Public back to Private if canPublish goes false underneath it', () => { + const { result, rerender } = renderHook(() => usePublishOnCreate(vi.fn())); + act(() => result.current.setVisibility('public')); + expect(result.current.visibility).toBe('public'); + + act(() => { + useAuth.setState({ + user: null, + status: 'guest', + error: null, + autoLinkedAt: null, + profile: null, + }); + }); + rerender(); + expect(result.current.visibility).toBe('private'); + }); +}); + +describe('usePublishOnCreate — publishAfterCreate', () => { + it('on success, threads isFirstPublish through onSettled', async () => { + const onSettled = vi.fn(); + const { result } = renderHook(() => usePublishOnCreate(onSettled)); + + await act(async () => { + await result.current.publishAfterCreate('deck-1'); + }); + + expect(publishDeckMock).toHaveBeenCalledTimes(1); + expect(onSettled).toHaveBeenCalledWith('deck-1', { isFirstPublish: true }); + expect(result.current.needsDisplayName).toBe(false); + }); + + it('retries once and succeeds when the deck is still racing its own fire-and-forget sync (DeckNotSyncedYetError)', async () => { + publishDeckMock + .mockRejectedValueOnce(new DeckNotSyncedYetError()) + .mockResolvedValueOnce(PUB_FIRST); + const onSettled = vi.fn(); + const { result } = renderHook(() => usePublishOnCreate(onSettled)); + + await act(async () => { + await result.current.publishAfterCreate('deck-1'); + }); + + expect(publishDeckMock).toHaveBeenCalledTimes(2); + expect(onSettled).toHaveBeenCalledWith('deck-1', { isFirstPublish: true }); + // Never surfaced as an error — the retry is invisible to the user. + expect(useToastsStore.getState().toasts.some((t) => t.tone === 'warn')).toBe(false); + }); + + it('gives up after exactly one retry, surfacing the failure like any other', async () => { + publishDeckMock + .mockRejectedValueOnce(new DeckNotSyncedYetError()) + .mockRejectedValueOnce(new DeckNotSyncedYetError()); + const onSettled = vi.fn(); + const { result } = renderHook(() => usePublishOnCreate(onSettled)); + + await act(async () => { + await result.current.publishAfterCreate('deck-1'); + }); + + expect(publishDeckMock).toHaveBeenCalledTimes(2); + expect(onSettled).toHaveBeenCalledWith('deck-1'); + expect(useToastsStore.getState().toasts.some((t) => t.tone === 'warn')).toBe(true); + }); + + it('on display_name_required, holds off onSettled and opens the inline substep', async () => { + publishDeckMock.mockRejectedValueOnce(new DisplayNameRequiredError()); + const onSettled = vi.fn(); + const { result } = renderHook(() => usePublishOnCreate(onSettled)); + + await act(async () => { + await result.current.publishAfterCreate('deck-2'); + }); + + expect(result.current.needsDisplayName).toBe(true); + expect(onSettled).not.toHaveBeenCalled(); + }); + + it('on a generic failure, toasts a warning and still calls onSettled with no outcome', async () => { + publishDeckMock.mockRejectedValueOnce(new Error('server exploded')); + const onSettled = vi.fn(); + const { result } = renderHook(() => usePublishOnCreate(onSettled)); + + await act(async () => { + await result.current.publishAfterCreate('deck-3'); + }); + + expect(onSettled).toHaveBeenCalledWith('deck-3'); + expect(result.current.needsDisplayName).toBe(false); + expect(useToastsStore.getState().toasts.some((t) => t.tone === 'warn')).toBe(true); + }); +}); + +describe('usePublishOnCreate — display-name substep', () => { + async function reachSubstep(onSettled = vi.fn()) { + publishDeckMock.mockRejectedValueOnce(new DisplayNameRequiredError()); + const hook = renderHook(() => usePublishOnCreate(onSettled)); + await act(async () => { + await hook.result.current.publishAfterCreate('deck-4'); + }); + return { ...hook, onSettled }; + } + + it('saveDisplayNameAndPublish updates the profile then retries publish exactly once, threading isFirstPublish', async () => { + updateProfileMock.mockResolvedValue({ + displayName: 'Bob', + bio: null, + avatarCardId: null, + avatarCardName: null, + avatarImageUrl: null, + }); + // reachSubstep() queues its own rejected-once first — do NOT queue a + // resolved-once ahead of it, that would jump the FIFO "once" queue and + // make the FIRST (expected-to-fail) publishDeck call resolve instead. + // The retry falls through to beforeEach's default mockResolvedValue. + const { result, onSettled } = await reachSubstep(); + + act(() => result.current.setDisplayNameDraft('Bob')); + await act(async () => { + await result.current.saveDisplayNameAndPublish(); + }); + + expect(updateProfileMock).toHaveBeenCalledWith({ displayName: 'Bob' }); + expect(publishDeckMock).toHaveBeenCalledTimes(2); // the original attempt + the one retry + expect(onSettled).toHaveBeenCalledWith('deck-4', { isFirstPublish: true }); + expect(result.current.needsDisplayName).toBe(false); + }); + + it('a failed save toasts a warning and still calls onSettled with no outcome', async () => { + updateProfileMock.mockRejectedValue(new Error('name taken')); + const { result, onSettled } = await reachSubstep(); + + act(() => result.current.setDisplayNameDraft('Bob')); + await act(async () => { + await result.current.saveDisplayNameAndPublish(); + }); + + expect(onSettled).toHaveBeenCalledWith('deck-4'); + expect(result.current.needsDisplayName).toBe(false); + expect(useToastsStore.getState().toasts.some((t) => t.tone === 'warn')).toBe(true); + }); + + it('cancelDisplayName never calls publishDeck again, and still settles the deck as created', async () => { + const { result, onSettled } = await reachSubstep(); + publishDeckMock.mockClear(); + + act(() => result.current.cancelDisplayName()); + + expect(publishDeckMock).not.toHaveBeenCalled(); + expect(result.current.needsDisplayName).toBe(false); + expect(onSettled).toHaveBeenCalledWith('deck-4'); + }); +}); + +describe('usePublishOnCreate — republish is never a first publish', () => { + it('threads isFirstPublish: false straight through when the server reports a refresh/republish', async () => { + publishDeckMock.mockResolvedValueOnce({ ...PUB_FIRST, isFirstPublish: false }); + const onSettled = vi.fn(); + const { result } = renderHook(() => usePublishOnCreate(onSettled)); + + await act(async () => { + await result.current.publishAfterCreate('deck-5'); + }); + + expect(onSettled).toHaveBeenCalledWith('deck-5', { isFirstPublish: false }); + }); +}); diff --git a/frontend/src/lib/use-publish-on-create.ts b/frontend/src/lib/use-publish-on-create.ts new file mode 100644 index 00000000..4fde8179 --- /dev/null +++ b/frontend/src/lib/use-publish-on-create.ts @@ -0,0 +1,171 @@ +import { useCallback, useEffect, useState } from 'react'; +import { useAuth } from '../store/auth'; +import { isOnline, onSyncedChange } from './sync'; +import { updateProfile } from './auth-api'; +import { + DeckNotSyncedYetError, + DisplayNameRequiredError, + publicationUrl, + publishDeck, + type PublishResult, +} from './publications-client'; +import { toast } from '../store/toasts'; + +export type CreateVisibility = 'private' | 'public'; + +/** Handed to `onSettled` only when a publish attempt actually succeeded. */ +export interface PublishOutcome { + isFirstPublish: boolean; +} + +/** How long a freshly-created deck's fire-and-forget local persist typically + * takes to reach the server (see publishWithSyncRetry below) — comfortably + * more than a signed-in web session's immediate write-through, and more + * than the native/guest debounced queue's 500ms window. */ +const SYNC_CATCH_UP_MS = 600; + +/** + * `publishDeck()`, tolerant of the one race every creation-time publish + * attempt is exposed to: `createDeck()`'s persist-to-server is fire-and- + * forget from the store's own subscriber (store/decks.ts) — there is no + * promise a caller can await to know the deck has actually reached the + * server. Firing `publishDeck()` immediately after `createDeck()` can + * therefore reach the server before the deck itself does, 404ing with "Deck + * not found." (caught live via browser validation — a mocked publishDeck in + * a unit test can't reproduce it). A single bounded retry after a short + * delay is far simpler and more robust than teaching this hook to couple to + * sync.ts's internals (E22 — audit before building anything sync-adjacent); + * by the second attempt the deck has always landed. + */ +async function publishWithSyncRetry(deckId: string): Promise { + try { + return await publishDeck(deckId); + } catch (err) { + if (!(err instanceof DeckNotSyncedYetError)) throw err; + await new Promise((r) => setTimeout(r, SYNC_CATCH_UP_MS)); + return await publishDeck(deckId); + } +} + +/** + * Shared creation-time "publish on create" flow — the exact choke point + * DeckNewPage's Private/Public fieldset (#1278) already used, now reused by + * ImportDeckDialog's single-deck path (E150) so this network/error dance + * doesn't fork into a second, slowly-drifting copy. Owns: guest/offline + * gating for the fieldset, the `display_name_required` inline substep + * (exactly one retry, mirroring ShareDialog's own fallback), and the success + * toast. + * + * Deliberately does NOT fire the first-publish seal moment itself — every + * caller navigates away the instant a publish resolves (straight to the new + * deck's editor), which would unmount the portal mid-animation. Instead it + * hands `PublishOutcome` to `onSettled`, whose caller threads it to wherever + * the flow actually lands (DeckEditorPage's `justPublished` router state), + * which is the real choke point for `shouldCelebrateFirstPublish`. + */ +export function usePublishOnCreate(onSettled: (deckId: string, outcome?: PublishOutcome) => void) { + const isGuest = useAuth((s) => s.status === 'guest'); + const [, forceOnlineTick] = useState(0); + useEffect(() => onSyncedChange(() => forceOnlineTick((n) => n + 1)), []); + const online = isOnline(); + const canPublish = !isGuest && online; + const publicDisabledReason = isGuest + ? 'Sign in to publish.' + : !online + ? "You're offline — reconnect to publish." + : null; + + const [visibility, setVisibility] = useState('private'); + // Never leave Public selected-but-disabled (e.g. connectivity drops after + // it was chosen) — snap back to Private during render, mirroring + // DeckNewPage's identical guarded render-time setState (terminating, so + // react-hooks/set-state-in-effect doesn't apply — there's no effect here). + if (!canPublish && visibility === 'public') { + setVisibility('private'); + } + + const [publishing, setPublishing] = useState(false); + const [needsDisplayName, setNeedsDisplayName] = useState(false); + const [displayNameDraft, setDisplayNameDraft] = useState(''); + const [pendingPublishId, setPendingPublishId] = useState(null); + + const announcePublished = (slug: string) => { + toast.show({ + message: `Published — anyone can view it at ${publicationUrl(slug)}`, + tone: 'success', + }); + }; + + /** Publish a just-created deck. On display_name_required, hold off calling + * onSettled and show the inline set-name substep; any other failure just + * toasts a warning — the deck already exists, so a failed publish never + * blocks getting to it. */ + const publishAfterCreate = useCallback( + async (deckId: string) => { + setPublishing(true); + try { + const pub = await publishWithSyncRetry(deckId); + announcePublished(pub.slug); + onSettled(deckId, { isFirstPublish: pub.isFirstPublish }); + } catch (err) { + if (err instanceof DisplayNameRequiredError) { + setPendingPublishId(deckId); + setNeedsDisplayName(true); + } else { + toast.show({ + message: err instanceof Error ? err.message : 'Failed to publish deck.', + tone: 'warn', + }); + onSettled(deckId); + } + } finally { + setPublishing(false); + } + }, + [onSettled] + ); + + const saveDisplayNameAndPublish = useCallback(async () => { + const trimmed = displayNameDraft.trim(); + if (!trimmed || !pendingPublishId || publishing) return; + setPublishing(true); + try { + const updated = await updateProfile({ displayName: trimmed }); + useAuth.setState((s) => (s.profile ? { profile: { ...s.profile, ...updated } } : s)); + // The display-name substep's own one retry after saving a name — not + // to be confused with publishWithSyncRetry's inner sync-catch-up retry. + const pub = await publishWithSyncRetry(pendingPublishId); + announcePublished(pub.slug); + onSettled(pendingPublishId, { isFirstPublish: pub.isFirstPublish }); + } catch (err) { + toast.show({ + message: err instanceof Error ? err.message : "Couldn't publish the deck.", + tone: 'warn', + }); + onSettled(pendingPublishId); + } finally { + setPublishing(false); + setNeedsDisplayName(false); + } + }, [displayNameDraft, pendingPublishId, publishing, onSettled]); + + const cancelDisplayName = useCallback(() => { + // Deck stays created + private — never blocks creation. + setNeedsDisplayName(false); + if (pendingPublishId) onSettled(pendingPublishId); + }, [pendingPublishId, onSettled]); + + return { + canPublish, + publicDisabledReason, + visibility, + setVisibility, + publishing, + needsDisplayName, + displayNameDraft, + setDisplayNameDraft, + publishAfterCreate, + saveDisplayNameAndPublish, + cancelDisplayName, + }; +} diff --git a/frontend/src/pages/DeckEditorPage.delete.test.tsx b/frontend/src/pages/DeckEditorPage.delete.test.tsx index 5f0d9822..456b2384 100644 --- a/frontend/src/pages/DeckEditorPage.delete.test.tsx +++ b/frontend/src/pages/DeckEditorPage.delete.test.tsx @@ -133,6 +133,13 @@ vi.mock('../components/deck/DeckDisplay', () => ({ vi.mock('../components/deck/DeckVisibilityChip', () => ({ DeckVisibilityChip: () => null, })); +// DeckPublishNudge (E150) pulls in the same ShareDialog tree as the chip +// above, for the identical reason — none of these delete-flow tests navigate +// here with promptVisibility router state, so it never actually renders, but +// mock it anyway so the import itself can't reintroduce the fetch flake. +vi.mock('../components/deck/DeckPublishNudge', () => ({ + DeckPublishNudge: () => null, +})); vi.mock('../components/deck/CardSearchPanel', () => ({ CardSearchPanel: () =>
, })); diff --git a/frontend/src/pages/DeckEditorPage.tsx b/frontend/src/pages/DeckEditorPage.tsx index 07faea32..cf8b5cb6 100644 --- a/frontend/src/pages/DeckEditorPage.tsx +++ b/frontend/src/pages/DeckEditorPage.tsx @@ -34,6 +34,9 @@ import { DeckTokensSheet } from '../components/deck/DeckTokensSheet'; import { DeckPrimerSheet } from '../components/deck/DeckPrimerSheet'; import { ForkedFromBadge } from '../components/deck/ForkedFromBadge'; import { DeckVisibilityChip } from '../components/deck/DeckVisibilityChip'; +import { DeckPublishNudge } from '../components/deck/DeckPublishNudge'; +import { useSealMoment } from '../components/shared/SealMoment'; +import { shouldCelebrateFirstPublish } from '../lib/first-publish-celebration'; import { PullListSheet } from '../components/deck/PullListSheet'; import { useDeckTokens } from '../components/deck/use-deck-tokens'; import { PowerHero } from '../components/deck/PowerHero'; @@ -341,6 +344,25 @@ export function DeckEditorPage() { !isBuildReportSeen(deck?.id ?? '') ) ); + // First-publish seal + post-create visibility nudge (E150) — two more + // one-shot router-state flags in the same `justGenerated` shape, set by + // whichever creation flow the deck just came from: + // - justPublished: the creation-time fieldset (DeckNewPage, + // ImportDeckDialog) already published it. Firing the seal HERE rather + // than on the creating page is deliberate — that page navigates away + // the instant publish resolves, which would unmount the portal + // mid-animation (see usePublishOnCreate's doc comment). + // - promptVisibility: the flow (CopyDeckButton, a multi-file import that + // landed on one deck) skipped the fieldset entirely — show the lighter + // DeckPublishNudge instead of celebrating. + // Mutually exclusive by construction (a given creation flow sets at most + // one), captured once at mount like showBuildReport above. + const [justPublished] = useState( + () => !!(location.state as { justPublished?: boolean } | null)?.justPublished + ); + const [showPublishNudge] = useState( + () => !!(location.state as { promptVisibility?: boolean } | null)?.promptVisibility + ); // Feedback Tool: mint/copy the suggestion link + review submitted responses. const [feedbackOpen, setFeedbackOpen] = useState(false); const [addZone, setAddZone] = useState<'main' | 'side'>('main'); @@ -710,6 +732,27 @@ export function DeckEditorPage() { const comboColorIdentity = deck?.commander || deck?.partnerCommander ? commanderColorIdentity : undefined; + // First-publish seal moment (E150), driven by the justPublished router + // state set above. Fires here rather than on the creating page (DeckNewPage + // / ImportDeckDialog navigate away the instant publish resolves, which + // would unmount the portal mid-animation — see usePublishOnCreate's doc + // comment). `shouldCelebrateFirstPublish` is the single dedup guard shared + // with ShareDialog's own direct-fire path (the chip + DeckPublishNudge), + // so a deck's first publish celebrates exactly once regardless of entry + // surface. Deliberately narrow deps — a one-shot flag that never resets, + // re-run only to catch `deck` resolving after the initial render (async + // store hydration); re-firing on a later commander edit would be a no-op + // anyway (the guard is keyed on deckId), so there's nothing to gain by + // tracking commanderColorIdentity here too. + const { fire: fireSealMoment, moment: sealMoment } = useSealMoment(); + useEffect(() => { + if (!justPublished || !deck) return; + if (shouldCelebrateFirstPublish(deck.id, true)) { + fireSealMoment(commanderColorIdentity); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [justPublished, deck?.id]); + // Commander (+partner) key for CardSearchPanel's per-row "N on SpellControl" // badge (social W4) — undefined for a no-commander deck, so the panel's // fetch never fires. @@ -2173,7 +2216,15 @@ export function DeckEditorPage() { return (
+ {sealMoment} + {showPublishNudge && ( + + )}
{/* Commander art rides behind the title as a right-anchored backdrop @@ -2264,7 +2315,11 @@ export function DeckEditorPage() { {`\u00A0· Bracket\u00A0${bracketValue}`} )}

- + {deck.forkedFrom && }
diff --git a/frontend/src/pages/DeckNewPage.test.tsx b/frontend/src/pages/DeckNewPage.test.tsx index 4bb88248..be5911fc 100644 --- a/frontend/src/pages/DeckNewPage.test.tsx +++ b/frontend/src/pages/DeckNewPage.test.tsx @@ -10,7 +10,7 @@ import 'fake-indexeddb/auto'; import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { Publication } from '../lib/publications-client'; +import type { PublishResult } from '../lib/publications-client'; const navigateMock = vi.fn(); vi.mock('react-router-dom', async (importOriginal) => { @@ -34,7 +34,7 @@ vi.mock('../lib/sync', () => ({ onSyncedChange: () => () => {}, })); -const publishDeckMock = vi.fn<() => Promise>(); +const publishDeckMock = vi.fn<() => Promise>(); vi.mock('../lib/publications-client', async (importOriginal) => { const actual = await importOriginal(); return { @@ -73,7 +73,7 @@ function selectStandardFormat() { fireEvent.click(screen.getByRole('radio', { name: 'Standard' })); } -const PUB: Publication = { +const PUB: PublishResult = { slug: 'my-new-deck', url: 'https://spellcontrol.com/d/my-new-deck', publishedAt: 1, @@ -81,6 +81,7 @@ const PUB: Publication = { unpublishedAt: null, viewCount: 0, copyCount: 0, + isFirstPublish: true, }; describe('DeckNewPage — creation-time visibility', () => { @@ -117,7 +118,7 @@ describe('DeckNewPage — creation-time visibility', () => { expect(createButton.disabled).toBe(false); }); - it('publishes the deck after creation when Public is selected, and navigates to the editor', async () => { + it('publishes the deck after creation when Public is selected, navigates to the editor, and flags the first-publish seal', async () => { renderPage(); selectStandardFormat(); @@ -129,7 +130,26 @@ describe('DeckNewPage — creation-time visibility', () => { expect.objectContaining({ format: 'standard', source: 'manual' }) ); await waitFor(() => expect(publishDeckMock).toHaveBeenCalledTimes(1)); - await waitFor(() => expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id')); + await waitFor(() => + expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id', { + state: { justPublished: true }, + }) + ); + }); + + it('threads justPublished: false through when the server reports a republish, not a first publish', async () => { + publishDeckMock.mockResolvedValue({ ...PUB, isFirstPublish: false }); + renderPage(); + selectStandardFormat(); + + fireEvent.click(screen.getByRole('radio', { name: 'Public' })); + fireEvent.click(screen.getByRole('button', { name: 'Create deck' })); + + await waitFor(() => + expect(navigateMock).toHaveBeenCalledWith('/decks/new-deck-id', { + state: { justPublished: false }, + }) + ); }); it('never calls publishDeck when Private (the default) is kept', async () => { diff --git a/frontend/src/pages/DeckNewPage.tsx b/frontend/src/pages/DeckNewPage.tsx index 9277917f..3f57df57 100644 --- a/frontend/src/pages/DeckNewPage.tsx +++ b/frontend/src/pages/DeckNewPage.tsx @@ -15,11 +15,7 @@ import { useGenerationTakeoverExit } from '../lib/use-generation-takeover-exit'; import { useCollectionStore } from '../store/collection'; import { useDecksStore } from '../store/decks'; import { buildAllocationMap, pickCollectionCopy } from '../lib/allocations'; -import { useAuth } from '../store/auth'; -import { toast } from '../store/toasts'; -import { updateProfile } from '../lib/auth-api'; -import { isOnline, onSyncedChange } from '../lib/sync'; -import { DisplayNameRequiredError, publicationUrl, publishDeck } from '../lib/publications-client'; +import { usePublishOnCreate, type PublishOutcome } from '../lib/use-publish-on-create'; import type { ScryfallCard, DeckFormat, EDHRECTheme } from '@/deck-builder/types'; import { DECK_FORMAT_CONFIGS } from '@/deck-builder/lib/constants/archetypes'; @@ -85,95 +81,32 @@ export function DeckNewPage() { const isPdh = selectedFormat === 'paupercommander'; // ── Visibility (creation-time choice) ────────────────────────────────── - const isGuest = useAuth((s) => s.status === 'guest'); - const [, forceOnlineTick] = useState(0); - useEffect(() => onSyncedChange(() => forceOnlineTick((n) => n + 1)), []); - const online = isOnline(); - const canPublish = !isGuest && online; - const publicDisabledReason = isGuest - ? 'Sign in to publish.' - : !online - ? "You're offline — reconnect to publish." - : null; - - const [visibility, setVisibility] = useState<'private' | 'public'>('private'); - // Never leave Public selected-but-disabled (e.g. connectivity drops after - // it was chosen) — snap back to Private during render (React's "adjusting - // state when a value changes" pattern, not an effect: this is a guarded, - // terminating setState call during render, so react-hooks/set-state-in-effect - // doesn't apply — there's no effect here to begin with). - if (!canPublish && visibility === 'public') { - setVisibility('private'); - } - - const [publishing, setPublishing] = useState(false); - const [needsDisplayName, setNeedsDisplayName] = useState(false); - const [displayNameDraft, setDisplayNameDraft] = useState(''); - const [pendingPublishId, setPendingPublishId] = useState(null); - - const announcePublished = (slug: string) => { - toast.show({ - message: `Published — anyone can view it at ${publicationUrl(slug)}`, - tone: 'success', - }); - }; - - /** Publish a just-created deck. On display_name_required, hold off - * navigating and show the same inline set-name substep ShareDialog uses; - * any other failure just toasts a warning — the deck already exists, so - * a failed publish never blocks getting to the editor. */ - const publishAfterCreate = useCallback( - async (id: string) => { - setPublishing(true); - try { - const pub = await publishDeck(id); - announcePublished(pub.slug); - navigate(`/decks/${id}`); - } catch (err) { - if (err instanceof DisplayNameRequiredError) { - setPendingPublishId(id); - setNeedsDisplayName(true); - } else { - toast.show({ - message: err instanceof Error ? err.message : 'Failed to publish deck.', - tone: 'warn', - }); - navigate(`/decks/${id}`); - } - } finally { - setPublishing(false); - } + // The fieldset/network/display-name-substep logic itself is shared with + // ImportDeckDialog's single-deck path (E150) — see usePublishOnCreate's + // own doc comment for why it hands off rather than firing the seal here: + // this page navigates away the instant a publish resolves. + const onPublishSettled = useCallback( + (id: string, outcome?: PublishOutcome) => { + navigate( + `/decks/${id}`, + outcome ? { state: { justPublished: outcome.isFirstPublish } } : undefined + ); }, [navigate] ); - - const handleSaveDisplayNameAndPublish = async () => { - const trimmed = displayNameDraft.trim(); - if (!trimmed || !pendingPublishId || publishing) return; - setPublishing(true); - try { - const updated = await updateProfile({ displayName: trimmed }); - useAuth.setState((s) => (s.profile ? { profile: { ...s.profile, ...updated } } : s)); - const pub = await publishDeck(pendingPublishId); // exactly one retry - announcePublished(pub.slug); - navigate(`/decks/${pendingPublishId}`); - } catch (err) { - toast.show({ - message: err instanceof Error ? err.message : "Couldn't publish the deck.", - tone: 'warn', - }); - navigate(`/decks/${pendingPublishId}`); - } finally { - setPublishing(false); - setNeedsDisplayName(false); - } - }; - - const handleCancelDisplayName = () => { - // Deck stays created + private — never blocks creation. - setNeedsDisplayName(false); - if (pendingPublishId) navigate(`/decks/${pendingPublishId}`); - }; + const { + canPublish, + publicDisabledReason, + visibility, + setVisibility, + publishing, + needsDisplayName, + displayNameDraft, + setDisplayNameDraft, + publishAfterCreate, + saveDisplayNameAndPublish, + cancelDisplayName, + } = usePublishOnCreate(onPublishSettled); // Keep the store's build-format in lockstep with the pill so generation and // the saved deck both know the format. Only PDH generates as its own format @@ -351,13 +284,13 @@ export function DeckNewPage() { onChange={(e) => setDisplayNameDraft(e.target.value)} />
-