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

회원의 닉네임을 일정주기를 통해 변경할 수 있도록 구현 #498

Merged
merged 7 commits into from
Sep 9, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import com.votogether.domain.member.entity.vo.Gender;
import com.votogether.domain.member.entity.vo.Nickname;
import com.votogether.domain.member.entity.vo.SocialType;
import com.votogether.domain.member.exception.MemberExceptionType;
import com.votogether.global.exception.BadRequestException;
import jakarta.persistence.Column;
import jakarta.persistence.Embedded;
import jakarta.persistence.Entity;
Expand All @@ -15,6 +17,7 @@
import jakarta.persistence.Id;
import jakarta.persistence.Index;
import jakarta.persistence.Table;
import java.time.LocalDateTime;
import lombok.AccessLevel;
import lombok.Builder;
import lombok.EqualsAndHashCode;
Expand Down Expand Up @@ -77,10 +80,21 @@ public static Member from(final KakaoMemberResponse response) {
.build();
}

public void changeNickname(final String nickname) {
public void changeNicknameByCycle(final String nickname, final Long days) {
if (nickname.startsWith(INITIAL_NICKNAME_PREFIX)) {
throw new BadRequestException(MemberExceptionType.INVALID_NICKNAME_LETTER);
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2
익명의손님은 초기 닉네임이름이라 유효하지않는건 아닌거같아서
초기 닉네임으로는 지정할수없습니다 처럼 예외 메시지를 새로 추가하는건 어떤가요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

P2
초기 닉네임에 대한 제한을 걸어 해당 닉네임으로 닉네임 변경 여부를 확인하는 방법으로 변경하셨네요 🤓

초기 닉네임이라 유효하지 않은 닉네임이라는 예외 메시지를 수정하거나, 닉네임 변경 여부 확인을 위해 리프레쉬 토큰을 저장하는 것처럼 레디스에 마지막 변경 시간을 저장하는 방법도 고려해볼 수 있을 것 같아요. 최대 2주기 때문에 2주 후에는 사라지는 데이터이고, 레디스가 만약 다운되었을 때 발생할 수 있는 이펙트로는 닉네임 변경이 한번 가능해진다 정도일 것 같아서 고려해볼 수 있을 것 같은데 정책에 맞지 않는 사용자의 행동이 발생할 수 있다는 단점도 있을 것 같네요 🥲

확실히 안정성을 보장하려면 DB에 새로운 테이블이나 컬럼을 추가하는 방법도 생각해볼 수 있을 것 같아요 :)

Copy link
Collaborator Author

@jeomxon jeomxon Sep 9, 2023

Choose a reason for hiding this comment

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

루쿠가 말씀하신대로 수정해볼게요!
단순히 유효하지 않다고 생각했었는데, 초기 닉네임으로 지정이 되니 유효하지 않다라는 메시지보다는
별도의 메시지로 분리되면 좋을 것 같네요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

레디스를 사용하는 방식에 대해서는 생각해보지 않았는데 좋은 의견 감사합니다 :)
확실히 말씀해주신 것처럼 정책이고, 사용자에게 안내가 되기 때문에 갑자기 닉네임이 변경된다면 의문이 들 것 같긴해요!
그래서 db에 추가하는 방법도 생각해봤는데, 초기에 한번 사용되고 그 이후에는 추가적으로 사용되지 않을 필드라는 생각이 들어서
최대한 db에 저장하는 방법을 가져가고 싶지 않았어요.
그래서 초기 닉네임에 제한을 거는 방법을 사용했습니다!

레디스 설정한 pr이 더 늦게 merge될 것 같기도 해서 레디스에 적용하기보다는 지금 형식을 그대로 가져갈 생각입니다.
또한 현재의 구조를 확장한다고 가정하면 초기 닉네임 prefix에 대한 제한 뿐만아니라 부적절한 언어들에 대해 필터링할 때 확장하기 쉬울 것 같아요.

}
if (isNotPassedChangingCycle(days)) {
throw new BadRequestException(MemberExceptionType.NOT_PASSED_NICKNAME_CHANGING_CYCLE);
}
this.nickname = new Nickname(nickname);
}

