Commit 42be0ec7 authored by Dallas Reedy's avatar Dallas Reedy

Fix ApplicationExperiment#publish_to_database functionality

The expectation is that we can either call `record!` on an experiment to
have it automatically run `publish_to_database` as part of the normal
`publish` flow or we can explicitly call `publish_to_database` ourselves
to get the same behavior (without needing to first call `record!`).

This was broken by https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67366
parent bfd7e162
...@@ -13,7 +13,7 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp ...@@ -13,7 +13,7 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp
super super
publish_to_client publish_to_client
publish_to_database publish_to_database if @record
end end
def publish_to_client def publish_to_client
...@@ -25,7 +25,6 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp ...@@ -25,7 +25,6 @@ class ApplicationExperiment < Gitlab::Experiment # rubocop:disable Gitlab/Namesp
end end
def publish_to_database def publish_to_database
return unless @record
return unless should_track? return unless should_track?
# if the context contains a namespace, group, project, user, or actor # if the context contains a namespace, group, project, user, or actor
......
...@@ -80,6 +80,8 @@ RSpec.describe ApplicationExperiment, :experiment do ...@@ -80,6 +80,8 @@ RSpec.describe ApplicationExperiment, :experiment do
end end
it "publishes to the database if we've opted for that" do it "publishes to the database if we've opted for that" do
subject.record!
expect(subject).to receive(:publish_to_database) expect(subject).to receive(:publish_to_database)
subject.publish subject.publish
...@@ -121,6 +123,8 @@ RSpec.describe ApplicationExperiment, :experiment do ...@@ -121,6 +123,8 @@ RSpec.describe ApplicationExperiment, :experiment do
end end
describe '#publish_to_database' do describe '#publish_to_database' do
using RSpec::Parameterized::TableSyntax
shared_examples 'does not record to the database' do shared_examples 'does not record to the database' do
it 'does not create an experiment record' do it 'does not create an experiment record' do
expect { subject.publish_to_database }.not_to change(Experiment, :count) expect { subject.publish_to_database }.not_to change(Experiment, :count)
...@@ -131,55 +135,43 @@ RSpec.describe ApplicationExperiment, :experiment do ...@@ -131,55 +135,43 @@ RSpec.describe ApplicationExperiment, :experiment do
end end
end end
context 'when we explicitly request to record' do context 'when there is a usable subject' do
using RSpec::Parameterized::TableSyntax let(:context) { { context_key => context_value } }
before do where(:context_key, :context_value, :object_type) do
subject.record! :namespace | build(:namespace) | :namespace
:group | build(:namespace) | :namespace
:project | build(:project) | :project
:user | build(:user) | :user
:actor | build(:user) | :user
end end
context 'when there is a usable subject' do with_them do
let(:context) { { context_key => context_value } } it 'creates an experiment and experiment subject record' do
expect { subject.publish_to_database }.to change(Experiment, :count).by(1)
where(:context_key, :context_value, :object_type) do
:namespace | build(:namespace) | :namespace
:group | build(:namespace) | :namespace
:project | build(:project) | :project
:user | build(:user) | :user
:actor | build(:user) | :user
end
with_them do
it 'creates an experiment and experiment subject record' do
expect { subject.publish_to_database }.to change(Experiment, :count).by(1)
expect(Experiment.last.name).to eq('namespaced/stub') expect(Experiment.last.name).to eq('namespaced/stub')
expect(ExperimentSubject.last.send(object_type)).to eq(context[context_key]) expect(ExperimentSubject.last.send(object_type)).to eq(context[context_key])
end
end end
end end
end
context 'when there is not a usable subject' do context 'when there is not a usable subject' do
let(:context) { { context_key => context_value } } let(:context) { { context_key => context_value } }
where(:context_key, :context_value) do
:namespace | nil
:foo | :bar
end
with_them do where(:context_key, :context_value) do
include_examples 'does not record to the database' :namespace | nil
end :foo | :bar
end end
context 'but we should not track' do with_them do
let(:should_track) { false }
include_examples 'does not record to the database' include_examples 'does not record to the database'
end end
end end
context 'when we have not explicitly requested to record' do context 'but we should not track' do
let(:should_track) { false }
include_examples 'does not record to the database' include_examples 'does not record to the database'
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