Fully supporting and integrating PathAuto and Token

So the fix to botanik's issue would be

nodehierarchy_token.inc

function nodehierarchy_token_get_hierarchypath($nid) {
  ...
  -// replace the separator with a space, so that pathauto replaces it with the separator again.
  -// a little hacky but prevents the separator from being stripped
  -return str_replace(variable_get('pathauto_separator', '-'), " ", $parent_path);
  +return $parent_path;
  ...
}

On a related note #881270: [bookpath], [menupath], [*path] tokens not cleaned: aliases without punctuation removed, lower casing, etc.

The current fix would be to update to pathauto 6.x-1.x-dev [August 13, 2010 - 01:15] and apply the following patch:
NOTE: Only tested for [fullhierarchypath]

nodehierarchy_token.inc

function nodehierarchy_token_get_fullhierarchypath($node) {
  -return trim(nodehierarchy_token_get_hierarchypath($node->nid) ."/". $node->title, "/");
  +module_load_include('inc', 'pathauto', 'pathauto');
  +return trim(nodehierarchy_token_get_hierarchypath($node->nid) ."/". pathauto_cleanstring($node->title), "/");
}

However I believe a pathauto solution is still being worked on

Not necessarily, everything worked fine in pathauto 1.3
So it was pathauto that broke things unintentionally after a security update

I'd be interested to see what Dave Reid of pathauto has to say

EDIT:
I just realised you were probably talking about the separator issue
In which case, yes fine, fix it in nodehierarchy, i.e. remove the hack

The cleanup issue is one for pathauto IMO

Just for clarity - does anyone have a solution (or patch) that will work against pathauto < 1.4, pathauto 1.4 and, hopefully pathauto > 1.4? (and does anyone have clarity on where this is going in pathauto > 1.4?)

Ok - I think the following does the trick for pathauto 1.3, 1.4 and beyond. It also includes a whitespace and typo correction as incidental cleanup. Please test and confirm that this works for all!

After speaking w/ Ronan am no longer happy w/ this. Dependency on pathauto is bad. The hack was meant to fix a pathauto oddity that is now in pathauto configuration (and warned against in the admin now - stripping the pathauto_separator). Please test this instead of #6.

Ok - after further digging and hair pulling pathauto #881270: [bookpath], [menupath], [*path] tokens not cleaned: aliases without punctuation removed, lower casing, etc. is the crucial issue.

Hybrid patch. This should work in pathauto 1.4 and 1.3.

Above requires an admin configuration change to allow slashes. Shorter/simpler/better version.

@ #8
Hair pulling? I posted the pathauto issue reference and solution in #1

@ #10
This is the same as my proposed fix in #1 (which has been working fine for me since then) with the addition of "module_exists" and "function_exists" checks, so marking RTBC

So Does #10 require that PA be updated to the 6.x-1.x-dev release or should it be able to be used with PA 6.x-1.4? I'm still having the issue after applying the patch for some reason.

Good point #12
This patch RTBC against 6.x-2.x-dev, so bumping the version number up to that
Once it gets committed we can back-port to 6.x-1.x-dev
I have no idea how/if this interacts with 6.x-1.4

@11 - yes - hair pulling, despite the fact you had it right in #1. #8 was my less-than-clear reference to that fact. It was a hectic day and the sequence of patches shows it. In any case, this is now fixed in latest dev.

Great stuff, really happy to see some love for Node Hierarchy happening at the moment

Gonna mark that as 'will not fix'
:)
Thx!

Oh - wait - that was dumb. This is a serious issue - people scanning the queue aren't going to get the context for that change. Sigh.

Applying the patch in #10 broke something... now in 'admin/build/path/pathauto', I get the following error for all the nodehierarchy tokens

The Default path pattern (applies to all node types with blank patterns below) is using the following invalid tokens: [fullhierarchytitlepath].

I couldn't get the patch to apply automatically, so I did it manually, but my syntax appears to be all correct, any ideas?

Thanks

#881270: [bookpath], [menupath], [*path] tokens not cleaned: aliases without punctuation removed, lower casing, etc. has landed.
Time to fix this properly now, yay =D

I'm a little confused by the posts on here... is there going to be a different fix to make Node Hierarchy 6.x-2.x-dev work with PathAuto 1.4? Or is it ok to use #10?

subscribing

Pathauto 6.x-1.5 and Token 6.x-1.15 were just released.
I would advise using these because now the issue has actually been addressed and a final Node Hierarchy patch can be made to move forward.
In short, #10 no longer applies, hence the issue has been reopened.

Trying to get some clarity on this. It sounds like fixes are in place in the latest pathauto and token, but nothing is in place for the 6.x version (either 1.x or 2.x) of nodehierarchy. Is that accurate?

I figured maybe this was rolled into the 2.x dev, but after installing that, my site completely croaked. Any clarity on the current status?

I'd say that is about the sum of it

There is a new page up in the Pathauto documentation that may be helpful: Using path style tokens.

I feel pretty legit in upgrading this to critical. If anyone relies at all on the node hierarchy data to create node path aliases using pathauto (which I'd think that 90% of people do), it makes this unusable.

I believe a new token is needed which complies to pathauto's requirements.

subscribing

Ok, so I've been a little hesitant to get involved with this issue because my plate has been a bit full lately. However, I certainly agree with #27 in that this issue is critical (on so many fronts). For my own sanity (a.k.a. typing out loud) I am going to try to make sense of this chaos and break down the comments into a "summary" list. Please keep in mind I am simply thinking out loud and please feel free to comment!

1. @dgorton in #7: Node hierarchy is not dependent on PathAuto nor Token. We simply support these modules, which came before NH and are used WIDELY. So naturally, we should provide the proper paths based on parent/child relations and the APIs provide by PathAuto and Token. Unfortunately, these modules have undergone tremendous amounts of change recently... sometimes that's unavoidable. PathAuto and Token are, in my opinion, very synonymous when dealing with URLs. We need to keep this in mind and continue providing support for both. Node Hierarchy provides structure, not naming conventions... no need to reinvent the wheel. This is why we support other modules.

2. #955148: 6.x-2.0 Roadmap Simply put, we NEED to separate development from production. This also includes making sure our [production] releases are in sync with supported module [production] releases. We will drive ourselves insane if we try to please everyone when a new dev comes out for any party involved. This is why there is a difference between "development" (not stable) and "releases" (production/stable). Yes there are often moments where there is a bug in the stable version, but they are usually found and fixed quickly because of their priority.

3. @btneil in #26: Using path style tokens states "Token names that end with the word 'path', 'alias', 'url', 'url-brief' (or any of the previous words with '-raw' appended) will be left untouched and not have pathauto_cleanstring() applied to it." I am a bit confused by this, we want PathAuto to leave some of NH's tokens alone, yes? If not, do we need to rename? Like I said, I really haven't dove too deep into this issue, but just from reading the comments and seeing a few commits recently, I feel there seems to be some issue to the naming conventions of the tokens provided by Node Hierarchy. I haven't ever dealt with Token, so I'm flying blind here now. Does anyone have some insight into explaining this further?

4. Include #950882: Cleanup of nodehierarchy_token.inc? Or is this relevant anymore?

5. This topic is something that I have noticed just from my own usage and is strictly hypothetical at the moment. This feature is relevant, but not entirely critical to this issue. I will eventually create a ticket for this, but thought I'd give everyone a heads up:

Automatic aliases (PathAuto). It would be nice to alter how these are generated. For example: place a check box in the "URL path settings" fieldset, alongside the "Automatic Alias" provided by PathAuto, that allows you to dynamically prepend the URL with the parent path. This way if you uncheck the automatic alias, you can set the path of the node... but include the parent's path automatically. Definitely needs some more thought, but in any case, this would definitely impact how tokens are generated. As it stands now, if you manually set the URL, you have to type the entire parent's path as well. Not a very useful, especially for:

6. #811274: Recursive Pathauto for 6-x-2.x and "Add Children Only" This is something that should be considered as well. It seems to me that this feature is also a very intricate part when providing support for PathAuto and Token. When dealing with vast and complex hierarchies, sometimes a top level node can change and it should propagate the appropriate changes as needed based on the many different settings, tokens and parent/child relations. As it stands, it doesn't.

Overall, I guess what I'm trying to say is that I feel we only have "semi" support for PathAuto and Token. We need to dive in and really think this through. I'm not going to lie and say that this doesn't seem like climbing a mountain with truck on your back, but I think these are issues that need to be address or at least recognized. While fixing the immediate issue is required, and should be patched as soon as possible, I think we should take a step back and look at the bigger picture: #955148: 6.x-2.0 Roadmap

Marked #967996: Separator character not replacing spaces as a duplicate of this issue.

3. @btneil in #26: Using path style tokens states "Token names that end with the word 'path', 'alias', 'url', 'url-brief' (or any of the previous words with '-raw' appended) will be left untouched and not have pathauto_cleanstring() applied to it." I am a bit confused by this, we want PathAuto to leave some of NH's tokens alone, yes? If not, do we need to rename? Like I said, I really haven't dove too deep into this issue, but just from reading the comments and seeing a few commits recently, I feel there seems to be some issue to the naming conventions of the tokens provided by Node Hierarchy. I haven't ever dealt with Token, so I'm flying blind here now. Does anyone have some insight into explaining this further?

@markcarver You missed the big section on "...except for tokens with 'path' in the name and who's corresponding value is an array of segments" on http://drupal.org/node/936068

What better than to have someone who understands PathAuto, than Dave :) I was just thinking yesterday that it would be fantastic if we could get some maintainers input, but I didn't want to bug y'all just yet as this IS our isssue lol

