From 2e66213fb93a76006cbadfe27102f22bd11ac111 Mon Sep 17 00:00:00 2001 From: minghansun1 Date: Sun, 17 Nov 2024 13:19:12 -0500 Subject: [PATCH] fixed most of Vincent's comments --- backend/market/models.py | 3 -- backend/market/permissions.py | 2 +- backend/market/serializers.py | 63 +++++++++-------------------------- backend/market/views.py | 54 +++++++++++++----------------- 4 files changed, 41 insertions(+), 81 deletions(-) diff --git a/backend/market/models.py b/backend/market/models.py index eaed50f4..ab683e8c 100644 --- a/backend/market/models.py +++ b/backend/market/models.py @@ -67,9 +67,6 @@ def delete(self, *args, **kwargs): self.item.delete() super().delete(*args, **kwargs) - -# TODO: Verify that this S2 bucket exists. -# Check if we need to make it manually or will Django make it for us? class ItemImage(models.Model): item = models.ForeignKey(Item, on_delete=models.CASCADE, related_name="images") image = models.ImageField(upload_to="marketplace/images") diff --git a/backend/market/permissions.py b/backend/market/permissions.py index 1f303483..fbc8408c 100644 --- a/backend/market/permissions.py +++ b/backend/market/permissions.py @@ -63,6 +63,6 @@ def has_permission(self, request, view): def has_object_permission(self, request, view, obj): if request.method in permissions.SAFE_METHODS: # Check if the user owns the item when getting list - return obj.seller == request.user + return obj.item.seller == request.user # This is redundant, here for safety return obj.user == request.user diff --git a/backend/market/serializers.py b/backend/market/serializers.py index 99c4caa9..21da5b55 100644 --- a/backend/market/serializers.py +++ b/backend/market/serializers.py @@ -13,13 +13,14 @@ class TagSerializer(serializers.ModelSerializer): class Meta: model = Tag fields = "__all__" + read_only_fields = [field.name for field in model._meta.fields] class CategorySerializer(serializers.ModelSerializer): class Meta: model = Category - fields = ["name"] - read_only_fields = ["name"] + fields = "__all__" + read_only_fields = [field.name for field in model._meta.fields] class OfferSerializer(serializers.ModelSerializer): @@ -41,12 +42,12 @@ class ItemImageSerializer(serializers.ModelSerializer): class Meta: model = ItemImage - fields = ["item", "image"] + fields = "__all__" # Browse images class ItemImageURLSerializer(serializers.ModelSerializer): - image_url = serializers.SerializerMethodField("get_image_url") + image_url = serializers.SerializerMethodField() def get_image_url(self, obj): image = obj.image @@ -62,27 +63,18 @@ def get_image_url(self, obj): class Meta: model = ItemImage - fields = ["id", "image_url"] + fields = "__all__" + read_only_fields = [field.name for field in model._meta.fields] # complex item serializer for use in C/U/D + getting info about a singular tag class ItemSerializer(serializers.ModelSerializer): - # amenities = ItemSerializer(many=True, required=False) - images = ItemImageURLSerializer(many=True, required=False) - - tags = serializers.PrimaryKeyRelatedField(many=True, queryset=Tag.objects.all(), required=False) - category = serializers.PrimaryKeyRelatedField(queryset=Category.objects.all(), required=True) - seller = serializers.PrimaryKeyRelatedField(queryset=User.objects.all(), required=False) - favorites = serializers.PrimaryKeyRelatedField( - many=True, queryset=User.objects.all(), required=False - ) - class Meta: model = Item - read_only_fields = ["id", "created_at", "seller", "buyer", "images"] fields = [ "id", "seller", + "buyers", "tags", "category", "favorites", @@ -93,10 +85,8 @@ class Meta: "negotiable", "expires_at", "images", - # images are now created/deleted through a separate endpoint (see urls.py) - # this serializer isn't used for getting, - # but gets on tags will include ids/urls for images ] + read_only_fields = ["id", "created_at", "seller", "buyers", "images"] def validate_title(self, value): if self.contains_profanity(value): @@ -113,11 +103,7 @@ def contains_profanity(self, text): def create(self, validated_data): validated_data["seller"] = self.context["request"].user - category_name = validated_data.pop("category") - category_instance = Category.objects.get(name=category_name) - validated_data["category"] = category_instance instance = super().create(validated_data) - instance.save() return instance # delete_images is a list of image ids to delete @@ -128,7 +114,6 @@ def update(self, instance, validated_data): or self.context["request"].user.is_superuser ): instance = super().update(instance, validated_data) - instance.save() return instance else: raise serializers.ValidationError("You do not have permission to update this item.") @@ -144,13 +129,8 @@ def destroy(self, instance): raise serializers.ValidationError("You do not have permission to delete this item.") -# simple tag serializer for use when pulling all serializers/etc -class ItemSerializerSimple(serializers.ModelSerializer): - tags = serializers.PrimaryKeyRelatedField(many=True, queryset=Tag.objects.all(), required=False) - images = ItemImageURLSerializer(many=True, required=False) - category = serializers.PrimaryKeyRelatedField(queryset=Category.objects.all(), required=True) - seller = serializers.PrimaryKeyRelatedField(queryset=User.objects.all(), required=False) - +# Read-only serializer for use when pulling all items/etc +class ItemSerializerRead(serializers.ModelSerializer): class Meta: model = Item fields = [ @@ -164,17 +144,7 @@ class Meta: "negotiable", "images", ] - read_only_fields = [ - "id", - "seller", - "tags", - "category", - "favorites", - "title", - "price", - "negotiable", - "images", - ] + read_only_fields = fields class SubletSerializer(serializers.ModelSerializer): @@ -186,12 +156,11 @@ class Meta: read_only_fields = ["id"] def create(self, validated_data): - item_data = validated_data.pop("item") - item_serializer = ItemSerializer(data=item_data, context=self.context) + item_serializer = ItemSerializer(data=validated_data.pop("item"), context=self.context) item_serializer.is_valid(raise_exception=True) - item_instance = item_serializer.save(seller=self.context["request"].user) - sublet_instance = Sublet.objects.create(item=item_instance, **validated_data) - return sublet_instance + validated_data["item"] = item_serializer.save() + instance = super().create(validated_data) + return instance def update(self, instance, validated_data): if ( diff --git a/backend/market/views.py b/backend/market/views.py index 6bf3c84c..ce2cbd18 100644 --- a/backend/market/views.py +++ b/backend/market/views.py @@ -5,6 +5,7 @@ from rest_framework.parsers import FormParser, MultiPartParser from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +from rest_framework.views import APIView from market.models import Category, Item, ItemImage, Offer, Sublet, Tag from market.permissions import ( @@ -19,7 +20,7 @@ ItemImageSerializer, ItemImageURLSerializer, ItemSerializer, - ItemSerializerSimple, + ItemSerializerRead, OfferSerializer, SubletSerializer, TagSerializer, @@ -30,28 +31,22 @@ User = get_user_model() -class Tags(generics.ListAPIView): - serializer_class = TagSerializer - queryset = Tag.objects.all() +class Tags(APIView): - def get(self, request, *args, **kwargs): - temp = super().get(self, request, *args, **kwargs).data - response_data = [a["name"] for a in temp] + def get(self, request, format=None): + response_data = Tag.objects.values_list("name", flat=True) return Response(response_data) -class Categories(generics.ListAPIView): - serializer_class = CategorySerializer - queryset = Category.objects.all() +class Categories(APIView): - def get(self, request, *args, **kwargs): - temp = super().get(self, request, *args, **kwargs).data - response_data = [a["name"] for a in temp] + def get(self, request, format=None): + response_data = Category.objects.values_list("name", flat=True) return Response(response_data) class UserFavorites(generics.ListAPIView): - serializer_class = ItemSerializerSimple + serializer_class = ItemSerializerRead permission_classes = [IsAuthenticated] def get_queryset(self): @@ -69,7 +64,7 @@ def get_queryset(self): def apply_filters( - queryset, params, filter_mappings, user=None, is_sublet=False, tag_field="tags__name" + queryset, params, filter_mappings, user=None, is_sublet=False ): # Build dynamic filters based on filter mappings filters = {} @@ -77,30 +72,29 @@ def apply_filters( if param_value := params.get(param): filters[field] = param_value - # Handle seller/owner filtering based on user ownership - # Apply basic filters to the queryset + queryset = queryset.filter(**filters) - # Exclude specific categories if provided + # Handle seller/owner filtering based on user ownership if not is_sublet: queryset = queryset.exclude(category__name__in=["Sublet"]) if params.get("seller", "false").lower() == "true" and user: filters["seller"] = user else: filters["expires_at__gte"] = timezone.now() + for tag in params.getlist("tags"): + queryset = queryset.filter(**{"tags__name": tag}) else: queryset = queryset.filter(item__category__name__in=["Sublet"]) if params.get("seller", "false").lower() == "true" and user: filters["item__seller"] = user else: filters["item__expires_at__gte"] = timezone.now() - - queryset = queryset.filter(**filters) + for tag in params.getlist("tags"): + queryset = queryset.filter(**{"item__tags__name": tag}) # Apply tag filtering iteratively if "tags" parameter is provided - if tags := params.getlist("tags"): - for tag in tags: - queryset = queryset.filter(**{tag_field: tag}) + return queryset @@ -124,6 +118,9 @@ class Items(viewsets.ModelViewSet): serializer_class = ItemSerializer queryset = Item.objects.all() + def get_serializer_class(self): + return ItemSerializerRead if self.action == "list" or self.action == "retrieve" else ItemSerializer + def list(self, request, *args, **kwargs): """Returns a list of Items that match query parameters and user ownership.""" params = request.query_params @@ -144,7 +141,7 @@ def list(self, request, *args, **kwargs): params=params, filter_mappings=filter_mappings, user=request.user, - is_sublet=True, + is_sublet=False, ) # Serialize and return the queryset @@ -184,14 +181,13 @@ def list(self, request, *args, **kwargs): filter_mappings=filter_mappings, user=request.user, is_sublet=True, - tag_field="item__tags__name", ) # Serialize and return the queryset serializer = self.get_serializer(queryset, many=True) return Response(serializer.data) - +# This doesn't use CreateAPIView's functionality since we overrode the create method class CreateImages(generics.CreateAPIView): serializer_class = ItemImageSerializer http_method_names = ["post"] @@ -210,11 +206,9 @@ def post(self, request, *args, **kwargs): images = request.data.getlist("images") item_id = int(self.kwargs["item_id"]) self.get_queryset() # check if item exists - img_serializers = [] - for img in images: - img_serializer = self.get_serializer(data={"item": item_id, "image": img}) + img_serializers = [self.get_serializer(data={"item": item_id, "image": img}) for img in images] + for img_serializer in img_serializers: img_serializer.is_valid(raise_exception=True) - img_serializers.append(img_serializer) instances = [img_serializer.save() for img_serializer in img_serializers] data = [ItemImageURLSerializer(instance=instance).data for instance in instances] return Response(data, status=status.HTTP_201_CREATED)