diff --git a/front/src/css/rule.css b/front/src/css/rule.css index 9fea690..57d9b17 100644 --- a/front/src/css/rule.css +++ b/front/src/css/rule.css @@ -82,3 +82,17 @@ font-size: 3em; margin-bottom: 1em; } +.offendersHtml { + display: inline-block; +} +.offendersHtml .domTree div { + text-align: left; + margin-left: 1em; +} +.offendersHtml .domTree div span:only-child { + font-weight: bold; +} +.offendersHtml .domTree div span:only-child span { + font-style: italic; + font-weight: normal; +} diff --git a/front/src/js/controllers/ruleCtrl.js b/front/src/js/controllers/ruleCtrl.js index 63fa2ef..e63adc1 100644 --- a/front/src/js/controllers/ruleCtrl.js +++ b/front/src/js/controllers/ruleCtrl.js @@ -22,7 +22,10 @@ ruleCtrl.controller('RuleCtrl', ['$scope', '$rootScope', '$routeParams', '$locat function init() { $scope.rule = $scope.result.rules[$scope.policyName]; - $scope.message = $sce.trustAsHtml($scope.rule.policy.message); + + if (angular.isString($scope.rule.offenders)) { + $scope.htmlOffenders = $scope.rule.offenders; + } } $scope.backToDashboard = function() { diff --git a/front/src/less/rule.less b/front/src/less/rule.less index a12463a..feddc09 100644 --- a/front/src/less/rule.less +++ b/front/src/less/rule.less @@ -88,4 +88,21 @@ font-size: 3em; margin-bottom: 1em; } +} + +.offendersHtml { + display: inline-block; + + .domTree div { + text-align: left; + margin-left: 1em; + + span:only-child { + font-weight: bold; + span { + font-style: italic; + font-weight: normal; + } + } + } } \ No newline at end of file diff --git a/front/src/views/rule.html b/front/src/views/rule.html index ee540ce..639ed91 100644 --- a/front/src/views/rule.html +++ b/front/src/views/rule.html @@ -10,24 +10,31 @@

Value: {{rule.value}}

-
+

Warning

This rule reached the abnormality threshold, which means there is a real problem you should care about.

-
+

-
{{offender}}
+
+
+

+ + +

+
+

404

Rule "{{policyName}}"" not found diff --git a/lib/metadata/policies.js b/lib/metadata/policies.js index b2b70fd..9c12353 100644 --- a/lib/metadata/policies.js +++ b/lib/metadata/policies.js @@ -1,4 +1,5 @@ var debug = require('debug')('ylt:policies'); +var offendersHelpers = require('../offendersHelpers'); var policies = { "DOMelementsCount": { @@ -15,7 +16,10 @@ var policies = { "message": "

A deep DOM makes the CSS matching with DOM elements difficult.

It also slows down JavaScript modifications to the DOM because changing the dimensions of an element makes the browser re-calculate the dimensions of it's parents. Same thing for JavaScript events, that bubble up to the document root.

", "isOkThreshold": 10, "isBadThreshold": 20, - "isAbnormalThreshold": 28 + "isAbnormalThreshold": 28, + "offendersTransformFn": function(offenders) { + return offendersHelpers.listOfDomPathsToHTML(offenders); + } }, "iframesCount": { "tool": "phantomas", @@ -31,7 +35,13 @@ var policies = { "message": "

IDs of HTML elements must be document-wide unique. This can cause problems with getElementById returning the wrong element.

", "isOkThreshold": 0, "isBadThreshold": 5, - "isAbnormalThreshold": 10 + "isAbnormalThreshold": 10, + "offendersTransformFn": function(offenders) { + return offenders.map(function(offender) { + var results = /^(.*): (\d) occurrences$/.exec(offender); + return '#' + results[1] + ': ' + results[2] + ' occurrences'; + }); + } }, "DOMinserts": { "tool": "phantomas", diff --git a/lib/offendersHelpers.js b/lib/offendersHelpers.js new file mode 100644 index 0000000..bbab2bf --- /dev/null +++ b/lib/offendersHelpers.js @@ -0,0 +1,65 @@ + + +var OffendersHelpers = function() { + + + this.domPathToArray = function(str) { + return str.split(/\s?>\s?/); + }; + + this.listOfDomArraysToTree = function(listOfDomArrays) { + var result = {}; + + function recursiveTreeBuilder(tree, domArray) { + if (domArray.length > 0) { + var currentDomElement = domArray.shift(domArray); + if (tree === null) { + tree = {}; + } + tree[currentDomElement] = recursiveTreeBuilder(tree[currentDomElement] || null, domArray); + return tree; + } else if (tree === null) { + return 1; + } else { + return tree + 1; + } + } + + listOfDomArrays.forEach(function(domArray) { + result = recursiveTreeBuilder(result, domArray); + }); + + return result; + }; + + this.domTreeToHTML = function(domTree) { + + function recursiveHtmlBuilder(tree) { + var html = ''; + var keys = Object.keys(tree); + + keys.forEach(function(key) { + if (isNaN(tree[key])) { + html += '
' + key + '' + recursiveHtmlBuilder(tree[key]) + '
'; + } else if (tree[key] > 1) { + html += '
' + key + ' (x' + tree[key] + ')
'; + } else { + html += '
' + key + '
'; + } + }); + + return html; + } + + return '
' + recursiveHtmlBuilder(domTree) + '
'; + }; + + this.listOfDomPathsToHTML = function(domPaths) { + var domArrays = domPaths.map(this.domPathToArray); + var domTree = this.listOfDomArraysToTree(domArrays); + return this.domTreeToHTML(domTree); + }; + +}; + +module.exports = new OffendersHelpers(); \ No newline at end of file diff --git a/lib/rulesChecker.js b/lib/rulesChecker.js index f8eabd1..7fb238d 100644 --- a/lib/rulesChecker.js +++ b/lib/rulesChecker.js @@ -24,10 +24,12 @@ var RulesChecker = function() { policy: policy }; + var offenders = []; + // Take DOMqueriesAvoidable's offenders from DOMqueriesDuplicated, for example. if (policy.takeOffendersFrom) { + var fromList = policy.takeOffendersFrom; - var offenders = []; // takeOffendersFrom option can be a string or an array of strings. if (typeof fromList === 'string') { @@ -35,16 +37,31 @@ var RulesChecker = function() { } fromList.forEach(function(from) { - offenders = offenders.concat(data.toolsResults[policy.tool].offenders[from]); + if (data.toolsResults[policy.tool] && + data.toolsResults[policy.tool].offenders && + data.toolsResults[policy.tool].offenders[from]) { + offenders = offenders.concat(data.toolsResults[policy.tool].offenders[from]); + } }); data.toolsResults[policy.tool].offenders[metricName] = offenders; + + } else if (data.toolsResults[policy.tool] && + data.toolsResults[policy.tool].offenders && + data.toolsResults[policy.tool].offenders[metricName]) { + offenders = data.toolsResults[policy.tool].offenders[metricName]; + } + + // It is possible to declare a transformation function for the offenders. + // The function should take an array of strings as single parameter and return a string. + if (policy.offendersTransformFn) { + rule.offendersCount = offenders.length; + offenders = policy.offendersTransformFn(offenders); + delete policy.offendersTransformFn; } - if (data.toolsResults[policy.tool].offenders && - data.toolsResults[policy.tool].offenders[metricName] && - data.toolsResults[policy.tool].offenders[metricName].length > 0) { - rule.offenders = data.toolsResults[policy.tool].offenders[metricName]; + if (offenders && offenders.length > 0) { + rule.offenders = offenders; } rule.bad = rule.value > policy.isOkThreshold; diff --git a/test/core/indexTest.js b/test/core/indexTest.js index 6207241..715f2f9 100644 --- a/test/core/indexTest.js +++ b/test/core/indexTest.js @@ -72,7 +72,8 @@ describe('index.js', function() { "abnormal": false, "score": 100, "abnormalityScore": 0, - "offenders": ["body > h1[1]"] + "offenders": "
body
h1[1]
", + "offendersCount": 1 }); // Test javascriptExecutionTree diff --git a/test/core/offendersHelpersTest.js b/test/core/offendersHelpersTest.js new file mode 100644 index 0000000..5c1a72c --- /dev/null +++ b/test/core/offendersHelpersTest.js @@ -0,0 +1,83 @@ +var should = require('chai').should(); +var offendersHelpers = require('../../lib/offendersHelpers'); + +describe('offendersHelpers', function() { + + describe('domPathToArray', function() { + + it('should transform a path to an array', function() { + var result = offendersHelpers.domPathToArray('body > section#page > div.alternate-color > ul.retroGuide > li[0] > div.retro-chaine.france2'); + result.should.deep.equal(['body', 'section#page', 'div.alternate-color', 'ul.retroGuide', 'li[0]', 'div.retro-chaine.france2']); + }); + + it('should work even if a space is missing', function() { + var result = offendersHelpers.domPathToArray('body > section#page> div.alternate-color > ul.retroGuide >li[0] > div.retro-chaine.france2'); + result.should.deep.equal(['body', 'section#page', 'div.alternate-color', 'ul.retroGuide', 'li[0]', 'div.retro-chaine.france2']); + }); + + }); + + describe('listOfDomArraysToTree', function() { + + it('should transform a list of arrays into a tree', function() { + var result = offendersHelpers.listOfDomArraysToTree([ + ['body', 'section#page', 'div.alternate-color', 'ul.retroGuide', 'li[0]', 'div.retro-chaine.france2'], + ['body', 'section#page', 'div.alternate-color', 'ul.retroGuide', 'li[0]', 'div.retro-chaine.france2'], + ['body', 'section#page', 'div.alternate-color', 'ul.retroGuide', 'li[1]', 'div.retro-chaine.france2'] + ]); + result.should.deep.equal({ + 'body': { + 'section#page': { + 'div.alternate-color': { + 'ul.retroGuide': { + 'li[0]': { + 'div.retro-chaine.france2': 2 + }, + 'li[1]': { + 'div.retro-chaine.france2': 1 + } + } + } + } + } + }); + }); + + }); + + describe('domTreeToHTML', function() { + + it('should transform a dom tree into HTML with the awaited format', function() { + var result = offendersHelpers.domTreeToHTML({ + 'body': { + 'ul.retroGuide': { + 'li[0]': { + 'div.retro-chaine.france2': 2 + }, + 'li[1]': { + 'div.retro-chaine.france2': 1 + } + } + } + }); + + result.should.equal('
body
ul.retroGuide
li[0]
div.retro-chaine.france2 (x2)
li[1]
div.retro-chaine.france2
'); + }); + + }); + + describe('listOfDomPathsToHTML', function() { + + it('should transform a list of path strings into HTML', function() { + var result = offendersHelpers.listOfDomPathsToHTML([ + 'body > ul.retroGuide > li[0] > div.retro-chaine.france2', + 'body > ul.retroGuide > li[1] > div.retro-chaine.france2', + 'body > ul.retroGuide > li[0] > div.retro-chaine.france2', + ]); + + result.should.equal('
body
ul.retroGuide
li[0]
div.retro-chaine.france2 (x2)
li[1]
div.retro-chaine.france2
'); + }); + + }); + +}); diff --git a/test/core/rulesCheckerTest.js b/test/core/rulesCheckerTest.js index 841c1ab..f979109 100644 --- a/test/core/rulesCheckerTest.js +++ b/test/core/rulesCheckerTest.js @@ -9,7 +9,7 @@ describe('rulesChecker', function() { it('should produce a nice rules object', function() { var data = require('../fixtures/rulesCheckerInput.json'); - var policies = require('../fixtures/rulesCheckerPolicies.json'); + var policies = require('../fixtures/rulesCheckerPolicies'); var expected = require('../fixtures/rulesCheckerOutput.json'); var results = rulesChecker.check(data, policies); diff --git a/test/fixtures/rulesCheckerOutput.json b/test/fixtures/rulesCheckerOutput.json index 5327aa5..9f6ddf4 100644 --- a/test/fixtures/rulesCheckerOutput.json +++ b/test/fixtures/rulesCheckerOutput.json @@ -25,7 +25,8 @@ "takeOffendersFrom": "metric3" }, "value": 222, - "offenders": ["offender1", "offender2"], + "offenders": "offender1 - offender2", + "offendersCount": 2, "bad": false, "abnormal": false, "score": 100, @@ -41,7 +42,8 @@ "isAbnormalThreshold": 5000 }, "value": 6666, - "offenders": ["offender1", "offender2"], + "offenders": "offender1/offender2", + "offendersCount": 2, "bad": true, "abnormal": true, "score": 0, diff --git a/test/fixtures/rulesCheckerPolicies.json b/test/fixtures/rulesCheckerPolicies.js similarity index 86% rename from test/fixtures/rulesCheckerPolicies.json rename to test/fixtures/rulesCheckerPolicies.js index 9147e01..a4fecab 100644 --- a/test/fixtures/rulesCheckerPolicies.json +++ b/test/fixtures/rulesCheckerPolicies.js @@ -1,4 +1,5 @@ -{ +var policies = { + "metric1": { "tool": "tool1", "label": "The metric 1", @@ -14,7 +15,10 @@ "isOkThreshold": 1000, "isBadThreshold": 3000, "isAbnormalThreshold": 5000, - "takeOffendersFrom": "metric3" + "takeOffendersFrom": "metric3", + "offendersTransformFn": function(offenders) { + return offenders.join(' - '); + } }, "metric3": { "tool": "tool1", @@ -22,7 +26,10 @@ "message": "A great message", "isOkThreshold": 1000, "isBadThreshold": 3000, - "isAbnormalThreshold": 5000 + "isAbnormalThreshold": 5000, + "offendersTransformFn": function(offenders) { + return offenders.join('/'); + } }, "metric4": { "tool": "tool1", @@ -81,4 +88,6 @@ "isBadThreshold": 3000, "isAbnormalThreshold": 5000 } -} \ No newline at end of file +}; + +module.exports = policies; \ No newline at end of file