Fix withTranslation query scope performance issues and fallback for c…

…ountry-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

@@ -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());
             },
         ]);
     }

tests/ScopesTest.php

@@ -1,6 +1,7 @@
 <?php
 
 use Dimsav\Translatable\Test\Model\Country;
+use Dimsav\Translatable\Test\Model\Vegetable;
 
 class ScopesTest extends TestsBase
 {
@@ -55,7 +56,7 @@ public function test_lists_of_translated_fields()
             'id'   => '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 */

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)

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'];
 }

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();
+        }
+    }
 }