Feature/#424 캘린더 도메인에 필요한 api를 개발한다 #438

02ggang9 · 2024-03-24T16:18:04Z

🔥 Related Issue

#424

📝 Description

캘린더 도메인 CRUD 중 C 부분 개발을 완료했습니다.

+)
Read 부분과 KEEPER DSL이 머지되면 Controller 테스트와 예외 테스트까지 작성하도록 하겠습니다.

⭐️ Review Request

shkisme

수고하셨습니다~~! 깔끔한 코틀린 코드 잘 봤습니다! 👍👍
리뷰 확인 부탁드립니다 🙇🙇🙇

shkisme · 2024-03-25T11:12:15Z

src/main/java/com/keeper/homepage/domain/calendar/api/ScheduleController.kt

+
+}


shkisme · 2024-03-25T11:14:36Z

src/test/java/com/keeper/homepage/global/util/thumbnail/server/ThumbnailServerUtilTest.java

    @Test
+    @Disabled
    @DisplayName("썸네일 저장 시 type이 null이면 NullPointerException을 발생시킨다.")


이 테스트는 왜 @disabled 처리 하셨나용?

퀵스타트 만들 때도 그렇고 항상 이 테스트가 깨졌습니다. 다른 분들은 통과 하시는지 궁금하네요..!! 메서드 들어가보면 null이면 NPE을 throw 하는게 아니라 optional empty로 반환해서 테스트가 깨지는 것 같습니다!

엇 뭐지 제 쪽에서는 다 통과하는데 한번 더 확인해보겠습니다

shkisme · 2024-03-25T11:22:49Z

src/main/java/com/keeper/homepage/domain/calendar/entity/ScheduleType.kt

+class ScheduleType(
+
+    @Column(name = "type", nullable = false)
+    val type: String,
+
+    @Column(name = "description", nullable = false)
+    val description: String,
+
+    @Id
+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    val id: Long?,
+)


inner enum class로 타입을 명시해두면 코드 가독성이 더 올라가지 않을까 합니다!
ex) MemberJob

좋습니다~! 감사합니다

shkisme · 2024-03-25T11:26:25Z

src/main/java/com/keeper/homepage/domain/calendar/entity/Schedule.kt

+
+    @Id
+    @GeneratedValue(strategy = GenerationType.IDENTITY)
+    val id: Long? = null,


Suggested change

val id: Long? = null,

val id: Long = 0

Nit: 어떨까요?!

저도 이 부분이 너무 헷갈리는데 다른 곳과 다른 분들은 어떻게 생각하시는지 궁금합니다.
코틀린 강의를 봤을 때는 nullable type을 사용하셨고 아래의 질문 게시판을 보니 nullable type 사용 이유를 설명해 주셨습니다. nullable type을 쓰면 테스트 코드 작성 등에서 !!을 계속 붙여줘야 해서 불편하다고 느꼈습니다.. 하지만 하이버네이트가 nullable type을 추천한다고 합니다.

아래 게시판을 보시고 의견 남겨주시면 감사하겠습니다..!!

https://www.inflearn.com/course/lecture?courseSlug=java-to-kotlin-2&unitId=121335&category=questionDetail&q=684857&tab=community

아하 하이버네이트가 nullable 타입을 추천하는군요... 처음 알았습니다!
엔티티의 필드를 Wrapper로 선언하는 것과 동일한 맥락이겠죠? 그렇다면 코드를 작성할 때 조금 번거롭더라도 Long?으로 선언하는 것도 좋아보입니다!

shkisme · 2024-03-25T11:27:37Z

src/main/java/com/keeper/homepage/domain/calendar/application/ScheduleService.kt

