Feature/reservationadmission page#91
Open
DankaMarci wants to merge 23 commits into
Open
Conversation
…, settings for penalties, and updates to reservation handling
…c; update reservation handling to exclude sanctioned reservations.
…or reservation management
…h related admin functionality; introduce new page protection system for enhanced role-based access control.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…reservationadmission-page
Collaborator
|
Please run |
added 2 commits
March 23, 2026 21:09
…tions service, add MyGatekeeps page for gatekeeper-specific reservations, and implement admin panels for managing opened weeks, periods, and settings. Update UI components for consistent styling and improve accessibility features.
…All method in ReservationsController and ReservationsService, improve error messages for better user experience, and update UI components for improved visibility and accessibility in reservation details.
added 3 commits
March 24, 2026 12:22
… record entity and DTO, implement findAll method in SanctionRecordsService, and update sanction management in controllers and UI components for better user experience. Added RBAC for sanctions, periods and openWeeks so that only admins or authorized personel can execute CRUD operations on them. Implemented the base structure for testing with Jest
justnyx
reviewed
Apr 22, 2026
| @Query('page_size', ParseIntPipe) pageSize: number, | ||
| @Query('gateKeeperId') gateKeeperId?: string | ||
| ) { | ||
| const parsedGateKeeperId = gateKeeperId ? parseInt(gateKeeperId, 10) : undefined; |
Collaborator
There was a problem hiding this comment.
do we need this? if not, remove it.
There was a problem hiding this comment.
Pull request overview
This PR introduces a gatekeeper-based reservation admission flow and expands reservation governance (open weeks/periods, sanctions, settings), while also aligning the frontend UI to a monochromatic design system and adding route/page protection scaffolding.
Changes:
- Add gatekeeper priority + “need to be let in” support across reservation UI and backend validation.
- Add admin tooling for Settings / Periods / Opened Weeks, plus sanction record endpoints and user-facing sanction views.
- Refresh UI theming to use
primary/bordertokens and add basic page protection (middleware + HOCs).
Reviewed changes
Copilot reviewed 103 out of 106 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| project_info/guidelines.md | Documents the monochromatic design system guidance. |
| apps/frontend/src/utils/withAuth.tsx | Adds client-side HOC for role-based page protection. |
| apps/frontend/src/types/settings.ts | Introduces Settings type used by admin/settings and quota logic. |
| apps/frontend/src/types/sanction-record.ts | Adds SanctionRecord type for sanction history UIs. |
| apps/frontend/src/types/reservation.ts | Adds gatekeeper priority + needToBeLetIn to Reservation type. |
| apps/frontend/src/types/period.ts | Adds Period type for period management UI. |
| apps/frontend/src/types/openedWeek.ts | Adds OpenedWeek type for open-week gating. |
| apps/frontend/src/types/admin.ts | Removes old admin config types (deprecated approach). |
| apps/frontend/src/middleware.ts | Adds server-side JWT presence check for protected routes. |
| apps/frontend/src/lib/reservationSubmitter.ts | Updates reservation submission for user-or-band, needToBeLetIn, better errors. |
| apps/frontend/src/lib/errorToast.ts | Improves extraction of backend error messages for toast display. |
| apps/frontend/src/hooks/useWeekStatus.ts | Adds hook to determine whether a week is open (period-based). |
| apps/frontend/src/hooks/useUser.tsx | Adds loading state to the user fetch hook. |
| apps/frontend/src/hooks/useReservationDetails.ts | Extends reservation details logic (gatekeeper priority, delete/edit handling). |
| apps/frontend/src/hooks/useQuotaCheck.ts | Adds quota computation including settings + sanction penalties. |
| apps/frontend/src/hooks/useAdminConfig.ts | Removes old admin config hook. |
| apps/frontend/src/hooks/collisionWithAdminRes.tsx | Attempts to adjust collision logic for band reservations. |
| apps/frontend/src/components/ui/rules-card.tsx | Updates link color tokens to text-primary. |
| apps/frontend/src/components/ui/card.tsx | Aligns Card border styling to border-border. |
| apps/frontend/src/components/ui/button.tsx | Replaces orange-based variants with primary token usage. |
| apps/frontend/src/components/ui/badge.tsx | Aligns default badge variant to primary tokens. |
| apps/frontend/src/components/profile/profile-page.tsx | Displays sanction history on user profile component. |
| apps/frontend/src/components/news/news-card.tsx | Updates pinned styling from orange to primary. |
| apps/frontend/src/components/layout/theme-toggle.tsx | Adds trigger id for testing/targeting. |
| apps/frontend/src/components/layout/sidebar.tsx | Adds gatekeeper/admin nav + stats link. |
| apps/frontend/src/components/layout/player.tsx | Updates player accent colors to primary tokens. |
| apps/frontend/src/components/layout/header.tsx | Adds trigger id for user menu. |
| apps/frontend/src/components/layout/footer.tsx | Aligns footer border color to border-border. |
| apps/frontend/src/components/calendar/week/daily-weekly-view.tsx | Adds open-week gating for week navigation + UI refresh. |
| apps/frontend/src/components/calendar/validDate.tsx | Tightens validation (duration + 15-min intervals). |
| apps/frontend/src/components/calendar/time-picker.tsx | Introduces 15-minute interval time picker component. |
| apps/frontend/src/components/calendar/reservation-legend.tsx | Adds legend UI for reservation types. |
| apps/frontend/src/components/calendar/reservation-details.tsx | Updates reservation details UI (gatekeeper priority actions, TimePicker). |
| apps/frontend/src/components/calendar/isReservationOvertime.tsx | Updates overtime calculations and day/week filtering signatures. |
| apps/frontend/src/components/calendar/interval-swithcer.tsx | Updates interval switcher styling to primary tokens. |
| apps/frontend/src/components/calendar/day/day-reservation.tsx | Improves day reservation rendering and labeling for user/band. |
| apps/frontend/src/components/calendar/day/day-comment.tsx | Aligns day comment layout to updated grid sizing and pointer events. |
| apps/frontend/src/components/calendar/day/daily-view.tsx | Updates day view layout + open-week gating for next-day navigation. |
| apps/frontend/src/components/calendar/day/daily-view-without-date.tsx | Updates grid borders to new tokens. |
| apps/frontend/src/components/calendar/comment-details.tsx | Uses TimePicker; limits edit/delete UI to admins. |
| apps/frontend/src/components/calendar/calendar.tsx | Fetches opened weeks and passes into day/week views. |
| apps/frontend/src/components/calendar/add-reservation.tsx | Adds exclusive user/band selection, TimePicker, week locking, needToBeLetIn. |
| apps/frontend/src/components/calendar/add-panel.tsx | UI tweaks + “comment” renamed to “felhívás” in modal. |
| apps/frontend/src/components/calendar/add-comment.tsx | Uses TimePicker and aligns styling to new tokens. |
| apps/frontend/src/components/calendar/Line.tsx | Updates current-time line styling to primary token. |
| apps/frontend/src/components/admin/reservation-limits-form.tsx | Removes old reservation limits admin UI. |
| apps/frontend/src/components/admin/SettingsPanel.tsx | Adds admin UI for Settings singleton editing. |
| apps/frontend/src/components/admin/PeriodsPanel.tsx | Adds admin UI for creating/toggling/deleting periods. |
| apps/frontend/src/components/admin/OpenedWeeksPanel.tsx | Adds admin UI for opening/closing upcoming weeks. |
| apps/frontend/src/app/stats/page.tsx | Restricts stats page to gatekeepers/admins via HOC. |
| apps/frontend/src/app/rules/page.tsx | Updates rule page links to primary tokens. |
| apps/frontend/src/app/reservation/page.tsx | Adds “Beengedéseim” navigation from reservation page header. |
| apps/frontend/src/app/profile/page.tsx | Adds authenticated “My profile” page with sanction history table. |
| apps/frontend/src/app/my-gatekeeps/page.tsx | Adds gatekeeper page for managing assigned reservations + sanctions. |
| apps/frontend/src/app/globals.css | Updates CSS variables to zinc-style monochrome palette and simplifies scroll tweaks. |
| apps/frontend/src/app/admin/page.tsx | Replaces old admin config section with OpenedWeeks/Periods/Settings panels. |
| apps/frontend/package.json | Updates sonner dependency version. |
| apps/frontend/docs/PAGE_PROTECTION.md | Documents middleware + HOC protection approach. |
| apps/frontend/declarations.d.ts | Adds framer module declaration. |
| apps/backend/src/settings/settings.service.ts | Implements singleton Settings retrieval + update. |
| apps/backend/src/settings/settings.module.ts | Registers Settings module. |
| apps/backend/src/settings/settings.controller.ts | Adds settings endpoints (get + patch). |
| apps/backend/src/settings/dto/update-settings.dto.ts | Adds validation for Settings updates. |
| apps/backend/src/sanction-records/sanction-records.service.ts | Adds sanction record CRUD + “me” aggregation logic. |
| apps/backend/src/sanction-records/sanction-records.module.ts | Registers sanction records module. |
| apps/backend/src/sanction-records/sanction-records.controller.ts | Adds sanction record endpoints (create/update/me/all/etc.). |
| apps/backend/src/sanction-records/entities/sanction-record.entity.ts | Defines sanction record validation entity. |
| apps/backend/src/sanction-records/dto/update-sanction-record.dto.ts | Adds update DTO validation. |
| apps/backend/src/sanction-records/dto/create-sanction-record.dto.ts | Adds create DTO validation. |
| apps/backend/src/reservations/reservations.service.ts | Adds reservation validation, open-week/period enforcement, quota/sanction-based status. |
| apps/backend/src/reservations/reservations.service.spec.ts | Introduces initial ReservationsService test scaffold. |
| apps/backend/src/reservations/reservations.controller.ts | Adds (currently unused) gateKeeperId query param parsing. |
| apps/backend/src/reservations/entities/reservation.entity.ts | Adds gatekeeper priority + needToBeLetIn fields to reservation entity. |
| apps/backend/src/reservations/dto/update-reservation.dto.ts | Simplifies UpdateReservationDto to PartialType of SimpleReservationDto. |
| apps/backend/src/reservations/dto/simple-reservation.dto.ts | Updates DTO to include userId/bandId (not omitted). |
| apps/backend/src/periods/periods.service.ts | Adds periods CRUD. |
| apps/backend/src/periods/periods.module.ts | Registers periods module. |
| apps/backend/src/periods/periods.controller.ts | Adds guarded periods CRUD endpoints. |
| apps/backend/src/periods/dto/update-period.dto.ts | Adds update DTO validation for periods. |
| apps/backend/src/periods/dto/create-period.dto.ts | Adds create DTO validation for periods. |
| apps/backend/src/opened-weeks/opened-weeks.service.ts | Adds opened-week list + upsert logic. |
| apps/backend/src/opened-weeks/opened-weeks.module.ts | Registers opened-weeks module. |
| apps/backend/src/opened-weeks/opened-weeks.controller.ts | Adds opened-weeks list + admin upsert endpoint. |
| apps/backend/src/opened-weeks/dto/update-opened-week.dto.ts | Adds opened-week DTO validation. |
| apps/backend/src/comments/comments.service.ts | Adds auto-deletion of overlapping reservations for non-reservable comments. |
| apps/backend/src/bands/bands.service.ts | Ensures band members are included in findOne response. |
| apps/backend/src/app.module.ts | Removes AdminModule and registers new Settings/Periods/Sanctions/OpenedWeeks modules. |
| apps/backend/src/admin/entities/reservation-config.entity.ts | Removes legacy reservation config entity. |
| apps/backend/src/admin/dto/update-config.dto.ts | Removes legacy config DTO. |
| apps/backend/src/admin/dto/set-role.dto.ts | Removes legacy set-role DTO. |
| apps/backend/src/admin/admin.service.ts | Removes legacy admin service. |
| apps/backend/src/admin/admin.module.ts | Removes legacy admin module. |
| apps/backend/src/admin/admin.controller.ts | Removes legacy admin controller. |
| apps/backend/prisma/schema.prisma | Adds Settings/OpenedWeek/SanctionRecord models; adds gatekeeper priority + needToBeLetIn. |
| apps/backend/prisma/migrations/20260326114226_init/migration.sql | Adds reservationId FK on SanctionRecord. |
| apps/backend/prisma/migrations/20260323103330_removed_unnecessary_reservation_config_models/migration.sql | Drops ReservationConfig/SanctionTier tables. |
| apps/backend/prisma/migrations/20260322155651_add_weeks_and_extra_settings_attributes/migration.sql | Adds OpenedWeek + extra Settings columns. |
| apps/backend/prisma/migrations/20260322135731_remove_sanction_type/migration.sql | Removes SANCTIONED from ReservationStatus enum. |
| apps/backend/prisma/migrations/20260320130812_add_gatekeeper_priority/migration.sql | Adds GateKeeperPriority enum + column. |
| apps/backend/prisma/migrations/20260116170512_add_sanction_records/migration.sql | Adds SanctionRecord table and removes User.sanctionPoints. |
| apps/backend/prisma/migrations/20260116023247_kolis_fogadas/migration.sql | Adds needToBeLetIn column to Reservation. |
| apps/backend/prisma/migrations/20260109180915_reservation_enhancements/migration.sql | Adds initial Settings + Period.isOpen + (legacy) SANCTIONED status. |
| apps/backend/package.json | Adds backend test script and Jest dev deps. |
| apps/backend/jest.config.js | Adds Jest config for backend tests. |
Comments suppressed due to low confidence (1)
apps/frontend/src/hooks/collisionWithAdminRes.tsx:17
reservationsOfDay.filter(...)never returns a boolean because the predicate only triggers an asyncgetUser(...).then(...)and returnsundefined. As a resultadminReservationswill always be empty, so admin-collision detection is effectively disabled. Consider making this function synchronous (e.g., rely onReservation.status === 'ADMINMADE'instead of fetching users) or refactor to async and await the role checks before filtering.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
18
to
20
| export default function ReservationDetails(props: EventDetailsProps) { | ||
| if (!props.clickedEvent) return null; | ||
| if (!props.clickedEvent) return; | ||
|
|
There was a problem hiding this comment.
This component returns undefined when clickedEvent is missing (if (!props.clickedEvent) return;). React components should return null to render nothing; returning undefined can trigger type/lint issues (and is easy to miss during refactors).
Comment on lines
+53
to
+66
| const isNextWeekOpen = () => { | ||
| const nextWeekMonday = new Date(firstDayOfWeek); | ||
| nextWeekMonday.setDate(nextWeekMonday.getDate() + 7); | ||
| nextWeekMonday.setHours(0, 0, 0, 0); | ||
|
|
||
| return props.openedWeeks.some((w) => { | ||
| const wDate = new Date(w.monday); | ||
| return wDate.getTime() === nextWeekMonday.getTime() && w.isOpen; | ||
| }); | ||
| }; | ||
|
|
||
| const handleNextWeek = () => { | ||
| if (!isNextWeekOpen()) return; | ||
|
|
There was a problem hiding this comment.
isNextWeekOpen() strictly requires an openedWeeks entry for the next week. If you navigate to an older week (or openedWeeks is only populated around “now”), the "next" button can become permanently disabled even though the next week is still in the past and should be viewable. Consider allowing navigation to past/current weeks unconditionally (similar to DailyView’s isNextDayOpen logic) and only enforcing openedWeeks for future weeks.
Comment on lines
+76
to
+92
| async update(id: number, updateCommentDto: UpdateCommentDto) { | ||
| try { | ||
| return await this.prisma.comment.update({ | ||
| const updated = await this.prisma.comment.update({ | ||
| where: { | ||
| id, | ||
| }, | ||
| data: { | ||
| ...updateCommentDto, | ||
| }, | ||
| }); | ||
|
|
||
| // If the updated comment now blocks reservations, purge any overlapping ones | ||
| if (updated.isReservable === false) { | ||
| await this.deleteOverlappingReservations(new Date(updated.startTime), new Date(updated.endTime)); | ||
| } | ||
|
|
||
| return updated; |
There was a problem hiding this comment.
Similarly in update(), the comment update is committed before overlapping reservations are deleted. If the deletion fails, the API call errors but the updated comment remains saved. Consider using a $transaction to update the comment and perform the conditional reservation deletion atomically.
Comment on lines
+38
to
+46
| @Get('user/:userId') | ||
| async findByUserId(@Param('userId', ParseIntPipe) userId: number) { | ||
| return this.sanctionRecordsService.findByUserId(userId); | ||
| } | ||
|
|
||
| @Get('band/:bandId') | ||
| async findByBandId(@Param('bandId', ParseIntPipe) bandId: number) { | ||
| return this.sanctionRecordsService.findByBandId(bandId); | ||
| } |
There was a problem hiding this comment.
The user/:userId and band/:bandId sanction-record endpoints are missing any auth guard, so they’re publicly accessible while other sanction endpoints require JWT. These records (points + reasons) are typically sensitive. Add @UseGuards(AuthGuard('jwt')) and enforce authorization (e.g., only ADMIN can query arbitrary user/band sanctions; non-admins should be restricted to their own via /me).
Comment on lines
+44
to
+48
| try { | ||
| const settingsRes = await axiosApi.get('/settings/1'); | ||
| if (settingsRes.data) { | ||
| settings = settingsRes.data; | ||
| } |
There was a problem hiding this comment.
This hook fetches settings from /settings/1, but the backend provides a singleton settings record via GET /settings and the id may not be 1 in existing environments. Fetch from /settings (or first fetch /settings to discover the actual id) to avoid 404s and silently falling back to defaults.
Comment on lines
+83
to
+87
| <select | ||
| value={selectedHour} | ||
| onChange={handleHourChange} | ||
| className='w-20 bg-white hover:bg-slate-200 dark:bg-zinc-700 dark:hover:bg-zinc-600 text-black dark:text-zinc-200 rounded-md border border-zinc-600 px-2 py-2 focus:ring-2 focus:ring-orange-500 focus:border-transparent' | ||
| > |
There was a problem hiding this comment.
The hour <select> still uses focus:ring-orange-500, which conflicts with the new monochromatic --primary/ring design tokens used elsewhere in this PR. Use focus:ring-primary/focus:ring-ring for consistency with the rest of the inputs.
Comment on lines
+10
to
+12
| import OpenedWeeksPanel from '../../components/admin/OpenedWeeksPanel'; | ||
| import PeriodsPanel from '../../components/admin/PeriodsPanel'; | ||
| import SettingsPanel from '../../components/admin/SettingsPanel'; |
There was a problem hiding this comment.
This page mixes absolute alias imports (e.g. @/components/...) with new relative ../../components/... imports. For consistency with the rest of the codebase (and to avoid fragile paths when files move), import these panels via the existing @/components/... alias as well.
| transform: { | ||
| '^.+\\.(t|j)s$': 'ts-jest', | ||
| }, | ||
| collectCoverageFrom: ['**/*.(t|j)s'], |
There was a problem hiding this comment.
collectCoverageFrom: ['**/*.(t|j)s'] is not a valid glob for Jest’s micromatch patterns, so coverage collection may silently skip files. Use a supported pattern such as ['**/*.{ts,js}'] (and optionally exclude .d.ts, main.ts, etc.).
Suggested change
| collectCoverageFrom: ['**/*.(t|j)s'], | |
| collectCoverageFrom: ['**/*.{ts,js}'], |
Comment on lines
132
to
139
| if (reservationTimes[2] && reservationTimes[3]) { | ||
| await axiosApi.post('http://localhost:3030/reservations', { | ||
| userId: submissionUserId, | ||
| bandId: band?.id, | ||
| ...(band?.id ? { bandId: band.id } : { userId: submissionUserId }), | ||
| startTime: reservationTimes[2].toISOString(), | ||
| endTime: reservationTimes[3].toISOString(), | ||
| status: 'OVERTIME', | ||
| needToBeLetIn: needToBeLetIn, | ||
| }); |
There was a problem hiding this comment.
The overtime reservation request still posts to a hard-coded http://localhost:3030/reservations URL. This will break outside local dev and also bypasses the configured axiosApi base URL/interceptors. Use the same relative /reservations endpoint (via axiosApi) as the other reservation creations.
Comment on lines
+23
to
30
| async findAll( | ||
| @Query('page', ParseIntPipe) page: number, | ||
| @Query('page_size', ParseIntPipe) pageSize: number, | ||
| @Query('gateKeeperId') gateKeeperId?: string | ||
| ) { | ||
| const parsedGateKeeperId = gateKeeperId ? parseInt(gateKeeperId, 10) : undefined; | ||
| return this.reservationsService.findAll(page, pageSize); | ||
| } |
There was a problem hiding this comment.
gateKeeperId is parsed (parsedGateKeeperId) but then ignored, and findAll always returns this.reservationsService.findAll(page, pageSize). Either remove this query param or pass it through and implement filtering in the service; otherwise callers will assume server-side filtering works when it doesn’t.
- Added JWT authentication and role-based access control for user and band ID retrieval in SanctionRecordsController. - Implemented role-based access control for updating settings in SettingsController. - Updated default settings values for maximum hours per week and day in SettingsService. - Enhanced user profile management in UsersController to restrict profile editing to own profiles for non-admin users. - Improved user data retrieval in UsersService to include dorm residency information based on user privileges. - Refactored admin page to streamline component imports and improve readability. - Created a new login route to redirect to the authentication page. - Optimized gatekeeper reservation fetching logic in MyGatekeepsPage. - Added registration route to mirror login route for consistency. - Updated Stats component to remove unnecessary period start calculation. - Expanded SettingsPanel to include minimum and maximum reservation lengths. - Adjusted band form dialog to redirect to the login page without hardcoding the API URL. - Enhanced reservation overtime logic to accommodate configurable limits. - Improved error handling and user feedback in reservation submission logic. - Updated mock user data to reflect changes in user model. - Refined user type definitions to clarify dorm residency properties.
|
Warning Review limit reached
More reviews will be available in 43 minutes and 40 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis change replaces the old admin reservation config with settings, periods, opened weeks, sanction records, stricter reservation and gatekeeper rules, broader backend auth guards, new protected frontend routes and pages, updated calendar submission flows, and a monochrome primary-based UI theme. ChangesReservation governance and frontend access
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical 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 (4)
apps/backend/src/comments/comments.service.ts (1)
47-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
findAllreturns reservation count instead of comment count.Line 47 calls
this.prisma.reservation.count()but this method is for comments. This causes incorrect pagination metadata (wrongcountandlimitvalues).🐛 Proposed fix
- const count = this.prisma.reservation.count(); + const count = this.prisma.comment.count();🤖 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 `@apps/backend/src/comments/comments.service.ts` at line 47, The findAll method in CommentsService is incorrectly calling this.prisma.reservation.count() instead of counting comments. Change the call to use the correct Prisma model for comments (this.prisma.comment.count() or the appropriate comments entity name based on your schema) to ensure the pagination metadata reflects the actual number of comments rather than reservations.apps/frontend/src/components/calendar/validDate.tsx (1)
47-50:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAsync
deleteReservationcall doesn't affect return value.The
deleteReservation(res.id).then(() => { return true; })is non-blocking. Thereturn trueonly returns from the.then()callback, not fromvalidDate. The loop continues, likely hitting line 58 (return false) before the deletion completes.This causes a race: the reservation is deleted but
validDatereturnsfalse, rejecting the submission.🐛 Proposed fix - make function async
-export default function validDate( +export default async function validDate( start: Date, end: Date, reservation: Reservation | undefined, reservations: Reservation[] -): boolean { +): Promise<boolean> {Then update the overlap check:
} else if (res.status === 'OVERTIME') { - deleteReservation(res.id).then(() => { - return true; - }); + await deleteReservation(res.id); + // Continue checking other reservations }Note: All callers of
validDatemust thenawaitthe result.🤖 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 `@apps/frontend/src/components/calendar/validDate.tsx` around lines 47 - 50, The validDate function has a race condition where the deleteReservation call is not properly awaited. The return true inside the .then() callback only returns from the promise callback, not from validDate itself, causing the function to continue and return false before the deletion completes. To fix this, convert the validDate function to be async, await the deleteReservation call in the OVERTIME case, and ensure all callers of validDate properly await its result.apps/frontend/src/hooks/collisionWithAdminRes.tsx (1)
27-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFire-and-forget deletion may cause race conditions.
deleteReservation(res.id)is called withoutawait, so the function returnsfalsebefore deletions complete. If the caller immediately creates a new reservation, it may collide with the not-yet-deleted OVERTIME reservation.Additionally, the function name
collisionWithAdminResdoesn't indicate it also deletes overtime reservations—this side effect is surprising.Proposed fix
Either await the deletions:
-export default function collisionWithAdminRes( +export default async function collisionWithAdminRes( startTime: Date, endTime: Date, reservationsOfDay: Reservation[] -): boolean { +): Promise<boolean> { // ... admin check ... for (const res of reservationsOfDay) { if (res.status !== ReservationStatus.OVERTIME) continue; // ... overlap check ... if (/* overlaps */) { - deleteReservation(res.id); + await deleteReservation(res.id); } } return false; }Or move deletion logic to a separate, explicitly-named function.
🤖 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 `@apps/frontend/src/hooks/collisionWithAdminRes.tsx` around lines 27 - 45, The collisionWithAdminRes function has a fire-and-forget call to deleteReservation without await, causing it to return before deletions complete and risking race conditions. Add await before each deleteReservation call inside the loop and make the function async to properly wait for all deletions to finish before the function returns false. Alternatively, consider extracting the deletion logic into a separate function with a clearer name that reflects its side effect of deleting OVERTIME reservations.apps/frontend/src/components/calendar/day/day-reservation.tsx (1)
21-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid per-card user lookups in the calendar grid.
Line 22 introduces an extra
/users/:idrequest per rendered reservation card. In this view, that scales into N+1 traffic and slower paint, while the reservation payload already includesuser(andband) data. Prefer rendering directly fromprops.reservationfields.Suggested fix
-import { User } from '`@/types/user`'; @@ - const [user, setUser] = useState<User>(); @@ - const getUser = (id: number) => { - axiosApi.get(`/users/${id}`).then((res) => { - setUser(res.data); - }); - }; @@ useEffect(() => { - if (props.reservation.userId) { - getUser(props.reservation.userId); - } if (props.reservation.bandId) { getBand(props.reservation.bandId); } }, []); @@ - {band?.name || user?.fullName || 'Loading...'} + {band?.name || props.reservation.user?.fullName || 'Loading...'}Also applies to: 110-110
🤖 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 `@apps/frontend/src/components/calendar/day/day-reservation.tsx` around lines 21 - 39, Remove the unnecessary getUser and getBand function calls that make individual API requests for each reservation card. Instead of using these functions in the useEffect hook, access the user and band data directly from the props.reservation object which already contains these fields. Delete the getUser and getBand function definitions and replace the useEffect hook logic with direct assignments from props.reservation.user and props.reservation.band to avoid N+1 API requests when rendering the calendar grid.
🟡 Minor comments (9)
apps/frontend/src/hooks/useReservationDetails.ts-247-262 (1)
247-262:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing error handling when unsetting gatekeeper.
The PATCH request to clear the gatekeeper has no
.catch()handler. If the request fails, the UI won't show an error and local state won't be updated, leaving the UI in an inconsistent state.🛡️ Proposed fix
if (priority === null && gateKeeperMembershipId && gateKeeperMembershipId === myMembership.id) { - axiosApi.patch(`/reservations/${props.clickedEvent.id}`, { gateKeeperId: null }).then(() => { + axiosApi.patch(`/reservations/${props.clickedEvent.id}`, { gateKeeperId: null }) + .then(() => { setGateKeeper(null); setGateKeeperMembershipId(null); setGateKeeperPriority(null); props.onGetData(); // Send email to all recipients if (emailRecipients.length > 0) { emailRecipients.forEach((email) => { handleSubmitMail('A beengedő visszamondta a foglalásod.', email, me!.email); }); } + }) + .catch((err: unknown) => { + showErrorToast(err); }); return; }🤖 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 `@apps/frontend/src/hooks/useReservationDetails.ts` around lines 247 - 262, The PATCH request to clear the gatekeeper (when priority is null and gateKeeperMembershipId matches myMembership.id) lacks error handling, which can leave the UI in an inconsistent state if the request fails. Add a .catch() handler after the .then() block for the axiosApi.patch call to `/reservations/${props.clickedEvent.id}` with gateKeeperId: null to properly handle errors, log them appropriately, and optionally notify the user of the failure so the UI state remains synchronized with the server state.apps/backend/src/reservations/reservations.controller.ts-28-31 (1)
28-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
parseIntreturnsNaNfor non-numeric strings, which could cause unexpected query behavior.If
gateKeeperIdis a non-numeric string like"abc",parseIntreturnsNaN. SinceNaNis truthy in the ternary, it gets passed tofindAll, potentially causing Prisma to behave unexpectedly.🛡️ Proposed fix
- const parsedGateKeeperId = gateKeeperId ? parseInt(gateKeeperId, 10) : undefined; + const parsedGateKeeperId = gateKeeperId ? parseInt(gateKeeperId, 10) : undefined; + if (parsedGateKeeperId !== undefined && isNaN(parsedGateKeeperId)) { + return this.reservationsService.findAll(page, pageSize, undefined); + } return this.reservationsService.findAll(page, pageSize, parsedGateKeeperId);Alternatively, use a
ParseIntPipewith optional transform:async findAll( `@Query`('page', ParseIntPipe) page: number, `@Query`('page_size', ParseIntPipe) pageSize: number, - `@Query`('gateKeeperId') gateKeeperId?: string + `@Query`('gateKeeperId', new ParseIntPipe({ optional: true })) gateKeeperId?: number ) { - const parsedGateKeeperId = gateKeeperId ? parseInt(gateKeeperId, 10) : undefined; - return this.reservationsService.findAll(page, pageSize, parsedGateKeeperId); + return this.reservationsService.findAll(page, pageSize, gateKeeperId); }🤖 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 `@apps/backend/src/reservations/reservations.controller.ts` around lines 28 - 31, The parseInt call on the gateKeeperId query parameter returns NaN for non-numeric strings, and since NaN is truthy it gets passed to findAll causing unexpected behavior. In the reservations.controller.ts method where gateKeeperId is being parsed, modify the ternary expression to validate that the parsed result is not NaN before returning it; if it is NaN, return undefined instead, ensuring only valid numeric IDs are passed to the reservationsService.findAll method.apps/frontend/src/components/calendar/isReservationOvertime.tsx-23-40 (1)
23-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winModule-level
startOfWeek/endOfWeekmay become stale.The week boundaries are computed once when the module is imported. If the user's session spans a week boundary (e.g., stays open from Sunday to Monday),
getReservationsOfWeekwill use outdated boundaries without a page refresh. Consider computing boundaries dynamically inside the function.Proposed fix
export function getReservationsOfWeek(reservations: Reservation[], bandId?: number, userId?: number): Reservation[] { + const startOfWeek = getFirstDayOfWeek(); + const endOfWeek = getLastDayOfWeek(); return reservations.filter((reservation) => {And remove the module-level constants.
🤖 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 `@apps/frontend/src/components/calendar/isReservationOvertime.tsx` around lines 23 - 40, The module-level constants startOfWeek and endOfWeek are computed once when the module loads, causing them to become stale if a user's session spans a week boundary without a page refresh. Move the computation of startOfWeek and endOfWeek from module scope into the getReservationsOfWeek function so that week boundaries are calculated dynamically each time the function is called, ensuring they always reflect the current week. Remove the original module-level constant declarations after moving the logic inside the function.apps/frontend/docs/PAGE_PROTECTION.md-144-155 (1)
144-155:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the file-structure snippet (and add a fenced language).
This block currently fails markdownlint (missing language) and shows a Pages Router tree that doesn’t match the App Router structure used in this PR.
💡 Suggested fix
-``` +```text src/ ├── middleware.ts # Server-side route protection ├── utils/ │ └── withAuth.tsx # Client-side HOC for role-based access -├── pages/ -│ ├── admin/ -│ │ └── index.tsx # Example admin page -│ └── login.tsx # Login page (public) +├── app/ +│ ├── admin/ +│ │ └── page.tsx # Admin page +│ └── login/ +│ └── route.ts # Login route └── hooks/ - └── useUser.ts # Hook for current user data + └── useUser.tsx # Hook for current user data</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@apps/frontend/docs/PAGE_PROTECTION.mdaround lines 144 - 155, Update the
file structure snippet in PAGE_PROTECTION.md to add a language identifier to the
code fence and replace the Pages Router directory structure with the App Router
structure. Add "text" as the language specification to the opening code fence,
then replace "pages/" directory with "app/", change "admin/index.tsx" to
"admin/page.tsx", replace "login.tsx" with "login/route.ts", and update the
useUser hook file extension from ".ts" to ".tsx". This will fix the markdownlint
warning and ensure the documentation accurately reflects the App Router
structure used in this PR.</details> <!-- cr-comment:v1:d61b41f00f1614d1a70eb004 --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>apps/frontend/src/components/admin/PeriodsPanel.tsx-51-57 (1)</summary><blockquote> `51-57`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Validate period range before creating.** The create flow only checks presence, not ordering. Reject `newStart > newEnd` on the client to prevent invalid submissions and inconsistent UX. <details> <summary>Suggested patch</summary> ```diff const handleCreate = async () => { if (!newStart || !newEnd) return; + if (new Date(newStart) > new Date(newEnd)) { + console.error('Invalid period range: startDate is after endDate'); + return; + } try { await axiosApi.post('/periods', {🤖 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 `@apps/frontend/src/components/admin/PeriodsPanel.tsx` around lines 51 - 57, The handleCreate function only validates that newStart and newEnd are present, but does not validate that the start date is not after the end date. Add a validation check after the presence check to ensure newStart is not greater than newEnd, and return early or show an error message if this condition is violated to prevent invalid period submissions to the server.apps/frontend/src/app/my-gatekeeps/page.tsx-75-83 (1)
75-83:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse strict integer validation for sanction points.
Line 77/80 currently uses
parseInt, which silently truncates non-integer inputs (e.g.1.9), so invalid values can pass as valid positive integers.Suggested fix
- const pointsToAdd = parseInt(inputValue, 10); + const pointsToAdd = Number(inputValue); @@ - if (isNaN(pointsToAdd) || pointsToAdd <= 0) { + if (!Number.isInteger(pointsToAdd) || pointsToAdd <= 0) { toast.error('Kérlek adj meg egy pozitív egész számot!'); return; }🤖 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 `@apps/frontend/src/app/my-gatekeeps/page.tsx` around lines 75 - 83, The handleSaveSanctionPoints function currently uses parseInt to validate sanction points input, but parseInt silently truncates decimal values (e.g., "1.9" becomes 1), allowing invalid non-integer inputs to pass validation. Replace or supplement the current parseInt validation with strict integer validation that explicitly rejects non-integer numeric strings. Use Number.isInteger() or a regex pattern check to ensure the inputValue represents a whole number before or after parsing, so that decimal inputs like "1.9" are properly rejected with the error message.apps/frontend/src/lib/errorToast.ts-14-16 (1)
14-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve server-error classification when backend includes a message.
If a 5xx response has
data.message, this branch marks it as non-server (isServerError: false), so server-error UX handling is skipped.Suggested fix
- if (serverMsg) { - return { message: serverMsg, isServerError: false }; + if (typeof serverMsg === 'string' && serverMsg.trim()) { + return { message: serverMsg, isServerError: typeof status === 'number' && status >= 500 }; }🤖 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 `@apps/frontend/src/lib/errorToast.ts` around lines 14 - 16, In the errorToast.ts file, the conditional block that checks if serverMsg exists is returning isServerError: false, but this incorrectly misclassifies 5xx server errors as non-server errors. When a 5xx response includes a message in data.message, the serverMsg branch should return isServerError: true to properly preserve the server-error classification and ensure server-error UX handling is applied. Change the isServerError value from false to true in the return statement within the if (serverMsg) block.apps/backend/src/sanction-records/dto/create-sanction-record.dto.ts-4-14 (1)
4-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConstrain relation IDs to positive integers.
userId,bandId, andreservationIdshould reject negative/fractional values at DTO level instead of deferring failure to persistence.Proposed patch
-import { IsNotEmpty, IsNumber, IsOptional, IsPositive, IsString } from 'class-validator'; +import { IsInt, IsNotEmpty, IsOptional, IsPositive, IsString } from 'class-validator'; export class CreateSanctionRecordDto { - `@IsNumber`() + `@IsInt`() + `@IsPositive`() `@IsOptional`() userId?: number; - `@IsNumber`() + `@IsInt`() + `@IsPositive`() `@IsOptional`() bandId?: number; - `@IsNumber`() + `@IsInt`() + `@IsPositive`() `@IsOptional`() reservationId?: number;🤖 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 `@apps/backend/src/sanction-records/dto/create-sanction-record.dto.ts` around lines 4 - 14, The userId, bandId, and reservationId fields in the CreateSanctionRecordDto class currently only validate that they are numbers with `@IsNumber`(), but do not enforce that they must be positive integers. Add validation decorators to these three fields to ensure they reject negative values and fractional numbers at the DTO level. Use decorators that validate for positive values and integer types (such as `@IsPositive`() and `@IsInt`() or equivalent) to catch invalid values during request validation rather than deferring the failure to the database layer.apps/backend/src/sanction-records/dto/update-sanction-record.dto.ts-9-11 (1)
9-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrevent blank
reasonupdates.Optional
reasoncurrently accepts"", which weakens sanction auditability. Keep it optional, but non-empty when provided.Proposed patch
-import { IsNumber, IsOptional, IsPositive, IsString } from 'class-validator'; +import { IsNotEmpty, IsNumber, IsOptional, IsPositive, IsString } from 'class-validator'; ... `@IsString`() `@IsOptional`() + `@IsNotEmpty`() reason?: string;🤖 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 `@apps/backend/src/sanction-records/dto/update-sanction-record.dto.ts` around lines 9 - 11, The `reason` field in the UpdateSanctionRecordDto class currently accepts empty strings due to missing validation. Add the `@MinLength`(1) decorator from class-validator to the `reason` property alongside the existing `@IsString`() and `@IsOptional`() decorators to ensure that when a reason is provided, it must be non-empty while still allowing the field to remain optional.
🧹 Nitpick comments (10)
apps/backend/src/reservations/reservations.service.spec.ts (1)
9-9: 💤 Low valueRemove commented-out code.
Lines 9 and 81 have commented-out
prismavariable declarations that are unused.let service: ReservationsService; - //let prisma: PrismaService;And at line 81:
service = module.get<ReservationsService>(ReservationsService); - //prisma = module.get<PrismaService>(PrismaService); });🤖 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 `@apps/backend/src/reservations/reservations.service.spec.ts` at line 9, Remove the unused commented-out variable declarations for prisma in the reservations.service.spec.ts test file. Locate and delete the commented-out line containing `//let prisma: PrismaService;` at line 9 and the similar commented-out prisma declaration at line 81. These lines are not being used in the test suite and should be removed to keep the codebase clean.apps/frontend/src/components/calendar/reservation-details.tsx (1)
196-204: 💤 Low valueDead code in button className.
The ternary
gateKeeper === null ? '...' : '...'is inside a block that only renders whengateKeeper !== null(line 196), so the first branch is unreachable. The button always renders withbg-secondarystyling.♻️ Simplify the className
<button - className={`px-4 py-2 text-sm font-medium text-white rounded-md transition-colors - ${gateKeeper === null ? 'bg-primary hover:bg-primary/90 text-primary-foreground' : 'bg-secondary hover:bg-secondary/80 text-secondary-foreground'}`} + className='px-4 py-2 text-sm font-medium text-secondary-foreground bg-secondary hover:bg-secondary/80 rounded-md transition-colors' onClick={() => onSetGK(null)} >🤖 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 `@apps/frontend/src/components/calendar/reservation-details.tsx` around lines 196 - 204, The className in the button element contains a ternary condition checking if gateKeeper === null, but this button only renders inside a conditional block that already ensures gateKeeper !== null. This makes the first branch of the ternary unreachable dead code. Remove the ternary condition from the className prop and use only the second branch styling (bg-secondary hover:bg-secondary/80 text-secondary-foreground) directly, since the gateKeeper is guaranteed to be non-null at this point.apps/backend/src/reservations/reservations.service.ts (2)
500-513: ⚡ Quick winType safety lost with
as anycasts.Casting to
anybypasses TypeScript checks on lines 509 and 512. If the merged DTO is missing required fields, the error surfaces at runtime rather than compile time.♻️ Proposed fix
Create a properly typed merged object or adjust the method signatures to accept partial DTOs:
const dto = { - userId: existing.userId, - bandId: existing.bandId, + userId: existing.userId ?? undefined, + bandId: existing.bandId ?? undefined, startTime: existing.startTime, endTime: existing.endTime, status: existing.status, ...updateReservationDto, - }; - await this.validateReservation(dto as any, id); + } as CreateReservationDto; + await this.validateReservation(dto, id); // Recalculate status if time or user/band changes - const newStatus = await this.determineReservationStatus(dto as any, id); + const newStatus = await this.determineReservationStatus(dto, id);Alternatively, define a shared type that both DTOs can satisfy for these validation methods.
🤖 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 `@apps/backend/src/reservations/reservations.service.ts` around lines 500 - 513, The code merges updateReservationDto with existing reservation fields into a dto object, then casts it to any before passing to validateReservation and determineReservationStatus methods on lines 509 and 512. This bypasses TypeScript type checking and can hide missing required fields until runtime. Instead of using as any casts on the dto variable, either adjust the method signatures of validateReservation and determineReservationStatus to accept a properly typed partial DTO that reflects the merged object structure, or create an intermediate variable with explicit typing that includes all required fields from both the existing reservation and the update DTO before passing it to these validation methods.
197-209: ⚖️ Poor tradeoffN+1 query pattern in band eligibility check.
For each band in
otherBands, a separate query fetches its memberships. With many bands, this becomes a performance bottleneck.♻️ Proposed optimization
Fetch all band memberships in one query and group in-memory:
- const otherBands = await this.prisma.band.findMany({ - where: { id: { not: bandId } }, - select: { id: true }, - }); - - for (const other of otherBands) { - const otherIds = await acceptedMemberIds(other.id); - if (otherIds.length === memberIds.length && otherIds.join(',') === fingerprint) { - throw new BadRequestException( - 'Egy másik, pontosan ugyanezekből a tagokból álló zenekar is létezik, ezért ez a zenekar nem foglalhat.' - ); - } - } + const allMemberships = await this.prisma.bandMembership.findMany({ + where: { bandId: { not: bandId }, status: BandMembershipStatus.ACCEPTED }, + select: { bandId: true, userId: true }, + }); + + const groupedByBand = new Map<number, number[]>(); + for (const m of allMemberships) { + if (m.userId === null) continue; + const arr = groupedByBand.get(m.bandId) ?? []; + arr.push(m.userId); + groupedByBand.set(m.bandId, arr); + } + + for (const [, otherIds] of groupedByBand) { + otherIds.sort((a, b) => a - b); + if (otherIds.length === memberIds.length && otherIds.join(',') === fingerprint) { + throw new BadRequestException( + 'Egy másik, pontosan ugyanezekből a tagokból álló zenekar is létezik, ezért ez a zenekar nem foglalhat.' + ); + } + }🤖 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 `@apps/backend/src/reservations/reservations.service.ts` around lines 197 - 209, The code exhibits an N+1 query pattern by calling acceptedMemberIds inside the loop for each band in otherBands, resulting in a database query per iteration. Instead, fetch all member data for all bands in a single query before the loop, then group the results in-memory by band ID using a map or similar structure. Replace the individual acceptedMemberIds calls inside the for loop with lookups against the pre-computed grouped data structure to eliminate the redundant database queries.apps/frontend/src/lib/reservationSubmitter.ts (1)
147-162: ⚖️ Poor tradeoffNon-atomic split reservation creation may leave partial state on failure.
If the first
POST /reservations(NORMAL) succeeds but the second (OVERTIME) fails, the user ends up with only the NORMAL portion saved. Consider wrapping in a try/catch that deletes the first reservation on failure, or document this as expected behavior.🤖 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 `@apps/frontend/src/lib/reservationSubmitter.ts` around lines 147 - 162, The two sequential axiosApi.post calls to create NORMAL and OVERTIME reservations are not atomic, meaning if the first POST succeeds but the second fails, the system is left with only a partial NORMAL reservation. Wrap both POST calls in a try/catch block and implement rollback logic that deletes the first reservation if the second POST call fails. Alternatively, if this partial state is acceptable, add clear documentation explaining this expected behavior.apps/frontend/src/types/openedWeek.ts (1)
3-3: ⚡ Quick winNarrow
mondayto a single wire format type.Using
Date | stringmakes consumers handle two runtime shapes for the same API field. Prefer a single serialized type (typically ISOstring) at the boundary, and convert toDateonly where needed.🤖 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 `@apps/frontend/src/types/openedWeek.ts` at line 3, The `monday` property in the openedWeek type accepts both Date and string types, creating ambiguous runtime shapes at the API boundary. Change the type of the `monday` property from `Date | string` to just `string` to enforce a single serialized wire format (ISO 8601 string), and handle conversion to Date objects only in the consuming code where necessary.apps/frontend/src/types/period.ts (1)
3-4: ⚡ Quick winUse one boundary type for
startDate/endDate.
Date | stringweakens the contract and pushes normalization to every consumer. Prefer serializedstringin this shared type, then parse locally where date math/display is required.🤖 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 `@apps/frontend/src/types/period.ts` around lines 3 - 4, In the period type definition, change both startDate and endDate properties to use only the string type instead of Date | string union. This standardizes on a serialized string format for date boundaries in the shared type contract, eliminating ambiguity and pushing the responsibility of date parsing to individual consumers where date operations or display formatting is actually needed.apps/backend/prisma/migrations/20260326114226_init/migration.sql (1)
2-5: ⚡ Quick winAdd an index for
SanctionRecord.reservationIdto avoid FK-related scan bottlenecks.The FK is correct, but without an index this can degrade join performance and increase lock/scan cost on
Reservationupdates/deletes as data grows.Suggested migration addition
-- AlterTable ALTER TABLE "SanctionRecord" ADD COLUMN "reservationId" INTEGER; -- AddForeignKey ALTER TABLE "SanctionRecord" ADD CONSTRAINT "SanctionRecord_reservationId_fkey" FOREIGN KEY ("reservationId") REFERENCES "Reservation"("id") ON DELETE SET NULL ON UPDATE CASCADE; + +-- CreateIndex +CREATE INDEX "SanctionRecord_reservationId_idx" ON "SanctionRecord"("reservationId");🤖 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 `@apps/backend/prisma/migrations/20260326114226_init/migration.sql` around lines 2 - 5, The migration adds a foreign key constraint for SanctionRecord.reservationId that references Reservation.id, but does not create an index on the reservationId column. Add a CREATE INDEX statement after the foreign key constraint to index the reservationId column on the SanctionRecord table. This will improve join performance and reduce lock/scan costs when performing updates or deletes on the Reservation table.apps/backend/src/bands/bands.service.ts (1)
27-30: ⚡ Quick winKeep
findOnemember projection consistent withfindAll.
findAllincludesmembers.user.fullName, butfindOnenow returns rawmembersonly. Returning two different shapes for the same resource increases client-side branching.Refactor suggestion
- const res = await this.prisma.band.findUniqueOrThrow({ - where: { id }, - include: { members: true }, - }); + const res = await this.prisma.band.findUniqueOrThrow({ + where: { id }, + include: { members: { include: { user: { select: { fullName: true } } } } }, + });🤖 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 `@apps/backend/src/bands/bands.service.ts` around lines 27 - 30, The findUniqueOrThrow call in the findOne method (around line 27-30) is returning raw members without projecting the nested user.fullName field, while findAll returns members with the user.fullName projection. Update the include clause in the findUniqueOrThrow call to match the member projection structure used in findAll, ensuring both methods return the same shape for the members resource. This will eliminate inconsistent data shapes between the two endpoints.apps/backend/src/opened-weeks/dto/update-opened-week.dto.ts (1)
4-6: ⚡ Quick winEnsure
mondayis truly the week anchor date.
@IsDateString()accepts any valid date, so non-Monday values can be stored and split week-level open/closed state across multiple keys. Add an explicit weekday check (or normalize to start-of-week before persistence).🤖 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 `@apps/backend/src/opened-weeks/dto/update-opened-week.dto.ts` around lines 4 - 6, The `monday` field in UpdateOpenedWeekDTO only validates that it is a valid date string using `@IsDateString`(), but does not verify that the date is actually a Monday. Add a custom validator (either a custom decorator or validation logic) to the `monday` property that checks the weekday of the provided date and ensures it is a Monday (dayOfWeek === 1 or similar, depending on your date library), or alternatively normalize the provided date to the start of its week before persistence to ensure data consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2aa7d8de-175e-4383-9b79-3fa949d0064d
⛔ Files ignored due to path filters (2)
avatar.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (120)
.eslintrc.js.prettierignoreapps/backend/jest.config.jsapps/backend/package.jsonapps/backend/prisma/migrations/20260109180915_reservation_enhancements/migration.sqlapps/backend/prisma/migrations/20260116023247_kolis_fogadas/migration.sqlapps/backend/prisma/migrations/20260116170512_add_sanction_records/migration.sqlapps/backend/prisma/migrations/20260320130812_add_gatekeeper_priority/migration.sqlapps/backend/prisma/migrations/20260322135731_remove_sanction_type/migration.sqlapps/backend/prisma/migrations/20260322155651_add_weeks_and_extra_settings_attributes/migration.sqlapps/backend/prisma/migrations/20260323103330_removed_unnecessary_reservation_config_models/migration.sqlapps/backend/prisma/migrations/20260326114226_init/migration.sqlapps/backend/prisma/migrations/20260618150000_align_reservation_quota_defaults/migration.sqlapps/backend/prisma/schema.prismaapps/backend/src/admin/admin.controller.tsapps/backend/src/admin/admin.module.tsapps/backend/src/admin/admin.service.tsapps/backend/src/admin/dto/set-role.dto.tsapps/backend/src/admin/dto/update-config.dto.tsapps/backend/src/admin/entities/reservation-config.entity.tsapps/backend/src/app.module.tsapps/backend/src/bands/bands.service.tsapps/backend/src/bands/bandsController.tsapps/backend/src/comments/comments.controller.tsapps/backend/src/comments/comments.service.tsapps/backend/src/main.tsapps/backend/src/memberships/memberships.controller.tsapps/backend/src/opened-weeks/dto/update-opened-week.dto.tsapps/backend/src/opened-weeks/opened-weeks.controller.tsapps/backend/src/opened-weeks/opened-weeks.module.tsapps/backend/src/opened-weeks/opened-weeks.service.tsapps/backend/src/periods/dto/create-period.dto.tsapps/backend/src/periods/dto/update-period.dto.tsapps/backend/src/periods/periods.controller.tsapps/backend/src/periods/periods.module.tsapps/backend/src/periods/periods.service.tsapps/backend/src/posts/posts.controller.tsapps/backend/src/reservations/dto/simple-reservation.dto.tsapps/backend/src/reservations/dto/update-reservation.dto.tsapps/backend/src/reservations/entities/reservation.entity.tsapps/backend/src/reservations/reservations.controller.tsapps/backend/src/reservations/reservations.service.spec.tsapps/backend/src/reservations/reservations.service.tsapps/backend/src/sanction-records/dto/create-sanction-record.dto.tsapps/backend/src/sanction-records/dto/update-sanction-record.dto.tsapps/backend/src/sanction-records/entities/sanction-record.entity.tsapps/backend/src/sanction-records/sanction-records.controller.tsapps/backend/src/sanction-records/sanction-records.module.tsapps/backend/src/sanction-records/sanction-records.service.tsapps/backend/src/settings/dto/update-settings.dto.tsapps/backend/src/settings/settings.controller.tsapps/backend/src/settings/settings.module.tsapps/backend/src/settings/settings.service.tsapps/backend/src/users/users.controller.tsapps/backend/src/users/users.service.tsapps/frontend/declarations.d.tsapps/frontend/docs/PAGE_PROTECTION.mdapps/frontend/package.jsonapps/frontend/src/app/admin/page.tsxapps/frontend/src/app/globals.cssapps/frontend/src/app/login/route.tsapps/frontend/src/app/my-gatekeeps/page.tsxapps/frontend/src/app/profile/page.tsxapps/frontend/src/app/register/route.tsapps/frontend/src/app/reservation/page.tsxapps/frontend/src/app/rules/page.tsxapps/frontend/src/app/stats/page.tsxapps/frontend/src/components/admin/OpenedWeeksPanel.tsxapps/frontend/src/components/admin/PeriodsPanel.tsxapps/frontend/src/components/admin/SettingsPanel.tsxapps/frontend/src/components/admin/reservation-limits-form.tsxapps/frontend/src/components/band/band-form-dialog.tsxapps/frontend/src/components/calendar/Line.tsxapps/frontend/src/components/calendar/add-comment.tsxapps/frontend/src/components/calendar/add-panel.tsxapps/frontend/src/components/calendar/add-reservation.tsxapps/frontend/src/components/calendar/calendar.tsxapps/frontend/src/components/calendar/comment-details.tsxapps/frontend/src/components/calendar/day/daily-view-without-date.tsxapps/frontend/src/components/calendar/day/daily-view.tsxapps/frontend/src/components/calendar/day/day-comment.tsxapps/frontend/src/components/calendar/day/day-reservation.tsxapps/frontend/src/components/calendar/interval-swithcer.tsxapps/frontend/src/components/calendar/isReservationOvertime.tsxapps/frontend/src/components/calendar/reservation-details.tsxapps/frontend/src/components/calendar/reservation-legend.tsxapps/frontend/src/components/calendar/time-picker.tsxapps/frontend/src/components/calendar/validDate.tsxapps/frontend/src/components/calendar/week/daily-weekly-view.tsxapps/frontend/src/components/layout/footer.tsxapps/frontend/src/components/layout/header.tsxapps/frontend/src/components/layout/player.tsxapps/frontend/src/components/layout/sidebar.tsxapps/frontend/src/components/layout/theme-toggle.tsxapps/frontend/src/components/member/member-tile.tsxapps/frontend/src/components/news/news-card.tsxapps/frontend/src/components/profile/profile-page.tsxapps/frontend/src/components/ui/badge.tsxapps/frontend/src/components/ui/button.tsxapps/frontend/src/components/ui/card.tsxapps/frontend/src/components/ui/rules-card.tsxapps/frontend/src/hooks/collisionWithAdminRes.tsxapps/frontend/src/hooks/useAdminConfig.tsapps/frontend/src/hooks/useQuotaCheck.tsapps/frontend/src/hooks/useReservationDetails.tsapps/frontend/src/hooks/useUser.tsxapps/frontend/src/hooks/useWeekStatus.tsapps/frontend/src/lib/errorToast.tsapps/frontend/src/lib/reservationSubmitter.tsapps/frontend/src/middleware.tsapps/frontend/src/mocks/users.tsapps/frontend/src/types/admin.tsapps/frontend/src/types/openedWeek.tsapps/frontend/src/types/period.tsapps/frontend/src/types/reservation.tsapps/frontend/src/types/sanction-record.tsapps/frontend/src/types/settings.tsapps/frontend/src/types/user.tsapps/frontend/src/utils/withAuth.tsxproject_info/guidelines.md
💤 Files with no reviewable changes (9)
- apps/backend/src/admin/admin.module.ts
- apps/backend/src/admin/dto/update-config.dto.ts
- apps/frontend/src/hooks/useAdminConfig.ts
- apps/frontend/src/types/admin.ts
- apps/backend/src/admin/dto/set-role.dto.ts
- apps/backend/src/admin/admin.controller.ts
- apps/frontend/src/components/admin/reservation-limits-form.tsx
- apps/backend/src/admin/entities/reservation-config.entity.ts
- apps/backend/src/admin/admin.service.ts
- users/me now includes clubMembership so gatekeepers keep their role - comments.findAll counts comments instead of reservations - guard NaN gateKeeperId query param in reservations controller - constrain sanction relation IDs to positive ints; forbid blank reason - errorToast classifies 5xx-with-message as server error - compute week bounds per call to avoid stale week window - read user/band from reservation prop (drop per-card N+1 lookups) - validate period range; strict integer sanction points - error handling when unsetting gatekeeper; remove dead className branch - bands.findOne member projection matches findAll Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/backend/src/comments/comments.service.ts (1)
140-148: Import and useReservationStatusenum instead of string literal for type safety.The status check uses the string literal
'ADMINMADE'instead of the enumReservationStatus.ADMINMADEfrom@prisma/client. Using the enum ensures consistency across the codebase and prevents issues if the enum value changes.Suggested change
-import { Prisma } from '`@prisma/client`'; +import { Prisma, ReservationStatus } from '`@prisma/client`';where: { AND: [ { startTime: { lt: endTime } }, { endTime: { gt: startTime } }, - { status: { not: 'ADMINMADE' } }, + { status: { not: ReservationStatus.ADMINMADE } }, ], },🤖 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 `@apps/backend/src/comments/comments.service.ts` around lines 140 - 148, In the deleteMany call within the comments service, replace the string literal 'ADMINMADE' in the status filter condition with the ReservationStatus enum value ReservationStatus.ADMINMADE. First ensure that ReservationStatus is imported from `@prisma/client` at the top of the file, then update the where clause to use the enum instead of the hardcoded string to maintain type safety and consistency across the codebase.apps/frontend/src/components/calendar/day/day-reservation.tsx (1)
25-25: ⚡ Quick winPrefer enum constants over string literals for type safety.
The status checks use string literals
'ADMINMADE'and'OVERTIME'instead of theReservationStatusenum constants. While this works, using enum values provides better type safety and refactoring support.♻️ Refactor to use enum constants
const getReservationColor = (): { bg: string; border: string } => { // 1. Admin-made reservations (purple) - if (props.reservation.status === 'ADMINMADE') { + if (props.reservation.status === ReservationStatus.ADMINMADE) { return { bg: 'bg-gradient-to-r from-violet-600 to-violet-500', border: 'border-violet-700', }; } // 2. Overtime reservations (blue) - if (props.reservation.status === 'OVERTIME') { + if (props.reservation.status === ReservationStatus.OVERTIME) { return { bg: 'bg-gradient-to-r from-blue-600 to-blue-500', border: 'border-blue-700', }; }Also applies to: 33-33
🤖 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 `@apps/frontend/src/components/calendar/day/day-reservation.tsx` at line 25, Replace the string literal comparisons for reservation status with enum constants from the ReservationStatus enum to improve type safety and maintainability. In the day-reservation.tsx file, change the status comparisons at lines 25 and 33 where you check props.reservation.status against 'ADMINMADE' and 'OVERTIME' string literals. Instead, use the corresponding constants from the ReservationStatus enum (such as ReservationStatus.ADMINMADE and ReservationStatus.OVERTIME). This ensures that any refactoring to the enum values will be caught at compile time rather than at runtime.
🤖 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.
Nitpick comments:
In `@apps/backend/src/comments/comments.service.ts`:
- Around line 140-148: In the deleteMany call within the comments service,
replace the string literal 'ADMINMADE' in the status filter condition with the
ReservationStatus enum value ReservationStatus.ADMINMADE. First ensure that
ReservationStatus is imported from `@prisma/client` at the top of the file, then
update the where clause to use the enum instead of the hardcoded string to
maintain type safety and consistency across the codebase.
In `@apps/frontend/src/components/calendar/day/day-reservation.tsx`:
- Line 25: Replace the string literal comparisons for reservation status with
enum constants from the ReservationStatus enum to improve type safety and
maintainability. In the day-reservation.tsx file, change the status comparisons
at lines 25 and 33 where you check props.reservation.status against 'ADMINMADE'
and 'OVERTIME' string literals. Instead, use the corresponding constants from
the ReservationStatus enum (such as ReservationStatus.ADMINMADE and
ReservationStatus.OVERTIME). This ensures that any refactoring to the enum
values will be caught at compile time rather than at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b3e3a101-9982-45fb-be92-609dfd3ce531
📒 Files selected for processing (18)
apps/backend/jest.config.jsapps/backend/src/bands/bands.service.tsapps/backend/src/comments/comments.service.tsapps/backend/src/reservations/reservations.controller.tsapps/backend/src/reservations/reservations.service.spec.tsapps/backend/src/sanction-records/dto/create-sanction-record.dto.tsapps/backend/src/sanction-records/dto/update-sanction-record.dto.tsapps/backend/src/users/users.service.tsapps/frontend/docs/PAGE_PROTECTION.mdapps/frontend/src/app/my-gatekeeps/page.tsxapps/frontend/src/components/admin/PeriodsPanel.tsxapps/frontend/src/components/calendar/day/day-reservation.tsxapps/frontend/src/components/calendar/isReservationOvertime.tsxapps/frontend/src/components/calendar/reservation-details.tsxapps/frontend/src/hooks/useReservationDetails.tsapps/frontend/src/lib/errorToast.tsapps/frontend/src/types/openedWeek.tsapps/frontend/src/types/period.ts
💤 Files with no reviewable changes (1)
- apps/backend/src/reservations/reservations.service.spec.ts
✅ Files skipped from review due to trivial changes (1)
- apps/frontend/docs/PAGE_PROTECTION.md
🚧 Files skipped from review as they are similar to previous changes (13)
- apps/frontend/src/types/openedWeek.ts
- apps/backend/jest.config.js
- apps/backend/src/sanction-records/dto/update-sanction-record.dto.ts
- apps/backend/src/bands/bands.service.ts
- apps/backend/src/sanction-records/dto/create-sanction-record.dto.ts
- apps/frontend/src/lib/errorToast.ts
- apps/frontend/src/components/admin/PeriodsPanel.tsx
- apps/backend/src/reservations/reservations.controller.ts
- apps/frontend/src/hooks/useReservationDetails.ts
- apps/frontend/src/components/calendar/reservation-details.tsx
- apps/backend/src/users/users.service.ts
- apps/frontend/src/app/my-gatekeeps/page.tsx
- apps/frontend/src/components/calendar/isReservationOvertime.tsx
…ents and day reservation components
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
Release Notes
New Features
Improvements
Removals