Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR consolidates partner data fetching into a new utility ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
lib/partners.ts (1)
32-66: Error handling silently returns empty array for all failure modes.
getAllPartners()returns[]for config errors, HTTP errors, and exceptions alike. Consumers (likeapp/api/partners/route.ts) cannot distinguish between "NocoDB returned zero partners" and "NocoDB is down." The route handler's check on lines 16-21 only catches the missing config case, not runtime failures.Consider returning a result object or throwing for actual errors so callers can respond appropriately (e.g., return 500 vs 200 with empty data).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/partners.ts` around lines 32 - 66, getAllPartners currently swallows all failures and returns [] so callers can't differentiate "no partners" from runtime/config errors; change getAllPartners to surface errors instead of always returning an empty array—either by throwing on config checks, non-2xx HTTP responses, and caught exceptions (e.g., throw new Error with the status/details) or by returning a discriminated result object like { success: boolean, data?: Partner[], error?: string } so callers (e.g., app/api/partners/route.ts) can map config errors to 400/500 and runtime failures to 500 while still returning 200 for an actual empty data array; implement the chosen approach in getAllPartners and update any callers to handle the thrown errors or inspect the result object accordingly.app/members/page.tsx (1)
540-555: Consider using a non-navigating element when no LinkedIn URL exists.Using
href="#"whenlinkedinUrlis falsy will scroll the page to the top on click, despite thecursor-defaultstyling. Consider using a<div>or<span>instead of<a>when there's no link, or usehref="javascript:void(0)"/ addonClick={(e) => e.preventDefault()}.🔧 Alternative approach using conditional element
{member.linkedinUrl ? ( <a href={member.linkedinUrl} target="_blank" rel="noopener noreferrer" className="relative cursor-pointer"> {/* card content */} </a> ) : ( <div className="relative cursor-default"> {/* card content */} </div> )}Or extract the card content to avoid duplication.
Also applies to: 568-583
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/members/page.tsx` around lines 540 - 555, The anchor uses href="#" when member.linkedinUrl is falsy causing unintended navigation; update the JSX around member.linkedinUrl (the element wrapping the card that currently uses <a key={i} href={member.linkedinUrl || '#'} …>) to conditionally render an <a> only when member.linkedinUrl exists and otherwise render a non-navigating element (e.g., <div> or <span>) with the same class names and content (the card including the Image, getInitials fallback, and the name/role blocks) to avoid page scroll; alternatively, if you prefer one element, remove href="#" and add onClick={(e) => e.preventDefault()} when member.linkedinUrl is falsy, and keep target/rel only when linkedinUrl is present.app/api/partners/route.ts (1)
7-12: Consider removing or reducing verbose debug logging in production.These console logs expose partial API tokens and configuration details. While the token is truncated, logging this on every request could clutter production logs and potentially leak sensitive information.
🔧 Suggested improvement
export async function GET() { - console.log('========== FETCHING ALL PARTNERS =========='); - console.log('🔍 Environment Variables:'); - const token = process.env.NOCODB_API_TOKEN; - console.log('NOCODB_API_TOKEN:', token ? `${token.substring(0, 10)}... (${token.length} chars)` : 'NOT SET'); - console.log('NOCODB_BASE_URL:', process.env.NOCODB_BASE_URL || 'https://ndb.startmunich.de'); - console.log('NOCODB_PARTNERS_TABLE_ID:', process.env.NOCODB_PARTNERS_TABLE_ID); - const partners = await getAllPartners();If debugging is needed, consider using a debug flag or
NODE_ENVcheck.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/partners/route.ts` around lines 7 - 12, The console.log statements in app/api/partners/route.ts (the "========== FETCHING ALL PARTNERS ==========" block) emit sensitive/config data on every request; update that block to remove or gate verbose logging behind a debug flag or NODE_ENV check (e.g., only log when process.env.DEBUG_PARTNERS === 'true' or process.env.NODE_ENV !== 'production'), stop printing any API token fragments, and replace with minimal safe logs (like "Fetching partners" and conditional detailed debug info) so that normal production requests do not leak or clutter logs.
🤖 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/about-us/page.tsx`:
- Around line 38-39: Replace the abrupt sentence "However START Munich and CDTM
/ M&M differ." with a smoother connective and adjust the following sentence for
flow; for example, change it to "However, START Munich differs from CDTM and
M&M: we're a community that emphasizes learning by doing, while those are
structured educational programs." Update the array entry containing that text
(the object with pink: true) and ensure the next object remains a separate
sentence about members joining both programs.
In `@app/api/partners/route.ts`:
- Around line 14-24: The endpoint in app/api/partners/route.ts currently treats
an empty partners array as success and only errors when env vars are missing;
change the flow so you first validate NOCODB_API_TOKEN and
NOCODB_PARTNERS_TABLE_ID and return 500 if missing, then call getAllPartners(),
and if getAllPartners() returns an empty array while config is present, log an
error and return a 500 (or propagate the actual error). Also update
lib/partners.ts to stop swallowing upstream errors from the NocoDB call (let
exceptions bubble or return error details) so the route can respond with proper
non-200 status when NocoDB fails; reference getAllPartners and the handler in
app/api/partners/route.ts.
In `@app/home/page.tsx`:
- Around line 45-51: The partner logo selection is inconsistent:
app/home/page.tsx chooses the last uploaded logo (logos[logos.length - 1]) while
lib/partners.ts currently uses the first (Logo[0]); update lib/partners.ts to
mirror app/home/page.tsx by selecting the last element of the Logo array (use
Logo[Logo.length - 1] or equivalent) and guard for empty/null Logo arrays so the
fallback avatar logic remains used—ensure the same signedPath/URL construction
logic is applied so both endpoints return the same logo for a partner.
In `@lib/partners.ts`:
- Around line 18-20: The code hardcodes "https://ndb.startmunich.de/" when
constructing logoUrl instead of using the existing NOCODB_BASE_URL constant;
update the assignment that sets logoUrl (the block using logo.signedPath) to
build the URL from NOCODB_BASE_URL and the signedPath (e.g., join
NOCODB_BASE_URL and logo.signedPath while avoiding duplicate slashes or using
the URL constructor) so logoUrl uses NOCODB_BASE_URL consistently.
- Around line 23-26: The mapping in lib/partners.ts uses the misspelled field
record.Categrory and a non-deterministic fallback id String(Math.random());
first verify the NocoDB table schema and correct the field access to the actual
column name (change record.Categrory to record.Category if the schema uses
"Category", or defensively read both record.Category || record.Categrory to
handle a misspelling at source), and second replace the unstable id fallback in
the partner mapping (and the similar use in app/home/page.tsx) with a stable
value — prefer record.Id when present, otherwise derive a deterministic hash
from record.Name (or another stable field) instead of Math.random() so
server/client renders remain consistent.
---
Nitpick comments:
In `@app/api/partners/route.ts`:
- Around line 7-12: The console.log statements in app/api/partners/route.ts (the
"========== FETCHING ALL PARTNERS ==========" block) emit sensitive/config data
on every request; update that block to remove or gate verbose logging behind a
debug flag or NODE_ENV check (e.g., only log when process.env.DEBUG_PARTNERS ===
'true' or process.env.NODE_ENV !== 'production'), stop printing any API token
fragments, and replace with minimal safe logs (like "Fetching partners" and
conditional detailed debug info) so that normal production requests do not leak
or clutter logs.
In `@app/members/page.tsx`:
- Around line 540-555: The anchor uses href="#" when member.linkedinUrl is falsy
causing unintended navigation; update the JSX around member.linkedinUrl (the
element wrapping the card that currently uses <a key={i}
href={member.linkedinUrl || '#'} …>) to conditionally render an <a> only when
member.linkedinUrl exists and otherwise render a non-navigating element (e.g.,
<div> or <span>) with the same class names and content (the card including the
Image, getInitials fallback, and the name/role blocks) to avoid page scroll;
alternatively, if you prefer one element, remove href="#" and add onClick={(e)
=> e.preventDefault()} when member.linkedinUrl is falsy, and keep target/rel
only when linkedinUrl is present.
In `@lib/partners.ts`:
- Around line 32-66: getAllPartners currently swallows all failures and returns
[] so callers can't differentiate "no partners" from runtime/config errors;
change getAllPartners to surface errors instead of always returning an empty
array—either by throwing on config checks, non-2xx HTTP responses, and caught
exceptions (e.g., throw new Error with the status/details) or by returning a
discriminated result object like { success: boolean, data?: Partner[], error?:
string } so callers (e.g., app/api/partners/route.ts) can map config errors to
400/500 and runtime failures to 500 while still returning 200 for an actual
empty data array; implement the chosen approach in getAllPartners and update any
callers to handle the thrown errors or inspect the result object accordingly.
🪄 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: 9fc0bd3e-ed52-48f8-8466-0b6bf79cff1f
⛔ Files ignored due to path filters (1)
public/ourMembers/boads/boad26.jpegis excluded by!**/*.jpeg
📒 Files selected for processing (13)
app/about-us/page.tsxapp/admin/page.tsxapp/api/partners/route.tsapp/api/startups/[id]/route.tsapp/api/startups/add/route.tsapp/for-partners/page.tsxapp/home/HomeClient.tsxapp/home/page.tsxapp/members/page.tsxapp/page.tsxlib/partners.tsscripts/landing_page_proposal.htmltsconfig.tsbuildinfo
💤 Files with no reviewable changes (3)
- app/api/startups/add/route.ts
- app/admin/page.tsx
- app/api/startups/[id]/route.ts
| { text: "However START Munich and CDTM / M&M differ. We are a community that encourages learning by doing, while they are educational programs that provide structured curriculum. ", pink: true }, | ||
| { text: "Some of our members also join CDTM or M&M, but handling the intensive time commitment for both is challenging." }, |
There was a problem hiding this comment.
Polish wording for readability and tone consistency.
The new copy is clear in intent, but “However START Munich and CDTM / M&M differ.” reads a bit abrupt. Consider tightening the phrasing to improve flow.
Suggested copy tweak
- { text: "However START Munich and CDTM / M&M differ. We are a community that encourages learning by doing, while they are educational programs that provide structured curriculum. ", pink: true },
- { text: "Some of our members also join CDTM or M&M, but handling the intensive time commitment for both is challenging." },
+ { text: "However, START Munich and CDTM / M&M differ in format: we are a community that encourages learning by doing, while they are educational programs with a structured curriculum. ", pink: true },
+ { text: "Some of our members also join CDTM or M&M, but balancing the intensive time commitment of both is challenging." },📝 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.
| { text: "However START Munich and CDTM / M&M differ. We are a community that encourages learning by doing, while they are educational programs that provide structured curriculum. ", pink: true }, | |
| { text: "Some of our members also join CDTM or M&M, but handling the intensive time commitment for both is challenging." }, | |
| { text: "However, START Munich and CDTM / M&M differ in format: we are a community that encourages learning by doing, while they are educational programs with a structured curriculum. ", pink: true }, | |
| { text: "Some of our members also join CDTM or M&M, but balancing the intensive time commitment of both is challenging." }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/about-us/page.tsx` around lines 38 - 39, Replace the abrupt sentence
"However START Munich and CDTM / M&M differ." with a smoother connective and
adjust the following sentence for flow; for example, change it to "However,
START Munich differs from CDTM and M&M: we're a community that emphasizes
learning by doing, while those are structured educational programs." Update the
array entry containing that text (the object with pink: true) and ensure the
next object remains a separate sentence about members joining both programs.
| const partners = await getAllPartners(); | ||
|
|
||
| // Check if NocoDB is configured | ||
| if (!NOCODB_API_TOKEN || !NOCODB_PARTNERS_TABLE_ID) { | ||
| console.error('NocoDB not configured - missing API token or partners table ID'); | ||
| if (partners.length === 0 && (!process.env.NOCODB_API_TOKEN || !process.env.NOCODB_PARTNERS_TABLE_ID)) { | ||
| return NextResponse.json( | ||
| { error: 'NocoDB not configured for partners' }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| console.log('Fetching partners from NocoDB...'); | ||
|
|
||
| const response = await fetch( | ||
| `${NOCODB_BASE_URL}/api/v2/tables/${NOCODB_PARTNERS_TABLE_ID}/records?limit=1000&offset=0`, | ||
| { | ||
| headers: { | ||
| 'xc-token': NOCODB_API_TOKEN, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| next: { revalidate: 3600 }, | ||
| } | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| const errorText = await response.text(); | ||
| console.error(`NocoDB API error: ${response.status} ${response.statusText}`, errorText); | ||
| throw new Error(`NocoDB API error: ${response.status} ${response.statusText}`); | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
|
|
||
| // Debug: Log the first raw record to see actual column names | ||
| if (data.list && data.list[0]) { | ||
| console.log('📋 Sample raw record from NocoDB:', JSON.stringify(data.list[0], null, 2)); | ||
| } | ||
|
|
||
| const partners = (data.list || []) | ||
| .filter((record: any) => { | ||
| // Check if 'Show' column is true, 1, or 'true' (case-insensitive) | ||
| const show = record.Show; | ||
| return show === true || show === 1 || String(show).toLowerCase() === 'true'; | ||
| }) | ||
| .map(transformNocoDBRecord); | ||
|
|
||
| console.log(`Successfully fetched ${partners.length} partners from NocoDB`); | ||
| return NextResponse.json(partners); | ||
|
|
||
| } catch (error) { | ||
| console.error('Error fetching from NocoDB:', error); | ||
| return NextResponse.json( | ||
| { error: 'Failed to fetch partners from database' }, | ||
| { status: 500 } | ||
| ); | ||
| } | ||
| console.log(`Successfully fetched ${partners.length} partners from NocoDB`); | ||
| return NextResponse.json(partners); |
There was a problem hiding this comment.
Error detection logic may mask upstream failures.
The current check only returns a 500 error when partners.length === 0 AND environment variables are missing. If NocoDB is configured but fails (network error, API error), getAllPartners() returns [] and this route returns 200 with an empty array—masking the actual failure from clients.
This ties back to the error-swallowing behavior in lib/partners.ts. If that's not addressed, consider at minimum logging when an empty result is returned despite valid config.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/partners/route.ts` around lines 14 - 24, The endpoint in
app/api/partners/route.ts currently treats an empty partners array as success
and only errors when env vars are missing; change the flow so you first validate
NOCODB_API_TOKEN and NOCODB_PARTNERS_TABLE_ID and return 500 if missing, then
call getAllPartners(), and if getAllPartners() returns an empty array while
config is present, log an error and return a 500 (or propagate the actual
error). Also update lib/partners.ts to stop swallowing upstream errors from the
NocoDB call (let exceptions bubble or return error details) so the route can
respond with proper non-200 status when NocoDB fails; reference getAllPartners
and the handler in app/api/partners/route.ts.
| const logos: any[] = r.Logo || [] | ||
| // Some partners have two images uploaded; always use the last one so that | ||
| // a newer/preferred logo can be added as a second upload without removing the original. | ||
| const logo = logos.length > 0 ? logos[logos.length - 1] : null | ||
| const logoUrl = logo?.signedPath | ||
| ? `${NOCODB_BASE_URL}/${logo.signedPath}` | ||
| : `https://ui-avatars.com/api/?name=${encodeURIComponent(r.Name || 'Partner')}&size=300&background=4f46e5&color=fff&bold=true&font-size=0.4` |
There was a problem hiding this comment.
Inconsistent logo selection logic across codebase.
This function picks the last logo (logos[logos.length - 1]) with the rationale that newer uploads should take precedence. However, lib/partners.ts:16-20 picks the first logo (Logo[0]). This divergence means the same partner could display different logos on /home vs /for-partners or the API.
Consider aligning both implementations to use the same selection strategy.
🔧 Option: Update lib/partners.ts to match this behavior
In lib/partners.ts:
if (record.Logo && Array.isArray(record.Logo) && record.Logo[0]) {
- const logo = record.Logo[0];
+ const logo = record.Logo[record.Logo.length - 1];
if (logo.signedPath) {
logoUrl = `${NOCODB_BASE_URL}/${logo.signedPath}`;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/home/page.tsx` around lines 45 - 51, The partner logo selection is
inconsistent: app/home/page.tsx chooses the last uploaded logo
(logos[logos.length - 1]) while lib/partners.ts currently uses the first
(Logo[0]); update lib/partners.ts to mirror app/home/page.tsx by selecting the
last element of the Logo array (use Logo[Logo.length - 1] or equivalent) and
guard for empty/null Logo arrays so the fallback avatar logic remains
used—ensure the same signedPath/URL construction logic is applied so both
endpoints return the same logo for a partner.
| if (logo.signedPath) { | ||
| logoUrl = `https://ndb.startmunich.de/${logo.signedPath}`; | ||
| } |
There was a problem hiding this comment.
Hardcoded base URL instead of using environment variable.
Line 19 hardcodes https://ndb.startmunich.de/ but NOCODB_BASE_URL is defined at the top of the file (line 2). This creates an inconsistency if the base URL ever changes.
🔧 Proposed fix
if (record.Logo && Array.isArray(record.Logo) && record.Logo[0]) {
const logo = record.Logo[0];
if (logo.signedPath) {
- logoUrl = `https://ndb.startmunich.de/${logo.signedPath}`;
+ logoUrl = `${NOCODB_BASE_URL}/${logo.signedPath}`;
}
}📝 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.
| if (logo.signedPath) { | |
| logoUrl = `https://ndb.startmunich.de/${logo.signedPath}`; | |
| } | |
| if (logo.signedPath) { | |
| logoUrl = `${NOCODB_BASE_URL}/${logo.signedPath}`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/partners.ts` around lines 18 - 20, The code hardcodes
"https://ndb.startmunich.de/" when constructing logoUrl instead of using the
existing NOCODB_BASE_URL constant; update the assignment that sets logoUrl (the
block using logo.signedPath) to build the URL from NOCODB_BASE_URL and the
signedPath (e.g., join NOCODB_BASE_URL and logo.signedPath while avoiding
duplicate slashes or using the URL constructor) so logoUrl uses NOCODB_BASE_URL
consistently.
| return { | ||
| id: record.Id || String(Math.random()), | ||
| name: record.Name || 'Unnamed Partner', | ||
| category: record.Categrory || 'Other', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for other usages of Categrory vs Category in the codebase
rg -n 'Categrory|Category' --type ts --type tsxRepository: startmunich/website-special-pages
Length of output: 104
🏁 Script executed:
#!/bin/bash
# Search for Categrory and Category usage without problematic file type
rg -n 'Categrory|Category' lib/Repository: startmunich/website-special-pages
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Also check the full context of lib/partners.ts around lines 23-26
wc -l lib/partners.tsRepository: startmunich/website-special-pages
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Read the relevant portion of lib/partners.ts
head -40 lib/partners.ts | tail -20Repository: startmunich/website-special-pages
Length of output: 773
🏁 Script executed:
#!/bin/bash
# Search entire codebase for Categrory
rg -n 'Categrory' .Repository: startmunich/website-special-pages
Length of output: 306
🏁 Script executed:
#!/bin/bash
# Check for any Partner type/interface definitions
rg -n 'interface Partner|type Partner|category' lib/partners.ts | head -20Repository: startmunich/website-special-pages
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Look for NocoDB-related configuration or field mappings
rg -n 'NOCODB|field|Field' lib/partners.tsRepository: startmunich/website-special-pages
Length of output: 514
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation about NocoDB field names
find . -type f \( -name "README*" -o -name "*.md" -o -name ".env*" \) | xargs grep -l "Categrory\|partner" 2>/dev/null | head -10Repository: startmunich/website-special-pages
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Look for the Partner interface definition to see full context
sed -n '5,12p' lib/partners.tsRepository: startmunich/website-special-pages
Length of output: 207
Verify NocoDB field name and address non-deterministic ID generation.
Two concerns:
-
Field name mismatch: The code accesses
record.Categrorywhile thePartnerinterface property iscategory. This pattern appears consistently across the codebase (also inapp/home/page.tsx:52), suggestingCategrorymay match the actual NocoDB field name. Verify this against the NocoDB table schema to confirm the field is indeed misspelled at the source or if it should be corrected toCategory. -
String(Math.random())produces non-deterministic IDs that could cause React hydration mismatches between server and client renders. This issue also appears inapp/home/page.tsx:52. Use a stable identifier such as a hash of the record's name or the record's ID value if available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/partners.ts` around lines 23 - 26, The mapping in lib/partners.ts uses
the misspelled field record.Categrory and a non-deterministic fallback id
String(Math.random()); first verify the NocoDB table schema and correct the
field access to the actual column name (change record.Categrory to
record.Category if the schema uses "Category", or defensively read both
record.Category || record.Categrory to handle a misspelling at source), and
second replace the unstable id fallback in the partner mapping (and the similar
use in app/home/page.tsx) with a stable value — prefer record.Id when present,
otherwise derive a deterministic hash from record.Name (or another stable field)
instead of Math.random() so server/client renders remain consistent.
1 participant
Summary by CodeRabbit
Release Notes
New Features
UI/Style Updates
Content Updates