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

"Changed by" displays None when change occurs outside of admin #43

Closed
rlander opened this issue May 13, 2013 · 6 comments
Closed

"Changed by" displays None when change occurs outside of admin #43

rlander opened this issue May 13, 2013 · 6 comments
Labels
bug Issues related to confirmed bugs docs P2

Comments

@rlander
Copy link

rlander commented May 13, 2013

When a model is changed within django's admin app, the "Change history" tab displays which user made the change (under Changed by).

However, when the same change occurs outside the admin app, "None" is shown.

Is this the expected behaviour? If so, how can I log the user that made the change?

Thanks!

@treyhunner
Copy link
Member

@rlander django-simple-history checks for a _history_user attribute on save.

I'm working on improving the documentation for djnago-simple-history in the sphinx-docs branch. Here's the documentation so far for _history_user

@rlander
Copy link
Author

rlander commented May 13, 2013

Awesome! Thanks @treyhunner for this great library!

@rlander
Copy link
Author

rlander commented May 13, 2013

@treyhunner after adding _history_user property to my model, I get a trace "AttributeError: can't set attribute" when saving an object inside the admin app (everything's ok outside).

Here's the full trace:
http://dpaste.com/1149337/

I haven't had the chance to dig into the code, but it seems that I'm missing a setter function, correct?

@treyhunner
Copy link
Member

Yes you need a setter attribute due to this line: https://github.com/treyhunner/django-simple-history/blob/master/simple_history/admin.py#L137

I wasn't aware of this behavior so it's not documented currently and there aren't tests demonstrating it.

In case you haven't used property setters before, here's an example: https://plus.google.com/116969159384245484847/posts/WJXWSxKKvKS

If you have an idea for a better implementation that doesn't require a setter feel free to make a pull request or a suggestion here.

@rlander
Copy link
Author

rlander commented May 13, 2013

Ok, here's my quick fix:

@_history_user.setter
def _history_user(self, value):
    self.__history_user = value

Tomorrow I'll take a look at the code and see if I can come up with a better solution.

@treyhunner
Copy link
Member

Now that the documentation is merged, it might be a good idea to add a note in the "Integrating with Django Admin" section that links to the "Recording Which User Changed a Model" section.

macro1 referenced this issue in macro1/django-simple-history Jun 27, 2014
treyhunner added a commit that referenced this issue Jun 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues related to confirmed bugs docs P2
Projects
None yet
Development

No branches or pull requests

2 participants