+@RequiredArgsConstructor
+@Transactional(readOnly = true)
+class ScheduleService(
+    val scheduleRepository: ScheduleRepository,
+    val scheduleTypeRepository: ScheduleTypeRepository,
+) {


코틀린에서는 @RequiredArgsConstructor가 없어도 동작하는 걸로 알고 있습니다!

보름님 말씀대로 RequiredArgsConstructor 필요 없습니다!

shkisme · 2024-03-25T11:28:45Z

src/main/java/com/keeper/homepage/domain/calendar/application/ScheduleService.kt

+        endTime: LocalDateTime, scheduleTypeId: Long,
+    ): Long {
+        val findSchedule = scheduleTypeRepository.findByIdOrNull(scheduleTypeId)
+            ?: throw IllegalArgumentException("ScheduleType not found")


Nit: 다른 코드들과 통일성 있게 커스텀 예외를 등록해두는 건 어떨까요?

shkisme · 2024-03-25T11:29:59Z

src/main/java/com/keeper/homepage/domain/calendar/application/ScheduleService.kt

+    fun saveSchedule(name: String, startTime: LocalDateTime,
+        endTime: LocalDateTime, scheduleTypeId: Long,
+    ): Long {


Suggested change

fun saveSchedule(name: String, startTime: LocalDateTime,

endTime: LocalDateTime, scheduleTypeId: Long,

): Long {

fun saveSchedule(

name: String,

startTime: LocalDateTime,

endTime: LocalDateTime,

scheduleTypeId: Long,

): Long {

너무나 사소하지만... 저는 이렇게 포맷팅하지 않으면 불편한 병에 걸렸습니다 ㅎㅎ 🤣 (코틀린 권장 스타일이긴 하더라구요!)

저도 이 부분을 5번 정도 지웠다 했는데 다시 보니 코틀린 권장 스타일이 좋을 것 같습니다 bb

감사합니다

shkisme · 2024-03-25T11:34:03Z

src/main/java/com/keeper/homepage/domain/calendar/api/ScheduleController.kt

+
+@Validated
+@RestController
+@RequestMapping("/schedule")


Suggested change

@RequestMapping("/schedule")

@RequestMapping("/schedules")

복수형 어떨까욥!

shkisme · 2024-03-25T11:35:25Z

src/main/java/com/keeper/homepage/domain/calendar/api/ScheduleController.kt

+            saveScheduleRequest.scheduleTypeId
+        )
+
+        return ResponseEntity.ok().build()


데이터를 생성하는 api이니, 201 Created 응답코드를 반환하는 건 어떨까요?

아 그렇네요 감사합니다

shkisme · 2024-03-25T11:38:13Z

src/main/java/com/keeper/homepage/domain/calendar/application/ScheduleService.kt

+        return scheduleRepository.save(
+            Schedule(
+                name = name,
+                startTime = startTime,
+                endTime = endTime,
+                scheduleType = findSchedule,
+            )
+        ).id!!


startTime이 endTime보다 앞선 시간이 맞는 지도 검증해보면 좋을 것 같습니다!
+) 세미나 도메인에도 비슷한 검증 로직이 있어서, 커스텀 어노테이션을 하나 만들어서 재사용하는 것도 좋을 것 같네요!!

stopmin

수고하셨습니다🔥
저도 빨리 도서관 도메인 끝내고 캘린더 하러 가겠습니당~~!!

좋은코드 잘 봤습니당!!

stopmin · 2024-03-25T14:47:06Z

src/main/java/com/keeper/homepage/domain/calendar/application/ScheduleService.kt

+@RequiredArgsConstructor
+@Transactional(readOnly = true)
+class ScheduleService(
+    val scheduleRepository: ScheduleRepository,
+    val scheduleTypeRepository: ScheduleTypeRepository,
+) {


보름님 말씀대로 RequiredArgsConstructor 필요 없습니다!

stopmin · 2024-03-25T14:50:05Z

src/main/java/com/keeper/homepage/domain/calendar/application/ScheduleService.kt

+
+    @Transactional
+    fun saveSchedule(name: String, startTime: LocalDateTime,
+        endTime: LocalDateTime, scheduleTypeId: Long,


개인적으로 편의상 endTime의 default 값을 startTime + 1로 잡아두는 것은 어떨까욤??

코틀린은 가능합니다!! endTime:LocalDateTime?= startTime.addDate(1)같이요! ㅎㅎ 보통 일정은 하루로 등록할 일이 많다고 생각합니당 :D

그렇네요.. 보통 디폴트 날짜는 보통 하루겠네요~ 감사합니다

stopmin · 2024-03-25T14:53:07Z

src/main/java/com/keeper/homepage/domain/calendar/dto/request/SaveScheduleRequest.kt

+
+data class SaveScheduleRequest(
+
+    @NotEmpty(message = "일정 이름을 입력해주세요.")


이거 혹시 동작하나요?? 코틀린은 @field:xxx방식으로 달아줘야하는 것으로 알고있습니당!

02ggang9 added 6 commits March 24, 2024 21:49

feat: 캘린더, 스케줄 타입 엔티티 작성

dfe48f8

feat: 캘린더 저장 기능

c24bedd

test: 캘린더 테스트 핼퍼 작성

adc2d80

fix: 캘린더 필드 네이밍 수정 및 테스트 코드 필드 이름 수정

01b860b

test: saveSchedule 메서드 테스트 작성

bb2c00d

chore: 테스트 코드 디렉토리 수정

f5e5dff

02ggang9 assigned 02ggang9 and stopmin Mar 24, 2024

02ggang9 requested review from lcqff, amaran-th, shkisme, sososo0, JJimini and stopmin March 24, 2024 16:18

feat: 스케줄 저장 컨트롤러 기능 구현

32c38e3

shkisme approved these changes Mar 25, 2024

View reviewed changes

stopmin approved these changes Mar 25, 2024

View reviewed changes

02ggang9 added 8 commits March 26, 2024 13:16

test 스케줄 컨트롤러 테스트 작성

6e9810a

test: 캘린더 저장 기능 예외 테스트

6124ed1

feat: 캘린더 조회 기능 구현

781e280

test: 캘린더 조회 테스트 작성

ed892e3

test: 일정 조회 테스트 코드 작성

fc71cb6

test: 캘린더 일정 조회 엣지 케이스 작성

126c7dd

fix: inner Enum을 사용하도록 수정

ad4a22d

fix: Reqeust DTO에 @field 추가

c75eb4f

02ggang9 requested review from shkisme, stopmin and yeochaeeon March 28, 2024 14:01

gusah009 marked this pull request as draft April 30, 2024 12:19

gusah009 removed the request for review from lcqff April 30, 2024 12:20

gusah009 removed request for amaran-th, shkisme, sososo0, JJimini, stopmin and yeochaeeon April 30, 2024 12:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/#424 캘린더 도메인에 필요한 api를 개발한다 #438

Feature/#424 캘린더 도메인에 필요한 api를 개발한다 #438

02ggang9 commented Mar 24, 2024 •

edited

Loading

shkisme left a comment

shkisme Mar 25, 2024

shkisme Mar 25, 2024

02ggang9 Mar 26, 2024

shkisme Mar 28, 2024

shkisme Mar 25, 2024

02ggang9 Mar 26, 2024

shkisme Mar 25, 2024

02ggang9 Mar 26, 2024

shkisme Mar 28, 2024 •

edited

Loading

shkisme Mar 25, 2024

stopmin Mar 25, 2024 •

edited

Loading

02ggang9 Mar 26, 2024

shkisme Mar 25, 2024

shkisme Mar 25, 2024

02ggang9 Mar 26, 2024

shkisme Mar 25, 2024

shkisme Mar 25, 2024 •

edited

Loading

02ggang9 Mar 26, 2024

shkisme Mar 25, 2024

stopmin left a comment

stopmin Mar 25, 2024 •

edited

Loading

stopmin Mar 25, 2024

02ggang9 Mar 26, 2024

stopmin Mar 25, 2024


		data class SaveScheduleRequest(

		@NotEmpty(message = "일정 이름을 입력해주세요.")

Feature/#424 캘린더 도메인에 필요한 api를 개발한다 #438

Are you sure you want to change the base?

Feature/#424 캘린더 도메인에 필요한 api를 개발한다 #438

Conversation

02ggang9 commented Mar 24, 2024 • edited Loading

🔥 Related Issue

📝 Description

⭐️ Review Request

shkisme left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shkisme Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stopmin Mar 25, 2024 • edited Loading

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

shkisme Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stopmin left a comment

Choose a reason for hiding this comment

stopmin Mar 25, 2024 • edited Loading

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

02ggang9 commented Mar 24, 2024 •

edited

Loading

shkisme Mar 28, 2024 •

edited

Loading

stopmin Mar 25, 2024 •

edited

Loading

shkisme Mar 25, 2024 •

edited

Loading

stopmin Mar 25, 2024 •

edited

Loading