Hi Guys, this is my first post here, but I'm happy to be contributing to your project and hope you find my contributions useful. I work for a company called 'Craftyspace', we do web development for customers using our fairly heavily modified version of Drupal. My job was to do a quick review/analysis on Ubercart, and determine if was a good cart system for us to switch to. Though I did find a bug (noted somehwere else in this issue tracker just recently) I thought the system overall was very impressive and I was able to spend the time to fix it.
A note about the code I'm going to paste in: Whenever we modify another modules code, we use html style tags to indicate what was changed. For example:
//<craftyspace+>
new;
code;
here;
//</craftyspace+>
//<craftyspace->
//this;
//code;
//removed;
//</craftyspace->
Or for 1 liners that are removed:
//<craftyspace-/>this line was removed;On to the bug:
On editing product options (node/n/edit/options):
* user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'AND oid = 1' at line 1 query: DELETE FROM uc_product_options WHERE nid = AND oid = 1 in /home/dev/common/crsp/branches/5-1/dev/drupal/includes/database.mysql.inc on line 172.
The problem is in function uc_object_options_form_submit() in uc_attribute.module (please forgive me for not having line numbers, I've already altered my copy of the module so my numbers wouldn't correspond).
In the following line, $type is not defined, and the first %d matches to $form_values['id'] instead of $oid.
db_query("DELETE FROM $opt_table WHERE $id = $type AND oid = %d", $form_values['id'], $oid);
This also yields a problem where if the form is submitted again, you'd have a sql error indicating duplicate entries because the previous ones aren't being deleted.
Here's my solution (note the craftyspace tags to see whats removed and added)
/**
* Submit handler for uc_object_options_form().
*/
function uc_object_options_form_submit($form_id, $form_values){
if ($form_values['type'] == 'product'){
$attr_table = '{uc_product_attributes}';
$opt_table = '{uc_product_options}';
$id = 'nid';
$sql_type = '%d';
//<craftyspace+>
$type = $form_values['id'];
//</craftyspace+>
}
else if ($form_values['type'] == 'class'){
$attr_table = '{uc_class_attributes}';
$opt_table = '{uc_class_attribute_options}';
$id = 'pcid';
$sql_type = "'%s'";
//<craftyspace+>
$node = node_load($form_values['id']);
$type = $node->type;
//</craftyspace+>
}
// Adjustments have no meaning if possible option combinations are changed in any way.
foreach ($form_values['prod_attr'] as $prod_attr){
db_query("DELETE FROM $attr_table WHERE $id = $sql_type AND aid = %d", $form_values['id'], $prod_attr['aid']);
db_query("INSERT INTO $attr_table ($id, aid, ordering, required, default_option) VALUES ($sql_type, %d, %d, %d, %d)",
$form_values['id'], $prod_attr['aid'], $prod_attr['ordering'], $prod_attr['required'], $prod_attr['default']);
if (is_array($prod_attr['options'])){
foreach ($prod_attr['options'] as $oid => $option){
//<craftyspace-/>db_query("DELETE FROM $opt_table WHERE $id = $type AND oid = %d", $form_values['id'], $oid);
//<craftyspace+>
db_query("DELETE FROM $opt_table WHERE $id = '$type' AND oid = %d",$oid);
//</craftyspace+>
if ($option['select']){
db_query("INSERT INTO $opt_table ($id, oid, cost, price, weight, ordering) VALUES ($sql_type, %d, %f, %f, %f, %d)",
$form_values['id'], $oid, $option['cost'], $option['price'], $option['weight'], $option['ordering']);
}
else if ($form_values['type'] == 'product'){
$aid = $prod_attr['aid'];
$match = 'i:'. $aid .';s:'. strlen($oid) .':"'. $oid .'";';
db_query("DELETE FROM {uc_product_adjustments} WHERE nid = %d AND combination LIKE '%%%s%%'", $form_values['id'], $match);
}
}
}
}
drupal_set_message(t('The @type options have been saved.', array('@type' => $form_values['type'])));
}I hope that's helpful, thanks for the great work so far!
-Justin
Tried the attributes fix, but get an error:
Hello, I tried implementing the above fix, but got an error...see attached image file for other info. THanks much for any help in advance!
* warning: Invalid argument supplied for foreach() in /home/p2011r74/public_html/sites/all/modules/drupal-5.7/modules/node/node.module on line 521.
* warning: implode() [function.implode]: Bad arguments. in /home/p2011r74/public_html/sites/all/modules/drupal-5.7/modules/node/node.module on line 525.
* user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1 query: SELECT n.nid, n.vid, n.type, n.status, n.created, n.changed, n.comment, n.promote, n.sticky, r.timestamp AS revision_timestamp, r.title, r.body, r.teaser, r.log, r.format, u.uid, u.name, u.picture, u.data FROM node n INNER JOIN users u ON u.uid = n.uid INNER JOIN node_revisions r ON r.vid = n.vid WHERE in /home/p2011r74/public_html/includes/database.mysql.inc on line 172.
Tried it and still have the original error
Has this been moved farther?
I am using 5.x-beta 8
and MySQL version 4.1.22-standard
Re: Tried it and still have the original error
Actually the bug was that I hadn't changed one of the $type variables to $sql_type, which stays the value it should be. It's fixed in the latest code.
Re: Re: Tried it and still have the original error
sorry, my error. I downloaded the current code yesterday.. 5x 1.0 beta 5. is it currenter than that? I am aware that it is probably all my own fault, but my class-->attributes fail to show and I get all this error message still. What am I still messed up on, Ryan???
-Wolf
Re: Re: Re: Tried it and still have the original error
When I say "latest code", I usually mean the development code in the Bazaar repository. This is where work is done after each release, so all the changes I make go there. Ryan should make a new release today, which will have this fix in it.
Now I am looking into the Bazaar
Canonical says (in Ubuntu repo information:
arch-based distributed revision control system
GNU Arch is a revision control system with features that are ideal for projects
characterised by widely distributed development, concurrent support of
multiple releases, and substantial amounts of development on branches.
It can be a replacement for CVS and corrects many mis-features of that system.
bazaar is an implementation of Arch in C, based on tla. It focuses on making
tla's UI more accessible, but also has smarter merging and gettext support.
Unless you have a pressing reason to use bazaar you should use some other
revision control system as upstream development has ceased.
Homepage: http://bazaar.canonical.com/
I am installing anyway since my pressing reason is to get the Ubercart working.
---Well I am not "getting" this bazaar thing, Lyle, though I downloaded the bazaar package. Does the ubercart-bzr.tar.gz file contain the most recent, dev code? If so, can I use that rather than figuring out the bazaar software? I have a project in production, so I am wanting the easier shorter route instead of embarking on truly memorizing the php code-base. It is 1:00am on "Presidents Day" and I still have hundreds of products to place and update.
---- OK, I answered my own question. THe attributes.module change, which was 1 artfully placed comma somewhere around line 1000 fixed the issue. Doesn't seem like it blew up anything else. Yayy!