From 9bea44732b61a41592eb8cabda0423cf35c7ee29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Chris=20Ch=C3=A1vez?= Date: Wed, 2 Aug 2023 14:22:23 -0500 Subject: [PATCH] System defined taxonomies (#32869) * feat: System defined taxonomies * style: models.py moved to models/base.py * feat: New Content System defined models * style: Lint and migration * fix: Fix migration error * chore: Rebase and compile requirements * refactor: adds ContentTaxonomyMixin for use when creating content system taxonomies Pulls the ContentTaxonomy-specific logic into a mixin class to bring the Content-specific logic into other Taxonony subclasses. * fix: Tests * test: System defined model validations * fix: Move language taxonomy creation to openedx-learning * style: Rename of OrganizationSystemDefinedTaxonomy * style: nits * chore: Update openedx-learning dependency --------- Co-authored-by: Jillian Vogel --- .../fixtures/system_defined.yaml | 22 ++++++ .../0002_system_defined_taxonomies.py | 59 +++++++++++++++ .../migrations/0003_system_defined_fixture.py | 39 ++++++++++ .../content_tagging/models/__init__.py | 13 ++++ .../{models.py => models/base.py} | 15 +++- .../content_tagging/models/system_defined.py | 75 +++++++++++++++++++ .../content_tagging/tests/test_models.py | 71 ++++++++++++++++++ .../content_tagging/tests/test_rules.py | 9 ++- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 3 +- requirements/edx/doc.txt | 4 +- requirements/edx/kernel.in | 2 +- requirements/edx/testing.txt | 2 +- 13 files changed, 304 insertions(+), 12 deletions(-) create mode 100644 openedx/features/content_tagging/fixtures/system_defined.yaml create mode 100644 openedx/features/content_tagging/migrations/0002_system_defined_taxonomies.py create mode 100644 openedx/features/content_tagging/migrations/0003_system_defined_fixture.py create mode 100644 openedx/features/content_tagging/models/__init__.py rename openedx/features/content_tagging/{models.py => models/base.py} (96%) create mode 100644 openedx/features/content_tagging/models/system_defined.py create mode 100644 openedx/features/content_tagging/tests/test_models.py diff --git a/openedx/features/content_tagging/fixtures/system_defined.yaml b/openedx/features/content_tagging/fixtures/system_defined.yaml new file mode 100644 index 0000000000..0744534627 --- /dev/null +++ b/openedx/features/content_tagging/fixtures/system_defined.yaml @@ -0,0 +1,22 @@ +- model: oel_tagging.taxonomy + pk: -2 + fields: + name: Organizations + description: Allows tags for any organization ID created on the instance. + enabled: true + required: true + allow_multiple: false + allow_free_text: false + visible_to_authors: false + _taxonomy_class: openedx.features.content_tagging.models.ContentAuthorTaxonomy +- model: oel_tagging.taxonomy + pk: -3 + fields: + name: Content Authors + description: Allows tags for any user ID created on the instance. + enabled: true + required: true + allow_multiple: false + allow_free_text: false + visible_to_authors: false + _taxonomy_class: openedx.features.content_tagging.models.ContentOrganizationTaxonomy diff --git a/openedx/features/content_tagging/migrations/0002_system_defined_taxonomies.py b/openedx/features/content_tagging/migrations/0002_system_defined_taxonomies.py new file mode 100644 index 0000000000..c743a70ce2 --- /dev/null +++ b/openedx/features/content_tagging/migrations/0002_system_defined_taxonomies.py @@ -0,0 +1,59 @@ +# Generated by Django 3.2.20 on 2023-07-31 21:07 + +from django.db import migrations +import openedx.features.content_tagging.models.base + + +class Migration(migrations.Migration): + + dependencies = [ + ('oel_tagging', '0005_language_taxonomy'), + ('content_tagging', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='ContentAuthorTaxonomy', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=(openedx.features.content_tagging.models.base.ContentTaxonomyMixin, 'oel_tagging.usersystemdefinedtaxonomy'), + ), + migrations.CreateModel( + name='ContentLanguageTaxonomy', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=(openedx.features.content_tagging.models.base.ContentTaxonomyMixin, 'oel_tagging.languagetaxonomy'), + ), + migrations.CreateModel( + name='ContentOrganizationTaxonomy', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=(openedx.features.content_tagging.models.base.ContentTaxonomyMixin, 'oel_tagging.modelsystemdefinedtaxonomy'), + ), + migrations.CreateModel( + name='OrganizationModelObjectTag', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('oel_tagging.modelobjecttag',), + ), + ] diff --git a/openedx/features/content_tagging/migrations/0003_system_defined_fixture.py b/openedx/features/content_tagging/migrations/0003_system_defined_fixture.py new file mode 100644 index 0000000000..c155b34151 --- /dev/null +++ b/openedx/features/content_tagging/migrations/0003_system_defined_fixture.py @@ -0,0 +1,39 @@ +# Generated by Django 3.2.20 on 2023-07-11 22:57 + +from django.db import migrations +from django.core.management import call_command +from openedx.features.content_tagging.models import ContentLanguageTaxonomy + + +def load_system_defined_taxonomies(apps, schema_editor): + """ + Creates system defined taxonomies + """ + + # Create system defined taxonomy instances + call_command('loaddata', '--app=content_tagging', 'system_defined.yaml') + + # Adding taxonomy class to the language taxonomy + Taxonomy = apps.get_model('oel_tagging', 'Taxonomy') + language_taxonomy = Taxonomy.objects.get(id=-1) + language_taxonomy.taxonomy_class = ContentLanguageTaxonomy + + +def revert_system_defined_taxonomies(apps, schema_editor): + """ + Deletes all system defined taxonomies + """ + Taxonomy = apps.get_model('oel_tagging', 'Taxonomy') + Taxonomy.objects.get(id=-2).delete() + Taxonomy.objects.get(id=-3).delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('content_tagging', '0002_system_defined_taxonomies'), + ] + + operations = [ + migrations.RunPython(load_system_defined_taxonomies, revert_system_defined_taxonomies), + ] diff --git a/openedx/features/content_tagging/models/__init__.py b/openedx/features/content_tagging/models/__init__.py new file mode 100644 index 0000000000..dc748c355b --- /dev/null +++ b/openedx/features/content_tagging/models/__init__.py @@ -0,0 +1,13 @@ +""" +Content Tagging and System defined models +""" +from .base import ( + TaxonomyOrg, + ContentObjectTag, + ContentTaxonomy, +) +from .system_defined import ( + ContentLanguageTaxonomy, + ContentAuthorTaxonomy, + ContentOrganizationTaxonomy, +) diff --git a/openedx/features/content_tagging/models.py b/openedx/features/content_tagging/models/base.py similarity index 96% rename from openedx/features/content_tagging/models.py rename to openedx/features/content_tagging/models/base.py index a3b094a8e9..255d3852fa 100644 --- a/openedx/features/content_tagging/models.py +++ b/openedx/features/content_tagging/models/base.py @@ -104,16 +104,13 @@ class ContentObjectTag(ObjectTag): return BlockUsageLocator.from_string(str(self.object_id)) -class ContentTaxonomy(Taxonomy): +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. """ - class Meta: - proxy = True - @classmethod def taxonomies_for_org( cls, @@ -164,3 +161,13 @@ class ContentTaxonomy(Taxonomy): ).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 diff --git a/openedx/features/content_tagging/models/system_defined.py b/openedx/features/content_tagging/models/system_defined.py new file mode 100644 index 0000000000..642e1c08b0 --- /dev/null +++ b/openedx/features/content_tagging/models/system_defined.py @@ -0,0 +1,75 @@ +""" +System defined models +""" +from typing import Type + +from openedx_tagging.core.tagging.models import ( + ModelSystemDefinedTaxonomy, + ModelObjectTag, + UserSystemDefinedTaxonomy, + LanguageTaxonomy, +) + +from organizations.models import Organization +from .base import ContentTaxonomyMixin + + +class OrganizationModelObjectTag(ModelObjectTag): + """ + ObjectTags for the OrganizationSystemDefinedTaxonomy. + """ + + class Meta: + proxy = True + + @property + def tag_class_model(self) -> Type: + """ + Associate the organization model + """ + return Organization + + @property + def tag_class_value(self) -> str: + """ + Returns the organization name to use it on Tag.value when creating Tags for this taxonomy. + """ + return "name" + + +class ContentOrganizationTaxonomy(ContentTaxonomyMixin, ModelSystemDefinedTaxonomy): + """ + Organization system-defined taxonomy that accepts ContentTags + + Side note: The organization of an object is already encoded in its usage ID, + but a Taxonomy with Organization as Tags is being used so that the objects can be + indexed and can be filtered in the same tagging system, without any special casing. + """ + + class Meta: + proxy = True + + @property + def object_tag_class(self) -> Type: + """ + Returns OrganizationModelObjectTag as ObjectTag subclass associated with this taxonomy. + """ + return OrganizationModelObjectTag + + +class ContentLanguageTaxonomy(ContentTaxonomyMixin, LanguageTaxonomy): + """ + Language system-defined taxonomy that accepts ContentTags + """ + + class Meta: + proxy = True + + +class ContentAuthorTaxonomy(ContentTaxonomyMixin, UserSystemDefinedTaxonomy): + """ + Author system-defined taxonomy that accepts ContentTags + """ + + class Meta: + proxy = True diff --git a/openedx/features/content_tagging/tests/test_models.py b/openedx/features/content_tagging/tests/test_models.py new file mode 100644 index 0000000000..a0b358ea31 --- /dev/null +++ b/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, + Tag, +) +from openedx_tagging.core.tagging.api import create_taxonomy +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 = create_taxonomy( + name='Test taxonomy', + taxonomy_class=taxonomy_cls, + ) + + tag = Tag( + value="value", + external_id="external_id", + 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, + ) diff --git a/openedx/features/content_tagging/tests/test_rules.py b/openedx/features/content_tagging/tests/test_rules.py index 029657e44f..77dcc2270b 100644 --- a/openedx/features/content_tagging/tests/test_rules.py +++ b/openedx/features/content_tagging/tests/test_rules.py @@ -3,7 +3,11 @@ import ddt from django.contrib.auth import get_user_model from django.test.testcases import TestCase, override_settings -from openedx_tagging.core.tagging.models import ObjectTag, Tag +from openedx_tagging.core.tagging.models import ( + ObjectTag, + Tag, + UserSystemDefinedTaxonomy, +) from organizations.models import Organization from common.djangoapps.student.auth import add_users, update_org_role @@ -136,7 +140,8 @@ class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): system_taxonomy = api.create_taxonomy( name="System Languages", ) - system_taxonomy.system_defined = True + system_taxonomy.taxonomy_class = UserSystemDefinedTaxonomy + system_taxonomy = system_taxonomy.cast() assert self.superuser.has_perm(perm, system_taxonomy) assert not self.staff.has_perm(perm, system_taxonomy) assert not self.user_all_orgs.has_perm(perm, system_taxonomy) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 67250d9d05..3a051ffb16 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -773,7 +773,7 @@ openedx-filters==1.4.0 # -r requirements/edx/kernel.in # lti-consumer-xblock # skill-tagging -openedx-learning==0.1.0 +openedx-learning==0.1.1 # via -r requirements/edx/kernel.in openedx-mongodbproxy==0.2.0 # via -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 743938fb17..6283b5cebe 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1307,7 +1307,7 @@ openedx-filters==1.4.0 # -r requirements/edx/testing.txt # lti-consumer-xblock # skill-tagging -openedx-learning==0.1.0 +openedx-learning==0.1.1 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -2138,6 +2138,7 @@ walrus==0.9.3 # edx-event-bus-redis watchdog==3.0.0 # via + # -r requirements/edx/development.in # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt wcwidth==0.2.6 diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 3b584a4bee..fb7ae3c890 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -556,7 +556,7 @@ edx-drf-extensions==8.8.0 # edx-rbac # edx-when # edxval -edx-enterprise==4.0.6 +edx-enterprise==4.0.7 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt @@ -916,7 +916,7 @@ openedx-filters==1.4.0 # -r requirements/edx/base.txt # lti-consumer-xblock # skill-tagging -openedx-learning==0.1.0 +openedx-learning==0.1.1 # via -r requirements/edx/base.txt openedx-mongodbproxy==0.2.0 # via -r requirements/edx/base.txt diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 8929520ea8..8c0258f55f 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -115,7 +115,7 @@ openedx-calc # Library supporting mathematical calculatio openedx-django-require openedx-events>=8.3.0 # Open edX Events from Hooks Extension Framework (OEP-50) openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50) -openedx-learning<=0.1 +openedx-learning # Open edX Learning core (experimental) openedx-mongodbproxy openedx-django-wiki openedx-blockstore diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 7165c677dc..707f40ca75 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -985,7 +985,7 @@ openedx-filters==1.4.0 # -r requirements/edx/base.txt # lti-consumer-xblock # skill-tagging -openedx-learning==0.1.0 +openedx-learning==0.1.1 # via -r requirements/edx/base.txt openedx-mongodbproxy==0.2.0 # via -r requirements/edx/base.txt