Repository navigation
fix upload warnings marked as translated - #4652
baha-bouali wants to merge 3 commits into
Conversation
|
Here is the Before/After GUI proof: warningsProof.webm |
mathjazz
left a comment
mathjazz
left a comment
There was a problem hiding this comment.
Thanks for tracking this down! The fix and the tests look good.
The swap seems to go back to the sync refactor (#3312), so new translations from sync and uploads haven't been checked since then. Since this has been happening for a while, existing translations created by sync or upload still lack their warnings and errors.
We need to re-check those separately.
| ActionLog.objects.bulk_create(actions) | ||
|
|
||
| return created, list(suggestions.values()) | ||
| return list(suggestions.values()), created |
There was a problem hiding this comment.
Nit: Since both helpers in write_db_updates() now get new_translations + updated_translations, the split return isn't needed anymore. Returning a single list (or a NamedTuple) would rule out this kind of swap in the future. Fine to leave as is, though.
There was a problem hiding this comment.
I changed to a single list. all tests passed and the behaviour should remain unchanged
mathjazz
left a comment
mathjazz
left a comment
There was a problem hiding this comment.
Thanks for the update, looks good!
Fixes #4625
Cause
update_db_translations()returns(created, approved_suggestions), butwrite_db_updates()recieved them in the reverse order, as(updated_translations, new_translations). As a result,add_failed_checks()only received the approved suggestions: newly created translations never had their warnings stored, so they showed as Translated instead of Warnings.This affects both callers of
write_db_updates(): translation uploads and repository sync.Changes
update_db_translations()now returns the lists in the order the caller expects, so the names match the intended contents.add_failed_checks()now receives both lists. Approved suggestions were already checked (by accident); checking only the new translations would have dropped that.Tests
Added regression tests:
test_upload_translations_stores_missing_placeholder_warningtest_upload_translations_approved_suggestion_stores_warning: an upload matching an existing suggestion re-approves it and keeps its warning.test_ftl_translations_failed_checks: a translation imported from the repository keeps its warning.