search

mozilla / pontoon

Mozilla's Localization Platform
https://pontoon.mozilla.org
BSD 3-Clause "New" or "Revised" License
1.46k stars 528 forks source link

Fuzzy status not updated properly #2078

Open bugzilla-to-github opened 7 years ago

bugzilla-to-github commented 7 years ago

This issue was created automatically by a script.

Bug 1381852

Bug Reporter: @mathjazz See also: https://bugzilla.mozilla.org/show_bug.cgi?id=1310668

Created attachment 8887495 Not unfuzzied, stats are off

There are two (possibly related) cases when the fuzzy status doesn't update properly in the UI.

  1. When an approved translation is submitted or a suggestion is approved, any fuzzy translations of the same string to the same language should get unfuzzied (fuzzy=False). However, it doesn't always happen. I haven't been able to reproduce it, but I've see it in action (checkout the attachment). The method for calculating stats assumes that string cannot have approved and fuzzy translations at the same time (https://github.com/mozilla/pontoon/blob/master/pontoon/base/models.py#L2208), so when they do, stats break.

  2. When an approved translation is made fuzzy in repository, we should not only mark it as fuzzy, but also unapprove it (approved=False) on next sync. However, it doesn't always happen. I haven't been able to reproduce it, but I've see it in action (it occured at least for the bg and nl locale after the following pluralized string was made fuzzy: https://github.com/mozilla/addons-frontend/commit/d532b1da895604bafb1c2a4b7177e9e128459f62#diff-2095e4bdf667d5a30ca23a48c0c6db7fL477). Note that the strings were properly marked as fuzzy, but the approved flag was kept set.

bugzilla-to-github commented 7 years ago

Comment Author: @mathjazz

PR 679 should address #2, at least partially: https://github.com/mozilla/pontoon/pull/679

bugzilla-to-github commented 7 years ago

Comment Author: @mathjazz

I was able to reproduce #1:

Prerequisite: String A needs to have a fuzzy translation in both, VCS and DB.

Steps to reproduce:

  1. Submit an approved translation to string B, which will prevent sync from skipping.
  2. Start sync.
  3. Submit an approved translation for string A AFTER update_translations() and BEFORE changeset.execute()[1], which will unfuzzy the fuzzy translation.
  4. Wait for sync to complete.

Result: The unfuzzied translation from step 3 became fuzzy again. String A now has an approved and a fuzzy translation, which also breaks stats.

Explanation: pontoon.sync.core.py:update_translations() decides if we need to update the translation in the DB or VCS. When it executes, the translation for string A is not submitted yet, so it decides to update the DB translation with anything that might have changed in the VCS (bug #1.1). That happens in pontoon.sync.changeset.py:update_entity_translations_from_vcs(), where the previously unfuzzied translation gets marked as fuzzy again. The approved translation fails to get unapproved, because we only unapprove translations submitted before the sync job started (bug #1.2).

[1] https://github.com/mozilla/pontoon/blob/master/pontoon/sync/tasks.py#L314

bugzilla-to-github commented 7 years ago

Comment Author: @mathjazz

Created attachment 8897544 Possible fix for bug #1B

Note that tests fail.

Fixing 1B only brings us to the aligned state between DB and VCS. And fixes stats.

We'd still need to fix 1A to prevent approved translations from becoming fuzzy again. It's also possible that fixing 1A prevents 1B from happening.

Attached file: untitled.diff (text/plain, 1733 bytes) Description: Possible fix for bug #1B

bugzilla-to-github commented 7 years ago

Comment Author: @github-actions

Commit pushed to master at https://github.com/mozilla/pontoon

https://github.com/mozilla/pontoon/commit/05e43fe825a1091f0ca7eb9a3aabb1a0bbb8f628 Bug #1381852: Unapprove fuzzy translations on import (#679)

If an existing translation is made fuzzy in the VCS, we mark it as fuzzy in the DB on next sync, but don't unapprove it. That is because two separate instances of the same translation are passed to bulk_update - the first one sets the fuzzy flag and the second one unsets the approved flag. bulk_update only executes the first instance, so now we properly set all flags on it and don't pass the second one.

Note: this only applies to translations that are made fuzzy in the locale file. Most of the time a new source strings is created when existing translation are made fuzzy on msgmerge. This case already behaves properly.

bugzilla-to-github commented 7 years ago

Comment Author: @mathjazz

Comment on attachment 8897544 Possible fix for bug #1B

This patch is foobar.

Attached file: untitled.diff (text/plain, 1733 bytes) Description: Possible fix for bug #1B