Fix import element sorting when saving (for related items)

rdmo/core/import_helpers.py

@@ -33,6 +33,8 @@ def get_value(self, **kwargs):
 
     @staticmethod
     def get_value_from_callback(callback, kwargs):
+        if not callable(callback):
+            raise TypeError('callback must be callable')
         sig = signature(callback)
         kwargs = {k: val for k, val in kwargs.items() if k in sig.parameters}
         value = callback(**kwargs)

rdmo/core/imports.py

@@ -9,7 +9,6 @@
 
 from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.db import models
-from django.db.models.fields import FloatField
 from django.utils.translation import gettext_lazy as _
 
 from rest_framework.utils import model_meta
@@ -135,19 +134,9 @@ def track_changes_on_element(element: dict,
 
     _initialize_track_changes_element_field(element, element_field)
 
-    lookup_model_field = None
     if original_value is None and original is not None:
         lookup_field = element_field if instance_field is None else instance_field
         original_value = getattr(original, lookup_field, '')
-        lookup_model_field = original._meta.get_field(lookup_field)
-
-    if isinstance(new_value,str) and isinstance(original_value,int):
-        # typecasting of original value to str, for comparison '0' == 0
-        # specific edge-case, maybe generalize later
-        original_value = str(original_value)
-
-    if isinstance(lookup_model_field, FloatField):
-        new_value = float(new_value)
 
     element[ImportElementFields.DIFF][element_field][ImportElementFields.CURRENT] = original_value
     element[ImportElementFields.DIFF][element_field][ImportElementFields.NEW] = new_value
@@ -213,7 +202,7 @@ def track_changes_on_uri_of_foreign_field(element, field_name, foreign_uri, orig
     track_changes_on_element(element, field_name, new_value=foreign_uri, original_value=original_foreign_uri)
 
 
-def set_foreign_field(instance, field_name, element, uploaded_uris=None, original=None) -> None:
+def set_foreign_field(instance, field_name, element, original=None) -> None:
     if field_name not in element:
         return
 
@@ -284,7 +273,8 @@ def set_foreign_field(instance, field_name, element, uploaded_uris=None, origina
 
 
 def set_extra_field(instance, field_name, element,
-                    extra_field_helper: Optional[ExtraFieldHelper] = None, original=None) -> None:
+                    extra_field_helper: Optional[ExtraFieldHelper] = None,
+                    ) -> None:
 
     extra_field_value = None
     if field_name in element:
@@ -308,9 +298,6 @@ def set_extra_field(instance, field_name, element,
 
     if extra_field_value is not None:
         setattr(instance, field_name, extra_field_value)
-        # track changes
-        track_changes_on_element(element, field_name, new_value=extra_field_value, original=original)
-
 
 def track_changes_m2m_instances(element, field_name,
                                 foreign_instances, original=None):

rdmo/domain/imports.py

@@ -18,7 +18,6 @@ def build_attribute_uri(instance: Optional[Attribute]=None):
         return instance.build_uri(instance.uri_prefix, instance.path)
 
 
-
 import_helper_attribute = ElementImportHelper(
     model=Attribute,
     common_fields=('uri_prefix', 'key', 'comment'),

rdmo/management/import_utils.py

@@ -1,8 +1,10 @@
+import logging
 from collections import defaultdict
 from dataclasses import asdict
 from typing import Dict, Set, Tuple
 
 from django.db.models import Model
+from django.forms import model_to_dict
 
 from rdmo.core.imports import (
     ImportElementFields,
@@ -13,8 +15,11 @@
     set_m2m_instances,
     set_m2m_through_instances,
     set_reverse_m2m_through_instance,
+    track_changes_on_element,
 )
 
+logger = logging.getLogger(__name__)
+
 IMPORT_ELEMENT_INIT_DICT = {
         ImportElementFields.WARNINGS: lambda: defaultdict(list),
         ImportElementFields.ERRORS: list,
@@ -24,6 +29,13 @@
     }
 
 
+def is_valid_import_element(element: dict) -> bool:
+    if element is None or not isinstance(element, dict):
+        return False
+    if not all(i in element for i in ['model', 'uri']):
+        return False
+    return True
+
 
 def get_redundant_keys_from_element(element_keys: Set, model: Model) -> Set:
     model_fields = {i.name for i in model._meta.get_fields()}
@@ -60,7 +72,7 @@ def strip_uri_prefix_endswith_slash(element: dict) -> dict:
     return element
 
 
-def apply_field_values(instance, element, import_helper, uploaded_uris, original) -> None:
+def apply_field_values(instance, element, import_helper, original) -> None:
     """Applies the field values from the element to the instance."""
     # start to set values on the instance
     # set common field values from element on instance
@@ -71,10 +83,58 @@ def apply_field_values(instance, element, import_helper, uploaded_uris, original
         set_lang_field(instance, field, element, original=original)
     # set foreign fields
     for field in import_helper.foreign_fields:
-        set_foreign_field(instance, field, element, uploaded_uris=uploaded_uris, original=original)
-
+        set_foreign_field(instance, field, element, original=original)
+    # set extra fields, track changes is done after instance.full_clean
     for extra_field in import_helper.extra_fields:
-        set_extra_field(instance, extra_field.field_name, element, extra_field_helper=extra_field, original=original)
+        set_extra_field(instance, extra_field.field_name, element,
+                        extra_field_helper=extra_field)
+
+
+def update_extra_fields_from_validated_instance(instance, element, import_helper, original=None) -> None:
+    extra_field_names = [i.field_name for i in import_helper.extra_fields]
+    instance_extra_fields = model_to_dict(instance, fields=extra_field_names)
+
+    for field_name in extra_field_names:
+        element_field_value = element.get(field_name)
+        if element_field_value is None:
+            continue
+
+        instance_field_value = instance_extra_fields[field_name]
+        # Properly handle boolean comparisons
+        if isinstance(instance_field_value, bool):
+            # Convert element field value to boolean if it's a string representation of a boolean
+            if isinstance(element_field_value, str):
+                element_field_value = element_field_value.strip().lower()
+                if element_field_value in ['true', '1', 'yes']:
+                    element_field_value = True
+                elif element_field_value in ['false', '0', 'no']:
+                    element_field_value = False
+                else:
+                    logger.debug("Cannot convert %s from %s to a boolean value.", element_field_value, field_name)
+
+            # Convert other types (e.g., integers) to boolean
+            elif isinstance(element_field_value, (int, float)):
+                element_field_value = bool(element_field_value)
+
+            # Assign the converted boolean value back to the element
+            element[field_name] = element_field_value
+
+        # Handle other data types and type casting
+        elif isinstance(element_field_value, str) and instance_field_value is not None:
+            try:
+                # Type cast element value to type of instance value
+                element[field_name] = type(instance_field_value)(element_field_value)
+            except ValueError:
+                logger.debug("Cannot convert '%s' from %s to %s.",
+                             element_field_value,
+                             field_name,
+                             type(instance_field_value).__name__
+                             )
+            except TypeError:
+                pass # ignore for NoneType
+
+        # track changes
+        track_changes_on_element(element, field_name, new_value=element[field_name], original=original)
 
 
 def update_related_fields(instance, element, import_helper, original, save) -> None:

rdmo/management/imports.py

@@ -1,13 +1,15 @@
 import copy
 import logging
-from typing import AbstractSet, Dict, List, Optional
+from collections import OrderedDict
+from typing import Dict, List, Optional
 
 from django.conf import settings
 from django.contrib.sites.shortcuts import get_current_site
 from django.http import HttpRequest
 
 from rdmo.conditions.imports import import_helper_condition
 from rdmo.core.imports import (
+    ImportElementFields,
     check_permissions,
     get_or_return_instance,
     make_import_info_msg,
@@ -19,7 +21,10 @@
     add_current_site_to_sites_and_editor,
     apply_field_values,
     initialize_and_clean_import_element_dict,
+    initialize_import_element_dict,
+    is_valid_import_element,
     strip_uri_prefix_endswith_slash,
+    update_extra_fields_from_validated_instance,
     update_related_fields,
 )
 from rdmo.options.imports import import_helper_option, import_helper_optionset
@@ -37,8 +42,8 @@
 
 ELEMENT_IMPORT_HELPERS = {
     "domain.attribute": import_helper_attribute,
-    "conditions.condition": import_helper_condition,
     "options.option": import_helper_option,
+    "conditions.condition": import_helper_condition,
     "options.optionset": import_helper_optionset,
     "questions.question": import_helper_question,
     "questions.questionset": import_helper_questionset,
@@ -50,27 +55,40 @@
 }
 
 
-def import_elements(uploaded_elements: Dict, save: bool = True, request: Optional[HttpRequest] = None) -> List[Dict]:
+def import_elements(uploaded_elements: OrderedDict,
+                    save: bool = True,
+                    request: Optional[HttpRequest] = None) -> List[Dict]:
     imported_elements = []
-    uploaded_elements_ordering_index = {uri: n for n, uri in enumerate(uploaded_elements.keys())}
+    uploaded_elements_initial_ordering = {uri: n for n, uri in enumerate(uploaded_elements.keys())}
     uploaded_uris = set(uploaded_elements.keys())
     current_site = get_current_site(request)
     if save:
         # when saving, the elements are ordered according to the rdmo models
-        uploaded_elements = order_elements(uploaded_elements, order_models=True)
+        pass
+        uploaded_elements = order_elements(uploaded_elements)
 
     for _uri, uploaded_element in uploaded_elements.items():
-        element = import_element(element=uploaded_element,
-                                 save=save,
-                                 uploaded_uris=uploaded_uris,
-                                 request=request,
-                                 current_site=current_site)
-        element['warnings'] = {k: val for k, val in element['warnings'].items() if k not in uploaded_uris}
+        if not is_valid_import_element(uploaded_element):
+            continue
+        element = import_element(
+            element=uploaded_element,
+            save=save,
+            request=request,
+            current_site=current_site
+        )
+        element[ImportElementFields.WARNINGS] = {
+            k: val for
+            k, val in element[ImportElementFields.WARNINGS].items()
+            if k not in uploaded_uris
+        }
         imported_elements.append(element)
 
-    # sort elements back to order of uploaded elements
-    imported_elements = sorted(imported_elements,
-                               key=lambda x: uploaded_elements_ordering_index.get(x['uri'], float('inf')))
+    # sort elements back to initial order of uploaded elements
+    imported_elements = sorted(
+        imported_elements,
+        key=lambda x: uploaded_elements_initial_ordering.get(x['uri'],
+        float('inf'))
+    )
 
     return imported_elements
 
@@ -79,16 +97,13 @@ def import_element(
         element: Optional[Dict] = None,
         save: bool = True,
         request: Optional[HttpRequest] = None,
-        uploaded_uris: Optional[AbstractSet[str]] = None,
         current_site = None
     ) -> Dict:
 
-    if element is None or not isinstance(element, dict):
-        return {}
-    if 'model' not in element:
-        return {}
+    initialize_import_element_dict(element)
 
     import_helper = ELEMENT_IMPORT_HELPERS[element['model']]
+
     uri = element.get('uri')
 
     element, _excluded_data = initialize_and_clean_import_element_dict(element, import_helper.model)
@@ -109,26 +124,27 @@ def import_element(
     perms_error_msg = check_permissions(instance, uri, user)
     if perms_error_msg:
         # when there is an error msg, the import can be stopped and return
-        element["errors"].append(perms_error_msg)
+        element[ImportElementFields.ERRORS].append(perms_error_msg)
         return element
 
-    element['created'] = created
-    element['updated'] = not created and original is not None
+    element[ImportElementFields.CREATED] = created
+    element[ImportElementFields.UPDATED] = not created and original is not None
     # INFO: the dict element[FieldNames.diff.value] is filled by calling track_changes_on_element
 
     element = strip_uri_prefix_endswith_slash(element)
-    # todo: remove keys that are not in model.fields
     # start to set values on the instance
-    apply_field_values(instance, element, import_helper, uploaded_uris, original)
+    apply_field_values(instance, element, import_helper, original)
 
     # call the validators on the instance
     validate_instance(instance, element, *import_helper.validators)
 
-    if element.get('errors'):
+    update_extra_fields_from_validated_instance(instance, element, import_helper, original=original)
+
+    if element.get(ImportElementFields.ERRORS):
         # when there is an error msg, the import can be stopped and return
         if save:
-            element['created'] = False
-            element['updated'] = False
+            element[ImportElementFields.CREATED] = False
+            element[ImportElementFields.UPDATED] = False
         return element
 
     if save:

rdmo/management/viewsets.py

@@ -72,8 +72,7 @@ def create(self, request, *args, **kwargs):
         # step 1: store xml file as tmp file
         try:
             elements_data = request.data['elements']
-            _elements = filter(lambda x: 'uri' in x, elements_data)
-            elements = {i['uri']: i for i in _elements}
+            elements = {i['uri']: i for i in elements_data if 'uri' in i}
         except KeyError as e:
             raise ValidationError({'elements': [_('This field may not be blank.')]}) from e
         except TypeError as e: