UI and copywriting improvements13948-access-request-to-projects-and-groups

+ Move 'Edit Project/Group' out of membership-related partial
+ Show the access request buttons only to logged-in users
+ Put the request access buttons out of in a more visible button
+ Improve the copy in the #remove_member_message helper

Signed-off-by: Rémy Coutable <remy@rymai.me>

17 files changed, 372 insertions, 317 deletions

diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb
index 0ca8a656f63..89c2c26a367 100644
--- a/spec/controllers/groups/group_members_controller_spec.rb
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -35,7 +35,7 @@ describe Groups::GroupMembersController do
       let(:group_user) { create(:user) }
       let(:member) do
         group.add_developer(group_user)
-        group.group_members.find_by(user_id: group_user.id)
+        group.members.find_by(user_id: group_user)
       end
 
       context 'when user does not have enough rights' do
@@ -103,7 +103,7 @@ describe Groups::GroupMembersController do
         it 'removes user from members' do
           delete :leave, group_id: group
 
-          expect(response).to set_flash.to "You left #{group.name} group."
+          expect(response).to set_flash.to "You left the \"#{group.name}\" group."
           expect(response).to redirect_to(dashboard_groups_path)
           expect(group.users).not_to include user
         end
@@ -118,8 +118,8 @@ describe Groups::GroupMembersController do
         it 'cannot removes himself from the group' do
           delete :leave, group_id: group
 
-          expect(response).to redirect_to(dashboard_groups_path)
-          expect(response).to set_flash[:alert].to "You can not leave #{group.name} group because you're the last owner. Transfer or delete the group."
+          expect(response).to redirect_to(group_path(group))
+          expect(response).to set_flash[:alert].to "You can not leave the \"#{group.name}\" group. Transfer or delete the group."
           expect(group.users).to include user
         end
       end
@@ -133,9 +133,9 @@ describe Groups::GroupMembersController do
         it 'removes user from members' do
           delete :leave, group_id: group
 
-          expect(response).to set_flash.to 'You withdrawn your access request to the group.'
+          expect(response).to set_flash.to 'Your access request to the group has been withdrawn.'
           expect(response).to redirect_to(dashboard_groups_path)
-          expect(group.group_members.request).to be_empty
+          expect(group.members.request).to be_empty
           expect(group.users).not_to include user
         end
       end
@@ -155,18 +155,18 @@ describe Groups::GroupMembersController do
 
       expect(response).to set_flash.to 'Your request for access has been queued for review.'
       expect(response).to redirect_to(group_path(group))
-      expect(group.group_members.request.find_by(created_by_id: user.id).created_by).to eq user
+      expect(group.members.request.exists?(user_id: user)).to be_truthy
       expect(group.users).not_to include user
     end
   end
 
-  describe '#approve' do
+  describe '#approve_access_request' do
     let(:group) { create(:group, :public) }
 
     context 'when member is not found' do
       it 'returns 403' do
         post :approve_access_request, group_id: group,
-                       id: 42
+                                      id: 42
 
         expect(response.status).to eq(403)
       end
@@ -177,7 +177,7 @@ describe Groups::GroupMembersController do
       let(:group_requester) { create(:user) }
       let(:member) do
         group.request_access(group_requester)
-        group.group_members.request.find_by(created_by_id: group_requester.id)
+        group.members.request.find_by(user_id: group_requester)
       end
 
       context 'when user does not have enough rights' do
@@ -188,7 +188,7 @@ describe Groups::GroupMembersController do
 
         it 'returns 403' do
           post :approve_access_request, group_id: group,
-                         id: member
+                                        id: member
 
           expect(response.status).to eq(403)
           expect(group.users).not_to include group_requester
@@ -203,7 +203,7 @@ describe Groups::GroupMembersController do
 
         it 'adds user to members' do
           post :approve_access_request, group_id: group,
-                         id: member
+                                        id: member
 
           expect(response).to redirect_to(group_group_members_path(group))
           expect(group.users).to include group_requester
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index d3bd2d0bbba..fc5f458e795 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -80,7 +80,7 @@ describe Projects::ProjectMembersController do
       let(:team_user) { create(:user) }
       let(:member) do
         project.team << [team_user, :developer]
-        project.project_members.find_by(user_id: team_user.id)
+        project.members.find_by(user_id: team_user.id)
       end
 
       context 'when user does not have enough rights' do
@@ -154,7 +154,7 @@ describe Projects::ProjectMembersController do
           delete :leave, namespace_id: project.namespace,
                          project_id: project
 
-          expect(response).to set_flash.to 'You left the project.'
+          expect(response).to set_flash.to "You left the \"#{project.human_name}\" project."
           expect(response).to redirect_to(dashboard_projects_path)
           expect(project.users).not_to include user
         end
@@ -167,14 +167,14 @@ describe Projects::ProjectMembersController do
           sign_in(user)
         end
 
-        it 'cannot removes himself from the project' do
+        it 'cannot remove himself from the project' do
           delete :leave, namespace_id: project.namespace,
                          project_id: project
 
           expect(response).to redirect_to(
-            namespace_project_project_members_path(project.namespace, project)
+            namespace_project_path(project.namespace, project)
           )
-          expect(response).to set_flash[:alert].to 'You can not leave your own project. Transfer or delete the project.'
+          expect(response).to set_flash[:alert].to "You can not leave the \"#{project.human_name}\" project. Transfer or delete the project."
           expect(project.users).to include user
         end
       end
@@ -189,9 +189,9 @@ describe Projects::ProjectMembersController do
           delete :leave, namespace_id: project.namespace,
                          project_id: project
 
-          expect(response).to set_flash.to 'You withdrawn your access request to the project.'
+          expect(response).to set_flash.to 'Your access request to the project has been withdrawn.'
           expect(response).to redirect_to(dashboard_projects_path)
-          expect(project.project_members.request).to be_empty
+          expect(project.members.request).to be_empty
           expect(project.users).not_to include user
         end
       end
@@ -214,7 +214,7 @@ describe Projects::ProjectMembersController do
       expect(response).to redirect_to(
         namespace_project_path(project.namespace, project)
       )
-      expect(project.project_members.request.find_by(created_by_id: user.id).created_by).to eq user
+      expect(project.members.request.exists?(user_id: user)).to be_truthy
       expect(project.users).not_to include user
     end
   end
@@ -225,8 +225,8 @@ describe Projects::ProjectMembersController do
     context 'when member is not found' do
       it 'returns 404' do
         post :approve_access_request, namespace_id: project.namespace,
