From 5516c88395586df4514d35c9c21ac2f1cec751f3 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 3 Oct 2023 14:54:37 -0700 Subject: [PATCH] feat: Squash Tagging/Taxonomy migrations, bump openedx-learning to 0.2.3 --- .../core/djangoapps/content_tagging/api.py | 2 - .../migrations/0001_squashed.py | 54 +++++++++++++++++++ .../migrations/0003_system_defined_fixture.py | 6 --- .../migrations/0004_system_defined_org.py | 9 ++-- .../migrations/0007_system_defined_org_2.py | 28 ++++++++++ .../rest_api/v1/tests/test_views.py | 13 ++--- .../content_tagging/tests/test_tasks.py | 45 ++++++++++------ requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 12 files changed, 126 insertions(+), 41 deletions(-) create mode 100644 openedx/core/djangoapps/content_tagging/migrations/0001_squashed.py create mode 100644 openedx/core/djangoapps/content_tagging/migrations/0007_system_defined_org_2.py diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index 4ff21dff3f3b..4867160c1b83 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -18,7 +18,6 @@ def create_taxonomy( name: str, description: str = None, enabled=True, - required=False, allow_multiple=False, allow_free_text=False, ) -> Taxonomy: @@ -29,7 +28,6 @@ def create_taxonomy( name=name, description=description, enabled=enabled, - required=required, allow_multiple=allow_multiple, allow_free_text=allow_free_text, ) diff --git a/openedx/core/djangoapps/content_tagging/migrations/0001_squashed.py b/openedx/core/djangoapps/content_tagging/migrations/0001_squashed.py new file mode 100644 index 000000000000..fa00df307c64 --- /dev/null +++ b/openedx/core/djangoapps/content_tagging/migrations/0001_squashed.py @@ -0,0 +1,54 @@ +# Generated by Django 3.2.21 on 2023-10-09 23:12 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + replaces = [ + ('content_tagging', '0001_initial'), + ('content_tagging', '0002_system_defined_taxonomies'), + ('content_tagging', '0003_system_defined_fixture'), + ('content_tagging', '0004_system_defined_org'), + ('content_tagging', '0005_auto_20230830_1517'), + ('content_tagging', '0006_simplify_models'), + ] + + initial = True + + dependencies = [ + ("oel_tagging", "0001_squashed"), + ('organizations', '0003_historicalorganizationcourse'), + ] + + operations = [ + migrations.CreateModel( + name='ContentObjectTag', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('oel_tagging.objecttag',), + ), + migrations.CreateModel( + name='TaxonomyOrg', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('rel_type', models.CharField(choices=[('OWN', 'owner')], default='OWN', max_length=3)), + ('org', models.ForeignKey(default=None, help_text='Organization that is related to this taxonomy.If None, then this taxonomy is related to all organizations.', null=True, on_delete=django.db.models.deletion.CASCADE, to='organizations.organization')), + ('taxonomy', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='oel_tagging.taxonomy')), + ], + ), + migrations.AddIndex( + model_name='taxonomyorg', + index=models.Index(fields=['taxonomy', 'rel_type'], name='content_tag_taxonom_b04dd1_idx'), + ), + migrations.AddIndex( + model_name='taxonomyorg', + index=models.Index(fields=['taxonomy', 'rel_type', 'org'], name='content_tag_taxonom_70d60b_idx'), + ), + ] diff --git a/openedx/core/djangoapps/content_tagging/migrations/0003_system_defined_fixture.py b/openedx/core/djangoapps/content_tagging/migrations/0003_system_defined_fixture.py index 7846b907c4e6..ff855482e1fc 100644 --- a/openedx/core/djangoapps/content_tagging/migrations/0003_system_defined_fixture.py +++ b/openedx/core/djangoapps/content_tagging/migrations/0003_system_defined_fixture.py @@ -38,12 +38,6 @@ def load_system_defined_taxonomies(apps, schema_editor): org_taxonomy.taxonomy_class = ContentOrganizationTaxonomy org_taxonomy.save() - # Adding taxonomy class to the language taxonomy - language_taxonomy = Taxonomy.objects.get(id=-1) - ContentLanguageTaxonomy = apps.get_model("content_tagging", "ContentLanguageTaxonomy") - language_taxonomy.taxonomy_class = ContentLanguageTaxonomy - language_taxonomy.save() - def revert_system_defined_taxonomies(apps, schema_editor): """ diff --git a/openedx/core/djangoapps/content_tagging/migrations/0004_system_defined_org.py b/openedx/core/djangoapps/content_tagging/migrations/0004_system_defined_org.py index 852d67ae4ab6..a60ef381cd54 100644 --- a/openedx/core/djangoapps/content_tagging/migrations/0004_system_defined_org.py +++ b/openedx/core/djangoapps/content_tagging/migrations/0004_system_defined_org.py @@ -6,8 +6,9 @@ def load_system_defined_org_taxonomies(apps, _schema_editor): Associates the system defined taxonomy Language (id=-1) to all orgs and removes the ContentOrganizationTaxonomy (id=-3) from the database """ - TaxonomyOrg = apps.get_model("content_tagging", "TaxonomyOrg") - TaxonomyOrg.objects.create(id=-1, taxonomy_id=-1, org=None) + # Disabled for now as the way that this taxonomy is created has changed. + # TaxonomyOrg = apps.get_model("content_tagging", "TaxonomyOrg") + # TaxonomyOrg.objects.create(id=-1, taxonomy_id=-1, org=None) Taxonomy = apps.get_model("oel_tagging", "Taxonomy") Taxonomy.objects.get(id=-3).delete() @@ -20,8 +21,8 @@ def revert_system_defined_org_taxonomies(apps, _schema_editor): Deletes association of system defined taxonomy Language (id=-1) to all orgs and creates the ContentOrganizationTaxonomy (id=-3) in the database """ - TaxonomyOrg = apps.get_model("content_tagging", "TaxonomyOrg") - TaxonomyOrg.objects.get(id=-1).delete() + # TaxonomyOrg = apps.get_model("content_tagging", "TaxonomyOrg") + # TaxonomyOrg.objects.get(id=-1).delete() Taxonomy = apps.get_model("oel_tagging", "Taxonomy") org_taxonomy = Taxonomy( diff --git a/openedx/core/djangoapps/content_tagging/migrations/0007_system_defined_org_2.py b/openedx/core/djangoapps/content_tagging/migrations/0007_system_defined_org_2.py new file mode 100644 index 000000000000..0a48016ca2b4 --- /dev/null +++ b/openedx/core/djangoapps/content_tagging/migrations/0007_system_defined_org_2.py @@ -0,0 +1,28 @@ +from django.db import migrations + + +def mark_language_taxonomy_as_all_orgs(apps, _schema_editor): + """ + Associates the system defined taxonomy Language (id=-1) to all orgs. + """ + TaxonomyOrg = apps.get_model("content_tagging", "TaxonomyOrg") + TaxonomyOrg.objects.update_or_create(taxonomy_id=-1, defaults={"org": None}) + + +def revert_mark_language_taxonomy_as_all_orgs(apps, _schema_editor): + """ + Deletes association of system defined taxonomy Language (id=-1) to all orgs. + """ + TaxonomyOrg = apps.get_model("content_tagging", "TaxonomyOrg") + TaxonomyOrg.objects.get(taxonomy_id=-1, org=None).delete() + + +class Migration(migrations.Migration): + dependencies = [ + ('content_tagging', '0001_squashed'), + ("oel_tagging", "0012_language_taxonomy"), + ] + + operations = [ + migrations.RunPython(mark_language_taxonomy_as_all_orgs, revert_mark_language_taxonomy_as_all_orgs), + ] diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 2eafb933b483..8e57c1546ff0 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py @@ -33,8 +33,7 @@ def check_taxonomy( name, description=None, enabled=True, - required=False, - allow_multiple=False, + allow_multiple=True, allow_free_text=False, system_defined=False, visible_to_authors=True, @@ -47,7 +46,6 @@ def check_taxonomy( assert data["name"] == name assert data["description"] == description assert data["enabled"] == enabled - assert data["required"] == required assert data["allow_multiple"] == allow_multiple assert data["allow_free_text"] == allow_free_text assert data["system_defined"] == system_defined @@ -350,7 +348,6 @@ def test_create_taxonomy(self, user_attr, expected_status): "name": "taxonomy_data", "description": "This is a description", "enabled": True, - "required": True, "allow_multiple": True, } @@ -444,7 +441,6 @@ def test_update_taxonomy(self, user_attr, taxonomy_attr, expected_status): "name": "new name", "description": taxonomy.description, "enabled": taxonomy.enabled, - "required": taxonomy.required, }, ) @@ -540,7 +536,6 @@ def test_patch_taxonomy(self, user_attr, taxonomy_attr, expected_status): "name": "new name", "description": taxonomy.description, "enabled": taxonomy.enabled, - "required": taxonomy.required, }, ) @@ -668,13 +663,13 @@ def setUp(self): ) self.multiple_taxonomy = Taxonomy.objects.create(name="Multiple Taxonomy", allow_multiple=True) - self.required_taxonomy = Taxonomy.objects.create(name="Required Taxonomy", required=True) + self.single_value_taxonomy = Taxonomy.objects.create(name="Required Taxonomy", allow_multiple=False) for i in range(20): # Valid ObjectTags Tag.objects.create(taxonomy=self.tA1, value=f"Tag {i}") Tag.objects.create(taxonomy=self.tA2, value=f"Tag {i}") Tag.objects.create(taxonomy=self.multiple_taxonomy, value=f"Tag {i}") - Tag.objects.create(taxonomy=self.required_taxonomy, value=f"Tag {i}") + Tag.objects.create(taxonomy=self.single_value_taxonomy, value=f"Tag {i}") self.open_taxonomy = Taxonomy.objects.create(name="Enabled Free-Text Taxonomy", allow_free_text=True) @@ -685,7 +680,7 @@ def setUp(self): rel_type=TaxonomyOrg.RelType.OWNER, ) TaxonomyOrg.objects.create( - taxonomy=self.required_taxonomy, + taxonomy=self.single_value_taxonomy, org=self.orgA, rel_type=TaxonomyOrg.RelType.OWNER, ) diff --git a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py index f3c964b836c7..39e1742b9ee4 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py @@ -5,10 +5,9 @@ from unittest.mock import patch -from django.core.management import call_command from django.test import override_settings from edx_toggles.toggles.testutils import override_waffle_flag -from openedx_tagging.core.tagging.models import LanguageTaxonomy, Tag, Taxonomy +from openedx_tagging.core.tagging.models import Tag, Taxonomy from organizations.models import Organization from common.djangoapps.student.tests.factories import UserFactory @@ -16,16 +15,43 @@ from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase from .. import api -from ..models import TaxonomyOrg +from ..models.base import TaxonomyOrg from ..toggles import CONTENT_TAGGING_AUTO from ..types import ContentKey LANGUAGE_TAXONOMY_ID = -1 +class LanguageTaxonomyTestMixin: + """ + Mixin for test cases that expect the Language System Taxonomy to exist. + """ + + def setUp(self): + """ + When pytest runs, it creates the database by inspecting models, not by + running migrations. So data created by our migrations is not present. + In particular, the Language Taxonomy is not present. So this mixin will + create the taxonomy, simulating the effect of the following migrations: + 1. openedx_tagging.core.tagging.migrations.0012_language_taxonomy + 2. content_tagging.migrations.0007_system_defined_org_2 + """ + super().setUp() + Taxonomy.objects.get_or_create(id=-1, defaults={ + "name": "Languages", + "description": "Languages that are enabled on this system.", + "enabled": True, + "allow_multiple": False, + "allow_free_text": False, + "visible_to_authors": True, + "_taxonomy_class": "openedx_tagging.core.tagging.models.system_defined.LanguageTaxonomy", + }) + TaxonomyOrg.objects.get_or_create(taxonomy_id=-1, defaults={"org": None}) + + @skip_unless_cms # Auto-tagging is only available in the CMS @override_waffle_flag(CONTENT_TAGGING_AUTO, active=True) -class TestAutoTagging(ModuleStoreTestCase): +class TestAutoTagging(LanguageTaxonomyTestMixin, ModuleStoreTestCase): """ Test if the Course and XBlock tags are automatically created """ @@ -51,17 +77,6 @@ def _check_tag(self, object_key: ContentKey, taxonomy_id: int, value: str | None return True - @classmethod - def setUpClass(cls): - # Run fixtures to create the system defined tags - call_command("loaddata", "--app=oel_tagging", "language_taxonomy.yaml") - - # Enable Language taxonomy for all orgs - language_taxonomy = LanguageTaxonomy.objects.get(id=LANGUAGE_TAXONOMY_ID) - TaxonomyOrg.objects.create(taxonomy=language_taxonomy, org=None) - - super().setUpClass() - def setUp(self): super().setUp() # Create user diff --git a/requirements/constraints.txt b/requirements/constraints.txt index e80db0b54320..c9dfcc927eb1 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -121,7 +121,7 @@ libsass==0.10.0 click==8.1.6 # pinning this version to avoid updates while the library is being developed -openedx-learning==0.2.0 +openedx-learning==0.2.3 # lti-consumer-xblock 9.6.2 contains a breaking change that makes # existing custom parameter configurations unusable. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 2438510c2764..60dd1232f626 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -785,7 +785,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -openedx-learning==0.2.0 +openedx-learning==0.2.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 95c68b314c75..595ebb3e8765 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1318,7 +1318,7 @@ openedx-filters==1.6.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # lti-consumer-xblock -openedx-learning==0.2.0 +openedx-learning==0.2.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index daf088184cf9..3b6aa57299a2 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -925,7 +925,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.2.0 +openedx-learning==0.2.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index a728884e70d5..125de023503a 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -992,7 +992,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.2.0 +openedx-learning==0.2.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt