Merge pull request #119 from Oak-Digital/fix/finds-duplicate-of-self

fix: duplicateCheck will use an ignore id

Author: Alexnortung

.changeset/dry-jobs-joke.md

@@ -0,0 +1,5 @@
+---
+"@pluginpal/webtools-core": patch
+---
+
+Duplicate check will use an ignore id so that it does not update the url path because it is a duplicate of itself

packages/core/server/admin-api/__tests__/query-layer-decorator.test.ts

@@ -155,6 +155,26 @@ describe('Query layer decorator', () => {
     expect(updatedPage).toHaveProperty('url_alias.generated', false);
   });
 
+  it('Update - Should not duplicate check the same entry when updated', async () => {
+    const page = await strapi.entityService.create("api::test.test", {
+      data: {
+        title: 'Unpublished page',
+      },
+      populate: ['url_alias']
+    });
+    const url = page.url_alias.url_path;
+
+    const updatedPage = await strapi.entityService.update("api::test.test", page.id, {
+      data: {
+        // @ts-ignore
+        published_at: new Date(),
+      },
+      populate: ['url_alias']
+    });
+
+    expect(updatedPage).toHaveProperty('url_alias.url_path', url);
+  });
+
   it('Delete - Should delete the corresponding URL alias as wel', async () => {
     const page = await strapi.entityService.create("api::test.test", {
       data: {

packages/core/server/admin-api/services/url-alias.ts

@@ -1,18 +1,22 @@
 
 
-import { EntityService } from '@strapi/types';
+import { EntityService, Entity } from '@strapi/types';
 import { getPluginService } from '../../util/getPluginService';
 
 /**
  * Finds a path from the original path that is unique
  */
-const duplicateCheck = async (originalPath: string, ext = -1): Promise<string> => {
+const duplicateCheck = async (
+  originalPath: string,
+  ignoreId?: Entity.ID,
+  ext: number = -1,
+): Promise<string> => {
   const extension = ext >= 0 ? `-${ext}` : '';
   const newPath = originalPath + extension;
-  const pathAlreadyExists = await getPluginService('urlAliasService').findByPath(newPath);
+  const pathAlreadyExists = await getPluginService('urlAliasService').findByPath(newPath, ignoreId);
 
   if (pathAlreadyExists) {
-    return duplicateCheck(originalPath, ext + 1);
+    return duplicateCheck(originalPath, ignoreId, ext + 1);
   }
 
   return newPath;
@@ -85,7 +89,7 @@ const findMany = async (showDrafts: boolean = false, query: EntityService.Params
  * @param {string} path the path.
  * @param {number} id the id to ignore.
  */
-const findByPath = async (path: string, id: number | string = 0) => {
+const findByPath = async (path: string, id: Entity.ID = 0) => {
   const pathEntity = await strapi.entityService.findMany('plugin::webtools.url-alias', {
     filters: {
       url_path: path,
@@ -106,8 +110,8 @@ const findByPath = async (path: string, id: number | string = 0) => {
  * @param {object} data the data.
  * @returns {void}
  */
-const update = async (id: number | string, data: EntityService.Params.Pick<'plugin::webtools.url-alias', 'data'>['data']) => {
-  const urlPath = await duplicateCheck(data.url_path);
+const update = async (id: Entity.ID, data: EntityService.Params.Pick<'plugin::webtools.url-alias', 'data'>['data']) => {
+  const urlPath = await duplicateCheck(data.url_path, id);
 
   const pathEntity = await strapi.entityService.update('plugin::webtools.url-alias', id, {
     data: {

playground/config/env/test/database.ts

@@ -0,0 +1,17 @@
+import path from 'path';
+
+export default ({ env }) => ({
+  connection: {
+    client: "sqlite",
+    connection: {
+      filename: path.join(
+        __dirname,
+        '..',
+        // We need to go back once more to get out of the dist folder
+        '..',
+        env("DATABASE_TEST_FILENAME", ".tmp/test.db")),
+    },
+    useNullAsDefault: true,
+    debug: false,
+  },
+});

playground/package.json

@@ -13,7 +13,7 @@
     "@strapi/plugin-i18n": "^4.19.0",
     "@strapi/plugin-users-permissions": "^4.19.0",
     "@strapi/strapi": "^4.19.0",
-    "better-sqlite3": "8.6.0",
+    "better-sqlite3": "^9.4.0",
     "pg": "^8.10.0",
     "react": "^18.0.0",
     "react-dom": "^18.0.0",

playground/src/index.ts

@@ -8,7 +8,7 @@ module.exports = {
    *
    * This gives you an opportunity to extend code.
    */
-  register(/*{ strapi }*/) {},
+  register(/*{ strapi }*/) { },
 
   /**
    * An asynchronous bootstrap function that runs before
@@ -64,7 +64,7 @@ module.exports = {
           .updateRole(publicRole.id, publicRole);
       }
 
-      strapi.entityService.create('plugin::webtools.url-pattern', {
+      await strapi.entityService.create('plugin::webtools.url-pattern', {
         data: {
           pattern: '/page/[title]',
           label: 'Test API pattern',
@@ -74,7 +74,7 @@ module.exports = {
         }
       });
 
-      strapi.entityService.create('plugin::webtools.url-pattern', {
+      await strapi.entityService.create('plugin::webtools.url-pattern', {
         data: {
           pattern: '/category/[title]',
           label: 'Category API pattern',
@@ -84,7 +84,7 @@ module.exports = {
         }
       });
 
-      strapi.entityService.create('plugin::webtools.url-pattern', {
+      await strapi.entityService.create('plugin::webtools.url-pattern', {
         data: {
           pattern: '/private-category/[title]',
           label: 'Private category API pattern',
@@ -114,7 +114,7 @@ module.exports = {
         }
       });
 
-      strapi.entityService.create('api::test.test', {
+      await strapi.entityService.create('api::test.test', {
         data: {
           title: 'Published test page',
           publishedAt: new Date(),
@@ -123,7 +123,7 @@ module.exports = {
         }
       });
 
-      strapi.entityService.create('api::test.test', {
+      await strapi.entityService.create('api::test.test', {
         data: {
           title: 'Unpublished test page',
           category: publishedCategory.id,

playground/tests/helpers.js

@@ -46,9 +46,10 @@ async function setupStrapi() {
       appDir: './playground',
       distDir: './playground/dist'
     }).load();
-    await waitForServer();
 
     instance = strapi; // strapi is global now
+
+    await instance.server.mount();
   }
   return instance;
 }
@@ -58,16 +59,17 @@ async function setupStrapi() {
  */
 async function stopStrapi() {
   if (instance) {
-
+    await instance.server.httpServer.close();
+    await instance.db.connection.destroy();
     instance.destroy();
-
     const tmpDbFile = strapi.config.get(
       "database.connection.connection.filename"
     );
 
     if (fs.existsSync(tmpDbFile)) {
       fs.unlinkSync(tmpDbFile);
     }
+
   }
   return instance;
 }

playground/yarn.lock

@@ -4079,14 +4079,14 @@ __metadata:
   languageName: node
   linkType: hard
 
-"better-sqlite3@npm:8.6.0":
-  version: 8.6.0
-  resolution: "better-sqlite3@npm:8.6.0"
+"better-sqlite3@npm:^9.4.0":
+  version: 9.4.0
+  resolution: "better-sqlite3@npm:9.4.0"
   dependencies:
     bindings: "npm:^1.5.0"
     node-gyp: "npm:latest"
     prebuild-install: "npm:^7.1.1"
-  checksum: 4d5481bd173331c163add61c2fd12577e235d62abb1f72500b3856fd2af70a448a5515a3f12dd48f34bb97cfdf13d200ca81cd2ae16fc55627b96c5b9bd47bb0
+  checksum: 42b2edfa46d62763514b87122245a3513a5ff20f05fef4fb49fec33f3de0a51a29025596178f57c634b8013f16bbdf8169a308fb3e3b8d126d715788d72d1e74
   languageName: node
   linkType: hard
 
@@ -10544,7 +10544,7 @@ __metadata:
     "@strapi/plugin-i18n": "npm:^4.19.0"
     "@strapi/plugin-users-permissions": "npm:^4.19.0"
     "@strapi/strapi": "npm:^4.19.0"
-    better-sqlite3: "npm:8.6.0"
+    better-sqlite3: "npm:^9.4.0"
     pg: "npm:^8.10.0"
     react: "npm:^18.0.0"
     react-dom: "npm:^18.0.0"