Merge branch '214103-remove-the-second-prompt-to-accept-or-decline-an-invitation' into 'master'

Remove the second prompt to accept or decline an invitation

See merge request gitlab-org/gitlab!35777

app/controllers/invites_controller.rb

 # frozen_string_literal: true
 
 class InvitesController < ApplicationController
+  include Gitlab::Utils::StrongMemoize
+
   before_action :member
   skip_before_action :authenticate_user!, only: :decline
 
+  helper_method :member?, :current_user_matches_invite?
+
   respond_to :html
 
   def show
+    accept if skip_invitation_prompt?
   end
 
   def accept
@@ -38,6 +43,20 @@ class InvitesController < ApplicationController
 
   private
 
+  def skip_invitation_prompt?
+    !member? && current_user_matches_invite?
+  end
+
+  def current_user_matches_invite?
+    @member.invite_email == current_user.email
+  end
+
+  def member?
+    strong_memoize(:is_member) do
+      @member.source.users.include?(current_user)
+    end
+  end
+
   def member
     return @member if defined?(@member)
 

app/views/invites/show.html.haml

@@ -20,21 +20,19 @@
       = link_to group.name, group_url(group)
   as #{@member.human_access}.
 
-- is_member = @member.source.users.include?(current_user)
-
-- if is_member
+- if member?
   %p
     - member_source = @member.source.is_a?(Group) ? _("group") : _("project")
     = _("However, you are already a member of this %{member_source}. Sign in using a different account to accept the invitation.") % { member_source: member_source }
 
-- if @member.invite_email != current_user.email
+- if !current_user_matches_invite?
   %p
     - mail_to_invite_email = mail_to(@member.invite_email)
     - mail_to_current_user = mail_to(current_user.email)
     - link_to_current_user = link_to(current_user.to_reference, user_url(current_user))
     = _("Note that this invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}.").html_safe % { mail_to_invite_email: mail_to_invite_email, mail_to_current_user: mail_to_current_user, link_to_current_user: link_to_current_user }
 
-- unless is_member
+- unless member?
   .actions
     = link_to _("Accept invitation"), accept_invite_url(@token), method: :post, class: "btn btn-success"
     = link_to _("Decline"), decline_invite_url(@token), method: :post, class: "btn btn-danger prepend-left-10"

changelogs/unreleased/214103-remove-the-second-prompt-to-accept-or-decline-an-invitation.yml

+---
+title: Remove the second prompt to accept or decline an invitation
+merge_request: 35777
+author:
+type: changed

spec/controllers/invites_controller_spec.rb

@@ -4,21 +4,44 @@ require 'spec_helper'
 
 RSpec.describe InvitesController do
   let(:token) { '123456' }
-  let(:user) { create(:user) }
-  let(:member) { create(:project_member, invite_token: token, invite_email: 'test@abc.com', user: user) }
+  let_it_be(:user) { create(:user) }
+  let(:member) { create(:project_member, :invited, invite_token: token, invite_email: user.email) }
+  let(:project_members) { member.source.users }
 
   before do
     controller.instance_variable_set(:@member, member)
     sign_in(user)
   end
 
-  describe 'GET #accept' do
+  describe 'GET #show' do
+    it 'accepts user if invite email matches signed in user' do
+      expect do
+        get :show, params: { id: token }
+      end.to change { project_members.include?(user) }.from(false).to(true)
+
+      expect(response).to have_gitlab_http_status(:found)
+      expect(flash[:notice]).to include 'You have been granted'
+    end
+
+    it 'forces re-confirmation if email does not match signed in user' do
+      member.invite_email = 'bogus@email.com'
+
+      expect do
+        get :show, params: { id: token }
+      end.not_to change { project_members.include?(user) }
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(flash[:notice]).to be_nil
+    end
+  end
+
+  describe 'POST #accept' do
     it 'accepts user' do
-      get :accept, params: { id: token }
-      member.reload
+      expect do
+        post :accept, params: { id: token }
+      end.to change { project_members.include?(user) }.from(false).to(true)
 
       expect(response).to have_gitlab_http_status(:found)
-      expect(member.user).to eq(user)
       expect(flash[:notice]).to include 'You have been granted'
     end
   end
@@ -26,8 +49,8 @@ RSpec.describe InvitesController do
   describe 'GET #decline' do
     it 'declines user' do
       get :decline, params: { id: token }
-      expect {member.reload}.to raise_error ActiveRecord::RecordNotFound
 
+      expect { member.reload }.to raise_error ActiveRecord::RecordNotFound
       expect(response).to have_gitlab_http_status(:found)
       expect(flash[:notice]).to include 'You have declined the invitation to join'
     end

spec/features/invites_spec.rb

@@ -2,8 +2,8 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Invites' do
-  let(:user) { create(:user) }
+RSpec.describe 'Invites', :aggregate_failures do
+  let(:user) { create(:user, email: 'user@example.com') }
   let(:owner) { create(:user, name: 'John Doe') }
   let(:group) { create(:group, name: 'Owned') }
   let(:project) { create(:project, :repository, namespace: group) }
@@ -11,7 +11,7 @@ RSpec.describe 'Invites' do
 
   before do
     project.add_maintainer(owner)
-    group.add_user(owner, Gitlab::Access::OWNER)
+    group.add_owner(owner)
     group.add_developer('user@example.com', owner)
     group_invite.generate_invite_token!
   end
@@ -28,7 +28,7 @@ RSpec.describe 'Invites' do
     fill_in 'new_user_email', with: new_user.email
     fill_in 'new_user_email_confirmation', with: new_user.email
     fill_in 'new_user_password', with: new_user.password
-    click_button "Register"
+    click_button 'Register'
   end
 
   def fill_in_sign_in_form(user)
@@ -48,19 +48,15 @@ RSpec.describe 'Invites' do
       expect(page).to have_content('To accept this invitation, sign in')
     end
 
-    it 'sign in and redirects to invitation page' do
+    it 'sign in, grants access and redirects to group page' do
       fill_in_sign_in_form(user)
 
-      expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
-      expect(page).to have_content(
-        'You have been invited by John Doe to join group Owned as Developer.'
-      )
-      expect(page).to have_link('Accept invitation')
-      expect(page).to have_link('Decline')
+      expect(current_path).to eq(group_path(group))
+      expect(page).to have_content('You have been granted Developer access to group Owned.')
     end
   end
 
-  context 'when signed in as an exists member' do
+  context 'when signed in as an existing member' do
     before do
       sign_in(owner)
     end
@@ -71,57 +67,13 @@ RSpec.describe 'Invites' do
     end
   end
 
-  describe 'accepting the invitation' do
-    before do
-      sign_in(user)
-      visit invite_path(group_invite.raw_invite_token)
-    end
-
-    it 'grants access and redirects to group page' do
-      page.click_link 'Accept invitation'
-      expect(current_path).to eq(group_path(group))
-      expect(page).to have_content(
-        'You have been granted Developer access to group Owned.'
-      )
-    end
-  end
-
-  describe 'declining the application' do
-    context 'when signed in' do
-      before do
-        sign_in(user)
-        visit invite_path(group_invite.raw_invite_token)
-      end
-
-      it 'declines application and redirects to dashboard' do
-        page.click_link 'Decline'
-        expect(current_path).to eq(dashboard_projects_path)
-        expect(page).to have_content(
-          'You have declined the invitation to join group Owned.'
-        )
-      end
-    end
-
-    context 'when signed out' do
-      before do
-        visit decline_invite_path(group_invite.raw_invite_token)
-      end
-
-      it 'declines application and redirects to sign in page' do
-        expect(current_path).to eq(new_user_session_path)
-        expect(page).to have_content(
-          'You have declined the invitation to join group Owned.'
-        )
-      end
-    end
-  end
-
-  describe 'invite an user using their email address' do
+  context 'when inviting a user using their email address' do
     let(:new_user) { build_stubbed(:user) }
     let(:invite_email) { new_user.email }
     let(:group_invite) { create(:group_member, :invited, group: group, invite_email: invite_email) }
     let!(:project_invite) { create(:project_member, :invited, project: project, invite_email: invite_email) }
 
+    context 'when user has not signed in yet' do
       before do
         stub_application_setting(send_user_confirmation_email: send_email_confirmation)
         visit invite_path(group_invite.raw_invite_token)
@@ -233,4 +185,48 @@ RSpec.describe 'Invites' do
         end
       end
     end
+
+    context 'when declining the invitation' do
+      let(:send_email_confirmation) { true }
+
+      context 'when signed in' do
+        before do
+          sign_in(user)
+          visit invite_path(group_invite.raw_invite_token)
+        end
+
+        it 'declines application and redirects to dashboard' do
+          page.click_link 'Decline'
+          expect(current_path).to eq(dashboard_projects_path)
+          expect(page).to have_content('You have declined the invitation to join group Owned.')
+        end
+      end
+
+      context 'when signed out' do
+        before do
+          visit decline_invite_path(group_invite.raw_invite_token)
+        end
+
+        it 'declines application and redirects to sign in page' do
+          expect(current_path).to eq(new_user_session_path)
+          expect(page).to have_content('You have declined the invitation to join group Owned.')
+        end
+      end
+    end
+
+    context 'when accepting the invitation' do
+      let(:send_email_confirmation) { true }
+
+      before do
+        sign_in(user)
+        visit invite_path(group_invite.raw_invite_token)
+      end
+
+      it 'grants access and redirects to group page' do
+        page.click_link 'Accept invitation'
+        expect(current_path).to eq(group_path(group))
+        expect(page).to have_content('You have been granted Owner access to group Owned.')
+      end
+    end
+  end
 end