From ea370c63867c5cbee247256d6d476b35881b7abb Mon Sep 17 00:00:00 2001 From: Marc-Roig Date: Wed, 17 May 2023 10:42:07 +0200 Subject: [PATCH 1/6] feat: add delete workflow route --- .../admin/ee/server/config/admin-actions.js | 8 +++++++ .../ee/server/controllers/workflows/index.js | 21 +++++++++++++++++++ .../ee/server/routes/review-workflows.js | 17 +++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/packages/core/admin/ee/server/config/admin-actions.js b/packages/core/admin/ee/server/config/admin-actions.js index 4429062bf9..91e373ad59 100644 --- a/packages/core/admin/ee/server/config/admin-actions.js +++ b/packages/core/admin/ee/server/config/admin-actions.js @@ -46,6 +46,14 @@ module.exports = { category: 'review workflows', subCategory: 'options', }, + { + uid: 'review-workflows.delete', + displayName: 'Delete', + pluginName: 'admin', + section: 'settings', + category: 'review workflows', + subCategory: 'options', + }, { uid: 'review-workflows.read', displayName: 'Read', diff --git a/packages/core/admin/ee/server/controllers/workflows/index.js b/packages/core/admin/ee/server/controllers/workflows/index.js index 70e12dbf64..4146fa275f 100644 --- a/packages/core/admin/ee/server/controllers/workflows/index.js +++ b/packages/core/admin/ee/server/controllers/workflows/index.js @@ -50,6 +50,27 @@ module.exports = { }; }, + /** + * Delete a workflow + * @param {import('koa').BaseContext} ctx - koa context + */ + async delete(ctx) { + const { id } = ctx.params; + const { populate } = ctx.query; + const workflowService = getService('workflows'); + + const workflow = await workflowService.findById(id, { populate: ['stages'] }); + if (!workflow) { + return ctx.notFound("Workflow doesn't exist"); + } + + const data = await workflowService.delete(workflow, { populate }); + + ctx.body = { + data, + }; + }, + /** * List all workflows * @param {import('koa').BaseContext} ctx - koa context diff --git a/packages/core/admin/ee/server/routes/review-workflows.js b/packages/core/admin/ee/server/routes/review-workflows.js index a859c0a394..dd1fe19c5d 100644 --- a/packages/core/admin/ee/server/routes/review-workflows.js +++ b/packages/core/admin/ee/server/routes/review-workflows.js @@ -40,6 +40,23 @@ module.exports = { ], }, }, + { + method: 'DELETE', + path: '/review-workflows/workflows/:id', + handler: 'workflows.delete', + config: { + middlewares: [enableFeatureMiddleware('review-workflows')], + policies: [ + 'admin::isAuthenticatedAdmin', + { + name: 'admin::hasPermissions', + config: { + actions: ['admin::review-workflows.delete'], + }, + }, + ], + }, + }, { method: 'GET', path: '/review-workflows/workflows', From b1c17c3b69b0e290136c8975d60915177726bdb4 Mon Sep 17 00:00:00 2001 From: Marc-Roig Date: Wed, 17 May 2023 10:43:12 +0200 Subject: [PATCH 2/6] feat: delete workflow service --- .../services/review-workflows/stages.js | 6 +++++ .../review-workflows/workflows/index.js | 24 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/packages/core/admin/ee/server/services/review-workflows/stages.js b/packages/core/admin/ee/server/services/review-workflows/stages.js index 6be23203c7..2b5e499f9d 100644 --- a/packages/core/admin/ee/server/services/review-workflows/stages.js +++ b/packages/core/admin/ee/server/services/review-workflows/stages.js @@ -60,6 +60,12 @@ module.exports = ({ strapi }) => { return stage; }, + async deleteMany(stagesId) { + return strapi.entityService.deleteMany(STAGE_MODEL_UID, { + filters: { id: { $in: stagesId } }, + }); + }, + count() { return strapi.entityService.count(STAGE_MODEL_UID); }, diff --git a/packages/core/admin/ee/server/services/review-workflows/workflows/index.js b/packages/core/admin/ee/server/services/review-workflows/workflows/index.js index 03e3152f4a..c7a8d05ca8 100644 --- a/packages/core/admin/ee/server/services/review-workflows/workflows/index.js +++ b/packages/core/admin/ee/server/services/review-workflows/workflows/index.js @@ -122,6 +122,30 @@ module.exports = ({ strapi }) => { }); }, + /** + * Deletes an existing workflow. + * Also deletes all the workflow stages and migrate all assigned the content types. + * @param {*} workflow + * @param {*} opts + * @returns + */ + async delete(workflow, opts) { + const stageService = getService('stages', { strapi }); + + return strapi.db.transaction(async () => { + // Delete stages + await stageService.deleteMany(workflow.stages.map((stage) => stage.id)); + + // Unassign all content types, this will migrate the content types to null + await workflowsContentTypes.migrate({ + srcContentTypes: workflow.contentTypes, + destContentTypes: [], + }); + + // Delete Workflow + return strapi.entityService.delete(WORKFLOW_MODEL_UID, workflow.id, opts); + }); + }, /** * Returns the total count of workflows. * @returns {Promise} - Total count of workflows. From 38377a89b793eeb10b9953d5432ff73c7d441920 Mon Sep 17 00:00:00 2001 From: Marc-Roig Date: Wed, 17 May 2023 10:43:23 +0200 Subject: [PATCH 3/6] test: review workflows delete --- ...review-workflows-content-types.test.api.js | 23 +++++++++++++++ .../admin/ee/review-workflows.test.api.js | 29 ++++++++++++++++--- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/api-tests/core/admin/ee/review-workflows-content-types.test.api.js b/api-tests/core/admin/ee/review-workflows-content-types.test.api.js index ad7229984e..5407f5ef6b 100644 --- a/api-tests/core/admin/ee/review-workflows-content-types.test.api.js +++ b/api-tests/core/admin/ee/review-workflows-content-types.test.api.js @@ -50,6 +50,10 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); }; + const deleteWorkflow = async (id) => { + return requests.admin.delete(`/admin/review-workflows/workflows/${id}`); + }; + const getWorkflow = async (id) => { const { body } = await requests.admin.get(`/admin/review-workflows/workflows/${id}?populate=*`); return body.data; @@ -253,6 +257,25 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); }); + describe('Delete workflow', () => { + let workflow; + test('Can delete workflow', async () => { + workflow = await createWorkflow({ contentTypes: [productUID] }).then((res) => res.body.data); + + const res = await deleteWorkflow(workflow.id); + expect(res.status).toBe(200); + }); + + // Depends on the previous test + test('All entities have null stage', async () => { + const products = await findAll(productUID); + + expect(products.results).toHaveLength(2); + for (const product of products.results) { + expect(product[ENTITY_STAGE_ATTRIBUTE]).toBeNull(); + } + }); + }); describe('Creating an entity in a review workflow content type', () => { let workflow; test('when content type is assigned to workflow, new entries should be added to the first stage of the default workflow', async () => { diff --git a/api-tests/core/admin/ee/review-workflows.test.api.js b/api-tests/core/admin/ee/review-workflows.test.api.js index 125e3ca3cc..a4592add91 100644 --- a/api-tests/core/admin/ee/review-workflows.test.api.js +++ b/api-tests/core/admin/ee/review-workflows.test.api.js @@ -174,7 +174,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); describe('Create workflow', () => { - test('You can not create a workflow without stages', async () => { + test('It should create a workflow without stages', async () => { const res = await requests.admin.post('/admin/review-workflows/workflows', { body: { name: 'testWorkflow', @@ -190,7 +190,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { expect(res.body.data).toBeUndefined(); } }); - test('You can create a workflow with stages', async () => { + test('It should create a workflow with stages', async () => { const res = await requests.admin.post('/admin/review-workflows/workflows?populate=stages', { body: { name: 'createdWorkflow', @@ -214,7 +214,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); describe('Update workflow', () => { - test('You can update a workflow', async () => { + test('It should update a workflow', async () => { const res = await requests.admin.put( `/admin/review-workflows/workflows/${createdWorkflow.id}`, { body: { name: 'updatedWorkflow' } } @@ -229,7 +229,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { } }); - test('You can update a workflow with stages', async () => { + test('It should update a workflow with stages', async () => { const res = await requests.admin.put( `/admin/review-workflows/workflows/${createdWorkflow.id}?populate=stages`, { @@ -259,6 +259,27 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); }); + describe('Delete workflow', () => { + test('It should delete a workflow', async () => { + const createdRes = await requests.admin.post('/admin/review-workflows/workflows', { + body: { name: 'testWorkflow', stages: [{ name: 'Stage 1' }] }, + }); + + const res = await requests.admin.delete( + `/admin/review-workflows/workflows/${createdRes.body.data.id}` + ); + + expect(res.status).toBe(200); + expect(res.body.data).toMatchObject({ name: 'testWorkflow' }); + }); + test("It shouldn't delete a workflow that does not exist", async () => { + const res = await requests.admin.delete(`/admin/review-workflows/workflows/123456789`); + + expect(res.status).toBe(404); + expect(res.body.data).toBeNull(); + }); + }); + describe('Get workflow stages', () => { test("It shouldn't be available for public", async () => { const res = await requests.public.get( From 72c08abd235f849193fc4552e47a72bfde9119f1 Mon Sep 17 00:00:00 2001 From: Marc-Roig Date: Wed, 17 May 2023 10:45:34 +0200 Subject: [PATCH 4/6] feat: do not delete the last workflow --- .../ee/server/services/review-workflows/workflows/index.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/core/admin/ee/server/services/review-workflows/workflows/index.js b/packages/core/admin/ee/server/services/review-workflows/workflows/index.js index c7a8d05ca8..07b55ba6fc 100644 --- a/packages/core/admin/ee/server/services/review-workflows/workflows/index.js +++ b/packages/core/admin/ee/server/services/review-workflows/workflows/index.js @@ -132,6 +132,12 @@ module.exports = ({ strapi }) => { async delete(workflow, opts) { const stageService = getService('stages', { strapi }); + const workflowCount = await this.count(); + + if (workflowCount <= 1) { + throw new ApplicationError('Can not delete the last workflow'); + } + return strapi.db.transaction(async () => { // Delete stages await stageService.deleteMany(workflow.stages.map((stage) => stage.id)); From bf1fa199532729fc83d0ced9b46bacb93e430040 Mon Sep 17 00:00:00 2001 From: Marc-Roig Date: Wed, 17 May 2023 11:20:34 +0200 Subject: [PATCH 5/6] chore: rename tests --- ...review-workflows-content-types.test.api.js | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/api-tests/core/admin/ee/review-workflows-content-types.test.api.js b/api-tests/core/admin/ee/review-workflows-content-types.test.api.js index 5407f5ef6b..68a638a2ea 100644 --- a/api-tests/core/admin/ee/review-workflows-content-types.test.api.js +++ b/api-tests/core/admin/ee/review-workflows-content-types.test.api.js @@ -106,7 +106,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { let workflow1, workflow2; describe('Create workflow and assign content type', () => { - test('Can create and assign a content type', async () => { + test('It should create a workflow and assign a content type', async () => { const res = await createWorkflow({ contentTypes: [productUID] }); expect(res.status).toBe(200); @@ -114,7 +114,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { workflow1 = res.body.data; }); - test('All product entities have the first stage', async () => { + test('All product entities should have the first stage', async () => { const products = await findAll(productUID); expect(products.results).toHaveLength(2); @@ -125,7 +125,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); describe('Create workflow and steal content type from another workflow', () => { - test('Can create workflow stealing content type from another', async () => { + test('It should create workflow stealing content type from another', async () => { const res = await createWorkflow({ contentTypes: [productUID], stages: [{ name: 'Review' }], @@ -135,7 +135,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { workflow2 = res.body.data; }); - test('All product entities have the new first stage', async () => { + test('All product entities should have the new first stage', async () => { const products = await findAll(productUID); expect(products.results).toHaveLength(2); @@ -144,13 +144,13 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { } }); - test('Original workflow is updated', async () => { + test('Original workflow should be updated', async () => { const workflow = await getWorkflow(workflow1.id); expect(workflow).toMatchObject({ contentTypes: [] }); }); }); - test('Can not create with invalid content type', async () => { + test("It shouldn't create a workflow with invalid content type", async () => { const res = await createWorkflow({ contentTypes: ['someUID'] }); expect(res.status).toBe(400); }); @@ -160,7 +160,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { let workflow1, workflow2; describe('Basic update', () => { - test('Can assign a content type', async () => { + test('It should assign a content type', async () => { workflow1 = await createWorkflow({ contentTypes: [] }).then((res) => res.body.data); const res = await updateWorkflow(workflow1.id, { @@ -171,7 +171,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { expect(res.body.data).toMatchObject({ contentTypes: [productUID] }); }); - test('All product entities have the first stage', async () => { + test('All product entities should have the first stage', async () => { const products = await findAll(productUID); expect(products.results).toHaveLength(2); @@ -183,14 +183,14 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { // Depends on the previous test describe('Steal content type', () => { - test('Can steal content type from another', async () => { + test('It should be able to steal a content type from another workflow', async () => { workflow2 = await createWorkflow({ contentTypes: [] }).then((res) => res.body.data); const res = await updateWorkflow(workflow2.id, { contentTypes: [productUID] }); expect(res.status).toBe(200); expect(res.body.data).toMatchObject({ contentTypes: [productUID] }); }); - test('All product entities have the new first stage', async () => { + test('All product entities should have the new first stage', async () => { const products = await findAll(productUID); expect(products.results).toHaveLength(2); @@ -199,7 +199,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { } }); - test('Original workflow is updated', async () => { + test('Original workflow should be updated', async () => { const workflow = await getWorkflow(workflow1.id); expect(workflow).toMatchObject({ contentTypes: [] }); }); @@ -207,13 +207,13 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { // Depends on the previous test describe('Unassign content type', () => { - test('Can unassign content type', async () => { + test('It should unassign content type', async () => { const res = await updateWorkflow(workflow2.id, { contentTypes: [] }); expect(res.status).toBe(200); expect(res.body.data).toMatchObject({ contentTypes: [] }); }); - test('All product entities have null stage', async () => { + test('All product entities should have null stage', async () => { const products = await findAll(productUID); expect(products.results).toHaveLength(2); @@ -224,7 +224,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); describe('Assign and update stages', () => { - test('Can assign and update stages', async () => { + test('It should assign and update stages', async () => { workflow1 = await createWorkflow({ contentTypes: [] }).then((res) => res.body.data); // Update stages @@ -240,7 +240,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); }); - test('All product entities have the new first stage', async () => { + test('All product entities should have the new first stage', async () => { const products = await findAll(productUID); expect(products.results).toHaveLength(2); @@ -251,7 +251,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); }); - test('Can not create with invalid content type', async () => { + test("It shouldn't create with invalid content type", async () => { const res = await createWorkflow({ contentTypes: ['someUID'] }); expect(res.status).toBe(400); }); @@ -259,7 +259,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { describe('Delete workflow', () => { let workflow; - test('Can delete workflow', async () => { + test('It should delete the workflow', async () => { workflow = await createWorkflow({ contentTypes: [productUID] }).then((res) => res.body.data); const res = await deleteWorkflow(workflow.id); @@ -267,7 +267,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); // Depends on the previous test - test('All entities have null stage', async () => { + test('All entities should have null stage', async () => { const products = await findAll(productUID); expect(products.results).toHaveLength(2); @@ -276,9 +276,9 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { } }); }); - describe('Creating an entity in a review workflow content type', () => { + describe('Creating entity assigned to a workflow', () => { let workflow; - test('when content type is assigned to workflow, new entries should be added to the first stage of the default workflow', async () => { + test('When content type is assigned to workflow, new entries should be added to the first stage of the default workflow', async () => { // Create a workflow with product content type workflow = await createWorkflow({ contentTypes: [productUID] }).then((res) => res.body.data); @@ -287,7 +287,7 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { }); // Depends on the previous test - test('when content type is not assigned to workflow, new entries should have a null stage', async () => { + test('When content type is not assigned to workflow, new entries should have a null stage', async () => { // Unassign product content type from default workflow await updateWorkflow(workflow.id, { contentTypes: [] }); From 84c37537c73cf7afa0868744fa8b97fdc82f2837 Mon Sep 17 00:00:00 2001 From: Marc-Roig Date: Fri, 19 May 2023 15:54:03 +0200 Subject: [PATCH 6/6] fix: udate admin permissions snapshot --- api-tests/core/admin/admin-permission.test.api.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api-tests/core/admin/admin-permission.test.api.js b/api-tests/core/admin/admin-permission.test.api.js index 96a952e8ce..5976e483c2 100644 --- a/api-tests/core/admin/admin-permission.test.api.js +++ b/api-tests/core/admin/admin-permission.test.api.js @@ -379,6 +379,12 @@ describe('Role CRUD End to End', () => { "displayName": "Create", "subCategory": "options", }, + { + "action": "admin::review-workflows.delete", + "category": "review workflows", + "displayName": "Delete", + "subCategory": "options", + }, { "action": "admin::review-workflows.read", "category": "review workflows",