Merge pull request #295 from nottinghamtec/reversion-upgrade

Reversion upgrade and versioning.py refactor

.travis.yml

@@ -1,4 +1,4 @@
-sudo: false
+sudo: true
 dist: trusty
 
 language: python

RIGS/models.py

@@ -9,6 +9,7 @@ from django.utils import timezone
 from django.utils.functional import cached_property
 from django.utils.encoding import python_2_unicode_compatible
 from reversion import revisions as reversion
+from reversion.models import Version
 import string
 
 import random
@@ -61,32 +62,31 @@ class Profile(AbstractUser):
 
 
 class RevisionMixin(object):
+    @property
+    def current_version(self):
+        version = Version.objects.get_for_object(self).select_related('revision').first()
+        return version
+
     @property
     def last_edited_at(self):
-        versions = reversion.get_for_object(self)
-        if versions:
-            version = reversion.get_for_object(self)[0]
-            return version.revision.date_created
-        else:
+        version = self.current_version
+        if version is None:
             return None
+        return version.revision.date_created
 
     @property
     def last_edited_by(self):
-        versions = reversion.get_for_object(self)
-        if versions:
-            version = reversion.get_for_object(self)[0]
-            return version.revision.user
-        else:
+        version = self.current_version
+        if version is None:
             return None
+        return version.revision.user
 
     @property
     def current_version_id(self):
-        versions = reversion.get_for_object(self)
-        if versions:
-            version = reversion.get_for_object(self)[0]
-            return "V{0} | R{1}".format(version.pk, version.revision.pk)
-        else:
+        version = self.current_version
+        if version is None:
             return None
+        return "V{0} | R{1}".format(version.pk, version.revision.pk)
 
 
 @reversion.register

RIGS/signals.py

@@ -3,7 +3,7 @@ import re
 import urllib2
 from io import BytesIO
 
-import reversion
+from django.db.models.signals import post_save
 from PyPDF2 import PdfFileReader, PdfFileMerger
 from django.conf import settings
 from django.contrib.staticfiles.storage import staticfiles_storage
@@ -89,10 +89,9 @@ def send_eventauthorisation_success_email(instance):
     mic_email.send(fail_silently=True)
 
 
-def on_revision_commit(instances, **kwargs):
-    for instance in instances:
-        if isinstance(instance, models.EventAuthorisation):
-            send_eventauthorisation_success_email(instance)
+def on_revision_commit(sender, instance, created, **kwargs):
+    if created:
+        send_eventauthorisation_success_email(instance)
 
 
-reversion.revisions.post_revision_commit.connect(on_revision_commit)
+post_save.connect(on_revision_commit, sender=models.EventAuthorisation)

RIGS/templates/RIGS/activity_feed_data.html

@@ -33,13 +33,13 @@
                 {% endif %}
                       <p>
                       <small>
-                        {% if version.old == None %}
+                        {% if version.changes.old == None %}
                           Created
                         {% else %}
                           Changed {% include 'RIGS/version_changes.html' %} in
                         {% endif %}
                           
-                        {% include 'RIGS/object_button.html' with object=version.new %}
+                        {% include 'RIGS/object_button.html' with object=version.changes.new %}
                         {% if version.revision.comment %}
                           ({{ version.revision.comment }})
                         {% endif %}

RIGS/templates/RIGS/activity_table.html

@@ -67,16 +67,16 @@
                         
                             <tr>
                                 <td>{{ version.revision.date_created }}</td>
-                                <td><a href="{{ version.new.get_absolute_url }}">{{version.new|to_class_name}} {{ version.new.pk|stringformat:"05d" }}</a></td>
-                           		<td>{{ version.version.pk }}|{{ version.revision.pk }}</td>
+                                <td><a href="{{ version.changes.new.get_absolute_url }}">{{version.changes.new|to_class_name}} {{ version.changes.new.pk|stringformat:"05d" }}</a></td>
+                           		<td>{{ version.pk }}|{{ version.revision.pk }}</td>
                                 <td>{{ version.revision.user.name }}</td>
                                 <td>
-                                  {% if version.old == None %}
-                                    {{version.new|to_class_name}} Created
+                                  {% if version.changes.old == None %}
+                                    {{version.changes.new|to_class_name}} Created
                                   {% else %}
                                     {% include 'RIGS/version_changes.html' %}
                                   {% endif %}                        </td>
-                                  <td>{{ version.revision.comment }}</td>
+                                  <td>{{ version.changes.revision.comment }}</td>
                             </tr>
                         
         			{% endfor %}

RIGS/templates/RIGS/version_changes.html

@@ -1,21 +1,26 @@
-{% for change in version.field_changes %}
+{% if version.changes.item_changes or version.changes.field_changes or version.changes.old == None %}
 
-	<button title="Changes to {{ change.field.verbose_name }}" type="button" class="btn btn-default btn-xs" data-container="body" data-html="true" data-trigger='hover' data-toggle="popover" data-content='{% spaceless %}
-        {% include "RIGS/version_changes_change.html" %}
-        {% endspaceless %}'>{{ change.field.verbose_name }}</button>
+    {% for change in version.changes.field_changes %}
 
-{% endfor %}
+    	<button title="Changes to {{ change.field.verbose_name }}" type="button" class="btn btn-default btn-xs" data-container="body" data-html="true" data-trigger='hover' data-toggle="popover" data-content='{% spaceless %}
+            {% include "RIGS/version_changes_change.html" %}
+            {% endspaceless %}'>{{ change.field.verbose_name }}</button>
 
-{% for itemChange in version.item_changes %}
-    <button title="Changes to item '{% if itemChange.new %}{{ itemChange.new.name }}{% else %}{{ itemChange.old.name }}{% endif %}'" type="button" class="btn btn-default btn-xs" data-container="body" data-html="true" data-trigger='hover' data-toggle="popover" data-content='{% spaceless %}
-        <ul class="list-group">
-        {% for change in itemChange.changes %}
-              <li class="list-group-item">
-                <h4 class="list-group-item-heading">{{ change.field.verbose_name }}</h4>
-                {% include "RIGS/version_changes_change.html" %}
-              </li>
-        {% endfor %}
-        </ul>
-        
-        {% endspaceless %}'>item '{% if itemChange.new %}{{ itemChange.new.name }}{% else %}{{ itemChange.old.name }}{% endif %}'</button>
-{% endfor %}
+    {% endfor %}
+
+    {% for itemChange in version.changes.item_changes %}
+        <button title="Changes to item '{% if itemChange.new %}{{ itemChange.new.name }}{% else %}{{ itemChange.old.name }}{% endif %}'" type="button" class="btn btn-default btn-xs" data-container="body" data-html="true" data-trigger='hover' data-toggle="popover" data-content='{% spaceless %}
+            <ul class="list-group">
+            {% for change in itemChange.field_changes %}
+                  <li class="list-group-item">
+                    <h4 class="list-group-item-heading">{{ change.field.verbose_name }}</h4>
+                    {% include "RIGS/version_changes_change.html" %}
+                  </li>
+            {% endfor %}
+            </ul>
+            
+            {% endspaceless %}'>item '{% if itemChange.new %}{{ itemChange.new.name }}{% else %}{{ itemChange.old.name }}{% endif %}'</button>
+    {% endfor %}
+{% else %}
+    nothing useful
+{% endif %}

RIGS/templates/RIGS/version_history.html

@@ -40,13 +40,13 @@
                 </thead>
                 <tbody>
                 	{% for version in object_list %}
-                        {% if version.item_changes or version.field_changes or version.old == None %}
+                        
                             <tr>
                            		<td>{{ version.revision.date_created }}</td>
-                                <td>{{ version.version.pk }}|{{ version.revision.pk }}</td>
+                                <td>{{ version.pk }}|{{ version.revision.pk }}</td>
                                 <td>{{ version.revision.user.name }}</td>
                                 <td>
-                                    {% if version.old == None %}
+                                    {% if version.changes.old is None %}
                                         {{object|to_class_name}} Created
                                     {% else %}
                                         {% include 'RIGS/version_changes.html' %}
@@ -56,7 +56,8 @@
                                     {{ version.revision.comment }}
                                 </td>
                             </tr>
-                        {% endif %}
+
+                        
         			{% endfor %}
 
                 </tbody>

RIGS/test_models.py

@@ -1,9 +1,11 @@
+from __future__ import unicode_literals
+
 import pytz
-import reversion
+from reversion import revisions as reversion
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.test import TestCase
-from RIGS import models
+from RIGS import models, versioning
 from datetime import date, timedelta, datetime, time
 from decimal import *
 
@@ -386,7 +388,191 @@ class EventAuthorisationTestCase(TestCase):
         self.assertTrue(self.event.authorised)
 
     def test_last_edited(self):
