Feature/#115 이벤트 cud 기능 구현 #138

hyeonjerry · 2023-07-29T06:08:57Z

#️⃣연관된 이슈

이벤트 CUD 기능 구현 #115

close #115

📝작업 내용

이벤트 Create 기능 구현
이벤트 Update 기능 구현
이벤트 Delete 기능 구현

스크린샷 (선택)

예상 소요 시간 및 실제 소요 시간

6시간 / 8시간

#115

amaran-th

수고하셨습니다~! 코드가 전체적으로 가독성이 좋아서 술술 잘 읽히네요
코멘트 확인 후 반영부탁드립니다~!
미리 approve할게요!

amaran-th · 2023-07-29T06:48:44Z

backend/emm-sale/src/main/java/com/emmsale/event/application/EventService.java

+      throw new EventException(EventExceptionType.FORBIDDEN_PARTICIPATE_EVENT);
+    }
+  }
+


static 메서드라 순서가 바뀐 것 같습니다! 원래 위치로 돌려주세요!

컨벤션이나 기능상 문제가 있는건 아닌데 혹시 되돌려야 하는 이유가 있을까요?

먼저 해당 메서드가 static으로 선언되어야 할 이유가 없는 것 같고, 제가 public 메서드 다음에 private 메서드를 작성하는 것이 코드 흐름을 따라가기 편하다고 생각해서 그렇게 요청 드렸었습니다!

amaran-th · 2023-07-29T06:50:12Z

backend/emm-sale/src/main/java/com/emmsale/event/domain/EventTag.java

@@ -23,11 +23,11 @@ public class EventTag {
  @ManyToOne(fetch = FetchType.LAZY)
  @JoinColumn(name = "event_id", nullable = false)
  private Event event;
-  @ManyToOne(fetch = FetchType.LAZY)
+  @ManyToOne(fetch = FetchType.EAGER)


혹시 fetch 타입을 EAGER로 바꾸신 이유가 무엇인가요?

가장 큰 이유는 EventServiceTest에서 이벤트에 태그가 정상적으로 생성/업데이트 되었는지 확인하기 위함입니다.
EventDetailResponse와 EventResponse를 보았을 때 모두 태그를 반환하고 있어 이벤트와 태그는 항상 같이 조회되므로 사실상 EAGER로 했을 때 불필요한 조회가 일어나지 않을 것으로 예상되어 EAGER로 설정하고 테스트했습니다.

amaran-th · 2023-07-30T11:25:30Z

backend/emm-sale/src/main/java/com/emmsale/event/application/EventService.java

+    }
+
+    return EventDetailResponse.from(event);
+  }


NIT: 이 메서드에서 사용된 saveNewEvent() 메서드와 findAllPersistTagsOrElseThrow() 메서드는 순서 상 addEvent 바로 다음에 오면 코드를 읽기 더 쉬울 것 같습니다!

java-saeng

고생하셨습니다 !!

코멘트에 대해 제리 생각 한번 써주시고 재요청 주세요

java-saeng · 2023-07-31T06:47:12Z

backend/emm-sale/src/main/java/com/emmsale/event/domain/Event.java

