Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multilanguage and sylius #704

Open
dbu opened this issue Apr 19, 2016 · 10 comments
Open

multilanguage and sylius #704

dbu opened this issue Apr 19, 2016 · 10 comments

Comments

@dbu
Copy link
Member

dbu commented Apr 19, 2016

follow up of #703

@steffenbrem lets continue the discussion here, and keep #703 for the bug you found.

when we built the phpcr-odm multilanguage system, the main goal was that translations are transparent for your model and for the application using it. you set the locale on the document manager (e.g. in a request listener) and from then on all code can act as if there was no translations, and just "happen" to end up with the correct language. this turned out to be quite powerful while being really simple to use.

what exactly do you need? you can query the document manager to fetch all available locales of a document.

i would love to find a way so that sylius can use phpcr-odm multilanguage directly, rather than build a custom solution on top of it.

did you look at the "child" strategy for translation? then you get a specific node per language. maybe we could find a way to expose those better, for applications that want to work on multiple languages at the same time, if that is what sylius wants to do.

@koemeet
Copy link
Contributor

koemeet commented Apr 19, 2016

I really do like how translations are done in phpcr-odm, but for Sylius I felt like it worked too different from how they are doing translations right now for ORM entities. I wanted to make working with translations with phpcr the same, so I used their interfaces for translation documents (such as PageTranslation) and translatable documents (such as a Page). The main problem for me was that I was unable to find a good way to re-use the sylius_translations form type, which creates a collection of <resource>_translation form types, indexed by the available locales (this is why I needed #702). This is what Sylius uses to persist multiple translations for a resource at once.

Here is one of the JSON payloads I use in one of my tests for translatable pages:

{
    "name": "about-us",
    "channel": {$channels['channel_1']->getId()},
    "translations": {
        "en_US": {
            "title": "About us",
            "body": "Some body in english"
        },
        "nl_NL": {
            "title": "Over ons",
            "body": "Wat content in het nederlands"
        }
    },
    "routes": [
        {
            "name": "about-us"
        }
    ]
}

Also, when editing a translatable phpcr document, the translations are all loaded and displayed in a tabs, separated by it's locale. Currently there is not an easy way to fetch all child translations of a phpcr document. Fetching all locales and doing loadTranslation over and over while cloning the outcome was not a very clean solution to solve this. If we could solve those problems, then I think Sylius can use it.

Right now I have a working (custom) solution, that uses the Sylius translation interfaces and so it works exactly the same as for the ORM entities. Only issue I am still having is #703.

I am not sure how much we can do to the phpcr-odm to make it fit more with how Sylius works with translations. It is very different.

@koemeet
Copy link
Contributor

koemeet commented Apr 19, 2016

What would be the downsides when dropping the multilang support that phpcr-odm provides? Right now I have found it quite easy to implement a custom one, which is very cool 👍 Not sure if I am missing something, something that I will get hit by when using a custom solution.

@dbu
Copy link
Member Author

dbu commented Apr 19, 2016

my main worries with the custom solution is that a) i have to do more
manual work to use sylius with phpcr-odm, and b) when i introduce sylius
into an existing project, i face a huge refactoring and data migration.
(not only for the documents, also for the code consuming the documents
and now suddenly needing to be aware of multilang).

i wonder if we could write some code on top of the child translation
strategy that can map the translated children of a document onto
translation documents and back. actually, if you define that document
with mappings for the fields (as a not translated document) then
phpcr-odm should be able to load those with the paths to the translation
children. you would have to be careful not to update the translated
fields of the parent document at the same time, however, or things will
try to overwrite each other.

you could even map the translation children on the document, with a
filter on phpcr_locale:* to only see translation children.

you still would have some overhead: duplicate the mappings of translated
properties in the translation child document - but imho this should be
optional, then you can use a normal document with sylius without
changing that document.

you could provide a little glue code in sylius to load all those
children. maybe you could even do a cache warmer that creates the
translation documents, from reading the doctrine metadata and looking
for all translated fields.

@koemeet
Copy link
Contributor

koemeet commented May 7, 2016

I have an implementation for this in Sylius that now works as follows:

Every translatable document has a translation document counterpart, so we have a Page object and a PageTranslation object. The page object implements Sylius\Component\Resource\Model\TranslatableInterface and the page translation extends the Sylius\Component\Resource\Model\AbstractTranslation class.

Mapping of Page.phpcr.xml:

