Skip to content

Commit

Permalink
Merge pull request #12960 from mkllnk/dfc-link-backorder
Browse files Browse the repository at this point in the history
Store link to open backorder
  • Loading branch information
mkllnk authored Nov 21, 2024
2 parents d0dcc92 + 46048dc commit 3ec8cd2
Show file tree
Hide file tree
Showing 21 changed files with 290 additions and 105 deletions.
2 changes: 1 addition & 1 deletion app/jobs/amend_backorder_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def amend_backorder(order)
urls = FdcUrlBuilder.new(reference_link)
orderer = FdcBackorderer.new(user, urls)

backorder = orderer.find_open_order
backorder = orderer.find_open_order(order)

variants = order_cycle.variants_distributed_by(distributor)
adjust_quantities(order_cycle, user, backorder, urls, variants)
Expand Down
4 changes: 3 additions & 1 deletion app/jobs/backorder_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class BackorderJob < ApplicationJob
sidekiq_options retry: 0

def self.check_stock(order)
links = SemanticLink.where(variant_id: order.line_items.select(:variant_id))
links = SemanticLink.where(subject: order.variants)

perform_later(order) if links.exists?
rescue StandardError => e
Expand Down Expand Up @@ -133,5 +133,7 @@ def place_order(user, order, orderer, backorder)
.perform_later(
user, order.distributor, order.order_cycle, placed_order.semanticId
)

order.exchange.semantic_links.create!(semantic_id: placed_order.semanticId)
end
end
6 changes: 6 additions & 0 deletions app/jobs/complete_backorder_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,18 @@ class CompleteBackorderJob < ApplicationJob
# someone else's order.
def perform(user, distributor, order_cycle, order_id)
order = FdcBackorderer.new(user, nil).find_order(order_id)

return if order&.lines.blank?

urls = FdcUrlBuilder.new(order.lines[0].offer.offeredItem.semanticId)

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

FdcBackorderer.new(user, urls).complete_order(order)

exchange = order_cycle.exchanges.outgoing.find_by(receiver: distributor)
exchange.semantic_links.find_by(semantic_id: order_id)&.destroy!
rescue StandardError
BackorderMailer.backorder_incomplete(user, distributor, order_cycle, order_id).deliver_later

Expand Down
2 changes: 1 addition & 1 deletion app/jobs/stock_sync_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def self.sync_linked_catalogs_now(order)

def self.catalog_ids(order)
stock_controlled_variants = order.variants.reject(&:on_demand)
links = SemanticLink.where(variant_id: stock_controlled_variants.map(&:id))
links = SemanticLink.where(subject: stock_controlled_variants)
semantic_ids = links.pluck(:semantic_id)
semantic_ids.map do |product_id|
FdcUrlBuilder.new(product_id).catalog_url
Expand Down
4 changes: 4 additions & 0 deletions app/models/exchange.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ class Exchange < ApplicationRecord
has_many :exchange_fees, dependent: :destroy
has_many :enterprise_fees, through: :exchange_fees

# Links to open backorders of a distributor (outgoing exchanges only)
# Don't allow removal of distributor from OC while we have an open backorder.
has_many :semantic_links, as: :subject, dependent: :restrict_with_error

validates :sender_id, uniqueness: { scope: [:order_cycle_id, :receiver_id, :incoming] }

before_destroy :delete_related_exchange_variants, prepend: true
Expand Down
4 changes: 3 additions & 1 deletion app/models/semantic_link.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

# Link a Spree::Variant to an external DFC SuppliedProduct.
class SemanticLink < ApplicationRecord
belongs_to :variant, class_name: "Spree::Variant"
self.ignored_columns += [:variant_id]

belongs_to :subject, polymorphic: true

validates :semantic_id, presence: true
end
6 changes: 5 additions & 1 deletion app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,12 @@ def states
class_name: 'Spree::Adjustment',
dependent: :destroy
has_many :invoices, dependent: :restrict_with_exception

belongs_to :order_cycle, optional: true
has_one :exchange, ->(order) {
outgoing.to_enterprise(order.distributor)
}, through: :order_cycle, source: :exchanges
has_many :semantic_links, through: :exchange

belongs_to :distributor, class_name: 'Enterprise', optional: true
belongs_to :customer, optional: true
has_one :proxy_order, dependent: :destroy
Expand Down
2 changes: 1 addition & 1 deletion app/models/spree/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class Variant < ApplicationRecord
has_many :exchanges, through: :exchange_variants
has_many :variant_overrides, dependent: :destroy
has_many :inventory_items, dependent: :destroy
has_many :semantic_links, dependent: :delete_all
has_many :semantic_links, as: :subject, dependent: :delete_all
has_many :supplier_properties, through: :supplier, source: :properties

