Product Code Review

Alrighty, I've fleshed out the product code from Damien's initial build to the following point, and have developed some standards for variable type / naming proposed elsewhere. I'm interested to hear feedback on what's missing in the API and what sorts of tests should be developed to cover the code here. You can view this code in my latest GitHub commit at http://github.com/rszrama/drupalcommerce (and check my other repo for the installation profile).

Product types

The product type object includes...

->type // The machine-readable type name.
->name // The human-readable name for the product type.
->description // A description used on the admin overview / product add page.
->help // Help text for the top of the product add form.

I've standardized the use of $product_type as the variable name for an entire product type object and $type or 'type' when referring to just the machine-readable type name, i.e. $product_type->type or...

foreach (commerce_product_types() as $type => $product_type) {
// ...

Product types should always be objects, not associative arrays. It was something of a hybrid system (like the comment system in core used to be / maybe still is : ?), so I standardized on objects.

The product type API includes the following functions:

  • commerce_product_types() - returns an array of all defined product types.
  • commerce_product_type_get_name($type = NULL) - returns the human-readable name given the machine-readable $type; if $type remains NULL, an array is returned of all types / names. Used for tasks like defining menu items / permissions and populating select lists.
  • commerce_product_types_reset() - resets the array of product types cached by commerce_product_types().
  • commerce_product_type_load($type) - returns the fully loaded product type object for the given type.
  • commerce_product_type_to_arg($type) - returns the path argument for a product type... is this used?
  • commerce_product_type_save($product_type) - saves a product type, performing an INSERT or UPDATE based on whether or not $product_type->is_new is set.
  • commerce_product_type_delete($type) - deletes a product type.
  • commerce_product_type_title($product_type) - returns the human-readable product type name given a full product type object (used as a menu item callback).


The product object includes...

->product_id // Serial numeric ID.
->sku // Merchant defined, human-readable unique ID.
->type // The machine-readable type name.
->title // Human-readable title of the product.
->uid // User ID of the product's creator.
->created // Creation timestamp.
->changed // Last changed timestamp.

The product API includes the following functions:

  • commerce_product_load($product_id) - returns a product object loaded by product ID.
  • commerce_product_load_by_sku($sku) - returns a product object loaded by SKU.
  • commerce_product_load_multiple(...) - loads multiple products through the entity system based on the supplied parameters; the above two functions are really just wrappers for this one.
  • commerce_product_save($product) - given a full product object, saves it to the database within a single transaction; performs an INSERT or UPDATE based on the existence of a $product->product_id.
  • commerce_product_delete($product_id) - deletes a product by product ID.
  • commerce_product_delete_multiple($product_ids) - deletes all the products specified by ID in the $product_ids array; the above function is a wrapper for this one.
  • commerce_product_path($product) - currently returns the path to the edit form for a product; used by the entity system.
  • commerce_product_autocomplete(...) - returns the autocomplete output for product autocompletes; matches based on SKU or title within a given set of product types.
  • commerce_product_match_products(...) - returns an array of product data for products matching the given parameters; used by the autocomplete and other product reference related functions.

Core / contrib integration

Products are handled through Drupal's entity system and are therefore integrated into the fields system. Product data is also exposed to Views in the various includes/views files. A base set of product tokens have been defined in includes/commerce_product.tokens.inc.

The product UI module provides a default View listing out created products with a contextual link to add a product. The View has a local task tab for administering product types and provides the forms necessary to add/edit and delete product types and products. The product reference module provides a Field to refer to products from another entity instance, like a node of a type used for product display. Default formatters for the Field include listing titles and SKUs with or without a link to view the product (which does not exist yet). I'm working locally with a mock-up Add to Cart form formatter.

Current permissions include:

  • Administer products
  • Administer product types
  • Access lists of products (primarily for viewing / referencing)
  • Create products
  • Product type specific permissions for creating and editing/deleting products (both your own and any)

Current proposals for improvement:

  1. The ability to define SKU patterns based on Tokens at the product type level, based on this thread. Problems with the implementation involve checking the validity (i.e. the uniqueness) of the SKU in the product add form's validate stage as opposed to its submission stage. We can't replace the product ID token prior to submission when we actually create the product. However, we can't simply store the pattern as the SKU, because once the SKU has been set we don't want it changing at a later time. I first thought this might not be an issue, since the only Tokens I'd consider allowing would be product ID and product type, which themselves won't change. However, if the concept of SKU is to cover multi-site instances of the same product, we can't presume the product ID will be the same. We need to save the full SKU at the moment the product is created. My proposed solution is to validate based on the next supposed product ID, which even if the product ID ends up being different when the product is actually saved, it will at least still be unique.
  2. The addition of initialization functions for creating new products / product types, based on this thread. Defining classes for products and product types, or at least particularly any object that is governed by Drupal's entity system, was ruled out. However, I asked whether or not it would be beneficial to have initialization functions for creating new products / product types and am still interested in feedback. It seems essential to me, but I didn't want to just add it in without anyone else feeling they were beneficial.
  3. The code currently doesn't provide any points to hook into processes like loaded, saving, or deleting a product. Should this sort of hook API be built, and do we need to consider adding in alteration hooks?
  4. Add a product view and use this in places like the product delete form to show the product that's being deleted.
Ryan Szrama
Posted: Feb 14, 2010


Bojhan Somers (not verified) on February 14, 2010

"with a contextual link to add a product" - I think you mean an action link here? Otherwise I am not sure how to envision that working.

I am intrested in how the "product reference" module it fields would look.

Good write up, clear and concise :)

redben on February 14, 2010

Most of the following suggestions are based on a review of node module and comparing it to commerce_product
Best practices
- implement hook_admin_paths http://api.drupal.org/api/function/hook_admin_paths/7
- may be product paths should not begin at /admin but just product/*/edit ? I think that website administration and store/catalogue administration are two distinct concepts.
- Automatic SKU's should be left out to contrib (the "core" philosophy) something like auto_product_sku (just like autonodetitle)
- Since not everybody likes SKU term as Product Reference/Number, product types should have a property product_type->sku_label that lets the user change the sku label to something meaningful to his/her business (Product code, ISBN or whatever). Just like the node_type->title_label
Code initialization function, product_type_create() would be a helpfull function. Node module has node_type_set_defaults() may be products can use a similar function.
- Product type classification : Taxonomy could be used to classify products types. If you have a store
that sells many different product types, their list could be hard to browse. A tree style navigation using taxonomy can help for that.
Add "view mode" "reference" in addition to full on commerce_product_entity_info. It could be used by default display formatter for product reference field and would allow for easy theming of product reference fields. If you think about it product reference is one way of viewing a product, so view-mode "reference" makes sense to me.

PS: a commerce_product_reference function seems to have stayed on commerce_product.module : commerce_product_reference_menu()

Ryan Ryan Szrama on February 17, 2010

Excellent feedback! I've implemented hook_admin_paths() and fixed that function name to be commerce_product_menu(). Not sure about where the product creation should be, because the initial specification was for products to be something only administrators see / manage. In other words, listing a product at admin/commerce/products is different from showing it to the end user with an Add to Cart form. Will think about that one.

And you're probably right on the automatic SKU creation... perhaps a contributed module could focus specifically on ways to enhance product entry, from automatic SKUs to simplified listing of multiple products at once.

svendecabooter on February 16, 2010

Couldn't you replace the function commerce_product_types_reset() with a $reset parameter in commerce_product_types($reset = FALSE).
I think that would make more sense.

APIs to hook into various actions associated with products (load, save, ...) seems like a must-have to me. Although i'm not sure if that would be duplicating the hook_entity_insert, hook_entity_load, etc... hooks. I assume those are more high level, but haven't dugg too deep into them to know what I'm talking about :)

Ryan Ryan Szrama on February 17, 2010

Chatted with DamZ about these today, the regarding the first, core is actually moving away from the $reset parameter in favor of *_reset() functions (it's still a little inconsistent). Conceptually, resetting is something separate from retrieving the array, so we'll have 1 function = 1 purpose as much as possible.

As for the hooks, I wasn't aware of the entity_*() hooks, but they're a little incomplete (no delete hook, for instance). We'll still go for a specific commerce_product_*() hook, though of course the entity hook could still be employed. This will just be a little more specific, like hook_form_alter() vs. hook_form_FORM_ID_alter().

Ryan Ryan Szrama on February 18, 2010

I've added a sheet to the Google Doc linked below to track hooks as well as other common API functions pertaining to various Commerce objects. Committing product type hooks now...

bendiy on March 9, 2010

Can someone further define the Product Type field?

I'm working on adding additional tokens, so I'm wondering if some tokens for tree categorization might be useful.

Is the Product Type only used to categories products (e.g. car, truck, boat) and then control some actions for that type (e.g. cars and trucks have wheels, boats do not)?

From my experience, one field for categorization is never enough. It makes it hard to get a tree of categories like: cars = sedan, two door, wagon, etc.; trucks = 1 ton, extended cab, SUV, semi, etc.; boats = fishing, speed, pontoon, yacht, etc. I'm assuming that categorization will be accomplished with additional custom fields.

Can a Product Type be changed on an existing product?

Looking at the code, a Product Type is hard coded as a string. This means that you cannot change the Product Type name and have all associated products update automatically. For example, I started out calling my Product Type, "Cars", but I now want to call it "Automobiles". Can you this be changed without ill effects?


Ryan Ryan Szrama on March 9, 2010

Product type is to products as Node type is to nodes. It isn't supposed to be changed and you'll use different types to have different fields on products. Certain fields will then be exposed through add to cart forms as a product selection mechanism.