mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-11 13:28:57 +00:00
feat(report): opt-in 'what shipped' completed-items list (TASK-1641) (#651)
* feat(report): opt-in 'what shipped' completed-items list (TASK-1641)
Add ?include_items=true → completed_items[{ref,title,collection,completed_at}]
on the report: items that reached a positive terminal in the window, deduped
by item (newest completion first), capped at 500 with
completed_items_overflow_count. Same positive-terminal source as
totals.completed (joins live items, deleted_at IS NULL), so the list reconciles
with the count. Opt-in so the interactive dashboard stays count-only; the
print/export report (TASK-1642) requests it.
Web ReportData gains completed_items + the api.report.get includeItems flag.
Parent: PLAN-1628.
* fix(report): scope completed-items list to the item's current visible collection per Codex review (round 1)
The list scoped transitions by st.collection_id (visible at completion) but
returned the item's CURRENT title/ref/collection — so an item completed while
visible then moved to a hidden collection could leak its hidden collection
slug/prefix + current title to a restricted caller. Require i.collection_id to
be in the resolved (scoped) collection set on both the count and list queries.
Adds a move-to-hidden-collection visibility test.
This commit is contained in:
@@ -38,6 +38,10 @@ func (s *Server) handleGetReport(w http.ResponseWriter, r *http.Request) {
|
||||
if n, err := strconv.Atoi(strings.TrimSpace(r.URL.Query().Get("offset"))); err == nil && n > 0 {
|
||||
opts.Offset = n
|
||||
}
|
||||
// include_items adds the "what shipped" completed-items list (opt-in).
|
||||
if v := r.URL.Query().Get("include_items"); v == "true" || v == "1" {
|
||||
opts.IncludeItems = true
|
||||
}
|
||||
if raw := strings.TrimSpace(r.URL.Query().Get("collections")); raw != "" {
|
||||
for _, slug := range strings.Split(raw, ",") {
|
||||
if s := strings.TrimSpace(slug); s != "" {
|
||||
|
||||
@@ -55,6 +55,10 @@ type ReportOptions struct {
|
||||
// Offset steps the window back by whole window-lengths (0 = current period,
|
||||
// 1 = the period immediately before, …). Negative is clamped to 0.
|
||||
Offset int
|
||||
// IncludeItems, when true, populates CompletedItems (the "what shipped"
|
||||
// list) — opt-in because the interactive dashboard only needs counts; the
|
||||
// print/export report requests it.
|
||||
IncludeItems bool
|
||||
// ScopeToVisible, when true, restricts the report to VisibleCollectionIDs
|
||||
// (the caller's visible collection set). Aggregate counts for collections
|
||||
// the caller can't see are never computed — preventing a member/guest from
|
||||
@@ -66,6 +70,11 @@ type ReportOptions struct {
|
||||
VisibleCollectionIDs []string
|
||||
}
|
||||
|
||||
// completedItemsCap bounds the "what shipped" list so a pathological window
|
||||
// can't produce an unbounded payload; the rest is reported via the overflow
|
||||
// count.
|
||||
const completedItemsCap = 500
|
||||
|
||||
// ReportBucket is one time-series point: items created and completed within
|
||||
// the bucket. Bucket is a sortable UTC label ("YYYY-MM-DD", or
|
||||
// "YYYY-MM-DDTHH" for the hourly window).
|
||||
@@ -128,6 +137,15 @@ type ReportWIP struct {
|
||||
ByCollection []ReportDuration `json:"by_collection"` // MedianHours = median open-item age
|
||||
}
|
||||
|
||||
// ReportCompletedItem is one item that reached a positive terminal within the
|
||||
// window — the "what shipped" list for the exec report (TASK-1641).
|
||||
type ReportCompletedItem struct {
|
||||
Ref string `json:"ref"` // e.g. TASK-5
|
||||
Title string `json:"title"`
|
||||
Collection string `json:"collection"` // slug
|
||||
CompletedAt string `json:"completed_at"` // RFC3339, latest completion in-window
|
||||
}
|
||||
|
||||
// ReportData is the full report response. This is the stable contract the
|
||||
// Reports UI (TASK-1633), CLI/MCP (TASK-1635), and charts (TASK-1632) consume.
|
||||
type ReportData struct {
|
||||
@@ -143,6 +161,12 @@ type ReportData struct {
|
||||
StatusDistribution []ReportStatusCount `json:"status_distribution"`
|
||||
CycleTime ReportCycleTime `json:"cycle_time"`
|
||||
WIP ReportWIP `json:"wip"`
|
||||
// CompletedItems is the "what shipped" list, populated only when
|
||||
// IncludeItems was requested (nil/omitted otherwise). Deduped by item,
|
||||
// newest completion first, capped at completedItemsCap with the remainder
|
||||
// reported in CompletedItemsOverflowCount.
|
||||
CompletedItems []ReportCompletedItem `json:"completed_items,omitempty"`
|
||||
CompletedItemsOverflow int `json:"completed_items_overflow_count,omitempty"`
|
||||
}
|
||||
|
||||
// windowSpec maps a window to its lookback duration and bucket granularity.
|
||||
@@ -296,9 +320,104 @@ func (s *Store) GetReport(workspaceID string, opts ReportOptions) (*ReportData,
|
||||
data.WIP = wip
|
||||
}
|
||||
|
||||
if opts.IncludeItems {
|
||||
items, overflow, ierr := s.reportCompletedItems(workspaceID, colls, startStr, endStr)
|
||||
if ierr != nil {
|
||||
return nil, ierr
|
||||
}
|
||||
data.CompletedItems = items
|
||||
data.CompletedItemsOverflow = overflow
|
||||
}
|
||||
|
||||
return data, nil
|
||||
}
|
||||
|
||||
// reportCompletedItems returns the items that reached a positive terminal in
|
||||
// the window (the "what shipped" list, TASK-1641): deduped by item (newest
|
||||
// completion first), capped at completedItemsCap with the remainder in the
|
||||
// returned overflow count. Same positive-terminal source as totals.completed,
|
||||
// so the list reconciles with the count (modulo the dedupe + cap). Joins live
|
||||
// items (deleted_at IS NULL), consistent with the completed counts.
|
||||
func (s *Store) reportCompletedItems(workspaceID string, colls []reportCollection, startStr, endStr string) ([]ReportCompletedItem, int, error) {
|
||||
posExpr, posArgs := s.positiveTerminalExpr(colls)
|
||||
if posExpr == "" {
|
||||
return []ReportCompletedItem{}, 0, nil
|
||||
}
|
||||
|
||||
// Restrict item-level rows to items whose CURRENT collection is in scope.
|
||||
// posExpr scopes by st.collection_id (the collection at completion time),
|
||||
// but this query returns the item's live title/ref/collection — so an item
|
||||
// completed while in a visible collection and since moved to a hidden one
|
||||
// must NOT leak its current (hidden) details to a restricted caller.
|
||||
curScope := make([]string, 0, len(colls))
|
||||
curArgs := make([]any, 0, len(colls))
|
||||
for _, c := range colls {
|
||||
curScope = append(curScope, "?")
|
||||
curArgs = append(curArgs, c.id)
|
||||
}
|
||||
curExpr := "i.collection_id IN (" + strings.Join(curScope, ",") + ")"
|
||||
|
||||
base := []any{workspaceID, startStr, endStr}
|
||||
|
||||
// Distinct completed-item count (for the overflow figure).
|
||||
var total int
|
||||
cArgs := append(append([]any{}, base...), posArgs...)
|
||||
cArgs = append(cArgs, curArgs...)
|
||||
if err := s.db.QueryRow(s.q(fmt.Sprintf(`
|
||||
SELECT COUNT(DISTINCT st.item_id)
|
||||
FROM status_transitions st
|
||||
JOIN items i ON i.id = st.item_id AND i.deleted_at IS NULL
|
||||
WHERE st.workspace_id = ? AND st.created_at >= ? AND st.created_at <= ?
|
||||
AND %s AND %s
|
||||
`, posExpr, curExpr)), cArgs...).Scan(&total); err != nil {
|
||||
return nil, 0, fmt.Errorf("count completed items: %w", err)
|
||||
}
|
||||
|
||||
// The list: one row per item (latest in-window completion), newest first.
|
||||
lArgs := append(append([]any{}, base...), posArgs...)
|
||||
lArgs = append(lArgs, curArgs...)
|
||||
lArgs = append(lArgs, completedItemsCap)
|
||||
rows, err := s.db.Query(s.q(fmt.Sprintf(`
|
||||
SELECT c.prefix, i.item_number, i.title, c.slug, MAX(st.created_at) AS completed_at
|
||||
FROM status_transitions st
|
||||
JOIN items i ON i.id = st.item_id AND i.deleted_at IS NULL
|
||||
JOIN collections c ON c.id = i.collection_id
|
||||
WHERE st.workspace_id = ? AND st.created_at >= ? AND st.created_at <= ?
|
||||
AND %s AND %s
|
||||
GROUP BY i.id, c.prefix, i.item_number, i.title, c.slug
|
||||
ORDER BY completed_at DESC
|
||||
LIMIT ?
|
||||
`, posExpr, curExpr)), lArgs...)
|
||||
if err != nil {
|
||||
return nil, 0, fmt.Errorf("list completed items: %w", err)
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
out := []ReportCompletedItem{}
|
||||
for rows.Next() {
|
||||
var prefix, title, slug, completedAt string
|
||||
var itemNumber int
|
||||
if err := rows.Scan(&prefix, &itemNumber, &title, &slug, &completedAt); err != nil {
|
||||
return nil, 0, fmt.Errorf("scan completed item: %w", err)
|
||||
}
|
||||
out = append(out, ReportCompletedItem{
|
||||
Ref: fmt.Sprintf("%s-%d", prefix, itemNumber),
|
||||
Title: title,
|
||||
Collection: slug,
|
||||
CompletedAt: completedAt,
|
||||
})
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, 0, err
|
||||
}
|
||||
|
||||
overflow := total - len(out)
|
||||
if overflow < 0 {
|
||||
overflow = 0
|
||||
}
|
||||
return out, overflow, nil
|
||||
}
|
||||
|
||||
// reportSnapshotAsOf reconstructs the status-distribution and WIP snapshot as
|
||||
// they stood at time t, from the status-transition history (PLAN-1628 /
|
||||
// TASK-1640). An item's status as-of-t is the to_status of its latest
|
||||
|
||||
@@ -512,3 +512,100 @@ func TestReportSnapshotAsOf(t *testing.T) {
|
||||
t.Fatalf("expected the no-transition no-status item to count as open (1), got %d", wipNow.OpenCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetReport_CompletedItems(t *testing.T) {
|
||||
s := testStore(t)
|
||||
wsID, colID := newTransitionTestWorkspace(t, s)
|
||||
a := createTestItem(t, s, wsID, colID, "Ship A", "")
|
||||
b := createTestItem(t, s, wsID, colID, "Ship B", "")
|
||||
createTestItem(t, s, wsID, colID, "Still open", "") // not completed
|
||||
for _, it := range []string{a.ID, b.ID} {
|
||||
if _, err := s.UpdateItem(it, models.ItemUpdate{Fields: strPtr(`{"status":"done"}`)}); err != nil {
|
||||
t.Fatalf("complete %s: %v", it, err)
|
||||
}
|
||||
}
|
||||
// Reopen + re-complete A → it must appear ONCE (deduped by item).
|
||||
if _, err := s.UpdateItem(a.ID, models.ItemUpdate{Fields: strPtr(`{"status":"open"}`)}); err != nil {
|
||||
t.Fatalf("reopen A: %v", err)
|
||||
}
|
||||
if _, err := s.UpdateItem(a.ID, models.ItemUpdate{Fields: strPtr(`{"status":"done"}`)}); err != nil {
|
||||
t.Fatalf("re-complete A: %v", err)
|
||||
}
|
||||
|
||||
now := time.Now().UTC()
|
||||
// Opt-in: completed_items present, deduped to A + B.
|
||||
rep, err := s.GetReport(wsID, ReportOptions{Window: "week", Now: now, IncludeItems: true})
|
||||
if err != nil {
|
||||
t.Fatalf("GetReport: %v", err)
|
||||
}
|
||||
if len(rep.CompletedItems) != 2 {
|
||||
t.Fatalf("expected 2 distinct completed items (A deduped), got %d: %+v", len(rep.CompletedItems), rep.CompletedItems)
|
||||
}
|
||||
titles := map[string]bool{}
|
||||
for _, ci := range rep.CompletedItems {
|
||||
titles[ci.Title] = true
|
||||
if ci.Collection != "tasks" {
|
||||
t.Fatalf("expected collection 'tasks', got %q", ci.Collection)
|
||||
}
|
||||
if !strings.HasPrefix(ci.Ref, "TASK-") {
|
||||
t.Fatalf("expected TASK- ref, got %q", ci.Ref)
|
||||
}
|
||||
if ci.CompletedAt == "" {
|
||||
t.Fatalf("expected completed_at on %q", ci.Ref)
|
||||
}
|
||||
}
|
||||
if !titles["Ship A"] || !titles["Ship B"] {
|
||||
t.Fatalf("missing expected titles: %+v", titles)
|
||||
}
|
||||
|
||||
// Not requested: completed_items omitted (nil).
|
||||
rep2, err := s.GetReport(wsID, ReportOptions{Window: "week", Now: now})
|
||||
if err != nil {
|
||||
t.Fatalf("GetReport (no items): %v", err)
|
||||
}
|
||||
if rep2.CompletedItems != nil {
|
||||
t.Fatalf("completed_items should be nil when not requested, got %+v", rep2.CompletedItems)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetReport_CompletedItemsRespectsCurrentCollectionVisibility(t *testing.T) {
|
||||
s := testStore(t)
|
||||
u, _ := s.CreateUser(models.UserCreate{Name: "V", Email: "v@example.com"})
|
||||
ws, _ := s.CreateWorkspace(models.WorkspaceCreate{Name: "Vis", Slug: "vis", OwnerID: u.ID})
|
||||
visible := createTestCollection(t, s, ws.ID, "Visible")
|
||||
secret := createTestCollection(t, s, ws.ID, "Secret")
|
||||
|
||||
item := createTestItem(t, s, ws.ID, visible.ID, "Mover", "")
|
||||
// Complete it while in the visible collection (transition stamped visible).
|
||||
if _, err := s.UpdateItem(item.ID, models.ItemUpdate{Fields: strPtr(`{"status":"done"}`)}); err != nil {
|
||||
t.Fatalf("complete: %v", err)
|
||||
}
|
||||
// Move it (status-preserving) into the hidden collection.
|
||||
if _, err := s.MoveItem(item.ID, secret.ID, `{"status":"done"}`); err != nil {
|
||||
t.Fatalf("move: %v", err)
|
||||
}
|
||||
|
||||
now := time.Now().UTC()
|
||||
// Scoped to the visible collection only: the item now lives in the hidden
|
||||
// collection, so it must NOT appear in completed_items (no current title /
|
||||
// hidden-collection-slug leak), even though it completed while visible.
|
||||
scoped, err := s.GetReport(ws.ID, ReportOptions{
|
||||
Window: "week", Now: now, IncludeItems: true,
|
||||
ScopeToVisible: true, VisibleCollectionIDs: []string{visible.ID},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("scoped: %v", err)
|
||||
}
|
||||
if len(scoped.CompletedItems) != 0 {
|
||||
t.Fatalf("item moved to a hidden collection must not leak into completed_items, got %+v", scoped.CompletedItems)
|
||||
}
|
||||
|
||||
// Unscoped (owner/full): the item appears, attributed to its current collection.
|
||||
full, err := s.GetReport(ws.ID, ReportOptions{Window: "week", Now: now, IncludeItems: true})
|
||||
if err != nil {
|
||||
t.Fatalf("full: %v", err)
|
||||
}
|
||||
if len(full.CompletedItems) != 1 || full.CompletedItems[0].Collection != "secret" {
|
||||
t.Fatalf("unscoped should list the item under its current collection, got %+v", full.CompletedItems)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -936,11 +936,15 @@ export const api = {
|
||||
* Windowed project report. window ∈ {day, week, 2wk, month} (default
|
||||
* week). collections optionally restricts to the given slugs.
|
||||
*/
|
||||
get: (ws: string, opts?: { window?: ReportWindow; collections?: string[]; offset?: number }) => {
|
||||
get: (
|
||||
ws: string,
|
||||
opts?: { window?: ReportWindow; collections?: string[]; offset?: number; includeItems?: boolean }
|
||||
) => {
|
||||
const params = new URLSearchParams();
|
||||
if (opts?.window) params.set('window', opts.window);
|
||||
if (opts?.collections?.length) params.set('collections', opts.collections.join(','));
|
||||
if (opts?.offset && opts.offset > 0) params.set('offset', String(opts.offset));
|
||||
if (opts?.includeItems) params.set('include_items', 'true');
|
||||
const qs = params.toString();
|
||||
return request<ReportData>(`/workspaces/${ws}/report${qs ? `?${qs}` : ''}`);
|
||||
},
|
||||
|
||||
@@ -896,6 +896,17 @@ export interface ReportData {
|
||||
status_distribution: ReportStatusCount[];
|
||||
cycle_time: ReportCycleTime;
|
||||
wip: ReportWIP;
|
||||
/** "What shipped" — present only when requested via include_items. Deduped by item, newest first. */
|
||||
completed_items?: ReportCompletedItem[];
|
||||
completed_items_overflow_count?: number;
|
||||
}
|
||||
|
||||
/** One item that reached a positive terminal within the window (the "what shipped" list). */
|
||||
export interface ReportCompletedItem {
|
||||
ref: string; // e.g. TASK-5
|
||||
title: string;
|
||||
collection: string; // slug
|
||||
completed_at: string; // RFC3339
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user