Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
f9623ecc
Commit
f9623ecc
authored
Dec 03, 2020
by
Alex Buijs
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Cleanup invitation_reminders experiment code
The experiment has proven successful, now enable it by default.
parent
c8202729
Changes
12
Hide whitespace changes
Inline
Side-by-side
Showing
12 changed files
with
51 additions
and
177 deletions
+51
-177
app/controllers/invites_controller.rb
app/controllers/invites_controller.rb
+0
-5
app/mailers/emails/members.rb
app/mailers/emails/members.rb
+0
-9
app/services/members/invitation_reminder_email_service.rb
app/services/members/invitation_reminder_email_service.rb
+0
-6
app/workers/member_invitation_reminder_emails_worker.rb
app/workers/member_invitation_reminder_emails_worker.rb
+0
-2
changelogs/unreleased/cleanup-invitation-reminders-experiment.yml
...gs/unreleased/cleanup-invitation-reminders-experiment.yml
+5
-0
doc/user/project/members/index.md
doc/user/project/members/index.md
+3
-0
lib/gitlab/experimentation.rb
lib/gitlab/experimentation.rb
+0
-4
spec/controllers/invites_controller_spec.rb
spec/controllers/invites_controller_spec.rb
+0
-39
spec/lib/gitlab/experimentation_spec.rb
spec/lib/gitlab/experimentation_spec.rb
+0
-1
spec/mailers/notify_spec.rb
spec/mailers/notify_spec.rb
+0
-32
spec/services/members/invitation_reminder_email_service_spec.rb
...ervices/members/invitation_reminder_email_service_spec.rb
+39
-57
spec/workers/member_invitation_reminder_emails_worker_spec.rb
.../workers/member_invitation_reminder_emails_worker_spec.rb
+4
-22
No files found.
app/controllers/invites_controller.rb
View file @
f9623ecc
...
...
@@ -20,7 +20,6 @@ class InvitesController < ApplicationController
def
accept
if
member
.
accept_invite!
(
current_user
)
track_invitation_reminders_experiment
(
'accepted'
)
redirect_to
invite_details
[
:path
],
notice:
_
(
"You have been granted %{member_human_access} access to %{title} %{name}."
)
%
{
member_human_access:
member
.
human_access
,
title:
invite_details
[
:title
],
name:
invite_details
[
:name
]
}
else
...
...
@@ -107,8 +106,4 @@ class InvitesController < ApplicationController
}
end
end
def
track_invitation_reminders_experiment
(
action
)
track_experiment_event
(
:invitation_reminders
,
action
,
subject:
member
)
end
end
app/mailers/emails/members.rb
View file @
f9623ecc
...
...
@@ -63,15 +63,6 @@ module Emails
subject:
subject_line
,
layout:
'unknown_user_mailer'
)
if
Gitlab
::
Experimentation
.
active?
(
:invitation_reminders
)
Gitlab
::
Tracking
.
event
(
Gitlab
::
Experimentation
.
get_experiment
(
:invitation_reminders
).
tracking_category
,
'sent'
,
property:
Gitlab
::
Experimentation
.
in_experiment_group?
(
:invitation_reminders
,
subject:
member
.
invite_email
)
?
'experimental_group'
:
'control_group'
,
label:
Digest
::
MD5
.
hexdigest
(
member
.
to_global_id
.
to_s
)
)
end
end
def
member_invited_reminder_email
(
member_source_type
,
member_id
,
token
,
reminder_index
)
...
...
app/services/members/invitation_reminder_email_service.rb
View file @
f9623ecc
...
...
@@ -14,8 +14,6 @@ module Members
end
def
execute
return
unless
experiment_enabled?
reminder_index
=
days_on_which_to_send_reminders
.
index
(
days_after_invitation_sent
)
return
unless
reminder_index
...
...
@@ -24,10 +22,6 @@ module Members
private
def
experiment_enabled?
Gitlab
::
Experimentation
.
in_experiment_group?
(
:invitation_reminders
,
subject:
invitation
.
invite_email
)
end
def
days_after_invitation_sent
(
Date
.
today
-
invitation
.
created_at
.
to_date
).
to_i
end
...
...
app/workers/member_invitation_reminder_emails_worker.rb
View file @
f9623ecc
...
...
@@ -8,8 +8,6 @@ class MemberInvitationReminderEmailsWorker # rubocop:disable Scalability/Idempot
urgency
:low
def
perform
return
unless
Gitlab
::
Experimentation
.
active?
(
:invitation_reminders
)
Member
.
not_accepted_invitations
.
not_expired
.
last_ten_days_excluding_today
.
find_in_batches
do
|
invitations
|
invitations
.
each
do
|
invitation
|
Members
::
InvitationReminderEmailService
.
new
(
invitation
).
execute
...
...
changelogs/unreleased/cleanup-invitation-reminders-experiment.yml
0 → 100644
View file @
f9623ecc
---
title
:
Add invitation reminders
merge_request
:
47920
author
:
type
:
added
doc/user/project/members/index.md
View file @
f9623ecc
...
...
@@ -96,6 +96,9 @@ invitation, change their access level, or even delete them.
![
Invite user members list
](
img/add_user_email_accept.png
)
While unaccepted, the system will automatically send reminder emails on the second, fifth,
and tenth day after the invitation was initially sent.
After the user accepts the invitation, they are prompted to create a new
GitLab account using the same e-mail address the invitation was sent to.
...
...
lib/gitlab/experimentation.rb
View file @
f9623ecc
...
...
@@ -66,10 +66,6 @@ module Gitlab
tracking_category:
'Growth::Acquisition::Experiment::InviteEmail'
,
use_backwards_compatible_subject_index:
true
},
invitation_reminders:
{
tracking_category:
'Growth::Acquisition::Experiment::InvitationReminders'
,
use_backwards_compatible_subject_index:
true
},
group_only_trials:
{
tracking_category:
'Growth::Conversion::Experiment::GroupOnlyTrials'
,
use_backwards_compatible_subject_index:
true
...
...
spec/controllers/invites_controller_spec.rb
View file @
f9623ecc
...
...
@@ -22,43 +22,6 @@ RSpec.describe InvitesController, :snowplow do
end
end
shared_examples
"tracks the 'accepted' event for the invitation reminders experiment"
do
before
do
stub_experiment
(
invitation_reminders:
true
)
stub_experiment_for_subject
(
invitation_reminders:
experimental_group
)
end
context
'when in the control group'
do
let
(
:experimental_group
)
{
false
}
it
"tracks the 'accepted' event"
do
request
expect_snowplow_event
(
category:
'Growth::Acquisition::Experiment::InvitationReminders'
,
label:
md5_member_global_id
,
property:
'control_group'
,
action:
'accepted'
)
end
end
context
'when in the experimental group'
do
let
(
:experimental_group
)
{
true
}
it
"tracks the 'accepted' event"
do
request
expect_snowplow_event
(
category:
'Growth::Acquisition::Experiment::InvitationReminders'
,
label:
md5_member_global_id
,
property:
'experimental_group'
,
action:
'accepted'
)
end
end
end
describe
'GET #show'
do
subject
(
:request
)
{
get
:show
,
params:
params
}
...
...
@@ -87,7 +50,6 @@ RSpec.describe InvitesController, :snowplow do
expect
(
flash
[
:notice
]).
to
be_nil
end
it_behaves_like
"tracks the 'accepted' event for the invitation reminders experiment"
it_behaves_like
'invalid token'
end
...
...
@@ -119,7 +81,6 @@ RSpec.describe InvitesController, :snowplow do
subject
(
:request
)
{
post
:accept
,
params:
params
}
it_behaves_like
"tracks the 'accepted' event for the invitation reminders experiment"
it_behaves_like
'invalid token'
end
...
...
spec/lib/gitlab/experimentation_spec.rb
View file @
f9623ecc
...
...
@@ -16,7 +16,6 @@ RSpec.describe Gitlab::Experimentation::EXPERIMENTS do
:contact_sales_btn_in_app
,
:customize_homepage
,
:invite_email
,
:invitation_reminders
,
:group_only_trials
,
:default_to_issues_board
]
...
...
spec/mailers/notify_spec.rb
View file @
f9623ecc
...
...
@@ -1450,38 +1450,6 @@ RSpec.describe Notify do
subject
{
described_class
.
member_invited_email
(
'Group'
,
group_member
.
id
,
group_member
.
invite_token
)
}
shared_examples
"tracks the 'sent' event for the invitation reminders experiment"
do
before
do
stub_experiment
(
invitation_reminders:
true
)
allow
(
Gitlab
::
Experimentation
).
to
receive
(
:in_experiment_group?
).
with
(
:invitation_reminders
,
subject:
group_member
.
invite_email
).
and_return
(
experimental_group
)
end
it
"tracks the 'sent' event"
,
:snowplow
do
subject
.
deliver_now
expect_snowplow_event
(
category:
'Growth::Acquisition::Experiment::InvitationReminders'
,
label:
Digest
::
MD5
.
hexdigest
(
group_member
.
to_global_id
.
to_s
),
property:
experimental_group
?
'experimental_group'
:
'control_group'
,
action:
'sent'
)
end
end
describe
'tracking for the invitation reminders experiment'
do
context
'when invite email is in the experimental group'
do
let
(
:experimental_group
)
{
true
}
it_behaves_like
"tracks the 'sent' event for the invitation reminders experiment"
end
context
'when invite email is in the control group'
do
let
(
:experimental_group
)
{
false
}
it_behaves_like
"tracks the 'sent' event for the invitation reminders experiment"
end
end
it_behaves_like
'an email sent from GitLab'
it_behaves_like
'it should not have Gmail Actions links'
it_behaves_like
"a user cannot unsubscribe through footer link"
...
...
spec/services/members/invitation_reminder_email_service_spec.rb
View file @
f9623ecc
...
...
@@ -9,67 +9,49 @@ RSpec.describe Members::InvitationReminderEmailService do
let_it_be
(
:frozen_time
)
{
Date
.
today
.
beginning_of_day
}
let_it_be
(
:invitation
)
{
build
(
:group_member
,
:invited
,
created_at:
frozen_time
)
}
context
'when the experiment is disabled'
do
before
do
allow
(
Gitlab
::
Experimentation
).
to
receive
(
:in_experiment_group?
).
and_return
(
false
)
invitation
.
expires_at
=
frozen_time
+
2
.
days
end
it
'does not send an invitation'
do
travel_to
(
frozen_time
+
1
.
day
)
do
expect
(
invitation
).
not_to
receive
(
:send_invitation_reminder
)
subject
end
end
before
do
invitation
.
expires_at
=
frozen_time
+
expires_at_days
.
days
if
expires_at_days
end
context
'when the experiment is enabled'
do
before
do
allow
(
Gitlab
::
Experimentation
).
to
receive
(
:in_experiment_group?
).
and_return
(
true
)
invitation
.
expires_at
=
frozen_time
+
expires_at_days
.
days
if
expires_at_days
end
using
RSpec
::
Parameterized
::
TableSyntax
where
(
:expires_at_days
,
:send_reminder_at_days
)
do
0
|
[]
1
|
[]
2
|
[
1
]
3
|
[
1
,
2
]
4
|
[
1
,
2
,
3
]
5
|
[
1
,
2
,
4
]
6
|
[
1
,
3
,
5
]
7
|
[
1
,
3
,
5
]
8
|
[
2
,
3
,
6
]
9
|
[
2
,
4
,
7
]
10
|
[
2
,
4
,
8
]
11
|
[
2
,
4
,
8
]
12
|
[
2
,
5
,
9
]
13
|
[
2
,
5
,
10
]
14
|
[
2
,
5
,
10
]
15
|
[
2
,
5
,
10
]
nil
|
[
2
,
5
,
10
]
end
with_them
do
# Create an invitation today with an expiration date from 0 to 10 days in the future or without an expiration date
# We chose 10 days here, because we fetch invitations that were created at most 10 days ago.
(
0
..
10
).
each
do
|
day
|
it
'sends an invitation reminder only on the expected days'
do
next
if
day
>
(
expires_at_days
||
10
)
# We don't need to test after the invitation has already expired
# We are traveling in a loop from today to 10 days from now
travel_to
(
frozen_time
+
day
.
days
)
do
# Given an expiration date and the number of days after the creation of the invitation based on the current day in the loop, a reminder may be sent
if
(
reminder_index
=
send_reminder_at_days
.
index
(
day
))
expect
(
invitation
).
to
receive
(
:send_invitation_reminder
).
with
(
reminder_index
)
else
expect
(
invitation
).
not_to
receive
(
:send_invitation_reminder
)
end
using
RSpec
::
Parameterized
::
TableSyntax
where
(
:expires_at_days
,
:send_reminder_at_days
)
do
0
|
[]
1
|
[]
2
|
[
1
]
3
|
[
1
,
2
]
4
|
[
1
,
2
,
3
]
5
|
[
1
,
2
,
4
]
6
|
[
1
,
3
,
5
]
7
|
[
1
,
3
,
5
]
8
|
[
2
,
3
,
6
]
9
|
[
2
,
4
,
7
]
10
|
[
2
,
4
,
8
]
11
|
[
2
,
4
,
8
]
12
|
[
2
,
5
,
9
]
13
|
[
2
,
5
,
10
]
14
|
[
2
,
5
,
10
]
15
|
[
2
,
5
,
10
]
nil
|
[
2
,
5
,
10
]
end
subject
with_them
do
# Create an invitation today with an expiration date from 0 to 10 days in the future or without an expiration date
# We chose 10 days here, because we fetch invitations that were created at most 10 days ago.
(
0
..
10
).
each
do
|
day
|
it
'sends an invitation reminder only on the expected days'
do
next
if
day
>
(
expires_at_days
||
10
)
# We don't need to test after the invitation has already expired
# We are traveling in a loop from today to 10 days from now
travel_to
(
frozen_time
+
day
.
days
)
do
# Given an expiration date and the number of days after the creation of the invitation based on the current day in the loop, a reminder may be sent
if
(
reminder_index
=
send_reminder_at_days
.
index
(
day
))
expect
(
invitation
).
to
receive
(
:send_invitation_reminder
).
with
(
reminder_index
)
else
expect
(
invitation
).
not_to
receive
(
:send_invitation_reminder
)
end
subject
end
end
end
...
...
spec/workers/member_invitation_reminder_emails_worker_spec.rb
View file @
f9623ecc
...
...
@@ -10,30 +10,12 @@ RSpec.describe MemberInvitationReminderEmailsWorker do
create
(
:group_member
,
:invited
,
created_at:
2
.
days
.
ago
)
end
context
'feature flag disabled
'
do
before
do
stub_experiment
(
invitation_reminders:
fals
e
)
it
'executes the invitation reminder email service
'
do
expect_next_instance_of
(
Members
::
InvitationReminderEmailService
)
do
|
service
|
expect
(
service
).
to
receive
(
:execut
e
)
end
it
'does not attempt to execute the invitation reminder service'
do
expect
(
Members
::
InvitationReminderEmailService
).
not_to
receive
(
:new
)
subject
end
end
context
'feature flag enabled'
do
before
do
stub_experiment
(
invitation_reminders:
true
)
end
it
'executes the invitation reminder email service'
do
expect_next_instance_of
(
Members
::
InvitationReminderEmailService
)
do
|
service
|
expect
(
service
).
to
receive
(
:execute
)
end
subject
end
subject
end
end
end
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment