Skip to content

feat: 지원서가 APPROVED 인 유저의 멘토 생성 기능 추가 #562

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.

Sign up for GitHub

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

Merged
sukangpunch merged 5 commits into from
Nov 22, 2025
Merged

feat: 지원서가 APPROVED 인 유저의 멘토 생성 기능 추가 #562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
74108c0
feat: 지원서가 APPROVED 인 유저의 멘토 생성 기능 추가
sukangpunch Nov 10, 2025
ac9f657
fix: 코드래빗 리뷰 적용
sukangpunch Nov 12, 2025
653578b
fix: 리뷰 수정사항 적용
sukangpunch Nov 20, 2025
64780bf
feat: MentorApplication에 termId 추가 flyway 스크립트 추가
sukangpunch Nov 20, 2025
38b3ad3
fix: flyway 버전 충돌 해결
sukangpunch Nov 20, 2025
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
2 changes: 2 additions & 0 deletions src/main/java/com/example/solidconnection/common/exception/ErrorCode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public enum ErrorCode {
BLOCK_USER_NOT_FOUND(HttpStatus.NOT_FOUND.value(), "차단 대상 사용자를 찾을 수 없습니다."),
TERM_NOT_FOUND(HttpStatus.NOT_FOUND.value(), "존재하지 않는 학기입니다."),
CURRENT_TERM_NOT_FOUND(HttpStatus.NOT_FOUND.value(), "현재 학기를 찾을 수 없습니다."),
MENTOR_APPLICATION_NOT_FOUND(HttpStatus.NOT_FOUND.value(), "멘토 지원서가 존재하지 않습니다."),

// auth
USER_ALREADY_SIGN_OUT(HttpStatus.UNAUTHORIZED.value(), "로그아웃 되었습니다."),
Expand Down Expand Up @@ -127,6 +128,7 @@ public enum ErrorCode {
UNIVERSITY_ID_REQUIRED_FOR_CATALOG(HttpStatus.BAD_REQUEST.value(), "목록에서 학교를 선택한 경우 학교 정보가 필요합니다."),
UNIVERSITY_ID_MUST_BE_NULL_FOR_OTHER(HttpStatus.BAD_REQUEST.value(), "기타 학교를 선택한 경우 학교 정보를 입력할 수 없습니다."),
INVALID_UNIVERSITY_SELECT_TYPE(HttpStatus.BAD_REQUEST.value(), "지원하지 않는 학교 선택 방식입니다."),
MENTOR_ALREADY_EXISTS(HttpStatus.BAD_REQUEST.value(), "이미 존재하는 멘토입니다."),

// socket
UNAUTHORIZED_SUBSCRIBE(HttpStatus.FORBIDDEN.value(), "구독 권한이 없습니다."),
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.example.solidconnection.mentor.controller;

import com.example.solidconnection.common.resolver.AuthorizedUser;
import com.example.solidconnection.mentor.dto.MentorMyPageCreateRequest;
import com.example.solidconnection.mentor.dto.MentorMyPageResponse;
import com.example.solidconnection.mentor.dto.MentorMyPageUpdateRequest;
import com.example.solidconnection.mentor.service.MentorMyPageService;
Expand All @@ -10,6 +11,7 @@
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
Expand Down Expand Up @@ -40,4 +42,14 @@ public ResponseEntity<Void> updateMentorMyPage(
mentorMyPageService.updateMentorMyPage(siteUserId, mentorMyPageUpdateRequest);
return ResponseEntity.ok().build();
}

@RequireRoleAccess(roles = Role.MENTOR)
@PostMapping
public ResponseEntity<Void> createMentorMyPage(
@AuthorizedUser long siteUserId,
@Valid @RequestBody MentorMyPageCreateRequest request
) {
mentorMyPageService.createMentorMyPage(siteUserId, request);
return ResponseEntity.ok().build();
}
Comment on lines +46 to +54
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

역할 기반 접근 제어에 잠재적 문제가 있습니다.

이 엔드포인트는 @RequireRoleAccess(roles = Role.MENTOR)를 사용하고 있는데, 이는 모순을 야기합니다:

문제:

  • 멘토 마이페이지를 생성하는 엔드포인트인데, 이미 MENTOR 역할을 가진 사용자만 접근할 수 있습니다.
  • PR 목표에 따르면, "지원서가 APPROVED 인 유저의 멘토 생성"이므로, 아직 MENTOR가 아닌 사용자가 이 엔드포인트를 호출해야 합니다.

고려사항:

  1. 옵션 1 (권장): 역할 체크를 제거하거나 다른 역할(예: Role.USER)로 변경하고, 서비스 레이어에서 APPROVED 지원서 존재 여부를 확인합니다.
  2. 옵션 2: 어드민이 승인 시 자동으로 역할을 MENTOR로 변경한 후, 사용자가 이 엔드포인트를 호출하여 마이페이지를 완성하는 플로우인 경우, 현재 설계가 맞을 수 있습니다. 하지만 이 경우 엔드포인트명이 createMentorMyPage가 아니라 completeMentorMyPage 같은 이름이 더 적절합니다.

현재 비즈니스 플로우를 명확히 하고, 그에 맞게 접근 제어를 조정해주세요.

현재 플로우를 확인하기 위해 다음을 검토해주세요:

🏁 Script executed:

#!/bin/bash
# Description: Verify the role assignment flow

# Check where Role.MENTOR is assigned
rg -n "Role\.MENTOR|setRole.*MENTOR|role.*=.*MENTOR" --type=java -C3

# Check MentorMyPageService.createMentorMyPage implementation
ast-grep --pattern $'public $_ createMentorMyPage($$$) {
  $$$
}'

Length of output: 11628

🏁 Script executed:

#!/bin/bash
# Find MentorMyPageService implementation
fd -e java "MentorMyPageService" --exec grep -l "createMentorMyPage"

# Check service implementation
rg -n "createMentorMyPage" --type=java -A 10 -B 2

# Search for where roles are actually assigned/updated
rg -n "\.setRole\(|updateRole|Role\.MENTOR" --type=java -g '!**/test/**' -g '!**/fixture/**' -A 2

# Look for mentor application approval flow
fd -e java ".*Application.*" | head -20
rg -n "APPROVED|application.*approved" --type=java -g '!**/test/**' -i -C 2

Length of output: 34228

🏁 Script executed:

#!/bin/bash
# Search for mentor application approval logic
rg -n "MentorApplicationService|approveMentorApplication|approveApplication" --type=java -A 5 -B 2

# Look for any place where role is updated/assigned
rg -n "siteUser\.setRole|updateRole|\.role\s*=" --type=java -g '!**/test/**' -g '!**/fixture/**' -A 2 -B 2

# Check if there's a separate Admin/Approval controller
fd -e java ".*Approval.*|.*Admin.*" --type=java | grep -i mentor

Length of output: 6720

@RequireRoleAccess 데코레이터가 서비스 로직과 모순됩니다 — 엔드포인트에 도달 불가능한 상태입니다.

코드베이스를 검증한 결과, 핵심 문제가 확인되었습니다.

1. 역할 할당 코드가 존재하지 않음

  • 프로덕션 코드 어디에도 사용자 역할을 MENTOR으로 업데이트하는 로직이 없습니다.
  • MentorMyPageService.createMentorMyPage()는 Mentor 엔티티만 생성하며 역할을 변경하지 않습니다.
  • MentorApplicationService에서도 신청 승인 시 역할 할당 코드가 없습니다.

2. 해결 불가능한 접근 제어 체크

컨트롤러: @RequireRoleAccess(roles = Role.MENTOR)  ← 사용자가 이미 MENTOR 역할을 가져야 함
서비스:   MentorApplicationStatus.APPROVED 확인    ← 승인된 신청만 허용
결과:     역할 할당 코드가 없으므로 누구도 접근 불가

필수 조치:

  1. 멘토 신청 승인 시 역할 업데이트

    • MentorApplicationService 또는 별도의 admin 승인 서비스에서 신청 승인 시 사용자 역할을 Role.MENTOR으로 변경하세요.

  2. 컨트롤러 가드 재검토

    • 역할 할당 후에 이 엔드포인트를 호출하도록 의도했다면, 현재 설계가 맞습니다.
    • 하지만 역할을 먼저 확인하고 나서 멘토 마이페이지를 생성하는 흐름이라는 점을 명확히 문서화하세요.
🤖 Prompt for AI Agents 
In
src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.java
around lines 46 to 54, the @RequireRoleAccess(roles = Role.MENTOR) guard makes
this endpoint unreachable because nowhere in the codebase users are assigned
Role.MENTOR (MentorMyPageService.createMentorMyPage only creates Mentor entity
and MentorApplicationService does not update roles). Fix by adding role
assignment when a mentor application is approved (update the user's roles to
include Role.MENTOR in MentorApplicationService or the admin approval flow)
and/or remove or adjust the controller guard to match the actual flow (e.g.,
require approved application status instead of MENTOR role) and document the
intended sequence so callers first get the role then call this endpoint.

Copy link
Contributor Author

@sukangpunch sukangpunch Nov 11, 2025

Choose a reason for hiding this comment

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

해당 기능은, 아직 admin에서 멘토 지원서 approve 기능을 추가하지 않았기 때문이야, 다음 pr에 해당 내용을 추가할 예정

Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

@sukangpunch, 명확한 설명 감사합니다! 단계적 구현 방식을 이해했습니다.

현재 상황 확인:

  1. 이번 PR: 멘토 마이페이지 생성 엔드포인트 추가 (역할 가드 포함)
  2. 다음 PR: 어드민 승인 기능 + Role.MENTOR 할당 로직 추가

다음 PR에서 확인해주세요:

  • 멘토 지원서를 APPROVED로 변경할 때, 해당 사용자의 역할을 Role.MENTOR로 업데이트하는 로직이 포함되어야 합니다.
  • 그래야 승인받은 사용자가 이 엔드포인트를 호출하여 마이페이지를 생성할 수 있습니다.

현재 설계는 최종 플로우를 고려한 것으로 보이므로, 다음 PR에서 역할 할당 로직만 추가되면 완벽하게 동작할 것 같습니다! 👍

}
21 changes: 21 additions & 0 deletions src/main/java/com/example/solidconnection/mentor/domain/Mentor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,20 @@ public class Mentor extends BaseEntity {
@OneToMany(mappedBy = "mentor", cascade = CascadeType.ALL, orphanRemoval = true)
private List<Channel> channels = new ArrayList<>();

public Mentor(
String introduction,
String passTip,
long siteUserId,
Long universityId,
long termId
) {
this.introduction = introduction;
this.passTip = passTip;
this.siteUserId = siteUserId;
this.universityId = universityId;
this.termId = termId;
}

public void increaseMenteeCount() {
this.menteeCount++;
}
Expand Down Expand Up @@ -82,4 +96,11 @@ public void updateChannels(List<Channel> channels) {
}
}
}

