fix(seo): keep whole words when truncating slugs, detect unique violations as Prisma errors
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
import { BadRequestException, ConflictException, Logger, NotFoundException } from '@nestjs/common';
|
import { BadRequestException, ConflictException, Logger, NotFoundException } from '@nestjs/common';
|
||||||
import { SlugService } from './slug.service';
|
import { SlugService } from './slug.service';
|
||||||
import { SlugEntityType } from '../../../prisma/generated/client';
|
import { Prisma, SlugEntityType } from '../../../prisma/generated/client';
|
||||||
import { SlugCandidateRow } from '../../database/slug.database.repository';
|
import { SlugCandidateRow } from '../../database/slug.database.repository';
|
||||||
|
|
||||||
const row = (hiboutikId: number, name: string): SlugCandidateRow => ({ id: `uuid-${hiboutikId}`, hiboutikId, name });
|
const row = (hiboutikId: number, name: string): SlugCandidateRow => ({ id: `uuid-${hiboutikId}`, hiboutikId, name });
|
||||||
@@ -93,10 +93,22 @@ describe('SlugService', () => {
|
|||||||
slugDatabaseRepository.changeSlug.mockResolvedValueOnce('TAKEN');
|
slugDatabaseRepository.changeSlug.mockResolvedValueOnce('TAKEN');
|
||||||
await expect(service.changeSlug(SlugEntityType.BRAND, 'uuid-1', 'bts')).rejects.toThrow(ConflictException);
|
await expect(service.changeSlug(SlugEntityType.BRAND, 'uuid-1', 'bts')).rejects.toThrow(ConflictException);
|
||||||
|
|
||||||
slugDatabaseRepository.changeSlug.mockRejectedValueOnce(Object.assign(new Error('Unique constraint failed'), { code: 'P2002' }));
|
slugDatabaseRepository.changeSlug.mockRejectedValueOnce(
|
||||||
|
new Prisma.PrismaClientKnownRequestError('Unique constraint failed', { code: 'P2002', clientVersion: 'test' }),
|
||||||
|
);
|
||||||
await expect(service.changeSlug(SlugEntityType.BRAND, 'uuid-1', 'bts')).rejects.toThrow(ConflictException);
|
await expect(service.changeSlug(SlugEntityType.BRAND, 'uuid-1', 'bts')).rejects.toThrow(ConflictException);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('rethrows any other error, even one carrying a P2002 code without being a Prisma error', async () => {
|
||||||
|
const notPrisma = Object.assign(new Error('boom'), { code: 'P2002' });
|
||||||
|
slugDatabaseRepository.changeSlug.mockRejectedValueOnce(notPrisma);
|
||||||
|
await expect(service.changeSlug(SlugEntityType.BRAND, 'uuid-1', 'bts')).rejects.toBe(notPrisma);
|
||||||
|
|
||||||
|
const otherPrismaError = new Prisma.PrismaClientKnownRequestError('Record not found', { code: 'P2025', clientVersion: 'test' });
|
||||||
|
slugDatabaseRepository.changeSlug.mockRejectedValueOnce(otherPrismaError);
|
||||||
|
await expect(service.changeSlug(SlugEntityType.BRAND, 'uuid-1', 'bts')).rejects.toBe(otherPrismaError);
|
||||||
|
});
|
||||||
|
|
||||||
it('accepts a valid new slug', async () => {
|
it('accepts a valid new slug', async () => {
|
||||||
slugDatabaseRepository.changeSlug.mockResolvedValue('CHANGED');
|
slugDatabaseRepository.changeSlug.mockResolvedValue('CHANGED');
|
||||||
|
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { BadRequestException, ConflictException, Injectable, Logger, NotFoundException } from '@nestjs/common';
|
import { BadRequestException, ConflictException, Injectable, Logger, NotFoundException } from '@nestjs/common';
|
||||||
import { SlugEntityType } from '../../../prisma/generated/client';
|
import { Prisma, SlugEntityType } from '../../../prisma/generated/client';
|
||||||
import { SlugCandidateRow, SlugDatabaseRepository } from '../../database/slug.database.repository';
|
import { SlugCandidateRow, SlugDatabaseRepository } from '../../database/slug.database.repository';
|
||||||
import { forEachSafely } from '../../utils/sync';
|
import { forEachSafely } from '../../utils/sync';
|
||||||
import { MAX_SLUG_LENGTH, pickAvailableSlug, SLUG_PATTERN, slugify, withSlugSuffix } from '../../utils/slug';
|
import { MAX_SLUG_LENGTH, pickAvailableSlug, SLUG_PATTERN, slugify, withSlugSuffix } from '../../utils/slug';
|
||||||
@@ -17,6 +17,9 @@ const FALLBACK_PREFIX: Record<SlugEntityType, string> = {
|
|||||||
* A slug is generated once, when an entity has none, and is never regenerated: a URL must not change when the name
|
* A slug is generated once, when an entity has none, and is never regenerated: a URL must not change when the name
|
||||||
* changes in Hiboutik. Generation is deterministic: entities are handled oldest first (by `hiboutikId`), so that a
|
* changes in Hiboutik. Generation is deterministic: entities are handled oldest first (by `hiboutikId`), so that a
|
||||||
* database rebuilt from scratch gets the same slugs, the oldest of two homonyms keeping the plain slug.
|
* database rebuilt from scratch gets the same slugs, the oldest of two homonyms keeping the plain slug.
|
||||||
|
* A rebuild only gives back the stored slugs as long as their history did not change them: a slug replaced by an admin,
|
||||||
|
* an entity renamed in Hiboutik after its slug was generated, or a homonym deleted since (the younger one, suffixed,
|
||||||
|
* would now get the plain slug) all give a different slug on a rebuild, and the redirects are lost with the database.
|
||||||
* An admin can replace a slug ({@link changeSlug}): the former one is kept as a redirect.
|
* An admin can replace a slug ({@link changeSlug}): the former one is kept as a redirect.
|
||||||
*/
|
*/
|
||||||
@Injectable()
|
@Injectable()
|
||||||
@@ -83,7 +86,7 @@ export class SlugService {
|
|||||||
result = await this.slugDatabaseRepository.changeSlug(type, id, slug);
|
result = await this.slugDatabaseRepository.changeSlug(type, id, slug);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
// Unique constraint: the same slug was written at the same time by someone else
|
// Unique constraint: the same slug was written at the same time by someone else
|
||||||
if ((error as { code?: string })?.code === 'P2002') result = 'TAKEN';
|
if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2002') result = 'TAKEN';
|
||||||
else throw error;
|
else throw error;
|
||||||
}
|
}
|
||||||
if (result === 'NOT_FOUND') throw new NotFoundException();
|
if (result === 'NOT_FOUND') throw new NotFoundException();
|
||||||
|
|||||||
@@ -21,6 +21,12 @@ describe('slugify', () => {
|
|||||||
expect(slug.length).toBeLessThanOrEqual(MAX_SLUG_LENGTH);
|
expect(slug.length).toBeLessThanOrEqual(MAX_SLUG_LENGTH);
|
||||||
expect(slug).toMatch(SLUG_PATTERN);
|
expect(slug).toMatch(SLUG_PATTERN);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('keeps the last word when the cut falls right before a hyphen', () => {
|
||||||
|
expect(slugify('aa bbb ccc', 6)).toBe('aa-bbb');
|
||||||
|
expect(slugify('aa bbb ccc', 7)).toBe('aa-bbb');
|
||||||
|
expect(slugify('aa bbb ccc', 5)).toBe('aa');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('withSlugSuffix', () => {
|
describe('withSlugSuffix', () => {
|
||||||
|
|||||||
@@ -15,7 +15,7 @@ const SPECIAL_LETTERS: Record<string, string> = { æ: 'ae', œ: 'oe', ø: 'o',
|
|||||||
export function slugify(name: string, maxLength = MAX_SLUG_LENGTH): string {
|
export function slugify(name: string, maxLength = MAX_SLUG_LENGTH): string {
|
||||||
const slug = name
|
const slug = name
|
||||||
.normalize('NFKD')
|
.normalize('NFKD')
|
||||||
.replace(/[̀-ͯ]/g, '')
|
.replace(/[\u0300-\u036f]/g, '')
|
||||||
.toLowerCase()
|
.toLowerCase()
|
||||||
.replace(/[æœøßđłþ]/g, (letter) => SPECIAL_LETTERS[letter])
|
.replace(/[æœøßđłþ]/g, (letter) => SPECIAL_LETTERS[letter])
|
||||||
.replace(/&/g, ' and ')
|
.replace(/&/g, ' and ')
|
||||||
@@ -24,10 +24,14 @@ export function slugify(name: string, maxLength = MAX_SLUG_LENGTH): string {
|
|||||||
return truncateSlug(slug, maxLength);
|
return truncateSlug(slug, maxLength);
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Cuts a slug to `maxLength`, at the last hyphen when there is one, never leaving a trailing hyphen. */
|
/**
|
||||||
|
* Cuts a slug to `maxLength` at a word boundary: keeps the cut as is when it ends on a whole word, else cuts at its last
|
||||||
|
* hyphen when there is one. Never leaves a trailing hyphen.
|
||||||
|
*/
|
||||||
function truncateSlug(slug: string, maxLength: number): string {
|
function truncateSlug(slug: string, maxLength: number): string {
|
||||||
if (slug.length <= maxLength) return slug;
|
if (slug.length <= maxLength) return slug;
|
||||||
const cut = slug.slice(0, maxLength);
|
const cut = slug.slice(0, maxLength);
|
||||||
|
if (slug[maxLength] === '-') return cut;
|
||||||
const lastHyphen = cut.lastIndexOf('-');
|
const lastHyphen = cut.lastIndexOf('-');
|
||||||
return (lastHyphen > 0 ? cut.slice(0, lastHyphen) : cut).replace(/-+$/, '');
|
return (lastHyphen > 0 ? cut.slice(0, lastHyphen) : cut).replace(/-+$/, '');
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user