fix(builder): dedupe page slugs on add/rename/replaceAll (D3)
Two pages that slugify to the same string (e.g. both named "About") previously both published to about.html, silently overwriting each other on publish. Add a pure uniqueSlug(base, existingSlugs) helper that appends -2, -3, ... on collision, and apply it in addPage, renamePage, and replaceAllPages. The landing page's slug stays locked to 'index' regardless of collisions, matching existing behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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 <Editor> 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<typeof vi.spyOn>;
|
||||||
|
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<typeof usePages> | null = null;
|
||||||
|
const Consumer: React.FC = () => {
|
||||||
|
ctx = usePages();
|
||||||
|
return null;
|
||||||
|
};
|
||||||
|
|
||||||
|
render(
|
||||||
|
<PageProvider>
|
||||||
|
<Consumer />
|
||||||
|
</PageProvider>,
|
||||||
|
);
|
||||||
|
|
||||||
|
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<typeof usePages> | null = null;
|
||||||
|
const Consumer: React.FC = () => {
|
||||||
|
ctx = usePages();
|
||||||
|
return null;
|
||||||
|
};
|
||||||
|
|
||||||
|
render(
|
||||||
|
<PageProvider>
|
||||||
|
<Consumer />
|
||||||
|
</PageProvider>,
|
||||||
|
);
|
||||||
|
|
||||||
|
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<typeof usePages> | null = null;
|
||||||
|
const Consumer: React.FC = () => {
|
||||||
|
ctx = usePages();
|
||||||
|
return null;
|
||||||
|
};
|
||||||
|
|
||||||
|
render(
|
||||||
|
<PageProvider>
|
||||||
|
<Consumer />
|
||||||
|
</PageProvider>,
|
||||||
|
);
|
||||||
|
|
||||||
|
const landingId = ctx!.pages[0].id;
|
||||||
|
act(() => {
|
||||||
|
ctx!.renamePage(landingId, 'Whatever', 'whatever');
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(ctx!.pages[0].slug).toBe('index');
|
||||||
|
|
||||||
|
unmount();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -148,6 +148,19 @@ function slugify(name: string): string {
|
|||||||
.replace(/-+/g, '-');
|
.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 = {
|
const DEFAULT_PAGE: PageData = {
|
||||||
id: 'home',
|
id: 'home',
|
||||||
name: 'Home',
|
name: 'Home',
|
||||||
@@ -278,7 +291,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
|
|||||||
|
|
||||||
const addPage = useCallback(
|
const addPage = useCallback(
|
||||||
(name: string, slug: string) => {
|
(name: string, slug: string) => {
|
||||||
const finalSlug = slug || slugify(name);
|
const requestedSlug = slug || slugify(name);
|
||||||
const id = `page_${Date.now()}`;
|
const id = `page_${Date.now()}`;
|
||||||
|
|
||||||
// Save current page first
|
// Save current page first
|
||||||
@@ -289,7 +302,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
|
|||||||
{
|
{
|
||||||
id,
|
id,
|
||||||
name,
|
name,
|
||||||
slug: finalSlug,
|
slug: uniqueSlug(requestedSlug, prev.map((p) => p.slug)),
|
||||||
craftState: null,
|
craftState: null,
|
||||||
headCode: '',
|
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
|
// 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
|
// the file always publishes to index.html regardless of the user-set
|
||||||
// name. The display name can change freely.
|
// name. The display name can change freely.
|
||||||
const nextSlug = i === 0 ? 'index' : (slug || slugify(name));
|
if (i === 0) return { ...p, name, slug: 'index' };
|
||||||
return { ...p, name, slug: nextSlug };
|
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 }[]) => {
|
const replaceAllPages = useCallback((newPages: { name: string; tree: SerializedTreeNode }[]) => {
|
||||||
if (newPages.length === 0) return;
|
if (newPages.length === 0) return;
|
||||||
const built = newPages.map((p, i) => ({
|
// Track slugs as they're assigned so later pages dedupe against earlier
|
||||||
id: i === 0 ? 'home' : `page_${Date.now()}_${i}`,
|
// ones in the same batch (e.g. the AI generating two "About" pages).
|
||||||
name: p.name,
|
const seenSlugs: string[] = [];
|
||||||
|
const built = newPages.map((p, i) => {
|
||||||
// First page must publish to index.html so it serves at the site root.
|
// 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
|
// 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.
|
// AI's "Home" page lands at /home.html and visitors hit a blank root.
|
||||||
slug: i === 0 ? 'index' : slugify(p.name),
|
const slug = i === 0 ? 'index' : uniqueSlug(slugify(p.name), seenSlugs);
|
||||||
craftState: treeToState(p.tree),
|
seenSlugs.push(slug);
|
||||||
headCode: '',
|
return {
|
||||||
}));
|
id: i === 0 ? 'home' : `page_${Date.now()}_${i}`,
|
||||||
|
name: p.name,
|
||||||
|
slug,
|
||||||
|
craftState: treeToState(p.tree),
|
||||||
|
headCode: '',
|
||||||
|
};
|
||||||
|
});
|
||||||
setPages(built);
|
setPages(built);
|
||||||
// Load the first page into the live canvas
|
// Load the first page into the live canvas
|
||||||
const firstState = built[0].craftState;
|
const firstState = built[0].craftState;
|
||||||
|
|||||||
Reference in New Issue
Block a user