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

feat: department-options, favorite-departments API #73

Merged
merged 6 commits into from
Feb 15, 2024

Conversation

doxylee
Copy link
Contributor

@doxylee doxylee commented Feb 8, 2024

리뷰 필요 사항

  1. /session 아래의 API이긴 한데 실제 동작은 각각 다른 모듈에 속해있는 것 같아 아래처럼 호출하고 dependency가 생기게 만들었습니다. 이렇게 해도 될지, 더 좋은 방법이 있을지 의견 주시면 감사하겠습니다.
  • /session/department-options -> SessionController -> DepartmentsService -> DepartmentsRepository
  • /session/favorite-departments -> SessionController -> SessionService -> UserRepository
  1. 기존에 /session/department-options API가 department별로 exist 쿼리를 한 번씩 날려서 3초 이상 걸리도록 비효율적으로 짜여져 있었는데, 이를 raw query를 이용해 효율화 했습니다. 다만 테이블 변경이 있을 시에 이 부분을 유의해야겠습니다.

@doxylee doxylee requested a review from ddungiii February 8, 2024 13:34
src/common/interfaces/ISession.ts Outdated Show resolved Hide resolved
Comment on lines 89 to 93
const res = (await this.prisma.$queryRaw`
SELECT DISTINCT d.code
FROM subject_lecture l
JOIN subject_department d on l.department_id = d.id
WHERE l.year >= ${yearThreshold};`) as { code: string }[];
Copy link
Member

Choose a reason for hiding this comment

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

raw query로 해결해야하는 이유가 있을까요?
distinct가 사용이 안되나요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

include된 필드에는 distinct가 안된다고 본 것 같아서 이렇게 했는데 다시 보니 distinct도 안 쓰고 가능한 방법이 있어서 고쳤습니다.

Copy link
Member

@ddungiii ddungiii left a comment

Choose a reason for hiding this comment

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

LGTM!
console.log만 지워주세요

),
]);

console.log('recentDepartmentCodes', recentDepartmentCodes);
Copy link
Member

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.

넵!

@doxylee doxylee merged commit b168064 into dev Feb 15, 2024
3 checks passed
@doxylee doxylee deleted the feat/department-options-favorite-departments branch February 15, 2024 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants