Skip to content

서현하 sprint6 심화는 못했습니다.#133

Open
by15622 wants to merge 6 commits intocodeit-bootcamp-spring:서현하from
by15622:서현하_sprint6

Hidden character warning

The head ref may contain hidden characters: "\uc11c\ud604\ud558_sprint6"
Open

서현하 sprint6 심화는 못했습니다.#133
by15622 wants to merge 6 commits intocodeit-bootcamp-spring:서현하from
by15622:서현하_sprint6

Conversation

@by15622
Copy link
Copy Markdown
Collaborator

@by15622 by15622 commented Mar 14, 2026

요구사항

기본

  • 기본 항목 1
  • [x ] 기본 항목 2

심화

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

주요 변경사항

스크린샷

image

pr첨부1 pr첨부2 pr첨부3

멘토에게

  • pr올리는중에 충돌이 일어나서 당황했었는데 올려도 괜찮다고 해서 올려봅니다 지각해서 죄송합니다!

@by15622 by15622 requested a review from yerin-mentor March 14, 2026 12:42
@by15622 by15622 added the 미완성🛠️ 스프린트 미션 제출일이지만 미완성했습니다. 죄송합니다. label Mar 14, 2026
Copy link
Copy Markdown
Collaborator

@yerin-mentor yerin-mentor left a comment

Choose a reason for hiding this comment

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

안녕하세요 현하님 :)
심화과정 못하신건 아쉽지만 엔티티 설계 잘 해 주셨네요!
@Transactional 이 전체적으로 누락되어 있는 것 같은데 한번 확인 부탁드려요~

과제 하시느라 수고하셨습니다. 😄

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LoginRequest는 controller 하위가 아니라 dto 하위로 가는게 맞는거 같네요 :)


@RequiredArgsConstructor
@RestController
@CrossOrigin(origins = "*")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@CrossOrigin(origins = "*") 모든 도메인에서 api 요청을 허용하겠다는 뜻이라 운영 환경에서는 보안상 좋지 않습니다.

기억해 두셨다가 추후 Spring Security 학습하실 때 CORS 설정 고려해 보면 좋을듯 합니다 :)

.build();
}

public <T> PageResponse<T> fromPage(Page<T> page) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

제네릭 타입을 활용해서 Page 응답을 공통화하신 점이 좋습니다 👍


@Override
public MessageDto create(MessageCreateRequest messageCreateRequest,
public Message create(MessageCreateRequest messageCreateRequest,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Transactional 은 어디갔을까요~~

public UserDto find(UUID userId) {
return userRepository.findById(userId)
.map(this::toDto)
.map(userMapper::toDto)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

여기 toDto 사용하시면서 LAZY 로딩에서 N+1 문제가 발생합니다.
데이터를 한번에 가져올수 있는 방법을 생각해보면 좋을것 같아요.

@yerin-mentor yerin-mentor marked this pull request as draft March 18, 2026 12:58
@yerin-mentor yerin-mentor marked this pull request as ready for review March 18, 2026 12:59
@yerin-mentor yerin-mentor marked this pull request as draft March 18, 2026 12:59
@yerin-mentor yerin-mentor marked this pull request as ready for review March 18, 2026 12:59
@yerin-mentor
Copy link
Copy Markdown
Collaborator

@by15622 현하님 discordit/.gradle/9.0.0/fileHashes/fileHashes.lock 이 충돌을 내고 있어서 처리를 해주셔야 제가 merge를 할수 있을것 같습니다.

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

Labels

미완성🛠️ 스프린트 미션 제출일이지만 미완성했습니다. 죄송합니다.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants