* feat: Enforce type hints for content_tagging app in edx-platform (#33484)

This commit is contained in:
Pooja Kulkarni
2023-10-16 15:25:59 -04:00
committed by GitHub
parent 6b082c728f
commit 9bf042b239
10 changed files with 66 additions and 19 deletions

View File

@@ -2,7 +2,6 @@
follow_imports = silent
ignore_missing_imports = True
allow_untyped_globals = True
exclude = tests
plugins =
mypy_django_plugin.main,
mypy_drf_plugin.main
@@ -11,8 +10,53 @@ files =
openedx/core/djangoapps/content_staging,
openedx/core/djangoapps/content_libraries,
openedx/core/djangoapps/xblock,
openedx/core/types
openedx/core/types,
openedx/core/djangoapps/content_tagging
[mypy.plugins.django-stubs]
# content_staging only works with CMS; others work with either, so we run mypy with CMS settings.
django_settings_module = "cms.envs.test"
# Selectively ignore packages known to be lacking type hints
[mypy-bridgekeeper.*]
ignore_missing_imports = True
[mypy-celery.*]
ignore_missing_imports = True
[mypy-celery_utils.*]
ignore_missing_imports = True
[mypy-completion.*]
ignore_missing_imports = True
[mypy-crum.*]
ignore_missing_imports = True
[mypy-ddt.*]
ignore_missing_imports = True
[mypy-edx_api_doc_tools.*]
ignore_missing_imports = True
[mypy-edx_django_utils.*]
ignore_missing_imports = True
[mypy-edx_proctoring.*]
ignore_missing_imports = True
[mypy-edx_rest_api_client.*]
ignore_missing_imports = True
[mypy-edx_rest_framework_extensions.*]
ignore_missing_imports = True
[mypy-eventtracking.*]
ignore_missing_imports = True
[mypy-fs.*]
ignore_missing_imports = True
[mypy-model_utils.*]
ignore_missing_imports = True
[mypy-openedx_events.*]
ignore_missing_imports = True
[mypy-organizations.*]
ignore_missing_imports = True
[mypy-search.*]
ignore_missing_imports = True
[mypy-rules.*]
ignore_missing_imports = True
[mypy-web_fragments.*]
ignore_missing_imports = True
[mypy-webob.*]
ignore_missing_imports = True
[mypy-xblock.*]
ignore_missing_imports = True

View File

@@ -546,14 +546,14 @@ class ContentLibraryXBlockUserStateTestMixin(ContentLibraryContentTestMixin):
@requires_blockstore
class ContentLibraryXBlockUserStateBServiceTest(ContentLibraryXBlockUserStateTestMixin, TestCase):
class ContentLibraryXBlockUserStateBServiceTest(ContentLibraryXBlockUserStateTestMixin, TestCase): # type: ignore[misc]
"""
Tests XBlock user state for XBlocks in a content library using the standalone Blockstore service.
"""
@requires_blockstore_app
class ContentLibraryXBlockUserStateTest(
class ContentLibraryXBlockUserStateTest( # type: ignore[misc]
ContentLibraryXBlockUserStateTestMixin,
BlockstoreAppTestMixin,
LiveServerTestCase,

View File

@@ -364,7 +364,7 @@ class ClipboardTestCase(ModuleStoreTestCase):
response = nonstaff_client.get(olx_url)
assert response.status_code == 403
def assertXmlEqual(self, xml_str_a: str, xml_str_b: str) -> bool:
def assertXmlEqual(self, xml_str_a: str, xml_str_b: str):
""" Assert that the given XML strings are equal, ignoring attribute order and some whitespace variations. """
a = ElementTree.canonicalize(xml_str_a, strip_text=True)
b = ElementTree.canonicalize(xml_str_b, strip_text=True)

View File

@@ -16,7 +16,7 @@ from .types import ContentKey
def create_taxonomy(
name: str,
description: str = None,
description: str | None = None,
enabled=True,
allow_multiple=False,
allow_free_text=False,
@@ -36,7 +36,7 @@ def create_taxonomy(
def set_taxonomy_orgs(
taxonomy: Taxonomy,
all_orgs=False,
orgs: list[Organization] = None,
orgs: list[Organization] | None = None,
relationship: TaxonomyOrg.RelType = TaxonomyOrg.RelType.OWNER,
):
"""
@@ -102,7 +102,7 @@ def get_taxonomies_for_org(
def get_content_tags(
object_key: ContentKey,
taxonomy_id: str | None = None,
taxonomy_id: int | None = None,
) -> Iterator[ContentObjectTag]:
"""
Generates a list of content tags for a given object.
@@ -120,7 +120,7 @@ def tag_content_object(
object_key: ContentKey,
taxonomy: Taxonomy,
tags: list,
) -> list[ContentObjectTag]:
) -> Iterator[ContentObjectTag]:
"""
This is the main API to use when you want to add/update/delete tags from a content object (e.g. an XBlock or
course).
@@ -138,7 +138,7 @@ def tag_content_object(
"""
if not taxonomy.system_defined:
# We require that this taxonomy is linked to the content object's "org" or linked to "all orgs" (None):
org_short_name = object_key.org
org_short_name = object_key.org # type: ignore
if not taxonomy.taxonomyorg_set.filter(Q(org__short_name=org_short_name) | Q(org=None)).exists():
raise ValueError(f"The specified Taxonomy is not enabled for the content object's org ({org_short_name})")
oel_tagging.tag_object(
@@ -147,7 +147,7 @@ def tag_content_object(
object_id=str(object_key),
object_tag_class=ContentObjectTag,
)
return get_content_tags(str(object_key), taxonomy_id=taxonomy.id)
return get_content_tags(object_key, taxonomy_id=taxonomy.id)
# Expose the oel_tagging APIs

View File

@@ -2,7 +2,7 @@
API Serializers for content tagging org
"""
from rest_framework import serializers
from rest_framework import serializers, fields
from openedx_tagging.core.tagging.rest_api.v1.serializers import (
TaxonomyListQueryParamsSerializer,
@@ -16,7 +16,7 @@ class TaxonomyOrgListQueryParamsSerializer(TaxonomyListQueryParamsSerializer):
Serializer for the query params for the GET view
"""
org = serializers.SlugRelatedField(
org: fields.Field = serializers.SlugRelatedField(
slug_field="short_name",
queryset=Organization.objects.all(),
required=False,

View File

@@ -6,7 +6,7 @@ from urllib.parse import parse_qs, urlparse
import ddt
from django.contrib.auth import get_user_model
from django.test.testcases import override_settings
from django.test import override_settings
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
from openedx_tagging.core.tagging.models import Tag, Taxonomy
from openedx_tagging.core.tagging.models.system_defined import SystemDefinedTaxonomy

View File

@@ -35,7 +35,7 @@ def _set_initial_language_tag(content_key: ContentKey, lang_code: str) -> None:
"""
lang_taxonomy = Taxonomy.objects.get(pk=LANGUAGE_TAXONOMY_ID).cast()
if lang_code and not api.get_content_tags(object_key=content_key, taxonomy_id=lang_taxonomy.id).exists():
if lang_code and not api.get_content_tags(object_key=content_key, taxonomy_id=lang_taxonomy.id):
try:
lang_tag = lang_taxonomy.tag_for_external_id(lang_code)
except api.oel_tagging.TagDoesNotExist:

View File

@@ -2,7 +2,7 @@
import ddt
from django.contrib.auth import get_user_model
from django.test.testcases import TestCase, override_settings
from django.test import TestCase, override_settings
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
from openedx_tagging.core.tagging.models import (
Tag,

View File

@@ -51,7 +51,7 @@ class LanguageTaxonomyTestMixin:
@skip_unless_cms # Auto-tagging is only available in the CMS
@override_waffle_flag(CONTENT_TAGGING_AUTO, active=True)
class TestAutoTagging(LanguageTaxonomyTestMixin, ModuleStoreTestCase):
class TestAutoTagging(LanguageTaxonomyTestMixin, ModuleStoreTestCase): # type: ignore[misc]
"""
Test if the Course and XBlock tags are automatically created
"""
@@ -64,7 +64,7 @@ class TestAutoTagging(LanguageTaxonomyTestMixin, ModuleStoreTestCase):
If value is None, check if the ObjectTag does not exists
"""
object_tags = api.get_content_tags(object_key, taxonomy_id=taxonomy_id)
object_tags = list(api.get_content_tags(object_key, taxonomy_id=taxonomy_id))
object_tag = object_tags[0] if len(object_tags) == 1 else None
if len(object_tags) > 1:
raise ValueError("Found too many object tags")

View File

@@ -136,6 +136,7 @@ def test_secure_token(param_delta: dict, expected_validation: bool):
SECRET_KEY=params["generation_secret_key"],
XBLOCK_HANDLER_TOKEN_KEYS=params["generation_xblock_handler_token_keys"],
):
assert isinstance(reference_time, datetime.datetime)
with freeze_time(reference_time):
token = get_secure_token_for_xblock_handler(
params["generation_user_id"], params["generation_block_key_str"]
@@ -145,7 +146,9 @@ def test_secure_token(param_delta: dict, expected_validation: bool):
SECRET_KEY=params["validation_secret_key"],
XBLOCK_HANDLER_TOKEN_KEYS=params["validation_xblock_handler_token_keys"],
):
with freeze_time(reference_time + datetime.timedelta(seconds=params["validation_time_delta_s"])):
assert isinstance(params["validation_time_delta_s"], int)
new_reference_time = reference_time + datetime.timedelta(seconds=float(params["validation_time_delta_s"]))
with freeze_time(new_reference_time):
assert (
validate_secure_token_for_xblock_handler(
params["validation_user_id"], params["validation_block_key_str"], token