diff --git a/craft/src/state/PageContext.slug.test.tsx b/craft/src/state/PageContext.slug.test.tsx new file mode 100644 index 0000000..03469ae --- /dev/null +++ b/craft/src/state/PageContext.slug.test.tsx @@ -0,0 +1,146 @@ +import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; +import React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; +import { PageProvider, usePages, uniqueSlug } from './PageContext'; + +/* PageContext only needs `useEditor` from @craftjs/core (for query.serialize / + actions.deserialize during page switches) — mock just that so PageProvider + can mount without a real tree, following the DOM-harness pattern + used in src/ui/AssetPicker.test.tsx (no @testing-library/react in this repo). */ +vi.mock('@craftjs/core', () => ({ + useEditor: () => ({ + query: { serialize: () => '{}' }, + actions: { deserialize: vi.fn() }, + }), +})); + +/* addPage mints ids from `Date.now()`. Two adds inside the same test can land + in the same millisecond and collide on id, which is an existing, unrelated + bug (id collision, not slug collision) — out of scope here but it makes + these tests flaky since a colliding id defeats the "other pages" slug + lookup. Force distinct ids so the slug-dedupe assertions below are stable. */ +let dateNowSpy: ReturnType; +beforeEach(() => { + let counter = 1_700_000_000_000; + dateNowSpy = vi.spyOn(Date, 'now').mockImplementation(() => counter++); +}); +afterEach(() => { + dateNowSpy.mockRestore(); +}); + +let container: HTMLDivElement; +let root: Root; + +function render(ui: React.ReactElement) { + container = document.createElement('div'); + document.body.appendChild(container); + act(() => { + root = createRoot(container); + root.render(ui); + }); +} + +function unmount() { + act(() => { + root.unmount(); + }); + container.remove(); +} + +describe('uniqueSlug', () => { + test('returns base unchanged when no collision', () => { + expect(uniqueSlug('about', ['index', 'contact'])).toBe('about'); + }); + + test('appends -2 on first collision, -3 on the next, etc', () => { + expect(uniqueSlug('about', ['about'])).toBe('about-2'); + expect(uniqueSlug('about', ['about', 'about-2'])).toBe('about-3'); + }); +}); + +describe('PageProvider slug dedupe', () => { + test('addPage: two pages with the same name yield distinct slugs', () => { + let ctx: ReturnType | null = null; + const Consumer: React.FC = () => { + ctx = usePages(); + return null; + }; + + render( + + + , + ); + + act(() => { + ctx!.addPage('About', ''); + }); + act(() => { + ctx!.addPage('About', ''); + }); + + const slugs = ctx!.pages.map((p) => p.slug); + expect(slugs).toEqual(['index', 'about', 'about-2']); + expect(new Set(slugs).size).toBe(slugs.length); + + unmount(); + }); + + test('renamePage: renaming to an already-used slug is deduped', () => { + let ctx: ReturnType | null = null; + const Consumer: React.FC = () => { + ctx = usePages(); + return null; + }; + + render( + + + , + ); + + act(() => { + ctx!.addPage('About', 'about'); + }); + act(() => { + ctx!.addPage('Contact', 'contact'); + }); + + const contactPage = ctx!.pages.find((p) => p.name === 'Contact')!; + act(() => { + ctx!.renamePage(contactPage.id, 'Contact', 'about'); + }); + + const renamed = ctx!.pages.find((p) => p.id === contactPage.id)!; + expect(renamed.slug).toBe('about-2'); + + const slugs = ctx!.pages.map((p) => p.slug); + expect(new Set(slugs).size).toBe(slugs.length); + + unmount(); + }); + + test('landing page slug always stays "index" even if renamed to collide', () => { + let ctx: ReturnType | null = null; + const Consumer: React.FC = () => { + ctx = usePages(); + return null; + }; + + render( + + + , + ); + + const landingId = ctx!.pages[0].id; + act(() => { + ctx!.renamePage(landingId, 'Whatever', 'whatever'); + }); + + expect(ctx!.pages[0].slug).toBe('index'); + + unmount(); + }); +}); diff --git a/craft/src/state/PageContext.tsx b/craft/src/state/PageContext.tsx index 66994af..55b2220 100644 --- a/craft/src/state/PageContext.tsx +++ b/craft/src/state/PageContext.tsx @@ -148,6 +148,19 @@ function slugify(name: string): string { .replace(/-+/g, '-'); } +/** + * Append `-2`, `-3`, … to `base` until it no longer collides with + * `existingSlugs`. Two pages that slugify to the same string (e.g. both + * named "About") must not both publish to `about.html` — the second write + * would silently overwrite the first on publish. + */ +export function uniqueSlug(base: string, existingSlugs: string[]): string { + if (!existingSlugs.includes(base)) return base; + let i = 2; + while (existingSlugs.includes(`${base}-${i}`)) i++; + return `${base}-${i}`; +} + const DEFAULT_PAGE: PageData = { id: 'home', name: 'Home', @@ -278,7 +291,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) => const addPage = useCallback( (name: string, slug: string) => { - const finalSlug = slug || slugify(name); + const requestedSlug = slug || slugify(name); const id = `page_${Date.now()}`; // Save current page first @@ -289,7 +302,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) => { id, name, - slug: finalSlug, + slug: uniqueSlug(requestedSlug, prev.map((p) => p.slug)), craftState: null, headCode: '', }, @@ -335,8 +348,10 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) => // First page is the landing page — its slug is locked to 'index' so // the file always publishes to index.html regardless of the user-set // name. The display name can change freely. - const nextSlug = i === 0 ? 'index' : (slug || slugify(name)); - return { ...p, name, slug: nextSlug }; + if (i === 0) return { ...p, name, slug: 'index' }; + const requestedSlug = slug || slugify(name); + const otherSlugs = prev.filter((pp) => pp.id !== pageId).map((pp) => pp.slug); + return { ...p, name, slug: uniqueSlug(requestedSlug, otherSlugs) }; }), ); }, []); @@ -467,16 +482,23 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) => */ const replaceAllPages = useCallback((newPages: { name: string; tree: SerializedTreeNode }[]) => { if (newPages.length === 0) return; - const built = newPages.map((p, i) => ({ - id: i === 0 ? 'home' : `page_${Date.now()}_${i}`, - name: p.name, + // Track slugs as they're assigned so later pages dedupe against earlier + // ones in the same batch (e.g. the AI generating two "About" pages). + const seenSlugs: string[] = []; + const built = newPages.map((p, i) => { // First page must publish to index.html so it serves at the site root. // Apache resolves '/' to index.html, not home.html — without this, the // AI's "Home" page lands at /home.html and visitors hit a blank root. - slug: i === 0 ? 'index' : slugify(p.name), - craftState: treeToState(p.tree), - headCode: '', - })); + const slug = i === 0 ? 'index' : uniqueSlug(slugify(p.name), seenSlugs); + seenSlugs.push(slug); + return { + id: i === 0 ? 'home' : `page_${Date.now()}_${i}`, + name: p.name, + slug, + craftState: treeToState(p.tree), + headCode: '', + }; + }); setPages(built); // Load the first page into the live canvas const firstState = built[0].craftState;