update :: self-study time #383

m2ri1 · 2024-07-15T12:04:23Z

💡 개요
profile이 dev일 때 자습신청이 "00:00-23:59"시에 모두 가능하도록 변경했습니다.

📃 작업내용
ValidDayOfWeekAndHourUtil 에서 selfStudyProperties값에 따라 자습신청을 다르게 제한하도록 작업하였습니다

🔀 변경사항
application-dev.yml, application-prod.yml에 self-study allowedTime 설정을 추가하였습니다

esperar · 2024-07-16T00:14:32Z

src/main/kotlin/com/dotori/v2/domain/selfstudy/util/ValidDayOfWeekAndHourUtil.kt

+         val allowedTime = selfStudyProperties.allowedTime ?: throw BasicException(ErrorCode.BAD_REQUEST);
+         val allowedStartTime = LocalTime.parse(allowedTime.split("-")[0])
+         val allowedEndTime = LocalTime.parse(allowedTime.split("-")[1])
+


이거 split("-") 두번이나 호출해서 쓰는것 같은데 split("-")으로 반환된 결과를 변수로 담아두고 [0] [1] 참조해서 쓰는게 더 나아보여요 그냥 allowedTime을 String[] 변수로 담아두고 쓰면 좋을 것 같습니다.

validateCancel 부분도 마찬가지구요!

#383 (comment)

리뷰 감사드립니다
properties에서 allowedStartTime, allowedEndTime 두 값을 받도록 로직 수정해보겠습니다

esperar · 2024-07-16T00:15:55Z

src/main/kotlin/com/dotori/v2/domain/selfstudy/properties/SelfStudyProperties.kt

+@Configuration
+@ConfigurationProperties(prefix = "self-study")
+class SelfStudyProperties {
+    var allowedTime: String? = null


기본값을 null로 초기화하기보다는 그냥 prod 환경에서의 기본 값으로 (8~9시 사이) 두는게 좋을 것 같아요. 아니면 그냥 아예 기본값을 두지 말거나?

allowedTime을 차라리 하나만 받아서 split하지말고 properties 자체에서 allowedStartTime, allowedEndTime 두개를 받아서 써도 될 것 같아요 그러면 #383 (comment) 이 리뷰도 해결가능할 것 같아요!

1630ed9
properties에서 allowedStartTime, allowedEndTime을 받아 신청 허용시간을 제한하도록 로직 변경했습니다!

아직도 null로 기본값을 넣어놓고있는데요? 수정 부탁드립니다.

@Configuration @ConfigurationProperties(prefix = "self-study") class SelfStudyProperties( val allowedStartTime, val allowedEndTime )

이런식으로요

04b821c
prod 환경에서의 기본값으로 초기화하고, dev 환경에서는 값을 오버라이드 할 수 있도록 수정하였습니다

esperar · 2024-07-16T00:18:06Z

src/main/kotlin/com/dotori/v2/domain/selfstudy/util/ValidDayOfWeekAndHourUtil.kt

        val currentTime = currentTime ?: LocalDateTime.now()
        val dayOfWeek = currentTime.dayOfWeek


currentTime 그냥 클래스 생성자로 받지 말고 없애버리고 validate 함수 호출되는 시점에 now를 호출하는게 좋아보이는데 어떻게 생각하시나요?

876ccd7
리뷰 반영해서 더욱 개선시켜 보았습니다! 감사합니다

update :: self-study time

5b89d27

m2ri1 requested review from uuuuuuuk, esperar and KimGyeongsuuu as code owners July 15, 2024 12:04

delete :: hour 변수 삭제

b4ee4bc

esperar reviewed Jul 16, 2024

View reviewed changes

m2ri1 added 6 commits July 17, 2024 20:46

update :: self-study allowed time 수정

1630ed9

update :: currentTime 로직 수정

876ccd7

update :: NotSelfStudyApplyDayException

24afc8c

update :: self-study ConfigurationProperties

04b821c

update :: ValidaDayOfWeekAndHourTest

c8d31bd

update :: ValidaDayOfWeekAndHourTest dev profile일때 수정

76e438e

esperar approved these changes Jul 20, 2024

View reviewed changes

m2ri1 merged commit 72cf03a into develop Jul 20, 2024
1 check passed

m2ri1 deleted the feat/selfstudy-timeset branch July 20, 2024 09:48

m2ri1 restored the feat/selfstudy-timeset branch July 23, 2024 11:31

m2ri1 deleted the feat/selfstudy-timeset branch August 8, 2024 15:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update :: self-study time #383

update :: self-study time #383

m2ri1 commented Jul 15, 2024

esperar Jul 16, 2024

esperar Jul 16, 2024

esperar Jul 16, 2024

m2ri1 Jul 17, 2024

esperar Jul 16, 2024

esperar Jul 16, 2024

m2ri1 Jul 17, 2024

esperar Jul 18, 2024

esperar Jul 18, 2024 •

edited

Loading

m2ri1 Jul 18, 2024 •

edited

Loading

esperar Jul 16, 2024

m2ri1 Jul 17, 2024

		val currentTime = currentTime ?: LocalDateTime.now()
		val dayOfWeek = currentTime.dayOfWeek

update :: self-study time #383

update :: self-study time #383

Conversation

m2ri1 commented Jul 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

esperar Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

m2ri1 Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

esperar Jul 18, 2024 •

edited

Loading

m2ri1 Jul 18, 2024 •

edited

Loading