Open brianmay opened 9 years ago
Oddly enough, e6781837267773bba25338fe33f9198e235bf26b 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...
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.
I feel I should contribute here as https://github.com/Atomidata/django-audit-log/pull/16 is where I proposed a fix for this very issue. The pull request was never merged...
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.
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.
I believe #16 was closed by 529327db618d63dccbce6f682cb27ec99c6997af
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.
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 e6781837267773bba25338fe33f9198e235bf26b broke things.
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')
As far as I could tell this issue is fixed with the patch from @ghinch. Can @brianmay confirm?
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 Person
object (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
Possibly this bug was fixed in c168ef241d967fef0236dff709b6d4dbf6ccd5d2?
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.
Hello,
I have in my Django settings file:
If I try to update this model to use django-audit-log, it gives me a migration containing the following field:
However, this is incorrect, it should point to karaage.Person, like for the other tables, eg:
This results in errors such as the following one:
Any ideas?
Regards