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

프로젝트 상세 조회 기능 구현 #56

Merged
merged 26 commits into from
Feb 20, 2024
Merged

Conversation

yenzip
Copy link
Contributor

@yenzip yenzip commented Feb 19, 2024

🎫 관련 이슈

Resolves #5
Resolves #47

✅ 구현 내용

  • 프로젝트 상세 조회 서비스 작성
  • 프로젝트 상세 조회 컨트롤러 작성
  • 프로젝트 Swagger 적용

💬 코멘트

  • 프로젝트 상세 조회 관련 서비스 코드가 길다보니, 이해하기 쉽게 주석을 달았습니다..! 피드백 받고 수정하면서 관련된 주석은 제거하도록 하겠습니다..!
  • 테스트 코드는 따로 PR 올리도록 하겠습니다.
  • 현재 N + 1 문제가 발생하기에 QueryDSL을 도입하려고 합니다! 따로 이슈 만들어서 PR 올리겠습니다.

@yenzip yenzip added the 🚀 더 만들어봤슈 기능 구현 label Feb 19, 2024
@yenzip yenzip added this to the 🚀 2차 스프린트 milestone Feb 19, 2024
@yenzip yenzip self-assigned this Feb 19, 2024
Copy link
Member

@Sehee-Lee-01 Sehee-Lee-01 left a comment

Choose a reason for hiding this comment

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

생각할 거리가 진짜 많았을 것 같은데 진짜 너무 고생 많으셨어요! 코드 양이 많은데 깔끔하게 작성해주셔서 보는데 코드 양이 많은 줄도 몰랐습니다...👍
몇 가지 코멘트 남겨두었는데 참고만 해주셔용~!!

@Operation(summary = "프로젝트 상세 조회")
@ApiResponse(responseCode = "200", description = "프로젝트 상세 조회 성공")
public ResponseEntity<ProjectResponse> getById(
// TODO: 로그인 아이디 추가
Copy link
Member

@Sehee-Lee-01 Sehee-Lee-01 Feb 20, 2024

Choose a reason for hiding this comment

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

혹시 조회도 로그인 아이디가 필요한가요! 아니면 나중에 isOwner도 고려해서 표시한 건가유? 확인차 여쭤봅니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

프로젝트 상세 조회에서 게시글 작성자인 경우, 게시글 수정과 삭제가 가능해야 하기에 필요하다고 생각해서 넣었는데.. 필요가 없을까요..? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

뭔가 프론트 단에서 유저 정보(엑세스 토큰, userSummary에 대한 정보)를 가지고 있긴 해서 조회할 때는 어제 회의처럼 그걸로 ownerId와 확인하는 걸로 알고 있어서욥! 저도 자세히는 모르겠어서 요 부분은 프론트엔드분들이랑 상의 해봐야 할 것 같네유!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👉 훈오님과 상의 후 로그인 관련 정보는 필요 없는 것으로 결정!

public record MemberSummary(
Long id,
String category,
UserSummaryResponse userSummary
Copy link
Member

Choose a reason for hiding this comment

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

요 UserSummaryResponse이름 그냥 UserSummaryㄹ 변경하느 것이 좋을까요? 이건 제가 제 PR 에서 반영하겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저는 UserSummary로 하면 더 통일성 있고 좋을 것 같아요! 반영해주시면 감사하겠습니다 👍

Comment on lines 63 to 69
private MemberSummary createMemberSummary(Member member) {
User user = member.getUser();
UserSummaryResponse userSummaryResponse = (user == null)
? UserSummaryResponse.from(member.getNickname())
: UserSummaryResponse.from(user);
return MemberSummary.from(member, userSummaryResponse);
}
Copy link
Member

Choose a reason for hiding this comment

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

혹시 요 메서드는 MemberSummary 안에서 메서드로 바꾸게 하는 것은 어떻게 생각하시나용!
근데 MemberSummary 안에서 두면 먼가 dto 역할이 많아지기도 하겠네요,,,
요 의견은 그냥 참고만 해주시면 감사하겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

엇 지금 보니까 MemberSummary 안에 정적 메서드 오버로딩으로 활용하는게 더 좋겠네요...! 바로 반영하겠습니다! 좋은 의견 감사합니다 ❤️‍🔥

src/main/resources/db/data/afterMigrate.sql Outdated Show resolved Hide resolved
Copy link

📝 Jacoco Test Coverage

Overall Project 53.5% -22.31%
Files changed 5.73%

File Coverage
FileType.java 100% 🍏
UserSummaryResponse.java 72.73% -27.27%
ProjectService.java 0%
ProjectResponse.java 0%
MemberSummary.java 0%
OverviewImageSummary.java 0%
ProjectSkillSummary.java 0%
Project.java 0% -29.09%
ProjectErrorCode.java 0%
ProjectController.java 0%
Member.java 0% -13.33%

@yenzip
Copy link
Contributor Author

yenzip commented Feb 20, 2024

common 패키지에서 ErrorCode 인터페이스 생성하고, Project에서 ErrorCode 생성해 적용했습니다.
QueryDSL 관련 PR은 너무 길어질 것 같아서, test와 함께 다시 PR 올리겠습니다..!
확인해주시고 merge해주시면 감사하겠습니다..! 🙇

@yenzip yenzip added 🛠 리팩토링했슈 리팩토링 🥕 더 수정해봤슈 리뷰 후 피드백 반영 labels Feb 20, 2024
@Sehee-Lee-01 Sehee-Lee-01 linked an issue Feb 20, 2024 that may be closed by this pull request
5 tasks

import sixgaezzang.sidepeek.common.exception.ErrorCode;

public enum ProjectErrorCode implements ErrorCode {
Copy link
Member

Choose a reason for hiding this comment

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

👍👍👍👍

@Sehee-Lee-01 Sehee-Lee-01 merged commit 00d28c2 into dev Feb 20, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 더 만들어봤슈 기능 구현 🛠 리팩토링했슈 리팩토링 🥕 더 수정해봤슈 리뷰 후 피드백 반영
Projects
None yet
Development

Successfully merging this pull request may close these issues.

프로젝트 상세 조회 기능 구현
2 participants