Add revision history to invoices/payments.

Also patches previously introduced reversion permissions hole.

Supersedes and closes #337.

RIGS/admin.py

@@ -19,8 +19,8 @@ from reversion import revisions as reversion
 admin.site.register(models.VatRate, VersionAdmin)
 admin.site.register(models.Event, VersionAdmin)
 admin.site.register(models.EventItem, VersionAdmin)
-admin.site.register(models.Invoice)
-admin.site.register(models.Payment)
+admin.site.register(models.Invoice, VersionAdmin)
+admin.site.register(models.Payment, VersionAdmin)
 
 
 def approve_user(modeladmin, request, queryset):

RIGS/finance.py

@@ -13,6 +13,9 @@ from django.db.models import Q
 from z3c.rml import rml2pdf
 from django.db.models import Q
 
+from django.db import transaction
+import reversion
+
 from RIGS import models
 
 from django import forms
@@ -192,7 +195,10 @@ class InvoiceWaiting(generic.ListView):
 
 
 class InvoiceEvent(generic.View):
+    @transaction.atomic()
+    @reversion.create_revision()
     def get(self, *args, **kwargs):
+        reversion.set_user(self.request.user)
         epk = kwargs.get('pk')
         event = models.Event.objects.get(pk=epk)
         invoice, created = models.Invoice.objects.get_or_create(event=event)
@@ -223,6 +229,13 @@ class PaymentCreate(generic.CreateView):
         initial.update({'invoice': invoice})
         return initial
 
+    @transaction.atomic()
+    @reversion.create_revision()
+    def form_valid(self, form, *args, **kwargs):
+        reversion.add_to_revision(form.cleaned_data['invoice'])
+        reversion.set_comment("Payment added")
+        return super().form_valid(form, *args, **kwargs)
+
     def get_success_url(self):
         messages.info(self.request, "location.reload()")
         return reverse_lazy('closemodal')
@@ -232,5 +245,12 @@ class PaymentDelete(generic.DeleteView):
     model = models.Payment
     template_name = 'payment_confirm_delete.html'
 
+    @transaction.atomic()
+    @reversion.create_revision()
+    def delete(self, *args, **kwargs):
+        reversion.add_to_revision(self.get_object().invoice)
+        reversion.set_comment("Payment removed")
+        return super().delete(*args, **kwargs)
+
     def get_success_url(self):
         return self.request.POST.get('next')

RIGS/models.py

@@ -489,7 +489,8 @@ class Event(models.Model, RevisionMixin):
         super(Event, self).save(*args, **kwargs)
 
 
-class EventItem(models.Model):
+@reversion.register
+class EventItem(models.Model, RevisionMixin):
     event = models.ForeignKey('Event', related_name='items', blank=True, on_delete=models.CASCADE)
     name = models.CharField(max_length=255)
     description = models.TextField(blank=True, null=True)
@@ -528,11 +529,14 @@ class EventAuthorisation(models.Model, RevisionMixin):
         return str("N%05d" % self.event.pk + ' (requested by ' + self.sent_by.initials + ')')
 
 
-class Invoice(models.Model):
+@reversion.register(follow=['payment_set'])
+class Invoice(models.Model, RevisionMixin):
     event = models.OneToOneField('Event', on_delete=models.CASCADE)
     invoice_date = models.DateField(auto_now_add=True)
     void = models.BooleanField(default=False)
 
+    reversion_perm = 'RIGS.view_invoice'
+
     @property
     def sum_total(self):
         return self.event.sum_total
@@ -556,6 +560,13 @@ class Invoice(models.Model):
     def is_closed(self):
         return self.balance == 0 or self.void
 
+    def get_absolute_url(self):
+        return reverse_lazy('invoice_detail', kwargs={'pk': self.pk})
+
+    @property
+    def activity_feed_string(self):
+        return "#{} for Event {}".format(self.pk, "N%05d" % self.event.pk)
+
     def __str__(self):
         return "%i: %s (%.2f)" % (self.pk, self.event, self.balance)
 
@@ -563,7 +574,8 @@ class Invoice(models.Model):
         ordering = ['-invoice_date']
 
 
-class Payment(models.Model):
+@reversion.register
+class Payment(models.Model, RevisionMixin):
     CASH = 'C'
     INTERNAL = 'I'
     EXTERNAL = 'E'
@@ -582,6 +594,8 @@ class Payment(models.Model):
     amount = models.DecimalField(max_digits=10, decimal_places=2, help_text='Please use ex. VAT')
     method = models.CharField(max_length=2, choices=METHODS, null=True, blank=True)
 
+    reversion_hide = True
+
     def __str__(self):
         return "%s: %d" % (self.get_method_display(), self.amount)
 

RIGS/templates/invoice_detail.html

@@ -103,5 +103,8 @@
                 </div>
             </div>
         </div>
+        <div class="col-12 text-right">
+            {% include 'partials/last_edited.html' with target="invoice_history" %}
+        </div>
     </div>
 {% endblock %}

RIGS/templates/payment_confirm_delete.html

@@ -3,35 +3,19 @@
 {% block title %}Delete payment on invoice {{ object.invoice.pk }}{% endblock %}
 
 {% block content %}
-    <div class="col-sm-offset-2 col-sm-8">
-        <div class="alert alert-danger" role="alert">
-            <h2>Delete payment on invoice {{ object.invoice.pk }}</h2>
+<div class="alert alert-danger" role="alert">
+    <h2>Delete payment on invoice {{ object.invoice.pk }}</h2>
 
