Support from Acquia helps fund testing for Drupal Acquia logo

Comments

wjuda created an issue. See original summary.

maaty388’s picture

If you go /admin/config/people/profiles and there is the last column for profiles operations, do you want here to add another operation link called "Translate" and then on new page Show title Edit profile_name [language_translation]. This is similar to creating the translation for nodes?

manuel.adan’s picture

Category: Feature request » Bug report

I can confirm that translation support is not available in profiles. Entity type is not listed in content language settings ( /admin/config/regional/content-language ). I tried it in Drupal 8.5. Marking as bug report since any entity type that is publicly shown must be translatable.

manuel.adan’s picture

Working on this, I surprising found that translation support was intentionally removed here: #2634230: Remove langcode entity key. The main two arguments:

1) Translating profiles doesn't make a lot of sense.
Your name, address, gender don't change between languages.

Profile is a fieldable entity, so any type of field can be added. E.g., in a social web site a really very basic field for a user profile is the user bio or status.

2) It would be very hard to provide decent UX.
Our entity translation UIs cover the admin use case, there's no UI designed to be used by the end-user in the non-admin theme.

Most of the support for multilingual content is present in core. Not sure what is exactly the obstacle here.

Any way, the lack of multilingual support must be clearly noticed. New issue was opened for that: #2983291: Notice in documentation that profile is not translatable.

minorOffense’s picture

Yeah this is a problem for us. I've posted to that issue explaining some really common use cases for user profiles to be translated.

bojanz’s picture

Version: 8.x-1.0-beta1 » 8.x-1.x-dev

Once Profile starts depending on Drupal 8.7.x, it might be possible to offer an upgrade path that makes profiles translatable.
I personally won't have time to work on this in the upcoming months, but I welcome patches.

jrochate’s picture

That would be a great feature and make sense in a multilingual D8.
Thank you for your efforts.

Harlor’s picture

First of all, I want to point out that a translatable profile makes a lot of sense to me.

I managed to have working translations witch the following changes.

There is no frontend UI and no upgrade path included.

Harlor’s picture

Sorry, the previous patch contains a wrong translation handler class.

Martijn de Wit’s picture

Status: Active » Needs review

The last submitted patch, 8: make-profiles-translatable-2899744-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

coozila’s picture

It is incredible that after 19 years of existence drupal still does not have multilangual support for profiles,the most important module . Whoever says it doesn't make sense is absurd, there are many fields in the profile, especially text those who need translation. No web page is like another, and each has different customization requirements but this is basic

fisherman90’s picture

Status: Needs review » Active

Info: @Harlor and me are working on this at DrupalCon Amsterdam :) Hopefully we can get this fixed and the tests green.

Harlor’s picture

We managed to fix several major bugs and hope to see fewer tests tail. A potential update path would require not only to migrate the data from the base table to the data-table but also to update all module configurations e.g. views that used to use the base table.

The provided patch enables translations for the profile entity and changes the data structure where necessary.
There is no upgrade path included yet.

xmacinfo’s picture

Status: Active » Needs review

Looks like the patch passes.

fisherman90’s picture

Harlor and me added an upgrade path for existing profiles. Also all views that relied on the "profile" base_table will be updated to use the "profile_field_data" base_table so the views won't break because of the change in data structure that comes with making the entity translatable.

This also means you have to export your config after the database update if you are using config-management to prevent the base_table to be set to the old value on next config-import.

From our point of view this should be finished if no bugs or regressions get discovered.

Reviews appreciated, since I need this soon for a client :)

tiagob’s picture

Hi guys,
Thank you so much for taking care of this.

I applied the latest patch and i am facing an error when trying to add a translation to a profile.

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Drupal\Core\Entity\Query\QueryException</em>: &#039;revision_translation_affected&#039; not found in <em class="placeholder">Drupal\Core\Entity\Query\Sql\Tables-&gt;ensureEntityTable()</em> (line <em class="placeholder">367</em> of <em class="placeholder">core/lib/Drupal/Core/Entity/Query/Sql/Tables.php</em>). <pre class="backtrace">Drupal\Core\Entity\Query\Sql\Tables-&gt;addField(&#039;revision_translation_affected&#039;, &#039;INNER&#039;, &#039;de&#039;) (Line: 52)

Any idea of what this could be? I installed the patch and run drush updb. I can also confirm that revision_translation_affected is presented in the new profile_field_data table.

fisherman90’s picture

Status: Needs review » Needs work

Hi tiagob,

thanks to you for testing :)

