kylewest / DotNetShipping

UPS, FedEx, USPS shipping rate calculators for .NET
MIT License
78 stars 66 forks source link

SmartPost and additional Service Codes #51

Closed StephenPAdams closed 7 years ago

StephenPAdams commented 7 years ago

Added service codes for USPS Domestic and USPS International as well as calls to GetServiceCodes. A client had the need to only accept certain service codes, so we can pull all the service codes via providers and let them exclude. The GetServiceCodes accomplished that.

The better change is added a new provider called FedExSmartPostProvider. Since this required a few changes and it only returns 1 rate due to the change, it will require a call to the Rate API with the FedExProvider to get all but SmartPost and then a follow up call with the FedExSmartPostProvider to get SmartPost rate(s).

kylewest commented 7 years ago

@StephenPAdams Thanks! 👍 I am on vacation this week and will take a look next week when I get back -- unless @Soulfire86 wants to review/merge?

StephenPAdams commented 7 years ago

No problem! Looks like it's just you @kylewest! Hope you had a great vacay. :)

kylewest commented 7 years ago

hey @StephenPAdams,

vacation was great but came back to the real world way behind. looking through your code today and I can't get the fedex tests (regular and smartpost) to pass due to an authentication error. I confirmed my fedex credentials are correct so I'm at a bit of a loss. If I run the sample app with production credentials I get FedEx rates but no SmartPost rates (probably because I don't know which hubId to use.

can you confirm that tests are passing with your test credentials?

screenshot-2016-09-24-at-14-09-45

Kyle

kylewest commented 7 years ago

@kylewest close #31 when this is merged.

StephenPAdams commented 7 years ago

@kylewest welcome back! For SmartPost, you need to use your production hubId that is designated to you by FedEx upon request of SmartPost functionality. If not using production, you'll need to use the test hubId (5531) that FedEx mandates per their documentation.

I can confirm this works for my production credentials as I've been using in our staging environment for the last month or so without issue.

kylewest commented 7 years ago

@StephenPAdams does it work for you in development though? Here are the unit test results with FedExUseProduction = false

2016-09-26 at 11 56 am

When I switch to FedExUseProduction = true the regular FedEx works but SmartPost does not (because I don't have SmartPost AFAIK).

2016-09-26 at 11 59 am

For reference, here is master:

2016-09-26 at 11 55 am

Issue 1: In master, FedEx was testing in production which shouldn't be the case but I didn't notice. You set it up to test in development which is correct, but now the tests are failing. If it works for you I will get in touch with FedEx to figure out why my credentials aren't working. If it doesn't work for you we'll have to do some digging.

Issue 2: Something in this PR broke the RateManagerFactory (probably the extra base class). I think we just need to update the where in RateManagerFactory: line:17 to exclude your base class. Should be relatively easy.

Let me know about Issue 1 above and we'll take it from there. Thanks for all your work on this.

Kyle

StephenPAdams commented 7 years ago

@kylewest #1 works fine here. Do you want to try using my FedEx test credentials? I can email you directly if you want to give them a go.

kylewest commented 7 years ago

@StephenPAdams, no, that's OK. I have an email into FedEx developer support to figure it out. Just need to get the tests passing so the build passes and I can push to NuGet. Hopefully I will hear back today and we can get this wrapped up.

Again, thanks for all the help.

kylewest commented 7 years ago

MERGED. 👍

Also closes #37 and closes #40.

kylewest commented 7 years ago

After fighting with appveyor for a while, deployed v0.9.5.61 to NuGet.