Pathauto for Drupal 7

I think user_cancel_reassign should delete the alias. Otherwise, looks good to me.

Great progress! Congrats.

Sure, please go for sweeping changes in that area. It makes a ton of sense. We can also change that in Drupal 7 core right now, so if you bump up against limitations of the token module...voice them here and I'll do my best to help.

Yeah, we can just always use the unsanitized tokens. Users should not see any options related to sanitized/not, and pathauto should choose the not-sanitized always.

Also, I created #609810: Create "token" component in core issue queue so we can better track token issues in core.

Great news and great work!

• Finish porting Forum module related stuff. I simply am not that familiar with the forum module.

I know a little about that based on the work I've done here. Hopefully we can get it close.

• Use node_load_multiple anywhere where there is still a loop with node_loads.

The only plaace I know of is bulk generate.

• Do we need to implement hook_menu_link_insert(), update(), delete() ?

Maybe, but let's leave that for later.

• Any Triggers and Actions related functionality.

Getting these to work should be a part of this issue, but not enhancing them.

• Update API.txt language and convert that to a pathauto.api.php as per the new recommendation.

Yes.

• Chained Tokens: Deal with chained tokens where they are needed (ex. [node:author:name]).

What does token need to do to handle those specifically? I thought they would "just work."

• Upgrade Path.

Which could also be a separate issue. I'd like to get this out soon so people can start testing. Then we can work on upgrade path.

• Use Batch API for bulk updates and do away with the # of aliases per batch restrictions.?

Let's keep this separate, so it can easily be backported if we end up doing it.

Ah, for the UI? There's been a lot of discussion on this and I'm not sure that there's a great solution yet. My feeling is that we should do two things:

1. Provide help text under each set of boxes that provides a list of the most popular relevant tokens for that area.
2. Provide a link in there that goes to a page with all of the available tokens on the site.

It may be easier to say the second part than do it... ;)

Regarding branches - yes, I plan to make a DRUPAL-6--2 branch soon and then commit this code to HEAD and we'll make HEAD the 7.x code. While I don't like it too much, there are a lot of people using 6.x-2.x in production and we can't leave them stranded.

Subscriben - ok point taken (below), my reason for subscribing is to help improve the UI for Pathauto in D7. I know I can at help out in this area and will be testing for usability and accessibility of this module and hopefully will even have time for some patches.

Joshua mentioned that this patch doesn't apply - I had applied several patches to pathauto for 6.x-2.x around the time you were rolling this, so it is likely those will need to be merged in here.

Also, anyone who subscribes to this issue without finding something constructive to say in their comment will get a -1 next to their name in the karma-book I keep in my brain :p I realize it's a module lots of people care about, but really there is a way to fix this and it's not to post a subscribe comment.

This is a straight upgrade and not a redesign. There are other issues in the queue for UX improvements (see this list).

I had a bit of time at work today dedicated to helping with this, and expect to have at least a bit more either tomorrow or Monday. @te-brian, are you continuing to work on it also? I want to see if there's something I can help out with that doesn't overlap.

In the meantime, I've posted reviews and updated patches for #609118: node.tokens.inc: $node->body issues and #610022: Taxonomy Tokens: Fix old queries, so hopefully those can get in soon.

I've also spent a fair amount of time looking through the patch above and just trying to get up to speed. It looks like it's very much on track to me. The only things that I noticed while reviewing were pretty minor, but listing them here:

+  if (($data = $result->fetchObject()) !== FALSE) {

Seems like if ($data = $result->fetchObject()) would work just as well here?

+Drupal.behaviors.pathauto = {

+  attach: function(context, settings) {

+    (function ($) {

+      if ($("#edit-pathauto-perform-alias", context).size() && $("#edit-pathauto-perform-alias", context).attr("checked")) {
...............
+    })(jQuery);

I am not even remotely good at jQuery so this comment may be totally wrong :) But I'm pretty sure that the function ($) and (jQuery) stuff goes outside of the Drupal.behaviors part rather than inside. It also seems like this is a case where jQuery.once() would be used - something like $('#edit-pathauto-perform-alias', context).once('pathauto', function () { and then checking $(this).size() and $(this).attr() inside that.

function pathauto_admin_settings()
function pathauto_admin_delete()

Do to (very) recent changes in D7, these form definition functions now need to at least take a $form parameter as input and then make sure to add on to that rather than setting $form = array() internally.

OK, here is a new patch:

1. Fixed the issues listed in #19.
2. In doing so, found that the JavaScript didn't work at all in D7 (for a variety of reasons), so fixed that - should be working now.
3. Also found that there were a few issues with the node form (URL alias checkbox not staying checked when the node was reedited, URL aliases not always getting saved correctly when the node was reedited, etc). Most of these were due to changes in the structure of $node in D7 and what is contained within it at various points. Those should be fixed now.
4. Fixed some broken URLs.

New (relatively minor) issue to point out:
1. When creating a new node, the summary info in the vertical tabs says "no alias" - Pathauto should ideally change that to at least read something like "automatically generated" so it's telling the truth...

I started looking at the forum stuff, but unfortunately did not have enough time to actually make it work. I'll hopefully have a bit more time on Monday to help with this, though, and can look at it then if it hasn't fixed itself by that point :)

Here's an updated patch:

1. This should get the taxonomy and forums both working correctly (at least based on my light testing).
2. It also makes it so that the module's SimpleTests will pass in D7.

I probably don't have much more time to give to this, but it seems like it's pretty far along. In terms of the remaining TODOs, I think most of the TODOs in #8 are still relevant (with the exception of the forum module stuff), as well as the minor issue I mentioned in #22. I don't know which issues on that list should still be considered critical, though.

A) Only tokens can be used, not arbitrarily supplied placeholders

I'll admit I haven't looked at the code, but...does this mean that people can't do something like "blog/[title-raw]" ?

@te-brian, regarding your first question, I'm not sure I understand the problem (but then again, I'm not that familiar with exactly how Pathauto worked in D6).... As far as I can see, in D6 the module also hardcodes the object that gets passed along for token generation - in that case, it gets passed to the pathauto_get_placeholders() function. And in D7, it's possible for modules to affect the list of allowed tokens by using hook_token_info_alter() and hook_tokens() to define their own additions to the list, which it looks to me like Pathauto will pick up. So is there a more specific scenario that you are worried about? (I haven't looked that carefully, but I don't see any major differences in the UI, and things like blog/[node:title] are certainly still possible.)

Regarding the internal helper functions, certainly from my point of view the way you did it is perfectly fine! Overall, the code looks very good to me.

subscribe!

Subscribe... and thanks for your work!!! :-)

Adding tag...

Subscribing...

Subscribing. Thank you for your work on this, but: I can't get any of the patches to apply to any version of pathauto 6.x-2.x-dev. Is it possible the patches need to be rerolled based on changes to 6.x-2.x ?

Thanks

Subscribing & waiting for 7.x release. Thank you for your work

I encounted some problems using the patch (maybe conflict of versions of pathauto). Could somebody post here not the patch, but a package of the files already patched (or the whole module already patched and repacked)? PLEASE!!!

The hunk failures were caused by #647812: Update to code required for Path_Redirect compatability which got applied after this was created.

This is now committed http://drupal.org/cvs?commit=326580 I created a Pathauto DRUPAL-6--2 branch from HEAD just before committing it in case we want to do anything with that, but I think it will most likely sit dormant.

The simpletests didn't work for me, but I think that's more about my environment than actual problems with the tests.

I'm going to leave this issue open for now so that we can try to address some of te-brian's items from comment #8 (with the exception of those in #9 that we shouldn't test).

I'll create a release tomorrow or so - I'm waiting on advice from dww about how to do it so that all issues marked 6.x-2.x are changed to 7.x-1.x.

WHOOPS: I forgot the most important part of my comment!

A huge thank you to te-brian and David_Rothstein and all the other participants who provided feedback. Great work on this, everyone!

@#11 greggles

Ah, for the UI? There's been a lot of discussion on this and I'm not sure that there's a great solution yet. My feeling is that we should do two things:

1. Provide help text under each set of boxes that provides a list of the most popular relevant tokens for that area.
2. Provide a link in there that goes to a page with all of the available tokens on the site.
   It may be easier to say the second part than do it... ;)

- would you provide links to discussions?
- the one I know #514990: Add a UI for browsing tokens

@12 te-brian

The tricky thing is that not all tokens are available for all items due to the $data objects that are available to be passed in, but it would be easy enough to make a page callback that displays all the tokens for one "type" (or two types in the case of "term and "vocabulary").

- the separated page sounds like a good first approach
- but
-- it will be madness unless using kind of advanced_help popup to allow the user to see both, the textfield where he/she is gonna enter the path and the available tokens at once
-- would the separated page be explicitly titled to clarify what does that list applies for? (e.g. "Available tokens for: taxonomy term path") since the module might select every available node/term tokens and filter out some of them knowing they won't apply (e.g. tokens providing URLs or huge data, like [node:body], [site:url], [site:login-url])

@#15 Jeff Burnz

my reason for subscribing is to help improve the UI for Pathauto in D7. I know I can at help out in this area and will be testing for usability and accessibility of this module

- a fieldset into vertical tabs (intentionally) stops being collapsible
- can you suggest another UI approach?

the first approach I toke for token UI was:
- select the tokens according to the $object (e.g. comment)
- prepare a table of: Token / Name / Description
-- chaining options are only displayed one level deep, like [comment:created:*], [comment:node:*], [comment:parent:*]
-- the chaining fragment being shown in bold i.e. [comment:<b>node:*</b>]
-- the chaining description being prepended with "chains:", e.g. "chains: The node comment was posted to."
- immediately bellow continue with another table of: Token chains / Name / Description
-- chaining options are displayed like ":node:title" if they have no more chaining options
-- a nested chain is displayed like ":node:author:*" with its description being prepended with a "chains:" prefix
-- chains of chains continue in the same table
-- more than one leve deep chains are displayed like: *:author:created:* with description "chains: The date the user account was created."
- until now there is the problem of listing chains multiple time for different chain suffix
-- i.e. node:created:* chains to a date, its chaining variants are *:created:short
-- but node:changed:* chains to a date as well, and its chaining variants are *:changed:short

tokens_UI.jpg (the snapshot attached to #514990-20)

PS: this first approach can be experienced with comment_subject-7.x-2.0-alpha1 (only fetching one nested level of chains)

@arhak: See #273104-11: Using "progressive disclosure" for alias patterns for the UI direction I'd like to take Pathauto for D7 in that'd also help control that the user see relevant tokens and not the giant list of them all.

in that'd also help control that the user see relevant tokens and not the giant list

that was discussed in #514990 but didn't see any conclusion

See #273104-11 for the UI direction I'd like to take Pathauto for D7

I'm talking about what would you like into the "Token browser stuff goes here" spot
for instance, comparing to proposals at #514990 what would you prefer?

where should I follow the topic?
- in this issue?
- don't know if the title of #273104 suits for this "token browser" stuff as well
- issue #514990 seems stuck and not necessary owned by pathauto
- should we open a new pathauto issue for "token browser"?

I'm interested on knowing what would be an approved UI approach
I'm thinking preferably with no fancy JavaScript stuff, since it will be needed to degrade anyway, so I would prefer to know how the degraded version would look like, and from there what enhancements can get

Now that the module is ported, can we mark this as fixed and open new issues for all the follow-ups so they're easier to track?

@te-brian: Can you please help create new issues for all the follow-ups that you identified?

And since we released 7.x-1.0-alpha1, I'm marking this as fixed. Also see #43 and #46. :)