Skip to content

Conversation

@ayoung-dev
Copy link
Collaborator

@ayoung-dev ayoung-dev commented Nov 29, 2024

resolved :

📌 과제 설명

커뮤니티 댓글 누락된 communityBoardId 추가

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

  • 엔티티 및 DTO에 필드 추가
  • 게시글 repository에 existsById 추가 및 검증 테스트 추가
  • 댓글 생성 serivce에 게시글 검증 로직 추가
  • 댓글 생성, 삭제 테스트 수정

✅ PR 포인트 & 궁금한 점

  • 뭔가 이상하다 했는데 조회 구현하다가 빠진 걸 알았네요... 죄송합니다.. 이거 머지하고 댓글 수정 쪽은 반영해서 pr 다시 올리겠습니다.

@ayoung-dev ayoung-dev added bug Something isn't working 아영 labels Nov 29, 2024
@ayoung-dev ayoung-dev self-assigned this Nov 29, 2024
Copy link
Collaborator

@m-a-king m-a-king 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 +16 to +18
default boolean doesNotExistById(Long id) {
return !existsById(id);
}
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.

서진님 코드에서 멋져 보여서 저도 따라해봤습니다 ㅎ

Copy link
Collaborator Author

@ayoung-dev ayoung-dev Nov 29, 2024

Choose a reason for hiding this comment

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

저는 !를 쓰지 않으려고 만든다고 생각했습니다..!

Copy link
Collaborator

Choose a reason for hiding this comment

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

넵 참고하겠습니다!

.fetchFirst() != null;
}

private JPAQuery<CommunityBoardView> getCommunityBoardsQuery() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 이번 변경사항은 아닌데 repo에 find로 맞추기로 하지 않았는지 여쭤보고 싶습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

안그래도 저도 다시 수정하면서 거슬렸는데
board의 controller 만들면서 같이 수정하려고 놔뒀습니다!
그떄 같이 수정할게욤

Copy link
Collaborator

Choose a reason for hiding this comment

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

넵 !

Comment on lines 55 to 65
@Override
public boolean existsById(Long id) {
QCommunityBoard communityBoard = QCommunityBoard.communityBoard;

return queryFactory
.selectOne()
.from(communityBoard)
.where(communityBoard.id.eq(id)
.and(communityBoard.deleted.eq(false)))
.fetchFirst() != null;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@Override
public boolean existsById(Long id) {
QCommunityBoard communityBoard = QCommunityBoard.communityBoard;
return queryFactory
.selectOne()
.from(communityBoard)
.where(communityBoard.id.eq(id)
.and(communityBoard.deleted.eq(false)))
.fetchFirst() != null;
}
@Override
public boolean existsById(Long id) {
return communityBoardJpaRepository.existsByIdAndDeletedFalse(id);
}
  1. 위 코드로 변경하는 것이 어떤지 여쭤보고 싶습니다.
  2. dsl로 존재 체크하는 fetchExists도 deprecated 된건지도 여쭤보고 싶습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

existsByIdAndDeletedFalse가 너무 긴 거 같아서 querydsl로 했었는데 이렇게 보니 jpa로 하는게 훨씬 간결할 거 같네요! 수정하겠습니다.

찾아보니 fetchCount() 메서드가 deprecated 되어서 fetchFirst() 또는 limit(1)을 사용하여 존재 여부를 확인하는 방식이 권장되고 있다고 하네요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

감사합니다. 조건이 1개가 아니라서 찝찝하긴 한데, id and deleted는 괜찮지 않을까 싶어서 제안드렸습니다.

Comment on lines +37 to +41
private void validateCommunityBoardExists(Long communityBoardId) {
if (communityBoardRepository.doesNotExistById(communityBoardId)) {
throw new BadRequestException(NOT_EXISTS_COMMUNITY_BOARD.getMessage());
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이전에는 early return 으로 해결하시지 않으셨는지, 변경하게 된 의도도 여쭤보고 싶습니다!

Copy link
Collaborator Author

@ayoung-dev ayoung-dev Nov 29, 2024

Choose a reason for hiding this comment

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

아 지난번에는 validator가 아니고 리턴 값이 있었는데
이번에는 void라 validate 부분은 이렇게 하게 되었습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

감사합니다. 이해했습니다. 저는 반환값이 없는 return을 적어두는 편이라서 여쭤보고 싶었습니다 ㅎㅎ

Comment on lines 43 to 45
private void validateParentCommentExists(Long parentCommentId) {
if (parentCommentId != null && !communityCommentRepository.existsById(parentCommentId)) {
throw new BadRequestException(NOT_EXISTS_COMMUNITY_COMMENT.getMessage());
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.

    if (requestDto.parentCommentId() != null) {
        validateParentCommentExists(communityComment.getParentCommentId());
    }

private void validateParentCommentExists(Long parentCommentId) {
    if (communityCommentRepository.doesNotExistById(parentCommentId)) {
        throw new BadRequestException(NOT_EXISTS_COMMUNITY_COMMENT.getMessage());
    }
}

이게 더 가독성 별로 인가요..
부모 아이디 값이 있을 경우 그 값을 검증하는 의도입니다

더 좋은 방법이 생각나시면 알려주세용

Copy link
Collaborator

Choose a reason for hiding this comment

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

아하 존재를 검증한다는 의도로 느껴지는 메서드 네이밍이라서 헷갈렸던 것 같아요

.communityBoardId(boardId)
.writerId(writerId)
.content("커뮤니티 댓글 테스트 내용")
.parentCommentId(null)
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.

수정했습니다!

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.

고생하셨습니다.!!

댓글에 커뮤니티 게시글 아이디 필드가 없었군요.

assertThatExceptionOfType(BadRequestException.class)
.isThrownBy(callable)
.withMessage(ExceptionMessage.NOT_EXISTS_COMMUNITY_BOARD.getMessage());
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

static import 하면 좋을 것 같아요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵! 수정했습니다

assertThatExceptionOfType(BadRequestException.class)
.isThrownBy(callable)
.withMessage(ExceptionMessage.NOT_EXISTS_COMMUNITY_COMMENT.getMessage());
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 static import 하면 좋을 것 같습니다.

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 CreateCommunityBoardUseCase createCommunityBoardUseCase;
@Autowired
private DeleteCommunityBoardUseCase deleteCommunityBoardUseCase;

Copy link
Collaborator

Choose a reason for hiding this comment

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

CommunityRepository 주입받아서도 가능할 것 같은데 맞나요..?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 repository로 하는게 나을까요?

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.

수정했습니다!
컨트롤러 구현할 때 board 쪽도 이렇게 수정하겠습니다!

@7zrv
Copy link
Collaborator

7zrv commented Nov 29, 2024

고생하셨습니다!

7zrv and others added 14 commits November 29, 2024 23:26
* feat: 관심기관 등록 기능 구현

- InterestCenter 엔티티에 누락된 기관, 봉사자 id 필드 추가
- 관심 기관 등록 유스케이스 추가및 서비스 레이어 구현
- 등록, 검증을 위한 영속성 레이어 구현
- 중복 예외 클래스 추가
- 예외 메세지 추가
- 테스트 코드 작성및 검증 완

* feat: 관심기관 취소 기능 구현

- 관심 기관 취소 유스케이스 추가및 서비스 레이어 구현
- 관심기관 취소, 검증을 위한 영속성 레이어 구현
- 예외 메세지 추가
- 테스트 코드 작성및 검증 완료

* feat: 관심기관 API 컨트롤러 구현

- 관심 기관 등록과 취소 엔드포인트 컨트롤러 구현
- ApiResponse 공통 응답 객체 메서드 추가(논의 예정)
- 테스트 코드 작성및 검증완료

* feat: 요청 유효성 검증추가

- 관심기관 등록 요청 Dto에 유효성 검증 어노테이션 추가
- 활성화를 위해 컨트롤러에 @Valid 어노테이션 추가

* refactor: 공통 응답 객체 수정및 적용

- 기존 와일드카드를 사용하던 ApiResponse의 ok 메서드를 String 타입으로 리팩토링
- 변경에 따른 컨트롤러 return문 수정

* chore: 요청 Dto swagger scheme 수정

- 봉사자 -> 기관으로 수정

* chore: 컨트롤러 엔드 포인트 수정

- 기존 RequestMapping을 이용한 전역적인 엔드포인트 적용에서 엔드포인트마다 url을 붙여주는 방식으로 수정
- 기본 이미지 주소를 저장한 환경변수 값추가
- swagger에서의 테스트를 위한 환경변수 추가
* feat(jwt): JwtAuthFilter 필터 로직 수정
 - Authorization 없다면 익명 유저
 - Authorization 있다면 Bearer 인증 작업

* feat(swagger): 토큰 인증 추가

* feat(center): 이름 기반 조회 기능 추가

* feat(security): 디버깅을 위한 기관 봉사 토큰 발급 기능

* chore: 디버깅 env 추가

* chore: 불필요한 import 제거
- existsById 메서드 QeuryDSL -> JPA
- repositoryTest 빌더 beforeEach로 추출
- serviceTest 예외 메세지 static import
- serviceTest usecase 주입 > repository로 주입
…filed' into bug/80-modify-community-comment-filed

# Conflicts:
#	src/main/java/com/somemore/community/repository/board/CommunityBoardRepositoryImpl.java
#	src/main/java/com/somemore/community/service/comment/CreateCommunityCommentService.java
#	src/test/java/com/somemore/community/repository/CommunityCommentRepositoryTest.java
#	src/test/java/com/somemore/community/service/comment/CreateCommunityCommentServiceTest.java
#	src/test/java/com/somemore/community/service/comment/DeleteCommunityCommentServiceTest.java
@sonarqubecloud
Copy link

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.

고생하셨습니다.

@ayoung-dev ayoung-dev merged commit be132e1 into main Nov 30, 2024
2 checks passed
@ayoung-dev ayoung-dev deleted the bug/80-modify-community-comment-filed branch November 30, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working 아영

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants