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

빈자리 알림 로직 추가 #110

Merged
merged 4 commits into from
Jul 6, 2023
Merged

빈자리 알림 로직 추가 #110

merged 4 commits into from
Jul 6, 2023

Conversation

Hank-Choi
Copy link
Member

변경사항

  • 이전 학기 빈자리 알림 전체 삭제 로직 추가
    • 이전 학기 빈자리 알림은 보관할 필요 없다고 기획 확인 받아서 다음 수강편람 업데이트 시 전체 삭제
  • 빈자리 알림 강좌 조회/삭제 변경
    • 조회
      • 화면에서 lecture를 직접 뿌려주므로 api에서도 lecture 형태로 반환
    • 삭제
      • vacancyNotification id를 알 수 없게 됐으므로 생성할 때와 마찬가지로 lecture id 받도록 변경

@Hank-Choi Hank-Choi requested review from a team and PFCJeong as code owners July 6, 2023 09:42
@Hank-Choi Hank-Choi requested review from davin111, Jhvictor4 and subeenpark-io and removed request for a team July 6, 2023 09:42
override suspend fun deleteVacancyNotification(id: String) {
vacancyNotificationRepository.deleteById(id)
override suspend fun deleteAll() {
vacancyNotificationRepository.deleteAll()
Copy link
Member

Choose a reason for hiding this comment

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

이거 mongodb 성능 괜찮으려나?

Copy link
Member Author

Choose a reason for hiding this comment

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

drop table 할걸?

Copy link
Member Author

Choose a reason for hiding this comment

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

아니네.. 흠

Copy link
Member Author

Choose a reason for hiding this comment

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

사실 데이터가 많다면 컬렉션 삭제, 재생성하고 인덱스 재생성하면 더 빠를텐데 DDL날리는것도 좀 그렇고 걍 써도 될듯

path = "/v1/vacancy-notifications/lectures", method = [RequestMethod.GET], produces = [MediaType.APPLICATION_JSON_VALUE],
operation = Operation(
operationId = "getVacancyNotificationLectures",
responses = [ApiResponse(responseCode = "200", content = [Content(array = ArraySchema(schema = Schema(implementation = Lecture::class)))])]
Copy link
Member

Choose a reason for hiding this comment

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

우리 지난 번에 얘기한 건데 #70 (comment)

db entity 를 그대로 응답에 반환하지 않고 변환해서 전달하면 좋을 거 같아.

Copy link
Member Author

Choose a reason for hiding this comment

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

이거 안그래도 반영중이었음
너무 리뷰가 빨랐음ㅋㅋㅋ

Copy link
Member Author

Choose a reason for hiding this comment

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

@davin111 후속 리뷰는?

Copy link
Member

Choose a reason for hiding this comment

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

내 pr 도 리뷰 좀..!

@Hank-Choi Hank-Choi force-pushed the feature/vacancyNotiFix branch from 2a11d0a to fa7893c Compare July 6, 2023 10:13
@Hank-Choi Hank-Choi merged commit ca52929 into develop Jul 6, 2023
@Hank-Choi Hank-Choi deleted the feature/vacancyNotiFix branch August 18, 2023 02:52
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