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

QueryBuilder::setLocale has no effect for HYDRATE_DOCUMENT #605

Open
uwej711 opened this issue Mar 6, 2015 · 16 comments
Open

QueryBuilder::setLocale has no effect for HYDRATE_DOCUMENT #605

uwej711 opened this issue Mar 6, 2015 · 16 comments

Comments

@uwej711
Copy link
Contributor

uwej711 commented Mar 6, 2015

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

@uwej711
Copy link
Contributor Author

uwej711 commented Mar 6, 2015

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

@dantleech
Copy link
Contributor

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?

@uwej711
Copy link
Contributor Author

uwej711 commented Mar 6, 2015

@uwej711
Copy link
Contributor Author

uwej711 commented Mar 6, 2015

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.

@dbu
Copy link
Member

dbu commented Mar 6, 2015

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 sonata-project/SonataDoctrinePhpcrAdminBundle#328 - if you have some inputs there, that would be highly appreciated.

@uwej711
Copy link
Contributor Author

uwej711 commented Mar 6, 2015

@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 ...

@dbu
Copy link
Member

dbu commented Mar 6, 2015

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.

@uwej711
Copy link
Contributor Author

uwej711 commented Mar 7, 2015

See failing test in #607

@uwej711
Copy link
Contributor Author

uwej711 commented Mar 9, 2015

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

@dbu
Copy link
Member

dbu commented Mar 9, 2015 via email

@uwej711
Copy link
Contributor Author

uwej711 commented Mar 9, 2015

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.

@dbu
Copy link
Member

dbu commented Mar 9, 2015

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)

@uwej711
Copy link
Contributor Author

uwej711 commented Mar 9, 2015

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.

@dbu
Copy link
Member

dbu commented Mar 15, 2015

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?

@dantleech
Copy link
Contributor

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.

@dbu
Copy link
Member

dbu commented Mar 21, 2015

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.

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

3 participants