From e804c6a4da58060662e5af75801deef512dee0bc Mon Sep 17 00:00:00 2001 From: Samantha Ming Date: Tue, 31 Mar 2020 22:19:00 -0700 Subject: [PATCH 1/6] Show commits by author - Add dropdown to filter commits by author - Disable filter by commit message when filtering by author - Add popover message and disable for commit message OR author filter --- .../pages/projects/commits/show/index.js | 3 + .../stylesheets/framework/dropdowns.scss | 2 +- app/views/projects/commits/show.html.haml | 3 +- .../14984-show-commits-by-author.yml | 5 + .../commits/components/author_select.vue | 124 +++++++++++ .../javascripts/projects/commits/index.js | 22 ++ .../projects/commits/store/actions.js | 25 +++ .../projects/commits/store/index.js | 15 ++ .../projects/commits/store/mutation_types.js | 2 + .../projects/commits/store/mutations.js | 10 + .../projects/commits/store/state.js | 5 + .../commits/components/author_select_spec.js | 197 ++++++++++++++++++ .../projects/commits/store/actions_spec.js | 47 +++++ .../projects/commits/store/mutations_spec.js | 43 ++++ locale/gitlab.pot | 12 ++ package.json | 2 +- 16 files changed, 514 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/14984-show-commits-by-author.yml create mode 100644 ee/app/assets/javascripts/projects/commits/components/author_select.vue create mode 100644 ee/app/assets/javascripts/projects/commits/index.js create mode 100644 ee/app/assets/javascripts/projects/commits/store/actions.js create mode 100644 ee/app/assets/javascripts/projects/commits/store/index.js create mode 100644 ee/app/assets/javascripts/projects/commits/store/mutation_types.js create mode 100644 ee/app/assets/javascripts/projects/commits/store/mutations.js create mode 100644 ee/app/assets/javascripts/projects/commits/store/state.js create mode 100644 ee/spec/frontend/projects/commits/components/author_select_spec.js create mode 100644 ee/spec/frontend/projects/commits/store/actions_spec.js create mode 100644 ee/spec/frontend/projects/commits/store/mutations_spec.js diff --git a/app/assets/javascripts/pages/projects/commits/show/index.js b/app/assets/javascripts/pages/projects/commits/show/index.js index ad671ce93516eb..890a80767d5c80 100644 --- a/app/assets/javascripts/pages/projects/commits/show/index.js +++ b/app/assets/javascripts/pages/projects/commits/show/index.js @@ -2,8 +2,11 @@ import CommitsList from '~/commits'; import GpgBadges from '~/gpg_badges'; import ShortcutsNavigation from '~/behaviors/shortcuts/shortcuts_navigation'; +import mountCommits from 'ee/projects/commits'; + document.addEventListener('DOMContentLoaded', () => { new CommitsList(document.querySelector('.js-project-commits-show').dataset.commitsLimit); // eslint-disable-line no-new new ShortcutsNavigation(); // eslint-disable-line no-new GpgBadges.fetch(); + mountCommits(document.getElementById('js-author-dropdown')); }); diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index b6edadb05a9c3e..c62a9668103a6e 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -317,7 +317,7 @@ } } - .dropdown-item { + li:not(.gl-new-dropdown-item) .dropdown-item { @include dropdown-link; } diff --git a/app/views/projects/commits/show.html.haml b/app/views/projects/commits/show.html.haml index 3f1d44a488aaaf..3bc59bc634b5fb 100644 --- a/app/views/projects/commits/show.html.haml +++ b/app/views/projects/commits/show.html.haml @@ -13,7 +13,8 @@ %ul.breadcrumb.repo-breadcrumb = commits_breadcrumbs - .tree-controls.d-none.d-sm-none.d-md-block< + .tree-insert-vue-here#js-author-dropdown{ data: { 'commits_path': project_commits_path(@project), 'project_id': @project.id } } + .tree-controls.d-none.d-sm-none.d-md-block - if @merge_request.present? .control = link_to _("View open merge request"), project_merge_request_path(@project, @merge_request), class: 'btn' diff --git a/changelogs/unreleased/14984-show-commits-by-author.yml b/changelogs/unreleased/14984-show-commits-by-author.yml new file mode 100644 index 00000000000000..89f90c074ca6fe --- /dev/null +++ b/changelogs/unreleased/14984-show-commits-by-author.yml @@ -0,0 +1,5 @@ +--- +title: Add ability to filter commits by author +merge_request: 28509 +author: +type: added diff --git a/ee/app/assets/javascripts/projects/commits/components/author_select.vue b/ee/app/assets/javascripts/projects/commits/components/author_select.vue new file mode 100644 index 00000000000000..71d9f143146c17 --- /dev/null +++ b/ee/app/assets/javascripts/projects/commits/components/author_select.vue @@ -0,0 +1,124 @@ + + + diff --git a/ee/app/assets/javascripts/projects/commits/index.js b/ee/app/assets/javascripts/projects/commits/index.js new file mode 100644 index 00000000000000..119ba67a9a62f2 --- /dev/null +++ b/ee/app/assets/javascripts/projects/commits/index.js @@ -0,0 +1,22 @@ +import Vue from 'vue'; +import Vuex from 'vuex'; +import AuthorSelectApp from './components/author_select.vue'; +import store from './store'; + +Vue.use(Vuex); + +export default el => { + if (!el) { + return null; + } + + store.dispatch('setInitialData', el.dataset); + + return new Vue({ + el, + store, + render(h) { + return h(AuthorSelectApp); + }, + }); +}; diff --git a/ee/app/assets/javascripts/projects/commits/store/actions.js b/ee/app/assets/javascripts/projects/commits/store/actions.js new file mode 100644 index 00000000000000..77c5ba3627cdc5 --- /dev/null +++ b/ee/app/assets/javascripts/projects/commits/store/actions.js @@ -0,0 +1,25 @@ +import * as types from './mutation_types'; +import axios from '~/lib/utils/axios_utils'; + +export default { + setInitialData({ commit }, data) { + commit(types.SET_INITIAL_DATA, data); + }, + receiveAuthors({ commit }, authors) { + commit(types.COMMITS_AUTHORS, authors); + }, + fetchAuthors({ dispatch, state }, author = null) { + const { projectId } = state; + const path = '/autocomplete/users.json'; + + return axios + .get(path, { + params: { + project_id: projectId, + active: true, + search: author, + }, + }) + .then(response => dispatch('receiveAuthors', response.data)); + }, +}; diff --git a/ee/app/assets/javascripts/projects/commits/store/index.js b/ee/app/assets/javascripts/projects/commits/store/index.js new file mode 100644 index 00000000000000..e864ef5716e494 --- /dev/null +++ b/ee/app/assets/javascripts/projects/commits/store/index.js @@ -0,0 +1,15 @@ +import Vue from 'vue'; +import Vuex from 'vuex'; +import actions from './actions'; +import mutations from './mutations'; +import state from './state'; + +Vue.use(Vuex); + +export const createStore = () => ({ + actions, + mutations, + state: state(), +}); + +export default new Vuex.Store(createStore()); diff --git a/ee/app/assets/javascripts/projects/commits/store/mutation_types.js b/ee/app/assets/javascripts/projects/commits/store/mutation_types.js new file mode 100644 index 00000000000000..0a6a5a0b902807 --- /dev/null +++ b/ee/app/assets/javascripts/projects/commits/store/mutation_types.js @@ -0,0 +1,2 @@ +export const SET_INITIAL_DATA = 'SET_INITIAL_DATA'; +export const COMMITS_AUTHORS = 'COMMITS_AUTHORS'; diff --git a/ee/app/assets/javascripts/projects/commits/store/mutations.js b/ee/app/assets/javascripts/projects/commits/store/mutations.js new file mode 100644 index 00000000000000..11f703c0946176 --- /dev/null +++ b/ee/app/assets/javascripts/projects/commits/store/mutations.js @@ -0,0 +1,10 @@ +import * as types from './mutation_types'; + +export default { + [types.SET_INITIAL_DATA](state, data) { + Object.assign(state, data); + }, + [types.COMMITS_AUTHORS](state, data) { + state.commitsAuthors = data; + }, +}; diff --git a/ee/app/assets/javascripts/projects/commits/store/state.js b/ee/app/assets/javascripts/projects/commits/store/state.js new file mode 100644 index 00000000000000..f074708ffa25d6 --- /dev/null +++ b/ee/app/assets/javascripts/projects/commits/store/state.js @@ -0,0 +1,5 @@ +export default () => ({ + commitsPath: null, + projectId: null, + commitsAuthors: [], +}); diff --git a/ee/spec/frontend/projects/commits/components/author_select_spec.js b/ee/spec/frontend/projects/commits/components/author_select_spec.js new file mode 100644 index 00000000000000..282080428d499c --- /dev/null +++ b/ee/spec/frontend/projects/commits/components/author_select_spec.js @@ -0,0 +1,197 @@ +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import Vuex from 'vuex'; +import * as urlUtility from '~/lib/utils/url_utility'; +import AuthorSelect from 'ee/projects/commits/components/author_select.vue'; +import { createStore } from 'ee/projects/commits/store'; +import { + GlNewDropdown, + GlNewDropdownHeader, + GlSearchBoxByType, + GlNewDropdownItem, +} from '@gitlab/ui'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +const commitsPath = 'author/search/url'; +const currentAuthor = 'lorem'; +const authors = [ + { + id: 1, + name: currentAuthor, + username: 'ipsum', + avatar_url: 'some/url', + }, + { + id: 2, + name: 'lorem2', + username: 'ipsum2', + avatar_url: 'some/url/2', + }, +]; + +describe('Author Select', () => { + let wrapper; + let store; + + const createComponent = () => { + wrapper = shallowMount(AuthorSelect, { + localVue, + store: new Vuex.Store(store), + attachToDocument: true, + computed: { + currentAuthor: () => currentAuthor, + }, + }); + }; + + beforeEach(() => { + store = createStore(); + store.actions.fetchAuthors = jest.fn(); + + jest.spyOn(store.actions, 'fetchAuthors'); + + const commitsSearchInput = document.createElement('input'); + commitsSearchInput.id = 'commits-search'; + document.body.appendChild(commitsSearchInput); + + const commitListElement = document.createElement('div'); + commitListElement.id = 'commits-list'; + document.body.appendChild(commitListElement); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + const findDropdownContainer = () => wrapper.find({ ref: 'dropdownContainer' }); + const findDropdown = () => wrapper.find(GlNewDropdown); + const findDropdownHeader = () => wrapper.find(GlNewDropdownHeader); + const findSearchBox = () => wrapper.find(GlSearchBoxByType); + const findDropdownItems = () => wrapper.findAll(GlNewDropdownItem); + + describe('user is searching via "filter by commit message"', () => { + beforeEach(() => { + createComponent(); + }); + + it('disables dropdown container', () => { + wrapper.vm.hasSearchParam = true; + + return wrapper.vm.$nextTick().then(() => { + expect(findDropdownContainer().attributes('disabled')).toBeFalsy(); + }); + }); + + it('has correct tooltip message', () => { + expect(findDropdownContainer().attributes('title')).toBe( + 'Filtering by both author and message is not supported at the moment', + ); + }); + + it('disables dropdown', () => { + wrapper.vm.hasSearchParam = false; + + return wrapper.vm.$nextTick().then(() => { + expect(findDropdown().attributes('disabled')).toBeFalsy(); + }); + }); + + describe('dropdown', () => { + it('displays correct text', () => { + createComponent(); + expect(findDropdown().attributes('text')).toBe('Author'); + }); + + it('displays correct header text', () => { + createComponent(); + expect(findDropdownHeader().text()).toBe('Filter by author'); + }); + }); + + describe('dropdown search box', () => { + it('has correct placeholder', () => { + createComponent(); + expect(findSearchBox().attributes('placeholder')).toBe('Search authors'); + }); + + it('fetch authors on input change', () => { + const authorName = 'lorem'; + createComponent(); + + findSearchBox().vm.$emit('input', authorName); + + expect(store.actions.fetchAuthors).toHaveBeenCalledWith( + expect.anything(), + authorName, + undefined, + ); + }); + }); + + describe('dropdown list', () => { + beforeEach(() => { + store.state.commitsAuthors = authors; + store.state.commitsPath = commitsPath; + createComponent(); + }); + + it('displays the project authors', () => { + return wrapper.vm.$nextTick().then(() => { + expect(findDropdownItems().length).toBe(authors.length + 1); + }); + }); + + it('has the correct props', () => { + const [{ avatar_url, username }] = authors; + + return wrapper.vm.$nextTick().then(() => { + const result = { + avatarUrl: avatar_url, + secondaryText: username, + isChecked: true, + }; + + expect( + findDropdownItems() + .at(1) + .props(), + ).toEqual(expect.objectContaining(result)); + }); + }); + + it("display the author's name", () => { + return wrapper.vm.$nextTick().then(() => { + expect( + findDropdownItems() + .at(1) + .text(), + ).toBe(currentAuthor); + }); + }); + + it('passes selected author to redirectPath', () => { + const redirectToUrl = `${commitsPath}?author=${currentAuthor}`; + const spy = jest.spyOn(urlUtility, 'redirectTo'); + spy.mockImplementation(() => 'mock'); + + findDropdownItems() + .at(1) + .vm.$emit('click'); + + expect(spy).toHaveBeenCalledWith(redirectToUrl); + }); + + it('does not pass any author to redirectPath', () => { + const redirectToUrl = commitsPath; + const spy = jest.spyOn(urlUtility, 'redirectTo'); + spy.mockImplementation(); + + findDropdownItems() + .at(0) + .vm.$emit('click'); + expect(spy).toHaveBeenCalledWith(redirectToUrl); + }); + }); + }); +}); diff --git a/ee/spec/frontend/projects/commits/store/actions_spec.js b/ee/spec/frontend/projects/commits/store/actions_spec.js new file mode 100644 index 00000000000000..3fff0037aef92d --- /dev/null +++ b/ee/spec/frontend/projects/commits/store/actions_spec.js @@ -0,0 +1,47 @@ +import axios from 'axios'; +import MockAdapter from 'axios-mock-adapter'; +import * as types from 'ee/projects/commits/store/mutation_types'; +import testAction from 'helpers/vuex_action_helper'; +import actions from 'ee/projects/commits/store/actions'; +import createState from 'ee/projects/commits/store/state'; + +describe('Project commits actions', () => { + let state; + let mock; + + beforeEach(() => { + state = createState(); + mock = new MockAdapter(axios); + }); + + afterEach(() => { + mock.restore(); + }); + + describe('setInitialData', () => { + it(`commits ${types.SET_INITIAL_DATA}`, () => + testAction(actions.setInitialData, undefined, state, [{ type: types.SET_INITIAL_DATA }])); + }); + + describe('receiveAuthors', () => { + it(`commits ${types.COMMITS_AUTHORS}`, () => + testAction(actions.receiveAuthors, undefined, state, [{ type: types.COMMITS_AUTHORS }])); + }); + + describe('fetchAuthors', () => { + it('dispatches request/receive', () => { + const path = '/autocomplete/users.json'; + state.projectId = '8'; + const data = [{ id: 1 }]; + + mock.onGet(path).replyOnce(200, data); + testAction( + actions.fetchAuthors, + null, + state, + [], + [{ type: 'receiveAuthors', payload: data }], + ); + }); + }); +}); diff --git a/ee/spec/frontend/projects/commits/store/mutations_spec.js b/ee/spec/frontend/projects/commits/store/mutations_spec.js new file mode 100644 index 00000000000000..e4fe76ba9b8d18 --- /dev/null +++ b/ee/spec/frontend/projects/commits/store/mutations_spec.js @@ -0,0 +1,43 @@ +import * as types from 'ee/projects/commits/store/mutation_types'; +import mutations from 'ee/projects/commits/store/mutations'; +import createState from 'ee/projects/commits/store/state'; + +describe('Project commits mutations', () => { + let state; + + beforeEach(() => { + state = createState(); + }); + + afterEach(() => { + state = null; + }); + + describe(types.SET_INITIAL_DATA, () => { + it('sets initial data', () => { + state.commitsPath = null; + state.projectId = null; + state.commitsAuthors = []; + + const data = { + commitsPath: 'some/path', + projectId: '8', + }; + + mutations[types.SET_INITIAL_DATA](state, data); + + expect(state).toEqual(expect.objectContaining(data)); + }); + }); + + describe(types.COMMITS_AUTHORS, () => { + it('sets commitsAuthors', () => { + const authors = [{ id: 1 }, { id: 2 }]; + state.commitsAuthors = []; + + mutations[types.COMMITS_AUTHORS](state, authors); + + expect(state.commitsAuthors).toEqual(authors); + }); + }); +}); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 58d2b4b9586028..3edb0764ca0e5d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2173,6 +2173,9 @@ msgstr "" msgid "Any" msgstr "" +msgid "Any Author" +msgstr "" + msgid "Any Label" msgstr "" @@ -9063,6 +9066,9 @@ msgstr "" msgid "Filter by %{issuable_type} that are currently opened." msgstr "" +msgid "Filter by author" +msgstr "" + msgid "Filter by commit message" msgstr "" @@ -9093,6 +9099,9 @@ msgstr "" msgid "Filter..." msgstr "" +msgid "Filtering by both author and message is not supported at the moment" +msgstr "" + msgid "Find by path" msgstr "" @@ -17685,6 +17694,9 @@ msgstr "" msgid "Search an environment spec" msgstr "" +msgid "Search authors" +msgstr "" + msgid "Search branches" msgstr "" diff --git a/package.json b/package.json index a48b2468ea8a5b..a08f073a743315 100644 --- a/package.json +++ b/package.json @@ -214,4 +214,4 @@ "node": ">=10.13.0", "yarn": "^1.10.0" } -} +} \ No newline at end of file -- GitLab From c0c31d2c69b38093b2d0e2140adc7bd88c6911c5 Mon Sep 17 00:00:00 2001 From: Samantha Ming Date: Wed, 8 Apr 2020 16:12:14 -0700 Subject: [PATCH 2/6] Move away from EE folder into CE --- .../javascripts/pages/projects/commits/show/index.js | 2 +- .../projects/commits/components/author_select.vue | 0 .../assets/javascripts/projects/commits/index.js | 0 .../javascripts/projects/commits/store/actions.js | 0 .../assets/javascripts/projects/commits/store/index.js | 0 .../projects/commits/store/mutation_types.js | 0 .../javascripts/projects/commits/store/mutations.js | 0 .../assets/javascripts/projects/commits/store/state.js | 0 package.json | 2 +- .../projects/commits/components/author_select_spec.js | 4 ++-- .../frontend/projects/commits/store/actions_spec.js | 6 +++--- .../frontend/projects/commits/store/mutations_spec.js | 10 +++++----- 12 files changed, 12 insertions(+), 12 deletions(-) rename {ee/app => app}/assets/javascripts/projects/commits/components/author_select.vue (100%) rename {ee/app => app}/assets/javascripts/projects/commits/index.js (100%) rename {ee/app => app}/assets/javascripts/projects/commits/store/actions.js (100%) rename {ee/app => app}/assets/javascripts/projects/commits/store/index.js (100%) rename {ee/app => app}/assets/javascripts/projects/commits/store/mutation_types.js (100%) rename {ee/app => app}/assets/javascripts/projects/commits/store/mutations.js (100%) rename {ee/app => app}/assets/javascripts/projects/commits/store/state.js (100%) rename {ee/spec => spec}/frontend/projects/commits/components/author_select_spec.js (97%) rename {ee/spec => spec}/frontend/projects/commits/store/actions_spec.js (85%) rename {ee/spec => spec}/frontend/projects/commits/store/mutations_spec.js (73%) diff --git a/app/assets/javascripts/pages/projects/commits/show/index.js b/app/assets/javascripts/pages/projects/commits/show/index.js index 890a80767d5c80..b456baac612149 100644 --- a/app/assets/javascripts/pages/projects/commits/show/index.js +++ b/app/assets/javascripts/pages/projects/commits/show/index.js @@ -2,7 +2,7 @@ import CommitsList from '~/commits'; import GpgBadges from '~/gpg_badges'; import ShortcutsNavigation from '~/behaviors/shortcuts/shortcuts_navigation'; -import mountCommits from 'ee/projects/commits'; +import mountCommits from '~/projects/commits'; document.addEventListener('DOMContentLoaded', () => { new CommitsList(document.querySelector('.js-project-commits-show').dataset.commitsLimit); // eslint-disable-line no-new diff --git a/ee/app/assets/javascripts/projects/commits/components/author_select.vue b/app/assets/javascripts/projects/commits/components/author_select.vue similarity index 100% rename from ee/app/assets/javascripts/projects/commits/components/author_select.vue rename to app/assets/javascripts/projects/commits/components/author_select.vue diff --git a/ee/app/assets/javascripts/projects/commits/index.js b/app/assets/javascripts/projects/commits/index.js similarity index 100% rename from ee/app/assets/javascripts/projects/commits/index.js rename to app/assets/javascripts/projects/commits/index.js diff --git a/ee/app/assets/javascripts/projects/commits/store/actions.js b/app/assets/javascripts/projects/commits/store/actions.js similarity index 100% rename from ee/app/assets/javascripts/projects/commits/store/actions.js rename to app/assets/javascripts/projects/commits/store/actions.js diff --git a/ee/app/assets/javascripts/projects/commits/store/index.js b/app/assets/javascripts/projects/commits/store/index.js similarity index 100% rename from ee/app/assets/javascripts/projects/commits/store/index.js rename to app/assets/javascripts/projects/commits/store/index.js diff --git a/ee/app/assets/javascripts/projects/commits/store/mutation_types.js b/app/assets/javascripts/projects/commits/store/mutation_types.js similarity index 100% rename from ee/app/assets/javascripts/projects/commits/store/mutation_types.js rename to app/assets/javascripts/projects/commits/store/mutation_types.js diff --git a/ee/app/assets/javascripts/projects/commits/store/mutations.js b/app/assets/javascripts/projects/commits/store/mutations.js similarity index 100% rename from ee/app/assets/javascripts/projects/commits/store/mutations.js rename to app/assets/javascripts/projects/commits/store/mutations.js diff --git a/ee/app/assets/javascripts/projects/commits/store/state.js b/app/assets/javascripts/projects/commits/store/state.js similarity index 100% rename from ee/app/assets/javascripts/projects/commits/store/state.js rename to app/assets/javascripts/projects/commits/store/state.js diff --git a/package.json b/package.json index a08f073a743315..a48b2468ea8a5b 100644 --- a/package.json +++ b/package.json @@ -214,4 +214,4 @@ "node": ">=10.13.0", "yarn": "^1.10.0" } -} \ No newline at end of file +} diff --git a/ee/spec/frontend/projects/commits/components/author_select_spec.js b/spec/frontend/projects/commits/components/author_select_spec.js similarity index 97% rename from ee/spec/frontend/projects/commits/components/author_select_spec.js rename to spec/frontend/projects/commits/components/author_select_spec.js index 282080428d499c..fca825f58c7a7a 100644 --- a/ee/spec/frontend/projects/commits/components/author_select_spec.js +++ b/spec/frontend/projects/commits/components/author_select_spec.js @@ -1,8 +1,8 @@ import { shallowMount, createLocalVue } from '@vue/test-utils'; import Vuex from 'vuex'; import * as urlUtility from '~/lib/utils/url_utility'; -import AuthorSelect from 'ee/projects/commits/components/author_select.vue'; -import { createStore } from 'ee/projects/commits/store'; +import AuthorSelect from '~/projects/commits/components/author_select.vue'; +import { createStore } from '~/projects/commits/store'; import { GlNewDropdown, GlNewDropdownHeader, diff --git a/ee/spec/frontend/projects/commits/store/actions_spec.js b/spec/frontend/projects/commits/store/actions_spec.js similarity index 85% rename from ee/spec/frontend/projects/commits/store/actions_spec.js rename to spec/frontend/projects/commits/store/actions_spec.js index 3fff0037aef92d..33e92d51e54ce6 100644 --- a/ee/spec/frontend/projects/commits/store/actions_spec.js +++ b/spec/frontend/projects/commits/store/actions_spec.js @@ -1,9 +1,9 @@ import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; -import * as types from 'ee/projects/commits/store/mutation_types'; +import * as types from '~/projects/commits/store/mutation_types'; import testAction from 'helpers/vuex_action_helper'; -import actions from 'ee/projects/commits/store/actions'; -import createState from 'ee/projects/commits/store/state'; +import actions from '~/projects/commits/store/actions'; +import createState from '~/projects/commits/store/state'; describe('Project commits actions', () => { let state; diff --git a/ee/spec/frontend/projects/commits/store/mutations_spec.js b/spec/frontend/projects/commits/store/mutations_spec.js similarity index 73% rename from ee/spec/frontend/projects/commits/store/mutations_spec.js rename to spec/frontend/projects/commits/store/mutations_spec.js index e4fe76ba9b8d18..4fc346beb332b8 100644 --- a/ee/spec/frontend/projects/commits/store/mutations_spec.js +++ b/spec/frontend/projects/commits/store/mutations_spec.js @@ -1,6 +1,6 @@ -import * as types from 'ee/projects/commits/store/mutation_types'; -import mutations from 'ee/projects/commits/store/mutations'; -import createState from 'ee/projects/commits/store/state'; +import * as types from '~/projects/commits/store/mutation_types'; +import mutations from '~/projects/commits/store/mutations'; +import createState from '~/projects/commits/store/state'; describe('Project commits mutations', () => { let state; @@ -13,7 +13,7 @@ describe('Project commits mutations', () => { state = null; }); - describe(types.SET_INITIAL_DATA, () => { + describe(`${types.SET_INITIAL_DATA}`, () => { it('sets initial data', () => { state.commitsPath = null; state.projectId = null; @@ -30,7 +30,7 @@ describe('Project commits mutations', () => { }); }); - describe(types.COMMITS_AUTHORS, () => { + describe(`${types.COMMITS_AUTHORS}`, () => { it('sets commitsAuthors', () => { const authors = [{ id: 1 }, { id: 2 }]; state.commitsAuthors = []; -- GitLab From 8412904783e564f2558da468d3d291084661f17f Mon Sep 17 00:00:00 2001 From: Samantha Ming Date: Wed, 8 Apr 2020 23:33:56 -0700 Subject: [PATCH 3/6] Address review comments - Clean up & minor tweaks - Add catch to fetching project authors & update test --- .../commits/components/author_select.vue | 3 +- .../projects/commits/store/actions.js | 10 +++++-- app/views/projects/commits/show.html.haml | 2 +- .../commits/components/author_select_spec.js | 11 ++++++-- .../projects/commits/store/actions_spec.js | 28 +++++++++++++++++-- 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/projects/commits/components/author_select.vue b/app/assets/javascripts/projects/commits/components/author_select.vue index 71d9f143146c17..1650aa1a454aa8 100644 --- a/app/assets/javascripts/projects/commits/components/author_select.vue +++ b/app/assets/javascripts/projects/commits/components/author_select.vue @@ -10,6 +10,7 @@ import { GlTooltipDirective, } from '@gitlab/ui'; import { redirectTo, getParameterValues } from '~/lib/utils/url_utility'; +import { parseBoolean } from '~/lib/utils/common_utils'; import { __ } from '~/locale'; const tooltipMessage = __('Filtering by both author and message is not supported at the moment'); @@ -60,7 +61,7 @@ export default { commitsSearchInput.addEventListener( 'keyup', debounce(event => { - this.hasSearchParam = Boolean(event.target.value); + this.hasSearchParam = parseBoolean(event.target.value); }, 500), // keyup & time is to match effect of "filter by commit message" ); }, diff --git a/app/assets/javascripts/projects/commits/store/actions.js b/app/assets/javascripts/projects/commits/store/actions.js index 77c5ba3627cdc5..726c4374779fcf 100644 --- a/app/assets/javascripts/projects/commits/store/actions.js +++ b/app/assets/javascripts/projects/commits/store/actions.js @@ -1,13 +1,18 @@ import * as types from './mutation_types'; import axios from '~/lib/utils/axios_utils'; +import createFlash from '~/flash'; +import { __ } from '~/locale'; export default { setInitialData({ commit }, data) { commit(types.SET_INITIAL_DATA, data); }, - receiveAuthors({ commit }, authors) { + receiveAuthorsSuccess({ commit }, authors) { commit(types.COMMITS_AUTHORS, authors); }, + receiveAuthorsError() { + createFlash(__('An error occurred fetching the project authors.')); + }, fetchAuthors({ dispatch, state }, author = null) { const { projectId } = state; const path = '/autocomplete/users.json'; @@ -20,6 +25,7 @@ export default { search: author, }, }) - .then(response => dispatch('receiveAuthors', response.data)); + .then(response => dispatch('receiveAuthorsSuccess', response.data)) + .catch(() => dispatch('receiveAuthorsError')); }, }; diff --git a/app/views/projects/commits/show.html.haml b/app/views/projects/commits/show.html.haml index 3bc59bc634b5fb..7722a3523a1377 100644 --- a/app/views/projects/commits/show.html.haml +++ b/app/views/projects/commits/show.html.haml @@ -13,7 +13,7 @@ %ul.breadcrumb.repo-breadcrumb = commits_breadcrumbs - .tree-insert-vue-here#js-author-dropdown{ data: { 'commits_path': project_commits_path(@project), 'project_id': @project.id } } + #js-author-dropdown{ data: { 'commits_path': project_commits_path(@project), 'project_id': @project.id } } .tree-controls.d-none.d-sm-none.d-md-block - if @merge_request.present? .control diff --git a/spec/frontend/projects/commits/components/author_select_spec.js b/spec/frontend/projects/commits/components/author_select_spec.js index fca825f58c7a7a..519bf347ddce79 100644 --- a/spec/frontend/projects/commits/components/author_select_spec.js +++ b/spec/frontend/projects/commits/components/author_select_spec.js @@ -38,7 +38,6 @@ describe('Author Select', () => { wrapper = shallowMount(AuthorSelect, { localVue, store: new Vuex.Store(store), - attachToDocument: true, computed: { currentAuthor: () => currentAuthor, }, @@ -136,9 +135,17 @@ describe('Author Select', () => { createComponent(); }); + it('has a "Any Author" as the first list item', () => { + expect( + findDropdownItems() + .at(0) + .text(), + ).toBe('Any Author'); + }); + it('displays the project authors', () => { return wrapper.vm.$nextTick().then(() => { - expect(findDropdownItems().length).toBe(authors.length + 1); + expect(findDropdownItems()).toHaveLength(authors.length + 1); }); }); diff --git a/spec/frontend/projects/commits/store/actions_spec.js b/spec/frontend/projects/commits/store/actions_spec.js index 33e92d51e54ce6..c9945e1cc2755e 100644 --- a/spec/frontend/projects/commits/store/actions_spec.js +++ b/spec/frontend/projects/commits/store/actions_spec.js @@ -4,6 +4,9 @@ import * as types from '~/projects/commits/store/mutation_types'; import testAction from 'helpers/vuex_action_helper'; import actions from '~/projects/commits/store/actions'; import createState from '~/projects/commits/store/state'; +import createFlash from '~/flash'; + +jest.mock('~/flash'); describe('Project commits actions', () => { let state; @@ -23,9 +26,21 @@ describe('Project commits actions', () => { testAction(actions.setInitialData, undefined, state, [{ type: types.SET_INITIAL_DATA }])); }); - describe('receiveAuthors', () => { + describe('receiveAuthorsSuccess', () => { it(`commits ${types.COMMITS_AUTHORS}`, () => - testAction(actions.receiveAuthors, undefined, state, [{ type: types.COMMITS_AUTHORS }])); + testAction(actions.receiveAuthorsSuccess, undefined, state, [ + { type: types.COMMITS_AUTHORS }, + ])); + }); + + describe('shows a flash message when there is an error', () => { + it('creates a flash', () => { + const mockDispatchContext = { dispatch: () => {}, commit: () => {}, state }; + actions.receiveAuthorsError(mockDispatchContext); + + expect(createFlash).toHaveBeenCalledTimes(1); + expect(createFlash).toHaveBeenCalledWith('An error occurred fetching the project authors.'); + }); }); describe('fetchAuthors', () => { @@ -40,8 +55,15 @@ describe('Project commits actions', () => { null, state, [], - [{ type: 'receiveAuthors', payload: data }], + [{ type: 'receiveAuthorsSuccess', payload: data }], ); }); + + it('dispatches request/receive on error', () => { + const path = '/autocomplete/users.json'; + mock.onGet(path).replyOnce(500); + + testAction(actions.fetchAuthors, null, state, [], [{ type: 'receiveAuthorsError' }]); + }); }); }); -- GitLab From 5e9f5dd24f8b50f97d98617de4e0a70935cf20b2 Mon Sep 17 00:00:00 2001 From: Samantha Ming Date: Thu, 9 Apr 2020 17:24:45 -0700 Subject: [PATCH 4/6] Addresss design review - Change text - Update spec accordinly - Make mobile friendly - Fix tooltip when hovering in dropdown list --- .../commits/components/author_select.vue | 30 +-- locale/gitlab.pot | 18 +- .../commits/components/author_select_spec.js | 195 ++++++++++-------- 3 files changed, 132 insertions(+), 111 deletions(-) diff --git a/app/assets/javascripts/projects/commits/components/author_select.vue b/app/assets/javascripts/projects/commits/components/author_select.vue index 1650aa1a454aa8..f251d4bb13eb55 100644 --- a/app/assets/javascripts/projects/commits/components/author_select.vue +++ b/app/assets/javascripts/projects/commits/components/author_select.vue @@ -10,13 +10,11 @@ import { GlTooltipDirective, } from '@gitlab/ui'; import { redirectTo, getParameterValues } from '~/lib/utils/url_utility'; -import { parseBoolean } from '~/lib/utils/common_utils'; import { __ } from '~/locale'; -const tooltipMessage = __('Filtering by both author and message is not supported at the moment'); +const tooltipMessage = __('Searching by both author and message is currently not supported.'); export default { - tooltipMessage, name: 'AuthorSelect', components: { GlNewDropdown, @@ -40,8 +38,13 @@ export default { currentAuthor() { return getParameterValues('author')[0]; }, + dropdownText() { + return this.currentAuthor || __('Author'); + }, + tooltipTitle() { + return this.hasSearchParam && tooltipMessage; + }, }, - mounted() { this.fetchAuthors(); const searchTerm = getParameterValues('search'); @@ -61,7 +64,7 @@ export default { commitsSearchInput.addEventListener( 'keyup', debounce(event => { - this.hasSearchParam = parseBoolean(event.target.value); + this.hasSearchParam = Boolean(event.target.value); }, 500), // keyup & time is to match effect of "filter by commit message" ); }, @@ -89,21 +92,20 @@ export default {