Skip to content

Add carriers and service levels to shipping methods.#730

Merged
jhawthorn merged 3 commits intosolidusio:masterfrom
Senjai:carriers
Jan 28, 2016
Merged

Add carriers and service levels to shipping methods.#730
jhawthorn merged 3 commits intosolidusio:masterfrom
Senjai:carriers

Conversation

@Senjai
Copy link
Copy Markdown
Contributor

@Senjai Senjai commented Jan 22, 2016

When we're dealing with a shipping method, we have no formal way of
adding additional information that would be useful for fulfilment
purposes.

We can add an "internal" name to the shipping method, but that would
require pattern matching to get any useful information from. Often, in
most fulfilment scenarios, we have a desire to know exactly what
carrier and service level a specific shipping method is representative
of.

Given a shipment I can then infer that it should be shipped with UPS one
day shipping by introspecting on the methods' carrier and service_level
columns as necessary.

This is optional as it stands, but validations can be added in stores as
required for complex fulfilment scenarios.

cc @forkata, @cbrunsdon, @athal7

Interface changes

When we're dealing with a shipping method, we have no formal way of
adding additional information that would be useful for fulfillment
purposes.

We can add an "internal" name to the shipping method, but that would
require pattern matching to get any useful information from. Often, in
most fulfillment scenarios, we have a desire to know _exactly_ what
carrier and service level a specific shipping method is representative
of.

Given a shipment I can then infer that it should be shipped with UPS one
day shipping by introspecting on the methods' carrier and service_level
columns as necessary.

This is optional as it stands, but validations can be added in stores as
required for complex fulfillment scenarios.
@qr8r
Copy link
Copy Markdown
Contributor

qr8r commented Jan 22, 2016

I am Pro this! In working on a Solidus Easypost gem I came accross the requirement for carton shipping rates to have shipping methods. This meant creating shipping methods for all the returned easypost rates . There was no clear way in the existing architecture to indicate which provider and which service a Shipping Method actually represents, So i ended up shoving that info into the Internal name and hoping it was never touched by an admin.

Not all providers include their company name in the shipping method Code. UPS usually does (ie: UPSGround), but USPS for instance does not (ie First).

This change would make it much easier to identify which 'real world' provider/service the ShippingMethod was modeling

@forkata
Copy link
Copy Markdown
Contributor

forkata commented Jan 22, 2016

👍 I think this will be very useful when mapping rates from shipping providers to shipping methods. Also good call on making these fields optional, often times one shipping method can be used for multiple service types from a given carrier and the logic for picking the service type lives in the calculator.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the translation here and let rails do it's work.

I know, that the other form fields still has the unnecessary translations in their labels and this is out of scope here, but we should not introduce even more too generic and context-less translation keys.

@tvdeyen
Copy link
Copy Markdown
Member

tvdeyen commented Jan 22, 2016

Otherwise I am 👍 on this, as this is often requested and useful information! Thanks

Allows users to set service levels and carriers for shipping methods on
the backend.
@Senjai
Copy link
Copy Markdown
Contributor Author

Senjai commented Jan 22, 2016

@tvdeyen Bumped, totally makes sense. Went with the original for consistency.

@athal7
Copy link
Copy Markdown

athal7 commented Jan 25, 2016

@Senjai this looks good and gets the job done in a simple manner, so 👍

A few thoughts potentially worth considering:

  1. In addition to other potential benefits, tracking urls are usually carrier-based, so it could make sense to break out carrier to its own table/model for that purpose as opposed to duplicating tracking url across shipping methods for a carrier
  2. Were there other uses for internal name in the community aside from an unstructured representation of what we have just broken out? If not, maybe we could deprecate that field to reduce confusion.

@deodad
Copy link
Copy Markdown
Contributor

deodad commented Jan 26, 2016

@athal7 in reference to 2, we have multiple shipping methods with the same name so that we consistently offer the customer the same options, but use the internal name to further describe what categories and zones that particular method is servicing. Ex:

  • Name: Standard, Internal: Standard (accessories, continental)
  • Name: Standard, Internal: Standard (accessories, non-continental)
  • Name: Standard, Internal: Standard (alcohol, international)

That being said, if the zones and the categories were both displayed on the index page, it wouldn't be necessary to have this internal name to distinguish between the different methods.

@Senjai
Copy link
Copy Markdown
Contributor Author

Senjai commented Jan 26, 2016

Internal name here, (admin_name on the model) is meant to be used only for display on the backend, but others might actually tie logic to it. I have no problem with keeping an internal name, as it may often be drastically different than the name shown to customers.

E.g. Free Super Saver Shipping! (known on the backend as UPS Ground). The backend name is usually easier to deal with for administrators, and the backend method used for Free Super Saver Shipping might change, unbeknownst to the user at a future date.

I wouldn't vote for extracting carrier out into another model, though I had thought of it. There isn't any logic tied to it. Tracking URLs are associated to carriers for sure, but if we have to make a separate shipping method for each carrier / service level pair, we might as well keep the tracking logic on the shipping method.

Typically stores have less than 10 methods and I'm not sure extracting this would remove more complexity than it would add. There are stores that have many more shipping methods (> 100) with a much more complex setup, but at that point it resembles very little of the traditional shipping method setup we know in Solidus.

@athal7
Copy link
Copy Markdown

athal7 commented Jan 27, 2016

👍

@tvdeyen
Copy link
Copy Markdown
Member

tvdeyen commented Jan 27, 2016

Still 👍

@dhonig
Copy link
Copy Markdown

dhonig commented Jan 27, 2016

So another example not mentioned that I am dealing with right now is when shipping methods are used in combination with promotions. For example, spend more than 200.00, receive free expedited shipping. The code I have inherited is hard coded to find the shipping method by the admin name, which someone may change not realizing they will cause something downstream to break. so 👍

@jhawthorn
Copy link
Copy Markdown
Contributor

👍

jhawthorn added a commit that referenced this pull request Jan 28, 2016
Add carriers and service levels to shipping methods.
@jhawthorn jhawthorn merged commit d12733a into solidusio:master Jan 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants