From a3f00c3a4db18073e0bb31c0a53e592fd2ae680a Mon Sep 17 00:00:00 2001 From: Thenkei Date: Tue, 30 May 2023 15:03:02 +0200 Subject: [PATCH 1/4] fix: actions endpoints should be unique --- packages/agent/src/routes/modification/action/action.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/agent/src/routes/modification/action/action.ts b/packages/agent/src/routes/modification/action/action.ts index e58d090937..bf2765c4f8 100644 --- a/packages/agent/src/routes/modification/action/action.ts +++ b/packages/agent/src/routes/modification/action/action.ts @@ -46,13 +46,15 @@ export default class ActionRoute extends CollectionRoute { const actionIndex = Object.keys(this.collection.schema.actions).indexOf(this.actionName); const path = `/_actions/${this.collection.name}/${actionIndex}`; + // Generate url-safe friendly name (which won't be unique, but that's OK). + const actionName = this.actionName.toLocaleLowerCase().replace(/[^a-z0-9-]+/g, '-'); router.post( - `${path}/:slug`, + `${path}/${actionName}`, this.middlewareCustomActionApprovalRequestData.bind(this), this.handleExecute.bind(this), ); - router.post(`${path}/:slug/hooks/load`, this.handleHook.bind(this)); - router.post(`${path}/:slug/hooks/change`, this.handleHook.bind(this)); + router.post(`${path}/${actionName}/hooks/load`, this.handleHook.bind(this)); + router.post(`${path}/${actionName}/hooks/change`, this.handleHook.bind(this)); } private async handleExecute(context: Context): Promise { From 52dc13e15cffde0544296ffa67fc82e472e6a94e Mon Sep 17 00:00:00 2001 From: Thenkei Date: Tue, 30 May 2023 15:11:07 +0200 Subject: [PATCH 2/4] test: update old tests --- .../agent/src/routes/modification/action/action.ts | 8 ++++---- .../test/routes/modification/action/action.test.ts | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/agent/src/routes/modification/action/action.ts b/packages/agent/src/routes/modification/action/action.ts index bf2765c4f8..e2b3e5550a 100644 --- a/packages/agent/src/routes/modification/action/action.ts +++ b/packages/agent/src/routes/modification/action/action.ts @@ -47,14 +47,14 @@ export default class ActionRoute extends CollectionRoute { const path = `/_actions/${this.collection.name}/${actionIndex}`; // Generate url-safe friendly name (which won't be unique, but that's OK). - const actionName = this.actionName.toLocaleLowerCase().replace(/[^a-z0-9-]+/g, '-'); + const urlSafeActionName = this.actionName.toLocaleLowerCase().replace(/[^a-z0-9-]+/g, '-'); router.post( - `${path}/${actionName}`, + `${path}/${urlSafeActionName}`, this.middlewareCustomActionApprovalRequestData.bind(this), this.handleExecute.bind(this), ); - router.post(`${path}/${actionName}/hooks/load`, this.handleHook.bind(this)); - router.post(`${path}/${actionName}/hooks/change`, this.handleHook.bind(this)); + router.post(`${path}/${urlSafeActionName}/hooks/load`, this.handleHook.bind(this)); + router.post(`${path}/${urlSafeActionName}/hooks/change`, this.handleHook.bind(this)); } private async handleExecute(context: Context): Promise { diff --git a/packages/agent/test/routes/modification/action/action.test.ts b/packages/agent/test/routes/modification/action/action.test.ts index 682f25fdd7..5b96401a61 100644 --- a/packages/agent/test/routes/modification/action/action.test.ts +++ b/packages/agent/test/routes/modification/action/action.test.ts @@ -53,7 +53,7 @@ describe('ActionRoute', () => { dataSource = factories.dataSource.buildWithCollections([ factories.collection.build({ name: 'books', - schema: { actions: { MySingleAction: null } }, + schema: { actions: { MySingleAction: undefined } }, getForm: jest.fn(), execute: jest.fn(), }), @@ -64,16 +64,16 @@ describe('ActionRoute', () => { route.setupRoutes(router); expect(router.post).toHaveBeenCalledWith( - '/_actions/books/0/:slug', + '/_actions/books/0/mysingleaction', expect.any(Function), // middlewareCustomActionApprovalRequestData expect.any(Function), ); expect(router.post).toHaveBeenCalledWith( - '/_actions/books/0/:slug/hooks/load', + '/_actions/books/0/mysingleaction/hooks/load', expect.any(Function), ); expect(router.post).toHaveBeenCalledWith( - '/_actions/books/0/:slug/hooks/change', + '/_actions/books/0/mysingleaction/hooks/change', expect.any(Function), ); }); @@ -83,7 +83,7 @@ describe('ActionRoute', () => { dataSource = factories.dataSource.buildWithCollections([ factories.collection.build({ name: 'books', - schema: { actions: { My_Action: null } }, + schema: { actions: { My_Action: undefined } }, getForm: jest.fn(), execute: jest.fn(), }), From 9c93fbc23affbff59eff3b319e0552c03f23bac4 Mon Sep 17 00:00:00 2001 From: Thenkei Date: Tue, 30 May 2023 15:50:44 +0200 Subject: [PATCH 3/4] refactor: with romain --- .../agent/src/routes/modification/action/action.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/agent/src/routes/modification/action/action.ts b/packages/agent/src/routes/modification/action/action.ts index e2b3e5550a..0c4be7c051 100644 --- a/packages/agent/src/routes/modification/action/action.ts +++ b/packages/agent/src/routes/modification/action/action.ts @@ -43,18 +43,18 @@ export default class ActionRoute extends CollectionRoute { } setupRoutes(router: Router): void { + // Generate url that matches the declaration in forest-schema/generator-actions.ts const actionIndex = Object.keys(this.collection.schema.actions).indexOf(this.actionName); - const path = `/_actions/${this.collection.name}/${actionIndex}`; + const slug = this.actionName.toLocaleLowerCase().replace(/[^a-z0-9-]+/g, '-'); + const path = `/_actions/${this.collection.name}/${actionIndex}/${slug}`; - // Generate url-safe friendly name (which won't be unique, but that's OK). - const urlSafeActionName = this.actionName.toLocaleLowerCase().replace(/[^a-z0-9-]+/g, '-'); router.post( - `${path}/${urlSafeActionName}`, + `${path}`, this.middlewareCustomActionApprovalRequestData.bind(this), this.handleExecute.bind(this), ); - router.post(`${path}/${urlSafeActionName}/hooks/load`, this.handleHook.bind(this)); - router.post(`${path}/${urlSafeActionName}/hooks/change`, this.handleHook.bind(this)); + router.post(`${path}/hooks/load`, this.handleHook.bind(this)); + router.post(`${path}/hooks/change`, this.handleHook.bind(this)); } private async handleExecute(context: Context): Promise { From 1047dc98f5d914fbb66d9c8f57508dbe0395643a Mon Sep 17 00:00:00 2001 From: Thenkei Date: Tue, 30 May 2023 16:12:28 +0200 Subject: [PATCH 4/4] fix: reviews --- packages/agent/src/routes/modification/action/action.ts | 2 +- packages/agent/src/utils/forest-schema/generator-actions.ts | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/agent/src/routes/modification/action/action.ts b/packages/agent/src/routes/modification/action/action.ts index 0c4be7c051..3c480d0e34 100644 --- a/packages/agent/src/routes/modification/action/action.ts +++ b/packages/agent/src/routes/modification/action/action.ts @@ -45,7 +45,7 @@ export default class ActionRoute extends CollectionRoute { setupRoutes(router: Router): void { // Generate url that matches the declaration in forest-schema/generator-actions.ts const actionIndex = Object.keys(this.collection.schema.actions).indexOf(this.actionName); - const slug = this.actionName.toLocaleLowerCase().replace(/[^a-z0-9-]+/g, '-'); + const slug = SchemaGeneratorActions.getActionSlug(this.actionName); const path = `/_actions/${this.collection.name}/${actionIndex}/${slug}`; router.post( diff --git a/packages/agent/src/utils/forest-schema/generator-actions.ts b/packages/agent/src/utils/forest-schema/generator-actions.ts index 8468a519fc..a6b733dbb9 100644 --- a/packages/agent/src/utils/forest-schema/generator-actions.ts +++ b/packages/agent/src/utils/forest-schema/generator-actions.ts @@ -33,12 +33,16 @@ export default class SchemaGeneratorActions { }, ]; + static getActionSlug(name: string) { + return name.toLocaleLowerCase().replace(/[^a-z0-9-]+/g, '-'); + } + static async buildSchema(collection: Collection, name: string): Promise { const schema = collection.schema.actions[name]; const actionIndex = Object.keys(collection.schema.actions).indexOf(name); // Generate url-safe friendly name (which won't be unique, but that's OK). - const slug = name.toLocaleLowerCase().replace(/[^a-z0-9-]+/g, '-'); + const slug = SchemaGeneratorActions.getActionSlug(name); const fields = await SchemaGeneratorActions.buildFields(collection, name, schema); return {