feat: 사전 식물 상세 정보 조회 기능 구현 #72

yeonkkk · 2023-07-17T16:59:29Z

🔥 연관 이슈

close 사전 식물 상세 정보를 조회 기능을 구현한다 #15

🚀 작업 내용

사전 식물 상세 정보 조회 기능 구현

💬 리뷰 중점사항

특별한 로직이 많이 들어가는 기능은 아니라서 전반적으로 봐주시면 좋을 것 같아요.
잘 부탁드립니다 🌱

github-actions · 2023-07-17T17:00:43Z

Unit Test Results

29 tests 29 ✔️ 1s ⏱️
10 suites   0 💤
10 files   0 ❌

Results for commit c9017ae.

♻️ This comment has been updated with latest results.

Choi-JJunho

고생하셨습니다~
전체적으로 잘 작성해주셨는데 develop 브랜치 내용이 반영이 안된 것 같아요~!

git checkout feature/15-사전_식물_상세_정보_조회_기능
git pull origin develop
git push

브랜치 최신화 및 충돌 해결 이후 리뷰요청 한번 더 부탁드릴게요

Choi-JJunho · 2023-07-18T01:51:41Z

backend/pium/src/main/java/com/official/pium/mapper/DictionaryPlantMapper.java

+public class DictionaryPlantMapper {
+    public static DictionaryPlantResponse toDictionaryPlantResponse(DictionaryPlant dictionaryPlant) {


C

Suggested change

public class DictionaryPlantMapper {

public static DictionaryPlantResponse toDictionaryPlantResponse(DictionaryPlant dictionaryPlant) {

public class DictionaryPlantMapper {

public static DictionaryPlantResponse toDictionaryPlantResponse(DictionaryPlant dictionaryPlant) {

공백한줄만 띄워주세요~

수정했습니다!

Kim0914

조이 !! 고생많았습니다 ㅎㅎ

꼼꼼히 개발해주신 덕분에 리뷰 남길 부분이 많이 없는 것 같아요 !
궁금한 점은 코멘트로 남겼습니다 😄

Kim0914 · 2023-07-18T04:20:03Z

backend/pium/src/main/java/com/official/pium/service/DictionaryPlantService.java

+
+    public DictionaryPlantResponse read(Long id) {
+        DictionaryPlant dictionaryPlant = dictionaryPlantRepository.findById(id)
+                .orElseThrow(() -> new NoSuchElementException("사전 식물이 존재하지 않습니다. id : " + id));


A

별건 아니지만..! id: 콜론과 앞 단어는 붙여서 쓰는 방식으로 통일하면 어떨까요?!

그렇게 변경하는게 보기에 더 자연스러울 것 같네요. 수정했습니다!

Kim0914 · 2023-07-18T04:21:53Z

backend/pium/src/test/java/com/official/pium/controller/DictionaryPlantControllerTest.java

+                .andExpect(jsonPath("$.postingPlace").isArray())
+                .andExpect(jsonPath("$.waterCycle.spring").value("겉흙이 마르면 촉촉하게"))
+                .andDo(print());
+    }


R

사전 식물 상세 정보 조회의 실패하는 경우에 대한 테스트를 추가해보는건 어떨까요?!
컨트롤러에서 @Positive로 검증하고 있는데 해당 테스트가 없는 것 같아요 !

검증만 넣고 테스트를 작성을 놓쳤네요.
수정했습니다! 감사해요👍

Kim0914 · 2023-07-18T04:47:41Z

backend/pium/src/test/java/com/official/pium/service/DictionaryPlantServiceTest.java

+    }
+
+    @Test
+    void 사전_식물_상세_정보를_조회한다() {


C

dictionaryPlant가 해당 테스트에서만 사용되고 있는데 @BeforeEach로 생성하는 이유가 궁금합니다 😀

dictionaryPlant를 다른 테스트도 같이 작성하면서 사용해보다가, 최종적으로는 하나의 메서드에서만 사용하게 됐습니다.
그 과정에서 setup 메서드를 제거하는걸 놓쳤네요! 수정했습니다. 감사해요!🥹

Kim0914 · 2023-07-18T04:48:29Z

backend/pium/src/test/java/com/official/pium/controller/DictionaryPlantControllerTest.java

+
+        mockMvc.perform(get("/dictionary-plants/{dictionaryPlantId}", response.getId())
+                        .contentType(MediaType.APPLICATION_JSON_VALUE))
+                .andExpect(MockMvcResultMatchers.status().isOk())


꼼꼼한 검증 👍👍

Kim0914 · 2023-07-18T04:49:03Z

backend/pium/src/test/java/com/official/pium/controller/DictionaryPlantControllerTest.java

+                .andExpect(MockMvcResultMatchers.status().isOk())
+                .andExpect(jsonPath("$.id").value(1))
+                .andExpect(jsonPath("$.name").value("스투키"))
+                .andExpect(jsonPath("$.postingPlace").isArray())


A

isArray()는 어떤 내용을 검증하는 테스트 메서드인가요?!
단순 궁금증입니다 !

해당 path 의 값이 array인지 확인하는 메서드 입니다.
postingPlace의 경우 응답을 위해 DTO(DictionaryPlantResponse)에서 String → List로 변환 처리되기 때문에, 이 작업이 잘 진행됐는지 검증 하면 좋을 것 같아서 넣어봤습니다🙂

꼼꼼한 검증 좋아요 ~!

* feat: 물주기 남은 날짜, 함께한 날 도메인 로직 추가 * chore: rebase conflict resolve * refactor: 테스트 대상 객체 수정 * feat: 예외를 전역으로 처리하는 handler 추가 * feat: 단일 반려 식물 조회 요청을 처리하는 기능 추가 * feat: 반려 식물 조회 기능 추가 * refactor: 테스트 fixture 및 반환 타입 수정 * chore: controller dto 패키지 삭제 * feat: IllegalArgumentException 핸들링 추가 * refactor: 물주기 주기, 함께한 날 계산 메서드 예외 메시지 변경 * refactor: 기본 생성자 접근자 변경 * refactor: 반려 식물 생성 시 물주기 날짜, 함께한 날 계산 로직 변경 * refactor: 테스트 fixture, 변수명 변경 * refactor: 반려 식물 PK 검증 메시지 수정 * test: 반려 식물 PK 검증 메시지 수정 * refactor: PathVariable 명 변경

yeonkkk

@Choi-JJunho @Kim0914

주노, 그레이! 꼼꼼하게 리뷰해주셔서 감사해요🍀
리뷰주신 부분 모두 반영했습니다.
혹시 추가적으로 수정해야할부분이 있다면 알려주세요.
감사합니다!

yeonkkk · 2023-07-18T07:09:44Z

backend/pium/src/main/java/com/official/pium/mapper/DictionaryPlantMapper.java

+public class DictionaryPlantMapper {
+    public static DictionaryPlantResponse toDictionaryPlantResponse(DictionaryPlant dictionaryPlant) {


수정했습니다!

yeonkkk · 2023-07-18T07:10:12Z

backend/pium/src/main/java/com/official/pium/service/DictionaryPlantService.java

+
+    public DictionaryPlantResponse read(Long id) {
+        DictionaryPlant dictionaryPlant = dictionaryPlantRepository.findById(id)
+                .orElseThrow(() -> new NoSuchElementException("사전 식물이 존재하지 않습니다. id : " + id));


그렇게 변경하는게 보기에 더 자연스러울 것 같네요. 수정했습니다!

yeonkkk · 2023-07-18T07:13:34Z

backend/pium/src/test/java/com/official/pium/controller/DictionaryPlantControllerTest.java

+                .andExpect(MockMvcResultMatchers.status().isOk())
+                .andExpect(jsonPath("$.id").value(1))
+                .andExpect(jsonPath("$.name").value("스투키"))
+                .andExpect(jsonPath("$.postingPlace").isArray())


해당 path 의 값이 array인지 확인하는 메서드 입니다.
postingPlace의 경우 응답을 위해 DTO(DictionaryPlantResponse)에서 String → List로 변환 처리되기 때문에, 이 작업이 잘 진행됐는지 검증 하면 좋을 것 같아서 넣어봤습니다🙂

yeonkkk · 2023-07-18T07:14:12Z

backend/pium/src/test/java/com/official/pium/controller/DictionaryPlantControllerTest.java

+                .andExpect(jsonPath("$.postingPlace").isArray())
+                .andExpect(jsonPath("$.waterCycle.spring").value("겉흙이 마르면 촉촉하게"))
+                .andDo(print());
+    }


검증만 넣고 테스트를 작성을 놓쳤네요.
수정했습니다! 감사해요👍

yeonkkk · 2023-07-18T07:16:19Z

backend/pium/src/test/java/com/official/pium/service/DictionaryPlantServiceTest.java

+    }
+
+    @Test
+    void 사전_식물_상세_정보를_조회한다() {


dictionaryPlant를 다른 테스트도 같이 작성하면서 사용해보다가, 최종적으로는 하나의 메서드에서만 사용하게 됐습니다.
그 과정에서 setup 메서드를 제거하는걸 놓쳤네요! 수정했습니다. 감사해요!🥹

Kim0914

리뷰 반영해주신 부분 확인했습니다 !

Approve 할게요 ㅎㅎ 고생많으셨습니다.

방금 하마드 PR이 머지되어서 pull로 최신화한 후에 merge를 해야할 것 같아요.
pull 할 때 git config pull.rebase false한 후에 git pull origin develop을 실행해야 Push reject가 발생하지 않을 것 같네요 🙃

확인 부탁드려요 !

Choi-JJunho · 2023-07-18T08:56:18Z

리뷰 반영내용은 이상없는 것 같아요!
다만 위에서 그레이가 이야기한것처럼 develop 브랜치 변경사항을 반영한 뒤에 다시 확인해볼 필요가 있는 것 같아 approve는 잠시 미뤄두겠습니다~

yeonkkk · 2023-07-18T12:32:01Z

@Choi-JJunho
반영 완료했습니다~🎶

Choi-JJunho

고생하셨습니다~

* feat: 사전 식물 상세 정보 조회 기능 구현 * refactor: PathVariable `@NotNull` 검증 제거 * feat: 반려 식물 단일 조회 (#67) * feat: 물주기 남은 날짜, 함께한 날 도메인 로직 추가 * chore: rebase conflict resolve * refactor: 테스트 대상 객체 수정 * feat: 예외를 전역으로 처리하는 handler 추가 * feat: 단일 반려 식물 조회 요청을 처리하는 기능 추가 * feat: 반려 식물 조회 기능 추가 * refactor: 테스트 fixture 및 반환 타입 수정 * chore: controller dto 패키지 삭제 * feat: IllegalArgumentException 핸들링 추가 * refactor: 물주기 주기, 함께한 날 계산 메서드 예외 메시지 변경 * refactor: 기본 생성자 접근자 변경 * refactor: 반려 식물 생성 시 물주기 날짜, 함께한 날 계산 로직 변경 * refactor: 테스트 fixture, 변수명 변경 * refactor: 반려 식물 PK 검증 메시지 수정 * test: 반려 식물 PK 검증 메시지 수정 * refactor: PathVariable 명 변경 * test: DictionaryPlantControllerTest에 ID 값에 대한 테스트 추가 * chore: 개행 추가, 공백문자 제거 * test: 불필요한 코드 제거 --------- Co-authored-by: gray <[email protected]>

feat: 사전 식물 상세 정보 조회 기능 구현

7440d02

refactor: PathVariable @NotNull 검증 제거

1fd271e

yeonkkk requested review from Choi-JJunho and Kim0914 July 18, 2023 00:10

yeonkkk added this to the 2차 데모데이 milestone Jul 18, 2023

yeonkkk added 🌈 기능 새로운 기능을 개발합니다 🍉 백엔드 백엔드 관련 이슈입니다 labels Jul 18, 2023

yeonkkk self-assigned this Jul 18, 2023

Choi-JJunho requested changes Jul 18, 2023

View reviewed changes

Kim0914 requested changes Jul 18, 2023

View reviewed changes

Kim0914 and others added 4 commits July 18, 2023 16:27

test: DictionaryPlantControllerTest에 ID 값에 대한 테스트 추가

cc8c0d6

chore: 개행 추가, 공백문자 제거

f38acb2

test: 불필요한 코드 제거

acb0c0a

yeonkkk commented Jul 18, 2023

View reviewed changes

Kim0914 approved these changes Jul 18, 2023

View reviewed changes

chore: conflict resolve

c9017ae

Choi-JJunho approved these changes Jul 19, 2023

View reviewed changes

Choi-JJunho merged commit d2e6422 into develop Jul 19, 2023
4 checks passed

Choi-JJunho deleted the feature/15-사전_식물_상세_정보_조회_기능 branch July 19, 2023 01:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 사전 식물 상세 정보 조회 기능 구현 #72

feat: 사전 식물 상세 정보 조회 기능 구현 #72

yeonkkk commented Jul 17, 2023

github-actions bot commented Jul 17, 2023 •

edited

Loading

Choi-JJunho left a comment

Choi-JJunho Jul 18, 2023

yeonkkk Jul 18, 2023

Kim0914 left a comment

Kim0914 Jul 18, 2023

yeonkkk Jul 18, 2023

Kim0914 Jul 18, 2023

yeonkkk Jul 18, 2023

Kim0914 Jul 18, 2023

yeonkkk Jul 18, 2023

Kim0914 Jul 18, 2023

Kim0914 Jul 18, 2023

yeonkkk Jul 18, 2023

Kim0914 Jul 18, 2023

yeonkkk left a comment

yeonkkk Jul 18, 2023

yeonkkk Jul 18, 2023

yeonkkk Jul 18, 2023

yeonkkk Jul 18, 2023

yeonkkk Jul 18, 2023

Kim0914 left a comment •

edited

Loading

Choi-JJunho commented Jul 18, 2023

yeonkkk commented Jul 18, 2023

Choi-JJunho left a comment

		public class DictionaryPlantMapper {
		public static DictionaryPlantResponse toDictionaryPlantResponse(DictionaryPlant dictionaryPlant) {

feat: 사전 식물 상세 정보 조회 기능 구현 #72

feat: 사전 식물 상세 정보 조회 기능 구현 #72

Conversation

yeonkkk commented Jul 17, 2023

🔥 연관 이슈

🚀 작업 내용

💬 리뷰 중점사항

github-actions bot commented Jul 17, 2023 • edited Loading

Unit Test Results

Choi-JJunho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C

Choose a reason for hiding this comment

Kim0914 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

A

Choose a reason for hiding this comment

Choose a reason for hiding this comment

R

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

A

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeonkkk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kim0914 left a comment • edited Loading

Choose a reason for hiding this comment

Choi-JJunho commented Jul 18, 2023

yeonkkk commented Jul 18, 2023

Choi-JJunho left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 17, 2023 •

edited

Loading

Kim0914 left a comment •

edited

Loading