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

Photo pipeline #42

Merged
merged 9 commits into from
Jan 22, 2025
Merged

Photo pipeline #42

merged 9 commits into from
Jan 22, 2025

Conversation

theosiemensrhodes
Copy link
Contributor

Overview of Changes

  • Combine profile photos and classes photos into one images table, with fks to users and class tables respectively
  • Change the frontend to load photos via image urls instead of JSON, improving caching and loading
  • Compress and restrict image size on upload, restrict mime types to only image types in multer
  • Switched to using mysql2/promise for databases:
    • this allows us to use async await syntax which in turn allows us to use transactions without many many nested calls
    • additionally mysql2 is significantly faster than mysql under load
  • Added a logging package, morgan, which logs requests for development

Feel free to reach out on discord if you have any questions

Copy link
Contributor

@jansm04 jansm04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks really good, only issue I'm having is with uploading a new profile picture on the volunteer profile page. Uploaded picture doesn't show on the ui and doesn't get inserted into the db

@theosiemensrhodes
Copy link
Contributor Author

Code looks really good, only issue I'm having is with uploading a new profile picture on the volunteer profile page. Uploaded picture doesn't show on the ui and doesn't get inserted into the db

There was an issue with checking mime types, its been resolved now. Additionally noticed some issues with creating accounts so updated the volunteerModel.createVolunteer method

@jansm04
Copy link
Contributor

jansm04 commented Jan 20, 2025

Code looks really good, only issue I'm having is with uploading a new profile picture on the volunteer profile page. Uploaded picture doesn't show on the ui and doesn't get inserted into the db

There was an issue with checking mime types, its been resolved now. Additionally noticed some issues with creating accounts so updated the volunteerModel.createVolunteer method

Ya everything looks good to me now, nice work man

@jjessieshang
Copy link
Collaborator

Great work Theo

Super clean optimization to have images in one table. As a sanity check I've dropped the original class_image and volunteer_profile_pic tables in the remote database - everything works.
Also logging in the backend is really nice for debugging

Before you merge:

  • go ahead and remove class_image and volunteer_profile_pic from the database init .sql files
  • add "images" table to uml diagram
  • resolve minor merge conflicts with main branch
  • Hit "squash and merge" when merging for cleaner history

@JoshFung
Copy link
Contributor

Looks good! Really like the adjustment to use transactions.

Let me know if you want me to take another look after resolving merge conflicts or any other changes 😄

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.

4 participants