Posted by ryan_courtnage on October 6, 2010 at 4:38pm
9 followers
| Project: | Spaces |
| Version: | 7.x-3.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | major |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
To reproduce:
URL where browser complains about redir will look like:
http://localhost/drupal/[space_path]/admin/content/taxonomy/edit/term/3?destination=admin/content/taxonomy/1
Comments
#1
I'm also seeing this error.
#2
Issue is triggered by the call to
$space->activateinspaces_taxonomy_form_taxonomy_form_term_alter(). The activate method is not overridden in the space_taxonomy class, so space_type_purl's method is called, including apurl_goto()call that leads to a loop.Is it necessary to activate the space on edit? If not, we could just drop this call. If so, we need to dig into what's causing the loop.
#3
Switching to major, since spaces_taxonomy is unusable.
This error is caused by activating a space in an excluded path. The space is activated, triggering purl_goto(), but on the new page load, since this is an excluded path, the deactivate method is triggered, returning to the original path, etc.
We can remove the activate call from spaces_taxonomy_form_taxonomy_form_term_alter(), since it's called in an excluded path (within admin/*). But we also should fix the activate method, to trigger a goto only if not on an excluded path.
Patch attached.
#4
The patch works. Thanks nedjo!
#5
This patch fixes the redirect issue for me.
#6
This should be rolled into stable release. As until it is the spacex_taxonomy portion of the module breaks sites.
#7
#8
This error also happens when you are editing features.
#9
Cross post to a duplicate issue #993618: purl_goto in spaces_type_purl->activate can cause infinite redirect loop on excluded paths..
On #3, was the change in the order of operations between purl_goto() and parent::activate() intentional?
Seems like that could slow things down when a redirect is appropriate, if nothing else.
#10
Yes, intentional to avoid the issues cited in the existing code comment:
// Activate the space before checking for excluded paths. This prevents// certain corner case badness, e.g. a stale $_GET['q'] set for the site
// space triggering an excluded path check.
#11
I'm not sure if this helps at all but I was having a very similar sounding issue that I unfortunately had to patch spaces_og.inc in order to fix.
line 149 is
spaces_load('og', current($node->og_groups))->activate();This is running when the activate function is called as part of the Space object. Here's the use-case where this is problematic:
I create a node called Course
Course is not a group, but during the saving of a Course, there is the option to automatically create a node of type group / space called Offering.
After Offering is established, we want to create a book of nodes (automatically) so that the user has something to work with.
In looping through and creating these nodes, the invoking of node_save for the node will trigger the ->activate() function in the Space.
This looks at the created node and evaluates:
-- I'm working on a node that claims to have a group.
-- That Group / Space currently isn't active
-- I need to activate the Space
This then triggers a purl_goto and forces the browser to the purl based address of the node creation form. It still makes the first node in my batch process (a node that creates other nodes) but it never finishes all of them. Unfortunately I had to add the following in order to prevent this from happening and I'll bet its a similar case for bulk operations / Batch API calls (had this issue before with that too).
if ($_GET['q'] != 'node/add/course') {spaces_load('og', current($node->og_groups))->activate();
}
Wrapping line 149 allows us to effectively block the activation of a space on certain paths. Extremely useful in my given context. If there's another way of doing this I'd love to know but this seems an issue with Spaces -- While outside the context of a space, you can't create content and tell it to be part of a Space w/o triggering the activation of that space. Moving up to 3.1 as I had to fix it in that version and hopefully bumping this issue a bit.
#12
Sounds like you need to add something like
node/add/[og-node-type]where [og-node-type] is the og node type to the excludedPaths for the og spaces type. Does that, in combination my patch above, address the issue?#13
not sure I'll have to try it out, not using Spaces Taxonomy so not sure the patch would help me there.
#14
thanks, patch works!
+1 to commit
#15
finally realized a way of fixing this in a separate module. If you implement the registry overrides available to spaces and do a class override then you can exclude additional paths as you want. I have this working on a module I'm making, here's the github space if anyone's interested in seeing how it's possible. 2 functions and an include file fix months of frustration for me :p
https://github.com/btopro/elms/tree/master/profiles/elms/modules/elms_co...
#16
This is an issue in Drupal 7 too. I assume new development is going into 7.x first, so bumping the version number on this issue.
#17
Here is the patch in 3 adjusted for Drupal 7. I found I also had to add taxonomy/term/*/* to the exclude paths.
#18
Ok, so adding taxonomy/term/*/* to the exclude paths was a bad idea as it makes it impossible to customize/override the features per taxonomy space.
#19
Revised patch attached. What was making this hard to debug is this bug in PURL #1351212: Redirect Loop on all pages but <front> which is incompatible with globalredirect. With globalredirect disabled, and this patch applied, things seem to be working with spaces_taxonomy in D7.
#20
Thanks for updating the patch!
A taxonomy-type excluded path should be in an override spaces_taxonomy/plugins/space_taxonomy.inc rather than space_type_purl. Draft:
<?php/**
* Override of excluded_paths().
*/
protected function excluded_paths() {
$paths = parent::excluded_paths();
$paths[] = 'taxonomy/term/*/*';
return $paths;
}
?>
#21
Updated patch attached.
EDIT: Although, I'm not entirely sure why those paths need to be excluded in the first place.
#22
The patch in #21 is no good. Please use the patch in #19. Those paths should not be excluded otherwise it is impossible to edit settings within the context of a space.
#23
k, I'd somehow missed reading #18 and #19 and was pointing out that, if a taxonomy path exclusion was included, it should be done in spaces taxonomy. But if it's not needed, great.