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
71b171fe
Commit
71b171fe
authored
Mar 10, 2021
by
Doug Stull
Committed by
Paul Slaughter
Mar 10, 2021
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Remove gon.global experiment scoping and organize helpers
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55421
parent
1142bacb
Changes
14
Show whitespace changes
Inline
Side-by-side
Showing
14 changed files
with
134 additions
and
93 deletions
+134
-93
app/assets/javascripts/experimentation/constants.js
app/assets/javascripts/experimentation/constants.js
+1
-0
app/assets/javascripts/experimentation/experiment_tracking.js
...assets/javascripts/experimentation/experiment_tracking.js
+5
-6
app/assets/javascripts/experimentation/utils.js
app/assets/javascripts/experimentation/utils.js
+10
-0
app/assets/javascripts/lib/utils/experimentation.js
app/assets/javascripts/lib/utils/experimentation.js
+0
-3
app/assets/javascripts/projects/upload_file_experiment.js
app/assets/javascripts/projects/upload_file_experiment.js
+1
-1
app/assets/javascripts/tracking.js
app/assets/javascripts/tracking.js
+5
-3
app/experiments/application_experiment.rb
app/experiments/application_experiment.rb
+9
-1
doc/development/experiment_guide/index.md
doc/development/experiment_guide/index.md
+3
-11
spec/experiments/application_experiment_spec.rb
spec/experiments/application_experiment_spec.rb
+27
-13
spec/frontend/experimentation/experiment_tracking_spec.js
spec/frontend/experimentation/experiment_tracking_spec.js
+13
-13
spec/frontend/experimentation/utils_spec.js
spec/frontend/experimentation/utils_spec.js
+38
-0
spec/frontend/lib/utils/experimentation_spec.js
spec/frontend/lib/utils/experimentation_spec.js
+0
-20
spec/frontend/projects/upload_file_experiment_spec.js
spec/frontend/projects/upload_file_experiment_spec.js
+6
-12
spec/frontend/tracking_spec.js
spec/frontend/tracking_spec.js
+16
-10
No files found.
app/assets/javascripts/experimentation/constants.js
0 → 100644
View file @
71b171fe
export
const
TRACKING_CONTEXT_SCHEMA
=
'
iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0
'
;
app/assets/javascripts/experiment_tracking.js
→
app/assets/javascripts/experiment
ation/experiment
_tracking.js
View file @
71b171fe
import
{
get
}
from
'
lodash
'
;
import
Tracking
from
'
~/tracking
'
;
const
TRACKING_CONTEXT_SCHEMA
=
'
iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0
'
;
import
{
TRACKING_CONTEXT_SCHEMA
}
from
'
./constants
'
;
import
{
getExperimentData
}
from
'
./utils
'
;
export
default
class
ExperimentTracking
{
constructor
(
experimentName
,
trackingArgs
=
{})
{
this
.
trackingArgs
=
trackingArgs
;
this
.
experimentData
=
get
(
window
,
[
'
gon
'
,
'
global
'
,
'
experiment
'
,
experimentName
]
);
this
.
data
=
getExperimentData
(
experimentName
);
}
event
(
action
)
{
if
(
!
this
.
experimentD
ata
)
{
if
(
!
this
.
d
ata
)
{
return
false
;
}
...
...
@@ -18,7 +17,7 @@ export default class ExperimentTracking {
...
this
.
trackingArgs
,
context
:
{
schema
:
TRACKING_CONTEXT_SCHEMA
,
data
:
this
.
experimentD
ata
,
data
:
this
.
d
ata
,
},
});
}
...
...
app/assets/javascripts/experimentation/utils.js
0 → 100644
View file @
71b171fe
// This file only applies to use of experiments through https://gitlab.com/gitlab-org/gitlab-experiment
import
{
get
}
from
'
lodash
'
;
export
function
getExperimentData
(
experimentName
)
{
return
get
(
window
,
[
'
gon
'
,
'
experiment
'
,
experimentName
]);
}
export
function
isExperimentVariant
(
experimentName
,
variantName
)
{
return
getExperimentData
(
experimentName
)?.
variant
===
variantName
;
}
app/assets/javascripts/lib/utils/experimentation.js
deleted
100644 → 0
View file @
1142bacb
export
function
isExperimentEnabled
(
experimentKey
)
{
return
Boolean
(
window
.
gon
?.
experiments
?.[
experimentKey
]);
}
app/assets/javascripts/projects/upload_file_experiment.js
View file @
71b171fe
import
ExperimentTracking
from
'
~/experiment_tracking
'
;
import
ExperimentTracking
from
'
~/experiment
ation/experiment
_tracking
'
;
function
trackEvent
(
eventName
)
{
const
isEmpty
=
Boolean
(
document
.
querySelector
(
'
.project-home-panel.empty-project
'
));
...
...
app/assets/javascripts/tracking.js
View file @
71b171fe
import
{
omitBy
,
isUndefined
,
get
}
from
'
lodash
'
;
import
{
omitBy
,
isUndefined
}
from
'
lodash
'
;
import
{
TRACKING_CONTEXT_SCHEMA
}
from
'
~/experimentation/constants
'
;
import
{
getExperimentData
}
from
'
~/experimentation/utils
'
;
const
standardContext
=
{
...
window
.
gl
?.
snowplowStandardContext
};
...
...
@@ -32,8 +34,8 @@ const createEventPayload = (el, { suffix = '' } = {}) => {
let
context
=
el
.
dataset
.
trackContext
;
if
(
el
.
dataset
.
trackExperiment
)
{
const
data
=
get
(
window
,
[
'
gon
'
,
'
global
'
,
'
experiment
'
,
el
.
dataset
.
trackExperiment
]
);
if
(
data
)
context
=
{
schema
:
'
iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0
'
,
data
};
const
data
=
get
ExperimentData
(
el
.
dataset
.
trackExperiment
);
if
(
data
)
context
=
{
schema
:
TRACKING_CONTEXT_SCHEMA
,
data
};
}
const
data
=
{
...
...
app/experiments/application_experiment.rb
View file @
71b171fe
...
...
@@ -11,7 +11,9 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp
def
publish
(
_result
)
track
(
:assignment
)
# track that we've assigned a variant for this context
Gon
.
global
.
push
({
experiment:
{
name
=>
signature
}
},
true
)
# push the experiment data to the client
# push the experiment data to the client
Gon
.
push
({
experiment:
{
name
=>
signature
}
},
true
)
if
in_request_cycle?
end
def
track
(
action
,
**
event_args
)
...
...
@@ -47,6 +49,12 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp
name
.
tr
(
'/'
,
'_'
)
end
def
in_request_cycle?
# Gon is only accessible when having a request. This will be fixed with
# https://gitlab.com/gitlab-org/gitlab/-/issues/323352
context
.
instance_variable_defined?
(
:@request
)
end
def
resolve_variant_name
case
rollout_strategy
when
:round_robin
...
...
doc/development/experiment_guide/index.md
View file @
71b171fe
...
...
@@ -140,15 +140,8 @@ You find out how to conduct experiments using `gitlab-experiment` in the [README
The above checks whether the experiment is enabled and pushes the result to the frontend.
You can check the state of the feature flag in JavaScript:
```javascript
import { isExperimentEnabled } from '~/experimentation';
if ( isExperimentEnabled('signupFlow') ) {
// ...
}
```
The Frontend helpers for this are no longer used in production.
[More details TBD](https://gitlab.com/gitlab-org/gitlab/-/issues/323934).
-
It is also possible to run an experiment outside of the controller scope, for example in a worker:
...
...
@@ -238,11 +231,10 @@ expect(Gon.tracking_data).to eq(
Which can then be used for tracking as follows:
```
javascript
import
{
isExperimentEnabled
}
from
'
~/lib/utils/experimentation
'
;
import
Tracking
from
'
~/tracking
'
;
document
.
addEventListener
(
'
DOMContentLoaded
'
,
()
=>
{
const
signupFlowExperimentEnabled
=
isExperimentEnabled
(
'
signupFlow
'
)
;
const
signupFlowExperimentEnabled
=
gon
.
experiments
[
'
signupFlow
'
]
;
if
(
signupFlowExperimentEnabled
&&
gon
.
tracking_data
)
{
const
{
category
,
action
,
...
data
}
=
gon
.
tracking_data
;
...
...
spec/experiments/application_experiment_spec.rb
View file @
71b171fe
...
...
@@ -64,8 +64,13 @@ RSpec.describe ApplicationExperiment, :experiment do
subject
.
publish
(
nil
)
end
it
"pushes the experiment knowledge into the client using Gon.global"
do
expect
(
Gon
.
global
).
to
receive
(
:push
).
with
(
context
"when inside a request cycle"
do
before
do
subject
.
context
.
instance_variable_set
(
:@request
,
double
(
'Request'
,
headers:
'true'
))
end
it
"pushes the experiment knowledge into the client using Gon"
do
expect
(
Gon
).
to
receive
(
:push
).
with
(
{
experiment:
{
'namespaced/stub'
=>
{
# string key because it can be namespaced
...
...
@@ -82,6 +87,15 @@ RSpec.describe ApplicationExperiment, :experiment do
end
end
context
"when outside a request cycle"
do
it
"does not push to gon when outside request cycle"
do
expect
(
Gon
).
not_to
receive
(
:push
)
subject
.
publish
(
nil
)
end
end
end
it
"can exclude from within the block"
do
expect
(
described_class
.
new
(
'namespaced/stub'
)
{
|
e
|
e
.
exclude!
}).
to
be_excluded
end
...
...
spec/frontend/experiment_tracking_spec.js
→
spec/frontend/experiment
ation/experiment
_tracking_spec.js
View file @
71b171fe
import
ExperimentTracking
from
'
~/experiment_tracking
'
;
import
{
TRACKING_CONTEXT_SCHEMA
}
from
'
~/experimentation/constants
'
;
import
ExperimentTracking
from
'
~/experimentation/experiment_tracking
'
;
import
{
getExperimentData
}
from
'
~/experimentation/utils
'
;
import
Tracking
from
'
~/tracking
'
;
jest
.
mock
(
'
~/tracking
'
);
const
oldGon
=
window
.
gon
;
let
newGon
=
{};
let
experimentTracking
;
let
label
;
let
property
;
jest
.
mock
(
'
~/tracking
'
);
jest
.
mock
(
'
~/experimentation/utils
'
,
()
=>
({
getExperimentData
:
jest
.
fn
()
}));
const
setup
=
()
=>
{
window
.
gon
=
newGon
;
experimentTracking
=
new
ExperimentTracking
(
'
sidebar_experiment
'
,
{
label
,
property
});
};
...
...
@@ -20,16 +19,18 @@ beforeEach(() => {
});
afterEach
(()
=>
{
window
.
gon
=
oldGon
;
Tracking
.
mockClear
();
label
=
undefined
;
property
=
undefined
;
});
describe
(
'
event
'
,
()
=>
{
beforeEach
(()
=>
{
getExperimentData
.
mockReturnValue
(
undefined
);
});
describe
(
'
when experiment data exists for experimentName
'
,
()
=>
{
beforeEach
(()
=>
{
newGon
=
{
global
:
{
experiment
:
{
sidebar_experiment
:
'
experiment-data
'
}
}
}
;
getExperimentData
.
mockReturnValue
(
'
experiment-data
'
)
;
setup
();
});
...
...
@@ -45,7 +46,7 @@ describe('event', () => {
label
:
'
sidebar-drawer
'
,
property
:
'
dark-mode
'
,
context
:
{
schema
:
'
iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0
'
,
schema
:
TRACKING_CONTEXT_SCHEMA
,
data
:
'
experiment-data
'
,
},
});
...
...
@@ -58,7 +59,7 @@ describe('event', () => {
expect
(
Tracking
.
event
).
toHaveBeenCalledTimes
(
1
);
expect
(
Tracking
.
event
).
toHaveBeenCalledWith
(
'
issues-page
'
,
'
click_sidebar_trigger
'
,
{
context
:
{
schema
:
'
iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0
'
,
schema
:
TRACKING_CONTEXT_SCHEMA
,
data
:
'
experiment-data
'
,
},
});
...
...
@@ -67,7 +68,6 @@ describe('event', () => {
describe
(
'
when experiment data does NOT exists for the experimentName
'
,
()
=>
{
beforeEach
(()
=>
{
newGon
=
{
global
:
{
experiment
:
{
unrelated_experiment
:
'
not happening
'
}
}
};
setup
();
});
...
...
spec/frontend/experimentation/utils_spec.js
0 → 100644
View file @
71b171fe
import
*
as
experimentUtils
from
'
~/experimentation/utils
'
;
const
TEST_KEY
=
'
abc
'
;
describe
(
'
experiment Utilities
'
,
()
=>
{
const
oldGon
=
window
.
gon
;
afterEach
(()
=>
{
window
.
gon
=
oldGon
;
});
describe
(
'
getExperimentData
'
,
()
=>
{
it
.
each
`
gon | input | output
${{
experiment
:
{
[
TEST_KEY
]:
'
_data_
'
}
}} |
${[
TEST_KEY
]}
|
${
'
_data_
'
}
${{}}
|
$
{[
TEST_KEY
]}
|
${
undefined
}
`
(
'
with input=$input and gon=$gon, returns $output
'
,
({
gon
,
input
,
output
})
=>
{
window
.
gon
=
gon
;
expect
(
experimentUtils
.
getExperimentData
(...
input
)).
toEqual
(
output
);
});
});
describe
(
'
isExperimentVariant
'
,
()
=>
{
it
.
each
`
gon | input | output
${{
experiment
:
{
[
TEST_KEY
]:
{
variant
:
'
control
'
}
} }} |
${[
TEST_KEY
,
'
control
'
]}
|
${
true
}
${{
experiment
:
{
[
TEST_KEY
]:
{
variant
:
'
_variant_name
'
}
} }} |
${[
TEST_KEY
,
'
_variant_name
'
]}
|
${
true
}
${{
experiment
:
{
[
TEST_KEY
]:
{
variant
:
'
_variant_name
'
}
} }} |
${[
TEST_KEY
,
'
_bogus_name
'
]}
|
${
false
}
${{
experiment
:
{
[
TEST_KEY
]:
{
variant
:
'
_variant_name
'
}
} }} |
${[
'
boguskey
'
,
'
_variant_name
'
]}
|
${
false
}
${{}}
|
$
{[
TEST_KEY
,
'
_variant_name
'
]}
|
${
false
}
`
(
'
with input=$input and gon=$gon, returns $output
'
,
({
gon
,
input
,
output
})
=>
{
window
.
gon
=
gon
;
expect
(
experimentUtils
.
isExperimentVariant
(...
input
)).
toEqual
(
output
);
});
});
});
spec/frontend/lib/utils/experimentation_spec.js
deleted
100644 → 0
View file @
1142bacb
import
*
as
experimentUtils
from
'
~/lib/utils/experimentation
'
;
const
TEST_KEY
=
'
abc
'
;
describe
(
'
experiment Utilities
'
,
()
=>
{
describe
(
'
isExperimentEnabled
'
,
()
=>
{
it
.
each
`
experiments | value
${{
[
TEST_KEY
]:
true
}
} |
${
true
}
${{
[
TEST_KEY
]:
false
}
} |
${
false
}
${{
def
:
true
}
} |
${
false
}
${{}}
|
$
{
false
}
${
null
}
|
${
false
}
`
(
'
returns correct value of $value for experiments=$experiments
'
,
({
experiments
,
value
})
=>
{
window
.
gon
=
{
experiments
};
expect
(
experimentUtils
.
isExperimentEnabled
(
TEST_KEY
)).
toEqual
(
value
);
});
});
});
spec/frontend/projects/upload_file_experiment_spec.js
View file @
71b171fe
import
ExperimentTracking
from
'
~/experiment_tracking
'
;
import
ExperimentTracking
from
'
~/experiment
ation/experiment
_tracking
'
;
import
*
as
UploadFileExperiment
from
'
~/projects/upload_file_experiment
'
;
const
mockExperimentTrackingEvent
=
jest
.
fn
();
jest
.
mock
(
'
~/experiment_tracking
'
,
()
=>
jest
.
fn
().
mockImplementation
(()
=>
({
event
:
mockExperimentTrackingEvent
,
})),
);
jest
.
mock
(
'
~/experimentation/experiment_tracking
'
);
const
fixture
=
`<a class='js-upload-file-experiment-trigger' data-toggle='modal' data-target='#modal-upload-blob'></a><div id='modal-upload-blob'></div><div class='project-home-panel empty-project'></div>`
;
const
findModal
=
()
=>
document
.
querySelector
(
'
[aria-modal="true"]
'
);
const
findTrigger
=
()
=>
document
.
querySelector
(
'
.js-upload-file-experiment-trigger
'
);
beforeEach
(()
=>
{
ExperimentTracking
.
mockClear
();
mockExperimentTrackingEvent
.
mockClear
();
document
.
body
.
innerHTML
=
fixture
;
});
...
...
@@ -31,7 +23,9 @@ describe('trackUploadFileFormSubmitted', () => {
label
:
'
blob-upload-modal
'
,
property
:
'
empty
'
,
});
expect
(
mockExperimentTrackingEvent
).
toHaveBeenCalledWith
(
'
click_upload_modal_form_submit
'
);
expect
(
ExperimentTracking
.
prototype
.
event
).
toHaveBeenCalledWith
(
'
click_upload_modal_form_submit
'
,
);
});
it
(
'
initializes ExperimentTracking with the correct arguments when the project is not empty
'
,
()
=>
{
...
...
@@ -53,6 +47,6 @@ describe('initUploadFileTrigger', () => {
expect
(
findModal
()).
not
.
toExist
();
findTrigger
().
click
();
expect
(
findModal
()).
toExist
();
expect
(
mockExperimentTrackingE
vent
).
toHaveBeenCalledWith
(
'
click_upload_modal_trigger
'
);
expect
(
ExperimentTracking
.
prototype
.
e
vent
).
toHaveBeenCalledWith
(
'
click_upload_modal_trigger
'
);
});
});
spec/frontend/tracking_spec.js
View file @
71b171fe
import
{
setHTMLFixture
}
from
'
helpers/fixtures
'
;
import
{
TRACKING_CONTEXT_SCHEMA
}
from
'
~/experimentation/constants
'
;
import
{
getExperimentData
}
from
'
~/experimentation/utils
'
;
import
Tracking
,
{
initUserTracking
,
initDefaultTrackers
,
STANDARD_CONTEXT
}
from
'
~/tracking
'
;
jest
.
mock
(
'
~/experimentation/utils
'
,
()
=>
({
getExperimentData
:
jest
.
fn
()
}));
describe
(
'
Tracking
'
,
()
=>
{
let
snowplowSpy
;
let
bindDocumentSpy
;
let
trackLoadEventsSpy
;
beforeEach
(()
=>
{
getExperimentData
.
mockReturnValue
(
undefined
);
window
.
snowplow
=
window
.
snowplow
||
(()
=>
{});
window
.
snowplowOptions
=
{
namespace
:
'
_namespace_
'
,
...
...
@@ -245,18 +251,18 @@ describe('Tracking', () => {
});
it
(
'
brings in experiment data if linked to an experiment
'
,
()
=>
{
const
d
ata
=
{
const
mockExperimentD
ata
=
{
variant
:
'
candidate
'
,
experiment
:
'
repo_integrations_link
'
,
key
:
'
2bff73f6bb8cc11156c50a8ba66b9b8b
'
,
};
getExperimentData
.
mockReturnValue
(
mockExperimentData
);
window
.
gon
.
global
=
{
experiment
:
{
example
:
data
}
};
document
.
querySelector
(
'
[data-track-event="click_input3"]
'
).
click
();
expect
(
eventSpy
).
toHaveBeenCalledWith
(
'
_category_
'
,
'
click_input3
'
,
{
value
:
'
_value_
'
,
context
:
{
schema
:
'
iglu:com.gitlab/gitlab_experiment/jsonschema/1-0-0
'
,
d
ata
},
context
:
{
schema
:
TRACKING_CONTEXT_SCHEMA
,
data
:
mockExperimentD
ata
},
});
});
});
...
...
@@ -301,21 +307,21 @@ describe('Tracking', () => {
describe
(
'
tracking mixin
'
,
()
=>
{
describe
(
'
trackingOptions
'
,
()
=>
{
it
(
'
return the options defined on initialisation
'
,
()
=>
{
it
(
'
return
s
the options defined on initialisation
'
,
()
=>
{
const
mixin
=
Tracking
.
mixin
({
foo
:
'
bar
'
});
expect
(
mixin
.
computed
.
trackingOptions
()).
toEqual
({
foo
:
'
bar
'
});
});
it
(
'
local tracking value override and extend options
'
,
()
=>
{
it
(
'
l
ets l
ocal tracking value override and extend options
'
,
()
=>
{
const
mixin
=
Tracking
.
mixin
({
foo
:
'
bar
'
});
//
the value of this in the vue lifecyle is different, but this serve the test
s purposes
//
The value of this in the Vue lifecyle is different, but this serves the test'
s purposes
mixin
.
computed
.
tracking
=
{
foo
:
'
baz
'
,
baz
:
'
bar
'
};
expect
(
mixin
.
computed
.
trackingOptions
()).
toEqual
({
foo
:
'
baz
'
,
baz
:
'
bar
'
});
});
});
describe
(
'
trackingCategory
'
,
()
=>
{
it
(
'
return the category set in the component properties first
'
,
()
=>
{
it
(
'
return
s
the category set in the component properties first
'
,
()
=>
{
const
mixin
=
Tracking
.
mixin
({
category
:
'
foo
'
});
mixin
.
computed
.
tracking
=
{
category
:
'
bar
'
,
...
...
@@ -323,12 +329,12 @@ describe('Tracking', () => {
expect
(
mixin
.
computed
.
trackingCategory
()).
toBe
(
'
bar
'
);
});
it
(
'
return the category set in the options
'
,
()
=>
{
it
(
'
return
s
the category set in the options
'
,
()
=>
{
const
mixin
=
Tracking
.
mixin
({
category
:
'
foo
'
});
expect
(
mixin
.
computed
.
trackingCategory
()).
toBe
(
'
foo
'
);
});
it
(
'
if no category is selected returns undefin
ed
'
,
()
=>
{
it
(
'
returns undefined if no category is select
ed
'
,
()
=>
{
const
mixin
=
Tracking
.
mixin
();
expect
(
mixin
.
computed
.
trackingCategory
()).
toBe
(
undefined
);
});
...
...
@@ -363,7 +369,7 @@ describe('Tracking', () => {
expect
(
eventSpy
).
toHaveBeenCalledWith
(
undefined
,
'
foo
'
,
{});
});
it
(
'
give precedence to data for category and options
'
,
()
=>
{
it
(
'
give
s
precedence to data for category and options
'
,
()
=>
{
mixin
.
trackingCategory
=
mixin
.
trackingCategory
();
mixin
.
trackingOptions
=
mixin
.
trackingOptions
();
const
data
=
{
category
:
'
foo
'
,
label
:
'
baz
'
};
...
...
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