Suggested changes for Content system taxonomies

openedx/features/content_tagging/migrations/0003_system_defined_taxonomies.py

@@ -1,12 +1,13 @@
-# Generated by Django 3.2.20 on 2023-07-22 18:30
+# Generated by Django 3.2.20 on 2023-07-24 01:58
 
 from django.db import migrations
+import openedx.features.content_tagging.models.base
 
 
 class Migration(migrations.Migration):
 
     dependencies = [
-        ('oel_tagging', '0003_auto_20230721_1238'),
+        ('oel_tagging', '__latest__'),
         ('content_tagging', '0002_system_defined_taxonomies'),
     ]
 
@@ -20,7 +21,7 @@ class Migration(migrations.Migration):
                 'indexes': [],
                 'constraints': [],
             },
-            bases=('oel_tagging.usersystemdefinedtaxonomy',),
+            bases=(openedx.features.content_tagging.models.base.ContentTaxonomyMixin, 'oel_tagging.usersystemdefinedtaxonomy'),
         ),
         migrations.CreateModel(
             name='ContentLanguageTaxonomy',
@@ -31,29 +32,29 @@ class Migration(migrations.Migration):
                 'indexes': [],
                 'constraints': [],
             },
-            bases=('oel_tagging.languagetaxonomy',),
+            bases=(openedx.features.content_tagging.models.base.ContentTaxonomyMixin, 'oel_tagging.languagetaxonomy'),
         ),
         migrations.CreateModel(
-            name='OrganizarionSystemDefinedTaxonomy',
+            name='OrganizationModelObjectTag',
             fields=[
             ],
             options={
                 'proxy': True,
                 'indexes': [],
                 'constraints': [],
             },
-            bases=('oel_tagging.modelsystemdefinedtaxonomy',),
+            bases=('oel_tagging.modelobjecttag',),
         ),
         migrations.CreateModel(
-            name='OrganizationModelObjectTag',
+            name='OrganizationSystemDefinedTaxonomy',
             fields=[
             ],
             options={
                 'proxy': True,
                 'indexes': [],
                 'constraints': [],
             },
-            bases=('oel_tagging.modelobjecttag',),
+            bases=('oel_tagging.modelsystemdefinedtaxonomy',),
         ),
         migrations.CreateModel(
             name='ContentOrganizationTaxonomy',
@@ -64,6 +65,6 @@ class Migration(migrations.Migration):
                 'indexes': [],
                 'constraints': [],
             },
-            bases=('content_tagging.organizarionsystemdefinedtaxonomy',),
+            bases=(openedx.features.content_tagging.models.base.ContentTaxonomyMixin, 'content_tagging.organizationsystemdefinedtaxonomy'),
         ),
     ]

openedx/features/content_tagging/models/base.py

@@ -98,15 +98,12 @@ def object_key(self) -> OpaqueKey:
         return None
 
 
-class ContentTaxonomy(Taxonomy):
+class ContentTaxonomyMixin:
     """
     Taxonomy that accepts ContentTags,
     and ensures a valid TaxonomyOrg owner relationship with the content object.
     """
 
-    class Meta:
-        proxy = True
-
     def _check_object(self, object_tag: ObjectTag) -> bool:
         """
         Returns True if this ObjectTag has a valid object_id.
@@ -124,3 +121,13 @@ def _check_taxonomy(self, object_tag: ObjectTag) -> bool:
             and object_key
             and TaxonomyOrg.is_owner(self, object_key.org)
         )
+
+
+class ContentTaxonomy(ContentTaxonomyMixin, Taxonomy):
+    """
+    Taxonomy that accepts ContentTags,
+    and ensures a valid TaxonomyOrg owner relationship with the content object.
+    """
+
+    class Meta:
+        proxy = True

openedx/features/content_tagging/models/system_defined.py

@@ -4,20 +4,19 @@
 from typing import Type
 
 from openedx_tagging.core.tagging.models import (
-    ObjectTag,
     ModelSystemDefinedTaxonomy,
     ModelObjectTag,
     UserSystemDefinedTaxonomy,
     LanguageTaxonomy,
 )
 
 from organizations.models import Organization
-from .base import ContentTaxonomy
+from .base import ContentTaxonomyMixin
 
 
 class OrganizationModelObjectTag(ModelObjectTag):
     """
-    ObjectTags for the OrganizarionSystemDefinedTaxonomy.
+    ObjectTags for the OrganizationSystemDefinedTaxonomy.
     """
 
     class Meta:
@@ -38,7 +37,7 @@ def tag_class_value(self) -> str:
         return "name"
 
 
-class OrganizarionSystemDefinedTaxonomy(ModelSystemDefinedTaxonomy):
+class OrganizationSystemDefinedTaxonomy(ModelSystemDefinedTaxonomy):
     """
     Organization based system taxonomy class.
     """
@@ -54,61 +53,28 @@ def object_tag_class(self) -> Type:
         return OrganizationModelObjectTag
 
 
-class ContentLanguageTaxonomy(LanguageTaxonomy):
+class ContentLanguageTaxonomy(ContentTaxonomyMixin, LanguageTaxonomy):
     """
     Language system-defined taxonomy that accepts ContentTags
-
-    Inherit `_check_tag` from LanguageTaxonomy and uses
-    `_check_object` and `_check_taxonomy` from ContentTaxonomy
     """
 
     class Meta:
         proxy = True
 
-    def _check_object(self, object_tag: ObjectTag) -> bool:
-        taxonomy = ContentTaxonomy().copy(self)
-        return taxonomy._check_object(object_tag)  # pylint: disable=protected-access
-
-    def _check_taxonomy(self, object_tag: ObjectTag) -> bool:
-        taxonomy = ContentTaxonomy().copy(self)
-        return taxonomy._check_taxonomy(object_tag)  # pylint: disable=protected-access
-
 
-class ContentAuthorTaxonomy(UserSystemDefinedTaxonomy):
+class ContentAuthorTaxonomy(ContentTaxonomyMixin, UserSystemDefinedTaxonomy):
     """
     Author system-defined taxonomy that accepts Content Tags
-
-    Inherit `_check_tag` from UserSystemDefinedTaxonomy and uses
-    `_check_object` and `_check_taxonomy` from ContentTaxonomy
     """
 
     class Meta:
         proxy = True
 
-    def _check_object(self, object_tag: ObjectTag) -> bool:
-        taxonomy = ContentTaxonomy().copy(self)
-        return taxonomy._check_object(object_tag)  # pylint: disable=protected-access
-
-    def _check_taxonomy(self, object_tag: ObjectTag) -> bool:
-        taxonomy = ContentTaxonomy().copy(self)
-        return taxonomy._check_taxonomy(object_tag)  # pylint: disable=protected-access
 
-
-class ContentOrganizationTaxonomy(OrganizarionSystemDefinedTaxonomy):
+class ContentOrganizationTaxonomy(ContentTaxonomyMixin, OrganizationSystemDefinedTaxonomy):
     """
     Organization system-defined taxonomy that accepts Content Tags
-
-    Inherit `_check_tag` from OrganizarionSystemDefinedTaxonomy and uses
-    `_check_object` and `_check_taxonomy` from ContentTaxonomy
     """
 
     class Meta:
         proxy = True
-
-    def _check_object(self, object_tag: ObjectTag) -> bool:
-        taxonomy = ContentTaxonomy().copy(self)
-        return taxonomy._check_object(object_tag)  # pylint: disable=protected-access
-
-    def _check_taxonomy(self, object_tag: ObjectTag) -> bool:
-        taxonomy = ContentTaxonomy().copy(self)
-        return taxonomy._check_taxonomy(object_tag)  # pylint: disable=protected-access

openedx/features/content_tagging/tests/test_models.py

@@ -0,0 +1,71 @@
+"""
+Test for Content models
+"""
+import ddt
+from django.test.testcases import TestCase
+
+from openedx_tagging.core.tagging.models import (
+    ObjectTag,
+    Taxonomy,
+    Tag,
+)
+from ..models import (
+    ContentLanguageTaxonomy,
+    ContentAuthorTaxonomy,
+    ContentOrganizationTaxonomy,
+)
+
+
+@ddt.ddt
+class TestSystemDefinedModels(TestCase):
+    """
+    Test for System defined models
+    """
+
+    @ddt.data(
+        (ContentLanguageTaxonomy, "taxonomy"),  # Invalid object key
+        (ContentLanguageTaxonomy, "tag"),  # Invalid external_id, invalid language
+        (ContentLanguageTaxonomy, "object"),  # Invalid object key
+        (ContentAuthorTaxonomy, "taxonomy"),  # Invalid object key
+        (ContentAuthorTaxonomy, "tag"),  # Invalid external_id, User don't exits
+        (ContentAuthorTaxonomy, "object"),  # Invalid object key
+        (ContentOrganizationTaxonomy, "taxonomy"),  # Invalid object key
+        (ContentOrganizationTaxonomy, "tag"),  # Invalid external_id, Organization don't exits
+        (ContentOrganizationTaxonomy, "object"),  # Invalid object key
+    )
+    @ddt.unpack
+    def test_validations(
+        self,
+        taxonomy_cls,
+        check,
+    ):
+        """
+        Test that the respective validations are being called
+        """
+        taxonomy = Taxonomy(
+            name='Test taxonomy'
+        )
+        taxonomy.taxonomy_class = taxonomy_cls
+        taxonomy.save()
+        taxonomy = taxonomy.cast()
+        tag = Tag(
+            value="value",
+            external_id="external_id",
+        )
+        tag.taxonomy = taxonomy
+        tag.save()
+        object_tag = ObjectTag(
+            object_id='object_id',
+            taxonomy=taxonomy,
+            tag=tag,
+        )
+
+        check_taxonomy = check == 'taxonomy'
+        check_object = check == 'object'
+        check_tag = check == 'tag'
+        assert not taxonomy.validate_object_tag(
+            object_tag=object_tag,
+            check_taxonomy=check_taxonomy,
+            check_object=check_object,
+            check_tag=check_tag,
+        )