Commit e5f31632 authored by Eugie Limpin's avatar Eugie Limpin Committed by Thong Kuah

Send invite team email after a root group is created

Update group create service to enqueue a background job to send an
invite team email 20 minutes after a root group is created.

Update EmailCampaignsController to handle CTA clicks from invite team
emails.

Changelog: added
parent 61ea24bf
...@@ -40,7 +40,7 @@ class Groups::EmailCampaignsController < Groups::ApplicationController ...@@ -40,7 +40,7 @@ class Groups::EmailCampaignsController < Groups::ApplicationController
project_pipelines_url(group.projects.first) project_pipelines_url(group.projects.first)
when :trial, :trial_short when :trial, :trial_short
'https://about.gitlab.com/free-trial/' 'https://about.gitlab.com/free-trial/'
when :team, :team_short when :team, :team_short, :invite_team
group_group_members_url(group) group_group_members_url(group)
when :admin_verify when :admin_verify
project_settings_ci_cd_path(group.projects.first, ci_runner_templates: true, anchor: 'js-runners-settings') project_settings_ci_cd_path(group.projects.first, ci_runner_templates: true, anchor: 'js-runners-settings')
...@@ -59,6 +59,11 @@ class Groups::EmailCampaignsController < Groups::ApplicationController ...@@ -59,6 +59,11 @@ class Groups::EmailCampaignsController < Groups::ApplicationController
@track = params[:track]&.to_sym @track = params[:track]&.to_sym
@series = params[:series]&.to_i @series = params[:series]&.to_i
# There is only one email that will be sent for invite team track so series
# should only have the value 0. Return early if track is invite team and
# condition for series value is met
return if @track == Namespaces::InviteTeamEmailService::TRACK && @series == 0
track_valid = @track.in?(Namespaces::InProductMarketingEmailsService::TRACKS.keys) track_valid = @track.in?(Namespaces::InProductMarketingEmailsService::TRACKS.keys)
return render_404 unless track_valid return render_404 unless track_valid
......
...@@ -22,7 +22,8 @@ module Users ...@@ -22,7 +22,8 @@ module Users
experience: 4, experience: 4,
team_short: 5, team_short: 5,
trial_short: 6, trial_short: 6,
admin_verify: 7 admin_verify: 7,
invite_team: 8
}, _suffix: true }, _suffix: true
scope :without_track_and_series, -> (track, series) do scope :without_track_and_series, -> (track, series) do
......
...@@ -57,7 +57,10 @@ module Groups ...@@ -57,7 +57,10 @@ module Groups
end end
def after_create_hook def after_create_hook
# overridden in EE if group.persisted? && group.root?
delay = Namespaces::InviteTeamEmailService::DELIVERY_DELAY_IN_MINUTES
Namespaces::InviteTeamEmailWorker.perform_in(delay, group.id, current_user.id)
end
end end
def remove_unallowed_params def remove_unallowed_params
......
# frozen_string_literal: true
module Namespaces
class InProductMarketingEmailRecords
attr_reader :records
def initialize
@records = []
end
def save!
Users::InProductMarketingEmail.bulk_insert!(@records)
@records = []
end
def add(user, track, series)
@records << Users::InProductMarketingEmail.new(
user: user,
track: track,
series: series,
created_at: Time.zone.now,
updated_at: Time.zone.now
)
end
end
end
...@@ -56,7 +56,7 @@ module Namespaces ...@@ -56,7 +56,7 @@ module Namespaces
def initialize(track, interval) def initialize(track, interval)
@track = track @track = track
@interval = interval @interval = interval
@in_product_marketing_email_records = [] @sent_email_records = InProductMarketingEmailRecords.new
end end
def execute def execute
...@@ -71,17 +71,21 @@ module Namespaces ...@@ -71,17 +71,21 @@ module Namespaces
private private
attr_reader :track, :interval, :in_product_marketing_email_records attr_reader :track, :interval, :sent_email_records
def send_email(user, group)
NotificationService.new.in_product_marketing(user.id, group.id, track, series)
end
def send_email_for_group(group) def send_email_for_group(group)
users_for_group(group).each do |user| users_for_group(group).each do |user|
if can_perform_action?(user, group) if can_perform_action?(user, group)
send_email(user, group) send_email(user, group)
track_sent_email(user, track, series) sent_email_records.add(user, track, series)
end end
end end
save_tracked_emails! sent_email_records.save!
end end
def groups_for_track def groups_for_track
...@@ -126,10 +130,6 @@ module Namespaces ...@@ -126,10 +130,6 @@ module Namespaces
end end
end end
def send_email(user, group)
NotificationService.new.in_product_marketing(user.id, group.id, track, series)
end
def completed_actions def completed_actions
TRACKS[track][:completed_actions] TRACKS[track][:completed_actions]
end end
...@@ -146,21 +146,6 @@ module Namespaces ...@@ -146,21 +146,6 @@ module Namespaces
def series def series
TRACKS[track][:interval_days].index(interval) TRACKS[track][:interval_days].index(interval)
end end
def save_tracked_emails!
Users::InProductMarketingEmail.bulk_insert!(in_product_marketing_email_records)
@in_product_marketing_email_records = []
end
def track_sent_email(user, track, series)
in_product_marketing_email_records << Users::InProductMarketingEmail.new(
user: user,
track: track,
series: series,
created_at: Time.zone.now,
updated_at: Time.zone.now
)
end
end end
end end
......
# frozen_string_literal: true
module Namespaces
class InviteTeamEmailService
include Gitlab::Experiment::Dsl
TRACK = :invite_team
DELIVERY_DELAY_IN_MINUTES = 20.minutes
def self.send_email(user, group)
new(user, group).execute
end
def initialize(user, group)
@group = group
@user = user
@sent_email_records = InProductMarketingEmailRecords.new
end
def execute
return unless user.email_opted_in?
return unless group.root?
return unless group.setup_for_company
# Exclude group if users other than the creator have already been
# added/invited
return unless group.member_count == 1
return if email_for_track_sent_to_user?
experiment(:invite_team_email, group: group) do |e|
e.candidate do
send_email(user, group)
sent_email_records.add(user, track, series)
sent_email_records.save!
end
e.record!
end
end
private
attr_reader :user, :group, :sent_email_records
def send_email(user, group)
NotificationService.new.in_product_marketing(user.id, group.id, track, series)
end
def track
TRACK
end
def series
0
end
def email_for_track_sent_to_user?
Users::InProductMarketingEmail.for_user_with_track_and_series(user, track, series).present?
end
end
end
...@@ -2411,6 +2411,15 @@ ...@@ -2411,6 +2411,15 @@
:weight: 1 :weight: 1
:idempotent: :idempotent:
:tags: [] :tags: []
- :name: namespaces_invite_team_email
:worker_name: Namespaces::InviteTeamEmailWorker
:feature_category: :experimentation_activation
:has_external_dependencies:
:urgency: :low
:resource_boundary: :unknown
:weight: 1
:idempotent:
:tags: []
- :name: namespaces_onboarding_issue_created - :name: namespaces_onboarding_issue_created
:worker_name: Namespaces::OnboardingIssueCreatedWorker :worker_name: Namespaces::OnboardingIssueCreatedWorker
:feature_category: :onboarding :feature_category: :onboarding
......
# frozen_string_literal: true
module Namespaces
class InviteTeamEmailWorker # rubocop:disable Scalability/IdempotentWorker
include ApplicationWorker
data_consistency :always
feature_category :experimentation_activation
urgency :low
def perform(group_id, user_id)
# rubocop: disable CodeReuse/ActiveRecord
user = User.find_by(id: user_id)
group = Group.find_by(id: group_id)
# rubocop: enable CodeReuse/ActiveRecord
return unless user && group
Namespaces::InviteTeamEmailService.send_email(user, group)
end
end
end
---
name: invite_team_email
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72470
rollout_issue_url:
milestone: '14.5'
type: experiment
group: group::activation
default_enabled: false
...@@ -253,6 +253,8 @@ ...@@ -253,6 +253,8 @@
- 1 - 1
- - namespaceless_project_destroy - - namespaceless_project_destroy
- 1 - 1
- - namespaces_invite_team_email
- 1
- - namespaces_onboarding_issue_created - - namespaces_onboarding_issue_created
- 1 - 1
- - namespaces_onboarding_pipeline_created - - namespaces_onboarding_pipeline_created
......
# frozen_string_literal: true # frozen_string_literal: true
response = ::Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService.new.execute response = Sidekiq::Worker.skipping_transaction_check do
::Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService.new.execute
end
if response[:status] == :success if response[:status] == :success
puts "Successfully created self monitoring project." puts "Successfully created self monitoring project."
......
...@@ -26,6 +26,8 @@ module EE ...@@ -26,6 +26,8 @@ module EE
override :after_create_hook override :after_create_hook
def after_create_hook def after_create_hook
super
if group.persisted? && create_event if group.persisted? && create_event
::Groups::CreateEventWorker.perform_async(group.id, current_user.id, :created) ::Groups::CreateEventWorker.perform_async(group.id, current_user.id, :created)
end end
......
...@@ -7,7 +7,7 @@ module Gitlab ...@@ -7,7 +7,7 @@ module Gitlab
UnknownTrackError = Class.new(StandardError) UnknownTrackError = Class.new(StandardError)
def self.for(track) def self.for(track)
valid_tracks = [:invite_team, Namespaces::InProductMarketingEmailsService::TRACKS.keys].flatten valid_tracks = [Namespaces::InviteTeamEmailService::TRACK, Namespaces::InProductMarketingEmailsService::TRACKS.keys].flatten
raise UnknownTrackError unless valid_tracks.include?(track) raise UnknownTrackError unless valid_tracks.include?(track)
"Gitlab::Email::Message::InProductMarketing::#{track.to_s.classify}".constantize "Gitlab::Email::Message::InProductMarketing::#{track.to_s.classify}".constantize
......
...@@ -177,8 +177,6 @@ module Gitlab ...@@ -177,8 +177,6 @@ module Gitlab
end end
def validate_series! def validate_series!
return unless series?
raise ArgumentError, "Only #{total_series} series available for this track." unless @series.between?(0, total_series - 1) raise ArgumentError, "Only #{total_series} series available for this track." unless @series.between?(0, total_series - 1)
end end
end end
......
...@@ -40,6 +40,12 @@ module Gitlab ...@@ -40,6 +40,12 @@ module Gitlab
def series? def series?
false false
end end
private
def validate_series!
raise ArgumentError, "Only one email is sent for this track. Value of `series` should be 0." unless @series == 0
end
end end
end end
end end
......
...@@ -829,7 +829,13 @@ module Gitlab ...@@ -829,7 +829,13 @@ module Gitlab
Users::InProductMarketingEmail.tracks.keys.each_with_object({}) do |track, result| Users::InProductMarketingEmail.tracks.keys.each_with_object({}) do |track, result|
# rubocop: enable UsageData/LargeTable: # rubocop: enable UsageData/LargeTable:
series_amount = Namespaces::InProductMarketingEmailsService::TRACKS[track.to_sym][:interval_days].count series_amount =
if track.to_sym == Namespaces::InviteTeamEmailService::TRACK
0
else
Namespaces::InProductMarketingEmailsService::TRACKS[track.to_sym][:interval_days].count
end
0.upto(series_amount - 1).map do |series| 0.upto(series_amount - 1).map do |series|
# When there is an error with the query and it's not the Hash we expect, we return what we got from `count`. # When there is an error with the query and it's not the Hash we expect, we return what we got from `count`.
sent_count = sent_emails.is_a?(Hash) ? sent_emails.fetch([track, series], 0) : sent_emails sent_count = sent_emails.is_a?(Hash) ? sent_emails.fetch([track, series], 0) : sent_emails
......
...@@ -6,7 +6,25 @@ RSpec.describe Gitlab::Email::Message::InProductMarketing::InviteTeam do ...@@ -6,7 +6,25 @@ RSpec.describe Gitlab::Email::Message::InProductMarketing::InviteTeam do
let_it_be(:group) { build(:group) } let_it_be(:group) { build(:group) }
let_it_be(:user) { build(:user) } let_it_be(:user) { build(:user) }
subject(:message) { described_class.new(group: group, user: user, series: 0) } let(:series) { 0 }
subject(:message) { described_class.new(group: group, user: user, series: series) }
describe 'initialize' do
context 'when series is valid' do
it 'does not raise error' do
expect { subject }.not_to raise_error(ArgumentError)
end
end
context 'when series is invalid' do
let(:series) { 1 }
it 'raises error' do
expect { subject }.to raise_error(ArgumentError)
end
end
end
it 'contains the correct message', :aggregate_failures do it 'contains the correct message', :aggregate_failures do
expect(message.subject_line).to eq 'Invite your teammates to GitLab' expect(message.subject_line).to eq 'Invite your teammates to GitLab'
......
...@@ -21,7 +21,8 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do ...@@ -21,7 +21,8 @@ RSpec.describe Users::InProductMarketingEmail, type: :model do
describe '.tracks' do describe '.tracks' do
it 'has an entry for every track' do it 'has an entry for every track' do
expect(Namespaces::InProductMarketingEmailsService::TRACKS.keys).to match_array(described_class.tracks.keys.map(&:to_sym)) tracks = [Namespaces::InviteTeamEmailService::TRACK, Namespaces::InProductMarketingEmailsService::TRACKS.keys].flatten
expect(tracks).to match_array(described_class.tracks.keys.map(&:to_sym))
end end
end end
......
...@@ -94,7 +94,7 @@ RSpec.describe Groups::EmailCampaignsController do ...@@ -94,7 +94,7 @@ RSpec.describe Groups::EmailCampaignsController do
describe 'track parameter' do describe 'track parameter' do
context 'when valid' do context 'when valid' do
where(track: Namespaces::InProductMarketingEmailsService::TRACKS.keys.without(:experience)) where(track: [Namespaces::InProductMarketingEmailsService::TRACKS.keys.without(:experience), Namespaces::InviteTeamEmailService::TRACK].flatten)
with_them do with_them do
it_behaves_like 'track and redirect' it_behaves_like 'track and redirect'
...@@ -117,6 +117,10 @@ RSpec.describe Groups::EmailCampaignsController do ...@@ -117,6 +117,10 @@ RSpec.describe Groups::EmailCampaignsController do
with_them do with_them do
it_behaves_like 'track and redirect' it_behaves_like 'track and redirect'
end end
it_behaves_like 'track and redirect' do
let(:track) { Namespaces::InviteTeamEmailService::TRACK.to_s }
end
end end
context 'when invalid' do context 'when invalid' do
...@@ -124,6 +128,10 @@ RSpec.describe Groups::EmailCampaignsController do ...@@ -124,6 +128,10 @@ RSpec.describe Groups::EmailCampaignsController do
with_them do with_them do
it_behaves_like 'no track and 404' it_behaves_like 'no track and 404'
it_behaves_like 'no track and 404' do
let(:track) { Namespaces::InviteTeamEmailService::TRACK.to_s }
end
end end
end end
end end
......
...@@ -242,4 +242,41 @@ RSpec.describe Groups::CreateService, '#execute' do ...@@ -242,4 +242,41 @@ RSpec.describe Groups::CreateService, '#execute' do
end end
end end
end end
describe 'invite team email' do
let(:service) { described_class.new(user, group_params) }
before do
allow(Namespaces::InviteTeamEmailWorker).to receive(:perform_in)
end
it 'is sent' do
group = service.execute
delay = Namespaces::InviteTeamEmailService::DELIVERY_DELAY_IN_MINUTES
expect(Namespaces::InviteTeamEmailWorker).to have_received(:perform_in).with(delay, group.id, user.id)
end
context 'when group has not been persisted' do
let(:service) { described_class.new(user, group_params.merge(name: '<script>alert("Attack!")</script>')) }
it 'not sent' do
expect(Namespaces::InviteTeamEmailWorker).not_to receive(:perform_in)
service.execute
end
end
context 'when group is not root' do
let(:parent_group) { create :group }
let(:service) { described_class.new(user, group_params.merge(parent_id: parent_group.id)) }
before do
parent_group.add_owner(user)
end
it 'not sent' do
expect(Namespaces::InviteTeamEmailWorker).not_to receive(:perform_in)
service.execute
end
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Namespaces::InProductMarketingEmailRecords do
let_it_be(:user) { create :user }
subject(:records) { described_class.new }
it 'initializes records' do
expect(subject.records).to match_array []
end
describe '#save!' do
before do
allow(Users::InProductMarketingEmail).to receive(:bulk_insert!)
records.add(user, :invite_team, 0)
records.add(user, :create, 1)
end
it 'bulk inserts added records' do
expect(Users::InProductMarketingEmail).to receive(:bulk_insert!).with(records.records)
records.save!
end
it 'resets its records' do
records.save!
expect(records.records).to match_array []
end
end
describe '#add' do
it 'adds a Users::InProductMarketingEmail record to its records' do
freeze_time do
records.add(user, :invite_team, 0)
records.add(user, :create, 1)
first, second = records.records
expect(first).to be_a Users::InProductMarketingEmail
expect(first.track.to_sym).to eq :invite_team
expect(first.series).to eq 0
expect(first.created_at).to eq Time.zone.now
expect(first.updated_at).to eq Time.zone.now
expect(second).to be_a Users::InProductMarketingEmail
expect(second.track.to_sym).to eq :create
expect(second.series).to eq 1
expect(second.created_at).to eq Time.zone.now
expect(second.updated_at).to eq Time.zone.now
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Namespaces::InviteTeamEmailService do
let_it_be(:user) { create(:user, email_opted_in: true) }
let(:track) { described_class::TRACK }
let(:series) { 0 }
let(:setup_for_company) { true }
let(:parent_group) { nil }
let(:group) { create(:group, parent: parent_group) }
subject(:action) { described_class.send_email(user, group) }
before do
group.add_owner(user)
allow(group).to receive(:setup_for_company).and_return(setup_for_company)
allow(Notify).to receive(:in_product_marketing_email).and_return(double(deliver_later: nil))
end
RSpec::Matchers.define :send_invite_team_email do |*args|
match do
expect(Notify).to have_received(:in_product_marketing_email).with(*args).once
end
match_when_negated do
expect(Notify).not_to have_received(:in_product_marketing_email)
end
end
shared_examples 'unexperimented' do
it { is_expected.not_to send_invite_team_email }
it 'does not record sent email' do
expect { subject }.not_to change { Users::InProductMarketingEmail.count }
end
end
shared_examples 'candidate' do
it { is_expected.to send_invite_team_email(user.id, group.id, track, 0) }
it 'records sent email' do
expect { subject }.to change { Users::InProductMarketingEmail.count }.by(1)
expect(
Users::InProductMarketingEmail.where(
user: user,
track: track,
series: 0
)
).to exist
end
it_behaves_like 'tracks assignment and records the subject', :invite_team_email, :group do
subject { group }
end
end
context 'when group is in control path' do
before do
stub_experiments(invite_team_email: :control)
end
it { is_expected.not_to send_invite_team_email }
it 'does not record sent email' do
expect { subject }.not_to change { Users::InProductMarketingEmail.count }
end
it_behaves_like 'tracks assignment and records the subject', :invite_team_email, :group do
subject { group }
end
end
context 'when group is in candidate path' do
before do
stub_experiments(invite_team_email: :candidate)
end
it_behaves_like 'candidate'
context 'when the user has not opted into marketing emails' do
let(:user) { create(:user, email_opted_in: false ) }
it_behaves_like 'unexperimented'
end
context 'when group is not top level' do
it_behaves_like 'unexperimented' do
let(:parent_group) do
create(:group).tap { |g| g.add_owner(user) }
end
end
end
context 'when group is not set up for a company' do
it_behaves_like 'unexperimented' do
let(:setup_for_company) { nil }
end
end
context 'when other users have already been added to the group' do
before do
group.add_developer(create(:user))
end
it_behaves_like 'unexperimented'
end
context 'when other users have already been invited to the group' do
before do
group.add_developer('not_a_user_yet@example.com')
end
it_behaves_like 'unexperimented'
end
context 'when the user already got sent the email' do
before do
create(:in_product_marketing_email, user: user, track: track, series: 0)
end
it_behaves_like 'unexperimented'
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Namespaces::InviteTeamEmailWorker do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
it 'sends the email' do
expect(Namespaces::InviteTeamEmailService).to receive(:send_email).with(user, group).once
subject.perform(group.id, user.id)
end
context 'when user id is non-existent' do
it 'does not send the email' do
expect(Namespaces::InviteTeamEmailService).not_to receive(:send_email)
subject.perform(group.id, non_existing_record_id)
end
end
context 'when group id is non-existent' do
it 'does not send the email' do
expect(Namespaces::InviteTeamEmailService).not_to receive(:send_email)
subject.perform(non_existing_record_id, user.id)
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