Commit 99f65fd5 authored by Paul Slaughter's avatar Paul Slaughter

Fix cannot merge badge showing bug in dropdown

**Why was this happening?**
The `can_merge` flag is appropriately sent, but we
don't get information on selected users from the BE.
Instead, this information comes from the HAML, which
wasn't setting `can_merge`.
parent c865089c
......@@ -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;
......@@ -279,12 +280,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),
};
});
......
# 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
......@@ -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)
......
......@@ -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?
......
......@@ -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'),
......
---
title: Fix cannot merge icon showing in dropdown for users who can merge
merge_request: 17306
author:
type: fixed
# 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
......@@ -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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment