FEAT: Add revision history to assets and suppliers (#387)

* FEAT: Initial work on revision history for assets

The revision history for individual items mostly works, though it shows database ID where it should show asset ID. Recent changes feed isn't yet done.

* FEAT: Initial implementation of asset activity stream

* CHORE: Fix pep8

* FIX: Asset history table 'branding'

* FIX: Individual asset version history is now correctly filtered

* FEAT: Make revision history for suppliers accessible

* CHORE: *sings* And a pep8 in a broken tree...

* Refactored out duplicated code from `AssetVersionHistory

* CHORE: pep8

And another random bit of wierd whitespace I found

Co-authored-by: Matthew Smith <mattysmith22@googlemail.com>

Closes #358

RIGS/templates/RIGS/event_detail.html

@@ -245,7 +245,7 @@
         <div class="row">
             <div class="col-sm-10 align-left">
                 <a href="{% url 'event_history' object.pk %}" title="View Revision History">
-                    Last edited at {{ object.last_edited_at }} by {{ object.last_edited_by.name }}
+                    Last edited at {{ object.last_edited_at|default:'never' }} by {{ object.last_edited_by.name|default:'nobody' }}
                 </a>
             </div>
             <div class="col-sm-2">

RIGS/templates/RIGS/index.html

@@ -72,9 +72,8 @@
         </div>
     </div>
     {% if perms.RIGS.view_event %}
-    <div class="col-sm-6" >
+    <div class="col-sm-6">
         {% include 'RIGS/activity_feed.html' %}    
-        
     </div>
     {% endif %}
 </div>

RIGS/versioning.py

@@ -206,17 +206,14 @@ class VersionHistory(generic.ListView):
     paginate_by = 25
 
     def get_queryset(self, **kwargs):
-        thisModel = self.kwargs['model']
+        return RIGSVersion.objects.get_for_object(self.get_object()).select_related("revision", "revision__user").all()
 
-        versions = RIGSVersion.objects.get_for_object_reference(thisModel, self.kwargs['pk']).select_related("revision", "revision__user").all()
+    def get_object(self, **kwargs):
-
+        return get_object_or_404(self.kwargs['model'], pk=self.kwargs['pk'])
-        return versions
 
     def get_context_data(self, **kwargs):
-        thisModel = self.kwargs['model']
         context = super(VersionHistory, self).get_context_data(**kwargs)
-        thisObject = get_object_or_404(thisModel, pk=self.kwargs['pk'])
+        context['object'] = self.get_object()
-        context['object'] = thisObject
 
         return context
 

assets/models.py

@@ -6,6 +6,11 @@ from django.urls import reverse
 from django.db.models.signals import pre_save
 from django.dispatch.dispatcher import receiver
 
+from reversion import revisions as reversion
+from reversion.models import Version
+
+from RIGS.models import RevisionMixin
+
 
 class AssetCategory(models.Model):
     class Meta:
@@ -24,13 +29,15 @@ class AssetStatus(models.Model):
         verbose_name_plural = 'Asset Statuses'
 
     name = models.CharField(max_length=80)
-    should_show = models.BooleanField(default=True, help_text="Should this be shown by default in the asset list.")
+    should_show = models.BooleanField(
+        default=True, help_text="Should this be shown by default in the asset list.")
 
     def __str__(self):
         return self.name
 
 
-class Supplier(models.Model):
+@reversion.register
+class Supplier(models.Model, RevisionMixin):
     name = models.CharField(max_length=80)
 
     class Meta:
@@ -55,7 +62,8 @@ class Connector(models.Model):
         return self.description
 
 
-class Asset(models.Model):
+@reversion.register
+class Asset(models.Model, RevisionMixin):
     class Meta:
         ordering = ['asset_id_prefix', 'asset_id_number']
         permissions = (
@@ -63,7 +71,8 @@ class Asset(models.Model):
             ('view_asset', 'Can view an asset')
         )
 
-    parent = models.ForeignKey(to='self', related_name='asset_parent', blank=True, null=True, on_delete=models.SET_NULL)
+    parent = models.ForeignKey(to='self', related_name='asset_parent',
+                               blank=True, null=True, on_delete=models.SET_NULL)
     asset_id = models.CharField(max_length=15, unique=True)
     description = models.CharField(max_length=120)
     category = models.ForeignKey(to=AssetCategory, on_delete=models.CASCADE)
@@ -79,10 +88,14 @@ class Asset(models.Model):
 
     # Cable assets
     is_cable = models.BooleanField(default=False)
-    plug = models.ForeignKey(Connector, on_delete=models.SET_NULL, related_name='plug', blank=True, null=True)
+    plug = models.ForeignKey(Connector, on_delete=models.SET_NULL,
-    socket = models.ForeignKey(Connector, on_delete=models.SET_NULL, related_name='socket', blank=True, null=True)
+                             related_name='plug', blank=True, null=True)
-    length = models.DecimalField(decimal_places=1, max_digits=10, blank=True, null=True, help_text='m')
+    socket = models.ForeignKey(Connector, on_delete=models.SET_NULL,
-    csa = models.DecimalField(decimal_places=2, max_digits=10, blank=True, null=True, help_text='mm^2')
+                               related_name='socket', blank=True, null=True)
+    length = models.DecimalField(decimal_places=1, max_digits=10,
+                                 blank=True, null=True, help_text='m')
+    csa = models.DecimalField(decimal_places=2, max_digits=10,
+                              blank=True, null=True, help_text='mm^2')
     circuits = models.IntegerField(blank=True, null=True)
     cores = models.IntegerField(blank=True, null=True)
 
@@ -125,7 +138,8 @@ class Asset(models.Model):
         self.asset_id = self.asset_id.upper()
         asset_search = re.search("^([a-zA-Z0-9]*?[a-zA-Z]?)([0-9]+)$", self.asset_id)
         if asset_search is None:
-                errdict["asset_id"] = ["An Asset ID can only consist of letters and numbers, with a final number"]
+            errdict["asset_id"] = [
+                "An Asset ID can only consist of letters and numbers, with a final number"]
 
         if self.purchase_price and self.purchase_price < 0:
             errdict["purchase_price"] = ["A price cannot be negative"]

assets/templates/asset_activity_table.html

@@ -0,0 +1,92 @@
+{% extends request.is_ajax|yesno:"base_ajax.html,base_assets.html" %}
+{% load static %}
+{% load paginator from filters %}
+{% load to_class_name from filters %}
+
+{% block title %}Asset Activity Stream{% endblock %}
+
+{# TODO: Find a way to reduce code duplication...can't just include the content because of the IDs... #}
+
+{% block js %}
+    <script src="{% static "js/tooltip.js" %}"></script>
+    <script src="{% static "js/popover.js" %}"></script>
+    <script src="{% static "js/moment.min.js" %}"></script>
+    <script>
+    	$(function () {
+		  $('[data-toggle="popover"]').popover().click(function(){
+        if($(this).attr('href')){
+          window.location.href = $(this).attr('href');
+        }
+      });
+
+          // This keeps timeago values correct, but uses an insane amount of resources
+          // $(function () {
+          //     setInterval(function() {
+          //         $('.date').each(function (index, dateElem) {
+          //           var $dateElem = $(dateElem);
+          //           var formatted = moment($dateElem.attr('data-date')).fromNow();
+          //           $dateElem.text(formatted);
+          //          })
+          //        });
+          //     }, 10000);
+
+
+            $('.date').each(function (index, dateElem) {
+                var $dateElem = $(dateElem);
+                var formatted = moment($dateElem.attr('data-date')).fromNow();
+                $dateElem.text(formatted);
+               });
+
+
+		})
+
+    </script>
+{% endblock %}
+
+{% block content %}
+<div class="col-sm-12">
+        <div class="row">
+            <div class="col-sm-12">
+                <h3>Asset Activity Stream</h3>
+            </div>
+            <div class="text-right col-sm-12">{% paginator %}</div>
+        </div>
+        <div class="table-responsive">
+
+        	<table class="table table-striped">
+                <thead>
+                <tr>
+                    <td>Date</td>
+                    <td>Object</td>
+                    <td>Version ID</td>
+                    <td>User</td>
+                    <td>Changes</td>
+                    <td>Comment</td>
+                </tr>
+                </thead>
+                <tbody>
+                	{% for version in object_list %}
+
+                            <tr>
+                                <td>{{ version.revision.date_created }}</td>
+                                <td><a href="{{ version.changes.new.get_absolute_url }}">{{version.changes.new|to_class_name}} {{ version.changes.new.asset_id|default:version.changes.new.pk }}</a></td>
+                           		<td>{{ version.pk }}|{{ version.revision.pk }}</td>
+                                <td>{{ version.revision.user.name }}</td>
+                                <td>
+                                  {% if version.changes.old == None %}
+                                    {{version.changes.new|to_class_name}} Created
+                                  {% else %}
+                                    {% include 'RIGS/version_changes.html' %}
+                                  {% endif %}                        </td>
+                                  <td>{{ version.changes.revision.comment }}</td>
+                            </tr>
+
+        			{% endfor %}
+
+                </tbody>
+            </table>
+
+        </div>
+        <div class="align-right">{% paginator %}</div>
+    </div>
+{% endblock %}

assets/templates/asset_update.html

@@ -45,6 +45,16 @@
   </div>
 </form>
 
+{% if not edit %}
+<div class="col-sm-12 text-right">
+    <div>
+        <a href="{% url 'asset_history' object.asset_id %}" title="View Revision History">
+            Last edited at {{ object.last_edited_at|default:'never' }} by {{ object.last_edited_by.name|default:'nobody' }}
+        </a>
+    </div>
+</div>
+{% endif %}
+
 {% endblock %}
 
 {% block js%}

assets/templates/asset_version_history.html

@@ -0,0 +1,68 @@
+{% extends request.is_ajax|yesno:"base_ajax.html,base_assets.html" %}
+{% load to_class_name from filters %}
+{% load paginator from filters %}
+{% load static %}
+
+{% block title %}{{object|to_class_name}} {{ object.asset_id }} - Revision History{% endblock %}
+
+{% block js %}
+    <script src="{% static "js/tooltip.js" %}"></script>
+    <script src="{% static "js/popover.js" %}"></script>
+    <script>
+    	$(function () {
+            $('[data-toggle="popover"]').popover().click(function(){
+                    if($(this).attr('href')){
+                      window.location.href = $(this).attr('href');
+                    }
+                  });
+        })
+    </script>
+{% endblock %}
+
+{% block content %}
+<div class="col-sm-12">
+        <div class="row">
+            <div class="col-sm-12">
+                <h3><a href="{{ object.get_absolute_url }}">{{object|to_class_name}} {{ object.asset_id|default:object.pk }}</a> - Revision History</h3>
+            </div>
+            <div class="text-right col-sm-12">{% paginator %}</div>
+        </div>
+        <div class="table-responsive">
+        	<table class="table table-striped">
+                <thead>
+                <tr>
+                    <td>Date</td>
+                    <td>Version ID</td>
+                    <td>User</td>
+                    <td>Changes</td>
+                    <td>Comment</td>
+                </tr>
+                </thead>
+                <tbody>
+                	{% for version in object_list %}
+
+                            <tr>
+                           		<td>{{ version.revision.date_created }}</td>
+                                <td>{{ version.pk }}|{{ version.revision.pk }}</td>
+                                <td>{{ version.revision.user.name }}</td>
+                                <td>
+                                    {% if version.changes.old is None %}
+                                        {{object|to_class_name}} Created
+                                    {% else %}
+                                        {% include 'RIGS/version_changes.html' %}
+                                    {% endif %}
+                                </td>
+                                <td>
+                                    {{ version.revision.comment }}
+                                </td>
+                            </tr>
+
+
+        			{% endfor %}
+
+                </tbody>
+            </table>
+         </div>
+    <div class="align-right">{% paginator %}</div>
+</div>
+{% endblock %}

assets/templates/supplier_list.html

@@ -31,6 +31,7 @@
       <td>{{ item.name }}</td>
       <td>
         <a href="{% url 'supplier_update' item.pk %}" class="btn btn-default"><i class="glyphicon glyphicon-edit"></i> Edit</a>
+        <a href="{% url 'supplier_history' item.pk %}" class="btn btn-default"><i class="glyphicon glyphicon-time"></i> History</a>
       </td>
     </tr>
     {% endfor %}

assets/urls.py

@@ -1,5 +1,7 @@
+from django.conf.urls import url
 from django.urls import path
-from assets import views
+from assets import views, models
+from RIGS import versioning
 
 from PyRIGS.decorators import permission_required_with_403
 
@@ -7,16 +9,27 @@ urlpatterns = [
     path('', views.AssetList.as_view(), name='asset_index'),
     path('asset/list/', views.AssetList.as_view(), name='asset_list'),
     path('asset/id/<str:pk>/', views.AssetDetail.as_view(), name='asset_detail'),
-    path('asset/create/', permission_required_with_403('assets.add_asset')(views.AssetCreate.as_view()), name='asset_create'),
+    path('asset/create/', permission_required_with_403('assets.add_asset')
-    path('asset/id/<str:pk>/edit/', permission_required_with_403('assets.change_asset')(views.AssetEdit.as_view()), name='asset_update'),
+         (views.AssetCreate.as_view()), name='asset_create'),
-    path('asset/id/<str:pk>/duplicate/', permission_required_with_403('assets.add_asset')(views.AssetDuplicate.as_view()), name='asset_duplicate'),
+    path('asset/id/<str:pk>/edit/', permission_required_with_403('assets.change_asset')
+         (views.AssetEdit.as_view()), name='asset_update'),
+    path('asset/id/<str:pk>/duplicate/', permission_required_with_403('assets.add_asset')
+         (views.AssetDuplicate.as_view()), name='asset_duplicate'),
+    path('asset/id/<str:pk>/history/', views.AssetVersionHistory.as_view(),
+         name='asset_history', kwargs={'model': models.Asset}),
+    path('activity', permission_required_with_403('assets.view_asset')
+         (views.ActivityTable.as_view()), name='asset_activity_table'),
 
     path('asset/search/', views.AssetSearch.as_view(), name='asset_search_json'),
 
     path('supplier/list', views.SupplierList.as_view(), name='supplier_list'),
     path('supplier/<int:pk>', views.SupplierDetail.as_view(), name='supplier_detail'),
-    path('supplier/create', permission_required_with_403('assets.add_supplier')(views.SupplierCreate.as_view()), name='supplier_create'),
+    path('supplier/create', permission_required_with_403('assets.add_supplier')
-    path('supplier/<int:pk>/edit', permission_required_with_403('assets.change_supplier')(views.SupplierUpdate.as_view()), name='supplier_update'),
+         (views.SupplierCreate.as_view()), name='supplier_create'),
+    path('supplier/<int:pk>/edit', permission_required_with_403('assets.change_supplier')
+         (views.SupplierUpdate.as_view()), name='supplier_update'),
+    path('supplier/<str:pk>/history/', views.SupplierVersionHistory.as_view(),
+         name='supplier_history', kwargs={'model': models.Supplier}),
 
     path('supplier/search/', views.SupplierSearch.as_view(), name='supplier_search_json'),
 ]

assets/views.py

@@ -5,7 +5,9 @@ from django.views.decorators.csrf import csrf_exempt
 from django.utils.decorators import method_decorator
 from django.urls import reverse
 from django.db.models import Q
+from django.shortcuts import get_object_or_404
 from assets import models, forms
+from RIGS import versioning
 
 
 @method_decorator(csrf_exempt, name='dispatch')
@@ -36,7 +38,8 @@ class AssetList(LoginRequiredMixin, generic.ListView):
         if len(query_string) == 0:
             queryset = self.model.objects.all()
         elif len(query_string) >= 3:
-            queryset = self.model.objects.filter(Q(asset_id__exact=query_string) | Q(description__icontains=query_string))
+            queryset = self.model.objects.filter(
+                Q(asset_id__exact=query_string) | Q(description__icontains=query_string))
         else:
             queryset = self.model.objects.filter(Q(asset_id__exact=query_string))
 
@@ -46,7 +49,8 @@ class AssetList(LoginRequiredMixin, generic.ListView):
         if len(form.cleaned_data['status']) > 0:
             queryset = queryset.filter(status__in=form.cleaned_data['status'])
         elif self.hide_hidden_status:
-            queryset = queryset.filter(status__in=models.AssetStatus.objects.filter(should_show=True))
+            queryset = queryset.filter(
+                status__in=models.AssetStatus.objects.filter(should_show=True))
 
         return queryset
 
@@ -203,3 +207,24 @@ class SupplierUpdate(generic.UpdateView):
     model = models.Supplier
     form_class = forms.SupplierForm
     template_name = 'supplier_update.html'
+
+
+class SupplierVersionHistory(versioning.VersionHistory):
+    template_name = "asset_version_history.html"
+
+
+# TODO: Reduce SQL queries
+class AssetVersionHistory(versioning.VersionHistory):
+    def get_object(self, **kwargs):
+        return get_object_or_404(models.Asset, asset_id=self.kwargs['pk'])
+
+
+class ActivityTable(versioning.ActivityTable):
+    model = versioning.RIGSVersion
+    template_name = "asset_activity_table.html"
+    paginate_by = 25
+
+    def get_queryset(self):
+        versions = versioning.RIGSVersion.objects.get_for_multiple_models(
+            [models.Asset, models.Supplier])
+        return versions

templates/base_assets.html

@@ -5,7 +5,7 @@
 {% endblock %}
 
 {% block titleelements %}
-    {% if perms.assets.view_asset%}
+    {% if perms.assets.view_asset %}
         <li class="dropdown">
             <a href="#" class="dropdown-toggle" data-toggle="dropdown">Assets<b class="caret"></b></a>
             <ul class="dropdown-menu">
@@ -16,7 +16,7 @@
             </ul>
         </li>
     {% endif %}
-    {% if perms.assets.view_supplier%}
+    {% if perms.assets.view_supplier %}
         <li class="dropdown">
             <a href="#" class="dropdown-toggle" data-toggle="dropdown"> Suppliers<b class="caret"></b></a>
             <ul class="dropdown-menu">
@@ -28,4 +28,7 @@
             </ul>
         </li>
     {% endif %}
+    {% if perms.assets.view_asset %}
+        <li><a href="{% url 'asset_activity_table' %}">Recent Changes</a></li>
+    {% endif %}
 {% endblock %}