From 7b5eca3a5088398899adc3a3a76be13bf938bc74 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 20 Aug 2013 09:19:23 +1000 Subject: [PATCH 1/3] Fix error when generating checkout email when distributor_info is nil --- app/helpers/html_helper.rb | 2 +- spec/helpers/html_helper_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 spec/helpers/html_helper_spec.rb diff --git a/app/helpers/html_helper.rb b/app/helpers/html_helper.rb index 85dc5b1c53..c648cf3ace 100644 --- a/app/helpers/html_helper.rb +++ b/app/helpers/html_helper.rb @@ -1,5 +1,5 @@ module HtmlHelper def strip_html(html) - strip_tags(html).gsub(/ /i, ' ').gsub(/&/i, '&') + strip_tags(html).andand.gsub(/ /i, ' ').andand.gsub(/&/i, '&') end end diff --git a/spec/helpers/html_helper_spec.rb b/spec/helpers/html_helper_spec.rb new file mode 100644 index 0000000000..606d301cbd --- /dev/null +++ b/spec/helpers/html_helper_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe HtmlHelper do + class HelperStub + extend ActionView::Helpers::SanitizeHelper::ClassMethods + include ActionView::Helpers::SanitizeHelper + end + + subject do + obj = HelperStub.new + obj.extend HtmlHelper + end + + describe "stripping html from a string" do + it "strips tags" do + subject.strip_html('

Hello world!

').should == 'Hello world!' + end + + it "removes nbsp and amp entities" do + subject.strip_html('Hello world&&').should == 'Hello world&&' + end + + it "returns nil for nil input" do + subject.strip_html(nil).should be_nil + end + end +end From 8dd602f9de5159c75f223d4cbe6c00c5a8f6bfcb Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 20 Aug 2013 09:19:38 +1000 Subject: [PATCH 2/3] Remove pry from spec --- .../chili/enterprises_distributor_info_rich_text_feature_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/features/chili/enterprises_distributor_info_rich_text_feature_spec.rb b/spec/features/chili/enterprises_distributor_info_rich_text_feature_spec.rb index ae919c1a04..cd95945f8b 100644 --- a/spec/features/chili/enterprises_distributor_info_rich_text_feature_spec.rb +++ b/spec/features/chili/enterprises_distributor_info_rich_text_feature_spec.rb @@ -80,7 +80,6 @@ feature "enterprises distributor info as rich text" do complete_purchase_from_checkout_address_page wait_until { ActionMailer::Base.deliveries.length == 1 } email = ActionMailer::Base.deliveries.last - binding.pry email.body.should =~ /Chu ge sai yubi dan bisento tobi ashi yubi ge omote./ email.body.should =~ /Thursday 2nd May/ end From fb33b853ddb35973d1b8e6422e491b2bf8f7d471 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 20 Aug 2013 10:47:56 +1000 Subject: [PATCH 3/3] Revert payment method changes that broke the build: 5f7cbe3, 5a3ad8e, 3ffe732, 2d305b5, 08babee, 6f73f41, 77329b5, 4be2fe5 --- .../payment_methods_controller_decorator.rb | 14 ----------- .../admin/payments_controller_decorator.rb | 25 +++++++------------ app/models/spree/ability_decorator.rb | 2 +- app/models/spree/payment_method_decorator.rb | 6 ----- ...admin_payment_method_edit.html.haml.deface | 2 +- lib/tasks/dev.rake | 8 +----- spec/factories.rb | 12 --------- spec/features/consumer/checkout_spec.rb | 2 ++ 8 files changed, 14 insertions(+), 57 deletions(-) delete mode 100644 app/controllers/spree/admin/payment_methods_controller_decorator.rb diff --git a/app/controllers/spree/admin/payment_methods_controller_decorator.rb b/app/controllers/spree/admin/payment_methods_controller_decorator.rb deleted file mode 100644 index d1d367fc47..0000000000 --- a/app/controllers/spree/admin/payment_methods_controller_decorator.rb +++ /dev/null @@ -1,14 +0,0 @@ -Spree::Admin::PaymentMethodsController.class_eval do - # Only show payment methods that user has access to. - # ! Redundant code copied from Spree::Admin::ResourceController with two added lines - def collection - return parent.send(controller_name) if parent_data.present? - if model_class.respond_to?(:accessible_by) && !current_ability.has_block?(params[:action], model_class) - model_class.accessible_by(current_ability, action). - managed_by(spree_current_user) # this line added - else - model_class.scoped. - managed_by(spree_current_user) # this line added - end - end -end \ No newline at end of file diff --git a/app/controllers/spree/admin/payments_controller_decorator.rb b/app/controllers/spree/admin/payments_controller_decorator.rb index 23db5eca09..4a58dbfe14 100644 --- a/app/controllers/spree/admin/payments_controller_decorator.rb +++ b/app/controllers/spree/admin/payments_controller_decorator.rb @@ -1,21 +1,14 @@ -Spree::Admin::PaymentsController.class_eval do - # When a user fires an event, take them back to where they came from - # Responder: http://guides.spreecommerce.com/developer/logic.html#overriding-controller-action-responses +# When a user fires an event, take them back to where they came from +# Responder: http://guides.spreecommerce.com/developer/logic.html#overriding-controller-action-responses - # For some strange reason, adding PaymentsController.class_eval will cause gems/spree/app/controllers/spree/admin/payments_controller.rb:37 to error: - # payments_url not defined. - # This could be fixed by replacing line 37 with: - # respond_with(@payment, location: admin_order_payments_url) { |format| format.html { redirect_to admin_order_payments_path(@order) } } +# For some strange reason, adding PaymentsController.class_eval will cause gems/spree/app/controllers/spree/admin/payments_controller.rb:37 to error: +# payments_url not defined. +# This could be fixed by replacing line 37 with: +# respond_with(@payment, location: admin_order_payments_url) { |format| format.html { redirect_to admin_order_payments_path(@order) } } + + +Spree::Admin::PaymentsController.class_eval do respond_override :fire => { :html => { :success => lambda { redirect_to request.referer # Keeps any filter and sort prefs } } } - - append_before_filter :filter_payment_methods - - # Only show payments for the order's distributor - def filter_payment_methods - @payment_methods = @payment_methods.select{ |pm| pm.has_distributor? @order.distributor} - @payment_method ||= @payment_methods.first - end - end diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index eae10efb66..34bd9679a4 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -30,7 +30,7 @@ class AbilityDecorator #Enterprise User can only access payment methods for their distributors can [:index, :create], Spree::PaymentMethod - can [:admin, :read, :update, :fire, :resend, :destroy ], Spree::PaymentMethod do |payment_method| + can [:admin, :read, :update, :fire, :resend ], Spree::PaymentMethod do |payment_method| user.enterprises.include? payment_method.distributor end diff --git a/app/models/spree/payment_method_decorator.rb b/app/models/spree/payment_method_decorator.rb index 0f52a4e291..f8759ac06b 100644 --- a/app/models/spree/payment_method_decorator.rb +++ b/app/models/spree/payment_method_decorator.rb @@ -1,8 +1,6 @@ Spree::PaymentMethod.class_eval do belongs_to :distributor, :class_name => 'Enterprise' - validates_presence_of :distributor_id - attr_accessible :distributor_id # -- Scopes @@ -15,10 +13,6 @@ Spree::PaymentMethod.class_eval do where('distributor_id IN (?)', user.enterprises.map {|enterprise| enterprise.id }) end } - - def has_distributor?(distributor) - self.distributor == distributor - end end # Ensure that all derived classes also allow distributor_id diff --git a/app/overrides/spree/admin/payment_methods/_form/add_distributor_to_admin_payment_method_edit.html.haml.deface b/app/overrides/spree/admin/payment_methods/_form/add_distributor_to_admin_payment_method_edit.html.haml.deface index d15ecec373..8ac8e7aad9 100644 --- a/app/overrides/spree/admin/payment_methods/_form/add_distributor_to_admin_payment_method_edit.html.haml.deface +++ b/app/overrides/spree/admin/payment_methods/_form/add_distributor_to_admin_payment_method_edit.html.haml.deface @@ -5,5 +5,5 @@ = f.label :distributor %br - = collection_select(:payment_method, :distributor_id, Enterprise.is_distributor.managed_by(spree_current_user), :id, :name, {:include_blank => false}, {:class => "select2 fullwidth"}) + = collection_select(:payment_method, :distributor_id, Enterprise.is_distributor.managed_by(spree_current_user), :id, :name, {:include_blank => true}, {:class => "select2 fullwidth"}) = f.error_message_on :distributor diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake index 97685544a8..0c397c6325 100644 --- a/lib/tasks/dev.rake +++ b/lib/tasks/dev.rake @@ -16,6 +16,7 @@ namespace :openfoodweb do country = Spree::Country.find_by_name('Australia') Spree::ZoneMember.create(:zone => zone, :zoneable => country) FactoryGirl.create(:shipping_method, :zone => zone) + FactoryGirl.create(:payment_method, :environment => 'development') end @@ -95,13 +96,6 @@ namespace :openfoodweb do end end - # -- Enterprise Payment Methods - unless Spree::PaymentMethod.count > 1 - Enterprise.is_distributor.each do |distributor| - FactoryGirl.create(:payment_method, distributor: distributor, name: "Cheque (#{distributor.name})", :environment => 'development') - end - end - # -- Products unless Spree::Product.count > 0 puts "[#{task_name}] Seeding products" diff --git a/spec/factories.rb b/spec/factories.rb index 7385352ac5..2ce408807f 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -158,18 +158,6 @@ FactoryGirl.modify do state { Spree::State.find_by_name 'Victoria' } country { Spree::Country.find_by_name 'Australia' || Spree::Country.first } end - - factory :payment do - ignore do - distributor { order.distributor || Enterprise.is_distributor.first || FactoryGirl.create(:distributor_enterprise) } - end - payment_method { FactoryGirl.create(:payment_method, distributor: distributor) } - end - - factory :payment_method do - distributor { Enterprise.is_distributor.first || FactoryGirl.create(:distributor_enterprise) } #Always need a distributor - end - end diff --git a/spec/features/consumer/checkout_spec.rb b/spec/features/consumer/checkout_spec.rb index d474ea76bc..b58a627c41 100644 --- a/spec/features/consumer/checkout_spec.rb +++ b/spec/features/consumer/checkout_spec.rb @@ -65,6 +65,7 @@ feature %q{ Spree::ZoneMember.create(:zoneable => c, :zone => @zone) create(:shipping_method, zone: @zone) + @payment_method_all = create(:payment_method, :name => 'Cheque payment method', :description => 'Cheque payment method') #valid for any distributor @payment_method_distributor = create(:payment_method, :name => 'Edible Garden payment method', :distributor => @distributor) @payment_method_alternative = create(:payment_method, :name => 'Alternative Distributor payment method', :distributor => @distributor_alternative) end @@ -309,6 +310,7 @@ feature %q{ # -- Checkout: Payment # Given the distributor I have selected for my order, I should only see payment methods valid for that distributor + page.should have_selector 'label', :text => @payment_method_all.name page.should have_selector 'label', :text => @payment_method_distributor.name page.should_not have_selector 'label', :text => @payment_method_alternative.name click_checkout_continue_button