From 813f6e55847a03118cdbfc1262f26b5f5cf1638e Mon Sep 17 00:00:00 2001 From: Bruno Bernardino Date: Mon, 17 Jul 2017 15:07:53 +0100 Subject: [PATCH 1/2] Improving ID checks Remove check of .id property in Hooks and make it more defensive (don't throw an exception when the `result` is `null`, for example). --- src/app-middlewares/after/checks.js | 2 +- src/checks/example.js | 2 +- src/checks/trigger-has-id.js | 7 +++++-- test/checks.js | 3 +++ 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/app-middlewares/after/checks.js b/src/app-middlewares/after/checks.js index 819d158..2ba1e0c 100644 --- a/src/app-middlewares/after/checks.js +++ b/src/app-middlewares/after/checks.js @@ -25,7 +25,7 @@ const checkOutput = (output) => { if (runChecks) { const rawResults = checks .filter((check) => { - return check.shouldRun(event.method); + return check.shouldRun(event.method, event.bundle); }) .map((check) => { return check.run(event.method, output.results) diff --git a/src/checks/example.js b/src/checks/example.js index 2887d85..0c6756f 100644 --- a/src/checks/example.js +++ b/src/checks/example.js @@ -8,7 +8,7 @@ */ const exampleChecker = { name: 'exampleChecker', - shouldRun: (method) => { + shouldRun: (method/*, bundle*/) => { return method && true; }, run: (method, results) => { diff --git a/src/checks/trigger-has-id.js b/src/checks/trigger-has-id.js index 987a216..347bf55 100644 --- a/src/checks/trigger-has-id.js +++ b/src/checks/trigger-has-id.js @@ -9,10 +9,13 @@ const isTrigger = require('./is-trigger'); */ const triggerHasId = { name: 'triggerHasId', - shouldRun: isTrigger, + shouldRun: (method, bundle) => { + // Hooks will have a bundle.cleanedRequest and we don't need to check they've got an id + return (isTrigger(method) && bundle.cleanedRequest); + }, run: (method, results) => { const missingIdResult = _.find(results, (result) => { - return _.isUndefined(result.id) || _.isNull(result.id); + return !result || _.isUndefined(result.id) || _.isNull(result.id); }); if (missingIdResult) { diff --git a/test/checks.js b/test/checks.js index 17df9f5..9695583 100644 --- a/test/checks.js +++ b/test/checks.js @@ -30,6 +30,9 @@ describe('checks', () => { checks.triggerHasId.run(testMethod, [{id: 1}, {id: 2}]).length.should.eql(0); checks.triggerHasId.run(testMethod, [{game_id: 1}]).length.should.eql(1); checks.triggerHasId.run(testMethod, []).length.should.eql(0, 'blank array'); + checks.triggerHasId.run(testMethod, [1]).length.should.eql(1); + checks.triggerHasId.run(testMethod, [{id: null}]).length.should.eql(1); + checks.triggerHasId.run(testMethod, [{}]).length.should.eql(1); }); it('should check for unique ids via triggerHasUniqueIds', () => { From 9f41a4f8a6ea70bd5931e2b59f9200a4fc036fd9 Mon Sep 17 00:00:00 2001 From: Bruno Bernardino Date: Mon, 17 Jul 2017 15:14:32 +0100 Subject: [PATCH 2/2] Revert the check as I was testing it. --- src/checks/trigger-has-id.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/checks/trigger-has-id.js b/src/checks/trigger-has-id.js index 347bf55..014fc3e 100644 --- a/src/checks/trigger-has-id.js +++ b/src/checks/trigger-has-id.js @@ -11,7 +11,7 @@ const triggerHasId = { name: 'triggerHasId', shouldRun: (method, bundle) => { // Hooks will have a bundle.cleanedRequest and we don't need to check they've got an id - return (isTrigger(method) && bundle.cleanedRequest); + return (isTrigger(method) && !bundle.cleanedRequest); }, run: (method, results) => { const missingIdResult = _.find(results, (result) => {