This patch changes the schema, and I don't know the first thing about module update scripts, so you should probably uninstall the module before patching. Sorry for the inconvenience.

The company I work for recently completed some work on a site that absolutely needed this module working, and I finally had the time to go back and turn it all into a patch.

I'd go through and submit patches for each of the issues here, but the fixes are fairly intertwined and depend on each other.

Here's what this fixes:

#738834: doesn't save settings
Turns out this was actually in issue with the schema, it was using the nid as the primary key, so only one method's settings could be saved per node. This patch adds another field to be the primary key.
#963318: Seems to only work with product nodes, not with classes
I think this was a mixture of missing the code for classes at one step, and slightly malformed query. Classes and Nodes both work now.
#849846: UPS Compatibility with Ubercart Product Quote module
The patches to uc_product_quote were applied, and then re-worked to include no references to any particular shipping module.
#886136: USPS
See below

Changes

  • The two tables in the schema have each had an extra field added to act as the primary key, and the tables have been renamed to uc_pq_product and uc_pq_class
  • The Fedex specific (and after patching, UPS specific) code has been completely removed from the module.
  • The module now implements a hook called hook_shipping_method_filter($method, $product, $services) that the shipping modules can invoke, instead of calling a function specially built for that module.
  • Any shipping module that properly invokes hook_shipping_method_filter() should be compatible. I'll attach patches for the the 6.x-2.x versions of the uc_fedex, uc_ups, and uc_usps modules. I'll submit them to the appropriate issue queue if this gets committed.
  • Due to the use of hook_shipping_method_filter, the old patch for uc_fedex is no longer compatible.
  • The administrative interface now has options to completely disable a certain method, and a few things have been reworded to be a little clearer.
CommentFileSizeAuthor
#6 service-hooks-canadapost.patch972 bytescyborg_572
service-hooks-usps.patch1.49 KBcyborg_572
service-hooks-ups.patch1.64 KBcyborg_572
service-hooks-fedex.patch1.05 KBcyborg_572
uc_product_quote-working.patch13.43 KBcyborg_572

Comments

end user’s picture

end user commented

Is it possible to get a patch for Canada post?

stesind’s picture

stesind commented

Thank you for the patch! Unfortunately if I use patch < ... with the actual dev module I only get errors that the patch cannot be applied. Is there a trick? Can you upload your version of the module?

cyborg_572’s picture

cyborg_572 commented

#1 - I'll look into this next chance I get, if it's structured like the other shipping modules then it shouldn't be too much trouble.
#2 - I've only been using git since about a week before I made these patches, so I have to ask: do git patches usually work with a standard patch command like that? I probably should have mentioned these are all git patches. Also, since I was just getting the hang of git when I made these, they may very well be broken. I don't have the files on this computer, so I will look into this when I get a chance. Keep me posted if you figure out anything about what the problem is before then.

stesind’s picture

stesind commented

Thank you for the answer! I found out in the meantime that you are using a git repository. I created one and applied the patches there. But since I had enabled the module before the changes in the tables were not applied. Uninstalling the module unfortunately does not uninstall the tables. I already deleted the tables and the entry in system but it did not help. Have to investigate more.

cyborg_572’s picture

cyborg_572 commented

The extra tables shouldn't be too much of an issue. They'll sit there taking up a bit of space, but they shouldn't get in the way or anything.

cyborg_572’s picture

cyborg_572 commented
StatusFileSize
new service-hooks-canadapost.patch972 bytes

I finally got a chance to try and put together a patch for Canad Post too. I don't have a very good test environment for these kinds of modules right now though, so in theory this patch should work.

djg_tram’s picture

djg_tram commented

@cyborg_572, could you just zip the whole package as it is working for you now and attach to a message here? I'd also need a D7 port and I'm willing to do it in the coming weeks, when I'll have some time to spare. We should also contact the original maintainer to see if she or he is still willing to maintain the product or whether it is abandoned.

djg_tram’s picture

djg_tram commented

I went deeply into it and that's not so simple. There are numerous issues in the original module, including some misunderstanding of how FAPI actually works, that you couldn't really sort out with those patches. The way you hooked the quote filter to Fedex is an issue for me, for instance, because I don't have Fedex over here in Europe... :-)

Also, this is not a safe way to handle database queries, besides, this duplication of tables is really unnecessary, simply a single table with a flag in each record whether it is a product or a class would be just fine. So, it would really need a major rewrite but that should preferably happen for D7 now but if you need it for D6, we could do it in parallel. But I'll contact AntoineSolutions first to see how we can proceed.

cyborg_572’s picture

cyborg_572 commented

The original version is tied to fedex, my patch removes that dependency, unless I missed something somewhere. I'm going to try and get this up in a sandbox project at some point today. I've got some updated patches for the new versions of UPS and USPS shipping that will go up with it. I'll post here when that happens.

djg_tram’s picture

djg_tram commented

Then I got lost among versions and patches. I'd need to rectify it, even if still somewhat ugly inside, for a site, the problem is that I have at least twenty other things to do at the same moment... :-)

I wrote to the project maintainer, we'll see what reply we get. It might be abandoned or we might get access to co-maintain it, after all, and then we can commit right here, that would be the best solution. I rewrote the admin form to handle FAPI properly, also needed to add some extra handling for UC Flatrate that seems to be a minor nuisance. I'll contact you by PM, then I can send you my current version (it can save but doesn't work right now because of the Fedex dependency). If you can see how this compares to your non-Fedex version, we can have something already working by the time Antoine Solutions replies back.

cyborg_572’s picture

cyborg_572 commented

I've misplaced the original repo I made this patch from, but I've cloned the orignal and patched it and confirmed that it is character-for-character identical to the version of this module being used in production on one of our client sites, and it seems to be working perfectly there. I then added all the shipping module patches into a subfolder.

Find it here: http://drupal.org/sandbox/cyborg_572/1363828

Master branch is identical to the master branch here, "sandbox" branch has all the fixes for now.

djg_tram’s picture

djg_tram commented

Yes, I've just seen it. And I've just received an e-mail from Jon Antoine saying that he no longer can maintain it and would be happy to pass it along, provided the persons have Git access. We do have that, so I endeavored to accept the responsibility :-) and mentioned that you might also be interested in doing it together, although I haven't promised anything in your name, of course.

But if you agree, we could do it for sure, actually, first we could finish up the D6 branch and then it could be ported to D7 easily. In this case, you could simply commit whatever changes you have here, without the need for the sandbox.

cyborg_572’s picture

cyborg_572 commented

Okay, so then what I will do is create merge into create the 6.x-1.x branch, tag it 6.x-1.0 (or 6.x-1.x-beta1? Is that how that works? In the state I have the module, it is working) and then when you give me access I will push that, and then see how it goes from there.

Does that sound good to you, does it make sense? I don't do a whole lot of module maintenance, so I'm always a bit unsure of the right way to structure these things.

EDIT: the branch was there already.
EDIT 2: I'm getting confused by the tags/branches whatever, and typing before I finish figuring things out.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Closed (outdated)

I am closing this issue, as it has been created for a release that is now not supported.