bmcclure / drupal-commerce_fedex

Provides FedEx integration with Commerce Shipping.
5 stars 8 forks source link

Set shipment packages correctly #2

Open bmcclure opened 7 years ago

bmcclure commented 7 years ago

Currently an issue @mdlutz24 and I have both been working on a bit. Just calling it out here so we can track it.

mdlutz24 commented 7 years ago

I don't think we are going to be able to use this arkitecht library. It's outdated, using old versions of the WSDLs, and they aren't set up right. It only accepts a single line item, when it should take more than one. I think we need to use wsdlToPhp to generate our own and include them in the module, that way we can keep them updated.

bmcclure commented 7 years ago

@mdlutz24 That's unfortunate. But I'm glad you discovered that!

This seems to be a more popular open source PHP library for FedEx: https://github.com/JeremyDunn/php-fedex-api-wrapper

Do you think it'd be worth making use of that to avoid having to maintain the wsdl service directly in the module, or do you think the preferred way would be to simply do it manually?

bmcclure commented 7 years ago

There's a nice set of examples in that library as well that may be useful, and it seems to be updated regularly. However if it's easier to not use a library and do it directly, I'm alright with that route.

bmcclure commented 7 years ago

@mdlutz24 Is this what you're currently working on? We're in urgent need of this as well, so I'm trying to determine what I should work on that won't conflict with what you're doing.

If you're looking into the traits, I could work on replacing the library here. Or if this is what you're currently working on, I could either start looking into traits or start working on other aspects entirely.

mdlutz24 commented 7 years ago

That one is even older. Arkitecht was using v18 of the rateService wsdl, and JeremyDunn is using v10. Fedex is on v20. Both of these are autogenrated libraries that use scripts and the wsdls to build them. I just tried using wsdlToPhp (the one that arkitecht used) against v20 of the rate request, and it looked like it generated everything correctly, accepting arrays for the packages. my thought was if we included the library as part of the module, we could keep it updated more easily, and if we used the wsdlToPhp to build it, we would only have to change out the namespaces and the rest of the code would be fine.

bmcclure commented 7 years ago

@mdlutz24 Now I understand a little better, that sounds like a great plan to me.

mdlutz24 commented 7 years ago

I was talking to bojanz, and apparently each shipment only technically supports one package, so with the traits, we will need our own packer that separates items out into separate shipments depending on whether they are hazmat, or dry ice or regular. We may need our own checkout pane too, that combines the shipments into one rate quote eventually.

mdlutz24 commented 7 years ago

Go ahead and work on traits. I think dry ice may need it's own package type, which would have weight of dry ice, hazmat as well. I'll generate this library in our namespace and swap out the namespaces to make it work, and I'll add a "All items in one package/each item in it's own package" option to the shipping method so we can test multiple packages in the same shipment.

mdlutz24 commented 7 years ago

I'm just going to generate rateservice right now, I'll add ship and track later when we get there.

bmcclure commented 7 years ago

I've gone ahead and added you as a collaborator here so that you can contribute directly to this repo if you'd like (perhaps starting with feature branches).

Feel free to continue working in your fork as well if you'd prefer, I just wanted to open the lines up a little bit more since we're both working on these items currently and figured we could at least collaborate on one issue queue.

mdlutz24 commented 7 years ago

Whichever you prefer. I just merged in a new rateService Library directly into the module, and pushed it to my fork. I'm not opposed to pulling it back out at some point, and maintaining a vendor module alongside commerce_fedex, but I'm convinced that maintaining it ourselves is the best way to go. Again, it's a simple matter of changing namespaces, so it's not a big deal. We are now on version 20 of rateRequest, which required a couple tweaks, but it accepts multiple packages now. Do you use IRC? I just joined the #drupal and #drupal-commerce channels on freenode. It might be a better way to collaborate real time, if we are both actively working.

mdlutz24 commented 7 years ago

I added a libraryreplace branch if you want to try to merge that in with what you are doing.

bmcclure commented 7 years ago

Awesome, thanks! Is that branch in a working state? If so I can get it merged into my traits branch now.

Do you happen to be on Drupal's Slack? http://drupalslack.herokuapp.com

I'm on there regularly, and we could also PM there or create a new channel for this as appropriate. If you'd prefer I can jump on IRC as well however, not opposed to that at all.

mdlutz24 commented 7 years ago

I'm not, but I should be. I'll see about getting it going. Yes, that branch should be in a working state. I have some additional code to merge in that sets the package line items more correctly, now that I can.