Skip to content

Commit 6938dbe

Browse files
edrplsclaude
andcommitted
fix: don't auto-redirect a slug another entry still holds
A collection's url_pattern has no locale token, so every locale variant of an entry generates the same URL — and slugs are unique per (slug, locale), so a translation may legitimately share its canonical's slug. Renaming either one emitted a 301 whose source was the other's live URL. The redirect middleware runs order: "pre", so the surviving page became unreachable with no way for routing to recover. Skip the auto-redirect when another non-deleted entry in the collection still holds the old slug. Any surviving row counts, published or not: a draft that publishes later would otherwise be shadowed by the redirect. Renames that genuinely free the URL still redirect as before. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent b6d3d83 commit 6938dbe

3 files changed

Lines changed: 157 additions & 0 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"emdash": patch
3+
---
4+
5+
Fixes renaming a translation taking its canonical page offline. When two locale variants share a slug, renaming either one created a 301 away from a URL the other still serves, making the live page unreachable. Slug-change redirects are now skipped when another entry still holds the old slug.

packages/core/src/api/handlers/content.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,15 @@ async function createSlugChangeRedirect(
376376
newSlug: string,
377377
contentId: string,
378378
): Promise<void> {
379+
// A URL pattern has no locale token, so every locale variant of an entry
380+
// generates the same URL, and slugs are unique per (slug, locale) — a
381+
// translation may still hold the old slug. Redirecting away from a URL
382+
// another row still answers on would take that page down: the redirect
383+
// middleware runs `order: "pre"`, so routing never gets a chance.
384+
// Any surviving row counts, published or not: a draft that publishes later
385+
// would otherwise be shadowed by the redirect.
386+
if (await slugStillTaken(db, collection, oldSlug, contentId)) return;
387+
379388
const collectionRow = await db
380389
.selectFrom("_emdash_collections")
381390
.select("url_pattern")
@@ -393,6 +402,24 @@ async function createSlugChangeRedirect(
393402
invalidateRedirectCache();
394403
}
395404

405+
/** Whether a row other than `contentId` still holds `slug` in this collection. */
406+
async function slugStillTaken(
407+
db: Kysely<Database>,
408+
collection: string,
409+
slug: string,
410+
contentId: string,
411+
): Promise<boolean> {
412+
validateIdentifier(collection, "collection slug");
413+
const result = await sql<{ id: string }>`
414+
SELECT id FROM ${sql.ref(`ec_${collection}`)}
415+
WHERE slug = ${slug}
416+
AND id != ${contentId}
417+
AND deleted_at IS NULL
418+
LIMIT 1
419+
`.execute(db);
420+
return result.rows.length > 0;
421+
}
422+
396423
/** Matches a date-only `YYYY-MM-DD` bound (no time component). */
397424
const DATE_ONLY_RE = /^\d{4}-\d{2}-\d{2}$/;
398425

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/**
2+
* Slug-change auto-redirects must never point away from a URL that still
3+
* serves content.
4+
*
5+
* A collection's `url_pattern` has no locale token, so every locale variant of
6+
* an entry maps to the same generated URL. Slugs are unique per
7+
* `(slug, locale)`, so a translation legitimately shares its canonical's slug
8+
* — and renaming one of them would otherwise emit a 301 whose source is the
9+
* other's live URL. The redirect middleware runs `order: "pre"`, so such a
10+
* redirect takes the live page down with no way for routing to recover.
11+
*/
12+
13+
import type { Kysely } from "kysely";
14+
import { afterEach, beforeEach, describe, expect, it } from "vitest";
15+
16+
import { handleContentUpdate } from "../../../src/api/handlers/content.js";
17+
import { ContentRepository } from "../../../src/database/repositories/content.js";
18+
import type { Database } from "../../../src/database/types.js";
19+
import { setI18nConfig } from "../../../src/i18n/config.js";
20+
import { SchemaRegistry } from "../../../src/schema/registry.js";
21+
import { setupTestDatabase, teardownTestDatabase } from "../../utils/test-db.js";
22+
23+
describe("slug-change auto-redirect — locale awareness", () => {
24+
let db: Kysely<Database>;
25+
let repo: ContentRepository;
26+
27+
beforeEach(async () => {
28+
db = await setupTestDatabase();
29+
repo = new ContentRepository(db);
30+
31+
const registry = new SchemaRegistry(db);
32+
await registry.createCollection({
33+
slug: "venue",
34+
label: "Venues",
35+
labelSingular: "Venue",
36+
urlPattern: "/sede/{slug}",
37+
});
38+
await registry.createField("venue", {
39+
slug: "title",
40+
label: "Title",
41+
type: "string",
42+
});
43+
44+
// Spanish canonical site: es is the default locale, en is a translation.
45+
setI18nConfig({ locales: ["es", "en"], defaultLocale: "es" });
46+
});
47+
48+
afterEach(async () => {
49+
setI18nConfig(null);
50+
await teardownTestDatabase(db);
51+
});
52+
53+
async function redirectSources(): Promise<string[]> {
54+
const rows = await db.selectFrom("_emdash_redirects").select(["source"]).execute();
55+
return rows.map((r) => r.source);
56+
}
57+
58+
it("does not redirect the canonical URL when a translation sharing its slug is renamed", async () => {
59+
const canonical = await repo.create({
60+
type: "venue",
61+
slug: "cineteca-nacional",
62+
locale: "es",
63+
status: "published",
64+
data: { title: "Cineteca Nacional" },
65+
});
66+
const twin = await repo.create({
67+
type: "venue",
68+
slug: "cineteca-nacional",
69+
locale: "en",
70+
status: "published",
71+
translationOf: canonical.id,
72+
data: { title: "Cineteca Nacional" },
73+
});
74+
75+
const result = await handleContentUpdate(db, "venue", twin.id, { slug: "cinematheque" });
76+
expect(result.success).toBe(true);
77+
78+
// The canonical Spanish page still lives at /sede/cineteca-nacional/.
79+
expect(await redirectSources()).not.toContain("/sede/cineteca-nacional");
80+
});
81+
82+
it("does not redirect a URL still held by a translation when the canonical is renamed", async () => {
83+
const canonical = await repo.create({
84+
type: "venue",
85+
slug: "cineteca-nacional",
86+
locale: "es",
87+
status: "published",
88+
data: { title: "Cineteca Nacional" },
89+
});
90+
await repo.create({
91+
type: "venue",
92+
slug: "cineteca-nacional",
93+
locale: "en",
94+
status: "published",
95+
translationOf: canonical.id,
96+
data: { title: "Cineteca Nacional" },
97+
});
98+
99+
const result = await handleContentUpdate(db, "venue", canonical.id, { slug: "cineteca" });
100+
expect(result.success).toBe(true);
101+
102+
expect(await redirectSources()).not.toContain("/sede/cineteca-nacional");
103+
});
104+
105+
it("still redirects when the renamed entry owned the URL outright", async () => {
106+
const solo = await repo.create({
107+
type: "venue",
108+
slug: "teatro-viejo",
109+
locale: "es",
110+
status: "published",
111+
data: { title: "Teatro Viejo" },
112+
});
113+
114+
const result = await handleContentUpdate(db, "venue", solo.id, { slug: "teatro-nuevo" });
115+
expect(result.success).toBe(true);
116+
117+
const rows = await db
118+
.selectFrom("_emdash_redirects")
119+
.select(["source", "destination"])
120+
.execute();
121+
expect(rows).toEqual([
122+
expect.objectContaining({ source: "/sede/teatro-viejo", destination: "/sede/teatro-nuevo" }),
123+
]);
124+
});
125+
});

0 commit comments

Comments
 (0)