Validate SKU formation to disallow commas, trim spaces

Tagging. This is an unexpected byproduct of removing the SKU validate function a while ago.

Also, validating SKUs to not include commas is easy enough. We also need to add an update function to perhaps replace commas in any existing SKUs with hyphens.

I support this change. However, since SKUs may be determined by an external system, it's a little iffy. And changing SKUs in an update seems questionable too.

But perhaps we should just bit the bullet and do it. It will definitely be a WTF for those few SKU users who have commas.

May I hijack this just a bit? I think it would be better to be about SKU validation in general.

I just discovered a case where a product was input with a trailing space after the SKU... and that trailing space followed it everywhere.

I think we should make a set of rules (trim spaces, remove commas, what else?) and implement that.

Oof, bummer. I suppose this is what happens when you hastily remove validation on something. : P

A while back, we limited it to only alphanumerics, underscores, and hyphens. This wasn't international friendly, so we just decided to remove validation altogether. We definitely need to disallow commas and trim spaces, I'll see if anything else comes to mind.

Just talking this over with rfay in IRC, I think what we'll do is trim on a core update function and provide a utility module for anyone needing to strip commas out of existing SKUs. Having a comma in a SKU doesn't actually break anything inherent to the product... just the product autocomplete. It's feasible that a site could function just fine with commas in their SKUs by using a select list widget on those reference fields.

However, as of 1.1 we will be using a product autocomplete to manually add products to orders, and there's no way to use a different widget there. I think the best solution as recommended by rfay is going to be a utility module that provides a very basic form for updating SKUs all at once to either remove commas or convert them to some other character. Putting this in an update function is just a bit too intrusive, though, since you'd have no way to specify what they should be changed to, and people may need to update import type scripts before proceeding.

Right, we'll still validate it so you can't use commas in the UI going forward. I'm just not going to correct existing sites in an update function; we'll leave that up to a helper module where users can control when and how they convert their SKUs. I'm not sure what commas have to do with the Add to Cart form, though, as that only knows about product IDs, not SKUs.

Actually, I'm not sure I am going to put the trim() in a core update function for the same reason as commas - someone may need to update their integration code in conjunction with the update. Let's put that into Commerce SKU Update as well.

Hrm... weird, nothing funny about the patch afaik, and git diff has generated fine patches on all my other issues. :-/

Just FYI, the git apply is shown on the details page, http://qa.drupal.org/pifr/test/191324. The problem was that this issue was set to 7.x-1.0, not 7.x-1.x.

#11: 1285360.no_comma_skus.patch queued for re-testing.

Ahh, great. Thanks, Randy. I'll be sure to check that in the future.

One way to help people update their SKUs (if they want to) is to just tell them the SQL query:

UPDATE commerce_product SET sku=TRIM(REPLACE(sku, ',', ''));

Another way is to wrap it in a module; another way is to post it as a script.

Alrighty, committing number #11, and talked the module idea over with Randy. Instead of a module, we're just going to supply a SQL query here that folks can refer to if they need to update their SKUs to remove whitespace / replace or remove commas. Marking the bug report as fixed, but leaving this tab open to put a query in later unless someone else gets to it first.

Commit: http://drupalcode.org/project/commerce.git/commitdiff/08e0e54

EDIT: looks like Randy went back in time and beat me. : )

I added an issue summary - feel free to edit it. It contains the fix query.

Hi

I know this is an old closed issue, however I came across it through google when I was looking for a quick way to edit multiple skus removing /s (slashes) which had been added into a large range of sku's in commerce.

The sql is simple and worked great, but a nasty side effect is the product displays that contained the product references to the slashed skus will now no longer add to basket.

I believe the solution is you need to run the same update on commerce_product_revisions too, if you don't do this also sorts of side effects start happening!

Anyone else considering this update sql, beware!

Added issue summary

Hi,

unfortunately commas are not allowed anymore inside the sku.

I'm converting a Magento webshop to Drupal Commerce and I have noticed that both in Magento and in SAP B1 (ERP-system), commas are allowed inside the sku.

Now I need to clean the sku on import (by replacing the comma by a point) and I'm storing the real sku inside a seperate textfield...

:-(