diff --git a/app/assets/javascripts/issuable_context.js b/app/assets/javascripts/issuable_context.js index 566efa0d7d63ac66e94a9dc7cc2fcc6d36213959..6f2bd2da07813daf4d336587df6160ed9ebc3137 100644 --- a/app/assets/javascripts/issuable_context.js +++ b/app/assets/javascripts/issuable_context.js @@ -6,6 +6,7 @@ import UsersSelect from './users_select'; export default class IssuableContext { constructor(currentUser) { this.userSelect = new UsersSelect(currentUser); + this.reviewersSelect = new UsersSelect(currentUser, '.js-reviewer-search'); import(/* webpackChunkName: 'select2' */ 'select2/select2') .then(() => { diff --git a/app/assets/javascripts/sidebar/sidebar_mediator.js b/app/assets/javascripts/sidebar/sidebar_mediator.js index 8f1f76a2e02af00412fa329b63843e597f612183..2146fb83b1364c027a909c13427431fca2630452 100644 --- a/app/assets/javascripts/sidebar/sidebar_mediator.js +++ b/app/assets/javascripts/sidebar/sidebar_mediator.js @@ -40,6 +40,17 @@ export default class SidebarMediator { return this.service.update(field, data); } + saveReviewers(field) { + const selected = this.store.reviewers.map(u => u.id); + + // If there are no ids, that means we have to unassign (which is id = 0) + // And it only accepts an array, hence [0] + const reviewers = selected.length === 0 ? [0] : selected; + const data = { reviewer_ids: reviewers }; + + return this.service.update(field, data); + } + setMoveToProjectId(projectId) { this.store.setMoveToProjectId(projectId); } @@ -55,6 +66,7 @@ export default class SidebarMediator { processFetchedData(data) { this.store.setAssigneeData(data); + this.store.setReviewerData(data); this.store.setTimeTrackingData(data); this.store.setParticipantsData(data); this.store.setSubscriptionsData(data); diff --git a/app/assets/javascripts/sidebar/stores/sidebar_store.js b/app/assets/javascripts/sidebar/stores/sidebar_store.js index 095f93b72a9f0dccacd16a4573c9caaa9030da98..8d0d093e920fceb586a7bf92c64ea6b877356211 100644 --- a/app/assets/javascripts/sidebar/stores/sidebar_store.js +++ b/app/assets/javascripts/sidebar/stores/sidebar_store.js @@ -18,8 +18,10 @@ export default class SidebarStore { this.humanTimeSpent = ''; this.timeTrackingLimitToHours = timeTrackingLimitToHours; this.assignees = []; + this.reviewers = []; this.isFetching = { assignees: true, + reviewers: true, participants: true, subscriptions: true, }; @@ -42,6 +44,13 @@ export default class SidebarStore { } } + setReviewerData(data) { + this.isFetching.reviewers = false; + if (data.reviewers) { + this.reviewers = data.reviewers; + } + } + setTimeTrackingData(data) { this.timeEstimate = data.time_estimate; this.totalTimeSpent = data.total_time_spent; @@ -75,20 +84,40 @@ export default class SidebarStore { } } + addReviewer(reviewer) { + if (!this.findReviewer(reviewer)) { + this.reviewers.push(reviewer); + } + } + findAssignee(findAssignee) { return this.assignees.find(assignee => assignee.id === findAssignee.id); } + findReviewer(findReviewer) { + return this.reviewers.find(reviewer => reviewer.id === findReviewer.id); + } + removeAssignee(removeAssignee) { if (removeAssignee) { this.assignees = this.assignees.filter(assignee => assignee.id !== removeAssignee.id); } } + removeReviewer(removeReviewer) { + if (removeReviewer) { + this.reviewers = this.reviewers.filter(reviewer => reviewer.id !== removeReviewer.id); + } + } + removeAllAssignees() { this.assignees = []; } + removeAllReviewers() { + this.reviewers = []; + } + setAssigneesFromRealtime(data) { this.assignees = data; } diff --git a/app/assets/javascripts/users_select/index.js b/app/assets/javascripts/users_select/index.js index 5f4260f26ff4cd6d73fd090ec93725335479be12..20d1a3c1fcd535d65f7a06c4eda63882340392ce 100644 --- a/app/assets/javascripts/users_select/index.js +++ b/app/assets/javascripts/users_select/index.js @@ -19,6 +19,7 @@ import initDeprecatedJQueryDropdown from '~/deprecated_jquery_dropdown'; window.emitSidebarEvent = window.emitSidebarEvent || $.noop; function UsersSelect(currentUser, els, options = {}) { + const elsClassName = els?.toString().match('.(.+$)')[1]; const $els = $(els || '.js-user-search'); this.users = this.users.bind(this); this.user = this.user.bind(this); @@ -127,9 +128,16 @@ function UsersSelect(currentUser, els, options = {}) { .find(`input[name='${$dropdown.data('fieldName')}'][value=${firstSelectedId}]`); firstSelected.remove(); - emitSidebarEvent('sidebar.removeAssignee', { - id: firstSelectedId, - }); + + if ($dropdown.hasClass(elsClassName)) { + emitSidebarEvent('sidebar.removeReviewer', { + id: firstSelectedId, + }); + } else { + emitSidebarEvent('sidebar.removeAssignee', { + id: firstSelectedId, + }); + } } } }; @@ -392,7 +400,11 @@ function UsersSelect(currentUser, els, options = {}) { defaultLabel, hidden() { if ($dropdown.hasClass('js-multiselect')) { - emitSidebarEvent('sidebar.saveAssignees'); + if ($dropdown.hasClass(elsClassName)) { + emitSidebarEvent('sidebar.saveReviewers'); + } else { + emitSidebarEvent('sidebar.saveAssignees'); + } } if (!$dropdown.data('alwaysShowSelectbox')) { @@ -428,10 +440,18 @@ function UsersSelect(currentUser, els, options = {}) { previouslySelected.each((index, element) => { element.remove(); }); - emitSidebarEvent('sidebar.removeAllAssignees'); + if ($dropdown.hasClass(elsClassName)) { + emitSidebarEvent('sidebar.removeAllReviewers'); + } else { + emitSidebarEvent('sidebar.removeAllAssignees'); + } } else if (isActive) { // user selected - emitSidebarEvent('sidebar.addAssignee', user); + if ($dropdown.hasClass(elsClassName)) { + emitSidebarEvent('sidebar.addReviewer', user); + } else { + emitSidebarEvent('sidebar.addAssignee', user); + } // Remove unassigned selection (if it was previously selected) const unassignedSelected = $dropdown @@ -448,7 +468,11 @@ function UsersSelect(currentUser, els, options = {}) { } // User unselected - emitSidebarEvent('sidebar.removeAssignee', user); + if ($dropdown.hasClass(elsClassName)) { + emitSidebarEvent('sidebar.removeReviewer', user); + } else { + emitSidebarEvent('sidebar.removeAssignee', user); + } } if (getSelected().find(u => u === gon.current_user_id)) { diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index b255597b18d6e2d2c29f029f924a629abadb810e..c85f2ceb82e1bcbc2f73ddbb14aad15376ca1821 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -386,6 +386,12 @@ def assignee_sidebar_data(assignee, merge_request: nil) end end + def reviewer_sidebar_data(reviewer, merge_request: nil) + { avatar_url: reviewer.avatar_url, name: reviewer.name, username: reviewer.username }.tap do |data| + data[:can_merge] = merge_request.can_be_merged_by?(reviewer) if merge_request + end + end + def issuable_squash_option?(issuable, project) if issuable.persisted? issuable.squash diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index b579f7510f9b33f2c42273498c0d49b46313d358..9a3078991c0b3aa7a47c609d620cb1234863e13b 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -92,7 +92,7 @@ .loading.hide .spinner.spinner-md -= render 'shared/issuable/sidebar', issuable_sidebar: @issuable_sidebar, assignees: @merge_request.assignees, source_branch: @merge_request.source_branch += render 'shared/issuable/sidebar', issuable_sidebar: @issuable_sidebar, assignees: @merge_request.assignees, reviewers: @merge_request.reviewers, source_branch: @merge_request.source_branch - if @merge_request.can_be_reverted?(current_user) = render "projects/commit/change", type: 'revert', commit: @merge_request.merge_commit, title: @merge_request.title diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 09ec49a040a1d178e804422bdd51a83fd7bd8648..86484239546fed9750d91693bc0360a8e4ba633d 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 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/app/views/shared/issuable/_sidebar_reviewers.html.haml b/app/views/shared/issuable/_sidebar_reviewers.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..8b546d5e3445ec5715acd84dbe712b58c3e71597 --- /dev/null +++ b/app/views/shared/issuable/_sidebar_reviewers.html.haml @@ -0,0 +1,56 @@ +- issuable_type = issuable_sidebar[:type] +- signed_in = !!issuable_sidebar.dig(:current_user, :id) + +#js-vue-sidebar-reviewers{ data: { field: issuable_type, signed_in: signed_in } } + .title.hide-collapsed + = _('Reviewer') + = loading_icon(css_class: 'gl-vertical-align-text-bottom') + +.selectbox.hide-collapsed + - if reviewers.none? + = hidden_field_tag "#{issuable_type}[reviewer_ids][]", 0, id: nil + - else + - reviewers.each do |reviewer| + = hidden_field_tag "#{issuable_type}[reviewer_ids][]", reviewer.id, id: nil, data: reviewer_sidebar_data(reviewer, merge_request: @merge_request) + + - options = { toggle_class: 'js-reviewer-search js-author-search', + title: _('Request review from'), + filter: true, + dropdown_class: 'dropdown-menu-user dropdown-menu-selectable dropdown-menu-author', + placeholder: _('Search users'), + data: { first_user: issuable_sidebar.dig(:current_user, :username), + current_user: true, + iid: issuable_sidebar[:iid], + issuable_type: issuable_type, + project_id: issuable_sidebar[:project_id], + author_id: issuable_sidebar[:author_id], + field_name: "#{issuable_type}[reviewer_ids][]", + issue_update: issuable_sidebar[:issuable_json_path], + ability_name: issuable_type, + null_user: true, + display: 'static' } } + + - dropdown_options = reviewers_dropdown_options(issuable_type) + - title = dropdown_options[:title] + - options[:toggle_class] += ' js-multiselect js-save-user-data' + - data = { field_name: "#{issuable_type}[reviewer_ids][]" } + - data[:multi_select] = true + - data['dropdown-title'] = title + - data['dropdown-header'] = dropdown_options[:data][:'dropdown-header'] + - data['max-select'] = dropdown_options[:data][:'max-select'] if dropdown_options[:data][:'max-select'] + - options[:data].merge!(data) + + - if experiment_enabled?(:invite_members_version_a) && can_import_members? + - options[:dropdown_class] += ' dropdown-extended-height' + - options[:footer_content] = true + - options[:wrapper_class] = 'js-sidebar-reviewer-dropdown' + + = dropdown_tag(title, options: options) do + %ul.dropdown-footer-list + %li + = link_to _('Invite Members'), + project_project_members_path(@project), + title: _('Invite Members'), + data: { 'is-link': true, 'track-event': 'click_invite_members', 'track-label': 'edit_reviewer' } + - else + = dropdown_tag(title, options: options) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0cbdd2ebf67b1186fdc3ed47f020b431a89f7be0..fc2067709d77bdeb0ff0fe2deb3ec84c436a0896 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -21456,6 +21456,9 @@ msgstr "" msgid "Request parameter %{param} is missing." msgstr "" +msgid "Request review from" +msgstr "" + msgid "Request to link SAML account must be authorized" msgstr "" @@ -21726,6 +21729,9 @@ msgstr "" msgid "ReviewApp|Enable Review App" msgstr "" +msgid "Reviewer" +msgstr "" + msgid "Reviewing" msgstr "" diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 89a2a92ea57d74f8b61881aaeff4d25cabe4dfb0..821440db5518780955669e78e86b912af256ea0c 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -306,6 +306,38 @@ end end + describe '#reviewer_sidebar_data' do + let(:user) { create(:user) } + + subject { helper.reviewer_sidebar_data(user, merge_request: merge_request) } + + context 'without merge_request' do + let(:merge_request) { nil } + + it 'returns hash of reviewer data' do + is_expected.to eql({ + avatar_url: user.avatar_url, + name: user.name, + username: user.username + }) + end + end + + context 'with merge_request' do + let(:merge_request) { build(:merge_request) } + + where(can_merge: [true, false]) + + with_them do + before do + allow(merge_request).to receive(:can_be_merged_by?).and_return(can_merge) + end + + it { is_expected.to include({ can_merge: can_merge })} + end + end + end + describe '#issuable_squash_option?' do using RSpec::Parameterized::TableSyntax