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

refactor: 지난 약속에 참여하지 못하도록 예외처리 #474

Merged
merged 2 commits into from
Sep 6, 2024

Conversation

coli-geonwoo
Copy link
Contributor

🚩 연관 이슈

close #473


📝 작업 내용


🏞️ 스크린샷 (선택)


🗣️ 리뷰 요구사항 (선택)

  • 약속 시간을 현재시간과 비교하여 지난 약속일 경우 400을 반환합니다.

Copy link

github-actions bot commented Sep 5, 2024

Test Results

121 tests   113 ✅  4s ⏱️
 40 suites    8 💤
 40 files      0 ❌

Results for commit 62e596a.

Copy link

github-actions bot commented Sep 5, 2024

📝 Test Coverage Report

Overall Project 78.22%
Files changed 100% 🍏

File Coverage
MeetingService.java 92.81% 🍏

Copy link
Contributor

@eun-byeol eun-byeol left a comment

Choose a reason for hiding this comment

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

빠르게 구현해주셨네요~👍👍

Copy link
Contributor

@mzeong mzeong left a comment

Choose a reason for hiding this comment

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

🥦 👍

Comment on lines 91 to 101
if (isOverDueMeeting(meeting)) {
throw new OdyBadRequestException("과거 약속에 참여할 수 없습니다.");
}
return mateService.saveAndSendNotifications(mateSaveRequest, member, meeting);
}

private boolean isOverDueMeeting(Meeting meeting) {
LocalDateTime now = TimeUtil.nowWithTrim();
LocalDateTime meetingTime = meeting.getMeetingTime();
return now.isAfter(meetingTime);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

[제안] 지난 약속인지 판별을 Meeting에서 해도 좋을 것 같아요

if (meeting.isOverdue()) {
     ...
}
public boolean isOverdue() {
    LocalDateTime now = TimeUtil.nowWithTrim();
    return now.isAfter(getMeetingTime());
}

+테스트는 overdue로 작성되었는데, 여기는 overDue로 작성되었어요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

[반영 완료]

  • meeting에게 끝났는지를 묻는 isEnd() 메서드가 있어서 활용했습니다!
@Transactional
    public MateSaveResponseV2 saveMateAndSendNotifications(MateSaveRequestV2 mateSaveRequest, Member member) {
        Meeting meeting = findByInviteCode(mateSaveRequest.inviteCode());
        if (meeting.isEnd()) {
            throw new OdyBadRequestException("과거 약속에 참여할 수 없습니다.");
        }
        return mateService.saveAndSendNotifications(mateSaveRequest, member, meeting);
    }

Copy link
Contributor

@hyeon0208 hyeon0208 left a comment

Choose a reason for hiding this comment

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

깔끔하게 코드 적용해주셨네요 ~ 고생했어요 콜리 🥦
#410 PR에 Meeting에 적용된 overdue 코드 때문에
충돌 최소화하려고 필드 추가 없이 구현하신 것 같네요
해당 PR 빠르게 리뷰 반영해서 머지되도록 할께요 👍🏻 👍🏻

@coli-geonwoo coli-geonwoo merged commit 22d4168 into develop Sep 6, 2024
1 check passed
@coli-geonwoo coli-geonwoo deleted the feature/473 branch September 6, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: 시간이 지난 약속에 참여 불가하도록 변경
4 participants