@@ -39,7 +41,7 @@ public class Event extends BaseEntity {
  private LocalDateTime endDate;
  @Column(nullable = false)
  private String informationUrl;
-  @OneToMany(mappedBy = "event")
+  @OneToMany(mappedBy = "event", fetch = FetchType.EAGER, cascade = CascadeType.PERSIST)


EAGER를 쓰신 이유가 있을까요?

저도 이 부분은 궁금하네요.

#138 (comment)

java-saeng · 2023-07-31T06:49:34Z

backend/emm-sale/src/main/java/com/emmsale/GlobalControllerAdvice.java

+    final String message = e.getBindingResult().getAllErrors().stream()
+        .map(DefaultMessageSourceResolvable::getDefaultMessage)
+        .collect(Collectors.joining("\n"));
+    log.warn("[WARN] MESSAGE: {}", message);


해당 에러가 "WARN 레벨이 타당할까?"에 대해 한번 생각해봐주시면 좋을 것 같아요

저는 개인적으로 warn 단계 적당하다고 생각합니다.
info라고 하기에는 일반적이지 않은 상황이고,
error라고 하기에는 예측한 에러 상황이니까요.

이에 대해서 한번 이야기를 해볼 수 있을 것 같아요.

저도 warn이 적당하다고 생각했으나 info로 해도 괜찮지 않을까 싶기도 하네요.
어디까질 warn, 어디부턴 info를 나누는 기준이 명확하지 않아 다같이 얘기해보면 좋을 것 같습니다.
혹시 의견 있으시면 남겨주세요.

java-saeng · 2023-07-31T06:49:45Z

backend/emm-sale/src/main/java/com/emmsale/GlobalControllerAdvice.java

+  public ResponseEntity<ExceptionResponse> handleDateTimeParseException(
+      final DateTimeParseException e) {
+    final String message = "DateTime 입력 형식이 올바르지 않습니다.";
+    log.warn("[WARN] MESSAGE: " + message);


위와 마찬가지입니다.

java-saeng · 2023-07-31T06:52:45Z

backend/emm-sale/src/main/java/com/emmsale/event/application/EventService.java

+
+    eventTagRepository.deleteAllByEventId(eventId);
+
+    final Event updatedEvent = event.updateEventContent(request.getName(), request.getLocation(),


매번 존재했던 태그들을 삭제하고 저장하는 방식인가요?

모든 태그들을 저장하는게 List 여서, 굳이 for문을 돌지 않고 바로 addAll 하는게 어떤가 해서요

네, 매번 모든 태그를 지우고 새로 등록합니다.
업데이트가 일어날 때 기존 이벤트에 등록된 태그를 확인해서 중복되는 것은 남겨두고 존재하지 않는 것은 지우고 새로운 것은 등록하는 것 보다 모두 지우고 새로 등록하는게 로직상 복잡도를 낮추고 업데이트가 자주 일어나는 일이 아니므로 괜찮을 것이라 생각하여 위와 같이 구현했습니다.
혹시 이에 대한 경험이 있으시면 공유해 주세요.

addAll 방식으로 리팩터링 완료 했습니다.

hong-sile

태그 추가하는 것이, participnat랑 비슷해서 괜찮았네요.

그 tag 변하는 부분 관련해서 확인을 잘 해주시면 좋을 것 같아요.

hong-sile · 2023-07-31T07:37:46Z

backend/emm-sale/src/main/java/com/emmsale/GlobalControllerAdvice.java

+    final String message = e.getBindingResult().getAllErrors().stream()
+        .map(DefaultMessageSourceResolvable::getDefaultMessage)
+        .collect(Collectors.joining("\n"));
+    log.warn("[WARN] MESSAGE: {}", message);


저는 개인적으로 warn 단계 적당하다고 생각합니다.
info라고 하기에는 일반적이지 않은 상황이고,
error라고 하기에는 예측한 에러 상황이니까요.

이에 대해서 한번 이야기를 해볼 수 있을 것 같아요.

hong-sile · 2023-07-31T07:38:36Z

backend/emm-sale/src/main/java/com/emmsale/event/domain/Event.java

@@ -39,7 +41,7 @@ public class Event extends BaseEntity {
  private LocalDateTime endDate;
  @Column(nullable = false)
  private String informationUrl;
-  @OneToMany(mappedBy = "event")
+  @OneToMany(mappedBy = "event", fetch = FetchType.EAGER, cascade = CascadeType.PERSIST)


저도 이 부분은 궁금하네요.

hong-sile · 2023-07-31T07:39:21Z

backend/emm-sale/src/main/java/com/emmsale/event/domain/Event.java

@@ -53,6 +55,8 @@ public Event(
      final LocalDateTime endDate,
      final String informationUrl
  ) {
+    validateStartBeforeOrEqualEndDateTime(startDate, endDate);


hong-sile · 2023-07-31T07:57:33Z

backend/emm-sale/src/main/java/com/emmsale/event/domain/Event.java

+    this.tags = new ArrayList<>();
+
+    for (final Tag tag : tags) {
+      addEventTag(tag);


혹시 이 부분 N+1문제 안 나나요?
궁금해서요

추가적으로 이런식으로 작성하면 기존에 있는, EventTag들은 테이블에서 다 지워지나요?
안 지워지고 남아있을수도 있을 것 같다는 생각이 들어서요

#138 (comment)

hong-sile · 2023-07-31T07:58:17Z

backend/emm-sale/src/test/java/com/emmsale/event/domain/EventTest.java

+        () -> assertEquals(newStartDateTime, updatedEvent.getStartDate()),
+        () -> assertEquals(newEndDateTime, updatedEvent.getEndDate()),
+        () -> assertEquals(newInformationUrl, updatedEvent.getInformationUrl()),
+        () -> assertEquals(newTags.size(), event.getTags().size())


태그는 사이즈 말고 더 정확한 방식으로 검증하면 어떨까요?

#115

hyeonjerry added 13 commits July 28, 2023 20:54

feat: 행사(Event) 생성 기능 구현

4381081

#115

refactor: EventCreateRequest 이름 수정

9288b16

#115

refactor: getPersistTags 메서드 파라미터 수정

5203ae1

#115

fix: add-event snippet명 수정

9d4823c

#115

feat: event 업데이트 기능 구현

0d9f73d

#115

test: 이벤트 생성 테스트 수정

0a30e2b

#115

refactor: event validate 메서드 Event 도메인 객체 안으로 이동

e642a34

#115

refactor: 메서드명 수정

6ad193c

#115

docs: index.adoc에 snippet 추가

8a4ea2f

#115

feat: 이벤트 삭제 기능 구현

f90c677

#115

test: 이벤트 업데이트시 이벤트가 존재하지 않는 경우 테스트 케이스 추가

ccf846e

#115

test: 테스트 코드 리팩터링

f4edde6

#115

test: Event 시작 및 종료 일시 검증 테스트 케이스 추가

0a2bdd8

#115

hyeonjerry added Backend 백엔드 관련 이슈 기능 추가 새로운 기능 추가 및 기존 기능 변경 Low Priority 리뷰 우선순위가 낮은 PR labels Jul 29, 2023

hyeonjerry added this to the 3차 스프린트 2주차 milestone Jul 29, 2023

hyeonjerry requested review from hong-sile, java-saeng and amaran-th July 29, 2023 06:08

hyeonjerry self-assigned this Jul 29, 2023

amaran-th approved these changes Jul 30, 2023

View reviewed changes

java-saeng requested changes Jul 31, 2023

View reviewed changes

hong-sile reviewed Jul 31, 2023

View reviewed changes

hyeonjerry added 2 commits July 31, 2023 17:37

refactor: 태그들 등록 기능 리팩터링

7ca159c

#115

test: 내부 데이터 검증 테스트 보완

180a12b

#115

hong-sile approved these changes Jul 31, 2023

View reviewed changes

merge: 머지 컨플릭트 해결

bcc8b67

#115

hyeonjerry merged commit 7754fc5 into backend-main Jul 31, 2023
1 check passed

amaran-th deleted the Feature/#115-이벤트_CUD_기능_구현 branch August 4, 2023 00:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/#115 이벤트 cud 기능 구현 #138

Feature/#115 이벤트 cud 기능 구현 #138

hyeonjerry commented Jul 29, 2023

amaran-th left a comment

amaran-th Jul 29, 2023

hyeonjerry Jul 31, 2023

amaran-th Jul 31, 2023

amaran-th Jul 29, 2023 •

edited

Loading

hyeonjerry Jul 31, 2023

amaran-th Jul 30, 2023

java-saeng left a comment

java-saeng Jul 31, 2023

hong-sile Jul 31, 2023

hyeonjerry Jul 31, 2023

java-saeng Jul 31, 2023

hong-sile Jul 31, 2023

hyeonjerry Jul 31, 2023

java-saeng Jul 31, 2023

java-saeng Jul 31, 2023

hyeonjerry Jul 31, 2023

hong-sile left a comment

hong-sile Jul 31, 2023

hong-sile Jul 31, 2023

hong-sile Jul 31, 2023

hong-sile Jul 31, 2023

hong-sile Jul 31, 2023

hyeonjerry Jul 31, 2023

hong-sile Jul 31, 2023


		eventTagRepository.deleteAllByEventId(eventId);

		final Event updatedEvent = event.updateEventContent(request.getName(), request.getLocation(),

Feature/#115 이벤트 cud 기능 구현 #138

Feature/#115 이벤트 cud 기능 구현 #138

Conversation

hyeonjerry commented Jul 29, 2023

#️⃣연관된 이슈

📝작업 내용

스크린샷 (선택)

예상 소요 시간 및 실제 소요 시간

amaran-th 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

amaran-th Jul 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

java-saeng 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

Choose a reason for hiding this comment

hong-sile 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

amaran-th Jul 29, 2023 •

edited

Loading