Right now, you use a constant to define the replacement as an empty string. This indiscriminately gets applied to any character in a token replacement. In my usage of this module, I'd prefer to actually use a hyphen in the place of spaces instead of just removing them, and because of my limited application, I could easily just change this replacement string to '-'. However, I don't see why this module shouldn't just follow Pathauto in allowing the replacement string to be defined in the settings form along with the SKU Case.

CommentFileSizeAuthor
#1 do_not_hardcode_the-2041991-1.patch6.6 KBrbayliss
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rbayliss’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
6.6 KB

Here's a first pass at this.

mibfire’s picture

If i dont set anything i get this error:

Notice: Undefined index: replacement in commerce_autosku_commerce_product_presave() (line 80 of /commerce_autosku/commerce_autosku.commerce.inc).

mibfire’s picture

I also get an error when i disable and enabled this module:

Fatal error: Unsupported operand types in commerce_autosku/commerce_autosku.module on line 316
function _commerce_autosku_object_factory($schema, $data) {
  $pattern = _ctools_export_unpack_object($schema, $data);
  $pattern->advanced += _commerce_autosku_default_settings();
  return $pattern;
}
mibfire’s picture

Priority: Normal » Major
SocialNicheGuru’s picture

Status: Needs review » Needs work

The patch doesn't check if the field "Automatically Generate SKU" is set or set good defaults.

Otherwise "$pattern->advanced" is null or not defined at all throwing the exception in #3.