-
Notifications
You must be signed in to change notification settings - Fork 0
Favorites functionality #47
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
Changes from 7 commits
2a2ed16
18f80b4
dbe6d5b
165bee5
c9ba984
5cadf68
477f429
bb7c7f7
fb8029c
45dfcd7
febbd34
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,7 +55,7 @@ class UserFavorites(ListAPIView, DefaultOrderMixin): | |
|
|
||
| def get_queryset(self): | ||
| user = self.request.user | ||
| return user.listings_favorited | ||
| return user.listings_favorited.all() | ||
|
|
||
|
|
||
| # TODO: Can add feature to filter for active offers only | ||
|
|
@@ -255,26 +255,32 @@ class Favorites( | |
|
|
||
| def get_queryset(self): | ||
| user = self.request.user | ||
| return user.listings_favorited | ||
| return user.listings_favorited.all() | ||
|
|
||
| def create(self, request, *args, **kwargs): | ||
| listing_id = int(self.kwargs["listing_id"]) | ||
| queryset = self.get_queryset() | ||
| if queryset.filter(id=listing_id).exists(): | ||
| raise exceptions.ValidationError("Favorite already exists") | ||
| favorites = request.user.listings_favorited | ||
| if favorites.filter(id=listing_id).exists(): | ||
| return Response( | ||
| {"favorited": True, "detail": "Favorite already exists"}, | ||
| status=status.HTTP_200_OK, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Try to see if 200 makes sense here and if not, maybe use a better status code |
||
| ) | ||
| listing = get_object_or_404(Listing, id=listing_id) | ||
| self.get_queryset().add(listing) | ||
| return Response(status=status.HTTP_201_CREATED) | ||
| favorites.add(listing) | ||
| return Response({"favorited": True}, status=status.HTTP_201_CREATED) | ||
|
|
||
| def destroy(self, request, *args, **kwargs): | ||
| listing_id = int(self.kwargs["listing_id"]) | ||
| listing = get_object_or_404(Listing, id=listing_id) | ||
|
|
||
| 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"}, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
| status=status.HTTP_404_NOT_FOUND, | ||
| ) | ||
|
|
||
| self.get_queryset().remove(listing) | ||
| return Response(status=status.HTTP_204_NO_CONTENT) | ||
| request.user.listings_favorited.remove(listing) | ||
| return Response({"favorited": False}, status=status.HTTP_200_OK) | ||
|
|
||
|
|
||
| class Offers(viewsets.ModelViewSet): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| import { ListingDetail } from "@/components/listings/detail/ListingDetail"; | ||
| import { getListing } from "@/lib/actions"; | ||
| import { getListing, getUsersFavorites } from "@/lib/actions"; | ||
|
|
||
| export default async function ItemPage({ params }: { params: Promise<{ id: string }> }) { | ||
| const { id } = await params; | ||
| const item = await getListing(id); | ||
| const favorites = await getUsersFavorites().catch(() => null); | ||
|
|
||
| return <ListingDetail listing={item} />; | ||
| return <ListingDetail listing={item} initialFavorites={favorites} />; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| import { ListingDetail } from "@/components/listings/detail/ListingDetail"; | ||
| import { getListing } from "@/lib/actions"; | ||
| import { getListing, getUsersFavorites } from "@/lib/actions"; | ||
|
|
||
| export default async function SubletPage({ params }: { params: Promise<{ id: string }> }) { | ||
| const { id } = await params; | ||
| const sublet = await getListing(id); | ||
| const favorites = await getUsersFavorites().catch(() => null); | ||
|
|
||
| return <ListingDetail listing={sublet} />; | ||
| return <ListingDetail listing={sublet} initialFavorites={favorites} />; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,27 +1,100 @@ | ||
| "use client"; | ||
|
|
||
| import { useMemo } from "react"; | ||
| import { Heart, Share } from "lucide-react"; | ||
| import { Item, Sublet } from "@/lib/types"; | ||
| import { Item, PaginatedResponse, Sublet } from "@/lib/types"; | ||
| import { ListingActions } from "@/components/listings/detail/ListingActions"; | ||
| import { ListingImageGallery } from "@/components/listings/detail/ListingImageGallery"; | ||
| import { ListingInfo } from "@/components/listings/detail/ListingInfo"; | ||
| 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"; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
|
||
| interface Props { | ||
| listing: Item | Sublet; | ||
| initialFavorites: PaginatedResponse<Item | Sublet> | null; | ||
| } | ||
|
|
||
| export const ListingDetail = ({ listing }: Props) => { | ||
| export const ListingDetail = ({ listing, initialFavorites }: Props) => { | ||
| const listingType = listing.listing_type; | ||
| const priceLabel = listingType === "sublet" ? "/mo" : undefined; | ||
| const listingOwnerLabel = listingType === "item" ? "Seller" : "Owner"; | ||
| const queryClient = useQueryClient(); | ||
| const favoritesQuery = useQuery({ | ||
| queryKey: ["favorites"], | ||
| queryFn: getUsersFavorites, | ||
| initialData: initialFavorites ?? undefined, | ||
| enabled: initialFavorites !== null, | ||
| }); | ||
|
|
||
| const isInsideFavorites = useMemo( | ||
| () => !!favoritesQuery.data?.results?.some((favorite) => favorite.id === listing.id), | ||
| [favoritesQuery.data, listing.id] | ||
| ); | ||
|
|
||
| const toggleFavoriteMutation = useMutation({ | ||
| mutationFn: async (shouldFavorite: boolean) => { | ||
| if (shouldFavorite) { | ||
| await addToUsersFavorites(listing.id); | ||
| } else { | ||
| await deleteFromUsersFavorites(listing.id); | ||
| } | ||
| }, | ||
| onMutate: async (shouldFavorite: boolean) => { | ||
| await queryClient.cancelQueries({ queryKey: ["favorites"] }); | ||
| const previous = queryClient.getQueryData<PaginatedResponse<Item | Sublet>>(["favorites"]); | ||
|
|
||
| if (previous) { | ||
| const exists = previous.results?.some((favorite) => favorite.id === listing.id); | ||
| let results = previous.results ?? []; | ||
|
|
||
| if (shouldFavorite && !exists) { | ||
| results = [...results, listing]; | ||
| } | ||
| if (!shouldFavorite && exists) { | ||
| results = results.filter((favorite) => favorite.id !== listing.id); | ||
| } | ||
|
|
||
| queryClient.setQueryData<PaginatedResponse<Item | Sublet>>(["favorites"], { | ||
| ...previous, | ||
| results, | ||
| }); | ||
| } | ||
|
|
||
| return { previous }; | ||
| }, | ||
| onError: (_error, _shouldFavorite, context) => { | ||
| if (context?.previous) { | ||
| queryClient.setQueryData(["favorites"], context.previous); | ||
| } | ||
| }, | ||
| onSettled: () => { | ||
| queryClient.invalidateQueries({ queryKey: ["favorites"] }); | ||
| }, | ||
| }); | ||
|
|
||
| const handleToggleFavorite = async () => { | ||
| toggleFavoriteMutation.mutate(!isInsideFavorites); | ||
| }; | ||
|
|
||
| return ( | ||
| <div className="mx-auto flex w-full max-w-[96rem] flex-col p-8 px-4 sm:px-12"> | ||
| <div className="mb-4 flex items-center justify-between"> | ||
| <BackButton /> | ||
| <div className="flex items-center gap-3"> | ||
| <Share className="h-5 w-5" /> | ||
| <Heart className="h-5 w-5" /> | ||
| <button | ||
| type="button" | ||
| className="cursor-pointer" | ||
| onClick={handleToggleFavorite} | ||
| aria-pressed={isInsideFavorites} | ||
| aria-label={isInsideFavorites ? "Remove from favorites" : "Add to favorites"} | ||
| > | ||
| <Heart | ||
| className={isInsideFavorites ? "h-5 w-5 fill-red-500 text-red-500" : "h-5 w-5"} | ||
| /> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| <div className="grid grid-cols-1 gap-8 lg:grid-cols-[minmax(0,1fr)_minmax(0,1fr)]"> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -210,6 +210,25 @@ export async function verifyPhoneCode(phoneNumber: string, code: string) { | |
| body: JSON.stringify({ phone_number: phoneNumber, code }), | ||
| }); | ||
| } | ||
| // ------------------------------------------------------------ | ||
| // adding and removing listings from favorites | ||
| // ------------------------------------------------------------ | ||
|
|
||
| export async function addToUsersFavorites(listingId: number) { | ||
| const res = await serverFetch<void>(`/market/listings/${listingId}/favorites/`, { | ||
| method: "POST", | ||
| }); | ||
| return res; | ||
| } | ||
| export async function deleteFromUsersFavorites(listingId: number) { | ||
| return await serverFetch<void>(`/market/listings/${listingId}/favorites/`, { | ||
| method: "DELETE", | ||
| }); | ||
| } | ||
|
|
||
| export async function getUsersFavorites() { | ||
| return await serverFetch<PaginatedResponse<Item | Sublet>>("/market/favorites/"); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Im just noticing this, but why are you using 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 If you decide to go with this approach, your What that means in practical terms:
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:
So the flow becomes:
If you have questions, we can talk about it tmrw during gbm |
||
| } | ||
|
|
||
| // ------------------------------------------------------------ | ||
| // creating new listings | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: ifl "Favorite already exists" is not really a good msg. Maybe something like "User has already liked the listing" or "Listing already liked"?