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

내가 제출한 솔루션 리스트 조회 기능 구현 (issue #288) #293

Merged
merged 5 commits into from
Aug 14, 2024

Conversation

lilychoibb
Copy link
Member

구현 요약

대시보드 페이지 - 내가 제출한 솔루션 리스트 조회 기능 구현

연관 이슈

참고

코드 리뷰에 RCA 룰을 적용할 시 참고해주세요.

헤더 설명
R (Request Changes) 적극적으로 반영을 고려해주세요
C (Comment) 웬만하면 반영해주세요
A (Approve) 반영해도 좋고, 넘어가도 좋습니다. 사소한 의견입니다.

@lilychoibb lilychoibb added 🚛 백엔드 백엔드 관련 이슈 ⚒️ 기능 작업해야하는 기능 labels Aug 13, 2024
@lilychoibb lilychoibb self-assigned this Aug 13, 2024
Copy link
Member

@le2sky le2sky 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 106 to 108
public String getThumbnail() {
return mission.getThumbnail();
}
Copy link
Member

Choose a reason for hiding this comment

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

[Approve]

이렇게 getter가 합쳐진 형태에서 네이밍이 고민되네요.
getMissionThumbnail이 제일 정확한 표현 같은데, 릴리의 의견은 어떤가요?
릴리의 선호에 따라 반영해주세요.

Copy link
Member Author

Choose a reason for hiding this comment

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

반영했습니다👍 현재 상태에선 getter가 합쳐졌다는 것이 명확하진 않네요

@@ -18,4 +18,6 @@ public interface SolutionRepository extends JpaRepository<Solution, Long> {
List<SolutionSummary> findCompletedSummaries();

Optional<Solution> findByMember_IdAndMission_IdAndStatus(Long memberId, Long missionId, SolutionStatus status);

List<Solution> findByMember_IdAndStatus(Long memberId, SolutionStatus solutionStatus);
Copy link
Member

Choose a reason for hiding this comment

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

[Request Changes]

반환값이 리스트인 경우 findBy로 시작하면 다른 개발자가 혼란스러울 수 있어요.
findAll로 시작하는 것은 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

수정했어요! 신경 못 썼던 부분 봐주셔서 감사합니다 :)

Copy link
Member

@alstn113 alstn113 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 ⛅️ 👍👍👍👍👍👍👍

Copy link
Contributor

@robinjoon robinjoon left a comment

Choose a reason for hiding this comment

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

안녕하세요 릴리! 수고하셨습니다!

반영해주셨으면 하는 코멘트가 있지만, 사소해서 이대로 머지해도 될 것 같아서 approve 드립니다!

Copy link
Contributor

@Minjoo522 Minjoo522 left a comment

Choose a reason for hiding this comment

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

안녕하세요 릴리 고생많으셨습니다!
다른 팀원들이 좋은 코멘트 남겨주셔서 바로 Approve합니다! 👍👍👍👍

Copy link
Member

@le2sky le2sky left a comment

Choose a reason for hiding this comment

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

릴쌤 고생 많으셨어요. 추가 반영 사항 없으시면 머지 부탁드려요. 👍

@lilychoibb lilychoibb merged commit 4d8b45c into main Aug 14, 2024
6 checks passed
@lilychoibb lilychoibb deleted the feat/#288 branch August 14, 2024 14:05
Minjoo522 pushed a commit that referenced this pull request Aug 19, 2024
* feat: 내가 제출한 솔루션 리스트 조회 기능 구현

* refactor: Solution thumbnail getter 이름 수정

* refactor: 멤버식별자와 상태로 솔루션을 조회하는 메서드의 네이밍 변경

* refactor: SolutionServiceTest 의 불필요한 Accessor 제거
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚒️ 기능 작업해야하는 기능 🚛 백엔드 백엔드 관련 이슈
Projects
Status: 😎 DONE
Development

Successfully merging this pull request may close these issues.

제출 미션 리스트 조회 기능
5 participants