켈리 1단계 PR 제출합니다.! #7

kelly6bf · 2024-05-02T13:07:46Z

No description provided.

기존 200 응답을 201로 변경함.

leegwichan

켈리! 커찬이야~

시간이 없어서 내 코드와 달랐던 부분 위주로 코멘트 남겨봤어!
질문도 달아놨으니까 여유있을 떄 코멘트 남겨줘!
(마이그레이션 한 코드 빼고 보는 걸 깜박함...)

leegwichan · 2024-05-03T00:17:39Z

docs/README.md

+
+### 테마 전체 조회
+
+```http request
+GET http://localhost:8080/themes
+Content-Type: application/json
+```


API 명세서 👍 👍 👍

기왕 추가할 꺼면, HTTP Response까지 추가했으면 어떨까?

leegwichan · 2024-05-03T00:19:20Z

src/main/java/roomescape/controller/ReservationApiController.java

각 도메인 (예약, 시간, 테마) 별로 Controller를 나누어보는 건 어때?

하나의 파일에서 너무 많은 엔트포인트를 관리하지 않나 싶어

오우 동의합니다

leegwichan · 2024-05-03T00:20:04Z

src/main/java/roomescape/domain/ClientName.java

도메인 객체 👍 👍 👍

leegwichan · 2024-05-03T00:22:15Z

src/main/java/roomescape/domain/ReservationTimeAvailabilities.java

+    public static ReservationTimeAvailabilities of(final List<ReservationTime> reservationTimes, final List<Reservation> reservations) {
+        final Map<ReservationTime, Boolean> reservationTimeAvailabilities = new HashMap<>();
+        reservationTimes.forEach(reservationTime -> reservationTimeAvailabilities.put(
+                reservationTime, isTimeAvailable(reservations, reservationTime)));
+
+        return new ReservationTimeAvailabilities(reservationTimeAvailabilities);
+    }


Suggested change

public static ReservationTimeAvailabilities of(final List<ReservationTime> reservationTimes, final List<Reservation> reservations) {

final Map<ReservationTime, Boolean> reservationTimeAvailabilities = new HashMap<>();

reservationTimes.forEach(reservationTime -> reservationTimeAvailabilities.put(

reservationTime, isTimeAvailable(reservations, reservationTime)));

return new ReservationTimeAvailabilities(reservationTimeAvailabilities);

}

public static ReservationTimeAvailabilities of(final List<ReservationTime> reservationTimes, final List<Reservation> reservations) {

final Map<ReservationTime, Boolean> reservationTimeAvailabilities = new HashMap<>();

reservationTimes.forEach(reservationTime ->

reservationTimeAvailabilities.put(reservationTime, isTimeAvailable(reservations, reservationTime)));

return new ReservationTimeAvailabilities(reservationTimeAvailabilities);

}

이게 가독성이 더 좋지 않을까 싶어.

이것도 동의합니다

leegwichan · 2024-05-03T00:23:29Z

src/main/java/roomescape/domain/Theme.java

+    public static Theme of(final String name, final String description, final String thumbnail) {
+        return new Theme(null, new ThemeName(name), new ThemeDescription(description), thumbnail);
+    }
+
+    public static Theme of(final Long id, final String name, final String description, final String thumbnail) {
+        return new Theme(id, new ThemeName(name), new ThemeDescription(description), thumbnail);
+    }


인자에만 final을 사용한 이유는? (여기도 final 광인인가요?)

leegwichan · 2024-05-03T00:26:05Z

src/main/java/roomescape/exception/ReservationExceptionHandler.java

+    @ExceptionHandler
+    protected ResponseEntity<ErrorResponse> handleIllegalArgumentException(IllegalArgumentException e) {
+        return ResponseEntity.badRequest().body(new ErrorResponse(e.getMessage()));
+    }
+
+    @ExceptionHandler
+    protected ResponseEntity<ErrorResponse> handleNoSuchElementException(NoSuchElementException e) {
+        return ResponseEntity.badRequest().body(new ErrorResponse(e.getMessage()));
+    }


