Language support in Feed import

Language support hasn't been ported to the D8 version yet. It will be ported after the base features of Feeds 8.x-3.x are finished. See also #1960800: [meta] Feeds 8.x roadmap. Porting features is listed as step 4 there.

I have found some people who may want to help with continuing the port (the port is put on hold at the moment). Hopefully, we can start with that in early 2017.

A patch is always welcome. :) It might take a while though before it gets a review.

Hello Takim !

Can you precise what is your method to import contents with different languages? For this time, we haven't this option in the feeds module. Can you share your solution for the maintainers of feeds module? This option is essential for the multilingual sites.

Thanks.

We also ran into this issue and would appreciate some info on how you fixed this. We don't want to reinvent the wheel.

The Cision Feeds module has the source mapping for the language, but there is no mapping target for the langcode.

Here is a simple patch that works for me. I only added a Language target definition, where the main point is the annotation.

Anyone feel free to add tests and perhaps validation / cleanup for the langcode formatting, if deemed necessary.

We have to import several different types of content in four languages and the feeds module is great to get the data into Drupal. However, we need all four languages to be translations of the same entity. Based on the discussion and after studying I've had a look at making the import multilingual as well.

Outline of my approach
- Use the language target plugin of @arskiainen to provide a suitable target field in the mapping.
- Check the value provided for the language target and load or create the entity in the corresponding language.
- Pass the entity in the correct language to hashing, map(), validate etc. and skip setting the langcode field (this would throw an error in Drupal Core, see https://www.drupal.org/node/2443989).

This approach allows to get the language code from the imported file and assumes that there is a unique value across all languages (e.g. an common id within the import data). One still has to create one feed per language though, which is okay for us though.
I've tested the code with our XML based imports and one unique field.

Let me know if you see any problems with my approach or code. I haven't used the feeds module before today, so I might have very well missed something.

Thanks for your efforts, @pvbergen
Ideally the language solution would follow a similar approach as in the D7 version of Feeds. Can't the language be set during mapping? Does it need to happen before setting other values?

Thanks for your quick response, MegaChriz! I have never used feeds in Drupal 7, so I can't tell how close my solution is.

In the approach in #10, you can indeed select the language as a target field in the mappings and it can be at any position.
Below is a screenshot of one of my tests, where you can see the setup (with XML/xpath parser from feeds_ex).

@pvbergen
I meant: can the language be changed after having set other values on the entity first in code?
For example (pseudo code):

$entity->field_name = 'Value';
$entity->body = 'Lorem ipsum';
// Set language. The two field values above will be saved for the Dutch language.
$entity->language = 'NL';

Ideally, the code that sets the language of the entity would live in \Drupal\feeds\Feeds\TargetLanguage::setTarget() instead of in EntityProcessorBase.

I now understand what you mean MegaChriz. I actually wanted to do this very thing first, setting/updating the language in \Drupal\feeds\Feeds\TargetLanguage::setTarget().

As far as I'm aware, this only works when you want to change the langcode of the entity translation. E.g. updating a Dutch translation to be the English translation, effectively removing the Dutch translation of the entity.

To add a new translation in another language while keeping the old translation, \Drupal\Core\Entity\ContentEntityBase::addTranslation has to be used.
The function creates a translation of the entity and stores it to the database immediately. That's why one has to give a value array as the second parameter, containing at least the title, too.

The processor needs to get/add a translation before any values are being set, hence the changes in EntityProcessorBase.
I guess your questions stems from the thought to allow hooks/subscribers/heirs to act upon the langcode before it is set.
I don't really have a solution for that..

Facing a similar D8 challenge (working with multiple languages) and, following the suggestions above, have introduced the patches #9/#10 above and that's provided 'Language' target option.

However, when attempting to import a translation, it fails with "Provided language is not installed" using default English [en] to successfully create the node and then introducing German [de] translation it topples.

Working with the simplest source possible, eg

<nodes>
  <node>
    <language>de</language>
    <title>title in german</title>
    <desc>desc in german</title>
  </node>
</nodes>

Tried multiple ways to address langcode to no avail. Am I missing something obvious?

Update/overlooking the obvious:
After running through, debugging values, etc I finally noticed that the langcode in languages array check was always throwing a fail when the langcode was found in the array of enabled languages.

Adding a '!' to line 131 of feeds/src/Processor/EntityProcessorBase.php...
if (!in_array($langcode, $languages)) {
...appears to create a reliable enough multi-lingual import for now.

Hi! Thanks, i've tested #10 and works for me adding the suggestion in #16.
I attach the modified patch here (also applying with composer the #10 didn't work, it seems some enconding problem, now i try with the one i'm attaching.)

Thank you

Hello, I do not find how to import several languages. I did the update but nothing. Can you tell me how do you do it?

#17 works ok for me (for a single language). Setting to Reviewed and tested.

@MegaChriz can we get at least this simple language support in the next feeds dev and alpha versions?

We desperately also need feeds multilngual entity support (entity translations) soon, but at please commit this stuff first?

Hello everyone,
To me the patch # 17 in localhost did not work with:
Drupal 8.6.1
Apache / 2.4.33 (Win64) OpenSSL / 1.1.0h PHP / 7.0.25

It would be good to work in this field.
Thank you very much for your work.

@ReViJa

You should be more specific how it did not work with D8.6.x? I've also upgraded to D8.6.x, but the language mapping is working for me currently.

However I had to create workaround for a D8.6.x issue related to translations and probably content_moderation. Without an entity alter hook I'd get this validation error for everything:

"Unable to change non-translatable field value on translatable content with content "

function hook_entity_type_alter(array &$entity_types) {
    foreach ($entity_types as $entity_type)
    {
        $constraints = $entity_type->getConstraints();
        unset($constraints['EntityUntranslatableFields']);
        $entity_type->setConstraints($constraints);
    }
}

More info:
* https://www.drupal.org/node/2938191 / New API to specify whether changes to untranslatable fields should affect only the default revision translation
* https://www.drupal.org/project/drupal/issues/2955321 / Unable to change non-translatable field value on translatable content with content moderation enabled.
* https://www.drupal.org/project/drupal/issues/2878556 / Ensure that changes to untranslatable fields affect only one translation in pending revisions
* https://www.drupal.org/project/drupal/issues/2959175 / Non-translatable fields error when using: Content Moderation + Content Translation + Comments
* https://www.drupal.org/project/drupal/issues/2955321 / Unable to change non-translatable field value on translatable content with content moderation enabled
*
* Related drupal API changes, setting which solves this in some cases, but moderation and feeds complicate this.
* https://www.drupal.org/node/2938193 / New option to hide untranslatable field widgets
* https://www.drupal.org/node/2950608 / Content translations can be moderated independently
* https://www.drupal.org/project/drupal/issues/2950626 / Allow flagging translations as outdated when content is moderated

NOTE: I'm still having other issues, especially related to content_moderation.

Good news: the language import works actually pretty nicely even with translated content (shared guid).
The origin language flag doest not need to be mapped, there were problems.

However, content_moderation breaks the import fully for every translation , so I created a new issue about that:
https://www.drupal.org/project/feeds/issues/3002606

A new issue I found with latest patch:
It seems that feeds tamper is not executed early enough, validation fails before that.

For example if the language source is set by a tamper plugin default value, it doesnt work but fails because of this line:
$state->setMessage('Provided language is not installed.', 'warning');

How to make feeds tamper run correctly before validation?

I can understand the high need for this feature, but I think the current implementation make things less maintainable as it special cases the langcode target. I haven't been working on multilingual sites in D8 yet, but it seems the code is also adding support for translations beside just setting the entity's language:

+++ b/src/Feeds/Processor/EntityProcessorBase.php
@@ -122,9 +122,32 @@ abstract class EntityProcessorBase extends ProcessorBase implements EntityProces
+        if (!$entity->hasTranslation($langcode)) {
+          $entity = $entity->addTranslation($langcode, ['title' => $entity->title->value]);
+        } else {
+          $entity = $entity->getTranslation($langcode);
+        }

What I would like to see is the following:

1. Have the implementation for setting the entity's language in the Language target. I haven't tried it, but according to https://drupal.stackexchange.com/questions/240086/how-to-update-the-lang... it looks like it would be possible.
2. Describe what this issue is trying to solve. I assume importing content on which the language is specified by the source as seen in the example from #16. Translating content doesn't fit in that picture, in my opinion. When I think of supporting translating content, I think of a source something like this:
   

   title_en,title_nl,title_fr
   Carrots,Wortelen,Carottes
   

   Anyway, update the issue summary to clearly reflect what this issue tries to solve (and what it does not try to solve - yet).

3. So leaving out code that supports translating content. A feature like this can be implemented better in smaller steps, so the code changes become easier to follow. So translation support should be postponed to a follow-up.
4. Automated tests: add a kernel test that shows us that importing content - where the source specifies the language per item - works.

Here is a patch that continues work from #9

In this patch the following improvements are done:
- Set langcode using ->set method from entity (@see drupal answer mentioned in #25 https://drupal.stackexchange.com/questions/240086/how-to-update-the-lang...).
- Allow set, for each mapping field, in which language set the entity field value. If empty uses default language. To test it you need add a multilanguage field in mapping, and you could select language in configuration.

Here is an screenshot of it:

With this patch a complete language support should be done.

Tests are still pending but code and behaviour can be reviewed.

Please review, thanks!

Adding new patch with the following changes:
- Correct dependency test, it was failing because the changes alter the default config for feed targets.
- Add tests for language.
- Add tests for content translation.
- Set correct author for content translation, it was not being correctly set before.

Adding to Needs review again.

@mistermoper
Thanks for working on this. And for providing tests! Would it be possible for you to break the patch down into smaller changes? That makes it easier to review for me. So separate the language target from content translation support. Else it would simply take me weeks before I fully reviewed and tested the patch (it may even be months as I have other things that need my attention too). To start, a patch that just provides the language target would be the easiest I think.

Ok then i will upload both features in different patches.

Here is a patch which allows make a mapping of language. Interdiff is from patch #9.

Here is the patch for content translation.

I have updated the summary to indicate which patches need to be applied in each case.

@mistermoper
Thanks, I hope I can take a look at these patches soon. It's very likely though that this needs more tests. In the D7 version of Feeds, there were a lot of tests for this.

See for example FeedsMapperMultilingualFieldsTestCase, which tested (among more) the following cases:

• If values from an other language are kept when not importing values for that language.
• If values for multiple languages at once can be imported.
• How values are imported after a language gets disabled. It may be that in D8 you cannot disable a language if configuration depends on it. I haven't checked that.
• How values are imported after a language gets removed. It may be that in D8 you cannot remove a language if configuration depends on it. I haven't checked that.
• When autocreating terms, make sure that the term gets saved in the right language.
• When importing empty values for a certain language, ensure that values for that language are wiped out (and not for other languages).

In #1183440-234: Multilingual Feeds - Make field import language-aware you can see how I tackled this issue for the D7 version.

Ideally, we would have above cases all covered. Else we could also mark language support as experimental. I don't want this issue sit here for another two years. Anyways, I need to plan in some time to review both patches.

I am starting with this.

There is, among others, one feature which is missing in these patches: allow set the language in the node processor, now is only possible by mapping. Is this required for a stable version?

Meanwhile, I will start covering this use case "If values from other language are kept when not importing values for that language." with field mappings.

@mistermoper
That would be great if you want to write a test for that!

About language set on the processor - the D7 version of Feeds had this. I think it would be useful to have in the D8 version as well. But not in this issue. Perhaps it would be better to split this issue up into three issues - one for each feature - and label this as a meta issue. The two features so far handled in this issue are already split apart into two patches, which is great as that allows us to focus on one feature at a time.

Seems #31 applies fine against alpha, but #33 requires 3.x-dev; haven't been able to get language name for Language in Mapper, after #31 has been applied though.
But after Feeds 3-x dev was installed, I've got "Class 'Drupal\feeds\Feeds\Target\Language' not found in Drupal\feeds\Entity\FeedType".
Hope it will help someone.

Hello,
It's not possible to apply a path.
I dont have a configure checkbox import languages.

It's possible to send me a complete working feed folder please ?

Thank you

Antho

Hello,

Here is my feedback.

The 2 patches need to be applied on a previous dev version (commit #72bed0603893ca814a159919d14cb7564b6d2a2e).

If you have composer you can do this by adding the patches in your composer.json file :

"drupal/feeds":{
  "[2829283-31] Allow set the content language":"https://www.drupal.org/files/issues/2018-11-01/feeds-language-target-2829283-31.patch",
  "[2829283-33] Allow translate content fields":"https://www.drupal.org/files/issues/2018-11-01/feeds-content-translation-2829283-33.patch"
}

And require the specific dev version :
composer require 'drupal/feeds:3.x-dev#72bed0603893ca814a159919d14cb7564b6d2a2e'

Tested ok on a term entity.
Tested on single formated textfield.

Have some others difficulties :
- need to duplicate the unique field in the other languages to avoid some SQL error.
- when update existing entity formatted text field, it doesn't replace the value but create another item in the fieldItemList despite the fact that the field is limited to 1 so the value is lost. Need to setValue of this on a hook_presave to fix it.
- Have some bad encoding characters éàù... need to utf8_encode the imported field on hook_presave.

Thank you all for your work !

Sorry for taking so many time to do this.

Attaching a new patch with some Tests to cover use cases #36, except for use case 'How values are imported after a language gets disabled. It may be that in D8 you cannot disable a language if configuration depends on it. I haven't checked that.' because there is no way to disable a language in Drupal 8. This changes made the code change as some checks were not working.

This tests is not prepared yet 'When importing empty values for a certain language, ensure that values for that language are wiped out (and not for other languages).
'.

Can you check this? Thanks!

Attaching a new patch, making the tests with the new Feeds API.

@mistermoper
Thanks for continuing to work on this. I'd like to review it, though I have other Feeds relates issues that I'd like to address first. These are:
#2971881: Plugins that depend on a tamperable item need to know the available properties
#2976180: Rewrite Plugin
#2799225: The "hidden" plugin does not exist: Drupal\Component\Plugin\Exception\PluginNotFoundException:
Once these are resolved, I can address this one.

I have tested the patch from #31 that provides a target to language.

I tested also with specifying a language that did not exist on my install and that import failed with a validation error:

langcode.0: The value you selected is not a valid choice.

Looks like expected behavior to me.

The patch has tests, so is good to go. So committed #31.

This issue remains open for content translation support. #42 has the latest patch for this feature.

Apparently, the patch in #31 caused some test failures. That's probably because I slightly changed the implementation for createFeedTypeForCsv() a few months ago. Corrected that. Also saw that the test used the deprecated 'entity.manager' service. Replaced that with the 'entity_type.manager' service on commit.

@mistermoper
Thanks again for your work! I finally started with looking at your patch from #42. Sorry it took so long.

Here is my code review:

1. +++ b/src/Feeds/Processor/EntityProcessorBase.php
   @@ -402,6 +404,26 @@ abstract class EntityProcessorBase extends ProcessorBase implements EntityProces
   +  public function isOwnerFeedAuthor() {
   ...
   +  public function getOwnerId() {
   
   @@ -477,6 +499,39 @@ abstract class EntityProcessorBase extends ProcessorBase implements EntityProces
   +  public function entityTranslationCheck(EntityInterface $entity) {
   ...
   +  public function ensureTranslationLabel(EntityInterface $entity, EntityInterface $translation) {
   
   +++ b/src/Plugin/Type/Target/FieldTargetBase.php
   @@ -77,12 +82,101 @@ abstract class FieldTargetBase extends TargetBase {
   +  public function valuesAreEmpty(array $values) {
   ...
   +  public function languageExists() {
   ...
   +  public function getEntityTarget(FeedInterface $feed, EntityInterface $entity) {
   ...
   +  public function ensureEntityTranslation(FeedInterface $feed, EntityInterface $entity) {
   
   @@ -246,4 +340,100 @@ abstract class FieldTargetBase extends TargetBase {
   +  public function isTargetFieldTranslatable() {
   ...
   +  public function isTargetTranslatable() {
   ...
   +  public function getLanguage() {
   ...
   +  public function showSummary($summary) {
   

   I don't think all these methods have to public. And the methods that do would need to be defined in an interface.

2. +++ b/src/Feeds/Processor/EntityProcessorBase.php
   @@ -796,28 +852,38 @@ abstract class EntityProcessorBase extends ProcessorBase implements EntityProces
   +        if (method_exists($plugin, 'getLanguage')) {
   +          $group = $plugin->getLanguage();
   +        }
   

   This requires the method getLanguage() to be defined in an interface. Then the code here can check if that interface gets implemented.

3. +++ b/src/Feeds/Processor/EntityProcessorBase.php
   @@ -796,28 +852,38 @@ abstract class EntityProcessorBase extends ProcessorBase implements EntityProces
   +        else {
   +          $group = 'default';
   +        }
   

   Maybe getting $group should be moved to a protected method? Further in the code getLanguage() is called again on a target plugin (which by the way misses the fallback to 'default' there).

4. +++ b/src/Feeds/Processor/EntityProcessorBase.php
   @@ -825,8 +891,9 @@ abstract class EntityProcessorBase extends ProcessorBase implements EntityProces
   +      $group = $plugin->getLanguage();
   

   A check for if the target plugin implements getLanguage() is missing here.
   Should have a fallback to 'default', I guess?

5. +++ b/src/Feeds/Target/FeedsItem.php
   @@ -34,7 +34,8 @@ class FeedsItem extends FieldTargetBase {
   +      $entity_target = $this->getEntityTarget($feed, $entity);
   +      $item_list = $entity_target->get($field_name);
   

   According to the implementation of FieldTargetBase::getEntityTarget(), that method could return NULL.

6. +++ b/src/Plugin/Type/Target/FieldTargetBase.php
   @@ -11,11 +11,16 @@ use Drupal\feeds\FieldTargetDefinition;
   +use Drupal\Core\Form\FormStateInterface;
   +use Drupal\feeds\Plugin\Type\Target\ConfigurableTargetInterface;
   +use Drupal\Core\Entity\TranslatableInterface;
   +use Drupal\user\EntityOwnerInterface;
   +use Drupal\Core\Language\LanguageInterface;
   

   Nitpick: the use statements should be in alphabetical order.

7. +++ b/src/Plugin/Type/Target/FieldTargetBase.php
   @@ -77,12 +82,101 @@ abstract class FieldTargetBase extends TargetBase {
   +        if (!empty($item_list->getValue()) && $item_list->getFieldDefinition()->getCardinality() > 1) {
   

   Checking the cardinality here makes me think the following: in the case that there are additional values, the values would get simply ignored without the user noticing?
   If it is to minimize failing imports then I agree that these can be annoying, but ignoring input without the user knowing is bad as well. Therefore I think that the cardinality check should be removed here.

8. +++ b/src/Plugin/Type/Target/FieldTargetBase.php
   @@ -77,12 +82,101 @@ abstract class FieldTargetBase extends TargetBase {
   +  /**
   +   * Check current field has value.
   +   *
   +   * This is done because we don't want set empty values to entity.
   +   *
   +   * @return bool
   +   *   TRUE if field has value.
   +   */
   +  public function valuesAreEmpty(array $values) {
   +    $empty = TRUE;
   +    foreach ($values as $value) {
   +      foreach ($value as $val) {
   +        if (!empty($val)) {
   +          $empty = FALSE;
   +          break 2;
   +        }
   +      }
   +    }
   +    return $empty;
   +  }
   

   It's completely valid to import empty values. Therefore I think that this method needs to be removed.

   Example: say you are importing user profiles and you are regularly updating these with subsequent imports. Then an user wants his phone number to be removed. You remove it on the source and then you expect it to be removed on the target site on the next import as well.

9. +++ b/src/Plugin/Type/Target/FieldTargetBase.php
   @@ -77,12 +82,101 @@ abstract class FieldTargetBase extends TargetBase {
   +  public function languageExists() {
   

   Missing docblock.

10. +++ b/src/Plugin/Type/Target/FieldTargetBase.php
    @@ -77,12 +82,101 @@ abstract class FieldTargetBase extends TargetBase {
    +   * Get entity , or entity translation to set the map.
    

    Nitpick: redundant space before comma.

11. +++ b/src/Plugin/Type/Target/FieldTargetBase.php
    @@ -77,12 +82,101 @@ abstract class FieldTargetBase extends TargetBase {
    +   * @param FeedInterface $feed
    ...
    +   * @param EntityInterface $entity
    ...
    +   * @return EntityInterface
    ...
    +   * @param FeedInterface $feed
    ...
    +   * @param EntityInterface $entity
    

    Interfaces/classes should be documented with their full namespace, according to the Drupal coding standards.

12. +++ b/src/Plugin/Type/Target/FieldTargetBase.php
    @@ -77,12 +82,101 @@ abstract class FieldTargetBase extends TargetBase {
    +  public function getEntityTarget(FeedInterface $feed, EntityInterface $entity) {
    ...
    +        $entity_target = NULL;
    

    getEntityTarget() can return NULL if the language does not exist, but that's not documented in the method. In point 5 I already mentioned that code calling this method is expecting an entity.

13. +++ b/src/Plugin/Type/Target/FieldTargetBase.php
    @@ -246,4 +340,100 @@ abstract class FieldTargetBase extends TargetBase {
    +  /**
    +   * ¶
    +   */
    +  public function isTargetFieldTranslatable() {
    

    Missing docblock.

14. +++ b/src/Plugin/Type/Target/FieldTargetBase.php
    @@ -246,4 +340,100 @@ abstract class FieldTargetBase extends TargetBase {
    +  public function showSummary($summary) {
    

    showSummary() doesn't entirely fit what the method does. It doesn't show anything. Perhaps formatSummary() would be better?

I haven't looked at the tests yet, but I plan to do so.

Thanks all for working on this patch! Working on a large Dutch project and this patch is really helpful to import Dutch/English content.

However, I found that the patch in #42 doesn't apply anymore to dev, so here's a re-roll against HEAD.

Also added an interdiff to keep track of changes

Hi, I have three languages Drupal 8 and taxonomy CSV in all three languages (Name_lang1, Name_lang2, Name_lang3). I try the #50 patch. Processed with success but I can view only terms in the original language. The translation tab contains other two languages translation, but if I click (view) the translation I get an error:

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">InvalidArgumentException</em>: Invalid translation language (fr) specified. in <em class="placeholder">Drupal\Core\Entity\ContentEntityBase-&gt;getTranslation()</em> (line <em class="placeholder">873</em> of <em class="placeholder">core/lib/Drupal/Core/Entity/ContentEntityBase.php</em>)

.

Any help will be appreciated.

This patch needs a reroll.

Additionally, there’s probably a bug in FieldTargetBase::setTarget():
+ if (!empty($item_list->getValue()) && $item_list->getFieldDefinition()->getCardinality() > 1) {
FieldDefinitionInterface doesn’t have a method called getCardinality(), which can result in a undefined method error as reported in #3090788: Unable to import file: undefined method getCardinality().

Also, I asked in #49 why the cardinality check is necessary:

Checking the cardinality here makes me think the following: in the case that there are additional values, the values would get simply ignored without the user noticing?
If it is to minimize failing imports then I agree that these can be annoying, but ignoring input without the user knowing is bad as well. Therefore I think that the cardinality check should be removed here.

I'm doing a reroll for now, still needs work as of #49.

Next step would probably be to port tests for emptying values (non-language related), to ensure that keeps working with the changes from the patch here.

Step after that would be adding test coverage for the bug reported in #3090788: Unable to import file: undefined method getCardinality().

I have tried the patch, when I try to save the form with language I get "The machine-readable name is already in use. It must be unique." and all machine name fields marked as the wrong one, even they have been generated and they look quite unique.
I have tried to replace them with really unique name I still have the same result.
If I don't choose any language it will save the same machine name correctly.

Patch 54 is failing on the Current feeds dev branch.

Same here, Patch 54 is failing on the Current feeds dev branch and Drupal 8.7.10

@camoa, @matthieu_collet
The patch in #54 is applying fine on the latest dev in my case. Could it be that you do have some other patches applied as well? Or do you mean that you get errors in the user interface after applying the patch?

Hi @MegaChriz

I have an error in composer, it says it's not applied, but it seems that it is and it works. And i don't have other patches for Feeds

I just now have an error with line 88 of FieldTargetBase.php

if (!empty($item_list->getValue()) && $item_list->getFieldDefinition()->getCardinality() > 1) {

Call to undefined method Drupal\field\Entity\FieldConfig::getCardinality() in Drupal\feeds\Plugin\Type\Target\FieldTargetBase->setTarget() (line 88 of /home/dkvision/staging/correct-immo.be/modules/contrib/feeds/src/Plugin/Type/Target/FieldTargetBase.php).

so I replaced "$item_list->getFieldDefinition()->getCardinality() > 1" by "0 > 1" so it's denied, and it works now, I imported content types in 2 languages

@ Matthieu: what did you replace exactly? Like this?

if (!empty($item_list->getValue()) && $item_list->getFieldDefinition()->getCardinality() > 1)

to

if (!empty($item_list->getValue()) && 0 > 1)

?

I've opened a spin-off issue for adding test coverage for clearing out values: #3103561: Add basic test coverage for clearing out values.

I also have the suspicion that the $item_list->getFieldDefinition()->getCardinality() bug is only exposed when mapping multiple times to the same target, so for example:

1. Map title_en to Node title with language set to English on the target configuration.
2. Map title_fr to Node title with language set to French on the target configuration.

My initial testing seems to confirm that suspicion. If so, then I think we need to fix #2994668: Values duplicate when mapping multiple columns to one multi-value text field first in order to add test coverage for this case.
@matthieu_collet, @fbreckx
Can you confirm that you map multiple times to the same target?

Hi @MegaChriz
Is there a reason that 'feeds_item' field type is not translatable?
We have an issue that nodes have the same feed_item value and it get overridden for each translataion import

@shkiper
The feeds_item field holds a reference to the feed entity and can only hold one reference at a time. So if you use multiple feeds to update the same node, then the feeds_item field get overwritten each time. I haven't diven deeply enough in the multilingual issue in order to answer the question why the feeds_item field is not translatable.

Hi @MegaChriz

Thank you for quick reply :)
Do you know any drawbacks of making this field translatable?

@MegaChris, that is correct! Mapping multiple times to same target.

I'm working on the tests to expose any bugs the current patch may have.

This patch revamps the test "TranslationTest". Among more, I made the following changes to it:

• I documented the tests and cleaned up code at several places.
• I ported the method testClearOutValues() from the class FeedsMapperMultilingualFieldsTestCase from the D7 version of Feeds.
• I added the yet to be ported methods testImportTranslationForExistingNode() and testAutocreatedTermLanguage() from the same D7 class and marked these as incomplete.

The tests no longer pass (at least locally).

Next steps are:

• Port the two newly added test methods.
• Update the code when #2994668: Values duplicate when mapping multiple columns to one multi-value text field gets committed (I'm currently testing that one).

No functional changes were made.

Hi,

I don't know if it's possible with this patch, but I have a feed that contains all my nodes, for both languages.

<node1>
--<langcode>FR
--<nodeID>123
--<title>Title FR
</node1>
<node2>
--<langcode>EN
--<nodeID>123
--<title>Title EN
</node2>

When I import this structure now, it ends up importing everything, but is doesn't link translations. It makes a new (untranslated) node per record. NodeID is set to unique, however. Does anyone have a clue on how to link translations?

@fbreckx
Does mapping source "langcode" to target "Language (langcode)" help? I see that in the source they are in capital letters, I haven't tested yet if that works. If you are already mapping to "Language (langcode)", you could try to convert the source to lowercase with the Feeds Tamper "Convert case" plugin. If you can confirm that lowercase works and uppercase doesn't, it would be a good idea to create a new issue (feature request) to let Feeds accept uppercase language codes.

@MegaChriz

I've used lowercase with tamper, didn't mention it. Targeting langcode to language doesn't work. It just creates a new node instead of translating one. I didn't set the language targeting in this scenario.

I will now try to split the feed (one for each language) and create two feeds with language targeting for each field.

UPDATE

Using two feed sources (one per language) and using two feed importers (one per language - with language targeting) doesn't work as well.
Upon importing the second source, the error says: A translation already exists for the specified language (fr).

This ports the tests testImportTranslationForExistingNode() and testAutocreatedTermLanguage(). They are not passing yet, so unless I made an error in writing these tests, there's serious work to be done to make the patch stable.

No functional changes were made.

I have tried many different combinations of mappings for en and fr sources of a very simple content type (title, one long text field, and unique ref text field).

The most common error I am seeing is the same as reported by @fbrekx: A translation already exists for the specified language (fr).

Please can someone who has had this successfully working provide an explanation of the mappings used, as I must be missing something?

Many thanks,

Reroll after committing #2994668: Values duplicate when mapping multiple columns to one multi-value text field.

This does remove the changes from EntityProcessorBase::map(), because that part is implemented differently in that other issue. By analyzing the code, it looks like the outcome is the same, but I haven't tested that yet. Let's see on the testbot if the amount of test failures is the same.

@qdoscc, @fbreckx
Sounds like a new error not covered by the automated tests yet. Does the issue only happen when using two feed types?

@MegaChriz: if I use 2 feeds, it doesn't link translations, but it creates 2 separate nodes instead (one per language, without translation).

@qdoscc, @fbreckx
I'm trying to reproduce the following error:

A translation already exists for the specified language

I tried the following to reproduce the error:

1. Installed Content Translation module.
2. Added two languages: Dutch and Spanish (with English being the default language)
3. Created a feed type with mapping to title, body and feeds item. Title and body set Dutch, feeds item GUID set as unique. Also mapped to title with default language.
4. Created a feed type with the same settings, except set title and body to Spanish instead of Dutch.
5. Created a feed based on the first feed type, imported Dutch values (content_nl.csv).
6. Created a feed based on the second feed type, imported Spanish values (content_es.csv).

I got values imported for both languages.

Note: when I tried to do a second import I got the following error:

Error: Call to undefined method Drupal\field\Entity\FieldConfig::getCardinality()

That is something to be fixed still.

Attached the feed types and sources I tested with.

@qdoscc, @fbreckx
Can you provide the exact steps to reproduce the "translation already exists" error?

I've created a small spin-off for this issue #3113960: Allow ConfigurableTargetInterface::getSummary() to return an array. It targets point 14 of #49. Since these changes don't depend on the multilingual feature, it is easier to handle it off separately to make future patches for this issue smaller.

As soon as a fix for that issue is committed, I'll create a reroll here. Will probably happen somewhere in the next few days.

I committed #3113960: Allow ConfigurableTargetInterface::getSummary() to return an array.

I've been puzzling a bit to get this done right. I'm not done with it yet, but I think it's getting better.

Here's what I've changed:

1. At the start of mapping - in EntityProcessorBase::map() - the target mapped to is emptied first. This happened on the entity in the default language. But if on the target a specific language is selected, the target should be emptied in that language instead. So I added a check for langcode and make sure the target on the entity gets emptied in the right language.
2. I added an interface called TranslatableTargetInterface, to get some of the public methods on FieldTargetBase in an actual interface. This addresses some of the points raised in #49.
3. I removed the cardinality check. I don't think we need it (point 7 of #49).
4. I also removed the valuesAreEmpty() as I yet fail to see why it's needed. And because it should be possible to empty values on the entity (point 8 of #49).
5. I think I noticed a bug in FeedType::removeMappings() (and FeedType::setMappings()): in FeedType::removeMapping() something on the property 'targetPlugins' is also emptied, but that didn't happen in FeedType::removeMappings(). This change should be probably be handled in a spin-off issue.
6. A few coding standard issues are resolved, but not all yet.

Locally, only TranslationTest::testAutocreatedTermLanguage() is failing, but I do expect there's more work to be done in this patch to get things correct. So probably some more automated tests are also needed. I do hope to get this issue done within a few weeks from now (ideally on February 29, but that may be too ambitious), so I may choose to postpone certain refinements.

I appreciate it if you want to test this patch on the latest dev, to catch some more bugs. Because the more bugs are catched now, the less likely I may need to make API changes (remove or replace methods) in future releases.

The interdiff isn't an exact interdiff from the patch in #72 as a reroll was also needed because of committing #3113960: Allow ConfigurableTargetInterface::getSummary() to return an array.

Adding this as a beta blocker.

Patch 76 applied but I can't test if it's worked or not.
Because it doesn't save changes of configure options. (like Autocreate terms(yes/no) or changing language option)

@MegaChriz sorry I've been away from this for a few days. Thanks for your continued work on the issue. I will test this week based on your instructions above and let you know my results.

One thought I had was that in my case I want to import translations of content that already exist on the site (the original language content is not coming from a feed). I am exporting it with views data export to CSV, then the translators will provide a translated CSV for each language. So the question may be how it works in terms of the unique mapping seeing that we cannot select node ID as a unique field (different from D7 where this was possible).

@toprak
Thanks for reporting. I identified that some changes that I made to FeedType in #76 caused this. I reverted that part for now, although I think it is needed to let TranslationTest::testMappingFieldsAnotherLanguageImport() pass.

@qdoscc
Great that you want to test the new patch soon! As a workaround, can you use an other field on your content type as unique identifier? Anyway, there's an issue open for mapping to entity ID, though no solutions for it yet: #2989279: Add a target to entity ID

Locally, I managed to get all tests from TranslationTest passing! I hope the tests pass on the testbot too.

Here's what I changed:

1. On EntityProcessorBase, I removed (among more) the methods entityTranslationCheck() and ensureTranslationLabel(). It seems this code was working around a problem where you don't map to the label of a translation, since the label field is required most of the time. I'm not sure if this should be Feeds responsibility. I figure if Feeds should prepopulate values of a translation, it should not only prepopulate the label, but all values. Because there can exist more required values on the translation.
2. Also on EntityProcessorBase, I removed the methods isOwnerFeedAuthor() and getOwnerId(). This was used by FieldTargetBase to prepopulate the author on the translation. Since Feeds does set an author on new entities (if configured), I think it's reasonable if Feeds would do the same for translations. I did move the code doing this from FieldTargetBase::ensureEntityTranslation() to EntityProcessorBase::getEntityTranslation() and put that method right under EntityProcessorBase::newEntity(), so it's easier to see there's some similarity between these two methods.
3. When creating a referenced entity, the language of the entity is set to the language configured on the target. The D7 version of Feeds does that as well for taxonomy reference targets and it makes the test TranslationTest::testAutocreatedTermLanguage() pass.
4. Test fixes for TranslationTest::testMappingFieldsAnotherLanguageImport() and EntityProcessorBaseTest::testMapWithEmptySource().
5. I think I fixed all of my concerns from #49
6. Code style fixes.

This patch could be ready for commit, if it doesn't break tests or introduces new code style issues.

Great, all test pass! Some code style issues still exist though. Fixing these in this patch.

Did some testing with the latest patch. After running into the A translation already exists for the specified language error already reported here a few times, I followed the mapping setup suggested in #74 which suggests to also map title to the default language. Only then I got the two importers working together.

The differences of my setup compared to that example is that I have 2 site languages: English (default) and Finnish, and a translation (EN) may not always exist in the source. This poses a problem, since the first importer (FI) will have already created both EN & FI versions due to that title mapping. Is there any way around this? I've tried mapping variations without the default language title and adding langcode mappings but that just leads back to the A translation already exists.. error.

Edit: Also, even after the EN importer runs and sets the correct title upon the next run of the FI importer it would just get rewritten which I'm guessing is related to this issue - https://www.drupal.org/project/feeds/issues/3105322.

@artis.bajars
Thanks for testing! Can you explain the exact steps to reproduce the issues you encounter? I understand you are using two feed types, but I find it hard to parse from your description how these are configured. You mention "langcode". Do you refer in this case to the mapping target "Language" or to the language configured on the mapping target "Title"?

To productively reproduce an issue, I need a numbered list of steps, preferable taken from a clean Drupal install. The list would probably start like this:

1. Installed Drupal with standard profile in English.
2. Enabled modules Feeds, Content Translation.
3. Added Finish language.

And so on.

Sorry, if my original description was unclear. Here's the steps to follow in my scenario:

1. Install Drupal, standard profile, default language set to English
2. Enable Language, Content translation and Feeds module (with the patch from #82)
3. Add Finnish as 2nd language, enable Basic page content to be translated
4. Create a Feed type with mappings to feeds item, title and body. Title and body set to English
5. Create an identical Feed type with title and body set to Finnish
6. Create a feed for each feed type with simple csv source files

Now we have two options:

A: Try to import FI values first. Result - "Import failed. Title (title): This value should not be null."
B: Try to import EN values first, FI after. Result - works as expected, the node has values in both languages.

Additionally I also tried to:

1. Add a langcode field to to the csv source files with values "en" and "fi"
2. Add mappings to the target Language in each feed type
3. Try to import FI values. Result: "Import failed. A translation already exists for the specified language (fi).

So it appears this only works if the feed that matches the site default language is imported first. In a situation where a source item only exists in the FI data but doesn't have a matching item in the EN source it seems that it's not possible to import it. The only way I was able to import the FI values first was to add another Title mapping to the FI feed type only set to default language. That of course also causes the EN language version of the node to be created and oddly shown as the "Original language" when checking the translations tab. I would kinda expect to be able to create the FI language version as the original without the requiring the EN one.

@artis.bajars
Thanks for your detailed report! This is useful. I hope to work on it soon.

Hi,
I am trying to import terms from a .csv to my vocabulary on my Drupal 8.8.3 site with Feeds 8.x-3.x-dev (2020-Mar-01)

- I prepare a feed type for the terms: name (unique) plus language code for my primary language (german)
- I prepare a feed type for the terms: name (unique) plus language code for my secondary language (english)
- I run the import from my .csv with the names and langcode=de
- I run the import from my .csv with the names and langcode=en
- Finally I get the german and english terms imported.
- But they seem not to be connected - when I click on translate no translation of the term is shown.

Did I miss something or do I take care of special parser settings?

Thanks for any help.

hi inst
you have to make one .csv file with all the language, and make just one import, each translated field tree times in the import (once per language)

so they will be connected

Hi Matthieu,
Ok so my .csv Looks like this:

Column1 Column2
Frankreich France
Italien Italy

How should I configure the Mapping?
And what do you mein with 3 times?

Thanks!

you attribute "Column1" to title, in options you choose "German"
you attribut "Column2" to title (also) and you choose "Français" in options

like this screenshot

that's all

Thanks again. -

OK I think this way doesn´t works for taxonomy terms.
Here all the fileds I can map. None of them is able to be configured by language.
I attached a screenshot.

yes you also have this possibility with taxonomy terms

Hi,
ouch - I see.
I don´t have this language tab where I can select the language. Strange.
Do you use the latest feeds 8.x-3.x-dev on Drupal 8.8.3 ?
And do I have to use patches on this feeds 8.x-3.x-dev version to make the language tab visible?

merci!

yes

Drupal core 8.8.3
feeds 8.x-3.x-dev
patch https://www.drupal.org/files/issues/2020-02-27/feeds-content-translation...

you didn't patch? did you read this issue ?

Good Morning Matthieu,
ooh - sorry I missed this.
I applied the patch and saw the language select.
It works now - super!!

Thanks again for your help and patience.

Import values for non-default language first

This patch allows you to configure the initial language on the processor. This should address case A reported in #85. If you import content in a non-default language, set the language on the processor. This is what the D7 version of Feeds also does.

@artis.bajars
Can you confirm you can import the FI values first with this patch?

A translation already exists for the specified language

I think this does not solve the error "A translation already exists for the specified language". I believe this to be an issue with the language Feeds target. Can everyone that had that error confirm this error only popped up when mapping to "Language (langcode)"?
Since the goal is create a new Feeds release in March, I propose to postpone this issue to a follow-up.

Appreciate anyone who wants to test this patch!

Updating EntityProcessorBaseTest and fixing coding standards.

Thank you very much for your excellent work on this. I have carried out the following test in order to import a French translation of content already existing in the site in the default language (English):

Patches applied:
#99 from this issue
#14 from Use Number fields as unique mapping target
#9 from Add a target to entity ID

Feed configured:
Upload file of CSV to Node processor.
Processor settings: Language French, Update existing contents

Source (CSV column) -> Mapping Target:
nid -> ID (nid) *Unique
title_fr -> Title (title) *Language: French
alias -> URL alias (path) *Language: French
my_field -> My Field (field_my_field) *Language: French

Results:
- French translations of nodes are created correctly without any errors
- Title and my_field are populated as expected
- Alias is not set (but I expected this as feeds showed it as Read only on the mapping... is there another way?)

Conclusion:
It works really well for my purposes! For now I will use URL alias patterns as a workaround for not being able to import the aliases.

Note:
As I was working on an existing installation with feeds already set up, I found it necessary to do things in the following order:

1. remove all existing feeds and feed types
2. uninstall the Feeds module
3. install the patched Feeds module
4. create the new feeds types

If I did not do the uninstall/reinstall then I encountered an error such as:
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'mysite_sql.feeds_clean_list' doesn't exist: DELETE FROM {feeds_clean_list} WHERE feed_id IN (:db_condition_placeholder_0); Array ( [:db_condition_placeholder_0] => 14 ) in Drupal\feeds\Entity\Feed::postDelete() (line 556 of /home/mysite/drupal/web/modules/feeds/src/Entity/Feed.php).

@qdoscc
Thanks a lot for testing!

Alias is not set (but I expected this as feeds showed it as Read only on the mapping... is there another way?)

#2989279: Add a target to entity ID exposes read only fields. I wasn't aware that it would also expose path alias as read only field. Read only fields can only be set once as else that causes issues for entity ID. Sounds like that needs more refinement. But you can import aliases with a separate feed type, using the URL alias processor (I hope, did not test that myself). Since D8.8 a path alias became a separate entity type in Drupal core, thus as a result you could no longer import it in one go with nodes.

If I did not do the uninstall/reinstall then I encountered an error such as Base table or view not found: 1146 Table 'mysite_sql.feeds_clean_list' doesn't exist

Did you run database updates after updating to the latest dev? Because feeds_update_8002() is supposed to add that table when it doesn't exist yet. If you remember that you did run database updates then there might be an issue with that update function.

Did you run database updates after updating to the latest dev?

No I didn't! That makes sense then :)

I noticed that for some mappers on the mapping form a wheel icon appeared, but when clicking it you got an empty form.

This is for example the case for the feeds item target for which no language selector is available:

The reason is that now all field targets have become "configurable", because of implementing ConfigurableTargetInterface.

So I checked core's EntityDisplayFormBase to see how that handles configurable items and it appears that it's checking for each plugin if the settings form method is returning something:

// Generate the settings form and allow other modules to alter it.
$settings_form = $plugin->settingsForm($form, $form_state);
$third_party_settings_form = $this->thirdPartySettingsForm($plugin, $field_definition, $form, $form_state);

if ($settings_form || $third_party_settings_form) {

Feeds did only check if a plugin implemented ConfigurableTargetInterface. I now applied similar logic as above to determine if a wheel icon should be displayed for a target.

In comparison of the patch in #99, this patch only introduces changes on the mapping form. There are no changes for the multilingual import functionality.

I noticed the following error on the mapping page when mapping to an entity reference field:

Notice: Undefined variable: delta in Drupal\feeds\Feeds\Target\EntityReference->buildConfigurationForm() (line 448 of modules/contrib/feeds/src/Feeds/Target/EntityReference.php).

The reason is that Feeds now calls buildConfigurationForm() when no target settings are open, so the following code does not result in $delta being set.

foreach ($form_state->getValues() as $key => $value) {
  if (strpos($key, 'target-settings-') === 0) {
    list(, , $delta) = explode('-', $key);
    break;
  }
}

Fixed the notice by setting $delta to 0.

There are no changes for the multilingual import functionality.

Hi MegaChriz

on which version of feeds exactly did you apply the patch ?
with 3.0.0-alpha6 I have the composer error :
Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2020-03-25/feeds-content-translation-2829283-104.patch
without any details

thank you

matthieu

@matthieu
The patch is created for the latest dev version, so it doesn’t apply on alpha6. I hope to release alpha7 soon and I hope this could be part of the new release. In the next few days I decide whether this is good enough for commit.

thank you!
problem was there, it works now

Minor code cleanup. No functional changes are made.

Good. Tests still passing :)

Committed #108! Thanks everyone.

I created a follow-up issue for fixing the "A translation already exists for the specified language" error:
#3126351: A translation already exists for the specified language

Thank you Megachriz, this thread saved my day:-)

Hi,
Translation works fine but only default language path alias is created.
I have product in 3 languages. italian is default language.
After import i have only path alias of italian product.
All path alias are set with patterns, one for language.

Thank you