Skip to content

Commit

Permalink
Refactor Variant Detail presentation (#47)
Browse files Browse the repository at this point in the history
* split presenter goes away and VariantDetail handles view related details
  • Loading branch information
agirlnamedsophia committed Apr 13, 2017
1 parent 7344a4e commit 2fec08f
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 90 deletions.
2 changes: 1 addition & 1 deletion app/controllers/admin/splits_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ def index
end

def show
@split = SplitPresenter.new(Split.find(params[:id]))
@split = Split.find(params[:id])
end
end
12 changes: 12 additions & 0 deletions app/models/variant_detail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ def display_name
super || variant
end

def weight
@weight ||= split.variant_weight(variant)
end

def assignment_count
@assignment_count ||= split.assignment_count_for_variant(variant)
end

def retirable?
weight == 0 && assignment_count > 0
end

private

def variant_must_exist
Expand Down
8 changes: 0 additions & 8 deletions app/presenters/split_presenter.rb

This file was deleted.

13 changes: 0 additions & 13 deletions app/presenters/variant_presenter.rb

This file was deleted.

2 changes: 1 addition & 1 deletion app/views/admin/splits/_variants.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<th></th>
</thead>
<tbody>
<% split.variant_details.each do |variant_detail| %>
<% split.detail.variant_details.each do |variant_detail| %>
<tr>
<td><%= variant_detail.display_name %></td>
<td><%= variant_detail.description %></td>
Expand Down
63 changes: 62 additions & 1 deletion spec/models/variant_detail_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,68 @@
require 'rails_helper'

RSpec.describe VariantDetail do
let(:split) { FactoryGirl.create(:split, name: 'split_enabled', registry: { true: 50, false: 50 }) }
let(:split) { FactoryGirl.create(:split, name: "some_feature_enabled", registry: { true: 40, false: 60 }) }

describe "#weight" do
subject { described_class.new(split: split, variant: 'true') }

it "is the weight of the given variant" do
expect(subject.weight).to eq 40
end
end

describe "#assignment_count" do
let!(:true_assignment) { FactoryGirl.create(:assignment, split: split, variant: "true") }
let!(:false_assignment) { FactoryGirl.create_pair(:assignment, split: split, variant: "false") }

let(:true_presenter) { described_class.new(split: split, variant: 'true') }
let(:false_presenter) { described_class.new(split: split, variant: 'false') }

it "is the number of assignments of given variant" do
expect(true_presenter.assignment_count).to eq 1
expect(false_presenter.assignment_count).to eq 2
end
end

describe "#retirable?" do
subject { described_class.new(split: split, variant: 'true') }

context 'with a 0% weight' do
let(:split) { FactoryGirl.create(:split, name: "some_feature_enabled", registry: { true: 0, false: 100 }) }

context 'with no assignments' do
it "is false" do
expect(subject).not_to be_retirable
end
end

context 'with some assignments' do
let!(:assignment) { FactoryGirl.create(:assignment, split: split, variant: "true") }

it "is true" do
expect(subject).to be_retirable
end
end
end

context 'with a non0% weight' do
let(:split) { FactoryGirl.create(:split, name: "some_feature_enabled", registry: { true: 1, false: 99 }) }

context 'with no assignments' do
it "is false" do
expect(subject).not_to be_retirable
end
end

context 'with some assignments' do
let!(:assignment) { FactoryGirl.create(:assignment, split: split, variant: "true") }

it "is false for a non 0% weight that has assignments" do
expect(subject).not_to be_retirable
end
end
end
end

describe '#valid?' do
context 'with a variant that exists' do
Expand Down
66 changes: 0 additions & 66 deletions spec/models/variant_presenter_spec.rb

This file was deleted.

0 comments on commit 2fec08f

Please sign in to comment.