Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new localized Student Council page under Student Activities and wires up the supporting i18n translation types/strings, while also applying small safety tweaks to the Faculty & Staff pages to address build/lint issues.
Changes:
- Added
StudentCounciltranslation types and EN/HI translation objects. - Introduced
/student-activities/student-councilpage that queriesstudentCouncil+ related tables and renders groups/tables. - Hardened Faculty & Staff external-link handling (type guards / safer URL building) and adjusted Suspense key formatting.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| i18n/translations.ts | Extends the global Translations shape to include StudentCouncil. |
| i18n/translate/student-council.ts | Adds EN/HI strings and the StudentCouncilTranslations interface. |
| i18n/translate/index.ts | Re-exports the new student council translation module. |
| i18n/en.ts | Registers StudentCouncil translations in the English bundle. |
| i18n/hi.ts | Registers StudentCouncil translations in the Hindi bundle. |
| app/[locale]/student-activities/student-council/page.tsx | New DB-backed page rendering Student Council sections, notifications, and member tables. |
| app/[locale]/faculty-and-staff/utils.tsx | Adds runtime type guard before calling .startsWith on profile fields. |
| app/[locale]/faculty-and-staff/page.tsx | Small tweaks to Suspense key creation and external link URL building. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| instituteFunctionaries: 'Institute Functionaries', | ||
| coreCommittee: 'Core Committee', | ||
| nominatedStudents: 'Nominated Students', | ||
| studentsRepresentatives: `Student's Representatives`, |
There was a problem hiding this comment.
Heading text uses incorrect apostrophe/grammar: Student's Representatives implies a single student’s representatives. Consider changing this to Student Representatives or Students' Representatives.
| studentsRepresentatives: `Student's Representatives`, | |
| studentsRepresentatives: 'Student Representatives', |
| laboratoriesEn, | ||
| studentCouncilEn, | ||
| } from './translate'; | ||
| import { Label } from '@radix-ui/react-label'; |
There was a problem hiding this comment.
Label is imported but never used in this file, which will trigger lint/build errors. Please remove the unused import (or use it if it’s required).
| import { Label } from '@radix-ui/react-label'; |
| </div> | ||
| ) : ( | ||
| <p className="text-gray-500 mt-6 text-center"> | ||
| No nominated students found. |
There was a problem hiding this comment.
This empty-state message is hardcoded in English (No nominated students found.). Please move it into the StudentCouncil translations so it’s localized per locale.
| No nominated students found. | |
| {text.StudentCouncil.emptyStates.nominatedStudents} |
| // Get faculty employee IDs | ||
| const facultyPersonIds = facultyMembers.map((m) => m.personId); | ||
| const facultyData = | ||
| facultyPersonIds.length > 0 | ||
| ? await db.query.faculty.findMany({ | ||
| where: (faculty, { inArray }) => | ||
| inArray(faculty.id, facultyPersonIds), | ||
| columns: { id: true, employeeId: true }, | ||
| }) | ||
| : []; | ||
|
|
||
| // Create map for faculty designations | ||
| const facultyDesignationMap = new Map( | ||
| facultyMembers.map((m) => [m.personId, m.section]) | ||
| ); | ||
|
|
||
| // Prepare data for components | ||
| const facultyGroupData = facultyData.map((f) => ({ | ||
| employeeId: f.employeeId, | ||
| designation: facultyDesignationMap.get(f.id) ?? '', | ||
| })); |
There was a problem hiding this comment.
Redundant DB work: this page queries faculty to compute facultyGroupData, and then FICGroup queries faculty again to render details. Consider changing FICGroup to accept personIds (or accept fully-hydrated faculty/person rows) so the faculty table is queried only once.
| // Prepare data for core committee (students only) | ||
| const coreCommitteePersonIds = coreCommitteeMembers.map((m) => m.personId); | ||
| const coreCommitteeStudentData = | ||
| coreCommitteePersonIds.length > 0 | ||
| ? await db.query.students.findMany({ | ||
| where: (students, { inArray }) => | ||
| inArray(students.id, coreCommitteePersonIds), | ||
| columns: { id: true, rollNumber: true }, | ||
| }) | ||
| : []; | ||
|
|
||
| const coreCommitteeDesignationMap = new Map( | ||
| coreCommitteeMembers.map((m) => [m.personId, m.section]) | ||
| ); | ||
|
|
||
| const coreCommitteeGroupData = coreCommitteeStudentData.map((s) => ({ | ||
| rollNumber: s.rollNumber, | ||
| designation: coreCommitteeDesignationMap.get(s.id) ?? undefined, | ||
| })); | ||
|
|
There was a problem hiding this comment.
Redundant DB work: this page queries students to get roll numbers, and then StudentGroup queries students again by roll number to fetch person details. Consider updating StudentGroup to accept student/person IDs (or pre-fetched student+person data) so you don’t need the extra query per group.
| import StudentGroup from '~/components/student-group'; | ||
| import { getTranslations } from '~/i18n/translations'; | ||
| import { db } from '~/server/db'; | ||
|
|
There was a problem hiding this comment.
This page makes DB queries but does not set a revalidate value. Other DB-backed pages typically export export const revalidate = 300; to avoid fully dynamic rendering and reduce DB load. Please add a revalidate export near the top of this file.
| export const revalidate = 300; |
| locale={locale} | ||
| category="student-activities" | ||
| showViewAll={true} | ||
| viewAllHref={`/${locale}/notifications?category=miscellaneous`} |
There was a problem hiding this comment.
NotificationsPanel is filtered to category="student-activities", but the "View all" link points to category=miscellaneous, which will show the wrong set of notifications on /notifications. Update viewAllHref to use category=student-activities (or omit the category param if you want the unfiltered notifications page).
| viewAllHref={`/${locale}/notifications?category=miscellaneous`} | |
| viewAllHref={`/${locale}/notifications?category=student-activities`} |
| <p className="text-gray-500 mt-6 text-center"> | ||
| No core committee members found. | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
The empty-state message is hardcoded in English (No core committee members found.). Since this is a localized route, this should come from i18n translations so it renders correctly in Hindi/other locales.
| /> | ||
| ) : ( | ||
| <p className="text-gray-500 text-center"> | ||
| No student representatives found. |
There was a problem hiding this comment.
This empty-state message is hardcoded in English (No student representatives found.). Please use i18n translation strings instead so the page remains fully localized.
| No student representatives found. | |
| {text.StudentCouncil.emptyStates.noStudentRepresentatives} |
made a new branch to avoid conflicts.Modified the faculty and staff files because the were causing build to fail and eslint errors.