From 1ec198596c65c1624dd840ed07ab4b23362ad42b Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 3 Oct 2023 14:54:37 -0700 Subject: [PATCH 1/4] feat: remove Taxonomy.required, make allow_multiple True by default --- .../core/djangoapps/content_tagging/api.py | 2 - .../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 +- 11 files changed, 72 insertions(+), 41 deletions(-) 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/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..e5a916f1fda5 --- /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.create(taxonomy_id=-1, 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', '0006_simplify_models'), + ("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..9e55a8a743e5 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.create(taxonomy_id=-1, 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 bae9a5be22e1..382ccd64c117 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.1 # 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 e2e9014d326f..b8adc5e12378 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -783,7 +783,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -openedx-learning==0.2.0 +openedx-learning==0.2.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 2c95eabd1882..3dc2dae200fa 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1316,7 +1316,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.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index b636f128f415..3416983a8113 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -923,7 +923,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.2.0 +openedx-learning==0.2.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index c8a9dc113106..fd7bcf7a2df7 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -990,7 +990,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.2.0 +openedx-learning==0.2.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From ece987a4624010a775d90db158990faafa3005fb Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 9 Oct 2023 16:18:51 -0700 Subject: [PATCH 2/4] chore: squash migrations --- .../migrations/0001_squashed.py | 54 +++++++++++++++++++ .../migrations/0007_system_defined_org_2.py | 2 +- 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 openedx/core/djangoapps/content_tagging/migrations/0001_squashed.py 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..de9d78cbdcb6 --- /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", "0012_language_taxonomy"), + ('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/0007_system_defined_org_2.py b/openedx/core/djangoapps/content_tagging/migrations/0007_system_defined_org_2.py index e5a916f1fda5..d9b0034c1398 100644 --- 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 @@ -19,7 +19,7 @@ def revert_mark_language_taxonomy_as_all_orgs(apps, _schema_editor): class Migration(migrations.Migration): dependencies = [ - ('content_tagging', '0006_simplify_models'), + ('content_tagging', '0001_squashed'), ("oel_tagging", "0012_language_taxonomy"), ] From 95a4e60ff5915018f1d255de73436d37d3bd21e9 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 9 Oct 2023 23:57:17 -0700 Subject: [PATCH 3/4] fix: more robust migration Co-authored-by: Jillian --- .../content_tagging/migrations/0007_system_defined_org_2.py | 2 +- openedx/core/djangoapps/content_tagging/tests/test_tasks.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 index d9b0034c1398..0a48016ca2b4 100644 --- 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 @@ -6,7 +6,7 @@ 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.create(taxonomy_id=-1, org=None) + TaxonomyOrg.objects.update_or_create(taxonomy_id=-1, defaults={"org": None}) def revert_mark_language_taxonomy_as_all_orgs(apps, _schema_editor): diff --git a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py index 9e55a8a743e5..39e1742b9ee4 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py @@ -46,7 +46,7 @@ def setUp(self): "visible_to_authors": True, "_taxonomy_class": "openedx_tagging.core.tagging.models.system_defined.LanguageTaxonomy", }) - TaxonomyOrg.objects.create(taxonomy_id=-1, org=None) + TaxonomyOrg.objects.get_or_create(taxonomy_id=-1, defaults={"org": None}) @skip_unless_cms # Auto-tagging is only available in the CMS From 8215a5e6370aec5a48dfd870e831965826917d08 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 10 Oct 2023 00:01:06 -0700 Subject: [PATCH 4/4] chore: version bump, use squashed migration --- .../core/djangoapps/content_tagging/migrations/0001_squashed.py | 2 +- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/content_tagging/migrations/0001_squashed.py b/openedx/core/djangoapps/content_tagging/migrations/0001_squashed.py index de9d78cbdcb6..fa00df307c64 100644 --- a/openedx/core/djangoapps/content_tagging/migrations/0001_squashed.py +++ b/openedx/core/djangoapps/content_tagging/migrations/0001_squashed.py @@ -18,7 +18,7 @@ class Migration(migrations.Migration): initial = True dependencies = [ - ("oel_tagging", "0012_language_taxonomy"), + ("oel_tagging", "0001_squashed"), ('organizations', '0003_historicalorganizationcourse'), ] diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 382ccd64c117..2fac98df9904 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.1 +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 b8adc5e12378..2be7ed3d3e02 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -783,7 +783,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock -openedx-learning==0.2.1 +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 3dc2dae200fa..be09c38419ac 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1316,7 +1316,7 @@ openedx-filters==1.6.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # lti-consumer-xblock -openedx-learning==0.2.1 +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 3416983a8113..c37327990dc8 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -923,7 +923,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.2.1 +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 fd7bcf7a2df7..704ec4810a39 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -990,7 +990,7 @@ openedx-filters==1.6.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock -openedx-learning==0.2.1 +openedx-learning==0.2.3 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt