Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Organizers module (controller/service/DTOs + tests) to manage event organizers via AdminPermission records, and wires it into the Nest application.
Changes:
- Introduces
OrganizersController+OrganizersServiceimplementing CRUD-ish endpoints for organizers under/events/:eventId/organizers. - Adds DTOs for organizer listing, creation, update, and Swagger response types.
- Adds unit + “integration” (controller-level) Jest tests and registers
OrganizersModuleinAppModule.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/organizers/organizers.service.ts | Implements organizer assignment, listing, lookup, permission update, and removal logic using Prisma. |
| src/organizers/organizers.controller.ts | Exposes REST endpoints + Swagger decorators for the organizers feature. |
| src/organizers/organizers.module.ts | Declares the Organizers module. |
| src/organizers/dto/create-organizer.dto.ts | Defines request shape for assigning an organizer. |
| src/organizers/dto/update-organizer.dto.ts | Defines request shape for updating organizer permissions. |
| src/organizers/dto/organizer-listing.dto.ts | Adds pagination + isActive filtering for organizer listing. |
| src/organizers/dto/organizer-response.dto.ts | Declares Swagger response DTOs for organizers and permissions. |
| src/organizers/organizers.service.spec.ts | Unit tests for OrganizersService. |
| src/organizers/organizers.int.spec.ts | Controller-level tests for OrganizersController. |
| src/app.module.ts | Registers OrganizersModule in the app. |
| src/events/events.service.ts | Removes an extraneous stray character/whitespace in findAll. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
loudsheep
left a comment
There was a problem hiding this comment.
Wszystko generalnie ładnie napisane, dodałem pare suesti oraz tam sie copilot pruje standardowo o literówki xd.
Uwagi o schema bardzo dobre, też mam takie wrażenie że można dużo poprawić jeżeli chodzi o cztelność tego wszystkiego. Permisje omówimy na kolejnym weekly zapewne bo to jest taka kalka 1:1 z eventownika v2 i imo można to lepiej rozwiązać (np przez te enumy tak jak pisałeś)
Closes #7
Wszystkie funkcjonalności tak jak w v2, z jednym wyjątkiem:
POST
/events/:eventId/organizersw sytuacji gdy admin nie istnieje, zwraca nam 404 zamiast go tworzyć.Podczas pisania znalazłem kilka rzeczy które można by zmienić w schemie:
adminPermissionspola (eventUuid, permissionUuid, adminUuid) nie są unikalne, wiec możemy np. nadać temu samemu organizatorowi to samo uprawnienie w tym samym evencie nieskończoność razy.Permissions- czy ona jest nam na pewno potrzebna? moglibyśmy to zastąpic listą enumów, np. permissions = ["READ", "ANALYTICS", "DELETE" itd.], bo obecnie, jeśli chcemy nadać 2 adminom po 3 uprawnienia, mamy w bazie 6 rekordów. Zdaje sobie sprawę, że jednak jakiś powód dla którego korzystamy z tej tabeli istnieje, więc daj znać czy ma to jakikolwiek sens.Nic z powyższych nie zmieniałem, bo to kwestia dyskusyjna, więc czekam na feedback ;)