Skip to content

feat(activists): activist details page#345

Merged
alexsapps merged 6 commits intomainfrom
alex/view-activist
Mar 20, 2026
Merged

feat(activists): activist details page#345
alexsapps merged 6 commits intomainfrom
alex/view-activist

Conversation

@alexsapps
Copy link
Copy Markdown
Collaborator

@alexsapps alexsapps commented Mar 19, 2026

read-only for now

Summary by CodeRabbit

  • New Features

    • Individual activist detail page with grouped, formatted fields, notes, linked contact values, prefetch navigation, loading and 404 pages.
    • Client and server endpoints to fetch single activist records; client caches server-fetched data for smoother navigation.
  • Bug Fixes

    • Improved HTTP error-to-response mapping and SSR error/redirect handling; more precise background behavior on nested routes.
    • Numeric blanks normalized to zero and required activist id/name validation to reduce missing-data issues.
  • Documentation

    • Frontend routing guidance and backend error-wrapping guidance updated.
  • Tests

    • Added server-side tests for chapter-scoped activist access.

@alexsapps alexsapps requested a review from jakehobbs as a code owner March 19, 2026 20:37
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 19, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 43ceca8a-6858-4e50-9c4d-ae01f8c7cd86

📥 Commits

Reviewing files that changed from the base of the PR and between a45be04 and e589859.

📒 Files selected for processing (3)
  • frontend-v2/src/app/(authed)/activists/activists-table.tsx
  • frontend-v2/src/app/(authed)/activists/format-value.ts
  • frontend-v2/src/lib/api/activists.ts

📝 Walkthrough

Walkthrough

Adds an Activist detail page (SSR + client hydration), a new backend GET /api/activists/{id} endpoint with organizer-scoped access, SSR HTTP-error routing helper rename/behavior change, formatting/linking utilities for activist fields, and related UI/import/path updates and docs.

Changes

Cohort / File(s) Summary
Activist detail UI
frontend-v2/src/app/(authed)/activists/[id]/page.tsx, frontend-v2/src/app/(authed)/activists/[id]/activist-detail.tsx, frontend-v2/src/app/(authed)/activists/[id]/loading.tsx
Added SSR page + client detail component and loading UI; server prefetch via React Query and hydration for client.
Formatting & linked values
frontend-v2/src/app/(authed)/activists/format-value.ts, frontend-v2/src/app/(authed)/activists/linked-value.tsx
New formatValue and LinkedValue modules for type-aware display and tel/mailto/https linking logic.
Column defs & table UX
frontend-v2/src/app/(authed)/activists/column-definitions.ts, frontend-v2/src/app/(authed)/activists/activists-table.tsx
Extended column defs (hideOnDetailPage, linkType); table now uses formatValue, name links prefetch detail, mobile cards are prefetch links.
API surface (client)
frontend-v2/src/lib/api.ts, frontend-v2/src/lib/api/activists.ts
Added API_PATH.ACTIVIST_GET, ApiClient.getActivist(...), ActivistGetResp schema; made ActivistJSON.id/name required; refactored blank-numeric-field helpers.
Server API & transport
server/src/main.go, server/src/transport/activists.go
Added route GET /api/activists/{id}MainController.ActivistGetHandler and ActivistGetHandler transport; validates id, maps model errors → HTTP statuses, returns { "activist": ... }.
Model & access control
server/src/model/activist.go, server/src/model/activist_test.go
Introduced ErrNotFound, tightened GetActivistJSON error messages, added GetActivistJSONForUser enforcing organizer/chapter scoping, and added tests for chapter-scoped behavior.
Server error semantics
server/src/model/event.go, server/src/transport/common.go
Propagated underlying errors and standardized not-found wrapping; added exported SendErrorMessage and ensure JSON Content-Type header.
SSR auth helper & call sites
frontend-v2/src/lib/server-auth.ts, many frontend-v2/src/app/(authed)/*/page.tsx
Renamed redirectIfForbiddenredirectForHttpError, expanded handling (403→forbidden, 404→notFound); updated many SSR call sites.
Imports & small fixes
frontend-v2/src/app/(authed)/activists/*filters*, filter-api-transform.ts, filter-url-codecs.ts
Switched several relative number-utils imports to @/lib/number-utils path-alias.
Layout & 404
frontend-v2/src/app/site-background-controller.tsx, frontend-v2/src/app/not-found.tsx
Narrowed full-screen route matching to exact prefixes; added site-level 404 page.
Docs
AGENTS.md
Documented frontend-v2 reverse-proxy routing and Go error-wrapping guidance (fmt.Errorf("context: %w", err)) and avoiding pkg/errors in new code.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Browser as Browser / Next.js
    participant SSR as SSR (Next)
    participant API as Go API Server
    participant DB as Database

    User->>Browser: Navigate to /activists/{id}
    Browser->>SSR: Request SSR page
    SSR->>SSR: parse id, build ApiClient with cookies
    SSR->>API: GET /api/activists/{id}
    API->>API: ActivistGetHandler (auth + params)
    API->>DB: GetActivistJSONForUser(db, authedUser, options)
    alt validation error
        DB-->>API: ErrValidation
        API-->>SSR: 400 Bad Request
    else not found / chapter mismatch
        DB-->>API: ErrNotFound
        API-->>SSR: 404 Not Found
    else success
        DB-->>API: ActivistJSON
        API-->>SSR: 200 + {activist}
        SSR->>SSR: dehydrate QueryClient
        SSR-->>Browser: HTML + hydrated state
        Browser->>Browser: Hydrate and render ActivistDetail
        Browser->>Browser: formatValue + LinkedValue render fields
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • jakehobbs

Poem

🐇 I hopped through code, a tiny glowing chart,
I stitched a detail page and gave each field heart.
Organizers guard chapters, links click true,
404 signs guide the lost back to view.
Hop on — the activist page is new.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.16% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(activists): activist details page' clearly summarizes the main change: adding a read-only activist details view page to the activists section.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch alex/view-activist
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend-v2/src/app/(authed)/activists/activists-table.tsx (1)

183-210: 🧹 Nitpick | 🔵 Trivial

Consider avoiding href="#" fallback.

When activist.id is falsy, using href="#" causes the page to scroll to the top on click, which is a poor UX. Since activists without IDs shouldn't occur in practice (they come from the database), consider either:

  1. Conditionally rendering a non-interactive <div> instead of a link
  2. Using href={undefined} and letting the link be non-functional
♻️ Optional: Conditional wrapper to avoid href="#"
-        {activists.map((activist) => (
-          <IntentPrefetchLink
+        {activists.map((activist) => {
+          const Wrapper = activist.id
+            ? ({ children, className }: { children: React.ReactNode; className: string }) => (
+                <IntentPrefetchLink
+                  href={`/activists/${activist.id}`}
+                  className={className}
+                >
+                  {children}
+                </IntentPrefetchLink>
+              )
+            : ({ children, className }: { children: React.ReactNode; className: string }) => (
+                <div className={className}>{children}</div>
+              )
+
+          return (
+            <Wrapper
             key={activist.id}
-            href={activist.id ? `/activists/${activist.id}` : '#'}
             className={`block rounded-lg border bg-card p-4 transition-opacity hover:border-primary/50 ${
               isStale ? 'opacity-60' : ''
             }`}
           >
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend-v2/src/app/`(authed)/activists/activists-table.tsx around lines 183
- 210, The current IntentPrefetchLink uses href="#" when activist.id is falsy
which causes scroll-to-top; update the render to avoid a "#": conditionally
render an interactive IntentPrefetchLink only when activist.id exists (use
href={`/activists/${activist.id}`}) and render a non-interactive wrapper (e.g.,
a <div> with the same className and children) when activist.id is missing,
keeping the inner mapping over visibleColumns, COLUMN_DEFINITIONS, formatValue
and ActivistJSON references unchanged; alternatively set href={undefined} on
IntentPrefetchLink when activist.id is falsy so the link is non-functional
instead of pointing to "#".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@AGENTS.md`:
- Line 13: Update the documentation text that references the old helper name
`redirectIfForbidden` to the new `redirectForHttpError` helper (and adjust any
explanatory examples or notes to call the new symbol), ensuring the guidance
still recommends using `fetchQuery` (not `prefetchQuery`) so 403s surface during
SSR; search for occurrences of `redirectIfForbidden` in the AGENTS.md content
and replace them with `redirectForHttpError`, and update any example code
snippets and surrounding wording to match the new helper signature and behavior.

In `@frontend-v2/src/app/`(authed)/activists/[id]/activist-detail.tsx:
- Around line 26-29: The component ActivistDetail currently returns null when
the hook useActivist(activistId) yields undefined, which hides UI during
client-side fetches; replace the early `if (!activist) return null` with a
loading state (spinner/skeleton) and/or use the hook's loading status if
available (e.g., isLoading or similar) to render a placeholder until activist
data is present; update the rendering logic in ActivistDetail to show the
loading indicator for client-side navigation and then render the full activist
UI once `activist` is defined.

In `@frontend-v2/src/app/`(authed)/activists/linked-value.tsx:
- Around line 11-12: The URL branch (case 'url' in linked-value.tsx) is too
strict — it only returns values that start with 'https' so http,
protocol-relative, or bare domains become non-clickable; replace the startsWith
check with a normalization/validation step (e.g., add a normalizeUrl(value)
helper called from the URL branch) that: 1) trims input, 2) converts
protocol-relative URLs ("//...") to "https://...", 3) prepends "https://" for
bare domains or known social domains (facebook.com, twitter.com, instagram.com,
etc.), 4) accepts "http://" by normalizing to "https://" or at least allowing it
as a clickable href, and 5) validates using the URL constructor (or a regex) and
returns null for invalid results; update the URL branch in linked-value.tsx to
call this helper and use the normalized href for clickable links.

In `@frontend-v2/src/lib/api.ts`:
- Around line 393-401: The detail fetch currently returns the parsed activist
directly which can leave numeric fields undefined; update getActivist to
normalize omitted zero-valued numeric fields after parsing by calling a helper
like fillSingleActivistBlankNumericFieldsWithZero(activist) (or implement that
logic in-place) so any BLANK_TO_ZERO_FIELDS (e.g., total_events,
months_since_last_action, total_points, total_interactions) that are undefined
are set to 0 before returning; locate getActivist and
ActivistGetResp.parse(resp).activist and ensure the normalized object is
returned instead of the raw parsed activist.

In `@server/src/model/activist_test.go`:
- Around line 470-508: Add a test that verifies GetActivistJSONForUser rejects a
non-admin from a different valid chapter: use GetOrCreateActivist to create an
activist in SFBayChapterIdDevTest (as in sfBayActivist), create a second real
chapter fixture ID (e.g., otherChapterID or OtherChapterIdDevTest), then call
GetActivistJSONForUser passing an ADBUser with Roles excluding admin and
ChapterID set to that other chapter and GetActivistOptions{ID:
sfBayActivist.ID}; assert it returns the same scoping error as GetActivistJSON
(ErrNotFound) to ensure user.ChapterID is being forwarded and cross-chapter
access is denied.

In `@server/src/model/activist.go`:
- Around line 889-890: You added the "a.hidden = false" predicate into the
shared activist query, which breaks callers that expect hidden rows (notably
UserUpdateActivist that calls GetActivistsExtra/GetActivistOptions and
immediately indexes orig[0]); either remove/move that predicate into the
caller-facing JSON helpers (GetActivistJSON / GetActivistJSONForUser) so the
shared lookup still returns hidden rows for internal callers, or add a defensive
check in UserUpdateActivist to reject zero-length results before indexing and
return a proper error; update the code paths referencing GetActivistsExtra,
GetActivistOptions, UserUpdateActivist, and
GetActivistJSON/GetActivistJSONForUser accordingly so hidden filtering is
applied only at the API layer or handled with a safe nil/empty check.
- Around line 30-31: Legacy callers of GetActivistJSON are invoking the old
sendErrorMessage(w, err) which doesn't map ErrNotFound or ErrValidation to
proper HTTP status codes; update those callers (the ones that call
sendErrorMessage from main.go) to either 1) detect error types using
errors.Is(err, model.ErrNotFound) and errors.Is(err, model.ErrValidation) and
pass the appropriate status via the newer transport sendErrorMessage(w, status,
err), or 2) replace the old sendErrorMessage(w, err) calls with direct calls to
the new sendErrorMessage(w, http.StatusNotFound/BadRequest/InternalServerError,
err) so that ErrNotFound and ErrValidation are returned with correct HTTP status
codes; ensure you reference GetActivistJSON and the ErrNotFound/ErrValidation
symbols when making these changes.

---

Outside diff comments:
In `@frontend-v2/src/app/`(authed)/activists/activists-table.tsx:
- Around line 183-210: The current IntentPrefetchLink uses href="#" when
activist.id is falsy which causes scroll-to-top; update the render to avoid a
"#": conditionally render an interactive IntentPrefetchLink only when
activist.id exists (use href={`/activists/${activist.id}`}) and render a
non-interactive wrapper (e.g., a <div> with the same className and children)
when activist.id is missing, keeping the inner mapping over visibleColumns,
COLUMN_DEFINITIONS, formatValue and ActivistJSON references unchanged;
alternatively set href={undefined} on IntentPrefetchLink when activist.id is
falsy so the link is non-functional instead of pointing to "#".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 83e5f64b-5c18-4ae8-93c3-0e5d02be60f9

📥 Commits

Reviewing files that changed from the base of the PR and between a9ccaf8 and 79c6953.

📒 Files selected for processing (30)
  • AGENTS.md
  • frontend-v2/src/app/(authed)/activists/[id]/activist-detail.tsx
  • frontend-v2/src/app/(authed)/activists/[id]/loading.tsx
  • frontend-v2/src/app/(authed)/activists/[id]/page.tsx
  • frontend-v2/src/app/(authed)/activists/activists-table.tsx
  • frontend-v2/src/app/(authed)/activists/column-definitions.ts
  • frontend-v2/src/app/(authed)/activists/filter-api-transform.ts
  • frontend-v2/src/app/(authed)/activists/filter-url-codecs.ts
  • frontend-v2/src/app/(authed)/activists/filters/date-range-filter.tsx
  • frontend-v2/src/app/(authed)/activists/filters/int-range-filter.tsx
  • frontend-v2/src/app/(authed)/activists/format-value.ts
  • frontend-v2/src/app/(authed)/activists/linked-value.tsx
  • frontend-v2/src/app/(authed)/activists/page.tsx
  • frontend-v2/src/app/(authed)/coachings/[id]/page.tsx
  • frontend-v2/src/app/(authed)/events/[id]/page.tsx
  • frontend-v2/src/app/(authed)/interest/generator/page.tsx
  • frontend-v2/src/app/(authed)/intl/organizers/page.tsx
  • frontend-v2/src/app/(authed)/users/[id]/page.tsx
  • frontend-v2/src/app/(authed)/users/new/page.tsx
  • frontend-v2/src/app/(authed)/users/page.tsx
  • frontend-v2/src/app/not-found.tsx
  • frontend-v2/src/app/site-background-controller.tsx
  • frontend-v2/src/lib/api.ts
  • frontend-v2/src/lib/number-utils.ts
  • frontend-v2/src/lib/server-auth.ts
  • server/src/main.go
  • server/src/model/activist.go
  • server/src/model/activist_test.go
  • server/src/model/event.go
  • server/src/transport/activists.go

Comment thread AGENTS.md Outdated
Comment thread frontend-v2/src/app/(authed)/activists/[id]/activist-detail.tsx Outdated
Comment thread frontend-v2/src/app/(authed)/activists/linked-value.tsx
Comment thread frontend-v2/src/lib/api.ts
Comment thread server/src/model/activist_test.go
Comment thread server/src/model/activist.go
Comment thread server/src/model/activist.go Outdated
@alexsapps alexsapps force-pushed the alex/view-activist branch from 79c6953 to f0325e2 Compare March 19, 2026 22:19
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (1)
server/src/model/activist.go (1)

581-596: ⚠️ Potential issue | 🟠 Major

Filter hidden activists in the API-facing lookup.

GetActivistJSONForUser now backs the public detail endpoint, but it delegates to an ID lookup that still skips a.hidden = false. A direct /api/activists/{id} request can therefore return a hidden/merged activist that never appears in list views. Keep that filter in this helper (or another API-only helper) instead of the shared ID query so internal callers like UserUpdateActivist can still load hidden rows safely.

💡 Suggested fix
 func GetActivistJSONForUser(db *sqlx.DB, authedUser ADBUser, options GetActivistOptions) (ActivistJSON, error) {
 	if !UserHasOrganizerAccess(authedUser) {
 		return ActivistJSON{}, ValidationErrorf("lacking permission to query activists")
 	}
 
 	if !UserHasRole("admin", authedUser) {
 		// options.ChapterID == 0 means "do not filter by chapter", so this
 		// check is important for security.
 		if authedUser.ChapterID == 0 {
 			return ActivistJSON{}, ValidationErrorf("cannot query activists without an active chapter")
 		}
 		options.ChapterID = authedUser.ChapterID
 	}
 
-	return GetActivistJSON(db, options)
+	activists, err := GetActivistsExtra(db, options)
+	if err != nil {
+		return ActivistJSON{}, err
+	}
+	if len(activists) == 0 || activists[0].Hidden {
+		return ActivistJSON{}, errors.Wrapf(ErrNotFound, "could not find activist with id %d", options.ID)
+	}
+	if len(activists) > 1 {
+		return ActivistJSON{}, errors.Errorf("found too many activists with id %d", options.ID)
+	}
+
+	return BuildActivistJSONArray(activists)[0], nil
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/model/activist.go` around lines 581 - 596, GetActivistJSONForUser
currently delegates to GetActivistJSON which may return hidden/merged activists;
update GetActivistJSONForUser to enforce the API-only filter (a.hidden = false)
by either setting a non-exported API flag or explicitly setting the options
field that prevents hidden rows (e.g. options.Hidden = false or
options.IgnoreHidden = false) before calling GetActivistJSON, or alternatively
create a new GetActivistJSONPublic wrapper that applies the hidden=false filter
and call that from GetActivistJSONForUser; ensure internal callers such as
UserUpdateActivist continue to call the original GetActivistJSON so they can
still load hidden rows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend-v2/src/app/`(authed)/activists/[id]/activist-detail.tsx:
- Around line 27-31: The loading check is conflating loading vs failed queries;
update the component to destructure isError and error from useActivist (e.g.,
const { data: activist, isLoading, isError, error } = useActivist(activistId))
and change the render logic so you only return the "Loading activist details..."
UI when isLoading is true; when isError or when !activist after loading, render
an error/fallback UI (e.g., show error?.message or a "Failed to load activist"
message) to surface failures instead of leaving the page stuck in the loading
state.

In `@frontend-v2/src/app/`(authed)/activists/activists-table.tsx:
- Around line 89-97: The link rendered inside the column check (colName ===
'name') uses the possibly empty value produced by formatValue(value, 'name'),
producing an unlabeled IntentPrefetchLink; change the render logic in
activists-table.tsx so that when formatted is falsy/empty you render a stable
fallback label (e.g. row.original.id or `Activist ${row.original.id}`) as the
link text and/or set an explicit aria-label using row.original.id, ensuring
IntentPrefetchLink remains accessible and never has an empty accessible name.

In `@frontend-v2/src/app/`(authed)/activists/format-value.ts:
- Around line 6-23: Precompute and cache column metadata at module scope to
avoid repeated Zod unwrapping and linear searches inside the hot path: build a
map from ActivistColumnName to its base type by iterating ActivistJSON.shape
once (using the same unwrapping logic now in getColumnType) and a map from
column name to its COLUMN_DEFINITIONS entry once (instead of repeated
COLUMN_DEFINITIONS.find calls); then update getColumnType and formatValue to
read from these cached maps (referencing getColumnType, formatValue,
COLUMN_DEFINITIONS, ActivistJSON, and ActivistColumnName) so each cell render
performs O(1) lookups only.

In `@frontend-v2/src/app/`(authed)/activists/linked-value.tsx:
- Around line 5-13: getLinkHref currently builds tel: and mailto: hrefs even
when value is an empty string; change getLinkHref to first treat
blank/whitespace-only values as absent (e.g., if value.trim() === '' return
null) and then construct the hrefs — specifically update the getLinkHref
function so the 'tel' and 'mailto' branches return null for blank values before
returning `tel:${value}` or `mailto:${value}` (you can keep the existing url
branch unchanged).

In `@server/src/main.go`:
- Around line 1111-1122: In EventGetHandler, avoid using http.Error (which
returns plain text) and instead write the API's JSON error envelope while
setting the HTTP status code: for the model.GetEvent call (using
model.GetEventOptions and checking model.ErrNotFound) set
w.Header().Set("Content-Type","application/json"), call w.WriteHeader with
either http.StatusNotFound or http.StatusInternalServerError, and encode a JSON
object like {"status": <code>, "message": "<human message>"} (use
json.NewEncoder(w).Encode) so clients receive the consistent {status,message}
shape instead of plain text.

---

Duplicate comments:
In `@server/src/model/activist.go`:
- Around line 581-596: GetActivistJSONForUser currently delegates to
GetActivistJSON which may return hidden/merged activists; update
GetActivistJSONForUser to enforce the API-only filter (a.hidden = false) by
either setting a non-exported API flag or explicitly setting the options field
that prevents hidden rows (e.g. options.Hidden = false or options.IgnoreHidden =
false) before calling GetActivistJSON, or alternatively create a new
GetActivistJSONPublic wrapper that applies the hidden=false filter and call that
from GetActivistJSONForUser; ensure internal callers such as UserUpdateActivist
continue to call the original GetActivistJSON so they can still load hidden
rows.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: a24bf291-61c1-4cb1-a567-3c00dde3992a

📥 Commits

Reviewing files that changed from the base of the PR and between 79c6953 and f0325e2.

📒 Files selected for processing (31)
  • AGENTS.md
  • frontend-v2/src/app/(authed)/activists/[id]/activist-detail.tsx
  • frontend-v2/src/app/(authed)/activists/[id]/loading.tsx
  • frontend-v2/src/app/(authed)/activists/[id]/page.tsx
  • frontend-v2/src/app/(authed)/activists/activists-table.tsx
  • frontend-v2/src/app/(authed)/activists/column-definitions.ts
  • frontend-v2/src/app/(authed)/activists/filter-api-transform.ts
  • frontend-v2/src/app/(authed)/activists/filter-url-codecs.ts
  • frontend-v2/src/app/(authed)/activists/filters/date-range-filter.tsx
  • frontend-v2/src/app/(authed)/activists/filters/int-range-filter.tsx
  • frontend-v2/src/app/(authed)/activists/format-value.ts
  • frontend-v2/src/app/(authed)/activists/linked-value.tsx
  • frontend-v2/src/app/(authed)/activists/page.tsx
  • frontend-v2/src/app/(authed)/coachings/[id]/page.tsx
  • frontend-v2/src/app/(authed)/events/[id]/page.tsx
  • frontend-v2/src/app/(authed)/interest/generator/page.tsx
  • frontend-v2/src/app/(authed)/intl/organizers/page.tsx
  • frontend-v2/src/app/(authed)/users/[id]/page.tsx
  • frontend-v2/src/app/(authed)/users/new/page.tsx
  • frontend-v2/src/app/(authed)/users/page.tsx
  • frontend-v2/src/app/not-found.tsx
  • frontend-v2/src/app/site-background-controller.tsx
  • frontend-v2/src/lib/api.ts
  • frontend-v2/src/lib/api/activists.ts
  • frontend-v2/src/lib/number-utils.ts
  • frontend-v2/src/lib/server-auth.ts
  • server/src/main.go
  • server/src/model/activist.go
  • server/src/model/activist_test.go
  • server/src/model/event.go
  • server/src/transport/activists.go

Comment thread frontend-v2/src/app/(authed)/activists/[id]/activist-detail.tsx Outdated
Comment thread frontend-v2/src/app/(authed)/activists/activists-table.tsx
Comment thread frontend-v2/src/app/(authed)/activists/format-value.ts Outdated
Comment thread frontend-v2/src/app/(authed)/activists/linked-value.tsx
Comment thread server/src/main.go Outdated
@alexsapps alexsapps force-pushed the alex/view-activist branch from f0325e2 to a45be04 Compare March 19, 2026 23:47
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
server/src/model/activist.go (1)

581-595: ⚠️ Potential issue | 🟠 Major

Hidden activists are still visible through the new detail endpoint.

This API-facing helper adds role and chapter checks, but it still delegates to the unrestricted ID lookup. Because that shared ID path does not filter a.hidden, an organizer who knows an ID can fetch a hidden activist. Please enforce hidden=false at this boundary—without breaking internal callers like UserUpdateActivist that still need unrestricted ID lookups—e.g. by selecting the hidden flag and mapping hidden rows to ErrNotFound.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/src/model/activist.go` around lines 581 - 595, GetActivistJSONForUser
currently delegates to the unrestricted GetActivistJSON and thus leaks hidden
activists; fix it by enforcing hidden=false at this API boundary: when preparing
options in GetActivistJSONForUser (GetActivistOptions) ensure the query includes
hidden=false (or explicitly select the hidden column and return ErrNotFound if
the row has hidden==true) before returning the ActivistJSON so internal callers
like UserUpdateActivist that rely on unrestricted ID lookups are not changed;
reference GetActivistJSONForUser, GetActivistJSON, GetActivistOptions,
UserUpdateActivist, and ErrNotFound when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend-v2/src/lib/api.ts`:
- Around line 551-591: The current normalization only fills missing numeric
fields (via BLANK_TO_ZERO_FIELDS) but doesn't handle boolean fields that arrive
as undefined when the server omits false; add a boolean normalization pass:
introduce a BLANK_TO_FALSE_FIELDS (or similar) list, add a helper like
fillActivistBlankBooleanFieldsWithFalseForFields (mirroring
fillActivistBlankNumericFieldsWithZeroForFields) that sets undefined boolean
fields to false on an ActivistJSON, and call it from
fillSingleActivistBlankNumericFieldsWithZero and
fillActivistBlankNumericFieldsWithZero (apply both numeric and boolean helpers
when mapping activists), referencing the existing functions
fillActivistBlankNumericFieldsWithZeroForFields,
fillSingleActivistBlankNumericFieldsWithZero,
fillActivistBlankNumericFieldsWithZero and BLANK_TO_ZERO_FIELDS to locate where
to add the boolean handling.

In `@frontend-v2/src/lib/api/activists.ts`:
- Around line 5-6: The ActivistJSON Zod schema currently declares name as
optional; change it to a required string by replacing z.string().optional() with
z.string() in the ActivistJSON definition so the schema matches the backend
guarantee (keep id as z.number() unchanged); update any imports/usages if type
inference changes.

---

Duplicate comments:
In `@server/src/model/activist.go`:
- Around line 581-595: GetActivistJSONForUser currently delegates to the
unrestricted GetActivistJSON and thus leaks hidden activists; fix it by
enforcing hidden=false at this API boundary: when preparing options in
GetActivistJSONForUser (GetActivistOptions) ensure the query includes
hidden=false (or explicitly select the hidden column and return ErrNotFound if
the row has hidden==true) before returning the ActivistJSON so internal callers
like UserUpdateActivist that rely on unrestricted ID lookups are not changed;
reference GetActivistJSONForUser, GetActivistJSON, GetActivistOptions,
UserUpdateActivist, and ErrNotFound when making the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 52a30e8e-e252-4ce1-a820-b28746ed216c

📥 Commits

Reviewing files that changed from the base of the PR and between f0325e2 and a45be04.

📒 Files selected for processing (31)
  • frontend-v2/src/app/(authed)/activists/[id]/activist-detail.tsx
  • frontend-v2/src/app/(authed)/activists/[id]/loading.tsx
  • frontend-v2/src/app/(authed)/activists/[id]/page.tsx
  • frontend-v2/src/app/(authed)/activists/activists-table.tsx
  • frontend-v2/src/app/(authed)/activists/column-definitions.ts
  • frontend-v2/src/app/(authed)/activists/filter-api-transform.ts
  • frontend-v2/src/app/(authed)/activists/filter-url-codecs.ts
  • frontend-v2/src/app/(authed)/activists/filters/date-range-filter.tsx
  • frontend-v2/src/app/(authed)/activists/filters/int-range-filter.tsx
  • frontend-v2/src/app/(authed)/activists/format-value.ts
  • frontend-v2/src/app/(authed)/activists/linked-value.tsx
  • frontend-v2/src/app/(authed)/activists/page.tsx
  • frontend-v2/src/app/(authed)/coachings/[id]/page.tsx
  • frontend-v2/src/app/(authed)/events/[id]/page.tsx
  • frontend-v2/src/app/(authed)/interest/generator/page.tsx
  • frontend-v2/src/app/(authed)/intl/organizers/page.tsx
  • frontend-v2/src/app/(authed)/users/[id]/page.tsx
  • frontend-v2/src/app/(authed)/users/new/page.tsx
  • frontend-v2/src/app/(authed)/users/page.tsx
  • frontend-v2/src/app/not-found.tsx
  • frontend-v2/src/app/site-background-controller.tsx
  • frontend-v2/src/lib/api.ts
  • frontend-v2/src/lib/api/activists.ts
  • frontend-v2/src/lib/number-utils.ts
  • frontend-v2/src/lib/server-auth.ts
  • server/src/main.go
  • server/src/model/activist.go
  • server/src/model/activist_test.go
  • server/src/model/event.go
  • server/src/transport/activists.go
  • server/src/transport/common.go

Comment thread frontend-v2/src/lib/api.ts
Comment thread frontend-v2/src/lib/api/activists.ts Outdated
@alexsapps alexsapps force-pushed the alex/view-activist branch from a45be04 to e589859 Compare March 20, 2026 00:14
@alexsapps alexsapps merged commit 36693ed into main Mar 20, 2026
1 of 2 checks passed
@alexsapps alexsapps deleted the alex/view-activist branch March 20, 2026 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant