dominno / django-moderation

django-moderation is reusable application for Django framework, that allows to moderate any model objects.
BSD 3-Clause "New" or "Revised" License
269 stars 90 forks source link

admin breaks if a moderated model was changed (migration) #112

Open artscoop opened 10 years ago

artscoop commented 10 years ago

there is a problem in django-moderation which prevents its use in production: the deserialization methods raise an error if a model instance was saved for moderation, then restored after the model definition was changed. You simply get an error stating that a field does not exist. Is this possible to fix this so that non existent fields don't raise an error ?

SamTShaw commented 10 years ago

Was the model definition changed to have an extra field that didn't exist when the object was moderated? If so, this is already taken care of, as evidenced by the tests I wrote.

artscoop commented 10 years ago

It breaks when a serialized field doesn't exist in the new model. For example, MyModel(a:FloatField, b:FloatField) is serialized to s. A migration is done, and now MyModel's definition is MyModel(a:FloatField, c:CharField). You cannot deserialize s into MyModel's definition because b doesn't exist anymore. Whet would be nice would be to try to deserialize every field of s separately, failing silently if a serialized field has no match.

artscoop commented 9 years ago

@Moustacha Hi, got some news on this bug ?

SamTShaw commented 9 years ago

Are you able to provide a sample project that hits this bug?

artscoop commented 9 years ago

A sample project ? That would be hard. However, there are steps to reproduce:

SamTShaw commented 9 years ago

I tried making a basic project (https://github.com/Moustacha/django-moderation-issue112) that re-creates those steps, and I'm still not hitting any bugs.

I created a model with two fields, a & b. Moderated it, and created a couple of moderations for an instance. Then i removed field b and added c. Viewing the current object doesn't result in any errors: image

brunosmartin commented 8 years ago

I had this issue here too. After one field was renamed in model.

@Moustacha Viewing the object works, but if you try to save it, you get the exception!

brunosmartin commented 8 years ago

The workarround was to erase all ModeratedObjects:

from moderation.models import ModeratedObject ModeratedObject.objects.all().delete()

brunosmartin commented 8 years ago

Here my traceback:

Traceback (most recent call last):
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/core/handlers/base.py", line 132, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/lib/python3.4/contextlib.py", line 30, in inner
    return func(*args, **kwds)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/contrib/admin/options.py", line 616, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/utils/decorators.py", line 110, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/views/decorators/cache.py", line 57, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/contrib/admin/sites.py", line 233, in inner
    return view(request, *args, **kwargs)
  File "/home/vagrant/mapaguarani-env/src/django-moderation/moderation/admin.py", line 55, in change_view
    extra_context=extra_context)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/contrib/admin/options.py", line 1519, in change_view
    return self.changeform_view(request, object_id, form_url, extra_context)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/utils/decorators.py", line 34, in _wrapper
    return bound_func(*args, **kwargs)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/utils/decorators.py", line 110, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/utils/decorators.py", line 30, in bound_func
    return func.__get__(self, type(self))(*args2, **kwargs2)
  File "/usr/lib/python3.4/contextlib.py", line 30, in inner
    return func(*args, **kwds)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/contrib/admin/options.py", line 1467, in changeform_view
    self.save_model(request, new_object, form, not add)
  File "/home/vagrant/mapaguarani-env/src/django-moderation/moderation/admin.py", line 74, in save_model
    obj.save()
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/models/base.py", line 734, in save
    force_update=force_update, update_fields=update_fields)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/models/base.py", line 771, in save_base
    update_fields=update_fields, raw=raw, using=using)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/dispatch/dispatcher.py", line 201, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/home/vagrant/mapaguarani-env/src/django-moderation/moderation/register.py", line 260, in post_save_handler
    moderated_obj.changed_object.save_base(raw=True)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/models/base.py", line 762, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/models/base.py", line 827, in _save_table
    forced_update)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/models/base.py", line 877, in _do_update
    return filtered._update(values) > 0
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/models/query.py", line 580, in _update
    return query.get_compiler(self.db).execute_sql(CURSOR)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/models/sql/compiler.py", line 1062, in execute_sql
    cursor = super(SQLUpdateCompiler, self).execute_sql(result_type)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/models/sql/compiler.py", line 840, in execute_sql
    cursor.execute(sql, params)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/backends/utils.py", line 79, in execute
    return super(CursorDebugWrapper, self).execute(sql, params)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/utils.py", line 97, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/utils/six.py", line 658, in reraise
    raise value.with_traceback(tb)
  File "/home/vagrant/mapaguarani-env/lib/python3.4/site-packages/django/db/backends/utils.py", line 64, in execute
    return self.cursor.execute(sql, params)
django.db.utils.IntegrityError: null value in column "geometry" violates not-null constraint
supervacuo commented 6 years ago

This also happens when you change a field definition such that previous data is invalid. Example:

# models.py
class Example(models.Model):
  field = models.NullBooleanField(null=True, blank=True) 
# moderator.py
class ExampleModerator(GenericModerator):
    pass
moderation.register(Example, ExampleModerator)

Then create a new Example object with field = None, then edit the definition:

# models.py
class Example(models.Model):
  field = models.BooleanField(default=False) 

django-admin.py makemigrations && django-admin.py migrate correctly updates existing Example objects to set field to False where it is None, but data in ModeratedObject.changed_object still contains an object with field = None, which causes a crash on trying to load either the Example or ModeratedObject admin pages:

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/django/core/serializers/python.py", line 179, in Deserializer
    data[field.name] = field.to_python(field_value)
  File "/usr/local/lib/python3.5/dist-packages/django/db/models/fields/__init__.py", line 1036, in to_python
    params={'value': value},
django.core.exceptions.ValidationError: ["'None' value must be either True or False."]
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/django/core/serializers/json.py", line 82, in Deserializer
    for obj in PythonDeserializer(objects, **options):
  File "/usr/local/lib/python3.5/dist-packages/django/core/serializers/python.py", line 181, in Deserializer
    raise base.DeserializationError.WithData(e, d['model'], d.get('pk'), field_value)
django.core.serializers.base.DeserializationError: ["'None' value must be either True or False."]: (example.example:pk=1) field_value was 'None'
zypro commented 4 years ago

Having same issue. I recognized if you approve your object before saving it, the process will work correctly... but this "work-around" is a problem because then rejected or re-edited objects get approved automatically...

dianamoreno commented 3 years ago

Having the same issue with Django 2.2. There was a solution for this bug?