Files
multica/server/internal/handler/activity_test.go
Bohan Jiang 3f20999597 refactor(timeline): drop server-side comment + timeline pagination (#2322)
* refactor(timeline): drop server-side comment + timeline pagination (MUL-1929)

The cursor-paginated /timeline and /comments endpoints were sized for a
problem the data shape doesn't have: prod p99 is ~30 comments per issue
and the all-time max is ~1.1k. Time-based pagination also splits reply
threads across page boundaries (orphan replies), which the frontend was
papering over with an "orphan rescue" that promoted disconnected replies
to top-level — confusing UX with no real benefit.

Replace both endpoints with a single full-issue fetch, capped server-side
at 2000 rows as a defensive safety net (never hit in practice).

Server
- /api/issues/:id/timeline now returns a flat ASC TimelineEntry[]
  (matches the legacy desktop contract — older Multica.app builds keep
  working because the wrapped TimelineResponse + cursors are gone, and
  the raw array shape was always what they consumed).
- /api/issues/:id/comments drops limit/offset; only ?since is honoured
  for the CLI agent-polling flow.
- Drop ListCommentsBefore/After/Latest, ListActivitiesBefore/After/Latest
  and the timelineCursor encoding.
- Replace with ListCommentsForIssue / ListCommentsSinceForIssue /
  ListActivitiesForIssue (capped by argument).

CLI
- multica issue comment list drops --limit / --offset and the X-Total-Count
  reporting; --since is preserved for incremental polling.

Frontend
- Replace useInfiniteQuery with useQuery in useIssueTimeline; drop
  fetchOlder/Newer, jumpToLatest, isAtLatest, newEntriesBelowCount.
- Remove timeline-cache helpers (mapAllEntries / filterAllEntries /
  prependToLatestPage) and the TimelinePage / TimelinePageParam types.
- WS event handlers update the single flat-array cache directly.
- Drop the orphan-reply rescue in issue-detail — every reply's parent
  is now guaranteed to be in the same array.
- Strip the "show older / show newer / jump to latest" buttons and their
  i18n strings.

Co-authored-by: multica-agent <github@multica.ai>

* fix(timeline): address review feedback on pagination removal

Three issues caught in PR #2322 review:

1. /timeline broke for stale clients between #2128 and this PR. They send
   ?limit/?before/?after/?around and parse with the wrapped TimelinePageSchema;
   the new flat-array response was failing schema validation and falling back
   to an empty timeline. Restore the wrapped shape on those query params
   (DESC entries, null cursors, has_more_*=false), keeping the flat ASC array
   for bare requests. Around-mode now also fills target_index from the merged
   slice so legacy clients can still scroll-to-anchor without a follow-up.

2. The agent prompts in runtime_config.go and prompt.go still told agents
   that `multica issue comment list` accepts --limit/--offset and to use
   `--limit 30` on truncated output. With those flags removed in this PR,
   new agent runs would hit "unknown flag" or skip context. Update the
   prompt copy to "returns all comments, capped at 2000; --since for
   incremental polling".

3. useCreateComment's onSuccess was a bare append to the timeline cache
   with no id-dedupe, so a fast comment:created WS event firing before
   onSuccess produced a transient duplicate. Restore the id guard the old
   prependToLatestPage helper used to provide.

Adds two new boundary tests:
- TestListTimeline_LegacyWrappedShape_OnPaginationParams
- TestListTimeline_LegacyWrappedShape_AroundFillsTargetIndex

Co-authored-by: multica-agent <github@multica.ai>

* test(handler): fix timeline test assertions for handler-package isolation

The TestListTimeline_* assertions assumed CreateIssue would seed an
"issue_created" activity_log row, but the activity listener that publishes
those rows is registered in cmd/server/main.go — handler-package tests
don't wire it up. CI saw 5 entries (3 comments + 2 activities) where the
test expected ≥6.

Drop the auto-activity assumption: assert exactly 5 entries in
TestListTimeline_MergesCommentsAndActivities, and tighten
TestListTimeline_EmptyIssue to assert a fully-empty timeline.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: multica-agent <github@multica.ai>
2026-05-09 16:11:58 +08:00

222 lines
7.9 KiB
Go

package handler
import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"testing"
"time"
)
// fetchTimeline issues a GET /timeline request and returns the decoded entries
// + HTTP status. The endpoint returns a flat array of TimelineEntry sorted by
// (created_at, id) ascending (oldest first); see ListTimeline / #1929.
func fetchTimeline(t *testing.T, issueID string) ([]TimelineEntry, int) {
t.Helper()
w := httptest.NewRecorder()
req := newRequest("GET", "/api/issues/"+issueID+"/timeline", nil)
req = withURLParam(req, "id", issueID)
testHandler.ListTimeline(w, req)
var entries []TimelineEntry
if w.Code == http.StatusOK {
json.NewDecoder(w.Body).Decode(&entries)
}
return entries, w.Code
}
// createIssueForTimeline returns a freshly-created issue id and registers a
// cleanup so its timeline rows are deleted after the test.
func createIssueForTimeline(t *testing.T, title string) string {
t.Helper()
w := httptest.NewRecorder()
req := newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{
"title": title,
"status": "todo",
})
testHandler.CreateIssue(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("CreateIssue: expected 201, got %d: %s", w.Code, w.Body.String())
}
var issue IssueResponse
json.NewDecoder(w.Body).Decode(&issue)
t.Cleanup(func() {
ctx := context.Background()
testPool.Exec(ctx, `DELETE FROM activity_log WHERE issue_id = $1`, issue.ID)
testPool.Exec(ctx, `DELETE FROM comment WHERE issue_id = $1`, issue.ID)
testPool.Exec(ctx, `DELETE FROM issue WHERE id = $1`, issue.ID)
})
return issue.ID
}
// seedTimelineEntries inserts <commentN> comments + <activityN> activities for
// the given issue with ascending timestamps. Returns the inserted ids in the
// order they were inserted (chronologically ascending).
func seedTimelineEntries(t *testing.T, issueID string, commentN, activityN int) (commentIDs, activityIDs []string) {
t.Helper()
ctx := context.Background()
base := time.Now().UTC().Add(-time.Duration(commentN+activityN) * time.Minute)
for i := 0; i < commentN; i++ {
var id string
ts := base.Add(time.Duration(i) * time.Minute)
if err := testPool.QueryRow(ctx, `
INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, created_at, updated_at)
VALUES ($1, $2, 'member', $3, $4, 'comment', $5, $5)
RETURNING id
`, issueID, testWorkspaceID, testUserID, fmt.Sprintf("comment %d", i), ts).Scan(&id); err != nil {
t.Fatalf("seed comment %d: %v", i, err)
}
commentIDs = append(commentIDs, id)
}
for i := 0; i < activityN; i++ {
var id string
ts := base.Add(time.Duration(commentN+i) * time.Minute)
if err := testPool.QueryRow(ctx, `
INSERT INTO activity_log (workspace_id, issue_id, actor_type, actor_id, action, details, created_at)
VALUES ($1, $2, 'member', $3, 'status_changed', '{"from":"todo","to":"in_progress"}'::jsonb, $4)
RETURNING id
`, testWorkspaceID, issueID, testUserID, ts).Scan(&id); err != nil {
t.Fatalf("seed activity %d: %v", i, err)
}
activityIDs = append(activityIDs, id)
}
return
}
func TestListTimeline_ReturnsAllEntriesAscending(t *testing.T) {
issueID := createIssueForTimeline(t, "All entries test")
commentIDs, _ := seedTimelineEntries(t, issueID, 5, 0)
entries, status := fetchTimeline(t, issueID)
if status != http.StatusOK {
t.Fatalf("status = %d, want 200", status)
}
// Handler tests don't register the activity listener (that lives in
// cmd/server), so issue creation does not seed an auto-activity here.
// We assert directly on the seeded comments.
commentEntries := []TimelineEntry{}
for _, e := range entries {
if e.Type == "comment" {
commentEntries = append(commentEntries, e)
}
}
if got, want := len(commentEntries), len(commentIDs); got != want {
t.Fatalf("comment count = %d, want %d", got, want)
}
for i, e := range commentEntries {
if e.ID != commentIDs[i] {
t.Errorf("entry %d: id = %s, want %s", i, e.ID, commentIDs[i])
}
}
}
func TestListTimeline_MergesCommentsAndActivities(t *testing.T) {
issueID := createIssueForTimeline(t, "Merged entries test")
seedTimelineEntries(t, issueID, 3, 2)
entries, status := fetchTimeline(t, issueID)
if status != http.StatusOK {
t.Fatalf("status = %d, want 200", status)
}
// Verify chronological non-decreasing order across types.
for i := 1; i < len(entries); i++ {
if entries[i-1].CreatedAt > entries[i].CreatedAt {
t.Errorf("not chronological at %d: %q then %q",
i, entries[i-1].CreatedAt, entries[i].CreatedAt)
}
}
// 3 seeded comments + 2 seeded activities = 5. Handler tests don't
// register the activity listener, so there is no auto issue-created row.
if got, want := len(entries), 5; got != want {
t.Fatalf("entries = %d, want %d", got, want)
}
}
// fetchTimelineWrapped exercises the legacy wrapped response shape that
// stale Multica.app v0.2.26+ builds still expect — sending any of
// limit/before/after/around makes the server emit a TimelinePage-style
// object (entries DESC, null cursors, has_more_*=false) instead of the new
// flat array. Used to verify the boundary-compat path doesn't regress.
func fetchTimelineWrapped(t *testing.T, issueID, query string) (timelinePaginatedResponse, int) {
t.Helper()
w := httptest.NewRecorder()
req := newRequest("GET", "/api/issues/"+issueID+"/timeline?"+query, nil)
req = withURLParam(req, "id", issueID)
testHandler.ListTimeline(w, req)
var resp timelinePaginatedResponse
if w.Code == http.StatusOK {
json.NewDecoder(w.Body).Decode(&resp)
}
return resp, w.Code
}
// Boundary-compat: a stale client between #2128 and #1929 sends ?limit=50
// and parses the response with TimelinePageSchema. The handler must keep
// returning the wrapped object so that path doesn't fall back to an empty
// timeline.
func TestListTimeline_LegacyWrappedShape_OnPaginationParams(t *testing.T) {
issueID := createIssueForTimeline(t, "Legacy wrapped shape test")
commentIDs, _ := seedTimelineEntries(t, issueID, 3, 0)
resp, status := fetchTimelineWrapped(t, issueID, "limit=50")
if status != http.StatusOK {
t.Fatalf("status = %d, want 200", status)
}
if resp.HasMoreBefore || resp.HasMoreAfter {
t.Errorf("has_more_*: want false/false, got before=%v after=%v",
resp.HasMoreBefore, resp.HasMoreAfter)
}
if resp.NextCursor != nil || resp.PrevCursor != nil {
t.Errorf("cursors: want nil/nil, got next=%v prev=%v", resp.NextCursor, resp.PrevCursor)
}
// DESC order: most recent comment first; activity from issue-creation
// sits at the bottom.
commentEntries := []TimelineEntry{}
for _, e := range resp.Entries {
if e.Type == "comment" {
commentEntries = append(commentEntries, e)
}
}
if got, want := len(commentEntries), len(commentIDs); got != want {
t.Fatalf("comment count = %d, want %d", got, want)
}
for i, e := range commentEntries {
want := commentIDs[len(commentIDs)-1-i]
if e.ID != want {
t.Errorf("DESC entry %d: id = %s, want %s", i, e.ID, want)
}
}
}
func TestListTimeline_LegacyWrappedShape_AroundFillsTargetIndex(t *testing.T) {
issueID := createIssueForTimeline(t, "Around target index test")
commentIDs, _ := seedTimelineEntries(t, issueID, 5, 0)
anchor := commentIDs[2] // pick a middle comment
resp, status := fetchTimelineWrapped(t, issueID, "around="+anchor)
if status != http.StatusOK {
t.Fatalf("status = %d, want 200", status)
}
if resp.TargetIndex == nil {
t.Fatalf("target_index: want non-nil for around mode")
}
if got := resp.Entries[*resp.TargetIndex].ID; got != anchor {
t.Errorf("target_index points at %s, want anchor %s", got, anchor)
}
}
func TestListTimeline_EmptyIssue(t *testing.T) {
issueID := createIssueForTimeline(t, "Empty timeline test")
entries, status := fetchTimeline(t, issueID)
if status != http.StatusOK {
t.Fatalf("status = %d, want 200", status)
}
// Handler tests don't wire the activity listener, so a freshly-created
// issue with no comments has an empty timeline.
if got := len(entries); got != 0 {
t.Fatalf("entries = %d, want 0", got)
}
}