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? :)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

das-peter’s picture

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

Dave Reid’s picture

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.

das-peter’s picture

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 :)

das-peter’s picture

Status: Needs work » Needs review
joostvdl’s picture

Subscribe

calculus’s picture

Subscribe

das-peter’s picture

Rerolled patch against latest dev version.

Dave Reid’s picture

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.

das-peter’s picture

Status: Needs work » Needs review
FileSize
3.37 KB

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

kdebaas’s picture

    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.)

Dave Reid’s picture

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

Dave Reid’s picture

Status: Needs review » Needs work

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

Dave Reid’s picture

Status: Needs work » Needs review

Nm, got it to apply.

omoeen’s picture

Subscribe

Dave Reid’s picture

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

das-peter’s picture

Awesome - thank you very much!

Status: Fixed » Closed (fixed)

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

plach’s picture

Status: Closed (fixed) » Postponed
FileSize
1.97 KB

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.

plach’s picture

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.

klonos’s picture

plach’s picture

Assigned: Unassigned » plach

Working on tests.

plach’s picture

Status: Needs work » Needs review

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

Dave Reid’s picture

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.

plach’s picture

Status: Needs work » Needs review

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

plach’s picture

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

plach’s picture

Assigned: plach » Unassigned

Both green :)

Review/test feedback welcome.

plach’s picture

@klonos:

Did you have the chance to test #26?

beanluc’s picture

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

plach’s picture

@beanluc:

Did you read the issue you linked and #24?

nikosnikos’s picture

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

plach’s picture

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.

nikosnikos’s picture

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.

jonhattan’s picture

Status: Needs review » Reviewed & tested by the community

I also confirm. Using drupal 7.x-dev

plach’s picture

@Dave Reid:

Any pending concern?

klonos’s picture

@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.

plach’s picture

@klonos:

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

colan’s picture

Title: Entity_translation integration » Add Entity Translation support to Pathauto

Improving title for specificity and findability.

das-peter’s picture

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():

  // 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;
  }
steinmb’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
3.72 KB

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.

steinmb’s picture

Status: Needs work » Needs review
FileSize
3.62 KB

Ooobs, let's do that one more time.

plach’s picture

Looks good to me. Someone else RTBC this please.

steinmb’s picture

Status: Needs review » Reviewed & tested by the community

Let's do this

plach’s picture

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

Dave Reid’s picture

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

plach’s picture

Whoo-hoo!

@davereid++

das-peter’s picture

Awesome, thanks!

Status: Fixed » Closed (fixed)

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

klonos’s picture

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

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

q0rban’s picture

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.

q0rban’s picture

@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

q0rban’s picture

Issue summary: View changes

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

kopeboy’s picture

Issue summary: View changes
Status: Closed (fixed) » Needs review

Is this included in any stable release yet? I don't get aliases for entities when viewed in a language different from the "original" one.

What about the other patch linked by q0rban? Is that required for this to work? Are they independent?

plach’s picture

This has been fixed for a long time so please do not re-open it. If you have issues please open a bug report and see https://www.drupal.org/project/entity_translation#bugfixing.

plach’s picture

Status: Needs review » Closed (fixed)
gambry’s picture

--Edit: sorry, wrong issue--

bwaindwain’s picture

fyi: I've added two issues with patches to entity_translation to improve handling of pathauto v1.3+

New translation ignores pathauto_state https://www.drupal.org/node/2741407

Pathauto update for all translations https://www.drupal.org/node/2743685