Clean up import UI, fix last string bug and add tests
| Project: | Localization server |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
- #607978: File to import not found when attempting to import translation reported that file imports are not showing Drupal error messages when the file is oversized
- #600002: Delete the "Initial translation import" (Polish group) showed that people might not understand what does the different status values mean in the import
- #561296: Error message when importing .po file where the last string is untranslated notes importing files with empty last items is bogus (and is bogus in D7 consequently, since we copied code from there)
- I've noted in #560132: Write tests that import/export tests are lacking, so time to cover import in tests
Since these are interrelated, I've decided to bring these under one roof. The attached patch is a first take at improving the import UI. Do people agree it is an actual improvement?
Not committing yet until writing tests to ensure it still works as expected.
BEFORE:
AFTER:
Changes include:
- more meaningful title for file field,
- file description now contains max file size enforced
- suggestion status not a checkbox anymore, since this allows us to explain what happens in both cases
- attribution is not a checkbox anymore, since this allows us to explain what happens in both cases
Tried to exercise brevity in writing these texts.
| Attachment | Size |
|---|---|
| ImportBefore.png | 37.8 KB |
| ImportAfter.png | 46.64 KB |
| new-import-ui.patch | 5.66 KB |
#1
This patch now includes the UI change as well as a bugfix for #561296: Error message when importing .po file where the last string is untranslated. The bugfix itself was only one line change, and the tests will not work at all, if the bug is there, so the tests verify that the bug is fixed. The positive feedback does not appear if the last item bug is there, so those asserts will fail if the bug is there. I'll quickly submit a core issue for this bug as well.
The tests include:
- importing a .po file with a simple translation, a duplicate translation, an unknown string, a context aware string, a plural and an empty last string
- importing the .po again, and seeing the duplicates are properly found
- importing as team with additional data
- importing as translation, but diverted to suggestion due to above lingering data
- importing as translation without lingering data
So the used .po file covers all kinds of constructs we support (singular, plural, context) and the test tries to cover most import option combinations. It does not cover all (import translation as team for example), but I believe this is already pretty nice coverage.
With these tests added, the l10n_server tests now pass 938 asserts, yay!
#2
Followup Drupal core bug with suggested patch for the last msgstr empty bug opened at #611786: .po file should not be considered broken if last msgstr is empty.
#3
It appears you've already marked it as fixed but yes, your proposed changes would make the UI much clearer.
#4
Automatically closed -- issue fixed for 2 weeks with no activity.