This patch adds Entity translation (entity_translation) integration for pathauto.
The alias update function is now triggered on entity_translation events, too.
Besides that the node specific stuff got the most attention:

  • The node form alteration is slightly modified to integrate seamless into the entity_translation form.
  • The bulk update process takes care of setting an alias for each available translation of a node.

I would like to see a more generic way to deal with the two points above but I couldn't really think of a good one.
Any ideas? :)

Files: 
CommentFileSizeAuthor
#42 pathauto_et-1155132-41.patch3.62 KBsteinmb
PASSED: [[SimpleTest]]: [MySQL] 316 pass(es).
[ View ]
#40 pathauto_et-1155132-40.patch3.72 KBsteinmb
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch pathauto_et-1155132-40.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#39 pathauto_et-1155132-39.patch3.62 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 316 pass(es).
[ View ]
#26 pathauto_et-1155132-26.patch2.84 KBplach
PASSED: [[SimpleTest]]: [MySQL] 316 pass(es).
[ View ]
#26 pathauto_et-1155132-26.BC_.do_not_commit.patch2.84 KBplach
PASSED: [[SimpleTest]]: [MySQL] 316 pass(es).
[ View ]
#18 pathauto_et-1155132-18.patch1.97 KBplach
FAILED: [[SimpleTest]]: [MySQL] 307 pass(es), 9 fail(s), and 0 exception(s).
[ View ]
#9 pathauto-entity_translation-integration-1155132-9.patch3.37 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#7 pathauto-entity_translation-integration-1155132-7.patch3.37 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
#3 pathauto-entity_translation-integration-1155132-3.patch4.02 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]
pathauto-entity_translation-integration.patch7.48 KBdas-peter
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Comments

Related feature request in entity_translation module: #1155134: Integrate pathauto bulk generation

Status:Needs review» Needs work

Keep in mind Pathauto can only support core modules, so entity_translation integration should go in entity_translation wherever possible. Please make sure any changes here are purely API changes that would allow the support to go into entity_translation.

StatusFileSize
new4.02 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Thanks for the feedback, I hope I got the point.
I rewrote the patch and tried to cover all the ugly stuff within entity_translation :)

Status:Needs work» Needs review

Subscribe

Subscribe

StatusFileSize
new3.37 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Rerolled patch against latest dev version.

Status:Needs review» Needs work

On review this seems pretty good and allows other modules to do things that we can't handle by deafult, so that's a plus.

