diff --git a/Makefile b/Makefile index 051053bab3..619fcee41e 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ migrate: # 4) Remove the management command from this `deploy-migrate` recipe # 5) Repeat! deploy-migrate: - echo "Nothing to do here!" + python contentcuration/manage.py rectify_incorrect_contentnode_source_fields contentnodegc: python contentcuration/manage.py garbage_collect diff --git a/contentcuration/contentcuration/tests/test_rectify_source_field_migraiton_command.py b/contentcuration/contentcuration/tests/test_rectify_source_field_migraiton_command.py new file mode 100644 index 0000000000..96382e25af --- /dev/null +++ b/contentcuration/contentcuration/tests/test_rectify_source_field_migraiton_command.py @@ -0,0 +1,167 @@ +# DELETE THIS FILE AFTER RUNNING THE MIGRATIONSSS +import datetime +import uuid + +from django.core.management import call_command +from django.utils import timezone +from le_utils.constants import content_kinds + +from contentcuration.models import Channel +from contentcuration.models import ContentNode +from contentcuration.models import License +from contentcuration.tests import testdata +from contentcuration.tests.base import StudioAPITestCase +from contentcuration.utils.publish import publish_channel + + +class TestRectifyMigrationCommand(StudioAPITestCase): + + @classmethod + def setUpClass(cls): + super(TestRectifyMigrationCommand, cls).setUpClass() + + def tearDown(self): + super(TestRectifyMigrationCommand, self).tearDown() + + def setUp(self): + super(TestRectifyMigrationCommand, self).setUp() + self.original_channel = testdata.channel() + self.license_original = License.objects.get(license_name="Special Permissions") + self.license_description_original = "License to chill" + self.original_contentnode = ContentNode.objects.create( + id=uuid.uuid4().hex, + title="Original Node", + parent=self.original_channel.main_tree, + license=self.license_original, + license_description=self.license_description_original, + original_channel_id=None, + source_channel_id=None, + author="old author" + ) + self.user = testdata.user() + self.original_channel.editors.add(self.user) + self.client.force_authenticate(user=self.user) + + def create_base_channel_and_contentnode(self, source_contentnode, source_channel): + base_channel = testdata.channel() + base_channel.public = True + base_channel.save() + base_node = ContentNode.objects.create( + id=uuid.uuid4().hex, + title="base contentnode", + parent=base_channel.main_tree, + kind_id=content_kinds.VIDEO, + original_channel_id=self.original_channel.id, + original_source_node_id=self.original_contentnode.node_id, + source_channel_id=source_channel.id, + source_node_id=source_contentnode.node_id, + author="source author", + license=self.license_original, + license_description=None, + ) + return base_node, base_channel + + def create_source_channel_and_contentnode(self): + source_channel = testdata.channel() + source_channel.public = True + source_channel.save() + source_node = ContentNode.objects.create( + id=uuid.uuid4().hex, + title="base contentnode", + parent=source_channel.main_tree, + kind_id=content_kinds.VIDEO, + license=self.license_original, + license_description="No chill", + original_channel_id=self.original_channel.id, + source_channel_id=self.original_channel.id, + source_node_id=self.original_contentnode.node_id, + original_source_node_id=self.original_contentnode.node_id, + author="source author", + ) + + return source_node, source_channel + + def run_migrations(self): + call_command('rectify_incorrect_contentnode_source_fields') + + def test_two_node_case(self): + base_node, base_channel = self.create_base_channel_and_contentnode(self.original_contentnode, self.original_channel) + + publish_channel(self.user.id, Channel.objects.get(pk=base_channel.pk).id) + + # main_tree node still has changed=true even after the publish + for node in Channel.objects.get(pk=base_channel.pk).main_tree.get_family().filter(changed=True): + node.changed = False + # This should probably again change the changed=true but suprisingly it doesnot + # Meaning the changed boolean doesnot change for the main_tree no matter what we do + # through ContentNode model methods like save. + node.save() + + ContentNode.objects.filter(pk=base_node.pk).update( + modified=datetime.datetime(2023, 7, 5, tzinfo=timezone.utc) + ) + + self.run_migrations() + updated_base_node = ContentNode.objects.get(pk=base_node.pk) + self.assertEqual(updated_base_node.license_description, self.original_contentnode.license_description) + self.assertEqual(Channel.objects.get(pk=base_channel.id).main_tree.get_family().filter(changed=True).exists(), True) + + def test_three_node_case_implicit(self): + source_node, source_channel = self.create_source_channel_and_contentnode() + base_node, base_channel = self.create_base_channel_and_contentnode(source_node, source_channel) + source_node.aggregator = "Nami" + source_node.save() + # Implicit case + base_node.author = source_node.author + base_node.license = source_node.license + base_node.aggregator = source_node.aggregator + base_node.save() + + publish_channel(self.user.id, Channel.objects.get(pk=base_channel.pk).id) + + for node in Channel.objects.get(pk=base_channel.pk).main_tree.get_family().filter(changed=True): + node.changed = False + node.save() + + ContentNode.objects.filter(pk=base_node.pk).update( + modified=datetime.datetime(2023, 7, 5, tzinfo=timezone.utc) + ) + + ContentNode.objects.filter(pk=source_node.pk).update( + modified=datetime.datetime(2023, 3, 5, tzinfo=timezone.utc) + ) + + self.run_migrations() + updated_base_node = ContentNode.objects.get(pk=base_node.pk) + updated_source_node = ContentNode.objects.get(pk=source_node.pk) + self.assertEqual(updated_base_node.license_description, self.original_contentnode.license_description) + self.assertEqual(updated_source_node.license_description, self.original_contentnode.license_description) + self.assertEqual(Channel.objects.get(pk=base_channel.id).main_tree.get_family().filter(changed=True).exists(), True) + + def test_three_node_case_explicit(self): + source_node, source_channel = self.create_source_channel_and_contentnode() + base_node, base_channel = self.create_base_channel_and_contentnode(source_node, source_channel) + source_node.license_description = "luffy" + base_node.license_description = "zoro" + base_node.save() + source_node.save() + publish_channel(self.user.id, Channel.objects.get(pk=base_channel.pk).id) + + for node in Channel.objects.get(pk=base_channel.pk).main_tree.get_family().filter(changed=True): + node.changed = False + node.save() + + ContentNode.objects.filter(pk=base_node.pk).update( + modified=datetime.datetime(2023, 7, 5, tzinfo=timezone.utc) + ) + + ContentNode.objects.filter(pk=source_node.pk).update( + modified=datetime.datetime(2023, 3, 5, tzinfo=timezone.utc) + ) + + self.run_migrations() + updated_base_node = ContentNode.objects.get(pk=base_node.pk) + updated_source_node = ContentNode.objects.get(pk=source_node.pk) + self.assertEqual(updated_base_node.license_description, self.original_contentnode.license_description) + self.assertEqual(updated_source_node.license_description, self.original_contentnode.license_description) + self.assertEqual(Channel.objects.get(pk=base_channel.id).main_tree.get_family().filter(changed=True).exists(), True) diff --git a/contentcuration/contentcuration/tests/test_sync.py b/contentcuration/contentcuration/tests/test_sync.py index 3a2ba590c5..923d8ad541 100644 --- a/contentcuration/contentcuration/tests/test_sync.py +++ b/contentcuration/contentcuration/tests/test_sync.py @@ -19,6 +19,7 @@ from contentcuration.models import Channel from contentcuration.models import ContentTag from contentcuration.models import File +from contentcuration.models import License from contentcuration.tests import testdata from contentcuration.tests.base import StudioAPITestCase from contentcuration.tests.viewsets.base import generate_create_event @@ -346,6 +347,43 @@ def test_sync_channel_titles_and_descriptions(self): for key, value in labels.items(): self.assertEqual(getattr(target_child, key), value) + def test_sync_license_description(self): + """ + Test that the license description field is synced correctly + Added as a regression test, as this was previously omitted. + """ + self.assertFalse(self.channel.has_changes()) + self.assertFalse(self.derivative_channel.has_changes()) + + contentnode = ( + self.channel.main_tree.get_descendants() + .exclude(kind_id=content_kinds.TOPIC) + .first() + ) + + special_permissions_license = License.objects.get(license_name="Special Permissions") + + contentnode.license = special_permissions_license + contentnode.license_description = "You cannot use this content on a Thursday" + contentnode.copyright_holder = "Thursday's child has far to go" + contentnode.save() + + sync_channel( + self.derivative_channel, + sync_titles_and_descriptions=False, + sync_resource_details=True, + sync_files=False, + sync_assessment_items=False, + ) + + target_child = self.derivative_channel.main_tree.get_descendants().get( + source_node_id=contentnode.node_id + ) + + self.assertEqual(target_child.license, special_permissions_license) + self.assertEqual(target_child.license_description, "You cannot use this content on a Thursday") + self.assertEqual(target_child.copyright_holder, "Thursday's child has far to go") + def test_sync_channel_other_metadata_labels(self): """ Test that calling sync channel will also bring in other metadata label updates. diff --git a/contentcuration/contentcuration/utils/sync.py b/contentcuration/contentcuration/utils/sync.py index 5b3664002a..1fb2dda566 100644 --- a/contentcuration/contentcuration/utils/sync.py +++ b/contentcuration/contentcuration/utils/sync.py @@ -70,6 +70,7 @@ def sync_node( if sync_resource_details: fields = [ "license_id", + "license_description", "copyright_holder", "author", "extra_fields", diff --git a/contentcuration/kolibri_public/management/commands/rectify_incorrect_contentnode_source_fields.py b/contentcuration/kolibri_public/management/commands/rectify_incorrect_contentnode_source_fields.py new file mode 100644 index 0000000000..c4d40af4c0 --- /dev/null +++ b/contentcuration/kolibri_public/management/commands/rectify_incorrect_contentnode_source_fields.py @@ -0,0 +1,92 @@ +import logging + +from django.core.management.base import BaseCommand +from django.db.models import Exists +from django.db.models import F +from django.db.models import OuterRef +from django.db.models import Value +from django.db.models.functions import Coalesce +from django_cte import With + +from contentcuration.models import Channel +from contentcuration.models import ContentNode + +logger = logging.getLogger(__file__) + + +class Command(BaseCommand): + + def handle(self, *args, **options): + + main_trees_cte = With( + ( + Channel.objects.filter( + main_tree__isnull=False + ) + .annotate(channel_id=F("id")) + .values("channel_id", "deleted", tree_id=F("main_tree__tree_id")) + ), + name="main_trees", + ) + + nodes = main_trees_cte.join( + ContentNode.objects.all(), + tree_id=main_trees_cte.col.tree_id, + ).annotate(channel_id=main_trees_cte.col.channel_id, deleted=main_trees_cte.col.deleted) + + original_source_nodes = ( + nodes.with_cte(main_trees_cte) + .filter( + node_id=OuterRef("original_source_node_id"), + ) + .exclude( + tree_id=OuterRef("tree_id"), + ) + .annotate( + coalesced_license_description=Coalesce("license_description", Value("")), + ) + ) + diff = ( + nodes.with_cte(main_trees_cte).filter( + deleted=False, # we dont want the channel to be deleted or else we are fixing ghost nodes + source_node_id__isnull=False, + original_source_node_id__isnull=False, + ) + ).annotate( + coalesced_license_description=Coalesce("license_description", Value("")), + ) + diff_combined = diff.annotate( + original_source_node_f_changed=Exists( + original_source_nodes.exclude( + coalesced_license_description=OuterRef("coalesced_license_description") + ) + ) + ).filter(original_source_node_f_changed=True) + + final_nodes = diff_combined.values( + "id", + "channel_id", + "original_channel_id", + "original_source_node_id", + ).order_by() + + for item in final_nodes: + base_node = ContentNode.objects.get(pk=item["id"]) + + original_source_channel_id = item["original_channel_id"] + original_source_node_id = item["original_source_node_id"] + tree_id = ( + Channel.objects.filter(pk=original_source_channel_id) + .values_list("main_tree__tree_id", flat=True) + .get() + ) + original_source_node = ContentNode.objects.filter( + tree_id=tree_id, node_id=original_source_node_id + ) + + if original_source_channel_id is not None and original_source_node.exists(): + # original source node exists and its license_description doesn't match + # update the base node + if base_node.license_description != original_source_node[0].license_description: + base_node.license_description = original_source_node[0].license_description + base_node.save()