ChangedObject: save objects and delete objects

src/c3nav/editor/models/changedobject.py

@@ -3,6 +3,7 @@ from itertools import chain
 
 from django.contrib.contenttypes.models import ContentType
 from django.db import models
+from django.db.models import Field
 from django.utils.translation import ugettext_lazy as _
 
 from c3nav.editor.utils import is_created_pk
@@ -76,28 +77,6 @@ class ChangedObject(models.Model):
         if hasattr(model._meta.pk, 'related_model'):
             setattr(obj, model._meta.pk.related_model._meta.pk.attname, pk)
         obj._state.adding = False
-
-        for name, value in self.updated_fields.items():
-            if name.startswith('title_'):
-                if value:
-                    obj.titles[name[6:]] = value
-                continue
-
-            field = model._meta.get_field(name)
-
-            if field.many_to_many:
-                continue
-
-            if field.many_to_one:
-                setattr(obj, field.attname, value)
-                if is_created_pk(value):
-                    setattr(obj, field.get_cache_name(), self.changeset.get_created_object(field.related_model, value))
-                elif get_foreign_objects:
-                    related_obj = self.changeset.wrap_model(field.related_model).objects.get(pk=value)
-                    setattr(obj, field.get_cache_name(), related_obj)
-                continue
-
-            setattr(obj, name, field.to_python(value))
         return self.changeset.wrap_instance(obj)
 
     def add_relevant_object_pks(self, object_pks):
@@ -141,17 +120,105 @@ class ChangedObject(models.Model):
             self.changeset.m2m_added.get(model, {}).pop(pk, None)
             self.changeset.m2m_removed.get(model, {}).pop(pk, None)
 
+    def apply_to_instance(self, instance: ModelInstanceWrapper):
+        for name, value in self.updated_fields.items():
+            if name.startswith('title_'):
+                if not value:
+                    instance.titles.pop(name[6:], None)
+                else:
+                    instance.titles[name[6:]] = value
+                continue
+
+            field = instance._meta.get_field(name)
+            if not field.is_relation:
+                setattr(instance, field.name, field.to_python(value))
+            elif field.many_to_one or field.one_to_one:
+                if is_created_pk(value):
+                    obj = self.changeset.get_created_object(field.related_model, value)
+                    setattr(instance, field.get_cache_name(), obj)
+                else:
+                    delattr(instance, field.get_cache_name())
+                setattr(instance, field.attname, value)
+            else:
+                raise NotImplementedError
+
+    def clean_updated_fields(self):
+        if self.is_created:
+            current_obj = self.model_class()
+        else:
+            current_obj = self.model_class.objects.get(pk=self.existing_object_pk)
+
+        delete_fields = set()
+        for name, new_value in self.updated_fields.items():
+            if name.startswith('title_'):
+                current_value = current_obj.titles.get(name[6:], '')
+            else:
+                field = self.model_class._meta.get_field(name)
+
+                if not field.is_relation:
+                    current_value = field.get_prep_value(getattr(current_obj, field.name))
+                elif field.many_to_one or field.one_to_one:
+                    current_value = getattr(current_obj, field.attname)
+                else:
+                    raise NotImplementedError
+
+            if current_value == new_value:
+                delete_fields.add(name)
+
+        self.updated_fields = {name: value for name, value in self.updated_fields.items() if name not in delete_fields}
+        return delete_fields
+
+    def save_instance(self, instance):
+        self.updated_fields = {}
+        for field in self.model_class._meta.get_fields():
+            if not isinstance(field, Field) or field.primary_key:
+                continue
+
+            if not field.is_relation:
+                value = getattr(instance, field.name)
+                if field.name == 'titles':
+                    for lang, title in value.items():
+                        self.updated_fields['title_'+lang] = title
+                else:
+                    self.updated_fields[field.name] = field.get_prep_value(value)
+            elif field.many_to_one or field.one_to_one:
+                try:
+                    value = getattr(instance, field.get_cache_name())
+                except AttributeError:
+                    value = getattr(instance, field.attname)
+                else:
+                    value = None if value is None else value.pk
+                self.updated_fields[field.name] = value
+            else:
+                raise NotImplementedError
+
+        self.clean_updated_fields()
+        self.save()
+        if instance.pk is None and self.pk is not None:
+            instance.pk = self.obj_pk
+
+    def mark_deleted(self):
+        self.deleted = True
+        self.save()
+
+    @property
+    def does_something(self):
+        return (self.updated_fields or self._m2m_added_cache or self._m2m_removed_cache or self.is_created or
+                (not self.is_created and self.deleted))
+
     def save(self, *args, **kwargs):
-        self.m2m_added = {name: tuple(values) for name, values in self._m2m_added_cache}
-        self.m2m_removed = {name: tuple(values) for name, values in self._m2m_added_cache}
         if self.changeset.proposed is not None or self.changeset.applied is not None:
             raise TypeError('can not add change object to uneditable changeset.')
+        if not self.does_something:
+            if self.pk is not None:
+                self.delete()
+            return False
+        self.m2m_added = {name: tuple(values) for name, values in self._m2m_added_cache}
+        self.m2m_removed = {name: tuple(values) for name, values in self._m2m_removed_cache}
         super().save(*args, **kwargs)
         if not self.changeset.fill_changes_cache():
             self.update_changeset_cache()
-
+        return True
-    def delete(self, *args, **kwargs):
-        raise TypeError('change objects can not be deleted directly.')
 
     def __repr__(self):
         return '<ChangedObject #%s on ChangeSet #%s>' % (str(self.pk), str(self.changeset_id))

src/c3nav/editor/models/changeset.py

@@ -154,7 +154,7 @@ class ChangeSet(models.Model):
 
         # collect pks of relevant objects
         object_pks = {}
-        for change in chain(self.changed_objects.values()):
+        for change in chain(*(objects.values() for objects in self.changed_objects.values())):
             change.add_relevant_object_pks(object_pks)
 
         # retrieve relevant objects
@@ -188,7 +188,10 @@ class ChangeSet(models.Model):
         r = tuple((pk, values[name]) for pk, values in self.updated_existing.get(model, {}).items() if name in values)
         return r
 
