-
Notifications
You must be signed in to change notification settings - Fork 136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[REVERTED] issue 157 - warn users when any language is missing a tranlsation and blanking fields #287
[REVERTED] issue 157 - warn users when any language is missing a tranlsation and blanking fields #287
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,6 +72,66 @@ def test_label_with_unknown_subtag_should_warn(self): | |
) | ||
os.unlink(tmp.name) | ||
|
||
def test_missing_translation_no_default_lang_media_has_no_language(self): | ||
# form should test media tag w NO default language set. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test should very narrowly identify the issue. For example, a label with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I follow, tests added lmk if they do the trick! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also going to try and cover untranslated choices and other stuff, not jsut media tags. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So with the current functionality, each language known to miss at least one field will get at least one warning. This prevents warnings from being overly verbose when there are multiple translations missing.... but is that what we want? or should I extend warnings to add a warning for each question/field missing a translation? Slack me if that question is confusing and I will try to jump on. |
||
survey = self.md_to_pyxform_survey( | ||
""" | ||
| survey | | | | | | ||
| | type | name | label::English (en) | media::image| | ||
| | integer | nums | How many nums? | opt1.jpg | | ||
""" | ||
) | ||
|
||
warnings = [] | ||
tmp = tempfile.NamedTemporaryFile(suffix=".xml", delete=False) | ||
tmp.close() | ||
survey.print_xform_to_file(tmp.name, warnings=warnings) | ||
|
||
# In this survey, when 'default' s selected the label of this question will be '-'. | ||
# Also, when 'English (en)` is selected, the medial will be '-' | ||
self.assertTrue(len(warnings) == 2) | ||
os.unlink(tmp.name) | ||
|
||
def test_missing_translation_media(self): | ||
survey = self.md_to_pyxform_survey( | ||
""" | ||
| survey | | | | | | | ||
| | type | name | label::English (en) | label::French (fr) | media::image::French (fr)| | ||
| | integer | nums | How many nums? | Combien noms? | opt1.jpg | | ||
""" | ||
) | ||
|
||
warnings = [] | ||
tmp = tempfile.NamedTemporaryFile(suffix=".xml", delete=False) | ||
tmp.close() | ||
survey.print_xform_to_file(tmp.name, warnings=warnings) | ||
self.assertTrue(len(warnings) == 1) | ||
self.assertIn( | ||
"Question missing translation: /pyxform_autotestname/nums", warnings[0] | ||
) | ||
self.assertIn("Column missing: image", warnings[0]) | ||
os.unlink(tmp.name) | ||
|
||
def test_missing_translation_hint(self): | ||
survey = self.md_to_pyxform_survey( | ||
""" | ||
| survey | | | | | | | ||
| | type | name | label::English (en) | label::French (fr) | hint::French (fr) | | ||
| | integer | nums | How many nums? | Combien noms? | noms est nombres | | ||
""" | ||
) | ||
|
||
warnings = [] | ||
tmp = tempfile.NamedTemporaryFile(suffix=".xml", delete=False) | ||
tmp.close() | ||
survey.print_xform_to_file(tmp.name, warnings=warnings) | ||
self.assertTrue(len(warnings) == 1) | ||
self.assertIn( | ||
"Question missing translation: /pyxform_autotestname/nums", warnings[0] | ||
) | ||
self.assertIn("Column missing: hint", warnings[0]) | ||
os.unlink(tmp.name) | ||
|
||
def test_default_language_only_should_not_warn(self): | ||
survey = self.md_to_pyxform_survey( | ||
""" | ||
|
@@ -82,7 +142,7 @@ def test_default_language_only_should_not_warn(self): | |
| | list_name | name | label | fake | | ||
| | opts | opt1 | Opt1 | 1 | | ||
| | opts | opt2 | Opt2 | 1 | | ||
""" | ||
""" | ||
) | ||
|
||
warnings = [] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont love this
path_has_warning
pattern at all. is there a list ofcontent_types
we support? couldnt find it in the spec for some reason, but id be happy to jsut make sure the content_type actually needs translation. That said if y'all are OK with this pattern then it should be fine to merge.