Merge branch '31386-fix-cannot-merge-icon-is-wrong' into 'master'

Fix cannot merge icon showing in dropdown when user can merge

See merge request gitlab-org/gitlab!17306

app/assets/javascripts/users_select.js

@@ -7,6 +7,7 @@ import _ from 'underscore';
 import axios from './lib/utils/axios_utils';
 import { s__, __, sprintf } from './locale';
 import ModalStore from './boards/stores/modal_store';
+import { parseBoolean } from './lib/utils/common_utils';
 
 // TODO: remove eventHub hack after code splitting refactor
 window.emitSidebarEvent = window.emitSidebarEvent || $.noop;
@@ -275,12 +276,13 @@ function UsersSelect(currentUser, els, options = {}) {
                 })
                 .map(input => {
                   const userId = parseInt(input.value, 10);
-                  const { avatarUrl, avatar_url, name, username } = input.dataset;
+                  const { avatarUrl, avatar_url, name, username, canMerge } = input.dataset;
                   return {
                     avatar_url: avatarUrl || avatar_url,
                     id: userId,
                     name,
                     username,
+                    can_merge: parseBoolean(canMerge),
                   };
                 });
 

app/controllers/concerns/renders_assignees.rb

+# frozen_string_literal: true
+
+module RendersAssignees
+  def preload_assignees_for_render(merge_request)
+    merge_request.project.team.max_member_access_for_user_ids(merge_request.assignees.map(&:id))
+  end
+end

app/controllers/projects/merge_requests_controller.rb

@@ -5,6 +5,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   include IssuableActions
   include RendersNotes
   include RendersCommits
+  include RendersAssignees
   include ToggleAwardEmoji
   include IssuableCollections
   include RecordUserLastActivity
@@ -41,6 +42,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
         # use next to appease Rubocop
         next render('invalid') if target_branch_missing?
 
+        preload_assignees_for_render(@merge_request)
+
         # Build a note object for comment form
         @note = @project.notes.new(noteable: @merge_request)
 

app/helpers/issuables_helper.rb

@@ -372,6 +372,12 @@ module IssuablesHelper
     finder.class.scalar_params.any? { |p| params[p].present? }
   end
 
+  def assignee_sidebar_data(assignee, merge_request: nil)
+    { avatar_url: assignee.avatar_url, name: assignee.name, username: assignee.username }.tap do |data|
+      data[:can_merge] = merge_request.can_be_merged_by?(assignee) if merge_request
+    end
+  end
+
   private
 
   def sidebar_gutter_collapsed?

app/views/shared/issuable/_sidebar_assignees.html.haml

@@ -11,7 +11,7 @@
     = hidden_field_tag "#{issuable_type}[assignee_ids][]", 0, id: nil
   - else
     - assignees.each do |assignee|
-      = hidden_field_tag "#{issuable_type}[assignee_ids][]", assignee.id, id: nil, data: { avatar_url: assignee.avatar_url, name: assignee.name, username: assignee.username }
+      = hidden_field_tag "#{issuable_type}[assignee_ids][]", assignee.id, id: nil, data: assignee_sidebar_data(assignee, merge_request: @merge_request)
 
   - options = { toggle_class: 'js-user-search js-author-search',
     title: _('Assign to'),

changelogs/unreleased/31386-fix-cannot-merge-icon-is-wrong.yml

+---
+title: Fix cannot merge icon showing in dropdown for users who can merge
+merge_request: 17306
+author:
+type: fixed

spec/features/merge_request/user_edits_assignees_sidebar_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Merge request > User edits assignees sidebar', :js do
+  let(:project) { create(:project, :public, :repository) }
+  let(:protected_branch) { create(:protected_branch, :maintainers_can_push, name: 'master', project: project) }
+  let(:merge_request) { create(:merge_request, :simple, source_project: project, target_branch: protected_branch.name) }
+
+  let(:users_find_limit) { 5 }
+
+  # Insert more than limit so that response doesn't include assigned user
+  let(:project_developers) { Array.new(users_find_limit + 1) { create(:user).tap { |u| project.add_developer(u) } } }
+  let(:project_maintainers) { Array.new(users_find_limit + 1) { create(:user).tap { |u| project.add_maintainer(u) } } }
+
+  # DOM finders to simplify and improve readability
+  let(:sidebar_assignee_block) { page.find('.js-issuable-sidebar .assignee') }
+  let(:sidebar_assignee_avatar_link) { sidebar_assignee_block.find_all('a').find { |a| a['href'].include? assignee.username } }
+  let(:sidebar_assignee_tooltip) { sidebar_assignee_avatar_link['data-original-title'] || '' }
+  let(:sidebar_assignee_dropdown_item) { sidebar_assignee_block.find(".dropdown-menu li[data-user-id=\"#{assignee.id}\"]") }
+  let(:sidebar_assignee_dropdown_tooltip) { sidebar_assignee_dropdown_item.find('a')['data-title'] || '' }
+
+  before do
+    stub_const('Autocomplete::UsersFinder::LIMIT', users_find_limit)
+
+    sign_in(project.owner)
+
+    merge_request.assignees << assignee
+
+    visit project_merge_request_path(project, merge_request)
+
+    wait_for_requests
+  end
+
+  shared_examples 'when assigned' do |expected_tooltip: ''|
+    it 'shows assignee name' do
+      expect(sidebar_assignee_block).to have_text(assignee.name)
+    end
+
+    it "shows assignee tooltip '#{expected_tooltip}'" do
+      expect(sidebar_assignee_tooltip).to eql(expected_tooltip)
+    end
+
+    context 'when edit is clicked' do
+      before do
+        sidebar_assignee_block.click_link('Edit')
+
+        wait_for_requests
+      end
+
+      it "shows assignee tooltip '#{expected_tooltip}" do
+        expect(sidebar_assignee_dropdown_tooltip).to eql(expected_tooltip)
+      end
+    end
+  end
+
+  context 'when assigned to maintainer' do
+    let(:assignee) { project_maintainers.last }
+
+    it_behaves_like 'when assigned', expected_tooltip: ''
+  end
+
+  context 'when assigned to developer' do
+    let(:assignee) { project_developers.last }
+
+    it_behaves_like 'when assigned', expected_tooltip: 'Cannot merge'
+  end
+end

spec/helpers/issuables_helper_spec.rb

@@ -243,4 +243,32 @@ describe IssuablesHelper do
       end
     end
   end
+
+  describe '#assignee_sidebar_data' do
+    let(:user) { create(:user) }
+    let(:merge_request) { nil }
+    subject { helper.assignee_sidebar_data(user, merge_request: merge_request) }
+
+    it 'returns hash of assignee data' do
+      is_expected.to eql({
+        avatar_url: user.avatar_url,
+        name: user.name,
+        username: user.username
+      })
+    end
+
+    context 'with merge_request' do
+      let(:merge_request) { build_stubbed(: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
 end