action_user created with incorrect to= model

[brianmay]

Hello,

I have in my Django settings file:

AUTH_USER_MODEL = 'karaage.Person'

If I try to update this model to use django-audit-log, it gives me a migration containing the following field:

('action_user', audit_log.models.fields.LastUserField(related_name='_person_audit_log_entry', editable=False, to='karaage.PersonAuditLogEntry', null=True)),

However, this is incorrect, it should point to karaage.Person, like for the other tables, eg:

('action_user', audit_log.models.fields.LastUserField(to='karaage.Person', null=True, editable=False, related_name='_projectlevel_audit_log_entry')),

This results in errors such as the following one:

  File "/home/brian/tree/karaage/karaage-working/karaage/people/models.py", line 137, in save
    super(Person, self).save(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/django/db/models/base.py", line 589, in save
    force_update=force_update, update_fields=update_fields)
  File "/usr/lib/python3/dist-packages/django/db/models/base.py", line 626, in save_base
    update_fields=update_fields, raw=raw, using=using)
  File "/usr/lib/python3/dist-packages/django/dispatch/dispatcher.py", line 198, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/usr/lib/python3/dist-packages/audit_log/models/managers.py", line 107, in post_save
    self.create_log_entry(instance, created and 'I' or 'U')
  File "/usr/lib/python3/dist-packages/audit_log/models/managers.py", line 102, in create_log_entry
    manager.create(action_type = action_type, **attrs)
  File "/usr/lib/python3/dist-packages/django/db/models/manager.py", line 92, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/usr/lib/python3/dist-packages/django/db/models/query.py", line 372, in create
    obj.save(force_insert=True, using=self.db)
  File "/usr/lib/python3/dist-packages/django/db/models/base.py", line 589, in save
    force_update=force_update, update_fields=update_fields)
  File "/usr/lib/python3/dist-packages/django/db/models/base.py", line 613, in save_base
    update_fields=update_fields)
  File "/usr/lib/python3/dist-packages/django/dispatch/dispatcher.py", line 198, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/usr/lib/python3/dist-packages/django/utils/functional.py", line 17, in _curried
    return _curried_func(*(args + moreargs), **dict(kwargs, **morekwargs))
  File "/usr/lib/python3/dist-packages/audit_log/middleware.py", line 51, in _update_pre_save_info
    setattr(instance, field.name, user)
  File "/usr/lib/python3/dist-packages/django/db/models/fields/related.py", line 597, in __set__
    self.field.rel.to._meta.object_name,
ValueError: Cannot assign "<SimpleLazyObject: <Person: Test User1>>": "PersonAuditLogEntry.action_user" must be a "PersonAuditLogEntry" instance.

Any ideas?

Regards

[brianmay]

Oddly enough, e678183 changes the behaviour so that an error is generated instead:

karaage.PersonAuditLogEntry.action_user: (fields.E300) Field defines a relation with model 'karaage.Person', which is either not installed, or is abstract.

Which is odd, I don't see anything in this commit which would have had such a significant change.

Still investigating...

[brianmay]

Defining a forward reference from PersonAuditLogEntry to Person here is not as easy you might imagine, for reasons I don't really understand.

• Karage.Person does not work.
• apps.get_app_config('karaage').get_model("Person") does not work.

If we had a class reference that would work (as this is how copy_fields() works), but we don't have a class reference. Only the AUTH_USER_MODEL setting value.

[tysonclugg]

I feel I should contribute here as #16 is where I proposed a fix for this very issue. The pull request was never merged...

[tysonclugg]

Hi Brian,

I'm not sure that the PersonAuditLogEntry.action_user field should be a foreign key to Person, it kind of makes sense that it should relate to the PersonAuditLogEntry model instead. Deleting a Person shouldn't cause all related $FooAuditLogEntry to be cascade deleted, should it?

Regards,
Tyson.

[brianmay]

Just for the record, I believe that #16 was applied, just not exactly the same way as in the merge request.

@tysonclugg Was going based on what happens for all other classes, where action_user is is a reference to Person. Like it or not. I think consistency is more important here, unless you want to change every action_user.

Cascaded deletes can be disabled by setting on_delete=models.SET_NULL.

Oddly enough, on some circumstances, action_user was being setup with Person not PersonAuditLogEntry, as mentioned above I was getting this error with the latest Master version:

karaage.PersonAuditLogEntry.action_user: (fields.E300) Field defines a relation with model 'karaage.Person', which is either not installed, or is abstract.

This is something I still don't really understand, it should be set to self, which becomes PersonAuditLogEntry not Person. I might try to investigate this in more detail now.

