Properly handle eventauthorisations in new versioning

It's not great, not terrible...

RIGS/admin.py

@@ -20,7 +20,6 @@ admin.site.register(models.VatRate, VersionAdmin)
 admin.site.register(models.Event, VersionAdmin)
 admin.site.register(models.EventItem, VersionAdmin)
 admin.site.register(models.Invoice, VersionAdmin)
-admin.site.register(models.Payment, VersionAdmin)
 
 
 def approve_user(modeladmin, request, queryset):

RIGS/management/commands/generateSampleRIGSData.py

@@ -219,7 +219,7 @@ class Command(BaseCommand):
 
         financeUser = models.Profile.objects.create(username="finance", first_name="Finance", last_name="User",
                                                     initials="FU",
-                                                    email="financeuser@example.com", is_active=True)
+                                                    email="financeuser@example.com", is_active=True, is_approved=True)
         financeUser.groups.add(self.finance_group)
         financeUser.groups.add(self.keyholder_group)
         financeUser.set_password('finance')
@@ -227,7 +227,7 @@ class Command(BaseCommand):
 
         hsUser = models.Profile.objects.create(username="hs", first_name="HS", last_name="User",
                                                initials="HSU",
-                                               email="hsuser@example.com", is_active=True)
+                                               email="hsuser@example.com", is_active=True, is_approved=True)
         hsUser.groups.add(self.hs_group)
         hsUser.groups.add(self.keyholder_group)
         hsUser.set_password('hs')
@@ -235,13 +235,13 @@ class Command(BaseCommand):
 
         keyholderUser = models.Profile.objects.create(username="keyholder", first_name="Keyholder", last_name="User",
                                                       initials="KU",
-                                                      email="keyholderuser@example.com", is_active=True)
+                                                      email="keyholderuser@example.com", is_active=True, is_approved=True)
         keyholderUser.groups.add(self.keyholder_group)
         keyholderUser.set_password('keyholder')
         keyholderUser.save()
 
         basicUser = models.Profile.objects.create(username="basic", first_name="Basic", last_name="User", initials="BU",
-                                                  email="basicuser@example.com", is_active=True)
+                                                  email="basicuser@example.com", is_active=True, is_approved=True)
         basicUser.set_password('basic')
         basicUser.save()
 

RIGS/models.py

@@ -518,6 +518,10 @@ class EventItem(models.Model, RevisionMixin):
     def __str__(self):
         return str(self.event.pk) + "." + str(self.order) + ": " + self.event.name + " | " + self.name
 
+    @property
+    def activity_feed_string(self):
+        return str("item {}".format(self.name))
+
 
 @reversion.register
 class EventAuthorisation(models.Model, RevisionMixin):
@@ -529,12 +533,14 @@ class EventAuthorisation(models.Model, RevisionMixin):
     amount = models.DecimalField(max_digits=10, decimal_places=2, verbose_name="authorisation amount")
     sent_by = models.ForeignKey('Profile', on_delete=models.CASCADE)
 
+    #reversion_perm = 'RIGS.view_eventauthorisation'
+
     def get_absolute_url(self):
         return reverse_lazy('event_detail', kwargs={'pk': self.event.pk})
 
     @property
     def activity_feed_string(self):
-        return str("N%05d" % self.event.pk + ' (requested by ' + self.sent_by.initials + ')')
+        return "{} (requested by {})".format(self.event.display_id, self.sent_by.initials)
 
 
 @reversion.register(follow=['payment_set'])
@@ -607,6 +613,10 @@ class Payment(models.Model, RevisionMixin):
     def __str__(self):
         return "%s: %d" % (self.get_method_display(), self.amount)
 
+    @property
+    def activity_feed_string(self):
+        return str("payment at £{}".format(self.amount))
+
 
 @reversion.register
 class RiskAssessment(models.Model, RevisionMixin):

RIGS/templates/eventauthorisation.html

@@ -36,7 +36,7 @@
     <div class="row mb-3">
         <div class="col-sm-12">
             <div class="card">
-            {% with object=event %}
+            {% with object=event auth=True %}
                 {% include 'item_table.html' %}
             {% endwith %}
             </div>

RIGS/templates/item_modal.html

@@ -3,6 +3,9 @@
         <div class="modal-content">
             <div class="modal-header">
                  <h4>{{ object.name|default:"New Event" }}</h4>
+                <button type="button" class="close" data-dismiss="modal" aria-label="Close">
+                  <span aria-hidden="true">&times;</span>
+                </button>
             </div>
             <form id="item-form">
                 <div class="modal-body">

RIGS/templates/item_table.html

@@ -26,7 +26,7 @@
 				{% include 'item_row.html' %}
 			{% endfor %}
 		</tbody>
-        {% if perms.RIGS.view_event %}
+        {% if auth or perms.RIGS.view_event %}
 		<tfoot>
 			<tr>
 				<td rowspan="3" colspan="2"></td>

versioning/templates/partials/activity_table_body.html

@@ -19,7 +19,7 @@
             <td>{{ version.revision.user.name|default:"System" }}</td>
             <td>
               {% if version.changes.old == None %}
-                {{version.changes.new|to_class_name}} Created
+                Created {{version.changes.new|to_class_name}}
               {% else %}
                 {% include 'partials/version_changes.html' %}
               {% endif %}

versioning/templates/partials/version_changes.html

@@ -1,4 +1,4 @@
-{% if version.changes.anything_changed or version.changes.old == None %}
+{% if version.changes.anything_changed %}
     {% for change in version.changes.field_changes %}
     	<span title="Changes to {{ change.field.verbose_name }}" class="badge badge-info p-2" data-container="body" data-html="true" data-trigger='hover' data-toggle="popover" data-content='{% spaceless %}{% include "partials/version_changes_change.html" %}{% endspaceless %}'>{{ change.field.verbose_name }}</span>
     {% endfor %}
@@ -7,8 +7,9 @@
             <ul class="list-group list-group-flush">
             {% for change in itemchange.field_changes %}
                   <li class="list-group-item">
-                    <p>{{ change.field.verbose_name|title }}:</p>
+                    <div class="dont-break-out">
-                    <div class="dont-break-out">{% include 'partials/version_changes_change.html' with change=change %}</div>
+                    {{ change.field.verbose_name|title }}:
+                    {% include 'partials/version_changes_change.html' with change=change %}</div>
                   </li>
             {% endfor %}
             </ul>

versioning/versioning.py

@@ -4,7 +4,7 @@ import logging
 from diff_match_patch import diff_match_patch
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ObjectDoesNotExist
-from django.db.models import EmailField, IntegerField, TextField
+from django.db.models import EmailField, IntegerField, TextField, CharField
 from django.shortcuts import get_object_or_404
 from django.utils.functional import cached_property
 from django.views import generic
@@ -24,8 +24,11 @@ class FieldComparison(object):
         self._new = new
 
     def display_value(self, value):
-        if isinstance(self.field, IntegerField) and self.field.choices is not None and len(self.field.choices) > 0:
+        if (isinstance(self.field, IntegerField) or isinstance(self.field, CharField)) and self.field.choices is not None and len(self.field.choices) > 0:
-            return [x[1] for x in self.field.choices if x[0] == value][0]
+            choice = [x[1] for x in self.field.choices if x[0] == value]
+            # TODO This defensive piece should not be necessary?
+            if len(choice) > 0:
+                return choice[0]
         return value
 
     @property
@@ -91,24 +94,21 @@ class ModelComparison(object):
         changes = []
         for field in self.fields:
             field_name = field.name
+            if field_name not in self.excluded_keys:  # if we're excluding this field, skip over it
+                try:
+                    oldValue = getattr(self.old, field_name, None)
+                except ObjectDoesNotExist:
+                    oldValue = None
 
-            if field_name in self.excluded_keys:
+                try:
-                continue  # if we're excluding this field, skip over it
+                    newValue = getattr(self.new, field_name, None)
+                except ObjectDoesNotExist:
+                    newValue = None
 
-            try:
+                bothBlank = (not oldValue) and (not newValue)
-                oldValue = getattr(self.old, field_name, None)
+                if oldValue != newValue and not bothBlank:
-            except ObjectDoesNotExist:
+                    comparison = FieldComparison(field, oldValue, newValue)
-                oldValue = None
+                    changes.append(comparison)
-
-            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
 
@@ -121,19 +121,17 @@ class ModelComparison(object):
         if self.follow and self.version.object is not None:
             item_type = ContentType.objects.get_for_model(self.version.object)
             old_item_versions = self.version.parent.revision.version_set.exclude(content_type=item_type)
-            new_item_versions = self.version.revision.version_set.exclude(content_type=item_type)
+            new_item_versions = self.version.revision.version_set.exclude(content_type=item_type).exclude(content_type=ContentType.objects.get_for_model(models.EventAuthorisation))
 
-            comparisonParams = {'excluded_keys': ['id', 'event', 'order', 'checklist', 'invoice']}
+            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
@@ -141,9 +139,15 @@ class ModelComparison(object):
                     compare = ModelComparison(new=version._object_version.object, **comparisonParams)
 
                 if compare.new:
-                    compare.name = str(compare.new)
+                    if(hasattr(compare.new, 'activity_feed_string')):
+                        compare.name = compare.new.activity_feed_string
+                    else:
+                        compare.name = str(compare.new)
                 else:
-                    compare.name = str(compare.old)
+                    if(hasattr(compare.old, 'activity_feed_string')):
+                        compare.name = compare.old.activity_feed_string
+                    else:
+                        compare.name = str(compare.old)
 
                 item_dict[version.object_id] = compare  # update the dictionary with the changes
 
@@ -156,7 +160,7 @@ class ModelComparison(object):
 
     @cached_property
     def items_changed(self):
-        return len(self.item_changes) > 0
+        return self.item_changes is not None and len(self.item_changes) > 0
 
     @cached_property
     def anything_changed(self):