Massively update doesnt update default locale when model has early load relation 'translation'

[p-andrey]

Describe the bug If you specify in the model $with = ['translation'] (not 'translations'!) and massively update the model with translations (using update method), then the fields with the default locale are not updated.

If you prescribe $with = ['translations'] everything works correctly.

To Reproduce Add to Country model (in tests directory)

/**
 * The relations to eager load on every query.
 *
 * @var array
 */
protected $with = ['translation'];

Then create test

/** @test */
public function it_update_all_translated_locales_when_translation_relation_is_loaded(): void
{
    $this->app->make('config')->set('translatable.locales', ['de', 'en']);
    $this->app->setLocale('de');
    $this->app->make(Locales::class)->load();

    // First create country
    Country::create([
        'id' => 100,
        'code' => 'my',
        'de' => [
            'name' => 'Deutschland',
        ],
        'en' => [
            'name' => 'Germany',
        ],
    ]);

    $country = Country::find(100);

    // try mass update
    $country->update([
        'code' => 'my',
        'de' => [
            'name' => 'New Deutschland',
        ],
        'en' => [
            'name' => 'New Germany',
        ],
    ]);

    $country = Country::find(100);

    static::assertEquals(100, $country->getKey());
    static::assertEquals('New Deutschland', $country->getTranslation('de', false)->name);
    static::assertEquals('New Germany', $country->getTranslation('en', false)->name);
}

The Test was failed with result:

Failed asserting that two strings are equal.
Expected :'New Deutschland'
Actual   :'Deutschland'

Expected behavior In spite of the fact that early loading is specified in the model $with = ['translation'] - the bulk update must take place completely (for all languages).

Versions (please complete the following information)

• PHP: 7.4.16
• Database: MySQL Ver 8.0.23
• Laravel: v7.30.4
• Package: v11.9.1

[p-andrey]

The problem is that the model, when filling the fields, uses the method getTranslationByLocaleKey() which in case of the existence of the relationship translation returns this model and it is populated with new data.

And when base model saving, saves only models with the relation translations

In my case, before saveTranslations base method call, I save the relation translation, if it exists. And then I save the rest of the models.

Of course, this is not the best solution. This is a temporary crutch, but a working one :) I'm just trying to help solve this problem and suggest why it is not working correctly.

trait Translatable
{
    use \Astrotomic\Translatable\Translatable {
        saveTranslations as private traitSaveTranslations;
    }

    protected function saveTranslations(): bool
    {
        if ($this->relationLoaded('translation')) {
            $translation = $this->translation;

            if ($this->isTranslationDirty($translation)) {
                if (!empty($connectionName = $this->getConnectionName())) {
                    $translation->setConnection($connectionName);
                }

                $translation->setAttribute($this->getTranslationRelationKey(), $this->getKey());
                $translation->save();
            }
        }

        return $this->traitSaveTranslations();
    }
}

[Gummibeer]

Hey, thanks for this detailed report and I'm sorry for the inconvenience. From my side I have to say that I'm rejecting this as a bug as I absolutely have no plans to fix this (myself). The translation relationship was introduced for applications with a massive set of locales or translated data and the need to get top-notch performance for reading requests. As it's a real duplicated relationship on the same table (which will always lead to conflicts) this was a risky compromise from my side and I'm thinking about dropping it again in v12 because of the overhead it adds. Always loading this relationship was also never a plan - in addition to the general problems of this relationship also based in my strong opinion against the $with property.

I will keep this issue open and add proper labels so the community or you can possibly fix it. I will also merge it in v11 - as long as it's not a massive or too complicated change - I have some ideas that could solve and are relatively simple but no time to check. One is to check if the translation relation is loaded and it's dirty and if so also save this one. I would also accept a PR that adds this limitation of the translation relationship to the documentation.

Keep in mind that this effort could be removed in the next major version. I will also stand my ground in the rating of this issue as "not a bug".

[p-andrey]

I think adding a public function translation(): HasOne method is a good idea. Please do not delete it.

I've always used $wit = ['translations']; but in most requests (when users visit the site) all translations are loaded and more memory is consumed (not good).

The option using scope withTranslation() is also good, but need to write it in almost all places.

I see 2 options for solving the problem:

1. When filling occurs - use only translations relationship. (I write PR for this case https://github.com/Astrotomic/laravel-translatable/pull/216)
2. What has already been suggested - when saving, also keep the translation relationship.

[Gummibeer]

Both scenarios will keep the problem that both relations are out of sync. So I would prefer a solution that at least unsets the singular relation after saving so that all relations/models have the same data. Otherwise I see the next issue incoming.

$post->translation; // en
$post->update(['en' => ..., 'de' => ..., 'fr' => ...]); // runs update only on ->translations
$post->translation != $post->translations['en'];

This is only pseudo-code but should showcase the problem I mean. 😉

[p-andrey]

I agree.

Maybe then just destroy the translation relation before filling the model?

public function fill(array $attributes)
{
    if ($this->relationLoaded('translation')) {
        $this->unsetRelation('translation');
    }

    // ...
}

[Gummibeer]

I would do it post save - to prevent unsetting it if the save fails. But in general: yes.

[p-andrey]

@Gummibeer If I understand you correctly, then here is the solution: https://github.com/Astrotomic/laravel-translatable/pull/216/commits/44799e1727a204c8a533c41bb12418923cf317e3