-        with reversion.revisions.create_revision():
+        with reversion.create_revision():
             auth = models.EventAuthorisation.objects.create(event=self.event, email="authroisation@model.test.case",
                                                         name="Test Auth", amount=self.event.total, sent_by=self.profile)
         self.assertIsNotNone(auth.last_edited_at)
+
+
+class RIGSVersionTestCase(TestCase):
+    def setUp(self):
+        models.VatRate.objects.create(rate=0.20, comment="TP V1", start_at='2013-01-01')
+
+        self.profile = models.Profile.objects.get_or_create(
+            first_name='Test',
+            last_name='TEC User',
+            username='eventauthtest',
+            email='teccie@functional.test',
+            is_superuser=True  # lazily grant all permissions
+        )[0]
+        with reversion.create_revision():
+            reversion.set_user(self.profile)
+            self.person = models.Person.objects.create(name='Authorisation Test Person')
+
+        with reversion.create_revision():
+            reversion.set_user(self.profile)
+            self.organisation = models.Organisation.objects.create(name='Authorisation Test Organisation')
+
+        with reversion.create_revision():
+            reversion.set_user(self.profile)
+            self.event = models.Event.objects.create(name="AuthorisationTestCase", person=self.person,
+                                                     start_date=date.today())
+
+        with reversion.create_revision():
+            reversion.set_user(self.profile)
+            self.event.notes = "A new note on the event"
+            self.event.save()
+
+    def test_find_parent_version(self):
+        # Find the most recent version
+        currentVersion = versioning.RIGSVersion.objects.get_for_object(self.event).latest(field_name='revision__date_created')
+        self.assertEqual(currentVersion._object_version.object.notes, "A new note on the event")
+
+        # Check the prev version is loaded correctly
+        previousVersion = currentVersion.parent
+        self.assertEqual(previousVersion._object_version.object.notes, None)
+
+        # Check that finding the parent of the first version fails gracefully
+        self.assertFalse(previousVersion.parent)
+
+    def test_changes_since(self):
+        # Find the most recent version
+        currentVersion = versioning.RIGSVersion.objects.get_for_object(self.event).latest(field_name='revision__date_created')
+
+        changes = currentVersion.changes
+        self.assertEqual(len(changes.field_changes), 1)
+
+    def test_manager(self):
+        objs = versioning.RIGSVersion.objects.get_for_multiple_models([models.Event, models.Person, models.Organisation])
+        self.assertEqual(len(objs), 4)
+
+    def test_text_field_types(self):
+        with reversion.create_revision():
+            reversion.set_user(self.profile)
+            self.event.name = "New event name"  # Simple text
+            self.event.description = "hello world"  # Long text
+            self.event.save()
+
+        # Find the most recent version
+        currentVersion = versioning.RIGSVersion.objects.get_for_object(self.event).latest(field_name='revision__date_created')
+        diff = currentVersion.changes
+
+        # There are two changes
+        self.assertEqual(len(diff.field_changes), 2)
+        self.assertFalse(currentVersion.changes.items_changed)
+        self.assertTrue(currentVersion.changes.fields_changed)
+        self.assertTrue(currentVersion.changes.anything_changed)
+
+        # Only one has "linebreaks"
+        self.assertEqual(sum([x.linebreaks for x in diff.field_changes]), 1)
+
+        # None are "long" (email address)
+        self.assertEqual(sum([x.long for x in diff.field_changes]), 0)
+
+        # Try changing email field in person
+        with reversion.create_revision():
+            reversion.set_user(self.profile)
+            self.person.email = "hello@world.com"
+            self.person.save()
+
+        # Find the most recent version
+        currentVersion = versioning.RIGSVersion.objects.get_for_object(self.person).latest(field_name='revision__date_created')
+        diff = currentVersion.changes
+
+        # Should be declared as long
+        self.assertTrue(diff.field_changes[0].long)
+
+    def test_text_diff(self):
+        with reversion.create_revision():
+            reversion.set_user(self.profile)
+            self.event.notes = "An old note on the event"  # Simple text
+            self.event.save()
+
+        # Find the most recent version
+        currentVersion = versioning.RIGSVersion.objects.get_for_object(self.event).latest(field_name='revision__date_created')
+
+        # Check the diff is correct
+        self.assertEqual(currentVersion.changes.field_changes[0].diff,
+                         [{'type': 'equal', 'text': "A"},
+                         {'type': 'delete', 'text': " new"},
+                         {'type': 'insert', 'text': "n old"},
+                         {'type': 'equal', 'text': " note on the event"}
+                          ])
+
+    def test_choice_field(self):
+        with reversion.create_revision():
+            reversion.set_user(self.profile)
+            self.event.status = models.Event.CONFIRMED
+            self.event.save()
+
+        currentVersion = versioning.RIGSVersion.objects.get_for_object(self.event).latest(field_name='revision__date_created')
+        self.assertEqual(currentVersion.changes.field_changes[0].old, 'Provisional')
+        self.assertEqual(currentVersion.changes.field_changes[0].new, 'Confirmed')
+
+    def test_creation_behaviour(self):
+        firstVersion = versioning.RIGSVersion.objects.get_for_object(self.event).latest(field_name='revision__date_created').parent
+        diff = firstVersion.changes
+
+        # Mainly to check for exceptions:
+        self.assertTrue(len(diff.field_changes) > 0)
+
+    def test_event_items(self):
+        with reversion.create_revision():
+            reversion.set_user(self.profile)
+            item1 = models.EventItem.objects.create(event=self.event, name="TI I1", quantity=1, cost=1.00, order=1)
+            self.event.save()
+
+        # Find the most recent version
+        currentVersion = versioning.RIGSVersion.objects.get_for_object(self.event).latest(field_name='revision__date_created')
+
+        diffs = currentVersion.changes.item_changes
+
+        self.assertEqual(len(diffs), 1)
+        self.assertTrue(currentVersion.changes.items_changed)
+        self.assertFalse(currentVersion.changes.fields_changed)
+        self.assertTrue(currentVersion.changes.anything_changed)
+
+        self.assertTrue(diffs[0].old is None)
+        self.assertEqual(diffs[0].new.name, "TI I1")
+
+        # Edit the item
+        with reversion.create_revision():
+            reversion.set_user(self.profile)
+            item1.name = "New Name"
+            item1.save()
+            self.event.save()
+
+        currentVersion = versioning.RIGSVersion.objects.get_for_object(self.event).latest(field_name='revision__date_created')
+
+        diffs = currentVersion.changes.item_changes
+
+        self.assertEqual(len(diffs), 1)
+
+        self.assertEqual(diffs[0].old.name, "TI I1")
+        self.assertEqual(diffs[0].new.name, "New Name")
+
+        # Check the diff
+        self.assertEqual(currentVersion.changes.item_changes[0].field_changes[0].diff,
+                         [{'type': 'delete', 'text': "TI I1"},
+                         {'type': 'insert', 'text': "New Name"},
+                          ])
+
+        # Delete the item
+
+        with reversion.create_revision():
+            item1.delete()
+            self.event.save()
+
+        # Find the most recent version
+        currentVersion = versioning.RIGSVersion.objects.get_for_object(self.event).latest(field_name='revision__date_created')
+
+        diffs = currentVersion.changes.item_changes
+
+        self.assertEqual(len(diffs), 1)
+        self.assertTrue(currentVersion.changes.items_changed)
+        self.assertFalse(currentVersion.changes.fields_changed)
+        self.assertTrue(currentVersion.changes.anything_changed)
+
+        self.assertEqual(diffs[0].old.name, "New Name")
+        self.assertTrue(diffs[0].new is None)
+        

RIGS/test_unit.py

@@ -294,6 +294,39 @@ class TestVersioningViews(TestCase):
         response = self.client.get(request_url, follow=True)
         self.assertEqual(response.status_code, 200)
 
+    # Some edge cases that have caused server errors in the past
+    def test_deleted_event(self):
+        request_url = reverse('activity_feed')
+
+        self.events[2].delete()
+
+        response = self.client.get(request_url, follow=True)
+        self.assertContains(response, "TE E2")
+        self.assertEqual(response.status_code, 200)
+
+    def test_deleted_relation(self):
+        request_url = reverse('activity_feed')
+
+        with reversion.create_revision():
+            person = models.Person.objects.create(name="Test Person")
+        with reversion.create_revision():
+            self.events[1].person = person
+            self.events[1].save()
+
+        # Check response contains person
+        response = self.client.get(request_url, follow=True)
+        self.assertContains(response, "Test Person")
+        self.assertEqual(response.status_code, 200)
+
+        # Delete person
+        person.delete()
+
+        # Check response still contains person
+        response = self.client.get(request_url, follow=True)
+        self.assertContains(response, "Test Person")
+        self.assertEqual(response.status_code, 200)
+
+
 
 class TestEmbeddedViews(TestCase):
     @classmethod

RIGS/versioning.py

