Zombie SKUs in stock table after option removal

[bugfolder]

Changes to attributes on a product can leave zombie entries in the stock table with no way to edit or remove them. (Applicable if the uc_stock module is enabled. [EDIT] And you'll need the uc_reports module enabled to see the stock report.)

Remove an option from an attribute

• Create a product (e.g., "Shirt") with an attribute (e.g., "Size") with multiple options (e.g., "Large", "Medium", "Small", "Tiny").
• In the Options tab, checkmark to make them all active for this product.
• In the Adjustments tab, create unique SKUs for each option.
• In the Stock tab, create some stock quantities for each SKU.
• Go back and edit the attribute (at admin/store/products/attributes), edit the option (OPTIONS dropbutton), and remove one of the options (e.g., "Tiny").
• Now go to the stock report at admin/store/reports/stock. You will see the SKU for the removed attribute option, but there is no way to edit the stock values. On the Stock tab for the product (at node/NNN/edit/stock), it's no longer listed with a checkbox.

In principle, this situation could be avoided if store managers were scrupulous about deactivating SKUs from the stock table before removing the corresponding attribute option, but if they delete the option first, then they're hosed and they have to beg the webmaster to manually edit the stock table in the db.

This can happen more easily for attributes that are shared by multiple products; for example, if "Size" is used for both Shirts and Pants products, and then the store manager decides to no longer carry Tiny of anything, but forgets all the products that it's used for and only deactivates some of the SKUs.

One possible solution would be for the stock module to check and delete stock table values for any affected SKUs when an attribute option is deleted, but that seems like it could be dangerous by resulting in unintended deletion of stock information. So perhaps a better solution would be to add validation to the attribute option edit page and fail validation if there are active SKUs that used that option, requiring the store manager to explicitly deactivate each SKU (and therefore giving him/her a chance to say "oops, I forgot that we still need "Tiny" for socks, guess I won't entirely remove it").

Since there are other ways of creating zombie SKUs in the stock table (see next issue), it might also be useful for the stock report to check for and flag zombies.

[argiepiano]

I'm having a hard time reproducing this. I went through all the bullet points above, but when I get to the last, and go to admin/store/reports/stock this page is empty - it doesn't show anything. It may be the case that I need to "buy" some of these products to produce a stock report?

But I did find another issue. After going through the bullets, I decided to add "tiny" again as an option. Then I went to adjustments and added the same sku (tiny-shirt) as I had used before deleting the option. At this point I got:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'tiny-shirt' for key 'uc_product_stock.PRIMARY': UPDATE {uc_product_stock} SET `sku`=:db_update_placeholder_0 WHERE (`sku` = :db_condition_placeholder_0) ; Array ( [:db_update_placeholder_0] => tiny-shirt [:db_condition_placeholder_0] => shirt )

Which may have been caused by that orphan stock entry in the uc_product_stock table.

I do think your second solution is the way to go: "So perhaps a better solution would be to add validation to the attribute option edit page and fail validation if there are active SKUs that used that option"

EDIT:
Even after "buying" some shirts, I don't see a stock report at all. Is there a step missing? Sorry, I currently don't use ubercart and have never used stock.

EDit 2:
OK, you have to enable the Reports module for the steps to work :)

[bugfolder]

PR created.

When we choose to delete an attribute option, we're presented with a confirmation dialog. So what I've done is to alter that dialog to list any future zombies that would be created by the deletion (thereby giving the store manager a chance to cancel the action), and if they do go forward, an additional submit function deletes the future zombies from the stock table.

This should prevent the integrity problem that you encountered (which was indeed due to the existence of the zombie in the stock table after the initial option deletion).

In theory, this check should prevent the appearance of zombies, making the fix in #566 far less likely to be needed. But it's still a useful thing in case there is some other route to zombiehood or, as was the case with my client, the table already contains zombies due to past actions.

To test:

• Create a product with an attribute that has multiple options.
• Give them all unique SKUs and put them into the stock table.
• Go back and delete the option from the attribute. You should be presented with the list of SKUs that will be removed as part of the deletion.
• Proceed with the deletion and check the stock table. No zombies.

[argiepiano]

Here's as couple of questions:

1. Is there any situation when someone may want to keep a record of the stock for an attribute option even after deleting or deactivating that option for a product? This PR doesn't give them a choice - you either keep the option, or delete it together with the stock record
2. If removing everything is the desired behavior, perhaps we should make it clearer that all stock records for those SKUs will be lost. Right now the notice says: "The following SKUs will be removed from the stock table", but that doesn't make it super clear that the stock record for the SKU will be gone forever.

[bugfolder]

For question 1, yes, there is an option for keeping the record of the stock without offering the option for sale: you deactivate the option in the Options sub-tab for the product. (That's what my client should have done when they quit carrying that color of paper.) If you also don't want it to show up in the stock report, you can deactivate it in the stock report as well. In both cases, the stock data is preserved.

For question 2, making it clearer that the stock records will be lost is a good thing. I'll update the PR.

[bugfolder]

Oops, I spoke too quickly. Deactivating the option in the Options sub-tab loses the "Alternate SKU", so it's basically exactly the same zombie-creation as deleting the option from the attribute. Back to "needs work." And therefore, we need a good solution for question 1.