Add the possibility to change offenders' layout

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;
+}

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() {

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;
+            }
+        }
+    }
 }

front/src/views/rule.html

@@ -10,24 +10,31 @@
         </div>
         <div class="right">
             <h3>Value: {{rule.value}}</h3>
-            <div ng-bind-html="message" class="message"></div>
+            <div ng-bind-html="rule.policy.message" class="message"></div>
         </div>
     </div>
     <div ng-if="rule.abnormal" class="warning">
         <h3>Warning</h3>
         <p>This rule reached the abnormality threshold, which means there is a real problem you should care about.</p>
     </div>
-    <div class="offenders">
+    <div class="offenders" ng-if="!htmlOffenders">
         <h3>
             <ng-pluralize count="rule.offenders.length || 0" when="{'0': 'No offenders', 'one': '1 offender', 'other': '{} offenders'}">
             </ng-pluralize>
         </h3>
         <div class="offendersTable">
             <div ng-repeat="offender in rule.offenders track by $index">
-                <div>{{offender}}</div>
+                <div ng-bind-html="offender"></div>
             </div>
         </div>
     </div>
+    <div class="offenders" ng-if="htmlOffenders">
+        <h3>
+            <ng-pluralize count="rule.offendersCount || 0" when="{'0': 'No offenders', 'one': '1 offender', 'other': '{} offenders'}">
+            </ng-pluralize>
+        </h3>
+        <div class="offendersHtml" ng-bind-html="htmlOffenders"></div>
+    </div>
     <div ng-if="!rule && rule !== null" class="notFound">
         <h2>404</h2>
         Rule "{{policyName}}"" not found

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": "<p>A deep DOM makes the CSS matching with DOM elements difficult.</p><p>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.</p>",
         "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": "<p>IDs of HTML elements must be document-wide unique. This can cause problems with getElementById returning the wrong element.</p>",
         "isOkThreshold": 0,
         "isBadThreshold": 5,
-        "isAbnormalThreshold": 10
+        "isAbnormalThreshold": 10,
+        "offendersTransformFn": function(offenders) {
+            return offenders.map(function(offender) {
+                var results = /^(.*): (\d) occurrences$/.exec(offender);
+                return '<b>#' + results[1] + '</b>: ' + results[2] + ' occurrences';
+            });
+        }
     },
     "DOMinserts": {
         "tool": "phantomas",

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 += '<div><span>' + key + '</span>' + recursiveHtmlBuilder(tree[key]) + '</div>';
+                } else if (tree[key] > 1) {
+                    html += '<div><span>' + key + ' <span>(x' + tree[key] + ')</span></span></div>';
+                } else {
+                    html += '<div><span>' + key + '</span></div>';
+                }
+            });
+
+            return html;
+        }
+
+        return '<div class="domTree">' + recursiveHtmlBuilder(domTree) + '</div>';
+    };
+
+    this.listOfDomPathsToHTML = function(domPaths) {
+        var domArrays = domPaths.map(this.domPathToArray);
+        var domTree = this.listOfDomArraysToTree(domArrays);
+        return this.domTreeToHTML(domTree);
+    };
+
+};
+
+module.exports = new OffendersHelpers();

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;

test/core/indexTest.js

@@ -72,7 +72,8 @@ describe('index.js', function() {
                     "abnormal": false,
                     "score": 100,
                     "abnormalityScore": 0,
-                    "offenders": ["body > h1[1]"]
+                    "offenders": "<div class=\"domTree\"><div><span>body</span><div><span>h1[1]</span></div></div></div>",
+                    "offendersCount": 1
                 });
 
                 // Test javascriptExecutionTree

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('<div class="domTree"><div><span>body</span><div><span>ul.retroGuide</span><div><span>li[0]</span><div><span>div.retro-chaine.france2 <span>(x2)</span></span></div></div><div><span>li[1]</span><div><span>div.retro-chaine.france2</span></div></div></div></div></div>');
+        });
+
+    });
+
+    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('<div class="domTree"><div><span>body</span><div><span>ul.retroGuide</span><div><span>li[0]</span><div><span>div.retro-chaine.france2 <span>(x2)</span></span></div></div><div><span>li[1]</span><div><span>div.retro-chaine.france2</span></div></div></div></div></div>');
+        });
+
+    });
+
+});

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);

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,

test/fixtures/rulesCheckerPolicies.json → 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
     }
-}
+};
+
+module.exports = policies;