billing: update customer records - do not create new ones

4 files changed, 48 insertions, 32 deletions

diff --git a/billing/app/controllers/customer_controller.rb b/billing/app/controllers/customer_controller.rb
index 1fa19c9..887dbfd 100644
--- a/billing/app/controllers/customer_controller.rb
+++ b/billing/app/controllers/customer_controller.rb
@@ -1,14 +1,12 @@
 class CustomerController < BillingBaseController
   before_filter :authorize
-  before_filter :fetch_customer_data, :only => [:show, :edit] #confirm???
-
   def show
-    @active_subscription = @customer.subscriptions(@braintree_data)
+    @active_subscription = customer.subscriptions
   end
 
   def new
-    if customer = Customer.find_by_user_id(current_user.id)
-      redirect_to edit_customer_path(customer.braintree_customer_id), :notice => 'Here is your saved customer data'
+    if customer.has_payment_info?
+      redirect_to edit_customer_path(customer), :notice => 'Here is your saved customer data'
     else
       fetch_new_transparent_redirect_data
     end
@@ -20,17 +18,12 @@ class CustomerController < BillingBaseController
 
   def confirm
     @result = Braintree::TransparentRedirect.confirm(request.query_string)
+
     if @result.success?
-      # customer = Customer.new(:user_id => current_user.id, :braintree_customer_id =>  @result.customer.id)
-      customer = Customer.new(:braintree_customer_id =>  @result.customer.id)
-      customer.user = current_user
+      customer.braintree_customer =  @result.customer
       customer.save
-      #current_user.save!
-      #debugger
       render :action => "confirm"
-    #elsif current_user.has_payment_info?
-    elsif (customer = Customer.find_by_user_id(current_user.id)) and customer.has_payment_info?
-      #customer.with_braintree_data!
+    elsif customer.has_payment_info?
       fetch_edit_transparent_redirect_data
       render :action => "edit"
     else
@@ -39,18 +32,7 @@ class CustomerController < BillingBaseController
     end
   end
 
-  private
-
-  def fetch_customer_data
-    if ((@customer = Customer.find_by_user_id(current_user.id)) and
-        (params[:id] == @customer.braintree_customer_id))
-      @braintree_data = Braintree::Customer.find(params[:id]) #used in editing form
-      @default_cc = @customer.default_credit_card(@braintree_data)
-    else
-      # TODO will want case for admins, presumably
-      access_denied
-    end
-  end
+  protected
 
   def fetch_new_transparent_redirect_data
     @tr_data = Braintree::TransparentRedirect.
@@ -58,10 +40,19 @@ class CustomerController < BillingBaseController
   end
 
   def fetch_edit_transparent_redirect_data
+    customer.with_braintree_data!
+    @default_cc = customer.default_credit_card
     @tr_data = Braintree::TransparentRedirect.
       update_customer_data(:redirect_url => confirm_customer_url,
-                           :customer_id => params[:id]) ##??
-
+                           :customer_id => customer.braintree_customer_id) ##??
   end
 
+  def customer
+    @customer ||= Customer.find(params[:id]) if params[:id]  # edit, show
+    @customer ||= Customer.find_by_user_id(current_user.id)  # confirm
+    @customer ||= Customer.new(user: current_user)
+    # TODO will want case for admins, presumably
+    access_denied unless @customer.user == current_user
+    return @customer
+  end
 end
diff --git a/billing/app/views/customer/edit.html.haml b/billing/app/views/customer/edit.html.haml
index f9c1383..dfc932e 100644
--- a/billing/app/views/customer/edit.html.haml
+++ b/billing/app/views/customer/edit.html.haml
@@ -2,7 +2,7 @@
   #total-errors{:style => "color:red;"}
     = h(@result.errors.size)
     error(s)
-= form_for :customer, :url => Braintree::TransparentRedirect.url, :params => @result && @result.params[:customer],:existing => @braintree_data, :builder => BraintreeFormHelper::BraintreeFormBuilder, :errors => @result && @result.errors.for(:customer) do |f| |
+= form_for :customer, :url => Braintree::TransparentRedirect.url, :params => @result && @result.params[:customer],:existing => @customer,  :builder => BraintreeHelper::BraintreeFormBuilder, :errors => @result && @result.errors.for(:customer) do |f| |
   = field_set_tag "Customer" do
     %dl
       %dt= f.label :first_name, 'First Name'
diff --git a/billing/test/functional/customer_controller_test.rb b/billing/test/functional/customer_controller_test.rb
index 86d40fd..af09a41 100644
--- a/billing/test/functional/customer_controller_test.rb
+++ b/billing/test/functional/customer_controller_test.rb
@@ -20,10 +20,10 @@ class CustomerControllerTest < ActionController::TestCase
     assert_redirected_to login_path
   end
 
-  test "edit uses current_user" do
+  test "edit uses params[:id]" do
     customer = FactoryGirl.create :customer_with_payment_info
     login customer.user
-    get :edit, id: :unused
+    get :edit, id: customer.id
 
     assert_response :success
     assert assigns(:tr_data)
@@ -37,7 +37,10 @@ class CustomerControllerTest < ActionController::TestCase
     to_confirm = prepare_confirmation :create_customer_data,
       customer: FactoryGirl.attributes_for(:braintree_customer),
       redirect_url: confirm_customer_url
-    post :confirm, to_confirm
+
+    assert_difference("Customer.count") do
+      post :confirm, to_confirm
+    end
 
     assert_response :success
     assert result = assigns(:result)
@@ -45,6 +48,21 @@ class CustomerControllerTest < ActionController::TestCase
     assert result.customer.id
   end
 
+  test "customer update" do
+    @customer = FactoryGirl.create :customer_with_payment_info
+    login @customer.user
+    Braintree::TransparentRedirect.expects(:confirm).returns(success_response)
+
+    assert_no_difference("Customer.count") do
+      post :confirm, query: :from_braintree
+    end
+
+    assert_response :success
+    assert result = assigns(:result)
+    assert result.success?
+    assert_equal @customer, result.customer
+  end
+
   test "failed user creation" do
     skip "can't get user creation to fail"
     login
@@ -89,6 +107,11 @@ class CustomerControllerTest < ActionController::TestCase
       params: {}
   end
 
+  def success_response
+    stub success?: true,
+      customer: @customer.with_braintree_data!
+  end
+
   def post_transparent_redirect(type, data)
     params = data.dup
     params[:tr_data] = Braintree::TransparentRedirect.send(type, params)
diff --git a/billing/test/functional/customers_controller_test.rb b/billing/test/functional/customers_controller_test.rb
index 45a14ed..58b6155 100644
--- a/billing/test/functional/customers_controller_test.rb
+++ b/billing/test/functional/customers_controller_test.rb
@@ -1,6 +1,8 @@
 require 'test_helper'
+require 'fake_braintree'
 
 class CustomersControllerTest < ActionController::TestCase
+  tests CustomerController
 
   setup do
     @user = FactoryGirl.create :user
@@ -43,7 +45,7 @@ class CustomersControllerTest < ActionController::TestCase
     login @other_user
     get :new
     assert_response :redirect
-    assert_equal edit_customer_url(@customer.braintree_customer_id), response.header['Location']
+    assert_equal edit_customer_url(@customer), response.header['Location']
   end