-
Notifications
You must be signed in to change notification settings - Fork 0
Lau/listing view #58
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
base: master
Are you sure you want to change the base?
Lau/listing view #58
Changes from 4 commits
fd9c0e7
5a10647
28cb881
34ae14f
9226799
5bbc124
8a00698
b553214
da542e0
35175a8
ca8ac1c
075c60b
bd8f917
79cafc1
35ed950
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 |
|---|---|---|
|
|
@@ -11,8 +11,10 @@ | |
| OffersReceived, | ||
| Tags, | ||
| UserFavorites, | ||
| accept_offer, | ||
| get_current_user, | ||
| get_phone_status, | ||
| reject_offer, | ||
| send_verification_code, | ||
| verify_phone_code, | ||
| ) | ||
|
|
@@ -46,9 +48,12 @@ | |
| # post: create an offer for an listing | ||
| # delete: delete an offer for an listing | ||
| path( | ||
| "listings/<listing_id>/offers/", | ||
| "listings/<int:listing_id>/offers/", | ||
| Offers.as_view({"get": "list", "post": "create", "delete": "destroy"}), | ||
| ), | ||
| # Offer accept / reject | ||
| path("offers/<int:offer_id>/accept/", accept_offer, name="offer-accept"), | ||
| path("offers/<int:offer_id>/reject/", reject_offer, name="offer-reject"), | ||
|
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. addressed offline, but this is not consistent w REST API convention. Instead of having verbs like "accept" and "reject", you can just directly edit the offer object (i.e. PATCH /offers), and then just modify the offer's status |
||
| # Image Creation | ||
| path("listings/<listing_id>/images/", CreateImages.as_view()), | ||
| # Image Deletion | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -342,6 +342,28 @@ def list(self, request, *args, **kwargs): | |
| return super().list(request, *args, **kwargs) | ||
|
|
||
|
|
||
| @api_view(["POST"]) | ||
| @permission_classes([IsAuthenticated]) | ||
| def accept_offer(request, offer_id): | ||
| offer = get_object_or_404(Offer, pk=offer_id) | ||
| if offer.listing.seller != request.user: | ||
| raise exceptions.PermissionDenied("Only the listing owner can accept offers.") | ||
| offer.status = Offer.Status.ACCEPTED | ||
| offer.save(update_fields=["status"]) | ||
| return Response(OfferSerializer(offer).data) | ||
|
|
||
|
|
||
| @api_view(["POST"]) | ||
| @permission_classes([IsAuthenticated]) | ||
| def reject_offer(request, offer_id): | ||
| offer = get_object_or_404(Offer, pk=offer_id) | ||
| if offer.listing.seller != request.user: | ||
| raise exceptions.PermissionDenied("Only the listing owner can reject offers.") | ||
| offer.status = Offer.Status.REJECTED | ||
| offer.save(update_fields=["status"]) | ||
| return Response(OfferSerializer(offer).data) | ||
|
|
||
|
Comment on lines
362
to
+365
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. 3 things:
Remember, once you make model changes you need to make migration files, then commit that to version control This technically worksd but it breaks the project's pattern. Every other view delegates authorization to a permission class in |
||
|
|
||
| @api_view(["POST"]) | ||
| @permission_classes([IsAuthenticated]) | ||
| def send_verification_code(request): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,20 @@ | ||
| import { ListingDetail } from "@/components/listings/detail/ListingDetail"; | ||
| import { getListingOrNotFound } from "@/lib/actions"; | ||
| import { getCurrentUser, getListingOrNotFound, getOffersReceived } from "@/lib/actions"; | ||
|
|
||
| export default async function ItemPage({ params }: { params: Promise<{ id: string }> }) { | ||
| const { id } = await params; | ||
| const item = await getListingOrNotFound(id); | ||
| const [item, currentUser] = await Promise.all([getListingOrNotFound(id), getCurrentUser()]); | ||
| const isOwner = currentUser?.id === item.seller.id; | ||
| const offersResponse = isOwner ? await getOffersReceived() : null; | ||
| const offers = offersResponse?.results?.filter((offer) => offer.listing === item.id) ?? []; | ||
|
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. addressed offline, but see if you can fetch offers just for this item, not all offers and filter by id |
||
|
|
||
| return <ListingDetail listing={item} initialIsFavorited={item.is_favorited ?? false} />; | ||
| return ( | ||
| <ListingDetail | ||
| listing={item} | ||
| initialIsFavorited={item.is_favorited ?? false} | ||
| offers={offers} | ||
| offersMode={isOwner ? "received" : "made"} | ||
| canEdit={isOwner} | ||
| /> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,25 @@ | ||
| import { ListingDetail } from "@/components/listings/detail/ListingDetail"; | ||
| import { getListingOrNotFound } from "@/lib/actions"; | ||
| import { | ||
| getCurrentUser, | ||
| getListingOrNotFound, | ||
| getOffersMade, | ||
| getOffersReceived, | ||
| } from "@/lib/actions"; | ||
|
|
||
| export default async function SubletPage({ params }: { params: Promise<{ id: string }> }) { | ||
| const { id } = await params; | ||
| const sublet = await getListingOrNotFound(id); | ||
| const [sublet, currentUser] = await Promise.all([getListingOrNotFound(id), getCurrentUser()]); | ||
| const isOwner = currentUser?.id === sublet.seller.id; | ||
| const offersResponse = await (isOwner ? getOffersReceived() : getOffersMade()); | ||
| const offers = offersResponse?.results?.filter((offer) => offer.listing === sublet.id) ?? []; | ||
|
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 comment here |
||
|
|
||
| return <ListingDetail listing={sublet} initialIsFavorited={sublet.is_favorited ?? false} />; | ||
| return ( | ||
| <ListingDetail | ||
| listing={sublet} | ||
| initialIsFavorited={sublet.is_favorited ?? false} | ||
| offers={offers} | ||
| offersMode={isOwner ? "received" : "made"} | ||
| canEdit={isOwner} | ||
| /> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ interface Props { | |
| listingPrice: number; | ||
| listingOwnerLabel: string; | ||
| priceLabel?: string; | ||
| canEdit?: boolean; | ||
|
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.
|
||
| } | ||
|
|
||
| type ModalState = "none" | "phone-input" | "verification" | "offer"; | ||
|
|
@@ -23,6 +24,7 @@ export const ListingActions = ({ | |
| listingPrice, | ||
| priceLabel, | ||
| listingOwnerLabel, | ||
| canEdit = false, | ||
| }: Props) => { | ||
| const [modalState, setModalState] = useState<ModalState>("none"); | ||
| const [pendingPhoneNumber, setPendingPhoneNumber] = useState<string>(""); | ||
|
|
@@ -35,6 +37,10 @@ export const ListingActions = ({ | |
| queryFn: getPhoneStatus, | ||
| }); | ||
|
|
||
| if (canEdit) { | ||
| return null; | ||
| } | ||
|
|
||
| const handleMakeOfferClick = () => { | ||
| if (!phoneStatus) return; | ||
|
|
||
|
|
||
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.
Once you add the status field, you should also add it here. Serializers are responsible for what data goes in and out of our API, and specifying in the fields section makes sure that when the serializer converts the python obj into a json respojnse, the field is included in the response. You should also additionally define status in
read_only_fieldsbecause this serializer gets used to actually create offers by buyers. We don't want them to be able to modify status field themselves. Because of this, we would likely need a new serializer likeOfferStatusSerializerthat onlyt exposes id and status with id as read only. So the only thing you can write thru this serializer is status, and this is how we can follow the principle of least privilge, where the update endpoint cna only do exactly what it is meant to do. You might also want to perform some field level validation in the new serializer to make sure that status is a valid status.