Enhance Interview Setup Page with Multi-Step Form and Resume Upload #101
Conversation
|
Someone is attempting to deploy a commit to the coderuzumaki's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughClient adds react-hot-toast and refactors App layout to include Toaster and Outlet; Login/SignUp and CreateInterview use Firebase ID tokens and toasts; several components receive UX/defensive updates. Server adds Firebase Admin init, token-verifying middleware, mounts auth/interview/report/contact routes, and moves nodemon to devDependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant C as Client (Login/SignUp)
participant FB as Firebase Client SDK
participant API as Backend (/api/auth)
participant ADM as Firebase Admin (Server)
U->>C: Submit credentials or click OAuth
C->>FB: signInWithEmailAndPassword / signInWithPopup
FB-->>C: Credential (user)
C->>FB: user.getIdToken()
FB-->>C: ID Token (JWT)
Note over C: Add Authorization: Bearer <ID Token>
C->>API: POST /api/auth/login
API->>ADM: admin.auth().verifyIdToken(ID Token)
alt Valid token
ADM-->>API: Decoded token
API-->>C: 200 OK (user/session)
C-->>U: Show success toast and navigate
else Invalid/expired
ADM-->>API: Error
API-->>C: 401 Unauthorized
C-->>U: Show error toast
end
sequenceDiagram
autonumber
participant Client as Frontend
participant API as Backend
participant MW as /api/protected middleware
participant ADM as Firebase Admin
Client->>API: Request to protected endpoint (Bearer token)
API->>MW: Invoke middleware
MW->>ADM: verifyIdToken(token)
alt Verified
MW-->>API: req.user populated
API-->>Client: 200 OK (protected data)
else Missing/Invalid
MW-->>Client: 401 Unauthorized
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@pushh-k @CoderUzumaki Please review this PR whenever you get time 🙌
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/src/pages/CreateInterview.jsx (2)
82-94: Enforce PDF-only and 5MB limit with clear user feedback.Currently only MIME is checked; size limit isn’t enforced and non‑PDF selections silently do nothing.
Apply:
+ const MAX_FILE_MB = 5; + const MAX_FILE_BYTES = MAX_FILE_MB * 1024 * 1024; + const handleDrop = (e) => { e.preventDefault(); e.stopPropagation(); setDrag(false); if (e.dataTransfer.files && e.dataTransfer.files[0]) { const file = e.dataTransfer.files[0]; - if (file.type === "application/pdf") { - handleInputChange("resume", file); - } + const isPdf = + file.type === "application/pdf" || file.name.toLowerCase().endsWith(".pdf"); + if (!isPdf) { + showToast("Only PDF files are allowed.", "error"); + return; + } + if (file.size > MAX_FILE_BYTES) { + showToast(`File too large. Max ${MAX_FILE_MB} MB.`, "error"); + return; + } + handleInputChange("resume", file); } }; const handleFileSelect = (e) => { if (e.target.files && e.target.files[0]) { const file = e.target.files[0]; - if (file.type === "application/pdf") { - handleInputChange("resume", file); - } + const isPdf = + file.type === "application/pdf" || file.name.toLowerCase().endsWith(".pdf"); + if (!isPdf) { + showToast("Only PDF files are allowed.", "error"); + return; + } + if (file.size > MAX_FILE_BYTES) { + showToast(`File too large. Max ${MAX_FILE_MB} MB.`, "error"); + return; + } + handleInputChange("resume", file); } }; @@ - accept=".pdf" + accept="application/pdf,.pdf"Also applies to: 95-103, 510-516
27-29: Avoid logging sensitive data (ID tokens, form content, user).These logs can leak credentials/PII to the console and monitoring tools.
Apply:
- useEffect(() => { - console.log("--------\nCurrent user from context:", user, "\n--------"); - }, [user]); + useEffect(() => {}, [user]); // consider removing entirely @@ - const token = await user.getIdToken(); - console.log("-------\nToken being sent to backend:", token, "\n-------"); + const token = await user.getIdToken(); @@ - console.log("-------\nForm Data: \n", [...data.entries()], "\n-------"); + // avoid logging form data (may contain PII) @@ - console.log("-------\nResponse from interview setup:", res.data, "\n-------"); + // successAlso applies to: 127-129, 135-136, 148-149
🧹 Nitpick comments (23)
client/src/pages/CreateInterview.jsx (4)
480-484: Conflicting border classes override the red required state.border-red-500 is overridden by later border-* classes. Make red the default when not dragging.
Apply:
- className={`relative border-2 border-dashed border-red-500 rounded-lg p-8 text-center transition-all ${ - drag - ? "border-blue-500 bg-blue-50" - : "border-gray-300 hover:border-gray-400" - }`} + className={`relative border-2 border-dashed rounded-lg p-8 text-center transition-all ${ + drag + ? "border-blue-500 bg-blue-50" + : "border-red-500 hover:border-red-400" + }`}
104-110: Remove stray statement.The standalone
1;is a no-op and looks accidental.const removeFile = () => { handleInputChange("resume", null); - 1; if (fileInputRef.current) { fileInputRef.current.value = ""; } };
131-136: FormData append skips falsy values (e.g., 0).Use null/undefined checks to avoid dropping valid falsy inputs.
- Object.keys(formData).forEach((key) => { - if (formData[key]) { - data.append(key, formData[key]); - } - }); + Object.keys(formData).forEach((key) => { + const val = formData[key]; + if (val !== undefined && val !== null) data.append(key, val); + });
13-24: Standardize to react-hot-toast for consistency with the app.This page still uses a local Toast component. Align with app-wide Toaster.
Apply:
-import Toast from "../components/Toast"; +import { toast } from "react-hot-toast"; @@ - const [toast, setToast] = useState({ - show: false, - message: "", - type: "success", - }); + // using global Toaster; no local toast state @@ - const showToast = (message, type) => { - setToast({ show: true, message, type }); - }; + const showToast = (message, type) => + type === "error" ? toast.error(message) : toast.success(message); @@ - const hideToast = () => { - setToast((prev) => ({ ...prev, show: false })); - }; + const hideToast = () => {}; @@ - {toast.show && ( - <Toast - message={toast.message} - type={toast.type} - onClose={hideToast} - /> - )} + {/* Toaster is mounted at app root */} @@ - showToast("Something went wrong", "error"); + showToast(err?.response?.data?.message || "Something went wrong", "error"); @@ - Start Interview + Start InterviewAlso applies to: 173-181, 150-160, 583-589
client/src/components/ContactForm.jsx (1)
32-37: Use app-wide toast instead of alert for consistent UX.Leverage react-hot-toast already added to the app.
Apply:
+import { toast } from "react-hot-toast"; @@ - alert("Message sent successfully ✅"); + toast.success("Message sent successfully ✅"); @@ - } catch (error) { + } catch (error) { console.error("Error submitting contact form:", error); - alert("❌ There was an error submitting your message."); + toast.error(error?.response?.data?.message || "❌ There was an error submitting your message.");Also applies to: 39-50
client/src/components/FAQ.jsx (1)
31-33: A11y improvements look good (aria-controls + rotating chevron).Clean, accessible accordion behavior. Consider adding aria-hidden on the panel when collapsed as a minor enhancement.
- <div - id={contentId} - className={`overflow-hidden transition-all duration-300 ease-in-out ${ - isOpen ? "max-h-96 opacity-100" : "max-h-0 opacity-0" - }`} - > + <div + id={contentId} + aria-hidden={!isOpen} + className={`overflow-hidden transition-all duration-300 ease-in-out ${ + isOpen ? "max-h-96 opacity-100" : "max-h-0 opacity-0" + }`} + >Also applies to: 39-46, 56-61, 64-66, 80-87
client/src/components/InterviewList.jsx (2)
31-37: Clamp current page when data shrinks to avoid empty views.Switching datasets can leave currentPage > totalPages.
+import { useEffect } from "react"; @@ const currentInterviews = safeInterviews.slice(startIndex, endIndex); + + useEffect(() => { + if (totalPages > 0 && currentPage > totalPages) { + setCurrentPage(totalPages); + } + }, [totalPages]); // eslint-disable-line react-hooks/exhaustive-deps
38-41: Guard page navigation.Prevent out-of-range page indices.
- const goToPage = (page) => setCurrentPage(page); + const goToPage = (page) => { + if (page >= 1 && page <= totalPages) setCurrentPage(page); + };Also applies to: 64-96
client/src/pages/SignUp.jsx (2)
292-303: Add accessible label to the password visibility toggle.Improves screen-reader support.
- <button + <button type="button" onClick={() => setShowPassword(!showPassword)} - className="absolute inset-y-0 right-0 pr-3 flex items-center text-gray-400 hover:text-gray-600" + aria-label={showPassword ? "Hide password" : "Show password"} + className="absolute inset-y-0 right-0 pr-3 flex items-center text-gray-400 hover:text-gray-600" >
351-356: Nit: Brand casing.Use “GitHub” for consistency.
- Github + GitHubclient/src/pages/Login.jsx (4)
6-10: Confirm source of auth helpers or adjust imports.Ensure
signInWithPopupandsignInWithEmailAndPasswordare re-exported from../firebase. If not, import them fromfirebase/auth.-import { - auth, - googleProvider, - signInWithPopup, - signInWithEmailAndPassword, -} from "../firebase"; +import { auth, googleProvider } from "../firebase"; +import { signInWithPopup, signInWithEmailAndPassword } from "firebase/auth";
71-83: Avoid leaking raw error messages to end users.
err.messagecan expose internal details. Prefer a friendly generic message and log the raw error to console.- } catch (err) { - toast.error(err.message || "❌ Login failed", { + } catch (err) { + console.error("Email login failed:", err); + toast.error("❌ Login failed. Please try again.", { style: { borderRadius: "10px", background: "#f97316", // orange color: "#fff", fontWeight: "600", }, iconTheme: { primary: "#fff", secondary: "#f97316", }, }); }
199-205: Add accessible label to password toggle.Screen readers need an aria-label; also reflect pressed state.
- <button + <button type="button" onClick={() => setShowPassword(!showPassword)} - className="absolute inset-y-0 right-0 pr-3 flex items-center text-gray-400 hover:text-gray-600" + aria-label={showPassword ? "Hide password" : "Show password"} + aria-pressed={showPassword} + className="absolute inset-y-0 right-0 pr-3 flex items-center text-gray-400 hover:text-gray-600" >
161-161: Tailwind classmin-w-screenis not standard.Use
w-screenor remove if unnecessary.-<div className="min-h-screen bg-gray-50 min-w-screen"> +<div className="min-h-screen bg-gray-50">client/src/components/Footer.jsx (1)
73-77: Replace placeholder links or gate behind a feature flag.Links pointing to
#should either route to real pages (privacy, terms, help) or be removed/disabled until available.Also applies to: 87-89
server/index.js (3)
28-39: Unify token field naming with routes and extract reusable middleware.Routes reference
req.firebaseUser(see authRoutes), but this middleware setsreq.user. Standardize on one (recommendreq.firebaseUser) and export this as a shared middleware to apply on protected routers.-app.use("/api/protected", async (req, res, next) => { +app.use("/api/protected", async (req, res, next) => { const token = req.headers.authorization?.split(" ")[1]; if (!token) return res.status(401).json({ error: "Unauthorized" }); try { const decoded = await admin.auth().verifyIdToken(token); - req.user = decoded; + req.firebaseUser = decoded; next(); } catch (err) { res.status(401).json({ error: "Invalid token" }); } });Optionally move this into
middleware/firebaseAuthMiddleware.jsand reuse across/api/interviewand/api/report.
21-26: Make CORS origin configurable and allow Authorization header.Hard-coding the origin hinders local dev. Prefer an env var fallback and explicitly allow
Authorization.-app.use(cors({ - origin: 'https://prepedgeai.vercel.app', - credentials: true, -})); +app.use(cors({ + origin: process.env.CORS_ORIGIN?.split(",") ?? ["http://localhost:5173", "https://prepedgeai.vercel.app"], + credentials: true, + allowedHeaders: ["Content-Type", "Authorization"], + methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"], +}));
42-46: DB connect logging is fine; consider failing fast on fatal errors.If
connectDB()alreadyprocess.exit(1)on error, this is OK. Otherwise, stop server start when DB is unavailable.client/src/pages/Dashboard.jsx (4)
20-25: Fetch interviews and reports in parallel to reduce latency.Use
Promise.allto halve network wait time.- // ---- Fetch Interviews ---- - const response = await axios.get( - `${import.meta.env.VITE_API_URL}/api/interview`, - { headers: { Authorization: `Bearer ${token}` } } - ); + const [response, reportResponse] = await Promise.all([ + axios.get(`${import.meta.env.VITE_API_URL}/api/interview`, { + headers: { Authorization: `Bearer ${token}` }, + }), + axios.get(`${import.meta.env.VITE_API_URL}/api/report`, { + headers: { Authorization: `Bearer ${token}` }, + }), + ]); - - // ---- Fetch Reports ---- - const reportResponse = await axios.get( - `${import.meta.env.VITE_API_URL}/api/report`, - { headers: { Authorization: `Bearer ${token}` } } - );Also applies to: 33-39
46-51: Stabilize date formatting for charts.
toLocaleDateString()varies by environment. Provide a locale or ISO format for consistent rendering.- date: new Date(report.createdAt).toLocaleDateString(), + date: new Date(report.createdAt).toLocaleDateString("en-US"),
54-56: Coerce scores to numbers before Math.max.Avoid accidental string comparison/coercion.
- const bestScore = Math.max(...reports.map((i) => i.finalScore || 0), 0); + const bestScore = Math.max(...reports.map((i) => Number(i.finalScore) || 0), 0);
57-66: Skip undefined/empty roles in aggregation.Prevents
undefinedfrom becoming the “most common role”.- const roleCounts = interviewsData - .map((i) => i.role) + const roleCounts = interviewsData + .map((i) => i.role) + .filter((r) => r && typeof r === "string") .reduce((acc, role) => { acc[role] = (acc[role] || 0) + 1; return acc; }, {});client/src/App.jsx (1)
60-67: Optional: move keyframes to a CSS file.Inline
<style>is fine, but extracting to a CSS module or global stylesheet reduces inline CSS and avoids re-injecting on re-mounts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
client/package-lock.jsonis excluded by!**/package-lock.jsonserver/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
client/package.json(1 hunks)client/src/App.jsx(1 hunks)client/src/components/ContactForm.jsx(1 hunks)client/src/components/FAQ.jsx(2 hunks)client/src/components/Footer.jsx(3 hunks)client/src/components/InterviewList.jsx(1 hunks)client/src/pages/CreateInterview.jsx(1 hunks)client/src/pages/Dashboard.jsx(1 hunks)client/src/pages/Login.jsx(1 hunks)client/src/pages/SignUp.jsx(1 hunks)server/firebase.js(1 hunks)server/index.js(2 hunks)server/package.json(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
client/src/App.jsx (1)
client/src/components/ScrollToTop.jsx (1)
ScrollToTop(4-15)
client/src/pages/Dashboard.jsx (6)
server/routes/interviewRoutes.js (4)
interviews(231-231)user(48-48)user(228-230)report(165-165)client/src/context/AuthContext.jsx (3)
useAuth(7-7)useAuth(7-7)user(10-10)server/routes/reportRoutes.js (3)
user(26-26)reports(27-29)report(14-14)client/src/components/ScoreTrendChart.jsx (1)
ScoreTrendChart(28-107)client/src/components/SummaryCard.jsx (1)
SummaryCard(1-8)client/src/components/InterviewList.jsx (1)
InterviewList(27-99)
client/src/pages/Login.jsx (4)
client/src/pages/SignUp.jsx (5)
githubProvider(17-17)useAuth(18-18)navigate(31-31)handleGoogleLogin(154-176)handleGithub(178-200)client/src/context/AuthContext.jsx (2)
useAuth(7-7)useAuth(7-7)client/src/firebase.js (2)
auth(16-16)googleProvider(17-17)server/middleware/firebaseAuthMiddleware.js (1)
idToken(4-4)
client/src/components/InterviewList.jsx (2)
client/src/pages/Dashboard.jsx (1)
interviews(9-9)server/routes/interviewRoutes.js (4)
interviews(231-231)interview(118-131)interview(145-145)interview(151-151)
server/index.js (3)
client/src/firebase.js (1)
app(15-15)server/routes/authRoutes.js (2)
req(14-14)req(36-36)server/config/db.js (1)
connectDB(3-12)
client/src/pages/SignUp.jsx (3)
client/src/pages/Login.jsx (5)
githubProvider(16-16)useAuth(17-17)navigate(23-23)handleGoogleLogin(87-123)handleGithub(126-158)client/src/context/AuthContext.jsx (2)
useAuth(7-7)useAuth(7-7)client/src/firebase.js (2)
auth(16-16)googleProvider(17-17)
🔇 Additional comments (10)
client/package.json (1)
23-23: Dependency addition looks good; ensure Toaster is mounted once app-wide.No issues with adding react-hot-toast. Just verify there’s a single at the app root to prevent duplicate stacks.
server/package.json (1)
11-13: Duplicate "start" script key; JSON overrides earlier value.This can confuse tooling and teammates. Keep a single "start" and one "dev".
Apply:
"scripts": { "test": "echo \"Error: no test specified\" && exit 1", "postinstall": "patch-package", - "start": "node index.js", - "dev": "nodemon index.js", - "start": "node index.js" + "start": "node index.js", + "dev": "nodemon index.js" },Likely an incorrect or invalid review comment.
client/src/pages/SignUp.jsx (1)
14-14: Incorrect import; toast is a named export.react-hot-toast exports toast as a named export. Default import will be undefined at runtime.
-import toast from "react-hot-toast"; // ✅ Import hot-toast +import { toast } from "react-hot-toast"; // ✅ Named importLikely an incorrect or invalid review comment.
client/src/pages/Login.jsx (2)
69-70: Verify the post-login navigation targets.Email/Google route to “/” while GitHub routes to “/dashboard”. Confirm this is intentional; otherwise, standardize for consistency.
Also applies to: 147-148
13-13: Fix incorrect toast import (breaks all notifications).react-hot-toast exports
toastas a named export. The default import will beundefinedat runtime.-import toast from "react-hot-toast"; // ✅ import toast +import { toast } from "react-hot-toast";Likely an incorrect or invalid review comment.
client/src/components/Footer.jsx (2)
58-66: Quick Links look good.Structure and responsive classes are clean and consistent.
21-29: External links: solid work on security attributes.
target="_blank"paired withrel="noopener noreferrer"is correct. Icons havearia-labels. 👍Also applies to: 31-38, 46-54
server/index.js (1)
12-16: Good: centralized Admin initialization + early dotenv.Importing the pre-initialized Admin instance and loading env early is correct.
client/src/pages/Dashboard.jsx (1)
90-103: UI composition looks clean and resilient.Nice separation of concerns with reusable components and safe array handling for lists.
client/src/App.jsx (1)
19-59: Global Toaster configuration is solid.Centralizing toast setup with consistent theming is a good call.
| useEffect(() => { | ||
| const fetchData = async () => { | ||
| try { | ||
| const token = await user.getIdToken(); | ||
|
|
||
| // ---- Fetch Interviews ---- |
There was a problem hiding this comment.
Guard against null user to prevent runtime errors.
user.getIdToken() will throw when user is null. Early return until auth is ready; narrow the effect dependency to user?.uid.
- useEffect(() => {
- const fetchData = async () => {
+ useEffect(() => {
+ if (!user) return;
+ const fetchData = async () => {
try {
- const token = await user.getIdToken();
+ const token = await user.getIdToken();
...
} catch (error) {
console.error("Failed to fetch interviews:", error);
}
};
-
- fetchData();
- }, [user]);
+ fetchData();
+ }, [user?.uid]);Also applies to: 78-79
🤖 Prompt for AI Agents
In client/src/pages/Dashboard.jsx around lines 14 to 19 (and also apply the same
fix at lines 78-79), the effect calls user.getIdToken() without guarding for a
null/undefined user; update the effect to early-return when user is falsy (e.g.,
if (!user) return;) and change the effect dependency to user?.uid so it only
runs when an authenticated user exists; ensure the other effect at lines 78-79
receives the same null check and narrowed dependency.
| const serviceAccount = JSON.parse(process.env.FIREBASE_SERVICE_ACCOUNT_KEY); | ||
|
|
||
| admin.initializeApp({ | ||
| credential: admin.credential.cert(serviceAccount), | ||
| }); |
There was a problem hiding this comment.
Hard crash if FIREBASE_SERVICE_ACCOUNT_KEY is missing/invalid; also handle escaped newlines and double init.
Guard parsing and initialize once to improve DX and reliability.
Apply:
-const serviceAccount = JSON.parse(process.env.FIREBASE_SERVICE_ACCOUNT_KEY);
-
-admin.initializeApp({
- credential: admin.credential.cert(serviceAccount),
-});
+const raw = process.env.FIREBASE_SERVICE_ACCOUNT_KEY;
+if (!raw) {
+ throw new Error("FIREBASE_SERVICE_ACCOUNT_KEY is not set");
+}
+let serviceAccount;
+try {
+ serviceAccount = JSON.parse(raw);
+} catch (e) {
+ throw new Error("FIREBASE_SERVICE_ACCOUNT_KEY is not valid JSON");
+}
+if (serviceAccount.private_key) {
+ serviceAccount.private_key = serviceAccount.private_key.replace(/\\n/g, "\n");
+}
+if (!admin.apps?.length) {
+ admin.initializeApp({
+ credential: admin.credential.cert(serviceAccount),
+ });
+}📝 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 serviceAccount = JSON.parse(process.env.FIREBASE_SERVICE_ACCOUNT_KEY); | |
| admin.initializeApp({ | |
| credential: admin.credential.cert(serviceAccount), | |
| }); | |
| const raw = process.env.FIREBASE_SERVICE_ACCOUNT_KEY; | |
| if (!raw) { | |
| throw new Error("FIREBASE_SERVICE_ACCOUNT_KEY is not set"); | |
| } | |
| let serviceAccount; | |
| try { | |
| serviceAccount = JSON.parse(raw); | |
| } catch (e) { | |
| throw new Error("FIREBASE_SERVICE_ACCOUNT_KEY is not valid JSON"); | |
| } | |
| if (serviceAccount.private_key) { | |
| serviceAccount.private_key = serviceAccount.private_key.replace(/\\n/g, "\n"); | |
| } | |
| if (!admin.apps?.length) { | |
| admin.initializeApp({ | |
| credential: admin.credential.cert(serviceAccount), | |
| }); | |
| } |
🤖 Prompt for AI Agents
In server/firebase.js around lines 6 to 10, the code unconditionally JSON.parses
FIREBASE_SERVICE_ACCOUNT_KEY (which crashes if missing/invalid or contains
escaped newlines) and calls admin.initializeApp without guarding double
initialization; wrap parsing in a try/catch, verify
process.env.FIREBASE_SERVICE_ACCOUNT_KEY exists, sanitize escaped newlines by
replacing '\\n' with '\n' before JSON.parse, handle JSON.parse errors by logging
and returning/throwing a controlled error, and only call admin.initializeApp
when admin.apps.length === 0 (or skip initialization if already initialized) so
the app is initialized exactly once.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
client/src/pages/CreateInterview.jsx (8)
47-55: Guard Next/Back, validate step 1, and use functional state updates.Prevents rapid double‑clicks and stale state; blocks moving past step 1 without required fields.
- const handleNext = () => { - if (currentStep < totalSteps) { - setIsTransitioning(true); - setTimeout(() => { - setCurrentStep(currentStep + 1); - setIsTransitioning(false); - }, 150); - } - }; + const handleNext = () => { + if (currentStep < totalSteps && !isTransitioning) { + if ( + currentStep === 1 && + (!formData.interviewName.trim() || !formData.role.trim()) + ) { + showToast("Please fill Interview Name and Role to continue.", "error"); + return; + } + setIsTransitioning(true); + setTimeout(() => { + setCurrentStep((prev) => Math.min(prev + 1, totalSteps)); + setIsTransitioning(false); + }, 150); + } + }; - const handleBack = () => { - if (currentStep > 1) { - setIsTransitioning(true); - setTimeout(() => { - setCurrentStep(currentStep - 1); - setIsTransitioning(false); - }, 150); - } - }; + const handleBack = () => { + if (currentStep > 1 && !isTransitioning) { + setIsTransitioning(true); + setTimeout(() => { + setCurrentStep((prev) => Math.max(prev - 1, 1)); + setIsTransitioning(false); + }, 150); + } + };Also applies to: 57-65
27-28: Harden file validation (+a11y for upload control).
- MIME-only checks are brittle; add .pdf extension fallback.
- Extend accept attr to include .pdf.
- Make the upload trigger a label for accessibility; add aria-label to the remove button.
const MAX_FILE_SIZE_MB = 5; + const PDF_MIME_TYPES = new Set(["application/pdf", "application/x-pdf"]);- if (file.type !== "application/pdf") { + const isPDF = + PDF_MIME_TYPES.has(file.type) || + file.name?.toLowerCase().endsWith(".pdf"); + if (!isPDF) { showToast("Only PDF files are allowed.", "error"); return; }- if (file.type !== "application/pdf") { - showToast("Only PDF files are allowed", "error"); + const isPDF = + PDF_MIME_TYPES.has(file.type) || + file.name?.toLowerCase().endsWith(".pdf"); + if (!isPDF) { + showToast("Only PDF files are allowed.", "error"); return; }- accept="application/pdf" + accept="application/pdf,.pdf" + id="resumeUpload"- <button type="button" onClick={removeFile}><FaTimes /></button> + <button type="button" onClick={removeFile} aria-label="Remove file"><FaTimes /></button>- <p onClick={() => fileInputRef.current.click()}> - <FaUpload className="inline mr-2" /> Drag & drop your resume here or click to upload - </p> + <label htmlFor="resumeUpload" className="cursor-pointer"> + <FaUpload className="inline mr-2" /> Drag & drop your resume here or click to upload + </label>Also applies to: 81-104, 106-125, 272-279, 281-283, 285-288
296-312: Disable navigation and submit during transitions/loading.Prevents double actions and accidental submits while loading.
- <button + <button type="button" onClick={handleBack} - className="px-4 py-2 bg-gray-300 rounded hover:bg-gray-400" + disabled={isTransitioning || loading} + className="px-4 py-2 bg-gray-300 rounded hover:bg-gray-400 disabled:opacity-50 disabled:cursor-not-allowed" >- <button + <button type="button" onClick={handleNext} - className="ml-auto px-4 py-2 bg-blue-500 text-white rounded hover:bg-blue-600" + disabled={isTransitioning || loading} + className="ml-auto px-4 py-2 bg-blue-500 text-white rounded hover:bg-blue-600 disabled:opacity-50 disabled:cursor-not-allowed" >- <button + <button type="submit" - className="ml-auto px-4 py-2 bg-green-500 text-white rounded hover:bg-green-600" + disabled={isTransitioning || loading || !formData.resume} + className="ml-auto px-4 py-2 bg-green-500 text-white rounded hover:bg-green-600 disabled:opacity-50 disabled:cursor-not-allowed" >Also applies to: 314-320
31-42: Trim unused/unclear fields and verify payload contract.
focusAtisn’t used anywhere;numOfQuestionsandinterviewTypearen’t user-editable here. Remove dead fields or confirm the backend expects them with these defaults.Apply this diff to drop the unused field:
resume: null, - focusAt: "",
183-186: Unify to one toast system (react-hot-toast).The app reportedly includes a global Toaster. Keeping a second custom Toast adds duplication and inconsistent UX. Replace local toast state/JSX with
react-hot-toastcalls inshowToast, and remove the custom Toast import/render.Suggested minimal changes:
- const [toast, setToast] = useState({ - show: false, - message: "", - type: "success", - }); ... - const showToast = (message, type) => { - setToast({ show: true, message, type }); - }; - - const hideToast = () => { - setToast((prev) => ({ ...prev, show: false })); - }; + const showToast = (message, type) => { + if (type === "success") window.toast?.success?.(message) || null; + else window.toast?.error?.(message) || null; + };- {toast.show && <Toast message={toast.message} type={toast.type} onClose={hideToast} />}Add (outside this hunk):
import { toast as toastLib } from "react-hot-toast"; window.toast = toastLib;or calltoastLib.success/errordirectly where needed.Also applies to: 20-24, 132-139
206-235: A11y: add labels for inputs (placeholders are not labels).Associate labels with inputs for screen readers and better click targets. Also add
idattributes andaria-requiredwhere applicable.
162-167: Server-side enforcement for uploads.Client checks are advisory. Ensure the backend for
/api/interview/setupenforces:
- Max size (5 MB) and content-type + PDF magic bytes (not just extension),
- Authentication/authorization for the user,
- Virus/malware scanning (as policy dictates),
- Safe storage (path sanitization, randomized names).
16-16: Consider separating auth loading from submit loading.Using
AuthContext.loadingto cover submission blocks the whole app and can show “Setting up your interview...” during initial auth. Prefer a localisSubmittingfor this screen while keeping auth loading for auth initialization only.Also applies to: 180-181
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/src/pages/CreateInterview.jsx(1 hunks)server/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- server/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
client/src/pages/CreateInterview.jsx (3)
client/src/context/AuthContext.jsx (4)
useAuth(7-7)useAuth(7-7)user(10-10)loading(12-12)client/src/components/LoadingScreen.jsx (1)
LoadingScreen(1-32)client/src/components/Toast.jsx (1)
Toast(6-55)
| setLoading(true); | ||
| const token = await user.getIdToken(); | ||
|
|
||
| const data = new FormData(); | ||
| Object.keys(formData).forEach((key) => { | ||
| if (formData[key]) data.append(key, formData[key]); | ||
| }); | ||
|
|
||
| try { | ||
| const res = await axios.post( | ||
| `${import.meta.env.VITE_API_URL}/api/interview/setup`, | ||
| data, | ||
| { headers: { Authorization: `Bearer ${token}` } } | ||
| ); | ||
|
|
||
| showToast("Interview setup complete!", "success"); | ||
| const interviewId = res.data.interview._id; | ||
| navigate(`/interview/${interviewId}`); | ||
| } catch (err) { | ||
| console.error("Error in interview setup:", err.response?.data || err); | ||
| showToast("Something went wrong", "error"); | ||
| } finally { | ||
| setLoading(false); | ||
| } |
There was a problem hiding this comment.
Critical: getIdToken is outside try/finally — can freeze the UI on failure.
If user.getIdToken() throws, finally never runs and loading stays true. Also, the current FormData append skips falsy values (e.g., 0). Move token fetch and FormData build inside the try, and only skip null/undefined. Surface server message when available.
Apply this diff:
- setLoading(true);
- const token = await user.getIdToken();
-
- const data = new FormData();
- Object.keys(formData).forEach((key) => {
- if (formData[key]) data.append(key, formData[key]);
- });
-
- try {
- const res = await axios.post(
- `${import.meta.env.VITE_API_URL}/api/interview/setup`,
- data,
- { headers: { Authorization: `Bearer ${token}` } }
- );
-
- showToast("Interview setup complete!", "success");
- const interviewId = res.data.interview._id;
- navigate(`/interview/${interviewId}`);
- } catch (err) {
- console.error("Error in interview setup:", err.response?.data || err);
- showToast("Something went wrong", "error");
- } finally {
- setLoading(false);
- }
+ setLoading(true);
+ try {
+ const token = await user.getIdToken();
+
+ const data = new FormData();
+ Object.entries(formData).forEach(([key, value]) => {
+ if (value !== null && value !== undefined) data.append(key, value);
+ });
+
+ const res = await axios.post(
+ `${import.meta.env.VITE_API_URL}/api/interview/setup`,
+ data,
+ { headers: { Authorization: `Bearer ${token}` } }
+ );
+
+ showToast("Interview setup complete!", "success");
+ const interviewId = res.data?.interview?._id;
+ if (!interviewId) {
+ console.error("Unexpected response shape:", res.data);
+ showToast("Interview created but ID missing in response.", "error");
+ return;
+ }
+ navigate(`/interview/${interviewId}`);
+ } catch (err) {
+ console.error("Error in interview setup:", err.response?.data || err);
+ showToast(err.response?.data?.message || "Something went wrong", "error");
+ } finally {
+ setLoading(false);
+ }📝 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.
| setLoading(true); | |
| const token = await user.getIdToken(); | |
| const data = new FormData(); | |
| Object.keys(formData).forEach((key) => { | |
| if (formData[key]) data.append(key, formData[key]); | |
| }); | |
| try { | |
| const res = await axios.post( | |
| `${import.meta.env.VITE_API_URL}/api/interview/setup`, | |
| data, | |
| { headers: { Authorization: `Bearer ${token}` } } | |
| ); | |
| showToast("Interview setup complete!", "success"); | |
| const interviewId = res.data.interview._id; | |
| navigate(`/interview/${interviewId}`); | |
| } catch (err) { | |
| console.error("Error in interview setup:", err.response?.data || err); | |
| showToast("Something went wrong", "error"); | |
| } finally { | |
| setLoading(false); | |
| } | |
| setLoading(true); | |
| try { | |
| const token = await user.getIdToken(); | |
| const data = new FormData(); | |
| Object.entries(formData).forEach(([key, value]) => { | |
| if (value !== null && value !== undefined) data.append(key, value); | |
| }); | |
| const res = await axios.post( | |
| `${import.meta.env.VITE_API_URL}/api/interview/setup`, | |
| data, | |
| { headers: { Authorization: `Bearer ${token}` } } | |
| ); | |
| showToast("Interview setup complete!", "success"); | |
| const interviewId = res.data?.interview?._id; | |
| if (!interviewId) { | |
| console.error("Unexpected response shape:", res.data); | |
| showToast("Interview created but ID missing in response.", "error"); | |
| return; | |
| } | |
| navigate(`/interview/${interviewId}`); | |
| } catch (err) { | |
| console.error("Error in interview setup:", err.response?.data || err); | |
| showToast(err.response?.data?.message || "Something went wrong", "error"); | |
| } finally { | |
| setLoading(false); | |
| } |
🤖 Prompt for AI Agents
In client/src/pages/CreateInterview.jsx around lines 154 to 177, fetch of
user.getIdToken() and FormData construction live outside the try block and the
code skips all falsy values when appending (which drops valid values like 0);
this can leave loading stuck true if getIdToken() throws. Move the token
retrieval and the FormData build inside the try block so the finally always
runs; when appending, only skip items that are null or undefined (e.g., use
explicit checks) so 0/'' are preserved; in the catch, surface the server error
message if present (err.response?.data?.message or similar) to show a more
informative toast, and keep setLoading(false) in finally.
|
Hi, thanks for the contribution! 🙌 Please follow the PR template. Kindly update your PR to match the required format(You've not mentioned the Issue number fixed). LGTM! |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
CoderUzumaki
left a comment
CoderUzumaki
left a comment
There was a problem hiding this comment.
Here are few changes I think would be required before merging the files. Please have a look at them and ensure that you make necessary changes.
- Currently the toasts on both pages are of different color.
- The toasts added lacks a close button, so I have no choice but to see the toast until it's timer runs out, which does not allow me to click buttons on the top-right corner.
3 participants
PR Description
Issue Number: #94
Type of Changes
Changes Implemented:
Screenshots / Demo:
Environment Variables
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores