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();
}
?>
Files: 
CommentFileSizeAuthor
#22 ubercart-stock_consistency-1398448-22.patch5.08 KBrjlang
PASSED: [[SimpleTest]]: [MySQL] 2,969 pass(es).
[ View ]
#19 ubercart-stock_consistency-2-1398448-19.patch3.59 KBsmartango
PASSED: [[SimpleTest]]: [MySQL] 2,887 pass(es).
[ View ]
#12 ubercart-stock_consistency-1398448-12.patch3.45 KBsmartango
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ubercart-stock_consistency-1398448-12.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#11 ubercart-stock-consistency-1398448.patch1.43 KBsmartango
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ubercart-stock-consistency-1398448.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

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();
}
?>

sorry again, update does not work with adjustments. Could be added only delete?

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();
}
?>

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

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.

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?

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.

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.

@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.

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.

Status:Active» Needs review
StatusFileSize
new1.43 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ubercart-stock-consistency-1398448.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

the patch

StatusFileSize
new3.45 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch ubercart-stock_consistency-1398448-12.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

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

What about in 6.x-2.10 version?

Priority:Normal» Major

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

It didn't work for me, so I queued it for retesting.

#12: ubercart-stock_consistency-1398448-12.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, ubercart-stock_consistency-1398448-12.patch, failed testing.

Status:Needs work» Needs review

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.

StatusFileSize
new3.59 KB
PASSED: [[SimpleTest]]: [MySQL] 2,887 pass(es).
[ View ]

this is the updated code (7.x-3.x)

Toc toc.
@debo7debo, @TR could it be committed? is it something I have to do myself?
I get forbidden, I am reading drupal git handbook, just thought I can not commit without maintainer approval, and I do not know what is the policy for sending patch.

This patch add code and a test case, and do not invalidate the other tests, the code is relly short, there are some other tests suggested for this?

StatusFileSize
new5.08 KB
PASSED: [[SimpleTest]]: [MySQL] 2,969 pass(es).
[ View ]

I've tested patch #19. It works properly for product nodes that have their own SKU. However, if the product node has adjustments that have their own SKU(s) (e.g., multiple sizes for a shirt product) altering the adjustment SKU on the node adjustment edit page does not change the SKU in the stock table.

For this use case, updating the stock table needs to be made to happen within or in connection with function uc_product_adjustments_form_submit().

I've used hook_form_FORM_ID_alter() to insert an additional submission function into uc_product_adjustments_form() that checks for changes and updates the stock table. This seems to work. I've rolled these changes together with those from #19 in the attached patch.

Thanks rjlang for your patch #22, I've tested it and it works very nice except when you have attributes.

First Test: You create a content, then you add attribute to your SKU product, then you adjust the SKU with extensions, then you display the SKU stock.
The SKU stock with attribute are fine.
And you find a line with the original SKU product that we do not use because the stock is in the attributes!
I think we need to delete this line with original SKU because we don't use it.

Second Test : When you modify the SKU product, the SKU stock with attribute do not change.
Only the SKU stock without attribute is modified.
Normally, if we modify the SKU product, the SKU stock with attribute needs to be updated. Isn't it ?

Sorry rjlang, but it does't work in multilingual, when you save the SKU Product in the other language, you find the two messages:
Notice: Undefined property: stdClass::$original in uc_stock_node_presave() (line 346 of /homepages/.../sites/all/modules/ubercart/uc_stock/uc_stock.module).
Notice: Trying to get property of non-object in uc_stock_node_presave() (line 346 of /homepages/.../sites/all/modules/ubercart/uc_stock/uc_stock.module).

And the system told me that the SKU product is "in use, not updated" and my content "has been created." without the mandatory SKU product.

@Phil88 #23:

Regarding "First Test," "And you find a line with the original SKU product that we do not use because the stock is in the attributes!". Let's suppose you have a product whose base SKU is P, and then you add attributes with SKUs P-1 and P-2. Now, if you don't want to track stock item P, only P-1 and P-2, then under the "Stock" tab, you should uncheck stock item P, and only leave P-1 and P-2 checked. If someone has a use case where they track both the base SKU and attributed SKUs, we should not arbitrarily break support for this usage (which was previously supported). Let the user decide which SKUs they want to track.

Regarding "Second Test", "When you modify the SKU product, the SKU stock with attribute do not change. Only the SKU stock without attribute is modified." This is entirely consistent and correct. If you modify the SKU of a product, you need to decide whether you as store manager are going to modify the SKUs of the attributed products and how to modify them. The addition of extensions to SKUs for attributes is always done manually according to the choice of the user; we cannot assume that each user is going to use the same system for modifying SKUs for attributes.

Conclusion: I think the original behavior is the correct behavior.

@Phil88 #24: I"ll look into this. I've not done anything with multilingual support before, so it may take a little while while I bone up on that topic.

I opened another report, complete of tests for attributes consistency. I have done so, at the same time I opened this, because someone sudjested to divide the two parts.
Now you can of course use your time to rewrite your solution, but I have to be honest saying you are solving an already solved problem.
check my opened issue, i'm not at work now

Thanks rjlang and smartango for your feeback,

#23 First Test: Yes, perfect and good logic and keep history, I will use ‘active’ mark.
My first thoughts were the mark ‘active’ is only when you want to follow the product with a threshold, and a publish product has automatically valid stocks but, perhaps, not for all the attributes we add. The best way will be to create a new flag ‘valid stock’ just near ‘active stock’ but, as smartango dixit, it’s another topic. I had also thought for this new flag to get easiest extracts from views, more comprehensive for the customers.

#23 Second Test: Your way is nice and I adopt it.

#24 Multilingual: I'm waiting for your birth in multilingual, I'm already ready in starting block to test your new patch, many thanks in advance,

I regret for my last comment, I did not recall that here https://drupal.org/node/1415590 tests are missing,
also rjlang's #22 is the way I like (form_alter way).

On tests matter I would suggest these :

MultipleProduct:
insert a product without variants and model: product0-xxl
then a product with variant, model: product0, variants: product0-l, product0-exxl

now change the second variant product0-exxl to product0-xxl
(there is a sku duplication problem)

Or something like that lead my patch to add nid condition, can you check?
I closed the #1415590: stock and attributes consistency as duplicate of this

@longwave I do not think so, in fact #745912: Stock values lost when SKU is altered relates to 6.x-2.4 branch, not 7.x-3.x.

Total newbie here having this exact issue (same as OP)

Not familiar with git or patches. Can someone direct me to how to update the .module file with the most up to date tweak?

Your help would be very much appreciated! Stock and SKU's in a mess!

@julinewbie paste the code at #10 at the end of uc_stock.module file (without

<?php
and
?>
), at least this solve the problem with removing product and sku.

Then ... @longwave why not commit patch at @22 ? does it invalidate existing tests? I think that the patch could be committed while leaving this bug open for missing tests

thank you! I will give that a go....

I can't thank you enough! It's worked! :o)

I use the patch #22 with only one language for a site and in multilingual for another site and it works perfectly.

If you have more than one language, the only thing that you cannot do is to modify the SKU. You can modify the first SKU but not the second SKU or the third SKU with the same value because the system tells you "in use, not updated", in fact, the system manages like double or something like that.

Any way, it's good patch because it tells you perfectly something right 'do not modify the SKU' to get a good management of the products sold by the web site, mainly in multilingual!

Issue summary:View changes

well actually reviewing the patch, this code:

<?php
function uc_stock_node_presave($node) {
  if(
uc_product_is_product($node)) {
   
$r  = db_select('uc_product_stock','stock')
      ->
fields('stock')
      ->
condition('sku',$node->model)
      ->
execute()
      ->
fetchAssoc();
    if(
$r) { // wayback: do not update
     
$newmodel = $node->model;
     
$node->model = $node->original->model;
     
drupal_set_message(t('SKU @newmodel in use, not updated',array('@newmodel'=>$newmodel)));
    }
  }
}
?>

would display a message even when the product model is not changed, say when
$node->model == $node->original->model
@rjlang, do you agree?

but it is only a message, no incorrect effects.

@smartango,

Yes, I agree. (And it does that in practice, since our production system uses the patch and I see the msg periodically.)