private boolean isNotPassedChangingCycle(final Long days) {
return (this.nickname.nonStartsWith(INITIAL_NICKNAME_PREFIX)) &&
(this.getUpdatedAt().isAfter(LocalDateTime.now().minusDays(days)));
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

P2
isNotPassedChangingCycle 메서드에 프리픽스로 시작하는 닉네임인지를 검증하는게 어색하다고 생각되는데
&& 로 연결 되어있는 두 boolen 식을 각각 메서드로 추출하고
추출한 메서드를 changeNicknameByCycle에서 &&로 연결한는건 어떤가요??

Suggested change
public void changeNicknameByCycle(final String nickname, final Long days) {
if (nickname.startsWith(INITIAL_NICKNAME_PREFIX)) {
throw new BadRequestException(MemberExceptionType.INVALID_NICKNAME_LETTER);
}
if (isNotPassedChangingCycle(days)) {
throw new BadRequestException(MemberExceptionType.NOT_PASSED_NICKNAME_CHANGING_CYCLE);
}
this.nickname = new Nickname(nickname);
}
private boolean isNotPassedChangingCycle(final Long days) {
return (this.nickname.nonStartsWith(INITIAL_NICKNAME_PREFIX)) &&
(this.getUpdatedAt().isAfter(LocalDateTime.now().minusDays(days)));
}
public void changeNicknameByCycle(final String nickname, final Long days) {
if (nickname.startsWith(INITIAL_NICKNAME_PREFIX)) {
throw new BadRequestException(MemberExceptionType.INVALID_NICKNAME_LETTER);
}
if (isNotPassedChangingCycle(days) && ! isInitalNickname()) {
throw new BadRequestException(MemberExceptionType.NOT_PASSED_NICKNAME_CHANGING_CYCLE);
}
this.nickname = new Nickname(nickname);
}
private boolean isNotPassedChangingCycle(final Long days) {
return this.getUpdatedAt().isAfter(LocalDateTime.now().minusDays(days));
}
private boolean isInitalNickname() {
return this.nickname.startsWith(INITIAL_NICKNAME_PREFIX)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋은 제안인 것 같아요 ㅎㅎ
안그래도 함수의 역할이 명확하지 않다는 생각만하고 그냥 넘어갔던 것 같은데 짚어주셔서 감사합니다!
말씀하신 방향대로 수정해볼게요 :)

public void changeNicknameByReport() {
final String reportedNickname = "Pause1" + RandomStringUtils.random(9, true, true);
this.nickname = new Nickname(reportedNickname);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,8 @@ private void validateNickname(final String nickname) {
}
}

public boolean nonStartsWith(final String prefix) {
return !this.value.startsWith(prefix);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public enum MemberExceptionType implements ExceptionType {
INVALID_AGE(804, "존재할 수 없는 연령입니다."),
ALREADY_ASSIGNED_GENDER(805, "이미 성별이 할당되어 있습니다."),
ALREADY_ASSIGNED_BIRTH_YEAR(806, "이미 출생년도가 할당되어 있습니다."),
NOT_PASSED_NICKNAME_CHANGING_CYCLE(807, "최소 닉네임 변경주기가 지나지 않았습니다."),
;

private final int code;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
@Service
public class MemberService {

private static final Long NICKNAME_CHANGING_CYCLE = 14L;

private final MemberRepository memberRepository;
private final MemberCategoryRepository memberCategoryRepository;
private final PostRepository postRepository;
Expand Down Expand Up @@ -68,7 +70,7 @@ public MemberInfoResponse findMemberInfo(final Member member) {
@Transactional
public void changeNickname(final Member member, final String nickname) {
validateExistentNickname(nickname);
member.changeNickname(nickname);
member.changeNicknameByCycle(nickname, NICKNAME_CHANGING_CYCLE);
}

private void validateExistentNickname(final String nickname) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package com.votogether.domain.member.entity;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import com.votogether.domain.member.entity.vo.Gender;
import com.votogether.domain.member.entity.vo.SocialType;
import com.votogether.global.exception.BadRequestException;
import java.time.LocalDateTime;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.springframework.test.util.ReflectionTestUtils;

class MemberTest {

@Nested
@DisplayName("닉네임을 주기에 따라 변경하는 경우")
class ChangeNicknameByCycle {

@Test
@DisplayName("15일전에 변경된 닉네임은 14일 주기로 변경할 때 성공적으로 변경된다.")
void success() {
// given
Member member = Member.builder()
.nickname("저문")
.gender(Gender.MALE)
.birthYear(1966)
.socialId("abc123")
.socialType(SocialType.KAKAO)
.build();

LocalDateTime now = LocalDateTime.now().minusDays(15L);
ReflectionTestUtils.setField(member, "createdAt", now);
ReflectionTestUtils.setField(member, "updatedAt", now.plusHours(1L));

// when
member.changeNicknameByCycle("저라니", 14L);

// then
assertThat(member.getNickname()).isEqualTo("저라니");
}

@Test
@DisplayName("13일 전에 변경된 닉네임은 14일 주기로 변경하더라도 한번도 닉네임을 변경하지 않았다면 성공적으로 변경된다.")
void successFirstChange() {
// given
Member member = Member.builder()
.nickname("익명의손님fFp4vAgX2d")
.gender(Gender.MALE)
.birthYear(1966)
.socialId("abc123")
.socialType(SocialType.KAKAO)
.build();

LocalDateTime now = LocalDateTime.now().minusDays(13L);
ReflectionTestUtils.setField(member, "createdAt", now);
ReflectionTestUtils.setField(member, "updatedAt", now.plusHours(1L));

// when
member.changeNicknameByCycle("저라니", 14L);

// then
assertThat(member.getNickname()).isEqualTo("저라니");
}

@Test
@DisplayName("13일 전에 변경된 닉네임은 14일 주기로 변경할 때 변경에 실패한다.")
void fail() {
// given
Member member = Member.builder()
.nickname("저문")
.gender(Gender.MALE)
.birthYear(1966)
.socialId("abc123")
.socialType(SocialType.KAKAO)
.build();

LocalDateTime createdTime = LocalDateTime.now().minusDays(20L);
LocalDateTime updatedTime = LocalDateTime.now().minusDays(13L);
ReflectionTestUtils.setField(member, "createdAt", createdTime);
ReflectionTestUtils.setField(member, "updatedAt", updatedTime);

// when, then
assertThatThrownBy(() -> member.changeNicknameByCycle("저라니", 14L))
.isInstanceOf(BadRequestException.class)
.hasMessage("최소 닉네임 변경주기가 지나지 않았습니다.");
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,24 @@ void register() {
class ChangeNickname {

@Test
@DisplayName("주어질 때 정상적으로 닉네임을 변경한다.")
@DisplayName("한번도 변경되지 않았다면 닉네임 변경 주기에 상관없이 닉네임을 변경한다.")
void changeNickname() {
// given
Member member = Member.builder()
.nickname("익명의손님fFp4vAgX2d")
.gender(Gender.MALE)
.birthYear(1966)
.socialId("abc123")
.socialType(SocialType.KAKAO)
.build();
String newNickname = "jeomxon";
Member member = memberRepository.save(MemberFixtures.FEMALE_30.get());
Member savedMember = memberRepository.save(member);

// when
memberService.changeNickname(member, newNickname);
memberService.changeNickname(savedMember, newNickname);

// then
assertThat(member.getNickname()).isEqualTo(newNickname);
assertThat(savedMember.getNickname()).isEqualTo(newNickname);
}

@ParameterizedTest
Expand Down Expand Up @@ -136,6 +143,27 @@ void changeNicknameEqualToPrevious() {
.hasMessage("이미 중복된 닉네임이 존재합니다.");
}

@Test
@DisplayName("최초 닉네임을 변경한 후 닉네임 변경 주기가 지나지 않았다면 예외가 발생한다.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

닉네임 변경 이후 14일이 지나면 닉네임이 변경가능한지 여부도 테스트해볼 수 없을까요? 😎

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

한번 작성해보겠습니다..!

void changeNicknameThrowsExceptionNotPassedChangingCycle() {
// given
Member member = Member.builder()
.nickname("익명의손님fFp4vAgX2d")
.gender(Gender.MALE)
.birthYear(1966)
.socialId("abc123")
.socialType(SocialType.KAKAO)
.build();
Member savedMember = memberRepository.save(member);

memberService.changeNickname(savedMember, "저문");

// when, then
assertThatThrownBy(() -> memberService.changeNickname(savedMember, "저라니"))
.isInstanceOf(BadRequestException.class)
.hasMessage("최소 닉네임 변경주기가 지나지 않았습니다.");
}

}

@Nested
Expand Down