Skip to content

Commit

Permalink
Allow custom Taxonomy, ObjectTag subclasses to customize tagging beha…
Browse files Browse the repository at this point in the history
…vior (#62)

Adds support for custom Taxonomy subclasses to be stored against a Taxonomy, to be used by the python API when instantiating and returning Taxonomies.

Also adds minimal support for ObjectTag subclasses. However, these are not stored against the ObjectTag instances; they can be instantiated by the Taxonomy subclasses if and when needed.

Related:
* docs: updates decisions to reflect this change
* feat: adds api.get_taxonomy, which returns a Taxonomy cast to its subclass, when set
* refactor: adds _check_taxonomy, _check_tag, and _check_object methods to the Taxonomy class, which can be overridden by subclasses when validating ObjectTags

Added to support system-defined Taxonomies:
* feat: adds un-editable Taxonomy.system_defined field so that system taxonomies can store this field and ensure no one edits them. 
* feat: adds Taxonomy.visible_to_authors, which is needed for fully automated tagging.

Cleanup changes:
* fix: updates Tag model to cascade delete if the Taxonomy or parent Tag is deleted.
* style: adds missing type annotations to rules and python API
  • Loading branch information
pomegranited authored Jul 20, 2023
1 parent a5b109f commit f5164bb
Show file tree
Hide file tree
Showing 11 changed files with 695 additions and 181 deletions.
7 changes: 2 additions & 5 deletions docs/decisions/0007-tagging-app.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
---------------------
Expand All @@ -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
2 changes: 1 addition & 1 deletion docs/decisions/0009-tagging-administrators.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
17 changes: 7 additions & 10 deletions docs/decisions/0012-system-taxonomy-creation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,23 @@
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's ObjectTag in a fixture. This instances will be used on different APIs.

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
Expand Down Expand Up @@ -54,5 +52,4 @@ And if it's a large list of objects (e.g. Users), then copying that list into th
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.


.. _document number 0013: https://github.com/openedx/openedx-learning/blob/main/docs/decisions/0013-system-taxonomy-auto-tagging.rst
63 changes: 46 additions & 17 deletions openedx_tagging/core/tagging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 _
Expand All @@ -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.
"""
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -79,22 +96,35 @@ 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)]
tags = (
ObjectTag.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.
Expand All @@ -105,5 +135,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)
79 changes: 79 additions & 0 deletions openedx_tagging/core/tagging/migrations/0002_auto_20230718_2026.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Generated by Django 3.2.19 on 2023-07-18 05:54

import django.db.models.deletion
from django.db import migrations, models

import openedx_learning.lib.fields


class Migration(migrations.Migration):
dependencies = [
("oel_tagging", "0001_initial"),
]

operations = [
migrations.AddField(
model_name="taxonomy",
name="system_defined",
field=models.BooleanField(
default=False,
editable=False,
help_text="Indicates that tags and metadata for this taxonomy are maintained by the system; taxonomy admins will not be permitted to modify them.",
),
),
migrations.AlterField(
model_name="tag",
name="parent",
field=models.ForeignKey(
default=None,
help_text="Tag that lives one level up from the current tag, forming a hierarchy.",
null=True,
on_delete=django.db.models.deletion.CASCADE,
related_name="children",
to="oel_tagging.tag",
),
),
migrations.AlterField(
model_name="tag",
name="taxonomy",
field=models.ForeignKey(
default=None,
help_text="Namespace and rules for using a given set of tags.",
null=True,
on_delete=django.db.models.deletion.CASCADE,
to="oel_tagging.taxonomy",
),
),
migrations.AddField(
model_name="taxonomy",
name="visible_to_authors",
field=models.BooleanField(
default=True,
editable=False,
help_text="Indicates whether this taxonomy should be visible to object authors.",
),
),
migrations.RemoveField(
model_name="objecttag",
name="object_type",
),
migrations.AddField(
model_name="taxonomy",
name="_taxonomy_class",
field=models.CharField(
help_text="Taxonomy subclass used to instantiate this instance; must be a fully-qualified module and class name. If the module/class cannot be imported, an error is logged and the base Taxonomy class is used instead.",
max_length=255,
null=True,
),
),
migrations.AlterField(
model_name="objecttag",
name="object_id",
field=openedx_learning.lib.fields.MultiCollationCharField(
db_collations={"mysql": "utf8mb4_unicode_ci", "sqlite": "NOCASE"},
editable=False,
help_text="Identifier for the object being tagged",
max_length=255,
),
),
]
Loading

0 comments on commit f5164bb

Please sign in to comment.