Skip to content

Avatar upload + dedicated endpoint for serving binary images from database #62

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

Merged
merged 5 commits into from
Dec 15, 2024

Conversation

chriscarrollsmith
Copy link
Contributor

Honestly, Claude + Cursor did this in just 3 prompts. I'm going to have to study it to make sure I understand fully how it works. Basically, we have:

  • Some new client-side Javascript to handle the url -> upload toggling behavior
  • A new dedicated GET endpoint for serving the image as binary data
  • Some fairly minor adjustments to the update_profile endpoint and corresponding database model to accommodate receipt and storage of file data

My own instinct was to provide the base64-encoded image as a context variable to the Jinja2 template rather than use a dedicated image endpoint, but Claude convinced me that was a bad idea because binary encoding is more efficient, and we should use a dedicated image endpoint instead. (The nested use of GET endpoints is intriguing; I may have to explore how to take advantage of this in other contexts.)

Honestly I think we should eliminate the URL option altogether, which should let us simplify the HTML, Javascript, and Pydantic/database models a bit. And we should probably do a little more robustness testing to see what happens with different file sizes, dimensions, image types, etc. But this is a pretty great start.

(When testing with changed database models, remember to set set_up_db(drop = True) in main.py.)

@chriscarrollsmith chriscarrollsmith linked an issue Dec 3, 2024 that may be closed by this pull request
@chriscarrollsmith
Copy link
Contributor Author

Tagging @AkanshuS here both for code review and maybe to work on eliminating the URL option.

@AkanshuS
Copy link
Collaborator

AkanshuS commented Dec 5, 2024

Get rid of URLs

Edge Case Testing:

  1. Large Files (Do we need validation?)
  2. File Types (Do we need validation?)
  3. What happens if people upload images with weird dimensions or aspect ratios?

@chriscarrollsmith
Copy link
Contributor Author

chriscarrollsmith commented Dec 15, 2024

I removed the URL upload option, so that item is resolved.

One more thing that needs to be implemented here is to add an extra macro to render the profile picture thumbnail if the user has set one, and only use the the render_silhouette macro if no profile picture has been set.

@chriscarrollsmith
Copy link
Contributor Author

Gonna merge this one and create some new issues.

@chriscarrollsmith chriscarrollsmith merged commit 6e6b13b into main Dec 15, 2024
2 checks passed
@chriscarrollsmith chriscarrollsmith deleted the 12-allow-upload-of-avatar-images branch December 15, 2024 21:46
@chriscarrollsmith chriscarrollsmith self-assigned this Dec 17, 2024
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.

Allow upload of avatar images
2 participants