Checking "Needs editing" on a translated entry trigger "Has been translated" warning

[pleonex]

Describe the bug After an entry has been already translated (even if it's already marked as "Need editing"), if the translation is modified and the user keeps the "Need editing" checked, it will trigger the warning "Has been translated".

I think it shouldn't trigger that warning at least, the message is misleading and in any case the report already marks the entry that needs editing as red.

To Reproduce Steps to reproduce the behavior:

• The review role is not enabled for the project.

1. Go to an entry for a component (.po in my case)
2. Translate for the first time the entry and click Save.
3. Go to that entry again, click on "Needs editing" and then Save.
4. The warning will appear.

Also:

1. Go to an entry.
2. Translate for the first time an entry, check "Needs editing" and click Save.
3. Go back to the entry, update the translation, keep the "Needs editing" checked and click Save.
4. The warning will show.

Expected behavior This specific warning shouldn't show every time a translation is made and Needs editing is there. It's not a warning and the user is already marking as needing some action.

Server configuration and status list_versions:

 * Weblate 3.7.1
 * Python 3.7.3
 * Django 2.2.4
 * Celery 4.3.0
 * celery-batches 0.2
 * six 1.12.0
 * social-auth-core 3.2.0
 * social-auth-app-django 3.1.0
 * django-appconf 1.0.3
 * translate-toolkit 2.4.0
 * translation-finder 1.6
 * Whoosh 2.7.4
 * defusedxml 0.6.0
 * Git 2.20.1
 * Pillow 5.4.1
 * python-dateutil 2.8.0
 * lxml 4.3.2
 * django-crispy-forms 1.7.2
 * django_compressor 2.3
 * djangorestframework 3.10.2
 * user-agents 2.0
 * jellyfish 0.7.2
 * diff-match-patch 20121119
 * pytz 2019.2
 * pyuca 1.2
 * ruamel.yaml 0.16.0
 * tesserocr 2.4.0
 * Mercurial 4.8.2
 * git-svn 2.20.1
 * Database backends: django.db.backends.postgresql
 * Cache backends: avatar:FileBasedCache, default:RedisCache
 * Email setup: django.core.mail.backends.smtp.EmailBackend: smtp.sendgrid.net
 * Celery: redis://cache:6379/1, redis://cache:6379/1, regular
 * Platform: Linux 5.1.20-200.fc29.x86_64 (x86_64)

check --deploy (I should fix those permissions errors... it's because I am mounting folders with Docker...)

23:07 $ docker-compose exec --user weblate weblate weblate check --deploy
SystemCheckError: System check identified some issues:

CRITICALS:
?: (weblate.E002) Path /app/data/extensions is not writable, check your DATA_DIR settings.
        HINT: https://docs.weblate.org/en/weblate-3.7.1/admin/install.html#file-permissions
?: (weblate.E002) Path /app/data/extensions/checks is not writable, check your DATA_DIR settings.
        HINT: https://docs.weblate.org/en/weblate-3.7.1/admin/install.html#file-permissions
?: (weblate.E002) Path /app/data/extensions/checks/brace.py is not writable, check your DATA_DIR settings.
        HINT: https://docs.weblate.org/en/weblate-3.7.1/admin/install.html#file-permissions
?: (weblate.E002) Path /app/data/settings-override.py is not writable, check your DATA_DIR settings.
        HINT: https://docs.weblate.org/en/weblate-3.7.1/admin/install.html#file-permissions

WARNINGS:
?: (security.W004) You have not set a value for the SECURE_HSTS_SECONDS setting. If your entire site is served only over SSL, you may want to consider setting a value and enabling HTTP Strict Transport Security. Be sure to read the documentation first; enabling HSTS carelessly can cause serious, irreversible problems.
?: (security.W008) Your SECURE_SSL_REDIRECT setting is not set to True. Unless your site should be available over both SSL and non-SSL connections, you may want to either set this setting True or configure a load balancer or reverse-proxy server to redirect all connections to HTTPS.
?: (security.W012) SESSION_COOKIE_SECURE is not set to True. Using a secure-only session cookie makes it more difficult for network traffic sniffers to hijack user sessions.

System check identified 7 issues (1 silenced).

[pleonex]

Is this warning issue related to the translate-toolkit? Isn't related to the warning logic about detecting empty entries when needs editing is enabled?

PD: Originally the issue had the diff from the other issue. I already removed it but just confirming in case that was causing some misunderstanding.

[nijel]

Yes, I just read the email notification and flagged the issue accordingly. The translate-toolkit issue is https://github.com/WeblateOrg/weblate/issues/2934, this one is on Weblate side.

[nijel]

Thank you for your report, the issue you have reported has just been fixed.

• In case you see problem with the fix, please comment on this issue.
• In case you see similar problem, please open separate issue.
• If you are happy with the outcome, consider supporting Weblate by donating.

[Daniel-Tr]

Hi! I'm afraid this problem occurs again in newer versions. We are on 4.3.2 still, but the 4.4-dev shows the same behavior: https://hosted.weblate.org/checks/translated/

[nijel]

For the few strings I've checked it seems correct - the string has been translated and was marked for edit. Can you please point to actual example where the check is wrong?

[Daniel-Tr]

Yes, those strings are marked for edit. But isn't it the point of this issue, that they should not - simply because of that - trigger a "Has been translated" warning?

The documentation for this warning states "This can happen when the translations have been reverted in VCS or lost otherwise.". This isn't the case here, leading to a very misleading information instead, covering up situations where actual interaction may be required (we got to this point recently due to changes in the JSON parser, which basically destroyed our VCS).

There are strings showing this warning, which seem to show it correctly: https://hosted.weblate.org/translate/andors-trail/user-interface/zh_Hans/?checksum=8b149fa03fec472a

But most of them seem to show it just because of the "Needs editing" marker. I also checked this with our translations to confirm that this is reproducable this way. Example: https://hosted.weblate.org/translate/andors-trail/user-interface/nl/?checksum=3a17e40effea7162

[nijel]

The original issue was different - the string was only marked as needing edit in Weblate and it was falsely flagged as being translated, while it has not been. If you think the check should behave differently than it does now, please open a separate issue. Right now it indicates all strings which have been translated regardless how it was changed to not translated state.

we got to this point recently due to changes in the JSON parser, which basically destroyed our VCS

There should be no breaking changes, can you please elaborate more on that (either in separate issue or in discussions).

https://hosted.weblate.org/translate/andors-trail/user-interface/nl/?checksum=3a17e40effea7162

The history shows here that the string was translated, so technically the check is correct. In reality there was a time, where need edit changes were tracked as completed translations in history and there is no way to recover this information now.

[Daniel-Tr]

Thank you for the follow up. I opened a new issue (but I still can't see the difference to this one to be honest. The description is pretty much the same, see #5107).

There should be no breaking changes, can you please elaborate more on that (either in separate issue or in discussions).

We used the WebExtension JSON format before, but not using a hierarchy for elements (using dot-separated identifiers instead). This used to work until some update, which converted all JSON files to include the hierarchy. I would have to ask the admin that fixed the problem for us for more information though, if necessary.