Allow translations in resource bundle

[gonzalovilaseca]

With this small refactorization it's possible to create a translatable resource entity, eg:

sylius_resource:
        size:
            driver: doctrine/orm
            classes:
                model: MyBundle\ProductBundle\Entity\Size
                repository: Sylius\Bundle\ResourceBundle\Doctrine\ORM\TranslatableEntityRepository
                translatable:
                    targetEntity: MyBundle\ProductBundle\Entity\SizeTranslation
                    translatable_fields:
                        - presentation
        size_translation:
            driver: doctrine/orm
            classes:
                model: MyBundle\ProductBundle\Entity\SizeTranslation

It would be great to have the translatable repo mapped by default, but that will be in another PR.

[pjedrzejewski]

Thanks Gonzalo! 👍

[stloyd]

This is wrong IMO as it adds (missing anyway) dependency for ResourceBundle on the TranslationBundle...

[gonzalovilaseca]

@stloyd What do you mean? I don't see why this is adding a dependency of ResourceBundle on TranslationBundle.

[stloyd]

Using class from another bundle is the additional dependency.

[gonzalovilaseca]

Ok, I thought you meant that the dependency was the other way round.
If we allow translations in the resource bundle it's normal to have a dependency on that bundle.

[pjedrzejewski]

I am thinking if we really need Transltion and TranslationBundle, maybe add translations support to Sylius resource directly. What do you think guys? @stloyd @gonzalovilaseca @Sylius/core-team

[gonzalovilaseca]

@pjedrzejewski At some point I thought that too. Let's see what others say. The integration would be quite easy.

[pjedrzejewski]

Yes, also, would be cool if we can move the translation resource creation to the translatable node directly. So instead of:

sylius_resource:
    resources:
        app.book:
            driver: doctrine/orm
            classes:
                model: App\Entity\Book
                translatable:
                    model: App\Entity\BookTranslation
        app.book_translation:
            driver: doctrine/orm
            classes:
                model: App\Entity\BookTranslation

We can have:

sylius_resource:
    resources:
        app.book:
            driver: doctrine/orm
            classes:
                model: App\Entity\Book
                translatation:
                    model: App\Entity\BookTranslation
                    repository: # And other configurations.

I think that should be quite simple to do. Basically: translation node under classes registers another resources with name X_translation. :)

Same for bundle configs of course.

[gonzalovilaseca]

@pjedrzejewski In fact, while refactoring the bundle I started to do it that way, but then I realised that any configuration that might be added in the future to the classes array node should be added to the new translation node too. I saw a bit of code duplication there that's why I went back and left it as it is now.
I also considered this approach too:

sylius_resource:
    resources:
        app.book:
            driver: doctrine/orm
            classes:
                model: App\Entity\Book
                translatation: app.book_translation

        app.book_translation:
            driver: doctrine/orm
            classes:
                    model: App\Entity\BookTranslation
                    repository: # And other configurations.

But with this approach I had issues in the case where app.book_translation was defined in another bundle whose configuration might not have been loaded yet, but I can have a look again, I'm sure there are workarounds.

First option is easier, but as I said, configuration nodes will need to be duplicated.

[pjedrzejewski]

I'd say I am fine with the little duplication here, we can look at it later, when we perhaps add something more than translation. log could work in a similar way, right now we have a bit hacky handling of entity history. :)