alternate path integration for D6 version

I explained this in the other issue as well. Adding separate path aliases for the /talk page will add significant overhead and nearly double the size of the already overburdened path alias table. This is the same reason for not aliasing the /edit page in pathauto. There's really no easily scalable way of doing this with the current path alias structure.

sorry to revisit this issue again - feels like a bit of a hornet's nest, but the rationale is not as clear to me as it seems to be for others - and lack of SEO friendly paths is a showstopper for me because my sites are totally search driven. i'd otherwise very much like to use the module.

when you say there is "no easily scalable way of doing this...", are you saying that it is not possible to integrate with pathauto or are you saying it is inadvisable to integrate? i'm a technical nobody, but it sort of sounds like the reasoning is based on theoretical possibilities that problems might/will emerge with path alias structure in a big site that made heavy use of comments on every page.

but what if you have small sites - like 100 pgs - and comments aren't on every page? does this create the same type of problems you are referencing? i assume my path alias tables are tiny compared to how big they get on larger sites, so i'm struggling to see how i could overburden much of anything.

if a different answer might apply to smaller sites with fewer comments, how hard would it be to write the code to do it?

thanks for considering this.

I suppose we could add this feature. It would need some sort of warning on it though, and wouldn't mass-add/delete path aliases upon changing the talk node types, that would have to be done manually. I'll consider this an active feature request, and if someone writes a patch for this, that would be amazing; otherwise, I may or may not get to it.

oh, awesome! that would be great.

i'm not quite sure what you mean by 'wouldn't mass-add/delete upon changing the talk node types'?

what would you guess is involved with writing a patch? in terms of hours, how to do it, ...

no promises, but i will look into getting a patch done.

In the patch, you would want to:

a) Add a setting to integrate with pathauto.
b) If this setting is enabled, add a path alias when a node is inserted or updated.
c) Delete the extra path alias when the node is deleted.

The part that would be hard, and which I advised not doing, would be to mass add/delete path aliases when one changes the talk node types. For example, consider what should happen on a site with 10,000 pages, where the page node type is now set to use talk pages. Ideally, all these nodes would be updated, and their path aliases' would be extended to their talk pages, but that would be dangerous to do on some (most?) sites. A workaround solution would require the nodes to be re-saved before having any changes go into effect.

In terms of hours, if you know what you're doing, you can probably do it in less than 3 hours. If you're not quite sure, it could take around 8 hours to figure everything out. If you need any help in learning to write code or create a patch, I'd be happy to help. Alternatively, I might look into this feature request in more detail in a few weeks or so and write the patch myself. It would be wonderful if you could write the patch though, and would ensure that the code makes it quickly and efficiently into the module. :)

To elaborate, you'll probably want to borrow some code from the path module, and simply modify it to work on talk pages if the node type is talk-enabled. Also, the option shouldn't be available unless the path module is enabled.

ok, excellent additional info. that's very helpful. thank you very much.

i see your point about the mass-add/delete now. doesn't apply to me, but i understand the issue there.

writing a patch is well beyond my skills - even your 8 hrs estimate is probably low by a factor of 10x. best to engage a professional. :)

what i had in mind was to ask a freelance consultant i work with to assist on this. if he can do in 3 hrs or so, that's affordable. 8 hrs not so much. if you are able to coordinate with him, that'd be great, too.

he's very skilled with drupal and i'm sure would generate code that's very clean and works well. obviously we'd test it on my little sites and you'd be able to review/refine. i'm thinking that tuning it for larger sites - the mass-add/delete thing - might be something we leave for you/others, but perhaps you guys can sort that as well.

not sure exactly when he'll be able to look into this. hopefully within the next week or so.

Hi there,
attached is a patch that addresses the issue as you suggested. There is also a draft of testing, but testing in Drupal is too slow to go for TDD and stay (little) under 2 hours for this implementation :)
Tested with (not so) good old point and clicking and looks fine, let me know!
Francesco

fantastic! that was quick.

i will give it a try and post results here.

wasn't able to get it working.

patch command wouldn't apply patch, so i hand patched. hopefully did that correctly.

some of the new mechanisms are visible - e.g., new path alias checkbox on talk config page. but it wouldn't enable via checkbox and save. so i manually changed in talk.module to 'TRUE' to enable. still no joy.

Previous patch was rolled against DRUPAL--1-6 with
# cvs diff -u -N -F^f . | grep -v -e ^\?
Rerolling against DRUPAL--1 with
# cvs diff -up | grep -v -e '^\?'
As per CVS instructions on project page tab.

This looks very good. I'm going to let the test bot run this because it has some nice-looking tests. I noticed a few minor things, though:

+      '#title' => t('Generate an alias version of bla bla'),
+      '#default_value' => variable_get('talk_pathauto', FALSE),
+      '#description' => t('BLA BLA.'),

Mind removing the "bla bla"s? :)

Also, it doesn't work to use variable_get('talk_pathauto') when the variable name is 'talk_pathauto_integration'.

Also, even though you added a setting, you never actually read the value anywhere, so if the path module is enabled, the talk page will be aliased regardless of the settings.

i am not able to get patch to apply. possible doing something wrong in patching process after recent change from windows to ubuntu. following procedure here - http://drupal.org/node/620014 - but this may not apply to ubuntu.

did not try to manually patch the latest version - but sounds like that wouldn't work due to items in #16.

The patch applies fine for me. Are you sure you're (a) working off the development branch, (b) running the patch command from the module's folder? Patching procedure should be the same on ubuntu.

yes, on both counts.

after further investigation, i figured it out. problem relates to permissions within ubuntu /talk directory. i don't have adequate permissions - and don't know how to change it. need to fix that. has been a problem in other instances, as well.

so, i moved /talk folder over to windows machine where i know i have the right permissions. ran patch command. patch applied with no problems. then moved updated module back over to ubuntu. bit cumbersome, but did the job.

even after changing FALSE > TRUE, patch doesn't actually work - but i think that's what you are saying in #16. requires some tweaks.

As English isn't my first language, I'd love if you or John could suggest a title and a description for the patch.

I now actually use the talk_pathauto_integration variable.
In the hook_nodeapi I'm also checking for path module existance, to address the case the talk_pathauto_integration was enabled and the path module was disabled later on (very edge case, but anyway).

how about something like this:

for checkbox line:

Generate aliased paths for talk pages

for description underneath checkbox:

Use pathauto to generate talk page paths based on the existing node alias. If existing path alias is /page-title, talk pages will have path /page-path/talk. If not enabled, default talk module paths will be: /node/xx/talk. Use caution when enabling this feature on large sites as this can significantly increase load on path alias table. Requires pathauto module.

cwgordon7: does this sound about right?

also, cwgordon7, can we add option in module to control the alias used for talk pages? so, for instance, if i wanted my talk pages to be /page-title/comments instead of /page-title/talk.

same idea as you have for the existing options - like renaming the 'talk' tab to be 'comments' tab.

flevour: can you implement this, please? i guess this requires a variable that works like the others on the talk config page.

okay, tried out the patch, including inserting suggested text.

1. suggested description text has a typo. corrected (and shortened) text is below:

Use pathauto to generate talk page paths based on existing node aliases. If existing alias is /page-title, talk pages will be /page-title/talk. If not enabled, talk module paths will be: /node/xx/talk. Use caution when enabling on large sites as it may significantly increase load on path alias table. Requires pathauto.

Checkbox label same as above: Generate aliased paths for talk pages

2. the alias checkbox now seems to save properly. default box setting is unchecked. box remains checked after saving.

3. alias are not being generated. do i need to do something to generate them? tried clearing cache and running cron.

#22: John, open one more issue to keep track of your "control talk' path alias" feature request.

#24: that's exactly something cwgordon is concerned about (see #5 of this issue, "wouldn't mass-add/delete path aliases upon changing the talk node types"). Anyway it would be cool to provide a checkbox for "Bulk update" very similar to pathauto option.

Attached is a patch that takes the suggestions from #21 (headline) and #26 (description).

Cwgordon, what is your opinion on the issue?

This feels like a bad idea to me for reasons charlie outlined in #4.

Also, there is a module - http://drupal.org/project/subpath_alias - which will allow for a scenario:

* node/10 has the alias "blog/greg/this-is-a-blog-post"
* node/10/talk without a new alias automatically becomes "blog/greg/this-is-a-blog-post/talk"

So, that part of this issue is now handled quite well. We do need to consider how to change the alias from "talk" to something else like "comments" but I think custom_url_rewrite/hook_url_rewrite is the right way to do that rather than thousands of aliases.

@a1tsal: did you have a chance to try the suggested sub-path alias module?

as i understand @greggles comment, this module (and required dependency module http://drupal.org/project/url_alter) is a complete replacement for patch flevour has written (#27) and this is his understanding as well. it isn't obvious to me how installing two additional modules to solve this problem which it seems we were very close to solving with ~40 extra lines in flevour's patch is a superior method, but apparently there are technical issues beyond my understanding and these modules solve those problems more comprehensively.

i am now not sure where/how the path gets changed from the default of '/talk' to an alternate, e.g., '/comments', but i believe we are saying that issue have been migrate off this thread to your thread here: http://drupal.org/node/751334. this is fine to move it - as long as there is effort to get it done.

The reason subpath_alias module is technically superior is one of slippery slopes. If we make an exception and create duplicates aliases for talk module then we should also do it for module Signup which creates about 5 sub tabs. So, if we have 6 sub aliases for every node that is a performance and maintenance nightmare.

oh, i think i may see what you are saying.

is this a fair restatement: instead of having talk module (and potentially every other module) implement their own individual subpath solutions, the subpath alias module solves the problem once for all modules that might require it.

and as well, that whereas scalability is stated as the problem at the beginning of the thread, the issue is perhaps more about consistency / consolidation of where/how this is done.

Yes, that is definitely a big part of it. The second part is this: having 5 aliases per node in the url_alias table will start to slow down performance of the site.

ok, very good. i think i see what you are saying. the subpath_alias module just works with the last bit of the path whereas what we were doing re-wrote the entire path, potentially multiple times if there are numerous subpages. i can see where that creates a lot of entries - maybe not for my sites which are small, but in the more general case. let's leave it there.

hopefully a1tsal and i will both get these modules working and we'll have part of the solution.

now, we just need to fix the /talk vs. /comments thing for it to be actually usable.

thank you for this. but it isn't working for me.

well, it is partly working. the path is getting rewritten, but it's taking me to a 'page not found' result.

i've tried doing with both /talk and /comments variations. this produces the path: /page_path/talk (or /comments) as you'd expect, but clicking on the link takes you to the page not found.

btw, i did need to clear cache before the new paths became visible to me at all.

might there be some other step?

fiddled with it some more. not working.

i tried uninstalling everything, reinstalling, clearing cache at each stage. i specifically made sure the patched talk module was replaced with the original.

same result: paths are rewritten (they look beautiful!), but when clicked they take you to page not found.

i tried installing subpath_alias to see if that helped that caused all subpaths to break on my test site. e.g., /page_path/edit also goes to page not found. if it worked, looks like it would be nice.

disabling both modules brings me back to the beginning with paths working again absent the aliases.

@#43
yes, i did remove the php tags. wouldn't actually accept them. makes you remove them, so that's not the problem.

not able to read you other post thoroughly at the moment, but wanted to clear up that one point. will get to it tomorrow.

thank you!

i tried this again today. still not able to make it work.

i am on a localhost test site and there is no caching enabled at all, so that's not a factor.

i cleared cache via pma. machine has also been shut down and restarted in the meantime, so that should clear most any other weird thing.

i tried out just url_alter and subpath alias on another site (no talk module) - the simplest one i've got in terms of fewer modules - but got same result. outbound path is rewritten, but the destination path is not seen to exist.

since subpath alias module does not work in any situation, i suspect the issue is not specific to interaction of talk and url_alter modules, but rather something to do with the new destination url not being written properly at all for anything.

any ideas what would cause that? i will post on url_alter to ask there because, as has been pointed out, this appears not to be about problem with talk module.

http://drupal.org/node/774596

thank you very much for your efforts to help.

i was also surprised that so many people seem to be using url_alter without incident. i assume its working for all of them. why it wouldn't work for me, too, i have no idea.

it sounds like you have a good understanding of the code involved. unfortunately, the solution you describe is well above my pay grade. no skills as a code writer.

i will put this back over to flevour and ask if he can make take a look and fix the problem.

thanks again.

that sounds right.

i put up an issue about this in the url_alter queue. no response so far.

it will be another week or more before flevour is able to take a look at this.