This module is for Drupal 7. It provides Canada Post shipping quotes for the Drupal Commerce module.

It sends an xml request (containing shipping/order data) to the Canada Post Sell Online api server and uses the results to build shipping rate options.

It requires Commerce Shipping 2.x and Commerce Physical Product.

project page: http://drupal.org/node/1356090
git: git clone http://git.drupal.org/sandbox/iswilson/1356090.git commerce_canada_post

Comments

patrickd’s picture

Status: Needs review » Needs work

Review of the 7.x-1.x branch:

  • Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):
    
    FILE: ...es/all/modules/pareview_temp/test_candidate/commerce_canadapost.install
    --------------------------------------------------------------------------------
    FOUND 14 ERROR(S) AND 1 WARNING(S) AFFECTING 8 LINE(S)
    --------------------------------------------------------------------------------
      5 | WARNING | Line exceeds 80 characters; contains 100 characters
     15 | ERROR   | Inline comments must start with a capital letter
     15 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
        |         | question marks
     28 | ERROR   | Inline comments must start with a capital letter
     28 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
        |         | question marks
     48 | ERROR   | Inline comments must start with a capital letter
     48 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
        |         | question marks
     55 | ERROR   | Inline comments must start with a capital letter
     55 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
        |         | question marks
     60 | ERROR   | Inline comments must start with a capital letter
     60 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
        |         | question marks
     63 | ERROR   | Inline comments must start with a capital letter
     63 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
        |         | question marks
     67 | ERROR   | Inline comments must start with a capital letter
     67 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
        |         | question marks
    --------------------------------------------------------------------------------
    
    
    FILE: ...tes/all/modules/pareview_temp/test_candidate/commerce_canadapost.module
    --------------------------------------------------------------------------------
    FOUND 23 ERROR(S) AND 5 WARNING(S) AFFECTING 18 LINE(S)
    --------------------------------------------------------------------------------
      96 | ERROR   | Inline comments must start with a capital letter
      96 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     114 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     123 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     129 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     148 | ERROR   | Inline comments must start with a capital letter
     148 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     197 | WARNING | Line exceeds 80 characters; contains 136 characters
     197 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     197 | ERROR   | Comments may not appear after statements.
     206 | ERROR   | Inline comments must start with a capital letter
     206 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     229 | WARNING | Line exceeds 80 characters; contains 100 characters
     238 | ERROR   | Comments may not appear after statements.
     239 | ERROR   | Comments may not appear after statements.
     240 | ERROR   | Comments may not appear after statements.
     241 | ERROR   | Comments may not appear after statements.
     242 | ERROR   | Comments may not appear after statements.
     243 | ERROR   | Comments may not appear after statements.
     244 | WARNING | Line exceeds 80 characters; contains 100 characters
     244 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     244 | ERROR   | Comments may not appear after statements.
     314 | WARNING | Line exceeds 80 characters; contains 97 characters
     406 | WARNING | Line exceeds 80 characters; contains 82 characters
     406 | ERROR   | Inline comments must start with a capital letter
     406 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     446 | ERROR   | Inline comments must start with a capital letter
     446 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
    --------------------------------------------------------------------------------
    
    
    FILE: ...les/pareview_temp/test_candidate/includes/commerce_canadapost.admin.inc
    --------------------------------------------------------------------------------
    FOUND 30 ERROR(S) AND 1 WARNING(S) AFFECTING 16 LINE(S)
    --------------------------------------------------------------------------------
      83 | ERROR   | Inline comments must start with a capital letter
      83 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     108 | ERROR   | Inline comments must start with a capital letter
     108 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     113 | ERROR   | Inline comments must start with a capital letter
     113 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     115 | WARNING | Line exceeds 80 characters; contains 141 characters
     115 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     120 | ERROR   | Inline comments must start with a capital letter
     120 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     124 | ERROR   | Inline comments must start with a capital letter
     124 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     129 | ERROR   | Inline comments must start with a capital letter
     129 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     131 | ERROR   | Inline comments must start with a capital letter
     131 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     134 | ERROR   | Inline comments must start with a capital letter
     134 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     136 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     139 | ERROR   | Inline comments must start with a capital letter
     139 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     154 | ERROR   | Inline comments must start with a capital letter
     154 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     214 | ERROR   | Inline comments must start with a capital letter
     214 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     217 | ERROR   | Inline comments must start with a capital letter
     217 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     219 | ERROR   | Inline comments must start with a capital letter
     219 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
     225 | ERROR   | Inline comments must start with a capital letter
     225 | ERROR   | Inline comments must end in  full-stops, exclamation marks, or
         |         | question marks
    --------------------------------------------------------------------------------
    
  • README.txt is missing, see the guidelines for in-project documentation.
  • All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
    ./commerce_canadapost.module
    

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

