Hotfix/update change reason with excluded fields (#637)

* update_change_reason error with excluded fields

If you have excluded fields update_change_reason generates an error, The instance will have values that won't match at line 18:
history.filter(**attrs)

Proposal change do avoid the error.

* Test and minor additional correction.

* Make compatible with other names for the history related field

* Ran `make format`

* Added authors and changes

Co-authored-by: Carlos-San-Emeterio <8135331+Carlos-San-Emeterio@users.noreply.github.com>
Co-authored-by: carlos.sanemeterio <carlos.sanemeterio@gdsmodellica.com>
Co-authored-by: Jonathan loo <jloo@travelanalytics.com.au>

Author: 4 people

AUTHORS.rst

@@ -20,6 +20,7 @@ Authors
 - Brian Armstrong (`barm <https://github.com/barm>`_)
 - Buddy Lindsey, Jr.
 - Brian Dixon
+- Carlos San Emeterio (`Carlos-San-Emeterio <https://github.com/Carlos-San-Emeterio>`_)
 - Christopher Broderick (`uhurusurfa <https://github.com/uhurusurfa>`_)
 - Corey Bertram
 - Craig Maloney (`craigmaloney <https://github.com/craigmaloney>`_)
@@ -52,6 +53,7 @@ Authors
 - `jofusa <https://github.com/jofusa>`_
 - John Whitlock
 - Jonathan Leroy
+- Jonathan Loo (`alpha1d3d <https://github.com/alpha1d3d>`_)
 - Jonathan Sanchez
 - Jonathan Zvesper (`zvesp <https://github.com/zvesp>`_)
 - Josh Fyne

CHANGES.rst

@@ -7,6 +7,7 @@ Unreleased
 - Add setting to convert `FileField` to `CharField` instead of `TextField` (gh-623)
 - import model `ContentType` in `SimpleHistoryAdmin` using `django_apps.get_model`
   to avoid possible `AppRegistryNotReady` exception (gh-630)
+- Fix `utils.update_change_reason` when user specifies excluded_fields
 
 2.8.0 (2019-12-02)
 ------------------

simple_history/tests/tests/test_utils.py

@@ -11,7 +11,7 @@
     PollWithExcludeFields,
     Street,
 )
-from simple_history.utils import bulk_create_with_history
+from simple_history.utils import bulk_create_with_history, update_change_reason
 
 
 class BulkCreateWithHistoryTestCase(TestCase):
@@ -144,3 +144,14 @@ def test_bulk_create_no_ids_return(self, hist_manager_mock):
         hist_manager_mock().bulk_history_create.assert_called_with(
             objects, batch_size=None
         )
+
+
+class UpdateChangeReasonTestCase(TestCase):
+    def test_update_change_reason_with_excluded_fields(self):
+        poll = PollWithExcludeFields(
+            question="what's up?", pub_date=now(), place="The Pub"
+        )
+        poll.save()
+        update_change_reason(poll, "Test change reason.")
+        most_recent = poll.history.order_by("-history_date").first()
+        self.assertEqual(most_recent.history_change_reason, "Test change reason.")

simple_history/utils.py

@@ -8,14 +8,18 @@ def update_change_reason(instance, reason):
     attrs = {}
     model = type(instance)
     manager = instance if instance.id is not None else model
+    history = get_history_manager_for_model(manager)
+    history_fields = [field.attname for field in history.model._meta.fields]
     for field in instance._meta.fields:
+        if field.attname not in history_fields:
+            continue
         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
-    history = get_history_manager_for_model(manager)
+
     record = history.filter(**attrs).order_by("-history_date").first()
     record.history_change_reason = reason
     record.save()