From 01a87e0e0be8cf0a139d7f2d6f321f4e10f5851f Mon Sep 17 00:00:00 2001 From: Arona Jones Date: Tue, 31 Dec 2019 12:25:42 +0000 Subject: [PATCH] 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 Closes #358 --- RIGS/templates/RIGS/event_detail.html | 2 +- RIGS/templates/RIGS/index.html | 3 +- RIGS/versioning.py | 11 +-- assets/models.py | 32 +++++-- assets/templates/asset_activity_table.html | 92 +++++++++++++++++++++ assets/templates/asset_update.html | 10 +++ assets/templates/asset_version_history.html | 68 +++++++++++++++ assets/templates/supplier_list.html | 1 + assets/urls.py | 25 ++++-- assets/views.py | 29 ++++++- templates/base_assets.html | 7 +- 11 files changed, 251 insertions(+), 29 deletions(-) create mode 100644 assets/templates/asset_activity_table.html create mode 100644 assets/templates/asset_version_history.html diff --git a/RIGS/templates/RIGS/event_detail.html b/RIGS/templates/RIGS/event_detail.html index 176756dc..8668bbac 100644 --- a/RIGS/templates/RIGS/event_detail.html +++ b/RIGS/templates/RIGS/event_detail.html @@ -245,7 +245,7 @@
- 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' }}
diff --git a/RIGS/templates/RIGS/index.html b/RIGS/templates/RIGS/index.html index 826839e6..acd1a707 100644 --- a/RIGS/templates/RIGS/index.html +++ b/RIGS/templates/RIGS/index.html @@ -72,9 +72,8 @@
{% if perms.RIGS.view_event %} -
+
{% include 'RIGS/activity_feed.html' %} -
{% endif %}
diff --git a/RIGS/versioning.py b/RIGS/versioning.py index d7b423c2..795ea9fe 100644 --- a/RIGS/versioning.py +++ b/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() - - return versions + def get_object(self, **kwargs): + return get_object_or_404(self.kwargs['model'], pk=self.kwargs['pk']) 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'] = thisObject + context['object'] = self.get_object() return context diff --git a/assets/models.py b/assets/models.py index 993c8c34..2398a563 100644 --- a/assets/models.py +++ b/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) - socket = models.ForeignKey(Connector, on_delete=models.SET_NULL, 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') + plug = models.ForeignKey(Connector, on_delete=models.SET_NULL, + related_name='plug', blank=True, null=True) + socket = models.ForeignKey(Connector, on_delete=models.SET_NULL, + 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"] diff --git a/assets/templates/asset_activity_table.html b/assets/templates/asset_activity_table.html new file mode 100644 index 00000000..aca6142c --- /dev/null +++ b/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 %} + + + + +{% endblock %} + +{% block content %} +
+
+
+

Asset Activity Stream

+
+
{% paginator %}
+
+
+ + + + + + + + + + + + + + {% for version in object_list %} + + + + + + + + + + + {% endfor %} + + +
DateObjectVersion IDUserChangesComment
{{ version.revision.date_created }}{{version.changes.new|to_class_name}} {{ version.changes.new.asset_id|default:version.changes.new.pk }}{{ version.pk }}|{{ version.revision.pk }}{{ version.revision.user.name }} + {% if version.changes.old == None %} + {{version.changes.new|to_class_name}} Created + {% else %} + {% include 'RIGS/version_changes.html' %} + {% endif %} {{ version.changes.revision.comment }}
+ +
+
{% paginator %}
+
+{% endblock %} diff --git a/assets/templates/asset_update.html b/assets/templates/asset_update.html index faee7662..9987e4fe 100644 --- a/assets/templates/asset_update.html +++ b/assets/templates/asset_update.html @@ -45,6 +45,16 @@ +{% if not edit %} +
+
+ + Last edited at {{ object.last_edited_at|default:'never' }} by {{ object.last_edited_by.name|default:'nobody' }} + +
+
+{% endif %} + {% endblock %} {% block js%} diff --git a/assets/templates/asset_version_history.html b/assets/templates/asset_version_history.html new file mode 100644 index 00000000..89223294 --- /dev/null +++ b/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 %} + + + +{% endblock %} + +{% block content %} +
+
+
+

{{object|to_class_name}} {{ object.asset_id|default:object.pk }} - Revision History

+
+
{% paginator %}
+
+
+ + + + + + + + + + + + {% for version in object_list %} + + + + + + + + + + + {% endfor %} + + +
DateVersion IDUserChangesComment
{{ version.revision.date_created }}{{ version.pk }}|{{ version.revision.pk }}{{ version.revision.user.name }} + {% if version.changes.old is None %} + {{object|to_class_name}} Created + {% else %} + {% include 'RIGS/version_changes.html' %} + {% endif %} + + {{ version.revision.comment }} +
+
+
{% paginator %}
+
+{% endblock %} diff --git a/assets/templates/supplier_list.html b/assets/templates/supplier_list.html index 71ceb78c..4de91bbc 100644 --- a/assets/templates/supplier_list.html +++ b/assets/templates/supplier_list.html @@ -31,6 +31,7 @@ {{ item.name }} Edit + History {% endfor %} diff --git a/assets/urls.py b/assets/urls.py index 0a8ff599..3acac945 100644 --- a/assets/urls.py +++ b/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//', 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/id//edit/', permission_required_with_403('assets.change_asset')(views.AssetEdit.as_view()), name='asset_update'), - path('asset/id//duplicate/', permission_required_with_403('assets.add_asset')(views.AssetDuplicate.as_view()), name='asset_duplicate'), + path('asset/create/', permission_required_with_403('assets.add_asset') + (views.AssetCreate.as_view()), name='asset_create'), + path('asset/id//edit/', permission_required_with_403('assets.change_asset') + (views.AssetEdit.as_view()), name='asset_update'), + path('asset/id//duplicate/', permission_required_with_403('assets.add_asset') + (views.AssetDuplicate.as_view()), name='asset_duplicate'), + path('asset/id//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/', 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//edit', permission_required_with_403('assets.change_supplier')(views.SupplierUpdate.as_view()), name='supplier_update'), + path('supplier/create', permission_required_with_403('assets.add_supplier') + (views.SupplierCreate.as_view()), name='supplier_create'), + path('supplier//edit', permission_required_with_403('assets.change_supplier') + (views.SupplierUpdate.as_view()), name='supplier_update'), + path('supplier//history/', views.SupplierVersionHistory.as_view(), + name='supplier_history', kwargs={'model': models.Supplier}), path('supplier/search/', views.SupplierSearch.as_view(), name='supplier_search_json'), ] diff --git a/assets/views.py b/assets/views.py index 06449cdb..1acc28fb 100644 --- a/assets/views.py +++ b/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 diff --git a/templates/base_assets.html b/templates/base_assets.html index 7dd53034..288d7dc9 100644 --- a/templates/base_assets.html +++ b/templates/base_assets.html @@ -5,7 +5,7 @@ {% endblock %} {% block titleelements %} - {% if perms.assets.view_asset%} + {% if perms.assets.view_asset %}
  • Assets
      @@ -16,7 +16,7 @@
    • {% endif %} - {% if perms.assets.view_supplier%} + {% if perms.assets.view_supplier %}
  • Suppliers
      @@ -28,4 +28,7 @@
    • {% endif %} + {% if perms.assets.view_asset %} +
  • Recent Changes
    • + {% endif %} {% endblock %}