[brianmay]

I believe #16 was closed by 529327d

[brianmay]

It seems to be that under certain circumstances the code introduced by #16 doesn't get AUTH_USER_MODEL value correctly (it gets the default value instead), and as a result, the condition fails, and the self value is never used. Still trying to understand why this is.

[brianmay]

Ok, one mystery solved. In audit_log/models/managers.py we have:

from django.conf import settings                                                 
from audit_log import settings

i.e. the settings value is being overridden. The audit_log/settings.py file does not pull in the value for the AUTH_USER_MODEL setting, it only defines the DISABLE_AUDIT_LOG setting.

Now I understand why e678183 broke things.

[brianmay]

Yesterday I made the following comment, but it accidentally ended up in the wrong place - it ended up in the commit, not here.

In get_logging_fields there is currently the following code:

        #check if the manager has been attached to auth user model                
        if [model._meta.app_label, model.__name__] == getattr(settings, 'AUTH_USER_MODEL', 'auth.User').split("."):
            action_user_field = LastUserField(related_name = rel_name, editable = False, to = 'self')

[vvangelovski]

As far as I could tell this issue is fixed with the patch from @ghinch. Can @brianmay confirm?

[brianmay]

No, problem still not resolved, just one of the issues I noticed along the way. We need to work out in action_user should point to the Personobject (makes it consistent with established schemas) or the PersonAuditLogEntry object (as per what @tysonclugg said before). At the moment we set the schema to use PersonAuditLogEntry but pass it a Person value which is broken:

======================================================================
ERROR: test_password_reset_by_self (karaage.tests.test_people.PersonTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/brian/tree/django/karaage/karaage/karaage/tests/test_people.py", line 472, in test_password_reset_by_self
    response = self.client.post(url, form_data, follow=True)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/test/client.py", line 512, in post
    secure=secure, **extra)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/test/client.py", line 313, in post
    secure=secure, **extra)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/test/client.py", line 379, in generic
    return self.request(**r)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/test/client.py", line 466, in request
    six.reraise(*exc_info)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/core/handlers/base.py", line 132, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/utils/decorators.py", line 145, in inner
    return func(*args, **kwargs)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/views/decorators/debug.py", line 76, in sensitive_post_parameters_wrapper
    return view(request, *args, **kwargs)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/views/decorators/cache.py", line 57, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/contrib/auth/views.py", line 246, in password_reset_confirm
    form.save()
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/contrib/auth/forms.py", line 290, in save
    self.user.save()
  File "/usr/lib/python2.7/dist-packages/model_utils/tracker.py", line 134, in save
    ret = original_save(**kwargs)
  File "/home/brian/tree/django/karaage/karaage/karaage/people/models.py", line 137, in save
    super(Person, self).save(*args, **kwargs)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/db/models/base.py", line 710, in save
    force_update=force_update, update_fields=update_fields)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/db/models/base.py", line 747, in save_base
    update_fields=update_fields, raw=raw, using=using)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/dispatch/dispatcher.py", line 201, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/home/brian/tmp/django-audit-log/audit_log/models/managers.py", line 110, in post_save
    self.create_log_entry(instance, created and 'I' or 'U')
  File "/home/brian/tmp/django-audit-log/audit_log/models/managers.py", line 105, in create_log_entry
    manager.create(action_type = action_type, **attrs)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/db/models/manager.py", line 127, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/db/models/query.py", line 348, in create
    obj.save(force_insert=True, using=self.db)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/db/models/base.py", line 710, in save
    force_update=force_update, update_fields=update_fields)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/db/models/base.py", line 734, in save_base
    update_fields=update_fields)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/dispatch/dispatcher.py", line 201, in send
    response = receiver(signal=self, sender=sender, **named)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/utils/functional.py", line 17, in _curried
    return _curried_func(*(args + moreargs), **dict(kwargs, **morekwargs))
  File "/home/brian/tmp/django-audit-log/audit_log/middleware.py", line 55, in _update_pre_save_info
    setattr(instance, field.name, user)
  File "/home/brian/tmp/python/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 627, in __set__
    self.field.rel.to._meta.object_name,
ValueError: Cannot assign "<SimpleLazyObject: <Person: Test User1>>": "PersonAuditLogEntry.action_user" must be a "PersonAuditLogEntry" instance.

----------------------------------------------------------------------
Ran 1 test in 1.695s

[brianmay]

Possibly this bug was fixed in c168ef2?

[vvangelovski]

No, it's not fixed yet. I was making the unit tests catch this bug as I was starting to fix it. But I've yet to work out a fix.