Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set shipment packages correctly #2

Open
bmcclure opened this issue Mar 7, 2017 · 14 comments
Open

Set shipment packages correctly #2

bmcclure opened this issue Mar 7, 2017 · 14 comments
Assignees
Labels

Comments

@bmcclure
Copy link
Owner

bmcclure commented Mar 7, 2017

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

@bmcclure bmcclure added the bug label Mar 7, 2017
@mdlutz24
Copy link
Collaborator

mdlutz24 commented Mar 7, 2017

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
Copy link
Owner Author

bmcclure commented Mar 7, 2017

@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
Copy link
Owner Author

bmcclure commented Mar 7, 2017

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
Copy link
Owner Author

bmcclure commented Mar 7, 2017

@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
Copy link
Collaborator

mdlutz24 commented Mar 7, 2017

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
Copy link
Owner Author

bmcclure commented Mar 7, 2017

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

@mdlutz24
Copy link
Collaborator

mdlutz24 commented Mar 7, 2017

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
Copy link
Collaborator

mdlutz24 commented Mar 7, 2017

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
Copy link
Collaborator

mdlutz24 commented Mar 7, 2017

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

@bmcclure
Copy link
Owner Author

bmcclure commented Mar 7, 2017

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
Copy link
Collaborator

mdlutz24 commented Mar 7, 2017

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
Copy link
Collaborator

mdlutz24 commented Mar 7, 2017

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

@bmcclure
Copy link
Owner Author

bmcclure commented Mar 7, 2017

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
Copy link
Collaborator

mdlutz24 commented Mar 7, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants