Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Update line items enterprise fee instead of deleting and recreating " #13218

Merged
merged 1 commit into from
Mar 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/models/enterprise_fee.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ class EnterpriseFee < ApplicationRecord
joins(:calculator).where(spree_calculators: { type: PER_ORDER_CALCULATORS })
}

def self.clear_order_adjustments(order)
order.all_adjustments.enterprise_fee.where.not(adjustable_type: "Spree::LineItem").destroy_all
def self.clear_all_adjustments(order)
order.all_adjustments.enterprise_fee.destroy_all
end

private
Expand Down
2 changes: 1 addition & 1 deletion app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def states

delegate :admin_and_handling_total, :payment_fee, :ship_total, to: :adjustments_fetcher
delegate :update_totals, :update_totals_and_states, to: :updater
delegate :create_order_fees!, :update_order_fees!,
delegate :create_line_item_fees!, :create_order_fees!, :update_order_fees!,
:update_line_item_fees!, :recreate_all_fees!, to: :fee_handler

validates :customer, presence: true, if: :require_customer?
Expand Down
50 changes: 6 additions & 44 deletions app/services/orders/handle_fees_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,21 @@ def recreate_all_fees!
# 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
order.with_lock do
EnterpriseFee.clear_order_adjustments order
EnterpriseFee.clear_all_adjustments order

# To prevent issue with fee being removed when a product is not linked to the order cycle
# anymore, we now create or update line item fees.
# Previously fees were deleted and recreated, like we still do for order fees.
create_or_update_line_item_fees!
create_line_item_fees!
create_order_fees!
end

tax_enterprise_fees! unless order.before_payment_state?
order.update_order!
end

def create_or_update_line_item_fees!
order.line_items.includes(:variant).each do |line_item|
# No fee associated with the line item so we just create them
if line_item.enterprise_fee_adjustments.blank?
create_line_item_fees!(line_item)
next
def create_line_item_fees!
order.line_items.includes(variant: :product).each do |line_item|
if provided_by_order_cycle? line_item
calculator.create_line_item_adjustments_for line_item
end

create_or_update_line_item_fee!(line_item)

delete_removed_fees!(line_item)
end
end

Expand Down Expand Up @@ -75,34 +66,5 @@ def provided_by_order_cycle?(line_item)
@order_cycle_variant_ids ||= order_cycle&.variants&.map(&:id) || []
@order_cycle_variant_ids.include? line_item.variant_id
end

def create_line_item_fees!(line_item)
return unless provided_by_order_cycle? line_item

calculator.create_line_item_adjustments_for(line_item)
end

def create_or_update_line_item_fee!(line_item)
fee_applicators(line_item.variant).each do |fee_applicator|
fee_adjustment = line_item.adjustments.find_by(originator: fee_applicator.enterprise_fee)

if fee_adjustment
fee_adjustment.update_adjustment!(line_item, force: true)
elsif provided_by_order_cycle? line_item
fee_applicator.create_line_item_adjustment(line_item)
end
end
end

def delete_removed_fees!(line_item)
order_cycle_fees = fee_applicators(line_item.variant).map(&:enterprise_fee)
removed_fees = line_item.enterprise_fee_adjustments.where.not(originator: order_cycle_fees)

removed_fees.each(&:destroy)
end

def fee_applicators(variant)
calculator.order_cycle_per_item_enterprise_fee_applicators_for(variant)
end
end
end
27 changes: 5 additions & 22 deletions lib/open_food_network/enterprise_fee_calculator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,13 @@ def per_item_enterprise_fee_applicators_for(variant)

@order_cycle.exchanges_carrying(variant, @distributor).each do |exchange|
exchange.enterprise_fees.per_item.each do |enterprise_fee|
fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, exchange.role)
fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, variant,
exchange.role)
end
end

@order_cycle.coordinator_fees.per_item.each do |enterprise_fee|
fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator')
fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator')
end

fees
Expand All @@ -85,30 +86,12 @@ def per_order_enterprise_fee_applicators_for(order)

@order_cycle.exchanges_supplying(order).each do |exchange|
exchange.enterprise_fees.per_order.each do |enterprise_fee|
fees << EnterpriseFeeApplicator.new(enterprise_fee, nil, exchange.role)
fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, nil, exchange.role)
end
end

@order_cycle.coordinator_fees.per_order.each do |enterprise_fee|
fees << EnterpriseFeeApplicator.new(enterprise_fee, nil, 'coordinator')
end

fees
end

def order_cycle_per_item_enterprise_fee_applicators_for(variant)
fees = []

return fees unless @order_cycle && @distributor

@order_cycle.exchanges.supplying_to(@distributor).each do |exchange|
exchange.enterprise_fees.per_item.each do |enterprise_fee|
fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, exchange.role)
end
end

@order_cycle.coordinator_fees.per_item.each do |enterprise_fee|
fees << EnterpriseFeeApplicator.new(enterprise_fee, variant, 'coordinator')
fees << OpenFoodNetwork::EnterpriseFeeApplicator.new(enterprise_fee, nil, 'coordinator')
end

fees
Expand Down
Loading
Loading