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.
Hi, currently entity translation is integrated with pathauto only for the "node" entity; It would be nice to have integration with other core entities, "user" and "taxonomy term".
Comment | File | Size | Author |
---|---|---|---|
#13 | et-pathauto_integration-1842540-13.patch | 2.48 KB | peximo |
#12 | et-pathauto_integration-1842540-12.patch | 2.48 KB | peximo |
#11 | et-pathauto_integration-1842540-11.patch | 2.52 KB | bforchhammer |
#8 | et-pathauto_integration-1842540-7.patch | 2.41 KB | peximo |
#7 | et-pathauto_integration-1842540-6.patch | 2.79 KB | peximo |
Comments
Comment #1
peximo CreditAttribution: peximo commentedAttached patch should add this feature.
Comment #2
peximo CreditAttribution: peximo commentedWrong status.
Comment #3
bforchhammer CreditAttribution: bforchhammer commentedThanks for your patch!
I'm wondering whether this can be done in a more generic way without any code specific to nodes (or taxonomy, or users)... e.g. use context module key as the entity type, replace
entity_translation_node
withentity_translation_enabled_bundle
, ... ?Why do we need this mapping? Maybe add a code-comment.
This comment needs to be updated to reflect the additional entity types.
Comment #4
peximo CreditAttribution: peximo commentedHi, I rewrited the patch by following your instructions; the mapping between entity type and token type can be done using the key "token type" in the entity info, I think it's better that way (I have adopted the same approach in a patch for the title module).
We need this mapping because taxonomy use "term" and not the entity type as token key.
Comment #5
plachThe mapped value needs to be used also here and replace
$context['module']
. However I'm a bit concerned about this approach because there is no guarantee that$context['type']
holds a bundle name, correct?Comment #6
plachActually there is also no guarantee that
$context['module']
holds a value mappable to an entity type, hence at very least we need a check for that as in the Title patch.Overall, I think that caring only about core entities and leaving the rest to modules implementing this hook would be an acceptable solution too.
Comment #7
peximo CreditAttribution: peximo commentedOk, I added a check on the mapping and rewrited the extraction of entity type and bundle to make it safer.
I checked also that
entity_translation_enabled()
andentity_translation_enabled_bundle()
don't generate errors or warnings if the entity_type and/or the bundle is invalid.Comment #8
peximo CreditAttribution: peximo commentedThe mapping is no longer necessary, is the same as the entities info. Also
isset($info[$context['module']])
implicitly checks that$context['module']
is an entity type.Comment #9
plachLooks good to me, thanks. One minor thing: can we have an $entity_type variable instead of $context['module'] to improve readbility?
Comment #10
plachTo be 100% safe I think this should be
!empty($info[$entity_type]['token info']) && !empty($context['data'][$info[$entity_type]['token info']])
Comment #11
bforchhammer CreditAttribution: bforchhammer commentedUpdated patch with changes suggested in #9 and #10.
Comment #12
peximo CreditAttribution: peximo commented@bforchhammer: the key is not "token info" as written by Plach but "token type".
Rerolled.
Comment #13
peximo CreditAttribution: peximo commentedSorry wrong patch.
Comment #14
plachI guess we need some tests here but they should not block commit (RTBC from me).
However I'd like to commit this after beta2 is out to minimize the risk of introducing new bugs.
Comment #15
plachNow the beta2 is out we can commit this.
Comment #16
bforchhammer CreditAttribution: bforchhammer commentedThanks, committed and pushed :)