This is simply a separate issue to attempt to implement the changes in #2461695: Support asymmetric translations, but for the experimental widget. This patch should be applied after the latest one in that issue -- this patch does not also include the removal of warnings about field translatability.

I am hoping for a review from someone who understands the new experimental widget, who can spot any howlers I made directly porting the patch from the Classic widget. Thanks!
even for paid work

CommentFileSizeAuthor
#142 2904705-128-1-17.patch18.87 KBshagel
#141 interdiff-2904705-122-141.txt1.48 KBbojan_dev
#141 paragraphs_support_asym_translations-2904705-141.patch2.4 KBbojan_dev
#140 interdiff-2904705-122-140.txt1.36 KBbojan_dev
#140 paragraphs_support_asym_translations-2904705-140.patch2.27 KBbojan_dev
#128 2904705-128.patch19.89 KBvolkerk
#126 2904705-126-experimental.patch20.19 KBidflood
#122 paragraphs_support_asym_translations-2904705-122.patch2.08 KBDuaelFr
#122 interdiff-2904705-121-122.txt723 bytesDuaelFr
#121 2904705-121.patch1.62 KBDuaelFr
#119 2904705-119.patch1002 bytesJeroenT
#115 2904705-115.patch1.66 KBRade
#88 2904705-88.patch20.19 KBbceyssens
#85 2904705-85.patch18.85 KBDuaelFr
#85 interdiff-2904705-82-85.txt2.55 KBDuaelFr
#82 interdiff-2904705-81-82.txt559 bytesJeroenT
#82 2904705-82.patch20.21 KBJeroenT
#81 interdiff-2904705-79-81.txt5.23 KBJeroenT
#81 2904705-81.patch20.54 KBJeroenT
#79 interdiff-2904705-75-79.txt13.06 KBJeroenT
#79 2904705-79.patch19.17 KBJeroenT
#75 experimental-widget-asymetric-translation-2904705-75.patch7.77 KBrp7
#70 experimental-widget-asymetric-translation-2904705-70.patch7.77 KBweseze
#68 experimental-widget-asymetric-translation-2904705-67.patch7.26 KBweseze
#47 experimental-widget-asymetric-translation-2904705-47.patch15.89 KBidflood
#42 experimental-widget-asymetric-translation-2904705-42.patch15.89 KBmaximpodorov
#38 experimental-widget-asymetric-translation-2904705-38.patch14.3 KBmaximpodorov
#25 experimental-widget-asymetric-translation-2904705-25.patch14.2 KBJeroenT
#22 experimental-widget-asymetric-translation-2904705-22.patch14.2 KBefpapado
#17 experimental-widget-asymetric-translation-2904705-17.patch9.21 KBjanmilinds
#10 experimental-widget-asymetric-translation-2904705-10.patch10.06 KBjanmilinds
#2 experimental-widget-asymetric-translation-2904705.patch10.87 KByareckon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yareckon created an issue. See original summary.

Status: Needs review » Needs work

The last submitted patch, 2: experimental-widget-asymetric-translation-2904705.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

miro_dietiker’s picture

Issue tags: +Needs tests

