From 9911cf24a3e2453da75c31595124046e2cbfc946 Mon Sep 17 00:00:00 2001
From: John Backus <jbackus@stanford.edu>
Date: Tue, 8 Sep 2015 10:45:29 -0700
Subject: [PATCH 01/21] Add rspec-its to gemspec

Currently devtools is adding the rspec-its dependency, but only for rubies
>= 2.1, so we add our own explicit dependency in our gemspec for earlier rubies
---
 blockscore.gemspec | 1 +
 1 file changed, 1 insertion(+)

diff --git a/blockscore.gemspec b/blockscore.gemspec
index 7425d4c..0754300 100644
--- a/blockscore.gemspec
+++ b/blockscore.gemspec
@@ -29,6 +29,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'bundler', '~> 1.0'
   spec.add_development_dependency 'simplecov', '~> 0'
   spec.add_development_dependency 'rspec', '~> 3'
+  spec.add_development_dependency 'rspec-its', '~> 1'
   spec.add_development_dependency 'webmock', '~> 1.21'
   spec.add_development_dependency 'faker', '~> 1.4'
   spec.add_development_dependency 'factory_girl', '~> 4.5'

From 0e68afae797da986f4b9ed9560b76378ab1c937f Mon Sep 17 00:00:00 2001
From: John Backus <jbackus@stanford.edu>
Date: Sat, 22 Aug 2015 19:28:12 -0700
Subject: [PATCH 02/21] Update FactoryGirl dependency to 4.5

---
 spec/factories.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/factories.rb b/spec/factories.rb
index 77209c3..c4ec804 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -302,8 +302,8 @@ def full_address
 
   # We can do this because the error type is determined by the
   # HTTP response code.
-  factory :blockscore_error, class: Hash, traits: [:resource] do
-    ignore do
+  factory :blockscore_error, :class => Hash, :traits => [:resource] do
+    transient do
       error_type 'api_error'
     end
 

From 760433a2c12a43813ffc819945c2dc82ad7907f3 Mon Sep 17 00:00:00 2001
From: John Backus <jbackus@stanford.edu>
Date: Sat, 22 Aug 2015 19:30:05 -0700
Subject: [PATCH 03/21] Refactor out different id string fakers to method

---
 spec/factories.rb | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/spec/factories.rb b/spec/factories.rb
index c4ec804..024656e 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -38,6 +38,10 @@ def full_address
   "#{street} #{city} #{country}"
 end
 
+def resource_id
+  Faker::Number.hexadecimal(24)
+end
+
 FactoryGirl.define do
   # Each response has this metadata so we define it as a trait
   trait :metadata do
@@ -199,15 +203,16 @@ def full_address
     address
     document
     details { build(:person_details) }
+
     question_sets do
-      rand(0..5).times.collect { Faker::Base.regexify(/\d{24}/) }
+      rand(0..5).times.collect { resource_id }
     end
   end
 
   # QuestionSet Factory
 
   factory :question_set_params, class: 'BlockScore::QuestionSet' do
-    person_id { Faker::Base.regexify(/\d{24}/) }
+    person_id { resource_id }
   end
 
   factory :question_set, class: 'BlockScore::QuestionSet' do
@@ -217,7 +222,7 @@ def full_address
     metadata
     timestamps
     testmode
-    person_id { Faker::Base.regexify(/\d{24}/) }
+    person_id { resource_id }
     score { rand * 100 }
     expired { false }
     time_limit { rand(120..360) }

From 553c52b30237f32025002739b3e60524496371dd Mon Sep 17 00:00:00 2001
From: John Backus <jbackus@stanford.edu>
Date: Sat, 22 Aug 2015 19:31:01 -0700
Subject: [PATCH 04/21] Add question_set_count option to person factory

Lets you specify how many question set ids should come with the
person resource you are creating
---
 spec/factories.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/spec/factories.rb b/spec/factories.rb
index 024656e..e90f496 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -190,6 +190,7 @@ def resource_id
 
   factory :person, class: 'BlockScore::Person' do
     skip_create
+    transient { question_sets_count 1 }
 
     object { 'person' }
     metadata
@@ -205,7 +206,9 @@ def resource_id
     details { build(:person_details) }
 
     question_sets do
-      rand(0..5).times.collect { resource_id }
+      question_sets_count.times.map do
+        resource_id
+      end
     end
   end
 

From 223a8a54a2538bb4ff7012718f1ab950936ae783 Mon Sep 17 00:00:00 2001
From: John Backus <jbackus@stanford.edu>
Date: Sat, 22 Aug 2015 19:32:01 -0700
Subject: [PATCH 05/21] Add fixes related to question sets

Update the collections with instance methods that proxy to the target
class, and lazy load embedded parent data in for the target.

fixes #36, #35, #34, #22
---
 .gitignore                                |   4 +-
 lib/blockscore/actions/retrieve.rb        |   6 +-
 lib/blockscore/base.rb                    |  22 ++-
 lib/blockscore/collection.rb              |  91 ++++++++-
 lib/blockscore/person.rb                  |   2 +-
 lib/blockscore/question_set.rb            |   9 +-
 spec/factories.rb                         |   6 +-
 spec/spec_helper.rb                       |   1 +
 spec/support/stubbed_request.rb           |   6 +
 spec/support/stubbed_response.rb          |   2 +-
 spec/unit/blockscore/actions_spec.rb      |   4 +-
 spec/unit/blockscore/person_spec.rb       |  13 ++
 spec/unit/blockscore/question_set_spec.rb | 226 ++++++++++++++++++----
 13 files changed, 326 insertions(+), 66 deletions(-)

diff --git a/.gitignore b/.gitignore
index e483dff..db2a87d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,5 @@
+refactor_notes.md
+
 # rcov generated
 coverage
 coverage.data
@@ -15,7 +17,7 @@ doc
 # jeweler generated
 pkg
 
-# Have editor/IDE/OS specific files you need to ignore? Consider using a global gitignore: 
+# Have editor/IDE/OS specific files you need to ignore? Consider using a global gitignore:
 #
 # * Create a file at ~/.gitignore
 # * Include files you want ignored
diff --git a/lib/blockscore/actions/retrieve.rb b/lib/blockscore/actions/retrieve.rb
index e0585af..383a036 100644
--- a/lib/blockscore/actions/retrieve.rb
+++ b/lib/blockscore/actions/retrieve.rb
@@ -11,8 +11,10 @@ module Actions
     # => #<BlockScore::Person:0x007fe39c424410>
     module Retrieve
       module ClassMethods
-        def retrieve(id)
-          get("#{endpoint}/#{id}", {})
+        def retrieve(id, options={})
+          fail ArgumentError if id.empty?
+          req = ->() { get("#{endpoint}/#{id}", options) }
+          new(&req)
         end
       end
 
diff --git a/lib/blockscore/base.rb b/lib/blockscore/base.rb
index b19d657..ad446a6 100644
--- a/lib/blockscore/base.rb
+++ b/lib/blockscore/base.rb
@@ -4,14 +4,30 @@ module BlockScore
   class Base
     extend Connection
 
-    attr_reader :attributes
 
-    def initialize(options = {})
+    def initialize(options = {}, &block)
+      @loaded = !(block)
+      @proc = block
       @attributes = options
     end
 
+    # potential issues here
+    def attributes
+      return @attributes if @loaded
+      force!
+      @attributes
+    end
+
+    def force!
+      r = @proc.call()
+      @attributes = r.attributes
+      @loaded = true
+      self
+    end
+
     def inspect
-      "#<#{self.class}:0x#{object_id.to_s(16)} JSON: " + JSON.pretty_generate(attributes)
+      str_attr = "JSON:#{ JSON.pretty_generate(attributes) }"
+      "#<#{self.class}:0x#{object_id.to_s(16)} #{str_attr}>"
     end
 
     def refresh
diff --git a/lib/blockscore/collection.rb b/lib/blockscore/collection.rb
index e81284d..db4ddd9 100644
--- a/lib/blockscore/collection.rb
+++ b/lib/blockscore/collection.rb
@@ -1,18 +1,95 @@
 module BlockScore
   class Collection < Array
-    def initialize(target)
-      super()
-      @target = target
+
+    attr_reader :default_params
+    protected :default_params
+
+    attr_reader :parent, :target, :data
+
+
+    def initialize(params)
+      @parent = params.fetch :parent
+      @target = params.fetch :target
+      register_parent_data
+    end
+
+    def default_params
+      {
+        :"#{parent_name}_id" => parent_id
+      }
+    end
+
+    def parent_id
+      parent.id if parent.respond_to?(:id)
+    end
+
+    def all
+      self
+    end
+
+    def new(params ={})
+      item = target.new(params.merge(default_params))
+      _self = self
+      item.define_singleton_method(:save) do
+        _self.parent.save unless _self.parent_id
+        send :"#{_self.parent_name}_id=", _self.parent_id
+        super()
+      end
+      self << item
+      item
+    end
+
+    def refresh
+      clear
+      register_parent_data
+      self
     end
 
-    def respond_to?(method, include_all = false)
-      @target.respond_to?(method, include_all)
+    def parent_name
+      parent.class.resource
+    end
+
+    def target_name
+      target.resource
+    end
+
+    def create(params = {})
+      fail Error, "Create parent first" unless parent_id
+      assoc_params = default_params.merge(params)
+      item = target.create assoc_params
+      @data << item.id
+      self << item
+      item
+    end
+
+    def retrieve(id)
+      return self[data.index(id)] if data.include? id
+      item = target.retrieve(id)
+      register_to_parent(item)
     end
 
     private
 
-    def method_missing(method, *args, &block)
-      @target.public_send(method, *args, &block)
+    def has_parent_id(item)
+      parent_id && item.send(:"#{parent_name}_id") == parent_id
     end
+
+    def register_to_parent(item)
+      fail Error, "None belonging" unless has_parent_id(item)
+      data << item.id
+      self << item
+      item
+    end
+
+    def register_parent_data
+      @data = parent.attributes.fetch( :"#{ Util.to_plural(target_name) }", [])
+
+      @data.each do |id|
+        item = target.retrieve(id)
+        self << item
+      end
+    end
+
+
   end
 end
diff --git a/lib/blockscore/person.rb b/lib/blockscore/person.rb
index ec311a7..8c86127 100644
--- a/lib/blockscore/person.rb
+++ b/lib/blockscore/person.rb
@@ -11,7 +11,7 @@ class Person < Base
 
     def initialize(options = {})
       super
-      @question_sets = Collection.new(QuestionSet.new(person: self))
+      @question_sets = Collection.new({parent: self, target: QuestionSet })
     end
 
     def valid?
diff --git a/lib/blockscore/question_set.rb b/lib/blockscore/question_set.rb
index 944230d..c79ab0c 100644
--- a/lib/blockscore/question_set.rb
+++ b/lib/blockscore/question_set.rb
@@ -6,14 +6,7 @@ class QuestionSet < Base
     include BlockScore::Actions::Retrieve
     include BlockScore::Actions::All
 
-    def_delegators 'self.class', :retrieve, :all, :post, :endpoint
-
-    def create
-      result = self.class.create(person_id: person.id)
-      person.question_sets << result.id
-
-      result
-    end
+    def_delegators 'self.class', :post, :endpoint
 
     def score(answers = nil)
       if answers.nil? && attributes
diff --git a/spec/factories.rb b/spec/factories.rb
index e90f496..a16feb7 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -9,10 +9,10 @@ def initialize
 
   delegate :association, to: :@strategy
 
-  def result(evaluation)
+  def result(evaluation, attrs = {})
     compiled = @strategy.result(evaluation)
     case compiled
-    when BlockScore::Base then compiled.attributes.to_json
+    when BlockScore::Base then compiled.attributes.merge(attrs).to_json
     when Hash             then compiled.to_json
     else
       fail ArgumentError, "don't know how to handle type #{evaluation.class.inspect}"
@@ -210,6 +210,8 @@ def resource_id
         resource_id
       end
     end
+
+    initialize_with { new(attributes)}
   end
 
   # QuestionSet Factory
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index cab1adf..fbfd78a 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -5,6 +5,7 @@
 require 'webmock'
 require 'webmock/rspec'
 require 'rspec'
+require 'rspec/its'
 
 BlockScore::Spec.setup
 
diff --git a/spec/support/stubbed_request.rb b/spec/support/stubbed_request.rb
index ee29519..4f1428d 100644
--- a/spec/support/stubbed_request.rb
+++ b/spec/support/stubbed_request.rb
@@ -5,6 +5,12 @@ def initialize(request)
       @uri     = request.uri
     end
 
+    def body
+      JSON.parse(request.body)
+    rescue
+      nil
+    end
+
     def factory_name
       resource.singularize
     end
diff --git a/spec/support/stubbed_response.rb b/spec/support/stubbed_response.rb
index 61fca05..9bce4dc 100644
--- a/spec/support/stubbed_response.rb
+++ b/spec/support/stubbed_response.rb
@@ -94,7 +94,7 @@ def response
       private
 
       def factory_response
-        json(factory_name)
+        json(factory_name, request.body)
       end
     end
 
diff --git a/spec/unit/blockscore/actions_spec.rb b/spec/unit/blockscore/actions_spec.rb
index 23d63d7..0b07b82 100644
--- a/spec/unit/blockscore/actions_spec.rb
+++ b/spec/unit/blockscore/actions_spec.rb
@@ -42,8 +42,8 @@ module BlockScore
       let(:route) { 'https://api.blockscore.com/fake_resources/abc123' }
 
       it 'uses the correct endpoint' do
-        should receive(:request).with(:get, route, {}).once
-        mock.retrieve('abc123')
+        should receive(:request).with(:get, route, {}).once {resource}
+        mock.retrieve('abc123').attributes
       end
     end
 
diff --git a/spec/unit/blockscore/person_spec.rb b/spec/unit/blockscore/person_spec.rb
index ae72825..23d1714 100644
--- a/spec/unit/blockscore/person_spec.rb
+++ b/spec/unit/blockscore/person_spec.rb
@@ -3,6 +3,19 @@ module BlockScore
     let(:api_stub) { @api_stub }
     let(:action) { -> { create(:person).details } }
 
+    context 'when person has existing question sets' do
+      let(:person) { create(:person, question_set_count: 1) }
+      subject(:question_sets) { person.question_sets }
+      its(:size) { should eql(1) }
+      its(:data) { should be(person.attributes[:question_sets])}
+      its(:first) {should be_an_instance_of(QuestionSet)}
+
+      it "should load attributes" do
+        expect(question_sets.first.attributes).not_to be(nil)
+      end
+
+    end
+
     it '#valid?' do
       person = create(:person, status: 'valid')
       expect(person.valid?).to eq(true)
diff --git a/spec/unit/blockscore/question_set_spec.rb b/spec/unit/blockscore/question_set_spec.rb
index d498205..ab584b7 100644
--- a/spec/unit/blockscore/question_set_spec.rb
+++ b/spec/unit/blockscore/question_set_spec.rb
@@ -1,62 +1,210 @@
 module BlockScore
   RSpec.describe QuestionSet do
-    let(:api_stub) { @api_stub }
     let(:person) { create(:person_params) }
 
-    it 'create' do
-      person.question_sets.count
-      person.question_sets.create
-      assert_requested(api_stub, times: 2)
+    describe "api requests" do
+      let(:api_stub) { @api_stub }
+
+      context "when creating" do
+        it 'create' do
+          person.question_sets.count
+          person.question_sets.create
+          assert_requested(api_stub, times: 2)
+        end
+
+        it 'create question_set count' do
+          count = person.question_sets.count
+          person.question_sets.create
+          expect(count + 1).to be_truthy
+          assert_requested(api_stub, times: 2)
+        end
+      end
+
+      context 'when scoring' do
+        let(:answers) do
+            [
+              { question_id: 1, answer_id: 1 },
+              { question_id: 2, answer_id: 1 },
+              { question_id: 3, answer_id: 1 },
+              { question_id: 4, answer_id: 1 },
+              { question_id: 5, answer_id: 1 }
+            ]
+        end
+
+        it 'score call does request' do
+          qs = person.question_sets.create
+          qs.score(answers)
+          assert_requested(api_stub, times: 3)
+        end
+      end
     end
 
-    it 'create question_set count' do
-      count = person.question_sets.count
-      person.question_sets.create
-      expect(count + 1).to be_truthy
-      assert_requested(api_stub, times: 2)
+    describe "#retrieve" do
+
+      context "when in person#attributes" do
+        let(:qs) { person.question_sets.create }
+        subject { person.question_sets.retrieve(qs.id) }
+
+        it 'loads from collection' do
+          expect(subject).to eql(qs)
+        end
+
+        it "retrieves and checks included person data" do
+          data = [resource_id]
+          allow(person.question_sets).to receive(:data) {data}
+          aggregate_failures("retrieves from data") do
+            expect(data).to receive(:include?).and_call_original
+            expect(data).to receive(:[]).and_call_original
+          end
+          person.question_sets.retrieve(data[0])
+        end
+      end
+
+      context "when not in person#attributes" do
+
+        let(:qs) { QuestionSet.new({person_id: person.id, id: resource_id}) }
+        let(:data) { person.question_sets.data }
+        subject { person.question_sets.retrieve(qs.id) }
+        before(:each) do
+          person.question_sets.data.clear
+          allow(QuestionSet).to receive(:retrieve).with(qs.id) { qs }
+        end
+
+
+        it "question_sets uses QuestionSet.retrieve" do
+          expect(QuestionSet).to receive(:retrieve).with(qs.id) { qs }
+          expect(subject).to be(qs)
+        end
+
+        it "registers new question set" do
+          aggregate_failures("for person and collection") do
+            expect(qs).to receive(:id).at_least(:once).and_call_original
+            expect(person.question_sets).to receive(:<<).with(qs).and_call_original
+            expect(data).to receive(:<<).with(qs.id).and_call_original
+          end
+          person.question_sets.retrieve(qs.id)
+        end
+
+        it "errors if not belonging" do
+          qs.person_id = "some_not_associated_id"
+          expect { subject }.to raise_error(Error, "None belonging")
+        end
+
+      end
     end
 
-    it 'retrieve' do
-      qs = person.question_sets.create
-      person.question_sets.retrieve(qs.id)
-      assert_requested(api_stub, times: 3)
+
+    context "when id is invalid" do
+      pending "should raise not found if not a resource style id" do
+        expect { person.question_sets.retrieve("bad_id") }.to raise_error(NotFoundError)
+      end
+
+      it "should raise ArgumentError if empty" do
+        expect { person.question_sets.retrieve("") }.to raise_error(ArgumentError)
+      end
     end
 
-    describe '#all' do
-      it 'requests' do
-        person.question_sets.all
-        assert_requested(api_stub, times: 2)
+
+    describe "#new" do
+
+      subject { person.question_sets }
+
+      it "should delegate to QuestionSet" do
+        expect(QuestionSet).to receive(:new).with(subject.default_params).and_call_original
+        result = person.question_sets.new
+        expect(result).to be_an_instance_of(QuestionSet)
       end
 
-      it ':count' do
-        response = person.question_sets.all(count: 2)
-        expect(response.count).to eq(2)
-        assert_requested(api_stub, times: 2)
+      it "should save person if not save" do
+        person = Person.new
+        qs = person.question_sets.new
+        expect(person).to receive(:save).and_call_original
+        qs.save
       end
 
-      it ':offset' do
-        response = person.question_sets.all(count: 2, offset: 2)
-        expect(response.count).to eq(2)
-        assert_requested(api_stub, times: 2)
+      it "should have person#id" do
+        qs = person.question_sets.new
+        qs.person_id = "some_id"
+        qs.save
+        expect(qs.person_id).to eq(person.id)
+        expect(qs.id).not_to be(nil)
       end
+
+      it "should add to question_sets" do
+        count = subject.size
+        qs = subject.new
+        expect(subject.size).to eq(count + 1)
+        expect(subject).to include(qs)
+      end
+
     end
 
-    describe '#score' do
-      let(:answers) do
-        [
-          { question_id: 1, answer_id: 1 },
-          { question_id: 2, answer_id: 1 },
-          { question_id: 3, answer_id: 1 },
-          { question_id: 4, answer_id: 1 },
-          { question_id: 5, answer_id: 1 }
-        ]
+    describe "#all" do
+      subject { person.question_sets.all }
+      it "should return self" do
+        is_expected.to be(person.question_sets)
+      end
+    end
+
+    describe "#create" do
+
+      let (:person) { create(:person) }
+      let (:data) { person.attributes[:question_sets] }
+      let (:qs) { QuestionSet.create({person_id: person.id}) }
+      subject { person.question_sets.create }
+
+      it "should pass the Person#id" do
+        expect(QuestionSet).to receive(:create).with({person_id: person.id}).and_call_original
+        subject
+      end
+
+      it "should update Person#attributes" do
+        expect(data).to include(subject.id)
+      end
+
+      it "should update question_sets collection" do
+        expect(person.question_sets).to include(subject)
+      end
+
+      it "should return question set" do
+        is_expected.to be_an_instance_of(QuestionSet)
       end
 
-      it 'score call does request' do
-        qs = person.question_sets.create
-        qs.score(answers)
-        assert_requested(api_stub, times: 3)
+      it "should error if parent not created" do
+        person = Person.new
+        expect { person.question_sets.create }.to raise_error(Error, "Create parent first")
       end
+
+      it "should retrieve from registered" do
+        found_qs = person.question_sets.retrieve(subject.id)
+        expect(subject).to be (found_qs)
+      end
+
+    end
+
+
+    describe "#refresh" do
+
+      subject { person.question_sets }
+      let(:qs) { QuestionSet.create }
+      before(:each) { person.attributes[:question_sets] << qs.id }
+
+      it "should register new data from parent" do
+        person.attributes[:question_sets].clear
+        person.attributes[:question_sets].push(qs.id)
+        expect(QuestionSet).to receive(:retrieve).with(qs.id).and_call_original
+        subject.refresh
+      end
+
+      it "should clear and reload" do
+        expect(subject).to receive(:clear).and_call_original
+        expect(QuestionSet).to receive(:retrieve).and_call_original
+        expect(QuestionSet).to receive(:retrieve).with(qs.id) { qs.clone }
+        result = subject.refresh
+        expect(subject.count).to be(2)
+        expect(subject).to be(result)
+      end
+
     end
   end
 end

From bdc3c8222410cee35087cff873b4ce12665eb4a5 Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Tue, 8 Sep 2015 00:38:29 -0700
Subject: [PATCH 06/21] Add formatting changes

Used rubocop autofix on lib and spec directories
---
 .gitignore                                    |   1 -
 lib/blockscore.rb                             |   8 +-
 lib/blockscore/actions/retrieve.rb            |   2 +-
 lib/blockscore/base.rb                        |  11 +-
 lib/blockscore/collection.rb                  |  20 ++--
 lib/blockscore/connection.rb                  |   4 +-
 lib/blockscore/errors/api_error.rb            |   6 +-
 .../errors/invalid_request_error.rb           |   6 +-
 lib/blockscore/person.rb                      |   2 +-
 lib/blockscore/util.rb                        |  16 +--
 lib/blockscore/version.rb                     |   2 +-
 spec/unit/blockscore/actions_spec.rb          |   2 +-
 spec/unit/blockscore/candidate_spec.rb        |   2 +-
 spec/unit/blockscore/person_spec.rb           |   7 +-
 spec/unit/blockscore/question_set_spec.rb     | 109 ++++++++----------
 15 files changed, 88 insertions(+), 110 deletions(-)

diff --git a/.gitignore b/.gitignore
index db2a87d..73b0073 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,4 +1,3 @@
-refactor_notes.md
 
 # rcov generated
 coverage
diff --git a/lib/blockscore.rb b/lib/blockscore.rb
index b652aa5..01f05eb 100644
--- a/lib/blockscore.rb
+++ b/lib/blockscore.rb
@@ -34,11 +34,11 @@
 require 'blockscore/version'
 
 module BlockScore
-  def self.api_key=(api_key)
-    @api_key = api_key
+  class << self
+    attr_writer :api_key
   end
 
-  def self.api_key
-    @api_key
+  class << self
+    attr_reader :api_key
   end
 end
diff --git a/lib/blockscore/actions/retrieve.rb b/lib/blockscore/actions/retrieve.rb
index 383a036..166f28d 100644
--- a/lib/blockscore/actions/retrieve.rb
+++ b/lib/blockscore/actions/retrieve.rb
@@ -11,7 +11,7 @@ module Actions
     # => #<BlockScore::Person:0x007fe39c424410>
     module Retrieve
       module ClassMethods
-        def retrieve(id, options={})
+        def retrieve(id, options = {})
           fail ArgumentError if id.empty?
           req = ->() { get("#{endpoint}/#{id}", options) }
           new(&req)
diff --git a/lib/blockscore/base.rb b/lib/blockscore/base.rb
index ad446a6..ff18da4 100644
--- a/lib/blockscore/base.rb
+++ b/lib/blockscore/base.rb
@@ -4,7 +4,6 @@ module BlockScore
   class Base
     extend Connection
 
-
     def initialize(options = {}, &block)
       @loaded = !(block)
       @proc = block
@@ -19,14 +18,14 @@ def attributes
     end
 
     def force!
-      r = @proc.call()
+      r = @proc.call
       @attributes = r.attributes
       @loaded = true
       self
     end
 
     def inspect
-      str_attr = "JSON:#{ JSON.pretty_generate(attributes) }"
+      str_attr = "JSON:#{JSON.pretty_generate(attributes)}"
       "#<#{self.class}:0x#{object_id.to_s(16)} #{str_attr}>"
     end
 
@@ -61,16 +60,14 @@ def self.api_url
     end
 
     def self.endpoint
-      if self == Base
-        fail NotImplementedError, 'Base is an abstract class, not an API resource'
-      end
+      fail NotImplementedError, 'Base is an abstract class, not an API resource' if self == Base
 
       "#{api_url}#{Util.to_plural(resource)}"
     end
 
     protected
 
-    def add_accessor(symbol, *args)
+    def add_accessor(symbol, *_args)
       singleton_class.instance_eval do
         define_method(symbol) do
           wrap_attribute(attributes[symbol])
diff --git a/lib/blockscore/collection.rb b/lib/blockscore/collection.rb
index db4ddd9..96c2005 100644
--- a/lib/blockscore/collection.rb
+++ b/lib/blockscore/collection.rb
@@ -1,12 +1,10 @@
 module BlockScore
   class Collection < Array
-
     attr_reader :default_params
     protected :default_params
 
     attr_reader :parent, :target, :data
 
