Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split Detail Remote Model #26

Merged
merged 13 commits into from Apr 10, 2017
5 changes: 5 additions & 0 deletions app/controllers/tt/api/v1/split_details_controller.rb
@@ -0,0 +1,5 @@
class Tt::Api::V1::SplitDetailsController < Tt::Api::V1::ApplicationController
def show
@split_detail = TestTrack::FakeServer.split_details
end
end
34 changes: 34 additions & 0 deletions app/models/test_track/fake/schema.rb
@@ -0,0 +1,34 @@
class TestTrack::Fake::Schema
include Singleton

def split_hash
@split_hash ||= _split_hash
end

private

def _split_hash
if test_track_schema_yml.present?
test_track_schema_yml[:splits]
else
{}
end
end

def test_track_schema_yml
unless instance_variable_defined?(:@test_track_schema_yml)
@test_track_schema_yml = _test_track_schema_yml
end
@test_track_schema_yml
end

def _test_track_schema_yml
YAML.load_file(test_track_schema_yml_path).with_indifferent_access
rescue
nil
end

def test_track_schema_yml_path
ENV["TEST_TRACK_SCHEMA_FILE_PATH"] || "#{Rails.root}/db/test_track_schema.yml"
end
end
14 changes: 14 additions & 0 deletions app/models/test_track/fake/split.rb
@@ -0,0 +1,14 @@
class TestTrack::Fake::Split
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is actually SplitDetail, right?

include Singleton

def split_details
@split_details ||= _split_details
end

private

