Commit 3cb3ef87 authored by Miguel Rincon's avatar Miguel Rincon

Remove ci_job_line_links feature flag

This change removes ci_job_line_links to allow links to be permanently
renders as part of the logs.
parent 7964dde2
...@@ -18,46 +18,33 @@ export default { ...@@ -18,46 +18,33 @@ export default {
render(h, { props }) { render(h, { props }) {
const { line, path } = props; const { line, path } = props;
let chars; const chars = line.content.map(content => {
if (gon?.features?.ciJobLineLinks) { return h(
chars = line.content.map(content => { 'span',
return h( {
'span', class: ['gl-white-space-pre-wrap', content.style],
{ },
class: ['gl-white-space-pre-wrap', content.style], // Simple "tokenization": Split text in chunks of text
}, // which alternate between text and urls.
// Simple "tokenization": Split text in chunks of text content.text.split(linkRegex).map(chunk => {
// which alternate between text and urls. // Return normal string for non-links
content.text.split(linkRegex).map(chunk => { if (!chunk.match(linkRegex)) {
// Return normal string for non-links return chunk;
if (!chunk.match(linkRegex)) { }
return chunk; return h(
} 'a',
return h( {
'a', attrs: {
{ href: chunk,
attrs: { class: 'gl-reset-color! gl-text-decoration-underline',
href: chunk, rel: 'nofollow noopener noreferrer', // eslint-disable-line @gitlab/require-i18n-strings
class: 'gl-reset-color! gl-text-decoration-underline',
rel: 'nofollow noopener noreferrer', // eslint-disable-line @gitlab/require-i18n-strings
},
}, },
chunk, },
); chunk,
}), );
); }),
}); );
} else { });
chars = line.content.map(content => {
return h(
'span',
{
class: ['gl-white-space-pre-wrap', content.style],
},
content.text,
);
});
}
return h('div', { class: 'js-line log-line' }, [ return h('div', { class: 'js-line log-line' }, [
h(LineNumber, { h(LineNumber, {
......
...@@ -14,9 +14,6 @@ class Projects::JobsController < Projects::ApplicationController ...@@ -14,9 +14,6 @@ class Projects::JobsController < Projects::ApplicationController
before_action :verify_api_request!, only: :terminal_websocket_authorize before_action :verify_api_request!, only: :terminal_websocket_authorize
before_action :authorize_create_proxy_build!, only: :proxy_websocket_authorize before_action :authorize_create_proxy_build!, only: :proxy_websocket_authorize
before_action :verify_proxy_request!, 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 before_action only: :index do
frontend_experimentation_tracking_data(:jobs_empty_state, 'click_button') frontend_experimentation_tracking_data(:jobs_empty_state, 'click_button')
end end
......
---
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
...@@ -22,7 +22,6 @@ const mockProps = ({ text = 'Running with gitlab-runner 12.1.0 (de7731dd)' } = { ...@@ -22,7 +22,6 @@ const mockProps = ({ text = 'Running with gitlab-runner 12.1.0 (de7731dd)' } = {
describe('Job Log Line', () => { describe('Job Log Line', () => {
let wrapper; let wrapper;
let data; let data;
let originalGon;
const createComponent = (props = {}) => { const createComponent = (props = {}) => {
wrapper = shallowMount(Line, { wrapper = shallowMount(Line, {
...@@ -41,19 +40,10 @@ describe('Job Log Line', () => { ...@@ -41,19 +40,10 @@ describe('Job Log Line', () => {
.attributes(); .attributes();
beforeEach(() => { beforeEach(() => {
originalGon = window.gon;
window.gon.features = {
ciJobLineLinks: false,
};
data = mockProps(); data = mockProps();
createComponent(data); createComponent(data);
}); });
afterEach(() => {
window.gon = originalGon;
});
it('renders the line number component', () => { it('renders the line number component', () => {
expect(wrapper.find(LineNumber).exists()).toBe(true); expect(wrapper.find(LineNumber).exists()).toBe(true);
}); });
...@@ -66,44 +56,7 @@ describe('Job Log Line', () => { ...@@ -66,44 +56,7 @@ describe('Job Log Line', () => {
expect(findLine().classes()).toContain(data.line.content[0].style); expect(findLine().classes()).toContain(data.line.content[0].style);
}); });
describe.each([true, false])('when feature ci_job_line_links enabled = %p', ciJobLineLinks => { describe('job urls as links', () => {
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,
};
});
it('renders an http link', () => { it('renders an http link', () => {
createComponent(mockProps({ text: httpUrl })); createComponent(mockProps({ text: httpUrl }));
...@@ -130,21 +83,21 @@ describe('Job Log Line', () => { ...@@ -130,21 +83,21 @@ describe('Job Log Line', () => {
expect(findLink().classes()).toEqual(['gl-reset-color!', 'gl-text-decoration-underline']); 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}??` })); createComponent(mockProps({ text: `Did you see my url ${queryUrl}??` }));
expect(findLine().text()).toBe('Did you see my url https://example.com?param=val??'); expect(findLine().text()).toBe('Did you see my url https://example.com?param=val??');
expect(findLinkAttributeByIndex(0).href).toBe(queryUrl); 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}!?` })); createComponent(mockProps({ text: `No! The ${queryUrl}!?` }));
expect(findLine().text()).toBe('No! The https://example.com?param=val!?'); expect(findLine().text()).toBe('No! The https://example.com?param=val!?');
expect(findLinkAttributeByIndex(0).href).toBe(queryUrl); expect(findLinkAttributeByIndex(0).href).toBe(queryUrl);
}); });
it('renders a multiple links surrounded by text', () => { it('renders multiple links surrounded by text', () => {
createComponent( createComponent(
mockProps({ text: `Well, my HTTP url: ${httpUrl} and my HTTPS url: ${httpsUrl}` }), mockProps({ text: `Well, my HTTP url: ${httpUrl} and my HTTPS url: ${httpsUrl}` }),
); );
...@@ -158,7 +111,7 @@ describe('Job Log Line', () => { ...@@ -158,7 +111,7 @@ describe('Job Log Line', () => {
expect(findLinkAttributeByIndex(1).href).toBe(httpsUrl); 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( createComponent(
mockProps({ text: `${httpUrl}, ${httpUrl}: ${httpsUrl}; ${httpsUrl}. ${httpsUrl}...` }), mockProps({ text: `${httpUrl}, ${httpUrl}: ${httpsUrl}; ${httpsUrl}. ${httpsUrl}...` }),
); );
...@@ -175,15 +128,25 @@ describe('Job Log Line', () => { ...@@ -175,15 +128,25 @@ describe('Job Log Line', () => {
expect(findLinkAttributeByIndex(4).href).toBe(httpsUrl); 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 const jshref = 'javascript:doEvil();'; // eslint-disable-line no-script-url
test.each` it.each`
type | text type | text
${'js'} | ${jshref} ${'html link'} | ${'<a href="#">linked</a>'}
${'file'} | ${'file:///a-file'} ${'html script'} | ${'<script>doEvil();</script>'}
${'ftp'} | ${'ftp://example.com/file'} ${'html strong'} | ${'<strong>highlighted</strong>'}
${'email'} | ${'email@example.com'} ${'js'} | ${jshref}
${'no scheme'} | ${'example.com/page'} ${'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 }) => { `('does not render a $type link', ({ text }) => {
createComponent(mockProps({ text })); createComponent(mockProps({ text }));
expect(findLink().exists()).toBe(false); expect(findLink().exists()).toBe(false);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment