Merge branch 'dz-refactor-group-members-tests' into 'master'

Refactor group members tests

See merge request !12465

features/group/members.feature

@@ -4,65 +4,6 @@ Feature: Group Members
     And "John Doe" is owner of group "Owned"
     And "John Doe" is guest of group "Guest"
 
-  # Leave
-
-  @javascript
-  Scenario: Owner should be able to remove himself from group if he is not the last owner
-    Given "Mary Jane" is owner of group "Owned"
-    When I visit group "Owned" members page
-    Then I should see user "John Doe" in team list
-    Then I should see user "Mary Jane" in team list
-    When I click on the "Remove User From Group" button for "John Doe"
-    And I visit group "Owned" members page
-    Then I should not see user "John Doe" in team list
-    Then I should see user "Mary Jane" in team list
-
-  @javascript
-  Scenario: Owner should not be able to remove himself from group if he is the last owner
-    Given "Mary Jane" is guest of group "Owned"
-    When I visit group "Owned" members page
-    Then I should see user "John Doe" in team list
-    Then I should see user "Mary Jane" in team list
-    Then I should not see the "Remove User From Group" button for "John Doe"
-
-  @javascript
-  Scenario: Guest should be able to remove himself from group
-    Given "Mary Jane" is guest of group "Guest"
-    When I visit group "Guest" members page
-    Then I should see user "John Doe" in team list
-    Then I should see user "Mary Jane" in team list
-    When I click on the "Remove User From Group" button for "John Doe"
-    When I visit group "Guest" members page
-    Then I should not see user "John Doe" in team list
-    Then I should see user "Mary Jane" in team list
-
-  @javascript
-  Scenario: Guest should be able to remove himself from group even if he is the only user in the group
-    When I visit group "Guest" members page
-    Then I should see user "John Doe" in team list
-    When I click on the "Remove User From Group" button for "John Doe"
-    When I visit group "Guest" members page
-    Then I should not see user "John Doe" in team list
-
-  # Remove others
-
-  Scenario: Owner should be able to remove other users from group
-    Given "Mary Jane" is owner of group "Owned"
-    When I visit group "Owned" members page
-    Then I should see user "John Doe" in team list
-    Then I should see user "Mary Jane" in team list
-    When I click on the "Remove User From Group" button for "Mary Jane"
-    When I visit group "Owned" members page
-    Then I should see user "John Doe" in team list
-    Then I should not see user "Mary Jane" in team list
-
-  Scenario: Guest should not be able to remove other users from group
-    Given "Mary Jane" is guest of group "Guest"
-    When I visit group "Guest" members page
-    Then I should see user "John Doe" in team list
-    Then I should see user "Mary Jane" in team list
-    Then I should not see the "Remove User From Group" button for "Mary Jane"
-
   Scenario: Search member by name
     Given "Mary Jane" is guest of group "Guest"
     And I visit group "Guest" members page

spec/features/groups/members/last_owner_cannot_leave_group_spec.rb

@@ -1,16 +0,0 @@
-require 'spec_helper'
-
-feature 'Groups > Members > Last owner cannot leave group', feature: true do
-  let(:owner) { create(:user) }
-  let(:group) { create(:group) }
-
-  background do
-    group.add_owner(owner)
-    gitlab_sign_in(owner)
-    visit group_path(group)
-  end
-
-  scenario 'user does not see a "Leave group" link' do
-    expect(page).not_to have_content 'Leave group'
-  end
-end

spec/features/groups/members/leave_group_spec.rb

@@ -0,0 +1,62 @@
+require 'spec_helper'
+
+feature 'Groups > Members > Leave group', feature: true do
+  let(:user) { create(:user) }
+  let(:other_user) { create(:user) }
+  let(:group) { create(:group) }
+
+  background do
+    gitlab_sign_in(user)
+  end
+
+  scenario 'guest leaves the group' do
+    group.add_guest(user)
+    group.add_owner(other_user)
+
+    visit group_path(group)
+    click_link 'Leave group'
+
+    expect(current_path).to eq(dashboard_groups_path)
+    expect(page).to have_content left_group_message(group)
+    expect(group.users).not_to include(user)
+  end
+
+  scenario 'guest leaves the group as last member' do
+    group.add_guest(user)
+
+    visit group_path(group)
+    click_link 'Leave group'
+
+    expect(current_path).to eq(dashboard_groups_path)
+    expect(page).to have_content left_group_message(group)
+    expect(group.users).not_to include(user)
+  end
+
+  scenario 'owner leaves the group if they is not the last owner' do
+    group.add_owner(user)
+    group.add_owner(other_user)
+
+    visit group_path(group)
+    click_link 'Leave group'
+
+    expect(current_path).to eq(dashboard_groups_path)
+    expect(page).to have_content left_group_message(group)
+    expect(group.users).not_to include(user)
+  end
+
+  scenario 'owner can not leave the group if they is a last owner' do
+    group.add_owner(user)
+
+    visit group_path(group)
+
+    expect(page).not_to have_content 'Leave group'
+
+    visit group_group_members_path(group)
+
+    expect(find(:css, '.project-members-page li', text: user.name)).not_to have_selector(:css, 'a.btn-remove')
+  end
+
+  def left_group_message(group)
+    "You left the \"#{group.name}\""
+  end
+end

spec/features/groups/members/list_members_spec.rb

@@ -0,0 +1,42 @@
+require 'spec_helper'
+
+feature 'Groups > Members > List members', feature: true do
+  include Select2Helper
+
+  let(:user1) { create(:user, name: 'John Doe') }
+  let(:user2) { create(:user, name: 'Mary Jane') }
+  let(:group) { create(:group) }
+  let(:nested_group) { create(:group, parent: group) }
+
+  background do
+    gitlab_sign_in(user1)
+  end
+
+  scenario 'show members from current group and parent', :nested_groups do
+    group.add_developer(user1)
+    nested_group.add_developer(user2)
+
+    visit group_group_members_path(nested_group)
+
+    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 current group and parent', :nested_groups do
+    group.add_developer(user1)
+    nested_group.add_developer(user1)
+
+    visit group_group_members_path(nested_group)
+
+    expect(first_row.text).to include(user1.name)
+    expect(second_row).to be_blank
+  end
+
+  def first_row
+    page.all('ul.content-list > li')[0]
+  end
+
+  def second_row
+    page.all('ul.content-list > li')[1]
+  end
+end

spec/features/groups/members/manage_access_requests_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-feature 'Groups > Members > Owner manages access requests', feature: true do
+feature 'Groups > Members > Manage access requests', feature: true do
   let(:user) { create(:user) }
   let(:owner) { create(:user) }
   let(:group) { create(:group, :public, :access_requestable) }
@@ -17,7 +17,7 @@ feature 'Groups > Members > Owner manages access requests', feature: true do
     expect_visible_access_request(group, user)
   end
 
-  scenario 'master can grant access' do
+  scenario 'owner can grant access' do
     visit group_group_members_path(group)
 
     expect_visible_access_request(group, user)
@@ -28,7 +28,7 @@ feature 'Groups > Members > Owner manages access requests', feature: true do
     expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was granted"
   end
 
-  scenario 'master can deny access' do
+  scenario 'owner can deny access' do
     visit group_group_members_path(group)
 
     expect_visible_access_request(group, user)

spec/features/groups/members/manage_members.rb

@@ -1,37 +1,16 @@
 require 'spec_helper'
 
-feature 'Groups members list', feature: true do
+feature 'Groups > Members > Manage members', feature: true do
   include Select2Helper
 
   let(:user1) { create(:user, name: 'John Doe') }
   let(:user2) { create(:user, name: 'Mary Jane') }
   let(:group) { create(:group) }
-  let(:nested_group) { create(:group, parent: group) }
 
   background do
     gitlab_sign_in(user1)
   end
 