접근 제어자가 protected 인 이유는?

아래처럼 묶어서 사용해도 되지 않나요?

Suggested change

@ExceptionHandler

protected ResponseEntity<ErrorResponse> handleIllegalArgumentException(IllegalArgumentException e) {

return ResponseEntity.badRequest().body(new ErrorResponse(e.getMessage()));

}

@ExceptionHandler

protected ResponseEntity<ErrorResponse> handleNoSuchElementException(NoSuchElementException e) {

return ResponseEntity.badRequest().body(new ErrorResponse(e.getMessage()));

}

@ExceptionHandler({IllegalArgumentException.class, NoSuchElementException.class})

protected ResponseEntity<ErrorResponse> handleIllegalArgumentException(RuntimeException e) {

return ResponseEntity.badRequest().body(new ErrorResponse(e.getMessage()));

}

leegwichan · 2024-05-03T00:26:55Z

src/main/java/roomescape/repository/H2ReservationRepository.java

+        final String sql = "SELECT "
+                + "r.id as reservation_id, r.name as reservation_name, r.date as reservation_date, "
+                + "rt.id as time_id, rt.start_at as reservation_time, "
+                + "th.id as theme_id, th.name as theme_name, th.description as theme_description, th.thumbnail as theme_thumbnail "
+                + "FROM reservation as r "
+                + "inner join reservation_time as rt "
+                + "on r.time_id = rt.id "
+                + "inner join theme as th "
+                + "on r.theme_id = th.id";


FROM 만 대문자인거 불~편

SELECT도 대문자 👀

leegwichan · 2024-05-03T00:28:41Z

src/test/resources/schema.sql

각 테스트가 동일한 데이터를 사용하게 된다면, 모든 테스트가 schema.sql에 종속되겠네요?
이것에 대해서는 어떻게 생각해?

leegwichan · 2024-05-03T00:42:39Z

src/main/resources/templates/index.html

-      <!--        <a class="nav-link dropdown-toggle" href="#" id="navbarDropdown" role="button" data-toggle="dropdown"-->
+      <!--        <a class="nav-link dropdown-toggle" href="#" themeId="navbarDropdown" role="button" data-toggle="dropdown"-->
      <!--           aria-haspopup="true" aria-expanded="false">-->
      <!--          <img class="profile-image" src="/image/default-profile.png" alt="Profile">-->
-      <!--          <span id="profile-name">Profile</span> &lt;!&ndash; 프로필 이름을 넣을 span 추가 &ndash;&gt;-->
+      <!--          <span themeId="profile-name">Profile</span> &lt;!&ndash; 프로필 이름을 넣을 span 추가 &ndash;&gt;-->
      <!--        </a>-->
      <!--        <div class="dropdown-menu" aria-labelledby="navbarDropdown">-->
      <!--          <a class="dropdown-item" href="/reservation-mine">My Reservation</a>-->
      <!--          <div class="dropdown-divider"></div>-->
-      <!--          <a class="dropdown-item" href="#" id="logout-btn">Logout</a>-->
+      <!--          <a class="dropdown-item" href="#" themeId="logout-btn">Logout</a>-->


HTML에서 id가 뭐하는 것인지 한번 알아봐야 할것 같은데?

leegwichan · 2024-05-03T00:43:25Z

src/main/java/roomescape/repository/ReservationRepository.java

+public interface ReservationRepository {
+    List<Reservation> findAll();
+
+    Optional<Reservation> findById(Long reservationId);
+
+    Reservation save(Reservation reservation);
+
+    void deleteById(Long reservationId);
+
+    boolean existByDateAndTimeIdAndThemeId(LocalDate date, Long timeId, Long themeId);
+
+    boolean existByTimeId(Long reservationTimeId);
+
+    List<Reservation> findAllByDateAndThemeId(LocalDate date, Long themeId);
+}


Interface를 도입한 근거는? 구현체는 H2밖에 없는뎁

hyxrxn

잘 읽었습니닷
다음 단계도 화이팅!

hyxrxn · 2024-05-03T06:16:52Z

docs/README.md

+- [X] 기존 API 입력값 검증 기능
+- [X] 유저 페이지 라우팅 기능
+- [X] 테마 도메인 추가
+    - [X] 테마 전체 조회 기능
+    - [X] 테마 추가 기능
+    - [X] 테마 삭제 기능
+- [X] 사용자 예약 가능 시간 조회 기능
+- [X] 사용자 페이지 방탈출 예약 기능
+- [X] 인기 테마 조회 기능


이번 미션 1단계가 예외 처리에 관한 내용이었으니 어떨 때 예외라고 생각했는지 함께 정리해주면 더 알아보기 좋을 것 같아요!

hyxrxn · 2024-05-03T06:20:04Z

src/main/java/roomescape/controller/ReservationApiController.java

+        return ResponseEntity.created(URI.create("/reservations/" + savedReservation.getId()))
+                .body(ReservationResponse.from(savedReservation));


로빈에게도 남긴 질문인데....

지금은 바디만 필요한 상태인데 URI까지 담아서 리턴하는 이유가 있을까요?

@ResponseStatus(HttpStatus.CREATED) 를 사용하면 ResponseEntity 없이도 201을 넘길 수 있답니닷...!
저는 이번에 알아서 이걸 사용해봤는데... 켈리는 알고 있지만 안썼을 수도 있겠다 싶어서 궁금하네용

hyxrxn · 2024-05-03T06:20:40Z

src/main/java/roomescape/controller/ReservationApiController.java

오우 동의합니다

hyxrxn · 2024-05-03T06:27:20Z

src/main/java/roomescape/service/ReservationService.java

+    public Theme saveTheme(final SaveThemeRequest saveThemeRequest) {
+        return themeRepository.save(saveThemeRequest.toTheme());
+    }


저는 request를 받는 만큼 response를 돌려주는게 서비스의 역할이라고 생각해요.
켈리는 도메인을 리턴하고 컨트롤러가 response로 변환하는 역할을 가지고 있는 것 같은데, 따로 이유가 있나요?

hyxrxn · 2024-05-03T06:32:47Z

src/main/java/roomescape/service/ReservationService.java

+    public void deleteTheme(final Long themeId) {
+        validateThemeExist(themeId);
+
+        themeRepository.deleteById(themeId);
+    }


예약이 존재하는 시간을 삭제하려고 하면 안내 메시지가 오는데요, 예약이 존재하는 테마를 삭제하려고 하면 그냥 404인 것 같네요

hyxrxn · 2024-05-03T06:38:25Z

src/main/java/roomescape/domain/ReservationTimeAvailabilities.java

