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

카테고리 별 템플릿 개수 조회 #1031

Open
wants to merge 8 commits into
base: dev/be
Choose a base branch
from

Conversation

HoeSeong123
Copy link
Contributor

⚡️ 관련 이슈

close #967

📍주요 변경 사항

카테고리 별로 템플릿 개수를 조회합니다.
좋아요와 마찬가지로 반정규화를 진행하였습니다!

🎸기타

고려해야 하는 내용을 작성해 주세요.

🍗 PR 첫 리뷰 마감 기한

1/17(금)23:59 (여행 이슈로 넉넉하게 두었습니다)

@HoeSeong123 HoeSeong123 added feature 기능 추가 BE 백엔드 labels Jan 13, 2025
@HoeSeong123 HoeSeong123 added this to the 7차 스프린트 💭 milestone Jan 13, 2025
@HoeSeong123 HoeSeong123 self-assigned this Jan 13, 2025
Copy link
Contributor

@jminkkk jminkkk 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

Choose a reason for hiding this comment

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

요고 파일은 포함되어야 하는 거 맞을까요?
체크 한번 부탁드려요~!

@@ -76,6 +76,15 @@ public Template update(
return template;
}

private void checkCategoryChanged(Category category, Template template) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private void checkCategoryChanged(Category category, Template template) {
private void updateCategoryCount(Category newCategory, Template template) {

메서드명과 달리 단순 체크만 하지는 않고 내부 로직이 있어서 변경 제안해요!

84, 85라인 변수명도 조금 헷갈릴 수 있을 것 같아서 더 명확하게 제안해봅니당~!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

변수명에 new가 들어가는게 묘하게 불편해서 changed로 변경하였습니다!

Copy link
Contributor

@zeus6768 zeus6768 left a comment

Choose a reason for hiding this comment

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

깔끔하네용~~!

@HoeSeong123 HoeSeong123 force-pushed the feat/967-count-template-by-category branch from f70727c to a3f1e2e Compare January 19, 2025 15:27
Copy link
Contributor Author

Choose a reason for hiding this comment

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

이거 커밋에서 지우고 강제푸시 했는데도 남아있네요?
그런데 들어가서 확인해보면 아무것도 변경된 건 없다고 뜨긴 합니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드 feature 기능 추가
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

3 participants