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

Make language slug available via session metadata #1630

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

minhaminha
Copy link
Contributor

@minhaminha minhaminha commented Oct 8, 2024

Product Description

Users will now be able to reference the current language slug (i.e. 'en', 'es', etc.) on the screen by referencing instance('commcaresession')/session/context/applanguage

Technical Summary

Jira Ticket

Associated commcare-core PR

This is the formplayer side of the changes. All this code does is ensures that EntityScreenContext has the right locale (a string) value provided on initialization and a null value for any instance that's not directly related to the workflow kicked off by a navigateSessionWithAuth request. Because this process continually recreates the context object, its the ideal entry point to insert this value which will be propagated to the eventual generateRoot function (actual code surrounding this part and further explanation will be in the commcare-core PR).

To be complete this PR also covers an instance of ExternalDataInstance initialization that will now optionally accept a locale argument.

Safety Assurance

Safety story

This shouldn't impact existing data since the EntityScreenContext object is constantly being recreated with updated information and most of the time the locale value will be null.

QA Plan

No plans for QA but will have delivery (who request this feature) do some testing to ensure it's doing what they want.

Special deploy instructions

  • This PR can be deployed after merge with no further considerations. (Of course the commcare-core PR will be merged first and this PR updated with those changes before merging/deploying).

Rollback instructions

  • This PR can be reverted after deploy with no further considerations. (Ideally should be rolled back with attached commcare-core PR?)

Review

  • The set of people pinged as reviewers is appropriate for the level of risk of the change.

Copy link

codecov bot commented Oct 16, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.22%. Comparing base (c069207) to head (3bd8611).

Files with missing lines Patch % Lines
...mplayer/session/FormplayerInstanceInitializer.java 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1630      +/-   ##
============================================
+ Coverage     70.16%   70.22%   +0.05%     
- Complexity     1999     2003       +4     
============================================
  Files           254      254              
  Lines          7881     7903      +22     
  Branches        741      743       +2     
============================================
+ Hits           5530     5550      +20     
- Misses         2069     2070       +1     
- Partials        282      283       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

String appLang = locale;
if (locale == null) {
String[] locales = Localization.getGlobalLocalizerAdvanced().getAvailableLocales();
appLang = locales[locales.length-1];
Copy link
Contributor

Choose a reason for hiding this comment

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

this grabs the last locale created? is that always right one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks so much for pointing this out, totally forgot to go back to this. The locales list is always as follows: ['default' (like literally just a string called default), , <all other slugs in order the were added]. The locales.length-1 was previously a placeholder and wouldn't have worked if there were more than one languages. Added a similar comment in the code too to avoid confusion.

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

Successfully merging this pull request may close these issues.

2 participants