Repository navigation
Update the carrers, roadmap, reviews - #7
Merged
Merged
Conversation
PR Summary by QodoFix official course roadmaps and review rendering
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
There was a problem hiding this comment.
🟡 Changes recommended
The backend change introduces a potential per-request full-course-list scan via getOfficialCourseDetail(), and the roadmap page effect dependencies risk using a stale hydrated profile.stream value.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the roadmap and reviews pages, and changes the backend roadmap-generation route to source course metadata from the official handbook query helper rather than the previous degree-programmes tables.
Changes:
- Frontend roadmap page: improves toast error messaging and removes unused language/target-career UI.
- Frontend reviews page: switches to
reviewerName, improves verified badge accessibility markup, and removes pros/cons rendering. - Backend roadmap generation: uses
getOfficialCourseDetail()and derivesdurationYearsfrom the officialdurationstring.
File summaries
| File | Description |
|---|---|
| frontend/src/pages/roadmap.tsx | Updates roadmap generation UX/error handling and removes unused language/target career display. |
| frontend/src/pages/reviews.tsx | Uses reviewerName, adjusts verified badge accessibility, and simplifies review content. |
| backend/src/routes/roadmaps.ts | Switches roadmap generation to official handbook course detail and parses duration from strings. |
| frontend/pnpm-lock.yaml | Aligns lockfile settings (autoInstallPeers: false) with repo .npmrc. |
| backend/pnpm-lock.yaml | Aligns lockfile settings (autoInstallPeers: false) with repo .npmrc. |
Review details
Files not reviewed (2)
- backend/pnpm-lock.yaml: Generated file
- frontend/pnpm-lock.yaml: Generated file
- Files reviewed: 3/5 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| eq(degreeProgrammesTable.universityId, universitiesTable.id), | ||
| ) | ||
| .where(eq(degreeProgrammesTable.id, courseId)); | ||
| const courseRow = await getOfficialCourseDetail(courseId, { district: "All Island" }); |
Comment on lines
107
to
111
| if (selectedCourseId && !roadmap) { | ||
| const langName = language === "si" ? "Sinhala" : language === "ta" ? "Tamil" : "English"; | ||
| generateRoadmap({ | ||
| data: { | ||
| courseId: parseInt(selectedCourseId), | ||
| stream: profile.stream || undefined, |
Comment on lines
2
to
+3
| import { db } from "../db"; | ||
| import { degreeProgrammesTable, universitiesTable, roadmapsTable } from "../db"; | ||
| import { eq } from "drizzle-orm"; | ||
| import { roadmapsTable } from "../db"; |
Comment on lines
+84
to
+88
| onError: (error) => { | ||
| const description = | ||
| error instanceof ApiError && error.status === 404 | ||
| ? "This course could not be found. Please pick a different course." | ||
| : error instanceof ApiError && error.status === 429 |
Code Review by Qodo
1. Saved roadmaps lose course identity
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| eq(degreeProgrammesTable.universityId, universitiesTable.id), | ||
| ) | ||
| .where(eq(degreeProgrammesTable.id, courseId)); | ||
| const courseRow = await getOfficialCourseDetail(courseId, { district: "All Island" }); |
There was a problem hiding this comment.
1. Saved roadmaps lose course identity 🐞 Bug ≡ Correctness
The /roadmaps/generate handler now stores the official-handbook courseId, but /roadmaps still joins that value to the unrelated legacy degree-programme primary key. For authenticated generations whose independent numeric IDs do not coincide, the saved-roadmap response returns a null or unrelated degreeName, violating its string contract and leaving the dashboard entry blank or mislabeled.
Agent Prompt
## Issue description
Generated roadmaps now use official-handbook course IDs, while the saved-roadmaps route resolves names by joining those IDs to the legacy degree-programme table. Update saved roadmap retrieval so course identity and names use the same official ID namespace as generation.
## Issue Context
Official course IDs are derived from handbook `uni_code` values and are not degree-programme primary keys. The dashboard displays the top-level `degreeName` returned by the saved-roadmaps endpoint.
## Fix Focus Areas
- backend/src/routes/roadmaps.ts[197-250]
- backend/src/routes/saved.ts[198-223]
- backend/src/db/official-handbook-query.ts[132-134]
- frontend/src/pages/dashboard.tsx[261-270]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| eq(degreeProgrammesTable.universityId, universitiesTable.id), | ||
| ) | ||
| .where(eq(degreeProgrammesTable.id, courseId)); | ||
| const courseRow = await getOfficialCourseDetail(courseId, { district: "All Island" }); |
There was a problem hiding this comment.
2. Roadmaps slow as the catalog grows 🐞 Bug ➹ Performance
The generation handler calls getOfficialCourseDetail, whose implementation fetches every official course and scans the resulting array to find one ID. Each generation therefore runs two correlated cutoff lookups for every catalog row before making the AI request, increasing database work and endpoint latency with the entire catalog rather than the selected course.
Agent Prompt
## Issue description
Roadmap generation now obtains one course through a helper that queries and materializes the complete official catalog. Add or use a targeted official-course query so generation loads only the selected course and its required details.
## Issue Context
`fetchCourseRows` has no course-ID predicate and contains two correlated cutoff subqueries. `getOfficialCourseDetail` then searches the complete result in application code.
## Fix Focus Areas
- backend/src/routes/roadmaps.ts[197-205]
- backend/src/db/official-handbook-query.ts[272-323]
- backend/src/db/official-handbook-query.ts[375-391]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.