-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor/database v3 migration #326
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?
Conversation
This reverts commit 565a32b.
the current dockerfile was using the `npm i` command instead of `npm ci`, which allowed it to automatically & silently update dependencies. this allowed for issues, where it could update a dependency to one that would break the build (but only if it was a minor/patch update, i.e. the dependency devs made a mistake) coincidentally, `adonis-autoswagger` devs appear to have just made that exact mistake, adding a new required field on a public type in a minor update. this commit fixes this issue by using `npm ci` instead. addtionally, there was no `.dockerignore` file, which combined with the `ADD . .` command just before the build in the `Dockerfile` caused the build to actually use the dependencies installed in the local repo, outside of the container image build environment, by copying the local `node_modules` directory into the build env. this made the bug explained earlier not reproducible locally, if your local repo had dependencies already installed. reproducing the bug required the cleanup of the repo by running `git reset --hard` and `git clean -fdx`. this commit fixes this issue by adding a `.dockerignore` which mostly mirrors the directories specified `.gitignore`, including the `node_modules` directory.
app/services/organizer_service.ts
Outdated
| await organizer | ||
| .related("permissions") | ||
| .attach({ [permissionId]: { event_id: eventId } }); | ||
| .attach({ [permissionId]: { event_id: eventUuid } }); |
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.
Use consistent pivot key when attaching permissions; use eventUuid (as in addOrganizer) instead of event_id for updateOrganizerPermissions.
| .attach({ [permissionId]: { event_id: eventUuid } }); | |
| .attach({ [permissionId]: { eventUuid } }); |
6ec9ba7 to
b271cd4
Compare
|
Generated with ❤️ by ellipsis.dev |
|
uwaga na mergowanie, bo mi część "postępów" wywaliło |
|
Generated with ❤️ by ellipsis.dev |
|
Generated with ❤️ by ellipsis.dev |
|
Generated with ❤️ by ellipsis.dev |
|
Wypisuję to co nie działa Organizers Forms Events (cały kontroler do obejrzenia) Events Import/Export Emails Admins Blocks (Całe) Auth Public Participants (całe) Przy okazji testów fajnie by było sprawdzić jeszcze czy gdzieś nie zostały, np. W responsie/validatorze requestu id zamiast uuid (napewno zostały IMO jakieś zagnieżdżone). W pewnym sensie przekazuję to już wam, ale dlatego że nie do końca już łapie te błędy tutaj.
|
Important
Refactor codebase to use UUIDs for primary keys across models, controllers, services, and validators, ensuring consistency and future-proofing.
uuidinstead ofidfor primary keys across models likeAdmin,Event,Attribute,Block,Email,Form,Participant, andPermission.uuidin routes and queries, e.g.,admins_controller.ts,events_controller.ts,participants_controller.ts.uuid, e.g.,admin_service.ts,attribute_service.ts,participant_service.ts.uuidfields, e.g.,admin.ts,attribute.ts,participants.ts.uuidgeneration usingrandomUUID()inbeforeCreatehooks for models.uuidin pivot tables, e.g.,AdminsPermissions,ParticipantsAttributes.PrismaNamingStrategyfor consistent naming conventions.routes.tsto reflectuuidusage in route parameters.uuidchanges, e.g.,auth/me.spec.ts.This description was created by
for 4e7038a. You can customize this summary. It will automatically update as commits are pushed.