[D6] Ubercart Product Registration

I'm sorry, but where is the archive with the code to be reviewed?

Hello, and thanks for applying for a CVS account. I am adding the review tags, and some volunteers will review the code, pointing out what it needs to be changed.

Tagging all CVS applications with ubercart-related code.

Here's a few review points, although this is not a comprehensive review:

1. Please remove the version line from the .info file.
2. In the .install file,
   

   while( $r = db_fetch_array($q))
     variable_del($r['name']);    

   should be:

   while( $r = db_fetch_array($q))
   {
     variable_del($r['name']);
   }   

   (see http://drupal.org/coding-standards#controlstruct)

3. Your commenting style doesn't comply with the coding standards either. ie,
   

     //
     // restrict control to delete a registered product
     //
   

   should be:

     // Restrict control to delete a registered product.
   

   Please review the coding standards section on comments at http://drupal.org/coding-standards#comment

4. There seemed to be some coder errors as well - please run this module through coder (http://drupal.org/project/coder), and fix the flagged errors.

Another suggestion (not an acceptance blocker, just a friendly suggestion):

Rather than loading the uc_product_registration.views.inc in hook_init() on every pageload, you could just put your hook_views_api() function in the .module file. This saves the .views.inc file from having to be loaded on pageviews where it isn't needed - Views will use the hook_views_api() file to find the location of it when it is needed.

I am leaving at 'Needs Review'. A quick look suggests this module is fine, but I don't have time to do a really in-depth review.

Hi. Please read all the following and the links provided as this is very important information about your CVS Application:

Drupal.org has moved from CVS to Git! This is a very significant change for the Drupal community and for your application. Please read the following documentation on how this affects and benefits you and the application process:
Migrating from CVS Applications to (Git) Full Project Applications

• The status of this application will be put to "postponed" and by following the instructions in the above link, you will be able to reopen it.
• Or if your application has been "needs work" for more than 5 weeks, your application will be marked as "closed (won't fix)". You can still reopen it, by reading the instructions above.

wjaspers reacted promptly and well to BrianV's feedback back in the CVS days, has figured out all the hoops to jump through to actually get a review now, and the code looks good to me. Setting RTBC to give vetted git user access.

@kiam, why did you move this from RTBC back to needs needs review?

Edit: I accidentally put an exclamation point on the end instead of a question mark - I meant it to be a question, not a yelling statement :)

There are a lot of coding style/documentation problems. See http://drupal.org/coding-standards

Some of these problems are (in no particular order):

A lot of the files in this project are missing an EOL at the end.

Get rid of $Id$ - this was only needed for CVS

Documentation comments need work. See http://drupal.org/node/1354
For example:

"Implementation of" should be "Implements".
Formatting of function documentation blocks.
Missing comments for some functions.

hook_uninstall() has a block of commented-out code that should be removed.

$q = db_query("SELECT name FROM {variable} WHERE name LIKE 'uc_product_registration%'");
should be
$q = db_query("SELECT name FROM {variable} WHERE name LIKE 'uc_product_registration%%'");

Get rid of extra spaces, for example elseif ( !$nid ) { should be written as elseif (!$nid) {

There's lots of trailing whitespace in your code.

Ternary expressions like ($node->vid?$node->vid:NULL) should be written as code>($node->vid ? $node->vid : NULL)

I'll give it a functional review after all the code style and documentation style problems are cleaned up.

Adding UC application tags.

@wjaspers: You should make your changes directly in the Git sandbox. Don't attach the changes as a ZIP here.

.install file - using unique keys where I think you want to be using primary key
registration date is a datetime but purchdate is char 20. What's the reason for this?
I've had problems using datetime in mysql. I wound up using an integer column to store unix timestamps, as some drupal core tables do. Have you tested this?

.module file

line 88: hook menu
- All you need in your doc block is 'Implements hook_menu'
- You don't need the $tree variable, and it isn't used anywhere
- $items['products/register/%'] - uses a place holder argument, but this is not included in your page arguments for this menu item. This makes me wonder if you've tested this module

Line 131 - Same documentation error as above. This is true for all hook implementations

Line 171 -uc_product_registration_checkanonymous() - drupal has a function for this: user_is_anonymous()

This is enough for now. There's quite a bit more review needed for this project.

It appears that you have successfully completed the application process with a different application, and been granted the 'create full projects' permission:

UC Add to Cart Block: http://drupal.org/node/1149550

Once their first application has been successfully approved, then an applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue.

With this in mind, I have marked your other applications as 'closed(duplicate)'. If this is incorrect, and you do not yet have the ability to create full projects, then please feel free to re-open this application.

Thanks in advance for your patience and understanding!