Conversation
… mod merge into main
… mod merge into main
… mod merge into main
… organization-fixes merge into main
…into organization-fixes
… hackathon-details fix: merge into main
… hackathon-details Merge into main
… hackathon-analytics merge into main
… hackathon-analytics merge into main
… hackathon-analytics merge into main
…ouncement features
|
@Benjtalkshow is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds announcements (public detail page, admin CRUD, API), a major judging redesign (criteria, per-judge scores, judge management, results/winners), new link-validation hook, mobile-friendly sidebars, participant-type UI, scoring refactors, and multiple supporting type/API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Browser
participant HackathonPage as Hackathon Page
participant HackathonAPI as Hackathon API
participant AnnouncementAPI as Announcement API
Client->>HackathonPage: navigate to hackathon slug
HackathonPage->>HackathonAPI: fetch hackathon by slug
HackathonPage->>AnnouncementAPI: listAnnouncements(hackathonId)
HackathonAPI-->>HackathonPage: hackathon data
AnnouncementAPI-->>HackathonPage: announcements[]
HackathonPage->>HackathonPage: filter published, compute tab badge
Client->>HackathonPage: click announcement link
HackathonPage->>AnnouncementAPI: getAnnouncementDetails(announcementId)
AnnouncementAPI-->>Client: announcement with markdown
Client->>Client: render announcement page, convert markdown -> HTML
sequenceDiagram
participant Judge as Judge UI
participant JudgingPage as Judging Dashboard
participant JudgingAPI as Judging API
participant SubmissionAPI as Submission API
participant GradeModal as Grade Modal
Judge->>JudgingPage: open judging dashboard
JudgingPage->>SubmissionAPI: getJudgingSubmissions(status=SHORTLISTED)
JudgingPage->>JudgingAPI: getJudgingCriteria(hackathon), getHackathonJudges, getJudgingResults
SubmissionAPI-->>JudgingPage: submissions[]
JudgingAPI-->>JudgingPage: criteria, judges, results
Judge->>JudgingPage: open participant -> GradeModal
GradeModal->>JudgingAPI: getSubmissionScores(submissionId)
JudgingAPI-->>GradeModal: IndividualJudgeScore[] (existing scores)
Judge->>GradeModal: enter per-criterion scores & comments
GradeModal->>JudgingAPI: submitJudgingScore({ submissionId, criteriaScores, comment })
JudgingAPI-->>JudgingPage: success -> refresh results
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (19)
app/(landing)/projects/[slug]/page.tsx (1)
115-121:⚠️ Potential issue | 🟡 MinorDuplicate
notFound()guard.The identical
if (error || !project) { notFound(); }check appears twice in a row (lines 115–117 and 119–121). Remove one of them.Proposed fix
if (error || !project) { notFound(); } - if (error || !project) { - notFound(); - } - return (components/hackathons/participants/profileCard.tsx (2)
120-124:⚠️ Potential issue | 🟠 MajorRemove debug
console.logstatements before merging.Lines 120–124 and 154–161 contain
[DEBUG]console logs that leak internal state (usernames, user IDs, roles) to the browser console. These should be removed or replaced with a proper dev-only logger to avoid leaking information in production.Also applies to: 154-161
211-211:⚠️ Potential issue | 🟡 MinorNon-null assertion on
participant.joinedDate!risks a runtime crash.If
joinedDateisundefinedornull,formatJoinDatewill receiveundefined, andnew Date(undefined)produces anInvalid Date, rendering"undefined undefined"in the UI. Guard against this or provide a fallback.🛡️ Proposed fix
- Joined {formatJoinDate(participant.joinedDate!)} + {participant.joinedDate + ? `Joined ${formatJoinDate(participant.joinedDate)}` + : null}components/organization/OrganizationSidebar.tsx (1)
229-236:⚠️ Potential issue | 🟠 Major"Host Hackathon" link navigates to
/organizations/undefined/hackathons/newwhenderivedOrgIdis absent.Unlike the
menuItemswhich guard against a missingderivedOrgId(falling back to'#'), the "Host Hackathon" quick action unconditionally interpolatesderivedOrgIdinto the href. When the org ID can't be derived, this produces a broken URL.🐛 Proposed fix
{ icon: Plus, label: 'Host Hackathon', - href: `/organizations/${derivedOrgId}/hackathons/new`, + href: derivedOrgId ? `/organizations/${derivedOrgId}/hackathons/new` : '#', gradient: 'from-primary/20 to-primary/5', disabled: false, },components/organization/hackathons/details/HackathonSidebar.tsx (1)
221-265: 🛠️ Refactor suggestion | 🟠 MajorMove the
hackathonsprop check to the top of the memo for an early return.Line 263 checks
if (hackathons.length > 0) return hackathonsonly after iterating overapiHackathonsanddrafts, discarding all that work. Moving this to the top avoids unnecessary computation and improves clarity.Proposed fix
const hackathonData = useMemo<Hackathon[]>(() => { + if (hackathons.length > 0) return hackathons; + const items: Hackathon[] = []; apiHackathons.forEach(hackathon => { // ... }); drafts.forEach(draft => { // ... }); - if (hackathons.length > 0) return hackathons; return items; }, [drafts, apiHackathons, derivedOrgId, hackathons]);As per coding guidelines, "Use early returns to improve code clarity."
hooks/hackathon/use-team-posts.ts (1)
118-139:⚠️ Potential issue | 🟡 MinorPass
organizationIdtogetMyTeamcall.
getMyTeamaccepts an optionalorganizationIdparameter (per API definition) and uses it to determine the endpoint: withorganizationIdit fetches/organizations/${organizationId}/hackathons/${hackathonSlugOrId}/my-team, without it fetches/hackathons/${hackathonSlugOrId}/my-team. Currently,organizationIdis in the dependency array but never passed to the function, leaving the code unable to fetch organization-scoped team data. Either passorganizationIdto the call and keep it in dependencies, or remove it entirely if organization context is not needed.Proposed fix
- const response = await getMyTeam(hackathonSlugOrId); + const response = await getMyTeam(hackathonSlugOrId, organizationId);components/organization/tabs/MembersTab.tsx (2)
200-211:⚠️ Potential issue | 🟠 MajorPermission mismatch: admins see the Remove button but the handler rejects them.
canManageTeam(line 403-405) istruefor bothownerandadmin, so admins will see the remove-member button viaMemberCard. However,handleRemoveMember(line 207-210) re-checks ownership and rejects non-owners with "Only organization owners can remove members".Either restrict
canManageTeamto owners only (for the remove action), or updatehandleRemoveMemberto also allow admins. As-is, this is a confusing UX where the button is visible but always fails for admins.Also applies to: 403-405
163-175:⚠️ Potential issue | 🟡 Minor
setInviteEmails([])andsetEmailInput('')should be moved after the loop.These calls are inside the
forloop, so they execute on every iteration. While React batches state updates and the closure captures the originalinviteEmailsarray so the loop still completes, the intent is clearly to clear inputs once after all invitations are sent.♻️ Suggested fix
for (const email of inviteEmails) { const { error } = await authClient.organization.inviteMember({ email, role: 'member', organizationId: activeOrgId, resend: true, }); if (error) { toast.error(error.message); } - setInviteEmails([]); - setEmailInput(''); } + setInviteEmails([]); + setEmailInput(''); await fetchInvitations();components/organization/tabs/MembersTab/TeamManagementSection.tsx (1)
48-64:⚠️ Potential issue | 🔴 CriticalBug:
orgOwner.imageis alwaysundefined— should beorgOwner.avatar.The data transform in
MembersTab.tsx(line 102) populates only theavatarfield (avatar: member.user.image), butTeamManagementSection.tsxline 51 referencesorgOwner.image, which is never set. This causes the owner's avatar to never render and fall back to the initials.🐛 Proposed fix
- <AvatarImage src={orgOwner.image} alt={orgOwner.name} /> + <AvatarImage src={orgOwner.avatar} alt={orgOwner.name} />components/organization/hackathons/new/NewHackathonTab.tsx (1)
85-85:⚠️ Potential issue | 🟡 MinorRemove debug
console.logstatement.
'sjcdkformData'looks like an accidental debug artifact left from development. This will clutter the console in production.🧹 Proposed fix
- console.log('sjcdkformData', formData);components/organization/cards/GradeSubmissionModal/useJudgingCriteria.ts (1)
31-49:⚠️ Potential issue | 🔴 CriticalPotential infinite fetch loop when the API returns an empty array.
localCriteriais listed in the dependency array (Line 49). IfgetJudgingCriteriareturns[], the state is set to a new empty array reference, the effect re-triggers, and the guardlocalCriteria.length === 0(Line 36) is still true — causing an unbounded loop of API calls.Consider introducing a
hasFetchedref to break the cycle:🐛 Proposed fix
+ const hasFetchedRef = useRef(false); + useEffect(() => { if ( open && organizationId && hackathonId && - (!localCriteria || localCriteria.length === 0) + (!localCriteria || localCriteria.length === 0) && + !hasFetchedRef.current ) { + hasFetchedRef.current = true; setIsFetchingCriteria(true); getJudgingCriteria(hackathonId) .then((criteria: JudgingCriterion[]) => { setLocalCriteria(Array.isArray(criteria) ? criteria : []); }) - .catch(() => {}) + .catch(() => { + // Reset so a retry is possible on next open + hasFetchedRef.current = false; + }) .finally(() => { setIsFetchingCriteria(false); }); } - }, [open, organizationId, hackathonId, localCriteria]); + }, [open, organizationId, hackathonId, localCriteria]);lib/api/hackathons/core.ts (1)
119-124:⚠️ Potential issue | 🟡 MinorUse generic type parameter instead of unsafe
ascast, and fix function naming.
getHackathonandGetHackathonBySluguse unsafe type casts (as GetHackathonResponse) without providing a generic type toapi.get(). This reduces type safety compared to the pattern used elsewhere (e.g., line 272 withapi.get<PublicHackathonsListResponse>(url)).Provide the generic type parameter:
api.get<GetHackathonResponse>(...)instead of casting the result. Also, renameGetHackathonBySlugtogetHackathonBySlugto follow the function naming convention (PascalCase is for components/classes).components/organization/tabs/TransferOwnershipTab.tsx (2)
242-246:⚠️ Potential issue | 🟡 MinorRemove debug info before shipping.
This "Available members" counter with loading indicator looks like development-time debugging. It leaks internal state to users and clutters the UI.
🧹 Proposed fix
- {/* Debug info */} - <div className='mt-2 text-xs text-gray-400'> - Available members: {members.length}{' '} - {loadingMembers && '(Loading...)'} - </div>
109-109:⚠️ Potential issue | 🟠 Major
createdAt.toISOString()may throw at runtime.
BetterAuthMember.createdAtis typed asDate, but JSON deserialization from the API typically yields a string. IfcreatedAtis actually a string at runtime, calling.toISOString()on it will throwTypeError: member.createdAt.toISOString is not a function.🛡️ Safer conversion
- joinedAt: member.createdAt.toISOString(), + joinedAt: new Date(member.createdAt).toISOString(),components/hackathons/HackathonsPage.tsx (1)
124-156:⚠️ Potential issue | 🟡 MinorMinor: Singular/plural inconsistency in empty state text.
The title was changed to singular "No available hackathon" but the description still uses plural "hackathons" (lines 134–135). Consider making them consistent — either both singular or both plural. Typically "No available hackathons" (plural) is more natural for a list page empty state.
components/organization/cards/GradeSubmissionModal/useScoreCalculation.ts (1)
16-27:⚠️ Potential issue | 🟠 Major
totalWeightis accumulated but unused in the score calculation — incorrect result when weights don't sum to 100.Line 24 accumulates
totalWeight, but line 27 divides by the hard-coded constant10instead of usingtotalWeight. The formulatotalWeightedScore / 10only produces correct percentages when criterion weights sum to exactly 100. If they don't (which theJudgingCriteriaListcomponent explicitly warns about), results will be wrong.The weighted-average percentage should be:
Proposed fix
- return totalWeight > 0 ? totalWeightedScore / 10 : 0; + return totalWeight > 0 ? (totalWeightedScore / totalWeight) * 10 : 0;This normalizes scores correctly regardless of whether weights sum to 100.
components/organization/cards/GradeSubmissionModal/index.tsx (1)
30-30:⚠️ Potential issue | 🟡 MinorUnused
participantIdprop —submission.idis used instead.The
participantIdprop is declared inGradeSubmissionModalProps(Line 30) and destructured (Line 41), but it's never referenced. BothuseSubmissionScores(Line 66) anduseScoreForm(Line 91) receiveparticipantId: submission.id. This makes the prop dead code and could mislead callers into thinking they must provide a separate participant ID.Either remove the prop from the interface and destructuring, or use it consistently instead of
submission.id.Proposed fix
interface GradeSubmissionModalProps { open: boolean; onOpenChange: (open: boolean) => void; organizationId: string; hackathonId: string; - participantId: string; judgingCriteria?: JudgingCriterion[]; submission: SubmissionData; onSuccess?: () => void; } export default function GradeSubmissionModal({ open, onOpenChange, organizationId, hackathonId, - participantId, judgingCriteria, submission, onSuccess, }: GradeSubmissionModalProps) {Also applies to: 41-41, 66-66, 91-91
app/(landing)/hackathons/[slug]/page.tsx (2)
404-405:⚠️ Potential issue | 🟠 MajorResources tab content rendered twice.
HackathonResourcesis rendered in two separate blocks: Lines 404-405 (activeTab === 'resources' && currentHackathon.resources?.length > 0) and Lines 443-445 (activeTab === 'resources' && currentHackathon?.resources?.[0]). Both conditions overlap, so the component renders twice when the resources tab is active and resources exist.Remove one of the duplicate blocks.
Proposed fix — remove the second block
- {activeTab === 'resources' && currentHackathon?.resources?.[0] && ( - <HackathonResources /> - )}Also applies to: 443-445
299-303:⚠️ Potential issue | 🟡 MinorVariable
paramsshadows outer scope.
const paramson Line 301 shadows the component-levelparamsfromuseParams()on Line 37. This compiles but harms readability and can lead to subtle bugs.Proposed fix
const handleTabChange = (tabId: string) => { setActiveTab(tabId); - const params = new URLSearchParams(searchParams.toString()); - params.set('tab', tabId); - router.push(`?${params.toString()}`, { scroll: false }); + const tabParams = new URLSearchParams(searchParams.toString()); + tabParams.set('tab', tabId); + router.push(`?${tabParams.toString()}`, { scroll: false }); };
🤖 Fix all issues with AI agents
In `@app/`(landing)/hackathons/[slug]/page.tsx:
- Line 95: The memoized hackathonTabs computed by useMemo references
announcements.length but announcements is not included in the dependency array;
update the dependency array for the useMemo that defines hackathonTabs (the
useMemo block containing hackathonTabs) to include announcements (or
announcements.length) so the memo recomputes when announcements change, ensuring
the announcements tab appears; locate the useMemo that returns hackathonTabs and
add announcements to its dependency list.
🟠 Major comments (20)
components/organization/hackathons/details/HackathonSidebar.tsx-51-51 (1)
51-51:⚠️ Potential issue | 🟠 MajorAbsolute-positioned gradient needs a
relativeparent.The gradient overlay on line 164 uses
absolutepositioning, but the parent<nav>(line 51) lacksrelative. This means the gradient will anchor to whatever ancestor happens to have positioning, likely causing layout issues — especially in the mobile Sheet where the containing hierarchy is different from desktop.Proposed fix
- <nav className='flex h-full flex-col overflow-y-auto px-4 py-6'> + <nav className='relative flex h-full flex-col overflow-y-auto px-4 py-6'>Also applies to: 164-164
components/organization/hackathons/details/HackathonSidebar.tsx-33-40 (1)
33-40: 🛠️ Refactor suggestion | 🟠 Major
menuItems: any[]defeats type safety.Define a
MenuIteminterface and use it here instead ofany[]. This also makes the component self-documenting about what shape each menu item should have.Proposed fix
+interface MenuItem { + icon: React.ComponentType<{ className?: string }>; + label: string; + href: string; + description: string; + disabled?: boolean; +} + interface SidebarContentProps { hackathonData: Hackathon[]; currentHackathon?: Hackathon; - menuItems: any[]; + menuItems: MenuItem[]; isLoading: boolean; normalizedPath: string | null; basePath: string; }As per coding guidelines, "Prefer const arrow functions with explicit type annotations over function declarations" —
any[]bypasses this intent.hooks/use-link-validation.ts-39-114 (1)
39-114:⚠️ Potential issue | 🟠 MajorClient-side
fetchto arbitrary user-supplied URLs is unreliable and has privacy implications.
validateWebsiteissues a fullGETrequest from the user's browser to any URL they type. This:
- Leaks the user's IP to arbitrary third-party domains on every debounced keystroke.
- Is blocked by CORS on most sites, so the "reachability" check silently falls back to "valid" (lines 104-113), making it a no-op for the majority of URLs.
- Can trigger side-effects on servers that don't treat GET as safe/idempotent.
Consider restricting this validator to format-only checks (like X handle validation) and, if reachability verification is needed, proxying through your own backend with
HEADrequests.hooks/use-link-validation.ts-226-267 (1)
226-267:⚠️ Potential issue | 🟠 MajorGitHub API rate limit (60 req/hour unauthenticated) will be exhausted quickly.
Every debounced edit triggers a GitHub API call. With an 800ms debounce and no caching, a single user editing the field a few times across multiple orgs can easily exhaust the per-IP limit. Once rate-limited, the validator rejects the input (line 255-264) even if it's valid.
Consider:
- Caching successful lookups (e.g., in a
MaporuseRef) to avoid repeat calls for the same handle.- Using a
HEADrequest or adding an auth token if available.- Falling back to format-only validation (like X) instead of rejecting on 403.
hooks/use-link-validation.ts-255-264 (1)
255-264:⚠️ Potential issue | 🟠 MajorRate-limited response should not reject a valid handle.
When GitHub returns 403 (rate limited), the validator marks the input as invalid. This falsely prevents the user from saving a perfectly valid GitHub username. A safer approach is to accept format-valid handles when the API is unavailable.
🐛 Proposed fix
} else if (response.status === 403) { - // Rate limited - we can't verify, so we'll reject it to be safe const result = { - isValid: false, + isValid: true, isValidating: false, - error: 'Unable to verify (rate limited). Please try again later.', + error: null, }; setValidationState(prev => ({ ...prev, github: result })); return result;components/organization/tabs/LinksTab.tsx-176-188 (1)
176-188:⚠️ Potential issue | 🟠 MajorSave is allowed while validation is still in-flight — potential race condition.
The pre-save guard only checks
isValid === false, but if the user clicks "Save" while a validation request is still pending (isValidating === true), the check passes becauseisValidisnull. This allows saving with unvalidated input.🐛 Proposed fix: also block save during validation
+ // Block save while any field is still validating + const isAnyValidating = + validationStates.website.isValidating || + validationStates.x.isValidating || + validationStates.github.isValidating; + if (isAnyValidating) { + toast.error('Please wait for link validation to complete.'); + return; + } + // Check for validation errors if (links.website && validationStates.website.isValid === false) {hooks/use-link-validation.ts-20-23 (1)
20-23: 🛠️ Refactor suggestion | 🟠 MajorInternal
validationStateis set but never returned or read — remove it.The hook maintains
validationStateviasetValidationStateacross all three validators, but this state is never exposed in the return value (lines 293-297) and never read internally. Meanwhile, the consumer (LinksTab.tsx) maintains its own parallelvalidationStates. All thesesetValidationStatecalls are dead writes that trigger unnecessary re-renders of every component using this hook.Either remove the internal state entirely (since each function already returns the result) or expose it and let the consumer use it directly instead of duplicating state.
♻️ Proposed fix: remove internal state
export function useLinkValidation(): LinkValidationHook { - const [validationState, setValidationState] = useState< - Record<string, ValidationResult> - >({});Then remove every
setValidationState(...)call throughout the file and drop theuseStateimport.components/hackathons/submissions/SubmissionForm.tsx-246-288 (1)
246-288:⚠️ Potential issue | 🟠 Major
currentStepin the dependency array will re-advance the user whenever they navigate back to step 0.For
INDIVIDUALandTEAM(with existing team) hackathons, if a user clicks "Back" to return to step 0, this effect re-fires, seescurrentStep === 0, and immediately pushes them forward to step 1 again — making step 0 unreachable.Consider removing
currentStepfrom the dependency array and instead using a ref or a "has already auto-advanced" flag to ensure the auto-skip only happens once (on initial open).Additionally, there's a duplicate effect at lines 595–599 that also sets
participationTypeto'TEAM'whenmyTeamexists. This overlaps with the logic here (lines 259–265, 268–274) and can cause subtle ordering issues. Consider consolidating the two into this single effect.Proposed approach
+ const hasAutoAdvanced = useRef(false); + // Participation type enforcement useEffect(() => { if (!open || !currentHackathon) return; + if (hasAutoAdvanced.current) return; const hackathonType = currentHackathon.participantType; if (hackathonType === 'INDIVIDUAL') { form.setValue('participationType', 'INDIVIDUAL'); - if (!submissionId && currentStep === 0) { + if (!submissionId) { setCurrentStep(1); updateStepState(0, 'completed'); updateStepState(1, 'active'); + hasAutoAdvanced.current = true; } } else if (hackathonType === 'TEAM') { form.setValue('participationType', 'TEAM'); - if (!submissionId && !!myTeam && currentStep === 0) { + if (!submissionId && !!myTeam) { setCurrentStep(1); updateStepState(0, 'completed'); updateStepState(1, 'active'); + hasAutoAdvanced.current = true; } } else if (hackathonType === 'TEAM_OR_INDIVIDUAL') { if (!submissionId) { if (myTeam) { form.setValue('participationType', 'TEAM'); - if (currentStep === 0) { - setCurrentStep(1); - updateStepState(0, 'completed'); - updateStepState(1, 'active'); - } + setCurrentStep(1); + updateStepState(0, 'completed'); + updateStepState(1, 'active'); + hasAutoAdvanced.current = true; } else { form.setValue('participationType', 'INDIVIDUAL'); } } } }, [ open, currentHackathon, myTeam, submissionId, form, - currentStep, updateStepState, ]);And reset the ref when the modal closes (in the existing close-reset effect):
useEffect(() => { if (!open) { const timer = setTimeout(() => { // ... existing reset logic + hasAutoAdvanced.current = false; }, 300); return () => clearTimeout(timer); } }, [open, form]);Then remove the duplicate effect at lines 595–599.
lib/providers/OrganizationProvider.tsx-1326-1342 (1)
1326-1342: 🛠️ Refactor suggestion | 🟠 MajorDead fetch — result is discarded.
getFullOrganizationis called but the returneddatais never used (theif (data)block is empty). This adds a network request with no effect. Either wire it up to update state or remove the call.♻️ Remove the unused fetch
// Refresh organizations list to update current user's role in the UI await fetchOrganizations(); - - // Refresh active organization details if possible - if (state.activeOrgId === orgId) { - try { - const { data } = - await authClient.organization.getFullOrganization({ - query: { - organizationId: orgId, - }, - }); - if (data) { - // We might want to update the active organization with full details - // creating a merged object if needed, but for now let's rely on fetchOrganizations - // to update the user's role in the list which matters most for permissions - } - } catch (err) { - console.error( - 'Failed to refresh organization details after transfer:', - err - ); - } - }components/organization/tabs/TransferOwnershipTab.tsx-57-57 (1)
57-57:⚠️ Potential issue | 🟠 MajorDefault
isCurrentUserOwnertofalseinstead oftrue.Initializing to
truemeans ifactiveOrgIdis falsy (or the check hasn't run), the user briefly sees the transfer form as if they had permission. Defaulting tofalseis the safer posture — deny by default, then grant after verification.🛡️ Proposed fix
- const [isCurrentUserOwner, setIsCurrentUserOwner] = useState(true); + const [isCurrentUserOwner, setIsCurrentUserOwner] = useState(false);components/organization/cards/GradeSubmissionModal/useSubmissionScores.ts-100-110 (1)
100-110:⚠️ Potential issue | 🟠 MajorFragile judge matching —
judgeName === user.emailcan cause false positives.Line 108 matches
s.judgeNameagainstuser.email. If any judge's display name happens to equal another user's email address, this returns the wrong judge's scores. The matching chain should prioritizejudgeIdand only fall back to secondary identifiers with greater care.Additionally, the multiple
(s as any)casts suggest theIndividualJudgeScoretype is missing fields. Consider extending the type instead.♻️ Safer matching approach
const currentUserScore = ( response.data as IndividualJudgeScore[] ).find( s => s.judgeId === user.id || - s.judgeName === user.name || - s.judgeName === user.email || - (s as any).judgeEmail === user.email + (s as any).judgeEmail === user.email || + (s.judgeName === user.name && user.name !== undefined) );components/organization/cards/JudgingParticipant/IndividualScoresBreakdown.tsx-170-178 (1)
170-178:⚠️ Potential issue | 🟠 MajorCriterion label likely shows an internal ID instead of the human-readable title.
c.criterionId || c.criterionTitlepreferscriterionId(which is typically a UUID or database identifier) overcriterionTitle. The displayed label will be an opaque ID whenever one exists. This should be reversed:Proposed fix
- {c.criterionId || c.criterionTitle} + {c.criterionTitle || c.criterionId}components/organization/hackathons/judging/JudgingResultsTable.tsx-42-57 (1)
42-57: 🛠️ Refactor suggestion | 🟠 MajorPervasive
as anycasts indicate a misalignedJudgingResulttype — extract a helper and fix the type.The score-extraction logic
(a as any).averageScore ?? (a as any).average_score ?? ...bypasses TypeScript's safety and is duplicated verbatim at lines 120–125 for display. This suggestsJudgingResultdoesn't include the properties the component actually receives.Fix the root cause by updating the
JudgingResulttype to include the expected score fields, then extract a helper:Suggested refactor
+const getScore = (result: JudgingResult): number => + Number(result.averageScore ?? result.average_score ?? result.score ?? 0); + const sortedResults = React.useMemo(() => { return [...results].sort((a, b) => { - const scoreA = Number( - (a as any).averageScore ?? - (a as any).average_score ?? - (a as any).score ?? - 0 - ); - const scoreB = Number( - (b as any).averageScore ?? - (b as any).average_score ?? - (b as any).score ?? - 0 - ); - return scoreB - scoreA; + return getScore(b) - getScore(a); }); }, [results]);Then reuse
getScore(result).toFixed(2)at lines 120–125.app/(landing)/organizations/[id]/hackathons/[hackathonId]/judging/page.tsx-142-155 (1)
142-155:⚠️ Potential issue | 🟠 Major
fetchDatahas stale closures overfetchJudgesandfetchResults.
fetchData(Line 142) callsfetchJudges()andfetchResults()(Lines 154-155), but itsuseCallbackdependency array (Line 251) only lists[organizationId, hackathonId]. SincefetchJudgescapturesactiveOrgIdin its closure and is itself auseCallback, changes toactiveOrgIdwon't be reflected by the stale reference insidefetchData.Add
fetchJudgesandfetchResultstofetchData's dependencies:Proposed fix
- }, [organizationId, hackathonId]); + }, [organizationId, hackathonId, fetchJudges, fetchResults]);Also applies to: 251-251
components/organization/cards/JudgingParticipant.tsx-47-58 (1)
47-58:⚠️ Potential issue | 🟠 MajorEach
JudgingParticipantinstance fetches its own session — N×API calls.Every card independently calls
authClient.getSession()on mount (Lines 52-58). If there are 50 submissions, that's 50 parallel session requests. Lift session fetching to the parent (JudgingPage) and passcurrentUserId(orisAssignedJudge) as a prop instead.lib/api/hackathons.ts-2304-2372 (1)
2304-2372:⚠️ Potential issue | 🟠 MajorJudges API is fully duplicated between
hackathons.tsandhackathons/judging.ts.
addJudge,removeJudge,getHackathonJudges, and their request/response types are defined in bothlib/api/hackathons.ts(Lines 2304-2372) andlib/api/hackathons/judging.ts(Lines 412-459). Worse, theAddJudgeRequesttype differs: here it has{ userId }(Line 2308-2310), while injudging.tsit has{ userId, email }(Lines 212-215). This type mismatch means callers importing from different modules get different contracts.Consolidate these to a single canonical location (likely
judging.ts) and re-export fromhackathons.tsif needed.lib/api/hackathons.ts-1292-1304 (1)
1292-1304: 🛠️ Refactor suggestion | 🟠 Major
getJudgingCriteriaduplicated — identical implementation exists injudging.ts.This function (Lines 1292-1304) is an exact copy of the one in
lib/api/hackathons/judging.ts(Lines 365-374). Consolidate into one module.components/organization/cards/GradeSubmissionModal/useScoreForm.ts-120-124 (1)
120-124:⚠️ Potential issue | 🟠 MajorA legitimate score of 0 is rejected by validation.
Line 120 treats
score === 0as invalid ('Score required'). If a judge intentionally gives 0/10 for a criterion, the form won't submit. If scores genuinely must be > 0, consider changing the minimum to 0.1 or making the intent clear in the UI. Otherwise, allow 0 as a valid score.Proposed fix (if 0 should be valid)
const score = scores[key]; - if (typeof score !== 'number' || score === 0) { + if (typeof score !== 'number') { errors[key] = 'Score required'; isValid = false; }components/organization/cards/GradeSubmissionModal/index.tsx-191-203 (1)
191-203:⚠️ Potential issue | 🟠 MajorUse
absolutepositioning for the footer or ensure BoundlessSheet establishes a stacking context.The footer's
fixedpositioning is viewport-relative and will overlay content outside the sheet. SinceBoundlessSheetdoesn't create a stacking context (theSheetContentlackstransform,z-index,opacity, or similar properties), the fixed element cannot be constrained to the sheet's bounds. Either reposition the footer withabsoluterelative to its parent scroll container, or add a stacking context property (e.g.,transform: translateZ(0)or explicitz-index) to the sheet wrapper.lib/api/hackathons/judging.ts-183-206 (1)
183-206:⚠️ Potential issue | 🟠 Major
& anyondatatype makes the entire union meaningless.Line 197's
& anywidens thedatafield toany, making the preceding union type (JudgingSubmission[] | { submissions, pagination }) effectively useless. Remove the& anyto preserve type safety.Proposed fix
export interface GetJudgingSubmissionsResponse extends ApiResponse<any> { success: true; data: | JudgingSubmission[] | { submissions: any[]; pagination: { page: number; limit: number; total: number; totalPages: number; }; - } - ) & - any; + }; pagination?: { currentPage: number; totalPages: number; totalItems: number; itemsPerPage: number; hasNext: boolean; hasPrev: boolean; }; }
🟡 Minor comments (18)
components/hackathons/participants/profileCard.tsx-76-88 (1)
76-88:⚠️ Potential issue | 🟡 MinorMissing abort controller — potential state update after unmount or stale response on rapid prop changes.
If
participant.usernamechanges quickly or the component unmounts before the fetch completes,setProfileDatawill be called with a stale/outdated result (or on an unmounted component). Use anAbortControlleror a cleanup flag to discard obsolete responses.🛡️ Proposed fix
useEffect(() => { + let cancelled = false; const fetchProfile = async () => { try { if (participant.username) { const data = await getUserProfileByUsername(participant.username); - setProfileData(data); + if (!cancelled) setProfileData(data); } } catch (error) { - console.error('Error fetching profile data:', error); + if (!cancelled) console.error('Error fetching profile data:', error); } }; fetchProfile(); + return () => { cancelled = true; }; }, [participant.username]);components/organization/OrganizationSidebar.tsx-273-276 (1)
273-276:⚠️ Potential issue | 🟡 MinorConflicting
topvalue: Tailwind classtop-4is overridden by inlinestyle={{ top: '90px' }}.The Tailwind
top-4(16px) inclassNameis silently overridden by the inline styletop: '90px'. This is confusing and the Tailwind class is dead code. Remove the stale Tailwind class to avoid misleading future readers.🧹 Proposed fix
<aside - className='fixed top-4 left-0 hidden w-[280px] border-r border-zinc-800/50 bg-black/40 backdrop-blur-xl md:block' + className='fixed left-0 hidden w-[280px] border-r border-zinc-800/50 bg-black/40 backdrop-blur-xl md:block' style={{ height: availableHeight, top: '90px' }} >As per coding guidelines, "Use TailwindCSS classes for styling; avoid plain CSS." Consider using a Tailwind arbitrary value
top-[90px]instead of an inline style to keep styling consistent.components/organization/OrganizationSidebar.tsx-32-179 (1)
32-179:⚠️ Potential issue | 🟡 MinorDecorative gradient at Line 178 uses
absolutepositioning but the parent<nav>lacksrelative.The
absolute-positioned gradient div on Line 178 will position itself relative to the nearest positioned ancestor, which may not be the<nav>. Addrelativeto the<nav>element to ensure the gradient anchors correctly.🐛 Proposed fix
- <nav className='flex h-full flex-col overflow-y-auto px-4 py-6'> + <nav className='relative flex h-full flex-col overflow-y-auto px-4 py-6'>components/organization/hackathons/details/HackathonSidebar.tsx-369-373 (1)
369-373:⚠️ Potential issue | 🟡 MinorMobile sidebar trigger button is missing an accessible label.
Screen readers will announce this as an unlabeled button. Add an
aria-label(or use the<SheetTrigger>title) so it's identifiable.- <button className='flex h-10 w-10 items-center justify-center rounded-lg border border-zinc-800 bg-black/60 shadow-lg backdrop-blur-xl'> + <button + aria-label='Open sidebar menu' + className='flex h-10 w-10 items-center justify-center rounded-lg border border-zinc-800 bg-black/60 shadow-lg backdrop-blur-xl' + >components/organization/tabs/ProfileTab.tsx-435-480 (1)
435-480:⚠️ Potential issue | 🟡 MinorExtract duplicate error-handling logic to a shared utility function.
The error message extraction pattern appears identically in both
ProfileTab.tsx(lines 435–480) andLinksTab.tsx(lines 218–236). More importantly, the order-dependent substring matching is fragile and can misclassify errors. For example, an error message like"Resource 'Unauthorized Access Log' not found"would match the'Unauthorized'check and incorrectly report authentication failure instead of the actual resource-not-found condition.Create a reusable
extractErrorMessage(error: unknown): stringutility and use it in both files. The utility should check for structured error properties (context.body.message,response.data.message) before attempting any string-based heuristics, and prefer numeric HTTP status codes when available over substring matching.components/hackathons/submissions/SubmissionForm.tsx-606-678 (1)
606-678:⚠️ Potential issue | 🟡 MinorGuard against
currentHackathonbeingnullduring initial render.If
currentHackathonhasn't loaded yet, the else branch (line 666) renders the informational banner. Line 671 falls through to display "Individual" even though the actual participation type is unknown. Consider either returning a loading/skeleton state whencurrentHackathonis nullish, or skipping the banner entirely until data is available.Also,
disabled={false}on line 620 is the default and can be removed.Suggested guard
+ {!currentHackathon ? ( + <div className="flex items-center justify-center py-4"> + <Loader2 className="h-6 w-6 animate-spin text-[`#a7f950`]" /> + </div> + ) : currentHackathon.participantType === 'TEAM_OR_INDIVIDUAL' ? ( - {currentHackathon?.participantType === 'TEAM_OR_INDIVIDUAL' ? ( <FormField ...components/organization/tabs/MembersTab.tsx-355-355 (1)
355-355:⚠️ Potential issue | 🟡 MinorBug: Stray
{userIsOwner}expression renders a boolean as text.Line 355 renders the raw boolean value (
trueorfalse) into the DOM as visible text. This is almost certainly a leftover from a refactor and should be removed.🐛 Proposed fix
- {userIsOwner} {userIsOwner && (components/organization/tabs/MembersTab/TeamManagementSection.tsx-26-31 (1)
26-31:⚠️ Potential issue | 🟡 MinorRemove
activeOrgfrom theTeamManagementSectionPropsinterface — it's passed by the parent but never used.The
activeOrgprop is declared in the interface (line 22) but not destructured in the function signature (lines 26–31) and appears only in commented-out debug code (line 66). Since the component doesn't consume it, remove it from the interface definition to prevent unnecessary prop passing from the parent.♻️ Suggested fix
Update
TeamManagementSectionPropsto remove the unused prop:interface TeamManagementSectionProps { members: Member[]; onRoleChange: (memberId: string, newRole: string) => void; onRemoveMember: (memberId: string) => void; - activeOrg?: Organization | null; canManageTeam?: boolean; }Also update the parent component to stop passing
activeOrg:<TeamManagementSection members={members} onRoleChange={handleRoleChange} onRemoveMember={handleRemoveMember} - activeOrg={activeOrg} canManageTeam={...} />app/(landing)/organizations/[id]/hackathons/[hackathonId]/announcement/page.tsx-310-337 (1)
310-337:⚠️ Potential issue | 🟡 MinorAction buttons are inaccessible via keyboard.
The publish, edit, and delete buttons use
opacity-0 group-hover:opacity-100, making them invisible and unreachable for keyboard-only users. Consider addingfocus-withinvisibility so the buttons appear when any child receives focus.♿ Proposed fix
- <div className='ml-4 flex items-center gap-2 opacity-0 transition-opacity group-hover:opacity-100'> + <div className='ml-4 flex items-center gap-2 opacity-0 transition-opacity group-hover:opacity-100 group-focus-within:opacity-100'>app/(landing)/organizations/[id]/hackathons/[hackathonId]/announcement/page.tsx-51-65 (1)
51-65:⚠️ Potential issue | 🟡 Minor
fetchAnnouncementsmissing from useEffect deps and not memoized.
fetchAnnouncementsis called inside the effect but not listed in the dependency array. IforganizationIdorhackathonIdchange, the stale closure won't pick up the new values. WrapfetchAnnouncementsinuseCallbackand include it in the deps.♻️ Proposed fix
+import { useState, useEffect, useCallback } from 'react'; ... - const fetchAnnouncements = async () => { + const fetchAnnouncements = useCallback(async () => { try { const data = await listAnnouncements(hackathonId); setAnnouncements(data); } catch (error) { console.error('Failed to fetch announcements:', error); toast.error('Failed to load announcements'); } finally { setIsLoading(false); } - }; + }, [hackathonId]); useEffect(() => { fetchAnnouncements(); - }, [hackathonId]); + }, [fetchAnnouncements]);app/(landing)/hackathons/[slug]/announcements/[announcementId]/page.tsx-28-49 (1)
28-49:⚠️ Potential issue | 🟡 MinorNo guard for missing
slugparameter before the API call.Line 30 guards against a missing
announcementIdbut notslug. Ifslugis somehow undefined,GetHackathonBySlugwould be called withundefined, andhackathonData.data.name(line 38) could throw if the response structure is unexpected. Consider addingslugto the guard:Suggested fix
- if (!announcementId) return; + if (!announcementId || !slug) return;app/(landing)/hackathons/[slug]/announcements/[announcementId]/page.tsx-92-98 (1)
92-98:⚠️ Potential issue | 🟡 Minor
window.close()is unreliable — will silently do nothing if the tab wasn't opened via JavaScript.Browsers restrict
window.close()to tabs/windows opened bywindow.open(). If a user navigates to this page directly or via a regular link, the button labeled "Close Tab" will be non-functional with no feedback. The same issue applies at line 189.Consider using
router.back()(as you already do in the error state) orrouter.push(...)to navigate to the hackathon page instead.Suggested fix
<button - onClick={() => window.close()} + onClick={() => router.push(`/hackathons/${slug}`)} className='flex items-center gap-2 text-sm text-zinc-400 transition-colors hover:text-white' > <ArrowLeft className='h-4 w-4' /> - Close Tab + Back to Hackathon </button>Apply similar change at line 189.
app/(landing)/hackathons/[slug]/page.tsx-76-76 (1)
76-76:⚠️ Potential issue | 🟡 Minor
announcementsLoadingstate is declared but never used.The loading state on Line 76 is set during the fetch but not consumed in the UI (no loading indicator shown while announcements load).
app/(landing)/organizations/[id]/hackathons/[hackathonId]/judging/page.tsx-257-260 (1)
257-260:⚠️ Potential issue | 🟡 Minor
handleSuccessdouble-fetches results.
handleSuccesscalls bothfetchData()andfetchResults(). ButfetchDataalready callsfetchResults()internally (Line 155), so results are fetched twice on every success callback.Proposed fix
const handleSuccess = () => { fetchData(); - fetchResults(); // Refresh results to update metrics/table };lib/api/hackathons/judging.ts-49-55 (1)
49-55:⚠️ Potential issue | 🟡 Minor
ranktyped asany— use a concrete type.
rank?: anyon Line 54 discards type information. Based on usage, this should benumber | nullornumber | undefined.Proposed fix
export interface JudgingResult { submissionId: string; projectName: string; averageScore: number; judgeCount: number; - rank?: any; + rank?: number | null; }components/organization/cards/JudgingParticipant.tsx-186-190 (1)
186-190:⚠️ Potential issue | 🟡 MinorPotential "Invalid Date" rendered if both date fields are missing.
new Date(submissionData.submissionDate || participant.registeredAt)will produceInvalid Datewhen both values are falsy, sincenew Date(undefined)is invalid. Add a fallback or guard.Proposed fix
- <span> - Submitted{' '} - {new Date( - submissionData.submissionDate || participant.registeredAt - ).toLocaleDateString()} - </span> + <span> + Submitted{' '} + {submissionData.submissionDate || participant.registeredAt + ? new Date( + submissionData.submissionDate || participant.registeredAt + ).toLocaleDateString() + : 'N/A'} + </span>lib/api/hackathons.ts-1742-1745 (1)
1742-1745:⚠️ Potential issue | 🟡 MinorDuplicate JSDoc block for
getSubmissionDetails.Lines 1742-1745 contain a second
/** Returns full submission... */JSDoc right after the first one that starts on Line 1739. Remove the duplicate.Proposed fix
-/** - * Returns full submission with votes and comments - */ export const getSubmissionDetails = async (app/(landing)/organizations/[id]/hackathons/[hackathonId]/judging/page.tsx-312-312 (1)
312-312:⚠️ Potential issue | 🟡 MinorDebug
console.logleft in production code.Line 312:
console.log('Judging Results Response:', res);— remove before merging.Proposed fix
const res = await getJudgingResults(organizationId, hackathonId); - console.log('Judging Results Response:', res); if (res.success) {
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 (1)
app/(landing)/hackathons/[slug]/page.tsx (1)
444-446:⚠️ Potential issue | 🟡 MinorDuplicate
resourcestab rendering —<HackathonResources />is rendered twice.Lines 405-406 already render the resources tab content when
activeTab === 'resources'. This block at lines 444-446 duplicates it with a slightly different guard, causing two instances of<HackathonResources />to mount simultaneously.Remove this duplicate block:
Proposed fix
- {activeTab === 'resources' && currentHackathon?.resources?.[0] && ( - <HackathonResources /> - )}
🧹 Nitpick comments (1)
app/(landing)/hackathons/[slug]/page.tsx (1)
76-76:announcementsLoadingis unused dead code.The state variable is set on lines 82 and 89 but never read anywhere in the component. Remove it or wire it to a loading indicator.
Proposed fix
const [announcements, setAnnouncements] = useState<HackathonAnnouncement[]>( [] ); - const [announcementsLoading, setAnnouncementsLoading] = useState(false); useEffect(() => { async function fetchAnnouncements() { if (!hackathonId) return; try { - setAnnouncementsLoading(true); const data = await listAnnouncements(hackathonId); // Only show published announcements for public view setAnnouncements(data.filter(a => !a.isDraft)); } catch (error) { console.error('Failed to fetch announcements:', error); - } finally { - setAnnouncementsLoading(false); } } fetchAnnouncements(); }, [hackathonId]);
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/organization/cards/GradeSubmissionModal/useScoreForm.ts (1)
130-183:⚠️ Potential issue | 🟠 MajorSilent failure when
response.successis falsy.If
submitJudgingScorereturns{ success: false, message: "..." }, the code reaches line 154, theif (response.success)check fails, and the function exits without any user feedback. The user sees the loading spinner disappear with no toast.Proposed fix
if (response.success) { setShowSuccess(true); toast.success( existingScore ? 'Grade updated successfully' : 'Grade submitted successfully', { duration: 2000, } ); if (onSuccess) { onSuccess(); } setTimeout(() => { setShowSuccess(false); onClose(); }, 2000); + } else { + toast.error(response.message || 'Failed to submit grade'); }lib/api/hackathons/judging.ts (1)
301-317:⚠️ Potential issue | 🔴 CriticalInclude
organizationIdin the request URL to match the pattern of all other judging functions in this file.
getJudgingSubmissionsacceptsorganizationIdbut the endpoint URL is/hackathons/${hackathonId}/submissions, unlike all other functions in the same file which use/organizations/${organizationId}/hackathons/${hackathonId}/.... Since callers intentionally passorganizationIdand every related function (submitGrade,getSubmissionScores,getJudgingResults, etc.) includes the organization scope in the URL, the endpoint should be/organizations/${organizationId}/hackathons/${hackathonId}/submissions?...to maintain consistency and ensure proper organization-level access control.
🤖 Fix all issues with AI agents
In `@app/`(landing)/organizations/[id]/hackathons/[hackathonId]/judging/page.tsx:
- Around line 289-292: handleSuccess is redundantly calling fetchResults() even
though fetchData() already invokes fetchResults() (see fetchData and
fetchResults). Remove the explicit fetchResults() call from handleSuccess so it
only calls fetchData(); ensure handleSuccess remains the post-submit callback
that triggers fetchData() which in turn refreshes results/metrics.
- Around line 148-172: In the useCallback fetchResults function remove the
leftover debug console.log('Judging Results Response:', res) so response data is
not leaked to the browser console; locate the log inside fetchResults
(referencing getJudgingResults, res, and setJudgingResults) and delete that
console.log call (or replace it with a conditional debug-only logger gated by
NODE_ENV !== 'production' if you need retained debug output).
- Around line 516-521: The UI is rendering internal judge.userId which may
expose PII/internal IDs; update the rendering to show a user-facing attribute
(e.g., judge.email or judge.role) instead of judge.userId, or remove the second
<p> entirely if no friendly field exists. Locate the JSX that displays
{judge.name} and {judge.userId} and replace {judge.userId} with the appropriate
display field (judge.email or judge.role) and add a safe fallback (e.g.,
fallback text like "No contact info") to avoid rendering raw IDs.
- Around line 174-283: fetchData currently calls fetchJudges() and
fetchResults() without awaiting them, so setIsLoading(false) fires before those
requests finish; change to await their execution (preferably in parallel) so
loading/error handling covers them — e.g. replace the fire-and-forget calls
inside fetchData with awaiting a combined promise (Promise.all or
Promise.allSettled) of fetchJudges() and fetchResults(), and handle their
failures (propagate or log/throw) so the catch block's toast is triggered; keep
names fetchData, fetchJudges, fetchResults, setIsLoading and ensure finally only
runs after those awaited calls complete.
In `@components/hackathons/submissions/SubmissionForm.tsx`:
- Around line 247-283: The effect that enforces participation type can later
override a user’s manual choice because hasAutoAdvanced.current is only set when
the effect itself auto-advances; modify the manual advancement logic (e.g., in
the handleNext handler) so that when the user successfully advances past step 0
(the same transition where setCurrentStep(1) and updateStepState(0/1) are
called), you also set hasAutoAdvanced.current = true; this prevents the
useEffect (which reads currentHackathon, myTeam, submissionId, and calls
form.setValue / setCurrentStep) from re-running later and clobbering the user’s
manually chosen participationType.
- Around line 597-669: The banner is rendered incorrectly when currentHackathon
is undefined because the existing conditional (currentHackathon?.participantType
=== 'TEAM_OR_INDIVIDUAL') falls through to the else branch; update the render
logic in the SubmissionForm component to first check for the presence/loading
state of currentHackathon (e.g., if (!currentHackathon) return a loading
placeholder or null) or change the conditional to explicitly handle undefined
(e.g., currentHackathon ? currentHackathon.participantType ===
'TEAM_OR_INDIVIDUAL' : loading), so the "Team/Individual only" banner is only
shown when currentHackathon.participantType is known.
In `@components/organization/cards/GradeSubmissionModal/useScoreForm.ts`:
- Around line 49-51: getCriterionKey currently uses criterion.id ||
criterion.name || criterion.title which can produce duplicate keys when id/name
are missing and titles repeat; update getCriterionKey to accept the criterion's
array index (e.g., getCriterionKey(criterion, index)) and return a disambiguated
key like "<id|name|title>-<index>" so keys are unique, and add a warning/log via
the existing logger when criterion.id is missing to surface potentially
non-unique identifiers; update all call sites that invoke getCriterionKey (e.g.,
where scores are built) to pass the index.
- Around line 139-146: In useScoreForm's scoreData mapping, don't duplicate the
fallback chain for criterion IDs; replace the inline `criterion.id ||
criterion.name || criterion.title` with a call to getCriterionKey(criterion) so
criterionId uses the same key derivation as the local `key` variable; update the
mapping in the scoreData construction (inside the criteria.map) to assign
criterionId = getCriterionKey(criterion) to keep key derivation consistent.
In `@components/organization/cards/GradeSubmissionModal/useSubmissionScores.ts`:
- Around line 78-82: The effect's early-return guard only helps when
loadedParticipantIdRef.current is set in the "happy path"; update the code so
loadedParticipantIdRef.current = participantId is assigned in every terminal
branch of the async flow (no auth session from authClient.getSession, no
matching score from getSubmissionScores, and API/network error) as well as the
existing happy path where a score is found, so the ref prevents repeated
refetches when criteria or other deps change.
- Around line 41-43: getCriterionKey can return undefined which becomes the
string "undefined" as an object key; update it to always return a stable string
key by using a deterministic fallback (e.g., return `${criterion.id ??
criterion.name ?? (criterion as any).title ?? `missing-criterion-${index}`}`)
and change callsites to pass the criterion index if needed; also extend the
JudgingCriterion type to include an optional title field (remove the repeated
"as any" casts at getCriterionKey and the uses around lines where casts occur)
so the function can use criterion.title without casting and avoid creating
“undefined” keys in the Records.
In `@components/organization/cards/JudgingParticipant.tsx`:
- Around line 64-66: The code casts submission to any (const sub = submission as
any) which removes type safety for JudgingSubmission; replace this with a proper
typed narrowing or union instead: define/extend the JudgingSubmission type (or
create a union type) to cover the response shapes and use type guards or
optional chaining to safely access fields, then change references to use the
typed variable (avoid sub:any) and update uses of participant and submissionData
to derive from the typed object (participant, submissionData) so accesses like
sub.averageScore, sub.id, sub.type are type-checked.
- Around line 176-181: The render currently calls new
Date(submissionData.submissionDate ||
participant.registeredAt).toLocaleDateString() which yields "Invalid Date" if
both are undefined; in the JudgingParticipant component guard this by checking
for a valid date source before constructing Date and rendering — e.g., compute
const dateSrc = submissionData.submissionDate ?? participant.registeredAt and if
(!dateSrc) render a fallback string like "N/A" (or empty) instead of calling new
Date, otherwise call new Date(dateSrc).toLocaleDateString(); update the span
that currently contains the Date construction to use this guarded value.
- Around line 148-265: The breakdown panel is being rendered as a child of the
responsive flex container that switches to md:flex-row causing it to appear as a
narrow column; move the showBreakdown block (the JSX that renders
IndividualScoresBreakdown when showBreakdown is true) out of the outer flex
container (the div with classes 'bg-background/8 ... md:flex-row
md:items-center') so it becomes a sibling below that container, or alternatively
wrap the image + content areas in a new inner container (e.g., a div around the
image and the content area) and keep the breakdown as a full-width sibling under
that inner container so IndividualScoresBreakdown spans the full width on
desktop.
In
`@components/organization/cards/JudgingParticipant/IndividualScoresBreakdown.tsx`:
- Around line 29-49: The component's fetchScores flow swallows network errors
and still renders "No individual scores available yet"; modify the
useEffect/fetchScores logic to track an error state (e.g., add setError / error
state alongside setIsLoading and setScores), clear error at the start of
fetchScores, and in the catch setError with a user-friendly message; then update
the render path so the error state displays a distinct error UI (with a retry
action that calls fetchScores again) before the empty-scores check. Ensure you
reference the existing fetchScores, getSubmissionScores, setIsLoading, setScores
and the participantId/organizationId/hackathonId inputs when wiring retry and
clearing error.
In `@components/organization/tabs/LinksTab.tsx`:
- Around line 99-163: The three near-identical useEffect blocks (validating
debouncedWebsite, debouncedX, debouncedGithub using
validateWebsite/validateXHandle/validateGithubHandle and updating
setValidationStates) are duplicated and prone to stale async results; extract a
helper/custom hook (e.g., useFieldValidation(debouncedValue, validatorFn,
fieldKey)) that encapsulates the shared logic: set isValidating true, call the
validator, and in the effect cleanup set an isCancelled flag (or use
AbortController) so the async result is ignored if cancelled; only call
setValidationStates(fieldKey, result) when not cancelled, and ensure
empty/cleared state is handled when debouncedValue is falsy.
In `@components/organization/tabs/TransferOwnershipTab.tsx`:
- Around line 185-225: The permission-denied early return is after the
members-loading check, causing non-owners to briefly see the members loading
overlay; move the ownership check (using loadingOwnerCheck and
isCurrentUserOwner) above the loadingMembers check so you return the
permission-denied UI immediately when ownership has been determined false, or
alternatively gate fetchMembers behind the ownership check so loadingMembers
never becomes true for non-owners (update references in TransferOwnershipTab to
loadingOwnerCheck, isCurrentUserOwner, loadingMembers and any fetchMembers
call).
In `@lib/api/hackathons/judging.ts`:
- Around line 50-56: The JudgingResult interface currently types rank as any;
change it to a proper union reflecting observed usage: rank can be a number or
an object with a numeric position (e.g., { position: number }), so update the
JudgingResult type (the rank field) to rank?: number | { position: number } and
adjust any code that treats result.rank to use the union (e.g.,
result.rank?.position and typeof result.rank === 'number' checks in
JudgingResultsTable will then be correct).
- Around line 388-406: The non-array API response in getJudgingResults is being
cast unsafely to GetJudgingResultsResponse, which can leave data undefined at
runtime; update getJudgingResults to validate the response shape and return a
safe default (e.g., { success: false, message: res.data?.message || 'Error',
data: [] } or { success: true, data: [], message: 'Results retrieved
successfully' } when appropriate) instead of using (res.data || {}) as
GetJudgingResultsResponse; apply the same defensive fix to getSubmissionScores,
getHackathonJudges, and getJudgingWinners by checking whether res.data is the
expected object with a .data field (or an array where applicable) and supplying
an explicit data fallback so callers never receive undefined.
In `@lib/providers/OrganizationProvider.tsx`:
- Around line 1290-1294: The call to listMembers with a hardcoded query {
organizationId: orgId, limit: 100 } (used around the demotion step in
OrganizationProvider.tsx) can miss the current user for orgs >100 members;
replace this by either paginating through listMembers until you find the current
user or, preferably, call a dedicated membership lookup (e.g., getMembership or
listMemberById) if available to fetch the current user's membership directly;
update the logic that checks for the current user's role (the demotion branch
that relies on the listMembers result) to use the found membership from
pagination or the direct lookup and handle the case where membership is not
found by retrying/paging or throwing a clear error.
🧹 Nitpick comments (23)
lib/providers/OrganizationProvider.tsx (5)
1308-1315: Uselogger.errorinstead ofconsole.errorfor consistency.The entire file uses the structured
loggerinstance for logging (e.g.,logger.error({ eventType: '...', error })), but this catch block falls back toconsole.error. This breaks structured logging consistency and may bypass log-level filtering configured on line 14.♻️ Proposed fix
} catch (error) { - console.error( - 'Failed to demote previous owner during transfer:', - error - ); + logger.error({ + eventType: 'org.transfer_ownership.demote_error', + message: 'Failed to demote previous owner during transfer', + error, + }); // Continue execution - user might end up with two owners but better than failing completely // They can fix it manually now via the Members tab }
1316-1321: Dispatched org state may be stale after demotion step.
response(line 1275) captures the org state after promoting the new owner but before demoting the current user (lines 1301-1305). When dispatched asUPDATE_ORGANIZATIONon line 1318, the local state will reflect pre-demotion member roles. WhilefetchOrganizations()on line 1321 refreshes the organization list, the active org detail (state.activeOrg) will still carry the stale snapshot.Consider re-fetching the active org after a successful transfer to ensure full consistency:
♻️ Proposed fix — refresh active org after transfer
dispatch({ type: 'UPDATE_ORGANIZATION', payload: updatedOrg }); // Refresh organizations list to update current user's role in the UI await fetchOrganizations(); + + // Also refresh the active org detail so member roles are up to date + if (state.activeOrgId === orgId) { + await fetchActiveOrganization(orgId); + }This would require adding
fetchActiveOrganizationto the dependency array on line 1341.
1341-1341:state.activeOrgIdis unused in the callback body — unnecessary dependency.
transferOwnershipreceivesorgIdas a parameter and never referencesstate.activeOrgId. Including it in the dependency array causes the callback identity to change whenever the active org switches, which can trigger unnecessary re-renders in consumers without any functional benefit.dispatchis also stable (guaranteed by React'suseReducer) and doesn't need to be listed, though it's harmless.♻️ Proposed fix
- [dispatch, fetchOrganizations, state.activeOrgId] + [fetchOrganizations]If you adopt the stale-data fix above (using
fetchActiveOrganizationandstate.activeOrgIdinside the body), then keep them in the array.
1269-1342: Dual-owner window acknowledged — consider a toast for partial-failure visibility.The approach of promoting first, then demoting, is pragmatic. However, when the demotion silently fails (lines 1308-1315), the user has no indication that the org now has two owners. A brief warning toast in the catch block would surface this to the user so they can fix it immediately.
♻️ Proposed enhancement
} catch (error) { logger.error({ eventType: 'org.transfer_ownership.demote_error', message: 'Failed to demote previous owner during transfer', error, }); + toast.warning( + 'Ownership transferred, but your role was not automatically updated. Please adjust roles manually in the Members tab.' + ); // Continue execution - user might end up with two owners but better than failing completely // They can fix it manually now via the Members tab }
327-360: Large block of commented-out code — consider removing.This ~35-line commented-out
useEffectfor auto-refresh references variables that no longer exist (refreshTimeoutRef,fetchActiveOrgTimeoutRef,autoRefresh,refreshInterval). Dead code like this adds noise and maintenance burden. If the feature is planned for later, tracking it in an issue would be cleaner.components/organization/tabs/LinksTab.tsx (4)
229-270: Fragile error classification via string matching; unsafeas anycast.The chain of
errorObj.message.includes('401')/'403'/'404'etc. is brittle — it will silently mis-classify any message that happens to contain those substrings (e.g., a URL containing "404"). Also,error as any(line 230) defeats TypeScript's type safety.Consider:
- Checking HTTP status codes directly (if the error carries a
statusorstatusCodeproperty) instead of searching inside freeform strings.- Defining a typed error interface (or a type guard) instead of
as any.
282-377: Extract a reusableValidationIndicatorcomponent to reduce JSX duplication.The loader/check/error icon block and the error-message
<p>are repeated verbatim for website, X, and GitHub. A small helper component (or render function) acceptingvalidationStatewould eliminate ~60 lines of near-identical markup and make future styling changes a single edit.Sketch
const ValidationIndicator = ({ state }: { state: { isValid: boolean | null; isValidating: boolean; error: string | null } }) => ( <> {state.isValidating && ( <Loader2 className='absolute top-1/2 right-3 h-5 w-5 -translate-y-1/2 animate-spin text-gray-400' /> )} {!state.isValidating && state.isValid === true && ( <CheckCircle2 className='absolute top-1/2 right-3 h-5 w-5 -translate-y-1/2 text-green-500' /> )} {!state.isValidating && state.isValid === false && ( <XCircle className='absolute top-1/2 right-3 h-5 w-5 -translate-y-1/2 text-red-500' /> )} </> );
26-29: Use a const arrow function with explicit type annotation for the component.Per the coding guidelines for
*.tsxfiles, preferconstarrow functions with explicit types over function declarations.Diff
-export default function LinksTab({ - initialLinks = { website: '', x: '', github: '', others: '' }, - onSave, -}: LinksTabProps) { +const LinksTab: React.FC<LinksTabProps> = ({ + initialLinks = { website: '', x: '', github: '', others: '' }, + onSave, +}) => {(Add
export default LinksTab;at the bottom of the file.)As per coding guidelines,
**/*.{ts,tsx}: "Prefer const arrow functions with explicit type annotations over function declarations."
165-168: RenameupdateLinkto use thehandleprefix.This is an event handler invoked from
onChange; the coding guidelines require thehandleprefix.Diff
- const updateLink = (field: keyof OrganizationLinks, value: string) => { + const handleLinkChange = (field: keyof OrganizationLinks, value: string) => {Update all call sites (
onChange={e => handleLinkChange(…)}).As per coding guidelines,
**/*.{jsx,tsx}: "Event handlers should start with 'handle' prefix (e.g., handleClick, handleSubmit)."components/organization/tabs/TransferOwnershipTab.tsx (2)
121-125: Members are fetched even when the user is not the owner.
fetchMembersfires as soon asactiveOrgIdis set, regardless of whether the ownership check has completed or the user is actually the owner. This is a wasted API call for non-owners. Consider gating the fetch:Proposed fix
useEffect(() => { - if (activeOrgId) { + if (activeOrgId && !loadingOwnerCheck && isCurrentUserOwner) { fetchMembers(); } - }, [activeOrgId, fetchMembers]); + }, [activeOrgId, fetchMembers, loadingOwnerCheck, isCurrentUserOwner]);
80-82:authClient.getSession()is called on every member fetch.This adds a redundant async call each time
fetchMembersruns (including after a successful transfer at Line 160). Consider hoisting the session/user ID into a ref or obtaining it once from a context/provider that likely already has it.components/organization/cards/GradeSubmissionModal/useSubmissionScores.ts (2)
180-180:criteriaarray reference in the dependency list may cause spurious effect runs.If the parent component doesn't memoize the
criteriaarray, a new reference is created every render, re-triggering this effect. Consider stabilizing the dependency — e.g., derive a primitive key from criteria:Example
+ const criteriaKey = JSON.stringify(criteria.map(c => c.id ?? c.name)); + useEffect(() => { // ... - }, [open, organizationId, hackathonId, participantId, criteria]); + }, [open, organizationId, hackathonId, participantId, criteriaKey]);Keep
criteriaaccessible via a ref or the outer closure so the effect body still reads the current value.
130-153: Criterion-score matching is overly broad and relies on multipleas anycasts.The 10-way cross-match (lines 131–141) plus the fallback key (lines 143–147) try every permutation of
id,name, and the untypedtitlefield. This makes it very difficult to reason about which branch actually matches at runtime, and a single API contract change could silently break the mapping.Consider aligning the
JudgingCriterionandCriterionScoretypes to share a single canonical identifier, then matching on that alone. If backward compatibility requires multiple fields, addtitleto the type definition rather than casting throughany.components/hackathons/submissions/SubmissionForm.tsx (1)
565-576:teamId ?? undefinedis a no-op.On line 563,
teamIdis already typedstring | undefined, so?? undefinedhas no effect. Harmless but redundant.app/(landing)/organizations/[id]/hackathons/[hackathonId]/judging/page.tsx (3)
126-145:getSession()is called on everyfetchJudgesinvocation, which is already triggered multiple times.
fetchJudgesis called fromfetchData,handleAddJudge,handleRemoveJudge, and tab switches — each time making an extra round-trip for the session. Consider fetching the session once on mount and storing it, or lifting it into a context/provider that already exists (authClient).
45-46: Pervasiveanyusage fororgMembersandcurrentJudges.Both state variables are typed
any[], and downstream usages also cast toany. This undermines type safety across the component. Consider defining minimal interfaces (e.g.,OrgMember,Judge) to catch shape mismatches at compile time.
34-34: Component usesfunctiondeclaration instead ofconstarrow function.Per coding guidelines for
*.tsxfiles: "Prefer const arrow functions with explicit type annotations over function declarations."Proposed fix
-export default function JudgingPage() { +const JudgingPage = () => { // ... component body ... -} +}; + +export default JudgingPage;As per coding guidelines: "Prefer const arrow functions with explicit type annotations over function declarations."
lib/api/hackathons/judging.ts (2)
184-205: Response types hardcodesuccess: true, making them unable to represent error states.
GetJudgingSubmissionsResponse,AddJudgeResponse,RemoveJudgeResponseall declaresuccess: trueas a literal type. This means TypeScript will reject assigning a{ success: false, ... }response to these types, forcing unsafe casts elsewhere. Consider usingbooleanor removing the literal override sinceApiResponsealready defines it.
347-358: Duplicated array-wrapping logic across four API functions.
getSubmissionScores,getJudgingResults,getHackathonJudges, andgetJudgingWinnersall repeat the same pattern: checkArray.isArray(res.data), wrap in{ success, data, message }, else cast. Extract a helper to DRY this up:Proposed helper
const wrapArrayResponse = <T>( data: T[] | ApiResponse<T[]> | null | undefined, fallbackMessage: string ): ApiResponse<T[]> => { if (Array.isArray(data)) { return { success: true, data, message: fallbackMessage }; } const response = data as ApiResponse<T[]>; return { success: response?.success ?? false, data: response?.data ?? [], message: response?.message ?? '', }; };Also applies to: 392-406, 444-458, 463-481
components/organization/cards/JudgingParticipant/IndividualScoresBreakdown.tsx (2)
51-56: RenametoggleExpandtohandleToggleExpand.Per coding guidelines, event handlers should start with the
handleprefix.As per coding guidelines: "Event handlers should start with 'handle' prefix (e.g., handleClick, handleSubmit)."
169-194: Using array index as key forcriteriaScoresitems.
idxis used as the React key. IfcriterionIdis available (and it should be based on theCriterionScoreinterface), prefer it for stable identity:- {score.criteriaScores.map((c, idx) => ( - <div key={idx} className='space-y-1.5'> + {score.criteriaScores.map((c) => ( + <div key={c.criterionId} className='space-y-1.5'>components/organization/hackathons/judging/JudgingResultsTable.tsx (2)
82-153: Each expand/collapse re-fetches scores sinceIndividualScoresBreakdownre-mounts.When a row is collapsed, the
IndividualScoresBreakdowncomponent unmounts (line 138 conditional rendering). Re-expanding it mounts a new instance that re-fetches from the API. For a table where users may toggle rows frequently, consider either:
- Keeping expanded components mounted but hidden (
display: none), or- Lifting the scores cache to this component level.
This is a nice-to-have — the current approach is simpler and correct.
40-45: RenametoggleRowtohandleToggleRow.Per coding guidelines, event handlers should start with the
handleprefix.As per coding guidelines: "Event handlers should start with 'handle' prefix (e.g., handleClick, handleSubmit)."
| const fetchResults = useCallback(async () => { | ||
| if (!organizationId || !hackathonId) return; | ||
|
|
||
| setIsFetchingResults(true); | ||
| try { | ||
| const res = await getJudgingResults(organizationId, hackathonId); | ||
| console.log('Judging Results Response:', res); | ||
| if (res.success) { | ||
| setJudgingResults(res.data || []); | ||
| } else { | ||
| setJudgingResults([]); | ||
| toast.error(res.message || 'Failed to load judging results'); | ||
| } | ||
| } catch (error: any) { | ||
| console.error('Error fetching results:', error); | ||
| setJudgingResults([]); | ||
| toast.error( | ||
| error.response?.data?.message || | ||
| error.message || | ||
| 'Failed to load judging results' | ||
| ); | ||
| } finally { | ||
| setIsFetchingResults(false); | ||
| } | ||
| }, [organizationId, hackathonId]); |
There was a problem hiding this comment.
Remove console.log left on line 154.
Debug logging (console.log('Judging Results Response:', res)) should not ship to production. It can leak response data in browser consoles.
- console.log('Judging Results Response:', res);📝 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.
| const fetchResults = useCallback(async () => { | |
| if (!organizationId || !hackathonId) return; | |
| setIsFetchingResults(true); | |
| try { | |
| const res = await getJudgingResults(organizationId, hackathonId); | |
| console.log('Judging Results Response:', res); | |
| if (res.success) { | |
| setJudgingResults(res.data || []); | |
| } else { | |
| setJudgingResults([]); | |
| toast.error(res.message || 'Failed to load judging results'); | |
| } | |
| } catch (error: any) { | |
| console.error('Error fetching results:', error); | |
| setJudgingResults([]); | |
| toast.error( | |
| error.response?.data?.message || | |
| error.message || | |
| 'Failed to load judging results' | |
| ); | |
| } finally { | |
| setIsFetchingResults(false); | |
| } | |
| }, [organizationId, hackathonId]); | |
| const fetchResults = useCallback(async () => { | |
| if (!organizationId || !hackathonId) return; | |
| setIsFetchingResults(true); | |
| try { | |
| const res = await getJudgingResults(organizationId, hackathonId); | |
| if (res.success) { | |
| setJudgingResults(res.data || []); | |
| } else { | |
| setJudgingResults([]); | |
| toast.error(res.message || 'Failed to load judging results'); | |
| } | |
| } catch (error: any) { | |
| console.error('Error fetching results:', error); | |
| setJudgingResults([]); | |
| toast.error( | |
| error.response?.data?.message || | |
| error.message || | |
| 'Failed to load judging results' | |
| ); | |
| } finally { | |
| setIsFetchingResults(false); | |
| } | |
| }, [organizationId, hackathonId]); |
🤖 Prompt for AI Agents
In `@app/`(landing)/organizations/[id]/hackathons/[hackathonId]/judging/page.tsx
around lines 148 - 172, In the useCallback fetchResults function remove the
leftover debug console.log('Judging Results Response:', res) so response data is
not leaked to the browser console; locate the log inside fetchResults
(referencing getJudgingResults, res, and setJudgingResults) and delete that
console.log call (or replace it with a conditional debug-only logger gated by
NODE_ENV !== 'production' if you need retained debug output).
| const fetchData = useCallback(async () => { | ||
| if (!organizationId || !hackathonId) return; | ||
|
|
||
| setIsLoading(true); | ||
| try { | ||
| // Fetch submissions, criteria, and judges/members | ||
| const [submissionsRes, criteriaRes] = await Promise.all([ | ||
| getJudgingSubmissions(organizationId, hackathonId, 1, 50), | ||
| getJudgingCriteria(hackathonId), | ||
| ]); | ||
|
|
||
| // Trigger judges and results fetch in parallel but handle separately | ||
| fetchJudges(); | ||
| fetchResults(); | ||
|
|
||
| let enrichedSubmissions: JudgingSubmission[] = []; | ||
|
|
||
| if (submissionsRes.success) { | ||
| // Standard submissions endpoint returns { data: { submissions: [], pagination: {} } } | ||
| const submissionData = | ||
| (submissionsRes.data as any)?.submissions || | ||
| submissionsRes.data || | ||
| []; | ||
| const basicSubmissions = Array.isArray(submissionData) | ||
| ? submissionData | ||
| : []; | ||
|
|
||
| // 2. Fetch full details for each submission to get user info | ||
| // We do this by fetching the project details, as submission endpoints lack user data | ||
| const detailsPromises = basicSubmissions.map(async (sub: any) => { | ||
| try { | ||
| // Check if we already have sufficient user data | ||
| if ( | ||
| sub.participant?.user?.profile?.firstName || | ||
| sub.participant?.name | ||
| ) | ||
| return sub; | ||
|
|
||
| // Try fetch project details if we have projectId | ||
| if (sub.projectId) { | ||
| const project = await getCrowdfundingProject(sub.projectId); | ||
| if (project && project.project && project.project.creator) { | ||
| const creator = project.project.creator; | ||
| return { | ||
| ...sub, | ||
| participant: { | ||
| ...sub.participant, | ||
| // Use creator info for participant | ||
| name: creator.name, | ||
| username: creator.username, | ||
| image: creator.image, | ||
| email: creator.email, | ||
| user: { | ||
| ...sub.participant?.user, | ||
| name: creator.name, | ||
| username: creator.username, | ||
| image: creator.image, | ||
| email: creator.email, | ||
| profile: { | ||
| ...sub.participant?.user?.profile, | ||
| firstName: creator.name?.split(' ')[0] || '', | ||
| lastName: | ||
| creator.name?.split(' ').slice(1).join(' ') || '', | ||
| username: creator.username, | ||
| avatar: creator.image, | ||
| }, | ||
| }, | ||
| }, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| // Fallback to submission details check if project fail or no projectId | ||
| const detailsRes = await getSubmissionDetails(sub.id); | ||
| if (detailsRes.success && detailsRes.data) { | ||
| const details = detailsRes.data as any; | ||
| return { | ||
| ...sub, | ||
| participant: { | ||
| ...sub.participant, | ||
| ...details.participant, | ||
| user: details.participant?.user || sub.participant?.user, | ||
| }, | ||
| }; | ||
| } | ||
| return sub; | ||
| } catch (err) { | ||
| console.error( | ||
| `Failed to fetch details for submission ${sub.id}`, | ||
| err | ||
| ); | ||
| return sub; | ||
| } | ||
| }); | ||
|
|
||
| enrichedSubmissions = await Promise.all(detailsPromises); | ||
| setSubmissions(enrichedSubmissions); | ||
| } else { | ||
| setSubmissions([]); | ||
| } | ||
|
|
||
| // Handle criteria response safely | ||
| setCriteria(Array.isArray(criteriaRes) ? criteriaRes : []); | ||
| } catch (error) { | ||
| console.error('Judging data fetch error:', error); | ||
| toast.error('Failed to load judging data'); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }, [organizationId, hackathonId, fetchJudges, fetchResults]); |
There was a problem hiding this comment.
Fire-and-forget async calls inside fetchData cause timing issues with isLoading.
fetchJudges() and fetchResults() (lines 186–187) are called without await, so isLoading is set to false in the finally block before judges and results have actually loaded. This means the "Overview" tab spinner disappears while judge/role data is still in-flight, and errors from these calls bypass the catch block's toast.
Consider awaiting them (or at least acknowledging the race in a comment):
Proposed fix — await parallel fetches
- // Trigger judges and results fetch in parallel but handle separately
- fetchJudges();
- fetchResults();
+ // Fetch judges and results in parallel
+ await Promise.all([fetchJudges(), fetchResults()]);📝 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.
| const fetchData = useCallback(async () => { | |
| if (!organizationId || !hackathonId) return; | |
| setIsLoading(true); | |
| try { | |
| // Fetch submissions, criteria, and judges/members | |
| const [submissionsRes, criteriaRes] = await Promise.all([ | |
| getJudgingSubmissions(organizationId, hackathonId, 1, 50), | |
| getJudgingCriteria(hackathonId), | |
| ]); | |
| // Trigger judges and results fetch in parallel but handle separately | |
| fetchJudges(); | |
| fetchResults(); | |
| let enrichedSubmissions: JudgingSubmission[] = []; | |
| if (submissionsRes.success) { | |
| // Standard submissions endpoint returns { data: { submissions: [], pagination: {} } } | |
| const submissionData = | |
| (submissionsRes.data as any)?.submissions || | |
| submissionsRes.data || | |
| []; | |
| const basicSubmissions = Array.isArray(submissionData) | |
| ? submissionData | |
| : []; | |
| // 2. Fetch full details for each submission to get user info | |
| // We do this by fetching the project details, as submission endpoints lack user data | |
| const detailsPromises = basicSubmissions.map(async (sub: any) => { | |
| try { | |
| // Check if we already have sufficient user data | |
| if ( | |
| sub.participant?.user?.profile?.firstName || | |
| sub.participant?.name | |
| ) | |
| return sub; | |
| // Try fetch project details if we have projectId | |
| if (sub.projectId) { | |
| const project = await getCrowdfundingProject(sub.projectId); | |
| if (project && project.project && project.project.creator) { | |
| const creator = project.project.creator; | |
| return { | |
| ...sub, | |
| participant: { | |
| ...sub.participant, | |
| // Use creator info for participant | |
| name: creator.name, | |
| username: creator.username, | |
| image: creator.image, | |
| email: creator.email, | |
| user: { | |
| ...sub.participant?.user, | |
| name: creator.name, | |
| username: creator.username, | |
| image: creator.image, | |
| email: creator.email, | |
| profile: { | |
| ...sub.participant?.user?.profile, | |
| firstName: creator.name?.split(' ')[0] || '', | |
| lastName: | |
| creator.name?.split(' ').slice(1).join(' ') || '', | |
| username: creator.username, | |
| avatar: creator.image, | |
| }, | |
| }, | |
| }, | |
| }; | |
| } | |
| } | |
| // Fallback to submission details check if project fail or no projectId | |
| const detailsRes = await getSubmissionDetails(sub.id); | |
| if (detailsRes.success && detailsRes.data) { | |
| const details = detailsRes.data as any; | |
| return { | |
| ...sub, | |
| participant: { | |
| ...sub.participant, | |
| ...details.participant, | |
| user: details.participant?.user || sub.participant?.user, | |
| }, | |
| }; | |
| } | |
| return sub; | |
| } catch (err) { | |
| console.error( | |
| `Failed to fetch details for submission ${sub.id}`, | |
| err | |
| ); | |
| return sub; | |
| } | |
| }); | |
| enrichedSubmissions = await Promise.all(detailsPromises); | |
| setSubmissions(enrichedSubmissions); | |
| } else { | |
| setSubmissions([]); | |
| } | |
| // Handle criteria response safely | |
| setCriteria(Array.isArray(criteriaRes) ? criteriaRes : []); | |
| } catch (error) { | |
| console.error('Judging data fetch error:', error); | |
| toast.error('Failed to load judging data'); | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| }, [organizationId, hackathonId, fetchJudges, fetchResults]); | |
| const fetchData = useCallback(async () => { | |
| if (!organizationId || !hackathonId) return; | |
| setIsLoading(true); | |
| try { | |
| // Fetch submissions, criteria, and judges/members | |
| const [submissionsRes, criteriaRes] = await Promise.all([ | |
| getJudgingSubmissions(organizationId, hackathonId, 1, 50), | |
| getJudgingCriteria(hackathonId), | |
| ]); | |
| // Fetch judges and results in parallel | |
| await Promise.all([fetchJudges(), fetchResults()]); | |
| let enrichedSubmissions: JudgingSubmission[] = []; | |
| if (submissionsRes.success) { | |
| // Standard submissions endpoint returns { data: { submissions: [], pagination: {} } } | |
| const submissionData = | |
| (submissionsRes.data as any)?.submissions || | |
| submissionsRes.data || | |
| []; | |
| const basicSubmissions = Array.isArray(submissionData) | |
| ? submissionData | |
| : []; | |
| // 2. Fetch full details for each submission to get user info | |
| // We do this by fetching the project details, as submission endpoints lack user data | |
| const detailsPromises = basicSubmissions.map(async (sub: any) => { | |
| try { | |
| // Check if we already have sufficient user data | |
| if ( | |
| sub.participant?.user?.profile?.firstName || | |
| sub.participant?.name | |
| ) | |
| return sub; | |
| // Try fetch project details if we have projectId | |
| if (sub.projectId) { | |
| const project = await getCrowdfundingProject(sub.projectId); | |
| if (project && project.project && project.project.creator) { | |
| const creator = project.project.creator; | |
| return { | |
| ...sub, | |
| participant: { | |
| ...sub.participant, | |
| // Use creator info for participant | |
| name: creator.name, | |
| username: creator.username, | |
| image: creator.image, | |
| email: creator.email, | |
| user: { | |
| ...sub.participant?.user, | |
| name: creator.name, | |
| username: creator.username, | |
| image: creator.image, | |
| email: creator.email, | |
| profile: { | |
| ...sub.participant?.user?.profile, | |
| firstName: creator.name?.split(' ')[0] || '', | |
| lastName: | |
| creator.name?.split(' ').slice(1).join(' ') || '', | |
| username: creator.username, | |
| avatar: creator.image, | |
| }, | |
| }, | |
| }, | |
| }; | |
| } | |
| } | |
| // Fallback to submission details check if project fail or no projectId | |
| const detailsRes = await getSubmissionDetails(sub.id); | |
| if (detailsRes.success && detailsRes.data) { | |
| const details = detailsRes.data as any; | |
| return { | |
| ...sub, | |
| participant: { | |
| ...sub.participant, | |
| ...details.participant, | |
| user: details.participant?.user || sub.participant?.user, | |
| }, | |
| }; | |
| } | |
| return sub; | |
| } catch (err) { | |
| console.error( | |
| `Failed to fetch details for submission ${sub.id}`, | |
| err | |
| ); | |
| return sub; | |
| } | |
| }); | |
| enrichedSubmissions = await Promise.all(detailsPromises); | |
| setSubmissions(enrichedSubmissions); | |
| } else { | |
| setSubmissions([]); | |
| } | |
| // Handle criteria response safely | |
| setCriteria(Array.isArray(criteriaRes) ? criteriaRes : []); | |
| } catch (error) { | |
| console.error('Judging data fetch error:', error); | |
| toast.error('Failed to load judging data'); | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| }, [organizationId, hackathonId, fetchJudges, fetchResults]); |
🤖 Prompt for AI Agents
In `@app/`(landing)/organizations/[id]/hackathons/[hackathonId]/judging/page.tsx
around lines 174 - 283, fetchData currently calls fetchJudges() and
fetchResults() without awaiting them, so setIsLoading(false) fires before those
requests finish; change to await their execution (preferably in parallel) so
loading/error handling covers them — e.g. replace the fire-and-forget calls
inside fetchData with awaiting a combined promise (Promise.all or
Promise.allSettled) of fetchJudges() and fetchResults(), and handle their
failures (propagate or log/throw) so the catch block's toast is triggered; keep
names fetchData, fetchJudges, fetchResults, setIsLoading and ensure finally only
runs after those awaited calls complete.
| const handleSuccess = () => { | ||
| fetchSubmissions(page); | ||
| fetchData(); | ||
| fetchResults(); // Refresh results to update metrics/table | ||
| }; |
There was a problem hiding this comment.
handleSuccess double-triggers fetchResults.
fetchData() already calls fetchResults() internally (line 187), so the explicit fetchResults() on line 291 causes a redundant API call every time a grade is submitted.
Proposed fix
const handleSuccess = () => {
fetchData();
- fetchResults(); // Refresh results to update metrics/table
};🤖 Prompt for AI Agents
In `@app/`(landing)/organizations/[id]/hackathons/[hackathonId]/judging/page.tsx
around lines 289 - 292, handleSuccess is redundantly calling fetchResults() even
though fetchData() already invokes fetchResults() (see fetchData and
fetchResults). Remove the explicit fetchResults() call from handleSuccess so it
only calls fetchData(); ensure handleSuccess remains the post-submit callback
that triggers fetchData() which in turn refreshes results/metrics.
| <p className='text-sm font-medium'> | ||
| {judge.name} | ||
| </p> | ||
| <p className='text-xs text-gray-500'> | ||
| {judge.userId} | ||
| </p> |
There was a problem hiding this comment.
Displaying raw judge.userId to the user.
Line 520 renders the internal userId under each judge name. This is likely not meaningful to end users and could be a minor PII/internal-data exposure concern. Consider showing the judge's email or role instead.
<p className='text-xs text-gray-500'>
- {judge.userId}
+ {judge.email || judge.role || ''}
</p>🤖 Prompt for AI Agents
In `@app/`(landing)/organizations/[id]/hackathons/[hackathonId]/judging/page.tsx
around lines 516 - 521, The UI is rendering internal judge.userId which may
expose PII/internal IDs; update the rendering to show a user-facing attribute
(e.g., judge.email or judge.role) instead of judge.userId, or remove the second
<p> entirely if no friendly field exists. Locate the JSX that displays
{judge.name} and {judge.userId} and replace {judge.userId} with the appropriate
display field (judge.email or judge.role) and add a safe fallback (e.g.,
fallback text like "No contact info") to avoid rendering raw IDs.
| // Participation type enforcement | ||
| useEffect(() => { | ||
| if (!open || !currentHackathon) return; | ||
| if (hasAutoAdvanced.current) return; | ||
|
|
||
| const hackathonType = currentHackathon.participantType; | ||
|
|
||
| if (hackathonType === 'INDIVIDUAL') { | ||
| form.setValue('participationType', 'INDIVIDUAL'); | ||
| if (!submissionId) { | ||
| setCurrentStep(1); | ||
| updateStepState(0, 'completed'); | ||
| updateStepState(1, 'active'); | ||
| hasAutoAdvanced.current = true; | ||
| } | ||
| } else if (hackathonType === 'TEAM') { | ||
| form.setValue('participationType', 'TEAM'); | ||
| if (!submissionId && !!myTeam) { | ||
| setCurrentStep(1); | ||
| updateStepState(0, 'completed'); | ||
| updateStepState(1, 'active'); | ||
| hasAutoAdvanced.current = true; | ||
| } | ||
| } else if (hackathonType === 'TEAM_OR_INDIVIDUAL') { | ||
| if (!submissionId) { | ||
| if (myTeam) { | ||
| form.setValue('participationType', 'TEAM'); | ||
| setCurrentStep(1); | ||
| updateStepState(0, 'completed'); | ||
| updateStepState(1, 'active'); | ||
| hasAutoAdvanced.current = true; | ||
| } else { | ||
| form.setValue('participationType', 'INDIVIDUAL'); | ||
| } | ||
| } | ||
| } | ||
| }, [open, currentHackathon, myTeam, submissionId, form, updateStepState]); |
There was a problem hiding this comment.
Late myTeam load can override manual user progress.
If hackathonType is TEAM_OR_INDIVIDUAL (or TEAM) and myTeam is initially undefined, hasAutoAdvanced stays false. If the user manually selects INDIVIDUAL and navigates forward, a later async resolution of myTeam will re-trigger this effect, overwrite participationType to TEAM, and call setCurrentStep(1) — yanking the user back and discarding their choice.
Consider setting hasAutoAdvanced.current = true when the user manually advances past step 0 (e.g., in handleNext), so this effect can no longer fire.
Proposed fix
In handleNext, after successfully advancing from step 0:
if (isValid && currentStep < steps.length - 1) {
+ if (currentStep === 0) {
+ hasAutoAdvanced.current = true;
+ }
updateStepState(currentStep, 'completed');
updateStepState(currentStep + 1, 'active');
setCurrentStep(currentStep + 1);
}🤖 Prompt for AI Agents
In `@components/hackathons/submissions/SubmissionForm.tsx` around lines 247 - 283,
The effect that enforces participation type can later override a user’s manual
choice because hasAutoAdvanced.current is only set when the effect itself
auto-advances; modify the manual advancement logic (e.g., in the handleNext
handler) so that when the user successfully advances past step 0 (the same
transition where setCurrentStep(1) and updateStepState(0/1) are called), you
also set hasAutoAdvanced.current = true; this prevents the useEffect (which
reads currentHackathon, myTeam, submissionId, and calls form.setValue /
setCurrentStep) from re-running later and clobbering the user’s manually chosen
participationType.
| // Validate website URL | ||
| useEffect(() => { | ||
| if (!debouncedWebsite) { | ||
| setValidationStates(prev => ({ | ||
| ...prev, | ||
| website: { isValid: null, isValidating: false, error: null }, | ||
| })); | ||
| return; | ||
| } | ||
|
|
||
| const validate = async () => { | ||
| setValidationStates(prev => ({ | ||
| ...prev, | ||
| website: { ...prev.website, isValidating: true }, | ||
| })); | ||
| const result = await validateWebsite(debouncedWebsite); | ||
| setValidationStates(prev => ({ ...prev, website: result })); | ||
| }; | ||
|
|
||
| validate(); | ||
| }, [debouncedWebsite, validateWebsite]); | ||
|
|
||
| // Validate X handle | ||
| useEffect(() => { | ||
| if (!debouncedX) { | ||
| setValidationStates(prev => ({ | ||
| ...prev, | ||
| x: { isValid: null, isValidating: false, error: null }, | ||
| })); | ||
| return; | ||
| } | ||
|
|
||
| const validate = async () => { | ||
| setValidationStates(prev => ({ | ||
| ...prev, | ||
| x: { ...prev.x, isValidating: true }, | ||
| })); | ||
| const result = await validateXHandle(debouncedX); | ||
| setValidationStates(prev => ({ ...prev, x: result })); | ||
| }; | ||
|
|
||
| validate(); | ||
| }, [debouncedX, validateXHandle]); | ||
|
|
||
| // Validate GitHub handle | ||
| useEffect(() => { | ||
| if (!debouncedGithub) { | ||
| setValidationStates(prev => ({ | ||
| ...prev, | ||
| github: { isValid: null, isValidating: false, error: null }, | ||
| })); | ||
| return; | ||
| } | ||
|
|
||
| const validate = async () => { | ||
| setValidationStates(prev => ({ | ||
| ...prev, | ||
| github: { ...prev.github, isValidating: true }, | ||
| })); | ||
| const result = await validateGithubHandle(debouncedGithub); | ||
| setValidationStates(prev => ({ ...prev, github: result })); | ||
| }; | ||
|
|
||
| validate(); | ||
| }, [debouncedGithub, validateGithubHandle]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Stale async results & heavy duplication across three identical validation effects.
Two concerns:
-
Race condition on rapid input: If two debounced values fire in quick succession, there's no cancellation of the previous
validate()call. The earlier promise can resolve after the later one, overwriting the correct result with a stale one. Use anisCancelledflag (orAbortController) in each effect's cleanup to discard outdated results. -
DRY violation: All three effects (website, X, GitHub) share the exact same structure. Extract a single helper or a custom hook (e.g.,
useFieldValidation(debouncedValue, validatorFn, fieldKey)) that encapsulates the debounce-triggered validation + cancellation logic.
Example: cancellation guard + extracted helper
- // Validate website URL
- useEffect(() => {
- if (!debouncedWebsite) {
- setValidationStates(prev => ({
- ...prev,
- website: { isValid: null, isValidating: false, error: null },
- }));
- return;
- }
-
- const validate = async () => {
- setValidationStates(prev => ({
- ...prev,
- website: { ...prev.website, isValidating: true },
- }));
- const result = await validateWebsite(debouncedWebsite);
- setValidationStates(prev => ({ ...prev, website: result }));
- };
-
- validate();
- }, [debouncedWebsite, validateWebsite]);
+ // Reusable validation runner with stale-result cancellation
+ const useFieldValidation = (
+ debouncedValue: string,
+ validatorFn: (v: string) => Promise<ValidationResult>,
+ fieldKey: 'website' | 'x' | 'github'
+ ) => {
+ useEffect(() => {
+ let cancelled = false;
+ if (!debouncedValue) {
+ setValidationStates(prev => ({
+ ...prev,
+ [fieldKey]: { isValid: null, isValidating: false, error: null },
+ }));
+ return;
+ }
+ setValidationStates(prev => ({
+ ...prev,
+ [fieldKey]: { ...prev[fieldKey], isValidating: true },
+ }));
+ validatorFn(debouncedValue).then(result => {
+ if (!cancelled) {
+ setValidationStates(prev => ({ ...prev, [fieldKey]: result }));
+ }
+ });
+ return () => { cancelled = true; };
+ }, [debouncedValue, validatorFn, fieldKey]);
+ };
+
+ useFieldValidation(debouncedWebsite, validateWebsite, 'website');
+ useFieldValidation(debouncedX, validateXHandle, 'x');
+ useFieldValidation(debouncedGithub, validateGithubHandle, 'github');(The helper above is inline for brevity — ideally move it to its own hook file.)
🤖 Prompt for AI Agents
In `@components/organization/tabs/LinksTab.tsx` around lines 99 - 163, The three
near-identical useEffect blocks (validating debouncedWebsite, debouncedX,
debouncedGithub using validateWebsite/validateXHandle/validateGithubHandle and
updating setValidationStates) are duplicated and prone to stale async results;
extract a helper/custom hook (e.g., useFieldValidation(debouncedValue,
validatorFn, fieldKey)) that encapsulates the shared logic: set isValidating
true, call the validator, and in the effect cleanup set an isCancelled flag (or
use AbortController) so the async result is ignored if cancelled; only call
setValidationStates(fieldKey, result) when not cancelled, and ensure
empty/cleared state is handled when debouncedValue is falsy.
| if (loadingMembers) { | ||
| return ( | ||
| <div className='relative h-[calc(100vh-300px)] space-y-8'> | ||
| {loadingui && ( | ||
| <div className='absolute top-0 right-0 bottom-0 left-0 z-10 flex items-center justify-center bg-black/50 backdrop-blur-sm'> | ||
| <LoadingSpinner size='lg' className='z-20 text-white' /> | ||
| <div className='absolute inset-0 z-10 flex items-center justify-center rounded-[12px] bg-black/50 backdrop-blur-sm'> | ||
| <LoadingSpinner size='lg' className='z-20 text-white' /> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
| // Show permission denied message if not owner | ||
| if (!loadingMembers && !loadingOwnerCheck && !isCurrentUserOwner) { | ||
| return ( | ||
| <div className='bg-background-card rounded-[12px] border border-gray-900 p-6'> | ||
| <div className='flex flex-col items-center justify-center space-y-4 py-8'> | ||
| <div className='rounded-full bg-red-500/10 p-3'> | ||
| <svg | ||
| className='h-8 w-8 text-red-500' | ||
| fill='none' | ||
| viewBox='0 0 24 24' | ||
| stroke='currentColor' | ||
| > | ||
| <path | ||
| strokeLinecap='round' | ||
| strokeLinejoin='round' | ||
| strokeWidth={2} | ||
| d='M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z' | ||
| /> | ||
| </svg> | ||
| </div> | ||
| )} | ||
| <div className='text-center'> | ||
| <h3 className='text-lg font-medium text-white'> | ||
| Permission Denied | ||
| </h3> | ||
| <p className='mt-2 text-sm text-gray-400'> | ||
| Only the organization owner can transfer ownership. | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Early-return ordering lets non-owners see the members loading overlay.
The loadingMembers check (Line 185) precedes the permission-denied check (Line 195). If both loadingOwnerCheck is false and isCurrentUserOwner is false, but loadingMembers is still true (due to the parallel fetch), the user briefly sees a loading overlay instead of the permission-denied screen. Move the permission check above the members-loading check:
Proposed fix
+ // Show permission denied message if not owner
+ if (!loadingOwnerCheck && !isCurrentUserOwner) {
+ return (
+ <div className='bg-background-card rounded-[12px] border border-gray-900 p-6'>
+ ...permission denied UI...
+ </div>
+ );
+ }
+
if (loadingMembers) {
return (
...loading overlay...
);
}
- // Show permission denied message if not owner
- if (!loadingMembers && !loadingOwnerCheck && !isCurrentUserOwner) {
- ...
- }This becomes a non-issue if you also gate fetchMembers behind the ownership check (previous comment), but the ordering is still worth correcting defensively.
📝 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 (loadingMembers) { | |
| return ( | |
| <div className='relative h-[calc(100vh-300px)] space-y-8'> | |
| {loadingui && ( | |
| <div className='absolute top-0 right-0 bottom-0 left-0 z-10 flex items-center justify-center bg-black/50 backdrop-blur-sm'> | |
| <LoadingSpinner size='lg' className='z-20 text-white' /> | |
| <div className='absolute inset-0 z-10 flex items-center justify-center rounded-[12px] bg-black/50 backdrop-blur-sm'> | |
| <LoadingSpinner size='lg' className='z-20 text-white' /> | |
| </div> | |
| </div> | |
| ); | |
| } | |
| // Show permission denied message if not owner | |
| if (!loadingMembers && !loadingOwnerCheck && !isCurrentUserOwner) { | |
| return ( | |
| <div className='bg-background-card rounded-[12px] border border-gray-900 p-6'> | |
| <div className='flex flex-col items-center justify-center space-y-4 py-8'> | |
| <div className='rounded-full bg-red-500/10 p-3'> | |
| <svg | |
| className='h-8 w-8 text-red-500' | |
| fill='none' | |
| viewBox='0 0 24 24' | |
| stroke='currentColor' | |
| > | |
| <path | |
| strokeLinecap='round' | |
| strokeLinejoin='round' | |
| strokeWidth={2} | |
| d='M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z' | |
| /> | |
| </svg> | |
| </div> | |
| )} | |
| <div className='text-center'> | |
| <h3 className='text-lg font-medium text-white'> | |
| Permission Denied | |
| </h3> | |
| <p className='mt-2 text-sm text-gray-400'> | |
| Only the organization owner can transfer ownership. | |
| </p> | |
| </div> | |
| </div> | |
| </div> | |
| ); | |
| } | |
| // Show permission denied message if not owner | |
| if (!loadingOwnerCheck && !isCurrentUserOwner) { | |
| return ( | |
| <div className='bg-background-card rounded-[12px] border border-gray-900 p-6'> | |
| <div className='flex flex-col items-center justify-center space-y-4 py-8'> | |
| <div className='rounded-full bg-red-500/10 p-3'> | |
| <svg | |
| className='h-8 w-8 text-red-500' | |
| fill='none' | |
| viewBox='0 0 24 24' | |
| stroke='currentColor' | |
| > | |
| <path | |
| strokeLinecap='round' | |
| strokeLinejoin='round' | |
| strokeWidth={2} | |
| d='M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z' | |
| /> | |
| </svg> | |
| </div> | |
| <div className='text-center'> | |
| <h3 className='text-lg font-medium text-white'> | |
| Permission Denied | |
| </h3> | |
| <p className='mt-2 text-sm text-gray-400'> | |
| Only the organization owner can transfer ownership. | |
| </p> | |
| </div> | |
| </div> | |
| </div> | |
| ); | |
| } | |
| if (loadingMembers) { | |
| return ( | |
| <div className='relative h-[calc(100vh-300px)] space-y-8'> | |
| <div className='absolute inset-0 z-10 flex items-center justify-center rounded-[12px] bg-black/50 backdrop-blur-sm'> | |
| <LoadingSpinner size='lg' className='z-20 text-white' /> | |
| </div> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@components/organization/tabs/TransferOwnershipTab.tsx` around lines 185 -
225, The permission-denied early return is after the members-loading check,
causing non-owners to briefly see the members loading overlay; move the
ownership check (using loadingOwnerCheck and isCurrentUserOwner) above the
loadingMembers check so you return the permission-denied UI immediately when
ownership has been determined false, or alternatively gate fetchMembers behind
the ownership check so loadingMembers never becomes true for non-owners (update
references in TransferOwnershipTab to loadingOwnerCheck, isCurrentUserOwner,
loadingMembers and any fetchMembers call).
| export interface JudgingResult { | ||
| submissionId: string; | ||
| projectName: string; | ||
| averageScore: number; | ||
| judgeCount: number; | ||
| rank?: any; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
rank typed as any — define its actual shape.
rank?: any discards type information. Based on usage in JudgingResultsTable (which accesses result.rank?.position and checks typeof result.rank === 'number'), it should be a union type:
- rank?: any;
+ rank?: number | { position: number };📝 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.
| export interface JudgingResult { | |
| submissionId: string; | |
| projectName: string; | |
| averageScore: number; | |
| judgeCount: number; | |
| rank?: any; | |
| } | |
| export interface JudgingResult { | |
| submissionId: string; | |
| projectName: string; | |
| averageScore: number; | |
| judgeCount: number; | |
| rank?: number | { position: number }; | |
| } |
🤖 Prompt for AI Agents
In `@lib/api/hackathons/judging.ts` around lines 50 - 56, The JudgingResult
interface currently types rank as any; change it to a proper union reflecting
observed usage: rank can be a number or an object with a numeric position (e.g.,
{ position: number }), so update the JudgingResult type (the rank field) to
rank?: number | { position: number } and adjust any code that treats result.rank
to use the union (e.g., result.rank?.position and typeof result.rank ===
'number' checks in JudgingResultsTable will then be correct).
| export const getJudgingResults = async ( | ||
| organizationId: string, | ||
| hackathonId: string | ||
| ): Promise<GetJudgingResultsResponse> => { | ||
| const res = await api.get<JudgingResult[] | ApiResponse<JudgingResult[]>>( | ||
| `/organizations/${organizationId}/hackathons/${hackathonId}/judging/results` | ||
| ); | ||
|
|
||
| // Handle raw array response format | ||
| if (Array.isArray(res.data)) { | ||
| return { | ||
| success: true, | ||
| data: res.data, | ||
| message: 'Results retrieved successfully', | ||
| } as GetJudgingResultsResponse; | ||
| } | ||
|
|
||
| return (res.data || {}) as GetJudgingResultsResponse; | ||
| }; |
There was a problem hiding this comment.
Unsafe fallback (res.data || {}) as GetJudgingResultsResponse when API returns a non-array error.
If the API returns { success: false, message: "..." }, the Array.isArray check is false, and the code falls through to line 405. The cast succeeds at compile time, but at runtime the object may lack data, causing downstream code (e.g., setJudgingResults(res.data || [])) to silently produce undefined. The same pattern repeats in getSubmissionScores (line 358), getHackathonJudges (line 457), and getJudgingWinners (line 480).
Consider validating the response shape or at minimum defaulting data:
- return (res.data || {}) as GetJudgingResultsResponse;
+ const response = res.data as ApiResponse<JudgingResult[]>;
+ return {
+ success: response?.success ?? false,
+ data: response?.data ?? [],
+ message: response?.message ?? '',
+ } as GetJudgingResultsResponse;🤖 Prompt for AI Agents
In `@lib/api/hackathons/judging.ts` around lines 388 - 406, The non-array API
response in getJudgingResults is being cast unsafely to
GetJudgingResultsResponse, which can leave data undefined at runtime; update
getJudgingResults to validate the response shape and return a safe default
(e.g., { success: false, message: res.data?.message || 'Error', data: [] } or {
success: true, data: [], message: 'Results retrieved successfully' } when
appropriate) instead of using (res.data || {}) as GetJudgingResultsResponse;
apply the same defensive fix to getSubmissionScores, getHackathonJudges, and
getJudgingWinners by checking whether res.data is the expected object with a
.data field (or an array where applicable) and supplying an explicit data
fallback so callers never receive undefined.
| query: { | ||
| organizationId: orgId, | ||
| limit: 100, // Should be enough to find the owner | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Magic limit: 100 may silently miss the current user in large orgs.
If an organization has more than 100 members, listMembers may not return the current user in the first page, causing the demotion step to silently skip. Consider paginating or using a dedicated endpoint to look up the current user's membership directly, if one is available.
🤖 Prompt for AI Agents
In `@lib/providers/OrganizationProvider.tsx` around lines 1290 - 1294, The call to
listMembers with a hardcoded query { organizationId: orgId, limit: 100 } (used
around the demotion step in OrganizationProvider.tsx) can miss the current user
for orgs >100 members; replace this by either paginating through listMembers
until you find the current user or, preferably, call a dedicated membership
lookup (e.g., getMembership or listMemberById) if available to fetch the current
user's membership directly; update the logic that checks for the current user's
role (the demotion branch that relies on the listMembers result) to use the
found membership from pagination or the direct lookup and handle the case where
membership is not found by retrying/paging or throwing a clear error.
* Judging dashboard (#382) * fix: modify api.ts * fix: remove google auth buttom * fix: fixes responsive fixes on organization * fix: minor fixes * fix: modify create organization * fix: modify create organization * fix: fix organization permission * fix: merge into main * feat: hackathon overview page * feat: hackathon overview page * feat: implement participant overview * feat: implement participant overview * feat: implement resources tab * feat: implement the submission tab * feat: implement comment tab * fix: implement provider for hackathon * fix: implement provider for hackathon * fix: minor fixes * fix: hackathon banner * fix: hackathon banner * fix: fix organization page * fix: fix organization page * fix: use transform * fix: add tagline * fix: add tagline * fix: minor fixes * fix: minor fixes * fix: fix timeline and prizes * fix: correct timeline events * fix: implement registration deadline policy * fix: implement registration deadline policy * feat: implement leave hackathon * feat: implement leave hackathon * fix: delete hackathon * fix: implement invite participants * fix: implement participant profile viewing * feat: fetch participants team * fix: redesign hackathon banner * fix: fix hackthon card * fix: fix search bar in blog page * fix: fix search bar in blog page * fix: fix search bar in blog page * fix: fix error in fetching team posts * feat: implement create team, get my team * feat: implement create team, get my team * feat: implement hackathon project submission flow * feat: implement voting for submission * fix: team formation updates * fix: implement team invitation * feat: hackathon submissions bulk actions, ranking and ui refinements * fix: implement empty state for hackathons page * feat: Implement submission visibility and explore submissions * feat: Implement winners tab for hackathon winners display * feat: implement hackathon analytics * fix: fix coderabbit corrections * fix: fix coderabbit corrections * fix: fix coderabbit corrections * fix: fix organization settings data persistence, hackathondrafts and population * feat: fix user profile * feat: Implemented the participant-facing and hackathon organizers announcement features * feat: Implement judging dashboard and detailed breakdowns * feat: Implement judging dashboard and detailed breakdowns * fix: fix coderabbit corrections * Refactor/updated the timeline tab (#383) * feat: enhance hackathon timeline management and publishing flow - Updated HackathonPublishedModal to utilize new publish response structure. - Refactored TimelineSection to display additional timeline details including judging start/end and winner announcement dates. - Introduced DateTimeInput component for better date and time selection. - Added FieldLabel component for improved form labeling with tooltips. - Created timelineConstants for standardized tooltips and timezone options. - Modified timelineSchema to include new fields for judging start, end, and winner announcement dates with validation. - Enhanced TimelineSettingsTab to support new timeline fields and improved user experience with time inputs. - Updated use-hackathon-publish hook to handle new publish response format and manage state accordingly. - Adjusted use-hackathons hook to ensure proper handling of publish responses. - Updated API interfaces and utility functions to accommodate new timeline structure. - Improved validation logic for hackathon steps to ensure all necessary dates are set correctly. - Enhanced timeline calculation logic to reflect new judging and announcement phases. * chore: update markdown-it to version 14.1.1 and remove framer-motion dependency * refactor: clean up ReviewTab and HackathonPublishedModal components, improve DateTimeInput logic, and update timeline constants * refactor: update timeline calculation logic to ensure phases are only created with non-zero duration and add endDate to HackathonTimeline interface * Judging dashboard (#384) * fix: modify api.ts * fix: remove google auth buttom * fix: fixes responsive fixes on organization * fix: minor fixes * fix: modify create organization * fix: modify create organization * fix: fix organization permission * fix: merge into main * feat: hackathon overview page * feat: hackathon overview page * feat: implement participant overview * feat: implement participant overview * feat: implement resources tab * feat: implement the submission tab * feat: implement comment tab * fix: implement provider for hackathon * fix: implement provider for hackathon * fix: minor fixes * fix: hackathon banner * fix: hackathon banner * fix: fix organization page * fix: fix organization page * fix: use transform * fix: add tagline * fix: add tagline * fix: minor fixes * fix: minor fixes * fix: fix timeline and prizes * fix: correct timeline events * fix: implement registration deadline policy * fix: implement registration deadline policy * feat: implement leave hackathon * feat: implement leave hackathon * fix: delete hackathon * fix: implement invite participants * fix: implement participant profile viewing * feat: fetch participants team * fix: redesign hackathon banner * fix: fix hackthon card * fix: fix search bar in blog page * fix: fix search bar in blog page * fix: fix search bar in blog page * fix: fix error in fetching team posts * feat: implement create team, get my team * feat: implement create team, get my team * feat: implement hackathon project submission flow * feat: implement voting for submission * fix: team formation updates * fix: implement team invitation * feat: hackathon submissions bulk actions, ranking and ui refinements * fix: implement empty state for hackathons page * feat: Implement submission visibility and explore submissions * feat: Implement winners tab for hackathon winners display * feat: implement hackathon analytics * fix: fix coderabbit corrections * fix: fix coderabbit corrections * fix: fix coderabbit corrections * fix: fix organization settings data persistence, hackathondrafts and population * feat: fix user profile * feat: Implemented the participant-facing and hackathon organizers announcement features * feat: Implement judging dashboard and detailed breakdowns * feat: Implement judging dashboard and detailed breakdowns * fix: fix coderabbit corrections * fix: submission management ux refinements * fix: submission management ux refinements * fix: submission management ux refinements * fix: submission management ux refinements * fix: submission management ux refinements * feat: implement organizer override functionality for judging scores a… (#385) * feat: implement organizer override functionality for judging scores and enhance judging UI * fix: update response handling in score submission and enhance submissions list styling * fix: update eslint dependencies to latest versions for improved linting * fix: update ajv and json-schema-traverse dependencies in package-lock.json * fix: update * fix: update security audit level to high in pre-push checks * fix: update eslint dependencies and ensure security audit fails on error --------- Co-authored-by: Collins Ikechukwu <collinschristroa@gmail.com>
* Judging dashboard (#382) * fix: modify api.ts * fix: remove google auth buttom * fix: fixes responsive fixes on organization * fix: minor fixes * fix: modify create organization * fix: modify create organization * fix: fix organization permission * fix: merge into main * feat: hackathon overview page * feat: hackathon overview page * feat: implement participant overview * feat: implement participant overview * feat: implement resources tab * feat: implement the submission tab * feat: implement comment tab * fix: implement provider for hackathon * fix: implement provider for hackathon * fix: minor fixes * fix: hackathon banner * fix: hackathon banner * fix: fix organization page * fix: fix organization page * fix: use transform * fix: add tagline * fix: add tagline * fix: minor fixes * fix: minor fixes * fix: fix timeline and prizes * fix: correct timeline events * fix: implement registration deadline policy * fix: implement registration deadline policy * feat: implement leave hackathon * feat: implement leave hackathon * fix: delete hackathon * fix: implement invite participants * fix: implement participant profile viewing * feat: fetch participants team * fix: redesign hackathon banner * fix: fix hackthon card * fix: fix search bar in blog page * fix: fix search bar in blog page * fix: fix search bar in blog page * fix: fix error in fetching team posts * feat: implement create team, get my team * feat: implement create team, get my team * feat: implement hackathon project submission flow * feat: implement voting for submission * fix: team formation updates * fix: implement team invitation * feat: hackathon submissions bulk actions, ranking and ui refinements * fix: implement empty state for hackathons page * feat: Implement submission visibility and explore submissions * feat: Implement winners tab for hackathon winners display * feat: implement hackathon analytics * fix: fix coderabbit corrections * fix: fix coderabbit corrections * fix: fix coderabbit corrections * fix: fix organization settings data persistence, hackathondrafts and population * feat: fix user profile * feat: Implemented the participant-facing and hackathon organizers announcement features * feat: Implement judging dashboard and detailed breakdowns * feat: Implement judging dashboard and detailed breakdowns * fix: fix coderabbit corrections * Refactor/updated the timeline tab (#383) * feat: enhance hackathon timeline management and publishing flow - Updated HackathonPublishedModal to utilize new publish response structure. - Refactored TimelineSection to display additional timeline details including judging start/end and winner announcement dates. - Introduced DateTimeInput component for better date and time selection. - Added FieldLabel component for improved form labeling with tooltips. - Created timelineConstants for standardized tooltips and timezone options. - Modified timelineSchema to include new fields for judging start, end, and winner announcement dates with validation. - Enhanced TimelineSettingsTab to support new timeline fields and improved user experience with time inputs. - Updated use-hackathon-publish hook to handle new publish response format and manage state accordingly. - Adjusted use-hackathons hook to ensure proper handling of publish responses. - Updated API interfaces and utility functions to accommodate new timeline structure. - Improved validation logic for hackathon steps to ensure all necessary dates are set correctly. - Enhanced timeline calculation logic to reflect new judging and announcement phases. * chore: update markdown-it to version 14.1.1 and remove framer-motion dependency * refactor: clean up ReviewTab and HackathonPublishedModal components, improve DateTimeInput logic, and update timeline constants * refactor: update timeline calculation logic to ensure phases are only created with non-zero duration and add endDate to HackathonTimeline interface * Judging dashboard (#384) * fix: modify api.ts * fix: remove google auth buttom * fix: fixes responsive fixes on organization * fix: minor fixes * fix: modify create organization * fix: modify create organization * fix: fix organization permission * fix: merge into main * feat: hackathon overview page * feat: hackathon overview page * feat: implement participant overview * feat: implement participant overview * feat: implement resources tab * feat: implement the submission tab * feat: implement comment tab * fix: implement provider for hackathon * fix: implement provider for hackathon * fix: minor fixes * fix: hackathon banner * fix: hackathon banner * fix: fix organization page * fix: fix organization page * fix: use transform * fix: add tagline * fix: add tagline * fix: minor fixes * fix: minor fixes * fix: fix timeline and prizes * fix: correct timeline events * fix: implement registration deadline policy * fix: implement registration deadline policy * feat: implement leave hackathon * feat: implement leave hackathon * fix: delete hackathon * fix: implement invite participants * fix: implement participant profile viewing * feat: fetch participants team * fix: redesign hackathon banner * fix: fix hackthon card * fix: fix search bar in blog page * fix: fix search bar in blog page * fix: fix search bar in blog page * fix: fix error in fetching team posts * feat: implement create team, get my team * feat: implement create team, get my team * feat: implement hackathon project submission flow * feat: implement voting for submission * fix: team formation updates * fix: implement team invitation * feat: hackathon submissions bulk actions, ranking and ui refinements * fix: implement empty state for hackathons page * feat: Implement submission visibility and explore submissions * feat: Implement winners tab for hackathon winners display * feat: implement hackathon analytics * fix: fix coderabbit corrections * fix: fix coderabbit corrections * fix: fix coderabbit corrections * fix: fix organization settings data persistence, hackathondrafts and population * feat: fix user profile * feat: Implemented the participant-facing and hackathon organizers announcement features * feat: Implement judging dashboard and detailed breakdowns * feat: Implement judging dashboard and detailed breakdowns * fix: fix coderabbit corrections * fix: submission management ux refinements * fix: submission management ux refinements * fix: submission management ux refinements * fix: submission management ux refinements * fix: submission management ux refinements * feat: implement organizer override functionality for judging scores a… (#385) * feat: implement organizer override functionality for judging scores and enhance judging UI * fix: update response handling in score submission and enhance submissions list styling * fix: update eslint dependencies to latest versions for improved linting * fix: update ajv and json-schema-traverse dependencies in package-lock.json * fix: update * fix: update security audit level to high in pre-push checks * fix: update eslint dependencies and ensure security audit fails on error * refactor: Dynamically determine Stellar network and centralize wallet utility functions, including explorer URLs and API key handling. --------- Co-authored-by: Nnaji Benjamin <60315147+Benjtalkshow@users.noreply.github.com>
Judging System Refinements & Competition Finalization Workflow
I have completed the refinements to the judging system and implemented the full competition finalization workflow, including winner retrieval and results publishing.
Key Changes
Advanced Leaderboard Visualization
Automatic Sorting
JudgingResultsTablenow automatically sorts projects byaverageScorein descending order, providing an instant and accurate view of competition standings.Participation Tracking
Added a Participation column showing
judgeCount / totalJudgesfor each submission.A green success badge appears once all assigned judges have completed their evaluations.
Improved Hierarchy
Enhanced rank display and project naming for better scannability and visual clarity.
Enhanced Individual Score Breakdowns
Visual Criteria Scores
Replaced plain text scores with color-coded progress bars (0–10) for each evaluation criterion, making score interpretation more intuitive at a glance.
Expert Feedback Highlighting
The global judge comment is now prominently displayed as "Expert Feedback" at the top of each judge’s breakdown.
Scoring Discrepancy Detection
Implemented a deviation-highlighting system that flags judge scores that differ by more than 2 points from the submission’s average.
A pulse badge alerts organizers to potential Scoring Discrepancies for review.
Competition Finalization Workflow
Publish Results
Added a dedicated "Publish Results" action within the Results tab.
This triggers the
publishJudgingResultsAPI to finalize competition rankings.Permission Control
Only users who are:
AND
can publish results — ensuring only active judging participants can finalize the competition.
Winners Display
Automatically fetches and displays finalized winners using the
getJudgingWinnersAPI.Winners appear in a dedicated "Final Winners" section with a trophy icon.
Contextual UI Improvements
Auto-Refresh Behavior
After publishing, the dashboard automatically refreshes:
This ensures the UI reflects the latest competition state immediately.
Implementation Details
judging.tsgetJudgingWinnerspublishJudgingResultsJudgingResultsTable.tsxuseMemofor stable and robust sortingIndividualScoresBreakdown.tsxJudgingPage.tsxOutcome
This PR significantly improves:
The judging system is now more visual, more robust, and fully supports end-to-end competition finalization.
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Enhancements