[FEAT] 주제, 선택지 조회 기능 구현 #36

leegwichan · 2024-07-19T01:01:29Z

Issue Number

#28

As-Is

방에 진행 중인(가장 최신의) 주제, 선택지 조회 기능 필요

To-Be

방에 진행 중인(가장 최신의) 주제, 선택지 조회 기능 구현

Check List

테스트가 전부 통과되었나요?
모든 commit이 push 되었나요?
merge할 branch를 확인했나요?
Assignee를 지정했나요?
Label을 지정했나요?

Test Screenshot

(Optional) Additional Description

백엔드 논의 짬통 확인 부탁해요~

서비스 계층 구현시 사용할 레포지토리 구현

leegwichan · 2024-07-19T01:05:44Z

리뷰 중점 사항

컨벤션이 잘 지켜졌나요?
추후 리팩토링 해야 할 사항은 무엇이 있을까요?
- Exception 관리 방법, 패키지 구조 관리, AuditingEntity, ...
- BaseXXXTest 관리, 테스트 데이터 관리, ...

leegwichan · 2024-07-19T04:37:47Z

application-local.yml

spring:
  h2:
    console:
      enabled: true
      path: /h2-console
  datasource:
    driver-class-name: org.h2.Driver
    url: jdbc:h2:mem:db;MODE=MYSQL
    username: sa
    password:
  jpa:
    generate-ddl: 'true'
    hibernate:
      ddl-auto: create
    properties:
      hibernate:
        show_sql: true
        format_sql: true
        highlight_sql: true
        use_sql_comments: true
    database-platform: org.hibernate.dialect.H2Dialect

logging:
  level:
    org:
      hibernate:
        orm:
          jdbc:
            bind: TRACE
      springframework:
        transaction:
          interceptor: TRACE

backend/src/test/java/ddangkong/controller/BaseControllerTest.java

leegwichan · 2024-07-20T02:36:49Z

TODO List

도메인
- AuditingEntity → BaseEntity로 이름 변경
- repository를 도메인 패키지 안으로 이동
- Repository 애노테이션 제거
- Lazy Loading을 했으므로 @ToString, @EqualsAndHashCode 제거
exception handler
- violateDataException → log.error로 변경
- exception을 아키텍쳐 별로 두도록 변경
테스트
- RestAssured 5.3.1로 명시되어 있는데, latest version인지 확인
- @SQL → executionPhase default 값 제거
- BaseServiceTest → RandomPort 빼기
- init-test.sql → insert query 하나로
- 테스트 케이스에서 ‘똥’ 빼주세요. 더러워요.
참고 문서 : https://www.notion.so/ghenmaru/be52b0047aa0416886b77f76199db2f0

init-test.sql에서 데이터 입력 시 한 번에 입력하도록 수정

jhon3242

코드리뷰 빠르게 반영한 커찬 칭찬해~

GIVEN53

빠른 반영 👍🏼
어제 논의했던 클래스명, 필드명 수정도 반영해주시면 감사하겠습니다!
이에 따라 dto 필드명도 몇 가지 수정이 생길 것 같아요 함께 확인 부탁드려요

approve 하겠습니당

- BalanceQuestion -> BalanceContent로 변경 - 필드명 content, title 등을 name으로 바꿈

leegwichan · 2024-07-20T16:09:18Z

변경 사항

구분하기 쉽게 Table, Column 명 변경
- 회의 결과에 따라 변경함
테이블 명, 필드 명 변경 사항
- BalanceQuestion -> BalanceContent
  - content -> name
- BalanceOption
  - content -> name
- RoomQuestion -> RoomContent
이와 관련된 Service, Repository 이름도 변경
- BalanceQuestionService -> BalanceContentService, ...

기타 사항

작업 관련
- 자질구레한 변경이나, 오타로서 놓친 부분은 바로 반영하도록 하겠습니다. 피드백 부탁드립니다~
- 추가적인 내용의 변경사항은 REFACTOR, FIX 형태로 새로 브랜치 타서 작업하겠습니다~ (PR단위가 계속 커지는 느낌이 들어서;;)
API 명세 관련
- Table, Column 이름은 바꾸었는데, API 명세는 그대로입니다.
- 일단 BE 논의 짬통에 넣어두었습니다~!

jhon3242

항상 열심히 하는 커찬 칭찬해~

