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

♻️ change package for category recipe search #112

Merged
merged 3 commits into from
Jul 29, 2024
Merged

Conversation

hyxrxn
Copy link
Contributor

@hyxrxn hyxrxn commented Jul 27, 2024

category 패키지에 있던 카테고리별 레시피 조회 기능을 recipe 패키지로 옮겼습니다.
recipe.sql에 테스트 데이터를 추가하며 깨지는 테스트가 생겨 함께 수정했습니다.

제가 맡았던 도메인이 아닌 부분을 약간 건드려 실수가 있을 수 있습니다.
꼼꼼한 확인 부탁드립니다!!

Copy link

Overall Project 89% 🍏
Files changed 100% 🍏

File Coverage
CategoryService.java 100% 🍏
RecipeService.java 100% 🍏
RecipeController.java 100% 🍏

List<Long> recipeIds = categoryRecipeRepository.findRecipeIdsByCategoryName(categoryName, pageable);

List<RecipeDataResponse> recipeDataResponses = recipeRepository.findRecipeData(recipeIds);
return convertToMainRecipeResponses(recipeDataResponses);

Choose a reason for hiding this comment

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

카테고리별 조회 로직이 recipe 패키지에 포함돼서 convert 메서드 접근제어자는 다시 private로 바꾸면 좋을 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은 생각이에요!!

);

List<RecipeDataResponse> recipeData = repository.findRecipeData(recipeIds);

assertAll(
() -> assertThat(recipeData).hasSize(8),
() -> assertThat(recipeData).hasSize(6 + 3),

Choose a reason for hiding this comment

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

제가 작성했던 부분이지만 다시 봐도 검증 방식이 애매하긴 하네요. 🥲 데이터 가져오는 방식이나 쿼리문을 향후 개선하기는 해야할 것 같습니다!
데이터 수정하시느라 고생 많으셨어요.

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
Contributor

@geoje geoje left a comment

Choose a reason for hiding this comment

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

코드 이동 및 병합이 대부분이네요!
이미 엘라의 리뷰로 충분한 것 같아요~ 고생하셨어요 굿굿!

@@ -27,4 +29,9 @@ public List<MainRecipeResponse> readRecipes(@RequestParam int pageNumber, @Reque
public List<RecipeStepResponse> readRecipeSteps(@PathVariable long id) {
return recipeService.readRecipeSteps(id);
}

@GetMapping("/search")
public List<MainRecipeResponse> readRecipesOfCategory(@ModelAttribute RecipeOfCategoryRequest request) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이제 슬슬 해당 DTO 안에 validation 을 추가하는 것은 어떠신가요~?

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

Overall Project 91.6% 🍏
Files changed 100% 🍏

File Coverage
CategoryService.java 100% 🍏
RecipeService.java 100% 🍏
RecipeController.java 100% 🍏

@hyxrxn hyxrxn merged commit eb405df into be/dev Jul 29, 2024
1 check passed
@hyxrxn hyxrxn deleted the be/feat/109 branch July 29, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE Backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants