-
Notifications
You must be signed in to change notification settings - Fork 2
a cop asked me what's my name and dont lie #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
yay conflicts!! |
src/app/page.tsx
Outdated
| const params = useParams() | ||
| useEffect(() => { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
what |
|
shes merging on my conflict |
|
sahana what |
|
mess with a sista* like me |
src/app/page.tsx
Outdated
| { | ||
| sum: "FOSS", | ||
| det: "Your game must be public on GitHub (or another Git server). Include a clear license, I recommend the WTFPL!", | ||
| det: `Your game must be public on ${<a href='https://github.com'>GitHub</a>} (or another Git server). Include a clear license, I recommend the WTFPL!`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: A JSX element is incorrectly placed inside a template string, causing it to render as "[object Object]" in the UI instead of a link.
Severity: MEDIUM | Confidence: High
🔍 Detailed Analysis
A JSX element (<a href='...'>) is embedded directly inside a template string on line 32 of src/app/page.tsx. When this string is created, the JSX object is coerced into the string "[object Object]". This string is later rendered directly in the UI, causing the text "[object Object]" to be displayed to the user instead of a clickable link. This breaks the user experience for the requirements section of the page.
💡 Suggested Fix
Refactor the data structure at src/app/page.tsx:32. Instead of embedding the <a> tag in a string, the det property should be changed to support JSX elements directly, allowing React to render the link correctly. For example, det: <>Your game must be public on <a href='https://github.com'>GitHub</a>...</>.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/app/page.tsx#L32
Potential issue: A JSX element (`<a href='...'>`) is embedded directly inside a template
string on line 32 of `src/app/page.tsx`. When this string is created, the JSX object is
coerced into the string `"[object Object]"`. This string is later rendered directly in
the UI, causing the text `"[object Object]"` to be displayed to the user instead of a
clickable link. This breaks the user experience for the requirements section of the
page.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7462125
|
@copilot review :3 |
|
@hanaeatsplanes I've opened a new pull request, #27, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request implements a significant architectural migration from Next.js Pages Router to App Router, upgrades Next.js from version 15.5.7 to 16.1.1, and introduces a new dashboard feature with authentication. The changes include removing legacy Pages Router files, adding new App Router structure, implementing login functionality, and updating various dependencies.
Key Changes:
- Migration from Pages Router (
src/pages/) to App Router (src/app/) architecture - Implementation of dashboard with authentication (OTP-based login)
- Addition of next-auth for authentication management
- Dependency updates including Next.js, TypeScript types, TailwindCSS, and various tooling packages
Reviewed changes
Copilot reviewed 26 out of 33 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Updated TypeScript configuration for App Router with new jsx transform and Next.js plugin |
| package.json | Updated Next.js to v16.1.1, added next-auth, updated multiple dependencies |
| src/app/layout.tsx | New root layout for App Router with global styles and font configuration |
| src/app/page.tsx | Migrated home page to App Router with embedded Flag, Navbar, and Footer components |
| src/app/login/page.tsx | New login page implementing OTP-based authentication flow |
| src/app/dashboard/layout.tsx | Dashboard layout with authentication check and user provider |
| src/app/dashboard/page.tsx | Dashboard home showing user's projects |
| src/app/dashboard/UserProvider.tsx | Context provider for user data across dashboard |
| src/app/dashboard/userContext.ts | React context definition for user state |
| src/app/dashboard/projects/page.tsx | Placeholder for explore projects page |
| src/app/dashboard/projects/[projectId]/page.tsx | Dynamic route for individual project pages |
| src/app/api/rsvp/route.ts | Migrated RSVP API endpoint to App Router format |
| src/app/api/git/route.ts | Migrated Git commit API endpoint to App Router format |
| src/app/api/dashboard/request_otp/route.ts | New API route for requesting OTP codes |
| src/app/api/dashboard/login/route.ts | New API route for validating OTP and setting session |
| src/components/Navbar.tsx | Updated to use App Router navigation hooks |
| src/components/Footer.tsx | Added "use client" directive for client-side functionality |
| src/components/Button.tsx | Added onClick prop to button interface |
| src/components/dashboard/DashNav.tsx | New dashboard navigation component |
| src/components/dashboard/ProjectCard.tsx | New component for displaying project cards |
| src/pages/* (removed) | Removed all Pages Router files including _app, _document, and API routes |
| export default function RootLayout({ children}: { children: ReactNode }) { | ||
| return ( | ||
| <html lang="en" className="scroll-smooth"> | ||
| <body className={`${outfit.className} bg-(--card)`}> |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid CSS class syntax. The class name should be bg-[var(--card)] instead of bg-(--card). The square brackets are missing, which will cause this CSS class to not work properly.
| <body className={`${outfit.className} bg-(--card)`}> | |
| <body className={`${outfit.className} bg-[var(--card)]`}> |
| const res = await fetch(req); | ||
|
|
||
| console.log(res) | ||
|
|
||
| if (!res.ok) { |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for failed API response. When the fetch to the backend API fails (non-200 response), the function returns an empty array without logging or handling the error. This silently swallows errors and makes debugging difficult. Consider adding error logging or throwing an error to handle this case properly.
| const res = await fetch(req); | |
| console.log(res) | |
| if (!res.ok) { | |
| let res: Response; | |
| try { | |
| res = await fetch(req); | |
| } catch (error) { | |
| console.error("Error fetching projects:", error); | |
| return []; | |
| } | |
| console.log(res) | |
| if (!res.ok) { | |
| console.error("Failed to fetch projects. Non-OK response received:", { | |
| status: res.status, | |
| statusText: res.statusText, | |
| }); |
| } catch { | ||
| // ignore |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling for the catch block. The try-catch at line 21 catches errors but doesn't handle them - it just has an empty comment. This means parsing errors will be silently ignored. Consider logging the error or setting a default value with proper error handling.
| } catch { | |
| // ignore | |
| } catch (error) { | |
| console.error("Failed to parse validate_otp response JSON:", error); | |
| cookie = null; |
| return ( | ||
| <a href={!disabled ? href : undefined} > | ||
| <div className={"p-3 rounded-lg bg-red-400 text-white text-center hover:bg-red-500 transition-all inline-flex items-center justify-center text-xl md:text-2xl" + (disabled ? " opacity-50 cursor-not-allowed hover:bg-red-400" : " hover:cursor-pointer")}> | ||
| <Icon className="mr-2" /> | ||
| {label} | ||
| </div> | ||
| </a> | ||
| ) |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Link href uses an undefined attribute pattern. The href is set to undefined when disabled is true, which could cause issues. Consider using a conditional to either render a div or prevent the link behavior more explicitly, such as with onClick preventDefault or using a button element when disabled.
| return ( | |
| <a href={!disabled ? href : undefined} > | |
| <div className={"p-3 rounded-lg bg-red-400 text-white text-center hover:bg-red-500 transition-all inline-flex items-center justify-center text-xl md:text-2xl" + (disabled ? " opacity-50 cursor-not-allowed hover:bg-red-400" : " hover:cursor-pointer")}> | |
| <Icon className="mr-2" /> | |
| {label} | |
| </div> | |
| </a> | |
| ) | |
| const baseClasses = "p-3 rounded-lg bg-red-400 text-white text-center hover:bg-red-500 transition-all inline-flex items-center justify-center text-xl md:text-2xl"; | |
| const disabledClasses = " opacity-50 cursor-not-allowed hover:bg-red-400"; | |
| const enabledClasses = " hover:cursor-pointer"; | |
| const className = baseClasses + (disabled ? disabledClasses : enabledClasses); | |
| if (disabled) { | |
| return ( | |
| <div className={className}> | |
| <Icon className="mr-2" /> | |
| {label} | |
| </div> | |
| ); | |
| } | |
| return ( | |
| <a href={href}> | |
| <div className={className}> | |
| <Icon className="mr-2" /> | |
| {label} | |
| </div> | |
| </a> | |
| ); |
| import Meta from "@/components/Meta"; | ||
| import { Outfit } from "next/font/google"; | ||
|
|
||
| const outfit = Outfit({ | ||
| subsets: ["latin"], | ||
| }); | ||
|
|
||
| export default function RootLayout({ children}: { children: ReactNode }) { | ||
| return ( | ||
| <html lang="en" className="scroll-smooth"> | ||
| <body className={`${outfit.className} bg-(--card)`}> | ||
| <Meta /> |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Meta component should not be placed inside the body tag in App Router. In Next.js App Router, metadata should be defined using the metadata export or Metadata API, not as a component inside the body. Meta components with head elements will not work correctly when placed in the body.
| import Meta from "@/components/Meta"; | |
| import { Outfit } from "next/font/google"; | |
| const outfit = Outfit({ | |
| subsets: ["latin"], | |
| }); | |
| export default function RootLayout({ children}: { children: ReactNode }) { | |
| return ( | |
| <html lang="en" className="scroll-smooth"> | |
| <body className={`${outfit.className} bg-(--card)`}> | |
| <Meta /> | |
| import { Outfit } from "next/font/google"; | |
| const outfit = Outfit({ | |
| subsets: ["latin"], | |
| subsets: ["latin"], | |
| }); | |
| export default function RootLayout({ children}: { children: ReactNode }) { | |
| return ( | |
| <html lang="en" className="scroll-smooth"> | |
| <body className={`${outfit.className} bg-(--card)`}> |
| import { createContext } from "react"; | ||
| import { User } from "@/app/dashboard/layout"; | ||
|
|
||
| export const UserContext = createContext<User>(null); No newline at end of file |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value for createContext should not be null directly when the type is User | null. TypeScript will infer the context type as User only. Consider using createContext<User | null>(null) with explicit type annotation, or provide a more appropriate default value. This could lead to type safety issues when consuming the context.
| export const UserContext = createContext<User>(null); | |
| export const UserContext = createContext<User | null>(null); |
| httpOnly: true, | ||
| secure: true, | ||
| path: '/', | ||
| sameSite: 'none', |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sameSite cookie attribute is set to 'none' which requires the secure flag. While secure is set to true, this configuration means cookies will be sent in cross-site requests. This could be a security risk if not intentional. If the frontend and API are on the same domain, consider using 'lax' or 'strict' instead for better security.
| sameSite: 'none', | |
| sameSite: 'lax', |
| if (!/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/.test(email)) { | ||
| setMessage({ | ||
| className: "text-red-500 mb-4 text-center", | ||
| message: "Please enter a valid email address." | ||
| }) | ||
| setEmailButtonAllow(true) | ||
| return | ||
| } |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Client-side email validation regex can be bypassed. The regex pattern at line 25 provides basic validation but doesn't cover all valid email formats and can be easily bypassed on the client side. Since this is a security-sensitive operation (authentication), ensure the backend also validates the email format properly. Client-side validation should only be for UX improvement.
| console.log(sessionId); | ||
|
|
||
| if (!sessionId) { | ||
| return []; | ||
| } | ||
|
|
||
| const req = new Request( | ||
| `${process.env.NEXT_PUBLIC_API_URL}/api/v1/projects`, | ||
| { | ||
| method: "GET", | ||
| headers: { | ||
| Cookie: `sessionId=${sessionId}`, | ||
| }, | ||
| cache: "no-store", | ||
| } | ||
| ) | ||
|
|
||
| console.log(req) | ||
|
|
||
| const res = await fetch(req); | ||
|
|
||
| console.log(res) |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Console.log statements should be removed from production code. These debug logs at lines 19, 36, and 40 are left in the code and will clutter the console output in production. Remove these or use a proper logging library with environment-based log levels.
| <div className="rounded-lg overflow-hidden shadow-md hover:shadow-lg transition-shadow bg-gray-50"> | ||
| <Image | ||
| src={imageSrc} | ||
| alt={project.project_name} |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing alt text content for fallback image. The alt attribute uses project.project_name which could be empty or undefined, resulting in an empty alt attribute. This impacts accessibility. Ensure there's always meaningful alt text, perhaps with a fallback like "Project preview image".
| alt={project.project_name} | |
| alt={project.project_name || "Project preview image"} |
395beb9 to
5936b2f
Compare
… project dashboard
|
it deloyed 🥹 |
and im asking like officer why, why you wanna mess with a brother like me when you know im living legit