From 9439a72e626ab62c7a1d1c3e109324898994dfe3 Mon Sep 17 00:00:00 2001
From: Rafael George <george.rafael@gmail.com>
Date: Fri, 28 Jun 2013 08:32:16 -0400
Subject: [PATCH 1/5] Refactor: RecurlyAccountChecker#customer_exists?

To remove duplication because of the exception handling
---
 app/models/user.rb                            | 20 +++++++------------
 app/services/recurly_account_checker.rb       | 18 +++++++++++++++++
 .../services/recurly_customer_checker_spec.rb | 13 ++++++++++++
 3 files changed, 38 insertions(+), 13 deletions(-)
 create mode 100644 app/services/recurly_account_checker.rb
 create mode 100644 spec/services/recurly_customer_checker_spec.rb

diff --git a/app/models/user.rb b/app/models/user.rb
index 509f92a..dd7a341 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -4,7 +4,7 @@ class User < ActiveRecord::Base
   # :token_authenticatable, :confirmable,
   # :lockable, :timeoutable and :omniauthable
   devise :database_authenticatable, :registerable,
-         :recoverable, :rememberable, :trackable, :validatable
+    :recoverable, :rememberable, :trackable, :validatable
 
   # Setup accessible (or protected) attributes for your model
   attr_accessible :first_name, :last_name, :email, :password, :password_confirmation, :remember_me, :card_token, :customer_id
@@ -17,17 +17,13 @@ def name
   end
 
   def check_recurly
-    customer = Recurly::Account.find(customer_id) unless customer_id.nil?
-  rescue Recurly::Resource::NotFound => e
-    logger.error e.message
-    errors.add :base, "Unable to create your subscription. #{e.message}"
-    false
+    RecurlyAccountChecker.customer_exists?(customer_id)
   end
 
   def update_plan(role)
+    customer = Recurly::Account.find(customer_id) unless customer_id.nil?
     self.role_ids = []
     self.add_role(role.name)
-    customer = Recurly::Account.find(customer_id) unless customer_id.nil?
     unless customer.nil?
       subscription = customer.subscriptions.first
       subscription.update_attributes! :timeframe => 'now', :plan_code => role.name
@@ -54,12 +50,10 @@ def update_recurly
   end
 
   def cancel_subscription
-    unless customer_id.nil?
-      customer = Recurly::Account.find(customer_id)
-      subscription = customer.subscriptions.first unless customer.nil?
-      if (!subscription.nil?) && (subscription.state == 'active')
-        subscription.cancel
-      end
+    customer = Recurly::Account.find(customer_id) unless customer_id.nil?
+    subscription = customer.subscriptions.first unless customer.nil?
+    if (!subscription.nil?) && (subscription.state == 'active')
+      subscription.cancel
     end
   rescue Recurly::Resource::NotFound => e
     logger.error e.message
diff --git a/app/services/recurly_account_checker.rb b/app/services/recurly_account_checker.rb
new file mode 100644
index 0000000..995a784
--- /dev/null
+++ b/app/services/recurly_account_checker.rb
@@ -0,0 +1,18 @@
+class RecurlyAccountChecker
+
+  def self.customer_exists?(customer_id)
+    retrieve_customer(customer_id)
+  end
+
+  private
+  def self.retrieve_customer(customer_id, &block)
+    customer = Recurly::Account.find(customer_id) unless customer_id.nil?
+    yield if block_given?
+    true
+  rescue Recurly::Resource::NotFound => e
+    logger.error e.message
+    errors.add :base, "Unable to create your subscription. #{e.message}"
+    false
+  end
+
+end
diff --git a/spec/services/recurly_customer_checker_spec.rb b/spec/services/recurly_customer_checker_spec.rb
new file mode 100644
index 0000000..6cd554f
--- /dev/null
+++ b/spec/services/recurly_customer_checker_spec.rb
@@ -0,0 +1,13 @@
+require_relative '../../app/services/recurly_account_checker'
+require 'recurly'
+describe RecurlyAccountChecker, '.customer_exists?' do 
+  subject { described_class.customer_exists?(customer_id) }
+  let(:customer_id) { -1 }
+
+  context "when customer exist" do 
+    before { Recurly::Account.stub(:find).with(customer_id) { true } }
+
+    it { should be_true }
+  end
+
+end