public void createChannels(List<Channel> channels) {
for(Channel channel : channels) {
channel.updateMentor(this);
this.channels.add(channel);
}
}
Comment on lines +100 to +105
Copy link

@coderabbitai coderabbitai bot Nov 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

createChannels 메서드가 추가되었습니다. Null 안전성을 고려해주세요.

새로운 createChannels 메서드는 채널 리스트를 받아 멘토와 연결하고 내부 리스트에 추가합니다.

잠재적 이슈:
channels 매개변수가 null인 경우 NullPointerException이 발생합니다.

Null 체크를 추가하여 안전성을 높이는 것을 권장합니다:

 public void createChannels(List<Channel> channels) {
+    if (channels == null) {
+        return; // 또는 throw new IllegalArgumentException("channels must not be null");
+    }
     for(Channel channel : channels) {
         channel.updateMentor(this);
         this.channels.add(channel);
     }
 }

또는 더 간결하게:

 public void createChannels(List<Channel> channels) {
+    if (channels == null || channels.isEmpty()) {
+        return;
+    }
     for(Channel channel : channels) {
         channel.updateMentor(this);
         this.channels.add(channel);
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void createChannels(List<Channel> channels) {
for(Channel channel : channels) {
channel.updateMentor(this);
this.channels.add(channel);
}
}
public void createChannels(List<Channel> channels) {
if (channels == null) {
return;
}
for(Channel channel : channels) {
channel.updateMentor(this);
this.channels.add(channel);
}
}
Suggested change
public void createChannels(List<Channel> channels) {
for(Channel channel : channels) {
channel.updateMentor(this);
this.channels.add(channel);
}
}
public void createChannels(List<Channel> channels) {
if (channels == null || channels.isEmpty()) {
return;
}
for(Channel channel : channels) {
channel.updateMentor(this);
this.channels.add(channel);
}
}
🤖 Prompt for AI Agents 
In src/main/java/com/example/solidconnection/mentor/domain/Mentor.java around
lines 100 to 105, the new createChannels(List<Channel> channels) method can
throw a NullPointerException if the channels parameter is null; add a
null-safety guard such as returning immediately when channels is null or
replacing it with Collections.emptyList(), then iterate over the safe list and
for each channel call channel.updateMentor(this) and add to this.channels;
ensure the method handles an empty list gracefully and does not modify state
when input is null.

Copy link
Contributor Author

@sukangpunch sukangpunch Nov 11, 2025

Choose a reason for hiding this comment

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

프론트에서 무조건 리스트를 보내준다 가정 해도 null 체크를 하는것이 좋아?

Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

@sukangpunch, 좋은 질문입니다. 프론트엔드에서 무조건 리스트를 보낸다고 해도 null 체크를 하는 것이 더 안전합니다.

Null 체크를 권장하는 이유:

  1. 다양한 호출 경로: 이 메서드는 컨트롤러를 통한 API 호출뿐만 아니라, 백엔드 내부의 다른 서비스나 테스트 코드에서도 호출될 수 있습니다.

  2. 미래의 변경사항: 나중에 코드가 리팩토링되거나 새로운 기능이 추가될 때, 다른 경로에서 이 메서드를 호출할 수 있습니다.

  3. 방어적 프로그래밍: 도메인 엔티티의 public 메서드는 잘못된 입력으로부터 자신을 보호하는 것이 좋습니다.

대안:

Null 체크가 과하다고 생각되신다면, 서비스 레이어에서 검증하거나 @NonNull 어노테이션을 사용하는 방법도 있습니다:

public void createChannels(@NonNull List<Channel> channels) {
    for(Channel channel : channels) {
        channel.updateMentor(this);
        this.channels.add(channel);
    }
}

다만 도메인 엔티티 레벨에서 간단한 null 체크를 추가하는 것이 가장 안전하고 명시적입니다. 최종 결정은 팀의 코딩 컨벤션에 따라 하시면 됩니다!

🧠 Learnings used 
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 426
File: src/main/java/com/example/solidconnection/mentor/domain/Mentor.java:80-80
Timestamp: 2025-08-02T21:44:08.950Z
Learning: Mentor.updateChannels() 메서드에서 채널 삭제 시 originalChannelSize를 메서드 시작 시점에 고정하여 사용하므로, 반복문 내에서 리스트 크기가 변경되어도 올바른 수의 채널이 삭제된다. i < originalChannelSize 조건으로 넘치는 채널을 정확히 제거할 수 있다.

Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 375
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:47-53
Timestamp: 2025-07-05T17:54:42.475Z
Learning: MentorMyPageService에서 PUT 메서드 구현 시 전체 채널을 새로 생성하여 교체하는 방식을 사용하는 것이 PUT의 의미론적 특성과 일치하며, 트랜잭션 로킹 관점에서도 합리적인 접근이다.

}
8 changes: 7 additions & 1 deletion src/main/java/com/example/solidconnection/mentor/domain/MentorApplication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ public class MentorApplication extends BaseEntity {
@Column(nullable = false, name = "mentor_proof_url", length = 500)
private String mentorProofUrl;

@Column(nullable = false, name = "term_id")
private long termId;

private String rejectedReason;

@Column(nullable = false)
Expand All @@ -61,7 +64,7 @@ public class MentorApplication extends BaseEntity {

@Column(nullable = false)
@Enumerated(EnumType.STRING)
private MentorApplicationStatus mentorApplicationStatus = MentorApplicationStatus.PENDING;
private MentorApplicationStatus mentorApplicationStatus;

private static final Set<ExchangeStatus> ALLOWED =
Collections.unmodifiableSet(EnumSet.of(ExchangeStatus.STUDYING_ABROAD, ExchangeStatus.AFTER_EXCHANGE));
Expand All @@ -72,6 +75,7 @@ public MentorApplication(
Long universityId,
UniversitySelectType universitySelectType,
String mentorProofUrl,
long termId,
ExchangeStatus exchangeStatus
) {
validateExchangeStatus(exchangeStatus);
Expand All @@ -82,7 +86,9 @@ public MentorApplication(
this.universityId = universityId;
this.universitySelectType = universitySelectType;
this.mentorProofUrl = mentorProofUrl;
this.termId = termId;
this.exchangeStatus = exchangeStatus;
this.mentorApplicationStatus = MentorApplicationStatus.PENDING;
}

private void validateUniversitySelection(UniversitySelectType universitySelectType, Long universityId) {
Expand Down
13 changes: 12 additions & 1 deletion src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,23 @@
import com.example.solidconnection.mentor.domain.UniversitySelectType;
import com.example.solidconnection.siteuser.domain.ExchangeStatus;
import com.fasterxml.jackson.annotation.JsonProperty;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;

public record MentorApplicationRequest(
@NotNull(message = "교환 상태를 입력해주세요.")
@JsonProperty("preparationStatus")
ExchangeStatus exchangeStatus,

@NotNull(message = "대학교 선택 유형을 입력해주세요.")
UniversitySelectType universitySelectType,

@NotNull(message = "국가를 입력해주세요")
String country,
Long universityId

Long universityId,

@NotBlank(message = "학기를 입력해주세요.")
String term
Comment on lines +10 to +23
Copy link

@coderabbitai coderabbitai bot Nov 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Validation 어노테이션이 적절히 추가되었습니다. universityId 검증 여부를 확인해주세요.

변경사항을 정리하면 다음과 같습니다:

  1. 이전 리뷰 코멘트 반영 완료

    • term 필드에 @NotBlank 어노테이션이 추가되어 이전 리뷰 요청사항이 해결되었습니다.

  2. 기존 필드에 validation 추가

    • exchangeStatus: @NotNull 추가 ✓
    • universitySelectType: @NotNull 추가 ✓
    • country: @NotNull 추가 ✓

  3. 신규 필드 validation

    • term: @NotBlank 추가 ✓

다만, universityId 필드만 validation 어노테이션이 없는데, 이것이 의도된 것인지 확인이 필요합니다. 만약 "대학교 미정" 같은 케이스를 허용하는 것이라면 현재 구현이 맞지만, 필수 입력이어야 한다면 @NotNull 추가를 고려해주세요.

🏁 Script executed:

#!/bin/bash
# universityId가 필수 필드인지 확인하기 위해 서비스 로직과 도메인 모델을 검토

# MentorApplication 도메인에서 universityId 사용 패턴 확인
ast-grep --pattern $'class MentorApplication {
  $$$
  universityId
  $$$
}'

# MentorApplicationService에서 universityId 검증 로직 확인
rg -n -A5 -B5 "universityId" --type=java -g '**/MentorApplicationService.java'

# universitySelectType enum 정의 확인 (UNDECIDED 같은 옵션이 있는지)
fd -e java UniversitySelectType.java --exec cat {}

Length of output: 1688

universityId 필드에 @NotNull 어노테이션을 추가해주세요.

검증 결과를 정리하면 다음과 같습니다:

  1. 이전 리뷰 요청사항은 완벽하게 반영됨

    • term 필드에 @NotBlank 어노테이션이 올바르게 추가되었습니다.

  2. 기존 필드 검증은 적절함

    • exchangeStatus, universitySelectType, country에 @NotNull 어노테이션이 모두 추가되었습니다.

  3. universityId 필수 검증 추가 필요

    • MentorApplicationService에서 universityId가 null 체크 없이 직접 사용됩니다.
    • UniversitySelectType 열거형에 "미정" 옵션이 없으므로, 항상 대학교 선택이 필수입니다.
    • 따라서 universityId에도 @NotNull 어노테이션을 추가해야 합니다.
🤖 Prompt for AI Agents 
In
src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java
around lines 10 to 23, the universityId field is missing validation and must be
required; add the @NotNull annotation (with an appropriate message like "대학교를
선택해주세요.") to the universityId field so requests are validated before
MentorApplicationService uses it without null checks.

Copy link

@coderabbitai coderabbitai bot Nov 12, 2025

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

@sukangpunch sukangpunch Nov 17, 2025

Choose a reason for hiding this comment

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

@coderabbitai MentorApplication 에서 universityId 는 null 일수 있어

Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

@sukangpunch, 명확한 설명 감사합니다!

universityId가 null일 수 있다는 점을 확인했습니다. 제가 이전에 @NotNull 추가를 제안한 것은 잘못된 제안이었습니다. 멘토 지원 시 대학교가 미정이거나 선택되지 않은 경우를 허용하는 설계가 맞네요.

현재 구현이 올바르며, universityId에는 validation 어노테이션을 추가하지 않는 것이 맞습니다. 🐰

/)  /)

( •_• )
/>📝 noted!

✏️ Learnings added 
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java:10-23
Timestamp: 2025-11-17T06:30:49.484Z
Learning: MentorApplication 도메인에서 universityId는 null일 수 있으며, MentorApplicationRequest에서도 이 필드에 대한 NotNull validation을 추가하지 않아야 한다.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 486
File: src/main/java/com/example/solidconnection/university/repository/custom/UnivApplyInfoFilterRepositoryImpl.java:166-178
Timestamp: 2025-08-26T05:00:56.556Z
Learning: UnivApplyInfo의 텍스트 검색 기능에서는 University.koreanName이 아닌 UnivApplyInfo.koreanName을 검색 대상으로 사용하는 것이 의도된 설계이다.

) {
}
20 changes: 20 additions & 0 deletions src/main/java/com/example/solidconnection/mentor/dto/MentorMyPageCreateRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.example.solidconnection.mentor.dto;

import jakarta.validation.Valid;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import java.util.List;

public record MentorMyPageCreateRequest(
@NotBlank(message = "자기소개를 입력해주세요.")
String introduction,

@NotBlank(message = "합격 레시피를 입력해주세요.")
String passTip,

@NotNull
@Valid
List<ChannelRequest> channels
) {

}
3 changes: 3 additions & 0 deletions src/main/java/com/example/solidconnection/mentor/repository/MentorApplicationRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
import com.example.solidconnection.mentor.domain.MentorApplication;
import com.example.solidconnection.mentor.domain.MentorApplicationStatus;
import java.util.List;
import java.util.Optional;
import org.springframework.data.jpa.repository.JpaRepository;

public interface MentorApplicationRepository extends JpaRepository<MentorApplication, Long> {

boolean existsBySiteUserIdAndMentorApplicationStatusIn(long siteUserId, List<MentorApplicationStatus> mentorApplicationStatuses);

Optional<MentorApplication> findBySiteUserIdAndMentorApplicationStatus(long siteUserId, MentorApplicationStatus mentorApplicationStatus);
}
23 changes: 17 additions & 6 deletions src/main/java/com/example/solidconnection/mentor/service/MentorApplicationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import com.example.solidconnection.s3.service.S3Service;
import com.example.solidconnection.siteuser.domain.SiteUser;
import com.example.solidconnection.siteuser.repository.SiteUserRepository;
import com.example.solidconnection.term.domain.Term;
import com.example.solidconnection.term.repository.TermRepository;
import java.util.List;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -18,6 +20,7 @@
import org.springframework.web.multipart.MultipartFile;

import static com.example.solidconnection.common.exception.ErrorCode.MENTOR_APPLICATION_ALREADY_EXISTED;
import static com.example.solidconnection.common.exception.ErrorCode.TERM_NOT_FOUND;
import static com.example.solidconnection.common.exception.ErrorCode.USER_NOT_FOUND;

@Service
Expand All @@ -28,31 +31,39 @@ public class MentorApplicationService {
private final MentorApplicationRepository mentorApplicationRepository;
private final SiteUserRepository siteUserRepository;
private final S3Service s3Service;
private final TermRepository termRepository;

@Transactional
public void submitMentorApplication(
long siteUserId,
MentorApplicationRequest mentorApplicationRequest,
MultipartFile file
) {
if (mentorApplicationRepository.existsBySiteUserIdAndMentorApplicationStatusIn(
siteUserId,
List.of(MentorApplicationStatus.PENDING, MentorApplicationStatus.APPROVED))
) {
throw new CustomException(MENTOR_APPLICATION_ALREADY_EXISTED);
}
ensureNoPendingOrApprovedMentorApplication(siteUserId);

SiteUser siteUser = siteUserRepository.findById(siteUserId)
.orElseThrow(() -> new CustomException(USER_NOT_FOUND));
Term term = termRepository.findByName(mentorApplicationRequest.term())
.orElseThrow(() -> new CustomException(TERM_NOT_FOUND));
UploadedFileUrlResponse uploadedFile = s3Service.uploadFile(file, ImgType.MENTOR_PROOF);
MentorApplication mentorApplication = new MentorApplication(
siteUser.getId(),
mentorApplicationRequest.country(),
mentorApplicationRequest.universityId(),
mentorApplicationRequest.universitySelectType(),
uploadedFile.fileUrl(),
term.getId(),
mentorApplicationRequest.exchangeStatus()
);
mentorApplicationRepository.save(mentorApplication);
}

private void ensureNoPendingOrApprovedMentorApplication(long siteUserId) {
if (mentorApplicationRepository.existsBySiteUserIdAndMentorApplicationStatusIn(
siteUserId,
List.of(MentorApplicationStatus.PENDING, MentorApplicationStatus.APPROVED))
) {
throw new CustomException(MENTOR_APPLICATION_ALREADY_EXISTED);
}
}
}
46 changes: 44 additions & 2 deletions src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.example.solidconnection.mentor.service;

import static com.example.solidconnection.common.exception.ErrorCode.CHANNEL_REGISTRATION_LIMIT_EXCEEDED;
import static com.example.solidconnection.common.exception.ErrorCode.MENTOR_ALREADY_EXISTS;
import static com.example.solidconnection.common.exception.ErrorCode.MENTOR_APPLICATION_NOT_FOUND;
import static com.example.solidconnection.common.exception.ErrorCode.MENTOR_NOT_FOUND;
import static com.example.solidconnection.common.exception.ErrorCode.TERM_NOT_FOUND;
import static com.example.solidconnection.common.exception.ErrorCode.UNIVERSITY_NOT_FOUND;
Expand All @@ -9,9 +11,13 @@
import com.example.solidconnection.common.exception.CustomException;
import com.example.solidconnection.mentor.domain.Channel;
import com.example.solidconnection.mentor.domain.Mentor;
import com.example.solidconnection.mentor.domain.MentorApplication;
import com.example.solidconnection.mentor.domain.MentorApplicationStatus;
import com.example.solidconnection.mentor.dto.ChannelRequest;
import com.example.solidconnection.mentor.dto.MentorMyPageCreateRequest;
import com.example.solidconnection.mentor.dto.MentorMyPageResponse;
import com.example.solidconnection.mentor.dto.MentorMyPageUpdateRequest;
import com.example.solidconnection.mentor.repository.MentorApplicationRepository;
import com.example.solidconnection.mentor.repository.MentorRepository;
import com.example.solidconnection.siteuser.domain.SiteUser;
import com.example.solidconnection.siteuser.repository.SiteUserRepository;
Expand All @@ -36,6 +42,7 @@ public class MentorMyPageService {
private final SiteUserRepository siteUserRepository;
private final UniversityRepository universityRepository;
private final TermRepository termRepository;
private final MentorApplicationRepository mentorApplicationRepository;

@Transactional(readOnly = true)
public MentorMyPageResponse getMentorMyPage(long siteUserId) {
Expand All @@ -61,18 +68,53 @@ public void updateMentorMyPage(long siteUserId, MentorMyPageUpdateRequest reques
updateChannel(request.channels(), mentor);
}

private void updateChannel(List<ChannelRequest> channelRequests, Mentor mentor) {
List<Channel> newChannels = buildChannels(channelRequests);
mentor.updateChannels(newChannels);
}
Copy link
Member

@whqtker whqtker Nov 20, 2025

Choose a reason for hiding this comment

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

updateChannel 메서드와 createChannels 메서드가 거의 동일한 로직을 가지고 있는 거 같은데, 메서드 분리하지 않고 상위 메서드에 직접 작성하는 건 어떻게 생각하시나요 ?

Copy link
Contributor Author

@sukangpunch sukangpunch Nov 20, 2025

Choose a reason for hiding this comment

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

그 방식도 괜찮은 듯 합니다!
하지만, 상위 메서드로 넣으면, buildChannel 이라는 메서드를 직접 호출 해야 하는데, 추상화 레벨이 조금 낮아지는 느낌이라, 혹시 상위 메서드로 올려야 하는 추가적인 이유가 있을까요?

Copy link
Member

@whqtker whqtker Nov 20, 2025

Choose a reason for hiding this comment

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

이 부분은 정말 개인의 성향에 따라 느끼는 바가 다른 거 같아요
사실 전 buildChannel을 상위 메서드에 올려도 추상화 레벨이 그렇게 낮아진다고 느껴지지 않는 거 같습니다. update/createChannel 메서드 또한 단순히 변환하고 넘겨주는 역할만 수행하고 있어서, 분리할 정도까지는 아니라고 생각했습니다.

코멘트 의도가 단순 궁금해서였고, 구현하신 근거가 분명하셔서 굳이 바꿀 필요는 없을 거 같습니다 !


@Transactional
public void createMentorMyPage(long siteUserId, MentorMyPageCreateRequest request) {
validateUserCanCreateMentor(siteUserId);
validateChannelRegistrationLimit(request.channels());
MentorApplication mentorApplication = mentorApplicationRepository.findBySiteUserIdAndMentorApplicationStatus(siteUserId, MentorApplicationStatus.APPROVED)
.orElseThrow(() -> new CustomException(MENTOR_APPLICATION_NOT_FOUND));

Mentor mentor = new Mentor(
request.introduction(),
request.passTip(),
siteUserId,
mentorApplication.getUniversityId(),
mentorApplication.getTermId()
);

createChannels(request.channels(), mentor);
mentorRepository.save(mentor);
}
Comment on lines +76 to +93
Copy link

@coderabbitai coderabbitai bot Nov 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

APPROVED 지원서에서 universityId가 null인 경우에 대한 처리 검토가 필요해 보입니다.

  1. createMentorMyPage에서 mentorApplication.getUniversityId()를 그대로 Mentor 생성자에 넘기고 있는데, 이전 러닝에 따르면 MentorApplication의 universityId는 null이 될 수 있는 설계입니다.
  2. 이 상태로 null인 universityId가 Mentor에 저장되면, 이후 getMentorMyPage에서 universityRepository.findById(mentor.getUniversityId()) 호출 시 null id로 인한 런타임 문제(예: NPE 또는 잘못된 쿼리)가 발생할 수 있습니다.
  3. 비즈니스적으로 “APPROVED 가능한 지원서는 반드시 universityId를 가져야 한다”는 규칙이 있다면,
    • (선호) 승인 시점(어드민 플로우)에서 universityId를 강제/검증하거나,
    • (대안) 여기 createMentorMyPage에서 universityId가 null이면 별도 예외(예: UNIVERSITY_NOT_FOUND 또는 새로운 에러 코드)를 던지는 식으로 방어 코드를 두는 편이 안전할 것 같습니다.
  4. 어떤 단계에서 이 무결성을 보장할지 한 번만 정리해 두시면, 이후 다른 멘토 생성 경로가 추가되더라도 일관성을 유지하기 쉬울 것 같습니다.

Based on learnings

Also applies to: 95-99

🏁 Script executed:

#!/bin/bash
# Mentor 및 MentorApplication에서 universityId의 null 가능성과 사용 패턴을 빠르게 훑어보기 위한 스크립트입니다.

# 1) Mentor.universityId 필드 선언과 생성자/팩토리에서의 사용 위치 확인
rg -n "universityId" src/main/java/com/example/solidconnection/mentor -C3

# 2) MentorApplication.universityId가 어디에서 세팅/검증되는지 확인
rg -n "universityId" src/main/java/com/example/solidconnection/mentor -C3

# 3) MentoryMyPageService에서 universityId 관련 사용 위치 재확인
rg -n "getUniversityId" src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java -C3

Length of output: 23853

Mentor.universityId 필드 타입 불일치로 인한 null 언박싱 위험

코드 검증 결과, 다음 문제들을 발견했습니다:

  1. 타입 불일치 및 NPE 위험

    • Mentor.universityId 필드는 기본형 long이지만, 생성자 매개변수는 Long (nullable wrapper)
    • mentorApplication.getUniversityId()가 null일 경우, 기본형 long에 할당될 때 자동 언박싱되면서 NullPointerException 발생

  2. 비즈니스 로직 간극

    • MentorApplication의 설계에서 universitySelectType = OTHER일 때는 universityId가 null로 유지되도록 함 (line 102-104)
    • 하지만 승인 단계에서 OTHER 타입 지원서가 APPROVED되는 것을 막는 로직이 없음
    • 결과적으로 OTHER 타입 지원서가 승인된 후 createMentorMyPage 호출 시 null 언박싱 NPE 발생 가능

  3. 추가 영향 범위

    • getMentorMyPage (line 55): universityRepository.findById(mentor.getUniversityId())도 동일한 위험

해결 권장사항 (선택 사항)

  1. (선호) Mentor.universityId를 Long (nullable)으로 변경하고, getMentorMyPage에서 null 체크 추가
  2. (대안) 승인 시점에서 universitySelectType == CATALOG만 승인 가능하도록 비즈니스 규칙 강제
  3. (방어) createMentorMyPage에서 null universityId 시 명확한 예외 던지기 (예: UNIVERSITY_ID_REQUIRED)

현재 MentorBatchQueryRepository (line 59-60)에서 이미 null 체크를 하고 있는 것으로 보아, 이 문제를 인식한 코드가 존재합니다. 한 곳에서만 일관성 있게 처리하면 향후 유지보수가 용이할 것입니다.

Copy link
Contributor Author

@sukangpunch sukangpunch Nov 20, 2025

Choose a reason for hiding this comment

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

@coderabbitai 다음 pr에 admin 멘토 지원서 approve/reject 하는 기능을 추가 할건데, 거기서 대학 증명 자료는 있지만, 현재 서버가 가지고 있지 않은 대학에 대해서는(universityId가 null 인경우) 대학 데이터를 만들고 지원서에 넣은 다음 approve 하려고 해

Copy link

@coderabbitai coderabbitai bot Nov 20, 2025

Choose a reason for hiding this comment

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

@sukangpunch 아, 다음 PR에서 admin이 승인 시점에 대학 데이터를 생성하고 universityId를 채운 다음 APPROVED로 전환하는 구조군요! 그렇다면 제가 우려했던 부분이 비즈니스 로직 수준에서 해결되는 것으로 이해했습니다.

설계 방향에 대한 정리:

  1. APPROVED 상태의 지원서는 항상 universityId를 가진다는 불변 조건(invariant)이 admin 승인 플로우에서 보장됨
  2. null universityId는 PENDING 상태에서만 일시적으로 존재
  3. 현재 PR은 인프라 구축, 다음 PR에서 검증 로직 추가

이 접근은 합리적이고 관심사의 분리(separation of concerns)가 잘 되어 있다고 생각합니다!

참고로 추가 고려사항:

만약 혹시라도 admin 플로우에서 예외가 발생하거나 데이터 정합성 문제가 생길 경우를 대비하여, createMentorMyPage 메서드에 방어 코드를 추가하는 것도 고려해볼 수 있습니다:

MentorApplication mentorApplication = mentorApplicationRepository
    .findBySiteUserIdAndMentorApplicationStatus(siteUserId, MentorApplicationStatus.APPROVED)
    .orElseThrow(() -> new CustomException(MENTOR_APPLICATION_NOT_FOUND));

// 방어적 검증 (선택 사항)
if (mentorApplication.getUniversityId() == null) {
    throw new CustomException(MENTOR_APPLICATION_INVALID); // 또는 적절한 에러 코드
}

하지만 이는 선택 사항이며, admin 승인 플로우에서 충분히 보장된다면 불필요한 코드일 수 있습니다. 팀의 방어적 프로그래밍 정책에 따라 결정하시면 될 것 같습니다!

✏️ Learnings added 
Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:76-93
Timestamp: 2025-11-20T14:03:56.450Z
Learning: MentorApplication의 universityId는 PENDING 상태에서는 null일 수 있지만, admin이 승인(APPROVED)할 때 반드시 대학 데이터를 생성하고 universityId를 채운 후 승인하므로, APPROVED 상태의 MentorApplication은 항상 non-null universityId를 가진다는 것이 비즈니스 규칙이다.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: nayonsoso
Repo: solid-connection/solid-connect-server PR: 375
File: src/main/java/com/example/solidconnection/mentor/service/MentorMyPageService.java:47-53
Timestamp: 2025-07-05T17:54:42.475Z
Learning: MentorMyPageService에서 PUT 메서드 구현 시 전체 채널을 새로 생성하여 교체하는 방식을 사용하는 것이 PUT의 의미론적 특성과 일치하며, 트랜잭션 로킹 관점에서도 합리적인 접근이다.

Learnt from: sukangpunch
Repo: solid-connection/solid-connect-server PR: 562
File: src/main/java/com/example/solidconnection/mentor/dto/MentorApplicationRequest.java:10-23
Timestamp: 2025-11-17T06:30:49.502Z
Learning: MentorApplication 도메인에서 universityId는 null일 수 있으며, MentorApplicationRequest에서도 이 필드에 대한 NotNull validation을 추가하지 않아야 한다.


private void validateUserCanCreateMentor(long siteUserId) {
if (mentorRepository.existsBySiteUserId(siteUserId)) {
throw new CustomException(MENTOR_ALREADY_EXISTS);
}
}

private void validateChannelRegistrationLimit(List<ChannelRequest> channelRequests) {
if (channelRequests.size() > CHANNEL_REGISTRATION_LIMIT) {
throw new CustomException(CHANNEL_REGISTRATION_LIMIT_EXCEEDED);
}
}

private void updateChannel(List<ChannelRequest> channelRequests, Mentor mentor) {
private void createChannels(List<ChannelRequest> channelRequests, Mentor mentor) {
List<Channel> newChannels = buildChannels(channelRequests);
mentor.createChannels(newChannels);
}

private List<Channel> buildChannels(List<ChannelRequest> channelRequests) {
int sequence = CHANNEL_SEQUENCE_START_NUMBER;
List<Channel> newChannels = new ArrayList<>();
for (ChannelRequest request : channelRequests) {
newChannels.add(new Channel(sequence++, request.type(), request.url()));
}
mentor.updateChannels(newChannels);
return newChannels;
}
}
2 changes: 2 additions & 0 deletions src/main/java/com/example/solidconnection/term/repository/TermRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@
public interface TermRepository extends JpaRepository<Term, Long> {

Optional<Term> findByIsCurrentTrue();

Optional<Term> findByName(String name);
}
6 changes: 6 additions & 0 deletions src/main/resources/db/migration/V38__add_term_id_to_mentor_application.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
ALTER TABLE mentor_application
ADD COLUMN term_id BIGINT NOT NULL;

ALTER TABLE mentor_application
ADD CONSTRAINT fk_mentor_application_term_id
FOREIGN KEY (term_id) REFERENCES term(id);
2 changes: 1 addition & 1 deletion src/main/resources/secret
Open in desktop
Submodule secret updated from 2f4a16 to 8300cd
5 changes: 5 additions & 0 deletions src/test/java/com/example/solidconnection/mentor/fixture/MentorApplicationFixture.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.example.solidconnection.mentor.domain.MentorApplicationStatus;
import com.example.solidconnection.mentor.domain.UniversitySelectType;
import com.example.solidconnection.siteuser.domain.ExchangeStatus;
import com.example.solidconnection.term.fixture.TermFixture;
import lombok.RequiredArgsConstructor;
import org.springframework.boot.test.context.TestComponent;

Expand All @@ -12,6 +13,7 @@
public class MentorApplicationFixture {

private final MentorApplicationFixtureBuilder mentorApplicationFixtureBuilder;
private final TermFixture termFixture;

private static final String DEFAULT_COUNTRY_CODE = "US";
private static final String DEFAULT_PROOF_URL = "/mentor-proof.pdf";
Expand All @@ -28,6 +30,7 @@ public class MentorApplicationFixture {
.universityId(universityId)
.universitySelectType(selectType)
.mentorProofUrl(DEFAULT_PROOF_URL)
.termId(termFixture.현재_학기("2025-1").getId())
.exchangeStatus(DEFAULT_EXCHANGE_STATUS)
.create();
}
Expand All @@ -43,6 +46,7 @@ public class MentorApplicationFixture {
.universityId(universityId)
.universitySelectType(selectType)
.mentorProofUrl(DEFAULT_PROOF_URL)
.termId(termFixture.현재_학기("2025-1").getId())
.exchangeStatus(DEFAULT_EXCHANGE_STATUS)
.mentorApplicationStatus(MentorApplicationStatus.APPROVED)
.create();
Expand All @@ -59,6 +63,7 @@ public class MentorApplicationFixture {
.universityId(universityId)
.universitySelectType(selectType)
.mentorProofUrl(DEFAULT_PROOF_URL)
.termId(termFixture.현재_학기("2025-1").getId())
.exchangeStatus(DEFAULT_EXCHANGE_STATUS)
.mentorApplicationStatus(MentorApplicationStatus.REJECTED)
.create();
Expand Down
Loading
Loading