Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion openedx_learning/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""
Open edX Learning ("Learning Core").
"""
__version__ = "0.7.0"
__version__ = "0.8.0"
2 changes: 1 addition & 1 deletion openedx_tagging/core/tagging/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ObjectTagAdmin(admin.ModelAdmin):
"""
fields = ["object_id", "taxonomy", "tag", "_value"]
autocomplete_fields = ["tag"]
list_display = ["object_id", "name", "value"]
list_display = ["object_id", "export_id", "value"]
readonly_fields = ["object_id"]

def has_add_permission(self, request):
Expand Down
141 changes: 106 additions & 35 deletions openedx_tagging/core/tagging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def get_object_tags(
Value("\t"),
output_field=models.CharField(),
)))
.annotate(taxonomy_name=Coalesce(F("taxonomy__name"), F("_name")))
.annotate(taxonomy_name=Coalesce(F("taxonomy__name"), F("_export_id")))
# Sort first by taxonomy name, then by tag value in tree order:
.order_by("taxonomy_name", "sort_key")
)
Expand Down Expand Up @@ -274,11 +274,58 @@ def delete_object_tags(object_id: str):
tags.delete()


def _check_new_tag_count(
new_tag_count: int,
taxonomy: Taxonomy | None,
object_id: str,
taxonomy_export_id: str | None = None,
) -> None:
"""
Checks if the new count of tags for the object is equal or less than 100
"""
# Exclude to avoid counting the tags that are going to be updated
if taxonomy:
current_count = ObjectTag.objects.filter(object_id=object_id).exclude(taxonomy_id=taxonomy.id).count()
else:
current_count = ObjectTag.objects.filter(object_id=object_id).exclude(_export_id=taxonomy_export_id).count()

if current_count + new_tag_count > 100:
raise ValueError(
_("Cannot add more than 100 tags to ({object_id}).").format(object_id=object_id)
)


def _get_current_tags(
taxonomy: Taxonomy | None,
tags: list[str],
object_id: str,
object_tag_class: type[ObjectTag] = ObjectTag,
taxonomy_export_id: str | None = None,
) -> list[ObjectTag]:
"""
Returns the current object tags of the related object_id with taxonomy
"""
ObjectTagClass = object_tag_class
if taxonomy:
if not taxonomy.allow_multiple and len(tags) > 1:
raise ValueError(_("Taxonomy ({name}) only allows one tag per object.").format(name=taxonomy.name))
current_tags = list(
ObjectTagClass.objects.filter(taxonomy=taxonomy, object_id=object_id)
)
else:
current_tags = list(
ObjectTagClass.objects.filter(_export_id=taxonomy_export_id, object_id=object_id)
)
return current_tags


def tag_object(
object_id: str,
taxonomy: Taxonomy,
taxonomy: Taxonomy | None,
tags: list[str],
object_tag_class: type[ObjectTag] = ObjectTag,
create_invalid: bool = False,
taxonomy_export_id: str | None = None,
) -> None:
"""
Replaces the existing ObjectTag entries for the given taxonomy + object_id
Expand All @@ -292,37 +339,34 @@ def tag_object(
Raised Tag.DoesNotExist if the proposed tags are invalid for this taxonomy.
Preserves existing (valid) tags, adds new (valid) tags, and removes omitted
(or invalid) tags.
"""

def _check_new_tag_count(new_tag_count: int) -> None:
"""
Checks if the new count of tags for the object is equal or less than 100
"""
# Exclude self.id to avoid counting the tags that are going to be updated
current_count = ObjectTag.objects.filter(object_id=object_id).exclude(taxonomy_id=taxonomy.id).count()

if current_count + new_tag_count > 100:
raise ValueError(
_("Cannot add more than 100 tags to ({object_id}).").format(object_id=object_id)
)
create_invalid: You can create invalid tags and avoid the previous behavior using.

taxonomy_export_id: You can create object tags without taxonomy using this param
and `taxonomy` as None. You need to use the taxonomy.export_id, so you can resycn
this object tag if the taxonomy is created in the future.
"""
if not isinstance(tags, list):
raise ValueError(_("Tags must be a list, not {type}.").format(type=type(tags).__name__))

ObjectTagClass = object_tag_class
taxonomy = taxonomy.cast() # Make sure we're using the right subclass. This is a no-op if we are already.
tags = list(dict.fromkeys(tags)) # Remove duplicates preserving order

_check_new_tag_count(len(tags))

if not taxonomy.allow_multiple and len(tags) > 1:
raise ValueError(_("Taxonomy ({name}) only allows one tag per object.").format(name=taxonomy.name))

current_tags = list(
ObjectTagClass.objects.filter(taxonomy=taxonomy, object_id=object_id)
if taxonomy:
taxonomy = taxonomy.cast() # Make sure we're using the right subclass. This is a no-op if we are already.
elif not taxonomy_export_id:
raise ValueError(_("`taxonomy_export_id` can't be None if `taxonomy` is None"))

_check_new_tag_count(len(tags), taxonomy, object_id, taxonomy_export_id)
current_tags = _get_current_tags(
taxonomy,
tags,
object_id,
object_tag_class,
taxonomy_export_id
)

updated_tags = []
if taxonomy.allow_free_text:
if taxonomy and taxonomy.allow_free_text:
for tag_value in tags:
object_tag_index = next((i for (i, t) in enumerate(current_tags) if t.value == tag_value), -1)
if object_tag_index >= 0:
Expand All @@ -334,19 +378,46 @@ def _check_new_tag_count(new_tag_count: int) -> None:
else:
# Handle closed taxonomies:
for tag_value in tags:
tag = taxonomy.tag_for_value(tag_value) # Will raise Tag.DoesNotExist if the value is invalid.
object_tag_index = next((i for (i, t) in enumerate(current_tags) if t.tag_id == tag.id), -1)
if object_tag_index >= 0:
# This tag is already applied.
object_tag = current_tags.pop(object_tag_index)
if object_tag._value != tag.value: # pylint: disable=protected-access
# The ObjectTag's cached '_value' is out of sync with the Tag, so update it:
object_tag._value = tag.value # pylint: disable=protected-access
tag = None
# When export, sometimes, the value has a space at the beginning and end.
tag_value = tag_value.strip()
if taxonomy:
try:
tag = taxonomy.tag_for_value(tag_value) # Will raise Tag.DoesNotExist if the value is invalid.
except Tag.DoesNotExist as e:
if not create_invalid:
raise e

if tag:
# Tag exists in the taxonomy
object_tag_index = next((i for (i, t) in enumerate(current_tags) if t.tag_id == tag.id), -1)
if object_tag_index >= 0:
# This tag is already applied.
object_tag = current_tags.pop(object_tag_index)
if object_tag._value != tag.value: # pylint: disable=protected-access
# The ObjectTag's cached '_value' is out of sync with the Tag, so update it:
object_tag._value = tag.value # pylint: disable=protected-access
updated_tags.append(object_tag)
else:
# We are newly applying this tag:
object_tag = ObjectTagClass(taxonomy=taxonomy, object_id=object_id, tag=tag)
updated_tags.append(object_tag)
else:
# We are newly applying this tag:
object_tag = ObjectTagClass(taxonomy=taxonomy, object_id=object_id, tag=tag)
elif taxonomy:
# Tag doesn't exist in the taxonomy and `create_invalid` is True
object_tag = ObjectTagClass(taxonomy=taxonomy, object_id=object_id, _value=tag_value)
updated_tags.append(object_tag)
else:
# Taxonomy is None (also tag doesn't exist)
if taxonomy_export_id:
# This will always be true, since it is verified at the beginning of the function.
# This condition is placed by the type checks.
object_tag = ObjectTagClass(
taxonomy=None,
object_id=object_id,
_value=tag_value,
_export_id=taxonomy_export_id
)
updated_tags.append(object_tag)

# Save all updated tags at once to avoid partial updates
with transaction.atomic():
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Generated by Django 3.2.22 on 2024-03-22 19:47

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

import openedx_learning.lib.fields


def migrate_export_id(apps, schema_editor):
ObjectTag = apps.get_model("oel_tagging", "ObjectTag")
for object_tag in ObjectTag.objects.all():
if object_tag.taxonomy:
object_tag.export_id = object_tag.taxonomy.export_id
object_tag.save(update_fields=["_export_id"])


def reverse(apps, schema_editor):
pass


class Migration(migrations.Migration):

dependencies = [
('oel_tagging', '0015_taxonomy_export_id'),
]

operations = [
migrations.RenameField(
model_name='objecttag',
old_name='_name',
new_name='_export_id',
),
migrations.RunPython(migrate_export_id, reverse),
migrations.AlterField(
model_name='objecttag',
name='taxonomy',
field=models.ForeignKey(blank=True, default=None, help_text="Taxonomy that this object tag belongs to. Used for validating the tag and provides the tag's 'name' if set.", null=True, on_delete=django.db.models.deletion.SET_NULL, to='oel_tagging.taxonomy'),
),
migrations.AlterField(
model_name='objecttag',
name='_export_id',
field=openedx_learning.lib.fields.MultiCollationCharField(db_collations={'mysql': 'utf8mb4_unicode_ci', 'sqlite': 'NOCASE'}, help_text='User-facing label used for this tag, stored in case taxonomy is (or becomes) null. If the taxonomy field is set, then taxonomy.export_id takes precedence over this field.', max_length=255),
),
]
50 changes: 31 additions & 19 deletions openedx_tagging/core/tagging/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,7 @@ class ObjectTag(models.Model):
taxonomy = models.ForeignKey(
Taxonomy,
null=True,
blank=True,
default=None,
on_delete=models.SET_NULL,
help_text=_(
Expand All @@ -792,11 +793,11 @@ class ObjectTag(models.Model):
"Tag associated with this object tag. Provides the tag's 'value' if set."
),
)
_name = case_insensitive_char_field(
_export_id = case_insensitive_char_field(
max_length=255,
help_text=_(
"User-facing label used for this tag, stored in case taxonomy is (or becomes) null."
" If the taxonomy field is set, then taxonomy.name takes precedence over this field."
" If the taxonomy field is set, then taxonomy.export_id takes precedence over this field."
),
)
_value = case_insensitive_char_field(
Expand All @@ -821,9 +822,9 @@ class Meta:
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if not self.pk: # This is a new instance:
# Set _name and _value automatically on creation, if they weren't set:
if not self._name and self.taxonomy:
self._name = self.taxonomy.name
# Set _export_id and _value automatically on creation, if they weren't set:
if not self._export_id and self.taxonomy:
self._export_id = self.taxonomy.export_id
if not self._value and self.tag:
self._value = self.tag.value

Expand All @@ -837,24 +838,28 @@ def __str__(self):
"""
User-facing string representation of an ObjectTag.
"""
return f"<{self.__class__.__name__}> {self.object_id}: {self.name}={self.value}"
if self.taxonomy:
name = self.taxonomy.name
else:
name = self.export_id
return f"<{self.__class__.__name__}> {self.object_id}: {name}={self.value}"

@property
def name(self) -> str:
def export_id(self) -> str:
"""
Returns this tag's name/label.

If taxonomy is set, then returns its name.
Otherwise, returns the cached _name field.
Otherwise, returns the cached _export_id field.
"""
return self.taxonomy.name if self.taxonomy else self._name
return self.taxonomy.export_id if self.taxonomy else self._export_id

@name.setter
def name(self, name: str):
@export_id.setter
def export_id(self, export_id: str):
"""
Stores to the _name field.
Stores to the _export_id field.
"""
self._name = name
self._export_id = export_id

@property
def value(self) -> str:
Expand Down Expand Up @@ -899,8 +904,8 @@ def clean(self):
# was deleted, but we still preserve this _value here in case the Taxonomy or Tag get re-created in future.
if self._value == "":
raise ValidationError("Invalid _value - empty string")
if self.taxonomy and self.taxonomy.name != self._name:
raise ValidationError("ObjectTag's _name is out of sync with Taxonomy.name")
if self.taxonomy and self.taxonomy.export_id != self._export_id:
raise ValidationError("ObjectTag's _export_id is out of sync with Taxonomy.name")
if "," in self.object_id or "*" in self.object_id:
# Some APIs may use these characters to allow wildcard matches or multiple matches in the future.
raise ValidationError("Object ID contains invalid characters")
Expand Down Expand Up @@ -930,11 +935,18 @@ def resync(self) -> bool:
# We used to have code here that would try to find a new taxonomy if the current taxonomy has been deleted.
# But for now that's removed, as it risks things like linking a tag to the wrong org's taxonomy.

# Sync the stored _name with the taxonomy.name
if self.taxonomy and self._name != self.taxonomy.name:
self.name = self.taxonomy.name
# Sync the stored _export_id with the taxonomy.name
if self.taxonomy and self._export_id != self.taxonomy.export_id:
self.export_id = self.taxonomy.export_id
changed = True

# Sync taxonomy with matching _export_id
if not self.taxonomy:
taxonomy = Taxonomy.objects.filter(export_id=self.export_id).first()
if taxonomy:
self.taxonomy = taxonomy
changed = True

# Closed taxonomies require a tag matching _value
if self.taxonomy and not self.taxonomy.allow_free_text and not self.tag_id:
tag = self.taxonomy.tag_set.filter(value=self.value).first()
Expand Down Expand Up @@ -965,5 +977,5 @@ def copy(self, object_tag: ObjectTag) -> Self:
self.taxonomy = object_tag.taxonomy
self.object_id = object_tag.object_id
self._value = object_tag._value # pylint: disable=protected-access
self._name = object_tag._name # pylint: disable=protected-access
self._export_id = object_tag._export_id # pylint: disable=protected-access
return self
5 changes: 3 additions & 2 deletions openedx_tagging/core/tagging/rest_api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,11 @@ def to_representation(self, instance: list[ObjectTag]) -> dict:
tax_entry = next((t for t in taxonomies if t["taxonomy_id"] == obj_tag.taxonomy_id), None)
if tax_entry is None:
tax_entry = {
"name": obj_tag.name,
"name": obj_tag.taxonomy.name if obj_tag.taxonomy else None,
"taxonomy_id": obj_tag.taxonomy_id,
"can_tag_object": self._can(can_tag_object_perm, obj_tag),
"tags": []
"tags": [],
"export_id": obj_tag.export_id,
}
taxonomies.append(tax_entry)
tax_entry["tags"].append(ObjectTagMinimalSerializer(obj_tag, context=self.context).data)
Expand Down
5 changes: 5 additions & 0 deletions openedx_tagging/core/tagging/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
get_object_tags,
get_taxonomies,
get_taxonomy,
resync_object_tags,
tag_object,
update_tag_in_taxonomy,
)
Expand Down Expand Up @@ -315,6 +316,8 @@ def create_import(self, request: Request, **_kwargs) -> Response:

if import_success:
serializer = self.get_serializer(taxonomy)
# We need to resync all object tags because, there may be tags that do not have a taxonomy.
resync_object_tags()
return Response(serializer.data, status=status.HTTP_201_CREATED)
else:
taxonomy.delete()
Expand All @@ -340,6 +343,8 @@ def update_import(self, request: Request, **_kwargs) -> Response:

if import_success:
serializer = self.get_serializer(taxonomy)
# We need to resync all object tags because, there may be tags that do not have a taxonomy.
resync_object_tags()
return Response(serializer.data)
else:
return Response(task.log, status=status.HTTP_400_BAD_REQUEST)
Expand Down
Loading