Merge branch 'fix-broken-file-link-pipeline-security-tab' into 'master'

Fix double line number hash on pipeline security tab

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/170541



Merged-by: Samantha Ming <sming@gitlab.com>
Approved-by: Samantha Ming <sming@gitlab.com>
Reviewed-by: David Pisek <dpisek@gitlab.com>
Co-authored-by: Lorenz van Herwaarden <lvanherwaarden@gitlab.com>

ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_path.vue

@@ -17,13 +17,14 @@ export default {
   computed: {
     fileUrl() {
       const { startLine: start, endLine: end, blobPath } = this.location;
-      const lineNumber = end > start ? `${start}-${end}` : start;
 
-      if (!blobPath) {
-        return '';
+      if (!start || blobPath.includes('#')) {
+        return blobPath;
       }
 
-      return `${blobPath}${lineNumber ? `#L${lineNumber}` : ''}`;
+      const lineNumber = end > start ? `${start}-${end}` : start;
+
+      return `${blobPath}#L${lineNumber}`;
     },
     locationString() {
       const { image, file, startLine, path } = this.location;

ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_path_spec.js

@@ -33,15 +33,15 @@ describe('VulnerabilityPath', () => {
   });
 
   it('should display path as link blob path', () => {
-    const vulnerabilityWithBloPathAndFile = vulnerabilities[1];
-    createWrapper({ location: vulnerabilityWithBloPathAndFile.location });
+    const vulnerabilityWithBlobPathAndFile = vulnerabilities[1];
+    createWrapper({ location: vulnerabilityWithBlobPathAndFile.location });
 
     expect(findVulnerabilityLink().exists()).toBe(true);
     expect(findVulnerabilityLink().attributes('href')).toBe(
-      `${vulnerabilityWithBloPathAndFile.location.blobPath}#L${vulnerabilityWithBloPathAndFile.location.startLine}`,
+      `${vulnerabilityWithBlobPathAndFile.location.blobPath}#L${vulnerabilityWithBlobPathAndFile.location.startLine}`,
     );
     expect(findVulnerabilityText().props()).toMatchObject({
-      text: `${vulnerabilityWithBloPathAndFile.location.file}:${vulnerabilityWithBloPathAndFile.location.startLine}`,
+      text: `${vulnerabilityWithBlobPathAndFile.location.file}:${vulnerabilityWithBlobPathAndFile.location.startLine}`,
       position: 'middle',
     });
   });
@@ -64,4 +64,41 @@ describe('VulnerabilityPath', () => {
     createWrapper();
     expect(wrapper.find('*').exists()).toBe(false);
   });
+
+  describe('file link', () => {
+    it('shows only the file name if there is no start line', () => {
+      createWrapper({ location: { file: 'test.txt', blobPath: 'blob_path.txt' } });
+
+      expect(findVulnerabilityLink().attributes('href')).toBe('blob_path.txt');
+    });
+
+    it('shows the correct line number when there is a start line', () => {
+      createWrapper({ location: { file: 'test.txt', startLine: 24, blobPath: 'blob.txt' } });
+
+      expect(findVulnerabilityLink().attributes('href')).toBe('blob.txt#L24');
+    });
+
+    it('does not append the line number when the blobPath already has a hash', () => {
+      const blobPath = 'blob.txt#L1';
+      createWrapper({ location: { file: 'test.txt', startLine: 24, blobPath } });
+
+      expect(findVulnerabilityLink().attributes('href')).toBe(blobPath);
+    });
+
+    it('shows the correct line numbers when there is a start and end line', () => {
+      createWrapper({
+        location: { file: 'test.txt', startLine: 24, endLine: 27, blobPath: 'blob.txt' },
+      });
+
+      expect(findVulnerabilityLink().attributes('href')).toBe('blob.txt#L24-27');
+    });
+
+    it('shows only the start line when the end line is the same', () => {
+      createWrapper({
+        location: { file: 'test.txt', startLine: 24, endLine: 24, blobPath: 'blob.txt' },
+      });
+
+      expect(findVulnerabilityLink().attributes('href')).toBe('blob.txt#L24');
+    });
+  });
 });