feat: add validation results to comments

.git2gus/config.json

@@ -1,6 +1,6 @@
 {
     "productTag": "a1aEE000000OVXJYA4",
     "defaultBuild": "Backlog",
-    "hideWorkItemUrl": "true",
+    "hideWorkItemUrl": true,
     "statusWhenClosed": "FIXED"
 }

api/__test__/policies.hasConfig.spec.js

@@ -150,7 +150,13 @@ describe('hasConfig policy', () => {
         getConfig.mockReturnValue(
             Promise.reject({
                 status: 403,
-                message: 'get config error'
+                message: 'get config error',
+                errors: [
+                    {
+                        field: 'mmmbop',
+                        message: 'ba duba bop ba'
+                    }
+                ]
             })
         );
         res.status.mockReset();
@@ -171,22 +177,34 @@ describe('hasConfig policy', () => {
         await hasConfig(req, res, next);
         expect(createComment).toHaveBeenCalledWith({
             req,
-            body: `Git2Gus App is installed but the \`.git2gus/config.json\` doesn't have right values. You should add the required configuration.`
+            body: [
+                'The Git2Gus app is installed but the `.git2gus/config.json` failed validation.',
+                '## Errors:',
+                '- **`mmmbop`**: ba duba bop ba'
+            ].join('\n\n')
         });
         expect(next).not.toHaveBeenCalled();
         expect(res.status).toHaveBeenCalledWith(403);
-        expect(res.status().send).toHaveBeenCalledWith({
-            status: 403,
-            message: 'get config error'
-        });
+        expect(res.status().send).toHaveBeenCalledWith(
+            expect.objectContaining({
+                status: 403,
+                message: 'get config error'
+            })
+        );
     });
     it('should call createComment with the right values when error status is other than 404 and a pull request is opened', async () => {
         expect.assertions(4);
         getConfig.mockReset();
         getConfig.mockReturnValue(
             Promise.reject({
                 status: 403,
-                message: 'get config error'
+                message: 'get config error',
+                warnings: [
+                    {
+                        field: 'oops',
+                        message: "that's not right"
+                    }
+                ]
             })
         );
         res.status.mockReset();
@@ -207,14 +225,18 @@ describe('hasConfig policy', () => {
         await hasConfig(req, res, next);
         expect(createComment).toHaveBeenCalledWith({
             req,
-            body: `Git2Gus App is installed but the \`.git2gus/config.json\` doesn't have right values. You should add the required configuration.`
+            body: expect.stringContaining(
+                'The Git2Gus app is installed but the `.git2gus/config.json` failed validation.'
+            )
         });
         expect(next).not.toHaveBeenCalled();
         expect(res.status).toHaveBeenCalledWith(403);
-        expect(res.status().send).toHaveBeenCalledWith({
-            status: 403,
-            message: 'get config error'
-        });
+        expect(res.status().send).toHaveBeenCalledWith(
+            expect.objectContaining({
+                status: 403,
+                message: 'get config error'
+            })
+        );
     });
     it('should respond with status 403 when error.status is other than 404', async () => {
         expect.assertions(4);

api/actions/__test__/createWorkItem.spec.js

@@ -532,7 +532,7 @@ describe('createGusItem action', () => {
         });
     });
 
-    it('should create a comment without the url when the git2gus.config.hideWorkItemUrl = true', async () => {
+    it('should create a comment without the url when the git2gus.config.hideWorkItemUrl = "true"', async () => {
         Github.getRecordTypeId.mockReturnValue('bug');
         Github.isSalesforceLabel.mockReturnValue(true);
         Builds.resolveBuild.mockReturnValue(Promise.resolve('qwerty1234'));
@@ -553,4 +553,26 @@ describe('createGusItem action', () => {
             body: `This issue has been linked to a new work item: new-work-item`
         });
     });
+
+    it('should create a comment without the url when the git2gus.config.hideWorkItemUrl = true', async () => {
+        Github.getRecordTypeId.mockReturnValue('bug');
+        Github.isSalesforceLabel.mockReturnValue(true);
+        Builds.resolveBuild.mockReturnValue(Promise.resolve('qwerty1234'));
+        formatToGus.formatToGus.mockReturnValue(
+            Promise.resolve('Body In Gus format')
+        );
+        Gus.resolveBuild.mockReturnValue(Promise.resolve('229'));
+        Gus.getByRelatedUrl.mockReturnValue(Promise.resolve(''));
+        Gus.getBugRecordTypeId.mockReturnValue(Promise.resolve('bug'));
+        const workItem = { id: '12345', Name: 'new-work-item' };
+        Gus.createWorkItemInGus.mockReturnValue(workItem);
+        Gus.getById.mockReturnValue(workItem);
+        const req1 = JSON.parse(JSON.stringify(req));
+        req1.git2gus.config.hideWorkItemUrl = true;
+        await fn(req1);
+        expect(Github.createComment).toHaveBeenCalledWith({
+            req: req1,
+            body: `This issue has been linked to a new work item: new-work-item`
+        });
+    });
 });

api/actions/__test__/createWorkItemForPR.spec.js

@@ -442,7 +442,7 @@ describe('createGusItem action', () => {
         });
     });
 
-    it('should create a comment without the url when the git2gus.config.hideWorkItemUrl = true', async () => {
+    it('should create a comment without the url when the git2gus.config.hideWorkItemUrl = "true"', async () => {
         expect.assertions(1);
         Github.getRecordTypeId.mockReturnValue('bug');
         Github.isSalesforceLabel.mockReturnValue(true);
@@ -464,4 +464,27 @@ describe('createGusItem action', () => {
             body: `This issue has been linked to a new work item: new-work-item`
         });
     });
+
+    it('should create a comment without the url when the git2gus.config.hideWorkItemUrl = true', async () => {
+        expect.assertions(1);
+        Github.getRecordTypeId.mockReturnValue('bug');
+        Github.isSalesforceLabel.mockReturnValue(true);
+        Builds.resolveBuild.mockReturnValue(Promise.resolve('qwerty1234'));
+        formatToGus.formatToGus.mockReturnValue(
+            Promise.resolve('Body In Gus format')
+        );
+        Gus.resolveBuild.mockReturnValue(Promise.resolve('229'));
+        Gus.getByRelatedUrl.mockReturnValue(Promise.resolve(''));
+        Gus.getBugRecordTypeId.mockReturnValue(Promise.resolve('bug'));
+        const workItem = { id: '12345', Name: 'new-work-item' };
+        Gus.createWorkItemInGus.mockReturnValue(workItem);
+        Gus.getById.mockReturnValue(workItem);
+        const req1 = JSON.parse(JSON.stringify(req));
+        req1.git2gus.config.hideWorkItemUrl = true;
+        await fn(req1);
+        expect(Github.createComment).toHaveBeenCalledWith({
+            req: req1,
+            body: `This issue has been linked to a new work item: new-work-item`
+        });
+    });
 });

api/actions/createWorkItem.js

@@ -29,7 +29,10 @@ module.exports = {
         // Only grab the label being added for comparison against Salesforce labels
         const { label: labelAdded } = req.body;
         const { config } = req.git2gus;
-        const { hideWorkItemUrl } = config;
+        const hideWorkItemUrl =
+            typeof config.hideWorkItemUrl === 'boolean'
+                ? String(config.hideWorkItemUrl)
+                : config.hideWorkItemUrl;
         let productTag = config.productTag;
         if (config.productTagLabels) {
             logger.info('createWorkItem will work with custom productTagLabels for issue titled: ', title);

api/actions/createWorkItemForPR.js

@@ -28,7 +28,10 @@ module.exports = {
         // Only grab the label being added for comparison against Salesforce labels
         const { label: labelAdded } = req.body;
         const { config } = req.git2gus;
-        const { hideWorkItemUrl } = config;
+        const hideWorkItemUrl =
+            typeof config.hideWorkItemUrl === 'boolean'
+                ? String(config.hideWorkItemUrl)
+                : config.hideWorkItemUrl;
         let productTag = config.productTag;
         if (config.productTagLabels) {
             console.log(

api/policies/hasConfig.js

@@ -9,6 +9,16 @@ const { getConfig, createComment } = require('../services/Github');
 const { github } = require('../../config/github');
 const logger = require('../services/Logs/logger');
 
+function toMarkdown(errors) {
+    if (!errors || errors.length === 0) {
+        return '';
+    }
+    const formatted = errors
+        .map(({ field, message }) => `- **\`${field}\`**: ${message}`)
+        .join('\n');
+    return `\n\n## Errors:\n\n${formatted}`;
+}
+
 module.exports = async function hasConfig(req, res, next) {
     const { action, repository } = req.body;
     const event = req.headers['x-github-event'];
@@ -55,7 +65,9 @@ module.exports = async function hasConfig(req, res, next) {
         if (isIssueOrPrOpened) {
             await createComment({
                 req,
-                body: `Git2Gus App is installed but the \`.git2gus/config.json\` doesn't have right values. You should add the required configuration.`
+                body: `The Git2Gus app is installed but the \`.git2gus/config.json\` failed validation.${toMarkdown(
+                    error.errors
+                )}${toMarkdown(error.warnings)}`
             });
         }
         return res.status(403).send(error);

api/services/Builds/__test__/resolveBuild.spec.js

@@ -14,16 +14,23 @@ const config = {
 };
 
 describe('resolveBuild builds service', () => {
-    it('should call getBuildByName with the right value when a milsetone is passed', () => {
+    let consoleSpy;
+    beforeAll(() => {
+        consoleSpy = jest.spyOn(console, 'log').mockReturnValue();
+    });
+    afterAll(() => {
+        consoleSpy.mockRestore();
+    });
+    it('should call getBuildByName with the right value when a milsetone is passed', async () => {
         const milestone = {
             title: 218
         };
-        resolveBuild(config, milestone);
+        await resolveBuild(config, milestone);
         expect(getBuildByName).toHaveBeenCalledWith(218);
     });
-    it('should call getBuildByName with the right value when a milsetone is not passed', () => {
+    it('should call getBuildByName with the right value when a milsetone is not passed', async () => {
         getBuildByName.mockReset();
-        resolveBuild(config);
+        await resolveBuild(config);
         expect(getBuildByName).toHaveBeenCalledWith(220);
     });
     it('should return the right build', async () => {

api/services/ConfigValidator/__test__/index.spec.js

@@ -58,7 +58,7 @@ describe('ConfigValidator service', () => {
                 productTag: 'a1aB0000000Jgm7IAC',
                 defaultBuild: 'Backlog',
                 statusWhenClosed: 'FIXED',
-                hideWorkItemUrl: 'true',
+                hideWorkItemUrl: true,
                 issueTypeLabels: {
                     feature: 'USER STORY',
                     urgent: 'BUG P0'
@@ -275,7 +275,7 @@ describe('ConfigValidator service', () => {
             });
         });
 
-        it('should warn about non-standard hideWorkItemUrl values', () => {
+        it('should reject non-standard hideWorkItemUrl values', () => {
             const config = JSON.stringify({
                 productTag: 'a1aB0000000Jgm7IAC',
                 defaultBuild: '218',
@@ -284,10 +284,10 @@ describe('ConfigValidator service', () => {
 
             const result = ConfigValidator.validate(config);
 
-            expect(result.valid).toBe(true);
-            expect(result.warnings).toContainEqual({
+            expect(result.valid).toBe(false);
+            expect(result.errors).toContainEqual({
                 field: 'hideWorkItemUrl',
-                message: expect.stringContaining('should be "true" or "false"')
+                message: 'Field "hideWorkItemUrl" must be a boolean value.'
             });
         });
     });

api/services/ConfigValidator/index.js

@@ -13,6 +13,13 @@
  */
 
 const VALID_STATUS_VALUES = ['INTEGRATE', 'FIXED', 'CLOSED'];
+const VALID_HAS_WORK_ITEM_URL_VALUES = [
+    undefined,
+    true,
+    false,
+    'true',
+    'false'
+];
 
 module.exports = {
     /**
@@ -148,21 +155,11 @@ module.exports = {
         }
 
         // hideWorkItemUrl
-        if (config.hideWorkItemUrl !== undefined) {
-            if (typeof config.hideWorkItemUrl !== 'string') {
-                errors.push({
-                    field: 'hideWorkItemUrl',
-                    message: 'Field "hideWorkItemUrl" must be a string.'
-                });
-            } else if (
-                config.hideWorkItemUrl !== 'true' &&
-                config.hideWorkItemUrl !== 'false'
-            ) {
-                warnings.push({
-                    field: 'hideWorkItemUrl',
-                    message: `Field "hideWorkItemUrl" should be "true" or "false" (as strings). Got: "${config.hideWorkItemUrl}"`
-                });
-            }
+        if (!VALID_HAS_WORK_ITEM_URL_VALUES.includes(config.hideWorkItemUrl)) {
+            errors.push({
+                field: 'hideWorkItemUrl',
+                message: 'Field "hideWorkItemUrl" must be a boolean value.'
+            });
         }
 
         // issueTypeLabels

api/services/Github/__test__/getConfig.spec.js

@@ -71,7 +71,19 @@ describe('getConfig github service', () => {
         );
         return expect(getConfig(payload)).rejects.toEqual({
             status: 'BAD_CONFIG_FILE',
-            message: 'Wrong config received.'
+            message: 'Wrong config received.',
+            valid: false,
+            errors: [
+                {
+                    field: 'defaultBuild',
+                    message:
+                        'Required field "defaultBuild" is missing or empty. Add: "defaultBuild": "218" (your sprint/build number)'
+                }
+            ],
+            warnings: [],
+            config: {
+                productTag: 'abcd1234'
+            }
         });
     });
     it('should rejects when there is not productTag', () => {
@@ -89,7 +101,17 @@ describe('getConfig github service', () => {
         );
         return expect(getConfig(payload)).rejects.toEqual({
             status: 'BAD_CONFIG_FILE',
-            message: 'Wrong config received.'
+            message: 'Wrong config received.',
+            valid: false,
+            errors: [
+                {
+                    field: 'productTag',
+                    message:
+                        'At least one of "productTag" or "productTagLabels" is required. Add: "productTag": "a1aB0000000Jgm7IAC"'
+                }
+            ],
+            warnings: [],
+            config: { defaultBuild: '218' }
         });
     });
     it('should rejects when the content is null', () => {
@@ -105,7 +127,17 @@ describe('getConfig github service', () => {
         );
         return expect(getConfig(payload)).rejects.toEqual({
             status: 'BAD_CONFIG_FILE',
-            message: 'Wrong config received.'
+            message: 'Wrong config received.',
+            valid: false,
+            errors: [
+                {
+                    field: 'root',
+                    message:
+                        'Configuration must be a JSON object, not null, string, or array.'
+                }
+            ],
+            warnings: [],
+            config: null
         });
     });
     it('should rejects when the content is not an object', () => {
@@ -121,7 +153,17 @@ describe('getConfig github service', () => {
         );
         return expect(getConfig(payload)).rejects.toEqual({
             status: 'BAD_CONFIG_FILE',
-            message: 'Wrong config received.'
+            message: 'Wrong config received.',
+            valid: false,
+            errors: [
+                {
+                    field: 'root',
+                    message:
+                        'Configuration must be a JSON object, not null, string, or array.'
+                }
+            ],
+            warnings: [],
+            config: null
         });
     });
 });