[BE] issue187: 특정 스터디에서 나의 role을 확인하는 API 생성 #193

jaejae-yoo · 2022-08-03T05:27:38Z

요약

특정 스터디에서 나의 role을 확인하는 API를 생성한다.

세부사항

close #187

sc0116

린론! 코드 잘봤어요! 혹시 구현중이신건가요? 몇가지 코멘트 남겼어요!

sc0116 · 2022-08-04T00:13:10Z

backend/src/main/java/com/woowacourse/moamoa/auth/controller/AuthController.java

@@ -18,4 +20,10 @@ public class AuthController {
    public ResponseEntity<TokenResponse> login(@RequestParam final String code) {
        return ResponseEntity.ok().body(authService.createToken(code));
    }
+
+    @GetMapping("/api/auth/role")


음 저는 특정 스터디에서 내 역할을 보는 API가 auth쪽에 있는게 어색하다고 느껴지는데요.
린론은 왜 auth에다 하셨나요?

auth는 정말 소셜로그인이나 회원가입 같은 인증관련 API만 두는거라고 생각하고 있어서 더 어색하다 느껴지는 것 같네요
auth에서 studyId를 받는건 더더욱 어색하구요!

명세 변경전에 있던 코드라서, 현재 /api/members/me/role?study-id={studyId} 로 변경됐습니다 ~

sc0116 · 2022-08-04T00:14:32Z

backend/src/main/java/com/woowacourse/moamoa/auth/controller/AuthController.java

+
+    @GetMapping("/api/auth/role")
+    public ResponseEntity<Void> getStudyRole(
+            @AuthenticationPrincipal Long githubId, @RequestParam(name = "study-id") Long studyId) {


어? studyId는 URI에 없는데 어디서 가져오는건가요?

sc0116 · 2022-08-04T00:14:46Z

backend/src/main/java/com/woowacourse/moamoa/auth/controller/AuthController.java

+    @GetMapping("/api/auth/role")
+    public ResponseEntity<Void> getStudyRole(
+            @AuthenticationPrincipal Long githubId, @RequestParam(name = "study-id") Long studyId) {
+        return ResponseEntity.ok().build();


그냥 ok만 넘겨주네요

tco0427

린론!! 구현 깔끔하게 잘 해주셨네요..!!

몇가지 의견 남겨봅니다!😁

tco0427 · 2022-08-04T03:05:09Z

backend/src/main/java/com/woowacourse/moamoa/study/controller/MyStudyController.java

+    @GetMapping("/api/members/me/role")
+    public ResponseEntity<MyRoleResponse> getMyRoleInStudy(
+            @AuthenticationPrincipal Long githubId, @RequestParam(name = "study-id") Long studyId
+    ) {
+        return ResponseEntity.ok().body(myStudyService.findMyRoleInStudy(githubId, studyId));


tco0427 · 2022-08-04T03:05:29Z

backend/src/main/java/com/woowacourse/moamoa/study/domain/MyRole.java

@@ -0,0 +1,6 @@
+package com.woowacourse.moamoa.study.domain;
+
+public enum MyRole {


MemberRole 가 더 명시적일 것 같습니다!

좋아요👍 클래스명 수정했습니다.

tco0427 · 2022-08-04T03:07:16Z

backend/src/main/java/com/woowacourse/moamoa/study/domain/Participants.java

+    public boolean isParticipate(Long memberId) {
+        return participants.contains(new Participant(memberId));
+    }


기존에 존재하는 isAlreadyParticipated 메소드를 활용해도 Service 로직에 이상이 없을 것 같아요!
(owner 를 먼저 검사하고 있어서)

물론 로직 자체로는 문제가 없지만, 해당 사용자가 참여자인지 확인하는 메서드로 분리하는 게 더 명확하다고 생각해서 나누는 게 좋을 것 같아요! 기존의 isAlreadyParticipated의 메서드명 때문에 헷갈릴 것 같아서요!

tco0427 · 2022-08-04T03:08:10Z

backend/src/test/java/com/woowacourse/moamoa/docs/MyStudyDocumentationTest.java

+    @DisplayName("스터디에서 사용자의 역할을 조회한다.")
+    @Test
+    void getMyRoleInStudy() throws Exception {
+        given(myStudyService.findMyRoleInStudy(any(), any())).willReturn(new MyRoleResponse(MyRole.MEMBER));
+
+        mockMvc.perform(get("/api/members/me/role")
+                        .header(HttpHeaders.AUTHORIZATION, "Bearer " + JWT_토큰)
+                        .queryParam("study-id", "3")
+                        .contentType(MediaType.APPLICATION_JSON_VALUE)
+                        .accept(MediaType.APPLICATION_JSON))
+                .andDo(document("members/me/role",
+                        requestHeaders(
+                                headerWithName("Authorization").description(
+                                        "Bearer Token")),
+                        requestParameters(parameterWithName("study-id").description("스터디 ID")),
+                        responseFields(fieldWithPath("role").type(JsonFieldType.STRING).description("해당 스터디에서 사용자의 역할"))))
+                .andDo(print())
+                .andExpect(status().isOk());
+    }


인수테스트로 문서화하기로 해서 이 테스트는 삭제해도 될 것 같습니다!

tco0427 · 2022-08-04T03:08:56Z

backend/src/test/java/com/woowacourse/acceptance/study/GettingMyStudiesAcceptanceTest.java

+        RestAssured.given().log().all()
+                .header(CONTENT_TYPE, APPLICATION_JSON_VALUE)
+                .header(AUTHORIZATION, token)
+                .queryParam("study-id", 7)
+                .when()
+                .get("/api/members/me/role")
+                .then().log().all()
+                .statusCode(HttpStatus.OK.value())
+                .body("role", is("OWNER"));
+    }


OWNER 이외에 NON_MEMBER 나 MEMBER 등의 테스트 케이스도 추가해주면 좋을 것 같아요!! (꼭 인수테스트에서 할 필요는 없을 것 같구...)

service에서 테스트하고 있습니다!

sc0116

린론 구현한 코드 확인했습니다!

sc0116 · 2022-08-04T05:27:24Z

backend/src/test/java/com/woowacourse/moamoa/docs/DocumentationTest.java

+})
+@ExtendWith(RestDocumentationExtension.class)
+@Import(JwtTokenProvider.class)
+public class DocumentationTest {


이 클래스 MockMvc에서 RestAssured로 변경하면서 사라졌습니다!

RestAssured로 추가했습니다 ~

sc0116 · 2022-08-04T05:30:25Z

backend/src/main/java/com/woowacourse/moamoa/study/controller/MyStudyController.java

+
+    @GetMapping("/api/members/me/role")
+    public ResponseEntity<MyRoleResponse> getMyRoleInStudy(
+            @AuthenticationPrincipal Long githubId, @RequestParam(name = "study-id") Long studyId


final이 빠졌어요..!

추가했습니다! 👍

verus-j · 2022-08-04T05:46:26Z

backend/src/test/java/com/woowacourse/moamoa/study/webmvc/BadRequestMyMyRoleWebMvcTest.java

+
+@WebMvcTest(controllers = MyStudyController.class)
+@Import(JwtTokenProvider.class)
+public class BadRequestMyMyRoleWebMvcTest {


study Id가 String인 경우 400 예외를 확인하는 테스트 추가 부탁드려요~

2237674

추가했습니다 ~ :D

verus-j · 2022-08-04T05:47:14Z

backend/src/test/java/com/woowacourse/moamoa/study/webmvc/UnauthorizedMyStudyWebMvcTest.java

-@Import({JwtTokenProvider.class})
-class UnauthorizedMyStudyControllerTest {
+@Import(JwtTokenProvider.class)
+class UnauthorizedMyStudyWebMvcTest {


/api/members/me/role에 대한 테스트도 추가 부탁드려요~

인증 테스트 추가했습니다 ~

verus-j · 2022-08-04T06:00:08Z

backend/src/main/java/com/woowacourse/moamoa/study/service/MyStudyService.java

+        final Participants participants = study.getParticipants();
+        return getMyRoleResponse(member, participants);
+    }
+
+    private MyRoleResponse getMyRoleResponse(final Member member, final Participants participants) {
+        if (Objects.equals(participants.getOwnerId(), member.getId())) {
+            return new MyRoleResponse(MemberRole.OWNER);
+        }
+        if (participants.isParticipate(member.getId())) {
+            return new MyRoleResponse(MemberRole.MEMBER);
+        }
+        return new MyRoleResponse(MemberRole.NON_MEMBER);
+    }


Suggested change

final Participants participants = study.getParticipants();

return getMyRoleResponse(member, participants);

}

private MyRoleResponse getMyRoleResponse(final Member member, final Participants participants) {

if (Objects.equals(participants.getOwnerId(), member.getId())) {

return new MyRoleResponse(MemberRole.OWNER);

}

if (participants.isParticipate(member.getId())) {

return new MyRoleResponse(MemberRole.MEMBER);

}

return new MyRoleResponse(MemberRole.NON_MEMBER);

}

Role role = study.getRole(member.getId());

return new MyRoleResponse(role);

이런 식으로 메시지를 날리면 좋을 것 같아요

그리고 Domain에 단위 테스트를 만들면 좋을 것 같아요.

로직 이동하고 단위 테스트 생성했습니다 ~

test: 특정 스터디에서 나의 role을 확인하는 API 인수 테스트 작성

43b3b40

jaejae-yoo added 🚀 feature New feature or request 🖥 backend New backend feature labels Aug 3, 2022

jaejae-yoo requested review from verus-j, sc0116 and tco0427 August 3, 2022 05:27

jaejae-yoo self-assigned this Aug 3, 2022

jaejae-yoo added 3 commits August 3, 2022 17:34

refactor: api 명세 변경

3c41971

feat: 스터디에서 사용자의 role을 확인하는 기능

ed84cd4

fix: 충돌 해결

588edcf

sc0116 requested changes Aug 4, 2022

View reviewed changes

jaejae-yoo added 2 commits August 4, 2022 09:44

feat: 스터디에서 사용자의 역할을 조회하는 rest docs API 생성

184bd65

refactor: 스터디에서 나의 역할을 조회하는 서비스 로직 메서드 분리

d5fed86

tco0427 requested changes Aug 4, 2022

View reviewed changes

jaejae-yoo added 3 commits August 4, 2022 12:14

fix: 충돌 해결

2bef2b1

fix: rest docs 문제 해결

220a59c

fix: 충돌 해결

4fd28f8

sc0116 approved these changes Aug 4, 2022

View reviewed changes

refactor: MyRole -> MemberRole 클래스명 변경

dbcc5ad

verus-j requested changes Aug 4, 2022

View reviewed changes

jaejae-yoo added 7 commits August 4, 2022 16:55

refactor: 스터디 사용자 역할을 판별하는 로직 스터디 객체로 이동

8a26d57

test: 사용자 역할 조회 사용자 인증 테스트 추가

785d1d7

Merge branch 'develop' into feat/187-check-is-my-study

7b678f4

refactor: rest docs 변경

f9e5a04

refactor: rest docs Mock Mvc -> RestAssuerd로 변경

12bfe15

test: 사용자가 존재하지 않을 때, 스터디가 존재하지 않을 때 예외 추가

7102615

test: study Id가 string일 경우 400 에러 테스트 추가

2237674

tco0427 approved these changes Aug 4, 2022

View reviewed changes

fix: 충돌 해결

99b3c86

verus-j approved these changes Aug 4, 2022

View reviewed changes

fix: 충돌 해결

19ed32e

jaejae-yoo merged commit 8cd2547 into develop Aug 4, 2022

jaejae-yoo deleted the feat/187-check-is-my-study branch August 4, 2022 12:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] issue187: 특정 스터디에서 나의 role을 확인하는 API 생성 #193

[BE] issue187: 특정 스터디에서 나의 role을 확인하는 API 생성 #193

jaejae-yoo commented Aug 3, 2022

sc0116 left a comment

sc0116 Aug 4, 2022

jaejae-yoo Aug 4, 2022

sc0116 Aug 4, 2022

sc0116 Aug 4, 2022

tco0427 left a comment

tco0427 Aug 4, 2022

tco0427 Aug 4, 2022

jaejae-yoo Aug 4, 2022

tco0427 Aug 4, 2022

jaejae-yoo Aug 4, 2022

tco0427 Aug 4, 2022

tco0427 Aug 4, 2022

jaejae-yoo Aug 4, 2022

sc0116 left a comment

sc0116 Aug 4, 2022

jaejae-yoo Aug 4, 2022

sc0116 Aug 4, 2022

jaejae-yoo Aug 4, 2022

verus-j Aug 4, 2022

jaejae-yoo Aug 4, 2022

verus-j Aug 4, 2022

jaejae-yoo Aug 4, 2022

verus-j Aug 4, 2022

verus-j Aug 4, 2022

jaejae-yoo Aug 4, 2022

		@@ -0,0 +1,6 @@
		package com.woowacourse.moamoa.study.domain;

		public enum MyRole {

[BE] issue187: 특정 스터디에서 나의 role을 확인하는 API 생성 #193

[BE] issue187: 특정 스터디에서 나의 role을 확인하는 API 생성 #193

Conversation

jaejae-yoo commented Aug 3, 2022

요약

세부사항

sc0116 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

tco0427 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

sc0116 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