feat : 회원 정보 수정 by duehee · Pull Request #236 · BCSDLab/KOIN_API_V2

duehee · 2024-03-23T12:38:02Z

🔥 연관 이슈

close PUT /user/student/me #23

🚀 작업 내용

회원 정보 수정 추가했습니다.

기존 Swagger에서 사용하던 Request 유형에서 Password를 제외하고 작성했습니다.(다른 이슈로 할당 예정)
기존에 사용하던 학번/학부 예외처리를 422번이 아닌 ~~404~~번 오류 -> 400번 오류로 작성했습니다.(3/27 수정)
학번에 관한 오류는 현재 10자리가 아니면 오류가 발생하도록 했습니다.(23년 이후 학번처리에 대한 의견이 필요)

~~2. 학부/학과에 대한 ENUM 추가했습니다.~~
~~- 24년 기준, 나뉜 학부도 추가해서 작성했습니다.~~
(현재 존재하는 학부/과 : 기계공학부, 컴퓨터공학부, 메카트로닉스공학부, 전기전자통신공학부, 디자인공학부, 건축공학부, 화학생명공학부, 에너지신소재공학부, 산업경영학부, 고용서비스정책학과)
~~- 기존 존재하던 TEST의 내용도 수정 완료했습니다.~~

-> ENUM을 사용하는 방향을 조금 고민해봐야 할 것 같아서 STRING으로 우선 작성했습니다.
서버 내에서 500번 오류 발생 이슈로 수정했고, 추가적으로 이슈 생기면 스레드에 작성하겠습니다!
StudentDepartment ENUM은 삭제하지 않고 유지해놨습니다. -> 검증 ENUM에서 진행

💬 리뷰 중점사항

기능 작성하는 시간이 조금 오래 걸렸습니다. 기존에 회원 정보 수정에 있던 내용인 비밀번호 관련이슈는 User 팀의 스레드에 올려놓았습니다.

작성하며 헤맨 부분이 많아서, 리뷰 작성해주시면 감사하겠습니다! 감사합니다 :D

… feature/23-put-user-student-me

github-actions · 2024-03-23T12:51:35Z

Unit Test Results

98 tests 98 ✔️ 40s ⏱️
17 suites   0 💤
17 files   0 ❌

Results for commit fe6adfb.

♻️ This comment has been updated with latest results.

kwoo28

기존 코인 보면서 해석하고 코마거치는 과정이 쉽진 않았을텐데 수고 많으셨습니다!
리뷰 남기겠습니다.

kwoo28 · 2024-03-23T17:11:12Z

+        }
+        if (user.resetExpiredAt != null) {
+            this.resetExpiredAt = user.resetExpiredAt;
+        }


C

StudentService에서 받는 필드는 gender, name, nickname, phoneNumber인데, 왜 모든 필드에 조건문을 달으신건가요??

기존 코인 API에서 update 메소드를 이용했는데, 이 구문에는 모든 값에 처리가 되어있어서 같이 작성해보았습니다!

수정이 필요한 필드가 있다면 해당 필드(들)만 수정하는 메서드를 작성하는 게 좋을 것 같아요~
사이드이펙트가 발생할 여지가 많아 보입니다.

추가적으로 수정해서 작성했습니다~

R

메소드 명만 봤을때는 null이 들어왔을 때 null로 update되는게 맞아보이는데 이거는 updateOptional... 같은 느낌이네요
굳이 null이면 업데이트를 안한다는 로직을 넣어야할 이유가 있나요?

오히려 이 검증이 있어서 메소드가 예상하지 못하는 동작을 수행하게 되는거같네요

아예 모르는 사람이 이 메소드를 사용했을 때 인자로 들어가는 모든 값으로 수정이 일어나길 원할텐데 한번 고민해보세요

+) 현재 업데이트 대상인 필드들이 엔티티 조건에서 NotNull이 다 걸려있나요?

현재 업데이트 대상인 필드들은 NN이 걸려있지 않습니다!(DB)
확인하여 업데이트 수정해보았습니다! 확인해주시면 감사하겠습니당

kwoo28 · 2024-03-24T06:06:59Z

+            .name(studentUpdateRequest.name())
+            .nickname(studentUpdateRequest.nickname())
+            .phoneNumber(studentUpdateRequest.phoneNumber())
+            .build();


C

필드4개를 변경시키고자 User객체를 한개 더만드신건가요? User클래스에서 변경시키고자 하는 필드만 받는 메서드를 만드는게 어떨까요?

User 객체에서 @Bulider를 이미 사용하고 있는 메소드가 있어서 추가적인 @Builder를 작성하지 않았습니다.
제가 기존에 사용하던 방법인 새 update 객체를 생성하여 적용하는 방법이 효율이 좋지 않은가에 대해선 한 번 생각해보겠습니다 :D

개인적으로 Builder를 타 객체 정보 수정을 위한 임시 객체 생성에 사용하는 것은 선호하지 않습니다. user.update 메서드에 파라미터로 User를 주는 것보다 수정이 필요한 각각의 변수를 할당하는 것이 훨씬 가독성에 좋지 않을까 싶습니다.
(user.update에 User 객체를 통째로 주면 호출자 입장에서는 어떤 필드가 사용될지 예측할 수 없습니다)

kwoo28 · 2024-03-24T06:07:47Z

+        Student updateStudent = Student.builder()
+            .department(StudentDepartment.from(studentUpdateRequest.major()))
+            .studentNumber(studentUpdateRequest.studentNumber())
+            .build();


C

이것도 User와 마찬가지로...

위의 사항과 동일하여 한 번 생각해보겠습니다!!

kwoo28 · 2024-03-24T06:11:25Z

+
+        if (studentUpdateRequest.studentNumber() != null &&
+            !Student.isValidStudentNumber(studentUpdateRequest.studentNumber())) {
+            throw new StudentNumberNotValidException(


C

이 부분은 예외를 만들어서 처리하기보다 Request에서 @Valid로 학번의 형식을 거르는 방법은 어떤가요?

그렇게 코드를 작성한다면 더 깔끔하게 처리할 수도 있겠네요!

이거 반영이 안된거같아요~

반영하여 원래 있던 예외처리 구문을 삭제했습니다!

seongjae6751

고생하셨습니다! 리뷰 드립니다

seongjae6751 · 2024-03-24T10:09:19Z

+        @Size(max = 50, message = "학번은 50자 이내여야 합니다.")
+        @Schema(description = "학번", example = "2020136065")
+        String studentNumber


C
원경님이 말씀하신대로 학번을 50자로 받고 10자인지 확인하는 함수로 확인하는 것보다
처음부터 10자인지 확인해주면 좋을 것 같아요!

@Size( max = 10, message = "학번은 정확히 10자 여야 합니다.") String studentNumber;

이런 식으로 쓸 수 있을 것 같네요!

기존 레거시 코드에는 학번이 50으로 되어있어서 이렇게 작성해보았는데, 확인해보고 수정 해보겠습니다!

seongjae6751 · 2024-03-24T10:57:06Z

+        String token = jwtProvider.createToken(student.getUser());
+
+        ExtractableResponse<Response> response = RestAssured
+            .given().log().all()


R
.log().all()은 빼주세요~

seongjae6751 · 2024-03-24T11:12:52Z

+            @ApiResponse(responseCode = "403", content = @Content(schema = @Schema(hidden = true))),
+            @ApiResponse(responseCode = "404", content = @Content(schema = @Schema(hidden = true))),
+            @ApiResponse(responseCode = "409", content = @Content(schema = @Schema(hidden = true))),
+            @ApiResponse(responseCode = "422", content = @Content(schema = @Schema(hidden = true)))


A
사소한 거지만 422 안 쓰기로 했으면 빼도 될 것 같네요~

songsunkook · 2024-03-25T05:12:56Z

+    public void update(User user) {
+        if (user.nickname != null) {
+            this.nickname = user.nickname;
+        }
+        if (user.name != null) {
+            this.name = user.name;
+        }
+        if (user.phoneNumber != null) {
+            this.phoneNumber = user.phoneNumber;
+        }
+        if (user.gender != null) {
+            this.gender = user.gender;
+        }
+    }


R

네이밍이 직관적이지 못한 것 같아요
User를 매개변수로 받았을 때 해당 변수에서 어떤 내용을 update해주는 것인지 알 수 없을 것 같습니다.
좀 더 리팩토링이 필요해 보여요~

+) Student도 수정해주세요

각 이름 userInfoUpdate, studentInfoUpdate로 수정 완료했습니다!

songsunkook

테스트 꼼꼼하게 작성해주셨네요!
코멘트 남겨드렸으니 확인 부탁드립니다~

songsunkook · 2024-03-25T23:58:09Z

+        @Schema(description = "[NOT UPDATE]졸업 여부(true, false)", example = "false")
+        Boolean isGraduated,
+
+        @Schema(description = "전공{기계공학부, 컴퓨터공학부, 메카트로닉스공학부, 전기전자통신공학부, 디자인공학부, 건축공학부, 화학생명공학부, 에너지신소재공학부, 산업경영학부, 고용서비스정책학과}", example = "컴퓨터공학부")


C

저희 코딩 컨벤션에 한 줄당 최대 길이가 정해져있지 않았나요??
중간에 개행을 넣을 수 있을 것 같아요

songsunkook · 2024-03-26T00:07:06Z

+    @Schema(description = "성별(남:0, 여:1)", example = "1")
+    Integer gender,
+
+    @Schema(description = "전공{기계공학부, 컴퓨터공학부, 메카트로닉스공학부, 전기전자통신공학부, 디자인공학부, 건축공학부, 화학생명공학부, 에너지신소재공학부, 산업경영학부, 고용서비스정책학과}", example = "컴퓨터공학부")


C

저희 코딩 컨벤션에 한 줄당 최대 길이가 정해져있지 않았나요??
중간에 개행을 넣을 수 있을 것 같아요

22

songsunkook · 2024-03-26T00:09:50Z

+
+import in.koreatech.koin.global.exception.DataNotFoundException;
+
+public class StudentDepartmentNotValidException extends DataNotFoundException {


A

StudentDepartmentNotValidException 보다는 StudentDepartmentNotFoundException가 더 적절한 네이밍일 것 같아요

형식이 맞지 않을때에 예외를 처리하는 것이어서 Valid를 사용했는데, 이름을 짓는다면 예외에 대한 이름을 짓는 게 맞는지, 아니면 여기서는 반환하는 값이 404여서 Not Found를 쓰는 게 맞는지 궁금합니다!
--> 형식이 맞지 않는 오류여서 예외처리는 Valid가 맞는 것 같습니다!

형식이 맞지 않는 오류라면 DataNotFoundException을 상속하는게 옳은 방향일까요~?

songsunkook · 2024-03-26T00:10:40Z

+
+import in.koreatech.koin.global.exception.DataNotFoundException;
+
+public class StudentNumberNotValidException extends DataNotFoundException {


A

StudentDepartmentNotValidException 보다는 StudentDepartmentNotFoundException가 더 적절한 네이밍일 것 같아요

22

songsunkook · 2024-03-26T00:14:59Z

+            .name(studentUpdateRequest.name())
+            .nickname(studentUpdateRequest.nickname())
+            .phoneNumber(studentUpdateRequest.phoneNumber())
+            .build();


개인적으로 Builder를 타 객체 정보 수정을 위한 임시 객체 생성에 사용하는 것은 선호하지 않습니다. user.update 메서드에 파라미터로 User를 주는 것보다 수정이 필요한 각각의 변수를 할당하는 것이 훨씬 가독성에 좋지 않을까 싶습니다.
(user.update에 User 객체를 통째로 주면 호출자 입장에서는 어떤 필드가 사용될지 예측할 수 없습니다)

songsunkook · 2024-03-26T00:19:03Z

+    }
+
+    @Test
+    @DisplayName("학생이 정보를 수정한다 - 이미 있는 닉네임이라면 409")


A

이걸 빨리 적용했어야 했는데 ;-;

Choi-JJunho

고생하셨습니다~
코멘트 몇개 확인부탁할게요

Choi-JJunho · 2024-03-27T02:30:52Z

            user.getEmail(),
            user.getGender().name(),
-            student.getDepartment(),
+            student.getDepartment().toString(),


C

toString, getValue가 혼용되어 사용되고있는 것 같아요~

Suggested change

student.getDepartment().toString(),

student.getDepartment().getValue(),

Department 관련해서 추가하며 다른 요소들을 너무 신경쓰지 않은 것 같네요..! :<

Choi-JJunho · 2024-03-27T02:31:44Z

+
+import in.koreatech.koin.global.exception.DataNotFoundException;
+
+public class StudentDepartmentNotValidException extends DataNotFoundException {


형식이 맞지 않는 오류라면 DataNotFoundException을 상속하는게 옳은 방향일까요~?

Choi-JJunho · 2024-03-27T02:34:10Z

+        String message = String.format("%s %s", DEFAULT_MESSAGE, detail);
+        return new StudentNumberNotValidException(message);
+    }
+


R

불필요한 공백 발견~

Choi-JJunho · 2024-03-27T02:40:39Z

+                "학생의 학번 형식이 아닙니다. studentNumber : " + studentUpdateRequest.studentNumber());
+        }
+
+        user.userInfoUpdate(studentUpdateRequest.nickname(), studentUpdateRequest.name(),


R

info라는 네이밍이 다소 추상적이게 다가올 수 있어요
그냥 update는 어떨까요?

Suggested change

user.userInfoUpdate(studentUpdateRequest.nickname(), studentUpdateRequest.name(),

user.update(studentUpdateRequest.nickname(), studentUpdateRequest.name(),

참고자료: https://github.com/Yooii-Studios/Clean-Code/blob/master/Chapter%2002%20-%20%EC%9D%98%EB%AF%B8%20%EC%9E%88%EB%8A%94%20%EC%9D%B4%EB%A6%84.md#%EC%9D%98%EB%AF%B8-%EC%9E%88%EA%B2%8C-%EA%B5%AC%EB%B6%84%ED%95%98%EB%9D%BC%EB%B6%88%EC%9A%A9%EC%96%B4-noise-word-%EB%A5%BC-%EC%93%B0%EC%A7%80-%EB%A7%90%EC%9E%90

그렇다면 패스워드를 수정하는 메소드가 추가된다면, 그 메소드 이름을 따로 설정하는 것이 더 나을 수 있을 거 같네요!

Choi-JJunho · 2024-03-27T02:41:25Z

+
+        user.userInfoUpdate(studentUpdateRequest.nickname(), studentUpdateRequest.name(),
+            studentUpdateRequest.phoneNumber(), UserGender.from(studentUpdateRequest.gender()));
+        student.studentInfoUpdate(studentUpdateRequest.studentNumber(),


R

info라는 네이밍이 다소 추상적이게 다가올 수 있어요
그냥 update는 어떨까요?

ditto

Choi-JJunho · 2024-03-27T02:44:08Z

                    .isEqualTo(user.getGender().name());
                softly.assertThat(response.body().jsonPath().getString("major"))
-                    .isEqualTo(student.getDepartment());
+                    .isEqualTo(student.getDepartment().name());


R

name이랑 같아야하는게 맞아요? getValue 랑 같아야하는거 아닌가??

앗.. StudentDepartment ENUM 추가 이후 수정하지 않은 것 같습니다!

Choi-JJunho

고생하셨습니다~
Approve의 길은 멀고도 험하죠 ㅋㅋㅋ
코멘트 확인 부탁드릴게요~

Choi-JJunho · 2024-03-27T05:52:51Z

-    @ApiResponses(
-        value = {
-            @ApiResponse(responseCode = "200"),
-            @ApiResponse(responseCode = "400", content = @Content(schema = @Schema(hidden = true))),
-            @ApiResponse(responseCode = "401", content = @Content(schema = @Schema(hidden = true))),
-            @ApiResponse(responseCode = "403", content = @Content(schema = @Schema(hidden = true))),
-            @ApiResponse(responseCode = "404", content = @Content(schema = @Schema(hidden = true))),
-        }
-    )
-    @Operation(summary = "푸쉬알림 동의 여부 조회")
-    @GetMapping("/user/notification")
-    ResponseEntity<NotificationStatusResponse> checkNotificationStatus(
-        @Auth(permit = {STUDENT, OWNER}) Long memberId
-    );
-
-    @ApiResponses(
-        value = {
-            @ApiResponse(responseCode = "200"),
-            @ApiResponse(responseCode = "400"),
-            @ApiResponse(responseCode = "401"),
-            @ApiResponse(responseCode = "403"),
-            @ApiResponse(responseCode = "404"),
-        }
-    )
-    @Operation(summary = "푸쉬알림 동의")
-    @PostMapping("/user/notification")
-    ResponseEntity<Void> permitNotification(


R

안돼~~ 알림기능 다 어디갔어

바로 다시 추가했씁니다..

Choi-JJunho · 2024-03-27T05:53:24Z

+public class StudentDepartmentNotValidException extends IllegalArgumentException {
+    private static final String DEFAULT_MESSAGE = "학생의 전공 형식이 아닙니다.";


R

개행

Suggested change

public class StudentDepartmentNotValidException extends IllegalArgumentException {

private static final String DEFAULT_MESSAGE = "학생의 전공 형식이 아닙니다.";

public class StudentDepartmentNotValidException extends IllegalArgumentException {

private static final String DEFAULT_MESSAGE = "학생의 전공 형식이 아닙니다.";

Choi-JJunho · 2024-03-27T05:53:33Z

+public class StudentNumberNotValidException extends IllegalArgumentException {
+    private static final String DEFAULT_MESSAGE = "학생의 학번 형식이 아닙니다.";


R

여기도 개행

Choi-JJunho · 2024-03-27T05:53:49Z

+    CHEMICAL("화학생명공학부"),
+    ENERGY("에너지신소재공학부"),
+    INDUSTRIAL("산업경영학부"),
+    EMPLOYMENT("고용서비스정책학과");


R

후행쉼표

Suggested change

EMPLOYMENT("고용서비스정책학과");

EMPLOYMENT("고용서비스정책학과"),

;

Choi-JJunho · 2024-03-27T05:54:18Z

+        return Arrays.stream(values())
+            .filter(it -> it.value.equals(value))
+            .findAny()
+            .orElseThrow(() -> new StudentDepartmentNotValidException("학생의 전공 형식이 아닙니다. " + value));


C

withDetail 메소드를 선언 & 활용하지 않은 이유가 있나요

사용해서 작성해보았습니다!!

Choi-JJunho · 2024-03-27T05:58:11Z

+        }
+        if (user.resetExpiredAt != null) {
+            this.resetExpiredAt = user.resetExpiredAt;
+        }


R

메소드 명만 봤을때는 null이 들어왔을 때 null로 update되는게 맞아보이는데 이거는 updateOptional... 같은 느낌이네요
굳이 null이면 업데이트를 안한다는 로직을 넣어야할 이유가 있나요?

오히려 이 검증이 있어서 메소드가 예상하지 못하는 동작을 수행하게 되는거같네요

아예 모르는 사람이 이 메소드를 사용했을 때 인자로 들어가는 모든 값으로 수정이 일어나길 원할텐데 한번 고민해보세요

+) 현재 업데이트 대상인 필드들이 엔티티 조건에서 NotNull이 다 걸려있나요?

Choi-JJunho · 2024-03-27T05:58:51Z

+            return Arrays.stream(values())
+                .filter(it -> it.ordinal() == index)
+                .findAny()
+                .orElseThrow(() -> new IllegalArgumentException("잘못된 성별 인덱스 입니다. index : " + index));


C

어떤 친구는 customException으로 notfound를 반환하고 어떤 친구는 IAE를 발생시키는 기준이 있나요?

제가 학번과 학부에 대한 예외처리는 custumException을 만들어서 처리했는데, 성별에 관한 사항은 그냥 적용시킨 것 같습니다!
성별에 관한 예외처리에 대한 상황은 한 번 고려해보는 것이 좋을 것 같습니다😄

이 사항은 추가적으로 custumException 작성해보았습니다!

Choi-JJunho · 2024-03-27T05:59:43Z

+                .findAny()
+                .orElseThrow(() -> new IllegalArgumentException("잘못된 성별 인덱스 입니다. index : " + index));
+        }
+        return null;


R

대상 index가 없으면 예외가 터지는게 맞는거같은데 어떻게 생각하나요
서비스 내에서 null이 자유분방하게 돌아다니면 어딘가에서는 NPE가 발생할 것 같네요

null 처리 구문 삭제했습니다!

Choi-JJunho · 2024-03-27T06:00:06Z

+
+        if (studentUpdateRequest.studentNumber() != null &&
+            !Student.isValidStudentNumber(studentUpdateRequest.studentNumber())) {
+            throw new StudentNumberNotValidException(


이거 반영이 안된거같아요~

Choi-JJunho · 2024-03-27T06:01:40Z

+        @Size(max = 10, message = "학번은 10자여야 합니다.")
+        @Schema(description = "학번", example = "2020136065")
+        String studentNumber


R

아래 코멘트에서 나온 내용을 여기에 적용시켜보면 될 것 같아요

https://github.com/BCSDLab/KOIN_API_V2/pull/236/files#r1536745436

추가하여 적용 완료 했습니다!

songsunkook

거의 다 온 것 같은데 조금만 더 힘내봅시다..!
화이팅~

songsunkook · 2024-03-28T11:47:51Z

-    @Size(max = 50)
    @Column(name = "major", length = 50)
-    private String department;
+    @Enumerated(EnumType.STRING)
+    private StudentDepartment department;


R

major에서 department로 컬럼명을 다르게 매핑한 이유가 있나요? DB와 헷갈릴 것 같아요

+) 중요한 내용인데, 이거 잘 실행되나요? 에러 납니다. 푸시 전에는 로컬에서 반드시 실행해보시기 바랍니다.

기존에 작성되어있던 Student의 major 매핑의 컬럼명이 department여서 동일하게 Enum 생성하는 과정에서 이름 사용했습니다!

songsunkook · 2024-03-28T11:54:51Z

+        if (studentUpdateRequest.nickname() != null &&
+            userRepository.existsByNickname(studentUpdateRequest.nickname())) {
+            throw DuplicationNicknameException.withDetail("nickname : " + studentUpdateRequest.nickname());
+        }


C

짜잘한 내용이지만 닉네임 검증 로직은 메서드를 분리하는 것도 좋아 보이네요

따로 Service 내 메소드를 작성하도록 하겠습니다!!!

Choi-JJunho

고생하셨습니다~
작은 컨벤션들만 확인 부탁드릴게요

Choi-JJunho · 2024-03-28T09:54:37Z

+    @Schema(description = "익명 닉네임", example = "익명_1676688416361")
+    String anonymousNickname,
+
+    @Schema(description = "이메일 주소 \n", example = "koin123@koreatech.ac.kr")


R

불필요한 내용같네요

Suggested change

@Schema(description = "이메일 주소 \n", example = "koin123@koreatech.ac.kr")

@Schema(description = "이메일 주소", example = "koin123@koreatech.ac.kr")

Choi-JJunho · 2024-03-29T03:23:44Z

+        + "건축공학부, 화학생명공학부, 에너지신소재공학부, 산업경영학부, 고용서비스정책학과}", example = "컴퓨터공학부")
+    String major,
+
+    @Schema(description = "이름 \n", example = "최준호")


R

불필요한 내용같네요

ditto

Suggested change

@Schema(description = "이름 \n", example = "최준호")

@Schema(description = "이름", example = "최준호")

Choi-JJunho · 2024-03-29T03:26:01Z

+        Student student = studentRepository.getById(userId);
+        User user = student.getUser();
+
+        if (studentUpdateRequest.nickname() != null &&
+            userRepository.existsByNickname(studentUpdateRequest.nickname())) {
+            throw DuplicationNicknameException.withDetail("nickname : " + studentUpdateRequest.nickname());
+        }
+
+        user.update(studentUpdateRequest.nickname(), studentUpdateRequest.name(),
+            studentUpdateRequest.phoneNumber(), UserGender.from(studentUpdateRequest.gender()));
+        student.update(studentUpdateRequest.studentNumber(),
+            StudentDepartment.from(studentUpdateRequest.major()));
+
+        studentRepository.save(student);
+


A

개인 취향입니다만 메소드 내부에 있는 친구들은 붙여줘도 좋지 않을까용

Suggested change

Student student = studentRepository.getById(userId);

User user = student.getUser();

if (studentUpdateRequest.nickname() != null &&

userRepository.existsByNickname(studentUpdateRequest.nickname())) {

throw DuplicationNicknameException.withDetail("nickname : " + studentUpdateRequest.nickname());

}

user.update(studentUpdateRequest.nickname(), studentUpdateRequest.name(),

studentUpdateRequest.phoneNumber(), UserGender.from(studentUpdateRequest.gender()));

student.update(studentUpdateRequest.studentNumber(),

StudentDepartment.from(studentUpdateRequest.major()));

studentRepository.save(student);

Student student = studentRepository.getById(userId);

User user = student.getUser();

if (studentUpdateRequest.nickname() != null &&

userRepository.existsByNickname(studentUpdateRequest.nickname())) {

throw DuplicationNicknameException.withDetail("nickname : " + studentUpdateRequest.nickname());

}

user.update(studentUpdateRequest.nickname(), studentUpdateRequest.name(),

studentUpdateRequest.phoneNumber(), UserGender.from(studentUpdateRequest.gender()));

student.update(studentUpdateRequest.studentNumber(),

StudentDepartment.from(studentUpdateRequest.major()));

studentRepository.save(student);

저는 주로 작동하는 단위로 끊는 편인데, 코드 내에서는 스타일 통일 해보겠습니당

Choi-JJunho · 2024-03-29T03:27:20Z

+        Student student = Student.builder()
+            .studentNumber("2019136135")
+            .anonymousNickname("익명")
+            .department(StudentDepartment.COMPUTER)
+            .userIdentity(UserIdentity.UNDERGRADUATE)
+            .isGraduated(false)
+            .user(
+                User.builder()
+                    .password("1234")
+                    .nickname("주노")
+                    .name("최준호")
+                    .phoneNumber("010-1234-5678")
+                    .userType(STUDENT)
+                    .gender(UserGender.MAN)
+                    .email("test@koreatech.ac.kr")
+                    .isAuthed(true)
+                    .isDeleted(false)
+                    .build()
+            )
+            .build();
+
+        studentRepository.save(student);


A

어짜피 인증에서 거르는 테스트면 저장 안해도되지않을까요

테스트 상단의 get /user/student/me 401번 오류 검증 과정과 동일하게 구성해보았습니다!

duehee · 2024-03-29T16:28:55Z

이번에 로컬 환경에서 확인하면서 작동해본 결과, 오류가 많이 발생해서 추가한 사항들입니다!
gender, department 등 현재 enum으로 작동하고 있는 코드들에 null 값을 허용하도록 메소드를 조금 수정했습니다.
(.ordinal() 사용 시 intValue() 호출로 인한 Npe 발생 및 department.getValue() 사용 간 동일한 npe 발생)
무분별한 null 사용은 문제가 될 수 있지만, DB 상으로 null 값이 문제가 없는 사항들이라 판단하여 추가했습니다.
코드가 조금 지저분한 점은 있지만, 감안하여 작성해봤습니다. 어떻게 수정하면 좋을지 의견 있으시다면 달아주세요!

현재 ENUM을 사용하여 "컴퓨터공학부"를 저장하면 COMPUTER로 DB에 저장되고 있는데, 혹시 이렇게 저장되는 방식이 맞는 지 궁금해서 이렇게 코멘트 남깁니다! 이게 맞다면 더 이상 로컬에서의 문제는 발생하지 않을 것 같아서, 의견 부탁드립니다.

songsunkook

자세한 내용은 DM으로 설명해드렸으니 좀 더 고민해보고 알려주세요
아직 500에러 날겁니다

songsunkook · 2024-03-30T06:00:56Z

+    public void CheckDepartmentValid(String department) {
+        List<String> departments = Arrays.asList(
+            "컴퓨터공학부",
+            "기계공학부",
+            "메카트로닉스공학부",
+            "전기전자통신공학부",
+            "디자인공학부",
+            "건축공학부",
+            "화학생명공학부",
+            "에너지신소재공학부",
+            "산업경영학부",
+            "고용서비스정책학과"
+        );
+
+        if (department != null & !departments.contains(department)) {
+            throw StudentDepartmentNotValidException.withDetail("학부(학과) : " + department);
+        }
+    }


R

이렇게 검증이 필요하다면 차라리 enum 클래스로 빼서 검증하는 건 어떨까요?

수정 완료했습니다!!!!!

songsunkook

코멘트 70개의 전설..
정말 고생하셨습니다 ! 👍

duehee added 3 commits March 20, 2024 15:56

feat : DTO 추가

17d1958

Merge branch 'develop' of https://github.com/BCSDLab/KOIN_API_V2 into…

688ef63

… feature/23-put-user-student-me

feat : 유저 수정 추가

2cd4809

duehee added the 기능 새로운 기능을 개발합니다. label Mar 23, 2024

duehee requested review from daheeParkk, kwoo28 and seongjae6751 March 23, 2024 12:38

duehee self-assigned this Mar 23, 2024

feat : 충돌 해결

a05f899

kwoo28 approved these changes Mar 24, 2024

View reviewed changes

seongjae6751 requested changes Mar 24, 2024

View reviewed changes

duehee added 2 commits March 25, 2024 13:32

test : log.all() 삭제 및 422번 오류 삭제

9a15e02

chore : update 수정 및 사용하지 않는 Request 객체 삭제

9948155

songsunkook requested changes Mar 25, 2024

View reviewed changes

refactor : update 이름 수정

9d03c7b

duehee added the Team User 유저 팀에서 작업할 이슈입니다 label Mar 25, 2024

songsunkook requested changes Mar 26, 2024

View reviewed changes

duehee added 2 commits March 26, 2024 18:18

refactor : DTO major 라인 포맷팅

fa1cace

chore : bulider() 미사용 및 update 로직 수

7c6aeb8

duehee requested review from seongjae6751 and songsunkook March 26, 2024 15:37

Choi-JJunho requested changes Mar 27, 2024

View reviewed changes

duehee added 3 commits March 27, 2024 14:25

chore : 리뷰 반영 및 학부/학번 404 -> 400번 오류로 수정

bbc8eb9

feat : 충돌 해결

14acf71

chore : ApiResponse 수정

bd1f71d

seongjae6751 approved these changes Mar 27, 2024

View reviewed changes

Choi-JJunho requested changes Mar 27, 2024

View reviewed changes

duehee added 2 commits March 28, 2024 15:06

chore : 리뷰 반영

7e4005b

chore : withDetail 수정

08664cd

duehee requested a review from Choi-JJunho March 28, 2024 06:14

songsunkook requested changes Mar 28, 2024

View reviewed changes

Choi-JJunho approved these changes Mar 29, 2024

View reviewed changes

duehee added 7 commits March 29, 2024 12:52

chore : 리뷰 반영(코드 스타일 및 메소드 분리)

a631ea5

feat : 충돌 해결

a0daa35

chore : Local 내 회원 정보 조회 오류 확인 추가

302e7dd

chore : 삼항 연산자 제거

f7df525

chore : Response department null 처리 추가

7cb4e31

chore : 변수 수정 및 Gender null 처리 추가

91c2de4

chore : 변수 수정 및 department null 처리 추가

d64505c

duehee added 4 commits March 30, 2024 01:51

chore : DB 학과 한글 저장

07cf7a7

chore : DB 저장 체크(Enumerated 삭제)

95accd0

chore : DB 저장 체크(Enumerated 삭제 및 model 이름 수정)

25db4ab

chore : ENUM 사용하도록 재수정

b8e5867

duehee requested a review from songsunkook March 29, 2024 17:37

songsunkook requested changes Mar 30, 2024

View reviewed changes

fix : ENUM -> STRING 사용

b592981

songsunkook requested changes Mar 30, 2024

View reviewed changes

duehee added 3 commits March 30, 2024 15:52

chore : 검증 ENUM으로 이동

fcea148

chore : ENUM 영어 변수로 수정

c13775c

chore : ENUM 후행 쉼표 추가

fe6adfb

songsunkook approved these changes Mar 30, 2024

View reviewed changes

duehee merged commit 0751eb7 into develop Mar 30, 2024

duehee deleted the feature/23-put-user-student-me branch March 30, 2024 07:22


		import in.koreatech.koin.global.exception.DataNotFoundException;

		public class StudentDepartmentNotValidException extends DataNotFoundException {


		import in.koreatech.koin.global.exception.DataNotFoundException;

		public class StudentNumberNotValidException extends DataNotFoundException {

	student.getDepartment().toString(),
	student.getDepartment().getValue(),

	user.userInfoUpdate(studentUpdateRequest.nickname(), studentUpdateRequest.name(),
	user.update(studentUpdateRequest.nickname(), studentUpdateRequest.name(),

		public class StudentDepartmentNotValidException extends IllegalArgumentException {
		private static final String DEFAULT_MESSAGE = "학생의 전공 형식이 아닙니다.";

	EMPLOYMENT("고용서비스정책학과");
	EMPLOYMENT("고용서비스정책학과"),
	;

	@Schema(description = "이메일 주소 \n", example = "koin123@koreatech.ac.kr")
	@Schema(description = "이메일 주소", example = "koin123@koreatech.ac.kr")

	@Schema(description = "이름 \n", example = "최준호")
	@Schema(description = "이름", example = "최준호")

Uh oh!

Conversation

duehee commented Mar 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔥 연관 이슈

🚀 작업 내용

💬 리뷰 중점사항

Uh oh!

github-actions Bot commented Mar 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

kwoo28 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

C

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

R

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

C

Uh oh!

duehee Mar 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

C

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

C

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seongjae6751 left a comment

Choose a reason for hiding this comment

Uh oh!

seongjae6751 Mar 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

songsunkook Mar 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

R

Uh oh!

Choose a reason for hiding this comment

Uh oh!

songsunkook left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

C

Uh oh!

Choose a reason for hiding this comment

C

duehee commented Mar 23, 2024 •

edited

Loading

github-actions Bot commented Mar 23, 2024 •

edited

Loading

duehee Mar 25, 2024 •

edited

Loading

seongjae6751 Mar 24, 2024 •

edited

Loading

songsunkook Mar 25, 2024 •

edited

Loading

duehee Mar 26, 2024 •

edited

Loading