Some more tweaks to have billing code work, and allow admins to view but not edit for other users.

14 files changed, 45 insertions, 36 deletions

diff --git a/billing/app/controllers/billing_base_controller.rb b/billing/app/controllers/billing_base_controller.rb
index 67dff72..f6e233b 100644
--- a/billing/app/controllers/billing_base_controller.rb
+++ b/billing/app/controllers/billing_base_controller.rb
@@ -4,12 +4,14 @@ class BillingBaseController < ApplicationController
   helper 'billing'
 
   # required for navigation to work.
-  #TODO doesn't work for admins
   def assign_user
-    if params[:id]
+    if params[:user_id]
+      @user = User.find_by_param(params[:user_id])
+    elsif params[:action] == "confirm" # confirms will come back with different ID set, so check for this first 
+      # This is only for cases where an admin cannot apply action for customer, but should be all confirms
+      @user = current_user
+    elsif params[:id]
       @user = User.find_by_param(params[:id])
-    else
-      @user = current_user #TODO not always correct for admins viewing another user!
     end
   end
 
diff --git a/billing/app/controllers/credit_card_info_controller.rb b/billing/app/controllers/credit_card_info_controller.rb
index 75865fe..717fa18 100644
--- a/billing/app/controllers/credit_card_info_controller.rb
+++ b/billing/app/controllers/credit_card_info_controller.rb
@@ -3,7 +3,7 @@ class CreditCardInfoController < ApplicationController
 
   def edit
     @credit_card = Braintree::CreditCard.find(params[:id])
-    customer = Customer.find_by_user_id(current_user.id)
+    customer = Customer.find_by_user_id(@user.id)
     if customer and customer.braintree_customer_id == @credit_card.customer_id
       @tr_data = Braintree::TransparentRedirect.
         update_credit_card_data(:redirect_url => confirm_credit_card_info_url,
@@ -27,7 +27,8 @@ class CreditCardInfoController < ApplicationController
 
   private
 
-  def set_user
+    def set_user
+    # this assumes anybody, even an admin, will not access for another user.
     @user = current_user
   end
 
diff --git a/billing/app/controllers/customer_controller.rb b/billing/app/controllers/customer_controller.rb
index f38f77e..0120e91 100644
--- a/billing/app/controllers/customer_controller.rb
+++ b/billing/app/controllers/customer_controller.rb
@@ -1,18 +1,18 @@
 class CustomerController < BillingBaseController
-  before_filter :authorize
+  before_filter :authorize, :fetch_customer
 
   def show
-    if customer = fetch_customer
-      customer.with_braintree_data!
-      @default_cc = customer.default_credit_card #TODO not actually right way
-      @active_subscription = customer.subscriptions
-      @transactions = customer.braintree_customer.transactions
+    if @customer
+      @customer.with_braintree_data!
+      @default_cc = @customer.default_credit_card #TODO not actually right way
+      @active_subscription = @customer.subscriptions
+      @transactions = @customer.braintree_customer.transactions
     end
   end
 
   def new
-    if customer.has_payment_info?
-      redirect_to edit_customer_path(customer), :notice => 'Here is your saved customer data'
+    if @customer.has_payment_info?
+      redirect_to edit_customer_path(@user), :notice => 'Here is your saved customer data'
     else
       fetch_new_transparent_redirect_data
     end
@@ -24,12 +24,11 @@ class CustomerController < BillingBaseController
 
   def confirm
     @result = Braintree::TransparentRedirect.confirm(request.query_string)
-
     if @result.success?
-      customer.braintree_customer =  @result.customer
-      customer.save
+      @customer.braintree_customer =  @result.customer
+      @customer.save
       render :action => "confirm"
-    elsif customer.has_payment_info?
+    elsif @customer.has_payment_info?
       fetch_edit_transparent_redirect_data
       render :action => "edit"
     else
@@ -41,16 +40,18 @@ class CustomerController < BillingBaseController
   protected
 
   def fetch_new_transparent_redirect_data
+    access_denied unless @user == current_user # admins cannot do this for others
     @tr_data = Braintree::TransparentRedirect.
       create_customer_data(:redirect_url => confirm_customer_url)
   end
 
   def fetch_edit_transparent_redirect_data
-    customer.with_braintree_data!
-    @default_cc = customer.default_credit_card
+    access_denied unless @user == current_user # admins cannot do this for others
+    @customer.with_braintree_data!
+    @default_cc = @customer.default_credit_card
     @tr_data = Braintree::TransparentRedirect.
       update_customer_data(:redirect_url => confirm_customer_url,
-                           :customer_id => customer.braintree_customer_id) ##??
+                           :customer_id => @customer.braintree_customer_id) ##??
   end
 
   def fetch_customer
