Feature/#133-마이페이지_회원_정보_수정_기능을_제작한다 #159

jimini1026 · 2024-06-26T06:27:25Z

#️⃣ 연관된 이슈

close: #133

📝 작업 내용

마이 페이지 이름 업데이트 기능 구현

💬 리뷰 요구사항(선택)

update하는 것에 있어서 새로운 객체를 만들어서 넣는 것으로 하였는데 괜찮을까요?

#133

JJimini

첫 pr!! 수고하셨습니다~!!👍👍 다만 현재 branch가 develop branch를 merge하지 않은 상태인 것 같아요~ 저희가 discussion을 통해 final을 사용하는 범위에 대해 이야기를 나눈적이 있습니다~ 현재 develop branch는 해당 내용이 다 적용이 된 상태입니다! 읽어보시고 코드 수정하시면 좋을 것 같아요~!
final 사용에 대한 논의

JJimini · 2024-06-26T10:05:47Z

src/main/java/doore/member/api/MemberController.java

+    @PatchMapping("/profile/name")
+    public ResponseEntity<Void> updateMyPageName(@RequestBody String newName, @LoginMember Member member) {


마이페이지는 회원 본인만 접근이 가능해야합니다! URI에 회원의 정보를 받아오면 좋을 것 같습니다! "/profile/members/{memberId}"로 수정하는 것은 어떨까 제안드려봅니다!

또한 메서드명이 너무 상세합니다. 회원의 이름을 수정하는 메서드가 아닌 회원의 정보를 수정하는 메서드가 되면 좋을 것 같습니다. ex) updateMyPage
이렇게 작성하면 추후 수정할 회원 정보가 확장되면 추가하기 쉽습니다~! (이름을 수정하는 메서드, 이메일을 수정하는 메서드 등등 수정 내용이 생길때마다 메서드를 만들 수는 없으니까요!)

추가로 저희는 DTO로 request를 받아옵니다! request에 수정할 회원 정보를 담으시면 될 것 같습니다!

@JJimini MemberArgumentResolver에서 보면 request의 헤더를 통해 전달되는 토큰을 이용해 memberID 정보를 들고 올 수 있는데 혹시 memberID를 path variable에 한 번 더 쓰는 이유가 있을까요?

우선 저희 두레 프로젝트는 URL에 memberId를 명시적으로 표현하여 RESTful API의 일관성을 유지하고자 했으며, 이것은 초기 컨벤션 논의 때 규칙으로 정하였고 그렇게 사용하고 있습니다!

JJimini · 2024-06-26T10:07:10Z

src/main/java/doore/member/application/MemberCommandService.java