From b3a3b7c84987c1a8b61fd024e7d7990cd0dc0549 Mon Sep 17 00:00:00 2001
From: Rafael George <george.rafael@gmail.com>
Date: Fri, 28 Jun 2013 12:36:29 -0400
Subject: [PATCH 2/5] Refactor User model

Fix spec for user model to use Factories instead of actual saves in the database
Add new a method to handle the update of subscriptions
---
 app/models/user.rb                            | 20 +++-----
 app/services/recurly_account_checker.rb       | 49 ++++++++++++++++---
 spec/models/user_spec.rb                      | 26 +++++++---
 .../services/recurly_customer_checker_spec.rb | 29 +++++++++--
 4 files changed, 91 insertions(+), 33 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index dd7a341..40d33f2 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -17,22 +17,11 @@ def name
   end
 
   def check_recurly
-    RecurlyAccountChecker.customer_exists?(customer_id)
+    recurly_account_checker.customer_exists?
   end
 
   def update_plan(role)
-    customer = Recurly::Account.find(customer_id) unless customer_id.nil?
-    self.role_ids = []
-    self.add_role(role.name)
-    unless customer.nil?
-      subscription = customer.subscriptions.first
-      subscription.update_attributes! :timeframe => 'now', :plan_code => role.name
-    end
-    true
-  rescue Recurly::Resource::Invalid => e
-    logger.error e.message
-    errors.add :base, "Unable to update your subscription. #{e.message}"
-    false
+    recurly_account_checker.update_subscriber(role)
   end
 
   def update_recurly
@@ -66,4 +55,9 @@ def expire
     destroy
   end
 
+  private
+  def recurly_account_checker
+    RecurlyAccountChecker.new(self)
+  end
+
 end
diff --git a/app/services/recurly_account_checker.rb b/app/services/recurly_account_checker.rb
index 995a784..2cbbd8f 100644
--- a/app/services/recurly_account_checker.rb
+++ b/app/services/recurly_account_checker.rb
@@ -1,17 +1,50 @@
 class RecurlyAccountChecker
+  attr_reader :user 
 
-  def self.customer_exists?(customer_id)
-    retrieve_customer(customer_id)
+  def initialize(user)
+    @user = user
+  end
+
+  def customer_exists?
+    !customer.nil?
+  end
+
+  def update_subscriber(role)
+    handle_recurly_exception do
+      user.role_ids = []
+      user.add_role role.name
+      if customer_exists?
+        subscription = customer.subscriptions.first
+        subscription.update_attributes! :timeframe => 'now', :plan_code => role.name
+      end
+      true
+    end
+  end
+
+  def customer
+    @customer ||= retrieve_customer
   end
 
   private
-  def self.retrieve_customer(customer_id, &block)
-    customer = Recurly::Account.find(customer_id) unless customer_id.nil?
-    yield if block_given?
-    true
+  def customer_id
+    user.customer_id 
+  end
+
+  def user_customer_id_exists?
+    !user.customer_id.nil?
+  end
+  
+  def retrieve_customer
+    handle_recurly_exception do 
+      Recurly::Account.find(customer_id) if user_customer_id_exists?
+    end
+  end
+
+  def handle_recurly_exception(&block)
+    yield 
   rescue Recurly::Resource::NotFound => e
-    logger.error e.message
-    errors.add :base, "Unable to create your subscription. #{e.message}"
+    user.logger.error e.message
+    user.errors.add :base, "Unable to create your subscription. #{e.message}"
     false
   end
 
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 5ff5164..e7b95ec 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe User do
+  let(:account_checker) { stub }
 
   before(:each) do
     @attr = {
@@ -10,10 +11,14 @@
       :password => "changeme",
       :password_confirmation => "changeme"
     }
+    account_checker.stub(:customer_exists?) { true }
   end
 
   it "should create a new instance given a valid attribute" do
