diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b0daa298ba..867205b8e4 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -7,6 +7,7 @@ on: env: DISABLE_KNAPSACK: true + TIMEZONE: UTC jobs: test-models: diff --git a/Gemfile.lock b/Gemfile.lock index 91d9975730..b6711f1f1f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -123,7 +123,7 @@ GEM atomic (1.1.101) awesome_nested_set (3.4.0) activerecord (>= 4.0.0, < 7.0) - awesome_print (1.8.0) + awesome_print (1.9.2) aws-sdk (1.67.0) aws-sdk-v1 (= 1.67.0) aws-sdk-v1 (1.67.0) @@ -266,7 +266,7 @@ GEM tilt hashdiff (1.0.1) highline (2.0.3) - i18n (1.8.5) + i18n (1.8.9) concurrent-ruby (~> 1.0) i18n-js (3.8.1) i18n (>= 0.6.6) @@ -450,10 +450,10 @@ GEM rspec-expectations (3.10.1) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.10.0) - rspec-mocks (3.10.0) + rspec-mocks (3.10.2) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.10.0) - rspec-rails (4.0.2) + rspec-rails (4.1.2) actionpack (>= 4.2) activesupport (>= 4.2) railties (>= 4.2) @@ -463,7 +463,7 @@ GEM rspec-support (~> 3.10) rspec-retry (0.6.2) rspec-core (> 3.3) - rspec-support (3.10.1) + rspec-support (3.10.2) rswag (2.4.0) rswag-api (= 2.4.0) rswag-specs (= 2.4.0) diff --git a/app/assets/stylesheets/darkswarm/footer.scss b/app/assets/stylesheets/darkswarm/footer.scss index 9826970141..43b5898c7c 100644 --- a/app/assets/stylesheets/darkswarm/footer.scss +++ b/app/assets/stylesheets/darkswarm/footer.scss @@ -4,8 +4,8 @@ footer { .row { - p a { - font-size: 0.875rem; + p { + font-size: 1rem; } a, a * { @@ -107,9 +107,14 @@ footer { color: rgba($disabled-med, 0.35); } + p.text-big { + font-size: 1.5rem; + font-weight: 300; + } + .social-icons { - margin-bottom: 0.25rem; - margin-top: 0.75rem; + margin-bottom: 0.9rem; + padding-top: 0.1rem; a { i { @@ -128,5 +133,9 @@ footer { } } } + + .legal p { + font-size: 0.875rem; + } } } diff --git a/app/controllers/admin/enterprise_groups_controller.rb b/app/controllers/admin/enterprise_groups_controller.rb index ad1489e654..0691d9eeaa 100644 --- a/app/controllers/admin/enterprise_groups_controller.rb +++ b/app/controllers/admin/enterprise_groups_controller.rb @@ -25,15 +25,14 @@ module Admin protected - def build_resource_with_address - enterprise_group = build_resource_without_address + def build_resource + enterprise_group = super enterprise_group.address = Spree::Address.new enterprise_group.address.country = Spree::Country.find_by( id: Spree::Config[:default_country_id] ) enterprise_group end - alias_method_chain :build_resource, :address # Overriding method on Spree's resource controller, # so that resources are found using permalink. diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 87a3dff224..9b42b39174 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -114,13 +114,12 @@ module Admin protected - def build_resource_with_address - enterprise = build_resource_without_address + def build_resource + enterprise = super enterprise.address ||= Spree::Address.new enterprise.address.country ||= Spree::Country.find_by(id: Spree::Config[:default_country_id]) enterprise end - alias_method_chain :build_resource, :address # Overriding method on Spree's resource controller, # so that resources are found using permalink diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 4022925385..1b8cd07d67 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -6,9 +6,9 @@ class CheckoutController < ::BaseController layout 'darkswarm' include OrderStockCheck - include CheckoutHelper - include OrderCyclesHelper - include EnterprisesHelper + + helper 'terms_and_conditions' + helper 'checkout' ssl_required diff --git a/app/controllers/spree/admin/payments_controller.rb b/app/controllers/spree/admin/payments_controller.rb index cddf685dd7..4865226c94 100644 --- a/app/controllers/spree/admin/payments_controller.rb +++ b/app/controllers/spree/admin/payments_controller.rb @@ -136,7 +136,7 @@ module Spree # # Otherwise redirect user to that step def can_transition_to_payment - return if @order.payment? || @order.complete? || @order.canceled? + return if @order.payment? || @order.complete? || @order.canceled? || @order.resumed? flash[:notice] = Spree.t(:fill_in_customer_info) redirect_to spree.edit_admin_order_customer_url(@order) diff --git a/app/helpers/footer_links_helper.rb b/app/helpers/footer_links_helper.rb index a4caad2abd..e084cca277 100644 --- a/app/helpers/footer_links_helper.rb +++ b/app/helpers/footer_links_helper.rb @@ -15,4 +15,10 @@ module FooterLinksHelper target: '_blank', rel: 'noopener' ) end + + def show_social_icons? + ContentConfig.footer_facebook_url.present? || ContentConfig.footer_twitter_url.present? || + ContentConfig.footer_instagram_url.present? || ContentConfig.footer_linkedin_url.present? || + ContentConfig.footer_googleplus_url.present? || ContentConfig.footer_pinterest_url.present? + end end diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index 7addd06135..3af44f885f 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -2,6 +2,8 @@ require 'open_food_network/enterprise_injection_data' module InjectionHelper include SerializerHelper + include EnterprisesHelper + include OrderCyclesHelper def inject_enterprises(enterprises = nil) inject_json_array( diff --git a/app/mailers/spree/order_mailer.rb b/app/mailers/spree/order_mailer.rb index 9fbed9f02b..00002715c3 100644 --- a/app/mailers/spree/order_mailer.rb +++ b/app/mailers/spree/order_mailer.rb @@ -2,7 +2,7 @@ module Spree class OrderMailer < BaseMailer - helper ::CheckoutHelper + helper 'checkout' helper SpreeCurrencyHelper helper Spree::Admin::PaymentsHelper helper OrderHelper diff --git a/app/mailers/subscription_mailer.rb b/app/mailers/subscription_mailer.rb index 0f60ccbeab..dd20008bfc 100644 --- a/app/mailers/subscription_mailer.rb +++ b/app/mailers/subscription_mailer.rb @@ -1,5 +1,5 @@ class SubscriptionMailer < Spree::BaseMailer - helper CheckoutHelper + helper 'checkout' helper MailerHelper helper ShopMailHelper helper OrderHelper diff --git a/app/models/concerns/balance.rb b/app/models/concerns/balance.rb new file mode 100644 index 0000000000..91019d401c --- /dev/null +++ b/app/models/concerns/balance.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'active_support/concern' + +# Contains the methods to compute an order balance form the point of view of the enterprise and not +# the individual shopper. +module Balance + FINALIZED_NON_SUCCESSFUL_STATES = %w(canceled returned).freeze + + # Returns the order balance by considering the total as money owed to the order distributor aka. + # the shop, and as a positive balance of said enterprise. If the customer pays it all, they + # distributor and customer are even. + # + # Note however, this is meant to be used only in the context of a single order object. When + # working with a collection of orders, such an index controller action, please consider using + # `app/queries/oustanding_balance.rb` instead so we avoid potential N+1s. + def new_outstanding_balance + if state.in?(FINALIZED_NON_SUCCESSFUL_STATES) + -payment_total + else + total - payment_total + end + end + + # This method is the one we're gradually replacing with `#new_outstanding_balance`. Having them + # separate enables us to choose which implementation we want depending on the context using + # a feature toggle. This avoids incosistent behavior across the app during that incremental + # refactoring. + # + # It is meant to be removed as soon as we get product approval that the new implementation has + # been working correctly in production. + def outstanding_balance + total - payment_total + end + + def outstanding_balance? + !outstanding_balance.zero? + end + + def display_outstanding_balance + Spree::Money.new(outstanding_balance, currency: currency) + end +end diff --git a/app/models/customer.rb b/app/models/customer.rb index 256fc1b61c..b1ba0f2651 100644 --- a/app/models/customer.rb +++ b/app/models/customer.rb @@ -2,15 +2,15 @@ class Customer < ActiveRecord::Base acts_as_taggable belongs_to :enterprise - belongs_to :user, class_name: Spree.user_class - has_many :orders, class_name: Spree::Order + belongs_to :user, class_name: Spree.user_class.to_s + has_many :orders, class_name: "Spree::Order" before_destroy :check_for_orders - belongs_to :bill_address, foreign_key: :bill_address_id, class_name: Spree::Address + belongs_to :bill_address, foreign_key: :bill_address_id, class_name: "Spree::Address" alias_attribute :billing_address, :bill_address accepts_nested_attributes_for :bill_address - belongs_to :ship_address, foreign_key: :ship_address_id, class_name: Spree::Address + belongs_to :ship_address, foreign_key: :ship_address_id, class_name: "Spree::Address" alias_attribute :shipping_address, :ship_address accepts_nested_attributes_for :ship_address diff --git a/app/models/distributor_shipping_method.rb b/app/models/distributor_shipping_method.rb index a2c81506dd..5c69cefbc0 100644 --- a/app/models/distributor_shipping_method.rb +++ b/app/models/distributor_shipping_method.rb @@ -1,5 +1,5 @@ class DistributorShippingMethod < ActiveRecord::Base self.table_name = "distributors_shipping_methods" - belongs_to :shipping_method, class_name: Spree::ShippingMethod, touch: true - belongs_to :distributor, class_name: Enterprise, touch: true + belongs_to :shipping_method, class_name: "Spree::ShippingMethod", touch: true + belongs_to :distributor, class_name: "Enterprise", touch: true end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index b3a5669f03..135358fdc7 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -250,7 +250,7 @@ class Enterprise < ActiveRecord::Base def plus_relatives_and_oc_producers(order_cycles) oc_producer_ids = Exchange.in_order_cycle(order_cycles).incoming.pluck :sender_id - Enterprise.relatives_of_one_union_others(id, oc_producer_ids | [id]) + Enterprise.is_primary_producer.relatives_of_one_union_others(id, oc_producer_ids | [id]) end def relatives_including_self diff --git a/app/models/enterprise_role.rb b/app/models/enterprise_role.rb index 3709f7e004..0b686e2e6e 100644 --- a/app/models/enterprise_role.rb +++ b/app/models/enterprise_role.rb @@ -1,5 +1,5 @@ class EnterpriseRole < ActiveRecord::Base - belongs_to :user, class_name: Spree.user_class + belongs_to :user, class_name: Spree.user_class.to_s belongs_to :enterprise validates :user_id, :enterprise_id, presence: true diff --git a/app/models/spree/app_configuration.rb b/app/models/spree/app_configuration.rb index 67b843a4ce..6b4a57aa76 100644 --- a/app/models/spree/app_configuration.rb +++ b/app/models/spree/app_configuration.rb @@ -36,8 +36,6 @@ module Spree preference :allow_ssl_in_development_and_test, :boolean, default: false preference :allow_ssl_in_production, :boolean, default: true preference :allow_ssl_in_staging, :boolean, default: true - # Automatically capture the credit card (as opposed to just authorize and capture later) - preference :auto_capture, :boolean, default: false # Replace with the name of a zone if you would like to limit the countries preference :checkout_zone, :string, default: nil preference :currency, :string, default: "USD" diff --git a/app/models/spree/gateway/pay_pal_express.rb b/app/models/spree/gateway/pay_pal_express.rb index 357d563264..bc7d8cef64 100644 --- a/app/models/spree/gateway/pay_pal_express.rb +++ b/app/models/spree/gateway/pay_pal_express.rb @@ -31,10 +31,6 @@ module Spree provider_class.new end - def auto_capture? - true - end - def method_type 'paypal' end diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index cda88aa395..d224b154a8 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -174,11 +174,11 @@ module Spree end def has_tax? - adjustments.included_tax.any? + adjustments.tax.any? end def included_tax - adjustments.included_tax.sum(:included_tax) + adjustments.tax.inclusive.sum(:amount) end def tax_rates diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 343fd59dce..b1731b42d4 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -9,7 +9,9 @@ require 'concerns/order_shipment' module Spree class Order < ActiveRecord::Base prepend OrderShipment + include Checkout + include Balance checkout_flow do go_to_state :address @@ -125,7 +127,7 @@ module Spree } scope :not_state, lambda { |state| - where("state != ?", state) + where.not(state: state) } # All the states an order can be in after completing the checkout @@ -166,10 +168,6 @@ module Spree self[:currency] || Spree::Config[:currency] end - def display_outstanding_balance - Spree::Money.new(outstanding_balance, currency: currency) - end - def display_item_total Spree::Money.new(item_total, currency: currency) end @@ -374,14 +372,6 @@ module Spree Spree::TaxRate.adjust(self) end - def outstanding_balance - total - payment_total - end - - def outstanding_balance? - outstanding_balance != 0 - end - def name address = bill_address || ship_address return unless address @@ -672,7 +662,9 @@ module Spree end def total_tax - all_adjustments.sum(:included_tax) + adjustments.sum(:included_tax) + + shipment_adjustments.sum(:included_tax) + + line_item_adjustments.tax.sum(:amount) end def has_taxes_included diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index e45730583b..e3a9aab7bb 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -154,8 +154,10 @@ module Spree return unless adjustment.try(:reload) return if adjustment.finalized? - adjustment.update_attribute(:eligible, false) - adjustment.finalize! + adjustment.update( + eligible: false, + state: "finalized" + ) end def validate_source diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb index 08eb305566..9e9c0d959f 100644 --- a/app/models/spree/payment/processing.rb +++ b/app/models/spree/payment/processing.rb @@ -6,22 +6,14 @@ module Spree def process! return unless validate! - if payment_method.auto_capture? - purchase! - else - authorize! - end + purchase! end def process_offline! return unless validate! return if authorization_action_required? - if payment_method.auto_capture? - charge_offline! - else - authorize! - end + charge_offline! end def authorize!(return_url = nil) diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index d519e3284a..4204632b91 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -89,10 +89,6 @@ module Spree true end - def auto_capture? - Spree::Config[:auto_capture] - end - def supports?(_source) true end diff --git a/app/models/spree/tax_rate.rb b/app/models/spree/tax_rate.rb index e0fe37a776..eb543306f7 100644 --- a/app/models/spree/tax_rate.rb +++ b/app/models/spree/tax_rate.rb @@ -84,7 +84,7 @@ module Spree order.line_items.reload # TaxRate adjustments (order.adjustments.tax) # and line item adjustments (tax included on line items) consist of 100% tax - (order.adjustments.tax + order.line_item_adjustments.reload).each do |adjustment| + order.adjustments.tax.additional.each do |adjustment| adjustment.set_absolute_included_tax! adjustment.amount end end diff --git a/app/models/subscription.rb b/app/models/subscription.rb index ca1fd676e8..dbb5a8bf23 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -8,8 +8,8 @@ class Subscription < ActiveRecord::Base belongs_to :schedule belongs_to :shipping_method, class_name: 'Spree::ShippingMethod' belongs_to :payment_method, class_name: 'Spree::PaymentMethod' - belongs_to :bill_address, foreign_key: :bill_address_id, class_name: Spree::Address - belongs_to :ship_address, foreign_key: :ship_address_id, class_name: Spree::Address + belongs_to :bill_address, foreign_key: :bill_address_id, class_name: "Spree::Address" + belongs_to :ship_address, foreign_key: :ship_address_id, class_name: "Spree::Address" has_many :subscription_line_items, inverse_of: :subscription has_many :order_cycles, through: :schedule has_many :proxy_orders diff --git a/app/queries/complete_visible_orders.rb b/app/queries/complete_visible_orders.rb new file mode 100644 index 0000000000..10eabe4f6d --- /dev/null +++ b/app/queries/complete_visible_orders.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CompleteVisibleOrders + def initialize(order_permissions) + @order_permissions = order_permissions + end + + def query + order_permissions.visible_orders.complete + end + + private + + attr_reader :order_permissions +end diff --git a/app/queries/outstanding_balance.rb b/app/queries/outstanding_balance.rb index ce4d4d78f4..48ee451009 100644 --- a/app/queries/outstanding_balance.rb +++ b/app/queries/outstanding_balance.rb @@ -8,6 +8,9 @@ # cases. # # See CompleteOrdersWithBalance or CustomersWithBalance as examples. +# +# Note this query object and `app/models/concerns/balance.rb` should implement the same behavior +# until we find a better way. If you change one, please, change the other too. class OutstandingBalance # All the states of a finished order but that shouldn't count towards the balance (the customer # didn't get the order for whatever reason). Note it does not include complete diff --git a/app/services/invoice_renderer.rb b/app/services/invoice_renderer.rb index 2f8663d24c..bbb1886a5b 100644 --- a/app/services/invoice_renderer.rb +++ b/app/services/invoice_renderer.rb @@ -1,4 +1,8 @@ class InvoiceRenderer + def initialize(renderer = ApplicationController.new) + @renderer = renderer + end + def render_to_string(order) renderer.instance_variable_set(:@order, order) renderer.render_to_string(args(order)) @@ -15,9 +19,7 @@ class InvoiceRenderer private - def renderer - @renderer ||= ApplicationController.new - end + attr_reader :renderer def invoice_template if Spree::Config.invoice_style2? diff --git a/app/services/order_tax_adjustments_fetcher.rb b/app/services/order_tax_adjustments_fetcher.rb index 23b5dde065..686efff9a4 100644 --- a/app/services/order_tax_adjustments_fetcher.rb +++ b/app/services/order_tax_adjustments_fetcher.rb @@ -21,7 +21,6 @@ class OrderTaxAdjustmentsFetcher def all Spree::Adjustment - .with_tax .where(order_adjustments.or(line_item_adjustments).or(shipment_adjustments)) .order('created_at ASC') end @@ -50,11 +49,27 @@ class OrderTaxAdjustmentsFetcher Hash[tax_rates.collect do |tax_rate| tax_amount = if tax_rates.one? - adjustment.included_tax + adjustment_tax_amount(adjustment) else tax_rate.compute_tax(adjustment.amount) end [tax_rate, tax_amount] end] end + + def adjustment_tax_amount(adjustment) + if no_tax_adjustments?(adjustment) + adjustment.included_tax + else + adjustment.amount + end + end + + def no_tax_adjustments?(adjustment) + # Enterprise Fees, Admin Adjustments, and Shipping Fees currently do not have tax adjustments. + # The tax amount is stored in the included_tax attribute. + adjustment.originator_type == "EnterpriseFee" || + adjustment.originator_type == "Spree::ShippingMethod" || + (adjustment.source_type.nil? && adjustment.originator_type.nil?) + end end diff --git a/app/services/tax_rate_finder.rb b/app/services/tax_rate_finder.rb index f27a42ef85..01b955d3d0 100644 --- a/app/services/tax_rate_finder.rb +++ b/app/services/tax_rate_finder.rb @@ -75,7 +75,7 @@ class TaxRateFinder # to the included tax. def find_closest_tax_rates_from_included_tax(amount, included_tax) approximation = (included_tax / (amount - included_tax)) - return [] if approximation.infinite? || approximation.zero? + return [] if approximation.infinite? || approximation.zero? || approximation.nan? [Spree::TaxRate.order("ABS(amount - #{approximation})").first] end diff --git a/app/views/shared/_footer.html.haml b/app/views/shared/_footer.html.haml index 2ba6278bec..e7fb4e7dcb 100644 --- a/app/views/shared/_footer.html.haml +++ b/app/views/shared/_footer.html.haml @@ -31,25 +31,26 @@ // This is the instance-managed set of links: %h4 = t '.footer_contact_headline' - %p.social-icons - - if ContentConfig.footer_facebook_url.present? - %a{href: ContentConfig.footer_facebook_url} - %i.ofn-i_044-facebook - - if ContentConfig.footer_twitter_url.present? - %a{href: ContentConfig.footer_twitter_url} - %i.ofn-i_041-twitter - - if ContentConfig.footer_instagram_url.present? - %a{href: ContentConfig.footer_instagram_url} - %i.ofn-i_043-instagram - - if ContentConfig.footer_linkedin_url.present? - %a{href: ContentConfig.footer_linkedin_url} - %i.ofn-i_042-linkedin - - if ContentConfig.footer_googleplus_url.present? - %a{href: ContentConfig.footer_googleplus_url} - %i.ofn-i_046-g - - if ContentConfig.footer_pinterest_url.present? - %a{href: ContentConfig.footer_pinterest_url} - %i.ofn-i_045-pintrest + - if show_social_icons? + %p.social-icons + - if ContentConfig.footer_facebook_url.present? + %a{href: ContentConfig.footer_facebook_url} + %i.ofn-i_044-facebook + - if ContentConfig.footer_twitter_url.present? + %a{href: ContentConfig.footer_twitter_url} + %i.ofn-i_041-twitter + - if ContentConfig.footer_instagram_url.present? + %a{href: ContentConfig.footer_instagram_url} + %i.ofn-i_043-instagram + - if ContentConfig.footer_linkedin_url.present? + %a{href: ContentConfig.footer_linkedin_url} + %i.ofn-i_042-linkedin + - if ContentConfig.footer_googleplus_url.present? + %a{href: ContentConfig.footer_googleplus_url} + %i.ofn-i_046-g + - if ContentConfig.footer_pinterest_url.present? + %a{href: ContentConfig.footer_pinterest_url} + %i.ofn-i_045-pintrest - if ContentConfig.footer_email.present? %p %a{href: ContentConfig.footer_email.reverse, mailto: true, target: '_blank'} @@ -92,7 +93,7 @@ %hr.hr-light %br - .row + .row.legal .small-12.medium-3.medium-offset-2.columns.text-left %a{href: main_app.root_path} %img{src: ContentConfig.footer_logo.url, width: "220"} @@ -107,10 +108,9 @@ %p.text-small = t '.footer_legal_text_html', {content_license: link_to('CC BY-SA 3.0', 'https://creativecommons.org/licenses/by-sa/3.0/'), code_license: link_to('AGPL 3', 'https://tldrlegal.com/license/gnu-affero-general-public-license-v3-(agpl-3.0)' )} %p.text-small - %div - - if Spree::Config.privacy_policy_url.present? - = t '.footer_data_text_with_privacy_policy_html', {cookies_policy: cookies_policy_link.html_safe, privacy_policy: privacy_policy_link.html_safe } - - else - = t '.footer_data_text_without_privacy_policy_html', {cookies_policy: cookies_policy_link.html_safe } + - if Spree::Config.privacy_policy_url.present? + = t '.footer_data_text_with_privacy_policy_html', {cookies_policy: cookies_policy_link.html_safe, privacy_policy: privacy_policy_link.html_safe } + - else + = t '.footer_data_text_without_privacy_policy_html', {cookies_policy: cookies_policy_link.html_safe } .medium-2.columns.text-center / Placeholder diff --git a/config/application.rb b/config/application.rb index 73d9e78184..33638102f5 100644 --- a/config/application.rb +++ b/config/application.rb @@ -200,7 +200,5 @@ module Openfoodnetwork config.active_support.escape_html_entities_in_json = true config.active_job.queue_adapter = :delayed_job - - config.active_record.raise_in_transactional_callbacks = true end end diff --git a/config/environments/production.rb b/config/environments/production.rb index 695f61a0a1..826a4de616 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -11,7 +11,7 @@ Openfoodnetwork::Application.configure do config.action_controller.perform_caching = true # Disable Rails's static asset server (Apache or nginx will already do this) - config.serve_static_files = false + config.public_file_server.enabled = false # Compress JavaScripts and CSS config.assets.compress = true diff --git a/config/environments/staging.rb b/config/environments/staging.rb index 695f61a0a1..826a4de616 100644 --- a/config/environments/staging.rb +++ b/config/environments/staging.rb @@ -11,7 +11,7 @@ Openfoodnetwork::Application.configure do config.action_controller.perform_caching = true # Disable Rails's static asset server (Apache or nginx will already do this) - config.serve_static_files = false + config.public_file_server.enabled = false # Compress JavaScripts and CSS config.assets.compress = true diff --git a/config/environments/test.rb b/config/environments/test.rb index 665525bb13..dd68bc0459 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -10,8 +10,8 @@ Openfoodnetwork::Application.configure do config.eager_load = false # Configure static asset server for tests with Cache-Control for performance - config.serve_static_files = true - config.static_cache_control = "public, max-age=3600" + config.public_file_server.enabled = true + config.public_file_server.headers = { 'Cache-Control' => 'public, max-age=3600' } # Separate cache stores when running in parallel config.cache_store = :file_store, Rails.root.join("tmp", "cache", "paralleltests#{ENV['TEST_ENV_NUMBER']}") diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb index 09e404fc9f..2d8a5bbe6a 100644 --- a/config/initializers/spree.rb +++ b/config/initializers/spree.rb @@ -22,10 +22,6 @@ Spree.config do |config| config.address_requires_state = true config.admin_interface_logo = '/default_images/ofn-logo.png' - # -- spree_paypal_express - # Auto-capture payments. Without this option, payments must be manually captured in the paypal interface. - config.auto_capture = true - # S3 settings config.s3_bucket = ENV['S3_BUCKET'] if ENV['S3_BUCKET'] config.s3_access_key = ENV['S3_ACCESS_KEY'] if ENV['S3_ACCESS_KEY'] diff --git a/config/locales/en_CA.yml b/config/locales/en_CA.yml index c07d8cb4a5..999346951b 100644 --- a/config/locales/en_CA.yml +++ b/config/locales/en_CA.yml @@ -1186,6 +1186,7 @@ en_CA: mailers: powered_by: open_food_network: "Open Food Network" + powered_html: "%{open_food_network}" menu: cart: cart: "Cart" @@ -1252,7 +1253,7 @@ en_CA: invoice_tax_total: "Sales Tax Total:" tax_invoice: "TAX INVOICE" tax_total: "Total tax (%{rate}):" - total_excl_tax: "Total (Excl. tax):" + total_excl_tax: "Total (incl. shipping tax if applicable)" total_incl_tax: "Total (Incl. tax):" abn: "Business Number:" acn: "Business Number:" @@ -2882,6 +2883,7 @@ en_CA: name_or_sku: "Name or SKU (enter at least first 4 characters of product name)" resend: "Resend" back_to_orders_list: "Back to Orders List" + back_to_payments_list: "Back To Payments List" return_authorizations: "Return Authoriations" cannot_create_returns: "Cannot create returns as this order has no shipped units" select_stock: "Select stock" @@ -3179,7 +3181,7 @@ en_CA: code: "Code" from: "From" to: "Bill to" - shipping: "Shipping" + shipping: "Delivery/Pick-up" form: distribution_fields: title: "Distribution" @@ -3451,6 +3453,7 @@ en_CA: pending: pending ready: ready shipped: shipped + canceled: canceled payment_states: balance_due: balance due completed: completed @@ -3572,6 +3575,8 @@ en_CA: past_orders: Past Orders transactions: transaction_history: Transaction History + authorisation_required: Authorisation Required + authorise: Authorize open_orders: order: Order shop: Shop diff --git a/config/locales/en_IE.yml b/config/locales/en_IE.yml index 86b30bee73..6f3ef2d189 100644 --- a/config/locales/en_IE.yml +++ b/config/locales/en_IE.yml @@ -1186,6 +1186,7 @@ en_IE: mailers: powered_by: open_food_network: "Open Food Network" + powered_html: "Your shopping experience is powered by the %{open_food_network}." menu: cart: cart: "Basket" @@ -1552,6 +1553,7 @@ en_IE: set_a_password: "You will then be prompted to set a password before you are able to administer the enterprise." mistakenly_sent: "Not sure why you have received this email? Please contact %{owner_email} for more information." producer_mail_greeting: "Dear" + producer_mail_text_before: "Please find below an update about the order cycle ready for:" producer_mail_order_text: "Here is a summary of the orders for your products:" producer_mail_delivery_instructions: "Stock pickup/delivery instructions:" producer_mail_signoff: "Thanks and best wishes" @@ -2888,6 +2890,7 @@ en_IE: name_or_sku: "Name or SKU (enter at least first 4 characters of product name)" resend: "Resend" back_to_orders_list: "Back To Orders List" + back_to_payments_list: "Back To Payments List" return_authorizations: "Return Authorisations" cannot_create_returns: "Cannot create returns as this order has no shipped units." select_stock: "Select stock" @@ -3457,6 +3460,7 @@ en_IE: pending: pending ready: ready shipped: shipped + canceled: canceled payment_states: balance_due: balance due completed: completed @@ -3578,6 +3582,8 @@ en_IE: past_orders: Past Orders transactions: transaction_history: Transaction History + authorisation_required: Authorisation Required + authorise: Authorize open_orders: order: Order shop: Shop diff --git a/config/locales/fr_CA.yml b/config/locales/fr_CA.yml index c340aa827d..aea564acf9 100644 --- a/config/locales/fr_CA.yml +++ b/config/locales/fr_CA.yml @@ -1188,6 +1188,7 @@ fr_CA: mailers: powered_by: open_food_network: "Open Food Network " + powered_html: "Cette commande a été rendue possible par %{open_food_network}." menu: cart: cart: "Panier" @@ -2896,6 +2897,7 @@ fr_CA: name_or_sku: "Nom ou Ref Produit (entrer au moins les 4 premiers caractères du nom du produit)" resend: "Renvoyer" back_to_orders_list: "Retour à la liste des commandes" + back_to_payments_list: "Retour à la liste des paiements" return_authorizations: "Autorisations de retours" cannot_create_returns: "Impossible de créer une autorisation de retour car aucun produit n'a été livré pour cette commande." select_stock: "Sélectionner le stock" @@ -3465,6 +3467,7 @@ fr_CA: pending: en attente ready: prêt shipped: envoyé + canceled: Annulé payment_states: balance_due: solde dû completed: effectué @@ -3587,6 +3590,8 @@ fr_CA: past_orders: Commandes Passées transactions: transaction_history: Historique des Transactions + authorisation_required: Autorisation nécessaire + authorise: Autorise open_orders: order: Commander shop: Faire mes courses diff --git a/engines/order_management/app/controllers/order_management/reports/bulk_coop_controller.rb b/engines/order_management/app/controllers/order_management/reports/bulk_coop_controller.rb index c1810a92b6..3de16eaf61 100644 --- a/engines/order_management/app/controllers/order_management/reports/bulk_coop_controller.rb +++ b/engines/order_management/app/controllers/order_management/reports/bulk_coop_controller.rb @@ -3,8 +3,8 @@ module OrderManagement module Reports class BulkCoopController < Spree::Admin::BaseController - before_filter :load_report_parameters - before_filter :load_permissions + before_action :load_report_parameters + before_action :load_permissions def new; end diff --git a/engines/order_management/app/controllers/order_management/reports/enterprise_fee_summaries_controller.rb b/engines/order_management/app/controllers/order_management/reports/enterprise_fee_summaries_controller.rb index 7cc1376034..9f52ab3e9b 100644 --- a/engines/order_management/app/controllers/order_management/reports/enterprise_fee_summaries_controller.rb +++ b/engines/order_management/app/controllers/order_management/reports/enterprise_fee_summaries_controller.rb @@ -3,8 +3,8 @@ module OrderManagement module Reports class EnterpriseFeeSummariesController < Spree::Admin::BaseController - before_filter :load_report_parameters - before_filter :load_permissions + before_action :load_report_parameters + before_action :load_permissions def new; end diff --git a/engines/order_management/app/services/order_management/order/updater.rb b/engines/order_management/app/services/order_management/order/updater.rb index 7ff69ceeaa..4aac1f7843 100644 --- a/engines/order_management/app/services/order_management/order/updater.rb +++ b/engines/order_management/app/services/order_management/order/updater.rb @@ -59,7 +59,7 @@ module OrderManagement def update_adjustment_total order.adjustment_total = all_adjustments.additional.eligible.sum(:amount) order.additional_tax_total = all_adjustments.tax.additional.sum(:amount) - order.included_tax_total = order.line_item_adjustments.tax.sum(:included_tax) + + order.included_tax_total = order.line_item_adjustments.tax.inclusive.sum(:amount) + all_adjustments.enterprise_fee.sum(:included_tax) + all_adjustments.shipping.sum(:included_tax) + adjustments.admin.sum(:included_tax) diff --git a/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb b/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb index c9c8b822f1..1f658b189a 100644 --- a/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb +++ b/engines/order_management/app/services/order_management/reports/bulk_coop/bulk_coop_report.rb @@ -14,6 +14,7 @@ module OrderManagement ].freeze attr_reader :params + def initialize(user, params = {}, render_table = false) @params = params @user = user @@ -21,6 +22,7 @@ module OrderManagement @supplier_report = BulkCoopSupplierReport.new @allocation_report = BulkCoopAllocationReport.new + @filter_canceled = false end def header @@ -137,6 +139,8 @@ module OrderManagement private + attr_reader :filter_canceled + def line_item_includes [ { @@ -148,25 +152,35 @@ module OrderManagement end def order_permissions - return @order_permissions unless @order_permissions.nil? - - @order_permissions = ::Permissions::Order.new(@user, @params[:q]) + @order_permissions ||= ::Permissions::Order.new(@user, filter_canceled) end def report_line_items - @report_line_items ||= OpenFoodNetwork::Reports::LineItems.new(order_permissions, @params) + @report_line_items ||= OpenFoodNetwork::Reports::LineItems.new( + order_permissions, + @params, + CompleteVisibleOrders.new(order_permissions).query + ) end def customer_payments_total_cost(line_items) - line_items.map(&:order).uniq.sum(&:total) + unique_orders(line_items).sum(&:total) end def customer_payments_amount_owed(line_items) - line_items.map(&:order).uniq.sum(&:outstanding_balance) + if OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, @user) + unique_orders(line_items).sum(&:new_outstanding_balance) + else + unique_orders(line_items).sum(&:outstanding_balance) + end end def customer_payments_amount_paid(line_items) - line_items.map(&:order).uniq.sum(&:payment_total) + unique_orders(line_items).sum(&:payment_total) + end + + def unique_orders(line_items) + line_items.map(&:order).uniq end def empty_cell(_line_items) diff --git a/engines/order_management/spec/services/order_management/order/updater_spec.rb b/engines/order_management/spec/services/order_management/order/updater_spec.rb index f0df8269af..47a92a520c 100644 --- a/engines/order_management/spec/services/order_management/order/updater_spec.rb +++ b/engines/order_management/spec/services/order_management/order/updater_spec.rb @@ -141,114 +141,119 @@ module OrderManagement updater.update end - it "is failed if no valid payments" do - allow(order).to receive_message_chain(:payments, :valid, :empty?).and_return(true) + describe "#update_payment_state" do + context "when the order has no valid payments" do + it "is failed" do + allow(order).to receive_message_chain(:payments, :valid, :empty?).and_return(true) - updater.update_payment_state - expect(order.payment_state).to eq('failed') - end - - context "payment total is greater than order total" do - it "is credit_owed" do - order.payment_total = 2 - order.total = 1 - - expect { updater.update_payment_state - }.to change { order.payment_state }.to 'credit_owed' - end - end - - context "order total is greater than payment total" do - it "is credit_owed" do - order.payment_total = 1 - order.total = 2 - - expect { - updater.update_payment_state - }.to change { order.payment_state }.to 'balance_due' - end - end - - context "order total equals payment total" do - it "is paid" do - order.payment_total = 30 - order.total = 30 - - expect { - updater.update_payment_state - }.to change { order.payment_state }.to 'paid' - end - end - - context "order is canceled" do - before do - order.state = 'canceled' - end - - context "and is still unpaid" do - it "is void" do - order.payment_total = 0 - order.total = 30 - expect { - updater.update_payment_state - }.to change { order.payment_state }.to 'void' + expect(order.payment_state).to eq('failed') end end - context "and is paid" do + context "payment total is greater than order total" do it "is credit_owed" do - order.payment_total = 30 - order.total = 30 - allow(order).to receive_message_chain(:payments, :valid, :empty?).and_return(false) - allow(order).to receive_message_chain(:payments, :completed, :empty?).and_return(false) + order.payment_total = 2 + order.total = 1 + expect { updater.update_payment_state }.to change { order.payment_state }.to 'credit_owed' end end - context "and payment is refunded" do - it "is void" do - order.payment_total = 0 - order.total = 30 - allow(order).to receive_message_chain(:payments, :valid, :empty?).and_return(false) - allow(order).to receive_message_chain(:payments, :completed, :empty?).and_return(false) + context "order total is greater than payment total" do + it "is credit_owed" do + order.payment_total = 1 + order.total = 2 + expect { updater.update_payment_state - }.to change { order.payment_state }.to 'void' - end - end - end - - context 'when the set payment_state does not match the last payment_state' do - before { order.payment_state = 'previous_to_paid' } - - context 'and the order is being updated' do - before { allow(order).to receive(:persisted?) { true } } - - it 'creates a new state_change for the order' do - expect { updater.update_payment_state } - .to change { order.state_changes.size }.by(1) + }.to change { order.payment_state }.to 'balance_due' end end - context 'and the order is being created' do - before { allow(order).to receive(:persisted?) { false } } + context "order total equals payment total" do + it "is paid" do + order.payment_total = 30 + order.total = 30 - it 'creates a new state_change for the order' do + expect { + updater.update_payment_state + }.to change { order.payment_state }.to 'paid' + end + end + + context "order is canceled" do + before { order.state = 'canceled' } + + context "and is still unpaid" do + it "is void" do + order.payment_total = 0 + order.total = 30 + + expect { + updater.update_payment_state + }.to change { order.payment_state }.to 'void' + end + end + + context "and is paid" do + it "is credit_owed" do + order.payment_total = 30 + order.total = 30 + allow(order).to receive_message_chain(:payments, :valid, :empty?) { false } + allow(order).to receive_message_chain(:payments, :completed, :empty?) { false } + + expect { + updater.update_payment_state + }.to change { order.payment_state }.to 'credit_owed' + end + end + + context "and payment is refunded" do + it "is void" do + order.payment_total = 0 + order.total = 30 + allow(order).to receive_message_chain(:payments, :valid, :empty?) { false } + allow(order).to receive_message_chain(:payments, :completed, :empty?) { false } + + expect { + updater.update_payment_state + }.to change { order.payment_state }.to 'void' + end + end + end + + context 'when the set payment_state matches the last payment_state' do + before { order.payment_state = 'paid' } + + it 'does not create any state_change' do expect { updater.update_payment_state } .not_to change { order.state_changes.size } end end - end - context 'when the set payment_state matches the last payment_state' do - before { order.payment_state = 'paid' } + context 'when the set payment_state does not match the last payment_state' do + before { order.payment_state = 'previous_to_paid' } - it 'does not create any state_change' do - expect { updater.update_payment_state } - .not_to change { order.state_changes.size } + context 'and the order is being updated' do + before { allow(order).to receive(:persisted?) { true } } + + it 'creates a new state_change for the order' do + expect { updater.update_payment_state } + .to change { order.state_changes.size }.by(1) + end + end + + context 'and the order is being created' do + before { allow(order).to receive(:persisted?) { false } } + + it 'creates a new state_change for the order' do + expect { updater.update_payment_state } + .not_to change { order.state_changes.size } + end + end end end diff --git a/engines/order_management/spec/services/order_management/reports/bulk_coop/bulk_coop_report_spec.rb b/engines/order_management/spec/services/order_management/reports/bulk_coop/bulk_coop_report_spec.rb index 5a580481de..f667c8fdc6 100644 --- a/engines/order_management/spec/services/order_management/reports/bulk_coop/bulk_coop_report_spec.rb +++ b/engines/order_management/spec/services/order_management/reports/bulk_coop/bulk_coop_report_spec.rb @@ -3,7 +3,12 @@ require 'spec_helper' describe OrderManagement::Reports::BulkCoop::BulkCoopReport do - describe "fetching orders" do + subject { OrderManagement::Reports::BulkCoop::BulkCoopReport.new user, params, true } + let(:user) { create(:admin_user) } + + describe '#table_items' do + let(:params) { {} } + let(:d1) { create(:distributor_enterprise) } let(:oc1) { create(:simple_order_cycle) } let(:o1) { create(:order, completed_at: 1.day.ago, order_cycle: oc1, distributor: d1) } @@ -12,19 +17,38 @@ describe OrderManagement::Reports::BulkCoop::BulkCoopReport do before { o1.line_items << li1 } context "as a site admin" do - let(:user) { create(:admin_user) } - subject { OrderManagement::Reports::BulkCoop::BulkCoopReport.new user, {}, true } + context 'when searching' do + let(:params) { { q: { completed_at_gt: '', completed_at_lt: '', distributor_id_in: [] } } } - it "fetches completed orders" do - o2 = create(:order) - o2.line_items << build(:line_item) - expect(subject.table_items).to eq([li1]) + it "fetches completed orders" do + o2 = create(:order, state: 'cart') + o2.line_items << build(:line_item) + expect(subject.table_items).to eq([li1]) + end + + it 'shows canceled orders' do + o2 = create(:order, state: 'canceled', completed_at: 1.day.ago, order_cycle: oc1, distributor: d1) + line_item = build(:line_item_with_shipment) + o2.line_items << line_item + expect(subject.table_items).to include(line_item) + end end - it "does not show cancelled orders" do - o2 = create(:order, state: "canceled", completed_at: 1.day.ago) - o2.line_items << build(:line_item_with_shipment) - expect(subject.table_items).to eq([li1]) + context 'when not searching' do + let(:params) { {} } + + it "fetches completed orders" do + o2 = create(:order, state: 'cart') + o2.line_items << build(:line_item) + expect(subject.table_items).to eq([li1]) + end + + it 'shows canceled orders' do + o2 = create(:order, state: 'canceled', completed_at: 1.day.ago, order_cycle: oc1, distributor: d1) + line_item = build(:line_item_with_shipment) + o2.line_items << line_item + expect(subject.table_items).to include(line_item) + end end end @@ -124,4 +148,56 @@ describe OrderManagement::Reports::BulkCoop::BulkCoopReport do end end end + + describe '#columns' do + context 'when report type is bulk_coop_customer_payments' do + let(:params) { { report_type: 'bulk_coop_customer_payments' } } + + it 'returns' do + expect(subject.columns).to eq( + [ + :order_billing_address_name, + :order_completed_at, + :customer_payments_total_cost, + :customer_payments_amount_owed, + :customer_payments_amount_paid, + ] + ) + end + end + end + + # Yes, I know testing a private method is bad practice but report's design, tighly coupling + # OpenFoodNetwork::OrderGrouper and OrderManagement::Reports::BulkCoop::BulkCoopReport, makes it + # very hard to make things testeable without ending up in a wormwhole. This is a trade-off. + describe '#customer_payments_amount_owed' do + let(:params) { {} } + let(:user) { build(:user) } + let!(:line_item) { create(:line_item) } + let(:order) { line_item.order } + + context 'when the customer_balance feature is enabled' do + before do + allow(OpenFoodNetwork::FeatureToggle) + .to receive(:enabled?).with(:customer_balance, user) { true } + end + + it 'calls #new_outstanding_balance' do + expect_any_instance_of(Spree::Order).to receive(:new_outstanding_balance) + subject.send(:customer_payments_amount_owed, [line_item]) + end + end + + context 'when the customer_balance feature is disabled' do + before do + allow(OpenFoodNetwork::FeatureToggle) + .to receive(:enabled?).with(:customer_balance, user) { false } + end + + it 'calls #outstanding_balance' do + expect_any_instance_of(Spree::Order).to receive(:outstanding_balance) + subject.send(:customer_payments_amount_owed, [line_item]) + end + end + end end diff --git a/engines/web/spec/features/consumer/cookies_spec.rb b/engines/web/spec/features/consumer/cookies_spec.rb index 5b55ece1fb..e447364fe2 100644 --- a/engines/web/spec/features/consumer/cookies_spec.rb +++ b/engines/web/spec/features/consumer/cookies_spec.rb @@ -155,7 +155,7 @@ feature "Cookies", js: true do end def click_footer_cookies_policy_link_and_wait - find("div > a", text: "cookies policy").click + find(".legal a", text: "cookies policy").click sleep 2 end diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb index 1858469c6e..2e562c876a 100644 --- a/lib/open_food_network/order_cycle_management_report.rb +++ b/lib/open_food_network/order_cycle_management_report.rb @@ -49,7 +49,7 @@ module OpenFoodNetwork if FeatureToggle.enabled?(:customer_balance, @user) Spree::Order. finalized. - where.not(spree_orders: { state: :canceled }). + not_state(:canceled). distributed_by_user(@user). managed_by(@user). search(params[:q]) diff --git a/lib/open_food_network/reports/line_items.rb b/lib/open_food_network/reports/line_items.rb index 7476dc85bf..88841001dd 100644 --- a/lib/open_food_network/reports/line_items.rb +++ b/lib/open_food_network/reports/line_items.rb @@ -2,9 +2,11 @@ module OpenFoodNetwork module Reports # shared code to search and list line items class LineItems - def initialize(order_permissions, params) + def initialize(order_permissions, params, orders_relation = nil) @order_permissions = order_permissions @params = params + complete_not_canceled_visible_orders = CompleteVisibleOrders.new(order_permissions).query.not_state(:canceled) + @orders_relation = orders_relation || complete_not_canceled_visible_orders end def orders @@ -12,7 +14,7 @@ module OpenFoodNetwork end def list(line_item_includes = nil) - line_items = @order_permissions.visible_line_items.in_orders(orders.result) + line_items = order_permissions.visible_line_items.in_orders(orders.result) if @params[:supplier_id_in].present? line_items = line_items.supplied_by_any(@params[:supplier_id_in]) @@ -22,11 +24,9 @@ module OpenFoodNetwork line_items = line_items.includes(*line_item_includes).references(:line_items) end - editable_line_items = editable_line_items(line_items) + without_editable_line_items = line_items - editable_line_items(line_items) - line_items.reject{ |li| - editable_line_items.include? li - }.each do |line_item| + without_editable_line_items.each do |line_item| OrderDataMasker.new(line_item.order).call end @@ -35,13 +35,15 @@ module OpenFoodNetwork private + attr_reader :orders_relation, :order_permissions + def search_orders - @order_permissions.visible_orders.complete.not_state(:canceled).search(@params[:q]) + orders_relation.search(@params[:q]) end # From the line_items given, returns the ones that are editable by the user def editable_line_items(line_items) - editable_line_items_ids = @order_permissions.editable_line_items.select(:id) + editable_line_items_ids = order_permissions.editable_line_items.select(:id) # Although merge could take a relation, here we convert line_items to array # because, if we pass a relation, merge will overwrite the conditions on the same field diff --git a/lib/open_food_network/sales_tax_report.rb b/lib/open_food_network/sales_tax_report.rb index 8abce8c0e4..83bfadd8b6 100644 --- a/lib/open_food_network/sales_tax_report.rb +++ b/lib/open_food_network/sales_tax_report.rb @@ -100,7 +100,7 @@ module OpenFoodNetwork end def tax_included_in(line_item) - line_item.adjustments.sum(:included_tax) + line_item.adjustments.tax.inclusive.sum(:amount) end def shipment_inc_vat diff --git a/lib/spree/core/controller_helpers/auth.rb b/lib/spree/core/controller_helpers/auth.rb index c6d05deaeb..fc2c1201e3 100644 --- a/lib/spree/core/controller_helpers/auth.rb +++ b/lib/spree/core/controller_helpers/auth.rb @@ -7,7 +7,7 @@ module Spree extend ActiveSupport::Concern included do - before_filter :ensure_api_key + before_action :ensure_api_key rescue_from CanCan::AccessDenied do unauthorized diff --git a/lib/spree/core/controller_helpers/common.rb b/lib/spree/core/controller_helpers/common.rb index 2854d8363b..78625c0bd8 100644 --- a/lib/spree/core/controller_helpers/common.rb +++ b/lib/spree/core/controller_helpers/common.rb @@ -12,7 +12,7 @@ module Spree layout :get_layout - before_filter :set_user_language + before_action :set_user_language protected diff --git a/lib/spree/core/controller_helpers/order.rb b/lib/spree/core/controller_helpers/order.rb index 460bd8e15f..d3f093cfd2 100644 --- a/lib/spree/core/controller_helpers/order.rb +++ b/lib/spree/core/controller_helpers/order.rb @@ -10,7 +10,7 @@ module Spree base.class_eval do helper_method :current_order helper_method :current_currency - before_filter :set_current_order + before_action :set_current_order end end diff --git a/lib/spree/core/controller_helpers/ssl.rb b/lib/spree/core/controller_helpers/ssl.rb index 6c923e4977..07a9ad5ccd 100644 --- a/lib/spree/core/controller_helpers/ssl.rb +++ b/lib/spree/core/controller_helpers/ssl.rb @@ -7,7 +7,7 @@ module Spree extend ActiveSupport::Concern included do - before_filter :force_non_ssl_redirect, if: proc { Spree::Config[:redirect_https_to_http] } + before_action :force_non_ssl_redirect, if: proc { Spree::Config[:redirect_https_to_http] } def self.ssl_allowed(*actions) class_attribute :ssl_allowed_actions diff --git a/spec/controllers/line_items_controller_spec.rb b/spec/controllers/line_items_controller_spec.rb index fbe0d74b7a..ce91ad74df 100644 --- a/spec/controllers/line_items_controller_spec.rb +++ b/spec/controllers/line_items_controller_spec.rb @@ -84,6 +84,18 @@ describe LineItemsController, type: :controller do expect(response.status).to eq 204 expect { item.reload }.to raise_error ActiveRecord::RecordNotFound end + + context "after a payment is captured" do + let(:payment) { create(:check_payment, amount: order.total, order: order, state: 'completed') } + before { payment.capture! } + + it 'updates the payment state' do + expect(order.payment_state).to eq 'paid' + delete :destroy, params + order.reload + expect(order.payment_state).to eq 'credit_owed' + end + end end end end diff --git a/spec/controllers/spree/admin/base_controller_spec.rb b/spec/controllers/spree/admin/base_controller_spec.rb index 07e3b90f0a..4bcc4b64db 100644 --- a/spec/controllers/spree/admin/base_controller_spec.rb +++ b/spec/controllers/spree/admin/base_controller_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe Spree::Admin::BaseController, type: :controller do controller(Spree::Admin::BaseController) do def index - before_filter :unauthorized + before_action :unauthorized render plain: "" end end diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb index cc743c1f4b..79479bf284 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb @@ -297,6 +297,17 @@ describe Spree::Admin::PaymentsController, type: :controller do spree_get :index, order_id: order.number expect(response.status).to eq 200 end + + context "order is then resumed" do + before do + order.resume + end + + it "still renders the payments tab" do + spree_get :index, order_id: order.number + expect(response.status).to eq 200 + end + end end end end diff --git a/spec/features/admin/enterprises/terms_and_conditions_spec.rb b/spec/features/admin/enterprises/terms_and_conditions_spec.rb index 311cf6e9cf..d465a7e651 100644 --- a/spec/features/admin/enterprises/terms_and_conditions_spec.rb +++ b/spec/features/admin/enterprises/terms_and_conditions_spec.rb @@ -54,7 +54,7 @@ feature "Uploading Terms and Conditions PDF" do go_to_business_details expect(page).to have_selector "a[href*='logo-white.pdf'][target=\"_blank\"]" - expect(page).to have_content time.strftime("%F %T %z") + expect(page).to have_content time.strftime("%F %T") # Replace PDF attach_file "enterprise[terms_and_conditions]", black_pdf_file_name diff --git a/spec/features/consumer/shopping/embedded_shopfronts_spec.rb b/spec/features/consumer/shopping/embedded_shopfronts_spec.rb index 2d2d3113f1..ff15d4a46d 100644 --- a/spec/features/consumer/shopping/embedded_shopfronts_spec.rb +++ b/spec/features/consumer/shopping/embedded_shopfronts_spec.rb @@ -90,7 +90,7 @@ feature "Using embedded shopfront functionality", js: true do end end - it "redirects to embedded hub on logout when embedded" do + xit "redirects to embedded hub on logout when embedded" do on_embedded_page do wait_for_cart find('#login-link a').click diff --git a/spec/lib/open_food_network/reports/line_items_spec.rb b/spec/lib/open_food_network/reports/line_items_spec.rb new file mode 100644 index 0000000000..63d9696f4b --- /dev/null +++ b/spec/lib/open_food_network/reports/line_items_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' +require 'open_food_network/reports/line_items' + +describe OpenFoodNetwork::Reports::LineItems do + subject(:reports_line_items) { described_class.new(order_permissions, params) } + + # This object lets us add some test coverage despite the very deep coupling between the class + # under test and the various objects it depends on. Other more common moking strategies where very + # hard. + class FakeOrderPermissions + def initialize(line_item, orders_relation) + @relation = Spree::LineItem.where(id: line_item.id) + @orders_relation = orders_relation + end + + def visible_line_items + relation + end + + def editable_line_items + line_item = FactoryBot.create(:line_item) + Spree::LineItem.where(id: line_item.id) + end + + def visible_orders + orders_relation + end + + private + + attr_reader :relation, :orders_relation + end + + describe '#list' do + let!(:order) do + create( + :order, + distributor: create(:enterprise), + completed_at: 1.day.ago, + shipments: [build(:shipment)] + ) + end + let!(:line_item) { create(:line_item, order: order) } + + let(:orders_relation) { Spree::Order.where(id: order.id) } + let(:order_permissions) { FakeOrderPermissions.new(line_item, orders_relation) } + let(:params) { {} } + + it 'returns masked data' do + line_items = reports_line_items.list + expect(line_items.first.order.email).to eq(I18n.t('admin.reports.hidden')) + end + end +end diff --git a/spec/mailers/order_mailer_spec.rb b/spec/mailers/order_mailer_spec.rb index fb3e22cd4b..d23b920c55 100644 --- a/spec/mailers/order_mailer_spec.rb +++ b/spec/mailers/order_mailer_spec.rb @@ -5,27 +5,77 @@ require 'spec_helper' describe Spree::OrderMailer do include OpenFoodNetwork::EmailHelper - context "basic behaviour" do + describe '#confirm_email_for_customer' do + subject(:email) { described_class.confirm_email_for_customer(order) } + let(:order) { build(:order_with_totals_and_distribution) } - context ":from not set explicitly" do - it "falls back to spree config" do - message = Spree::OrderMailer.confirm_email_for_customer(order) - expect(message.from).to eq [Spree::Config[:mails_from]] + it 'renders the shared/_payment.html.haml partial' do + expect(email.body).to include(I18n.t(:email_payment_summary)) + end + + context 'when the order has outstanding balance' do + before { allow(order).to receive(:outstanding_balance) { 123 } } + + it 'renders the amount as money' do + expect(email.body).to include('$123') end end - it "doesn't aggressively escape double quotes in confirmation body" do - confirmation_email = Spree::OrderMailer.confirm_email_for_customer(order) - expect(confirmation_email.body).to_not include(""") + context 'when the order has no outstanding balance' do + before { allow(order).to receive(:outstanding_balance) { 0 } } + + it 'displays the payment status' do + expect(email.body).to include(I18n.t(:email_payment_not_paid)) + end end - it "confirm_email_for_customer accepts an order id as an alternative to an Order object" do + context "when :from is not set explicitly" do + it "falls back to spree config" do + expect(email.from).to eq [Spree::Config[:mails_from]] + end + end + + it "doesn't aggressively escape double quotes body" do + expect(email.body).to_not include(""") + end + + it "accepts an order id as an alternative to an Order object" do expect(Spree::Order).to receive(:find).with(order.id).and_return(order) expect { - Spree::OrderMailer.confirm_email_for_customer(order.id).deliver_now + described_class.confirm_email_for_customer(order.id).deliver_now }.to_not raise_error end + end + + describe '#confirm_email_for_shop' do + subject(:email) { described_class.confirm_email_for_shop(order) } + + let(:order) { build(:order_with_totals_and_distribution) } + + it 'renders the shared/_payment.html.haml partial' do + expect(email.body).to include(I18n.t(:email_payment_summary)) + end + + context 'when the order has outstanding balance' do + before { allow(order).to receive(:outstanding_balance) { 123 } } + + it 'renders the amount as money' do + expect(email.body).to include('$123') + end + end + + context 'when the order has no outstanding balance' do + before { allow(order).to receive(:outstanding_balance) { 0 } } + + it 'displays the payment status' do + expect(email.body).to include(I18n.t(:email_payment_not_paid)) + end + end + end + + context "basic behaviour" do + let(:order) { build(:order_with_totals_and_distribution) } it "cancel_email accepts an order id as an alternative to an Order object" do expect(Spree::Order).to receive(:find).with(order.id).and_return(order) diff --git a/spec/mailers/subscription_mailer_spec.rb b/spec/mailers/subscription_mailer_spec.rb index f50f58b5e4..0cd77c047a 100644 --- a/spec/mailers/subscription_mailer_spec.rb +++ b/spec/mailers/subscription_mailer_spec.rb @@ -8,7 +8,10 @@ describe SubscriptionMailer, type: :mailer do before { setup_email } - describe "order placement" do + describe '#placement_email' do + subject(:email) { SubscriptionMailer.placement_email(order, changes) } + let(:changes) { {} } + let(:shop) { create(:enterprise) } let(:customer) { create(:customer, enterprise: shop) } let(:subscription) { create(:subscription, shop: shop, customer: customer, with_items: true) } @@ -16,31 +19,24 @@ describe SubscriptionMailer, type: :mailer do let!(:order) { proxy_order.initialise_order! } context "when changes have been made to the order" do - let(:changes) { {} } - - before do - changes[order.line_items.first.id] = 2 - expect do - SubscriptionMailer.placement_email(order, changes).deliver_now - end.to change{ SubscriptionMailer.deliveries.count }.by(1) - end + before { changes[order.line_items.first.id] = 2 } it "sends the email, which notifies the customer of changes made" do + expect { email.deliver_now }.to change { SubscriptionMailer.deliveries.count }.by(1) + body = SubscriptionMailer.deliveries.last.body.encoded + expect(body).to include "This order was automatically created for you." expect(body).to include "Unfortunately, not all products that you requested were available." end end context "and changes have not been made to the order" do - before do - expect do - SubscriptionMailer.placement_email(order, {}).deliver_now - end.to change{ SubscriptionMailer.deliveries.count }.by(1) - end - it "sends the email" do + expect { email.deliver_now }.to change { SubscriptionMailer.deliveries.count }.by(1) + body = SubscriptionMailer.deliveries.last.body.encoded + expect(body).to include "This order was automatically created for you." expect(body).to_not include "Unfortunately, not all products that you requested were available." end @@ -52,12 +48,9 @@ describe SubscriptionMailer, type: :mailer do let(:shop) { create(:enterprise, allow_order_changes: true) } - let(:email) { SubscriptionMailer.deliveries.last } - let(:body) { email.body.encoded } + let(:body) { SubscriptionMailer.deliveries.last.body.encoded } - before do - SubscriptionMailer.placement_email(order, {}).deliver_now - end + before { email.deliver_now } context "when the customer has a user account" do let(:customer) { create(:customer, enterprise: shop) } @@ -93,21 +86,36 @@ describe SubscriptionMailer, type: :mailer do end end end + + context 'when the order has outstanding balance' do + before { allow(order).to receive(:outstanding_balance) { 123 } } + + it 'renders the amount as money' do + expect(email.body).to include('$123') + end + end + + context 'when the order has no outstanding balance' do + before { allow(order).to receive(:outstanding_balance) { 0 } } + + it 'displays the payment status' do + expect(email.body).to include(I18n.t(:email_payment_not_paid)) + end + end end - describe "order confirmation" do + describe '#confirmation_email' do + subject(:email) { SubscriptionMailer.confirmation_email(order) } + let(:customer) { create(:customer) } let(:subscription) { create(:subscription, customer: customer, with_items: true) } let(:proxy_order) { create(:proxy_order, subscription: subscription) } let!(:order) { proxy_order.initialise_order! } - - before do - expect do - SubscriptionMailer.confirmation_email(order).deliver_now - end.to change{ SubscriptionMailer.deliveries.count }.by(1) - end + let(:user) { order.user } it "sends the email" do + expect { email.deliver_now }.to change{ SubscriptionMailer.deliveries.count }.by(1) + body = SubscriptionMailer.deliveries.last.body.encoded expect(body).to include "This order was automatically placed for you" end @@ -115,14 +123,11 @@ describe SubscriptionMailer, type: :mailer do describe "linking to order page" do let(:order_link_href) { "href=\"#{order_url(order)}\"" } - let(:email) { SubscriptionMailer.deliveries.last } - let(:body) { email.body.encoded } - context "when the customer has a user account" do let(:customer) { create(:customer) } it "provides link to view details" do - expect(body).to match /#{order_link_href}/ + expect(email.body.encoded).to include(order_url(order)) end end @@ -130,10 +135,26 @@ describe SubscriptionMailer, type: :mailer do let(:customer) { create(:customer, user: nil) } it "does not provide link" do - expect(body).to_not match /#{order_link_href}/ + expect(email.body).to_not match /#{order_link_href}/ end end end + + context 'when the order has outstanding balance' do + before { allow(order).to receive(:outstanding_balance) { 123 } } + + it 'renders the amount as money' do + expect(email.body).to include('$123') + end + end + + context 'when the order has no outstanding balance' do + before { allow(order).to receive(:outstanding_balance) { 0 } } + + it 'displays the payment status' do + expect(email.body).to include(I18n.t(:email_payment_not_paid)) + end + end end describe "empty order notification" do diff --git a/spec/models/concerns/balance_spec.rb b/spec/models/concerns/balance_spec.rb new file mode 100644 index 0000000000..331e521442 --- /dev/null +++ b/spec/models/concerns/balance_spec.rb @@ -0,0 +1,241 @@ +require 'spec_helper' + +describe Balance do + context "#new_outstanding_balance" do + context 'when orders are in cart state' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'cart') } + + it 'returns the order balance' do + expect(order.new_outstanding_balance).to eq(100 - 10) + end + end + + context 'when orders are in address state' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'address') } + + it 'returns the order balance' do + expect(order.new_outstanding_balance).to eq(100 - 10) + end + end + + context 'when orders are in delivery state' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'delivery') } + + it 'returns the order balance' do + expect(order.new_outstanding_balance).to eq(100 - 10) + end + end + + context 'when orders are in payment state' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'payment') } + + it 'returns the order balance' do + expect(order.new_outstanding_balance).to eq(100 - 10) + end + end + + context 'when no orders where paid' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') } + + it 'returns the customer balance' do + expect(order.new_outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order was paid' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') } + + it 'returns the customer balance' do + expect(order.new_outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order is canceled' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'canceled') } + + it 'returns the customer balance' do + expect(order.new_outstanding_balance).to eq(-10) + end + end + + context 'when an order is resumed' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'resumed') } + + it 'returns the customer balance' do + expect(order.new_outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order is in payment' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'payment') } + + it 'returns the customer balance' do + expect(order.new_outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order is awaiting_return' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'awaiting_return') } + + it 'returns the customer balance' do + expect(order.new_outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order is returned' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'returned') } + + it 'returns the balance' do + expect(order.new_outstanding_balance).to eq(-10) + end + end + + context 'when payment_total is less than total' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') } + + it "returns positive" do + expect(order.new_outstanding_balance).to eq(100 - 10) + end + end + + context 'when payment_total is greater than total' do + let(:order) { create(:order, total: 8.20, payment_total: 10.20, state: 'complete') } + + it "returns negative amount" do + expect(order.new_outstanding_balance).to eq(-2.00) + end + end + end + + context '#outstanding_balance?' do + context 'when total is greater than payment_total' do + let(:order) { build(:order, total: 10.10, payment_total: 9.50) } + + it 'returns true' do + expect(order.outstanding_balance?).to eq(true) + end + end + + context 'when total is less than payment_total' do + let(:order) { build(:order, total: 8.25, payment_total: 10.44) } + + it 'returns true' do + expect(order.outstanding_balance?).to eq(true) + end + end + + context "when total equals payment_total" do + let(:order) { build(:order, total: 10.10, payment_total: 10.10) } + + it 'returns false' do + expect(order.outstanding_balance?).to eq(false) + end + end + end + + context "#outstanding_balance" do + context 'when orders are in cart state' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'cart') } + + it 'returns the order balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when orders are in address state' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'address') } + + it 'returns the order balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when orders are in delivery state' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'delivery') } + + it 'returns the order balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when orders are in payment state' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'payment') } + + it 'returns the order balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when no orders where paid' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') } + + it 'returns the customer balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order was paid' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') } + + it 'returns the customer balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order is canceled' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'canceled') } + + it 'returns the customer balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order is resumed' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'resumed') } + + it 'returns the customer balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order is in payment' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'payment') } + + it 'returns the customer balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order is awaiting_return' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'awaiting_return') } + + it 'returns the customer balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when an order is returned' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'returned') } + + it 'returns the balance' do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when payment_total is less than total' do + let(:order) { build(:order, total: 100, payment_total: 10, state: 'complete') } + + it "returns positive" do + expect(order.outstanding_balance).to eq(100 - 10) + end + end + + context 'when payment_total is greater than total' do + let(:order) { create(:order, total: 8.20, payment_total: 10.20, state: 'complete') } + + it "returns negative amount" do + expect(order.outstanding_balance).to eq(-2.00) + end + end + end +end diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 670ae6170b..492f21fc60 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -599,4 +599,19 @@ describe Enterprise do end end end + + describe "#plus_relatives_and_oc_producers" do + it "does not find non-produders " do + supplier = create(:supplier_enterprise) + distributor = create(:distributor_enterprise, is_primary_producer: false) + product = create(:product) + order_cycle = create( + :simple_order_cycle, + suppliers: [supplier], + distributors: [distributor], + variants: [product.master] + ) + expect(distributor.plus_relatives_and_oc_producers(order_cycle)).to eq([supplier]) + end + end end diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index 1df48296ca..b46a431041 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -185,9 +185,9 @@ module Spree tax_rate.adjust(order) end - it "has 100% tax included" do + it "has tax included" do expect(adjustment.amount).to be > 0 - expect(adjustment.included_tax).to eq(adjustment.amount) + expect(adjustment.included).to be true end it "does not crash when order data has been updated previously" do @@ -365,7 +365,7 @@ module Spree # so tax on the enterprise_fee adjustment will be 0 # Tax on line item is: 0.2/1.2 x $10 = $1.67 expect(adjustment.included_tax).to eq(0.0) - expect(line_item.adjustments.first.included_tax).to eq(1.67) + expect(line_item.adjustments.tax.first.amount).to eq(1.67) end end @@ -395,7 +395,7 @@ module Spree # gives tax on fee of 0.2/1.2 x $50 = $8.33 # Tax on line item is: 0.2/1.2 x $10 = $1.67 expect(adjustment.included_tax).to eq(8.33) - expect(line_item.adjustments.first.included_tax).to eq(1.67) + expect(line_item.adjustments.tax.first.amount).to eq(1.67) end end diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 0ac72a7f4b..0c0277f00a 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -443,7 +443,10 @@ module Spree let(:li_no_tax) { create(:line_item) } let(:li_tax) { create(:line_item) } let(:tax_rate) { create(:tax_rate, calculator: ::Calculator::DefaultTax.new) } - let!(:adjustment) { create(:adjustment, adjustable: li_tax, originator: tax_rate, label: "TR", amount: 123, included_tax: 10.00) } + let!(:adjustment) { + create(:adjustment, adjustable: li_tax, originator: tax_rate, label: "TR", + amount: 10.00, included: true) + } context "checking if a line item has tax included" do it "returns true when it does" do diff --git a/spec/models/spree/order/payment_spec.rb b/spec/models/spree/order/payment_spec.rb index 44e434bd5e..224f0be0dc 100644 --- a/spec/models/spree/order/payment_spec.rb +++ b/spec/models/spree/order/payment_spec.rb @@ -9,9 +9,6 @@ module Spree let(:bogus) { create(:bogus_payment_method, distributors: [create(:enterprise)]) } before do - # So that Payment#purchase! is called during processing - Spree::Config[:auto_capture] = true - allow(order).to receive_message_chain(:line_items, :empty?).and_return(false) allow(order).to receive_messages total: 100 end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 9ad47c5918..6dbd4764c4 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -234,37 +234,6 @@ describe Spree::Order do end end - context "#outstanding_balance" do - it "should return positive amount when payment_total is less than total" do - order.payment_total = 20.20 - order.total = 30.30 - expect(order.outstanding_balance).to eq 10.10 - end - it "should return negative amount when payment_total is greater than total" do - order.total = 8.20 - order.payment_total = 10.20 - expect(order.outstanding_balance).to be_within(0.001).of(-2.00) - end - end - - context "#outstanding_balance?" do - it "should be true when total greater than payment_total" do - order.total = 10.10 - order.payment_total = 9.50 - expect(order.outstanding_balance?).to be_truthy - end - it "should be true when total less than payment_total" do - order.total = 8.25 - order.payment_total = 10.44 - expect(order.outstanding_balance?).to be_truthy - end - it "should be false when total equals payment_total" do - order.total = 10.10 - order.payment_total = 10.10 - expect(order.outstanding_balance?).to be_falsy - end - end - context "#completed?" do it "should indicate if order is completed" do order.completed_at = nil diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 9a0ab803d0..64d9c04afe 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -81,20 +81,12 @@ describe Spree::Payment do end context "#process!" do - it "should purchase if with auto_capture" do + it "should call purchase!" do payment = build_stubbed(:payment, payment_method: gateway) - expect(payment.payment_method).to receive(:auto_capture?).and_return(true) expect(payment).to receive(:purchase!) payment.process! end - it "should authorize without auto_capture" do - payment = build_stubbed(:payment, payment_method: gateway) - expect(payment.payment_method).to receive(:auto_capture?).and_return(false) - expect(payment).to receive(:authorize!) - payment.process! - end - it "should make the state 'processing'" do expect(payment).to receive(:started_processing!) payment.process! diff --git a/spec/queries/complete_visible_orders_spec.rb b/spec/queries/complete_visible_orders_spec.rb new file mode 100644 index 0000000000..82f66093ff --- /dev/null +++ b/spec/queries/complete_visible_orders_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe CompleteVisibleOrders do + subject(:complete_visible_orders) { described_class.new(order_permissions) } + let(:filter_canceled) { false } + + describe '#query' do + let(:user) { create(:user) } + let(:enterprise) { create(:enterprise) } + let(:order_permissions) { ::Permissions::Order.new(user, filter_canceled) } + + before do + user.enterprises << enterprise + user.save! + end + + context 'when an order has no completed_at' do + let(:cart_order) { create(:order, distributor: enterprise) } + + it 'does not return it' do + expect(complete_visible_orders.query).not_to include(cart_order) + end + end + + context 'when an order has complete_at' do + let(:complete_order) { create(:order, completed_at: 1.day.ago, distributor: enterprise) } + + it 'does not return it' do + expect(complete_visible_orders.query).to include(complete_order) + end + end + + it 'calls #visible_orders' do + expect(order_permissions).to receive(:visible_orders).and_call_original + complete_visible_orders.query + end + end +end diff --git a/spec/services/invoice_renderer_spec.rb b/spec/services/invoice_renderer_spec.rb index 87e654eb3f..d78f5e97a8 100644 --- a/spec/services/invoice_renderer_spec.rb +++ b/spec/services/invoice_renderer_spec.rb @@ -4,19 +4,46 @@ require 'spec_helper' describe InvoiceRenderer do let(:service) { described_class.new } - - it "creates a PDF invoice with two different templates" do + let(:order) do order = create(:completed_order_with_fees) order.bill_address = order.ship_address order.save! + order + end - result = service.render_to_string(order) - expect(result).to match /^%PDF/ + context 'when invoice_style2 is configured' do + before { allow(Spree::Config).to receive(:invoice_style2?).and_return(true) } - allow(Spree::Config).to receive(:invoice_style2?).and_return true + it 'uses the invoice2 template' do + renderer = instance_double(ApplicationController) + expect(renderer) + .to receive(:render_to_string) + .with(include(template: 'spree/admin/orders/invoice2')) - alternative = service.render_to_string(order) - expect(alternative).to match /^%PDF/ - expect(alternative).to_not eq result + described_class.new(renderer).render_to_string(order) + end + + it 'creates a PDF invoice' do + result = service.render_to_string(order) + expect(result).to match /^%PDF/ + end + end + + context 'when invoice_style2 is not configured' do + before { allow(Spree::Config).to receive(:invoice_style2?).and_return(false) } + + it 'uses the invoice template' do + renderer = instance_double(ApplicationController) + expect(renderer) + .to receive(:render_to_string) + .with(include(template: 'spree/admin/orders/invoice')) + + described_class.new(renderer).render_to_string(order) + end + + it 'creates a PDF invoice' do + result = service.render_to_string(order) + expect(result).to match /^%PDF/ + end end end diff --git a/spec/services/order_data_masker_spec.rb b/spec/services/order_data_masker_spec.rb new file mode 100644 index 0000000000..70aeab7bd9 --- /dev/null +++ b/spec/services/order_data_masker_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +describe OrderDataMasker do + describe '#call' do + let(:distributor) { create(:enterprise) } + let(:order) { create(:order, distributor: distributor, ship_address: create(:address)) } + + context 'when displaying customer names is allowed' do + before { distributor.preferences[:show_customer_names_to_suppliers] = true } + + it 'masks personal addresses and email' do + described_class.new(order).call + + expect(order.bill_address.attributes).to include( + 'phone' => '', + 'address1' => '', + 'address2' => '', + 'city' => '', + 'zipcode' => '', + 'state_id' => nil + ) + + expect(order.ship_address.attributes).to include( + 'phone' => '', + 'address1' => '', + 'address2' => '', + 'city' => '', + 'zipcode' => '', + 'state_id' => nil + ) + + expect(order.email).to eq(I18n.t('admin.reports.hidden')) + end + + it 'does not mask the full name' do + described_class.new(order).call + + expect(order.bill_address.attributes).not_to include( + firstname: I18n.t('admin.reports.hidden'), + lastname: '' + ) + expect(order.ship_address.attributes).not_to include( + firstname: I18n.t('admin.reports.hidden'), + lastname: '' + ) + end + end + + context 'when displaying customer names is not allowed' do + before { distributor.preferences[:show_customer_names_to_suppliers] = false } + + it 'masks personal addresses and email' do + described_class.new(order).call + + expect(order.bill_address.attributes).to include( + 'phone' => '', + 'address1' => '', + 'address2' => '', + 'city' => '', + 'zipcode' => '', + 'state_id' => nil + ) + + expect(order.ship_address.attributes).to include( + 'phone' => '', + 'address1' => '', + 'address2' => '', + 'city' => '', + 'zipcode' => '', + 'state_id' => nil + ) + + expect(order.email).to eq(I18n.t('admin.reports.hidden')) + end + + it 'masks the full name' do + described_class.new(order).call + + expect(order.bill_address.attributes).to include( + 'firstname' => I18n.t('admin.reports.hidden'), + 'lastname' => '' + ) + expect(order.ship_address.attributes).to include( + 'firstname' => I18n.t('admin.reports.hidden'), + 'lastname' => '' + ) + end + end + end +end diff --git a/spec/services/order_tax_adjustments_fetcher_spec.rb b/spec/services/order_tax_adjustments_fetcher_spec.rb index de7f788230..f5edd0d848 100644 --- a/spec/services/order_tax_adjustments_fetcher_spec.rb +++ b/spec/services/order_tax_adjustments_fetcher_spec.rb @@ -44,8 +44,9 @@ describe OrderTaxAdjustmentsFetcher do tax_category: tax_category20, calculator: Calculator::FlatRate.new(preferred_amount: 48.0)) end - let(:additional_adjustment) do - create(:adjustment, amount: 50.0, included_tax: tax_rate25.compute_tax(50.0)) + let(:admin_adjustment) do + create(:adjustment, order: order, amount: 50.0, included_tax: tax_rate25.compute_tax(50.0), + source: nil, label: "Admin Adjustment") end let(:order_cycle) do @@ -62,8 +63,7 @@ describe OrderTaxAdjustmentsFetcher do line_items: [line_item1, line_item2], bill_address: create(:address), order_cycle: order_cycle, - distributor: coordinator, - adjustments: [additional_adjustment] + distributor: coordinator ) end @@ -80,6 +80,8 @@ describe OrderTaxAdjustmentsFetcher do end before do + order.reload + order.adjustments << admin_adjustment order.create_tax_charge! order.recreate_all_fees! end @@ -102,7 +104,7 @@ describe OrderTaxAdjustmentsFetcher do expect(subject[tax_rate20]).to eq(8.0) end - it "contains tax on order adjustment" do + it "contains tax on admin adjustment" do expect(subject[tax_rate25]).to eq(10.0) end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ab8a9802e4..c4280c90e4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -178,7 +178,6 @@ RSpec.configure do |config| spree_config.checkout_zone = checkout_zone spree_config.currency = currency spree_config.shipping_instructions = true - spree_config.auto_capture = true end end