Skip to content

G
gitlab-ce
Commit d39bcf8c authored by Douwe Maan's avatar Douwe Maan
Browse files
Options

Merge branch 'dz-refactor-create-members' into 'master' 

Refactor code that creates and destroys project/group members

See merge request !10735
parents cc9e92a0 5f087604
Hide whitespace changes
Inline Side-by-side
Showing with 200 additions and 248 deletions
app/controllers/concerns/membership_actions.rb
View file @ d39bcf8c
module MembershipActions
extend ActiveSupport::Concern
def create
status = Members::CreateService.new(membershipable, current_user, params).execute
redirect_url = members_page_url
if status
redirect_to redirect_url, notice: 'Users were successfully added.'
else
redirect_to redirect_url, alert: 'No users specified.'
end
end
def destroy
Members::DestroyService.new(membershipable, current_user, params).
execute(:all)
respond_to do |format|
format.html do
message = "User was successfully removed from #{source_type}."
redirect_to members_page_url, notice: message
end
format.js { head :ok }
end
end
def request_access
membershipable.request_access(current_user)
......@@ -11,20 +37,20 @@ module MembershipActions
def approve_access_request
Members::ApproveAccessRequestService.new(membershipable, current_user, params).execute
redirect_to polymorphic_url([membershipable, :members])
redirect_to members_page_url
end
def leave
member = Members::DestroyService.new(membershipable, current_user, user_id: current_user.id).
execute(:all)
source_type = membershipable.class.to_s.humanize(capitalize: false)
notice =
if member.request?
"Your access request to the #{source_type} has been withdrawn."
else
"You left the \"#{membershipable.human_name}\" #{source_type}."
end
redirect_path = member.request? ? member.source : [:dashboard, membershipable.class.to_s.tableize]
redirect_to redirect_path, notice: notice
......@@ -35,4 +61,16 @@ module MembershipActions
def membershipable
raise NotImplementedError
end
def members_page_url
if membershipable.is_a?(Project)
project_settings_members_path(membershipable)
else
polymorphic_url([membershipable, :members])
end
end
def source_type
@source_type ||= membershipable.class.to_s.humanize(capitalize: false)
end
end
app/controllers/groups/group_members_controller.rb
View file @ d39bcf8c
......@@ -21,21 +21,6 @@ class Groups::GroupMembersController < Groups::ApplicationController
@group_member = @group.group_members.new
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],
current_user: current_user,
expires_at: params[:expires_at]
)
redirect_to group_group_members_path(@group), notice: 'Users were successfully added.'
end
def update
@group_member = @group.group_members.find(params[:id])
......@@ -44,15 +29,6 @@ class Groups::GroupMembersController < Groups::ApplicationController
@group_member.update_attributes(member_params)
end
def destroy
Members::DestroyService.new(@group, current_user, id: params[:id]).execute(:all)
respond_to do |format|
format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' }
format.js { head :ok }
end
end
def resend_invite
redirect_path = group_group_members_path(@group)
......
app/controllers/projects/project_members_controller.rb
View file @ d39bcf8c
......@@ -10,18 +10,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController
redirect_to namespace_project_settings_members_path(@project.namespace, @project, sort: sort)
end
def create
status = Members::CreateService.new(@project, current_user, params).execute
redirect_url = namespace_project_settings_members_path(@project.namespace, @project)
if status
redirect_to redirect_url, notice: 'Users were successfully added.'
else
redirect_to redirect_url, alert: 'No users or groups specified.'
end
end
def update
@project_member = @project.project_members.find(params[:id])
......@@ -30,18 +18,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController
@project_member.update_attributes(member_params)
end
def destroy
Members::DestroyService.new(@project, current_user, params).
execute(:all)
respond_to do |format|
format.html do
redirect_to namespace_project_settings_members_path(@project.namespace, @project)
end
format.js { head :ok }
end
end
def resend_invite
redirect_path = namespace_project_settings_members_path(@project.namespace, @project)
......
app/models/project.rb
View file @ d39bcf8c
......@@ -181,7 +181,7 @@ class Project < ActiveRecord::Base
delegate :name, to: :owner, allow_nil: true, prefix: true
delegate :count, to: :forks, prefix: true
delegate :members, to: :team, prefix: true
delegate :add_user, to: :team
delegate :add_user, :add_users, to: :team
delegate :add_guest, :add_reporter, :add_developer, :add_master, to: :team
delegate :empty_repo?, to: :repository
......
app/services/members/create_service.rb
View file @ d39bcf8c
module Members
class CreateService < BaseService
def initialize(source, current_user, params = {})
@source = source
@current_user = current_user
@params = params
end
def execute
return false if params[:user_ids].blank?
project.team.add_users(
@source.add_users(
params[:user_ids].split(','),
params[:access_level],
expires_at: params[:expires_at],
......
changelogs/unreleased/dz-refactor-create-members.yml 0 → 100644
View file @ d39bcf8c
---
title: Refactor code that creates project/group members
merge_request: 10735
author:
features/group/members.feature
View file @ d39bcf8c
......@@ -4,40 +4,6 @@ Feature: Group Members
And "John Doe" is owner of group "Owned"
And "John Doe" is guest of group "Guest"
@javascript
Scenario: I should add user to group "Owned"
Given User "Mary Jane" exists
When I visit group "Owned" members page
And I select user "Mary Jane" from list with role "Reporter"
Then I should see user "Mary Jane" in team list
@javascript
Scenario: Add user to group
Given gitlab user "Mike"
When I visit group "Owned" members page
When I select "Mike" as "Reporter"
Then I should see "Mike" in team list as "Reporter"
@javascript
Scenario: Ignore add user to group when is already Owner
Given gitlab user "Mike"
When I visit group "Owned" members page
When I select "Mike" as "Reporter"
Then I should see "Mike" in team list as "Owner"
@javascript
Scenario: Invite user to group
When I visit group "Owned" members page
When I select "sjobs@apple.com" as "Reporter"
Then I should see "sjobs@apple.com" in team list as invited "Reporter"
@javascript
Scenario: Edit group member permissions
Given "Mary Jane" is guest of group "Owned"
And I visit group "Owned" members page
When I change the "Mary Jane" role to "Developer"
Then I should see "Mary Jane" as "Developer"
# Leave
@javascript
......
features/project/team_management.feature
View file @ d39bcf8c
......@@ -7,26 +7,6 @@ Feature: Project Team Management
And "Dmitriy" is "Shop" developer
And I visit project "Shop" team page
Scenario: See all team members
Then I should be able to see myself in team
And I should see "Dmitriy" in team list
@javascript
Scenario: Add user to project
When I select "Mike" as "Reporter"
Then I should see "Mike" in team list as "Reporter"
@javascript
Scenario: Invite user to project
When I select "sjobs@apple.com" as "Reporter"
Then I should see "sjobs@apple.com" in team list as invited "Reporter"
@javascript
Scenario: Update user access
Given I should see "Dmitriy" in team list as "Developer"
And I change "Dmitriy" role to "Reporter"
And I should see "Dmitriy" in team list as "Reporter"
Scenario: Cancel team member
Given I click cancel link for "Dmitriy"
Then I visit project "Shop" team page
......
features/steps/group/members.rb
View file @ d39bcf8c
......@@ -4,71 +4,6 @@ class Spinach::Features::GroupMembers < Spinach::FeatureSteps
include SharedPaths
include SharedGroup
include SharedUser
include Select2Helper
step 'I select "Mike" as "Reporter"' do
user = User.find_by(name: "Mike")
page.within ".users-group-form" do
select2(user.id, from: "#user_ids", multiple: true)
select "Reporter", from: "access_level"
end
click_button "Add to group"
end
step 'I select "Mike" as "Master"' do
user = User.find_by(name: "Mike")
page.within ".users-group-form" do
select2(user.id, from: "#user_ids", multiple: true)
select "Master", from: "access_level"
end
click_button "Add to group"
end
step 'I should see "Mike" in team list as "Reporter"' do
page.within '.content-list' do
expect(page).to have_content('Mike')
expect(page).to have_content('Reporter')
end
end
step 'I should see "Mike" in team list as "Owner"' do
page.within '.content-list' do
expect(page).to have_content('Mike')
expect(page).to have_content('Owner')
end
end
step 'I select "sjobs@apple.com" as "Reporter"' do
page.within ".users-group-form" do
select2("sjobs@apple.com", from: "#user_ids", multiple: true)
select "Reporter", from: "access_level"
end
click_button "Add to group"
end
step 'I should see "sjobs@apple.com" in team list as invited "Reporter"' do
page.within '.content-list' do
expect(page).to have_content('sjobs@apple.com')
expect(page).to have_content('Invited')
expect(page).to have_content('Reporter')
end
end
step 'I select user "Mary Jane" from list with role "Reporter"' do
user = User.find_by(name: "Mary Jane") || create(:user, name: "Mary Jane")
page.within ".users-group-form" do
select2(user.id, from: "#user_ids", multiple: true)
select "Reporter", from: "access_level"
end
click_button "Add to group"
end
step 'I should see user "John Doe" in team list' do
expect(group_members_list).to have_content("John Doe")
......
features/steps/project/team_management.rb
View file @ d39bcf8c
......@@ -4,25 +4,10 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
include SharedPaths
include Select2Helper
step 'I should be able to see myself in team' do
expect(page).to have_content(@user.name)
expect(page).to have_content(@user.username)
end
step 'I should see "Dmitriy" in team list' do
step 'I should not see "Dmitriy" in team list' do
user = User.find_by(name: "Dmitriy")
expect(page).to have_content(user.name)
expect(page).to have_content(user.username)
end
step 'I select "Mike" as "Reporter"' do
user = User.find_by(name: "Mike")
page.within ".users-project-form" do
select2(user.id, from: "#user_ids", multiple: true)
select "Reporter", from: "access_level"
end
click_button "Add to project"
expect(page).not_to have_content(user.name)
expect(page).not_to have_content(user.username)
end
step 'I should see "Mike" in team list as "Reporter"' do
......@@ -34,60 +19,6 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
end
end
step 'I select "sjobs@apple.com" as "Reporter"' do
page.within ".users-project-form" do
find('#user_ids', visible: false).set('sjobs@apple.com')
select "Reporter", from: "access_level"
end
click_button "Add to project"
end
step 'I should see "sjobs@apple.com" in team list as invited "Reporter"' do
project_member = project.project_members.find_by(invite_email: 'sjobs@apple.com')
page.within "#project_member_#{project_member.id}" do
expect(page).to have_content('sjobs@apple.com')
expect(page).to have_content('Invited')
expect(page).to have_content('Reporter')
end
end
step 'I should see "Dmitriy" in team list as "Developer"' do
user = User.find_by(name: 'Dmitriy')
project_member = project.project_members.find_by(user_id: user.id)
page.within "#project_member_#{project_member.id}" do
expect(page).to have_content('Dmitriy')
expect(page).to have_content('Developer')
end
end
step 'I change "Dmitriy" role to "Reporter"' do
project = Project.find_by(name: "Shop")
user = User.find_by(name: 'Dmitriy')
project_member = project.project_members.find_by(user_id: user.id)
page.within "#project_member_#{project_member.id}" do
click_button project_member.human_access
page.within '.dropdown-menu' do
click_link 'Reporter'
end
end
end
step 'I should see "Dmitriy" in team list as "Reporter"' do
user = User.find_by(name: 'Dmitriy')
project_member = project.project_members.find_by(user_id: user.id)
page.within "#project_member_#{project_member.id}" do
expect(page).to have_content('Dmitriy')
expect(page).to have_content('Reporter')
end
end
step 'I should not see "Dmitriy" in team list' do
user = User.find_by(name: "Dmitriy")
expect(page).not_to have_content(user.name)
expect(page).not_to have_content(user.username)
end
step 'gitlab user "Mike"' do
create(:user, name: "Mike")
end
......@@ -113,7 +44,7 @@ class Spinach::Features::ProjectTeamManagement < Spinach::FeatureSteps
project.team << [user, :reporter]
end
step 'I click link "Import team from another project"' do
step 'I click link "Import team from another project"' do
page.within '.users-project-form' do
click_link "Import"
end
......
spec/controllers/projects/project_members_controller_spec.rb
View file @ d39bcf8c
......@@ -55,7 +55,7 @@ describe Projects::ProjectMembersController do
user_ids: '',
access_level: Gitlab::Access::GUEST
expect(response).to set_flash.to 'No users or groups specified.'
expect(response).to set_flash.to 'No users specified.'
expect(response).to redirect_to(namespace_project_settings_members_path(project.namespace, project))
end
end
......@@ -225,7 +225,7 @@ describe Projects::ProjectMembersController do
id: member
expect(response).to redirect_to(
namespace_project_project_members_path(project.namespace, project)
namespace_project_settings_members_path(project.namespace, project)
)
expect(project.members).to include member
end
......
spec/features/groups/members/list_spec.rb
View file @ d39bcf8c
require 'spec_helper'
feature 'Groups members list', feature: true do
include Select2Helper
let(:user1) { create(:user, name: 'John Doe') }
let(:user2) { create(:user, name: 'Mary Jane') }
let(:group) { create(:group) }
......@@ -30,7 +32,7 @@ feature 'Groups members list', feature: true do
expect(second_row).to be_blank
end
it 'updates user to owner level', :js do
scenario 'update user to owner level', :js do
group.add_owner(user1)
group.add_developer(user2)
......@@ -38,13 +40,52 @@ feature 'Groups members list', feature: true do
page.within(second_row) do
click_button('Developer')
click_link('Owner')
expect(page).to have_button('Owner')
end
end
scenario 'add user to group', :js do
group.add_owner(user1)
visit group_group_members_path(group)
add_user(user2.id, 'Reporter')
page.within(second_row) do
expect(page).to have_content(user2.name)
expect(page).to have_button('Reporter')
end
end
scenario 'add yourself to group when already an owner', :js do
group.add_owner(user1)
visit group_group_members_path(group)
add_user(user1.id, 'Reporter')
page.within(first_row) do
expect(page).to have_content(user1.name)
expect(page).to have_content('Owner')
end
end
scenario 'invite user to group', :js do
group.add_owner(user1)
visit group_group_members_path(group)
add_user('test@example.com', 'Reporter')
page.within(second_row) do
expect(page).to have_content('test@example.com')
expect(page).to have_content('Invited')
expect(page).to have_button('Reporter')
end
end
def first_row
page.all('ul.content-list > li')[0]
end
......@@ -52,4 +93,13 @@ feature 'Groups members list', feature: true do
def second_row
page.all('ul.content-list > li')[1]
end
def add_user(id, role)
page.within ".users-group-form" do
select2(id, from: "#user_ids", multiple: true)
select(role, from: "access_level")
end
click_button "Add to group"
end
end
spec/features/projects/members/list_spec.rb 0 → 100644
View file @ d39bcf8c
require 'spec_helper'
feature 'Project members list', feature: true do
include Select2Helper
let(:user1) { create(:user, name: 'John Doe') }
let(:user2) { create(:user, name: 'Mary Jane') }
let(:group) { create(:group) }
let(:project) { create(:project, namespace: group) }
background do
login_as(user1)
group.add_owner(user1)
end
scenario 'show members from project and group' do
project.add_developer(user2)
visit_members_page
expect(first_row.text).to include(user1.name)
expect(second_row.text).to include(user2.name)
end
scenario 'show user once if member of both group and project' do
project.add_developer(user1)
visit_members_page
expect(first_row.text).to include(user1.name)
expect(second_row).to be_blank
end
scenario 'update user acess level', :js do
project.add_developer(user2)
visit_members_page
page.within(second_row) do
click_button('Developer')
click_link('Reporter')
expect(page).to have_button('Reporter')
end
end
scenario 'add user to project', :js do
visit_members_page
add_user(user2.id, 'Reporter')
page.within(second_row) do
expect(page).to have_content(user2.name)
expect(page).to have_button('Reporter')
end
end
scenario 'invite user to project', :js do
visit_members_page
add_user('test@example.com', 'Reporter')
page.within(second_row) do
expect(page).to have_content('test@example.com')
expect(page).to have_content('Invited')
expect(page).to have_button('Reporter')
end
end
def first_row
page.all('ul.content-list > li')[0]
end
def second_row
page.all('ul.content-list > li')[1]
end
def add_user(id, role)
page.within ".users-project-form" do
select2(id, from: "#user_ids", multiple: true)
select(role, from: "access_level")
end
click_button "Add to project"
end
def visit_members_page
visit namespace_project_settings_members_path(project.namespace, project)
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7