-
Notifications
You must be signed in to change notification settings - Fork 45
Create Sales Order Documentation #138
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
base: main
Are you sure you want to change the base?
Conversation
* Input Parameters | ||
* orderJson (Map) | ||
2. Initialize orderContext | ||
3. Set following to orderContext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed point 3, since this can be taken care of in mapping service, the input map to create#SalesOrder should already have these values set
2. Validate order by checking OrderHeader.externalId to avoid duplication of order | ||
3. Initialize payload.roles list | ||
* Call findOrCreate#Person for payload.customer | ||
* Add [partyId:findOrCreatePersonOutput.partyId, roleTypeId:"SHIP_TO_CUSTOMER"] to payload.roles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have observed the records for order roles to be created for all below roleTypeIds, do we need to create orderRoles for all roleTypeIds ?
- Customer - BILL_TO_CUSTOMER, END_USER_CUSTOMER, PLACING_CUSTOMER, SHIP_TO_CUSTOMER
- vendor - SHIP_FROM_VENDOR, BILL_FROM_VENDOR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point.
We don't need all these roles.
Important question is:
What roles do we need?
I think, SHIP_TO_CUSTOMER is all we need.
"attrDescription": "" | ||
} | ||
], | ||
"email": "[email protected]", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added field to store for OrderContactMech with contactMechPurposeTypeId:"ORDER_EMAIL"
"partyId": "Doe", | ||
"externalId": 22889483207024, | ||
"first_name": "John", | ||
"last_name": "Doe" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added customer map for basic info, if partyId does not exist, then we findOrCreate#Person
"longitude": -90.07084449999999, | ||
"name": "John Doe", | ||
"country_code": "US", | ||
"province_code": "LA" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added billing_address map for OrderContactMechs BILLING_LOCATION, PHONE_BILLING and BILLING_EMAIL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this?
I am not saying no we don't.
But,
We know we are not billing from OMS. What is the purpose of tracking this data.
"longitude": -90.07084449999999, | ||
"name": "John Doe", | ||
"country_code": "US", | ||
"province_code": "LA" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added billing_address map for OrderContactMechs SHIPPING_LOCATION, PHONE_SHIPPING and SHIPPING_EMAIL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have shipping location data on Ship group. Do we need it at the order level ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this should go in shipGroup level. I will update this.
{ | ||
"externalId": "100097001712202", | ||
"productId": "10022", | ||
"sku": "BLACK_BELL_BOTTOM_S", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we create product if productId missing in the json in order creation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same,
Application layer will do whatever it needs to ensure system runs predictably.
We know Shopify captured the order.
So the Product on OrderItem has to exist.
We should do whatever it takes to ensure order is recorded,
Then also prepare context information and log so support system can do its job.
], | ||
"shipGroups": [ | ||
{ | ||
"facilityId": "NEW_ERA_HARAJUKU", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we always expect facilityId i.e. OMS internal Id for the location for order creation?
As a generic API design rule, we can add facilityId and/or facilityExternalId, but if missing, do we return error and not create order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the createOrder service in OMS will expect fully resolved data.
the createSalesOrder service is Shopify Integration application service. It has to take responsibility to resolve the data to ensure predictable results including graceful degradation.
@@ -1,47 +1,210 @@ | |||
### co.hotwax.orderledger.order.OrderServices.create#SalesOrder (Application Layer - OMS) | |||
This service will take in the order JSON in OMSNewOrdersFeed and set up a complete order by performing any surrounding crud operations as needed. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this service is application level service.
What if we define this service for now, in Shopify connector component.
for this conversation, Let's call it:
shopifyconnector.createSalesOrder
This service will have all the freedom to do whatever it takes to create JSON for createOrder service is OMS.
Later,
We can write worker class to do tasks that we may find reusable for other similar application layer service.
@gurveenbagga
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do this for now, but yes later we may need to re-think it.
We have another use case to create exchange order wrt Return Exchange v2 work, where we will need oms layer application service to create order. OR NOW that I think of it, the service which will process shopify exchange order json will transform it and prepare the required JSON again for the create Order in OMS.
* If orderJson.billToPhone | ||
* Call create#ContactMech for [contactMechTypeId:"TELECOM_NUMBER", infoString:orderJson.billToPhone] | ||
* Add [contatctMechId:createContactMechOutput.contactMechId, contactMechPurposeTypeId:"PHONE_BILLING"] to orderContext.contactMechs | ||
* Add [partyId:ProductStore.payToPartyId, roleTypeId:"SHIP_FROM_VENDOR"] to payload.roles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have customer Party in our database, we may already have all these details.
lookup can be super expensive.
Is it better to just create on every next order by same person?
Or is there efficient algorithm to lookup / compare?
What if we create hash of the elements in contact mach and then compare the hash. We did similar thing in Product sync?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or may be Shopify has some identifier like we have "contactMechId"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are creating contact mechs for the order and not for customer. Do you mean we dont need to create the same address eg, shipping address in every order, and check if any way to re-use the same address id and just associate the order and the contact mech id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but what if the customer updates his/her address? We are not syncing customer and address separately, I think its better to create the contact mechs (address, phone, email) with every order.
4. Initialize orderContext.roles list | ||
* Call findOrCreate#Person for orderJson.customer | ||
* Add [partyId:findOrCreatePartyOutput.partyId, roleTypeId:"SHIP_TO_CUSTOMER"] to orderContext.roles | ||
* payload (Map) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we are using key word payload to represent the json, The actual JSON will be native to how Moqui service and API handles hierarchical parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this payload will be the order json which will be based on the spec of master of order JSON required by core create#org.apache.ofbiz.order.order.OrderHeader.
"priority": "2", | ||
"productStoreId": "STORE", | ||
"salesChannelEnumId": "UNKNWN_SALES_CHANNEL", | ||
"webSiteId": "WEBSTORE", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use of attribute entity is good when there is no other option.
Today, we are reengineering our system.
We should spend sometime to understand,
Taxation on Sales order in Apache OFBiz.
better understanding will help consider changes in how we track tax authority details.
No description provided.