@jubamaculele, yes we are aware of this issue and and working to fix this. Sorry this has affected your site. Might I inquire what version of NH you are using? This is also part of the other major issue at hand.

@Dave, I should have explained a little more what I meant by how I was confused. I didn't miss the big section on if the values were arrays. What I was trying to figure out was where this logic took place. In http://drupal.org/node/936068 you say:

Pathauto enforces a couple standards when writing or using 'path' style tokens like 'root/subpage/page'.

Could you reference the function pathauto_clean_token_values() on this help page? After some digging this morning, this function is where the logic takes really takes place and if skimming the page one could infer that pathauto_cleanstring() is it instead.

My confusion was in the naming convention of the tokens itself. I wasn't sure if the token should be 'fullhierarchypath' or 'fullhierarchy-path' and how/if it would affect the logic differently. Your example just shows 'object-path'. Now that I can see it doesn't matter which and we don't need to change token names, we need to focus strictly on how what is returned to the token being used.

I do not have time today to work on this fully because of my day job, but I'm starting to get an idea of what's going on here. Tomorrow is a holiday, so I'll work on getting this fixed. I would like to invite anyone interested to meet in IRC tomorrow if possible to discuss, in real time, the best solution in fixing this.

Ok, from #13 it seems that we had planned on porting pack to 6.x-1.x-dev, but I don't think this ever happened. And once we get this resolved, we will need to also make sure to port this to the 6.x-1.x branch and make a patch release (6.x-1.4) as this is a major issue.

Agreed w/ #35 - this does need to make it back to 6.x-1.x, see also #955148: 6.x-2.0 Roadmap for more discussion on the forward path for all versions.

Re #29.1 - yes #7 happened on a mess of a day - #14 is perhaps a better point in the dialogue.

Thx #26 / @btneil for posting that link - it has been very helpful.

I also realized today (to my horror) that this was still claimed by me. When the issue was reopened I didn't register that it meant my ownership reactivated as well - N00b mistake. So - the involvement of others is most definitely welcome and appreciated (and thanks to all who have been pushing this forward).

Tomorrow isn't super-optimal for me, but I like the idea of getting this moving. Time / place suggestion?

@37 - yes - this is the same problem.

Took some time to get there, but this patch is working in all my test cases. All - please test and give feedback.

Also in my testing discovered a pre-existing bug with hierarchypath: nodehierarchy_token_get_hierarchypath was calling nodehierarchy_token_get_fullhierarchypath with a nid instead of a full node.

Also addressed are jbylsma's token description cleanups - with one more fix - from #950882: Cleanup of nodehierarchy_token.inc

Marked #970508: Hierarchy tokens return non-transliterated strings [hierarchytitlepath-raw], [fullhierarchypath] as a duplicate of this issue.

Awesome, I will test tomorrow when I can get back to my site.

#41 @jubamaculele - what happened when you tried? Were there any error messages? Can you describe the steps you took and what, specifically, didn't work?

Also, it's possible that the nodehierarchy_token.inc from 6.x-2.x works as-is with the 6.x-1.x versions of the module. I haven't checked, but it may be worth a try for anyone else in version 1 who wants to test. Regardless, once this is resolved, the final fix will make it back to 6.x-1.x.

Just to rule out the obvious - can you verify that you're deleting the old aliases first and then checking 'automatic' on the URL aliases when editing (or -alternatively - if you;'re doing this in bulk that you are deleting all and then recreating all from admin/build/path/pathauto?) Can you also verify that you manually applied this patch - it isn't yet in the dev version - just as the patch file in #38.

In terms of the tokens to use, [fullhierarchypath] is the recommended token and probably the one used in the vast majority of places.

subscribing

Tested on a fresh install with 6.x-2.x-dev and all is working well. Thanks dgorton!

I also tried dropping the token.inc into the 6.x-1.x-dev and it was a no go. Got an error when trying to create a node of:

Fatal error: Call to undefined function nodehierarchy_get_node_parent_nids() in /sites/all/modules/nodehierarchy/nodehierarchy_token.inc on line 17

Glad to see this working on 2.x!

I know I shouldn't do this, but in order to get more people testing this (and get it rolled into a release sooner), here's the patched version of the current 6.x-2.x-dev (I realize lots of people don't know how to patch, so I'm hoping this will increase testing). Please download it and see if it's working for you.

Note that all the regular warnings apply: backup your stuff and dont blame me or dgorton if it breaks everything.

#38 seems to be working for me. I have done several days of testing and haven't seen a problem so far.

I have no idea what 5.x-1.x's equivalent would be. I thought I could just copy and paste the patch, but I don't have any 5.x install to test with. After reading the code though, I noticed that several of the tokens call nodehierarchy_get_node_parent_nids() which doesn't exist in 5.x-1.x.

I'm still marking this reviewed and tested by the community so we can wait for a RTBC. Then we can work on the patch (to be ported) for 5.x-1.x.

@49 - I'm definitely not having those results - can you backtrace and see where the cause may be (just in case there's a different module or code snippet in play)?

(For a quick-n-drity method of doing so - see http://www.lullabot.com/articles/quick-and-dirty-debugging)

jubamaculele (#52): This isn't really an error related to this issue. Your php memory limit is being exceeded and, when that happens, Views is often the first one to complain (since it's memory intensive). You could either raise your php memory limit or disable a bunch of modules.

Hope that helps, let's try and not sidetrack on that issue too much...anyone else tested with results?

My vote is to roll this patch into a dev release since it seems to work for anyone who's tried it and haven't had anyone report otherwise.

@jubamaculele: I have to agree with Rob, this isn't related to NH as seems to be a Views issue or at the very least how you might have Views configured or used on your site. Now I'm not saying that this is entirely unrelated as it sounds like you only started getting this after you patched, however there could be numerous reasons why this is happening. As dgorton said in #51, I'm not receiving these results either. Which is why I marked this issue appropriately in #48 and should get this committed.

Now on a side note, just from reading your posts in this thread, it might be that your PathAuto settings are not properly configured (#50) or it could be related to an entirely separate issue (ie: breadcrumbs) for which a new issue could be started.

@Rob_Feature: I think this is actually one part of many catalysts in making a 2.0 release. Please see #955148: 6.x-2.0 Roadmap .

Seems to work great! Thanks!

I've applied the patch in #38 and it's still not working for me. I'm using Pathauto 6.x-1.5, Token 6.x-1.15, and Node Hierarchy 6.x-1.3.

I've got parent nodes, children, and grandchildren and am having an issue at the grandchild level. The content types are Projects, Projects Tier 2 and Project Tier 3.

I'm wondering if the issue I'm having is that the top level parents' URL alias isn't simply "title-raw", it's "projects/[title-raw]". I'm doing that because there's a view on this content that appears as the landing page, before you start drilling down into the 3 levels of hierarchy.

I'm only able to get the Tier 2 url alias working by using "projects/[hierarchyparenttitle-raw]/[title-raw]". That successfully turns a url into something like "/projects/top-level/get-started". On Tier 3 I am using the token "[fullhierarchypath]" and it is resulting in urls like "/projects/top%20level/get%20started/alias%20test%20again".

I'm wondering if in the function "nodehierarchy_token_get_hierarchypath", when it does the if statement:
if ($parent_path != "node/$node->parent") {

it's not matching somehow, since it's more like "projects/node/$node->parent". but, I don't know the correct PHP to use there.

Any ideas? Or am I barking up the wrong tree?

Not positive, but I think dgorton's patch in #38 is for the 2.x-dev branch. He mentioned that its on his priority list for 6.x-1.x and 5.x at #955148-17: 6.x-2.0 Roadmap but I don't think he has had a chance to get to it yet.

#57/58 confirmed: This issue and patch are for 2.x version of the module. Once it's working, he was going to roll it back to 1.x.

ah ok, I didn't realize. the fixes I need where the two patches detailed in #1. I did that and I'm in business. thanks!

Committed to dev now. Thanks all for the hard work.

@ronan - we're still on 1.3 and would like to test this fix. Should we then consider testing an upgrade from 1.3 to 2.x-dev or what is the timeframe for a 1.x-dev patch? We'd like to help out the best way we can.

@btneil - the latest 6.x-1.x-dev should have a version that will work - please test and verify. If it does work, we'll roll a 1.4 with it.

Subscribing.

(Edit: 6.x-1.x-dev seems to fix the issue here...)

@dgorton - I tested 6.x-1.x-dev and can confirm it's working as expected. Thank you for the fix!

@dgorton - any update on a 6.x-1.4 release? I understand you're probably waiting on #955148: 6.x-2.0 Roadmap for a 2.0 stable release, but for those of us still on 1.x rolling a new version soon would be great. I've got 1.3 patched on two sites with no issues.

6.x-1.x-dev fixes this (which is a duplicate of this issue, but better describes what I tested):
#970508: Hierarchy tokens return non-transliterated strings [hierarchytitlepath-raw], [fullhierarchypath]

@dgorton - Any progress on a 1.4 release (or forecast for 2.0)? We'd really prefer not to implement development code on our production sites.

Thanks!

Bumping to "needs review" pending non-dev release.

Yes - we're planning on working on this on the Drupalbus to Drupalcon - http://groups.drupal.org/node/120989

Marking as Fixed as a 6.x-2.0 release has been made. Do not re-open this issue.