-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New place order refactor validate order #6616
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
New place order refactor validate order #6616
Conversation
Signed-off-by: Sujith <mail.sujithvn@gmail.com>
Signed-off-by: Sujith <mail.sujithvn@gmail.com>
Signed-off-by: Sujith <mail.sujithvn@gmail.com>
Signed-off-by: Sujith <mail.sujithvn@gmail.com>
Signed-off-by: Sujith <mail.sujithvn@gmail.com>
|
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.
Aside from the few inline suggestions, prepareOrder
is a good example of a fn where returning errors rather than throwing them would be a lot cleaner. You wouldn't need to try/catch and formatErrors everywhere. I'm not sure whether you want to tackle that level of refactoring across all those functions now, but it would be good to do eventually.
} | ||
|
||
group.shipmentMethod = { | ||
const output = { |
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 this will no longer be mutating the group object, maybe it should be in a new function with a different name, such as getShipmentMethodForGroup
?
packages/api-plugin-orders/src/util/orderValidators/createPayments.js
Outdated
Show resolved
Hide resolved
packages/api-plugin-orders/src/util/orderValidators/getDiscounts.js
Outdated
Show resolved
Hide resolved
packages/api-plugin-orders/src/util/orderValidators/validateInitialOrderData.js
Outdated
Show resolved
Hide resolved
packages/api-plugin-orders/src/util/orderValidators/prepareOrder.js
Outdated
Show resolved
Hide resolved
packages/api-plugin-orders/src/util/orderValidators/getCustomFields.test.js
Outdated
Show resolved
Hide resolved
packages/api-plugin-orders/src/util/orderValidators/getFinalFulfillmentGroups.js
Outdated
Show resolved
Hide resolved
packages/api-plugin-orders/src/util/orderValidators/getFinalFulfillmentGroups.js
Outdated
Show resolved
Hide resolved
* @param {String} flag - flag which define if the call is from placeOrder or validateOrder | ||
* @returns {Promise<Object>} output - order, token and validation results | ||
*/ | ||
export default async function prepareOrder(context, input, flag) { |
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.
This function is quite long with many duplications. This should be cleaned up.
Signed-off-by: Sujith <mail.sujithvn@gmail.com>
…ateOrder Signed-off-by: Sujith <mail.sujithvn@gmail.com>
…ctor-validateOrder Signed-off-by: Sujith <mail.sujithvn@gmail.com>
…or-validateOrder Signed-off-by: Sujith <mail.sujithvn@gmail.com>
Signed-off-by: Sujith <mail.sujithvn@gmail.com>
…efactor-validateOrder Signed-off-by: Sujith <mail.sujithvn@gmail.com>
Signed-off-by: Sujith <mail.sujithvn@gmail.com>
Signed-off-by: Sujith <mail.sujithvn@gmail.com>
…ction into 00-fulfillment-base Signed-off-by: Sujith <mail.sujithvn@gmail.com>
…ateOrder Signed-off-by: Sujith <mail.sujithvn@gmail.com>
Signed-off-by: Sujith mail.sujithvn@gmail.com
Resolves #6471
Impact: breaking
Type: feature
This PR replaces #6583
Issue
In the current system, the only way for order fulfillment is via shipping which is hardcoded into the system. We need to introduce the flexibility for users to add their own fulfillment types via plugins.
There is no way to verify if the order data in the UI is ready/complete and free of errors.
Solution
We are creating a base fulfillment plugin which would enable other fulfillment types (like shipping, pickup, digital) to be introduced via plugins. Each of the newly introduced fulfillment type plugin would need to have the specific fulfillment methods also to be added as separate plugins. Example, fulfillment type 'pickup' could have fulfillment methods like 'store pickup' and 'curb-side pickup'.
Along with the changes, we are creating a new API query end-point validateOrder which provides the option for frontend to validate the correctness and completeness of the order before actually placing the order.
Also, we are refactoring the placeOrder mutation and cutting down the functional layers
This PR is 5th entry (new-placeOrder refactor validateOrder)
PR Details in order
Existing PRs based on the #6480 (to be updated to point to #6610)
We shall change the base branch of the 3 existing PRs (i18n #6545, dataMigration #6544, setDefaultFFtype #6543) to point to 00 fulfillment base
Details (specific to placeOrder refactor and introduction of validateOrder end-point)
Breaking changes
Since this is the new way of handling fulfillment, the existing shipment plugins would not work along with this. User would need to migrate the data (migration script will be developed) and remove the shipment plugins (api-plugin-shipments and api-plugin-shipments-flat-rate).
Testing
All this is new code and at this stage this is not integrated to reaction, meaning the plugins.json is not updated to include this plugin. Hence proper testing can be carried out only after merging in the remaining PRs.