added little flag, updated card view matching hi-fi for programs, fix…#104
Merged
added little flag, updated card view matching hi-fi for programs, fix…#104
Conversation
…ed mapping of instruments in program table
alexy-ok
requested changes
Mar 26, 2026
Member
alexy-ok
left a comment
alexy-ok
left a comment
There was a problem hiding this comment.
Great work, really impressed with how clean the backend changes were. Just a few small changes 🤠
| status: row.status, | ||
| launchDate: row.launchDate, | ||
| location: row.countryName ?? '', | ||
| location: row.countryName + ', ' + row.regionName, |
Member
There was a problem hiding this comment.
fix: change this to city, country as seen in hi-fi
| 'Program Directors', | ||
| 'Curriculum Links', | ||
| ]; | ||
| console.log(p.instrumentsMap); |
Comment on lines
+131
to
+135
| <Td> | ||
| <span class={`fi fi-${p.iso_code?.toLowerCase()}`}></span> | ||
| {' '} | ||
| {p.location} | ||
| </Td> |
Member
There was a problem hiding this comment.
This isn't working on my end for some reason, could you guys confirm that it's working for both of you
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
12 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/src/components/dashboard/ProgramTable.jsx">
<violation number="1" location="client/src/components/dashboard/ProgramTable.jsx:75">
P2: Remove debug `console.log` statements. These fire once per program row and will clutter the console in production.</violation>
<violation number="2" location="client/src/components/dashboard/ProgramTable.jsx:117">
P2: If `row.programLocation` or `row.regionName` is null/undefined, this produces `"undefined, ..."` or `"..., undefined"` in the UI. Add null checks similar to `mapAdminRow`'s pattern.</violation>
<violation number="3" location="client/src/components/dashboard/ProgramTable.jsx:746">
P1: Bug: `.catch(() => ({ data }))` references an undefined variable `data`. This will throw a `ReferenceError` when the catch triggers, breaking the entire fetch. Should be `{ data: [] }` to match the other catch handlers.</violation>
</file>
<file name="server/routes/country.js">
<violation number="1" location="server/routes/country.js:48">
P2: Add a `regionName.length === 0` check to return 404 when the country (or its region) isn't found, consistent with the `/:id` endpoint above.</violation>
<violation number="2" location="server/routes/country.js:48">
P2: Use `200` instead of `201` for a GET endpoint. `201 Created` is meant for POST/PUT responses that create a resource; every other GET handler in this file correctly returns `200`.</violation>
</file>
<file name="server/routes/program.js">
<violation number="1" location="server/routes/program.js:111">
P1: The `languages` column was added to the INSERT column list but the VALUES clause was not updated to include `$12`. The query has 12 placeholder-needing columns but only 11 placeholders (`$1`–`$11`), causing a column count mismatch that will error at runtime.</violation>
<violation number="2" location="server/routes/program.js:190">
P0: Missing comma between `launch_date = COALESCE($10, launch_date)` and `languages = COALESCE($11, languages)` in the SET clause. This SQL syntax error will crash the PUT `/:id` endpoint at runtime.</violation>
<violation number="3" location="server/routes/program.js:458">
P1: `result[0].name` will throw a `TypeError` if no matching record is found (empty result set). Add an empty-result guard, consistent with other routes in this file.</violation>
</file>
<file name="client/src/components/dashboard/CardView.jsx">
<violation number="1" location="client/src/components/dashboard/CardView.jsx:35">
P2: Hardcoded `repeat(5, 1fr)` removes the responsive breakpoints the old code had. Combined with `minW={324}` on each card, this will overflow on smaller viewports. Restore responsive column definitions.</violation>
<violation number="2" location="client/src/components/dashboard/CardView.jsx:119">
P2: Typo: `"55x"` is not a valid CSS unit — should be `"55px"`.</violation>
<violation number="3" location="client/src/components/dashboard/CardView.jsx:310">
P2: `p.students` is used for both "Total" and "Graduated" counts, so they always display the same number. The graduated count should likely reference a distinct field.</violation>
<violation number="4" location="client/src/components/dashboard/CardView.jsx:364">
P1: A single `isFlipped` boolean controls the entire grid, so clicking any card flips every card simultaneously. Move the flip state into a per-card wrapper component so each card flips independently.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| [id] | ||
| ); | ||
|
|
||
| res.status(201).json(keysToCamel(regionName)); |
Contributor
There was a problem hiding this comment.
P2: Add a regionName.length === 0 check to return 404 when the country (or its region) isn't found, consistent with the /:id endpoint above.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/routes/country.js, line 48:
<comment>Add a `regionName.length === 0` check to return 404 when the country (or its region) isn't found, consistent with the `/:id` endpoint above.</comment>
<file context>
@@ -34,6 +34,24 @@ countryRouter.get('/:id', async (req, res) => {
+ [id]
+ );
+
+ res.status(201).json(keysToCamel(regionName));
+ } catch (err) {
+ console.error(err);
</file context>
Suggested change
| res.status(201).json(keysToCamel(regionName)); | |
| if (regionName.length === 0) { | |
| return res.status(404).send('Item not found'); | |
| } | |
| res.status(200).json(keysToCamel(regionName)); |
| <HStack height="25px"> | ||
| <Text>{p.students} Total</Text> | ||
| <Divider orientation="vertical" /> | ||
| <Text>{p.students} Graduated</Text> |
Contributor
There was a problem hiding this comment.
P2: p.students is used for both "Total" and "Graduated" counts, so they always display the same number. The graduated count should likely reference a distinct field.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/components/dashboard/CardView.jsx, line 310:
<comment>`p.students` is used for both "Total" and "Graduated" counts, so they always display the same number. The graduated count should likely reference a distinct field.</comment>
<file context>
@@ -57,61 +291,95 @@ const CardView = ({ data, openEditForm }) => {
+ <HStack height="25px">
+ <Text>{p.students} Total</Text>
+ <Divider orientation="vertical" />
+ <Text>{p.students} Graduated</Text>
+ </HStack>
+
</file context>
Contributor
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/src/components/dashboard/CardView.jsx">
<violation number="1" location="client/src/components/dashboard/CardView.jsx:173">
P2: Clicking a playlist link will also flip the card back due to event bubbling from the `Link` to the `Card`'s `onClick`. Add `e.stopPropagation()` on the link's `onClick` to prevent the unintended flip.</violation>
<violation number="2" location="client/src/components/dashboard/CardView.jsx:284">
P1: Clicking the edit button will also flip the card due to event bubbling. The click event propagates from the `IconButton` up to the `Card`'s `onClick`, triggering both `openEditForm` and the flip. Add `e.stopPropagation()` to prevent the card from flipping when editing.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…/github.com/ctc-uci/gcf into GCF96-Add-Little-Flag-and-Program-Card-View
alexy-ok
approved these changes
Apr 4, 2026
Member
Contributor
|
@alexy-ok I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
7 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/routes/country.js">
<violation number="1" location="server/routes/country.js:40">
P2: Missing empty-result check. If the country ID is invalid or has no region, this silently returns an empty array. Add a guard consistent with the `GET /:id` handler.</violation>
<violation number="2" location="server/routes/country.js:48">
P2: Use status `200` instead of `201` for a GET endpoint. `201 Created` is semantically incorrect for a read-only query.</violation>
</file>
<file name="client/src/components/profile/Profile.jsx">
<violation number="1" location="client/src/components/profile/Profile.jsx:111">
P2: `bio` is missing from the `formData` initialization and from the `setFormData` call in `fetchUserData`. The Bio edit field will always start empty, discarding any existing bio value from the server.</violation>
<violation number="2" location="client/src/components/profile/Profile.jsx:174">
P1: `handleSave` shows a success toast but never calls the backend to persist changes. The user's edits are silently lost after the apparent "save".</violation>
<violation number="3" location="client/src/components/profile/Profile.jsx:496">
P2: The preferred language display is hardcoded to `🇺🇸 English` instead of reflecting `formData.language`. After a user changes their language, this will still show English.</violation>
</file>
<file name="client/src/components/dashboard/CardView.jsx">
<violation number="1" location="client/src/components/dashboard/CardView.jsx:37">
P2: `br` is not a valid Chakra UI style prop and will be silently ignored. Use `borderRadius={20}` instead to apply the intended rounding.</violation>
<violation number="2" location="client/src/components/dashboard/CardView.jsx:308">
P1: Both "Total" and "Graduated" student counts render `p.students`, so they always display the same number. The "Graduated" text should use a distinct field (e.g., `p.graduatedStudents`) or be removed if that data isn't available yet.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| <HStack height="25px"> | ||
| <Text>{p.students} Total</Text> | ||
| <Divider orientation="vertical" /> | ||
| <Text>{p.students} Graduated</Text> |
Contributor
There was a problem hiding this comment.
P1: Both "Total" and "Graduated" student counts render p.students, so they always display the same number. The "Graduated" text should use a distinct field (e.g., p.graduatedStudents) or be removed if that data isn't available yet.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/components/dashboard/CardView.jsx, line 308:
<comment>Both "Total" and "Graduated" student counts render `p.students`, so they always display the same number. The "Graduated" text should use a distinct field (e.g., `p.graduatedStudents`) or be removed if that data isn't available yet.</comment>
<file context>
@@ -1,114 +1,390 @@
+ <HStack height="25px">
+ <Text>{p.students} Total</Text>
+ <Divider orientation="vertical" />
+ <Text>{p.students} Graduated</Text>
+ </HStack>
+
</file context>
Comment on lines
+40
to
+48
| const regionName = await db.query( | ||
| `SELECT r.name | ||
| FROM region as r | ||
| INNER JOIN country c ON c.region_id = r.id | ||
| WHERE c.id = $1`, | ||
| [id] | ||
| ); | ||
|
|
||
| res.status(201).json(keysToCamel(regionName)); |
Contributor
There was a problem hiding this comment.
P2: Missing empty-result check. If the country ID is invalid or has no region, this silently returns an empty array. Add a guard consistent with the GET /:id handler.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/routes/country.js, line 40:
<comment>Missing empty-result check. If the country ID is invalid or has no region, this silently returns an empty array. Add a guard consistent with the `GET /:id` handler.</comment>
<file context>
@@ -34,6 +34,24 @@ countryRouter.get('/:id', async (req, res) => {
+countryRouter.get('/:id/region', async (req, res) => {
+ try {
+ const { id } = req.params;
+ const regionName = await db.query(
+ `SELECT r.name
+ FROM region as r
</file context>
Suggested change
| const regionName = await db.query( | |
| `SELECT r.name | |
| FROM region as r | |
| INNER JOIN country c ON c.region_id = r.id | |
| WHERE c.id = $1`, | |
| [id] | |
| ); | |
| res.status(201).json(keysToCamel(regionName)); | |
| const regionName = await db.query( | |
| `SELECT r.name | |
| FROM region as r | |
| INNER JOIN country c ON c.region_id = r.id | |
| WHERE c.id = $1`, | |
| [id] | |
| ); | |
| if (regionName.length === 0) { | |
| return res.status(404).send('Item not found'); | |
| } | |
| res.status(200).json(keysToCamel(regionName)); |
| gap={6} | ||
| <Card | ||
| h="100%" | ||
| br={20} |
Contributor
There was a problem hiding this comment.
P2: br is not a valid Chakra UI style prop and will be silently ignored. Use borderRadius={20} instead to apply the intended rounding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/components/dashboard/CardView.jsx, line 37:
<comment>`br` is not a valid Chakra UI style prop and will be silently ignored. Use `borderRadius={20}` instead to apply the intended rounding.</comment>
<file context>
@@ -1,114 +1,390 @@
- gap={6}
+ <Card
+ h="100%"
+ br={20}
+ w="100%"
+ maxW="600px"
</file context>
Suggested change
| br={20} | |
| borderRadius={20} |
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/src/components/dashboard/ProgramForm/LocationForm.jsx">
<violation number="1" location="client/src/components/dashboard/ProgramForm/LocationForm.jsx:87">
P2: When `id` is falsy, `countriesList` is not cleared, so stale countries from a previous region may briefly appear in the dropdown when the user switches to a new region. Add an `else` branch to reset the list.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| const id = formState.regionId; | ||
| const response = await backend.get(`/region/${id}/countries`); | ||
| setCountriesList(response.data); | ||
| if (id) { |
Contributor
There was a problem hiding this comment.
P2: When id is falsy, countriesList is not cleared, so stale countries from a previous region may briefly appear in the dropdown when the user switches to a new region. Add an else branch to reset the list.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/components/dashboard/ProgramForm/LocationForm.jsx, line 87:
<comment>When `id` is falsy, `countriesList` is not cleared, so stale countries from a previous region may briefly appear in the dropdown when the user switches to a new region. Add an `else` branch to reset the list.</comment>
<file context>
@@ -84,8 +84,10 @@ export function LocationForm({ formState, setFormData }) {
const id = formState.regionId;
- const response = await backend.get(`/region/${id}/countries`);
- setCountriesList(response.data);
+ if (id) {
+ const response = await backend.get(`/region/${id}/countries`);
+ setCountriesList(response.data);
</file context>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
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
3 participants
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.
…ed mapping of instruments in program table
Description
Screenshots/Media
Issues
Closes #96