feat: 인기있는 학습로그를 각 카테고리 별 공통, 프론트엔드, 백엔드로 제공하는 기능 구현 #897
Conversation
There was a problem hiding this comment.
현재 작성되어있는 학습로그가 어떤 코스(프론트엔드 or 백엔드)에 속해있는지 확인하기 어려울꺼에요
그래서 정규식으로 구분을 하고 있는 것 같은데 조금 더 명확하게 하면 좋을 것 같아요.
도메인 중에 MemberGroup이라는 도메인이 있을꺼에요
이 도메인으로 백엔드와 프론트엔드를 구분할 수 있어요
그래서 학습로그를 조회할 때 작성자가 어떤 MemberGroup에 속해있는지를 판단해서
어떤 과정의 학습로그인지를 판단하면 조금 더 명확하지 않을까? 라는 생각이 들어요
확인해서 궁금한 점 있으면 다시 질문 주세요~
| // assertThat(popularStudylogs.getAllResponse().getData().get(0).getId()).isEqualTo(studylogResponse1.getId()); | ||
| // assertThat(popularStudylogs.getAllResponse().getData().get(1).getId()).isEqualTo(studylogResponse2.getId()); |
There was a problem hiding this comment.
주석은 과감히 제거해주셔도 됩니다 :)
| @@ -43,7 +42,7 @@ class PopularStudylogServiceTest { | |||
| private static final String STUDYLOG2_TITLE = "이것은 두번째 제목"; | |||
| private static final String STUDYLOG3_TITLE = "이것은 3 제목"; | |||
| private static final String STUDYLOG4_TITLE = "이것은 네번 제목"; | |||
|
|
|||
| private static final String URL = "http://localhost:8080"; | |||
There was a problem hiding this comment.
작성되어 있는 코드라 크게 신경쓰지 않았는데 사용되지 않고 있군요 ㅎㅎ.. 제거하도록 하겠습니다 😁
| this.studylogRequest = new StudylogRequest(title, content, sessionId, missionId, | ||
| tagRequests); |
There was a problem hiding this comment.
기존 Fixture들이 파라미터마다 개행이 되어있는데요,
이 생성자 또한 같은 모양새를 갖추면 조금 더 가독성이 올라갈거 같아요
There was a problem hiding this comment.
놓친 부분이네요 ㅎㅎ 수정해서 반영하겠습니다~
| STUDYLOG8( | ||
| "[자바스크립트] JS JS JS 신나는 노래", | ||
| "덤덤 노래 아시는 분", | ||
| 3L, | ||
| 3L, | ||
| TAG5, | ||
| TAG6 | ||
| ), | ||
| STUDYLOG9( | ||
| "[자바] JAVA 주세요", | ||
| "Java를 잡아라", | ||
| 4L, | ||
| 4L, | ||
| TAG5, | ||
| TAG6 |
There was a problem hiding this comment.
(기존 픽스쳐들과 통일성을 위해서 번호를 이용한 네이밍을 유지한 것 같지만 😄)
조금 더 구체적인 변수명을 가져도 좋을거 같아요!
There was a problem hiding this comment.
저도 네이밍을 이용한 픽스쳐가 명확한 전달을 해줄 것으로 생각하는데요ㅎㅎ 현재 구조가 선택된 이유는 현구막 말씀처럼 기존 픽스쳐들과 통일성을 위해서 유지를 하였습니다.
다만 다른 의견도 있었는데요 #875 (comment) 와 같은 리뷰를 참고해서 기존처럼 번호 하드코딩으로 하자는 의견도 있었습니다.
There was a problem hiding this comment.
굿굿! 단순히 따라한게 아니라 의도를 가지고 진행하신거라면 충분하다고 생각합니다!
| public boolean isSameAs(Curriculum curriculum) { | ||
| return curriculum.findCurriculum(this.name); | ||
| } | ||
|
|
There was a problem hiding this comment.
픽스쳐에서 사용되는 메서드를 프로덕션 엔티티가 가지고 있는 이유는 무엇인가요?
There was a problem hiding this comment.
엇 저는 해당 메서드를 픽스쳐에서만 사용되지 않는다고 생각하는데요. 이 메서드를 통해서 Seesion이 어디 과정에 속하는지 PopularStudylogService에서 확인할 수 있다고 생각하고 있습니다.
There was a problem hiding this comment.
오 그렇네요!! 제가 섣부르게 확인한거 같습니다 🙏
(제 환경에서 로드가 덜 된건지, 이번에는 픽스쳐에는 안보이네요... 뭐지...)
isSameAs 메서드가 PopularStudylogService에서만 사용되는거 같은데요,
Studylog 엔티티에서 Session을 뽑아낸 다음 isSameAs를 호출하고 있는거 같습니다.
Studylog 에서 Session 쪽으로 메세지를 보내보면 어떨까요?
There was a problem hiding this comment.
앗 그렇네요 ㅎㅎ.. 현구막 말씀대로 메세지를 보내는 방향으로 수정했습니다!!
추가적으로 브라운이랑 얘기해본 결과 Curriculum을 사용하지 않고 기존의 클래스 중 하나인 MemberGroup을 활용하는 방향으로 진행하기로 했습니다~! 그래서 기존 코드가 수정이 이루어질 것으로 보입니다!!
|
추가로 SonarCloud 코드 스멜도 확인해주시면 좋을거 같아요! |
|
SonarCloud Quality Gate failed. |
|
Close 후 PR 다시 올리겠습니다! |
기능 구현 전
기능 구현
Issue
#881