filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
18.39k stars 2.87k forks source link

Repeater does not work with translations #8328

Closed pdaether closed 5 months ago

pdaether commented 1 year ago

Package

filament/spatie-laravel-translatable-plugin

Package Version

v3.0.40

Laravel Version

v10.22.0

Livewire Version

v3.0.2

PHP Version

PHP 8.2.9

Problem description

I have a form for editing a model with a Repeater for a hasMany relation. With filamentphp/spatie-laravel-translatable-plugin the translation of all translatable fields of the main model works like a charm, but the translation of the translatable fields from the related models does not work. When I switch the language with the language switcher, the fields of the main model gets updated accordingly, but not inside of the repeater.

image

Also after changing a translatable text from one of the related model in the Repeater, all languages gets overwritten. Eg editing the german translation results from {"en":"Text EN","de":"Text DE"} to {"en":"New Text DE","de":"New Text DE"}.

Expected behavior

Expected behavior would be that it is possible to translate the translatable fields inside of a Repeater form element and that it behaves like the fields of the main model itself.

Steps to reproduce

To reproduce this issue you can use the linked minimal demo project which provides a Post model having many Comments. The PostResource ist setup in a way that the Comments of a Posts are in a Repeater form field. Translating the Post itself works, but translating the comments does not.

Reproduction repository

https://github.com/pdaether/filament-translation

Relevant log output

No response

sanis commented 1 year ago

Totally the same thing: https://github.com/filamentphp/filament/issues/8068

haleyngonadi commented 11 months ago

Hi @pdaether, Did you ever figure this one out?

MahdiJalilvandir commented 11 months ago

I have the same problem. Has it been fixed?

stealeks commented 11 months ago

Looks like an issue occurs because of non-translatable data overwriting. It overwrites non-translatable data, including relations. As I understand, it must overwrite non-translatable data, including relations' non-translatable data only.

Switching locales: EditRecord/Concerns/Translatable::updatedActiveLocale

Saving: EditRecord/Concerns/Translatable::save

(similar for CreateRecord/Concerns/Translatable)

I am not sure how to fix it properly. Hope this info will be useful to someone

pdaether commented 10 months ago

Hi @pdaether, Did you ever figure this one out?

No, I'm still not sure how to fix this.

MahdiJalilvandir commented 10 months ago

I've discovered a way to resolve this issue. You just need to add the names of the relations to the $translatable property in the Post Model like this:

public $translatable = ['title', 'comments'];

This adjustment should solve the problem smoothly.

pdaether commented 10 months ago

@MahdiJalilvandir 👍 Perfect, I've tested this and it solved the problem for me as well.

MichaelMdp commented 10 months ago

@pdaether @MahdiJalilvandir What version of filament did the workaround fix this? I am testing on this very repro running on 3.0.40. Adding 'comments' to $translatable does not work for me.

I get :

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'comments' in 'field list'

update `posts` set `comments` = {"de":""}, `posts`.`updated_at` = 2023-12-13 09:20:37 where `id` = 2
corept commented 9 months ago

Having the same problem. Proposed solution did not work. This very critical for me.

@MichaelMdp Did you end up solving the issue?


Edit: Using a Relation Manager instead of a Repeater with a relationship, seems to be a temporary solution untill this is fixed.

MichaelMdp commented 9 months ago

@corept I am investigating a temporary solution with storing the activeLocale in session and forcing a reload of the page, but it is messy.

Beaudinn commented 9 months ago

@danharrin I think I have found the problem that is causing multiple issues and is probably also the cause of this issue. I tried to make a PR, but the code in the main repository fo packages /spatie-laravel-translatable-plugin doesn't match https://github.com/filamentphp/spatie-laravel-translatable-plugin

Inside Filament\Resources\Pages\EditRecord\Concerns\Translatable

The data is changes based on locale change but the data is not mutated

  $this->data = [
    ...Arr::except($this->data, $translatableAttributes),
    ...$this->otherLocaleData[$this->activeLocale] ?? [],
  ];

These code adjustment resolve this

  $this->fillFormWithDataAndCallHooks([
      ...Arr::except($this->data, $translatableAttributes),
      ...$this->otherLocaleData[$this->activeLocale] ?? [],
  ]);

related issues are #8656 and #7591

danharrin commented 9 months ago

Hey, please make a PR to this repo. You can find the code in /packages. Then we can review the effect of the changes there.

Beaudinn commented 9 months ago

Hey, please make a PR to this repo. You can find the code in /packages. Then we can review the effect of the changes there.

Why is the code in the main repo different than https://github.com/filamentphp/spatie-laravel-translatable-plugin

danharrin commented 9 months ago

Because this is a monorepo for maintainability purposes, and we need individual repos for each package, because Packagist needs that to be able to distribute via Composer.

chosten commented 9 months ago

I have also noticed that there is a difference between the data in otherLocaleData and in data. It cause various problems for me and corrupts the data if I save after a locale switch.

When the form is first loaded the data in otherLocaleData is dehydrated and data is hydrated. This works fine when you save. But when you switch to another locale the hydrated data is copied over to the otherLocaleData array and, if you save now, you will get the hydrated data in your database.

Also the data that was now copied to data is dehydrated. That's fixed by @Beaudinn I think.

These code adjustment resolve this

  $this->fillFormWithDataAndCallHooks([
      ...Arr::except($this->data, $translatableAttributes),
      ...$this->otherLocaleData[$this->activeLocale] ?? [],
  ]);