def _split_details
split_hash = TestTrack::Fake::Schema.instance.split_hash
{ "name" => "banner_color" }.merge(split_hash["banner_color"].except("weights"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

going along with the comment i left elsewhere about not putting the split details in the schema.yml file, i think that we should just have this return fake data just like we do in the fake_instance_attributes method.

i am also noticing that this fake doesn't include any of the details about the variants in this split. is that an intentional omission?

end
end
31 changes: 4 additions & 27 deletions app/models/test_track/fake/split_registry.rb
Expand Up @@ -17,37 +17,14 @@ def splits

private

def split_hash
if test_track_schema_yml.present?
test_track_schema_yml[:splits]
else
{}
end
end

def test_track_schema_yml
unless instance_variable_defined?(:@test_track_schema_yml)
@test_track_schema_yml = _test_track_schema_yml
end
@test_track_schema_yml
end

def _test_track_schema_yml
YAML.load_file(test_track_schema_yml_path).with_indifferent_access
rescue
nil
end

def test_track_schema_yml_path
ENV["TEST_TRACK_SCHEMA_FILE_PATH"] || "#{Rails.root}/db/test_track_schema.yml"
end

def splits_with_deterministic_weights
split_hash = TestTrack::Fake::Schema.instance.split_hash

split_hash.each_with_object({}) do |(split_name, weighting_registry), split_registry|
default_variant = weighting_registry.keys.sort.first
default_variant = weighting_registry["weights"].keys.sort.first

adjusted_weights = { default_variant => 100 }
weighting_registry.except(default_variant).keys.each do |variant|
weighting_registry["weights"].except(default_variant).keys.each do |variant|
adjusted_weights[variant] = 0
end

Expand Down
4 changes: 4 additions & 0 deletions app/models/test_track/fake_server.rb
Expand Up @@ -4,6 +4,10 @@ def split_registry
TestTrack::Fake::SplitRegistry.instance.splits
end

def split_details
TestTrack::Fake::Split.instance.split_details
end

def visitor
TestTrack::Fake::Visitor.instance
end
Expand Down
28 changes: 28 additions & 0 deletions app/models/test_track/remote/split_detail.rb
@@ -0,0 +1,28 @@
class TestTrack::Remote::SplitDetail
include TestTrack::RemoteModel

collection_path '/api/v1/split_details'

attributes :name, :hypothesis, :assignment_criteria, :description, :owner, :location, :platform

def self.from_name(name)
# TODO: FakeableHer needs to make this faking a feature of `get`
if faked?
new(fake_instance_attributes(nil))
else
get("/api/v1/split_details/#{name}")
end
end

def self.fake_instance_attributes(_)
{
name: "fake_split_name",
hypothesis: "fake hypothesis",
assignment_criteria: "fake criteria for everyone",
description: "fake but still good description",
owner: "fake owner",
location: "fake activity",
platform: "mobile"
}
end
end
1 change: 1 addition & 0 deletions app/views/tt/api/v1/split_details/show.json.jbuilder
@@ -0,0 +1 @@
json.(@split_detail, "name", "hypothesis", "assignment_criteria", "description", "owner", "location", "platform")
2 changes: 2 additions & 0 deletions config/routes.rb
Expand Up @@ -11,6 +11,8 @@

resources :visitors, only: :show

resource :split_detail, only: :show

resources :identifier_types, only: [], param: :name do
resources :identifiers, only: [], param: :value do
resource :visitor, only: :show, controller: 'identifier_visitors'
Expand Down
23 changes: 23 additions & 0 deletions spec/controllers/tt/api/v1/split_details_controller_spec.rb
@@ -0,0 +1,23 @@
require 'rails_helper'

RSpec.describe Tt::Api::V1::SplitDetailsController do
let(:response_json) { JSON.parse(response.body, symbolize_names: true) }

describe '#show' do
it 'returns fake split details' do
get :show, format: :json

expected_response = {
name: "banner_color",
hypothesis: "user will interact more with blue banner",
location: "home screen",
platform: "mobile",
owner: "mobile team",
assignment_criteria: "user has mobile app",
description: "banner test to see if users will interact more"
}

expect(response_json).to eq expected_response
end
end
end
24 changes: 19 additions & 5 deletions spec/dummy/db/test_track_schema.yml
@@ -1,9 +1,23 @@
---
splits:
buy_one_get_one_promotion_enabled:
'false': 50
'true': 50
weights:
'false': 50
'true': 50
hypothesis: "user will buy more hats with promotion"
location: "hat store"
platform: "desktop"
owner: "hat makers"
assignment_criteria: "user has bought one hat"
description: "promotion test to see if users will buy more hats"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this actually getting dumped into the schema.yml now? i don't think that it is and i don't think that it should be.

the schema.yml is only meant to track the critical details of splits, their variants, and the associated weighting. we don't want it to include any of the optional split detail data that is inputted through the admin ui.

banner_color:
blue: 34
red: 33
white: 33
weights:
blue: 34
red: 33
white: 33
hypothesis: "user will interact more with blue banner"
location: "home screen"
platform: "mobile"
owner: "mobile team"
assignment_criteria: "user has mobile app"
description: "banner test to see if users will interact more"
1 change: 1 addition & 0 deletions spec/models/test_track/fake/split_registry_spec.rb
Expand Up @@ -35,6 +35,7 @@
context 'when test_track_schema.yml does not exist' do
before do
allow(YAML).to receive(:load_file).with("#{Rails.root}/db/test_track_schema.yml").and_return(nil)
allow(TestTrack::Fake::Schema.instance).to receive(:split_hash).and_return({})
end

describe '#to_h' do
Expand Down
18 changes: 18 additions & 0 deletions spec/models/test_track/fake/split_spec.rb
@@ -0,0 +1,18 @@
require 'rails_helper'

RSpec.describe TestTrack::Fake::Split do
subject { Class.new(described_class).instance }

context 'when splits exist' do
describe '#split_details' do
it 'returns a hash of split details for a split' do
expect(subject.split_details["name"]).to eq "banner_color"
expect(subject.split_details["owner"]).to eq "mobile team"
expect(subject.split_details["platform"]).to eq "mobile"
expect(subject.split_details["location"]).to eq "home screen"
expect(subject.split_details["assignment_criteria"]).to eq "user has mobile app"
expect(subject.split_details["hypothesis"]).to eq "user will interact more with blue banner"
end
end
end
end
59 changes: 59 additions & 0 deletions spec/models/test_track/remote/split_detail_spec.rb
@@ -0,0 +1,59 @@
require 'rails_helper'

RSpec.describe TestTrack::Remote::SplitDetail do
before do
stub_request(:get, url)
.with(basic_auth: %w(dummy fakepassword))
.to_return(status: 200, body: {
name: "fake_split_name_from_server",
hypothesis: "hypothesis from a real server",
assignment_criteria: "criteria about real not fake users",
description: "description about a very real test",
owner: "best owner ever",
location: "the homepage above the fold",
platform: "mobile"
}.to_json)
end

describe ".find" do
let(:url) { "http://testtrack.dev/api/v1/split_details/fake_split_name_from_server" }
subject { described_class.find("fake_split_name_from_server") }

it "loads split details with fake instance attributes" do
expect(subject.name).to eq("fake_split_name")
expect(subject.hypothesis).to eq("fake hypothesis")
expect(subject.assignment_criteria).to eq("fake criteria for everyone")
expect(subject.description).to eq("fake but still good description")
expect(subject.owner).to eq("fake owner")
expect(subject.location).to eq("fake activity")
expect(subject.platform).to eq("mobile")
end

it "fetches attributes from the test track server when enabled" do
with_test_track_enabled do
expect(subject.name).to eq("fake_split_name_from_server")
expect(subject.hypothesis).to eq("hypothesis from a real server")
expect(subject.assignment_criteria).to eq("criteria about real not fake users")
expect(subject.description).to eq("description about a very real test")
expect(subject.owner).to eq("best owner ever")
expect(subject.location).to eq("the homepage above the fold")
expect(subject.platform).to eq("mobile")
end
end
end

describe ".from_name" do
subject { described_class.from_name("clown_id") }
let(:url) { "http://testtrack.dev/api/v1/split_details/clown_id" }

it "loads split details with instance attributes" do
expect(subject.name).to eq("fake_split_name")
end

it "fetches attributes from the test track server when enabled" do
with_test_track_enabled do
expect(subject.name).to eq("fake_split_name_from_server")
end
end
end
end