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

Translatable after adding a new language #118

Closed
ZeeCoder opened this issue Jun 16, 2014 · 14 comments
Closed

Translatable after adding a new language #118

ZeeCoder opened this issue Jun 16, 2014 · 14 comments

Comments

@ZeeCoder
Copy link

After adding a new language, I can't update my entries properly.
When I try to update them, the added language (de) falls back to the default (en) overwriting it.

I have no problem adding new entries, and then updating them.
I only have this issue with entires persisted before I added a new language.

@ZeeCoder
Copy link
Author

ZeeCoder commented Jul 8, 2014

I got it working, but at a cost, so I'm still searching for a better solution.

The code I used, and where my problem is:

foreach ($translations as $locale => $trans_data) {
    $entity->translate($locale)->setName($trans_data['name']);
}

Basically this is how I add/edit the translations of new and old entities.

If I add a new language - which will result in a new $locale in the foreach -, then the translate() call will fall back for it to the default translation, instead of creating a new one with the new locale.

I think this behaviour wants to handle the case if I try to access a non-existent translation in for ex.: the twig template, but it also stops me from adding new translations.

What do I do wrong?

My hacky fix for now: I added the following snippet to my translatable entites:

public function getDefaultLocale()
{
    return null;
}

With this, no default translations will be found, and a new one is persisted.

Is there a solution, or does this mean that this behaviour cannot be used when a project will add new languages in the future?

@nifr
Copy link

nifr commented Jul 9, 2014

Related: #107

@docteurklein
Copy link
Contributor

I really have to check that. I'm sorry I don't have time to investigate, but if you have, don't hesitate to PR. We'll merge and tag a new release. If it breaks BC, we'll release a major release, and try to backport the fix on previous versions.

@amcastror
Copy link

@docteurklein , IMO, the problem is that the translations are been created by the same method used to get a translation. If there is a translation for "en", then you can't create new ones. I think that a simple solution would be to remove the creation part from that method (public function translate) and make a new method that will create (or get if already exists) a translation.

The drawback for this solution is that it's not backward compatible: new translations won't be created and codes will break. Maybe, a better solution it's to only create the new method (for creating translations) and leaving the other one unchanged.

If you like this approach, I can make the PR.

@docteurklein
Copy link
Contributor

I agree it would be proper to separate read and writes with 2 different methods.

The reason we made this at first was to always have a result (even if empty) in order to avoid if's in view files (f.e).

I think we can keep this behavior even if we split methods. The getter would return an empty object if no result (and no fallback). the setter would simply add.

Thoughts?

@docteurklein
Copy link
Contributor

PS: the fix proposed in #107 is very correct too.

@docteurklein
Copy link
Contributor

DerekRoth@b292f57

@ZeeCoder
Copy link
Author

That seems good to me.
Will there be a new release?
I would update the project I found the bug in.

@docteurklein
Copy link
Contributor

I'll try to do my best to find time to merge a fix, and will release right after.

@amcastror
Copy link

I think DerekRoth's fix is very good too.

@docteurklein , do you need anything else that we can help with?

@he-li
Copy link

he-li commented Oct 20, 2014

ping @docteurklein : what is the status of this issue ?

@docteurklein
Copy link
Contributor

sorry, didn't have time. What' the status of the different proposals ? Do you guys have a preference on which I should merge ? thanks :)

@ZeeCoder
Copy link
Author

I prefer the solution in #157 since it's backward compatible.
Basically translate() works as usual in twig templates, but you can use translate('xx', false) to update without falling back to the default language.

@ZeeCoder
Copy link
Author

I just realised, that the solution was merged in. :)

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

5 participants