Skip to content

Commit

Permalink
feat: remove Taxonomy.required, squash taxonomy migrations (#33438)
Browse files Browse the repository at this point in the history
* feat: remove Taxonomy.required, make allow_multiple True by default

* chore: squash migrations

* fix: more robust migration

Co-authored-by: Jillian <[email protected]>

* chore: version bump, use squashed migration

---------

Co-authored-by: Jillian <[email protected]>
  • Loading branch information
bradenmacdonald and pomegranited authored Oct 10, 2023
1 parent 565b34e commit d6e21a1
Show file tree
Hide file tree
Showing 12 changed files with 126 additions and 41 deletions.
2 changes: 0 additions & 2 deletions openedx/core/djangoapps/content_tagging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ def create_taxonomy(
name: str,
description: str = None,
enabled=True,
required=False,
allow_multiple=False,
allow_free_text=False,
) -> Taxonomy:
Expand All @@ -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,
)
Expand Down
Original file line number Diff line number Diff line change
@@ -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'),
),
]
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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),
]
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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,
},
)

Expand Down Expand Up @@ -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,
},
)

Expand Down Expand Up @@ -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)

Expand All @@ -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,
)
Expand Down
45 changes: 30 additions & 15 deletions openedx/core/djangoapps/content_tagging/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,53 @@

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
from openedx.core.djangolib.testing.utils import skip_unless_cms
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
"""
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/development.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/doc.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/testing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d6e21a1

Please sign in to comment.