Skip to content

feat: enhance approvals page with approved papers and update API for …#17

Open
harshchill wants to merge 1 commit into
mainfrom
SEO-specific
Open

feat: enhance approvals page with approved papers and update API for …#17
harshchill wants to merge 1 commit into
mainfrom
SEO-specific

Conversation

@harshchill

Copy link
Copy Markdown
Owner

…distinct institutes

Description

What does this PR do? Provide a clear summary of the changes.

Related Issues

Closes #(issue number)

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔧 Breaking change (fix or feature that would cause existing functionality to change)
  • 📚 Documentation update

Checklist

  • Branch name follows convention (fix/, feat/, docs/, etc.)
  • Commits follow conventional commit format
  • npm run lint passes without errors
  • Tested locally and works as expected
  • Updated README or documentation if needed
  • No hardcoded secrets or environment variables
  • No debug console.log() statements left in code

Screenshots (if applicable)

Add screenshots of UI changes here.

@nemesis-chill nemesis-chill Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
}

⚠️ Line 242 — Missing error handling for the Paper.find query, which can cause the application to crash if the query fails.
↳ 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.

⚠️ Line 238 — The requireAdmin function is not awaited in a try-catch block, which can cause the application to crash if the function throws an error.
↳ Fix:

try {
  await requireAdmin();
} catch (error) {
  // handle error
}

app/admin/approvals/ApprovalsClient.js

⚠️ Line 43 — Unhandled error in 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]);

⚠️ Line 75 — Missing error handling in handleAction function.
↳ Fix:

const handleAction = async (status) => {
  if (!selectedPaper) return;
  try {
    // existing code
  } catch (error) {
    console.error("Error handling action:", error);
  }
};

⚠️ Line 123 — Unhandled error in 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);
  }
};

⚠️ Line 143 — Unhandled error in 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

⚠️ Line 17 — Unhandled error for getApprovedPapersForMatching call.
↳ Fix: const approvedPapers = await getApprovedPapersForMatching().catch((error) => { console.error(error); return []; });

⚠️ Line 17 — Unhandled error for 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.

⚠️ Line 279 — The 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 !== "").

⚠️ Line 283 — The error response does not follow a standard error format, which could make it difficult for clients to handle.
↳ 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

⚠️ Line 74 — Unauthenticated users are being redirected to "/user/papers" instead of the authentication page.
↳ Fix: <Link href="/user/auth" ...>

⚠️ Line 74 — The link for unauthenticated users has the same styling as authenticated links, which may cause confusion.
↳ 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

⚠️ Line 239 — Unhandled promise rejection in fetchInstitutes function if response.json() fails.
↳ Fix: const data = await response.json().catch((err) => console.error('Failed to parse JSON:', err));

⚠️ Line 236 — Missing dependency in 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 };
});

⚠️ Line 239 — No check for 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant