Skip to content
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

Fix root cause of language problems #668

Closed
BPerlakiH opened this issue Feb 24, 2024 · 2 comments · Fixed by #670
Closed

Fix root cause of language problems #668

BPerlakiH opened this issue Feb 24, 2024 · 2 comments · Fixed by #670
Assignees
Milestone

Comments

@BPerlakiH
Copy link
Collaborator

BPerlakiH commented Feb 24, 2024

After throughout analysis, why the AppStore version is working as it is, what were the changes in the meantime.
I did found the following:

  • it all started with: Replace deprecated function use #588
  • I removed the deprecated function in https://github.com/kiwix/apple/pull/629/files
  • The former function was returning a single string, whereas the new one might return a comma separated list of strings.
  • The mapping from a list of strings to a list of Locales, was done a "level higher" not in the objective-c function but in swift.
  • The result is that the objective-c function was in fact converting the language codes from the feed to a 2 letter language code (across all iOS versions), and that is what we were storing in the database, which is accidentally or purposefully but in an undocumented way is working
  • When adding the swift function I was checking against the feed values, and from that PR on the DB was using 3 letter codes.
  • Unfortunately the unit-tests were not run by me on that PR, the CI is currently not running them
  • The tests were updated at a later point in time here: https://github.com/kiwix/apple/pull/652/files#diff-edd62679239139566e8891207312793f6afaf35c590661caebec8c72f81bbfc3

Proposition

The simplest, and most straightforward solution at this point is, use the existing working solution and store the language codes as 2 letter codes, apply the same for user settings. This solves the issue with iOS 15 and below, without having to manually map codes, as suggested in: #667

This purposeful workaround has to be clearly documented in code.

To avoid similar problems in the future, we need to make sure unit tests are run during CI builds (issue created)

@BPerlakiH
Copy link
Collaborator Author

@rgaudin I put the ISO-3 migration into a PR on top of this, see: #672
I discovered even more things, and implemented in a more consistent way.

@rgaudin
Copy link
Member

rgaudin commented Feb 26, 2024

So maybe close this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: DONE
Development

Successfully merging a pull request may close this issue.

2 participants