Event log: only the newest event expanded, and only the 50 most recent (closes #349)
check / check (push) Successful in 3m35s
check / check (push) Successful in 3m35s
The event log now loads the 50 newest events in its query and opens only the newest on load, as the recent events on a webhook's page already do. With 50 at most there is never a second page, so paging is gone: the page links, the `page` query parameter, the page number that Replay and Resubmit carried back to the log, and `pageOrFirst` with its test. The count beside the heading reads "50 most recent of N events" when there are more. The comments and README lines that named `page` as the query parameter the service reads now name `next` and `notice`. Model: opus-5-5
This commit is contained in:
@@ -2,7 +2,6 @@ package handlers
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"strconv"
|
||||
|
||||
"github.com/go-chi/chi"
|
||||
"gorm.io/gorm"
|
||||
@@ -329,24 +328,15 @@ func replayBody(body string) *string {
|
||||
}
|
||||
|
||||
// redirectToEventLog redirects a replay or resubmit back to the event
|
||||
// log it was triggered from, carrying the outcome as its notice and
|
||||
// the page number the form submitted.
|
||||
// log it was triggered from, carrying the outcome as its notice.
|
||||
func redirectToEventLog(
|
||||
w http.ResponseWriter,
|
||||
r *http.Request,
|
||||
webhook database.Webhook,
|
||||
code noticeCode,
|
||||
) {
|
||||
dest := withNotice("/hook/"+webhook.ID+"/events", code)
|
||||
|
||||
// The page is read from the form rather than the query string:
|
||||
// this is a POST, and its query string is what logs and Referer
|
||||
// headers record.
|
||||
if page := pageOrFirst(
|
||||
r.PostFormValue("page"),
|
||||
); page > 1 {
|
||||
dest += "&page=" + strconv.Itoa(page)
|
||||
}
|
||||
|
||||
http.Redirect(w, r, dest, http.StatusSeeOther)
|
||||
http.Redirect(
|
||||
w, r, withNotice("/hook/"+webhook.ID+"/events", code),
|
||||
http.StatusSeeOther,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -435,9 +435,7 @@ func TestHandleSourceLogs_BoundsRenderedAttempts(t *testing.T) {
|
||||
}).Error)
|
||||
}
|
||||
|
||||
views := h.LoadEventLogViewsForTest(
|
||||
httptest.NewRecorder(), *wh, 1,
|
||||
)
|
||||
views := h.LoadEventLogViewsForTest(httptest.NewRecorder(), *wh)
|
||||
require.Len(t, views, 1)
|
||||
require.Len(t, views[0].Deliveries, 1)
|
||||
|
||||
@@ -489,9 +487,7 @@ func TestHandleSourceLogs_BoundsOversizeResponse(t *testing.T) {
|
||||
stored := strings.Repeat("A", responseCap*4) + tail
|
||||
seedFailedDeliveryWithResponse(t, dbMgr, wh.ID, tgt.ID, stored)
|
||||
|
||||
views := h.LoadEventLogViewsForTest(
|
||||
httptest.NewRecorder(), *wh, 1,
|
||||
)
|
||||
views := h.LoadEventLogViewsForTest(httptest.NewRecorder(), *wh)
|
||||
require.Len(t, views, 1)
|
||||
require.Len(t, views[0].Deliveries, 1)
|
||||
require.Len(t, views[0].Deliveries[0].Results, 1)
|
||||
|
||||
@@ -75,9 +75,7 @@ func seedAndProject(
|
||||
wh := seedWebhook(t, db)
|
||||
seedEventWithBody(t, dbMgr, wh.ID, body)
|
||||
|
||||
views := h.LoadEventLogViewsForTest(
|
||||
httptest.NewRecorder(), *wh, 1,
|
||||
)
|
||||
views := h.LoadEventLogViewsForTest(httptest.NewRecorder(), *wh)
|
||||
require.Len(t, views, 1)
|
||||
|
||||
return views[0]
|
||||
|
||||
@@ -51,12 +51,6 @@ const (
|
||||
SidecarLeftMsgForTest = sidecarLeftMsg
|
||||
)
|
||||
|
||||
// PageOrFirstForTest exposes pageOrFirst for use in the handlers_test
|
||||
// package.
|
||||
func PageOrFirstForTest(s string) int {
|
||||
return pageOrFirst(s)
|
||||
}
|
||||
|
||||
// DummyVerificationsForTest reports how many equivalent-cost
|
||||
// verifications were charged for usernames that do not exist. It
|
||||
// lets a test prove the anti-enumeration path ran without timing
|
||||
@@ -79,10 +73,9 @@ func TrimPartialRuneForTest(b []byte) []byte {
|
||||
func (s *Handlers) LoadEventLogViewsForTest(
|
||||
w http.ResponseWriter,
|
||||
webhook database.Webhook,
|
||||
page int,
|
||||
) []EventLogView {
|
||||
views, _, _ := s.loadEventsWithDeliveries(
|
||||
w, newRequestForTest(), webhook, nil, page,
|
||||
w, newRequestForTest(), webhook, nil,
|
||||
)
|
||||
|
||||
return views
|
||||
|
||||
@@ -30,10 +30,9 @@ import (
|
||||
const (
|
||||
// maxBodyShift is the bit shift for 1 MB body limit.
|
||||
maxBodyShift = 20
|
||||
// recentEventLimit is the number of recent events to show.
|
||||
// recentEventLimit is the number of most recent events that a
|
||||
// webhook's page and its event log show.
|
||||
recentEventLimit = 50
|
||||
// paginationPerPage is the number of items per page.
|
||||
paginationPerPage = 25
|
||||
|
||||
// tmplKeyError is the template data key for an error message.
|
||||
tmplKeyError = "Error"
|
||||
|
||||
@@ -57,7 +57,11 @@ func TestEveryPageRendersItsOwnTitle(t *testing.T) {
|
||||
},
|
||||
{
|
||||
"source_logs.html",
|
||||
map[string]any{dataKeyWebhook: webhook, "TotalEvents": int64(0)},
|
||||
map[string]any{
|
||||
dataKeyWebhook: webhook,
|
||||
dataKeyEvents: []handlers.EventLogView{},
|
||||
"TotalEvents": int64(0),
|
||||
},
|
||||
"Full Event Log - orders - Webhooker",
|
||||
},
|
||||
{
|
||||
|
||||
@@ -2,9 +2,12 @@ package handlers_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi"
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -153,3 +156,55 @@ func TestHandleSourceLogs_MasksSlackWebhookURL(t *testing.T) {
|
||||
assert.Contains(t, body, tgt.Name)
|
||||
assert.Contains(t, body, "delivered")
|
||||
}
|
||||
|
||||
// TestHandleSourceLogs_ShowsFiftyNewestEvents proves the event log
|
||||
// holds the 50 newest events, newest first, and not one more, and says
|
||||
// how many events there are in all.
|
||||
func TestHandleSourceLogs_ShowsFiftyNewestEvents(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
f := newRecentEventsFixture(t)
|
||||
base := time.Now().Add(-time.Hour)
|
||||
|
||||
for i := range 51 {
|
||||
f.event(
|
||||
t, fmt.Sprintf("application/x-log-%02d", i), "{}",
|
||||
base.Add(time.Duration(i)*time.Second),
|
||||
)
|
||||
}
|
||||
|
||||
body := renderSourceLogsPage(t, f.h, f.sess, f.webhook.ID)
|
||||
|
||||
assert.Equal(t, 50, strings.Count(body, `role="button"`))
|
||||
assert.NotContains(t, body, "application/x-log-00")
|
||||
assert.Contains(t, body, "application/x-log-01")
|
||||
assert.Less(
|
||||
t,
|
||||
strings.Index(body, "application/x-log-50"),
|
||||
strings.Index(body, "application/x-log-49"),
|
||||
)
|
||||
assert.Contains(t, body, "50 most recent of 51 events")
|
||||
}
|
||||
|
||||
// TestHandleSourceLogs_OnlyNewestStartsExpanded proves that of the
|
||||
// events in the log only the newest starts expanded.
|
||||
func TestHandleSourceLogs_OnlyNewestStartsExpanded(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
f := newRecentEventsFixture(t)
|
||||
now := time.Now()
|
||||
|
||||
f.event(t, "application/x-older", "{}", now.Add(-time.Minute))
|
||||
f.event(t, "application/x-newer", "{}", now)
|
||||
|
||||
body := renderSourceLogsPage(t, f.h, f.sess, f.webhook.ID)
|
||||
|
||||
assert.Equal(t, 1, strings.Count(body, " data-open>"))
|
||||
|
||||
open := strings.Index(body, " data-open>")
|
||||
newer := strings.Index(body, "application/x-newer")
|
||||
older := strings.Index(body, "application/x-older")
|
||||
|
||||
assert.Less(t, open, newer, "the newest event is not the open one")
|
||||
assert.Less(t, newer, older)
|
||||
}
|
||||
|
||||
@@ -1112,30 +1112,17 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
page := h.parsePage(r)
|
||||
|
||||
evts, total, ok := h.loadEventsWithDeliveries(
|
||||
w, r, webhook, targets, page,
|
||||
w, r, webhook, targets,
|
||||
)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
|
||||
totalPages := int(total) / paginationPerPage
|
||||
if int(total)%paginationPerPage != 0 {
|
||||
totalPages++
|
||||
}
|
||||
|
||||
data := map[string]any{
|
||||
tmplKeyWebhook: &webhook,
|
||||
"Events": evts,
|
||||
"Page": page,
|
||||
"TotalPages": totalPages,
|
||||
"TotalEvents": total,
|
||||
"HasPrev": page > 1,
|
||||
"HasNext": page < totalPages,
|
||||
"PrevPage": page - 1,
|
||||
"NextPage": page + 1,
|
||||
}
|
||||
|
||||
h.renderTemplate(w, r, "source_logs.html", data)
|
||||
@@ -1195,15 +1182,11 @@ func (h *Handlers) loadTargetMap(
|
||||
return targetMap, nil
|
||||
}
|
||||
|
||||
// parsePage extracts a page number from the query string.
|
||||
func (h *Handlers) parsePage(r *http.Request) int {
|
||||
return pageOrFirst(r.URL.Query().Get("page"))
|
||||
}
|
||||
|
||||
// loadEventsWithDeliveries loads paginated events and their
|
||||
// deliveries from the per-webhook database. Events come back
|
||||
// as capped projections rather than database.Event rows: see
|
||||
// eventLogColumns for why the cut happens in SQL.
|
||||
// loadEventsWithDeliveries loads the recentEventLimit newest events
|
||||
// and their deliveries from the per-webhook database, and the total
|
||||
// number of events stored. Events come back as capped projections
|
||||
// rather than database.Event rows: see eventLogColumns for why the
|
||||
// cut happens in SQL.
|
||||
//
|
||||
// The bool reports whether the load succeeded. It is false
|
||||
// once this has answered the request with an error, and the
|
||||
@@ -1213,7 +1196,6 @@ func (h *Handlers) loadEventsWithDeliveries(
|
||||
r *http.Request,
|
||||
webhook database.Webhook,
|
||||
targetMap map[string]eventLogTarget,
|
||||
page int,
|
||||
) ([]EventLogView, int64, bool) {
|
||||
if !h.dbMgr.DBExists(webhook.ID) {
|
||||
return nil, 0, true
|
||||
@@ -1228,9 +1210,7 @@ func (h *Handlers) loadEventsWithDeliveries(
|
||||
return nil, 0, false
|
||||
}
|
||||
|
||||
rows, totalEvents := loadEventLogRows(
|
||||
webhookDB, webhook.ID, page,
|
||||
)
|
||||
rows, totalEvents := loadEventLogRows(webhookDB, webhook.ID)
|
||||
|
||||
result, ok := h.eventLogViews(
|
||||
w, r, webhookDB, webhook.ID, rows, targetMap,
|
||||
@@ -1303,10 +1283,11 @@ func (h *Handlers) eventLogViews(
|
||||
return result, true
|
||||
}
|
||||
|
||||
// loadEventLogRows reads one page of the event log projection, newest
|
||||
// first, and the total number of events the pager counts against.
|
||||
// loadEventLogRows reads the event log projection of the
|
||||
// recentEventLimit newest events, newest first, and the total number
|
||||
// of events stored.
|
||||
func loadEventLogRows(
|
||||
webhookDB *gorm.DB, webhookID string, page int,
|
||||
webhookDB *gorm.DB, webhookID string,
|
||||
) ([]eventLogRow, int64) {
|
||||
var totalEvents int64
|
||||
|
||||
@@ -1320,9 +1301,7 @@ func loadEventLogRows(
|
||||
eventLogColumns, maxRenderedBodyBytes,
|
||||
).Where(
|
||||
"webhook_id = ?", webhookID,
|
||||
).Order("created_at DESC").Offset(
|
||||
(page - 1) * paginationPerPage,
|
||||
).Limit(paginationPerPage).Find(&rows)
|
||||
).Order("created_at DESC").Limit(recentEventLimit).Find(&rows)
|
||||
|
||||
return rows, totalEvents
|
||||
}
|
||||
@@ -1331,9 +1310,9 @@ func loadEventLogRows(
|
||||
// events have been resubmitted from it.
|
||||
//
|
||||
// One grouped query covers the page rather than one query per event.
|
||||
// A page holds paginationPerPage ids, far below SQLite's bound
|
||||
// parameter ceiling, so it needs no chunking as the delivery result
|
||||
// load does.
|
||||
// The page shows at most recentEventLimit events, far below SQLite's
|
||||
// bound parameter ceiling, so it needs no chunking as the delivery
|
||||
// result load does.
|
||||
func resubmitCounts(
|
||||
webhookDB *gorm.DB, eventIDs []string,
|
||||
) (map[string]int, error) {
|
||||
@@ -1759,24 +1738,6 @@ func (h *Handlers) setTargetFromForm(
|
||||
return "", nil
|
||||
}
|
||||
|
||||
// pageOrFirst parses a paginated page number, answering 1 for
|
||||
// anything empty, unparseable or out of range.
|
||||
//
|
||||
// Falling back rather than rejecting is correct here and only here:
|
||||
// a page number is where to send the browser next, not configuration
|
||||
// the operator is storing, and the actions that submit one have
|
||||
// already completed by the time it is read — answering 400 would
|
||||
// report a failure that did not happen. Anything an operator SETS
|
||||
// must be validated instead; see parseMaxRetries.
|
||||
func pageOrFirst(s string) int {
|
||||
v, err := strconv.Atoi(strings.TrimSpace(s))
|
||||
if err != nil || v < 1 {
|
||||
return 1
|
||||
}
|
||||
|
||||
return v
|
||||
}
|
||||
|
||||
// targetFormInput carries the raw values of a target form. Both the
|
||||
// create and the edit path fill one and hand it to setTargetFromForm,
|
||||
// so neither can come to validate a target differently from the
|
||||
|
||||
@@ -383,20 +383,3 @@ func TestTargetRetries_CreateAndEditAgreeOnEveryCase(t *testing.T) {
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestPageOrFirst_CoercesRatherThanRejects pins the one place a
|
||||
// non-numeric form value legitimately falls back. A page number says
|
||||
// where to send the browser after an action that has already
|
||||
// happened, so it is not configuration and rejecting it would report
|
||||
// a failure that did not occur.
|
||||
func TestPageOrFirst_CoercesRatherThanRejects(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, s := range []string{"", "abc", "0", "-1", "2.7", " "} {
|
||||
assert.Equal(t, 1, handlers.PageOrFirstForTest(s),
|
||||
"%q should fall back to the first page", s)
|
||||
}
|
||||
|
||||
assert.Equal(t, 4, handlers.PageOrFirstForTest("4"))
|
||||
assert.Equal(t, 4, handlers.PageOrFirstForTest(" 4 "))
|
||||
}
|
||||
|
||||
@@ -21,6 +21,7 @@ import (
|
||||
const (
|
||||
dataKeyWebhook = "Webhook"
|
||||
dataKeyError = "Error"
|
||||
dataKeyEvents = "Events"
|
||||
)
|
||||
|
||||
// testWebhookID is the identifier given to the webhook under test on
|
||||
@@ -144,9 +145,10 @@ func TestEventLogPageIsCalledFullEventLog(t *testing.T) {
|
||||
t.Cleanup(app.RequireStop)
|
||||
|
||||
// A pointer, as in the handlers: source_detail.html calls
|
||||
// Webhook.RetentionLabel, a pointer method. Both pages only range
|
||||
// over their lists, and a list left out renders as empty, so the
|
||||
// lists are left out.
|
||||
// Webhook.RetentionLabel, a pointer method. The webhook page only
|
||||
// ranges over its lists, and a list left out renders as empty, so
|
||||
// its lists are left out. The event log also counts its events, so
|
||||
// it gets an empty list.
|
||||
webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
|
||||
webhook.ID = testWebhookID
|
||||
|
||||
@@ -169,6 +171,7 @@ func TestEventLogPageIsCalledFullEventLog(t *testing.T) {
|
||||
|
||||
logBody := renderPage(t, h, sess, "source_logs.html", map[string]any{
|
||||
dataKeyWebhook: webhook,
|
||||
dataKeyEvents: []handlers.EventLogView{},
|
||||
"TotalEvents": int64(0),
|
||||
})
|
||||
|
||||
@@ -317,9 +320,9 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
|
||||
"Entrypoints": handlers.NewEntrypointViews(
|
||||
[]database.Entrypoint{entrypoint},
|
||||
),
|
||||
"Targets": delivery.NewTargetViews(nil),
|
||||
"Events": []database.Event{},
|
||||
"BaseURL": "https://hooks.example.com",
|
||||
"Targets": delivery.NewTargetViews(nil),
|
||||
dataKeyEvents: []database.Event{},
|
||||
"BaseURL": "https://hooks.example.com",
|
||||
})
|
||||
|
||||
assert.Contains(
|
||||
@@ -384,9 +387,9 @@ func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
|
||||
"Entrypoints": handlers.NewEntrypointViews(
|
||||
[]database.Entrypoint{entrypoint},
|
||||
),
|
||||
"Targets": delivery.NewTargetViews(nil),
|
||||
"Events": []database.Event{},
|
||||
"BaseURL": "https://hooks.example.com",
|
||||
"Targets": delivery.NewTargetViews(nil),
|
||||
dataKeyEvents: []database.Event{},
|
||||
"BaseURL": "https://hooks.example.com",
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user