backend/src/main/java/ddangkong/domain/content/RoomContentRepository.java

backend/src/main/java/ddangkong/controller/content/dto/BalanceContentResponse.java

backend/src/main/java/ddangkong/controller/exception/GlobalExceptionHandler.java

backend/src/main/java/ddangkong/controller/option/dto/BalanceOptionResponse.java

PgmJun

커찬 고생하셨숩니다! 질문이나 의견 남겨두었는데 확인 부탁드려요!

backend/src/main/java/ddangkong/controller/content/dto/BalanceContentResponse.java

PgmJun · 2024-07-21T11:29:37Z

backend/src/main/java/ddangkong/controller/exception/GlobalExceptionHandler.java

    @ExceptionHandler
    public ErrorResponse handleBindingException(BindException e) {


의견) 여기 ResponseStatus가 안되어 있는 것 같네요! 확인 부탁드려요!

backend/src/main/java/ddangkong/domain/BaseEntity.java

PgmJun · 2024-07-21T11:56:11Z

backend/src/main/java/ddangkong/controller/content/dto/BalanceContentResponse.java

@@ -0,0 +1,27 @@
+package ddangkong.controller.content.dto;


의견) BalanceOption, BalanceContent 라는 도메인 네이밍이지만 패키지는 그냥 content 인 것이 괜찮을 지 고민이 되네요
저희가 밸런스 게임 제공 서비스가 아니라 대화주제 제공 서비스이며, 밸런스 게임은 그 중 하나의 도메인일 뿐이니 이런식으로 두는 건 어떨까에 대해 같이 고민해보면 좋을 것 같아요!
depth가 깊어지지만 알아보기는 더 쉬울 것 같기도 합니다:)

controller/ │ ├── balance/ │ │ ├── option/ │ │ │ └── BalanceOptionController.java │ │ └── question/ │ │ └── BalanceQuestionController.java

7/22 회의 결과, balance package 추가하도록 하겠습니다~

backend/src/main/java/ddangkong/controller/content/dto/BalanceContentResponse.java

backend/src/test/resources/init-test.sql

PgmJun · 2024-07-21T12:31:36Z

backend/src/test/java/ddangkong/controller/content/BalanceContentControllerTest.java

+    private static final BalanceContentResponse EXPECTED_RESPONSE = new BalanceContentResponse(
+            1L, Category.EXAMPLE, "민초 vs 반민초",
+            new BalanceOptionResponse(1L, "민초"),
+            new BalanceOptionResponse(2L, "반민초"));


의견) 특정 기능에서 필요한 클래스 변수는 Nested Class 내부에 넣어두는 것이 어떨까요?

PgmJun · 2024-07-21T12:37:10Z

backend/src/test/java/ddangkong/controller/content/BalanceContentControllerTest.java

+            new BalanceOptionResponse(2L, "반민초"));
+
+    @Nested
+    class 방의_질문_조회 {


사소한 의견) 기능에 대한 설명을 명확히 하기위해 방의_현재_질문_조회 로 변경하는 것은 어떨까요?

PgmJun · 2024-07-21T12:38:58Z

backend/src/test/java/ddangkong/domain/content/RoomContentRepositoryTest.java

+        void 방의_가장_최신의_질문을_조회할_수_있다() {
+            RoomContent actual = roomContentRepository.findTopByRoomIdOrderByCreatedAtDesc(1L).get();
+
+            assertThat(actual.getId()).isEqualTo(2L);
+        }


사소한 의견) Service 로직에 given, when, then 을 사용해서 테스트에 대한 이해도를 높여주면 어떨까 싶습니다~!
테스트 코드는 기능에 대한 문서라고 생각하기 잘 정의해두면 좋을 것 같아서 때문에 제안드려봤어요!

PgmJun · 2024-07-21T12:40:00Z

backend/src/test/java/ddangkong/controller/content/BalanceContentControllerTest.java

+        @Test
+        void 현재_방의_질문을_조회할_수_있다() {
+            BalanceContentResponse actual = RestAssured.given().log().all()
+                    .when().get("/api/balances/rooms/1/question")
+                    .then().log().all()
+                    .statusCode(200)
+                    .extract().as(BalanceContentResponse.class);
+
+            assertThat(actual).isEqualTo(EXPECTED_RESPONSE);
+        }
+    }


의견) 방에 질문이 없다면 예외를 발생시키거나, 방이 존재하지 않으면 예외를 발생시키는 부분도 같이 테스트해주면 좋을 것 같습니다!

컨트롤러 레이어에서는 해피케이스, Request와 관련된 내용만 테스트하고 나머지 예외 상황은 서비스 레이어에서 테스트 했습니다.

현 상황에서 컨트롤러는 서비스가 예외를 일으키는지 알 수 없으니까, 예외와 관련된 테스트는 서비스 레이어만으로 충분하다고 생각합니다. 만약 예외 상황에 대한 테스트가 필요하다고 생각된다면, GlobalExceptionHandlerTest를 만들도록 하겠습니다~

leegwichan · 2024-07-22T08:19:18Z

TODO

비즈니스 코드
- RoomContentRepository를 RoomContent와 동일한 패키지로 변경
- 서버 에러 시, 에러 스택 트레이스까지 출력하도록 변경 (error 상황 한정)
- GlobalExceptionHandler에서 누락된 @ResponseStatus 추가
- 각 계층별로 balance package 추가 (domain.content -> domain.balance.content)
- 변경된 API 적용 (일부 json key 값)
- Validation 적용 (@Positive)
테스트 코드
- 특정 기능에서 필요한 클래스 변수는 Nested Class 내부로 이동
- 방의_질문_조회에서 방의_현재_질문_조회로 변경
- given, when, then 주석 추가

GIVEN53

LGTM 🤗🤗

PgmJun

LGTM🔥
네이밍관련 사소한 의견 한 가지 남겨두었으니 확인부탁드려요!

PgmJun · 2024-07-22T13:33:37Z

backend/src/test/java/ddangkong/service/balance/content/BalanceContentServiceTest.java

+    private BalanceContentService balanceContentService;
+
+    @Nested
+    class 현재_방의_내용_조회 {


사소한 의견) 방의 내용이라기 보단, 현재_방의_밸런스_게임_내용_조회가 BalanceContent라는 도메인에 적합한 테스트 클래스명이지 않을까 싶습니다:)

jhon3242

커칭커칭~

테스트가 어떤 도메인인지 드러나도록 변경

leegwichan added 5 commits July 19, 2024 09:47

feat: RoomQuestion에 생성 일자 추가 #28

009f49d

feat: RoomQuestion, BalanceOption의 Repository 구현 #28

5d970fb

서비스 계층 구현시 사용할 레포지토리 구현

feat: 방 질문 조회 서버스 계층 구현 #28

c4ae03e

chore: 컨트롤러 테스트를 위해 Rest-Assured 의존성 추가 #28

c1867d3

feat: 방 질문 조회 API 구현 #28

81b9d14

leegwichan added ✨ feat 기능 추가 🍃 BE back end labels Jul 19, 2024

leegwichan requested review from jhon3242, PgmJun and GIVEN53 July 19, 2024 01:01

leegwichan self-assigned this Jul 19, 2024

leegwichan changed the title ~~[FEAT] 주제, 선택지 조회 디능 구현~~ [FEAT] 주제, 선택지 조회 기능 구현 Jul 19, 2024

PgmJun reviewed Jul 19, 2024

View reviewed changes

backend/src/test/java/ddangkong/controller/BaseControllerTest.java Outdated Show resolved Hide resolved

leegwichan added 9 commits July 20, 2024 14:45

build: rest-assured을 최신 버전(5.5.0)으로 변경 #28

62a14a9

test: 테스트 케이스 일부 변경 및 추가 #28

0a4501b

init-test.sql에서 데이터 입력 시 한 번에 입력하도록 수정

test: 일부 어노테이션의 기본값 명시한 것 제거, ServiceTest에서 Mock 환경 사용 #28

da4ed13

style: test 파일 개행 및 들여쓰기 맞춤 #28

f57c30c

refactor: 용도를 나타내기 위해 AuditingEntity 에서 BaseEntity로 파일 이름 변경 #28

8566753

refactor: Repository 를 domain 패키지로 이동 및 필요없는 에노테이션 제거 #28

935ff1e

refactor: 지연 로딩의 여파를 막기 위해, 엔티티의 @EqualsAndHashCode, @tostring 제거 #28

458cd54

fix: 서버 문제 트래킹을 명확히 하기 위해, ViolateDataException을 error 수준으로 로깅함 #28

de80d87

refactor: 예외를 아키텍쳐 단위의 패키지로 이동 #28

31e3543

jhon3242 previously approved these changes Jul 20, 2024

View reviewed changes

GIVEN53 previously approved these changes Jul 20, 2024

View reviewed changes

refactor: 구분을 쉽게 하기 위해 일부 테이블 및 컬럼 이름 변경 #28

06b3dba

- BalanceQuestion -> BalanceContent로 변경 - 필드명 content, title 등을 name으로 바꿈

leegwichan dismissed stale reviews from GIVEN53 and jhon3242 via 06b3dba July 20, 2024 15:59

jhon3242 reviewed Jul 21, 2024

View reviewed changes

PgmJun reviewed Jul 21, 2024

View reviewed changes

fix: RoomContentRepository를 room 패키지로 변경 #28

9e061d6

leegwichan added 6 commits July 22, 2024 17:33

fix: 서버 에러 시, 에러 스택 트레이스까지 출력하도록 변경, 일부 @ResponseStatus 추가 #28

246538a

refactor: balance 패키지 추가 #28

b042859

fix: API 명세서를 반영하여 Response 수정 #28

3d5b963

feat: BalanceContentController roomId 유효성 검사 추가

112e91b

test: 테스트를 명확하게 설명하기 위해, Nested class 이름 수정

39691e1

test: given, when, then 주석 추가 #28

b7ba2f4

GIVEN53 previously approved these changes Jul 22, 2024

View reviewed changes

PgmJun previously approved these changes Jul 22, 2024

View reviewed changes

jhon3242 previously approved these changes Jul 22, 2024

View reviewed changes

test: 테스트 클래스 변경 #28

2cc2472

테스트가 어떤 도메인인지 드러나도록 변경

leegwichan dismissed stale reviews from jhon3242, PgmJun, and GIVEN53 via 2cc2472 July 22, 2024 14:32

GIVEN53 merged commit 3041d4a into develop Jul 22, 2024
1 check passed

leegwichan mentioned this pull request Jul 22, 2024

주제, 선택지 조회 기능을 구현한다. #28

Closed

leegwichan deleted the feat/#28 branch December 9, 2024 06:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEAT] 주제, 선택지 조회 기능 구현 #36

[FEAT] 주제, 선택지 조회 기능 구현 #36

leegwichan commented Jul 19, 2024

leegwichan commented Jul 19, 2024

leegwichan commented Jul 19, 2024

leegwichan commented Jul 20, 2024 •

edited

Loading

jhon3242 left a comment

GIVEN53 left a comment

leegwichan commented Jul 20, 2024

jhon3242 left a comment

PgmJun left a comment

PgmJun Jul 21, 2024

PgmJun Jul 21, 2024

leegwichan Jul 22, 2024

PgmJun Jul 21, 2024

PgmJun Jul 21, 2024

PgmJun Jul 21, 2024

PgmJun Jul 21, 2024

leegwichan Jul 22, 2024

leegwichan commented Jul 22, 2024 •

edited

Loading

GIVEN53 left a comment

PgmJun left a comment

PgmJun Jul 22, 2024

jhon3242 left a comment

		@ExceptionHandler
		public ErrorResponse handleBindingException(BindException e) {

[FEAT] 주제, 선택지 조회 기능 구현 #36

[FEAT] 주제, 선택지 조회 기능 구현 #36

Conversation

leegwichan commented Jul 19, 2024

Issue Number

As-Is

To-Be

Check List

Test Screenshot

(Optional) Additional Description

leegwichan commented Jul 19, 2024

리뷰 중점 사항

leegwichan commented Jul 19, 2024

leegwichan commented Jul 20, 2024 • edited Loading

TODO List

jhon3242 left a comment

Choose a reason for hiding this comment

GIVEN53 left a comment

Choose a reason for hiding this comment

leegwichan commented Jul 20, 2024

변경 사항

기타 사항

jhon3242 left a comment

Choose a reason for hiding this comment

PgmJun 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

leegwichan commented Jul 22, 2024 • edited Loading

TODO

GIVEN53 left a comment

Choose a reason for hiding this comment

PgmJun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhon3242 left a comment

Choose a reason for hiding this comment

leegwichan commented Jul 20, 2024 •

edited

Loading

leegwichan commented Jul 22, 2024 •

edited

Loading