Skip to content

Fix paywall locale receiving UN M.49 standard localization #5309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 23, 2025

Conversation

ajpallares
Copy link
Member

@ajpallares ajpallares commented Jun 20, 2025

Motivation

Found a bug where displaying a paywall with the only Spanish localization being "es_MX" would not show some localization strings (device region: Spain, preferred languages: es, en_US)

Description

In this case, the server's ui_config.localization contains "es_419" (UN M.49 standard for the Latin America and the Caribbean region) but no "es_MX". With the existing implementation, the SDK was not able to match "es_MX" with "es_419".

This PR adds an extra step when trying to find the matching locale from UIConfig.localizations. As a last resort, it tries to find a key in ui_config.localization with matching language and script.

@ajpallares ajpallares requested review from a team June 20, 2025 10:34
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense! :shipit:

// This covers cases where dictionary keys contain the UN M.49 standard. For example:
// If locale = "es_MX" and dictionary key = "es_419" (both have rc_languageAndScript = "es-Latn")
if let onlyLanguageAndScriptIdentifier = locale.rc_languageAndScript,
let match = self.valueForLanguageAndScript(onlyLanguageAndScriptIdentifier) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's possible/worth to add some tests for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Good point that I totally forgot about! Thank you for the suggestion!

@ajpallares
Copy link
Member Author

ajpallares commented Jun 20, 2025

Actually, I was writing a test for this and the first one I wrote did not pass

func test_es_MX_Matches_es_419_ByLanguageAndScript() {
    let localizations = [
        "es_419": Self.expectedTranslations,
        "es": ["wrong": "this is es"]
    ]

    let locale = Locale(identifier: "es_MX")

    // This tests that "es_MX" matches "es_419" due to both sharing the same language and script ("es-Latn")
    let foundLocalizations = localizations.findLocale(locale)
    expect(foundLocalizations).to(equal(Self.expectedTranslations))
}

The reason is that I originally added the new check as a last resort (it was the last check). But that makes "es_MX" match "es" before "es_419", which is wrong.

Therefore, I think the new check should happen before checking only the language identifier (done 14d3c99)

Please lmk if you think otherwise!

@ajpallares ajpallares requested review from tonidero, MarkVillacampa and a team June 20, 2025 16:26
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeei for tests! 🙌

@ajpallares ajpallares merged commit 99eb52b into main Jun 23, 2025
12 checks passed
@ajpallares ajpallares deleted the fix-paywall-locales-in-UN-M49-standard branch June 23, 2025 06:56
@ajpallares
Copy link
Member Author

The fix in this PR is not correct (tests were failing randomly; thanks again @tonidero for suggesting the tests 🙏). I created a new PR with a whole different approach here #5316

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants