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.
This commit is contained in:
@@ -104,7 +104,7 @@ func (s *AppointmentService) ListVisibleForUser(ctx context.Context, userID uuid
|
|||||||
}
|
}
|
||||||
|
|
||||||
if filter.ProjectID != nil {
|
if filter.ProjectID != nil {
|
||||||
conds = append(conds, `t.project_id = :project_id`)
|
conds = append(conds, projectDescendantPredicate("p"))
|
||||||
args["project_id"] = *filter.ProjectID
|
args["project_id"] = *filter.ProjectID
|
||||||
}
|
}
|
||||||
if filter.From != nil {
|
if filter.From != nil {
|
||||||
|
|||||||
@@ -124,7 +124,7 @@ func (s *DeadlineService) ListVisibleForUser(ctx context.Context, userID uuid.UU
|
|||||||
"user_id": userID,
|
"user_id": userID,
|
||||||
}
|
}
|
||||||
if filter.ProjectID != nil {
|
if filter.ProjectID != nil {
|
||||||
conds = append(conds, `f.project_id = :project_id`)
|
conds = append(conds, projectDescendantPredicate("p"))
|
||||||
args["project_id"] = *filter.ProjectID
|
args["project_id"] = *filter.ProjectID
|
||||||
}
|
}
|
||||||
if etCond := buildEventTypeFilterClause(filter, args); etCond != "" {
|
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,
|
"week_after": b.weekAfter,
|
||||||
}
|
}
|
||||||
if projectID != nil {
|
if projectID != nil {
|
||||||
conds = append(conds, `f.project_id = :project_id`)
|
conds = append(conds, projectDescendantPredicate("p"))
|
||||||
args["project_id"] = *projectID
|
args["project_id"] = *projectID
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -406,7 +406,7 @@ func (s *EventService) deadlineBuckets(ctx context.Context, userID uuid.UUID, pr
|
|||||||
"week_after": b.weekAfter,
|
"week_after": b.weekAfter,
|
||||||
}
|
}
|
||||||
if projectID != nil {
|
if projectID != nil {
|
||||||
conds = append(conds, `f.project_id = :project_id`)
|
conds = append(conds, projectDescendantPredicate("p"))
|
||||||
args["project_id"] = *projectID
|
args["project_id"] = *projectID
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -450,7 +450,7 @@ func (s *EventService) appointmentBuckets(ctx context.Context, userID uuid.UUID,
|
|||||||
"week_after": b.weekAfter,
|
"week_after": b.weekAfter,
|
||||||
}
|
}
|
||||||
if projectID != nil {
|
if projectID != nil {
|
||||||
conds = append(conds, `t.project_id = :project_id`)
|
conds = append(conds, projectDescendantPredicate("p"))
|
||||||
args["project_id"] = *projectID
|
args["project_id"] = *projectID
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
280
internal/services/project_filter_descendants_test.go
Normal file
280
internal/services/project_filter_descendants_test.go
Normal file
@@ -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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -53,3 +53,18 @@ func visibilityPredicatePositional(alias string, userArg int) string {
|
|||||||
AND pt.project_id = ANY(string_to_array(%s.path, '.')::uuid[])
|
AND pt.project_id = ANY(string_to_array(%s.path, '.')::uuid[])
|
||||||
))`, userArg, userArg, alias)
|
))`, 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[]))`
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user