Person page: server-side search; stop loading the whole tree
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 <justin@jpaul.me>
This commit is contained in:
@@ -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
|
||||
)
|
||||
|
||||
@@ -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)
|
||||
)
|
||||
|
||||
@@ -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]:
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user