@@ -1,303 +1,259 @@
+from __future__ import unicode_literals
+
 import logging
+import datetime
 
 from django.core.exceptions import ObjectDoesNotExist
 from django.shortcuts import get_object_or_404
 from django.views import generic
-
-# Versioning
-import reversion
-from reversion.models import Version
-from django.contrib.contenttypes.models import ContentType  # Used to lookup the content_type
+from django.utils.functional import cached_property
 from django.db.models import IntegerField, EmailField, TextField
+from django.contrib.contenttypes.models import ContentType
+
+from reversion.models import Version, VersionQuerySet
 from diff_match_patch import diff_match_patch
 
 from RIGS import models
-import datetime
 
 logger = logging.getLogger('tec.pyrigs')
 
 
-def model_compare(oldObj, newObj, excluded_keys=[]):
-    # recieves two objects of the same model, and compares them. Returns an array of FieldCompare objects
-    try:
-        theFields = oldObj._meta.fields  # This becomes deprecated in Django 1.8!!!!!!!!!!!!! (but an alternative becomes available)
-    except AttributeError:
-        theFields = newObj._meta.fields
+class FieldComparison(object):
+    def __init__(self, field=None, old=None, new=None):
+        self.field = field
+        self._old = old
+        self._new = new
 
-    class FieldCompare(object):
-        def __init__(self, field=None, old=None, new=None):
-            self.field = field
-            self._old = old
-            self._new = new
+    def display_value(self, value):
+        if isinstance(self.field, IntegerField) and len(self.field.choices) > 0:
+            return [x[1] for x in self.field.choices if x[0] == value][0]
+        return value
 
-        def display_value(self, value):
-            if isinstance(self.field, IntegerField) and len(self.field.choices) > 0:
-                return [x[1] for x in self.field.choices if x[0] == value][0]
-            return value
+    @property
+    def old(self):
+        return self.display_value(self._old)
 
-        @property
-        def old(self):
-            return self.display_value(self._old)
+    @property
+    def new(self):
+        return self.display_value(self._new)
 
-        @property
-        def new(self):
-            return self.display_value(self._new)
-
-        @property
-        def long(self):
-            if isinstance(self.field, EmailField):
-                return True
-            return False
-
-        @property
-        def linebreaks(self):
-            if isinstance(self.field, TextField):
-                return True
-            return False
-
-        @property
-        def diff(self):
-            oldText = unicode(self.display_value(self._old)) or ""
-            newText = unicode(self.display_value(self._new)) or ""
-            dmp = diff_match_patch()
-            diffs = dmp.diff_main(oldText, newText)
-            dmp.diff_cleanupSemantic(diffs)
-
-            outputDiffs = []
-
-            for (op, data) in diffs:
-                if op == dmp.DIFF_INSERT:
-                    outputDiffs.append({'type': 'insert', 'text': data})
-                elif op == dmp.DIFF_DELETE:
-                    outputDiffs.append({'type': 'delete', 'text': data})
-                elif op == dmp.DIFF_EQUAL:
-                    outputDiffs.append({'type': 'equal', 'text': data})
-            return outputDiffs
-
-    changes = []
-
-    for thisField in theFields:
-        name = thisField.name
-
-        if name in excluded_keys:
-            continue  # if we're excluding this field, skip over it
-
-        try:
-            oldValue = getattr(oldObj, name, None)
-        except ObjectDoesNotExist:
-            oldValue = None
-
-        try:
-            newValue = getattr(newObj, name, None)
-        except ObjectDoesNotExist:
-            newValue = None
-
-        try:
-            bothBlank = (not oldValue) and (not newValue)
-            if oldValue != newValue and not bothBlank:
-                compare = FieldCompare(thisField, oldValue, newValue)
-                changes.append(compare)
-        except TypeError:  # logs issues with naive vs tz-aware datetimes
-            logger.error('TypeError when comparing models')
-
-    return changes
-
-
-def compare_event_items(old, new):
-    # Recieves two event version objects and compares their items, returns an array of ItemCompare objects
-
-    item_type = ContentType.objects.get_for_model(models.EventItem)
-    old_item_versions = old.revision.version_set.filter(content_type=item_type)
-    new_item_versions = new.revision.version_set.filter(content_type=item_type)
-
-    class ItemCompare(object):
-        def __init__(self, old=None, new=None, changes=None):
-            self.old = old
-            self.new = new
-            self.changes = changes
-
-    # Build some dicts of what we have
-    item_dict = {}  # build a list of items, key is the item_pk
-    for version in old_item_versions:  # put all the old versions in a list
-        if version.field_dict["event"] == old.object_id_int:
-            compare = ItemCompare(old=version.object_version.object)
-            item_dict[version.object_id] = compare
-
-    for version in new_item_versions:  # go through the new versions
-        if version.field_dict["event"] == new.object_id_int:
-            try:
-                compare = item_dict[version.object_id]  # see if there's a matching old version
-                compare.new = version.object_version.object  # then add the new version to the dictionary
-            except KeyError:  # there's no matching old version, so add this item to the dictionary by itself
-                compare = ItemCompare(new=version.object_version.object)
-
-            item_dict[version.object_id] = compare  # update the dictionary with the changes
-
-    changes = []
-    for (_, compare) in item_dict.items():
-        compare.changes = model_compare(compare.old, compare.new, ['id', 'event', 'order'])  # see what's changed
-        if len(compare.changes) >= 1:
-            changes.append(compare)  # transfer into a sequential array to make it easier to deal with later
-
-    return changes
-
-
-def get_versions_for_model(models):
-    content_types = []
-    for model in models:
-        content_types.append(ContentType.objects.get_for_model(model))
-
-    versions = reversion.models.Version.objects.filter(
-        content_type__in=content_types,
-    ).select_related("revision").order_by("-pk")
-
-    return versions
-
-
-def get_previous_version(version):
-    thisId = version.object_id
-    thisVersionId = version.pk
-
-    versions = reversion.revisions.get_for_object_reference(version.content_type.model_class(), thisId)
-
-    try:
-        previousVersions = versions.filter(revision_id__lt=version.revision_id).latest(
-            field_name='revision__date_created')
-    except ObjectDoesNotExist:
+    @property
+    def long(self):
+        if isinstance(self.field, EmailField):
+            return True
         return False
 
-    return previousVersions
+    @property
+    def linebreaks(self):
+        if isinstance(self.field, TextField):
+            return True
+        return False
+
+    @property
+    def diff(self):
+        oldText = unicode(self.display_value(self._old)) or ""
+        newText = unicode(self.display_value(self._new)) or ""
+        dmp = diff_match_patch()
+        diffs = dmp.diff_main(oldText, newText)
+        dmp.diff_cleanupSemantic(diffs)
+
+        outputDiffs = []
+
+        for (op, data) in diffs:
+            if op == dmp.DIFF_INSERT:
+                outputDiffs.append({'type': 'insert', 'text': data})
+            elif op == dmp.DIFF_DELETE:
+                outputDiffs.append({'type': 'delete', 'text': data})
+            elif op == dmp.DIFF_EQUAL:
+                outputDiffs.append({'type': 'equal', 'text': data})
+        return outputDiffs
 
 
-def get_changes_for_version(newVersion, oldVersion=None):
-    # Pass in a previous version if you already know it (for efficiancy)
-    # if not provided then it will be looked up in the database
+class ModelComparison(object):
 
-    if oldVersion == None:
-        oldVersion = get_previous_version(newVersion)
+    def __init__(self, old=None, new=None, version=None, excluded_keys=[]):
+        # recieves two objects of the same model, and compares them. Returns an array of FieldCompare objects
+        try:
+            self.fields = old._meta.get_fields()
+        except AttributeError:
+            self.fields = new._meta.get_fields()
 
-    modelClass = newVersion.content_type.model_class()
+        self.old = old
+        self.new = new
+        self.excluded_keys = excluded_keys
+        self.version = version
 
-    compare = {
-        'revision': newVersion.revision,
-        'new': newVersion.object_version.object,
-        'current': modelClass.objects.filter(pk=newVersion.pk).first(),
-        'version': newVersion,
+    @cached_property
+    def revision(self):
+        return self.version.revision
 
-        # Old things that may not be used
-        'old': None,
-        'field_changes': None,
-        'item_changes': None,
-    }
+    @cached_property
+    def field_changes(self):
+        changes = []
+        for field in self.fields:
+            field_name = field.name
 
-    if oldVersion:
-        compare['old'] = oldVersion.object_version.object
-        compare['field_changes'] = model_compare(compare['old'], compare['new'])
-        compare['item_changes'] = compare_event_items(oldVersion, newVersion)
+            if field_name in self.excluded_keys:
+                continue  # if we're excluding this field, skip over it
 
-    return compare
+            try:
+                oldValue = getattr(self.old, field_name, None)
+            except ObjectDoesNotExist:
+                oldValue = None
+
+            try:
+                newValue = getattr(self.new, field_name, None)
+            except ObjectDoesNotExist:
+                newValue = None
+
+            bothBlank = (not oldValue) and (not newValue)
+            if oldValue != newValue and not bothBlank:
+                comparison = FieldComparison(field, oldValue, newValue)
+                changes.append(comparison)
+
+        return changes
+
+    @cached_property
+    def fields_changed(self):
+        return len(self.field_changes) > 0
+
+    @cached_property
+    def item_changes(self):
+        # Recieves two event version objects and compares their items, returns an array of ItemCompare objects
+
+        item_type = ContentType.objects.get_for_model(models.EventItem)
+        old_item_versions = self.version.parent.revision.version_set.filter(content_type=item_type)
+        new_item_versions = self.version.revision.version_set.filter(content_type=item_type)
+
+        comparisonParams = {'excluded_keys': ['id', 'event', 'order']}
+
+        # Build some dicts of what we have
+        item_dict = {}  # build a list of items, key is the item_pk
+        for version in old_item_versions:  # put all the old versions in a list
+            if version.field_dict["event_id"] == int(self.new.pk):
+                compare = ModelComparison(old=version._object_version.object, **comparisonParams)
+                item_dict[version.object_id] = compare
+
+        for version in new_item_versions:  # go through the new versions
+            if version.field_dict["event_id"] == int(self.new.pk):
+                try:
+                    compare = item_dict[version.object_id]  # see if there's a matching old version
+                    compare.new = version._object_version.object  # then add the new version to the dictionary
+                except KeyError:  # there's no matching old version, so add this item to the dictionary by itself
+                    compare = ModelComparison(new=version._object_version.object, **comparisonParams)
+
+                item_dict[version.object_id] = compare  # update the dictionary with the changes
+
+        changes = []
+        for (_, compare) in item_dict.items():
+            if compare.fields_changed:
+                changes.append(compare)
+
+        return changes
+
+    @cached_property
+    def items_changed(self):
+        return len(self.item_changes) > 0
+
+    @cached_property
+    def anything_changed(self):
+        return self.fields_changed or self.items_changed
+
+
+class RIGSVersionManager(VersionQuerySet):
+    def get_for_multiple_models(self, model_array):
+        content_types = []
+        for model in model_array:
+            content_types.append(ContentType.objects.get_for_model(model))
+
+        return self.filter(content_type__in=content_types).select_related("revision").order_by("-pk")
+
+
+class RIGSVersion(Version):
+    class Meta:
+        proxy = True
+
+    objects = RIGSVersionManager.as_manager()
+
+    @cached_property
+    def parent(self):
+        thisId = self.object_id
+
+        versions = RIGSVersion.objects.get_for_object_reference(self.content_type.model_class(), thisId).select_related("revision", "revision__user").all()
+
+        try:
+            previousVersion = versions.filter(revision_id__lt=self.revision_id).latest(
+                field_name='revision__date_created')
+        except ObjectDoesNotExist:
+            return False
+
+        return previousVersion
+
+    @cached_property
+    def changes(self):
+        return ModelComparison(
+            version=self,
+            new=self._object_version.object,
+            old=self.parent._object_version.object if self.parent else None
+        )
 
 
 class VersionHistory(generic.ListView):
-    model = Version
+    model = RIGSVersion
     template_name = "RIGS/version_history.html"
     paginate_by = 25
 
     def get_queryset(self, **kwargs):
         thisModel = self.kwargs['model']
 
-        # thisObject = get_object_or_404(thisModel, pk=self.kwargs['pk'])
-        versions = reversion.revisions.get_for_object_reference(thisModel, self.kwargs['pk'])
+        versions = RIGSVersion.objects.get_for_object_reference(thisModel, self.kwargs['pk']).select_related("revision", "revision__user").all()
 
         return versions
 
     def get_context_data(self, **kwargs):
         thisModel = self.kwargs['model']
-
         context = super(VersionHistory, self).get_context_data(**kwargs)
