[FEAT] 멤버 관련 API 구현 #34

leegwichan · 2024-12-24T11:07:39Z

🚩 연관 이슈

closed #33

🗣️ 리뷰 요구사항 (선택)

테스트 내 스타일로 작성했는데, 불편한 거 있으면 언급 바람
member 패키지 만들었습니다. (dto 패키지 만들어야 하기도 하고, 그냥 따로 패키지 분리해도 무리 없다고 판단했음)
to.비토 : 토론 테이블 이름에 띄어쓰기도 가능하도록 수정했습니다. 확인 바람
to.콜리 : Swagger 잘 썼는지 체크 좀

github-actions · 2024-12-24T11:08:58Z

Test Results

24 tests 24 ✅ 2s ⏱️
10 suites 0 💤
10 files 0 ❌

Results for commit beb68e0.

♻️ This comment has been updated with latest results.

github-actions · 2024-12-24T11:09:00Z

📝 Test Coverage Report

Overall Project	87.45% `-1.85%`	🍏
Files changed	90.74%	🍏

File	Coverage
Member.java	100%	🍏
MemberService.java	100%	🍏
ParliamentaryTable.java	100%	🍏
MemberController.java	100%	🍏
MemberRepository.java	58.33% `-41.67%`	❌
ParliamentaryTimeBox.java	40%	🍏

unifolio0 · 2024-12-24T11:37:18Z

/noti
@leegwichan 리뷰를 하려다가 이 부분은 한번 고민하는게 좋을 것 같아 코멘트 먼저 달아요. 혹시 계층구조에서 하위 계층이 상위 계층을 알고있는 것에 대해 어떻게 생각하시나요? 지금 코드에서 import 를 보면 서비스 계층에서 컨트롤러 계층의 클래스를 import 하고 있어요. 물론 기능상으로, 이후 확장면에서도 dto기 때문에 문제가 없겠지만 좀 어색한 부분 같아요.

coli-geonwoo · 2024-12-24T16:12:02Z

@unifolio0

물론 기능상으로, 이후 확장면에서도 dto기 때문에 문제가 없겠지만 좀 어색한 부분 같아요.

상위 layer를 하위 layer가 알고 있는 것에 대해서 좋지 않은 점은 동의해요. 2가지 질문을 비토에게 하고 싶어요.
1. dto 패키지를 어디에 넣는 것이 좋다고 생각하는지?
1. service 계층에서 dto를 반환하는 것이 좋다고 생각하는지, 도메인 객체를 반환하는 것이 좋다고 생각하는지 == domain to dto를 service에서 해야하는지, 혹은 controller에서 해야 하는지

저는 오디에서 controller와 dto 패키지가 구분되어 있어서 이런 문제를 고민하지 않아도 되었어서 비토의 의견과 개선안이 궁금합니다.

coli-geonwoo

/noti

커찬, 서비스 센터에서 얻은 보조 배터리로 빠르게 리뷰 남깁니다.. (이후 리뷰까지는 시간이 걸릴 수 있겠지만요 ㅠㅠ)

리뷰 내용들을 요약하면 다음과 같아요.

커찬이 생각하는 domain to dto의 적절한 레이어 (service에서 dto반환? or 도메인 반환?)
ResponseEntity vs ResponseStatus 어떤걸 더 선호하는지?
ArraySchema generic 사용은 어떤지?

남겨주신 검토 포인트인 Swagger 코드는 문제가 없어보여요. 다만 Rest Docs가 마려워진 이유가 궁금하긴 합니다 ㅋㅋ

빠른 시일 안에 작업 해주어 감사합니다 ㅎㅎ

coli-geonwoo · 2024-12-24T16:14:19Z

src/main/java/com/debatetimer/controller/member/MemberController.java

+
+    @Override
+    @PostMapping("/api/member")
+    @ResponseStatus(HttpStatus.CREATED)


컨벤션에서 ResponseStatus와 관련된 이야기가 나오지 않아서 그런데

비토와 커찬 모두에게 어떤 방향을 더 선호하는지 묻습니다.

ResponseStatus 쓰기

ResponseEntity 쓰기

저는 responsebody 이외에 응답에 대한 header와 같은 설정을 해줄 수 있다는 점에서 확장성을 고려해 ResponseEntity를 사용해오긴 했습니다.

저는 header 설정이 필요할 땜만 ResponseEntity 를 사용하는 편입니다. 주로 헤더는 인증 용도로 사용되서 잘 사용할 일도 적을 것 같구요

