Merge pull request #9141 from rtibbles/foreign_languages_are_the_key

Delete any annotated channelmetadata many to many fields to avoid integrity errors

kolibri/core/content/test/test_channel_import.py

@@ -29,6 +29,7 @@
 from kolibri.core.content.models import ChannelMetadata
 from kolibri.core.content.models import ContentNode
 from kolibri.core.content.models import File
+from kolibri.core.content.models import Language
 from kolibri.core.content.models import LocalFile
 from kolibri.core.content.utils.annotation import recurse_annotation_up_tree
 from kolibri.core.content.utils.annotation import (
@@ -476,6 +477,17 @@ def test_residual_contentnode_deleted_after_reimport(self):
         with self.assertRaises(ContentNode.DoesNotExist):
             assert ContentNode.objects.get(pk=obj_id)
 
+    def test_residual_included_languages_deleted_after_reimport(self):
+        lang = Language.objects.create(id="en")
+        channel = ChannelMetadata.objects.first()
+        # Decrement current channel version to ensure reimport
+        channel.version -= 1
+        channel.included_languages.add(lang)
+        channel.save()
+        self.set_content_fixture()
+        channel = ChannelMetadata.objects.first()
+        self.assertEqual(channel.included_languages.count(), 0)
+
     def test_prerequisites_not_duplicated(self):
         prereqs = ContentNode.has_prerequisite.through.objects.all().count()
         channel = ChannelMetadata.objects.first()

kolibri/core/content/utils/channel_import.py

@@ -230,7 +230,14 @@ def __init__(
         if destination is None:
             # If no destination is set then we are targeting the default database
             self.destination = Bridge(
-                schema_version=CONTENT_SCHEMA_VERSION, app_name=CONTENT_APP_NAME
+                # It is a little counter intuitive that we are setting the schema version for the
+                # Django database *not* to be CURRENT_SCHEMA_VERSION, but we do so because
+                # this allows for precise mapping from channel database tables and columns to
+                # the Django database tables and columns. If we were to reference the current schema,
+                # then channel imports would fail because we did not properly specify the other
+                # fields that are annotated not imported.
+                schema_version=CONTENT_SCHEMA_VERSION,
+                app_name=CONTENT_APP_NAME,
             )
         else:
             # If a destination is set then pass that explicitly. At the moment, this only supports
@@ -729,8 +736,9 @@ def check_and_delete_existing_channel(self):
                     )
                 ).fetchone()
 
+                self.delete_old_channel_many_to_many_fields(self.channel_id)
                 if root_node:
-                    self.delete_old_channel_data(root_node["tree_id"])
+                    self.delete_old_channel_tree_data(root_node["tree_id"])
             else:
                 # We have previously loaded this channel, with the same or newer version, so our work here is done
                 logger.warn(
@@ -753,7 +761,21 @@ def _can_use_optimized_pre_deletion(self, model):
         table_mapper = self.generate_table_mapper(mapping.get("per_table"))
         return self.can_use_sqlite_attach_method(model, table_mapper)
 
-    def delete_old_channel_data(self, old_tree_id):
+    def delete_old_channel_many_to_many_fields(self, channel_id):
+        # Delete all many to many through entries for the channel
+        # being deleted to prevent referential integrity errors in Postgresql.
+        for m2m in ChannelMetadata._meta.local_many_to_many:
+            model = getattr(ChannelMetadata, m2m.attname).through
+            # Our destination's schema is set to the latest import schema version
+            # NOT the schema that precisely reflects the tables of the Django database.
+            # So here we reference the actual current schema for the Django database
+            # because annotated channel metadata many to many fields are not included
+            # in channel databases, and their information is annotated after import.
+            m2mtable = self.destination.get_current_table(model)
+            query = m2mtable.delete().where(m2mtable.c.channelmetadata_id == channel_id)
+            self.destination.execute(query)
+
+    def delete_old_channel_tree_data(self, old_tree_id):
 
         # we want to delete all content models, but not "merge models" (ones that might also be used by other channels), and ContentNode last
         models_to_delete = [

kolibri/core/content/utils/sqlalchemybridge.py

@@ -337,6 +337,12 @@ def get_table(self, DjangoModel):
         """
         return self.get_class(DjangoModel).__table__
 
+    def get_current_table(self, DjangoModel):
+        """
+        Convenience method to get a table for the Django database schema
+        """
+        return get_class(DjangoModel, BASES[CURRENT_SCHEMA_VERSION]).__table__
+
     def get_raw_connection(self):
         conn = self.get_connection()
         return conn.connection