It solve part of the problem but the data transferred to otherLocaleData needs to be dehydrated.

chosten commented 9 months ago

This works fine for me but I'm not sure it's the proper way to do this.

Before:

$this->otherLocaleData[$this->oldActiveLocale] = Arr::only($this->data, $translatableAttributes);

$this->data = [
    ...Arr::except($this->data, $translatableAttributes),
    ...$this->otherLocaleData[$this->activeLocale] ?? [],
];

After:

$this->otherLocaleData[$this->oldActiveLocale] = Arr::only($this->form->getState(), $translatableAttributes);

$this->fillFormWithDataAndCallHooks([
    ...Arr::except($this->data, $translatableAttributes),
    ...$this->otherLocaleData[$this->activeLocale] ?? [],
]);
ondroftw commented 8 months ago

I would love the fix for this, I've tried all of the suggestions above, but none of them worked for me. Some even yielded weird results, like for example the suggestion from @chosten was saving repeater items ON LOCALE CHANGE, and with one same value for all locales.

tahacankurt commented 6 months ago

Is there any fix except using inline translation labels.

dmitry-udod commented 5 months ago

Is there any fix except using inline translation labels.

@tahacankurt Hi! As a quick fix you can override updatedActiveLocale method of the EditRecord\Concerns\Translatable trait. For example, for this demo repository, EditPost.php should look like this:



<?php

namespace App\Filament\Resources\PostResource\Pages;

use App\Filament\Resources\PostResource;
use Filament\Actions;
use Filament\Resources\Pages\EditRecord;
use Illuminate\Support\Arr;

class EditPost extends EditRecord
{
    use EditRecord\Concerns\Translatable;

    protected static string $resource = PostResource::class;

    protected function getHeaderActions(): array
    {
        return [
            Actions\DeleteAction::make(),
            Actions\LocaleSwitcher::make(),
        ];
    }

    public function updatedActiveLocale(): void
    {
        if (blank($this->oldActiveLocale)) {
            return;
        }

        $this->resetValidation();

        $translatableAttributes = static::getResource()::getTranslatableAttributes();

        $this->otherLocaleData[$this->oldActiveLocale] = Arr::only($this->data, $translatableAttributes);

        // Fix part - Start
        $this->form->fill([
            ...Arr::except($this->data, $translatableAttributes),
            ...$this->otherLocaleData[$this->activeLocale] ?? [],
        ]);
        // Fix part - End

        unset($this->otherLocaleData[$this->activeLocale]);
    }
}`
danharrin commented 5 months ago

12372 has been merged, please let me know if it doesn't fix in the next release and I'll reopen this

chosten commented 5 months ago

@danharrin There is still a problem with the Builder field. I explained the problem here but here is an example.

When I load the edit record page, it opens with the 'fr' locale selected. Now, if I switch the locale to 'en' and save, the data saved in the database for the Builder field is changed from this:

{
   "fr":[
      {
         "type":"block1",
         "data":{
            "text":"bonjour"
         }
      },
      {
         "type":"block2",
         "data":{
            "text":"test"
         }
      }
   ],
   "en":[
      {
         "type":"block1",
         "data":{
            "text":"hello"
         }
      },
      {
         "type":"block2",
         "data":{
            "text":"world"
         }
      }
   ]
}

To this:

{
   "fr":{
      "153ca2fd-4ca5-4741-ab1f-5c81ea9f2dfe":{
         "type":"headline",
         "data":{
            "text":"bonjour"
         }
      },
      "7ba735d1-1a21-4d16-9d23-61eed3779a23":{
         "type":"block2",
         "data":{
            "text":"test"
         }
      }
   },
   "en":[
      {
         "type":"block1",
         "data":{
            "text":"hello"
         }
      },
      {
         "type":"block2",
         "data":{
            "text":"test"
         }
      }
   ]
}

It was not the case before v3.1.19. It broke some of my custom actions and I'd like the content of otherLocaleData to be in the same format as data.

danharrin commented 5 months ago

Reverted https://github.com/filamentphp/filament/pull/12372

danharrin commented 5 months ago

I am posting this same message across all Spatie Translatable issues & PRs

Hey all! Wanted to update you on where we're at with the translatable plugin.

I really appreciate your patience while this issue has been active. While I created the plugin for the community, I've never actually had a project where I needed to use it, and that's the same for the rest of the Filament team. As such, it's the reason why the plugin hasn't had as much attention as the others, and is much more unstable: I just don't have the environment to test all the use cases, nor the motivation to make it truly great.

As such, I put out a post a month ago and asked who in the community uses the plugin and has knowledge of plugin development. Luckily, Lara Zeus and Mohamed Sabil stepped forward, who are both authors of popular Filament plugins and are trusted by the community.

We are strongly considering handing over maintenance of the plugin to those developers, for the good of the community. You can find their fork at https://github.com/lara-zeus/translatable.

Since the new developers have lots of experience with the plugin and active projects that use it, they should be able to help debug issues easier and make a much more stable experience for other users.

If their work goes well until v4 is released, we will likely retire the plugin at that point and recommend the fork as an official replacement. If it does not go as expected and the community is unhappy, then we will reconsider this decision. I am closing this for now, and if we decide to take maintenance back officially then we will probably reopen it.

The existing plugin will continue to receive security updates indefinitely. Please let me know if you have any further questions.

Many thanks, Dan

atmonshi commented 5 months ago

this an attempt to fix the issue if anyone willing to test this PR I would appreciate the feedback.