elixir-cldr / cldr_trans

Cldr-based fork of the most excellent "trans" library
Other
10 stars 3 forks source link

Translating a whole struct does not use the fallback if the locale is configured but translation is missing #10

Open tozz opened 3 months ago

tozz commented 3 months ago

I have a very basic setup (anonymized a bit)

defmodule X.Cldr do
  use Cldr,
    locales: [:en, :sv],
    providers: [Cldr.Number, Cldr.Calendar, Cldr.DateTime, Cldr.Trans],
    default_locale: :en
end

I then want to translate a struct but it fails if the locale has been configured but the translation is missing.

    test = %X.Y{
      title: "a title",
      translations: %X.Y.Translations{}
    }

    test_sv = Cldr.Trans.Translator.translate(test, :sv)
    test_sv.title == nil

If I use a locale that is not configured the fallback works as expected.

    test = %X.Y{
      title: "a title",
      translations: %X.Y.Translations{}
    }

    test_de = Cldr.Trans.Translator.translate(test, :de)
    test_de.title == "a title"

And finally, when using a fallback chain it also works as expected.

    test = %X.Y{
      title: "a title",
      translations: %X.Y.Translations{}
    }

    test_sv = Cldr.Trans.Translator.translate(test, [:sv, :en])
    test_sv.title == "a title"

Giving the field also works as expected.

    test = %X.Y{
      title: "a title",
      translations: %X.Y.Translations{}
    }

    Cldr.Trans.Translator.translate(test, :title, :sv)
    # "a title"

So there's clearly some kind of mismatch here, or I am not reading the docs correctly. I'm fine with the behavior now that I understand why it happens, but it took a while, is it a bug or expected behaviour?

kipcole9 commented 3 months ago

@tozz, thanks for the very complete report, greatly appreciated.

You are seeing the behaviour as implemented. But I'm not sure either if it's the right behaviour. Fair to say the user population of this lib isn't high so it's hard to get a feeling for expectations. At minimum the documentation clearly needs improvement.

Part of the challenge is that sometimes the translation is resolved in a database query and sometimes in Elixir - no matter what the two need to agree on the fallback strategy. Today, the database query expects a translation to be available.

The primary question then is: should the default be returned when a locale is configured but a translation is not available.

What makes the most sense to you?

tozz commented 3 months ago

Regarding the user population I think it's a missed opportunity for them, I love the lib and the Elixir CLDR ecosystem as a whole, makes life a lot easier, so thank you for all the effort :)

I feel like the consistent way would be preferred, so a change from today so that the default is returned when a locale is configured but a translation is not available. That would align it with how the fallback chain and "get by field" works.

kipcole9 commented 3 months ago

Makes sense to me, and thank you for the kind words. I'll tackle this ASAP but since I'm on the road it might take a few days.

kipcole9 commented 3 months ago

I've push a commit that fixes this issue I believe, would you consider giving this a spin using the main branch? If you're ok, I'll add some additional tests and publish an update.

tozz commented 3 months ago

Thanks for the quick turn around, I actually don't see any difference with running this from main, I made sure to wipe deps and _build too, but behavior is the same.

kipcole9 commented 3 months ago

@tozz, sorry for wasting your time. I'm a bit perplexed because my testing seems to show the correct results for your example. I'll go back and test more closely to your examples.