From 58400ffdf7f1c55168c2dab70878df9927d7e7db Mon Sep 17 00:00:00 2001 From: Justin Paul Date: Thu, 11 Jun 2026 08:29:13 -0400 Subject: [PATCH] Person page: server-side search; stop loading the whole tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The person page fetched the entire tree on every open — all persons (to build a name map + power the relative pickers) and all events (to find partnership events). On a 2k-person tree that's a ~230KB person list + ~600KB event list per view. Now it loads only what the page shows: Frontend: - The relationship & spouse pickers use the backend's fuzzy pg_trgm search (debounced, typo-tolerant) instead of substring-filtering a preloaded array — better search, and no need to preload every person. PersonCombobox gained an `onSearch` server mode (client `people` mode still works). - The page drops the all-persons and all-events fetches; it resolves just this person's relatives' names via GET /persons?ids=..., and reads partnership events from the per-person events endpoint. Backend: - GET /trees/{id}/persons?ids=a,b,c — batch by id (privacy-filtered, names batched), for relative-name display. - list_events_for_person (member path) now also returns the person's partnership events, so the page needn't scan every event in the tree. Adversarial review (frontend logic + backend/privacy) found no issues. Suite 105 passing. Signed-off-by: Justin Paul --- backend/app/api/v1/persons.py | 13 ++- backend/app/services/event_service.py | 23 +++- backend/app/services/person_service.py | 41 +++++++ backend/tests/test_person_page_fetch.py | 60 ++++++++++ .../trees/[id]/persons/[personId]/page.tsx | 106 ++++++++++-------- frontend/components/person-combobox.tsx | 82 ++++++++++---- frontend/lib/api/schema.d.ts | 1 + frontend/openapi.json | 16 +++ 8 files changed, 275 insertions(+), 67 deletions(-) create mode 100644 backend/tests/test_person_page_fetch.py diff --git a/backend/app/api/v1/persons.py b/backend/app/api/v1/persons.py index 7641cc3..eae1e83 100644 --- a/backend/app/api/v1/persons.py +++ b/backend/app/api/v1/persons.py @@ -1,6 +1,6 @@ import uuid -from fastapi import APIRouter, status +from fastapi import APIRouter, HTTPException, status from app.api.deps import CurrentUser, SessionDep from app.schemas.person import PersonCreate, PersonRead, PersonUpdate @@ -41,9 +41,18 @@ async def list_persons( current: CurrentUser, deleted: bool = False, q: str | None = None, + ids: str | None = None, ) -> list[PersonRead]: tree = await tree_service.get_tree(session, viewer_id=current.id, tree_id=tree_id) - if q: + if ids is not None: + try: + id_list = [uuid.UUID(x) for x in ids.split(",") if x.strip()] + except ValueError as exc: + raise HTTPException(status.HTTP_422_UNPROCESSABLE_ENTITY, "invalid ids") from exc + persons = await person_service.list_persons_by_ids( + session, viewer_id=current.id, tree=tree, ids=id_list + ) + elif q: persons = await person_service.search_persons( session, viewer_id=current.id, tree=tree, query=q ) diff --git a/backend/app/services/event_service.py b/backend/app/services/event_service.py index 5a72309..86d20fa 100644 --- a/backend/app/services/event_service.py +++ b/backend/app/services/event_service.py @@ -4,9 +4,10 @@ engine. Every event has exactly one subject — a Person or a partnership.""" import uuid from datetime import date -from sqlalchemy import select +from sqlalchemy import or_, select from sqlalchemy.ext.asyncio import AsyncSession +from app.models.enums import RelationshipType from app.models.event import Event from app.models.person import Person from app.models.place import Place @@ -124,12 +125,30 @@ async def list_events_for_person( return await public_view_service.list_public_person_events( session, viewer_id=viewer_id, tree=tree, person_id=person_id ) + # Member view: this person's own events PLUS their partnership events (which + # live on the relationship and show on both partners). Returning both here + # means the person page doesn't have to load every event in the tree. + partner_rel_ids = ( + select(Relationship.id) + .where( + Relationship.tree_id == tree.id, + Relationship.type == RelationshipType.partnership, + Relationship.deleted_at.is_(None), + or_( + Relationship.person_from_id == person_id, + Relationship.person_to_id == person_id, + ), + ) + ) stmt = ( select(Event) .where( Event.tree_id == tree.id, - Event.person_id == person_id, Event.deleted_at.is_(None), + or_( + Event.person_id == person_id, + Event.relationship_id.in_(partner_rel_ids), + ), ) .order_by(Event.date_start.nulls_last(), Event.created_at) ) diff --git a/backend/app/services/person_service.py b/backend/app/services/person_service.py index f55f3fe..4a3e80e 100644 --- a/backend/app/services/person_service.py +++ b/backend/app/services/person_service.py @@ -404,6 +404,47 @@ async def list_persons( return visible +async def list_persons_by_ids( + session: AsyncSession, *, viewer_id: uuid.UUID, tree: Tree, ids: list[uuid.UUID] +) -> list[Person]: + """Just the named persons (privacy-filtered, names batched). Lets a page show + the names of someone's relatives without loading the whole tree.""" + role = await privacy.get_membership_role(session, viewer_id, tree.id) + if role is None and not await privacy.can_view_tree(session, user_id=viewer_id, tree=tree): + raise Forbidden("not permitted to view this tree") + if not ids: + return [] + persons = list( + ( + await session.execute( + select(Person).where( + Person.id.in_(ids), + Person.tree_id == tree.id, + Person.deleted_at.is_(None), + ) + ) + ).scalars().all() + ) + if role is not None: + await _attach_primary_names(session, persons) + return persons + visible: list[Person] = [] + full: list[Person] = [] + for person in persons: + vis = await privacy.person_visibility( + session, user_id=viewer_id, tree=tree, person=person + ) + if vis == Visibility.hidden: + continue + if vis == Visibility.redacted: + _redact(person) + else: + full.append(person) + visible.append(person) + await _attach_primary_names(session, full) + return visible + + async def search_persons( session: AsyncSession, *, viewer_id: uuid.UUID, tree: Tree, query: str, limit: int = 50 ) -> list[Person]: diff --git a/backend/tests/test_person_page_fetch.py b/backend/tests/test_person_page_fetch.py new file mode 100644 index 0000000..34052f3 --- /dev/null +++ b/backend/tests/test_person_page_fetch.py @@ -0,0 +1,60 @@ +"""Backing the trimmed person-page fetch: batch persons by id (for relative-name +display) and partnership events on the per-person events endpoint (so the page +doesn't load every event in the tree).""" + +from tests.conftest import auth, register + + +async def _tree(client, h): + return (await client.post("/api/v1/trees", json={"name": "T"}, headers=h)).json()["id"] + + +async def test_list_persons_by_ids(client): + h = auth(await register(client, "ids@ex.com")) + tid = await _tree(client, h) + a = (await client.post(f"/api/v1/trees/{tid}/persons", json={"given": "Aaa"}, headers=h)).json()["id"] + b = (await client.post(f"/api/v1/trees/{tid}/persons", json={"given": "Bbb"}, headers=h)).json()["id"] + c = (await client.post(f"/api/v1/trees/{tid}/persons", json={"given": "Ccc"}, headers=h)).json()["id"] + + r = await client.get(f"/api/v1/trees/{tid}/persons", params={"ids": f"{a},{c}"}, headers=h) + assert r.status_code == 200 + assert {p["id"] for p in r.json()} == {a, c} # only the requested, not b + assert all(p["primary_name"] for p in r.json()) # names resolved + + assert ( + await client.get(f"/api/v1/trees/{tid}/persons", params={"ids": "nope"}, headers=h) + ).status_code == 422 + assert ( + await client.get(f"/api/v1/trees/{tid}/persons", params={"ids": ""}, headers=h) + ).json() == [] + + +async def test_person_events_include_partnership(client): + h = auth(await register(client, "pev@ex.com")) + tid = await _tree(client, h) + p1 = (await client.post(f"/api/v1/trees/{tid}/persons", json={"given": "P1"}, headers=h)).json()["id"] + p2 = (await client.post(f"/api/v1/trees/{tid}/persons", json={"given": "P2"}, headers=h)).json()["id"] + await client.post( + f"/api/v1/trees/{tid}/events", + json={"event_type": "birth", "person_id": p1, "date_value": "1900"}, + headers=h, + ) + rel = ( + await client.post( + f"/api/v1/trees/{tid}/relationships", + json={"type": "partnership", "person_from_id": p1, "person_to_id": p2}, + headers=h, + ) + ).json()["id"] + await client.post( + f"/api/v1/trees/{tid}/events", + json={"event_type": "marriage", "relationship_id": rel, "date_value": "1925"}, + headers=h, + ) + + # P1's events: own birth + the partnership marriage, in one call. + e1 = {e["event_type"] for e in (await client.get(f"/api/v1/trees/{tid}/persons/{p1}/events", headers=h)).json()} + assert {"birth", "marriage"} <= e1 + # The marriage shows on BOTH partners' pages. + e2 = {e["event_type"] for e in (await client.get(f"/api/v1/trees/{tid}/persons/{p2}/events", headers=h)).json()} + assert "marriage" in e2 diff --git a/frontend/app/trees/[id]/persons/[personId]/page.tsx b/frontend/app/trees/[id]/persons/[personId]/page.tsx index 211796f..7fedc44 100644 --- a/frontend/app/trees/[id]/persons/[personId]/page.tsx +++ b/frontend/app/trees/[id]/persons/[personId]/page.tsx @@ -135,7 +135,6 @@ export default function PersonDetailPage() { const [evType, setEvType] = useState("birth"); const [evTypeOther, setEvTypeOther] = useState(""); const [evSpouse, setEvSpouse] = useState(""); // partner for a partnership event - const [allEvents, setAllEvents] = useState([]); // tree-wide, for partnership events const [dateQual, setDateQual] = useState("exact"); const [dateDay, setDateDay] = useState(""); const [dateMonth, setDateMonth] = useState(""); @@ -189,8 +188,9 @@ export default function PersonDetailPage() { return; } setPerson(p.data ?? null); - const [all, nm, mine, tr, ev, rl, src, cit, evAll, med] = await Promise.all([ - api.GET("/api/v1/trees/{tree_id}/persons", { params: { path: { tree_id: treeId } } }), + // Person-scoped fetches only — the page no longer pulls the whole tree. + // /persons/{id}/events now includes this person's partnership events too. + const [nm, mine, tr, ev, rl, src, cit, med] = await Promise.all([ api.GET("/api/v1/trees/{tree_id}/persons/{person_id}/names", { params: { path: { tree_id: treeId, person_id: personId } }, }), @@ -204,22 +204,49 @@ export default function PersonDetailPage() { }), api.GET("/api/v1/trees/{tree_id}/sources", { params: { path: { tree_id: treeId } } }), api.GET("/api/v1/trees/{tree_id}/citations", { params: { path: { tree_id: treeId } } }), - api.GET("/api/v1/trees/{tree_id}/events", { params: { path: { tree_id: treeId } } }), api.GET("/api/v1/trees/{tree_id}/media", { params: { path: { tree_id: treeId } } }), ]); - setPeople(all.data ?? []); setNames(nm.data ?? []); setMe(mine.data ?? null); setTree(tr.data ?? null); setEvents(ev.data ?? []); - setAllEvents(evAll.data ?? []); setMedia(med.data ?? []); setRels(rl.data ?? []); setSources(src.data ?? []); setCitations(cit.data ?? []); + // Resolve the names of just this person's relatives (for display), by id — + // not the whole tree. The relationship/spouse pickers search on demand. + const relList = rl.data ?? []; + const relatedIds = Array.from( + new Set( + relList + .flatMap((r) => [r.person_from_id, r.person_to_id]) + .filter((id): id is string => !!id && id !== personId), + ), + ); + if (relatedIds.length) { + const rel = await api.GET("/api/v1/trees/{tree_id}/persons", { + params: { path: { tree_id: treeId }, query: { ids: relatedIds.join(",") } }, + }); + setPeople(rel.data ?? []); + } else { + setPeople([]); + } setReady(true); }, [router, treeId, personId]); + // Server-side fuzzy search for the relative/spouse pickers — avoids loading + // every person just to search. + const searchPeople = useCallback( + async (query: string) => { + const r = await api.GET("/api/v1/trees/{tree_id}/persons", { + params: { path: { tree_id: treeId }, query: { q: query } }, + }); + return (r.data ?? []).filter((pp) => pp.id !== personId); + }, + [treeId, personId], + ); + useEffect(() => { load(); }, [load]); @@ -233,7 +260,6 @@ export default function PersonDetailPage() { return (id: string) => m.get(id) ?? "source"; }, [sources]); - const others = people.filter((p) => p.id !== personId); const parents = rels.filter((r) => r.type === "parent_child" && r.person_to_id === personId); const children = rels.filter((r) => r.type === "parent_child" && r.person_from_id === personId); const partners = rels.filter((r) => r.type === "partnership"); @@ -241,22 +267,18 @@ export default function PersonDetailPage() { const eventCites = (id: string) => citations.filter((c) => c.event_id === id); const personCites = citations.filter((c) => c.person_id === personId); - // Partnership events live on the relationship and show on both partners. + // Partnership events live on the relationship and show on both partners; the + // /persons/{id}/events endpoint now returns them alongside personal events. const myPartnerRels = rels.filter( (r) => r.type === "partnership" && (r.person_from_id === personId || r.person_to_id === personId), ); - const myPartnerRelIds = new Set(myPartnerRels.map((r) => r.id)); - const relEvents = allEvents.filter( - (e) => e.relationship_id && myPartnerRelIds.has(e.relationship_id), - ); const spouseOfRelEvent = (relId: string | null | undefined) => { const r = myPartnerRels.find((x) => x.id === relId); if (!r) return null; return r.person_from_id === personId ? r.person_to_id : r.person_from_id; }; const isPartnershipType = (t: string) => PARTNERSHIP_EVENTS.includes(t); - // Personal events + this person's partnership events, shown together. - const shownEvents = [...events, ...relEvents]; + const shownEvents = events; async function addEvent(e: React.FormEvent) { e.preventDefault(); @@ -1090,7 +1112,7 @@ export default function PersonDetailPage() {