search

spatie / laravel-backup

A package to backup your Laravel app
https://spatie.be/docs/laravel-backup
MIT License
5.61k stars 758 forks source link

Localization issue in notifications #1803

Closed snipe closed 2 months ago

snipe commented 2 months ago

First off, hello! And thank you as always for such an excellent package. We're big fans over here at Snipe-IT. <3

We just upgraded Laravel Backup, and some of our users are noticing that the email notifications they're getting look weird. I think I've tracked this down to the way the vendor translations are handled. We made the change a few versions ago to use the more specific RFC-compliant way of referring to language locales (en-US vs en), I think partly because of some weirdness in Symfony's pluralization around trans_choice(). It gets a bit weird, since en is RFC-correct, but so is en-US, and that makes we run into issues like this sometimes. It's all a bit frustrating, since some JS packages use the two-letter locale, some Laravel packages use the longer way, etc. Inconsistent standards, yay!

Is it possible to override the locale in the notifications, perhaps via config? We already have to do some jiggery-pokery for this internally, and we already have that mapping set up:

https://github.com/snipe/snipe-it/blob/61ced0b2219b453371086e11bbd9003423dc61a6/app/Helpers/Helper.php#L22-L78

I'm just not sure if that's already available for us to override in the config, or how we should best handle that. I can just rename the vendor language files generated from laravel-backup, but we all know that will likely bite us in the butt down the line. If I change the Laravel fallback_locale in the app.config to use the two letter code, this problem goes away, but I'm not sure if that's the right move either, since for the rest of the application, we don't have resources/lang/en, only resources/lang/en-US.

We're sort of damned-if-we-do, damned-if-we-don't.

Forgive me if this is a very stupid approach or question. It's a big release week and I'm pretty toasty.

snipe commented 2 months ago

Actually, never mind - I think I'm just an idiot. Sorry to have wasted your time.

freekmurze commented 2 months ago

Glad you found a solution 🙂 Hope your big release week went well.

snipe commented 2 months ago

The issue wasn’t even what I originally thought it was. 😩 We use CrowdIn to manage translations, so even though we published the translations from laravel-backup when we first upgraded, because the vendors directory isn’t within the en-US directory we use as the “source of truth” for CrowdIn, when I downloaded the updated Snipe-IT translations, your translations got removed by accident.

So, yeah, that’s how my big release went lol.

Sorry for bothering you - it was my mistake the whole time.