rusackas commented on PR #39448: URL: https://github.com/apache/superset/pull/39448#issuecomment-4426329721
**Caught a real bug from doing a fresh test run** — pushed fix in fe3fa946c4. I dry-ran the script then did a small `--limit 6 --batch-size 2` real run on French to confirm the per-batch save was working. The save loop is fine, but the run surfaced a regression in plural handling: the model sometimes returns a JSON **array** for plural forms (e.g. `["form0", "form1"]`, which is a perfectly valid representation since plural forms are ordered) and `_apply_translation` only handled the dict / scalar / non-JSON-string cases — the list case fell through to `str(plural_value)`, broadcasting Python list-repr (`['form0', 'form1']`) into **both** plural slots: ```po msgstr[0] "['Ajouté 1 nouvelle colonne au jeu de données virtuel', 'Ajouté %s nouvelles colonnes au jeu de données virtuel']" msgstr[1] "['Ajouté 1 nouvelle colonne au jeu de données virtuel', 'Ajouté %s nouvelles colonnes au jeu de données virtuel']" ``` The committed Spanish translations escaped this — I scanned the .po and didn't find any list-repr — but it would have hit us on any future run for any language. **Fix:** - Extracted `_apply_plural_translation` helper. Handles dict (existing), list (new), scalar, and non-JSON-string. List path distributes by index and repeats the last form if the model returned fewer forms than the language requires (better than leaving slots blank, which would render as the raw English msgid via gettext fallback). - The split also drops the cyclomatic complexity of `_apply_translation` back below the C901 threshold. - 4 new regression tests: list response, list round-tripped through `parse_response`, list shorter than required forms, and empty list. **Verified end-to-end:** with the fix, the same French entry that previously broke now writes correctly: ```po msgstr[0] "1 nouvelle colonne ajoutée au jeu de données virtuel" msgstr[1] "%s nouvelles colonnes ajoutées au jeu de données virtuel" ``` So @sadpandajoe — the test push was worth it; the consistency check exposed something we'd have shipped. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
