FieldTracker should patch save_base instead of save (#404)

* add test for reproducing FieldTracker update_fields issue

FieldTracker does not see update_fields changed in tracked model.save() method

* Patch Model.save_base instead of save

This change allows proper capturing update_fields kwarg if is is changed in overridden save() method.

* Add some details about FieldTracker implementation

* Mention FieldTracker behavior change

CHANGES.rst

@@ -4,6 +4,8 @@ CHANGES
 4.0.1 (unreleased)
 ------------------
 - `FieldTracker` now marks fields as not changed after `refresh_from_db`
+- `FieldTracker` now respects `update_fields` changed in overridden `save()`
+  method
 
 4.0.0 (2019-12-11)
 ------------------

docs/utilities.rst

@@ -258,12 +258,12 @@ It should be noted that a generic FieldTracker tracks Foreign Keys by db_column
         tracker = FieldTracker()
 
 .. code-block:: pycon
-    
+
     >>> p = Parent.objects.create(name='P')
     >>> c = Child.objects.create(name='C', parent=p)
     >>> c.tracker.has_changed('parent_id')
-    
-    
+
+
 To find the db_column names of your model (using the above example):
 
 .. code-block:: pycon
@@ -273,10 +273,10 @@ To find the db_column names of your model (using the above example):
     ('id', 'id')
     ('name', 'name')
     ('parent_id', 'parent_id')
-    
+
 
 The model field name *may* be used when tracking with a specific tracker:
- 
+
 .. code-block:: python
 
     specific_tracker = FieldTracker(fields=['parent'])
@@ -295,3 +295,49 @@ signal handlers to identify field changes on model save.
     Due to the implementation of ``FieldTracker``, ``post_save`` signal
     handlers relying on field tracker methods should only be registered after
     model creation.
+
+FieldTracker implementation details
+-----------------------------------
+
+.. code-block:: python
+
+    from django.db import models
+    from model_utils import FieldTracker, TimeStampedModel
+
+    class MyModel(TimeStampedModel):
+        name = models.CharField(max_length=64)
+        tracker = FieldTracker()
+
+        def save(self, *args, **kwargs):
+            """ Automatically add "modified" to update_fields."""
+            update_fields = kwargs.get('update_fields')
+            if update_fields is not None:
+                kwargs['update_fields'] = set(update_fields) | {'modified'}
+            super().save(*args, **kwargs)
+
+    # [...]
+
+    instance = MyModel.objects.first()
+    instance.name = 'new'
+    instance.save(update_fields={'name'})
+
+This is how ``FieldTracker`` tracks field changes on ``instance.save`` call.
+
+1. In ``class_prepared`` handler ``FieldTracker`` patches ``save_base`` and
+   ``refresh_from_db`` methods to reset initial state for tracked fields.
+2. In ``post_init`` handler ``FieldTracker`` saves initial values for tracked
+   fields.
+3. ``MyModel.save`` changes ``update_fields`` in order to store auto updated
+   ``modified`` timestamp. Complete list of saved fields is now known.
+4. ``Model.save`` does nothing interesting except calling ``save_base``.
+5. Decorated ``save_base()`` method calls ``super().save_base`` and all fields
+   that have values different to initial are considered as changed.
+6. ``Model.save_base`` sends ``pre_save`` signal, saves instance to database and
+   sends ``post_save`` signal. All ``pre_save/post_save`` receivers can query
+   ``instance.tracker`` for a set of changed fields etc.
+7. After ``Model.save_base`` return ``FieldTracker`` resets initial state for
+   updated fields (if no ``update_fields`` passed - whole initial state is
+   reset).
+8. ``instance.refresh_from_db()`` call causes initial state reset like for
+   ``save_base()``.
+

model_utils/tracker.py

@@ -234,7 +234,7 @@ class FieldTracker:
         instance._instance_intialized = True
 
     def patch_save(self, model):
-        self._patch(model, 'save', 'update_fields')
+        self._patch(model, 'save_base', 'update_fields')
         self._patch(model, 'refresh_from_db', 'fields')
 
     def _patch(self, model, method, fields_kwarg):

tests/models.py

@@ -247,6 +247,21 @@ class Tracked(models.Model):
         super().save(*args, **kwargs)
 
 
+class TrackerTimeStamped(TimeStampedModel):
+    name = models.CharField(max_length=20)
+    number = models.IntegerField()
+    mutable = MutableField(default=None)
+
+    tracker = FieldTracker()
+
+    def save(self, *args, **kwargs):
+        """ Automatically add "modified" to update_fields."""
+        update_fields = kwargs.get('update_fields')
+        if update_fields is not None:
+            kwargs['update_fields'] = set(update_fields) | {'modified'}
+        super().save(*args, **kwargs)
+
+
 class TrackedFK(models.Model):
     fk = models.ForeignKey('Tracked', on_delete=models.CASCADE)
 

tests/test_fields/test_field_tracker.py

@@ -6,7 +6,7 @@ from model_utils import FieldTracker
 from model_utils.tracker import DescriptorWrapper
 from tests.models import (
     Tracked, TrackedFK, InheritedTrackedFK, TrackedNotDefault, TrackedNonFieldAttr, TrackedMultiple,
-    InheritedTracked, TrackedFileField, TrackedAbstract,
+    InheritedTracked, TrackedFileField, TrackedAbstract, TrackerTimeStamped,
     ModelTracked, ModelTrackedFK, ModelTrackedNotDefault, ModelTrackedMultiple, InheritedModelTracked,
 )
 
@@ -523,6 +523,30 @@ class FieldTrackerForeignKeyTests(FieldTrackerTestCase):
         self.assertCurrent(fk=self.instance.fk_id)
 
 
+class FieldTrackerTimeStampedTests(FieldTrackerTestCase):
+
+    fk_class = Tracked
+    tracked_class = TrackerTimeStamped
+
+    def setUp(self):
+        self.instance = self.tracked_class.objects.create(name='old', number=1)
+        self.tracker = self.instance.tracker
+
+    def test_set_modified_on_save(self):
+        old_modified = self.instance.modified
+        self.instance.name = 'new'
+        self.instance.save()
+        self.assertGreater(self.instance.modified, old_modified)
+        self.assertChanged()
+
+    def test_set_modified_on_save_update_fields(self):
+        old_modified = self.instance.modified
+        self.instance.name = 'new'
+        self.instance.save(update_fields=('name',))
+        self.assertGreater(self.instance.modified, old_modified)
+        self.assertChanged()
+
+
 class InheritedFieldTrackerTests(FieldTrackerTests):
 
     tracked_class = InheritedTracked