-    User.create!(@attr)
+    u = User.new(@attr)
+    u.stub(:recurly_account_checker) {  account_checker }
+
+    u.should be_valid
   end
 
   it "should require an email address" do
@@ -38,14 +43,18 @@
   end
 
   it "should reject duplicate email addresses" do
-    User.create!(@attr)
+    u = User.new(@attr)
+    u.stub(:recurly_account_checker) { account_checker }
+    u.save
     user_with_duplicate_email = User.new(@attr)
     user_with_duplicate_email.should_not be_valid
   end
 
   it "should reject email addresses identical up to case" do
     upcased_email = @attr[:email].upcase
-    User.create!(@attr.merge(:email => upcased_email))
+    u = User.new(@attr.merge(:email => upcased_email))
+    u.stub(:recurly_account_checker) { account_checker }
+    u.save
     user_with_duplicate_email = User.new(@attr)
     user_with_duplicate_email.should_not be_valid
   end
@@ -88,7 +97,7 @@
   describe "password encryption" do
 
     before(:each) do
-      @user = User.create!(@attr)
+      @user = User.new(@attr)
     end
 
     it "should have an encrypted password attribute" do
@@ -104,7 +113,7 @@
   describe "expire" do
 
     before(:each) do
-      @user = User.create!(@attr)
+      @user = User.new(@attr)
     end
 
     it "sends an email to user" do
@@ -116,9 +125,9 @@
 
   describe "#update_plan" do
     before do
-      @user = FactoryGirl.create(:user, email: "test@example.com")
-      @role1 = FactoryGirl.create(:role, name: "silver")
-      @role2 = FactoryGirl.create(:role, name: "gold")
+      @user = FactoryGirl.build(:user, email: "test@example.com")
+      @role1 = FactoryGirl.build(:role, name: "silver")
+      @role2 = FactoryGirl.build(:role, name: "gold")
       @user.add_role(@role1.name)
     end
 
@@ -130,6 +139,7 @@
 
     it "wont remove original role from database" do
       @user.update_plan(@role2)
+      @user.stub(:recurly_account_checker) { account_checker }
       Role.all.count.should == 2
     end
   end
diff --git a/spec/services/recurly_customer_checker_spec.rb b/spec/services/recurly_customer_checker_spec.rb
index 6cd554f..621a36b 100644
--- a/spec/services/recurly_customer_checker_spec.rb
+++ b/spec/services/recurly_customer_checker_spec.rb
@@ -1,11 +1,32 @@
 require_relative '../../app/services/recurly_account_checker'
 require 'recurly'
-describe RecurlyAccountChecker, '.customer_exists?' do 
-  subject { described_class.customer_exists?(customer_id) }
+describe RecurlyAccountChecker do 
+  let(:user) { stub }
   let(:customer_id) { -1 }
+  let(:customer) { stub }
+  before do 
+    user.stub(:customer_id) { customer_id } 
+    Recurly::Account.stub(:find) { customer } 
+  end
+
+  context "#customer_exists?" do 
+
+    subject { described_class.new(user).customer_exists? }
+
+    it { should be_true }
+  end
 
-  context "when customer exist" do 
-    before { Recurly::Account.stub(:find).with(customer_id) { true } }
+  context "#update_subscriber" do 
+    let(:role) { stub }
+    let(:subscription) { stub }
+    subject { described_class.new(user).update_subscriber(role) }
+    before do 
+      user.stub(:role_ids=) 
+      user.stub(:add_role) 
+      role.stub(:name) { 'role' }
+      subscription.stub(:update_attributes!) { true }
+      customer.stub_chain(:subscriptions, :first) { subscription }
+    end
 
     it { should be_true }
   end

From 33276b1e1d35feda76022d4770e5b38e057ed3d8 Mon Sep 17 00:00:00 2001
From: Rafael George <george.rafael@gmail.com>
Date: Fri, 28 Jun 2013 12:52:40 -0400
Subject: [PATCH 3/5] Add RecurlyAccountChecker#update_customer

