Data import fix (#12568)

* Specify JSON encoder for data import fields

- Fixes encoding issues when importing from XLSX file

* Add regression test

* Fix faulty error handler

* Fix for unit test
This commit is contained in:
Oliver
2026-08-09 08:19:19 +10:00
committed by GitHub
parent 901dc024c0
commit afcc89ecfb
3 changed files with 181 additions and 5 deletions
@@ -0,0 +1,88 @@
# Generated by Django 5.2.16 on 2026-08-08 01:36
import django.core.serializers.json
import importer.validators
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
("importer", "0007_dataimportsession_completed_row_count_history_and_more"),
]
operations = [
migrations.AlterField(
model_name="dataimportrow",
name="data",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
null=True,
verbose_name="Data",
),
),
migrations.AlterField(
model_name="dataimportrow",
name="errors",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
null=True,
verbose_name="Errors",
),
),
migrations.AlterField(
model_name="dataimportrow",
name="row_data",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
null=True,
verbose_name="Original row data",
),
),
migrations.AlterField(
model_name="dataimportsession",
name="columns",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
null=True,
verbose_name="Columns",
),
),
migrations.AlterField(
model_name="dataimportsession",
name="field_defaults",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
null=True,
validators=[importer.validators.validate_field_defaults],
verbose_name="Field Defaults",
),
),
migrations.AlterField(
model_name="dataimportsession",
name="field_filters",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
null=True,
validators=[importer.validators.validate_field_defaults],
verbose_name="Field Filters",
),
),
migrations.AlterField(
model_name="dataimportsession",
name="field_overrides",
field=models.JSONField(
blank=True,
encoder=django.core.serializers.json.DjangoJSONEncoder,
null=True,
validators=[importer.validators.validate_field_defaults],
verbose_name="Field Overrides",
),
),
]
+22 -5
View File
@@ -8,6 +8,7 @@ from typing import Optional
from django.contrib.auth.models import User
from django.core.exceptions import FieldDoesNotExist
from django.core.exceptions import ValidationError as DjangoValidationError
from django.core.serializers.json import DjangoJSONEncoder
from django.core.validators import FileExtensionValidator
from django.db import models, transaction
from django.urls import reverse
@@ -82,7 +83,9 @@ class DataImportSession(models.Model):
],
)
columns = models.JSONField(blank=True, null=True, verbose_name=_('Columns'))
columns = models.JSONField(
blank=True, null=True, verbose_name=_('Columns'), encoder=DjangoJSONEncoder
)
model_type = models.CharField(
blank=False,
@@ -107,6 +110,7 @@ class DataImportSession(models.Model):
null=True,
verbose_name=_('Field Defaults'),
validators=[importer.validators.validate_field_defaults],
encoder=DjangoJSONEncoder,
)
field_overrides = models.JSONField(
@@ -114,6 +118,7 @@ class DataImportSession(models.Model):
null=True,
verbose_name=_('Field Overrides'),
validators=[importer.validators.validate_field_defaults],
encoder=DjangoJSONEncoder,
)
field_filters = models.JSONField(
@@ -121,6 +126,7 @@ class DataImportSession(models.Model):
null=True,
verbose_name=_('Field Filters'),
validators=[importer.validators.validate_field_defaults],
encoder=DjangoJSONEncoder,
)
update_records = models.BooleanField(
@@ -668,12 +674,19 @@ class DataImportRow(models.Model):
row_index = models.PositiveIntegerField(default=0, verbose_name=_('Row Index'))
row_data = models.JSONField(
blank=True, null=True, verbose_name=_('Original row data')
blank=True,
null=True,
verbose_name=_('Original row data'),
encoder=DjangoJSONEncoder,
)
data = models.JSONField(blank=True, null=True, verbose_name=_('Data'))
data = models.JSONField(
blank=True, null=True, verbose_name=_('Data'), encoder=DjangoJSONEncoder
)
errors = models.JSONField(blank=True, null=True, verbose_name=_('Errors'))
errors = models.JSONField(
blank=True, null=True, verbose_name=_('Errors'), encoder=DjangoJSONEncoder
)
valid = models.BooleanField(default=False, verbose_name=_('Valid'))
@@ -762,7 +775,11 @@ class DataImportRow(models.Model):
field, value, lookup_field=field_lookup_mapping.get(field)
)
except DjangoValidationError as exc:
extract_errors[field] = exc.message
# exc.message only exists if the error was raised with a single
# message string - lookup_related_field may also raise with a
# dict (message_dict) or list (message), so use exc.messages,
# which normalizes any construction to a flat list of strings.
extract_errors[field] = '; '.join(exc.messages)
continue
# Use the default value, if provided
+71
View File
@@ -1,5 +1,6 @@
"""Unit tests for the 'importer' app."""
import datetime
import os
import threading
from unittest import mock
@@ -81,6 +82,37 @@ class ImporterTest(ImporterMixin, InvenTreeTestCase):
# Check that the new companies have been created
self.assertEqual(n + 12, Company.objects.count())
def test_row_data_datetime_serialization(self):
"""Test that row data containing native datetime values can be saved.
Regression test for a Sentry crash: "Object of type datetime is not JSON
serializable when serializing dict item 'COMMENT'". Excel imports (via
tablib/openpyxl) return date-formatted cells as native datetime.datetime
objects rather than strings. These flow unmodified into row_data, which
is then persisted via DataImportRow.objects.bulk_create().
DataImportRow.row_data / data / errors (and the related JSONFields on
DataImportSession) previously had no `encoder=DjangoJSONEncoder`, so
Django's JSONField fell back to the plain stdlib json.JSONEncoder, which
cannot serialize datetime objects - raising a TypeError on save/bulk_create.
"""
data_file = self.helper_file('companies.csv')
session = DataImportSession.objects.create(
data_file=data_file, model_type='company'
)
row = DataImportRow(
session=session,
row_index=0,
row_data={'COMMENT': datetime.datetime(2024, 5, 1, 12, 30)},
)
# This must not raise: TypeError: Object of type datetime is not JSON serializable
DataImportRow.objects.bulk_create([row])
row = DataImportRow.objects.get(session=session, row_index=0)
self.assertEqual(row.row_data['COMMENT'], '2024-05-01T12:30:00')
def test_import_header_whitespace(self):
"""Test that column headers with leading/trailing whitespace are handled correctly.
@@ -338,6 +370,45 @@ class ImporterTest(ImporterMixin, InvenTreeTestCase):
result = row.lookup_related_field('part', 'AMBIG-001', lookup_field='name')
self.assertNotEqual(result, part_a.pk)
def test_extract_data_related_field_validation_error(self):
"""Test that extract_data() handles a dict-constructed ValidationError.
Regression test: django.core.exceptions.ValidationError only exposes a
`.message` attribute when it was raised with a single message string.
lookup_related_field can also raise with a dict (e.g. "no related model
found for field") or a list, in which case `.message` does not exist and
accessing it raises: AttributeError: 'ValidationError' object has no
attribute 'message'. extract_data() must use `.messages` instead, which
normalizes any construction to a flat list of strings.
"""
from django.core.exceptions import ValidationError as DjangoValidationError
data_file = self.helper_file('companies.csv')
session = DataImportSession.objects.create(
data_file=data_file, model_type='stockitem'
)
row = DataImportRow(session=session, row_data={'Website': 'foo'})
field_mapping = {'part': 'Website'}
available_fields = {'part': {'type': 'related field'}}
with mock.patch.object(
DataImportRow,
'lookup_related_field',
side_effect=DjangoValidationError({
'session': 'No related model found for field: part'
}),
):
# Must not raise: AttributeError: 'ValidationError' object has no attribute 'message'
row.extract_data(
field_mapping=field_mapping,
available_fields=available_fields,
commit=False,
)
self.assertEqual(row.errors, {'part': 'No related model found for field: part'})
def test_lookup_field_validation(self):
"""Test that DataImportColumnMap.clean() validates the lookup_field value."""
from django.core.exceptions import ValidationError as DjangoValidationError