feat: enhance approvals page with approved papers and update API for …#17
feat: enhance approvals page with approved papers and update API for …#17harshchill wants to merge 1 commit into
Conversation
…distinct institutes
There was a problem hiding this comment.
Nemesis Review Report
Summary
🟡 Verdict: Needs Attention
This PR needs some TLC to handle errors, validate inputs, and prevent potential performance issues — let's get it ship-shape before merging.
📝 What This PR Does
This PR enhances the approvals page with approved papers and updates the API for fetching papers. The changes aim to improve the user experience and system functionality by providing a more comprehensive view of approved papers and streamlining the paper fetching process.
🏗️ Key Changes
- Auth flow updated to use session tokens and handle errors for admin actions
- Approvals page enhanced with approved papers and error handling for paper fetching
- API updated for paper fetching with error handling and input validation
🚨 Must Fix Before Merge
- Handle unhandled errors in adminActions.js, ApprovalsClient.js, page.js, route.js, and upload/page.js to prevent application crashes
- Validate inputs for paper fetching and handle potential performance issues in adminActions.js and route.js
- Fix undefined variables and functions in route.js and upload/page.js to prevent runtime errors
- Add error handling for fetch API errors and HTTP errors in upload/page.js to handle unexpected responses
app/actions/adminActions.js
💀 Line 238 — Unhandled error when connecting to the database or requiring admin, which can cause the application to crash.
↳ Fix:
try {
await requireAdmin();
await connectDB();
} catch (error) {
// handle error
}
↳ Fix:
try {
const papers = await Paper.find({ status: "approved" })
// ...
} catch (error) {
// handle error
}🟡 Line 238 — Potential performance issue if a large number of approved papers exist, as all papers are retrieved and sorted.
↳ Fix: Consider adding pagination or limiting the number of papers retrieved.
🟡 Line 242 — Missing validation for the "status" field in the Paper.find query, which can cause unexpected results if the field is missing or has an incorrect value.
↳ Fix: Add validation for the "status" field before querying.
↳ Fix:
try {
await requireAdmin();
} catch (error) {
// handle error
}app/admin/approvals/ApprovalsClient.js
useEffect hook when setting initial selected paper.
↳ Fix:
useEffect(() => {
if (!selectedPaper && papers.length > 0) {
try {
setSelectedPaper(papers[0]);
} catch (error) {
console.error("Error setting initial selected paper:", error);
}
}
}, [papers, selectedPaper]);handleAction function.
↳ Fix:
const handleAction = async (status) => {
if (!selectedPaper) return;
try {
// existing code
} catch (error) {
console.error("Error handling action:", error);
}
};handleUpdate function.
↳ Fix:
const handleUpdate = async (e) => {
e.preventDefault();
if (!editPaper) return;
setModalLoading(true);
try {
// existing code
} catch (error) {
console.error("Error updating paper:", error);
} finally {
setModalLoading(false);
}
};handleDelete function.
↳ Fix:
const handleDelete = async () => {
if (!deletePaperId) return;
setModalLoading(true);
try {
// existing code
} catch (error) {
console.error("Error deleting paper:", error);
} finally {
setModalLoading(false);
}
};🟡 Line 29 — Potential performance issue with useMemo hook.
↳ Fix: Consider adding a dependency array to the useMemo hook to prevent unnecessary recalculations.
app/admin/approvals/page.js
getApprovedPapersForMatching call.
↳ Fix: const approvedPapers = await getApprovedPapersForMatching().catch((error) => { console.error(error); return []; });
getPendingPapers call.
↳ Fix: const pendingPapers = await getPendingPapers().catch((error) => { console.error(error); return []; });
🟡 Line 20 — Missing validation for approvedPapers before passing to ApprovalsClient.
↳ Fix: if (!approvedPapers) return <ApprovalsClient initialPapers={pendingPapers} />;
🟡 Line 20 — Missing validation for pendingPapers before passing to ApprovalsClient.
↳ Fix: if (!pendingPapers) return <ApprovalsClient approvedPapers={approvedPapers} />;
🟡 Line 15 — Potential memory leak if ApprovalsPage is re-rendered multiple times.
↳ Fix:
useEffect(() => {
const pendingPapers = await getPendingPapers();
const approvedPapers = await getApprovedPapersForMatching();
// ...
}, []);app/api/papers/route.js
💀 Line 240 — The normalizeString function is not defined in this scope, which could lead to a runtime error.
↳ Fix: Import or define the normalizeString function.
distinct parameter is not validated for empty strings, which could lead to unexpected behavior.
↳ Fix: Add a check for empty strings, e.g., if (distinct && distinct !== "").
↳ Fix:
return NextResponse.json(
{ error: { message: "Invalid distinct field. Use distinct=institute." } },
{ status: 400 }
);🟡 Line 291 — The Paper.distinct method is not awaited in a try-catch block, which could lead to unhandled errors.
↳ Fix:
try {
const institutes = await Paper.distinct("institute", filter);
// ...
} catch (error) {
// Handle the error
}🟡 Line 294 — The normalized array is not checked for null or undefined values before calling sort, which could lead to a runtime error.
↳ Fix: Add a null check before calling sort, e.g., if (normalized) normalized.sort(...);
app/page.js
↳ Fix: <Link href="/user/auth" ...>
↳ Fix:
<Link href="/user/auth" className="group relative inline-flex items-center justify-center px-8 py-3.5 text-base font-semibold text-white bg-teal-600 rounded-full overflow-hidden transition-all hover:scale-105 hover:shadow-xl hover:shadow-teal-600/20 focus:outline-none focus:ring-2 focus:ring-teal-600 focus:ring-offset-2">🟡 Line 81 — The link to "#features" is not wrapped in a conditional statement, which may cause issues if the features section does not exist.
↳ Fix:
{featuresExist && <Link href="#features" className="px-8 py-3.5 text-base font-semibold text-slate-600 hover:text-slate-900 transition-colors">}🟡 Line 70 — The isAuthenticated variable is not checked for null or undefined, which may cause errors if it is not properly initialized.
↳ Fix:
{isAuthenticated !== null && isAuthenticated !== undefined && isAuthenticated ? (🟡 Line 74 — The link to "/user/papers" for unauthenticated users does not check if the user has permission to access the page, which may cause security issues.
↳ Fix:
{hasPermission && <Link href="/user/papers" ...>}app/user/upload/page.js
fetchInstitutes function if response.json() fails.
↳ Fix: const data = await response.json().catch((err) => console.error('Failed to parse JSON:', err));
useEffect hook for fetchInstitutes function.
↳ Fix: useEffect(() => { ... }, [formData.program]);
🟡 Line 454 — Missing validation for instituteOptions before rendering datalist.
↳ Fix:
{instituteOptions && instituteOptions.length > 0 && (
<datalist id="institute-options">
{instituteOptions.map((institute) => (
<option key={institute} value={institute} />
))}
</datalist>
)}🟡 Line 239 — No error handling for fetch API errors other than JSON parsing errors.
↳ Fix:
const response = await fetch('/api/papers?distinct=institute').catch((err) => {
console.error('Failed to fetch institute options:', err);
return { ok: false };
});response.status to handle HTTP errors.
↳ Fix:
if (response.ok && response.status === 200 && data.success && Array.isArray(data.institutes)) {
setInstituteOptions(data.institutes);
} else {
console.error('Failed to fetch institute options:', response.status);
}public/sw.js
✅ No issues found.
Issued by Nemesis
…distinct institutes
Description
What does this PR do? Provide a clear summary of the changes.
Related Issues
Closes #(issue number)
Type of Change
Checklist
fix/,feat/,docs/, etc.)npm run lintpasses without errorsconsole.log()statements left in codeScreenshots (if applicable)
Add screenshots of UI changes here.