Commit 78142b39 authored by Doug Stull's avatar Doug Stull

Refactor invites member method

- rendering 404 for this method was unituitive
  and was also causing issues with use in
  authenticate_user! method.
parent 42f26785
...@@ -4,6 +4,7 @@ class InvitesController < ApplicationController ...@@ -4,6 +4,7 @@ class InvitesController < ApplicationController
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
before_action :member before_action :member
before_action :ensure_member_exists
before_action :invite_details before_action :invite_details
skip_before_action :authenticate_user!, only: :decline skip_before_action :authenticate_user!, only: :decline
...@@ -59,14 +60,16 @@ class InvitesController < ApplicationController ...@@ -59,14 +60,16 @@ class InvitesController < ApplicationController
end end
def member def member
return @member if defined?(@member) strong_memoize(:member) do
@token = params[:id] @token = params[:id]
@member = Member.find_by_invite_token(@token) Member.find_by_invite_token(@token)
end
end
return render_404 unless @member def ensure_member_exists
return if member
@member render_404
end end
def authenticate_user! def authenticate_user!
...@@ -76,10 +79,7 @@ class InvitesController < ApplicationController ...@@ -76,10 +79,7 @@ class InvitesController < ApplicationController
notice << "or create an account" if Gitlab::CurrentSettings.allow_signup? notice << "or create an account" if Gitlab::CurrentSettings.allow_signup?
notice = notice.join(' ') + "." notice = notice.join(' ') + "."
# this is temporary finder instead of using member method due to render_404 possibility redirect_params = member ? { invite_email: member.invite_email } : {}
# will be resolved via https://gitlab.com/gitlab-org/gitlab/-/issues/245325
initial_member = Member.find_by_invite_token(params[:id])
redirect_params = initial_member ? { invite_email: initial_member.invite_email } : {}
store_location_for :user, request.fullpath store_location_for :user, request.fullpath
...@@ -87,20 +87,20 @@ class InvitesController < ApplicationController ...@@ -87,20 +87,20 @@ class InvitesController < ApplicationController
end end
def invite_details def invite_details
@invite_details ||= case @member.source @invite_details ||= case member.source
when Project when Project
{ {
name: @member.source.full_name, name: member.source.full_name,
url: project_url(@member.source), url: project_url(member.source),
title: _("project"), title: _("project"),
path: project_path(@member.source) path: project_path(member.source)
} }
when Group when Group
{ {
name: @member.source.name, name: member.source.name,
url: group_url(@member.source), url: group_url(member.source),
title: _("group"), title: _("group"),
path: group_path(@member.source) path: group_path(member.source)
} }
end end
end end
......
---
title: Refactor the invites controller member method
merge_request: 42727
author:
type: other
...@@ -17,8 +17,16 @@ RSpec.describe InvitesController, :snowplow do ...@@ -17,8 +17,16 @@ RSpec.describe InvitesController, :snowplow do
} }
end end
before do shared_examples 'invalid token' do
controller.instance_variable_set(:@member, member) context 'when invite token is not valid' do
let(:params) { { id: '_bogus_token_' } }
it 'renders the 404 page' do
request
expect(response).to have_gitlab_http_status(:not_found)
end
end
end end
describe 'GET #show' do describe 'GET #show' do
...@@ -39,7 +47,7 @@ RSpec.describe InvitesController, :snowplow do ...@@ -39,7 +47,7 @@ RSpec.describe InvitesController, :snowplow do
end end
it 'forces re-confirmation if email does not match signed in user' do it 'forces re-confirmation if email does not match signed in user' do
member.invite_email = 'bogus@email.com' member.update!(invite_email: 'bogus@email.com')
expect do expect do
request request
...@@ -80,6 +88,8 @@ RSpec.describe InvitesController, :snowplow do ...@@ -80,6 +88,8 @@ RSpec.describe InvitesController, :snowplow do
expect_snowplow_event(snowplow_event.merge(action: 'accepted')) expect_snowplow_event(snowplow_event.merge(action: 'accepted'))
end end
end end
it_behaves_like 'invalid token'
end end
context 'when not logged in' do context 'when not logged in' do
...@@ -139,5 +149,27 @@ RSpec.describe InvitesController, :snowplow do ...@@ -139,5 +149,27 @@ RSpec.describe InvitesController, :snowplow do
expect_snowplow_event(snowplow_event.merge(action: 'accepted')) expect_snowplow_event(snowplow_event.merge(action: 'accepted'))
end end
end end
it_behaves_like 'invalid token'
end
describe 'POST #decline for link in UI' do
before do
sign_in(user)
end
subject(:request) { post :decline, params: params }
it_behaves_like 'invalid token'
end
describe 'GET #decline for link in email' do
before do
sign_in(user)
end
subject(:request) { get :decline, params: params }
it_behaves_like 'invalid token'
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment