Fallback locale not taken from locale list in translation exists, even if property doesn't.

[redalpha01]

Describe the bug
When fallback_locale is set to null, if a translation model is found for the main locale, it will not try the locales is the locales array. This comes into contradiction with https://docs.astrotomic.info/laravel-translatable/package/fallback-locale#app-wide . (It will however go through the array if a translation model for the main locale is not found, which renders the scenario even stranger)

To Reproduce
Set a model's $useTranslationFallback to true.
Set config fallback_locale to null.
Have at least two locales in locales array.
Save instance of model with its translations having null for the property in one locale and some data (a string in my case) in the other. While app is in locale that has null property, try to call the property that has null.
Notice that it wasn't translated from the other locale.

Expected behavior
A clear and concise description of what you expected to happen.

Versions (please complete the following information)

• PHP: 7.3
• Database: mySQL
• Laravel: 6.18.3
• Package: 11.8.0

Additional context
The getTranslation() method to get translation model properly calls the array if no other model has been found before

if ($withFallback && $configFallbackLocale === null) {
    $configuredLocales = $this->getLocalesHelper()->all();

    foreach ($configuredLocales as $configuredLocale) {
        if (
            $locale !== $configuredLocale
            && $fallbackLocale !== $configuredLocale
            && $translation = $this->getTranslationByLocaleKey($configuredLocale)
        ) {
            return $translation;
        }
    }
}

but the getAttributeOrFallback() only calls getFallBackLocale()

$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) {
	return $translation->$attribute;
}

return null;

which does not check the array (getFallbackLocale())

if ($locale && $this->getLocalesHelper()->isLocaleCountryBased($locale)) {
	if ($fallback = $this->getLocalesHelper()->getLanguageFromCountryBasedLocale($locale)) {
		return $fallback;
	}
}

return config('translatable.fallback_locale');

[redalpha01]

I patched it by copying getAttributeOrFallback in my model with this structure

	private function getAttributeOrFallback(?string $locale, string $attribute)
	{
		$translation = $this->getTranslation($locale);
		$fallbackLocale = $this->getFallbackLocale($locale);

		if (
			(
				! $translation instanceof \Illuminate\Database\Eloquent\Model
				||
				$this->isEmptyTranslatableAttribute($attribute, $translation->$attribute)
			)
			&& $this->usePropertyFallback()
		) {
			$translation = $this->getTranslation($fallbackLocale, false);
		}

		if ($translation instanceof Model && $value = $translation->$attribute) {
			return $value;
		}

		foreach ($this->getLocalesHelper()->all() as $configuredLocale) {
			if (
				$locale !== $configuredLocale
				&& $fallbackLocale !== $configuredLocale
				&& ($translation = $this->getTranslationByLocaleKey($configuredLocale))
				&& ($value = $translation->$attribute)
			) {
				return $value;
			}
		}

		return null;
	}

[Gummibeer]

Hey,
thanks for the report and even if I don't 100% get the described usecase I know about the fundamental issue described.
The solution will be #41 - feel free to comment there if you think that something is missed.

Right now the whole fallback topic is a total mess. It's a pain for me as the maintainer to, well, maintain but also for the users to understand.

It's also pretty hard to configure everything that it works like expected, it's nearly impossible to adjust and most times the performance isn't the best because the code covers a lot more functionality.

That's why I've come up with the "strategy" idea (naming isn't final, open for ideas). The basic idea is something like a pipeline of translation finders/strategies/pipes and the first that doesn't return null will be used.
That will increase readability, testability, performance and extendability and for sure some more bilities. 😉😅

There's also #129 already with the first implementation idea.
The best is that these classes will cover model and attribute resolving the same.
This way it will also be super easy to add a strategy that uses the preferred language http header or any user DB settings or whatever.

Because of all this and the fact that I'm not 100% sure if the described case and problem is a real bug or more like a feature request. 🤔
I would like to use the next major release with the new fallback strategies to solve this issue.

[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.