[feat] 반복옵션 로직 추가 #34

jaea-kim · 2024-03-27T21:13:44Z

✨ Key changes

[feat] 반복 옵션 기능 추가 #29

👋 To reviewers

반복 횟수, 종료 시간 서버에서 계산해서 넣어주기

종료 시간 연산 : Instant -> ZonedDateTime -> 연산 로직 -> Instant 변환
반복 횟수 연산 : Instant -> ZonedDateTime -> ChronoUnit 활용하여 연산

`Instant` -> `ZonedDateTime` 변환 유틸 클래스 분리 여부 질문

분리 시
path : ~/plan/util/DateTimeUtils.java
Frequency 코드

WEEKLY {
        @Override
        public Instant calculateEndDate(Instant startDate, int count) {
            ZonedDateTime zdtStart = DateTimeUtils.toZonedDateTime(startDate);
            return DateTimeUtils.toInstant(zdtStart.plusWeeks(count - 1));
        }

        @Override
        public int calculateCount(Instant startDate, Instant endDate) {
            ZonedDateTime zdtStart = DateTimeUtils.toZonedDateTime(startDate);
            ZonedDateTime zdtEnd = DateTimeUtils.toZonedDateTime(endDate);
            return (int) ChronoUnit.WEEKS.between(zdtStart, zdtEnd) + 1;
        }
    }

분리 X
Frequency 코드

    MONTHLY {
        @Override
        public Instant calculateEndDate(Instant startDate, int count) {
            ZonedDateTime zdtStart = startDate.atZone(ZoneId.systemDefault());
            return zdtStart.plusMonths(count - 1).toInstant();
        }

        @Override
        public int calculateCount(Instant startDate, Instant endDate) {
            ZonedDateTime zdtStart = startDate.atZone(ZoneId.systemDefault());
            ZonedDateTime zdtEnd = endDate.atZone(ZoneId.systemDefault());
            long monthsBetween = ChronoUnit.MONTHS.between(zdtStart, zdtEnd);
            return (int) monthsBetween + 1;
        }
    }

util 클래스로 분리해도 코드가 줄어들지 않고 그렇다고 엄청난 분리 효과..? 가 느껴지지 않는 거 같은데 어떤가요?

Plan 생성 시 반복옵션 수만큼 추가 생성

횟수 추가(3번) 쿼리

Hibernate: 
    insert 
    into
        recurring_option
        (end_date, end_option, frequency, recurrence_count) 
    values
        (?, ?, ?, ?)
Hibernate: 
    insert 
    into
        plan
        (description, end_time, notification_time, plan_category_id, recurring_option_id, start_time, study_id, title) 
    values
        (?, ?, ?, ?, ?, ?, ?, ?)
Hibernate: 
    insert 
    into
        plan
        (description, end_time, notification_time, plan_category_id, recurring_option_id, start_time, study_id, title) 
    values
        (?, ?, ?, ?, ?, ?, ?, ?)
Hibernate: 
    insert 
    into
        plan
        (description, end_time, notification_time, plan_category_id, recurring_option_id, start_time, study_id, title) 
    values
        (?, ?, ?, ?, ?, ?, ?, ?)

날짜 추가 (다음주 까지 총 2번 반복)

Hibernate: 
    insert 
    into
        recurring_option
        (end_date, end_option, frequency, recurrence_count) 
    values
        (?, ?, ?, ?)
Hibernate: 
    insert 
    into
        plan
        (description, end_time, notification_time, plan_category_id, recurring_option_id, start_time, study_id, title) 
    values
        (?, ?, ?, ?, ?, ?, ?, ?)
Hibernate: 
    insert 
    into
        plan
        (description, end_time, notification_time, plan_category_id, recurring_option_id, start_time, study_id, title) 
    values
        (?, ?, ?, ?, ?, ?, ?, ?)

@PrePersist 옵션 부작용(?)

@PrePersist 어노테이션이 붙은 메소드는 엔티티가 영속성 컨텍스트에 저장되기 직전에 자동으로 호출되는 특징이 있습니다.
현재 해당 메소드에서 recurringOption의 반복횟수 및 종료시간 연산을 진행 중 입니다.

추가로 생성된 plan이 저장될 때에도 반복옵션이 종료시간이 새롭게 생긴 시간을 기준으로 다시 연산을 하여 저장하게 되는 문제가 발생하였습니다.

아래 코드를 추가하여 추가로 연산되는 작업을 방지했습니다. 좀,, 코드가 별로라 혹시 다른 방법 있으면 추천 부탁드려요,,

        if (recurrenceCount != null && endDate != null) {
            return;
        }

반복옵션만큼 plan 이 생성됐을 때 반환값은 어떻게 할까요?

현재 첫번째로 생성된 Plan id 반환하고 있습니다.
해당 메소드 리턴값이 Long 타입이라 어떤 것을 반환해야할지🤔

Plan 삭제 시 전체 삭제일 경우, 관련 반복옵션 모두 삭제

임시로 기능을 구현하기 위해서 delete API 에 boolean applyAll 이라는 쿼리 파라미터를 추가하여 구현하였습니다.

추가 논의

plan 삭제를 누르면 반복옵션이 존재할 경우, 관련 paln 모두 삭제 / 하나만 삭제 여부를 묻는 팝업창이 필요할 것 같습니다.
전체 삭제 API 를 분리 여부도 논의가 필요할 것 같습니다.

TODO

API 명세 보면 수정 기능은 모든 기능이 완료되면 다시 회의해서 정하자고 나와있어서 일단 PR에서 제외했습니다.
- Plan 변경 시 전체 변경일 경우, 관련 반복옵션 모두 변경
- Plan 변경 시 해당 일정만 변경 일 경우
Plan 관련 예외처리도 아직 에이프 예외처리 PR이 머지되지 않아서 PR 머지 후에 추가하겠습니다.

crtEvent · 2024-03-28T04:13:28Z

Instant -> ZonedDateTime 변환 유틸 클래스 분리 여부 질문

util 클래스로 분리해도 코드가 줄어들지 않고 그렇다고 엄청난 분리 효과..? 가 느껴지지 않는 거 같은데 어떤가요?

DateTimeUtils에 두 '날짜의 차이 계산하기', '날짜에 1달 더하기' 같은 기능을 가지게 하면 1~2줄로 줄일 수 있지 않을까 하는 생각이 듭니다.

        @Override
        public int calculateCount(Instant startDate, Instant endDate) {
            ZonedDateTime zdtStart = DateTimeUtils.toZonedDateTime(startDate);
            ZonedDateTime zdtEnd = DateTimeUtils.toZonedDateTime(endDate);
            return (int) ChronoUnit.WEEKS.between(zdtStart, zdtEnd) + 1;
        }

        @Override
        public int calculateCount(Instant startDate, Instant endDate) {
            return DateTimeUtils.calculateWeeksBetween(startDate, endDate);
        }

        @Override
        public Instant calculateEndDate(Instant startDate, int count) {
            ZonedDateTime zdtStart = startDate.atZone(ZoneId.systemDefault());
            return zdtStart.plusMonths(count - 1).toInstant();
        }

        @Override
        public Instant calculateEndDate(Instant startDate, int count) {
            return DateTimeUtils.plusMonths(startDate, count - 1)
        }

crtEvent · 2024-03-28T04:33:52Z

반복옵션만큼 plan 이 생성됐을 때 반환값은 어떻게 할까요?
현재 첫번째로 생성된 Plan id 반환하고 있습니다.
해당 메소드 리턴값이 Long 타입이라 어떤 것을 반환해야할지🤔

List을 해주면 어떨까요?

{
  "planIds": [1, 2, 3, 4, 5]
}

다른 의견은 생성된 Plan의 개수를 알려주는게 좋을수도 있다는 생각이 드네요.
Plan 생성 후 다시 get 요청으로 Plan들을 불러 올거라면 planId가 필요 없을 것 같아요...

아니면 savePlan 한정으로 Plan Entity객체를 다 반환해주거나(List<Plan>)

crtEvent

고생하셨어요!

crtEvent · 2024-03-28T04:41:21Z

src/main/java/com/flytrap/venusplanner/api/study/business/service/StudyValid.java

+
+import com.flytrap.venusplanner.api.study.domain.Study;
+
+public interface StudyValid {


저는 StudyValidator라고 명명했어요. 이름 회의 때 이야기해보면 좋을 것 같아요

음 저도 Validator가 좀 더 자연스럽게 읽히는 것 같습니당

crtEvent · 2024-03-28T04:44:09Z

src/main/java/com/flytrap/venusplanner/api/plan/business/service/EndOptionCalculate.java

+
+import java.time.Instant;
+
+public interface EndOptionCalculate {


Calculate가 동사라서 Calculator나 Calculatable? 어떠세요?

crtEvent · 2024-03-28T04:45:05Z

src/main/java/com/flytrap/venusplanner/api/plan/domain/Frequency.java

+    WEEKLY(ChronoUnit.WEEKS) {
+        @Override
+        public Instant calculateEndDate(Instant startDate, int count) {
+            ZonedDateTime zdtStart = DateTimeUtils.toZonedDateTime(startDate);
+            return DateTimeUtils.toInstant(zdtStart.plusWeeks(count - 1));
+        }
+
+        @Override
+        public int calculateCount(Instant startDate, Instant endDate) {
+            ZonedDateTime zdtStart = DateTimeUtils.toZonedDateTime(startDate);
+            ZonedDateTime zdtEnd = DateTimeUtils.toZonedDateTime(endDate);
+            return (int) ChronoUnit.WEEKS.between(zdtStart, zdtEnd) + 1;
+        }
+    },


이 방식 보기 좋아요!

jinny-l

코드에서부터 느껴지는 복잡함.. 너무 고생하셨습니다.

jinny-l · 2024-04-01T03:08:37Z

src/main/java/com/flytrap/venusplanner/api/study/business/service/StudyValid.java

+
+import com.flytrap.venusplanner.api.study.domain.Study;
+
+public interface StudyValid {


음 저도 Validator가 좀 더 자연스럽게 읽히는 것 같습니당

jinny-l · 2024-04-01T03:17:37Z

src/main/java/com/flytrap/venusplanner/api/plan/business/service/PlanService.java

+            Plan newPlan = Plan.builder()
+                    .study(planTemplate.getStudy())
+                    .planCategory(planTemplate.getPlanCategory())
+                    .recurringOption(planTemplate.getRecurringOption())
+                    .title(planTemplate.getTitle())
+                    .description(planTemplate.getDescription())
+                    .startTime(startTime)
+                    .endTime(endTime)
+                    .notificationTime(planTemplate.getNotificationTime())
+                    .build();


서비스 로직 코드 읽는데 빌더가 너무 길어서 Plan에 static으로 메서드 하나 만드는건 어떨까요

jinny-l · 2024-04-01T03:30:12Z

src/main/java/com/flytrap/venusplanner/api/plan/domain/RecurringOption.java

+
+    public void calculate(Instant startDate) {


calculate 메서드명이 좀 더 구체적이면 좋을 것 같아요

반복 옵션이 생성될 때 생성자에서 계산해서 넣어주면 안되나요?? calculate가 있어야 하는 이유가 궁금합니다

jinny-l · 2024-04-01T03:35:44Z

Instant -> ZonedDateTime 변환 유틸 클래스 분리 여부
- (의견) 여러 곳에서 쓰이고 로직이 복잡하면 유틸 클래스로 분리하면 좋을 것 같은데 지금 상황에서는 굳이 분리하지 않아도 될 것 같습니다.
- 그런데 로직에서 + 1, -1 하는게 있는데 주석이나 상수화하면 좀 더 로직을 잘 이해할 수 있을 것 같아요!
@PrePersist 옵션 부작용(?)
- 전체적인 플로우가 복잡해서 왜 문제가 발생했고 무엇을 해결해야 하는지 잘 파악이 안되는 것 같아요

jaea-kim added 9 commits March 28, 2024 04:07

rename: studyPlan 패키지 plan 패키지로 이동

5477cb9

refactor: studyValid 분리

1444372

feat: 종료옵션 관련 계산 인터페이스 및 구현

49eab29

feat: Plan 저장 전 반복옵션 관련 연산 로직 처리

ce80aef

feat: 연,월 연산하도록 시간 타입 변환 로직 추가

76509f5

feat: 주 반복 시간 타입 변환 로직 추가

ef232b3

refactor: 시간 변환 유틸함수 분리

ad6baca

feat: 반복 옵션 전체 삭제 구현

df6fcb3

feat: 반복 수만큼 plan 생성하여 저장 로직 추가

962ddcf

jaea-kim self-assigned this Mar 27, 2024

jaea-kim added the ✨feat 기능 label Mar 27, 2024

jaea-kim linked an issue Mar 27, 2024 that may be closed by this pull request

[feat] 반복 옵션 기능 추가 #29

Open

5 tasks

jaea-kim changed the base branch from release to dev March 27, 2024 23:38

crtEvent reviewed Mar 28, 2024

View reviewed changes

jaea-kim requested a review from jinny-l March 28, 2024 09:55

jinny-l approved these changes Apr 1, 2024

View reviewed changes

jinny-l reviewed Apr 1, 2024

View reviewed changes


		import com.flytrap.venusplanner.api.study.domain.Study;

		public interface StudyValid {


		import java.time.Instant;

		public interface EndOptionCalculate {

[feat] 반복옵션 로직 추가 #34

Are you sure you want to change the base?

[feat] 반복옵션 로직 추가 #34

Uh oh!

Conversation

jaea-kim commented Mar 27, 2024 • edited by jinny-l Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✨ Key changes

👋 To reviewers

반복 횟수, 종료 시간 서버에서 계산해서 넣어주기

Instant -> ZonedDateTime 변환 유틸 클래스 분리 여부 질문

Plan 생성 시 반복옵션 수만큼 추가 생성

횟수 추가(3번) 쿼리

날짜 추가 (다음주 까지 총 2번 반복)

@PrePersist 옵션 부작용(?)

반복옵션만큼 plan 이 생성됐을 때 반환값은 어떻게 할까요?

Plan 삭제 시 전체 삭제일 경우, 관련 반복옵션 모두 삭제

추가 논의

TODO

Uh oh!

crtEvent commented Mar 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Instant -> ZonedDateTime 변환 유틸 클래스 분리 여부 질문

Uh oh!

crtEvent commented Mar 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crtEvent left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jinny-l left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jinny-l commented Apr 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jaea-kim commented Mar 27, 2024 •

edited by jinny-l

Loading

`Instant` -> `ZonedDateTime` 변환 유틸 클래스 분리 여부 질문

crtEvent commented Mar 28, 2024 •

edited

Loading

crtEvent commented Mar 28, 2024 •

edited

Loading

jinny-l left a comment •

edited

Loading

jinny-l commented Apr 1, 2024 •

edited

Loading