Address Douwe's feedbackfix-18717

- Make it more explicit that `source` must respond to `#group` and that
`#group` must be present.
- Indent subsequent lines of a multi-line condition.

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

4 files changed, 53 insertions, 20 deletions

diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb
index 3897cfa2e10..ec106418f2d 100644
--- a/app/helpers/members_helper.rb
+++ b/app/helpers/members_helper.rb
@@ -8,8 +8,8 @@ module MembersHelper
 
   def default_show_roles(member)
     can?(current_user, action_member_permission(:update, member), member) ||
-    can?(current_user, action_member_permission(:destroy, member), member) ||
-    can?(current_user, action_member_permission(:admin, member), member.source)
+      can?(current_user, action_member_permission(:destroy, member), member) ||
+      can?(current_user, action_member_permission(:admin, member), member.source)
   end
 
   def remove_member_message(member, user: nil)
diff --git a/app/views/shared/members/_access_request_buttons.html.haml b/app/views/shared/members/_access_request_buttons.html.haml
index e0fc9f7af37..480e8ba6c85 100644
--- a/app/views/shared/members/_access_request_buttons.html.haml
+++ b/app/views/shared/members/_access_request_buttons.html.haml
@@ -1,5 +1,5 @@
 - member = source.members.find_by(user_id: current_user.id)
-- group_member = source.group.members.find_by(user_id: current_user.id) if source.try(:group)
+- group_member = source.group.members.find_by(user_id: current_user.id) if source.respond_to?(:group) && source.group
 
 - unless group_member
   - if member
diff --git a/spec/features/projects/members/group_member_cannot_request_access_to_his_group_project_spec.rb b/spec/features/projects/members/group_member_cannot_request_access_to_his_group_project_spec.rb
new file mode 100644
index 00000000000..4d5d656f00c
--- /dev/null
+++ b/spec/features/projects/members/group_member_cannot_request_access_to_his_group_project_spec.rb
@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+feature 'Projects > Members > Group member cannot request access to his group project', feature: true do
+  let(:user) { create(:user) }
+  let(:group) { create(:group) }
+  let(:project) { create(:project, namespace: group) }
+
+  background do
+  end
+
+  scenario 'owner does not see the request access button' do
+    group.add_owner(user)
+    login_and_visit_project_page(user)
+
+    expect(page).not_to have_content 'Request Access'
+  end
+
+  scenario 'master does not see the request access button' do
+    group.add_master(user)
+    login_and_visit_project_page(user)
+
+    expect(page).not_to have_content 'Request Access'
+  end
+
+  scenario 'developer does not see the request access button' do
+    group.add_developer(user)
+    login_and_visit_project_page(user)
+
+    expect(page).not_to have_content 'Request Access'
+  end
+
+  scenario 'reporter does not see the request access button' do
+    group.add_reporter(user)
+    login_and_visit_project_page(user)
+
+    expect(page).not_to have_content 'Request Access'
+  end
+
+  scenario 'guest does not see the request access button' do
+    group.add_guest(user)
+    login_and_visit_project_page(user)
+
+    expect(page).not_to have_content 'Request Access'
+  end
+
+  def login_and_visit_project_page(user)
+    login_as(user)
+    visit namespace_project_path(project.namespace, project)
+  end
+end
diff --git a/spec/features/projects/members/group_owner_cannot_request_access_to_his_group_project_spec.rb b/spec/features/projects/members/group_owner_cannot_request_access_to_his_group_project_spec.rb
deleted file mode 100644
index c4686ccceff..00000000000
--- a/spec/features/projects/members/group_owner_cannot_request_access_to_his_group_project_spec.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-require 'spec_helper'
-
-feature 'Projects > Members > Group owner cannot request access to his group project', feature: true do
-  let(:owner) { create(:user) }
-  let(:group) { create(:group) }
-  let(:project) { create(:project, namespace: group) }
-
-  background do
-    group.add_owner(owner)
-    login_as(owner)
-    visit namespace_project_path(project.namespace, project)
-  end
-
-  scenario 'owner does not see the request access button' do
-    expect(page).not_to have_content 'Request Access'
-  end
-end