Skip to content

Comments

[박도겸] sprint9#139

Open
2-d0 wants to merge 5 commits intocodeit-bootcamp-spring:박도겸from
2-d0:박도겸-sprint9

Hidden character warning

The head ref may contain hidden characters: "\ubc15\ub3c4\uacb8-sprint9"
Open

[박도겸] sprint9#139
2-d0 wants to merge 5 commits intocodeit-bootcamp-spring:박도겸from
2-d0:박도겸-sprint9

Conversation

@2-d0
Copy link
Collaborator

@2-d0 2-d0 commented Feb 8, 2026

요구사항

기본

  • 기본 항목 1
  • 기본 항목 2

심화

  • 심화 항목 1
  • 심화 항목 2

주요 변경사항

스크린샷

Uploading 스크린샷 2026-02-05 오후 2.42.04.png…

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@2-d0 2-d0 force-pushed the 박도겸-sprint9 branch from 5a434d9 to 1f893d2 Compare February 8, 2026 15:13
@2-d0 2-d0 marked this pull request as draft February 8, 2026 15:16
@2-d0 2-d0 marked this pull request as ready for review February 8, 2026 15:17
Copy link
Collaborator

@jinho-yoo-jack jinho-yoo-jack left a comment

Choose a reason for hiding this comment

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

전반적으로 코드를 잘 작성하셨으나, 프로덕트 코드의 형태로 사용할 수 있는 수준은 아닙니다.
(디테일한 부분들이 누락이 많이 되어 있거나, 생략된 부분이 있습니다.)

이러한 부분들을 고려하여 코드를 다시 리팩토링 해보시기를 권고 드립니다.
고생하셨습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

AppConfig보다는 JpaAuditing 설정을 활성화 하기 위한 코드라면, JpaConfig가 더 맞지 않을까요?

}

@Bean
public RoleHierarchy roleHierarchy() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

하나의 클래스에서 너무 많은 것들을 하고 있는 것 같아요!
SecurityConfig 역할을 하는 코드는 그대로 두고, 그외 것들은 따로 클래스로 분리하는 것이 더 좋아 보입니다. 관심사 분리를 위해서

ro


public class DiscodeitUserDetails implements UserDetails {

private final User user;
Copy link
Collaborator

Choose a reason for hiding this comment

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

User Entity를 바로 사용하지 말고,
User Entity에 선언되어 있는 필드(멤버)를 여기에서도 선언해주는 것이 올바른 방법입니다.

return user.getUsername();
}

@Override public boolean isAccountNonExpired() { return true; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

아마도 이 부분은 시간이 없어서 제대로 구현을 하지 못한 것으로 보여집니다.
시간이 생겼을 때, 구현 해보시기를 권고 드립니다.

.orElseThrow(() ->
new UsernameNotFoundException("User not found: " + username));

return new DiscodeitUserDetails(user);
Copy link
Collaborator

Choose a reason for hiding this comment

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

위에서 말한 것처럼, 조회된 User Entity의 필드를 이용해서 UserDetail 객체를 생성할 수 있는 정적 메서드를 정의해서 변환이 될 수 있도록 설계하는 것이 올바른 설계 방법 입니다.

package com.sprint.mission.discodeit.entity;

public enum Role {
ADMIN,
Copy link
Collaborator

Choose a reason for hiding this comment

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

단순히 권한에 대한 정의만 하는게 아니라,
숫자 값을 함께 정의했다면, 데이터베이스에 데이터를 저장할 때, 더 작은 크기의 데이터 형태로 데이터를 저장하면 저장소 공간을 효율적으로 사용할 수 있겠죠?

@jinho-yoo-jack
Copy link
Collaborator

@2-d0 코드가 현재 Conflict되고 있습니다. 이 부분을 해결하고 알려주시면, Merge해드리겠습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants