Skip to content

Demonstrate expected behaviour for QueryBuilder::setLocale#607

Open
uwej711 wants to merge 1 commit into
doctrine:2.0.xfrom
uwej711:demonstrate_issues_605
Open

Demonstrate expected behaviour for QueryBuilder::setLocale#607
uwej711 wants to merge 1 commit into
doctrine:2.0.xfrom
uwej711:demonstrate_issues_605

Conversation

@uwej711

@uwej711 uwej711 commented Mar 6, 2015

Copy link
Copy Markdown
Contributor

Failing test for issue #605

I would expect setLocale on the QueryBuilder would return the found document in the specified locale

@dantleech

Copy link
Copy Markdown
Contributor

I will try and have a look at this tomorrow. This is surely meant to work but couldn't find any existing tests, very odd.

@uwej711

uwej711 commented Mar 8, 2015

Copy link
Copy Markdown
Contributor Author

Maybe adding a doLoadTranslation here would be a way to do it:
https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/Query/Query.php#L183

@dbu

dbu commented Mar 9, 2015 via email

Copy link
Copy Markdown
Member

@dantleech

Copy link
Copy Markdown
Contributor

It seems that it would be ideal to add a $locale argument to the loadDocumentsByPhpcrQuery, but that cascades then to findMany() (which can then pass the locale hint to the unit of work getOrCreateDocuments method).

But by this logic we should add the $locale argument to all of the document manager methods (including persist). If that is a good idea or not, I don't know, but I guess its out of the scope here.

Another option would be to add translation aware methods, e.g. getTranslatedDocumentsByPhpcrQuery, findManyTranslated.

But I vote for @dbu s suggestion for now.. and we could create an issue to solve this in 2.0

@dbu

dbu commented Mar 14, 2015

Copy link
Copy Markdown
Member

the only other option i see would be to remove the possiblility to specify the locale in the query builder. that would be more consistent with the rest. then we would say whatever you do, you need yourself to set the default locale if you want to change something about what the default locale is.

i think we can say that changing the default locale for a single call is out of scope of phpcr-odm, and you just need to change the default if you need this.

or we could offer some sort of wrapper-system where you can say $dm->defaultLocale('de')->find... defaultLocale() would return a dm decorator that changes the locale on the fly and changes it back.

@dantleech

Copy link
Copy Markdown
Contributor

Opened issue #623 to discuss how to handle this

@lsmith77 lsmith77 changed the title Demonstrate expected behaviour for QueryBuilder::setLocale [RFC] Demonstrate expected behaviour for QueryBuilder::setLocale May 1, 2015
@lsmith77 lsmith77 changed the title [RFC] Demonstrate expected behaviour for QueryBuilder::setLocale Demonstrate expected behaviour for QueryBuilder::setLocale May 1, 2015
@lsmith77 lsmith77 added ready and removed in progress labels May 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants