Fanyalla [Fanyalla]

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

As per http://drupal.org/cvs-application/requirements, the motivation message should be expanded to contain more details about the features of the proposed module, and it should include also a comparison with the existing solutions.

Sounds like a pretty cool module and something i need as well? So you have a project page for your module?

Hello,

I am interested in this module, and this will be my 1st attempt at posting an issue!

I am getting an error on the cart page:

warning: Invalid argument supplied for foreach() in /httpdocs/includes/form.inc on line 1207.

Currently, I am running Drupal 6.16 (Apache/2.2.3, PHP 5.2.13, & MySQL 5.0.90), Ubercart 6.x-2.2, with your module from comment #7.

My attributes are set as checkboxes. The cart will show attributes and update, however any selected attributes are unchecked when you visit the cart; see the attached image. Also the error is duplicated if multiple items are in the cart.

Thank you for working on this module. I can use this module and will be willing to test and provide whatever help I can.

Thanks,
e.

Hello,

Thank you very much for your time and effort!!

The latest version of the module (#14) seems to work. I will test more and report back any issues.

One thing I would need to implement is some sort of collapsible attribute list in the cart. The site I need it for is gong to sell pizzas and they have many topping choices. I will attach a couple of screen shots. One will be the cart (with only a few attributes), and one the product listing page (attributes will be collapsible).

Thank you,
e.

I tested this code and it seems to be working pretty well! Of course it would need to be looked at by a couple of other people maybe? Have you tried to post this as a patch to the current uc_attributes module?

I tested this code and it seems to be working pretty well!

That is not the only criteria used in reviewing the proposed modules. The module needs an appropriate review.

For all us newbies: what is an "appropriate review" ?

That way we can be more helpfull, because this issue queue is quite long :-)

Subscribing - Have been working on a cart theme since Drupalcon SF following Johnathon Rowney's presentation. Came across the attributes issue (amongst others) and am working a solution here within our needs.

Does seem to need more work :)

After installation of module in #14, onto a working UC installation with 12 products and 5 defined Attributes with 2 or 3 options each and each Product carrying one or two attributes errors/warnings returned from the db layer as I attempted to turn of a few Attributes :-(

I saw roughly 30 messages, all SQL errors, and the new settings did not take :-(

I wish I had the time to debug the issue, but I must move on for now...

1.     $schema['uc_attributes_in_cart'] = array(
               'description' => t('Determine which attributes to display in cart.'),
               'fields' => array(
                       'aid' => array(
                               'description' => t('Primary identifier for attributes.'),
                               'type' => 'int',
                               'size' => 'tiny',
                               'unsigned' => TRUE,
                               'not null' => TRUE,
                       ),
   

   Database schema descriptions should not be passed to t(); see what done from Drupal core modules.
   Check also the indentation character used in the installation file because it's not the two spaces reported in the coding standards.

2. function uc_attributes_in_cart_install() {
       drupal_install_schema('uc_attributes_in_cart');
   
       drupal_set_message(st("The <em>Attributes In Cart</em> module has been installed. You can change the settings of this module under !link.", array('!link' => l(st('Administer > Store administration > Configuration > Attribute settings > Attributes in Cart Settings'), 'admin/store/settings/attributes/in-cart'))));
   }
   

   The function t() is available to hook_install(), and hook_uninstall().

3.             $form['items'][$i]['attributes_in_cart'] = _uc_attribute_alter_form(node_load($form['items'][$i]['nid']['#value']));
   

   The function is loading a node without to check if the currently logged in user has the permission of seeing any data from that node, depending on the case, that can be a security issue.

4. 
                   // Do not show the attribute form.
                   else $form['items'][$i]['attributes_in_cart'][$option['aid']]['#access'] = FALSE;
               }
   

   See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how the code must be formatted.

5.     $items['admin/store/settings/attributes/in-cart'] = array(
               'title' => 'Attributes in Cart Settings',
               'description' => 'Administer the settings of attributes in cart.',
               'page callback' => 'drupal_get_form',
               'page arguments' => array('uc_attributes_in_cart_settings'),
               'access arguments' => array('administer attributes in cart settings')
       );
   

   Strings used in the user interface should be in sentence case.

6.     for ($i = 0; $i < count($items); $i++) {
           // We set the value of the attributes for each item, based on the value of the selected option.
           foreach ($items[$i]->data['attributes'] as $aid => $option) {
               $items[$i]->data['attributes'][$aid] = $form_state['values']['items'][$i]['attributes_in_cart'][$aid];
   
               // We must add useless data to the data array of the cart item, because if we have multiple products that have the same nid and attributes, the module uc_products treats these items as on item if you try to change the quantity of one of the items.
               $items[$i]->data['uc_attributes_in_cart'] = $i;
           }
   

   When possible, the code lines should not be longer than 80 characters. That limit should be always respected for the comments.

I forgot to change the status.

The indentation needs to be reduced in some cases, but the code is better now.

Thank you for your contribution!
I am going to update your account so you can opt into security advisory coverage now.
These are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, 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.

I thank all the dedicated reviewers as well.

I just tested this out for a client of mine needing this functionality. It's a pretty good module. There is just a small syntax error on line 196 of the module file; the table field is in quotes (either show_form or show_title) and errors when I attempt to change the settings in admin/store/settings/attributes/in-cart. I removed the quotes and it worked as expect. Thanks for module!

-Marcus

I am giving credits to the users who participated in this issue.