Merge pull request #275 from joaojunior/change_reason

add history change reason to allow reasoning for the changes[2]

Author: leportella

CHANGES.rst

@@ -51,6 +51,7 @@ tip (unreleased)
 - Fix a bug when models have a ``ForeignKey`` with ``primary_key=True``
 - Do NOT delete the history elements when a user is deleted.
 - Add support for ``latest``
+- Allow setting a reason for change. [using option changeReason]
 
 1.5.3 (2014-11-18)
 ------------------

docs/advanced.rst

@@ -213,3 +213,43 @@ You can use the ``table_name`` parameter with both ``HistoricalRecords()`` or
         pub_date = models.DateTimeField('date published')
 
     register(Question, table_name='polls_question_history')
+
+Change Reason
+-------------
+
+Change reason is a message to explain why the change was made in the instance. It is stored in the
+field ``history_change_reason`` and its default value is ``None``.
+
+By default, the django-simple-history gets the change reason in the field ``changeReason`` of the instance. Also, is possible to pass
+the ``changeReason`` explicitly. For this, after a save or delete in an instance, is necessary call the
+function ``utils.update_change_reason``. The first argument of this function is the instance and the seccond
+is the message that represents the change reason.
+
+For instance, for the model:
+
+.. code-block:: python
+
+    from django.db import models
+    from simple_history.models import HistoricalRecords
+
+    class Poll(models.Model):
+        question = models.CharField(max_length=200)
+        history = HistoricalRecords()
+
+You can create a instance with a implicity change reason.
+
+.. code-block:: python
+
+    poll = Poll(question='Question 1')
+    poll.changeReason = 'Add a question'
+    poll.save()
+
+Or you can pass the change reason explicitly:
+
+.. code-block:: python
+
+    from simple_history.utils import update_change_reason
+
+    poll = Poll(question='Question 1')
+    poll.save()
+    update_change_reason(poll, 'Add a question')

simple_history/models.py

@@ -4,16 +4,18 @@
 import importlib
 import threading
 
-from django.db import models, router
-from django.db.models.fields.proxy import OrderWrt
 from django.conf import settings
 from django.contrib import admin
+from django.db import models, router
+from django.db.models.fields.proxy import OrderWrt
 from django.utils import six
-from django.utils.encoding import python_2_unicode_compatible
-from django.utils.encoding import smart_text
+from django.utils.encoding import python_2_unicode_compatible, smart_text
 from django.utils.timezone import now
 from django.utils.translation import string_concat, ugettext as _
 
+from . import exceptions
+from .manager import HistoryDescriptor
+
 try:
     from django.apps import apps
 except ImportError:  # Django < 1.7
@@ -26,8 +28,6 @@
     add_introspection_rules(
         [], ["^simple_history.models.CustomForeignKeyField"])
 
-from . import exceptions
-from .manager import HistoryDescriptor
 
 registered_models = {}
 
@@ -202,6 +202,8 @@ def get_instance(self):
         return {
             'history_id': models.AutoField(primary_key=True),
             'history_date': models.DateTimeField(),
+            'history_change_reason': models.CharField(max_length=100,
+                                                      null=True),
             'history_user': models.ForeignKey(
                 user_model, null=True, related_name=self.user_related_name,
                 on_delete=models.SET_NULL),
@@ -247,12 +249,14 @@ def post_delete(self, instance, **kwargs):
     def create_historical_record(self, instance, history_type):
         history_date = getattr(instance, '_history_date', now())
         history_user = self.get_history_user(instance)
+        history_change_reason = getattr(instance, 'changeReason', None)
         manager = getattr(instance, self.manager_name)
         attrs = {}
         for field in instance._meta.fields:
             attrs[field.attname] = getattr(instance, field.attname)
         manager.create(history_date=history_date, history_type=history_type,
-                       history_user=history_user, **attrs)
+                       history_user=history_user,
+                       history_change_reason=history_change_reason, **attrs)
 
     def get_history_user(self, instance):
         """Get the modifying user from instance or middleware."""

simple_history/registry_tests/migration_test_app/migrations/0001_initial.py

@@ -29,6 +29,7 @@ class Migration(migrations.Migration):
                 ('history_id', models.AutoField(primary_key=True, serialize=False)),
                 ('history_date', models.DateTimeField()),
                 ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)),
+                ('history_change_reason', models.CharField(max_length=100, null=True)),
                 ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)),
             ],
             options={

simple_history/registry_tests/tests.py

@@ -1,23 +1,21 @@
 from __future__ import unicode_literals
 
-from datetime import datetime, timedelta
-from six.moves import cStringIO as StringIO
 import unittest
+from datetime import datetime, timedelta
 
 import django
 from django.contrib.auth import get_user_model
 from django.core import management
 from django.test import TestCase
-
 from simple_history import exceptions, register
-from ..tests.models import (
-    Poll, Choice, Voter, Restaurant, HistoricalPoll, HistoricalChoice,
-    HistoricalState, HistoricalCustomFKError,
-    UserAccessorDefault, UserAccessorOverride,
-    TrackedAbstractBaseA, TrackedAbstractBaseB,
-    TrackedWithAbstractBase, TrackedWithConcreteBase,
-    InheritTracking1, InheritTracking2, InheritTracking3, InheritTracking4,
-)
+from six.moves import cStringIO as StringIO
+
+from ..tests.models import (Choice, InheritTracking1, InheritTracking2,
+                            InheritTracking3, InheritTracking4, Poll,
+                            Restaurant, TrackedAbstractBaseA,
+                            TrackedAbstractBaseB, TrackedWithAbstractBase,
+                            TrackedWithConcreteBase, UserAccessorDefault,
+                            UserAccessorOverride, Voter)
 
 try:
     from django.apps import apps
@@ -86,7 +84,8 @@ def test_tracked_abstract_base(self):
                 for f in TrackedWithAbstractBase.history.model._meta.fields
             ],
             [
-                'id', 'history_id', 'history_date', 'history_user_id',
+                'id', 'history_id', 'history_date',
+                'history_change_reason', 'history_user_id',
                 'history_type',
             ],
         )
@@ -99,7 +98,8 @@ def test_tracked_concrete_base(self):
             ],
             [
                 'id', 'trackedconcretebase_ptr_id', 'history_id',
-                'history_date', 'history_user_id', 'history_type',
+                'history_date', 'history_change_reason', 'history_user_id',
+                'history_type',
             ],
         )
 
@@ -114,7 +114,8 @@ def test_tracked_abstract_and_untracked_concrete_base(self):
             [f.attname for f in InheritTracking1.history.model._meta.fields],
             [
                 'id', 'untrackedconcretebase_ptr_id', 'history_id',
-                'history_date', 'history_user_id', 'history_type',
+                'history_date', 'history_change_reason',
+                'history_user_id', 'history_type',
             ],
         )
 
@@ -123,7 +124,8 @@ def test_indirect_tracked_abstract_base(self):
             [f.attname for f in InheritTracking2.history.model._meta.fields],
             [
                 'id', 'baseinherittracking2_ptr_id', 'history_id',
-                'history_date', 'history_user_id', 'history_type',
+                'history_date', 'history_change_reason',
+                'history_user_id', 'history_type',
             ],
         )
 
@@ -132,7 +134,8 @@ def test_indirect_tracked_concrete_base(self):
             [f.attname for f in InheritTracking3.history.model._meta.fields],
             [
                 'id', 'baseinherittracking3_ptr_id', 'history_id',
-                'history_date', 'history_user_id', 'history_type',
+                'history_date', 'history_change_reason',
+                'history_user_id', 'history_type',
             ],
         )
 

simple_history/tests/tests/test_models.py

@@ -1,27 +1,27 @@
 from __future__ import unicode_literals
 
-from datetime import datetime, timedelta
 import unittest
 import warnings
+from datetime import datetime, timedelta
 
 import django
 from django.contrib.auth import get_user_model
 from django.core.files.base import ContentFile
 from django.db import models
 from django.db.models.fields.proxy import OrderWrt
 from django.test import TestCase
-
 from simple_history.models import HistoricalRecords, convert_auto_field
-from ..models import (
-    AdminProfile, Bookcase, MultiOneToOne, Poll, Choice, Restaurant,
-    Person, FileModel, Document, Book, HistoricalPoll, Library, State,
-    AbstractBase, ConcreteAttr, ConcreteUtil, SelfFK, Temperature, WaterLevel,
-    ExternalModel1, ExternalModel3, UnicodeVerboseName, HistoricalChoice,
-    HistoricalState, HistoricalCustomFKError, Series, SeriesWork, PollInfo,
-    Employee, Country, Province,
-    City, Contact, ContactRegister,
-)
+from simple_history.utils import update_change_reason
+
 from ..external.models import ExternalModel2, ExternalModel4
+from ..models import (AbstractBase, AdminProfile, Book, Bookcase, Choice, City,
+                      ConcreteAttr, ConcreteUtil, Contact, ContactRegister,
+                      Country, Document, Employee, ExternalModel1,
+                      ExternalModel3, FileModel, HistoricalChoice,
+                      HistoricalCustomFKError, HistoricalPoll, HistoricalState,
+                      Library, MultiOneToOne, Person, Poll, PollInfo, Province,
+                      Restaurant, SelfFK, Series, SeriesWork, State,
+                      Temperature, UnicodeVerboseName, WaterLevel)
 
 try:
     from django.apps import apps
@@ -52,7 +52,7 @@ def assertRecordValues(self, record, klass, values_dict):
             self.assertEqual(getattr(record, key), value)
         self.assertEqual(record.history_object.__class__, klass)
         for key, value in values_dict.items():
-            if key != 'history_type':
+            if key not in ['history_type', 'history_change_reason']:
                 self.assertEqual(getattr(record.history_object, key), value)
 
     def test_create(self):
@@ -72,36 +72,63 @@ def test_update(self):
         p = Poll.objects.get()
         p.pub_date = tomorrow
         p.save()
+        update_change_reason(p, 'future poll')
         update_record, create_record = p.history.all()
         self.assertRecordValues(create_record, Poll, {
             'question': "what's up?",
             'pub_date': today,
             'id': p.id,
+            'history_change_reason': None,
             'history_type': "+"
         })
         self.assertRecordValues(update_record, Poll, {
             'question': "what's up?",
             'pub_date': tomorrow,
             'id': p.id,
+            'history_change_reason': 'future poll',
             'history_type': "~"
         })
         self.assertDatetimesEqual(update_record.history_date, datetime.now())
 
-    def test_delete(self):
+    def test_delete_verify_change_reason_implicitly(self):
+        p = Poll.objects.create(question="what's up?", pub_date=today)
+        poll_id = p.id
+        p.changeReason = 'wrongEntry'
+        p.delete()
+        delete_record, create_record = Poll.history.all()
+        self.assertRecordValues(create_record, Poll, {
+            'question': "what's up?",
+            'pub_date': today,
+            'id': poll_id,
+            'history_change_reason': None,
+            'history_type': "+"
+        })
+        self.assertRecordValues(delete_record, Poll, {
+            'question': "what's up?",
+            'pub_date': today,
+            'id': poll_id,
+            'history_change_reason': 'wrongEntry',
+            'history_type': "-"
+        })
+
+    def test_delete_verify_change_reason_explicity(self):
         p = Poll.objects.create(question="what's up?", pub_date=today)
         poll_id = p.id
         p.delete()
+        update_change_reason(p, 'wrongEntry')
         delete_record, create_record = Poll.history.all()
         self.assertRecordValues(create_record, Poll, {
             'question': "what's up?",
             'pub_date': today,
             'id': poll_id,
+            'history_change_reason': None,
             'history_type': "+"
         })
         self.assertRecordValues(delete_record, Poll, {
             'question': "what's up?",
             'pub_date': today,
             'id': poll_id,
+            'history_change_reason': 'wrongEntry',
             'history_type': "-"
         })
 
@@ -492,7 +519,8 @@ def test_string_related(self):
             related_model = field_object.related.model
         self.assertEqual(related_model, HistoricalState)
 
-    @unittest.skipUnless(django.get_version() >= "1.7", "Requires 1.7 migrations")
+    @unittest.skipUnless(django.get_version() >= "1.7",
+                         "Requires 1.7 migrations")
     def test_state_serialization_of_customfk(self):
         from django.db.migrations import state
         state.ModelState.from_model(HistoricalCustomFKError)
@@ -644,7 +672,8 @@ def test_restore_object_with_changed_order(self):
         self.assertEqual(order[5], self.w_chair.pk)
         self.assertEqual(order[6], self.w_battle.pk)
 
-    @unittest.skipUnless(django.get_version() >= "1.7", "Requires 1.7 migrations")
+    @unittest.skipUnless(django.get_version() >= "1.7",
+                         "Requires 1.7 migrations")
     def test_migrations_include_order(self):
         from django.db.migrations import state
         model_state = state.ModelState.from_model(SeriesWork.history.model)

simple_history/utils.py

@@ -0,0 +1,14 @@
+def update_change_reason(instance, reason):
+    attrs = {}
+    model = type(instance)
+    manager = instance if instance.id is not None else model
+    for field in instance._meta.fields:
+        value = getattr(instance, field.attname)
+        if field.primary_key is True:
+            if value is not None:
+                attrs[field.attname] = value
+        else:
+            attrs[field.attname] = value
+    record = manager.history.filter(**attrs).order_by('-history_date').first()
+    record.history_change_reason = reason
+    record.save()