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

결석 처리 API 및 배치 개발 #105

Merged
merged 8 commits into from
Jul 29, 2023
Merged

결석 처리 API 및 배치 개발 #105

merged 8 commits into from
Jul 29, 2023

Conversation

mkSpace
Copy link
Collaborator

@mkSpace mkSpace commented Jul 29, 2023

결석 처리 API 및 배치 개발

@@ -113,4 +113,9 @@ class StudyService(
.filter { !it.isInitialized } // 모종의 initialize 실패 인입 방지
.map { it.toDomain() }
}

fun findUnfinalizedStudiesByEndAt(endAt: LocalDateTime): List<Study> {
Copy link
Contributor

Choose a reason for hiding this comment

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

메소드 뒤에 Before 도 넣어줘야 더 명확할 것 같습니다..!

.forEach { (moitId, studies) ->
studies.forEach { study ->
attendanceService.adjustUndecidedAttendancesByStudyId(study.id)
.forEach { attendanceId -> fineService.create(attendanceId, moitId) }
Copy link
Contributor

Choose a reason for hiding this comment

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

eventProducer.produce(StudyAttendanceEvent(attendanceId = attendance.id, moitId = it.moitId)
로 벌금 생성은 비동기로 뺄 수 있을 것 같습니다!
(com.mashup.moit.facade.StudyFacade#registerAttendanceKeyword 참고)

Comment on lines 77 to 84
studyService.findByStudyIds(studyIds.toList())
.groupBy { study -> study.moitId }
.forEach { (moitId, studies) ->
studies.forEach { study ->
attendanceService.adjustUndecidedAttendancesByStudyId(study.id)
.forEach { attendanceId -> fineService.create(attendanceId, moitId) }
}
}
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
studyService.findByStudyIds(studyIds.toList())
.groupBy { study -> study.moitId }
.forEach { (moitId, studies) ->
studies.forEach { study ->
attendanceService.adjustUndecidedAttendancesByStudyId(study.id)
.forEach { attendanceId -> fineService.create(attendanceId, moitId) }
}
}
studyService.findByStudyIds(studyIds.toList()).forEach {
attendanceService.adjustUndecidedAttendancesByStudyId(it.id)
.forEach { attendanceId -> fineService.create(attendanceId, it.moitId) }
}

이 로직이랑 동일한 로직인거 같은데 좀 더 간결하게 줄이면 어떨까요~?

.map { it.id }
logger.info("{} undecided studies start! Start adjusting absence status at {}.", undecided.size, scheduleContext)

studyService.findByStudyIds(undecided)
Copy link
Contributor

Choose a reason for hiding this comment

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

studyService.findUnfinalizedStudiesByEndAtBefore() 가 이미 Study List 반환해서 다시 studyService 에서 조회할 필요가 없을 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

엇 코드 고치다가 까먹었네요! 감사합니당!!

// 결석 확정을 지을 때 endAt을 현재 시간보다 15초 정도 유예기간을 줌. 5분마다 배치가 돌기 때문에 95% 시간 내에 끝난 스터디를 반환함
val scheduleContext = LocalDateTime.now()
val undecided = studyService
.findUnfinalizedStudiesByEndAtBefore(LocalDateTime.now().minusSeconds(DECIDE_ABSENCE_RANGE_SECONDS))
Copy link
Contributor

Choose a reason for hiding this comment

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

위에서 now 를 변수로 선언해 두었다면, 여기서도 사용하면 어떨까요~?

@@ -40,6 +40,9 @@ class StudyEntity(
@Column(name = "is_initialized", nullable = false)
var isInitialized: Boolean = false

@Column(name = "is_finalized", nullable = false)
var isFinalized: Boolean = false
Copy link
Member

Choose a reason for hiding this comment

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

스터디가 끝나고 지각/결석처리가 다 되었을 때 확인 필드용

@mkSpace mkSpace force-pushed the feature/absence_api branch from 3c517f7 to 0c5bceb Compare July 29, 2023 17:18
Copy link
Member

@KimDoubleB KimDoubleB left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

Comment on lines +89 to +91
event.fineIds.forEach { fineId ->
bannerService.update(MoitUnapprovedFineExistBannerUpdateRequest(fineId))
}
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
Collaborator Author

Choose a reason for hiding this comment

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

넵! 배너 부분 숙지하고 제가 다음에 PR 올리도록 하겠습니다!

@mkSpace mkSpace merged commit ad381a4 into develop Jul 29, 2023
@mkSpace mkSpace deleted the feature/absence_api branch July 29, 2023 17:42
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.

4 participants