[FEAT] 방 생성 및 참여 기능 구현 #64

jhon3242 · 2024-07-24T06:46:38Z

Issue Number

#56

As-Is

Sprint 2 P2에 맞추어 주제, 방 생성 및 참여 API 구현

To-Be

방 생성 API 추가
방 참여 API 추가

Check List

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

Test Screenshot

(Optional) Additional Description

leegwichan

타콩! 빠르게 구현하느라 고생 많았어!
일단 내 눈에 보이는 부분 위주로 봤어 한 번 확인해 줘!

leegwichan · 2024-07-24T07:09:54Z

backend/src/main/java/ddangkong/controller/balance/room/RoomController.java

+    public RoomJoinResponse joinRoom(@PathVariable @Positive Long roomId, @RequestBody String nickname) {
+        return roomService.joinRoom(nickname, roomId);
+    }


의견) 여기도 Validation 들어가야하지 않나요?

Suggested change

public RoomJoinResponse joinRoom(@PathVariable @Positive Long roomId, @RequestBody String nickname) {

return roomService.joinRoom(nickname, roomId);

}

public RoomJoinResponse joinRoom(@PathVariable @Positive Long roomId, @RequestBody @NotNull String nickname) {

return roomService.joinRoom(nickname, roomId);

}

leegwichan · 2024-07-24T07:16:30Z

backend/src/test/java/ddangkong/service/balance/room/RoomServiceTest.java

+        @Test
+        void 존재하지_않는_방에_참여시_예외를_던진다() {
+            // given & when
+            String nickname = "나는참가자";
+            Long nonExistId = 99999999999L;
+            MemberResponse expectedMemberResponse = new MemberResponse(5L, nickname, false);
+            RoomJoinResponse expected = new RoomJoinResponse(nonExistId, expectedMemberResponse);
+
+            // then
+            assertThatThrownBy(() -> roomService.joinRoom(nickname, nonExistId))
+                    .isInstanceOf(BadRequestException.class);
+        }


의견) 사용하지 않는 변수들을 제거해 보았습니다~

Suggested change

@Test

void 존재하지_않는_방에_참여시_예외를_던진다() {

// given & when

String nickname = "나는참가자";

Long nonExistId = 99999999999L;

MemberResponse expectedMemberResponse = new MemberResponse(5L, nickname, false);

RoomJoinResponse expected = new RoomJoinResponse(nonExistId, expectedMemberResponse);

// then

assertThatThrownBy(() -> roomService.joinRoom(nickname, nonExistId))

.isInstanceOf(BadRequestException.class);

}

@Test

void 존재하지_않는_방에_참여시_예외를_던진다() {

// given

String nickname = "나는참가자";

Long nonExistId = 99999999999L;

// when & then

assertThatThrownBy(() -> roomService.joinRoom(nickname, nonExistId))

.isInstanceOf(BadRequestException.class);

}

leegwichan · 2024-07-24T07:18:37Z

backend/src/main/java/ddangkong/service/balance/room/RoomService.java

+
+    public RoomJoinResponse createRoom(String nickname) {
+        Room room = roomRepository.save(new Room());
+        Member member = memberRepository.save(new Member(nickname, room, true));


사소한 의견) Member에 정적 팩토리 메서드를 도입해서 createMaster(nickname, room), createCommon(nickname, room)을 만드는 건 어떨까요?

그게 의미가 더 잘 전달 될 것 같네요! 바로 반영해볼게요~

GIVEN53

고생했어요 딱칸~~ 컨트롤러 테스트도 추가해주시면 감사하겠습니다 🙃

GIVEN53 · 2024-07-24T08:25:18Z

backend/src/main/java/ddangkong/controller/balance/room/RoomController.java

+
+    @RequestMapping("/balances/rooms")
+    @PostMapping
+    public RoomJoinResponse creatRoom(@RequestBody @NotNull String nickname) {


Suggested change

public RoomJoinResponse creatRoom(@RequestBody @NotNull String nickname) {

public RoomJoinResponse creatRoom(@Valid @RequestBody String nickname) {

의견) 수정이 필요합니다

GIVEN53 · 2024-07-24T08:26:15Z

backend/src/main/java/ddangkong/controller/balance/room/RoomController.java

+    private final RoomService roomService;
+
+    @RequestMapping("/balances/rooms")
+    @PostMapping


Suggested change

@PostMapping

@PostMapping("/balances/rooms")

의견) 메서드 레벨에 @RequestMapping을 지우고 위처럼 수정하는 것은 어떨까요?

GIVEN53 · 2024-07-24T08:28:04Z

backend/src/main/java/ddangkong/controller/balance/room/RoomController.java

+    }
+
+    @RequestMapping("/balances/rooms/{roomId}/members")
+    @PostMapping


