Skip to content

Commit

Permalink
feat: Simplify Content Tagging Models (openedx#33378)
Browse files Browse the repository at this point in the history
* feat: Simplify tagging - remove Content*Taxonomy models
* chore: bump version of openedx-learning
* chore: fix lint errors
  • Loading branch information
bradenmacdonald authored Oct 5, 2023
1 parent 6710af6 commit 0a6eb51
Show file tree
Hide file tree
Showing 17 changed files with 173 additions and 365 deletions.
58 changes: 32 additions & 26 deletions openedx/core/djangoapps/content_tagging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
"""
from __future__ import annotations

from typing import Iterator, List, Type
from typing import Iterator

import openedx_tagging.core.tagging.api as oel_tagging
from django.db.models import QuerySet
from opaque_keys.edx.keys import CourseKey, UsageKey
from django.db.models import Q, QuerySet, Exists, OuterRef
from openedx_tagging.core.tagging.models import Taxonomy
from organizations.models import Organization

from .models import ContentObjectTag, ContentTaxonomy, TaxonomyOrg
from .models import ContentObjectTag, TaxonomyOrg
from .types import ContentKey


def create_taxonomy(
Expand All @@ -21,12 +21,9 @@ def create_taxonomy(
required=False,
allow_multiple=False,
allow_free_text=False,
taxonomy_class: Type = ContentTaxonomy,
) -> Taxonomy:
"""
Creates, saves, and returns a new Taxonomy with the given attributes.
If `taxonomy_class` not provided, then uses ContentTaxonomy.
"""
return oel_tagging.create_taxonomy(
name=name,
Expand All @@ -35,14 +32,13 @@ def create_taxonomy(
required=required,
allow_multiple=allow_multiple,
allow_free_text=allow_free_text,
taxonomy_class=taxonomy_class,
)


def set_taxonomy_orgs(
taxonomy: Taxonomy,
all_orgs=False,
orgs: List[Organization] = None,
orgs: list[Organization] = None,
relationship: TaxonomyOrg.RelType = TaxonomyOrg.RelType.OWNER,
):
"""
Expand Down Expand Up @@ -81,7 +77,7 @@ def set_taxonomy_orgs(

def get_taxonomies_for_org(
enabled=True,
org_owner: Organization = None,
org_owner: Organization | None = None,
) -> QuerySet:
"""
Generates a list of the enabled Taxonomies available for the given org, sorted by name.
Expand All @@ -94,32 +90,38 @@ def get_taxonomies_for_org(
If you want the disabled Taxonomies, pass enabled=False.
If you want all Taxonomies (both enabled and disabled), pass enabled=None.
"""
taxonomies = oel_tagging.get_taxonomies(enabled=enabled)
return ContentTaxonomy.taxonomies_for_org(
org=org_owner,
queryset=taxonomies,
org_short_name = org_owner.short_name if org_owner else None
return oel_tagging.get_taxonomies(enabled=enabled).filter(
Exists(
TaxonomyOrg.get_relationships(
taxonomy=OuterRef("pk"),
rel_type=TaxonomyOrg.RelType.OWNER,
org_short_name=org_short_name,
)
)
)


def get_content_tags(
object_id: str, taxonomy_id: str = None
object_key: ContentKey,
taxonomy_id: str | None = None,
) -> Iterator[ContentObjectTag]:
"""
Generates a list of content tags for a given object.
Pass taxonomy to limit the returned object_tags to a specific taxonomy.
"""
for object_tag in oel_tagging.get_object_tags(
object_id=object_id,
return oel_tagging.get_object_tags(
object_id=str(object_key),
taxonomy_id=taxonomy_id,
):
yield ContentObjectTag.cast(object_tag)
object_tag_class=ContentObjectTag,
)


def tag_content_object(
object_key: ContentKey,
taxonomy: Taxonomy,
tags: list,
object_id: CourseKey | UsageKey,
) -> list[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
Expand All @@ -136,14 +138,18 @@ def tag_content_object(
Raises ValueError if the proposed tags are invalid for this taxonomy.
Preserves existing (valid) tags, adds new (valid) tags, and removes omitted (or invalid) tags.
"""
content_tags = []
for object_tag in oel_tagging.tag_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
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(
taxonomy=taxonomy,
tags=tags,
object_id=str(object_id),
):
content_tags.append(ContentObjectTag.cast(object_tag))
return content_tags
object_id=str(object_key),
object_tag_class=ContentObjectTag,
)
return get_content_tags(str(object_key), taxonomy_id=taxonomy.id)


# Expose the oel_tagging APIs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class Migration(migrations.Migration):
'indexes': [],
'constraints': [],
},
bases=(openedx.core.djangoapps.content_tagging.models.base.ContentTaxonomyMixin, 'oel_tagging.usersystemdefinedtaxonomy'),
bases=('oel_tagging.usersystemdefinedtaxonomy', ),
),
migrations.CreateModel(
name='ContentLanguageTaxonomy',
Expand All @@ -32,7 +32,7 @@ class Migration(migrations.Migration):
'indexes': [],
'constraints': [],
},
bases=(openedx.core.djangoapps.content_tagging.models.base.ContentTaxonomyMixin, 'oel_tagging.languagetaxonomy'),
bases=('oel_tagging.languagetaxonomy', ),
),
migrations.CreateModel(
name='ContentOrganizationTaxonomy',
Expand All @@ -43,7 +43,7 @@ class Migration(migrations.Migration):
'indexes': [],
'constraints': [],
},
bases=(openedx.core.djangoapps.content_tagging.models.base.ContentTaxonomyMixin, 'oel_tagging.modelsystemdefinedtaxonomy'),
bases=('oel_tagging.modelsystemdefinedtaxonomy', ),
),
migrations.CreateModel(
name='OrganizationModelObjectTag',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Generated by Django 3.2.21 on 2023-09-29 23:32

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('content_tagging', '0005_auto_20230830_1517'),
]

operations = [
migrations.DeleteModel(
name='ContentAuthorTaxonomy',
),
migrations.DeleteModel(
name='ContentLanguageTaxonomy',
),
migrations.DeleteModel(
name='ContentTaxonomy',
),
]
5 changes: 0 additions & 5 deletions openedx/core/djangoapps/content_tagging/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,4 @@
from .base import (
TaxonomyOrg,
ContentObjectTag,
ContentTaxonomy,
)
from .system_defined import (
ContentLanguageTaxonomy,
ContentAuthorTaxonomy,
)
85 changes: 12 additions & 73 deletions openedx/core/djangoapps/content_tagging/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
"""
from __future__ import annotations

from django.core.exceptions import ValidationError
from django.db import models
from django.db.models import Exists, OuterRef, Q, QuerySet
from django.db.models import Q, QuerySet
from django.utils.translation import gettext as _
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import LearningContextKey
from opaque_keys.edx.locator import BlockUsageLocator
from opaque_keys.edx.keys import LearningContextKey, UsageKey
from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy
from organizations.models import Organization

Expand Down Expand Up @@ -91,83 +91,22 @@ class Meta:
proxy = True

@property
def object_key(self) -> BlockUsageLocator | LearningContextKey:
def object_key(self) -> UsageKey | LearningContextKey:
"""
Returns the object ID parsed as a UsageKey or LearningContextKey.
Raises InvalidKeyError object_id cannot be parse into one of those key types.
Returns None if there's no object_id.
"""
try:
return LearningContextKey.from_string(str(self.object_id))
return LearningContextKey.from_string(self.object_id)
except InvalidKeyError:
return BlockUsageLocator.from_string(str(self.object_id))
return UsageKey.from_string(self.object_id)


class ContentTaxonomyMixin:
"""
Taxonomy which can only tag Content objects (e.g. XBlocks or Courses) via ContentObjectTag.
Also ensures a valid TaxonomyOrg owner relationship with the content object.
"""

@classmethod
def taxonomies_for_org(
cls,
queryset: QuerySet,
org: Organization | None = None,
) -> QuerySet:
"""
Filters the given QuerySet to those ContentTaxonomies which are available for the given organization.
If no `org` is provided, then only ContentTaxonomies available to all organizations are returned.
If `org` is provided, then ContentTaxonomies available to this organizations are also returned.
"""
org_short_name = org.short_name if org else None
return queryset.filter(
Exists(
TaxonomyOrg.get_relationships(
taxonomy=OuterRef("pk"),
rel_type=TaxonomyOrg.RelType.OWNER,
org_short_name=org_short_name,
)
)
)

def _check_object(self, object_tag: ObjectTag) -> bool:
"""
Returns True if this ObjectTag has a valid object_id.
"""
content_tag = ContentObjectTag.cast(object_tag)
def clean(self):
super().clean()
# Make sure that object_id is a valid key
try:
content_tag.object_key
except InvalidKeyError:
return False
return super()._check_object(content_tag)

def _check_taxonomy(self, object_tag: ObjectTag) -> bool:
"""
Returns True if this taxonomy is owned by the tag's org.
"""
content_tag = ContentObjectTag.cast(object_tag)
try:
object_key = content_tag.object_key
except InvalidKeyError:
return False
if not TaxonomyOrg.get_relationships(
taxonomy=self,
rel_type=TaxonomyOrg.RelType.OWNER,
org_short_name=object_key.org,
).exists():
return False
return super()._check_taxonomy(content_tag)


class ContentTaxonomy(ContentTaxonomyMixin, Taxonomy):
"""
Taxonomy that accepts ContentTags,
and ensures a valid TaxonomyOrg owner relationship with the content object.
"""

class Meta:
proxy = True
self.object_key
except InvalidKeyError as err:
raise ValidationError("object_id is not a valid opaque key string.") from err
27 changes: 0 additions & 27 deletions openedx/core/djangoapps/content_tagging/models/system_defined.py

This file was deleted.

Loading

0 comments on commit 0a6eb51

Please sign in to comment.