-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/str 56 data models #30
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
Conversation
…artucalgary/Networking-Icebreakers into feature/STR-56-DataModels
| // router is like a mini-express app that allows grouping of related routes together | ||
| import { Router } from 'express'; | ||
| import { getUsers, createUser } from '../controllers/user_controller'; | ||
| import { getUsers, createUser } from '../controllers/user_controller.ts'; |
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.
doesn't matter much, but it is better to omit .ts
| import { Router } from 'express'; | ||
| import { getUsers, createUser } from '../controllers/user_controller'; | ||
| import { getUsers, createUser } from '../controllers/user_controller.ts'; | ||
| // Importing controller functions that handle logic for each route |
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 only diff in this file is verbose comments, it is recommended to keep comments minimal
| passwordChangedAt?: Date; | ||
| createdAt?: Date; | ||
| updatedAt?: Date; | ||
| password: string; |
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.
it is standard practice to store passwordHash instead of password, I think naming wise passwordHash is more descriptive
| name: string; | ||
| email: string; | ||
| passwordHash: string; | ||
| lastLogin?: Date; |
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.
not sure why the rest of the fields are removed here, they have been used and tested. For example passwordChangedAt is useful to invalidate existing JWTs/sessions after a password change
|
The reason failing of CI Build because of the change of user model |
Created Event and Participant Data model. Created routes and controllers for creating events, and getting event information. Commit History Is kind of messed up because My git crashed and bricked itself mid rebase while also committing, I cant even tell you how it happened. The only changes should be the creating event and participant models, event controller and routes. As well as adding said routes to the app.ts.