World Quotes module

Lyle's picture
Offline
AdministratoreLiTe!
Joined: 08/07/2007
Juice: 6841
World Quotes module

World Quotes module review
This is just based on looking at the code. I haven't actually installed or tested it yet. That said, it looks like it should work properly. Good job.

Things to fix:
Most of these are the same as for the Regions module, but I put them here again for completeness.

  • The calls to db_query() on lines 308, 314, and 325 are open to an SQL Injection Attack. Parameterize the data. See below for how I dealt with the NULL values problem.
  • The unit conversion constants are define()d in uc_store.module, and do not need to be here.
  • MENU_CALLBACK_ITEM is not defined, MENU_CALLBACK is. My fault for writing modules that way before.
  • I think it's a good idea to remove the version and CVS information added by Drupal in the .info file. We've had version confusion because of that.
  • Please style the code according to the Drupal coding standards. Specifically, use two spaces instead of tabs to indent code. This keeps indentations consistent across text editors.
  • Remove extra files from the .zip archive (.M, .php). I was a little confused by them. *shrug*

I understand that the queries to INSERT and UPDATE the quote rules were made so that NULL values can be used. db_query() doesn't easily allow that to happen. There is still a better way than to insert the data directly into the SQL string.

The best example I can show you is in uc_shipping_package_save(). When packages are first created, they won't have tracking numbers or shipping labels, so those columns should remain NULL until they are set. The function builds the query based on what information it has and INSERTs and UPDATEs only those columns. A similar technique should work for World Quote.

World Quotes By: arbel (79 replies) Wed, 09/26/2007 - 20:45