Astrotomic / laravel-translatable

A Laravel package for multilingual models
https://docs.astrotomic.info/laravel-translatable/
MIT License
1.23k stars 156 forks source link

Scope that retrieves for country based locale all translations (without duplicates) #164

Closed rezaffm closed 4 years ago

rezaffm commented 4 years ago

Hello (Hallo, hoff es geht Dir gut!),

I recently added country based locales to my project (de-AT, de-CH) to de. I have noticed that there is no scope that would retrieve all translations for a given country locale. Clearly, it's easy to retrieve all posts (for instance) in de-AT by using Post::translatedIn('de-AT')->get(), but there is no similiar method or scope that would retrieve all posts in de or in de-AT (I hope that makes sense). Also duplicates should be avoided if there is a post available for de and de-AT

I am not a mysql expect, so I asked at Stackoverflow how to design a query for such a scope or method: https://stackoverflow.com/questions/62620047/one-query-for-one-join-that-uses-first-result-in-second-subquery-to-get-all-ro

I noticed that it would be possible to first check per post if a translation at all exist for the given post (once we retrieved all posts) and then, if that's true, we can use the translate method with the current locale and use fall_back set true as a work around.

$posts = Post::withTranslation()->get();

in a blade view then:

@foreach ($posts as $post)
    @if($post->hasTranslation($post->locale))
    <div>
        <p>{{ $post->locale }}</p>
        <p>{{ $post->translate(app()->getLocale(), true)->title }}</p>
        <p>{{ $post->translate(app()->getLocale(), true)->content }}</p>
    </div>
    @endif()
@endforeach

Demo db according to documentation:

posts

# id, created_at, updated_at
'1', '2020-07-02 10:11:23', '2020-07-02 10:11:23'
'2', '2020-07-02 10:11:23', '2020-07-02 10:11:23'
'3', '2020-07-02 10:12:25', '2020-07-02 10:12:25'

post_translations

# id, post_id, locale, title, content
'1', '1', 'de', 'Girokonto de', 'Girokonto de'
'2', '1', 'de-AT', 'Girokonto de-AT', 'Girokonto de-AT'
'3', '2', 'en', 'Depot en', 'Depot en'
'4', '3', 'en-US', 'credit card en-US', 'credit card en-US'
'5', '2', 'de', 'Depot de', 'Depot content de'
'6', '1', 'en', 'Current account en', 'Current account en'

If I then browse the website for posts in de-AT, I'd get the desired result:

de-AT

Girokonto de-AT

Girokonto de-AT

de

Depot de

Depot content de

So my request would be to add a scope or maybe method that retrieves all the translations for a given country based locale. If that's already implemented, happy to learn how to use it tough.

Thank You.

Gummibeer commented 4 years ago

Moin 😉

a short conclusion - if I'm right you want to get all models with a translation in a country based locale de-AT or the corresponding country-less locale de!?

post_id locale title
1 de post #1 de title
1 de-AT post #1 de-AT title
2 de-AT post #2 de-AT title
2 de-DE post #2 de-DE title
3 de post #3 de title
4 en post #4 en title

So this table would result in:

app()->setLocale('de-AT');

$posts = Post::translatedInCountryOrLanguage('de-AT')->get();
foreach($posts as $post) {
    $post->title;
}
// post #1 de-AT title
// post #2 de-AT title
// post #3 de title

If that's your goal I'm not quite sure if I will add a scope to thee package. But II will definitely help/guide you to a solution!

The package translatedIn() scope uses the Laravel/Eloquent default whereHas().

https://github.com/Astrotomic/laravel-translatable/blob/7ada6f2de63dc1d56d979ad3e08a4f12a06e4260/src/Translatable/Traits/Scopes.php#L84-L91

You only have to add one orWhere() to the sub-query - okay, and some logic to prevent or strangeness. 😂

public function scopeTranslatedIn(Builder $builder, ?string $locale = null)
{
    $locale = $locale ?: $this->locale();

    // build relationship exists condition
    return $builder->whereHas(
        'translations',
        // add conditions the translations have to fulfill to "exist"
        function (Builder $query) use ($locale) {
            // wrap "or" query in one "where" to prevent strangeness
            return $query->where(function (Builder $q) use ($locale) {
                // build real translation conditions
                return $q
                    // translation locale has to match
                    ->where($this->getLocaleKey(), $locale)
                    // when locale is country based
                    ->when(
                        $this->getLocalesHelper()->isLocaleCountryBased($locale),
                        // add "or" locale matches language without country
                        function (Builder $q) use ($locale) {
                            return $q->orWhere(
                                $this->getLocaleKey(),
                                $this->getLocalesHelper()->getLanguageFromCountryBasedLocale($locale)
                            );
                        }
                    );
            });
        }
    );
}

I've commented every closure - feel free to reformat it, rename variables or whatever. Possibly you even could reduce it a bit - but I've done the long/excessive version for "better" readability. (it's still a long and too much indented method 😂)

This scope isn't tested but should do exactly wha you've described.

rezaffm commented 4 years ago

Moin :-)

I just tested it and it works as expected. I actually never had to use that when() function and realized that this is a quite nice solution.

The project I am currently working on is available in de and de-AT. And while I will translate many posts (and pages) into German and also German-Austrian there will be always some posts only available in de. But on the index site for Austrian I'd like to show all posts that are available in de-AT and if there is no de-AT translation, I then offer the de post.

I think that scope would be very worthwhile to add to the package and of course leave that up to your decision. The whole thing started to get a bit tricky for me once I started with regions. For "none-region" locales the scope would be certainly not that useful.

Gummibeer commented 4 years ago

AD: If you like this when() method - here's a package to add it to every class you like. 😉 https://github.com/Astrotomic/php-conditional-proxy

I will think about adding it during v12. What do you mean with regions? How do these locales differ from the country based? And what is your expected behavior? Do you mean something like de-HH (German in Hamburg)? And the expected behavior would be to check for de-HH fallback to de-DE fallback to de?

If so I can tell you that the query would be a bit more complex - because you will have to add another when() inside the first when() and use a region to country mapping. But it's possible and the v12 translation resolving strategies will save your live. 😉 Because they will do exactly this - the concept of the new v12 fallback logic is like a pipeline and use the first non null value.

I'm thinking about any way to use these classes also for queries to do the same logic on database. But there are a ton of limitations in databases/query builder. So I think that I won't add it, at least not in the first v12 release.

rezaffm commented 4 years ago

Oh sorry for the confusing, by regions I actually meant "countries", I think going for de-HH, de-MUC and so far would go maybe a bit too far and I'd doubt if that would have any use for SEO et cetera.

Indeed the whole fallback logic so far gave me a bit of a hard time. Having more controle about it also on a model based approach makes a lot of sense to me.

I was also quite suprised how much complexity the whole localization topic adds to the "development experience" and wonder why it's such a minor topic in laravel's documentation.

Gummibeer commented 4 years ago

A, okay - yeah, for SEO not really. But who knows what you are building. 😂 Yes, it's a much harder topic to maintain. Because there are so many if anything in the code. We decide between true, false, null and so on - so also in the configuration there are too few keys for what they do right now. All this will change - but primary on package side it's a massive change, I will try to keep it as much backward compatible for the default usecase as possible. But it will add a lot of cool new features and possibilities and also readability.^^

Localization at all is a massive topic, I think that's why Laravel itself handles the static translations via Symfony translator but don't want to get into dynamic/model translations. Because it's such a hard topic with a ton of special/edge cases. Spatie for example uses a simple JSON column approach, removes the second model but isn't this good for larger texts and doesn't enforce a given type of the translations and also not such a complex/advanced fallback logic. So their package is great for simple translations of Role/Permission names for example. But translating a Shop or Blog with several translated fields per model, different types and also some very long texts would be possible but a pain to handle in Table Plus or any similar tool.

After the differences in type we start with country based and language only locales or possibly a mix of them. And so on ...

So there are a lot of different use-cases with different expectations. Building something good for all of them isn't really possible.

rezaffm commented 4 years ago

To be frank, I didn't see that much value added by using the spatie package and from what I saw some people were quite surprised that the name was exactly the same as this package. I used this package from the beginning and really like it (good work btw).

I also like the two table structure as JSON brings a lot of complexcity if you want to run pure, very simple, sql queries. The JSOn structure adds a lof of value as an alternative for the anti pattern EAV tough. But that's not relevant for this package.

Where things for me also got complicated is when I started to use relationships. For instance, if you have posts and categories and you want to for for the following structure '/locale/category-slugy/(optional: sub-category-slug/post-slug - it become quicky evident that you will need in all cases a fallback locale or even better, you make sure that for a given language 'en' and 'de' you have the correct language in place otherwise if you might add a category translation you mess up your urls ... and they might get indexed wrongly by google and as I have found out recently once google indexed something it can take weeks until it re-index everything correctly.

So what I will do for this case, for instance, is that I ll only offer to use a category for a post, if we have the language in place. So for a post in 'de-AT' it is mandatory to have at least a 'de' translation for the given category. Actually for my use case I might even only offer none-country languages such as 'en' and 'de'.

Then of course it would be great to change the fallback logic on the specific model. As I wrote most of the stuff first in 'de' I could then easily setup the validation for that.

A bit off topic here, as the whole topic is very complex.... I think it would be great to have something as a "best practice" section in the documentation ... maybe I try to come up with something and you can then adjust as you wish. At the start, I found it for instance quite complex to setup the right structures for the typical CRUD tasks.

...and thanks again for maintaing this package. Very happy that we have it.

Gummibeer commented 4 years ago

We use the spatie package for some customer projects where we only have to translate short texts and we also use PostgreSQL with outstanding JSON support. But I'm happy to hear that you like this package. 😉 🙂

Google adjusts rescan intervals based on activity. If the page itself doesn't change quite often they don't rescan it a lot. If the page has a lots of traffic and content changes Google will rescan more often. (just a super simplified description of the logic) But you are right, as soon as translations get part of URLs it gets harder, but primary on the validation/generation side. Resolving shouldn't be more complicated?

Category::whereTranslation('slug', $request->categorySlug, app()->getLocale())->firstOrFail();

Our major problem was most times the router performance if it has to resolve a lot of models only to check if the path is okay. We've reworked all this to a "static" router based on artisan scheduler and redis. We had a catch all route and stored all dynamic paths in redis with the in app route post/1/8/15 (post/{category}/{subcategory}/{post}) as value. We also had a vice versa index to quickly generate the canonical/readable route by the in-app/system route. This way we don't had to care about response caching, which is often the solution for slow controllers, but we reduced the complexity of the queries a lot to simple ID ones.

Regarding the fallback logic per model: that's one topic on my roadmap. To configure the translation pipeline per model and have a default one in the global config.

To your last part, best practice: I always try to keep up a good documentation and by using gitbook for this package there's also much room for examples, additional pages and so on. But I also like to simply link good blog posts describing one real-world setup. So if you or your company have a blog and you would write about your special requirements and how you've solved them with code examples and so on I will link it happily in the documentation. https://docs.astrotomic.info/laravel-translatable/#tutorials

I won't be able to write/maintain several scenarios in the official documentations. Primary because I don't have hundreds of apps with different setups. 😂 That's why I think that it's better to link good tutorials and usecase descriptions written by others. This way there could also be several different languages.^^

I'm happy to keep the package up! 🎉 But I want to ask you two things:

  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, it's the first time I've requested this directly. 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. 😉

rezaffm commented 4 years ago

Hi,

Our major problem was most times the router performance if it has to resolve a lot of models only to check if the path is okay. We've reworked all this to a "static" router based on artisan scheduler and redis. We had a catch all route and stored all dynamic paths in redis with the in app route post/1/8/15 (post/{category}/{subcategory}/{post}) as value.

Can you elaborate a bit more on this? Sounds quite interesting.

Well this whole fallback thing gets actually quite complex once you work with regions, no? I also have tags in my project and I write a lot of content first in German, so what would be the use to show an English guy some German tags (a bit wordpress style actually...).

That's also why I came up with this initial idea to have a scope for the region and the default language. It would also help for these nasty tags. If you have posts that have many-to-many tags... I could then chain the whole thing by post->tags()-> translatedInCountryOrLanguage()->whereHas('translations', function($q) { retun $q->where('tag', $tagName)} (not sure if would definitely work that way...just writing it down without coding..), but you get the idea.

In terms of documentation I am a bit out of time to write something but it's on my list. I will let you know and maybe you can make it even better.

The project I am working on is a startup so we are very happy to have you (and open source).

I just planted 100 trees.