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

Set history_user automatically using middleware #108

Merged
merged 4 commits into from
Aug 5, 2014

Conversation

macro1
Copy link
Collaborator

@macro1 macro1 commented Jul 27, 2014

This adds an optional feature to populate history_user on historical models using middleware to retrieve the request object.

@treyhunner, @mauricioabreu I think this should be an acceptable implimentation of feature request #25. It probably could use a few tests around the new get_history_user method, and the documentation could be clearer.

@@ -221,6 +224,19 @@ def create_historical_record(self, instance, type):
manager.create(history_date=history_date, history_type=type,
history_user=history_user, **attrs)

def get_history_user(self, instance):
"""Use hints from the instance and middleware to identify the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nitpick: I prefer to follow PEP 257 for one line docstrings.

@treyhunner
Copy link
Member

Can we only enable the middleware during relevant tests? Maybe override_settings can be used for this?

@macro1
Copy link
Collaborator Author

macro1 commented Aug 3, 2014

This will pass once 1.3 is removed from the matrix in #103.

@treyhunner
Copy link
Member

This looks great for me. 👍 for merging.

@mauricioabreu
Copy link
Contributor

It is a good solution. Thanks for your efforts!

macro1 added a commit that referenced this pull request Aug 5, 2014
Set history_user automatically using middleware
@macro1 macro1 merged commit bb23f85 into jazzband:master Aug 5, 2014
@macro1 macro1 deleted the user-from-request branch August 5, 2014 04:42
macro1 added a commit to macro1/django-simple-history that referenced this pull request Aug 8, 2014
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

Successfully merging this pull request may close these issues.

None yet

3 participants