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

Place backorders for linked products via DFC integration #12856

Merged
merged 37 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
260e4f7
Create BackorderJob to place wholesale orders
mkllnk Mar 20, 2024
98966f6
Place backorders for DFC products
mkllnk Mar 26, 2024
439f0ca
Raise errors on DFC requests
mkllnk Aug 23, 2024
6c6927a
Add SaleSession with correct OrderCycle times
mkllnk Aug 23, 2024
827e37c
Start moving backorder logic to service
mkllnk Aug 23, 2024
caa6d28
Find and update existing open order
mkllnk Aug 30, 2024
a7a3889
Add needed quantities to existing line items
mkllnk Sep 4, 2024
f839452
Complete an open order
mkllnk Sep 6, 2024
c7fa3ff
Simplify order update logic
mkllnk Sep 6, 2024
3e0eb87
Simplify service with ivar
mkllnk Sep 6, 2024
7b286ea
Complete test for FDC Orders API
mkllnk Sep 6, 2024
3849db7
Simplify order update call
mkllnk Sep 6, 2024
3ec53a7
Parse updated order result
mkllnk Sep 6, 2024
c0ae2ed
Complete order 4 hours after order cycle closed
mkllnk Sep 6, 2024
8f4f873
Move offer finding into separate class
mkllnk Sep 10, 2024
14c32c0
Reduce complexity
mkllnk Sep 10, 2024
efe2b72
Find wholesale offer for retail variant
mkllnk Sep 10, 2024
95bc0cc
Reduce complexity of BackorderJob
mkllnk Sep 11, 2024
c948efd
Add structure to adjust final quantities
mkllnk Sep 11, 2024
95e620a
Add lookup of variants by semantic id
mkllnk Sep 12, 2024
283db8f
Adjust quantities of backorder before completion
mkllnk Sep 12, 2024
5ef85ae
Handle backorder cancellations
mkllnk Sep 13, 2024
2eec4c7
Apply 4 hour completion delay only to one enterprise
mkllnk Sep 13, 2024
4303f0e
Build API URLs to work with any FDC Shopify shop
mkllnk Sep 13, 2024
fb96f8f
Fall back to given product w/o wholesale variant
mkllnk Sep 16, 2024
070b93c
Fall back to givin product id w/o retail variant
mkllnk Sep 17, 2024
7f62b49
Move catalog loading to where it's needed
mkllnk Sep 17, 2024
66f0802
Import DFC product images
mkllnk Sep 17, 2024
9f43244
Import on-demand stock setting in DFC import
mkllnk Sep 19, 2024
2465780
Import prices and stock levels from DFC catalog
mkllnk Sep 25, 2024
eece738
Restore concurrency spec for the checkout
mkllnk Sep 25, 2024
61fec65
Abstract OrderLocker for re-use
mkllnk Sep 25, 2024
e31e45b
Place backorders in the background
mkllnk Sep 25, 2024
49fd1dc
Report backorder errors instead of failing checkout
mkllnk Sep 25, 2024
495634b
Send error notification to owner
mkllnk Sep 25, 2024
989a6d5
Notify user of failed backorder completion
mkllnk Sep 26, 2024
51b3770
Keep failed backorder job in dead set
mkllnk Sep 26, 2024
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
107 changes: 107 additions & 0 deletions app/jobs/backorder_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# frozen_string_literal: true

class BackorderJob < ApplicationJob
# In the current FDC project, one shop wants to review and adjust orders
# before finalising. They also run a market stall and need to adjust stock
# levels after the market. This should be done within four hours.
SALE_SESSION_DELAYS = {
# https://openfoodnetwork.org.uk/handleyfarm/shop
"https://openfoodnetwork.org.uk/api/dfc/enterprises/203468" => 4.hours,
}.freeze

queue_as :default
sidekiq_options retry: 0

def self.check_stock(order)
variants_needing_stock = order.variants.select do |variant|
# TODO: scope variants to hub.
# We are only supporting producer stock at the moment.
variant.on_hand&.negative?
end

linked_variants = variants_needing_stock.select do |variant|
variant.semantic_links.present?
end

