[#2668] Translations sprint 25 #1352
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1352 +/- ##
========================================
Coverage 95.18% 95.19%
========================================
Files 1001 1002 +1
Lines 37012 37029 +17
========================================
+ Hits 35231 35248 +17
Misses 1781 1781 ☔ View full report in Codecov by Sentry. |
b72a1b3 to
35c128a
Compare
81ef608 to
a84710a
Compare
a7cd810 to
50b725a
Compare
There was a problem hiding this comment.
Whoops, 2 testen falen nog, kijk er maandag even naar:
FAIL: test_login_for_inactive_user_shows_appropriate_message (open_inwoner.accounts.tests.test_auth.TestLoginLogoutFunctionality.test_login_for_inactive_user_shows_appropriate_message)
AssertionError: ['Voer een juiste E-mailadres en wachtwoord in. Let op da[37 chars]jn.'] != ['Voer een juist E-mailadres en wachtwoord in. Let op dat[36 chars]jn.']
======================================================================
FAIL: test_login_with_wrong_credentials_shows_appropriate_message (open_inwoner.accounts.tests.test_auth.TestLoginLogoutFunctionality.test_login_with_wrong_credentials_shows_appropriate_message)
50b725a to
24157bb
Compare
| #| "Voer een juiste E-mailadres en wachtwoord in. Let op dat beide velden " | ||
| #| "hoofdlettergevoelig zijn." | ||
| #: open_inwoner/accounts/tests/test_auth.py:1775 | ||
| #: open_inwoner/accounts/tests/test_auth.py:1790 | ||
| msgid "" | ||
| "Voer een juiste E-mailadres en wachtwoord in. Let op dat beide velden " |
There was a problem hiding this comment.
@swrichards Deze passage ('juiste' zou 'juist' moeten zijn) krijg ik niet gecorrigeerd zonder dat de tests falen... Het veranderen van de strings in src/open_inwoner/accounts/tests/test_auth.py helpt ook al niet.
Omdat deze teksten niet zichtbaar zijn voor gebruikers, heb ik de 'vertaling' hier maar verwijderd, maar suggesties zijn welkom...
Misschien komt 't door het verschil in grammatica met de Django core hier: env/lib/python3.11/site-packages/django/contrib/auth/locale/nl/LC_MESSAGES/django.po
There was a problem hiding this comment.
Zo te zien is zoals je zegt dit inderdaad de (in onze ogen onjuiste) vertaling die Django zelf heeft voor NL hier.
Daarom is het gek dat we in src/open_inwoner/accounts/tests/test_auth.py uberhaupt de errors in gettext wrapper met een Nederlandse boodschap: dat zou ofwel de Engelse tekst moeten zijn door Django gedefinieerd (met gettext), of de naar NL vertaalde tekst (zonder gettext).
Enfin, lang verhaal kort: ik denk dat dit hier een prima oplossing is, maar ik zou wel die tests graag fixen zodat we hier de volgende keer niet tegenaan lopen. Zal dat zo even proberen te doen in een aparte PR.
There was a problem hiding this comment.
@swrichards Die aparte PR zou je dan misschien moeten branchen vanaf deze, want anders krijg je wellicht dezelfde PO/MO conflicten en fails als ik :-O
There was a problem hiding this comment.
Yes ik wacht tot deze gemerged is.
| msgid "%d actions was successfully marked as deleted." | ||
| msgid_plural "%d stories were successfully marked as deleted." |
There was a problem hiding this comment.
Weet niet zeker of we hier nu wat mee moeten, maar: waarom is het enkelvoud actions en het meervoud plural?
There was a problem hiding this comment.
Je bedoelt 'stories'? Ik zag hetzelfde verschijnsel bij een zelfde door Bart gemaakte actions/stories constructie bij de vertalingen. Geen idee waar de term 'stories' vandaan komt, die zijn ws. allemaal 'action/actions'
There was a problem hiding this comment.
Dan laten we het voor nu rusten. Zal die ook meenemen in mijn aparte PR.
| #| "Voer een juiste E-mailadres en wachtwoord in. Let op dat beide velden " | ||
| #| "hoofdlettergevoelig zijn." | ||
| #: open_inwoner/accounts/tests/test_auth.py:1775 | ||
| #: open_inwoner/accounts/tests/test_auth.py:1790 | ||
| msgid "" | ||
| "Voer een juiste E-mailadres en wachtwoord in. Let op dat beide velden " |
There was a problem hiding this comment.
Zo te zien is zoals je zegt dit inderdaad de (in onze ogen onjuiste) vertaling die Django zelf heeft voor NL hier.
Daarom is het gek dat we in src/open_inwoner/accounts/tests/test_auth.py uberhaupt de errors in gettext wrapper met een Nederlandse boodschap: dat zou ofwel de Engelse tekst moeten zijn door Django gedefinieerd (met gettext), of de naar NL vertaalde tekst (zonder gettext).
Enfin, lang verhaal kort: ik denk dat dit hier een prima oplossing is, maar ik zou wel die tests graag fixen zodat we hier de volgende keer niet tegenaan lopen. Zal dat zo even proberen te doen in een aparte PR.
Taiga: https://taiga.maykinmedia.nl/project/open-inwoner/us/2673
➕ corrected typo in model "upddates" => "updates"