Fix children translation based on the parent document.

[nicolas-bastien]

Fix children translation based on the parent document.
(Before the children locale was the request one)

[lsmith77]

the proper way would be to get the locale via the ClassMetadata.

if ($class->localeMapping) {
    $childrenHints = array('locale' => $class->reflFields[$class->localeMapping]->getValue($document));
} else {

[nicolas-bastien]

I saw a isDocumentTranslatable method which is not based on localeMapping but
return !empty($metadata->translator)
&& is_string($metadata->translator)
&& count($metadata->translatableFields) !== 0;

Shouldn't be cleaner to do this :
$metadata = $this->dm->getClassMetadata(get_class($document));
if ($this->isDocumentTranslatable($metadata)) {
}
?

[lsmith77]

i dont think that this is necessary .. the hint is then passed to loadTranslations() which in turn calls isDocumentTranslatable()

[lsmith77]

but the code i gave you is indeed wrong .. sine this will only work if the locale is mapped in the document

the better way to get the current locale would be:

$oid = spl_object_hash($document);
if (isset($this->documentLocales[$oid]['current'])) {
    $childrenHints = array('locale' => $this->documentLocales[$oid]['current']);
} else {

[lsmith77]

could you also try to add a functional test for this?

[lsmith77]

doh .. i also just found UnitOfWork::getLocale($document, $metadata)

[nicolas-bastien]

I'm on the functional test but right now I'm stick with this error (I try to run the tests without my modification first):

Fatal error: Call to a member function getRootNode() on a non-object in /home-filer21/nbastien/Workspace/PrestaConcept/website/vendor/doctrine/phpcr-odm/tests/Doctrine/Tests/ODM/PHPCR/Functional/Translation/AttributeTranslationStrategyTest.php on line 218

[nicolas-bastien]

Ok I just didn't initialized my testing database !

[nicolas-bastien]

So here is the functionnal test, I added a Comment entity adn a getChildren method on article

I my testCase you will noticed I test on $doc->getChildren() and $dm->getChildren($doc)

My modification corrects the $dm->getChildren($doc).

I'm waiting for your review !

[dbu]

when we wrote the multilang things, we did not do this on purpose. the reasoning was: the document object only ever exists in one language at the same time. so assume i load one of the children in english, then load its parent in french => the child will change its content to french at this point. or assume i load the parent in french and then again load the child through the document manager. now the child will have changed its language to english because of the dm.

see also the discussion on https://github.com/doctrine/phpcr-odm/wiki/Multilanguage - there we thought we would need to load children detached to avoid those issues.

i think the use case you fix in this pull request is far more natural and common than the weird thing i describe :-) so maybe indeed its better to do things this way - but then we need to put a warning into the documentation about the issue when working with several languages in parallel.

for the content of this PR: the locale is passed along to the proxy, right?

[nicolas-bastien]

for the content of this PR: the locale is passed along to the proxy, right?

Hi David, I don't get what you want to do in the proxy ?

[dbu]

are the children documents not proxy objects? so they are not loaded right away but only later if you actually try to access a value of them...

oh, and the language propagating does not happen for references, right? maybe it would be more consequent to do that as well.

[nicolas-bastien]

In this PR I just add the locale in the getChildren method, and simply add the locale in the hint when children document is created so I don't really see where proxy objects are important.

For the reference, yes I think you are right.

So from my little understanding, I should modify the getReferrers method, right ?
And in the test I can do an Author document to test it ?

[lsmith77]

@dbu: i see .. i think that in general we should tell people to really only work with one language per request.

if they do work with different locales then they should always load documents as explicit as possible. now i do agree that we should propagate the locale as much as possible. this also means that if you use findTranslation() to fetch a document that is already in memory in a different locale we should propagate the different locale to its children etc.

[dbu]

ok, makes sense to recommend only one language per request (or really understand what you do). propagating sounds right. what happens right now if the child is already loaded? is it reloaded in another language?

regarding proxy, @lsmith77 can you explain this better than i? or am i just confused and we don't use proxies here?

@nicolas-bastien referrers are the documents that point to this document, so the inverse relation of the references. there is ReferenceOne and ReferenceMany with the weak and strong options. (but all going through the same code i think.) i think if we propagate, we should do it for all cases, so references and referrers.

[lsmith77]

so i think what we need to do is the following:

1. add a way to pass the locale hint to proxies
2. ensure that all child, children, references, referrers already in memory are reloaded to the given locale

then in a 2nd step we need to review if we need to do anything to assist in the workflow of working with multiple locales in a single request. especially when working with a single document in different locales within the same request non sequentially (ie. i am manipulating the documents in the different locales and i am going back and forth). i see the following scenarios:

1. an admin panel where one can translate a document in multiple locales
2. ..?

[lsmith77]

@nicolas-bastien would you be willing to take the lead here?

[nicolas-bastien]

Hi Lukas, ok I booked tomorrow morning to work on it.

I don't know if my phpcr-odm understanding allow me to be lead but I can
push and it ;-)

Nico

On 26 January 2013 12:23, Lukas Kahwe Smith [email protected]:

@nicolas-bastien https://github.com/nicolas-bastien would you be
willing to take the lead here?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/226#issuecomment-12734124.

[lsmith77]

ok good :)

[lsmith77]

@nicolas-bastien: step one can be to create test cases for the desired behavior

[nicolas-bastien]

Hi Lukas, I have been reassigned this week so I will work on it this
afternoon.

Nico

On 30 January 2013 08:55, Lukas Kahwe Smith [email protected]:

@nicolas-bastien https://github.com/nicolas-bastien: step one can be to
create test cases for the desired behavior

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/226#issuecomment-12878196.

[dbu]

we recently noticed there is another issue. i did not yet get around to write the test but it seems the scenario is:

• default language is en
• i already have an existing document "a" that is only de
• i create another document "b" that is not multilanguage and assign a to a reference field of b $b->ref = $a;
• i persist b + flush

now the problem is that "a" now has a translation duplicated for locale en in addition to still have the de translation.