New fulfillment type shipping #6614

sujithvn · 2022-11-03T12:23:57Z

Signed-off-by: Sujith mail.sujithvn@gmail.com

Resolves #6471
Impact: breaking
Type: feature

This PR replaces #6579

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.

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'.

This PR is for 2nd entry new-fulfillment-type-shipping

PR Details in order

feat/fulfillment-types
 | 
 | __ 00 fulfillment base #6610 
 |        | __ new-fulfillment-type-shipping       <--- This PR #6614
 |        | __ new-fulfillment-type-pickup.    #6613 
 |__ new-fulfillment-impacted-plugins. [carts, products, orders, catalogs]  #6615 
 |__ new-placeOrder refactor validateOrder #6616

Existing PRs originally 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 #6633 , setDefaultFFtype #6543) to point to 00 fulfillment base

API-PLUGIN-FULFILLMENT-TYPE-SHIPPING

Registers the 'shipping' as a ff-type via registeredFulfillmentTypes: ["shipping"]
Inserts the default entry for shipping ff-type in Fulfillment collection

API-PLUGIN-FULFILLMENT-METHOD-SHIPPING-FLAT-RATE

Mostly copied over from api-plugin-shipment-flat-rate by making certain fixes to adhere to new structure and new DB collections. Fixes are similar to the details provided for the other 2 ff-method plugins below.
The mutations & queries present were left as it is and it works slightly differently compared to the new mutations/queries that were introduced in api-plugin-fulfillment (which is used by remaining ff-methods)
Shipping restrictions feature is copied over from existing code. This feature is not added to other ff-methods.

API-PLUGIN-FULFILLMENT-METHOD-SHIPPING-DYNAMIC-RATE

getFulfillmentMethodsWithQuotesShippingDynamicRate - returns the quote or equivalent details for the method when called from base ff plugin
preStartup - extends the union of "methodAdditionalData" with data structure specific to UPS
startup - Inserts the required ff-method entry into Fulfillment collection
util/checkAndCreateFulfillmentMethod - confirms existing ff-type entry and adds a new ff-method under it.
util/calculateUPSRate - dummy function to simulate api providing UPS specific info while returning quotes.
util/ validateOrderMethodsDynamicRate - dummy function to simulate UPS specific validations done. Called by prepareOrder.js

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.

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

changeset-bot · 2022-11-03T12:24:02Z

⚠️ No Changeset found

Latest commit: fe15a55

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

tedraykov · 2022-11-10T07:40:17Z

packages/api-plugin-fulfillment-type-shipping/src/startup.js

+    // We do not have validatePermissions in context during this startup stage, hence commenting below
+    // await context.validatePermissions("reaction:legacy:fulfillmentTypes", "read", { shopId });
+
+    const insertSuccess = await checkAndCreateFulfillmentType(context, shopId);


When we create an entity, isn't it implied that we check the prerequisite conditions anyway? The checkAnd part of the function name seems a bit overexplicit.

Inside the checkAndCreateFulfillmentType we have the call to the original createFulfillmentType mutation.

Although the mutation call is explicitly making the call to the one defined in context, I thought I could make this a bit explicit to avoid any quick confusion.

tedraykov · 2022-11-10T13:29:40Z

...lment-method-shipping-dynamic-rate/src/getFulfillmentMethodsWithQuotesShippingDynamicRate.js

+
+  shippingRateDocs.map(async (doc) => {
+    const carrier = doc.provider.label;
+    const currentPluginMethods = doc.methods.filter((method) => ((method.fulfillmentMethod === (fulfillmentMethodName)) && (method.enabled)));


Why is the reasoning behind persisting the fulfillment method for a dynamic method? If the rates for are method are dynamically calculated on every fulfillmentMethodQuotes by calling an external API I don't see much point in having a method with predefined rates here. It can be dynamically constructed on every get quotes request. The only reasoning I can think of is i we allow the method to be enabled or not. But in that case we should check that before pushing the newly calculated rate. The same goes of all other method plugins.

Not sure if I completely understood your point. The idea is that we are showing a placeholder function here which could be replaced by an actual API invoking function to collect the dynamic rate. The placeholder function is a dummy and currently returning some hard-coded values. It is expected to collect the actual dynamic quote/rate each time. Now the call to fulfillmentMethodQuotes will be placed to all ff-methods defined under the specific fulfillment-type (like all shipping or all pickup) and this is collected and presented to the user to select one of them as the preferred method via the selectFulfillmentOptionForGroup call.

We can have a quick call if I got your question wrong.

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

...fillment-method-shipping-dynamic-rate/src/fulfillmentMethodsWithQuotesShippingDynamicRate.js

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

aldeed

Similar comments as in the pickup PR and my comments about "type" vs. "method" vs. "provider" in another PR.

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

…ction into 00-fulfillment-base

conflict resolved: pnpm-lock Signed-off-by: Sujith <mail.sujithvn@gmail.com>

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

sujithvn added 9 commits November 3, 2022 17:31

feat: fftype-shipping with methods flatrate ups

e0c056e

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: updated test cases

d5252f5

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: filename fixed

3156bae

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: review comment fixes

0c30b61

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: picked up the fix #6578 from old plugin

2d9044d

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: renamed collection in test.js

37e3dd6

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: import fix

70bb0b9

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: renamed top-level folder ups to dynamic-rate

975ea93

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: pnpm-lock without snyk

2d543a3

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

sujithvn marked this pull request as ready for review November 3, 2022 12:33

sujithvn requested review from aldeed, tedraykov, vanpho93 and brent-hoover and removed request for brent-hoover November 3, 2022 12:33

sujithvn mentioned this pull request Nov 3, 2022

01 01 fulfillment type shipping #6579

Closed

tedraykov reviewed Nov 10, 2022

View reviewed changes

sujithvn added 3 commits November 15, 2022 16:49

fix: review comment fix 3

da70978

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: fix validatePermissions

7dfa566

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: review comment fixes

ddca358

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

vanpho93 reviewed Nov 20, 2022

View reviewed changes

...fillment-method-shipping-dynamic-rate/src/fulfillmentMethodsWithQuotesShippingDynamicRate.js Outdated Show resolved Hide resolved

fix: review comment fixes

3862d7b

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

aldeed reviewed Dec 8, 2022

View reviewed changes

sujithvn added 4 commits March 10, 2023 20:26

Merge 00-fulfillment-base into new-fulfillment-type-shipping

7ed401f

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

Merge branch '00-fulfillment-base' of github.com:reactioncommerce/rea…

c7a719d

…ction into 00-fulfillment-base

Merge branch '00-fulfillment-base' into new-fulfillment-type-shipping

f790476

conflict resolved: pnpm-lock Signed-off-by: Sujith <mail.sujithvn@gmail.com>

fix: fixed test errors

fe15a55

Signed-off-by: Sujith <mail.sujithvn@gmail.com>

sujithvn merged commit 97946d6 into 00-fulfillment-base May 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

New fulfillment type shipping #6614

New fulfillment type shipping #6614

Uh oh!

sujithvn commented Nov 3, 2022 •

edited

Loading

Uh oh!

changeset-bot bot commented Nov 3, 2022 •

edited

Loading

Uh oh!

tedraykov Nov 10, 2022 •

edited

Loading

Uh oh!

sujithvn Nov 21, 2022

Uh oh!

tedraykov Nov 10, 2022

Uh oh!

sujithvn Nov 21, 2022

Uh oh!

Uh oh!

aldeed left a comment

Uh oh!

Uh oh!

New fulfillment type shipping #6614

New fulfillment type shipping #6614

Uh oh!

Conversation

sujithvn commented Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR replaces #6579

Issue

Solution

PR Details in order

Existing PRs originally based on the #6480 (to be updated to point to #6610)

API-PLUGIN-FULFILLMENT-TYPE-SHIPPING

API-PLUGIN-FULFILLMENT-METHOD-SHIPPING-FLAT-RATE

API-PLUGIN-FULFILLMENT-METHOD-SHIPPING-DYNAMIC-RATE

Breaking changes

Testing

Uh oh!

changeset-bot bot commented Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

tedraykov Nov 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sujithvn Nov 21, 2022

Choose a reason for hiding this comment

Uh oh!

tedraykov Nov 10, 2022

Choose a reason for hiding this comment

Uh oh!

sujithvn Nov 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aldeed left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sujithvn commented Nov 3, 2022 •

edited

Loading

changeset-bot bot commented Nov 3, 2022 •

edited

Loading

tedraykov Nov 10, 2022 •

edited

Loading