+++ pathauto.moduleundefined
@@ -413,15 +413,17 @@ function pathauto_node_update_alias(stdClass $node, $op, array $options = array(
+    'language' => ((isset($node->language) ? $node->language : LANGUAGE_NONE)),

Unnecessary parenthesis. Both sets can be removed.

+++ pathauto.moduleundefined
@@ -499,7 +501,10 @@ function pathauto_taxonomy_term_update_alias(stdClass $term, $op, array $options
+    'language' => LANGUAGE_NONE,

Would it be worth it to check if isset($term->language) just in case another module provides it?

+++ pathauto.moduleundefined
@@ -622,11 +630,11 @@ function pathauto_user_update_alias(stdClass $account, $op, array $options = arr
+    pathauto_blog_update_alias($account, $op, array('language' => $options['language']));

Let's just pass through the $options variable into pathauto_blog_update_alias().

Powered by Dreditor.

Status:Needs work» Needs review
StatusFileSize
new3.37 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es).
[ View ]

Patch updated - @Dave Reid thank you for the feedback.

    Notice: Undefined index: can_transliterate en pathauto_cleanstring() (línea 188 de /home/klaas/web/parallelports/modules/pathauto/pathauto.inc).
    Notice: Undefined index: can_transliterate en pathauto_cleanstring() (línea 188 de /home/klaas/web/parallelports/modules/pathauto/pathauto.inc).

The above error message appears when I try to edit / translate a node, after applying this patch. (I also applied #1155134: Integrate pathauto bulk generation and #1155128: Enhance support for tokens.)

That notice has already been fixed in the 7.x-1.x Git branch.

Status:Needs review» Needs work

Please also roll the patch with git diff (to make sure it's using -p1).

Status:Needs work» Needs review

Nm, got it to apply.

Subscribe

Status:Needs review» Fixed

Committed #9 with a few tweaks (resetting $options['language'] when aliasing sub-terms, etc) to Git:
http://drupalcode.org/project/pathauto.git/commit/ac0355e

Awesome - thank you very much!

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Status:Closed (fixed)» Postponed
StatusFileSize
new1.97 KB
FAILED: [[SimpleTest]]: [MySQL] 307 pass(es), 9 fail(s), and 0 exception(s).
[ View ]

Sorry for having to reopen this one: entity language is going to be introduced in #1495648: Introduce entity language support, hence, as discussed in Denver with Dave, we need to update the code here to support it. Setting to postponed as we need to wait for the core patch to land.

Status:Postponed» Needs review

The core patch has landed!

Status:Needs review» Needs work

The last submitted patch, pathauto_et-1155132-18.patch, failed testing.

Assigned:Unassigned» plach

Working on tests.

Status:Needs work» Needs review

#18: pathauto_et-1155132-18.patch queued for re-testing.

Status:Needs review» Needs work

We also need to ensure that this patch makes entity_language integration optional. I don't really want to break installs where people update contrib faster than core. I'm ok with a wrapper function like pathauto_entity_language() that uses entity_language() if it exists and has some fallback logic if not.

Status:Needs work» Needs review

Working on #24, tests all green locally with #18 however.

StatusFileSize
new2.84 KB
PASSED: [[SimpleTest]]: [MySQL] 316 pass(es).
[ View ]
new2.84 KB
PASSED: [[SimpleTest]]: [MySQL] 316 pass(es).
[ View ]

The attached patch implements #24. Attached there is also a version for the bot to test the BC if-branch.

Assigned:plach» Unassigned

Both green :)

Review/test feedback welcome.

@klonos:

Did you have the chance to test #26?

DaveReid said in #2 above:

Keep in mind Pathauto can only support core modules, so entity_translation integration should go in entity_translation wherever possible

And that's been done.
http://drupal.org/node/1155134

@beanluc:

Did you read the issue you linked and #24?

I tested this patch on the last dev with the last dev of entity_translation.
When I delete all content aliases and I bulk update my content pathes I don't get back my translated paths I only get the aliases for the node in the original content. And when I edit the translation "Generate automatic URL alias" is unchecked (it was checked before delete/bulk update).
I'm not sure that this as to be reported here or in #1155134: Integrate pathauto bulk generation

The aim of this patch is just to make the entity form work with pathauto. Bulk updates are not covered here and will have to be addressed in entity translation, I guess. You should only confirm that this patch provides a translated alias when saving the entity form in the various languages with the "Generate automatic URL alias" option checked.

You should only confirm that this patch provides a translated alias when saving the entity form in the various languages with the "Generate automatic URL alias" option checked.

I confirm.

Status:Needs review» Reviewed & tested by the community

I also confirm. Using drupal 7.x-dev

@Dave Reid:

Any pending concern?

@plach, #28: Sorry for the late reply Francesco (being summer and all). Is there anything specific you need to test? Do I need to apply the latest from #1155134: Integrate pathauto bulk generation as well (scratch that - already committed and I'm using latest dev)? Let me know ...if it's not too late.

@klonos:

Additional patch review/RTBC confirmation would be welcome. Hopefully Dave will finde some time to commit the patch here.

Title:Entity_translation integrationAdd Entity Translation support to Pathauto

Improving title for specificity and findability.

StatusFileSize
new3.62 KB
PASSED: [[SimpleTest]]: [MySQL] 316 pass(es).
[ View ]

I just updated one of our projects to the latest entity_translation version *yay*.
This gave me the ability to give this a review.
The nodes worked just fine, but I found an issue with the taxonomy terms.
To use entity_extract_ids() which is triggered by pathauto_entity_language() the term property vocabulary_machine_name is necessary.
Thus I moved following code before the call of pathauto_entity_language() in pathauto_taxonomy_term_update_alias():

<?php
 
// Check that the term has its bundle, which is the vocabulary's machine name.
 
if (!isset($term->vocabulary_machine_name)) {
   
$vocabulary = taxonomy_vocabulary_load($term->vid);
   
$term->vocabulary_machine_name = $vocabulary->machine_name;
  }
?>

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new3.72 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch pathauto_et-1155132-40.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Git did not like the patch, formatting? Applied it with patch -p1 and rerolled it with git diff made it happy again. Ehhh anyway, the patch fixes pathauto for entity type 'nodes'. At admin/config/search/path/patterns was there not replacement pattern for taxonomy so I was unable to test terms. I must be missing something in my sandbox, hints?

Putting it back to needs reviews then I feel we could do with more reviewers.

Status:Needs review» Needs work

The last submitted patch, pathauto_et-1155132-40.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.62 KB
PASSED: [[SimpleTest]]: [MySQL] 316 pass(es).
[ View ]

Ooobs, let's do that one more time.

Looks good to me. Someone else RTBC this please.

Status:Needs review» Reviewed & tested by the community

Let's do this

#26 still holds a patch that successfully tests the BC code path.

Priority:Normal» Major
Status:Reviewed & tested by the community» Fixed

Committed #42 with some modifications to 7.x-1.x. Thanks everyone for your patience with me to commit this.
http://drupalcode.org/project/pathauto.git/commit/e95ceaf

Whoo-hoo!

@davereid++

Awesome, thanks!

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Perfect!! Thanx Dave! Better late then never ;)

...(another) one less patch to babysit between upgrades!

I am very late to the game, but am not understanding the need for all this logic. Why aren't we just checking $node->path['language'] for the language instead of doing again what core has already done here, here and here? The reason I ask is this is breaking the ability to programmatically set the language on the path from the node form without having to resave the path alias. See #1499532: Cannot programmatically specify node path alias language for a related Drupal core issue.

@davereid made a good point in IRC that we can't just trust $entity->path['language'], as when programmatic saves are happening, that value may not be present. I will create a separate issue to allow for respecting the path language if it exists.

Edit: here is the issue: #2030983: Cannot programmatically specify entity path alias language

Issue summary:View changes

Add in "entity translation" (without underscore), otherwise you can't find this issue when searching the queue for "entity translation"