1237667 - [translate][sync] Run compare-locales checks and display errors

Status: RESOLVED INCOMPLETE

(Webtools Graveyard :: Pontoon, defect, P2)

Description

[Francesco Lodolo [:flod]]

Currently quality checks are performed when a localizer submits a string, but it would be helpful to have a filter that let you go through the strings with failing checks.

Comment 1

[Matjaz Horvat [:mathjazz]]

Implementation note: to implement this, we'll have to store quality check errors in the database.

Comment 2

[[github robot]]

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

https://github.com/mozilla/pontoon/commit/21acbbda328fc849f9d2487e408500d72843a605
Bug 1237667 - Update to latest fluent (#627)

* Bump python-fluent to 0.4.1
* Bump fluent-syntaxt.js to 0.5.0
* Add docs for updating fluent
* Update code to reflect fluent AST and API changes

Comment 3

[Matjaz Horvat [:mathjazz]]

To clarify changes to the title - we should run compare-locales checks whenever a translation is stored in a DB:
- translate UI: single submission
- translate UI: mass actions (e.g. search & replace)
- sync
- file upload

In translate UI, we should prevent submission of translations with errors and show them to the user instantly.

On sync and file upload, we should import translations with errors and mark them as such (in the dashboard, string list, filters).

We should also run checks across all strings that are already in the DB using a DB migration.

Comment 4

[Axel Hecht [:Pike]]

Attachment: WIP pull request

Attaching the pull request that we had last summer.

Comment 5

[Jarek Śmiejczak [:jotes]]

So, as we discussed during watercooling meeting I'm planning to rebase this PR against the current codebase and see how much We're able to use from this patch.

:Axel
Do you think It's a good plan to use compare-locales 2.6.3 or should I use the current master/trunk?
I remember discussions about compare-locales 3.0 but that was a while ago.

Comment 6

[Axel Hecht [:Pike]]

>=2.7 is good.

3.0 is probably going to be a bigger fish.

Comment 7

[Axel Hecht [:Pike]]

I've glanced a bit at the code.

I don't have good suggestions on how to split the code up, the initial commit is already pretty big. Maybe the fuzzy changes and just the data model can land ahead of time.

As for the integration with compare-locales. getChecker() now doesn't need the references ahead of time, you can check needs_references on the resulting checker.
The mock entities probably need to be replaced with actual entities, the current code often does code branches based on python type. There's more in bug 1432178, so I would surely wait for that and the c-l release that comes with it.

That'd be my current observations.

Comment 8

[Jarek Śmiejczak [:jotes]]

Thanks :Pike,

I'll start from rebase then update the code related to c-l integration.

Comment 9

[Jarek Śmiejczak [:jotes]]

:mathazz, :pike

So after some work on the rebase, I've (probably) started to have general grasp how to split this pull request into smaller ones.
My initial suggestion:
* Pr 1: create Django app called "errorchecks", add code responsible for the integration with c-l with tests and add validation in Pontoon translate vie  (with possibility to skip warnings) for translators.
That would be a good case to check if integration with c-l works okay and doesn't introduce unnecessary DB changes.
* Pr 2: add models, save errors to database when somebody decides to force-submit translation.
* Pr 3: save errors during the sync process.

If you feel that We should change order or scope of those PRs - I'm open to hear your thoughts/suggestions/preferences/wisdom :D
My general goal is to make smaller and review-able changes with the full test coverage.

Comment 10

[Axel Hecht [:Pike]]

Sounds good to me.

Comment 11

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/pontoon/pull/856

Comment 12

[Matjaz Horvat [:mathjazz]]

LGTM.

Comment 13

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/pontoon/pull/931

Comment 14

[Matjaz Horvat [:mathjazz]]

Comment on attachment 8969712 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/pontoon/pull/931

Wrong bug number in commit message. Sorry for the noise.

Comment 15

[Matjaz Horvat [:mathjazz]]

Resolving as incomplete and filing separate bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=1458343
https://bugzilla.mozilla.org/show_bug.cgi?id=1458344
https://bugzilla.mozilla.org/show_bug.cgi?id=1458345
https://bugzilla.mozilla.org/show_bug.cgi?id=1458346
https://bugzilla.mozilla.org/show_bug.cgi?id=1458347