Some first thoughts from reading the code. Didn't test it yet.

  1. +++ b/src/Plugin/Field/FieldWidget/ParagraphsWidget.php
    @@ -2073,6 +2028,89 @@ class ParagraphsWidget extends WidgetBase {
    +    // Localised Paragraphs.
    ...
    +    // Translated Paragraphs
    
    @@ -2105,6 +2143,66 @@ class ParagraphsWidget extends WidgetBase {
    +      }
    

    Without reading the code, i would think that localised means a different thing.

    We could use terms:
    - Translatable Paragraph entity
    - Translatable host field
    Or find something better like "asymmetric translation"... ;-)

  2. +++ b/src/Plugin/Field/FieldWidget/ParagraphsWidget.php
    @@ -2073,6 +2028,89 @@ class ParagraphsWidget extends WidgetBase {
    +        $entity = $this->cloneReferencedEntity($entity, $langcode);
    

    OK so this excessive cloning might do its job. But we are cloning again and again. If cloning does what it should, we would end up with duplicate paragraphs entity on every save. Then storage is a mess. If not, storage is still a mess because we have multiple diverging unrelated revisions (both used in parallel) in the same entity...

  3. +++ b/src/Plugin/Field/FieldWidget/ParagraphsWidget.php
    @@ -2073,6 +2028,89 @@ class ParagraphsWidget extends WidgetBase {
    +      if ($entity->hasTranslation($langcode)) {
    +        $entity = $entity->getTranslation($langcode);
    

    After excessively cloning, i would expect that the child Paragraph would be single language only. Shouldn't we skip creating a translation then? I would expect that for these disconnected Paragraphs, we set the values to the default/source language.

    Simply said, i would expect this:
    - Host with EN + DE
    -- Paragraph field in EN
    --- Paragraph A field item in EN (default)
    -- Paragraph field item in DE
    --- Paragraph B in DE (default)

    So to be clear, if a user later removes translatability of the field, all translations would be lost.

    The idea to support a mixed mode where translations by default sit on the same Paragraph while the user can still add specific Paragraphs to a translation only, doesn't exist here. Even if a parent field is translatable, it could stay pointing to the same child (by default). In that situation, i would add a identifyer "shared" and a new command such as "detach (from translation)".

All in all, repeating myself from the other issue: It'd say we should start with outlining the scenarios you try to cover and provide step definitions, test coverage. And similarly important describe what advanced use cases / edge cases you exclude and we want to cover in follow-ups (without creating regressions). Let's try to create a proper issue / goal summary (use template)... and create screenshots and visualizations (mock-ups, process, storage situation) to make people understand this feature.

I appreciate restarting fresh here after the other issue is a back-and-forth, without clear goal.

yareckon’s picture

OK, let's step back a little bit first:

Paragraphs is partly popular because you don't have to worry about semantics or content structure where defining those things ahead of time would be too restrictive. Paragraphs offers the ability to define a big blob of "Content" in places where it is appropriate to allow that flexibility. Unfortunately, the translation strategy to date has forced translations to follow the "schema" (Structure etc) of the main language. This patch would allow a translation approach more inline with the freedom implied by the Paragraphs module workflow.

Those who wish to use Paragraphs with strongly a defined semantic relationship between host and items (i.e. more like Field Collections) can still stay with the old behavior by marking the host's containing field as non-translatable. This makes the main language into a "schema" for any translations, and removes the ability to have different number and kind of items in another language.

I'm still not sure about the use cases for 1) keeping the present paragraph item with a translation vs 2) cloning and forking for the new language. Dunno if other folks would like to weigh in on that?

miro_dietiker’s picture

The answer is simple... ;-)

The biggest Paragraphs pain you can experience is, if the field was accidentally set wrong. You experience data loss.
And core is against us - by default it is set wrong.
So if we want to release pain, best would be to support switching with a minimum impact.

Also note that many people request that asymmetric feature, but most people are simply just happy how translatability works currently.
So it is randomly guessed less than 10% of the translation cases that want to have different structure per language.

From cases i analyzed, it is in many cases an optional need. So best would be to start with same structure and break it if needed.

That's where my proposal of mixed mode comes in:
If a translatable field would support mixed mode, it would be a perfect fit.
All content items that do not have a language specific override in structure would work the same way.
Only language specific paragraphs would then have a different representation.
The user is aware about what is shared and what is specific for a language.
If the field is ever switched to non-translatable, only these overrides are lost.
Semantics would be correct. If pieces represent the same information (a specific Paragraph), they maintain their corresponding translation.
As a result of that, nothing would be wrong to have that field translatable for most cases except you really want to disallow asymmetry.

BTW as much as you could delete a paragraph for a specific language, you also could list the orphaned Paragraphs in the add widget to readd them again.

Anyway, so much about end goals...
IMHO we can start small and disconnect/clone them completely here as a first step. But we need to make users aware of the fact that switching back will basically drop all previous translations.

weseze’s picture

@miro_dietiker

The biggest Paragraphs pain you can experience is, if the field was accidentally set wrong. You experience data loss.
And core is against us - by default it is set wrong.
So if we want to release pain, best would be to support switching with a minimum impact.

I agree with you, however I think this should be out of scope for this widget or the patch at [META] Support translatable paragraph entity reference revision field

1/ How core works by default is something we will just have to deal with. I also think it is wrong, but the core maintainers obviously think different then us... Or maybe they don't and this is being worked on in an issue I haven't found yet?

2/ The loss of data when changing a fields translatability is how Drupal core works. Do the same with any other field and your data will also be lost. So in my opinion this is not in the scope for this widget/patch, nor even in the scope of the paragraphs module itself. We should consider opening a more general issue against Drupal core to get this "fixed", or at least clearly explained to end users.

screon’s picture

Hey everyone,
great work on the experimental widget and the work in [META] Support translatable paragraph entity reference revision field, it's looking great!

I didn't follow the whole conversation in the meta ticket and this one, but I have a small concern I'd like to mention:

I just started playing around with the experimental widget, since I am setting up a new project. I noticed that the experimental widget doesn't (fully) support assymetric translation: I can not add new paragraphs to translation, only edit them. This does to work in the classic widget using the patch from that META issue.

What is the reason for not having full assymetric translation in the experimental widget from the beginning?
As I read through this issue it sounds like you guys are starting a whole new discussion, but to me it's just weird that translations would work differently per widget. Shouldn't we just implement the same workflow (I don't know how complex that is)?

Looking at the original META issue, it has been open for over 2 years and (finally) close to completion. Does everything really have to be thought over again for the experimental widget? It would be a bummer if Paragraphs users have to wait another 2 years to have async translations in the new widget.

yareckon’s picture

Hi Screon,
We are using the patch above successfully to do fully asymmetric translations, so perhaps something else is different? Perhaps you still need to mark the fields you want to be asymmetric as "translateable"? Be warned if you do that you will lose the data in your second language at least, since it will no longer pull the original field items. As it stands then, unless you wish to write a migration script, you should use this new approach only for new fields and data.

janmilinds’s picture

Rerolling patch for the latest dev version.

Anybody’s picture

Tests fail in #10, any idea why? Or do the tests also have to be changed accordingly?

johnchque’s picture

Hmm it seems that tests need an update or maybe patch in general, there are various fails that are related with multilingual scenarios.

miro_dietiker’s picture

I see that the patch here needs the other patch applied first.

That however can not be committed. The classic widget is feature frozen. If we want to support asymmetric translation, it would be inside the new widget.

Anybody’s picture

Anybody’s picture

Title: Support asymetric translation in experimental widget » Support asymmetric translation in experimental widget
mariancalinro’s picture

@miro_dietiker

I wanted to give some feedback about possible use cases, and our experience with customer needs. In our case, we work with editorial teams that have design knowledge, and who want control over each translation for content reasons, but also aestetic reasons. From ~10 medium or large projects we built with paragraphs in the last 2 years, on 2 the default was ok, but on the rest the editorial teams are not happy with the default symmetric translation, because it doesn't cover the following use cases:

1. On different languages the content is different in substance so certain paragraphs are not needed on translations, and they want to remove certain paragraphs on translations. Examples may include:

  • A page with offers, where the french version is missing a package
  • A page with a media paragraph where they want a different movie because of the language used in the movie, so they would like to remove the original paragraph and add a new one with a different movie. Setting the media field as translatable would not be ok because of this is an exception, not the rule for the media paragraph.

2. On a translation, they want to add a new paragraph without influencing the other languages. Examples:

  • Changing a media paragraph (by removing the original one and adding another one) because the media itself is language dependent. This may be a presentation video that has multiple voiceovers for multiple languages, or an image with some text in it, like a diagramm. Usually we don't want to make the media fields translatable because these are usually exceptions, not the rule, and 99% of the media is language agnostic.
  • There is some content that is specific to a language, such as a list of public holidays, where each holiday is a paragraph. Some will be common, and are translated, other do not apply and need to be removed, and others are new and need to be added.

3. On a translation, they want to change the order of the paragraphs without influencing the other languages. This in itself is no very useful, but if you previously removed paragraphs and added new ones (points 1 and 2), this usually becomes needed.

Notes:
1. I am aware that in some of the scenarios I am refering to different languages in the context of different countries, but it is a use case we meet regularly, where a company is present in multiple markets, but they want a single site, and they use the translations for having content for diferent countries.
2. I am aware that our clients are maybe not a representative sample of the drupal world, but it is valuable feedback about use cases we met.
3. In our case sometimes the reason they want to make the translation asymmetric may even by aestetics: translated content doesn't look good (text length or text cadence may change substantially), and they want to change the type of paragraphs or order of the paragraphs to improve the aestetics.

janmilinds’s picture

With pathes #2 and #10 I somehow ran into a problem where the contents of paragraph elements were not duplicated when I was creating a new translation.

I substituted the clone function with a function from the Classic widget (introduced in the parent issue's patch). Providing a new experimental widget patch for the latest dev version. Naturally this requires also the patch from the parent issue #2461695: Support asymmetric translations.

Anybody’s picture

@janmilinds: Could you please send a screenshot of your settings on the content translation settings page? I think you may have missed to enable some important settings.

/admin/config/regional/content-language

Paragraphs should be checked in the content and paragraph itself (on top) and all translatable fields to enable asymmetric translations. Did you do that?

janmilinds’s picture

@anybody thanks for the advice. I didn't realize I need to check all the fields in paragraphs too to be translatable and to be honest it doesn't sound very logical to me since I actually use both translating methods within the same project: some paragraphs are translated in this asymmetric method where I have enabled the translation for the entity reference revisions field in the parent entity. Other paragraphs are translated in the traditional way and for those I have checked the translatable fields separately.

So at least for me the logic would be easier to understand if I enable the asymmetric translation from the field in parent entity and then enable the translation separately for paragraphs I want to translate in the traditional way. Also, I think it's better when the logic goes same way than in the classic widget since I didn't have similar problem with that.

Anybody’s picture

@janmilinds: I hope I got you right. If you have enabled both, the field and the paragraph with asymmetric translation, everything is and was allright. If not, you should give it a try and reply if you needed the patch from #17 to fix your problems.

janmilinds’s picture

@anybody: I tested now and enabled the translation also for the paragraphs and it didn't fix the problem when I was using patch #10.

Compared to the classic widget, with the classic widget it's not even necessary to enable the translations for paragraph types and the translation works then like a charm. I substituted the clone function with a function from the classic widget and then I got it working with experimental widget too.

efpapado’s picture

Attaching a patch that is based on #17.
More specifically, it is extending #17, by adding the changes in the .module file from the classic widget task.

It does NOT contain test classes yet.

Status: Needs review » Needs work

The last submitted patch, 22: experimental-widget-asymetric-translation-2904705-22.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Anybody’s picture

@efpapado: #22 can not be applied against latest 8.x-1.x-dev. #17 can be applied. I can't tell yet if it works.

JeroenT’s picture

Created a re-roll.

Status: Needs review » Needs work

The last submitted patch, 25: experimental-widget-asymetric-translation-2904705-25.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

ralphvdhoudt’s picture

I've used the patch in https://www.drupal.org/project/paragraphs_asymmetric_translation_widgets... because it wont be committed to the standard paragraphs module and needed to be in a separate module

drc0’s picture

Can someone clarify what is the latest patch to apply before this one as indicated by the issue description? "This patch should be applied after the latest one in that issue".
Or can we apply this patch alone on https://cgit.drupalcode.org/paragraphs/commit/?h=8.x-1.x&id=ea257e467b64...
if wanting to support async translation with the experimental widget ?

idflood’s picture

The patch in #25 works nicely but we have found one small issue with it.

When you edit multiple translations of the same node simultaneously it normally works. But with this patch, as soon as you start to change the content of paragraphs in both languages the following error message appear "The content has been modified by another user, …". Once this happen the page can't be saved.

edit: The validation which cause this issue is from the EntityChangedConstraintValidator and use this condition: if ($saved_entity->getTranslation($langcode)->getChangedTime() > $entity->getTranslation($langcode)->getChangedTime())

idflood’s picture

We found another small issue, only visible when the diff module is enabled.

If you edit a page which already has some paragraph and only edit the content of paragraphs then the revisions page will not show the changes. Here are the detailed steps to reproduce the issue:
- enable the diff module
- go to a page, edit the title and create a new revision
- once saved the new revision is highlighted at the top of the "revisions" tab and selected by default
- edit the page again and this time only change the content of a paragraph
- once saved the "revisions" tab doesn't show the last revision and none of the revision is selected and highlighted by default

The issue is caused by the condition if ($revision->hasTranslation($langcode) && $revision->getTranslation($langcode)->isRevisionTranslationAffected()) { in RevisionOverviewForm.php of the diff module. The fix is probably to call $entity->setRevisionTranslationAffected(TRUE) at some point in paragraphs but I'm not sure where. Also this needs to be called only if the asymetric translation is enabled and if there is a change in one of the paragraph.

Berdir’s picture

If it only happens with diff module then it is possible that this actually a problem in diff.module. It has to duplicate the whole page and tends to get out of sync when core changes. See if you can find if something changed recently in core.

idflood’s picture

Thanks for the tip, but after trying to debug this further I found out that it was also visible without diff module. In fact the condition is the same, and if i comment the $revision->getTranslation($langcode)->isRevisionTranslationAffected() part it also kind of "fix" the issue.

With core only revision, if you check "create a new revision", leave a message and only modify the content inside of a paragraph then it will not show up in the revisions list.

Berdir’s picture

Ok. As mentioned, the easiest thing for me to work with is a patch that extend test coverage, since this doesn't extend the UI, that would need to be in the paragraphs patch.

We might also be updating node titles there in the current paragraphs patch, so do another edit and make sure that shows up on the revision page, best check each language for which revisions are are expecting to see. i'd add a unique revision log message to each so that we can easily identify them.

rp7’s picture

I'm working on a project for which this functionality is pretty crucial. I was wondering how - bar development - I can help moving this further?

I'm a bit confused with the https://www.drupal.org/project/paragraphs_asymmetric_translation_widgets module. Is this only temporary until the final patch lands in the Paragraphs module?

miro_dietiker’s picture

@rp7 we have a Roadmap issue declaring that we will add full multilingual content moderation support first and then add asymmetrical translation support once it is ready. New features like this are limited to the new experimental widget.

Committing these asymmetrical patches to Paragraphs now would make our life much harder with Content Moderation.

Forked widgets or rerolling patches are the only way to get this functionality right now. We welcome and encourage everyone to help fix these challenges in Paragraphs itself. For some fixes for content moderation we even need core patches to deal with limitations, making it impossible to ship it by default now.

Anybody’s picture

@rp7 and @miro_dietiker,

I suggested to document the plans and current status in #2983657: Document the plan for asymmetric translation in Paragraphs module then we could perhaps link the module page here and close the issue to provide a clear way for this functionality until it becomes part of paragraphs. That would also make it easier to write an upgrade path later on from the module to an integrated version.

maximpodorov’s picture

It's time to re-roll the patch after the last commits.

maximpodorov’s picture

maximpodorov’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 38: experimental-widget-asymetric-translation-2904705-38.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

seanB’s picture

I think the issue described in #2995669-2: Error translating content with paragraph as fields is also relevant here.

maximpodorov’s picture

Status: Needs review » Needs work

The last submitted patch, 42: experimental-widget-asymetric-translation-2904705-42.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Anybody’s picture

@maximpodorov: The patch from #42 is in conflict with the latest from #2461695: Support asymmetric translations do you know why? I think both are needed?

maximpodorov’s picture

That issue has "Won't fix" status. It's better to try patches from the current issue instead (and use the experimental widget).

Siavash’s picture

This is great. Thank you! Looking forward to this getting merged in. It seems to be the best path of localizing paragraphs as of now. Looking forward to seeing it get merged into paragraphs.

DrDam’s picture

Status: Needs work » Needs review

#47 are OK for me

mrweiner’s picture

Status: Needs review » Needs work

I've got the above running on a production site and it appears to be mostly working. However, it seems that it doesn't play nicely with revisions on the parent entity. For example:

  1. Go to edit page for a node translation containing your paragraph field.
  2. Update the content of the paragraph field
  3. Add a revision message
  4. Save content
  5. Check revisions for the node -- the last revision is not listed.
maximpodorov’s picture

mrweiner’s picture

Status: Needs work » Needs review

@maximpodorov thanks! I think a combination of that and #2904231: Parent fields are not revisionable did the trick.

DrDam’s picture

#47 passe on 1.7

Stephan de Bruin’s picture

The patch in #47 works like a charm on most content types. However, I have one content type where it doesn't. I think it may have something to do with the content type's machine name, which is 'project'.

Could this be the case? Other CT's like blog and news work just fine. All CT's share the same paragraph field, so i don't think the issue can be in the field.

miro_dietiker’s picture

@Stephan de Bruin: Paragraphs is fully configurable and never treats a specific Paragraph type based on its magic name. Your custom code might do so though.

miro_dietiker’s picture

zeuty’s picture

Patch from #47 worked for me on dev.

hctom’s picture

I just came across a huge problem with this approach: We have an entity type containing 2 translatable paragraph fields. In order to not clutter the default entity form with tons of paragraphs from these fields, we have a custom form mode containing one of the paragraphs fields. This works well until you translate the entity, where in fact the default form mode is used by content translation (without the second paragraphs field). When we open up the other form mode with the second paragraphs field, all contained paragraphs are not duplicated, because this form mode is not used in "create translation" context anymore having $form_state->get('content_translation'), which is the criteria to decide whether the paragraph entities have to be duplicated.

My current solution plan is to add the second field to the default entity form and hide it via form alter, so the duplicated paragraph entities get created. But does anyone has a better solution for this problem?

Thanx in advance!

adel-by’s picture

Patch from #47 is OK for me.

bserem’s picture

#47 worked for me too.

I would love to see this approved, assymetric translations are a neat future.

stillworks’s picture

#47 worked for me as well with 1.9

hctom’s picture

As of #57 I would still see this as "Needs work" :(

mrweiner’s picture

@htcom it might be helpful if you could provide instructions to reproduce your issue from a fresh install, just to make sure that everybody understands the exact use case. I've not used any custom form modes before so it's difficult for me to follow what you're describing.

mrweiner’s picture

Status: Needs review » Needs work

Forgot to change the status per last comments. If this is in conflict with core-provided features then that's something that should probably be addressed.

geek-merlin’s picture

Status: Needs work » Active

#57 is a bit difficult to understand, but after reading it several times i understand this:

It's about a setup with a custom form mode and custom code that work together copying some field content (for reasons i do not understand). The problem is raised as the translation form does not use the custom form mode.

If that's correct, this is a support question that does not block moving on, so setting NR.
(If not, please provide exact steps to reproduce. Thanks!)

geek-merlin’s picture

Issue summary: View changes
Status: Active » Needs review
miro_dietiker’s picture

Status: Needs review » Needs work

I see many test runs above failing, please check.
This also seems to be important as upgrading for patched systems could currently be a problem without a fix.

We fixed many storage / consistency problems recently and i always outlined those as a dependency prior to adding asymmetrical support.
We're not yet fully there, but we're progressing.

stillworks’s picture

Any progress on that issue?

weseze’s picture

I have tried to address the problem outlined in #57.

The previous concept of patches tried to fix the duplication process for creating a new translation in the widget itself, but after analysing the issue I think this is wrong. Because that process fails the moment the field is not present on the default edit form. Which is very possible. This behaviour also ties the async translation option to the widget, which isn't technically necessary.

As always, there is a hook for that :) : hook_entity_translation_create(). So I've implemented that one and there seems to be a much more robust implementation now.
It might be possible that this kind of logic could be implemented in the entity_reference_revisions module and that paragraphs module could just inherit it from there since it is a dependency. We would have to analyse that a bit further, bu I think it is a real possibility and maybe a better solution?

The attached patch is a proof-of-concept, not a polished thing to use in production!
The patch enables async translations regardless of widget used, but only the experimental widget was altered to allow it.

weseze’s picture

Status: Needs work » Needs review
weseze’s picture

Made a small update so the correct revision for the paragraphs are cloned.

I could really use some help trying to figure out why the tests are failing...

rp7’s picture

Tested the patch in #70 and I'm pleasantly surprised, it works. Great stuff, nice work!

Any chance a maintainer can have a look and see if this is the way forward? Besides that & the failing tests, what still needs to be done here?

stillworks’s picture

#70 works so far, but only for Paragraphs that have no translation content stored. If I check everything with "*not recommended" in /admin/config/regional/content-language my translated content disappears. The original language stays the same, uncheck everything and my content appears again.

Hide non translatable fields on translation forms, on/off makes no difference here.

If I create a page, translate into language (node gets duplicated) it works aswell.

BramDriesen’s picture

Status: Needs review » Needs work

Can't get the patch to apply.

eelkeblok’s picture

@stillworks I think this is described somewhere above and is "expected". It also touches on the mixed mode mentioned by @miro_dietiker; when you switch a Paragraph field from non-translatable to translatable, the translated field will not have any values (there is no code to turn the translations of the paragraphs that are in the untranslated fields into full-blown, referenceable paragraph entities). What is in the patch (IIUC), is code to clone paragraphs when creating a translation for a parent entity that has the translatable field switched on).

rp7’s picture

BramDriesen’s picture

Status: Needs work » Needs review

Triggered tests.

Status: Needs review » Needs work

The last submitted patch, 75: experimental-widget-asymetric-translation-2904705-75.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mErilainen’s picture

@eelkeblok: About switching the field from non-translatable to tranlatable, there is an approach for https://www.drupal.org/project/paragraphs_asymmetric_translation_widgets at #2992777: Implement a migration path. Not sure if the same approach works for the experimental widget, but maybe it does.

JeroenT’s picture

This should fix most of the test failures.

ParagraphsConfigTest was failing because the message shown on the language_content_form was updated from unsupported to not recommended.

ParagraphsWidgetElementsTest, ParagraphsWidgetButtonsTest and ParagraphsHeaderActionsTest were failing because on the language_content_form the checkbox to translate a specific content type was checked, which means all fields of that content type are also selected. Including the paragraphs field. Checking the paragraphs field on the language_content_form means you also activate async translations for paragraphs.

Status: Needs review » Needs work

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

JeroenT’s picture

Status: Needs work » Needs review
FileSize
20.54 KB
5.23 KB
JeroenT’s picture

FileSize
20.21 KB
559 bytes
penyaskito’s picture

+++ b/tests/src/Functional/WidgetStable/ParagraphsHeaderActionsTest.php
@@ -332,6 +332,9 @@ class ParagraphsHeaderActionsTest extends ParagraphsTestBase {
+      // will enable aync translation of paragraphs.

+++ b/tests/src/Functional/WidgetStable/ParagraphsWidgetButtonsTest.php
@@ -201,6 +201,9 @@ class ParagraphsWidgetButtonsTest extends ParagraphsTestBase {
+      // will enable aync translation of paragraphs.

+++ b/tests/src/FunctionalJavascript/ParagraphsWidgetElementsTest.php
@@ -58,6 +58,9 @@ class ParagraphsWidgetElementsTest extends WebDriverTestBase {
+      // will enable aync translation of paragraphs.

typo: async

Which scenarios are required to test for this to land?

Also, as far as I'm aware, after changing translatability there is a "soft" lost of content. Should we also include a migration of the content as part of this patch? Should that happen automatically after changing the translatability? Is that a requirement for this to land or can be done in a separate issue?

miha.wagner’s picture

So does this patch work on its own or does it need to be combined with the patch in this thread: https://www.drupal.org/project/paragraphs/issues/2461695. If so, which one?

DuaelFr’s picture

FileSize
2.55 KB
18.85 KB

Rerolled + fixed typos from #83
Is the "Needs tests" tag still accurate?

rgpublic’s picture

@miha.wagner My take on this:

* #2461695 is the patch for the normal widget
* This issue here is a patch for the experimental widget
* Those patches are not connected in some way. They are two different issues/patches for two distinct widgets.
* You can change the widget in the form settings of your paragraph field(s)
* #2461695 isn't recommended, because the normal widget stays as-is and won't get major new features. That's why it is the normal aka "stable" widget
* The experimental widget is IMHO not so "experimental" after all. We're using it on dozens of websites (many multilingual ones) without any major problem.
* We're still using patch #47 for the experimental widget in *production* for many, many months (almost a year) now without any problems whatsoever
* YMMV though, but feel free to try it out yourself.

DuaelFr’s picture

@penyaskito As this patch does not include anything that allows or needs to change the "translatability", I'm not sure it has to provide an upgrade path. This patch only allows the widget to act properly when it has to deal with translatable fields for what I can see.

bceyssens’s picture

Status: Needs review » Needs work

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

daveiano’s picture

I got a few questions regarding this issue. I am currently working on a Project which uses the patch from #70.

The translation configuration is the following: The paragraph field on the host node is translatable, but also the paragraphs with their fields are set translatable. Is this the right configuration? In my understanding, only the paragraphs field on the host should be marked translatable?

With this configuration set, I am experiencing the following thing:

Publishing/Unpublishing Problems:

1. Create a german node with 2 paragraphs
2. Translate that node to English (the german paragraphs have been transferred to the translation and I see them in the English translation)
3. Now I unpublish the first paragraph on the English translation - on the English site there is the german paragraph displayed
4. Go to the german source and also unpublish that paragraph results in German site: Paragraph 1 is not displayed - correct. On the English site: Still, the german unpublished paragraph is displayed.

But if I create a node in german and do not add paragraphs, then hit translate and add an English translation and add paragraphs separately for each language it works as expected.

stillworks’s picture

#85 works for me, but I agree we need an upgrade path for existing content.

xmacinfo’s picture

Can someone reroll the patch? It does not apply on version 8.x-1.12.

- Installing drupal/paragraphs (1.12.0): Loading from cache
- Applying patches for drupal/paragraphs
    https://www.drupal.org/files/issues/2020-08-12/2904705-85.patch (Patch #2904705-85 to fix additional paragraphs translations)
Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2020-08-12/2904705-85.patch
xmacinfo’s picture

#2461695 isn't recommended, because the normal widget stays as-is and won't get major new features. That's why it is the normal aka "stable" widget

When a feature is EXPERIMENTAL, we expect fixes going to both branches, main and experimental. So we would expect Classic fixed first and then Experimental branches of Paragraphs.

rgpublic’s picture

@xmacinfo: Well, the patch here is not a (bug) fix. It provides a brand new feature (asymmetric translation) which isn't really available otherwise. If you want this feature *now*, you should use the future (experimental) widget and the patch here. Period. Now, if I glance into my crystal ball I recently bought as a bargain from a yard sale, the experimental widget will one day be renamed to "stable widget" and the current stable widget will be removed. Only *then* will this feature (asymmetric translation) be available for the stable widget, because that future stable widget is what is now called our experimental widget - and that's why we have this patch here. Magic :-)

xmacinfo’s picture

I had to switch to the Classic widget to let us translate a page with Paragraphs and use patch 2461695-276. So far, it fails using EXPERIMENTAL.

If you want this feature *now*, you should use the future (experimental) widget and the patch here.

This patch does not apply.

Berdir’s picture

> Now, if I glance into my crystal ball I recently bought as a bargain from a yard sale, the experimental widget will one day be renamed to "stable widget" and the current stable widget will be removed

You should return your crystal ball, you were tricked. That rename already happened in 8.x-1.x-dev, it would already be in the current release if I wouldn't have messed up. That said, there are no plans to remove the old widget any time soon, it's just renamed to legacy and receives minimal fixes only.

That said:

* This isn't about supported vs development branches, so it's quite opposite. The previous experimental and now default and recommended widget is the one being actively developed.
* Yes, this is a new feature, not a bugfix. One that I'm still uncertain about supporting at all
* The reason this is issue is about the experimental widget is because there already was one for the old widget, which was closed as won't fix, partially because that won't receive new features and because there is https://www.drupal.org/project/paragraphs_asymmetric_translation_widgets, which offers that functionality for that widget. And I think it should do the same for the new recommended/experimental widget too.

rgpublic’s picture

Thanks a lot for the clarification. A proper Berdir is of course always better than a dusty crystal ball. Perhaps the subject of both issues should be renamed because the widgets have been renamed?

So if I see it correctly and to get this out of this state of paralysis, the patch in this issue should be integrated into the paragraphs_asymmetric_translation_widgets module and we should close this issue as WONTFIX. Should I open a new issue over there?

@macinfo: "I had to switch to the Classic widget to let us translate a page with Paragraphs and use patch 2461695-276.": This, of course, is NOT correct. You can translate paragraphs perfectly well without ANY patches with both widgets. Doing this literally every day without any major problem on >50 Drupal websites. And even with multiple levels of nested paragraphs and lots of other frightening things :-) So if this doesn't work for you, you have already found your problem. Are you absolutely sure you followed the advice given here?:

https://www.drupal.org/docs/8/modules/paragraphs/multilingual-paragraphs...

The asymmetric translation patch is about when you don't have a 1:1 translation (paragraph by paragraph) but instead want to remove or add paragraphs in different languages. Only then will you need the patch. Make absolutely sure that you cannot live without that feature first.

xmacinfo’s picture

@rgpublic Thanks for the doc link about D8 multilingual. Of course, we prefer 1:1 translations. But I got bugged down when trying to add a paragraph to a fully translated node and discovered that the newly added paragraph did not show at all in the translation. So I will review our setup and adjust the settings accordingly.

rgpublic’s picture

Because I didn't hear anything to the contrary, I now filed this issue here:

https://www.drupal.org/project/paragraphs_asymmetric_translation_widgets...

@Berdir: IMHO you could now close this here as WONTFIX. Won't do it myself because I don't want to preempt your final decision.

maximpodorov’s picture

Please no. We have a working solution here (the patch) that is used by hundreds of sites without problems.

rgpublic’s picture

Yeah, but Berdir is the major maintainer of this module. If he says he doubts he'll ever integrate this, people will have to patch and patch endlessly forever. I just doesn't make sense - we need to move this forward and find a solid permanent solution. Endless patching and this issue here staying open for an indefinite amount of time is not a solution. I think it's quite valuable to have this open discussion about what the maintainers want and don't want to do and act accordingly instead of chasing a fantasy that will never happen.

Of course, I'd expect the solution in asymmetric_translation_widgets to be a 1:1 drop-in replacement for the patch here. Meaning you could, in the future, uninstall the patch and instead install that other module and your websites with asymmetric paragraphs will keep on running as usual. Sounds not too unreasonable for me. Also, from a merely technical point-of-view, I guess it's not a completely weird idea to say we split this rather complicated functionality which isn't needed by many sites off into a separate module.

Anybody’s picture

Of course, I'd expect the solution in asymmetric_translation_widgets to be a 1:1 drop-in replacement for the patch here. Meaning you could, in the future, uninstall the patch and instead install that other module and your websites with asymmetric paragraphs will keep on running as usual. Sounds not too unreasonable for me. Also, from a merely technical point-of-view, I guess it's not a completely weird idea to say we split this rather complicated functionality which isn't needed by many sites off into a separate module.

+1 in all points!

https://www.drupal.org/project/paragraphs_asymmetric_translation_widgets currently for example requires you to change the settings for all paragraphs fields, which can be annoying. So putting this into a module with tests that ensure that updates to paragraphs don't break the modules functionality would be a fair choice.

Btw we experienced quite often that a project started with symmetric translation and then the customer came to a point where he needed asymmetric... perhaps if the maintainer doesn't think it's a good idea to put asymmetric translations in, a further way (in a separate module or not) might be to think completely different.
One idea might be to allow to disable / hide single paragraphs per language which would quite smart allow to add or remove certain paragraphs per language and with a good ui it could perhaps even work around complex logical problems?

But back to topic, the module solution would be good!

hctom’s picture

While haven't looked at the paragraphs_asymmetric_translation_widgets module yet, it sounds like the problem from #57 (or perhaps better explained in #68) will persist, doesn't it? That would be quite bad :(

rgpublic’s picture

@hctom: Not necessarily! If the idea for a fix brought in here with #68 are eventually ported over there. I think, first and foremost, the general feature "asymmetric translation in experimental widget" needs a final "home" where it resides. As long as everything is in limbo - neither here nor there - significant progress just cannot occur. So, if there's reluctancy by the maintainers to integrate it here, we need to tear down our tents and simply move over there - anything else is wasted time. This I guess is the most important step. But only a first step. That's why I filed that other issue and propose to set this issue to WONTFIX. Anything else (problems with form modes etc.), I'd simply consider a bug we could then file over on that other module.

weseze’s picture

Just want to weigh in here, because I am (or at least was) one of the maintainers for the paragraphs_asymmetric_translation_widgets.

I ceased further development in that module because the async translation behaviour would be accepted in the experimental widget. At least, at one point that was the case here... If that is not the case, we all have a problem...

It would be really great if we could get a definitive answer on whether or not the async translation functionality will ever be added or not. If this is under serious consideration, then we need very clear goals here to make it happen. I'm more then willing to put it in the work if I know it's going to happen. But I'm unwilling to put much work in this if it's "probably" not going to happen and no-one can explain clearly why...
If it is not going to happen, we should focus on the paragraphs_asymmetric_translation_widgets module and cease all further development here.

@Berdir: your input here would be much appreciated: do we continue on this patch, or do we continue in a separate module? What is the best way forward for the paragraphs eco-system?

@rgpubli: paragraphs_asymmetric_translation_widgets will very likely never be a drop in replacement for the patch from this issue. It is (near) impossible to override the existing (experimental) widget without altering its core code. Therefore a patch is needed, or a different widget is needed...

hctom’s picture

+1 for weseze's comment!

IMHO this feature should be supported by paragraphs module itself, because forcing editors to have to use the same paragraphs in every language version of a content does not make sense at all (just think of legal limits that might allow publishing a video paragraph in one language version but not the other).

But let's see what the maintainers think about it.

rgpublic’s picture

Understood. It could then also be a completely new module, or a submodule of paragraphs - I'm not particularly in favor of either of those possible solutions and also not personally against keeping it in paragraphs - but I hope *everyone* agrees that we definitely need to have this open discussion - so that everyone knows where to invest their work in and how to proceed from here :-) Now I'd say it's probably up to @Berdir, right?

dksdev01’s picture

+1 for getting the final confirmation from the maintainers. thanks

flocondetoile’s picture

From #97

The asymmetric translation patch is about when you don't have a 1:1 translation (paragraph by paragraph) but instead want to remove or add paragraphs in different languages. Only then will you need the patch. Make absolutely sure that you cannot live without that feature first.

If you need "small" asymmetric translation, this patch is not absolutely necessary. You can simply play with the "published" checkbox, to publish / unpublish paragraph when translating them. You just need this little hook which is more sustainable in the time until this issue #2902371: Unpublish paragraph doesn't work correctly in multilangual landed.

/**
 * FIX for issue with paragraph translated and published/unpublished per
 * language for anonymous users. Content translation give the fallback paragraph
 * if the paragraph in unpublished in the current language !
 * @See https://www.drupal.org/project/paragraphs/issues/2902371
 *
 * Implements hook_entity_access().
 */
function MYMODULE_paragraph_access(ParagraphInterface $entity, $operation, AccountInterface $account) {
  $language = \Drupal::languageManager()->getCurrentLanguage()->getId();
  if ($operation == 'view') {
    if ($entity->isTranslatable() && $entity->hasTranslation($language) && !$entity->getTranslation($language)->status->value) {
      return AccessResult::forbidden();
    }
    elseif ((!$entity->isTranslatable() OR !$entity->hasTranslation($language)) && !$entity->status->value) {
      return AccessResult::forbidden();
    }
  }
}
Berdir’s picture

That or use the language behaviour plugin of paragraphs_collection or something like it. Both options do have a disadvantage, all paragraphs that should be shown or not in any language need to be added first on the default language. So as #109 said, this is better suited if those differences are the exception, and not the rule.

I can't read all the comments right now, but I think my previous comment is still correct. I would prefer if this feature would be maintained in a separate project, but I'm open to make improvements to our widgets to make them more reusable for example. We don't use this feature and currently don't have the bandwith to maintain it. As mentioned several times, a hard requirement to consider it for paragraphs.module itself would be _extensive_ test coverage, along the lines of \Drupal\Tests\paragraphs\Functional\WidgetStable\ParagraphsTranslationTest.

I'm also aware that https://www.drupal.org/project/paragraphs_asymmetric_translation_widgets is pretty fragile and often breaks on paragraphs updates and that the maintainers are currently not too active, but considering how many people are interested in this, it should be possible to find others to step up and help?

And last, as also mentioned several times in here before, this approach isn't without its drawbacks. Among other things, maintaining the content of your sites and making sure that things are in sync between translations when they have to be is likely going to be much more work with asymmetric translations, and some integrations like our TMGMT project doesn't support it yet. I think the asymmetric project page should explain the advantages/disadvantages of this more clearly. Something else that a contributor could help with, just create an issue and propose some content updates.

rgpublic’s picture

Okay, I think the best idea is to create a new separate module for this. If I look at the latest patch that passes the tests (#85) I see only one change in /src/Plugin/Field/FieldWidget/ParagraphsWidget.php which probably couldn't be overriden easily by a module. All the other stuff are alter_hooks etc which could be called after the original hooks and augment/modify what's already there and, of course, tests which I'd like to ignore for a moment.

In ParagraphsWidget.php we have this change here:

-        if (!empty($settings['untranslatable_fields_hide']) && $this->isTranslating) {
+        if (!empty($settings['untranslatable_fields_hide']) && $this->isTranslating && !$items->getFieldDefinition()->isTranslatable()) 

and this change:

   protected function allowReferenceChanges() {
-    return !$this->isTranslating;
+    return !$this->isTranslating || $this->fieldDefinition->isTranslatable();
   }

Isn't there a way to factor this difference out into a separate function/hook which could then easily be overriden in the module someshow? It seems doable...

weseze’s picture

@rgpublic: If I understand @berdir correctly we would be best to focus our efforts in https://www.drupal.org/project/paragraphs_asymmetric_translation_widgets and not here for the time being. I will commit myself to further maintaining that module and getting it to a point where inclusion in the paragraphs module itself could be considered.
A small patch included in the module might fix the issue you've documented, at least until we can get the change in paragraphs module.

@berdir: Can we coordinate releases for paragraphs with paragraphs_asymmetric_translation_widgets? Where or how could I best follow up the release plans for the paragraphs module? If we can coordinate this a lot if the instability and incompatibilities should be resolved.

Berdir’s picture

Releases are currently fairly spontaneous, but if you have (good enough) test coverage, you can always run it against paragraphs 8.x-1.x-dev and have a daily test run with e-mail notifications to be notified about problems immediately.

rgpublic’s picture

@wesze: I had the idea with a new, separate module just because it might be easier to remove the "legacy" functionality for the classic widget one day. But if an integration into the existing paragraphs_asymmetric_translation_widgets is the preferred solution, I'm also perfectly fine with that. Great that things are moving forward :-)

Rade’s picture

Status: Needs work » Needs review
FileSize
1.66 KB

Big thanks to everyone who has worked on this issue!

We are right now starting a project where we have the need for asymmetric translations. In our previous projects we have used the classic/legacy widget together with paragraphs_asymmetric_translation_widgets, which has worked just fine, but we would very much like to use the stable widget this time.

Having read through this whole thread, I fully respect that Paragraph maintainers have priorities and can't include this feature right now. I'm in favor of the idea that the feature would be provided by another module, for example by paragraphs_asymmetric_translation_widgets.

In reference to @Berdir in #110

I'm open to make improvements to our widgets to make them more reusable for example.

and to the changes in ParagraphsWidget.php mentioned by @rgpublic in #111,

I was thinking that maybe a patch like this would cover our needs. If this would be included in Paragraphs module, I believe that rest of the logic could be implemented in paragraphs_asymmetric_translation_widgets.

Rade’s picture

I also added a first version of a patch for paragraphs_asymmetric_translation_widgets that would work together with the one from #115

Find it here: #3171810-2: Support experimental widget

JeroenT’s picture

Status: Needs review » Reviewed & tested by the community

I tried the patch in #3171810: Support experimental widget which is working fine, but currently it needs the patch in #115.

miro_dietiker’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/src/Plugin/Field/FieldWidget/ParagraphsWidget.php
@@ -479,7 +479,7 @@ class ParagraphsWidget extends WidgetBase {
-        if (!empty($settings['untranslatable_fields_hide']) && $this->isTranslating) {
+        if (!empty($settings['untranslatable_fields_hide']) && $this->isTranslating && !$this->supportsAsymmetricTranslations()) {

I think this should be changed to this then:

-        if (!empty($settings['untranslatable_fields_hide']) && !$this->allowReferenceChanges()) {

And instead you could override this existing method instead in the additional module. That's why we defined the method already, we seem to have missed using it at that point. Hope it doesn't break anything. :-)

JeroenT’s picture

Status: Needs work » Needs review
FileSize
1002 bytes

Made changes as suggested by #118.

stillworks’s picture

For some reason #119 doesn't work for me with the latest -dev. It removes the possibility to edit the translation. The content is still there.
No errors what so ever. Even in combination check/uncheck the "hide untranslatable fields".

#115 works had to revert too it.

DuaelFr’s picture

This patch merges #115 and #119 together.
For people around here for a bit like me, note that this patch does not allow to have asymmetric translations on its own. You'll need the paragraphs_asymmetric_translation_widgets module to take advantage of the patch changes.
I suppose more changes are going to be needed as I can see a lot of conditions on $this->isTranslating that might be using $this->allowReferenceChanges() instead.

DuaelFr’s picture

I had a big issue with the last patch + paragraphs_asymmetric_translation_widgets module because adding a new paragraph on a translation was not populating its langcode then failed badly when trying to addTranslation on the paragraph entity because it has no langcode (zxx).
Setting the langcode on the paragraph when it is created seem totally safe to me and will prevent future issues on the module even without asym translations (even if this case should not occur without asym translations, we never know as there is no condition to make it safe).

Siavash’s picture

Hello and thanks for this much needed functionality.

We have been using this patch for a long time now but functionality broke after updating Drupal core + paragraphs module.

I switched to using the asymmetric module but I'm still having a problem where anytime I save a translation, the original (first created) translation looses all it's paragraphs. I tested it with #122 patch, with experimental widget, and with asymmetric classic widget. The experimental widget no longer lets me add new paragraphs on the translations and none of them work.

I would really appreciate any support in resolving this issue.

Thanks,
Sia

Siavash’s picture

Update:

By using Patch 2904705 #115 + 3171810 + #2992777 [partial patch, see comments in issue] and the paragraph asymmetric module I was able to get everything working, thanks!

NOTE that patch #122 with the asymmetric module does not work for me.

rang501’s picture

The #122 works with the asymmetric module for me. The only thing that doesn't work and I can't find the cause is with base fields, I can add a widget, make the field translatable, on the code side, it seems the state of allowing reference changes is correct, but when saved I can see on the database that translation is referencing exactly same paragraph id and revision. It can be seen on any custom entity, i.e paragraph library.

idflood’s picture

While having a more official solution would be good the approach described in #124 didn't work for us. The paragraph asymmetric module had a few blocker issues in our tests, for ex 3226372.

So I rerolled the patch in #88 to have a temporary working solution. It's not an ideal solution and thiswill have to migrate to paragraphs_asymmetric_translation_widgets in the future but it should allows us to upgrade to drupal 9 in time.

Status: Needs review » Needs work

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

volkerk’s picture

Re-rolled for 1.13

bellemaremederic’s picture

I just want to add my voice to the 10% of people that will appreciate the asymmetric possibility for content :)

rgpublic’s picture

Note that #122 is a very slim patch which only enables the paragraphs_asymmetric_translation_widgets to properly do its thing. IMHO this approach is far superior and the way to go. We are using it for many months now without any problems. If #122 were committed (and the risks of doing so are probably quite minor as the patch by itself doesn't actually change that much), it'd make it possible to have asymmetric translation *without any patches*. That should be the goal, I guess.

So I think it probably doesn't make that much sense if people keep posting rerolls of older patches here. If you have problems with #122 we should now make an effort to actually analyze and solve these. Comments like "#122 didn't work for me" without any specifics nor associated bug reports won't help us in that regard. Really hope we can finally move this forward - looking for patch-free asymmetric future ;-)

Anybody’s picture

Status: Needs work » Needs review

Setting to "Needs review" for maintainers to check #122 based on #130.

DrDam’s picture

Patch #122 and paragraphs_asymmetric_translation_widgets module work perfectly

julien.sibi’s picture

Hi,

in my case #122 + paragraphs 1.14 + module paragraphs_asymmetric_translation_widgets + field translation enabled despite warning worked fine.

daveiano’s picture

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.

rgpublic’s picture

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.

bojan_dev’s picture

#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).

Status: Needs review » Needs work

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.

amaisano’s picture

Status: Needs work » Needs review

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. The _expectation_ would be that for the _initial_ TMGMT translation creation, _if_ there is a job translation, use that instead of cloning.

rgpublic’s picture

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

bojan_dev’s picture

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.

bojan_dev’s picture

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.

shagel’s picture

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

Status: Needs review » Needs work

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