Merge branch '281727-ci_job_line_links-cleanup' into 'master'

Remove ci_job_line_links feature flag

See merge request gitlab-org/gitlab!49454

app/assets/javascripts/jobs/components/log/line.vue

@@ -18,46 +18,33 @@ export default {
   render(h, { props }) {
     const { line, path } = props;
 
-    let chars;
-    if (gon?.features?.ciJobLineLinks) {
-      chars = line.content.map(content => {
-        return h(
-          'span',
-          {
-            class: ['gl-white-space-pre-wrap', content.style],
-          },
-          // Simple "tokenization": Split text in chunks of text
-          // which alternate between text and urls.
-          content.text.split(linkRegex).map(chunk => {
-            // Return normal string for non-links
-            if (!chunk.match(linkRegex)) {
-              return chunk;
-            }
-            return h(
-              'a',
-              {
-                attrs: {
-                  href: chunk,
-                  class: 'gl-reset-color! gl-text-decoration-underline',
-                  rel: 'nofollow noopener noreferrer', // eslint-disable-line @gitlab/require-i18n-strings
-                },
+    const chars = line.content.map(content => {
+      return h(
+        'span',
+        {
+          class: ['gl-white-space-pre-wrap', content.style],
+        },
+        // Simple "tokenization": Split text in chunks of text
+        // which alternate between text and urls.
+        content.text.split(linkRegex).map(chunk => {
+          // Return normal string for non-links
+          if (!chunk.match(linkRegex)) {
+            return chunk;
+          }
+          return h(
+            'a',
+            {
+              attrs: {
+                href: chunk,
+                class: 'gl-reset-color! gl-text-decoration-underline',
+                rel: 'nofollow noopener noreferrer', // eslint-disable-line @gitlab/require-i18n-strings
               },
-              chunk,
-            );
-          }),
-        );
-      });
-    } else {
-      chars = line.content.map(content => {
-        return h(
-          'span',
-          {
-            class: ['gl-white-space-pre-wrap', content.style],
-          },
-          content.text,
-        );
-      });
-    }
+            },
+            chunk,
+          );
+        }),
+      );
+    });
 
     return h('div', { class: 'js-line log-line' }, [
       h(LineNumber, {

app/controllers/projects/jobs_controller.rb

@@ -14,9 +14,6 @@ class Projects::JobsController < Projects::ApplicationController
   before_action :verify_api_request!, only: :terminal_websocket_authorize
   before_action :authorize_create_proxy_build!, only: :proxy_websocket_authorize
   before_action :verify_proxy_request!, only: :proxy_websocket_authorize
-  before_action do
-    push_frontend_feature_flag(:ci_job_line_links, @project, default_enabled: true)
-  end
   before_action only: :index do
     frontend_experimentation_tracking_data(:jobs_empty_state, 'click_button')
   end

config/feature_flags/development/ci_job_line_links.yml

----
-name: ci_job_line_links
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47532
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/281727
-milestone: '13.6'
-type: development
-group: group::continuous integration
-default_enabled: true

spec/frontend/jobs/components/log/line_spec.js

@@ -22,7 +22,6 @@ const mockProps = ({ text = 'Running with gitlab-runner 12.1.0 (de7731dd)' } = {
 describe('Job Log Line', () => {
   let wrapper;
   let data;
-  let originalGon;
 
   const createComponent = (props = {}) => {
     wrapper = shallowMount(Line, {
@@ -41,19 +40,10 @@ describe('Job Log Line', () => {
       .attributes();
 
   beforeEach(() => {
-    originalGon = window.gon;
-    window.gon.features = {
-      ciJobLineLinks: false,
-    };
-
     data = mockProps();
     createComponent(data);
   });
 
-  afterEach(() => {
-    window.gon = originalGon;
-  });
-
   it('renders the line number component', () => {
     expect(wrapper.find(LineNumber).exists()).toBe(true);
   });
@@ -66,44 +56,7 @@ describe('Job Log Line', () => {
     expect(findLine().classes()).toContain(data.line.content[0].style);
   });
 
-  describe.each([true, false])('when feature ci_job_line_links enabled = %p', ciJobLineLinks => {
-    beforeEach(() => {
-      window.gon.features = {
-        ciJobLineLinks,
-      };
-    });
-
-    it('renders text with symbols', () => {
-      const text = 'apt-get update < /dev/null > /dev/null';
-      createComponent(mockProps({ text }));
-
-      expect(findLine().text()).toBe(text);
-    });
-
-    it.each`
-      tag         | text
-      ${'a'}      | ${'<a href="#">linked</a>'}
-      ${'script'} | ${'<script>doEvil();</script>'}
-      ${'strong'} | ${'<strong>highlighted</strong>'}
-    `('escapes `<$tag>` tags in text', ({ tag, text }) => {
-      createComponent(mockProps({ text }));
-
-      expect(
-        findLine()
-          .find(tag)
-          .exists(),
-      ).toBe(false);
-      expect(findLine().text()).toBe(text);
-    });
-  });
-
-  describe('when ci_job_line_links is enabled', () => {
-    beforeEach(() => {
-      window.gon.features = {
-        ciJobLineLinks: true,
-      };
-    });
-
+  describe('job urls as links', () => {
     it('renders an http link', () => {
       createComponent(mockProps({ text: httpUrl }));
 
@@ -130,21 +83,21 @@ describe('Job Log Line', () => {
       expect(findLink().classes()).toEqual(['gl-reset-color!', 'gl-text-decoration-underline']);
     });
 
-    it('renders a links with queries, surrounded by questions marks', () => {
+    it('renders links with queries, surrounded by questions marks', () => {
       createComponent(mockProps({ text: `Did you see my url ${queryUrl}??` }));
 
       expect(findLine().text()).toBe('Did you see my url https://example.com?param=val??');
       expect(findLinkAttributeByIndex(0).href).toBe(queryUrl);
     });
 
-    it('renders a links with queries, surrounded by exclamation marks', () => {
+    it('renders links with queries, surrounded by exclamation marks', () => {
       createComponent(mockProps({ text: `No! The ${queryUrl}!?` }));
 
       expect(findLine().text()).toBe('No! The https://example.com?param=val!?');
       expect(findLinkAttributeByIndex(0).href).toBe(queryUrl);
     });
 
-    it('renders a multiple links surrounded by text', () => {
+    it('renders multiple links surrounded by text', () => {
       createComponent(
         mockProps({ text: `Well, my HTTP url: ${httpUrl} and my HTTPS url: ${httpsUrl}` }),
       );
@@ -158,7 +111,7 @@ describe('Job Log Line', () => {
       expect(findLinkAttributeByIndex(1).href).toBe(httpsUrl);
     });
 
-    it('renders a multiple links surrounded by text, with other symbols', () => {
+    it('renders multiple links surrounded by text, with other symbols', () => {
       createComponent(
         mockProps({ text: `${httpUrl}, ${httpUrl}: ${httpsUrl}; ${httpsUrl}. ${httpsUrl}...` }),
       );
@@ -175,15 +128,25 @@ describe('Job Log Line', () => {
       expect(findLinkAttributeByIndex(4).href).toBe(httpsUrl);
     });
 
+    it('renders text with symbols in it', () => {
+      const text = 'apt-get update < /dev/null > /dev/null';
+      createComponent(mockProps({ text }));
+
+      expect(findLine().text()).toBe(text);
+    });
+
     const jshref = 'javascript:doEvil();'; // eslint-disable-line no-script-url
 
-    test.each`
-      type           | text
-      ${'js'}        | ${jshref}
-      ${'file'}      | ${'file:///a-file'}
-      ${'ftp'}       | ${'ftp://example.com/file'}
-      ${'email'}     | ${'email@example.com'}
-      ${'no scheme'} | ${'example.com/page'}
+    it.each`
+      type             | text
+      ${'html link'}   | ${'<a href="#">linked</a>'}
+      ${'html script'} | ${'<script>doEvil();</script>'}
+      ${'html strong'} | ${'<strong>highlighted</strong>'}
+      ${'js'}          | ${jshref}
+      ${'file'}        | ${'file:///a-file'}
+      ${'ftp'}         | ${'ftp://example.com/file'}
+      ${'email'}       | ${'email@example.com'}
+      ${'no scheme'}   | ${'example.com/page'}
     `('does not render a $type link', ({ text }) => {
       createComponent(mockProps({ text }));
       expect(findLink().exists()).toBe(false);