coli-geonwoo · 2024-12-24T16:17:34Z

src/main/java/com/debatetimer/controller/member/dto/TableType.java

@@ -0,0 +1,6 @@
+package com.debatetimer.controller.member.dto;
+
+public enum TableType {


컨벤션에 따라 한 줄 개행 부탁드립니다 😄

네 반영하겠습니다.

coli-geonwoo · 2024-12-24T16:20:01Z

src/main/java/com/debatetimer/controller/member/dto/TableResponses.java

+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.List;
+
+public record TableResponses(


ArrayResponse의 경우 제네릭으로 선언하여 사용하는 건 어떨까요?

Suggested change

public record TableResponses(

public record ArrayResponse<T>(

List<T> responses

)

api에 responses 대신에 tables로 되어 있어서 TableResponses로 만들었습니다.

ArrayResponse의 경우 제네릭으로 선언하여 사용하는 건 어떨까요?

모든 배열 응답 값이 responses로 의무적으로 통일하는 것도 썩... 좋진 않은 것 같습니다. 콜리의 생각은 어떤가요?

coli-geonwoo · 2024-12-24T16:20:37Z

src/main/java/com/debatetimer/domain/parliamentary_debate/ParliamentaryTable.java

@@ -18,7 +18,7 @@
 @NoArgsConstructor(access = AccessLevel.PROTECTED)
 public class ParliamentaryTable {

-    private static final String NAME_REGEX = "^[a-zA-Z가-힣]+$";
+    private static final String NAME_REGEX = "^[a-zA-Z가-힣\\s]+$";


디테일 👍

coli-geonwoo · 2024-12-24T16:21:58Z

src/main/java/com/debatetimer/repository/member/MemberRepository.java

+
+public interface MemberRepository extends JpaRepository<Member, Long> {
+
+    default Member getById(Long id) {


[순수한 질문]

Optional 가 아니라 default 메서드를 통해 예외 처리를 해준 이유가 궁금해요!

보통 findById().orElseThrow()는 이 서비스 저 서비스에서 자주 사용되서, 레포지토리에서 default method로 만들어 사용하는 편입니다.

coli-geonwoo · 2024-12-24T16:26:04Z

src/test/java/com/debatetimer/controller/member/MemberControllerTest.java

+                    .statusCode(201)
+                    .extract().as(MemberCreateResponse.class);
+
+            assertThat(response.nickname()).isEqualTo("커찬");


[사소한 제안]

response의 nickname이 request의 nickname과 동일하다는 점에서 문자열로 하드코딩하기보다 request에서 꺼내서 검증해주는 것도 좋을 것 같아요. 반영은 커찬의 의사에 맡길게요 ㅎㅎ

Suggested change

assertThat(response.nickname()).isEqualTo("커찬");

assertThat(response.nickname()).isEqualTo(request.nickname());

반영하겠습니다~

coli-geonwoo · 2024-12-24T16:27:43Z

src/test/java/com/debatetimer/controller/member/MemberControllerTest.java

+
+            TableResponses response = RestAssured.given().log().all()
+                    .contentType(ContentType.JSON)
+                    .queryParam("memberId", member.getId())


오.. 저는 url에 직접 명시해주었는데 queryparameter를 지정해주는 이런 방법이 있었군요 👍

coli-geonwoo · 2024-12-24T16:30:59Z

src/test/java/com/debatetimer/service/member/MemberServiceTest.java

+
+            assertAll(
+                    () -> assertThat(actual.nickname()).isEqualTo("커찬"),
+                    () -> assertThat(memberRepository.count()).isEqualTo(1L)


[사소한 제안]

한번 변수로 분리해주면 읽기 편할 것 같습니다!

int memberCount = memberRepository.count() assertThat(memberCount).isOne();

반영하겠습니다.

coli-geonwoo · 2024-12-24T16:34:34Z

src/test/java/com/debatetimer/service/member/MemberServiceTest.java

+    class GetTables {
+
+        @Test
+        void 회원의_전체_토론_시간표를_조회한다() {


[질문]

저도 findByXX 테스트 작성할 때마다 고민이 되던 부분이라 커찬의 생각이 궁금해 리뷰 남겨요.

1안은 지금 커찬의 테스트처럼 깔끔하게 조회 성공여부를 검증하는 테스트입니다.

2안은 여기서 특정 회원의 의미를 조금 더 넣는건데요.

member1과 member2를 만들고

member1의 토론시간표 한개 / member2의 토론시간표를 한개 만들어요.

그리고 member1을 조회했을 때 특정 1개가 조회되는지 검증합니다.

이렇게 하면 조금 더 특정 member의 토론 시간표를 조회하는 것을 테스트 하는 느낌이 나긴하는데 1) 테스트가 복잡해지고 2) 1안과 큰 이점이 없다고 느낄 수도 있을 것 같아요. 커찬이 선호하는 방식이 궁금해요.

뭐... "내가 어디까지 테스트 하고 싶은가?"에 따라 다를 것 같아요. 복잡하고 중요한 로직이라면 1안 2안 둘 다 하나씩 만들어 볼 것 같아요. 지금은 그렇게 중요하다고 생각하지 않으니, 1안으로 한 번 검증한 것 같습니다.

unifolio0

/noti

수정할 부분이 있는 것 같아 rc 남겼습니다! 확인하고 의견 남겨주세요!

@coli-geonwoo

상위 layer를 하위 layer가 알고 있는 것에 대해서 좋지 않은 점은 동의해요. 2가지 질문을 비토에게 하고 싶어요.

dto 패키지를 어디에 넣는 것이 좋다고 생각하는지?

service 계층에서 dto를 반환하는 것이 좋다고 생각하는지, 도메인 객체를 반환하는 것이 좋다고 생각하는지 == domain to dto를 service에서 해야하는지, 혹은 controller에서 해야 하는지
저는 오디에서 controller와 dto 패키지가 구분되어 있어서 이런 문제를 고민하지 않아도 되었어서 비토의 의견과 개선안이 궁금합니다.

아마 저라면 dto패키지를 최외각 패키지 또는 service로 옮길 것 같아요. 하지만 지금 패키지 구조로 할 꺼라면 controller에서 domain <-> dto과정을 수행할 것 같네요.

unifolio0 · 2024-12-25T04:54:02Z

src/main/java/com/debatetimer/controller/member/dto/MemberCreateResponse.java

+public record MemberCreateResponse(
+
+        @Schema(description = "멤버 아이디", example = "1")
+        long id,


이곳은 클래스 최상단이 아닌 일종의 생성자 파라미터를 입력하는 곳인데 개행을 없애는건 어떤가요?

좋은 것 같네요! 반영하도록 하겠습니다~!

unifolio0 · 2024-12-25T05:02:47Z

src/main/java/com/debatetimer/domain/parliamentary_debate/ParliamentaryTable.java

@@ -18,7 +18,7 @@
 @NoArgsConstructor(access = AccessLevel.PROTECTED)
 public class ParliamentaryTable {

-    private static final String NAME_REGEX = "^[a-zA-Z가-힣]+$";
+    private static final String NAME_REGEX = "^[a-zA-Z가-힣\\s]+$";


테이블 이름으로 띄어쓰기만 가능하게 할 껀가요, 모든 개행문자가 다 가능하도록 할 껀가요? 지금은 모든 개행문자가 다 가능할 것 같아요.

"^[a-zA-Z가-힣 ]+$" 으로 수정했습니다~

unifolio0 · 2024-12-25T05:11:38Z

src/test/java/com/debatetimer/domain/parliamentary_debate/ParliamentaryTableTest.java

+        @ValueSource(strings = {"a bc가다", "가나 다ab"})
+        void 테이블_이름은_영문과_한글_띄어쓰기만_가능하다(String name) {


지금 정규식 기준으론 띄어쓰기뿐만이 아닌 줄바꿈이나 탭도 가능할 것 같아요.

leegwichan · 2024-12-25T07:01:16Z

/noti
의견 주셨던 코멘트에 모두 답변했으니, 다시 확인 부탁드릴께요!

unifolio0

/noti
남은 부분이 회의에서 애기하기로 한 부분만 남아서 일단 approve합니다

- parliamentary_debate -> parliamentary

leegwichan · 2024-12-27T07:12:38Z

/noti @debate-timer/backend

자, 회의 내용 반영해서 parliamentary 패키지 이름이랑 dto 패키지 위치 바꿔놨습니다. 얼른 Approve 해줄 수 있도록

coli-geonwoo

/noti

회위 내용이 잘 반영된 것 같아 approve합니다!

leegwichan added 6 commits December 24, 2024 15:19

feat: 회원 생성 기능 추가

6ded067

feat: 의회식 토론 테이블의 이름에 띄어쓰기 허용

35a7902

feat: 회원의 전체 토론 시간표 조회 기능 추가

394305c

style: 메서드 순서를 CRUD 순으로 변경

76c03f4

docs: 멤버 생성, 멤버 토론 시간표 조회 API 문서 작성

132761f

test: ParliamentaryTable 테스트 케이스 추가

c82cf3b

unifolio0 added the feat 기능 추가 label Dec 24, 2024

unifolio0 self-requested a review December 24, 2024 11:07

unifolio0 assigned leegwichan Dec 24, 2024

unifolio0 requested a review from coli-geonwoo December 24, 2024 11:07

coli-geonwoo requested changes Dec 24, 2024

View reviewed changes

leegwichan added 2 commits December 25, 2024 12:22

style: TableType 컨벤션에 맞추어 공백 추가

d5aef09

test: 테스트 코드 수정

efefdbc

unifolio0 requested changes Dec 25, 2024

View reviewed changes

leegwichan added 2 commits December 25, 2024 15:55

style: 레코드 상단 공백 제거

16a32cb

fix: ParliamentaryTable 이름 공백은 띄어쓰기만 허용

490b4b6

unifolio0 approved these changes Dec 26, 2024

View reviewed changes

leegwichan added 2 commits December 27, 2024 16:04

refactor: 패키지 네이밍 변경

d6de359

- parliamentary_debate -> parliamentary

refactor: dto 패키지 최외곽으로 이동

beb68e0

coli-geonwoo approved these changes Dec 27, 2024

View reviewed changes

coli-geonwoo merged commit 09287ab into develop Dec 27, 2024
4 checks passed

coli-geonwoo deleted the feat/#33 branch December 27, 2024 11:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEAT] 멤버 관련 API 구현 #34

[FEAT] 멤버 관련 API 구현 #34

leegwichan commented Dec 24, 2024

github-actions bot commented Dec 24, 2024 •

edited

Loading

github-actions bot commented Dec 24, 2024 •

edited

Loading

unifolio0 commented Dec 24, 2024

coli-geonwoo commented Dec 24, 2024

coli-geonwoo left a comment

coli-geonwoo Dec 24, 2024

leegwichan Dec 25, 2024

coli-geonwoo Dec 24, 2024

leegwichan Dec 25, 2024

coli-geonwoo Dec 24, 2024

leegwichan Dec 25, 2024

coli-geonwoo Dec 24, 2024

coli-geonwoo Dec 24, 2024

leegwichan Dec 25, 2024

coli-geonwoo Dec 24, 2024

leegwichan Dec 25, 2024

coli-geonwoo Dec 24, 2024

coli-geonwoo Dec 24, 2024

leegwichan Dec 25, 2024

coli-geonwoo Dec 24, 2024

leegwichan Dec 25, 2024

unifolio0 left a comment

unifolio0 Dec 25, 2024

leegwichan Dec 25, 2024

unifolio0 Dec 25, 2024

leegwichan Dec 25, 2024

unifolio0 Dec 25, 2024

leegwichan commented Dec 25, 2024

unifolio0 left a comment

leegwichan commented Dec 27, 2024

coli-geonwoo left a comment

		@@ -0,0 +1,6 @@
		package com.debatetimer.controller.member.dto;

		public enum TableType {

-public record TableResponses(
+public record ArrayResponse<T>(
+        List<T> responses
+    )


		public interface MemberRepository extends JpaRepository<Member, Long> {

		default Member getById(Long id) {

	assertThat(response.nickname()).isEqualTo("커찬");
	assertThat(response.nickname()).isEqualTo(request.nickname());

		@ValueSource(strings = {"a bc가다", "가나 다ab"})
		void 테이블_이름은_영문과_한글_띄어쓰기만_가능하다(String name) {

[FEAT] 멤버 관련 API 구현 #34

[FEAT] 멤버 관련 API 구현 #34

Conversation

leegwichan commented Dec 24, 2024

🚩 연관 이슈

🗣️ 리뷰 요구사항 (선택)

github-actions bot commented Dec 24, 2024 • edited Loading

Test Results

github-actions bot commented Dec 24, 2024 • edited Loading

📝 Test Coverage Report

unifolio0 commented Dec 24, 2024

coli-geonwoo commented Dec 24, 2024

coli-geonwoo 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

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

unifolio0 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

leegwichan commented Dec 25, 2024

unifolio0 left a comment

Choose a reason for hiding this comment

leegwichan commented Dec 27, 2024

coli-geonwoo left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 24, 2024 •

edited

Loading

github-actions bot commented Dec 24, 2024 •

edited

Loading