| Project: | Drupal.org customizations |
| Version: | 6.x-3.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | sandbox projects |
Issue Summary
Since project.module is managing path aliases for project nodes, I think it's somewhat responsible for dealing with link rot. Right now, there's nothing to help people who change shortnames to get a path redirect from the old name to the new one. Once #986718: Add support for sandbox projects lands, this is going to be a lot more common. Once a sandbox is promoted to a full project, the shortname is going to be changed from a numeric NID to a human-readable short-name. At that point, it'd be nice to either manage a redirect ourself, or at least notice if path_redirect is installed and add a redirect that way. Maybe this needs a little checkbox that appears if you're changing the shortname about "Add automatic redirect from [existing/path] to the new short name"?
Anyway, I didn't want to complicate #986718 any further with this, but I wanted to open an issue to discuss if/what we should do about this.
Comments
#1
Tagging for Git Sprint 9, not needed for git-dev
#2
Moving to Sprint 10. I don't believe this will be touched during Sprint 9.
#3
While it does make sense to have this in Project, the actual handling for it needs to be in drupalorg_git_gateway, as project itself is no longer responsible for generating the sandbox path aliases (it can't be, they rely on a piece of drupalorg-specific data), so we'll also need to fix it over there. That does, however, relieve the pressure on project to do this for the time being, and takes it out of scope for the git migration, so untagging.
#4
dww marked #1139046: Automatically create redirects when sandbox projects are promoted as a duplicate of this issue. So, if we're using $node->path to update the URL alias for the projects, this is automatic in path_redirect. But since project.module doesn't seem to put its data into $node->path, then this needs custom code in project module or drupalorg module. :(
#5
subscribe
#6
We're creating dead links all over the place as sandbox projects get promoted to full, which is going to frustrate a lot of people, as it is the discussion in the project maintainer application / code review process that is likely to come up in searches. Then our intelligent, actually-used-search new user is going to click on the project link, and be told that the page is not found and leave Drupal for WordPress— when really she should be getting the fantastic news that this sandbox project discussion she found now actually points to a full project!
If you know that the number in a sandbox link is the node ID, you can rewrite the URL yourself:
drupal.org/sandbox/csdco/1079042Goes to 404 page not found, but replace 'sandbox/username' with 'node'
drupal.org/node/1079042Goes to the full project page, and click on the View tab on this page to end up at the proper URL alias.
drupal.org/project/d7permissionsI'm not sure how easy it is to catch a 404 in Apache or Drupal, but the rewriting 'sandbox/username' to 'node' part is pretty easy, as an alternate approach to dealing with this.
#7
An issue I opened (#1193872: Redirect sandbox URLs to full project URLs once the sandbox is promoted) was closed as a dupe of this.
#8
Do we really need a path_redirect per item?
Here's a .htaccess line to handle all of them:
RewriteRule ^sandbox/(.+)/(.+)$ /node/$2 [NC,L,R=301]It should go somewhere inside the mod_rewrite block.
#9
@greggles: That'll redirect *all* sandbox URLs to node/$nid, then why bother with the sandbox/user/$nid pattern?
#10
Heh. Riiiiiight. That won't work.
#11
Subscribe.
#12
As we know have very many broken links to deal with, a future redirect won't be enough.
If patching Project to create the redirect on project promotion and also doing a batch operation for all existing projects (or rather those created since sandboxes came into the game) will be accepted then i'll pursue that route.
Otherwise i'd look at replacing Drupal.org's 404 page with a function that first checks for sandbox in the path, creates a 301 redirect on the fly if present, and if not treats as usual.
#13
Yes, that's what we need and killes has agreed to it.
#14
I'm less enthused about replacing the 404 page because it requires running the menu_execute_active_handler function just to return whether it should do anything or not. So i'm creating a hook menu and page callback just to interject into the 404 handling a redirect in certain cases.
The alternative though is creating tons of redirects from each sandbox alias ever to the full project it becomes. Drupal.org would go from ~350 redirects now to at least 1,000 more and however many sandbox projects are promoted each day.
But going with the agreed-upon method unless someone suggests an alternative-- a menu callback at 'sandbox' that actually checked if the project was a full project and setting the redirect if so could also work, and then we could stop creating path aliases for sandbox projects. Thoughts?
#15
(immediate feedback from Neil Drumm here in Denver, going to try the menu callback for 'sandbox' and put it in drupalorg/drupalorg_project.module)
#16
And here we are.
#17
The last submitted patch, drupal_org_sandbox_redirect-994552-16.patch, failed testing.
#18
Moving to drupalorg project queue and marking this a bug, since Sam Boyer considered it to be so.
Note: Follow-up would be needed to make sandbox path aliases unneccessary, and stop producing them. Not sure about that, really, but this stops the bleeding and should not add anything to the overhead of Drupal.org's operation.
#19
Two quibbles
1) that I think should be fixed is to rename the function from drupalorg_project_sandbox to drupalorg_project_sandbox_redirect or something like that. Right now it's not obvious that this function exists solely to redirect.
2)
<?php+ else {
?>
I guess there is a stupidly small chance that
a) someone would request a sandbox/%/%node url for a NID that is not a project_project node type
b) for that node type it would be inappropriate to go to node/NID
So it's OK that this else doesn't also check for the node type or publish status or anything else prior to redirecting.
Thanks for your work on this!
#20
1) It's technically serving up the node page if it is a sandbox, and redirecting if it is *not* a sandbox. So i'm not sure the name change clarifies. Granted the function is just a pass-through to node_page_view() and will presumably stay that way. If you greggles or anyone else wants that change for this to go in, i'm happy to make that.
2) That was my conclusion also!
#21
Ok, that makes a ton of sense.
Looks good to me!
#22
Committed.
#23
Deployed
#24
Automatically closed -- issue fixed for 2 weeks with no activity.