Favorites functionality#47
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
| favorites = request.user.listings_favorited | ||
| if favorites.filter(id=listing_id).exists(): | ||
| return Response( | ||
| {"favorited": True, "detail": "Favorite already exists"}, |
There was a problem hiding this comment.
Nit: ifl "Favorite already exists" is not really a good msg. Maybe something like "User has already liked the listing" or "Listing already liked"?
| if listing not in request.user.listings_favorited.all(): | ||
| raise exceptions.NotFound("Favorite does not exist.") | ||
| return Response( | ||
| {"favorited": False, "detail": "Favorite does not exist"}, |
There was a problem hiding this comment.
Same here. Something like "User hasn't liked the listing yet"?
As a side not, i don't know how you feel, but i'm personally not a huge fan of how we use the word "favorites". I think something like "likes" is much better. What do you think?
| import { UserCard } from "@/components/listings/detail/UserCard"; | ||
| import { BackButton } from "@/components/listings/detail/BackButton"; | ||
| import { addToUsersFavorites, deleteFromUsersFavorites, getUsersFavorites } from "@/lib/actions"; | ||
| import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; |
There was a problem hiding this comment.
This is really really small nit lol but in a large react application, its generally preferred to import third party libraries first before any absolute imports. So this should ideally go before or after lucide-react import
| } | ||
|
|
||
| export async function getUsersFavorites() { | ||
| return await serverFetch<PaginatedResponse<Item | Sublet>>("/market/favorites/"); |
There was a problem hiding this comment.
Im just noticing this, but why are you using PaginatedResponse? This is usually for a very large list of data, and you want to load them in chunks (like we do it for list of items or sublets in the main grid view).
Also, this is my b for also just bringing this up, but ideally, we should not fetch the entire list of user's favorites just to see if one current item/sublet is favorited (this is inefficient). Don't remove this function (since we'll prob use it later), but ideally, we should be able to do this on a single listing object. One way to do this is computing and returning is_favorited as one of the fields when you fetch an item or sublet. That way, you only need one backend call when you enter the details page (as opposed to two calls we had for fetching both item/sublet AND also favorites data). This will involve backend changes, so lmk if you want to take a stab at it
If you decide to go with this approach, your useQuery logic in ListingDetail.tsx will become much simpler too: since the server already includes is_favorited when we fetch the listing, we don’t actually need a separate request just to get that boolean value again. Instead, we can just simply treat Tanstack Query as a place to store that value on the client side
What that means in practical terms:
- The listing query fetches the listing (including
is_favorited) - We use that server-provided value as the initial state
- We set
staleTime:Infinityso Tanstack Query doesn’t try to refetch it automatically - The
queryFncan just return a resolved promise (e.g.Promise.resolve(initialIsFavorited)) because we’re not actually fetching anything new (i believe we still need to define this peruseQueryAPI contract)
So at this point, the query isn’t being used to “fetch” the data anymore - it’s just holding the current value in the cache. Then when favorite/unfavorite mutation happens:
onMutateimmediately updates the cached value thru optimistic updateonErrorrestores the previous value if something fails (likely not cause we are no longer making any actual network request but still safe to have it)
So the flow becomes:
- Server sends the correct initial value
- Tanstack Query stores it
- Mutations update it optimistically
- No extra background refetch needed
If you have questions, we can talk about it tmrw during gbm
Co-authored-by: Cursor <cursoragent@cursor.com>
jamesdoh0109
left a comment
There was a problem hiding this comment.
Approved w comments - right now threre are 2 same blocks of code (get_is_favorited), so see if you can combine them into one. If not, all g
| staleTime: Infinity, | ||
| }); | ||
|
|
||
| const isInsideFavorites = favoritesQuery.data ?? false; |
There was a problem hiding this comment.
Maybe isLiked or isFavorited? isInsideFavorites sounds p confusing
| if favorites.filter(id=listing_id).exists(): | ||
| return Response( | ||
| {"liked": True, "detail": "User has already liked the listing"}, | ||
| status=status.HTTP_200_OK, |
There was a problem hiding this comment.
Try to see if 200 makes sense here and if not, maybe use a better status code
Users can now mark items as favorites
