Port circles page to React - #466
Conversation
Add a single /v2/circles page (mode selected via ?type=interest|geo nuqs query param) that replaces the legacy CirclesList.vue component, which serves both the "Interest Circles" and "Geo-Circles" nav items. Supports listing, create/edit/delete, host/member assignment via a new ActivistTagInput combobox, and the geo-mode member visibility toggle, calling the existing /circle/list, /circle/save, /circle/delete, and /activist_names/get_chaptermembers Go endpoints unchanged. Updates shared/nav.json to point the "Groups" dropdown at the new page and adds the legacy routes under "Legacy". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Ports the legacy Vue circles management page to the Next.js /v2 app by introducing a new authenticated route for managing both “Interest Circles” and “Geo-Circles” via a ?type=interest|geo query param, while keeping the existing Vue routes intact. This adds corresponding typed API client methods/schemas, React Query prefetch/hydration, and new UI components (table + dialogs + autocomplete tag input) to achieve feature parity.
Changes:
- Add new Next.js
/circlespage (served as/v2/circles) with server-side access gating + React Query hydration. - Implement circles list/table and create/edit/delete dialogs, including a purpose-built tag-input autocomplete for host/members.
- Extend the typed
ApiClientwith circle CRUD + chapter-member activist-names methods and update shared navigation to point to the new route.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/nav.json | Updates “Groups” nav items to point to the new /v2/circles?type=... route and adds legacy links under “Legacy”. |
| frontend-v2/src/lib/api.ts | Adds API paths, zod schemas, and ApiClient methods for circles + chapter-member activist names. |
| frontend-v2/src/app/(authed)/circles/search-params.ts | Defines typed parsing/loading for the `type=interest |
| frontend-v2/src/app/(authed)/circles/page.tsx | Server component route entry: evaluates access, prefetches initial queries, and hydrates the client page. |
| frontend-v2/src/app/(authed)/circles/loading.tsx | Loading UI for the circles route. |
| frontend-v2/src/app/(authed)/circles/circles-page.tsx | Client page: manages mode tabs, queries circles + activist names, and opens edit/delete dialogs. |
| frontend-v2/src/app/(authed)/circles/circle-table.tsx | Renders sortable table with last-meeting freshness indicator and geo members visibility toggling. |
| frontend-v2/src/app/(authed)/circles/circle-form-dialog.tsx | Create/edit modal with fields varying by mode and host/members selection. |
| frontend-v2/src/app/(authed)/circles/delete-circle-dialog.tsx | Delete confirmation dialog wired to the new delete endpoint. |
| frontend-v2/src/app/(authed)/circles/activist-tag-input.tsx | Chip/tag input with Popover-based autocomplete for selecting activists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { | ||
| data: circles, | ||
| isLoading: isCirclesLoading, | ||
| isError, | ||
| } = useQuery({ | ||
| queryKey: [API_PATH.CIRCLE_LIST], | ||
| queryFn: ({ signal }) => apiClient.getCircles(signal), | ||
| }) | ||
|
|
||
| const { data: activistNames, isLoading: isActivistsLoading } = useQuery({ | ||
| queryKey: [API_PATH.ACTIVIST_NAMES_CHAPTER_MEMBERS], | ||
| queryFn: ({ signal }) => apiClient.getChapterMemberActivistNames(signal), | ||
| }) | ||
|
|
There was a problem hiding this comment.
[Claude] Fixed in d931b46: a failed activist-names query now surfaces an inline error notice on the page instead of silently leaving the host/members autocomplete empty and non-functional.
- Drop the manual evaluateNavAccess/forbidden() gate in circles/page.tsx; it wrongly 403'd non-SF-Bay admins because it rejects on chapter before checking roles, while the Go backend grants admins access regardless of chapter. Rely on redirectForHttpError surfacing the backend's real 403 from the prefetch calls instead (same pattern as users/page.tsx). - Trim description, meeting time, meeting location, and coords at submit time for parity with the Vue form's v-model.trim. - Remove unused loadCircleSearchParams export. - Replace dead colSpan ternary with a constant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o jake/react-circles
Replace the local activist-tag-input.tsx with the shared TagInput component from jake/react-shared-tag-input (extracted from this implementation), renaming maxItems to max. External labels are kept for consistency with the rest of the circle form. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Narrow CircleGroupSchema.type from z.string() to the circle type enum so parsed circles carry the same narrow type as SaveCircleParams. - Add throwIfApiError to getChapterMemberActivistNames for consistency with sibling circle API methods. - Surface an activist-names query failure on the circles page with an inline notice instead of silently rendering an empty host/members autocomplete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o jake/react-circles
Merge the base branch's TagInput a11y fix (daced44) and switch the host/members fields from external <Label htmlFor> elements to the component's built-in label prop, so the host field's label no longer references an unmounted input once a host is selected (max=1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Final-review follow-ups for the shared TagInput base PR: - Add lib/members.ts with findPointPerson and countMailingListMembers, which the Working Groups (#464) and Circles (#466) tables currently duplicate (getPointPersonName/countMailingListMembers and hostName/memberCount respectively). Typed against a minimal structural member shape so both pages' member types satisfy them; the adopting branches will switch to these themselves. - Replace the multi-line JSX comment around the conditional Label htmlFor in tag-input.tsx with a named labelFor variable and a one-line comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Convert circle-table.tsx from hand-rolled useState/useMemo sorting to @tanstack/react-table's useReactTable + getSortedRowModel + SortIndicator, matching user-table.tsx and the other app tables. Columns per mode, default name-asc sort, and freshness coloring are unchanged. - Convert circle-form-dialog.tsx from seven useStates with manual submit-time trimming to @tanstack/react-form's useForm with a zod schema that handles trimming and the blank-name validation. Payloads and behavior are unchanged. - Extract STALE_AFTER_DAYS/FRESH_WITHIN_DAYS constants for the last-meeting freshness helper and shrink its comment to one line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o jake/react-circles
Merge the base branch's shared member helpers (125adfa) and replace the circle table's local hostName/memberCount functions with findPointPerson/countMailingListMembers from lib/members, and use findPointPerson for the circle form's host initializer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Compress multi-line comments across the circles PR's files to one line (two for access-gating and mode-semantics context): the page-level gating rationale, the client-side mode filter note, the form's trim parity note, the search-params mode explanation, and the chapter-member names API method description. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o jake/react-circles
Merge the base branch's TagInput API change (546908a: max replaced by single, id prop removed) and switch the host field from max={1} to single. Member usages are unlimited and unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
||
| const mutation = useMutation({ | ||
| mutationFn: (value: z.output<typeof circleFormSchema>) => { | ||
| const memberParams: SaveCircleMemberParams[] = [] |
There was a problem hiding this comment.
let's create this array by spreading and mapping instead of pushing and forEaching
There was a problem hiding this comment.
| }, | ||
| ] | ||
|
|
||
| if (mode === 'interest') { |
There was a problem hiding this comment.
pls just spread these cols into array above instead of pushing
There was a problem hiding this comment.
[Claude] Done in a8f0a73 — the columns are now one array literal ending in a conditional spread that picks the mode-specific column (Last Event for interest; Members or Total Members for geo), with the mode-specific columns hoisted to named consts inside the useMemo. No more push.
| queryFn: ({ signal }) => apiClient.getChapterMemberActivistNames(signal), | ||
| }) | ||
|
|
||
| const [membersVisible, setMembersVisible] = useState(false) |
There was a problem hiding this comment.
[Claude] Done in a8f0a73 — renamed to isMembersVisible/setIsMembersVisible, and the CircleTable prop is renamed to isMembersVisible to match.
| Before deleting a circle, be sure to remove all members of that | ||
| circle. | ||
| </p> |
There was a problem hiding this comment.
we should really just enforce this client-side
There was a problem hiding this comment.
[Claude] Done in a8f0a73 — verified the backend behaves like working groups: model.DeleteCircleGroup rejects circles that still have circle_members rows ("Cannot delete circle because it has members associated with it", server/src/model/circles.go:289). Mirrored #464's fix: the Delete button is disabled while the circle has members, and the warning switches to explain that they must be removed before deletion.
- Build the save payload's members array declaratively by spreading the host entry and the filtered/mapped member list; ordering and host dedup are unchanged. - Build the table's columns array in one literal with a conditional spread for the mode-specific column instead of pushing. - Rename membersVisible to isMembersVisible (state and table prop). - Disable the delete confirm button when the circle still has members and explain why in the warning, mirroring the backend rule that circle/delete rejects non-empty circles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Note
Stacked on #468 (
jake/react-shared-tag-input), which provides the sharedTagInputcomponent this page uses for the host/members fields. Merge #468 first.Summary
Ports
frontend/CirclesList.vue(~510 lines) to Next.js at a single new route,/v2/circles, with the "Interest Circles" vs "Geo-Circles" mode selected by a?type=interest|geoURL query param (defaultinterest) managed vianuqs— mirroring how the legacy Vue component is mounted twice with a differenttitleprop from the same Go template (server/templates/circles_list.html), driven byListCirclesHandler/ListGeoCirclesHandler.The legacy Vue page (
frontend/CirclesList.vue) and its routes (/list_circles,/list_geocircles) are untouched and still work.Endpoints used (all pre-existing, no backend changes)
POST /circle/list— fetches all circle groups (both types in one flat list, same as the Vue page); the new client-side filters bytypeparam just like the legacy component'sthis.title === 'GeoCirclesList' ? c.type === 'geo-circle' : c.type === 'circle'check.POST /circle/save— create (id: 0) or update (id: <existing>) a circle group.POST /circle/delete— delete bycircle_id; the backend refuses if the circle still has members, matching the legacy delete-modal warning.GET /activist_names/get_chaptermembers— chapter-member/organizer names for the host/members autocomplete (this is a different, more restrictive endpoint thanactivist_names/get, matching exactly what the Vue page calls).Added typed
ApiClientmethods (getCircles,saveCircle,deleteCircle,getChapterMemberActivistNames) andAPI_PATHconstants (CIRCLE_LIST,CIRCLE_SAVE,CIRCLE_DELETE,ACTIVIST_NAMES_CHAPTER_MEMBERS) plus zod schemas (CircleGroupSchema,CircleMemberSchema) infrontend-v2/src/lib/api.ts.Access gating
The
/circle/*endpoints are backed byapiSFBayOrganizerAuthMiddlewarein Go (admin always allowed regardless of chapter; organizer only if the user's chapter is SF Bay). Rather than duplicating that role logic client-side,page.tsxwraps the initialcircle/list/activist_names/get_chaptermembersprefetch inredirectForHttpError(theusers/page.tsxpattern), so the backend's real 403 surfaces as Next's forbidden UI. This correctly preserves the backend's unconditional admin bypass for non-SF-Bay admins.Nav changes (
shared/nav.jsononly)/v2/circles?type=geo, pageCirclesList_beta; "Interest Circles" →/v2/circles?type=interest, pageCirclesList_beta./list_geocircles//list_circleslegacy routes) to the end of the "Legacy" dropdown, each withroleRequired: ["organizer"],visibleForNonSFBay: false.Feature parity / intentional differences
date-fnsinstead of the Vue page'sdayjs).TagInputcomponent (chip list +Popover-based autocomplete filtered by "starts with", matchinggetFilteredActivists) from Add shared activist tag-input component #468 in place of Buefy'sb-taginput.Checkboxprimitive (noSwitchcomponent exists in this codebase yet) instead of a switch control.user-table.tsx) is overkill for this dataset size.Test plan
cd frontend-v2 && pnpm exec tsc --noEmit --ignoreDeprecations 6.0— no errors from this change. (Note: without--ignoreDeprecations 6.0,tscfails ontsconfig.json'sbaseUrloption withTS5101; confirmed viagit stash -uthat this is pre-existing/environmental, present onmainbefore this change, likely from thetypescript: ^6.0.3dependency bump vs. the currenttsconfig.json. Next's own build-time type checking, run below, is unaffected.)pnpm lint— clean.pnpm build— succeeds;/circlesappears in the route list. (Confirmed the$public/logo.pngmodule-resolution warning some tooling surfaces innav.tsxis also pre-existing/unrelated, via the samegit stash -ucheck.)pnpm dev+curl /v2/circles?type=geo— page compiles and renders (200); the only runtime error is a missingNEXT_PUBLIC_API_BASE_URL/live Go backend in this standalone check, expected since no backend/DB was running./v2/circles.🤖 Generated with Claude Code