System defined taxonomies

MANIFEST.in

@@ -3,4 +3,4 @@ include LICENSE.txt
 include README.rst
 include requirements/base.in
 recursive-include openedx_learning *.html *.png *.gif *.js *.css *.jpg *.jpeg *.svg *.py
-recursive-include openedx_tagging *.html *.png *.gif *.js *.css *.jpg *.jpeg *.svg *.py
+recursive-include openedx_tagging *.html *.png *.gif *.js *.css *.jpg *.jpeg *.svg *.py *.yaml

docs/decisions/0007-tagging-app.rst

@@ -19,15 +19,15 @@ Taxonomy
 
 The ``openedx_tagging`` module defines ``openedx_tagging.core.models.Taxonomy``, whose data and functionality are self-contained to the ``openedx_tagging`` app. However in Studio, we need to be able to limit access to some Taxonomy by organization, using the same "course creator" access which limits course creation for an organization to a defined set of users.
 
-So in edx-platform, we will create the ``openedx.features.tagging`` app, to contain ``models.OrgTaxonomy``. OrgTaxonomy subclasses ``openedx_tagging.core.models.Taxonomy``, employing Django's `multi-table inheritance`_ feature, which allows the base Tag class to keep foreign keys to the Taxonomy, while allowing OrgTaxonomy to store foreign keys into Studio's Organization table.
+So in edx-platform, we will create the ``openedx.features.content_tagging`` app, to contain the models and logic for linking Organization owners to Taxonomies. Here, we can subclass ``Taxonomy`` as needed, preferably using proxy models. The APIs are responsible for ensuring that any ``Taxonomy`` instances are cast to the appropriate subclass.
 
 ObjectTag
 ~~~~~~~~~
 
 Similarly, the ``openedx_tagging`` module defined ``openedx_tagging.core.models.ObjectTag``, also self-contained to the
 ``openedx_tagging`` app.
 
-But to tag content in the LMS/Studio, we create ``openedx.features.tagging.models.ContentTag``, which subclasses ``ObjectTag``, and can then reference functionality available in the platform code.
+But to tag content in the LMS/Studio, we need to enforce ``object_id`` as a CourseKey or UsageKey type. So to do this, we subclass ``ObjectTag``, and use this class when creating content object tags. Once the ``object_id`` is set, it is not editable, and so this key validation need not happen again.
 
 Rejected Alternatives
 ---------------------
@@ -38,6 +38,3 @@ Embed in edx-platform
 Embedding the logic in edx-platform would provide the content tagging logic specifically required for the MVP.
 
 However, we plan to extend tagging to other object types (e.g. People) and contexts (e.g. Marketing), and so a generic, standalone library is preferable in the log run.
-
-
-.. _multi-table inheritance: https://docs.djangoproject.com/en/3.2/topics/db/models/#multi-table-inheritance

docs/decisions/0009-tagging-administrators.rst

@@ -22,7 +22,7 @@ In the Studio context, a modified version of "course creator" access will be use
 
 Permission #1 requires no external access, so can be enforced by the ``openedx_tagging`` app.
 
-But because permissions #2 + #3 require access to the edx-platform CMS model `CourseCreator`_, this access can only be enforced in Studio, and so will live under `cms.djangoapps.tagging` along with the ``ContentTag`` class. Tagging MVP must work for libraries v1, v2 and courses created in Studio, and so tying these permissions to Studio is reasonable for the MVP.
+But because permissions #2 + #3 require access to the edx-platform CMS model `CourseCreator`_, this access can only be enforced in Studio, and so will live under ``cms.djangoapps.content_tagging`` along with the ``ContentTag`` class. Tagging MVP must work for libraries v1, v2 and courses created in Studio, and so tying these permissions to Studio is reasonable for the MVP.
 
 Per `OEP-9`_, ``openedx_tagging`` will allow applications to use the standard Django API to query permissions, for example: ``user.has_perm('openedx_tagging.edit_taxonomy', taxonomy)``, and the appropriate permissions will be applied in that application's context.
 

docs/decisions/0012-system-taxonomy-creation.rst

@@ -4,25 +4,24 @@
 Context
 --------
 
-System-defined taxonomies are closed taxonomies created by the system. Some of these are totally static (e.g Language)
+System-defined taxonomies are taxonomies created by the system. Some of these are totally static (e.g Language)
 and some depends on a core data model (e.g. Organizations). It is necessary to define how to create and validate 
 the System-defined taxonomies and their tags.
 
 
 Decision
 ---------
 
