From 290d4baf267df07047d381f191a4aefc7c3cd137 Mon Sep 17 00:00:00 2001 From: MastersGame Date: Sat, 14 Nov 2026 12:23:47 +0100 Subject: [PATCH] fix(seo): keep former slugs reserved to their entity, date sitemap products from Hiboutik --- CLAUDE.md | 2 +- .../examples/Public pages.example.yaml | 6 +- .../DinggaDingga/getSitemap.request.yaml | 2 +- src/database/sitemap.database.repository.ts | 34 ++++++----- src/database/slug.database.repository.spec.ts | 57 +++++++++++++++++++ src/database/slug.database.repository.ts | 14 ++++- test/app.e2e-spec.ts | 25 +++++--- 7 files changed, 111 insertions(+), 29 deletions(-) create mode 100644 src/database/slug.database.repository.spec.ts diff --git a/CLAUDE.md b/CLAUDE.md index 7e2d068..9ffa50a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,7 +65,7 @@ Domain notes: - Customers: an email or phone removed in Hiboutik is erased in database (`null`, data minimization). - Dates: Hiboutik sends French local time without offset; `parseHiboutikDate` reads datetimes as `Europe/Paris` whatever the server time zone, and date-only values (date of birth) as midnight UTC. - Orders come from the customer's `sales`: sales already stored as `COMPLETED` and linked to this customer are skipped (an order that lost its customer is fetched again to be linked back), the others are fetched and upserted on `hiboutikSaleId` (`source: STORE` on creation only). `status` is `COMPLETED` or `PENDING`, `orderedAt` is the completion date else the creation date. Lines are replaced on each upsert and snapshot name, version, barcode, price and Hiboutik ids; the `ProductVersion` link is optional (`SetNull`). `unitPrice` is the paid price, taxes and discount included. Quotations are skipped, anonymous sales are never synced (only customers' sales are read), and orders are never deleted. -- Slugs (SEO): `Brand`, `Category` and `Product` have a unique `slug`, generated once by `SlugService.assignMissingSlugs` at the end of their sync step and never overwritten by the sync (an admin may replace it). Generation handles the oldest `hiboutikId` first: the slugified name, else `name-` on a collision (logged as a warning), else `brand|category|product-` for a name without latin letters. `SlugService.changeSlug` keeps the former slug in `SlugRedirect` (for a 301); slugs of redirects stay reserved. `GET /sitemap` lists the public slugs (brands `enabled`, categories `webEnabled`, products `enabled` and not `archived`) for the storefront's sitemap.xml. +- Slugs (SEO): `Brand`, `Category` and `Product` have a unique `slug`, generated once by `SlugService.assignMissingSlugs` at the end of their sync step and never overwritten by the sync (an admin may replace it). Generation handles the oldest `hiboutikId` first: the slugified name, else `name-` on a collision (logged as a warning), else `brand|category|product-` for a name without latin letters. `SlugService.changeSlug` keeps the former slug in `SlugRedirect` (for a 301); slugs of redirects stay reserved (an entity may only take back its own former slug). `GET /sitemap` lists the public slugs (brands `enabled`, categories `webEnabled`, products `enabled` and not `archived`) for the storefront's sitemap.xml; only products carry a date, `hiboutikUpdatedAt` (the row's `updatedAt` changes on every sync). - Shared Prisma payload types (e.g. `ProductWithVersions`) live in `prisma/customObjects/`. ## Input validation diff --git a/postman/collections/DinggaDingga/.resources/getSitemap.resources/examples/Public pages.example.yaml b/postman/collections/DinggaDingga/.resources/getSitemap.resources/examples/Public pages.example.yaml index 8aced12..b5d4729 100644 --- a/postman/collections/DinggaDingga/.resources/getSitemap.resources/examples/Public pages.example.yaml +++ b/postman/collections/DinggaDingga/.resources/getSitemap.resources/examples/Public pages.example.yaml @@ -14,8 +14,8 @@ response: type: json content: | { - "brands": [{ "slug": "stray-kids", "updatedAt": "2026-11-08T10:00:00.000Z" }], - "categories": [{ "slug": "albums", "updatedAt": "2026-11-08T10:00:00.000Z" }], - "products": [{ "slug": "ate-digipack", "updatedAt": "2026-11-08T10:00:00.000Z" }] + "brands": [{ "slug": "stray-kids" }], + "categories": [{ "slug": "albums" }], + "products": [{ "slug": "ate-digipack", "updatedAt": "2026-10-28T14:32:00.000Z" }] } order: 1000 diff --git a/postman/collections/DinggaDingga/getSitemap.request.yaml b/postman/collections/DinggaDingga/getSitemap.request.yaml index a958226..2116542 100644 --- a/postman/collections/DinggaDingga/getSitemap.request.yaml +++ b/postman/collections/DinggaDingga/getSitemap.request.yaml @@ -1,5 +1,5 @@ $kind: http-request -description: "Public pages of the storefront (brands enabled, categories displayed on the website, products displayed on the website and not archived) that have a slug, with the date of the last write of their row. The storefront builds its sitemap.xml from it. Answered with Cache-Control: public, max-age=600." +description: "Public pages of the storefront (brands enabled, categories displayed on the website, products displayed on the website and not archived) that have a slug. Products carry `updatedAt`, their last update in Hiboutik (null when unknown); brands and categories have no date, as the sync rewrites their rows every time. The storefront builds its sitemap.xml from it. Answered with Cache-Control: public, max-age=600." url: "{{baseUrl}}/sitemap" method: GET headers: diff --git a/src/database/sitemap.database.repository.ts b/src/database/sitemap.database.repository.ts index fc3c10b..8f4add7 100644 --- a/src/database/sitemap.database.repository.ts +++ b/src/database/sitemap.database.repository.ts @@ -1,16 +1,20 @@ import { Injectable } from '@nestjs/common'; import { PrismaService } from '../../prisma/prisma.service'; -/** Public page of the storefront: its slug and the last change of its content. */ +/** Public page of the storefront, identified by its slug. */ export interface SitemapEntry { slug: string; - updatedAt: Date; +} + +/** Public product page, with the last change of its content in Hiboutik (null when Hiboutik does not give it). */ +export interface ProductSitemapEntry extends SitemapEntry { + updatedAt: Date | null; } export interface Sitemap { brands: SitemapEntry[]; categories: SitemapEntry[]; - products: SitemapEntry[]; + products: ProductSitemapEntry[]; } /** Reads the public pages of the storefront, for its sitemap. */ @@ -20,26 +24,30 @@ export class SitemapDatabaseRepository { /** * Brands enabled, categories displayed on the website and products displayed on the website and not archived, that - * have a slug. Dates are the last write of the row: the sync rewrites every row, so it is the date of the last sync. + * have a slug. Only products carry a date, their last update in Hiboutik: the `updatedAt` of the rows is rewritten by + * every sync, so it would give every page the date of the last sync, which search engines learn to ignore. */ async getSitemap(): Promise { - const select = { slug: true, updatedAt: true } as const; const orderBy = { slug: 'asc' } as const; const hasSlug = { slug: { not: null } }; const [brands, categories, products] = await Promise.all([ - this.prisma.brand.findMany({ where: { ...hasSlug, enabled: true }, select, orderBy }), - this.prisma.category.findMany({ where: { ...hasSlug, webEnabled: true }, select, orderBy }), - this.prisma.product.findMany({ where: { ...hasSlug, enabled: true, archived: false }, select, orderBy }), + this.prisma.brand.findMany({ where: { ...hasSlug, enabled: true }, select: { slug: true }, orderBy }), + this.prisma.category.findMany({ where: { ...hasSlug, webEnabled: true }, select: { slug: true }, orderBy }), + this.prisma.product.findMany({ + where: { ...hasSlug, enabled: true, archived: false }, + select: { slug: true, hiboutikUpdatedAt: true }, + orderBy, + }), ]); return { - brands: brands.map(toEntry), - categories: categories.map(toEntry), - products: products.map(toEntry), + brands: brands.map((row) => ({ slug: toSlug(row.slug) })), + categories: categories.map((row) => ({ slug: toSlug(row.slug) })), + products: products.map((row) => ({ slug: toSlug(row.slug), updatedAt: row.hiboutikUpdatedAt })), }; } } /** The query filters out null slugs; the type does not know it. */ -function toEntry(row: { slug: string | null; updatedAt: Date }): SitemapEntry { - return { slug: row.slug as string, updatedAt: row.updatedAt }; +function toSlug(slug: string | null): string { + return slug as string; } diff --git a/src/database/slug.database.repository.spec.ts b/src/database/slug.database.repository.spec.ts new file mode 100644 index 0000000..7bf408e --- /dev/null +++ b/src/database/slug.database.repository.spec.ts @@ -0,0 +1,57 @@ +import { SlugEntityType } from '../../prisma/generated/client'; +import { SlugDatabaseRepository } from './slug.database.repository'; + +describe('SlugDatabaseRepository.changeSlug', () => { + const tx = { + product: { findUnique: jest.fn(), update: jest.fn() }, + slugRedirect: { findUnique: jest.fn(), delete: jest.fn(), create: jest.fn() }, + }; + const prisma = { $transaction: jest.fn((fn: (client: typeof tx) => unknown) => fn(tx)) }; + let repository: SlugDatabaseRepository; + + beforeEach(() => { + jest.clearAllMocks(); + repository = new SlugDatabaseRepository(prisma as never); + // product-a currently named "ate"; no product currently named with the new slug + tx.product.findUnique.mockImplementation(({ where }: { where: { id?: string } }) => + Promise.resolve(where.id === 'product-a' ? { slug: 'ate' } : null), + ); + tx.slugRedirect.findUnique.mockResolvedValue(null); + }); + + it('keeps the former slug as a redirect', async () => { + await expect(repository.changeSlug(SlugEntityType.PRODUCT, 'product-a', 'ate-digipack')).resolves.toBe('CHANGED'); + + expect(tx.slugRedirect.delete).not.toHaveBeenCalled(); + expect(tx.slugRedirect.create).toHaveBeenCalledWith({ data: { entityType: 'PRODUCT', slug: 'ate', productId: 'product-a' } }); + expect(tx.product.update).toHaveBeenCalledWith({ where: { id: 'product-a' }, data: { slug: 'ate-digipack' } }); + }); + + it('lets an entity take back one of its own former slugs', async () => { + tx.slugRedirect.findUnique.mockResolvedValue({ id: 'redirect-1', brandId: null, categoryId: null, productId: 'product-a' }); + + await expect(repository.changeSlug(SlugEntityType.PRODUCT, 'product-a', 'ate-old')).resolves.toBe('CHANGED'); + + expect(tx.slugRedirect.delete).toHaveBeenCalledWith({ where: { id: 'redirect-1' } }); + expect(tx.product.update).toHaveBeenCalledWith({ where: { id: 'product-a' }, data: { slug: 'ate-old' } }); + }); + + it("refuses another entity's former slug: its old URL keeps redirecting to it", async () => { + tx.slugRedirect.findUnique.mockResolvedValue({ id: 'redirect-1', brandId: null, categoryId: null, productId: 'product-b' }); + + await expect(repository.changeSlug(SlugEntityType.PRODUCT, 'product-a', 'maxident')).resolves.toBe('TAKEN'); + + expect(tx.slugRedirect.delete).not.toHaveBeenCalled(); + expect(tx.slugRedirect.create).not.toHaveBeenCalled(); + expect(tx.product.update).not.toHaveBeenCalled(); + }); + + it("refuses another entity's current slug", async () => { + tx.product.findUnique.mockImplementation(({ where }: { where: { id?: string; slug?: string } }) => + Promise.resolve(where.id === 'product-a' ? { slug: 'ate' } : where.slug === 'maxident' ? { slug: 'maxident' } : null), + ); + + await expect(repository.changeSlug(SlugEntityType.PRODUCT, 'product-a', 'maxident')).resolves.toBe('TAKEN'); + expect(tx.product.update).not.toHaveBeenCalled(); + }); +}); diff --git a/src/database/slug.database.repository.ts b/src/database/slug.database.repository.ts index 0aeb295..4c70bba 100644 --- a/src/database/slug.database.repository.ts +++ b/src/database/slug.database.repository.ts @@ -62,8 +62,9 @@ export class SlugDatabaseRepository { } /** - * Replaces the slug of an entity, in one transaction: its former slug becomes a redirect to it, and a redirect that - * held the new slug (one of its own former slugs, or another entity's) is dropped, the slug now naming this entity. + * Replaces the slug of an entity, in one transaction: its former slug becomes a redirect to it. + * The new slug may be one of its own former slugs (that redirect is dropped, the slug naming it again), never another + * entity's former slug: that URL keeps redirecting to its entity (`TAKEN`). * A concurrent write of the same slug makes the transaction fail on the unique constraint (Prisma error P2002). */ async changeSlug(type: SlugEntityType, id: string, newSlug: string): Promise { @@ -75,7 +76,14 @@ export class SlugDatabaseRepository { const owner = await this.findSlugOwner(tx, type, { slug: newSlug }); if (owner) return 'TAKEN'; - await tx.slugRedirect.deleteMany({ where: { entityType: type, slug: newSlug } }); + const redirect = await tx.slugRedirect.findUnique({ + where: { entityType_slug: { entityType: type, slug: newSlug } }, + select: { id: true, brandId: true, categoryId: true, productId: true }, + }); + if (redirect) { + if ((redirect.brandId ?? redirect.categoryId ?? redirect.productId) !== id) return 'TAKEN'; + await tx.slugRedirect.delete({ where: { id: redirect.id } }); + } if (entity.slug) { await tx.slugRedirect.create({ data: { entityType: type, slug: entity.slug, ...this.redirectTarget(type, id) } }); } diff --git a/test/app.e2e-spec.ts b/test/app.e2e-spec.ts index 687da1c..b7408a6 100644 --- a/test/app.e2e-spec.ts +++ b/test/app.e2e-spec.ts @@ -54,24 +54,33 @@ describe('AppController (e2e)', () => { }); it('GET /sitemap lists the public pages', async () => { - prisma.brand.findMany.mockResolvedValue([{ slug: 'bts', updatedAt: new Date('2026-11-01T10:00:00Z') }]); - prisma.category.findMany.mockResolvedValue([{ slug: 'albums', updatedAt: new Date('2026-10-01T10:00:00Z') }]); - prisma.product.findMany.mockResolvedValue([{ slug: 'bts-proof', updatedAt: new Date('2026-11-05T08:00:00Z') }]); + prisma.brand.findMany.mockResolvedValue([{ slug: 'bts' }]); + prisma.category.findMany.mockResolvedValue([{ slug: 'albums' }]); + prisma.product.findMany.mockResolvedValue([ + { slug: 'bts-proof', hiboutikUpdatedAt: new Date('2026-11-05T08:00:00Z') }, + { slug: 'bts-wings', hiboutikUpdatedAt: null }, + ]); await request(app.getHttpServer()) .get('/sitemap') .expect(200) .expect('Cache-Control', 'public, max-age=600') .expect({ - brands: [{ slug: 'bts', updatedAt: '2026-11-01T10:00:00.000Z' }], - categories: [{ slug: 'albums', updatedAt: '2026-10-01T10:00:00.000Z' }], - products: [{ slug: 'bts-proof', updatedAt: '2026-11-05T08:00:00.000Z' }], + brands: [{ slug: 'bts' }], + categories: [{ slug: 'albums' }], + products: [ + { slug: 'bts-proof', updatedAt: '2026-11-05T08:00:00.000Z' }, + { slug: 'bts-wings', updatedAt: null }, + ], }); - // only public rows with a slug are read + // only public rows with a slug are read; products are dated by Hiboutik, not by the last sync expect(prisma.brand.findMany).toHaveBeenCalledWith(expect.objectContaining({ where: { slug: { not: null }, enabled: true } })); expect(prisma.category.findMany).toHaveBeenCalledWith(expect.objectContaining({ where: { slug: { not: null }, webEnabled: true } })); expect(prisma.product.findMany).toHaveBeenCalledWith( - expect.objectContaining({ where: { slug: { not: null }, enabled: true, archived: false } }), + expect.objectContaining({ + where: { slug: { not: null }, enabled: true, archived: false }, + select: { slug: true, hiboutikUpdatedAt: true }, + }), ); });