Hackathon analytics #378
Hackathon analytics #378
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
… fix-blog Merge into main
… fix-blog Merge into main
… fix-hackathon Merge into main
… fix-hackathon merge into main
… fix-hackathon merge into main
… fix-hackathon Merge into main
|
@Benjtalkshow is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a Winners tab and WinnersTab UI, consolidates analytics into a single hook and timeline, introduces submission visibility settings and API endpoints, and extends HackathonDataProvider to fetch/expose explore submissions and winners. Changes
Sequence DiagramsequenceDiagram
participant Page as Hackathon Page
participant Provider as HackathonDataProvider
participant API as API Layer
participant UI as WinnersTab / Charts / Settings
Page->>Provider: initialize with slug/org/hackathonId
Provider->>API: getHackathon(slug)
Provider->>API: getHackathonAnalytics(orgId, hackathonId)
Provider->>API: getHackathonWinners(idOrSlug)
Provider->>API: getExploreSubmissions(hackathonId)
API-->>Provider: hackathon, analytics, winners, exploreSubmissions
Provider->>Provider: consolidate context (analytics, winners, exploreSubmissions)
Provider-->>Page: provide updated context
Page->>UI: render WinnersTab / Charts / Settings with context
UI->>API: updateSubmissionVisibility (on save) -- optional
API-->>UI: success / error
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
components/hackathons/HackathonsPage.tsx (2)
101-122:⚠️ Potential issue | 🟡 MinorError state renders an empty container — no user feedback.
The error block only contains a commented-out
EmptyState. Users seeing an error will get a blank section with no indication of what happened. Either uncomment and fix the error state or add a minimal error message.
44-54:⚠️ Potential issue | 🟡 Minor
transformHackathonForCardis in the useMemo dependency array but unused in the callback.This is misleading — the memo body never calls
transformHackathonForCard, so it shouldn't be a dependency. React will re-execute the memo unnecessarily when this reference changes.♻️ Proposed fix
const hackathonCards = React.useMemo(() => { return hackathons.map(hackathon => { return ( <HackathonCard isFullWidth={true} key={hackathon.id || hackathon.slug} {...hackathon} /> ); }); - }, [hackathons, transformHackathonForCard]); + }, [hackathons]);app/(landing)/organizations/[id]/hackathons/page.tsx (1)
153-162:⚠️ Potential issue | 🟠 MajorCategory filter always excludes published hackathons.
For non-draft items, the category string is hardcoded to
''(line 160), socategory.includes(categoryFilter.toLowerCase())will never match. Published hackathons are invisible when any category filter is selected.This should reference the hackathon's categories:
🐛 Proposed fix
filtered = filtered.filter(item => { const category = item.type === 'draft' ? (item.data as HackathonDraft).data.information?.categories ?.join(',') ?.toLowerCase() || '' - : ''; + : (item.data as Hackathon).categories + ?.join(',') + ?.toLowerCase() || ''; return category.includes(categoryFilter.toLowerCase()); });hooks/hackathon/use-submissions.ts (1)
114-116:⚠️ Potential issue | 🔴 CriticalSort lookup compares
selectedSort(a value) againstopt.label— sort will always fall back to'newest'.
selectedSortis initialized to'newest'(a value like'upvotes_high'), but thefindcompares againstopt.label(e.g.'Newest First'). The find will never match, sosortValuealways falls back to'newest'. Sorting by anything other than "newest" is broken.Proposed fix
- const sortValue = - sortOptions.find(opt => opt.label === selectedSort)?.value || 'newest'; + const sortValue = + sortOptions.find(opt => opt.value === selectedSort)?.value || 'newest';app/(landing)/hackathons/[slug]/page.tsx (1)
368-402:⚠️ Potential issue | 🔴 CriticalDuplicate rendering of
<HackathonResources />— the component renders twice when the resources tab is active.Lines 368–369 and Lines 400–402 both conditionally render
<HackathonResources />whenactiveTab === 'resources'. When resources exist, both conditions are true simultaneously, producing two instances of the resources component.Proposed fix — remove the duplicate block
{activeTab === 'resources' && - currentHackathon.resources?.length > 0 && <HackathonResources />} + currentHackathon.resources?.length > 0 && <HackathonResources />} {activeTab === 'participants' && currentHackathon.participants?.length > 0 && ( <HackathonParticipants /> )} ... {activeTab === 'winners' && ( <WinnersTab winners={winners} hackathonSlug={hackathonId} /> )} - - {activeTab === 'resources' && currentHackathon?.resources?.[0] && ( - <HackathonResources /> - )}
🤖 Fix all issues with AI agents
In `@app/`(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx:
- Around line 189-193: Replace the semantically incorrect AlertCircle used for
completed state in the JSX (the isCompleted branch) with a check/success icon;
update the import to bring in Check or CheckCircle from lucide-react (replace
AlertCircle in the import list and in the JSX), and adjust the icon's classes
(color/background) to a success style if needed so the completed state visually
matches success instead of warning. Ensure the JSX branch referencing
isCompleted now renders the new Check/CheckCircle component instead of
AlertCircle.
- Around line 144-170: The Winner Announcement timeline entry incorrectly uses
currentHackathon.registrationDeadline as its date; change it to use
currentHackathon.endDate (and adjust the status logic to base comparison against
new Date(currentHackathon.endDate)) so the fallback event aligns with the
Hackathon.endDate used elsewhere (e.g., the mock "Winners Announced" in
hackathonProvider). Update occurrences of registrationDeadline within the Winner
Announcement block to endDate and ensure the winnerDate variable and now
comparisons use the parsed endDate.
- Around line 178-180: The JSX uses key={phase.phase} which can produce
duplicate keys when multiple timeline items share the same phase; update the key
to be a stable composite (for example `${phase.phase}-${index}`) or use a unique
identifier from the item (e.g., phase.id) in the element returned by the map
that renders the timeline (the spot using phase.phase as the key).
In
`@components/organization/hackathons/settings/SubmissionVisibilitySettingsTab.tsx`:
- Around line 54-75: The useEffect fetching settings is including form.reset in
its dependency array which may be unstable; update the dependency list to avoid
repeated fetches by removing form.reset and using either [hackathonId] or
[hackathonId, form] instead so the effect only reruns when the hackathonId (or
the form object itself) changes; keep the fetchSettings async function and calls
to form.reset inside the effect unchanged (refer to useEffect, fetchSettings,
form.reset and hackathonId to locate the code).
- Around line 57-65: The code assumes getHackathon returns a wrapped { success,
data } object but it returns the Hackathon directly; update the conditional in
SubmissionVisibilitySettingsTab to check the returned hackathon object (e.g., if
(response) or if (hackathon)) instead of response.success, and call form.reset
using response.submissionVisibility and response.submissionStatusVisibility
(falling back to SubmissionVisibility.PUBLIC and SubmissionStatusVisibility.ALL)
so the form actually populates; reference getHackathon, form.reset,
SubmissionVisibility, and SubmissionStatusVisibility when making this change.
In `@hooks/hackathon/use-submissions.ts`:
- Around line 46-50: The filter on variable filtered in use-submissions.ts
inconsistently compares status strings (uses sub.status?.toLowerCase() for
'shortlisted' but exact === 'Approved' for approved); update the predicate in
the filtered.filter call to normalize sub.status (e.g., use
sub.status?.toLowerCase()) for both checks and compare against lowercased
literals (e.g., 'shortlisted' and 'approved') so all case variants like
'approved' or 'APPROVED' are matched.
In `@lib/providers/hackathonProvider.tsx`:
- Line 231: The code unsafely casts ExploreSubmissionsResponse.status to
SubmissionCardProps['status'] (status: sub.status as
SubmissionCardProps['status']) even though API values ('SUBMITTED',
'SHORTLISTED', 'DISQUALIFIED', etc.) differ from UI values ('Pending' |
'Approved' | 'Rejected'); replace the cast with an explicit mapper function
(e.g., mapSubmissionStatus(apiStatus): SubmissionCardProps['status']) and use it
where the status is assigned (the object constructing SubmissionCardProps in
hackathonProvider.tsx), handling all known API values and a safe default to
avoid runtime/UI breakage.
- Around line 298-305: The callback that calls fetchWinners (inside the
effect/useCallback where currentHackathonSlug, fetchHackathonBySlug,
fetchSubmissions, fetchExploreSubmissions are listed) is missing fetchWinners
from its dependency array, which can lead to a stale reference; update the
dependency array to include fetchWinners (or ensure fetchWinners is memoized) so
the callback re-binds when fetchWinners changes, referencing the fetchWinners
function name to locate and fix the dependency list.
🧹 Nitpick comments (14)
components/hackathons/submissions/submissionTab.tsx (3)
267-278: Defensive type handling is good, but the nested ternaries are hard to scan.Consider extracting a small helper (e.g.,
toCount(value)) to normalize bothvotesandcomments, reducing duplication and improving readability.♻️ Suggested helper
+const toCount = (value: unknown): number => + typeof value === 'number' ? value : Array.isArray(value) ? value.length : 0;Then use
upvotes={toCount(mySubmission.votes)}andcomments={toCount(mySubmission.comments)}.
306-331: Repeated unsafe cast(submission as { _id?: string })?._id— extract to a variable.The same cast appears four times (lines 311, 313, 320, 328). Extract it once at the top of the
.mapcallback to reduce noise and avoid inconsistencies.♻️ Proposed fix
.map((submission, index) => { + const submissionId = (submission as { _id?: string })?._id; const status = ... return ( <SubmissionCard key={submission._id || index} {...submission} status={status} - submissionId={(submission as { _id?: string })?._id} + submissionId={submissionId} - onViewClick={() => - handleViewSubmission((submission as { _id?: string })?._id) - } + onViewClick={() => handleViewSubmission(submissionId)} onUpvoteClick={() => { if (!isAuthenticated) { return; } - handleUpvoteSubmission( - (submission as { _id?: string })?._id - ); + handleUpvoteSubmission(submissionId); }} onCommentClick={() => { if (!isAuthenticated) { return; } - handleCommentSubmission( - (submission as { _id?: string })?._id - ); + handleCommentSubmission(submissionId); }}
136-153: Remove commented-out code block.This block is dead code. If it's needed later, it lives in version control history.
components/organization/hackathons/settings/SubmissionVisibilitySettingsTab.tsx (1)
39-42: Consider using a const arrow function with explicit return type.As per coding guidelines, "Prefer const arrow functions with explicit type annotations over function declarations."
♻️ Proposed change
-export default function SubmissionVisibilitySettingsTab({ - organizationId, - hackathonId, -}: SubmissionVisibilitySettingsTabProps) { +const SubmissionVisibilitySettingsTab: React.FC<SubmissionVisibilitySettingsTabProps> = ({ + organizationId, + hackathonId, +}) => {Then at the bottom:
+export default SubmissionVisibilitySettingsTab;As per coding guidelines, "Prefer const arrow functions with explicit type annotations over function declarations".
hooks/hackathon/use-submissions.ts (1)
9-9: Function declaration used instead of const arrow function.As per coding guidelines, "Prefer const arrow functions with explicit type annotations over function declarations" for
.tsfiles.Proposed refactor
-export function useSubmissions() { +export const useSubmissions = () => {(and close with
};at the end)app/(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx (1)
136-227: Extract the timeline IIFE into a separate component or helper.This 90-line IIFE embedded in JSX significantly hurts readability and testability. Consider extracting it into a
<AnalyticsTimeline>component or at least a memoized variable above thereturn.components/hackathons/winners/WinnersTab.tsx (3)
37-39: UsesubmissionIdfor a more robust React key.
${winner.rank}-${winner.projectName}could theoretically collide.HackathonWinnerhas asubmissionIdfield which is guaranteed unique.Proposed fix
- key={`${winner.rank}-${winner.projectName}`} + key={winner.submissionId}
85-89: Useclsxfor conditional class composition instead of template literal ternary.As per coding guidelines, "For conditional classes, prefer clsx or similar helper functions over ternary operators in JSX."
Proposed fix
+import clsx from 'clsx'; ... const CardContent = ( <div - className={`group relative h-full overflow-hidden rounded-xl border p-6 transition-all duration-300 ${getRankColor( - winner.rank - )}`} + className={clsx( + 'group relative h-full overflow-hidden rounded-xl border p-6 transition-all duration-300', + getRankColor(winner.rank) + )} >
106-108: PreferuserIdorusernameover array index for participant keys.The
participantsarray items have optionaluserIdandusername. Using one of those avoids potential issues if the list is reordered.Proposed fix
- {winner.participants.map((p, i) => ( - <div - key={i} + {winner.participants.map((p) => ( + <div + key={p.userId ?? p.username}app/(landing)/hackathons/[slug]/page.tsx (2)
26-26: Unused import:HackathonWinneris imported but never referenced in this file.The
winnersvariable is already typed through theuseHackathonData()context return type.Proposed fix
-import { HackathonWinner } from '@/lib/api/hackathons';
81-92: Remove commented-out debug/test code before merging.Lines 81–92 contain commented-out mock data and forced-enable logic for testing the Winners tab. These artifacts should be cleaned up.
Proposed fix
- // For testing: Use mock winners if real winners are empty - // const displayWinners = - // winners && winners.length > 0 ? winners : MOCK_WINNERS; - // const hasWinners = displayWinners.length > 0; const hasWinners = winners && winners.length > 0; - // For testing: Force enable winners tab - // const isWinnersTabEnabled = - // currentHackathon?.enabledTabs?.includes('winnersTab') !== false; - // const isWinnersTabEnabled = true; const isWinnersTabEnabled = currentHackathon?.enabledTabs?.includes('winnersTab') !== false;lib/api/hackathon.ts (1)
3-9:HackathonWinneris imported but not referenced in this file.Only
GetHackathonWinnersResponseis used bygetHackathonWinners. TheHackathonWinnertype can be removed from this import.Proposed fix
import { GetHackathonResponse, Hackathon, GetHackathonWinnersResponse, - HackathonWinner, GetHackathonAnalyticsResponse, } from '@/lib/api/hackathons';lib/api/hackathons.ts (2)
535-583:HackathonAnalyticsSummaryandHackathonStatisticsare identical — consolidate into one type.Both have the exact same fields (
participantsCount,submissionsCount,activeJudges,completedMilestones). The legacyHackathonStatistics(Line 578) and the newHackathonAnalyticsSummary(Line 535) should be unified. Also, Lines 574–576 contain development notes that should be removed.Proposed approach
Either alias one to the other or remove the duplicate:
-// Deprecated or legacy statistics types (keeping if still used elsewhere, otherwise replacing if identical) -// Checking usage, it seems these might be used by existing hooks. -// Given the request asks for a specific response structure, I will add the new ones. - -export interface HackathonStatistics { - participantsCount: number; - submissionsCount: number; - activeJudges: number; - completedMilestones: number; -} +/** `@deprecated` Use HackathonAnalyticsSummary instead */ +export type HackathonStatistics = HackathonAnalyticsSummary;
2737-2742:GetHackathonWinnersResponsedoes not explicitly narrowdatato non-optional, unlikeGetHackathonAnalyticsResponse.
GetHackathonAnalyticsResponse(Line 559) redeclaresdataas required, butGetHackathonWinnersResponseonly relies on the baseApiResponse<T>wheredatais optional (data?: T). Consumers must null-checkresponse.databefore accessing.winners. For consistency, consider adding an explicitdatafield.Proposed fix
export interface GetHackathonWinnersResponse extends ApiResponse<{ hackathonId: string; winners: HackathonWinner[]; }> { success: true; + data: { + hackathonId: string; + winners: HackathonWinner[]; + }; }
app/(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx
Outdated
Show resolved
Hide resolved
app/(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx
Outdated
Show resolved
Hide resolved
components/organization/hackathons/settings/SubmissionVisibilitySettingsTab.tsx
Outdated
Show resolved
Hide resolved
components/organization/hackathons/settings/SubmissionVisibilitySettingsTab.tsx
Show resolved
Hide resolved
| filtered = filtered.filter( | ||
| sub => | ||
| sub.status?.toLowerCase() === 'shortlisted' || | ||
| sub.status === 'Approved' | ||
| ); |
There was a problem hiding this comment.
Inconsistent status string comparison may miss matches.
Line 48 uses .toLowerCase() === 'shortlisted' (case-insensitive), but Line 49 uses exact === 'Approved' (case-sensitive). If the API returns 'approved' or 'APPROVED', the filter will not match. Apply .toLowerCase() consistently.
Proposed fix
filtered = filtered.filter(
sub =>
- sub.status?.toLowerCase() === 'shortlisted' ||
- sub.status === 'Approved'
+ sub.status?.toLowerCase() === 'shortlisted' ||
+ sub.status?.toLowerCase() === 'approved'
);📝 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.
| filtered = filtered.filter( | |
| sub => | |
| sub.status?.toLowerCase() === 'shortlisted' || | |
| sub.status === 'Approved' | |
| ); | |
| filtered = filtered.filter( | |
| sub => | |
| sub.status?.toLowerCase() === 'shortlisted' || | |
| sub.status?.toLowerCase() === 'approved' | |
| ); |
🤖 Prompt for AI Agents
In `@hooks/hackathon/use-submissions.ts` around lines 46 - 50, The filter on
variable filtered in use-submissions.ts inconsistently compares status strings
(uses sub.status?.toLowerCase() for 'shortlisted' but exact === 'Approved' for
approved); update the predicate in the filtered.filter call to normalize
sub.status (e.g., use sub.status?.toLowerCase()) for both checks and compare
against lowercased literals (e.g., 'shortlisted' and 'approved') so all case
variants like 'approved' or 'APPROVED' are matched.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx (1)
17-17:⚠️ Potential issue | 🟡 MinorUnused import:
HackathonTimeline.The timeline is now rendered inline, so this import is dead code.
-import { HackathonTimeline } from '@/components/organization/hackathons/details/HackathonTimeline';
🤖 Fix all issues with AI agents
In `@app/`(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx:
- Around line 182-183: The element currently sets padding-bottom both via
Tailwind `pb-6` in className and via a conditional inline style using `isLast`,
which is redundant and fails to remove padding for the last item; update the JSX
that contains `className='relative flex items-start gap-3 pb-6 sm:gap-4'` and
the `style={isLast ? {} : { paddingBottom: '1.5rem' }}` to remove the inline
style and instead apply the padding conditionally with Tailwind classes based on
`isLast` (use `isLast` to omit the `pb-6` class for the last item), keeping the
same other classes (`relative`, `flex`, `items-start`, `gap-3`, `sm:gap-4`)
referenced in this component.
🧹 Nitpick comments (5)
app/(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx (3)
142-227: Extract inline timeline IIFE into a dedicated component or helper.This ~85-line IIFE inside JSX is hard to read and test. Consider extracting it into a separate
<AnalyticsTimeline>component (or at minimum aconstabove thereturn) that receivestimelineEventsandendDateas props. This would also let you unit-test the "Winner Announcement" append logic in isolation.
208-215: Useclsxfor conditional class names.As per coding guidelines, prefer
clsx(or similar helper) over ternary operators in JSX for conditional classes.Example
- className={`text-xs sm:text-sm ${ - isCompleted - ? 'text-gray-400' - : isActive - ? 'text-white/60' - : 'text-white/40' - }`} + className={clsx( + 'text-xs sm:text-sm', + isCompleted && 'text-gray-400', + !isCompleted && isActive && 'text-white/60', + !isCompleted && !isActive && 'text-white/40', + )}
148-171: Synthetic "Winner Announcement" never getsongoingstatus.The status logic only distinguishes
completedvsupcomingbased on whethernow > winnerDate. If you want to surface anongoingstate (e.g., on announcement day), consider adding a same-day check. Otherwise, remove the commented-out musings to avoid confusion.lib/providers/hackathonProvider.tsx (2)
14-20:ExploreSubmissionsResponseis imported but never directly referenced.The type is inferred from the return type of
getExploreSubmissions, so this import is unused. Consider removing it to keep imports clean.import { Hackathon, HackathonResourceItem, getExploreSubmissions, - ExploreSubmissionsResponse, HackathonWinner, } from '@/lib/api/hackathons';
244-264: Variablesubmissionsshadows the outer state variable.Line 246 declares a local
const submissionsthat shadows thesubmissionsstate from Line 143. This can cause confusion if the callback is later modified to reference the state variable. Consider renaming the local variable.♻️ Suggested rename
- const submissions = await getExploreSubmissions(hackathonId); - const mappedSubmissions: SubmissionCardProps[] = submissions.map(sub => ({ + const rawSubmissions = await getExploreSubmissions(hackathonId); + const mappedSubmissions: SubmissionCardProps[] = rawSubmissions.map(sub => ({
app/(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/`(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx:
- Around line 32-35: Destructure the error from the useHackathonAnalytics hook
(e.g., const { analytics, loading: analyticsLoading, error: analyticsError } =
useHackathonAnalytics(organizationId, hackathonId)) and surface it in the UI by
rendering a subtle alert/fallback near the statistics section when
analyticsError is truthy; ensure the alert uses existing UI components/styles
and does not block the rest of the page, and include a short message like
"Unable to load analytics" plus the error.message for debugging.
🧹 Nitpick comments (4)
app/(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx (4)
21-21: Preferconstarrow function with explicit type annotation per coding guidelines.As per coding guidelines, "Prefer const arrow functions with explicit type annotations over function declarations."
Proposed fix
-export default function HackathonPage() { +const HackathonPage = (): React.JSX.Element => {And at the bottom of the file:
export default HackathonPage;
142-226: Extract timeline-building logic out of JSX.The IIFE spanning ~85 lines makes the render tree hard to follow. Extract the
fullTimelineconstruction into auseMemoabove the return, then.map()it inline. This also avoids re-computing the timeline on every render when inputs haven't changed.Proposed approach
+ const fullTimeline = useMemo(() => { + const events = analytics?.timeline || []; + const hasWinnerAnnouncement = events.some( + e => e.phase === 'Winner Announcement' + ); + if (!hasWinnerAnnouncement && currentHackathon?.endDate) { + const winnerDate = new Date(currentHackathon.endDate); + const status: 'completed' | 'ongoing' | 'upcoming' = + new Date() > winnerDate ? 'completed' : 'upcoming'; + return [ + ...events, + { + phase: 'Winner Announcement', + description: 'Final results published and prizes distributed to winners.', + date: currentHackathon.endDate, + status, + }, + ]; + } + return events; + }, [analytics?.timeline, currentHackathon?.endDate]);Then in JSX simply:
{fullTimeline.map((phase, index) => { // ...render each item })}
153-163: Remove leftover development comments.Lines 153-163 contain stream-of-consciousness TODO/design notes (
"If date is passed, completed...","keeping simple for now."). These add noise — either convert them into a brief rationale or remove them before merging.
207-214: Useclsxfor conditional class strings.Per coding guidelines, prefer
clsx(or similar helper) over ternary operators in JSX for conditional classes.Proposed fix
- className={`text-xs sm:text-sm ${ - isCompleted - ? 'text-gray-400' - : isActive - ? 'text-white/60' - : 'text-white/40' - }`} + className={clsx( + 'text-xs sm:text-sm', + isCompleted && 'text-gray-400', + !isCompleted && isActive && 'text-white/60', + !isCompleted && !isActive && 'text-white/40', + )}
| const { analytics, loading: analyticsLoading } = useHackathonAnalytics( | ||
| organizationId, | ||
| hackathonId | ||
| ); |
There was a problem hiding this comment.
Analytics error is silently discarded.
The useHackathonAnalytics hook returns an error field, but it's not destructured or surfaced to the user. If the analytics API fails, the page will silently show empty data with no feedback.
Proposed fix
- const { analytics, loading: analyticsLoading } = useHackathonAnalytics(
+ const { analytics, loading: analyticsLoading, error: analyticsError } = useHackathonAnalytics(
organizationId,
hackathonId
);Then render a subtle alert or fallback when analyticsError is truthy, e.g. near the statistics section.
📝 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 { analytics, loading: analyticsLoading } = useHackathonAnalytics( | |
| organizationId, | |
| hackathonId | |
| ); | |
| const { analytics, loading: analyticsLoading, error: analyticsError } = useHackathonAnalytics( | |
| organizationId, | |
| hackathonId | |
| ); |
🤖 Prompt for AI Agents
In `@app/`(landing)/organizations/[id]/hackathons/[hackathonId]/page.tsx around
lines 32 - 35, Destructure the error from the useHackathonAnalytics hook (e.g.,
const { analytics, loading: analyticsLoading, error: analyticsError } =
useHackathonAnalytics(organizationId, hackathonId)) and surface it in the UI by
rendering a subtle alert/fallback near the statistics section when
analyticsError is truthy; ensure the alert uses existing UI components/styles
and does not block the rest of the page, and include a short message like
"Unable to load analytics" plus the error.message for debugging.
1 participant
Summary by CodeRabbit
New Features
Improvements