Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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? :)
Comment | File | Size | Author |
---|---|---|---|
#42 | pathauto_et-1155132-41.patch | 3.62 KB | steinmb |
#40 | pathauto_et-1155132-40.patch | 3.72 KB | steinmb |
#39 | pathauto_et-1155132-39.patch | 3.62 KB | das-peter |
#26 | pathauto_et-1155132-26.patch | 2.84 KB | plach |
#26 | pathauto_et-1155132-26.BC_.do_not_commit.patch | 2.84 KB | plach |
Comments
Comment #1
das-peter CreditAttribution: das-peter commentedRelated feature request in entity_translation module: #1155134: Integrate pathauto bulk generation
Comment #2
Dave ReidKeep 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.
Comment #3
das-peter CreditAttribution: das-peter commentedThanks for the feedback, I hope I got the point.
I rewrote the patch and tried to cover all the ugly stuff within entity_translation :)
Comment #4
das-peter CreditAttribution: das-peter commentedComment #5
joostvdl CreditAttribution: joostvdl commentedSubscribe
Comment #6
calculus CreditAttribution: calculus commentedSubscribe
Comment #7
das-peter CreditAttribution: das-peter commentedRerolled patch against latest dev version.
Comment #8
Dave ReidOn review this seems pretty good and allows other modules to do things that we can't handle by deafult, so that's a plus.
Unnecessary parenthesis. Both sets can be removed.
Would it be worth it to check if isset($term->language) just in case another module provides it?
Let's just pass through the $options variable into pathauto_blog_update_alias().
Powered by Dreditor.
Comment #9
das-peter CreditAttribution: das-peter commentedPatch updated - @Dave Reid thank you for the feedback.
Comment #10
kdebaas CreditAttribution: kdebaas commentedThe 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.)
Comment #11
Dave ReidThat notice has already been fixed in the 7.x-1.x Git branch.
Comment #12
Dave ReidPlease also roll the patch with git diff (to make sure it's using -p1).
Comment #13
Dave ReidNm, got it to apply.
Comment #14
omoeen CreditAttribution: omoeen commentedSubscribe
Comment #15
Dave ReidCommitted #9 with a few tweaks (resetting $options['language'] when aliasing sub-terms, etc) to Git:
http://drupalcode.org/project/pathauto.git/commit/ac0355e
Comment #16
das-peter CreditAttribution: das-peter commentedAwesome - thank you very much!
Comment #18
plachSorry 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.
Comment #19
plachThe core patch has landed!
Comment #21
klonos#1155134: Integrate pathauto bulk generation got fixed!
Comment #22
plachWorking on tests.
Comment #23
plach#18: pathauto_et-1155132-18.patch queued for re-testing.
Comment #24
Dave ReidWe 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.
Comment #25
plachWorking on #24, tests all green locally with #18 however.
Comment #26
plachThe attached patch implements #24. Attached there is also a version for the bot to test the BC if-branch.
Comment #27
plachBoth green :)
Review/test feedback welcome.
Comment #28
plach@klonos:
Did you have the chance to test #26?
Comment #29
beanluc CreditAttribution: beanluc commentedDaveReid said in #2 above:
And that's been done.
http://drupal.org/node/1155134
Comment #30
plach@beanluc:
Did you read the issue you linked and #24?
Comment #31
nikosnikos CreditAttribution: nikosnikos commentedI 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
Comment #32
plachThe 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.
Comment #33
nikosnikos CreditAttribution: nikosnikos commentedI confirm.
Comment #34
jonhattanI also confirm. Using drupal 7.x-dev
Comment #35
plach@Dave Reid:
Any pending concern?
Comment #36
klonos@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.Comment #37
plach@klonos:
Additional patch review/RTBC confirmation would be welcome. Hopefully Dave will finde some time to commit the patch here.
Comment #38
colanImproving title for specificity and findability.
Comment #39
das-peter CreditAttribution: das-peter commentedI 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 bypathauto_entity_language()
the term propertyvocabulary_machine_name
is necessary.Thus I moved following code before the call of
pathauto_entity_language()
inpathauto_taxonomy_term_update_alias()
:Comment #40
steinmb CreditAttribution: steinmb commentedGit 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.
Comment #42
steinmb CreditAttribution: steinmb commentedOoobs, let's do that one more time.
Comment #43
plachLooks good to me. Someone else RTBC this please.
Comment #44
steinmb CreditAttribution: steinmb commentedLet's do this
Comment #45
plach#26 still holds a patch that successfully tests the BC code path.
Comment #46
Dave ReidCommitted #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
Comment #47
plachWhoo-hoo!
@davereid++
Comment #48
das-peter CreditAttribution: das-peter commentedAwesome, thanks!
Comment #50
klonosPerfect!! Thanx Dave! Better late then never ;)
...(another) one less patch to babysit between upgrades!
Comment #51
q0rban CreditAttribution: q0rban commentedI 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.
Comment #52
q0rban CreditAttribution: q0rban commented@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
Comment #52.0
q0rban CreditAttribution: q0rban commentedAdd in "entity translation" (without underscore), otherwise you can't find this issue when searching the queue for "entity translation"
Comment #53
kopeboy CreditAttribution: kopeboy commentedIs 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?
Comment #54
plachThis 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.
Comment #55
plachComment #56
gambry--Edit: sorry, wrong issue--
Comment #57
bwaindwain CreditAttribution: bwaindwain as a volunteer commentedfyi: 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