From 5a4cf547f42f69cb04e153ad0c7619999c47e718 Mon Sep 17 00:00:00 2001 From: hoverlover Date: Sat, 17 Mar 2012 17:48:22 -0500 Subject: [PATCH 1/6] Incorrect extension: rb.rb --- ...se_controller_decorator.rb.rb => base_controller_decorator.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename app/controllers/spree/{base_controller_decorator.rb.rb => base_controller_decorator.rb} (100%) diff --git a/app/controllers/spree/base_controller_decorator.rb.rb b/app/controllers/spree/base_controller_decorator.rb similarity index 100% rename from app/controllers/spree/base_controller_decorator.rb.rb rename to app/controllers/spree/base_controller_decorator.rb From 4f6463175bdb2addd767f62f5662de6e6cb94a53 Mon Sep 17 00:00:00 2001 From: hoverlover Date: Sat, 17 Mar 2012 17:55:43 -0500 Subject: [PATCH 2/6] Add some gems to make testing more enjoyable. --- Gemfile | 9 ++ Gemfile.lock | 226 +++++++++++++++++++++++++++++++++++++++++++++++++++ Guardfile | 19 +++++ 3 files changed, 254 insertions(+) create mode 100644 Gemfile.lock create mode 100644 Guardfile diff --git a/Gemfile b/Gemfile index 0bf92bd..9941e6f 100644 --- a/Gemfile +++ b/Gemfile @@ -2,6 +2,15 @@ source 'http://rubygems.org' group :test do gem 'ffaker' + gem "fuubar" +end + +group :development do + gem "guard-rspec" +end + +group :development, :test do + gem "combustion" end if RUBY_VERSION < "1.9" diff --git a/Gemfile.lock b/Gemfile.lock new file mode 100644 index 0000000..a275dd1 --- /dev/null +++ b/Gemfile.lock @@ -0,0 +1,226 @@ +PATH + remote: . + specs: + spree_paypal_website_standard (1.0.0.beta1) + spree_core (>= 1.0.0.rc1) + +GEM + remote: http://rubygems.org/ + specs: + actionmailer (3.1.4) + actionpack (= 3.1.4) + mail (~> 2.3.0) + actionpack (3.1.4) + activemodel (= 3.1.4) + activesupport (= 3.1.4) + builder (~> 3.0.0) + erubis (~> 2.7.0) + i18n (~> 0.6) + rack (~> 1.3.6) + rack-cache (~> 1.1) + rack-mount (~> 0.8.2) + rack-test (~> 0.6.1) + sprockets (~> 2.0.3) + active_utils (1.0.3) + activesupport (>= 2.3.11) + i18n + activemerchant (1.20.1) + active_utils (>= 1.0.1) + activesupport (>= 2.3.11) + braintree (>= 2.0.0) + builder (>= 2.0.0) + i18n + json (>= 1.5.1) + money (<= 3.7.1) + activemodel (3.1.4) + activesupport (= 3.1.4) + builder (~> 3.0.0) + i18n (~> 0.6) + activerecord (3.1.4) + activemodel (= 3.1.4) + activesupport (= 3.1.4) + arel (~> 2.2.3) + tzinfo (~> 0.3.29) + activeresource (3.1.4) + activemodel (= 3.1.4) + activesupport (= 3.1.4) + activesupport (3.1.4) + multi_json (~> 1.0) + acts_as_list (0.1.4) + archive-tar-minitar (0.5.2) + arel (2.2.3) + braintree (2.14.0) + builder (>= 2.0.0) + builder (3.0.0) + capybara (1.0.1) + mime-types (>= 1.16) + nokogiri (>= 1.3.3) + rack (>= 1.0.0) + rack-test (>= 0.5.4) + selenium-webdriver (~> 2.0) + xpath (~> 0.1.4) + childprocess (0.3.1) + ffi (~> 1.0.6) + cocaine (0.2.1) + columnize (0.3.6) + combustion (0.3.2) + rails (>= 3.0.0) + thor (>= 0.14.6) + deface (0.8.0) + nokogiri (~> 1.5.0) + rails (>= 3.0.9) + diff-lcs (1.1.3) + erubis (2.7.0) + factory_girl (2.6.4) + activesupport (>= 2.3.9) + ffaker (1.12.1) + ffi (1.0.11) + fuubar (1.0.0) + rspec (~> 2.0) + rspec-instafail (~> 0.2.0) + ruby-progressbar (~> 0.0.10) + guard (1.0.1) + ffi (>= 0.5.0) + thor (~> 0.14.6) + guard-rspec (0.6.0) + guard (>= 0.10.0) + highline (1.6.8) + hike (1.2.1) + i18n (0.6.0) + jquery-rails (1.0.19) + railties (~> 3.0) + thor (~> 0.14) + json (1.6.5) + kaminari (0.13.0) + actionpack (>= 3.0.0) + activesupport (>= 3.0.0) + railties (>= 3.0.0) + linecache19 (0.5.12) + ruby_core_source (>= 0.1.4) + mail (2.3.3) + i18n (>= 0.4.0) + mime-types (~> 1.16) + treetop (~> 1.4.8) + meta_search (1.1.1) + actionpack (~> 3.1.0) + activerecord (~> 3.1.0) + activesupport (~> 3.1.0) + polyamorous (~> 0.5.0) + mime-types (1.17.2) + money (3.7.1) + i18n (~> 0.4) + multi_json (1.1.0) + nested_set (1.6.8) + activerecord (>= 3.0.0) + railties (>= 3.0.0) + nokogiri (1.5.2) + paperclip (2.5.0) + activerecord (>= 2.3.0) + activesupport (>= 2.3.2) + cocaine (>= 0.0.2) + mime-types + polyamorous (0.5.0) + activerecord (~> 3.0) + polyglot (0.3.3) + rack (1.3.6) + rack-cache (1.2) + rack (>= 0.4) + rack-mount (0.8.3) + rack (>= 1.0.0) + rack-ssl (1.3.2) + rack + rack-test (0.6.1) + rack (>= 1.0) + rails (3.1.4) + actionmailer (= 3.1.4) + actionpack (= 3.1.4) + activerecord (= 3.1.4) + activeresource (= 3.1.4) + activesupport (= 3.1.4) + bundler (~> 1.0) + railties (= 3.1.4) + railties (3.1.4) + actionpack (= 3.1.4) + activesupport (= 3.1.4) + rack-ssl (~> 1.3.2) + rake (>= 0.8.7) + rdoc (~> 3.4) + thor (~> 0.14.6) + rake (0.9.2.2) + rdoc (3.12) + json (~> 1.4) + rspec (2.8.0) + rspec-core (~> 2.8.0) + rspec-expectations (~> 2.8.0) + rspec-mocks (~> 2.8.0) + rspec-core (2.8.0) + rspec-expectations (2.8.0) + diff-lcs (~> 1.1.2) + rspec-instafail (0.2.2) + rspec-mocks (2.8.0) + rspec-rails (2.8.1) + actionpack (>= 3.0) + activesupport (>= 3.0) + railties (>= 3.0) + rspec (~> 2.8.0) + ruby-debug-base19 (0.11.25) + columnize (>= 0.3.1) + linecache19 (>= 0.5.11) + ruby_core_source (>= 0.1.4) + ruby-debug19 (0.11.6) + columnize (>= 0.3.1) + linecache19 (>= 0.5.11) + ruby-debug-base19 (>= 0.11.19) + ruby-progressbar (0.0.10) + ruby_core_source (0.1.5) + archive-tar-minitar (>= 0.5.2) + rubyzip (0.9.6.1) + selenium-webdriver (2.20.0) + childprocess (>= 0.2.5) + ffi (~> 1.0) + multi_json (~> 1.0) + rubyzip + spree_core (1.0.3) + activemerchant (= 1.20.1) + acts_as_list (= 0.1.4) + deface (>= 0.7.2) + ffaker (~> 1.12.0) + highline (= 1.6.8) + jquery-rails (>= 1.0.18, <= 1.0.19) + kaminari (>= 0.13.0) + meta_search (= 1.1.1) + nested_set (= 1.6.8) + paperclip (= 2.5.0) + rails (>= 3.1.1, <= 3.1.4) + state_machine (= 1.1.1) + stringex (~> 1.3.0) + sprockets (2.0.3) + hike (~> 1.2) + rack (~> 1.0) + tilt (~> 1.1, != 1.3.0) + sqlite3 (1.3.5) + state_machine (1.1.1) + stringex (1.3.2) + thor (0.14.6) + tilt (1.3.3) + treetop (1.4.10) + polyglot + polyglot (>= 0.3.1) + tzinfo (0.3.32) + xpath (0.1.4) + nokogiri (~> 1.3) + +PLATFORMS + ruby + +DEPENDENCIES + capybara (= 1.0.1) + combustion + factory_girl + ffaker + fuubar + guard-rspec + rspec-rails (~> 2.7) + ruby-debug19 + spree_paypal_website_standard! + sqlite3 diff --git a/Guardfile b/Guardfile new file mode 100644 index 0000000..8320ec2 --- /dev/null +++ b/Guardfile @@ -0,0 +1,19 @@ +# A sample Guardfile +# More info at https://github.com/guard/guard#readme + +guard 'rspec', :version => 2, :cli => "--format Fuubar" do + watch(%r{^spec/.+_spec\.rb$}) + watch(%r{^lib/(.+)\.rb$}) { |m| "spec/lib/#{m[1]}_spec.rb" } + watch('spec/spec_helper.rb') { "spec" } + + # Rails example + watch(%r{^app/(.+)\.rb$}) { |m| "spec/#{m[1]}_spec.rb" } + watch(%r{^app/(.*)(\.erb|\.haml)$}) { |m| "spec/#{m[1]}#{m[2]}_spec.rb" } + watch(%r{^app/controllers/(.+)_(controller)\.rb$}) { |m| ["spec/routing/#{m[1]}_routing_spec.rb", "spec/#{m[2]}s/#{m[1]}_#{m[2]}_spec.rb", "spec/acceptance/#{m[1]}_spec.rb"] } + watch(%r{^spec/support/(.+)\.rb$}) { "spec" } + watch('config/routes.rb') { "spec/routing" } + watch('app/controllers/application_controller.rb') { "spec/controllers" } + # Capybara request specs + watch(%r{^app/views/(.+)/.*\.(erb|haml)$}) { |m| "spec/requests/#{m[1]}_spec.rb" } +end + From b3450bba167c3f663e18361aa831178c4b78d79a Mon Sep 17 00:00:00 2001 From: hoverlover Date: Sat, 17 Mar 2012 17:56:59 -0500 Subject: [PATCH 3/6] Stuff for testing engine. --- app/controllers/application_controller.rb | 1 + spec/internal/config/database.yml | 3 +++ spec/internal/config/routes.rb | 4 ++++ spec/internal/db/schema.rb | 3 +++ spec/internal/log/.gitignore | 1 + spec/spec_helper.rb | 5 +++-- 6 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 app/controllers/application_controller.rb create mode 100644 spec/internal/config/database.yml create mode 100644 spec/internal/config/routes.rb create mode 100644 spec/internal/db/schema.rb create mode 100644 spec/internal/log/.gitignore diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb new file mode 100644 index 0000000..c7056de --- /dev/null +++ b/app/controllers/application_controller.rb @@ -0,0 +1 @@ +class ApplicationController < ActionController::Base; end diff --git a/spec/internal/config/database.yml b/spec/internal/config/database.yml new file mode 100644 index 0000000..b978119 --- /dev/null +++ b/spec/internal/config/database.yml @@ -0,0 +1,3 @@ +test: + adapter: sqlite3 + database: db/combustion_test.sqlite diff --git a/spec/internal/config/routes.rb b/spec/internal/config/routes.rb new file mode 100644 index 0000000..58c83a5 --- /dev/null +++ b/spec/internal/config/routes.rb @@ -0,0 +1,4 @@ +Rails.application.routes.draw do + resources :payment_notifications, module: 'spree' + mount Spree::Core::Engine, :at => "/" +end diff --git a/spec/internal/db/schema.rb b/spec/internal/db/schema.rb new file mode 100644 index 0000000..a4ab1bb --- /dev/null +++ b/spec/internal/db/schema.rb @@ -0,0 +1,3 @@ +ActiveRecord::Schema.define do + # +end diff --git a/spec/internal/log/.gitignore b/spec/internal/log/.gitignore new file mode 100644 index 0000000..bf0824e --- /dev/null +++ b/spec/internal/log/.gitignore @@ -0,0 +1 @@ +*.log \ No newline at end of file diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a350164..50d5dfc 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,8 +1,9 @@ # Configure Rails Environment ENV["RAILS_ENV"] = "test" -require File.expand_path("../dummy/config/environment.rb", __FILE__) - +require 'spree_paypal_website_standard' +require 'combustion' +Combustion.initialize! require 'rspec/rails' # Requires supporting ruby files with custom matchers and macros, etc, From 7f8755426b8dbf08d18fe8376c47015ef0948656 Mon Sep 17 00:00:00 2001 From: hoverlover Date: Sat, 17 Mar 2012 17:57:20 -0500 Subject: [PATCH 4/6] Fixed indentation. --- lib/spree_paypal_website_standard/engine.rb | 26 ++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/spree_paypal_website_standard/engine.rb b/lib/spree_paypal_website_standard/engine.rb index 3686f46..f54b8b7 100644 --- a/lib/spree_paypal_website_standard/engine.rb +++ b/lib/spree_paypal_website_standard/engine.rb @@ -7,7 +7,7 @@ class Engine < Rails::Engine initializer "spree.active_shipping.configuration", :after => "spree.environment" do |app| Dir.glob(File.join(File.dirname(__FILE__), "../../lib/spree_paypal_website_standard_configuration.rb")) do |c| - Rails.configuration.cache_classes ? require(c) : load(c) + Rails.configuration.cache_classes ? require(c) : load(c) end end @@ -24,20 +24,20 @@ class Engine < Rails::Engine def self.activate Dir.glob(File.join(File.dirname(__FILE__), "../../app/**/*_decorator*.rb")) do |c| - Rails.configuration.cache_classes ? require(c) : load(c) - end + Rails.configuration.cache_classes ? require(c) : load(c) + end - # add new events and states to the FSM + # add new events and states to the FSM =begin - fsm = Order.state_machines[:state] - fsm.events << StateMachine::Event.new(fsm, "fail_payment") - fsm.events["fail_payment"].transition(:to => 'payment_failure', :from => ['in_progress', 'payment_pending']) - - fsm.events << StateMachine::Event.new(fsm, "pend_payment") - fsm.events["pend_payment"].transition(:to => 'payment_pending', :from => 'in_progress') - fsm.after_transition(:to => 'payment_pending', :do => lambda {|order| order.update_attribute(:checkout_complete, true)}) - - fsm.events["pay"].transition(:to => 'paid', :from => ['payment_pending', 'in_progress']) + fsm = Order.state_machines[:state] + fsm.events << StateMachine::Event.new(fsm, "fail_payment") + fsm.events["fail_payment"].transition(:to => 'payment_failure', :from => ['in_progress', 'payment_pending']) + + fsm.events << StateMachine::Event.new(fsm, "pend_payment") + fsm.events["pend_payment"].transition(:to => 'payment_pending', :from => 'in_progress') + fsm.after_transition(:to => 'payment_pending', :do => lambda {|order| order.update_attribute(:checkout_complete, true)}) + + fsm.events["pay"].transition(:to => 'paid', :from => ['payment_pending', 'in_progress']) =end end From c1f9213f7ff2ff1dd4bae42f257283c980541cbe Mon Sep 17 00:00:00 2001 From: hoverlover Date: Sat, 17 Mar 2012 17:57:53 -0500 Subject: [PATCH 5/6] Ignore IPN that aren't associated with a Spree order. --- .../spree/payment_notifications_controller.rb | 11 ++++++++-- config/locales/en.yml | 2 ++ .../payment_notifications_controller_spec.rb | 20 +++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 spec/controllers/payment_notifications_controller_spec.rb diff --git a/app/controllers/spree/payment_notifications_controller.rb b/app/controllers/spree/payment_notifications_controller.rb index 1fa4517..c2b2e6a 100644 --- a/app/controllers/spree/payment_notifications_controller.rb +++ b/app/controllers/spree/payment_notifications_controller.rb @@ -2,6 +2,7 @@ module Spree class PaymentNotificationsController < BaseController protect_from_forgery :except => [:create] skip_before_filter :restriction_access + before_filter :ignore_foreign_notifications def create if(Spree::PaypalWebsiteStandard::Config.encrypted && (params[:secret] != Spree::PaypalWebsiteStandard::Config.ipn_secret)) @@ -84,6 +85,12 @@ def after_complete def default_country Country.find Spree::PaypalWebsiteStandard::Config.default_country_id end - + + def ignore_foreign_notifications + unless params[:invoice].index /R[0-9]{9}/ + logger.info(I18n.t(:foreign_ipn_message, :scope => "paypal")) + render :nothing => true + end + end end -end \ No newline at end of file +end diff --git a/config/locales/en.yml b/config/locales/en.yml index e62c5a4..942e75c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -8,3 +8,5 @@ en: check_out_with_paypal: "Check out with PayPal" pp_ws_payment_received: "Thank you.. We received the payment for your order from PayPal. Your order will be shipped shortly" pp_ws_order_processed_successfully: "Thank you.. Your order processed succesfully. You will be notified once we receive the payment from PayPal" + paypal: + foreign_ipn_message: "Ignoring IPN with invoice # %{invoice_num} as it didn't originate from a Spree order" diff --git a/spec/controllers/payment_notifications_controller_spec.rb b/spec/controllers/payment_notifications_controller_spec.rb new file mode 100644 index 0000000..c808d5e --- /dev/null +++ b/spec/controllers/payment_notifications_controller_spec.rb @@ -0,0 +1,20 @@ +require "spec_helper" + +module Spree + describe PaymentNotificationsController do + context "when the IPN has an invoice number that is not from a Spree order" do + let(:ipn) { { :invoice => "INV2-AAAA-BBBB-CCCC-DDDD" } } + + it "doesn't create a PaymentNotification" do + expect { + post :create, ipn + }.to_not change { PaymentNotification.count } + end + + it "returns a 200 response" do + post :create, ipn + response.code.should == "200" + end + end + end +end From c0381e29266c65bb1ba07fdaa5deb3e8e867032d Mon Sep 17 00:00:00 2001 From: hoverlover Date: Sat, 17 Mar 2012 18:30:08 -0500 Subject: [PATCH 6/6] Whoops! Failed to notice that when not Spree order, param is actually invoice_id. --- app/controllers/spree/payment_notifications_controller.rb | 2 +- spec/controllers/payment_notifications_controller_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/spree/payment_notifications_controller.rb b/app/controllers/spree/payment_notifications_controller.rb index c2b2e6a..7f7b7b6 100644 --- a/app/controllers/spree/payment_notifications_controller.rb +++ b/app/controllers/spree/payment_notifications_controller.rb @@ -87,7 +87,7 @@ def default_country end def ignore_foreign_notifications - unless params[:invoice].index /R[0-9]{9}/ + if params[:invoice_id].present? logger.info(I18n.t(:foreign_ipn_message, :scope => "paypal")) render :nothing => true end diff --git a/spec/controllers/payment_notifications_controller_spec.rb b/spec/controllers/payment_notifications_controller_spec.rb index c808d5e..d2caeb8 100644 --- a/spec/controllers/payment_notifications_controller_spec.rb +++ b/spec/controllers/payment_notifications_controller_spec.rb @@ -3,7 +3,7 @@ module Spree describe PaymentNotificationsController do context "when the IPN has an invoice number that is not from a Spree order" do - let(:ipn) { { :invoice => "INV2-AAAA-BBBB-CCCC-DDDD" } } + let(:ipn) { { :invoice_id => "INV2-AAAA-BBBB-CCCC-DDDD" } } it "doesn't create a PaymentNotification" do expect {