Update frontend code after review

ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js

+import { n__ } from '~/locale';
 import CEWidgetOptions from '~/vue_merge_request_widget/mr_widget_options';
 import WidgetApprovals from './components/approvals/mr_widget_approvals';
 import GeoSecondaryNode from './components/states/mr_widget_secondary_geo_node';
@@ -33,47 +34,49 @@ export default {
     },
     codequalityText() {
       const { newIssues, resolvedIssues } = this.mr.codeclimateMetrics;
-      let newIssuesText;
-      let resolvedIssuesText;
-      let text = [];
+      const text = [];
 
       if (!newIssues.length && !resolvedIssues.length) {
         text.push('No changes to code quality');
       } else if (newIssues.length || resolvedIssues.length) {
-        if (newIssues.length) {
-          newIssuesText = ` degraded on ${newIssues.length} ${this.pointsText(newIssues)}`;
-        }
+        text.push('Code quality');
 
         if (resolvedIssues.length) {
-          resolvedIssuesText = ` improved on ${resolvedIssues.length} ${this.pointsText(resolvedIssues)}`;
-        }
-
-        const connector = (newIssues.length > 0 && resolvedIssues.length > 0) ? ' and' : null;
-
-        text = ['Code quality'];
-        if (resolvedIssuesText) {
-          text.push(resolvedIssuesText);
+          text.push(n__(
+            ' improved on %d point',
+            ' improved on %d points',
+            resolvedIssues.length,
+          ));
         }
 
-        if (connector) {
-          text.push(connector);
+        if (newIssues.length > 0 && resolvedIssues.length > 0) {
+          text.push(' and');
         }
 
-        if (newIssuesText) {
-          text.push(newIssuesText);
+        if (newIssues.length) {
+          text.push(n__(
+            ' degraded on %d point',
+            ' degraded on %d points',
+            newIssues.length,
+          ));
         }
       }
 
       return text.join('');
     },
+
     securityText() {
-      const { securityReport } = this.mr;
-      if (securityReport.length) {
-        return `${securityReport.length} security ${this.pluralizeVulnerability(securityReport.length)} detected`;
+      if (this.mr.securityReport.length) {
+        return n__(
+          '%d security vulnerability detected',
+          '%d security vulnerabilities detected',
+          this.mr.securityReport.length,
+        );
       }
 
       return 'No security vulnerabilities detected';
     },
+
     codequalityStatus() {
       if (this.isLoadingCodequality) {
         return 'loading';
@@ -82,6 +85,7 @@ export default {
       }
       return 'success';
     },
+
     securityStatus() {
       if (this.isLoadingSecurity) {
         return 'loading';
@@ -92,9 +96,6 @@ export default {
     },
   },
   methods: {
-    pluralizeVulnerability(length) {
-      return length === 1 ? 'vulnerability' : 'vulnerabilities';
-    },
     fetchCodeQuality() {
       const { head_path, head_blob_path, base_path, base_blob_path } = this.mr.codeclimate;
 
@@ -115,11 +116,12 @@ export default {
     },
 
     fetchSecurity() {
+      const { path, blob_path } = this.mr.security.sast;
       this.isLoadingSecurity = true;
 
-      this.service.fetchReport(this.mr.security.sast)
+      this.service.fetchReport(path)
         .then((data) => {
-          this.mr.setSecurityReport(data);
+          this.mr.setSecurityReport(data, blob_path);
           this.isLoadingSecurity = false;
         })
         .catch(() => {
@@ -127,10 +129,6 @@ export default {
           this.loadingSecurityFailed = true;
         });
     },
-
-    pointsText(issues) {
-      return gl.text.pluralize('point', issues.length);
-    },
   },
   created() {
     if (this.shouldRenderCodeQuality) {
@@ -160,9 +158,9 @@ export default {
         v-if="shouldRenderCodeQuality"
         type="codequality"
         :status="codequalityStatus"
-        loadingText="Loading codeclimate report"
-        errorText="Failed to load codeclimate report"
-        :successText="codequalityText"
+        loading-text="Loading codeclimate report"
+        error-text="Failed to load codeclimate report"
+        :success-text="codequalityText"
         :unresolvedIssues="mr.codeclimateMetrics.newIssues"
         :resolvedIssues="mr.codeclimateMetrics.resolvedIssues"
         />
@@ -170,9 +168,9 @@ export default {
         v-if="shouldRenderSecurityReport"
         type="security"
         :status="securityStatus"
-        loadingText="Loading security report"
-        errorText="Failed to load security report"
-        :successText="securityText"
+        loading-text="Loading security report"
+        error-text="Failed to load security report"
+        :success-text="securityText"
         :unresolvedIssues="mr.securityReport"
         />
       <div class="mr-widget-section">

ee/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js

@@ -62,10 +62,10 @@ export default class MergeRequestStore extends CEMergeRequestStore {
     this.securityReport = [];
   }
 
-  setSecurityReport(issues) {
-    this.securityReport = MergeRequestStore.parseIssues(issues);
+  setSecurityReport(issues, path) {
+    this.securityReport = MergeRequestStore.parseIssues(issues, path);
   }
-  // TODO: get changes from codequality MR
+
   compareCodeclimateMetrics(headIssues, baseIssues, headBlobPath, baseBlobPath) {
     const parsedHeadIssues = MergeRequestStore.parseIssues(headIssues, headBlobPath);
     const parsedBaseIssues = MergeRequestStore.parseIssues(baseIssues, baseBlobPath);
@@ -113,19 +113,20 @@ export default class MergeRequestStore extends CEMergeRequestStore {
 
         if (issue.location.lines && issue.location.lines.begin) {
           parsedIssue.line = issue.location.lines.begin;
-          parsedIssue.urlPath = parseCodeQualityUrl += `#L${issue.location.lines.begin}`;
-        }
-      } else {
-        // security
-        let parsedSecurityUrl;
-        if (issue.file) {
-          parsedSecurityUrl = `${path}/${issue.file}`;
-          parsedIssue.path = issue.file;
+          parseCodeQualityUrl += `#L${issue.location.lines.begin}`;
         }
 
+        parsedIssue.urlPath = parseCodeQualityUrl;
+
+      // security
+      } else if (issue.file) {
+        let parsedSecurityUrl = `${path}/${issue.file}`;
+        parsedIssue.path = issue.file;
+
         if (issue.line) {
-          parsedIssue.urlPath = parsedSecurityUrl += `#L${issue.line}`;
+          parsedSecurityUrl += `#L${issue.line}`;
         }
+        parsedIssue.urlPath = parsedSecurityUrl;
       }
 
       return parsedIssue;

spec/javascripts/vue_mr_widget/components/mr_widget_report_collapsible_section_spec.js

@@ -28,7 +28,7 @@ describe('Merge Request collapsible section', () => {
     });
   });
 
-  describe('with successful request', () => {
+  describe('with success status', () => {
     it('should render provided data', () => {
       vm = mountComponent(MRWidgetCodeQuality, {
         type: 'codequality',
@@ -49,7 +49,7 @@ describe('Merge Request collapsible section', () => {
     });
 
     describe('toggleCollapsed', () => {
-      it('toggles issues', () => {
+      it('toggles issues', (done) => {
         vm = mountComponent(MRWidgetCodeQuality, {
           type: 'codequality',
           status: 'success',
@@ -63,8 +63,8 @@ describe('Merge Request collapsible section', () => {
 
         Vue.nextTick(() => {
           expect(
-            vm.$el.querySelector('.code-quality-container').geAttribute('style'),
-          ).toEqual(null);
+            vm.$el.querySelector('.code-quality-container').getAttribute('style'),
+          ).toEqual('');
           expect(
             vm.$el.querySelector('button').textContent.trim(),
           ).toEqual('Collapse');
@@ -73,11 +73,13 @@ describe('Merge Request collapsible section', () => {
 
           Vue.nextTick(() => {
             expect(
-              vm.$el.querySelector('.code-quality-container').geAttribute('style'),
+              vm.$el.querySelector('.code-quality-container').getAttribute('style'),
             ).toEqual('display: none;');
             expect(
               vm.$el.querySelector('button').textContent.trim(),
             ).toEqual('Expand');
+
+            done();
           });
         });
       });

spec/javascripts/vue_mr_widget/components/mr_widget_report_issues_spec.js

@@ -32,8 +32,8 @@ describe('merge request report issues', () => {
       it('should render "Fixed" keyword', () => {
         expect(vm.$el.querySelector('.mr-widget-code-quality-list li').textContent).toContain('Fixed');
         expect(
-          vm.$el.querySelector('.mr-widget-code-quality-list li').textContent.trim().replace(/\s+/g, ''),
-        ).toEqual('Fixed:InsecureDependencyinGemfile.lock:12');
+          vm.$el.querySelector('.mr-widget-code-quality-list li').textContent.replace(/\s+/g, ' ').trim(),
+        ).toEqual('Fixed: Insecure Dependency in Gemfile.lock:12');
       });
     });
 

spec/javascripts/vue_mr_widget/mock_data.js

@@ -221,95 +221,91 @@ export default {
 
 export const headIssues = [
   {
-    "check_name": "Rubocop/Lint/UselessAssignment",
-    "location": {
-      "path": "lib/six.rb",
-      "lines": {
-        "begin": 6,
-        "end": 7,
+    check_name: 'Rubocop/Lint/UselessAssignment',
+    location: {
+      path: 'lib/six.rb',
+      lines: {
+        begin: 6,
+        end: 7,
       }
     },
-    "fingerprint": "e879dd9bbc0953cad5037cde7ff0f627",
+    fingerprint: 'e879dd9bbc0953cad5037cde7ff0f627',
   },
   {
-    "categories": ["Security"],
-    "check_name": "Insecure Dependency",
-    "location": {
-      "path": "Gemfile.lock",
-      "lines": {
-        "begin": 22,
-        "end": 22
+    categories: ['Security'],
+    check_name: 'Insecure Dependency',
+    location: {
+      path: 'Gemfile.lock',
+      lines: {
+        begin: 22,
+        end: 22
       }
     },
-    "fingerprint": "ca2e59451e98ae60ba2f54e3857c50e5",
+    fingerprint: 'ca2e59451e98ae60ba2f54e3857c50e5',
   }
 ];
 
 export const parsedHeadIssues = [
   {
-    "check_name": "Rubocop/Lint/UselessAssignment",
-    "location": {
-      "path": "lib/six.rb",
-      "positions": {
-        "begin": {
-          "column": 6,
-          "line": 59
-        },
-        "end": {
-          "column": 7,
-          "line": 59
-        }
-      }
+    check_name: 'Rubocop/Lint/UselessAssignment',
+    location: {
+      path: 'lib/six.rb',
+      lines: {
+        begin: 6,
+        end: 7
+      },
     },
-    "fingerprint": "e879dd9bbc0953cad5037cde7ff0f627",
+    fingerprint: 'e879dd9bbc0953cad5037cde7ff0f627',
     name: 'Rubocop/Lint/UselessAssignment',
     path: 'lib/six.rb',
-  }
+    urlPath: 'headPath/lib/six.rb#L6',
+    line: 6,
+  },
 ];
 
 export const baseIssues = [
   {
-    "categories": ["Security"],
-    "check_name": "Insecure Dependency",
-    "location": {
-      "path": "Gemfile.lock",
-      "lines": {
-        "begin": 22,
-        "end": 22
+    categories: ['Security'],
+    check_name: 'Insecure Dependency',
+    location: {
+      path: 'Gemfile.lock',
+      lines: {
+        begin: 22,
+        end: 22
       }
     },
-    "fingerprint": "ca2e59451e98ae60ba2f54e3857c50e5",
+    fingerprint: 'ca2e59451e98ae60ba2f54e3857c50e5',
   },
   {
-    "categories": ["Security"],
-    "check_name": "Insecure Dependency",
-    "location": {
-      "path": "Gemfile.lock",
-      "lines": {
-        "begin": 21,
-        "end": 21
+    categories: ['Security'],
+    check_name: 'Insecure Dependency',
+    location: {
+      path: 'Gemfile.lock',
+      lines: {
+        begin: 21,
+        end: 21
       }
     },
-    "fingerprint": "ca2354534dee94ae60ba2f54e3857c50e5",
+    fingerprint: 'ca2354534dee94ae60ba2f54e3857c50e5',
   }
 ];
 
 export const parsedBaseIssues = [
   {
-    "categories": ["Security"],
-    "check_name": "Insecure Dependency",
-    "location": {
-      "path": "Gemfile.lock",
-      "lines": {
-        "begin": 21,
-        "end": 21
-      }
+    categories: ['Security'],
+    check_name: 'Insecure Dependency',
+    location: {
+      path: 'Gemfile.lock',
+      lines: {
+        begin: 21,
+        end: 21,
+      },
     },
-    "fingerprint": "ca2354534dee94ae60ba2f54e3857c50e5",
+    fingerprint: "ca2354534dee94ae60ba2f54e3857c50e5",
     name: "Insecure Dependency",
     path: "Gemfile.lock",
     line: 21,
-    urlPath: 'undefined/Gemfile.lock#L21',
+    urlPath: 'basePath/Gemfile.lock#L21',
   },
 ];
 
@@ -352,5 +348,33 @@ export const securityIssues = [
     file: 'Gemfile.lock',
     solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
     priority: 'Medium',
-  }
+  },
+];
+
+export const parsedSecurityIssuesStore = [
+  {
+    tool: 'bundler_audit',
+    message: 'Arbitrary file existence disclosure in Action Pack',
+    url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
+    cve: 'CVE-2014-7829',
+    file: 'Gemfile.lock',
+    solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
+    priority:'High',
+    line: 12,
+    name: 'Arbitrary file existence disclosure in Action Pack',
+    path: 'Gemfile.lock',
+    urlPath: 'path/Gemfile.lock#L12'
+  },
+  {
+    tool: 'bundler_audit',
+    message: 'Possible Information Leak Vulnerability in Action View',
+    url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
+    cve: 'CVE-2016-0752',
+    file: 'Gemfile.lock',
+    solution: 'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
+    priority: 'Medium',
+    name: 'Possible Information Leak Vulnerability in Action View',
+    path: 'Gemfile.lock',
+    urlPath: 'path/Gemfile.lock',
+  },
 ];

spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js

@@ -5,6 +5,7 @@ import mockData, {
   securityIssues,
   parsedBaseIssues,
   parsedHeadIssues,
+  parsedSecurityIssuesStore,
 } from '../mock_data';
 
 describe('MergeRequestStore', () => {
@@ -74,6 +75,14 @@ describe('MergeRequestStore', () => {
     });
   });
 
+  describe('setSecurityReport', () => {
+    it('should set security issues', () => {
+      store.setSecurityReport(securityIssues, 'path');
+
+      expect(store.securityReport).toEqual(parsedSecurityIssuesStore);
+    });
+  });
+
   describe('parseIssues', () => {
     it('should parse the received issues', () => {
       const codequality = MergeRequestStore.parseIssues(baseIssues, 'path')[0];