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

Jump to bottom

[feat #16] : 질문 게시글 등록, 상세조회 #17

Merged
merged 37 commits into from
Aug 4, 2024
Merged

[feat #16] : 질문 게시글 등록, 상세조회 #17

merged 37 commits into from
Aug 4, 2024
+501 −57

Conversation

hyun2371
Copy link
Member

@hyun2371 hyun2371 commented Aug 3, 2024 •
edited
Loading

관련 이슈

📑 작업 상세 내용

  • 질문 게시글 관련 API 시 필요한 DTO 추가
  • 테스트 코드 작성

💫 작업 요약

  • 질문 게시글 등록 API
  • 질문 게시글 상세 조회 API
  • API 단위 테스트, 통합 테스트

🔍 중점적으로 리뷰 할 부분

  • 양방향 연관관계 주입 괜찮은지
  • 통합 테스트는 403 에러로 작동하지 않아서 disabled 처리해놓았습니다!
  • 멤버에 프로필 이미지 타입 Enum 추가하시면 응답 dto에도 반영하겠습니다.

hyun2371 added 25 commits August 1, 2024 19:48
@hyun2371
[chore] : redis 의존성 추가
e8135d1
@hyun2371
[chore] : redis 설정 파일 추가
3f8d660
@hyun2371
[feat] : 요구사항 변경에 따른 엔티티 수정
278c16b
@hyun2371
[feat] : QuestionPost Repository 추가
15c0d2f
@hyun2371
[feat] : memberRepository 추가
2bb2463
@hyun2371
[feat] : 정적 팩토리 메서드 추가 및 양방향 편의 메서드 추가
7fad0e2
@hyun2371
[feat] : 질문글 등록 관련 DTO 추가
8050daf
@hyun2371
[feat] : DTO <-> 엔티티 변환QuestionPost Mapper 추가
d1895ff
@hyun2371
[feat] : questionPost 등록 비즈니스 로직 추가
f39a1d6
@hyun2371
[feat] : questionPost 등록 컨트롤러 코드 추가
a1ab1ef
@hyun2371
[style] : dto 변수명 수정
e66fa5f
@hyun2371
[fix] : 누락된 저장 코드 추가
aaef6a9
@hyun2371
[test] : test용 fixture 추가
7c048e0
@hyun2371
[test] : 질문글 등록 단위 테스트 추가
8a4bf9b
@hyun2371
[test] : 제약 조건에 따른 memberFixture 수정
53bd098
@hyun2371
[test] : 질문글 등록 통합 테스트 작성
a65380e
@hyun2371
[feat] : 아이디로 질문글 조회 비즈니스 로직 작성
7a8617b
@hyun2371
[feat] : 아이디로 질문 조회 controller 코드 작성
043dd4d
@hyun2371
[feat] : 도메인 내 테스트용 생성자 추가
bca93f1
@hyun2371
[test] : 테스트 픽스처에 아이디 필드 추가
a76a831
@hyun2371
[test] : 질문글 아이디로 조회 단위 테스트 작성
1a171eb
@hyun2371
[test] : 질문글 아이디로 조회 통합 테스트 수행
454898e
@hyun2371
[style] : 코드 리포멧팅
1c74c64
@hyun2371
Merge branch 'dev' into feat/#16/question-register-get-api
d16c138
@hyun2371
[fix] : 병합 시 충돌 사항 해결
c3e32fe
@hyun2371 hyun2371 added the ✨ feat 기능 추가 label Aug 3, 2024
@hyun2371 hyun2371 requested a review from dudxo August 3, 2024 13:14
@hyun2371 hyun2371 self-assigned this Aug 3, 2024
@hyun2371 hyun2371 linked an issue Aug 3, 2024 that may be closed by this pull request
✨ 질문 게시글 등록, 상세조회 API #16
Closed
4 tasks
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 3, 2024 •
edited
Loading

Test Results

18 tests    4 ✅  1s ⏱️
 7 suites  14 💤
 7 files     0 ❌

Results for commit 77ecdd2.

♻️ This comment has been updated with latest results.

dudxo
dudxo reviewed Aug 3, 2024
View reviewed changes
Copy link
Collaborator

@dudxo dudxo left a comment

Choose a reason for hiding this comment

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

양방향 관계성 주입에 대해서는 지난 논의했던 내용대로 일단 이대로 진행하는게 좋을것 같습니다!

멤머 프로필 필드는 추후 추가하겠습니다~

@hyun2371 hyun2371 changed the title [feat] : 질문 게시글 등록, 상세조회 [feat #16] : 질문 게시글 등록, 상세조회 Aug 4, 2024
@hyun2371 hyun2371 requested a review from dudxo August 4, 2024 02:26
hyun2371
hyun2371 commented Aug 4, 2024
View reviewed changes
src/main/java/com/dnd/gongmuin/question_post/dto/QuestionPostDetailResponse.java Outdated
Copy link
Member Author

Choose a reason for hiding this comment

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

저희가 변환 로직 때문에 Response DTO에서 정적 팩토리 메서드를 쓰기로 했었잖아요. 생각해보니까 변환 로직을 Mapper에 다 넣으면 팩토리 메서드가 필요없지 않나 생각이듭니다. 그리고 단일책임원칙에 따라서 DTO는 변환이 아닌 값을 옮기는 역할만 해도 충분한 것 같은데 어떠신가요..?

Copy link
Member Author

Choose a reason for hiding this comment

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

처음에 말씀해주신 것처럼 DTO에서는 new로 생성해도 괜찮겠다고 생각되네요

Copy link
Collaborator

Choose a reason for hiding this comment

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

음 위 두 내용이 혼선이 되는데 값을 옮기는 역할만 하기를 말씀하시는 건지 아니면 현재 코드처럼 new로 생성하기를 말씀하시는 건지 헷갈립니다ㅜㅜ

Copy link
Collaborator

Choose a reason for hiding this comment

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

MemberInfo가 RequestDto인가요? ReqeustDto라면 MemberInfo를 생성하는 팩토리 메서드는 필요 없다고 생각됩니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

추가적으로 Request DTO 경우 DTO 내부에서 toEntity()를 사용해 Entity를 생성하는 방식 어떠신가요? Entity 내부 정적 팩토리 메서드를 이용하게 되면 DTO <-> Entity 간의 결합도가 높아지는데, DTO 내부 toEntity()를 사용하면 결합도가 낮아지고 Entity의 생성 로직이 변경되어도 DTO에서는 큰 수정사항이 필요 없어져서요!

Copy link
Member Author

@hyun2371 hyun2371 Aug 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

모든 dto에 팩토리 메서드를 없애고, new로 생성하는걸 말했습니다! mapper 클래스 내에서 변환 로직을 수행하면 팩토리 메서드 존재 이유가 없을 것 같아서요

Copy link
Collaborator

Choose a reason for hiding this comment

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

음 지금 Mapper 클래스가 따로 안보이는데 미리 말씀하시는건가요?
그리고 저는 rqeustEntity 경우 new 보다 toEntity() 방식이 더 좋아보여요!

Copy link
Member Author

Choose a reason for hiding this comment

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

저희 Mapper 클래스를 생성해서 DTO<-> Entity간 변환 로직을 처리하고 컨벤션으로 정했었습니다! 그래서 말씀하신 toEntity() 같은 함수를 mapper 클래스에 생성하면 좋을 것 같아요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

앗 죄송합니다,,ㅜㅜ 컨벤션을 제대로 숙지하지 못했었네요..
그럼 Mapper 클래스가 있으니 말씀대로 모든 dto에 팩토리 메서드를 없애는게 좋겠습니다 :)
다만 dto 생성 시 Builder로 생성하기로도 정해서, new로 생성하지 않고 Builder로 생성하면 될 것 같아요!

Copy link
Member Author

@hyun2371 hyun2371 Aug 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Request/Response 경우 new를 이용해 생성한다.
어 저희 new를 이용해 사용하는걸로 정했습니다!
DTO는 들어오는 매개변수가 항상 똑같아서 @builder가 불필요할 것 같다고 얘기 나눴었어요

dudxo
dudxo approved these changes Aug 4, 2024
View reviewed changes
Copy link
Collaborator

@dudxo dudxo left a comment

Choose a reason for hiding this comment

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

LGTM

@hyun2371 hyun2371 force-pushed the feat/#16/question-register-get-api branch from 492a88d to 5bd1013 Compare August 4, 2024 06:17
@hyun2371 hyun2371 force-pushed the feat/#16/question-register-get-api branch 2 times, most recently from 2f0ef04 to bec784f Compare August 4, 2024 07:33
@hyun2371 hyun2371 merged commit 5b4229b into dev Aug 4, 2024
3 checks passed
@hyun2371 hyun2371 deleted the feat/#16/question-register-get-api branch August 4, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dudxo dudxo

Assignees

@hyun2371 hyun2371

Labels
✨ feat 기능 추가
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

✨ 질문 게시글 등록, 상세조회 API
2 participants
@hyun2371 @dudxo