Skip to content
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

feat: starter code cleanup draft #37

Draft
wants to merge 5 commits into
base: dev
Choose a base branch
from
Draft

Conversation

Danie1T
Copy link
Contributor

@Danie1T Danie1T commented Dec 13, 2023

Just a draft PR for now. A bit messy but wanted a review to see if there are any glaring issues.

Tested notification APIs with these changes and the db flow seems ok.

@Danie1T
Copy link
Contributor Author

Danie1T commented Dec 14, 2023

also frontend docker has issues, just testing backend and db with this pr

@Danie1T
Copy link
Contributor Author

Danie1T commented Dec 14, 2023

Some functions in the kept starter code implementations, authService, emailService, fileStorageService, and userService seem to rely on Sequelize to use data from Sequelize tables (Ex: userService finding users by an identifier in Sequelize table). May need to adjust Prisma Schema so that the Prisma tables can hold this data and change functions to work with data in Prisma tables instead.

Would be beneficial to get a greater scope of which of these services rely on Sequelize and make adjustments so that there are none of these instances.

@williamtran10
Copy link
Contributor

Yes, it will be a good idea to refactor those services to use Prisma instead of Sequelize. At the end of the ticket, there optimally should be now references to sequelize in thte code at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants