Make form title translatable #1640

avazirna · 2024-11-06T16:05:05Z

Product Description

This PR makes it possible to set the form title according to the user's preferred language or by changing the language in the middle of a form session. Currently, the form title is derived from the form XML, which is defined by the active language when the form is saved, after a change or form creation.
In the video at the bottom of the description, it is possible to see that:

While the language of the user is Portuguese and most of the app content is shown in that language, when entering the form, the form title is in English;
Changing the language in app builder to Portuguese, making a change to the app and then creating a new build, results in the form title be presented in that language

Additionally, switching to another language in the middle of a form session doesn't affect the form title in any way.

Technical Summary

In terms of design decisions, two options were entertained during this work:

Setting the title in the appropriate language when preparing response objects to be sent to the front-end;
Maintaining the title in SerializingFormSession objects in the appropriate language;

Considering that the title in response objects is set based on SerializedFormSession objects, number 2 was the chosen option. This not only ensured fewer changes to the codebase, but it will also improve maintainability since changes to response classes are unlikely to affect this capability.

Safety Assurance

Safety story

Local tests were successful but one resource had to be update as with this change, the form title is retrieved from the translations.

Special deploy instructions

This PR can be deployed after merge with no further considerations.

Rollback instructions

This PR can be reverted after deploy with no further considerations.

Review

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

issue_form_title.mov

This is to be used to retrieve the translation

codecov · 2024-11-07T12:58:53Z

Codecov Report

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

Project coverage is 70.27%. Comparing base (bb7cdbb) to head (6730246).

Files with missing lines	Patch %	Lines
...a/org/commcare/formplayer/session/FormSession.java	94.73%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1640      +/-   ##
============================================
+ Coverage     70.22%   70.27%   +0.05%     
- Complexity     2007     2014       +7     
============================================
  Files           254      254              
  Lines          7902     7919      +17     
  Branches        745      748       +3     
============================================
+ Hits           5549     5565      +16     
  Misses         2070     2070              
- Partials        283      284       +1

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

shubham1g5

We should add some unit tests that can demonstrate the title changing appropriately with the change of language.

shubham1g5 · 2024-11-08T06:56:49Z

src/main/java/org/commcare/formplayer/objects/SerializableFormSession.java

@@ -51,7 +51,7 @@ public enum SubmitStatus {
    @Column(name = "menu_session_id", updatable = false)
    private String menuSessionId;

-    @Column(updatable = false)


Since this is a db schema change, does this break resuming into incomplete forms saved before this change ?

This should only impact DML statements, not the schema itself, but happy to test to make sure it doesn't break anything

shubham1g5 · 2024-11-08T07:01:20Z

src/main/java/org/commcare/formplayer/session/FormSession.java

+    private String getFormTitleLocaleKey() {
+        if (commandId != null && !commandId.equals("")) {
+            String preparedCommandId = commandId.replace("-", "");
+            return "forms." + preparedCommandId;
+        }
+        return "";
+    }


there is no guarantee in Xform spec that forms locale key will follow this format (it can be dianasour.m0f0 for exmaple). This should be using the same logic as we use to load the display text for menus instead.

@shubham1g5 opted for a different approach, instead of storing the command ID to then convert it, I'm not storing the form title locale key.

shubham1g5 · 2024-11-08T07:03:11Z

src/main/java/org/commcare/formplayer/services/NewFormResponseFactory.java

@@ -108,7 +108,8 @@ public NewFormResponse getResponse(NewSessionRequestBean bean, String postUrl) t
                bean.getRestoreAsCaseId(),
                null,
                formplayerRemoteInstanceFetcher,
-                bean.getWindowWidth()
+                bean.getWindowWidth(),
+                null


this doesn't need to be translated ?

because SessionFrame is null, I believe there is no commandId. But I'm not too familiar with this workflow, so happy to follow your lead. However, the fallback there is the title from FormDef, as it is right now.

shubham1g5 · 2024-11-08T07:03:58Z

src/main/java/org/commcare/formplayer/application/FormController.java

@@ -141,6 +141,7 @@ public FormEntryResponseBean changeLocale(@RequestBody ChangeLocaleRequestBean c
                changeLocaleBean.getSessionId());
        FormSession formEntrySession = formSessionFactory.getFormSession(serializableFormSession, changeLocaleBean.getWindowWidth());
        formEntrySession.changeLocale(changeLocaleBean.getLocale());
+        formEntrySession.updateFormTitle(changeLocaleBean.getLocale());


this can reside inside formEntrySession.changeLocale itself.

avazirna · 2024-11-11T09:23:06Z

We should add some unit tests that can demonstrate the title changing appropriately with the change of language.

@shubham1g5 I have incorporated this into existing localization tests: 2f1f467

shubham1g5 · 2024-11-11T11:13:26Z

src/main/java/org/commcare/formplayer/session/FormSession.java

@@ -108,7 +111,9 @@ public FormSession(SerializableFormSession session,

        this.sandbox = restoreFactory.getSandbox();
        this.windowWidth = windowWidth;
-
+        if (commCareSession != null && commCareSession.getCurrentEntry() != null) {
+            this.formTitleLocaleKey = commCareSession.getCurrentEntry().getText().getArgument();


think we need to usegetText().evaluate() which already seems to have locale support, so you might be able to use it's value directly without doing any further locale processing.

HI @shubham1g5 I explored this approach before and revisited after seeing your comment. The issue is that it seems that getText().evaluate() fetches the values using the default locale and not the current language

I think that takes into account whatever locale is set to the Localiser, saying as we use Localization.get() at other several places in the app to get the translation.

Yes, but when we change language in the middle of a form session, it doesn't change the global locale, just the form-level locale. So calling getText().evaluate() continues to return the value in the default language, right?

avazirna and others added 7 commits November 5, 2024 00:26

Add Command ID to FormSession

0486fdc

This is to be used to retrieve the translation

Initialize SerializableFormSession with title in user default language

6908c93

Get form title by locale

c7a6b99

Update form title when language changes

392ab07

Merge branch 'master' into make-form-title-translatable

fc164a3

Use default locale if locale is not registered

5789451

Update tests

847e1f1

avazirna requested a review from shubham1g5 November 7, 2024 12:57

avazirna marked this pull request as ready for review November 7, 2024 13:14

shubham1g5 requested changes Nov 8, 2024

View reviewed changes

avazirna added 4 commits November 8, 2024 13:48

Refactor

f27e5a8

Replace commandId with formTitleLocaleKey

033ec95

Update tests

16aa0fc

Assert form title in LocalizationTests

2f1f467

avazirna requested a review from shubham1g5 November 11, 2024 09:24

Update test

6730246

shubham1g5 requested changes Nov 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make form title translatable #1640

Make form title translatable #1640

avazirna commented Nov 6, 2024 •

edited

Loading

codecov bot commented Nov 7, 2024 •

edited

Loading

shubham1g5 left a comment

shubham1g5 Nov 8, 2024

avazirna Nov 8, 2024

shubham1g5 Nov 8, 2024

avazirna Nov 8, 2024

avazirna Nov 11, 2024

shubham1g5 Nov 8, 2024

avazirna Nov 8, 2024 •

edited

Loading

shubham1g5 Nov 8, 2024

avazirna commented Nov 11, 2024

shubham1g5 Nov 11, 2024

avazirna Nov 11, 2024

shubham1g5 Nov 11, 2024

avazirna Nov 12, 2024

Make form title translatable #1640

Are you sure you want to change the base?

Make form title translatable #1640

Conversation

avazirna commented Nov 6, 2024 • edited Loading

Product Description

Technical Summary

Safety Assurance

Safety story

Special deploy instructions

Rollback instructions

Review

codecov bot commented Nov 7, 2024 • edited Loading

Codecov Report

shubham1g5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avazirna Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avazirna commented Nov 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avazirna commented Nov 6, 2024 •

edited

Loading

codecov bot commented Nov 7, 2024 •

edited

Loading

avazirna Nov 8, 2024 •

edited

Loading