Support asymmetric translation in experimental widget

I just tested patch #122 with unpatched paragraphs_asymmetric_translation_widgets - it seems to work very well. Did not notice any issues yet. Will report if I spot any.

I hope a maintainer can finally review #122. It should not have any negative effect. I'm not just claiming this. Proof:

if ($host->isTranslatable()) {...

Usually, the host paragraph should be set to untranslatable. Only with async paragraphs, the host is set to translatable. That means, with normal synced paragraphs, this won't run any additional code.

Next:

$this->isTranslating

is replaced with

!$this->allowReferenceChanges()

however allowReferenceChanges is modified by the patch to:

!$this->isTranslating || $this->supportsAsymmetricTranslations();

and supportsAsymmetricTranslations always returns "false".

Thus, isTranslating is effectively changed to:

!(!$this->isTranslating || false)

This can be simplified to:

!(!$this->isTranslating)

This can be further simplified to:

$this->isTranslating

Which is what the original code was.
Conclusion: Patch #122 literally does not change *ANYTHING*. It is *mathematically* equivalent to what is there before. The risk of including this should be zero.

#122 has one case not covered: when adding a new translation to a node and reusing the same paragraphs, this will result in an issue with functionality that relies on the langcode of the associated field, for example: linkit with “Linkit URL converter” filter enabled, will generate URL aliases in a wrong language.

HTR:

• Add paragraphs with long_text fields.
• Make the paragraphs and underlying fields not translatable.
• Setup entity revision reference field on a node and make it translatable.
• Create a node with paragraphs.
• Add a translation to the node and just save.
• Look in the database, look up the associated paragraph in “paragraphs_item_field_data” table.
• You will see two rows with the same id and revision_id but with different langcode.
• Also the associated long text field in the DB, will give back a wrong langcode.

I updated patch #122, by moving the logic where the langcode is being set when the host entity is translatable, to a condition which covers both cases (adding new paragraphs or reusing them from translations).

The last submitted patch, 136: paragraphs_support_asym_translations-2904705-136.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Patch #122 is _not_ enough on its own to restore the ability to add/remove/drag paragraphs field items around per translation. In fact I'm not sure what it means to accomplish. Using just D10 + Paragraphs 1.6 + Content Translation (and marking all fields as translatable) results in no visible difference in user experience.

Is it meant to be used in combination with other patches/modules/versions?

Patch #128 works essentially the same as the paragraphs_asymmetric_translation_widgets module. It restores all edit functions to translated node paragraph fields. Both 128 and the spin-off module work on their own.

Also, none of these solutions work with TMGMT jobs. Because ¶s are cloned, they never get the accepted translations from a job. They just get the base values copied from the base translation.

@amaisano: #122 is meant to be used in conjunction with paragraphs_asymmetric_translation_widgets. It has not functionality on its own.

I'm using patch #122 in combination with the paragraphs_asymmetric_translation_widgets module. The client doesn't want any UI changes while having async paragraphs, so thats why I'm using the "stable" widget. But like I tried to explain in #136, there is a issue when initially translating the paragraphs, it doesn't set the host langcode on the paragraphs, while it does when you just add new paragraphs. There is a clear difference how paragraphs are being stored in "Paragraphs Legacy Asymmetric" vs "Paragraphs (stable)", while I assume it should be the same.

While trying to solve the above issue, I'm trying to get my head around patch #122:

This chunk makes sense for async paragraphs, but it doesn't get here when I translate the node and reuse the existing paragraphs (see #136 HTR). I moved this chunk lower in the code where it gets triggered when reusing paragraphs and creating new ones.

// Set the new paragraph language if the host entity is translatable.
if ($host->isTranslatable()) {
  $langcode_key = $paragraphs_entity->getEntityType()->getKey('langcode');
  $paragraphs_entity->set($langcode_key, $form_state->get('langcode'));
}

The problem now is that the functional tests fail, in my new patch #140 I have added an additional condition to check on "allowReferenceChanges", so that this logic only applies when dealing with async paragraphs.

I was getting some errors, e.g: a translation already exists for the specified language, duplicate entry. I resolved it by adding additional condition to make it more specific.

We used 2904705-128.patch with version 1.16 but with 1.17 it doesnt apply so i created a new one 2904705-128-1-17.patch

The last submitted patch, 142: 2904705-128-1-17.patch, failed testing. View results →
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Did a reroll for this patch

add reroll patch

Since we were als using 2904705-128.patch with version 1.16, i did a quick reroll of the patch to get it working with 1.18 also.

Another reroll of the latest patch for 1.18. I was missing the
use Drupal\paragraphs\ParagraphInterface;
while Copy&Pasting the code.

Why has the patch gotten more complex again? It seems this was introduced around #146. I thought the consensus was: The complex/longer patch which tries to modify the paragraphs module only so much that it supports async translations out-of-the-box has *no chance of ever getting in*. The short patch as in #141 which only introduces absolutely minimal code to support the external module "paragraphs_asymmetric_translation_widgets" is the one that actually has chances of being comitted.

@rgpublic: You are right but ;)
In our case, we did not switch to paragraphs_asymmetric_translation_widgets module, since few sites only get security updates in terms of module updates. In this case, it made more sense, to fix the existing patch for the latest paragraphs version.
For our recent projects we switched to paragraphs_asymmetric_translation_widgets and use the "minimal" version of the patch proposed in this issue..

@steffenr: Absolutely. I didn't want to complain about anyone who finds the fully-integrated/all-in-one patch that doesn't require that additional module to still be useful for their specific needs. And (at least for me) it's okay to post rerolled versions of these patches here. But: IMHO these patches should be strictly marked as *not for review* in the corresponding comment. And the patch actually intended for review should be based on the simple patch. These thing get mixed up here a lot, we switch back and forth between both versions and that's quite bad IMHO, because the resulting confusion prevents this patch from ever being review and subsequently committed.

Thanks @steffenr and @rgpublic. I have to agree with the points made here by @rgpublic, furthermore those changes to a widely used patch can cause even more trouble. For example in our case it doesn't seem to work with the latest reroll from #2461695: Support asymmetric translations → .

So separating these different approaches and documenting that is definitely useful and makes a lot of sense.

Thanks for all the rerolls anyway, I'm hoping for the day we can get rid of these.