tractorcow-farm / silverstripe-fluent

Multi-language translate module for Silverstripe, without having to manage separate site trees.
BSD 3-Clause "New" or "Revised" License
91 stars 109 forks source link

Copy from/Copy to overwrites $has_one fields #848

Open polhec opened 3 months ago

polhec commented 3 months ago

Module version(s) affected

5 and up

Description

Copy to and Copy from CMS actions load object in From locale and then write object in To locale. This will overwrite all translated fields on object including all translated $has_one fields.

This behaviour may be unwanted in specific cases like when using Fluent together with Elemental module in indirect translation configuration. In this case $has_one relation is translated and localised page should have different ElementalArea-s.

If page does not exist in target locale, everything is OK as different elemental area is created on write.

Problem emerges when page exists in target locale. In this case ElementalAreaID value is overwritten by value from source locale. Final efect is, that two page localizations share same ElementalArea and all elements. That should not happen when using Elemental in indirect localization configuration.

How to reproduce

Install Elemental extension and fluent. Configure Elemental extension in indirect translation mode. Publish page in two different locales. Add some Elements in each locale. USe Copy from or Copy To CMS action.

Possible Solution

Different solutions may be possible:

Additional Context

No response

Validations

tractorcow commented 3 months ago

I remember this problem coming up several times in the past. One will need to go over the history and investigate the solution that was used in the past. :) I believe there was a module for elemental + fluent that formalised the solution. It was years ago since I last investigated it though, and years still since I've touched elemental. I'm mostly familiar with the problem though.

tractorcow commented 3 months ago

https://github.com/tractorcow-farm/silverstripe-fluent/issues/499

polhec commented 3 months ago

499 is not really connected to our problem, as both translations already have existing ElementalArea objects.

625 hints that extension hooks were added to allow extra control for cases like ElementalArea relation.

I think that Copy action should not overwrite has_one relation fields by default as it can not know what kind of data object is linked by it. It is true, that in most cases that is not big problem, as editor can just change related object in CMS afer Copy action has been performed, but in cases like Elemental, that is not possible.

AljosaB commented 3 months ago

To simplify the explained issue, I believe the copy from / copy to should be disabled / grayed out for the locales where translation already exists, which is currently not the case. It should be required to archive the translation, before you're allowed to create a new one.

It also rises the question, what is the procedure when creating translation over existing one - does it just overwrite the fields or is the page being archived first and new instance created (duplicated)? I did some testing tho, if I archive already existing translation and create a new one, the elemental area still gets the same ID as source locale. So the real question is, why does it work ok only the first time?

Either way, this needs some kind of fix, since it's quite destructive... the editor is unable to fix the situation without deleting all translations and start from the scratch.

tractorcow commented 3 months ago

so essentially, what you want is a set of translate fields that are overwritable (e.g. text fields), but you also want the ability to specify certain fields (e.g. has one relations) are ignored for the purpose of certain actions (e.g. copy from locale), right?

I.e. that some fields can be localised, but have a default action of only copying from another locale if they were blank.

At the moment we make no such distinction, and you would need to write custom code in order to specially exclude those fields I think. However, it's possible we could add a hook (with example overrides) that let you control this action.

Just making sure I understand the problem correctly, as I don't think I understood it properly when I first read your report. :)

tractorcow commented 3 months ago

I don't think there's an easy or simple solution to this, to be honest. I'll need to think about it.