@@ -58,8 +59,6 @@ class CustomerController < BillingBaseController
     if @user == current_user
       @customer ||= Customer.new(user: @user)
     end
-    # TODO will want case for admins, presumably
     access_denied unless (@customer and (@customer.user == current_user)) or admin?
-    return @customer
   end
 end
diff --git a/billing/app/controllers/payments_controller.rb b/billing/app/controllers/payments_controller.rb
index 224b78e..3ffc5a3 100644
--- a/billing/app/controllers/payments_controller.rb
+++ b/billing/app/controllers/payments_controller.rb
@@ -16,9 +16,10 @@ class PaymentsController < BillingBaseController
   end
 
   def index
-    customer = Customer.find_by_user_id(current_user.id)
+    customer = Customer.find_by_user_id(@user.id)
     braintree_data = Braintree::Customer.find(customer.braintree_customer_id)
     # these will be ordered by created_at descending, per http://stackoverflow.com/questions/16425475/
+    # TODO permissions
     @transactions = braintree_data.transactions
   end
 
diff --git a/billing/app/controllers/subscriptions_controller.rb b/billing/app/controllers/subscriptions_controller.rb
index 38dbff1..8030c88 100644
--- a/billing/app/controllers/subscriptions_controller.rb
+++ b/billing/app/controllers/subscriptions_controller.rb
@@ -21,7 +21,7 @@ class SubscriptionsController < BillingBaseController
   end
 
   def index
-    customer = Customer.find_by_user_id(current_user.id)
+    customer = Customer.find_by_user_id(@user.id)
     @subscriptions = customer.subscriptions(nil, false)
   end
 
@@ -31,7 +31,7 @@ class SubscriptionsController < BillingBaseController
     @subscription = Braintree::Subscription.find params[:id]
     @subscription_customer_id = @subscription.transactions.first.customer_details.id #all of subscriptions transactions should have same customer
     @customer = Customer.find_by_user_id(current_user.id)
-    access_denied unless @customer and @customer.braintree_customer_id == @subscription_customer_id
+    access_denied unless admin? or (@customer and @customer.braintree_customer_id == @subscription_customer_id)
     # TODO: will presumably want to allow admins to view/cancel subscriptions for all users
   end
 
diff --git a/billing/app/views/credit_card_info/edit.html.haml b/billing/app/views/credit_card_info/edit.html.haml
index 39269ca..bd86a4c 100644
--- a/billing/app/views/credit_card_info/edit.html.haml
+++ b/billing/app/views/credit_card_info/edit.html.haml
@@ -14,4 +14,4 @@
       %dd= f.text_field :cvv
   = hidden_field_tag :tr_data, @tr_data
   = f.submit 'Save Payment Info', :class => :btn
-  = link_to t(:cancel), edit_customer_path(@credit_card.customer_id), :class => :btn
+  = link_to t(:cancel), edit_customer_path(@user.id), :class => :btn
diff --git a/billing/app/views/customer/confirm.html.haml b/billing/app/views/customer/confirm.html.haml
index 5551622..49a1e91 100644
--- a/billing/app/views/customer/confirm.html.haml
+++ b/billing/app/views/customer/confirm.html.haml
@@ -11,4 +11,4 @@
   - @result.customer.credit_cards.each do |cc|
     %dd= cc.masked_number
 - customer = Customer.find_by_user_id(current_user.id)
-= link_to 'View Customer Info', show_customer_path(customer.braintree_customer_id), :class=> :btn
\ No newline at end of file
+= link_to 'View Customer Info', show_customer_path(@user.id), :class=> :btn
\ No newline at end of file
diff --git a/billing/app/views/customer/show.html.haml b/billing/app/views/customer/show.html.haml
index 38b1cb2..243bd3b 100644
--- a/billing/app/views/customer/show.html.haml
+++ b/billing/app/views/customer/show.html.haml
@@ -12,7 +12,7 @@
     - break if counter > 2 # not ruby-like, but object is a Braintree::ResourceCollection so limited methods available
     = render :partial => "payments/transaction_details", :locals => {:transaction => t}
     - counter += 1
-  = link_to t(:transaction_history), payments_path
+  = link_to t(:transaction_history), user_payments_path(@user)
   %legend= t(:subscriptions)
   - if @active_subscription
     = render :partial => "subscriptions/subscription_details", :locals => {:subscription => @active_subscription}
