Skip to content
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

fix: status로 필터링 시 발생하는 500 오류 해결 #131

Conversation

amaran-th
Copy link
Collaborator

@amaran-th amaran-th commented Jul 28, 2023

#️⃣연관된 이슈

#130

📝작업 내용

  • 필터링한 EnumMap 객체에 대해 null 체크를 수행하여 NPE가 터지지 않게 하였습니다.

예상 소요 시간 및 실제 소요 시간

급하게 작업하느라 측정하지 않았습니다...죄송합니다

if (cannotFoundKeyStatus(filteredEvents)) {
return List.of();
}
return EventResponse.makeEventResponsesByStatus(status, filteredEvents);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이러한 null 체크해서 빈 리스트를 반환하는 로직이 생겨났으니 테스트도 추가해보는 것이 어떨까요?

@java-saeng
Copy link
Collaborator

기존에 테스트 코드를 많이 작성하셨음에도 불구하고
이런 경우도 있었으니까 다른 문제점이 없는지 테스트 코드나 요청 툴(postman, .http)을 통해서 더 천천히 확인해보시는건 어떨까요??

@amaran-th amaran-th requested a review from hong-sile July 29, 2023 02:14
@amaran-th amaran-th self-assigned this Jul 29, 2023
@amaran-th amaran-th added 버그 개발자가 의도하지 않은 상황 Backend 백엔드 관련 이슈 High Priority 리뷰 우선순위가 높은 PR labels Jul 29, 2023
@amaran-th amaran-th added this to the 3차 스프린트 1주차 milestone Jul 29, 2023
Copy link
Collaborator

@hyeonjerry hyeonjerry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다.

@@ -148,12 +147,19 @@ private List<EventResponse> filterEventResponsesByStatus(final String statusName
final EnumMap<EventStatus, List<Event>> sortAndGroupByEventStatus) {
if (isExistStatusName(statusName)) {
EventStatus status = EventStatus.from(statusName);
return EventResponse.makeEventResponsesByStatus(status,
sortAndGroupByEventStatus.get(status));
List<Event> filteredEvents = sortAndGroupByEventStatus.get(status);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sortAndGroupByEventStatus라는게 저는 메서드 명처럼 느껴지는데 아마란스는 어떻게 생각하시나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

듣고 보니 그렇게 보이네요...! eventsForEventStatus로 변경했습니다😀

- sortAndGroupByStatus을 eventsForEventStatus로 변경

#130
Copy link
Collaborator

@hong-sile hong-sile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 반영도 잘 해줬네요. 이만 Approve할게요

Copy link
Collaborator

@java-saeng java-saeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다 !!

@amaran-th amaran-th merged commit 6c4b410 into backend-main Jul 31, 2023
1 check passed
@amaran-th amaran-th deleted the fix/#130-행사_목록_조회에서_Status_필터링_시_에러_발생 branch August 4, 2023 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend 백엔드 관련 이슈 High Priority 리뷰 우선순위가 높은 PR 버그 개발자가 의도하지 않은 상황
Projects
Status: Archive
Development

Successfully merging this pull request may close these issues.

4 participants