Currently there is no translation integration for IEF. I believe that this feature is crucial and needs discussion/work.
There is an issue for the D7 version of IEF: https://www.drupal.org/node/1545896
Proposal:
For new entities added with IEF:
Create nodes in parent node's language. If parent has no language set (because it has not been created yet), set language to undefined and iterate over all children to update their languages when parent is saved.
For entities which are referenced via IEF inside a parent entity:
Try to load referenced entity in parent's language. If there is no entity in the required language, load entity in default language and add a translate button next to it (right next to edit/remove buttons). Allow inline translation.
Once the entity is translated, hide the translate button.
What do you think?
| Comment | File | Size | Author |
|---|---|---|---|
| #55 | 2494959-55-add-translation-support.patch | 18.28 KB | bojanz |
| #54 | 2494959-54-add-translation-support.patch | 20.68 KB | bojanz |
| #47 | interdiff-2494959-36-47.txt | 2.45 KB | mikgreen |
| #47 | load_correct_existing_translation-2494959-47.patch | 7.54 KB | mikgreen |
| #37 | load_correct_existing_translation-2494959-36.patch | 6.71 KB | mikgreen |
Comments
Comment #1
bojanz commentedThis is super important.
Agreed.
I don't see why we need the translate button, if the parent entity is being translated, and the reference field is translatable, we can just start translating the referenced entities, no?
Comment #2
woprrr commentedI agree that, and in the single case add a button seem to be complex for ui... is better to start translating if the entity_reference is translatable.
This is the best way :D Good job.
But I think we'll have the same kind of problem that I face on https://www.drupal.org/node/2534408 outcome. We should perhaps consider this a method to simplify / debuger submit this part with a huge loop through the twigs, I analyze at length and I think it lacks a stage to say whether the entity is not therefore create author then puts the current user. This would be the same principle as for this issue, which aims to put the language by default if the parent entity has no language.
Comment #3
woprrr commentedComment #4
agoradesign commentedAny progress on this issue? Does anyone have some kind of unfinished patch file? At least the existing entities should be loaded in the parent's language... Of course I'm willing to help/contribute
Comment #5
agoradesign commentedFirst step towards translation integration: this patch loads the correct translation, if a translation for the given language already exists. So actually creating a translation for a given entity still has to be done, but at least one can now edit the correct translation.
Comment #6
miro_dietikerBoth highly confuse me.
We have ported paragraphs module for D8 and covered the most common use cases in translation.
If you have entity translation enabled then the parent entity is multilingual and has certain translatable fields.
Similarly the child entity is multilingual and has translatable fields.
As a result, the parent points to the very same child entity which means the reference field is NOT translatable.
Chances are high that a user wrongly makes the entity reference field translatable because he does not fully understand how it works. With paragraphs we decided to make this use case perfect until we ever think about supporting translatable references. We even discussed how we can prevent the user from doing this or at least put a warning.
And yeah, i warned in the beginning that supporting translations is complex and there are many edge cases to cover and everyone disagreed with comments like with D8 things should be super easy. But we still discover special cases whenever we use it.
The paragraphs tests might give a hint about relevant cases / workflows.
Mind that with IEF things are much more troublesome than with Paragraphs, since the (component) entities could be edited separately and through this e.g. the source language might be out of sync!
Comment #7
agoradesign commentedThat's right. But currently we have one problem with IEF:
Imagine, you have created a node in German, having a reference to another translatable entity, which also is created in German first automatically. Next, you wan to translate the node to English. Of course you still have the reference to the same entity. And of course the entity is translatable per se, but at this point it has no english translation. So, when create or edit the english translation of the node, and edit the referenced entity per IEF widget, than you still would modify the german version instead of creating an english translation without knowing it. That's the big problem.
Once you have the english translation of the referenced entity, my patch in #5 ensures that the correct language is loaded within the IEF widget.
I disagree with that. It really depends on the use case and your customer's needs. E.g. we are currently working on 2 multilingual D8 sites, both having lots of entity references. One site has translatable reference fields to offer true internationalization, allowing to have differently structured content per language and not to force to translate and keep every single field of the source language or add own content. The second site however is meant to have really a strict 1:1 translation of german and english. In this case we decided to have language neutral reference fields, but translate the entities itself.
Comment #8
agoradesign commentedHere's an updated patch, which...
Thanks to miro_dietiker - I've looked into paragraphs module, how this is solved there.
This patch solved our problem for the common scenario: node is translatable, has untranslatable entity reference field, the referenced entities again are translatable.
I have not tested any other scenario: reference field translatable, no translation at all,... Also note, that I've introduced a $target_langcode variable besides the $parent_langcode variable, and most uses of the parent langcode have been replaced by target_langcode, which takes its value from the $form_state (like paragraphs does). I'm not sure, if this is always correct.
Again: it solved our problem with the described scenario, but it's far from RTBC: code reviews, both manual and automatic testing are necessary. But I think, it's a great start and hopefully a big help for everyone, that needs IEF in multilingual pages urgently :-)
Comment #9
agoradesign commentedsmall update of my patch: now trying to initialize the target language with the language information from the form state - this fixes a bug, that was raised, when you tried to add a new entity on editing the node translation, because the wrong language was set for the new entity
Comment #10
agoradesign commentedI'll change status to needs review now. We experienced no problems so far. Time for others to try ;)
Comment #12
agoradesign commentedRe-roll patch against current dev
Comment #13
slashrsm commentedIt would be very beneficial to add a test to this patch. It would show how this is supposed to work and make sure we are all on the same page WRT that.
I think that we need to support translatable/non-translatable fields in the long run. There are use cases for both. Patch is a great start, but is missing huge part, which was already mentioned in #8.
Add descriptions for arguments and return.
Comment #14
idflood commentedSince I'm particularly interested in untranslatable entity with translatable entity reference I tried to implement a little change. This workflow is useful when the content is not the same between translations.
Before this change the entity reference would still point to the same entity between translations. So when translating a page if the user also modified the content of the entity reference it would also change the content of the original one since it was still the same entity.
Comment #15
miro_dietikerThank you for your update.
I would highly recommend to start ASAP with test coverage for what was discussed and improve it with every update when we cover more cases.
Comment #16
martijn houtman commentedThank you for this! I have tested it with the Paragraphs module, and this seems to work nicely. Will test some more!
Comment #17
agoradesign commentedregarding patch #14:
You'll need a specific if-condition in your added else-branch. It may work for your use-case, but for every other use-case this causes a big problem. No matter, if you have a single-language page or using a non-translatable reference fields pointing to translatable entities, you'll always get duplicates of your referenced entities on every save action.
We were wondering today, because an existing site behaved oddly, creating duplicate entities on every node save. We suspected that one of the last updates we made, must be guilty. After having the same problems on a fresh and clean installation, we tried different versions of core and IEF. And finally we saw, that using the patch from #14 instead of #12, was the root of our problem!
Comment #18
paulmckibbenAn additional problem:
When creating a new translation for content with an inline entity form, if you change the source language for the translation, you would expect the inline entity references to change to match those of the new source language, same behavior as other fields. However, they continue to match the first source language.
For example, suppose your site supports three languages: English, French, and Spanish. The default site language is English.
You have a node that has a multi-valued inline entity form, and this node was originally created in English, and then translated to French.
You would like to translate this node to Spanish, using the French version as the source.
You click the link to create a new Spanish translation. By default, the source is set to English, and all fields including the inline entities have the English versions pre-populated.
You change the source language to French.
You observe all the fields EXCEPT the inline entity reference fields change to match the French version.
Expected behavior: in this scenario, the inline entity reference fields should change to match the source version.
Comment #19
agoradesign commented@paulmckibben: I'm not sure, but I guess that is not IEF behaviour but core's. Have you tried the same scenario using one of core's entity reference field widget, not IEF, e.g. a select list?
Comment #20
tedbowThe patch needs a re-roll if anybody has time.
This has been marked as a must have for 8.x-1.0-beta1
Comment #21
agoradesign commentedHere's the re-roll... I did some quick tests with the complex widget only. Still works as expected for me.
Apropos tests.... we still need some automated tests, If I found time, I'd would contribute some. Unfortunately tonight and the next days it won't be possible for me - maybe on friday afternoon, but I can't promise now...
Comment #22
tedbowSetting to needs review to see if this breaks currents tests
Comment #23
tedbow@agoradesign or anyone else can someone explain to me in steps the expected behavior and how I can test manually and/or write tests.
I have time to work on IEF this week.
Comment #24
agoradesign commentedSounds good.. ok, let's start with all possible and legitimate use cases, that may occur. From my point of view I'd say, that all that things have to be covered: to add and edit referenced entities...
I'm not sure, if you should also address comment #18, as I already mentioned, that I'm not sure, if this is really a IEF problem/bug or a core bug or maybe normal/expected core behaviour.
I'm also not sure, if other constellations, like parent entity untranslatable, field (of course) untranslatable, but referenced entity translatable, make sense and should be tested too.
But anyway: these are just the use cases that come into my mind and make sense for me (and especially the first example is that, what I would expect). This maybe incomplete as well as incorrect - I'm no maintainer of IEF...
And don't forget: There are two widget types: InlineEntityFormComplex and InlineEntityFormSimple -> so every use case has to work in both variants.
Comment #25
agoradesign commentedRegarding manual/automatic testing: of course it would help and is important, if someone can confirm, that everything works expected after instensively testing everything manually, but the most important thing now would be to have a high as possible automated test coverage, in order to have everything well documented in these tests and to ensure that no future change breaks the functionality.
I don't know, if you have experience with writing automated tests (in Drupal or at least in PHP in general). The best thing is always to look at existing examples. It's always a good choice to look into core test classes. In this case, you should of course have a look into the existing tests in IEF - you should either add functions to the existing classes (probably the best) or add your own classes for multilingual test cases and still look at the existing tests and how they are written (http://cgit.drupalcode.org/inline_entity_form/tree/src/Tests?h=8.x-1.x)
Maybe you could also benefit from looking at the tests in Paragraphs module (http://cgit.drupalcode.org/paragraphs/tree/src/Tests/ParagraphsTranslati...)
Comment #26
loopduplicate commentedIn comment #24 it says:
Here's some ideas I had. Coffee is still kicking in so my apologies if the ideas are stupid:
Option 1: The referenced entity should not be cloned automatically. The form should show the fields of the referenced entity. When editing the form, the referenced entity fieldset should be styled differently with messaging that conveys the inability to translate the fields.
Option 2: Have a setting in the admin UI for this and let the site administrator choose whether or not to clone in this situation.
Option 3: Disable the field on the edit form and have two buttons. One button would say "Clone" and the other button would say "Edit". If the editor chooses "Edit" then they are allowed to edit the source values. If they choose "Clone" then the entity is cloned.
Regards,
Loop
Comment #27
reekris commentedThe patch in #21 is no longer applying against the latest dev version of IEF. Would it be possible to get this first version of translation support merged and then continue with further improvements?
Comment #28
bojanz commented@reekris
The patch isn't mergeable without tests.
Comment #29
bojanz commentedWe also need to replicate the functionality of this alter:
Comment #30
reekris commentedHere is a reroll of patch #21 until the tests are added
Comment #31
tedbow$parent_langcode var has not been defined at this point?
InlineEntityFormComplex and InlineEntityFormSimple have same 14 lines to get the new entity translation this should be refactored into a new method on InlineEntityFormBase
Comment #32
tedbowHere is an updated with a new function InlineEntityFormBase::getEntityItemTranslation which refactors the current logic that is that same between the 2 widget classes.
Comment #33
agoradesign commentedMaybe we could completely merge $parent_langcode and $target_langcode. Like I stated in #8, I was introducing the $target_langcode, as I needed this information, and I was unsure about possible side effects of completely changing the $parent_langcode value instead. So I took the more careful way to introduce a new variable instead. I fear, I'm not that well inwrought into IEF to decide this, but imho we should try to reduce this to one variable, if possible.
Before refactoring, the tests should be created first, as the current patch is at least tested by hand and already in production use on some sites.
Comment #34
paulmckibbenThis is looking better and better. However, I have one issue with translation: when I create a new translation of a parent entity, alter nothing, and save the translation, the child entities (using inline entity form) do not have corresponding translations--they still are only in the source language. All fields, including the entity reference, are translatable. Expected behavior: if I create a translation of the parent entity, the child entities should automatically have a translation to the same language if it does not already exist.
Note: I can work around this by manually editing each child entity.
I understand that an argument could be made that other entity references are not translated automatically, so why should IEF be different? I contend that since the typical use case of IEF is as a means for creating a composite field, that it should be treated the same as other translatable fields, and translations of the entities should automatically be created as long as the field itself is marked as translatable.
Thanks for the good work so far!
Comment #35
mikgreen commentedReroll of patch.
Comment #36
berdirWe recently spent a lot of time in paragraphs to improve the multilingual handling, you might want to compare with #2698239: Multilingual workflow bugs. There are special cases when for example the language of the host entity is switched.
Comment #37
mikgreen commentedI think $parent_langcode is irrelevant, instead we should use $original_langcode (original language of entity).
This fixes the case where entity is added to translation of main node, and then entity is translated in the original node language.
In this case $parent_langcode and $target_langcode are both "en" which causes crash.
Comment #38
agoradesign commentedCould you provide an interdiff please?
Comment #39
mikgreen commentedThe old patch from 32 does not apply anymore to current dev.
I don't think I can create an interdiff in that case, can I?
Patch code was not changed. It as just adapted to latest dev.
Edit: No, I replaced $parent_langcode with $original_langcode as I mentioned before.
Comment #40
mikgreen commentedHere is a use case I'm working on:
If reference field is not translatable (which means I want to add content to each language independently), but subentity is translatable. E.g. I can add different IEF entities to each translation of parent node.
Do you think it's a valid use case?
This is currently not working with this patch.
Comment #41
agoradesign commentedSorry being unprecise. Yes, I was interested in the diff between your patches. If $parent_langcode -> $original_langcode is the only difference, I guess it's ok without interdiff (although they are always very handy)
Regarding the use cases: Maybe I misunderstand, but it sounds, you've mixed a little bit the scenarios.
I believe, that the default use case is to have the parent (node) entity and the subentity translatable, but the reference field not translatable, like you mentioned. And IEF has to take care, that always the right subentity gets edited/created/loaded.
Example:
BUT of course IEF must also be able to handle other use cases, where the reference field is translatable. In this case, you can add different entities to each translation, independent from each other.
In my original patch I was concentrating on the first use case, however the second should have also worked correctly. It's a good idea, like I mentioned in a previous comment, to simplify and reduce the different langcode variables. Like berdir mentioned in #36, we should orientate on Paragraphs, how the translation handling was solved there.
At the end, we'd need a couple of unit tests to ensure that every possible use case is working as expected. However, every working patch would be a good start and great help for all users of IEF.
Comment #42
agoradesign commentedOf course there's still work to be done, but I'm setting the status to "needs review", in order to have the existing tests executed
Comment #45
mikgreen commentedYou've described it very elegantly in #41.
The main use case where parent node and entity are translatable, but reference is not - I think it works very well now.
What I'm trying to do it is the other use case. Where reference is translatable. I think it also works for the most part.
The only problem is with entities translated from original node (when node translation is created), those become translations, not independent entities.
Meaning they will be deleted from all languages when deleting from one.
I agree about adding tests and taking experience from the Paragraphs fixes. But for now I just want to get it working.
Comment #46
agoradesign commentedOk, I believe in the 2nd case, it would be best to always clone the referenced entity, so that it really becomes a separate entity, not only a translation.
I believe, that most of the time, when you make the field translatable, the referenced sub entity often won't be translatable at all, as it is a sub entity of an already translated field, and therefore its own language does not matter at all, until you display the same entity additionally somewhere else on the site independently from the parent node. But this is really an edge case. I've thought about this and still can't find a realistic example that makes sense at all.
@idflood in #14 tried to modify the patch to cover this use case. However it completely broke use-case 1, like I explained in #17. Maybe it helps you anyway - but the better role model is for sure Paragraphs.
Absolutely agree, but I'm still hoping, that anyone except bojanz (please keep working on Commerce checkout ;-))) ) soon finds time for the tests too. The permanent changes and refactoring over the last couple of weeks leaded to so many necessary patch re-rolls. I hope, that we can achieve the final solution soon. Thanks to you I didn't have to do that this time... ;)
Comment #47
mikgreen commentedOk, I think this does the trick. At lease I've tested both cases.
If reference field is translatable, we clone current entities when translating original node.
Comment #48
mikeryan@mikgreen, did you mean for this to get tested?
Comment #50
miro_dietikerJust a hint, we have been working on making Paragraphs ready for a stable release and learned that we were wrong again after multilingual support was "completed"...
We thought we have great test coverage and covered "all" multilingual cases when many issues went green.
Then we realised that all multilingual consistency, revision writing, validation, deep nesting, and many more were still wrong and needed to add tons of fixes and improve test coverage even more, eating even multiple weeks of developer time again.
So even after extensive review, expect that there will be many follow-ups and hidden areas in multilingual support and something stable needs multiple iterations.
Comment #51
mikgreen commented@mikeryan thanks.
We still need to work on the patch.
Need to see what we can gather from Paragraphs translation experience.
Need to write tests.
Comment #52
agoradesign commentedIt's unfortunately too late for the current versions, but imho it would be a great idea for future versions (D9 or different D8 branch) to refactor both codebases and remove redundancies. Both need to do the basically the same form handling stuff. Both run into the same hurdles and problems. A well working IEF could be the base for Paragraphs, which add their own forms, extending the basic IEF one
Comment #53
bojanz commentedWorking on this.
Comment #54
bojanz commentedInitial version, written from scratch after reviewing the Paragraphs implementation.
Notable pieces:
1) TranslationHelper::prepareTranslation() ensures that the inline entity is being edited in the right language
2) When translating, the Complex widget hides the add and remove buttons, you can only edit what's already there.
3) When translating, the shared (non-translatable) fields are hidden from the inline entity form.
4) The complex widget enters the translation mode only if at least one of the allowed bundles has translations enabled
5) InlineEntityFormBase::removeTranslatabilityCue()
Automated tests in progress.
Known issue: Inline entities are created with the wrong langcode if you create the parent entity with a non-default language (by choosing a different language in the dropdown).
Comment #55
bojanz commentedCommitted a piece in http://cgit.drupalcode.org/inline_entity_form/commit/?id=89584bc.
Fixed the issue mentioned in #54.
Here's the latest version of the patch.
Comment #57
bojanz commentedAdded tests and a few tweaks, committed. See you in followups :)
Comment #58
bojanz commentedComment #60
xanoI'm still researching this, but it looks like this makes IEF widgets fail if the 'parent' entity is newly created in a language other than the site's default.
Comment #61
xanoPatch uploaded to #2747781: Entity reference translation breaks adding new parent entities in non-default languages.
Comment #62
rp7 commentedI have created a follow-up issue for the known issue mentioned by bojanz in #54: #3218441: Inline entities are created with the wrong langcode if you create the parent entity with a non-default language.