diff --git a/CHANGELOG.md b/CHANGELOG.md index ca6b9bc99d5..02291b1679a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ Please view this file on the master branch, on stable branches it's out of date. - Fix reply-by-email not working due to queue name mismatch - Expire and build repository cache after project import - Fix 404 for group pages when GitLab setup uses relative url + - Simpler arguments passed to named_route on toggle_award_url helper method + - Better handle when no users were selected for adding to group or project. (Linus Thiel) ## 8.13.0 (2016-10-22) - Removes extra line for empty issue description. (!7045) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 8a61669822c..17cbfd0e66f 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -83,14 +83,15 @@ }; // Disable button if text field is empty - window.disableButtonIfEmptyField = function(field_selector, button_selector) { + window.disableButtonIfEmptyField = function(field_selector, button_selector, event_name) { + event_name = event_name || 'input'; var closest_submit, field; field = $(field_selector); closest_submit = field.closest('form').find(button_selector); if (rstrip(field.val()) === "") { closest_submit.disable(); } - return field.on('input', function() { + return field.on(event_name, function() { if (rstrip($(this).val()) === "") { return closest_submit.disable(); } else { diff --git a/app/assets/javascripts/members.js.es6 b/app/assets/javascripts/members.js.es6 index a0cd20f21e8..2bdd0f7a637 100644 --- a/app/assets/javascripts/members.js.es6 +++ b/app/assets/javascripts/members.js.es6 @@ -10,6 +10,7 @@ $('.project_member, .group_member').off('ajax:success').on('ajax:success', this.removeRow); $('.js-member-update-control').off('change').on('change', this.formSubmit); $('.js-edit-member-form').off('ajax:success').on('ajax:success', this.formSuccess); + disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change'); } removeRow(e) { diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index 18cd800c619..940a3ad20ba 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -21,6 +21,10 @@ class Groups::GroupMembersController < Groups::ApplicationController end def create + if params[:user_ids].blank? + return redirect_to(group_group_members_path(@group), alert: 'No users specified.') + end + @group.add_users( params[:user_ids].split(','), params[:access_level], diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index 2a07d154853..d08f490de18 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -25,6 +25,10 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def create + if params[:user_ids].blank? + return redirect_to(namespace_project_project_members_path(@project.namespace, @project), alert: 'No users or groups specified.') + end + @project.team.add_users( params[:user_ids].split(','), params[:access_level], @@ -32,7 +36,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController current_user: current_user ) - redirect_to namespace_project_project_members_path(@project.namespace, @project) + redirect_to namespace_project_project_members_path(@project.namespace, @project), notice: 'Users were successfully added.' end def update diff --git a/app/models/group.rb b/app/models/group.rb index 6865e610718..00a595d2705 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -68,7 +68,7 @@ class Group < Namespace end def web_url - Gitlab::Routing.url_helpers.group_canonical_url(self) + Gitlab::Routing.url_helpers.group_url(self) end def human_name diff --git a/config/routes/group.rb b/config/routes/group.rb index 826048ba196..4838c9d91c6 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -12,26 +12,23 @@ constraints(GroupUrlConstrainer.new) do end end -scope constraints: { id: /[a-zA-Z.0-9_\-]+(? 'groups#show', as: :group_canonical end diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index ad15b3f8f40..c7db84dd5f9 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -13,6 +13,49 @@ describe Groups::GroupMembersController do end end + describe 'POST create' do + let(:group_user) { create(:user) } + + before { sign_in(user) } + + context 'when user does not have enough rights' do + before { group.add_developer(user) } + + it 'returns 403' do + post :create, group_id: group, + user_ids: group_user.id, + access_level: Gitlab::Access::GUEST + + expect(response).to have_http_status(403) + expect(group.users).not_to include group_user + end + end + + context 'when user has enough rights' do + before { group.add_owner(user) } + + it 'adds user to members' do + post :create, group_id: group, + user_ids: group_user.id, + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.users).to include group_user + end + + it 'adds no user to members' do + post :create, group_id: group, + user_ids: '', + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'No users specified.' + expect(response).to redirect_to(group_group_members_path(group)) + expect(group.users).not_to include group_user + end + end + end + describe 'DELETE destroy' do let(:member) { create(:group_member, :developer, group: group) } diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 5e487241d07..b4f066d8600 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -13,6 +13,54 @@ describe Projects::ProjectMembersController do end end + describe 'POST create' do + context 'when users are added' do + let(:project_user) { create(:user) } + + before { sign_in(user) } + + context 'when user does not have enough rights' do + before { project.team << [user, :developer] } + + it 'returns 404' do + post :create, namespace_id: project.namespace, + project_id: project, + user_ids: project_user.id, + access_level: Gitlab::Access::GUEST + + expect(response).to have_http_status(404) + expect(project.users).not_to include project_user + end + end + + context 'when user has enough rights' do + before { project.team << [user, :master] } + + it 'adds user to members' do + post :create, namespace_id: project.namespace, + project_id: project, + user_ids: project_user.id, + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project)) + expect(project.users).to include project_user + end + + it 'adds no user to members' do + post :create, namespace_id: project.namespace, + project_id: project, + user_ids: '', + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'No users or groups specified.' + expect(response).to redirect_to(namespace_project_project_members_path(project.namespace, project)) + expect(project.users).not_to include project_user + end + end + end + end + describe 'DELETE destroy' do let(:member) { create(:project_member, :developer, project: project) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 47f89f744cb..ac862055ebc 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -265,10 +265,4 @@ describe Group, models: true do members end - - describe '#web_url' do - it 'returns the canonical URL' do - expect(group.web_url).to include("groups/#{group.name}") - end - end end diff --git a/spec/support/select2_helper.rb b/spec/support/select2_helper.rb index 35cc51725c6..d30cc8ff9f2 100644 --- a/spec/support/select2_helper.rb +++ b/spec/support/select2_helper.rb @@ -17,9 +17,9 @@ module Select2Helper selector = options.fetch(:from) if options[:multiple] - execute_script("$('#{selector}').select2('val', ['#{value}'], true);") + execute_script("$('#{selector}').select2('val', ['#{value}']).trigger('change');") else - execute_script("$('#{selector}').select2('val', '#{value}', true);") + execute_script("$('#{selector}').select2('val', '#{value}').trigger('change');") end end end