-
     def initialize(params)
       @parent = params.fetch :parent
       @target = params.fetch :target
@@ -27,12 +25,12 @@ def all
       self
     end
 
-    def new(params ={})
+    def new(params = {})
       item = target.new(params.merge(default_params))
-      _self = self
+      ctxt = self
       item.define_singleton_method(:save) do
-        _self.parent.save unless _self.parent_id
-        send :"#{_self.parent_name}_id=", _self.parent_id
+        ctxt.parent.save unless ctxt.parent_id
+        send :"#{ctxt.parent_name}_id=", ctxt.parent_id
         super()
       end
       self << item
@@ -54,7 +52,7 @@ def target_name
     end
 
     def create(params = {})
-      fail Error, "Create parent first" unless parent_id
+      fail Error, 'Create parent first' unless parent_id
       assoc_params = default_params.merge(params)
       item = target.create assoc_params
       @data << item.id
@@ -70,26 +68,24 @@ def retrieve(id)
 
     private
 
-    def has_parent_id(item)
+    def parent_id?(item)
       parent_id && item.send(:"#{parent_name}_id") == parent_id
     end
 
     def register_to_parent(item)
-      fail Error, "None belonging" unless has_parent_id(item)
+      fail Error, 'None belonging' unless parent_id?(item)
       data << item.id
       self << item
       item
     end
 
     def register_parent_data
-      @data = parent.attributes.fetch( :"#{ Util.to_plural(target_name) }", [])
+      @data = parent.attributes.fetch(:"#{ Util.to_plural(target_name) }", [])
 
       @data.each do |id|
         item = target.retrieve(id)
         self << item
       end
     end
-
-
   end
 end
diff --git a/lib/blockscore/connection.rb b/lib/blockscore/connection.rb
index ade3d24..9d1c5df 100644
--- a/lib/blockscore/connection.rb
+++ b/lib/blockscore/connection.rb
@@ -32,7 +32,7 @@ def request(method, path, params)
       begin
         response = execute_request(method, path, params)
       rescue SocketError, Errno::ECONNREFUSED => e
-        fail APIConnectionError, e.message
+        raise APIConnectionError, e.message
       end
 
       Response.handle_response(resource, response)
@@ -55,7 +55,7 @@ def execute_request(method, path, params)
 
     def encode_path_params(path, params)
       encoded = URI.encode_www_form(params)
-      [path, encoded].join("?")
+      [path, encoded].join('?')
     end
   end
 end
diff --git a/lib/blockscore/errors/api_error.rb b/lib/blockscore/errors/api_error.rb
index 1b9bd6b..43a6d0d 100644
--- a/lib/blockscore/errors/api_error.rb
+++ b/lib/blockscore/errors/api_error.rb
@@ -15,7 +15,7 @@ class APIError < Error
     # APIError - Indicates an error on the server side (HTTP 5xx)
     # AuthenticationError - Indicates an authentication error (HTTP 401)
     def initialize(response)
-      body = JSON.parse(response.body, :symbolize_names => true)
+      body = JSON.parse(response.body, symbolize_names: true)
 
       @message = body[:error][:message]
       @http_status = response.code
@@ -24,8 +24,8 @@ def initialize(response)
     end
 
     def to_s
-      status_string = @http_status ? "(Status: #{@http_status})" : ""
-      type_string = @error_type ? "(Type: #{@error_type})" : ""
+      status_string = @http_status ? "(Status: #{@http_status})" : ''
+      type_string = @error_type ? "(Type: #{@error_type})" : ''
 
       "#{type_string} #{@message} #{status_string}"
     end
diff --git a/lib/blockscore/errors/invalid_request_error.rb b/lib/blockscore/errors/invalid_request_error.rb
index f76873e..e7ff856 100644
--- a/lib/blockscore/errors/invalid_request_error.rb
+++ b/lib/blockscore/errors/invalid_request_error.rb
@@ -19,9 +19,9 @@ def initialize(response)
     end
 
     def to_s
-      status_string = @http_status ? "(Status: #{@http_status})" : ""
-      type_string = @error_type ? "(Type: #{@error_type})" : ""
-      param_string = @param ? "(#{@param})" : ""
+      status_string = @http_status ? "(Status: #{@http_status})" : ''
+      type_string = @error_type ? "(Type: #{@error_type})" : ''
+      param_string = @param ? "(#{@param})" : ''
 
       "#{type_string} #{@message} #{param_string} #{status_string}"
     end
diff --git a/lib/blockscore/person.rb b/lib/blockscore/person.rb
index 8c86127..fa51a1d 100644
--- a/lib/blockscore/person.rb
+++ b/lib/blockscore/person.rb
@@ -11,7 +11,7 @@ class Person < Base
 
     def initialize(options = {})
       super
-      @question_sets = Collection.new({parent: self, target: QuestionSet })
+      @question_sets = Collection.new(parent: self, target: QuestionSet)
     end
 
     def valid?
diff --git a/lib/blockscore/util.rb b/lib/blockscore/util.rb
index 3624aeb..6c5cf6e 100644
--- a/lib/blockscore/util.rb
+++ b/lib/blockscore/util.rb
@@ -11,13 +11,13 @@ module Util
     }
 
     def parse_json!(json_obj)
-      JSON.parse(json_obj, :symbolize_names => true)
+      JSON.parse(json_obj, symbolize_names: true)
     end
 
     def parse_json(json_obj)
       parse_json! json_obj
     rescue JSON::ParserError
-      fail Error, "An error has occurred. If this problem persists, please message support@blockscore.com."
+      raise Error, 'An error has occurred. If this problem persists, please message support@blockscore.com.'
     end
 
     def create_object(resource, options = {})
@@ -65,16 +65,16 @@ def to_constant(camel_cased_word)
     end
 
     def to_camelcase(str)
-      str.split('_').map { |i| i.capitalize }.join('')
+      str.split('_').map(&:capitalize).join('')
     end
 
     # Taken from Rulers: http://git.io/vkWqf
     def to_underscore(str)
-      str.gsub(/::/, '/').
-      gsub(/([A-Z]+)([A-Z][a-z])/,'\1_\2').
-      gsub(/([a-z\d])([A-Z])/,'\1_\2').
-      tr("-", "_").
-      downcase
+      str.gsub(/::/, '/')
+        .gsub(/([A-Z]+)([A-Z][a-z])/, '\1_\2')
+        .gsub(/([a-z\d])([A-Z])/, '\1_\2')
+        .tr('-', '_')
+        .downcase
     end
   end
 end
diff --git a/lib/blockscore/version.rb b/lib/blockscore/version.rb
index 26e1366..3664c01 100644
--- a/lib/blockscore/version.rb
+++ b/lib/blockscore/version.rb
@@ -1,3 +1,3 @@
 module BlockScore
   VERSION = '4.1.2'.freeze
-end
\ No newline at end of file
+end
diff --git a/spec/unit/blockscore/actions_spec.rb b/spec/unit/blockscore/actions_spec.rb
index 0b07b82..e7d6509 100644
--- a/spec/unit/blockscore/actions_spec.rb
+++ b/spec/unit/blockscore/actions_spec.rb
@@ -42,7 +42,7 @@ module BlockScore
       let(:route) { 'https://api.blockscore.com/fake_resources/abc123' }
 
       it 'uses the correct endpoint' do
-        should receive(:request).with(:get, route, {}).once {resource}
+        should receive(:request).with(:get, route, {}).once { resource }
         mock.retrieve('abc123').attributes
       end
     end
diff --git a/spec/unit/blockscore/candidate_spec.rb b/spec/unit/blockscore/candidate_spec.rb
index 3ef3a2f..bde5e3a 100644
--- a/spec/unit/blockscore/candidate_spec.rb
+++ b/spec/unit/blockscore/candidate_spec.rb
@@ -32,7 +32,7 @@ module BlockScore
       subject(:search) { -> { candidate.search(constraints) } }
 
       context 'search request' do
-        let(:uri)      { "/watchlists"                                         }
+        let(:uri)      { '/watchlists'                                         }
         let(:body)     { { 'candidate_id' => candidate.id }.merge(constraints) }
         let(:expected) { { body: hash_including(body) }                        }
         before { search.call }
diff --git a/spec/unit/blockscore/person_spec.rb b/spec/unit/blockscore/person_spec.rb
index 23d1714..9f46833 100644
--- a/spec/unit/blockscore/person_spec.rb
+++ b/spec/unit/blockscore/person_spec.rb
@@ -7,13 +7,12 @@ module BlockScore
       let(:person) { create(:person, question_set_count: 1) }
       subject(:question_sets) { person.question_sets }
       its(:size) { should eql(1) }
-      its(:data) { should be(person.attributes[:question_sets])}
-      its(:first) {should be_an_instance_of(QuestionSet)}
+      its(:data) { should be(person.attributes[:question_sets]) }
+      its(:first) { should be_an_instance_of(QuestionSet) }
 
-      it "should load attributes" do
+      it 'should load attributes' do
         expect(question_sets.first.attributes).not_to be(nil)
       end
-
     end
 
     it '#valid?' do
diff --git a/spec/unit/blockscore/question_set_spec.rb b/spec/unit/blockscore/question_set_spec.rb
index ab584b7..40ba559 100644
--- a/spec/unit/blockscore/question_set_spec.rb
+++ b/spec/unit/blockscore/question_set_spec.rb
@@ -2,10 +2,10 @@ module BlockScore
   RSpec.describe QuestionSet do
     let(:person) { create(:person_params) }
 
-    describe "api requests" do
+    describe 'api requests' do
       let(:api_stub) { @api_stub }
 
-      context "when creating" do
+      context 'when creating' do
         it 'create' do
           person.question_sets.count
           person.question_sets.create
@@ -22,13 +22,13 @@ module BlockScore
 
       context 'when scoring' do
         let(:answers) do
-            [
-              { question_id: 1, answer_id: 1 },
-              { question_id: 2, answer_id: 1 },
-              { question_id: 3, answer_id: 1 },
-              { question_id: 4, answer_id: 1 },
-              { question_id: 5, answer_id: 1 }
-            ]
+          [
+            { question_id: 1, answer_id: 1 },
+            { question_id: 2, answer_id: 1 },
+            { question_id: 3, answer_id: 1 },
+            { question_id: 4, answer_id: 1 },
+            { question_id: 5, answer_id: 1 }
+          ]
         end
 
         it 'score call does request' do
@@ -39,9 +39,8 @@ module BlockScore
       end
     end
 
-    describe "#retrieve" do
-
-      context "when in person#attributes" do
+    describe '#retrieve' do
+      context 'when in person#attributes' do
         let(:qs) { person.question_sets.create }
         subject { person.question_sets.retrieve(qs.id) }
 
@@ -49,10 +48,10 @@ module BlockScore
           expect(subject).to eql(qs)
         end
 
-        it "retrieves and checks included person data" do
+        it 'retrieves and checks included person data' do
           data = [resource_id]
-          allow(person.question_sets).to receive(:data) {data}
-          aggregate_failures("retrieves from data") do
+          allow(person.question_sets).to receive(:data) { data }
+          aggregate_failures('retrieves from data') do
             expect(data).to receive(:include?).and_call_original
             expect(data).to receive(:[]).and_call_original
           end
@@ -60,9 +59,8 @@ module BlockScore
         end
       end
 
-      context "when not in person#attributes" do
-
-        let(:qs) { QuestionSet.new({person_id: person.id, id: resource_id}) }
+      context 'when not in person#attributes' do
+        let(:qs) { QuestionSet.new(person_id: person.id, id: resource_id) }
         let(:data) { person.question_sets.data }
         subject { person.question_sets.retrieve(qs.id) }
         before(:each) do
@@ -70,14 +68,13 @@ module BlockScore
           allow(QuestionSet).to receive(:retrieve).with(qs.id) { qs }
         end
 
-
-        it "question_sets uses QuestionSet.retrieve" do
+        it 'question_sets uses QuestionSet.retrieve' do
           expect(QuestionSet).to receive(:retrieve).with(qs.id) { qs }
           expect(subject).to be(qs)
         end
 
-        it "registers new question set" do
-          aggregate_failures("for person and collection") do
+        it 'registers new question set' do
+          aggregate_failures('for person and collection') do
             expect(qs).to receive(:id).at_least(:once).and_call_original
             expect(person.question_sets).to receive(:<<).with(qs).and_call_original
             expect(data).to receive(:<<).with(qs.id).and_call_original
@@ -85,118 +82,109 @@ module BlockScore
           person.question_sets.retrieve(qs.id)
         end
 
-        it "errors if not belonging" do
-          qs.person_id = "some_not_associated_id"
-          expect { subject }.to raise_error(Error, "None belonging")
+        it 'errors if not belonging' do
+          qs.person_id = 'some_not_associated_id'
+          expect { subject }.to raise_error(Error, 'None belonging')
         end
-
       end
     end
 
-
-    context "when id is invalid" do
-      pending "should raise not found if not a resource style id" do
-        expect { person.question_sets.retrieve("bad_id") }.to raise_error(NotFoundError)
+    context 'when id is invalid' do
+      pending 'should raise not found if not a resource style id' do
+        expect { person.question_sets.retrieve('bad_id') }.to raise_error(NotFoundError)
       end
 
-      it "should raise ArgumentError if empty" do
-        expect { person.question_sets.retrieve("") }.to raise_error(ArgumentError)
+      it 'should raise ArgumentError if empty' do
+        expect { person.question_sets.retrieve('') }.to raise_error(ArgumentError)
       end
     end
 
-
-    describe "#new" do
-
+    describe '#new' do
       subject { person.question_sets }
 
-      it "should delegate to QuestionSet" do
+      it 'should delegate to QuestionSet' do
         expect(QuestionSet).to receive(:new).with(subject.default_params).and_call_original
         result = person.question_sets.new
         expect(result).to be_an_instance_of(QuestionSet)
       end
 
-      it "should save person if not save" do
+      it 'should save person if not save' do
         person = Person.new
         qs = person.question_sets.new
         expect(person).to receive(:save).and_call_original
         qs.save
       end
 
-      it "should have person#id" do
+      it 'should have person#id' do
         qs = person.question_sets.new
-        qs.person_id = "some_id"
+        qs.person_id = 'some_id'
         qs.save
         expect(qs.person_id).to eq(person.id)
         expect(qs.id).not_to be(nil)
       end
 
-      it "should add to question_sets" do
+      it 'should add to question_sets' do
         count = subject.size
         qs = subject.new
         expect(subject.size).to eq(count + 1)
         expect(subject).to include(qs)
       end
-
     end
 
-    describe "#all" do
+    describe '#all' do
       subject { person.question_sets.all }
-      it "should return self" do
+      it 'should return self' do
         is_expected.to be(person.question_sets)
       end
     end
 
-    describe "#create" do
-
+    describe '#create' do
       let (:person) { create(:person) }
       let (:data) { person.attributes[:question_sets] }
-      let (:qs) { QuestionSet.create({person_id: person.id}) }
+      let (:qs) { QuestionSet.create(person_id: person.id) }
       subject { person.question_sets.create }
 
-      it "should pass the Person#id" do
-        expect(QuestionSet).to receive(:create).with({person_id: person.id}).and_call_original
+      it 'should pass the Person#id' do
+        expect(QuestionSet).to receive(:create).with(person_id: person.id).and_call_original
         subject
       end
 
-      it "should update Person#attributes" do
+      it 'should update Person#attributes' do
         expect(data).to include(subject.id)
       end
 
-      it "should update question_sets collection" do
+      it 'should update question_sets collection' do
         expect(person.question_sets).to include(subject)
       end
 
-      it "should return question set" do
+      it 'should return question set' do
         is_expected.to be_an_instance_of(QuestionSet)
       end
 
-      it "should error if parent not created" do
+      it 'should error if parent not created' do
         person = Person.new
-        expect { person.question_sets.create }.to raise_error(Error, "Create parent first")
+        expect { person.question_sets.create }.to raise_error(Error, 'Create parent first')
       end
 
-      it "should retrieve from registered" do
+      it 'should retrieve from registered' do
         found_qs = person.question_sets.retrieve(subject.id)
         expect(subject).to be (found_qs)
       end
-
     end
 
-
-    describe "#refresh" do
-
+    describe '#refresh' do
       subject { person.question_sets }
       let(:qs) { QuestionSet.create }
       before(:each) { person.attributes[:question_sets] << qs.id }
 
-      it "should register new data from parent" do
+      it 'should register new data from parent' do
         person.attributes[:question_sets].clear
         person.attributes[:question_sets].push(qs.id)
         expect(QuestionSet).to receive(:retrieve).with(qs.id).and_call_original
         subject.refresh
       end
 
-      it "should clear and reload" do
+      it 'should clear and reload' do
         expect(subject).to receive(:clear).and_call_original
         expect(QuestionSet).to receive(:retrieve).and_call_original
         expect(QuestionSet).to receive(:retrieve).with(qs.id) { qs.clone }
@@ -204,7 +192,6 @@ module BlockScore
         expect(subject.count).to be(2)
         expect(subject).to be(result)
       end
-
     end
   end
 end

From db03f710f5eeb5c9781ee63e120edaab4e9d3732 Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Tue, 8 Sep 2015 09:52:22 -0700
Subject: [PATCH 07/21] Add merge to lazy loaded attributes with current

---
 lib/blockscore/base.rb | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/blockscore/base.rb b/lib/blockscore/base.rb
index ff18da4..e205f4d 100644
--- a/lib/blockscore/base.rb
+++ b/lib/blockscore/base.rb
@@ -10,7 +10,6 @@ def initialize(options = {}, &block)
       @attributes = options
     end
 
-    # potential issues here
     def attributes
       return @attributes if @loaded
       force!
@@ -18,8 +17,8 @@ def attributes
     end
 
     def force!
-      r = @proc.call
-      @attributes = r.attributes
+      res = @proc.call
+      @attributes = res.attributes.merge(@attributes)
       @loaded = true
       self
     end

From 089e5db191f31c853f0c41847f33e14dbe751861 Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Tue, 8 Sep 2015 13:00:23 -0700
Subject: [PATCH 08/21] Add houndci comment suggestions

---
 spec/factories.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/factories.rb b/spec/factories.rb
index a16feb7..0dceee0 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -211,7 +211,7 @@ def resource_id
       end
     end
 
-    initialize_with { new(attributes)}
+    initialize_with { new(attributes) }
   end
 
   # QuestionSet Factory
@@ -312,7 +312,7 @@ def resource_id
 
   # We can do this because the error type is determined by the
   # HTTP response code.
-  factory :blockscore_error, :class => Hash, :traits => [:resource] do
+  factory :blockscore_error, class: Hash, traits: [:resource] do
     transient do
       error_type 'api_error'
     end

From 0cd470685c32410f28f361ee68cfd8b3ab90ca8d Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Tue, 8 Sep 2015 13:49:41 -0700
Subject: [PATCH 09/21] Add changes to address PR comments

See
https://github.com/BlockScore/blockscore-ruby/pull/23/files#r38977260
---
 lib/blockscore/base.rb       |  4 ++++
 lib/blockscore/collection.rb | 14 +++++---------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/blockscore/base.rb b/lib/blockscore/base.rb
index e205f4d..4cc55ca 100644
--- a/lib/blockscore/base.rb
+++ b/lib/blockscore/base.rb
@@ -23,6 +23,10 @@ def force!
       self
     end
 
+    def id
+      attributes.fetch(:id, nil)
+    end
+
     def inspect
       str_attr = "JSON:#{JSON.pretty_generate(attributes)}"
       "#<#{self.class}:0x#{object_id.to_s(16)} #{str_attr}>"
diff --git a/lib/blockscore/collection.rb b/lib/blockscore/collection.rb
index 96c2005..27cc3aa 100644
--- a/lib/blockscore/collection.rb
+++ b/lib/blockscore/collection.rb
@@ -13,14 +13,10 @@ def initialize(params)
 
     def default_params
       {
-        :"#{parent_name}_id" => parent_id
+        :"#{parent_name}_id" => parent.id
       }
     end
 
-    def parent_id
-      parent.id if parent.respond_to?(:id)
-    end
-
     def all
       self
     end
@@ -29,8 +25,8 @@ def new(params = {})
       item = target.new(params.merge(default_params))
       ctxt = self
       item.define_singleton_method(:save) do
-        ctxt.parent.save unless ctxt.parent_id
-        send :"#{ctxt.parent_name}_id=", ctxt.parent_id
+        ctxt.parent.save unless ctxt.parent.id
+        send :"#{ctxt.parent_name}_id=", ctxt.parent.id
         super()
       end
       self << item
@@ -52,7 +48,7 @@ def target_name
     end
 
     def create(params = {})
-      fail Error, 'Create parent first' unless parent_id
+      fail Error, 'Create parent first' unless parent.id
       assoc_params = default_params.merge(params)
       item = target.create assoc_params
       @data << item.id
@@ -69,7 +65,7 @@ def retrieve(id)
     private
 
     def parent_id?(item)
-      parent_id && item.send(:"#{parent_name}_id") == parent_id
+      parent.id && item.send(:"#{parent_name}_id") == parent.id
     end
 
     def register_to_parent(item)

From 6ccdac4a16785daf0e6ce7eae70e944d1e06b443 Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Tue, 8 Sep 2015 14:22:14 -0700
Subject: [PATCH 10/21] Refactor public default_params method

See
https://github.com/BlockScore/blockscore-ruby/commit/223a8a54a2538bb4ff7012718f1ab950936ae783#commitcomment-13121687
---
 lib/blockscore/collection.rb              | 17 ++++++++---------
 spec/unit/blockscore/person_spec.rb       |  5 +++++
 spec/unit/blockscore/question_set_spec.rb |  5 +++--
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/lib/blockscore/collection.rb b/lib/blockscore/collection.rb
index 27cc3aa..0dcff79 100644
--- a/lib/blockscore/collection.rb
+++ b/lib/blockscore/collection.rb
@@ -1,8 +1,5 @@
 module BlockScore
   class Collection < Array
-    attr_reader :default_params
-    protected :default_params
-
     attr_reader :parent, :target, :data
 
     def initialize(params)
@@ -11,12 +8,6 @@ def initialize(params)
       register_parent_data
     end
 
-    def default_params
-      {
-        :"#{parent_name}_id" => parent.id
-      }
-    end
-
     def all
       self
     end
@@ -62,6 +53,14 @@ def retrieve(id)
       register_to_parent(item)
     end
 
+    protected
+
+    def default_params
+      {
+        :"#{parent_name}_id" => parent.id
+      }
+    end
+
     private
 
     def parent_id?(item)
diff --git a/spec/unit/blockscore/person_spec.rb b/spec/unit/blockscore/person_spec.rb
index 9f46833..124d481 100644
--- a/spec/unit/blockscore/person_spec.rb
+++ b/spec/unit/blockscore/person_spec.rb
@@ -15,6 +15,11 @@ module BlockScore
       end
     end
 
+    it "#id" do
+      person = create(:person)
+      expect(person.id).not_to be(nil)
+    end
+
     it '#valid?' do
       person = create(:person, status: 'valid')
       expect(person.valid?).to eq(true)
diff --git a/spec/unit/blockscore/question_set_spec.rb b/spec/unit/blockscore/question_set_spec.rb
index 40ba559..26e71e7 100644
--- a/spec/unit/blockscore/question_set_spec.rb
+++ b/spec/unit/blockscore/question_set_spec.rb
@@ -103,7 +103,8 @@ module BlockScore
       subject { person.question_sets }
 
       it 'should delegate to QuestionSet' do
-        expect(QuestionSet).to receive(:new).with(subject.default_params).and_call_original
+        args = { person_id: person.id }
+        expect(QuestionSet).to receive(:new).with(args).and_call_original
         result = person.question_sets.new
         expect(result).to be_an_instance_of(QuestionSet)
       end
@@ -194,4 +195,4 @@ module BlockScore
       end
     end
   end
-end
+end
\ No newline at end of file

From 4102fa3f39a16ab15fc6d4f5a4a18b7c4d844f33 Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Tue, 8 Sep 2015 14:26:10 -0700
Subject: [PATCH 11/21] Add houndci formatting for spec

---
 spec/unit/blockscore/person_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/unit/blockscore/person_spec.rb b/spec/unit/blockscore/person_spec.rb
index 124d481..e32e2e4 100644
--- a/spec/unit/blockscore/person_spec.rb
+++ b/spec/unit/blockscore/person_spec.rb
@@ -15,7 +15,7 @@ module BlockScore
       end
     end
 
-    it "#id" do
+    it '#id' do
       person = create(:person)
       expect(person.id).not_to be(nil)
     end

From 153a26f9f905c78dab0737adf0e007e41ca8e507 Mon Sep 17 00:00:00 2001
From: Delmer Reed and John Backus <jbackus@stanford.edu>
Date: Tue, 8 Sep 2015 16:18:34 -0700
Subject: [PATCH 12/21] Add documentation for collection

- Moved a few attributes into protected/private in the process.
- Also changed `data` to be a method in collection
---
 lib/blockscore/collection.rb              | 157 +++++++++++++++++++++-
 spec/unit/blockscore/question_set_spec.rb |  10 +-
 2 files changed, 154 insertions(+), 13 deletions(-)

diff --git a/lib/blockscore/collection.rb b/lib/blockscore/collection.rb
index 0dcff79..3f16753 100644
--- a/lib/blockscore/collection.rb
+++ b/lib/blockscore/collection.rb
@@ -1,17 +1,66 @@
 module BlockScore
+  # Collection is a proxy between the parent and the asssociated targets
+  # where parent is some instance of a resource and
+  # where target is the Class associated to an embedded
+  #
   class Collection < Array
-    attr_reader :parent, :target, :data
+    # @!attribute [r] parent
+    # resource which owns a collection of other resources
+    #
+    # @example
+    #   person.question_sets.parent # => person
+    #
+    # @return [BlockScore::Base] a resource
+    #
+    # @api private
+    attr_reader :parent
 
+    # Sets parent and target then registers embedded ids
+    #
+    # @param params [Hash] options hash
+    # @option parent [BlockScore::Base] :parent
+    # @option target [Class] :target
+    #
+    # @return [undefined]
+    #
+    # @api private
     def initialize(params)
       @parent = params.fetch :parent
       @target = params.fetch :target
       register_parent_data
     end
 
+    # Syntactic sugar method for returning collection
+    #
+    # @example
+    #   all # returns collection
+    #
+    # @return [self]
+    #
+    # @api public
     def all
       self
     end
 
+    # Initializes new {target} with `params`
+    #
+    # - Ensures a parent id is meged into `params` (see #default_params).
+    # - Defines method `#save` on new collection member
+    # - Adds new item to collection
+    #
+    # @example usage
+    #
+    #   >> person = person = BlockScore::Person.retrieve('55de4af7643735000300000f')
+    #   >> person.question_sets.new
+    #   => #<BlockScore::QuestionSet:0x3fc67902f1b4 JSON:{
+    #     "person_id": "55de4af7643735000300000f"
+    #   }>
+    #
+    # @param params [Hash] initial params for member
+    #
+    # @return instance of {target}
+    #
+    # @api public
     def new(params = {})
       item = target.new(params.merge(default_params))
       ctxt = self
@@ -24,29 +73,78 @@ def new(params = {})
       item
     end
 
+    # Relaod the contents of the collection
+    #
+    # @example usage
+    #   person.question_sets.refresh # => [#<BlockScore::QuestionSet...]
+    #
+    # @return [self]
+    #
+    # @api public
     def refresh
       clear
       register_parent_data
       self
     end
 
+    # Name of parent resource
+    #
+    # @example
+    #   self.parent_name # => 'person'
+    #
+    # @return [String]
+    #
+    # @api semipublic
     def parent_name
       parent.class.resource
     end
 
+    # Name of target resource
+    #
+    # @example
+    #   self.target_name # => 'question_sets'
+    #
+    # @return [String]
+    #
+    # @api private
     def target_name
       target.resource
     end
 
+    # Initialize a collection member and save it
+    #
+    # @example
+    #   >> person.question_sets.create
+    #   => #<BlockScore::QuestionSet:0x3fc67a6007f4 JSON:{
+    #     "object": "question_set",
+    #     "id": "55ef5d5b62386200030001b3",
+    #     "created_at": 1441750363,
+    #     ...
+    #     }
+    #
+    # @param params [Hash] params
+    #
+    # @return new saved instance of {target}
+    #
+    # @api public
     def create(params = {})
       fail Error, 'Create parent first' unless parent.id
       assoc_params = default_params.merge(params)
       item = target.create assoc_params
-      @data << item.id
-      self << item
-      item
+      register_to_parent(item)
     end
 
+    # Retrieve a collection member by its id
+    #
+    # @example usage
+    #   person.question_sets.retrieve('55ef5b4e3532630003000178') # => instance of QuestionSet
+    #
+    # @param id [String] resource id
+    #
+    # @return instance of {target} if found
+    # @raise [BlockScore::NotFoundError] otherwise
+    #
+    # @api public
     def retrieve(id)
       return self[data.index(id)] if data.include? id
       item = target.retrieve(id)
@@ -55,6 +153,22 @@ def retrieve(id)
 
     protected
 
+    # @!attribute [r] target
+    # class which will be used for the embedded
+    # resources in the collection
+    #
+    # @todo rename to `target_class`?
+    #
+    # @return [Class]
+    #
+    # @api private
+    attr_reader :target
+
+    # Default params for making an instance of {target}
+    #
+    # @return [Hash]
+    #
+    # @api private
     def default_params
       {
         :"#{parent_name}_id" => parent.id
@@ -63,10 +177,25 @@ def default_params
 
     private
 
+    # Check if `parent_id` is defined on `item`
+    #
+    # @param item [BlockScore::Base] any resource
+    #
+    # @return [Boolean]
+    #
+    # @api private
     def parent_id?(item)
       parent.id && item.send(:"#{parent_name}_id") == parent.id
     end
 
+    # Register a resource in collection
+    #
+    # @param item [BlockScore::Base] a resource
+    #
+    # @raise [BlockScore::Error] if no `parent_id`
+    # @return [BlockScore::Base] otherwise
+    #
+    # @api private
     def register_to_parent(item)
       fail Error, 'None belonging' unless parent_id?(item)
       data << item.id
@@ -74,13 +203,27 @@ def register_to_parent(item)
       item
     end
 
+    # Fetches embedded ids from parent and adds to self
+    #
+    # @return [undefined]
+    #
+    # @api private
     def register_parent_data
-      @data = parent.attributes.fetch(:"#{ Util.to_plural(target_name) }", [])
-
-      @data.each do |id|
+      data.each do |id|
         item = target.retrieve(id)
         self << item
       end
     end
+
+    # ids that belong to the collection
+    #
+    # @todo rename to `ids`?
+    #
+    # @return [Array<String>]
+    #
+    # @api private
+    def data
+      parent.attributes.fetch(:"#{ Util.to_plural(target_name) }", [])
+    end
   end
 end
diff --git a/spec/unit/blockscore/question_set_spec.rb b/spec/unit/blockscore/question_set_spec.rb
index 26e71e7..587e21a 100644
--- a/spec/unit/blockscore/question_set_spec.rb
+++ b/spec/unit/blockscore/question_set_spec.rb
@@ -42,11 +42,10 @@ module BlockScore
     describe '#retrieve' do
       context 'when in person#attributes' do
         let(:qs) { person.question_sets.create }
+        let(:person) { create(:person) }
         subject { person.question_sets.retrieve(qs.id) }
 
-        it 'loads from collection' do
-          expect(subject).to eql(qs)
-        end
+        it { should eql(qs) }
 
         it 'retrieves and checks included person data' do
           data = [resource_id]
@@ -61,10 +60,10 @@ module BlockScore
 
       context 'when not in person#attributes' do
         let(:qs) { QuestionSet.new(person_id: person.id, id: resource_id) }
-        let(:data) { person.question_sets.data }
+        let(:data) { person.attributes.fetch :question_sets }
         subject { person.question_sets.retrieve(qs.id) }
+
         before(:each) do
-          person.question_sets.data.clear
           allow(QuestionSet).to receive(:retrieve).with(qs.id) { qs }
         end
 
@@ -75,7 +74,6 @@ module BlockScore
 
         it 'registers new question set' do
           aggregate_failures('for person and collection') do
-            expect(qs).to receive(:id).at_least(:once).and_call_original
             expect(person.question_sets).to receive(:<<).with(qs).and_call_original
             expect(data).to receive(:<<).with(qs.id).and_call_original
           end

From b54647368fc2d3a2f2873fc8f6dbd869631ea0ff Mon Sep 17 00:00:00 2001
From: John Backus <jbackus@stanford.edu>
Date: Tue, 8 Sep 2015 17:46:11 -0700
Subject: [PATCH 13/21] Refactor minor style and naming in collection

- renamed `target` to `member_class`
- renamed `data` to `ids`
- changed Collection to use positional args instead of a hash
---
 lib/blockscore/collection.rb | 66 +++++++++++++-----------------------
 lib/blockscore/person.rb     |  2 +-
 2 files changed, 25 insertions(+), 43 deletions(-)

diff --git a/lib/blockscore/collection.rb b/lib/blockscore/collection.rb
index 3f16753..93985c9 100644
--- a/lib/blockscore/collection.rb
+++ b/lib/blockscore/collection.rb
@@ -1,7 +1,6 @@
 module BlockScore
-  # Collection is a proxy between the parent and the asssociated targets
-  # where parent is some instance of a resource and
-  # where target is the Class associated to an embedded
+  # Collection is a proxy between the parent and the asssociated members
+  # where parent is some instance of a resource
   #
   class Collection < Array
     # @!attribute [r] parent
@@ -15,18 +14,17 @@ class Collection < Array
     # @api private
     attr_reader :parent
 
-    # Sets parent and target then registers embedded ids
+    # Sets parent and member_class then registers embedded ids
     #
-    # @param params [Hash] options hash
-    # @option parent [BlockScore::Base] :parent
-    # @option target [Class] :target
+    # @param [BlockScore::Base] parent
+    # @param [Class] class of collection members
     #
     # @return [undefined]
     #
     # @api private
-    def initialize(params)
-      @parent = params.fetch :parent
-      @target = params.fetch :target
+    def initialize(parent, member_class)
+      @parent       = parent
+      @member_class = member_class
       register_parent_data
     end
 
@@ -42,7 +40,7 @@ def all
       self
     end
 
-    # Initializes new {target} with `params`
+    # Initializes new {member_class} with `params`
     #
     # - Ensures a parent id is meged into `params` (see #default_params).
     # - Defines method `#save` on new collection member
@@ -58,11 +56,11 @@ def all
     #
     # @param params [Hash] initial params for member
     #
-    # @return instance of {target}
+    # @return instance of {member_class}
     #
     # @api public
     def new(params = {})
-      item = target.new(params.merge(default_params))
+      item = member_class.new(params.merge(default_params))
       ctxt = self
       item.define_singleton_method(:save) do
         ctxt.parent.save unless ctxt.parent.id
@@ -99,18 +97,6 @@ def parent_name
       parent.class.resource
     end
 
-    # Name of target resource
-    #
-    # @example
-    #   self.target_name # => 'question_sets'
-    #
-    # @return [String]
-    #
-    # @api private
-    def target_name
-      target.resource
-    end
-
     # Initialize a collection member and save it
     #
     # @example
@@ -124,13 +110,13 @@ def target_name
     #
     # @param params [Hash] params
     #
-    # @return new saved instance of {target}
+    # @return new saved instance of {member_class}
     #
     # @api public
     def create(params = {})
       fail Error, 'Create parent first' unless parent.id
       assoc_params = default_params.merge(params)
-      item = target.create assoc_params
+      item = member_class.create(assoc_params)
       register_to_parent(item)
     end
 
@@ -141,30 +127,28 @@ def create(params = {})
     #
     # @param id [String] resource id
     #
-    # @return instance of {target} if found
+    # @return instance of {member_class} if found
     # @raise [BlockScore::NotFoundError] otherwise
     #
     # @api public
     def retrieve(id)
-      return self[data.index(id)] if data.include? id
-      item = target.retrieve(id)
+      return self[ids.index(id)] if ids.include?(id)
+      item = member_class.retrieve(id)
       register_to_parent(item)
     end
 
     protected
 
-    # @!attribute [r] target
+    # @!attribute [r] member_class
     # class which will be used for the embedded
     # resources in the collection
     #
-    # @todo rename to `target_class`?
-    #
     # @return [Class]
     #
     # @api private
-    attr_reader :target
+    attr_reader :member_class
 
-    # Default params for making an instance of {target}
+    # Default params for making an instance of {member_class}
     #
     # @return [Hash]
     #
@@ -198,7 +182,7 @@ def parent_id?(item)
     # @api private
     def register_to_parent(item)
       fail Error, 'None belonging' unless parent_id?(item)
-      data << item.id
+      ids << item.id
       self << item
       item
     end
@@ -209,21 +193,19 @@ def register_to_parent(item)
     #
     # @api private
     def register_parent_data
-      data.each do |id|
-        item = target.retrieve(id)
+      ids.each do |id|
+        item = member_class.retrieve(id)
         self << item
       end
     end
 
     # ids that belong to the collection
     #
-    # @todo rename to `ids`?
-    #
     # @return [Array<String>]
     #
     # @api private
-    def data
-      parent.attributes.fetch(:"#{ Util.to_plural(target_name) }", [])
+    def ids
+      parent.attributes.fetch(:"#{Util.to_plural(member_class.resource)}", [])
     end
   end
 end
diff --git a/lib/blockscore/person.rb b/lib/blockscore/person.rb
index fa51a1d..b26e3a4 100644
--- a/lib/blockscore/person.rb
+++ b/lib/blockscore/person.rb
@@ -11,7 +11,7 @@ class Person < Base
 
     def initialize(options = {})
       super
-      @question_sets = Collection.new(parent: self, target: QuestionSet)
+      @question_sets = Collection.new(self, QuestionSet)
     end
 
     def valid?

From 68a8968c7397ff6896b0abb79b3a66a4d1feb7ec Mon Sep 17 00:00:00 2001
From: John Backus <jbackus@stanford.edu>
Date: Tue, 8 Sep 2015 17:48:01 -0700
Subject: [PATCH 14/21] [WIP] Add collection member class

---
 lib/blockscore.rb                         |  2 +
 lib/blockscore/collection.rb              | 39 ++++++----
 lib/blockscore/collection/member.rb       | 87 +++++++++++++++++++++++
 spec/unit/blockscore/person_spec.rb       |  2 +-
 spec/unit/blockscore/question_set_spec.rb | 15 ++--
 5 files changed, 122 insertions(+), 23 deletions(-)
 create mode 100644 lib/blockscore/collection/member.rb

diff --git a/lib/blockscore.rb b/lib/blockscore.rb
index 01f05eb..1107a50 100644
--- a/lib/blockscore.rb
+++ b/lib/blockscore.rb
@@ -1,3 +1,4 @@
+require 'delegate'
 require 'forwardable'
 require 'httparty'
 require 'json'
@@ -26,6 +27,7 @@
 require 'blockscore/watchlist_hit'
 
 require 'blockscore/collection'
+require 'blockscore/collection/member'
 require 'blockscore/connection'
 require 'blockscore/dispatch'
 require 'blockscore/fingerprint'
diff --git a/lib/blockscore/collection.rb b/lib/blockscore/collection.rb
index 93985c9..c151c74 100644
--- a/lib/blockscore/collection.rb
+++ b/lib/blockscore/collection.rb
@@ -60,15 +60,12 @@ def all
     #
     # @api public
     def new(params = {})
-      item = member_class.new(params.merge(default_params))
-      ctxt = self
-      item.define_singleton_method(:save) do
-        ctxt.parent.save unless ctxt.parent.id
-        send :"#{ctxt.parent_name}_id=", ctxt.parent.id
-        super()
+      attributes = params.merge(default_params)
+      instance   = member_class.new(attributes)
+
+      new_member(instance) do |member|
+        self << member
       end
-      self << item
-      item
     end
 
     # Relaod the contents of the collection
@@ -116,8 +113,11 @@ def parent_name
     def create(params = {})
       fail Error, 'Create parent first' unless parent.id
       assoc_params = default_params.merge(params)
-      item = member_class.create(assoc_params)
-      register_to_parent(item)
+      instance = member_class.create(assoc_params)
+
+      new_member(instance) do |member|
+        register_to_parent(member)
+      end
     end
 
     # Retrieve a collection member by its id
@@ -133,8 +133,11 @@ def create(params = {})
     # @api public
     def retrieve(id)
       return self[ids.index(id)] if ids.include?(id)
-      item = member_class.retrieve(id)
-      register_to_parent(item)
+      instance = member_class.retrieve(id)
+
+      new_member(instance) do |member|
+        register_to_parent(member)
+      end
     end
 
     protected
@@ -161,6 +164,18 @@ def default_params
 
     private
 
+    # Initialize a new collection member
+    #
+    # @param instance [BlockScore::Base] collection member instance
+    # @yield [Member] initialized member
+    #
+    # @return [Member] new meber
+    #
+    # @api private
+    def new_member(instance, &blk)
+      Member.new(parent, instance).tap(&blk)
+    end
+
     # Check if `parent_id` is defined on `item`
     #
     # @param item [BlockScore::Base] any resource
diff --git a/lib/blockscore/collection/member.rb b/lib/blockscore/collection/member.rb
new file mode 100644
index 0000000..2230257
--- /dev/null
+++ b/lib/blockscore/collection/member.rb
@@ -0,0 +1,87 @@
+module BlockScore
+  class Collection
+    # Member of a {Collection} class
+    class Member < SimpleDelegator
+      # Initialize a new member
+      #
+      # @param parent [BlockScore::Base] parent resource
+      # @param instance [BlockScore::Base] member instance
+      #
+      # @return [undefined]
+      #
+      # @api private
+      def initialize(parent, instance)
+        @instance = instance
+        @parent = parent
+
+        super(instance)
+      end
+
+      # Save parent, set parent id, and save instance
+      #
+      # @example
+      #   # saves both unsaved person and unsaved question_set
+      #   person = Person.new(attributes)
+      #   question_set = QuestionSet.new
+      #   Member.new(person, question_set).save
+      #
+      # @return return value of instance `#save`
+      #
+      # @api public
+      def save
+        save_parent
+        send(:"#{parent_name}_id=", parent.id)
+        instance.save
+      end
+
+      private
+
+      # Name of parent resource
+      #
+      # @example
+      #   self.parent_name # => 'person'
+      #
+      # @return [String]
+      #
+      # @api private
+      def parent_name
+        parent.class.resource
+      end
+
+      # Save parent if it hasn't already been saved
+      #
+      # @return return of parent.save if previously unsaved
+      # @return nil otherwise
+      #
+      # @api private
+      def save_parent
+        parent.save unless parent_saved?
+      end
+
+      # Check if parent is saved
+      #
+      # @return [Boolean]
+      #
+      # @api private
+      def parent_saved?
+        parent.id
+      end
+
+      # @!attribute [r] instance
+      # member instance methods are delegated to
+      #
+      # @return [BlockScore::Base]
+      #
+      # @api private
+      attr_reader :instance
+
+      # @!attribute [r] parent
+      # collection parent the collectino conditionally updates
+      #
+      # @return [BlockScore::Base]
+      #
+      # @api private
+      attr_reader :parent
+    end
+  end
+end
diff --git a/spec/unit/blockscore/person_spec.rb b/spec/unit/blockscore/person_spec.rb
index e32e2e4..0cce797 100644
--- a/spec/unit/blockscore/person_spec.rb
+++ b/spec/unit/blockscore/person_spec.rb
@@ -7,7 +7,7 @@ module BlockScore
       let(:person) { create(:person, question_set_count: 1) }
       subject(:question_sets) { person.question_sets }
       its(:size) { should eql(1) }
-      its(:data) { should be(person.attributes[:question_sets]) }
+      its(:ids) { should be(person.attributes[:question_sets]) }
       its(:first) { should be_an_instance_of(QuestionSet) }
 
       it 'should load attributes' do
diff --git a/spec/unit/blockscore/question_set_spec.rb b/spec/unit/blockscore/question_set_spec.rb
index 587e21a..9f99a0f 100644
--- a/spec/unit/blockscore/question_set_spec.rb
+++ b/spec/unit/blockscore/question_set_spec.rb
@@ -45,11 +45,11 @@ module BlockScore
         let(:person) { create(:person) }
         subject { person.question_sets.retrieve(qs.id) }
 
-        it { should eql(qs) }
+        it { should eq(qs) }
 
         it 'retrieves and checks included person data' do
           data = [resource_id]
-          allow(person.question_sets).to receive(:data) { data }
+          allow(person.question_sets).to receive(:ids) { data }
           aggregate_failures('retrieves from data') do
             expect(data).to receive(:include?).and_call_original
             expect(data).to receive(:[]).and_call_original
@@ -69,12 +69,12 @@ module BlockScore
 
         it 'question_sets uses QuestionSet.retrieve' do
           expect(QuestionSet).to receive(:retrieve).with(qs.id) { qs }
-          expect(subject).to be(qs)
+          expect(subject).to eq(qs)
         end
 
         it 'registers new question set' do
           aggregate_failures('for person and collection') do
-            expect(person.question_sets).to receive(:<<).with(qs).and_call_original
+            expect(person.question_sets).to receive(:<<).and_call_original
             expect(data).to receive(:<<).with(qs.id).and_call_original
           end
           person.question_sets.retrieve(qs.id)
@@ -104,7 +104,6 @@ module BlockScore
         args = { person_id: person.id }
         expect(QuestionSet).to receive(:new).with(args).and_call_original
         result = person.question_sets.new
-        expect(result).to be_an_instance_of(QuestionSet)
       end
 
       it 'should save person if not save' do
@@ -156,10 +155,6 @@ module BlockScore
         expect(person.question_sets).to include(subject)
       end
 
-      it 'should return question set' do
-        is_expected.to be_an_instance_of(QuestionSet)
-      end
-
       it 'should error if parent not created' do
         person = Person.new
         expect { person.question_sets.create }.to raise_error(Error, 'Create parent first')
@@ -167,7 +162,7 @@ module BlockScore
 
       it 'should retrieve from registered' do
         found_qs = person.question_sets.retrieve(subject.id)
-        expect(subject).to be (found_qs)
+        expect(subject).to eq(found_qs)
       end
     end
 

From f43d11f0ff18b188c89189ee840a0cd2faecd446 Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Wed, 9 Sep 2015 13:07:52 -0700
Subject: [PATCH 15/21] Add #saved? to base and spec

---
 lib/blockscore.rb                   | 6 +-----
 lib/blockscore/base.rb              | 4 ++++
 spec/unit/blockscore/person_spec.rb | 7 +++++++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/blockscore.rb b/lib/blockscore.rb
index 1107a50..71e547a 100644
--- a/lib/blockscore.rb
+++ b/lib/blockscore.rb
@@ -37,10 +37,6 @@
 
 module BlockScore
   class << self
-    attr_writer :api_key
-  end
-
-  class << self
-    attr_reader :api_key
+    attr_accessor :api_key
   end
 end
diff --git a/lib/blockscore/base.rb b/lib/blockscore/base.rb
index 4cc55ca..25692a2 100644
--- a/lib/blockscore/base.rb
+++ b/lib/blockscore/base.rb
@@ -68,6 +68,10 @@ def self.endpoint
       "#{api_url}#{Util.to_plural(resource)}"
     end
 
+    def saved?
+      !!id
+    end
+
     protected
 
     def add_accessor(symbol, *_args)
diff --git a/spec/unit/blockscore/person_spec.rb b/spec/unit/blockscore/person_spec.rb
index 0cce797..81b0761 100644
--- a/spec/unit/blockscore/person_spec.rb
+++ b/spec/unit/blockscore/person_spec.rb
@@ -15,6 +15,13 @@ module BlockScore
       end
     end
 
+    it "#saved?" do
+      saved_person = create(:person)
+      new_person = Person.new
+      expect(saved_person.saved?).to be(true)
+      expect(new_person.saved?).to be(false)
+    end
+
     it '#id' do
       person = create(:person)
       expect(person.id).not_to be(nil)

From a4dd8e1469e7aad3bd375ac20f479d0613185b93 Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Thu, 10 Sep 2015 09:58:12 -0700
Subject: [PATCH 16/21] Add initial changes for spec refactor

Refactors specs related to collection from question sets to separate
spec and moves some member related specs out of collection specs. Still
requires more specs on Member to raise mutant coverage. It also renames
the Base#saved? to Base#persisted?.
---
 lib/blockscore/actions/retrieve.rb        |   2 +-
 lib/blockscore/base.rb                    |  12 +-
 lib/blockscore/collection.rb              |  20 ++-
 lib/blockscore/collection/member.rb       |  15 +-
 spec/factories.rb                         |  23 +++
 spec/unit/blockscore/actions_spec.rb      |  10 ++
 spec/unit/blockscore/collection_spec.rb   | 172 ++++++++++++++++++++++
 spec/unit/blockscore/member_spec.rb       |  50 +++++++
 spec/unit/blockscore/person_spec.rb       |   8 +-
 spec/unit/blockscore/question_set_spec.rb | 148 -------------------
 10 files changed, 296 insertions(+), 164 deletions(-)
 create mode 100644 spec/unit/blockscore/collection_spec.rb
 create mode 100644 spec/unit/blockscore/member_spec.rb

diff --git a/lib/blockscore/actions/retrieve.rb b/lib/blockscore/actions/retrieve.rb
index 166f28d..87725d7 100644
--- a/lib/blockscore/actions/retrieve.rb
+++ b/lib/blockscore/actions/retrieve.rb
@@ -14,7 +14,7 @@ module ClassMethods
         def retrieve(id, options = {})
           fail ArgumentError if id.empty?
           req = ->() { get("#{endpoint}/#{id}", options) }
-          new(&req)
+          new(id: id, &req)
         end
       end
 
diff --git a/lib/blockscore/base.rb b/lib/blockscore/base.rb
index 25692a2..b56d3b4 100644
--- a/lib/blockscore/base.rb
+++ b/lib/blockscore/base.rb
@@ -24,7 +24,7 @@ def force!
     end
 
     def id
-      attributes.fetch(:id, nil)
+      @attributes.fetch(:id, nil)
     end
 
     def inspect
@@ -33,8 +33,8 @@ def inspect
     end
 
     def refresh
-      r = self.class.retrieve(id)
-      @attributes = r.attributes
+      res = self.class.retrieve(id)
+      @attributes = res.attributes
 
       true
     rescue Error
@@ -63,13 +63,13 @@ def self.api_url
     end
 
     def self.endpoint
-      fail NotImplementedError, 'Base is an abstract class, not an API resource' if self == Base
+      fail NotImplementedError, 'Base is an abstract class, not an API resource' if equal?(Base)
 
       "#{api_url}#{Util.to_plural(resource)}"
     end
 
-    def saved?
-      !!id
+    def persisted?
+      !id.nil?
     end
 
     protected
diff --git a/lib/blockscore/collection.rb b/lib/blockscore/collection.rb
index c151c74..f08762b 100644
--- a/lib/blockscore/collection.rb
+++ b/lib/blockscore/collection.rb
@@ -132,7 +132,10 @@ def create(params = {})
     #
     # @api public
     def retrieve(id)
-      return self[ids.index(id)] if ids.include?(id)
+      each do |item|
+        return item if item.id == id
+      end
+
       instance = member_class.retrieve(id)
 
       new_member(instance) do |member|
@@ -158,18 +161,27 @@ def retrieve(id)
     # @api private
     def default_params
       {
-        :"#{parent_name}_id" => parent.id
+        foriegn_key => parent.id
       }
     end
 
     private
 
+    # Generate foriegn key name for parent resource
+    #
+    # @return [Symbol] resource name as id
+    #
+    # @api private
+    def foriegn_key
+      :"#{parent_name}_id"
+    end
+
     # Initialize a new collection member
     #
     # @param instance [BlockScore::Base] collection member instance
     # @yield [Member] initialized member
     #
-    # @return [Member] new meber
+    # @return [Member] new member
     #
     # @api private
     def new_member(instance, &blk)
@@ -184,7 +196,7 @@ def new_member(instance, &blk)
     #
     # @api private
     def parent_id?(item)
-      parent.id && item.send(:"#{parent_name}_id") == parent.id
+      parent.id && item.send(foriegn_key) == parent.id
     end
 
     # Register a resource in collection
diff --git a/lib/blockscore/collection/member.rb b/lib/blockscore/collection/member.rb
index 2230257..ff50e08 100644
--- a/lib/blockscore/collection/member.rb
+++ b/lib/blockscore/collection/member.rb
@@ -31,7 +31,9 @@ def initialize(parent, instance)
       def save
         save_parent
         send(:"#{parent_name}_id=", parent.id)
-        instance.save
+        result = instance.save
+        ids.push(instance.id) unless ids.include?(instance.id)
+        result
       end
 
       private
@@ -82,6 +84,17 @@ def parent_saved?
       #
       # @api private
       attr_reader :parent
+
+      private
+
+      # ids that belong to associated parent resource
+      #
+      # @return [Array<String>]
+      #
+      # @api private
+      def ids
+        parent.attributes.fetch(:"#{Util.to_plural(instance.class.resource)}", [])
+      end
     end
   end
 end
diff --git a/spec/factories.rb b/spec/factories.rb
index 0dceee0..c5d8b45 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -179,6 +179,29 @@ def resource_id
     details { build(:company_details) }
   end
 
+  factory :fake_member, class: 'BlockScore::FakeResource' do
+    object { 'fake_member' }
+    transient do
+      given_id resource_id
+      parent_id resource_id
+    end
+
+    id { given_id }
+    fake_resource_id { parent_id }
+  end
+
+  factory :fake_resource, class: 'BlockScore::FakeResource' do
+    object { 'fake_resource' }
+    metadata
+    transient { members_count 2 }
+
+    fake_resources do
+      members_count.times.map do
+        resource_id
+      end
+    end
+  end
+
   factory :person_params, class: 'BlockScore::Person' do
     name
     document
diff --git a/spec/unit/blockscore/actions_spec.rb b/spec/unit/blockscore/actions_spec.rb
index e7d6509..c3aa3c7 100644
--- a/spec/unit/blockscore/actions_spec.rb
+++ b/spec/unit/blockscore/actions_spec.rb
@@ -45,6 +45,16 @@ module BlockScore
         should receive(:request).with(:get, route, {}).once { resource }
         mock.retrieve('abc123').attributes
       end
+
+      context 'when id is invalid' do
+        pending 'should raise not found if not a resource style id' do
+          expect { mock.retrieve('bad_id') }.to raise_error(NotFoundError)
+        end
+
+        it 'should raise ArgumentError if empty' do
+          expect { mock.retrieve('') }.to raise_error(ArgumentError)
+        end
+      end
     end
 
     describe '#update' do
diff --git a/spec/unit/blockscore/collection_spec.rb b/spec/unit/blockscore/collection_spec.rb
new file mode 100644
index 0000000..f9eaa70
--- /dev/null
+++ b/spec/unit/blockscore/collection_spec.rb
@@ -0,0 +1,172 @@
+module BlockScore
+  RSpec.describe Collection do
+    let(:parent) { create(:fake_resource) }
+    let(:member_class) { FakeResource }
+    let(:collection) { Collection.new(parent, member_class) }
+
+    before do
+      Util::PLURAL_LOOKUP['fake_resource'] = 'fake_resources'
+      allow(member_class).to receive(:create) { create(:fake_member, parent_id: parent.id) }
+    end
+
+    describe '#all' do
+      subject { collection.all  }
+
+      it 'should return self' do
+        is_expected.to be(collection)
+      end
+    end
+
+    describe '#create' do
+      let(:created) { create(:fake_member, parent_id: parent.id) }
+      subject { collection.create }
+      before(:each) do
+        allow(member_class).to receive(:create) { created }
+      end
+
+      it 'should update ids in Parent#attributes' do
+        expect(parent.attributes[:fake_resources]).to include(subject.id)
+      end
+
+      it 'should add to collection' do
+        expect(collection).to include(subject)
+      end
+
+      it 'should error if parent not created' do
+        parent = FakeResource.new
+        collection = Collection.new(parent, member_class)
+        expect { collection.create }.to raise_error(Error, 'Create parent first')
+      end
+
+      it 'should retrieve item from registered' do
+        found_qs = collection.retrieve(subject.id)
+        expect(subject).to eq(found_qs)
+      end
+    end
+
+    describe '#new' do
+      subject { collection.new }
+
+      it 'should add to collection' do
+        count = collection.size
+        result = collection.new
+        expect(collection.size).to eq(count + 1)
+        expect(collection).to include(result)
+      end
+
+      context 'when saving' do
+        it 'should have parent#id after saving' do
+          subject.fake_resource_id = 'some_id'
+          subject.save
+          expect(subject.fake_resource_id).to eq(parent.id)
+        end
+
+        it 'should be retrievable after' do
+          collection.create
+          subject.save
+          collection.create
+          expect(collection.retrieve(subject.id)).to be(subject)
+        end
+      end
+    end
+
+    describe '#retrieve' do
+      context 'when in person#attributes' do
+        let(:parent) { create(:fake_resource) }
+        let(:existing_id) { parent.attributes[:fake_resources].first }
+        subject { collection.retrieve(existing_id) }
+
+        it 'should retrieve correctly' do
+          first = collection.first
+          expect(subject).to eq(first)
+          expect(subject.id).to eq(existing_id)
+        end
+
+        it 'checks collect' do
+          expect(collection).to receive(:each).and_call_original
+          collection.retrieve(existing_id)
+        end
+      end
+
+      context 'when not in person#attributes' do
+        let(:parent) { create(:fake_resource, members_count: 0) }
+        let(:collection) { Collection.new(parent, member_class) }
+        let(:item) { create(:fake_member, parent_id: parent.id) }
+        let(:data) { parent.attributes.fetch(:fake_resources) }
+        subject { collection.retrieve(item.id) }
+
+        before(:each) do
+          allow(member_class).to receive(:retrieve).with(item.id) { item }
+        end
+
+        it 'uses member_class.retrieve' do
+          expect(member_class).to receive(:retrieve).with(item.id).and_return(item)
+          expect(collection.retrieve(item.id)).to eq(item)
+        end
+
+        it 'registers new question set' do
+          aggregate_failures('for person and collection') do
+            expect(collection).to receive(:<<).and_call_original
+            expect(data).to receive(:<<).with(item.id).and_call_original
+          end
+          collection.retrieve(item.id)
+        end
+
+        it 'errors if not belonging' do
+          item.fake_resource_id = 'some_not_associated_id'
+          expect { subject }.to raise_error(Error, 'None belonging')
+        end
+      end
+    end
+
+    describe '#refresh' do
+      subject { collection }
+      let(:item) { member_class.create }
+      before(:each) { parent.attributes[:fake_resources] << item.id }
+
+      # refactor
+      it 'should register new data from parent' do
+        parent.attributes[:fake_resources].clear
+        parent.attributes[:fake_resources].push(item.id)
+        expect(member_class).to receive(:retrieve).with(item.id).at_least(:twice).and_call_original
+        subject.refresh
+      end
+
+      it 'should clear and reload' do
+        expect(subject).to receive(:clear).and_call_original
+        expect(subject).to receive(:register_parent_data).and_call_original
+        result = subject.refresh
+        expect(subject).to be(result)
+      end
+
+      context 'when creating' do
+        it 'should be included in refresh' do
+          result = subject.create
+          subject.refresh
+          expect(subject.map(&:id)).to include(result.id)
+        end
+
+        it 'should include saved in refresh' do
+          item = FakeResource.new(fake_resource_id: parent.id)
+          allow(member_class).to receive(:new) { item }
+          allow(item).to receive(:save) { create(:fake_member, parent_id: parent.id) }
+          result = subject.new
+          result.save
+          subject.refresh
+          expect(subject.map(&:id)).to include(result.id)
+        end
+      end
+
+      context 'when retrieving belonging' do
+        it 'should be included in refresh' do
+          item = create(:fake_resource)
+          item.parent_id = parent.id
+          allow(member_class).to receive(:retrieve) { item }
+          result = subject.retrieve(item.id)
+          subject.refresh
+          expect(subject.map(&:id)).to include(result.id)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/unit/blockscore/member_spec.rb b/spec/unit/blockscore/member_spec.rb
new file mode 100644
index 0000000..e06f1c5
--- /dev/null
+++ b/spec/unit/blockscore/member_spec.rb
@@ -0,0 +1,50 @@
+module BlockScore
+  RSpec.describe Collection::Member do
+    let(:parent) { create(:fake_resource) }
+    let(:member_class) { FakeResource }
+    let(:collection) { Collection.new(parent, member_class) }
+    before do
+      Util::PLURAL_LOOKUP['fake_resource'] = 'fake_resources'
+      allow(member_class).to receive(:create) { create(:fake_member, parent_id: parent.id) }
+    end
+
+    context 'when created' do
+      let(:created) { create(:fake_member, parent_id: parent.id) }
+      subject { collection.create }
+      before(:each) do
+        allow(member_class).to receive(:create) { created }
+      end
+
+      it 'should have the Parent#id' do
+        expect(member_class).to receive(:create).with(fake_resource_id: parent.id).and_call_original
+        subject
+      end
+    end
+
+    context 'when instantiating a new' do
+      subject { collection.new }
+
+      it 'should delegate to member_class' do
+        # odd bug... test fails when replaced with parent.id
+        args = { fake_resource_id: collection.parent.id }
+        expect(member_class).to receive(:new).with(args).and_call_original
+        collection.new
+      end
+
+      it 'should save parent if not persisted' do
+        parent = create(:fake_resource)
+        parent.id = nil
+        collection = Collection.new(parent, member_class)
+        item = collection.new
+        expect(parent).to receive(:save).and_call_original
+        item.save
+      end
+
+      it 'should have Parent#id' do
+        subject.person_id = 'some_id'
+        subject.save
+        expect(subject.fake_resource_id).to eq(collection.parent.id)
+      end
+    end
+  end
+end
diff --git a/spec/unit/blockscore/person_spec.rb b/spec/unit/blockscore/person_spec.rb
index 81b0761..a840832 100644
--- a/spec/unit/blockscore/person_spec.rb
+++ b/spec/unit/blockscore/person_spec.rb
@@ -15,11 +15,11 @@ module BlockScore
       end
     end
 
-    it "#saved?" do
-      saved_person = create(:person)
+    it '#persisted?' do
+      persisted_person = create(:person)
       new_person = Person.new
-      expect(saved_person.saved?).to be(true)
-      expect(new_person.saved?).to be(false)
+      expect(persisted_person.persisted?).to be(true)
+      expect(new_person.persisted?).to be(false)
     end
 
     it '#id' do
diff --git a/spec/unit/blockscore/question_set_spec.rb b/spec/unit/blockscore/question_set_spec.rb
index 9f99a0f..9833907 100644
--- a/spec/unit/blockscore/question_set_spec.rb
+++ b/spec/unit/blockscore/question_set_spec.rb
@@ -39,153 +39,5 @@ module BlockScore
       end
     end
 
-    describe '#retrieve' do
-      context 'when in person#attributes' do
-        let(:qs) { person.question_sets.create }
-        let(:person) { create(:person) }
-        subject { person.question_sets.retrieve(qs.id) }
-
-        it { should eq(qs) }
-
-        it 'retrieves and checks included person data' do
-          data = [resource_id]
-          allow(person.question_sets).to receive(:ids) { data }
-          aggregate_failures('retrieves from data') do
-            expect(data).to receive(:include?).and_call_original
-            expect(data).to receive(:[]).and_call_original
-          end
-          person.question_sets.retrieve(data[0])
-        end
-      end
-
-      context 'when not in person#attributes' do
-        let(:qs) { QuestionSet.new(person_id: person.id, id: resource_id) }
-        let(:data) { person.attributes.fetch :question_sets }
-        subject { person.question_sets.retrieve(qs.id) }
-
-        before(:each) do
-          allow(QuestionSet).to receive(:retrieve).with(qs.id) { qs }
-        end
-
-        it 'question_sets uses QuestionSet.retrieve' do
-          expect(QuestionSet).to receive(:retrieve).with(qs.id) { qs }
-          expect(subject).to eq(qs)
-        end
-
-        it 'registers new question set' do
-          aggregate_failures('for person and collection') do
-            expect(person.question_sets).to receive(:<<).and_call_original
-            expect(data).to receive(:<<).with(qs.id).and_call_original
-          end
-          person.question_sets.retrieve(qs.id)
-        end
-
-        it 'errors if not belonging' do
-          qs.person_id = 'some_not_associated_id'
-          expect { subject }.to raise_error(Error, 'None belonging')
-        end
-      end
-    end
-
-    context 'when id is invalid' do
-      pending 'should raise not found if not a resource style id' do
-        expect { person.question_sets.retrieve('bad_id') }.to raise_error(NotFoundError)
-      end
-
-      it 'should raise ArgumentError if empty' do
-        expect { person.question_sets.retrieve('') }.to raise_error(ArgumentError)
-      end
-    end
-
-    describe '#new' do
-      subject { person.question_sets }
-
-      it 'should delegate to QuestionSet' do
-        args = { person_id: person.id }
-        expect(QuestionSet).to receive(:new).with(args).and_call_original
-        result = person.question_sets.new
-      end
-
-      it 'should save person if not save' do
-        person = Person.new
-        qs = person.question_sets.new
-        expect(person).to receive(:save).and_call_original
-        qs.save
-      end
-
-      it 'should have person#id' do
-        qs = person.question_sets.new
-        qs.person_id = 'some_id'
-        qs.save
-        expect(qs.person_id).to eq(person.id)
-        expect(qs.id).not_to be(nil)
-      end
-
-      it 'should add to question_sets' do
-        count = subject.size
-        qs = subject.new
-        expect(subject.size).to eq(count + 1)
-        expect(subject).to include(qs)
-      end
-    end
-
-    describe '#all' do
-      subject { person.question_sets.all }
-      it 'should return self' do
-        is_expected.to be(person.question_sets)
-      end
-    end
-
-    describe '#create' do
-      let (:person) { create(:person) }
-      let (:data) { person.attributes[:question_sets] }
-      let (:qs) { QuestionSet.create(person_id: person.id) }
-      subject { person.question_sets.create }
-
-      it 'should pass the Person#id' do
-        expect(QuestionSet).to receive(:create).with(person_id: person.id).and_call_original
-        subject
-      end
-
-      it 'should update Person#attributes' do
-        expect(data).to include(subject.id)
-      end
-
-      it 'should update question_sets collection' do
-        expect(person.question_sets).to include(subject)
-      end
-
-      it 'should error if parent not created' do
-        person = Person.new
-        expect { person.question_sets.create }.to raise_error(Error, 'Create parent first')
-      end
-
-      it 'should retrieve from registered' do
-        found_qs = person.question_sets.retrieve(subject.id)
-        expect(subject).to eq(found_qs)
-      end
-    end
-
-    describe '#refresh' do
-      subject { person.question_sets }
-      let(:qs) { QuestionSet.create }
-      before(:each) { person.attributes[:question_sets] << qs.id }
-
-      it 'should register new data from parent' do
-        person.attributes[:question_sets].clear
-        person.attributes[:question_sets].push(qs.id)
-        expect(QuestionSet).to receive(:retrieve).with(qs.id).and_call_original
-        subject.refresh
-      end
-
-      it 'should clear and reload' do
-        expect(subject).to receive(:clear).and_call_original
-        expect(QuestionSet).to receive(:retrieve).and_call_original
-        expect(QuestionSet).to receive(:retrieve).with(qs.id) { qs.clone }
-        result = subject.refresh
-        expect(subject.count).to be(2)
-        expect(subject).to be(result)
-      end
-    end
   end
 end
\ No newline at end of file

From fc05a5bf371be03745568723f6712fa57d2d66dc Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Thu, 10 Sep 2015 11:44:15 -0700
Subject: [PATCH 17/21] Add README updated testing instructions

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index ad2c698..a406eae 100644
--- a/README.md
+++ b/README.md
@@ -60,5 +60,5 @@ To see the list of calls you can make, please visit our [full Ruby API reference
 The test suite uses a public BlockScore API key that was created specifically to ease the testing and contribution processes. **Please do not enter personal details for tests.** In order to run the test suite:
 
 ```shell
-$ rake test
+$ rspec spec
 ```

From 999a725638dbac8b796e6ce984c78ae0ca119e1c Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Thu, 10 Sep 2015 16:41:57 -0700
Subject: [PATCH 18/21] Add member spec and bug fixes related to Base#id

---
 lib/blockscore/actions/update.rb          |  2 +-
 lib/blockscore/base.rb                    |  1 +
 lib/blockscore/collection.rb              |  3 +-
 spec/unit/blockscore/collection_spec.rb   | 12 ++++--
 spec/unit/blockscore/member_spec.rb       | 47 +++++++++++++++++++++++
 spec/unit/blockscore/question_set_spec.rb |  3 +-
 6 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/lib/blockscore/actions/update.rb b/lib/blockscore/actions/update.rb
index df9e25e..1de39ed 100644
--- a/lib/blockscore/actions/update.rb
+++ b/lib/blockscore/actions/update.rb
@@ -28,7 +28,7 @@ module Update
       def_delegators 'self.class', :endpoint, :patch
 
       def save!
-        if respond_to? :id
+        if persisted?
           patch("#{endpoint}/#{id}", filter_params)
           true
         else
diff --git a/lib/blockscore/base.rb b/lib/blockscore/base.rb
index b56d3b4..c24f5c9 100644
--- a/lib/blockscore/base.rb
+++ b/lib/blockscore/base.rb
@@ -49,6 +49,7 @@ def save
 
     def save!
       response = self.class.post(self.class.endpoint, attributes)
+      # binding.pry
       @attributes = response.attributes
 
       true
diff --git a/lib/blockscore/collection.rb b/lib/blockscore/collection.rb
index f08762b..b1b4f12 100644
--- a/lib/blockscore/collection.rb
+++ b/lib/blockscore/collection.rb
@@ -133,7 +133,8 @@ def create(params = {})
     # @api public
     def retrieve(id)
       each do |item|
-        return item if item.id == id
+        next unless item.id == id
+        return item
       end
 
       instance = member_class.retrieve(id)
diff --git a/spec/unit/blockscore/collection_spec.rb b/spec/unit/blockscore/collection_spec.rb
index f9eaa70..1ed9cd0 100644
--- a/spec/unit/blockscore/collection_spec.rb
+++ b/spec/unit/blockscore/collection_spec.rb
@@ -24,6 +24,12 @@ module BlockScore
         allow(member_class).to receive(:create) { created }
       end
 
+      it 'should send merged default and arg params' do
+        foriegn_key = :"#{parent.class.resource}_id"
+        expect(member_class).to receive(:create).with(foriegn_key => parent.id, foo_param: 'bar_attr')
+        collection.create(foo_param: 'bar_attr')
+      end
+
       it 'should update ids in Parent#attributes' do
         expect(parent.attributes[:fake_resources]).to include(subject.id)
       end
@@ -73,12 +79,12 @@ module BlockScore
     describe '#retrieve' do
       context 'when in person#attributes' do
         let(:parent) { create(:fake_resource) }
-        let(:existing_id) { parent.attributes[:fake_resources].first }
+        let(:existing_id) { parent.attributes[:fake_resources].last }
         subject { collection.retrieve(existing_id) }
 
         it 'should retrieve correctly' do
-          first = collection.first
-          expect(subject).to eq(first)
+          last = collection.last
+          expect(subject).to eq(last)
           expect(subject.id).to eq(existing_id)
         end
 
diff --git a/spec/unit/blockscore/member_spec.rb b/spec/unit/blockscore/member_spec.rb
index e06f1c5..b80ac79 100644
--- a/spec/unit/blockscore/member_spec.rb
+++ b/spec/unit/blockscore/member_spec.rb
@@ -8,6 +8,53 @@ module BlockScore
       allow(member_class).to receive(:create) { create(:fake_member, parent_id: parent.id) }
     end
 
+    context 'when saving' do
+      let(:member_class) { FakeResource }
+      let(:parent) {  FakeResource.new }
+      let(:collection) { Collection.new(parent, member_class) }
+      subject { collection.new }
+
+      it 'should save parent and self if parent not persisted' do
+        aggregate_failures('when saving') do
+          expect(parent).to receive(:save).and_call_original
+          expect(subject.save).to be(true)
+        end
+
+        aggregate_failures('after saving') do
+          expect(subject.persisted?).to be(true)
+          expect(parent.persisted?).to be(true)
+          foriegn_key = :"#{parent.class.resource}_id"
+          expect(subject.send(foriegn_key)).to eq(parent.id)
+        end
+      end
+
+      it 'should not save persisted parent' do
+        parent.save
+
+        aggregate_failures('when saving') do
+          expect(parent).not_to receive(:save)
+          expect(subject.save).to be(true)
+        end
+
+        aggregate_failures('after saving') do
+          expect(subject.persisted?).to be(true)
+          expect(parent.persisted?).to be(true)
+          foriegn_key = :"#{parent.class.resource}_id"
+          expect(subject.send(foriegn_key)).to eq(parent.id)
+        end
+      end
+
+      it 'should not add to parent if existing' do
+        subject.save
+        embedded_resource = :"#{Util.to_plural(parent.class.resource)}"
+        tracked_ids = parent.send(embedded_resource)
+        size = tracked_ids.size
+        subject.save
+        expect(size).to eql(size)
+        expect(tracked_ids).to include(subject.id)
+      end
+    end
+
     context 'when created' do
       let(:created) { create(:fake_member, parent_id: parent.id) }
       subject { collection.create }
diff --git a/spec/unit/blockscore/question_set_spec.rb b/spec/unit/blockscore/question_set_spec.rb
index 9833907..e2feac6 100644
--- a/spec/unit/blockscore/question_set_spec.rb
+++ b/spec/unit/blockscore/question_set_spec.rb
@@ -38,6 +38,5 @@ module BlockScore
         end
       end
     end
-
   end
-end
\ No newline at end of file
+end

From 969e450cacabc6068fbe47d2de20f22ed71e3b10 Mon Sep 17 00:00:00 2001
From: John Backus <jbackus@stanford.edu>
Date: Fri, 11 Sep 2015 13:07:04 -0700
Subject: [PATCH 19/21] Refactor a few specs in member_spec

---
 spec/unit/blockscore/member_spec.rb | 45 +++++++++++++++++------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/spec/unit/blockscore/member_spec.rb b/spec/unit/blockscore/member_spec.rb
index b80ac79..3b88b57 100644
--- a/spec/unit/blockscore/member_spec.rb
+++ b/spec/unit/blockscore/member_spec.rb
@@ -3,28 +3,35 @@ module BlockScore
     let(:parent) { create(:fake_resource) }
     let(:member_class) { FakeResource }
     let(:collection) { Collection.new(parent, member_class) }
+
     before do
       Util::PLURAL_LOOKUP['fake_resource'] = 'fake_resources'
       allow(member_class).to receive(:create) { create(:fake_member, parent_id: parent.id) }
     end
 
     context 'when saving' do
-      let(:member_class) { FakeResource }
-      let(:parent) {  FakeResource.new }
-      let(:collection) { Collection.new(parent, member_class) }
-      subject { collection.new }
+      let(:member_class) { FakeResource                         }
+      let(:parent)       { FakeResource.new                     }
+      let(:collection)   { Collection.new(parent, member_class) }
+      subject(:member)   { collection.new                       }
 
-      it 'should save parent and self if parent not persisted' do
-        aggregate_failures('when saving') do
-          expect(parent).to receive(:save).and_call_original
-          expect(subject.save).to be(true)
+      context 'when parent is not persisted' do
+        before { allow(parent).to receive(:save).and_call_original }
+
+        it { expect(member.save).to be(true) }
+
+        it do
+          member.save
+          expect(parent).to have_received(:save).once
         end
 
-        aggregate_failures('after saving') do
-          expect(subject.persisted?).to be(true)
-          expect(parent.persisted?).to be(true)
-          foriegn_key = :"#{parent.class.resource}_id"
-          expect(subject.send(foriegn_key)).to eq(parent.id)
+        context 'after saving' do
+          before { member.save }
+          let(:member_parent_id) { member.fake_resource_id }
+
+          it { is_expected.to be_persisted }
+          it { expect(parent).to be_persisted }
+          it { expect(member_parent_id).to eql(parent.id) }
         end
       end
 
@@ -33,25 +40,25 @@ module BlockScore
 
         aggregate_failures('when saving') do
           expect(parent).not_to receive(:save)
-          expect(subject.save).to be(true)
+          expect(member.save).to be(true)
         end
 
         aggregate_failures('after saving') do
-          expect(subject.persisted?).to be(true)
+          expect(member.persisted?).to be(true)
           expect(parent.persisted?).to be(true)
           foriegn_key = :"#{parent.class.resource}_id"
-          expect(subject.send(foriegn_key)).to eq(parent.id)
+          expect(member.send(foriegn_key)).to eq(parent.id)
         end
       end
 
       it 'should not add to parent if existing' do
-        subject.save
+        member.save
         embedded_resource = :"#{Util.to_plural(parent.class.resource)}"
         tracked_ids = parent.send(embedded_resource)
         size = tracked_ids.size
-        subject.save
+        member.save
         expect(size).to eql(size)
-        expect(tracked_ids).to include(subject.id)
+        expect(tracked_ids).to include(member.id)
       end
     end
 

From 59b6a293226468c8bdaaeddc7a8d58d9f7f1dc3e Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Mon, 14 Sep 2015 14:55:38 -0700
Subject: [PATCH 20/21] Update version

---
 README.md                 | 2 +-
 lib/blockscore/version.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index a406eae..f0a5147 100644
--- a/README.md
+++ b/README.md
@@ -13,7 +13,7 @@ gem install blockscore
 If you are using Rails, add the following to your `Gemfile`:
 
 ```ruby
-gem 'blockscore', '~> 4.1.2'
+gem 'blockscore', '~> 4.2.0'
 ```
 
 ## Getting Started
diff --git a/lib/blockscore/version.rb b/lib/blockscore/version.rb
index 3664c01..5f17869 100644
--- a/lib/blockscore/version.rb
+++ b/lib/blockscore/version.rb
@@ -1,3 +1,3 @@
 module BlockScore
-  VERSION = '4.1.2'.freeze
+  VERSION = '4.2.0'.freeze
 end

From ffb5320e00b497ee988aaa2a244f442f2522eaf8 Mon Sep 17 00:00:00 2001
From: DelmerGA <delmer@blockscore.com>
Date: Mon, 14 Sep 2015 15:27:15 -0700
Subject: [PATCH 21/21] Remove comments

There was a comment in base.rb that needed removal
---
 lib/blockscore/base.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/blockscore/base.rb b/lib/blockscore/base.rb
index c24f5c9..b56d3b4 100644
--- a/lib/blockscore/base.rb
+++ b/lib/blockscore/base.rb
@@ -49,7 +49,6 @@ def save
 
     def save!
       response = self.class.post(self.class.endpoint, attributes)
-      # binding.pry
       @attributes = response.attributes
 
       true