-    def get_changed_object(self, model, pk=None):
+    def get_changed_object(self, model, pk=None) -> ChangedObject:
+        if pk is None:
+            return ChangedObject(changeset=self, model_class=model)
+
         self.fill_changes_cache()
 
         objects = tuple(obj for obj in ((submodel, self.changed_objects.get(submodel, {}).get(pk, None))
@@ -196,12 +199,12 @@ class ChangeSet(models.Model):
         if len(objects) > 1:
             raise model.MultipleObjectsReturned
         if objects:
-            return objects[0]
+            return objects[0][1]
 
         if is_created_pk(pk):
             raise model.DoesNotExist
 
-        return ChangedObject(changeset=self, model_class=model, existing_obj_pk=pk)
+        return ChangedObject(changeset=self, model_class=model, existing_object_pk=pk)
 
     def get_created_object(self, model, pk, get_foreign_objects=False, allow_deleted=False):
         """

src/c3nav/editor/views/changes.py

@@ -47,8 +47,7 @@ def changeset_detail(request, pk):
     changed_objects_data = []
 
     for model, changed_objects in changeset.changed_objects.items():
-        for changed_object in changed_objects:
+        for pk, changed_object in changed_objects.items():
-            pk = changed_object.obj_pk
             obj = objects[model][pk]
 
             obj_desc = _('%(model)s #%(id)s') % {'model': obj.__class__._meta.verbose_name, 'id': pk}
@@ -79,7 +78,7 @@ def changeset_detail(request, pk):
             }
             changed_objects_data.append(changed_object_data)
 
-            form_fields = changeset.wrap(type(obj)).EditorForm._meta.fields
+            form_fields = changeset.wrap_model(type(obj)).EditorForm._meta.fields
 
             if changed_object.is_created:
                 changes.append({
@@ -90,7 +89,7 @@ def changeset_detail(request, pk):
 
             update_changes = []
 
-            for name, value in changed_object.updated_fields:
+            for name, value in changed_object.updated_fields.items():
                 change_data = {
                     'icon': 'option-vertical',
                     'class': 'muted',

src/c3nav/editor/wrappers.py

@@ -4,8 +4,8 @@ from collections import OrderedDict
 from functools import reduce, wraps
 from itertools import chain
 
-from django.db import models, transaction
+from django.db import models
-from django.db.models import Field, FieldDoesNotExist, Manager, ManyToManyRel, Prefetch, Q
+from django.db.models import FieldDoesNotExist, Manager, ManyToManyRel, Prefetch, Q
 from django.utils.functional import cached_property
 
 from c3nav.editor.forms import create_editor_form
@@ -21,7 +21,7 @@ class BaseWrapper:
     Callables will only be returned be getattr when they are inside _allowed_callables.
     Callables in _wrapped_callables will be returned wrapped, so that their self if the wrapping instance.
     """
-    _not_wrapped = ('_changeset', '_obj', '_created_pks', '_result', '_extra', '_result_cache', '_initial_values')
+    _not_wrapped = ('_changeset', '_obj', '_created_pks', '_result', '_extra', '_result_cache')
     _allowed_callables = ()
     _wrapped_callables = ()
 
@@ -184,44 +184,9 @@ class ModelInstanceWrapper(BaseWrapper):
     _wrapped_callables = ('validate_unique', '_get_pk_val')
 
     def __init__(self, *args, **kwargs):
-        """
-        Get initial values of this instance, so we know what changed on save.
-        Updates values according to cangeset if this is an existing object.
-        """
         super().__init__(*args, **kwargs)
-        updates = self._changeset.updated_existing.get(type(self._obj), {}).get(self._obj.pk, {})
+        if self._obj.pk is not None:
-        self._initial_values = {}
+            self._changeset.get_changed_object(self._obj.__class__, self._obj.pk).apply_to_instance(self)
-        for field in self._obj._meta.get_fields():
-            if not isinstance(field, Field):
-                continue
-            if field.name in self._obj.get_deferred_fields():
-                continue
-            if field.related_model is None:
-                if field.primary_key:
-                    continue
-
-                if field.name == 'titles':
-                    for name, value in updates.items():
-                        if not name.startswith('title_'):
-                            continue
-                        if not value:
-                            self._obj.titles.pop(name[6:], None)
-                        else:
-                            self._obj.titles[name[6:]] = value
-                elif field.name in updates:
-                    setattr(self._obj, field.name, field.to_python(updates[field.name]))
-                self._initial_values[field] = getattr(self._obj, field.name)
-            elif (field.many_to_one or field.one_to_one) and not field.primary_key:
-                if field.name in updates:
-                    value_pk = updates[field.name]
-                    if is_created_pk(value_pk):
-                        obj = self._wrap_model(field.model).get(pk=value_pk)
-                        setattr(self._obj, field.get_cache_name(), obj)
-                        setattr(self._obj, field.attname, obj.pk)
-                    else:
-                        delattr(self._obj, field.get_cache_name())
-                        setattr(self._obj, field.attname, value_pk)
-                self._initial_values[field] = getattr(self._obj, field.attname)
 
     def __eq__(self, other):
         if isinstance(other, BaseWrapper):
@@ -234,7 +199,7 @@ class ModelInstanceWrapper(BaseWrapper):
     def __setattr__(self, name, value):
         """
         We have to intercept here because RelatedFields won't accept
-        Wrapped model instances values, so we have to trick them.
+        wrapped model instances values, so we have to trick them.
         """
         if name in self._not_wrapped:
             return super().__setattr__(name, value)
@@ -269,37 +234,10 @@ class ModelInstanceWrapper(BaseWrapper):
         """
         Create changes in changeset instead of saving.
         """
-        if self.pk is None:
+        self._changeset.get_changed_object(self._obj.__class__, self.pk).save_instance(self)
-            self._changeset.add_create(self)
-        with transaction.atomic():
-            for field, initial_value in self._initial_values.items():
-                if field.many_to_one:
-                    try:
-                        new_value = getattr(self._obj, field.get_cache_name())
-                    except AttributeError:
-                        new_value = getattr(self._obj, field.attname)
-                    else:
-                        new_value = None if new_value is None else new_value.pk
-
-                    if new_value != initial_value:
-                        self._changeset.add_update(self, name=field.name, value=new_value)
-                    continue
-
-                new_value = getattr(self._obj, field.name)
-                if new_value == initial_value:
-                    continue
-
-                if field.name == 'titles':
-                    for lang in (set(initial_value.keys()) | set(new_value.keys())):
-                        new_title = new_value.get(lang, '')
-                        if new_title != initial_value.get(lang, ''):
-                            self._changeset.add_update(self, name='title_'+lang, value=new_title)
-                    continue
-
-                self._changeset.add_update(self, name=field.name, value=field.get_prep_value(new_value))
 
     def delete(self):
-        self._changeset.add_delete(self)
+        self._changeset.get_changed_object(self._obj.__class__, self.pk).mark_deleted()
 
 
 def get_queryset(func):