-
-        versions = context['object_list']
         thisObject = get_object_or_404(thisModel, pk=self.kwargs['pk'])
-
-        items = []
-
-        for versionNo, thisVersion in enumerate(versions):
-            if versionNo >= len(versions) - 1:
-                thisItem = get_changes_for_version(thisVersion, None)
-            else:
-                thisItem = get_changes_for_version(thisVersion, versions[versionNo + 1])
-
-            items.append(thisItem)
-
-        context['object_list'] = items
         context['object'] = thisObject
 
         return context
 
 
 class ActivityTable(generic.ListView):
-    model = Version
+    model = RIGSVersion
     template_name = "RIGS/activity_table.html"
     paginate_by = 25
 
     def get_queryset(self):
-        versions = get_versions_for_model([models.Event, models.Venue, models.Person, models.Organisation, models.EventAuthorisation])
+        versions = RIGSVersion.objects.get_for_multiple_models([models.Event, models.Venue, models.Person, models.Organisation, models.EventAuthorisation])
         return versions
 
-    def get_context_data(self, **kwargs):
-        # Call the base implementation first to get a context
-        context = super(ActivityTable, self).get_context_data(**kwargs)
-
-        items = []
-
-        for thisVersion in context['object_list']:
-            thisItem = get_changes_for_version(thisVersion, None)
-            items.append(thisItem)
-
-        context['object_list'] = items
-
-        return context
-
 
 class ActivityFeed(generic.ListView):
-    model = Version
+    model = RIGSVersion
     template_name = "RIGS/activity_feed_data.html"
     paginate_by = 25
 
     def get_queryset(self):
-        versions = get_versions_for_model([models.Event, models.Venue, models.Person, models.Organisation, models.EventAuthorisation])
+        versions = RIGSVersion.objects.get_for_multiple_models([models.Event, models.Venue, models.Person, models.Organisation, models.EventAuthorisation])
         return versions
 
     def get_context_data(self, **kwargs):
-        maxTimeDelta = []
-
-        maxTimeDelta.append({'maxAge': datetime.timedelta(days=1), 'group': datetime.timedelta(hours=1)})
-        maxTimeDelta.append({'maxAge': None, 'group': datetime.timedelta(days=1)})
-
         # Call the base implementation first to get a context
         context = super(ActivityFeed, self).get_context_data(**kwargs)
 
+        maxTimeDelta = datetime.timedelta(hours=1)
+
         items = []
 
         for thisVersion in context['object_list']:
-            thisItem = get_changes_for_version(thisVersion, None)
-            if thisItem['item_changes'] or thisItem['field_changes'] or thisItem['old'] == None:
-                thisItem['withPrevious'] = False
-                if len(items) >= 1:
-                    timeAgo = datetime.datetime.now(thisItem['revision'].date_created.tzinfo) - thisItem[
-                        'revision'].date_created
-                    timeDiff = items[-1]['revision'].date_created - thisItem['revision'].date_created
-                    timeTogether = False
-                    for params in maxTimeDelta:
-                        if params['maxAge'] is None or timeAgo <= params['maxAge']:
-                            timeTogether = timeDiff < params['group']
-                            break
+            thisVersion.withPrevious = False
+            if len(items) >= 1:
+                timeDiff = items[-1].revision.date_created - thisVersion.revision.date_created
+                timeTogether = timeDiff < maxTimeDelta
+                sameUser = thisVersion.revision.user_id == items[-1].revision.user_id
+                thisVersion.withPrevious = timeTogether & sameUser
 
-                    sameUser = thisItem['revision'].user == items[-1]['revision'].user
-                    thisItem['withPrevious'] = timeTogether & sameUser
-
-                items.append(thisItem)
-
-        context['object_list'] = items
+            items.append(thisVersion)
 
         return context

requirements.txt

@@ -3,12 +3,12 @@ contextlib2==0.5.5
 diff-match-patch==20121119
 dj-database-url==0.4.2
 dj-static==0.0.6
-Django==1.11.1
+Django==1.11.2
 django-debug-toolbar==1.8
 django-ical==1.4
 django-recaptcha==1.3.0
 django-registration-redux==1.6
-django-reversion==1.10.2
+django-reversion==2.0.9
 django-toolbelt==0.0.1
 premailer==3.0.1
 django-widget-tweaks==1.4.1

runtime.txt

@@ -0,0 +1 @@
+python-2.7.13