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.

Sign up for GitHub

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

Allow case_search_display and case_search_hint values to be translated in Transifex #34794

Merged
merged 5 commits into from
Jun 26, 2024
Merged

Allow case_search_display and case_search_hint values to be translated in Transifex #34794

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cdd5635
Include case_search_display and case_search_hint in the module sheet …
minhaminha Jun 18, 2024
07bc788
Allow case search translations to upload despite missing case_search_…
minhaminha Jun 19, 2024
7a6a14c
Refactor _update_case_search_labels to allow for partial case_search_…
minhaminha Jun 21, 2024
04266bb
Use prop.label and prop.hint appropriately + add error message for ex…
minhaminha Jun 21, 2024
1626b86
Make similar error messages reusable
minhaminha Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions corehq/apps/translations/app_translations/upload_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,29 +107,23 @@ def update(self, rows):
if self.no_items_text:
self._update_translation(self.no_items_text, self.module.case_details.short.no_items_text)

self._update_case_search_labels(rows)
self._update_case_search_labels()

return self.msgs

def _update_case_search_labels(self, rows):
def _update_case_search_labels(self):
properties = self.module.search_config.properties
displays = [row for row in self.condensed_rows if row['list_or_detail'] == 'case_search_display']
hints = [row for row in self.condensed_rows if row['list_or_detail'] == 'case_search_hint']
if len(displays) != len(hints) or len(displays) != len(properties):

message = _(
'Expected {expected_count} case_search_display and case_search_hint '
'properties in menu {index}, found {actual_label_count} for case_search_display and '
'{actual_hint_count} for case_search_hint'
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
'No Case Search config properties for menu {index} were updated.'
).format(
expected_count=len(properties),
actual_label_count=len(displays),
actual_hint_count=len(hints),
index=self.module.id + 1,
)
self.msgs.append((messages.error, message))
else:
def partially_update_translations(incomplete_rows):
property_dict = {prop.name: prop for prop in properties}
for row in incomplete_rows:
prop_name = row.get('case_property')
if prop_name in property_dict:
self._update_translation(row, property_dict[prop_name].hint)
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved

if len(displays) == len(properties):
for display_row, prop in itertools.chain(zip(displays, properties)):
if display_row.get('case_property') != prop.name:
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
message = _('A display row for menu {index} has an unexpected case search property "{field}". '
Expand All @@ -140,6 +134,20 @@ def _update_case_search_labels(self, rows):
self.msgs.append((messages.error, message))
continue
self._update_translation(display_row, prop.label)
else:
message = _(
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
'Expected {expected_count} case_search_display '
'properties in menu {index}, found {actual_label_count} for case_search_display. '
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
'Case Search config properties for menu {index} were only partially updated.'
).format(
expected_count=len(properties),
actual_label_count=len(displays),
index=self.module.id + 1,
)
self.msgs.append((messages.warning, message))
partially_update_translations(displays)

if len(hints) == len(properties):
Copy link
Contributor

Choose a reason for hiding this comment

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

Recognizing this wasn't part of your changes, but this message could be consolidated with the almost exact same one for display

'A hint row for menu {index} has an unexpected case search property "{field}". '
                                'Case properties must appear in the same order as they do in the bulk '
                                'app translation download. No translations updated for this row.'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my response to your other comment also applies to this one!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm seeing that string is already used in two places, but one says "A hint row" while the other is "A display row"

I was thinking we can do something like

msg = 'A {row_property} row for menu {index} has an unexpected case search property "{field}". '
                                'Case properties must appear in the same order as they do in the bulk '
                                'app translation download. No translations updated for this row.

and we can do msg.format('hint'...) and msg.format('display'...). But nbd, certainly not blocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah you're right I misread the part of the code this comment was attached to - made the error message reusable!

for hint_row, prop in itertools.chain(zip(hints, properties)):
if hint_row.get('case_property') != prop.name:
message = _('A hint row for menu {index} has an unexpected case search property "{field}". '
Expand All @@ -150,6 +158,10 @@ def _update_case_search_labels(self, rows):
self.msgs.append((messages.error, message))
continue
self._update_translation(hint_row, prop.hint)
else:
# Empty hint rows are not pushed to Transifex
# So it could commonly give back a translations file with these rows "missing"
partially_update_translations(hints)

def _update_report_module_rows(self, rows):
new_headers = [None for i in self.module.report_configs]
Expand Down
4 changes: 2 additions & 2 deletions corehq/apps/translations/integrations/transifex/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
CONTEXT_REGEXS = {
# Module or Form: sheet name for module/form: unique id
'module_and_forms_sheet': r'^(Module|Form):(\w+):(\w+)$', # maintain legacy module usage instead of menu
# case property: list/detail
'module_sheet': r'^(.+):(list|detail)$',
# case property: list/detail/case_search_display/case_search_hint
'module_sheet': r'^(.+):(list|detail|case_search_display|case_search_hint)$',
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
}

TRANSIFEX_MODULE_RESOURCE_NAME = re.compile(r'^module_(\w+)(_v\d+)?$') # module_moduleUniqueID_v123
Expand Down
Loading