Comments

shabana.navas created an issue. See original summary.

shabana.navas’s picture

Status: Active » Needs work
StatusFileSize
new55.62 KB

Initial cleaned up version. Looking for review and hoping to get some answers to the following questions so that we can further clean this up:

In the work done here: https://github.com/mattwebdev/commerce_usps the USPS services are not explicitly listed.

  1. Why do we not use services i.e. register the services in the shipping method's annotations, like we do in UPS?
  2. Why do we allow excluding services instead of including, like we do in UPS and other modules?
andyg5000’s picture

Hey shabana.navas,

I started https://github.com/blueoakinteractive/commerce_usps/tree/8.x-3.x as a fork of Matt's stuff to get it ready for D.O a while back and guess I never worked to move it over here. Not saying we should keep it, but it might have some things we can keep. I do agree that we should follow what UPS is doing regarding defined services and match the patterns so there similar to maintain.

shabana.navas’s picture

StatusFileSize
new36.57 KB

Thanks andyg5000, I'll take a look at the repo and try and merge the two. As for the services, yeah, it's better to define them like UPS is doing. There are quite a few USPS services that are available, do we have a preference for which ones we should offer?

shabana.navas’s picture

shabana.navas’s picture

Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new628.55 KB
new42.33 KB

@andyg5000 I've gone through the repo and we've got most of the stuff in the patch here. It just adds on to those changes.

The major difference is we don't have the UPSTrack class in this patch. But the class mostly contains test code so I left it out until we fully build out the feature.

The new patch attached here adds the services explicitly and removes the exclude options.

shabana.navas’s picture

shabana.navas’s picture

cameron prince’s picture

So how do https://github.com/trobey/commerce_usps and https://github.com/blueoakinteractive/commerce_usps compare?

I have been testing with the trobey version and it seems to work well. I have found a bug related to having a donation in an order along with a product shipped via USPS. I came here to check for patches and found this.

It sure would be nice to consolidate our efforts.

andyg5000’s picture

We should continue to push forward with what @shabana.navas has done above. I apologize that more hasn't been done on the 8.x version of this, I just haven't had the need for it. I didn't know there was yet another fork/version with Trobey's version. If there is useful stuff in his version, we should work to add it to #6.

andyg5000’s picture

Hey @shabana.navas

Good news... I rolled 8.x-1.x and committed your patch. The only thing that I did not include were the package definitions. I wasn't sure why you put those in config/install instead of using the shipping API and having them in the root. Let me know your thoughts there an we'll get them included. I was also not sure why there were separate files for each.

andyg5000’s picture

Version: 7.x-2.x-dev » 8.x-1.x-dev
jonnyeom’s picture

Just want to say that I am currently running the latest version of commerce_usps 8.x-1.x and everything is working great in our environments.
Thanks for the great work!

Also, extra thanks for implementing the V2 International API!

Jonathan

andyg5000’s picture

w00t. Great to hear, thanks for the feedback!

smccabe’s picture

Title: Commerce USPS 8.x-2.x » Commerce USPS 8.x-1.x
shabana.navas’s picture

@andyg5000 Thanks for committing the patch and thanks guys for testing this out. You're right, I have no idea why I added the package types in their own files and put it under config/install. The attached patch keeps it the same and adds the weight attribute to the package types.

andyg5000’s picture

Hey @shabana.navas,

What does the weight attribute on these do? For example, what significance does 3lbs have on usps_medium_flat_rate_box?

If you got this from USPS, can you include a link to the reference?

Thanks!

shabana.navas’s picture

StatusFileSize
new1.23 KB

Sorry, that was the wrong patch.

shabana.navas’s picture

StatusFileSize
new743 bytes

Actually, on further testing, I'm getting a Call to a member function getRemoteId() error with the default package declarations. Maybe something wonky on my local.

I've left the weights off for now. The dimensions have been corrected and have been fetched from https://www.stamps.com/usps/priority-mail-flat-rate/ and https://help.shipstation.com/hc/en-us/articles/360001779252-What-package...()

andyg5000’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.