Filament•12mo ago

adding trait Translatable will make all fields Translatable

@Dan Harrin
sorry to @ you 😅 continuing the discussion here I think it's easier and maybe others can help! the issue root: https://github.com/filamentphp/filament/issues/7156 long story short: when using the trait Translatable all none translateable attributes won't be saved. I found that there is two loops first one for $this->getTranslatableLocales() and the inside it another foreach getTranslatableAttributes it only saves one language, and on the next, it will revert to the original data, should it be saving only one lang, the selected one? attached a dump file

GitHub

adding trait Translatable will make all fields Translatable · Issue...

Package filament/filament Package Version v3 Laravel Version v10 Livewire Version v3 PHP Version v8.1 Problem description when using the trait Translatable all fields will be translatable by adding...

dump.html

54 Replies

Dan Harrin•12mo ago

in v3 we have changed it so you can add multiple languages when creating, not just 1 hence why the state path is scoped to the locale

Lara Zeus•12mo ago

the issue is in the next iteration the none translatable fields wont be exist and wont be saved

Dan Harrin•12mo ago

ill get back to you on the issue when i have a look dont really have time right now

Lara Zeus•12mo ago

no problem

Lara Zeus•12mo ago

adding this may help to debug later

Dan Harrin•12mo ago

that looks correct? there should be 2 copies of data

Lara Zeus•12mo ago

the array repeated_text should be the same in both lang since it's not a translatable attribute the problem also happens when I have a relationship, the records are deleted when saving since they not available for one of the languages

nielsdscrd•12mo ago

@larazeus just added my comment to your GitHub issue. I have the same issue. Initially all fields are updated in the database correctly. Then unexpected additional queries reset all non-translatable fields back to their original values.

Lara Zeus•12mo ago

Thank you. still didn't find any solution I'll try to create a test in a few days to check if I can replicate the issue

nielsdscrd•12mo ago

I use this modified save() function. It merges the correct data into every locale. I tried filtering out the data initially, but then validation may fail due to required fields.

php public function save(bool $shouldRedirect = true): void
{
    $this->authorizeAccess();

    $originalData = $this->data;
    $originalActiveLocale = $this->activeLocale;

    $nonTranslatableData = Arr::except($originalData[$originalActiveLocale], $this->record->getTranslatableAttributes());

    try {
        foreach ($this->getTranslatableLocales() as $locale) {
            $this->setActiveLocale($locale);

            // Merge the original non-translatable data into this locale's data
            $this->data[$locale] = array_merge($this->data[$locale], $nonTranslatableData);

            /** @internal Read the DocBlock above the following method. */
            $this->validateFormAndUpdateRecordAndCallHooks();
        }
    } catch (Halt $exception) {
        return;
    }

    $this->data = $originalData;
    $this->setActiveLocale($originalActiveLocale);

    /** @internal Read the DocBlock above the following method. */
    $this->sendSavedNotificationAndRedirect(shouldRedirect: $shouldRedirect);
}

php public function save(bool $shouldRedirect = true): void
{
    $this->authorizeAccess();

    $originalData = $this->data;
    $originalActiveLocale = $this->activeLocale;

    $nonTranslatableData = Arr::except($originalData[$originalActiveLocale], $this->record->getTranslatableAttributes());

    try {
        foreach ($this->getTranslatableLocales() as $locale) {
            $this->setActiveLocale($locale);

            // Merge the original non-translatable data into this locale's data
            $this->data[$locale] = array_merge($this->data[$locale], $nonTranslatableData);

            /** @internal Read the DocBlock above the following method. */
            $this->validateFormAndUpdateRecordAndCallHooks();
        }
    } catch (Halt $exception) {
        return;
    }

    $this->data = $originalData;
    $this->setActiveLocale($originalActiveLocale);

    /** @internal Read the DocBlock above the following method. */
    $this->sendSavedNotificationAndRedirect(shouldRedirect: $shouldRedirect);
}

Lara Zeus•12mo ago

i am bad at testing 🤦‍♂️ anyway I made this, and it's falling with the default save function but passes with the overwrite function @nielsdscrd suggested

it('can do it all', function () {
    // create a new form and set the names per language manually
    $form = Form::factory()->create();

    // create a form using filament
    livewire(FormResource\Pages\CreateForm::class)
        ->fillForm([
            'status' => $form->status,
            'repeated_text' => $form->repeated_text,
        ])
        ->call('create')
        ->assertHasNoFormErrors();

    // check it's actually saved to the db
    $this->assertDatabaseHas(Form::class, [
        'status' => $form->status,
        'repeated_text' => $form->repeated_text,
    ]);

    // go to edit page
    $this->get(FormResource::getUrl('edit', [
        'record' => $form,
    ]))->assertSuccessful();

    // get a new form and edit the names manually per language
    $newData = Form::factory()->create();

    // fill the form with the new data and call save
    livewire(FormResource\Pages\EditForm::class, [
        'record' => $form->getRouteKey(),
    ])
        ->fillForm([
            'status' => $newData->status,
            'repeated_text' => $newData->repeated_text,
        ])
        ->call('save')
        ->assertHasNoFormErrors();

    expect($form->refresh())
        ->status->toBe($newData->status)
        ->repeated_text->toBe($newData->repeated_text);

    $this->assertDatabaseHas(Form::class, [
        'status' => $newData->status,
        'repeated_text' => $newData->repeated_text,
    ]);
});

it('can do it all', function () {
    // create a new form and set the names per language manually
    $form = Form::factory()->create();

    // create a form using filament
    livewire(FormResource\Pages\CreateForm::class)
        ->fillForm([
            'status' => $form->status,
            'repeated_text' => $form->repeated_text,
        ])
        ->call('create')
        ->assertHasNoFormErrors();

    // check it's actually saved to the db
    $this->assertDatabaseHas(Form::class, [
        'status' => $form->status,
        'repeated_text' => $form->repeated_text,
    ]);

    // go to edit page
    $this->get(FormResource::getUrl('edit', [
        'record' => $form,
    ]))->assertSuccessful();

    // get a new form and edit the names manually per language
    $newData = Form::factory()->create();

    // fill the form with the new data and call save
    livewire(FormResource\Pages\EditForm::class, [
        'record' => $form->getRouteKey(),
    ])
        ->fillForm([
            'status' => $newData->status,
            'repeated_text' => $newData->repeated_text,
        ])
        ->call('save')
        ->assertHasNoFormErrors();

    expect($form->refresh())
        ->status->toBe($newData->status)
        ->repeated_text->toBe($newData->repeated_text);

    $this->assertDatabaseHas(Form::class, [
        'status' => $newData->status,
        'repeated_text' => $newData->repeated_text,
    ]);
});

I also updated the repo: https://github.com/lara-zeus/test-fila-three/pull/2 CC: @Dan Harrin

GitHub

test translatable isseu by atmonshi · Pull Request #2 · lara-zeus/t...

Lara Zeus•12mo ago

update: the issue won't happen if I disable other languages and keep only one
is anyone using translatable in their apps? to confirm the issue?

mitchbricks•12mo ago

@larazeus I use it and only if I set my translation to English it saves the other fields I use Dutch as main language in my app and it doesn't save other fields until I choose English from the language selector It is only so on updating, creating works fine @nielsdscrd your snippet seems to fix it

Lind•12mo ago

I'm also struggling with this and i think the root cause is each language is its own form? So if you don't change the non-translatable field in the last language that is what is gonna stick in the database? I'm working on disabling all non translatable attributes on all languages but the default one and then override the other forms with the data from the default language form. But thats alot of checks xD

Dan Harrin•12mo ago

the way it should work is that while there are multiple forms, one for each language, when you switch langauges the non-translatable attributes should be carried over to the new language form, if that makes sense? and when you're on ES language, the EN language form data should not contain any non-translatable attributes as we only store one copy of the non-translatable attributes thats how I think i built it for v3, which allows you to switch languages without saving which was a complaint about v2

mitchbricks•12mo ago

That sounds logical yes, however it does only save all fields on one specific language, and not all I’m relatively new to Filament, so I am not sure if it would be the most logical for me to check if I can propose a PR for the plugin. If one of you don’t beat me to it I might look into it next week

Lind•12mo ago

If i dump the $data in the save() function i can see an array with all the languages as keys and the data from their forms, changing a non-translatable field in 1 form doesn't change it in all forms. I don't know if this is the right bevaviour, but its what i've seen in my own code and when i run the repo linked earlier. The issue with the fix niels provides assumes you're still on the language you changed the non-translatable field when you hit save. If you're not, it'll still override it with what was in the last form.

Lara Zeus•12mo ago

i think maybe because all inputs scoped to the lang prefix es.status and en.status

Dan Harrin•12mo ago

thats deliberate

Lara Zeus•12mo ago

the $data will include all fields values so this way there is two versions of the non-translatable fields with two diff values

Dan Harrin•12mo ago

where is $data

Lara Zeus•12mo ago

validateFormAndUpdateRecordAndCallHooks in EditRecord the data is not mutateFormDataBeforeSave for Translatable

Dan Harrin•12mo ago

that will run for each language, the data is not for all languages at once and you have to put the non-translatable attributes in there otherwise the validation would fail

Lara Zeus•12mo ago

I know 🙂 this is the issue: data for en: name: en name status: new2 <<< edited data for es: name: es name status: new << old the save will run for each lang and it's in loop the first will correctly updated the second loop will revert the changes status is not traslatable Is there any reason for having this structure? the user can switch to any lang and change any fields, eventually it will be impossible to know what data is correct and final to save them to db

Dan Harrin•12mo ago

i dont understand how its impossible if its done properly, where the data is carried between languages the reason for storing all locales at once is so that you dont have to save when switching between them, which was a complaint

Lara Zeus•12mo ago

which part that carry the data between langs? looks like it always there is two arrays with diff values

Lind•12mo ago

Could possible work out something with the updated() function, seems to be able to do real-time saving i LW3

Lara Zeus•12mo ago

I think thats mean the data will save immediately to db even if the user didnt actually save the form!?

Lind•12mo ago

Yea, and i don't think its implemented in filament anyway. But i was just thinking you could then update the other forms when you swapped language with data from the DB. But i'm not knowledgeable enough on either livewire or filament for it. Already out of my depth here So after more testing, i figured the solution was in updatedActiveLocale on EditRecord/Translatable and after finding a solution there i went looking at CreateRecord, where theres already implemented a solution. So i guess you could just copy that on to EditRecord

Dan Harrin•12mo ago

oh really, let me try that out quickly then i havent been able to test this locally yet because i thought this would be a tricky fix, but if its that easy i might as well do it now

Lind•12mo ago

Yea, my solution copied it out to all languages but copying from old to new is probably quicker incase you have alot of languages and non translateable fields i guess. I did it after the validate in edit, not sure if thats the correct place 🙂

Dan Harrin•12mo ago

@mitchbricks @nielsdscrd @larazeus @l.nd does this fix it? please pull down the branch or add the code to your Edit/Translatable trait https://github.com/filamentphp/filament/pull/7471

GitHub

fix: Translatable edit by danharrin · Pull Request #7471 · filament...

Changes have been thoroughly tested to not break existing functionality. New functionality has been documented or existing documentation has been updated to reflect changes. Visual changes are ex...

Lind•12mo ago

Works on my end

Dan Harrin•12mo ago

no issues at all now? with the whole plugin?

Lind•12mo ago

No errors, i only copied the function into a fresh pull of the repo in the other issue.. Not exactly sure whats the best approach to get the plugin from the monorepo?

Dan Harrin•12mo ago

yeah just checking that i dont need to fix anything else apart from this issue. the thread is long and i havent read much of it

Lara Zeus•12mo ago

willl.... 🙂 part of the issue fixed, now if I fill a field and change the lang, the updatedActiveLocale works perfectly but: edit a form and edit and save without changing the local not working "notice the status revert back to old value"

guilebc•12mo ago

Hello, same here =(, but this https://github.com/filamentphp/filament/issues/7156#issuecomment-1663512999 work for me

GitHub

adding trait Translatable will make all fields Translatable · Issue...

Lara Zeus•12mo ago

I had to remove $this->setActiveLocale($locale); and $this->setActiveLocale($originalActiveLocale); from function save and it worked now will do more testing

Lind•12mo ago

I added this code to my save function, that seemed to have fixed it:

    public function save(bool $shouldRedirect = true): void
    {
        $this->authorizeAccess();

        $originalActiveLocale = $this->activeLocale;

        try {
            foreach ($this->getTranslatableLocales() as $locale) {
                $this->setActiveLocale($locale);

                /* ADDITION start */
                $this->data[$locale] = array_merge(
                    $this->data[$locale] ?? [],
                    Arr::except($this->data[$originalActiveLocale], app(static::getModel())->getTranslatableAttributes()),
                );
                /* ADDITION end */

                /** @internal Read the DocBlock above the following method. */
                $this->validateFormAndUpdateRecordAndCallHooks();
            }
        } catch (Halt $exception) {
            return;
        }

        $this->setActiveLocale($originalActiveLocale);

        /** @internal Read the DocBlock above the following method. */
        $this->sendSavedNotificationAndRedirect(shouldRedirect: $shouldRedirect);
    }

    public function save(bool $shouldRedirect = true): void
    {
        $this->authorizeAccess();

        $originalActiveLocale = $this->activeLocale;

        try {
            foreach ($this->getTranslatableLocales() as $locale) {
                $this->setActiveLocale($locale);

                /* ADDITION start */
                $this->data[$locale] = array_merge(
                    $this->data[$locale] ?? [],
                    Arr::except($this->data[$originalActiveLocale], app(static::getModel())->getTranslatableAttributes()),
                );
                /* ADDITION end */

                /** @internal Read the DocBlock above the following method. */
                $this->validateFormAndUpdateRecordAndCallHooks();
            }
        } catch (Halt $exception) {
            return;
        }

        $this->setActiveLocale($originalActiveLocale);

        /** @internal Read the DocBlock above the following method. */
        $this->sendSavedNotificationAndRedirect(shouldRedirect: $shouldRedirect);
    }

I think we should just look closer at the CreateRecord trait again. I think it works as intended. Went to bed but couldn't stop thinking about it 😅

Lara Zeus•12mo ago

ya it's 2 am here and I cant sleep try remove $this->setActiveLocale($locale); and $this->setActiveLocale($originalActiveLocale); from function save this will fix it too your solution is the same as dan in the PR

Lind•12mo ago

The solution I just wrote is not the one Dan made a PR for. The PR fixes the swapping between languages. The second one I posted here is to avoid other forms overwriting non-translatables. But I'm gonna look deeper into it tomorrow because the CreateRecord trait seems to be written as intended

black_gallardo•12mo ago

Hello, i am facing the same issue @larazeus mentioned. I have 2 locales (en, ar) and when update a form that contains non translatable field (toggle in my case) it will change again back to basic stats and i noticed in the request it goes twice: data.en.is_published = true , then data.ar.is_published = false which is the previous value

black_gallardo•12mo ago

as can you see, is_published is not a in $translatable array of model

black_gallardo•12mo ago

here what i get when i try to debug with ray:

    protected function afterSave()
    {
        ray($this->record);
    }

black_gallardo•12mo ago

for me the PR posted did not fix the issue, but this fix work: https://github.com/filamentphp/filament/issues/7156#issuecomment-1663512999 with a small issue that when save and switch the locale from the dropdown the old result will fill the form again but acceptable for me

GitHub

adding trait Translatable will make all fields Translatable · Issue...

Lind•12mo ago

What if you implement the small fix i suggested here https://discord.com/channels/883083792112300104/1133478284676562974/1137157678511177838 into the original save function in the PR?

black_gallardo•12mo ago

works perfectly for me 👍🏽

Lara Zeus•12mo ago

I added your fix to a new PR https://github.com/filamentphp/filament/pull/7503 if you can test and confirm

GitHub

fix Translatable edit by atmonshi · Pull Request #7503 · filamentph...

fix for translatable edit #7156 and #7471

nielsdscrd•12mo ago

Much like my own, this is a work-around, not a true fix. Replacing the incorrect data at the moment of Saving means the problem may pop up in other places as the actual cause is not resolved. It may be difficult for us to trace it back to the root, but hopefully @Dan Harrin or someone else with deeper knowledge of Filament can find the time to have look at this.

black_gallardo•12mo ago

@Dan Harrin using this PR code solved the problem (https://github.com/filamentphp/filament/pull/7503) but the one in the repo now is not, because setting "$nonTranslatableData" out of foreach loop causing bugs when using media library (https://github.com/filamentphp/spatie-laravel-translatable-plugin/blob/3.x/src/Resources/Pages/EditRecord/Concerns/Translatable.php#L43)

GitHub

fix Translatable edit by atmonshi · Pull Request #7503 · filamentph...

fix for translatable edit #7156 and #7471

GitHub

spatie-laravel-translatable-plugin/src/Resources/Pages/EditRecord/C...

[READ ONLY] Subtree split of the Filament spatie/laravel-translatable Plugin (see filamentphp/filament) - filamentphp/spatie-laravel-translatable-plugin

black_gallardo•12mo ago

Error: Call to a member function hasGeneratedConversion() on null

Dan Harrin•12mo ago

sure, open an issue with a reproduction repository then

black_gallardo•12mo ago

https://github.com/filamentphp/filament/issues/7591 thank you

GitHub

Translatable resource with media bug · Issue #7591 · filamentphp/fi...

Package filament/spatie-laravel-translatable-plugin Package Version v3.0.13 Laravel Version v10.18.0 Livewire Version v3.0.0-beta.8 PHP Version PHP 8.2.8 Problem description Editing translatable re...

Gaming

Programming

adding trait Translatable will make all fields Translatable