diff --git a/app/assets/javascripts/sidebar/components/reviewers/collapsed_reviewer.vue b/app/assets/javascripts/sidebar/components/reviewers/collapsed_reviewer.vue new file mode 100644 index 0000000000000000000000000000000000000000..6de926e0ff98a1ca4bb2be9e36fd5d4e4ba533f1 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/reviewers/collapsed_reviewer.vue @@ -0,0 +1,24 @@ + + + + + + {{ user.name }} + + diff --git a/app/assets/javascripts/sidebar/components/reviewers/collapsed_reviewer_list.vue b/app/assets/javascripts/sidebar/components/reviewers/collapsed_reviewer_list.vue new file mode 100644 index 0000000000000000000000000000000000000000..45707c18f7bca021789235365650a48d7a319d8a --- /dev/null +++ b/app/assets/javascripts/sidebar/components/reviewers/collapsed_reviewer_list.vue @@ -0,0 +1,107 @@ + + + + + + + + {{ sidebarAvatarCounter }} + + + + diff --git a/app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar.vue b/app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar.vue new file mode 100644 index 0000000000000000000000000000000000000000..9fa3fa38eacf75f097a9001ad00e50c7745cb3cf --- /dev/null +++ b/app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar.vue @@ -0,0 +1,43 @@ + + + + + + + + diff --git a/app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar_link.vue b/app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar_link.vue new file mode 100644 index 0000000000000000000000000000000000000000..b1b04564a62f5915dde383b785931c44a731deee --- /dev/null +++ b/app/assets/javascripts/sidebar/components/reviewers/reviewer_avatar_link.vue @@ -0,0 +1,84 @@ + + + + + + + + + + + + diff --git a/app/assets/javascripts/sidebar/components/reviewers/reviewer_title.vue b/app/assets/javascripts/sidebar/components/reviewers/reviewer_title.vue new file mode 100644 index 0000000000000000000000000000000000000000..437f28907fd352f0a697110248e1bfe44e8c3c46 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/reviewers/reviewer_title.vue @@ -0,0 +1,64 @@ + + + + {{ reviewerTitle }} + + + {{ __('Edit') }} + + + + + + diff --git a/app/assets/javascripts/sidebar/components/reviewers/reviewers.vue b/app/assets/javascripts/sidebar/components/reviewers/reviewers.vue new file mode 100644 index 0000000000000000000000000000000000000000..6a3d88f6385a07c49fc6267389911231635c32a6 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/reviewers/reviewers.vue @@ -0,0 +1,72 @@ + + + + + + + + + + {{ __('None') }} + + + + + + + diff --git a/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue b/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue new file mode 100644 index 0000000000000000000000000000000000000000..5d8a2e6fa6599a5aebf56307601c7118c55c6b89 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue @@ -0,0 +1,107 @@ + + + + + + + + diff --git a/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue b/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue new file mode 100644 index 0000000000000000000000000000000000000000..2ae4a114b36101af9d1b05ef21a31bccd70de27d --- /dev/null +++ b/app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue @@ -0,0 +1,103 @@ + + + + + + {{ user.name }} + {{ username }} + + + + + + + + + + + + {{ hiddenReviewersLabel }} + + {{ __('- show less') }} + + + + diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js index 047d9a4d0e8a3a3d387cf81870184d0dd509cbed..ffc7f2c07ba1f16193a9c6328d6dc48da27c0259 100644 --- a/app/assets/javascripts/sidebar/mount_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -5,6 +5,7 @@ import Vuex from 'vuex'; import SidebarTimeTracking from './components/time_tracking/sidebar_time_tracking.vue'; import SidebarAssignees from './components/assignees/sidebar_assignees.vue'; import SidebarLabels from './components/labels/sidebar_labels.vue'; +import SidebarReviewers from './components/reviewers/sidebar_reviewers.vue'; import ConfidentialIssueSidebar from './components/confidential/confidential_issue_sidebar.vue'; import SidebarMoveIssue from './lib/sidebar_move_issue'; import IssuableLockForm from './components/lock/issuable_lock_form.vue'; @@ -56,6 +57,36 @@ function mountAssigneesComponent(mediator) { }); } +function mountReviewersComponent(mediator) { + const el = document.getElementById('js-vue-sidebar-reviewers'); + const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient(), + }); + + if (!el) return; + + const { iid, fullPath } = getSidebarOptions(); + // eslint-disable-next-line no-new + new Vue({ + el, + apolloProvider, + components: { + SidebarReviewers, + }, + render: createElement => + createElement('sidebar-reviewers', { + props: { + mediator, + issuableIid: String(iid), + projectPath: fullPath, + field: el.dataset.field, + signedIn: el.hasAttribute('data-signed-in'), + issuableType: isInIssuePage() ? 'issue' : 'merge_request', + }, + }), + }); +} + export function mountSidebarLabels() { const el = document.querySelector('.js-sidebar-labels'); @@ -245,6 +276,7 @@ function mountSeverityComponent() { export function mountSidebar(mediator) { mountAssigneesComponent(mediator); + mountReviewersComponent(mediator); mountConfidentialComponent(mediator); mountLockComponent(); mountParticipantsComponent(mediator); diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 305022058a770083f1ec8e1bd26f3ec7efbab2ef..7097c2b10c4c6577ba8dfc7c606b6f67881b9ac7 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -117,7 +117,8 @@ } } -.assignee { +.assignee, +.reviewer { .merge-icon { color: $orange-400; position: absolute; diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 68aa66074b7dc0f04994a1f5dba3d485919ac93a..014ada03686047e481bf2e8b89ecd61894d41361 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -5,6 +5,7 @@ - signed_in = !!issuable_sidebar.dig(:current_user, :id) - can_edit_issuable = issuable_sidebar.dig(:current_user, :can_edit) - add_page_startup_api_call "#{issuable_sidebar[:issuable_json_path]}?serializer=sidebar_extras" +- reviewers = local_assigns.fetch(:reviewers, nil) - if Feature.enabled?(:vue_issuable_sidebar, @project.group) %aside#js-vue-issuable-sidebar{ data: { signed_in: signed_in, @@ -28,6 +29,10 @@ .block.assignee.qa-assignee-block = render "shared/issuable/sidebar_assignees", issuable_sidebar: issuable_sidebar, assignees: assignees + - if Feature.enabled?(:merge_request_reviewers, @project) && reviewers + .block.reviewer.qa-reviewer-block + = render "shared/issuable/sidebar_reviewers", issuable_sidebar: issuable_sidebar, reviewers: reviewers + = render_if_exists 'shared/issuable/sidebar_item_epic', issuable_sidebar: issuable_sidebar - if issuable_sidebar[:supports_milestone] diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a1733dc74db0bfb2845a8c83ef02a08789546e73..fa829e7fa3551e8d23585eda33c080c68e371fe7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -977,6 +977,9 @@ msgstr "" msgid "+ %{numberOfHiddenAssignees} more" msgstr "" +msgid "+ %{numberOfHiddenReviewers} more" +msgstr "" + msgid "+%d more" msgid_plural "+%d more" msgstr[0] "" @@ -10220,6 +10223,9 @@ msgstr "" msgid "Error occurred when saving assignees" msgstr "" +msgid "Error occurred when saving reviewers" +msgstr "" + msgid "Error occurred when toggling the notification subscription" msgstr "" @@ -21987,6 +21993,11 @@ msgid "ReviewApp|Enable Review App" msgstr "" msgid "Reviewer" +msgid_plural "%d Reviewers" +msgstr[0] "" +msgstr[1] "" + +msgid "Reviewer(s)" msgstr "" msgid "Reviewing" diff --git a/spec/features/merge_request/user_edits_mr_spec.rb b/spec/features/merge_request/user_edits_mr_spec.rb index 397ca70f4a103172360cc05e2d6a79528de2ee68..817b4e0b48e67aac3903601037aa563624981477 100644 --- a/spec/features/merge_request/user_edits_mr_spec.rb +++ b/spec/features/merge_request/user_edits_mr_spec.rb @@ -21,24 +21,6 @@ it_behaves_like 'an editable merge request' end - context 'when merge_request_reviewers is turned on' do - before do - stub_feature_flags(merge_request_reviewers: true) - end - - context 'non-fork merge request' do - include_context 'merge request edit context' - it_behaves_like 'an editable merge request with reviewers' - end - - context 'for a forked project' do - let(:source_project) { fork_project(target_project, nil, repository: true) } - - include_context 'merge request edit context' - it_behaves_like 'an editable merge request with reviewers' - end - end - context 'when merge_request_reviewers is turned off' do before do stub_feature_flags(merge_request_reviewers: false) diff --git a/spec/features/merge_request/user_views_open_merge_request_spec.rb b/spec/features/merge_request/user_views_open_merge_request_spec.rb index 44e6baa29311334529944b13e0976187b4134980..e8998f9457a9db3560f3369d67aec6f5f657ac04 100644 --- a/spec/features/merge_request/user_views_open_merge_request_spec.rb +++ b/spec/features/merge_request/user_views_open_merge_request_spec.rb @@ -24,6 +24,23 @@ expect(page).to have_content(merge_request.title) end + + it 'has reviewers in sidebar' do + expect(page).to have_css('.reviewer') + end + end + + context 'when merge_request_reviewers is turned off' do + let(:project) { create(:project, :public, :repository) } + + before do + stub_feature_flags(merge_request_reviewers: false) + visit(merge_request_path(merge_request)) + end + + it 'has reviewers in sidebar' do + expect(page).not_to have_css('.reviewer') + end end context 'when a merge request has repository', :js do diff --git a/spec/frontend/sidebar/reviewer_title_spec.js b/spec/frontend/sidebar/reviewer_title_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..eae266688d59d0f8277199bcb01e8314cda45f52 --- /dev/null +++ b/spec/frontend/sidebar/reviewer_title_spec.js @@ -0,0 +1,116 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlLoadingIcon } from '@gitlab/ui'; +import { mockTracking, triggerEvent } from 'helpers/tracking_helper'; +import Component from '~/sidebar/components/reviewers/reviewer_title.vue'; + +describe('ReviewerTitle component', () => { + let wrapper; + + const createComponent = props => { + return shallowMount(Component, { + propsData: { + numberOfReviewers: 0, + editable: false, + ...props, + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + describe('reviewer title', () => { + it('renders reviewer', () => { + wrapper = createComponent({ + numberOfReviewers: 1, + editable: false, + }); + + expect(wrapper.vm.$el.innerText.trim()).toEqual('Reviewer'); + }); + + it('renders 2 reviewers', () => { + wrapper = createComponent({ + numberOfReviewers: 2, + editable: false, + }); + + expect(wrapper.vm.$el.innerText.trim()).toEqual('2 Reviewers'); + }); + }); + + describe('gutter toggle', () => { + it('does not show toggle by default', () => { + wrapper = createComponent({ + numberOfReviewers: 2, + editable: false, + }); + + expect(wrapper.vm.$el.querySelector('.gutter-toggle')).toBeNull(); + }); + + it('shows toggle when showToggle is true', () => { + wrapper = createComponent({ + numberOfReviewers: 2, + editable: false, + showToggle: true, + }); + + expect(wrapper.vm.$el.querySelector('.gutter-toggle')).toEqual(expect.any(Object)); + }); + }); + + it('does not render spinner by default', () => { + wrapper = createComponent({ + numberOfReviewers: 0, + editable: false, + }); + + expect(wrapper.find(GlLoadingIcon).exists()).toBeFalsy(); + }); + + it('renders spinner when loading', () => { + wrapper = createComponent({ + loading: true, + numberOfReviewers: 0, + editable: false, + }); + + expect(wrapper.find(GlLoadingIcon).exists()).toBeTruthy(); + }); + + it('does not render edit link when not editable', () => { + wrapper = createComponent({ + numberOfReviewers: 0, + editable: false, + }); + + expect(wrapper.vm.$el.querySelector('.edit-link')).toBeNull(); + }); + + it('renders edit link when editable', () => { + wrapper = createComponent({ + numberOfReviewers: 0, + editable: true, + }); + + expect(wrapper.vm.$el.querySelector('.edit-link')).not.toBeNull(); + }); + + it('tracks the event when edit is clicked', () => { + wrapper = createComponent({ + numberOfReviewers: 0, + editable: true, + }); + + const spy = mockTracking('_category_', wrapper.element, jest.spyOn); + triggerEvent('.js-sidebar-dropdown-toggle'); + + expect(spy).toHaveBeenCalledWith('_category_', 'click_edit_button', { + label: 'right_sidebar', + property: 'reviewer', + }); + }); +}); diff --git a/spec/frontend/sidebar/reviewers_spec.js b/spec/frontend/sidebar/reviewers_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..effcac266f0697166ad5967566bae21e965d9106 --- /dev/null +++ b/spec/frontend/sidebar/reviewers_spec.js @@ -0,0 +1,169 @@ +import { mount } from '@vue/test-utils'; +import { trimText } from 'helpers/text_helper'; +import { GlIcon } from '@gitlab/ui'; +import Reviewer from '~/sidebar/components/reviewers/reviewers.vue'; +import UsersMock from './mock_data'; +import UsersMockHelper from '../helpers/user_mock_data_helper'; + +describe('Reviewer component', () => { + const getDefaultProps = () => ({ + rootPath: 'http://localhost:3000', + users: [], + editable: false, + }); + let wrapper; + + const createWrapper = (propsData = getDefaultProps()) => { + wrapper = mount(Reviewer, { + propsData, + }); + }; + + const findCollapsedChildren = () => wrapper.findAll('.sidebar-collapsed-icon > *'); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('No reviewers/users', () => { + it('displays no reviewer icon when collapsed', () => { + createWrapper(); + const collapsedChildren = findCollapsedChildren(); + const userIcon = collapsedChildren.at(0).find(GlIcon); + + expect(collapsedChildren.length).toBe(1); + expect(collapsedChildren.at(0).attributes('aria-label')).toBe('None'); + expect(userIcon.exists()).toBe(true); + expect(userIcon.props('name')).toBe('user'); + }); + }); + + describe('One reviewer/user', () => { + it('displays one reviewer icon when collapsed', () => { + createWrapper({ + ...getDefaultProps(), + users: [UsersMock.user], + }); + + const collapsedChildren = findCollapsedChildren(); + const reviewer = collapsedChildren.at(0); + + expect(collapsedChildren.length).toBe(1); + expect(reviewer.find('.avatar').attributes('src')).toBe(UsersMock.user.avatar); + expect(reviewer.find('.avatar').attributes('alt')).toBe(`${UsersMock.user.name}'s avatar`); + + expect(trimText(reviewer.find('.author').text())).toBe(UsersMock.user.name); + }); + }); + + describe('Two or more reviewers/users', () => { + it('displays two reviewer icons when collapsed', () => { + const users = UsersMockHelper.createNumberRandomUsers(2); + createWrapper({ + ...getDefaultProps(), + users, + }); + + const collapsedChildren = findCollapsedChildren(); + + expect(collapsedChildren.length).toBe(2); + + const first = collapsedChildren.at(0); + + expect(first.find('.avatar').attributes('src')).toBe(users[0].avatar_url); + expect(first.find('.avatar').attributes('alt')).toBe(`${users[0].name}'s avatar`); + + expect(trimText(first.find('.author').text())).toBe(users[0].name); + + const second = collapsedChildren.at(1); + + expect(second.find('.avatar').attributes('src')).toBe(users[1].avatar_url); + expect(second.find('.avatar').attributes('alt')).toBe(`${users[1].name}'s avatar`); + + expect(trimText(second.find('.author').text())).toBe(users[1].name); + }); + + it('displays one reviewer icon and counter when collapsed', () => { + const users = UsersMockHelper.createNumberRandomUsers(3); + createWrapper({ + ...getDefaultProps(), + users, + }); + + const collapsedChildren = findCollapsedChildren(); + + expect(collapsedChildren.length).toBe(2); + + const first = collapsedChildren.at(0); + + expect(first.find('.avatar').attributes('src')).toBe(users[0].avatar_url); + expect(first.find('.avatar').attributes('alt')).toBe(`${users[0].name}'s avatar`); + + expect(trimText(first.find('.author').text())).toBe(users[0].name); + + const second = collapsedChildren.at(1); + + expect(trimText(second.find('.avatar-counter').text())).toBe('+2'); + }); + + it('Shows two reviewers', () => { + const users = UsersMockHelper.createNumberRandomUsers(2); + createWrapper({ + ...getDefaultProps(), + users, + editable: true, + }); + + expect(wrapper.findAll('.user-item').length).toBe(users.length); + expect(wrapper.find('.user-list-more').exists()).toBe(false); + }); + + it('shows sorted reviewer where "can merge" users are sorted first', () => { + const users = UsersMockHelper.createNumberRandomUsers(3); + users[0].can_merge = false; + users[1].can_merge = false; + users[2].can_merge = true; + + createWrapper({ + ...getDefaultProps(), + users, + editable: true, + }); + + expect(wrapper.vm.sortedReviewers[0].can_merge).toBe(true); + }); + + it('passes the sorted reviewers to the uncollapsed-reviewer-list', () => { + const users = UsersMockHelper.createNumberRandomUsers(3); + users[0].can_merge = false; + users[1].can_merge = false; + users[2].can_merge = true; + + createWrapper({ + ...getDefaultProps(), + users, + }); + + const userItems = wrapper.findAll('.user-list .user-item a'); + + expect(userItems.length).toBe(3); + expect(userItems.at(0).attributes('title')).toBe(users[2].name); + }); + + it('passes the sorted reviewers to the collapsed-reviewer-list', () => { + const users = UsersMockHelper.createNumberRandomUsers(3); + users[0].can_merge = false; + users[1].can_merge = false; + users[2].can_merge = true; + + createWrapper({ + ...getDefaultProps(), + users, + }); + + const collapsedButton = wrapper.find('.sidebar-collapsed-user button'); + + expect(trimText(collapsedButton.text())).toBe(users[2].name); + }); + }); +}); diff --git a/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb b/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb index c9910487798de65eaa68c5dbfa073ece816bbff6..218f1afce9885e3c942ee35943697bedb11529fc 100644 --- a/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb +++ b/spec/support/shared_examples/features/editable_merge_request_shared_examples.rb @@ -11,6 +11,15 @@ expect(page).to have_content user.name end + find('.js-reviewer-search').click + page.within '.dropdown-menu-user' do + click_link user.name + end + expect(find('input[name="merge_request[reviewer_ids][]"]', visible: false).value).to match(user.id.to_s) + page.within '.js-reviewer-search' do + expect(page).to have_content user.name + end + click_button 'Milestone' page.within '.issue-milestone' do click_link milestone.title @@ -38,6 +47,10 @@ expect(page).to have_content user.name end + page.within '.reviewer' do + expect(page).to have_content user.name + end + page.within '.milestone' do expect(page).to have_content milestone.title end @@ -124,16 +137,3 @@ def get_textarea_height page.evaluate_script('document.getElementById("merge_request_description").offsetHeight') end - -RSpec.shared_examples 'an editable merge request with reviewers' do - it 'updates merge request', :js do - find('.js-reviewer-search').click - page.within '.dropdown-menu-user' do - click_link user.name - end - expect(find('input[name="merge_request[reviewer_ids][]"]', visible: false).value).to match(user.id.to_s) - page.within '.js-reviewer-search' do - expect(page).to have_content user.name - end - end -end