-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feature/#86 컨퍼런스 목록 조회 api 구현 #108
The head ref may contain hidden characters: "Feature/#86-\uCEE8\uD37C\uB7F0\uC2A4_\uBAA9\uB85D_\uC870\uD68C_API_\uAD6C\uD604"
Conversation
- 월 별로 필터링할 수 있다. - 결과는 status가 진행중, 진행 예정, 종료된 행사 순으로 정렬한다. - 같은 status인 결과들은 시작일을 기준으로 오름차순 정렬한다. #86
- repository 패키지로 이동 #86
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.
확인해주세용
@@ -15,12 +34,130 @@ | |||
@RequiredArgsConstructor | |||
public class EventService { | |||
|
|||
private static final int MIN_MONTH = 1; | |||
private static final int MAX_MONTH = 12; | |||
private static final int MIN_YEAR = 2015; |
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.
MIN_YEAR를 정한 이유가 있을까요?
return criteria.isBefore(comparison) || criteria.isEqual(comparison); | ||
} | ||
|
||
private EventStatus extractEventStatus(LocalDate now, LocalDate startDate, LocalDate endDate) { |
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.
이 메서드를 Event 안으로 밀어넣어 처리할 수 있지 않을까요?
(Event의 메서드를 정의한다는 뜻입니다.)
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.
현장 리뷰 완료요~
|
||
private final EventRepository eventRepository; | ||
|
||
public List<EventResponse> findEvents(final LocalDate nowDate, final int year, final int month, |
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.
nowDate를 컨트롤러에서 줘야하는지 궁금합니다~~
저는 파라미터로 안 넘기고 service에서 해주면 되지 않을까해서요
@@ -44,3 +44,56 @@ values (3, 3, 1, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()); | |||
|
|||
insert into member_activity(id, activity_id, member_id, created_at, updated_at) | |||
values (4, 1, 2, CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()); | |||
|
|||
insert into tag(id, name) |
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.
처음 data.sql 수정한 이후로는 data.sql 을 수정하지 않는 것으로 이해했는데, 혹시 제가 잘못이해하고 있었을까요??
상대방의 테스트가 깨질 것을 염려해서 각자 테스트 안에서 만들자고 기억이 나는 것 같아서요
return EventStatus.IN_PROGRESS; | ||
} | ||
|
||
private List<EventResponse> makeEventResponses( |
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.
메서드 이름을 보면 Response json format 을 만드는 것 같은데 Response 안에 정적 팩터리 메서드에 넣는 건 어떠실까요??
뷰 관련 로직은 DTO에서 하게 되면 비즈니스 로직이 더 돋보이지 않을까해서요
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.
현장 리뷰 완료
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.
수고하셨습니다
현장 리뷰 완료 |
#️⃣연관된 이슈
📝작업 내용
컨퍼런스 목록 조회 API 구현
예상 소요 시간 및 실제 소요 시간
3일/3일
💬리뷰 요구사항(선택)
변수&메서드 이름이 적절한지 점검 부탁드립니다.
Fixture를 제대로 이해하고 사용한 것인지 확신이 없어서 테스트 코드를 주의 깊게 봐주셨으면 좋겠습니다.