Make Footer Mobile-Friendly #99
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. |
WalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant C as Client (Login.jsx)
participant FB as Firebase Auth
participant API as Backend /api/auth/login
U->>C: Submit credentials or click provider
C->>FB: signInWithEmailAndPassword / signInWithPopup
FB-->>C: Firebase user + ID token
C->>API: POST /api/auth/login (Authorization: Bearer <token>)
API-->>C: 200 OK
C-->>U: Show success toast, setUser, navigate
alt Failure
FB-->>C: Error / API returns error
C-->>U: Show error toast
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
client/src/pages/CreateInterview.jsx (4)
104-110: Stray statement1;will execute and serves no purposeRemove it.
const removeFile = () => { handleInputChange("resume", null); - 1; if (fileInputRef.current) { fileInputRef.current.value = ""; } };
27-29: Avoid logging user objectsConsole logging user objects may leak PII.
- useEffect(() => { - console.log("--------\nCurrent user from context:", user, "\n--------"); - }, [user]); + useEffect(() => {}, [user]);
127-136: Remove token and full form data logsID tokens and raw form data should not be printed.
setLoading(true); const token = await user.getIdToken(); - console.log("-------\nToken being sent to backend:", token, "\n-------"); const data = new FormData(); Object.keys(formData).forEach((key) => { if (formData[key]) { data.append(key, formData[key]); } - console.log("-------\nForm Data: \n", [...data.entries()], "\n-------"); });
82-103: Enforce the advertised 5MB size limit on resume uploadsValidate in both drop and file-select paths; show a toast on violation.
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") { + if (file.type === "application/pdf") { + if (file.size > 5 * 1024 * 1024) { + showToast("File too large. Max 5MB.", "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") { + if (file.type === "application/pdf") { + if (file.size > 5 * 1024 * 1024) { + showToast("File too large. Max 5MB.", "error"); + e.target.value = ""; + return; + } handleInputChange("resume", file); } } };
🧹 Nitpick comments (34)
client/src/components/ContactForm.jsx (10)
61-61: Announce loading state to assistive techAdd aria-busy for better a11y during submission.
- <form onSubmit={handleSubmit} className="space-y-6"> + <form onSubmit={handleSubmit} className="space-y-6" aria-busy={isSubmitting}>
69-69: Hide decorative asterisks from screen readersPrevents SRs reading “star”.
- Full Name <span className="text-red-600">*</span> + Full Name <span className="text-red-600" aria-hidden="true">*</span> @@ - Email Address <span className="text-red-600">*</span> + Email Address <span className="text-red-600" aria-hidden="true">*</span> @@ - Subject <span className="text-red-600">*</span> + Subject <span className="text-red-600" aria-hidden="true">*</span> @@ - Message <span className="text-red-600">*</span> + Message <span className="text-red-600" aria-hidden="true">*</span>Also applies to: 87-87, 130-130, 151-151
72-80: Add autocomplete and sensible length caps to inputsImproves mobile keyboards, reduces server-side validation churn.
- <input + <input type="text" id="name" name="name" value={formData.name} onChange={handleChange} required - className="w-full px-3 sm:px-4 py-2.5 sm:py-3 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent transition-all text-sm sm:text-base" + autoComplete="name" + maxLength={80} + className="w-full px-3 sm:px-4 py-2.5 sm:py-3 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent transition-all text-sm sm:text-base" placeholder="Your full name" />
89-98: Autocomplete for email and cap length- <input + <input type="email" id="email" name="email" value={formData.email} onChange={handleChange} required - className="w-full px-3 sm:px-4 py-2.5 sm:py-3 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent transition-all text-sm sm:text-base" + autoComplete="email" + maxLength={254} + className="w-full px-3 sm:px-4 py-2.5 sm:py-3 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent transition-all text-sm sm:text-base" placeholder="your.email@example.com" />
132-141: Limit subject length to keep payloads small- <input + <input type="text" id="subject" name="subject" value={formData.subject} onChange={handleChange} required - className="w-full px-3 sm:px-4 py-2.5 sm:py-3 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent transition-all text-sm sm:text-base" + autoComplete="off" + maxLength={120} + className="w-full px-3 sm:px-4 py-2.5 sm:py-3 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent transition-all text-sm sm:text-base" placeholder="Brief description of your inquiry" />
153-162: Add min/max length to message (basic client-side validation)- <textarea + <textarea id="message" name="message" value={formData.message} onChange={handleChange} required rows={5} - className="w-full px-3 sm:px-4 py-2.5 sm:py-3 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent transition-all resize-none text-sm sm:text-base" + minLength={10} + maxLength={2000} + className="w-full px-3 sm:px-4 py-2.5 sm:py-3 border border-gray-300 rounded-lg focus:ring-2 focus:ring-blue-500 focus:border-transparent transition-all resize-none text-sm sm:text-base" placeholder="Please provide details about your inquiry..." />
166-176: Improve spinner/icon accessibilityAnnounce status and hide decorative icon.
- <button + <button type="submit" disabled={isSubmitting} className="w-full bg-blue-500 hover:bg-blue-600 disabled:bg-blue-300 text-white font-semibold py-2.5 sm:py-3 px-4 sm:px-6 rounded-lg transition-colors flex items-center justify-center space-x-2 text-sm sm:text-base" + aria-disabled={isSubmitting} > {isSubmitting ? ( <> - <div className="w-4 h-4 sm:w-5 sm:h-5 border-2 border-white border-t-transparent rounded-full animate-spin"></div> - <span>Sending...</span> + <div className="w-4 h-4 sm:w-5 sm:h-5 border-2 border-white border-t-transparent rounded-full animate-spin" role="status" aria-label="Sending"></div> + <span>Sending...</span> </> ) : ( <> - <FaPaperPlane className="w-4 h-4" /> + <FaPaperPlane className="w-4 h-4" aria-hidden="true" /> <span>Send Message</span> </> )}Also applies to: 178-179
16-21: Minor perf: move static categories outside the componentAvoids re-allocation on each render. Low impact but easy win.
Outside this range, define once:
// top-level (module scope) const CATEGORIES = [ { value: "general", label: "General Inquiry" }, { value: "feature", label: "Feature Request" }, { value: "bug", label: "Bug Report" }, { value: "collaboration", label: "Collaboration" }, ]; // inside component // const categories = CATEGORIES;
23-26: Optional: defensive handleChange to guard unknown fieldsPrevents accidental state pollution if names change.
- const handleChange = (e) => { - const { name, value } = e.target; - setFormData((prev) => ({ ...prev, [name]: value })); - }; + const handleChange = (e) => { + const { name, value } = e.target; + if (!(name in formData)) return; + setFormData((prev) => ({ ...prev, [name]: value })); + };
28-53: Consider centralizing API clientIf multiple components post to the API, use an axios instance with baseURL from env, default timeout, and JSON headers to reduce duplication.
Example (outside this file):
- client/src/lib/api.js: export axios.create({ baseURL, timeout: 10000, headers: { "Content-Type": "application/json" } })
- Import and use api.post("/api/contact", payload)
client/src/components/FAQ.jsx (3)
12-24: Derived state won’t update iffaqschangesIf
faqsis fetched/updated after mount,openItemsstays stale. Consider syncing whenfaqsorallowMultipleOpenchanges.-import { useState } from "react"; +import { useState, useEffect } from "react"; @@ const [openItems, setOpenItems] = useState( faqs.map((faq, index) => (faq.defaultOpen ? index : -1)).filter((index) => index !== -1) ); + + useEffect(() => { + if (allowMultipleOpen) { + setOpenItems(faqs.map((f, i) => (f.defaultOpen ? i : -1)).filter((i) => i !== -1)); + } else { + const first = faqs.findIndex((f) => !!f.defaultOpen); + setOpenItems(first >= 0 ? [first] : []); + } + }, [faqs, allowMultipleOpen]);
39-46: Avoid ID collisions across multiple FAQ instances; use stable keysIndex-based IDs can collide if this component renders more than once on a page. Also, index keys break on reordering.
-import { useState } from "react"; +import { useState, useId } from "react"; @@ - const contentId = `faq-content-${index}`; + const buttonId = `${baseId}-button-${index}`; + const contentId = `${baseId}-content-${index}`; @@ - <div - key={index} + <div + key={faq.id ?? index}Add a base ID near state:
const [openItems, setOpenItems] = useState( faqs.map((faq, index) => (faq.defaultOpen ? index : -1)).filter((index) => index !== -1) ); + const baseId = useId();
63-67: A11y + overflow: content may be read when “closed”, and long answers will be clipped
- Screen readers may still read closed content.
max-h-96(24rem) will clip longer answers due tooverflow-hidden.Minimal fix:
- <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} + role="region" + aria-labelledby={buttonId} + aria-hidden={!isOpen} + className={`overflow-hidden transition-all duration-300 ease-in-out ${ + isOpen ? "max-h-[80vh] opacity-100" : "max-h-0 opacity-0" + }`} + >Optional (smoother animation without clipping): use a ref + measured
scrollHeightto setmaxHeightinline; happy to provide a patch if you want it.client/src/components/InterviewList.jsx (3)
16-21: Prefer Link over button for navigation; add type and a11y attributes if keeping buttonUse a semantic Link for SPA routing (enables open-in-new-tab, correct semantics). If you keep the button, add type="button" and an accessible label.
Apply either:
Option A — minimal a11y fixes:
- <button - onClick={() => navigate(`/interview/report/${interviewId}`)} - className="px-4 py-2 text-sm font-medium text-gray-700 bg-gray-100 hover:bg-gray-200 rounded-md transition-colors" - > + <button + type="button" + aria-label={`View report for ${title}`} + onClick={() => navigate(`/interview/report/${interviewId}`)} + className="px-4 py-2 text-sm font-medium text-gray-700 bg-gray-100 hover:bg-gray-200 rounded-md transition-colors focus-visible:outline focus-visible:outline-2 focus-visible:outline-blue-600 focus-visible:outline-offset-2" + > View Report </button>Option B — semantic Link (recommended):
- <button - onClick={() => navigate(`/interview/report/${interviewId}`)} - className="px-4 py-2 text-sm font-medium text-gray-700 bg-gray-100 hover:bg-gray-200 rounded-md transition-colors" - > - View Report - </button> + <Link + to={`/interview/report/${interviewId}`} + aria-label={`View report for ${title}`} + className="px-4 py-2 text-sm font-medium text-gray-700 bg-gray-100 hover:bg-gray-200 rounded-md transition-colors focus-visible:outline focus-visible:outline-2 focus-visible:outline-blue-600 focus-visible:outline-offset-2" + > + View Report + </Link>If you choose Option B, also update imports:
import { Link } from "react-router-dom";
51-57: Format created_at before passing to the cardImproves readability and avoids leaking raw ISO strings.
- date={interview.created_at} + date={ + interview?.created_at + ? new Date(interview.created_at).toLocaleDateString(undefined, { + year: "numeric", + month: "short", + day: "numeric", + }) + : "" + }
64-97: Add pagination a11y: nav landmark, aria-current, labels, and button typesBetter keyboard/screen-reader UX; avoids accidental form submission.
- <div className="flex justify-center items-center space-x-2"> + <nav className="flex justify-center items-center space-x-2" aria-label="Interviews pagination"> <button + type="button" onClick={goToPrevious} disabled={currentPage === 1} - className="p-2 rounded-md text-gray-400 hover:text-gray-600 disabled:opacity-50 disabled:cursor-not-allowed" + aria-label="Previous page" + title="Previous page" + className="p-2 rounded-md text-gray-400 hover:text-gray-600 disabled:opacity-50 disabled:cursor-not-allowed focus-visible:outline focus-visible:outline-2 focus-visible:outline-blue-600 focus-visible:outline-offset-2" > <FaChevronLeft className="w-4 h-4" /> </button> @@ {Array.from({ length: totalPages }, (_, i) => i + 1).map((page) => ( <button key={page} + type="button" onClick={() => goToPage(page)} - className={`w-8 h-8 rounded-md text-sm font-medium transition-colors ${ - currentPage === page + aria-current={currentPage === page ? "page" : undefined} + className={`w-8 h-8 rounded-md text-sm font-medium transition-colors focus-visible:outline focus-visible:outline-2 focus-visible:outline-blue-600 focus-visible:outline-offset-2 ${ + currentPage === page ? "bg-blue-600 text-white" : "text-gray-600 hover:text-gray-900 hover:bg-gray-100" }`} > {page} </button> ))} @@ <button + type="button" onClick={goToNext} disabled={currentPage === totalPages} - className="p-2 rounded-md text-gray-400 hover:text-gray-600 disabled:opacity-50 disabled:cursor-not-allowed" + aria-label="Next page" + title="Next page" + className="p-2 rounded-md text-gray-400 hover:text-gray-600 disabled:opacity-50 disabled:cursor-not-allowed focus-visible:outline focus-visible:outline-2 focus-visible:outline-blue-600 focus-visible:outline-offset-2" > <FaChevronRight className="w-4 h-4" /> </button> - </div> + </nav>Note: If you adopt the normalizedPage change above, replace currentPage with normalizedPage in the aria-current and disabled bindings.
client/src/pages/CreateInterview.jsx (3)
282-300: Add accessibility states to toggle buttonsExpose selection to assistive tech via aria-pressed.
Apply:
- <button + <button + aria-pressed={formData.interviewType === type} + aria-label={`Interview type ${type}`}
120-126: If unauthenticated, redirect instead of silently returningGuide the user to login for a smoother flow.
const handleSubmit = async (e) => { e.preventDefault(); if (!user) { - console.error("❌ User is null in CreateInterview.jsx"); - return; + navigate("/login"); + return; }
173-181: Consider unifying to react-hot-toastProject now includes react-hot-toast; replacing the custom Toast would reduce duplicate systems.
client/src/pages/Dashboard.jsx (2)
54-55: Normalize bestScore to numberBe explicit for consistency with chartData casting.
-const bestScore = Math.max(...reports.map((i) => i.finalScore || 0), 0); +const bestScore = Math.max(...reports.map((i) => Number(i.finalScore) || 0), 0);
95-97: Prefer stable keysUse title (unique here) instead of array index to avoid reconciliation quirks.
- {summaryData.map((item, index) => ( - <SummaryCard key={index} title={item.title} value={item.value} /> + {summaryData.map((item) => ( + <SummaryCard key={item.title} title={item.title} value={item.value} /> ))}client/src/pages/Login.jsx (2)
161-163: Tailwind class "min-w-screen" is non-standardUse w-screen or remove.
- <div className="min-h-screen bg-gray-50 min-w-screen"> + <div className="min-h-screen bg-gray-50 w-screen">
199-205: Improve accessibility of password visibility toggleAdd aria-label and aria-pressed.
- <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" + 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} >server/index.js (3)
21-26: Make CORS origin configurable (supports prod and local dev)Hardcoding a single origin will block localhost during development; credentials=true also requires exact origin reflection.
-// --- Middlewares --- -app.use(cors({ - origin: 'https://prepedgeai.vercel.app', - credentials: true, -})); +// --- Middlewares --- +const allowedOrigins = (process.env.CLIENT_ORIGINS || "https://prepedgeai.vercel.app,http://localhost:5173").split(","); +app.use(cors({ + origin: allowedOrigins, + credentials: true, +}));
28-40: Avoid duplicating auth logic; reuse existing firebaseAuthMiddleware and align request propertyYou already have middleware that sets req.firebaseUser. Reuse it here to prevent divergence from routes expecting req.firebaseUser.
-// --- Protected route middleware --- -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; - next(); - } catch (err) { - res.status(401).json({ error: "Invalid token" }); - } -}); +// --- Protected route middleware --- +import firebaseAuth from "./middleware/firebaseAuthMiddleware.js"; +app.use("/api/protected", firebaseAuth);
12-16: dotenv usage appears in both firebase.js and hereMinor duplication; keep one place (typically index/bootstrap). Not urgent.
client/src/components/Footer.jsx (3)
74-77: Replace placeholder “#” routes or hide themLinks to "#" are non-functional and can confuse users and screen readers. Point to real routes or remove until available.
-<li><Link to="#" className="text-gray-600 hover:text-cyan-500 transition-all duration-200 inline-block">Help Center</Link></li> -<li><Link to="#" className="text-gray-600 hover:text-cyan-500 transition-all duration-200 inline-block">Privacy Policy</Link></li> -<li><Link to="#" className="text-gray-600 hover:text-cyan-500 transition-all duration-200 inline-block">Terms of Service</Link></li> +<li><Link to="/help" className="text-gray-600 hover:text-cyan-500 transition-all duration-200 inline-block">Help Center</Link></li> +<li><Link to="/privacy" className="text-gray-600 hover:text-cyan-500 transition-all duration-200 inline-block">Privacy Policy</Link></li> +<li><Link to="/terms" className="text-gray-600 hover:text-cyan-500 transition-all duration-200 inline-block">Terms of Service</Link></li>-<Link to="#" className="text-gray-500 hover:text-cyan-500 text-sm sm:text-base transition-colors duration-200">Privacy</Link> -<Link to="#" className="text-gray-500 hover:text-cyan-500 text-sm sm:text-base transition-colors duration-200">Terms</Link> -<Link to="#" className="text-gray-500 hover:text-cyan-500 text-sm sm:text-base transition-colors duration-200">Cookies</Link> +<Link to="/privacy" className="text-gray-500 hover:text-cyan-500 text-sm sm:text-base transition-colors duration-200">Privacy</Link> +<Link to="/terms" className="text-gray-500 hover:text-cyan-500 text-sm sm:text-base transition-colors duration-200">Terms</Link> +<Link to="/cookies" className="text-gray-500 hover:text-cyan-500 text-sm sm:text-base transition-colors duration-200">Cookies</Link>Also applies to: 87-89
31-38: Update aria-label to reflect “X” brandUse “X (Twitter)” for clarity.
-aria-label="Twitter" +aria-label="X (Twitter)"
3-3: Remove unused FaTwitter importFaTwitter isn’t used.
-import { FaGithub, FaTwitter, FaLinkedin, FaEnvelope } from "react-icons/fa"; +import { FaGithub, FaLinkedin, FaEnvelope } from "react-icons/fa";client/src/App.jsx (1)
19-33: Respect prefers-reduced-motion for toast animationDisable gradient animation when users prefer reduced motion.
function App() { return ( <div className="min-h-screen bg-white flex flex-col"> @@ - <Toaster + <Toaster position="top-right" toastOptions={{ duration: 4000, - style: { + style: { fontSize: "15px", fontWeight: "600", borderRadius: "14px", padding: "14px 20px", color: "#fff", boxShadow: "0 6px 18px rgba(0,0,0,0.15)", backgroundSize: "200% 200%", - animation: "gradientShift 3s ease infinite", + animation: + typeof window !== "undefined" && + window.matchMedia && + window.matchMedia("(prefers-reduced-motion: reduce)").matches + ? "none" + : "gradientShift 3s ease infinite", },Also applies to: 60-67
client/src/pages/SignUp.jsx (4)
203-203: Fix invalid Tailwind classmin-w-screen isn’t a Tailwind utility; use min-w-full.
-<div className="min-h-screen bg-gray-50 min-w-screen"> +<div className="min-h-screen bg-gray-50 min-w-full">
224-232: Add autocomplete attributes for better UX and a11yEnable browser assistance on common fields.
-<input +<input id="name" type="text" value={name} onChange={(e) => handleNameChange(e.target.value)} placeholder="Enter your name" + autoComplete="name"-<input +<input id="email" type="email" value={email} onChange={(e) => handleEmailChange(e.target.value)} placeholder="Enter your email" + autoComplete="email"-<input +<input id="password" type={showPassword ? "text" : "password"} value={password} onChange={(e) => handlePasswordChange(e.target.value)} placeholder="Enter your password" + autoComplete="new-password"Also applies to: 253-261, 283-291
16-18: Initialize GithubAuthProvider onceSlight perf/readability win by memoizing.
-import { useState } from "react"; +import { useState, useMemo } from "react"; @@ -export default function SignUp() { - const githubProvider = new GithubAuthProvider(); +export default function SignUp() { + const githubProvider = useMemo(() => new GithubAuthProvider(), []);
154-173: Endpoint consistency: register vs loginSignUp uses /api/auth/register for Google/GitHub; Login.jsx uses /api/auth/login. If register is idempotent for existing users, fine—otherwise align endpoints to avoid surprises.
Also applies to: 178-200
📜 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 (9)
server/firebase.js (1)
server/middleware/firebaseAuthMiddleware.js (1)
req(3-18)
client/src/components/ContactForm.jsx (1)
client/src/pages/Contact.jsx (1)
Contact(5-41)
client/src/App.jsx (2)
client/src/components/ScrollToTop.jsx (1)
ScrollToTop(4-15)client/src/components/Toast.jsx (1)
Toast(6-55)
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)
client/src/components/FAQ.jsx (2)
client/src/components/QuestionReview.jsx (1)
QuestionReview(6-70)client/src/pages/Contact.jsx (1)
Contact(5-41)
client/src/pages/Dashboard.jsx (7)
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/pages/Report.jsx (3)
report(11-11)InterviewReport(8-132)response(15-29)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)
server/index.js (4)
server/routes/interviewRoutes.js (4)
upload(22-22)req(36-46)req(150-150)req(223-233)server/routes/authRoutes.js (4)
req(14-14)req(36-36)req(13-33)req(35-45)server/middleware/firebaseAuthMiddleware.js (1)
req(3-18)server/routes/reportRoutes.js (1)
req(21-31)
client/src/components/InterviewList.jsx (3)
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)client/src/pages/Report.jsx (1)
InterviewReport(8-132)
🔇 Additional comments (16)
client/src/components/ContactForm.jsx (2)
55-60: Nice mobile-friendly layout changesResponsive grid, spacing, and typographic scales look good and should behave well on small screens.
Also applies to: 63-65, 102-105
39-50: Replace blocking alerts with non-blocking toasts and surface server error messagesFile: client/src/components/ContactForm.jsx (lines 39–50) — replace alerts with react-hot-toast toasts; add import.
- alert("Message sent successfully ✅"); + toast.success("Message sent successfully ✅"); @@ - console.error("Error submitting contact form:", error); - alert("❌ There was an error submitting your message."); + console.error("Error submitting contact form:", error); + const msg = + error?.response?.data?.message || + error?.message || + "There was an error submitting your message."; + toast.error(`❌ ${msg}`);Add this import (outside the selected range, matching project style):
import toast from "react-hot-toast";Verified: react-hot-toast is in client/package.json (^2.6.0) and is mounted in client/src/App.jsx; other files use the default
import toast from "react-hot-toast".client/src/components/FAQ.jsx (6)
1-2: Single chevron + rotation: LGTMDropping the up icon and rotating a single down icon is cleaner and reduces DOM noise.
31-33: Subtitle semantics: LGTMClear typography, centered layout reads well on mobile.
56-60: Icon animation: LGTMRotation with
transform+ transition keeps DOM light and is accessible.
70-72: Answer typography: LGTMMobile-friendly text sizing and leading look good.
80-87: Contact link: LGTMAnchor targets existing
#contact-form(see Contact.jsx). Good hover affordance.
1-91: PR description vs. changesPR description says only Footer.jsx, CreateInterview.jsx, ContactForm.jsx are in scope, but this PR includes FAQ.jsx. Please confirm scope or update the description to avoid confusion.
client/src/components/InterviewList.jsx (3)
28-30: Good defensive normalization of input.Forcing interviews to an array prevents runtime errors downstream. Nice.
49-62: Helpful empty state and conditional paginationClear copy and only renders controls when needed. Looks good.
17-18: Confirm route /interview/report/:interviewId exists
Search returned no matching Route declaration; verify the client router defines /interview/report/:interviewId or update the navigate target to an existing route.
Location: client/src/components/InterviewList.jsx (lines 17–18)client/package.json (1)
23-23: Add react-hot-toast: LGTMDependency addition looks correct.
client/src/pages/CreateInterview.jsx (1)
276-301: Responsive grid for Interview Type: LGTMSwitching to grid-cols-1 sm:grid-cols-3 improves mobile UX.
server/package.json (1)
35-35: Move nodemon to devDependencies: LGTMCorrect placement.
client/src/pages/Login.jsx (1)
112-113: Different post-login redirects; confirm intentGoogle/email go to "/", GitHub goes to "/dashboard". If unintended, align behavior.
Also applies to: 147-148
server/index.js (1)
21-39: Scope check: server/auth changes are outside the stated “Footer mobile” PRConfirm these server-side additions are intended for this PR; otherwise consider moving them to a focused backend/auth PR.
| await axios.post( | ||
| `${import.meta.env.VITE_API_URL}/api/contact`, | ||
| formData, | ||
| { headers: { "Content-Type": "application/json" } } | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden API URL + timeout + payload normalization to avoid “undefined/api/contact” and hangs
Build the URL robustly, trim inputs, and set a request timeout.
Apply this diff:
- await axios.post(
- `${import.meta.env.VITE_API_URL}/api/contact`,
- formData,
- { headers: { "Content-Type": "application/json" } }
- );
+ const baseUrl = (import.meta.env.VITE_API_URL || "").replace(/\/+$/, "");
+ const url = baseUrl ? `${baseUrl}/api/contact` : "/api/contact";
+ const payload = {
+ ...formData,
+ name: formData.name.trim(),
+ email: formData.email.trim().toLowerCase(),
+ subject: formData.subject.trim(),
+ message: formData.message.trim(),
+ };
+ await axios.post(url, payload, {
+ headers: { "Content-Type": "application/json" },
+ timeout: 10000,
+ });📝 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.
| await axios.post( | |
| `${import.meta.env.VITE_API_URL}/api/contact`, | |
| formData, | |
| { headers: { "Content-Type": "application/json" } } | |
| ); | |
| const baseUrl = (import.meta.env.VITE_API_URL || "").replace(/\/+$/, ""); | |
| const url = baseUrl ? `${baseUrl}/api/contact` : "/api/contact"; | |
| const payload = { | |
| ...formData, | |
| name: formData.name.trim(), | |
| email: formData.email.trim().toLowerCase(), | |
| subject: formData.subject.trim(), | |
| message: formData.message.trim(), | |
| }; | |
| await axios.post(url, payload, { | |
| headers: { "Content-Type": "application/json" }, | |
| timeout: 10000, | |
| }); |
| <button | ||
| onClick={() => toggleItem(index)} | ||
| className="w-full px-6 py-4 text-left flex justify-between items-center hover:bg-gray-50 transition-colors duration-200 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-inset rounded-lg cursor-pointer" | ||
| aria-expanded={isOpen} | ||
| aria-controls={contentId} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Set type="button" and expose id for a11y linkage
Prevents accidental form submission; enables aria-labelledby on the panel.
- <button
- onClick={() => toggleItem(index)}
+ <button
+ type="button"
+ onClick={() => toggleItem(index)}
className="w-full px-6 py-4 text-left flex justify-between items-center hover:bg-gray-50 transition-colors duration-200 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-inset rounded-lg cursor-pointer"
aria-expanded={isOpen}
- aria-controls={contentId}
+ aria-controls={contentId}
+ id={buttonId}
>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In client/src/components/FAQ.jsx around lines 47 to 52, the button element is
missing an explicit type and an id to link with the panel for accessibility;
update the button to include type="button" to prevent accidental form submission
and add a unique id (e.g., `button-${index}` or similar predictable string) so
the corresponding panel can reference it via aria-labelledby; ensure the panel
element has aria-labelledby set to that same id and keep aria-controls on the
button as-is for bidirectional a11y linkage.
| const [currentPage, setCurrentPage] = useState(1); | ||
| const totalPages = Math.ceil(safeInterviews.length / itemsPerPage); | ||
|
|
||
| const goToPage = (page) => { | ||
| setCurrentPage(page); | ||
| }; | ||
| const startIndex = (currentPage - 1) * itemsPerPage; | ||
| const endIndex = startIndex + itemsPerPage; | ||
| const currentInterviews = safeInterviews.slice(startIndex, endIndex); | ||
|
|
||
| const goToPrevious = () => { | ||
| if (currentPage > 1) { | ||
| setCurrentPage(currentPage - 1); | ||
| } | ||
| }; | ||
| const goToPage = (page) => setCurrentPage(page); | ||
| const goToPrevious = () => currentPage > 1 && setCurrentPage(currentPage - 1); | ||
| const goToNext = () => | ||
| currentPage < totalPages && setCurrentPage(currentPage + 1); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize itemsPerPage and clamp currentPage to avoid empty pages when data shrinks
Prevents NaN/Infinity when itemsPerPage is 0/invalid and fixes the stale-page bug when the list size changes.
Apply:
- const [currentPage, setCurrentPage] = useState(1);
- const totalPages = Math.ceil(safeInterviews.length / itemsPerPage);
+ const [currentPage, setCurrentPage] = useState(1);
+ const pageSize = Math.max(1, Number(itemsPerPage) || 1);
+ const totalPages = Math.max(1, Math.ceil(safeInterviews.length / pageSize));
- const startIndex = (currentPage - 1) * itemsPerPage;
- const endIndex = startIndex + itemsPerPage;
+ const normalizedPage = Math.min(Math.max(1, currentPage), totalPages);
+ const startIndex = (normalizedPage - 1) * pageSize;
+ const endIndex = startIndex + pageSize;
- const goToPage = (page) => setCurrentPage(page);
- const goToPrevious = () => currentPage > 1 && setCurrentPage(currentPage - 1);
- const goToNext = () =>
- currentPage < totalPages && setCurrentPage(currentPage + 1);
+ const goToPage = (page) =>
+ setCurrentPage(Math.min(Math.max(1, Number(page) || 1), totalPages));
+ const goToPrevious = () => setCurrentPage((p) => Math.max(1, p - 1));
+ const goToNext = () => setCurrentPage((p) => Math.min(totalPages, p + 1));Also reflect normalizedPage in controls:
- disabled={currentPage === 1}
+ disabled={normalizedPage === 1}
@@
- currentPage === page
+ normalizedPage === page
@@
- disabled={currentPage === totalPages}
+ disabled={normalizedPage === totalPages}Also applies to: 68-69, 79-81, 90-91
🤖 Prompt for AI Agents
In client/src/components/InterviewList.jsx around lines 31 to 41 (and also
update corresponding logic at lines 68-69, 79-81, 90-91), normalize itemsPerPage
to a safe positive integer (e.g., const safeItemsPerPage = Math.max(1,
parseInt(itemsPerPage) || 1)), compute totalPages using safeItemsPerPage (const
totalPages = Math.max(1, Math.ceil(safeInterviews.length / safeItemsPerPage))),
clamp currentPage to the valid range after data or itemsPerPage changes
(setCurrentPage(prev => Math.min(Math.max(1, prev), totalPages))), derive a
normalizedPage variable from the clamped state to use in rendering and controls,
and update goToPrevious/goToNext/goToPage to operate on the clamped/normalized
page to avoid NaN/Infinity and stale empty pages when list size shrinks.
| useEffect(() => { | ||
| const fetchData = async () => { | ||
| try { | ||
| const token = await user.getIdToken(); | ||
|
|
||
| // ---- Fetch Interviews ---- | ||
| const response = await axios.get( | ||
| `${import.meta.env.VITE_API_URL}/api/interview`, |
There was a problem hiding this comment.
Guard against null user before calling getIdToken
Without a guard, this throws when user is not yet initialized.
-useEffect(() => {
+useEffect(() => {
+ if (!user) return;
const fetchData = async () => {
try {
const token = await user.getIdToken();📝 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.
| useEffect(() => { | |
| const fetchData = async () => { | |
| try { | |
| const token = await user.getIdToken(); | |
| // ---- Fetch Interviews ---- | |
| const response = await axios.get( | |
| `${import.meta.env.VITE_API_URL}/api/interview`, | |
| useEffect(() => { | |
| if (!user) return; | |
| const fetchData = async () => { | |
| try { | |
| const token = await user.getIdToken(); |
🤖 Prompt for AI Agents
In client/src/pages/Dashboard.jsx around lines 14 to 21, the effect calls
user.getIdToken() without ensuring user is initialized which can throw when user
is null; add a guard at the start of the fetchData routine (or before calling
it) to check if user is truthy and return early (or wait) when it's not, e.g. if
(!user) return; so you only call getIdToken on a valid user, and ensure
useEffect dependencies include user so the effect runs when the user becomes
available.
server/firebase.js
Outdated
| dotenv.config(); | ||
|
|
||
| const serviceAccount = JSON.parse(process.env.FIREBASE_SERVICE_ACCOUNT_KEY); | ||
|
|
||
| admin.initializeApp({ | ||
| credential: admin.credential.cert(serviceAccount), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden Firebase Admin init: avoid startup crash, guard re-init, and add ADC fallback
JSON.parse on a missing/invalid FIREBASE_SERVICE_ACCOUNT_KEY will crash the server, and repeated imports can re-initialize the app. Guard init, wrap parsing, and fall back to applicationDefault credentials.
Apply:
-import dotenv from "dotenv";
-
-dotenv.config();
-
-const serviceAccount = JSON.parse(process.env.FIREBASE_SERVICE_ACCOUNT_KEY);
-
-admin.initializeApp({
- credential: admin.credential.cert(serviceAccount),
-});
+import dotenv from "dotenv";
+dotenv.config();
+
+const rawKey = process.env.FIREBASE_SERVICE_ACCOUNT_KEY;
+let credential;
+
+if (rawKey) {
+ try {
+ const serviceAccount = JSON.parse(rawKey);
+ credential = admin.credential.cert(serviceAccount);
+ } catch (e) {
+ console.warn("Invalid FIREBASE_SERVICE_ACCOUNT_KEY JSON. Falling back to application default credentials.");
+ }
+} else {
+ console.warn("FIREBASE_SERVICE_ACCOUNT_KEY not set. Falling back to application default credentials.");
+}
+
+if (!admin.apps.length) {
+ admin.initializeApp({
+ credential: credential || admin.credential.applicationDefault(),
+ });
+}Follow-up: if you centralize dotenv in index.js later, we can drop it here.
📝 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.
| dotenv.config(); | |
| const serviceAccount = JSON.parse(process.env.FIREBASE_SERVICE_ACCOUNT_KEY); | |
| admin.initializeApp({ | |
| credential: admin.credential.cert(serviceAccount), | |
| }); | |
| import dotenv from "dotenv"; | |
| dotenv.config(); | |
| const rawKey = process.env.FIREBASE_SERVICE_ACCOUNT_KEY; | |
| let credential; | |
| if (rawKey) { | |
| try { | |
| const serviceAccount = JSON.parse(rawKey); | |
| credential = admin.credential.cert(serviceAccount); | |
| } catch (e) { | |
| console.warn("Invalid FIREBASE_SERVICE_ACCOUNT_KEY JSON. Falling back to application default credentials."); | |
| } | |
| } else { | |
| console.warn("FIREBASE_SERVICE_ACCOUNT_KEY not set. Falling back to application default credentials."); | |
| } | |
| if (!admin.apps.length) { | |
| admin.initializeApp({ | |
| credential: credential || admin.credential.applicationDefault(), | |
| }); | |
| } |
🤖 Prompt for AI Agents
In server/firebase.js around lines 4 to 10, the current initialization
unconditionally calls dotenv.config(), JSON.parse on
FIREBASE_SERVICE_ACCOUNT_KEY (which will crash if missing/invalid), and always
initializes the admin app (which can re-init on repeated imports); wrap
JSON.parse in a try/catch and only build a serviceAccount object if the env var
exists and is valid, guard initialization with a check like if
(!admin.apps.length) before calling admin.initializeApp, and when no valid
service account is present fall back to admin.credential.applicationDefault() so
startup won’t crash and ADC is used; keep dotenv.config() here only if dotenv
hasn’t been centralized elsewhere.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.gitignore (3)
30-30: Drop redundant.enventryYou already ignore all
.envfiles via**/.envand.env.*. The root-level.envline is duplicate noise.-.env
31-33: Deduplicate and anchorserver/firebase.jsignoreThe path is listed twice and isn’t anchored. Prefer a single root-anchored rule for clarity and to avoid accidental matches in nested
server/dirs.-server/firebase.js - -server/firebase.js +# Secrets / local-only server init +/server/firebase.js
30-33: Scope check: .gitignore changes vs. “Footer mobile-friendly” PRThis backend ignore tweak seems unrelated to the UI responsiveness objective. Consider splitting into a tiny housekeeping PR or note it in the description for traceability.
| server/firebase.js | ||
|
|
||
| server/firebase.js |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify server/firebase.js was never committed; rotate if it was
If the file was ever tracked, adding to .gitignore won’t remove it from history. Check and purge/rotate secrets if needed.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Is it currently tracked?
if git ls-files --error-unmatch server/firebase.js >/dev/null 2>&1; then
echo "Tracked: server/firebase.js"
echo "Run: git rm --cached server/firebase.js && git commit -m 'remove secret from repo'"
else
echo "Not tracked in index."
fi
# Was it ever committed?
if git log -n 1 --pretty=format:%H -- server/firebase.js >/dev/null 2>&1; then
echo "Found in history. Consider BFG/git-filter-repo to purge and rotate keys."
else
echo "No history entries found."
fiLength of output: 94
Purge server/firebase.js from repository history and rotate any exposed credentials.
server/firebase.js is not tracked in the index but is present in git history.
- List commits referencing the file: git log --pretty=format:'%h %ad %an %s' -- server/firebase.js
- Remove from history (recommended): git clone --mirror .git && cd .git && git filter-repo --path server/firebase.js --invert-paths && git push --force --all && git push --force --tags (or use BFG).
- Rotate/revoke any API keys or service-account credentials that were in that file.
🤖 Prompt for AI Agents
.gitignore lines 31-33 reference server/firebase.js — the file exists in
repository history and may contain secrets; list all commits referencing it
(e.g., via git log for that path), then permanently remove it from history using
a tool like git filter-repo or BFG (mirror-clone the repo, remove the path, and
force-push all refs and tags), and finally rotate/revoke any API keys or
service-account credentials that were exposed in that file; after cleanup,
inform the team and update any CI/secret stores with new credentials.
|
@pushh-k and @CoderUzumaki kindly review this PR. Could you please add the GSSoC labels to this PR? |
|
Hi, thanks for the contribution! 🙌 A few requests before we proceed: Recording PR Template Thanks! |
|
I have mentioned issue number, updated my PR to following PR template and attached screenshots of the result. @pushh-k |
CoderUzumaki
left a comment
CoderUzumaki
left a comment
There was a problem hiding this comment.
Requested Changes
Firebase Admin Initialization: Please do not re-import/change import or re-initialize Firebase Admin in other files. It must be initialized only once (as it was before). Revert this change to prevent duplicate app errors.
Duplicate start Script: There is a duplicate "start" script in package.json. Please remove the redundancy and keep only one entry.
Once these are addressed, the PR will be good to go
|
i have resolved all issues. Please check now @CoderUzumaki |
Initialize Firebase Admin SDK with service account credentials.
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)
server/index.js (2)
6-16: Load env before importing modules that use it.If
./firebase.jsreads env at import time, callingdotenv.config()afterward won’t populate variables, leading to init failures. Preload via side‑effect import at the very top and drop the manualconfig()call.Apply this diff:
-import express from "express"; -import cors from "cors"; -import dotenv from "dotenv"; +import "dotenv/config"; +import express from "express"; +import cors from "cors"; import connectDB from "./config/db.js"; //import multer from "multer"; // Import Firebase Admin instance import admin from "./firebase.js"; -// Load environment variables -dotenv.config(); +// Load environment variables (preloaded via import "dotenv/config")
1-68: Fix Firebase init/order, missing firebase module, and auth-middleware mismatch
- Move dotenv.config() to the very top of server/index.js (before importing anything that may read process.env).
- server/index.js imports "./firebase.js" but server/firebase.js was not found — either add/restore that module or correct the import path so a single firebase admin instance is exported.
- admin.initializeApp(...) is currently in server/index.js — centralize Firebase Admin initialization in the single firebase module (remove any duplicate in index.js). (server/index.js ~lines 29, 43–47)
- Auth property mismatch: the protected middleware in server/index.js sets req.user but the codebase uses req.firebaseUser (see server/middleware/firebaseAuthMiddleware.js and routes: server/routes/authRoutes.js, server/routes/interviewRoutes.js, server/routes/reportRoutes.js). Unify on one property (recommend req.firebaseUser) and update the protected middleware accordingly.
🧹 Nitpick comments (3)
server/index.js (3)
21-25: Make CORS origin configurable.Hard‑coding prod origin blocks local dev and previews. Read from env with a sane default.
-app.use(cors({ - origin: 'https://prepedgeai.vercel.app', +app.use(cors({ + origin: process.env.CORS_ORIGIN || 'https://prepedgeai.vercel.app', credentials: true, }));
49-53: Start server after a successful DB connection (fail fast otherwise).Currently the server can accept requests before Mongo connects.
-// --- Connect to database --- -connectDB() - .then(() => console.log("Database connected successfully")) - .catch((error) => console.error("Database connection failed:", error)); - -// --- Start server --- -const PORT = process.env.PORT || 5000; -app.listen(PORT, () => console.log(`Server running on port ${PORT}`)); +// --- Start server after DB --- +const PORT = process.env.PORT || 5000; +const start = async () => { + try { + await connectDB(); + console.log("Database connected successfully"); + app.listen(PORT, () => console.log(`Server running on port ${PORT}`)); + } catch (error) { + console.error("Database connection failed:", error); + process.exit(1); + } +}; +start();Also applies to: 65-67
28-41: Token format hardening (optional).Guard against malformed Authorization headers to avoid
.splitsurprises and send a clearer 401.-const token = req.headers.authorization?.split(" ")[1]; +const auth = req.headers.authorization || ""; +const [scheme, token] = auth.split(" "); +if (scheme !== "Bearer" || !token) { + return res.status(401).json({ error: "Unauthorized" }); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/index.js(2 hunks)server/package.json(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/index.js (2)
server/routes/authRoutes.js (2)
req(14-14)req(36-36)server/config/db.js (1)
connectDB(3-12)
🔇 Additional comments (2)
server/package.json (2)
7-13: Scripts block fixed — duplicate "start" removed.JSON now parses cleanly and tooling won’t choke on duplicate keys.
34-37: Good move: nodemon to devDependencies.Keeps production install slim and avoids accidental prod dependency on dev tooling.
server/index.js
Outdated
| // --- Protected route middleware --- | ||
| 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; | ||
| next(); | ||
| } catch (err) { | ||
| res.status(401).json({ error: "Invalid token" }); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Middleware user property mismatch with routes.
authRoutes.js references req.firebaseUser, but this middleware sets req.user. Set both (or standardize) to avoid subtle breakage.
Apply this diff:
try {
const decoded = await admin.auth().verifyIdToken(token);
- req.user = decoded;
+ req.firebaseUser = decoded;
+ req.user = decoded; // backward compatible
next();Also confirm /api/protected actually protects any routes; currently it’s a mount point without handlers beneath it. If you intended a reusable middleware, export it and apply per‑route.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "---- check for ripgrep ----"
if command -v rg >/dev/null 2>&1; then
RG="rg -n --hidden -S"
else
RG=""
fi
echo "=== Search: req.firebaseUser ==="
if [ -n "$RG" ]; then
$RG '\breq\.firebaseUser\b' || true
else
grep -RIn --exclude-dir=node_modules 'req.firebaseUser' || true
fi
echo "=== Search: req.user ==="
if [ -n "$RG" ]; then
$RG '\breq\.user\b' || true
else
grep -RIn --exclude-dir=node_modules 'req.user' || true
fi
echo "=== Search: authRoutes.js filename ==="
if [ -n "$RG" ]; then
$RG 'authRoutes\.js' || true
else
grep -RIn --exclude-dir=node_modules 'authRoutes.js' || true
fi
echo "=== Search: /api/protected ==="
if [ -n "$RG" ]; then
$RG '/api/protected' || true
else
grep -RIn --exclude-dir=node_modules '/api/protected' || true
fi
echo "=== Show server/index.js (if exists) around lines 1-240 ==="
if [ -f server/index.js ]; then
sed -n '1,240p' server/index.js || true
else
echo "server/index.js not found"
fi
echo "=== If authRoutes.js found, print file(s) ==="
if [ -n "$RG" ]; then
for f in $($RG -l 'authRoutes\.js' || true); do
echo "---- $f ----"
sed -n '1,240p' "$f" || true
done
else
for f in $(grep -RIl --exclude-dir=node_modules 'authRoutes.js' || true); do
echo "---- $f ----"
sed -n '1,240p' "$f" || true
done
fi
echo "=== done ==="Length of output: 4682
Standardize auth payload and apply middleware correctly
- Issue: routes expect req.firebaseUser (server/routes/authRoutes.js, server/routes/interviewRoutes.js, server/routes/reportRoutes.js) but the inline middleware in server/index.js sets only req.user; server/middleware/firebaseAuthMiddleware.js already sets req.firebaseUser.
- Fix: set both for backward compatibility (or standardize to one name). Apply this diff in server/index.js if keeping the inline middleware:
try {
const decoded = await admin.auth().verifyIdToken(token);
- req.user = decoded;
+ req.firebaseUser = decoded;
+ req.user = decoded; // backward compatible
next();- Also: app.use("/api/protected", ...) in server/index.js is a mount point that currently protects no mounted routes. Either export the middleware (use server/middleware/firebaseAuthMiddleware.js) and apply it to the routes you want protected (e.g., app.use("/api/interview", firebaseAuthMiddleware, interviewRoutes)) or mount handlers under /api/protected.
📝 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.
| // --- Protected route middleware --- | |
| 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; | |
| next(); | |
| } catch (err) { | |
| res.status(401).json({ error: "Invalid token" }); | |
| } | |
| }); | |
| // --- Protected route middleware --- | |
| 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.firebaseUser = decoded; | |
| req.user = decoded; // backward compatible | |
| next(); | |
| } catch (err) { | |
| res.status(401).json({ error: "Invalid token" }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In server/index.js around lines 28-41, the inline auth middleware only sets
req.user and is mounted at /api/protected (which currently protects no actual
routes) while the app expects req.firebaseUser; update the middleware to set
both req.user and req.firebaseUser for backward compatibility (or choose one
name and update callers), and stop using the ad-hoc inline middleware — instead
import and use the existing server/middleware/firebaseAuthMiddleware (or export
the inline function) and apply it to the actual route mounts that need
protection (e.g., app.use("/api/interview", firebaseAuthMiddleware,
interviewRoutes)) or alternatively mount your route handlers under
/api/protected so the middleware actually protects them.
CoderUzumaki
left a comment
CoderUzumaki
left a comment
There was a problem hiding this comment.
The /api/protected middleware is currently unused since none of the registered routes (/api/auth, /api/interview, /api/report, /api/contact) are mounted under it. This makes it effectively dead code.
If you intend to keep it, then all protected routes should be explicitly mounted under /api/protected/... and corresponding API calls in the frontend updated accordingly. Otherwise, since protection is already being handled on the frontend, you can simply remove this middleware block to avoid confusion and redundancy.
|
Please check now @CoderUzumaki its done now. |
|
@CoderUzumaki @pushh-k please merge it if it has no issues. and let me know if it has issues |
3 participants
Related Issue
Closes #85
Description
This PR improves the mobile responsiveness of the website footer. The layout and text scaling have been adjusted for small screens to ensure proper readability and alignment across devices. Also, I have added mobile responsiveness to CreateInterview form.
Type of Change:
Changes Made:
Testing:
Screenshots:
Notes:
Checklist
Contributor @namami-13
Summary by CodeRabbit
New Features
Refactor
Style
Bug Fixes
Chores