Merge pull request #297 from mfikria/dev-296

maxceem · web-flow · commit 866c8f5bc933 · 2019-04-30T09:28:00.000+07:00
Improvements for product/project template refactoring issue #296
diff --git a/src/routes/productTemplates/create.js b/src/routes/productTemplates/create.js
@@ -23,8 +23,8 @@ const schema = {
       brief: Joi.string().max(45).required(),
       details: Joi.string().max(255).required(),
       aliases: Joi.array().required(),
-      template: Joi.object(),
-      form: Joi.object(),
+      template: Joi.object().empty(null),
+      form: Joi.object().empty(null),
       disabled: Joi.boolean().optional(),
       hidden: Joi.boolean().optional(),
       isAddOn: Joi.boolean().optional(),
@@ -34,7 +34,9 @@ const schema = {
       createdBy: Joi.any().strip(),
       updatedBy: Joi.any().strip(),
       deletedBy: Joi.any().strip(),
-    }).required(),
+    })
+      .xor('form', 'template')
+      .required(),
   },
 };
 
diff --git a/src/routes/productTemplates/create.spec.js b/src/routes/productTemplates/create.spec.js
@@ -89,13 +89,18 @@ describe('CREATE product template', () => {
       },
     };
 
-    const bodyWithForm = _.cloneDeep(body);
-    bodyWithForm.param.form = {
+    const bodyDefinedFormTemplate = _.cloneDeep(body);
+    bodyDefinedFormTemplate.param.form = {
       version: 1,
       key: 'dev',
     };
+
+    const bodyWithForm = _.cloneDeep(bodyDefinedFormTemplate);
     delete bodyWithForm.param.template;
 
+    const bodyMissingFormTemplate = _.cloneDeep(bodyWithForm);
+    delete bodyMissingFormTemplate.param.form;
+
     const bodyInvalidForm = _.cloneDeep(body);
     bodyInvalidForm.param.form = {
       version: 1,
@@ -277,5 +282,27 @@ describe('CREATE product template', () => {
         .send(bodyInvalidForm)
         .expect(422, done);
     });
+
+    it('should return 422 if both form or template field are defined', (done) => {
+      request(server)
+        .post('/v4/projects/metadata/productTemplates')
+        .set({
+          Authorization: `Bearer ${testUtil.jwts.admin}`,
+        })
+        .send(bodyDefinedFormTemplate)
+        .expect('Content-Type', /json/)
+        .expect(422, done);
+    });
+
+    it('should return 422 if both form or template field are missing', (done) => {
+      request(server)
+        .post('/v4/projects/metadata/productTemplates')
+        .set({
+          Authorization: `Bearer ${testUtil.jwts.admin}`,
+        })
+        .send(bodyMissingFormTemplate)
+        .expect('Content-Type', /json/)
+        .expect(422, done);
+    });
   });
 });
diff --git a/src/routes/productTemplates/update.js b/src/routes/productTemplates/update.js
@@ -26,8 +26,8 @@ const schema = {
       brief: Joi.string().max(45),
       details: Joi.string().max(255),
       aliases: Joi.array(),
-      template: Joi.object(),
-      form: Joi.object(),
+      template: Joi.object().empty(null),
+      form: Joi.object().empty(null),
       disabled: Joi.boolean().optional(),
       hidden: Joi.boolean().optional(),
       isAddOn: Joi.boolean().optional(),
@@ -37,7 +37,9 @@ const schema = {
       createdBy: Joi.any().strip(),
       updatedBy: Joi.any().strip(),
       deletedBy: Joi.any().strip(),
-    }).required(),
+    })
+      .xor('form', 'template')
+      .required(),
   },
 };
 
diff --git a/src/routes/productTemplates/update.spec.js b/src/routes/productTemplates/update.spec.js
@@ -133,12 +133,18 @@ describe('UPDATE product template', () => {
       },
     };
 
-    const bodyWithForm = _.cloneDeep(body);
-    bodyWithForm.param.form = {
+    const bodyDefinedFormTemplate = _.cloneDeep(body);
+    bodyDefinedFormTemplate.param.form = {
       version: 1,
       key: 'dev',
     };
 
+    const bodyWithForm = _.cloneDeep(bodyDefinedFormTemplate);
+    delete bodyWithForm.param.template;
+
+    const bodyMissingFormTemplate = _.cloneDeep(bodyWithForm);
+    delete bodyMissingFormTemplate.param.form;
+
     const bodyInvalidForm = _.cloneDeep(body);
     bodyInvalidForm.param.form = {
       version: 1,
@@ -312,5 +318,25 @@ describe('UPDATE product template', () => {
         .send(bodyInvalidForm)
         .expect(422, done);
     });
+
+    it('should return 422 if both form or template field are defined', (done) => {
+      request(server)
+        .patch(`/v4/projects/metadata/productTemplates/${templateId}`)
+        .set({
+          Authorization: `Bearer ${testUtil.jwts.admin}`,
+        })
+        .send(bodyDefinedFormTemplate)
+        .expect(422, done);
+    });
+
+    it('should return 422 if both form or template field are missing', (done) => {
+      request(server)
+        .patch(`/v4/projects/metadata/productTemplates/${templateId}`)
+        .set({
+          Authorization: `Bearer ${testUtil.jwts.admin}`,
+        })
+        .send(bodyMissingFormTemplate)
+        .expect(422, done);
+    });
   });
 });