Remove duplication code from User model
---
 app/models/user.rb                            | 12 +----------
 app/services/recurly_account_checker.rb       | 11 ++++++++++
 .../services/recurly_customer_checker_spec.rb | 20 +++++++++++++++++--
 3 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 40d33f2..29dff6e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -25,17 +25,7 @@ def update_plan(role)
   end
 
   def update_recurly
-    customer = Recurly::Account.find(customer_id) unless customer_id.nil?
-    unless customer.nil?
-      customer.email = email
-      customer.first_name = first_name
-      customer.last_name = last_name
-      customer.save!
-    end
-  rescue Recurly::Resource::NotFound => e
-    logger.error e.message
-    errors.add :base, "Unable to update your subscription. #{e.message}"
-    false
+    recurly_account_checker.update_customer
   end
 
   def cancel_subscription
diff --git a/app/services/recurly_account_checker.rb b/app/services/recurly_account_checker.rb
index 2cbbd8f..2df634c 100644
--- a/app/services/recurly_account_checker.rb
+++ b/app/services/recurly_account_checker.rb
@@ -9,6 +9,17 @@ def customer_exists?
     !customer.nil?
   end
 
+  def update_customer 
+    handle_recurly_exception do 
+      if customer_exists?
+        customer.email = user.email
+        customer.first_name = user.first_name
+        customer.last_name = user.last_name
+        customer.save!
+      end
+    end
+  end
+
   def update_subscriber(role)
     handle_recurly_exception do
       user.role_ids = []
diff --git a/spec/services/recurly_customer_checker_spec.rb b/spec/services/recurly_customer_checker_spec.rb
index 621a36b..74967be 100644
--- a/spec/services/recurly_customer_checker_spec.rb
+++ b/spec/services/recurly_customer_checker_spec.rb
@@ -3,20 +3,20 @@
 describe RecurlyAccountChecker do 
   let(:user) { stub }
   let(:customer_id) { -1 }
-  let(:customer) { stub }
   before do 
     user.stub(:customer_id) { customer_id } 
     Recurly::Account.stub(:find) { customer } 
   end
 
   context "#customer_exists?" do 
-
+    let(:customer) { stub }
     subject { described_class.new(user).customer_exists? }
 
     it { should be_true }
   end
 
   context "#update_subscriber" do 
+    let(:customer) { stub }
     let(:role) { stub }
     let(:subscription) { stub }
     subject { described_class.new(user).update_subscriber(role) }
@@ -31,4 +31,20 @@
     it { should be_true }
   end
 
+  context "#update_customer" do 
+    let(:customer) { stub }
+    subject { described_class.new(user).update_customer }
+    before do 
+      user.stub(:email) { 'email@example.com' }
+      user.stub(:first_name) { 'first_name' }
+      user.stub(:last_name) {  'last_name' }
+      customer.stub(:email=).with(user.email)
+      customer.stub(:first_name=).with(user.first_name)
+      customer.stub(:last_name=).with(user.last_name)
+      customer.stub(:save!) { true }
+    end
+
+    it { should be_true }
+  end
+
 end

From 1cfe6ccc35c73d806e5497f8339058cb1af86833 Mon Sep 17 00:00:00 2001
From: Rafael George <george.rafael@gmail.com>
Date: Fri, 28 Jun 2013 12:59:13 -0400
Subject: [PATCH 4/5] Add RecurlyAccountChecker#cancel_subscription

---
 app/models/user.rb                             | 10 +---------
 app/services/recurly_account_checker.rb        |  9 +++++++++
 spec/services/recurly_customer_checker_spec.rb | 16 +++++++++++++---
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 29dff6e..a7eaa26 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -29,15 +29,7 @@ def update_recurly
   end
 
   def cancel_subscription
-    customer = Recurly::Account.find(customer_id) unless customer_id.nil?
-    subscription = customer.subscriptions.first unless customer.nil?
-    if (!subscription.nil?) && (subscription.state == 'active')
-      subscription.cancel
-    end
-  rescue Recurly::Resource::NotFound => e
-    logger.error e.message
-    errors.add :base, "Unable to cancel your subscription. #{e.message}"
-    false
+    recurly_account_checker.cancel_subscription
   end
 
   def expire