-            <p>Are you sure you wish to delete a payment for £{{ object.amount|floatformat:2 }}
-                ({{ object.get_method_display }})
-                from {{ object.date }} on invoice {{ object.invoice.pk }}.</p>
-
-            <p class="text-center"><strong>This action cannot be undone!</strong></p>
-
-            <div class="row">
-                <div class="col-sm-12">
-                    <form action="{{ action_link }}" method="post">{% csrf_token %}
-                        <input type="hidden" name="next" value="{% url 'invoice_detail' object.invoice.pk %}"/>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                        <input type="submit" value="Yes" class="btn btn-danger col-sm-1"/>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                        <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-default col-sm-1">No</a>
-                    </form>
-                </div>
-            </div>
-        </div>
+    <p>Are you sure you wish to delete a payment for £{{ object.amount|floatformat:2 }}
+        ({{ object.get_method_display }})
+        from {{ object.date }} on invoice {{ object.invoice.pk }}?</p>
+    <hr>
+    <div class="text-right">
+        <form action="{{ action_link }}" method="post">{% csrf_token %}
+            <input type="hidden" name="next" value="{% url 'invoice_detail' object.invoice.pk %}"/>
+            <input type="submit" value="Yes" class="btn btn-danger col-sm-1"/>
+            <a href="{% url 'invoice_detail' object.invoice.pk %}" class="btn btn-success col-sm-1">No</a>
+        </form>
     </div>
-{% endblock %}
+</div>
+{% endblock %}

assets/models.py

@@ -126,6 +126,8 @@ class Asset(models.Model, RevisionMixin):
     asset_id_prefix = models.CharField(max_length=8, default="")
     asset_id_number = models.IntegerField(default=1)
 
+    reversion_perm = 'assets.asset_finance'
+
     def get_available_asset_id(wanted_prefix=""):
         sql = """
         SELECT a.asset_id_number+1

versioning/urls.py

@@ -16,19 +16,29 @@ urlpatterns = [
          name='activity_feed'),
 ]
 
+# Well except this specific hack for legacy URLs...if only the RIGS app had been named 'rigboard'!
 for app in apps.get_app_configs():
     appname = str(app.label)
-    # Well except this specific hack for legacy URLs...if only the RIGS app had been named 'rigboard'!
+
     if appname == 'RIGS':
         appname = 'rigboard'
-        urlpatterns += [path(appname + '/activity/', permission_required_with_403('RIGS.view_event')(views.ActivityTable.as_view()),
-                             name='activity_table', kwargs={'app': appname, 'models': views.get_models(app.label)}), ]
+        table_name = 'activity_table'
     else:
-        urlpatterns += [path(appname + '/activity/', permission_required_with_403('RIGS.view_event')(views.ActivityTable.as_view()),
-                             name=appname + '_activity_table', kwargs={'app': appname, 'models': views.get_models(app.label)}), ]
-    for model in views.get_models(app.label):
+        table_name = appname + '_activity_table'
+
+    # TODO Permissions
+    urlpatterns += [path(appname + '/activity/', permission_required_with_403('RIGS.view_event')(views.ActivityTable.as_view()),
+                         name=table_name, kwargs={'app': appname, 'models': views.get_models(app.label)}), ]
+
+    for model in views.get_models(app=app.label):
         modelname = model.__name__.lower()
-        urlpatterns += [
-            path(appname + '/' + modelname + '/<str:pk>/history/', permission_required_with_403('{}.change_{}'.format(app.label, modelname))(views.VersionHistory.as_view()),
-                 name='{}_history'.format(modelname), kwargs={'model': model, 'app': appname, }),
-        ]
+        if appname == 'rigboard':
+            urlpatterns += [
+                path('{}/<str:pk>/history/'.format(modelname), permission_required_with_403('{}.change_{}'.format(app.label, modelname))(views.VersionHistory.as_view()),
+                     name='{}_history'.format(modelname), kwargs={'model': model, 'app': appname, }),
+            ]
+        else:
+            urlpatterns += [
+                path('{}/{}/<str:pk>/history/'.format(appname, modelname), permission_required_with_403('{}.change_{}'.format(app.label, modelname))(views.VersionHistory.as_view()),
+                     name='{}_history'.format(modelname), kwargs={'model': model, 'app': appname, }),
+            ]

versioning/views.py

@@ -52,13 +52,21 @@ def get_models(app=None):
     return models
 
 
+# TODO Default filter of having permission to view associated object
+def filter_models(models, user):
+    if user is not None:
+        models = filter(lambda model: not hasattr(model, 'reversion_perm') or user.has_perm(model.reversion_perm), models)
+
+    return models
+
+
 class ActivityTable(generic.ListView):
     model = RIGSVersion
     template_name = "activity_table.html"
     paginate_by = 25
 
     def get_queryset(self):
-        versions = RIGSVersion.objects.get_for_multiple_models(self.kwargs['models'])
+        versions = RIGSVersion.objects.get_for_multiple_models(filter_models(self.kwargs['models'], self.request.user))
         return versions.order_by("-revision__date_created")
 
     def get_context_data(self, **kwargs):
@@ -69,17 +77,15 @@ class ActivityTable(generic.ListView):
 
         return context
 
-# Appears on homepage
-
 
 @method_decorator(never_cache, name='dispatch')  # Disable browser based caching
-class ActivityFeed(generic.ListView):
+class ActivityFeed(generic.ListView):  # Appears on homepage
     model = RIGSVersion
     template_name = "activity_feed_data.html"
     paginate_by = 25
 
     def get_queryset(self):
-        versions = RIGSVersion.objects.get_for_multiple_models(get_models())
+        versions = RIGSVersion.objects.get_for_multiple_models(filter_models(get_models(), self.request.user))
         return versions.order_by("-revision__date_created")
 
     def get_context_data(self, **kwargs):