Posted by smartango on January 9, 2012 at 10:10am
9 followers
| Project: | Ubercart |
| Version: | 7.x-3.x-dev |
| Component: | Stock |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
stock does not update neither remove entry from its table when node update sku or node is removed.
adding these function to uc_stock.module make stock table consistent
<?php
/**
* Implements hook_node_delete().
*/
function uc_stock_node_delete($node) {
db_delete('uc_product_stock')
->condition('nid',$node->nid)
->execute();
}
/**
* Implements hook_node_update().
*/
function uc_stock_node_update($node) {
db_update('uc_product_stock')
->fields(array('nid'=>$node->nid))
->condition('sku',$node->model)
->execute();
}
?>
Comments
#1
Sorry, this is the correct one:
<?php
/**
* Implements hook_node_delete().
*/
function uc_stock_node_delete($node) {
db_delete('uc_product_stock')
->condition('nid',$node->nid)
->execute();
}
/**
* Implements hook_node_update().
*/
function uc_stock_node_update($node) {
db_update('uc_product_stock')
->fields(array('sku'=>$node->model))
->condition('nid',$node->nid)
->execute();
}
?>
#2
sorry again, update does not work with adjustments. Could be added only delete?
#3
this for update?
<?php
/**
* Implements hook_node_presave().
*/
function uc_stock_node_presave($node) {
$node->old_model = $node->model;
}
/**
* Implements hook_node_update().
*/
function uc_stock_node_update($node) {
db_update('uc_product_stock')
->fields(array('sku'=>$node->model))
->condition('sku',$node->old_model)
->execute();
}
?>
#4
Whole change (tested) on delete and update:
<?php
/**
* Implements hook_node_delete().
*/
function uc_stock_node_delete($node) {
db_delete('uc_product_stock')
->condition('nid',$node->nid)
->execute();
}
/**
* Implements hook_node_presave().
*/
function uc_stock_node_presave($node) {
if(isset($node->nid)) {
$n = node_load($node->nid);
$node->old_model = $n->model;
}
}
/**
* Implements hook_node_update().
*/
function uc_stock_node_update($node) {
if(!isset($node->old_model)) return;
db_update('uc_product_stock')
->fields(array('sku'=>$node->model))
->condition('sku',$node->old_model)
->execute();
}
?>
the test is:
1. create product with sku
2. activate stock level setting to a number (say 1)
3. change product sku and update
4. look at stock level if it is still to 1
passed
#5
If you could write that test in code as a SimpleTest that would be awesome - it would prove that this works, and ensure that it is never broken again in the future.
#6
One thing your code may not take into account: what if the new SKU already exists in the stock table, attached to another product or an attribute combination of the same product?
#7
Isn't this a duplicate of a number of other stock issues, such as #315585: nodeapi support for stock levels and customer viewable stock levels? Any proposed fix needs to be aware of those other issues and the concerns raised there.
#8
what shoud be done? check and report to original model if it exists in stock table?
<?php/**
* Implements hook_node_presave().
*/
function uc_stock_node_presave($node) {
if(isset($node->nid)) {
$n = node_load($node->nid);
$r = db_select('uc_product_stock','stock')
->fields('stock')
->condition('sku',$node->model)
->fetchAssoc();
if($r && $node->model != $n->model) $node->model = $n->model;
else $node->old_model = $n->model;
}
}
?>
I have to check and test this code.
#9
@TR patches at #315585: nodeapi support for stock levels and customer viewable stock levels are not correct for me, it update stock table for all entry with this nid, but there could be attributes combination which lead to the same nid and differents sku. (also those use nodeapi which is not available in drupal 7)
But definitively it is related to.
#10
I miss execute(). This is the whole change set
<?php
/**
* Implements hook_node_delete().
*/
function uc_stock_node_delete($node) {
db_delete('uc_product_stock')
->condition('nid',$node->nid)
->execute();
}
/**
* Implements hook_node_presave().
*/
function uc_stock_node_presave($node) {
if(isset($node->nid)) {
$n = node_load($node->nid);
$r = db_select('uc_product_stock','stock')
->fields('stock')
->condition('sku',$node->model)
->execute()
->fetchAssoc();
if($r && $node->model != $n->model) {
$node->model = $n->model;
drupal_set_message(t('SKU in use, not updated'));
} else $node->old_model = $n->model;
}
}
/**
* Implements hook_node_update().
*/
function uc_stock_node_update($node) {
if(!isset($node->old_model)) return;
db_update('uc_product_stock')
->fields(array('sku'=>$node->model))
->condition('sku',$node->old_model)
->execute();
}
?>
tested
I think one would prefer to add validation to form for sku.
Anyway this works and is a step forward.
#11
the patch
#12
a patch with test of stock consistency.
@longwave on comment 6, consistency in this patch do not take consider attribute module, for that there is another issue opened, but this patch (or something that pass the test) has to be included before, or I can make a whole patch for all, but someone suggested me to divide thing in two parts (sorry, I cant recall), the other part is in http://drupal.org/node/1415590
#13
What about in 6.x-2.10 version?
#14
maksim24 try http://drupal.org/node/315585 , we are very lazy on applying this kind of patch, not great interest on this, I think.
I raise priority, if it does not hurt to much
#15
It didn't work for me, so I queued it for retesting.
#12: ubercart-stock_consistency-1398448-12.patch queued for re-testing.
#16
The last submitted patch, ubercart-stock_consistency-1398448-12.patch, failed testing.
#17
#11: ubercart-stock-consistency-1398448.patch queued for re-testing.
#18
The patch doesn't apply anymore - it needs to be re-rolled. That's what the testbot output is saying. Testing and retesting won't help anything.
#19
this is the updated code (7.x-3.x)