I tried to reproduce your behavior to debug it further, but I cannot. I've tried the following:

* vanilla 8.7.9 installation of drupal
* installed profile (1.x-dev)
* added a profile type with a formatted text field
* added a profile to a user (to have test-data)
* installed the patch #16
* did the database-update
* enabled content_translation module
* enabled translation for the profile entity type

After all that I can translate the profile entities.

I've also tried it with profiles being revisionable and them being not revisionable.

Can you please provide us with a step-by-step manual for reproducing the error you mention? Then we can debug it and make a better version of the patch :)

Also I discovered an issue with the update failing sometimes when there is already profile data, so for anyone wanting to use the patch:
An updated version will be made. The current one doesn't seem stable enough for using, except if you install the module freshly with the patch.

When it is installed first time it seems everything is working. So there must be still a bug in the update process.

tiagob’s picture

Hi fisherman90,

Thanks for the reply.

Maybe our issue is that we already have existing profiles on our website, and as you mention "The current one doesn't seem stable enough for using, except if you install the module freshly with the patch."

Is there a chance to try the new patch over existing profiles?

fisherman90’s picture

Status: Needs work » Needs review
FileSize
8.14 KB

Hey tiagob,

I've made little changes to the update mechanism and the entity definition, so the bug I've discovered won't appear (at least in my scenario) again.

Can you test this patch again with the database dump from before you tried the update?
Maybe the fixed update mechanism also fixes your error, at least I hope so :)

I've successfully tested it against pre-filled profile data on a client project (Drupal 8.7.9), but maybe yours is a little different. If the error still persists, we need a way to reproduce it. If you can't find out how to reproduce it, I offer you that we try to find out via pair programming with screensharing or something. You can reach me on drupalchat.me ;)

tstoeckler’s picture

Status: Needs review » Needs work

Took a look at the patch. Really impressive work! I looked through the module but everything I found (storage schema, views data...) was already handled in the patch and it comes with an update! Awesome.

