Aashna/challenge schedule map qr routes #361
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request updates several aspects of the application. The changes include a renaming in the navigation icon mapping and the introduction of new route components with asynchronous loader functions. Database collections for challenges and events have been revised or introduced with updated access controls and seeding functions. Additionally, UI components such as the accordion, glassmorphic card, button, and stat icon have been enhanced with new properties and behavior. New dependencies have been added along with migration scripts to update the database schema. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant Loader as Challenges Loader
participant API as Challenges API
participant Page as ChallengesPage
Browser->>Loader: Request /challenges
Loader->>API: Fetch challenge data (with pagination)
API-->>Loader: Return paginated data
Loader-->>Browser: Return challenges data
Browser->>Page: Render ChallengesPage with data
sequenceDiagram
participant Browser as Browser
participant Loader as QR Loader
participant API as Users API
participant Page as QrPage
Browser->>Loader: Request /qr
Loader->>API: Fetch user details (/api/users/me) with cookies
alt Authenticated and terms accepted
API-->>Loader: Return user data
Loader-->>Browser: Return user details/URL info
Browser->>Page: Render QrPage with user information
else Not authenticated or terms not agreed
API-->>Loader: Error/redirect trigger
Loader-->>Browser: Redirect to home page
end
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
Progress: resolved 0, reused 0, downloaded 1, added 0 This error happened while installing a direct dependency of /tmp/eslint 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (7)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 32
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (18)
apps/axiom/src/components/Nav/navIconMap.ts(1 hunks)apps/portal/app/routes/challenges.tsx(1 hunks)apps/portal/app/routes/map.tsx(1 hunks)apps/portal/app/routes/qr.tsx(1 hunks)apps/portal/app/routes/schedule.tsx(1 hunks)libs/cms/configs/server.ts(2 hunks)libs/db/collections/models/Challenge.ts(0 hunks)libs/db/collections/models/Challenges.ts(1 hunks)libs/db/collections/models/Events.ts(3 hunks)libs/db/collections/models/index.ts(1 hunks)libs/db/seed/index.ts(3 hunks)libs/portal/pages/challenges.tsx(1 hunks)libs/portal/pages/map.tsx(1 hunks)libs/portal/pages/qr.tsx(1 hunks)libs/portal/pages/schedule.tsx(1 hunks)libs/shared/ui/accordion/accordion.tsx(2 hunks)libs/shared/ui/glassmorphic-card/glassmorphic-card.tsx(3 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- libs/db/collections/models/Challenge.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/portal/app/routes/map.tsx
[error] 23-23: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
libs/shared/ui/glassmorphic-card/glassmorphic-card.tsx
[error] 56-56: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 57-57: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 58-58: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
libs/portal/pages/challenges.tsx
[error] 61-70: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (18)
package.json (1)
104-104: New dependency added correctly.The addition of
react-inlinesvgseems appropriate for handling SVG content inline within React components.libs/db/collections/models/index.ts (1)
1-1:✅ Verification successful
Export updated to reflect model name change
The change from singular 'Challenge' to plural 'Challenges' aligns with the collection name changes observed in other files.
🏁 Script executed:
#!/bin/bash # Verify that all references to the model have been updated consistently across the codebase rg -i 'import.*from.*Challenge' --glob="*.{ts,tsx,js,jsx}" -A 1 -B 1Length of output: 336
Model naming is consistent across the codebase.
The export change from'Challenge'to'Challenges'is confirmed by the grep results, as references inapps/portal/app/routes/challenges.tsxnow correctly import from the updated module. No further changes are needed.libs/db/seed/index.ts (3)
12-12: Import for new seed function added correctlyThe import for
seedChallengesis properly added to support the new challenges collection seeding.
38-38: Collection name added to deletion listThe "challenges" collection is correctly added to the list of collections that get cleared before seeding.
80-80: Seed function integrated into the workflowThe
seedChallengesfunction is properly integrated into the seeding process, matching the pattern used for other collections.apps/portal/app/routes/qr.tsx (1)
8-8: API endpoint mismatch with route purposeThe loader is fetching from
/api/challengesbut this is a QR route. Is this intentional, or should it be fetching QR-specific data instead?libs/cms/configs/server.ts (2)
11-12: Approve change from ChallengePrize to ChallengesThe renaming from ChallengePrize to Challenges is consistent with the changes in other parts of the application. This provides better semantic clarity about the entity's purpose.
44-44: Approve collection configuration updateThe update to include Challenges in the collections array correctly reflects the import change above, ensuring proper registration of the Challenges collection in the CMS.
apps/portal/app/routes/schedule.tsx (1)
8-8: API endpoint mismatch with route purposeThe loader is fetching from
/api/challengesbut this is a Schedule route. Is this the correct endpoint, or should it be fetching schedule-specific data?apps/portal/app/routes/challenges.tsx (2)
6-23: Good error handling with fallback dataThe error handling in this loader is properly implemented, returning a default structure when an error occurs. This ensures the component can render even when data fetching fails.
25-31: Correct implementation of loader data usageThe component correctly uses useLoaderData to access the data from the loader and passes it to the ChallengesPage component.
libs/shared/ui/accordion/accordion.tsx (1)
30-32: Consider centralizing chevron rendering logic.
Currently, each side has nearly identical code. You can either keep the split for clarity or unify it to reduce duplication. Otherwise, this looks fine.libs/shared/ui/glassmorphic-card/glassmorphic-card.tsx (3)
5-5: Import usage verified.
The newTypographyimport is put to good use in the header.
15-18: New optional props look valid.
IntroducingpathTitle,minimize,maximize, andclosefor improved flexibility is good practice.
43-46: Neat handling of new properties.
The function signature properly reflects the expanded props, ensuring maintainability.libs/db/collections/models/Challenges.ts (1)
431-459: Verify consistent block type usage.
Some entries use"infos"while others use"info". Confirm if mixing these block types is intended or a typo.Would you like me to search the codebase for other references to
"infos"vs."info"?libs/db/collections/models/Events.ts (2)
3-3: Good addition of typed importsAdding typed imports from 'payload' is a good practice for TypeScript type safety.
65-70: LGTM: Event type additionsThe two new event types ('Hackathon' and 'Fun') are valuable categorization options for the events collection.
| organizerTeams: Signature, | ||
| groups: PersonStanding, | ||
| challengePrize: Trophy, | ||
| challenges: Trophy, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Key rename from challengePrize to challenges.
This change aligns with the broader terminology standardization across the PR. Ensure all references to this key have been updated accordingly.
🏁 Script executed:
#!/bin/bash
# Find any remaining references to challengePrize that may need updating
rg "challengePrize" --type ts --type tsxLength of output: 68
Action: Verify Key Renaming Across the Codebase
The key in apps/axiom/src/components/Nav/navIconMap.ts has been renamed to challenges: Trophy, to align with our broader terminology standardization. However, the original verification command encountered an issue with tsx file type recognition. Please re-run the above command manually and confirm that no remaining references to challengePrize exist.
🏁 Script executed:
#!/bin/bash
# Re-run grep with proper file globs to search both .ts and .tsx files
rg "challengePrize" -g "*.ts" -g "*.tsx"Length of output: 530
Attention: Incomplete Key Renaming for "challengePrize"
The change in apps/axiom/src/components/Nav/navIconMap.ts correctly updates the key to challenges: Trophy, aligning with our standardized terminology. However, the search revealed lingering references to challengePrize in libs/db/payload-types.ts, including:
challengePrize: ChallengePrize;challengePrize: ChallengePrizeSelect<false> | ChallengePrizeSelect<true>;- Comment references and relation definitions referencing
challengePrize.
Please review whether these instances should also be updated for consistency across the application. If they need renaming, ensure the changes are applied to the database payload typings as well. If these references are intentionally maintained for backward compatibility or other reasons, kindly add documentation explaining the rationale.
| import { Layout } from '@cuhacking/portal/ui/layout' | ||
|
|
||
| export function QrPage() { | ||
| return ( | ||
| <Layout isCompleteProfile={false}> | ||
| <section className="max-w-screen-xl mx-auto grid sm:grid-cols-2 gap-5 p-5 sm:px-10 py-40 pt-20"> | ||
| <div> | ||
| QR code page | ||
| </div> | ||
| </section> | ||
| </Layout> | ||
| ) | ||
| } |
There was a problem hiding this comment.
QR page implementation lacks data fetching.
The component structure looks good, but similar to the schedule page, this implementation is missing the loader function mentioned in the summary to fetch data from an API endpoint.
Consider adding a loader function for QR data:
import { Layout } from '@cuhacking/portal/ui/layout'
+import { json, type LoaderFunction } from '@remix-run/node'
+export const loader: LoaderFunction = async () => {
+ const apiUrl = process.env.NODE_ENV === 'production'
+ ? 'https://api.cuhacking.com/qr'
+ : 'http://localhost:3000/api/qr'
+
+ const response = await fetch(apiUrl)
+ const data = await response.json()
+
+ return json({ qrCodes: data.docs })
+}
export function QrPage() {
return (
<Layout isCompleteProfile={false}>
<section className="max-w-screen-xl mx-auto grid sm:grid-cols-2 gap-5 p-5 sm:px-10 py-40 pt-20">
<div>
QR code page
</div>
</section>
</Layout>
)
}📝 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.
| import { Layout } from '@cuhacking/portal/ui/layout' | |
| export function QrPage() { | |
| return ( | |
| <Layout isCompleteProfile={false}> | |
| <section className="max-w-screen-xl mx-auto grid sm:grid-cols-2 gap-5 p-5 sm:px-10 py-40 pt-20"> | |
| <div> | |
| QR code page | |
| </div> | |
| </section> | |
| </Layout> | |
| ) | |
| } | |
| import { Layout } from '@cuhacking/portal/ui/layout' | |
| import { json, type LoaderFunction } from '@remix-run/node' | |
| export const loader: LoaderFunction = async () => { | |
| const apiUrl = process.env.NODE_ENV === 'production' | |
| ? 'https://api.cuhacking.com/qr' | |
| : 'http://localhost:3000/api/qr' | |
| const response = await fetch(apiUrl) | |
| const data = await response.json() | |
| return json({ qrCodes: data.docs }) | |
| } | |
| export function QrPage() { | |
| return ( | |
| <Layout isCompleteProfile={false}> | |
| <section className="max-w-screen-xl mx-auto grid sm:grid-cols-2 gap-5 p-5 sm:px-10 py-40 pt-20"> | |
| <div> | |
| QR code page | |
| </div> | |
| </section> | |
| </Layout> | |
| ) | |
| } |
| </Button> | ||
| </a> | ||
| ), | ||
| )} |
There was a problem hiding this comment.
Fix trailing comma in JSX syntax
Similar to the earlier issue, there's an incorrect trailing comma inside parentheses.
- {blockData.buttons.map(btn => (
- <a href={btn.link} key={btn.id}>
- <Button variant="secondary">
- {btn.title}
- </Button>
- </a>
- ),
- )}
+ {blockData.buttons.map(btn => (
+ <a href={btn.link} key={btn.id}>
+ <Button variant="secondary">
+ {btn.title}
+ </Button>
+ </a>
+ ))}📝 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.
| )} | |
| {blockData.buttons.map(btn => ( | |
| <a href={btn.link} key={btn.id}> | |
| <Button variant="secondary"> | |
| {btn.title} | |
| </Button> | |
| </a> | |
| ))} |
Aashna-Verma
force-pushed
the
aashna/challenge-schedule-map-qr-routes
branch
from
867d886 to
e1cfa59
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (4)
libs/portal/pages/qr.tsx (1)
1-13: 🛠️ Refactor suggestionQR page lacks implementation and data fetching
The component only contains placeholder text without actual QR code rendering or data fetching logic. Consider implementing:
- Add a loader function to fetch QR data
- Implement the UI to display QR codes
- Add proper error handling
import { Layout } from '@cuhacking/portal/ui/layout' +import { json, type LoaderFunction, useLoaderData } from '@remix-run/node' +import { useState } from 'react' +export const loader: LoaderFunction = async () => { + try { + const apiUrl = process.env.NODE_ENV === 'production' + ? 'https://api.cuhacking.com/qr' + : 'http://localhost:3000/api/qr' + + const response = await fetch(apiUrl) + if (!response.ok) { + throw new Error('Failed to fetch QR data') + } + const data = await response.json() + + return json({ qrCodes: data.docs }) + } catch (error) { + console.error('Error fetching QR data:', error) + return json({ qrCodes: [], error: 'Failed to load QR codes' }) + } +} export function QrPage() { + const { qrCodes, error } = useLoaderData<{ qrCodes: any[], error?: string }>() return ( <Layout isCompleteProfile={false}> <section className="max-w-screen-xl mx-auto gap-5 p-5 sm:px-10 py-40 pt-20"> - <div> - QR code page - </div> + <h1 className="text-2xl font-bold mb-6">QR Codes</h1> + {error && <div className="text-red-500 mb-4">{error}</div>} + <div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-6"> + {qrCodes.length > 0 ? ( + qrCodes.map((qr) => ( + <div key={qr.id} className="border rounded-lg p-4 shadow-sm"> + {/* Render your QR code here */} + <h2 className="font-medium mb-2">{qr.title}</h2> + <p className="text-sm text-gray-600 mb-3">{qr.description}</p> + {/* Add QR code image/component here */} + </div> + )) + ) : ( + <p>No QR codes available at this time.</p> + )} + </div> </section> </Layout> ) }libs/portal/pages/map.tsx (1)
1-13: 🛠️ Refactor suggestionMap page needs implementation with SVG integration
This component contains only placeholder text. Since you've added the
react-inlinesvgdependency, consider implementing actual map functionality:
- Add data fetching with a loader function
- Implement SVG map rendering using react-inlinesvg
- Add interactive elements to the map as needed
import { Layout } from '@cuhacking/portal/ui/layout' +import { json, type LoaderFunction, useLoaderData } from '@remix-run/node' +import SVG from 'react-inlinesvg' +import { useState } from 'react' +export const loader: LoaderFunction = async () => { + try { + const apiUrl = process.env.NODE_ENV === 'production' + ? 'https://api.cuhacking.com/maps' + : 'http://localhost:3000/api/maps' + + const response = await fetch(apiUrl) + if (!response.ok) { + throw new Error('Failed to fetch map data') + } + const data = await response.json() + + return json({ maps: data.docs }) + } catch (error) { + console.error('Error fetching map data:', error) + return json({ maps: [], error: 'Failed to load maps' }) + } +} export function MapsPage() { + const { maps, error } = useLoaderData<{ maps: any[], error?: string }>() + const [selectedMap, setSelectedMap] = useState(maps[0]?.id || null) + + const currentMap = maps.find(map => map.id === selectedMap) || maps[0] return ( <Layout isCompleteProfile={false}> <section className="max-w-screen-xl mx-auto grid sm:grid-cols-2 gap-5 p-5 sm:px-10 py-40 pt-20"> - <div> - Map Page - </div> + <div className="col-span-2 mb-6"> + <h1 className="text-2xl font-bold mb-2">Venue Maps</h1> + {error && <div className="text-red-500">{error}</div>} + </div> + + <div className="flex flex-col gap-3"> + {maps.length > 0 ? ( + maps.map((map) => ( + <button + key={map.id} + className={`p-3 border rounded-md text-left ${map.id === selectedMap ? 'border-blue-500 bg-blue-50' : ''}`} + onClick={() => setSelectedMap(map.id)} + > + <h3 className="font-medium">{map.title}</h3> + <p className="text-sm text-gray-600">{map.description}</p> + </button> + )) + ) : ( + <p>No maps available at this time.</p> + )} + </div> + + <div className="border rounded-lg p-4 h-[500px] bg-white"> + {currentMap ? ( + <SVG + src={currentMap.svgUrl} + className="w-full h-full" + preProcessor={(code) => code.replace(/fill=".*?"/g, 'fill="currentColor"')} + title={currentMap.title} + /> + ) : ( + <div className="flex items-center justify-center h-full"> + <p>Select a map to view</p> + </div> + )} + </div> </section> </Layout> ) }apps/portal/app/routes/qr.tsx (1)
5-21:⚠️ Potential issueMissing return value in the catch block of the loader function
The loader function needs to return a value even when an error occurs. Currently, if an error is caught, the function doesn't return anything, which would result in
undefinedbeing returned to the component.export const loader: LoaderFunction = async () => { try { const API_URL = process.env.NODE_ENV === 'development' ? 'http://localhost:8000' : 'https://axiom.cuhacking.ca' const req = await fetch(`${API_URL}/api/challenges`) if (!req.ok) { throw new Error('Error') } const data = await req.json() return data.docs } catch (error) { console.error(`Error fetching challenges`, error) + return [] // Return empty array as fallback } }libs/portal/pages/challenges.tsx (1)
61-62: 🧹 Nitpick (assertive)Use optional chaining for safer access.
- {challenge.challengeBlock - && challenge.challengeBlock.map(block => + {challenge.challengeBlock?.map(block =>🧰 Tools
🪛 Biome (1.9.4)
[error] 61-70: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (18)
apps/axiom/src/components/Nav/navIconMap.ts(1 hunks)apps/portal/app/routes/challenges.tsx(1 hunks)apps/portal/app/routes/map.tsx(1 hunks)apps/portal/app/routes/qr.tsx(1 hunks)apps/portal/app/routes/schedule.tsx(1 hunks)libs/cms/configs/server.ts(1 hunks)libs/db/collections/models/Challenge.ts(0 hunks)libs/db/collections/models/Challenges.ts(1 hunks)libs/db/collections/models/Events.ts(3 hunks)libs/db/collections/models/index.ts(1 hunks)libs/db/seed/index.ts(3 hunks)libs/portal/pages/challenges.tsx(1 hunks)libs/portal/pages/map.tsx(1 hunks)libs/portal/pages/qr.tsx(1 hunks)libs/portal/pages/schedule.tsx(1 hunks)libs/shared/ui/accordion/accordion.tsx(2 hunks)libs/shared/ui/glassmorphic-card/glassmorphic-card.tsx(3 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- libs/db/collections/models/Challenge.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/portal/app/routes/map.tsx
[error] 23-23: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
libs/portal/pages/schedule.tsx
[error] 37-38: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
libs/shared/ui/glassmorphic-card/glassmorphic-card.tsx
[error] 56-56: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 57-57: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 58-58: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
libs/portal/pages/challenges.tsx
[error] 61-70: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (27)
package.json (1)
104-104: Added dependency for SVG handlingThe addition of
react-inlinesvgis appropriate for handling SVG content in React components. This will likely be used for map or QR code rendering in the new routes being added.apps/axiom/src/components/Nav/navIconMap.ts (1)
31-31: Key rename fromchallengePrizetochallengesThe key has been renamed to align with terminology standardization across the application. However, there may still be references to
challengePrizein other files likelibs/db/payload-types.tsthat need updating.#!/bin/bash # Check if there are any remaining references to challengePrize that need updating rg "challengePrize" -g "*.ts" -g "*.tsx"libs/db/collections/models/index.ts (1)
1-1: Export updated to use plural formThe change from
ChallengetoChallengesreflects the shift to a more normalized schema structure. This aligns with the collection name updates across the application.libs/db/seed/index.ts (3)
12-12: Added import for seedChallenges functionAppropriate import for the new challenges seeding function.
38-38: Added challenges to collections listGood addition of "challenges" to the collections array for database cleanup before seeding.
80-80: Added seedChallenges invocationCorrectly implemented call to the new challenges seeding function.
libs/portal/pages/schedule.tsx (2)
9-18: Event category options definedGood set of event categories with emoji prefixes for visual distinction.
20-43: Schedule page layout structure needs data integrationThe page layout is well-structured, but it's not connected to any data source. According to the PR context, there should be loader functionality to fetch schedule data.
#!/bin/bash # Check if there's a loader implementation for schedule data rg -A 5 "export const loader.*schedule" --type typescript🧰 Tools
🪛 Biome (1.9.4)
[error] 37-38: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
apps/portal/app/routes/qr.tsx (2)
23-29: Uncomment data usage or remove loader if data isn't neededThe loader is fetching data, but the component isn't using it (useLoaderData is commented out). Either uncomment the data usage or consider if the loader is necessary.
export default function QR() { - // const data = useLoaderData<typeof loader>() + const data = useLoaderData<typeof loader>() return ( - <QrPage /> + <QrPage data={data} /> ) }
7-7: Consider using environment variables for API URLsHardcoding URLs can make environment management difficult. Consider using environment variables for both development and production URLs.
-const API_URL = process.env.NODE_ENV === 'development' ? 'http://localhost:8000' : 'https://axiom.cuhacking.ca' +const API_URL = process.env.NODE_ENV === 'development' + ? process.env.API_LOCAL_URL || 'http://localhost:8000' + : process.env.API_PRODUCTION_URL || 'https://axiom.cuhacking.ca'apps/portal/app/routes/map.tsx (3)
5-21: Loader function needs error handling improvementsThe current error handling approach logs errors but doesn't return a proper response, which could cause runtime issues.
export const loader: LoaderFunction = async () => { try { const API_URL = process.env.NODE_ENV === 'development' ? 'http://localhost:8000' : 'https://axiom.cuhacking.ca' const req = await fetch(`${API_URL}/api/challenges`) if (!req.ok) { - throw new Error('Error') + throw new Error(`Failed to fetch challenges: ${req.status} ${req.statusText}`) } const data = await req.json() return data.docs } catch (error) { console.error(`Error fetching challenges`, error) + return [] // Return empty array as fallback } }
23-29: Rename component to avoid shadowing global Map objectThe component name
Mapshadows the global JavaScriptMapobject, which could lead to confusion.-export default function Map() { +export default function MapRoute() { // const data = useLoaderData<typeof loader>() return ( <MapsPage /> ) }🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
8-8:❓ Verification inconclusive
Ensure API endpoint matches the route purpose
This map route is fetching challenge data, which seems misaligned. Consider either updating the endpoint to fetch map-specific data or confirming that challenge data is intentionally used for the map functionality.
🏁 Script executed:
#!/bin/bash # Check if there's a map-specific API endpoint in the backend echo "Checking for map-specific API endpoints..." rg --files | grep -i "api.*map"Length of output: 128
Attention: Verify API Endpoint Consistency for Map Functionality
The map route in
apps/portal/app/routes/map.tsx(line 8) is currently fetching data from the challenges endpoint (${API_URL}/api/challenges). Our search for any dedicated map-specific endpoints in the backend returned no matches. Please verify if:
- The use of challenge data in the map view is intentional.
- A map-specific API endpoint should be used instead.
apps/portal/app/routes/schedule.tsx (1)
23-29: Uncomment data usage or remove loader if data isn't neededThe loader is fetching data, but the component isn't using it (useLoaderData is commented out). Either uncomment the data usage or consider if the loader is necessary.
export default function Schedule() { - // const data = useLoaderData<typeof loader>() + const data = useLoaderData<typeof loader>() return ( - <SchedulePage /> + <SchedulePage data={data} /> ) }libs/shared/ui/accordion/accordion.tsx (1)
16-19: Add JSDoc to document the newnoChevronpropThis newly introduced boolean prop affects how chevrons are rendered. Consider adding a brief JSDoc or type definition for clarity.
const AccordionTrigger = React.forwardRef< React.ElementRef<typeof AccordionPrimitive.Trigger>, - React.ComponentPropsWithoutRef<typeof AccordionPrimitive.Trigger> & { isLeft?: boolean } + React.ComponentPropsWithoutRef<typeof AccordionPrimitive.Trigger> & { + /** Position the chevron on the left side of the content */ + isLeft?: boolean; + /** Hide the chevron completely */ + noChevron?: boolean; + } >(({ className, children, isLeft, noChevron, ...props }, ref) => (apps/portal/app/routes/challenges.tsx (2)
8-9: Consider using environment variables for API URLs
This suggestion was previously mentioned. Using environment variables would align with other route files and improve configurability.
11-12: Use a more descriptive error message
This was flagged in a past review. Throwing'Error'is not sufficiently descriptive for debugging.libs/shared/ui/glassmorphic-card/glassmorphic-card.tsx (1)
56-58: Convert empty<div>elements to self-closing
This issue was previously flagged. It keeps the JSX cleaner and is recommended by static analysis.🧰 Tools
🪛 Biome (1.9.4)
[error] 56-56: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 57-57: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 58-58: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
libs/db/collections/models/Challenges.ts (2)
61-646: Refine bulk creation error handling and transactions
This was mentioned previously, suggesting a single transaction approach to improve consistency on partial failures.
433-439:❓ Verification inconclusive
Verify block type naming
Inconsistent usage of"infos"vs."info". Confirm if this is intentional, as it may cause mismatch in the UI or data processing.
🏁 Script executed:
#!/bin/bash # Searching for 'blockType: "infos"' usage in the repo rg --context 3 'blockType:\s*"infos"'Length of output: 1010
Inconsistent Block Type Naming – Confirm Intentionality
It appears that the
blockTypevalue is consistently set as"infos"for blocks like"Auth0 Challenge infos"and"Cloudflare AI infos", while one block (e.g.,"Auth0 Challenge Prizes") uses"info". Please confirm whether this distinction is intentional or if a consistent naming convention should be applied to avoid potential issues in the UI or during data processing.libs/db/collections/models/Events.ts (6)
3-3: No concerns on the import statement.
65-70: Thanks for adding more event types.Also applies to: 74-74
224-224: Remove backticks in description text.
232-233: Description appears incomplete.
460-462: Event end time is before start time.
477-477: Remove the question mark in location for clarity.libs/portal/pages/challenges.tsx (1)
89-92: Remove trailing commas from map callbacks.Also applies to: 104-111
| import type { LoaderFunction } from '@remix-run/node' | ||
| import process from 'node:process' | ||
| import { SchedulePage } from '@cuhacking/portal/pages/schedule' | ||
|
|
||
| export const loader: LoaderFunction = async () => { | ||
| try { | ||
| const API_URL = process.env.NODE_ENV === 'development' ? 'http://localhost:8000' : 'https://axiom.cuhacking.ca' | ||
| const req = await fetch(`${API_URL}/api/events`) | ||
|
|
||
| if (!req.ok) { | ||
| throw new Error('Error') | ||
| } | ||
|
|
||
| const data = await req.json() | ||
|
|
||
| return data.docs | ||
| } | ||
| catch (error) { | ||
| console.error(`Error fetching challenges`, error) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider creating a shared utility for loaders
Multiple loader functions across different route files share nearly identical code with only minor differences. Consider creating a shared utility function to reduce duplication.
Create a utility file like libs/portal/utils/dataLoaders.ts:
// libs/portal/utils/dataLoaders.ts
export async function fetchApiData<T>(endpoint: string): Promise<T[]> {
try {
const API_URL = process.env.NODE_ENV === 'development'
? process.env.API_LOCAL_URL || 'http://localhost:8000'
: process.env.API_PRODUCTION_URL || 'https://axiom.cuhacking.ca'
const req = await fetch(`${API_URL}/api/${endpoint}`)
if (!req.ok) {
throw new Error(`Error fetching ${endpoint}: ${req.status} ${req.statusText}`)
}
const data = await req.json()
return data.docs
}
catch (error) {
console.error(`Error fetching ${endpoint}`, error)
return []
}
}
// Then in your route files:
// export const loader: LoaderFunction = () => fetchApiData<ChallengeType>('challenges')
// export const loader: LoaderFunction = () => fetchApiData<EventType>('events')| { | ||
| blockName: "Cloudflare AI infos", | ||
| blockType: "infos", | ||
| title: "getPrompts()", | ||
| bullets: [] // No infos provided | ||
| }, | ||
| { | ||
| blockName: "Cloudflare AI Prizes", | ||
| blockType: "info", | ||
| title: "getPrizes()", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify block type naming
Again, "infos" is used here, which differs from the more common "info".
🏁 Script executed:
#!/bin/bash
# Searching for 'blockType: "infos"' usage in the repo again
rg --context 3 'blockType:\s*"infos"'Length of output: 1010
Action Required: Standardize Block Type Naming
The code uses "infos" in some blocks while another block uses "info". For consistency, please update the blocks in libs/db/collections/models/Challenges.ts to use a single, standardized naming convention. For example, consider changing:
- blockType: "infos",
+ blockType: "info",- Ensure that both the
"Auth0 Challenge infos"and"Cloudflare AI infos"blocks adopt the same block type. - Verify that this change does not affect any downstream logic that relies on the block type value.
📝 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.
| { | |
| blockName: "Cloudflare AI infos", | |
| blockType: "infos", | |
| title: "getPrompts()", | |
| bullets: [] // No infos provided | |
| }, | |
| { | |
| blockName: "Cloudflare AI Prizes", | |
| blockType: "info", | |
| title: "getPrizes()", | |
| { | |
| blockName: "Cloudflare AI infos", | |
| - blockType: "infos", | |
| + blockType: "info", | |
| title: "getPrompts()", | |
| bullets: [] // No infos provided | |
| }, | |
| { | |
| blockName: "Cloudflare AI Prizes", | |
| blockType: "info", | |
| title: "getPrizes()", |
libs/db/collections/models/Events.ts
Outdated
| start: new Date(2025, 2, 14, 23, 30), | ||
| end: new Date(2025, 2, 14, 2, 0), |
There was a problem hiding this comment.
Fix the inverted time range.
The event ends at 2:00 AM, which is earlier than the 11:30 PM start time on the same date. Consider setting a later end time:
- end: new Date(2025, 2, 14, 2, 0),
+ end: new Date(2025, 2, 15, 2, 0),📝 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.
| start: new Date(2025, 2, 14, 23, 30), | |
| end: new Date(2025, 2, 14, 2, 0), | |
| start: new Date(2025, 2, 14, 23, 30), | |
| end: new Date(2025, 2, 15, 2, 0), |
| location: "RB 1201", | ||
| start: new Date(2025, 2, 14, 19, 0), | ||
| end: new Date(2025, 2, 15, -4, 0), | ||
| type: ["workshop"], |
There was a problem hiding this comment.
Negative hour (-4) usage is confusing.
Confirm if you intend to schedule at 19:00 or a different time. Using negative hours may lead to unintentional day shifts:
- end: new Date(2025, 2, 15, -4, 0),
+ end: new Date(2025, 2, 15, 4, 0),📝 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.
| end: new Date(2025, 2, 15, -4, 0), | |
| end: new Date(2025, 2, 15, 4, 0), |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/portal/app/routes/profile.tsx(2 hunks)apps/portal/app/routes/qr.tsx(1 hunks)libs/cms/configs/server.ts(2 hunks)libs/db/collections/models/Users.ts(1 hunks)libs/portal/pages/qr.tsx(1 hunks)package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: build (22.13.1)
🔇 Additional comments (12)
libs/cms/configs/server.ts (2)
10-10: Import statement updated to match collection name changeThe import has been updated from
ChallengePrizetoChallenges, which aligns with the collection name change in the configuration array (line 43).
42-43: Collection reference updated from ChallengePrize to ChallengesThe collection entry has been properly updated to use
Challengesinstead ofChallengePrize, which completes the entity renaming across the system. This change aligns with the modification to the import statement on line 10.libs/db/collections/models/Users.ts (1)
270-270: Relaxed admin access in development environmentAccess control for admin has been modified to use a less restrictive
authenticatedcheck in development environments, while preserving the stricterisOrganizerOrSponsorcheck for production. This pattern facilitates easier development and testing.Make sure this environment-specific access control is used consistently across other collections that have similar restrictions. You might want to verify that NODE_ENV is properly set in all your deployment environments to prevent accidental security weaknesses.
apps/portal/app/routes/profile.tsx (2)
5-5: No issues at this line.
Standard usage ofredirectfrom@remix-run/node.
37-37: Verify the security implications of returning cookies in loader data.
Returningcookiein the loader might expose session data to the client-side. Confirm if this is intentional or whether you should omit or encrypt it.package.json (2)
99-99: Looks good.
Dependencyqrcode.reactis a well-established library for QR code generation.
105-105: Looks good.
Dependencyreact-inlinesvgis a common library for rendering inline SVGs.libs/portal/pages/qr.tsx (3)
1-2: Imports are valid.
No concerns with the references toLayoutandQRCodeCanvas.
11-17: Usage of<QRCodeCanvas>is correct.
Displays a QR code for the providedqrValue. Good approach.
18-21: No issues with the wrapping layout.
The structure is straightforward and consistent with the rest of the codebase.apps/portal/app/routes/qr.tsx (2)
1-7: Imports look fine.
No concerns with these references.
44-50: No issues with the default export.
It properly consumes the loader data and renders<QrPage>.
| const qrValue = 'https://yourwebsite.com' // Change this to your desired URL or text | ||
| return ( |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
No issue with the QR code value.
Consider using environment-based URLs if you plan to differentiate staging vs. production.
| import { QRCodeCanvas } from 'qrcode.react' | ||
|
|
||
| export function QrPage({ user }) { | ||
| /* console.log(user) */ |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove the commented console log.
The commented code drifting over time can cause confusion.
Apply this diff to remove the comment:
export function QrPage({ user }) {
- /* console.log(user) */
const qrValue = 'https://yourwebsite.com'📝 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.
| /* console.log(user) */ | |
| export function QrPage({ user }) { | |
| const qrValue = 'https://yourwebsite.com' | |
| // ... rest of the implementation | |
| } |
| export const loader: LoaderFunction = async ({ request }) => { | ||
| const cookie = request.headers.get('Cookie') | ||
|
|
||
| const baseUrl | ||
| = process.env.NODE_ENV === 'development' | ||
| ? 'http://localhost:8000' | ||
| : 'https://axiom.cuhacking.ca' | ||
|
|
||
| const API_URL = baseUrl | ||
| try { | ||
| const res = await fetch(`${API_URL}/api/users/me`, { | ||
| credentials: 'include', | ||
| headers: { Cookie: cookie || '' }, | ||
| }) | ||
|
|
||
| if (!res.ok) { | ||
| throw new Error('Not Authenticated') | ||
| } | ||
|
|
||
| const { user } = await res.json() | ||
|
|
||
| if (!user) { | ||
| return redirect('/') | ||
| } | ||
|
|
||
| if (!user.agreedToTerms) { | ||
| return redirect('/') | ||
| } | ||
|
|
||
| return { user, cookie, API_URL } | ||
| } | ||
| catch { | ||
| return redirect('/') | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider extracting loader logic to a shared utility.
This loader largely duplicates the logic found in other routes like profile.tsx. Centralizing this logic improves maintainability.
feat(axiom): events seed data
MFarabi619
force-pushed
the
aashna/challenge-schedule-map-qr-routes
branch
from
e0947f5 to
d73af6b
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
libs/db/collections/models/Events.ts (1)
50-50: 🛠️ Refactor suggestionNegative-hour default values may lead to unexpected dates.
Specifying negative hours (e.g.,-4) can cause day shifts in unexpected ways. Consider using a later date/time explicitly to avoid confusion.Also applies to: 62-62
♻️ Duplicate comments (2)
libs/portal/pages/schedule.tsx (2)
36-36: 🧹 Nitpick (assertive)Make the
<MultipleSelector>self-closing.
This follows recommended JSX style.- <MultipleSelector ...></MultipleSelector> + <MultipleSelector ... />🧰 Tools
🪛 Biome (1.9.4)
[error] 36-38: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
32-34: 🧹 Nitpick (assertive)Use distinct dates if needed.
All three buttons show “Fri Mar 14th,” potentially confusing users if they span multiple days.- <Button className="hover:bg-red-500 cursor-pointer">Fri Mar 14th</Button> - <Button className="cursor-pointer">Fri Mar 14th</Button> - <Button className="cursor-pointer">Fri Mar 14th</Button> + <Button className="hover:bg-red-500 cursor-pointer">Fri Mar 14th</Button> + <Button className="cursor-pointer">Sat Mar 15th</Button> + <Button className="cursor-pointer">Sun Mar 16th</Button>
🛑 Comments failed to post (4)
libs/portal/pages/dialog.tsx (2)
43-47: 🧹 Nitpick (assertive)
Consider extracting close button to a separate component.
The close button styling and implementation is embedded directly within DialogContent. For better maintainability and reusability, consider extracting it to a separate component.
const DialogCloseButton = React.forwardRef< React.ElementRef<typeof DialogPrimitive.Close>, React.ComponentPropsWithoutRef<typeof DialogPrimitive.Close> >(({ className, ...props }, ref) => ( <DialogPrimitive.Close ref={ref} className={cn( "absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground", className )} {...props} > <X className="h-4 w-4" /> <span className="sr-only">Close</span> </DialogPrimitive.Close> )); DialogCloseButton.displayName = "DialogCloseButton";Then use it in DialogContent:
<DialogPrimitive.Content /*...props*/> {children} <DialogCloseButton /> </DialogPrimitive.Content>
29-51: 🧹 Nitpick (assertive)
Consider breaking down the extensive Tailwind classes.
The DialogContent implementation is solid but has a very long className string that could impact readability and maintainability.
Consider extracting these styles into smaller, logical groups:
className={cn( - 'fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border bg-background p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-lg', + [ + 'fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%]', // positioning + 'gap-4 border bg-background p-6 shadow-lg sm:rounded-lg', // styling + 'duration-200', // transition + 'data-[state=open]:animate-in data-[state=closed]:animate-out', // animation triggers + 'data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', // fade animations + 'data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95', // zoom animations + 'data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%]', // slide out + 'data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%]', // slide in + ].join(' '), className, )}Or alternatively, consider extracting this to a constant or utility function.
📝 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 DialogContent = React.forwardRef< React.ElementRef<typeof DialogPrimitive.Content>, React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> >(({ className, children, ...props }, ref) => ( <DialogPortal> <DialogOverlay /> <DialogPrimitive.Content ref={ref} className={cn( [ 'fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%]', // positioning 'gap-4 border bg-background p-6 shadow-lg sm:rounded-lg', // styling 'duration-200', // transition 'data-[state=open]:animate-in data-[state=closed]:animate-out', // animation triggers 'data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', // fade animations 'data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95', // zoom animations 'data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%]', // slide out 'data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%]', // slide in ].join(' '), className, )} {...props} > {children} <DialogPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground"> <X className="h-4 w-4" /> <span className="sr-only">Close</span> </DialogPrimitive.Close> </DialogPrimitive.Content> </DialogPortal> )) DialogContent.displayName = DialogPrimitive.Content.displayNameapps/portal/app/routes/schedule.tsx (1)
19-21:
⚠️ Potential issueReturn fallback value and fix error message.
Currently, the catch block neither returns data nor accurately references “events” or “schedule.”catch (error) { - console.error(`Error fetching challenges`, error) + console.error('Error fetching events', error) + return [] // Return fallback to prevent undefined }📝 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.catch (error) { console.error('Error fetching events', error) return [] // Return fallback to prevent undefined }libs/portal/pages/schedule.tsx (1)
49-85: 🧹 Nitpick (assertive)
Remove or integrate the commented-out dialog.
To maintain clean code, either fully implement or remove it.
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (6)
libs/portal/pages/qr.tsx (1)
5-5: 🧹 Nitpick (assertive)Remove commented console.log statement.
Commented debug code should be removed before production.
export function QrPage({ user }) { - /* console.log(user) */ const qrValue = 'https://yourwebsite.com' // Change this to your desired URL or textapps/portal/app/routes/map.tsx (1)
23-29:⚠️ Potential issueRename component to avoid shadowing global Map object.
The component name
Mapshadows the global JavaScriptMapobject, which could lead to confusion.-export default function Map() { +export default function MapRoute() { // const data = useLoaderData<typeof loader>() return ( <MapsPage /> ) }🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
apps/portal/app/routes/schedule.tsx (1)
6-22:⚠️ Potential issueFix error message and add return value in catch block.
The loader function has an incorrect error message in the catch block and doesn't return anything on error.
export const loader: LoaderFunction = async () => { try { const API_URL = process.env.NODE_ENV === 'development' ? 'http://localhost:8000' : 'https://axiom.cuhacking.ca' const req = await fetch(`${API_URL}/api/events`) if (!req.ok) { - throw new Error('Error') + throw new Error(`Failed to fetch events: ${req.status} ${req.statusText}`) } const data = await req.json() - // console.log(data) return data.docs } catch (error) { - console.error(`Error fetching challenges`, error) + console.error(`Error fetching events`, error) + return [] // Return empty array as fallback } }libs/shared/ui/accordion/accordion.tsx (1)
16-19: 🧹 Nitpick (assertive)Add JSDoc to document the new
noChevronprop.The newly introduced boolean prop affects how chevrons are rendered, but lacks documentation.
const AccordionTrigger = React.forwardRef< React.ElementRef<typeof AccordionPrimitive.Trigger>, - React.ComponentPropsWithoutRef<typeof AccordionPrimitive.Trigger> & { isLeft?: boolean } + React.ComponentPropsWithoutRef<typeof AccordionPrimitive.Trigger> & { + /** Position the chevron on the left side instead of the right */ + isLeft?: boolean; + /** When true, hides the chevron completely */ + noChevron?: boolean; + } >(({ className, children, isLeft, noChevron, ...props }, ref) => (libs/portal/pages/schedule.tsx (1)
36-36: 🧹 Nitpick (assertive)MultipleSelector should be self-closing
JSX elements without children should use self-closing syntax.
- <MultipleSelector isRequired={false} name="Filter" form={undefined} options={options} placeholder="filter" hidePlaceholderWhenSelected></MultipleSelector> + <MultipleSelector isRequired={false} name="Filter" form={undefined} options={options} placeholder="filter" hidePlaceholderWhenSelected />🧰 Tools
🪛 Biome (1.9.4)
[error] 36-38: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
libs/portal/pages/challenges.tsx (1)
61-70: 🧹 Nitpick (assertive)Leverage optional chaining for readability and safety.
Replace:
- {challenge.challengeBlock && challenge.challengeBlock.map(block => (...))} + {challenge.challengeBlock?.map(block => (...))}This ensures a cleaner syntax and avoids potential undefined property access.
🧰 Tools
🪛 Biome (1.9.4)
[error] 61-70: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
apps/axiom/src/components/Nav/navIconMap.ts(1 hunks)apps/portal/app/routes/challenges.tsx(1 hunks)apps/portal/app/routes/map.tsx(1 hunks)apps/portal/app/routes/profile.tsx(2 hunks)apps/portal/app/routes/qr.tsx(1 hunks)apps/portal/app/routes/schedule.tsx(1 hunks)libs/cms/configs/server.ts(2 hunks)libs/db/collections/models/Challenge.ts(0 hunks)libs/db/collections/models/Challenges.ts(1 hunks)libs/db/collections/models/Events.ts(4 hunks)libs/db/collections/models/Users.ts(1 hunks)libs/db/collections/models/index.ts(1 hunks)libs/db/seed/index.ts(3 hunks)libs/portal/pages/challenges.tsx(1 hunks)libs/portal/pages/dialog.tsx(1 hunks)libs/portal/pages/map.tsx(1 hunks)libs/portal/pages/qr.tsx(1 hunks)libs/portal/pages/schedule.tsx(1 hunks)libs/shared/ui/accordion/accordion.tsx(2 hunks)libs/shared/ui/glassmorphic-card/glassmorphic-card.tsx(3 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- libs/db/collections/models/Challenge.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/portal/app/routes/map.tsx
[error] 23-23: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
libs/shared/ui/glassmorphic-card/glassmorphic-card.tsx
[error] 56-56: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 57-57: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 58-58: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
libs/portal/pages/schedule.tsx
[error] 36-38: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
libs/portal/pages/challenges.tsx
[error] 61-70: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (22.13.1)
- GitHub Check: test
🔇 Additional comments (32)
apps/axiom/src/components/Nav/navIconMap.ts (1)
31-31: Correctly updated key name to match standardized terminology.This change from
challengePrizetochallengesaligns with the broader terminology standardization across the codebase, maintaining the Trophy icon assignment.libs/portal/pages/map.tsx (1)
3-13: Basic component structure is in place, but needs further development.This component provides a foundational structure with Layout and responsive styling, but it's currently just a placeholder without actual map functionality.
Consider enhancing this implementation by:
- Adding proper map display components
- Including JSDoc comment describing the component's purpose
- Preparing for handling the map data that will be fetched in the route loader
libs/cms/configs/server.ts (1)
10-10: Import and collection configuration properly synchronized.Both the import statement and collection array now consistently use
Challengesinstead ofChallengePrize, resolving the previous mismatch that could have caused runtime errors.Also applies to: 43-43
libs/db/collections/models/index.ts (1)
1-1: Export path correctly updated to match renamed collection file.The export statement now properly references the new
./Challengespath instead of./Challenge, completing the collection model renaming process.package.json (2)
99-99: New dependency for QR code generation added.This QR code library is being used in the new QR page component. The version looks appropriate.
105-105: New dependency for inline SVG handling added.The react-inlinesvg library will help with SVG rendering, likely for map components. Good choice for handling SVGs in React.
apps/portal/app/routes/profile.tsx (1)
5-5: Simplified loader return pattern.Removed the
jsonwrapper in favor of directly returning the object, which is a cleaner approach in Remix. This change properly aligns with Remix's automatic serialization of loader return values.Also applies to: 37-37
libs/db/seed/index.ts (1)
12-12: Added challenges seeding support.Good implementation of the challenges seeding functionality, following the same pattern used for other entities. The addition to the collections list and the seeding function call are properly placed.
Also applies to: 38-38, 80-80
libs/db/collections/models/Users.ts (1)
270-270:✅ Verification successful
Security policy adjustment to relax admin access in development.
The change modifies admin access control to use
authenticatedin development but keepsisOrganizerOrSponsorin production. This makes local development easier while maintaining stricter security in production.Verify this approach is intentional and all team members understand the security implications:
🏁 Script executed:
#!/bin/bash # Check if this pattern is used consistently across the codebase rg "process\.env\.NODE_ENV === 'development' \? authenticated" --type tsLength of output: 206
Security Adjustment Verification – Confirmed
The use of
process.env.NODE_ENV === 'development' ? authenticated : isOrganizerOrSponsorinlibs/db/collections/models/Users.ts(line 270) was verified with a grep search and appears to be intentionally applied only in this file. This design enables relaxed admin access during development (usingauthenticated) while enforcing stricter controls in production (usingisOrganizerOrSponsor). Please ensure that all team members understand these security implications.apps/portal/app/routes/map.tsx (1)
24-24: Unused loader dataThe loader fetches data but it's not being used in the component. Either uncomment this line if the data is needed or consider simplifying the loader.
apps/portal/app/routes/challenges.tsx (4)
8-8: Consider using environment variables for API URLsSimilar to the other route files, consider using environment variables for API URLs to improve configuration management.
-const API_URL = process.env.NODE_ENV === 'development' ? 'http://localhost:8000' : 'https://axiom.cuhacking.ca' +const API_URL = process.env.NODE_ENV === 'development' + ? process.env.API_LOCAL_URL || 'http://localhost:8000' + : process.env.API_PRODUCTION_URL || 'https://axiom.cuhacking.ca'
12-12: Improve error message specificityThe error message "Error" is not descriptive. Consider adding more context to help with debugging.
- throw new Error('Error') + throw new Error('Failed to fetch challenges: Invalid response status ' + req.status)
19-22: Good error handling with fallback dataThe catch block provides a default structure when an error occurs, which is a good practice to prevent UI errors.
25-30: Clean and concise component implementationThe component correctly uses the loader data and passes it to the ChallengesPage component.
libs/portal/pages/schedule.tsx (1)
32-34: Button dates are duplicatedAll three date buttons display the same date. They should presumably show different dates.
- <Button className="hover:bg-red-500 cursor-pointer">Fri Mar 14th</Button> - <Button className="cursor-pointer">Fri Mar 14th</Button> - <Button className="cursor-pointer">Fri Mar 14th</Button> + <Button className="hover:bg-red-500 cursor-pointer">Fri Mar 14th</Button> + <Button className="cursor-pointer">Sat Mar 15th</Button> + <Button className="cursor-pointer">Sun Mar 16th</Button>apps/portal/app/routes/qr.tsx (1)
8-42: Good authentication validation in loaderThe loader properly validates authentication and user agreement to terms, with appropriate redirects.
libs/shared/ui/glassmorphic-card/glassmorphic-card.tsx (2)
15-18: Good prop additions for enhanced functionalityThe new props (pathTitle, minimize, maximize, close) provide useful window-like functionality to the card component.
56-58: Convert empty<div>elements to self-closing elementsThe static analysis tool flags these divs as elements that should be self-closing.
- {minimize && (<div className="size-2 bg-green-500 rounded"></div>)} - {maximize && (<div className="size-2 bg-yellow-500 rounded"></div>)} - {close && (<div className="size-2 bg-red-500 rounded"></div>)} + {minimize && <div className="size-2 bg-green-500 rounded" />} + {maximize && <div className="size-2 bg-yellow-500 rounded" />} + {close && <div className="size-2 bg-red-500 rounded" />}🧰 Tools
🪛 Biome (1.9.4)
[error] 56-56: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 57-57: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 58-58: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
libs/db/collections/models/Challenges.ts (2)
15-15: Consider indexing the sponsor field.If you anticipate frequent lookups by the sponsor, adding an index can greatly improve query performance in larger datasets.
619-646: Refine bulk creation error handling and consider transactions.While the try/catch block records errors, a partial failure mid-seed might leave your challenges only partially written. For consistency, consider using a single transaction or more robust rollback logic to preserve data integrity.
libs/portal/pages/challenges.tsx (3)
83-95: Add TypeScript interfaces for block props.These components would be safer and clearer if you typed
blockData. For example:interface BlockBullet { id: string; title: string; bullets: { id: string; point: string }[]; } interface BlockButtons { id: string; title: string; buttons: { id: string; title: string; link: string }[]; }Then apply them in
ChallangeBlockBulletandChallangeBlockButtons.Also applies to: 97-115
90-92: Remove trailing commas in JSX mappings.Trailing commas inside parentheses in JSX can lead to syntax errors in some setups:
- ), + )Also applies to: 111-111
104-105: Addtargetandrelattributes for external links.For security and best user experience, set external links to open safely:
- <a href={btn.link} key={btn.id}> + <a href={btn.link} key={btn.id} target="_blank" rel="noopener noreferrer">libs/db/collections/models/Events.ts (2)
479-479: Remove the “?” in the location.The question mark indicates uncertainty. If the location is confirmed, please remove it for clarity.
50-50:❓ Verification inconclusive
Verify negative hour usage.
Using negative hours in Date constructors can shift the date, potentially leading to unintended day offsets. Confirm this aligns with your event schedule or adjust the hour/time to accurately represent the intended date and time.
Also applies to: 62-62, 256-256, 282-282, 459-459, 463-463
🏁 Script executed:
#!/bin/bash # Searching for negative-hour usage to confirm correctness rg -A 2 'new Date\(2025,.*,-\d+'Length of output: 34
Action Required: Confirm Negative Hour Usage
Please verify that the use of a negative hour (as seen in
defaultValue: () => new Date(2025, 2, 15, -4, 0, 0),in
libs/db/collections/models/Events.tsat line 50) is intentional. Negative values for the hour parameter will shift the date to the previous day, so ensure that this behavior aligns with your planned event schedule. This concern also applies to the similar usages at lines 62, 256, 282, 459, and 463.Since the automated search did not yield conclusive results, please perform a manual verification to confirm the intended behavior of these date constructors.
libs/portal/pages/dialog.tsx (7)
1-4: Import organization looks good.Clean import structure with proper separation of external libraries and internal utilities.
6-12: Appropriate component aliasing.Good practice to create clean aliases for the Radix UI primitives, making the code more readable and maintainable.
14-27: Well-implemented DialogOverlay component.The forwardRef implementation correctly passes refs to the underlying Radix UI component. The className composition with the cn utility follows best practices.
53-63: DialogHeader uses responsive design appropriately.The component properly implements text alignment that adapts to different screen sizes (centered on mobile, left-aligned on desktop).
66-76: DialogFooter has proper responsive layout.The footer correctly uses flex-direction changes based on screen size, providing a more natural button layout on different devices.
79-89: DialogTitle and DialogDescription follow consistent patterns.Both components follow the same pattern as other dialog components with proper forwardRef implementation and styling. Good consistency.
Also applies to: 91-101
103-114: Clean exports section.The named exports are well-organized and make all the dialog components available for use throughout the application.
| <QRCodeCanvas value={qrValue} size={200} /> | ||
| Hi | ||
| {' '} | ||
| {user.preferredDisplayName} | ||
| {' '} | ||
| ur doing great! | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve QR code presentation and user greeting.
The current implementation has informal language and lacks proper structure around the QR code. Consider improving the layout and wording for a more professional user experience.
- <QRCodeCanvas value={qrValue} size={200} />
- Hi
- {' '}
- {user.preferredDisplayName}
- {' '}
- ur doing great!
+ <div className="flex flex-col items-center space-y-4">
+ <h2 className="text-xl font-semibold">Your Participant QR Code</h2>
+ <QRCodeCanvas value={qrValue} size={200} />
+ <p className="text-center mt-2">
+ Welcome, {user?.preferredDisplayName || 'Participant'}!
+ <br />
+ Present this QR code at check-in and event stations.
+ </p>
+ </div>📝 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.
| <QRCodeCanvas value={qrValue} size={200} /> | |
| Hi | |
| {' '} | |
| {user.preferredDisplayName} | |
| {' '} | |
| ur doing great! | |
| </div> | |
| <div className="flex flex-col items-center space-y-4"> | |
| <h2 className="text-xl font-semibold">Your Participant QR Code</h2> | |
| <QRCodeCanvas value={qrValue} size={200} /> | |
| <p className="text-center mt-2"> | |
| Welcome, {user?.preferredDisplayName || 'Participant'}! | |
| <br /> | |
| Present this QR code at check-in and event stations. | |
| </p> | |
| </div> |
| import { Layout } from '@cuhacking/portal/ui/layout' | ||
| import { QRCodeCanvas } from 'qrcode.react' | ||
|
|
||
| export function QrPage({ user }) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type definition for user prop.
The user parameter lacks type information, which could lead to runtime errors if properties are accessed incorrectly.
+import type { UserDetails } from '@cuhacking/portal/types/user'
import { Layout } from '@cuhacking/portal/ui/layout'
import { QRCodeCanvas } from 'qrcode.react'
-export function QrPage({ user }) {
+export function QrPage({ user }: { user: UserDetails }) {📝 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 function QrPage({ user }) { | |
| import type { UserDetails } from '@cuhacking/portal/types/user' | |
| import { Layout } from '@cuhacking/portal/ui/layout' | |
| import { QRCodeCanvas } from 'qrcode.react' | |
| export function QrPage({ user }: { user: UserDetails }) { | |
| // ...rest of the implementation... | |
| } |
|
|
||
| export function QrPage({ user }) { | ||
| /* console.log(user) */ | ||
| const qrValue = 'https://yourwebsite.com' // Change this to your desired URL or text |
There was a problem hiding this comment.
Replace placeholder QR URL with dynamic value.
The current URL is a placeholder as indicated by the comment. This should be replaced with an actual application URL, ideally one that's specific to this user.
- const qrValue = 'https://yourwebsite.com' // Change this to your desired URL or text
+ const qrValue = `https://hackathon-domain.com/participant/${user.id}` // Dynamic URL based on user📝 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 qrValue = 'https://yourwebsite.com' // Change this to your desired URL or text | |
| const qrValue = `https://hackathon-domain.com/participant/${user.id}` // Dynamic URL based on user |
| export const loader: LoaderFunction = async () => { | ||
| try { | ||
| const API_URL = process.env.NODE_ENV === 'development' ? 'http://localhost:8000' : 'https://axiom.cuhacking.ca' | ||
| const req = await fetch(`${API_URL}/api/challenges`) | ||
|
|
||
| if (!req.ok) { | ||
| throw new Error('Error') | ||
| } | ||
|
|
||
| const data = await req.json() | ||
|
|
||
| return data.docs | ||
| } | ||
| catch (error) { | ||
| console.error(`Error fetching challenges`, error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix missing return value in catch block and API endpoint inconsistency.
The loader function has several issues:
- It doesn't return anything when an error occurs, which will cause runtime errors
- It fetches from
/api/challengesdespite being a map route - The error message isn't descriptive
Apply these fixes to prevent runtime errors and improve error handling:
export const loader: LoaderFunction = async () => {
try {
const API_URL = process.env.NODE_ENV === 'development' ? 'http://localhost:8000' : 'https://axiom.cuhacking.ca'
- const req = await fetch(`${API_URL}/api/challenges`)
+ const req = await fetch(`${API_URL}/api/maps`)
if (!req.ok) {
- throw new Error('Error')
+ throw new Error(`Failed to fetch maps: ${req.status} ${req.statusText}`)
}
const data = await req.json()
return data.docs
}
catch (error) {
- console.error(`Error fetching challenges`, error)
+ console.error(`Error fetching maps`, error)
+ return [] // Return empty array as fallback
}
}📝 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 const loader: LoaderFunction = async () => { | |
| try { | |
| const API_URL = process.env.NODE_ENV === 'development' ? 'http://localhost:8000' : 'https://axiom.cuhacking.ca' | |
| const req = await fetch(`${API_URL}/api/challenges`) | |
| if (!req.ok) { | |
| throw new Error('Error') | |
| } | |
| const data = await req.json() | |
| return data.docs | |
| } | |
| catch (error) { | |
| console.error(`Error fetching challenges`, error) | |
| } | |
| } | |
| export const loader: LoaderFunction = async () => { | |
| try { | |
| const API_URL = | |
| process.env.NODE_ENV === 'development' | |
| ? 'http://localhost:8000' | |
| : 'https://axiom.cuhacking.ca' | |
| const req = await fetch(`${API_URL}/api/maps`) | |
| if (!req.ok) { | |
| throw new Error(`Failed to fetch maps: ${req.status} ${req.statusText}`) | |
| } | |
| const data = await req.json() | |
| return data.docs | |
| } catch (error) { | |
| console.error(`Error fetching maps`, error) | |
| return [] // Return empty array as fallback | |
| } | |
| } |
| const baseUrl | ||
| = process.env.NODE_ENV === 'development' | ||
| ? 'http://localhost:8000' | ||
| : 'https://axiom.cuhacking.ca' | ||
|
|
||
| const API_URL = baseUrl |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fix indentation in API_URL definition
The baseUrl variable definition has inconsistent indentation. Standardize the formatting.
- const baseUrl
- = process.env.NODE_ENV === 'development'
- ? 'http://localhost:8000'
- : 'https://axiom.cuhacking.ca'
+ const baseUrl = process.env.NODE_ENV === 'development'
+ ? 'http://localhost:8000'
+ : 'https://axiom.cuhacking.ca'📝 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 baseUrl | |
| = process.env.NODE_ENV === 'development' | |
| ? 'http://localhost:8000' | |
| : 'https://axiom.cuhacking.ca' | |
| const API_URL = baseUrl | |
| const baseUrl = process.env.NODE_ENV === 'development' | |
| ? 'http://localhost:8000' | |
| : 'https://axiom.cuhacking.ca' | |
| const API_URL = baseUrl |
| {pathTitle && ( | ||
| <div className={cn('flex justify-between items-center p-2 h-9 w-full !rounded-b-[0px] border-bottom', glassmorphicCardVariants({ variant }), className)}> | ||
| <Typography variant="paragraph-xs">{pathTitle}</Typography> | ||
| <div className="flex flex-shrink-0 items-center gap-2 h-full"> | ||
| {minimize && (<div className="size-2 bg-green-500 rounded"></div>)} | ||
| {maximize && (<div className="size-2 bg-yellow-500 rounded"></div>)} | ||
| {close && (<div className="size-2 bg-red-500 rounded"></div>)} | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider extracting window controls to a separate component
The window-like controls (title bar with minimize, maximize, close buttons) could be extracted to a separate component for reusability.
type WindowControlsProps = {
title?: string;
minimize?: boolean;
maximize?: boolean;
close?: boolean;
};
function WindowControls({ title, minimize, maximize, close }: WindowControlsProps) {
return (
<div className="flex justify-between items-center p-2 h-9 w-full !rounded-b-[0px] border-bottom">
<Typography variant="paragraph-xs">{title}</Typography>
<div className="flex flex-shrink-0 items-center gap-2 h-full">
{minimize && <div className="size-2 bg-green-500 rounded" />}
{maximize && <div className="size-2 bg-yellow-500 rounded" />}
{close && <div className="size-2 bg-red-500 rounded" />}
</div>
</div>
);
}Then in GlassmorphicCard:
- {pathTitle && (
- <div className={cn('flex justify-between items-center p-2 h-9 w-full !rounded-b-[0px] border-bottom', glassmorphicCardVariants({ variant }), className)}>
- <Typography variant="paragraph-xs">{pathTitle}</Typography>
- <div className="flex flex-shrink-0 items-center gap-2 h-full">
- {minimize && (<div className="size-2 bg-green-500 rounded"></div>)}
- {maximize && (<div className="size-2 bg-yellow-500 rounded"></div>)}
- {close && (<div className="size-2 bg-red-500 rounded"></div>)}
- </div>
- </div>
- )}
+ {pathTitle && (
+ <div className={cn(glassmorphicCardVariants({ variant }), className)}>
+ <WindowControls
+ title={pathTitle}
+ minimize={minimize}
+ maximize={maximize}
+ close={close}
+ />
+ </div>
+ )}🧰 Tools
🪛 Biome (1.9.4)
[error] 56-56: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 57-57: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 58-58: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
| blockName: "QNX infos", | ||
| blockType: "info", |
There was a problem hiding this comment.
Standardize block type naming “info” vs. “infos.”
Multiple blocks use “info” or “infos,” causing inconsistency that can complicate data processing. Align all blocks on “info” or whichever name you prefer for uniformity.
Also applies to: 432-433, 457-458
| const DialogContent = React.forwardRef< | ||
| React.ElementRef<typeof DialogPrimitive.Content>, | ||
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> | ||
| >(({ className, children, ...props }, ref) => ( | ||
| <DialogPortal> | ||
| <DialogOverlay /> | ||
| <DialogPrimitive.Content | ||
| ref={ref} | ||
| className={cn( | ||
| 'fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border bg-background p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-lg', | ||
| className, | ||
| )} | ||
| {...props} | ||
| > | ||
| {children} | ||
| <DialogPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground"> | ||
| <X className="h-4 w-4" /> | ||
| <span className="sr-only">Close</span> | ||
| </DialogPrimitive.Close> | ||
| </DialogPrimitive.Content> | ||
| </DialogPortal> | ||
| )) | ||
| DialogContent.displayName = DialogPrimitive.Content.displayName |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
DialogContent implements proper accessibility.
Good implementation of the dialog content with proper z-index, animations, and accessibility features. The close button includes both a visual icon and screen reader text.
Consider extracting the lengthy className string to a constant or using a styling system that allows for more maintainable class definitions.
| import { cn } from '@cuhacking/shared/utils/cn' | ||
| import * as DialogPrimitive from '@radix-ui/react-dialog' | ||
| import { X } from 'lucide-react' | ||
| import * as React from 'react' | ||
|
|
||
| const Dialog = DialogPrimitive.Root | ||
|
|
||
| const DialogTrigger = DialogPrimitive.Trigger | ||
|
|
||
| const DialogPortal = DialogPrimitive.Portal | ||
|
|
||
| const DialogClose = DialogPrimitive.Close | ||
|
|
||
| const DialogOverlay = React.forwardRef< | ||
| React.ElementRef<typeof DialogPrimitive.Overlay>, | ||
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Overlay> | ||
| >(({ className, ...props }, ref) => ( | ||
| <DialogPrimitive.Overlay | ||
| ref={ref} | ||
| className={cn( | ||
| 'fixed inset-0 z-50 bg-black/80 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', | ||
| className, | ||
| )} | ||
| {...props} | ||
| /> | ||
| )) | ||
| DialogOverlay.displayName = DialogPrimitive.Overlay.displayName | ||
|
|
||
| const DialogContent = React.forwardRef< | ||
| React.ElementRef<typeof DialogPrimitive.Content>, | ||
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> | ||
| >(({ className, children, ...props }, ref) => ( | ||
| <DialogPortal> | ||
| <DialogOverlay /> | ||
| <DialogPrimitive.Content | ||
| ref={ref} | ||
| className={cn( | ||
| 'fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border bg-background p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-lg', | ||
| className, | ||
| )} | ||
| {...props} | ||
| > | ||
| {children} | ||
| <DialogPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground"> | ||
| <X className="h-4 w-4" /> | ||
| <span className="sr-only">Close</span> | ||
| </DialogPrimitive.Close> | ||
| </DialogPrimitive.Content> | ||
| </DialogPortal> | ||
| )) | ||
| DialogContent.displayName = DialogPrimitive.Content.displayName | ||
|
|
||
| function DialogHeader({ | ||
| className, | ||
| ...props | ||
| }: React.HTMLAttributes<HTMLDivElement>) { | ||
| return ( | ||
| <div | ||
| className={cn('flex flex-col space-y-1.5 text-center sm:text-left', className)} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } | ||
| DialogHeader.displayName = 'DialogHeader' | ||
|
|
||
| function DialogFooter({ | ||
| className, | ||
| ...props | ||
| }: React.HTMLAttributes<HTMLDivElement>) { | ||
| return ( | ||
| <div | ||
| className={cn('flex flex-col-reverse sm:flex-row sm:justify-end sm:space-x-2', className)} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } | ||
| DialogFooter.displayName = 'DialogFooter' | ||
|
|
||
| const DialogTitle = React.forwardRef< | ||
| React.ElementRef<typeof DialogPrimitive.Title>, | ||
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Title> | ||
| >(({ className, ...props }, ref) => ( | ||
| <DialogPrimitive.Title | ||
| ref={ref} | ||
| className={cn('text-lg font-semibold leading-none tracking-tight', className)} | ||
| {...props} | ||
| /> | ||
| )) | ||
| DialogTitle.displayName = DialogPrimitive.Title.displayName | ||
|
|
||
| const DialogDescription = React.forwardRef< | ||
| React.ElementRef<typeof DialogPrimitive.Description>, | ||
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Description> | ||
| >(({ className, ...props }, ref) => ( | ||
| <DialogPrimitive.Description | ||
| ref={ref} | ||
| className={cn('text-sm text-muted-foreground', className)} | ||
| {...props} | ||
| /> | ||
| )) | ||
| DialogDescription.displayName = DialogPrimitive.Description.displayName | ||
|
|
||
| export { | ||
| Dialog, | ||
| DialogClose, | ||
| DialogContent, | ||
| DialogDescription, | ||
| DialogFooter, | ||
| DialogHeader, | ||
| DialogOverlay, | ||
| DialogPortal, | ||
| DialogTitle, | ||
| DialogTrigger, | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding animation preference support.
The dialog includes several animations which may cause issues for users with vestibular disorders or those who prefer reduced motion.
Consider conditionally applying animations based on the user's motion preferences:
import { cn } from '@cuhacking/shared/utils/cn'
import * as DialogPrimitive from '@radix-ui/react-dialog'
import { X } from 'lucide-react'
import * as React from 'react'
+import { useReducedMotion } from '@cuhacking/shared/hooks/use-reduced-motion'
// Later in your component:
const DialogOverlay = React.forwardRef<
React.ElementRef<typeof DialogPrimitive.Overlay>,
React.ComponentPropsWithoutRef<typeof DialogPrimitive.Overlay>
>(({ className, ...props }, ref) => {
+ const prefersReducedMotion = useReducedMotion()
+
+ const animationClasses = prefersReducedMotion
+ ? ''
+ : 'data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0'
+
return (
<DialogPrimitive.Overlay
ref={ref}
className={cn(
- 'fixed inset-0 z-50 bg-black/80 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0',
+ `fixed inset-0 z-50 bg-black/80 ${animationClasses}`,
className,
)}
{...props}
/>
)
})You would need to implement similar changes for DialogContent component animations.
📝 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.
| import { cn } from '@cuhacking/shared/utils/cn' | |
| import * as DialogPrimitive from '@radix-ui/react-dialog' | |
| import { X } from 'lucide-react' | |
| import * as React from 'react' | |
| const Dialog = DialogPrimitive.Root | |
| const DialogTrigger = DialogPrimitive.Trigger | |
| const DialogPortal = DialogPrimitive.Portal | |
| const DialogClose = DialogPrimitive.Close | |
| const DialogOverlay = React.forwardRef< | |
| React.ElementRef<typeof DialogPrimitive.Overlay>, | |
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Overlay> | |
| >(({ className, ...props }, ref) => ( | |
| <DialogPrimitive.Overlay | |
| ref={ref} | |
| className={cn( | |
| 'fixed inset-0 z-50 bg-black/80 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', | |
| className, | |
| )} | |
| {...props} | |
| /> | |
| )) | |
| DialogOverlay.displayName = DialogPrimitive.Overlay.displayName | |
| const DialogContent = React.forwardRef< | |
| React.ElementRef<typeof DialogPrimitive.Content>, | |
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> | |
| >(({ className, children, ...props }, ref) => ( | |
| <DialogPortal> | |
| <DialogOverlay /> | |
| <DialogPrimitive.Content | |
| ref={ref} | |
| className={cn( | |
| 'fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border bg-background p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-lg', | |
| className, | |
| )} | |
| {...props} | |
| > | |
| {children} | |
| <DialogPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground"> | |
| <X className="h-4 w-4" /> | |
| <span className="sr-only">Close</span> | |
| </DialogPrimitive.Close> | |
| </DialogPrimitive.Content> | |
| </DialogPortal> | |
| )) | |
| DialogContent.displayName = DialogPrimitive.Content.displayName | |
| function DialogHeader({ | |
| className, | |
| ...props | |
| }: React.HTMLAttributes<HTMLDivElement>) { | |
| return ( | |
| <div | |
| className={cn('flex flex-col space-y-1.5 text-center sm:text-left', className)} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| DialogHeader.displayName = 'DialogHeader' | |
| function DialogFooter({ | |
| className, | |
| ...props | |
| }: React.HTMLAttributes<HTMLDivElement>) { | |
| return ( | |
| <div | |
| className={cn('flex flex-col-reverse sm:flex-row sm:justify-end sm:space-x-2', className)} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| DialogFooter.displayName = 'DialogFooter' | |
| const DialogTitle = React.forwardRef< | |
| React.ElementRef<typeof DialogPrimitive.Title>, | |
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Title> | |
| >(({ className, ...props }, ref) => ( | |
| <DialogPrimitive.Title | |
| ref={ref} | |
| className={cn('text-lg font-semibold leading-none tracking-tight', className)} | |
| {...props} | |
| /> | |
| )) | |
| DialogTitle.displayName = DialogPrimitive.Title.displayName | |
| const DialogDescription = React.forwardRef< | |
| React.ElementRef<typeof DialogPrimitive.Description>, | |
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Description> | |
| >(({ className, ...props }, ref) => ( | |
| <DialogPrimitive.Description | |
| ref={ref} | |
| className={cn('text-sm text-muted-foreground', className)} | |
| {...props} | |
| /> | |
| )) | |
| DialogDescription.displayName = DialogPrimitive.Description.displayName | |
| export { | |
| Dialog, | |
| DialogClose, | |
| DialogContent, | |
| DialogDescription, | |
| DialogFooter, | |
| DialogHeader, | |
| DialogOverlay, | |
| DialogPortal, | |
| DialogTitle, | |
| DialogTrigger, | |
| } | |
| import { cn } from '@cuhacking/shared/utils/cn' | |
| import * as DialogPrimitive from '@radix-ui/react-dialog' | |
| import { X } from 'lucide-react' | |
| import * as React from 'react' | |
| import { useReducedMotion } from '@cuhacking/shared/hooks/use-reduced-motion' | |
| const Dialog = DialogPrimitive.Root | |
| const DialogTrigger = DialogPrimitive.Trigger | |
| const DialogPortal = DialogPrimitive.Portal | |
| const DialogClose = DialogPrimitive.Close | |
| const DialogOverlay = React.forwardRef< | |
| React.ElementRef<typeof DialogPrimitive.Overlay>, | |
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Overlay> | |
| >(({ className, ...props }, ref) => { | |
| const prefersReducedMotion = useReducedMotion() | |
| const animationClasses = prefersReducedMotion | |
| ? '' | |
| : 'data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0' | |
| return ( | |
| <DialogPrimitive.Overlay | |
| ref={ref} | |
| className={cn( | |
| `fixed inset-0 z-50 bg-black/80 ${animationClasses}`, | |
| className, | |
| )} | |
| {...props} | |
| /> | |
| ) | |
| }) | |
| DialogOverlay.displayName = DialogPrimitive.Overlay.displayName | |
| const DialogContent = React.forwardRef< | |
| React.ElementRef<typeof DialogPrimitive.Content>, | |
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> | |
| >(({ className, children, ...props }, ref) => ( | |
| <DialogPortal> | |
| <DialogOverlay /> | |
| <DialogPrimitive.Content | |
| ref={ref} | |
| className={cn( | |
| 'fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border bg-background p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-lg', | |
| className, | |
| )} | |
| {...props} | |
| > | |
| {children} | |
| <DialogPrimitive.Close className="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground"> | |
| <X className="h-4 w-4" /> | |
| <span className="sr-only">Close</span> | |
| </DialogPrimitive.Close> | |
| </DialogPrimitive.Content> | |
| </DialogPortal> | |
| )) | |
| DialogContent.displayName = DialogPrimitive.Content.displayName | |
| function DialogHeader({ | |
| className, | |
| ...props | |
| }: React.HTMLAttributes<HTMLDivElement>) { | |
| return ( | |
| <div | |
| className={cn('flex flex-col space-y-1.5 text-center sm:text-left', className)} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| DialogHeader.displayName = 'DialogHeader' | |
| function DialogFooter({ | |
| className, | |
| ...props | |
| }: React.HTMLAttributes<HTMLDivElement>) { | |
| return ( | |
| <div | |
| className={cn('flex flex-col-reverse sm:flex-row sm:justify-end sm:space-x-2', className)} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| DialogFooter.displayName = 'DialogFooter' | |
| const DialogTitle = React.forwardRef< | |
| React.ElementRef<typeof DialogPrimitive.Title>, | |
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Title> | |
| >(({ className, ...props }, ref) => ( | |
| <DialogPrimitive.Title | |
| ref={ref} | |
| className={cn('text-lg font-semibold leading-none tracking-tight', className)} | |
| {...props} | |
| /> | |
| )) | |
| DialogTitle.displayName = DialogPrimitive.Title.displayName | |
| const DialogDescription = React.forwardRef< | |
| React.ElementRef<typeof DialogPrimitive.Description>, | |
| React.ComponentPropsWithoutRef<typeof DialogPrimitive.Description> | |
| >(({ className, ...props }, ref) => ( | |
| <DialogPrimitive.Description | |
| ref={ref} | |
| className={cn('text-sm text-muted-foreground', className)} | |
| {...props} | |
| /> | |
| )) | |
| DialogDescription.displayName = DialogPrimitive.Description.displayName | |
| export { | |
| Dialog, | |
| DialogClose, | |
| DialogContent, | |
| DialogDescription, | |
| DialogFooter, | |
| DialogHeader, | |
| DialogOverlay, | |
| DialogPortal, | |
| DialogTitle, | |
| DialogTrigger, | |
| } |
MFarabi619
force-pushed
the
aashna/challenge-schedule-map-qr-routes
branch
from
d73af6b to
c7035d8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
libs/portal/pages/dialog.tsx (1)
1-115: 🧹 Nitpick (assertive)Support reduced motion for accessibility.
You’re applying animation classes for show/hide transitions. Consider conditionally disabling these animations if the user prefers reduced motion.+import { useReducedMotion } from '@cuhacking/shared/hooks/use-reduced-motion' const DialogOverlay = React.forwardRef< React.ElementRef<typeof DialogPrimitive.Overlay>, React.ComponentPropsWithoutRef<typeof DialogPrimitive.Overlay> >(({ className, ...props }, ref) => { + const prefersReducedMotion = useReducedMotion() + const animationClasses = prefersReducedMotion + ? '' + : 'data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0' return ( <DialogPrimitive.Overlay ref={ref} - className={cn('fixed inset-0 z-50 bg-black/80 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', className)} + className={cn('fixed inset-0 z-50 bg-black/80', animationClasses, className)} {...props} /> ) })
♻️ Duplicate comments (1)
libs/portal/pages/schedule.tsx (1)
87-128: 🛠️ Refactor suggestionAdd or enforce typing for eventData.
TheEventcomponent relies oneventData.type.map(...)and other fields. IfeventData.typeis guaranteed to be a string array, consider adding a proper TypeScript interface to avoid runtime errors. If it's not always defined, use optional chaining (eventData.type?.map(...)) to prevent crashes.-function Event({ eventData }) { +interface ScheduleEvent { + id: string; + title: string; + location: string; + start: string; + end: string; + description?: string; + type: string[]; +} + +function Event({ eventData }: { eventData: ScheduleEvent }) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apps/portal/app/routes/schedule.tsx(1 hunks)libs/db/collections/models/Challenges.ts(1 hunks)libs/db/collections/models/Events.ts(4 hunks)libs/portal/pages/dialog.tsx(1 hunks)libs/portal/pages/schedule.tsx(1 hunks)libs/shared/ui/button/button.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
libs/portal/pages/schedule.tsx
[error] 41-41: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (22.13.1)
- GitHub Check: test
🔇 Additional comments (14)
libs/db/collections/models/Challenges.ts (4)
14-15: Consider indexing commonly queried fields liketitleandsponsor.In large datasets, indexing helps improve query performance. If these fields are heavily queried, adding indexes in your schema definition can significantly speed up lookups.
431-436: Standardize block type naming.Currently this block uses
"infos"while others use"info". For consistency, consider renaming"infos"to"info"or vice versa.
456-461: Standardize block type naming (repeated).This block once again uses
"infos"instead of"info". Standardizing these block types helps avoid confusion.
61-646: Enhance bulk insert reliability and atomicity.Using
Promise.allfor multiple inserts can lead to partial failures if one insert fails and isn’t properly retried. Consider wrapping these operations in transactions or adding retry logic to maintain data consistency.libs/shared/ui/button/button.tsx (1)
13-13: Hover text color addition looks good.The hover state now changes the text color to
text-background, which improves visual consistency with the background color. No concerns.apps/portal/app/routes/schedule.tsx (2)
6-36: Robust multi-page fetch approach.The pagination loop and error handling are well implemented. Returning all events once pagination completes is straightforward and clear.
38-42: Neat page rendering.Passing fetched data to
SchedulePageis concise and properly uses the loader’s return value.libs/portal/pages/schedule.tsx (1)
16-20: Static dates look fine.
Defining thedaysarray is clear and concise, aligning well with event scheduling.libs/db/collections/models/Events.ts (6)
3-3: Configuration imports look good.
No issues spotted with the newCollectionConfigorPayloadimports.
12-17: Organizers-only write access.
This approach is secure, restricting modifications to authorized roles.
76-81: Additional event type options.
Great to see expanded coverage for different event types, adding "Hackathon" and "Fun."
50-50: Repeated negative-hour usage.
Negative hours can shift the date backward and may cause confusion about intended scheduling. This concern was raised previously. Confirm that the negative hours correctly reflect your intended times.Also applies to: 255-256, 273-273, 279-279, 283-283, 291-291, 297-297, 309-309, 318-319, 327-327, 444-445, 453-454, 462-463
479-479: Remove the question mark from location.
"Hackathon RB (Main Stage?)" contains a question mark suggesting uncertainty. This was flagged previously; consider clarifying the location if finalized.
110-552: Seeding logic appears correct.
Inserting event records with images and associated data aligns with the schema. No pitfalls detected in the bulk creation flow.
| const formatTime = (isoString: string) => format(new Date(isoString), 'h:mm a') | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider time zone implications.
formatTime uses local time conversion. If your events cater to attendees in multiple time zones, you may want to standardize the display with a UTC offset or time zone info.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
libs/portal/pages/schedule.tsx (1)
52-53: 🧹 Nitpick (assertive)Provide an explicit button type.
Usetype="button"to avoid default form submission behavior.- <button key={day.date} onClick={() => setSelectedDay(day.date)}> + <button key={day.date} type="button" onClick={() => setSelectedDay(day.date)}>🧰 Tools
🪛 Biome (1.9.4)
[error] 52-53: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset(lint/a11y/useButtonType)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apps/axiom/src/migrations/20250314_225134.ts(1 hunks)apps/axiom/src/migrations/index.ts(2 hunks)apps/portal/app/routes/challenges.tsx(1 hunks)apps/portal/app/routes/schedule.tsx(1 hunks)libs/db/collections/models/Challenge.ts(1 hunks)libs/portal/pages/schedule.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
libs/portal/pages/schedule.tsx
[error] 52-53: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (22.13.1)
🔇 Additional comments (18)
apps/axiom/src/migrations/index.ts (3)
6-6: Great import for the new migration.No concerns; the import path looks correct and follows the existing convention.
32-33: Trailing comma alignment.The trailing comma after
'20250314_204702'is consistent with modern JavaScript/TypeScript styling. No issues here.
34-37: New migration appended correctly.The new entry for
'20250314_225134'is properly included. Ensure you run the migrations locally to validate the ordering and references.apps/axiom/src/migrations/20250314_225134.ts (5)
3-4: Signature consistency check.The
upfunction signature aligns with other migrations. Continue to maintain consistency across all migration functions.
5-7: Event type expansion.Adding new enum values might affect pre-existing data or references. Verify that no older records assume a specific enum ordering.
8-48: New tables schema.The schema definitions look sound. Double-check whether
ON DELETE CASCADEsuits your data retention and maintainability needs.
50-59: Dropping legacy tables.Double-check that all relevant data in
challenge_prize*tables is either backed up or no longer needed. Potential data loss may occur.
113-186: Down migration thoroughness.Recreation of legacy tables and type reversion appears complete. If partial or incremental data migrated externally, verify that reversions don’t cause inconsistencies.
libs/db/collections/models/Challenge.ts (8)
146-148: Consistent Challenge Prompts Formatting
The prompts are now formatted with double quotes and maintain consistency across the challenge object.
152-155: Standardized Prize Format
The prize entries now explicitly define the numeric keys with double-quoted string values. Ensure that this structure meets the requirements for data consumption by downstream processes.
164-165: Explicit Details for Gadget Challenge
The details array for the "Best use of Gadget" challenge is now explicitly defined with consistent formatting. This improves clarity and ensures uniform data structure.
173-175: Explicit Empty Arrays and Prize Definition
For the "Best overall hack" challenge, explicitly setting empty arrays fordetailsandprompts—along with a definedprizesfield—enhances consistency and simplifies downstream processing.
179-182: Uniform Representation for Missing Data
For the "Most Outrageous Hack/Turn Brainrot into brain nourishment" challenge, empty arrays fordetails,prompts, andprizesensure a predictable data format even when no values are provided.
191-194: Standardized Data for Wolfram Challenge
The explicit declaration of empty arrays fordetailsandpromptsin the "Top 5 teams" challenge promotes uniformity across documents.
199-203: Explicit Empty Arrays in 'Best First Time Hack'
Defining empty arrays fordetails,prompts, andprizesin the "Best First Time Hack" challenge maintains consistency with other challenge entries, which can help avoid null-related errors.
211-214: Consistent Data Formatting in Collaboration Challenge
Ensuring that all fields (prompts,prizes, anddetails) are explicitly defined—even as empty arrays—in the "Best uOttawa x Carleton x Algonquin collaboration" challenge reinforces a reliable data contract.apps/portal/app/routes/schedule.tsx (1)
43-47: Loader usage looks good.
Overall, the usage ofuseLoaderDatawith the fetched events is straightforward and clear.apps/portal/app/routes/challenges.tsx (1)
54-58: Rendered challenges look fine.
No issues found with how the loader data is passed toChallengesPage.
| DO $$ BEGIN | ||
| ALTER TABLE "challenges_blocks_info_bullets" ADD CONSTRAINT "challenges_blocks_info_bullets_parent_id_fk" FOREIGN KEY ("_parent_id") REFERENCES "public"."challenges_blocks_info"("id") ON DELETE cascade ON UPDATE no action; | ||
| EXCEPTION | ||
| WHEN duplicate_object THEN null; | ||
| END $$; | ||
| DO $$ BEGIN | ||
| ALTER TABLE "challenges_blocks_info" ADD CONSTRAINT "challenges_blocks_info_parent_id_fk" FOREIGN KEY ("_parent_id") REFERENCES "public"."challenges"("id") ON DELETE cascade ON UPDATE no action; | ||
| EXCEPTION | ||
| WHEN duplicate_object THEN null; | ||
| END $$; | ||
| DO $$ BEGIN | ||
| ALTER TABLE "challenges_blocks_resources_buttons" ADD CONSTRAINT "challenges_blocks_resources_buttons_parent_id_fk" FOREIGN KEY ("_parent_id") REFERENCES "public"."challenges_blocks_resources"("id") ON DELETE cascade ON UPDATE no action; | ||
| EXCEPTION | ||
| WHEN duplicate_object THEN null; | ||
| END $$; | ||
| DO $$ BEGIN | ||
| ALTER TABLE "challenges_blocks_resources" ADD CONSTRAINT "challenges_blocks_resources_parent_id_fk" FOREIGN KEY ("_parent_id") REFERENCES "public"."challenges"("id") ON DELETE cascade ON UPDATE no action; | ||
| EXCEPTION | ||
| WHEN duplicate_object THEN null; | ||
| END $$; | ||
| DO $$ BEGIN | ||
| ALTER TABLE "challenges" ADD CONSTRAINT "challenges_sponsor_id_brands_id_fk" FOREIGN KEY ("sponsor_id") REFERENCES "public"."brands"("id") ON DELETE set null ON UPDATE no action; | ||
| EXCEPTION | ||
| WHEN duplicate_object THEN null; | ||
| END $$; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Handling duplicate constraints.
Catching duplicate_object silently could mask unexpected issues. Consider logging or verifying the reason to avoid swallowing actual errors.
| "Create a project using QNX OS 8.0 that drives some hardware components.", | ||
| "Note: for cameras, only very specific camera units are supported. ", |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consistent Challenge Details Formatting
The challenge details for the QNX challenge now use double quotes for consistency. Please verify that no trailing whitespace (note the extra space at the end of line 143) is unintentionally introduced.
| } | ||
| ] | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Trailing Comma/Bracket Formatting
The formatting change at the end of the challengesData array appears to adjust the placement of the closing bracket. Verify that this change aligns with the project’s style guidelines regarding trailing commas and array termination.
| export const loader: LoaderFunction = async ({ request }) => { | ||
| const cookie = request.headers.get('Cookie') | ||
|
|
||
| try { | ||
| const API_URL | ||
| = process.env.NODE_ENV === 'development' | ||
| ? 'http://localhost:8000' | ||
| : 'https://axiom.cuhacking.ca' | ||
|
|
||
| let allEvents: any[] = [] | ||
| let page = 1 | ||
| let hasNextPage = true | ||
|
|
||
| while (hasNextPage) { | ||
| const req = await fetch(`${API_URL}/api/events?page=${page}&limit=100`, { | ||
| credentials: 'include', | ||
| headers: { Cookie: cookie || '' }, | ||
| }) | ||
|
|
||
| if (!req.ok) { | ||
| throw new Response('Error fetching events', { status: req.status }) | ||
| } | ||
|
|
||
| const data = await req.json() | ||
| allEvents = [...allEvents, ...data.docs] | ||
| hasNextPage = data.hasNextPage | ||
| page = data.nextPage | ||
| } | ||
|
|
||
| return allEvents | ||
| } | ||
| catch (error) { | ||
| console.error(`Error fetching events`, error) | ||
| throw new Response('Internal Server Error', { status: 500 }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use a typed event interface and bound pagination.
Defining a dedicated interface for the returned events would help maintain clarity and consistency. Also, consider bounding the pagination loop to avoid potential infinite iterations if the server misbehaves.
| export function SchedulePage({ data }) { | ||
| const [selectedDay, setSelectedDay] = useState('2025-03-14') | ||
|
|
||
| const filteredEvents = data.filter((event) => { | ||
| const eventDate = format(parseISO(event.start), 'yyyy-MM-dd') | ||
| return eventDate === selectedDay | ||
| }) | ||
|
|
||
| return ( | ||
| <Layout isCompleteProfile={false}> | ||
| <section className="max-w-screen-xl mx-auto p-5 sm:px-10 py-40 pt-10"> | ||
| <GlassmorphicCard className="row-span-2 p-5 text-center mb-5"> | ||
| <Typography variant="h2">SCHEDULE</Typography> | ||
| </GlassmorphicCard> | ||
|
|
||
| <div className="flex flex-col gap-5"> | ||
| {/* DAY SELECTION */} | ||
| <div className="flex flex-row gap-3 justify-center"> | ||
| {days.map(day => ( | ||
| <button key={day.date} onClick={() => setSelectedDay(day.date)}> | ||
| <GlassmorphicCard | ||
| className={cn( | ||
| 'flex flex-col gap-y-1 p-3 w-auto cursor-pointer hover:border-primary', | ||
| selectedDay === day.date ? 'border-2 border-primary' : '', | ||
| )} | ||
| > | ||
| <Typography | ||
| variant="h6" | ||
| className={selectedDay === day.date ? 'text-primary' : ''} | ||
| > | ||
| {day.label} | ||
| </Typography> | ||
| <Typography | ||
| variant="h2" | ||
| className={ | ||
| selectedDay === day.date | ||
| ? 'text-primary text-center m-auto' | ||
| : 'text-center m-auto' | ||
| } | ||
| > | ||
| {day.date.split('-')[2]} | ||
| </Typography> | ||
| </GlassmorphicCard> | ||
| </button> | ||
| ))} | ||
| </div> | ||
|
|
||
| {/* EVENTS LIST */} | ||
| {filteredEvents.length > 0 | ||
| ? ( | ||
| filteredEvents.map(event => ( | ||
| <Event key={event.id} eventData={event} /> | ||
| )) | ||
| ) | ||
| : ( | ||
| <Typography variant="paragraph-lg" className="text-center"> | ||
| No events for this day. | ||
| </Typography> | ||
| )} | ||
| </div> | ||
| </section> | ||
| </Layout> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add type definitions for schedule data.
Defining a custom interface for each event in the data prop will make the code more robust and self-documenting.
🧰 Tools
🪛 Biome (1.9.4)
[error] 52-53: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
| function Event({ eventData }) { | ||
| return ( | ||
| <Dialog> | ||
| <DialogTrigger asChild> | ||
| <GlassmorphicCard className="p-4 flex flex-col sm:flex-row cursor-pointer"> | ||
| <div className="flex-grow-1 space-y-3"> | ||
| <Typography variant="h5">{eventData.title}</Typography> | ||
| <Typography variant="paragraph-xs"> | ||
| 📍 | ||
| {' '} | ||
| { eventData.location} | ||
| </Typography> | ||
| <Typography variant="paragraph-xs"> | ||
| ⌛ | ||
| {' '} | ||
| {formatTime(eventData.start)} | ||
| {' '} | ||
| - | ||
| {' '} | ||
| {formatTime(eventData.end)} | ||
| </Typography> | ||
| <Typography variant="paragraph-xs"> | ||
| {eventData.description} | ||
| </Typography> | ||
| </div> | ||
| <div className="flex flex-wrap p-2 w-fit my-auto gap-2"> | ||
| {eventData.type.map((tag, index) => { | ||
| const matchedOption = options.find(option => option.value === tag) | ||
|
|
||
| return ( | ||
| <Badge | ||
| key={index} | ||
| variant="outline" | ||
| className="px-4 py-1 text-sm font-thin uppercase text-center min-w-[8rem] flex justify-center" | ||
| > | ||
| {matchedOption ? matchedOption.label : tag} | ||
| {' '} | ||
| </Badge> | ||
| ) | ||
| })} | ||
| </div> | ||
| </GlassmorphicCard> | ||
| </DialogTrigger> | ||
|
|
||
| <DialogContent className="sm:max-w-[425px]"> | ||
| <Typography variant="h4">{eventData.title}</Typography> | ||
| <Typography variant="paragraph-sm">{eventData.description}</Typography> | ||
| </DialogContent> | ||
| </Dialog> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Strongly type the eventData prop.
Assigning a named interface or type will improve maintainability, especially for properties like start, end, and type.
| export const loader: LoaderFunction = async ({ request }) => { | ||
| const cookie = request.headers.get('Cookie') | ||
|
|
||
| try { | ||
| const API_URL | ||
| = process.env.NODE_ENV === 'development' | ||
| ? 'http://localhost:8000' | ||
| : 'https://axiom.cuhacking.ca' | ||
|
|
||
| let allChallenges: any[] = [] | ||
| let page = 1 | ||
| let hasNextPage = true | ||
|
|
||
| while (hasNextPage) { | ||
| const req = await fetch(`${API_URL}/api/challenges?page=${page}&limit=100`, { | ||
| credentials: 'include', | ||
| headers: { Cookie: cookie || '' }, | ||
| }) | ||
|
|
||
| if (!req.ok) { | ||
| throw new Error('Error fetching challenges') | ||
| } | ||
|
|
||
| const data = await req.json() | ||
| allChallenges = [...allChallenges, ...data.docs] | ||
| hasNextPage = data.hasNextPage | ||
| page = data.nextPage | ||
| } | ||
|
|
||
| return allChallenges.map(challenge => ({ | ||
| id: challenge.id || Math.random(), | ||
| title: challenge.title || 'Untitled Challenge', | ||
| pathTitle: challenge.pathTitle || '', | ||
| sponsor: { | ||
| symbol: { | ||
| url: challenge.sponsor?.symbol?.url || '', | ||
| alt: challenge.sponsor?.symbol?.alt || 'No Sponsor', | ||
| }, | ||
| }, | ||
| challengeBlock: challenge.challengeBlock || [], | ||
| })) | ||
| } | ||
| catch (error) { | ||
| console.error('Error fetching challenges', error) | ||
| return [] | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add clearer types for the challenge data.
Defining a typed interface for each challenge in allChallenges avoids using any[] and helps enforce data consistency.
Checklist
mainbefore creating this PR.Summary by CodeRabbit
New Features
Style