+
+    private final Map<ReservationTime, Boolean> values;
+
+    public static ReservationTimeAvailabilities of(final List<ReservationTime> reservationTimes, final List<Reservation> reservations) {


이 한 줄이... 너무 길어요 ㅠ

hyxrxn · 2024-05-03T06:39:26Z

src/main/java/roomescape/domain/ReservationTimeAvailabilities.java

+    public static ReservationTimeAvailabilities of(final List<ReservationTime> reservationTimes, final List<Reservation> reservations) {
+        final Map<ReservationTime, Boolean> reservationTimeAvailabilities = new HashMap<>();
+        reservationTimes.forEach(reservationTime -> reservationTimeAvailabilities.put(
+                reservationTime, isTimeAvailable(reservations, reservationTime)));
+
+        return new ReservationTimeAvailabilities(reservationTimeAvailabilities);
+    }


이것도 동의합니다

hyxrxn · 2024-05-03T06:45:51Z

src/main/java/roomescape/domain/ThemeDescription.java

+    private void validateValue(final String value) {
+        if (value == null || value.isEmpty() || value.length() > MAXIMUM_ENABLE_NAME_LENGTH) {
+            throw new IllegalArgumentException("테마 설명은 1글자 이상 80글자 이하여야 합니다.");
+        }
+    }


설명 검증을 한 것은 좋은데요....!
isBlank가 아니라 isEmpty여서...
테마 제목과 설명과 썸네일에 띄어쓰기만 입력해도 등록이 되네요 ㅠ
의도한 것은 아니겠죠...?!

hyxrxn · 2024-05-03T06:47:43Z

src/main/java/roomescape/repository/H2ReservationTimeRepository.java

+        final MapSqlParameterSource param = new MapSqlParameterSource()
+                .addValue("reservationTimeId", reservationTimeId);
+
+        return Boolean.TRUE.equals(template.queryForObject(sql, param, Boolean.class));


이런 비교 방식이 있군요!

hyxrxn · 2024-05-03T06:51:29Z

src/main/resources/schema.sql

+INSERT INTO theme(name, description, thumbnail)
+VALUES ('테바와 비밀친구', '나랑.. 비밀친구할래..?', '테바 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('켈리의 댄스교실', '켈켈켈켈켈', '켈리 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('우테코 탈출하기', '우테코를... 탈출..하자..!', '우테코 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('네오의 두근두근 피드백 강의', '??? : 네오가 setter 쓰라고 했는데요?', '분노하는 네오 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('리사의 소프트 교육', '에궁..ㅜㅜ', '공감하는 리사 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('신천직화 탈출하기', '저희 3인분 시켰는데 왜 계란말이 안주세요?', '제육 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('장미상가 탈출하기', '여기 A동 아니에요?', '장미상가 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('페드로의 주먹', '페급~ (페드로 급이라는 뜻~)', '페드로 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('사물함 탈취하기', '니 사물함 쩔더라 ㅋ', '사물함 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('아루의 입기타', '뚜루루루룰루루루룰', '아루 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('이든의 프로틴 쉐이크 제작 강의', '내 단백질 어디있죠?', '이든 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('솔라의 솔라빔', '자라나라 머리머리!', '솔라 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('포비의 긴급 포수타', '포비는 크루들에게 실망했다.', '포비 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('브리와 솔라의 페어프로그래밍 연극', '... 이거 이렇게 짜는거 맞아요?', '훈훈한 브리와 솔라 사진');
+INSERT INTO theme(name, description, thumbnail)
+VALUES ('레디의 코드리뷰', '아씨 깜짝아! 내 코드인줄 알았네 (제제의 코드를 보며)', '레디 사진');


ㅋㅋㅋㅋㅋㅋㅋㅋㅋ 실행해보고 재밌었어요

아니 레디의 코드리뷰 뭔데 ㅋㅋㅋㅋ

기존 삭제전 데이터 존재 여부를 조회하는 로직을 제거하고, 삭제된 데이터의 개수를 토대로 예외를 발생시키도록 로직을 수정함.

kelly6bf added 30 commits April 30, 2024 14:40

chore: 관리자 프로젝트 마이그레이션

f54ef96

feat: 예약시간 중복 검증 추가

7b13afc

refactor: 예약 시간 중복 검증 방식을 단순 조회 방식으로 변경

3802ff0

config: 테스트 데이터 추가 쿼리 작성

b9057be

refactor: 예약 중복 검증시 날짜와 시간을 함께 검사하도록 수정

0271e2c

feat: 예약시간을 참조하는 예약 검증 추가

9b1bf5c

feat: 날짜 공백 검증 로직 추가

b70fded

refactor: 생성 API HTTP 상태 코드 변경

61c0499

기존 200 응답을 201로 변경함.

config: schema.sql 테마 테이블 관련 쿼리문 추가

a011a70

feat: 테마 관련 어드민 페이지 라우팅 추가

f27af3e

feat: Theme 도메인 객체 생성

fc3a74a

feat: ThemeRepository 생성 및 아이디 기반 조회 기능 구현

db57aad

refactor: 기존 코드에 새로 추가된 Theme 도메인 로직 적용

65ee3d6

test: 테스트용 쿼리 수정

1709055

test: Theme 도메인 추가로 인한 테스트 변경 적용

724646a

feat: ThemeResponse 객체 정의 및 ReservationResponse에 적용

0c5681a

feat: 테마 전체 조회 기능 구현

b87dd23

feat: 테마 저장 기능 구현

1ee0a60

feat: 테마 삭제 기능 구현

18d919a

test: 테스트 쿼리문 수정

79ad47d

refactor: delete 쿼리 응답코드 수정

f2122c5

feat: 유저 페이지 접근 라우팅 구현

36dc83c

refactor: 3단계 요구사항에 맞춰 클라이언트 코드 수정

2095516

feat: 예약 가능 시간 조회 & 인기 테마 조회 기능 구현

aafe8e4

config: schema.sql 현재를 기점으로 시간 설정 쿼리 추가

455b3a2

refactor: 도메인 패키지 코드 개선

c65f30e

test: 도메인 패키지 테스트 코드 개선

b2d63d1

refactor: DTO 로직 및 테스트 개선

f9ac2bc

refactor: ThemeName의 최대 입력 길이를 20으로 변경

278b87d

refactor: Repository 레이어 로직 및 테스트 개선

9ba3943

docs: README.md 생성

cdd9ae7

leegwichan reviewed May 3, 2024

View reviewed changes

hyxrxn reviewed May 3, 2024

View reviewed changes

kelly6bf added 27 commits May 4, 2024 21:28

refactor: ReservationApiController 도메인별 분리

206eaea

refactor: id를 인자로 받지 않는 생성 메서드 이름 변경

0a8070d

refactor: ReservationTime equals & hashcode 비교 인자 추가

bdab964

refactor: SQL 문법을 """""로 정리

cb84783

test: test 데이터 스크립트 분리

29dac81

test: DML Script 파일명 변경

f32c5ed

config: DML 스크립트 파일명 변경

d32b136

style: 완료된 TODO 주석 제거

7c94cd2

refactor: 존재하지 않는 아이디의 데이터 삭제 로직 수정

ab9eeae

기존 삭제전 데이터 존재 여부를 조회하는 로직을 제거하고, 삭제된 데이터의 개수를 토대로 예외를 발생시키도록 로직을 수정함.

refactor: Reservation 생성 메서드의 이름을 좀 더 명확하게 변경

4d54a95

refactor: Reservation null 검증 코드 추가

7313318

fix: Reservation 코드 누락 정정

6154be5

refactor: 인메모리를 사용하는 Repository 구현체들의 클래스명 변경

3796648

refactor: 기존 ReservationService를 도메인별로 분리

80e8f93

fix: 잘못 변경된 repository 클래스명 정정

472f30c

refactor: 테마 정보 삭제시 해당 테마 정보를 참조하는 예약이 있는지 검증하는 로직 추가

05436e2

remove: 사용하지 않는 메서드 제거

6c3d2fc

test: 요청 데이터 생성 방식을 DTO로 변경

feb0a95

refactor: " "과 같은 공백 검증 기능 추가 및 테스트가 경계값을 검사하도록 수정

0915051

refactor: ThemeThumbnail 분리 및 null 검사 로직 추가

734ae04

refactor: 예약 생성시 현재보다 과거 날짜/시간인지 검증하는 로직을 서비스로 위임

22e8237

refactor: 불변 객체 record 변환 및 필요한 클래스에 equals & hashcode 재정의

e9ded6d

feat: Exception Handler 추가

3cbfafa

config: 스키마 데이터 크기 및 타입 정정

bc66106

chore: Exception Handler InternalServerError 추가

8068c0e

refactor: 도메인 일관성을 위해 null 검증 로직 제거

b108cae

test: 유닛 & 슬라이스 테스트 제거

06785a1

		return ResponseEntity.created(URI.create("/reservations/" + savedReservation.getId()))
		.body(ReservationResponse.from(savedReservation));


		private final Map<ReservationTime, Boolean> values;

		public static ReservationTimeAvailabilities of(final List<ReservationTime> reservationTimes, final List<Reservation> reservations) {

켈리 1단계 PR 제출합니다.! #7

Are you sure you want to change the base?

켈리 1단계 PR 제출합니다.! #7

Uh oh!

Conversation

kelly6bf commented May 2, 2024

Uh oh!

leegwichan 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!

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!

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!

hyxrxn 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

leegwichan left a comment •

edited

Loading