From a69fff73e9ffab58c94ea8e669a8df6f63cb9418 Mon Sep 17 00:00:00 2001 From: m Date: Mon, 4 May 2026 18:57:06 +0200 Subject: [PATCH] feat(t-paliad-124): project filter includes descendant projects Selecting a Client in the project filter now returns rows attached to that Client AND every Litigation / Patent / Case below it (and so on down the tree). Previously the filter was exact-match: picking a Client hid every item in the subtree, which was the opposite of what users expect when they pick a parent in a hierarchical picker. The descendant set comes from paliad.projects.path - every project's path always contains its own id and every ancestor's id, so any project whose path includes the filter UUID is either that project or a descendant. Pattern matches the existing visibility predicate (which walks the path UPWARD for inheritance); the new helper just inverts the direction. Filter sites updated: - DeadlineService.ListVisibleForUser (/deadlines, /events) - DeadlineService.SummaryCounts (deadline summary cards) - AppointmentService.ListVisibleForUser (/appointments, /events) - EventService.deadlineBuckets (/events deadline rail) - EventService.appointmentBuckets (/events appointment rail) ListForProject (deadline/appointment/checklist/note) is unchanged - it fetches items for ONE specific project on the project detail page, not a filter. Visibility predicate (paliad.can_see_project) untouched - that walks upward and is a different concern. --- internal/services/appointment_service.go | 2 +- internal/services/deadline_service.go | 4 +- internal/services/event_service.go | 4 +- .../project_filter_descendants_test.go | 280 ++++++++++++++++++ internal/services/visibility.go | 15 + 5 files changed, 300 insertions(+), 5 deletions(-) create mode 100644 internal/services/project_filter_descendants_test.go diff --git a/internal/services/appointment_service.go b/internal/services/appointment_service.go index d828736..e40b080 100644 --- a/internal/services/appointment_service.go +++ b/internal/services/appointment_service.go @@ -104,7 +104,7 @@ func (s *AppointmentService) ListVisibleForUser(ctx context.Context, userID uuid } if filter.ProjectID != nil { - conds = append(conds, `t.project_id = :project_id`) + conds = append(conds, projectDescendantPredicate("p")) args["project_id"] = *filter.ProjectID } if filter.From != nil { diff --git a/internal/services/deadline_service.go b/internal/services/deadline_service.go index 4e97d4d..2d54eb8 100644 --- a/internal/services/deadline_service.go +++ b/internal/services/deadline_service.go @@ -124,7 +124,7 @@ func (s *DeadlineService) ListVisibleForUser(ctx context.Context, userID uuid.UU "user_id": userID, } if filter.ProjectID != nil { - conds = append(conds, `f.project_id = :project_id`) + conds = append(conds, projectDescendantPredicate("p")) args["project_id"] = *filter.ProjectID } if etCond := buildEventTypeFilterClause(filter, args); etCond != "" { @@ -634,7 +634,7 @@ func (s *DeadlineService) SummaryCounts(ctx context.Context, userID uuid.UUID, p "week_after": b.weekAfter, } if projectID != nil { - conds = append(conds, `f.project_id = :project_id`) + conds = append(conds, projectDescendantPredicate("p")) args["project_id"] = *projectID } diff --git a/internal/services/event_service.go b/internal/services/event_service.go index 7541777..849861c 100644 --- a/internal/services/event_service.go +++ b/internal/services/event_service.go @@ -406,7 +406,7 @@ func (s *EventService) deadlineBuckets(ctx context.Context, userID uuid.UUID, pr "week_after": b.weekAfter, } if projectID != nil { - conds = append(conds, `f.project_id = :project_id`) + conds = append(conds, projectDescendantPredicate("p")) args["project_id"] = *projectID } @@ -450,7 +450,7 @@ func (s *EventService) appointmentBuckets(ctx context.Context, userID uuid.UUID, "week_after": b.weekAfter, } if projectID != nil { - conds = append(conds, `t.project_id = :project_id`) + conds = append(conds, projectDescendantPredicate("p")) args["project_id"] = *projectID } diff --git a/internal/services/project_filter_descendants_test.go b/internal/services/project_filter_descendants_test.go new file mode 100644 index 0000000..a3bde8a --- /dev/null +++ b/internal/services/project_filter_descendants_test.go @@ -0,0 +1,280 @@ +package services + +import ( + "context" + "os" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jmoiron/sqlx" + _ "github.com/lib/pq" + + "mgit.msbls.de/m/paliad/internal/db" +) + +// TestProjectFilter_IncludesDescendants pins the t-paliad-124 contract: when +// the caller filters a list/summary by a Project UUID, the result includes +// rows attached to that Project AND every descendant Project (Litigation, +// Patent, Case below it). The descendant set is derived from +// paliad.projects.path, which the schema's path trigger keeps in sync from +// parent_id. The check is exercised against four entry points: +// - DeadlineService.ListVisibleForUser +// - DeadlineService.SummaryCounts +// - AppointmentService.ListVisibleForUser +// - EventService.ListVisibleForUser (union of deadlines + appointments) +// +// Skipped when TEST_DATABASE_URL is unset. +func TestProjectFilter_IncludesDescendants(t *testing.T) { + url := os.Getenv("TEST_DATABASE_URL") + if url == "" { + t.Skip("TEST_DATABASE_URL not set — skipping live DB test") + } + if err := db.ApplyMigrations(url); err != nil { + t.Fatalf("apply migrations: %v", err) + } + pool, err := sqlx.Connect("postgres", url) + if err != nil { + t.Fatalf("connect: %v", err) + } + defer pool.Close() + + ctx := context.Background() + + adminID := uuid.New() + clientID := uuid.New() // root: Client + litigationID := uuid.New() // child of Client: Litigation + caseID := uuid.New() // child of Litigation: Case + + // Six items: deadline + appointment at each of the three levels. + dClient := uuid.New() + dLitigation := uuid.New() + dCase := uuid.New() + aClient := uuid.New() + aLitigation := uuid.New() + aCase := uuid.New() + + cleanup := func() { + pool.ExecContext(ctx, `DELETE FROM paliad.appointments WHERE id IN ($1, $2, $3)`, aClient, aLitigation, aCase) + pool.ExecContext(ctx, `DELETE FROM paliad.deadlines WHERE id IN ($1, $2, $3)`, dClient, dLitigation, dCase) + // project_events / project_teams cleanup: hit each project_id. + for _, pid := range []uuid.UUID{caseID, litigationID, clientID} { + pool.ExecContext(ctx, `DELETE FROM paliad.project_events WHERE project_id = $1`, pid) + pool.ExecContext(ctx, `DELETE FROM paliad.project_teams WHERE project_id = $1`, pid) + } + // Delete bottom-up so the parent_id FK doesn't bite. + pool.ExecContext(ctx, `DELETE FROM paliad.projects WHERE id = $1`, caseID) + pool.ExecContext(ctx, `DELETE FROM paliad.projects WHERE id = $1`, litigationID) + pool.ExecContext(ctx, `DELETE FROM paliad.projects WHERE id = $1`, clientID) + pool.ExecContext(ctx, `DELETE FROM paliad.users WHERE id = $1`, adminID) + pool.ExecContext(ctx, `DELETE FROM auth.users WHERE id = $1`, adminID) + } + cleanup() + defer cleanup() + + if _, err := pool.ExecContext(ctx, + `INSERT INTO auth.users (id, email) VALUES ($1, $2)`, + adminID, "filter-desc@hlc.com"); err != nil { + t.Fatalf("seed auth.users: %v", err) + } + if _, err := pool.ExecContext(ctx, + `INSERT INTO paliad.users (id, email, display_name, office, global_role, lang) + VALUES ($1, $2, 'Filter Desc', 'munich', 'global_admin', 'de')`, + adminID, "filter-desc@hlc.com"); err != nil { + t.Fatalf("seed paliad.users: %v", err) + } + + // Build the Client → Litigation → Case tree. The BEFORE-INSERT path + // trigger fills the materialised path from parent_id; we still have to + // supply a non-null placeholder because path is NOT NULL — the trigger + // then overwrites it. Passing the row's own id as the placeholder + // matches the existing test convention (see deadline_service_test). + for _, p := range []struct { + id uuid.UUID + typ string + parent *uuid.UUID + title string + reference string + }{ + {clientID, "client", nil, "Acme Corp", "2026/9001"}, + {litigationID, "litigation", &clientID, "Acme v. Foo", "2026/9002"}, + {caseID, "case", &litigationID, "EP1234567 B1", "2026/9003"}, + } { + // path is overwritten by the BEFORE-INSERT trigger; pass the row's id + // stringified just to satisfy the NOT NULL column. parent_id is nullable + // uuid; passing a typed *uuid.UUID lets lib/pq encode NULL correctly. + var parent any + if p.parent != nil { + parent = *p.parent + } + if _, err := pool.ExecContext(ctx, + `INSERT INTO paliad.projects (id, type, parent_id, path, title, reference, status, created_by) + VALUES ($1, $2, $3, $4, $5, $6, 'active', $7)`, + p.id, p.typ, parent, p.id.String(), p.title, p.reference, adminID); err != nil { + t.Fatalf("seed paliad.projects %s: %v", p.id, err) + } + } + + // Sanity-check the path trigger built the tree correctly. + var paths struct { + Client string `db:"client_path"` + Litigation string `db:"lit_path"` + Case string `db:"case_path"` + } + if err := pool.GetContext(ctx, &paths, ` + SELECT + (SELECT path FROM paliad.projects WHERE id = $1) AS client_path, + (SELECT path FROM paliad.projects WHERE id = $2) AS lit_path, + (SELECT path FROM paliad.projects WHERE id = $3) AS case_path`, + clientID, litigationID, caseID); err != nil { + t.Fatalf("read paths: %v", err) + } + if paths.Client != clientID.String() { + t.Fatalf("client.path = %q, want %q", paths.Client, clientID.String()) + } + wantLit := clientID.String() + "." + litigationID.String() + if paths.Litigation != wantLit { + t.Fatalf("litigation.path = %q, want %q", paths.Litigation, wantLit) + } + wantCase := wantLit + "." + caseID.String() + if paths.Case != wantCase { + t.Fatalf("case.path = %q, want %q", paths.Case, wantCase) + } + + // Seed one deadline + one appointment at each level. Use a date well in + // the future so all of them land in a single SummaryCounts bucket and + // the math is unambiguous regardless of when the test runs. + now := time.Now().UTC() + due := now.AddDate(0, 6, 0).Truncate(24 * time.Hour) + startAt := now.AddDate(0, 6, 0) + for _, d := range []struct { + id uuid.UUID + pid uuid.UUID + }{ + {dClient, clientID}, + {dLitigation, litigationID}, + {dCase, caseID}, + } { + if _, err := pool.ExecContext(ctx, + `INSERT INTO paliad.deadlines + (id, project_id, title, due_date, source, status, created_by) + VALUES ($1, $2, 'D', $3::date, 'manual', 'pending', $4)`, + d.id, d.pid, due.Format("2006-01-02"), adminID); err != nil { + t.Fatalf("seed deadline %s: %v", d.id, err) + } + } + for _, a := range []struct { + id uuid.UUID + pid uuid.UUID + }{ + {aClient, clientID}, + {aLitigation, litigationID}, + {aCase, caseID}, + } { + if _, err := pool.ExecContext(ctx, + `INSERT INTO paliad.appointments + (id, project_id, title, start_at, appointment_type, created_by) + VALUES ($1, $2, 'A', $3, 'meeting', $4)`, + a.id, a.pid, startAt, adminID); err != nil { + t.Fatalf("seed appointment %s: %v", a.id, err) + } + } + + users := NewUserService(pool) + projects := NewProjectService(pool, users) + eventTypes := NewEventTypeService(pool, users) + deadlines := NewDeadlineService(pool, projects, eventTypes) + appointments := NewAppointmentService(pool, projects) + events := NewEventService(pool, deadlines, appointments) + + // countSeed counts how many of our six seed UUIDs appear in `ids`. + // Filtering to seed UUIDs avoids interference from any other rows the + // shared dev DB might carry. + seedDeadlines := map[uuid.UUID]bool{dClient: true, dLitigation: true, dCase: true} + seedAppointments := map[uuid.UUID]bool{aClient: true, aLitigation: true, aCase: true} + + type expect struct { + name string + filter uuid.UUID + wantDeadlines int + wantAppointmts int + } + cases := []expect{ + {"filter Client → all 3 levels", clientID, 3, 3}, + {"filter Litigation → Litigation + Case", litigationID, 2, 2}, + {"filter Case → just Case", caseID, 1, 1}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + pid := c.filter + + // DeadlineService.ListVisibleForUser + drows, err := deadlines.ListVisibleForUser(ctx, adminID, ListFilter{ProjectID: &pid}) + if err != nil { + t.Fatalf("deadlines.ListVisibleForUser: %v", err) + } + gotD := 0 + for _, r := range drows { + if seedDeadlines[r.ID] { + gotD++ + } + } + if gotD != c.wantDeadlines { + t.Errorf("deadlines.ListVisibleForUser: got %d seed rows, want %d", gotD, c.wantDeadlines) + } + + // DeadlineService.SummaryCounts + summary, err := deadlines.SummaryCounts(ctx, adminID, &pid) + if err != nil { + t.Fatalf("deadlines.SummaryCounts: %v", err) + } + // All seed deadlines are pending and far-future, so they all land + // in Total but in none of the time buckets we constrain here. We + // can't assert Total exactly (other rows may exist in the dev DB + // for the same project tree), so assert "at least N". + if summary.Total < c.wantDeadlines { + t.Errorf("deadlines.SummaryCounts.Total = %d, want >= %d", summary.Total, c.wantDeadlines) + } + + // AppointmentService.ListVisibleForUser + arows, err := appointments.ListVisibleForUser(ctx, adminID, AppointmentListFilter{ProjectID: &pid}) + if err != nil { + t.Fatalf("appointments.ListVisibleForUser: %v", err) + } + gotA := 0 + for _, r := range arows { + if seedAppointments[r.ID] { + gotA++ + } + } + if gotA != c.wantAppointmts { + t.Errorf("appointments.ListVisibleForUser: got %d seed rows, want %d", gotA, c.wantAppointmts) + } + + // EventService.ListVisibleForUser (union shape — same filter UUID). + erows, err := events.ListVisibleForUser(ctx, adminID, EventListFilter{ + Type: EventTypeAll, + ProjectID: &pid, + }) + if err != nil { + t.Fatalf("events.ListVisibleForUser: %v", err) + } + gotED, gotEA := 0, 0 + for _, r := range erows { + if seedDeadlines[r.ID] { + gotED++ + } + if seedAppointments[r.ID] { + gotEA++ + } + } + if gotED != c.wantDeadlines { + t.Errorf("events.ListVisibleForUser deadlines: got %d, want %d", gotED, c.wantDeadlines) + } + if gotEA != c.wantAppointmts { + t.Errorf("events.ListVisibleForUser appointments: got %d, want %d", gotEA, c.wantAppointmts) + } + }) + } +} diff --git a/internal/services/visibility.go b/internal/services/visibility.go index 0c11043..d56cd7f 100644 --- a/internal/services/visibility.go +++ b/internal/services/visibility.go @@ -53,3 +53,18 @@ func visibilityPredicatePositional(alias string, userArg int) string { AND pt.project_id = ANY(string_to_array(%s.path, '.')::uuid[]) ))`, userArg, userArg, alias) } + +// projectDescendantPredicate matches rows whose joined `paliad.projects` +// row (under `alias`) is the project bound to :project_id OR any descendant +// of it. The check inspects the materialised path column on paliad.projects +// — :project_id appears in the path of every descendant, plus the project +// itself (since path always includes self as the last label, t-paliad-018). +// +// Uses CAST(... AS uuid[]) — not the `::uuid[]` shorthand — for the same +// sqlx-named-parameter reason called out on visibilityPredicate above. +// +// Caller must JOIN paliad.projects under the given alias and bind +// :project_id to the selected project's UUID. +func projectDescendantPredicate(alias string) string { + return `:project_id = ANY(CAST(string_to_array(` + alias + `.path, '.') AS uuid[]))` +}