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

Support translatable words within calculated properties in case tiles #34809

Merged
merged 11 commits into from
Aug 8, 2024

Conversation

minhaminha
Copy link
Contributor

@minhaminha minhaminha commented Jun 21, 2024

Product Description

This PR introduces UI that allows for users to define translatable blocks of text to insert in the Calculated Properties under the Case List and Case Detail tabs for a certain case list module. Users will be able to select the Translatable Text format and assign the translatable text to a certain key, which can then be referenced in the calculated property. These text blocks are Transifex compatible.

Technical Summary

Jira ticket

The PR is mostly two parts: the UI, which borrows from the existing Conditional ID Mapping workflow and the modified suite generation for properties of the Translatable Text type. The tech spec/design discussion can be found in this ticket.

Feature Flag

Feature flags that enable the use of case tiles

Safety Assurance

Safety story

Tested locally and on staging, passed QA-testing.

Overall I think the change is pretty safe from existing data since it only works with new data under the TranslatableEnum type.

Automated test coverage

QA Plan

QA Ticket

Rollback instructions

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

Labels & Review

  • Risk label is set correctly
  • The set of people pinged as reviewers is appropriate for the level of risk of the change

@minhaminha minhaminha added the product/feature-flag Change will only affect users who have a specific feature flag enabled label Jun 21, 2024
@dimagimon dimagimon added the Core Compatibility Changes may impact Mobile and Formplayer label Jun 21, 2024
@minhaminha minhaminha marked this pull request as ready for review July 9, 2024 15:11
self.format.ui.parent().append(self.enum_extra.ui);
} else if (this.val() === "clickable-icon") {
self.enum_extra.values_are_icons(true);
self.enum_extra.keys_are_conditions(true);
self.enum_extra.values_are_translatable(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

what needs to be translated for clickable icons? The alt text?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to double check since it was a month ago but I think I forgot to take that line out

"format": column.format
"format": column.format,
"variables": '',
"endpoint_action": '',
Copy link
Contributor

Choose a reason for hiding this comment

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

was endpoint_action just missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, both variables and endpoint_action were set as blank strings later down the function so I just grouped them into the initial dictionary creation

@@ -200,6 +201,10 @@ def _get_xpath_function(self, column):
else:
xpath_function = self._escape_xpath_function(get_column_generator(
self.app, self.module, self.detail, column).xpath_function)
if column.format == "translatable-enum":
for enum in column.enum:
if enum.key in xpath_function and 'k' + enum.key not in xpath_function:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain the 'k'? I am confused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when adding the enum keys to the app_strings file, it always uses key_as_variable which prepends a k to the variable name so it won't start with a numeral/be a valid xml variable. I don't want the user to always have to add k when referencing the variable so I'm correcting it in the backend instead so that a calculation like concat($test) is actually read as concat($ktest) which maps to a valid string in the app_strings file.

Looking at it again... I definitely could've just made it so it doesn't prepend k for translatable-text formats but I guess it's nice that you could potentially start a key name with a number and you could reuse keys you use for custom variables without it overlapping.

@@ -86,19 +86,30 @@ class XPathEnum(TextXPath):
@classmethod
def build(cls, enum, format, type, template, get_template_context, get_value):
variables = []
variable_keys = []
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't reviewed this PR as a whole. A little overwhelmed with all the context needed. But Instead of a separate list for the keys, it might be simpler to just iterate through enum again, lower down:

for item in enum:
    key = item.key_as_variable
    if key[1:] in calculated_property:
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true or even

for item in enum:
    if item.key_as_variable[1:] in calculated_property:
...

@minhaminha minhaminha merged commit e97b0e1 into master Aug 8, 2024
13 checks passed
@minhaminha minhaminha deleted the ml/case-tile-translations branch August 8, 2024 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Compatibility Changes may impact Mobile and Formplayer product/feature-flag Change will only affect users who have a specific feature flag enabled QA Passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants