Feature/issue 379 history change reason (#379)

* First pass at changing how the history_change_reason works

* Use a better name for the class of the field.

* Initial work with passing tests

* Fix default field type

Make max_length configurable via settings if the other field is configured via settings.

* Add tests
Fix setting name

* Fix positional arguments suggestion, add my new thing at the end.

* Again, consider the usage of positional args. Move history_id after excluded fields.

* Trying to get my tests working.

Revert null change on field

* Still working on my tests.

Error:

Traceback (most recent call last):
  File "/Users/kneuharth/projects/django-simple-history/simple_history/tests/tests/test_models.py", line 402, in test_textfield_history_change_reason2
    TextFieldChangeReasonModel2.objects.create(greeting="what's up?")

  File "/Users/kneuharth/projects/django-simple-history/.eggs/Django-2.0.4-py3.5.egg/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/Users/kneuharth/projects/django-simple-history/.eggs/Django-2.0.4-py3.5.egg/django/db/models/query.py", line 417, in create
    obj.save(force_insert=True, using=self.db)
  File "/Users/kneuharth/projects/django-simple-history/.eggs/Django-2.0.4-py3.5.egg/django/db/models/base.py", line 729, in save
    force_update=force_update, update_fields=update_fields)
  File "/Users/kneuharth/projects/django-simple-history/.eggs/Django-2.0.4-py3.5.egg/django/db/models/base.py", line 769, in save_base
    update_fields=update_fields, raw=raw, using=using,
  File "/Users/kneuharth/projects/django-simple-history/.eggs/Django-2.0.4-py3.5.egg/django/dispatch/dispatcher.py", line 178, in send
    for receiver in self._live_receivers(sender)
  File "/Users/kneuharth/projects/django-simple-history/.eggs/Django-2.0.4-py3.5.egg/django/dispatch/dispatcher.py", line 178, in <listcomp>
    for receiver in self._live_receivers(sender)
  File "/Users/kneuharth/projects/django-simple-history/simple_history/models.py", line 316, in post_save
    self.create_historical_record(instance, created and '+' or '~')
  File "/Users/kneuharth/projects/django-simple-history/simple_history/models.py", line 332, in create_historical_record
    history_change_reason=history_change_reason, **attrs)
  File "/Users/kneuharth/projects/django-simple-history/.eggs/Django-2.0.4-py3.5.egg/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/Users/kneuharth/projects/django-simple-history/.eggs/Django-2.0.4-py3.5.egg/django/db/models/query.py", line 415, in create
    obj = self.model(**kwargs)
  File "/Users/kneuharth/projects/django-simple-history/.eggs/Django-2.0.4-py3.5.egg/django/db/models/base.py", line 495, in __init__
    raise TypeError("'%s' is an invalid keyword argument for this function" % kwarg)
TypeError: 'history_change_reason' is an invalid keyword argument for this function

* Fix indexing

* Cleanup and passing tests now.

Abandoned the max_length flag because TextField doesn’t use it.

* Cleanup after pep8 and pyflakes and start of docs.

* Finish the documentation for usage of customizations.

* Sleight documentation tweaks.

Author: only-entertainment

AUTHORS.rst

@@ -32,6 +32,7 @@ Authors
 - Jonathan Sanchez
 - Josh Fyne
 - Klaas van Schelven
+- Kris Neuharth
 - Maciej "RooTer" Urbański
 - Martin Bachwerk
 - Marty Alchin

CHANGES.rst

@@ -3,6 +3,7 @@ Changes
 
 Unreleased
 ----------
+- Add ability to specify custom history_reason field (gh-379)
 - Add ability to specify custom history_id field (gh-368)
 - Add HistoricalRecord instance properties `prev_record` and `next_record` (gh-365)
 - Can set admin methods as attributes on object history change list template (gh-390)

docs/usage.rst

@@ -167,6 +167,55 @@ admin class
 .. image:: screens/5_history_list_display.png
 
 
+Customizations
+----------------
+
+UUID as `history_id`
+~~~~~~~~~~~~~~~~~~~~
+
+The ``HistoricalRecords`` object can be customized to use an ``UUIDField`` instead
+of the default ``IntegerField`` as the object `history_id` either through
+Django settings or via the constructor on the model.
+
+.. code-block:: python
+
+    SIMPLE_HISTORY_HISTORY_ID_USE_UUID = True
+
+or
+
+.. code-block:: python
+
+    class UUIDExample(models.Model):
+        id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
+        history = HistoricalRecords(
+            history_id_field=models.UUIDField(default=uuid.uuid4)
+        )
+
+
+TextField as `history_change_reason`
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The ``HistoricalRecords`` object can be customized to accept a
+``TextField`` model field for saving the
+`history_change_reason` either through settings or via the constructor on the
+model. The common use case for this is for supporting larger model change
+histories to support changelog-like features.
+
+.. code-block:: python
+
+    SIMPLE_HISTORY_HISTORY_CHANGE_REASON_USE_TEXT_FIELD=True
+
+or
+
+.. code-block:: python
+
+    class TextFieldExample(models.Model):
+        greeting = models.CharField(max_length=100)
+        history = HistoricalRecords(
+            history_change_reason_field=models.TextField(null=True)
+        )
+
+
 Querying history
 ----------------
 

simple_history/models.py

@@ -29,13 +29,15 @@ class HistoricalRecords(object):
 
     def __init__(self, verbose_name=None, bases=(models.Model,),
                  user_related_name='+', table_name=None, inherit=False,
+                 excluded_fields=None,
                  history_id_field=None,
-                 excluded_fields=None):
+                 history_change_reason_field=None):
         self.user_set_verbose_name = verbose_name
         self.user_related_name = user_related_name
         self.table_name = table_name
         self.inherit = inherit
         self.history_id_field = history_id_field
+        self.history_change_reason_field = history_change_reason_field
         if excluded_fields is None:
             excluded_fields = []
         self.excluded_fields = excluded_fields
@@ -250,11 +252,25 @@ def get_prev_record(self):
         else:
             history_id_field = models.AutoField(primary_key=True)
 
+        if self.history_change_reason_field:
+            # User specific field from init
+            history_change_reason_field = self.history_change_reason_field
+        elif getattr(
+            settings, 'SIMPLE_HISTORY_HISTORY_CHANGE_REASON_USE_TEXT_FIELD',
+            False
+        ):
+            # Use text field with no max length, not enforced by DB anyways
+            history_change_reason_field = models.TextField(null=True)
+        else:
+            # Current default, with max length
+            history_change_reason_field = models.CharField(
+                max_length=100, null=True
+            )
+
         return {
             'history_id': history_id_field,
             'history_date': models.DateTimeField(),
-            'history_change_reason': models.CharField(max_length=100,
-                                                      null=True),
+            'history_change_reason': history_change_reason_field,
             'history_user': models.ForeignKey(
                 user_model, null=True, related_name=self.user_related_name,
                 on_delete=models.SET_NULL),
@@ -306,6 +322,7 @@ 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 self.fields_included(instance):

simple_history/registry_tests/tests.py

@@ -101,8 +101,8 @@ def test_tracked_abstract_base(self):
                 for f in TrackedWithAbstractBase.history.model._meta.fields
             ],
             [
-                'id', 'history_id', 'history_date',
-                'history_change_reason', 'history_user_id',
+                'id', 'history_id',
+                'history_change_reason', 'history_date', 'history_user_id',
                 'history_type',
             ],
         )
@@ -115,7 +115,7 @@ def test_tracked_concrete_base(self):
             ],
             [
                 'id', 'trackedconcretebase_ptr_id', 'history_id',
-                'history_date', 'history_change_reason', 'history_user_id',
+                'history_change_reason', 'history_date', 'history_user_id',
                 'history_type',
             ],
         )
@@ -131,7 +131,7 @@ 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_change_reason',
+                'history_change_reason', 'history_date',
                 'history_user_id', 'history_type',
             ],
         )
@@ -141,7 +141,7 @@ 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_change_reason',
+                'history_change_reason', 'history_date',
                 'history_user_id', 'history_type',
             ],
         )
@@ -151,7 +151,7 @@ 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_change_reason',
+                'history_change_reason', 'history_date',
                 'history_user_id', 'history_type',
             ],
         )

simple_history/tests/models.py

@@ -433,3 +433,28 @@ class UUIDDefaultModel(models.Model):
 
 # Clear the SIMPLE_HISTORY_HISTORY_ID_USE_UUID
 delattr(settings, 'SIMPLE_HISTORY_HISTORY_ID_USE_UUID')
+
+
+# Set the SIMPLE_HISTORY_HISTORY_CHANGE_REASON_FIELD
+setattr(settings, 'SIMPLE_HISTORY_HISTORY_CHANGE_REASON_USE_TEXT_FIELD', True)
+
+
+class TextFieldChangeReasonModel1(models.Model):
+    greeting = models.CharField(max_length=100)
+    history = HistoricalRecords()
+
+
+# Clear the SIMPLE_HISTORY_HISTORY_CHANGE_REASON_FIELD
+delattr(settings, 'SIMPLE_HISTORY_HISTORY_CHANGE_REASON_USE_TEXT_FIELD')
+
+
+class TextFieldChangeReasonModel2(models.Model):
+    greeting = models.CharField(max_length=100)
+    history = HistoricalRecords(
+        history_change_reason_field=models.TextField(null=True)
+    )
+
+
+class CharFieldChangeReasonModel(models.Model):
+    greeting = models.CharField(max_length=100)
+    history = HistoricalRecords()

simple_history/tests/tests/test_models.py

@@ -53,7 +53,10 @@
     UnicodeVerboseName,
     UUIDModel,
     UUIDDefaultModel,
-    WaterLevel
+    WaterLevel,
+    TextFieldChangeReasonModel1,
+    TextFieldChangeReasonModel2,
+    CharFieldChangeReasonModel,
 )
 
 get_model = apps.get_model
@@ -382,6 +385,57 @@ def test_uuid_default_history_id(self):
         history = entry.history.all()[0]
         self.assertTrue(isinstance(history.history_id, uuid.UUID))
 
+    def test_default_history_change_reason(self):
+        entry = CharFieldChangeReasonModel.objects.create(
+            greeting="what's up?"
+        )
+        history = entry.history.get()
+
+        self.assertEqual(history.history_change_reason, None)
+
+    def test_charfield_history_change_reason(self):
+        # Default CharField and length
+        entry = CharFieldChangeReasonModel.objects.create(
+            greeting="what's up?"
+        )
+        entry.greeting = "what is happening?"
+        entry.save()
+        update_change_reason(entry, 'Change greeting.')
+
+        history = entry.history.all()[0]
+        field = history._meta.get_field('history_change_reason')
+
+        self.assertTrue(isinstance(field, models.CharField))
+        self.assertTrue(field.max_length, 100)
+
+    def test_textfield_history_change_reason1(self):
+        # TextField usage is determined by settings
+        entry = TextFieldChangeReasonModel1.objects.create(
+            greeting="what's up?"
+        )
+        entry.greeting = "what is happening?"
+        entry.save()
+        update_change_reason(entry, 'Change greeting.')
+
+        history = entry.history.all()[0]
+        field = history._meta.get_field('history_change_reason')
+
+        self.assertTrue(isinstance(field, models.TextField))
+
+    def test_textfield_history_change_reason2(self):
+        # TextField instance is passed in init
+        entry = TextFieldChangeReasonModel2.objects.create(
+            greeting="what's up?"
+        )
+        entry.greeting = "what is happening?"
+        entry.save()
+        update_change_reason(entry, 'Change greeting.')
+
+        history = entry.history.all()[0]
+        field = history._meta.get_field('history_change_reason')
+
+        self.assertTrue(isinstance(field, models.TextField))
+
     def test_get_prev_record(self):
         poll = Poll(question="what's up?", pub_date=today)
         poll.save()