Commit bad77543 authored by Doug Stull's avatar Doug Stull

Add basic acceptance tracking for invite emails

- since we automatically accept invites on registration,
  we skip the invite path where we originally track it.
parent 4a61262a
...@@ -6,7 +6,14 @@ module AcceptsPendingInvitations ...@@ -6,7 +6,14 @@ module AcceptsPendingInvitations
def accept_pending_invitations def accept_pending_invitations
return unless resource.active_for_authentication? return unless resource.active_for_authentication?
clear_stored_location_for_resource if resource.accept_pending_invitations!.any? if resource.accept_pending_invitations!.any?
clear_stored_location_for_resource
after_pending_invitations_hook
end
end
def after_pending_invitations_hook
# no-op
end end
def clear_stored_location_for_resource def clear_stored_location_for_resource
......
...@@ -3,10 +3,10 @@ ...@@ -3,10 +3,10 @@
class InvitesController < ApplicationController class InvitesController < ApplicationController
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
prepend_before_action :authenticate_user!, :track_invite_join_click, only: :show
before_action :member before_action :member
before_action :ensure_member_exists before_action :ensure_member_exists
before_action :invite_details before_action :invite_details
before_action :set_invite_type, only: :show
skip_before_action :authenticate_user!, only: :decline skip_before_action :authenticate_user!, only: :decline
helper_method :member?, :current_user_matches_invite? helper_method :member?, :current_user_matches_invite?
...@@ -16,16 +16,11 @@ class InvitesController < ApplicationController ...@@ -16,16 +16,11 @@ class InvitesController < ApplicationController
feature_category :authentication_and_authorization feature_category :authentication_and_authorization
def show def show
experiment('members/invite_email', actor: member).track(:opened) if initial_invite_email?
accept if skip_invitation_prompt? accept if skip_invitation_prompt?
end end
def accept def accept
if member.accept_invite!(current_user) if member.accept_invite!(current_user)
experiment('members/invite_email', actor: member).track(:accepted) if initial_invite_email?
session.delete(:invite_type)
redirect_to invite_details[:path], notice: helpers.invite_accepted_notice(member) redirect_to invite_details[:path], notice: helpers.invite_accepted_notice(member)
else else
redirect_back_or_default(options: { alert: _("The invitation could not be accepted.") }) redirect_back_or_default(options: { alert: _("The invitation could not be accepted.") })
...@@ -52,14 +47,6 @@ class InvitesController < ApplicationController ...@@ -52,14 +47,6 @@ class InvitesController < ApplicationController
private private
def set_invite_type
session[:invite_type] = params[:invite_type] if params[:invite_type].in?([Members::InviteEmailExperiment::INVITE_TYPE])
end
def initial_invite_email?
session[:invite_type] == Members::InviteEmailExperiment::INVITE_TYPE
end
def skip_invitation_prompt? def skip_invitation_prompt?
!member? && current_user_matches_invite? !member? && current_user_matches_invite?
end end
...@@ -87,13 +74,17 @@ class InvitesController < ApplicationController ...@@ -87,13 +74,17 @@ class InvitesController < ApplicationController
render_404 render_404
end end
def track_invite_join_click
experiment('members/invite_email', actor: member).track(:join_clicked) if member && Members::InviteEmailExperiment.initial_invite_email?(params[:invite_type])
end
def authenticate_user! def authenticate_user!
return if current_user return if current_user
store_location_for(:user, invite_landing_url) if member store_location_for(:user, invite_landing_url) if member
if user_sign_up? if user_sign_up?
session[:invite_email] = member.invite_email set_session_invite_params
redirect_to new_user_registration_path(invite_email: member.invite_email), notice: _("To accept this invitation, create an account or sign in.") redirect_to new_user_registration_path(invite_email: member.invite_email), notice: _("To accept this invitation, create an account or sign in.")
else else
...@@ -101,6 +92,12 @@ class InvitesController < ApplicationController ...@@ -101,6 +92,12 @@ class InvitesController < ApplicationController
end end
end end
def set_session_invite_params
session[:invite_email] = member.invite_email
session[:originating_member_id] = member.id if Members::InviteEmailExperiment.initial_invite_email?(params[:invite_type])
end
def sign_in_redirect_params def sign_in_redirect_params
member ? { invite_email: member.invite_email } : {} member ? { invite_email: member.invite_email } : {}
end end
......
...@@ -187,6 +187,20 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -187,6 +187,20 @@ class RegistrationsController < Devise::RegistrationsController
def set_invite_params def set_invite_params
@invite_email = ActionController::Base.helpers.sanitize(params[:invite_email]) @invite_email = ActionController::Base.helpers.sanitize(params[:invite_email])
end end
def after_pending_invitations_hook
member_id = session.delete(:originating_member_id)
return unless member_id
# if invited multiple times to different projects, only the email clicked will be counted as accepted
# for the specific member on a project or group
member = resource.members.find_by(id: member_id) # rubocop: disable CodeReuse/ActiveRecord
return unless member
experiment('members/invite_email', actor: member).track(:accepted)
end
end end
RegistrationsController.prepend_if_ee('EE::RegistrationsController') RegistrationsController.prepend_if_ee('EE::RegistrationsController')
...@@ -7,6 +7,10 @@ module Members ...@@ -7,6 +7,10 @@ module Members
INVITE_TYPE = 'initial_email' INVITE_TYPE = 'initial_email'
def self.initial_invite_email?(invite_type)
invite_type == INVITE_TYPE
end
def resolve_variant_name def resolve_variant_name
RoundRobin.new(feature_flag_name, %i[avatar permission_info control]).execute RoundRobin.new(feature_flag_name, %i[avatar permission_info control]).execute
end end
......
...@@ -8,11 +8,12 @@ RSpec.describe InvitesController do ...@@ -8,11 +8,12 @@ RSpec.describe InvitesController do
let(:raw_invite_token) { member.raw_invite_token } let(:raw_invite_token) { member.raw_invite_token }
let(:project_members) { member.source.users } let(:project_members) { member.source.users }
let(:md5_member_global_id) { Digest::MD5.hexdigest(member.to_global_id.to_s) } let(:md5_member_global_id) { Digest::MD5.hexdigest(member.to_global_id.to_s) }
let(:params) { { id: raw_invite_token } } let(:extra_params) { {} }
let(:params) { { id: raw_invite_token }.merge(extra_params) }
shared_examples 'invalid token' do shared_examples 'invalid token' do
context 'when invite token is not valid' do context 'when invite token is not valid' do
let(:params) { { id: '_bogus_token_' } } let(:raw_invite_token) { '_bogus_token_' }
it 'renders the 404 page' do it 'renders the 404 page' do
request request
...@@ -25,6 +26,37 @@ RSpec.describe InvitesController do ...@@ -25,6 +26,37 @@ RSpec.describe InvitesController do
describe 'GET #show' do describe 'GET #show' do
subject(:request) { get :show, params: params } subject(:request) { get :show, params: params }
context 'when it is part of our invite email experiment' do
let(:extra_params) { { invite_type: 'initial_email' } }
it 'tracks the experiment' do
experiment = double(track: true)
allow(controller).to receive(:experiment).with('members/invite_email', actor: member).and_return(experiment)
request
expect(experiment).to have_received(:track).with(:join_clicked)
end
context 'when member does not exist' do
let(:raw_invite_token) { '_bogus_token_' }
it 'does not track the experiment' do
expect(controller).not_to receive(:experiment).with('members/invite_email', actor: member)
request
end
end
end
context 'when it is not part of our invite email experiment' do
it 'does not track via experiment' do
expect(controller).not_to receive(:experiment).with('members/invite_email', actor: member)
request
end
end
context 'when logged in' do context 'when logged in' do
before do before do
sign_in(user) sign_in(user)
...@@ -51,32 +83,10 @@ RSpec.describe InvitesController do ...@@ -51,32 +83,10 @@ RSpec.describe InvitesController do
end end
it_behaves_like 'invalid token' it_behaves_like 'invalid token'
context 'when invite comes from the initial email invite' do
let(:params) { { id: raw_invite_token, invite_type: Members::InviteEmailExperiment::INVITE_TYPE } }
it 'tracks via experiment', :aggregate_failures do
experiment = double(track: true)
allow(controller).to receive(:experiment).and_return(experiment)
request
expect(experiment).to have_received(:track).with(:opened)
expect(experiment).to have_received(:track).with(:accepted)
end
end
context 'when invite does not come from initial email invite' do
it 'does not track via experiment' do
expect(controller).not_to receive(:experiment)
request
end
end
end end
context 'when not logged in' do context 'when not logged in' do
context 'when inviter is a member' do context 'when invite token belongs to a valid member' do
context 'when instance allows sign up' do context 'when instance allows sign up' do
it 'indicates an account can be created in notice' do it 'indicates an account can be created in notice' do
request request
...@@ -121,6 +131,30 @@ RSpec.describe InvitesController do ...@@ -121,6 +131,30 @@ RSpec.describe InvitesController do
expect(response).to redirect_to(new_user_registration_path(invite_email: member.invite_email)) expect(response).to redirect_to(new_user_registration_path(invite_email: member.invite_email))
end end
it 'sets session keys for auto email confirmation on sign up' do
request
expect(session[:invite_email]).to eq(member.invite_email)
end
context 'when it is part of our invite email experiment' do
let(:extra_params) { { invite_type: 'initial_email' } }
it 'sets session key for invite acceptance tracking on sign-up' do
request
expect(session[:originating_member_id]).to eq(member.id)
end
end
context 'when it is not part of our invite email experiment' do
it 'does not set the session key for invite acceptance tracking on sign-up' do
request
expect(session[:originating_member_id]).to be_nil
end
end
end end
end end
...@@ -157,7 +191,7 @@ RSpec.describe InvitesController do ...@@ -157,7 +191,7 @@ RSpec.describe InvitesController do
end end
end end
context 'when inviter is not a member' do context 'when invite token does not belong to a valid member' do
let(:params) { { id: '_bogus_token_' } } let(:params) { { id: '_bogus_token_' } }
it 'is redirected to a new session' do it 'is redirected to a new session' do
...@@ -177,25 +211,6 @@ RSpec.describe InvitesController do ...@@ -177,25 +211,6 @@ RSpec.describe InvitesController do
subject(:request) { post :accept, params: params } subject(:request) { post :accept, params: params }
it_behaves_like 'invalid token' it_behaves_like 'invalid token'
context 'when invite comes from the initial email invite' do
it 'tracks via experiment' do
experiment = double(track: true)
allow(controller).to receive(:experiment).and_return(experiment)
post :accept, params: params, session: { invite_type: Members::InviteEmailExperiment::INVITE_TYPE }
expect(experiment).to have_received(:track).with(:accepted)
end
end
context 'when invite does not come from initial email invite' do
it 'does not track via experiment' do
expect(controller).not_to receive(:experiment)
request
end
end
end end
describe 'POST #decline for link in UI' do describe 'POST #decline for link in UI' do
......
...@@ -20,8 +20,12 @@ RSpec.describe RegistrationsController do ...@@ -20,8 +20,12 @@ RSpec.describe RegistrationsController do
end end
describe '#create' do describe '#create' do
let(:base_user_params) { { first_name: 'first', last_name: 'last', username: 'new_username', email: 'new@user.com', password: 'Any_password' } } let_it_be(:base_user_params) do
let(:user_params) { { user: base_user_params } } { first_name: 'first', last_name: 'last', username: 'new_username', email: 'new@user.com', password: 'Any_password' }
end
let_it_be(:user_params) { { user: base_user_params } }
let(:session_params) { {} } let(:session_params) { {} }
subject { post(:create, params: user_params, session: session_params) } subject { post(:create, params: user_params, session: session_params) }
...@@ -151,6 +155,38 @@ RSpec.describe RegistrationsController do ...@@ -151,6 +155,38 @@ RSpec.describe RegistrationsController do
end end
context 'when registration is triggered from an accepted invite' do context 'when registration is triggered from an accepted invite' do
context 'when it is part of our invite email experiment', :experiment do
let_it_be(:member) { create(:project_member, :invited, invite_email: user_params.dig(:user, :email)) }
let(:originating_member_id) { member.id }
let(:session_params) do
{
invite_email: user_params.dig(:user, :email),
originating_member_id: originating_member_id
}
end
context 'when member exists from the session key value' do
it 'tracks the experiment' do
expect(experiment('members/invite_email')).to track(:accepted)
.with_context(actor: member)
.on_next_instance
subject
end
end
context 'when member does not exist from the session key value' do
let(:originating_member_id) { -1 }
it 'tracks the experiment' do
expect(experiment('members/invite_email')).not_to track(:accepted)
subject
end
end
end
context 'when invite email matches email used on registration' do context 'when invite email matches email used on registration' do
let(:session_params) { { invite_email: user_params.dig(:user, :email) } } let(:session_params) { { invite_email: user_params.dig(:user, :email) } }
......
...@@ -11,6 +11,16 @@ RSpec.describe Members::InviteEmailExperiment, :clean_gitlab_redis_shared_state ...@@ -11,6 +11,16 @@ RSpec.describe Members::InviteEmailExperiment, :clean_gitlab_redis_shared_state
allow(invite_email).to receive(:enabled?).and_return(true) allow(invite_email).to receive(:enabled?).and_return(true)
end end
describe ".initial_invite_email?" do
it "is an initial invite email" do
expect(described_class.initial_invite_email?('initial_email')).to be(true)
end
it "is not an initial invite email" do
expect(described_class.initial_invite_email?('_bogus_')).to be(false)
end
end
describe "exclusions", :experiment do describe "exclusions", :experiment do
it "excludes when created by is nil" do it "excludes when created by is nil" do
expect(experiment('members/invite_email')).to exclude(actor: double(created_by: nil)) expect(experiment('members/invite_email')).to exclude(actor: double(created_by: nil))
......
...@@ -3,10 +3,11 @@ ...@@ -3,10 +3,11 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Group or Project invitations', :aggregate_failures do RSpec.describe 'Group or Project invitations', :aggregate_failures do
let_it_be(:owner) { create(:user, name: 'John Doe') }
let_it_be(:group) { create(:group, name: 'Owned') }
let_it_be(:project) { create(:project, :repository, namespace: group) }
let(:user) { create(:user, email: 'user@example.com') } 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) }
let(:group_invite) { group.group_members.invite.last } let(:group_invite) { group.group_members.invite.last }
before do before do
...@@ -90,11 +91,15 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do ...@@ -90,11 +91,15 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
let(:new_user) { build_stubbed(:user) } let(:new_user) { build_stubbed(:user) }
let(:invite_email) { new_user.email } let(:invite_email) { new_user.email }
let(:group_invite) { create(:group_member, :invited, group: group, invite_email: invite_email, created_by: owner) } let(:group_invite) { create(:group_member, :invited, group: group, invite_email: invite_email, created_by: owner) }
let(:send_email_confirmation) { true }
before do
stub_application_setting(send_user_confirmation_email: send_email_confirmation)
end
context 'when registering using invitation email' do context 'when registering using invitation email' do
before do before do
stub_application_setting(send_user_confirmation_email: send_email_confirmation) visit invite_path(group_invite.raw_invite_token, invite_type: Members::InviteEmailExperiment::INVITE_TYPE)
visit invite_path(group_invite.raw_invite_token)
end end
context 'with admin approval required enabled' do context 'with admin approval required enabled' do
...@@ -102,8 +107,6 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do ...@@ -102,8 +107,6 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
stub_application_setting(require_admin_approval_after_user_signup: true) stub_application_setting(require_admin_approval_after_user_signup: true)
end end
let(:send_email_confirmation) { true }
it 'does not sign the user in' do it 'does not sign the user in' do
fill_in_sign_up_form(new_user) fill_in_sign_up_form(new_user)
...@@ -136,7 +139,15 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do ...@@ -136,7 +139,15 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
end end
context 'email confirmation enabled' do context 'email confirmation enabled' do
let(:send_email_confirmation) { true } context 'with members/invite_email experiment', :experiment do
it 'tracks the accepted invite' do
expect(experiment('members/invite_email')).to track(:accepted)
.with_context(actor: group_invite)
.on_next_instance
fill_in_sign_up_form(new_user)
end
end
context 'when soft email confirmation is not enabled' do context 'when soft email confirmation is not enabled' do
before do before do
...@@ -201,8 +212,6 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do ...@@ -201,8 +212,6 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
end end
context 'when declining the invitation' do context 'when declining the invitation' do
let(:send_email_confirmation) { true }
context 'as an existing user' do context 'as an existing user' do
let(:group_invite) { create(:group_member, user: user, group: group, created_by: owner) } let(:group_invite) { create(:group_member, user: user, group: group, created_by: owner) }
...@@ -246,8 +255,6 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do ...@@ -246,8 +255,6 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
end end
context 'when accepting the invitation as an existing user' do context 'when accepting the invitation as an existing user' do
let(:send_email_confirmation) { true }
before do before do
sign_in(user) sign_in(user)
visit invite_path(group_invite.raw_invite_token) visit invite_path(group_invite.raw_invite_token)
......
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