From 6bbb9079837e9a3273045f922d34279efad2fc02 Mon Sep 17 00:00:00 2001 From: Kyle Seever Date: Tue, 10 Jul 2018 17:48:12 -0400 Subject: [PATCH 01/10] Implement the ability to diff HistoricalRecords (Credit goes to leportella for the initial commit) --- AUTHORS.rst | 1 + docs/advanced.rst | 18 ++++++++++ simple_history/models.py | 43 ++++++++++++++++++++++- simple_history/tests/tests/test_models.py | 27 +++++++++++++- 4 files changed, 87 insertions(+), 2 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 226d27a04..ba25a7aec 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -35,6 +35,7 @@ Authors - Kris Neuharth - Maciej "RooTer" Urbański - Mark Davidoff +- Leticia Portella - Martin Bachwerk - Marty Alchin - Mauricio de Abreu Antunes diff --git a/docs/advanced.rst b/docs/advanced.rst index 261f157f3..adcc6dc44 100644 --- a/docs/advanced.rst +++ b/docs/advanced.rst @@ -406,3 +406,21 @@ If you want to save a model without a historical record, you can use the followi poll = Poll(question='something') poll.save_without_historical_record() + +History Diffing +------------------- + +When you have two instances of the same HistoricalRecord, such as HistoricalPoll example above, +you can perform diffs to see what changed. This will result in a ModelDelta containing some properties such as a +list with each field changed between this historical records, a list with the names of all +fields that incurred changes from one record to another, and the old and new records. +This may be useful when you want timelines, and need to get only the model modifications. + +.. code-block:: python + + p = Poll.objects.create(question="what's up?") + p.question = "what's up, man?" + p.save() + + new_record, old_record = p.history.all() + delta = new_record.diff_against(old_record) diff --git a/simple_history/models.py b/simple_history/models.py index 629fcaf14..a0615bcab 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -53,7 +53,7 @@ def __init__(self, verbose_name=None, bases=(models.Model,), try: if isinstance(bases, six.string_types): raise TypeError - self.bases = tuple(bases) + self.bases = (HistoricalChanges,) + tuple(bases) except TypeError: raise TypeError("The `bases` option must be a list or a tuple.") @@ -412,3 +412,44 @@ def __get__(self, instance, owner): values = (getattr(instance, f.attname) for f in self.fields_included) return self.model(*values) + + +class HistoricalChanges(object): + def diff_against(self, old_history): + if not isinstance(old_history, type(self)): + raise TypeError(("unsupported operand type(s) for diffing: " + "'{}' and '{}'").format( + type(self), + type(old_history))) + + model_delta_class = getattr(self, 'model_delta_class', ModelDelta) + changes = [] + changed_fields = [] + for field in self._meta.fields: + if hasattr(self.instance, field.name) and \ + hasattr(old_history.instance, field.name): + old_value = getattr(old_history, field.name, '') + new_value = getattr(self, field.name) + if old_value != new_value: + changes.append(ModelChange(field.name, old_value, new_value)) + changed_fields.append(field.name) + + return model_delta_class(changes, + changed_fields=changed_fields, + old_record=old_history, + new_record=self) + + +class ModelChange(object): + def __init__(self, field_name, old_value, new_value): + self.field = field_name + self.old = old_value + self.new = new_value + + +class ModelDelta(object): + def __init__(self, changes, changed_fields, old_record, new_record): + self.changes = changes + self.changed_fields = changed_fields + self.old_record = old_record + self.new_record = new_record diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index dfa9ac17b..89deb75af 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -12,7 +12,11 @@ from django.db.models.fields.proxy import OrderWrt from django.test import TestCase -from simple_history.models import HistoricalRecords, convert_auto_field +from simple_history.models import ( + HistoricalRecords, + convert_auto_field, + ModelChange +) from simple_history.utils import update_change_reason from ..external.models import ExternalModel2, ExternalModel4 from ..models import ( @@ -549,6 +553,27 @@ def test_get_next_record_none_if_most_recent(self): recent_record = poll.history.filter(question="ask questions?").get() self.assertIsNone(recent_record.next_record) + def test_history_diff(self): + p = Poll.objects.create(question="what's up?", pub_date=today) + p.question = "what's up, man?" + p.save() + new_record, old_record = p.history.all() + delta = new_record.diff_against(old_record) + expected_change = ModelChange("question", + "what's up?", + "what's up, man") + self.assertEqual(delta.changed_fields, ['question']) + self.assertEqual(delta.old_record, old_record) + self.assertEqual(delta.new_record, new_record) + self.assertEqual(expected_change.field, delta.changes[0].field) + + def test_history_diff_with_incorrect_type(self): + p = Poll.objects.create(question="what's up?", pub_date=today) + p.question = "what's up, man?" + p.save() + new_record, old_record = p.history.all() + with self.assertRaises(TypeError): + delta = new_record.diff_against('something') class CreateHistoryModelTests(unittest.TestCase): From 447972b9bc337ea18ce2363aaa61e9f6c28a48aa Mon Sep 17 00:00:00 2001 From: Kyle Seever Date: Tue, 10 Jul 2018 18:19:15 -0400 Subject: [PATCH 02/10] Fixup documentation --- docs/advanced.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/advanced.rst b/docs/advanced.rst index adcc6dc44..584779d96 100644 --- a/docs/advanced.rst +++ b/docs/advanced.rst @@ -410,11 +410,11 @@ If you want to save a model without a historical record, you can use the followi History Diffing ------------------- -When you have two instances of the same HistoricalRecord, such as HistoricalPoll example above, -you can perform diffs to see what changed. This will result in a ModelDelta containing some properties such as a -list with each field changed between this historical records, a list with the names of all -fields that incurred changes from one record to another, and the old and new records. -This may be useful when you want timelines, and need to get only the model modifications. +When you have two instances of the same HistoricalRecord (such as the HistoricalPoll example above), +you can perform diffs to see what changed. This will result in a ModelDelta containing properties including +a list with each field changed between the two historical records, a list with the names of all +fields that incurred changes from one record to the other, and the old and new records. +This may be useful when you want to construct timelines and need to get only the model modifications. .. code-block:: python From c915b6042ba1058d3665e333f707d68c7e07b905 Mon Sep 17 00:00:00 2001 From: Kyle Seever Date: Tue, 10 Jul 2018 18:58:32 -0400 Subject: [PATCH 03/10] More documentation sprucing --- docs/advanced.rst | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/advanced.rst b/docs/advanced.rst index 584779d96..4f3260f69 100644 --- a/docs/advanced.rst +++ b/docs/advanced.rst @@ -410,10 +410,11 @@ If you want to save a model without a historical record, you can use the followi History Diffing ------------------- -When you have two instances of the same HistoricalRecord (such as the HistoricalPoll example above), -you can perform diffs to see what changed. This will result in a ModelDelta containing properties including -a list with each field changed between the two historical records, a list with the names of all -fields that incurred changes from one record to the other, and the old and new records. +When you have two instances of the same ``HistoricalRecord`` (such as the ``HistoricalPoll`` example above), +you can perform diffs to see what changed. This will result in a ``ModelDelta`` containing the following properties: + 1. A list with each field changed between the two historical records + 2. A list with the names of all fields that incurred changes from one record to the other + 3. the old and new records. This may be useful when you want to construct timelines and need to get only the model modifications. .. code-block:: python @@ -424,3 +425,5 @@ This may be useful when you want to construct timelines and need to get only the new_record, old_record = p.history.all() delta = new_record.diff_against(old_record) + for change in delta.changes: + print("{} changed from {} to {}".format(change.field, change.old, change.new)) \ No newline at end of file From e7bdcd71dfcd7e4227345f39bdb9265a85388988 Mon Sep 17 00:00:00 2001 From: Kyle Seever Date: Tue, 10 Jul 2018 18:58:43 -0400 Subject: [PATCH 04/10] Remove vestigial operand terminology --- simple_history/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/simple_history/models.py b/simple_history/models.py index a0615bcab..e84a02139 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -417,7 +417,7 @@ def __get__(self, instance, owner): class HistoricalChanges(object): def diff_against(self, old_history): if not isinstance(old_history, type(self)): - raise TypeError(("unsupported operand type(s) for diffing: " + raise TypeError(("unsupported type(s) for diffing: " "'{}' and '{}'").format( type(self), type(old_history))) From 075d0936deb5c1d1011019b94870c7c072772091 Mon Sep 17 00:00:00 2001 From: Kyle Seever Date: Tue, 10 Jul 2018 19:00:07 -0400 Subject: [PATCH 05/10] Add to CHANGES.rst --- CHANGES.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 05f46b626..affd5868c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,10 @@ Changes ======= +Unreleased +----------------- +- Add ability to diff HistoricalRecords (gh-244) + 2.2.0 (2018-07-02) ------------------ - Add ability to specify alternative user_model for tracking (gh-371) From ae4b08ea5a414306076d6502fb69541d1e57101c Mon Sep 17 00:00:00 2001 From: Kyle Seever Date: Tue, 10 Jul 2018 21:50:30 -0400 Subject: [PATCH 06/10] Remove support for configurable modeL_delta_class --- simple_history/models.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/simple_history/models.py b/simple_history/models.py index e84a02139..9007b69bf 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -422,7 +422,6 @@ def diff_against(self, old_history): type(self), type(old_history))) - model_delta_class = getattr(self, 'model_delta_class', ModelDelta) changes = [] changed_fields = [] for field in self._meta.fields: @@ -434,10 +433,10 @@ def diff_against(self, old_history): changes.append(ModelChange(field.name, old_value, new_value)) changed_fields.append(field.name) - return model_delta_class(changes, - changed_fields=changed_fields, - old_record=old_history, - new_record=self) + return ModelDelta(changes, + changed_fields, + old_history, + self) class ModelChange(object): From 7eb290581f6f38f3b72aa72c5eecd95380550cd1 Mon Sep 17 00:00:00 2001 From: Kyle Seever Date: Wed, 11 Jul 2018 10:05:07 -0400 Subject: [PATCH 07/10] advanced.rst formatting --- docs/advanced.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/advanced.rst b/docs/advanced.rst index 4f3260f69..47a3356e6 100644 --- a/docs/advanced.rst +++ b/docs/advanced.rst @@ -412,9 +412,9 @@ History Diffing When you have two instances of the same ``HistoricalRecord`` (such as the ``HistoricalPoll`` example above), you can perform diffs to see what changed. This will result in a ``ModelDelta`` containing the following properties: - 1. A list with each field changed between the two historical records - 2. A list with the names of all fields that incurred changes from one record to the other - 3. the old and new records. +1. A list with each field changed between the two historical records +2. A list with the names of all fields that incurred changes from one record to the other +3. the old and new records. This may be useful when you want to construct timelines and need to get only the model modifications. .. code-block:: python From 4263a2594ae45fff79e1dae387a7a82685c0c1dd Mon Sep 17 00:00:00 2001 From: Kyle Seever Date: Wed, 11 Jul 2018 10:05:22 -0400 Subject: [PATCH 08/10] add test for unchanged fields in delta --- simple_history/tests/tests/test_models.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index 89deb75af..81f99ac1e 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -553,7 +553,7 @@ def test_get_next_record_none_if_most_recent(self): recent_record = poll.history.filter(question="ask questions?").get() self.assertIsNone(recent_record.next_record) - def test_history_diff(self): + def test_history_diff_includes_changed_fields(self): p = Poll.objects.create(question="what's up?", pub_date=today) p.question = "what's up, man?" p.save() @@ -567,6 +567,14 @@ def test_history_diff(self): self.assertEqual(delta.new_record, new_record) self.assertEqual(expected_change.field, delta.changes[0].field) + def test_history_diff_does_not_include_unchanged_fields(self): + p = Poll.objects.create(question="what's up?", pub_date=today) + p.question = "what's up, man?" + p.save() + new_record, old_record = p.history.all() + delta = new_record.diff_against(old_record) + self.assertNotIn('pub_date', delta.changed_fields) + def test_history_diff_with_incorrect_type(self): p = Poll.objects.create(question="what's up?", pub_date=today) p.question = "what's up, man?" @@ -575,6 +583,7 @@ def test_history_diff_with_incorrect_type(self): with self.assertRaises(TypeError): delta = new_record.diff_against('something') + class CreateHistoryModelTests(unittest.TestCase): def test_create_history_model_with_one_to_one_field_to_integer_field(self): From bb875bbdecf6676a93c651056b7eccf7a660b96b Mon Sep 17 00:00:00 2001 From: Kyle Seever Date: Wed, 11 Jul 2018 10:17:28 -0400 Subject: [PATCH 09/10] documentation formatting --- docs/advanced.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/advanced.rst b/docs/advanced.rst index 47a3356e6..4f3260f69 100644 --- a/docs/advanced.rst +++ b/docs/advanced.rst @@ -412,9 +412,9 @@ History Diffing When you have two instances of the same ``HistoricalRecord`` (such as the ``HistoricalPoll`` example above), you can perform diffs to see what changed. This will result in a ``ModelDelta`` containing the following properties: -1. A list with each field changed between the two historical records -2. A list with the names of all fields that incurred changes from one record to the other -3. the old and new records. + 1. A list with each field changed between the two historical records + 2. A list with the names of all fields that incurred changes from one record to the other + 3. the old and new records. This may be useful when you want to construct timelines and need to get only the model modifications. .. code-block:: python From cdb0e15bfcd11589d9d4b9d6acc14185edbc4440 Mon Sep 17 00:00:00 2001 From: Kyle Seever Date: Wed, 11 Jul 2018 10:38:14 -0400 Subject: [PATCH 10/10] address linter issues --- docs/advanced.rst | 8 +++++--- simple_history/models.py | 3 ++- simple_history/tests/tests/test_models.py | 6 +++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/docs/advanced.rst b/docs/advanced.rst index 4f3260f69..4d0ad7fb5 100644 --- a/docs/advanced.rst +++ b/docs/advanced.rst @@ -412,9 +412,11 @@ History Diffing When you have two instances of the same ``HistoricalRecord`` (such as the ``HistoricalPoll`` example above), you can perform diffs to see what changed. This will result in a ``ModelDelta`` containing the following properties: - 1. A list with each field changed between the two historical records - 2. A list with the names of all fields that incurred changes from one record to the other - 3. the old and new records. + +1. A list with each field changed between the two historical records +2. A list with the names of all fields that incurred changes from one record to the other +3. the old and new records. + This may be useful when you want to construct timelines and need to get only the model modifications. .. code-block:: python diff --git a/simple_history/models.py b/simple_history/models.py index 9007b69bf..3f427bcaa 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -430,7 +430,8 @@ def diff_against(self, old_history): old_value = getattr(old_history, field.name, '') new_value = getattr(self, field.name) if old_value != new_value: - changes.append(ModelChange(field.name, old_value, new_value)) + change = ModelChange(field.name, old_value, new_value) + changes.append(change) changed_fields.append(field.name) return ModelDelta(changes, diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index 81f99ac1e..4ffd48597 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -560,8 +560,8 @@ def test_history_diff_includes_changed_fields(self): new_record, old_record = p.history.all() delta = new_record.diff_against(old_record) expected_change = ModelChange("question", - "what's up?", - "what's up, man") + "what's up?", + "what's up, man") self.assertEqual(delta.changed_fields, ['question']) self.assertEqual(delta.old_record, old_record) self.assertEqual(delta.new_record, new_record) @@ -581,7 +581,7 @@ def test_history_diff_with_incorrect_type(self): p.save() new_record, old_record = p.history.all() with self.assertRaises(TypeError): - delta = new_record.diff_against('something') + new_record.diff_against('something') class CreateHistoryModelTests(unittest.TestCase):