Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
artscoop opened this issue Jul 10, 2014 · 12 comments
Open

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

artscoop opened this issue Jul 10, 2014 · 12 comments

Comments

@artscoop
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Author

@Moustacha Hi, got some news on this bug ?

@SamTShaw
Copy link
Contributor

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

@artscoop
Copy link
Author

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

  • Have a model registered with a moderator. This model has 5 fields monitored.
  • Change objects of this model (Moderation instances are created for these instances)
  • Remove a monitored field from the model.
  • Migrate.
  • You get a serialization error in the moderation admin, because the moderation instances contain data for unknown fields that cannot be deserialized back to the actual instance.

@SamTShaw
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

The workarround was to erase all ModeratedObjects:

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

@brunosmartin
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

zypro commented Dec 13, 2019

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants