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

성별, 나이 수정 기능 구현 #339

Merged
merged 6 commits into from
Aug 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
@@ -1,5 +1,6 @@
package com.votogether.domain.member.controller;

import com.votogether.domain.member.dto.MemberDetailRequest;
import com.votogether.domain.member.dto.MemberInfoResponse;
import com.votogether.domain.member.dto.MemberNicknameUpdateRequest;
import com.votogether.domain.member.entity.Member;
Expand Down Expand Up @@ -51,6 +52,20 @@ public ResponseEntity<Void> changeNickname(
return ResponseEntity.ok().build();
}

@Operation(summary = "회원 상세 정보 변경", description = "회원의 로그인 직후에 상세정보를 설정한다.")
@ApiResponses({
@ApiResponse(responseCode = "200", description = "상세정보 변경 성공"),
@ApiResponse(responseCode = "400", description = "올바르지 않은 상세정보 변경 요청")
})
@PatchMapping("/me/detail")
public ResponseEntity<Void> updateDetails(
@Valid @RequestBody final MemberDetailRequest request,
@Auth final Member member
) {
memberService.updateDetails(request, member);
return ResponseEntity.ok().build();
}

@Operation(summary = "회원 탈퇴", description = "서비스를 탈퇴한다.")
@ApiResponses({
@ApiResponse(responseCode = "200", description = "회원 탈퇴 성공"),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.votogether.domain.member.dto;

import com.votogether.domain.member.entity.Gender;
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.Max;
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.NotNull;

@Schema(description = "회원 상세 정보 수정 요청")
public record MemberDetailRequest(
@Schema(description = "성별", example = "MALE")
Gender gender,

@Schema(description = "출생년도", example = "2000")
@NotNull(message = "출생년도는 빈 값일 수 없습니다.")
@Min(value = 1800, message = "출생년도는 1800년 이상부터 가능합니다.")
@Max(value = 2100, message = "출생년도는 2100년 이하만 가능합니다.")
Integer birthYear
) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.Index;
import jakarta.persistence.Table;
import lombok.AccessLevel;
import lombok.Builder;
import lombok.EqualsAndHashCode;
Expand All @@ -18,6 +20,7 @@
import lombok.ToString;
import org.apache.commons.lang3.RandomStringUtils;

@Table(indexes = {@Index(columnList = "socialId, socialType")})
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@EqualsAndHashCode(of = {"id"})
@ToString
Expand Down Expand Up @@ -80,6 +83,11 @@ public void changeNicknameByReport() {
this.nickname = new Nickname(reportedNickname);
}

public void updateDetails(final Gender gender, final Integer birthYear) {
this.gender = gender;
this.birthYear = birthYear;
}

public String getNickname() {
return this.nickname.getValue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.Table;
import jakarta.persistence.UniqueConstraint;
import lombok.AccessLevel;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Table(uniqueConstraints = {@UniqueConstraint(columnNames = {"member_id", "category_id"})})
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
@Entity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class Nickname {
private static final int MINIMUM_NICKNAME_LENGTH = 2;
private static final int MAXIMUM_NICKNAME_LENGTH = 15;

@Column(name = "nickname", length = 15, unique = true, nullable = false)
@Column(name = "nickname", length = 20, unique = true, nullable = false)
private String value;

public Nickname(final String nickname) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ public enum MemberExceptionType implements ExceptionType {
ALREADY_EXISTENT_NICKNAME(802, "이미 중복된 닉네임이 존재합니다."),
NONEXISTENT_MEMBER(803, "해당 회원이 존재하지 않습니다."),
INVALID_AGE(804, "존재할 수 없는 연령입니다."),
ALREADY_ASSIGNED_GENDER(805, "이미 성별이 할당되어 있습니다."),
ALREADY_ASSIGNED_BIRTH_YEAR(806, "이미 출생년도가 할당되어 있습니다."),
;

private final int code;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.votogether.domain.member.service;

import com.votogether.domain.member.dto.MemberDetailRequest;
import com.votogether.domain.member.dto.MemberInfoResponse;
import com.votogether.domain.member.entity.Member;
import com.votogether.domain.member.entity.Nickname;
Expand Down Expand Up @@ -64,6 +65,21 @@ private void validateExistentNickname(final String nickname) {
}
}

@Transactional
public void updateDetails(final MemberDetailRequest request, final Member member) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1
기획적으로 출생년도와 성별은 한번 입력하고 수정이 불가능한 것으로 알고 있습니다!

이미 출생년도와 성별이 지정되어 있는지에 대한 검증이 필요할 것 같아요 😙

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

최상단에서 검증해주면 된다는 생각에 여기서 하게된다면 두번의 조회를 통한 중복 검증이 발생하게 되어서 과연 필요할까?에 대해서 생각해봤습니다.
이런 관점에서 봤을 때도 검증이 꼭 필요하다고 생각하시나요?

Copy link
Collaborator

@woo-chang woo-chang Aug 11, 2023

Choose a reason for hiding this comment

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

해당 API는 성별, 출생년도가 null인 회원도 호출가능하게 해야 null인 회원이 수정할 수 있을 것 같아요 :)

따라서 최상단에서 검증하는 로직이 빠져야할 것 같은데 제가 이해한게 맞을까요?! 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

최상단에서 검증하는게 requestdto에서 하는 검증인가요?
requestdto에서 빈값인지 검증하고
해당 메서드에서는 회원의 성별, 출생년도가 null일때만 수정할 수 있도록 조건을 걸어줘야할거같아요.


해당 API는 성별, 출생년도가 null인 회원도 호출가능하게 해야 null인 회원이 수정할 수 있을 것 같아요 :)

-> 아니면 argumentResolver에서 성별, 출생년도가 매번 null인지 아닌지 확인하는 것보다 투표하는 메서드에서 성별, 출생년도가 null인 회원은 투표하지 못하게 막는건 어떤것 같나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

레전드 생각을 하고 있었네요!
다들 의견주셔서 감사합니다
수정해볼게요~

validateExistentDetails(member);
member.updateDetails(request.gender(), request.birthYear());
}

private void validateExistentDetails(final Member member) {
if (member.getGender() != null) {
throw new BadRequestException(MemberExceptionType.ALREADY_ASSIGNED_GENDER);
}
if (member.getBirthYear() != null) {
throw new BadRequestException(MemberExceptionType.ALREADY_ASSIGNED_BIRTH_YEAR);
}
}

@Transactional
public void deleteMember(final Member member) {
final Member existentMember = memberRepository.findById(member.getId())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.Table;
import jakarta.persistence.UniqueConstraint;
import lombok.AccessLevel;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Table(uniqueConstraints = {@UniqueConstraint(columnNames = {"post_id", "category_id"})})
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
@Entity
Expand All @@ -24,7 +27,7 @@ public class PostCategory extends BaseEntity {
private Long id;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(nullable = false)
@JoinColumn(name = "post_id", nullable = false)
private Post post;

@ManyToOne(fetch = FetchType.LAZY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class PostContentImage extends BaseEntity {
@JoinColumn(name = "post_id", nullable = false)
private Post post;

@Column
@Column(nullable = false)
private String imageUrl;

@Builder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.OneToMany;
import jakarta.persistence.Table;
import jakarta.persistence.UniqueConstraint;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
Expand All @@ -21,6 +23,7 @@
import lombok.Getter;
import lombok.NoArgsConstructor;

@Table(uniqueConstraints = {@UniqueConstraint(columnNames = {"post_id", "sequence"})})
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
@Entity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

@Table(
uniqueConstraints = {@UniqueConstraint(columnNames = {"member_id", "reportType", "targetId"})},
indexes = {@Index(columnList = "reportType, targetId")}
indexes = {@Index(columnList = "targetId, reportType")}
)
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
Expand All @@ -38,7 +38,7 @@ public class Report extends BaseEntity {
private Member member;

@Enumerated(value = EnumType.STRING)
@Column(length = 10, nullable = false)
@Column(length = 20, nullable = false)
private ReportType reportType;

@Column(nullable = false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.Table;
import jakarta.persistence.UniqueConstraint;
import lombok.AccessLevel;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Table(uniqueConstraints = {@UniqueConstraint(columnNames = {"member_id", "post_option_id"})})
@Entity
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Getter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.mockito.BDDMockito.given;
import static org.mockito.BDDMockito.willDoNothing;

import com.votogether.domain.member.dto.MemberDetailRequest;
import com.votogether.domain.member.dto.MemberInfoResponse;
import com.votogether.domain.member.dto.MemberNicknameUpdateRequest;
import com.votogether.domain.member.entity.Gender;
Expand All @@ -23,6 +24,9 @@
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.NullSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.HttpHeaders;
Expand Down Expand Up @@ -140,6 +144,89 @@ void changeNicknameFail() throws Exception {

}

@Nested
@DisplayName("회원 정보 수정을 할 때")
class UpdateDetails {

@Test
@DisplayName("요청이 정상적이라면 성공한다.")
void updateDetails() throws Exception {
// given
Member member = MemberFixtures.MALE_20.get();
MemberDetailRequest request = new MemberDetailRequest(Gender.MALE, 2000);

TokenPayload tokenPayload = new TokenPayload(1L, 1L, 1L);
given(tokenProcessor.resolveToken(anyString())).willReturn("token");
given(tokenProcessor.parseToken(anyString())).willReturn(tokenPayload);
given(memberService.findById(anyLong())).willReturn(MemberFixtures.FEMALE_20.get());

willDoNothing().given(memberService).updateDetails(request, member);

// when
RestAssuredMockMvc
.given().log().all()
.headers(HttpHeaders.AUTHORIZATION, "Bearer token")
.contentType(ContentType.JSON)
.body(request)
.when().patch("/members/me/detail")
.then().log().all()
.statusCode(HttpStatus.OK.value());
}

@ParameterizedTest
@ValueSource(ints = {1, 222, 55555})
@DisplayName("유효하지 않은 출생년도라면 400을 반환한다.")
void invalidRangeOfBirthYear(int birthYear) throws Exception {
// given
Member member = MemberFixtures.MALE_20.get();
MemberDetailRequest request = new MemberDetailRequest(Gender.MALE, birthYear);

TokenPayload tokenPayload = new TokenPayload(1L, 1L, 1L);
given(tokenProcessor.resolveToken(anyString())).willReturn("token");
given(tokenProcessor.parseToken(anyString())).willReturn(tokenPayload);
given(memberService.findById(anyLong())).willReturn(MemberFixtures.FEMALE_20.get());

willDoNothing().given(memberService).updateDetails(request, member);

// when
RestAssuredMockMvc
.given().log().all()
.headers(HttpHeaders.AUTHORIZATION, "Bearer token")
.contentType(ContentType.JSON)
.body(request)
.when().patch("/members/me/detail")
.then().log().all()
.statusCode(HttpStatus.BAD_REQUEST.value());
}

@ParameterizedTest
@NullSource
@DisplayName("출생년도가 빈 값이면 400을 반환한다.")
void invalidNullOfBirthYear(Integer birthYear) throws Exception {
// given
Member member = MemberFixtures.MALE_20.get();
MemberDetailRequest request = new MemberDetailRequest(Gender.MALE, birthYear);

TokenPayload tokenPayload = new TokenPayload(1L, 1L, 1L);
given(tokenProcessor.resolveToken(anyString())).willReturn("token");
given(tokenProcessor.parseToken(anyString())).willReturn(tokenPayload);
given(memberService.findById(anyLong())).willReturn(MemberFixtures.FEMALE_20.get());

willDoNothing().given(memberService).updateDetails(request, member);

// when
RestAssuredMockMvc
.given().log().all()
.headers(HttpHeaders.AUTHORIZATION, "Bearer token")
.contentType(ContentType.JSON)
.body(request)
.when().patch("/members/me/detail")
.then().log().all()
.statusCode(HttpStatus.BAD_REQUEST.value());
}

}

@Test
@DisplayName("회원 탈퇴에 성공하면 204를 반환한다.")
void deleteMember() throws Exception {
Expand Down
Loading