-  scenario 'show members from current group and parent', :nested_groups do
-    group.add_developer(user1)
-    nested_group.add_developer(user2)
-
-    visit group_group_members_path(nested_group)
-
-    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 current group and parent', :nested_groups do
-    group.add_developer(user1)
-    nested_group.add_developer(user1)
-
-    visit group_group_members_path(nested_group)
-
-    expect(first_row.text).to include(user1.name)
-    expect(second_row).to be_blank
-  end
-
   scenario 'update user to owner level', :js do
     group.add_owner(user1)
     group.add_developer(user2)
@@ -59,6 +38,18 @@ feature 'Groups members list', feature: true do
     end
   end
 
+  scenario 'remove user from group', :js do
+    group.add_owner(user1)
+    group.add_developer(user2)
+
+    visit group_group_members_path(group)
+
+    find(:css, '.project-members-page li', text: user2.name).find(:css, 'a.btn-remove').click
+
+    expect(page).not_to have_content(user2.name)
+    expect(group.users).not_to include(user2)
+  end
+
   scenario 'add yourself to group when already an owner', :js do
     group.add_owner(user1)
 
@@ -86,6 +77,23 @@ feature 'Groups members list', feature: true do
     end
   end
 
+  scenario 'guest can not manage other users' do
+    group.add_guest(user1)
+    group.add_developer(user2)
+
+    visit group_group_members_path(group)
+
+    expect(page).not_to have_button 'Add to group'
+
+    page.within(second_row) do
+      # Can not modify user2 role
+      expect(page).not_to have_button 'Developer'
+
+      # Can not remove user2
+      expect(page).not_to have_css('a.btn-remove')
+    end
+  end
+
   def first_row
     page.all('ul.content-list > li')[0]
   end

spec/features/groups/members/member_cannot_request_access_to_his_project_spec.rb

@@ -1,16 +0,0 @@
-require 'spec_helper'
-
-feature 'Groups > Members > Member cannot request access to his project', feature: true do
-  let(:member) { create(:user) }
-  let(:group) { create(:group) }
-
-  background do
-    group.add_developer(member)
-    gitlab_sign_in(member)
-    visit group_path(group)
-  end
-
-  scenario 'member does not see the request access button' do
-    expect(page).not_to have_content 'Request Access'
-  end
-end

spec/features/groups/members/member_leaves_group_spec.rb

@@ -1,21 +0,0 @@
-require 'spec_helper'
-
-feature 'Groups > Members > Member leaves group', feature: true do
-  let(:user) { create(:user) }
-  let(:owner) { create(:user) }
-  let(:group) { create(:group, :public) }
-
-  background do
-    group.add_owner(owner)
-    group.add_developer(user)
-    gitlab_sign_in(user)
-    visit group_path(group)
-  end
-
-  scenario 'user leaves group' do
-    click_link 'Leave group'
-
-    expect(current_path).to eq(dashboard_groups_path)
-    expect(group.users.exists?(user.id)).to be_falsey
-  end
-end

spec/features/groups/members/request_access_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-feature 'Groups > Members > User requests access', feature: true do
+feature 'Groups > Members > Request access', feature: true do
   let(:user) { create(:user) }
   let(:owner) { create(:user) }
   let(:group) { create(:group, :public, :access_requestable) }
@@ -68,4 +68,11 @@ feature 'Groups > Members > User requests access', feature: true do
     expect(group.requesters.exists?(user_id: user)).to be_falsey
     expect(page).to have_content 'Your access request to the group has been withdrawn.'
   end
+
+  scenario 'member does not see the request access button' do
+    group.add_owner(user)
+    visit group_path(group)
+
+    expect(page).not_to have_content 'Request Access'
+  end
 end

spec/features/groups/members/sort_members_spec.rb

@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-feature 'Groups > Members > Sorting', feature: true do
+feature 'Groups > Members > Sort members', feature: true do
   let(:owner)     { create(:user, name: 'John Doe') }
   let(:developer) { create(:user, name: 'Mary Jane', last_sign_in_at: 5.days.ago) }
   let(:group)     { create(:group) }