Skip to content

Conversation

@joycrwu
Copy link
Contributor

@joycrwu joycrwu commented Nov 22, 2023

Overview

Modified the changing a person's profile POST endpoint to take form data instead of a JSON body for image uploads.

Changes Made

Changed the UpdatePersonController to take in a profile_pic when initialized
Changed the upload_profile_pic function to take in form data to ping the upload service
Changed the MeView class to parse form data

Test Coverage

Manually tested with Postman

Next Steps (delete if not applicable)

We probably need to change the logic on the upload profile picture function. Right now, we're making a lot of extra calls to the upload service because even if a user doesn't change their profile pic, we'll call the upload service with the same pic. Should probably check for whether the same pic is being uploaded or not before calling the upload service, but didn't make that change in this PR b/c we're pretty close to demo day

Copy link

@vinnie4k vinnie4k left a comment

Choose a reason for hiding this comment

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

Good to go.

I would also test this on a larger image (~10 MB) and see if that works locally. Once this is deployed and you still receive a 413 error, it is most like an NGINX issue in which you will need to edit client_max_body_size.

Edit: I didn't look into the backend codebase yet but if we have a "Create User Profile" call in addition to updating, make sure to use formdata there as well.

@joycrwu joycrwu requested a review from vinnie4k November 27, 2023 04:27
Copy link

@vinnie4k vinnie4k left a comment

Choose a reason for hiding this comment

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

Good to go.

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