Source: http://ventral.org/pareview - PAReview.sh online service

iswilson’s picture

Status: Needs work » Needs review

Fixed everything in this list.

rade’s picture

Status: Needs review » Needs work

Review of the 7.x-1.x branch:

  • Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):
    
    FILE: ...tes/all/modules/pareview_temp/test_candidate/commerce_canadapost.module
    --------------------------------------------------------------------------------
    FOUND 3 ERROR(S) AFFECTING 3 LINE(S)
    --------------------------------------------------------------------------------
     121 | ERROR | else must start on a new line
     181 | ERROR | Missing parameter type at position 1
     184 | ERROR | Data type of return value is missing
    --------------------------------------------------------------------------------
    
    
    FILE: ...les/pareview_temp/test_candidate/includes/commerce_canadapost.admin.inc
    --------------------------------------------------------------------------------
    FOUND 7 ERROR(S) AFFECTING 6 LINE(S)
    --------------------------------------------------------------------------------
     368 | ERROR | Missing parameter type at position 1
     379 | ERROR | Missing parameter type at position 1
     381 | ERROR | Last parameter comment requires a blank newline after it
     381 | ERROR | Missing parameter type at position 2
     384 | ERROR | Data type of return value is missing
     404 | ERROR | Missing parameter type at position 1
     448 | ERROR | Data type of return value is missing
    --------------------------------------------------------------------------------
    
    
    FILE: ...les/pareview_temp/test_candidate/includes/commerce_canadapost.rates.inc
    --------------------------------------------------------------------------------
    FOUND 12 ERROR(S) AFFECTING 12 LINE(S)
    --------------------------------------------------------------------------------
      13 | ERROR | Missing parameter type at position 1
      16 | ERROR | Data type of return value is missing
      95 | ERROR | Missing parameter type at position 1
     134 | ERROR | Missing parameter type at position 1
     136 | ERROR | Missing parameter type at position 2
     139 | ERROR | Data type of return value is missing
     197 | ERROR | Missing parameter type at position 1
     200 | ERROR | Data type of return value is missing
     246 | ERROR | Missing parameter type at position 1
     249 | ERROR | Missing parameter type at position 2
     253 | ERROR | Missing parameter type at position 3
     258 | ERROR | Data type of return value is missing
    --------------------------------------------------------------------------------
    

This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.

Source: http://ventral.org/pareview - PAReview.sh online service

patrickd’s picture

Status: Needs work » Needs review

these are really small code formatting issues, so we wont block further reviews by this

klausi’s picture

Status: Needs review » Needs work
  • why can't you use drupal_http_request() instead of cURL?
  • "db_delete('rules_config')": do not run such raw SQL queries, use the API with rules_config_delete().
  • "t('Estimated delivery') . ': ' . $delivery_date": do not concatenate translated strings like this, use just one string with placeholders.
  • "'!package_count' => $package_count,": $package_count does not contain any HTML, so you should use the @ placeholder.
  • "drupal_alter('commerce_canadapost_rate_request', $rate_request_element);": aha, your module exposes hooks, so I suggest to add commerce_canadapost.api.php where all your hooks are listed.
  • commerce_canadapost_api_request(): "@return object": "object" type should only be used for stdClass objects, if you have a specific class here you should use that as type (SimpleXMLElement). Also elsewhere.

Otherwise this looks nearly ready.

tr’s picture

I'm the author of the Canada Post Shipping Quotes module uc_canadapost. I've expressed on the project page my intent to make that module work with Commerce as well as Ubercart. I would appreciate *help* with that project, rather than opening up a brand-new project for Commerce which starts from scratch and lacks some of the features of my module. I feel the community would be better served through one module used by twice as many users and developed by twice as many developers, rather than going forward with separate projects. I think it's a little silly to have all the payment methods and shipping methods duplicated between Ubercart and Commerce, with just uc_ changed to commerce_. Many years of experience have gone into the current Ubercart modules; starting from scratch and trying to rebuild the same thing in Commerce does not help anyone. Supporting both packages is a little harder than a simple port, because it requires the web service functionality to be abstracted from the source of the data, but in the end it make for a much more robust and flexible module that can even be used outside the context of Ubercart or Commerce (e.g. from a webform or a view). I think that's where the effort should go. I have started this process and I hope you will join me and help out. I am using Canada Post as the testbed because it is a very simple web service - the real benefit will show when more complex web services like FedEx are needed. The architectural changes developed for Canada Post will directly apply to all shipping quote web services.

It would be a different story if there were any significant architectural differences in shipping with Ubercart v. Commerce, but that's not the case. While Commerce has re-architected the core cart functions, things like shipping are entirely in the contribution sphere and have been simply ported from their Ubercart versions.

iswilson’s picture

TR, you make a valid case and I'd like to help you with adding drupal commerce support to the uc_canadapost project. Please message me on irc (iswilson) to discuss.

klausi’s picture

Status: Needs work » Closed (won't fix)

Alright, so I'm closing this application. If you need to promote another project at some point feel free to come back to us.

Horroshow’s picture

Hi, any progress? Is the project still alive?

storytellerjeff’s picture

Hi Iswilson,

It seems that TR doesn't have the time to release a 7.x-3.x release of Canada Post Shipping Quotes which would allow it to work with Commerce. (as per his comment 2 months ago - http://drupal.org/node/1757348 )

As it's been almost a year since this discussion began, and you seemed to have done the lion's share of the work to get this up and running, is it possible for this module to be completed and submitted? Or is there an update on your work with TR?

It would be great to have a functioning Canada Post module. On behalf of those of us that don't know how to code - Thanks for your help with this!

tr’s picture

I'd like to point out that I wasn't the one who set this to "won't fix". My main point is that I'd like HELP from the community rather than having this project replicated with minimal changes so it would work with Drupal Commerce. I think the points that I laid out above adequately express my concerns. I do have an enormous amount of code already written in a private sandbox, but at the present I don't have free time to provide the necessary support if I make this public. If anyone is interested, please contact me directly and I'll give you my phone number so we can discuss what needs to be done.

storytellerjeff’s picture

TR - I think you make a very reasonable point and hopefully you'll have the help you need soon.

I think this is just a case where those of us on the sidelines that don't have the skills to provide that help are trying to figure out how we can get things back on track. This module is an important part of the Commerce package for many of us and it's a little disheartening to have gone from having two modules nearing completion a year ago, to not having one at least in a dev release by now. It seems that until we can get a few more Drupal Commerce users with coding expertise who need this, many of us will be stuck using a ton of custom rules to try and compensate.

Again, I wasn't trying to criticize the lack of progress, just trying to spread the word that a Canada Post module for Commerce needs to get back to the level of attention and enthusiasm that it had a year ago.

rszrama’s picture

Status: Closed (won't fix) » Needs work

fwiw, I've chatted w/ iswilson about this in IRC and via e-mail. I think it's worthwhile to have a Commerce Canada Post that sits alongside the existing UPS, USPS, FedEx, etc. modules to offer calculated shipping rates to Drupal Commerce users. The Commerce Shipping 2.x branch is not part of Drupal Commerce core, but it does in fact share a very similar re-architecture to the core modules - I wrote it based on the strengths and weaknesses of what we have in core. : )

The architecture is actually quite different from what Ubercart offered on D6 at least, defining shipping methods separate from shipping services, supporting conditional availability at the method or service level for each order via Rules, and using Rules to support additional shipping rate manipulations (e.g. handling charges, weight or quantity based shipping, discounts, etc.)

iswilson appears responsive and interested in fixing up and publishing his code, and as anyone can attest, finishing up a project with your own code that tries to do one specific task (Commerce integration for Canada Post) is easier than trying to take over someone else's code that is more abstract and targets two very different systems. I'm more than happy to help iswilson get the release out, as I've worked with andyg5000 and others to get the shipping modules out for UPS, USPS, and FedEx, and if we get a better solution in the future I'm happy to champion that as well. My hunch is that iswilson himself would support such a move, especially if it were to bring additional features to the table like package sorting.

storytellerjeff’s picture

This is great news. Thanks for staying on top of this Ryan! Looking forward to testing a dev release in the future.

P.S. THIS kind of dedication is why I'm a Drupal Commerce user :)

iswilson’s picture

Status: Needs work » Needs review

I'd like to reopen this project application.

I've already put a good chunk of work into this module and I think it is fairly close to release-ready. I plan on supporting it and adding new features such as support for the new Canada Post API.

@klausi
I've made the changes you suggested in #5 and run the module through the latest coder/pareviewsh.

@horroshow, storytellerjeff
Thanks for your interest in the module. There's actually been code available for testing since November 30th of last year. It is located in my sandbox here and can be downloaded through git with "git clone --branch 7.x-1.x http://git.drupal.org/sandbox/iswilson/1356090.git commerce_canadapost". There is an issue queue on that page as well if you want to report any problems / suggest features.

Horroshow’s picture

@iswilson I used your sandboxed module in my development environment (MAMP) and it worked great for my needs (getting shipping quotes for expedited parcels). For some reasons it did not communicate with CP server when I moved it to the production server (Windows Server 2008 R2). I did not had time to investigate because I had to launch the website quickly but my guess is there's a security configuration on the server (firewall or SSL certificate maybe) that causes the problem. Is your module using a particular port or protocol to communicate with Canada Post server?

Support for the new Canada Post API would be great since they want to shut down the old CPC.

Thanks for your work, the only way I can help is by testing and suggesting as I'm not a programmer but let me know if I can do anything.

storytellerjeff’s picture

@iswilson - any update?

As above, I have a MAMP set up and don't have a dev site up on a production server that I can use to test the security/SSL issue with.

I'm unfamiliar with the release requirements for new modules, but perhaps we could get a dev version up, which would help get a few more eyes on this? I see that the module is now listed on the Commerce Shipping page, so I imagine there would be at least a few developers ready to pitch in and test if there was a dev version.

iswilson’s picture

Apologies for the lag on replies, the update notifications for this thread were going into my spam folder.

@horroshow
It's most likely a firewall issue; http is used and an exception has to be made for sellonline.canadapost.ca port 30000. I will add a note to the documentation about this thanks.

@storytellerjeff
I believe it's ready for a version 1.x but the module approval process takes time. Again it can be checked out via git for testing in the meantime (links are at the top of the page).

aendra’s picture

Status: Needs review » Reviewed & tested by the community

✗ Automated Review

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.


FILE: .../drupal-7-pareview/pareview_temp/includes/commerce_canadapost.admin.inc
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 384 | ERROR | Invalid @param data type, expected bool but found boolean
 388 | ERROR | Invalid @return data type, expected int but found integer
--------------------------------------------------------------------------------


FILE: .../drupal-7-pareview/pareview_temp/includes/commerce_canadapost.rates.inc
--------------------------------------------------------------------------------
FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 258 | ERROR | Invalid @return data type, expected int but found integer
--------------------------------------------------------------------------------

Source: http://ventral.org/pareview - PAReview.sh online service

✓ Manual Review

  • ✓ README.txt -- Good! Clear, concise, well-written. I think you need a single new-line at the end of it, though.
  • ✓ commerce_canadapost.install -- Good install routine! Couldn't find any issues.
  • ✓ commerce_canadapost.module -- Wow, couldn't find any issues. Nice!
  • ✓ commerce_canadapost.info -- No problems.
  • ✓ includes/commerce_canadapost.admin.inc -- Nicely done...
  • ✓ includes/commerce_canadapost.rates.inc -- Ditto.
  • ✓ theme/* -- No issues.

Congrats, this is one of the cleanest modules I've had the pleasure of reviewing during my short tenure in this queue. :)

You fixed all of Klausi's issues and there don't appear to be any new ones introduced. The three "use bool instead of boolean"-type messages from PAReview.sh shouldn't hold up this module, which seems both necessary and in demand at this point. Marking as RTBC.

storytellerjeff’s picture

@iswilson - Well done! :)

jthorson’s picture

Status: Reviewed & tested by the community » Fixed

Quick scan looks good. There may be an opportunity to leverage system_settings_form() on the settings form, instead of manually setting each of the variables.

Thanks for your contribution, iswilson!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

aendra’s picture

Congrats, iswilson!

rszrama’s picture

Yeah, congratulations. : )

iswilson’s picture

Thanks everybody for the support, feedback, and code reviews.

Status: Fixed » Closed (fixed)

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