+
+    public void updateMyPageName(Long memberId, String newName) {


앞 리뷰와 동일합니다~ 메서드명 수정하면 좋을 것 같아요!

JJimini · 2024-06-26T10:09:15Z

src/main/java/doore/member/application/MemberCommandService.java

+        Member member = validateExistMember(memberId);
+        Member updatedMember = Member.builder()


저희가 클라이언트로부터 받아온 memberId와 토큰의 memberId가 같은지(본인확인절차)가 필요할 것 같습니다~

@JJimini 프론트의 localStorage에서 저장한 것은 클라이언트에서 바꿀 수 있어서 그 값을 받아서 굳이 확인하기보다는 저희가 보낸 토큰을 통해 복호화하여 멤버의 권한을 확인 할 필요 없이 그 사람의 myPage를 보여주는 것이 괜찮을 것 같은데 어떻게 생각하시나요?

@jimini1026 우선 저희 두레는 회원(본인) 접근 api는 2가지 검증 절차를 거칩니다.

클라이언트로 받은 값이 회원이 맞는지 확인

그 회원이 토큰에 들어있는 회원과 일치하는지 확인

1번은 클라이언트에서 변경할 수 있기 때문에 확인하는 것이고, 그 값이 맞다면 2번 확인을 통해 본인이 맞는지 확인합니다.

예를 들어, 클라이언트로부터 memberId가 1이 들어왔고 token에 있는 memberId는 5라고 가정합시다. memberId 1이 데이터베이스에 존재하면 1번 검증을 통과하지만 본인이 아니기 때문에 2번 검증에서 걸러지는 것이죠.

요시님 말씀처럼 토큰의 memberId를 가지고 해당 API에 접근할 수 있도록 한다면 어떻게 API를 요청한 사람이 본인인지 확인할 수 있는지 잘 모르겠습니다,,

JJimini · 2024-06-26T10:14:44Z

src/main/java/doore/member/application/MemberCommandService.java

+                .id(memberId)
+                .name(newName)
+                .googleId(member.getGoogleId())
+                .email(member.getEmail())
+                .imageUrl(member.getImageUrl())
+                .build();


이 부분은 더 간단하게 작성할 수 있는 방법이 있을 것 같아용~ 업데이트 요청이 있을 때마다 객체를 생성하여 db에 저장하면 성능 측면에서 문제가 있을 것 같아요~!

Study entity나 CurriculumItem entity, Team entity를 참고하시면 좋을 것 같습니다

네 좋습니다!

JJimini · 2024-06-26T10:16:23Z

src/test/java/doore/member/application/MemberCommandServiceTest.java

+    @Test
+    @DisplayName("[성공] 프로필 이름 수정에 성공한다.")
+    void updateMyPageName_프로필_이름_수정에_성공한다_성공() {


👍👍 저희 테스트 이름 작성하는 방식이 조금 복잡한데, 잘하신 것 같아요!

JJimini · 2024-06-26T10:19:16Z

src/test/java/doore/member/application/MemberCommandServiceTest.java

+        String updateName = "요시";
+        memberCommandService.updateMyPageName(member.getId(), updateName);


request를 전달하고, request 내용대로 정보가 바뀌었는지 테스트하는 코드를 짜면 좋을 것 같아요~

JJimini · 2024-06-26T10:26:35Z

src/test/java/doore/member/application/MemberCommandServiceTest.java

+
+        Member findMember = memberRepository.findById(member.getId()).get();


여기서 .get()은 따로 동작하지 않아요~ 혹시 findById를 했을 때, error가 떠서 작성하신거라면, findById는 Optional 객체를 반환하기 때문에 데이터가 존재하지 않는 경우를 위한 예외 처리가 필요해서 그런겁니다!
테스트니까 .orElseThrow()로 처리하시고, 예외 처리가 정상적으로 되는지 확인하는 실패테스트를 작성해보시는 것도 좋을 것 같아요!

JJimini · 2024-06-26T10:27:37Z

src/test/java/doore/restdocs/docs/MemberApiDocsTest.java

+    //todo: docs code
+


todo는 진행하셨으면 삭제해주셔도 될 것 같아요~

JJimini · 2024-06-26T10:30:25Z

src/test/java/doore/restdocs/docs/MemberApiDocsTest.java

+    @Test
+    @DisplayName("[성공] 프로필 이름 수정에 성공한다.")
+    void updateMyPageName_프로필_이름_수정에_성공한다() throws Exception {
+        String requestJson = "{\"newName\":\"요시\"}";
+        doNothing().when(memberCommandService).updateMyPageName(any(), any());
+
+        mockMvc.perform(RestDocumentationRequestBuilders.patch("/profile/name")
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .content(requestJson)
+                        .header(HttpHeaders.AUTHORIZATION, accessToken))
+                .andExpect(status().isNoContent())
+                .andDo(document("update-my-page-name"));
+    }


이 부분도 request를 사용하면 코드가 바뀌겠죠?! 그리고 asciidoc - member.adoc 파일에 추가하면 프론트분들이 보고 작업하신답니다~

lcqff

오류 해결하느라 확인 못했는데 미나님이 너무 리뷰를 잘해주셨네요!!👍👍

lcqff · 2024-06-28T14:43:29Z

src/test/java/doore/restdocs/docs/MemberApiDocsTest.java

+        String requestJson = "{\"newName\":\"요시\"}";
+        doNothing().when(memberCommandService).updateMyPageName(any(), any());


위에서 미나님이 피드백 해주셨듯이 실제 service의 return값은 dto이니 String 대신 결과 응답 dto를 만들면 좋을 거 같아요~!
거기에 맞는 requestFields도 추가해주시고요!

JJimini

수고하셨습니다~ 간단한 코멘트 남겨두었으니 확인 부탁드려요!😄😄

JJimini · 2024-08-02T13:58:37Z

src/main/java/doore/member/api/MemberController.java

+    public ResponseEntity<Void> updateMyPage(@PathVariable Long memberId,
+                                             @RequestBody MemberUpdateRequest memberUpdateRequest,
+                                             @LoginMember Member member) {


파라미터에 final 붙여주시면 좋을 것 같아요!

JJimini · 2024-08-02T14:00:17Z

src/main/java/doore/member/application/MemberCommandService.java

+
+    public void updateMyPage(Long tokenMemberId, Long pathMemberId, MemberUpdateRequest memberUpdateRequest) {


여기도 final 붙여주세요! 다른 코드에도 동일하게 파라미터와 객체에 붙여주시면 좋을 것 같아요~

JJimini · 2024-08-02T14:07:01Z

src/main/java/doore/member/application/MemberCommandService.java

+        if (!tokenMemberId.equals(pathMemberId)) {
+            throw new MemberException(UNAUTHORIZED);
+        }


updateMyPage라는 메서드의 역할과 맞지 않는 것 같아요! 다른 메서드로 빼서 작성해보는건 어떨까요?? 예시로 study domain의 나의 스터디 조회 메서드를 참고하시면 좋을 것 같아요~!

JJimini · 2024-08-02T14:12:55Z

src/main/java/doore/member/application/MemberCommandService.java

+        Member member = validateExistMember(pathMemberId);
+        Member updatedMember = Member.builder()
+                .id(pathMemberId)
+                .name(memberUpdateRequest.getNewName())
+                .googleId(member.getGoogleId())
+                .email(member.getEmail())
+                .imageUrl(member.getImageUrl())
+                .build();
+        memberRepository.save(updatedMember);


public void updateName(String name) { this.name = name; }

member entity에 해당 코드를 작성하시면 builder를 이용해서 객체를 생성하지 않아도 데이터베이스에 값이 변경될 것 같아요~ 저희가 수정할 데이터는 이름 하나니까용

JJimini · 2024-08-02T14:13:56Z

src/main/java/doore/member/application/dto/request/MemberUpdateRequest.java

+public class MemberUpdateRequest {
+    private String newName;
+}


저희는 request를 record로 생성하고 있습니다!
Dto를 Record로 생성하는 것에 대해

JJimini · 2024-08-02T14:19:14Z

src/test/java/doore/member/application/MemberCommandServiceTest.java

+        MemberUpdateRequest request = new MemberUpdateRequest();
+        request.setNewName("요시");


동일합니다!

JJimini · 2024-08-02T14:25:09Z

src/test/java/doore/member/application/MemberCommandServiceTest.java

+        assertThatThrownBy(() -> {
+            memberCommandService.updateMyPage(2L, member.getId(), request);


저희가 권한이라는 단어는 직위와 관련하여 사용하고 있어요~ 권한이 없으면 실패한다라는 네이밍도 좋지만 본인이 아닐 경우 프로필 이름 수정은 실패한다 등의 이름으로 바꿔주면 더 좋을 것 같아요!

그리고 tokenMemberId를 invalid한 값으로 설정해주셨는데, 저희가 검증하는 flow는 클라이언트로부터 받아오는 값이 토큰의 값과 같은지 확인한다 입니다 그래서 invalid한 값은 memberId로 변경하시면 더 좋을 것 같아용

JJimini · 2024-08-02T14:27:46Z

src/test/java/doore/restdocs/docs/MemberApiDocsTest.java

-    @DisplayName("[성공] 사이드바에 들어가는 정보를 조회한다.")
-    void getSideBarInfo_사이드바에_들어가는_정보를_조회한다_성공() throws Exception {
-        //given
-        final Long memberId = 1L;
-        final List<StudyNameResponse> studyResponses = List.of(
-                new StudyNameResponse(1L, "알고리즘 스터디"),
-                new StudyNameResponse(2L, "개발 스터디")
-        );
-        final List<MyTeamsAndStudiesResponse> response = List.of(
-                new MyTeamsAndStudiesResponse(1L, "BDD", studyResponses)
-        );
-        final MemberAndMyTeamsAndStudiesResponse memberAndMyTeamsAndStudiesResponse = new MemberAndMyTeamsAndStudiesResponse(
-                1L, "이름", "프로필사진", response);
-        final PathParametersSnippet pathParameters = pathParameters(
-                parameterWithName("memberId").description("사이드바 정보목록을 조회하는 회원 ID")
-        );
-        final ResponseFieldsSnippet responseFieldsSnippet = responseFields(
-                numberFieldWithPath("id", "멤버 ID"),
-                stringFieldWithPath("name", "멤버 이름"),
-                stringFieldWithPath("imageUrl", "멤버 프로필 경로"),
-                numberFieldWithPath("myTeamsAndStudies[].teamId", "팀 ID"),
-                stringFieldWithPath("myTeamsAndStudies[].teamName", "팀 이름"),
-                numberFieldWithPath("myTeamsAndStudies[].teamStudies[].id", "팀에 포함되는 스터디 id"),
-                stringFieldWithPath("myTeamsAndStudies.[].teamStudies[].name", "팀에 포함되는 스터디 이름")
-        );


다른 api 테스트 코드가 삭제된 것 같아요ㅠㅠ 복구해주시면 좋을 것 같습니다ㅠㅠ

JJimini · 2024-08-02T14:28:15Z

src/test/java/doore/restdocs/docs/MemberApiDocsTest.java

-        //when
-        when(memberQueryService.getSideBarInfo(any(), any())).thenReturn(memberAndMyTeamsAndStudiesResponse);
-
-        //then
-        mockMvc.perform(get("/members/{memberId}", memberId)
-                        .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)
-                        .contentType(MediaType.APPLICATION_JSON))
-                .andExpect(status().isOk())
-                .andDo(document("get-sidebar-info", pathParameters, responseFieldsSnippet));


이것도용ㅠㅠ

JJimini · 2024-08-02T14:35:31Z

src/test/java/doore/restdocs/docs/MemberApiDocsTest.java

+    void updateMyPageName_프로필_이름_수정에_성공한다() throws Exception {
+        String requestJson = "{\"newName\":\"요시\"}";


request를 생성할 수 있다면 코드가 바뀌겠죵?

JJimini

다른 일 일정이 몰아쳐서... 확인이 조금 늦었습니다..! 적다 보니 comment가 많아졌네요ㅠㅠ 확인부탁드립니다...!

JJimini · 2024-08-19T06:55:10Z

src/main/java/doore/member/api/MemberController.java

+    @PatchMapping("/profile/members")
+    public ResponseEntity<Void> updateMyPage(


윗 코멘트 참고해주세요!

JJimini · 2024-08-19T06:57:08Z

src/main/java/doore/member/application/MemberCommandService.java

-    private Member validateExistMember(final Long memberId) {
+    public Member validateExistMember(final Long memberId) {


테스트 때문에 public으로 바꾸신 것 같은데 저희는 각 service에서만 사용되는 메서드의 접근제어자는 private으로 설정합니다! 테스트는 테스트일 뿐인데 테스트를 위해 실제 코드의 접근을 public으로 여는 것은 위험하다고 생각해요~!

JJimini · 2024-08-19T06:58:07Z

src/main/java/doore/member/application/MemberCommandService.java

+    public void updateMyPage(final Long memberId, final MemberUpdateRequest memberUpdateRequest) {
+        Member member = validateExistMember(memberId);


이 부분이 질문 주셨던 부분 같은데 이렇게 된다면 API를 요청한 사람이 본인인지는 어떻게 알까요??

JJimini · 2024-08-19T07:00:13Z

src/main/java/doore/member/application/MemberCommandService.java

+        member.updateName(memberUpdateRequest.newName());
+        memberRepository.save(member);


굿굿! 추가로 member를 따로 저장해 줄 필요 없이 자동으로 변경감지가 된답니다~ 108번 줄은 삭제해도 될 것 같아요~!

JJimini · 2024-08-19T07:02:02Z

src/main/java/doore/member/application/dto/request/MemberUpdateRequest.java

+public record MemberUpdateRequest(
+        String newName


여기서 newName은 Null이 들어오면 어떻게 될까요?? Null이 들어올 수 없도록 처리해야 할 것 같아요~

JJimini · 2024-08-19T07:07:06Z

src/test/java/doore/member/application/MemberCommandServiceTest.java

+    @Test
+    @DisplayName("[실패] 멤버 조회에 실패한다.")
+    void validateExistMember_멤버_조회에_실패한다() {
+        assertThatThrownBy(() -> memberCommandService.validateExistMember(member.getId() + 1))
+                .isInstanceOf(MemberException.class)
+                .hasMessage(NOT_FOUND_MEMBER.errorMessage());
+    }


validatExistMember를 호출해서 테스트하는 것이 아니라, updateMyPage를 호출할 때 잘못된 memberId라면 멤버 조회 에러가 나는지 확인하면 될 것 같아요~

추가로 코드의 가독성을 위해 잘못된 멤버 아이디는 invalidMemberId 와 같이 선언해서 표현합니다!

JJimini · 2024-08-19T07:09:25Z

src/test/java/doore/member/application/MemberCommandServiceTest.java

+    @Test
+    @DisplayName("[성공] 프로필 이름 수정에 성공한다.")
+    void updateMyPage_프로필_이름_수정에_성공한다() {
+        MemberUpdateRequest request = MemberUpdateRequest.builder()
+                .newName("요시")
+                .build();
+
+        memberCommandService.updateMyPage(member.getId(), request);
+
+        Member findMember = memberRepository.findById(member.getId()).orElseThrow();
+        assertThat(findMember.getName()).isEqualTo(request.newName());
+    }


JJimini · 2024-08-19T07:10:45Z

src/test/java/doore/restdocs/docs/MemberApiDocsTest.java

+    void updateMyPageName_프로필_이름_수정에_성공한다() throws Exception {
+


Nit: 개행이 없어도 될 것 같아요~

JJimini · 2024-08-19T07:16:43Z

src/test/java/doore/restdocs/docs/MemberApiDocsTest.java

+        doNothing().when(memberCommandService)
+                .updateMyPage(any(Long.class), any(MemberUpdateRequest.class));


formatter가 깨진 것 같아요~ 그리고 any는 해당 방식처럼 사용하는 것보다 그냥 any()를 사용해주셔도 됩니다! anyLong을 했을 때 오류가 나서 이렇게 하신 것 같은데, anyLong은 문자 그대로 보면 래퍼클래스를 매칭할 것 같지만 기본 자료 타입만 매칭해서 그렇습니다!

JJimini · 2024-08-19T07:18:59Z

src/test/java/doore/restdocs/docs/MemberApiDocsTest.java

+        String requestJson = objectMapper.writeValueAsString(request);
+


저희 asJsonString 이라는 메서드에 정의가 되어 있어서 따로 안쓰셔도 될 것 같아요!

JJimini · 2024-11-05T09:17:40Z

전체 팀원들과의 재기획 후 재개발 예정입니다.

ã�feat: 마이페이지 이름 업데이트

bac8277

#133

jimini1026 added the 🌱 기능 구현 새로운 기능을 구현하는 데 필요한 작업 또는 변경 사항 label Jun 26, 2024

jimini1026 requested review from lcqff, shkisme and JJimini June 26, 2024 06:27

jimini1026 self-assigned this Jun 26, 2024

fix: 비밀번호 재수정

6a4405c

JJimini requested changes Jun 26, 2024

View reviewed changes

JJimini removed the request for review from shkisme June 26, 2024 10:37

lcqff reviewed Jun 28, 2024

View reviewed changes

jimini1026 added 3 commits August 2, 2024 19:36

Merge branch 'develop' into Feature/#133-마이페이지_회원_정보_수정_기능을_제작한다

0e29cc5

refactor: 함수명 수정, DTO 추가

6aa2662

refactor: DTO 생성, 함수명 수정 및 테스트 생성

d1c1c58

jimini1026 requested review from JJimini and lcqff August 2, 2024 12:48

JJimini reviewed Aug 2, 2024

View reviewed changes

refactor: 피드백 반영

077c3f3

JJimini reviewed Aug 19, 2024

View reviewed changes

JJimini closed this Nov 5, 2024

		@PatchMapping("/profile/name")
		public ResponseEntity<Void> updateMyPageName(@RequestBody String newName, @LoginMember Member member) {


		public void updateMyPageName(Long memberId, String newName) {

		Member member = validateExistMember(memberId);
		Member updatedMember = Member.builder()

		String updateName = "요시";
		memberCommandService.updateMyPageName(member.getId(), updateName);


		Member findMember = memberRepository.findById(member.getId()).get();

		String requestJson = "{\"newName\":\"요시\"}";
		doNothing().when(memberCommandService).updateMyPageName(any(), any());


		public void updateMyPage(Long tokenMemberId, Long pathMemberId, MemberUpdateRequest memberUpdateRequest) {

		MemberUpdateRequest request = new MemberUpdateRequest();
		request.setNewName("요시");

		assertThatThrownBy(() -> {
		memberCommandService.updateMyPage(2L, member.getId(), request);

		void updateMyPageName_프로필_이름_수정에_성공한다() throws Exception {
		String requestJson = "{\"newName\":\"요시\"}";

		@PatchMapping("/profile/members")
		public ResponseEntity<Void> updateMyPage(

		private Member validateExistMember(final Long memberId) {
		public Member validateExistMember(final Long memberId) {

		public void updateMyPage(final Long memberId, final MemberUpdateRequest memberUpdateRequest) {
		Member member = validateExistMember(memberId);

		member.updateName(memberUpdateRequest.newName());
		memberRepository.save(member);

		doNothing().when(memberCommandService)
		.updateMyPage(any(Long.class), any(MemberUpdateRequest.class));

		String requestJson = objectMapper.writeValueAsString(request);

Feature/#133-마이페이지_회원_정보_수정_기능을_제작한다 #159

Feature/#133-마이페이지_회원_정보_수정_기능을_제작한다 #159

Conversation

jimini1026 commented Jun 26, 2024 • edited by JJimini Loading

#️⃣ 연관된 이슈

📝 작업 내용

💬 리뷰 요구사항(선택)

JJimini left a comment • 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

Choose a reason for hiding this comment

JJimini Jun 26, 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

lcqff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JJimini 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

JJimini Aug 2, 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

JJimini left a comment • 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

JJimini Aug 19, 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

Choose a reason for hiding this comment

JJimini commented Nov 5, 2024

jimini1026 commented Jun 26, 2024 •

edited by JJimini

Loading

JJimini left a comment •

edited

Loading

JJimini Jun 26, 2024 •

edited

Loading

JJimini Aug 2, 2024 •

edited

Loading

JJimini left a comment •

edited

Loading

JJimini Aug 19, 2024 •

edited

Loading