From e5dfc2c05fb4fda940a774d904b6cf8a17be9030 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Tue, 20 Feb 2024 10:53:24 -0500 Subject: [PATCH 1/3] Fix bulk_update() on I18n_JSON fields for homogenous values --- arches/app/models/fields/i18n.py | 14 +++++++++ tests/localization/field_tests.py | 47 ++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/arches/app/models/fields/i18n.py b/arches/app/models/fields/i18n.py index 08cbf364973..1b2135d5dbb 100644 --- a/arches/app/models/fields/i18n.py +++ b/arches/app/models/fields/i18n.py @@ -5,6 +5,7 @@ from django.utils.translation import gettext_lazy as _ from django.db.migrations.serializer import BaseSerializer, Serializer from django.db.models import JSONField +from django.db.models.functions.comparison import Cast from django.db.models.sql.compiler import SQLInsertCompiler from django.db.models.sql.where import NothingNode from django.utils.translation import get_language @@ -237,6 +238,17 @@ def __init__(self, value=None, lang=None, use_nulls=False, attname=None): def _parse(self, value, lang, use_nulls): ret = {} + if isinstance(value, Cast): + # Django 4.2 regression: bulk_update() sends Cast expressions + # https://code.djangoproject.com/ticket/35167 + values = set(case.result.value for case in value.source_expressions[0].cases) + value = list(values)[0] + if len(values) > 1: + # Prevent silent data loss. + raise NotImplementedError( + "Heterogenous values provided to I18n_JSON field bulk_update():\n" + f"{tuple(str(v) for v in values)}" + ) if isinstance(value, str): try: ret = json.loads(value) @@ -248,6 +260,8 @@ def _parse(self, value, lang, use_nulls): ret = value.raw_value elif isinstance(value, dict): ret = value + else: + raise TypeError(value) self.raw_value = ret if "i18n_properties" in self.raw_value: diff --git a/tests/localization/field_tests.py b/tests/localization/field_tests.py index a2ef3fe69d6..bc1d6527940 100644 --- a/tests/localization/field_tests.py +++ b/tests/localization/field_tests.py @@ -1,4 +1,7 @@ import json +import unittest + +from arches.app.models.models import DDataType from arches.app.models.fields.i18n import I18n_String, I18n_TextField, I18n_JSON, I18n_JSONField from tests.base_test import ArchesTestCase from django.contrib.gis.db import models @@ -6,7 +9,7 @@ from django.db import connection # these tests can be run from the command line via -# python manage.py test tests/localization/field_tests.py --pattern="*.py" --settings="tests.test_settings" +# python manage.py test tests.localization.field_tests --settings="tests.test_settings" class Customi18nTextFieldTests(ArchesTestCase): @@ -371,3 +374,45 @@ def test_i18nJSONField_can_handle_different_initial_states(self): m.save() m = self.LocalizationTestJsonModel.objects.get(pk=3) self.assertEqual(m.config.raw_value, expected_output_json) + + +class I18nJSONFieldBulkUpdateTests(ArchesTestCase): + def test_bulk_update_node_config_homogenous_value(self): + new_config = I18n_JSON({ + "en": "some", + "zh": "json", + }) + for_bulk_update = [] + for dt in DDataType.objects.all()[:3]: + dt.defaultconfig = new_config + for_bulk_update.append(dt) + + DDataType.objects.bulk_update(for_bulk_update, fields=["defaultconfig"]) + + for i, obj in enumerate(for_bulk_update): + with self.subTest(obj_index=i): + obj.refresh_from_db() + self.assertEqual(str(obj.defaultconfig), str(new_config)) + + @unittest.skip("https://github.com/archesproject/arches/issues/10619") + def test_bulk_update_heterogenous_values(self): + new_configs = [ + I18n_JSON({ + "en": "some", + "zh": "json", + }), + I18n_JSON({}), + None, + ] + for_bulk_update = [] + for i, dt in enumerate(DDataType.objects.all()[:3]): + dt.defaultconfig = new_configs[i] + for_bulk_update.append(dt) + + DDataType.objects.bulk_update(for_bulk_update, fields=["defaultconfig"]) + + for i, obj in enumerate(for_bulk_update): + new_config_as_string = str(new_configs[i]) + with self.subTest(new_config=new_config_as_string): + obj.refresh_from_db() + self.assertEqual(str(obj.defaultconfig), new_config_as_string) From 20ab2a7c4306a6e09e8823272b36fbb12301e5ec Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Thu, 22 Feb 2024 15:44:06 -0500 Subject: [PATCH 2/3] nit re #10618, prefer expectedFailure vs skip --- tests/localization/field_tests.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/localization/field_tests.py b/tests/localization/field_tests.py index bc1d6527940..2264fbc43a2 100644 --- a/tests/localization/field_tests.py +++ b/tests/localization/field_tests.py @@ -394,8 +394,11 @@ def test_bulk_update_node_config_homogenous_value(self): obj.refresh_from_db() self.assertEqual(str(obj.defaultconfig), str(new_config)) - @unittest.skip("https://github.com/archesproject/arches/issues/10619") + @unittest.expectedFailure def test_bulk_update_heterogenous_values(self): + """Situation may improve in Django 5.1? + https://github.com/archesproject/arches/issues/10619 + """ new_configs = [ I18n_JSON({ "en": "some", From 5bea7993110a7daf8a6fd5c9f1e8821d036009bc Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Fri, 23 Feb 2024 08:54:58 -0500 Subject: [PATCH 3/3] Provide a hint for the future if Django bug fixed re #10618 --- tests/localization/field_tests.py | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/tests/localization/field_tests.py b/tests/localization/field_tests.py index 2264fbc43a2..37ad39f6f80 100644 --- a/tests/localization/field_tests.py +++ b/tests/localization/field_tests.py @@ -1,5 +1,4 @@ import json -import unittest from arches.app.models.models import DDataType from arches.app.models.fields.i18n import I18n_String, I18n_TextField, I18n_JSON, I18n_JSONField @@ -394,11 +393,7 @@ def test_bulk_update_node_config_homogenous_value(self): obj.refresh_from_db() self.assertEqual(str(obj.defaultconfig), str(new_config)) - @unittest.expectedFailure def test_bulk_update_heterogenous_values(self): - """Situation may improve in Django 5.1? - https://github.com/archesproject/arches/issues/10619 - """ new_configs = [ I18n_JSON({ "en": "some", @@ -412,10 +407,20 @@ def test_bulk_update_heterogenous_values(self): dt.defaultconfig = new_configs[i] for_bulk_update.append(dt) - DDataType.objects.bulk_update(for_bulk_update, fields=["defaultconfig"]) + with self.assertRaises(NotImplementedError): + DDataType.objects.bulk_update(for_bulk_update, fields=["defaultconfig"]) - for i, obj in enumerate(for_bulk_update): - new_config_as_string = str(new_configs[i]) - with self.subTest(new_config=new_config_as_string): - obj.refresh_from_db() - self.assertEqual(str(obj.defaultconfig), new_config_as_string) + # If the above starts failing, it's likely the underlying Django + # regression was fixed. + # https://code.djangoproject.com/ticket/35167 + + # In that case, remove the with statement, de-indent the bulk_update, + # and comment the following code back in: + + # for i, obj in enumerate(for_bulk_update): + # new_config_as_string = str(new_configs[i]) + # with self.subTest(new_config=new_config_as_string): + # obj.refresh_from_db() + # self.assertEqual(str(obj.defaultconfig), new_config_as_string) + + # Also consider removing the code at the top of I18n_JSON._parse()