diff --git a/src/cli/usage.js b/src/cli/usage.js index 34cf3cfb9..da0d44145 100644 --- a/src/cli/usage.js +++ b/src/cli/usage.js @@ -63,6 +63,9 @@ ${bold('OPTIONS')} --skip-translation-check Skips checking message translations + --skip-validate + Skips form validation + --force CAN BE DANGEROUS! Passes yes to all commands and any where that would prompt to overwrite changes will overwrite automatically. `); diff --git a/src/fn/validate-app-forms.js b/src/fn/validate-app-forms.js new file mode 100644 index 000000000..9dcfc753e --- /dev/null +++ b/src/fn/validate-app-forms.js @@ -0,0 +1,12 @@ +const validateForms = require('../lib/validate-forms'); +const environment = require('../lib/environment'); + +const validateAppForms = (forms) => { + return validateForms(environment.pathToProject, 'app', { forms }); +}; + +module.exports = { + requiresInstance: false, + validateAppForms, + execute: () => validateAppForms(environment.extraArgs) +}; diff --git a/src/fn/validate-collect-forms.js b/src/fn/validate-collect-forms.js new file mode 100644 index 000000000..546438a0c --- /dev/null +++ b/src/fn/validate-collect-forms.js @@ -0,0 +1,12 @@ +const validateForms = require('../lib/validate-forms'); +const environment = require('../lib/environment'); + +const validateCollectForms = (forms) => { + return validateForms(environment.pathToProject, 'collect', { forms }); +}; + +module.exports = { + requiresInstance: false, + validateCollectForms, + execute: () => validateCollectForms(environment.extraArgs) +}; diff --git a/src/fn/validate-contact-forms.js b/src/fn/validate-contact-forms.js new file mode 100644 index 000000000..84279ad58 --- /dev/null +++ b/src/fn/validate-contact-forms.js @@ -0,0 +1,12 @@ +const validateForms = require('../lib/validate-forms'); +const environment = require('../lib/environment'); + +const validateContactForms = (forms) => { + return validateForms(environment.pathToProject, 'contact', { forms }); +}; + +module.exports = { + requiresInstance: false, + validateContactForms, + execute: () => validateContactForms(environment.extraArgs) +}; diff --git a/src/fn/watch-project.js b/src/fn/watch-project.js index 05f1f2009..7a52ca2dd 100644 --- a/src/fn/watch-project.js +++ b/src/fn/watch-project.js @@ -5,6 +5,9 @@ const fs = require('fs'); const { error, warn, info } = require('../lib/log'); const Queue = require('queue-promise'); const watcher = require('@parcel/watcher'); +const { validateAppForms } = require('./validate-app-forms'); +const { validateContactForms } = require('./validate-contact-forms'); +const { validateCollectForms } = require('./validate-collect-forms'); const { uploadAppForms } = require('./upload-app-forms'); const { uploadContactForms } = require('./upload-contact-forms'); const { uploadCollectForms } = require('./upload-collect-forms'); @@ -25,8 +28,17 @@ const watcherEvents = { UpdateEvent: 'update' }; +const runValidation = (validation, forms) => { + if(environment.skipValidate) { + return; + } + return validation(forms); +}; + const uploadInitialState = async (api) => { await uploadResources(); + await runValidation(validateAppForms, environment.extraArgs); + await runValidation(validateContactForms, environment.extraArgs); await uploadAppForms(environment.extraArgs); await uploadContactForms(environment.extraArgs); await uploadCustomTranslations(); @@ -82,6 +94,7 @@ const processAppForm = (eventType, fileName) => { let form = uploadForms.formFileMatcher(fileName); if (form) { eventQueue.enqueue(async () => { + await runValidation(validateAppForms, [form]); await uploadAppForms([form]); return fileName; }); @@ -103,6 +116,7 @@ const processAppFormMedia = (formMediaDir, fileName) => { const form = uploadForms.formMediaMatcher(formMediaDir); if (form) { eventQueue.enqueue(async () => { + await runValidation(validateAppForms,[form]); await uploadAppForms([form]); return fileName; }); @@ -127,6 +141,7 @@ const processContactForm = (eventType, fileName) => { form = uploadForms.formFileMatcher(fileName); if (form) { eventQueue.enqueue(async () => { + await runValidation(validateContactForms,[form]); await uploadContactForms([form]); return fileName; }); @@ -142,6 +157,7 @@ const processCollectForm = (eventType, fileName) => { let form = uploadForms.formFileMatcher(fileName); if (form) { eventQueue.enqueue(async () => { + await runValidation(validateCollectForms,[form]); await uploadCollectForms([form]); return fileName; }); diff --git a/src/lib/environment.js b/src/lib/environment.js index 31a659ad4..b336e1cff 100644 --- a/src/lib/environment.js +++ b/src/lib/environment.js @@ -12,7 +12,8 @@ const initialize = ( extraArgs, apiUrl, force, - skipTranslationCheck + skipTranslationCheck, + skipValidate ) => { if (state.initialized) { throw Error('environment is already initialized'); @@ -26,7 +27,8 @@ const initialize = ( isArchiveMode, pathToProject, force, - skipTranslationCheck + skipTranslationCheck, + skipValidate }); }; @@ -51,6 +53,7 @@ module.exports = { get apiUrl() { return getState('apiUrl'); }, get force() { return getState('force'); }, get skipTranslationCheck() { return getState('skipTranslationCheck'); }, + get skipValidate() { return getState('skipValidate'); }, /** * Return `true` if the environment **seems** to be production. diff --git a/src/lib/main.js b/src/lib/main.js index a842953c5..5b92c661c 100755 --- a/src/lib/main.js +++ b/src/lib/main.js @@ -21,6 +21,9 @@ const defaultActions = [ 'convert-app-forms', 'convert-collect-forms', 'convert-contact-forms', + 'validate-app-forms', + 'validate-collect-forms', + 'validate-contact-forms', 'backup-all-forms', 'delete-all-forms', 'upload-app-forms', @@ -38,6 +41,9 @@ const defaultArchiveActions = [ 'convert-app-forms', 'convert-collect-forms', 'convert-contact-forms', + 'validate-app-forms', + 'validate-collect-forms', + 'validate-contact-forms', 'upload-app-forms', 'upload-collect-forms', 'upload-contact-forms', @@ -123,19 +129,40 @@ module.exports = async (argv, env) => { // Build up actions // let actions = cmdArgs._; - if (!actions.length) { + if (actions.length) { + const unsupported = actions.filter(a => !supportedActions.includes(a)); + if(unsupported.length) { + throw new Error(`Unsupported action(s): ${unsupported.join(' ')}`); + } + } else { actions = !cmdArgs.archive ? defaultActions : defaultArchiveActions; } - const unsupported = actions.filter(a => !supportedActions.includes(a)); - if(unsupported.length) { - throw new Error(`Unsupported action(s): ${unsupported.join(' ')}`); - } - if (cmdArgs['skip-git-check']) { actions = actions.filter(a => a !== 'check-git'); } + const skipValidate = cmdArgs['skip-validate']; + if(skipValidate) { + warn('Skipping all form validation.'); + const validateActions = [ + 'validate-app-forms', + 'validate-collect-forms', + 'validate-contact-forms' + ]; + actions = actions.filter(action => !validateActions.includes(action)); + } else { + const addFormValidationIfNecessary = (formType) => { + const updateFormsIndex = actions.indexOf(`upload-${formType}-forms`); + if (updateFormsIndex >= 0 && actions.indexOf(`validate-${formType}-forms`) < 0) { + actions.splice(updateFormsIndex, 0, `validate-${formType}-forms`); + } + }; + addFormValidationIfNecessary('app'); + addFormValidationIfNecessary('collect'); + addFormValidationIfNecessary('contact'); + } + actions = actions.map(actionName => { const action = require(`../fn/${actionName}`); @@ -157,9 +184,9 @@ module.exports = async (argv, env) => { // const projectName = fs.path.basename(pathToProject); - let apiUrl; - if (actions.some(action => action.requiresInstance)) { - apiUrl = getApiUrl(cmdArgs, env); + const apiUrl = getApiUrl(cmdArgs, env); + const requiresInstance = actions.some(action => action.requiresInstance); + if (requiresInstance) { if (!apiUrl) { throw new Error('Failed to obtain a url to the API'); } @@ -177,10 +204,11 @@ module.exports = async (argv, env) => { extraArgs, apiUrl, cmdArgs.force, - cmdArgs['skip-translation-check'] + cmdArgs['skip-translation-check'], + skipValidate ); - if (apiUrl) { + if (requiresInstance && apiUrl) { await api().available(); } diff --git a/src/lib/upload-forms.js b/src/lib/upload-forms.js index a821fd6ae..dfb675153 100644 --- a/src/lib/upload-forms.js +++ b/src/lib/upload-forms.js @@ -13,7 +13,6 @@ const { readTitleFrom, readIdFrom } = require('./forms-utils'); -const validateForms = require('./validate-forms'); const SUPPORTED_PROPERTIES = ['context', 'icon', 'title', 'xml2sms', 'subject_key', 'hidden_fields']; const FORM_EXTENSTION = '.xml'; @@ -40,7 +39,6 @@ const formMediaMatcher = (formMediaDir) => { }; const execute = async (projectDir, subDirectory, options) => { - await validateForms(projectDir, subDirectory, options); const db = pouch(); if (!options) options = {}; const formsDir = getFormDir(projectDir, subDirectory); diff --git a/src/lib/validate-forms.js b/src/lib/validate-forms.js index 283568a7e..e321f9405 100644 --- a/src/lib/validate-forms.js +++ b/src/lib/validate-forms.js @@ -1,64 +1,82 @@ -const api = require('./api'); const argsFormFilter = require('./args-form-filter'); +const environment = require('./environment'); const fs = require('./sync-fs'); const log = require('./log'); const { getFormDir, - getFormFilePaths, - formHasInstanceId + getFormFilePaths } = require('./forms-utils'); +const VALIDATIONS_PATH = fs.path.resolve(__dirname, './validation/form'); +const validations = fs.readdir(VALIDATIONS_PATH) + .filter(name => name.endsWith('.js')) + .map(validationName => { + const validation = require(fs.path.join(VALIDATIONS_PATH, validationName)); + validation.name = validationName; + if(!Object.hasOwnProperty.call(validation, 'requiresInstance')) { + validation.requiresInstance = true; + } + if(!Object.hasOwnProperty.call(validation, 'skipFurtherValidation')) { + validation.skipFurtherValidation = false; + } + + return validation; + }) + .sort((a, b) => { + if(a.skipFurtherValidation && !b.skipFurtherValidation) { + return -1; + } + if(!a.skipFurtherValidation && b.skipFurtherValidation) { + return 1; + } + return a.name.localeCompare(b.name); + }); + module.exports = async (projectDir, subDirectory, options={}) => { const formsDir = getFormDir(projectDir, subDirectory); if(!formsDir) { - log.info(`Forms dir not found: ${formsDir}`); + log.info(`Forms dir not found: ${projectDir}/forms/${subDirectory}`); return; } - let idValidationsPassed = true; - let validateFormsPassed = true; + const instanceProvided = environment.apiUrl; + let validationSkipped = false; const fileNames = argsFormFilter(formsDir, '.xml', options); - for (const fileName of fileNames) { + + let errorFound = false; + for(const fileName of fileNames) { log.info(`Validating form: ${fileName}…`); - const { xformPath, filePath } = getFormFilePaths(formsDir, fileName); + const { xformPath } = getFormFilePaths(formsDir, fileName); const xml = fs.read(xformPath); - if(!formHasInstanceId(xml)) { - log.error(`Form at ${xformPath} appears to be missing node. This form will not work on CHT webapp.`); - idValidationsPassed = false; - continue; - } - try { - await _formsValidate(xml); - } catch (err) { - log.error(`Error found while validating "${filePath}". Validation response: ${err.message}`); - validateFormsPassed = false; + const valParams = { xformPath, xmlStr: xml }; + for(const validation of validations) { + if(validation.requiresInstance && !instanceProvided) { + validationSkipped = true; + continue; + } + + const output = await validation.execute(valParams); + if(output.warnings) { + output.warnings.forEach(warnMsg => log.warn(warnMsg)); + } + if(output.errors && output.errors.length) { + output.errors.forEach(errorMsg => log.error(errorMsg)); + errorFound = true; + if(validation.skipFurtherValidation) { + break; + } + } } } - // Once all the fails were checked raise an exception if there were errors - let errors = []; - if (!validateFormsPassed) { - errors.push('One or more forms appears to have errors found by the API validation endpoint.'); - } - if (!idValidationsPassed) { - errors.push('One or more forms appears to be missing node.'); - } - if (errors.length) { - // the blank spaces are a trick to align the errors in the log ;) - throw new Error(errors.join('\n ')); - } -}; -let validateEndpointNotFoundLogged = false; -const _formsValidate = async (xml) => { - const resp = await api().formsValidate(xml); - if (resp.formsValidateEndpointFound === false && !validateEndpointNotFoundLogged) { - log.warn('Form validation endpoint not found in your version of CHT Core, ' + - 'no form will be checked before push'); - validateEndpointNotFoundLogged = true; // Just log the message once + if(validationSkipped) { + log.warn('Some validations have been skipped because they require a CHT instance.'); + } + if(errorFound) { + throw new Error('One or more forms have failed validation.'); } - return resp; }; diff --git a/src/lib/validation/form/README.md b/src/lib/validation/form/README.md new file mode 100644 index 000000000..3a4cd6b9c --- /dev/null +++ b/src/lib/validation/form/README.md @@ -0,0 +1,32 @@ +# Form validations + +Form validations should be added to this directory. + +Their module should look something like this: + +```js +module.exports = { + requiresInstance: false, + skipFurtherValidation: true, + execute: async ({ xformPath, xmlStr }) => { + ... + } +}; +``` + +## Module Exports + +| Field | Required | Notes | +|-------------------------|--------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `requiresInstance` | Optional, defaults to `true` | The validation needs the user to have provided a instance location, e.g. via `--local` or `--instance` | +| `skipFurtherValidation` | Optional, defaults to `false` | When `true`, additional validations will not be performed on a form that fails the current validation. The goal is to avoid unnecessary noise in the validation log for forms that are grossly invalid (e.g. XML files that are not actually xForms at all). | +| `execute` | Required | The function that is run when the validation is executed. The provided input argument contains the form XML string. | + +## Result + +The result has the following format: + +| Field | Notes | +|----------|----------------------------------------------------------------------------------------------------| +| warnings | Array containing the descriptions of any warnings. | +| errors | Array containing the descriptions of any errors. If this is empty, the form has passed validation. | diff --git a/src/lib/validation/form/can-generate-xform.js b/src/lib/validation/form/can-generate-xform.js new file mode 100644 index 000000000..0fc058364 --- /dev/null +++ b/src/lib/validation/form/can-generate-xform.js @@ -0,0 +1,18 @@ +const api = require('../../api'); + +module.exports = { + execute: async ({ xformPath, xmlStr }) => { + const warnings = []; + const errors = []; + try { + const resp = await api().formsValidate(xmlStr); + if (resp.formsValidateEndpointFound === false) { + warnings.push('Form validation endpoint not found in your version of CHT Core, no form will be checked before push'); + } + } catch (err) { + errors.push(`Error found while validating "${xformPath}". Validation response: ${err.message}`); + } + + return { errors, warnings }; + } +}; diff --git a/src/lib/validation/form/has-instance-id.js b/src/lib/validation/form/has-instance-id.js new file mode 100644 index 000000000..36642aafb --- /dev/null +++ b/src/lib/validation/form/has-instance-id.js @@ -0,0 +1,14 @@ +const { formHasInstanceId } = require('../../forms-utils'); + +module.exports = { + requiresInstance: false, + skipFurtherValidation: true, + execute: async({ xformPath, xmlStr }) => { + const errors = []; + if(!formHasInstanceId(xmlStr)) { + errors.push(`Form at ${xformPath} appears to be missing node. This form will not work on CHT webapp.`); + } + + return { errors, warnings: [] }; + } +}; diff --git a/test/fn/watch-project.spec.js b/test/fn/watch-project.spec.js index e0aa53233..5410115b5 100644 --- a/test/fn/watch-project.spec.js +++ b/test/fn/watch-project.spec.js @@ -87,6 +87,7 @@ describe('watch-project', function () { sinon.stub(environment, 'extraArgs').get(() => { }); sinon.stub(environment, 'isArchiveMode').get(() => false); sinon.stub(environment, 'skipTranslationCheck').get(() => false); + sinon.stub(environment, 'skipValidate').get(() => false); sinon.stub(environment, 'force').get(() => false); return api.db.put({ _id: '_design/medic-client', deploy_info: { version: '3.5.0' } }).then(() => api.start()); }); @@ -295,4 +296,22 @@ describe('watch-project', function () { .then(() => cleanFormDir(contactFormsDir, form)); }); + it('watch-project: upload app forms --skip-validate', () => { + sinon.stub(environment, 'skipValidate').get(() => true); + const form = 'death'; + const copySampleForm = () => { + copySampleForms('upload-app-form'); + }; + + return watchWrapper(copySampleForm, `${form}.xml`) + .then(() => api.db.allDocs()) + .then(docs => { + const docIds = docs.rows.map(row => row.id); + expect(docIds).to.include(`form:${form}`); + // No requests should have been made to the api since the validations were not run + expect(api.requestLog()).to.be.empty; + }) + .then(() => cleanFormDir(appFormDir, form)); + }); + }); diff --git a/test/lib/main.spec.js b/test/lib/main.spec.js index 679e49e05..db3d6ec19 100644 --- a/test/lib/main.spec.js +++ b/test/lib/main.spec.js @@ -119,7 +119,7 @@ describe('main', () => { expect(mocks.executeAction.args[0][0].name).to.eq('initialise-project-layout'); }); - const expectExecuteActionBehavior = (expectedActions, expectedExtraParams, needsApi) => { + const expectExecuteActionBehavior = (expectedActions, expectedExtraParams) => { if (Array.isArray(expectedActions)) { expect(mocks.executeAction.args.map(args => args[0].name)).to.deep.eq(expectedActions); } else { @@ -128,18 +128,18 @@ describe('main', () => { expect(mocks.environment.initialize.args[0][3]).to.deep.eq(expectedExtraParams); - expect(mocks.environment.initialize.args[0][4]).to.eq(needsApi ? 'http://api' : undefined); + expect(mocks.environment.initialize.args[0][4]).to.eq('http://api'); }; it('--local no COUCH_URL', async () => { await main([...normalArgv, '--local'], {}); - expectExecuteActionBehavior(defaultActions, undefined, true); + expectExecuteActionBehavior(defaultActions, undefined); }); it('--local with COUCH_URL to localhost', async () => { const COUCH_URL = 'http://user:pwd@localhost:5988/medic'; await main([...normalArgv, '--local'], { COUCH_URL }); - expectExecuteActionBehavior(defaultActions, undefined, true); + expectExecuteActionBehavior(defaultActions, undefined); }); it('--instance + 2 ordered actions', async () => { @@ -166,6 +166,49 @@ describe('main', () => { } }); + it('add validate forms actions for upload forms actions', async () => { + await main([...normalArgv, '--local', 'upload-collect-forms', 'upload-contact-forms', 'upload-app-forms'], {}); + expectExecuteActionBehavior( + [ + 'validate-collect-forms', 'upload-collect-forms', + 'validate-contact-forms', 'upload-contact-forms', + 'validate-app-forms', 'upload-app-forms' + ], undefined + ); + expect(mocks.environment.initialize.args[0][7]).to.be.undefined; + }); + + it('--skip-validate for upload forms actions', async () => { + await main([...normalArgv, '--local', '--skip-validate', 'upload-collect-forms', 'upload-contact-forms', 'upload-app-forms'], {}); + expectExecuteActionBehavior( + [ + 'upload-collect-forms', + 'upload-contact-forms', + 'upload-app-forms' + ], undefined + ); + expect(mocks.warn.callCount).to.equal(1); + expect(mocks.warn.args[0][0]).to.equal('Skipping all form validation.'); + // The skipValidate param should be `true` when initializing the environment + expect(mocks.environment.initialize.args[0][7]).to.eq(true); + }); + + it('--skip-validate for validate forms actions', async () => { + await main([...normalArgv, '--local', '--skip-validate', 'validate-collect-forms', 'validate-contact-forms', + 'validate-app-forms', 'upload-collect-forms', 'upload-contact-forms', 'upload-app-forms'], {}); + expectExecuteActionBehavior( + [ + 'upload-collect-forms', + 'upload-contact-forms', + 'upload-app-forms' + ], undefined + ); + expect(mocks.warn.callCount).to.equal(1); + expect(mocks.warn.args[0][0]).to.equal('Skipping all form validation.'); + // The skipValidate param should be `true` when initializing the environment + expect(mocks.environment.initialize.args[0][7]).to.eq(true); + }); + describe('--archive', () => { it('default actions', async () => { await main([...normalArgv, '--archive', '--destination=foo'], {}); @@ -176,7 +219,7 @@ describe('main', () => { it('single action', async () => { await main([...normalArgv, '--archive', '--destination=foo', 'upload-app-settings'], {}); - expectExecuteActionBehavior('upload-app-settings', undefined, true); + expectExecuteActionBehavior('upload-app-settings', undefined); expect(userPrompt.keyInYN.callCount).to.eq(0); }); diff --git a/test/lib/validate-forms.spec.js b/test/lib/validate-forms.spec.js index 9a76eab6f..2f0895d43 100644 --- a/test/lib/validate-forms.spec.js +++ b/test/lib/validate-forms.spec.js @@ -3,85 +3,132 @@ const rewire = require('rewire'); const sinon = require('sinon'); const log = require('../../src/lib/log'); +const environment = require('../../src/lib/environment'); const validateForms = rewire('../../src/lib/validate-forms'); const BASE_DIR = 'data/lib/upload-forms'; const FORMS_SUBDIR = '.'; +const mockValidation = (output = {}) => ({ + requiresInstance: true, + skipFurtherValidation: false, + execute: sinon.stub().resolves(output) +}); + describe('validate-forms', () => { + let logInfo; + let logWarn; + let logError; + + beforeEach(() => { + sinon.stub(environment, 'apiUrl').get(() => true); + logInfo = sinon.stub(log, 'info'); + logWarn = sinon.stub(log, 'warn'); + logError = sinon.stub(log, 'error'); + }); afterEach(sinon.restore); - it('should reject forms which do not have ', () => { - const logInfo = sinon.stub(log, 'info'); - const logError = sinon.stub(log, 'error'); - const apiMock = { - formsValidate: sinon.stub().resolves({ok:true}) - }; - return validateForms.__with__({api: apiMock})(async () => { - try { - await validateForms(`${BASE_DIR}/no-instance-id`, FORMS_SUBDIR); - assert.fail('Expected Error to be thrown.'); - } catch (e) { - assert.notInclude(e.message, 'One or more forms appears to have errors found by the API validation endpoint.'); - assert.include(e.message, 'One or more forms appears to be missing node.'); - expect(logInfo.args[0][0]).to.equal('Validating form: example.xml…'); - expect(logError.callCount).to.equal(1); - } - }); + it('should properly load validations', () => { + const validations = validateForms.__get__('validations'); + + const hasInstanceId = validations.shift(); + expect(hasInstanceId.name).to.equal('has-instance-id.js'); + expect(hasInstanceId.requiresInstance).to.equal(false); + expect(hasInstanceId.skipFurtherValidation).to.equal(true); + + const canGeneratexForm = validations.shift(); + expect(canGeneratexForm.name).to.equal('can-generate-xform.js'); + expect(canGeneratexForm.requiresInstance).to.equal(true); + expect(canGeneratexForm.skipFurtherValidation).to.equal(false); + + expect(validations).to.be.empty; }); - it('should reject forms that the api validations reject', () => { - const logInfo = sinon.stub(log, 'info'); - const logError = sinon.stub(log, 'error'); - const formsValidateMock = sinon.stub().rejects('The error'); - const apiMock = () => ({ - formsValidate: formsValidateMock - }); - return validateForms.__with__({api: apiMock})(async () => { + it('should throw an error when there are validation errors', () => { + const errorValidation = mockValidation({ errors: ['Error 1', 'Error 2'] }); + return validateForms.__with__({ validations: [errorValidation] })(async () => { try { - await validateForms(`${BASE_DIR}/merge-properties`, FORMS_SUBDIR); + await validateForms(`${BASE_DIR}/good-and-bad-forms`, FORMS_SUBDIR); assert.fail('Expected Error to be thrown.'); } catch (e) { - expect(formsValidateMock.called).to.be.true; - assert.include(e.message, 'One or more forms appears to have errors found by the API validation endpoint.'); - assert.notInclude(e.message, 'One or more forms appears to be missing node.'); - expect(logInfo.args[0][0]).to.equal('Validating form: example.xml…'); - expect(logError.callCount).to.equal(1); + assert.include(e.message, 'One or more forms have failed validation.'); + expect(logInfo.callCount).to.equal(2); + expect(logInfo.args[0][0]).to.equal('Validating form: example-no-id.xml…'); + expect(logInfo.args[1][0]).to.equal('Validating form: example.xml…'); + expect(logError.callCount).to.equal(4); + expect(logError.args[0][0]).to.equal('Error 1'); + expect(logError.args[1][0]).to.equal('Error 2'); + expect(logError.args[2][0]).to.equal('Error 1'); + expect(logError.args[3][0]).to.equal('Error 2'); } }); }); - it('should execute all validations and fail with all the errors concatenated', () => { - const logInfo = sinon.stub(log, 'info'); - const logError = sinon.stub(log, 'error'); - const formsValidateMock = sinon.stub().rejects('The error'); - const apiMock = () => ({ - formsValidate: formsValidateMock + it('should warn when skipping validation that requires instance', () => { + sinon.stub(environment, 'apiUrl').get(() => false); + const errorValidation = mockValidation({ errors: ['Should not see this error.'] }); + const warningValidation = mockValidation({ warnings: ['Warning'] }); + warningValidation.requiresInstance = false; + return validateForms.__with__({ validations: [errorValidation, warningValidation] })(async () => { + await validateForms(`${BASE_DIR}/merge-properties`, FORMS_SUBDIR); + expect(logInfo.callCount).to.equal(1); + expect(logInfo.args[0][0]).to.equal('Validating form: example.xml…'); + expect(logWarn.callCount).to.equal(2); + expect(logWarn.args[0][0]).to.equal('Warning'); + expect(logWarn.args[1][0]).to.equal('Some validations have been skipped because they require a CHT instance.'); }); - return validateForms.__with__({api: apiMock})(async () => { + }); + + it('should skip additional validations for form when configured', () => { + const errorValidation = mockValidation({ errors: ['Error'] }); + errorValidation.skipFurtherValidation = true; + const skippedValidation = mockValidation({ errors: ['Should not see this error.'] }); + return validateForms.__with__({ validations: [errorValidation, skippedValidation] })(async () => { try { await validateForms(`${BASE_DIR}/good-and-bad-forms`, FORMS_SUBDIR); assert.fail('Expected Error to be thrown.'); } catch (e) { - assert.include(e.message, 'One or more forms appears to have errors found by the API validation endpoint.'); - assert.include(e.message, 'One or more forms appears to be missing node.'); + assert.include(e.message, 'One or more forms have failed validation.'); + expect(logInfo.callCount).to.equal(2); expect(logInfo.args[0][0]).to.equal('Validating form: example-no-id.xml…'); expect(logInfo.args[1][0]).to.equal('Validating form: example.xml…'); expect(logError.callCount).to.equal(2); + expect(logError.args[0][0]).to.equal('Error'); + expect(logError.args[1][0]).to.equal('Error'); } }); }); it('should resolve OK if all validations pass', () => { - const logInfo = sinon.stub(log, 'info'); - const apiMock = () => ({ - formsValidate: sinon.stub().resolves({ok:true}) - }); - return validateForms.__with__({api: apiMock})(async () => { + return validateForms.__with__({ validations: [mockValidation(), mockValidation(), mockValidation()] })(async () => { await validateForms(`${BASE_DIR}/merge-properties`, FORMS_SUBDIR); + expect(logInfo.callCount).to.equal(1); expect(logInfo.args[0][0]).to.equal('Validating form: example.xml…'); }); }); + + it('should resolve OK if there are only warnings', () => { + const warningValidation = mockValidation({ warnings: ['Warning 1', 'Warning 2'] }); + return validateForms.__with__({ validations: [warningValidation] })(async () => { + await validateForms(`${BASE_DIR}/good-and-bad-forms`, FORMS_SUBDIR); + expect(logInfo.callCount).to.equal(2); + expect(logInfo.args[0][0]).to.equal('Validating form: example-no-id.xml…'); + expect(logInfo.args[1][0]).to.equal('Validating form: example.xml…'); + expect(logWarn.callCount).to.equal(4); + expect(logWarn.args[0][0]).to.equal('Warning 1'); + expect(logWarn.args[1][0]).to.equal('Warning 2'); + expect(logWarn.args[2][0]).to.equal('Warning 1'); + expect(logWarn.args[3][0]).to.equal('Warning 2'); + }); + }); + + it('should resolve OK if form directory cannot be found', () => { + return validateForms.__with__({ validations: [mockValidation(), mockValidation(), mockValidation()] })(async () => { + await validateForms(`${BASE_DIR}/non-existant-directory`, FORMS_SUBDIR); + expect(logInfo.callCount).to.equal(1); + expect(logInfo.args[0][0]).to.equal(`Forms dir not found: ${BASE_DIR}/non-existant-directory/forms/${FORMS_SUBDIR}`); + }); + }); }); diff --git a/test/lib/validation/form/can-generate-xform.spec.js b/test/lib/validation/form/can-generate-xform.spec.js new file mode 100644 index 000000000..4ef4d38e8 --- /dev/null +++ b/test/lib/validation/form/can-generate-xform.spec.js @@ -0,0 +1,50 @@ +const { expect } = require('chai'); +const sinon = require('sinon'); + +const canGenerateXForm = require('../../../../src/lib/validation/form/can-generate-xform'); + +const api = require('../../../../src/lib/api'); +const environment = require('../../../../src/lib/environment'); + +const xformPath = '/my/form/path/form.xml'; +const xmlStr = ''; + +describe('can-generate-xform', () => { + beforeEach(() => { + sinon.stub(environment, 'isArchiveMode').get(() => false); + }); + + afterEach(sinon.restore); + + it('should resolve OK when form has instance id', () => { + sinon.stub(api(), 'formsValidate').resolves({ formsValidateEndpointFound: true }); + + return canGenerateXForm.execute({ xformPath, xmlStr }) + .then(output => { + expect(output.warnings).is.empty; + expect(output.errors).is.empty; + }); + }); + + it('should return warning when forms validation endpoint not found', () => { + sinon.stub(api(), 'formsValidate').resolves({ formsValidateEndpointFound: false }); + + return canGenerateXForm.execute({ xformPath, xmlStr }) + .then(output => { + expect(output.warnings).deep + .equals(['Form validation endpoint not found in your version of CHT Core, no form will be checked before push']); + expect(output.errors).is.empty; + }); + }); + + it('should return error when forms validation endpoint not found', () => { + const err = new Error('Failed validation.'); + sinon.stub(api(), 'formsValidate').throws(err); + + return canGenerateXForm.execute({ xformPath, xmlStr }) + .then(output => { + expect(output.warnings).is.empty; + expect(output.errors).deep.equals([`Error found while validating "${xformPath}". Validation response: ${err.message}`]); + }); + }); +}); diff --git a/test/lib/validation/form/has-instance-id.spec.js b/test/lib/validation/form/has-instance-id.spec.js new file mode 100644 index 000000000..76fe1bd2c --- /dev/null +++ b/test/lib/validation/form/has-instance-id.spec.js @@ -0,0 +1,47 @@ +const { expect } = require('chai'); + +const hasInstanceId = require('../../../../src/lib/validation/form/has-instance-id'); + +const xformPath = '/my/form/path/form.xml'; +const getXml = (metaNodes = '') => ` + + + + No Instance ID + + + + + + + + + ${metaNodes} + + + + + + + + +`; + +describe('has-instance-id', () => { + it('should resolve OK when form has instance id', () => { + return hasInstanceId.execute({ xformPath, xmlStr: getXml('') }) + .then(output => { + expect(output.warnings).is.empty; + expect(output.errors).is.empty; + }); + }); + + it('should return error when form does not have an instance id', () => { + return hasInstanceId.execute({ xformPath, xmlStr: getXml() }) + .then(output => { + expect(output.warnings).is.empty; + expect(output.errors).deep + .equals([`Form at ${xformPath} appears to be missing node. This form will not work on CHT webapp.`]); + }); + }); +});