phpcr-odm icon indicating copy to clipboard operation
phpcr-odm copied to clipboard

QueryBuilder::setLocale has no effect for HYDRATE_DOCUMENT

Open uwej711 opened this issue 9 years ago • 16 comments

in DocumentManager::getDocumentsByPhpcrQuery the locale of the query is ignored.

uwej711 avatar Mar 06 '15 11:03 uwej711

The locale is actually not even passed from the QueryBuilder to the Query, it is lost ...

uwej711 avatar Mar 06 '15 11:03 uwej711

Not sure I understand. a PHPCR QueryInterface does not know anything about locales.

Why use the PHPCR query object instead of the ODM query builder?

dantleech avatar Mar 06 '15 11:03 dantleech

From http://doctrine-phpcr-odm.readthedocs.org/en/latest/reference/query-builder.html#querying-translated-documents

uwej711 avatar Mar 06 '15 11:03 uwej711

This suggest that you just use setLocale to get your content in a different language. But this does not work, the content is always returned in the current locale.

Background: I want to allow my editors to edit foreign language without changing the language of the rest of the UI. This works in Sonata in the edit view since there I can call doLoadTranslation before showing the content. But for the list view I need to have the query return the content in the correct language.

uwej711 avatar Mar 06 '15 11:03 uwej711

i fear you mixed up the phpcr-odm query builder and the phpcr query builder, could that be? on phpcr level, there is indeed nothing with locales.

  • there is createPhpcrQueryBuilder() which has no arguments and does not care about the language.
  • there is createPhpcrQuery and createPhpcrQuery which accept a phpcr query with a specified query language, as in "SQL2" or "XPATH". this is not a locale.

i think what you want to do is getLocaleChooserStrategy()->setLocale('x').

btw, i think the feature you describe sounds a lot like https://github.com/sonata-project/SonataDoctrinePhpcrAdminBundle/pull/328 - if you have some inputs there, that would be highly appreciated.

dbu avatar Mar 06 '15 13:03 dbu

@dbu this is exactly where i started from, but no, I do not mix ODM and PHPCR QueryBuilder. I will try to do a failing test ...

uwej711 avatar Mar 06 '15 13:03 uwej711

ok, sorry. then that sounds like a bug, or a feature that we thought of but in the end did not finish to implement. glad if you can provide a functional test so we can start fixing this.

dbu avatar Mar 06 '15 14:03 dbu

See failing test in #607

uwej711 avatar Mar 07 '15 15:03 uwej711

see https://github.com/valiton-forks/phpcr-odm/tree/sd_fixes_1_2_2 for a minimal fix (not really tested yet)

uwej711 avatar Mar 09 '15 11:03 uwej711

can you open a WIP pull request that we can comment on?

i think we should wrap $this->dm->getDocumentsByPhpcrQuery($this->query, $this->documentClass, $this->primaryAlias); into a try-catch and change the default locale while fetching documents, then change it back. otherwise we add quite some overhead i fear. and possibly destroy lazy loading.

dbu avatar Mar 09 '15 11:03 dbu

I don't like the idea of setting a rather global setting just for doing this, currently it might work but I can see scenarios where this could break. Rather create a equivalent to findMany that works with locales like findTranslation. What I also miss is a shortcut for doLoadTranslation in the DocumentManager. There seems to be no other way to load different translations into the same document instance.

uwej711 avatar Mar 09 '15 11:03 uwej711

yeah, i see that this is not exactly elegant. maybe the getDocumentsByPhpcrQuery could pass around a locale?

the document manager has findTranslation, this should be all that is needed. findTranslation updates the already loaded document, it does not create duplicates (as documents have the same id regardless of the locale they are currently in)

dbu avatar Mar 09 '15 12:03 dbu

OK, checked findTranslation. But findTranslation uses the id of the document. If I already have a document, I need the meta class to extract the id before I'm able to use findTranslation. I would prefer a DocumentManager::loadTranslation($document, $locale, $fallback) method.

uwej711 avatar Mar 09 '15 12:03 uwej711

or maybe we could redefine findTranslation to accept the document instead of an id as well. there is no reason why a valid document should not represent the document just as well as the id. wdyt?

dbu avatar Mar 15 '15 10:03 dbu

Semantically that would seem strange. "loading" is more appropriate, also it would make the first argument redundant:

$dm->findTranslation('SomeClassNameThatDoesntMatter', $document, 'de');

So I would vote for loadTranslation I think.

dantleech avatar Mar 15 '15 10:03 dantleech

see also #616 - most DocumentManager methods depend on the locale returned by LocaleChooser, we should have a generic solution without exploding the number of methods.

dbu avatar Mar 21 '15 17:03 dbu