Merge pull request #12709 from jredrejo/fix_channel_import_in_pg

Fix import channel in Postgresql

docs/conf.py

@@ -95,7 +95,13 @@ def process_docstring(app, what, name, obj, options, lines):
 
 # Add any Sphinx extension module names here, as strings. They can be extensions
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
-extensions = ["sphinx.ext.autodoc", "sphinx.ext.viewcode", "m2r", "notfound.extension"]
+extensions = [
+    "sphinx.ext.autodoc",
+    "sphinx.ext.viewcode",
+    "m2r",
+    "notfound.extension",
+    "sphinxcontrib.jquery",
+]
 
 linkcheck_ignore = [
     "https://groups.google.com/a/learningequality.org/forum/#!forum/dev"

kolibri/core/content/fixtures/longdescriptions_content_data.json

@@ -91,6 +91,14 @@
     {
       "id": "222455c2cc484298b1501a13e1c7eb5c",
       "tag_name": "tag_3"
+    },
+    {
+      "id": "9e4760e53568402bb287dcdb8466758a",
+      "tag_name": "transformación de energía"
+    },
+    {
+      "id": "0c20e2eb254b4070a713da63380ff0a3",
+      "tag_name": "velocidad de reacciones químicas"
     }
   ],
   "content_contentnode": [

kolibri/core/content/test/test_channel_import.py

@@ -3,9 +3,11 @@
 import logging
 import os
 import tempfile
+import unittest
 import uuid
 
 import pytest
+from django.conf import settings
 from django.core.management import call_command
 from django.test import TestCase
 from django.test import TransactionTestCase
@@ -30,6 +32,7 @@
 from kolibri.core.content.models import AssessmentMetaData
 from kolibri.core.content.models import ChannelMetadata
 from kolibri.core.content.models import ContentNode
+from kolibri.core.content.models import ContentTag
 from kolibri.core.content.models import File
 from kolibri.core.content.models import Language
 from kolibri.core.content.models import LocalFile
@@ -832,6 +835,9 @@ class ImportLongDescriptionsTestCase(ContentImportTestBase, TransactionTestCase)
     data_name = "longdescriptions"
 
     longdescription = "soverylong" * 45
+    long_tag_id = "0c20e2eb254b4070a713da63380ff0a3"
+    utf_tag_id = "9e4760e53568402bb287dcdb8466758a"
+    utf_tag_name = "transformación de energía"
 
     def test_long_descriptions(self):
         self.assertEqual(
@@ -845,6 +851,32 @@ def test_long_descriptions(self):
             self.longdescription,
         )
 
+    @unittest.skipIf(
+        getattr(settings, "DATABASES")["default"]["ENGINE"]
+        != "django.db.backends.postgresql",
+        "Postgresql only test",
+    )
+    def test_import_too_long_content_tags(self):
+        """
+        Test that importing content tags with overly long tag_name fields will truncate correctly.
+        """
+        max_length = ContentTag._meta.get_field("tag_name").max_length
+        long_imported_tag = ContentTag.objects.get(id=self.long_tag_id)
+        assert len(long_imported_tag.tag_name) == max_length
+
+    @unittest.skipIf(
+        getattr(settings, "DATABASES")["default"]["ENGINE"]
+        != "django.db.backends.postgresql",
+        "Postgresql only test",
+    )
+    def test_utf_test_keeps_its_length(self):
+        """
+        Test inserting non-English chars keeps length
+        of the strings as are not decoded into bytes.
+        """
+        imported_tag = ContentTag.objects.get(id=self.utf_tag_id)
+        assert len(imported_tag.tag_name) == len(self.utf_tag_name)
+
 
 class Version4ImportTestCase(NaiveImportTestCase):
     """

kolibri/core/content/utils/channel_import.py

@@ -8,6 +8,7 @@
 from django.db.models.fields.related import ForeignKey
 from sqlalchemy import and_
 from sqlalchemy import or_
+from sqlalchemy import String as sa_String
 from sqlalchemy.dialects.postgresql import insert
 from sqlalchemy.exc import OperationalError
 from sqlalchemy.exc import SQLAlchemyError
@@ -598,7 +599,14 @@ def postgres_table_import(self, model, row_mapper, table_mapper):
         def generate_data_with_default(record):
             for col_name, column_obj in columns:
                 default = self.get_and_set_column_default(column_obj)
-                value = row_mapper(record, column_obj.name)
+                value = row_mapper(record, col_name)
+                if not column_obj.primary_key and isinstance(
+                    column_obj.type, sa_String
+                ):
+                    max_length = column_obj.type.length
+                    if max_length is not None:
+                        value = value[:max_length] if value is not None else default
+
                 yield value if value is not None else default
 
         if not merge:
@@ -624,7 +632,6 @@ def generate_data_with_default(record):
             )
         else:
             # Import here so that we don't need to depend on psycopg2 for Kolibri in general.
-            from psycopg2.extras import execute_values
 
             pk_name = DestinationTable.primary_key.columns.values()[0].name
 
@@ -647,13 +654,23 @@ def generate_data_with_default(record):
                         )
                     )
                 else:
-                    execute_values(
-                        cursor,
-                        # We want to overwrite new values that we are inserting here, so we use an ON CONFLICT DO UPDATE here
-                        # for the resulting SET statement, we generate a statement for each column we are trying to update
-                        "INSERT INTO {table} AS SOURCE ({column_names}) VALUES %s ON CONFLICT ({pk_name}) DO UPDATE SET {set_statement};".format(
+                    list_of_values = (
+                        tuple(datum for datum in generate_data_with_default(record))
+                        for record in results_slice
+                    )
+                    values_str = ", ".join(
+                        cursor.mogrify(
+                            f"({', '.join(['%s'] * len(column_names))})", v
+                        ).decode("utf-8")
+                        for v in list_of_values
+                    )
+                    insert_sql = (
+                        "INSERT INTO {table} AS SOURCE ({column_names}) "
+                        "VALUES {values_str} "
+                        "ON CONFLICT ({pk_name}) DO UPDATE SET {set_statement};".format(
                             table=DestinationTable.name,
                             column_names=", ".join(column_names),
+                            values_str=values_str,
                             pk_name=pk_name,
                             set_statement=", ".join(
                                 [
@@ -670,13 +687,10 @@ def generate_data_with_default(record):
                                     if column_name != pk_name
                                 ]
                             ),
-                        ),
-                        (
-                            tuple(datum for datum in generate_data_with_default(record))
-                            for record in results_slice
-                        ),
-                        template="(" + "%s, " * (len(columns) - 1) + "%s)",
+                        )
                     )
+                    cursor.execute(insert_sql)
+
                 i += BATCH_SIZE
                 results_slice = list(islice(results, i, i + BATCH_SIZE))
         cursor.close()

requirements/docs.txt

@@ -1,9 +1,12 @@
 -r base.txt
 
 # These are for building the docs
-sphinx
+
+# Sphinx stack requires a version of requests that's incompatible with Morango, so downgrading
+sphinx>=6,<7
 sphinx-intl
-sphinx-rtd-theme
+# We want to ensure the latest version of sphinx-rtd-theme that has sphinxcontrib-jquery as a dependency
+sphinx-rtd-theme~=2.0
 sphinx-autobuild
 m2r
 sphinx-notfound-page