Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request adds two new API route handlers for fetching board and batch member data from an external API, then integrates them into the members page with state management, data transformation logic, and updated UI rendering for displaying batch and board members. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MembersPage as Members Page
participant BoardAPI as /api/board
participant BatchAPI as /api/members/batch
participant ExternalAPI as External API
User->>MembersPage: Page loads / Expand Board
MembersPage->>BoardAPI: GET /api/board?termStartYears=...
BoardAPI->>ExternalAPI: Fetch board data
ExternalAPI-->>BoardAPI: Board JSON response
BoardAPI->>BoardAPI: Parse & transform to Members
BoardAPI-->>MembersPage: Member list JSON
MembersPage->>MembersPage: Normalize roles, match by alias, update state
MembersPage->>User: Render board member cards
User->>MembersPage: Expand batch
MembersPage->>BatchAPI: GET /api/members/batch/[batchId]
BatchAPI->>ExternalAPI: Fetch members for batch
ExternalAPI-->>BatchAPI: Members JSON response
BatchAPI->>BatchAPI: Transform to typed Members
BatchAPI->>BatchAPI: Normalize image URLs
BatchAPI-->>MembersPage: Member array JSON
MembersPage->>MembersPage: Set batchMembers state
MembersPage->>User: Render batch member grid
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✨ Simplify code
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: 6
🧹 Nitpick comments (4)
app/members/page.tsx (2)
905-913: Inconsistent fallback rendering in advisory board vs main board.The advisory board section renders an empty
<div>whenprofileImageis missing (line 912), while the main board section (lines 798-800) displays initials. AlthoughshowAdvisoryBoardis currentlyfalse, this inconsistency should be addressed for future use.Proposed fix for consistency
{member.profileImage ? ( <img src={member.profileImage} alt={member.name} className="w-full h-64 object-cover" /> ) : ( - <div className="w-full h-64 bg-[`#00002c`]" /> + <div className="w-full h-64 flex items-center justify-center bg-[`#00002c`] text-white text-4xl font-black"> + {getInitials(member.name)} + </div> )}Apply similar fix to lines 940-942.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/members/page.tsx` around lines 905 - 913, The advisory board card currently uses an empty <div> when member.profileImage is missing; update the JSX that checks member.profileImage (the block rendering the empty div) to render the same initials fallback used in the main board instead of the empty div — compute initials from member.name and render the same styled element/classNames used for the main board initials so both boards are consistent (also apply this same change to the similar fallback at the other advisory-board location referenced around the later check for profileImage).
345-353: Sequential board loading could be parallelized.The
loadAllBoardsfunction awaits each board sequentially. With only 2 boards this is negligible, but for scalability consider usingPromise.all.Parallel loading
useEffect(() => { const loadAllBoards = async () => { - for (const board of boards) { - await loadBoardMembers(board.id) - } + await Promise.all(boards.map(board => loadBoardMembers(board.id))) } void loadAllBoards() }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/members/page.tsx` around lines 345 - 353, The current loadAllBoards function inside the useEffect is awaiting each board sequentially; change it to run in parallel by replacing the for-loop with a Promise.all over boards.map(board => loadBoardMembers(board.id)) and await that Promise.all, ensuring this change is made inside the same useEffect and referencing loadAllBoards, loadBoardMembers, boards and useEffect; also consider whether boards should be added to the effect dependency array if boards can change.app/api/members/batch/[batchId]/route.ts (2)
91-96: Logic gap: code continues to transform emptydataMemberswhen fallback is also empty.When
dataMembersis empty andfallbackMembersalso has zero length, the code falls through to the transformation at line 164, which will return an empty array. This is fine but the conditional structure is confusing. IffallbackMembersis empty, perhaps return early with[]or let it continue explicitly.Suggested clarification
if (!Array.isArray(dataMembers) || dataMembers.length === 0) { const fallbackMembers = getMembersByBatch(batchId) - if (fallbackMembers.length > 0) { - return NextResponse.json(fallbackMembers) - } + // Always return fallback when API returns no data + return NextResponse.json(fallbackMembers) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/members/batch/`[batchId]/route.ts around lines 91 - 96, The branch that handles an empty or non-array dataMembers calls getMembersByBatch(batchId) and returns fallbackMembers only if non-empty, otherwise it falls through to the later transformation causing confusing control flow; change the if-block so that when Array.isArray(dataMembers) is false or dataMembers.length === 0 you call getMembersByBatch(batchId) and if fallbackMembers.length > 0 return NextResponse.json(fallbackMembers) else return NextResponse.json([]) immediately (i.e., return early) instead of letting execution continue to the transformation logic that maps members.
182-185: Inconsistent error handling: catch returns empty array while non-OK response returns fallback.When the upstream responds with a non-2xx status (line 82-83), fallback data is returned. However, when an exception occurs (e.g., network error, JSON parse failure), an empty array is returned. Consider returning fallback data here as well for consistency.
Proposed fix
} catch (error) { console.error('Error fetching batch members:', error) - // Return empty array on error - return NextResponse.json([]) + // Fallback to mock members on error + const fallbackMembers = getMembersByBatch(batchId) + return NextResponse.json(fallbackMembers) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/members/batch/`[batchId]/route.ts around lines 182 - 185, The catch block in app/api/members/batch/[batchId]/route.ts currently returns NextResponse.json([]) on exceptions; change it to return the same fallback response used in the non-OK response branch so both error paths are consistent. Locate the catch that logs "Error fetching batch members" and replace the empty-array response with the same fallback variable or construct used in the earlier response.ok check (i.e., return the identical NextResponse.json(...) payload that the non-2xx branch returns). Ensure you keep the console.error logging but unify the returned payload across both the response.ok false branch and the catch block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/board/route.ts`:
- Line 9: The code currently sets const API_KEY =
process.env.STARTMUNICH_API_KEY || 'YOUR_API_KEY_PLACEHOLDER', which masks
missing configuration; change this to fail fast by checking
process.env.STARTMUNICH_API_KEY and either throw an Error or log a clear error
and return a 5xx response during initialization/use instead of using the
placeholder. Locate the API_KEY constant and replace the fallback behavior with
an explicit check that raises an exception (or logs and halts request handling)
when STARTMUNICH_API_KEY is absent so consumers cannot proceed with the
placeholder value.
In `@app/api/members/batch/`[batchId]/route.ts:
- Line 66: Remove the hardcoded fallback API key by not assigning
'YOUR_API_KEY_PLACEHOLDER' to API_KEY; instead read
process.env.STARTMUNICH_API_KEY into API_KEY and validate it at the start of the
route handler (the GET/POST export in app/api/members/batch/[batchId]/route.ts).
If API_KEY is missing or empty, return an appropriate HTTP error response (e.g.,
401/500 with a clear message) before making any downstream requests, and ensure
any logs reference API_KEY absence for easier debugging.
- Line 172: The object construction for linkedinUrl contains a duplicate access
to member.linkedinUrl; remove the redundant final `member.linkedinUrl` and keep
a clear fallback chain (e.g., assign linkedinUrl using member.linkedinUrl ||
member.linkedin || member.linkedin_profile || null) so the value resolution is
correct and not repeated; update the expression in the code that builds the
member object (the property labeled linkedinUrl) accordingly.
In `@app/members/page.tsx`:
- Around line 120-130: Remove the temporary debug console.log calls in
app/members/page.tsx that inspect the API response and image URLs (the logs
around "API Response data:", "First member imageUrl:", "Checking if
placeholder:", and "Transformed data:"); keep the transformation logic that maps
Member to add profileImage using isPlaceholderImage but delete the console.log
statements so only the const transformedData = data.map((member: Member) => ({
... })) and related production code remain.
- Around line 1036-1039: The anchor currently uses href="#" when
member.linkedinUrl is missing, causing page scroll-to-top; update the JSX around
the element that uses member.linkedinUrl so you conditionally render a real <a>
when member.linkedinUrl is truthy and render a non-anchor element (e.g., <div>
or <span>) with the exact same classes/structure when falsy, preserving
cursor/default styles and accessibility (role/aria-disabled if needed), or
alternatively attach an onClick that calls event.preventDefault() when rendering
the anchor without a real URL; target symbols to change: the JSX element that
sets href={member.linkedinUrl || '#'} and the conditional use of
member.linkedinUrl for target/rel/className.
- Around line 339-353: The first useEffect that calls loadBoardMembers should
include boards in its dependency array to avoid a stale closure—change its
dependencies to [expandedBoard, boards]; the second useEffect that defines
loadAllBoards currently has an empty dependency array but references boards, so
either document the intent to run only once or change its dependency to [boards]
so it re-runs when boards updates (the symbols to modify are loadAllBoards,
loadBoardMembers and the useEffect wrappers); finally remove the temporary debug
console.log statements scattered in this file (the debug logs that were added
around earlier board loading logic) before merging.
---
Nitpick comments:
In `@app/api/members/batch/`[batchId]/route.ts:
- Around line 91-96: The branch that handles an empty or non-array dataMembers
calls getMembersByBatch(batchId) and returns fallbackMembers only if non-empty,
otherwise it falls through to the later transformation causing confusing control
flow; change the if-block so that when Array.isArray(dataMembers) is false or
dataMembers.length === 0 you call getMembersByBatch(batchId) and if
fallbackMembers.length > 0 return NextResponse.json(fallbackMembers) else return
NextResponse.json([]) immediately (i.e., return early) instead of letting
execution continue to the transformation logic that maps members.
- Around line 182-185: The catch block in
app/api/members/batch/[batchId]/route.ts currently returns NextResponse.json([])
on exceptions; change it to return the same fallback response used in the non-OK
response branch so both error paths are consistent. Locate the catch that logs
"Error fetching batch members" and replace the empty-array response with the
same fallback variable or construct used in the earlier response.ok check (i.e.,
return the identical NextResponse.json(...) payload that the non-2xx branch
returns). Ensure you keep the console.error logging but unify the returned
payload across both the response.ok false branch and the catch block.
In `@app/members/page.tsx`:
- Around line 905-913: The advisory board card currently uses an empty <div>
when member.profileImage is missing; update the JSX that checks
member.profileImage (the block rendering the empty div) to render the same
initials fallback used in the main board instead of the empty div — compute
initials from member.name and render the same styled element/classNames used for
the main board initials so both boards are consistent (also apply this same
change to the similar fallback at the other advisory-board location referenced
around the later check for profileImage).
- Around line 345-353: The current loadAllBoards function inside the useEffect
is awaiting each board sequentially; change it to run in parallel by replacing
the for-loop with a Promise.all over boards.map(board =>
loadBoardMembers(board.id)) and await that Promise.all, ensuring this change is
made inside the same useEffect and referencing loadAllBoards, loadBoardMembers,
boards and useEffect; also consider whether boards should be added to the effect
dependency array if boards can change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c38041fa-f029-44fc-9e8d-38e8fe3bd175
⛔ Files ignored due to path filters (9)
public/images/batches_group_pictures/SS22.jpgis excluded by!**/*.jpgpublic/images/batches_group_pictures/SS23.JPGis excluded by!**/*.jpgpublic/images/batches_group_pictures/SS24.jpgis excluded by!**/*.jpgpublic/images/batches_group_pictures/SS25.jpgis excluded by!**/*.jpgpublic/images/batches_group_pictures/WS21.jpgis excluded by!**/*.jpgpublic/images/batches_group_pictures/WS22.jpgis excluded by!**/*.jpgpublic/images/batches_group_pictures/WS23.JPGis excluded by!**/*.jpgpublic/images/batches_group_pictures/WS24.JPGis excluded by!**/*.jpgpublic/images/batches_group_pictures/WS25.JPGis excluded by!**/*.jpg
📒 Files selected for processing (3)
app/api/board/route.tsapp/api/members/batch/[batchId]/route.tsapp/members/page.tsx
| const url = new URL(request.url) | ||
| const termStartYears = url.searchParams.get('termStartYears') || '2024,2025' | ||
|
|
||
| const API_KEY = process.env.STARTMUNICH_API_KEY || 'YOUR_API_KEY_PLACEHOLDER' |
There was a problem hiding this comment.
API key fallback masks configuration errors and may leak placeholder.
If STARTMUNICH_API_KEY is not set, requests proceed with 'YOUR_API_KEY_PLACEHOLDER' which will fail at the upstream API. This silently hides a configuration issue. Consider failing fast or logging a warning when the key is missing.
Proposed fix
- const API_KEY = process.env.STARTMUNICH_API_KEY || 'YOUR_API_KEY_PLACEHOLDER'
+ const API_KEY = process.env.STARTMUNICH_API_KEY
+ if (!API_KEY) {
+ console.error('STARTMUNICH_API_KEY environment variable is not set')
+ return NextResponse.json({ error: 'Server configuration error' }, { status: 500 })
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const API_KEY = process.env.STARTMUNICH_API_KEY || 'YOUR_API_KEY_PLACEHOLDER' | |
| const API_KEY = process.env.STARTMUNICH_API_KEY | |
| if (!API_KEY) { | |
| console.error('STARTMUNICH_API_KEY environment variable is not set') | |
| return NextResponse.json({ error: 'Server configuration error' }, { status: 500 }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/board/route.ts` at line 9, The code currently sets const API_KEY =
process.env.STARTMUNICH_API_KEY || 'YOUR_API_KEY_PLACEHOLDER', which masks
missing configuration; change this to fail fast by checking
process.env.STARTMUNICH_API_KEY and either throw an Error or log a clear error
and return a 5xx response during initialization/use instead of using the
placeholder. Locate the API_KEY constant and replace the fallback behavior with
an explicit check that raises an exception (or logs and halts request handling)
when STARTMUNICH_API_KEY is absent so consumers cannot proceed with the
placeholder value.
|
|
||
| const apiBatch = resolveApiBatch(batchId) | ||
|
|
||
| const API_KEY = process.env.STARTMUNICH_API_KEY || 'YOUR_API_KEY_PLACEHOLDER' |
There was a problem hiding this comment.
Same API key fallback issue as the board route.
Consider validating the presence of the API key and returning an appropriate error, similar to the suggestion for app/api/board/route.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/members/batch/`[batchId]/route.ts at line 66, Remove the hardcoded
fallback API key by not assigning 'YOUR_API_KEY_PLACEHOLDER' to API_KEY; instead
read process.env.STARTMUNICH_API_KEY into API_KEY and validate it at the start
of the route handler (the GET/POST export in
app/api/members/batch/[batchId]/route.ts). If API_KEY is missing or empty,
return an appropriate HTTP error response (e.g., 401/500 with a clear message)
before making any downstream requests, and ensure any logs reference API_KEY
absence for easier debugging.
| study: member.study || member.field || member.degree, | ||
| university: member.university || member.school, | ||
| company: member.company || member.employer, | ||
| linkedinUrl: member.linkedinUrl || member.linkedin || member.linkedin_profile || member.linkedinUrl || null, |
There was a problem hiding this comment.
Duplicate field access: member.linkedinUrl appears twice.
This looks like a copy-paste oversight. The second member.linkedinUrl at the end is redundant.
Proposed fix
- linkedinUrl: member.linkedinUrl || member.linkedin || member.linkedin_profile || member.linkedinUrl || null,
+ linkedinUrl: member.linkedinUrl || member.linkedin || member.linkedin_profile || null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| linkedinUrl: member.linkedinUrl || member.linkedin || member.linkedin_profile || member.linkedinUrl || null, | |
| linkedinUrl: member.linkedinUrl || member.linkedin || member.linkedin_profile || null, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/members/batch/`[batchId]/route.ts at line 172, The object
construction for linkedinUrl contains a duplicate access to member.linkedinUrl;
remove the redundant final `member.linkedinUrl` and keep a clear fallback chain
(e.g., assign linkedinUrl using member.linkedinUrl || member.linkedin ||
member.linkedin_profile || null) so the value resolution is correct and not
repeated; update the expression in the code that builds the member object (the
property labeled linkedinUrl) accordingly.
Comment on lines
+120
to
+130
| console.log('API Response data:', data) | ||
| if (Array.isArray(data) && data.length > 0) { | ||
| // Log first few members to check imageUrl | ||
| console.log('First member imageUrl:', data[0]?.imageUrl) | ||
| console.log('Checking if placeholder:', isPlaceholderImage(data[0]?.imageUrl)) | ||
| // Transform API response to add profileImage field (clean up placeholder images) | ||
| const transformedData = data.map((member: Member) => ({ | ||
| ...member, | ||
| profileImage: isPlaceholderImage(member.imageUrl) ? undefined : member.imageUrl | ||
| })) | ||
| console.log('Transformed data:', transformedData) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statements before merging.
Multiple console.log calls for debugging API responses and image URLs should be removed for production code.
Proposed fix
if (Array.isArray(data) && data.length > 0) {
- // Log first few members to check imageUrl
- console.log('First member imageUrl:', data[0]?.imageUrl)
- console.log('Checking if placeholder:', isPlaceholderImage(data[0]?.imageUrl))
// Transform API response to add profileImage field (clean up placeholder images)
const transformedData = data.map((member: Member) => ({
...member,
profileImage: isPlaceholderImage(member.imageUrl) ? undefined : member.imageUrl
}))
- console.log('Transformed data:', transformedData)
setBatchMembers(transformedData)Also remove line 120:
- console.log('API Response data:', data)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| console.log('API Response data:', data) | |
| if (Array.isArray(data) && data.length > 0) { | |
| // Log first few members to check imageUrl | |
| console.log('First member imageUrl:', data[0]?.imageUrl) | |
| console.log('Checking if placeholder:', isPlaceholderImage(data[0]?.imageUrl)) | |
| // Transform API response to add profileImage field (clean up placeholder images) | |
| const transformedData = data.map((member: Member) => ({ | |
| ...member, | |
| profileImage: isPlaceholderImage(member.imageUrl) ? undefined : member.imageUrl | |
| })) | |
| console.log('Transformed data:', transformedData) | |
| if (Array.isArray(data) && data.length > 0) { | |
| // Transform API response to add profileImage field (clean up placeholder images) | |
| const transformedData = data.map((member: Member) => ({ | |
| ...member, | |
| profileImage: isPlaceholderImage(member.imageUrl) ? undefined : member.imageUrl | |
| })) | |
| setBatchMembers(transformedData) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/members/page.tsx` around lines 120 - 130, Remove the temporary debug
console.log calls in app/members/page.tsx that inspect the API response and
image URLs (the logs around "API Response data:", "First member imageUrl:",
"Checking if placeholder:", and "Transformed data:"); keep the transformation
logic that maps Member to add profileImage using isPlaceholderImage but delete
the console.log statements so only the const transformedData = data.map((member:
Member) => ({ ... })) and related production code remain.
Comment on lines
+339
to
+353
| useEffect(() => { | ||
| if (expandedBoard) { | ||
| loadBoardMembers(expandedBoard) | ||
| } | ||
| }, [expandedBoard]) | ||
|
|
||
| useEffect(() => { | ||
| const loadAllBoards = async () => { | ||
| for (const board of boards) { | ||
| await loadBoardMembers(board.id) | ||
| } | ||
| } | ||
|
|
||
| void loadAllBoards() | ||
| }, []) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are eslint rules for exhaustive-deps that might catch this
fd -e json -e js "eslint" --max-depth 2 --exec cat {} 2>/dev/null | grep -i "exhaustive-deps" || echo "No eslint config found or rule not configured"Repository: startmunich/website-special-pages
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Get the full context around lines 339-360 and the loadBoardMembers function
wc -l app/members/page.tsxRepository: startmunich/website-special-pages
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Read the problematic section
sed -n '330,365p' app/members/page.tsxRepository: startmunich/website-special-pages
Length of output: 954
🏁 Script executed:
#!/bin/bash
# Find the loadBoardMembers function definition
rg -n "loadBoardMembers" app/members/page.tsx | head -20Repository: startmunich/website-special-pages
Length of output: 223
🏁 Script executed:
#!/bin/bash
# Get the loadBoardMembers function implementation (lines 239+)
sed -n '239,320p' app/members/page.tsxRepository: startmunich/website-special-pages
Length of output: 2897
🏁 Script executed:
#!/bin/bash
# Check the context around the loadBoardMembers function to see if it's declared before or after boards state
sed -n '235,242p' app/members/page.tsxRepository: startmunich/website-special-pages
Length of output: 247
🏁 Script executed:
#!/bin/bash
# Check where setBoards is called to see if boards state changes after initial load
rg -n "setBoards" app/members/page.tsx | head -10Repository: startmunich/website-special-pages
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Get broader context around the board initialization and effects
sed -n '60,75p' app/members/page.tsxRepository: startmunich/website-special-pages
Length of output: 701
🏁 Script executed:
#!/bin/bash
# Check how many times setBoards is called and in what context
rg -B 2 -A 2 "setBoards" app/members/page.tsx | head -40Repository: startmunich/website-special-pages
Length of output: 334
🏁 Script executed:
#!/bin/bash
# Check for console.log statements in the file
rg -n "console\.(log|warn|error)" app/members/page.tsx | grep -v "console.warn\|console.error"Repository: startmunich/website-special-pages
Length of output: 372
🏁 Script executed:
#!/bin/bash
# Check lines around 120-124, 130 mentioned in scratchpad for console.log
sed -n '115,135p' app/members/page.tsxRepository: startmunich/website-special-pages
Length of output: 1227
🏁 Script executed:
#!/bin/bash
# Verify the dependency array issue by checking if boards state could change
# Look for scenarios where boards is updated and expandedBoard might also be active
rg -n "setExpandedBoard" app/members/page.tsxRepository: startmunich/website-special-pages
Length of output: 413
🏁 Script executed:
#!/bin/bash
# Check the entire useEffect blocks and their full context
sed -n '337,360p' app/members/page.tsxRepository: startmunich/website-special-pages
Length of output: 538
Add missing dependencies to useEffect hooks and remove debug logs.
The first useEffect (lines 339-344) calls loadBoardMembers, which reads from the boards state via closure. Since boards can change (updated at line 305), missing boards from the dependency array creates a stale closure—when expandedBoard changes after boards has been updated, the effect will use an outdated reference. Add boards to the dependency array: [expandedBoard, boards].
The second useEffect (lines 345-353) has empty dependencies [] but references boards through loadAllBoards. This is less critical if the intent is to load all boards only once on mount, but the pattern is fragile and should be documented or adjusted to [boards] if subsequent board updates should trigger reloads.
Additionally, remove the debug console.log statements at lines 120, 123, 124, and 130 before merging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/members/page.tsx` around lines 339 - 353, The first useEffect that calls
loadBoardMembers should include boards in its dependency array to avoid a stale
closure—change its dependencies to [expandedBoard, boards]; the second useEffect
that defines loadAllBoards currently has an empty dependency array but
references boards, so either document the intent to run only once or change its
dependency to [boards] so it re-runs when boards updates (the symbols to modify
are loadAllBoards, loadBoardMembers and the useEffect wrappers); finally remove
the temporary debug console.log statements scattered in this file (the debug
logs that were added around earlier board loading logic) before merging.
Comment on lines
+1036
to
+1039
| href={member.linkedinUrl || '#'} | ||
| target={member.linkedinUrl ? "_blank" : undefined} | ||
| rel={member.linkedinUrl ? "noopener noreferrer" : undefined} | ||
| className={`group relative overflow-hidden transition-all duration-300 bg-white/5 hover:bg-white/10 border border-white/10 hover:border-[#d0006f] rounded-lg hover:scale-105 ${member.linkedinUrl ? 'cursor-pointer' : 'cursor-default'} z-10 aspect-square`} |
There was a problem hiding this comment.
Anchor with href="#" scrolls to top when linkedinUrl is missing.
When member.linkedinUrl is falsy, the anchor gets href="#" which causes the page to scroll to top on click. Consider using a <div> or <span> for non-linked members, or use event.preventDefault().
Proposed fix using conditional rendering
- {batchMembers.map((member) => (
- <a
- key={member.id}
- href={member.linkedinUrl || '#'}
- target={member.linkedinUrl ? "_blank" : undefined}
- rel={member.linkedinUrl ? "noopener noreferrer" : undefined}
- className={`group relative overflow-hidden transition-all duration-300 bg-white/5 hover:bg-white/10 border border-white/10 hover:border-[`#d0006f`] rounded-lg hover:scale-105 ${member.linkedinUrl ? 'cursor-pointer' : 'cursor-default'} z-10 aspect-square`}
- >
+ {batchMembers.map((member) => {
+ const CardWrapper = member.linkedinUrl ? 'a' : 'div'
+ const linkProps = member.linkedinUrl
+ ? { href: member.linkedinUrl, target: "_blank", rel: "noopener noreferrer" }
+ : {}
+ return (
+ <CardWrapper
+ key={member.id}
+ {...linkProps}
+ className={`group relative overflow-hidden transition-all duration-300 bg-white/5 hover:bg-white/10 border border-white/10 hover:border-[`#d0006f`] rounded-lg hover:scale-105 ${member.linkedinUrl ? 'cursor-pointer' : 'cursor-default'} z-10 aspect-square`}
+ >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/members/page.tsx` around lines 1036 - 1039, The anchor currently uses
href="#" when member.linkedinUrl is missing, causing page scroll-to-top; update
the JSX around the element that uses member.linkedinUrl so you conditionally
render a real <a> when member.linkedinUrl is truthy and render a non-anchor
element (e.g., <div> or <span>) with the exact same classes/structure when
falsy, preserving cursor/default styles and accessibility (role/aria-disabled if
needed), or alternatively attach an onClick that calls event.preventDefault()
when rendering the anchor without a real URL; target symbols to change: the JSX
element that sets href={member.linkedinUrl || '#'} and the conditional use of
member.linkedinUrl for target/rel/className.
This was referenced Mar 28, 2026
Merged
Open
Merged
Merged
Merged
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
2 participants
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.
Member website - First version merge
Summary by CodeRabbit
New Features
Improvements