[DFC Orders II] Create orders endpoint #13208

dacook · 2025-03-12T03:34:01Z

What? Why?

Part 1 of [DFC Orders II] Add endpoint to create Order, update and delete #12181

What should we test?

Visit ... page.

Release notes

Changelog Category (reviewers may add a label for the release notes):

User facing changes
API changes (V0, V1, DFC or Webhook)
Technical changes only
Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

dacook · 2025-03-12T03:36:08Z

Hi @mkllnk , can you please take a look and let me know if this is what you had in mind?
The integration spec could probably be in a separate file

engines/dfc_provider/spec/requests/orders_spec.rb

mkllnk · 2025-03-12T03:42:12Z

engines/dfc_provider/app/controllers/dfc_provider/orders_controller.rb

+      if current_enterprise.distributed_orders.create user: current_user
+        head :created


Later we'll need to return the order to provide the client with an id.

engines/dfc_provider/spec/requests/orders_spec.rb

mkllnk · 2025-03-12T03:46:24Z

engines/dfc_provider/spec/requests/orders_spec.rb

+        context "with given enterprise id" do
+          let(:enterprise_id) { 10_000 }
+
+          run_test! {
+            expect(enterprise.distributed_orders.count).to eq 1
+          }
+        end


You may already know this, but Swagger documents only the last example in a group. It overrides the data of the previous ones.

I didn't know!

engines/dfc_provider/spec/requests/orders_spec.rb

engines/dfc_provider/spec/requests/orders_backorder_spec.rb

But there's not much in the object, we need to check what's required.

But I need to know what details are going to be included. probably sales session. and line items of course.

dacook · 2025-03-20T00:17:40Z

@mkllnk I've achieved end to end, with a few little fixes along the way. Can you please review and confirm if ok to proceed this way?

Probably should have just hardcoded it. Hopefully we can remove this soon anyway.

dacook · 2025-03-20T00:36:06Z

(just pushed up some lint fixes and fixup commits)

mkllnk · 2025-03-20T02:18:19Z

app/services/fdc_url_builder.rb

+  # Known product link formats:
+  # * https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts/44519466467635
+  # * http://test.host/api/dfc/enterprises/10000/supplied_products/10001 (OFN)


Nice. I guess now it's a DfcUrlBuilder instead of an FdcUrlBuilder.

mkllnk · 2025-03-20T02:19:47Z

engines/dfc_provider/spec/services/offer_builder_spec.rb


+    it "assigns a price without unknown currency" do


without known

with unknown

mkllnk · 2025-03-20T02:21:46Z

app/services/fdc_url_builder.rb

-    @catalog_url, _slash, _id = semantic_id.rpartition("/")
-    @orders_url = @catalog_url.sub("/SuppliedProducts", "/Orders")
+    base_url, _slash, _id = semantic_id.rpartition("/")
+    @catalog_url = base_url.sub("/supplied_products", "/catalog_items")


The DFC standard doesn't define how much of the graph to include in an endpoint. It also doesn't define listing endpoints like these catalog URLs (yet). That's why OFN and FDC are different. Neither is wrong.

mkllnk · 2025-03-20T02:26:43Z

engines/dfc_provider/app/services/dfc_builder.rb

+  def self.catalog_item(variant, include_product: true)
    id = urls.enterprise_catalog_item_url(
      enterprise_id: variant.supplier_id,
      id: variant.id,
    )
-    product = SuppliedProductBuilder.supplied_product(variant)
+    product = SuppliedProductBuilder.supplied_product(variant) if include_product


😬 Maybe better to just remove the product and the caller of catalog_item can decide to add it separately?

Hm, but I guess that we now build the catalog item twice, once for the enterprise and once for the product. It would be good to avoid that.

mkllnk · 2025-03-20T02:28:05Z

engines/dfc_provider/app/controllers/dfc_provider/orders_controller.rb

+
+      order = current_enterprise.distributed_orders.build(user: current_user)
+
+      # rubocop:disable Style/GuardClause


Isn't it as easy to return an error instead of disabling Rubocop here?

mkllnk · 2025-03-20T02:30:00Z

engines/dfc_provider/app/services/offer_builder.rb

+  def self.price_measure(variant)
+    measures = DfcLoader.vocabulary("measures")
+
+    case variant.currency
+    when "AUD"
+      measures.AUSTRALIANDOLLAR
+    when "CAD"
+      measures.CANADIANDOLLAR
+    when "EUR"


Good workaround. I tried getting the QUDT vocabulary to work but it's defined in a completely different way and the DFC Connector parser doesn't understand it. So let's wait until it's actually a standard.

I'm probably not understanding the full context, but it seems weird that the initial FDC pilot works by loading the catalog from the supplied_products URL. Alternatively, maybe we coudl load the catalog from the OFN supplied_products index endpoint. But it dosn't exist yet.

It's required by the FdcOfferBroker.

Simpler this way.

ned to rethink where. is it a OrderLineItem builder? or soemthing else.

dacook self-assigned this Mar 12, 2025

mkllnk reviewed Mar 12, 2025

View reviewed changes

dacook force-pushed the dfc-orders-endpoint-12181 branch from 386011d to 23e0706 Compare March 12, 2025 06:14

mkllnk reviewed Mar 13, 2025

View reviewed changes

engines/dfc_provider/spec/requests/orders_backorder_spec.rb Outdated Show resolved Hide resolved

mkllnk reviewed Mar 13, 2025

View reviewed changes

engines/dfc_provider/spec/requests/orders_backorder_spec.rb Outdated Show resolved Hide resolved

dacook force-pushed the dfc-orders-endpoint-12181 branch 13 times, most recently from c32d817 to 43e39ff Compare March 19, 2025 06:13

dacook added 3 commits March 20, 2025 10:07

Add dfc_provider/orders_controller to create order

f843466

wip: Return the order as a DFC JSON object

cef8e9e

But there's not much in the object, we need to check what's required.

wip: Submit order object to create

531d31d

But I need to know what details are going to be included. probably sales session. and line items of course.

dacook force-pushed the dfc-orders-endpoint-12181 branch from 43e39ff to f8afb02 Compare March 19, 2025 23:26

dacook requested a review from mkllnk March 20, 2025 00:15

dacook added 5 commits March 20, 2025 11:33

Integration test

773aa68

ensure default_url_options[:host] is set correct for DfcProvider::Engine

9d1368b

fixup: default_url_option

603af44

Ah ha, it's waiting for a lock here, possibly at find_ofn_user

d054d68

Add support for OFN-style DFC URIs

df08c1a

Map currency to DFC codes

30decc4

Probably should have just hardcoded it. Hopefully we can remove this soon anyway.

dacook force-pushed the dfc-orders-endpoint-12181 branch from f8afb02 to d58d4e8 Compare March 20, 2025 00:35

mkllnk reviewed Mar 20, 2025

View reviewed changes

dacook added 9 commits March 20, 2025 16:07

fixup: Map currency to DFC codes

973d222

Include CatalogItem with SuppliedProduct

3bc7569

It's required by the FdcOfferBroker.

fixup: integariton spe

fd1a0b7

fix: Handle proper DFC graph format

1071161

fixup: cuprite setup

0dd9e6f

Simpler this way.

fix: return the order as a DFC Json object

9353b1d

Apply order state if recognised

4e5534b

wip: create line items

59b3d3a

ned to rethink where. is it a OrderLineItem builder? or soemthing else.

dacook force-pushed the dfc-orders-endpoint-12181 branch from d58d4e8 to 59b3d3a Compare March 20, 2025 06:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DFC Orders II] Create orders endpoint #13208

[DFC Orders II] Create orders endpoint #13208

dacook commented Mar 12, 2025

dacook commented Mar 12, 2025

mkllnk Mar 12, 2025

mkllnk Mar 12, 2025

dacook Mar 12, 2025

dacook commented Mar 20, 2025

dacook commented Mar 20, 2025

mkllnk Mar 20, 2025

mkllnk Mar 20, 2025

mkllnk Mar 20, 2025

mkllnk Mar 20, 2025

mkllnk Mar 20, 2025

mkllnk Mar 20, 2025

		if current_enterprise.distributed_orders.create user: current_user
		head :created


		order = current_enterprise.distributed_orders.build(user: current_user)

		# rubocop:disable Style/GuardClause

[DFC Orders II] Create orders endpoint #13208

Are you sure you want to change the base?

[DFC Orders II] Create orders endpoint #13208

Conversation

dacook commented Mar 12, 2025

What? Why?

What should we test?

Release notes

Dependencies

Documentation updates

dacook commented Mar 12, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dacook commented Mar 20, 2025

dacook commented Mar 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment