Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: 어드민 멤버 정보 수정 api 구현 #39

Merged
merged 32 commits into from
Feb 11, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
3a0b42f
feat: 어드민 멤버 수정 api 추가
Sangwook02 Feb 8, 2024
c15036b
feat: 어드민 멤버 수정 request dto 추가
Sangwook02 Feb 8, 2024
3c64a2b
feat: 정규표현식 상수 클래스 추가
Sangwook02 Feb 8, 2024
e2551c6
feat: 어드민 멤버 수정 서비스 추가
Sangwook02 Feb 8, 2024
2eb830f
feat: 멤버 수정 메서드 추가
Sangwook02 Feb 8, 2024
e663303
feat: 존재하지 않는 멤버 예외 추가
Sangwook02 Feb 8, 2024
9f6e251
fix: 닉네임 최소 1자 이상이도록 수정
Sangwook02 Feb 8, 2024
10cb890
fix: RegexConstant 수정
Sangwook02 Feb 8, 2024
4cc0683
style: 개행 제거
Sangwook02 Feb 8, 2024
5131e5e
feat: 검증 메서드 추가
Sangwook02 Feb 8, 2024
4d0ca15
refactor: update 메서드 수정
Sangwook02 Feb 8, 2024
f9c6e98
feat: ErrorCode 추가
Sangwook02 Feb 8, 2024
87db165
feat: 삭제된 멤버 수정 못하도록 검증 추가
Sangwook02 Feb 8, 2024
dab588e
fix: 필드명 변경
Sangwook02 Feb 8, 2024
1a4ad96
chore: merge conflict resolve
Sangwook02 Feb 9, 2024
768cc7b
refactor: if문 제거
Sangwook02 Feb 9, 2024
6377e53
refactor: NotBlank로 변경
Sangwook02 Feb 9, 2024
1375307
feat: dto 검증 예외 처리 메서드 생성
Sangwook02 Feb 9, 2024
c6df1bd
test: 탈퇴한 회원 정보 수정 시 예외 처리 테스트 추가
Sangwook02 Feb 9, 2024
42d72ce
refactor: 탈퇴 여부 확인을 MemberStatus이 처리
Sangwook02 Feb 9, 2024
e15cf62
fix: 메서드명 수정
Sangwook02 Feb 11, 2024
3e7501f
remove: 중복 null-check 제거
Sangwook02 Feb 11, 2024
f5400ef
fix: 메서드명 수정
Sangwook02 Feb 11, 2024
eeadd68
refactor: 멤버 상태 검증 시 차단 여부도 확인
Sangwook02 Feb 11, 2024
f94c844
refactor: db에 하이픈 없이 저장하도록 수정
Sangwook02 Feb 11, 2024
b8d8fa0
fix: 메서드명 수정
Sangwook02 Feb 11, 2024
746b3f2
refactor: 검증 메서드를 수정 메서드 내부로 이동
Sangwook02 Feb 11, 2024
5120ace
refactor: ErrorCode의 이름을 Response 내에서 처리하도록 수정
Sangwook02 Feb 11, 2024
b570260
refactor: 수정 로직 수정
Sangwook02 Feb 11, 2024
d40429a
remove: 사용하지 않는 import 제거
Sangwook02 Feb 11, 2024
4f0c248
Merge branch 'develop' into feature/24-member-info-modification
Sangwook02 Feb 11, 2024
47c4120
style: spotless apply
Sangwook02 Feb 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@

import com.gdschongik.gdsc.domain.member.application.MemberService;
import com.gdschongik.gdsc.domain.member.dto.request.MemberQueryRequest;
import com.gdschongik.gdsc.domain.member.dto.request.MemberUpdateRequest;
import com.gdschongik.gdsc.domain.member.dto.response.MemberFindAllResponse;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

Expand All @@ -31,4 +35,11 @@ public ResponseEntity<Void> withdrawMember(@PathVariable Long memberId) {
memberService.withdrawMember(memberId);
return ResponseEntity.ok().build();
}

@PutMapping("/{memberId}")
public ResponseEntity<Void> withdrawMember(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메서드 이름이 잘못되었습니다.

@PathVariable Long memberId, @Valid @RequestBody MemberUpdateRequest request) {
memberService.updateMember(memberId, request);
return ResponseEntity.ok().build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
import com.gdschongik.gdsc.domain.member.dao.MemberRepository;
import com.gdschongik.gdsc.domain.member.domain.Member;
import com.gdschongik.gdsc.domain.member.dto.request.MemberQueryRequest;
import com.gdschongik.gdsc.domain.member.dto.request.MemberUpdateRequest;
import com.gdschongik.gdsc.domain.member.dto.response.MemberFindAllResponse;
import com.gdschongik.gdsc.global.exception.CustomException;
import com.gdschongik.gdsc.global.exception.ErrorCode;
import lombok.RequiredArgsConstructor;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
Expand All @@ -30,4 +32,13 @@ public void withdrawMember(Long memberId) {
Member member = memberRepository.findById(memberId).orElseThrow(() -> new CustomException(MEMBER_NOT_FOUND));
member.withdraw();
}

@Transactional
public void updateMember(Long memberId, MemberUpdateRequest request) {
Member member =
memberRepository.findById(memberId).orElseThrow(() -> new CustomException(ErrorCode.MEMBER_NOT_FOUND));

member.validateStatus();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어떤 Status임을 검증하는지 궁금한데, "삭제된 유저가 아님을 검증"으로 바꾸는건 어떨까요?
일부러 "상태를 검증한다"로 어떤 상태를 검증하는 것인지 숨긴거라면,
"업데이트가 가능한 상태인지 검증"으로 바꾸는건 어떤가요?

저만의 생각이라 혹시 의도가 있는 거라면 알려주세요!

Copy link
Member Author

@Sangwook02 Sangwook02 Feb 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateStatusUpdatable로 수정했습니다.
이와 별개로 validateStatusUpdatablemember.updateMemberInfo안에서 호출하는게 맞을지 아니면 밖에서 먼저 호출하는게 맞을지 고민되는데 어떻게 생각하시나요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우선은 member.updateMemberInfo안에서 호출하도록 수정했습니다

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 Member가 업데이트를 해줄 수 있는 상태인지 확인하는거라 member.updateMemberInfo 이런 형태가 좋습니다 ㅎㅎ

member.updateMemberInfo(request);
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package com.gdschongik.gdsc.domain.member.domain;

import static com.gdschongik.gdsc.global.common.constant.RegexConstant.*;
import static com.gdschongik.gdsc.global.exception.ErrorCode.*;

import com.gdschongik.gdsc.domain.common.model.BaseTimeEntity;
import com.gdschongik.gdsc.domain.member.dto.request.MemberUpdateRequest;
import com.gdschongik.gdsc.global.exception.CustomException;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
Expand Down Expand Up @@ -100,4 +102,70 @@ public void withdraw() {
public boolean isDeleted() {
return this.status.isDeleted();
}

public void updateMemberInfo(MemberUpdateRequest request) {
updateStudentId(request.studentId());
updateName(request.name());
updatePhone(request.phone());
updateDepartment(request.department());
updateEmail(request.email());
updateDiscordUsername(request.discordUsername());
updateNickname(request.nickname());
}

private void updateStudentId(String studentId) {
validateStringNotNull(studentId);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

업데이트 시 not null 정책은 dto 단에서 검증하는 것으로 하면 어떨까요?
도메인에서 입력값 유효성 검증까지 처리하게 되면 로직이 너무 복잡해지는 것도 있어서,
입력값 신택스보다는 시멘틱스 위주로 처리하는 쪽으로 가면 좋을 것 같습니다.

validateRegex(studentId, STUDENT_ID);
this.studentId = studentId;
}

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

private void updatePhone(String phone) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전화번호는 굳이 하이픈 붙여서 저장할 필요는 없을 것 같아요.
DB에는 제거해서 넣고 나중에 프론트로 내려줄 때 변환해주든 하는 게 나을 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

db에 넣을 때 제거해서 넣으면 좋은 점이 있나요?
의도가 무엇인지 궁금합니다

Copy link
Member

@uwoobeat uwoobeat Feb 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

010-1234-5678 에 대해서 4567 이라는 키워드로 검색한다고 칩시다.
사용자는 010-1234-5678 이 검색될 것을 기대하지만 실제로는 4-5674567 이 일치하지 않기 떄문에 검색되지 않습니다.
해당 문제를 방지하기 위해서는 하이픈 없이 저장해야 합니다.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

검색하려면 필요하겠네요👍
수정하겠습니다!

validateStringNotNull(phone);
validateRegex(phone, PHONE);
this.phone = phone;
}

private void updateDepartment(String department) {
validateStringNotNull(department);
this.department = department;
}

private void updateEmail(String email) {
validateStringNotNull(email);
this.email = email;
}

private void updateDiscordUsername(String discordUsername) {
validateStringNotNull(discordUsername);
this.discordUsername = discordUsername;
}

private void updateNickname(String nickname) {
validateStringNotNull(nickname);
validateRegex(nickname, NICKNAME);
this.nickname = nickname;
}

private void validateStringNotNull(String value) {
if (value == null) {
throw new CustomException(METHOD_ARGUMENT_NULL);
}
}

private void validateRegex(String value, String regex) {
if (!value.matches(regex)) {
throw new CustomException(REGEX_VIOLATION);
}
}

public void validateStatus() {
if (isDeleted()) {
throw new CustomException(MEMBER_DELETED);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.gdschongik.gdsc.domain.member.dto.request;

import static com.gdschongik.gdsc.global.common.constant.RegexConstant.*;

import jakarta.validation.constraints.Email;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.Pattern;

public record MemberUpdateRequest(
@NotBlank @Pattern(regexp = STUDENT_ID, message = "학번은 " + STUDENT_ID + " 형식이어야 합니다.") String studentId,
@NotBlank String name,
@NotBlank @Pattern(regexp = PHONE, message = "전화번호는 " + PHONE + " 형식이어야 합니다.") String phone,
@NotBlank String department,
@NotBlank @Email String email,
@NotBlank String discordUsername,
@NotBlank @Pattern(regexp = NICKNAME, message = "닉네임은 " + NICKNAME + " 형식이어야 합니다.") String nickname) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.gdschongik.gdsc.global.common.constant;

public class RegexConstant {
public static final String STUDENT_ID = "^[A-C]{1}[0-9]{6}$";
public static final String PHONE = "^010-[0-9]{4}-[0-9]{4}$";
public static final String NICKNAME = "[ㄱ-ㅣ가-힣]{1,6}$";

private RegexConstant() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,20 @@
@AllArgsConstructor
public enum ErrorCode {
INTERNAL_SERVER_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "서버 에러입니다."),
METHOD_ARGUMENT_NULL(HttpStatus.BAD_REQUEST, "인자는 null이 될 수 없습니다."),
METHOD_ARGUMENT_NOT_VALID(HttpStatus.BAD_REQUEST, "인자가 유효하지 않습니다."),
REGEX_VIOLATION(HttpStatus.BAD_REQUEST, "정규표현식을 위반했습니다."),

// Jwt
INVALID_JWT_TOKEN(HttpStatus.UNAUTHORIZED, "유효하지 않은 JWT 토큰입니다."),
EXPIRED_JWT_TOKEN(HttpStatus.UNAUTHORIZED, "만료된 JWT 토큰입니다."),

// Parameter
INVALID_QUERY_PARAMETER(HttpStatus.BAD_REQUEST, "잘못된 쿼리 파라미터입니다."),

// Member
MEMBER_NOT_FOUND(HttpStatus.NOT_FOUND, "존재하지 않는 회원입니다."),
MEMBER_DELETED(HttpStatus.CONFLICT, "탈퇴한 회원입니다.");
MEMBER_DELETED(HttpStatus.CONFLICT, "탈퇴한 회원입니다."),

// Parameter
INVALID_QUERY_PARAMETER(HttpStatus.BAD_REQUEST, "잘못된 쿼리 파라미터입니다.");

private final HttpStatus status;
private final String message;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ public record ErrorResponse(String errorCodeName, String errorMessage) {
public static ErrorResponse of(ErrorCode errorCode) {
return new ErrorResponse(errorCode.name(), errorCode.getMessage());
}

public static ErrorResponse of(String errorCodeName, String errorMessage) {
return new ErrorResponse(errorCodeName, errorMessage);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GlobalExceptionHandler에서 사용하는 방식을 보긴 했는데,
ErrorCode와 Message를 받지 않고, 이름을 String으로 바로 받는 이유가 궁금합니다!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다른 handler 메서드에서는
return ResponseEntity .status(ErrorCode.INTERNAL_SERVER_ERROR.getStatus()) .body(ErrorResponse.of(ErrorCode.INTERNAL_SERVER_ERROR));와 같이 ErrorCode를 바로 넘겼는데 해당 handler 메서드에서는 Validation 어노테이션이 넘겨주는 default message를 이용하려고 했습니다.

더 좋은 방법이 있을까요?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 취향 차이일 수 있는데,
저는 만약에 Message를 받아서 사용하고 싶은 것이라면 아래와 같은 형태가 어떤가 했어요

public static ErrorResponse of(ErrorCode errorCode, String errorMessage) {
        return new ErrorResponse(errorCode.name(), errorMessage);

errorCodeName도 자유롭게 사용하려면 지금 짜주신 방법이 더 좋은 것 같구요 ㅎㅎ

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 제가 잘못 이해했었네요
말씀해주신 방법이 GlobalExceptionHandler 쪽에서 더 깔끔할 것 같아서 수정했습니다
감사합니다!

}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package com.gdschongik.gdsc.global.exception;

import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatusCode;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;
import org.springframework.web.context.request.WebRequest;
import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;

@Slf4j
Expand All @@ -22,4 +26,13 @@ public ResponseEntity<ErrorResponse> handleException(Exception e) {
return ResponseEntity.status(ErrorCode.INTERNAL_SERVER_ERROR.getStatus())
.body(ErrorResponse.of(ErrorCode.INTERNAL_SERVER_ERROR));
}

@Override
protected ResponseEntity<Object> handleMethodArgumentNotValid(
MethodArgumentNotValidException e, HttpHeaders headers, HttpStatusCode status, WebRequest request) {
log.error("METHOD_ARGUMENT_NOT_VALID : {}", e.getMessage(), e);
String errorMessage = e.getBindingResult().getAllErrors().get(0).getDefaultMessage();
return ResponseEntity.status(status.value())
.body(ErrorResponse.of(ErrorCode.METHOD_ARGUMENT_NOT_VALID.name(), errorMessage));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.gdschongik.gdsc.domain.member.application;

import static org.assertj.core.api.Assertions.*;

import com.gdschongik.gdsc.domain.member.dao.MemberRepository;
import com.gdschongik.gdsc.domain.member.domain.Member;
import com.gdschongik.gdsc.domain.member.dto.request.MemberUpdateRequest;
import com.gdschongik.gdsc.global.exception.CustomException;
import com.gdschongik.gdsc.global.exception.ErrorCode;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;

@SpringBootTest
class MemberServiceTest {
@Autowired
private MemberRepository memberRepository;

@Autowired
private MemberService memberService;

@Test
void status가_DELETED라면_예외_발생() {
// given
Member member = Member.createGuestMember("oAuthId");
member.withdraw();
memberRepository.save(member);

// when & then
MemberUpdateRequest requestBody = new MemberUpdateRequest(
"A111111", "name", "010-1234-5678", "department", "[email protected]", "discordUsername", "한글");
assertThatThrownBy(() -> memberService.updateMember(member.getId(), requestBody))
.isInstanceOf(CustomException.class)
.hasMessage(ErrorCode.MEMBER_DELETED.getMessage());
}
}
Loading