orderByTranslation scope seems to ignore fallback locale

[RichardDern]

Describe the bug
orderByTranslation scope seems to ignore fallback locale.

To Reproduce

• Save model translation to 'fr'
• List models in 'fr' using orderByTranslation: list is OK
• Use app()->setLocale('en')
• List models in 'en' using orderByTranslation: list is empty (assuming no model was ever translated to en)

I'm supposed to write mostly in French, but some articles in English. As such, my main locale as well as my fallback_locale both are 'fr' in Laravel's config. In config/translatable.php, I set locale to null, use_fallback to true, use_property_fallback to true, and fallback_locale to null.

Expected behavior
In the absence of translation in English, I would expect orderByTranslation to return the expected list with the French translations available, instead of nothing at all (still assuming I have only French Translations in my database at the moment).

Versions (please complete the following information)

• PHP: 7.3.9
• Database: MariaDB 10.3.17
• Laravel: 6.1.0
• Package: 11.5

[Gummibeer]

https://github.com/Astrotomic/laravel-translatable/releases/tag/v11.5.2

[RichardDern]

I'm sorry to re-open this issue, but it now returns fallback translation indeed, but not sorted, despite method's name.

[Lloople]

I would love to take a look into this and proper fix it this time.

Could you please provide an example of what you have on your database, your expected output and the final result? It would be easier to put into context 👍

[RichardDern]

Sure !

This is my database:

> select * from folder_translations;
+----+-----------+--------+-------------+------------+-------------+---------------------+---------------------+
| id | folder_id | locale | name        | slug       | description | created_at          | updated_at          |
+----+-----------+--------+-------------+------------+-------------+---------------------+---------------------+
|  1 |         1 | fr     | Images      | images     | NULL        | 2019-10-06 23:47:17 | 2019-10-06 23:47:17 |
| 12 |        12 | fr     | Projets     | projets    | NULL        | 2019-10-06 23:47:19 | 2019-10-06 23:47:19 |
| 18 |        18 | fr     | Liens       | liens      | NULL        | 2019-10-06 23:47:20 | 2019-10-06 23:47:20 |
| 23 |        23 | fr     | Réflexions  | reflexions | NULL        | 2019-10-06 23:47:20 | 2019-10-06 23:47:20 |
| 25 |        25 | fr     | Mémos       | memos      | NULL        | 2019-10-06 23:47:20 | 2019-10-06 23:47:20 |
| 28 |        28 | fr     | Relecture   | relecture  | NULL        | 2019-10-06 23:47:21 | 2019-10-06 23:55:22 |
| 35 |        35 | fr     | Blog        | blog       | NULL        | 2019-10-06 23:47:21 | 2019-10-06 23:47:21 |
+----+-----------+--------+-------------+------------+-------------+---------------------+---------------------+

Then:

php artisan tinker
>>> app()->setLocale('fr');
>>> Folder::orderByTranslation('name')->get()->pluck('name')
=> Illuminate\Support\Collection {#3173
     all: [
       "Blog",
       "Images",
       "Liens",
       "Mémos",
       "Projets",
       "Réflexions",
       "Relecture",
     ],
   }
>>> app()->setLocale('en');
>>> Folder::orderByTranslation('name')->get()->pluck('name')
=> Illuminate\Support\Collection {#3087
     all: [
       "Mémos",
       "Relecture",
       "Blog",
       "Images",
       "Projets",
       "Liens",
       "Réflexions",
     ],
   }

I expected both lists to be sorted the same.

Hope this helps !

[Lloople]

Thank you for such a detailed report! I'll check it out tomorrow and see if I can fix it 👍

[Gummibeer]

The problem is that it joins and orders the current app translation en. If this translation is missing it uses null as order value.
After DB the PHP trait takes over and properly resolves the fallback translation. But doesn't change the order.

The scope should use a similar approach like scopeListsTranslations.

BUT I want to note that this will possibly get dropped during #41 because I see no way to order in DB by PHP written fallback detection logic. For me the fallback customization and refactoring has higher priority than scopes.

I'm open for a discussion and also for ideas how to still fix it? An idea would be to switch to Laravel 6 only and use lazy collections to increase PHP sort performance? 🤔

[Lloople]

I think this can be solved using a CASE when ordering like the one I deleted on previous commits, but inside the join in this case. So if you order in the join you would get the en translation or the fr if en is missing.

Saying all of this from memory on the phone. I could be totally wrong on this 😅

[Gummibeer]

I would say no because you have to order by different rows.

folder: 12
fr: "Projets"
en: null

To order folder#12 with the right value into the right place you have to use the fr value/row (because it's fallback).

After the strategy classes the only way I see would be to order in PHP to use the strategies and 100% order by the same values that are displayed. For smaller collections this is no problem. But the moment you have a lot locales and/or entities it will reduce performance a lot. 😔

[Lloople]

We faced this same scenario at work and I think we solved it with that order inside the join. I’ll take a look at it tomorrow and see if it works.

I wouldn’t consider it an option to do it in php though.

[Gummibeer]

Not yet but I don't want to reject #41 because of an order scope.
It could be that some more scopes will get dropped if we make the fallback logic dynamic and move it more into code.

So I'm fine with every fix, but it could be that v12 will drop this scope.

[RichardDern]

My use case is two locales and few records to sort, so I'm ok with Laravel's collection sorting for now.

Maybe I just can make a suggestion: always load translation for requested locale AND fallback locale unless requested locale is the same. It wouldn't be too hard on the DB.

Unless it's already planned for next release ?

[Gummibeer]

The next major version won't have any some fallback locale. Atm we already have - I believe 2 or 3 fallback algorithms implemented.

• Fixed fallback locale for model
• Fixed fallback locale for attribute
• Loop all fallbacks until filled attribute

So everything done in DB could match but doesn't have to the PHP result. It's also a pain to maintain. 😔 That's why I want to split this into separate classes which could get combined or not, the user can write custom fallback algorithms, like looping the request accept-language header or whatever the user wants to do.
So yes this change will remove some of the scopes, because there wouldn't be a reliable way to get fallback locale for query. But it makes things easier for us and adds also a great feature (I think) for the user.

[RichardDern]

I understand how hard it can be. Translations have always been a PITA.

I can't wait to see what the future release will have to offer on that matter, and will try to contribute to the extend of my possibilities.

[Gummibeer]

I will reopen it if David wants to fix it? #41 will need some time, I would say not even or very end of this year. It still needs some more planning and also some code work.

[Lloople]

I tried to find a solution for this for a time now but I can't accomplish a thing.

This is the result I came up with for the join statement:

->leftJoin($translationTable, function (JoinClause $join) use ($translationTable, $localeKey, $table, $keyName) {
                $join
                    ->on("{$translationTable}.{$this->getTranslationRelationKey()}", '=', "{$table}.{$keyName}")
                    ->where(function ($query) use ($translationTable, $localeKey) {
                        $query->where("{$translationTable}.{$localeKey}", $this->locale())
                            ->orWhere("{$translationTable}.{$localeKey}", $this->getFallbackLocale());
                    });
            })

The issue is that the name attribute is not gathered from the join, so when I try the ->pluck('name') there's nothing to retrieve.

I'm sorry, but I'll put my efforts on the Test refactoring features for now. I recommend to write the query by yourself, @RichardDern, since this looks like a very specific scenario. Sorry for the inconvenience.

[Gummibeer]

Okay, i would keep it open. If someone else finds a solution.
And let the author or stale not close it.

[stale]

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.

[stale]

This issue has been automatically closed because it has not had recent activity. Thank you for your contributions.