Skip to content

Conversation

@junjinyun
Copy link
Contributor

@junjinyun junjinyun commented Aug 24, 2025

Issue

변경 내용

  • 모집일정의 key값에 Generation 추가 및 RecruitService 의 조회 부분에 반영
  • RecruitService 에 기수수정, 기수 조회 기능 추가
  • AdminRecruitController 에 기수 수정기능 추가
  • 멤버 엔티티에 기수정보 필드 추가 및 회원가입 기능에 RecruitService 를 이용해 자동으로 현재 기수로 값을 부여되게 지정하며, 회원가입 시 generation속성을 포함하여 기수 지정도 가능하게 지정

테스트

  • 로컬에서 테스트 완료
  • 기존 기능에 영향 없음 확인
  • 연관된 기존 테스트 코드들을 일부 수정(기수 코드 수정 반영)하여 모든 테스트가 정상작동 확인
image image

기수 수정 정상작동 확인

image image image

회원가입 정상 작동 및 기수 기능 작동 확인

리뷰 요구사항(필요시)

  • Generation 을 통해 관리 하는게 아닌 단순히 클라이언트에서 기수 정보를 받아서 값을 대입 시키는게 더 나을지(구조를 간단하게 만드는게 더 좋을지)
  • 이슈내용 대로 멤버 부분은 제외하고 모집일정 부분에만 적용시키는것으로 되돌릴지
  • 각 기능의 완료 이후 반환값 형식 같은 부분 수정 여부

@junjinyun junjinyun self-assigned this Aug 24, 2025
@junjinyun junjinyun linked an issue Aug 24, 2025 that may be closed by this pull request
@junjinyun junjinyun force-pushed the refactor/DASOMBE-16-generation branch from 7a40405 to 9f31849 Compare August 25, 2025 17:57
@dohy-eon dohy-eon requested review from dohy-eon, hodoon and ysw789 August 27, 2025 00:28
@junjinyun junjinyun force-pushed the refactor/DASOMBE-16-generation branch from 0079bfe to 45abfc2 Compare August 27, 2025 02:09
Copy link
Member

@ysw789 ysw789 left a comment

Choose a reason for hiding this comment

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

작업 고생하셨습니다! 아래 사항 참고 부탁드립니다.

  1. GenerationService의 역할
    GenerationService는 모집 기수 관련 로직을 포함하고 있는데, RecruitService가 이미 부원 모집 로직을 다루고 있으므로 RecruitService에 통합하는 것이 더 자연스러워 보입니다.

  2. 패키지 구조(global → domain)
    global 패키지는 특정 도메인에 종속되지 않는 공용 처리만을 위한 공간입니다. 따라서 global/generation 보다는 domain/generation으로 이동하는 것이 패키지 구조에 더 적합합니다.

  3. Generation 엔티티 관리 방식
    Generation이 별도의 엔티티로 정의되어 있음에도 모집 기수를 RecruitRepository에서 조회하는 부분(RecruitServiceImpl 65번째 행)은 의도가 불분명해 보입니다. 개인적으로는 Recruit에서 모집 설정과 함께 기수까지 한 번에 관리하는 방식이 더 일관성 있어 보입니다.

리뷰 요구사항 피드백

  1. Generation 을 통해 관리 하는게 아닌 단순히 클라이언트에서 기수 정보를 받아서 값을 대입 시키는게 더 나을지
    이슈의 목적이 모집 기수를 서버에서 관리하기 위함인 것 같으므로 현상태를 유지하는 것이 좋아보입니다.
  2. 이슈내용 대로 멤버 부분은 제외하고 모집일정 부분에만 적용시키는것으로 되돌릴지
    이 부분은 논의가 필요해보네요. 신입 부원이 아닌 사람이 가입했을 경우도 있으므로 다른 처리가 필요해보입니다. @dohy-eon @hodoon

Comment on lines 35 to 46
if(config.getKey() == ConfigKey.GENERATION) { //기수 조회 추가
String currentGeneration = generationService.getCurrentGeneration();
return RecruitConfigResponseDto.builder()
.key(ConfigKey.GENERATION)
.value(currentGeneration)
.build();
} else if(config.getKey() == ConfigKey.INTERVIEW_TIME_START || config.getKey() == ConfigKey.INTERVIEW_TIME_END) {
return config.toTimeResponse();
} else {
return config.toResponse();
}
})
Copy link
Member

Choose a reason for hiding this comment

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

if-else 문이 길어질 경우 switch-case 문을 활용해보세요


}
// 기수 정보를 저장할 필드 추가
@Column(name = "generation", length = 16, nullable = false)
Copy link
Member

Choose a reason for hiding this comment

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

기수 번호는 2자리수인데 속성 길이가 16이나 필요할까요?

@junjinyun
Copy link
Contributor Author

기수 기능 Recruit에 통합 및 회원가입시 선택적으로 기수 입력기능 추가, 코드 정리 완료했습니다.

@junjinyun junjinyun requested a review from ysw789 August 28, 2025 11:03
@junjinyun junjinyun merged commit 0f7e3ce into dev Aug 29, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] 모집 일정에 기수 기능 추가

4 participants