diff --git a/frontend/src/client/filter-bar/compute-effective.test.ts b/frontend/src/client/filter-bar/compute-effective.test.ts new file mode 100644 index 0000000..7e0e14b --- /dev/null +++ b/frontend/src/client/filter-bar/compute-effective.test.ts @@ -0,0 +1,126 @@ +// Unit tests for the FilterBar's computeEffective() overlay. These pin +// the contract that any chip the user clicks ends up as a predicate the +// server can see — the t-paliad-283 regression had four sources picking +// up zero narrowing for /views/any because the bar's chip click didn't +// produce a non-empty `filter.predicates` for that source. +// +// Run with `bun test`. + +import { test, expect, describe } from "bun:test"; +import { computeEffective } from "./index"; +import type { FilterSpec, RenderSpec } from "../views/types"; +import type { BarState } from "./types"; + +// Mirrors paliad.user_views row {slug: "any"} — the saved Custom View +// that triggered the t-paliad-283 regression report. +const ANY_VIEW_FILTER: FilterSpec = { + version: 1, + sources: ["deadline", "appointment", "project_event", "approval_request"], + scope: { projects: { mode: "all_visible" } }, + time: { field: "auto", horizon: "past_30d" }, +}; + +const ANY_VIEW_RENDER: RenderSpec = { + shape: "list", + list: { sort: "date_asc", density: "comfortable" }, +}; + +describe("filter-bar/computeEffective — /views/any (all 4 sources)", () => { + test("empty state leaves base spec intact (no overlays)", () => { + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, {}); + expect(eff.filter.sources).toEqual([ + "deadline", "appointment", "project_event", "approval_request", + ]); + expect(eff.filter.time).toEqual({ field: "auto", horizon: "past_30d" }); + // predicates may be {} (the bar zero-fills it) but never carries a + // stray narrowing on any source — that would silently filter + // results the user never asked to filter. + for (const src of ANY_VIEW_FILTER.sources) { + expect(eff.filter.predicates?.[src]).toBeUndefined(); + } + }); + + test("deadline_status chip narrows deadline predicate", () => { + const state: BarState = { deadline_status: ["pending"] }; + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, state); + expect(eff.filter.predicates?.deadline?.status).toEqual(["pending"]); + }); + + test("appointment_type chip narrows appointment predicate", () => { + const state: BarState = { appointment_type: ["hearing"] }; + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, state); + expect(eff.filter.predicates?.appointment?.appointment_types).toEqual(["hearing"]); + }); + + test("approval_viewer_role chip narrows approval predicate", () => { + const state: BarState = { approval_viewer_role: "any_visible" }; + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, state); + expect(eff.filter.predicates?.approval_request?.viewer_role).toBe("any_visible"); + }); + + test("approval_status chip narrows approval predicate", () => { + const state: BarState = { approval_status: ["pending", "approved"] }; + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, state); + expect(eff.filter.predicates?.approval_request?.status).toEqual(["pending", "approved"]); + }); + + test("approval_entity_type chip narrows approval predicate", () => { + const state: BarState = { approval_entity_type: ["deadline"] }; + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, state); + expect(eff.filter.predicates?.approval_request?.entity_types).toEqual(["deadline"]); + }); + + test("project_event_kind chip narrows project_event predicate", () => { + const state: BarState = { project_event_kind: ["deadline_created"] }; + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, state); + expect(eff.filter.predicates?.project_event?.event_types).toEqual(["deadline_created"]); + }); + + test("time chip overrides base horizon", () => { + const state: BarState = { time: { horizon: "past_7d" } }; + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, state); + expect(eff.filter.time.horizon).toBe("past_7d"); + expect(eff.filter.time.field).toBe("auto"); // preserved from base + }); + + test("personal_only chip flips scope flag", () => { + const state: BarState = { personal_only: true }; + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, state); + expect(eff.filter.scope.personal_only).toBe(true); + }); + + test("multiple chips combine into the same effective spec", () => { + const state: BarState = { + time: { horizon: "past_7d" }, + deadline_status: ["pending"], + appointment_type: ["hearing"], + approval_status: ["pending"], + project_event_kind: ["deadline_created"], + }; + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, state); + expect(eff.filter.time.horizon).toBe("past_7d"); + expect(eff.filter.predicates?.deadline?.status).toEqual(["pending"]); + expect(eff.filter.predicates?.appointment?.appointment_types).toEqual(["hearing"]); + expect(eff.filter.predicates?.approval_request?.status).toEqual(["pending"]); + expect(eff.filter.predicates?.project_event?.event_types).toEqual(["deadline_created"]); + }); + + test("overlay does not mutate the caller's base filter", () => { + const base: FilterSpec = JSON.parse(JSON.stringify(ANY_VIEW_FILTER)); + const state: BarState = { deadline_status: ["pending"], time: { horizon: "past_7d" } }; + computeEffective(base, ANY_VIEW_RENDER, state); + // The bar deep-clones; the base must come back unchanged so a + // second click doesn't compound the previous click's overlay. + expect(base).toEqual(ANY_VIEW_FILTER); + }); + + test("inbox-only axes do not affect a /views/any spec (no inbox axis exposed)", () => { + // /views/any's axes don't include unread_only or inbox_focus, so + // those keys never appear in state. Verify that even if they did, + // the bar's overlay doesn't silently mutate sources or predicates + // in a way that would break a 4-source Custom View. + const eff = computeEffective(ANY_VIEW_FILTER, ANY_VIEW_RENDER, {}); + expect(eff.filter.sources).toHaveLength(4); + expect(eff.filter.unread_only ?? false).toBe(false); + }); +}); diff --git a/frontend/src/client/views/types.ts b/frontend/src/client/views/types.ts index 1caba1c..e8d6d8f 100644 --- a/frontend/src/client/views/types.ts +++ b/frontend/src/client/views/types.ts @@ -66,7 +66,13 @@ export interface FilterSpec { sources: DataSource[]; scope: ScopeSpec; time: TimeSpec; - predicates?: Partial>; + // Per-source narrowing. Flat shape — one entry per data source. The + // Go side (internal/services/filter_spec.go: FilterSpec.Predicates) + // mirrors this exactly; the previous Partial> spelling was a latent contract bug (t-paliad-283) + // where every chip click sent a single-nested shape the server + // unmarshalled to no-op. + predicates?: Predicates; // Inbox unread-only overlay (t-paliad-249). When true, the view // service drops project_event rows older than the caller's // users.inbox_seen_at cursor. Pending approval_requests always diff --git a/internal/services/filter_spec.go b/internal/services/filter_spec.go index ae73f1c..46ba122 100644 --- a/internal/services/filter_spec.go +++ b/internal/services/filter_spec.go @@ -51,13 +51,20 @@ const SpecVersion = 1 // can't bury an in-flight approval, per the design doc §3 carve-out). // Set by the bar's `unread_only` axis on /inbox; other surfaces leave // it false and the spec is a no-op. +// +// Predicates is a flat per-source narrowing record: keys at the top +// level are data sources ("deadline", "appointment", …) and values are +// the per-source predicate structs directly. The shape on the wire and +// the shape the frontend emits agree exactly — see t-paliad-283 for the +// latent contract bug (Go used to wrap each entry in another Predicates +// struct, so the frontend's overlay clicks parsed back as no-op). type FilterSpec struct { - Version int `json:"version"` - Sources []DataSource `json:"sources"` - Scope ScopeSpec `json:"scope"` - Time TimeSpec `json:"time"` - Predicates map[DataSource]Predicates `json:"predicates,omitempty"` - UnreadOnly bool `json:"unread_only,omitempty"` + Version int `json:"version"` + Sources []DataSource `json:"sources"` + Scope ScopeSpec `json:"scope"` + Time TimeSpec `json:"time"` + Predicates *Predicates `json:"predicates,omitempty"` + UnreadOnly bool `json:"unread_only,omitempty"` } // ScopeSpec narrows which projects contribute rows. Resolved at query @@ -147,7 +154,8 @@ const ( ) // Predicates is the per-source narrowing payload. Empty fields mean -// "no narrowing" — never "exclude all". +// "no narrowing" — never "exclude all". One field per data source; +// the wire shape is the same: `{"deadline": {...}, "appointment": {...}}`. type Predicates struct { Deadline *DeadlinePredicates `json:"deadline,omitempty"` Appointment *AppointmentPredicates `json:"appointment,omitempty"` @@ -305,14 +313,25 @@ func (s *FilterSpec) Validate() error { return err } - for src, preds := range s.Predicates { - if !isKnownSource(src) { - return fmt.Errorf("%w: predicates set on unknown source %q", ErrInvalidInput, src) + if s.Predicates != nil { + // Reject predicates set on a source the spec doesn't list — we'd + // silently drop the narrowing otherwise. Walk the set fields. + type srcCheck struct { + src DataSource + present bool } - if !seen[src] { - return fmt.Errorf("%w: predicates set on source %q which is not selected", ErrInvalidInput, src) + checks := []srcCheck{ + {SourceDeadline, s.Predicates.Deadline != nil}, + {SourceAppointment, s.Predicates.Appointment != nil}, + {SourceProjectEvent, s.Predicates.ProjectEvent != nil}, + {SourceApprovalRequest, s.Predicates.ApprovalRequest != nil}, } - if err := preds.validate(); err != nil { + for _, c := range checks { + if c.present && !seen[c.src] { + return fmt.Errorf("%w: predicates set on source %q which is not selected", ErrInvalidInput, c.src) + } + } + if err := s.Predicates.validate(); err != nil { return err } } diff --git a/internal/services/filter_spec_predicates_test.go b/internal/services/filter_spec_predicates_test.go new file mode 100644 index 0000000..16c5b52 --- /dev/null +++ b/internal/services/filter_spec_predicates_test.go @@ -0,0 +1,125 @@ +package services + +import ( + "encoding/json" + "testing" +) + +// t-paliad-283 regression: the bar's chip clicks POST a `predicates` +// payload shaped as `{: }`. The Go side previously +// declared `Predicates map[DataSource]Predicates` — a doubled-nested +// shape — which silently unmarshalled the bar's payload as no-op +// narrowing. This test pins the wire shape so the contract can't drift +// again. +// +// Run with `go test ./internal/services/`. + +func TestFilterSpec_FlatPredicatesWireShape(t *testing.T) { + // The shape every chip click in the FilterBar emits: predicates is + // keyed by data source, value is the per-source predicate struct + // directly. Doubled-nesting would unmarshal as empty Predicates. + const wire = `{ + "version": 1, + "sources": ["deadline", "appointment", "project_event", "approval_request"], + "scope": {"projects": {"mode": "all_visible"}}, + "time": {"field": "auto", "horizon": "past_30d"}, + "predicates": { + "deadline": {"status": ["pending"]}, + "appointment": {"appointment_types": ["hearing"]}, + "project_event": {"event_types": ["deadline_created"]}, + "approval_request": {"viewer_role": "any_visible", "status": ["pending"]} + } + }` + + var spec FilterSpec + if err := json.Unmarshal([]byte(wire), &spec); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if err := spec.Validate(); err != nil { + t.Fatalf("validate: %v", err) + } + if spec.Predicates == nil { + t.Fatal("predicates must be non-nil after unmarshalling the bar's shape") + } + if spec.Predicates.Deadline == nil || len(spec.Predicates.Deadline.Status) != 1 || spec.Predicates.Deadline.Status[0] != "pending" { + t.Errorf("deadline.status must round-trip, got %+v", spec.Predicates.Deadline) + } + if spec.Predicates.Appointment == nil || len(spec.Predicates.Appointment.AppointmentTypes) != 1 { + t.Errorf("appointment.appointment_types must round-trip, got %+v", spec.Predicates.Appointment) + } + if spec.Predicates.ProjectEvent == nil || len(spec.Predicates.ProjectEvent.EventTypes) != 1 { + t.Errorf("project_event.event_types must round-trip, got %+v", spec.Predicates.ProjectEvent) + } + if spec.Predicates.ApprovalRequest == nil || spec.Predicates.ApprovalRequest.ViewerRole != "any_visible" { + t.Errorf("approval_request.viewer_role must round-trip, got %+v", spec.Predicates.ApprovalRequest) + } +} + +// The shipped FilterSpec must marshal back to exactly the flat shape +// the frontend declares in views/types.ts. Otherwise /api/views/system +// (which serializes the InboxSystemView's Filter for the bar) returns a +// shape the frontend can't consume without translation gymnastics. +func TestFilterSpec_MarshalFlatPredicatesShape(t *testing.T) { + spec := FilterSpec{ + Version: SpecVersion, + Sources: []DataSource{SourceDeadline}, + Scope: ScopeSpec{Projects: ScopeProjects{Mode: ScopeAllVisible}}, + Time: TimeSpec{Horizon: HorizonNext30d, Field: FieldAuto}, + Predicates: &Predicates{ + Deadline: &DeadlinePredicates{Status: []string{"pending"}}, + }, + } + b, err := json.Marshal(spec) + if err != nil { + t.Fatalf("marshal: %v", err) + } + // Parse back generically so the assertion is on the wire shape, not + // on the Go type system that produced it. + var raw map[string]json.RawMessage + if err := json.Unmarshal(b, &raw); err != nil { + t.Fatalf("re-unmarshal: %v", err) + } + var preds map[string]json.RawMessage + if err := json.Unmarshal(raw["predicates"], &preds); err != nil { + t.Fatalf("predicates re-unmarshal: %v", err) + } + dl, ok := preds["deadline"] + if !ok { + t.Fatal("predicates.deadline missing — wire shape regressed") + } + var dlBody map[string]json.RawMessage + if err := json.Unmarshal(dl, &dlBody); err != nil { + t.Fatalf("deadline body unmarshal: %v", err) + } + if _, ok := dlBody["status"]; !ok { + t.Errorf("predicates.deadline.status must be a top-level field; doubled-nesting reappeared. Body: %s", string(dl)) + } + if _, ok := dlBody["deadline"]; ok { + t.Errorf("predicates.deadline must NOT wrap a nested deadline key — that's the t-paliad-283 bug. Body: %s", string(dl)) + } +} + +// End-to-end pin: the bar's payload after the user clicks +// "Frist-Status: Erledigt" (completed) must produce a spec whose +// runDeadlines branch narrows to completed deadlines. Without the +// t-paliad-283 fix, the unmarshal silently produced an empty Predicates +// and the SQL ran without the `status='completed'` clause. +func TestFilterSpec_BarChipPayloadNarrowsDeadlineStatus(t *testing.T) { + const barPayload = `{ + "version": 1, + "sources": ["deadline"], + "scope": {"projects": {"mode": "all_visible"}}, + "time": {"field": "auto", "horizon": "past_30d"}, + "predicates": {"deadline": {"status": ["completed"]}} + }` + var spec FilterSpec + if err := json.Unmarshal([]byte(barPayload), &spec); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if spec.Predicates == nil || spec.Predicates.Deadline == nil { + t.Fatal("deadline predicate must survive the round-trip") + } + if len(spec.Predicates.Deadline.Status) != 1 || spec.Predicates.Deadline.Status[0] != "completed" { + t.Errorf("deadline.status must be [\"completed\"], got %+v", spec.Predicates.Deadline.Status) + } +} diff --git a/internal/services/filter_spec_test.go b/internal/services/filter_spec_test.go index 724f418..5906d26 100644 --- a/internal/services/filter_spec_test.go +++ b/internal/services/filter_spec_test.go @@ -180,8 +180,8 @@ func TestFilterSpec_NewSymmetricHorizonsValidate(t *testing.T) { func TestFilterSpec_PredicatesRequireSourceSelected(t *testing.T) { s := validBaseSpec() s.Sources = []DataSource{SourceDeadline} - s.Predicates = map[DataSource]Predicates{ - SourceAppointment: {Appointment: &AppointmentPredicates{AppointmentTypes: []string{"hearing"}}}, + s.Predicates = &Predicates{ + Appointment: &AppointmentPredicates{AppointmentTypes: []string{"hearing"}}, } if err := s.Validate(); !errors.Is(err, ErrInvalidInput) { t.Fatalf("predicates on unselected source must reject, got %v", err) @@ -190,8 +190,8 @@ func TestFilterSpec_PredicatesRequireSourceSelected(t *testing.T) { func TestFilterSpec_DeadlineStatusEnum(t *testing.T) { s := validBaseSpec() - s.Predicates = map[DataSource]Predicates{ - SourceDeadline: {Deadline: &DeadlinePredicates{Status: []string{"weird"}}}, + s.Predicates = &Predicates{ + Deadline: &DeadlinePredicates{Status: []string{"weird"}}, } if err := s.Validate(); !errors.Is(err, ErrInvalidInput) { t.Fatalf("unknown deadline.status must reject, got %v", err) @@ -201,8 +201,8 @@ func TestFilterSpec_DeadlineStatusEnum(t *testing.T) { func TestFilterSpec_AppointmentTypeEnum(t *testing.T) { s := validBaseSpec() s.Sources = append(s.Sources, SourceAppointment) - s.Predicates = map[DataSource]Predicates{ - SourceAppointment: {Appointment: &AppointmentPredicates{AppointmentTypes: []string{"bogus"}}}, + s.Predicates = &Predicates{ + Appointment: &AppointmentPredicates{AppointmentTypes: []string{"bogus"}}, } if err := s.Validate(); !errors.Is(err, ErrInvalidInput) { t.Fatalf("unknown appointment_type must reject, got %v", err) @@ -212,8 +212,8 @@ func TestFilterSpec_AppointmentTypeEnum(t *testing.T) { func TestFilterSpec_ProjectEventKindMustBeKnown(t *testing.T) { s := validBaseSpec() s.Sources = []DataSource{SourceProjectEvent} - s.Predicates = map[DataSource]Predicates{ - SourceProjectEvent: {ProjectEvent: &ProjectEventPredicates{EventTypes: []string{"unknown_kind"}}}, + s.Predicates = &Predicates{ + ProjectEvent: &ProjectEventPredicates{EventTypes: []string{"unknown_kind"}}, } if err := s.Validate(); !errors.Is(err, ErrInvalidInput) { t.Fatalf("unknown project_event kind must reject, got %v", err) @@ -223,8 +223,8 @@ func TestFilterSpec_ProjectEventKindMustBeKnown(t *testing.T) { func TestFilterSpec_ApprovalViewerRoleEnum(t *testing.T) { s := validBaseSpec() s.Sources = []DataSource{SourceApprovalRequest} - s.Predicates = map[DataSource]Predicates{ - SourceApprovalRequest: {ApprovalRequest: &ApprovalRequestPredicates{ViewerRole: "everyone"}}, + s.Predicates = &Predicates{ + ApprovalRequest: &ApprovalRequestPredicates{ViewerRole: "everyone"}, } if err := s.Validate(); !errors.Is(err, ErrInvalidInput) { t.Fatalf("unknown viewer_role must reject, got %v", err) @@ -234,8 +234,8 @@ func TestFilterSpec_ApprovalViewerRoleEnum(t *testing.T) { func TestFilterSpec_ApprovalRequestStatusEnum(t *testing.T) { s := validBaseSpec() s.Sources = []DataSource{SourceApprovalRequest} - s.Predicates = map[DataSource]Predicates{ - SourceApprovalRequest: {ApprovalRequest: &ApprovalRequestPredicates{Status: []string{"weird"}}}, + s.Predicates = &Predicates{ + ApprovalRequest: &ApprovalRequestPredicates{Status: []string{"weird"}}, } if err := s.Validate(); !errors.Is(err, ErrInvalidInput) { t.Fatalf("unknown approval_request.status must reject, got %v", err) @@ -251,15 +251,15 @@ func TestFilterSpec_RoundTripJSON(t *testing.T) { PersonalOnly: false, }, Time: TimeSpec{Horizon: HorizonNext30d, Field: FieldAuto}, - Predicates: map[DataSource]Predicates{ - SourceDeadline: {Deadline: &DeadlinePredicates{ + Predicates: &Predicates{ + Deadline: &DeadlinePredicates{ Status: []string{"pending"}, ApprovalStatus: []string{"approved", "pending"}, - }}, - SourceApprovalRequest: {ApprovalRequest: &ApprovalRequestPredicates{ + }, + ApprovalRequest: &ApprovalRequestPredicates{ ViewerRole: "approver_eligible", Status: []string{"pending"}, - }}, + }, }, } b, err := MarshalFilterSpec(original) diff --git a/internal/services/system_views.go b/internal/services/system_views.go index 737819e..b8586c7 100644 --- a/internal/services/system_views.go +++ b/internal/services/system_views.go @@ -66,8 +66,8 @@ func AgendaSystemView() SystemView { Sources: []DataSource{SourceDeadline, SourceAppointment}, Scope: ScopeSpec{Projects: ScopeProjects{Mode: ScopeAllVisible}}, Time: TimeSpec{Horizon: HorizonNext30d, Field: FieldAuto}, - Predicates: map[DataSource]Predicates{ - SourceDeadline: {Deadline: &DeadlinePredicates{Status: []string{"pending"}}}, + Predicates: &Predicates{ + Deadline: &DeadlinePredicates{Status: []string{"pending"}}, }, }, Render: RenderSpec{ @@ -126,14 +126,14 @@ func InboxSystemView() SystemView { Sources: []DataSource{SourceApprovalRequest, SourceProjectEvent}, Scope: ScopeSpec{Projects: ScopeProjects{Mode: ScopeAllVisible}}, Time: TimeSpec{Horizon: HorizonPast30d, Field: FieldAuto}, - Predicates: map[DataSource]Predicates{ - SourceApprovalRequest: {ApprovalRequest: &ApprovalRequestPredicates{ + Predicates: &Predicates{ + ApprovalRequest: &ApprovalRequestPredicates{ ViewerRole: "any_visible", Status: []string{"pending"}, - }}, - SourceProjectEvent: {ProjectEvent: &ProjectEventPredicates{ + }, + ProjectEvent: &ProjectEventPredicates{ EventTypes: InboxProjectEventKinds, - }}, + }, }, }, Render: RenderSpec{ @@ -159,10 +159,10 @@ func InboxRequesterSystemView() SystemView { Sources: []DataSource{SourceApprovalRequest}, Scope: ScopeSpec{Projects: ScopeProjects{Mode: ScopeAllVisible}}, Time: TimeSpec{Horizon: HorizonAny, Field: FieldAuto}, - Predicates: map[DataSource]Predicates{ - SourceApprovalRequest: {ApprovalRequest: &ApprovalRequestPredicates{ + Predicates: &Predicates{ + ApprovalRequest: &ApprovalRequestPredicates{ ViewerRole: "self_requested", - }}, + }, }, }, Render: RenderSpec{ diff --git a/internal/services/system_views_test.go b/internal/services/system_views_test.go index 9750705..8c9f86a 100644 --- a/internal/services/system_views_test.go +++ b/internal/services/system_views_test.go @@ -82,11 +82,10 @@ func TestInboxSystemView_RowActionInbox(t *testing.T) { func TestInboxSystemView_CuratedProjectEventKinds(t *testing.T) { sv := InboxSystemView() - preds := sv.Filter.Predicates[SourceProjectEvent] - if preds.ProjectEvent == nil { + if sv.Filter.Predicates == nil || sv.Filter.Predicates.ProjectEvent == nil { t.Fatal("InboxSystemView must narrow project_event predicates") } - got := preds.ProjectEvent.EventTypes + got := sv.Filter.Predicates.ProjectEvent.EventTypes if len(got) != len(InboxProjectEventKinds) { t.Errorf("expected %d curated kinds, got %d", len(InboxProjectEventKinds), len(got)) } diff --git a/internal/services/view_service.go b/internal/services/view_service.go index ce33fe5..b1308bb 100644 --- a/internal/services/view_service.go +++ b/internal/services/view_service.go @@ -234,8 +234,8 @@ func (s *EventService) runDeadlines(ctx context.Context, userID uuid.UUID, spec uid := userID df.CreatedBy = &uid } - if preds, ok := spec.Predicates[SourceDeadline]; ok && preds.Deadline != nil { - dp := preds.Deadline + if spec.Predicates != nil && spec.Predicates.Deadline != nil { + dp := spec.Predicates.Deadline // Status: ListFilter has DeadlineStatusFilter (single-value filter). // If the spec asks for both pending+completed → no narrowing; if // only pending → DeadlineFilterPending; only completed → Completed. @@ -317,8 +317,8 @@ func (s *EventService) runAppointments(ctx context.Context, userID uuid.UUID, sp } af.From = bounds.from af.To = bounds.to - if preds, ok := spec.Predicates[SourceAppointment]; ok && preds.Appointment != nil { - ap := preds.Appointment + if spec.Predicates != nil && spec.Predicates.Appointment != nil { + ap := spec.Predicates.Appointment // AppointmentListFilter takes a single Type today; narrow to first // listed value, fall back to all if multiple. if len(ap.AppointmentTypes) == 1 { @@ -482,21 +482,24 @@ func (s *EventService) runProjectEvents(ctx context.Context, userID uuid.UUID, s // ApprovalService inbox queries. ViewerRole picks which underlying // query runs. func (s *EventService) runApprovalRequests(ctx context.Context, userID uuid.UUID, spec FilterSpec, approval *ApprovalService, bounds viewSpecBounds) ([]ViewRow, error) { - preds := spec.Predicates[SourceApprovalRequest] + var ap *ApprovalRequestPredicates + if spec.Predicates != nil { + ap = spec.Predicates.ApprovalRequest + } role := "approver_eligible" - if preds.ApprovalRequest != nil && preds.ApprovalRequest.ViewerRole != "" { - role = preds.ApprovalRequest.ViewerRole + if ap != nil && ap.ViewerRole != "" { + role = ap.ViewerRole } filter := InboxFilter{} - if preds.ApprovalRequest != nil { + if ap != nil { // InboxFilter takes a single status today. If the spec says // only one, narrow; if multiple, leave open. - if len(preds.ApprovalRequest.Status) == 1 { - filter.Status = preds.ApprovalRequest.Status[0] + if len(ap.Status) == 1 { + filter.Status = ap.Status[0] } - if len(preds.ApprovalRequest.EntityTypes) == 1 { - filter.EntityType = preds.ApprovalRequest.EntityTypes[0] + if len(ap.EntityTypes) == 1 { + filter.EntityType = ap.EntityTypes[0] } } if spec.Scope.Projects.Mode == ScopeExplicit && len(spec.Scope.Projects.IDs) == 1 { @@ -665,19 +668,18 @@ func explicitProjectSet(spec FilterSpec) map[uuid.UUID]bool { // approvalStatusMatches checks the entity-side approval_status filter. // Returns true when the row passes (no filter set → always true). func approvalStatusMatches(rowStatus string, spec FilterSpec, src DataSource) bool { - preds, ok := spec.Predicates[src] - if !ok { + if spec.Predicates == nil { return true } var allowed []string switch src { case SourceDeadline: - if preds.Deadline != nil { - allowed = preds.Deadline.ApprovalStatus + if spec.Predicates.Deadline != nil { + allowed = spec.Predicates.Deadline.ApprovalStatus } case SourceAppointment: - if preds.Appointment != nil { - allowed = preds.Appointment.ApprovalStatus + if spec.Predicates.Appointment != nil { + allowed = spec.Predicates.Appointment.ApprovalStatus } } if len(allowed) == 0 { @@ -689,15 +691,15 @@ func approvalStatusMatches(rowStatus string, spec FilterSpec, src DataSource) bo // allowedAppointmentTypes returns nil when the filter is open, otherwise // a set of legal appointment_type values. func allowedAppointmentTypes(spec FilterSpec) map[string]bool { - preds, ok := spec.Predicates[SourceAppointment] - if !ok || preds.Appointment == nil { + if spec.Predicates == nil || spec.Predicates.Appointment == nil { return nil } - if len(preds.Appointment.AppointmentTypes) <= 1 { + ap := spec.Predicates.Appointment + if len(ap.AppointmentTypes) <= 1 { return nil // single-value already pushed down via AppointmentListFilter.Type } - out := make(map[string]bool, len(preds.Appointment.AppointmentTypes)) - for _, t := range preds.Appointment.AppointmentTypes { + out := make(map[string]bool, len(ap.AppointmentTypes)) + for _, t := range ap.AppointmentTypes { out[t] = true } return out @@ -712,13 +714,16 @@ func allowedAppointmentTypes(spec FilterSpec) map[string]bool { // don't want both rows showing up side-by-side. The drop applies to // both the explicit caller list and the implicit "all kinds" path. func allowedProjectEventKinds(spec FilterSpec) []string { - preds, ok := spec.Predicates[SourceProjectEvent] + var pe *ProjectEventPredicates + if spec.Predicates != nil { + pe = spec.Predicates.ProjectEvent + } dedupApprovals := slices.Contains(spec.Sources, SourceApprovalRequest) var requested []string switch { - case ok && preds.ProjectEvent != nil && len(preds.ProjectEvent.EventTypes) > 0: - requested = preds.ProjectEvent.EventTypes + case pe != nil && len(pe.EventTypes) > 0: + requested = pe.EventTypes case dedupApprovals: // No explicit narrowing, but ApprovalRequest is in sources — // rebuild the implicit "all" list so we can subtract approvals. @@ -750,30 +755,30 @@ func isApprovalAuditKind(kind string) bool { // allowedRequestStatuses returns nil for "no narrowing" (or "single value // already pushed into InboxFilter.Status"). func allowedRequestStatuses(spec FilterSpec) map[string]bool { - preds, ok := spec.Predicates[SourceApprovalRequest] - if !ok || preds.ApprovalRequest == nil { + if spec.Predicates == nil || spec.Predicates.ApprovalRequest == nil { return nil } - if len(preds.ApprovalRequest.Status) <= 1 { + ap := spec.Predicates.ApprovalRequest + if len(ap.Status) <= 1 { return nil } - out := make(map[string]bool, len(preds.ApprovalRequest.Status)) - for _, s := range preds.ApprovalRequest.Status { + out := make(map[string]bool, len(ap.Status)) + for _, s := range ap.Status { out[s] = true } return out } func allowedRequestEntityTypes(spec FilterSpec) map[string]bool { - preds, ok := spec.Predicates[SourceApprovalRequest] - if !ok || preds.ApprovalRequest == nil { + if spec.Predicates == nil || spec.Predicates.ApprovalRequest == nil { return nil } - if len(preds.ApprovalRequest.EntityTypes) <= 1 { + ap := spec.Predicates.ApprovalRequest + if len(ap.EntityTypes) <= 1 { return nil } - out := make(map[string]bool, len(preds.ApprovalRequest.EntityTypes)) - for _, t := range preds.ApprovalRequest.EntityTypes { + out := make(map[string]bool, len(ap.EntityTypes)) + for _, t := range ap.EntityTypes { out[t] = true } return out diff --git a/internal/services/view_service_inbox_test.go b/internal/services/view_service_inbox_test.go index 63e5562..c3ccd06 100644 --- a/internal/services/view_service_inbox_test.go +++ b/internal/services/view_service_inbox_test.go @@ -13,8 +13,8 @@ func TestAllowedProjectEventKinds_DedupsApprovalAudits(t *testing.T) { spec := FilterSpec{ Version: SpecVersion, Sources: []DataSource{SourceApprovalRequest, SourceProjectEvent}, - Predicates: map[DataSource]Predicates{ - SourceProjectEvent: {ProjectEvent: &ProjectEventPredicates{ + Predicates: &Predicates{ + ProjectEvent: &ProjectEventPredicates{ EventTypes: []string{ "deadline_created", "deadline_approval_requested", @@ -22,7 +22,7 @@ func TestAllowedProjectEventKinds_DedupsApprovalAudits(t *testing.T) { "approval_decided", "note_created", }, - }}, + }, }, } got := allowedProjectEventKinds(spec) @@ -47,13 +47,13 @@ func TestAllowedProjectEventKinds_NoDedupWhenApprovalsAbsent(t *testing.T) { spec := FilterSpec{ Version: SpecVersion, Sources: []DataSource{SourceProjectEvent}, - Predicates: map[DataSource]Predicates{ - SourceProjectEvent: {ProjectEvent: &ProjectEventPredicates{ + Predicates: &Predicates{ + ProjectEvent: &ProjectEventPredicates{ EventTypes: []string{ "deadline_created", "deadline_approval_requested", }, - }}, + }, }, } got := allowedProjectEventKinds(spec)