From c4094b7ec4699811f928699ad67c90e1686da6e2 Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Thu, 4 May 2017 15:55:36 -0500 Subject: [PATCH] Fix specs --- .../blob/file_template_mediator.js | 6 ++- .../blob/file_template_selector.js | 10 ++++- .../template_selectors/ci_yaml_selector.js | 2 +- .../template_selectors/dockerfile_selector.js | 2 +- .../template_selectors/gitignore_selector.js | 2 +- .../template_selectors/license_selector.js | 13 +++++- .../blob/template_selectors/type_selector.js | 2 +- .../protected_tag_access_dropdown.js | 4 +- .../protected_tags/protected_tag_dropdown.js | 4 +- app/assets/javascripts/users_select.js | 41 +++++++++++-------- .../stylesheets/framework/dropdowns.scss | 4 ++ app/helpers/form_helper.rb | 11 +++-- .../_create_protected_branch.html.haml | 4 +- .../_update_protected_branch.html.haml | 4 +- .../protected_tags/_dropdown.html.haml | 2 +- .../protected_tags/_update_protected_tag.haml | 2 +- .../shared/issuable/form/_metadata.html.haml | 5 ++- spec/features/issues/form_spec.rb | 13 +++--- spec/features/issues_spec.rb | 2 +- spec/policies/issue_policy_spec.rb | 8 ++-- .../authorized_destroy_service_spec.rb | 6 +-- spec/services/users/destroy_service_spec.rb | 4 +- 22 files changed, 93 insertions(+), 58 deletions(-) diff --git a/app/assets/javascripts/blob/file_template_mediator.js b/app/assets/javascripts/blob/file_template_mediator.js index 3062cd51ee3..a20c6ca7a21 100644 --- a/app/assets/javascripts/blob/file_template_mediator.js +++ b/app/assets/javascripts/blob/file_template_mediator.js @@ -99,7 +99,7 @@ export default class FileTemplateMediator { }); } - selectTemplateType(item, el, e) { + selectTemplateType(item, e) { if (e) { e.preventDefault(); } @@ -117,6 +117,10 @@ export default class FileTemplateMediator { this.cacheToggleText(); } + selectTemplateTypeOptions(options) { + this.selectTemplateType(options.selectedObj, options.e); + } + selectTemplateFile(selector, query, data) { selector.renderLoading(); // in case undo menu is already already there diff --git a/app/assets/javascripts/blob/file_template_selector.js b/app/assets/javascripts/blob/file_template_selector.js index 31dd45fac89..ab5b3751c4e 100644 --- a/app/assets/javascripts/blob/file_template_selector.js +++ b/app/assets/javascripts/blob/file_template_selector.js @@ -52,9 +52,17 @@ export default class FileTemplateSelector { .removeClass('fa-spinner fa-spin'); } - reportSelection(query, el, e, data) { + reportSelection(options) { + const { query, e, data } = options; e.preventDefault(); return this.mediator.selectTemplateFile(this, query, data); } + + reportSelectionName(options) { + const opts = options; + opts.query = options.selectedObj.name; + + this.reportSelection(opts); + } } diff --git a/app/assets/javascripts/blob/template_selectors/ci_yaml_selector.js b/app/assets/javascripts/blob/template_selectors/ci_yaml_selector.js index 935df07677c..f2f81af137b 100644 --- a/app/assets/javascripts/blob/template_selectors/ci_yaml_selector.js +++ b/app/assets/javascripts/blob/template_selectors/ci_yaml_selector.js @@ -25,7 +25,7 @@ export default class BlobCiYamlSelector extends FileTemplateSelector { search: { fields: ['name'], }, - clicked: (query, el, e) => this.reportSelection(query.name, el, e), + clicked: options => this.reportSelectionName(options), text: item => item.name, }); } diff --git a/app/assets/javascripts/blob/template_selectors/dockerfile_selector.js b/app/assets/javascripts/blob/template_selectors/dockerfile_selector.js index b4b4d09c315..3cb7b960aaa 100644 --- a/app/assets/javascripts/blob/template_selectors/dockerfile_selector.js +++ b/app/assets/javascripts/blob/template_selectors/dockerfile_selector.js @@ -25,7 +25,7 @@ export default class DockerfileSelector extends FileTemplateSelector { search: { fields: ['name'], }, - clicked: (query, el, e) => this.reportSelection(query.name, el, e), + clicked: options => this.reportSelectionName(options), text: item => item.name, }); } diff --git a/app/assets/javascripts/blob/template_selectors/gitignore_selector.js b/app/assets/javascripts/blob/template_selectors/gitignore_selector.js index aefae54ae71..7efda8e7f50 100644 --- a/app/assets/javascripts/blob/template_selectors/gitignore_selector.js +++ b/app/assets/javascripts/blob/template_selectors/gitignore_selector.js @@ -24,7 +24,7 @@ export default class BlobGitignoreSelector extends FileTemplateSelector { search: { fields: ['name'], }, - clicked: (query, el, e) => this.reportSelection(query.name, el, e), + clicked: options => this.reportSelectionName(options), text: item => item.name, }); } diff --git a/app/assets/javascripts/blob/template_selectors/license_selector.js b/app/assets/javascripts/blob/template_selectors/license_selector.js index c8abd689ab4..1d757332f6c 100644 --- a/app/assets/javascripts/blob/template_selectors/license_selector.js +++ b/app/assets/javascripts/blob/template_selectors/license_selector.js @@ -24,13 +24,22 @@ export default class BlobLicenseSelector extends FileTemplateSelector { search: { fields: ['name'], }, - clicked: (query, el, e) => { + clicked: (options) => { + const { e } = options; + const el = options.$el; + const query = options.selectedObj; + const data = { project: this.$dropdown.data('project'), fullname: this.$dropdown.data('fullname'), }; - this.reportSelection(query.id, el, e, data); + this.reportSelection({ + query: query.id, + el, + e, + data, + }); }, text: item => item.name, }); diff --git a/app/assets/javascripts/blob/template_selectors/type_selector.js b/app/assets/javascripts/blob/template_selectors/type_selector.js index 56f23ef0568..a09381014a7 100644 --- a/app/assets/javascripts/blob/template_selectors/type_selector.js +++ b/app/assets/javascripts/blob/template_selectors/type_selector.js @@ -17,7 +17,7 @@ export default class FileTemplateTypeSelector extends FileTemplateSelector { filterable: false, selectable: true, toggleLabel: item => item.name, - clicked: (item, el, e) => this.mediator.selectTemplateType(item, el, e), + clicked: options => this.mediator.selectTemplateTypeOptions(options), text: item => item.name, }); } diff --git a/app/assets/javascripts/protected_tags/protected_tag_access_dropdown.js b/app/assets/javascripts/protected_tags/protected_tag_access_dropdown.js index fff83f3af3b..d4c9a91a74a 100644 --- a/app/assets/javascripts/protected_tags/protected_tag_access_dropdown.js +++ b/app/assets/javascripts/protected_tags/protected_tag_access_dropdown.js @@ -17,8 +17,8 @@ export default class ProtectedTagAccessDropdown { } return 'Select'; }, - clicked(item, $el, e) { - e.preventDefault(); + clicked(options) { + options.e.preventDefault(); onSelect(); }, }); diff --git a/app/assets/javascripts/protected_tags/protected_tag_dropdown.js b/app/assets/javascripts/protected_tags/protected_tag_dropdown.js index 5ff4e443262..068e9698e1d 100644 --- a/app/assets/javascripts/protected_tags/protected_tag_dropdown.js +++ b/app/assets/javascripts/protected_tags/protected_tag_dropdown.js @@ -39,8 +39,8 @@ export default class ProtectedTagDropdown { return _.escape(protectedTag.id); }, onFilter: this.toggleCreateNewButton.bind(this), - clicked: (item, $el, e) => { - e.preventDefault(); + clicked: (options) => { + options.e.preventDefault(); this.onSelect(); }, }); diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index 2dcaa24e93a..be29b08c343 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -77,7 +77,11 @@ import eventHub from './sidebar/event_hub'; input.value = _this.currentUser.id; } - $dropdown.before(input); + if ($selectbox) { + $dropdown.parent().before(input); + } else { + $dropdown.after(input); + } }; if ($block[0]) { @@ -95,6 +99,24 @@ import eventHub from './sidebar/event_hub'; .get(); }; + const checkMaxSelect = function() { + const maxSelect = $dropdown.data('max-select'); + if (maxSelect) { + const selected = getSelected(); + + if (selected.length > maxSelect) { + const firstSelectedId = selected[0]; + const firstSelected = $dropdown.closest('.selectbox') + .find(`input[name='${$dropdown.data('field-name')}'][value=${firstSelectedId}]`); + + firstSelected.remove(); + eventHub.$emit('sidebar.removeAssignee', { + id: firstSelectedId, + }); + } + } + }; + const getMultiSelectDropdownTitle = function(selectedUser, isSelected) { const selectedUsers = getSelected() .filter(u => u !== 0); @@ -125,6 +147,7 @@ import eventHub from './sidebar/event_hub'; if ($dropdown.data('multiSelect')) { assignYourself(); + checkMaxSelect(); const currentUserInfo = $dropdown.data('currentUserInfo'); $dropdown.find('.dropdown-toggle-text').text(getMultiSelectDropdownTitle(currentUserInfo)).removeClass('is-default'); @@ -333,21 +356,7 @@ import eventHub from './sidebar/event_hub'; // Enables support for limiting the number of users selected // Automatically removes the first on the list if more users are selected - const maxSelect = $dropdown.data('max-select'); - if (maxSelect) { - const selected = getSelected(); - - if (selected.length > maxSelect) { - const firstSelectedId = selected[0]; - const firstSelected = $dropdown.closest('.selectbox') - .find(`input[name='${$dropdown.data('field-name')}'][value=${firstSelectedId}]`); - - firstSelected.remove(); - eventHub.$emit('sidebar.removeAssignee', { - id: firstSelectedId, - }); - } - } + checkMaxSelect(); if (user.beforeDivider && user.name.toLowerCase() === 'unassigned') { // Unassigned selected diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 856989bccf1..5c9b71a452c 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -257,6 +257,10 @@ padding: 0 16px; } + &.capitalize-header .dropdown-header { + text-transform: capitalize; + } + .separator + .dropdown-header { padding-top: 2px; } diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb index 639a720b024..53962b84618 100644 --- a/app/helpers/form_helper.rb +++ b/app/helpers/form_helper.rb @@ -18,7 +18,7 @@ module FormHelper def issue_dropdown_options(issuable, has_multiple_assignees = true) options = { - toggle_class: 'js-user-search js-assignee-search', + toggle_class: 'js-user-search js-assignee-search js-multiselect js-save-user-data', title: 'Select assignee', filter: true, dropdown_class: 'dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee', @@ -32,16 +32,15 @@ module FormHelper default_label: 'Assignee', 'max-select': 1, 'dropdown-header': 'Assignee', + multi_select: true, + 'input-meta': 'name', + 'always-show-selectbox': true, + current_user_info: current_user.to_json(only: [:id, :name]) } } if has_multiple_assignees - options[:toggle_class] += ' js-multiselect js-save-user-data' options[:title] = 'Select assignee(s)' - options[:data][:multi_select] = true - options[:data][:'input-meta'] = 'name' - options[:data][:'always-show-selectbox'] = true - options[:data][:current_user_info] = current_user.to_json(only: [:id, :name]) options[:data][:'dropdown-header'] = 'Assignee(s)' options[:data].delete(:'max-select') end diff --git a/app/views/projects/protected_branches/_create_protected_branch.html.haml b/app/views/projects/protected_branches/_create_protected_branch.html.haml index b8e885b4d9a..99bc2516366 100644 --- a/app/views/projects/protected_branches/_create_protected_branch.html.haml +++ b/app/views/projects/protected_branches/_create_protected_branch.html.haml @@ -25,7 +25,7 @@ .merge_access_levels-container = dropdown_tag('Select', options: { toggle_class: 'js-allowed-to-merge wide', - dropdown_class: 'dropdown-menu-selectable', + dropdown_class: 'dropdown-menu-selectable capitalize-header', data: { field_name: 'protected_branch[merge_access_levels_attributes][0][access_level]', input_id: 'merge_access_levels_attributes' }}) .form-group %label.col-md-2.text-right{ for: 'push_access_levels_attributes' } @@ -34,7 +34,7 @@ .push_access_levels-container = dropdown_tag('Select', options: { toggle_class: 'js-allowed-to-push wide', - dropdown_class: 'dropdown-menu-selectable', + dropdown_class: 'dropdown-menu-selectable capitalize-header', data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes' }}) .panel-footer diff --git a/app/views/projects/protected_branches/_update_protected_branch.html.haml b/app/views/projects/protected_branches/_update_protected_branch.html.haml index d6044aacaec..c61b2951e1e 100644 --- a/app/views/projects/protected_branches/_update_protected_branch.html.haml +++ b/app/views/projects/protected_branches/_update_protected_branch.html.haml @@ -1,10 +1,10 @@ %td = hidden_field_tag "allowed_to_merge_#{protected_branch.id}", protected_branch.merge_access_levels.first.access_level = dropdown_tag( (protected_branch.merge_access_levels.first.humanize || 'Select') , - options: { toggle_class: 'js-allowed-to-merge', dropdown_class: 'dropdown-menu-selectable js-allowed-to-merge-container', + options: { toggle_class: 'js-allowed-to-merge', dropdown_class: 'dropdown-menu-selectable js-allowed-to-merge-container capitalize-header', data: { field_name: "allowed_to_merge_#{protected_branch.id}", access_level_id: protected_branch.merge_access_levels.first.id }}) %td = hidden_field_tag "allowed_to_push_#{protected_branch.id}", protected_branch.push_access_levels.first.access_level = dropdown_tag( (protected_branch.push_access_levels.first.humanize || 'Select') , - options: { toggle_class: 'js-allowed-to-push', dropdown_class: 'dropdown-menu-selectable js-allowed-to-push-container', + options: { toggle_class: 'js-allowed-to-push', dropdown_class: 'dropdown-menu-selectable js-allowed-to-push-container capitalize-header', data: { field_name: "allowed_to_push_#{protected_branch.id}", access_level_id: protected_branch.push_access_levels.first.id }}) diff --git a/app/views/projects/protected_tags/_dropdown.html.haml b/app/views/projects/protected_tags/_dropdown.html.haml index 74851519077..c50515cfe06 100644 --- a/app/views/projects/protected_tags/_dropdown.html.haml +++ b/app/views/projects/protected_tags/_dropdown.html.haml @@ -2,7 +2,7 @@ = dropdown_tag('Select tag or create wildcard', options: { toggle_class: 'js-protected-tag-select js-filter-submit wide', - filter: true, dropdown_class: "dropdown-menu-selectable", placeholder: "Search protected tag", + filter: true, dropdown_class: "dropdown-menu-selectable capitalize-header", placeholder: "Search protected tag", footer_content: true, data: { show_no: true, show_any: true, show_upcoming: true, selected: params[:protected_tag_name], diff --git a/app/views/projects/protected_tags/_update_protected_tag.haml b/app/views/projects/protected_tags/_update_protected_tag.haml index 62823bee46e..cc80bd04dd0 100644 --- a/app/views/projects/protected_tags/_update_protected_tag.haml +++ b/app/views/projects/protected_tags/_update_protected_tag.haml @@ -1,5 +1,5 @@ %td = hidden_field_tag "allowed_to_create_#{protected_tag.id}", protected_tag.create_access_levels.first.access_level = dropdown_tag( (protected_tag.create_access_levels.first.humanize || 'Select') , - options: { toggle_class: 'js-allowed-to-create', dropdown_class: 'dropdown-menu-selectable js-allowed-to-create-container', + options: { toggle_class: 'js-allowed-to-create', dropdown_class: 'dropdown-menu-selectable capitalize-header js-allowed-to-create-container', data: { field_name: "allowed_to_create_#{protected_tag.id}", access_level_id: protected_tag.create_access_levels.first.id }}) diff --git a/app/views/shared/issuable/form/_metadata.html.haml b/app/views/shared/issuable/form/_metadata.html.haml index 411cb717fc7..9281a515744 100644 --- a/app/views/shared/issuable/form/_metadata.html.haml +++ b/app/views/shared/issuable/form/_metadata.html.haml @@ -17,7 +17,10 @@ - issuable.assignees.each do |assignee| = hidden_field_tag "#{issuable.to_ability_name}[assignee_ids][]", assignee.id, id: nil, data: { meta: assignee.name } - = dropdown_tag(users_dropdown_label(issuable.assignees), options: issue_dropdown_options(issuable, true)) + - if issuable.assignees.length === 0 + = hidden_field_tag "#{issuable.to_ability_name}[assignee_ids][]", 0, id: nil, data: { meta: '' } + + = dropdown_tag(users_dropdown_label(issuable.assignees), options: issue_dropdown_options(issuable,false)) = link_to 'Assign to me', '#', class: "assign-to-me-link #{'hide' if issuable.assignees.include?(current_user)}" - else = form.label :assignee_id, "Assignee", class: "control-label #{"col-lg-4" if has_due_date}" diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index 5798292033b..87adce3cddd 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -77,11 +77,10 @@ describe 'New/edit issue', feature: true, js: true do click_link 'Assign to me' assignee_ids = page.all('input[name="issue[assignee_ids][]"]', visible: false) - expect(assignee_ids[0].value).to match(user2.id.to_s) - expect(assignee_ids[1].value).to match(user.id.to_s) + expect(assignee_ids[0].value).to match(user.id.to_s) page.within '.js-assignee-search' do - expect(page).to have_content "#{user2.name} + 1 more" + expect(page).to have_content user.name end expect(find('a', text: 'Assign to me', visible: false)).not_to be_visible @@ -109,7 +108,7 @@ describe 'New/edit issue', feature: true, js: true do page.within '.issuable-sidebar' do page.within '.assignee' do - expect(page).to have_content "2 Assignees" + expect(page).to have_content "Assignee" end page.within '.milestone' do @@ -148,12 +147,12 @@ describe 'New/edit issue', feature: true, js: true do end it 'correctly updates the selected user when changing assignee' do - click_button 'Assignee' + click_button 'Unassigned' page.within '.dropdown-menu-user' do click_link user.name end - expect(find('input[name="issue[assignee_id]"]', visible: false).value).to match(user.id.to_s) + expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match(user.id.to_s) click_button user.name @@ -167,7 +166,7 @@ describe 'New/edit issue', feature: true, js: true do click_link user2.name end - expect(find('input[name="issue[assignee_id]"]', visible: false).value).to match(user2.id.to_s) + expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match(user2.id.to_s) click_button user2.name diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index cc81303f032..5285dda361b 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -710,7 +710,7 @@ describe 'Issues', feature: true do include WaitForVueResource it 'updates the title', js: true do - issue = create(:issue, author: @user, assignee: @user, project: project, title: 'new title') + issue = create(:issue, author: @user, assignees: [@user], project: project, title: 'new title') visit namespace_project_issue_path(project.namespace, project, issue) diff --git a/spec/policies/issue_policy_spec.rb b/spec/policies/issue_policy_spec.rb index 9a870b7fda1..4a07c864428 100644 --- a/spec/policies/issue_policy_spec.rb +++ b/spec/policies/issue_policy_spec.rb @@ -15,7 +15,7 @@ describe IssuePolicy, models: true do context 'a private project' do let(:non_member) { create(:user) } let(:project) { create(:empty_project, :private) } - let(:issue) { create(:issue, project: project, assignee: assignee, author: author) } + let(:issue) { create(:issue, project: project, assignees: [assignee], author: author) } let(:issue_no_assignee) { create(:issue, project: project) } before do @@ -69,7 +69,7 @@ describe IssuePolicy, models: true do end context 'with confidential issues' do - let(:confidential_issue) { create(:issue, :confidential, project: project, assignee: assignee, author: author) } + let(:confidential_issue) { create(:issue, :confidential, project: project, assignees: [assignee], author: author) } let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } it 'does not allow non-members to read confidential issues' do @@ -110,7 +110,7 @@ describe IssuePolicy, models: true do context 'a public project' do let(:project) { create(:empty_project, :public) } - let(:issue) { create(:issue, project: project, assignee: assignee, author: author) } + let(:issue) { create(:issue, project: project, assignees: [assignee], author: author) } let(:issue_no_assignee) { create(:issue, project: project) } before do @@ -157,7 +157,7 @@ describe IssuePolicy, models: true do end context 'with confidential issues' do - let(:confidential_issue) { create(:issue, :confidential, project: project, assignee: assignee, author: author) } + let(:confidential_issue) { create(:issue, :confidential, project: project, assignees: [assignee], author: author) } let(:confidential_issue_no_assignee) { create(:issue, :confidential, project: project) } it 'does not allow guests to read confidential issues' do diff --git a/spec/services/members/authorized_destroy_service_spec.rb b/spec/services/members/authorized_destroy_service_spec.rb index 3b35a3b8e3a..ab440d18e9f 100644 --- a/spec/services/members/authorized_destroy_service_spec.rb +++ b/spec/services/members/authorized_destroy_service_spec.rb @@ -14,8 +14,8 @@ describe Members::AuthorizedDestroyService, services: true do it "unassigns issues and merge requests" do group.add_developer(member_user) - issue = create :issue, project: group_project, assignee: member_user - create :issue, assignee: member_user + issue = create :issue, project: group_project, assignees: [member_user] + create :issue, assignees: [member_user] merge_request = create :merge_request, target_project: group_project, source_project: group_project, assignee: member_user create :merge_request, target_project: project, source_project: project, assignee: member_user @@ -33,7 +33,7 @@ describe Members::AuthorizedDestroyService, services: true do it "unassigns issues and merge requests" do project.team << [member_user, :developer] - create :issue, project: project, assignee: member_user + create :issue, project: project, assignees: [member_user] create :merge_request, target_project: project, source_project: project, assignee: member_user member = project.members.find_by(user_id: member_user.id) diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index 4bc30018ebd..8aa900d1a75 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -47,7 +47,7 @@ describe Users::DestroyService, services: true do end context "for an issue the user was assigned to" do - let!(:issue) { create(:issue, project: project, assignee: user) } + let!(:issue) { create(:issue, project: project, assignees: [user]) } before do service.execute(user) @@ -60,7 +60,7 @@ describe Users::DestroyService, services: true do it 'migrates the issue so that it is "Unassigned"' do migrated_issue = Issue.find_by_id(issue.id) - expect(migrated_issue.assignee).to be_nil + expect(migrated_issue.assignees).to be_nil end end end