-                       project_id: project,
-                       id: 42
+                                      project_id: project,
+                                      id: 42
 
         expect(response.status).to eq(404)
       end
@@ -237,7 +237,7 @@ describe Projects::ProjectMembersController do
       let(:team_requester) { create(:user) }
       let(:member) do
         project.request_access(team_requester)
-        project.project_members.request.find_by(created_by_id: team_requester.id)
+        project.members.request.find_by(user_id: team_requester.id)
       end
 
       context 'when user does not have enough rights' do
@@ -248,8 +248,8 @@ describe Projects::ProjectMembersController do
 
         it 'returns 404' do
           post :approve_access_request, namespace_id: project.namespace,
-                         project_id: project,
-                         id: member
+                                        project_id: project,
+                                        id: member
 
           expect(response.status).to eq(404)
           expect(project.users).not_to include team_requester
@@ -264,8 +264,8 @@ describe Projects::ProjectMembersController do
 
         it 'adds user to members' do
           post :approve_access_request, namespace_id: project.namespace,
-                         project_id: project,
-                         id: member
+                                        project_id: project,
+                                        id: member
 
           expect(response).to redirect_to(
             namespace_project_project_members_path(project.namespace, project)
diff --git a/spec/features/groups/members/owner_manages_access_requests_spec.rb b/spec/features/groups/members/owner_manages_access_requests_spec.rb
index d5b5e0e35ea..22525ce530b 100644
--- a/spec/features/groups/members/owner_manages_access_requests_spec.rb
+++ b/spec/features/groups/members/owner_manages_access_requests_spec.rb
@@ -22,12 +22,10 @@ feature 'Groups > Members > Owner manages access requests', feature: true do
 
     expect_visible_access_request(group, user)
 
-    perform_enqueued_jobs do
-      click_on 'Grant access'
-    end
+    perform_enqueued_jobs { click_on 'Grant access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{group.name} group was granted/
+    expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was granted"
   end
 
   scenario 'master can deny access' do
@@ -35,17 +33,15 @@ feature 'Groups > Members > Owner manages access requests', feature: true do
 
     expect_visible_access_request(group, user)
 
-    perform_enqueued_jobs do
-      click_on 'Deny access'
-    end
+    perform_enqueued_jobs { click_on 'Deny access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{group.name} group was denied/
+    expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{group.name} group was denied"
   end
 
 
   def expect_visible_access_request(group, user)
-    expect(group.access_requested?(user)).to be_truthy
+    expect(group.members.request.exists?(user_id: user)).to be_truthy
     expect(page).to have_content "#{group.name} access requests (1)"
     expect(page).to have_content user.name
   end
diff --git a/spec/features/groups/members/user_requests_access_spec.rb b/spec/features/groups/members/user_requests_access_spec.rb
index 9b8492807fa..a878a96b6ee 100644
--- a/spec/features/groups/members/user_requests_access_spec.rb
+++ b/spec/features/groups/members/user_requests_access_spec.rb
@@ -8,47 +8,41 @@ feature 'Groups > Members > User requests access', feature: true do
   background do
     group.add_owner(owner)
     login_as(user)
+    visit group_path(group)
   end
 
   scenario 'user can request access to a group' do
-    visit group_path(group)
-
-    perform_enqueued_jobs do
-      click_link 'Request Access'
-    end
+    perform_enqueued_jobs { click_link 'Request Access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [owner.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Request to join #{group.name} group/
+    expect(ActionMailer::Base.deliveries.last.subject).to match "Request to join the #{group.name} group"
 
-    expect(group.access_requested?(user)).to be_truthy
+    expect(group.members.request.exists?(user_id: user)).to be_truthy
     expect(page).to have_content 'Your request for access has been queued for review.'
-    expect(page).to have_content 'Withdraw Request'
+
+    expect(page).to have_content 'Withdraw Access Request'
   end
 
   scenario 'user is not listed in the group members page' do
-    visit group_path(group)
-
     click_link 'Request Access'
 
-    expect(group.access_requested?(user)).to be_truthy
+    expect(group.members.request.exists?(user_id: user)).to be_truthy
 
     click_link 'Members'
 
-    visit group_group_members_path(group)
     page.within('.content') do
       expect(page).not_to have_content(user.name)
     end
   end
 
   scenario 'user can withdraw its request for access' do
-    visit group_path(group)
     click_link 'Request Access'
 
-    expect(group.access_requested?(user)).to be_truthy
+    expect(group.members.request.exists?(user_id: user)).to be_truthy
 
-    click_link 'Withdraw Request'
+    click_link 'Withdraw Access Request'
 
-    expect(group.access_requested?(user)).to be_falsey
-    expect(page).to have_content 'You withdrawn your access request to the group.'
+    expect(group.members.request.exists?(user_id: user)).to be_falsey
+    expect(page).to have_content 'Your access request to the group has been withdrawn.'
   end
 end
diff --git a/spec/features/projects/members/master_manages_access_requests_spec.rb b/spec/features/projects/members/master_manages_access_requests_spec.rb
index 1b5490ba97f..5fe4caa12f0 100644
--- a/spec/features/projects/members/master_manages_access_requests_spec.rb
+++ b/spec/features/projects/members/master_manages_access_requests_spec.rb
@@ -22,12 +22,10 @@ feature 'Projects > Members > Master manages access requests', feature: true do
 
     expect_visible_access_request(project, user)
 
-    perform_enqueued_jobs do
-      click_on 'Grant access'
-    end
+    perform_enqueued_jobs { click_on 'Grant access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{project.name_with_namespace} project was granted/
+    expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{project.name_with_namespace} project was granted"
   end
 
   scenario 'master can deny access' do
@@ -35,16 +33,14 @@ feature 'Projects > Members > Master manages access requests', feature: true do
 
     expect_visible_access_request(project, user)
 
-    perform_enqueued_jobs do
-      click_on 'Deny access'
-    end
+    perform_enqueued_jobs { click_on 'Deny access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [user.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Access to #{project.name_with_namespace} project was denied/
+    expect(ActionMailer::Base.deliveries.last.subject).to match "Access to the #{project.name_with_namespace} project was denied"
   end
 
   def expect_visible_access_request(project, user)
-    expect(project.access_requested?(user)).to be_truthy
+    expect(project.members.request.exists?(user_id: user)).to be_truthy
     expect(page).to have_content "#{project.name} access requests (1)"
     expect(page).to have_content user.name
   end
diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb
index 58a7ec1880d..fd92a3a2f0c 100644
--- a/spec/features/projects/members/user_requests_access_spec.rb
+++ b/spec/features/projects/members/user_requests_access_spec.rb
@@ -8,30 +8,27 @@ feature 'Projects > Members > User requests access', feature: true do
   background do
     project.team << [master, :master]
     login_as(user)
+    visit namespace_project_path(project.namespace, project)
   end
 
   scenario 'user can request access to a project' do
-    visit namespace_project_path(project.namespace, project)
-
-    perform_enqueued_jobs do
-      click_link 'Request Access'
-    end
+    perform_enqueued_jobs { click_link 'Request Access' }
 
     expect(ActionMailer::Base.deliveries.last.to).to eq [master.notification_email]
-    expect(ActionMailer::Base.deliveries.last.subject).to match /Request to join #{project.name_with_namespace} project/
+    expect(ActionMailer::Base.deliveries.last.subject).to eq "Request to join the #{project.name_with_namespace} project"
 
-    expect(project.access_requested?(user)).to be_truthy
+    expect(project.members.request.exists?(user_id: user)).to be_truthy
     expect(page).to have_content 'Your request for access has been queued for review.'
-    expect(page).to have_content 'Withdraw Request'
+
+    expect(page).to have_content 'Withdraw Access Request'
   end
 
   scenario 'user is not listed in the project members page' do
-    visit namespace_project_path(project.namespace, project)
-
     click_link 'Request Access'
 
-    expect(project.access_requested?(user)).to be_truthy
+    expect(project.members.request.exists?(user_id: user)).to be_truthy
 
+    open_project_settings_menu
     click_link 'Members'
 
     visit namespace_project_project_members_path(project.namespace, project)
@@ -41,14 +38,17 @@ feature 'Projects > Members > User requests access', feature: true do
   end
 
   scenario 'user can withdraw its request for access' do
-    visit namespace_project_path(project.namespace, project)
     click_link 'Request Access'
 
-    expect(project.access_requested?(user)).to be_truthy
+    expect(project.members.request.exists?(user_id: user)).to be_truthy
 
-    click_link 'Withdraw Request'
+    click_link 'Withdraw Access Request'
+
+    expect(project.members.request.exists?(user_id: user)).to be_falsey
+    expect(page).to have_content 'Your access request to the project has been withdrawn.'
+  end
 
-    expect(project.access_requested?(user)).to be_falsey
-    expect(page).to have_content 'You withdrawn your access request to the project.'
+  def open_project_settings_menu
+    find('#project-settings-button').click
   end
 end
diff --git a/spec/helpers/gitlab_routing_helper_spec.rb b/spec/helpers/gitlab_routing_helper_spec.rb
new file mode 100644
index 00000000000..14847d0a49e
--- /dev/null
+++ b/spec/helpers/gitlab_routing_helper_spec.rb
@@ -0,0 +1,79 @@
+require 'spec_helper'
+
+describe GitlabRoutingHelper do
+  describe 'Project URL helpers' do
+    describe '#project_members_url' do
+      let(:project) { build_stubbed(:empty_project) }
+
+      it { expect(project_members_url(project)).to eq namespace_project_project_members_url(project.namespace, project) }
+    end
+
+    describe '#project_member_path' do
+      let(:project_member) { create(:project_member) }
+
+      it { expect(project_member_path(project_member)).to eq namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
+    end
+
+    describe '#request_access_project_members_path' do
+      let(:project) { build_stubbed(:empty_project) }
+
+      it { expect(request_access_project_members_path(project)).to eq request_access_namespace_project_project_members_path(project.namespace, project) }
+    end
+
+    describe '#leave_project_members_path' do
+      let(:project) { build_stubbed(:empty_project) }
+
+      it { expect(leave_project_members_path(project)).to eq leave_namespace_project_project_members_path(project.namespace, project) }
+    end
+
+    describe '#approve_access_request_project_member_path' do
+      let(:project_member) { create(:project_member) }
+
+      it { expect(approve_access_request_project_member_path(project_member)).to eq approve_access_request_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
+    end
+
+    describe '#resend_invite_project_member_path' do
+      let(:project_member) { create(:project_member) }
+
+      it { expect(resend_invite_project_member_path(project_member)).to eq resend_invite_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
+    end
+  end
+
+  describe 'Group URL helpers' do
+    describe '#group_members_url' do
+      let(:group) { build_stubbed(:group) }
+
+      it { expect(group_members_url(group)).to eq group_group_members_url(group) }
+    end
+
+    describe '#group_member_path' do
+      let(:group_member) { create(:group_member) }
+
+      it { expect(group_member_path(group_member)).to eq group_group_member_path(group_member.source, group_member) }
+    end
+
+    describe '#request_access_group_members_path' do
+      let(:group) { build_stubbed(:group) }
+
+      it { expect(request_access_group_members_path(group)).to eq request_access_group_group_members_path(group) }
+    end
+
+    describe '#leave_group_members_path' do
+      let(:group) { build_stubbed(:group) }
+
+      it { expect(leave_group_members_path(group)).to eq leave_group_group_members_path(group) }
+    end
+
+    describe '#approve_access_request_group_member_path' do
+      let(:group_member) { create(:group_member) }
+
+      it { expect(approve_access_request_group_member_path(group_member)).to eq approve_access_request_group_group_member_path(group_member.source, group_member) }
+    end
+
+    describe '#resend_invite_group_member_path' do
+      let(:group_member) { create(:group_member) }
+
+      it { expect(resend_invite_group_member_path(group_member)).to eq resend_invite_group_group_member_path(group_member.source, group_member) }
+    end
+  end
+end
diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb
index c2f10e1db75..0b1a76156e0 100644
--- a/spec/helpers/members_helper_spec.rb
+++ b/spec/helpers/members_helper_spec.rb
@@ -1,22 +1,6 @@
 require 'spec_helper'
 
 describe MembersHelper do
-  describe '#member_class' do
-    let(:project_member) { build(:project_member) }
-    let(:group_member) { build(:group_member) }
-
-    it { expect(member_class(project_member)).to eq ProjectMember }
-    it { expect(member_class(group_member)).to eq GroupMember }
-  end
-
-  describe '#members_association' do
-    let(:project) { build_stubbed(:project) }
-    let(:group) { build_stubbed(:group) }
-
-    it { expect(members_association(project)).to eq :project_members }
-    it { expect(members_association(group)).to eq :group_members }
-  end
-
   describe '#action_member_permission' do
     let(:project_member) { build(:project_member) }
     let(:group_member) { build(:group_member) }
@@ -25,73 +9,20 @@ describe MembersHelper do
     it { expect(action_member_permission(:admin, group_member)).to eq :admin_group_member }
   end
 
-  describe '#can_see_entity_roles?' do
-    let(:project) { create(:project) }
+  describe '#can_see_member_roles?' do
+    let(:project) { create(:empty_project) }
     let(:group) { create(:group) }
     let(:user) { build(:user) }
     let(:admin) { build(:user, :admin) }
     let(:project_member) { create(:project_member, project: project) }
     let(:group_member) { create(:group_member, group: group) }
 
-    it { expect(can_see_entity_roles?(nil, project)).to be_falsy }
-    it { expect(can_see_entity_roles?(nil, group)).to be_falsy }
-    it { expect(can_see_entity_roles?(admin, project)).to be_truthy }
-    it { expect(can_see_entity_roles?(admin, group)).to be_truthy }
-    it { expect(can_see_entity_roles?(project_member.user, project)).to be_truthy }
-    it { expect(can_see_entity_roles?(group_member.user, group)).to be_truthy }
-  end
-
-  describe '#member_path' do
-    let(:project_member) { create(:project_member) }
-    let(:group_member) { create(:group_member) }
-
-    it { expect(member_path(project_member)).to eq namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
-    it { expect(member_path(group_member)).to eq group_group_member_path(group_member.source, group_member) }
-    it { expect { member_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' }
-  end
-
-  describe '#resend_invite_member_path' do
-    let(:project_member) { create(:project_member) }
-    let(:group_member) { create(:group_member) }
-
-    it { expect(resend_invite_member_path(project_member)).to eq resend_invite_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
-    it { expect(resend_invite_member_path(group_member)).to eq resend_invite_group_group_member_path(group_member.source, group_member) }
-    it { expect { resend_invite_member_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' }
-  end
-
-  describe '#request_access_path' do
-    let(:project) { build_stubbed(:project) }
-    let(:group) { build_stubbed(:group) }
-
-    it { expect(request_access_path(project)).to eq request_access_namespace_project_project_members_path(project.namespace, project) }
-    it { expect(request_access_path(group)).to eq request_access_group_group_members_path(group) }
-    it { expect { request_access_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' }
-  end
-
-  describe '#approve_request_member_path' do
-    let(:project_member) { create(:project_member) }
-    let(:group_member) { create(:group_member) }
-
-    it { expect(approve_request_member_path(project_member)).to eq approve_access_request_namespace_project_project_member_path(project_member.source.namespace, project_member.source, project_member) }
-    it { expect(approve_request_member_path(group_member)).to eq approve_access_request_group_group_member_path(group_member.source, group_member) }
-    it { expect { approve_request_member_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' }
-  end
-
-  describe '#leave_path' do
-    let(:project) { build_stubbed(:project) }
-    let(:group) { build_stubbed(:group) }
-
-    it { expect(leave_path(project)).to eq leave_namespace_project_project_members_path(project.namespace, project) }
-    it { expect(leave_path(group)).to eq leave_group_group_members_path(group) }
-    it { expect { leave_path(double(:member, source: 'foo')) }.to raise_error ArgumentError, 'Unknown object class' }
-  end
-
-  describe '#withdraw_request_message' do
-    let(:project) { build_stubbed(:project) }
-    let(:group) { build_stubbed(:group) }
-
-    it { expect(withdraw_request_message(project)).to eq "Are you sure you want to withdraw your access request for the \"#{project.name_with_namespace}\" project?" }
-    it { expect(withdraw_request_message(group)).to eq "Are you sure you want to withdraw your access request for the \"#{group.name}\" group?" }
+    it { expect(can_see_member_roles?(source: project, user: nil)).to be_falsy }
+    it { expect(can_see_member_roles?(source: group, user: nil)).to be_falsy }
+    it { expect(can_see_member_roles?(source: project, user: admin)).to be_truthy }
+    it { expect(can_see_member_roles?(source: group, user: admin)).to be_truthy }
+    it { expect(can_see_member_roles?(source: project, user: project_member.user)).to be_truthy }
+    it { expect(can_see_member_roles?(source: group, user: group_member.user)).to be_truthy }
   end
 
   describe '#remove_member_message' do
@@ -105,12 +36,14 @@ describe MembersHelper do
     let(:group_member_invite) { build(:group_member, group: group).tap { |m| m.generate_invite_token! } }
     let(:group_member_request) { group.request_access(requester) }
 
-    it { expect(remove_member_message(project_member)).to eq "You are going to remove #{project_member.user.name} from the #{project.name_with_namespace} project. Are you sure?" }
-    it { expect(remove_member_message(project_member_invite)).to eq "You are going to revoke the invitation for #{project_member_invite.invite_email} to join the #{project.name_with_namespace} project. Are you sure?" }
-    it { expect(remove_member_message(project_member_request)).to eq "You are going to deny #{requester.name}'s request to join the #{project.name_with_namespace} project. Are you sure?" }
-    it { expect(remove_member_message(group_member)).to eq "You are going to remove #{group_member.user.name} from the #{group.name} group. Are you sure?" }
-    it { expect(remove_member_message(group_member_invite)).to eq "You are going to revoke the invitation for #{group_member_invite.invite_email} to join the #{group.name} group. Are you sure?" }
-    it { expect(remove_member_message(group_member_request)).to eq "You are going to deny #{requester.name}'s request to join the #{group.name} group. Are you sure?" }
+    it { expect(remove_member_message(project_member)).to eq "Are you sure you want to remove #{project_member.user.name} from the #{project.name_with_namespace} project?" }
+    it { expect(remove_member_message(project_member_invite)).to eq "Are you sure you want to revoke the invitation for #{project_member_invite.invite_email} to join the #{project.name_with_namespace} project?" }
+    it { expect(remove_member_message(project_member_request)).to eq "Are you sure you want to deny #{requester.name}'s request to join the #{project.name_with_namespace} project?" }
+    it { expect(remove_member_message(project_member_request, user: requester)).to eq "Are you sure you want to withdraw your access request for the #{project.name_with_namespace} project?" }
+    it { expect(remove_member_message(group_member)).to eq "Are you sure you want to remove #{group_member.user.name} from the #{group.name} group?" }
+    it { expect(remove_member_message(group_member_invite)).to eq "Are you sure you want to revoke the invitation for #{group_member_invite.invite_email} to join the #{group.name} group?" }
+    it { expect(remove_member_message(group_member_request)).to eq "Are you sure you want to deny #{requester.name}'s request to join the #{group.name} group?" }
+    it { expect(remove_member_message(group_member_request, user: requester)).to eq "Are you sure you want to withdraw your access request for the #{group.name} group?" }
   end
 
   describe '#remove_member_title' do
@@ -122,10 +55,10 @@ describe MembersHelper do
     let(:group_member) { build(:group_member, group: group) }
     let(:group_member_request) { group.request_access(requester) }
 
-    it { expect(remove_member_title(project_member)).to eq 'Remove user' }
-    it { expect(remove_member_title(project_member_request)).to eq 'Deny access request' }
-    it { expect(remove_member_title(group_member)).to eq 'Remove user' }
-    it { expect(remove_member_title(group_member_request)).to eq 'Deny access request' }
+    it { expect(remove_member_title(project_member)).to eq 'Remove user from project' }
+    it { expect(remove_member_title(project_member_request)).to eq 'Deny access request from project' }
+    it { expect(remove_member_title(group_member)).to eq 'Remove user from group' }
+    it { expect(remove_member_title(group_member_request)).to eq 'Deny access request from group' }
   end
 
   describe '#leave_confirmation_message' do
@@ -133,7 +66,7 @@ describe MembersHelper do
     let(:group) { build_stubbed(:group) }
     let(:user) { build_stubbed(:user) }
 
-    it { expect(leave_confirmation_message(project)).to eq "Are you sure you want to leave \"#{project.name_with_namespace}\" project?" }
-    it { expect(leave_confirmation_message(group)).to eq "Are you sure you want to leave \"#{group.name}\" group?" }
+    it { expect(leave_confirmation_message(project)).to eq "Are you sure you want to leave the \"#{project.name_with_namespace}\" project?" }
+    it { expect(leave_confirmation_message(group)).to eq "Are you sure you want to leave the \"#{group.name}\" group?" }
   end
 end
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index fa81c28849e..09e0bbfd00b 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -1,25 +1,6 @@
 require 'spec_helper'
 
 describe ProjectsHelper do
-  describe '#max_access_level' do
-    let(:master) { create(:user) }
-    let(:owner) { create(:user) }
-    let(:reporter) { create(:user) }
-    let(:group) { create(:group) }
-    let(:project) { build_stubbed(:empty_project, namespace: group) }
-
-    before do
-      group.add_master(master)
-      group.add_owner(owner)
-      group.add_reporter(reporter)
-    end
-
-    it { expect(max_access_level(project, master)).to eq 'Master' }
-    it { expect(max_access_level(project, owner)).to eq 'Owner' }
-    it { expect(max_access_level(project, reporter)).to eq 'Reporter' }
-    it { expect(max_access_level(project, build_stubbed(:user))).to be_nil }
-  end
-
   describe "#project_status_css_class" do
     it "returns appropriate class" do
       expect(project_status_css_class("started")).to eq("active")
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index a86ec865b5d..1e6eb20ab39 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -405,7 +405,7 @@ describe Notify do
       let(:user) { create(:user) }
       let(:project_member) do
         project.request_access(user)
-        project.project_members.find_by(created_by_id: user.id)
+        project.members.request.find_by(user_id: user.id)
       end
       subject { Notify.member_access_requested_email('project', project_member.id) }
 
@@ -413,10 +413,12 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject "Request to join the #{project.name_with_namespace} project" }
-      it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
-      it { is_expected.to have_body_text /#{project.web_url}/ }
-      it { is_expected.to have_body_text /#{project_member.human_access}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject "Request to join the #{project.name_with_namespace} project"
+        is_expected.to have_body_text /#{project.name_with_namespace}/
+        is_expected.to have_body_text /#{namespace_project_project_members_url(project.namespace, project)}/
+        is_expected.to have_body_text /#{project_member.human_access}/
+      end
     end
 
     describe 'project access denied' do
@@ -424,7 +426,7 @@ describe Notify do
       let(:user) { create(:user) }
       let(:project_member) do
         project.request_access(user)
-        project.project_members.find_by(created_by_id: user.id)
+        project.members.request.find_by(user_id: user.id)
       end
       subject { Notify.member_access_denied_email('project', project.id, user.id) }
 
@@ -432,9 +434,11 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject "Access to the #{project.name_with_namespace} project was denied" }
-      it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
-      it { is_expected.to have_body_text /#{project.web_url}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject "Access to the #{project.name_with_namespace} project was denied"
+        is_expected.to have_body_text /#{project.name_with_namespace}/
+        is_expected.to have_body_text /#{project.web_url}/
+      end
     end
 
     describe 'project access changed' do
@@ -447,10 +451,12 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject "Access to the #{project.name_with_namespace} project was granted" }
-      it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
-      it { is_expected.to have_body_text /#{project.web_url}/ }
-      it { is_expected.to have_body_text /#{project_member.human_access}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject "Access to the #{project.name_with_namespace} project was granted"
+        is_expected.to have_body_text /#{project.name_with_namespace}/
+        is_expected.to have_body_text /#{project.web_url}/
+        is_expected.to have_body_text /#{project_member.human_access}/
+      end
     end
 
     def invite_to_project(project:, email:, inviter:)
@@ -470,11 +476,13 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject "Invitation to join the #{project.name_with_namespace} project" }
-      it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
-      it { is_expected.to have_body_text /#{project.web_url}/ }
-      it { is_expected.to have_body_text /#{project_member.human_access}/ }
-      it { is_expected.to have_body_text /#{project_member.invite_token}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject "Invitation to join the #{project.name_with_namespace} project"
+        is_expected.to have_body_text /#{project.name_with_namespace}/
+        is_expected.to have_body_text /#{project.web_url}/
+        is_expected.to have_body_text /#{project_member.human_access}/
+        is_expected.to have_body_text /#{project_member.invite_token}/
+      end
     end
 
     describe 'project invitation accepted' do
@@ -493,11 +501,13 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject 'Invitation accepted' }
-      it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
-      it { is_expected.to have_body_text /#{project.web_url}/ }
-      it { is_expected.to have_body_text /#{project_member.invite_email}/ }
-      it { is_expected.to have_body_text /#{invited_user.name}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject 'Invitation accepted'
+        is_expected.to have_body_text /#{project.name_with_namespace}/
+        is_expected.to have_body_text /#{project.web_url}/
+        is_expected.to have_body_text /#{project_member.invite_email}/
+        is_expected.to have_body_text /#{invited_user.name}/
+      end
     end
 
     describe 'project invitation declined' do
@@ -515,10 +525,12 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject 'Invitation declined' }
-      it { is_expected.to have_body_text /#{project.name_with_namespace}/ }
-      it { is_expected.to have_body_text /#{project.web_url}/ }
-      it { is_expected.to have_body_text /#{project_member.invite_email}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject 'Invitation declined'
+        is_expected.to have_body_text /#{project.name_with_namespace}/
+        is_expected.to have_body_text /#{project.web_url}/
+        is_expected.to have_body_text /#{project_member.invite_email}/
+      end
     end
 
     context 'items that are noteable, the email for a note' do
@@ -639,7 +651,7 @@ describe Notify do
       let(:user) { create(:user) }
       let(:group_member) do
         group.request_access(user)
-        group.group_members.find_by(created_by_id: user.id)
+        group.members.request.find_by(user_id: user.id)
       end
       subject { Notify.member_access_requested_email('group', group_member.id) }
 
@@ -647,10 +659,12 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject "Request to join the #{group.name} group" }
-      it { is_expected.to have_body_text /#{group.name}/ }
-      it { is_expected.to have_body_text /#{group.web_url}/ }
-      it { is_expected.to have_body_text /#{group_member.human_access}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject "Request to join the #{group.name} group"
+        is_expected.to have_body_text /#{group.name}/
+        is_expected.to have_body_text /#{group_group_members_url(group)}/
+        is_expected.to have_body_text /#{group_member.human_access}/
+      end
     end
 
     describe 'group access denied' do
@@ -658,7 +672,7 @@ describe Notify do
       let(:user) { create(:user) }
       let(:group_member) do
         group.request_access(user)
-        group.group_members.find_by(created_by_id: user.id)
+        group.members.request.find_by(user_id: user.id)
       end
       subject { Notify.member_access_denied_email('group', group.id, user.id) }
 
@@ -666,9 +680,11 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject "Access to the #{group.name} group was denied" }
-      it { is_expected.to have_body_text /#{group.name}/ }
-      it { is_expected.to have_body_text /#{group.web_url}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject "Access to the #{group.name} group was denied"
+        is_expected.to have_body_text /#{group.name}/
+        is_expected.to have_body_text /#{group.web_url}/
+      end
     end
 
     describe 'group access changed' do
@@ -682,10 +698,12 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject "Access to the #{group.name} group was granted" }
-      it { is_expected.to have_body_text /#{group.name}/ }
-      it { is_expected.to have_body_text /#{group.web_url}/ }
-      it { is_expected.to have_body_text /#{group_member.human_access}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject "Access to the #{group.name} group was granted"
+        is_expected.to have_body_text /#{group.name}/
+        is_expected.to have_body_text /#{group.web_url}/
+        is_expected.to have_body_text /#{group_member.human_access}/
+      end
     end
 
     def invite_to_group(group:, email:, inviter:)
@@ -705,11 +723,13 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject "Invitation to join the #{group.name} group" }
-      it { is_expected.to have_body_text /#{group.name}/ }
-      it { is_expected.to have_body_text /#{group.web_url}/ }
-      it { is_expected.to have_body_text /#{group_member.human_access}/ }
-      it { is_expected.to have_body_text /#{group_member.invite_token}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject "Invitation to join the #{group.name} group"
+        is_expected.to have_body_text /#{group.name}/
+        is_expected.to have_body_text /#{group.web_url}/
+        is_expected.to have_body_text /#{group_member.human_access}/
+        is_expected.to have_body_text /#{group_member.invite_token}/
+      end
     end
 
     describe 'group invitation accepted' do
@@ -728,11 +748,13 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject 'Invitation accepted' }
-      it { is_expected.to have_body_text /#{group.name}/ }
-      it { is_expected.to have_body_text /#{group.web_url}/ }
-      it { is_expected.to have_body_text /#{group_member.invite_email}/ }
-      it { is_expected.to have_body_text /#{invited_user.name}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject 'Invitation accepted'
+        is_expected.to have_body_text /#{group.name}/
+        is_expected.to have_body_text /#{group.web_url}/
+        is_expected.to have_body_text /#{group_member.invite_email}/
+        is_expected.to have_body_text /#{invited_user.name}/
+      end
     end
 
     describe 'group invitation declined' do
@@ -750,10 +772,12 @@ describe Notify do
       it_behaves_like 'it should not have Gmail Actions links'
       it_behaves_like "a user cannot unsubscribe through footer link"
 
-      it { is_expected.to have_subject 'Invitation declined' }
-      it { is_expected.to have_body_text /#{group.name}/ }
-      it { is_expected.to have_body_text /#{group.web_url}/ }
-      it { is_expected.to have_body_text /#{group_member.invite_email}/ }
+      it 'contains all the useful information' do
+        is_expected.to have_subject 'Invitation declined'
+        is_expected.to have_body_text /#{group.name}/
+        is_expected.to have_body_text /#{group.web_url}/
+        is_expected.to have_body_text /#{group_member.invite_email}/
+      end
     end
   end
 
diff --git a/spec/models/concerns/access_requestable_spec.rb b/spec/models/concerns/access_requestable_spec.rb
index 2dfed1eb4c4..98307876962 100644
--- a/spec/models/concerns/access_requestable_spec.rb
+++ b/spec/models/concerns/access_requestable_spec.rb
@@ -7,8 +7,7 @@ describe AccessRequestable do
       let(:user) { create(:user) }
 
       it { expect(group.request_access(user)).to be_a(GroupMember) }
-      it { expect(group.request_access(user).user).to be_nil }
-      it { expect(group.request_access(user).created_by).to eq(user) }
+      it { expect(group.request_access(user).user).to eq(user) }
     end
 
     describe '#access_requested?' do
@@ -17,7 +16,7 @@ describe AccessRequestable do
 
       before { group.request_access(user) }
 
-      it { expect(group.access_requested?(user)).to be_truthy }
+      it { expect(group.members.request.exists?(user_id: user)).to be_truthy }
     end
   end
 
@@ -35,7 +34,7 @@ describe AccessRequestable do
 
       before { project.request_access(user) }
 
-      it { expect(project.access_requested?(user)).to be_truthy }
+      it { expect(project.members.request.exists?(user_id: user)).to be_truthy }
     end
   end
 end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 52f9d57bc0a..ccdcb29f773 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -10,17 +10,6 @@ describe Group, models: true do
     it { is_expected.to have_many(:project_group_links).dependent(:destroy) }
     it { is_expected.to have_many(:shared_projects).through(:project_group_links) }
     it { is_expected.to have_many(:notification_settings).dependent(:destroy) }
-
-    describe '#group_members' do
-      let(:user) { create(:user) }
-      let(:group) { create(:group) }
-
-      before { group.request_access(user) }
-
-      it 'does not includes membership requests' do
-        expect(user.group_members).to be_empty
-      end
-    end
   end
 
   describe 'modules' do
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index a3d525d8d56..3ed3202ac6c 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -55,45 +55,78 @@ describe Member, models: true do
     end
   end
 
-  describe 'Scopes' do
+  describe 'Scopes & finders' do
     before do
       project = create(:project)
-      @invited_member = build(:project_member, user: nil).tap { |m| m.generate_invite_token! }
-      @accepted_invite_member = build(:project_member, user: nil).tap { |m| m.generate_invite_token! && m.accept_invite!(build(:user)) }
+      group = create(:group)
+      @owner_user = create(:user).tap { |u| group.add_owner(u) }
+      @owner = group.members.find_by(user_id: @owner_user.id)
+
+      @master_user = create(:user).tap { |u| project.team << [u, :master] }
+      @master = project.members.find_by(user_id: @master_user.id)
+
+      ProjectMember.add_user(project.members, 'toto1@example.com', Gitlab::Access::DEVELOPER, @master_user)
+      @invited_member = project.members.invite.find_by_invite_email('toto1@example.com')
+
+      accepted_invite_user = build(:user)
+      ProjectMember.add_user(project.members, 'toto2@example.com', Gitlab::Access::DEVELOPER, @master_user)
+      @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) }
 
       requested_user = create(:user).tap { |u| project.request_access(u) }
-      @requested_member = project.project_members.find_by(created_by_id: requested_user.id)
+      @requested_member = project.members.request.find_by(user_id: requested_user.id)
+
       accepted_request_user = create(:user).tap { |u| project.request_access(u) }
-      @accepted_request_member = project.project_members.find_by(created_by_id: accepted_request_user.id).tap { |m| m.accept_request }
+      @accepted_request_member = project.members.request.find_by(user_id: accepted_request_user.id).tap { |m| m.accept_request }
     end
 
-    describe '#invite' do
+    describe '.invite' do
+      it { expect(described_class.invite).not_to include @master }
       it { expect(described_class.invite).to include @invited_member }
       it { expect(described_class.invite).not_to include @accepted_invite_member }
       it { expect(described_class.invite).not_to include @requested_member }
       it { expect(described_class.invite).not_to include @accepted_request_member }
     end
 
-    describe '#request' do
+    describe '.non_invite' do
+      it { expect(described_class.non_invite).to include @master }
+      it { expect(described_class.non_invite).not_to include @invited_member }
+      it { expect(described_class.non_invite).to include @accepted_invite_member }
+      it { expect(described_class.non_invite).to include @requested_member }
+      it { expect(described_class.non_invite).to include @accepted_request_member }
+    end
+
+    describe '.request' do
+      it { expect(described_class.request).not_to include @master }
       it { expect(described_class.request).not_to include @invited_member }
       it { expect(described_class.request).not_to include @accepted_invite_member }
       it { expect(described_class.request).to include @requested_member }
       it { expect(described_class.request).not_to include @accepted_request_member }
     end
 
-    describe '#non_request' do
+    describe '.non_request' do
+      it { expect(described_class.non_request).to include @master }
       it { expect(described_class.non_request).to include @invited_member }
       it { expect(described_class.non_request).to include @accepted_invite_member }
       it { expect(described_class.non_request).not_to include @requested_member }
       it { expect(described_class.non_request).to include @accepted_request_member }
     end
 
-    describe '#non_pending' do
+    describe '.non_pending' do
+      it { expect(described_class.non_pending).to include @master }
       it { expect(described_class.non_pending).not_to include @invited_member }
       it { expect(described_class.non_pending).to include @accepted_invite_member }
       it { expect(described_class.non_pending).not_to include @requested_member }
       it { expect(described_class.non_pending).to include @accepted_request_member }
     end
+
+    describe '.owners_and_masters' do
+      it { expect(described_class.owners_and_masters).to include @owner }
+      it { expect(described_class.owners_and_masters).to include @master }
+      it { expect(described_class.owners_and_masters).not_to include @invited_member }
+      it { expect(described_class.owners_and_masters).not_to include @accepted_invite_member }
+      it { expect(described_class.owners_and_masters).not_to include @requested_member }
+      it { expect(described_class.owners_and_masters).not_to include @accepted_request_member }
+    end
   end
 
   describe "Delegate methods" do
@@ -101,6 +134,18 @@ describe Member, models: true do
     it { is_expected.to respond_to(:user_email) }
   end
 
+  describe 'Callbacks' do
+    describe 'after_destroy :post_decline_request, if: :request?' do
+      let(:member) { create(:project_member, requested_at: Time.now.utc) }
+
+      it 'calls #post_decline_request' do
+        expect(member).to receive(:post_decline_request)
+
+        member.destroy
+      end
+    end
+  end
+
   describe ".add_user" do
     let!(:user)    { create(:user) }
     let(:project) { create(:project) }
@@ -139,18 +184,9 @@ describe Member, models: true do
   end
 
   describe '#accept_request' do
-    let(:user) { create(:user) }
-    let(:member) { create(:project_member, requested_at: Time.now.utc, user: nil, created_by: user) }
-
-    it 'returns true' do
-      expect(member.accept_request).to be_truthy
-    end
+    let(:member) { create(:project_member, requested_at: Time.now.utc) }
 
-    it 'sets the user' do
-      member.accept_request
-
-      expect(member.user).to eq(user)
-    end
+    it { expect(member.accept_request).to be_truthy }
 
     it 'clears requested_at' do
       member.accept_request
@@ -165,25 +201,24 @@ describe Member, models: true do
     end
   end
 
-  describe '#decline_request' do
-    let(:user) { create(:user) }
-    let(:member) { create(:project_member, requested_at: Time.now.utc, user: nil, created_by: user) }
+  describe '#invite?' do
+    subject { create(:project_member, invite_email: "user@example.com", user: nil) }
 
-    it 'returns true' do
-      expect(member.decline_request).to be_truthy
-    end
+    it { is_expected.to be_invite }
+  end
 
-    it 'destroys the member' do
-      member.decline_request
+  describe '#request?' do
+    subject { create(:project_member, requested_at: Time.now.utc) }
 
-      expect(member).to be_destroyed
-    end
+    it { is_expected.to be_request }
+  end
 
-    it 'calls #after_decline_request' do
-      expect(member).to receive(:after_decline_request)
+  describe '#pending?' do
+    let(:invited_member) { create(:project_member, invite_email: "user@example.com", user: nil) }
+    let(:requester) { create(:project_member, requested_at: Time.now.utc) }
 
-      member.decline_request
-    end
+    it { expect(invited_member).to be_invite }
+    it { expect(requester).to be_pending }
   end
 
   describe "#accept_invite!" do
diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb
index c3070d4cb78..eeb74a462ac 100644
--- a/spec/models/members/group_member_spec.rb
+++ b/spec/models/members/group_member_spec.rb
@@ -51,24 +51,30 @@ describe GroupMember, models: true do
       end
     end
 
-    describe 'after accept_request' do
-      let(:member) { create(:group_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) }
+    describe '#after_accept_request' do
+      it 'calls NotificationService.accept_group_access_request' do
+        member = create(:group_member, user: build_stubbed(:user), requested_at: Time.now)
 
-      it "calls #accept_group_access_request" do
         expect_any_instance_of(NotificationService).to receive(:new_group_member)
 
-        member.accept_request
+        member.__send__(:after_accept_request)
       end
     end
 
-    describe 'after decline_request' do
-      let(:member) { create(:group_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) }
+    describe '#post_decline_request' do
+      it 'calls NotificationService.decline_group_access_request' do
+        member = create(:group_member, user: build_stubbed(:user), requested_at: Time.now)
 
-      it "calls #decline_group_access_request" do
         expect_any_instance_of(NotificationService).to receive(:decline_group_access_request)
 
-        member.decline_request
+        member.__send__(:post_decline_request)
       end
     end
+
+    describe '#real_source_type' do
+      subject { create(:group_member).real_source_type }
+
+      it { is_expected.to eq 'Group' }
+    end
   end
 end
diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb
index 99b3c77c6cd..1e466f9c620 100644
--- a/spec/models/members/project_member_spec.rb
+++ b/spec/models/members/project_member_spec.rb
@@ -33,6 +33,12 @@ describe ProjectMember, models: true do
     it { is_expected.to include_module(Gitlab::ShellAdapter) }
   end
 
+  describe '#real_source_type' do
+    subject { create(:project_member).real_source_type }
+
+    it { is_expected.to eq 'Project' }
+  end
+
   describe "#destroy" do
     let(:owner)   { create(:project_member, access_level: ProjectMember::OWNER) }
     let(:project) { owner.project }
@@ -137,23 +143,23 @@ describe ProjectMember, models: true do
   end
 
   describe 'notifications' do
-    describe 'after accept_request' do
-      let(:member) { create(:project_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) }
+    describe '#after_accept_request' do
+      it 'calls NotificationService.new_project_member' do
+        member = create(:project_member, user: build_stubbed(:user), requested_at: Time.now)
 
-      it 'calls #accept_project_access_request' do
         expect_any_instance_of(NotificationService).to receive(:new_project_member)
 
-        member.accept_request
+        member.__send__(:after_accept_request)
       end
     end
 
-    describe 'after decline_request' do
-      let(:member) { create(:project_member, user: nil, created_by: build_stubbed(:user), requested_at: Time.now) }
+    describe '#post_decline_request' do
+      it 'calls NotificationService.decline_project_access_request' do
+        member = create(:project_member, user: build_stubbed(:user), requested_at: Time.now)
 
-      it 'calls #decline_project_access_request' do
         expect_any_instance_of(NotificationService).to receive(:decline_project_access_request)
 
-        member.decline_request
+        member.__send__(:post_decline_request)
       end
     end
   end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index d5a4b73affd..30aa2b70c8d 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -29,17 +29,6 @@ describe Project, models: true do
     it { is_expected.to have_many(:variables) }
     it { is_expected.to have_many(:triggers) }
     it { is_expected.to have_many(:todos).dependent(:destroy) }
-
-    describe '#project_members' do
-      let(:user) { create(:user) }
-      let(:project) { create(:project) }
-
-      before { project.request_access(user) }
-
-      it 'does not includes membership requests' do
-        expect(user.project_members).to be_empty
-      end
-    end
   end
 
   describe 'modules' do
@@ -100,11 +89,17 @@ describe Project, models: true do
     it { is_expected.to respond_to(:repo_exists?) }
     it { is_expected.to respond_to(:update_merge_requests) }
     it { is_expected.to respond_to(:execute_hooks) }
-    it { is_expected.to respond_to(:name_with_namespace) }
     it { is_expected.to respond_to(:owner) }
     it { is_expected.to respond_to(:path_with_namespace) }
   end
 
+  describe '#name_with_namespace' do
+    let(:project) { build_stubbed(:empty_project) }
+
+    it { expect(project.name_with_namespace).to eq "#{project.namespace.human_name} / #{project.name}" }
+    it { expect(project.human_name).to eq project.name_with_namespace }
+  end
+
   describe '#to_reference' do
     let(:project) { create(:empty_project) }
 
diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb
index 36b1f439955..9262aeb6ed8 100644
--- a/spec/models/project_team_spec.rb
+++ b/spec/models/project_team_spec.rb
@@ -112,6 +112,28 @@ describe ProjectTeam, models: true do
     end
   end
 
+  describe "#human_max_access" do
+    it 'returns Master role' do
+      user = create(:user)
+      group = create(:group)
+      group.add_master(user)
+
+      project = build_stubbed(:empty_project, namespace: group)
+
+      expect(project.team.human_max_access(user.id)).to eq 'Master'
+    end
+
+    it 'returns Owner role' do
+      user = create(:user)
+      group = create(:group)
+      group.add_owner(user)
+
+      project = build_stubbed(:empty_project, namespace: group)
+
+      expect(project.team.human_max_access(user.id)).to eq 'Owner'
+    end
+  end
+
   describe '#max_member_access' do
     let(:requester) { create(:user) }