Navigation: fix for missing events

Now that we have eviction, events may disappear. Deal with it:

* event-specific 404 that still allows for navigation
* first/last buttons
* navigation to prev/next when prev/next is not just 1 step away
* don't use HttpRedirect for "lookup based" views
    in principle, the thing you were looking for might go missing in-between
    drawback: these URLs are not "stable"

events/models.py

@@ -4,6 +4,7 @@ import uuid
 
 from django.db import models
 from django.db.utils import IntegrityError
+from django.db.models import Min, Max
 
 from projects.models import Project
 from compat.timestamp import parse_timestamp
@@ -230,3 +231,15 @@ class Event(models.Model):
             assert re.match(
                 r".*unique constraint failed.*events_event.*project_id.*events_event.*event_id", str(e).lower())
             return None, False
+
+    def get_digest_order_bounds(self):
+        if not hasattr(self, "_digest_order_bounds"):
+            d = Event.objects.filter(issue_id=self.issue.id).aggregate(lo=Min("digest_order"), hi=Max("digest_order"))
+            self._digest_order_bounds = d["lo"], d["hi"]
+        return self._digest_order_bounds
+
+    def has_prev(self):
+        return self.digest_order > self.get_digest_order_bounds()[0]
+
+    def has_next(self):
+        return self.digest_order < self.get_digest_order_bounds()[1]

issues/templates/issues/_event_nav.html

