Skip to content

G
gitlab-ce
Commit 9dc72acc authored by mfluharty's avatar mfluharty
Browse files
Options

Make corrections to address review feedback 

Refactor tests to follow conventions
Add XSS test
Eliminate a few unnecessary lines, comments, and parameters
Use Vue.set for nested state changes
parent 78530032
Hide whitespace changes
Inline Side-by-side
Showing with 160 additions and 194 deletions
app/assets/javascripts/lib/utils/text_utility.js
View file @ 9dc72acc
......@@ -172,8 +172,8 @@ export const splitCamelCase = string =>
* @param {String} string A string namespace,
* i.e. "My Group / My Subgroup / My Project"
*/
export const truncateNamespace = string => {
if (_.isUndefined(string) || _.isNull(string) || !_.isString(string)) {
export const truncateNamespace = (string = '') => {
if (_.isNull(string) || !_.isString(string)) {
return '';
}
......
app/assets/javascripts/vue_shared/components/project_selector/project_selector.vue
View file @ 9dc72acc
......@@ -56,8 +56,8 @@ export default {
focusSearchInput() {
this.$refs.searchInput.focus();
},
onInput: _.debounce(function debouncedOnInput(e) {
this.$emit('searched', e.target.value);
onInput: _.debounce(function debouncedOnInput() {
this.$emit('searched', this.searchQuery);
}, SEARCH_INPUT_TIMEOUT_MS),
},
};
......
app/assets/stylesheets/components/project_list_item.scss
View file @ 9dc72acc
......@@ -17,9 +17,6 @@
margin-left: -$gl-padding;
margin-right: -$gl-padding;
// should be replaced by Bootstrap's
// .overflow-hidden utility class once
// we upgrade Bootstrap to at least 4.2.x
.project-namespace-name-container {
overflow: hidden;
}
......
ee/app/assets/javascripts/operations/components/dashboard/dashboard.vue
View file @ 9dc72acc
......@@ -125,7 +125,6 @@ export default {
<gl-button
v-if="projects.length"
v-gl-modal="$options.modalId"
type="button"
class="js-add-projects-button btn btn-success"
>
{{ s__('OperationsDashboard|Add projects') }}
......@@ -158,7 +157,6 @@ export default {
<div class="col-12">
<gl-button
v-gl-modal="$options.modalId"
type="button"
class="js-add-projects-button btn btn-success prepend-top-default append-bottom-default"
>
{{ s__('OperationsDashboard|Add projects') }}
......
ee/app/assets/javascripts/operations/store/mutations.js
View file @ 9dc72acc
import _ from 'underscore';
import Vue from 'vue';
import * as types from './mutation_types';
export default {
......@@ -26,10 +26,7 @@ export default {
}
},
[types.REMOVE_SELECTED_PROJECT](state, project) {
state.selectedProjects = _.without(
state.selectedProjects,
..._.where(state.selectedProjects, { id: project.id }),
);
state.selectedProjects = state.selectedProjects.filter(p => p.id !== project.id);
},
[types.REQUEST_PROJECTS](state) {
......@@ -53,22 +50,22 @@ export default {
// Flipping this property separately to allows the UI
// to hide the "minimum query" message
// before the seach results arrive from the API
state.messages.minimumQuery = false;
Vue.set(state.messages, 'minimumQuery', false);
state.searchCount += 1;
},
[types.RECEIVE_SEARCH_RESULTS_SUCCESS](state, results) {
state.projectSearchResults = results;
state.messages.noResults = state.projectSearchResults.length === 0;
state.messages.searchError = false;
state.messages.minimumQuery = false;
Vue.set(state.messages, 'noResults', state.projectSearchResults.length === 0);
Vue.set(state.messages, 'searchError', false);
Vue.set(state.messages, 'minimumQuery', false);
state.searchCount -= 1;
},
[types.RECEIVE_SEARCH_RESULTS_ERROR](state, message) {
state.projectSearchResults = [];
state.messages.noResults = false;
state.messages.searchError = true;
state.messages.minimumQuery = message === 'minimumQuery';
Vue.set(state.messages, 'noResults', false);
Vue.set(state.messages, 'searchError', true);
Vue.set(state.messages, 'minimumQuery', message === 'minimumQuery');
state.searchCount -= 1;
},
};
ee/spec/javascripts/operations/components/dashboard/dashboard_spec.js
View file @ 9dc72acc
......@@ -50,7 +50,7 @@ describe('dashboard component', () => {
expect(button.innerText.trim()).toBe(mockText.ADD_PROJECTS);
});
it('opens the Add Projects modal', () => {
it('renders the projects modal', () => {
button.click();
expect(vm.$el.querySelector('.add-projects-modal')).toBeDefined();
......
spec/javascripts/frequent_items/components/frequent_items_list_item_spec.js
View file @ 9dc72acc
import Vue from 'vue';
import frequentItemsListItemComponent from '~/frequent_items/components/frequent_items_list_item.vue';
import mountComponent from 'spec/helpers/vue_mount_component_helper';
import { shallowMount } from '@vue/test-utils';
import { trimText } from 'spec/helpers/vue_component_helper';
import { mockProject } from '../mock_data'; // can also use 'mockGroup', but not useful to test here
const createComponent = () => {
const Component = Vue.extend(frequentItemsListItemComponent);
return mountComponent(Component, {
itemId: mockProject.id,
itemName: mockProject.name,
namespace: mockProject.namespace,
webUrl: mockProject.webUrl,
avatarUrl: mockProject.avatarUrl,
return shallowMount(Component, {
propsData: {
itemId: mockProject.id,
itemName: mockProject.name,
namespace: mockProject.namespace,
webUrl: mockProject.webUrl,
avatarUrl: mockProject.avatarUrl,
},
});
};
describe('FrequentItemsListItemComponent', () => {
let wrapper;
let vm;
beforeEach(() => {
vm = createComponent();
wrapper = createComponent();
({ vm } = wrapper);
});
afterEach(() => {
......@@ -30,81 +35,61 @@ describe('FrequentItemsListItemComponent', () => {
describe('computed', () => {
describe('hasAvatar', () => {
it('should return `true` or `false` if whether avatar is present or not', () => {
vm.avatarUrl = 'path/to/avatar.png';
wrapper.setProps({ avatarUrl: 'path/to/avatar.png' });
expect(vm.hasAvatar).toBe(true);
vm.avatarUrl = null;
wrapper.setProps({ avatarUrl: null });
expect(vm.hasAvatar).toBe(false);
});
});
describe('highlightedItemName', () => {
it('should enclose part of project name in <b> & </b> which matches with `matcher` prop', done => {
vm.matcher = 'lab';
vm.$nextTick()
.then(() => {
expect(vm.$el.querySelector('.js-frequent-items-item-title').innerHTML).toContain(
'<b>L</b><b>a</b><b>b</b>',
);
})
.then(done)
.catch(done.fail);
it('should enclose part of project name in <b> & </b> which matches with `matcher` prop', () => {
wrapper.setProps({ matcher: 'lab' });
expect(wrapper.find('.js-frequent-items-item-title').html()).toContain(
'<b>L</b><b>a</b><b>b</b>',
);
});
it('should return project name as it is if `matcher` is not available', done => {
vm.matcher = null;
vm.$nextTick()
.then(() => {
expect(vm.$el.querySelector('.js-frequent-items-item-title').innerHTML).toBe(
mockProject.name,
);
})
.then(done)
.catch(done.fail);
it('should return project name as it is if `matcher` is not available', () => {
wrapper.setProps({ matcher: null });
expect(trimText(wrapper.find('.js-frequent-items-item-title').text())).toBe(
mockProject.name,
);
});
});
describe('truncatedNamespace', () => {
it('should truncate project name from namespace string', done => {
vm.namespace = 'platform / nokia-3310';
vm.$nextTick()
.then(() => {
expect(
trimText(vm.$el.querySelector('.js-frequent-items-item-namespace').innerHTML),
).toBe('platform');
})
.then(done)
.catch(done.fail);
it('should truncate project name from namespace string', () => {
wrapper.setProps({ namespace: 'platform / nokia-3310' });
expect(trimText(wrapper.find('.js-frequent-items-item-namespace').text())).toBe('platform');
});
it('should truncate namespace string from the middle if it includes more than two groups in path', done => {
vm.namespace = 'platform / hardware / broadcom / Wifi Group / Mobile Chipset / nokia-3310';
vm.$nextTick()
.then(() => {
expect(
trimText(vm.$el.querySelector('.js-frequent-items-item-namespace').innerHTML),
).toBe('platform / ... / Mobile Chipset');
})
.then(done)
.catch(done.fail);
it('should truncate namespace string from the middle if it includes more than two groups in path', () => {
wrapper.setProps({
namespace: 'platform / hardware / broadcom / Wifi Group / Mobile Chipset / nokia-3310',
});
expect(trimText(wrapper.find('.js-frequent-items-item-namespace').text())).toBe(
'platform / ... / Mobile Chipset',
);
});
});
});
describe('template', () => {
it('should render component element', () => {
expect(vm.$el.classList.contains('frequent-items-list-item-container')).toBeTruthy();
expect(vm.$el.querySelectorAll('a').length).toBe(1);
expect(vm.$el.querySelectorAll('.frequent-items-item-avatar-container').length).toBe(1);
expect(vm.$el.querySelectorAll('.frequent-items-item-metadata-container').length).toBe(1);
expect(vm.$el.querySelectorAll('.js-frequent-items-item-title').length).toBe(1);
expect(vm.$el.querySelectorAll('.js-frequent-items-item-namespace').length).toBe(1);
expect(wrapper.classes()).toContain('frequent-items-list-item-container');
expect(wrapper.findAll('a').length).toBe(1);
expect(wrapper.findAll('.frequent-items-item-avatar-container').length).toBe(1);
expect(wrapper.findAll('.frequent-items-item-metadata-container').length).toBe(1);
expect(wrapper.findAll('.frequent-items-item-title').length).toBe(1);
expect(wrapper.findAll('.frequent-items-item-namespace').length).toBe(1);
});
});
});
spec/javascripts/frequent_items/components/frequent_items_search_input_spec.js
View file @ 9dc72acc
import Vue from 'vue';
import searchComponent from '~/frequent_items/components/frequent_items_search_input.vue';
import eventHub from '~/frequent_items/event_hub';
import mountComponent from 'spec/helpers/vue_mount_component_helper';
import { shallowMount } from '@vue/test-utils';
const createComponent = (namespace = 'projects') => {
const Component = Vue.extend(searchComponent);
return mountComponent(Component, { namespace });
return shallowMount(Component, { propsData: { namespace } });
};
describe('FrequentItemsSearchInputComponent', () => {
let wrapper;
let vm;
beforeEach(() => {
vm = createComponent();
wrapper = createComponent();
({ vm } = wrapper);
});
afterEach(() => {
......@@ -35,7 +38,7 @@ describe('FrequentItemsSearchInputComponent', () => {
describe('mounted', () => {
it('should listen `dropdownOpen` event', done => {
spyOn(eventHub, '$on');
const vmX = createComponent();
const vmX = createComponent().vm;
Vue.nextTick(() => {
expect(eventHub.$on).toHaveBeenCalledWith(
......@@ -49,7 +52,7 @@ describe('FrequentItemsSearchInputComponent', () => {
describe('beforeDestroy', () => {
it('should unbind event listeners on eventHub', done => {
const vmX = createComponent();
const vmX = createComponent().vm;
spyOn(eventHub, '$off');
vmX.$mount();
......@@ -67,12 +70,12 @@ describe('FrequentItemsSearchInputComponent', () => {
describe('template', () => {
it('should render component element', () => {
const inputEl = vm.$el.querySelector('input.form-control');
expect(vm.$el.classList.contains('search-input-container')).toBeTruthy();
expect(inputEl).not.toBe(null);
expect(inputEl.getAttribute('placeholder')).toBe('Search your projects');
expect(vm.$el.querySelector('.search-icon')).toBeDefined();
expect(wrapper.classes()).toContain('search-input-container');
expect(wrapper.contains('input.form-control')).toBe(true);
expect(wrapper.contains('.search-icon')).toBe(true);
expect(wrapper.find('input.form-control').attributes('placeholder')).toBe(
'Search your projects',
);
});
});
});
spec/javascripts/vue_shared/components/project_selector/project_list_item_spec.js
View file @ 9dc72acc
import _ from 'underscore';
import ProjectListItem from '~/vue_shared/components/project_selector/project_list_item.vue';
import { shallowMount, createLocalVue } from '@vue/test-utils';
import { trimText } from 'spec/helpers/vue_component_helper';
......@@ -6,99 +5,106 @@ import { trimText } from 'spec/helpers/vue_component_helper';
const localVue = createLocalVue();
describe('ProjectListItem component', () => {
const Component = localVue.extend(ProjectListItem);
let wrapper;
let vm;
let options;
loadJSONFixtures('projects.json');
const project = getJSONFixture('projects.json')[0];
beforeEach(() => {
wrapper = shallowMount(localVue.extend(ProjectListItem), {
options = {
propsData: {
project,
selected: false,
},
sync: false,
localVue,
});
({ vm } = wrapper);
};
});
afterEach(() => {
vm.$destroy();
wrapper.vm.$destroy();
});
it('does not render a check mark icon if selected === false', () => {
expect(vm.$el.querySelector('.js-selected-icon.js-unselected')).toBeTruthy();
wrapper = shallowMount(Component, options);
expect(wrapper.contains('.js-selected-icon.js-unselected')).toBe(true);
});
it('renders a check mark icon if selected === true', done => {
wrapper.setProps({ selected: true });
it('renders a check mark icon if selected === true', () => {
options.propsData.selected = true;
vm.$nextTick(() => {
expect(vm.$el.querySelector('.js-selected-icon.js-selected')).toBeTruthy();
done();
});
wrapper = shallowMount(Component, options);
expect(wrapper.contains('.js-selected-icon.js-selected')).toBe(true);
});
it(`emits a "clicked" event when clicked`, () => {
wrapper = shallowMount(Component, options);
({ vm } = wrapper);
spyOn(vm, '$emit');
vm.onClick();
wrapper.vm.onClick();
expect(vm.$emit).toHaveBeenCalledWith('click');
expect(wrapper.vm.$emit).toHaveBeenCalledWith('click');
});
it(`renders the project avatar`, () => {
expect(vm.$el.querySelector('.js-project-avatar')).toBeTruthy();
wrapper = shallowMount(Component, options);
expect(wrapper.contains('.js-project-avatar')).toBe(true);
});
it(`renders a simple namespace name with a trailing slash`, done => {
project.name_with_namespace = 'a / b';
wrapper.setProps({ project: _.clone(project) });
it(`renders a simple namespace name with a trailing slash`, () => {
options.propsData.project.name_with_namespace = 'a / b';
vm.$nextTick(() => {
const renderedNamespace = trimText(vm.$el.querySelector('.js-project-namespace').textContent);
wrapper = shallowMount(Component, options);
const renderedNamespace = trimText(wrapper.find('.js-project-namespace').text());
expect(renderedNamespace).toBe('a /');
done();
});
expect(renderedNamespace).toBe('a /');
});
it(`renders a properly truncated namespace with a trailing slash`, done => {
project.name_with_namespace = 'a / b / c / d / e / f';
wrapper.setProps({ project: _.clone(project) });
it(`renders a properly truncated namespace with a trailing slash`, () => {
options.propsData.project.name_with_namespace = 'a / b / c / d / e / f';
vm.$nextTick(() => {
const renderedNamespace = trimText(vm.$el.querySelector('.js-project-namespace').textContent);
wrapper = shallowMount(Component, options);
const renderedNamespace = trimText(wrapper.find('.js-project-namespace').text());
expect(renderedNamespace).toBe('a / ... / e /');
done();
});
expect(renderedNamespace).toBe('a / ... / e /');
});
it(`renders the project name`, done => {
project.name = 'my-test-project';
wrapper.setProps({ project: _.clone(project) });
it(`renders the project name`, () => {
options.propsData.project.name = 'my-test-project';
vm.$nextTick(() => {
const renderedName = trimText(vm.$el.querySelector('.js-project-name').innerHTML);
wrapper = shallowMount(Component, options);
const renderedName = trimText(wrapper.find('.js-project-name').text());
expect(renderedName).toBe('my-test-project');
done();
});
expect(renderedName).toBe('my-test-project');
});
it(`renders the project name with highlighting in the case of a search query match`, done => {
project.name = 'my-test-project';
wrapper.setProps({ project: _.clone(project), matcher: 'pro' });
it(`renders the project name with highlighting in the case of a search query match`, () => {
options.propsData.project.name = 'my-test-project';
options.propsData.matcher = 'pro';
wrapper = shallowMount(Component, options);
const renderedName = trimText(wrapper.find('.js-project-name').html());
const expected = 'my-test-<b>p</b><b>r</b><b>o</b>ject';
expect(renderedName).toContain(expected);
});
vm.$nextTick(() => {
const renderedName = trimText(vm.$el.querySelector('.js-project-name').innerHTML);
it('prevents search query and project name XSS', () => {
const alertSpy = spyOn(window, 'alert');
options.propsData.project.name = "my-xss-pro<script>alert('XSS');</script>ject";
options.propsData.matcher = "pro<script>alert('XSS');</script>";
const expected = 'my-test-<b>p</b><b>r</b><b>o</b>ject';
wrapper = shallowMount(Component, options);
const renderedName = trimText(wrapper.find('.js-project-name').html());
const expected = 'my-xss-project';
expect(renderedName).toBe(expected);
done();
});
expect(renderedName).toContain(expected);
expect(alertSpy).not.toHaveBeenCalled();
});
});
spec/javascripts/vue_shared/components/project_selector/project_selector_spec.js
View file @ 9dc72acc
......@@ -38,28 +38,25 @@ describe('ProjectSelector component', () => {
});
it('renders the search results', () => {
expect(vm.$el.querySelectorAll('.js-project-list-item').length).toBe(5);
expect(wrapper.findAll('.js-project-list-item').length).toBe(5);
});
it(`triggers a (debounced) search when the search input value changes`, done => {
it(`triggers a (debounced) search when the search input value changes`, () => {
spyOn(vm, '$emit');
const query = 'my test query!';
const searchInput = vm.$el.querySelector('.js-project-selector-input');
searchInput.value = query;
searchInput.dispatchEvent(new Event('input'));
const searchInput = wrapper.find('.js-project-selector-input');
searchInput.setValue(query);
searchInput.trigger('input');
vm.$nextTick(() => {
expect(vm.$emit).not.toHaveBeenCalledWith();
jasmine.clock().tick(501);
expect(vm.$emit).not.toHaveBeenCalledWith();
jasmine.clock().tick(501);
expect(vm.$emit).toHaveBeenCalledWith('searched', query);
done();
});
expect(vm.$emit).toHaveBeenCalledWith('searched', query);
});
it(`debounces the search input`, done => {
it(`debounces the search input`, () => {
spyOn(vm, '$emit');
const searchInput = vm.$el.querySelector('.js-project-selector-input');
const searchInput = wrapper.find('.js-project-selector-input');
const updateSearchQuery = (count = 0) => {
if (count === 10) {
......@@ -67,15 +64,12 @@ describe('ProjectSelector component', () => {
expect(vm.$emit).toHaveBeenCalledTimes(1);
expect(vm.$emit).toHaveBeenCalledWith('searched', `search query #9`);
done();
} else {
searchInput.value = `search query #${count}`;
searchInput.dispatchEvent(new Event('input'));
searchInput.setValue(`search query #${count}`);
searchInput.trigger('input');
vm.$nextTick(() => {
jasmine.clock().tick(400);
updateSearchQuery(count + 1);
});
jasmine.clock().tick(400);
updateSearchQuery(count + 1);
}
};
......@@ -83,7 +77,7 @@ describe('ProjectSelector component', () => {
});
it(`includes a placeholder in the search box`, () => {
expect(vm.$el.querySelector('.js-project-selector-input').placeholder).toBe(
expect(wrapper.find('.js-project-selector-input').attributes('placeholder')).toBe(
'Search your projects',
);
});
......@@ -95,58 +89,44 @@ describe('ProjectSelector component', () => {
expect(vm.$emit).toHaveBeenCalledWith('projectClicked', _.first(searchResults));
});
it(`shows a "no results" message if showNoResultsMessage === true`, done => {
it(`shows a "no results" message if showNoResultsMessage === true`, () => {
wrapper.setProps({ showNoResultsMessage: true });
vm.$nextTick(() => {
const noResultsEl = vm.$el.querySelector('.js-no-results-message');
expect(noResultsEl).toBeTruthy();
expect(wrapper.contains('.js-no-results-message')).toBe(true);
expect(trimText(noResultsEl.textContent)).toEqual('Sorry, no projects matched your search');
const noResultsEl = wrapper.find('.js-no-results-message');
done();
});
expect(trimText(noResultsEl.text())).toEqual('Sorry, no projects matched your search');
});
it(`shows a "minimum seach query" message if showMinimumSearchQueryMessage === true`, done => {
it(`shows a "minimum seach query" message if showMinimumSearchQueryMessage === true`, () => {
wrapper.setProps({ showMinimumSearchQueryMessage: true });
vm.$nextTick(() => {
const minimumSearchEl = vm.$el.querySelector('.js-minimum-search-query-message');
expect(minimumSearchEl).toBeTruthy();
expect(wrapper.contains('.js-minimum-search-query-message')).toBe(true);
expect(trimText(minimumSearchEl.textContent)).toEqual(
'Enter at least three characters to search',
);
const minimumSearchEl = wrapper.find('.js-minimum-search-query-message');
done();
});
expect(trimText(minimumSearchEl.text())).toEqual('Enter at least three characters to search');
});
it(`shows a error message if showSearchErrorMessage === true`, done => {
it(`shows a error message if showSearchErrorMessage === true`, () => {
wrapper.setProps({ showSearchErrorMessage: true });
vm.$nextTick(() => {
const errorMessageEl = vm.$el.querySelector('.js-search-error-message');
expect(errorMessageEl).toBeTruthy();
expect(wrapper.contains('.js-search-error-message')).toBe(true);
expect(trimText(errorMessageEl.textContent)).toEqual(
'Something went wrong, unable to search projects',
);
const errorMessageEl = wrapper.find('.js-search-error-message');
done();
});
expect(trimText(errorMessageEl.text())).toEqual(
'Something went wrong, unable to search projects',
);
});
it(`focuses the input element when the focusSearchInput() method is called`, () => {
const input = vm.$el.querySelector('.js-project-selector-input');
const input = wrapper.find('.js-project-selector-input');
expect(document.activeElement).not.toBe(input);
expect(document.activeElement).not.toBe(input.element);
vm.focusSearchInput();
expect(document.activeElement).toBe(input);
expect(document.activeElement).toBe(input.element);
});
});
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7