I have some notes, most of which are not super important, but a couple definitely need to be fixed, so kicking back to needs work. Will take the patch for a spin now...

  1. +++ w/profile.module
    +++ w/profile.module
    @@ -219,7 +219,7 @@ function profile_views_data_alter(&$data) {
    

    The patch currently only updates the base table of the "profile" relationship, not of the "profile_type" relationship. I'm not 100% sure, but I think they should both be updated. I will test this out a bit, but noting here for now.

  2. +++ w/profile.post_update.php
    @@ -109,3 +112,75 @@ function profile_post_update_replace_view() {
    +  $entity_keys['langcode'] = 'langcode';
    +  $entity_keys['revision_translation_affected'] = 'revision_translation_affected';
    

    I was wondering whether the default_langcode key should be set here, as well. But apparently \Drupal\Core\Entity\EntityType::__construct() sets that unconditionally even for untranslatable entity types so it should be set already. The same goes for revision_translation_affected, however, so I guess that could be removed, as well.

  3. +++ w/profile.post_update.php
    @@ -109,3 +112,75 @@ function profile_post_update_replace_view() {
    +  $entity_type->setHandlerClass('translation', 'Drupal\content_translation\ContentTranslationHandler');
    
    +++ w/src/Entity/Profile.php
    @@ -28,6 +28,7 @@ use Drupal\profile\Event\ProfileLabelEvent;
    + *     "translation" = "Drupal\content_translation\ContentTranslationHandler",
    

    This is not necessary to be set as Content Translation will alter that in dynamically. And since it doesn't affect the storage, it's fine not to set it explicitly in the update, as well. (Although it doesn't hurt either...)

  4. +++ w/profile.post_update.php
    @@ -109,3 +112,75 @@ function profile_post_update_replace_view() {
    +  $entity_type->set('data_table', 'profile_field_data');
    
    +++ w/src/Entity/Profile.php
    @@ -50,14 +51,17 @@ use Drupal\profile\Event\ProfileLabelEvent;
      *   base_table = "profile",
    + *   data_table = "profile_field_data",
      *   revision_table = "profile_revision",
    

    We also need to set the revision_data_table here. There is some fallback handling if you don't specify it (which is why you probably didn't encounter any problems) but that fallback never worked reliably, so not setting this will lead to bugs at some point.

  5. +++ w/profile.post_update.php
    @@ -109,3 +112,75 @@ function profile_post_update_replace_view() {
    +    ->setRevisionable(TRUE);
    ...
    +    ->setRevisionable(TRUE)
    ...
    +  $field_storage_definitions['revision_translation_affected'] = BaseFieldDefinition::create('boolean')
    +    ->setName('revision_translation_affected')
    +    ->setTargetEntityTypeId('profile')
    +    ->setTargetBundle(NULL)
    +    ->setLabel(new TranslatableMarkup('Revision translation affected'))
    +    ->setDescription(new TranslatableMarkup('Indicates if the last edit of a translation belongs to current revision.'))
    +    ->setReadOnly(TRUE)
    +    ->setRevisionable(TRUE)
    +    ->setTranslatable(TRUE);
    

    Strictly speaking it is allowed to make revisionable entities non-revisionable with a custom (or contrib) module - at least that is the policy in core - in which case adding the revision_translation_affected field would fail (I think?). I personally never found this policy particularly realistic, so I'm not sure that needs to be accounted for on the other hand it's not too complex to wrap the field (and arguably the setRevisionable() calls) in an isRevisionable() call.

  6. +++ w/src/Entity/Profile.php
    @@ -50,14 +51,17 @@ use Drupal\profile\Event\ProfileLabelEvent;
    @@ -336,6 +340,13 @@ class Profile extends EditorialContentEntityBase implements ProfileInterface {
    

    By using EntityOwnerTrait::ownerBaseFieldDefinitions() the user/owner field will automatically become translatable with this patch, which I think is not intended as it doesn't make much sense for profiles. So we need to add a setTranslatable(FALSE) in ::baseFieldDefinitions(). (Otherwise the field would need to be updated explicitly in the update.)

  7. +++ w/src/Entity/Profile.php
    @@ -336,6 +340,13 @@ class Profile extends EditorialContentEntityBase implements ProfileInterface {
    +    $fields['langcode'] = BaseFieldDefinition::create('language')
    +      ->setStorageRequired(TRUE)
    +      ->setTargetEntityTypeId('profile')
    +      ->setLabel(t('Language code'))
    +      ->setDescription(t('The profile language code.'))
    +      ->setRevisionable(TRUE);
    

    This shouldn't be necessary as ContentEntityBase::baseFieldDefinitions() should provide that already.

tstoeckler’s picture

Tested the patch now and it works really great! The update patch worked fine and with the patch I can make profile fields translatable and properly translate the field values.

I did verify that #21.1 does in fact need to be fixed, by adding a view with the profile type relationship. After the update that view then fatals because it looks for the "uid" column in the "profile" table (instead of the "profile_field_data") table.

Harlor’s picture

Status: Needs work » Needs review
FileSize
8.3 KB

Thanks for the detailed review @tstoeckler!

The new patch 23 should exactly correct the things you mentioned.

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the quick update! Looks great, and as said in #22 I did actually test this manually, as well.

So marking RTBC, despite having two minor nitpicks. Feel free to fix those, of course, but I'm hoping those could also be fixed on commit (or not, as they are really just nitpicks...).

  1. +++ b/profile.post_update.php
    @@ -109,3 +112,74 @@ function profile_post_update_replace_view() {
    +  // Update the entity type definition.
    +  $entity_keys = $entity_type->getKeys();
    +  $entity_type->set('entity_keys', $entity_keys);
    

    This is now dead code.

  2. +++ b/src/Entity/Profile.php
    @@ -28,6 +28,7 @@ use Drupal\profile\Event\ProfileLabelEvent;
    + *     "translation" = "Drupal\content_translation\ContentTranslationHandler",
    

    This is not necessary.

Harlor’s picture

FileSize
7.76 KB

Sorry, I should have checked this more carefully. Let's hope I fixed this correctly this time.

tstoeckler’s picture

Perfect, thanks! Couldn't find anything to complain now ;-)

fisherman90’s picture

Status: Reviewed & tested by the community » Needs work

Hey Harlor and tstoeckler, thx for the good work :)

But I'm getting errors on the upgrade path now. I guess one of the removals you made was neccessary for the upgrade path to work. My Drush output:

Failed: Drupal\Core\Entity\EntityStorageException: Missing langcode      [error]
field. in
Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema->traitOnFieldableEntityTypeUpdate()
(Line 49 in
/app/web/core/lib/Drupal/Core/Entity/Sql/SqlFieldableEntityTypeListenerTrait.php).

Guess we have to do some work on it again, I will provide an updated patch when I find the right line that's missing.

fisherman90’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
7.89 KB

I've found the issue, we deleted a little bit too much :D

As you mentioned in #21, issue 2 the "revision_translation_affected" key doesn't need to be set, but the "langcode" key definitely needs to be set, otherwise the upgrade path fails with the error in #27.

So the following change:

+  $entity_keys = $entity_type->getKeys();
+  $entity_keys['langcode'] = 'langcode';
+  $entity_keys['revision_translation_affected'] = 'revision_translation_affected';
+  $entity_type->set('entity_keys', $entity_keys);

becomes now:

+  $entity_keys = $entity_type->getKeys();
+  $entity_keys['langcode'] = 'langcode';
+  $entity_type->set('entity_keys', $entity_keys);

Then it is working again. Since it was RTBC before and I found and fixed the issue on reviewing it too, I will mark it as RTBC again.

Thx for the work tstoeckler and Harlor :)

bojanz’s picture

Great job everyone!

This looks good at a glance. I'll get back to it once we tag 1.1. That way we can commit to -dev and issue a call for testing.

hexabinaer’s picture

I have been (im)patiently waiting for this issue to be fixed. Scope for the migration/relaunch of cms-garden.org was to wait for translatable profiles. Really grateful for your work, folks!

Should I wait for 1.1 or should I try -dev with the patch?

hexabinaer’s picture

Status: Reviewed & tested by the community » Needs work

I have done some (UI) testing and found some enhancement potential:

  1. Entity translation support (I gather this might not be trivial and might be related to the Paragraphs issue - with complex setups I experienced problems in the past (not) saving the settings at /admin/config/regional/content-language. It would be helpful to bypass that workaround and allow "Language settings" for the respective profile
  2. Missing "translate this profile" link for users - at least when only a single instance per profile per user is allowed. I see the concept is there but only for multiple instances.
  3. Missing profile type filter when multiple instances are allowed. To reproduce:
    • create two profile types with a textarea each
    • tick "Allow multiple profiles per user" for one of these
    • configure translations for these profile types + textareas
    • create test profiles
    • click the tab of the multi-instance profile: you get a list of all profile fields on any profile
    • Even worse: I entered test data for another user and that user's fields show up as well! (haven't tested with different permissions yet this is a major bug)

Because of the last finding I will reset to "needs work", sorry.

hexabinaer’s picture

Status: Needs work » Reviewed & tested by the community

I am sorry, I take the blame. Since no work has been done on any UI component my findings should not affect this issue. I created a new one: Translatable profiles UI issues

Jax’s picture

sanduhrs’s picture

As per #29:

This looks good at a glance. I'll get back to it once we tag 1.1. That way we can commit to -dev and issue a call for testing.

Version 1.1 has been tagged some time ago now ;)
Any chance we can get this into -dev now?

bojanz’s picture

I no longer maintain Profile, so this is now up to @mglaman.

ClassicCut’s picture

I attempted the patch on latest 1.x-dev and 1.2 but couldn't get it to apply. It's not a big fix, but I updated the patch to work with latest.

Thanks all for this, really appreciate all the work put into it.

anacolautti’s picture

Thanks for rerolling the patch! Works for me!

I agree, this is a very much needed feature. Could someone please merge this into a release?

Pete B’s picture

I tried the patch in #36 but I keep seeing the error "Failed: Drupal\Core\Entity\EntityStorageException: Missing revision_translation_affected field" when running the database updates

Does anyone know what the problem might be?

gulshk’s picture

Any updates for merge?

jungle’s picture

FileSize
7.92 KB
986 bytes

Rerolled again based on #28, keep it RTBC unless the CI rejects it.

司南’s picture

It's really make sence, this patch should be merge.

GrahamShepherd’s picture

This patch #40 applies to 8.x-1.x.

Can it be re-rolled for the current version 8.x_1.2?

GrahamShepherd’s picture

Re-rolled patch for 8.x_1.2

GrahamShepherd’s picture

This patch does apply correctly, using composer.json, to profile version 8.x-1.2. It works for my situation, which is a single language site but with multiple profiles per person. It ensures that the langcode = site default. Others will need to verify that it covers multi-language sites as they require.

GrahamShepherd’s picture

Thanks to the really clever people who did the hard work.

oakulm’s picture

I'm testing this with Profile ^1.2 and latest Drupal 9.2.7. Seems to be working

mlecha’s picture

Patch #43 tested with Profile 8.x-1.3 on Drupal 9.3.3 (Commerce Base install profile) and found it to be working.

It took me a while to figure out that you need to add a "formated" text field to a profile before you can enable translation. It's mentioned in a comment #18 way above.. thank you!

How can we get this merged into a release?

xmacinfo’s picture

Status: Reviewed & tested by the community » Needs work

Based on #43, the patch needs a reroll:

8.x-1.x: PHP 7 & MySQL 5.5, D8.9 Patch Failed to Apply

xmacinfo’s picture

Status: Needs work » Reviewed & tested by the community

OK, the real patch is in #40.

mlecha’s picture

Patch #40 tested with Profile 8.x-1.3 on Drupal 9.3.3 (Commerce Base install profile) and found profiles to be translatable.

The view "Profiles" had two issues:

1. Sort Criteria: Profile: ID (desc) is "Broken: missing handler"
2. Contextual Filters: Profile: Profile type is "Broken: missing handler"

I was able to add them back manually to the view.

sir_squall’s picture

Can we try to merge already at least in the dev branch?

Also I have tried the patch #40 it's working well, only one small improvement, the tabs menu are not having the "translate" entry:
Only local images are allowed.

aurelianzaha’s picture

patch#40 works fine for us as well
thanks for the effort!

sir_squall’s picture

Someone know how I can update the "menu_local_task" for profile, I tried but I didn't found the right way I think.

I just want to add in the edition page, a link to the "/profile/{profile_id}/translations"

sanduhrs’s picture

FileSize
7.94 KB

Reroll.

sir_squall’s picture

Hi,

I tried to add the menu local task for translation, but it's not working, and I don't know why, can someone please help me:
in "profile.links.task.yml", I added a new Derivative:

profile.user_page.single:
deriver: Drupal\profile\Plugin\Derivative\UserSingleLocalTask

and the new one, I tried to add the translate link:
<?php

namespace Drupal\profile\Plugin\Derivative;

use Drupal\Component\Plugin\Derivative\DeriverBase;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Plugin\Discovery\ContainerDeriverInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Drupal\Core\StringTranslation\TranslationManager;

/**
* Provides a user page local task for each profile type.
*/
class UserSingleLocalTask extends DeriverBase implements ContainerDeriverInterface {

/**
* The entity type manager.
*
* @var \Drupal\Core\Entity\EntityTypeManagerInterface
*/
protected $entityTypeManager;

/**
* The translation manager.
*
* @var \Drupal\Core\StringTranslation\TranslationManager
*/
protected $translationManager;

/**
* Constructs a new DynamicLocalTasks.
*
* @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
* The entity type manager.
* @param \Drupal\Core\StringTranslation\TranslationManager $string_translation
* The translation manager.
*/
public function __construct(EntityTypeManagerInterface $entity_type_manager, TranslationManager $string_translation) {
$this->entityTypeManager = $entity_type_manager;
$this->translationManager = $string_translation;
}

/**
* {@inheritdoc}
*/
public static function create(ContainerInterface $container, $base_plugin_id) {
return new static(
$container->get('entity_type.manager'),
$container->get('string_translation')
);
}

/**
* {@inheritdoc}
*/
public function getDerivativeDefinitions($base_plugin_definition) {
$this->derivatives = [];

$user = \Drupal::currentUser();
$storage = \Drupal::entityTypeManager()->getStorage('profile');
$profiles = $storage->loadMultipleByUser($user, 'public');

if ($profiles) {
foreach($profiles as $profile_id => $profile) {
$this->derivatives["$profile_id.profile_translation"] = [
'route_name' => 'entity.profile.content_translation_overview',
'route_parameters' => ['profile' => $profile_id],
'title' => $this->translationManager->translate('Translate'),
'base_route' => "entity.user.canonical",
'weight' => 100,
];
}
}

return $this->derivatives;
}

}

Orkut Murat Yılmaz’s picture

Any updates on merging?

paul_leclerc’s picture

After using this patch, I believe that it breaks some handlers of the view (/config/install/views.view.profiles.yml)
The patch make some changes on
-base_table: profile
+base_table: profile_field_data

I had to make similar updates on other lines (on sorts and arguments) that still mentionned the previous table.
These lines were sets on fields that also are on the profile table but it was still generate broken handlers.

It fixed the broken handlers.

jurgenhaas’s picture

Status: Reviewed & tested by the community » Needs work

While this patch worked on a couple of sites for me quite nicely, I was hit by an issue after updating from a distribution update. Turns out, that distro came with new and updated views, and those referred to the old/original table for views base tables, filters, fields, sorts and references.

Updating all views entities with a regex like /table: profile$/table: profile_field_data/g seems to have fixed it for me, once I imported those updated views entities.

That said, there should be 2 enhancements, I guess:

  • Enhance profile_post_update_profile_views to not only update the base table but also all other table usages
  • Find a way to re-run this hook when views get added or updated by module updates