diff --git a/src/routes/projectTemplates/create.js b/src/routes/projectTemplates/create.js
@@ -22,11 +22,11 @@ const schema = {
       question: Joi.string().max(255).required(),
       info: Joi.string().max(255).required(),
       aliases: Joi.array().required(),
-      scope: Joi.object().optional().allow(null),
-      phases: Joi.object().optional().allow(null),
-      form: Joi.object().optional().allow(null),
-      planConfig: Joi.object().optional().allow(null),
-      priceConfig: Joi.object().optional().allow(null),
+      scope: Joi.object().empty(null),
+      phases: Joi.object().empty(null),
+      form: Joi.object().empty(null),
+      planConfig: Joi.object().empty(null),
+      priceConfig: Joi.object().empty(null),
       disabled: Joi.boolean().optional(),
       hidden: Joi.boolean().optional(),
       createdAt: Joi.any().strip(),
@@ -35,7 +35,11 @@ const schema = {
       createdBy: Joi.any().strip(),
       updatedBy: Joi.any().strip(),
       deletedBy: Joi.any().strip(),
-    }).required(),
+    })
+      .xor('form', 'scope')
+      .xor('phases', 'planConfig')
+      .nand('priceConfig', 'scope')
+      .required(),
   },
 };
 
@@ -47,54 +51,12 @@ module.exports = [
     const param = req.body.param;
     const { form, priceConfig, planConfig } = param;
 
-    const checkModel = (keyInfo, modelName, model) => {
-      let errorMessage = '';
-      if (keyInfo == null) {
-        return Promise.resolve(null);
-      }
-      if ((keyInfo.version != null) && (keyInfo.key != null)) {
-        errorMessage = `${modelName} with key ${keyInfo.key} and version ${keyInfo.version}`
-          + ' referred in the project template is not found';
-        return (model.findOne({
-          where: {
-            key: keyInfo.key,
-            version: keyInfo.version,
-          },
-        })).then((record) => {
-          if (record == null) {
-            return Promise.resolve(errorMessage);
-          }
-          return Promise.resolve(null);
-        });
-      } else if ((keyInfo.version == null) && (keyInfo.key != null)) {
-        errorMessage = `${modelName} with key ${keyInfo.key}`
-          + ' referred in the project template is not found';
-        return model.findOne({
-          where: {
-            key: keyInfo.key,
-          },
-        }).then((record) => {
-          if (record == null) {
-            return Promise.resolve(errorMessage);
-          }
-          return Promise.resolve(null);
-        });
-      }
-      return Promise.resolve(null);
-    };
-
     return Promise.all([
-      checkModel(form, 'Form', models.Form, next),
-      checkModel(priceConfig, 'PriceConfig', models.PriceConfig, next),
-      checkModel(planConfig, 'PlanConfig', models.PlanConfig, next),
+      util.checkModel(form, 'Form', models.Form, 'project template'),
+      util.checkModel(priceConfig, 'PriceConfig', models.PriceConfig, 'project template'),
+      util.checkModel(planConfig, 'PlanConfig', models.PlanConfig, 'project template'),
     ])
-      .then((errorMessages) => {
-        const errorMessage = errorMessages.find(e => e && e.length > 0);
-        if (errorMessage) {
-          const apiErr = new Error(errorMessage);
-          apiErr.status = 422;
-          throw apiErr;
-        }
+      .then(() => {
         const entity = _.assign(req.body.param, {
           createdBy: req.authUser.userId,
           updatedBy: req.authUser.userId,
diff --git a/src/routes/projectTemplates/create.spec.js b/src/routes/projectTemplates/create.spec.js
@@ -108,6 +108,17 @@ describe('CREATE project template', () => {
       },
     };
 
+    const bodyDefinedFormScope = _.cloneDeep(body);
+    bodyDefinedFormScope.param.form = {
+      scope1: {
+        subScope1A: 1,
+        subScope1B: 2,
+      },
+      scope2: [1, 2, 3],
+    };
+    const bodyMissingFormScope = _.cloneDeep(body);
+    delete bodyMissingFormScope.param.scope;
+
     it('should return 403 if user is not authenticated', (done) => {
       request(server)
         .post('/v4/projects/metadata/projectTemplates')
@@ -270,5 +281,27 @@ describe('CREATE project template', () => {
           done();
         });
     });
+
+    it('should return 422 if both scope and form are defined', (done) => {
+      request(server)
+        .post('/v4/projects/metadata/projectTemplates')
+        .set({
+          Authorization: `Bearer ${testUtil.jwts.admin}`,
+        })
+        .send(bodyDefinedFormScope)
+        .expect('Content-Type', /json/)
+        .expect(422, done);
+    });
+
+    it('should return 422 if both scope and form are missing', (done) => {
+      request(server)
+        .post('/v4/projects/metadata/projectTemplates')
+        .set({
+          Authorization: `Bearer ${testUtil.jwts.admin}`,
+        })
+        .send(bodyMissingFormScope)
+        .expect('Content-Type', /json/)
+        .expect(422, done);
+    });
   });
 });
diff --git a/src/routes/projectTemplates/update.js b/src/routes/projectTemplates/update.js
@@ -25,11 +25,11 @@ const schema = {
       question: Joi.string().max(255),
       info: Joi.string().max(255),
       aliases: Joi.array(),
-      scope: Joi.object().optional().allow(null),
-      phases: Joi.object().optional().allow(null),
-      form: Joi.object().optional().allow(null),
-      planConfig: Joi.object().optional().allow(null),
-      priceConfig: Joi.object().optional().allow(null),
+      scope: Joi.object().empty(null),
+      phases: Joi.object().empty(null),
+      form: Joi.object().empty(null),
+      planConfig: Joi.object().empty(null),
+      priceConfig: Joi.object().empty(null),
       disabled: Joi.boolean().optional(),
       hidden: Joi.boolean().optional(),
       createdAt: Joi.any().strip(),
@@ -38,7 +38,11 @@ const schema = {
       createdBy: Joi.any().strip(),
       updatedBy: Joi.any().strip(),
       deletedBy: Joi.any().strip(),
-    }).required(),
+    })
+      .xor('form', 'scope')
+      .xor('phases', 'planConfig')
+      .nand('priceConfig', 'scope')
+      .required(),
   },
 };
 
