Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Refactored the IPN notification logic and associated tests.

  • Loading branch information...
commit 532493740d47a9641011534e541d3ee6eca9e11d 1 parent 9d5d570
@schof schof authored
View
5 CHANGELOG.markdown
@@ -0,0 +1,5 @@
+1.1
+---
+
+* `checkout_controller` is obsolete now that Spree has been refactored to use REST.
+* `after_notify` and `after_success` hooks are gone. See the `README` for how to implement them as hooks in the "fat" order model.
View
18 README.markdown
@@ -6,23 +6,23 @@ You'll want to test this using a paypal sandbox account first. Once you have a
Regarding Taxes and shipping, we assumed you'd want to use Paypal's system for this, which can also be configured through the "profile" page. Taxes have been tested (sales tax), but not shipping, so you may want to give that a test run on the sandbox.
-There are also `after_notify` and `after_success` hooks which allow you to implment your own custom logic after the standard processing is performed. These hooks should be added to `checkout_controller` in the extension you are using for your site specific customizations.
-
-For example:
+You may want to implement your own custom logic by adding `state_machine` hooks. Just add these hooks in your site extension (don't change the `pp_website_standard` extension.) Here's an example of how to add the hooks to your site extension.
<pre>
-CheckoutController.class_eval do
- def after_notify(payment)
+fsm = Order.state_machines['state']
+fsm.after_transition :to => 'paid', :do => :after_payment
+fsm.after_transition :to => 'pending_payment', :do => :after_pending
+
+Order.class_eval do
+ def after_payment
# email user and tell them we received their payment
end
- def after_success(payment)
+ def after_pending
# email user and thell them that we are processing their order, etc.
end
end
-</pre>
-
-
+</pre>
* TODO: User account creation (if necessary) after notify and associate order with a user
* TODO: Make the paypal account stuff configurable via new preferences system
View
56 app/controllers/paypal_payments_controller.rb
@@ -1,14 +1,58 @@
class PaypalPaymentsController < Spree::BaseController
+ include ActiveMerchant::Billing::Integrations
+
# before_filter :verify_authenticity_token, :except => 'create'
-
- resource_controller
+ layout 'application'
+
+ resource_controller :singleton
belongs_to :order
#protect_from_forgery :except => [:create, :notify]
- create.before do
-debugger
- puts ">>>>>>>> hello"
-
+ # NOTE: The Paypal Instant Payment Notification (IPN) results in the creation of a PaypalPayment
+ create.after do
+ ipn = Paypal::Notification.new(request.raw_post)
+
+ # create a transaction which records the details of the notification
+ object.txns.create(:transaction_id => ipn.transaction_id,
+ :amount => ipn.gross,
+ :fee => ipn.fee,
+ :currency_type => ipn.currency,
+ :status => ipn.status,
+ :received_at => ipn.received_at)
+ if ipn.acknowledge
+ case ipn.status
+ when "Completed"
+ if ipn.gross.to_d == @order.total
+ @order.pay!
+ @order.update_attribute("tax_amount", params[:tax].to_d) if params[:tax]
+ @order.update_attribute("ship_amount", params[:mc_shipping].to_d) if params[:mc_shipping]
+ else
+ @order.fail_payment!
+ logger.error("Incorrect order total during Paypal's notification, please investigate (Paypal processed #{ipn.gross}, and order total is #{@order.total})")
+ end
+ when "Pending"
+ @order.fail_payment!
+ logger.info("Received an unexpected pending status for order: #{@order.number}")
+ else
+ @order.fail_payment!
+ logger.info("Received an unexpected status for order: #{@order.number}")
+ end
+ else
+ @order.fail_payment!
+ logger.info("Failed to acknowledge Paypal's notification, please investigate [order: #{@order.number}]")
+ end
+=begin
+ @order.save
+
+ # call notify hook (which will email users, etc.)
+ after_notify(@payment) if @order.status == Order::Status::PAID
+=end
+ end
+
+ create.response do |wants|
+ wants.html do
+ render :nothing => true
+ end
end
end
View
2  app/views/orders/_paypal_checkout.html.erb
@@ -28,7 +28,7 @@
Zipcode (if you have one): <input id="zip" name="zip" type="text" value="" />
</p>
-<input id="notify_url" name="notify_url" type="hidden" value="<%= Spree::Paypal::Config[:ipn_notify_url] %>" />
+<input id="notify_url" name="notify_url" type="hidden" value="<%= Spree::Paypal::Config[:ipn_notify_host] + order_paypal_payment_path(@order) %>" />
<input type="hidden" name="rm" value ="2"> <!-- tells paypal that the return should be POST instead of GET -->
<input id="return" name="return" type="hidden" value="<%= order_paypal_payment_url(@order) %>" />
View
125 lib/paypal/orders_controller.rb
@@ -1,67 +1,72 @@
module Paypal
-module OrdersController
-
- # You can send in test notifications on the developer page here:
- # https://developer.paypal.com/us/cgi-bin/devscr?cmd=_ipn-link-session
- def notify
- ipn = Paypal::Notification.new(request.raw_post)
- @order = find_order(ipn.invoice)
+ module OrdersController
+
+ include ActiveMerchant::Billing::Integrations
+
+ # You can send in test notifications on the developer page here:
+ # https://developer.paypal.com/us/cgi-bin/devscr?cmd=_ipn-link-session
+ def notify
+ ipn = Paypal::Notification.new(request.raw_post)
+ # the IPN invoice is equivalent to the order number
+ @order = Order.find_by_number(ipn.invoice)
- # create a transaction which records the details of the notification
- @payment.txns.build :transaction_id => ipn.transaction_id, :amount => ipn.gross, :fee => ipn.fee,
- :currency_type => ipn.currency, :status => ipn.status, :received_at => ipn.received_at
- @payment.save
+ # create a payment for the order
+
+ # create a transaction which records the details of the notification
+ @payment.txns.build :transaction_id => ipn.transaction_id, :amount => ipn.gross, :fee => ipn.fee,
+ :currency_type => ipn.currency, :status => ipn.status, :received_at => ipn.received_at
+ @payment.save
- if ipn.acknowledge
- case ipn.status
- when "Completed"
- if ipn.gross.to_d == @order.total
- @order.status = Order::Status::PAID
+ if ipn.acknowledge
+ case ipn.status
+ when "Completed"
+ if ipn.gross.to_d == @order.total
+ @order.status = Order::Status::PAID
+ else
+ @order.status = Order::Status::INCOMPLETE
+ logger.error("Incorrect order total during Paypal's notification, please investigate (Paypal processed #{ipn.gross}, and order total is #{@order.total})")
+ end
+ when "Pending"
+ @order.status = Order::Status::PENDING_PAYMENT
else
@order.status = Order::Status::INCOMPLETE
- logger.error("Incorrect order total during Paypal's notification, please investigate (Paypal processed #{ipn.gross}, and order total is #{@order.total})")
+ logger.error("Failed to verify Paypal's notification, please investigate")
end
- when "Pending"
- @order.status = Order::Status::PENDING_PAYMENT
else
+ logger.error("Failed to acknowledge Paypal's notification, please investigate.")
@order.status = Order::Status::INCOMPLETE
- logger.error("Failed to verify Paypal's notification, please investigate")
end
- else
- logger.error("Failed to acknowledge Paypal's notification, please investigate.")
- @order.status = Order::Status::INCOMPLETE
- end
- @order.save
+ @order.save
- # call notify hook (which will email users, etc.)
- after_notify(@payment) if @order.status == Order::Status::PAID
+ # call notify hook (which will email users, etc.)
+ after_notify(@payment) if @order.status == Order::Status::PAID
- render :nothing => true
- end
+ render :nothing => true
+ end
- # When they've returned from paypal
- # Not really "success" as in they've paid. "Success" as in the transaction is in progress
- # Notify is called when the transaction is successfull
- def successful
+ # When they've returned from paypal
+ # Not really "success" as in they've paid. "Success" as in the transaction is in progress
+ # Notify is called when the transaction is successfull
+ def successful
-puts ">>>>>>>>>>>>>>>>> order: #{@order}"
+ puts ">>>>>>>>>>>>>>>>> order: #{@order}"
=begin
ref_hash = params[:invoice]
@order = find_order(ref_hash)
-
+
store_user_in_order(@order)
-
+
# create a transaction for the order (record what little information we have from paypal)
@payment.txns.build :amount => params[:mc_gross], :status => "order-processed"
@payment.save
=end
- # call success hook (which will email users, etc.)
- after_success(@payment)
+ # call success hook (which will email users, etc.)
+ after_success(@payment)
- # Render thank you (unless redirected by hook of course)
+ # Render thank you (unless redirected by hook of course)
=begin
if logged_in?
store_user_in_order(@order)
@@ -72,30 +77,30 @@ def successful
redirect_to signup_path
end
=end
- end
+ end
- def after_notify(payment)
- # override this method in your own custom extension if you wish (see README for details)
- end
+ def after_notify(payment)
+ # override this method in your own custom extension if you wish (see README for details)
+ end
- def after_success(payment)
- # override this method in your own custom extension if you wish (see README for details)
- end
+ def after_success(payment)
+ # override this method in your own custom extension if you wish (see README for details)
+ end
- def thank_you
- if logged_in? # If the user is logged in then show the thank you
- @order = Order.find_by_number(params[:id])
- store_user_in_order(@order)
- else # redirect them to make an account. For some reason they may have not hit the success action,
- # in which case, they still need to create an account
- flash[:notice] = "Please create an account or login so you can view this invoice"
- session[:return_to] = url_for(:action => :thank_you, :id => @order.number)
- redirect_to signup_path
+ def thank_you
+ if logged_in? # If the user is logged in then show the thank you
+ @order = Order.find_by_number(params[:id])
+ store_user_in_order(@order)
+ else # redirect them to make an account. For some reason they may have not hit the success action,
+ # in which case, they still need to create an account
+ flash[:notice] = "Please create an account or login so you can view this invoice"
+ session[:return_to] = url_for(:action => :thank_you, :id => @order.number)
+ redirect_to signup_path
+ end
end
- end
=begin
private
-
+
def find_order(ref_hash)
# Check to see if there is a cart record matching the invoice hash
if cart = Cart.find_by_reference_hash(ref_hash)
@@ -121,10 +126,10 @@ def find_order(ref_hash)
@payment = PaypalPayment.find_by_reference_hash ref_hash
@order = @payment.order
end
-
+
@order
end
-
+
def store_user_in_order(order)
# if this user is logged in, but order doesn't have a user yet, associate it
if !order.user_id && logged_in?
@@ -133,5 +138,5 @@ def store_user_in_order(order)
end
end
=end
-end
+ end
end
View
2  lib/paypal_configuration.rb
@@ -6,7 +6,7 @@ class PaypalConfiguration < Configuration
# these are just default preferences of course, you'll need to change them to something meaningful
preference :account, :string, :default => "kevin@schoftech.net"
- preference :ipn_notify_url, :string, :default => "http://11.22.33.44:3000/notify"
+ preference :ipn_notify_host, :string, :default => "http://96.255.82.213:3000"
preference :success_url, :string, :default => "http://localhost:3000/checkout/success"
validates_presence_of :name
View
19 pp_website_standard_extension.rb
@@ -9,7 +9,7 @@
end
class PpWebsiteStandardExtension < Spree::Extension
- version "1.0"
+ version "1.1"
description "Describe your extension here"
url "http://yourwebsite.com/spree_pp_website_standard"
@@ -27,13 +27,20 @@ def add_pp_standard_txns
end
end
- OrdersController.class_eval do
-skip_before_filter :verify_authenticity_token
+ # add new events and states to the FSM
+ fsm = Order.state_machines['state']
+ fsm.events["fail_payment"] = PluginAWeek::StateMachine::Event.new(fsm, "fail_payment")
+ fsm.events["pend_payment"] = PluginAWeek::StateMachine::Event.new(fsm, "pend_payment")
+ fsm.events["fail_payment"].transition(:to => 'payment_failure')
+ fsm.events["pend_payment"].transition(:to => 'payment_pending')
+
+# OrdersController.class_eval do
+#skip_before_filter :verify_authenticity_token
# before_filter :verify_authenticity_token, :except => 'notify'
# before_filter :load_object, :only => [:successful, :notify]
- include ActiveMerchant::Billing::Integrations
- include Paypal::OrdersController
- end
+# include ActiveMerchant::Billing::Integrations
+# include Paypal::PaController
+# end
# add a PaypalPayment association to the Order model
Order.class_eval do
View
245 spec/controllers/orders_controller_spec.rb
@@ -1,248 +1,9 @@
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
describe OrdersController do
+# verify that tax is being added to the order
+# verify that shipping is being added to the order
- before(:each) do
- #@ipn = mock("IPN Notification", :invoice => @mock_hash, :gross => 50, :null_object => true)
- #@ipn.stub!(:acknowledge).and_return(true)
- #ActiveMerchant::Billing::Integrations::Paypal::Notification.stub!(:new).with(any_args).and_return @ipn
-
- @order = mock_model(Order, :number => "123", :id => 999, :total => 50)
- Order.stub!(:find).with(999).and_return(@order)
- end
-
- describe "/notify" do
-
- before(:each) do
-# @mock_hash = "M0CKH4SH"
- @ipn = mock("IPN Notification", :invoice => @mock_hash, :gross => 50, :null_object => true)
- @ipn.stub!(:acknowledge).and_return(true)
- ActiveMerchant::Billing::Integrations::Paypal::Notification.stub!(:new).with(any_args).and_return @ipn
-
- @order = mock_model(Order, :number => "123", :total => 50)
-
- mock_txns = mock("txns")
- mock_txns.stub!(:build).with(any_args).and_return(mock_model(PaypalTxn))
-
- @payment = mock_model(PaypalPayment, :order => @order, :txns => mock_txns)#, :null_object => true)
- PaypalPayment.stub!(:create).with(any_args).and_return(@payment)
- PaypalPayment.stub!(:find_by_reference_hash).with(@mock_hash).and_return(@payment)
- end
-=begin
- describe "notifications in general", :shared => true do
-
- it "should acknowledge the notification" do
- @ipn.should_receive(:acknowledge)
- post :notify
- end
-
- describe "when acknowledgement succeeds" do
-
- before(:each) do
- @ipn.should_receive(:acknowledge).and_return(true)
- end
-
- describe "when status is completed" do
-
- before(:each) do
- @ipn.should_receive(:status).at_least(:once).and_return("Completed")
- end
-
- describe "when the order total is verfied" do
- before(:each) do
- @order.should_receive(:total).and_return(@ipn.gross)
- end
- it "should change the order status to paid if total is verifed" do
- @order.should_receive(:status=).with(Order::Status::PAID)
- post :notify
- end
- it "should call the notify hook" do
- @order.stub!(:status).and_return(Order::Status::PAID)
- @controller.should_receive(:after_notify).with(@payment)
- post :notify
- end
- end
-
- describe "when the order total is not verified" do
- before(:each) do
- @order.should_receive(:total).twice.and_return(1)
- end
- it "should change the order status to incomplete" do
- @order.should_receive(:status=).with(Order::Status::INCOMPLETE)
- post :notify
- end
- it "should not call the notify hook" do
- @order.stub!(:status).and_return(Order::Status::INCOMPLETE)
- @controller.should_not_receive(:after_notify).with(@payment)
- post :notify
- end
- end
- end
-
- describe "when status is pending" do
-
- before(:each) do
- @ipn.should_receive(:status).at_least(:once).and_return("Pending")
- end
-
- it "should change the order status to pending payment" do
- @order.should_receive(:status=).at_least(:once).with(Order::Status::PENDING_PAYMENT)
- post :notify
- end
- it "should not call the notify hook" do
- @order.stub!(:status).and_return(Order::Status::PENDING_PAYMENT)
- @controller.should_not_receive(:after_notify).with(@payment)
- post :notify
- end
- end
- end
-
- describe "when acknowledgement fails" do
-
- before(:each) do
- @ipn.should_receive(:acknowledge).and_return(true)
- end
-
- it "should change the order status to incomplete" do
- @order.should_receive(:status=).with(Order::Status::INCOMPLETE)
- post :notify
- end
- it "should not call the notify hook" do
- @order.stub!(:status).and_return(Order::Status::INCOMPLETE)
- @controller.should_not_receive(:after_notify).with(@payment)
- post :notify
- end
- end
- end
-=end
- describe "before success" do
-
-# before :each do
-# @cart = mock_model(Cart, :null_object => true)
-# Cart.stub!(:find_by_reference_hash).with(any_args).and_return(@cart)
-# Order.stub!(:new_from_cart).with(any_args).and_return(@order)
-# end
-
- it "should locate the order using the reference hash" do
- Order.should_receive(:find_by_number).with("123").and_return(@order)
- post :notify
- end
-
-# it "should create an order from the cart" do
-# Order.should_receive(:new_from_cart).with(@cart).and_return(@order)
-# post :notify
-# end
-
- it "should create a payment for the order" do
-# expected_payment_params = {:reference_hash => @mock_hash}
- PaypalPayment.should_receive(:create).with(:order => @order).and_return(@payment)
- post :notify
- end
-
-# it "should associate the payment with the order" do
-# @order.should_receive(:paypal_payment=).with(@payment)
-# post :notify
-# end
-
-# it "should destroy the cart" do
-# @cart.should_receive(:destroy)
-# post :notify
-# end
-
-# it_should_behave_like "notifications in general"
- end
-
- describe "after success" do
-
- before :each do
-# Cart.should_receive(:find_by_reference_hash).with(@mock_hash).and_return(nil)
- @payment.should_receive(:order).and_return(@order)
- end
-# not sure if this still applies
- it "should find the payment using the order id" do
- PaypalPayment.should_receive(:find_by_order_id).with(999).and_return(@payment)
- post :notify
- end
-
-# it_should_behave_like "notifications in general"
- end
-
-
- end
-
- describe "/success" do
-
- before(:each) do
- #@mock_hash = "M0CKH4SH"
- #@ipn = mock("IPN Notification", :invoice => @mock_hash, :gross => 50, :null_object => true)
- #@ipn.stub!(:acknowledge).and_return(true)
- #ActiveMerchant::Billing::Integrations::Paypal::Notification.stub!(:new).with(any_args).and_return @ipn
-
- #@order = mock_model(Order, :null_object => true, :total => 50)
-
- mock_txns = mock("txns")
- mock_txns.stub!(:build).with(any_args).and_return(mock_model(PaypalTxn))
-
- @payment = mock_model(PaypalPayment, :txns => mock_txns)#, :null_object => true)
- PaypalPayment.stub!(:create).with(any_args).and_return(@payment)
- PaypalPayment.stub!(:find_by_reference_hash).with(@mock_hash).and_return(@payment)
- end
-
- describe "successful in general", :shared => true do
- it "should call the success hook" do
- @controller.should_receive(:after_success).with(@payment)
- get 'successful/123', {:mc_gross => 50, :invoice => "123"}
- end
- end
-
- describe "before notify" do
-=begin
- before :each do
- @cart = mock_model(Cart, :null_object => true)
- Cart.stub!(:find_by_reference_hash).with(any_args).and_return(@cart)
- Order.stub!(:new_from_cart).with(any_args).and_return(@order)
- end
-
- it "should locate the cart using the reference hash" do
- Cart.should_receive(:find_by_reference_hash).with(@mock_hash).and_return(@cart)
- get :success, {:mc_gross => 50, :invoice => @mock_hash}
- end
-
- it "should create an order from the cart" do
- Order.should_receive(:new_from_cart).with(@cart).and_return(@order)
- get :success, {:mc_gross => 50, :invoice => @mock_hash}
- end
-
- it "should create a payment for the order" do
- expected_payment_params = {:reference_hash => @mock_hash}
- PaypalPayment.should_receive(:create).with(expected_payment_params).and_return(@payment)
- get :success, {:mc_gross => 50, :invoice => @mock_hash}
- end
-
- it "should associate the payment with the order" do
- @order.should_receive(:paypal_payment=).with(@payment)
- get :success, {:mc_gross => 50, :invoice => @mock_hash}
- end
-
- it "should destroy the cart" do
- @cart.should_receive(:destroy)
- get :success, {:mc_gross => 50, :invoice => @mock_hash}
- end
-=end
- it_should_behave_like "successful in general"
- end
-
- describe "after notify" do
-=begin
- it "should find the payment using the reference hash" do
- Cart.should_receive(:find_by_reference_hash).with(@mock_hash).and_return(nil)
- PaypalPayment.should_receive(:find_by_reference_hash).with(@mock_hash).and_return(@payment)
- @payment.should_receive(:order).and_return(@order)
- get :success, {:mc_gross => 50, :invoice => @mock_hash}
- end
-=end
- it_should_behave_like "successful in general"
- end
-
+ before(:each) do
end
end
View
68 spec/controllers/paypal_payments_controllers_spec.rb
@@ -0,0 +1,68 @@
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
+include ActiveMerchant::Billing::Integrations
+
+describe PaypalPaymentsController do
+ fixtures :orders
+
+ before(:each) do
+ @order = Order.create(:id => 100, :number => "SAMP-1001", :state => "in_progress", :total => 75.00)
+ Order.stub!(:find).with(any_args).and_return(@order)
+ @ipn = mock("IPN Notification", :invoice => @order.number, :gross => @order.total.to_s, :transaction_id => "TXN1", :fee => "2.00", :currency => "USD", :status => "foo", :received_at => Time.now)
+ # mock the parsing of the IPN object since that's just active_merchant functionality that is already tested
+ Paypal::Notification.stub!(:new).with(any_args).and_return(@ipn)
+ end
+
+ describe "create" do
+ before(:each) { @ipn.stub!(:acknowledge).and_return(true) }
+ it "should create a paypal payment associated with the order" do
+ post :create, :order_id => @order.id
+ @order.paypal_payment.should_not be_nil
+ end
+ it "should create a transaction for the paypal payment" do
+ post :create, :order_id => @order.id
+ @order.paypal_payment.txns.first.should_not be_nil
+ end
+ # TODO - check that the correct values are being assigned to the transaction
+
+ describe "with acknowledge" do
+ before(:each) { @ipn.stub!(:acknowledge).and_return(true) }
+ describe "with ipn status completed" do
+ before(:each) { @ipn.stub!(:status).and_return("Completed") }
+ describe "with matching order total" do
+ it "should change the order state to paid" do
+ post :create, :order_id => @order.id
+ @order.state.should == "paid"
+ end
+ it "should set the tax amount" do
+ post :create, :order_id => @order.id, :tax => "1.50"
+ @order.tax_amount.should == BigDecimal.new("1.50")
+ end
+ it "should set the shipping amount" do
+ post :create, :order_id => @order.id, :mc_shipping => "10.75"
+ @order.ship_amount.should == BigDecimal.new("10.75")
+ end
+ end
+ it "should change the order status to payment failure when order total does not match" do
+ @ipn.stub!(:gross).and_return("1.00")
+ @order.total = 20.75
+ post :create, :order_id => @order.id
+ @order.state.should == "payment_failure"
+ end
+ end
+ it "should set order status to payment failure when ipn status is not completed" do
+ @ipn.stub!(:status).and_return("Foo")
+ post :create, :order_id => @order.id
+ @order.state.should == "payment_failure"
+ end
+ end
+ it "should set order status to payment failure if acknowledge is false" do
+ @ipn.stub!(:acknowledge).and_return(false)
+ post :create, :order_id => @order.id
+ @order.state.should == "payment_failure"
+ end
+ end
+
+ describe "success" do
+ it "should set the IP address of the order"
+ end
+end
View
10 spec/spec_helper.rb
@@ -18,10 +18,10 @@
Dir[File.dirname(__FILE__) + "/matchers/*.rb"].each {|file| require file }
end
-Spec::Runner.configure do |config|
- # config.use_transactional_fixtures = true
- # config.use_instantiated_fixtures = false
- # config.fixture_path = RAILS_ROOT + '/spec/fixtures'
+#Spec::Runner.configure do |config|
+# config.use_transactional_fixtures = true
+# config.use_instantiated_fixtures = false
+# config.fixture_path = File.dirname(__FILE__) + '/fixtures'
# You can declare fixtures for each behaviour like this:
# describe "...." do
@@ -34,4 +34,4 @@
#
# If you declare global fixtures, be aware that they will be declared
# for all of your examples, even those that don't use them.
-end
+#end
Please sign in to comment.
Something went wrong with that request. Please try again.