GIVEN53 · 2024-07-24T08:28:14Z

backend/src/main/java/ddangkong/controller/balance/room/RoomController.java

+
+    @RequestMapping("/balances/rooms/{roomId}/members")
+    @PostMapping
+    public RoomJoinResponse joinRoom(@PathVariable @Positive Long roomId, @RequestBody String nickname) {


@Valid ditto

GIVEN53 · 2024-07-24T08:30:01Z

backend/src/main/java/ddangkong/service/balance/room/RoomService.java

+
+    private final RoomRepository roomRepository;
+
+    public RoomJoinResponse createRoom(String nickname) {


의견) 트랜잭션이 필요할 것 같습니당

동의합니다~

GIVEN53 · 2024-07-24T08:30:09Z

backend/src/main/java/ddangkong/service/balance/room/RoomService.java

+        return new RoomJoinResponse(room.getId(), new MemberResponse(member));
+    }
+
+    public RoomJoinResponse joinRoom(String nickname, Long roomId) {


GIVEN53 · 2024-07-24T08:32:10Z

backend/src/main/java/ddangkong/domain/balance/content/Room.java


 @Entity
-@NoArgsConstructor(access = AccessLevel.PROTECTED)


질문) 제거된 이유가 있나요??

RoomService 에 createRoom 메서드에서 Room 객체를 생성해야하는데, Default 값을 통해 생성자를 생성하려면 기본생성자가 호출해야하더라고요~ 그래서 지워주었습니다!

PgmJun

고생하셨습니다 타칸! 리뷰 확인 부탁드려요 :)

PgmJun · 2024-07-24T08:18:46Z

backend/src/main/java/ddangkong/controller/balance/member/dto/MemberResponse.java

+        String nickname,
+        boolean isMaster
+) {
+    public MemberResponse(Member member) {


사소한 의견) 한칸 띄워야하지 않나용?

클래스는 띄어주는게 맞는데 레코드도 띄어줘야할까요?? 이 부분에 대해선 컨벤션으로 정하지 않았던 것 같아요~ 내일 다 같이 말해보죠!

PgmJun · 2024-07-24T08:21:30Z

backend/src/main/java/ddangkong/controller/balance/room/RoomController.java

+    @RequestMapping("/balances/rooms")
+    @PostMapping
+    public RoomJoinResponse creatRoom(@RequestBody @NotNull String nickname) {
+        return roomService.createRoom(nickname);
+    }
+
+    @RequestMapping("/balances/rooms/{roomId}/members")
+    @PostMapping
+    public RoomJoinResponse joinRoom(@PathVariable @Positive Long roomId, @RequestBody String nickname) {


의견) PostMapping("/balances/rooms/{roomId}/members") 만 써주어도 되지 않을까요? 두 개를 다 써주신 이유가 있는 지 궁금합니다~

의견) ResponseEntity로 감싸주지 않고 있는데 created, ok 등 설정하려면 필요하지 않을까 싶습니다~
아니라면 ResponseStatus 애노테이션 사용을 하는 방법도 있습니다

PgmJun · 2024-07-24T08:37:00Z

backend/src/main/java/ddangkong/domain/member/Member.java

+    public Member(String nickname, Room room, boolean isMaster) {
+        this.nickname = nickname;
+        this.room = room;
+        this.isMaster = isMaster;
+    }


의견) 아까 보여주셨던 정팩메 사용해서 처리해주셔도 좋을거 같아요! createRoomMaster? 네이밍으로 말씀해주셨던 부분이요!

PgmJun · 2024-07-24T08:37:18Z

backend/src/main/java/ddangkong/service/balance/room/RoomService.java

+
+    private final RoomRepository roomRepository;
+
+    public RoomJoinResponse createRoom(String nickname) {


동의합니다~

PgmJun · 2024-07-24T08:38:55Z

backend/src/test/java/ddangkong/service/balance/room/RoomServiceTest.java

+            RoomJoinResponse expected = new RoomJoinResponse(3L, expectedMemberResponse);
+
+            // when
+            RoomJoinResponse roomServiceRoom = roomService.createRoom(nickname);


사소한 의견) 커찬이 검증할 매개변수명을 actual로 스타트 끊어주셔서 이어가도 좋을 것 같아요:)

PgmJun · 2024-07-24T08:39:45Z

backend/src/test/java/ddangkong/service/balance/room/RoomServiceTest.java

+            // given & when
+            String nickname = "나는참가자";
+            Long nonExistId = 99999999999L;
+            MemberResponse expectedMemberResponse = new MemberResponse(5L, nickname, false);
+            RoomJoinResponse expected = new RoomJoinResponse(nonExistId, expectedMemberResponse);
+
+            // then
+            assertThatThrownBy(() -> roomService.joinRoom(nickname, nonExistId))


의견) given, when&then 이 맞지 않을까요??

jhon3242

리뷰 반영했습니다!😊 다시 한번 확인 부탁드립니다~~

leegwichan

오! 타칸 빠른 반영~~ 고생했어!! 리뷰한 내용들은 전부 문제없는 것 같아!
(Merge의 일감은 이든에게로 넘어가나요? ㅋㅋ)

GIVEN53

LGTM 🤗🤗🤗

PgmJun

LGTM!

jhon3242 added 2 commits July 24, 2024 15:38

feat: 방 생성 기능 추가 #56

0057ac6

feat: 방 참여 기능 구현 #56

84b4c9b

jhon3242 added ✨ feat 기능 추가 🍃 BE back end labels Jul 24, 2024

jhon3242 added this to the BE Sprint2 milestone Jul 24, 2024

jhon3242 requested review from leegwichan, PgmJun and GIVEN53 July 24, 2024 06:46

jhon3242 self-assigned this Jul 24, 2024

jhon3242 added 2 commits July 24, 2024 15:59

feat: Member 에 isMaster 필드 추가 #56

60bdfbb

fix: sql 통과하도록 수정

3fc64ed

leegwichan reviewed Jul 24, 2024

View reviewed changes

feat: 불필요한 코드 삭제 및 정팩메, 검증 추가

3ac0a9d

GIVEN53 requested changes Jul 24, 2024

View reviewed changes

PgmJun reviewed Jul 24, 2024

View reviewed changes

feat: 컨트롤러 응답 코드 추가 및 트랜잭션 추가

5210df2

jhon3242 commented Jul 24, 2024

View reviewed changes

leegwichan approved these changes Jul 24, 2024

View reviewed changes

fix: RequestBody dto로 받도록 수정

e8911a7

GIVEN53 approved these changes Jul 24, 2024

View reviewed changes

PgmJun approved these changes Jul 25, 2024

View reviewed changes

jhon3242 merged commit 3f756de into develop Jul 25, 2024
1 check passed

GIVEN53 deleted the feat/#56 branch December 10, 2024 11:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEAT] 방 생성 및 참여 기능 구현 #64

[FEAT] 방 생성 및 참여 기능 구현 #64

jhon3242 commented Jul 24, 2024 •

edited

Loading

leegwichan left a comment

leegwichan Jul 24, 2024

leegwichan Jul 24, 2024

leegwichan Jul 24, 2024

jhon3242 Jul 24, 2024

GIVEN53 left a comment

GIVEN53 Jul 24, 2024

GIVEN53 Jul 24, 2024

GIVEN53 Jul 24, 2024

GIVEN53 Jul 24, 2024

GIVEN53 Jul 24, 2024

PgmJun Jul 24, 2024

GIVEN53 Jul 24, 2024

GIVEN53 Jul 24, 2024

jhon3242 Jul 24, 2024

PgmJun left a comment

PgmJun Jul 24, 2024

jhon3242 Jul 24, 2024 •

edited

Loading

PgmJun Jul 24, 2024

PgmJun Jul 24, 2024

PgmJun Jul 24, 2024

PgmJun Jul 24, 2024

PgmJun Jul 24, 2024

PgmJun Jul 24, 2024

jhon3242 left a comment

leegwichan left a comment

GIVEN53 left a comment

PgmJun left a comment

	public RoomJoinResponse creatRoom(@RequestBody @NotNull String nickname) {
	public RoomJoinResponse creatRoom(@Valid @RequestBody String nickname) {


		private final RoomRepository roomRepository;

		public RoomJoinResponse createRoom(String nickname) {

[FEAT] 방 생성 및 참여 기능 구현 #64

[FEAT] 방 생성 및 참여 기능 구현 #64

Conversation

jhon3242 commented Jul 24, 2024 • edited Loading

Issue Number

As-Is

To-Be

Check List

Test Screenshot

(Optional) Additional Description

leegwichan 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

GIVEN53 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

PgmJun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhon3242 Jul 24, 2024 • edited Loading

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

jhon3242 left a comment

Choose a reason for hiding this comment

leegwichan left a comment

Choose a reason for hiding this comment

GIVEN53 left a comment

Choose a reason for hiding this comment

PgmJun left a comment

Choose a reason for hiding this comment

jhon3242 commented Jul 24, 2024 •

edited

Loading

jhon3242 Jul 24, 2024 •

edited

Loading