@@ -50,54 +54,12 @@ module.exports = [
     const param = req.body.param;
     const { form, priceConfig, planConfig } = param;
 
-    const checkModel = (keyInfo, modelName, model) => {
-      let errorMessage = '';
-      if (keyInfo == null) {
-        return Promise.resolve(null);
-      }
-      if ((keyInfo.version != null) && (keyInfo.key != null)) {
-        errorMessage = `${modelName} with key ${keyInfo.key} and version ${keyInfo.version}`
-          + ' referred in the project template is not found';
-        return (model.findOne({
-          where: {
-            key: keyInfo.key,
-            version: keyInfo.version,
-          },
-        })).then((record) => {
-          if (record == null) {
-            return Promise.resolve(errorMessage);
-          }
-          return Promise.resolve(null);
-        });
-      } else if ((keyInfo.version == null) && (keyInfo.key != null)) {
-        errorMessage = `${modelName} with key ${keyInfo.key}`
-          + ' referred in the project template is not found';
-        return model.findOne({
-          where: {
-            key: keyInfo.key,
-          },
-        }).then((record) => {
-          if (record == null) {
-            return Promise.resolve(errorMessage);
-          }
-          return Promise.resolve(null);
-        });
-      }
-      return Promise.resolve(null);
-    };
-
     return Promise.all([
-      checkModel(form, 'Form', models.Form, next),
-      checkModel(priceConfig, 'PriceConfig', models.PriceConfig, next),
-      checkModel(planConfig, 'PlanConfig', models.PlanConfig, next),
+      util.checkModel(form, 'Form', models.Form, 'project template'),
+      util.checkModel(priceConfig, 'PriceConfig', models.PriceConfig, 'project template'),
+      util.checkModel(planConfig, 'PlanConfig', models.PlanConfig, 'project template'),
     ])
-        .then((errorMessages) => {
-          const errorMessage = errorMessages.find(e => e && e.length > 0);
-          if (errorMessage) {
-            const apiErr = new Error(errorMessage);
-            apiErr.status = 422;
-            throw apiErr;
-          }
+        .then(() => {
           const entityToUpdate = _.assign(req.body.param, {
             updatedBy: req.authUser.userId,
           });
@@ -115,7 +77,7 @@ module.exports = [
             if (!projectTemplate) {
               const apiErr = new Error(`Project template not found for template id ${req.params.templateId}`);
               apiErr.status = 404;
-              return Promise.reject(apiErr);
+              throw apiErr;
             }
 
             // Merge JSON fields
@@ -132,9 +94,7 @@ module.exports = [
           })
           .then((projectTemplate) => {
             res.json(util.wrapResponse(req.id, projectTemplate));
-            return Promise.resolve();
-          })
-          .catch(next);
+          });
         }).catch(next);
   },
 ];
diff --git a/src/routes/projectTemplates/update.spec.js b/src/routes/projectTemplates/update.spec.js
diff --git a/src/routes/projectTemplates/upgrade.js b/src/routes/projectTemplates/upgrade.js
