Skip to content

Conversation

@m-a-king
Copy link
Collaborator

@m-a-king m-a-king commented Nov 30, 2024

resolved :

📌 과제 설명

봉사자 조회 및 검증 기능을 구현했습니다.
기관, 봉사자, 봉사 모집, 봉사 신청의 상호작용으로 검증을 구현했으며, 이를 위한 여러 도메인의 조회 로직을 추가했습니다.

👩‍💻 요구 사항과 구현 내용

  • 봉사자 프로필 조회

    • 본인 프로필 조회 (상세 포함)
    • 타인 프로필 조회 (상세 미포함)
    • 타인 상세 프로필 조회
      • 기관만 조회 가능
        • 기관이 작성한 모집 글에 지원한 봉사자만 조회 가능 (검증)
  • 추가 기능

    • BadRequestException 오버로딩: ExceptionMessage 타입을 인자로 받을 수 있도록 변경

✅ PR 포인트 & 궁금한 점

  • 여러 도메인을 결합해서 사용하는 VolunteerDetailAccessValidator에 대한 가감없는 리뷰를 부탁드립니다.
  • 컨트롤러 테스트를 작성하지 못해서 아쉽습니다.
    • 컨트롤러가 Security와 많이 엮여있고, Mock을 활용하는 부분에서 테스트의 의도가 점점 흐릿해지는 것 같았는데 이에 대한 의견을 부탁드립니다.
  • 주요 요구사항에 벗어나는 로직과 도메인을 많이 수정해서 번거로우실 것 같아서 죄송합니다.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

빌드에 실패했습니다.

@github-actions github-actions bot changed the title Feature/63 봉사자 조회, 상세 조회, 검증 [BUILD FAIL] Feature/63 봉사자 조회, 상세 조회, 검증 Nov 30, 2024
@github-actions github-actions bot closed this Nov 30, 2024
Copy link
Collaborator

@leebs0521 leebs0521 left a comment

Choose a reason for hiding this comment

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

작업량 상당하네요.. 고생 진짜 많으셨어요ㅠㅠ


return new EncodedToken(accessToken);
throw new JwtException(JwtErrorType.MISSING_TOKEN);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

얼리 리턴 굳입니당

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

감사합니다 ㅎㅎ

}
}
throw new JwtException(UNKNOWN_ERROR);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

깔끔하네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네, 컨트롤러 테스트 만들면서 겸사겸사 수정해봤습니다~

}

throw new BadRequestException(UNAUTHORIZED_VOLUNTEER_DETAIL);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

조인없이 객체로 잘 풀어내신 것 같아요.
굳입니당~

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

감사합니다!

private final VolunteerRepository volunteerRepository;
private final VolunteerDetailRepository volunteerDetailRepository;
private final VolunteerDetailAccessValidatorImpl volunteerDetailAccessValidatorImpl;

Copy link
Collaborator

Choose a reason for hiding this comment

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

VolunteerDetailAccessValidatorImpl 인터페이스를 의존하도록 바꾸면 좋을 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아니 이런 실수를... 감사합니다 !

@m-a-king m-a-king reopened this Nov 30, 2024
@m-a-king m-a-king changed the title [BUILD FAIL] Feature/63 봉사자 조회, 상세 조회, 검증 Feature/63 봉사자 조회, 상세 조회, 검증 Nov 30, 2024
Copy link
Collaborator

@ayoung-dev ayoung-dev left a comment

Choose a reason for hiding this comment

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

전체적으로 중복되는 부분을 처리를 잘 하셔서 코드가 깔끔하네요! 배워갑니다

정말 고생하셨어요!!

cicd(jacoco): controller 패키지를 테스트 커버리지에서 제외

  • 나는 컨트롤리 테스트가 밉다.

너무 슬픈데요..

Comment on lines +65 to +46
private BooleanExpression isNotDeleted() {
return volunteerDetail.deleted.isFalse();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 부분 좋네요 👍🏻

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

감사합니다!

import com.somemore.global.exception.BadRequestException;
import com.somemore.recruitboard.usecase.query.RecruitBoardQueryUseCase;
import com.somemore.volunteerApply.usecase.VolunteerApplyQueryUseCase;
import com.somemore.volunteerapply.usecase.VolunteerApplyQueryUseCase;
Copy link
Collaborator

Choose a reason for hiding this comment

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

대문자 수정은 왜 하게 되신건가용?
궁금해서 여쭤봅니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아하 소나가 짚어주기도 했고, 패키지는 소문자로 이뤄져야하는데 제가 실수 했습니다

Comment on lines +67 to +72
String nickname = volunteerRepository.findNicknameById(id);

if (nickname == null || nickname.isBlank()) {
throw new BadRequestException(NOT_EXISTS_VOLUNTEER);
}

return nickname;
Copy link
Collaborator

Choose a reason for hiding this comment

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

null 처리 해주셔서 감사합니다..ㅎㅎ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

특별히 해드렸습니다 ~

.where(isNotDeleted().and(idEq(id)))
.fetchOne();
.selectFrom(recruitBoard)
.where(isNotDeleted().and(idEq(id)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

idEq가 앞으로 오는게 더 효율적일것 같은데 어떻게 생각하시나요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

맞습니다. 이 부분 범수님 코드를 자동 정렬만 한 것 같습니다.

private BooleanExpression isNotDeleted() {
return volunteer.deleted.isFalse();
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기 개행 넣어줘야할 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵! 감사합니다

private final VolunteerJpaRepository volunteerJpaRepository;
private final JPAQueryFactory queryFactory;

private static final QVolunteer volunteer = QVolunteer.volunteer;
Copy link
Collaborator

Choose a reason for hiding this comment

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

이렇게 밖으로 빼주는걸 왜 생각을 못햇을까요 저도 해보겠습니다

}
}

// TODO 상태 업데이트 메서드들을 만들고 빌더에서 status를 변경 불가하도록 No newline at end of file
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 개행 경고가 보입니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이것도 수정해두겠습니다

@m-a-king m-a-king force-pushed the feature/63-add-volunteer-find-manage branch from a00e403 to bef0ddb Compare December 1, 2024 08:43
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 1, 2024

@m-a-king m-a-king merged commit 28badd4 into main Dec 1, 2024
2 checks passed
@m-a-king m-a-king deleted the feature/63-add-volunteer-find-manage branch December 1, 2024 08:48
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.

5 participants