diff --git a/.changeset/plain-states-reply.md b/.changeset/plain-states-reply.md new file mode 100644 index 00000000..216722ca --- /dev/null +++ b/.changeset/plain-states-reply.md @@ -0,0 +1,6 @@ +--- +"strapi-plugin-webtools": minor +"docs": patch +--- + +feat: unique aliases per locale diff --git a/jest.config.js b/jest.config.js index bc93dae1..fcb63534 100644 --- a/jest.config.js +++ b/jest.config.js @@ -11,4 +11,5 @@ module.exports = { transform: { '^.+\\.(ts|tsx|js|jsx)$': 'ts-jest', }, + testTimeout: 30_000, }; diff --git a/packages/addons/sitemap/server/utils/enabledContentTypes.ts b/packages/addons/sitemap/server/utils/enabledContentTypes.ts index 402194fa..3428bb5e 100644 --- a/packages/addons/sitemap/server/utils/enabledContentTypes.ts +++ b/packages/addons/sitemap/server/utils/enabledContentTypes.ts @@ -1,20 +1,8 @@ import get from 'lodash/get'; -import { UID, Schema } from '@strapi/strapi'; +import { Schema } from '@strapi/strapi'; -export const isContentTypeEnabled = (ct: UID.ContentType) => { - let contentType: Schema.ContentType; - - if (typeof ct === 'string') { - contentType = strapi.contentTypes[ct]; - } else { - contentType = ct; - } - +export const isContentTypeEnabled = (contentType: Schema.ContentType) => { const { pluginOptions } = contentType; - const enabled = get(pluginOptions, ['webtools', 'enabled'], false) as boolean; - - if (!enabled) return false; - - return true; + return get(pluginOptions, ['webtools', 'enabled'], false) as boolean; }; diff --git a/packages/core/server/config.ts b/packages/core/server/config.ts index 922cd5c9..f98761aa 100644 --- a/packages/core/server/config.ts +++ b/packages/core/server/config.ts @@ -5,7 +5,8 @@ import kebabCase from 'lodash/kebabCase'; export interface Config { website_url: string; default_pattern: string, - slugify: (fieldValue: string) => string + unique_per_locale: boolean, + slugify: (fieldValue: string) => string, } const config: { @@ -16,6 +17,7 @@ const config: { website_url: null, default_pattern: '/[pluralName]/[documentId]', slugify: (fieldValue) => kebabCase(deburr(toLower(fieldValue))), + unique_per_locale: false, }, validator() {}, }; diff --git a/packages/core/server/content-types/url-alias/schema.json b/packages/core/server/content-types/url-alias/schema.json index 0c7b3fe4..e622c4f0 100644 --- a/packages/core/server/content-types/url-alias/schema.json +++ b/packages/core/server/content-types/url-alias/schema.json @@ -25,7 +25,6 @@ "url_path": { "type": "string", "required": true, - "unique": true, "pluginOptions": { "i18n": { "localized": true diff --git a/packages/core/server/middlewares/__tests__/middlewares.test.js b/packages/core/server/middlewares/__tests__/middlewares.test.js index 2e69c9ae..ffdb9ef1 100644 --- a/packages/core/server/middlewares/__tests__/middlewares.test.js +++ b/packages/core/server/middlewares/__tests__/middlewares.test.js @@ -49,6 +49,100 @@ describe('Query layer decorator', () => { expect(page).toHaveProperty('url_alias[0].contenttype', 'api::test.test'); }); + it('Create - Should generate a unique URL alias for duplicate source content', async () => { + const page1 = await strapi.documents('api::test.test').create({ + data: { + title: 'Some amazing new page duplicate', + }, + populate: ['url_alias'], + }); + const page2 = await strapi.documents('api::test.test').create({ + data: { + title: 'Some amazing new page duplicate', + }, + populate: ['url_alias'], + }); + + expect(page1).toHaveProperty('url_alias[0].url_path', '/page/some-amazing-new-page-duplicate'); + expect(page2).toHaveProperty('url_alias[0].url_path', '/page/some-amazing-new-page-duplicate-0'); + }); + + it('Create - Should generate a unique URL alias for duplicate source content across locales', async () => { + const page1 = await strapi.documents('api::test.test').create({ + data: { + title: 'Some amazing new localized page', + }, + locale: 'en', + populate: ['url_alias'], + }); + const page2 = await strapi.documents('api::test.test').create({ + data: { + title: 'Some amazing new localized page', + }, + locale: 'nl', + populate: ['url_alias'], + }); + + expect(page1).toHaveProperty('locale', 'en'); + expect(page2).toHaveProperty('locale', 'nl'); + expect(page1).toHaveProperty('url_alias[0].url_path', '/page/some-amazing-new-localized-page'); + expect(page2).toHaveProperty('url_alias[0].url_path', '/page/some-amazing-new-localized-page-0'); + }); + + it('Create - Should generate a unique URL alias for duplicate source content across translated content', async () => { + const english_page = await strapi.documents('api::test.test').create({ + data: { + title: 'Some amazing new translated page', + }, + locale: 'en', + populate: ['url_alias'], + }); + const dutch_page = await strapi.documents('api::test.test').update({ + documentId: english_page.documentId, + data: { + title: 'Some amazing new translated page', + }, + locale: 'nl', + populate: ['url_alias'], + }); + + expect(english_page).toHaveProperty('locale', 'en'); + expect(dutch_page).toHaveProperty('locale', 'nl'); + expect(dutch_page).toHaveProperty('documentId', english_page.documentId); + expect(english_page).toHaveProperty('url_alias[0].url_path', '/page/some-amazing-new-translated-page'); + expect(dutch_page).toHaveProperty('url_alias[0].url_path', '/page/some-amazing-new-translated-page-0'); + }); + + it('Create - Should allow duplicate URL alias for duplicate source content within locale if configured', async () => { + try { + strapi.config.set('plugin::webtools.unique_per_locale', true); + + const page1 = await strapi.documents('api::test.test') + .create({ + data: { + title: 'Some amazing new localized unique page', + }, + locale: 'en', + populate: ['url_alias'], + }); + const page2 = await strapi.documents('api::test.test') + .create({ + data: { + title: 'Some amazing new localized unique page', + }, + locale: 'nl', + populate: ['url_alias'], + }); + + expect(page1) + .toHaveProperty('url_alias[0].url_path', '/page/some-amazing-new-localized-unique-page'); + expect(page2) + .toHaveProperty('url_alias[0].url_path', '/page/some-amazing-new-localized-unique-page'); + } finally { + strapi.config.get('plugin::webtools.unique_per_locale', false); + } + }); + it('Create - Should re-generate a pre-created URL alias if generated is set to true', async () => { const alias = await strapi.documents('plugin::webtools.url-alias').create({ data: { diff --git a/packages/core/server/middlewares/generate-url-alias.ts b/packages/core/server/middlewares/generate-url-alias.ts index 8c1e38bc..9df9e9aa 100644 --- a/packages/core/server/middlewares/generate-url-alias.ts +++ b/packages/core/server/middlewares/generate-url-alias.ts @@ -64,7 +64,7 @@ const generateUrlAliasMiddleware: Modules.Documents.Middleware.Middleware = asyn }, }); - // If the document already has an URL alias, fetch it. + // If the document already has a URL alias, fetch it. if (params.data.url_alias?.[0]) { urlAliasEntity = await strapi.documents('plugin::webtools.url-alias').findOne({ ...(params.locale ? { locale: params.locale } : {}), diff --git a/packages/core/server/middlewares/prevent-duplicate-urls.ts b/packages/core/server/middlewares/prevent-duplicate-urls.ts index 3158c2d3..b6c005ff 100644 --- a/packages/core/server/middlewares/prevent-duplicate-urls.ts +++ b/packages/core/server/middlewares/prevent-duplicate-urls.ts @@ -18,15 +18,8 @@ const preventDuplicateUrlsMiddleware: Modules.Documents.Middleware.Middleware = const params = context.params as Modules.Documents.ServiceParams<'plugin::webtools.url-alias'>['create' | 'update' | 'clone'] & { documentId: string }; if (params.data.url_path) { - const excludeFilters: { [key: string]: any }[] = []; - - excludeFilters.push({ documentId: params.documentId }); - - if (params.locale) { - excludeFilters.push({ locale: params.locale }); - } - - params.data.url_path = await getPluginService('url-alias').makeUniquePath(params.data.url_path, action !== 'clone' && excludeFilters); + params.data.url_path = await getPluginService('url-alias') + .makeUniquePath(params.data.url_path, action !== 'clone' && params.documentId, action !== 'clone' && params.locale); } return next(); diff --git a/packages/core/server/services/url-alias.ts b/packages/core/server/services/url-alias.ts index 5b98c2e2..22f63b56 100644 --- a/packages/core/server/services/url-alias.ts +++ b/packages/core/server/services/url-alias.ts @@ -44,8 +44,8 @@ const customServices = () => ({ /** * findByPath. * - * @param {string} path the path. - * @param {number} id the id to ignore. + * @param path the path. + * @param excludeFilters the id to ignore. */ findByPath: async (path: string, excludeFilters: { [key: string]: any }[] = [{}]) => { const locales = await strapi.documents('plugin::i18n.locale').findMany({ fields: 'code' }); @@ -72,18 +72,56 @@ const customServices = () => ({ /** * Finds a path from the original path that is unique + * + * @param originalPath The path as generated from the pattern and document + * @param currentDocumentId If generating for an existing document, its document id + * @param currentLocale If generating for an existing document, its locale code */ makeUniquePath: async ( originalPath: string, - excludeFilters?: { [key: string]: any }[], - ext: number = -1, + currentDocumentId?: string, + currentLocale?: string, ): Promise => { - const extension = ext >= 0 ? `-${ext}` : ''; - const newPath = originalPath + extension; - const pathAlreadyExists = await getPluginService('url-alias').findByPath(newPath, excludeFilters); + const uniquePerLocale = strapi.config.get('plugin::webtools.unique_per_locale', false); + let newPath = originalPath; + + // FIXME: limit number of iterations to prevent overloading the server? + for (let iteration = -1; ; ++iteration) { + if (iteration >= 0) { + newPath = `${originalPath}-${iteration}`; + } + + const filters: { $and: unknown[] } = { + $and: [ + { url_path: newPath }, + ], + }; + + if (currentDocumentId) { + filters.$and.push({ + $not: { + $and: [ + { + documentId: { $eq: currentDocumentId }, + }, + { + locale: { $eq: currentLocale }, + }, + ], + }, + }); + } + + // This loop can't be parallelized as the iteration is increased between each step. + // eslint-disable-next-line no-await-in-loop + const existingPathEntity = await strapi.documents('plugin::webtools.url-alias').findFirst({ + locale: uniquePerLocale ? currentLocale : undefined, + filters, + }); - if (pathAlreadyExists) { - return getPluginService('url-alias').makeUniquePath(originalPath, excludeFilters, ext + 1); + if (!existingPathEntity) { + break; + } } return newPath; diff --git a/packages/core/server/util/enabledContentTypes.ts b/packages/core/server/util/enabledContentTypes.ts index c0ad9220..0a78e8ed 100644 --- a/packages/core/server/util/enabledContentTypes.ts +++ b/packages/core/server/util/enabledContentTypes.ts @@ -13,9 +13,6 @@ export const isContentTypeEnabled = (ct: Schema.ContentType) => { } const { pluginOptions } = contentType; - const enabled = get(pluginOptions, [pluginId, 'enabled'], false) as boolean; - if (!enabled) return false; - - return true; + return get(pluginOptions, [pluginId, 'enabled'], false) as boolean; }; diff --git a/packages/docs/docs/configuration/unique-per-locale.md b/packages/docs/docs/configuration/unique-per-locale.md new file mode 100644 index 00000000..2d68922e --- /dev/null +++ b/packages/docs/docs/configuration/unique-per-locale.md @@ -0,0 +1,19 @@ +--- +sidebar_label: 'Unique alias per locale' +displayed_sidebar: webtoolsSidebar +slug: /configuration/unique-per-locale +--- + +# Unique URL alias per locale + +By default Webtools generates URL aliases that are unique within entire Strapi database. +When you use a separate domain name per locale this prevents reusing the same alias between locales. + +Set `unique_per_locale` to `true` to allow Webtools to generate the same alias as long as the locale is different. + +| Name | Details | +| ---- |---------------------| +| Key | `unique_per_locale` | +| Required | false | +| Type | boolean | +| Default | false | diff --git a/playground/src/index.ts b/playground/src/index.ts index 54e5675e..9e4b13f1 100644 --- a/playground/src/index.ts +++ b/playground/src/index.ts @@ -19,6 +19,9 @@ export default { async bootstrap({ strapi }: { strapi: Core.Strapi }) { // Seed the database with some test data for the integration tests. if (process.env.NODE_ENV === 'test') { + // Hide repeated startup messages while running tests. + strapi.config.set('server.logger.startup.enabled', false); + // Give the public role some permissions to test with const roles = await strapi .service('plugin::users-permissions.role') @@ -63,13 +66,20 @@ export default { .updateRole(publicRole.id, publicRole); } + await strapi.documents('plugin::i18n.locale').create({ + data: { + code: 'nl', + name: 'Dutch (nl)', + }, + }); + await strapi.documents('plugin::webtools.url-pattern').create({ data: { pattern: '/page/[title]', label: 'Test API pattern', code: 'test_api_pattern', contenttype: 'api::test.test', - languages: ['en'], + languages: ['en', 'nl'], }, }); diff --git a/playground/tests/healthcheck.test.js b/playground/tests/healthcheck.test.js index c7ebdff3..63554364 100644 --- a/playground/tests/healthcheck.test.js +++ b/playground/tests/healthcheck.test.js @@ -1,4 +1,4 @@ -const { setupStrapi, stopStrapi } = require("./helpers"); +const { setupStrapi, stopStrapi } = require('./helpers'); jest.setTimeout(50000); @@ -10,8 +10,8 @@ afterAll(async () => { await stopStrapi(); }); -describe("Strapi is defined", () => { - it("just works", () => { +describe('Strapi is defined', () => { + it('just works', () => { expect(strapi).toBeDefined(); }); }); diff --git a/playground/tests/helpers.ts b/playground/tests/helpers.ts index 890af7e6..71663977 100644 --- a/playground/tests/helpers.ts +++ b/playground/tests/helpers.ts @@ -14,7 +14,8 @@ export async function setupStrapi() { appDir: './playground', distDir: './playground/dist', }).load(); - strapi.server.mount(); + + await strapi.start(); instance = strapi; // strapi is global now } @@ -31,8 +32,6 @@ export async function stopStrapi() { assert(typeof tmpDbFile === 'string'); - instance.server.httpServer.close(); - await instance.db.connection.destroy(); await instance.destroy(); if (fs.existsSync(tmpDbFile)) { diff --git a/playground/types/generated/contentTypes.d.ts b/playground/types/generated/contentTypes.d.ts index 6a99026e..6370c4f6 100644 --- a/playground/types/generated/contentTypes.d.ts +++ b/playground/types/generated/contentTypes.d.ts @@ -1139,7 +1139,6 @@ export interface PluginWebtoolsUrlAlias extends Struct.CollectionTypeSchema { Schema.Attribute.Private; url_path: Schema.Attribute.String & Schema.Attribute.Required & - Schema.Attribute.Unique & Schema.Attribute.SetPluginOptions<{ i18n: { localized: true;