From 47d2f698ef273e8978bfa2b43bc06d31319cdbc7 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 14:44:30 +0100 Subject: [PATCH 01/21] Bring models related to Order from spree_core EPIC COMMIT ALERT :-) --- app/models/spree/inventory_unit.rb | 70 ++ app/models/spree/line_item.rb | 102 +++ app/models/spree/order.rb | 575 ++++++++++++++++ app/models/spree/order_contents.rb | 67 ++ app/models/spree/order_inventory.rb | 106 +++ app/models/spree/return_authorization.rb | 103 +++ app/models/spree/state_change.rb | 15 + app/models/spree/tokenized_permission.rb | 6 + spec/models/spree/inventory_unit_spec.rb | 85 +++ spec/models/spree/line_item_spec.rb | 137 ++++ spec/models/spree/order/address_spec.rb | 50 ++ spec/models/spree/order/adjustments_spec.rb | 148 +++++ spec/models/spree/order/callbacks_spec.rb | 42 ++ spec/models/spree/order/checkout_spec.rb | 4 +- spec/models/spree/order/payment_spec.rb | 54 ++ spec/models/spree/order/state_machine_spec.rb | 183 ++++++ spec/models/spree/order/tax_spec.rb | 115 ++++ spec/models/spree/order/updating_spec.rb | 18 + spec/models/spree/order_contents_spec.rb | 92 +++ spec/models/spree/order_inventory_spec.rb | 174 +++++ spec/models/spree/order_spec.rb | 611 ++++++++++++++++++ .../models/spree/return_authorization_spec.rb | 138 ++++ 22 files changed, 2893 insertions(+), 2 deletions(-) create mode 100644 app/models/spree/inventory_unit.rb create mode 100644 app/models/spree/line_item.rb create mode 100644 app/models/spree/order.rb create mode 100644 app/models/spree/order_contents.rb create mode 100644 app/models/spree/order_inventory.rb create mode 100644 app/models/spree/return_authorization.rb create mode 100644 app/models/spree/state_change.rb create mode 100644 app/models/spree/tokenized_permission.rb create mode 100644 spec/models/spree/inventory_unit_spec.rb create mode 100644 spec/models/spree/order/address_spec.rb create mode 100644 spec/models/spree/order/adjustments_spec.rb create mode 100644 spec/models/spree/order/callbacks_spec.rb create mode 100644 spec/models/spree/order/payment_spec.rb create mode 100644 spec/models/spree/order/state_machine_spec.rb create mode 100644 spec/models/spree/order/tax_spec.rb create mode 100644 spec/models/spree/order/updating_spec.rb create mode 100644 spec/models/spree/order_contents_spec.rb create mode 100644 spec/models/spree/order_inventory_spec.rb create mode 100644 spec/models/spree/return_authorization_spec.rb diff --git a/app/models/spree/inventory_unit.rb b/app/models/spree/inventory_unit.rb new file mode 100644 index 00000000000..7380bc40ece --- /dev/null +++ b/app/models/spree/inventory_unit.rb @@ -0,0 +1,70 @@ +module Spree + class InventoryUnit < ActiveRecord::Base + belongs_to :variant, class_name: "Spree::Variant" + belongs_to :order, class_name: "Spree::Order" + belongs_to :shipment, class_name: "Spree::Shipment" + belongs_to :return_authorization, class_name: "Spree::ReturnAuthorization" + + scope :backordered, -> { where state: 'backordered' } + scope :shipped, -> { where state: 'shipped' } + scope :backordered_per_variant, ->(stock_item) do + includes(:shipment) + .where("spree_shipments.state != 'canceled'").references(:shipment) + .where(variant_id: stock_item.variant_id) + .backordered.order("#{self.table_name}.created_at ASC") + end + + # state machine (see http://github.com/pluginaweek/state_machine/tree/master for details) + state_machine initial: :on_hand do + event :fill_backorder do + transition to: :on_hand, from: :backordered + end + after_transition on: :fill_backorder, do: :update_order + + event :ship do + transition to: :shipped, if: :allow_ship? + end + + event :return do + transition to: :returned, from: :shipped + end + end + + # This was refactored from a simpler query because the previous implementation + # lead to issues once users tried to modify the objects returned. That's due + # to ActiveRecord `joins(shipment: :stock_location)` only return readonly + # objects + # + # Returns an array of backordered inventory units as per a given stock item + def self.backordered_for_stock_item(stock_item) + backordered_per_variant(stock_item).select do |unit| + unit.shipment.stock_location == stock_item.stock_location + end + end + + def self.finalize_units!(inventory_units) + inventory_units.map { |iu| iu.update_column(:pending, false) } + end + + def find_stock_item + Spree::StockItem.where(stock_location_id: shipment.stock_location_id, + variant_id: variant_id).first + end + + # Remove variant default_scope `deleted_at: nil` + def variant + Spree::Variant.unscoped { super } + end + + private + + def allow_ship? + Spree::Config[:allow_backorder_shipping] || self.on_hand? + end + + def update_order + order.update! + end + end +end + diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb new file mode 100644 index 00000000000..2e160ba018e --- /dev/null +++ b/app/models/spree/line_item.rb @@ -0,0 +1,102 @@ +module Spree + class LineItem < ActiveRecord::Base + before_validation :adjust_quantity + belongs_to :order, class_name: "Spree::Order" + belongs_to :variant, class_name: "Spree::Variant" + belongs_to :tax_category, class_name: "Spree::TaxCategory" + + has_one :product, through: :variant + has_many :adjustments, as: :adjustable, dependent: :destroy + + before_validation :copy_price + before_validation :copy_tax_category + + validates :variant, presence: true + validates :quantity, numericality: { + only_integer: true, + greater_than: -1, + message: Spree.t('validation.must_be_int') + } + validates :price, numericality: true + validates_with Stock::AvailabilityValidator + + before_save :update_inventory + + after_save :update_order + after_destroy :update_order + + attr_accessor :target_shipment + + def copy_price + if variant + self.price = variant.price if price.nil? + self.cost_price = variant.cost_price if cost_price.nil? + self.currency = variant.currency if currency.nil? + end + end + + def copy_tax_category + if variant + self.tax_category = variant.product.tax_category + end + end + + def amount + price * quantity + end + alias total amount + + def single_money + Spree::Money.new(price, { currency: currency }) + end + alias single_display_amount single_money + + def money + Spree::Money.new(amount, { currency: currency }) + end + alias display_total money + alias display_amount money + + def adjust_quantity + self.quantity = 0 if quantity.nil? || quantity < 0 + end + + def sufficient_stock? + Stock::Quantifier.new(variant_id).can_supply? quantity + end + + def insufficient_stock? + !sufficient_stock? + end + + def assign_stock_changes_to=(shipment) + @preferred_shipment = shipment + end + + # Remove product default_scope `deleted_at: nil` + def product + variant.product + end + + # Remove variant default_scope `deleted_at: nil` + def variant + Spree::Variant.unscoped { super } + end + + private + def update_inventory + if changed? + Spree::OrderInventory.new(self.order).verify(self, target_shipment) + end + end + + def update_order + if changed? || destroyed? + # update the order totals, etc. + order.create_tax_charge! + order.update! + end + end + end +end + diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb new file mode 100644 index 00000000000..b1056f3479f --- /dev/null +++ b/app/models/spree/order.rb @@ -0,0 +1,575 @@ +require 'spree/core/validators/email' +require 'spree/order/checkout' + +module Spree + class Order < ActiveRecord::Base + include Checkout + + checkout_flow do + go_to_state :address + go_to_state :delivery + go_to_state :payment, if: ->(order) { + order.update_totals + order.payment_required? + } + go_to_state :confirm, if: ->(order) { order.confirmation_required? } + go_to_state :complete + remove_transition from: :delivery, to: :confirm + end + + token_resource + + attr_reader :coupon_code + + if Spree.user_class + belongs_to :user, class_name: Spree.user_class.to_s + belongs_to :created_by, class_name: Spree.user_class.to_s + else + belongs_to :user + belongs_to :created_by + end + + belongs_to :bill_address, foreign_key: :bill_address_id, class_name: 'Spree::Address' + alias_attribute :billing_address, :bill_address + + belongs_to :ship_address, foreign_key: :ship_address_id, class_name: 'Spree::Address' + alias_attribute :shipping_address, :ship_address + + has_many :state_changes, as: :stateful + has_many :line_items, -> { order('created_at ASC') }, dependent: :destroy + has_many :payments, dependent: :destroy + has_many :return_authorizations, dependent: :destroy + has_many :adjustments, -> { order("#{Adjustment.table_name}.created_at ASC") }, as: :adjustable, dependent: :destroy + has_many :line_item_adjustments, through: :line_items, source: :adjustments + + has_many :shipments, dependent: :destroy do + def states + pluck(:state).uniq + end + end + + accepts_nested_attributes_for :line_items + accepts_nested_attributes_for :bill_address + accepts_nested_attributes_for :ship_address + accepts_nested_attributes_for :payments + accepts_nested_attributes_for :shipments + + # Needs to happen before save_permalink is called + before_validation :set_currency + before_validation :generate_order_number, on: :create + before_validation :clone_billing_address, if: :use_billing? + attr_accessor :use_billing + + before_create :link_by_email + after_create :create_tax_charge! + + validates :email, presence: true, if: :require_email + validates :email, email: true, if: :require_email, allow_blank: true + validate :has_available_shipment + validate :has_available_payment + + make_permalink field: :number + + class_attribute :update_hooks + self.update_hooks = Set.new + + def self.by_number(number) + where(number: number) + end + + def self.between(start_date, end_date) + where(created_at: start_date..end_date) + end + + def self.by_customer(customer) + joins(:user).where("#{Spree.user_class.table_name}.email" => customer) + end + + def self.by_state(state) + where(state: state) + end + + def self.complete + where('completed_at IS NOT NULL') + end + + def self.incomplete + where(completed_at: nil) + end + + # Use this method in other gems that wish to register their own custom logic + # that should be called after Order#update + def self.register_update_hook(hook) + self.update_hooks.add(hook) + end + + # For compatiblity with Calculator::PriceSack + def amount + line_items.inject(0.0) { |sum, li| sum + li.amount } + end + + def currency + 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 + + def display_adjustment_total + Spree::Money.new(adjustment_total, { currency: currency }) + end + + def display_tax_total + Spree::Money.new(tax_total, { currency: currency }) + end + + def display_ship_total + Spree::Money.new(ship_total, { currency: currency }) + end + + def display_total + Spree::Money.new(total, { currency: currency }) + end + + def to_param + number.to_s.to_url.upcase + end + + def completed? + completed_at.present? + end + + # Indicates whether or not the user is allowed to proceed to checkout. + # Currently this is implemented as a check for whether or not there is at + # least one LineItem in the Order. Feel free to override this logic in your + # own application if you require additional steps before allowing a checkout. + def checkout_allowed? + line_items.count > 0 + end + + # Is this a free order in which case the payment step should be skipped + def payment_required? + total.to_f > 0.0 + end + + # If true, causes the confirmation step to happen during the checkout process + def confirmation_required? + payments.map(&:payment_method).compact.any?(&:payment_profiles_supported?) + end + + # Indicates the number of items in the order + def item_count + line_items.inject(0) { |sum, li| sum + li.quantity } + end + + def backordered? + shipments.any?(&:backordered?) + end + + # Returns the relevant zone (if any) to be used for taxation purposes. + # Uses default tax zone unless there is a specific match + def tax_zone + Zone.match(tax_address) || Zone.default_tax + end + + # Indicates whether tax should be backed out of the price calcualtions in + # cases where prices include tax but the customer is not required to pay + # taxes in that case. + def exclude_tax? + return false unless Spree::Config[:prices_inc_tax] + return tax_zone != Zone.default_tax + end + + # Returns the address for taxation based on configuration + def tax_address + Spree::Config[:tax_using_ship_address] ? ship_address : bill_address + end + + # Array of totals grouped by Adjustment#label. Useful for displaying line item + # adjustments on an invoice. For example, you can display tax breakout for + # cases where tax is included in price. + def line_item_adjustment_totals + Hash[self.line_item_adjustments.eligible.group_by(&:label).map do |label, adjustments| + total = adjustments.sum(&:amount) + [label, Spree::Money.new(total, { currency: currency })] + end] + end + + def updater + @updater ||= Spree::Config.order_updater_decorator.new( + Spree::OrderUpdater.new(self) + ) + end + + def update! + updater.update + end + + def update_totals + updater.update_totals + end + + def clone_billing_address + if bill_address and self.ship_address.nil? + self.ship_address = bill_address.clone + else + self.ship_address.attributes = bill_address.attributes.except('id', 'updated_at', 'created_at') + end + true + end + + def allow_cancel? + return false unless completed? and state != 'canceled' + shipment_state.nil? || %w{ready backorder pending}.include?(shipment_state) + end + + def allow_resume? + # we shouldn't allow resume for legacy orders b/c we lack the information + # necessary to restore to a previous state + return false if state_changes.empty? || state_changes.last.previous_state.nil? + true + end + + def awaiting_returns? + return_authorizations.any? { |return_authorization| return_authorization.authorized? } + end + + def contents + @contents ||= Spree::OrderContents.new(self) + end + + # Associates the specified user with the order. + def associate_user!(user) + self.user = user + self.email = user.email + self.created_by = user if self.created_by.blank? + + if persisted? + # immediately persist the changes we just made, but don't use save since we might have an invalid address associated + self.class.unscoped.where(id: id).update_all(email: user.email, user_id: user.id, created_by_id: self.created_by_id) + end + end + + # FIXME refactor this method and implement validation using validates_* utilities + def generate_order_number + record = true + while record + random = "R#{Array.new(9){rand(9)}.join}" + record = self.class.where(number: random).first + end + self.number = random if self.number.blank? + self.number + end + + def shipped_shipments + shipments.shipped + end + + def contains?(variant) + find_line_item_by_variant(variant).present? + end + + def quantity_of(variant) + line_item = find_line_item_by_variant(variant) + line_item ? line_item.quantity : 0 + end + + def find_line_item_by_variant(variant) + line_items.detect { |line_item| line_item.variant_id == variant.id } + end + + def ship_total + adjustments.shipping.map(&:amount).sum + end + + def tax_total + adjustments.tax.map(&:amount).sum + end + + # Creates new tax charges if there are any applicable rates. If prices already + # include taxes then price adjustments are created instead. + def create_tax_charge! + Spree::TaxRate.adjust(self) + end + + def outstanding_balance + total - payment_total + end + + def outstanding_balance? + self.outstanding_balance != 0 + end + + def name + if (address = bill_address || ship_address) + "#{address.firstname} #{address.lastname}" + end + end + + def can_ship? + self.complete? || self.resumed? || self.awaiting_return? || self.returned? + end + + def credit_cards + credit_card_ids = payments.from_credit_card.pluck(:source_id).uniq + CreditCard.where(id: credit_card_ids) + end + + # Finalizes an in progress order after checkout is complete. + # Called after transition to complete state when payments will have been processed + def finalize! + touch :completed_at + + # lock all adjustments (coupon promotions, etc.) + adjustments.update_all state: 'closed' + + # update payment and shipment(s) states, and save + updater.update_payment_state + shipments.each do |shipment| + shipment.update!(self) + shipment.finalize! + end + + updater.update_shipment_state + updater.before_save_hook + save + updater.run_hooks + + deliver_order_confirmation_email + + self.state_changes.create( + previous_state: 'cart', + next_state: 'complete', + name: 'order' , + user_id: self.user_id + ) + end + + def deliver_order_confirmation_email + begin + OrderMailer.confirm_email(self.id).deliver + rescue Exception => e + logger.error("#{e.class.name}: #{e.message}") + logger.error(e.backtrace * "\n") + end + end + + # Helper methods for checkout steps + def paid? + payment_state == 'paid' || payment_state == 'credit_owed' + end + + def available_payment_methods + @available_payment_methods ||= PaymentMethod.available(:front_end) + end + + def pending_payments + payments.select(&:checkout?) + end + + # processes any pending payments and must return a boolean as it's + # return value is used by the checkout state_machine to determine + # success or failure of the 'complete' event for the order + # + # Returns: + # - true if all pending_payments processed successfully + # - true if a payment failed, ie. raised a GatewayError + # which gets rescued and converted to TRUE when + # :allow_checkout_gateway_error is set to true + # - false if a payment failed, ie. raised a GatewayError + # which gets rescued and converted to FALSE when + # :allow_checkout_on_gateway_error is set to false + # + def process_payments! + if pending_payments.empty? + raise Core::GatewayError.new Spree.t(:no_pending_payments) + else + pending_payments.each do |payment| + break if payment_total >= total + + payment.process! + + if payment.completed? + self.payment_total += payment.amount + end + end + end + rescue Core::GatewayError => e + result = !!Spree::Config[:allow_checkout_on_gateway_error] + errors.add(:base, e.message) and return result + end + + def billing_firstname + bill_address.try(:firstname) + end + + def billing_lastname + bill_address.try(:lastname) + end + + def products + line_items.map(&:product) + end + + def variants + line_items.map(&:variant) + end + + def insufficient_stock_lines + line_items.select &:insufficient_stock? + end + + def merge!(order) + order.line_items.each do |line_item| + next unless line_item.currency == currency + current_line_item = self.line_items.find_by(variant: line_item.variant) + if current_line_item + current_line_item.quantity += line_item.quantity + current_line_item.save + else + line_item.order_id = self.id + line_item.save + end + end + # So that the destroy doesn't take out line items which may have been re-assigned + order.line_items.reload + order.destroy + end + + def empty! + line_items.destroy_all + adjustments.destroy_all + end + + def clear_adjustments! + self.adjustments.destroy_all + self.line_item_adjustments.destroy_all + end + + def has_step?(step) + checkout_steps.include?(step) + end + + def state_changed(name) + state = "#{name}_state" + if persisted? + old_state = self.send("#{state}_was") + self.state_changes.create( + previous_state: old_state, + next_state: self.send(state), + name: name, + user_id: self.user_id + ) + end + end + + def coupon_code=(code) + @coupon_code = code.strip.downcase rescue nil + end + + # Tells us if there if the specified promotion is already associated with the order + # regardless of whether or not its currently eligible. Useful because generally + # you would only want a promotion action to apply to order no more than once. + # + # Receives an adjustment +originator+ (here a PromotionAction object) and tells + # if the order has adjustments from that already + def promotion_credit_exists?(originator) + !! adjustments.includes(:originator).promotion.reload.detect { |credit| credit.originator.id == originator.id } + end + + def promo_total + adjustments.eligible.promotion.map(&:amount).sum + end + + def shipped? + %w(partial shipped).include?(shipment_state) + end + + def create_proposed_shipments + adjustments.shipping.delete_all + shipments.destroy_all + + packages = Spree::Stock::Coordinator.new(self).packages + packages.each do |package| + shipments << package.to_shipment + end + + shipments + end + + # Clean shipments and make order back to address state + # + # At some point the might need to force the order to transition from address + # to delivery again so that proper updated shipments are created. + # e.g. customer goes back from payment step and changes order items + def ensure_updated_shipments + if shipments.any? + self.shipments.destroy_all + self.update_column(:state, "address") + end + end + + def refresh_shipment_rates + shipments.map &:refresh_rates + end + + private + + def link_by_email + self.email = user.email if self.user + end + + # Determine if email is required (we don't want validation errors before we hit the checkout) + def require_email + return true unless new_record? or state == 'cart' + end + + def ensure_line_items_present + unless line_items.present? + errors.add(:base, Spree.t(:there_are_no_items_for_this_order)) and return false + end + end + + def has_available_shipment + return unless has_step?("delivery") + return unless address? + return unless ship_address && ship_address.valid? + # errors.add(:base, :no_shipping_methods_available) if available_shipping_methods.empty? + end + + def ensure_available_shipping_rates + if shipments.empty? || shipments.any? { |shipment| shipment.shipping_rates.blank? } + errors.add(:base, Spree.t(:items_cannot_be_shipped)) and return false + end + end + + def has_available_payment + return unless delivery? + # errors.add(:base, :no_payment_methods_available) if available_payment_methods.empty? + end + + def after_cancel + shipments.each { |shipment| shipment.cancel! } + + OrderMailer.cancel_email(self.id).deliver + self.payment_state = 'credit_owed' unless shipped? + end + + def after_resume + shipments.each { |shipment| shipment.resume! } + end + + def use_billing? + @use_billing == true || @use_billing == 'true' || @use_billing == '1' + end + + def set_currency + self.currency = Spree::Config[:currency] if self[:currency].nil? + end + end +end diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb new file mode 100644 index 00000000000..c6295455788 --- /dev/null +++ b/app/models/spree/order_contents.rb @@ -0,0 +1,67 @@ +module Spree + class OrderContents + attr_accessor :order, :currency + + def initialize(order) + @order = order + end + + # Get current line item for variant if exists + # Add variant qty to line_item + def add(variant, quantity = 1, currency = nil, shipment = nil) + line_item = order.find_line_item_by_variant(variant) + add_to_line_item(line_item, variant, quantity, currency, shipment) + end + + # Get current line item for variant + # Remove variant qty from line_item + def remove(variant, quantity = 1, shipment = nil) + line_item = order.find_line_item_by_variant(variant) + + unless line_item + raise ActiveRecord::RecordNotFound, "Line item not found for variant #{variant.sku}" + end + + remove_from_line_item(line_item, variant, quantity, shipment) + end + + private + + def add_to_line_item(line_item, variant, quantity, currency=nil, shipment=nil) + if line_item + line_item.target_shipment = shipment + line_item.quantity += quantity.to_i + line_item.currency = currency unless currency.nil? + else + line_item = order.line_items.new(quantity: quantity, variant: variant) + line_item.target_shipment = shipment + if currency + line_item.currency = currency unless currency.nil? + line_item.price = variant.price_in(currency).amount + else + line_item.price = variant.price + end + end + + line_item.save + order.reload + line_item + end + + def remove_from_line_item(line_item, variant, quantity, shipment=nil) + line_item.quantity += -quantity + line_item.target_shipment= shipment + + if line_item.quantity == 0 + Spree::OrderInventory.new(order).verify(line_item, shipment) + line_item.destroy + else + line_item.save! + end + + order.reload + line_item + end + + end +end diff --git a/app/models/spree/order_inventory.rb b/app/models/spree/order_inventory.rb new file mode 100644 index 00000000000..d04db006e90 --- /dev/null +++ b/app/models/spree/order_inventory.rb @@ -0,0 +1,106 @@ +module Spree + class OrderInventory + attr_accessor :order + + def initialize(order) + @order = order + end + + # Only verify inventory for completed orders (as orders in frontend checkout + # have inventory assigned via +order.create_proposed_shipment+) or when + # shipment is explicitly passed + # + # In case shipment is passed the stock location should only unstock or + # restock items if the order is completed. That is so because stock items + # are always unstocked when the order is completed through +shipment.finalize+ + def verify(line_item, shipment = nil) + if order.completed? || shipment.present? + + variant_units = inventory_units_for(line_item.variant) + + if variant_units.size < line_item.quantity + quantity = line_item.quantity - variant_units.size + + shipment = determine_target_shipment(line_item.variant) unless shipment + add_to_shipment(shipment, line_item.variant, quantity) + elsif variant_units.size > line_item.quantity + remove(line_item, variant_units, shipment) + end + else + true + end + end + + def inventory_units_for(variant) + units = order.shipments.collect{|s| s.inventory_units.to_a}.flatten + units.group_by(&:variant_id)[variant.id] || [] + end + + private + def remove(line_item, variant_units, shipment = nil) + quantity = variant_units.size - line_item.quantity + + if shipment.present? + remove_from_shipment(shipment, line_item.variant, quantity) + else + order.shipments.each do |shipment| + break if quantity == 0 + quantity -= remove_from_shipment(shipment, line_item.variant, quantity) + end + end + end + + # Returns either one of the shipment: + # + # first unshipped that already includes this variant + # first unshipped that's leaving from a stock_location that stocks this variant + def determine_target_shipment(variant) + shipment = order.shipments.detect do |shipment| + (shipment.ready? || shipment.pending?) && shipment.include?(variant) + end + + shipment ||= order.shipments.detect do |shipment| + (shipment.ready? || shipment.pending?) && variant.stock_location_ids.include?(shipment.stock_location_id) + end + end + + def add_to_shipment(shipment, variant, quantity) + on_hand, back_order = shipment.stock_location.fill_status(variant, quantity) + + on_hand.times { shipment.set_up_inventory('on_hand', variant, order) } + back_order.times { shipment.set_up_inventory('backordered', variant, order) } + + # adding to this shipment, and removing from stock_location + if order.completed? + shipment.stock_location.unstock(variant, quantity, shipment) + end + + quantity + end + + def remove_from_shipment(shipment, variant, quantity) + return 0 if quantity == 0 || shipment.shipped? + + shipment_units = shipment.inventory_units_for(variant).reject do |variant_unit| + variant_unit.state == 'shipped' + end.sort_by(&:state) + + removed_quantity = 0 + + shipment_units.each do |inventory_unit| + break if removed_quantity == quantity + inventory_unit.destroy + removed_quantity += 1 + end + + shipment.destroy if shipment.inventory_units.count == 0 + + # removing this from shipment, and adding to stock_location + if order.completed? + shipment.stock_location.restock variant, removed_quantity, shipment + end + + removed_quantity + end + end +end diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb new file mode 100644 index 00000000000..0a98f55feca --- /dev/null +++ b/app/models/spree/return_authorization.rb @@ -0,0 +1,103 @@ +module Spree + class ReturnAuthorization < ActiveRecord::Base + belongs_to :order, class_name: 'Spree::Order' + + has_many :inventory_units + has_one :stock_location + before_create :generate_number + before_save :force_positive_amount + + validates :order, presence: true + validates :amount, numericality: true + validate :must_have_shipped_units + + state_machine initial: :authorized do + after_transition to: :received, do: :process_return + + event :receive do + transition to: :received, from: :authorized, if: :allow_receive? + end + event :cancel do + transition to: :canceled, from: :authorized + end + end + + def currency + order.nil? ? Spree::Config[:currency] : order.currency + end + + def display_amount + Spree::Money.new(amount, { currency: currency }) + end + + def add_variant(variant_id, quantity) + order_units = returnable_inventory.group_by(&:variant_id) + returned_units = inventory_units.group_by(&:variant_id) + return false if order_units.empty? + + count = 0 + + if returned_units[variant_id].nil? || returned_units[variant_id].size < quantity + count = returned_units[variant_id].nil? ? 0 : returned_units[variant_id].size + + order_units[variant_id].each do |inventory_unit| + next unless inventory_unit.return_authorization.nil? && count < quantity + + inventory_unit.return_authorization = self + inventory_unit.save! + + count += 1 + end + elsif returned_units[variant_id].size > quantity + (returned_units[variant_id].size - quantity).times do |i| + returned_units[variant_id][i].return_authorization_id = nil + returned_units[variant_id][i].save! + end + end + + order.authorize_return! if inventory_units.reload.size > 0 && !order.awaiting_return? + end + + def returnable_inventory + order.shipped_shipments.collect{|s| s.inventory_units.to_a}.flatten + end + + private + def must_have_shipped_units + errors.add(:order, Spree.t(:has_no_shipped_units)) if order.nil? || !order.shipped_shipments.any? + end + + def generate_number + return if number + + record = true + while record + random = "RMA#{Array.new(9){rand(9)}.join}" + record = self.class.where(number: random).first + end + self.number = random + end + + def process_return + inventory_units.each do |iu| + iu.return! + Spree::StockMovement.create!(stock_item_id: iu.find_stock_item.id, quantity: 1) + end + + credit = Adjustment.new(amount: amount.abs * -1, label: Spree.t(:rma_credit)) + credit.source = self + credit.adjustable = order + credit.save + + order.return if inventory_units.all?(&:returned?) + end + + def allow_receive? + !inventory_units.empty? + end + + def force_positive_amount + self.amount = amount.abs + end + end +end diff --git a/app/models/spree/state_change.rb b/app/models/spree/state_change.rb new file mode 100644 index 00000000000..8954dffe81a --- /dev/null +++ b/app/models/spree/state_change.rb @@ -0,0 +1,15 @@ +module Spree + class StateChange < ActiveRecord::Base + belongs_to :user + belongs_to :stateful, polymorphic: true + before_create :assign_user + + def <=>(other) + created_at <=> other.created_at + end + + def assign_user + true # don't stop the filters + end + end +end diff --git a/app/models/spree/tokenized_permission.rb b/app/models/spree/tokenized_permission.rb new file mode 100644 index 00000000000..29cc24e8b42 --- /dev/null +++ b/app/models/spree/tokenized_permission.rb @@ -0,0 +1,6 @@ +module Spree + class TokenizedPermission < ActiveRecord::Base + belongs_to :permissable, polymorphic: true + end +end + diff --git a/spec/models/spree/inventory_unit_spec.rb b/spec/models/spree/inventory_unit_spec.rb new file mode 100644 index 00000000000..2beacf1976e --- /dev/null +++ b/spec/models/spree/inventory_unit_spec.rb @@ -0,0 +1,85 @@ +require 'spec_helper' + +describe Spree::InventoryUnit do + let(:stock_location) { create(:stock_location_with_items) } + let(:stock_item) { stock_location.stock_items.order(:id).first } + + context "#backordered_for_stock_item" do + let(:order) { create(:order) } + + let(:shipment) do + shipment = Spree::Shipment.new + shipment.stock_location = stock_location + shipment.shipping_methods << create(:shipping_method) + shipment.order = order + # We don't care about this in this test + shipment.stub(:ensure_correct_adjustment) + shipment.tap(&:save!) + end + + let!(:unit) do + unit = shipment.inventory_units.build + unit.state = 'backordered' + unit.variant_id = stock_item.variant.id + unit.tap(&:save!) + end + + # Regression for #3066 + it "returns modifiable objects" do + units = Spree::InventoryUnit.backordered_for_stock_item(stock_item) + expect { units.first.save! }.to_not raise_error + end + + it "finds inventory units from its stock location when the unit's variant matches the stock item's variant" do + Spree::InventoryUnit.backordered_for_stock_item(stock_item).should =~ [unit] + end + + it "does not find inventory units that aren't backordered" do + on_hand_unit = shipment.inventory_units.build + on_hand_unit.state = 'on_hand' + on_hand_unit.variant_id = 1 + on_hand_unit.save! + + Spree::InventoryUnit.backordered_for_stock_item(stock_item).should_not include(on_hand_unit) + end + + it "does not find inventory units that don't match the stock item's variant" do + other_variant_unit = shipment.inventory_units.build + other_variant_unit.state = 'backordered' + other_variant_unit.variant = create(:variant) + other_variant_unit.save! + + Spree::InventoryUnit.backordered_for_stock_item(stock_item).should_not include(other_variant_unit) + end + end + + context "variants deleted" do + let!(:unit) do + Spree::InventoryUnit.create(variant: stock_item.variant) + end + + it "can still fetch variant" do + unit.variant.destroy + expect(unit.reload.variant).to be_a Spree::Variant + end + + it "can still fetch variants by eager loading (remove default_scope)" do + unit.variant.destroy + expect(Spree::InventoryUnit.joins(:variant).includes(:variant).first.variant).to be_a Spree::Variant + end + end + + context "#finalize_units!" do + let!(:stock_location) { create(:stock_location) } + let(:variant) { create(:variant) } + let(:inventory_units) { [ + create(:inventory_unit, variant: variant), + create(:inventory_unit, variant: variant) + ] } + + it "should create a stock movement" do + Spree::InventoryUnit.finalize_units!(inventory_units) + inventory_units.any?(&:pending).should be_false + end + end +end diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 4410a009759..65d1924dd59 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -2,6 +2,143 @@ module Spree describe LineItem do + let(:order) { create :order_with_line_items, line_items_count: 1 } + let(:line_item) { order.line_items.first } + + context '#save' do + it 'should update inventory, totals, and tax' do + # Regression check for Spree #1481 + line_item.order.should_receive(:create_tax_charge!) + line_item.order.should_receive(:update!) + line_item.quantity = 2 + line_item.save + end + end + + context '#destroy' do + # Regression test for Spree #1481 + it "applies tax adjustments" do + line_item.order.should_receive(:create_tax_charge!) + line_item.destroy + end + + it "fetches deleted products" do + line_item.product.destroy + expect(line_item.reload.product).to be_a Spree::Product + end + + it "fetches deleted variants" do + line_item.variant.destroy + expect(line_item.reload.variant).to be_a Spree::Variant + end + end + + # Test for Spree #3391 + context '#copy_price' do + it "copies over a variant's prices" do + line_item.price = nil + line_item.cost_price = nil + line_item.currency = nil + line_item.copy_price + variant = line_item.variant + line_item.price.should == variant.price + line_item.cost_price.should == variant.cost_price + line_item.currency.should == variant.currency + end + end + + # Test for Spree #3481 + context '#copy_tax_category' do + it "copies over a variant's tax category" do + line_item.tax_category = nil + line_item.copy_tax_category + line_item.tax_category.should == line_item.variant.product.tax_category + end + end + + describe '.currency' do + it 'returns the globally configured currency' do + line_item.currency == 'USD' + end + end + + describe ".money" do + before do + line_item.price = 3.50 + line_item.quantity = 2 + end + + it "returns a Spree::Money representing the total for this line item" do + line_item.money.to_s.should == "$7.00" + end + end + + describe '.single_money' do + before { line_item.price = 3.50 } + it "returns a Spree::Money representing the price for one variant" do + line_item.single_money.to_s.should == "$3.50" + end + end + + context "has inventory (completed order so items were already unstocked)" do + let(:order) { Spree::Order.create } + let(:variant) { create(:variant) } + + context "nothing left on stock" do + before do + variant.stock_items.update_all count_on_hand: 5, backorderable: false + order.contents.add(variant, 5) + order.create_proposed_shipments + order.finalize! + end + + it "allows to decrease item quantity" do + line_item = order.line_items.first + line_item.quantity -= 1 + line_item.target_shipment = order.shipments.first + + line_item.save + expect(line_item).to have(0).errors_on(:quantity) + end + + it "doesnt allow to increase item quantity" do + line_item = order.line_items.first + line_item.quantity += 2 + line_item.target_shipment = order.shipments.first + + line_item.save + expect(line_item).to have(1).errors_on(:quantity) + end + end + + context "2 items left on stock" do + before do + variant.stock_items.update_all count_on_hand: 7, backorderable: false + order.contents.add(variant, 5) + order.create_proposed_shipments + order.finalize! + end + + it "allows to increase quantity up to stock availability" do + line_item = order.line_items.first + line_item.quantity += 2 + line_item.target_shipment = order.shipments.first + + line_item.save + expect(line_item).to have(0).errors_on(:quantity) + end + + it "doesnt allow to increase quantity over stock availability" do + line_item = order.line_items.first + line_item.quantity += 3 + line_item.target_shipment = order.shipments.first + + line_item.save + expect(line_item).to have(1).errors_on(:quantity) + end + end + end + describe "scopes" do let(:o) { create(:order) } diff --git a/spec/models/spree/order/address_spec.rb b/spec/models/spree/order/address_spec.rb new file mode 100644 index 00000000000..2efbc20be6d --- /dev/null +++ b/spec/models/spree/order/address_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe Spree::Order do + let(:order) { Spree::Order.new } + + context 'validation' do + context "when @use_billing is populated" do + before do + order.bill_address = stub_model(Spree::Address) + order.ship_address = nil + end + + context "with true" do + before { order.use_billing = true } + + it "clones the bill address to the ship address" do + order.valid? + order.ship_address.should == order.bill_address + end + end + + context "with 'true'" do + before { order.use_billing = 'true' } + + it "clones the bill address to the shipping" do + order.valid? + order.ship_address.should == order.bill_address + end + end + + context "with '1'" do + before { order.use_billing = '1' } + + it "clones the bill address to the shipping" do + order.valid? + order.ship_address.should == order.bill_address + end + end + + context "with something other than a 'truthful' value" do + before { order.use_billing = '0' } + + it "does not clone the bill address to the shipping" do + order.valid? + order.ship_address.should be_nil + end + end + end + end +end diff --git a/spec/models/spree/order/adjustments_spec.rb b/spec/models/spree/order/adjustments_spec.rb new file mode 100644 index 00000000000..d35945b3c50 --- /dev/null +++ b/spec/models/spree/order/adjustments_spec.rb @@ -0,0 +1,148 @@ +require 'spec_helper' +describe Spree::Order do + let(:order) { Spree::Order.new } + + context "clear_adjustments" do + + let(:adjustment) { double("Adjustment") } + + it "destroys all order adjustments" do + order.stub(:adjustments => adjustment) + adjustment.should_receive(:destroy_all) + order.clear_adjustments! + end + + it "destroy all line item adjustments" do + order.stub(:line_item_adjustments => adjustment) + adjustment.should_receive(:destroy_all) + order.clear_adjustments! + end + end + + context "totaling adjustments" do + let(:adjustment1) { mock_model(Spree::Adjustment, :amount => 5) } + let(:adjustment2) { mock_model(Spree::Adjustment, :amount => 10) } + + context "#ship_total" do + it "should return the correct amount" do + order.stub_chain :adjustments, :shipping => [adjustment1, adjustment2] + order.ship_total.should == 15 + end + end + + context "#tax_total" do + it "should return the correct amount" do + order.stub_chain :adjustments, :tax => [adjustment1, adjustment2] + order.tax_total.should == 15 + end + end + end + + + context "line item adjustment totals" do + before { @order = Spree::Order.create! } + + + context "when there are no line item adjustments" do + before { @order.stub_chain(:line_item_adjustments, :eligible => []) } + + it "should return an empty hash" do + @order.line_item_adjustment_totals.should == {} + end + end + + context "when there are two adjustments with different labels" do + let(:adj1) { mock_model Spree::Adjustment, :amount => 10, :label => "Foo" } + let(:adj2) { mock_model Spree::Adjustment, :amount => 20, :label => "Bar" } + + before do + @order.stub_chain(:line_item_adjustments, :eligible => [adj1, adj2]) + end + + it "should return exactly two totals" do + @order.line_item_adjustment_totals.size.should == 2 + end + + it "should return the correct totals" do + @order.line_item_adjustment_totals["Foo"].should == Spree::Money.new(10) + @order.line_item_adjustment_totals["Bar"].should == Spree::Money.new(20) + end + end + + context "when there are two adjustments with one label and a single adjustment with another" do + let(:adj1) { mock_model Spree::Adjustment, :amount => 10, :label => "Foo" } + let(:adj2) { mock_model Spree::Adjustment, :amount => 20, :label => "Bar" } + let(:adj3) { mock_model Spree::Adjustment, :amount => 40, :label => "Bar" } + + before do + @order.stub_chain(:line_item_adjustments, :eligible => [adj1, adj2, adj3]) + end + + it "should return exactly two totals" do + @order.line_item_adjustment_totals.size.should == 2 + end + it "should return the correct totals" do + @order.line_item_adjustment_totals["Foo"].should == Spree::Money.new(10) + @order.line_item_adjustment_totals["Bar"].should == Spree::Money.new(60) + end + end + end + + context "line item adjustments" do + before do + @order = Spree::Order.create! + @order.stub :line_items => [line_item1, line_item2] + end + + let(:line_item1) { create(:line_item, :order => @order) } + let(:line_item2) { create(:line_item, :order => @order) } + + context "when there are no line item adjustments" do + it "should return nothing if line items have no adjustments" do + @order.line_item_adjustments.should be_empty + end + end + + context "when only one line item has adjustments" do + before do + @adj1 = line_item1.adjustments.create( + :amount => 2, + :source => line_item1, + :label => "VAT 5%" + ) + + @adj2 = line_item1.adjustments.create( + :amount => 5, + :source => line_item1, + :label => "VAT 10%" + ) + end + + it "should return the adjustments for that line item" do + @order.line_item_adjustments.should =~ [@adj1, @adj2] + end + end + + context "when more than one line item has adjustments" do + before do + @adj1 = line_item1.adjustments.create( + :amount => 2, + :source => line_item1, + :label => "VAT 5%" + ) + + @adj2 = line_item2.adjustments.create( + :amount => 5, + :source => line_item2, + :label => "VAT 10%" + ) + end + + it "should return the adjustments for each line item" do + expect(@order.line_item_adjustments).to include @adj1 + expect(@order.line_item_adjustments).to include @adj2 + end + end + end +end + diff --git a/spec/models/spree/order/callbacks_spec.rb b/spec/models/spree/order/callbacks_spec.rb new file mode 100644 index 00000000000..2743242a751 --- /dev/null +++ b/spec/models/spree/order/callbacks_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +describe Spree::Order do + let(:order) { stub_model(Spree::Order) } + before do + Spree::Order.define_state_machine! + end + + context "validations" do + context "email validation" do + # Regression test for Spree #1238 + it "o'brien@gmail.com is a valid email address" do + order.state = 'address' + order.email = "o'brien@gmail.com" + order.should have(:no).error_on(:email) + end + end + end + + context "#save" do + context "when associated with a registered user" do + let(:user) { double(:user, :email => "test@example.com") } + + before do + order.stub :user => user + end + + it "should assign the email address of the user" do + order.run_callbacks(:create) + order.email.should == user.email + end + end + end + + context "in the cart state" do + it "should not validate email address" do + order.state = "cart" + order.email = nil + order.should have(:no).error_on(:email) + end + end +end diff --git a/spec/models/spree/order/checkout_spec.rb b/spec/models/spree/order/checkout_spec.rb index 1020fb0fe55..f25df0b1d4c 100644 --- a/spec/models/spree/order/checkout_spec.rb +++ b/spec/models/spree/order/checkout_spec.rb @@ -147,7 +147,7 @@ end end - # Regression test for #2028 + # Regression test for Spree #2028 context "when payment is not required" do before do allow(order).to receive_messages payment_required?: false @@ -211,7 +211,7 @@ class SubclassedOrder < Spree::Order end end - # Regression test for #3665 + # Regression test for Spree #3665 context "with only a complete step" do before do @old_checkout_flow = Spree::Order.checkout_flow diff --git a/spec/models/spree/order/payment_spec.rb b/spec/models/spree/order/payment_spec.rb new file mode 100644 index 00000000000..8c53b45d70f --- /dev/null +++ b/spec/models/spree/order/payment_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +module Spree + describe Spree::Order do + let(:order) { stub_model(Spree::Order) } + let(:updater) { Spree::OrderUpdater.new(order) } + + before do + # So that Payment#purchase! is called during processing + Spree::Config[:auto_capture] = true + + order.stub_chain(:line_items, :empty?).and_return(false) + order.stub :total => 100 + end + + it 'processes all payments' do + payment_1 = create(:payment, :amount => 50) + payment_2 = create(:payment, :amount => 50) + order.stub(:pending_payments).and_return([payment_1, payment_2]) + + order.process_payments! + updater.update_payment_state + order.payment_state.should == 'paid' + + payment_1.should be_completed + payment_2.should be_completed + end + + it 'does not go over total for order' do + payment_1 = create(:payment, :amount => 50) + payment_2 = create(:payment, :amount => 50) + payment_3 = create(:payment, :amount => 50) + order.stub(:pending_payments).and_return([payment_1, payment_2, payment_3]) + + order.process_payments! + updater.update_payment_state + order.payment_state.should == 'paid' + + payment_1.should be_completed + payment_2.should be_completed + payment_3.should be_checkout + end + + it "does not use failed payments" do + payment_1 = create(:payment, :amount => 50) + payment_2 = create(:payment, :amount => 50, :state => 'failed') + order.stub(:pending_payments).and_return([payment_1]) + + payment_2.should_not_receive(:process!) + + order.process_payments! + end + end +end diff --git a/spec/models/spree/order/state_machine_spec.rb b/spec/models/spree/order/state_machine_spec.rb new file mode 100644 index 00000000000..a64a793b953 --- /dev/null +++ b/spec/models/spree/order/state_machine_spec.rb @@ -0,0 +1,183 @@ +require 'spec_helper' + +describe Spree::Order do + let(:order) { Spree::Order.new } + before do + # Ensure state machine has been re-defined correctly + Spree::Order.define_state_machine! + # We don't care about this validation here + order.stub(:require_email) + end + + context "#next!" do + context "when current state is confirm" do + before do + order.state = "confirm" + order.run_callbacks(:create) + order.stub :payment_required? => true + order.stub :process_payments! => true + order.stub :has_available_shipment + end + + context "when payment processing succeeds" do + before { order.stub :process_payments! => true } + + it "should finalize order when transitioning to complete state" do + order.should_receive(:finalize!) + order.next! + end + + context "when credit card processing fails" do + before { order.stub :process_payments! => false } + + it "should not complete the order" do + order.next + order.state.should == "confirm" + end + end + + end + + context "when payment processing fails" do + before { order.stub :process_payments! => false } + + it "cannot transition to complete" do + order.next + order.state.should == "confirm" + end + end + + end + + context "when current state is address" do + before do + order.stub(:has_available_payment) + order.stub(:ensure_available_shipping_rates) + order.state = "address" + end + + it "adjusts tax rates when transitioning to delivery" do + # Once because the record is being saved + # Twice because it is transitioning to the delivery state + Spree::TaxRate.should_receive(:adjust).twice + order.next! + end + end + + context "when current state is delivery" do + before do + order.state = "delivery" + order.stub :total => 10.0 + end + end + + end + + context "#can_cancel?" do + + %w(pending backorder ready).each do |shipment_state| + it "should be true if shipment_state is #{shipment_state}" do + order.stub :completed? => true + order.shipment_state = shipment_state + order.can_cancel?.should be_true + end + end + + (Spree::Shipment.state_machine.states.keys - %w(pending backorder ready)).each do |shipment_state| + it "should be false if shipment_state is #{shipment_state}" do + order.stub :completed? => true + order.shipment_state = shipment_state + order.can_cancel?.should be_false + end + end + + end + + context "#cancel" do + let!(:variant) { stub_model(Spree::Variant) } + let!(:inventory_units) { [stub_model(Spree::InventoryUnit, :variant => variant), + stub_model(Spree::InventoryUnit, :variant => variant) ]} + let!(:shipment) do + shipment = stub_model(Spree::Shipment) + shipment.stub :inventory_units => inventory_units + order.stub :shipments => [shipment] + shipment + end + + before do + order.stub :line_items => [stub_model(Spree::LineItem, :variant => variant, :quantity => 2)] + order.line_items.stub :find_by_variant_id => order.line_items.first + + order.stub :completed? => true + order.stub :allow_cancel? => true + end + + it "should send a cancel email" do + # Stub methods that cause side-effects in this test + shipment.stub(:cancel!) + order.stub :has_available_shipment + order.stub :restock_items! + mail_message = double "Mail::Message" + order_id = nil + Spree::OrderMailer.should_receive(:cancel_email) { |*args| + order_id = args[0] + mail_message + } + mail_message.should_receive :deliver + order.cancel! + order_id.should == order.id + end + + context "restocking inventory" do + before do + shipment.stub(:ensure_correct_adjustment) + shipment.stub(:update_order) + Spree::OrderMailer.stub(:cancel_email).and_return(mail_message = double) + mail_message.stub :deliver + + order.stub :has_available_shipment + end + end + + context "resets payment state" do + before do + # Stubs methods that cause unwanted side effects in this test + Spree::OrderMailer.stub(:cancel_email).and_return(mail_message = double) + mail_message.stub :deliver + order.stub :has_available_shipment + order.stub :restock_items! + shipment.stub(:cancel!) + end + + context "without shipped items" do + it "should set payment state to 'credit owed'" do + order.cancel! + order.payment_state.should == 'credit_owed' + end + end + + context "with shipped items" do + before do + order.stub :shipment_state => 'partial' + end + + it "should not alter the payment state" do + order.cancel! + order.payment_state.should be_nil + end + end + end + end + + # Another regression test for Spree #729 + context "#resume" do + before do + order.stub :email => "user@spreecommerce.com" + order.stub :state => "canceled" + order.stub :allow_resume? => true + + # Stubs method that cause unwanted side effects in this test + order.stub :has_available_shipment + end + end +end diff --git a/spec/models/spree/order/tax_spec.rb b/spec/models/spree/order/tax_spec.rb new file mode 100644 index 00000000000..045e0ee9e28 --- /dev/null +++ b/spec/models/spree/order/tax_spec.rb @@ -0,0 +1,115 @@ +require 'spec_helper' + +module Spree + describe Spree::Order do + let(:order) { stub_model(Spree::Order) } + + context "#tax_zone" do + let(:bill_address) { create :address } + let(:ship_address) { create :address } + let(:order) { Spree::Order.create(:ship_address => ship_address, :bill_address => bill_address) } + let(:zone) { create :zone } + + context "when no zones exist" do + before { Spree::Zone.destroy_all } + + it "should return nil" do + order.tax_zone.should be_nil + end + end + + context "when :tax_using_ship_address => true" do + before { Spree::Config.set(:tax_using_ship_address => true) } + + it "should calculate using ship_address" do + Spree::Zone.should_receive(:match).at_least(:once).with(ship_address) + Spree::Zone.should_not_receive(:match).with(bill_address) + order.tax_zone + end + end + + context "when :tax_using_ship_address => false" do + before { Spree::Config.set(:tax_using_ship_address => false) } + + it "should calculate using bill_address" do + Spree::Zone.should_receive(:match).at_least(:once).with(bill_address) + Spree::Zone.should_not_receive(:match).with(ship_address) + order.tax_zone + end + end + + context "when there is a default tax zone" do + before do + @default_zone = create(:zone, :name => "foo_zone") + Spree::Zone.stub :default_tax => @default_zone + end + + context "when there is a matching zone" do + before { Spree::Zone.stub(:match => zone) } + + it "should return the matching zone" do + order.tax_zone.should == zone + end + end + + context "when there is no matching zone" do + before { Spree::Zone.stub(:match => nil) } + + it "should return the default tax zone" do + order.tax_zone.should == @default_zone + end + end + end + + context "when no default tax zone" do + before { Spree::Zone.stub :default_tax => nil } + + context "when there is a matching zone" do + before { Spree::Zone.stub(:match => zone) } + + it "should return the matching zone" do + order.tax_zone.should == zone + end + end + + context "when there is no matching zone" do + before { Spree::Zone.stub(:match => nil) } + + it "should return nil" do + order.tax_zone.should be_nil + end + end + end + end + + context "#exclude_tax?" do + before do + @order = create(:order) + @default_zone = create(:zone) + Spree::Zone.stub :default_tax => @default_zone + end + + context "when prices include tax" do + before { Spree::Config.set(:prices_inc_tax => true) } + + it "should be true when tax_zone is not the same as the default" do + @order.stub :tax_zone => create(:zone, :name => "other_zone") + @order.exclude_tax?.should be_true + end + + it "should be false when tax_zone is the same as the default" do + @order.stub :tax_zone => @default_zone + @order.exclude_tax?.should be_false + end + end + + context "when prices do not include tax" do + before { Spree::Config.set(:prices_inc_tax => false) } + + it "should be false" do + @order.exclude_tax?.should be_false + end + end + end + end +end diff --git a/spec/models/spree/order/updating_spec.rb b/spec/models/spree/order/updating_spec.rb new file mode 100644 index 00000000000..9b50d029867 --- /dev/null +++ b/spec/models/spree/order/updating_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe Spree::Order do + let(:order) { stub_model(Spree::Order) } + + context "#update!" do + let(:line_items) { [mock_model(Spree::LineItem, :amount => 5) ]} + + context "when there are update hooks" do + before { Spree::Order.register_update_hook :foo } + after { Spree::Order.update_hooks.clear } + it "should call each of the update hooks" do + order.should_receive :foo + order.update! + end + end + end +end diff --git a/spec/models/spree/order_contents_spec.rb b/spec/models/spree/order_contents_spec.rb new file mode 100644 index 00000000000..3ee1bb01f77 --- /dev/null +++ b/spec/models/spree/order_contents_spec.rb @@ -0,0 +1,92 @@ +require 'spec_helper' + +describe Spree::OrderContents do + let(:order) { Spree::Order.create } + subject { described_class.new(order) } + + context "#add" do + let(:variant) { create(:variant) } + + context 'given quantity is not explicitly provided' do + it 'should add one line item' do + line_item = subject.add(variant) + line_item.quantity.should == 1 + order.line_items.size.should == 1 + end + end + + it 'should add line item if one does not exist' do + line_item = subject.add(variant, 1) + line_item.quantity.should == 1 + order.line_items.size.should == 1 + end + + it 'should update line item if one exists' do + subject.add(variant, 1) + line_item = subject.add(variant, 1) + line_item.quantity.should == 2 + order.line_items.size.should == 1 + end + + it "should update order totals" do + order.item_total.to_f.should == 0.00 + order.total.to_f.should == 0.00 + + subject.add(variant, 1) + + order.item_total.to_f.should == 19.99 + order.total.to_f.should == 19.99 + end + end + + context "#remove" do + let(:variant) { create(:variant) } + + context "given an invalid variant" do + it "raises an exception" do + expect { + subject.remove(variant, 1) + }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'given quantity is not explicitly provided' do + it 'should remove one line item' do + line_item = subject.add(variant, 3) + subject.remove(variant) + + line_item.reload.quantity.should == 2 + end + end + + it 'should reduce line_item quantity if quantity is less the line_item quantity' do + line_item = subject.add(variant, 3) + subject.remove(variant, 1) + + line_item.reload.quantity.should == 2 + end + + it 'should remove line_item if quantity matches line_item quantity' do + subject.add(variant, 1) + subject.remove(variant, 1) + + order.reload.find_line_item_by_variant(variant).should be_nil + end + + it "should update order totals" do + order.item_total.to_f.should == 0.00 + order.total.to_f.should == 0.00 + + subject.add(variant,2) + + order.item_total.to_f.should == 39.98 + order.total.to_f.should == 39.98 + + subject.remove(variant,1) + order.item_total.to_f.should == 19.99 + order.total.to_f.should == 19.99 + end + + end + +end diff --git a/spec/models/spree/order_inventory_spec.rb b/spec/models/spree/order_inventory_spec.rb new file mode 100644 index 00000000000..5ca8b57f6e5 --- /dev/null +++ b/spec/models/spree/order_inventory_spec.rb @@ -0,0 +1,174 @@ +require 'spec_helper' + +describe Spree::OrderInventory do + let(:order) { create :completed_order_with_totals } + let(:line_item) { order.line_items.first } + subject { described_class.new(order) } + + it 'inventory_units_for should return array of units for a given variant' do + units = subject.inventory_units_for(line_item.variant) + units.map(&:variant_id).should == [line_item.variant.id] + end + + context "when order is missing inventory units" do + before do + line_item.update_column(:quantity, 2) + end + + it 'should be a messed up order' do + order.shipments.first.inventory_units_for(line_item.variant).size.should == 1 + line_item.reload.quantity.should == 2 + end + + it 'should increase the number of inventory units' do + subject.verify(line_item) + order.reload.shipments.first.inventory_units_for(line_item.variant).size.should == 2 + end + end + + context "#add_to_shipment" do + let(:shipment) { order.shipments.first } + let(:variant) { create :variant } + + context "order is not completed" do + before { order.stub completed?: false } + + it "doesn't unstock items" do + shipment.stock_location.should_not_receive(:unstock) + subject.send(:add_to_shipment, shipment, variant, 5).should == 5 + end + end + + it 'should create inventory_units in the necessary states' do + shipment.stock_location.should_receive(:fill_status).with(variant, 5).and_return([3, 2]) + + subject.send(:add_to_shipment, shipment, variant, 5).should == 5 + + units = shipment.inventory_units.group_by &:variant_id + units = units[variant.id].group_by &:state + units['backordered'].size.should == 2 + units['on_hand'].size.should == 3 + end + + it 'should create stock_movement' do + subject.send(:add_to_shipment, shipment, variant, 5).should == 5 + + stock_item = shipment.stock_location.stock_item(variant) + movement = stock_item.stock_movements.last + # movement.originator.should == shipment + movement.quantity.should == -5 + end + end + + context "#determine_target_shipment" do + let(:stock_location) { create :stock_location } + let(:variant) { line_item.variant } + + before do + order.shipments.create(:stock_location_id => stock_location.id) + shipped = order.shipments.create(:stock_location_id => order.shipments.first.stock_location.id) + shipped.update_column(:state, 'shipped') + end + + it 'should select first non-shipped shipment that already contains given variant' do + shipment = subject.send(:determine_target_shipment, variant) + shipment.shipped?.should be_false + shipment.inventory_units_for(variant).should_not be_empty + variant.stock_location_ids.include?(shipment.stock_location_id).should be_true + end + + context "when no shipments already contain this varint" do + it 'selects first non-shipped shipment that leaves from same stock_location' do + subject.send(:remove_from_shipment, order.shipments.first, variant, line_item.quantity) + + shipment = subject.send(:determine_target_shipment, variant) + shipment.reload + shipment.shipped?.should be_false + shipment.inventory_units_for(variant).should be_empty + variant.stock_location_ids.include?(shipment.stock_location_id).should be_true + end + end + end + + context 'when order has too many inventory units' do + before do + line_item.quantity = 3 + line_item.save! + + line_item.update_column(:quantity, 2) + order.reload + end + + it 'should be a messed up order' do + order.shipments.first.inventory_units_for(line_item.variant).size.should == 3 + line_item.quantity.should == 2 + end + + it 'should decrease the number of inventory units' do + subject.verify(line_item) + order.reload.shipments.first.inventory_units_for(line_item.variant).size.should == 2 + end + + context '#remove_from_shipment' do + let(:shipment) { order.shipments.first } + let(:variant) { order.line_items.first.variant } + + context "order is not completed" do + before { order.stub completed?: false } + + it "doesn't restock items" do + shipment.stock_location.should_not_receive(:restock) + subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 + end + end + + it 'should create stock_movement' do + subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 + + stock_item = shipment.stock_location.stock_item(variant) + movement = stock_item.stock_movements.last + # movement.originator.should == shipment + movement.quantity.should == 1 + end + + it 'should destroy backordered units first' do + shipment.stub(:inventory_units_for => [ mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'backordered'), + mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'on_hand'), + mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'backordered') ]) + + shipment.inventory_units_for[0].should_receive(:destroy) + shipment.inventory_units_for[1].should_not_receive(:destroy) + shipment.inventory_units_for[2].should_receive(:destroy) + + subject.send(:remove_from_shipment, shipment, variant, 2).should == 2 + end + + it 'should destroy unshipped units first' do + shipment.stub(:inventory_units_for => [ mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'shipped'), + mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'on_hand') ] ) + + shipment.inventory_units_for[0].should_not_receive(:destroy) + shipment.inventory_units_for[1].should_receive(:destroy) + + subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 + end + + it 'only attempts to destroy as many units as are eligible, and return amount destroyed' do + shipment.stub(:inventory_units_for => [ mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'shipped'), + mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'on_hand') ] ) + + shipment.inventory_units_for[0].should_not_receive(:destroy) + shipment.inventory_units_for[1].should_receive(:destroy) + + subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 + end + + it 'should destroy self if not inventory units remain' do + shipment.inventory_units.stub(:count => 0) + shipment.should_receive(:destroy) + + subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 + end + end + end +end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 5c583a52521..566e579e88f 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -3,6 +3,617 @@ describe Spree::Order do include OpenFoodNetwork::EmailHelper + let(:user) { stub_model(Spree::LegacyUser, :email => "spree@example.com") } + let(:order) { stub_model(Spree::Order, :user => user) } + + before do + Spree::LegacyUser.stub(:current => mock_model(Spree::LegacyUser, :id => 123)) + end + + context "#products" do + before :each do + @variant1 = mock_model(Spree::Variant, :product => "product1") + @variant2 = mock_model(Spree::Variant, :product => "product2") + @line_items = [mock_model(Spree::LineItem, :product => "product1", :variant => @variant1, :variant_id => @variant1.id, :quantity => 1), + mock_model(Spree::LineItem, :product => "product2", :variant => @variant2, :variant_id => @variant2.id, :quantity => 2)] + order.stub(:line_items => @line_items) + end + + it "should return ordered products" do + order.products.should == ['product1', 'product2'] + end + + it "contains?" do + order.contains?(@variant1).should be_true + end + + it "gets the quantity of a given variant" do + order.quantity_of(@variant1).should == 1 + + @variant3 = mock_model(Spree::Variant, :product => "product3") + order.quantity_of(@variant3).should == 0 + end + + it "can find a line item matching a given variant" do + order.find_line_item_by_variant(@variant1).should_not be_nil + order.find_line_item_by_variant(mock_model(Spree::Variant)).should be_nil + end + end + + context "#generate_order_number" do + it "should generate a random string" do + order.generate_order_number.is_a?(String).should be_true + (order.generate_order_number.to_s.length > 0).should be_true + end + end + + context "#associate_user!" do + it "should associate a user with a persisted order" do + order = FactoryGirl.create(:order_with_line_items, created_by: nil) + user = FactoryGirl.create(:user) + + order.user = nil + order.email = nil + order.associate_user!(user) + order.user.should == user + order.email.should == user.email + order.created_by.should == user + + # verify that the changes we made were persisted + order.reload + order.user.should == user + order.email.should == user.email + order.created_by.should == user + end + + it "should not overwrite the created_by if it already is set" do + creator = create(:user) + order = FactoryGirl.create(:order_with_line_items, created_by: creator) + user = FactoryGirl.create(:user) + + order.user = nil + order.email = nil + order.associate_user!(user) + order.user.should == user + order.email.should == user.email + order.created_by.should == creator + + # verify that the changes we made were persisted + order.reload + order.user.should == user + order.email.should == user.email + order.created_by.should == creator + end + + + it "should associate a user with a non-persisted order" do + order = Spree::Order.new + + expect do + order.associate_user!(user) + end.to change { [order.user, order.email] }.from([nil, nil]).to([user, user.email]) + end + + it "should not persist an invalid address" do + address = Spree::Address.new + order.user = nil + order.email = nil + order.ship_address = address + expect do + order.associate_user!(user) + end.not_to change { address.persisted? }.from(false) + end + end + + context "#create" do + it "should assign an order number" do + order = Spree::Order.create + order.number.should_not be_nil + end + end + + context "#can_ship?" do + let(:order) { Spree::Order.create } + + it "should be true for order in the 'complete' state" do + order.stub(:complete? => true) + order.can_ship?.should be_true + end + + it "should be true for order in the 'resumed' state" do + order.stub(:resumed? => true) + order.can_ship?.should be_true + end + + it "should be true for an order in the 'awaiting return' state" do + order.stub(:awaiting_return? => true) + order.can_ship?.should be_true + end + + it "should be true for an order in the 'returned' state" do + order.stub(:returned? => true) + order.can_ship?.should be_true + end + + it "should be false if the order is neither in the 'complete' nor 'resumed' state" do + order.stub(:resumed? => false, :complete? => false) + order.can_ship?.should be_false + end + end + + context "checking if order is paid" do + context "payment_state is paid" do + before { order.stub payment_state: 'paid' } + it { expect(order).to be_paid } + end + + context "payment_state is credit_owned" do + before { order.stub payment_state: 'credit_owed' } + it { expect(order).to be_paid } + end + end + + context "#finalize!" do + let(:order) { Spree::Order.create } + it "should set completed_at" do + order.should_receive(:touch).with(:completed_at) + order.finalize! + end + + it "should sell inventory units" do + order.shipments.each do |shipment| + shipment.should_receive(:update!) + shipment.should_receive(:finalize!) + end + order.finalize! + end + + it "should decrease the stock for each variant in the shipment" do + order.shipments.each do |shipment| + shipment.stock_location.should_receive(:decrease_stock_for_variant) + end + order.finalize! + end + + it "should change the shipment state to ready if order is paid" do + Spree::Shipment.create(order: order) + order.shipments.reload + + order.stub(:paid? => true, :complete? => true) + order.finalize! + order.reload # reload so we're sure the changes are persisted + order.shipment_state.should == 'ready' + end + + after { Spree::Config.set :track_inventory_levels => true } + it "should not sell inventory units if track_inventory_levels is false" do + Spree::Config.set :track_inventory_levels => false + Spree::InventoryUnit.should_not_receive(:sell_units) + order.finalize! + end + + it "should send an order confirmation email" do + mail_message = double "Mail::Message" + Spree::OrderMailer.should_receive(:confirm_email).with(order.id).and_return mail_message + mail_message.should_receive :deliver + order.finalize! + end + + it "should continue even if confirmation email delivery fails" do + Spree::OrderMailer.should_receive(:confirm_email).with(order.id).and_raise 'send failed!' + order.finalize! + end + + it "should freeze all adjustments" do + # Stub this method as it's called due to a callback + # and it's irrelevant to this test + order.stub :has_available_shipment + Spree::OrderMailer.stub_chain :confirm_email, :deliver + adjustments = double + order.stub :adjustments => adjustments + expect(adjustments).to receive(:update_all).with(state: 'closed') + order.finalize! + end + + it "should log state event" do + order.state_changes.should_receive(:create).exactly(3).times #order, shipment & payment state changes + order.finalize! + end + + it 'calls updater#before_save' do + order.updater.should_receive(:before_save_hook) + order.finalize! + end + end + + context "#process_payments!" do + let(:payment) { stub_model(Spree::Payment) } + before { order.stub :pending_payments => [payment], :total => 10 } + + it "should process the payments" do + payment.should_receive(:process!) + order.process_payments!.should be_true + end + + it "should return false if no pending_payments available" do + order.stub :pending_payments => [] + order.process_payments!.should be_false + end + + context "when a payment raises a GatewayError" do + before { payment.should_receive(:process!).and_raise(Spree::Core::GatewayError) } + + it "should return true when configured to allow checkout on gateway failures" do + Spree::Config.set :allow_checkout_on_gateway_error => true + order.process_payments!.should be_true + end + + it "should return false when not configured to allow checkout on gateway failures" do + Spree::Config.set :allow_checkout_on_gateway_error => false + order.process_payments!.should be_false + end + + 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 + order.outstanding_balance.should == 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 + order.outstanding_balance.should 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 + order.outstanding_balance?.should be_true + end + it "should be true when total less than payment_total" do + order.total = 8.25 + order.payment_total = 10.44 + order.outstanding_balance?.should be_true + end + it "should be false when total equals payment_total" do + order.total = 10.10 + order.payment_total = 10.10 + order.outstanding_balance?.should be_false + end + end + + context "#completed?" do + it "should indicate if order is completed" do + order.completed_at = nil + order.completed?.should be_false + + order.completed_at = Time.now + order.completed?.should be_true + end + end + + it 'is backordered if one of the shipments is backordered' do + order.stub(:shipments => [mock_model(Spree::Shipment, :backordered? => false), + mock_model(Spree::Shipment, :backordered? => true)]) + order.should be_backordered + end + + context "#allow_checkout?" do + it "should be true if there are line_items in the order" do + order.stub_chain(:line_items, :count => 1) + order.checkout_allowed?.should be_true + end + it "should be false if there are no line_items in the order" do + order.stub_chain(:line_items, :count => 0) + order.checkout_allowed?.should be_false + end + end + + context "#item_count" do + before do + @order = create(:order, :user => user) + @order.line_items = [ create(:line_item, :quantity => 2), create(:line_item, :quantity => 1) ] + end + it "should return the correct number of items" do + @order.item_count.should == 3 + end + end + + context "#amount" do + before do + @order = create(:order, :user => user) + @order.line_items = [create(:line_item, :price => 1.0, :quantity => 2), + create(:line_item, :price => 1.0, :quantity => 1)] + end + it "should return the correct lum sum of items" do + @order.amount.should == 3.0 + end + end + + context "#can_cancel?" do + it "should be false for completed order in the canceled state" do + order.state = 'canceled' + order.shipment_state = 'ready' + order.completed_at = Time.now + order.can_cancel?.should be_false + end + + it "should be true for completed order with no shipment" do + order.state = 'complete' + order.shipment_state = nil + order.completed_at = Time.now + order.can_cancel?.should be_true + end + end + + context "insufficient_stock_lines" do + let(:line_item) { mock_model Spree::LineItem, :insufficient_stock? => true } + + before { order.stub(:line_items => [line_item]) } + + it "should return line_item that has insufficient stock on hand" do + order.insufficient_stock_lines.size.should == 1 + order.insufficient_stock_lines.include?(line_item).should be_true + end + + end + + context "empty!" do + it "should clear out all line items and adjustments" do + order = stub_model(Spree::Order) + order.stub(:line_items => line_items = []) + order.stub(:adjustments => adjustments = []) + order.line_items.should_receive(:destroy_all) + order.adjustments.should_receive(:destroy_all) + + order.empty! + end + end + + context "#display_outstanding_balance" do + it "returns the value as a spree money" do + order.stub(:outstanding_balance) { 10.55 } + order.display_outstanding_balance.should == Spree::Money.new(10.55) + end + end + + context "#display_item_total" do + it "returns the value as a spree money" do + order.stub(:item_total) { 10.55 } + order.display_item_total.should == Spree::Money.new(10.55) + end + end + + context "#display_adjustment_total" do + it "returns the value as a spree money" do + order.adjustment_total = 10.55 + order.display_adjustment_total.should == Spree::Money.new(10.55) + end + end + + context "#display_total" do + it "returns the value as a spree money" do + order.total = 10.55 + order.display_total.should == Spree::Money.new(10.55) + end + end + + context "#currency" do + context "when object currency is ABC" do + before { order.currency = "ABC" } + + it "returns the currency from the object" do + order.currency.should == "ABC" + end + end + + context "when object currency is nil" do + before { order.currency = nil } + + it "returns the globally configured currency" do + order.currency.should == "USD" + end + end + end + + # Regression tests for Spree #2179 + context "#merge!" do + let(:variant) { create(:variant) } + let(:order_1) { Spree::Order.create } + let(:order_2) { Spree::Order.create } + + it "destroys the other order" do + order_1.merge!(order_2) + lambda { order_2.reload }.should raise_error(ActiveRecord::RecordNotFound) + end + + context "merging together two orders with line items for the same variant" do + before do + order_1.contents.add(variant, 1) + order_2.contents.add(variant, 1) + end + + specify do + order_1.merge!(order_2) + order_1.line_items.count.should == 1 + + line_item = order_1.line_items.first + line_item.quantity.should == 2 + line_item.variant_id.should == variant.id + end + end + + context "merging together two orders with different line items" do + let(:variant_2) { create(:variant) } + + before do + order_1.contents.add(variant, 1) + order_2.contents.add(variant_2, 1) + end + + specify do + order_1.merge!(order_2) + line_items = order_1.line_items + line_items.count.should == 2 + + # No guarantee on ordering of line items, so we do this: + line_items.pluck(:quantity).should =~ [1, 1] + line_items.pluck(:variant_id).should =~ [variant.id, variant_2.id] + end + end + end + + context "#confirmation_required?" do + it "does not bomb out when an order has an unpersisted payment" do + order = Spree::Order.new + order.payments.build + assert !order.confirmation_required? + end + end + + # Regression test for Spree #2191 + context "when an order has an adjustment that zeroes the total, but another adjustment for shipping that raises it above zero" do + let!(:persisted_order) { create(:order) } + let!(:line_item) { create(:line_item) } + let!(:shipping_method) do + sm = create(:shipping_method) + sm.calculator.preferred_amount = 10 + sm.save + sm + end + + before do + # Don't care about available payment methods in this test + persisted_order.stub(:has_available_payment => false) + persisted_order.line_items << line_item + persisted_order.adjustments.create(:amount => -line_item.amount, :label => "Promotion") + persisted_order.state = 'delivery' + persisted_order.save # To ensure new state_change event + end + + it "transitions from delivery to payment" do + persisted_order.stub(payment_required?: true) + persisted_order.next! + persisted_order.state.should == "payment" + end + end + + context "promotion adjustments" do + let(:originator) { double("Originator", id: 1) } + let(:adjustment) { double("Adjustment", originator: originator) } + + before { order.stub_chain(:adjustments, :includes, :promotion, reload: [adjustment]) } + + context "order has an adjustment from given promo action" do + it { expect(order.promotion_credit_exists? originator).to be_true } + end + + context "order has no adjustment from given promo action" do + before { originator.stub(id: 12) } + it { expect(order.promotion_credit_exists? originator).to be_true } + end + end + + context "payment required?" do + let(:order) { Spree::Order.new } + + context "total is zero" do + it { order.payment_required?.should be_false } + end + + context "total > zero" do + before { order.stub(total: 1) } + it { order.payment_required?.should be_true } + end + end + + context "add_update_hook" do + before do + Spree::Order.class_eval do + register_update_hook :add_awesome_sauce + end + end + + after do + Spree::Order.update_hooks = Set.new + end + + it "calls hook during update" do + order = create(:order) + order.should_receive(:add_awesome_sauce) + order.update! + end + + it "calls hook during finalize" do + order = create(:order) + order.should_receive(:add_awesome_sauce) + order.finalize! + end + end + + context "ensure shipments will be updated" do + before { Spree::Shipment.create!(order: order) } + + it "destroys current shipments" do + order.ensure_updated_shipments + expect(order.shipments).to be_empty + end + + it "puts order back in address state" do + order.ensure_updated_shipments + expect(order.state).to eql "address" + end + end + + describe ".tax_address" do + before { Spree::Config[:tax_using_ship_address] = tax_using_ship_address } + subject { order.tax_address } + + context "when tax_using_ship_address is true" do + let(:tax_using_ship_address) { true } + + it 'returns ship_address' do + subject.should == order.ship_address + end + end + + context "when tax_using_ship_address is not true" do + let(:tax_using_ship_address) { false } + + it "returns bill_address" do + subject.should == order.bill_address + end + end + end + + context '#updater' do + class FakeOrderUpdaterDecorator + attr_reader :decorated_object + + def initialize(decorated_object) + @decorated_object = decorated_object + end + end + + before do + Spree::Config.stub(:order_updater_decorator) { FakeOrderUpdaterDecorator } + end + + it 'returns an order_updater_decorator class' do + order.updater.class.should == FakeOrderUpdaterDecorator + end + + it 'decorates a Spree::OrderUpdater' do + order.updater.decorated_object.class.should == Spree::OrderUpdater + end + end + describe "email validation" do let(:order) { build(:order) } diff --git a/spec/models/spree/return_authorization_spec.rb b/spec/models/spree/return_authorization_spec.rb new file mode 100644 index 00000000000..f76854ec8b3 --- /dev/null +++ b/spec/models/spree/return_authorization_spec.rb @@ -0,0 +1,138 @@ +require 'spec_helper' + +describe Spree::ReturnAuthorization do + let(:stock_location) {Spree::StockLocation.create(:name => "test")} + let(:order) { FactoryGirl.create(:shipped_order)} + let(:variant) { order.shipments.first.inventory_units.first.variant } + let(:return_authorization) { Spree::ReturnAuthorization.new(:order => order, :stock_location_id => stock_location.id) } + + context "save" do + it "should be invalid when order has no inventory units" do + order.shipments.destroy_all + return_authorization.save + return_authorization.errors[:order].should == ["has no shipped units"] + end + + it "should generate RMA number" do + return_authorization.should_receive(:generate_number) + return_authorization.save + end + end + + context "add_variant" do + context "on empty rma" do + it "should associate inventory unit" do + return_authorization.add_variant(variant.id, 1) + return_authorization.inventory_units.size.should == 1 + end + + it "should associate inventory units as shipped" do + return_authorization.add_variant(variant.id, 1) + expect(return_authorization.inventory_units.where(state: 'shipped').size).to eq 1 + end + + it "should update order state" do + order.should_receive(:authorize_return!) + return_authorization.add_variant(variant.id, 1) + end + end + + context "on rma that already has inventory_units" do + before do + return_authorization.add_variant(variant.id, 1) + end + + it "should not associate more inventory units than there are on the order" do + return_authorization.add_variant(variant.id, 1) + expect(return_authorization.inventory_units.size).to eq 1 + end + + it "should not update order state" do + expect{return_authorization.add_variant(variant.id, 1)}.to_not change{order.state} + end + + end + + end + + context "can_receive?" do + it "should allow_receive when inventory units assigned" do + return_authorization.stub(:inventory_units => [1,2,3]) + return_authorization.can_receive?.should be_true + end + + it "should not allow_receive with no inventory units" do + return_authorization.stub(:inventory_units => []) + return_authorization.can_receive?.should be_false + end + end + + context "receive!" do + let(:inventory_unit) { order.shipments.first.inventory_units.first } + + before do + return_authorization.stub(:inventory_units => [inventory_unit], :amount => -20) + Spree::Adjustment.stub(:create) + order.stub(:update!) + end + + it "should mark all inventory units are returned" do + inventory_unit.should_receive(:return!) + return_authorization.receive! + end + + it "should add credit for specified amount" do + return_authorization.amount = 20 + mock_adjustment = double + mock_adjustment.should_receive(:source=).with(return_authorization) + mock_adjustment.should_receive(:adjustable=).with(order) + mock_adjustment.should_receive(:save) + Spree::Adjustment.should_receive(:new).with(:amount => -20, :label => Spree.t(:rma_credit)).and_return(mock_adjustment) + return_authorization.receive! + end + + it "should update order state" do + order.should_receive :update! + return_authorization.receive! + end + end + + context "force_positive_amount" do + it "should ensure the amount is always positive" do + return_authorization.amount = -10 + return_authorization.send :force_positive_amount + return_authorization.amount.should == 10 + end + end + + context "after_save" do + it "should run correct callbacks" do + return_authorization.should_receive(:force_positive_amount) + return_authorization.run_callbacks(:save) + end + end + + context "currency" do + before { order.stub(:currency) { "ABC" } } + it "returns the order currency" do + return_authorization.currency.should == "ABC" + end + end + + context "display_amount" do + it "returns a Spree::Money" do + return_authorization.amount = 21.22 + return_authorization.display_amount.should == Spree::Money.new(21.22) + end + end + + context "returnable_inventory" do + pending "should return inventory from shipped shipments" do + return_authorization.returnable_inventory.should == [inventory_unit] + end + + pending "should not return inventory from unshipped shipments" do + return_authorization.returnable_inventory.should == [] + end + end +end From 6900f7a46fc00ac3392e9fb3111f24318714d4af Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 15:26:56 +0100 Subject: [PATCH 02/21] Merge decorators with original files from spree_core EPIC COMMIT ALERT :-) --- app/models/spree/line_item.rb | 170 ++++++++- app/models/spree/line_item_decorator.rb | 185 ---------- app/models/spree/order.rb | 421 +++++++++++++++++++++- app/models/spree/order_decorator.rb | 446 ------------------------ 4 files changed, 572 insertions(+), 650 deletions(-) delete mode 100644 app/models/spree/line_item_decorator.rb delete mode 100644 app/models/spree/order_decorator.rb diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index 2e160ba018e..c84a3727db0 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -1,13 +1,21 @@ +require 'open_food_network/scope_variant_to_hub' +require 'variant_units/variant_and_line_item_naming' + module Spree class LineItem < ActiveRecord::Base - before_validation :adjust_quantity - belongs_to :order, class_name: "Spree::Order" + include VariantUnits::VariantAndLineItemNaming + include LineItemBasedAdjustmentHandling + + belongs_to :order, class_name: "Spree::Order", inverse_of: :line_items belongs_to :variant, class_name: "Spree::Variant" belongs_to :tax_category, class_name: "Spree::TaxCategory" has_one :product, through: :variant has_many :adjustments, as: :adjustable, dependent: :destroy + has_and_belongs_to_many :option_values, join_table: 'spree_option_values_line_items', class_name: 'Spree::OptionValue' + + before_validation :adjust_quantity before_validation :copy_price before_validation :copy_tax_category @@ -21,12 +29,73 @@ class LineItem < ActiveRecord::Base validates_with Stock::AvailabilityValidator before_save :update_inventory - + before_save :calculate_final_weight_volume, if: :quantity_changed?, unless: :final_weight_volume_changed? after_save :update_order + after_save :update_units + before_destroy :update_inventory_before_destroy after_destroy :update_order + delegate :product, :unit_description, :display_name, to: :variant + + attr_accessor :skip_stock_check # Allows manual skipping of Stock::AvailabilityValidator attr_accessor :target_shipment + # -- Scopes + scope :managed_by, lambda { |user| + if user.has_spree_role?('admin') + where(nil) + else + # Find line items that are from orders distributed by the user or supplied by the user + joins(variant: :product). + joins(:order). + where('spree_orders.distributor_id IN (?) OR spree_products.supplier_id IN (?)', user.enterprises, user.enterprises). + select('spree_line_items.*') + end + } + + scope :in_orders, lambda { |orders| + where(order_id: orders) + } + + # Find line items that are from order sorted by variant name and unit value + scope :sorted_by_name_and_unit_value, -> { + joins(variant: :product). + reorder(" + lower(spree_products.name) asc, + lower(spree_variants.display_name) asc, + spree_variants.unit_value asc") + } + + scope :from_order_cycle, lambda { |order_cycle| + joins(order: :order_cycle). + where('order_cycles.id = ?', order_cycle) + } + + # Here we are simply joining the line item to its variant and product + # We dont use joins here to avoid the default scopes, + # and with that, include deleted variants and deleted products + scope :supplied_by_any, lambda { |enterprises| + product_ids = Spree::Product.unscoped.where(supplier_id: enterprises).select(:id) + variant_ids = Spree::Variant.unscoped.where(product_id: product_ids).select(:id) + where("spree_line_items.variant_id IN (?)", variant_ids) + } + + scope :with_tax, -> { + joins(:adjustments). + where('spree_adjustments.originator_type = ?', 'Spree::TaxRate'). + select('DISTINCT spree_line_items.*') + } + + # Line items without a Spree::TaxRate-originated adjustment + scope :without_tax, -> { + joins(" + LEFT OUTER JOIN spree_adjustments + ON (spree_adjustments.adjustable_id=spree_line_items.id + AND spree_adjustments.adjustable_type = 'Spree::LineItem' + AND spree_adjustments.originator_type='Spree::TaxRate')"). + where('spree_adjustments.id IS NULL') + } + def copy_price if variant self.price = variant.price if price.nil? @@ -61,8 +130,15 @@ def adjust_quantity self.quantity = 0 if quantity.nil? || quantity < 0 end + # Here we skip stock check if skip_stock_check flag is active, + # we skip stock check if requested quantity is zero or negative, + # and we scope variants to hub and thus acivate variant overrides. def sufficient_stock? - Stock::Quantifier.new(variant_id).can_supply? quantity + return true if skip_stock_check + return true if quantity <= 0 + + scoper.scope(variant) + variant.can_supply?(quantity) end def insufficient_stock? @@ -78,14 +154,74 @@ def product variant.product end - # Remove variant default_scope `deleted_at: nil` + # This ensures that LineItems always have access to soft-deleted variants. + # In some situations, unscoped super will be nil. In these cases, + # we fetch the variant using variant_id. See issue #4946 for more details. def variant - Spree::Variant.unscoped { super } + Spree::Variant.unscoped { super } || Spree::Variant.unscoped.find(variant_id) + end + + def cap_quantity_at_stock! + scoper.scope(variant) + return if variant.on_demand + + update!(quantity: variant.on_hand) if quantity > variant.on_hand + end + + def has_tax? + adjustments.included_tax.any? + end + + def included_tax + adjustments.included_tax.sum(&:included_tax) + end + + def tax_rates + product.tax_category.andand.tax_rates || [] + end + + def price_with_adjustments + # EnterpriseFee#create_adjustment applies adjustments on line items to their parent order, + # so line_item.adjustments returns an empty array + return 0 if quantity.zero? + + line_item_adjustments = OrderAdjustmentsFetcher.new(order).line_item_adjustments(self) + + (price + line_item_adjustments.sum(&:amount) / quantity).round(2) + end + + def single_display_amount_with_adjustments + Spree::Money.new(price_with_adjustments, currency: currency) + end + + def amount_with_adjustments + # We calculate from price_with_adjustments here rather than building our own value because + # rounding errors can produce discrepencies of $0.01. + price_with_adjustments * quantity + end + + def display_amount_with_adjustments + Spree::Money.new(amount_with_adjustments, currency: currency) + end + + def display_included_tax + Spree::Money.new(included_tax, currency: currency) + end + + def unit_value + return variant.unit_value if quantity == 0 || !final_weight_volume + + final_weight_volume / quantity + end + + def scoper + @scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(order.distributor) end private def update_inventory if changed? + scoper.scope(variant) Spree::OrderInventory.new(self.order).verify(self, target_shipment) end end @@ -97,6 +233,26 @@ def update_order order.update! end end + + def update_inventory_before_destroy + # This is necessary before destroying the line item + # so that update_inventory will restore stock to the variant + self.quantity = 0 + + update_inventory + + # This is necessary after updating inventory + # because update_inventory may delete the last shipment in the order + # and that makes update_order fail if we don't reload the shipments + order.shipments.reload + end + + def calculate_final_weight_volume + if final_weight_volume.present? && quantity_was > 0 + self.final_weight_volume = final_weight_volume * quantity / quantity_was + elsif variant.andand.unit_value.present? + self.final_weight_volume = variant.andand.unit_value * quantity + end + end end end - diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb deleted file mode 100644 index 558b2efc59d..00000000000 --- a/app/models/spree/line_item_decorator.rb +++ /dev/null @@ -1,185 +0,0 @@ -require 'open_food_network/scope_variant_to_hub' -require 'variant_units/variant_and_line_item_naming' - -Spree::LineItem.class_eval do - include VariantUnits::VariantAndLineItemNaming - include LineItemBasedAdjustmentHandling - has_and_belongs_to_many :option_values, join_table: 'spree_option_values_line_items', class_name: 'Spree::OptionValue' - - # Redefining here to add the inverse_of option - belongs_to :order, class_name: "Spree::Order", inverse_of: :line_items - - # Allows manual skipping of Stock::AvailabilityValidator - attr_accessor :skip_stock_check - - before_save :calculate_final_weight_volume, if: :quantity_changed?, unless: :final_weight_volume_changed? - after_save :update_units - - before_destroy :update_inventory_before_destroy - - delegate :product, :unit_description, to: :variant - - # -- Scopes - scope :managed_by, lambda { |user| - if user.has_spree_role?('admin') - where(nil) - else - # Find line items that are from orders distributed by the user or supplied by the user - joins(variant: :product). - joins(:order). - where('spree_orders.distributor_id IN (?) OR spree_products.supplier_id IN (?)', user.enterprises, user.enterprises). - select('spree_line_items.*') - end - } - - scope :in_orders, lambda { |orders| - where(order_id: orders) - } - - # Find line items that are from order sorted by variant name and unit value - scope :sorted_by_name_and_unit_value, -> { - joins(variant: :product). - reorder(" - lower(spree_products.name) asc, - lower(spree_variants.display_name) asc, - spree_variants.unit_value asc") - } - - scope :from_order_cycle, lambda { |order_cycle| - joins(order: :order_cycle). - where('order_cycles.id = ?', order_cycle) - } - - # Here we are simply joining the line item to its variant and product - # We dont use joins here to avoid the default scopes, - # and with that, include deleted variants and deleted products - scope :supplied_by_any, lambda { |enterprises| - product_ids = Spree::Product.unscoped.where(supplier_id: enterprises).select(:id) - variant_ids = Spree::Variant.unscoped.where(product_id: product_ids).select(:id) - where("spree_line_items.variant_id IN (?)", variant_ids) - } - - scope :with_tax, -> { - joins(:adjustments). - where('spree_adjustments.originator_type = ?', 'Spree::TaxRate'). - select('DISTINCT spree_line_items.*') - } - - # Line items without a Spree::TaxRate-originated adjustment - scope :without_tax, -> { - joins(" - LEFT OUTER JOIN spree_adjustments - ON (spree_adjustments.adjustable_id=spree_line_items.id - AND spree_adjustments.adjustable_type = 'Spree::LineItem' - AND spree_adjustments.originator_type='Spree::TaxRate')"). - where('spree_adjustments.id IS NULL') - } - - # Overridden so that LineItems always have access to soft-deleted Variant - # attributes. In some situations, unscoped super will be nil, in these cases - # we fetch the variant using the variant_id. See isssue #4946 for more - # details - def variant - Spree::Variant.unscoped { super } || Spree::Variant.unscoped.find(variant_id) - end - - def cap_quantity_at_stock! - scoper.scope(variant) - return if variant.on_demand - - update!(quantity: variant.on_hand) if quantity > variant.on_hand - end - - def has_tax? - adjustments.included_tax.any? - end - - def included_tax - adjustments.included_tax.sum(&:included_tax) - end - - def tax_rates - product.tax_category.andand.tax_rates || [] - end - - def price_with_adjustments - # EnterpriseFee#create_adjustment applies adjustments on line items to their parent order, - # so line_item.adjustments returns an empty array - return 0 if quantity.zero? - - line_item_adjustments = OrderAdjustmentsFetcher.new(order).line_item_adjustments(self) - - (price + line_item_adjustments.sum(&:amount) / quantity).round(2) - end - - def single_display_amount_with_adjustments - Spree::Money.new(price_with_adjustments, currency: currency) - end - - def amount_with_adjustments - # We calculate from price_with_adjustments here rather than building our own value because - # rounding errors can produce discrepencies of $0.01. - price_with_adjustments * quantity - end - - def display_amount_with_adjustments - Spree::Money.new(amount_with_adjustments, currency: currency) - end - - def display_included_tax - Spree::Money.new(included_tax, currency: currency) - end - - delegate :display_name, to: :variant - - def unit_value - return variant.unit_value if quantity == 0 || !final_weight_volume - - final_weight_volume / quantity - end - - # Overrides Spree version to: - # - skip stock check if skip_stock_check flag is active - # - skip stock check if requested quantity is zero or negative - # - scope variants to hub and thus acivate variant overrides - def sufficient_stock? - return true if skip_stock_check - return true if quantity <= 0 - - scoper.scope(variant) - variant.can_supply?(quantity) - end - - def scoper - @scoper ||= OpenFoodNetwork::ScopeVariantToHub.new(order.distributor) - end - - private - - def update_inventory_with_scoping - scoper.scope(variant) - update_inventory_without_scoping - end - alias_method_chain :update_inventory, :scoping - - def update_inventory_before_destroy - # This is necessary before destroying the line item - # so that update_inventory will restore stock to the variant - self.quantity = 0 - - update_inventory - - # This is necessary after updating inventory - # because update_inventory may delete the last shipment in the order - # and that makes update_order fail if we don't reload the shipments - order.shipments.reload - end - - def calculate_final_weight_volume - if final_weight_volume.present? && quantity_was > 0 - self.final_weight_volume = final_weight_volume * quantity / quantity_was - elsif variant.andand.unit_value.present? - self.final_weight_volume = variant.andand.unit_value * quantity - end - end -end diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index b1056f3479f..bfafea0ec4f 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -1,8 +1,17 @@ require 'spree/core/validators/email' require 'spree/order/checkout' +require 'open_food_network/enterprise_fee_calculator' +require 'open_food_network/feature_toggle' +require 'open_food_network/tag_rule_applicator' +require 'concerns/order_shipment' + +ActiveSupport::Notifications.subscribe('spree.order.contents_changed') do |_name, _start, _finish, _id, payload| + payload[:order].reload.update_distribution_charge! +end module Spree class Order < ActiveRecord::Base + prepend OrderShipment include Checkout checkout_flow do @@ -17,6 +26,17 @@ class Order < ActiveRecord::Base remove_transition from: :delivery, to: :confirm end + # Orders are confirmed with their payment, we don't use the confirm step. + # Here we remove that step from Spree's checkout state machine. + # See: https://guides.spreecommerce.org/developer/checkout.html#modifying-the-checkout-flow + remove_checkout_step :confirm + + state_machine.after_transition to: :payment, do: :charge_shipping_and_payment_fees! + + state_machine.event :restart_checkout do + transition to: :cart, unless: :completed? + end + token_resource attr_reader :coupon_code @@ -39,7 +59,10 @@ class Order < ActiveRecord::Base has_many :line_items, -> { order('created_at ASC') }, dependent: :destroy has_many :payments, dependent: :destroy has_many :return_authorizations, dependent: :destroy - has_many :adjustments, -> { order("#{Adjustment.table_name}.created_at ASC") }, as: :adjustable, dependent: :destroy + has_many :adjustments, -> { order "#{Spree::Adjustment.table_name}.created_at ASC" }, + as: :adjustable, + dependent: :destroy + has_many :line_item_adjustments, through: :line_items, source: :adjustments has_many :shipments, dependent: :destroy do @@ -48,16 +71,31 @@ def states end end + belongs_to :order_cycle + belongs_to :distributor, class_name: 'Enterprise' + belongs_to :customer + has_one :proxy_order + has_one :subscription, through: :proxy_order + accepts_nested_attributes_for :line_items accepts_nested_attributes_for :bill_address accepts_nested_attributes_for :ship_address accepts_nested_attributes_for :payments accepts_nested_attributes_for :shipments + delegate :admin_and_handling_total, :payment_fee, :ship_total, to: :adjustments_fetcher + # Needs to happen before save_permalink is called before_validation :set_currency before_validation :generate_order_number, on: :create before_validation :clone_billing_address, if: :use_billing? + before_validation :associate_customer, unless: :customer_id? + before_validation :ensure_customer, unless: :customer_is_valid? + + validates :customer, presence: true, if: :require_customer? + validate :products_available_from_new_distribution, if: lambda { distributor_id_changed? || order_cycle_id_changed? } + validate :disallow_guest_order + attr_accessor :use_billing before_create :link_by_email @@ -68,11 +106,57 @@ def states validate :has_available_shipment validate :has_available_payment + # The EmailValidator introduced in Spree 2.1 is not working + # So here we remove it and re-introduce the regexp validation rule from Spree 2.0 + _validate_callbacks.each do |callback| + if callback.raw_filter.respond_to? :attributes + callback.raw_filter.attributes.delete :email + end + end + validates :email, presence: true, format: /\A([\w\.%\+\-']+)@([\w\-]+\.)+([\w]{2,})\z/i, + if: :require_email + make_permalink field: :number + before_save :update_shipping_fees!, if: :complete? + before_save :update_payment_fees!, if: :complete? + class_attribute :update_hooks self.update_hooks = Set.new + # -- Scopes + scope :managed_by, lambda { |user| + if user.has_spree_role?('admin') + where(nil) + else + # Find orders that are distributed by the user or have products supplied by the user + # WARNING: This only filters orders, you'll need to filter line items separately using LineItem.managed_by + with_line_items_variants_and_products_outer. + where('spree_orders.distributor_id IN (?) OR spree_products.supplier_id IN (?)', + user.enterprises.select(&:id), + user.enterprises.select(&:id)). + select('DISTINCT spree_orders.*') + end + } + + scope :distributed_by_user, lambda { |user| + if user.has_spree_role?('admin') + where(nil) + else + where('spree_orders.distributor_id IN (?)', user.enterprises.select(&:id)) + end + } + + scope :with_line_items_variants_and_products_outer, lambda { + joins('LEFT OUTER JOIN spree_line_items ON (spree_line_items.order_id = spree_orders.id)'). + joins('LEFT OUTER JOIN spree_variants ON (spree_variants.id = spree_line_items.variant_id)'). + joins('LEFT OUTER JOIN spree_products ON (spree_products.id = spree_variants.product_id)') + } + + scope :not_state, lambda { |state| + where("state != ?", state) + } + def self.by_number(number) where(number: number) end @@ -152,9 +236,16 @@ def checkout_allowed? line_items.count > 0 end + def changes_allowed? + complete? && distributor.andand.allow_order_changes? && order_cycle.andand.open? + end + # Is this a free order in which case the payment step should be skipped + # This allows unpaid subscription orders to be completed. + # Subscriptions place orders at the beginning of an order cycle. They need to + # be completed to draw from stock levels and trigger emails. def payment_required? - total.to_f > 0.0 + total.to_f > 0.0 && !skip_payment_for_subscription? end # If true, causes the confirmation step to happen during the checkout process @@ -201,9 +292,7 @@ def line_item_adjustment_totals end def updater - @updater ||= Spree::Config.order_updater_decorator.new( - Spree::OrderUpdater.new(self) - ) + @updater ||= OrderManagement::Order::Updater.new(self) end def update! @@ -239,10 +328,68 @@ def awaiting_returns? return_authorizations.any? { |return_authorization| return_authorization.authorized? } end + # This is currently used when adding a variant to an order in the BackOffice. + # Spree::OrderContents#add is equivalent but slightly different from add_variant below. def contents @contents ||= Spree::OrderContents.new(self) end + # This is currently used when adding a variant to an order in the FrontOffice. + # This add_variant is equivalent but slightly different from Spree::OrderContents#add above. + # Spree::OrderContents#add is the more modern version in Spree history + # but this add_variant has been customized for OFN FrontOffice. + def add_variant(variant, quantity = 1, max_quantity = nil, currency = nil) + line_items(:reload) + current_item = find_line_item_by_variant(variant) + + # Notify bugsnag if we get line items with a quantity of zero + if quantity == 0 + Bugsnag.notify(RuntimeError.new("Zero Quantity Line Item"), + current_item: current_item.as_json, + line_items: line_items.map(&:id), + variant: variant.as_json) + end + + if current_item + current_item.quantity = quantity + current_item.max_quantity = max_quantity + + # This is the original behaviour, behaviour above is so that we can resolve the order populator bug + # current_item.quantity ||= 0 + # current_item.max_quantity ||= 0 + # current_item.quantity += quantity.to_i + # current_item.max_quantity += max_quantity.to_i + current_item.currency = currency unless currency.nil? + current_item.save + else + current_item = Spree::LineItem.new(quantity: quantity, max_quantity: max_quantity) + current_item.variant = variant + if currency + current_item.currency = currency unless currency.nil? + current_item.price = variant.price_in(currency).amount + else + current_item.price = variant.price + end + line_items << current_item + end + + reload + current_item + end + + def set_variant_attributes(variant, attributes) + line_item = find_line_item_by_variant(variant) + + if line_item + if attributes.key?(:max_quantity) && attributes[:max_quantity].to_i < line_item.quantity + attributes[:max_quantity] = line_item.quantity + end + + line_item.assign_attributes(attributes) + line_item.save! + end + end + # Associates the specified user with the order. def associate_user!(user) self.user = user @@ -351,11 +498,8 @@ def finalize! end def deliver_order_confirmation_email - begin - OrderMailer.confirm_email(self.id).deliver - rescue Exception => e - logger.error("#{e.class.name}: #{e.message}") - logger.error(e.backtrace * "\n") + if subscription.blank? + Delayed::Job.enqueue ConfirmOrderJob.new(id) end end @@ -368,8 +512,10 @@ def available_payment_methods @available_payment_methods ||= PaymentMethod.available(:front_end) end + # "Checkout" is the initial state and, for card payments, "pending" is the state after authorization + # These are both valid states to process the payment def pending_payments - payments.select(&:checkout?) + (payments.select(&:pending?) + payments.select(&:processing?) + payments.select(&:checkout?)).uniq end # processes any pending payments and must return a boolean as it's @@ -444,6 +590,8 @@ def merge!(order) def empty! line_items.destroy_all adjustments.destroy_all + payments.clear + shipments.destroy_all end def clear_adjustments! @@ -490,11 +638,38 @@ def shipped? %w(partial shipped).include?(shipment_state) end + # Does this order have shipments that can be shipped? + def ready_to_ship? + shipments.any?(&:can_ship?) + end + + # Ship all pending orders + def ship + shipments.each do |s| + s.ship if s.can_ship? + end + end + + def line_item_variants + if line_items.loaded? + line_items.map(&:variant) + else + line_items.includes(:variant).map(&:variant) + end + end + + # Show already bought line items of this order cycle + def finalised_line_items + return [] unless order_cycle && user && distributor + + order_cycle.items_bought_by_user(user, distributor) + end + def create_proposed_shipments adjustments.shipping.delete_all shipments.destroy_all - packages = Spree::Stock::Coordinator.new(self).packages + packages = OrderManagement::Stock::Coordinator.new(self).packages packages.each do |package| shipments << package.to_shipment end @@ -518,6 +693,159 @@ def refresh_shipment_rates shipments.map &:refresh_rates end + def products_available_from_new_distribution + # Check that the line_items in the current order are available from a newly selected distribution + errors.add(:base, I18n.t(:spree_order_availability_error)) unless OrderCycleDistributedVariants.new(order_cycle, distributor).distributes_order_variants?(self) + end + + def disallow_guest_order + if using_guest_checkout? && registered_email? + errors.add(:base, I18n.t('devise.failure.already_registered')) + end + end + + # After changing line items of a completed order + def update_shipping_fees! + shipments.each do |shipment| + next if shipment.shipped? + + update_adjustment! shipment.adjustment if shipment.adjustment + save_or_rescue_shipment(shipment) + end + end + + def save_or_rescue_shipment(shipment) + shipment.save # updates included tax + rescue ActiveRecord::RecordNotUnique => e + # This error was seen in production on `shipment.save` above. + # It caused lost payments and duplicate payments due to database rollbacks. + # While we don't understand the cause of this error yet, we rescue here + # because an outdated shipping fee is not as bad as a lost payment. + # And the shipping fee is already up-to-date when this error occurs. + # https://github.com/openfoodfoundation/openfoodnetwork/issues/3924 + Bugsnag.notify(e) do |report| + report.add_tab(:order, attributes) + report.add_tab(:shipment, shipment.attributes) + report.add_tab(:shipment_in_db, Spree::Shipment.find_by(id: shipment.id).attributes) + end + end + + # After changing line items of a completed order + def update_payment_fees! + payments.each do |payment| + next if payment.completed? + + update_adjustment! payment.adjustment if payment.adjustment + payment.save + end + end + + def update_distribution_charge! + # `with_lock` acquires an exclusive row lock on order so no other + # requests can update it until the transaction is commited. + # See https://github.com/rails/rails/blob/3-2-stable/activerecord/lib/active_record/locking/pessimistic.rb#L69 + # and https://www.postgresql.org/docs/current/static/sql-select.html#SQL-FOR-UPDATE-SHARE + with_lock do + EnterpriseFee.clear_all_adjustments_on_order self + + loaded_line_items = + line_items.includes(variant: :product, order: [:distributor, :order_cycle]).all + + loaded_line_items.each do |line_item| + if provided_by_order_cycle? line_item + OpenFoodNetwork::EnterpriseFeeCalculator.new.create_line_item_adjustments_for line_item + end + end + + if order_cycle + OpenFoodNetwork::EnterpriseFeeCalculator.new.create_order_adjustments_for self + end + end + end + + def set_order_cycle!(order_cycle) + return if self.order_cycle == order_cycle + + self.order_cycle = order_cycle + self.distributor = nil unless order_cycle.nil? || order_cycle.has_distributor?(distributor) + empty! + save! + end + + def remove_variant(variant) + line_items(:reload) + current_item = find_line_item_by_variant(variant) + current_item.andand.destroy + end + + def cap_quantity_at_stock! + line_items.includes(variant: :stock_items).all.each(&:cap_quantity_at_stock!) + end + + def set_distributor!(distributor) + self.distributor = distributor + self.order_cycle = nil unless order_cycle.andand.has_distributor? distributor + save! + end + + def set_distribution!(distributor, order_cycle) + self.distributor = distributor + self.order_cycle = order_cycle + save! + end + + def distribution_set? + distributor && order_cycle + end + + def shipping_tax + adjustments(:reload).shipping.sum(&:included_tax) + end + + def enterprise_fee_tax + adjustments(:reload).enterprise_fee.sum(&:included_tax) + end + + def total_tax + (adjustments + price_adjustments).sum(&:included_tax) + end + + def price_adjustments + adjustments = [] + + line_items.each { |line_item| adjustments.concat line_item.adjustments } + + adjustments + end + + def price_adjustment_totals + Hash[tax_adjustment_totals.map do |tax_rate, tax_amount| + [tax_rate.name, + Spree::Money.new(tax_amount, currency: currency)] + end] + end + + def has_taxes_included + !line_items.with_tax.empty? + end + + def address_from_distributor + address = distributor.address.clone + if bill_address + address.firstname = bill_address.firstname + address.lastname = bill_address.lastname + address.phone = bill_address.phone + end + address + end + + # Update attributes of a record in the database without callbacks, validations etc. + # This was originally an extension to ActiveRecord in Spree but only used for Spree::Order + def update_attributes_without_callbacks(attributes) + assign_attributes(attributes) + Spree::Order.where(id: id).update_all(attributes) + end + private def link_by_email @@ -571,5 +899,74 @@ def use_billing? def set_currency self.currency = Spree::Config[:currency] if self[:currency].nil? end + + def using_guest_checkout? + require_email && !user.andand.id + end + + def registered_email? + Spree.user_class.exists?(email: email) + end + + def adjustments_fetcher + @adjustments_fetcher ||= OrderAdjustmentsFetcher.new(self) + end + + def skip_payment_for_subscription? + subscription.present? && order_cycle.orders_close_at.andand > Time.zone.now + end + + def provided_by_order_cycle?(line_item) + order_cycle_variants = order_cycle.andand.variants || [] + order_cycle_variants.include? line_item.variant + end + + def require_customer? + return true unless new_record? || state == 'cart' + end + + def customer_is_valid? + return true unless require_customer? + + customer.present? && customer.enterprise_id == distributor_id && customer.email == email_for_customer + end + + def email_for_customer + (user.andand.email || email).andand.downcase + end + + def associate_customer + return customer if customer.present? + + self.customer = Customer.of(distributor).find_by(email: email_for_customer) + end + + def ensure_customer + unless associate_customer + customer_name = bill_address.andand.full_name + self.customer = Customer.create(enterprise: distributor, email: email_for_customer, user: user, name: customer_name, bill_address: bill_address.andand.clone, ship_address: ship_address.andand.clone) + end + end + + def update_adjustment!(adjustment) + return if adjustment.finalized? + + state = adjustment.state + adjustment.state = 'open' + adjustment.update! + update! + adjustment.state = state + end + + # object_params sets the payment amount to the order total, but it does this + # before the shipping method is set. This results in the customer not being + # charged for their order's shipping. To fix this, we refresh the payment + # amount here. + def charge_shipping_and_payment_fees! + update_totals + return unless pending_payments.any? + + pending_payments.first.update_attribute :amount, total + end end end diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb deleted file mode 100644 index 2b033065cc9..00000000000 --- a/app/models/spree/order_decorator.rb +++ /dev/null @@ -1,446 +0,0 @@ -require 'open_food_network/enterprise_fee_calculator' -require 'open_food_network/feature_toggle' -require 'open_food_network/tag_rule_applicator' -require 'concerns/order_shipment' - -ActiveSupport::Notifications.subscribe('spree.order.contents_changed') do |_name, _start, _finish, _id, payload| - payload[:order].reload.update_distribution_charge! -end - -Spree::Order.class_eval do - prepend OrderShipment - - delegate :admin_and_handling_total, :payment_fee, :ship_total, to: :adjustments_fetcher - - belongs_to :order_cycle - belongs_to :distributor, class_name: 'Enterprise' - belongs_to :customer - has_one :proxy_order - has_one :subscription, through: :proxy_order - - # This removes "inverse_of: source" which breaks shipment adjustment calculations - # This change is done in Spree 2.1 (see https://github.com/spree/spree/commit/3fa44165c7825f79a2fa4eb79b99dc29944c5d55) - # When OFN gets to Spree 2.1, this can be removed - has_many :adjustments, -> { order "#{Spree::Adjustment.table_name}.created_at ASC" }, - as: :adjustable, - dependent: :destroy - - validates :customer, presence: true, if: :require_customer? - validate :products_available_from_new_distribution, if: lambda { distributor_id_changed? || order_cycle_id_changed? } - validate :disallow_guest_order - - # The EmailValidator introduced in Spree 2.1 is not working - # So here we remove it and re-introduce the regexp validation rule from Spree 2.0 - _validate_callbacks.each do |callback| - if callback.raw_filter.respond_to? :attributes - callback.raw_filter.attributes.delete :email - end - end - validates :email, presence: true, format: /\A([\w\.%\+\-']+)@([\w\-]+\.)+([\w]{2,})\z/i, - if: :require_email - - before_validation :associate_customer, unless: :customer_id? - before_validation :ensure_customer, unless: :customer_is_valid? - - before_save :update_shipping_fees!, if: :complete? - before_save :update_payment_fees!, if: :complete? - - # Orders are confirmed with their payment, we don't use the confirm step. - # Here we remove that step from Spree's checkout state machine. - # See: https://guides.spreecommerce.org/developer/checkout.html#modifying-the-checkout-flow - remove_checkout_step :confirm - - state_machine.after_transition to: :payment, do: :charge_shipping_and_payment_fees! - - state_machine.event :restart_checkout do - transition to: :cart, unless: :completed? - end - - # -- Scopes - scope :managed_by, lambda { |user| - if user.has_spree_role?('admin') - where(nil) - else - # Find orders that are distributed by the user or have products supplied by the user - # WARNING: This only filters orders, you'll need to filter line items separately using LineItem.managed_by - with_line_items_variants_and_products_outer. - where('spree_orders.distributor_id IN (?) OR spree_products.supplier_id IN (?)', - user.enterprises.select(&:id), - user.enterprises.select(&:id)). - select('DISTINCT spree_orders.*') - end - } - - scope :distributed_by_user, lambda { |user| - if user.has_spree_role?('admin') - where(nil) - else - where('spree_orders.distributor_id IN (?)', user.enterprises.select(&:id)) - end - } - - scope :with_line_items_variants_and_products_outer, lambda { - joins('LEFT OUTER JOIN spree_line_items ON (spree_line_items.order_id = spree_orders.id)'). - joins('LEFT OUTER JOIN spree_variants ON (spree_variants.id = spree_line_items.variant_id)'). - joins('LEFT OUTER JOIN spree_products ON (spree_products.id = spree_variants.product_id)') - } - - scope :not_state, lambda { |state| - where("state != ?", state) - } - - def updater - @updater ||= OrderManagement::Order::Updater.new(self) - end - - def create_proposed_shipments - adjustments.shipping.delete_all - shipments.destroy_all - - packages = OrderManagement::Stock::Coordinator.new(self).packages - packages.each do |package| - shipments << package.to_shipment - end - - shipments - end - - # -- Methods - def products_available_from_new_distribution - # Check that the line_items in the current order are available from a newly selected distribution - errors.add(:base, I18n.t(:spree_order_availability_error)) unless OrderCycleDistributedVariants.new(order_cycle, distributor).distributes_order_variants?(self) - end - - def using_guest_checkout? - require_email && !user.andand.id - end - - def registered_email? - Spree.user_class.exists?(email: email) - end - - def disallow_guest_order - if using_guest_checkout? && registered_email? - errors.add(:base, I18n.t('devise.failure.already_registered')) - end - end - - def empty_with_clear_shipping_and_payments! - empty_without_clear_shipping_and_payments! - payments.clear - shipments.destroy_all - end - alias_method_chain :empty!, :clear_shipping_and_payments - - def set_order_cycle!(order_cycle) - return if self.order_cycle == order_cycle - - self.order_cycle = order_cycle - self.distributor = nil unless order_cycle.nil? || order_cycle.has_distributor?(distributor) - empty! - save! - end - - # "Checkout" is the initial state and, for card payments, "pending" is the state after authorization - # These are both valid states to process the payment - def pending_payments - (payments.select(&:pending?) + payments.select(&:processing?) + payments.select(&:checkout?)).uniq - end - - def remove_variant(variant) - line_items(:reload) - current_item = find_line_item_by_variant(variant) - current_item.andand.destroy - end - - # Overridden to support max_quantity - def add_variant(variant, quantity = 1, max_quantity = nil, currency = nil) - line_items(:reload) - current_item = find_line_item_by_variant(variant) - - # Notify bugsnag if we get line items with a quantity of zero - if quantity == 0 - Bugsnag.notify(RuntimeError.new("Zero Quantity Line Item"), - current_item: current_item.as_json, - line_items: line_items.map(&:id), - variant: variant.as_json) - end - - if current_item - current_item.quantity = quantity - current_item.max_quantity = max_quantity - - # This is the original behaviour, behaviour above is so that we can resolve the order populator bug - # current_item.quantity ||= 0 - # current_item.max_quantity ||= 0 - # current_item.quantity += quantity.to_i - # current_item.max_quantity += max_quantity.to_i - current_item.currency = currency unless currency.nil? - current_item.save - else - current_item = Spree::LineItem.new(quantity: quantity, max_quantity: max_quantity) - current_item.variant = variant - if currency - current_item.currency = currency unless currency.nil? - current_item.price = variant.price_in(currency).amount - else - current_item.price = variant.price - end - line_items << current_item - end - - reload - current_item - end - - # After changing line items of a completed order - def update_shipping_fees! - shipments.each do |shipment| - next if shipment.shipped? - - update_adjustment! shipment.adjustment if shipment.adjustment - save_or_rescue_shipment(shipment) - end - end - - def save_or_rescue_shipment(shipment) - shipment.save # updates included tax - rescue ActiveRecord::RecordNotUnique => e - # This error was seen in production on `shipment.save` above. - # It caused lost payments and duplicate payments due to database rollbacks. - # While we don't understand the cause of this error yet, we rescue here - # because an outdated shipping fee is not as bad as a lost payment. - # And the shipping fee is already up-to-date when this error occurs. - # https://github.com/openfoodfoundation/openfoodnetwork/issues/3924 - Bugsnag.notify(e) do |report| - report.add_tab(:order, attributes) - report.add_tab(:shipment, shipment.attributes) - report.add_tab(:shipment_in_db, Spree::Shipment.find_by(id: shipment.id).attributes) - end - end - - # After changing line items of a completed order - def update_payment_fees! - payments.each do |payment| - next if payment.completed? - - update_adjustment! payment.adjustment if payment.adjustment - payment.save - end - end - - def cap_quantity_at_stock! - line_items.includes(variant: :stock_items).all.each(&:cap_quantity_at_stock!) - end - - def set_distributor!(distributor) - self.distributor = distributor - self.order_cycle = nil unless order_cycle.andand.has_distributor? distributor - save! - end - - def set_distribution!(distributor, order_cycle) - self.distributor = distributor - self.order_cycle = order_cycle - save! - end - - def distribution_set? - distributor && order_cycle - end - - def update_distribution_charge! - # `with_lock` acquires an exclusive row lock on order so no other - # requests can update it until the transaction is commited. - # See https://github.com/rails/rails/blob/3-2-stable/activerecord/lib/active_record/locking/pessimistic.rb#L69 - # and https://www.postgresql.org/docs/current/static/sql-select.html#SQL-FOR-UPDATE-SHARE - with_lock do - EnterpriseFee.clear_all_adjustments_on_order self - - loaded_line_items = - line_items.includes(variant: :product, order: [:distributor, :order_cycle]).all - - loaded_line_items.each do |line_item| - if provided_by_order_cycle? line_item - OpenFoodNetwork::EnterpriseFeeCalculator.new.create_line_item_adjustments_for line_item - end - end - - if order_cycle - OpenFoodNetwork::EnterpriseFeeCalculator.new.create_order_adjustments_for self - end - end - end - - def set_variant_attributes(variant, attributes) - line_item = find_line_item_by_variant(variant) - - if line_item - if attributes.key?(:max_quantity) && attributes[:max_quantity].to_i < line_item.quantity - attributes[:max_quantity] = line_item.quantity - end - - line_item.assign_attributes(attributes) - line_item.save! - end - end - - def line_item_variants - if line_items.loaded? - line_items.map(&:variant) - else - line_items.includes(:variant).map(&:variant) - end - end - - # Show already bought line items of this order cycle - def finalised_line_items - return [] unless order_cycle && user && distributor - - order_cycle.items_bought_by_user(user, distributor) - end - - # Does this order have shipments that can be shipped? - def ready_to_ship? - shipments.any?(&:can_ship?) - end - - # Ship all pending orders - def ship - shipments.each do |s| - s.ship if s.can_ship? - end - end - - def shipping_tax - adjustments(:reload).shipping.sum(&:included_tax) - end - - def enterprise_fee_tax - adjustments(:reload).enterprise_fee.sum(&:included_tax) - end - - def total_tax - (adjustments + price_adjustments).sum(&:included_tax) - end - - def price_adjustments - adjustments = [] - - line_items.each { |line_item| adjustments.concat line_item.adjustments } - - adjustments - end - - def price_adjustment_totals - Hash[tax_adjustment_totals.map do |tax_rate, tax_amount| - [tax_rate.name, - Spree::Money.new(tax_amount, currency: currency)] - end] - end - - def has_taxes_included - !line_items.with_tax.empty? - end - - # Overrride of Spree method, that allows us to send separate confirmation emails to user and shop owners - def deliver_order_confirmation_email - if subscription.blank? - Delayed::Job.enqueue ConfirmOrderJob.new(id) - end - end - - def changes_allowed? - complete? && distributor.andand.allow_order_changes? && order_cycle.andand.open? - end - - # Override Spree method to allow unpaid orders to be completed. - # Subscriptions place orders at the beginning of an order cycle. They need to - # be completed to draw from stock levels and trigger emails. - # Spree doesn't allow this. Other options would be to introduce an additional - # order state or implement a special proxy payment method. - # https://github.com/openfoodfoundation/openfoodnetwork/pull/3012#issuecomment-438146484 - def payment_required? - total.to_f > 0.0 && !skip_payment_for_subscription? - end - - def address_from_distributor - address = distributor.address.clone - if bill_address - address.firstname = bill_address.firstname - address.lastname = bill_address.lastname - address.phone = bill_address.phone - end - address - end - - # Update attributes of a record in the database without callbacks, validations etc. - # This was originally an extension to ActiveRecord in Spree but only used for Spree::Order - def update_attributes_without_callbacks(attributes) - assign_attributes(attributes) - Spree::Order.where(id: id).update_all(attributes) - end - - private - - def adjustments_fetcher - @adjustments_fetcher ||= OrderAdjustmentsFetcher.new(self) - end - - def skip_payment_for_subscription? - subscription.present? && order_cycle.orders_close_at.andand > Time.zone.now - end - - def provided_by_order_cycle?(line_item) - order_cycle_variants = order_cycle.andand.variants || [] - order_cycle_variants.include? line_item.variant - end - - def require_customer? - return true unless new_record? || state == 'cart' - end - - def customer_is_valid? - return true unless require_customer? - - customer.present? && customer.enterprise_id == distributor_id && customer.email == email_for_customer - end - - def email_for_customer - (user.andand.email || email).andand.downcase - end - - def associate_customer - return customer if customer.present? - - self.customer = Customer.of(distributor).find_by(email: email_for_customer) - end - - def ensure_customer - unless associate_customer - customer_name = bill_address.andand.full_name - self.customer = Customer.create(enterprise: distributor, email: email_for_customer, user: user, name: customer_name, bill_address: bill_address.andand.clone, ship_address: ship_address.andand.clone) - end - end - - def update_adjustment!(adjustment) - return if adjustment.finalized? - - state = adjustment.state - adjustment.state = 'open' - adjustment.update! - update! - adjustment.state = state - end - - # object_params sets the payment amount to the order total, but it does this - # before the shipping method is set. This results in the customer not being - # charged for their order's shipping. To fix this, we refresh the payment - # amount here. - def charge_shipping_and_payment_fees! - update_totals - return unless pending_payments.any? - - pending_payments.first.update_attribute :amount, total - end -end From 2a6d83b4ddd9e9eda7bee789f3c176f669f78240 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 15:28:37 +0100 Subject: [PATCH 03/21] Remove confirm checkout step and it's additional removal --- app/models/spree/order.rb | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index bfafea0ec4f..f5726631713 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -21,16 +21,10 @@ class Order < ActiveRecord::Base order.update_totals order.payment_required? } - go_to_state :confirm, if: ->(order) { order.confirmation_required? } go_to_state :complete remove_transition from: :delivery, to: :confirm end - # Orders are confirmed with their payment, we don't use the confirm step. - # Here we remove that step from Spree's checkout state machine. - # See: https://guides.spreecommerce.org/developer/checkout.html#modifying-the-checkout-flow - remove_checkout_step :confirm - state_machine.after_transition to: :payment, do: :charge_shipping_and_payment_fees! state_machine.event :restart_checkout do @@ -248,11 +242,6 @@ def payment_required? total.to_f > 0.0 && !skip_payment_for_subscription? end - # If true, causes the confirmation step to happen during the checkout process - def confirmation_required? - payments.map(&:payment_method).compact.any?(&:payment_profiles_supported?) - end - # Indicates the number of items in the order def item_count line_items.inject(0) { |sum, li| sum + li.quantity } From cc87e8c9a2d3e53e791f15bf5df6a26fdf246f84 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 15:33:51 +0100 Subject: [PATCH 04/21] Remove code related to promotions --- app/models/spree/order.rb | 21 --------------------- spec/models/spree/order_spec.rb | 16 ---------------- 2 files changed, 37 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index f5726631713..4308381a9da 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -33,8 +33,6 @@ class Order < ActiveRecord::Base token_resource - attr_reader :coupon_code - if Spree.user_class belongs_to :user, class_name: Spree.user_class.to_s belongs_to :created_by, class_name: Spree.user_class.to_s @@ -461,7 +459,6 @@ def credit_cards def finalize! touch :completed_at - # lock all adjustments (coupon promotions, etc.) adjustments.update_all state: 'closed' # update payment and shipment(s) states, and save @@ -605,24 +602,6 @@ def state_changed(name) end end - def coupon_code=(code) - @coupon_code = code.strip.downcase rescue nil - end - - # Tells us if there if the specified promotion is already associated with the order - # regardless of whether or not its currently eligible. Useful because generally - # you would only want a promotion action to apply to order no more than once. - # - # Receives an adjustment +originator+ (here a PromotionAction object) and tells - # if the order has adjustments from that already - def promotion_credit_exists?(originator) - !! adjustments.includes(:originator).promotion.reload.detect { |credit| credit.originator.id == originator.id } - end - - def promo_total - adjustments.eligible.promotion.map(&:amount).sum - end - def shipped? %w(partial shipped).include?(shipment_state) end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 566e579e88f..afbd6998a93 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -504,22 +504,6 @@ end end - context "promotion adjustments" do - let(:originator) { double("Originator", id: 1) } - let(:adjustment) { double("Adjustment", originator: originator) } - - before { order.stub_chain(:adjustments, :includes, :promotion, reload: [adjustment]) } - - context "order has an adjustment from given promo action" do - it { expect(order.promotion_credit_exists? originator).to be_true } - end - - context "order has no adjustment from given promo action" do - before { originator.stub(id: 12) } - it { expect(order.promotion_credit_exists? originator).to be_true } - end - end - context "payment required?" do let(:order) { Spree::Order.new } From 82a116a92fe19cd63d0354f1e4a6462e441aac4a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 15:35:09 +0100 Subject: [PATCH 05/21] We always define Spree.user_class --- app/models/spree/order.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 4308381a9da..0dbd0c7728e 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -33,13 +33,8 @@ class Order < ActiveRecord::Base token_resource - if Spree.user_class - belongs_to :user, class_name: Spree.user_class.to_s - belongs_to :created_by, class_name: Spree.user_class.to_s - else - belongs_to :user - belongs_to :created_by - end + belongs_to :user, class_name: Spree.user_class.to_s + belongs_to :created_by, class_name: Spree.user_class.to_s belongs_to :bill_address, foreign_key: :bill_address_id, class_name: 'Spree::Address' alias_attribute :billing_address, :bill_address From 3c5a35df271832a513cddef34af61fcf864ea88a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 15:36:21 +0100 Subject: [PATCH 06/21] Remove original email validator and keep only previous OFN validator --- app/models/spree/order.rb | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 0dbd0c7728e..9eb46ae51c8 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -88,19 +88,10 @@ def states before_create :link_by_email after_create :create_tax_charge! - validates :email, presence: true, if: :require_email - validates :email, email: true, if: :require_email, allow_blank: true validate :has_available_shipment validate :has_available_payment - - # The EmailValidator introduced in Spree 2.1 is not working - # So here we remove it and re-introduce the regexp validation rule from Spree 2.0 - _validate_callbacks.each do |callback| - if callback.raw_filter.respond_to? :attributes - callback.raw_filter.attributes.delete :email - end - end - validates :email, presence: true, format: /\A([\w\.%\+\-']+)@([\w\-]+\.)+([\w]{2,})\z/i, + validates :email, presence: true, + format: /\A([\w\.%\+\-']+)@([\w\-]+\.)+([\w]{2,})\z/i, if: :require_email make_permalink field: :number From 94ad02abbe25cf36a5e8e72e2f17ea2dc443aa8b Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 15:42:52 +0100 Subject: [PATCH 07/21] Run rubocop autocorrect --- app/models/spree/inventory_unit.rb | 21 ++-- app/models/spree/line_item.rb | 25 ++-- app/models/spree/order.rb | 154 ++++++++++++----------- app/models/spree/order_contents.rb | 9 +- app/models/spree/order_inventory.rb | 9 +- app/models/spree/return_authorization.rb | 63 +++++----- app/models/spree/state_change.rb | 4 +- app/models/spree/tokenized_permission.rb | 3 +- 8 files changed, 154 insertions(+), 134 deletions(-) diff --git a/app/models/spree/inventory_unit.rb b/app/models/spree/inventory_unit.rb index 7380bc40ece..c7cfeb3e68e 100644 --- a/app/models/spree/inventory_unit.rb +++ b/app/models/spree/inventory_unit.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree class InventoryUnit < ActiveRecord::Base belongs_to :variant, class_name: "Spree::Variant" @@ -11,7 +13,7 @@ class InventoryUnit < ActiveRecord::Base includes(:shipment) .where("spree_shipments.state != 'canceled'").references(:shipment) .where(variant_id: stock_item.variant_id) - .backordered.order("#{self.table_name}.created_at ASC") + .backordered.order("#{table_name}.created_at ASC") end # state machine (see http://github.com/pluginaweek/state_machine/tree/master for details) @@ -34,7 +36,7 @@ class InventoryUnit < ActiveRecord::Base # lead to issues once users tried to modify the objects returned. That's due # to ActiveRecord `joins(shipment: :stock_location)` only return readonly # objects - # + # # Returns an array of backordered inventory units as per a given stock item def self.backordered_for_stock_item(stock_item) backordered_per_variant(stock_item).select do |unit| @@ -48,7 +50,7 @@ def self.finalize_units!(inventory_units) def find_stock_item Spree::StockItem.where(stock_location_id: shipment.stock_location_id, - variant_id: variant_id).first + variant_id: variant_id).first end # Remove variant default_scope `deleted_at: nil` @@ -58,13 +60,12 @@ def variant private - def allow_ship? - Spree::Config[:allow_backorder_shipping] || self.on_hand? - end + def allow_ship? + Spree::Config[:allow_backorder_shipping] || on_hand? + end - def update_order - order.update! - end + def update_order + order.update! + end end end - diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index c84a3727db0..4f39d97c2ee 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'open_food_network/scope_variant_to_hub' require 'variant_units/variant_and_line_item_naming' @@ -219,20 +221,21 @@ def scoper end private - def update_inventory - if changed? - scoper.scope(variant) - Spree::OrderInventory.new(self.order).verify(self, target_shipment) - end + + def update_inventory + if changed? + scoper.scope(variant) + Spree::OrderInventory.new(order).verify(self, target_shipment) end + end - def update_order - if changed? || destroyed? - # update the order totals, etc. - order.create_tax_charge! - order.update! - end + def update_order + if changed? || destroyed? + # update the order totals, etc. + order.create_tax_charge! + order.update! end + end def update_inventory_before_destroy # This is necessary before destroying the line item diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 9eb46ae51c8..0d67498e9fd 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spree/core/validators/email' require 'spree/order/checkout' require 'open_food_network/enterprise_fee_calculator' @@ -162,7 +164,7 @@ def self.incomplete # Use this method in other gems that wish to register their own custom logic # that should be called after Order#update def self.register_update_hook(hook) - self.update_hooks.add(hook) + update_hooks.add(hook) end # For compatiblity with Calculator::PriceSack @@ -246,7 +248,8 @@ def tax_zone # taxes in that case. def exclude_tax? return false unless Spree::Config[:prices_inc_tax] - return tax_zone != Zone.default_tax + + tax_zone != Zone.default_tax end # Returns the address for taxation based on configuration @@ -258,7 +261,7 @@ def tax_address # adjustments on an invoice. For example, you can display tax breakout for # cases where tax is included in price. def line_item_adjustment_totals - Hash[self.line_item_adjustments.eligible.group_by(&:label).map do |label, adjustments| + Hash[line_item_adjustments.eligible.group_by(&:label).map do |label, adjustments| total = adjustments.sum(&:amount) [label, Spree::Money.new(total, { currency: currency })] end] @@ -272,21 +275,20 @@ def update! updater.update end - def update_totals - updater.update_totals - end + delegate :update_totals, to: :updater def clone_billing_address - if bill_address and self.ship_address.nil? + if bill_address && ship_address.nil? self.ship_address = bill_address.clone else - self.ship_address.attributes = bill_address.attributes.except('id', 'updated_at', 'created_at') + ship_address.attributes = bill_address.attributes.except('id', 'updated_at', 'created_at') end true end def allow_cancel? - return false unless completed? and state != 'canceled' + return false unless completed? && (state != 'canceled') + shipment_state.nil? || %w{ready backorder pending}.include?(shipment_state) end @@ -294,11 +296,12 @@ def allow_resume? # we shouldn't allow resume for legacy orders b/c we lack the information # necessary to restore to a previous state return false if state_changes.empty? || state_changes.last.previous_state.nil? + true end def awaiting_returns? - return_authorizations.any? { |return_authorization| return_authorization.authorized? } + return_authorizations.any?(&:authorized?) end # This is currently used when adding a variant to an order in the BackOffice. @@ -367,11 +370,11 @@ def set_variant_attributes(variant, attributes) def associate_user!(user) self.user = user self.email = user.email - self.created_by = user if self.created_by.blank? + self.created_by = user if created_by.blank? if persisted? # immediately persist the changes we just made, but don't use save since we might have an invalid address associated - self.class.unscoped.where(id: id).update_all(email: user.email, user_id: user.id, created_by_id: self.created_by_id) + self.class.unscoped.where(id: id).update_all(email: user.email, user_id: user.id, created_by_id: created_by_id) end end @@ -379,11 +382,11 @@ def associate_user!(user) def generate_order_number record = true while record - random = "R#{Array.new(9){rand(9)}.join}" + random = "R#{Array.new(9){ rand(9) }.join}" record = self.class.where(number: random).first end - self.number = random if self.number.blank? - self.number + self.number = random if number.blank? + number end def shipped_shipments @@ -422,7 +425,7 @@ def outstanding_balance end def outstanding_balance? - self.outstanding_balance != 0 + outstanding_balance != 0 end def name @@ -432,7 +435,7 @@ def name end def can_ship? - self.complete? || self.resumed? || self.awaiting_return? || self.returned? + complete? || resumed? || awaiting_return? || returned? end def credit_cards @@ -461,11 +464,11 @@ def finalize! deliver_order_confirmation_email - self.state_changes.create( + state_changes.create( previous_state: 'cart', - next_state: 'complete', - name: 'order' , - user_id: self.user_id + next_state: 'complete', + name: 'order', + user_id: user_id ) end @@ -505,7 +508,7 @@ def pending_payments # def process_payments! if pending_payments.empty? - raise Core::GatewayError.new Spree.t(:no_pending_payments) + raise Core::GatewayError, Spree.t(:no_pending_payments) else pending_payments.each do |payment| break if payment_total >= total @@ -519,7 +522,7 @@ def process_payments! end rescue Core::GatewayError => e result = !!Spree::Config[:allow_checkout_on_gateway_error] - errors.add(:base, e.message) and return result + errors.add(:base, e.message) && (return result) end def billing_firstname @@ -545,12 +548,13 @@ def insufficient_stock_lines def merge!(order) order.line_items.each do |line_item| next unless line_item.currency == currency - current_line_item = self.line_items.find_by(variant: line_item.variant) + + current_line_item = line_items.find_by(variant: line_item.variant) if current_line_item current_line_item.quantity += line_item.quantity current_line_item.save else - line_item.order_id = self.id + line_item.order_id = id line_item.save end end @@ -567,8 +571,8 @@ def empty! end def clear_adjustments! - self.adjustments.destroy_all - self.line_item_adjustments.destroy_all + adjustments.destroy_all + line_item_adjustments.destroy_all end def has_step?(step) @@ -578,12 +582,12 @@ def has_step?(step) def state_changed(name) state = "#{name}_state" if persisted? - old_state = self.send("#{state}_was") - self.state_changes.create( + old_state = send("#{state}_was") + state_changes.create( previous_state: old_state, - next_state: self.send(state), - name: name, - user_id: self.user_id + next_state: send(state), + name: name, + user_id: user_id ) end end @@ -635,11 +639,11 @@ def create_proposed_shipments # # At some point the might need to force the order to transition from address # to delivery again so that proper updated shipments are created. - # e.g. customer goes back from payment step and changes order items + # e.g. customer goes back from payment step and changes order items def ensure_updated_shipments if shipments.any? - self.shipments.destroy_all - self.update_column(:state, "address") + shipments.destroy_all + update_column(:state, "address") end end @@ -733,7 +737,7 @@ def remove_variant(variant) end def cap_quantity_at_stock! - line_items.includes(variant: :stock_items).all.each(&:cap_quantity_at_stock!) + line_items.includes(variant: :stock_items).all.find_each(&:cap_quantity_at_stock!) end def set_distributor!(distributor) @@ -802,57 +806,57 @@ def update_attributes_without_callbacks(attributes) private - def link_by_email - self.email = user.email if self.user - end + def link_by_email + self.email = user.email if user + end - # Determine if email is required (we don't want validation errors before we hit the checkout) - def require_email - return true unless new_record? or state == 'cart' - end + # Determine if email is required (we don't want validation errors before we hit the checkout) + def require_email + return true unless new_record? || (state == 'cart') + end - def ensure_line_items_present - unless line_items.present? - errors.add(:base, Spree.t(:there_are_no_items_for_this_order)) and return false - end + def ensure_line_items_present + if line_items.blank? + errors.add(:base, Spree.t(:there_are_no_items_for_this_order)) && (return false) end + end - def has_available_shipment - return unless has_step?("delivery") - return unless address? - return unless ship_address && ship_address.valid? - # errors.add(:base, :no_shipping_methods_available) if available_shipping_methods.empty? - end + def has_available_shipment + return unless has_step?("delivery") + return unless address? + return unless ship_address&.valid? + # errors.add(:base, :no_shipping_methods_available) if available_shipping_methods.empty? + end - def ensure_available_shipping_rates - if shipments.empty? || shipments.any? { |shipment| shipment.shipping_rates.blank? } - errors.add(:base, Spree.t(:items_cannot_be_shipped)) and return false - end + def ensure_available_shipping_rates + if shipments.empty? || shipments.any? { |shipment| shipment.shipping_rates.blank? } + errors.add(:base, Spree.t(:items_cannot_be_shipped)) && (return false) end + end - def has_available_payment - return unless delivery? - # errors.add(:base, :no_payment_methods_available) if available_payment_methods.empty? - end + def has_available_payment + return unless delivery? + # errors.add(:base, :no_payment_methods_available) if available_payment_methods.empty? + end - def after_cancel - shipments.each { |shipment| shipment.cancel! } + def after_cancel + shipments.each(&:cancel!) - OrderMailer.cancel_email(self.id).deliver - self.payment_state = 'credit_owed' unless shipped? - end + OrderMailer.cancel_email(id).deliver + self.payment_state = 'credit_owed' unless shipped? + end - def after_resume - shipments.each { |shipment| shipment.resume! } - end + def after_resume + shipments.each(&:resume!) + end - def use_billing? - @use_billing == true || @use_billing == 'true' || @use_billing == '1' - end + def use_billing? + @use_billing == true || @use_billing == 'true' || @use_billing == '1' + end - def set_currency - self.currency = Spree::Config[:currency] if self[:currency].nil? - end + def set_currency + self.currency = Spree::Config[:currency] if self[:currency].nil? + end def using_guest_checkout? require_email && !user.andand.id diff --git a/app/models/spree/order_contents.rb b/app/models/spree/order_contents.rb index c6295455788..74650adc1f2 100644 --- a/app/models/spree/order_contents.rb +++ b/app/models/spree/order_contents.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree class OrderContents attr_accessor :order, :currency @@ -27,7 +29,7 @@ def remove(variant, quantity = 1, shipment = nil) private - def add_to_line_item(line_item, variant, quantity, currency=nil, shipment=nil) + def add_to_line_item(line_item, variant, quantity, currency = nil, shipment = nil) if line_item line_item.target_shipment = shipment line_item.quantity += quantity.to_i @@ -48,9 +50,9 @@ def add_to_line_item(line_item, variant, quantity, currency=nil, shipment=nil) line_item end - def remove_from_line_item(line_item, variant, quantity, shipment=nil) + def remove_from_line_item(line_item, _variant, quantity, shipment = nil) line_item.quantity += -quantity - line_item.target_shipment= shipment + line_item.target_shipment = shipment if line_item.quantity == 0 Spree::OrderInventory.new(order).verify(line_item, shipment) @@ -62,6 +64,5 @@ def remove_from_line_item(line_item, variant, quantity, shipment=nil) order.reload line_item end - end end diff --git a/app/models/spree/order_inventory.rb b/app/models/spree/order_inventory.rb index d04db006e90..69d1cf283e8 100644 --- a/app/models/spree/order_inventory.rb +++ b/app/models/spree/order_inventory.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree class OrderInventory attr_accessor :order @@ -21,7 +23,7 @@ def verify(line_item, shipment = nil) if variant_units.size < line_item.quantity quantity = line_item.quantity - variant_units.size - shipment = determine_target_shipment(line_item.variant) unless shipment + shipment ||= determine_target_shipment(line_item.variant) add_to_shipment(shipment, line_item.variant, quantity) elsif variant_units.size > line_item.quantity remove(line_item, variant_units, shipment) @@ -32,11 +34,12 @@ def verify(line_item, shipment = nil) end def inventory_units_for(variant) - units = order.shipments.collect{|s| s.inventory_units.to_a}.flatten + units = order.shipments.collect{ |s| s.inventory_units.to_a }.flatten units.group_by(&:variant_id)[variant.id] || [] end private + def remove(line_item, variant_units, shipment = nil) quantity = variant_units.size - line_item.quantity @@ -45,6 +48,7 @@ def remove(line_item, variant_units, shipment = nil) else order.shipments.each do |shipment| break if quantity == 0 + quantity -= remove_from_shipment(shipment, line_item.variant, quantity) end end @@ -89,6 +93,7 @@ def remove_from_shipment(shipment, variant, quantity) shipment_units.each do |inventory_unit| break if removed_quantity == quantity + inventory_unit.destroy removed_quantity += 1 end diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb index 0a98f55feca..3a7da8ff25b 100644 --- a/app/models/spree/return_authorization.rb +++ b/app/models/spree/return_authorization.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree class ReturnAuthorization < ActiveRecord::Base belongs_to :order, class_name: 'Spree::Order' @@ -55,49 +57,50 @@ def add_variant(variant_id, quantity) end end - order.authorize_return! if inventory_units.reload.size > 0 && !order.awaiting_return? + order.authorize_return! if !inventory_units.reload.empty? && !order.awaiting_return? end def returnable_inventory - order.shipped_shipments.collect{|s| s.inventory_units.to_a}.flatten + order.shipped_shipments.collect{ |s| s.inventory_units.to_a }.flatten end private - def must_have_shipped_units - errors.add(:order, Spree.t(:has_no_shipped_units)) if order.nil? || !order.shipped_shipments.any? - end - def generate_number - return if number + def must_have_shipped_units + errors.add(:order, Spree.t(:has_no_shipped_units)) if order.nil? || order.shipped_shipments.none? + end - record = true - while record - random = "RMA#{Array.new(9){rand(9)}.join}" - record = self.class.where(number: random).first - end - self.number = random + def generate_number + return if number + + record = true + while record + random = "RMA#{Array.new(9){ rand(9) }.join}" + record = self.class.where(number: random).first end + self.number = random + end - def process_return - inventory_units.each do |iu| - iu.return! - Spree::StockMovement.create!(stock_item_id: iu.find_stock_item.id, quantity: 1) - end + def process_return + inventory_units.each do |iu| + iu.return! + Spree::StockMovement.create!(stock_item_id: iu.find_stock_item.id, quantity: 1) + end - credit = Adjustment.new(amount: amount.abs * -1, label: Spree.t(:rma_credit)) - credit.source = self - credit.adjustable = order - credit.save + credit = Adjustment.new(amount: amount.abs * -1, label: Spree.t(:rma_credit)) + credit.source = self + credit.adjustable = order + credit.save - order.return if inventory_units.all?(&:returned?) - end + order.return if inventory_units.all?(&:returned?) + end - def allow_receive? - !inventory_units.empty? - end + def allow_receive? + !inventory_units.empty? + end - def force_positive_amount - self.amount = amount.abs - end + def force_positive_amount + self.amount = amount.abs + end end end diff --git a/app/models/spree/state_change.rb b/app/models/spree/state_change.rb index 8954dffe81a..9f527770952 100644 --- a/app/models/spree/state_change.rb +++ b/app/models/spree/state_change.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree class StateChange < ActiveRecord::Base belongs_to :user @@ -9,7 +11,7 @@ def <=>(other) end def assign_user - true # don't stop the filters + true # don't stop the filters end end end diff --git a/app/models/spree/tokenized_permission.rb b/app/models/spree/tokenized_permission.rb index 29cc24e8b42..58c5882c3a6 100644 --- a/app/models/spree/tokenized_permission.rb +++ b/app/models/spree/tokenized_permission.rb @@ -1,6 +1,7 @@ +# frozen_string_literal: true + module Spree class TokenizedPermission < ActiveRecord::Base belongs_to :permissable, polymorphic: true end end - From 2cd066237d449ab3404173a64c2cac339ec377da Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 16:02:13 +0100 Subject: [PATCH 08/21] Fix easy rubocop issues --- app/models/spree/inventory_unit.rb | 4 +- app/models/spree/line_item.rb | 43 ++++---- app/models/spree/order.rb | 135 ++++++++++++----------- app/models/spree/order_inventory.rb | 7 +- app/models/spree/return_authorization.rb | 6 +- 5 files changed, 105 insertions(+), 90 deletions(-) diff --git a/app/models/spree/inventory_unit.rb b/app/models/spree/inventory_unit.rb index c7cfeb3e68e..d54dbb141fb 100644 --- a/app/models/spree/inventory_unit.rb +++ b/app/models/spree/inventory_unit.rb @@ -49,8 +49,8 @@ def self.finalize_units!(inventory_units) end def find_stock_item - Spree::StockItem.where(stock_location_id: shipment.stock_location_id, - variant_id: variant_id).first + Spree::StockItem.find_by(stock_location_id: shipment.stock_location_id, + variant_id: variant_id) end # Remove variant default_scope `deleted_at: nil` diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index 4f39d97c2ee..f71477cd06c 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -15,7 +15,8 @@ class LineItem < ActiveRecord::Base has_one :product, through: :variant has_many :adjustments, as: :adjustable, dependent: :destroy - has_and_belongs_to_many :option_values, join_table: 'spree_option_values_line_items', class_name: 'Spree::OptionValue' + has_and_belongs_to_many :option_values, join_table: 'spree_option_values_line_items', + class_name: 'Spree::OptionValue' before_validation :adjust_quantity before_validation :copy_price @@ -31,7 +32,8 @@ class LineItem < ActiveRecord::Base validates_with Stock::AvailabilityValidator before_save :update_inventory - before_save :calculate_final_weight_volume, if: :quantity_changed?, unless: :final_weight_volume_changed? + before_save :calculate_final_weight_volume, if: :quantity_changed?, + unless: :final_weight_volume_changed? after_save :update_order after_save :update_units before_destroy :update_inventory_before_destroy @@ -50,7 +52,8 @@ class LineItem < ActiveRecord::Base # Find line items that are from orders distributed by the user or supplied by the user joins(variant: :product). joins(:order). - where('spree_orders.distributor_id IN (?) OR spree_products.supplier_id IN (?)', user.enterprises, user.enterprises). + where('spree_orders.distributor_id IN (?) OR spree_products.supplier_id IN (?)', + user.enterprises, user.enterprises). select('spree_line_items.*') end } @@ -99,17 +102,17 @@ class LineItem < ActiveRecord::Base } def copy_price - if variant - self.price = variant.price if price.nil? - self.cost_price = variant.cost_price if cost_price.nil? - self.currency = variant.currency if currency.nil? - end + return unless variant + + self.price = variant.price if price.nil? + self.cost_price = variant.cost_price if cost_price.nil? + self.currency = variant.currency if currency.nil? end def copy_tax_category - if variant - self.tax_category = variant.product.tax_category - end + return unless variant + + self.tax_category = variant.product.tax_category end def amount @@ -223,18 +226,18 @@ def scoper private def update_inventory - if changed? - scoper.scope(variant) - Spree::OrderInventory.new(order).verify(self, target_shipment) - end + return unless changed? + + scoper.scope(variant) + Spree::OrderInventory.new(order).verify(self, target_shipment) end def update_order - if changed? || destroyed? - # update the order totals, etc. - order.create_tax_charge! - order.update! - end + return unless changed? || destroyed? + + # update the order totals, etc. + order.create_tax_charge! + order.update! end def update_inventory_before_destroy diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 0d67498e9fd..cc617cc0e1f 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -7,7 +7,8 @@ require 'open_food_network/tag_rule_applicator' require 'concerns/order_shipment' -ActiveSupport::Notifications.subscribe('spree.order.contents_changed') do |_name, _start, _finish, _id, payload| +ActiveSupport::Notifications + .subscribe('spree.order.contents_changed') do |_name, _start, _finish, _id, payload| payload[:order].reload.update_distribution_charge! end @@ -82,7 +83,9 @@ def states before_validation :ensure_customer, unless: :customer_is_valid? validates :customer, presence: true, if: :require_customer? - validate :products_available_from_new_distribution, if: lambda { distributor_id_changed? || order_cycle_id_changed? } + validate :products_available_from_new_distribution, if: lambda { + distributor_id_changed? || order_cycle_id_changed? + } validate :disallow_guest_order attr_accessor :use_billing @@ -110,7 +113,8 @@ def states where(nil) else # Find orders that are distributed by the user or have products supplied by the user - # WARNING: This only filters orders, you'll need to filter line items separately using LineItem.managed_by + # WARNING: This only filters orders, + # you'll need to filter line items separately using LineItem.managed_by with_line_items_variants_and_products_outer. where('spree_orders.distributor_id IN (?) OR spree_products.supplier_id IN (?)', user.enterprises.select(&:id), @@ -330,11 +334,6 @@ def add_variant(variant, quantity = 1, max_quantity = nil, currency = nil) current_item.quantity = quantity current_item.max_quantity = max_quantity - # This is the original behaviour, behaviour above is so that we can resolve the order populator bug - # current_item.quantity ||= 0 - # current_item.max_quantity ||= 0 - # current_item.quantity += quantity.to_i - # current_item.max_quantity += max_quantity.to_i current_item.currency = currency unless currency.nil? current_item.save else @@ -356,14 +355,14 @@ def add_variant(variant, quantity = 1, max_quantity = nil, currency = nil) def set_variant_attributes(variant, attributes) line_item = find_line_item_by_variant(variant) - if line_item - if attributes.key?(:max_quantity) && attributes[:max_quantity].to_i < line_item.quantity - attributes[:max_quantity] = line_item.quantity - end + return unless line_item - line_item.assign_attributes(attributes) - line_item.save! + if attributes.key?(:max_quantity) && attributes[:max_quantity].to_i < line_item.quantity + attributes[:max_quantity] = line_item.quantity end + + line_item.assign_attributes(attributes) + line_item.save! end # Associates the specified user with the order. @@ -372,10 +371,13 @@ def associate_user!(user) self.email = user.email self.created_by = user if created_by.blank? - if persisted? - # immediately persist the changes we just made, but don't use save since we might have an invalid address associated - self.class.unscoped.where(id: id).update_all(email: user.email, user_id: user.id, created_by_id: created_by_id) - end + return unless persisted? + + # Persist the changes we just made, + # but don't use save since we might have an invalid address associated + self.class.unscoped.where(id: id).update_all(email: user.email, + user_id: user.id, + created_by_id: created_by_id) end # FIXME refactor this method and implement validation using validates_* utilities @@ -383,7 +385,7 @@ def generate_order_number record = true while record random = "R#{Array.new(9){ rand(9) }.join}" - record = self.class.where(number: random).first + record = self.class.find_by(number: random) end self.number = random if number.blank? number @@ -429,9 +431,10 @@ def outstanding_balance? end def name - if (address = bill_address || ship_address) - "#{address.firstname} #{address.lastname}" - end + address = bill_address || ship_address + return unless address + + "#{address.firstname} #{address.lastname}" end def can_ship? @@ -487,10 +490,12 @@ def available_payment_methods @available_payment_methods ||= PaymentMethod.available(:front_end) end - # "Checkout" is the initial state and, for card payments, "pending" is the state after authorization + # "Checkout" is the initial state and, for card payments, "pending" is the state after auth # These are both valid states to process the payment def pending_payments - (payments.select(&:pending?) + payments.select(&:processing?) + payments.select(&:checkout?)).uniq + (payments.select(&:pending?) + + payments.select(&:processing?) + + payments.select(&:checkout?)).uniq end # processes any pending payments and must return a boolean as it's @@ -507,17 +512,15 @@ def pending_payments # :allow_checkout_on_gateway_error is set to false # def process_payments! - if pending_payments.empty? - raise Core::GatewayError, Spree.t(:no_pending_payments) - else - pending_payments.each do |payment| - break if payment_total >= total + raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty? - payment.process! + pending_payments.each do |payment| + break if payment_total >= total - if payment.completed? - self.payment_total += payment.amount - end + payment.process! + + if payment.completed? + self.payment_total += payment.amount end end rescue Core::GatewayError => e @@ -542,7 +545,7 @@ def variants end def insufficient_stock_lines - line_items.select &:insufficient_stock? + line_items.select(&:insufficient_stock?) end def merge!(order) @@ -581,15 +584,15 @@ def has_step?(step) def state_changed(name) state = "#{name}_state" - if persisted? - old_state = send("#{state}_was") - state_changes.create( - previous_state: old_state, - next_state: send(state), - name: name, - user_id: user_id - ) - end + return unless persisted? + + old_state = __send__("#{state}_was") + state_changes.create( + previous_state: old_state, + next_state: __send__(state), + name: name, + user_id: user_id + ) end def shipped? @@ -641,25 +644,28 @@ def create_proposed_shipments # to delivery again so that proper updated shipments are created. # e.g. customer goes back from payment step and changes order items def ensure_updated_shipments - if shipments.any? - shipments.destroy_all - update_column(:state, "address") - end + return unless shipments.any? + + shipments.destroy_all + update_column(:state, "address") end def refresh_shipment_rates - shipments.map &:refresh_rates + shipments.map(&:refresh_rates) end + # Check that line_items in the current order are available from a newly selected distribution def products_available_from_new_distribution - # Check that the line_items in the current order are available from a newly selected distribution - errors.add(:base, I18n.t(:spree_order_availability_error)) unless OrderCycleDistributedVariants.new(order_cycle, distributor).distributes_order_variants?(self) + return if OrderCycleDistributedVariants.new(order_cycle, distributor) + .distributes_order_variants?(self) + + errors.add(:base, I18n.t(:spree_order_availability_error)) end def disallow_guest_order - if using_guest_checkout? && registered_email? - errors.add(:base, I18n.t('devise.failure.already_registered')) - end + return unless using_guest_checkout? && registered_email? + + errors.add(:base, I18n.t('devise.failure.already_registered')) end # After changing line items of a completed order @@ -816,9 +822,9 @@ def require_email end def ensure_line_items_present - if line_items.blank? - errors.add(:base, Spree.t(:there_are_no_items_for_this_order)) && (return false) - end + return if line_items.present? + + errors.add(:base, Spree.t(:there_are_no_items_for_this_order)) && (return false) end def has_available_shipment @@ -829,9 +835,9 @@ def has_available_shipment end def ensure_available_shipping_rates - if shipments.empty? || shipments.any? { |shipment| shipment.shipping_rates.blank? } - errors.add(:base, Spree.t(:items_cannot_be_shipped)) && (return false) - end + return unless shipments.empty? || shipments.any? { |shipment| shipment.shipping_rates.blank? } + + errors.add(:base, Spree.t(:items_cannot_be_shipped)) && (return false) end def has_available_payment @@ -900,10 +906,13 @@ def associate_customer end def ensure_customer - unless associate_customer - customer_name = bill_address.andand.full_name - self.customer = Customer.create(enterprise: distributor, email: email_for_customer, user: user, name: customer_name, bill_address: bill_address.andand.clone, ship_address: ship_address.andand.clone) - end + return if associate_customer + + customer_name = bill_address.andand.full_name + self.customer = Customer.create(enterprise: distributor, email: email_for_customer, + user: user, name: customer_name, + bill_address: bill_address.andand.clone, + ship_address: ship_address.andand.clone) end def update_adjustment!(adjustment) diff --git a/app/models/spree/order_inventory.rb b/app/models/spree/order_inventory.rb index 69d1cf283e8..2a0554a29fa 100644 --- a/app/models/spree/order_inventory.rb +++ b/app/models/spree/order_inventory.rb @@ -59,12 +59,13 @@ def remove(line_item, variant_units, shipment = nil) # first unshipped that already includes this variant # first unshipped that's leaving from a stock_location that stocks this variant def determine_target_shipment(variant) - shipment = order.shipments.detect do |shipment| + target_shipment = order.shipments.detect do |shipment| (shipment.ready? || shipment.pending?) && shipment.include?(variant) end - shipment ||= order.shipments.detect do |shipment| - (shipment.ready? || shipment.pending?) && variant.stock_location_ids.include?(shipment.stock_location_id) + target_shipment || order.shipments.detect do |shipment| + (shipment.ready? || shipment.pending?) && + variant.stock_location_ids.include?(shipment.stock_location_id) end end diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb index 3a7da8ff25b..6419d7dde18 100644 --- a/app/models/spree/return_authorization.rb +++ b/app/models/spree/return_authorization.rb @@ -67,7 +67,9 @@ def returnable_inventory private def must_have_shipped_units - errors.add(:order, Spree.t(:has_no_shipped_units)) if order.nil? || order.shipped_shipments.none? + return unless order.nil? || order.shipped_shipments.none? + + errors.add(:order, Spree.t(:has_no_shipped_units)) end def generate_number @@ -76,7 +78,7 @@ def generate_number record = true while record random = "RMA#{Array.new(9){ rand(9) }.join}" - record = self.class.where(number: random).first + record = self.class.find_by(number: random) end self.number = random end From 2753e863250b30017b57a261c30efeb610846b77 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 16:04:36 +0100 Subject: [PATCH 09/21] Run rubocop autocorrect --- spec/models/spree/inventory_unit_spec.rb | 14 +-- spec/models/spree/order/address_spec.rb | 2 + spec/models/spree/order/adjustments_spec.rb | 66 +++++++------- spec/models/spree/order/callbacks_spec.rb | 6 +- spec/models/spree/order/payment_spec.rb | 18 ++-- spec/models/spree/order/state_machine_spec.rb | 59 ++++++------- spec/models/spree/order/tax_spec.rb | 32 +++---- spec/models/spree/order/updating_spec.rb | 4 +- spec/models/spree/order_contents_spec.rb | 8 +- spec/models/spree/order_inventory_spec.rb | 22 ++--- spec/models/spree/order_spec.rb | 88 +++++++++---------- .../models/spree/return_authorization_spec.rb | 26 +++--- 12 files changed, 177 insertions(+), 168 deletions(-) diff --git a/spec/models/spree/inventory_unit_spec.rb b/spec/models/spree/inventory_unit_spec.rb index 2beacf1976e..01019f7a1cf 100644 --- a/spec/models/spree/inventory_unit_spec.rb +++ b/spec/models/spree/inventory_unit_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::InventoryUnit do @@ -24,7 +26,7 @@ unit.tap(&:save!) end - # Regression for #3066 + # Regression for Spree #3066 it "returns modifiable objects" do units = Spree::InventoryUnit.backordered_for_stock_item(stock_item) expect { units.first.save! }.to_not raise_error @@ -72,10 +74,12 @@ context "#finalize_units!" do let!(:stock_location) { create(:stock_location) } let(:variant) { create(:variant) } - let(:inventory_units) { [ - create(:inventory_unit, variant: variant), - create(:inventory_unit, variant: variant) - ] } + let(:inventory_units) { + [ + create(:inventory_unit, variant: variant), + create(:inventory_unit, variant: variant) + ] + } it "should create a stock movement" do Spree::InventoryUnit.finalize_units!(inventory_units) diff --git a/spec/models/spree/order/address_spec.rb b/spec/models/spree/order/address_spec.rb index 2efbc20be6d..ecdfd7a91d4 100644 --- a/spec/models/spree/order/address_spec.rb +++ b/spec/models/spree/order/address_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Order do diff --git a/spec/models/spree/order/adjustments_spec.rb b/spec/models/spree/order/adjustments_spec.rb index d35945b3c50..f4891cf70b6 100644 --- a/spec/models/spree/order/adjustments_spec.rb +++ b/spec/models/spree/order/adjustments_spec.rb @@ -1,50 +1,49 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Order do let(:order) { Spree::Order.new } context "clear_adjustments" do - let(:adjustment) { double("Adjustment") } it "destroys all order adjustments" do - order.stub(:adjustments => adjustment) + order.stub(adjustments: adjustment) adjustment.should_receive(:destroy_all) order.clear_adjustments! end it "destroy all line item adjustments" do - order.stub(:line_item_adjustments => adjustment) + order.stub(line_item_adjustments: adjustment) adjustment.should_receive(:destroy_all) order.clear_adjustments! end end context "totaling adjustments" do - let(:adjustment1) { mock_model(Spree::Adjustment, :amount => 5) } - let(:adjustment2) { mock_model(Spree::Adjustment, :amount => 10) } + let(:adjustment1) { mock_model(Spree::Adjustment, amount: 5) } + let(:adjustment2) { mock_model(Spree::Adjustment, amount: 10) } context "#ship_total" do it "should return the correct amount" do - order.stub_chain :adjustments, :shipping => [adjustment1, adjustment2] + order.stub_chain :adjustments, shipping: [adjustment1, adjustment2] order.ship_total.should == 15 end end context "#tax_total" do it "should return the correct amount" do - order.stub_chain :adjustments, :tax => [adjustment1, adjustment2] + order.stub_chain :adjustments, tax: [adjustment1, adjustment2] order.tax_total.should == 15 end end end - context "line item adjustment totals" do before { @order = Spree::Order.create! } - context "when there are no line item adjustments" do - before { @order.stub_chain(:line_item_adjustments, :eligible => []) } + before { @order.stub_chain(:line_item_adjustments, eligible: []) } it "should return an empty hash" do @order.line_item_adjustment_totals.should == {} @@ -52,11 +51,11 @@ end context "when there are two adjustments with different labels" do - let(:adj1) { mock_model Spree::Adjustment, :amount => 10, :label => "Foo" } - let(:adj2) { mock_model Spree::Adjustment, :amount => 20, :label => "Bar" } + let(:adj1) { mock_model Spree::Adjustment, amount: 10, label: "Foo" } + let(:adj2) { mock_model Spree::Adjustment, amount: 20, label: "Bar" } before do - @order.stub_chain(:line_item_adjustments, :eligible => [adj1, adj2]) + @order.stub_chain(:line_item_adjustments, eligible: [adj1, adj2]) end it "should return exactly two totals" do @@ -70,12 +69,12 @@ end context "when there are two adjustments with one label and a single adjustment with another" do - let(:adj1) { mock_model Spree::Adjustment, :amount => 10, :label => "Foo" } - let(:adj2) { mock_model Spree::Adjustment, :amount => 20, :label => "Bar" } - let(:adj3) { mock_model Spree::Adjustment, :amount => 40, :label => "Bar" } + let(:adj1) { mock_model Spree::Adjustment, amount: 10, label: "Foo" } + let(:adj2) { mock_model Spree::Adjustment, amount: 20, label: "Bar" } + let(:adj3) { mock_model Spree::Adjustment, amount: 40, label: "Bar" } before do - @order.stub_chain(:line_item_adjustments, :eligible => [adj1, adj2, adj3]) + @order.stub_chain(:line_item_adjustments, eligible: [adj1, adj2, adj3]) end it "should return exactly two totals" do @@ -91,11 +90,11 @@ context "line item adjustments" do before do @order = Spree::Order.create! - @order.stub :line_items => [line_item1, line_item2] + @order.stub line_items: [line_item1, line_item2] end - let(:line_item1) { create(:line_item, :order => @order) } - let(:line_item2) { create(:line_item, :order => @order) } + let(:line_item1) { create(:line_item, order: @order) } + let(:line_item2) { create(:line_item, order: @order) } context "when there are no line item adjustments" do it "should return nothing if line items have no adjustments" do @@ -106,35 +105,35 @@ context "when only one line item has adjustments" do before do @adj1 = line_item1.adjustments.create( - :amount => 2, - :source => line_item1, - :label => "VAT 5%" + amount: 2, + source: line_item1, + label: "VAT 5%" ) @adj2 = line_item1.adjustments.create( - :amount => 5, - :source => line_item1, - :label => "VAT 10%" + amount: 5, + source: line_item1, + label: "VAT 10%" ) end it "should return the adjustments for that line item" do - @order.line_item_adjustments.should =~ [@adj1, @adj2] + @order.line_item_adjustments.should =~ [@adj1, @adj2] end end context "when more than one line item has adjustments" do before do @adj1 = line_item1.adjustments.create( - :amount => 2, - :source => line_item1, - :label => "VAT 5%" + amount: 2, + source: line_item1, + label: "VAT 5%" ) @adj2 = line_item2.adjustments.create( - :amount => 5, - :source => line_item2, - :label => "VAT 10%" + amount: 5, + source: line_item2, + label: "VAT 10%" ) end @@ -145,4 +144,3 @@ end end end - diff --git a/spec/models/spree/order/callbacks_spec.rb b/spec/models/spree/order/callbacks_spec.rb index 2743242a751..b983c4e90ad 100644 --- a/spec/models/spree/order/callbacks_spec.rb +++ b/spec/models/spree/order/callbacks_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Order do @@ -19,10 +21,10 @@ context "#save" do context "when associated with a registered user" do - let(:user) { double(:user, :email => "test@example.com") } + let(:user) { double(:user, email: "test@example.com") } before do - order.stub :user => user + order.stub user: user end it "should assign the email address of the user" do diff --git a/spec/models/spree/order/payment_spec.rb b/spec/models/spree/order/payment_spec.rb index 8c53b45d70f..db2180255a0 100644 --- a/spec/models/spree/order/payment_spec.rb +++ b/spec/models/spree/order/payment_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' module Spree @@ -10,12 +12,12 @@ module Spree Spree::Config[:auto_capture] = true order.stub_chain(:line_items, :empty?).and_return(false) - order.stub :total => 100 + order.stub total: 100 end it 'processes all payments' do - payment_1 = create(:payment, :amount => 50) - payment_2 = create(:payment, :amount => 50) + payment_1 = create(:payment, amount: 50) + payment_2 = create(:payment, amount: 50) order.stub(:pending_payments).and_return([payment_1, payment_2]) order.process_payments! @@ -27,9 +29,9 @@ module Spree end it 'does not go over total for order' do - payment_1 = create(:payment, :amount => 50) - payment_2 = create(:payment, :amount => 50) - payment_3 = create(:payment, :amount => 50) + payment_1 = create(:payment, amount: 50) + payment_2 = create(:payment, amount: 50) + payment_3 = create(:payment, amount: 50) order.stub(:pending_payments).and_return([payment_1, payment_2, payment_3]) order.process_payments! @@ -42,8 +44,8 @@ module Spree end it "does not use failed payments" do - payment_1 = create(:payment, :amount => 50) - payment_2 = create(:payment, :amount => 50, :state => 'failed') + payment_1 = create(:payment, amount: 50) + payment_2 = create(:payment, amount: 50, state: 'failed') order.stub(:pending_payments).and_return([payment_1]) payment_2.should_not_receive(:process!) diff --git a/spec/models/spree/order/state_machine_spec.rb b/spec/models/spree/order/state_machine_spec.rb index a64a793b953..7b65b683d37 100644 --- a/spec/models/spree/order/state_machine_spec.rb +++ b/spec/models/spree/order/state_machine_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Order do @@ -14,13 +16,13 @@ before do order.state = "confirm" order.run_callbacks(:create) - order.stub :payment_required? => true - order.stub :process_payments! => true + order.stub payment_required?: true + order.stub process_payments!: true order.stub :has_available_shipment end context "when payment processing succeeds" do - before { order.stub :process_payments! => true } + before { order.stub process_payments!: true } it "should finalize order when transitioning to complete state" do order.should_receive(:finalize!) @@ -28,25 +30,23 @@ end context "when credit card processing fails" do - before { order.stub :process_payments! => false } + before { order.stub process_payments!: false } it "should not complete the order" do - order.next - order.state.should == "confirm" - end + order.next + order.state.should == "confirm" + end end - end context "when payment processing fails" do - before { order.stub :process_payments! => false } + before { order.stub process_payments!: false } it "cannot transition to complete" do - order.next - order.state.should == "confirm" + order.next + order.state.should == "confirm" end end - end context "when current state is address" do @@ -67,17 +67,15 @@ context "when current state is delivery" do before do order.state = "delivery" - order.stub :total => 10.0 + order.stub total: 10.0 end end - end context "#can_cancel?" do - %w(pending backorder ready).each do |shipment_state| it "should be true if shipment_state is #{shipment_state}" do - order.stub :completed? => true + order.stub completed?: true order.shipment_state = shipment_state order.can_cancel?.should be_true end @@ -85,31 +83,32 @@ (Spree::Shipment.state_machine.states.keys - %w(pending backorder ready)).each do |shipment_state| it "should be false if shipment_state is #{shipment_state}" do - order.stub :completed? => true + order.stub completed?: true order.shipment_state = shipment_state order.can_cancel?.should be_false end end - end context "#cancel" do let!(:variant) { stub_model(Spree::Variant) } - let!(:inventory_units) { [stub_model(Spree::InventoryUnit, :variant => variant), - stub_model(Spree::InventoryUnit, :variant => variant) ]} + let!(:inventory_units) { + [stub_model(Spree::InventoryUnit, variant: variant), + stub_model(Spree::InventoryUnit, variant: variant)] + } let!(:shipment) do shipment = stub_model(Spree::Shipment) - shipment.stub :inventory_units => inventory_units - order.stub :shipments => [shipment] + shipment.stub inventory_units: inventory_units + order.stub shipments: [shipment] shipment end before do - order.stub :line_items => [stub_model(Spree::LineItem, :variant => variant, :quantity => 2)] - order.line_items.stub :find_by_variant_id => order.line_items.first + order.stub line_items: [stub_model(Spree::LineItem, variant: variant, quantity: 2)] + order.line_items.stub find_by_variant_id: order.line_items.first - order.stub :completed? => true - order.stub :allow_cancel? => true + order.stub completed?: true + order.stub allow_cancel?: true end it "should send a cancel email" do @@ -158,7 +157,7 @@ context "with shipped items" do before do - order.stub :shipment_state => 'partial' + order.stub shipment_state: 'partial' end it "should not alter the payment state" do @@ -172,9 +171,9 @@ # Another regression test for Spree #729 context "#resume" do before do - order.stub :email => "user@spreecommerce.com" - order.stub :state => "canceled" - order.stub :allow_resume? => true + order.stub email: "user@spreecommerce.com" + order.stub state: "canceled" + order.stub allow_resume?: true # Stubs method that cause unwanted side effects in this test order.stub :has_available_shipment diff --git a/spec/models/spree/order/tax_spec.rb b/spec/models/spree/order/tax_spec.rb index 045e0ee9e28..bd2dbd8a476 100644 --- a/spec/models/spree/order/tax_spec.rb +++ b/spec/models/spree/order/tax_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' module Spree @@ -7,7 +9,7 @@ module Spree context "#tax_zone" do let(:bill_address) { create :address } let(:ship_address) { create :address } - let(:order) { Spree::Order.create(:ship_address => ship_address, :bill_address => bill_address) } + let(:order) { Spree::Order.create(ship_address: ship_address, bill_address: bill_address) } let(:zone) { create :zone } context "when no zones exist" do @@ -19,7 +21,7 @@ module Spree end context "when :tax_using_ship_address => true" do - before { Spree::Config.set(:tax_using_ship_address => true) } + before { Spree::Config.set(tax_using_ship_address: true) } it "should calculate using ship_address" do Spree::Zone.should_receive(:match).at_least(:once).with(ship_address) @@ -29,7 +31,7 @@ module Spree end context "when :tax_using_ship_address => false" do - before { Spree::Config.set(:tax_using_ship_address => false) } + before { Spree::Config.set(tax_using_ship_address: false) } it "should calculate using bill_address" do Spree::Zone.should_receive(:match).at_least(:once).with(bill_address) @@ -40,12 +42,12 @@ module Spree context "when there is a default tax zone" do before do - @default_zone = create(:zone, :name => "foo_zone") - Spree::Zone.stub :default_tax => @default_zone + @default_zone = create(:zone, name: "foo_zone") + Spree::Zone.stub default_tax: @default_zone end context "when there is a matching zone" do - before { Spree::Zone.stub(:match => zone) } + before { Spree::Zone.stub(match: zone) } it "should return the matching zone" do order.tax_zone.should == zone @@ -53,7 +55,7 @@ module Spree end context "when there is no matching zone" do - before { Spree::Zone.stub(:match => nil) } + before { Spree::Zone.stub(match: nil) } it "should return the default tax zone" do order.tax_zone.should == @default_zone @@ -62,10 +64,10 @@ module Spree end context "when no default tax zone" do - before { Spree::Zone.stub :default_tax => nil } + before { Spree::Zone.stub default_tax: nil } context "when there is a matching zone" do - before { Spree::Zone.stub(:match => zone) } + before { Spree::Zone.stub(match: zone) } it "should return the matching zone" do order.tax_zone.should == zone @@ -73,7 +75,7 @@ module Spree end context "when there is no matching zone" do - before { Spree::Zone.stub(:match => nil) } + before { Spree::Zone.stub(match: nil) } it "should return nil" do order.tax_zone.should be_nil @@ -86,25 +88,25 @@ module Spree before do @order = create(:order) @default_zone = create(:zone) - Spree::Zone.stub :default_tax => @default_zone + Spree::Zone.stub default_tax: @default_zone end context "when prices include tax" do - before { Spree::Config.set(:prices_inc_tax => true) } + before { Spree::Config.set(prices_inc_tax: true) } it "should be true when tax_zone is not the same as the default" do - @order.stub :tax_zone => create(:zone, :name => "other_zone") + @order.stub tax_zone: create(:zone, name: "other_zone") @order.exclude_tax?.should be_true end it "should be false when tax_zone is the same as the default" do - @order.stub :tax_zone => @default_zone + @order.stub tax_zone: @default_zone @order.exclude_tax?.should be_false end end context "when prices do not include tax" do - before { Spree::Config.set(:prices_inc_tax => false) } + before { Spree::Config.set(prices_inc_tax: false) } it "should be false" do @order.exclude_tax?.should be_false diff --git a/spec/models/spree/order/updating_spec.rb b/spec/models/spree/order/updating_spec.rb index 9b50d029867..0f94681ab35 100644 --- a/spec/models/spree/order/updating_spec.rb +++ b/spec/models/spree/order/updating_spec.rb @@ -1,10 +1,12 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::Order do let(:order) { stub_model(Spree::Order) } context "#update!" do - let(:line_items) { [mock_model(Spree::LineItem, :amount => 5) ]} + let(:line_items) { [mock_model(Spree::LineItem, amount: 5)] } context "when there are update hooks" do before { Spree::Order.register_update_hook :foo } diff --git a/spec/models/spree/order_contents_spec.rb b/spec/models/spree/order_contents_spec.rb index 3ee1bb01f77..df804df072f 100644 --- a/spec/models/spree/order_contents_spec.rb +++ b/spec/models/spree/order_contents_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::OrderContents do @@ -77,16 +79,14 @@ order.item_total.to_f.should == 0.00 order.total.to_f.should == 0.00 - subject.add(variant,2) + subject.add(variant, 2) order.item_total.to_f.should == 39.98 order.total.to_f.should == 39.98 - subject.remove(variant,1) + subject.remove(variant, 1) order.item_total.to_f.should == 19.99 order.total.to_f.should == 19.99 end - end - end diff --git a/spec/models/spree/order_inventory_spec.rb b/spec/models/spree/order_inventory_spec.rb index 5ca8b57f6e5..2a1c3baa18a 100644 --- a/spec/models/spree/order_inventory_spec.rb +++ b/spec/models/spree/order_inventory_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::OrderInventory do @@ -65,8 +67,8 @@ let(:variant) { line_item.variant } before do - order.shipments.create(:stock_location_id => stock_location.id) - shipped = order.shipments.create(:stock_location_id => order.shipments.first.stock_location.id) + order.shipments.create(stock_location_id: stock_location.id) + shipped = order.shipments.create(stock_location_id: order.shipments.first.stock_location.id) shipped.update_column(:state, 'shipped') end @@ -132,9 +134,9 @@ end it 'should destroy backordered units first' do - shipment.stub(:inventory_units_for => [ mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'backordered'), - mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'on_hand'), - mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'backordered') ]) + shipment.stub(inventory_units_for: [mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'backordered'), + mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'on_hand'), + mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'backordered')]) shipment.inventory_units_for[0].should_receive(:destroy) shipment.inventory_units_for[1].should_not_receive(:destroy) @@ -144,8 +146,8 @@ end it 'should destroy unshipped units first' do - shipment.stub(:inventory_units_for => [ mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'shipped'), - mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'on_hand') ] ) + shipment.stub(inventory_units_for: [mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'shipped'), + mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'on_hand')] ) shipment.inventory_units_for[0].should_not_receive(:destroy) shipment.inventory_units_for[1].should_receive(:destroy) @@ -154,8 +156,8 @@ end it 'only attempts to destroy as many units as are eligible, and return amount destroyed' do - shipment.stub(:inventory_units_for => [ mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'shipped'), - mock_model(Spree::InventoryUnit, :variant_id => variant.id, :state => 'on_hand') ] ) + shipment.stub(inventory_units_for: [mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'shipped'), + mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'on_hand')] ) shipment.inventory_units_for[0].should_not_receive(:destroy) shipment.inventory_units_for[1].should_receive(:destroy) @@ -164,7 +166,7 @@ end it 'should destroy self if not inventory units remain' do - shipment.inventory_units.stub(:count => 0) + shipment.inventory_units.stub(count: 0) shipment.should_receive(:destroy) subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index afbd6998a93..3dcebe191be 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -3,20 +3,20 @@ describe Spree::Order do include OpenFoodNetwork::EmailHelper - let(:user) { stub_model(Spree::LegacyUser, :email => "spree@example.com") } - let(:order) { stub_model(Spree::Order, :user => user) } + let(:user) { stub_model(Spree::LegacyUser, email: "spree@example.com") } + let(:order) { stub_model(Spree::Order, user: user) } before do - Spree::LegacyUser.stub(:current => mock_model(Spree::LegacyUser, :id => 123)) + Spree::LegacyUser.stub(current: mock_model(Spree::LegacyUser, id: 123)) end context "#products" do before :each do - @variant1 = mock_model(Spree::Variant, :product => "product1") - @variant2 = mock_model(Spree::Variant, :product => "product2") - @line_items = [mock_model(Spree::LineItem, :product => "product1", :variant => @variant1, :variant_id => @variant1.id, :quantity => 1), - mock_model(Spree::LineItem, :product => "product2", :variant => @variant2, :variant_id => @variant2.id, :quantity => 2)] - order.stub(:line_items => @line_items) + @variant1 = mock_model(Spree::Variant, product: "product1") + @variant2 = mock_model(Spree::Variant, product: "product2") + @line_items = [mock_model(Spree::LineItem, product: "product1", variant: @variant1, variant_id: @variant1.id, quantity: 1), + mock_model(Spree::LineItem, product: "product2", variant: @variant2, variant_id: @variant2.id, quantity: 2)] + order.stub(line_items: @line_items) end it "should return ordered products" do @@ -30,7 +30,7 @@ it "gets the quantity of a given variant" do order.quantity_of(@variant1).should == 1 - @variant3 = mock_model(Spree::Variant, :product => "product3") + @variant3 = mock_model(Spree::Variant, product: "product3") order.quantity_of(@variant3).should == 0 end @@ -43,7 +43,7 @@ context "#generate_order_number" do it "should generate a random string" do order.generate_order_number.is_a?(String).should be_true - (order.generate_order_number.to_s.length > 0).should be_true + (!order.generate_order_number.to_s.empty?).should be_true end end @@ -85,7 +85,6 @@ order.created_by.should == creator end - it "should associate a user with a non-persisted order" do order = Spree::Order.new @@ -116,27 +115,27 @@ let(:order) { Spree::Order.create } it "should be true for order in the 'complete' state" do - order.stub(:complete? => true) + order.stub(complete?: true) order.can_ship?.should be_true end it "should be true for order in the 'resumed' state" do - order.stub(:resumed? => true) + order.stub(resumed?: true) order.can_ship?.should be_true end it "should be true for an order in the 'awaiting return' state" do - order.stub(:awaiting_return? => true) + order.stub(awaiting_return?: true) order.can_ship?.should be_true end it "should be true for an order in the 'returned' state" do - order.stub(:returned? => true) + order.stub(returned?: true) order.can_ship?.should be_true end it "should be false if the order is neither in the 'complete' nor 'resumed' state" do - order.stub(:resumed? => false, :complete? => false) + order.stub(resumed?: false, complete?: false) order.can_ship?.should be_false end end @@ -179,15 +178,15 @@ Spree::Shipment.create(order: order) order.shipments.reload - order.stub(:paid? => true, :complete? => true) + order.stub(paid?: true, complete?: true) order.finalize! order.reload # reload so we're sure the changes are persisted order.shipment_state.should == 'ready' end - after { Spree::Config.set :track_inventory_levels => true } + after { Spree::Config.set track_inventory_levels: true } it "should not sell inventory units if track_inventory_levels is false" do - Spree::Config.set :track_inventory_levels => false + Spree::Config.set track_inventory_levels: false Spree::InventoryUnit.should_not_receive(:sell_units) order.finalize! end @@ -210,13 +209,13 @@ order.stub :has_available_shipment Spree::OrderMailer.stub_chain :confirm_email, :deliver adjustments = double - order.stub :adjustments => adjustments + order.stub adjustments: adjustments expect(adjustments).to receive(:update_all).with(state: 'closed') order.finalize! end it "should log state event" do - order.state_changes.should_receive(:create).exactly(3).times #order, shipment & payment state changes + order.state_changes.should_receive(:create).exactly(3).times # order, shipment & payment state changes order.finalize! end @@ -228,7 +227,7 @@ context "#process_payments!" do let(:payment) { stub_model(Spree::Payment) } - before { order.stub :pending_payments => [payment], :total => 10 } + before { order.stub pending_payments: [payment], total: 10 } it "should process the payments" do payment.should_receive(:process!) @@ -236,7 +235,7 @@ end it "should return false if no pending_payments available" do - order.stub :pending_payments => [] + order.stub pending_payments: [] order.process_payments!.should be_false end @@ -244,15 +243,14 @@ before { payment.should_receive(:process!).and_raise(Spree::Core::GatewayError) } it "should return true when configured to allow checkout on gateway failures" do - Spree::Config.set :allow_checkout_on_gateway_error => true + Spree::Config.set allow_checkout_on_gateway_error: true order.process_payments!.should be_true end it "should return false when not configured to allow checkout on gateway failures" do - Spree::Config.set :allow_checkout_on_gateway_error => false + Spree::Config.set allow_checkout_on_gateway_error: false order.process_payments!.should be_false end - end end @@ -267,7 +265,6 @@ order.payment_total = 10.20 order.outstanding_balance.should be_within(0.001).of(-2.00) end - end context "#outstanding_balance?" do @@ -293,32 +290,32 @@ order.completed_at = nil order.completed?.should be_false - order.completed_at = Time.now + order.completed_at = Time.zone.now order.completed?.should be_true end end it 'is backordered if one of the shipments is backordered' do - order.stub(:shipments => [mock_model(Spree::Shipment, :backordered? => false), - mock_model(Spree::Shipment, :backordered? => true)]) + order.stub(shipments: [mock_model(Spree::Shipment, backordered?: false), + mock_model(Spree::Shipment, backordered?: true)]) order.should be_backordered end context "#allow_checkout?" do it "should be true if there are line_items in the order" do - order.stub_chain(:line_items, :count => 1) + order.stub_chain(:line_items, count: 1) order.checkout_allowed?.should be_true end it "should be false if there are no line_items in the order" do - order.stub_chain(:line_items, :count => 0) + order.stub_chain(:line_items, count: 0) order.checkout_allowed?.should be_false end end context "#item_count" do before do - @order = create(:order, :user => user) - @order.line_items = [ create(:line_item, :quantity => 2), create(:line_item, :quantity => 1) ] + @order = create(:order, user: user) + @order.line_items = [create(:line_item, quantity: 2), create(:line_item, quantity: 1)] end it "should return the correct number of items" do @order.item_count.should == 3 @@ -327,9 +324,9 @@ context "#amount" do before do - @order = create(:order, :user => user) - @order.line_items = [create(:line_item, :price => 1.0, :quantity => 2), - create(:line_item, :price => 1.0, :quantity => 1)] + @order = create(:order, user: user) + @order.line_items = [create(:line_item, price: 1.0, quantity: 2), + create(:line_item, price: 1.0, quantity: 1)] end it "should return the correct lum sum of items" do @order.amount.should == 3.0 @@ -340,35 +337,34 @@ it "should be false for completed order in the canceled state" do order.state = 'canceled' order.shipment_state = 'ready' - order.completed_at = Time.now + order.completed_at = Time.zone.now order.can_cancel?.should be_false end it "should be true for completed order with no shipment" do order.state = 'complete' order.shipment_state = nil - order.completed_at = Time.now + order.completed_at = Time.zone.now order.can_cancel?.should be_true end end context "insufficient_stock_lines" do - let(:line_item) { mock_model Spree::LineItem, :insufficient_stock? => true } + let(:line_item) { mock_model Spree::LineItem, insufficient_stock?: true } - before { order.stub(:line_items => [line_item]) } + before { order.stub(line_items: [line_item]) } it "should return line_item that has insufficient stock on hand" do order.insufficient_stock_lines.size.should == 1 order.insufficient_stock_lines.include?(line_item).should be_true end - end context "empty!" do it "should clear out all line items and adjustments" do order = stub_model(Spree::Order) - order.stub(:line_items => line_items = []) - order.stub(:adjustments => adjustments = []) + order.stub(line_items: line_items = []) + order.stub(adjustments: adjustments = []) order.line_items.should_receive(:destroy_all) order.adjustments.should_receive(:destroy_all) @@ -490,9 +486,9 @@ before do # Don't care about available payment methods in this test - persisted_order.stub(:has_available_payment => false) + persisted_order.stub(has_available_payment: false) persisted_order.line_items << line_item - persisted_order.adjustments.create(:amount => -line_item.amount, :label => "Promotion") + persisted_order.adjustments.create(amount: -line_item.amount, label: "Promotion") persisted_order.state = 'delivery' persisted_order.save # To ensure new state_change event end diff --git a/spec/models/spree/return_authorization_spec.rb b/spec/models/spree/return_authorization_spec.rb index f76854ec8b3..c57f6b4d8b3 100644 --- a/spec/models/spree/return_authorization_spec.rb +++ b/spec/models/spree/return_authorization_spec.rb @@ -1,12 +1,14 @@ +# frozen_string_literal: true + require 'spec_helper' describe Spree::ReturnAuthorization do - let(:stock_location) {Spree::StockLocation.create(:name => "test")} - let(:order) { FactoryGirl.create(:shipped_order)} + let(:stock_location) { Spree::StockLocation.create(name: "test") } + let(:order) { FactoryGirl.create(:shipped_order) } let(:variant) { order.shipments.first.inventory_units.first.variant } - let(:return_authorization) { Spree::ReturnAuthorization.new(:order => order, :stock_location_id => stock_location.id) } + let(:return_authorization) { Spree::ReturnAuthorization.new(order: order, stock_location_id: stock_location.id) } - context "save" do + context "save" do it "should be invalid when order has no inventory units" do order.shipments.destroy_all return_authorization.save @@ -38,7 +40,7 @@ end context "on rma that already has inventory_units" do - before do + before do return_authorization.add_variant(variant.id, 1) end @@ -48,21 +50,19 @@ end it "should not update order state" do - expect{return_authorization.add_variant(variant.id, 1)}.to_not change{order.state} + expect{ return_authorization.add_variant(variant.id, 1) }.to_not change{ order.state } end - end - end context "can_receive?" do it "should allow_receive when inventory units assigned" do - return_authorization.stub(:inventory_units => [1,2,3]) + return_authorization.stub(inventory_units: [1, 2, 3]) return_authorization.can_receive?.should be_true end it "should not allow_receive with no inventory units" do - return_authorization.stub(:inventory_units => []) + return_authorization.stub(inventory_units: []) return_authorization.can_receive?.should be_false end end @@ -70,8 +70,8 @@ context "receive!" do let(:inventory_unit) { order.shipments.first.inventory_units.first } - before do - return_authorization.stub(:inventory_units => [inventory_unit], :amount => -20) + before do + return_authorization.stub(inventory_units: [inventory_unit], amount: -20) Spree::Adjustment.stub(:create) order.stub(:update!) end @@ -87,7 +87,7 @@ mock_adjustment.should_receive(:source=).with(return_authorization) mock_adjustment.should_receive(:adjustable=).with(order) mock_adjustment.should_receive(:save) - Spree::Adjustment.should_receive(:new).with(:amount => -20, :label => Spree.t(:rma_credit)).and_return(mock_adjustment) + Spree::Adjustment.should_receive(:new).with(amount: -20, label: Spree.t(:rma_credit)).and_return(mock_adjustment) return_authorization.receive! end From 8643cbd8ce6389eeec899694ee29077e9303dd88 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 16:58:17 +0100 Subject: [PATCH 10/21] Delete unused order.merge! and fix specs --- app/models/spree/order.rb | 29 +-- spec/models/spree/inventory_unit_spec.rb | 20 +- spec/models/spree/line_item_spec.rb | 20 +- spec/models/spree/order_contents_spec.rb | 38 +-- spec/models/spree/order_inventory_spec.rb | 86 +++---- spec/models/spree/order_spec.rb | 219 ++++++------------ .../models/spree/return_authorization_spec.rb | 21 +- 7 files changed, 143 insertions(+), 290 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index cc617cc0e1f..7af93671933 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -399,11 +399,6 @@ def contains?(variant) find_line_item_by_variant(variant).present? end - def quantity_of(variant) - line_item = find_line_item_by_variant(variant) - line_item ? line_item.quantity : 0 - end - def find_line_item_by_variant(variant) line_items.detect { |line_item| line_item.variant_id == variant.id } end @@ -476,9 +471,9 @@ def finalize! end def deliver_order_confirmation_email - if subscription.blank? - Delayed::Job.enqueue ConfirmOrderJob.new(id) - end + return if subscription.present? + + Delayed::Job.enqueue ConfirmOrderJob.new(id) end # Helper methods for checkout steps @@ -548,24 +543,6 @@ def insufficient_stock_lines line_items.select(&:insufficient_stock?) end - def merge!(order) - order.line_items.each do |line_item| - next unless line_item.currency == currency - - current_line_item = line_items.find_by(variant: line_item.variant) - if current_line_item - current_line_item.quantity += line_item.quantity - current_line_item.save - else - line_item.order_id = id - line_item.save - end - end - # So that the destroy doesn't take out line items which may have been re-assigned - order.line_items.reload - order.destroy - end - def empty! line_items.destroy_all adjustments.destroy_all diff --git a/spec/models/spree/inventory_unit_spec.rb b/spec/models/spree/inventory_unit_spec.rb index 01019f7a1cf..87088a61217 100644 --- a/spec/models/spree/inventory_unit_spec.rb +++ b/spec/models/spree/inventory_unit_spec.rb @@ -33,16 +33,7 @@ end it "finds inventory units from its stock location when the unit's variant matches the stock item's variant" do - Spree::InventoryUnit.backordered_for_stock_item(stock_item).should =~ [unit] - end - - it "does not find inventory units that aren't backordered" do - on_hand_unit = shipment.inventory_units.build - on_hand_unit.state = 'on_hand' - on_hand_unit.variant_id = 1 - on_hand_unit.save! - - Spree::InventoryUnit.backordered_for_stock_item(stock_item).should_not include(on_hand_unit) + expect(Spree::InventoryUnit.backordered_for_stock_item(stock_item)).to eq [unit] end it "does not find inventory units that don't match the stock item's variant" do @@ -51,7 +42,7 @@ other_variant_unit.variant = create(:variant) other_variant_unit.save! - Spree::InventoryUnit.backordered_for_stock_item(stock_item).should_not include(other_variant_unit) + expect(Spree::InventoryUnit.backordered_for_stock_item(stock_item)).to_not include(other_variant_unit) end end @@ -64,11 +55,6 @@ unit.variant.destroy expect(unit.reload.variant).to be_a Spree::Variant end - - it "can still fetch variants by eager loading (remove default_scope)" do - unit.variant.destroy - expect(Spree::InventoryUnit.joins(:variant).includes(:variant).first.variant).to be_a Spree::Variant - end end context "#finalize_units!" do @@ -83,7 +69,7 @@ it "should create a stock movement" do Spree::InventoryUnit.finalize_units!(inventory_units) - inventory_units.any?(&:pending).should be_false + expect(inventory_units.any?(&:pending)).to be_falsy end end end diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 65d1924dd59..4307913a1d2 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -41,9 +41,9 @@ module Spree line_item.currency = nil line_item.copy_price variant = line_item.variant - line_item.price.should == variant.price - line_item.cost_price.should == variant.cost_price - line_item.currency.should == variant.currency + expect(line_item.price).to eq variant.price + expect(line_item.cost_price).to eq variant.cost_price + expect(line_item.currency).to eq variant.currency end end @@ -52,7 +52,7 @@ module Spree it "copies over a variant's tax category" do line_item.tax_category = nil line_item.copy_tax_category - line_item.tax_category.should == line_item.variant.product.tax_category + expect(line_item.tax_category).to eq line_item.variant.product.tax_category end end @@ -69,14 +69,14 @@ module Spree end it "returns a Spree::Money representing the total for this line item" do - line_item.money.to_s.should == "$7.00" + expect(line_item.money.to_s).to eq "$7.00" end end describe '.single_money' do before { line_item.price = 3.50 } it "returns a Spree::Money representing the price for one variant" do - line_item.single_money.to_s.should == "$3.50" + expect(line_item.single_money.to_s).to eq "$3.50" end end @@ -98,7 +98,7 @@ module Spree line_item.target_shipment = order.shipments.first line_item.save - expect(line_item).to have(0).errors_on(:quantity) + expect(line_item.errors[:quantity]).to be_empty end it "doesnt allow to increase item quantity" do @@ -107,7 +107,7 @@ module Spree line_item.target_shipment = order.shipments.first line_item.save - expect(line_item).to have(1).errors_on(:quantity) + expect(line_item.errors[:quantity].first).to include "is out of stock" end end @@ -125,7 +125,7 @@ module Spree line_item.target_shipment = order.shipments.first line_item.save - expect(line_item).to have(0).errors_on(:quantity) + expect(line_item.errors[:quantity]).to be_empty end it "doesnt allow to increase quantity over stock availability" do @@ -134,7 +134,7 @@ module Spree line_item.target_shipment = order.shipments.first line_item.save - expect(line_item).to have(1).errors_on(:quantity) + expect(line_item.errors[:quantity].first).to include "is out of stock" end end end diff --git a/spec/models/spree/order_contents_spec.rb b/spec/models/spree/order_contents_spec.rb index df804df072f..7a333521ee2 100644 --- a/spec/models/spree/order_contents_spec.rb +++ b/spec/models/spree/order_contents_spec.rb @@ -12,32 +12,32 @@ context 'given quantity is not explicitly provided' do it 'should add one line item' do line_item = subject.add(variant) - line_item.quantity.should == 1 - order.line_items.size.should == 1 + expect(line_item.quantity).to eq 1 + expect(order.line_items.size).to eq 1 end end it 'should add line item if one does not exist' do line_item = subject.add(variant, 1) - line_item.quantity.should == 1 - order.line_items.size.should == 1 + expect(line_item.quantity).to eq 1 + expect(order.line_items.size).to eq 1 end it 'should update line item if one exists' do subject.add(variant, 1) line_item = subject.add(variant, 1) - line_item.quantity.should == 2 - order.line_items.size.should == 1 + expect(line_item.quantity).to eq 2 + expect(order.line_items.size).to eq 1 end it "should update order totals" do - order.item_total.to_f.should == 0.00 - order.total.to_f.should == 0.00 + expect(order.item_total.to_f).to eq 0.00 + expect(order.total.to_f).to eq 0.00 subject.add(variant, 1) - order.item_total.to_f.should == 19.99 - order.total.to_f.should == 19.99 + expect(order.item_total.to_f).to eq 19.99 + expect(order.total.to_f).to eq 19.99 end end @@ -57,7 +57,7 @@ line_item = subject.add(variant, 3) subject.remove(variant) - line_item.reload.quantity.should == 2 + expect(line_item.reload.quantity).to eq 2 end end @@ -65,28 +65,28 @@ line_item = subject.add(variant, 3) subject.remove(variant, 1) - line_item.reload.quantity.should == 2 + expect(line_item.reload.quantity).to eq 2 end it 'should remove line_item if quantity matches line_item quantity' do subject.add(variant, 1) subject.remove(variant, 1) - order.reload.find_line_item_by_variant(variant).should be_nil + expect(order.reload.find_line_item_by_variant(variant)).to be_nil end it "should update order totals" do - order.item_total.to_f.should == 0.00 - order.total.to_f.should == 0.00 + expect(order.item_total.to_f).to eq 0.00 + expect(order.total.to_f).to eq 0.00 subject.add(variant, 2) - order.item_total.to_f.should == 39.98 - order.total.to_f.should == 39.98 + expect(order.item_total.to_f).to eq 39.98 + expect(order.total.to_f).to eq 39.98 subject.remove(variant, 1) - order.item_total.to_f.should == 19.99 - order.total.to_f.should == 19.99 + expect(order.item_total.to_f).to eq 19.99 + expect(order.total.to_f).to eq 19.99 end end end diff --git a/spec/models/spree/order_inventory_spec.rb b/spec/models/spree/order_inventory_spec.rb index 2a1c3baa18a..39f48aed0e1 100644 --- a/spec/models/spree/order_inventory_spec.rb +++ b/spec/models/spree/order_inventory_spec.rb @@ -9,7 +9,7 @@ it 'inventory_units_for should return array of units for a given variant' do units = subject.inventory_units_for(line_item.variant) - units.map(&:variant_id).should == [line_item.variant.id] + expect(units.map(&:variant_id)).to eq [line_item.variant.id] end context "when order is missing inventory units" do @@ -18,13 +18,13 @@ end it 'should be a messed up order' do - order.shipments.first.inventory_units_for(line_item.variant).size.should == 1 - line_item.reload.quantity.should == 2 + expect(order.shipments.first.inventory_units_for(line_item.variant).size).to eq 1 + expect(line_item.reload.quantity).to eq 2 end it 'should increase the number of inventory units' do subject.verify(line_item) - order.reload.shipments.first.inventory_units_for(line_item.variant).size.should == 2 + expect(order.reload.shipments.first.inventory_units_for(line_item.variant).size).to eq 2 end end @@ -37,58 +37,27 @@ it "doesn't unstock items" do shipment.stock_location.should_not_receive(:unstock) - subject.send(:add_to_shipment, shipment, variant, 5).should == 5 + expect(subject.send(:add_to_shipment, shipment, variant, 5)).to eq 5 end end it 'should create inventory_units in the necessary states' do shipment.stock_location.should_receive(:fill_status).with(variant, 5).and_return([3, 2]) - subject.send(:add_to_shipment, shipment, variant, 5).should == 5 + expect(subject.send(:add_to_shipment, shipment, variant, 5)).to eq 5 units = shipment.inventory_units.group_by &:variant_id units = units[variant.id].group_by &:state - units['backordered'].size.should == 2 - units['on_hand'].size.should == 3 + expect(units['backordered'].size).to eq 2 + expect(units['on_hand'].size).to eq 3 end it 'should create stock_movement' do - subject.send(:add_to_shipment, shipment, variant, 5).should == 5 + expect(subject.send(:add_to_shipment, shipment, variant, 5)).to eq 5 stock_item = shipment.stock_location.stock_item(variant) movement = stock_item.stock_movements.last - # movement.originator.should == shipment - movement.quantity.should == -5 - end - end - - context "#determine_target_shipment" do - let(:stock_location) { create :stock_location } - let(:variant) { line_item.variant } - - before do - order.shipments.create(stock_location_id: stock_location.id) - shipped = order.shipments.create(stock_location_id: order.shipments.first.stock_location.id) - shipped.update_column(:state, 'shipped') - end - - it 'should select first non-shipped shipment that already contains given variant' do - shipment = subject.send(:determine_target_shipment, variant) - shipment.shipped?.should be_false - shipment.inventory_units_for(variant).should_not be_empty - variant.stock_location_ids.include?(shipment.stock_location_id).should be_true - end - - context "when no shipments already contain this varint" do - it 'selects first non-shipped shipment that leaves from same stock_location' do - subject.send(:remove_from_shipment, order.shipments.first, variant, line_item.quantity) - - shipment = subject.send(:determine_target_shipment, variant) - shipment.reload - shipment.shipped?.should be_false - shipment.inventory_units_for(variant).should be_empty - variant.stock_location_ids.include?(shipment.stock_location_id).should be_true - end + expect(movement.quantity).to eq(-5) end end @@ -102,13 +71,13 @@ end it 'should be a messed up order' do - order.shipments.first.inventory_units_for(line_item.variant).size.should == 3 - line_item.quantity.should == 2 + expect(order.shipments.first.inventory_units_for(line_item.variant).size).to eq 3 + expect(line_item.quantity).to eq 2 end it 'should decrease the number of inventory units' do subject.verify(line_item) - order.reload.shipments.first.inventory_units_for(line_item.variant).size.should == 2 + expect(order.reload.shipments.first.inventory_units_for(line_item.variant).size).to eq 2 end context '#remove_from_shipment' do @@ -120,56 +89,55 @@ it "doesn't restock items" do shipment.stock_location.should_not_receive(:restock) - subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 + expect(subject.send(:remove_from_shipment, shipment, variant, 1)).to eq 1 end end it 'should create stock_movement' do - subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 + expect(subject.send(:remove_from_shipment, shipment, variant, 1)).to eq 1 stock_item = shipment.stock_location.stock_item(variant) movement = stock_item.stock_movements.last - # movement.originator.should == shipment - movement.quantity.should == 1 + expect(movement.quantity).to eq 1 end it 'should destroy backordered units first' do - shipment.stub(inventory_units_for: [mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'backordered'), - mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'on_hand'), - mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'backordered')]) + shipment.stub(inventory_units_for: [build(:inventory_unit, variant_id: variant.id, state: 'backordered'), + build(:inventory_unit, variant_id: variant.id, state: 'on_hand'), + build(:inventory_unit, variant_id: variant.id, state: 'backordered')]) shipment.inventory_units_for[0].should_receive(:destroy) shipment.inventory_units_for[1].should_not_receive(:destroy) shipment.inventory_units_for[2].should_receive(:destroy) - subject.send(:remove_from_shipment, shipment, variant, 2).should == 2 + expect(subject.send(:remove_from_shipment, shipment, variant, 2)).to eq 2 end it 'should destroy unshipped units first' do - shipment.stub(inventory_units_for: [mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'shipped'), - mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'on_hand')] ) + shipment.stub(inventory_units_for: [build(:inventory_unit, variant_id: variant.id, state: 'shipped'), + build(:inventory_unit, variant_id: variant.id, state: 'on_hand')] ) shipment.inventory_units_for[0].should_not_receive(:destroy) shipment.inventory_units_for[1].should_receive(:destroy) - subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 + expect(subject.send(:remove_from_shipment, shipment, variant, 1)).to eq 1 end it 'only attempts to destroy as many units as are eligible, and return amount destroyed' do - shipment.stub(inventory_units_for: [mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'shipped'), - mock_model(Spree::InventoryUnit, variant_id: variant.id, state: 'on_hand')] ) + shipment.stub(inventory_units_for: [build(:inventory_unit, variant_id: variant.id, state: 'shipped'), + build(:inventory_unit, variant_id: variant.id, state: 'on_hand')] ) shipment.inventory_units_for[0].should_not_receive(:destroy) shipment.inventory_units_for[1].should_receive(:destroy) - subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 + expect(subject.send(:remove_from_shipment, shipment, variant, 1)).to eq 1 end it 'should destroy self if not inventory units remain' do shipment.inventory_units.stub(count: 0) shipment.should_receive(:destroy) - subject.send(:remove_from_shipment, shipment, variant, 1).should == 1 + expect(subject.send(:remove_from_shipment, shipment, variant, 1)).to eq 1 end end end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 3dcebe191be..eccf529c92d 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -3,47 +3,34 @@ describe Spree::Order do include OpenFoodNetwork::EmailHelper - let(:user) { stub_model(Spree::LegacyUser, email: "spree@example.com") } - let(:order) { stub_model(Spree::Order, user: user) } + let(:user) { build(:user, email: "spree@example.com") } + let(:order) { build(:order, user: user) } before do - Spree::LegacyUser.stub(current: mock_model(Spree::LegacyUser, id: 123)) + Spree::LegacyUser.stub(current: build(:user, id: 123)) end context "#products" do - before :each do - @variant1 = mock_model(Spree::Variant, product: "product1") - @variant2 = mock_model(Spree::Variant, product: "product2") - @line_items = [mock_model(Spree::LineItem, product: "product1", variant: @variant1, variant_id: @variant1.id, quantity: 1), - mock_model(Spree::LineItem, product: "product2", variant: @variant2, variant_id: @variant2.id, quantity: 2)] - order.stub(line_items: @line_items) - end + let(:order) { create(:order_with_line_items) } it "should return ordered products" do - order.products.should == ['product1', 'product2'] + expect(order.products.first).to eq order.line_items.first.product end it "contains?" do - order.contains?(@variant1).should be_true - end - - it "gets the quantity of a given variant" do - order.quantity_of(@variant1).should == 1 - - @variant3 = mock_model(Spree::Variant, product: "product3") - order.quantity_of(@variant3).should == 0 + expect(order.contains?(order.line_items.first.variant)).to be_truthy end it "can find a line item matching a given variant" do - order.find_line_item_by_variant(@variant1).should_not be_nil - order.find_line_item_by_variant(mock_model(Spree::Variant)).should be_nil + expect(order.find_line_item_by_variant(order.line_items.third.variant)).to_not be_nil + expect(order.find_line_item_by_variant(build(:variant))).to be_nil end end context "#generate_order_number" do it "should generate a random string" do - order.generate_order_number.is_a?(String).should be_true - (!order.generate_order_number.to_s.empty?).should be_true + expect(order.generate_order_number.is_a?(String)).to be_truthy + expect((!order.generate_order_number.to_s.empty?)).to be_truthy end end @@ -55,15 +42,15 @@ order.user = nil order.email = nil order.associate_user!(user) - order.user.should == user - order.email.should == user.email - order.created_by.should == user + expect(order.user).to eq user + expect(order.email).to eq user.email + expect(order.created_by).to eq user # verify that the changes we made were persisted order.reload - order.user.should == user - order.email.should == user.email - order.created_by.should == user + expect(order.user).to eq user + expect(order.email).to eq user.email + expect(order.created_by).to eq user end it "should not overwrite the created_by if it already is set" do @@ -74,15 +61,15 @@ order.user = nil order.email = nil order.associate_user!(user) - order.user.should == user - order.email.should == user.email - order.created_by.should == creator + expect(order.user).to eq user + expect(order.email).to eq user.email + expect(order.created_by).to eq creator # verify that the changes we made were persisted order.reload - order.user.should == user - order.email.should == user.email - order.created_by.should == creator + expect(order.user).to eq user + expect(order.email).to eq user.email + expect(order.created_by).to eq creator end it "should associate a user with a non-persisted order" do @@ -107,7 +94,7 @@ context "#create" do it "should assign an order number" do order = Spree::Order.create - order.number.should_not be_nil + expect(order.number).to_not be_nil end end @@ -116,27 +103,27 @@ it "should be true for order in the 'complete' state" do order.stub(complete?: true) - order.can_ship?.should be_true + expect(order.can_ship?).to be_truthy end it "should be true for order in the 'resumed' state" do order.stub(resumed?: true) - order.can_ship?.should be_true + expect(order.can_ship?).to be_truthy end it "should be true for an order in the 'awaiting return' state" do order.stub(awaiting_return?: true) - order.can_ship?.should be_true + expect(order.can_ship?).to be_truthy end it "should be true for an order in the 'returned' state" do order.stub(returned?: true) - order.can_ship?.should be_true + expect(order.can_ship?).to be_truthy end it "should be false if the order is neither in the 'complete' nor 'resumed' state" do order.stub(resumed?: false, complete?: false) - order.can_ship?.should be_false + expect(order.can_ship?).to be_falsy end end @@ -181,7 +168,7 @@ order.stub(paid?: true, complete?: true) order.finalize! order.reload # reload so we're sure the changes are persisted - order.shipment_state.should == 'ready' + expect(order.shipment_state).to eq 'ready' end after { Spree::Config.set track_inventory_levels: true } @@ -192,15 +179,9 @@ end it "should send an order confirmation email" do - mail_message = double "Mail::Message" - Spree::OrderMailer.should_receive(:confirm_email).with(order.id).and_return mail_message - mail_message.should_receive :deliver - order.finalize! - end - - it "should continue even if confirmation email delivery fails" do - Spree::OrderMailer.should_receive(:confirm_email).with(order.id).and_raise 'send failed!' - order.finalize! + expect do + order.finalize! + end.to enqueue_job ConfirmOrderJob end it "should freeze all adjustments" do @@ -226,17 +207,17 @@ end context "#process_payments!" do - let(:payment) { stub_model(Spree::Payment) } + let(:payment) { build(:payment) } before { order.stub pending_payments: [payment], total: 10 } it "should process the payments" do payment.should_receive(:process!) - order.process_payments!.should be_true + expect(order.process_payments!).to be_truthy end it "should return false if no pending_payments available" do order.stub pending_payments: [] - order.process_payments!.should be_false + expect(order.process_payments!).to be_falsy end context "when a payment raises a GatewayError" do @@ -244,12 +225,12 @@ it "should return true when configured to allow checkout on gateway failures" do Spree::Config.set allow_checkout_on_gateway_error: true - order.process_payments!.should be_true + expect(order.process_payments!).to be_truthy end it "should return false when not configured to allow checkout on gateway failures" do Spree::Config.set allow_checkout_on_gateway_error: false - order.process_payments!.should be_false + expect(order.process_payments!).to be_falsy end end end @@ -258,12 +239,12 @@ it "should return positive amount when payment_total is less than total" do order.payment_total = 20.20 order.total = 30.30 - order.outstanding_balance.should == 10.10 + 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 - order.outstanding_balance.should be_within(0.001).of(-2.00) + expect(order.outstanding_balance).to be_within(0.001).of(-2.00) end end @@ -271,44 +252,38 @@ it "should be true when total greater than payment_total" do order.total = 10.10 order.payment_total = 9.50 - order.outstanding_balance?.should be_true + 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 - order.outstanding_balance?.should be_true + 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 - order.outstanding_balance?.should be_false + expect(order.outstanding_balance?).to be_falsy end end context "#completed?" do it "should indicate if order is completed" do order.completed_at = nil - order.completed?.should be_false + expect(order.completed?).to be_falsy order.completed_at = Time.zone.now - order.completed?.should be_true + expect(order.completed?).to be_truthy end end - it 'is backordered if one of the shipments is backordered' do - order.stub(shipments: [mock_model(Spree::Shipment, backordered?: false), - mock_model(Spree::Shipment, backordered?: true)]) - order.should be_backordered - end - context "#allow_checkout?" do it "should be true if there are line_items in the order" do order.stub_chain(:line_items, count: 1) - order.checkout_allowed?.should be_true + expect(order.checkout_allowed?).to be_truthy end it "should be false if there are no line_items in the order" do order.stub_chain(:line_items, count: 0) - order.checkout_allowed?.should be_false + expect(order.checkout_allowed?).to be_falsy end end @@ -318,7 +293,7 @@ @order.line_items = [create(:line_item, quantity: 2), create(:line_item, quantity: 1)] end it "should return the correct number of items" do - @order.item_count.should == 3 + expect(@order.item_count).to eq 3 end end @@ -329,7 +304,7 @@ create(:line_item, price: 1.0, quantity: 1)] end it "should return the correct lum sum of items" do - @order.amount.should == 3.0 + expect(@order.amount).to eq 3.0 end end @@ -338,31 +313,34 @@ order.state = 'canceled' order.shipment_state = 'ready' order.completed_at = Time.zone.now - order.can_cancel?.should be_false + expect(order.can_cancel?).to be_falsy end it "should be true for completed order with no shipment" do order.state = 'complete' order.shipment_state = nil order.completed_at = Time.zone.now - order.can_cancel?.should be_true + expect(order.can_cancel?).to be_truthy end end context "insufficient_stock_lines" do - let(:line_item) { mock_model Spree::LineItem, insufficient_stock?: true } + let(:line_item) { build(:line_item) } - before { order.stub(line_items: [line_item]) } + before do + order.stub(line_items: [line_item]) + allow(line_item).to receive(:insufficient_stock?) { true } + end it "should return line_item that has insufficient stock on hand" do - order.insufficient_stock_lines.size.should == 1 - order.insufficient_stock_lines.include?(line_item).should be_true + expect(order.insufficient_stock_lines.size).to eq 1 + expect(order.insufficient_stock_lines.include?(line_item)).to be_truthy end end context "empty!" do it "should clear out all line items and adjustments" do - order = stub_model(Spree::Order) + order = build(:order) order.stub(line_items: line_items = []) order.stub(adjustments: adjustments = []) order.line_items.should_receive(:destroy_all) @@ -375,28 +353,28 @@ context "#display_outstanding_balance" do it "returns the value as a spree money" do order.stub(:outstanding_balance) { 10.55 } - order.display_outstanding_balance.should == Spree::Money.new(10.55) + expect(order.display_outstanding_balance).to eq Spree::Money.new(10.55) end end context "#display_item_total" do it "returns the value as a spree money" do order.stub(:item_total) { 10.55 } - order.display_item_total.should == Spree::Money.new(10.55) + expect(order.display_item_total).to eq Spree::Money.new(10.55) end end context "#display_adjustment_total" do it "returns the value as a spree money" do order.adjustment_total = 10.55 - order.display_adjustment_total.should == Spree::Money.new(10.55) + expect(order.display_adjustment_total).to eq Spree::Money.new(10.55) end end context "#display_total" do it "returns the value as a spree money" do order.total = 10.55 - order.display_total.should == Spree::Money.new(10.55) + expect(order.display_total).to eq Spree::Money.new(10.55) end end @@ -405,7 +383,7 @@ before { order.currency = "ABC" } it "returns the currency from the object" do - order.currency.should == "ABC" + expect(order.currency).to eq "ABC" end end @@ -413,64 +391,9 @@ before { order.currency = nil } it "returns the globally configured currency" do - order.currency.should == "USD" - end - end - end - - # Regression tests for Spree #2179 - context "#merge!" do - let(:variant) { create(:variant) } - let(:order_1) { Spree::Order.create } - let(:order_2) { Spree::Order.create } - - it "destroys the other order" do - order_1.merge!(order_2) - lambda { order_2.reload }.should raise_error(ActiveRecord::RecordNotFound) - end - - context "merging together two orders with line items for the same variant" do - before do - order_1.contents.add(variant, 1) - order_2.contents.add(variant, 1) - end - - specify do - order_1.merge!(order_2) - order_1.line_items.count.should == 1 - - line_item = order_1.line_items.first - line_item.quantity.should == 2 - line_item.variant_id.should == variant.id + expect(order.currency).to eq Spree::Config[:currency] end end - - context "merging together two orders with different line items" do - let(:variant_2) { create(:variant) } - - before do - order_1.contents.add(variant, 1) - order_2.contents.add(variant_2, 1) - end - - specify do - order_1.merge!(order_2) - line_items = order_1.line_items - line_items.count.should == 2 - - # No guarantee on ordering of line items, so we do this: - line_items.pluck(:quantity).should =~ [1, 1] - line_items.pluck(:variant_id).should =~ [variant.id, variant_2.id] - end - end - end - - context "#confirmation_required?" do - it "does not bomb out when an order has an unpersisted payment" do - order = Spree::Order.new - order.payments.build - assert !order.confirmation_required? - end end # Regression test for Spree #2191 @@ -496,7 +419,7 @@ it "transitions from delivery to payment" do persisted_order.stub(payment_required?: true) persisted_order.next! - persisted_order.state.should == "payment" + expect(persisted_order.state).to eq "payment" end end @@ -504,12 +427,12 @@ let(:order) { Spree::Order.new } context "total is zero" do - it { order.payment_required?.should be_false } + it { expect(order.payment_required?).to be_falsy } end context "total > zero" do before { order.stub(total: 1) } - it { order.payment_required?.should be_true } + it { expect(order.payment_required?).to be_truthy } end end @@ -559,7 +482,7 @@ let(:tax_using_ship_address) { true } it 'returns ship_address' do - subject.should == order.ship_address + expect(subject).to eq order.ship_address end end @@ -567,7 +490,7 @@ let(:tax_using_ship_address) { false } it "returns bill_address" do - subject.should == order.bill_address + expect(subject).to eq order.bill_address end end end @@ -586,11 +509,11 @@ def initialize(decorated_object) end it 'returns an order_updater_decorator class' do - order.updater.class.should == FakeOrderUpdaterDecorator + expect(order.updater.class).to eq FakeOrderUpdaterDecorator end it 'decorates a Spree::OrderUpdater' do - order.updater.decorated_object.class.should == Spree::OrderUpdater + expect(order.updater.decorated_object.class).to eq Spree::OrderUpdater end end diff --git a/spec/models/spree/return_authorization_spec.rb b/spec/models/spree/return_authorization_spec.rb index c57f6b4d8b3..893965850ac 100644 --- a/spec/models/spree/return_authorization_spec.rb +++ b/spec/models/spree/return_authorization_spec.rb @@ -3,16 +3,15 @@ require 'spec_helper' describe Spree::ReturnAuthorization do - let(:stock_location) { Spree::StockLocation.create(name: "test") } let(:order) { FactoryGirl.create(:shipped_order) } let(:variant) { order.shipments.first.inventory_units.first.variant } - let(:return_authorization) { Spree::ReturnAuthorization.new(order: order, stock_location_id: stock_location.id) } + let(:return_authorization) { Spree::ReturnAuthorization.new(order: order) } context "save" do it "should be invalid when order has no inventory units" do order.shipments.destroy_all return_authorization.save - return_authorization.errors[:order].should == ["has no shipped units"] + expect(return_authorization.errors[:order]).to eq ["has no shipped units"] end it "should generate RMA number" do @@ -25,7 +24,7 @@ context "on empty rma" do it "should associate inventory unit" do return_authorization.add_variant(variant.id, 1) - return_authorization.inventory_units.size.should == 1 + expect(return_authorization.inventory_units.size).to eq 1 end it "should associate inventory units as shipped" do @@ -58,12 +57,12 @@ context "can_receive?" do it "should allow_receive when inventory units assigned" do return_authorization.stub(inventory_units: [1, 2, 3]) - return_authorization.can_receive?.should be_true + expect(return_authorization.can_receive?).to be_truthy end it "should not allow_receive with no inventory units" do return_authorization.stub(inventory_units: []) - return_authorization.can_receive?.should be_false + expect(return_authorization.can_receive?).to be_falsy end end @@ -101,7 +100,7 @@ it "should ensure the amount is always positive" do return_authorization.amount = -10 return_authorization.send :force_positive_amount - return_authorization.amount.should == 10 + expect(return_authorization.amount).to eq 10 end end @@ -115,24 +114,24 @@ context "currency" do before { order.stub(:currency) { "ABC" } } it "returns the order currency" do - return_authorization.currency.should == "ABC" + expect(return_authorization.currency).to eq "ABC" end end context "display_amount" do it "returns a Spree::Money" do return_authorization.amount = 21.22 - return_authorization.display_amount.should == Spree::Money.new(21.22) + expect(return_authorization.display_amount).to eq Spree::Money.new(21.22) end end context "returnable_inventory" do pending "should return inventory from shipped shipments" do - return_authorization.returnable_inventory.should == [inventory_unit] + expect(return_authorization.returnable_inventory).to eq [inventory_unit] end pending "should not return inventory from unshipped shipments" do - return_authorization.returnable_inventory.should == [] + expect(return_authorization.returnable_inventory).to eq [] end end end From 31f9cd3caf95a218b7c5c46a6eec9511b23d91e7 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 17:21:42 +0100 Subject: [PATCH 11/21] Fix spec/models/spree/order specs --- spec/models/spree/order/address_spec.rb | 10 ++--- spec/models/spree/order/adjustments_spec.rb | 37 +++++++++--------- spec/models/spree/order/callbacks_spec.rb | 8 ++-- spec/models/spree/order/payment_spec.rb | 39 ++++++++++--------- spec/models/spree/order/state_machine_spec.rb | 28 ++++++------- spec/models/spree/order/tax_spec.rb | 18 ++++----- spec/models/spree/order/updating_spec.rb | 4 +- spec/models/spree/order_spec.rb | 7 ---- 8 files changed, 73 insertions(+), 78 deletions(-) diff --git a/spec/models/spree/order/address_spec.rb b/spec/models/spree/order/address_spec.rb index ecdfd7a91d4..78ad56a0efb 100644 --- a/spec/models/spree/order/address_spec.rb +++ b/spec/models/spree/order/address_spec.rb @@ -8,7 +8,7 @@ context 'validation' do context "when @use_billing is populated" do before do - order.bill_address = stub_model(Spree::Address) + order.bill_address = build(:address) order.ship_address = nil end @@ -17,7 +17,7 @@ it "clones the bill address to the ship address" do order.valid? - order.ship_address.should == order.bill_address + expect(order.ship_address).to eq order.bill_address end end @@ -26,7 +26,7 @@ it "clones the bill address to the shipping" do order.valid? - order.ship_address.should == order.bill_address + expect(order.ship_address).to eq order.bill_address end end @@ -35,7 +35,7 @@ it "clones the bill address to the shipping" do order.valid? - order.ship_address.should == order.bill_address + expect(order.ship_address).to eq order.bill_address end end @@ -44,7 +44,7 @@ it "does not clone the bill address to the shipping" do order.valid? - order.ship_address.should be_nil + expect(order.ship_address).to be_nil end end end diff --git a/spec/models/spree/order/adjustments_spec.rb b/spec/models/spree/order/adjustments_spec.rb index f4891cf70b6..bb0101991ea 100644 --- a/spec/models/spree/order/adjustments_spec.rb +++ b/spec/models/spree/order/adjustments_spec.rb @@ -21,20 +21,20 @@ end context "totaling adjustments" do - let(:adjustment1) { mock_model(Spree::Adjustment, amount: 5) } - let(:adjustment2) { mock_model(Spree::Adjustment, amount: 10) } + let(:adjustment1) { build(:adjustment, amount: 5) } + let(:adjustment2) { build(:adjustment, amount: 10) } context "#ship_total" do it "should return the correct amount" do order.stub_chain :adjustments, shipping: [adjustment1, adjustment2] - order.ship_total.should == 15 + expect(order.ship_total).to eq 15 end end context "#tax_total" do it "should return the correct amount" do order.stub_chain :adjustments, tax: [adjustment1, adjustment2] - order.tax_total.should == 15 + expect(order.tax_total).to eq 15 end end end @@ -46,43 +46,43 @@ before { @order.stub_chain(:line_item_adjustments, eligible: []) } it "should return an empty hash" do - @order.line_item_adjustment_totals.should == {} + expect(@order.line_item_adjustment_totals).to eq({}) end end context "when there are two adjustments with different labels" do - let(:adj1) { mock_model Spree::Adjustment, amount: 10, label: "Foo" } - let(:adj2) { mock_model Spree::Adjustment, amount: 20, label: "Bar" } + let(:adj1) { build(:adjustment, amount: 10, label: "Foo") } + let(:adj2) { build(:adjustment, amount: 20, label: "Bar") } before do @order.stub_chain(:line_item_adjustments, eligible: [adj1, adj2]) end it "should return exactly two totals" do - @order.line_item_adjustment_totals.size.should == 2 + expect(@order.line_item_adjustment_totals.size).to eq 2 end it "should return the correct totals" do - @order.line_item_adjustment_totals["Foo"].should == Spree::Money.new(10) - @order.line_item_adjustment_totals["Bar"].should == Spree::Money.new(20) + expect(@order.line_item_adjustment_totals["Foo"]).to eq Spree::Money.new(10) + expect(@order.line_item_adjustment_totals["Bar"]).to eq Spree::Money.new(20) end end context "when there are two adjustments with one label and a single adjustment with another" do - let(:adj1) { mock_model Spree::Adjustment, amount: 10, label: "Foo" } - let(:adj2) { mock_model Spree::Adjustment, amount: 20, label: "Bar" } - let(:adj3) { mock_model Spree::Adjustment, amount: 40, label: "Bar" } + let(:adj1) { build(:adjustment, amount: 10, label: "Foo") } + let(:adj2) { build(:adjustment, amount: 20, label: "Bar") } + let(:adj3) { build(:adjustment, amount: 40, label: "Bar") } before do @order.stub_chain(:line_item_adjustments, eligible: [adj1, adj2, adj3]) end it "should return exactly two totals" do - @order.line_item_adjustment_totals.size.should == 2 + expect(@order.line_item_adjustment_totals.size).to eq 2 end it "should return the correct totals" do - @order.line_item_adjustment_totals["Foo"].should == Spree::Money.new(10) - @order.line_item_adjustment_totals["Bar"].should == Spree::Money.new(60) + expect(@order.line_item_adjustment_totals["Foo"]).to eq Spree::Money.new(10) + expect(@order.line_item_adjustment_totals["Bar"]).to eq Spree::Money.new(60) end end end @@ -98,7 +98,7 @@ context "when there are no line item adjustments" do it "should return nothing if line items have no adjustments" do - @order.line_item_adjustments.should be_empty + expect(@order.line_item_adjustments).to be_empty end end @@ -118,7 +118,8 @@ end it "should return the adjustments for that line item" do - @order.line_item_adjustments.should =~ [@adj1, @adj2] + expect(@order.line_item_adjustments).to include @adj1 + expect(@order.line_item_adjustments).to include @adj2 end end diff --git a/spec/models/spree/order/callbacks_spec.rb b/spec/models/spree/order/callbacks_spec.rb index b983c4e90ad..4c8f287650d 100644 --- a/spec/models/spree/order/callbacks_spec.rb +++ b/spec/models/spree/order/callbacks_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Spree::Order do - let(:order) { stub_model(Spree::Order) } + let(:order) { build(:order) } before do Spree::Order.define_state_machine! end @@ -14,7 +14,7 @@ it "o'brien@gmail.com is a valid email address" do order.state = 'address' order.email = "o'brien@gmail.com" - order.should have(:no).error_on(:email) + expect(order.errors[:email]).to be_empty end end end @@ -29,7 +29,7 @@ it "should assign the email address of the user" do order.run_callbacks(:create) - order.email.should == user.email + expect(order.email).to eq user.email end end end @@ -38,7 +38,7 @@ it "should not validate email address" do order.state = "cart" order.email = nil - order.should have(:no).error_on(:email) + expect(order.errors[:email]).to be_empty end end end diff --git a/spec/models/spree/order/payment_spec.rb b/spec/models/spree/order/payment_spec.rb index db2180255a0..a9083035735 100644 --- a/spec/models/spree/order/payment_spec.rb +++ b/spec/models/spree/order/payment_spec.rb @@ -4,8 +4,9 @@ module Spree describe Spree::Order do - let(:order) { stub_model(Spree::Order) } + let(:order) { build(:order) } let(:updater) { Spree::OrderUpdater.new(order) } + let(:bogus) { create(:bogus_payment_method, distributors: [create(:enterprise)]) } before do # So that Payment#purchase! is called during processing @@ -16,39 +17,39 @@ module Spree end it 'processes all payments' do - payment_1 = create(:payment, amount: 50) - payment_2 = create(:payment, amount: 50) - order.stub(:pending_payments).and_return([payment_1, payment_2]) + payment1 = create(:payment, amount: 50, payment_method: bogus) + payment2 = create(:payment, amount: 50, payment_method: bogus) + order.stub(:pending_payments).and_return([payment1, payment2]) order.process_payments! updater.update_payment_state - order.payment_state.should == 'paid' + expect(order.payment_state).to eq 'paid' - payment_1.should be_completed - payment_2.should be_completed + expect(payment1).to be_completed + expect(payment2).to be_completed end it 'does not go over total for order' do - payment_1 = create(:payment, amount: 50) - payment_2 = create(:payment, amount: 50) - payment_3 = create(:payment, amount: 50) - order.stub(:pending_payments).and_return([payment_1, payment_2, payment_3]) + payment1 = create(:payment, amount: 50, payment_method: bogus) + payment2 = create(:payment, amount: 50, payment_method: bogus) + payment3 = create(:payment, amount: 50, payment_method: bogus) + order.stub(:pending_payments).and_return([payment1, payment2, payment3]) order.process_payments! updater.update_payment_state - order.payment_state.should == 'paid' + expect(order.payment_state).to eq 'paid' - payment_1.should be_completed - payment_2.should be_completed - payment_3.should be_checkout + expect(payment1).to be_completed + expect(payment2).to be_completed + expect(payment3).to be_checkout end it "does not use failed payments" do - payment_1 = create(:payment, amount: 50) - payment_2 = create(:payment, amount: 50, state: 'failed') - order.stub(:pending_payments).and_return([payment_1]) + payment1 = create(:payment, amount: 50, payment_method: bogus) + payment2 = create(:payment, amount: 50, state: 'failed', payment_method: bogus) + order.stub(:pending_payments).and_return([payment1]) - payment_2.should_not_receive(:process!) + payment2.should_not_receive(:process!) order.process_payments! end diff --git a/spec/models/spree/order/state_machine_spec.rb b/spec/models/spree/order/state_machine_spec.rb index 7b65b683d37..ed5ffb7289b 100644 --- a/spec/models/spree/order/state_machine_spec.rb +++ b/spec/models/spree/order/state_machine_spec.rb @@ -12,9 +12,9 @@ end context "#next!" do - context "when current state is confirm" do + context "when current state is payment" do before do - order.state = "confirm" + order.state = "payment" order.run_callbacks(:create) order.stub payment_required?: true order.stub process_payments!: true @@ -34,7 +34,7 @@ it "should not complete the order" do order.next - order.state.should == "confirm" + expect(order.state).to eq "payment" end end end @@ -44,7 +44,7 @@ it "cannot transition to complete" do order.next - order.state.should == "confirm" + expect(order.state).to eq "payment" end end end @@ -77,7 +77,7 @@ it "should be true if shipment_state is #{shipment_state}" do order.stub completed?: true order.shipment_state = shipment_state - order.can_cancel?.should be_true + expect(order.can_cancel?).to be_truthy end end @@ -85,26 +85,26 @@ it "should be false if shipment_state is #{shipment_state}" do order.stub completed?: true order.shipment_state = shipment_state - order.can_cancel?.should be_false + expect(order.can_cancel?).to be_falsy end end end context "#cancel" do - let!(:variant) { stub_model(Spree::Variant) } + let!(:variant) { build(:variant) } let!(:inventory_units) { - [stub_model(Spree::InventoryUnit, variant: variant), - stub_model(Spree::InventoryUnit, variant: variant)] + [build(:inventory_unit, variant: variant), + build(:inventory_unit, variant: variant)] } let!(:shipment) do - shipment = stub_model(Spree::Shipment) + shipment = build(:shipment) shipment.stub inventory_units: inventory_units order.stub shipments: [shipment] shipment end before do - order.stub line_items: [stub_model(Spree::LineItem, variant: variant, quantity: 2)] + order.stub line_items: [build(:line_item, variant: variant, quantity: 2)] order.line_items.stub find_by_variant_id: order.line_items.first order.stub completed?: true @@ -124,7 +124,7 @@ } mail_message.should_receive :deliver order.cancel! - order_id.should == order.id + expect(order_id).to eq order.id end context "restocking inventory" do @@ -151,7 +151,7 @@ context "without shipped items" do it "should set payment state to 'credit owed'" do order.cancel! - order.payment_state.should == 'credit_owed' + expect(order.payment_state).to eq 'credit_owed' end end @@ -162,7 +162,7 @@ it "should not alter the payment state" do order.cancel! - order.payment_state.should be_nil + expect(order.payment_state).to be_nil end end end diff --git a/spec/models/spree/order/tax_spec.rb b/spec/models/spree/order/tax_spec.rb index bd2dbd8a476..780673d1131 100644 --- a/spec/models/spree/order/tax_spec.rb +++ b/spec/models/spree/order/tax_spec.rb @@ -4,7 +4,7 @@ module Spree describe Spree::Order do - let(:order) { stub_model(Spree::Order) } + let(:order) { build(:order) } context "#tax_zone" do let(:bill_address) { create :address } @@ -16,7 +16,7 @@ module Spree before { Spree::Zone.destroy_all } it "should return nil" do - order.tax_zone.should be_nil + expect(order.tax_zone).to be_nil end end @@ -50,7 +50,7 @@ module Spree before { Spree::Zone.stub(match: zone) } it "should return the matching zone" do - order.tax_zone.should == zone + expect(order.tax_zone).to eq zone end end @@ -58,7 +58,7 @@ module Spree before { Spree::Zone.stub(match: nil) } it "should return the default tax zone" do - order.tax_zone.should == @default_zone + expect(order.tax_zone).to eq @default_zone end end end @@ -70,7 +70,7 @@ module Spree before { Spree::Zone.stub(match: zone) } it "should return the matching zone" do - order.tax_zone.should == zone + expect(order.tax_zone).to eq zone end end @@ -78,7 +78,7 @@ module Spree before { Spree::Zone.stub(match: nil) } it "should return nil" do - order.tax_zone.should be_nil + expect(order.tax_zone).to be_nil end end end @@ -96,12 +96,12 @@ module Spree it "should be true when tax_zone is not the same as the default" do @order.stub tax_zone: create(:zone, name: "other_zone") - @order.exclude_tax?.should be_true + expect(@order.exclude_tax?).to be_truthy end it "should be false when tax_zone is the same as the default" do @order.stub tax_zone: @default_zone - @order.exclude_tax?.should be_false + expect(@order.exclude_tax?).to be_falsy end end @@ -109,7 +109,7 @@ module Spree before { Spree::Config.set(prices_inc_tax: false) } it "should be false" do - @order.exclude_tax?.should be_false + expect(@order.exclude_tax?).to be_falsy end end end diff --git a/spec/models/spree/order/updating_spec.rb b/spec/models/spree/order/updating_spec.rb index 0f94681ab35..d21d45c2ca0 100644 --- a/spec/models/spree/order/updating_spec.rb +++ b/spec/models/spree/order/updating_spec.rb @@ -3,10 +3,10 @@ require 'spec_helper' describe Spree::Order do - let(:order) { stub_model(Spree::Order) } + let(:order) { build(:order) } context "#update!" do - let(:line_items) { [mock_model(Spree::LineItem, amount: 5)] } + let(:line_items) { [build(:line_item, amount: 5)] } context "when there are update hooks" do before { Spree::Order.register_update_hook :foo } diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index eccf529c92d..193981dc812 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -171,13 +171,6 @@ expect(order.shipment_state).to eq 'ready' end - after { Spree::Config.set track_inventory_levels: true } - it "should not sell inventory units if track_inventory_levels is false" do - Spree::Config.set track_inventory_levels: false - Spree::InventoryUnit.should_not_receive(:sell_units) - order.finalize! - end - it "should send an order confirmation email" do expect do order.finalize! From 4215dcb9271ea59f4caa7b604e020d0e65e49b18 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 17:29:49 +0100 Subject: [PATCH 12/21] Run transpec on the new specs from spree_core --- spec/models/spree/inventory_unit_spec.rb | 2 +- spec/models/spree/line_item_spec.rb | 6 +- spec/models/spree/order/adjustments_spec.rb | 20 ++--- spec/models/spree/order/callbacks_spec.rb | 2 +- spec/models/spree/order/payment_spec.rb | 12 +-- spec/models/spree/order/state_machine_spec.rb | 80 +++++++++--------- spec/models/spree/order/tax_spec.rb | 26 +++--- spec/models/spree/order/updating_spec.rb | 2 +- spec/models/spree/order_inventory_spec.rb | 34 ++++---- spec/models/spree/order_spec.rb | 82 +++++++++---------- .../models/spree/return_authorization_spec.rb | 30 +++---- 11 files changed, 148 insertions(+), 148 deletions(-) diff --git a/spec/models/spree/inventory_unit_spec.rb b/spec/models/spree/inventory_unit_spec.rb index 87088a61217..e83056c2643 100644 --- a/spec/models/spree/inventory_unit_spec.rb +++ b/spec/models/spree/inventory_unit_spec.rb @@ -15,7 +15,7 @@ shipment.shipping_methods << create(:shipping_method) shipment.order = order # We don't care about this in this test - shipment.stub(:ensure_correct_adjustment) + allow(shipment).to receive(:ensure_correct_adjustment) shipment.tap(&:save!) end diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 4307913a1d2..5aec211ee9b 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -8,8 +8,8 @@ module Spree context '#save' do it 'should update inventory, totals, and tax' do # Regression check for Spree #1481 - line_item.order.should_receive(:create_tax_charge!) - line_item.order.should_receive(:update!) + expect(line_item.order).to receive(:create_tax_charge!) + expect(line_item.order).to receive(:update!) line_item.quantity = 2 line_item.save end @@ -18,7 +18,7 @@ module Spree context '#destroy' do # Regression test for Spree #1481 it "applies tax adjustments" do - line_item.order.should_receive(:create_tax_charge!) + expect(line_item.order).to receive(:create_tax_charge!) line_item.destroy end diff --git a/spec/models/spree/order/adjustments_spec.rb b/spec/models/spree/order/adjustments_spec.rb index bb0101991ea..b3ad28b40d8 100644 --- a/spec/models/spree/order/adjustments_spec.rb +++ b/spec/models/spree/order/adjustments_spec.rb @@ -8,14 +8,14 @@ let(:adjustment) { double("Adjustment") } it "destroys all order adjustments" do - order.stub(adjustments: adjustment) - adjustment.should_receive(:destroy_all) + allow(order).to receive_messages(adjustments: adjustment) + expect(adjustment).to receive(:destroy_all) order.clear_adjustments! end it "destroy all line item adjustments" do - order.stub(line_item_adjustments: adjustment) - adjustment.should_receive(:destroy_all) + allow(order).to receive_messages(line_item_adjustments: adjustment) + expect(adjustment).to receive(:destroy_all) order.clear_adjustments! end end @@ -26,14 +26,14 @@ context "#ship_total" do it "should return the correct amount" do - order.stub_chain :adjustments, shipping: [adjustment1, adjustment2] + allow(order).to receive_message_chain :adjustments, shipping: [adjustment1, adjustment2] expect(order.ship_total).to eq 15 end end context "#tax_total" do it "should return the correct amount" do - order.stub_chain :adjustments, tax: [adjustment1, adjustment2] + allow(order).to receive_message_chain :adjustments, tax: [adjustment1, adjustment2] expect(order.tax_total).to eq 15 end end @@ -43,7 +43,7 @@ before { @order = Spree::Order.create! } context "when there are no line item adjustments" do - before { @order.stub_chain(:line_item_adjustments, eligible: []) } + before { allow(@order).to receive_message_chain(:line_item_adjustments, eligible: []) } it "should return an empty hash" do expect(@order.line_item_adjustment_totals).to eq({}) @@ -55,7 +55,7 @@ let(:adj2) { build(:adjustment, amount: 20, label: "Bar") } before do - @order.stub_chain(:line_item_adjustments, eligible: [adj1, adj2]) + allow(@order).to receive_message_chain(:line_item_adjustments, eligible: [adj1, adj2]) end it "should return exactly two totals" do @@ -74,7 +74,7 @@ let(:adj3) { build(:adjustment, amount: 40, label: "Bar") } before do - @order.stub_chain(:line_item_adjustments, eligible: [adj1, adj2, adj3]) + allow(@order).to receive_message_chain(:line_item_adjustments, eligible: [adj1, adj2, adj3]) end it "should return exactly two totals" do @@ -90,7 +90,7 @@ context "line item adjustments" do before do @order = Spree::Order.create! - @order.stub line_items: [line_item1, line_item2] + allow(@order).to receive_messages line_items: [line_item1, line_item2] end let(:line_item1) { create(:line_item, order: @order) } diff --git a/spec/models/spree/order/callbacks_spec.rb b/spec/models/spree/order/callbacks_spec.rb index 4c8f287650d..a3cb0744ecb 100644 --- a/spec/models/spree/order/callbacks_spec.rb +++ b/spec/models/spree/order/callbacks_spec.rb @@ -24,7 +24,7 @@ let(:user) { double(:user, email: "test@example.com") } before do - order.stub user: user + allow(order).to receive_messages user: user end it "should assign the email address of the user" do diff --git a/spec/models/spree/order/payment_spec.rb b/spec/models/spree/order/payment_spec.rb index a9083035735..2562a8d226e 100644 --- a/spec/models/spree/order/payment_spec.rb +++ b/spec/models/spree/order/payment_spec.rb @@ -12,14 +12,14 @@ module Spree # So that Payment#purchase! is called during processing Spree::Config[:auto_capture] = true - order.stub_chain(:line_items, :empty?).and_return(false) - order.stub total: 100 + allow(order).to receive_message_chain(:line_items, :empty?).and_return(false) + allow(order).to receive_messages total: 100 end it 'processes all payments' do payment1 = create(:payment, amount: 50, payment_method: bogus) payment2 = create(:payment, amount: 50, payment_method: bogus) - order.stub(:pending_payments).and_return([payment1, payment2]) + allow(order).to receive(:pending_payments).and_return([payment1, payment2]) order.process_payments! updater.update_payment_state @@ -33,7 +33,7 @@ module Spree payment1 = create(:payment, amount: 50, payment_method: bogus) payment2 = create(:payment, amount: 50, payment_method: bogus) payment3 = create(:payment, amount: 50, payment_method: bogus) - order.stub(:pending_payments).and_return([payment1, payment2, payment3]) + allow(order).to receive(:pending_payments).and_return([payment1, payment2, payment3]) order.process_payments! updater.update_payment_state @@ -47,9 +47,9 @@ module Spree it "does not use failed payments" do payment1 = create(:payment, amount: 50, payment_method: bogus) payment2 = create(:payment, amount: 50, state: 'failed', payment_method: bogus) - order.stub(:pending_payments).and_return([payment1]) + allow(order).to receive(:pending_payments).and_return([payment1]) - payment2.should_not_receive(:process!) + expect(payment2).not_to receive(:process!) order.process_payments! end diff --git a/spec/models/spree/order/state_machine_spec.rb b/spec/models/spree/order/state_machine_spec.rb index ed5ffb7289b..f9a4164ef13 100644 --- a/spec/models/spree/order/state_machine_spec.rb +++ b/spec/models/spree/order/state_machine_spec.rb @@ -8,7 +8,7 @@ # Ensure state machine has been re-defined correctly Spree::Order.define_state_machine! # We don't care about this validation here - order.stub(:require_email) + allow(order).to receive(:require_email) end context "#next!" do @@ -16,21 +16,21 @@ before do order.state = "payment" order.run_callbacks(:create) - order.stub payment_required?: true - order.stub process_payments!: true - order.stub :has_available_shipment + allow(order).to receive_messages payment_required?: true + allow(order).to receive_messages process_payments!: true + allow(order).to receive :has_available_shipment end context "when payment processing succeeds" do - before { order.stub process_payments!: true } + before { allow(order).to receive_messages process_payments!: true } it "should finalize order when transitioning to complete state" do - order.should_receive(:finalize!) + expect(order).to receive(:finalize!) order.next! end context "when credit card processing fails" do - before { order.stub process_payments!: false } + before { allow(order).to receive_messages process_payments!: false } it "should not complete the order" do order.next @@ -40,7 +40,7 @@ end context "when payment processing fails" do - before { order.stub process_payments!: false } + before { allow(order).to receive_messages process_payments!: false } it "cannot transition to complete" do order.next @@ -51,15 +51,15 @@ context "when current state is address" do before do - order.stub(:has_available_payment) - order.stub(:ensure_available_shipping_rates) + allow(order).to receive(:has_available_payment) + allow(order).to receive(:ensure_available_shipping_rates) order.state = "address" end it "adjusts tax rates when transitioning to delivery" do # Once because the record is being saved # Twice because it is transitioning to the delivery state - Spree::TaxRate.should_receive(:adjust).twice + expect(Spree::TaxRate).to receive(:adjust).twice order.next! end end @@ -67,7 +67,7 @@ context "when current state is delivery" do before do order.state = "delivery" - order.stub total: 10.0 + allow(order).to receive_messages total: 10.0 end end end @@ -75,7 +75,7 @@ context "#can_cancel?" do %w(pending backorder ready).each do |shipment_state| it "should be true if shipment_state is #{shipment_state}" do - order.stub completed?: true + allow(order).to receive_messages completed?: true order.shipment_state = shipment_state expect(order.can_cancel?).to be_truthy end @@ -83,7 +83,7 @@ (Spree::Shipment.state_machine.states.keys - %w(pending backorder ready)).each do |shipment_state| it "should be false if shipment_state is #{shipment_state}" do - order.stub completed?: true + allow(order).to receive_messages completed?: true order.shipment_state = shipment_state expect(order.can_cancel?).to be_falsy end @@ -98,54 +98,54 @@ } let!(:shipment) do shipment = build(:shipment) - shipment.stub inventory_units: inventory_units - order.stub shipments: [shipment] + allow(shipment).to receive_messages inventory_units: inventory_units + allow(order).to receive_messages shipments: [shipment] shipment end before do - order.stub line_items: [build(:line_item, variant: variant, quantity: 2)] - order.line_items.stub find_by_variant_id: order.line_items.first + allow(order).to receive_messages line_items: [build(:line_item, variant: variant, quantity: 2)] + allow(order.line_items).to receive_messages find_by_variant_id: order.line_items.first - order.stub completed?: true - order.stub allow_cancel?: true + allow(order).to receive_messages completed?: true + allow(order).to receive_messages allow_cancel?: true end it "should send a cancel email" do # Stub methods that cause side-effects in this test - shipment.stub(:cancel!) - order.stub :has_available_shipment - order.stub :restock_items! + allow(shipment).to receive(:cancel!) + allow(order).to receive :has_available_shipment + allow(order).to receive :restock_items! mail_message = double "Mail::Message" order_id = nil - Spree::OrderMailer.should_receive(:cancel_email) { |*args| + expect(Spree::OrderMailer).to receive(:cancel_email) { |*args| order_id = args[0] mail_message } - mail_message.should_receive :deliver + expect(mail_message).to receive :deliver order.cancel! expect(order_id).to eq order.id end context "restocking inventory" do before do - shipment.stub(:ensure_correct_adjustment) - shipment.stub(:update_order) - Spree::OrderMailer.stub(:cancel_email).and_return(mail_message = double) - mail_message.stub :deliver + allow(shipment).to receive(:ensure_correct_adjustment) + allow(shipment).to receive(:update_order) + allow(Spree::OrderMailer).to receive(:cancel_email).and_return(mail_message = double) + allow(mail_message).to receive :deliver - order.stub :has_available_shipment + allow(order).to receive :has_available_shipment end end context "resets payment state" do before do # Stubs methods that cause unwanted side effects in this test - Spree::OrderMailer.stub(:cancel_email).and_return(mail_message = double) - mail_message.stub :deliver - order.stub :has_available_shipment - order.stub :restock_items! - shipment.stub(:cancel!) + allow(Spree::OrderMailer).to receive(:cancel_email).and_return(mail_message = double) + allow(mail_message).to receive :deliver + allow(order).to receive :has_available_shipment + allow(order).to receive :restock_items! + allow(shipment).to receive(:cancel!) end context "without shipped items" do @@ -157,7 +157,7 @@ context "with shipped items" do before do - order.stub shipment_state: 'partial' + allow(order).to receive_messages shipment_state: 'partial' end it "should not alter the payment state" do @@ -171,12 +171,12 @@ # Another regression test for Spree #729 context "#resume" do before do - order.stub email: "user@spreecommerce.com" - order.stub state: "canceled" - order.stub allow_resume?: true + allow(order).to receive_messages email: "user@spreecommerce.com" + allow(order).to receive_messages state: "canceled" + allow(order).to receive_messages allow_resume?: true # Stubs method that cause unwanted side effects in this test - order.stub :has_available_shipment + allow(order).to receive :has_available_shipment end end end diff --git a/spec/models/spree/order/tax_spec.rb b/spec/models/spree/order/tax_spec.rb index 780673d1131..dae075c9fa1 100644 --- a/spec/models/spree/order/tax_spec.rb +++ b/spec/models/spree/order/tax_spec.rb @@ -24,8 +24,8 @@ module Spree before { Spree::Config.set(tax_using_ship_address: true) } it "should calculate using ship_address" do - Spree::Zone.should_receive(:match).at_least(:once).with(ship_address) - Spree::Zone.should_not_receive(:match).with(bill_address) + expect(Spree::Zone).to receive(:match).at_least(:once).with(ship_address) + expect(Spree::Zone).not_to receive(:match).with(bill_address) order.tax_zone end end @@ -34,8 +34,8 @@ module Spree before { Spree::Config.set(tax_using_ship_address: false) } it "should calculate using bill_address" do - Spree::Zone.should_receive(:match).at_least(:once).with(bill_address) - Spree::Zone.should_not_receive(:match).with(ship_address) + expect(Spree::Zone).to receive(:match).at_least(:once).with(bill_address) + expect(Spree::Zone).not_to receive(:match).with(ship_address) order.tax_zone end end @@ -43,11 +43,11 @@ module Spree context "when there is a default tax zone" do before do @default_zone = create(:zone, name: "foo_zone") - Spree::Zone.stub default_tax: @default_zone + allow(Spree::Zone).to receive_messages default_tax: @default_zone end context "when there is a matching zone" do - before { Spree::Zone.stub(match: zone) } + before { allow(Spree::Zone).to receive_messages(match: zone) } it "should return the matching zone" do expect(order.tax_zone).to eq zone @@ -55,7 +55,7 @@ module Spree end context "when there is no matching zone" do - before { Spree::Zone.stub(match: nil) } + before { allow(Spree::Zone).to receive_messages(match: nil) } it "should return the default tax zone" do expect(order.tax_zone).to eq @default_zone @@ -64,10 +64,10 @@ module Spree end context "when no default tax zone" do - before { Spree::Zone.stub default_tax: nil } + before { allow(Spree::Zone).to receive_messages default_tax: nil } context "when there is a matching zone" do - before { Spree::Zone.stub(match: zone) } + before { allow(Spree::Zone).to receive_messages(match: zone) } it "should return the matching zone" do expect(order.tax_zone).to eq zone @@ -75,7 +75,7 @@ module Spree end context "when there is no matching zone" do - before { Spree::Zone.stub(match: nil) } + before { allow(Spree::Zone).to receive_messages(match: nil) } it "should return nil" do expect(order.tax_zone).to be_nil @@ -88,19 +88,19 @@ module Spree before do @order = create(:order) @default_zone = create(:zone) - Spree::Zone.stub default_tax: @default_zone + allow(Spree::Zone).to receive_messages default_tax: @default_zone end context "when prices include tax" do before { Spree::Config.set(prices_inc_tax: true) } it "should be true when tax_zone is not the same as the default" do - @order.stub tax_zone: create(:zone, name: "other_zone") + allow(@order).to receive_messages tax_zone: create(:zone, name: "other_zone") expect(@order.exclude_tax?).to be_truthy end it "should be false when tax_zone is the same as the default" do - @order.stub tax_zone: @default_zone + allow(@order).to receive_messages tax_zone: @default_zone expect(@order.exclude_tax?).to be_falsy end end diff --git a/spec/models/spree/order/updating_spec.rb b/spec/models/spree/order/updating_spec.rb index d21d45c2ca0..068fde6be91 100644 --- a/spec/models/spree/order/updating_spec.rb +++ b/spec/models/spree/order/updating_spec.rb @@ -12,7 +12,7 @@ before { Spree::Order.register_update_hook :foo } after { Spree::Order.update_hooks.clear } it "should call each of the update hooks" do - order.should_receive :foo + expect(order).to receive :foo order.update! end end diff --git a/spec/models/spree/order_inventory_spec.rb b/spec/models/spree/order_inventory_spec.rb index 39f48aed0e1..57df61ef140 100644 --- a/spec/models/spree/order_inventory_spec.rb +++ b/spec/models/spree/order_inventory_spec.rb @@ -33,16 +33,16 @@ let(:variant) { create :variant } context "order is not completed" do - before { order.stub completed?: false } + before { allow(order).to receive_messages completed?: false } it "doesn't unstock items" do - shipment.stock_location.should_not_receive(:unstock) + expect(shipment.stock_location).not_to receive(:unstock) expect(subject.send(:add_to_shipment, shipment, variant, 5)).to eq 5 end end it 'should create inventory_units in the necessary states' do - shipment.stock_location.should_receive(:fill_status).with(variant, 5).and_return([3, 2]) + expect(shipment.stock_location).to receive(:fill_status).with(variant, 5).and_return([3, 2]) expect(subject.send(:add_to_shipment, shipment, variant, 5)).to eq 5 @@ -85,10 +85,10 @@ let(:variant) { order.line_items.first.variant } context "order is not completed" do - before { order.stub completed?: false } + before { allow(order).to receive_messages completed?: false } it "doesn't restock items" do - shipment.stock_location.should_not_receive(:restock) + expect(shipment.stock_location).not_to receive(:restock) expect(subject.send(:remove_from_shipment, shipment, variant, 1)).to eq 1 end end @@ -102,40 +102,40 @@ end it 'should destroy backordered units first' do - shipment.stub(inventory_units_for: [build(:inventory_unit, variant_id: variant.id, state: 'backordered'), + allow(shipment).to receive_messages(inventory_units_for: [build(:inventory_unit, variant_id: variant.id, state: 'backordered'), build(:inventory_unit, variant_id: variant.id, state: 'on_hand'), build(:inventory_unit, variant_id: variant.id, state: 'backordered')]) - shipment.inventory_units_for[0].should_receive(:destroy) - shipment.inventory_units_for[1].should_not_receive(:destroy) - shipment.inventory_units_for[2].should_receive(:destroy) + expect(shipment.inventory_units_for[0]).to receive(:destroy) + expect(shipment.inventory_units_for[1]).not_to receive(:destroy) + expect(shipment.inventory_units_for[2]).to receive(:destroy) expect(subject.send(:remove_from_shipment, shipment, variant, 2)).to eq 2 end it 'should destroy unshipped units first' do - shipment.stub(inventory_units_for: [build(:inventory_unit, variant_id: variant.id, state: 'shipped'), + allow(shipment).to receive_messages(inventory_units_for: [build(:inventory_unit, variant_id: variant.id, state: 'shipped'), build(:inventory_unit, variant_id: variant.id, state: 'on_hand')] ) - shipment.inventory_units_for[0].should_not_receive(:destroy) - shipment.inventory_units_for[1].should_receive(:destroy) + expect(shipment.inventory_units_for[0]).not_to receive(:destroy) + expect(shipment.inventory_units_for[1]).to receive(:destroy) expect(subject.send(:remove_from_shipment, shipment, variant, 1)).to eq 1 end it 'only attempts to destroy as many units as are eligible, and return amount destroyed' do - shipment.stub(inventory_units_for: [build(:inventory_unit, variant_id: variant.id, state: 'shipped'), + allow(shipment).to receive_messages(inventory_units_for: [build(:inventory_unit, variant_id: variant.id, state: 'shipped'), build(:inventory_unit, variant_id: variant.id, state: 'on_hand')] ) - shipment.inventory_units_for[0].should_not_receive(:destroy) - shipment.inventory_units_for[1].should_receive(:destroy) + expect(shipment.inventory_units_for[0]).not_to receive(:destroy) + expect(shipment.inventory_units_for[1]).to receive(:destroy) expect(subject.send(:remove_from_shipment, shipment, variant, 1)).to eq 1 end it 'should destroy self if not inventory units remain' do - shipment.inventory_units.stub(count: 0) - shipment.should_receive(:destroy) + allow(shipment.inventory_units).to receive_messages(count: 0) + expect(shipment).to receive(:destroy) expect(subject.send(:remove_from_shipment, shipment, variant, 1)).to eq 1 end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 193981dc812..419c3de6d62 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -7,7 +7,7 @@ let(:order) { build(:order, user: user) } before do - Spree::LegacyUser.stub(current: build(:user, id: 123)) + allow(Spree::LegacyUser).to receive_messages(current: build(:user, id: 123)) end context "#products" do @@ -102,39 +102,39 @@ let(:order) { Spree::Order.create } it "should be true for order in the 'complete' state" do - order.stub(complete?: true) + allow(order).to receive_messages(complete?: true) expect(order.can_ship?).to be_truthy end it "should be true for order in the 'resumed' state" do - order.stub(resumed?: true) + allow(order).to receive_messages(resumed?: true) expect(order.can_ship?).to be_truthy end it "should be true for an order in the 'awaiting return' state" do - order.stub(awaiting_return?: true) + allow(order).to receive_messages(awaiting_return?: true) expect(order.can_ship?).to be_truthy end it "should be true for an order in the 'returned' state" do - order.stub(returned?: true) + allow(order).to receive_messages(returned?: true) expect(order.can_ship?).to be_truthy end it "should be false if the order is neither in the 'complete' nor 'resumed' state" do - order.stub(resumed?: false, complete?: false) + allow(order).to receive_messages(resumed?: false, complete?: false) expect(order.can_ship?).to be_falsy end end context "checking if order is paid" do context "payment_state is paid" do - before { order.stub payment_state: 'paid' } + before { allow(order).to receive_messages payment_state: 'paid' } it { expect(order).to be_paid } end context "payment_state is credit_owned" do - before { order.stub payment_state: 'credit_owed' } + before { allow(order).to receive_messages payment_state: 'credit_owed' } it { expect(order).to be_paid } end end @@ -142,21 +142,21 @@ context "#finalize!" do let(:order) { Spree::Order.create } it "should set completed_at" do - order.should_receive(:touch).with(:completed_at) + expect(order).to receive(:touch).with(:completed_at) order.finalize! end it "should sell inventory units" do order.shipments.each do |shipment| - shipment.should_receive(:update!) - shipment.should_receive(:finalize!) + expect(shipment).to receive(:update!) + expect(shipment).to receive(:finalize!) end order.finalize! end it "should decrease the stock for each variant in the shipment" do order.shipments.each do |shipment| - shipment.stock_location.should_receive(:decrease_stock_for_variant) + expect(shipment.stock_location).to receive(:decrease_stock_for_variant) end order.finalize! end @@ -165,7 +165,7 @@ Spree::Shipment.create(order: order) order.shipments.reload - order.stub(paid?: true, complete?: true) + allow(order).to receive_messages(paid?: true, complete?: true) order.finalize! order.reload # reload so we're sure the changes are persisted expect(order.shipment_state).to eq 'ready' @@ -180,41 +180,41 @@ it "should freeze all adjustments" do # Stub this method as it's called due to a callback # and it's irrelevant to this test - order.stub :has_available_shipment - Spree::OrderMailer.stub_chain :confirm_email, :deliver + allow(order).to receive :has_available_shipment + allow(Spree::OrderMailer).to receive_message_chain :confirm_email, :deliver adjustments = double - order.stub adjustments: adjustments + allow(order).to receive_messages adjustments: adjustments expect(adjustments).to receive(:update_all).with(state: 'closed') order.finalize! end it "should log state event" do - order.state_changes.should_receive(:create).exactly(3).times # order, shipment & payment state changes + expect(order.state_changes).to receive(:create).exactly(3).times # order, shipment & payment state changes order.finalize! end it 'calls updater#before_save' do - order.updater.should_receive(:before_save_hook) + expect(order.updater).to receive(:before_save_hook) order.finalize! end end context "#process_payments!" do let(:payment) { build(:payment) } - before { order.stub pending_payments: [payment], total: 10 } + before { allow(order).to receive_messages pending_payments: [payment], total: 10 } it "should process the payments" do - payment.should_receive(:process!) + expect(payment).to receive(:process!) expect(order.process_payments!).to be_truthy end it "should return false if no pending_payments available" do - order.stub pending_payments: [] + allow(order).to receive_messages pending_payments: [] expect(order.process_payments!).to be_falsy end context "when a payment raises a GatewayError" do - before { payment.should_receive(:process!).and_raise(Spree::Core::GatewayError) } + before { expect(payment).to receive(:process!).and_raise(Spree::Core::GatewayError) } it "should return true when configured to allow checkout on gateway failures" do Spree::Config.set allow_checkout_on_gateway_error: true @@ -271,11 +271,11 @@ context "#allow_checkout?" do it "should be true if there are line_items in the order" do - order.stub_chain(:line_items, count: 1) + allow(order).to receive_message_chain(:line_items, count: 1) expect(order.checkout_allowed?).to be_truthy end it "should be false if there are no line_items in the order" do - order.stub_chain(:line_items, count: 0) + allow(order).to receive_message_chain(:line_items, count: 0) expect(order.checkout_allowed?).to be_falsy end end @@ -321,7 +321,7 @@ let(:line_item) { build(:line_item) } before do - order.stub(line_items: [line_item]) + allow(order).to receive_messages(line_items: [line_item]) allow(line_item).to receive(:insufficient_stock?) { true } end @@ -334,10 +334,10 @@ context "empty!" do it "should clear out all line items and adjustments" do order = build(:order) - order.stub(line_items: line_items = []) - order.stub(adjustments: adjustments = []) - order.line_items.should_receive(:destroy_all) - order.adjustments.should_receive(:destroy_all) + allow(order).to receive_messages(line_items: line_items = []) + allow(order).to receive_messages(adjustments: adjustments = []) + expect(order.line_items).to receive(:destroy_all) + expect(order.adjustments).to receive(:destroy_all) order.empty! end @@ -345,14 +345,14 @@ context "#display_outstanding_balance" do it "returns the value as a spree money" do - order.stub(:outstanding_balance) { 10.55 } + allow(order).to receive(:outstanding_balance) { 10.55 } expect(order.display_outstanding_balance).to eq Spree::Money.new(10.55) end end context "#display_item_total" do it "returns the value as a spree money" do - order.stub(:item_total) { 10.55 } + allow(order).to receive(:item_total) { 10.55 } expect(order.display_item_total).to eq Spree::Money.new(10.55) end end @@ -402,7 +402,7 @@ before do # Don't care about available payment methods in this test - persisted_order.stub(has_available_payment: false) + allow(persisted_order).to receive_messages(has_available_payment: false) persisted_order.line_items << line_item persisted_order.adjustments.create(amount: -line_item.amount, label: "Promotion") persisted_order.state = 'delivery' @@ -410,7 +410,7 @@ end it "transitions from delivery to payment" do - persisted_order.stub(payment_required?: true) + allow(persisted_order).to receive_messages(payment_required?: true) persisted_order.next! expect(persisted_order.state).to eq "payment" end @@ -424,7 +424,7 @@ end context "total > zero" do - before { order.stub(total: 1) } + before { allow(order).to receive_messages(total: 1) } it { expect(order.payment_required?).to be_truthy } end end @@ -442,13 +442,13 @@ it "calls hook during update" do order = create(:order) - order.should_receive(:add_awesome_sauce) + expect(order).to receive(:add_awesome_sauce) order.update! end it "calls hook during finalize" do order = create(:order) - order.should_receive(:add_awesome_sauce) + expect(order).to receive(:add_awesome_sauce) order.finalize! end end @@ -498,7 +498,7 @@ def initialize(decorated_object) end before do - Spree::Config.stub(:order_updater_decorator) { FakeOrderUpdaterDecorator } + allow(Spree::Config).to receive(:order_updater_decorator) { FakeOrderUpdaterDecorator } end it 'returns an order_updater_decorator class' do @@ -514,7 +514,7 @@ def initialize(decorated_object) let(:order) { build(:order) } it "has errors if email is blank" do - order.stub(require_email: true) + allow(order).to receive_messages(require_email: true) order.email = "" order.valid? @@ -522,7 +522,7 @@ def initialize(decorated_object) end it "has errors if email is invalid" do - order.stub(require_email: true) + allow(order).to receive_messages(require_email: true) order.email = "invalid_email" order.valid? @@ -530,7 +530,7 @@ def initialize(decorated_object) end it "has errors if email has invalid domain" do - order.stub(require_email: true) + allow(order).to receive_messages(require_email: true) order.email = "single_letter_tld@domain.z" order.valid? @@ -538,7 +538,7 @@ def initialize(decorated_object) end it "is valid if email is valid" do - order.stub(require_email: true) + allow(order).to receive_messages(require_email: true) order.email = "a@b.ca" order.valid? diff --git a/spec/models/spree/return_authorization_spec.rb b/spec/models/spree/return_authorization_spec.rb index 893965850ac..2b0b9db5473 100644 --- a/spec/models/spree/return_authorization_spec.rb +++ b/spec/models/spree/return_authorization_spec.rb @@ -15,7 +15,7 @@ end it "should generate RMA number" do - return_authorization.should_receive(:generate_number) + expect(return_authorization).to receive(:generate_number) return_authorization.save end end @@ -33,7 +33,7 @@ end it "should update order state" do - order.should_receive(:authorize_return!) + expect(order).to receive(:authorize_return!) return_authorization.add_variant(variant.id, 1) end end @@ -56,12 +56,12 @@ context "can_receive?" do it "should allow_receive when inventory units assigned" do - return_authorization.stub(inventory_units: [1, 2, 3]) + allow(return_authorization).to receive_messages(inventory_units: [1, 2, 3]) expect(return_authorization.can_receive?).to be_truthy end it "should not allow_receive with no inventory units" do - return_authorization.stub(inventory_units: []) + allow(return_authorization).to receive_messages(inventory_units: []) expect(return_authorization.can_receive?).to be_falsy end end @@ -70,28 +70,28 @@ let(:inventory_unit) { order.shipments.first.inventory_units.first } before do - return_authorization.stub(inventory_units: [inventory_unit], amount: -20) - Spree::Adjustment.stub(:create) - order.stub(:update!) + allow(return_authorization).to receive_messages(inventory_units: [inventory_unit], amount: -20) + allow(Spree::Adjustment).to receive(:create) + allow(order).to receive(:update!) end it "should mark all inventory units are returned" do - inventory_unit.should_receive(:return!) + expect(inventory_unit).to receive(:return!) return_authorization.receive! end it "should add credit for specified amount" do return_authorization.amount = 20 mock_adjustment = double - mock_adjustment.should_receive(:source=).with(return_authorization) - mock_adjustment.should_receive(:adjustable=).with(order) - mock_adjustment.should_receive(:save) - Spree::Adjustment.should_receive(:new).with(amount: -20, label: Spree.t(:rma_credit)).and_return(mock_adjustment) + expect(mock_adjustment).to receive(:source=).with(return_authorization) + expect(mock_adjustment).to receive(:adjustable=).with(order) + expect(mock_adjustment).to receive(:save) + expect(Spree::Adjustment).to receive(:new).with(amount: -20, label: Spree.t(:rma_credit)).and_return(mock_adjustment) return_authorization.receive! end it "should update order state" do - order.should_receive :update! + expect(order).to receive :update! return_authorization.receive! end end @@ -106,13 +106,13 @@ context "after_save" do it "should run correct callbacks" do - return_authorization.should_receive(:force_positive_amount) + expect(return_authorization).to receive(:force_positive_amount) return_authorization.run_callbacks(:save) end end context "currency" do - before { order.stub(:currency) { "ABC" } } + before { allow(order).to receive(:currency) { "ABC" } } it "returns the order currency" do expect(return_authorization.currency).to eq "ABC" end From 7884dbfeb17e7bf8e415a3db239a1655ff4d506f Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 8 Aug 2020 20:36:34 +0100 Subject: [PATCH 13/21] Revert rubocop autocorrect, each is needed here for find_each is not available for Arrays --- app/models/spree/order.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 7af93671933..97c5f899833 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -720,7 +720,7 @@ def remove_variant(variant) end def cap_quantity_at_stock! - line_items.includes(variant: :stock_items).all.find_each(&:cap_quantity_at_stock!) + line_items.includes(variant: :stock_items).all.each(&:cap_quantity_at_stock!) end def set_distributor!(distributor) From c49dbec85ade4b1d3fa7d3c2d83b2d17bf700b1c Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 21 Aug 2020 11:31:52 +0100 Subject: [PATCH 14/21] Adapt order_spec to new updater code --- spec/models/spree/order_spec.rb | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 419c3de6d62..48a1a50d7f3 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -489,24 +489,8 @@ end context '#updater' do - class FakeOrderUpdaterDecorator - attr_reader :decorated_object - - def initialize(decorated_object) - @decorated_object = decorated_object - end - end - - before do - allow(Spree::Config).to receive(:order_updater_decorator) { FakeOrderUpdaterDecorator } - end - - it 'returns an order_updater_decorator class' do - expect(order.updater.class).to eq FakeOrderUpdaterDecorator - end - - it 'decorates a Spree::OrderUpdater' do - expect(order.updater.decorated_object.class).to eq Spree::OrderUpdater + it 'returns an OrderManagement::Order::Updater' do + expect(order.updater.class).to eq OrderManagement::Order::Updater end end From 03419bbc3508f55b35800bad51b2f51ed8815dc9 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 1 Sep 2020 12:10:46 +0100 Subject: [PATCH 15/21] Remove all and use find_each instead! --- app/models/spree/order.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 97c5f899833..b6c5a99a110 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -720,7 +720,7 @@ def remove_variant(variant) end def cap_quantity_at_stock! - line_items.includes(variant: :stock_items).all.each(&:cap_quantity_at_stock!) + line_items.includes(variant: :stock_items).find_each(&:cap_quantity_at_stock!) end def set_distributor!(distributor) From a5ff4d6853dab7c102d1edf949cb4eeb2bc6284e Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 2 Sep 2020 20:42:50 +0100 Subject: [PATCH 16/21] Remove unneeded setup code and remove unnecessary reference to FactoryGirl --- spec/models/spree/order_spec.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 48a1a50d7f3..77fa763f46a 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -6,10 +6,6 @@ let(:user) { build(:user, email: "spree@example.com") } let(:order) { build(:order, user: user) } - before do - allow(Spree::LegacyUser).to receive_messages(current: build(:user, id: 123)) - end - context "#products" do let(:order) { create(:order_with_line_items) } @@ -36,8 +32,8 @@ context "#associate_user!" do it "should associate a user with a persisted order" do - order = FactoryGirl.create(:order_with_line_items, created_by: nil) - user = FactoryGirl.create(:user) + order = create(:order_with_line_items, created_by: nil) + user = create(:user) order.user = nil order.email = nil @@ -55,8 +51,8 @@ it "should not overwrite the created_by if it already is set" do creator = create(:user) - order = FactoryGirl.create(:order_with_line_items, created_by: creator) - user = FactoryGirl.create(:user) + order = create(:order_with_line_items, created_by: creator) + user = create(:user) order.user = nil order.email = nil From e0d731b92b5d6fa81829ba37c4ec4b14f47f0ed9 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 5 Sep 2020 15:01:06 +0100 Subject: [PATCH 17/21] Remove unused email validator --- app/models/spree/order.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index b6c5a99a110..284d4a33b3d 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require 'spree/core/validators/email' require 'spree/order/checkout' require 'open_food_network/enterprise_fee_calculator' require 'open_food_network/feature_toggle' From f81d4596aa7d3c2c01013e8b0d6864ed864e2656 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 4 Sep 2020 00:24:05 +0100 Subject: [PATCH 18/21] Use correct updater --- .../admin/orders/payments/payments_controller_refunds_spec.rb | 1 + spec/models/spree/order/payment_spec.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb index 08e923b5c35..0342180a009 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require 'spree/core/gateway_error' describe Spree::Admin::PaymentsController, type: :controller do let!(:shop) { create(:enterprise) } diff --git a/spec/models/spree/order/payment_spec.rb b/spec/models/spree/order/payment_spec.rb index 2562a8d226e..94087fa7990 100644 --- a/spec/models/spree/order/payment_spec.rb +++ b/spec/models/spree/order/payment_spec.rb @@ -5,7 +5,7 @@ module Spree describe Spree::Order do let(:order) { build(:order) } - let(:updater) { Spree::OrderUpdater.new(order) } + let(:updater) { OrderManagement::Order::Updater.new(order) } let(:bogus) { create(:bogus_payment_method, distributors: [create(:enterprise)]) } before do From fe7cf0cf44ac296f5663c3f030affa38556304c9 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 3 Sep 2020 22:34:14 +0100 Subject: [PATCH 19/21] Remove reference to FactoryGirl --- spec/models/spree/return_authorization_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/spree/return_authorization_spec.rb b/spec/models/spree/return_authorization_spec.rb index 2b0b9db5473..d37bac00ed0 100644 --- a/spec/models/spree/return_authorization_spec.rb +++ b/spec/models/spree/return_authorization_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Spree::ReturnAuthorization do - let(:order) { FactoryGirl.create(:shipped_order) } + let(:order) { create(:shipped_order) } let(:variant) { order.shipments.first.inventory_units.first.variant } let(:return_authorization) { Spree::ReturnAuthorization.new(order: order) } From 4b597ada12db798b6e48662bab6a6ea8d619c577 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Sat, 5 Sep 2020 16:43:04 +0100 Subject: [PATCH 20/21] Fix easy rubocop issues --- app/models/spree/line_item.rb | 4 ++-- app/models/spree/order.rb | 14 +++++++------- app/models/spree/order_inventory.rb | 4 ++-- app/models/spree/return_authorization.rb | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index f71477cd06c..61aa00eb065 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -121,12 +121,12 @@ def amount alias total amount def single_money - Spree::Money.new(price, { currency: currency }) + Spree::Money.new(price, currency: currency) end alias single_display_amount single_money def money - Spree::Money.new(amount, { currency: currency }) + Spree::Money.new(amount, currency: currency) end alias display_total money alias display_amount money diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 284d4a33b3d..047b50b82e5 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -180,27 +180,27 @@ def currency end def display_outstanding_balance - Spree::Money.new(outstanding_balance, { currency: currency }) + Spree::Money.new(outstanding_balance, currency: currency) end def display_item_total - Spree::Money.new(item_total, { currency: currency }) + Spree::Money.new(item_total, currency: currency) end def display_adjustment_total - Spree::Money.new(adjustment_total, { currency: currency }) + Spree::Money.new(adjustment_total, currency: currency) end def display_tax_total - Spree::Money.new(tax_total, { currency: currency }) + Spree::Money.new(tax_total, currency: currency) end def display_ship_total - Spree::Money.new(ship_total, { currency: currency }) + Spree::Money.new(ship_total, currency: currency) end def display_total - Spree::Money.new(total, { currency: currency }) + Spree::Money.new(total, currency: currency) end def to_param @@ -266,7 +266,7 @@ def tax_address def line_item_adjustment_totals Hash[line_item_adjustments.eligible.group_by(&:label).map do |label, adjustments| total = adjustments.sum(&:amount) - [label, Spree::Money.new(total, { currency: currency })] + [label, Spree::Money.new(total, currency: currency)] end] end diff --git a/app/models/spree/order_inventory.rb b/app/models/spree/order_inventory.rb index 2a0554a29fa..8416bb47553 100644 --- a/app/models/spree/order_inventory.rb +++ b/app/models/spree/order_inventory.rb @@ -46,10 +46,10 @@ def remove(line_item, variant_units, shipment = nil) if shipment.present? remove_from_shipment(shipment, line_item.variant, quantity) else - order.shipments.each do |shipment| + order.shipments.each do |each_shipment| break if quantity == 0 - quantity -= remove_from_shipment(shipment, line_item.variant, quantity) + quantity -= remove_from_shipment(each_shipment, line_item.variant, quantity) end end end diff --git a/app/models/spree/return_authorization.rb b/app/models/spree/return_authorization.rb index 6419d7dde18..41c17cb10f6 100644 --- a/app/models/spree/return_authorization.rb +++ b/app/models/spree/return_authorization.rb @@ -29,7 +29,7 @@ def currency end def display_amount - Spree::Money.new(amount, { currency: currency }) + Spree::Money.new(amount, currency: currency) end def add_variant(variant_id, quantity) From a4c8380d7e2c8adb05649ae6344287a3bd4626df Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 6 Oct 2020 13:00:43 +0100 Subject: [PATCH 21/21] Remove removal of transition to confirm as confirm does not exist anymore --- app/models/spree/order.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 047b50b82e5..d44455d53b1 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -24,7 +24,6 @@ class Order < ActiveRecord::Base order.payment_required? } go_to_state :complete - remove_transition from: :delivery, to: :confirm end state_machine.after_transition to: :payment, do: :charge_shipping_and_payment_fees!