-
Notifications
You must be signed in to change notification settings - Fork 0
feature: Events #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feature: Events #53
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a comprehensive Events feature to the application, adding full CRUD functionality for managing academic events with filtering capabilities by year and faculty. The implementation follows the existing NestJS patterns in the codebase and includes database seeding for development/testing purposes.
Key changes:
- Added Event entity with relationships to Faculty, supporting flexible date configurations for various event types (single-day, period, TBD, until-deadline)
- Implemented EventsService with create, read, update, delete operations and filtering by year/facultyId
- Created REST API endpoints with JWT authentication for mutating operations and Swagger documentation
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/events/event.entity.ts | Defines the Event entity with fields for name, description, year, date ranges, and faculty relationship |
| src/events/events.service.ts | Implements business logic for CRUD operations on events with filtering support |
| src/events/events.controller.ts | Exposes REST API endpoints with authentication guards and Swagger documentation |
| src/events/events.module.ts | Configures the Events module with TypeORM integration |
| src/events/dto/create-event.dto.ts | Defines validation rules for creating events including facultyId |
| src/events/dto/update-event.dto.ts | Extends CreateEventDto as a partial type for updates |
| src/events/dto/event-filter.dto.ts | Defines query parameters for filtering events by year and faculty |
| src/events/events.service.spec.ts | Adds basic unit test setup for EventsService |
| src/events/events.controller.spec.ts | Adds basic unit test setup for EventsController |
| src/faculties/faculty.entity.ts | Adds OneToMany relationship to Event entities |
| src/app.module.ts | Registers EventsModule in the application |
| src/database/seed.ts | Adds Event entity to seeding configuration |
| src/database/factories/event.factory.ts | Provides factory with 26 event templates based on academic calendar |
| src/database/seeds/1-user.seeder.ts | Seeds 5 user records for testing |
| src/database/seeds/2-faculty.seeder.ts | Seeds 10 faculty records for testing |
| src/database/seeds/3-event.seeder.ts | Seeds 20 event records with random year and faculty assignments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/events/events.service.ts
Outdated
| if (!updatedEvent) { | ||
| throw new NotFoundException( | ||
| `Event with id ${id} not found after update`, | ||
| ); | ||
| } |
Copilot
AI
Dec 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The update method has unnecessary complexity. After updating an entity with eventRepo.update(), you can directly query the updated entity instead of checking for null twice. The entity existence is already verified before the update, and the update operation doesn't delete the entity. The second null check on line 55-59 is redundant and will never be reached in normal operation.
| if (!updatedEvent) { | |
| throw new NotFoundException( | |
| `Event with id ${id} not found after update`, | |
| ); | |
| } |
src/events/events.service.ts
Outdated
| create(createEventDto: CreateEventDto): Promise<Event> { | ||
| return this.eventRepository.manager.transaction(async (manager) => { | ||
| const eventRepo = manager.getRepository(Event); | ||
| const event = eventRepo.create(createEventDto); |
Copilot
AI
Dec 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CreateEventDto uses facultyId as a number, but the service directly passes the DTO to create() without transforming facultyId into a faculty relationship object. TypeORM's create method won't automatically convert facultyId to the proper faculty relationship. This will likely result in the faculty not being associated with the event, or a database error.
| const event = eventRepo.create(createEventDto); | |
| const { facultyId, ...rest } = createEventDto as any; | |
| const event = eventRepo.create({ | |
| ...rest, | |
| ...(facultyId && { faculty: { id: facultyId } }), | |
| }); |
Closes #45
Closes #47