@@ -1,5 +1,16 @@
-                {% if event.digest_order > 1 %}
+                {% if event.has_prev %} {# no need for 'is_first': if you can go to the left, you can go all the way to the left too #}
-                <a href="{% url this_view issue_pk=issue.pk digest_order=event.digest_order|add:"-1" %}" class="font-bold text-slate-500 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md hover:bg-slate-200 active:ring inline-flex items-center justify-center">
+                <a href="{% url this_view issue_pk=issue.pk nav="first" %}" class="font-bold text-slate-500 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md hover:bg-slate-200 active:ring inline-flex items-center justify-center">
+                <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M3.22 7.595a.75.75 0 0 0 0 1.06l3.25 3.25a.75.75 0 0 0 1.06-1.06l-2.72-2.72 2.72-2.72a.75.75 0 0 0-1.06-1.06l-3.25 3.25Zm8.25-3.25-3.25 3.25a.75.75 0 0 0 0 1.06l3.25 3.25a.75.75 0 1 0 1.06-1.06l-2.72-2.72 2.72-2.72a.75.75 0 0 0-1.06-1.06Z" clip-rule="evenodd" />
+</svg>
+                </a>
+                {% else %}
+                <div class="font-bold text-slate-300 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md inline-flex items-center justify-center">
+                <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M3.22 7.595a.75.75 0 0 0 0 1.06l3.25 3.25a.75.75 0 0 0 1.06-1.06l-2.72-2.72 2.72-2.72a.75.75 0 0 0-1.06-1.06l-3.25 3.25Zm8.25-3.25-3.25 3.25a.75.75 0 0 0 0 1.06l3.25 3.25a.75.75 0 1 0 1.06-1.06l-2.72-2.72 2.72-2.72a.75.75 0 0 0-1.06-1.06Z" clip-rule="evenodd" />
+                </div>
+                {% endif %}
+
+                {% if event.has_prev %}
+                <a href="{% url this_view issue_pk=issue.pk digest_order=event.digest_order nav="prev" %}" class="font-bold text-slate-500 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md hover:bg-slate-200 active:ring inline-flex items-center justify-center">
                 <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M9.78 4.22a.75.75 0 0 1 0 1.06L7.06 8l2.72 2.72a.75.75 0 1 1-1.06 1.06L5.47 8.53a.75.75 0 0 1 0-1.06l3.25-3.25a.75.75 0 0 1 1.06 0Z" clip-rule="evenodd" /></svg>
                 </a>
                 {% else %}
@@ -8,8 +19,8 @@
                 </div>
                 {% endif %}
 
-                {% if event.digest_order < issue.digested_event_count %}
+                {% if event.has_next %}
-                <a href="{% url this_view issue_pk=issue.pk digest_order=event.digest_order|add:"1" %}"" class="font-bold text-slate-500 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md hover:bg-slate-200 active:ring inline-flex items-center justify-center">
+                <a href="{% url this_view issue_pk=issue.pk digest_order=event.digest_order nav="next" %}"" class="font-bold text-slate-500 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md hover:bg-slate-200 active:ring inline-flex items-center justify-center">
                 <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M6.22 4.22a.75.75 0 0 1 1.06 0l3.25 3.25a.75.75 0 0 1 0 1.06l-3.25 3.25a.75.75 0 0 1-1.06-1.06L8.94 8 6.22 5.28a.75.75 0 0 1 0-1.06Z" clip-rule="evenodd" /></svg>
                 </a>
                 {% else %}
@@ -17,3 +28,14 @@
                 <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M6.22 4.22a.75.75 0 0 1 1.06 0l3.25 3.25a.75.75 0 0 1 0 1.06l-3.25 3.25a.75.75 0 0 1-1.06-1.06L8.94 8 6.22 5.28a.75.75 0 0 1 0-1.06Z" clip-rule="evenodd" /></svg>
                 </div>
                 {% endif %}
+
+                {% if event.has_next %}
+                <a href="{% url this_view issue_pk=issue.pk nav="last" %}"" class="font-bold text-slate-500 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md hover:bg-slate-200 active:ring inline-flex items-center justify-center">
+                <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M12.78 7.595a.75.75 0 0 1 0 1.06l-3.25 3.25a.75.75 0 0 1-1.06-1.06l2.72-2.72-2.72-2.72a.75.75 0 0 1 1.06-1.06l3.25 3.25Zm-8.25-3.25 3.25 3.25a.75.75 0 0 1 0 1.06l-3.25 3.25a.75.75 0 0 1-1.06-1.06l2.72-2.72-2.72-2.72a.75.75 0 0 1 1.06-1.06Z" clip-rule="evenodd" /></svg>
+                </a>
+                {% else %}
+                <div class="font-bold text-slate-300 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md inline-flex items-center justify-center">
+                <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M12.78 7.595a.75.75 0 0 1 0 1.06l-3.25 3.25a.75.75 0 0 1-1.06-1.06l2.72-2.72-2.72-2.72a.75.75 0 0 1 1.06-1.06l3.25 3.25Zm-8.25-3.25 3.25 3.25a.75.75 0 0 1 0 1.06l-3.25 3.25a.75.75 0 0 1-1.06-1.06l2.72-2.72-2.72-2.72a.75.75 0 0 1 1.06-1.06Z" clip-rule="evenodd" /></svg>
+
+                </div>
+                {% endif %}

issues/templates/issues/event_404.html

@@ -0,0 +1,47 @@
+{% extends "issues/base.html" %}
+{% load static %}
+{% load stricter_templates %}
+
+{% block tab_content %}
+
+{# this is here to fool tailwind (because we're foolish enough to put html in python) <span class="text-xs"></span> #}
+<div class="flex">
+
+    <div class="overflow-hidden">
+        <div class="italic">xxxx xx xx xx:xx (Event xxx of {{ issue.digested_event_count }})</div>
+    </div>
+
+    <div class="ml-auto flex-none">
+        <div class="flex place-content-end">
+            {# copy/paste of _event_nav, but not based on any event (we have none), prev/next are meaningless also #}
+            {# so we have first/last enabled, and the middle ones disabled #}
+
+                <a href="{% url this_view issue_pk=issue.pk nav="first" %}" class="font-bold text-slate-500 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md hover:bg-slate-200 active:ring inline-flex items-center justify-center">
+                <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M3.22 7.595a.75.75 0 0 0 0 1.06l3.25 3.25a.75.75 0 0 0 1.06-1.06l-2.72-2.72 2.72-2.72a.75.75 0 0 0-1.06-1.06l-3.25 3.25Zm8.25-3.25-3.25 3.25a.75.75 0 0 0 0 1.06l3.25 3.25a.75.75 0 1 0 1.06-1.06l-2.72-2.72 2.72-2.72a.75.75 0 0 0-1.06-1.06Z" clip-rule="evenodd" />
+</svg>
+                </a>
+
+                <div class="font-bold text-slate-300 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md inline-flex items-center justify-center">
+                <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M9.78 4.22a.75.75 0 0 1 0 1.06L7.06 8l2.72 2.72a.75.75 0 1 1-1.06 1.06L5.47 8.53a.75.75 0 0 1 0-1.06l3.25-3.25a.75.75 0 0 1 1.06 0Z" clip-rule="evenodd" /></svg>
+                </div>
+
+                <div class="font-bold text-slate-300 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md inline-flex items-center justify-center">
+                <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M6.22 4.22a.75.75 0 0 1 1.06 0l3.25 3.25a.75.75 0 0 1 0 1.06l-3.25 3.25a.75.75 0 0 1-1.06-1.06L8.94 8 6.22 5.28a.75.75 0 0 1 0-1.06Z" clip-rule="evenodd" /></svg>
+                </div>
+
+                <a href="{% url this_view issue_pk=issue.pk nav="last" %}"" class="font-bold text-slate-500 border-slate-300 pl-4 pr-4 pb-1 pt-1 mr-2 border-2 rounded-md hover:bg-slate-200 active:ring inline-flex items-center justify-center">
+                <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="currentColor" class="w-6 h-6"><path fill-rule="evenodd" d="M12.78 7.595a.75.75 0 0 1 0 1.06l-3.25 3.25a.75.75 0 0 1-1.06-1.06l2.72-2.72-2.72-2.72a.75.75 0 0 1 1.06-1.06l3.25 3.25Zm-8.25-3.25 3.25 3.25a.75.75 0 0 1 0 1.06l-3.25 3.25a.75.75 0 0 1-1.06-1.06l2.72-2.72-2.72-2.72a.75.75 0 0 1 1.06-1.06Z" clip-rule="evenodd" /></svg>
+                </a>
+
+        </div>
+    </div>
+</div>
+
+
+<h1 class="text-2xl font-bold mt-4">404: Event missing from Bugsink</h1>
+
+<div class="mb-6">
+    This event cannot be found. It could have been removed manually or as part of the eviction process.
+</div>
+
+{% endblock %}

issues/tests.py

@@ -407,11 +407,6 @@ class ViewTests(TransactionTestCase):
         response = self.client.get(f"/issues/issue/{self.issue.id}/history/")
         self.assertContains(response, self.issue.title())
 
-    def test_issue_last_event(self):
-        response = self.client.get(f"/issues/issue/{self.issue.id}/event/last/")
-        self.assertEquals(302, response.status_code)
-        self.assertTrue(str(self.event.id) in response.url)
-
     def test_issue_event_list(self):
         response = self.client.get(f"/issues/issue/{self.issue.id}/events/")
         self.assertContains(response, self.issue.title())

issues/urls.py

@@ -1,9 +1,27 @@
-from django.urls import path
+from django.urls import path, register_converter
 
 from .views import (
-    issue_list, issue_event_stacktrace, issue_event_details, issue_last_event, issue_event_list, issue_history,
+    issue_list, issue_event_stacktrace, issue_event_details, issue_event_list, issue_history, issue_grouping,
-    issue_grouping, issue_event_breadcrumbs, event_by_internal_id, history_comment_new, history_comment_edit,
+    issue_event_breadcrumbs, event_by_internal_id, history_comment_new, history_comment_edit, history_comment_delete)
-    history_comment_delete)
+
+
+def regex_converter(passed_regex):
+
+    class RegexConverter:
+        regex = passed_regex
+
+        def to_python(self, value):
+            return value
+
+        def to_url(self, value):
+            return value
+
+    return RegexConverter
+
+
+register_converter(regex_converter("(first|last)"), "first-last")
+register_converter(regex_converter("(prev|next)"), "prev-next")
+
 
 urlpatterns = [
     path('<int:project_pk>/', issue_list, {"state_filter": "open"}, name="issue_list_open"),
@@ -13,7 +31,6 @@ urlpatterns = [
     path('<int:project_pk>/all/', issue_list, {"state_filter": "all"}, name="issue_list_all"),
 
     path('issue/<uuid:issue_pk>/event/<uuid:event_pk>/', issue_event_stacktrace, name="event_stacktrace"),
-
     path('issue/<uuid:issue_pk>/event/<uuid:event_pk>/details/', issue_event_details, name="event_details"),
     path('issue/<uuid:issue_pk>/event/<uuid:event_pk>/breadcrumbs/', issue_event_breadcrumbs, name="event_breadcrumbs"),
 
@@ -22,9 +39,24 @@ urlpatterns = [
     path('issue/<uuid:issue_pk>/event/<int:digest_order>/breadcrumbs/', issue_event_breadcrumbs,
          name="event_breadcrumbs"),
 
+    path('issue/<uuid:issue_pk>/event/<int:digest_order>/', issue_event_stacktrace, name="event_stacktrace"),
+    path('issue/<uuid:issue_pk>/event/<int:digest_order>/details/', issue_event_details, name="event_details"),
+    path('issue/<uuid:issue_pk>/event/<int:digest_order>/breadcrumbs/', issue_event_breadcrumbs,
+         name="event_breadcrumbs"),
+
+    path('issue/<uuid:issue_pk>/event/<int:digest_order>/<prev-next:nav>/', issue_event_stacktrace,
+         name="event_stacktrace"),
+    path('issue/<uuid:issue_pk>/event/<int:digest_order>/<prev-next:nav>/details/', issue_event_details,
+         name="event_details"),
+    path('issue/<uuid:issue_pk>/event/<int:digest_order>/<prev-next:nav>/breadcrumbs/', issue_event_breadcrumbs,
+         name="event_breadcrumbs"),
+
+    path('issue/<uuid:issue_pk>/event/<first-last:nav>/', issue_event_stacktrace, name="event_stacktrace"),
+    path('issue/<uuid:issue_pk>/event/<first-last:nav>/details/', issue_event_details, name="event_details"),
+    path('issue/<uuid:issue_pk>/event/<first-last:nav>/breadcrumbs/', issue_event_details, name="event_breadcrumbs"),
+
     path('issue/<uuid:issue_pk>/history/', issue_history),
     path('issue/<uuid:issue_pk>/grouping/', issue_grouping),
-    path('issue/<uuid:issue_pk>/event/last/', issue_last_event),
     path('issue/<uuid:issue_pk>/events/', issue_event_list),
 
     path('event/<uuid:event_pk>/', event_by_internal_id, name="event_by_internal_id"),

issues/views.py

@@ -9,6 +9,7 @@ from django.utils.safestring import mark_safe
 from django.template.defaultfilters import date
 from django.urls import reverse
 from django.core.exceptions import PermissionDenied
+from django.http import Http404
 
 from bugsink.decorators import project_membership_required, issue_membership_required, atomic_for_request_method
 from bugsink.transaction import durable_atomic
@@ -235,14 +236,6 @@ def event_by_internal_id(request, event_pk):
     return redirect(issue_event_stacktrace, issue_pk=issue.pk, event_pk=event.pk)
 
 
-@atomic_for_request_method
-@issue_membership_required
-def issue_last_event(request, issue):
-    last_event = issue.event_set.order_by("timestamp").last()
-
-    return redirect(issue_event_stacktrace, issue_pk=issue.pk, event_pk=last_event.pk)
-
-
 def _handle_post(request, issue):
     if _is_valid_action(request.POST["action"], issue):
         _apply_action(IssueStateManager, issue, request.POST["action"], request.user)
@@ -257,27 +250,49 @@ def _handle_post(request, issue):
     return HttpResponseRedirect(request.path_info)
 
 
-def _get_event(issue, event_pk, digest_order):
+def _get_event(issue, event_pk, digest_order, nav):
+    if nav is not None:
+        if nav == "first":
+            return Event.objects.filter(issue=issue).order_by("digest_order").first()
+        if nav == "last":
+            return Event.objects.filter(issue=issue).order_by("digest_order").last()
+
+        if nav in ["prev", "next"]:
+            if nav == "prev":
+                result = Event.objects.filter(
+                    issue=issue, digest_order__lt=digest_order).order_by("-digest_order").first()
+            elif nav == "next":
+                result = Event.objects.filter(
+                    issue=issue, digest_order__gt=digest_order).order_by("digest_order").first()
+            if result is None:
+                raise Event.DoesNotExist
+            return result
+
+        raise Http404("Cannot look up with '%s'" % nav)
+
     if event_pk is not None:
         # we match on both internal and external id, trying internal first
         try:
             return Event.objects.get(pk=event_pk)
         except Event.DoesNotExist:
-            return get_object_or_404(Event, issue=issue, event_id=event_pk)
+            return Event.objects.get(issue=issue, event_id=event_pk)
 
     elif digest_order is not None:
-        return get_object_or_404(Event, issue=issue, digest_order=digest_order)
+        return Event.objects.get(issue=issue, digest_order=digest_order)
     else:
         raise ValueError("either event_pk or digest_order must be provided")
 
 
 @atomic_for_request_method
 @issue_membership_required
-def issue_event_stacktrace(request, issue, event_pk=None, digest_order=None):
+def issue_event_stacktrace(request, issue, event_pk=None, digest_order=None, nav=None):
     if request.method == "POST":
         return _handle_post(request, issue)
 
-    event = _get_event(issue, event_pk, digest_order)
+    try:
+        event = _get_event(issue, event_pk, digest_order, nav)
+    except Event.DoesNotExist:
+        return issue_event_404(request, issue, "stacktrace", "event_stacktrace")
 
     parsed_data = json.loads(event.data)
 
@@ -318,13 +333,31 @@ def issue_event_stacktrace(request, issue, event_pk=None, digest_order=None):
     })
 
 
+def issue_event_404(request, issue, tab, this_view):
+    """If the Event is 404, but the issue is not, we can still show the issue page; we show a message for the event"""
+
+    last_event = issue.event_set.order_by("timestamp").last()  # the template needs this for the tabs, we pick the last
+    return render(request, "issues/event_404.html", {
+        "tab": tab,
+        "this_view": this_view,
+        "project": issue.project,
+        "issue": issue,
+        "event": last_event,
+        "is_event_page": False,  # this variable is used to denote "we have event-related info", which we don't
+        "mute_options": GLOBAL_MUTE_OPTIONS,
+    })
+
+
 @atomic_for_request_method
 @issue_membership_required
-def issue_event_breadcrumbs(request, issue, event_pk=None, digest_order=None):
+def issue_event_breadcrumbs(request, issue, event_pk=None, digest_order=None, nav=None):
     if request.method == "POST":
         return _handle_post(request, issue)
 
-    event = _get_event(issue, event_pk, digest_order)
+    try:
+        event = _get_event(issue, event_pk, digest_order, nav)
+    except Event.DoesNotExist:
+        return issue_event_404(request, issue, "breadcrumbs", "event_breadcrumbs")
 
     parsed_data = json.loads(event.data)
 
@@ -348,11 +381,14 @@ def _date_with_milis_html(timestamp):
 
 @atomic_for_request_method
 @issue_membership_required
-def issue_event_details(request, issue, event_pk=None, digest_order=None):
+def issue_event_details(request, issue, event_pk=None, digest_order=None, nav=None):
     if request.method == "POST":
         return _handle_post(request, issue)
 
-    event = _get_event(issue, event_pk, digest_order)
+    try:
+        event = _get_event(issue, event_pk, digest_order, nav)
+    except Event.DoesNotExist:
+        return issue_event_404(request, issue, "event-details", "event_details")
     parsed_data = json.loads(event.data)
 
     key_info = [
@@ -361,8 +397,9 @@ def issue_event_details(request, issue, event_pk=None, digest_order=None):
         ("bugsink_internal_id", event.id),
         ("issue_id", issue.id),
         ("timestamp", _date_with_milis_html(event.timestamp)),
-        ("ingested_at", _date_with_milis_html(event.ingested_at)),
+        ("ingested at", _date_with_milis_html(event.ingested_at)),
-        ("digested_at", _date_with_milis_html(event.digested_at)),
+        ("digested at", _date_with_milis_html(event.digested_at)),
+        ("digest order", event.digest_order),
     ]
     if parsed_data.get("logger"):
         key_info.append(("logger", parsed_data["logger"]))