add telemetry and grafana links on my work page#43
Open
skand088 wants to merge 5 commits into
Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 25 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces telemetry links to the Our Work page. Environment variables for telemetry and Grafana URLs are defined, then a new TelemetryLinks component is rendered on the page with those URLs passed as configuration. ChangesTelemetry Links Integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/our-work/index.module.scss (1)
182-223: Add explicit:focus-visiblestyles for telemetry action links.Hover states are defined, but explicit keyboard focus treatment is missing. Adding it improves accessibility consistency.
♿ Suggested focus-visible enhancement
.telemetryPrimaryButton, .telemetrySecondaryButton { display: inline-flex; @@ &:hover { transform: translateY(-1px); } + + &:focus-visible { + outline: 2px solid `#fff`; + outline-offset: 2px; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/our-work/index.module.scss` around lines 182 - 223, The telemetry button styles (.telemetryPrimaryButton and .telemetrySecondaryButton) define hover but lack keyboard focus treatment; add explicit :focus-visible rules for both classes to provide a visible, high-contrast focus indicator (for example an outline or focused box-shadow and ensure border-color/ background adjustments match the hover/active visual language) and make sure to include focus-visible on the shared selector (e.g., .telemetryPrimaryButton:focus-visible, .telemetrySecondaryButton:focus-visible) so keyboard users see a clear, accessible focus state without interfering with mouse hover styles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/_components/OurWork/TelemetryLinks.tsx`:
- Around line 5-13: TelemetryLinks currently accepts grafanaUrl but the Grafana
CTA is commented out, leaving unused prop plumbing; restore or implement the
Grafana link UI so grafanaUrl is actually used. In the TelemetryLinks component,
add back the Grafana CTA (the commented-out link/button) and wire its href to
the grafanaUrl prop, ensuring it follows the same styling/behavior as the
existing telemetrySiteUrl entry and conditionally render the Grafana CTA only
when grafanaUrl is a non-empty string to avoid rendering a broken link.
In `@src/env.js`:
- Around line 13-16: .env.example is missing newly required public env vars
(NEXT_PUBLIC_GRAFANA_URL and NEXT_PUBLIC_TELEMETRY_SITE_URL) which breaks zod
validation for NEXT_PUBLIC_GRAFANA_URL, NEXT_PUBLIC_TELEMETRY_SITE_URL,
NEXT_PUBLIC_SUPABASE_ANON_KEY and NEXT_PUBLIC_SUPABASE_URL; update .env.example
to add entries for NEXT_PUBLIC_GRAFANA_URL and NEXT_PUBLIC_TELEMETRY_SITE_URL
(and ensure NEXT_PUBLIC_SUPABASE_ANON_KEY / NEXT_PUBLIC_SUPABASE_URL are
present) with placeholder values and brief comments so fresh clones pass the zod
validation in src/env.js (also mirror the same additions for the other
occurrence referenced around lines 34-37).
---
Nitpick comments:
In `@src/app/our-work/index.module.scss`:
- Around line 182-223: The telemetry button styles (.telemetryPrimaryButton and
.telemetrySecondaryButton) define hover but lack keyboard focus treatment; add
explicit :focus-visible rules for both classes to provide a visible,
high-contrast focus indicator (for example an outline or focused box-shadow and
ensure border-color/ background adjustments match the hover/active visual
language) and make sure to include focus-visible on the shared selector (e.g.,
.telemetryPrimaryButton:focus-visible, .telemetrySecondaryButton:focus-visible)
so keyboard users see a clear, accessible focus state without interfering with
mouse hover styles.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 675e28cf-23e9-4cfc-a832-3841d299468f
📒 Files selected for processing (4)
src/app/_components/OurWork/TelemetryLinks.tsxsrc/app/our-work/index.module.scsssrc/app/our-work/page.tsxsrc/env.js
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Line 24: Remove the trailing whitespace on the final line of .env.example
(line 24) and ensure the file ends with a single newline character (no extra
spaces or blank lines) so dotenv-linter and EOF newline hygiene pass; simply
delete any spaces/tabs at the end of the last line and save the file with a
single terminating newline.
- Around line 22-23: .env.example uses non-URL placeholders for
NEXT_PUBLIC_TELEMETRY_SITE_URL and NEXT_PUBLIC_GRAFANA_URL which will fail the
z.string().url() validation in src/env.js; update those two entries to include
valid example URLs (e.g., https://telemetry.example.com and
https://grafana.example.com) so they pass the URL schema and serve as correct
copy-pasteable examples for NEXT_PUBLIC_GRAFANA_URL and
NEXT_PUBLIC_TELEMETRY_SITE_URL.
Member
|
@copilot resolve the merge conflicts in this pull request |
Agent-Logs-Url: https://github.com/UCSolarCarTeam/Solar-Car-Website-Next/sessions/5fdbcf5e-bc7c-4250-88f7-6172d6f3aec5 Co-authored-by: promatty <106455288+promatty@users.noreply.github.com>
Contributor
Resolved. I pulled in the latest base changes and fixed the merge conflicts in No new UI changes were introduced in this conflict-resolution commit, so there is no new UI screenshot to share. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/portal/page.tsx (1)
91-116:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle signed-out users before the role gate.
Line 113 currently treats
user === nullas “unverified”, so signed-out visitors never reach theRedirectToSignInbranch at Line 188. In the same state, Line 91 still firesgetCurrentDBUser, which means the page does an unnecessary auth-dependent fetch before rendering the wrong UI.Suggested fix
const currentDBUser = trpc.portal.getCurrentDBUser.useQuery( - showAdminTables ? skipToken : undefined, + !user || showAdminTables ? skipToken : undefined, ); @@ + if (!user) { + return ( + <div className={styles.signInLayout}> + <RedirectToSignIn /> + </div> + ); + } + if ( - !user?.publicMetadata?.role || + !user.publicMetadata?.role || user.publicMetadata.role === "unverified" ) { @@ - {user ? ( - <> - <PortalPageHeader - currentPage={currentPage} - currentUser={user} - setCurrentPage={setCurrentPage} - /> - <div className={styles.portalContent}> - {adminClerkRoles.includes( - user.publicMetadata?.role as AdminRoles, - ) ? ( - <> - {currentPage === PortalNavigationLinks.Team ? ( - <TeamTable currentUser={user} users={dbUsers.data ?? []} /> - ) : currentPage === PortalNavigationLinks.Users ? ( - <UsersTable - currentUser={user} - users={clerkUsers.data ?? []} - /> - ) : currentPage === PortalNavigationLinks.Invitations ? ( - <InvitationsTable - currentUser={user} - invitations={invitedUsers.data ?? []} - /> - ) : currentPage === PortalNavigationLinks.Sponsors ? ( - <SponsorsTable - currentUser={user} - sponsors={sponsors.data ?? []} - /> - ) : currentPage === PortalNavigationLinks.Recruitment ? ( - <RecruitmentTable - currentUser={user} - forms={forms.data ?? []} - /> - ) : currentPage === PortalNavigationLinks.OurWork ? ( - <OurWorkEntriesTable - currentUser={user} - entries={ourWork.data ?? []} - /> - ) : currentPage === PortalNavigationLinks.Alumni ? ( - <AlumniTable alumni={alumniList.data ?? []} /> - ) : null} - </> - ) : ( - <> - {currentDBUser.data ? ( - <InlineUserPopup - clerkUser={user} - user={currentDBUser.data} - /> - ) : ( - <></> - )} - </> - )} - </div> - </> - ) : ( - <div className={styles.signInLayout}> - <RedirectToSignIn /> - </div> - )} + <> + <PortalPageHeader + currentPage={currentPage} + currentUser={user} + setCurrentPage={setCurrentPage} + /> + <div className={styles.portalContent}> + {adminClerkRoles.includes(user.publicMetadata?.role as AdminRoles) ? ( + <> + {currentPage === PortalNavigationLinks.Team ? ( + <TeamTable currentUser={user} users={dbUsers.data ?? []} /> + ) : currentPage === PortalNavigationLinks.Users ? ( + <UsersTable + currentUser={user} + users={clerkUsers.data ?? []} + /> + ) : currentPage === PortalNavigationLinks.Invitations ? ( + <InvitationsTable + currentUser={user} + invitations={invitedUsers.data ?? []} + /> + ) : currentPage === PortalNavigationLinks.Sponsors ? ( + <SponsorsTable + currentUser={user} + sponsors={sponsors.data ?? []} + /> + ) : currentPage === PortalNavigationLinks.Recruitment ? ( + <RecruitmentTable + currentUser={user} + forms={forms.data ?? []} + /> + ) : currentPage === PortalNavigationLinks.OurWork ? ( + <OurWorkEntriesTable + currentUser={user} + entries={ourWork.data ?? []} + /> + ) : currentPage === PortalNavigationLinks.Alumni ? ( + <AlumniTable alumni={alumniList.data ?? []} /> + ) : null} + </> + ) : currentDBUser.data ? ( + <InlineUserPopup clerkUser={user} user={currentDBUser.data} /> + ) : null} + </div> + </>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/portal/page.tsx` around lines 91 - 116, The page runs trpc.portal.getCurrentDBUser.useQuery (currentDBUser) even for signed-out visitors and treats user === null as "unverified", preventing RedirectToSignIn from showing; update the logic so you early-check for signed-out users (user === null) before the role gate and return/redirect to the sign-in UI (RedirectToSignIn) there, and change the getCurrentDBUser hook invocation to skip the auth-dependent fetch when no session exists (use skipToken or undefined based on user presence) so currentDBUser is only fetched for authenticated users.
🟡 Minor comments (4)
README.md-91-96 (1)
91-96:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the new Prisma migration script name in docs.
The README still references
yarn db:migrate-dev, but scripts were renamed toyarn db:migrate:dev. New contributors will hit command-not-found during setup.Doc fix
- yarn db:migrate-dev + yarn db:migrate:devAlso applies to: 200-206
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 91 - 96, The README references the old Prisma migration script name `yarn db:migrate-dev`; update all occurrences (e.g., the block showing "yarn db:migrate-dev") to the new script name `yarn db:migrate:dev` so docs match package.json; search for and replace `yarn db:migrate-dev` with `yarn db:migrate:dev` in the README (including the section around lines 91–96 and the other mention around 200–206).src/app/cars/carInformation.ts-27-28 (1)
27-28:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale future-tense date copy.
At Line 27 and Line 28, the text says Elysia “will be retiring” in summer 2025, which is now in the past (as of May 23, 2026). Please switch this to past tense with the explicit date/year.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/cars/carInformation.ts` around lines 27 - 28, Update the stale future-tense copy in src/app/cars/carInformation.ts: change the sentence "In summer of 2025, Elysia will be retiring at its final race at the Formula Sun Grand Prix in Bowling Green, Kentucky. Stay tuned for race updates!" to past tense with an explicit year (e.g., "In the summer of 2025, Elysia retired at its final race at the Formula Sun Grand Prix in Bowling Green, Kentucky.") so the text reflects that the event occurred; locate and edit the string literal in carInformation.ts where this description appears.src/app/_components/Recruitment/index.module.scss-8-8 (1)
8-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the extra blank line before declaration to satisfy Stylelint.
At Line 8, the empty line triggers
declaration-empty-line-before.Proposed fix
text-align: center; - min-width: 20%;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/_components/Recruitment/index.module.scss` at line 8, Remove the stray blank line immediately before the min-width declaration in src/app/_components/Recruitment/index.module.scss so the declaration (min-width: 20%) sits directly after its selector/opening brace or previous declaration; this will satisfy the Stylelint rule declaration-empty-line-before.src/app/_components/TeamMember/index.tsx-27-29 (1)
27-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCompute
hasOverlayas a boolean.Using
??here short-circuits on"", so members with an emptycompany/fieldOfStudybut a description or LinkedIn never get the overlay.Proposed fix
- const hasOverlay = overlayLeft ?? description ?? user.linkedIn ?? false; + const hasOverlay = Boolean(overlayLeft || description || user.linkedIn);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/_components/TeamMember/index.tsx` around lines 27 - 29, The hasOverlay computation uses the nullish coalescing operator (??) so empty strings count as present; change it to compute a boolean presence check instead: evaluate whether overlayLeft (from isAlumni ? user.company : user.fieldOfStudy), description, or user.linkedIn are non-empty values and coerce to true/false. Update the reference to overlayLeft, description, and user.linkedIn (and isAlumni) so hasOverlay becomes something like a Boolean(...) of (overlayLeft || description || user.linkedIn) or explicit non-empty checks (e.g., trim() !== ""), ensuring hasOverlay is a proper boolean.
🧹 Nitpick comments (1)
src/app/_components/Recruitment/RecruitmentForms.tsx (1)
13-13: ⚡ Quick winSkip fetching recruitment forms when closed.
Line 13 calls
trpc.fe.getRecruitment()even whenisRecruitmentClosedistrue. Move the fetch into the open branch to avoid unnecessary server work.♻️ Suggested refactor
- const recruitmentForms = await trpc.fe.getRecruitment(); + const recruitmentForms = isRecruitmentClosed + ? [] + : await trpc.fe.getRecruitment();Also applies to: 17-31
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/_components/Recruitment/RecruitmentForms.tsx` at line 13, The component currently calls trpc.fe.getRecruitment() unconditionally (const recruitmentForms = await trpc.fe.getRecruitment()), causing an unnecessary fetch when isRecruitmentClosed is true; move the fetch inside the branch that handles the open state (where you render or process recruitmentForms) so that trpc.fe.getRecruitment() is only invoked when isRecruitmentClosed is false, and ensure recruitmentForms is declared/assigned only in that branch (or set to null/undefined otherwise) to avoid using it when recruitment is closed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose.yml`:
- Around line 7-11: Replace the hardcoded DB credentials and host-exposed port
mapping: stop committing POSTGRES_USER/POSTGRES_PASSWORD/POSTGRES_DB values
directly in the compose file (reference the same keys) and instead load them
from external env or Docker secrets (e.g., use ${POSTGRES_USER},
${POSTGRES_PASSWORD}, ${POSTGRES_DB} or secret references), and remove or
restrict the published port mapping ("5432:5432") so Postgres is not bound to
all interfaces (either remove the ports entry for internal-only services or bind
to localhost like "127.0.0.1:5432:5432" if local access is needed).
In `@package.json`:
- Line 39: The package.json has mismatched Next/Eslint tooling and an
unnecessary runtime dependency: update the "eslint-config-next" dependency to a
16.x version to match "next": "^16.1.6" (ensure the package name
"eslint-config-next" is bumped to ^16) and remove the "npm" entry from
"dependencies" (or move "npm" to "devDependencies" if some tooling in the repo
explicitly requires the npm package at build/test time); then run npm/yarn
install and lint/tests to verify compatibility.
In `@prisma/migrations/20260509120000_communications_team_roles/migration.sql`:
- Around line 9-13: The enum/string literal for the team roles includes escaped
ampersands ("Marketing \& Events Associate") which prevents matching existing
rows and may cause the final cast to AllTeamRoles to fail; update the
enum/member list (the INSERT/ENUM literals that populate or alter AllTeamRoles)
to use plain ampersands ("Marketing & Events Associate") and remove any other
backslash-escaped ampersands found later in the migration (the occurrences noted
around the second block at lines 55-63), so the enum values exactly match
existing DB rows before performing the cast to AllTeamRoles.
In `@prisma/migrations/20260517100001_migrate_alumni_data/migration.sql`:
- Around line 19-20: The migration copies "yearJoinedSolarCar" and
"yearLeftSolarCar" directly into User.yearJoined and User.yearRetired; change
the SELECT that maps those source columns so it normalizes values to either a
four-digit year string ('YYYY') or NULL by extracting a 4-digit year with a
regex (or returning NULL if none), e.g. replace direct references to
"yearJoinedSolarCar"/"yearLeftSolarCar" with expressions that return the first
matched /^\d{4}$/ or NULL (using CASE/NULLIF + regexp or
substring/regexp_matches) so User.yearJoined and User.yearRetired always receive
a 4-digit year or NULL.
In
`@prisma/migrations/20260517100002_convert_year_fields_to_datetime/migration.sql`:
- Around line 3-4: The migration must preserve yearJoined and yearRetired as
4-digit year strings rather than DATEs: instead of adding "yearJoined_new" and
"yearRetired_new" as DATE columns, create them as text/varchar(4) (or the DB's
CHAR(4)) allowing NULL, copy/transform values to only the 4-digit year (e.g.,
using substring or to_char(EXTRACT(YEAR ...) ) only if source contains full
dates), validate/clean values to match /^\d{4}$/ during copy, then drop/rename
so the final columns are text "yearJoined" and "yearRetired" storing 'YYYY' or
NULL; update any index/constraint/rename statements that reference
yearJoined_new/yearRetired_new accordingly and do not materialize 'YYYY-01-01'
or change column types to DATE.
In `@prisma/schema.prisma`:
- Line 112: The Prisma enum member is mistakenly named ElectricalTeamTeam which
will generate the symbol ElectricalTeamTeam; rename the enum variant to
ElectricalTeam and keep the database mapping via `@map`("Electrical Team") so the
generated Prisma symbol matches the rest of the app; update the enum member in
the schema (change ElectricalTeamTeam -> ElectricalTeam) and then regenerate
Prisma client so the symbol and downstream usages align.
In `@prisma/seed.ts`:
- Around line 18-19: The logs in prisma/seed.ts print full connection strings
(LOCAL_DATABASE_URL and databaseUrl), leaking credentials; change these
console.error calls to avoid sensitive data by redacting credentials or logging
only non-sensitive parts (e.g., host and database name). Use the database URL
variables (LOCAL_DATABASE_URL and databaseUrl) to construct a scrubbed
message—parse with URL or regex to remove username/password or replace the
authority component with "<redacted>"—and emit that sanitized string or a
presence/host-only message instead of the full URL.
In `@src/app/_components/PortalComponents/EditUserCell/MoveToAlumniModal.tsx`:
- Line 44: The code in MoveToAlumniModal is currently sending yearRetired as a
full date string (YYYY-MM-DD); update the payload preparation in
MoveToAlumniModal (where yearRetired is collected/returned) to validate and
normalize yearRetired to a 4-digit year string or null — if a date string is
supplied, extract the year portion (e.g., slice or parse and format as YYYY),
ensure it's exactly four digits and set null for empty/invalid values, and then
send that normalized value to the backend/mutation instead of the original
full-date string.
- Around line 47-61: The current try/catch treats failures of cache invalidation
as if the mutation failed; keep the successful mutation path and call
onSuccess() and onClose() immediately after the mutation (the block around
mutateAsync/move-to-alumni logic), then run the two invalidations
(utils.portal.getDBUsers.invalidate and utils.portal.getAlumniList.invalidate)
in a separate try/catch and surface only refresh errors (e.g.,
toast.error("Failed to refresh tables") or similar) without reverting the
success flow; ensure you remove the invalidation Promise.all from the mutation's
toast.promise and relocate it so mutation success toast (e.g., `${userName}
moved to alumni`) is shown independently while handling refresh failures
separately.
In `@src/app/_hooks/useIntersectionObserver.ts`:
- Around line 14-31: The hook currently accepts any entry.target.id into
intersectingElements (via entry.target.id and the observer callback), allowing
unrelated DOM ids to become active; restrict tracking to only ids that come from
the hook's data by building a Set of allowed IDs (e.g., from the hook parameter
like data/sections) and, inside the IntersectionObserver callback (the code that
reads entry.target.id and updates intersectingElements), bail out early unless
allowedIds.has(id); apply the same whitelist check to the other spots mentioned
(the other observer handlers/updates at the 34-56 and 70-71 ranges) so only
elements whose ids exist in the provided data are ever added or removed from
intersectingElements.
In `@src/app/_lib/userValidation.ts`:
- Around line 122-130: The validateUserFormField function's value parameter is
currently typed as string | number | null | undefined but the userFormSchema
allows yearJoined and yearRetired to be z.date() (Date) or validated date
string/empty/optional; update the value type in validateUserFormField to include
Date (e.g., string | number | Date | null | undefined or a union that covers
Date) and adjust any related overloads/signatures so callers don't need to cast
Date values when validating yearJoined/yearRetired; keep references to
userFormSchema, validateUserFormField, yearJoined and yearRetired when locating
the change and update any unit tests/call sites that pass Date instances.
In `@src/app/_lib/utils.ts`:
- Around line 87-88: The fallback branch currently returns the raw parsed Date
(variable parsed) which preserves local time/offset and can shift the calendar
day; change the fallback to normalize parsed to UTC midnight like the other
branches before returning (e.g., create a UTC-midnight Date from parsed using
its UTC year/month/day or zero out UTC hours via setUTCHours(0,0,0,0)) so the
function returns a true date-only value consistently.
In `@src/app/our-work/page.tsx`:
- Line 1: The page is a Server Component but is being wrapped with React.memo;
remove the import of memo and stop wrapping the component export with memo so
the default export is the raw async page component (e.g., remove memo import and
change any "export default memo(Page)" or similar to directly export the async
Page function). Ensure no other references to memo remain in
src/app/our-work/page.tsx so the page stays a Server Component.
In `@src/app/team/page.tsx`:
- Around line 43-50: The Co-Chairs block currently renders three TeamMember
components unguarded (engineeringTeamManager, teamCaptain, businessTeamManager)
even though teamHierarchy may be null/partial; instead, build a guarded coChairs
array from teamHierarchy (e.g., const coChairs =
[teamHierarchy?.engineeringTeamManager, teamHierarchy?.teamCaptain,
teamHierarchy?.businessTeamManager].filter(Boolean)) and replace the three
hardcoded TeamMember usages with coChairs.map(user => <TeamMember user={user}
.../>), ensuring you only render when user exists; update the JSX around
styles.teamMembersContainer / styles.teamRoleContainer accordingly.
In `@src/server/api/routers/fe.ts`:
- Around line 20-29: The public alumni query currently returns full User records
via ctx.db.user.findMany (assigned to alumni); restrict returned columns by
adding a select object to that findMany call so only alumni-facing fields are
returned (for example: id, name, yearRetired and any public profile fields like
bio or avatarUrl) instead of all columns; update the code in the fe router where
the alumni variable is populated (ctx.db.user.findMany) to include the select
whitelist and remove any sensitive fields from the response.
In `@src/server/api/routers/portal.ts`:
- Around line 490-495: The update currently uses "company: input.company ??
undefined" which filters out explicit nulls so fields cannot be cleared; change
the assignment for company and companyTitle to pass null through but still skip
undefined (e.g., set company to null when input.company === null, otherwise use
input.company if defined, otherwise undefined) so ctx.db.user.update receives
null to clear the field and undefined to leave it unchanged; apply the same
logic for companyTitle (locate the ctx.db.user.update call and update the
company and companyTitle assignments accordingly).
- Around line 31-68: parseAndNormalizeDate currently accepts any 4-digit year or
calendar date, allowing out-of-range years to sneak past client validation;
update parseAndNormalizeDate so that for all branches (Date instance, year-only
branch that parses /^\d{4}$/, and full ISO YYYY-MM-DD branch) you enforce the
same year bounds as the client (minYear = 2000 and maxYear = currentYear + 1)
and return null when the parsed year is outside that range; keep the existing
UTC-normalization (Date.UTC) logic but add the year-range check before returning
the normalized Date.
---
Outside diff comments:
In `@src/app/portal/page.tsx`:
- Around line 91-116: The page runs trpc.portal.getCurrentDBUser.useQuery
(currentDBUser) even for signed-out visitors and treats user === null as
"unverified", preventing RedirectToSignIn from showing; update the logic so you
early-check for signed-out users (user === null) before the role gate and
return/redirect to the sign-in UI (RedirectToSignIn) there, and change the
getCurrentDBUser hook invocation to skip the auth-dependent fetch when no
session exists (use skipToken or undefined based on user presence) so
currentDBUser is only fetched for authenticated users.
---
Minor comments:
In `@README.md`:
- Around line 91-96: The README references the old Prisma migration script name
`yarn db:migrate-dev`; update all occurrences (e.g., the block showing "yarn
db:migrate-dev") to the new script name `yarn db:migrate:dev` so docs match
package.json; search for and replace `yarn db:migrate-dev` with `yarn
db:migrate:dev` in the README (including the section around lines 91–96 and the
other mention around 200–206).
In `@src/app/_components/Recruitment/index.module.scss`:
- Line 8: Remove the stray blank line immediately before the min-width
declaration in src/app/_components/Recruitment/index.module.scss so the
declaration (min-width: 20%) sits directly after its selector/opening brace or
previous declaration; this will satisfy the Stylelint rule
declaration-empty-line-before.
In `@src/app/_components/TeamMember/index.tsx`:
- Around line 27-29: The hasOverlay computation uses the nullish coalescing
operator (??) so empty strings count as present; change it to compute a boolean
presence check instead: evaluate whether overlayLeft (from isAlumni ?
user.company : user.fieldOfStudy), description, or user.linkedIn are non-empty
values and coerce to true/false. Update the reference to overlayLeft,
description, and user.linkedIn (and isAlumni) so hasOverlay becomes something
like a Boolean(...) of (overlayLeft || description || user.linkedIn) or explicit
non-empty checks (e.g., trim() !== ""), ensuring hasOverlay is a proper boolean.
In `@src/app/cars/carInformation.ts`:
- Around line 27-28: Update the stale future-tense copy in
src/app/cars/carInformation.ts: change the sentence "In summer of 2025, Elysia
will be retiring at its final race at the Formula Sun Grand Prix in Bowling
Green, Kentucky. Stay tuned for race updates!" to past tense with an explicit
year (e.g., "In the summer of 2025, Elysia retired at its final race at the
Formula Sun Grand Prix in Bowling Green, Kentucky.") so the text reflects that
the event occurred; locate and edit the string literal in carInformation.ts
where this description appears.
---
Nitpick comments:
In `@src/app/_components/Recruitment/RecruitmentForms.tsx`:
- Line 13: The component currently calls trpc.fe.getRecruitment()
unconditionally (const recruitmentForms = await trpc.fe.getRecruitment()),
causing an unnecessary fetch when isRecruitmentClosed is true; move the fetch
inside the branch that handles the open state (where you render or process
recruitmentForms) so that trpc.fe.getRecruitment() is only invoked when
isRecruitmentClosed is false, and ensure recruitmentForms is declared/assigned
only in that branch (or set to null/undefined otherwise) to avoid using it when
recruitment is closed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ebbef202-3c9f-4c06-b358-e4817e6e90da
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (62)
.env.example.gitignoreREADME.mddocker-compose.ymlnext.config.jspackage.jsonprisma/migrations/20260509120000_communications_team_roles/migration.sqlprisma/migrations/20260517100000_consolidate_alumni_to_user/migration.sqlprisma/migrations/20260517100001_migrate_alumni_data/migration.sqlprisma/migrations/20260517100002_convert_year_fields_to_datetime/migration.sqlprisma/schema.prismaprisma/seed.tssrc/app/_components/Buttons/BasicButton.tsxsrc/app/_components/Cars/CarScreenView.tsxsrc/app/_components/OurWork/YearSection.tsxsrc/app/_components/Pagebullets/index.tsxsrc/app/_components/PortalComponents/EditOurWorkEntryCell/DeleteOurWorkEntry.tsxsrc/app/_components/PortalComponents/EditOurWorkEntryCell/index.tsxsrc/app/_components/PortalComponents/EditRecruitmentFormCell/DeleteForm.tsxsrc/app/_components/PortalComponents/EditRecruitmentFormCell/index.tsxsrc/app/_components/PortalComponents/EditSponsorCell/DeleteSponsor.tsxsrc/app/_components/PortalComponents/EditSponsorCell/index.tsxsrc/app/_components/PortalComponents/EditUserCell/DeleteUser.tsxsrc/app/_components/PortalComponents/EditUserCell/EditAlumniPopupAdmin.tsxsrc/app/_components/PortalComponents/EditUserCell/EditUserPopupAdmin.tsxsrc/app/_components/PortalComponents/EditUserCell/InlineUserPopup.tsxsrc/app/_components/PortalComponents/EditUserCell/MoveToAlumniModal.tsxsrc/app/_components/PortalComponents/EditUserCell/index.module.scsssrc/app/_components/PortalComponents/EditUserCell/index.tsxsrc/app/_components/PortalComponents/Portal/AlumniTable.tsxsrc/app/_components/PortalComponents/Portal/Invitations/InvitationsTable.tsxsrc/app/_components/PortalComponents/Portal/OurWorkEntriesTable.tsxsrc/app/_components/PortalComponents/Portal/PortalPageHeader.tsxsrc/app/_components/PortalComponents/Portal/RecruitmentTable.tsxsrc/app/_components/PortalComponents/Portal/SponsorsTable.tsxsrc/app/_components/PortalComponents/Portal/TeamTable.tsxsrc/app/_components/PortalComponents/Portal/UsersTable.tsxsrc/app/_components/Recruitment/RecruitmentForms.tsxsrc/app/_components/Recruitment/index.module.scsssrc/app/_components/TeamMember/index.tsxsrc/app/_hooks/useIntersectionObserver.tssrc/app/_lib/userValidation.tssrc/app/_lib/utils.tssrc/app/_types/index.tssrc/app/cars/carInformation.tssrc/app/cars/page.tsxsrc/app/loading.tsxsrc/app/our-work/page.tsxsrc/app/page.tsxsrc/app/portal/page.tsxsrc/app/recruitment/index.module.scsssrc/app/recruitment/loading.tsxsrc/app/recruitment/page.tsxsrc/app/sponsors/loading.tsxsrc/app/sponsors/page.tsxsrc/app/support-us/page.tsxsrc/app/team/page.tsxsrc/flags.tssrc/proxy.tssrc/server/api/routers/fe.tssrc/server/api/routers/portal.tstsconfig.json
💤 Files with no reviewable changes (4)
- src/app/sponsors/loading.tsx
- src/app/recruitment/loading.tsx
- src/app/recruitment/index.module.scss
- src/app/loading.tsx
✅ Files skipped from review due to trivial changes (12)
- src/app/_components/PortalComponents/EditOurWorkEntryCell/index.tsx
- src/app/_components/PortalComponents/EditUserCell/index.tsx
- src/flags.ts
- src/app/_components/PortalComponents/Portal/RecruitmentTable.tsx
- src/app/_components/PortalComponents/EditSponsorCell/DeleteSponsor.tsx
- src/app/_components/PortalComponents/Portal/OurWorkEntriesTable.tsx
- src/app/_components/PortalComponents/EditRecruitmentFormCell/DeleteForm.tsx
- src/app/_components/PortalComponents/EditSponsorCell/index.tsx
- src/app/_components/PortalComponents/Portal/SponsorsTable.tsx
- src/app/_components/PortalComponents/EditRecruitmentFormCell/index.tsx
- src/app/_components/PortalComponents/EditOurWorkEntryCell/DeleteOurWorkEntry.tsx
- src/app/_components/PortalComponents/Portal/UsersTable.tsx
This reverts commit 16dd609. revert copilot changes#
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.env.example (2)
24-24:⚠️ Potential issue | 🟡 MinorRemove trailing whitespace on the last line
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.example at line 24, The file .env.example contains trailing whitespace on the final line; remove the trailing space or newline characters at the end of the file so the last line has no extra whitespace; simply edit the file to delete the trailing spaces on the final line and save the file (no code changes required).
22-23:⚠️ Potential issue | 🟠 MajorUse valid URL examples for required public env vars
These environment variables are validated as
z.string().url()insrc/env.jsand will cause startup failures if invalid URLs are used.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.example around lines 22 - 23, The .env example uses placeholder tokens that fail z.string().url() validation; update NEXT_PUBLIC_TELEMETRY_SITE_URL and NEXT_PUBLIC_GRAFANA_URL to valid URL examples (e.g., use full https:// URLs such as https://telemetry.example.com and https://grafana.example.com) so they pass the URL validation in src/env.js; ensure the values include the scheme (http/https) and look like real hostnames.
🧹 Nitpick comments (1)
src/app/our-work/page.tsx (1)
1-1: 🏗️ Heavy liftConsider keeping this page as a Server Component with selective client components
Converting the entire page to a client component loses the benefits of server-side rendering (initial HTML with data, better SEO, faster first paint). The interactive features (intersection observer, scroll behavior) could be isolated in client components (Timeline, Pagebullets) while the page itself remains a server component that pre-fetches data via
await trpc.fe.getOurWork().This architectural pattern would:
- Improve initial page load performance (no loading spinner on first render)
- Enhance SEO (search engines receive pre-rendered content)
- Follow Next.js App Router best practices for mixing server and client components
Also applies to: 20-71
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In @.env.example:
- Line 24: The file .env.example contains trailing whitespace on the final line;
remove the trailing space or newline characters at the end of the file so the
last line has no extra whitespace; simply edit the file to delete the trailing
spaces on the final line and save the file (no code changes required).
- Around line 22-23: The .env example uses placeholder tokens that fail
z.string().url() validation; update NEXT_PUBLIC_TELEMETRY_SITE_URL and
NEXT_PUBLIC_GRAFANA_URL to valid URL examples (e.g., use full https:// URLs such
as https://telemetry.example.com and https://grafana.example.com) so they pass
the URL validation in src/env.js; ensure the values include the scheme
(http/https) and look like real hostnames.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df312111-d328-45fe-b45a-51af31671963
📒 Files selected for processing (2)
.env.examplesrc/app/our-work/page.tsx
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.
Summary by CodeRabbit