-System-defined Taxonomy creation
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+System Tag lists and validation
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Each System-defined Taxonomy has its own class, which is used for tag validation (e.g. ``LanguageSystemTaxonomy``, ``OrganizationSystemTaxonomy``).
-Each can overwrite ``get_tags``; to configure the valid tags, and ``validate_object_tag``; to check if a list of tags are valid.
-Both functions are implemented on the ``Taxonomy`` base class, but can be overwritten to handle special cases.
+Each System-defined Taxonomy will have its own ``ObjectTag`` subclass which is used for tag validation (e.g. ``LanguageObjectTag``, ``OrganizationObjectTag``).
+Each subclass can overwrite ``get_tags``; to configure the valid tags, and ``is_valid``; to check if a list of tags are valid.  Both functions are implemented on the ``ObjectTag`` base class, but can be overwritten to handle special cases.
 
-We need to create an instance of each System-defined Taxonomy in a fixture. This instances will be used on different APIs.
+We need to create an instance of each System-defined Taxonomy in a fixture. With their respective characteristics and subclasses.
+The ``pk`` of these instances must be negative so as not to affect the auto-incremented ``pk`` of Taxonomies.
 
-Later, we need to create a ``Content-side`` class that lives on ``openedx.features.tagging`` for each content and taxonomy to be used 
-(eg. ``CourseLanguageSystemTaxonomy``, ``CourseOrganizationSystemTaxonomy``).
+Later, we need to create content-side ObjectTags that live on ``openedx.features.content_tagging`` for each content and taxonomy to be used (eg. ``CourseLanguageObjectTag``, ``CourseOrganizationObjectTag``).
 This new class is used to configure the automatic content tagging. You can read the `document number 0013`_ to see this configuration.
 
 Tags creation
@@ -32,27 +31,28 @@ We have two ways to handle Tags creation and validation for System-defined Taxon
 
 **Hardcoded by fixtures/migrations**
 
-#. If the tags don't change over the time, you can create all on a fixture (e.g Languages). 
+#. If the tags don't change over the time, you can create all on a fixture (e.g Languages).
+   The ``pk`` of these instances must be negative.
 #. If the tags change over the time, you can create all on a migration. If you edit, delete, or add new tags, you should also do it in a migration.
 
-**Free-form tags**
+**Dynamic tags**
 
-This taxonomy depends on a core data model, but simplifies the creation of Tags by allowing free-form tags,
-but we can validate the tags using the ``validate_object_tag`` method. For example we can put the ``AuthorSystemTaxonomy`` associated with
-the ``User`` model and use the ``ID`` field as tags. Also we can validate if an ``User`` still exists or has been deleted over time.
+Closed Taxonomies that depends on a core data model. Ex. AuthorTaxonomy with Users as Tags
+
+#. Tags are created on the fly when new ObjectTags are added.
+#. Tag.external_id we store an identifier from the instance (eg. User.pk).
+#. Tag.value we store a human readable representation of the instance (eg. User.username).
+#. Resync the tags to re-fetch the value. 
 
 
 Rejected Options
 -----------------
 
-Tags created by Auto-generated from the codebase
+Free-form tags
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Taxonomies that depend on a core data model could create a Tag for each eligible object. 
-Maintaining this dynamic list of available Tags is cumbersome: we'd need triggers for creation, editing, and deletion.
-And if it's a large list of objects (e.g. Users), then copying that list into the Tag table is overkill.
-It is better to dynamically generate the list of available Tags, and/or dynamically validate a submitted object tag than
-to store the options in the database.
+Open Taxonomy that depends on a core data model, but simplifies the creation of Tags by allowing free-form tags,
 
+Rejected because it has been seen that using dynamic tags provides more functionality and more advantages.
 
 .. _document number 0013: https://github.com/openedx/openedx-learning/blob/main/docs/decisions/0013-system-taxonomy-auto-tagging.rst

openedx_learning/core/contents/migrations/0001_initial.py

@@ -7,20 +7,6 @@
 import openedx_learning.lib.validators
 
 
-def use_compressed_table_format(apps, schema_editor):
-    """
-    Use the COMPRESSED row format for TextContent if we're using MySQL.
-
-    This table will hold a lot of OLX, which compresses very well using MySQL's
-    built-in zlib compression. This is especially important because we're
-    keeping so much version history.
-    """
-    if schema_editor.connection.vendor == 'mysql':
-        table_name = apps.get_model("oel_contents", "TextContent")._meta.db_table
-        sql = f"ALTER TABLE {table_name} ROW_FORMAT=COMPRESSED;"
-        schema_editor.execute(sql)
-
-
 class Migration(migrations.Migration):
 
     initial = True
@@ -55,7 +41,6 @@ class Migration(migrations.Migration):
             ],
         ),
         # Call out to custom code here to change row format for TextContent
-        migrations.RunPython(use_compressed_table_format, reverse_code=migrations.RunPython.noop, atomic=False),
         migrations.AddIndex(
             model_name='rawcontent',
             index=models.Index(fields=['learning_package', 'mime_type'], name='oel_content_idx_lp_mime_type'),

openedx_tagging/core/tagging/admin.py

@@ -1,8 +1,99 @@
 """ Tagging app admin """
+from django import forms
 from django.contrib import admin
 
 from .models import ObjectTag, Tag, Taxonomy
 
-admin.site.register(Taxonomy)
-admin.site.register(Tag)
+
+def check_taxonomy(taxonomy: Taxonomy):
+    """
+    Checks if the taxonomy is valid to edit or delete
+    """
+    taxonomy = taxonomy.cast()
+    return not taxonomy.system_defined
+
+
+class TaxonomyAdmin(admin.ModelAdmin):
+    """
+    Admin for Taxonomy Model
+    """
+
+    def has_change_permission(self, request, obj=None):
+        """
+        Avoid edit system-defined taxonomies
+        """
+        if obj is not None:
+            return check_taxonomy(taxonomy=obj)
+        return super().has_change_permission(request, obj)
+
+    def has_delete_permission(self, request, obj=None):
+        """
+        Avoid delete system-defined taxonomies
+        """
+        if obj is not None:
+            return check_taxonomy(taxonomy=obj)
+        return super().has_change_permission(request, obj)
+
+
+class TagForm(forms.ModelForm):
+    """
+    Form for create a Tag
+    """
+    class Meta:
+        model = Tag
+        fields = '__all__'
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        print(self.fields)
+        if 'taxonomy' in self.fields:
+            self.fields['taxonomy'].queryset = self._filter_taxonomies()
+
+    def _filter_taxonomies(self):
+        """
+        Returns taxonomies that allows Tag creation
+
+        - Not allow free text
+        - Not system defined
+        """
+        taxonomy_queryset = Taxonomy.objects.filter(
+            allow_free_text=False
+        )
+        valid_taxonomy_ids = [
+            taxonomy.id for taxonomy 
+            in taxonomy_queryset if check_taxonomy(taxonomy)
+        ]
+
+        return taxonomy_queryset.filter(id__in=valid_taxonomy_ids)
+
+
+class TagAdmin(admin.ModelAdmin):
+    """
+    Admin for Tag Model
+    """
+    form = TagForm
+
+    def has_change_permission(self, request, obj=None):
+        """
+        Avoid edit system-defined taxonomies
+        """
+        if obj is not None:
+            taxonomy = obj.taxonomy
+            if taxonomy:
+                return check_taxonomy(taxonomy)
+        return super().has_change_permission(request, obj)
+
+    def has_delete_permission(self, request, obj=None):
+        """
+        Avoid delete system-defined taxonomies
+        """
+        if obj is not None:
+            taxonomy = obj.taxonomy
+            if taxonomy:
+                return check_taxonomy(taxonomy)
+        return super().has_change_permission(request, obj)
+
+
+admin.site.register(Taxonomy, TaxonomyAdmin)
+admin.site.register(Tag, TagAdmin)
 admin.site.register(ObjectTag)

openedx_tagging/core/tagging/api.py

@@ -10,7 +10,7 @@
 Please look at the models.py file for more information about the kinds of data
 are stored in this app.
 """
-from typing import List, Type
+from typing import Iterator, List, Type, Union
 
 from django.db.models import QuerySet
 from django.utils.translation import gettext_lazy as _
@@ -19,29 +19,46 @@
 
 
 def create_taxonomy(
-    name,
-    description=None,
+    name: str,
+    description: str = None,
     enabled=True,
     required=False,
     allow_multiple=False,
     allow_free_text=False,
+    taxonomy_class: Type = None,
 ) -> Taxonomy:
     """
     Creates, saves, and returns a new Taxonomy with the given attributes.
     """
-    return Taxonomy.objects.create(
+    taxonomy = Taxonomy(
         name=name,
         description=description,
         enabled=enabled,
         required=required,
         allow_multiple=allow_multiple,
         allow_free_text=allow_free_text,
     )
+    if taxonomy_class:
+        taxonomy.taxonomy_class = taxonomy_class
+    taxonomy.save()
+    return taxonomy.cast()
+
+
+def get_taxonomy(id: int) -> Union[Taxonomy, None]:
+    """
+    Returns a Taxonomy cast to the appropriate subclass which has the given ID.
+    """
+    taxonomy = Taxonomy.objects.filter(id=id).first()
+    return taxonomy.cast() if taxonomy else None
 
 
 def get_taxonomies(enabled=True) -> QuerySet:
     """
     Returns a queryset containing the enabled taxonomies, sorted by name.
+
+    We return a QuerySet here for ease of use with Django Rest Framework and other query-based use cases.
+    So be sure to use `Taxonomy.cast()` to cast these instances to the appropriate subclass before use.
+
     If you want the disabled taxonomies, pass enabled=False.
     If you want all taxonomies (both enabled and disabled), pass enabled=None.
     """
@@ -57,7 +74,7 @@ def get_tags(taxonomy: Taxonomy) -> List[Tag]:
 
     Note that if the taxonomy allows free-text tags, then the returned list will be empty.
     """
-    return taxonomy.get_tags()
+    return taxonomy.cast().get_tags()
 
 
 def resync_object_tags(object_tags: QuerySet = None) -> int:
@@ -67,7 +84,7 @@ def resync_object_tags(object_tags: QuerySet = None) -> int:
     By default, we iterate over all ObjectTags. Pass a filtered ObjectTags queryset to limit which tags are resynced.
     """
     if not object_tags:
-        object_tags = ObjectTag.objects.all()
+        object_tags = ObjectTag.objects.select_related("tag", "taxonomy")
 
     num_changed = 0
     for object_tag in object_tags:
@@ -79,22 +96,36 @@ def resync_object_tags(object_tags: QuerySet = None) -> int:
 
 
 def get_object_tags(
-    taxonomy: Taxonomy, object_id: str, object_type: str, valid_only=True
-) -> List[ObjectTag]:
+    object_id: str, taxonomy: Taxonomy = None, valid_only=True
+) -> Iterator[ObjectTag]:
     """
-    Returns a list of tags for a given taxonomy + content.
+    Generates a list of object tags for a given object.
+
+    Pass taxonomy to limit the returned object_tags to a specific taxonomy.
 
     Pass valid_only=False when displaying tags to content authors, so they can see invalid tags too.
-    Invalid tags will likely be hidden from learners.
+    Invalid tags will (probably) be hidden from learners.
     """
-    tags = ObjectTag.objects.filter(
-        taxonomy=taxonomy, object_id=object_id, object_type=object_type
-    ).order_by("id")
-    return [tag for tag in tags if not valid_only or taxonomy.validate_object_tag(tag)]
+    ObjectTagClass = taxonomy.object_tag_class if taxonomy else ObjectTag
+    tags = (
+        ObjectTagClass.objects.filter(
+            object_id=object_id,
+        )
+        .select_related("tag", "taxonomy")
+        .order_by("id")
+    )
+    if taxonomy:
+        tags = tags.filter(taxonomy=taxonomy)
+
+    for object_tag in tags:
+        if not valid_only or object_tag.is_valid():
+            yield object_tag
 
 
 def tag_object(
-    taxonomy: Taxonomy, tags: List, object_id: str, object_type: str
+    taxonomy: Taxonomy,
+    tags: List,
+    object_id: str,
 ) -> List[ObjectTag]:
     """
     Replaces the existing ObjectTag entries for the given taxonomy + object_id with the given list of tags.
@@ -105,5 +136,4 @@ def tag_object(
     Raised ValueError if the proposed tags are invalid for this taxonomy.
     Preserves existing (valid) tags, adds new (valid) tags, and removes omitted (or invalid) tags.
     """
-
-    return taxonomy.tag_object(tags, object_id, object_type)
+    return taxonomy.cast().tag_object(tags, object_id)