Merge pull request #533 from derekmd/fix-untranslated-save-n-plus-1-q…

…ueries Fix N+1 queries when updating non-translated model attributes
dimsav · Feb 3, 2019 · 36c729f · 36c729f
2 parents cf6c8dc + c1096c5
commit 36c729f
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 22 deletions.
diff --git a/src/Translatable/Translatable.php b/src/Translatable/Translatable.php
@@ -252,31 +252,23 @@ public function setAttribute($key, $value)
      */
     public function save(array $options = [])
     {
-        if ($this->exists) {
-            if ($this->isDirty()) {
-                // If $this->exists and dirty, parent::save() has to return true. If not,
-                // an error has occurred. Therefore we shouldn't save the translations.
-                if (parent::save($options)) {
-                    return $this->saveTranslations();
-                }
-
+        if ($this->exists && ! $this->isDirty()) {
+            // If $this->exists and not dirty, parent::save() skips saving and returns
+            // false. So we have to save the translations
+            if ($this->fireModelEvent('saving') === false) {
                 return false;
-            } else {
-                // If $this->exists and not dirty, parent::save() skips saving and returns
-                // false. So we have to save the translations
-                if ($this->fireModelEvent('saving') === false) {
-                    return false;
-                }
-
-                if ($saved = $this->saveTranslations()) {
-                    $this->fireModelEvent('saved', false);
-                    $this->fireModelEvent('updated', false);
-                }
+            }
 
-                return $saved;
+            if ($saved = $this->saveTranslations()) {
+                $this->fireModelEvent('saved', false);
+                $this->fireModelEvent('updated', false);
             }
-        } elseif (parent::save($options)) {
-            // We save the translations only if the instance is saved in the database.
+
+            return $saved;
+        }
+
+        // We save the translations only if the instance is saved in the database.
+        if (parent::save($options)) {
             return $this->saveTranslations();
         }
 
@@ -452,6 +444,11 @@ protected function getLocaleSeparator()
     protected function saveTranslations()
     {
         $saved = true;
+
+        if (! $this->relationLoaded('translations')) {
+            return $saved;
+        }
+
         foreach ($this->translations as $translation) {
             if ($saved && $this->isTranslationDirty($translation)) {
                 if (! empty($connectionName = $this->getConnectionName())) {

diff --git a/tests/TranslatableTest.php b/tests/TranslatableTest.php
@@ -127,6 +127,29 @@ public function test_it_saves_translations_with_mutator()
         $this->assertEquals('5678', $translation->name);
     }
 
+    public function test_it_does_not_lazy_load_translations_when_updating_non_translated_attributes()
+    {
+        DB::enableQueryLog();
+
+        $country = Country::create(['code' => 'be']);
+        $this->assertFalse($country->relationLoaded('translations'));
+        $this->assertCount(1, DB::getQueryLog());
+
+        DB::flushQueryLog();
+
+        $country->update(['code' => 'de']);
+        $this->assertFalse($country->relationLoaded('translations'));
+        $this->assertCount(1, DB::getQueryLog());
+
+        DB::flushQueryLog();
+
+        $country->update(['name' => 'Germany']);
+        $this->assertTrue($country->relationLoaded('translations'));
+        $this->assertCount(2, DB::getQueryLog());
+
+        DB::disableQueryLog();
+    }
+
     public function test_it_uses_default_locale_to_return_translations()
     {
         $country = Country::whereCode('gr')->first();