<document name="AcmeSylius\Document\Page">
   <id name="id">
        <generator strategy="PARENT"/>
    </id>

    <nodename name="name"/>

    <!-- ... -->

    <children name="translations" filter="phpcr_locale:*">
        <cascade>
            <cascade-all/>
        </cascade>
    </children>
</document>

Mapping of PageTranslation.phpcr.xml:

<document name="AcmeSylius\Document\PageTranslation">
    <id name="id">
        <generator strategy="PARENT"/>
    </id>

    <parent-document name="translatable"/>
    <nodename name="nodename"/>

    <field name="locale" type="string"/>
    <field name="title" type="string"/>
    <field name="body" type="string"/>
</document>

In order for this to work properly, the PageTranslation object sets its own nodename property when you call setLocale. So if you would call $translation->setLocale('en_US'), that object would have it's nodename set to phpcr_locale:en_US. It is still very hacky, but it works, putting this in a listener would be better.

With this setup, you get properly persisted documents and also retrieving works fine, because of the filter on the children collection.

For now this is my best solution for translatable PHPCR documents for Sylius. I found that the child strategy was too different and I could not come up with a good implementation that solves the issues I described before.

@dbu What do you think about this? Data migration will be not that hard to do right? I am using this setup in production and for now it is working great for me. Would this be good enough as a PR in Sylius, or do you think a different solution is better.

@koemeet
Copy link
Contributor

koemeet commented May 7, 2016

@dbu Do you btw agree that using separate translation objects for phpcr documents is preferable for Sylius? In my opinion it is best to keep consistency and re-use the Sylius translation interfaces.

@dbu
Copy link
Member Author

dbu commented May 27, 2016

i don't know sylius and don't know if they had to build a custom translation system. the orm doctrine extension translatable supports something that sounds very much like this, where you specify a translation model that is stored in its own table and used for the translated fields. is that what sylius is using?

if so, i think it would be better to integrate this deeper into doctrine. the child translation strategy already does store a phpcr node per language. it also allows to fetch all translation, but only as a raw phpcr node (the phpcr equivalent to database table rows) and not mapped to a model. it should not be too hard to add an option to the child translation strategy to support such translation models. if i understand your current proposal correctly, there is too much details leaking into the model and the application using the model. with phpcr-odm multilang, the model is transparently in the right language without needing any listeners or other ways to tell the model about the current language.

@dantleech
Copy link
Contributor

where you specify a translation model that is stored in its own table and used for the translated fields. is that what sylius is using?

No, Sylius has its own system

So if you would call $translation->setLocale('en_US'), that object would have it's nodename set to phpcr_locale:en_US. It is still very hacky

Doesn't seem like a hack to me.

This looks to me like @steffenbrem solution is correct for Sylius. @steffenbrem I guess you had to implement an PHPCRODM event listener to set the current locale? Would that be all that is needed to add document translation support in Sylius?

@koemeet
Copy link
Contributor

koemeet commented Jul 28, 2016

@dantleech Yes you are correct, the following rough steps are needed to do what I did:

  1. Document listener that listens on postLoad (those that extend Sylius\Component\Resource\Model\TranslatableInterface). I set the current locale on them and also map the translations collection to have the locale as the key of the array (this is needed for updating the collection with the build in translations type in Sylius).

    This will make sure that you can use newly fetched documents without triggering an error

  2. Override the setLocale method of the translation class (it extends Sylius\Component\Resource\Model\AbstractTranslation) with the following implementation:

/**
 * @param string $locale
 */
public function setLocale($locale)
{
    parent::setLocale($locale);

    $this->nodename = 'phpcr_locale:'.$locale;
}

Maybe this can be handled in a listener before persisting, haven't tested that yet. Would be a lot cleaner if we could get rid of this code.

  1. The mapping of the translatable document (e.g. a Page) should have the following to be able to fetch all translations:
<children name="translations" filter="phpcr_locale:*">
    <cascade>
        <cascade-all/>
    </cascade>
</children>

I think I did not forget something, but that is how I implemented it right now for myself. To me it feels a bit hacky, maybe some improvements can be made. Would be cool if we could get this implemented in Sylius.

@koemeet
Copy link
Contributor

koemeet commented Jul 28, 2016

A big issue may be that it always fetches all translations and you don't have any "extra lazy loading". For me it is not a big problem, but it may be for some people.

@dbu
Copy link
Member Author

dbu commented Jul 28, 2016 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants