I'm going to implement this for a personal site as a follow-up to #2041987: Create and use an API function to generate a SKU. Basically, if Pathauto is installed on the site, instead of generating the full SKU using the simple preg_replace() you use to strip out invalid characters, I'd like to perform pattern replacement using Pathauto settings. This would let me trim out words, custom handle punctuation, etc. without all of these settings having to be reproduced inside Commerce AutoSKU itself. Given that most sites use Pathauto anyways, this seemed like a reasonable approach to get more flexibility in SKU patterns for free. : )

Comments

rszrama’s picture

Status: Active » Needs review
StatusFileSize
new5.22 KB

This patch depends on the patch in #2041987: Create and use an API function to generate a SKU and adds a setting to defer to the Pathauto settings if the Pathauto module is enabled.

rszrama’s picture

Bah, I screwed up. That patch is from the root of my site repository in Pantheon, not the module root. When the time comes, I can roll a fresh patch.

rbayliss’s picture

Status: Needs review » Needs work

I really like the idea of delegating the cleaning off to another module. I'm afraid it might not make sense to use the pathauto settings all the time though. What do you think about using ctools_cleanstring() instead? It's already a dependency, offers an alter hook for the settings, and it's pretty much the same as pathauto_cleanstring(). The downside is we'd need to implement a settings form on our side, but it seems worth it.

rszrama’s picture

Perhaps a two stage deal? Use ctools_cleanstring() but then use Pathauto if the user wants to alter the settings?

My rationale here was I wanted to establish a particular format for my product display node URLs based on the referenced product's SKU, but I really needed that finer grained control that Pathauto provided. Honestly, I saw a use case here for a more generic module that perhaps Pathauto could depend on in the future but couldn't imagine that happening on D7, so I made this option only appear if Pathauto was present.

If other people want the feature for the same reason as me, it stands to reason they'd have Pathauto. Even if they had ulterior motives, a full half of all D7 sites report using it, though there's no way to know how many users of this module use that. I'm guessing most. From a pure maintenance standpoint, I'd say it makes sense to depend on the existing form until you actually get feedback from folks requesting such support who do not use Pathauto. Your call, but I think copying and maintaining the form wouldn't be worth the effort.

The last submitted patch, 1: 2041997.pathauto_settings.patch, failed testing.

bgilhome’s picture

Issue summary: View changes
StatusFileSize
new5.1 KB

Here's a patch rerolled against current 7.x-1.x.

bgilhome’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: 2041997.pathauto_settings-6.patch, failed testing.