Skip to content

Commit 1c0dbf2

Browse files
committed
feat: warn when name or description translations are missing
1 parent ef96559 commit 1c0dbf2

File tree

10 files changed

+148
-19
lines changed

10 files changed

+148
-19
lines changed

questionpy_sdk/_package/_builder.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@
2121
from questionpy_sdk._i18n_utils import bcp47_to_posix
2222
from questionpy_sdk._package._ignores import create_ignore_spec
2323
from questionpy_sdk._package._targets import BuildTarget, DirBuildTarget
24-
from questionpy_sdk._package._validate import validate_dist_structure, validate_requested_lms_attributes
24+
from questionpy_sdk._package._validate import (
25+
validate_dist_structure,
26+
validate_package_name_and_description,
27+
validate_requested_lms_attributes,
28+
)
2529
from questionpy_sdk._package.errors import PackageBuildError
2630
from questionpy_sdk._package.source import PackageSource
2731
from questionpy_sdk.models import BuildHookName, SourceStaticQPyDependency
@@ -86,6 +90,7 @@ def write_package(self) -> None:
8690

8791
validate_dist_structure(self._manifest, self._target.dist)
8892
validate_requested_lms_attributes(self._manifest)
93+
validate_package_name_and_description(self._manifest)
8994

9095
def _run_build_hooks(self, hook_name: BuildHookName) -> None:
9196
commands = self._source.config.build_hooks.get(hook_name, [])

questionpy_sdk/_package/_validate.py

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from pathlib import Path
55

66
from questionpy_common.constants import MANIFEST_FILENAME
7-
from questionpy_common.manifest import Manifest, PackageType
7+
from questionpy_common.manifest import Bcp47LanguageTag, Manifest, PackageType
88

99
_log = logging.getLogger(__name__)
1010

@@ -128,3 +128,38 @@ def validate_requested_lms_attributes(manifest: Manifest) -> None:
128128
if unknown_attributes:
129129
unknown_attribute_list = "\n\t- " + "\n\t- ".join(unknown_attributes)
130130
_log.warning("The following LMS attributes are requested but unknown:%s", unknown_attribute_list)
131+
132+
133+
def validate_package_name_and_description(manifest: Manifest) -> None:
134+
if not _log.isEnabledFor(logging.WARNING):
135+
return
136+
137+
declared_translations = set(manifest.languages)
138+
name_translations = set(manifest.name.keys())
139+
description_translations = set(manifest.description.keys())
140+
141+
if missing_name_translations := name_translations.difference(declared_translations):
142+
missing_translations_list = "\n\t- " + "\n\t- ".join(missing_name_translations)
143+
_log.warning("The following package name translations are missing:%s", missing_translations_list)
144+
145+
if unused_name_translations := declared_translations.difference(name_translations):
146+
unused_translations_list = "\n\t- " + "\n\t- ".join(unused_name_translations)
147+
_log.warning(
148+
"The following package name translations are given but missing in the languages list:%s",
149+
unused_translations_list,
150+
)
151+
152+
if description_translations:
153+
if missing_description_translations := description_translations.difference(declared_translations):
154+
missing_translations_list = "\n\t- " + "\n\t- ".join(missing_description_translations)
155+
_log.warning("The following package description translations are missing:%s", missing_translations_list)
156+
157+
if unused_description_translations := declared_translations.difference(description_translations):
158+
unused_translations_list = "\n\t- " + "\n\t- ".join(unused_description_translations)
159+
message = "The following package description translations are given but missing in the languages list:%s"
160+
if Bcp47LanguageTag("en") in unused_description_translations:
161+
message += "\nThe package description should be available in English as it is used as a fallback."
162+
_log.warning(
163+
message,
164+
unused_translations_list,
165+
)

tests/questionpy/wrappers/conftest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ def package() -> ImportablePackage:
3333
short_name="test_package",
3434
version="1.2.3",
3535
author="Testy McTestface",
36+
name={Bcp47LanguageTag("en"): "Test Package"},
3637
api_version="0.3",
3738
languages=[Bcp47LanguageTag("en")],
3839
static_files=STATIC_FILES,

tests/questionpy_sdk/commands/repo/conftest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ def create_package(
4646
version=version,
4747
api_version="0.1",
4848
author="pytest",
49+
name={Bcp47LanguageTag("en"): "Test Package"},
4950
languages=[Bcp47LanguageTag("en")],
5051
)
5152

tests/questionpy_sdk/commands/test_package.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@
2424
def create_config(source: Path) -> PackageConfig:
2525
"""Creates a config in the given `source` directory."""
2626
config = PackageConfig(
27-
short_name="short_name", author="pytest", api_version="0.1", version="0.1.0", languages=[Bcp47LanguageTag("en")]
27+
short_name="short_name",
28+
author="pytest",
29+
api_version="0.1",
30+
version="0.1.0",
31+
name={Bcp47LanguageTag("en"): "Test Package"},
32+
languages=[Bcp47LanguageTag("en")],
2833
)
2934
with (source / PACKAGE_CONFIG_FILENAME).open("w") as file:
3035
yaml.dump(config.model_dump(exclude={"type"}), file)

tests/questionpy_sdk/package/test_validate.py

Lines changed: 84 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@
66
from _pytest.logging import LogCaptureFixture
77

88
from questionpy_common.constants import MANIFEST_FILENAME
9-
from questionpy_common.manifest import Manifest, PartialPackagePermissions
9+
from questionpy_common.manifest import Bcp47LanguageTag, Manifest, PartialPackagePermissions
1010
from questionpy_sdk._package import DirBuildTarget, build_qpy_package
11-
from questionpy_sdk._package._validate import validate_dist_structure, validate_requested_lms_attributes
11+
from questionpy_sdk._package._validate import (
12+
validate_dist_structure,
13+
validate_package_name_and_description,
14+
validate_requested_lms_attributes,
15+
)
1216
from questionpy_sdk._package.source import PackageSource
1317

1418

@@ -24,10 +28,13 @@ def manifest(dist_dir: Path) -> Manifest:
2428
return Manifest.model_validate_json((dist_dir / MANIFEST_FILENAME).read_text())
2529

2630

31+
def get_warnings(caplog: LogCaptureFixture) -> list[str]:
32+
return [record.message for record in caplog.records if record.levelno == logging.WARNING]
33+
34+
2735
def test_should_not_warn_for_valid_package(dist_dir: Path, manifest: Manifest, caplog: LogCaptureFixture) -> None:
2836
validate_dist_structure(manifest, dist_dir)
29-
for record in caplog.records:
30-
assert record.levelno < logging.WARNING
37+
assert len(get_warnings(caplog)) == 0
3138

3239

3340
def test_warn_when_dist_contains_another_python_package(
@@ -38,7 +45,7 @@ def test_warn_when_dist_contains_another_python_package(
3845

3946
validate_dist_structure(manifest, dist_dir)
4047

41-
warn_messages = [record.message for record in caplog.records if record.levelno == logging.WARNING]
48+
warn_messages = get_warnings(caplog)
4249
assert len(warn_messages) == 1
4350
assert "- python/another/package\n" in warn_messages[0]
4451
assert "- python/another/package/__init__.py\n" in warn_messages[0]
@@ -52,7 +59,7 @@ def test_warn_when_dist_contains_wrong_js_dir(dist_dir: Path, manifest: Manifest
5259

5360
validate_dist_structure(manifest, dist_dir)
5461

55-
warn_messages = [record.message for record in caplog.records if record.levelno == logging.WARNING]
62+
warn_messages = get_warnings(caplog)
5663
assert len(warn_messages) == 1
5764
assert "- js\n" in warn_messages[0]
5865
assert "- js/bundle.js\n" in warn_messages[0]
@@ -68,22 +75,88 @@ def test_warn_when_dist_is_missing_python_dir(dist_dir: Path, manifest: Manifest
6875

6976
def test_should_not_warn_when_requesting_no_lms_attributes(manifest: Manifest, caplog: LogCaptureFixture) -> None:
7077
validate_requested_lms_attributes(manifest)
71-
for record in caplog.records:
72-
assert record.levelno < logging.WARNING
78+
79+
assert len(get_warnings(caplog)) == 0
7380

7481

7582
def test_should_not_warn_when_requesting_known_lms_attributes(manifest: Manifest, caplog: LogCaptureFixture) -> None:
7683
manifest.permissions = PartialPackagePermissions(lms_attributes={"attempt_id", "lms_sdk_xyz", "profile_field_xyz"})
84+
7785
validate_requested_lms_attributes(manifest)
78-
for record in caplog.records:
79-
assert record.levelno < logging.WARNING
86+
87+
assert len(get_warnings(caplog)) == 0
8088

8189

8290
def test_warn_when_requesting_unknown_lms_attributes(manifest: Manifest, caplog: LogCaptureFixture) -> None:
8391
manifest.permissions = PartialPackagePermissions(lms_attributes={"foo", "bar"})
92+
8493
validate_requested_lms_attributes(manifest)
8594

86-
warn_messages = [record.message for record in caplog.records if record.levelno == logging.WARNING]
95+
warn_messages = get_warnings(caplog)
8796
assert len(warn_messages) == 1
8897
assert "- foo" in warn_messages[0]
8998
assert "- bar" in warn_messages[0]
99+
100+
101+
def test_warn_when_name_translations_are_missing(manifest: Manifest, caplog: LogCaptureFixture) -> None:
102+
manifest.languages = [Bcp47LanguageTag("en"), Bcp47LanguageTag("de")]
103+
manifest.name = {Bcp47LanguageTag("en"): "Test Name"}
104+
105+
validate_package_name_and_description(manifest)
106+
107+
warn_messages = get_warnings(caplog)
108+
assert len(warn_messages) == 1
109+
assert "- de" in warn_messages[0]
110+
assert "- en" not in warn_messages[0]
111+
112+
113+
def test_warn_when_description_translations_are_missing(manifest: Manifest, caplog: LogCaptureFixture) -> None:
114+
manifest.languages = [Bcp47LanguageTag("en"), Bcp47LanguageTag("de")]
115+
manifest.name = {Bcp47LanguageTag("en"): "Test Name", Bcp47LanguageTag("de"): "Test Name"}
116+
manifest.description = {Bcp47LanguageTag("en"): "Test Name"}
117+
118+
validate_package_name_and_description(manifest)
119+
120+
warn_messages = get_warnings(caplog)
121+
assert len(warn_messages) == 1
122+
assert "- de" in warn_messages[0]
123+
assert "- en" not in warn_messages[0]
124+
125+
126+
def test_warn_when_name_translation_is_give_in_missing_language(manifest: Manifest, caplog: LogCaptureFixture) -> None:
127+
manifest.languages = [Bcp47LanguageTag("en")]
128+
manifest.name = {Bcp47LanguageTag("en"): "Test Name", Bcp47LanguageTag("de"): "Test Name"}
129+
130+
validate_package_name_and_description(manifest)
131+
132+
warn_messages = get_warnings(caplog)
133+
assert len(warn_messages) == 1
134+
assert "- de" in warn_messages[0]
135+
assert "- en" not in warn_messages[0]
136+
137+
138+
def test_warn_when_description_translation_is_give_in_missing_language(
139+
manifest: Manifest, caplog: LogCaptureFixture
140+
) -> None:
141+
manifest.languages = [Bcp47LanguageTag("en")]
142+
manifest.name = {Bcp47LanguageTag("en"): "Test Name"}
143+
manifest.description = {Bcp47LanguageTag("en"): "Test Description", Bcp47LanguageTag("de"): "Test Beschreibung"}
144+
145+
validate_package_name_and_description(manifest)
146+
147+
warn_messages = get_warnings(caplog)
148+
assert len(warn_messages) == 1
149+
assert "- de" in warn_messages[0]
150+
assert "- en" not in warn_messages[0]
151+
152+
153+
def test_warn_when_description_is_given_but_not_in_english(manifest: Manifest, caplog: LogCaptureFixture) -> None:
154+
manifest.languages = [Bcp47LanguageTag("en"), Bcp47LanguageTag("de")]
155+
manifest.name = {Bcp47LanguageTag("en"): "Test Name", Bcp47LanguageTag("de"): "Test Name"}
156+
manifest.description = {Bcp47LanguageTag("de"): "Beschreibung"}
157+
158+
validate_package_name_and_description(manifest)
159+
160+
warn_messages = get_warnings(caplog)
161+
assert len(warn_messages) == 1
162+
assert "description should be available in English" in warn_messages[0]

tests/questionpy_sdk/test_models.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,15 @@ def test_package_config_strips_config_fields() -> None:
1313
api_version="0.1",
1414
author="John Doe",
1515
build_hooks={"pre": "npm start"},
16+
name={Bcp47LanguageTag("en"): "Test Package"},
1617
languages=[Bcp47LanguageTag("en")],
1718
)
1819
exp = SourceManifest(
19-
short_name="foo", version="0.0.1", api_version="0.1", author="John Doe", languages=[Bcp47LanguageTag("en")]
20+
short_name="foo",
21+
version="0.0.1",
22+
api_version="0.1",
23+
author="John Doe",
24+
name={Bcp47LanguageTag("en"): "Test Package"},
25+
languages=[Bcp47LanguageTag("en")],
2026
)
2127
assert dict(config.to_manifest()) == dict(exp)

tests/questionpy_sdk/webserver/conftest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ def mock_worker() -> AsyncMock:
1717
version="7.3.1",
1818
api_version="9.4",
1919
author="Testy McTestface",
20+
name={Bcp47LanguageTag("en"): "Test Package"},
2021
languages=[Bcp47LanguageTag("en")],
2122
)
2223
return AsyncMock(get_manifest=AsyncMock(return_value=manifest))

tests/questionpy_sdk/webserver/controllers/test_manifest.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import pytest
88

9-
from questionpy import Manifest
9+
from questionpy_common.manifest import Bcp47LanguageTag, Manifest
1010
from questionpy_sdk.webserver.controllers.manifest import ManifestController
1111

1212

@@ -21,7 +21,8 @@ def test_get_manifest(controller: ManifestController, mock_webserver: Mock) -> N
2121
version="0.0.1",
2222
api_version="0.1",
2323
author="Jane Doe <[email protected]>",
24-
languages=["de", "en"],
24+
name={Bcp47LanguageTag("en"): "Test Package"},
25+
languages=[Bcp47LanguageTag("de"), Bcp47LanguageTag("en")],
2526
)
2627
mock_webserver.manifest = test_manifest
2728
manifest = controller.get_manifest()

tests/questionpy_sdk/webserver/routes/test_manifest.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from aiohttp.test_utils import TestClient
99
from aiohttp.web_exceptions import HTTPOk
1010

11-
from questionpy_common.manifest import Manifest
11+
from questionpy_common.manifest import Bcp47LanguageTag, Manifest
1212
from questionpy_sdk.webserver.routes.manifest import routes
1313

1414

@@ -20,7 +20,8 @@ async def test_get_manifest(client: TestClient, mock_controller: AsyncMock) -> N
2020
version="0.0.1",
2121
api_version="0.1",
2222
author="Jane Doe <[email protected]>",
23-
languages=["de", "en"],
23+
name={Bcp47LanguageTag("en"): "Test Package"},
24+
languages=[Bcp47LanguageTag("de"), Bcp47LanguageTag("en")],
2425
)
2526

2627
async with client.get("/manifest") as resp:

0 commit comments

Comments
 (0)