feat: 세션 시간표 담기 API 작성 및 테스트 로직 추가#56
Conversation
| def _resolve_event(self, request: Request) -> Event: | ||
| event_name = request.query_params.get("event") | ||
| if not event_name: | ||
| event = Event.objects.filter_active().first() | ||
| if not event: | ||
| raise NotFound("해당 행사 정보가 없습니다.") | ||
| return event | ||
|
|
||
| event = Event.objects.filter_active().filter(Q(name_ko=event_name) | Q(name_en=event_name)).first() | ||
| if not event: | ||
| raise NotFound("해당 행사 정보가 없습니다.") | ||
| return event |
There was a problem hiding this comment.
filterset_class나 filterset_fields 사용은 어떠신가요?
|
|
||
| @extend_schema( | ||
| summary="북마크 목록 조회", | ||
| parameters=[], |
There was a problem hiding this comment.
결과적으로는 event라는 parameter를 받는 것 아닐까 싶어요!
추가로, 아마 filterset_class나 filterset_fields를 사용하시면 자동으로 문서에 정의될 것 같아요...?
| presentation_ids = list( | ||
| PresentationBookmark.objects.filter(user=request.user, event=event).values_list( | ||
| "presentation_id", flat=True | ||
| ) | ||
| ) |
There was a problem hiding this comment.
- 위의 filterset_class나 filterset_fields을 사용하고
- queryset은 PresentationBookmark.objects.all() 로 정의한 후
- get_queryset에서 super().get_queryset().filter(user=request.user)로 정의하면
단순하게 list(self.get_queryset().values_list("presentation_id", flat=True)) 로 요약 가능할 것 같아요!
|
|
||
|
|
||
| class PresentationBookmarkRequestSerializer(serializers.Serializer): | ||
| presentation_id = serializers.UUIDField() |
There was a problem hiding this comment.
serializers.PrimaryKeyRelatedField를 사용하시면, 나중에 create 메소드에서 한번 더 쿼리하실 필요 없을 것 같아요!
| presentation = Presentation.objects.filter_active().filter(id=presentation_id).first() | ||
| if not presentation: | ||
| raise NotFound("해당 세션 정보가 없습니다.") |
There was a problem hiding this comment.
위에서 언급한대로 serializers.PrimaryKeyRelatedField 한번 추천 드립니다!
추가로 walrus는 어떠신지요ㅎㅎ
| event = presentation.type.event | ||
| _, created = PresentationBookmark.objects.get_or_create( | ||
| user=request.user, | ||
| presentation=presentation, | ||
| defaults={"event": event}, | ||
| ) |
There was a problem hiding this comment.
serializer에서 create 메소드를 정의하고, save 메소드를 호출해서 생성되게 하는건 어때요?
추가로 serializer 내에서 self.context["request"].user가 가능할거에요 아마.
| def destroy(self, request: Request, pk: str = None) -> Response: | ||
| presentation = Presentation.objects.filter_active().filter(id=pk).first() | ||
| if not presentation: | ||
| raise NotFound("해당 세션 정보가 없습니다.") | ||
|
|
||
| PresentationBookmark.objects.filter(user=request.user, presentation=presentation).delete() | ||
| return Response(status=status.HTTP_204_NO_CONTENT) |
There was a problem hiding this comment.
get_queryset() + mixins.DestroyModelMixin 조합을 사용하시면 요 destroy 메소드는 필요없을수도 있을 것 같슴다.
| filterset_class = PresentationFilterSet | ||
|
|
||
|
|
||
| @extend_schema(tags=[OpenAPITag.EVENT_PRESENTATION_BOOKMARK]) |
There was a problem hiding this comment.
요게 GenericViewSet 클래스의 모든 view 메소드에 tags를 적용시켜주는지는 잘 모르겠네요 🤔
| from rest_framework import mixins, status, viewsets | ||
| from rest_framework.exceptions import NotFound | ||
| from rest_framework.permissions import IsAuthenticated | ||
| from rest_framework.request import Request | ||
| from rest_framework.response import Response |
There was a problem hiding this comment.
| from rest_framework import mixins, status, viewsets | |
| from rest_framework.exceptions import NotFound | |
| from rest_framework.permissions import IsAuthenticated | |
| from rest_framework.request import Request | |
| from rest_framework.response import Response | |
| from rest_framework import exceptions, mixins, permissions, request, response, status, viewsets |
나
| from rest_framework import mixins, status, viewsets | |
| from rest_framework.exceptions import NotFound | |
| from rest_framework.permissions import IsAuthenticated | |
| from rest_framework.request import Request | |
| from rest_framework.response import Response | |
| from rest_framework.exceptions import NotFound | |
| from rest_framework.mixins import ... | |
| from rest_framework.permissions import IsAuthenticated | |
| from rest_framework.request import Request | |
| from rest_framework.response import Response | |
| from rest_framework.status import ... | |
| from rest_framework.viewsets import GenericViewSet |
로 통일하는건 어떨까요?
| class PresentationBookmark(models.Model): | ||
| id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) | ||
| user = models.ForeignKey(User, on_delete=models.CASCADE, related_name="presentation_bookmarks") | ||
| event = models.ForeignKey(Event, on_delete=models.CASCADE, related_name="presentation_bookmarks") |
There was a problem hiding this comment.
여기에 event를 두신건 빠른 쿼리를 위해서일까요? presentation__type__event로도 쿼리는 가능할 것 같아서요!
그리고 앞서 말씀드린 filterset_class에선 아래처럼 정의하고요.
class PresentationBookmark(filters.FilterSet):
event_id = filters.UUIDFilter(field_name="presentation__type__event_id")
event_name = filters.CharFilter(field_name="presentation__type__event__name", lookup_expr="icontains")There was a problem hiding this comment.
오 감사합니다. 이렇게 바꿔볼게요!
| bookmark_router.register("bookmarks", views.PresentationBookmarkViewSet, basename="presentation-bookmark") | ||
|
|
||
| urlpatterns = [ | ||
| path("", include(bookmark_router.urls)), |
There was a problem hiding this comment.
요거 최종 경로가 어떻게 될까요?
rest-api.pycon.kr/v1/bookmarks/
라면, 혹시 presentation의 bookmark라는 것을 명시적으로 하기 위해 (+ 기존 공개용 API가 복수형을 안 쓰던 컨벤션을 감안하여)
rest-api.pycon.kr/v1/event/presentation-bookmark/
는 어떠실지 권장드립니다!
MU-Software
left a comment
There was a problem hiding this comment.
경로만 제외하면 대부분 nitpick 성입니다, 적절히 고려 부탁드려요~
구현한 배경입니다:
구현한 내용은 아래와 같습니다:
PresentationBookmark모델 추가 (event.presentationapp 에다가 추가해 놓았어요)BaseAbstractModel은 사용하지 않았습니다..!Bookmark를 추가하면 중복이므로 (user, presentation)에 대해서 unique constraint를 걸어 두었어요.작업한 API 엔드포인트들은 아래와 같습니다:
PresentationBookmarkID 목록 조회PresentationBookmark로 추가PresentationBookmark에서 제거