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

Implement Coffee Chat Image API #642

Closed
wants to merge 2 commits into from
Closed

Conversation

patriciaahuang
Copy link
Contributor

@patriciaahuang patriciaahuang commented Sep 27, 2024

Unified image API logic in PR #647

Summary

Created an API for coffee chat images, allowing setting, getting, and deleting images.

Notion/Figma Link

Coffee Chat API Design Doc

Test Plan

I manually tested each of the endpoints by replacing team event image endpoints with the corresponding coffee chat image endpoints. I set the images by submitting a team event credit, then tested if the images were set properly by retrieving the images in "Check Team Event Credits" section. I checked that the image deleted properly by opening the image as a URL, then deleting the team event, then refreshing the URL to see if it still exists in storage.

Screen.Recording.2024-09-27.at.5.17.22.PM.mov

Notes

I noticed some of the endpoints in team events API did not use the "user" parameter, so I removed it.

Breaking Changes

I renamed the "EventProofImage" type to just "ProofImage" but ensured all use cases were modified properly.

@patriciaahuang patriciaahuang requested a review from a team as a code owner September 27, 2024 21:42
@dti-github-bot
Copy link
Member

[diff-counting] Significant lines: 126.

Copy link
Collaborator

@andrew032011 andrew032011 left a comment

Choose a reason for hiding this comment

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

Thoughts on unifying the logic between coffeeChatImageAPI.ts and teamEventsImageAPI.ts? We could just create something like image-utils.ts under backend/src/utils/ and add the logic for getting the signed URL by name, uploading the image by name, and deleting all in that and just call them from coffeeChatImageAPI and teamEventsImageAPI.

This also makes it easier for testing too since both coffee chat and team event image are using the same code path, so in theory if one of them works, the other one does.

@patriciaahuang
Copy link
Contributor Author

Thoughts on unifying the logic between coffeeChatImageAPI.ts and teamEventsImageAPI.ts? We could just create something like image-utils.ts under backend/src/utils/ and add the logic for getting the signed URL by name, uploading the image by name, and deleting all in that and just call them from coffeeChatImageAPI and teamEventsImageAPI.

This also makes it easier for testing too since both coffee chat and team event image are using the same code path, so in theory if one of them works, the other one does.

I agree that we should unify the logic in coffeeChatImageAPI.ts and teamEventsImageAPI.ts. I'm thinking we could also unify the logic in ImagesAPI.ts since that is also starting to get repetitive.

@kevinmram
Copy link
Contributor

kevinmram commented Sep 29, 2024

and similarly to what Patricia mentioned in my PR (#643), a unified ImagesAPI would be used for adding images to shoutouts (instead of reusing teamEventsImageAPI) and anywhere else we made need similar functionality in the future!

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