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

app/assets/javascripts/lib/utils/text_utility.js

@@ -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

@@ -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

@@ -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;
     }

spec/javascripts/frequent_items/components/frequent_items_list_item_spec.js

 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

 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

-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 });
-    vm.$nextTick(() => {
-      expect(vm.$el.querySelector('.js-selected-icon.js-selected')).toBeTruthy();
-      done();
-    });
+  it('renders a check mark icon if selected === true', () => {
+    options.propsData.selected = true;
+    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();
-    expect(vm.$emit).toHaveBeenCalledWith('click');
+    wrapper.vm.onClick();
+    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) });
-    vm.$nextTick(() => {
-      const renderedNamespace = trimText(vm.$el.querySelector('.js-project-namespace').textContent);
-      expect(renderedNamespace).toBe('a /');
-      done();
-    });
+  it(`renders a simple namespace name with a trailing slash`, () => {
+    options.propsData.project.name_with_namespace = 'a / b';
+    wrapper = shallowMount(Component, options);
+    const renderedNamespace = trimText(wrapper.find('.js-project-namespace').text());
+    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) });
-    vm.$nextTick(() => {
-      const renderedNamespace = trimText(vm.$el.querySelector('.js-project-namespace').textContent);
-      expect(renderedNamespace).toBe('a / ... / e /');
-      done();
-    });
+  it(`renders a properly truncated namespace with a trailing slash`, () => {
+    options.propsData.project.name_with_namespace = 'a / b / c / d / e / f';
+    wrapper = shallowMount(Component, options);
+    const renderedNamespace = trimText(wrapper.find('.js-project-namespace').text());
+    expect(renderedNamespace).toBe('a / ... / e /');
   });
-  it(`renders the project name`, done => {
-    project.name = 'my-test-project';
-    wrapper.setProps({ project: _.clone(project) });
-    vm.$nextTick(() => {
-      const renderedName = trimText(vm.$el.querySelector('.js-project-name').innerHTML);
-      expect(renderedName).toBe('my-test-project');
-      done();
-    });
+  it(`renders the project name`, () => {
+    options.propsData.project.name = 'my-test-project';
+    wrapper = shallowMount(Component, options);
+    const renderedName = trimText(wrapper.find('.js-project-name').text());
+    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' });
-    vm.$nextTick(() => {
-      const renderedName = trimText(vm.$el.querySelector('.js-project-name').innerHTML);
-      const expected = 'my-test-<b>p</b><b>r</b><b>o</b>ject';
-      expect(renderedName).toBe(expected);
-      done();
-    });
+  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);
+  });
+  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>";
+    wrapper = shallowMount(Component, options);
+    const renderedName = trimText(wrapper.find('.js-project-name').html());
+    const expected = 'my-xss-project';
+    expect(renderedName).toContain(expected);
+    expect(alertSpy).not.toHaveBeenCalled();
   });
 });

spec/javascripts/vue_shared/components/project_selector/project_selector_spec.js

@@ -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'));
-    vm.$nextTick(() => {
-      expect(vm.$emit).not.toHaveBeenCalledWith();
-      jasmine.clock().tick(501);
-      expect(vm.$emit).toHaveBeenCalledWith('searched', query);
-      done();
-    });
+    const searchInput = wrapper.find('.js-project-selector-input');
+    searchInput.setValue(query);
+    searchInput.trigger('input');
+    expect(vm.$emit).not.toHaveBeenCalledWith();
+    jasmine.clock().tick(501);
+    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'));
-        vm.$nextTick(() => {
-          jasmine.clock().tick(400);
-          updateSearchQuery(count + 1);
-        });
+        searchInput.setValue(`search query #${count}`);
+        searchInput.trigger('input');
+        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(trimText(noResultsEl.textContent)).toEqual('Sorry, no projects matched your search');
-      done();
-    });
+    expect(wrapper.contains('.js-no-results-message')).toBe(true);
+    const noResultsEl = wrapper.find('.js-no-results-message');
+    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(trimText(minimumSearchEl.textContent)).toEqual(
-        'Enter at least three characters to search',
-      );
-      done();
-    });
+    expect(wrapper.contains('.js-minimum-search-query-message')).toBe(true);
+    const minimumSearchEl = wrapper.find('.js-minimum-search-query-message');
+    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(trimText(errorMessageEl.textContent)).toEqual(
-        'Something went wrong, unable to search projects',
-      );
-      done();
-    });
+    expect(wrapper.contains('.js-search-error-message')).toBe(true);
+    const errorMessageEl = wrapper.find('.js-search-error-message');
+    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');
-    expect(document.activeElement).not.toBe(input);
+    const input = wrapper.find('.js-project-selector-input');
+    expect(document.activeElement).not.toBe(input.element);
     vm.focusSearchInput();
-    expect(document.activeElement).toBe(input);
+    expect(document.activeElement).toBe(input.element);
   });
 });