@@ -24,4 +24,4 @@
         .form-actions
           = link_to t(:subscribe_to_plan), new_subscription_path, :class => :btn
   %p
-    = link_to t(:all_subscriptions), subscriptions_path
+    = link_to t(:all_subscriptions), user_subscriptions_path(@user)
diff --git a/billing/app/views/payments/_customer_data.html.haml b/billing/app/views/payments/_customer_data.html.haml
index 87b8209..e9df040 100644
--- a/billing/app/views/payments/_customer_data.html.haml
+++ b/billing/app/views/payments/_customer_data.html.haml
@@ -12,4 +12,5 @@
   %dd= @default_cc.masked_number
   %dt Expiration Date
   %dd= @default_cc.expiration_date
-  = link_to t(:edit_saved_data), edit_customer_path(@customer), :class => :btn
+  - if current_user == @user
+    = link_to t(:edit_saved_data), edit_customer_path(@user.id), :class => :btn
diff --git a/billing/app/views/payments/index.html.haml b/billing/app/views/payments/index.html.haml
index f994fe5..7a89917 100644
--- a/billing/app/views/payments/index.html.haml
+++ b/billing/app/views/payments/index.html.haml
@@ -1,3 +1,4 @@
+%h2=t :transaction_history
 - if (@transactions.count == 0)
   = t(:no_transaction_history)
 - @transactions.each do |t|
diff --git a/billing/app/views/subscriptions/_subscription_details.html.haml b/billing/app/views/subscriptions/_subscription_details.html.haml
index fb18210..6eda7ca 100644
--- a/billing/app/views/subscriptions/_subscription_details.html.haml
+++ b/billing/app/views/subscriptions/_subscription_details.html.haml
@@ -1,5 +1,5 @@
 %p
-  = link_to subscription.id, subscription_path(subscription.id)
+  = link_to subscription.id, user_subscription_path(@user, subscription.id)
   Balance:
   = number_to_currency(subscription.balance)
   Bill on:
diff --git a/billing/app/views/subscriptions/index.html.haml b/billing/app/views/subscriptions/index.html.haml
index 0e84619..87771e5 100644
--- a/billing/app/views/subscriptions/index.html.haml
+++ b/billing/app/views/subscriptions/index.html.haml
@@ -1,7 +1,8 @@
+%h2=t :all_subscriptions
 - active = false
 - @subscriptions.each do |s|
   - if s.status == 'Active'
     - active = true
   = render :partial => "subscription_details", :locals => {:subscription => s}
-- if !active
+- if !active and @user == current_user
   = link_to 'subscribe to plan', new_subscription_path, :class => :btn
\ No newline at end of file
diff --git a/billing/config/routes.rb b/billing/config/routes.rb
index 1bb32df..8b7b5bf 100644
--- a/billing/config/routes.rb
+++ b/billing/config/routes.rb
@@ -2,7 +2,10 @@ Rails.application.routes.draw do
 
   match 'payments/new' => 'payments#new', :as => :new_payment
   match 'payments/confirm' => 'payments#confirm', :as => :confirm_payment
-  resources :payments, :only => [:index]
+  resources :users do
+    resources :payments, :only => [:index]
+    resources :subscriptions, :only => [:index, :show]
+  end
 
   resources :customer, :only => [:new, :edit]
   resources :credit_card_info, :only => [:edit]
@@ -11,7 +14,7 @@ Rails.application.routes.draw do
   match 'customer/show/:id' => 'customer#show', :as => :show_customer
   match 'credit_card_info/confirm' => 'credit_card_info#confirm', :as => :confirm_credit_card_info
 
-  resources :subscriptions, :only => [:new, :create, :index, :show, :update, :destroy]
+  resources :subscriptions, :only => [:new, :create, :update, :destroy] # index and show are within users path
 
   #match 'transactions/:product_id/new' => 'transactions#new', :as => :new_transaction
   #match 'transactions/confirm/:product_id' => 'transactions#confirm', :as => :confirm_transaction
diff --git a/billing/test/functional/customers_controller_test.rb b/billing/test/functional/customers_controller_test.rb
index 58b6155..2a431da 100644
--- a/billing/test/functional/customers_controller_test.rb
+++ b/billing/test/functional/customers_controller_test.rb
@@ -45,7 +45,7 @@ class CustomersControllerTest < ActionController::TestCase
     login @other_user
     get :new
     assert_response :redirect
-    assert_equal edit_customer_url(@customer), response.header['Location']
+    assert_equal edit_customer_url(@customer), response.header['Location'] #todo should pass user not customer
   end