From 3593ff8f27b0554a771eadef8adb8ac57bf38505 Mon Sep 17 00:00:00 2001
From: Alec Lofquist <alec.lofquist@affirm.com>
Date: Thu, 20 Sep 2018 09:24:36 -0700
Subject: [PATCH 1/2] Track revisions to services, status checks, and
schedules.
Show last 3 changes on the service and status check pages.
---
cabot/cabotapp/admin.py | 14 +-
cabot/cabotapp/models.py | 7 +
cabot/cabotapp/revision_utils.py | 205 ++++++++++++++++++
cabot/cabotapp/tests/test_revisions.py | 193 +++++++++++++++++
cabot/cabotapp/views.py | 35 ++-
cabot/metricsapp/views/grafana_elastic.py | 10 +-
cabot/settings.py | 1 +
cabot/templates/cabotapp/model_diff.html | 29 +++
cabot/templates/cabotapp/service_detail.html | 5 +
.../cabotapp/statuscheck_detail.html | 9 +-
requirements.txt | 1 +
11 files changed, 493 insertions(+), 16 deletions(-)
create mode 100644 cabot/cabotapp/revision_utils.py
create mode 100644 cabot/cabotapp/tests/test_revisions.py
create mode 100644 cabot/templates/cabotapp/model_diff.html
diff --git a/cabot/cabotapp/admin.py b/cabot/cabotapp/admin.py
index 85b7300f1..878a90c46 100644
--- a/cabot/cabotapp/admin.py
+++ b/cabot/cabotapp/admin.py
@@ -1,3 +1,4 @@
+import reversion
from django.contrib import admin
from .models import (
UserProfile,
@@ -13,11 +14,20 @@
)
from .alert import AlertPluginUserData, AlertPlugin
+
+class ServiceAdmin(reversion.VersionAdmin):
+ pass
+
+
+class StatusCheckAdmin(reversion.VersionAdmin):
+ pass
+
+
admin.site.register(UserProfile)
admin.site.register(Shift)
-admin.site.register(Service)
+admin.site.register(Service, ServiceAdmin)
admin.site.register(ServiceStatusSnapshot)
-admin.site.register(StatusCheck)
+admin.site.register(StatusCheck, StatusCheckAdmin)
admin.site.register(StatusCheckResult)
admin.site.register(ActivityCounter)
admin.site.register(AlertPlugin)
diff --git a/cabot/cabotapp/models.py b/cabot/cabotapp/models.py
index 46e5db97f..642e00175 100644
--- a/cabot/cabotapp/models.py
+++ b/cabot/cabotapp/models.py
@@ -26,6 +26,7 @@
import socket
import time
import yaml
+import reversion
import requests
from celery.utils.log import get_task_logger
@@ -76,6 +77,7 @@ def get_success_with_retries(recent_results, retries=0):
return False
+@reversion.register
class CheckGroupMixin(models.Model):
class Meta:
@@ -351,6 +353,7 @@ def is_silenced(self, now=None):
return self.silence_warnings_until is not None and self.silence_warnings_until > now
+@reversion.register
class Service(CheckGroupMixin):
def update_status(self):
@@ -401,6 +404,7 @@ def __unicode__(self):
return u"%s: %s" % (self.service.name, self.overall_status)
+@reversion.register(exclude=['last_run', 'cached_health', 'calculated_status'])
class StatusCheck(PolymorphicModel):
"""
Base class for polymorphic models. We're going to use
@@ -665,6 +669,7 @@ def reset_and_save(self):
self.save()
+@reversion.register(follow=['statuscheck_ptr'])
class HttpStatusCheck(StatusCheck):
@property
@@ -814,6 +819,7 @@ def _run(self):
return result
+@reversion.register(follow=['statuscheck_ptr'])
class JenkinsStatusCheck(StatusCheck):
@property
@@ -903,6 +909,7 @@ def _run(self):
return result
+@reversion.register(follow=['statuscheck_ptr'])
class TCPStatusCheck(StatusCheck):
@property
diff --git a/cabot/cabotapp/revision_utils.py b/cabot/cabotapp/revision_utils.py
new file mode 100644
index 000000000..abe13964c
--- /dev/null
+++ b/cabot/cabotapp/revision_utils.py
@@ -0,0 +1,205 @@
+from collections import defaultdict
+
+import reversion
+from django.contrib.auth.models import AnonymousUser
+from django.db import transaction
+from django.db.models import Manager, Model
+from django.db.models.query import QuerySet
+from django.utils.html import escape
+
+
+def add_to_revision(obj):
+ """Manually add the current version of obj to the current revision."""
+ # backport of reversion.add_to_revision() from django-reversion 2.x
+ manager = reversion.default_revision_manager
+ ctx_manager = reversion.revision_context_manager
+ adapter = manager.get_adapter(type(obj))
+ ctx_manager.add_to_context(manager, obj, adapter.get_version_data(obj, ctx_manager.get_db()))
+
+
+def get_revisions(obj, n_revisions=3):
+ """
+ Get the last N diffs for an object that is tracked by django-reversion, as (v1, v2) human-readable string tuples.
+ Foreign key fields will show their *current* string representation.
+ Deleted foreign key fields will show as 'deleted' or 'deleted: <last known object repr>'.
+ Diff strings may be HTML formatted. Any user-supplied values are first HTML-escaped.
+ :param obj: object to get a diff for
+ :param n_revisions: maximum number of revisions to get (default 3)
+ :return: list of (revision, {field: (v1_str, v2_str), ...}) tuples
+ """
+ # type: (Any, int) -> List[Tuple[reversion.models.Revision, Dict[str, Tuple[str, str]]]]
+ recent_versions = reversion.get_for_object(obj)[:n_revisions+1]
+
+ # grab fields on the object's model
+ fields = [f for f in obj._meta.fields]
+
+ # also grab many-to-many fields
+ concrete_model = obj._meta.concrete_model
+ fields += concrete_model._meta.many_to_many
+
+ revisions = []
+ for i in range(len(recent_versions) - 1):
+ cur = recent_versions[i]
+ prev = recent_versions[i+1]
+ changes = compare_versions(fields, prev, cur)
+ if len(changes) > 0:
+ revisions.append((cur.revision, changes))
+
+ return revisions
+
+
+def compare_versions(fields, v1, v2):
+ """
+ Return HTML-formatted diffs for a list of fields.
+ :param fields: list of Django field objects
+ :param v1: reversion.Version for the previous version of the object
+ :param v2: reversion.Version for the new version of the object
+ :return: dict of {'field_name': (v1_value_str, v2_value_str), ...}
+ """
+ # type: (List[Field], reversion.models.Version, reversion.models.Version) -> Dict[str, Tuple[str, str]]
+
+ class MutedStr:
+ def __init__(self, s):
+ self.msg = s
+
+ def __str__(self):
+ return '<span class="text-muted">{}</span>'.format(escape(self.msg))
+
+ def obj_to_str_escaped(obj):
+ # type: (Any) -> str
+ """
+ Converts an object to an HTML-escaped string (but note the return value may contain HTML for styling special
+ values like None).
+ """
+
+ if obj is None or obj == '':
+ return str(MutedStr('none'))
+ elif isinstance(obj, list):
+ return '[' + ', '.join([obj_to_str_escaped(o) for o in obj]) + ']'
+ elif isinstance(obj, MutedStr):
+ return str(obj) # already escaped
+
+ try:
+ return escape(str(obj))
+ except:
+ try:
+ return escape(repr(obj))
+ except:
+ return str(MutedStr('<error: could not build string>'))
+
+ def field_to_str_escaped(field, version):
+ """
+ Converts data from django-reversion to a nice string, based on the given django fields. Most importantly,
+ handles looking up foreign keys and many-to-many relationships if they still exist in the DB.
+ :param field: list of django field objects
+ :param version: django-reversion Version object
+ :return: human-readable string representing field of version; may contain HTML
+ """
+ value = version.field_dict[field.name]
+ if field.get_internal_type() == 'ManyToManyField':
+ ids = [int(v) for v in value]
+ related_model = field.rel.to
+
+ related_objs = []
+ for related in related_model.objects.filter(id__in=ids):
+ related_objs.append(related)
+ ids.remove(related.pk)
+
+ # if it's not in the live set, try and find it in a "deleted" revision
+ for version in reversion.get_deleted(related_model).filter(object_id_int__in=ids):
+ # we use the object repr here because version.object is none for some reason
+ related_objs.append(MutedStr('deleted: {}'.format(version.object_repr)))
+ ids.remove(version.object_id_int)
+
+ # alternatively pull related objects from same revision
+ # value = version.revision.version_set.filter(
+ # content_type=ContentType.objects.get_for_model(related_model),
+ # object_id_int__in=ids
+ # )
+
+ # for anything we couldn't find, just give a fixed 'deleted' string
+ value = related_objs + [MutedStr('deleted') for _ in ids]
+ elif field.get_internal_type() == 'ForeignKey':
+ try:
+ value = value.rel.get()
+ except:
+ value = MutedStr('deleted')
+
+ return obj_to_str_escaped(value)
+
+ changes = {}
+ for field in fields:
+ v1_value = v1.field_dict[field.name]
+ v2_value = v2.field_dict[field.name]
+ if v1_value != v2_value:
+ v1_str = field_to_str_escaped(field, v1)
+ v2_str = field_to_str_escaped(field, v2)
+ changes[field.name] = (v1_str, v2_str)
+
+ return changes
+
+
+class RevisionMixin(object):
+ """
+ Backport of reversion.views.RevisionMixin since we are using a version of reversion from 2015.
+ Wraps HTTP requests (except for GET/HEAD/OPTIONS) in an atomic django-reversion revision, plus
+ some magic to follow reverse m2m fields in checks (which django-reversion should handle, but I
+ can't get it to work :shakes fist:).
+
+ Set 'revision_comment' if you would like this revision to be recorded as a fixed string instead
+ of a list of changed fields. Currently used for 'created' and 'deleted' messages.
+
+ Set 'revision_follow' to a list of attributes on self.object that return querysets or foreign keys
+ that we should include with any revisions (for example, 'service_set' for status checks).
+ """
+
+ revision_comment = ''
+ revision_follow = []
+
+ def dispatch(self, *args, **kwargs):
+ if self.request.method not in ('GET', 'HEAD', 'OPTIONS'):
+ with transaction.atomic(), reversion.create_revision():
+ # fill in the revision metadata
+ if not isinstance(self.request.user, AnonymousUser):
+ reversion.set_user(self.request.user)
+ if self.revision_comment:
+ reversion.set_comment(self.revision_comment)
+
+ related = self._gather_reverse_pks()
+ ret = super(RevisionMixin, self).dispatch(*args, **kwargs)
+ related = self._gather_reverse_pks(related)
+
+ for model, pks in related.items():
+ for obj in model.objects.filter(pk__in=pks):
+ add_to_revision(obj)
+
+ return ret
+ else:
+ return super(RevisionMixin, self).dispatch(*args, **kwargs)
+
+ def _gather_reverse_pks(self, model_to_pks=None):
+ model_to_pks = model_to_pks or defaultdict(list)
+
+ if not self.revision_follow:
+ return model_to_pks
+
+ obj = getattr(self, 'object', None)
+ if obj is None and 'pk' in getattr(self, 'kwargs', []) and hasattr(self, 'get_object'):
+ obj = self.get_object()
+ if obj is None or obj.pk is None:
+ return model_to_pks
+
+ for field_name in self.revision_follow:
+ value = getattr(obj, field_name, None)
+ if isinstance(value, (Manager, QuerySet)):
+ # m2m/o2m relationship
+ field_model = value.model
+ model_to_pks[field_model].extend(value.values_list('pk', flat=True))
+ elif isinstance(value, Model):
+ # foreign key
+ field_model = value._meta.model
+ model_to_pks[field_model].append(value.pk)
+ else:
+ raise RuntimeError('Could not follow field {} (missing? unsupported type?)'.format(field_name))
+
+ return model_to_pks
diff --git a/cabot/cabotapp/tests/test_revisions.py b/cabot/cabotapp/tests/test_revisions.py
new file mode 100644
index 000000000..7487271c8
--- /dev/null
+++ b/cabot/cabotapp/tests/test_revisions.py
@@ -0,0 +1,193 @@
+from urlparse import urlsplit
+
+from django.core.urlresolvers import reverse, resolve
+
+from cabot.cabotapp.models import Service, StatusCheck, HttpStatusCheck, TCPStatusCheck, JenkinsStatusCheck
+from cabot.cabotapp.tests.utils import LocalTestCase
+from cabot.cabotapp.revision_utils import get_revisions
+from cabot.cabotapp.views import ServiceForm, HttpStatusCheckForm, JenkinsStatusCheckForm, TCPStatusCheckForm
+
+import reversion
+
+
+def get_post_data(obj, form_class, changed_fields):
+ form = form_class(instance=obj)
+ old_fields = {}
+ for field_name in form.fields:
+ val = form[field_name].value()
+ if val is None:
+ val = ''
+ old_fields[field_name] = val
+ return dict(old_fields.items() + changed_fields.items())
+
+
+def _create_check_url(check_cls):
+ urls = {
+ HttpStatusCheck: reverse('create-http-check'),
+ JenkinsStatusCheck: reverse('create-jenkins-check'),
+ TCPStatusCheck: reverse('create-tcp-check'),
+ }
+ return urls[check_cls]
+
+
+def _edit_check_url(check):
+ if type(check) == HttpStatusCheck:
+ return reverse('update-http-check', kwargs={'pk': check.pk})
+ raise NotImplemented()
+
+
+_check_form_classes = {
+ HttpStatusCheck: HttpStatusCheckForm,
+ JenkinsStatusCheck: JenkinsStatusCheckForm,
+ TCPStatusCheck: TCPStatusCheckForm,
+}
+
+
+class TestRevisions(LocalTestCase):
+ def setUp(self):
+ # make sure we create initial revisions for everything
+ with reversion.create_revision():
+ super(TestRevisions, self).setUp()
+
+ def _update_service(self, service, changed_fields):
+ data = get_post_data(service, ServiceForm, changed_fields)
+ response = self.client.post(reverse('update-service', kwargs={'pk': service.pk}), data=data)
+ self.assertEqual(response.status_code, 302)
+
+ # return refreshed from db obj
+ return Service.objects.get(pk=self.service.pk)
+
+ def _delete_check(self, check):
+ response = self.client.post(reverse('delete-check', kwargs={'pk': check.pk}))
+ self.assertEqual(response.status_code, 302)
+ return None
+
+ def _create_check(self, model, fields):
+ data = get_post_data(None, _check_form_classes[model], fields)
+ response = self.client.post(_create_check_url(model), data=data)
+ self.assertEqual(response.status_code, 302)
+ pk = resolve(urlsplit(response.url).path).kwargs['pk']
+ return model.objects.get(pk=pk)
+
+ def _update_check(self, check, changed_fields):
+ model = type(check)
+ data = get_post_data(check, _check_form_classes[model], changed_fields)
+ response = self.client.post(_edit_check_url(check), data=data)
+ self.assertEqual(response.status_code, 302)
+ return model.objects.get(pk=check.pk)
+
+ def test_update_service(self):
+ self.service = self._update_service(self.service, {'name': 'cool service'})
+ self.assertEqual(self.service.name, 'cool service')
+
+ def test_delete_check(self):
+ self.assertTrue(StatusCheck.objects.filter(pk=self.jenkins_check.pk).exists())
+ self.assertTrue(StatusCheck.objects.filter(pk=self.http_check.pk).exists())
+
+ self._delete_check(self.jenkins_check)
+ self.assertFalse(StatusCheck.objects.filter(pk=self.jenkins_check.pk).exists())
+ self._delete_check(self.http_check)
+ self.assertFalse(StatusCheck.objects.filter(pk=self.http_check.pk).exists())
+
+ def test_service_revision_single_field(self):
+ self.service = self._update_service(self.service, {'name': 'cool service'})
+
+ revisions = get_revisions(self.service, 3) # request more revisions than there actually are
+ self.assertEqual(len(revisions), 1)
+ changes = revisions[0][1]
+ self.assertEqual(changes['name'][0], 'Service') # old name
+ self.assertEqual(changes['name'][1], 'cool service') # new name
+
+ def test_service_revision_multiple_fields(self):
+ self.service = self._update_service(self.service, {
+ 'name': 'cool service',
+ 'users_to_notify': [self.user.pk],
+ 'url': 'http://google.com',
+ })
+
+ revisions = get_revisions(self.service, 1)
+ self.assertEqual(len(revisions), 1)
+ changes = revisions[0][1]
+ self.assertEqual(changes['name'][0], 'Service')
+ self.assertEqual(changes['name'][1], 'cool service')
+ self.assertEqual(changes['users_to_notify'][0], '[]')
+ self.assertEqual(changes['users_to_notify'][1], '[testuser]')
+ self.assertEqual(changes['url'][0], '<span class="text-muted">none</span>')
+ self.assertEqual(changes['url'][1], 'http://google.com')
+
+ def test_service_multiple_revisions_and_fields(self):
+ self.service = self._update_service(self.service, {
+ 'name': 'revision 1',
+ })
+ self.service = self._update_service(self.service, {
+ 'name': 'revision 2',
+ 'users_to_notify': [self.user.pk],
+ })
+ self.service = self._update_service(self.service, {
+ 'name': 'revision 3',
+ 'url': 'http://google.com',
+ })
+ self.service = self._update_service(self.service, {
+ 'name': 'revision 4',
+ })
+
+ revisions = get_revisions(self.service, 3) # request fewer revisions than there actually are
+ self.assertEqual(len(revisions), 3)
+
+ none_str = '<span class="text-muted">none</span>'
+ expected_changes = [
+ {
+ 'name': ('revision 3', 'revision 4'),
+ },
+ {
+ 'name': ('revision 2', 'revision 3'),
+ 'url': (none_str, 'http://google.com')
+ },
+ {
+ 'name': ('revision 1', 'revision 2'),
+ 'users_to_notify': ('[]', '[testuser]')
+ }
+ ]
+
+ for expected, revision in zip(expected_changes, revisions):
+ actual_changes = revision[1]
+ for field_name, (expected_old, expected_new) in expected.items():
+ self.assertEqual(actual_changes[field_name][0], expected_old)
+ self.assertEqual(actual_changes[field_name][1], expected_new)
+
+ def test_service_check_deleted(self):
+ self.assertTrue(self.service.status_checks.filter(pk=self.http_check.pk).exists())
+ self._delete_check(self.http_check)
+ self.assertFalse(self.service.status_checks.filter(pk=self.http_check.pk).exists())
+
+ revisions = get_revisions(self.service, 1)
+ self.assertEqual(len(revisions), 1)
+ self.assertEqual(revisions[0][1]['status_checks'],
+ ('[Jenkins Check, TCP Check, <span class="text-muted">deleted: Http Check</span>]',
+ '[Jenkins Check, TCP Check]'))
+
+ def test_check_added_with_service(self):
+ """Check if a Service revision is saved when a check is created with some service(s) immediately specified"""
+ new_check = self._create_check(HttpStatusCheck, {
+ 'name': 'New Check',
+ 'endpoint': 'http://affirm.com',
+ 'service_set': [self.service.pk]
+ })
+ self.assertTrue(new_check.service_set.filter(pk=self.service.pk).exists())
+
+ revisions = get_revisions(self.service, 1)
+ self.assertEqual(len(revisions), 1)
+ self.assertEqual(revisions[0][1]['status_checks'], ('[Http Check, Jenkins Check, TCP Check]',
+ '[Http Check, Jenkins Check, New Check, TCP Check]'))
+
+ def test_change_service_from_check(self):
+ self.assertTrue(self.service.status_checks.filter(pk=self.http_check.pk).exists())
+ self.http_check = self._update_check(self.http_check, {
+ 'service_set': []
+ })
+ self.assertFalse(self.service.status_checks.filter(pk=self.http_check.pk).exists())
+
+ revisions = get_revisions(self.service, 1)
+ self.assertEqual(len(revisions), 1)
+ self.assertEqual(revisions[0][1]['status_checks'], ('[Http Check, Jenkins Check, TCP Check]',
+ '[Jenkins Check, TCP Check]'))
diff --git a/cabot/cabotapp/views.py b/cabot/cabotapp/views.py
index 40ed6fb87..61bf766bc 100644
--- a/cabot/cabotapp/views.py
+++ b/cabot/cabotapp/views.py
@@ -8,6 +8,7 @@
from timezone_field import TimeZoneFormField
from cabot.cabotapp.alert import AlertPlugin
+from cabot.cabotapp.revision_utils import get_revisions, RevisionMixin
from models import (StatusCheck,
JenkinsStatusCheck,
HttpStatusCheck,
@@ -379,8 +380,10 @@ def get_report(self):
return checks
-class CheckCreateView(LoginRequiredMixin, CreateView):
+class CheckCreateView(LoginRequiredMixin, RevisionMixin, CreateView):
template_name = 'cabotapp/statuscheck_form.html'
+ revision_comment = 'created check'
+ revision_follow = ['service_set']
def form_valid(self, form):
if self.request.user is not None and not isinstance(self.request.user, AnonymousUser):
@@ -412,8 +415,9 @@ def get_success_url(self):
return reverse('check', kwargs={'pk': self.object.id})
-class CheckUpdateView(LoginRequiredMixin, UpdateView):
+class CheckUpdateView(LoginRequiredMixin, RevisionMixin, UpdateView):
template_name = 'cabotapp/statuscheck_form.html'
+ revision_follow = ['service_set']
def get_success_url(self):
return reverse('check', kwargs={'pk': self.object.id})
@@ -465,11 +469,13 @@ def get_queryset(self):
return StatusCheck.objects.all().order_by('name').prefetch_related('service_set')
-class StatusCheckDeleteView(LoginRequiredMixin, DeleteView):
+class StatusCheckDeleteView(LoginRequiredMixin, RevisionMixin, DeleteView):
model = StatusCheck
success_url = reverse_lazy('checks')
context_object_name = 'check'
template_name = 'cabotapp/statuscheck_confirm_delete.html'
+ revision_comment = 'deleted check'
+ revision_follow = ['service_set']
class StatusCheckDetailView(LoginRequiredMixin, DetailView):
@@ -482,6 +488,7 @@ def render_to_response(self, context, *args, **kwargs):
context = {}
context['checkresults'] = self.object.statuscheckresult_set.order_by(
'-time_complete')[:100]
+ context['revisions'] = get_revisions(self.object)
return super(StatusCheckDetailView, self).render_to_response(context, *args, **kwargs)
@@ -636,26 +643,30 @@ def get_context_data(self, **kwargs):
'checks': self.object.status_checks.all(),
'service': self.object,
'date_from': date_from,
- 'date_to': date_from + relativedelta(months=1) - relativedelta(days=1)
+ 'date_to': date_from + relativedelta(months=1) - relativedelta(days=1),
})
+ context['revisions'] = get_revisions(self.object)
return context
-class ServiceCreateView(LoginRequiredMixin, CreateView):
+class ServiceCreateView(LoginRequiredMixin, RevisionMixin, CreateView):
model = Service
form_class = ServiceForm
+ revision_comment = 'created service'
def get_success_url(self):
return reverse('service', kwargs={'pk': self.object.id})
-class ScheduleCreateView(LoginRequiredMixin, CreateView):
+class ScheduleCreateView(LoginRequiredMixin, RevisionMixin, CreateView):
model = Schedule
form_class = ScheduleForm
success_url = reverse_lazy('shifts')
+ revision_comment = 'created schedule'
+ revision_follow = ['service_set']
-class ServiceUpdateView(LoginRequiredMixin, UpdateView):
+class ServiceUpdateView(LoginRequiredMixin, RevisionMixin, UpdateView):
model = Service
form_class = ServiceForm
@@ -663,27 +674,31 @@ def get_success_url(self):
return reverse('service', kwargs={'pk': self.object.id})
-class ScheduleUpdateView(LoginRequiredMixin, UpdateView):
+class ScheduleUpdateView(LoginRequiredMixin, RevisionMixin, UpdateView):
model = Schedule
form_class = ScheduleForm
context_object_name = 'schedules'
success_url = reverse_lazy('shifts')
+ revision_follow = ['service_set']
-class ServiceDeleteView(LoginRequiredMixin, DeleteView):
+class ServiceDeleteView(LoginRequiredMixin, RevisionMixin, DeleteView):
model = Service
success_url = reverse_lazy('services')
context_object_name = 'service'
template_name = 'cabotapp/service_confirm_delete.html'
+ revision_comment = 'deleted service'
-class ScheduleDeleteView(LoginRequiredMixin, DeleteView):
+class ScheduleDeleteView(LoginRequiredMixin, RevisionMixin, DeleteView):
model = Schedule
form_class = ScheduleForm
success_url = reverse_lazy('shifts')
context_object_name = 'schedule'
template_name = 'cabotapp/schedule_confirm_delete.html'
+ revision_comment = 'deleted schedule'
+ revision_follow = ['service_set']
class ScheduleSnoozeWarningsView(LoginRequiredMixin, View):
diff --git a/cabot/metricsapp/views/grafana_elastic.py b/cabot/metricsapp/views/grafana_elastic.py
index a9a8b25fc..5e22a7f81 100644
--- a/cabot/metricsapp/views/grafana_elastic.py
+++ b/cabot/metricsapp/views/grafana_elastic.py
@@ -4,16 +4,19 @@
from django.shortcuts import render
from django.views.generic import UpdateView, CreateView
from cabot.cabotapp.views import LoginRequiredMixin
+from cabot.cabotapp.revision_utils import RevisionMixin
from cabot.metricsapp.api import get_es_status_check_fields, get_status_check_fields
from cabot.metricsapp.forms import GrafanaElasticsearchStatusCheckForm, GrafanaElasticsearchStatusCheckUpdateForm
from cabot.metricsapp.models import ElasticsearchStatusCheck, GrafanaDataSource
from cabot.metricsapp.models.grafana import build_grafana_panel_from_session, set_grafana_panel_from_session
-class GrafanaElasticsearchStatusCheckCreateView(LoginRequiredMixin, CreateView):
+class GrafanaElasticsearchStatusCheckCreateView(LoginRequiredMixin, RevisionMixin, CreateView):
model = ElasticsearchStatusCheck
form_class = GrafanaElasticsearchStatusCheckForm
template_name = 'metricsapp/grafana_create.html'
+ revision_comment = 'created check'
+ revision_follow = ['service_set']
def get_form_kwargs(self):
kwargs = super(GrafanaElasticsearchStatusCheckCreateView, self).get_form_kwargs()
@@ -53,10 +56,11 @@ def form_valid(self, form):
return response
-class GrafanaElasticsearchStatusCheckUpdateView(LoginRequiredMixin, UpdateView):
+class GrafanaElasticsearchStatusCheckUpdateView(LoginRequiredMixin, RevisionMixin, UpdateView):
model = ElasticsearchStatusCheck
form_class = GrafanaElasticsearchStatusCheckUpdateForm
template_name = 'metricsapp/grafana_create.html'
+ revision_follow = ['service_set']
# note - this view also updates self.object.grafana_panel (via ElasticsearchStatusCheckUpdateForm)
@@ -94,6 +98,8 @@ class GrafanaElasticsearchStatusCheckRefreshView(GrafanaElasticsearchStatusCheck
Note this requires the session vars by earlier Views in the "create Grafana check" flow to work
(GrafanaDashboardSelectView, etc).
"""
+ revision_comment = "Refreshed from Grafana"
+
def get_form_kwargs(self):
"""Add kwargs['data'] and fill it with the latest values from Grafana"""
kwargs = super(GrafanaElasticsearchStatusCheckRefreshView, self).get_form_kwargs()
diff --git a/cabot/settings.py b/cabot/settings.py
index 2170052cf..477b3639c 100644
--- a/cabot/settings.py
+++ b/cabot/settings.py
@@ -142,6 +142,7 @@
'rest_framework',
'social_django',
'timezone_field',
+ 'reversion',
)
# Load additional apps from configuration file
diff --git a/cabot/templates/cabotapp/model_diff.html b/cabot/templates/cabotapp/model_diff.html
new file mode 100644
index 000000000..172451c9d
--- /dev/null
+++ b/cabot/templates/cabotapp/model_diff.html
@@ -0,0 +1,29 @@
+<ul class="list-unstyled">
+ {% for revision, changes in revisions %}
+ <li {% if not forloop.first %}class="collapse last-changed-detail"{% endif %}>
+ {{ revision.date_created }}
+ {% if revision.user %}<a href="mailto:{{ revision.user.email }}">{{ revision.user.username }}</a>{% endif %}
+ {% if revision.comment %}
+ {{ revision.comment }}
+ {% else %}
+ changed {{ changes.keys|join:", "|truncatechars:30 }}
+ {% endif %}
+ {% if forloop.first %}
+ <span class="glyphicon glyphicon-plus" data-toggle="collapse" data-target=".last-changed-detail"></span>
+ {% endif %}
+ <ul class="collapse last-changed-detail">
+ {% for field_name, change in changes.items %}
+ {% autoescape off %}
+ {# need to use the truncate_html version because change strings can be HTML (for styling...) #}
+ {# truncatechars_html doesn't exist in django 1.6 and backporting requires changing the Truncator class #}
+ {# :D :( :'( #}
+ <li>{{ field_name }}: {{ change.0|truncatewords_html:12 }}
+ <span class="glyphicon glyphicon-arrow-right"></span>
+ {{ change.1|truncatewords_html:6 }}
+ </li>
+ {% endautoescape %}
+ {% endfor %}
+ </ul>
+ </li>
+ {% endfor %}
+</ul>
diff --git a/cabot/templates/cabotapp/service_detail.html b/cabot/templates/cabotapp/service_detail.html
index 17cadc505..c9a62ff30 100644
--- a/cabot/templates/cabotapp/service_detail.html
+++ b/cabot/templates/cabotapp/service_detail.html
@@ -57,6 +57,11 @@ <h5>
<div class="col-xs-8"><h5>None</h5></div>
{% endif %}
</div>
+ <div class="col-xs-12">
+ <div class="col-xs-1"><h5><i class="glyphicon glyphicon-flag"></i></h5></div>
+ <div class="col-xs-3"><h5><span class="break"></span>Last changed</h5></div>
+ <div class="col-xs-8">{% include 'cabotapp/model_diff.html' with revisions=revisions only %}</div>
+ </div>
</div>
<div class="col-xs-12 col-md-6">
<div class="col-xs-1"><h3><i class="fa fa-bar-chart-o"></i></h3></div>
diff --git a/cabot/templates/cabotapp/statuscheck_detail.html b/cabot/templates/cabotapp/statuscheck_detail.html
index 3e466f31a..060e55f43 100644
--- a/cabot/templates/cabotapp/statuscheck_detail.html
+++ b/cabot/templates/cabotapp/statuscheck_detail.html
@@ -25,10 +25,15 @@
</div>
<hr>
<div class="row">
- <div class="col-xs-12">
- <div class="col-xs-11"><h3>Runbook</h3></div>
+ <div class="col-xs-6">
+ <div class="col-xs-12"><h3>Runbook</h3></div>
<div class="col-xs-12">{% autoescape off %}{{ check.runbook }}{% endautoescape %}</div>
</div>
+ <div class="col-xs-6">
+ <div class="col-xs-12"><h4>Last Changed</h4></div>
+ <div class="col-xs-12">{% include 'cabotapp/model_diff.html' with revisions=revisions only %}</div>
+ </div>
+</div>
<hr>
<div class="row">
<div class="col-xs-12">
diff --git a/requirements.txt b/requirements.txt
index 991b50d54..834ca36c9 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -48,3 +48,4 @@ unittest-xml-reporting==2.1.0
PyMySQL==0.8.0
jsondiff==1.1.2
django-timezone-field==3.0
+django-reversion==1.8.7
From 642423a80c32405f92badcd0e49493ea8e3d7794 Mon Sep 17 00:00:00 2001
From: Alec Lofquist <alec.lofquist@affirm.com>
Date: Tue, 19 Feb 2019 14:55:55 -0800
Subject: [PATCH 2/2] Upgrade django-reversion now that we're on django 1.11
---
cabot/cabotapp/admin.py | 6 +-
cabot/cabotapp/revision_utils.py | 92 ++---------------------
cabot/cabotapp/views.py | 3 +-
cabot/metricsapp/views/grafana_elastic.py | 2 +-
requirements.txt | 2 +-
5 files changed, 13 insertions(+), 92 deletions(-)
diff --git a/cabot/cabotapp/admin.py b/cabot/cabotapp/admin.py
index 878a90c46..91a1f4357 100644
--- a/cabot/cabotapp/admin.py
+++ b/cabot/cabotapp/admin.py
@@ -1,4 +1,4 @@
-import reversion
+from reversion.admin import VersionAdmin
from django.contrib import admin
from .models import (
UserProfile,
@@ -15,11 +15,11 @@
from .alert import AlertPluginUserData, AlertPlugin
-class ServiceAdmin(reversion.VersionAdmin):
+class ServiceAdmin(VersionAdmin):
pass
-class StatusCheckAdmin(reversion.VersionAdmin):
+class StatusCheckAdmin(VersionAdmin):
pass
diff --git a/cabot/cabotapp/revision_utils.py b/cabot/cabotapp/revision_utils.py
index abe13964c..7db51a9d1 100644
--- a/cabot/cabotapp/revision_utils.py
+++ b/cabot/cabotapp/revision_utils.py
@@ -1,22 +1,8 @@
-from collections import defaultdict
-
import reversion
-from django.contrib.auth.models import AnonymousUser
-from django.db import transaction
-from django.db.models import Manager, Model
-from django.db.models.query import QuerySet
+from reversion.models import Version
from django.utils.html import escape
-def add_to_revision(obj):
- """Manually add the current version of obj to the current revision."""
- # backport of reversion.add_to_revision() from django-reversion 2.x
- manager = reversion.default_revision_manager
- ctx_manager = reversion.revision_context_manager
- adapter = manager.get_adapter(type(obj))
- ctx_manager.add_to_context(manager, obj, adapter.get_version_data(obj, ctx_manager.get_db()))
-
-
def get_revisions(obj, n_revisions=3):
"""
Get the last N diffs for an object that is tracked by django-reversion, as (v1, v2) human-readable string tuples.
@@ -28,7 +14,7 @@ def get_revisions(obj, n_revisions=3):
:return: list of (revision, {field: (v1_str, v2_str), ...}) tuples
"""
# type: (Any, int) -> List[Tuple[reversion.models.Revision, Dict[str, Tuple[str, str]]]]
- recent_versions = reversion.get_for_object(obj)[:n_revisions+1]
+ recent_versions = Version.objects.get_for_object(obj)[:n_revisions+1]
# grab fields on the object's model
fields = [f for f in obj._meta.fields]
@@ -95,7 +81,7 @@ def field_to_str_escaped(field, version):
:param version: django-reversion Version object
:return: human-readable string representing field of version; may contain HTML
"""
- value = version.field_dict[field.name]
+ value = version.field_dict.get(field.name)
if field.get_internal_type() == 'ManyToManyField':
ids = [int(v) for v in value]
related_model = field.rel.to
@@ -106,7 +92,7 @@ def field_to_str_escaped(field, version):
ids.remove(related.pk)
# if it's not in the live set, try and find it in a "deleted" revision
- for version in reversion.get_deleted(related_model).filter(object_id_int__in=ids):
+ for version in Version.objects.get_deleted(related_model).filter(object_id__in=ids):
# we use the object repr here because version.object is none for some reason
related_objs.append(MutedStr('deleted: {}'.format(version.object_repr)))
ids.remove(version.object_id_int)
@@ -129,77 +115,11 @@ def field_to_str_escaped(field, version):
changes = {}
for field in fields:
- v1_value = v1.field_dict[field.name]
- v2_value = v2.field_dict[field.name]
+ v1_value = v1.field_dict.get(field.name)
+ v2_value = v2.field_dict.get(field.name)
if v1_value != v2_value:
v1_str = field_to_str_escaped(field, v1)
v2_str = field_to_str_escaped(field, v2)
changes[field.name] = (v1_str, v2_str)
return changes
-
-
-class RevisionMixin(object):
- """
- Backport of reversion.views.RevisionMixin since we are using a version of reversion from 2015.
- Wraps HTTP requests (except for GET/HEAD/OPTIONS) in an atomic django-reversion revision, plus
- some magic to follow reverse m2m fields in checks (which django-reversion should handle, but I
- can't get it to work :shakes fist:).
-
- Set 'revision_comment' if you would like this revision to be recorded as a fixed string instead
- of a list of changed fields. Currently used for 'created' and 'deleted' messages.
-
- Set 'revision_follow' to a list of attributes on self.object that return querysets or foreign keys
- that we should include with any revisions (for example, 'service_set' for status checks).
- """
-
- revision_comment = ''
- revision_follow = []
-
- def dispatch(self, *args, **kwargs):
- if self.request.method not in ('GET', 'HEAD', 'OPTIONS'):
- with transaction.atomic(), reversion.create_revision():
- # fill in the revision metadata
- if not isinstance(self.request.user, AnonymousUser):
- reversion.set_user(self.request.user)
- if self.revision_comment:
- reversion.set_comment(self.revision_comment)
-
- related = self._gather_reverse_pks()
- ret = super(RevisionMixin, self).dispatch(*args, **kwargs)
- related = self._gather_reverse_pks(related)
-
- for model, pks in related.items():
- for obj in model.objects.filter(pk__in=pks):
- add_to_revision(obj)
-
- return ret
- else:
- return super(RevisionMixin, self).dispatch(*args, **kwargs)
-
- def _gather_reverse_pks(self, model_to_pks=None):
- model_to_pks = model_to_pks or defaultdict(list)
-
- if not self.revision_follow:
- return model_to_pks
-
- obj = getattr(self, 'object', None)
- if obj is None and 'pk' in getattr(self, 'kwargs', []) and hasattr(self, 'get_object'):
- obj = self.get_object()
- if obj is None or obj.pk is None:
- return model_to_pks
-
- for field_name in self.revision_follow:
- value = getattr(obj, field_name, None)
- if isinstance(value, (Manager, QuerySet)):
- # m2m/o2m relationship
- field_model = value.model
- model_to_pks[field_model].extend(value.values_list('pk', flat=True))
- elif isinstance(value, Model):
- # foreign key
- field_model = value._meta.model
- model_to_pks[field_model].append(value.pk)
- else:
- raise RuntimeError('Could not follow field {} (missing? unsupported type?)'.format(field_name))
-
- return model_to_pks
diff --git a/cabot/cabotapp/views.py b/cabot/cabotapp/views.py
index 61bf766bc..24b4c924d 100644
--- a/cabot/cabotapp/views.py
+++ b/cabot/cabotapp/views.py
@@ -8,7 +8,8 @@
from timezone_field import TimeZoneFormField
from cabot.cabotapp.alert import AlertPlugin
-from cabot.cabotapp.revision_utils import get_revisions, RevisionMixin
+from cabot.cabotapp.revision_utils import get_revisions
+from reversion.views import RevisionMixin
from models import (StatusCheck,
JenkinsStatusCheck,
HttpStatusCheck,
diff --git a/cabot/metricsapp/views/grafana_elastic.py b/cabot/metricsapp/views/grafana_elastic.py
index 5e22a7f81..d3f5a1682 100644
--- a/cabot/metricsapp/views/grafana_elastic.py
+++ b/cabot/metricsapp/views/grafana_elastic.py
@@ -3,8 +3,8 @@
from django.core.urlresolvers import reverse
from django.shortcuts import render
from django.views.generic import UpdateView, CreateView
+from reversion.views import RevisionMixin
from cabot.cabotapp.views import LoginRequiredMixin
-from cabot.cabotapp.revision_utils import RevisionMixin
from cabot.metricsapp.api import get_es_status_check_fields, get_status_check_fields
from cabot.metricsapp.forms import GrafanaElasticsearchStatusCheckForm, GrafanaElasticsearchStatusCheckUpdateForm
from cabot.metricsapp.models import ElasticsearchStatusCheck, GrafanaDataSource
diff --git a/requirements.txt b/requirements.txt
index 834ca36c9..7b8c5a12e 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -48,4 +48,4 @@ unittest-xml-reporting==2.1.0
PyMySQL==0.8.0
jsondiff==1.1.2
django-timezone-field==3.0
-django-reversion==1.8.7
+django-reversion==3.0.3