diff --git a/app/services/recurly_account_checker.rb b/app/services/recurly_account_checker.rb
index 2df634c..f4dd5fe 100644
--- a/app/services/recurly_account_checker.rb
+++ b/app/services/recurly_account_checker.rb
@@ -9,6 +9,15 @@ def customer_exists?
     !customer.nil?
   end
 
+  def cancel_subscription
+    handle_recurly_exception do 
+      if customer_exists?
+        subscription = customer.subscription.first
+        subscription.cancel if !subscription.nil? && subscription.state == 'active'
+      end
+    end
+  end
+
   def update_customer 
     handle_recurly_exception do 
       if customer_exists?
diff --git a/spec/services/recurly_customer_checker_spec.rb b/spec/services/recurly_customer_checker_spec.rb
index 74967be..d819de6 100644
--- a/spec/services/recurly_customer_checker_spec.rb
+++ b/spec/services/recurly_customer_checker_spec.rb
@@ -2,6 +2,7 @@
 require 'recurly'
 describe RecurlyAccountChecker do 
   let(:user) { stub }
+  let(:customer) { stub }
   let(:customer_id) { -1 }
   before do 
     user.stub(:customer_id) { customer_id } 
@@ -9,14 +10,12 @@
   end
 
   context "#customer_exists?" do 
-    let(:customer) { stub }
     subject { described_class.new(user).customer_exists? }
 
     it { should be_true }
   end
 
   context "#update_subscriber" do 
-    let(:customer) { stub }
     let(:role) { stub }
     let(:subscription) { stub }
     subject { described_class.new(user).update_subscriber(role) }
@@ -32,7 +31,6 @@
   end
 
   context "#update_customer" do 
-    let(:customer) { stub }
     subject { described_class.new(user).update_customer }
     before do 
       user.stub(:email) { 'email@example.com' }
@@ -47,4 +45,16 @@
     it { should be_true }
   end
 
+  context "#cancel_subscription" do 
+    subject { described_class.new(user).cancel_subscription }
+    let(:subscription) { stub }
+    before do 
+      customer.stub_chain(:subscription, :first) { subscription }
+      subscription.stub(:state) { 'active' }
+      subscription.stub(:cancel) { true }
+    end
+
+    it { should be_true }
+  end
+
 end

From f07e9f4179e9e539e5a1b20f7553d1493f31e0e1 Mon Sep 17 00:00:00 2001
From: Rafael George <george.rafael@gmail.com>
Date: Fri, 28 Jun 2013 13:03:06 -0400
Subject: [PATCH 5/5] Refactor: Add a RecurlyAccountChecker#subscription

Retrieve the subscription body
---
 app/services/recurly_account_checker.rb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/services/recurly_account_checker.rb b/app/services/recurly_account_checker.rb
index f4dd5fe..2a04f42 100644
--- a/app/services/recurly_account_checker.rb
+++ b/app/services/recurly_account_checker.rb
@@ -12,7 +12,6 @@ def customer_exists?
   def cancel_subscription
     handle_recurly_exception do 
       if customer_exists?
-        subscription = customer.subscription.first
         subscription.cancel if !subscription.nil? && subscription.state == 'active'
       end
     end
@@ -33,14 +32,11 @@ def update_subscriber(role)
     handle_recurly_exception do
       user.role_ids = []
       user.add_role role.name
-      if customer_exists?
-        subscription = customer.subscriptions.first
-        subscription.update_attributes! :timeframe => 'now', :plan_code => role.name
-      end
-      true
+      subscription.update_attributes! :timeframe => 'now', :plan_code => role.name if customer_exists?
     end
   end
 
+
   def customer
     @customer ||= retrieve_customer
   end
@@ -50,6 +46,10 @@ def customer_id
     user.customer_id 
   end
 
+  def subscription
+    @subscription ||= customer.subscriptions.first
+  end
+
   def user_customer_id_exists?
     !user.customer_id.nil?
   end