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

Comments

rbayliss’s picture

rbayliss commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new do_not_hardcode_the-2041991-1.patch6.6 KB

Here's a first pass at this.

mibfire’s picture

mibfire commented

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

mibfire commented

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

mibfire commented
Priority: Normal » Major
socialnicheguru’s picture

socialnicheguru commented
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.