mirror of
https://github.com/inventree/InvenTree.git
synced 2026-08-10 15:36:17 +00:00
Fix for data import "update" (#12513)
- Use partial update on serializer - Mark other fields as not required (pk only)
This commit is contained in:
@@ -237,7 +237,14 @@ class DataImportSession(models.Model):
|
|||||||
|
|
||||||
# Extract a "default" value for the field, if one exists
|
# Extract a "default" value for the field, if one exists
|
||||||
# Skip if one has already been provided by the user
|
# Skip if one has already been provided by the user
|
||||||
if field not in self.field_defaults and 'default' in field_def:
|
# Skip entirely when updating existing records - a model-level "creation"
|
||||||
|
# default (e.g. quantity=1) must not silently overwrite existing data
|
||||||
|
# for fields the user hasn't chosen to map or default
|
||||||
|
if (
|
||||||
|
not self.update_records
|
||||||
|
and field not in self.field_defaults
|
||||||
|
and 'default' in field_def
|
||||||
|
):
|
||||||
self.field_defaults[field] = field_def['default']
|
self.field_defaults[field] = field_def['default']
|
||||||
|
|
||||||
# Generate a list of possible column names for this field
|
# Generate a list of possible column names for this field
|
||||||
@@ -472,11 +479,14 @@ class DataImportSession(models.Model):
|
|||||||
required = {}
|
required = {}
|
||||||
|
|
||||||
for field, info in fields.items():
|
for field, info in fields.items():
|
||||||
if info.get('required', False):
|
if self.update_records:
|
||||||
required[field] = info
|
# When updating existing records, only the ID field is required -
|
||||||
|
# all other fields fall back to the existing value on the record
|
||||||
elif self.update_records and field == self.ID_FIELD_LABEL:
|
# being updated, so the serializer's usual "required" flag
|
||||||
# If we are updating records, the ID field is required
|
# (which applies to *creating* a new instance) does not apply here
|
||||||
|
if field == self.ID_FIELD_LABEL:
|
||||||
|
required[field] = info
|
||||||
|
elif info.get('required', False):
|
||||||
required[field] = info
|
required[field] = info
|
||||||
|
|
||||||
return required
|
return required
|
||||||
@@ -933,6 +943,11 @@ class DataImportRow(models.Model):
|
|||||||
return serializer_class(
|
return serializer_class(
|
||||||
instance=instance,
|
instance=instance,
|
||||||
data=self.serializer_data(),
|
data=self.serializer_data(),
|
||||||
|
# When updating an existing instance, treat this as a partial
|
||||||
|
# update - fields not present in the imported data should fall
|
||||||
|
# back to the existing value on the record, not fail validation
|
||||||
|
# or be overwritten with a blank/default value
|
||||||
|
partial=instance is not None,
|
||||||
context={'request': request},
|
context={'request': request},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -194,6 +194,68 @@ class ImporterTest(ImporterMixin, InvenTreeTestCase):
|
|||||||
with self.assertRaises(DjangoValidationError):
|
with self.assertRaises(DjangoValidationError):
|
||||||
session.full_clean()
|
session.full_clean()
|
||||||
|
|
||||||
|
def test_update_existing_records(self):
|
||||||
|
"""Test updating existing records via import, providing only the ID field.
|
||||||
|
|
||||||
|
Regression test for GH #12499: when updating existing records, only the
|
||||||
|
primary key should be required. Fields which are not mapped to a column
|
||||||
|
(and have no explicit override/default) must:
|
||||||
|
- not be flagged as "required" and block the mapping wizard
|
||||||
|
- not be auto-populated with a model-level "creation" default
|
||||||
|
- not overwrite the existing value on the target record
|
||||||
|
"""
|
||||||
|
from part.models import Part, PartCategory
|
||||||
|
from stock.models import StockItem
|
||||||
|
|
||||||
|
category = PartCategory.objects.create(
|
||||||
|
name='Update Test Category', description='Test category'
|
||||||
|
)
|
||||||
|
part = Part.objects.create(
|
||||||
|
category=category, name='Update Test Part', description='Test part'
|
||||||
|
)
|
||||||
|
|
||||||
|
item = StockItem.objects.create(part=part, quantity=42, batch='Original batch')
|
||||||
|
|
||||||
|
csv_content = f'ID,batch\n{item.pk},Updated batch\n'
|
||||||
|
data_file = ContentFile(csv_content, 'stock_update.csv')
|
||||||
|
|
||||||
|
session = DataImportSession.objects.create(
|
||||||
|
data_file=data_file, model_type='stockitem', update_records=True
|
||||||
|
)
|
||||||
|
|
||||||
|
# Only the 'id' field should be required - 'quantity' and 'part' already
|
||||||
|
# exist on the target record, and must not be forced
|
||||||
|
required = session.required_fields()
|
||||||
|
self.assertIn('id', required)
|
||||||
|
self.assertNotIn('quantity', required)
|
||||||
|
self.assertNotIn('part', required)
|
||||||
|
|
||||||
|
# No model-level "creation" default should have been auto-populated for
|
||||||
|
# 'quantity' - doing so would silently overwrite the existing value
|
||||||
|
self.assertNotIn('quantity', session.field_defaults or {})
|
||||||
|
|
||||||
|
# Mapping is valid, even though 'quantity' and 'part' are unmapped
|
||||||
|
session.accept_mapping()
|
||||||
|
|
||||||
|
session.refresh_from_db()
|
||||||
|
self.assertEqual(session.rows.count(), 1)
|
||||||
|
|
||||||
|
row = session.rows.first()
|
||||||
|
self.assertTrue(row.valid)
|
||||||
|
|
||||||
|
# Unmapped fields must not appear in the extracted row data at all
|
||||||
|
self.assertNotIn('quantity', row.data)
|
||||||
|
self.assertNotIn('part', row.data)
|
||||||
|
|
||||||
|
self.assertTrue(row.validate(commit=True))
|
||||||
|
self.assertTrue(row.complete)
|
||||||
|
|
||||||
|
# Existing values for unmapped fields must be preserved
|
||||||
|
item.refresh_from_db()
|
||||||
|
self.assertEqual(item.quantity, 42)
|
||||||
|
self.assertEqual(item.part, part)
|
||||||
|
self.assertEqual(item.batch, 'Updated batch')
|
||||||
|
|
||||||
def test_lookup_field_ambiguous_match(self):
|
def test_lookup_field_ambiguous_match(self):
|
||||||
"""Test the behavior of lookup_related_field for ambiguous and pinned matches."""
|
"""Test the behavior of lookup_related_field for ambiguous and pinned matches."""
|
||||||
from django.core.exceptions import ValidationError as DjangoValidationError
|
from django.core.exceptions import ValidationError as DjangoValidationError
|
||||||
|
|||||||
Reference in New Issue
Block a user