Check and remove null translated fields from the input request and delete from model

[weidmaster]

Is your feature request related to a problem? Please describe. I identified a need that the translatable fields in a form should be nullable, but not save an empty string in the translations related table of the model, because if the locale is available it would return an empty string instead of the configured default from the model.

When trying to save a request with request->all() the NULL values are not removed at all and the database fails to save.

Describe the solution you'd like The elegant solution would be for the trait to check and remove NULL values from the input request array and also if there were NULL values in a locale translation, to also remove from the model itself.

Describe alternatives you've considered For my application I developed a solution, that may point towards some good discussions and possible correct implementation of this in the package. I just don't have the skills to make a pull request of said feature myself, but I hope the following code may inspire you all:

1. The Model using Translatable must set the default language for this to work

   
   namespace App\Models;

use Illuminate\Database\Eloquent\Model; use Astrotomic\Translatable\Contracts\Translatable as TranslatableContract; use Astrotomic\Translatable\Translatable;

class LocalizedModel extends Model implements TranslatableContract {

use Translatable;

public function __construct(array $attributes = [])
{
    parent::__construct($attributes);

    $lang = Language::find(1);
    $this->setDefaultLocale($lang->locale);
}

}

1. Assuming Category extends LocalizedModel, the following is how I call my method from the Controller:
```php
$data = Category::findOrFail($id);
$input = $this->removeEmptyTranslations($request->all(), $data);

1. Assuming I have the locales available in my application in $this->locales I will always have the default locale setup in the model, and therefore the method removeEmptyTranslations does not consider the default locale for removing. The implemented logic is the following:

   /**
    * Remove Empty Translation fields from an input array and also delete
    * from a LocalizedModel instance
    *
    * The array is in the format:
    * "locale" =>  [
    *      "field" => "value"
    *  ]
    * @param array $input
    * @param LocalizedModel $model
    * @return array
    */
   public function removeEmptyTranslations(array $input, LocalizedModel $model = null)
   {
       $removed = [];
   
       foreach($this->locales as $locale) {
           if($locale->locale === $this->lang->locale) {
               continue;
           }
   
           if(isset($input[$locale->locale])) {
               $input[$locale->locale] = array_filter($input[$locale->locale]);
               if(empty($input[$locale->locale])) {
                   $removed[] = $locale->locale;
                   unset($input[$locale->locale]);
               }
           }
       }
   
       if($model) {
           $model->deleteTranslations($removed);
       }
   
       return $input;
   }

Additional context How the form input looks like. It is a tabbed interface, that changes the input according to the locales provided

[Gummibeer]

Hey,

thanks for your detailed issue. At first I will try to help you with your current solution because it could be that something like this won't get into the package and I've seen some things that could get improved.

I'm not saying that this is wrong or you shouldn't use this - but you shouldn't use this. 😆 The default locale, bad naming right now, is more like the enforced locale. As soon as you set it every translatable logic in this model instance will use this locale. Independent of your current app locale or any other locale setting. Keep in mind that it's only this exact instance, because the property isn't static.

$lang = Language::find(1);
$this->setDefaultLocale($lang->locale);

https://github.com/Astrotomic/laravel-translatable/blob/7ada6f2de63dc1d56d979ad3e08a4f12a06e4260/src/Translatable/Translatable.php#L353-L360

Instead I would recommend you to use config('app.fallback_locale') or add a new config key like config('app.default_locale'). If you want to use your database, don't use id:1 if you don't have a migration that adds your languages including the ID. Instead I would recommend to do something like Lang::whereLocale('en')->first() - or even better Lang::whereDefault(true)->first(). The first one is questionable where's the benefit to do Lang::whereLocale('en')->first()->locale because for sure it would always be en. 😉 The second one would be the only case for which I would use the database in this context.

Regarding your foreach($locales ...) - this package comes with a locales helper which we use internally.

• https://docs.astrotomic.info/laravel-translatable/package/locales-helper
• https://github.com/Astrotomic/laravel-translatable/blob/master/src/Translatable/Locales.php

This one will contain all locales the package is configured with and also knows about the current one. For sure you could also extend and rebind it to the container with a new default() method that will always return your primary/default/fallback locale of your app.

Regarding the empty() check - you could use the the isEmptyTranslatableAttribute() method, if your logic relies in the same model. Keep in mind that 0 or even "0" are empty. Just as a notice, this logic will change with the up-coming major release introducing fallback strategies #129 https://github.com/Astrotomic/laravel-translatable/blob/b4201e5500572955c2dd8a636da75ad555d37070/src/Translatable/Translatable.php#L340-L343

---

Now why I won't add this to the package: This package doesn't enforce anything in the database regarding the translated columns. So you can make them nullable or change their type to int, json or whatever you want. It's also possible to have multiple translated attributes per model - so a single empty shouldn't delete the whole translation. It's also possible to use this package with partial updates - so even if every attribute in a translation array is empty it doesn't mean that there isn't another attribute stored in the database.

Covering all these cases in a solution inside this package seems nearly impossible. And because this freedom in your database design is the keypoint of this package I won't add a method that would restrict this.

Possibly your logic could get replaced by the current or at least the coming fallback possibilities. So if you make the translation column nullable but define your fallback locale this package would still return the fallback translation instead of null.

[weidmaster]

Thank you for your insights and suggestions. Yes I know that calling the Language with ID 1 is not good at all, but that is how the app is setup for now and I tried to use the fallback helpers and methods, but the app used dynamic added locales. So the default is not always "en" as I would like to 😆

Yes I can set the colums as nullable, but then for this particular project there won't be translations for everything all the time, so I would end up with a ton of nulls, and I wanted to avoid it. That's why it is a cumbersome code for now.

I get your point to not restrict the package usage. So maybe there should be a way for better setup with database, using locales loaded from database and setting the models properly.

You said that setting the default in the Model instance is not static and that is all fine. It is working correctly when changing the app locale. So it loads the default from DB but if the app locale is different it does get the correct translation, even calling the attribute directly. 👍

Here is how I get the locales to be dynamically changed. Just some food for thought, as always:

inside boot() method in AppServiceProvider

$storeSettings = DB::table('generalsettings')->whereRaw("'{$currentUrl}' LIKE CONCAT(domain,'%')")->first();

        if (!$storeSettings || empty($storeSettings->domain)) {
            $storeSettings = DB::table('generalsettings')->where('is_default', 1)->first();
        }

        $storeLocale = DB::table('languages')->find($storeSettings->lang_id);
        $locales = DB::table('languages')->get();

        $transLocales = App::make('translatable.locales');

        foreach ($locales as $locale) {
            $transLocales->add($locale->locale);
        }

[Gummibeer]

If it works - everything fine! 👍

Instead of doing it in your app service provider I would override the Locales::load() method. This will persist your changes if anything calls the load() method on runtime. https://github.com/Astrotomic/laravel-translatable/blob/7ada6f2de63dc1d56d979ad3e08a4f12a06e4260/src/Translatable/Locales.php#L86-L107

And regarding your need to delete empty translations - have you thought about using an observer for the translation model? Just a quick scripting - not tested or fully runnable:

Translation::saving(function(Translation $translation) {
    if(empty($translation->translatable_column)) {
        if($translation->exists) {
            $translation->delete();
        }
        return false;
    }

    return true;
});

Could be a bit cleaner to catch this in the last instance and prevent empty translations at all and from everywhere instead of cleaning and manipulating the request data.

[weidmaster]

Nice tips, I will sure take a look. I am not familiar with observers yet, but I always try to get my code as clean as possible.

Regarding your comment:

The default locale, bad naming right now, is more like the enforced locale. As soon as you set it every translatable logic in this model instance will use this locale. Independent of your current app locale or any other locale setting. Keep in mind that it's only this exact instance, because the property isn't static.

You are correct. I somehow had a different behaviour but it was probably some cache. I just tested it and in fact it is not working anymore when changing the app locale at all. It is enforcing it as you say, that is not what I want to achieve.

What I am trying to do now is to fix it so I don't need to spam ->translateOrDefault() everywhere in my blade views 🤣

I will try your other tips and see if it works. Thank you very much.

[Gummibeer]

Thanks for feedback. ☺️

A new major version is on the road but not feature freezed. So if you think that something is missing, should be changed or anything - let me know.

I know that v12 is open since a while and won't be done in next few days/weeks. But it's one of these chances to rework "everything". 😅

[weidmaster]

A new major version is on the road but not feature freezed. So if you think that something is missing, should be changed or anything - let me know.

Regarding the nullable fields, I stumbled upon an issue where I do have null values in the translation table (because the attribute should not be translated or is not know at the time), but even with all the config using fallback_locale and property_fallback, it is not working for me at all.

My table is as follows:

As the title is a required attribute in the original table it gets the value added during input request, but details is not required.

The fallback locale is pt-brbecause it is the first available locale. I just noticed that when retrieving the attribute, as usual like $subscription->detailsor even with $subscription->translate('fr', true)->detailsor anything, it is not loading the pt-br details field at all, thus I end up with a blank string in my view file.

As you said that a major version is on the road, I think all the problems with missing translation fields could be solved if the default locale field would be available in the original table. So, when loading the translations table, if the field is missing you would request the original table field. Because using the package more and more I noticed that it considers everything a translation, even though the content should be defined in a specifc locale first, and translations be available later.

It is a suggestion @Gummibeer but for now I would like to know how can I have those nullable fields get the value from the default locale. It does work if I don't have the translation row at all in a locale, but as I said, sometimes the translations won't be available for all fields in a table, because they are not required for instance.

Thanks for your patience and help as always.

[Gummibeer]

Hey,

the package supports property fallback - so falling back per property if the single property is empty but the translation row exists. So exactly your use-case. Have you checked the documentation and the test case covering this?

https://docs.astrotomic.info/laravel-translatable/package/fallback-locale#for-properties https://github.com/Astrotomic/laravel-translatable/blob/7ada6f2de63dc1d56d979ad3e08a4f12a06e4260/tests/TranslatableTest.php#L718-L734

If you have done it like described and done in the test an example would help. So your real code and primary configuration. Without it's hard to help you. If it's too much code - a gist or even a working repo would also be okay! :)

---

Regarding your idea to put the primary language in the main table: This was discussed quite often since nearly the beginning of this package (even as it was maintained by Dimitris). The reason against is again the top level bird view. I understand why some want it like this! But this package is based on key translation - we use the column/attribute - and not text translation. For key translations it's common that there's no primary/default/always existing translation - in opposite to text translation in which you translate a full text like Hello World so you always have at least this text you can fallback to. There are also apps with super mixed locales and default locales per user - like a multi user blog/recipe platform. So for me German would be my primary/fallback locale I write in and could translate into english and John Doe will do it vice versa. Doing this in the main table would be total confusion and a mess to maintain, migrate and handle.

id	author	title
1	Tom	Kartoffelsuppe
2	John	Peanut-Butter-Cookies
3	Juan	Churros con Chocolate

It would be nearly impossible to work with such a table setup on the long run. That's the reason why this won't change in the official package - there are some closed issues with some snippets they've used. With all translations in one table separated from the main table allows to easily migrate, auto translate, clear, list or whatever you want to do with them. How would you create a "missing translations" view with the translations splited across two tables and without really knowing which language is used in the main table? 🤔

The new strategies will allow much more granular and customized fallback logic. By default the come with a translation (translated row) and a translated attribute resolver. https://github.com/Astrotomic/laravel-translatable/pull/129/files#diff-0f1e5641b8f3309c976830cf355df4e4

interface TranslationResolver
{
    public function resolve(
        TranslatableContract $translatable,
        string $locale,
        bool $withFallback,
        Collection $alreadyCheckedLocales
    ): ?Model;

    public function resolveWithAttribute(
        TranslatableContract $translatable,
        string $locale,
        bool $withFallback,
        Collection $alreadyCheckedLocales,
        string $attribute
    ): ?Model;
}

I hope that this helps you and answered your questions!? And it's a pleasure to work/discuss/write/elaborate with you as you write pretty long/explaining issues/comments and don't expect me to guess your app/code like others.^^ So keep up with this good attitude! 👍 🚀

[weidmaster]

I checked the documentation but now that I see the Test case better, I think I know what the problem is. I am using the documented fallback_localeas null so it gets the first defined locale (pt-br in my example). Seeing the Test code it is defining the fallback_localein this line:

$this->app->make('config')->set('translatable.fallback_locale', 'en');

So I will try to do like that in the Provider and hope to see it working, because my translatable config is as follows:

[
'locales' => [
        'en999'
    ],
'use_fallback' => true,
'use_property_fallback' => true,
'fallback_locale' => null,
]

And in my AppServiceProvider, as I mentioned, I have this setup to get the locales from database:

$locales = DB::table('languages')->get();

$transLocales = App::make('translatable.locales');

foreach ($locales as $locale) {
    $transLocales->add($locale->locale);
}

$transLocales->forget('en999');

That does make my default being the first language found, not relying on ID 1, as you suggested. So I will try to set the fallback_localekey there as well, to see if it works.

---

I am happy to share my different approaches and solutions and I am really interested in localization of apps in general, so I am glad to get valuable tips and code examples to use and I understand that, being a developer myself, every little detail is important to understand an issue, and I am glad to help anytime I can 😄

I will try to make a video of how the application is working, so you can see the use case better. I can't provide the codebase because it is private for a company. But I can also try to setup a minimal use case with a similar app after the video, if it would still be confusing 😅

[weidmaster]

And setting fallback_localeindeed does the trick!

I have debugged the package with xdebug again to find how it checks for this fallback and because mine is set initially as NULL, according to the documentation, it should fallback to the first available locale, pt-br. It only works this way if the other locale key is not found at all, not when the column value is NULL, as in, not when there is an incomplete set of translation values.

Setting the config at run time like this works as intended:

Config::set('translatable.fallback_locale', $this->lang->locale);

So, according to the test you provided, it is doing what it is supposed to do. So the suggestion is to make this case of NULL values being in the test as well, and not only the locale key 😉

---

This is the method for reference, annotated:

private function getAttributeOrFallback(?string $locale, string $attribute)
    {
        //$locale is 'fr'
        $translation = $this->getTranslation($locale);

        if (
            (
                ! $translation instanceof Model
                || $this->isEmptyTranslatableAttribute($attribute, $translation->$attribute)
            )
            && $this->usePropertyFallback()
        ) {
            $translation = $this->getTranslation($this->getFallbackLocale(), false);
        }

        if ($translation instanceof Model) {
        //this returns because the 'fr' key is found in the translations table, but $attribute is NULL
            return $translation->$attribute;
        }

        return null;
    }

---

And the table for reference as well:

[Gummibeer]

Hey,

there is a testcase for the null behavior: https://github.com/Astrotomic/laravel-translatable/blob/7ada6f2de63dc1d56d979ad3e08a4f12a06e4260/tests/TranslatableTest.php#L864-L938

The null feature was added in #35 and #36 and it was also the drop that overflows the barrel - so this change and getting into all this fallback logic again brought up the idea to introduce the #129 translation resolving strategies. It could be that the difference between translation and attribute fallback was intentional (too much effort to make it work in current setup on attribute level). This will definitely change and work like you expect it in v12! I've worked again on the new features today and the main thing missing are the unittests.

So I fear that this won't be fixed/updated in v11.

One last thing I want to ask you - I've already tried it today in another issue with a similar long/well discussion.

1. the package is MIT and will be but it's also Treeware, so if you use the package (in your company) in any production apps I would be happy if you would plant some trees - in best case via the official link. 🌳
2. it's a lot of work to enhance this package and also a lot of time was spent into the current state of development. No one is in no way required to pay something for the package. But I believe that, primary companies, should be able to pay some money for the open-source packages they use. So this request isn't only for me but more in general. So if you could bring up this topic in your company I would be pretty happy! 🙂 It's not only the time we spent into development but some packages also have real costs, for example the astrotomic.info domain, coffee and food during development and so on. One major argument to support OSS: your business depends on it, what have you saved during initial development and what will it cost you to fork and maintain it yourself. 😉

Don't feel you forced to anything. I just thought that you are possibly the/a right one to ask for. If you don't want to or whatever - no need to explain or feel bad. 😉

[weidmaster]

This will definitely change and work like you expect it in v12! I've worked again on the new features today and the main thing missing are the unittests.

Awesome! Yeah I can see it works different in the current version and it ok for such major improvements to be done in a new version. I am looking forward to it.

---

Yes, I agree that companies should pay for technology they use for private and commercial stuff when they don't contribute back to the Open Source Community. I will definetely bring the topic to the team here. Specially when the package author takes the time to reply issues and have some developing discussions 👍

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.