From 6548fd77ca378473c973992f784ef965d54846e2 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Tue, 7 Mar 2023 11:56:09 +0000 Subject: [PATCH 1/8] refactor(review-workflows): use knex for raw sql query --- .../review-workflows/review-workflows.js | 51 ++++++++++++++----- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js index 3f513aad8d..2d72639d44 100644 --- a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js +++ b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js @@ -107,20 +107,43 @@ function enableReviewWorkflow({ strapi }) { const { idColumn, typeColumn } = joinTable.morphColumn; // Execute a raw SQL query to insert records into the join table mapping the specified content type with the first stage of the default workflow. - // Only entities that do not have a record in the join table yet are selected. - await strapi.db.connection - .raw(`INSERT INTO ${joinTable.name} (${idColumn.name}, field, "order", ${joinTable.joinColumn.name}, ${typeColumn.name}) - SELECT - entity.id as ${idColumn.name}, - '${ENTITY_STAGE_ATTRIBUTE}' as field, - 1 as "order", - ${firstStage.id} as ${joinTable.joinColumn.name}, - '${contentTypeUID}' as ${typeColumn.name} - FROM ${contentTypeMetadata.tableName} entity - LEFT JOIN ${joinTable.name} jointable - ON entity.id = jointable.${idColumn.name} - AND jointable.${typeColumn.name} = '${contentTypeUID}' - WHERE jointable.${idColumn.name} IS NULL`); + // Only entities that do not have a record in the join table yet are + // selected. + const connection = strapi.db.connection; + const columnsToInsert = [ + idColumn.name, + 'field', + `"order"`, + joinTable.joinColumn.name, + typeColumn.name, + ]; + + const selectStatement = connection + .select( + `entity.id as ${idColumn.name}`, + connection.raw(`'${ENTITY_STAGE_ATTRIBUTE}' as field`), + connection.raw(`1 as "order"`), + connection.raw(`'${firstStage.id}' as ${joinTable.joinColumn.name}`), + connection.raw(`'${contentTypeUID}' as ${typeColumn.name}`) + ) + .leftJoin(`${joinTable.name} AS jointable`, function () { + this.on('entity.id', '=', `jointable.${idColumn.name}`).andOn( + `jointable.${typeColumn.name}`, + '=', + connection.raw(`'${contentTypeUID}'`) + ); + }) + .where(`jointable.${idColumn.name}`, null) + .from(`${contentTypeMetadata.tableName} AS entity`) + .toSQL(); + + // Insert the clone relations + await connection(joinTable.name).insert( + connection.raw( + `(${columnsToInsert.join(',')}) ${selectStatement.sql}`, + selectStatement.bindings + ) + ); }; return pipe([ From 4bd99fce7262f4631911aee2363c844892d2efe3 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Tue, 7 Mar 2023 14:36:17 +0000 Subject: [PATCH 2/8] chore: code cleanup --- .../ee/server/services/review-workflows/review-workflows.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js index 2d72639d44..5ff28e3700 100644 --- a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js +++ b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js @@ -106,9 +106,8 @@ function enableReviewWorkflow({ strapi }) { const { joinTable } = strapi.db.metadata.get(target).attributes[morphBy]; const { idColumn, typeColumn } = joinTable.morphColumn; - // Execute a raw SQL query to insert records into the join table mapping the specified content type with the first stage of the default workflow. - // Only entities that do not have a record in the join table yet are - // selected. + // Execute an SQL query to insert records into the join table mapping the specified content type with the first stage of the default workflow. + // Only entities that do not have a record in the join table yet are selected. const connection = strapi.db.connection; const columnsToInsert = [ idColumn.name, From 9dacb1676fea304a8cdd066bfcf9e8bf9b32afdc Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 8 Mar 2023 11:08:50 +0000 Subject: [PATCH 3/8] chore: use db.getConnection chore: use identifier syntax --- .../review-workflows/review-workflows.js | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js index 5ff28e3700..0a1e984dc1 100644 --- a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js +++ b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js @@ -108,7 +108,7 @@ function enableReviewWorkflow({ strapi }) { // Execute an SQL query to insert records into the join table mapping the specified content type with the first stage of the default workflow. // Only entities that do not have a record in the join table yet are selected. - const connection = strapi.db.connection; + const connection = strapi.db.getConnection(); const columnsToInsert = [ idColumn.name, 'field', @@ -125,17 +125,23 @@ function enableReviewWorkflow({ strapi }) { connection.raw(`'${firstStage.id}' as ${joinTable.joinColumn.name}`), connection.raw(`'${contentTypeUID}' as ${typeColumn.name}`) ) - .leftJoin(`${joinTable.name} AS jointable`, function () { - this.on('entity.id', '=', `jointable.${idColumn.name}`).andOn( - `jointable.${typeColumn.name}`, - '=', - connection.raw(`'${contentTypeUID}'`) - ); - }) + .leftJoin( + { + jointable: joinTable.name, + }, + function () { + this.on('entity.id', '=', `jointable.${idColumn.name}`).andOn( + `jointable.${typeColumn.name}`, + '=', + connection.raw(`'${contentTypeUID}'`) + ); + } + ) .where(`jointable.${idColumn.name}`, null) - .from(`${contentTypeMetadata.tableName} AS entity`) + .from({ + entity: contentTypeMetadata.tableName, + }) .toSQL(); - // Insert the clone relations await connection(joinTable.name).insert( connection.raw( From 8a80c2a68ebedbcca0494cad88a9b1065bee0d8e Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 8 Mar 2023 11:20:23 +0000 Subject: [PATCH 4/8] chore: code cleanup --- .../ee/server/services/review-workflows/review-workflows.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js index 0a1e984dc1..a3c5f57175 100644 --- a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js +++ b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js @@ -142,7 +142,9 @@ function enableReviewWorkflow({ strapi }) { entity: contentTypeMetadata.tableName, }) .toSQL(); - // Insert the clone relations + + // Insert rows for all entries of the content type that do not have a + // default stage await connection(joinTable.name).insert( connection.raw( `(${columnsToInsert.join(',')}) ${selectStatement.sql}`, From 5ade199d36568394e88ee26447f437e73a123de4 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 8 Mar 2023 12:28:08 +0000 Subject: [PATCH 5/8] fix: specify order columns using knex.raw --- .../review-workflows/review-workflows.js | 56 +++++++++---------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js index a3c5f57175..807295d524 100644 --- a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js +++ b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js @@ -106,45 +106,39 @@ function enableReviewWorkflow({ strapi }) { const { joinTable } = strapi.db.metadata.get(target).attributes[morphBy]; const { idColumn, typeColumn } = joinTable.morphColumn; + const connection = strapi.db.getConnection(); + // Execute an SQL query to insert records into the join table mapping the specified content type with the first stage of the default workflow. // Only entities that do not have a record in the join table yet are selected. - const connection = strapi.db.getConnection(); - const columnsToInsert = [ - idColumn.name, - 'field', - `"order"`, - joinTable.joinColumn.name, - typeColumn.name, - ]; - const selectStatement = connection - .select( - `entity.id as ${idColumn.name}`, - connection.raw(`'${ENTITY_STAGE_ATTRIBUTE}' as field`), - connection.raw(`1 as "order"`), - connection.raw(`'${firstStage.id}' as ${joinTable.joinColumn.name}`), - connection.raw(`'${contentTypeUID}' as ${typeColumn.name}`) - ) - .leftJoin( - { - jointable: joinTable.name, - }, - function () { - this.on('entity.id', '=', `jointable.${idColumn.name}`).andOn( - `jointable.${typeColumn.name}`, - '=', - connection.raw(`'${contentTypeUID}'`) - ); - } - ) - .where(`jointable.${idColumn.name}`, null) - .from({ - entity: contentTypeMetadata.tableName, + .select({ + [idColumn.name]: 'entity.id', + field: connection.raw('?', [ENTITY_STAGE_ATTRIBUTE]), + order: 1, + [joinTable.joinColumn.name]: firstStage.id, + [typeColumn.name]: connection.raw('?', [contentTypeUID]), }) + .leftJoin(`${joinTable.name} AS jointable`, function () { + this.on('entity.id', '=', `jointable.${idColumn.name}`).andOn( + `jointable.${typeColumn.name}`, + '=', + connection.raw('?', [contentTypeUID]) + ); + }) + .where(`jointable.${idColumn.name}`, null) + .from(`${contentTypeMetadata.tableName} AS entity`) .toSQL(); // Insert rows for all entries of the content type that do not have a // default stage + const columnsToInsert = [ + idColumn.name, + 'field', + connection.raw('??', 'order'), + joinTable.joinColumn.name, + typeColumn.name, + ]; + await connection(joinTable.name).insert( connection.raw( `(${columnsToInsert.join(',')}) ${selectStatement.sql}`, From 28032f884e20125222949a85312b5d81d2a0a4eb Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 8 Mar 2023 14:41:19 +0000 Subject: [PATCH 6/8] fix(review-workflows): refactor to support all db types --- .../ee/server/services/review-workflows/review-workflows.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js index 807295d524..f3908882de 100644 --- a/packages/core/admin/ee/server/services/review-workflows/review-workflows.js +++ b/packages/core/admin/ee/server/services/review-workflows/review-workflows.js @@ -129,16 +129,16 @@ function enableReviewWorkflow({ strapi }) { .from(`${contentTypeMetadata.tableName} AS entity`) .toSQL(); - // Insert rows for all entries of the content type that do not have a - // default stage const columnsToInsert = [ idColumn.name, 'field', - connection.raw('??', 'order'), + connection.raw('??', ['order']), joinTable.joinColumn.name, typeColumn.name, ]; + // Insert rows for all entries of the content type that do not have a + // default stage await connection(joinTable.name).insert( connection.raw( `(${columnsToInsert.join(',')}) ${selectStatement.sql}`, From 6ac56deb8d422c9a037abb2296a514801707ff1a Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 8 Mar 2023 16:12:53 +0000 Subject: [PATCH 7/8] test(review-workflows): API test for RW up migration --- .../server/tests/review-workflows.test.api.js | 83 ++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/packages/core/admin/ee/server/tests/review-workflows.test.api.js b/packages/core/admin/ee/server/tests/review-workflows.test.api.js index e6dded7979..4615b71928 100644 --- a/packages/core/admin/ee/server/tests/review-workflows.test.api.js +++ b/packages/core/admin/ee/server/tests/review-workflows.test.api.js @@ -2,12 +2,31 @@ const { createStrapiInstance } = require('../../../../../../test/helpers/strapi'); const { createAuthRequest, createRequest } = require('../../../../../../test/helpers/request'); +const { createTestBuilder } = require('../../../../../../test/helpers/builder'); + const { describeOnCondition } = require('../utils/test'); const { STAGE_MODEL_UID, WORKFLOW_MODEL_UID } = require('../constants/workflows'); const edition = process.env.STRAPI_DISABLE_EE === 'true' ? 'CE' : 'EE'; +const productUID = 'api::product.product'; +const model = { + draftAndPublish: true, + pluginOptions: {}, + singularName: 'product', + pluralName: 'products', + displayName: 'Product', + kind: 'collectionType', + attributes: { + name: { + type: 'string', + }, + }, +}; + describeOnCondition(edition === 'EE')('Review workflows', () => { + const builder = createTestBuilder(); + const requests = { public: null, admin: null, @@ -18,11 +37,37 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { let secondStage; let testWorkflow; - beforeAll(async () => { + const createEntry = async (uid, data) => { + const { body } = await requests.admin({ + method: 'POST', + url: `/content-manager/collection-types/${uid}`, + body: data, + }); + return body; + }; + + const updateContentType = async (uid, data) => { + const result = await requests.admin({ + method: 'PUT', + url: `/content-type-builder/content-types/${uid}`, + body: data, + }); + + expect(result.statusCode).toBe(201); + }; + + const restart = async () => { + await strapi.destroy(); strapi = await createStrapiInstance(); + requests.admin = await createAuthRequest({ strapi }); + }; + + beforeAll(async () => { + await builder.addContentTypes([model]).build(); // eslint-disable-next-line node/no-extraneous-require hasRW = require('@strapi/strapi/lib/utils/ee').features.isEnabled('review-workflows'); + strapi = await createStrapiInstance(); requests.public = createRequest({ strapi }); requests.admin = await createAuthRequest({ strapi }); @@ -42,6 +87,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { afterAll(async () => { await strapi.destroy(); + await builder.cleanup(); }); describe('Get workflows', () => { @@ -269,4 +315,39 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { } }); }); + + describe('Enable review workflows on a content type', () => { + test('When enabled entries in the content type should be added to the first stage of a workflow', async () => { + await createEntry(productUID, { name: 'Product' }); + await createEntry(productUID, { name: 'Product 1' }); + await createEntry(productUID, { name: 'Product 2' }); + + await updateContentType(productUID, { + components: [], + contentType: { ...model, reviewWorkflows: true }, + }); + + await restart(); + + const res = await requests.admin({ + method: 'GET', + url: `/content-type-builder/content-types/api::product.product`, + }); + + expect(res.body.data.schema.reviewWorkflows).toBeTruthy(); + + const connection = strapi.db.getConnection(); + const RWMorphTableResults = await connection + .select('*') + .from('strapi_workflows_stages_related_morphs') + .where('related_type', productUID); + + expect(RWMorphTableResults.length).toEqual(3); + for (let i = 0; i < RWMorphTableResults.length; i += 1) { + const entry = RWMorphTableResults[i]; + expect(entry.related_id).toEqual(i + 1); + expect(entry.order).toEqual(1); + } + }); + }); }); From f1136fedd461a1a97f18c84fba93ebd5192a7b86 Mon Sep 17 00:00:00 2001 From: Jamie Howard Date: Wed, 8 Mar 2023 16:17:00 +0000 Subject: [PATCH 8/8] chore: wording --- .../core/admin/ee/server/tests/review-workflows.test.api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/admin/ee/server/tests/review-workflows.test.api.js b/packages/core/admin/ee/server/tests/review-workflows.test.api.js index 4615b71928..0bca5d5524 100644 --- a/packages/core/admin/ee/server/tests/review-workflows.test.api.js +++ b/packages/core/admin/ee/server/tests/review-workflows.test.api.js @@ -317,7 +317,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); describe('Enable review workflows on a content type', () => { - test('When enabled entries in the content type should be added to the first stage of a workflow', async () => { + test('when enabled on a content type, entries of this type should be added to the first stage of the workflow', async () => { await createEntry(productUID, { name: 'Product' }); await createEntry(productUID, { name: 'Product 1' }); await createEntry(productUID, { name: 'Product 2' });