diff --git a/.git2gus/config.json b/.git2gus/config.json index adc969c..ab8db5a 100644 --- a/.git2gus/config.json +++ b/.git2gus/config.json @@ -1,6 +1,6 @@ { "productTag": "a1aEE000000OVXJYA4", "defaultBuild": "Backlog", - "hideWorkItemUrl": "true", + "hideWorkItemUrl": true, "statusWhenClosed": "FIXED" } diff --git a/api/__test__/policies.hasConfig.spec.js b/api/__test__/policies.hasConfig.spec.js index df71301..281fba3 100644 --- a/api/__test__/policies.hasConfig.spec.js +++ b/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,14 +177,20 @@ 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); @@ -186,7 +198,13 @@ describe('hasConfig policy', () => { 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); diff --git a/api/actions/__test__/createWorkItem.spec.js b/api/actions/__test__/createWorkItem.spec.js index ea71a55..a3d5afe 100644 --- a/api/actions/__test__/createWorkItem.spec.js +++ b/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` + }); + }); }); diff --git a/api/actions/__test__/createWorkItemForPR.spec.js b/api/actions/__test__/createWorkItemForPR.spec.js index cc90522..f933c3f 100644 --- a/api/actions/__test__/createWorkItemForPR.spec.js +++ b/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` + }); + }); }); diff --git a/api/actions/createWorkItem.js b/api/actions/createWorkItem.js index ee4fde4..c24d7f4 100644 --- a/api/actions/createWorkItem.js +++ b/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); diff --git a/api/actions/createWorkItemForPR.js b/api/actions/createWorkItemForPR.js index 626339f..4adcb5b 100644 --- a/api/actions/createWorkItemForPR.js +++ b/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( diff --git a/api/policies/hasConfig.js b/api/policies/hasConfig.js index ef77523..270cc09 100644 --- a/api/policies/hasConfig.js +++ b/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); diff --git a/api/services/Builds/__test__/resolveBuild.spec.js b/api/services/Builds/__test__/resolveBuild.spec.js index ed68a8b..75e7cc5 100644 --- a/api/services/Builds/__test__/resolveBuild.spec.js +++ b/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 () => { diff --git a/api/services/ConfigValidator/__test__/index.spec.js b/api/services/ConfigValidator/__test__/index.spec.js index 5eed97b..44cca2b 100644 --- a/api/services/ConfigValidator/__test__/index.spec.js +++ b/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.' }); }); }); diff --git a/api/services/ConfigValidator/index.js b/api/services/ConfigValidator/index.js index 55fe3db..19ca3ba 100644 --- a/api/services/ConfigValidator/index.js +++ b/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 diff --git a/api/services/Github/__test__/getConfig.spec.js b/api/services/Github/__test__/getConfig.spec.js index 85f6e7a..a3a1d0e 100644 --- a/api/services/Github/__test__/getConfig.spec.js +++ b/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 }); }); }); diff --git a/api/services/Github/getConfig.js b/api/services/Github/getConfig.js index 8fa293b..2438515 100644 --- a/api/services/Github/getConfig.js +++ b/api/services/Github/getConfig.js @@ -25,6 +25,7 @@ module.exports = async function getConfig({ octokitClient, owner, repo }) { return Promise.reject({ status: 'BAD_CONFIG_FILE', - message: 'Wrong config received.' + message: 'Wrong config received.', + ...validationResult }); }; diff --git a/views/pages/config-validator.ejs b/views/pages/config-validator.ejs index 7c7f864..16edf0f 100644 --- a/views/pages/config-validator.ejs +++ b/views/pages/config-validator.ejs @@ -114,7 +114,7 @@ const ADVANCED_EXAMPLE = { "productTag": "a1aB0000000Jgm7IAC", "defaultBuild": "Backlog", "statusWhenClosed": "FIXED", - "hideWorkItemUrl": "true", + "hideWorkItemUrl": true, "productTagLabels": { "area:lwc": "a1aEE000000OVXJYA4", "area:aura": "a1aEE000000OVXKYA4"