UI and copywriting improvements

+ 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>

4 files changed, 37 insertions, 51 deletions

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