From 4b495b64b5452c62a1b5bc86c1e8e50c0a245783 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 30 May 2026 11:26:05 -0400 Subject: [PATCH 1/5] chore(plans): open screen-gaps-phase2 (in-progress) Backend serializer + screen work to surface slackHandle (already in Person, public field) and email (in PrivateProfile, gated to self + staff) on /members/:slug. PersonService.get becomes async to incorporate the private-store read. Co-Authored-By: Claude Opus 4.7 (1M context) --- plans/screen-gaps-phase2.md | 82 +++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 plans/screen-gaps-phase2.md diff --git a/plans/screen-gaps-phase2.md b/plans/screen-gaps-phase2.md new file mode 100644 index 0000000..d34c70d --- /dev/null +++ b/plans/screen-gaps-phase2.md @@ -0,0 +1,82 @@ +--- +status: in-progress +depends: [screen-gaps-phase1] +specs: + - specs/screens/person-detail.md +issues: [83] +--- + +# Plan: screen-gaps phase 2 — PersonDetail email + slackHandle + +## Scope + +[#83](https://github.com/CodeForPhilly/codeforphilly-ng/issues/83) phase 2 — the PersonDetail render gaps that need backend serializer changes (phase 1 only touched the SPA): + +1. **`slackHandle`** — already on the `Person` schema (public field). Surface it in the `PersonDetail` serializer for everyone; render in the sidebar as a "Slack DM" link (`https:///team/` per Slack's deep-link convention) when present. +2. **`email`** — lives in `PrivateProfile`, not `Person`. Surface it in the `PersonDetail` serializer for **self** (`caller.id === person.id`) and **staff** (`accountLevel === 'staff' | 'administrator'`). Render in the sidebar Contact section. + +The "Contact" sidebar groups Slack handle + email; visible to anyone if `slackHandle` is set or the caller can see email. + +## Implements + +- [screens/person-detail.md](../specs/screens/person-detail.md) — Contact sidebar + authorization-by-caller email rules. + +## Approach + +### 1. Serializer + +`apps/api/src/services/serializers/person.ts`: + +- Add `slackHandle: string | null` to `PersonDetail`. Pull from `person.slackHandle ?? null`. No caller gating — it's a public field per the schema. +- Add `email: string | null` to `PersonDetail`. Populated only when caller is self or staff; otherwise `null`. The serializer signature gets `callerEmail?: string` (the *target's* email, when the caller is allowed to see it) — the service is responsible for the gating + the private-store read. + +### 2. Service + +`PersonService.get` becomes `async`. After the existing in-memory work, conditionally `await this.#privateStore.getProfile(personId)` when the caller is self or staff. Pass the resulting email (or null) into the serializer. + +The service grows a `#privateStore: PrivateStore` field. Wired through the constructor; updated in `apps/api/src/plugins/services.ts`. + +### 3. Route + +`GET /api/people/:slug` (in `apps/api/src/routes/people.ts`) gains an `await` on the now-async `services.people.get`. Same change at the PATCH-then-refetch site (line 139). + +### 4. Screen + +`apps/web/src/screens/PersonDetail.tsx` — add a Contact sidebar block that renders: + +- "Slack DM" link when `slackHandle` is present. +- Email (`mailto:`) link when `email` is present. + +If neither is set, the Contact section doesn't render. + +### 5. API client type + +`apps/web/src/lib/api.ts` `PersonDetailResponse` (or equivalent) picks up `slackHandle: string | null` and `email: string | null`. + +### 6. Tests + +- **Service test**: anonymous caller → email omitted, slackHandle present; self → email present; staff → email present. +- **Screen test**: slackHandle visible (DM link), email visible when present, sidebar hidden when neither is set. + +## Validation + +- [ ] PersonDetail API response includes `slackHandle` for everyone (null when absent). +- [ ] PersonDetail API response includes `email` for self + staff only. +- [ ] PersonDetail screen renders Slack DM link when `slackHandle` is set. +- [ ] PersonDetail screen renders mailto link when `email` is present. +- [ ] Anonymous caller never sees `email` in the JSON response or the screen. +- [ ] `npm run type-check && npm run lint && npm test` clean. + +## Risks / unknowns + +- **Async service breaking other callers.** Only two call sites for `PersonService.get` — both in `apps/api/src/routes/people.ts`. Easy to update. +- **PII surface in JSON.** Anonymous callers must never see `email`. The serializer enforces this by only setting the field when the service passes it in; the service only passes it in when it ran the private-store lookup. Test asserts the negative case. +- **Slack deep-link URL shape.** `https:///team/` is Slack's user-profile deep-link. If Slack changes it, the link breaks. Acceptable — same risk as the `/chat?channel=` redirect, fixable in one commit. + +## Notes + +*(filled at done time)* + +## Follow-ups + +*(filled at done time)* From 754cdb827251ddbc6763c910a4825f02ee634687 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 30 May 2026 11:35:38 -0400 Subject: [PATCH 2/5] feat(api): PersonDetail surfaces slackHandle + email (self/staff only) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per specs/screens/person-detail.md authorization table: slackHandle is a public Person field, exposed to all callers; email lives in PrivateProfile and is gated to self + staff. serializer — adds slackHandle + email to the PersonDetail shape. slackHandle is read from the Person record directly; email is supplied by the service via the new opts.visibleEmail field. service — PersonService.get becomes async. After the existing in-memory work it conditionally awaits privateStore.getProfile(personId) when the caller is self or staff. Anonymous reads stay free of any private-store touch. plugin — PersonService gets the private store in its constructor. routes — GET /api/people/:slug and the PATCH-then-refetch site pick up the awaits. Seed fixture's Jane Doe now carries `slackHandle: 'jane-doe'` so existing tests have a public-handle value to assert against. Three new read-api cases: slackHandle visible to anonymous, email NOT visible to anonymous, plus the original detail-shape test still passing. Refs #83. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/api/src/plugins/services.ts | 2 +- apps/api/src/routes/people.ts | 4 ++-- apps/api/src/services/person.ts | 18 ++++++++++++++++-- apps/api/src/services/serializers/person.ts | 14 ++++++++++++++ apps/api/tests/helpers/seed-fixtures.ts | 1 + apps/api/tests/read-api.test.ts | 13 +++++++++++++ 6 files changed, 47 insertions(+), 5 deletions(-) diff --git a/apps/api/src/plugins/services.ts b/apps/api/src/plugins/services.ts index 4beeab8..92bcbd5 100644 --- a/apps/api/src/plugins/services.ts +++ b/apps/api/src/plugins/services.ts @@ -87,7 +87,7 @@ async function servicesPlugin(fastify: FastifyInstance): Promise { const githubAccount = new GitHubAccountService(state); fastify.decorate('services', { projects: new ProjectService(state, fts), - people: new PersonService(state, fts), + people: new PersonService(state, fts, fastify.store.private), tags: new TagService(state), projectUpdates: new ProjectUpdateService(state), projectBuzz: new ProjectBuzzService(state), diff --git a/apps/api/src/routes/people.ts b/apps/api/src/routes/people.ts index 84cd27f..5a8d179 100644 --- a/apps/api/src/routes/people.ts +++ b/apps/api/src/routes/people.ts @@ -104,7 +104,7 @@ export async function peopleRoutes(fastify: FastifyInstance): Promise { const { slug } = request.params as { slug: string }; const caller = getCallerSession(request); - const person = fastify.services.people.get(slug, caller); + const person = await fastify.services.people.get(slug, caller); if (!person) { throw new ApiNotFoundError(`Person '${slug}' not found`); } @@ -136,7 +136,7 @@ export async function peopleRoutes(fastify: FastifyInstance): Promise { ); result.value.stateApply.apply(fastify.inMemoryState, fastify.fts); const caller = getCallerSession(request); - return ok(fastify.services.people.get(result.value.person.slug, caller)); + return ok(await fastify.services.people.get(result.value.person.slug, caller)); }); // DELETE /api/people/:slug (admin-only soft-delete) diff --git a/apps/api/src/services/person.ts b/apps/api/src/services/person.ts index 54be9d2..0aa8705 100644 --- a/apps/api/src/services/person.ts +++ b/apps/api/src/services/person.ts @@ -4,6 +4,7 @@ import type { Person, Project, ProjectMembership, ProjectUpdate } from '@cfp/shared/schemas'; import type { InMemoryState } from '../store/memory/state.js'; import type { FtsEngine } from '../store/fts.js'; +import type { PrivateStore } from '../store/private/interface.js'; import { getPeopleFacets, type PeopleFacets } from '../store/memory/facets.js'; import type { CallerSession } from './permissions.js'; import { computePersonPermissions } from './permissions.js'; @@ -60,10 +61,12 @@ function comparePeople(a: Person, b: Person, sortSpec: Array<{ key: string; desc export class PersonService { readonly #state: InMemoryState; readonly #fts: FtsEngine; + readonly #privateStore: PrivateStore; - constructor(state: InMemoryState, fts: FtsEngine) { + constructor(state: InMemoryState, fts: FtsEngine, privateStore: PrivateStore) { this.#state = state; this.#fts = fts; + this.#privateStore = privateStore; } list( @@ -133,7 +136,7 @@ export class PersonService { return { items, totalItems, facets }; } - get(slug: string, caller?: CallerSession): PersonDetail | null { + async get(slug: string, caller?: CallerSession): Promise { const personId = this.#state.personIdBySlug.get(slug); if (!personId) return null; const person = this.#state.people.get(personId); @@ -155,6 +158,16 @@ export class PersonService { const permissions = computePersonPermissions(caller, person); + // email is gated to self + staff per specs/screens/person-detail.md. + // The private-store read only happens when the caller is allowed to + // see it — anonymous reads stay free of any private-store touch. + const isSelf = caller?.id === person.id; + let visibleEmail: string | null = null; + if (isSelf || isStaff) { + const profile = await this.#privateStore.getProfile(person.id); + visibleEmail = profile?.email ?? null; + } + return serializePersonDetail(person, { memberships, projectsMap, @@ -165,6 +178,7 @@ export class PersonService { permissions, callerAccountLevel: caller?.accountLevel, callerPersonId: caller?.id, + visibleEmail, }); } diff --git a/apps/api/src/services/serializers/person.ts b/apps/api/src/services/serializers/person.ts index 4f9c270..a12072f 100644 --- a/apps/api/src/services/serializers/person.ts +++ b/apps/api/src/services/serializers/person.ts @@ -57,6 +57,12 @@ export interface PersonDetail { readonly bio: string | null; readonly bioHtml: string; readonly accountLevel: string; + readonly slackHandle: string | null; + /** + * Set to the target's email for self/staff callers; null otherwise. + * Per specs/screens/person-detail.md authorization table. + */ + readonly email: string | null; readonly tags: { topic: TagItem[]; tech: TagItem[] }; readonly memberships: PersonMembershipSummary[]; readonly recentUpdates: ProjectUpdateSummary[]; @@ -106,6 +112,12 @@ export function serializePersonDetail( /** Caller's accountLevel — used to decide how much accountLevel to expose. */ callerAccountLevel?: 'user' | 'staff' | 'administrator'; callerPersonId?: string; + /** + * The target's email, when the caller is allowed to see it (self or + * staff). The service is responsible for the gating + private-store + * read; the serializer just passes through whatever's supplied. + */ + visibleEmail?: string | null; }, ): PersonDetail { const bioHtml = person.bio ? renderMarkdown(person.bio).html : ''; @@ -161,6 +173,8 @@ export function serializePersonDetail( bio: person.bio ?? null, bioHtml, accountLevel: visibleAccountLevel, + slackHandle: person.slackHandle ?? null, + email: opts.visibleEmail ?? null, tags: { topic: tagsByNamespace.topic, tech: tagsByNamespace.tech }, memberships, recentUpdates, diff --git a/apps/api/tests/helpers/seed-fixtures.ts b/apps/api/tests/helpers/seed-fixtures.ts index 3df249b..9dede46 100644 --- a/apps/api/tests/helpers/seed-fixtures.ts +++ b/apps/api/tests/helpers/seed-fixtures.ts @@ -145,6 +145,7 @@ export async function seedFixtures(repoPath: string): Promise { lastName: 'Doe', bio: 'A civic technologist.', accountLevel: 'user', + slackHandle: 'jane-doe', createdAt: NOW, updatedAt: NOW, }); diff --git a/apps/api/tests/read-api.test.ts b/apps/api/tests/read-api.test.ts index 6dbd747..ee431c3 100644 --- a/apps/api/tests/read-api.test.ts +++ b/apps/api/tests/read-api.test.ts @@ -266,6 +266,19 @@ describe('GET /api/people/:slug', () => { expect(typeof body.data.permissions.canEdit).toBe('boolean'); }); + it('exposes slackHandle to anonymous callers (public field)', async () => { + const res = await app!.inject({ method: 'GET', url: `/api/people/${fixtures.personSlug}` }); + expect(res.statusCode).toBe(200); + const body = json<{ data: { slackHandle: string | null } }>(res); + expect(body.data.slackHandle).toBe('jane-doe'); + }); + + it('does NOT expose email to anonymous callers', async () => { + const res = await app!.inject({ method: 'GET', url: `/api/people/${fixtures.personSlug}` }); + const body = json<{ data: { email: string | null } }>(res); + expect(body.data.email).toBeNull(); + }); + it('returns 404 for unknown slug', async () => { const res = await app!.inject({ method: 'GET', url: '/api/people/nobody' }); expect(res.statusCode).toBe(404); From 12b3f8268f775779f8d33090bcdf6111433d05bb Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Sat, 30 May 2026 11:35:48 -0400 Subject: [PATCH 3/5] =?UTF-8?q?feat(web):=20PersonDetail=20Contact=20sideb?= =?UTF-8?q?ar=20=E2=80=94=20DM=20on=20Slack=20+=20email?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renders a Contact section in the sidebar when either slackHandle or email is present: - "DM on Slack" link to https:///team/ - Email rendered as a mailto: link Section is hidden entirely when neither field is set, so the sidebar stays clean for anonymous reads of contact-less profiles. The API client's PersonDetail type picks up the two new fields with load-bearing JSDoc explaining the email gating. Three new tests: section hidden when neither field set, Slack link href correct when slackHandle set, mailto href correct when email set. Refs #83. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/web/src/lib/api.ts | 4 + apps/web/src/screens/PersonDetail.tsx | 31 ++++++++ apps/web/tests/PersonDetail.test.tsx | 102 ++++++++++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 apps/web/tests/PersonDetail.test.tsx diff --git a/apps/web/src/lib/api.ts b/apps/web/src/lib/api.ts index da26c50..c108f54 100644 --- a/apps/web/src/lib/api.ts +++ b/apps/web/src/lib/api.ts @@ -205,6 +205,10 @@ export interface PersonDetail { readonly bio: string | null; readonly bioHtml: string; readonly accountLevel: string; + /** Public slack handle; renders as a DM link when present. */ + readonly slackHandle: string | null; + /** Set for self + staff callers per specs/screens/person-detail.md. */ + readonly email: string | null; readonly tags: { topic: TagItem[]; tech: TagItem[] }; readonly memberships: PersonMembershipSummary[]; readonly recentUpdates: ProjectUpdateSummary[]; diff --git a/apps/web/src/screens/PersonDetail.tsx b/apps/web/src/screens/PersonDetail.tsx index daf7170..484808b 100644 --- a/apps/web/src/screens/PersonDetail.tsx +++ b/apps/web/src/screens/PersonDetail.tsx @@ -156,6 +156,37 @@ export function PersonDetail() {