perform_later(order, linked_variants) if linked_variants.present?
rescue StandardError => e
# Errors here shouldn't affect the checkout. So let's report them
# separately:
Bugsnag.notify(e) do |payload|
payload.add_metadata(:order, order)
end
end

def perform(order, linked_variants)
OrderLocker.lock_order_and_variants(order) do
place_backorder(order, linked_variants)
end
rescue StandardError
# If the backordering fails, we need to tell the shop owner because they
# need to organgise more stock.
BackorderMailer.backorder_failed(order, linked_variants).deliver_later

raise
end

def place_backorder(order, linked_variants)
user = order.distributor.owner

# We are assuming that all variants are linked to the same wholesale
# shop and its catalog:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with that assumption (for now 😉 ), and it will be true for the pilots we're running.

urls = FdcUrlBuilder.new(linked_variants[0].semantic_links[0].semantic_id)
orderer = FdcBackorderer.new(user, urls)

backorder = orderer.find_or_build_order(order)
broker = load_broker(order.distributor.owner, urls)
ordered_quantities = {}

linked_variants.each do |variant|
retail_quantity = add_item_to_backorder(variant, broker, backorder, orderer)
ordered_quantities[variant] = retail_quantity
end

place_order(user, order, orderer, backorder)

linked_variants.each do |variant|
variant.on_hand += ordered_quantities[variant]
end
end

def add_item_to_backorder(variant, broker, backorder, orderer)
needed_quantity = -1 * variant.on_hand
solution = broker.best_offer(variant.semantic_links[0].semantic_id)

# The number of wholesale packs we need to order to fulfill the
# needed quantity.
# For example, we order 2 packs of 12 cans if we need 15 cans.
wholesale_quantity = (needed_quantity.to_f / solution.factor).ceil

# The number of individual retail items we get with the wholesale order.
# For example, if we order 2 packs of 12 cans, we will get 24 cans
# and we'll account for that in our stock levels.
retail_quantity = wholesale_quantity * solution.factor

line = orderer.find_or_build_order_line(backorder, solution.offer)
line.quantity = line.quantity.to_i + wholesale_quantity

retail_quantity
end

def load_broker(user, urls)
FdcOfferBroker.new(user, urls)
end

def place_order(user, order, orderer, backorder)
placed_order = orderer.send_order(backorder)

return unless orderer.new?(backorder)

delay = SALE_SESSION_DELAYS.fetch(backorder.client, 1.minute)
wait_until = order.order_cycle.orders_close_at + delay
CompleteBackorderJob.set(wait_until:)
.perform_later(
user, order.distributor, order.order_cycle, placed_order.semanticId
)
end
end
62 changes: 62 additions & 0 deletions app/jobs/complete_backorder_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# frozen_string_literal: true

# After an order cycle closed, we need to finalise open draft orders placed
# to replenish stock.
class CompleteBackorderJob < ApplicationJob
sidekiq_options retry: 0

# Required parameters:
#
# * user: to authenticate DFC requests
# * distributor: to reconile with its catalog
# * order_cycle: to scope the catalog when looking up variants
# Multiple variants can be linked to the same remote product.
# To reduce ambiguity, we'll reconcile only with products
# from the given distributor in a given order cycle for which
# the remote backorder was placed.
# * order_id: the remote semantic id of a draft order
# Having the id makes sure that we don't accidentally finalise
# someone else's order.
def perform(user, distributor, order_cycle, order_id)
order = FdcBackorderer.new(user, nil).find_order(order_id)
urls = FdcUrlBuilder.new(order.lines[0].offer.offeredItem.semanticId)

variants = order_cycle.variants_distributed_by(distributor)
adjust_quantities(user, order, urls, variants)

FdcBackorderer.new(user, urls).complete_order(order)
rescue StandardError
BackorderMailer.backorder_incomplete(user, distributor, order_cycle, order_id).deliver_later

raise
end

# Check if we have enough stock to reduce the backorder.
#
# Our local stock can increase when users cancel their orders.
# But stock levels could also have been adjusted manually. So we review all
# quantities before finalising the order.
def adjust_quantities(user, order, urls, variants)
broker = FdcOfferBroker.new(user, urls)

order.lines.each do |line|
line.quantity = line.quantity.to_i
wholesale_product_id = line.offer.offeredItem.semanticId
transformation = broker.wholesale_to_retail(wholesale_product_id)
linked_variant = variants.linked_to(transformation.retail_product_id)

# Note that a division of integers dismisses the remainder, like `floor`:
wholesale_items_contained_in_stock = linked_variant.on_hand / transformation.factor

# But maybe we didn't actually order that much:
deductable_quantity = [line.quantity, wholesale_items_contained_in_stock].min
line.quantity -= deductable_quantity

retail_stock_changes = deductable_quantity * transformation.factor
linked_variant.on_hand -= retail_stock_changes
end

# Clean up empty lines:
order.lines.reject! { |line| line.quantity.zero? }
end
end
24 changes: 24 additions & 0 deletions app/mailers/backorder_mailer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

class BackorderMailer < ApplicationMailer
include I18nHelper

def backorder_failed(order, linked_variants)
@order = order
@linked_variants = linked_variants

I18n.with_locale valid_locale(order.distributor.owner) do
mail(to: order.distributor.owner.email)
end
end

def backorder_incomplete(user, distributor, order_cycle, order_id)
@distributor = distributor
@order_cycle = order_cycle
@order_id = order_id

I18n.with_locale valid_locale(user) do
mail(to: user.email)
end
end
end
2 changes: 2 additions & 0 deletions app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,8 @@ def finalize!

deliver_order_confirmation_email

BackorderJob.check_stock(self)

state_changes.create(
previous_state: 'cart',
next_state: 'complete',
Expand Down
7 changes: 7 additions & 0 deletions app/models/spree/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ def self.active(currency = nil)
select("spree_variants.id") })
end

def self.linked_to(semantic_id)
includes(:semantic_links).references(:semantic_links)
.where(semantic_links: { semantic_id: }).first
dacook marked this conversation as resolved.
Show resolved Hide resolved
end

def tax_category
super || TaxCategory.find_by(is_default: true)
end
Expand Down Expand Up @@ -235,6 +240,8 @@ def set_cost_currency
end

def create_stock_items
return unless stock_items.empty?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not covered directly by spec, but not a big deal.


StockLocation.find_each do |stock_location|
stock_location.propagate_variant(self)
end
Expand Down
29 changes: 1 addition & 28 deletions app/services/current_order_locker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,6 @@ class CurrentOrderLocker
# https://guides.rubyonrails.org/action_controller_overview.html#filters
#
def self.around(controller, &)
lock_order_and_variants(controller.current_order, &)
OrderLocker.lock_order_and_variants(controller.current_order, &)
end

# Locking will not prevent all access to these rows. Other processes are
# only waiting if they try to lock one of these rows as well.
#
# https://api.rubyonrails.org/classes/ActiveRecord/Locking/Pessimistic.html
#
def self.lock_order_and_variants(order)
return yield if order.nil?

order.with_lock do
lock_variants_of(order)
yield
end
end
private_class_method :lock_order_and_variants

# There are many places in which stock is stored in the database. Row locking
# on variant level ensures that there are no conflicts even when an item is
# sold through multiple shops.
def self.lock_variants_of(order)
variant_ids = order.line_items.select(:variant_id)

# Ordering the variants by id prevents deadlocks. Plucking the ids sends
# the locking query without building Spree::Variant objects.
Spree::Variant.where(id: variant_ids).order(:id).lock.pluck(:id)
end
private_class_method :lock_variants_of
end
133 changes: 133 additions & 0 deletions app/services/fdc_backorderer.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
# frozen_string_literal: true

# Place and update orders based on missing stock.
class FdcBackorderer
attr_reader :user, :urls

def initialize(user, urls)
@user = user
@urls = urls
end

def find_or_build_order(ofn_order)
find_open_order || build_new_order(ofn_order)
end

def build_new_order(ofn_order)
OrderBuilder.new_order(ofn_order, urls.orders_url).tap do |order|
order.saleSession = build_sale_session(ofn_order)
end
end

def find_open_order
graph = import(urls.orders_url)
open_orders = graph&.select do |o|
o.semanticType == "dfc-b:Order" && o.orderStatus[:path] == "Held"
end

return if open_orders.blank?

# If there are multiple open orders, we don't know which one to choose.
# We want the order we placed for the same distributor in the same order
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I'm pretty sure the Shopify Orders endpoint will display old (completed orders), we should only have 1 Draft order though.

The Shopify Hub app is storing the ID of the Order returned from the initial POST request, to utilise in future PUT requests. Could we do that ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could. And maybe we need to. I tried to get around it to have minimal local data. Local data structures increase maintenance and can become out of sync. And I was worried about longer dev times as well. But I agree that the current solution isn't great.

# cycle before. So here are some assumptions for this to work:
#
# * We see only orders for our distributor. The endpoint URL contains the
# the distributor name and is currently hardcoded.
# * There's only one open order cycle at a time. Otherwise we may select
# an order of an old order cycle.
# * Orders are finalised when the order cycle closes. So _Held_ orders
# always belong to an open order cycle.
# * We see only our own orders. This assumption is wrong. The Shopify
# integration places held orders as well and they are visible to us.
#
# Unfortunately, the endpoint doesn't tell who placed the order.
# TODO: We need to remember the link to the order locally.
# Or the API is updated to include the orderer.
#
# For now, we just guess:
open_orders.last.tap do |order|
# The DFC Connector doesn't recognise status values properly yet.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkllnk could you elaborate on what you mean by "DFC Connector doesn't recognise status values properly"? 😟

I thought this was sorted & we haven't had any issues with the TS Connector. @lecoqlibre do we need to update the Ruby Connector?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we can load the vocabulary with order states. The Connector doesn't have a built-in method for that but I did make it work in a separate branch. I could work on a pull request to make this part work with the importer. I was just shying away from the effort to meet the deadline. The current version is also not recognising the transformation links. That may just be the outdated context though. We are still on 1.8, I think. And I wasn't keen to follow the updates closely because you discovered bugs here and there. Happy to give it another go though.

# So we are overriding the value with something that can be exported.
order.orderStatus = "dfc-v:Held"
end
end

def find_order(semantic_id)
find_subject(import(semantic_id), "dfc-b:Order")
end

def find_or_build_order_line(order, offer)
find_order_line(order, offer) || build_order_line(order, offer)
end

def build_order_line(order, offer)
# Order lines are enumerated in the FDC API and we must assign a unique
# semantic id. We need to look at current ids to avoid collisions.
# existing_ids = order.lines.map do |line|
# line.semanticId.match(/[0-9]+$/).to_s.to_i
# end
# next_id = existing_ids.max.to_i + 1

# Suggested by FDC team:
next_id = order.lines.count + 1

OrderLineBuilder.build(offer, 0).tap do |line|
line.semanticId = "#{order.semanticId}/OrderLines/#{next_id}"
order.lines << line
end
end

def find_order_line(order, offer)
order.lines.find do |line|
line.offer.offeredItem.semanticId == offer.offeredItem.semanticId
end
end

def find_subject(object_or_graph, type)
if object_or_graph.is_a?(Array)
object_or_graph.find { |i| i.semanticType == type }
else
object_or_graph
end
end

def import(url)
api = DfcRequest.new(user)
json = api.call(url)
DfcIo.import(json)
end

def send_order(backorder)
lines = backorder.lines
offers = lines.map(&:offer)
products = offers.map(&:offeredItem)
sessions = [backorder.saleSession].compact
json = DfcIo.export(backorder, *lines, *offers, *products, *sessions)

api = DfcRequest.new(user)

method = if new?(backorder)
:post # -> create
else
:put # -> update
end

result = api.call(backorder.semanticId, json, method:)
find_subject(DfcIo.import(result), "dfc-b:Order")
end

def complete_order(backorder)
backorder.orderStatus = "dfc-v:Complete"
send_order(backorder)
end

def new?(order)
order.semanticId == urls.orders_url
end

def build_sale_session(order)
SaleSessionBuilder.build(order.order_cycle).tap do |session|
session.semanticId = urls.sale_session_url
end
end
end
Loading
Loading