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.

Sign up for GitHub

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

Feat: 콜백 단건 조회 로직 개선 #123

Merged
merged 1 commit into from
Oct 31, 2024
Merged

Feat: 콜백 단건 조회 로직 개선 #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/main/java/com/example/sinitto/callback/service/CallbackService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,11 @@ public CallbackForSinittoResponse getCallbackForSinitto(Long memberId, Long call
Callback callback = callbackRepository.findById(callbackId)
.orElseThrow(() -> new NotFoundException("해당 콜백 id에 해당하는 콜백이 없습니다."));

if (callback.getAssignedMemberId().equals(memberId)) {
return new CallbackForSinittoResponse(callback.getId(), callback.getSeniorName(), callback.getPostTime(), callback.getStatus(), callback.getSeniorId(), true, callback.getSenior().getPhoneNumber());
if (!callback.getStatus().equals(Callback.Status.WAITING.toString())) {
if (callback.getAssignedMemberId() != null && callback.getAssignedMemberId().equals(memberId)) {
Comment on lines +237 to +238
Copy link
Collaborator

Choose a reason for hiding this comment

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

callback.getAssignedMemberId().equals(memberId) 이 조건을 만족한다면 callback.getAssignedMemberId()이 null일 수 없고 "callback.getAssignedMemberId() != null" 를 무조건 만족하기 때문에 이 조건은 빼도 같은 로직으로 작동하지 않을까요?

if (callback.getAssignedMemberId().equals(memberId)) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

의견 감사드립니다! 사실 수정 전에는 은선님이 제안주신 코드를 사용하였었는데 NullPointException 이 발생해서 수정하게되었습니다. 예외 발생의 이유를 설명드리자면

가정: callback 은 아직 할당이 안되어 assignedMemberId = null 인 상황
callback.getAssignedMemberId().equals(memberId) 이것만 있을경우, callback.getAssignedMemberId() 이 null이라서 null.equals(memberId) 가 되어 NullPointException 이 발생하는것으로 파악이 됩니다.

그래서 callback.getAssignedMemberId() != null 로 미리 null을 확인하여 예외 발생을 방지해 보았습니다!

callback.getAssignedMemberId() 는 null 이 있는 경우가 많기때문에
SCR-20241030-tdbh
SCR-20241030-tdoh

위 테스트 코드를 돌리면 아래와 같은 예외가 발생

Copy link
Collaborator

Choose a reason for hiding this comment

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

아하 그런 이유가 있었네요~
예외 상황을 정확히 파악하고 수정하셨네요!😊

return new CallbackForSinittoResponse(callback.getId(), callback.getSeniorName(), callback.getPostTime(), callback.getStatus(), callback.getSeniorId(), true, callback.getSenior().getPhoneNumber());
}
throw new ForbiddenException("대기중인 콜백이 아닌경우 오직 할당받은 시니또만이 콜백을 조회할 수 있습니다.");
}

return new CallbackForSinittoResponse(callback.getId(), callback.getSeniorName(), callback.getPostTime(), callback.getStatus(), callback.getSeniorId(), false, "");
Expand Down
46 changes: 38 additions & 8 deletions src/test/java/com/example/sinitto/callback/service/CallbackServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ void localDateTimeBeforeCalculateTest() {
}

@Test
@DisplayName("시니또용 콜백 단건 조회 - api 호출한 시니또 본인이 할당된 콜백일 경우")
@DisplayName("시니또용 콜백 단건 조회 - 1.대기상태 아님 2.AssignedMemberId 이 null 아님 + 해당 콜백에 할당 된 시니또 맞음 ")
void getCallbackForSinitto() {
//given
Long memberId = 1L;
Expand All @@ -415,11 +415,12 @@ void getCallbackForSinitto() {
Senior senior = mock(Senior.class);

when(callbackRepository.findById(callbackId)).thenReturn(Optional.of(callback));

when(callback.getStatus()).thenReturn(Callback.Status.IN_PROGRESS.toString());
when(callback.getAssignedMemberId()).thenReturn(1L);
when(callback.getId()).thenReturn(1L);
when(callback.getSeniorName()).thenReturn("SeniorName");
when(callback.getPostTime()).thenReturn(LocalDateTime.now());
when(callback.getStatus()).thenReturn(Callback.Status.WAITING.toString());
when(callback.getSeniorId()).thenReturn(1L);
when(callback.getSenior()).thenReturn(senior);
when(callback.getSenior().getPhoneNumber()).thenReturn("01012341234");
Expand All @@ -429,28 +430,57 @@ void getCallbackForSinitto() {

//then
assertTrue(result.isAssignedToSelf());
assertEquals("01012341234", result.seniorPhoneNumber());
}

@Test
@DisplayName("시니또용 콜백 단건 조회 - api 호출한 시니또 본인이 할당된 콜백이 아닌 경우")
@DisplayName("시니또용 콜백 단건 조회 - 1.대기상태 아님 2.AssignedMemberId 이 null 인 상황 ")
void getCallbackForSinitto2() {
//given
Long memberId = 1L;
Long callbackId = 1L;
Callback callback = mock(Callback.class);

when(callbackRepository.findById(callbackId)).thenReturn(Optional.of(callback));
when(callback.getAssignedMemberId()).thenReturn(999L); // 여기서 시니또 본인에게 할당된 콜백이 아닌걸 확인
when(callback.getId()).thenReturn(1L);
when(callback.getSeniorName()).thenReturn("SeniorName");
when(callback.getPostTime()).thenReturn(LocalDateTime.now());
when(callback.getAssignedMemberId()).thenReturn(null);
when(callback.getStatus()).thenReturn(Callback.Status.IN_PROGRESS.toString());

//when then
assertThrows(ForbiddenException.class, () -> callbackService.getCallbackForSinitto(memberId, callbackId));
}

@Test
@DisplayName("시니또용 콜백 단건 조회 - 1.대기상태 아님 2.AssignedMemberId 이 null 은 아닌데 해당 콜백에 할당된 시니또는 아닌 상황")
void getCallbackForSinitto3() {
//given
Long memberId = 1L;
Long callbackId = 1L;
Callback callback = mock(Callback.class);

when(callbackRepository.findById(callbackId)).thenReturn(Optional.of(callback));
when(callback.getAssignedMemberId()).thenReturn(999L);
when(callback.getStatus()).thenReturn(Callback.Status.IN_PROGRESS.toString());

//when then
assertThrows(ForbiddenException.class, () -> callbackService.getCallbackForSinitto(memberId, callbackId));
}

@Test
@DisplayName("시니또용 콜백 단건 조회 - 1.콜백이 '대기상태' 인 경우(모든 시니또가 조회 가능하다)")
void getCallbackForSinitto4() {
//given
Long memberId = 1L;
Long callbackId = 1L;
Callback callback = mock(Callback.class);

when(callbackRepository.findById(callbackId)).thenReturn(Optional.of(callback));
when(callback.getStatus()).thenReturn(Callback.Status.WAITING.toString());
when(callback.getSeniorId()).thenReturn(1L);

//when
CallbackForSinittoResponse result = callbackService.getCallbackForSinitto(memberId, callbackId);

//then
assertFalse(result.isAssignedToSelf());
assertEquals("", result.seniorPhoneNumber());
}
}