If you go to admin/structure/types and change the machine name of any of the product types, it breaks Ubercart. The {uc_product_classes} table is not synchronize properly. This leads to error messages like "Notice: Undefined property: stdClass::$price in uc_product_uc_cart_display()".

At least part of the problem is in the implementation of hook_node_type_update() in uc_product.module. The code is as follows:

/**
 * Implements hook_node_type_update().
 *
 * Ensure product class names and descriptions are synchronised if the
 * content type is updated.
 */
function uc_product_node_type_update($type) {
  db_update('uc_product_classes')
    ->fields(array(
      'name' => $type->name,
      'description' => $type->description,
    ))
    ->condition('pcid', $type->type)
    ->execute();
}

This function doesn't account for $type->type changing. If it does change, then $type->old_type is set and is different than $type->type.

So, the function needs to check {uc_product_classes} to see if there is an entry where $type->old_type == $record->pcid. If so, it needs to delete the old record and create a new one with the pcid field set to $type->type.

CommentFileSizeAuthor
#7 1831900-pcid-update.patch866 byteslongwave
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dan Z’s picture

Dan Z CreditAttribution: Dan Z commented

If anyone has decent documentation of the hook_node_type_update() input, please point me to it, and I might be able to patch this. http://api.drupal.org/api/drupal/modules!node!node.api.php/function/hook... doesn't provide enough information.

TR’s picture

TR CreditAttribution: TR commented
Category: bug » feature

I'm pretty sure this is a duplicate of #525612: Allow any node type to become a product.
Ubercart currently requires products to be of the Product content type, with or without added fields.

Dan Z’s picture

Dan Z CreditAttribution: Dan Z commented
Title: Changing product node type breaks Ubercart » Changing product node type breaks Ubercart (Notice: Undefined property: stdClass::$price in uc_product_uc_cart_display())
Category: feature » bug

The error message pops up when viewing any product node, and the price doesn't show up.

Updating the {uc_product_classes} manually to synchronize it doesn't seem to help. Doing this causes nasty error messages like:

"Fatal error: Unsupported operand types in /var/www/html/drupal_dev/sites/all/modules/ubercart/uc_product_kit/uc_product_kit.module on line 661
Uncaught exception thrown in session handler.
Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 28618086 bytes) in /var/www/html/drupal_dev/includes/bootstrap.inc on line 1572 "

Actually, I'm not sure of the purpose of {uc_product_classes}. It duplicates information in the {node_type} table without adding anything new. It seems redundant. Why not just use {node_type}?

TR’s picture

TR CreditAttribution: TR commented

{node_type} would be appropriate if and only if any node type could become a product. However, that's not the case. Product types must be "descended" from the the Product type, by adding fields.

Dan Z’s picture

Dan Z CreditAttribution: Dan Z commented
Version: 7.x-3.2 » 7.x-3.x-dev
Assigned: Unassigned » Dan Z

No, this is not a duplicate of #525612: Allow any node type to become a product. This is clearly a bug. A table is not getting updated. I'll give an example:

I add a product type using the usual Ubercart interface at admin/store/products/classes.

There is now a record in {node_type} with the following fields (among others):
type: book
name: Book
base: uc_product
module: uc_product
description: A book you can read
orig_type: book

This corresponds to a record in {uc_product_classes} containing only the following three fields:
pcid: book
name: Book
description: A book you can read

Now, say I update the {node_type} record via admin/structure/types. This does NOT change the "base" or "module" fields, so it's still a product node type. It might look like the following:
type: fred_book
name: Fred Book
base: uc_product
module: uc_product
description: Life of Fred book
orig_type: book

But, because of the bug in uc_product_node_type_update(), the {uc_product_classes} record does not change. It remains:
pcid: book
name: Book
description: A book you can read

The {uc_product_classes} record SHOULD change to:
pcid: fred_book
name: Fred Book
description: Life of Fred book

In other news, the error messages from #3 went away on their own. The manual update of {uc_product_classes} appears to have done the trick. I'm going to assume that this was a caching issue.

As such, it's a small fix, and I'll try to work out a patch on my own. It's time I got Ubercart into GIT and figured out patching from there, anyway.

Dan Z’s picture

Dan Z CreditAttribution: Dan Z commented

{node_type} would be appropriate if and only if any node type could become a product. However, that's not the case. Product types must be "descended" from the the Product type, by adding fields.

It's not directly relevant to fixing the current issue, but this got me thinking.

There is no information in {uc_product_classes} that is not already in {node_type}. All the fields are duplicates. I assume that this means that Ubercart uses the presence of a pcid in that table to determine if a node is a product node, or to get a list of product node types, right?

Just something to think about for future architectures: Could the same things be done by examining the base and/or module fields in {node_type}? I don't know, but those two fields are "uc_product" in all my product types.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Active » Needs review
FileSize
1831900-pcid-update.patch866 bytes

The attached patch seems to fix this for me, although on my test site I need to explicitly clear cache for the new machine name to show up at /admin/structure/types.

Yes, {uc_product_classes} is redundant but it's not worth changing this now, instead it would be better to implement #525612: Allow any node type to become a product and make it easier to attach product fields to any content type.

Dan Z’s picture

Dan Z CreditAttribution: Dan Z commented
Status: Needs review » Reviewed & tested by the community

Tested. Works great for fixing {uc_product_classes}.

The issue with a manual cache clear requirement is a Drupal problem, not an Ubercart problem. I reported it at #1832640: Changing node content type machine name fails until manual cache reset.

There's a separate problem I reported at #1832724: PDOException on duplicate node type machine name. I think that this is a Drupal issue, but have a look and see if there's anything in Ubercart that could possibly have caused this.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave commented
Status: Reviewed & tested by the community » Fixed

Committed #7. I think both of the issues listed in #8 are core bugs, Ubercart is just responding to hooks here and not enforcing any of these changes by itself.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

Added error message.