F
Filamentβ€’17mo ago
Lara Zeus

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...
54 Replies
Dan Harrin
Dan Harrinβ€’17mo 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
Lara ZeusOPβ€’17mo ago
the issue is in the next iteration the none translatable fields wont be exist and wont be saved
Dan Harrin
Dan Harrinβ€’17mo ago
ill get back to you on the issue when i have a look dont really have time right now
Lara Zeus
Lara ZeusOPβ€’17mo ago
no problem
Lara Zeus
Lara ZeusOPβ€’17mo ago
adding this may help to debug later
Dan Harrin
Dan Harrinβ€’17mo ago
that looks correct? there should be 2 copies of data
Lara Zeus
Lara ZeusOPβ€’16mo 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
nielsdscrdβ€’16mo 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
Lara ZeusOPβ€’16mo 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
nielsdscrdβ€’16mo 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
Lara ZeusOPβ€’16mo 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
Lara Zeus
Lara ZeusOPβ€’16mo 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
MitchBricksβ€’16mo 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
Lindβ€’16mo 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
Dan Harrinβ€’16mo 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
MitchBricksβ€’16mo 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
Lindβ€’16mo 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
Lara ZeusOPβ€’16mo ago
i think maybe because all inputs scoped to the lang prefix es.status and en.status
Dan Harrin
Dan Harrinβ€’16mo ago
thats deliberate
Lara Zeus
Lara ZeusOPβ€’16mo 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
Dan Harrinβ€’16mo ago
where is $data
Lara Zeus
Lara ZeusOPβ€’16mo ago
validateFormAndUpdateRecordAndCallHooks in EditRecord the data is not mutateFormDataBeforeSave for Translatable
Dan Harrin
Dan Harrinβ€’16mo 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
Lara ZeusOPβ€’16mo 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
Dan Harrinβ€’16mo 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
Lara ZeusOPβ€’16mo ago
which part that carry the data between langs? looks like it always there is two arrays with diff values
Lind
Lindβ€’16mo ago
Could possible work out something with the updated() function, seems to be able to do real-time saving i LW3
Lara Zeus
Lara ZeusOPβ€’16mo ago
I think thats mean the data will save immediately to db even if the user didnt actually save the form!?
Lind
Lindβ€’16mo 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
Dan Harrinβ€’16mo 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
Lindβ€’16mo 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
Dan Harrinβ€’16mo 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
Lindβ€’16mo ago
Works on my end
Dan Harrin
Dan Harrinβ€’16mo ago
no issues at all now? with the whole plugin?
Lind
Lindβ€’16mo 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
Dan Harrinβ€’16mo 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
Lara ZeusOPβ€’16mo 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
guilebcβ€’16mo ago
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...
Lara Zeus
Lara ZeusOPβ€’16mo ago
I had to remove $this->setActiveLocale($locale); and $this->setActiveLocale($originalActiveLocale); from function save and it worked now will do more testing
Lind
Lindβ€’16mo 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
Lara ZeusOPβ€’16mo 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
Lindβ€’16mo 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
black_gallardoβ€’16mo 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
black_gallardoβ€’16mo ago
as can you see, is_published is not a in $translatable array of model
black_gallardo
black_gallardoβ€’16mo ago
here what i get when i try to debug with ray: protected function afterSave() { ray($this->record); }
black_gallardo
black_gallardoβ€’16mo 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...
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...
Lind
Lindβ€’16mo 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
black_gallardoβ€’16mo ago
works perfectly for me πŸ‘πŸ½
Lara Zeus
Lara ZeusOPβ€’16mo ago
I added your fix to a new PR https://github.com/filamentphp/filament/pull/7503 if you can test and confirm
nielsdscrd
nielsdscrdβ€’16mo 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
black_gallardoβ€’16mo 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
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
black_gallardoβ€’16mo ago
Error: Call to a member function hasGeneratedConversion() on null
Dan Harrin
Dan Harrinβ€’16mo ago
sure, open an issue with a reproduction repository then
black_gallardo
black_gallardoβ€’16mo ago
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...
Want results from more Discord servers?
Add your server