localize_number :price, :weight
Expand Down
34 changes: 32 additions & 2 deletions app/services/fdc_backorderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def initialize(user, urls)
end

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

def build_new_order(ofn_order)
Expand All @@ -19,7 +19,37 @@ def build_new_order(ofn_order)
end
end

def find_open_order
# Try the new method and fall back to old method.
def find_open_order(ofn_order)
lookup_open_order(ofn_order) || find_last_open_order
end

def lookup_open_order(ofn_order)
# There should be only one link at the moment but we may support
# ordering from multiple suppliers one day.
semantic_ids = ofn_order.semantic_links.pluck(:semantic_id)

semantic_ids.lazy
# Make sure we select an order from the right supplier:
.select { |id| id.starts_with?(urls.orders_url) }
# Fetch the order from the remote DFC server, lazily:
.map { |id| find_order(id) }
.compact
# Just in case someone completed the order without updating our database:
.select { |o| o.orderStatus[:path] == "Held" }
.first
# The DFC Connector doesn't recognise status values properly yet.
# So we are overriding the value with something that can be exported.
&.tap { |o| o.orderStatus = "dfc-v:Held" }
end

# DEPRECATED
#
# We now store links to orders we placed. So we don't need to search
# through all orders and pick a random open one.
# But for compatibility with currently open order cycles that don't have
# a stored link yet, we keep this method as well.
def find_last_open_order
graph = import(urls.orders_url)
open_orders = graph&.select do |o|
o.semanticType == "dfc-b:Order" && o.orderStatus[:path] == "Held"
Expand Down
14 changes: 14 additions & 0 deletions db/migrate/20241030023153_add_subject_to_semantic_links.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# frozen_string_literal: true

# rails g migration AddSubjectToSemanticLinks subject:references{polymorphic}
#
# We want to add links to Exchanges as well as Variants.
# The word subject comes from the triple structure of the Semantic Web:
#
# Subject predicate object (variant has linke URL)
class AddSubjectToSemanticLinks < ActiveRecord::Migration[7.0]
def change
# We allow `null` until we filled the new columns with existing data.
add_reference :semantic_links, :subject, polymorphic: true, null: true
end
end
11 changes: 11 additions & 0 deletions db/migrate/20241030025540_copy_subject_on_semantic_links.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

class CopySubjectOnSemanticLinks < ActiveRecord::Migration[7.0]
def up
execute <<~SQL.squish
UPDATE semantic_links SET
subject_id = variant_id,
subject_type = 'Spree::Variant'
SQL
end
end
8 changes: 8 additions & 0 deletions db/migrate/20241030033956_change_null_on_semantic_links.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class ChangeNullOnSemanticLinks < ActiveRecord::Migration[7.0]
def change
change_column_null :semantic_links, :subject_id, false
change_column_null :semantic_links, :subject_type, false

change_column_null :semantic_links, :variant_id, true
end
end
7 changes: 5 additions & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2024_10_23_054951) do
ActiveRecord::Schema[7.0].define(version: 2024_10_30_033956) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_stat_statements"
enable_extension "plpgsql"
Expand Down Expand Up @@ -401,10 +401,13 @@
end

create_table "semantic_links", force: :cascade do |t|
t.bigint "variant_id", null: false
t.bigint "variant_id"
t.string "semantic_id", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "subject_type", null: false
t.bigint "subject_id", null: false
t.index ["subject_type", "subject_id"], name: "index_semantic_links_on_subject"
t.index ["variant_id"], name: "index_semantic_links_on_variant_id"
end

Expand Down

Large diffs are not rendered by default.

9 changes: 8 additions & 1 deletion spec/jobs/backorder_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,19 @@

describe "#place_order" do
it "schedules backorder completion for specific enterprises" do
order.order_cycle = build(
order.order_cycle = create(
:simple_order_cycle,
id: 1,
orders_close_at: Date.tomorrow.noon,
)
completion_time = Date.tomorrow.noon + 4.hours

exchange = order.order_cycle.exchanges.create!(
incoming: false,
sender: order.order_cycle.coordinator,
receiver: order.distributor,
)

urls = FdcUrlBuilder.new(product_link)
orderer = FdcBackorderer.new(user, urls)
backorder = orderer.build_new_order(order)
Expand All @@ -132,6 +138,7 @@
expect {
subject.place_order(user, order, orderer, backorder)
}.to enqueue_job(CompleteBackorderJob).at(completion_time)
.and change { exchange.semantic_links.count }.by(1)
end
end
end
24 changes: 23 additions & 1 deletion spec/jobs/complete_backorder_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,14 @@
chia_line = orderer.find_or_build_order_line(backorder, chia_offer)
chia_line.quantity = 5

orderer.send_order(backorder)
orderer.send_order(backorder).tap do |o|
exchange.semantic_links.create!(semantic_id: o.semanticId)
end
}
let(:ofn_order) { create(:completed_order_with_totals) }
let(:distributor) { ofn_order.distributor }
let(:order_cycle) { ofn_order.order_cycle }
let(:exchange) { order_cycle.exchanges.outgoing.first }
let(:beans) { ofn_order.line_items[0].variant }
let(:chia) { chia_item.variant }
let(:chia_item) { ofn_order.line_items[1] }
Expand Down Expand Up @@ -77,6 +80,9 @@
.and change {
current_order.lines[1].quantity.to_i
}.from(5).to(7)
.and change {
exchange.semantic_links.count
}.by(-1)
end

it "removes line items", vcr: true do
Expand Down Expand Up @@ -109,5 +115,21 @@
}.to enqueue_mail(BackorderMailer, :backorder_incomplete)
.and raise_error VCR::Errors::UnhandledHTTPRequestError
end

it "skips empty backorders" do
user = nil
distributor = nil
order_cycle = nil
order_id = nil
backorder = DataFoodConsortium::Connector::Order.new(
order_id, orderStatus: "dfc-v:Held"
)
expect_any_instance_of(FdcBackorderer)
.to receive(:find_order).and_return(backorder)

expect {
subject.perform(user, distributor, order_cycle, order_id)
}.not_to raise_error
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

require 'spec_helper'
require_relative '../../db/migrate/20241030025540_copy_subject_on_semantic_links'

RSpec.describe CopySubjectOnSemanticLinks do
describe "#up" do
let(:original_variant) { create(:variant) }
let(:dummy_variant) { create(:variant) }

it "copies the original data" do
link = SemanticLink.create!(
subject: dummy_variant, # This would be NULL when migration runs.
semantic_id: "some-url",
)
SemanticLink.update_all("variant_id = #{original_variant.id}")

expect { subject.up }.to change {
link.reload.subject
}.from(dummy_variant).to(original_variant)
end
end
end
2 changes: 2 additions & 0 deletions spec/models/exchange_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
require 'spec_helper'

RSpec.describe Exchange do
it { is_expected.to have_many :semantic_links }

it "should be valid when built from factory" do
expect(build(:exchange)).to be_valid
end
Expand Down
2 changes: 1 addition & 1 deletion spec/models/semantic_link_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
require 'spec_helper'

RSpec.describe SemanticLink, type: :model do
it { is_expected.to belong_to :variant }
it { is_expected.to belong_to :subject }
it { is_expected.to validate_presence_of(:semantic_id) }
end
3 changes: 3 additions & 0 deletions spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
let(:user) { build(:user, email: "[email protected]") }
let(:order) { build(:order, user:) }

it { is_expected.to have_one :exchange }
it { is_expected.to have_many :semantic_links }

describe "#errors" do
it "provides friendly error messages" do
order.ship_address = Spree::Address.new
Expand Down
13 changes: 11 additions & 2 deletions spec/services/fdc_backorderer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
# After closing the order at the end, the test can be repeated live again.

# Build a new order when no open one is found:
order.order_cycle = build(:order_cycle)
order.order_cycle = create(:order_cycle, distributors: [order.distributor])
backorder = subject.find_or_build_order(order)
expect(backorder.semanticId).to eq urls.orders_url
expect(backorder.lines).to eq []
Expand All @@ -50,10 +50,19 @@
expect(found_backorder.lines.count).to eq 1
expect(found_backorder.lines[0].quantity.to_i).to eq 3

# Without a stored semantic link, it can't look it up directly though:
found_backorder = subject.lookup_open_order(order)
expect(found_backorder).to eq nil

# But with a semantic link, it works:
order.exchange.semantic_links.create!(semantic_id: placed_order.semanticId)
found_backorder = subject.lookup_open_order(order)
expect(found_backorder.semanticId).to eq placed_order.semanticId

# And close the order again:
subject.complete_order(placed_order)
remaining_open_order = subject.find_or_build_order(order)
expect(remaining_open_order.semanticId).not_to eq placed_order.semanticId
expect(remaining_open_order.semanticId).to eq urls.orders_url
end

describe "#find_or_build_order" do
Expand Down

0 comments on commit 3ec8cd2

Please sign in to comment.