fix: fix bugs in event attendance page#309
Merged
Conversation
…for autocomplete, address misc comments from prior changes
📝 WalkthroughWalkthroughAdds last-event timestamps end-to-end: DB migration and indexes, API schema and server model/queries exposing Changes
Sequence Diagram(s)sequenceDiagram
participant Backend as Backend (server/model)
participant API as API (server handler)
participant Frontend as Frontend (useActivistRegistry)
participant Storage as Storage (IndexedDB)
participant Registry as Registry (activist-registry)
participant UI as UI (attendee-input-field)
Backend->>Backend: Query activists with MAX(e.date) AS last_event_date
Backend->>API: Return ActivistListBasicResp (includes last_event_date)
API->>Frontend: Deliver activists payload
Frontend->>Frontend: Compare last_event_date to local lastEventDate (ms)
Frontend->>Storage: Persist StoredActivist including lastEventDate
Frontend->>Registry: Load and sort activists by lastEventDate (desc, zeros last)
Registry->>UI: Provide sorted suggestions for Popover
UI->>Frontend: User selects/updates attendee -> update flows back to registry/storage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/src/model/activist.go (1)
1665-1754:⚠️ Potential issue | 🟡 MinorFormatting check is currently blocking CI.
Build logs indicate a misformatted file. Please run
make fmtbefore merge.🤖 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 1665 - 1754, The file is failing gofmt; run the repository formatter (make fmt) or gofmt/goimports over this file (e.g., reformat server/src/model/activist.go) to fix spacing/indentation and import formatting, then recommit; ensure functions/structs such as ActivistBasicInfoJSON, ActivistBasicInfo, ToJSON, GetActivistListBasicJSON and GetHiddenActivistIDs remain unchanged except for whitespace.
🤖 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/event/activist-registry.ts`:
- Around line 153-160: The current per-keystroke sort in getSuggestions() is
expensive; remove the .sort(...) call from getSuggestions() and keep
this.activists pre-sorted by lastEventDate when you merge/update the registry
(e.g., in the method that inserts/updates activists or the
mergeActivists/updateActivists routine). Ensure merge/update maintains
descending order (treat lastEventDate===0 as lowest) by performing stable
insertion (binary insert) or sorting only once after batch updates, then have
getSuggestions() simply filter the pre-sorted this.activists and slice the top N
results.
In `@frontend-v2/src/app/event/attendee-input-field.tsx`:
- Around line 82-85: When clearing suggestions on Escape (the keydown 'Escape'
case) and in the blur handler that also calls setSuggestions([]) /
setOpen(false), also reset the selected suggestion state by calling
setSelectedSuggestionIndex(-1) (or the appropriate "no-selection" value) so
selectedSuggestionIndex cannot point to a stale entry; update the 'Escape'
branch and the blur/clear locations (where setSuggestions([]) and setOpen(false)
are used) to include this setter call.
In `@frontend-v2/src/app/event/event-form.tsx`:
- Around line 115-117: There is a duplicate call to
queryClient.invalidateQueries with queryKey [API_PATH.ACTIVIST_LIST_BASIC];
remove the earlier/extra invocation so only the intended single invalidation
remains (keep the existing invalidateQueries([API_PATH.ACTIVIST_LIST_BASIC])
call that runs later in the save flow). Locate the duplicate by searching for
queryClient.invalidateQueries and the queryKey API_PATH.ACTIVIST_LIST_BASIC in
event-form.tsx and delete the redundant call.
---
Outside diff comments:
In `@server/src/model/activist.go`:
- Around line 1665-1754: The file is failing gofmt; run the repository formatter
(make fmt) or gofmt/goimports over this file (e.g., reformat
server/src/model/activist.go) to fix spacing/indentation and import formatting,
then recommit; ensure functions/structs such as ActivistBasicInfoJSON,
ActivistBasicInfo, ToJSON, GetActivistListBasicJSON and GetHiddenActivistIDs
remain unchanged except for whitespace.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
frontend-v2/src/app/event/activist-registry.tsfrontend-v2/src/app/event/activist-storage.tsfrontend-v2/src/app/event/attendee-input-field.tsxfrontend-v2/src/app/event/event-form.tsxfrontend-v2/src/app/event/useActivistRegistry.tsfrontend-v2/src/components/ui/README.mdfrontend-v2/src/lib/api.tspkg/shared/db-migrations/20260131054408_activist_timestamp_indexes.up.sqlserver/src/main.goserver/src/model/activist.go
There was a problem hiding this comment.
Pull request overview
Updates activist basic-list syncing and UI autocomplete to better reflect recent event activity, adding support for “last event date” so the frontend can sort suggestions by most recent attendee activity.
Changes:
- Server: extend activist “list_basic” payload with
last_event_date, and update query/sync filtering to incorporate event attendance. - Frontend: store/consume
last_event_date, sort autocomplete suggestions by recency, and render suggestions in a popover. - Misc: IndexedDB schema bump + migration behavior, cache invalidation after event save, and small robustness/docs tweaks.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/model/activist.go | Adds last_event_date to basic activist payload and updates list query/sync filtering + ordering |
| server/src/main.go | Switches modified_since handling from nullable pointer to time.Time zero-value semantics |
| pkg/shared/db-migrations/20260131054408_activist_timestamp_indexes.up.sql | Adds/clarifies index/migration notes for timestamp-based filtering |
| frontend-v2/src/lib/api.ts | Extends Zod schema to include last_event_date |
| frontend-v2/src/components/ui/README.md | Documents shadcn UI component customization to preserve on upgrades |
| frontend-v2/src/app/event/useActivistRegistry.ts | Adds mounted guards; maps last_event_date into registry records |
| frontend-v2/src/app/event/event-form.tsx | Invalidates activist list query after save; guards undefined attendee names |
| frontend-v2/src/app/event/attendee-input-field.tsx | Replaces always-visible suggestion list with a controlled popover-based autocomplete |
| frontend-v2/src/app/event/activist-storage.ts | Adds lastEventDate, bumps IndexedDB version, and clears cache/metadata on upgrade |
| frontend-v2/src/app/event/activist-registry.ts | Adds lastEventDate, sorts suggestions by recency, and adjusts method docs/logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend-v2/src/app/event/activist-registry.ts (1)
76-79: 🧹 Nitpick | 🔵 TrivialConsider removing O(n×m) lookup in
mergeActivists.
findIndexper incoming activist can get expensive on large full-sync batches. Building an ID→index map once per merge keeps this path linear before the final sort.♻️ Refactor sketch
async mergeActivists(newActivists: ActivistRecord[]): Promise<void> { if (newActivists.length === 0) { return } + const indexById = new Map(this.activists.map((a, i) => [a.id, i])) + for (const activist of newActivists) { - const existingIndex = this.activists.findIndex( - (a) => a.id === activist.id, - ) + const existingIndex = indexById.get(activist.id) ?? -1 if (existingIndex >= 0) { // Update existing activist (handles renames properly) const oldActivist = this.activists[existingIndex] this.activists[existingIndex] = activist @@ } else { // Add new activist this.activists.push(activist) + indexById.set(activist.id, this.activists.length - 1) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend-v2/src/app/event/activist-registry.ts` around lines 76 - 79, The mergeActivists function currently does an O(n×m) lookup by calling this.activists.findIndex for each activist in newActivists; instead, build a single Map (or plain object) mapping existing activist id → index from this.activists once, then iterate newActivists and use that map to locate and update existing entries (or push new ones) before performing the final sort. Update references to existingIndex to use the map lookup, and ensure the map is built from this.activists only once at the start of mergeActivists so the merge work is linear in the number of activists plus the sort.frontend-v2/src/app/event/activist-storage.ts (1)
25-42:⚠️ Potential issue | 🟠 MajorAdd
onblockedandonversionchangehandlers to prevent indefinite loading from IndexedDB upgrade conflicts.With
DB_VERSIONat 2, database upgrades can be blocked by an older open tab. TheopenDB()method currently lacks arequest.onblockedhandler and does not close connections onversionchange, allowing the promise to hang indefinitely and keep the UI in a loading state.🔧 Proposed fix
this.dbPromise = new Promise((resolve, reject) => { const request = indexedDB.open(DB_NAME, DB_VERSION) request.onerror = () => reject(request.error) + request.onblocked = () => { + reject( + new Error( + 'IndexedDB upgrade blocked by another open tab. Close other tabs and retry.', + ), + ) + } request.onsuccess = () => resolve(request.result) + request.onsuccess = () => { + const db = request.result + db.onversionchange = () => { + db.close() + } + resolve(db) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend-v2/src/app/event/activist-storage.ts` around lines 25 - 42, The openDB() promise can hang during upgrades; add a request.onblocked handler to reject the promise with a clear error when another tab is preventing the upgrade, and after request.onsuccess set the returned DB's onversionchange to immediately close the connection (db.onversionchange = () => db.close()) so other tabs can upgrade and the UI won't remain stuck; reference DB_VERSION, openDB(), the IndexedDB request object (request.onblocked) and the resolved IDBDatabase (db.onversionchange) when making these changes.
♻️ Duplicate comments (1)
frontend-v2/src/app/event/attendee-input-field.tsx (1)
64-75:⚠️ Potential issue | 🟠 MajorValidate suggestion index bounds before resolving selected values.
At Line 90 and Line 106,
suggestions[selectedSuggestionIndex]is used when index is only checked for>= 0. Combined with Line 64-75 (ArrowDown/ArrowUp), this can produce out-of-range access and submitundefinedon Enter/Tab when suggestions are empty.🔧 Proposed fix
case 'ArrowDown': { + if (suggestions.length === 0) { + return + } e.preventDefault() setSelectedSuggestionIndex((prev) => prev === suggestions.length - 1 ? 0 : prev + 1, ) return } case 'ArrowUp': { + if (suggestions.length === 0) { + return + } e.preventDefault() setSelectedSuggestionIndex((prev) => prev === 0 ? suggestions.length - 1 : prev - 1, ) return } @@ - const selectedValue = - selectedSuggestionIndex >= 0 - ? suggestions[selectedSuggestionIndex] - : trimmedValue + const hasSelectedSuggestion = + selectedSuggestionIndex >= 0 && + selectedSuggestionIndex < suggestions.length + const selectedValue = hasSelectedSuggestion + ? suggestions[selectedSuggestionIndex] + : trimmedValue handleSelectSuggestion(selectedValue) return } @@ - const selectedValue = - selectedSuggestionIndex >= 0 - ? suggestions[selectedSuggestionIndex] - : trimmedValue + const hasSelectedSuggestion = + selectedSuggestionIndex >= 0 && + selectedSuggestionIndex < suggestions.length + const selectedValue = hasSelectedSuggestion + ? suggestions[selectedSuggestionIndex] + : trimmedValue handleSelectSuggestion(selectedValue) }Also applies to: 90-93, 106-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend-v2/src/app/event/attendee-input-field.tsx` around lines 64 - 75, The arrow key handlers and Enter/Tab handlers can produce out-of-range accesses when suggestions is empty or selectedSuggestionIndex points outside 0..suggestions.length-1; update the logic in the ArrowDown/ArrowUp handlers (where setSelectedSuggestionIndex is used) to guard with suggestions.length>0 and keep selectedSuggestionIndex within [0, suggestions.length-1] (e.g., set to -1 or 0 appropriately when suggestions is empty), and in the Enter/Tab handling (where suggestions[selectedSuggestionIndex] is read) validate suggestions.length>0 and selectedSuggestionIndex >= 0 && selectedSuggestionIndex < suggestions.length before using suggestions[selectedSuggestionIndex] — if the check fails, avoid submitting/using undefined and handle the empty case (no-op or fallback) instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend-v2/src/app/event/activist-registry.ts`:
- Around line 76-79: The mergeActivists function currently does an O(n×m) lookup
by calling this.activists.findIndex for each activist in newActivists; instead,
build a single Map (or plain object) mapping existing activist id → index from
this.activists once, then iterate newActivists and use that map to locate and
update existing entries (or push new ones) before performing the final sort.
Update references to existingIndex to use the map lookup, and ensure the map is
built from this.activists only once at the start of mergeActivists so the merge
work is linear in the number of activists plus the sort.
In `@frontend-v2/src/app/event/activist-storage.ts`:
- Around line 25-42: The openDB() promise can hang during upgrades; add a
request.onblocked handler to reject the promise with a clear error when another
tab is preventing the upgrade, and after request.onsuccess set the returned DB's
onversionchange to immediately close the connection (db.onversionchange = () =>
db.close()) so other tabs can upgrade and the UI won't remain stuck; reference
DB_VERSION, openDB(), the IndexedDB request object (request.onblocked) and the
resolved IDBDatabase (db.onversionchange) when making these changes.
---
Duplicate comments:
In `@frontend-v2/src/app/event/attendee-input-field.tsx`:
- Around line 64-75: The arrow key handlers and Enter/Tab handlers can produce
out-of-range accesses when suggestions is empty or selectedSuggestionIndex
points outside 0..suggestions.length-1; update the logic in the
ArrowDown/ArrowUp handlers (where setSelectedSuggestionIndex is used) to guard
with suggestions.length>0 and keep selectedSuggestionIndex within [0,
suggestions.length-1] (e.g., set to -1 or 0 appropriately when suggestions is
empty), and in the Enter/Tab handling (where
suggestions[selectedSuggestionIndex] is read) validate suggestions.length>0 and
selectedSuggestionIndex >= 0 && selectedSuggestionIndex < suggestions.length
before using suggestions[selectedSuggestionIndex] — if the check fails, avoid
submitting/using undefined and handle the empty case (no-op or fallback)
instead.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
frontend-v2/src/app/event/activist-registry.tsfrontend-v2/src/app/event/activist-storage.tsfrontend-v2/src/app/event/attendee-input-field.tsxfrontend-v2/src/app/event/event-form.tsxfrontend-v2/src/app/event/useActivistRegistry.tsfrontend-v2/src/components/ui/README.mdfrontend-v2/src/lib/api.tspkg/shared/db-migrations/20260131054408_activist_timestamp_indexes.up.sqlserver/src/main.goserver/src/model/activist.go
There was a problem hiding this comment.
Actionable comments posted: 1
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/event/event-form.tsx (1)
307-314:⚠️ Potential issue | 🟡 MinorGuard
ensureMinimumEmptyFieldsagainst undefined attendee names.Line 313 still dereferences
it.name.lengthdirectly. If undefined names are possible (as handled in the updated segments), this can still throw at runtime.♻️ Proposed fix
const ensureMinimumEmptyFields = () => { const currentAttendees = form.state.values.attendees - const emptyCount = currentAttendees.filter((it) => !it.name.length).length + const emptyCount = currentAttendees.filter( + (it) => (it.name || '').trim() === '', + ).length if (emptyCount < MIN_EMPTY_FIELDS) { form.pushFieldValue('attendees', { name: '' }) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend-v2/src/app/event/event-form.tsx` around lines 307 - 314, In ensureMinimumEmptyFields, avoid dereferencing undefined attendee names by changing the emptyCount calculation to check names safely; when iterating currentAttendees (from form.state.values.attendees) use a guarded expression such as (it.name || '') instead of it.name so you compute emptyCount via ((it.name || '').length === 0) or !((it.name || '').length); update the emptyCount line in the ensureMinimumEmptyFields function accordingly to prevent runtime exceptions.
🤖 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/components/ui/README.md`:
- Around line 5-15: Add a concrete code example and upgrade notes for the
SelectTrigger customization: paste the actual SelectTrigger JSX from select.tsx
showing the className composition (reference the SelectTrigger component, the
className prop, and the cn utility) and highlight the custom classes
"hover:border-gray-400", "focus:border-primary", and
"focus:hover:border-primary"; add a short upgrade instruction telling
maintainers to run `npx shadcn@latest add select` then manually reapply those
classes to the SelectTrigger's className if the generator overwrites the file;
finally add a one-line rationale stating these classes were added to improve
hover/focus visual feedback during user interaction.
---
Outside diff comments:
In `@frontend-v2/src/app/event/event-form.tsx`:
- Around line 307-314: In ensureMinimumEmptyFields, avoid dereferencing
undefined attendee names by changing the emptyCount calculation to check names
safely; when iterating currentAttendees (from form.state.values.attendees) use a
guarded expression such as (it.name || '') instead of it.name so you compute
emptyCount via ((it.name || '').length === 0) or !((it.name || '').length);
update the emptyCount line in the ensureMinimumEmptyFields function accordingly
to prevent runtime exceptions.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
frontend-v2/src/app/event/activist-registry.tsfrontend-v2/src/app/event/activist-storage.tsfrontend-v2/src/app/event/attendee-input-field.tsxfrontend-v2/src/app/event/event-form.tsxfrontend-v2/src/app/event/useActivistRegistry.tsfrontend-v2/src/components/ui/README.mdfrontend-v2/src/lib/api.tspkg/shared/db-migrations/20260131054408_activist_timestamp_indexes.up.sqlserver/src/main.goserver/src/model/activist.go
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend-v2/src/app/event/activist-registry.ts (2)
195-201: 🧹 Nitpick | 🔵 TrivialRegex compilation on every filter call is acceptable but could be optimized.
The
nameFilterfunction compiles a new RegExp for each activist in the filter loop. For large activist lists, this could be optimized by compiling once pergetSuggestionscall.♻️ Compile regex once per search
- getSuggestions(input: string, maxResults = 10): string[] { + getSuggestions(input: string, maxResults = 10): string[] { const trimmedInput = input.trim() if (!trimmedInput.length) { return [] } - // this.activists is pre-sorted by lastEventDate descending - return this.activists - .filter(({ name }) => nameFilter(name, input)) + const pattern = trimmedInput + .replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + .replace(/ +/g, '.*') + const regex = new RegExp(pattern, 'i') + + return this.activists + .filter(({ name }) => regex.test(name)) .slice(0, maxResults) .map((a) => a.name) }This compiles the regex once per
getSuggestionscall instead of once per activist.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend-v2/src/app/event/activist-registry.ts` around lines 195 - 201, The current nameFilter(text: string, input: string) compiles a new RegExp for every activist; change the approach so the regex is compiled once in getSuggestions and then reused for each activist: add an overload or new helper (e.g., nameFilterWithRegex or change nameFilter signature to nameFilter(text: string, pattern: RegExp)) and move the escaping/replace logic out of the per-item loop into getSuggestions where you build the RegExp once from the raw input, then call the updated nameFilter for each activist; ensure existing callers are updated to pass a RegExp and keep the same case-insensitive behavior (/i) and escaping logic.
112-126: 🧹 Nitpick | 🔵 TrivialConsider adding early return for empty
idsarray.When
idsis empty, the method still creates a Set, filters the array, and potentially calls storage. Adding an early return would avoid unnecessary work.♻️ Add early return for empty input
async removeActivistsByIds(ids: number[]): Promise<void> { + if (ids.length === 0) return + const idsToRemove = new Set(ids) this.activists = this.activists.filter((activist) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend-v2/src/app/event/activist-registry.ts` around lines 112 - 126, The method removeActivistsByIds currently does work even when ids is empty; add an early return at the start of removeActivistsByIds to return immediately if ids.length === 0 to avoid creating idsToRemove, iterating this.activists, and calling this.storage.deleteActivistsByIds; keep the existing logic intact for non-empty ids and ensure the signature and Promise<void> behavior remain unchanged.
🤖 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/event/activist-storage.ts`:
- Around line 41-55: The cached dbPromise can become stale after a version
change; update the handlers so that when db.onversionchange runs you both close
the connection and reset the shared dbPromise (e.g., set it to null/undefined)
so subsequent callers can reopen the DB, and likewise ensure the
request.onblocked path clears the cached dbPromise when rejecting so the cache
doesn't hold a rejected/stale promise; locate the logic around
request.onsuccess, db.onversionchange and request.onblocked to implement these
changes.
In `@frontend-v2/src/app/event/attendee-input-field.tsx`:
- Around line 36-43: The open boolean is derived from suggestions.length > 0 &&
isFocused so remove the useEffect and the open state variable; instead compute a
derived value (e.g., const open = suggestions.length > 0 && isFocused or use
useMemo) and remove any setOpen(...) calls in handlers (references: useEffect
block, setOpen calls in handlers that currently close the popover). Ensure
handlers that cleared suggestions still work (they should just clear suggestions
or blur focus) so the derived open becomes false automatically.
- Around line 73-79: The ArrowUp handler for suggestions can produce -2 when
selectedSuggestionIndex is -1; update the logic in the ArrowUp case (where
setSelectedSuggestionIndex is called) to explicitly handle prev === -1 by
setting the index to suggestions.length - 1, otherwise keep the existing wrap
logic (prev === 0 ? suggestions.length - 1 : prev - 1); ensure this change is
made inside the same ArrowUp branch that checks suggestions.length > 0 so
keyboard cycling from no selection behaves correctly.
In `@frontend-v2/src/app/event/useActivistRegistry.ts`:
- Around line 78-91: Remove query.error from the useEffect dependency list so
the effect only reacts to query.isError changes; inside the effect, read
query.error only when query.isError is true (for logging) or capture it into a
local variable at the top of the effect if you need its value for the log, and
keep the mounted guard and setIsServerLoaded usage (functions/methods involved:
useEffect, mounted flag, query.isError, query.error, setIsServerLoaded,
toast.error, console.error).
---
Outside diff comments:
In `@frontend-v2/src/app/event/activist-registry.ts`:
- Around line 195-201: The current nameFilter(text: string, input: string)
compiles a new RegExp for every activist; change the approach so the regex is
compiled once in getSuggestions and then reused for each activist: add an
overload or new helper (e.g., nameFilterWithRegex or change nameFilter signature
to nameFilter(text: string, pattern: RegExp)) and move the escaping/replace
logic out of the per-item loop into getSuggestions where you build the RegExp
once from the raw input, then call the updated nameFilter for each activist;
ensure existing callers are updated to pass a RegExp and keep the same
case-insensitive behavior (/i) and escaping logic.
- Around line 112-126: The method removeActivistsByIds currently does work even
when ids is empty; add an early return at the start of removeActivistsByIds to
return immediately if ids.length === 0 to avoid creating idsToRemove, iterating
this.activists, and calling this.storage.deleteActivistsByIds; keep the existing
logic intact for non-empty ids and ensure the signature and Promise<void>
behavior remain unchanged.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
frontend-v2/src/app/event/activist-registry.tsfrontend-v2/src/app/event/activist-storage.tsfrontend-v2/src/app/event/attendee-input-field.tsxfrontend-v2/src/app/event/useActivistRegistry.ts
jakehobbs
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/event/useActivistRegistry.ts`:
- Around line 118-129: The current merge logic in useActivistRegistry overwrites
the entire activist record when either attendance or profile is newer, causing
fresher profile fields to be rolled back when only attendance is newer
(hasNewAttendanceData) and vice versa; change the push into activistsToUpdate to
do a field-wise merge: start from existingActivist (if any), then overwrite only
profile fields when incomingTimestamp > existingTimestamp, and overwrite only
lastEventDate when hasNewAttendanceData (convert last_event_date * 1000), and
set lastUpdated only when the profile timestamp is newer; ensure you reference
hasNewAttendanceData, incomingTimestamp, existingTimestamp, last_event_date,
lastUpdated and lastEventDate while composing the merged object so untouched
newer fields are preserved.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
frontend-v2/src/app/event/activist-registry.tsfrontend-v2/src/app/event/activist-storage.tsfrontend-v2/src/app/event/attendee-input-field.tsxfrontend-v2/src/app/event/useActivistRegistry.ts
This was referenced Mar 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores