webfactory / WebfactoryPolyglotBundle

Symfony bundle simplifying translations in Doctrine.
MIT License
3 stars 2 forks source link

Show that new translations can be persisted #29

Closed mpdude closed 6 months ago

mpdude commented 7 months ago

This adds a test to show that the internal mechanisms of Translatable objects will take care of registering new translation entities with the Entity Manager. Explicit cascade={"persist"} configuration is not necessary.

mpdude commented 7 months ago

@MalteWunsch Do you have any preferences or thoughts about whether explicit cascade={"persist"} configuration would be better, helpful or similar?

If we'd prefer that, one question would be when and how the new entities could be returned to clients using this library, so that they can call EntityManager::persist() themselves for these entities.

MalteWunsch commented 6 months ago

I'm not really happy with the transparent replacement with *Translation entities to begin with. Back in the days I found it highly surprising and have vague memories of having difficulties with types, which are at least unintuitive and maybe a leaky abstraction. I'm afraid this replacement idea might not go well with the increased usage of types in the last decade.

I consider the possible need to return the replacements (so the user can persist them explicitly) to be only a symptom of that idea. I'd rather spend our efforts on the suspected root cause - or keep the behaviour as it is, with the absence of cascade={"persist"}.

mpdude commented 6 months ago

Together with @janopae, I've created #31 which allows type-safe field declarations, as long as the underlying values (those to be translated) are strings.

Regarding automated registration of new translation entities (instances created by code in this bundle) with the entity manager, I think that's a sane thing to do. The general idea of this bundle is that users need to provide the "translations" class according to the structure of their entities/data, but they never need to deal directly with it. Also, that translations class can completely omit any getters/setters etc.