verification page and updated packages#172
Conversation
Certificate verification enabled
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Adds a certificate verification feature to the Next.js app, including new routes and UI to validate certificate authenticity via the backend API, plus a small dependency update.
Changes:
- Added
/verifyand/verify/[id]pages that render a newVerifyPagecomponent and support verifying by URL parameter. - Introduced a new API endpoint helper for certificate verification (
API_ENDPOINTS.CERTIFICATES.VERIFY). - Updated sitemap entries to include
/verifyand refreshedlastmodtimestamps; bumped@vercel/analyticsversion.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/config.js | Adds CERTIFICATES.VERIFY endpoint builder used by the new verification UI. |
| public/sitemap-0.xml | Adds /verify to the sitemap and updates lastmod timestamps. |
| pages/verify/[id].js | New SSR route that passes certificateId into the verification UI. |
| pages/verify.js | New base verification route (manual entry flow). |
| package.json | Updates @vercel/analytics dependency version. |
| components/Verify/VerifyPage.jsx | Implements the verification UI and client-side fetch to the verify endpoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const handleSubmit = async (event) => { | ||
| event.preventDefault(); | ||
|
|
||
| const trimmed = queryId.trim(); | ||
| if (!trimmed) { | ||
| return; | ||
| } | ||
|
|
||
| await router.push(`/verify/${encodeURIComponent(trimmed)}`, undefined, { | ||
| shallow: true, | ||
| }); | ||
| await verifyCertificate(trimmed); | ||
| }; |
There was a problem hiding this comment.
handleSubmit calls router.push and then immediately calls verifyCertificate. Since a route change to /verify/[id] (or a param change on the same page) will also trigger the effect that calls verifyCertificate, this can result in duplicate verification requests and UI flicker. Consider making the URL change the single source of truth (push only, let the effect run the verification), and note that shallow: true won’t apply when navigating between different pages (/verify -> /verify/[id]).
| useEffect(() => { | ||
| const routeId = initialCertificateId || router.query.id; | ||
| if (typeof routeId === "string" && routeId.trim()) { | ||
| setQueryId(routeId); | ||
| verifyCertificate(routeId); | ||
| } | ||
| }, [initialCertificateId, router.query.id]); |
There was a problem hiding this comment.
The useEffect that auto-verifies depends on both initialCertificateId and router.query.id, but it always prefers initialCertificateId via initialCertificateId || router.query.id. On the /verify/[id] page, router.query.id typically becomes available after hydration, so this effect can run twice and re-verify the same ID. A common fix is to gate on router.isReady and/or track the last verified ID in a ref so you only call verifyCertificate when the effective ID actually changes.
| useEffect(() => { | ||
| const routeId = initialCertificateId || router.query.id; | ||
| if (typeof routeId === "string" && routeId.trim()) { | ||
| setQueryId(routeId); | ||
| verifyCertificate(routeId); | ||
| } | ||
| }, [initialCertificateId, router.query.id]); |
There was a problem hiding this comment.
Hook linting: verifyCertificate is referenced inside this useEffect, but it isn’t listed in the dependency array, which will trigger react-hooks/exhaustive-deps under next lint. Consider wrapping verifyCertificate in useCallback (with stable deps) and adding it to the dependency list, or moving the verification logic into the effect so the deps are correct.
No description provided.