From 68a0b789d66dc14ce0af81fe5f953be3d5e68f2a Mon Sep 17 00:00:00 2001 From: Raphael Saunier Date: Thu, 4 Jan 2018 20:55:32 +0100 Subject: [PATCH] Fix withTranslation query scope performance issues and fallback for country-based locales (#417) * Fix failing tests by checking for presence of translations as a subset Tests were failing because the return array also contained an unexpected 'translations' key. * Correctly load country-based fallback when calling withTranslation. When using the `withTranslation` query scope along with country-based locales, the generated query didn't include the country fallback (e.g. `de-DE` should first fall back to `de` before using the fallback defined in the configuration). Also fix #241 by generating a WHERE IN clause instead chaining AND/ORs * StyleCI * StyleCI 2 --- src/Translatable/Translatable.php | 10 +++++--- tests/ScopesTest.php | 23 +++++++++++++++-- tests/TestsBase.php | 3 ++- tests/models/Vegetable.php | 4 ++- tests/seeds/AddFreshSeeds.php | 41 +++++++++++++++++++++++++++++++ 5 files changed, 74 insertions(+), 7 deletions(-) diff --git a/src/Translatable/Translatable.php b/src/Translatable/Translatable.php index cb33d2b..34b9e7d 100644 --- a/src/Translatable/Translatable.php +++ b/src/Translatable/Translatable.php @@ -582,11 +582,15 @@ public function scopeWithTranslation(Builder $query) { $query->with([ 'translations' => function (Relation $query) { - $query->where($this->getTranslationsTable().'.'.$this->getLocaleKey(), $this->locale()); - if ($this->useFallback()) { - return $query->orWhere($this->getTranslationsTable().'.'.$this->getLocaleKey(), $this->getFallbackLocale()); + $locale = $this->locale(); + $countryFallbackLocale = $this->getFallbackLocale($locale); // e.g. de-DE => de + $locales = array_unique([$locale, $countryFallbackLocale, $this->getFallbackLocale()]); + + return $query->whereIn($this->getTranslationsTable().'.'.$this->getLocaleKey(), $locales); } + + return $query->where($this->getTranslationsTable().'.'.$this->getLocaleKey(), $this->locale()); }, ]); } diff --git a/tests/ScopesTest.php b/tests/ScopesTest.php index 6f90170..5edc4c5 100644 --- a/tests/ScopesTest.php +++ b/tests/ScopesTest.php @@ -1,6 +1,7 @@ '1', 'name' => 'Griechenland', ]]; - $this->assertEquals($list, Country::listsTranslations('name')->get()->toArray()); + $this->assertArraySubset($list, Country::listsTranslations('name')->get()->toArray()); } public function test_lists_of_translated_fields_with_fallback() @@ -72,7 +73,7 @@ public function test_lists_of_translated_fields_with_fallback() 'id' => 2, 'name' => 'France', ]]; - $this->assertEquals($list, $country->listsTranslations('name')->get()->toArray()); + $this->assertArraySubset($list, $country->listsTranslations('name')->get()->toArray()); } public function test_scope_withTranslation_without_fallback() @@ -95,6 +96,24 @@ public function test_scope_withTranslation_with_fallback() $this->assertSame('Griechenland', $loadedTranslations[1]['name']); } + public function test_scope_withTranslation_with_country_based_fallback() + { + App::make('config')->set('translatable.fallback_locale', 'en'); + App::make('config')->set('translatable.use_fallback', true); + App::setLocale('en-GB'); + $result = Vegetable::withTranslation()->find(1)->toArray(); + $this->assertSame('courgette', $result['name']); + + App::setLocale('de-CH'); + $result = Vegetable::withTranslation()->find(1)->toArray(); + $expectedTranslations = [ + ['name' => 'zucchini', 'locale' => 'en'], + ['name' => 'Zucchini', 'locale' => 'de'], + ['name' => 'Zucchetti', 'locale' => 'de-CH'], + ]; + $this->assertArraySubset($expectedTranslations, $result['translations']); + } + public function test_whereTranslation_filters_by_translation() { /** @var Country $country */ diff --git a/tests/TestsBase.php b/tests/TestsBase.php index c73f6be..ef10001 100644 --- a/tests/TestsBase.php +++ b/tests/TestsBase.php @@ -123,7 +123,8 @@ protected function getEnvironmentSetUp($app) 'collation' => 'utf8_unicode_ci', 'strict' => false, ]); - $app['config']->set('translatable.locales', ['el', 'en', 'fr', 'de', 'id']); + $locales = ['el', 'en', 'fr', 'de', 'id', 'en-GB', 'en-US', 'de-DE', 'de-CH']; + $app['config']->set('translatable.locales', $locales); } protected function getPackageAliases($app) diff --git a/tests/models/Vegetable.php b/tests/models/Vegetable.php index 900feb9..9d13737 100644 --- a/tests/models/Vegetable.php +++ b/tests/models/Vegetable.php @@ -9,7 +9,9 @@ class Vegetable extends Eloquent { use Translatable; - protected $primaryKey = 'vegetable_identity'; + protected $primaryKey = 'identity'; + + protected $translationForeignKey = 'vegetable_identity'; public $translatedAttributes = ['name']; } diff --git a/tests/seeds/AddFreshSeeds.php b/tests/seeds/AddFreshSeeds.php index 08ddf5e..c5e7afe 100644 --- a/tests/seeds/AddFreshSeeds.php +++ b/tests/seeds/AddFreshSeeds.php @@ -2,8 +2,10 @@ use Dimsav\Translatable\Test\Model\City; use Dimsav\Translatable\Test\Model\Country; +use Dimsav\Translatable\Test\Model\Vegetable; use Dimsav\Translatable\Test\Model\CityTranslation; use Dimsav\Translatable\Test\Model\CountryTranslation; +use Dimsav\Translatable\Test\Model\VegetableTranslation; class AddFreshSeeds { @@ -42,6 +44,25 @@ public function run() ]; $this->createCityTranslations($cityTranslations); + + $vegetables = [ + ['vegetable_identity' => 1], + ['vegetable_identity' => 2], + ]; + + $this->createVegetables($vegetables); + + $vegetableTranslations = [ + ['vegetable_identity' => 1, 'locale' => 'en', 'name' => 'zucchini'], + ['vegetable_identity' => 1, 'locale' => 'en-GB', 'name' => 'courgette'], + ['vegetable_identity' => 1, 'locale' => 'en-US', 'name' => 'zucchini'], + ['vegetable_identity' => 1, 'locale' => 'de', 'name' => 'Zucchini'], + ['vegetable_identity' => 1, 'locale' => 'de-CH', 'name' => 'Zucchetti'], + ['vegetable_identity' => 2, 'locale' => 'en', 'name' => 'aubergine'], + ['vegetable_identity' => 2, 'locale' => 'en-US', 'name' => 'eggplant'], + ]; + + $this->createVegetableTranslations($vegetableTranslations); } private function createCountries($countries) @@ -85,4 +106,24 @@ private function createCityTranslations($translations) $translation->save(); } } + + private function createVegetables($vegetables) + { + foreach ($vegetables as $vegetable) { + $vegetable = new Vegetable(); + $vegetable->identity = $vegetable['identity']; + $vegetable->save(); + } + } + + private function createVegetableTranslations($translations) + { + foreach ($translations as $data) { + $translation = new VegetableTranslation(); + $translation->vegetable_identity = $data['vegetable_identity']; + $translation->locale = $data['locale']; + $translation->name = $data['name']; + $translation->save(); + } + } }