uc_dropdown_attributes

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.

Seems like an interesting module.

Just tried it on a test site of mine, doesn't seem to work. (the backend seems alright, but nothing happens on the frontend).
Can you post a screen of a configured node/20/edit/dependencies page? Maybe I configured something incorrectly.

I am no developer, just a simple drupal user.

Thanks.

• The points reported in this review are not in order or importance / relevance.
• Most of the times I report the code that present an issue. In such cases, the same error can be present in other parts of the code; the fact I don't report the same issue more than once doesn't mean the same issue is not present in different places.
• Not all the reported points are application blockers; some of the points I report are simple suggestions to who applies for a CVS account. For a list of what is considered a blocker for the application approval, see CVS applications review, what to expect. Keep in mind the list is still under construction, and can be changed to adapt it to what has been found out during code review, or to make the list clearer to who applies for a CVS account.

1. Menu descriptions and titles, schema descriptions are not passed to t().
2. Hook implementation comments should be like the following one

   /**
    * Implements hook_menu().
    */
   

3. function uc_dropdown_attributes_form_alter(&$form, &$form_state, $form_id) {
     if (preg_match('/^uc_product_add_to_cart_form.*/', $form_id)) {
       $nrows=db_result(db_query('SELECT COUNT(*) FROM {uc_dropdown_attributes} WHERE nid="' . $form['nid']['#value'] . '"'));
   
   

   Code that doesn't use db_query()-placeholders is considered not secure.

4. Code should use Drupal Unicode functions, when the string can be an Unicode string.
5. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how the code should be formatted.

6.         if (strlen($form_state['values']['attributes'][$item->aid])==0) {
             $attribute = uc_attribute_load($item->aid);
             form_set_error('edit-attributes-' . 
                            $item->aid . '-wrapper', $attribute->name . ' field is required');
           }
   
   

   Strings used in the user interface should be translated.

7.   $form['intro'] = array(
       '#type' => 'markup',
       '#value' => '<p>Since dropdown attributes may not appear, they cannot be always required.  The required checkbox applies only when the dropdown attribute appears.  Any dropdown attribute is also checked under the attributes table to make sure it is not required there as this would cause validation errors.</p><p>If Javascript is not enabled, a Choose button appears at the bottom of the page to allow this page to be reloaded with the updated values.</p>',
     );
   
   

   It should be drop down

I am adding the tag we are using for Ubercart related modules.

Trobey,

As Drupal has since moved over from CVS to Git, I'd suggest migrating this to a Git Full Project Access application as per http://drupal.org/node/1075406.

Updating title and tagging.

You can remove the $Id$ as we don't use those since the git migration.

Probably want to remove the '

' from within the t() in your long text; normally you only want the translatable stuff to be in there, not markup.

A @file header on each file would be nice. A function header for every function would also be nice.

Have you tested if this works with js off? Is that a goal?

It looks to me like this should go through OK.

Works for me. Unless there are objections someone will come by and promote you to have full project permissions within the next several days. Thanks for your contribution and your future contributions! And thanks for your patience. Sorry this was so awful.

Hi trobey,

Thanks for your contribution and welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

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. As someone who has recently completed this process, your input would be especially useful in the code review group as we work to improve this process.